Re: [PATCH v3] irqchip: gic-v3-its: irq_pipeline: Fix OOB context in-band lock warning for ITS lock

linz <[email protected]> writes:

> The warning shown below could be observed on dovetail 6.6 with arm64
> architecture when CONFIG_DEBUG_IRQ_PIPELINE is enabled.
>
> The reason is the usage of a raw_spinlock_t in hard IRQ masking code
> path. A migration to hard_spinlock_t fixes this issue.
>
> [=C2=A0 =C2=A0 1.510903] IRQ pipeline: some code running in oob context '=
Xenomai'
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0called an in-band only routine
> [=C2=A0 =C2=A0 1.510912] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G S=C2=A0=
 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A06.6.63-dovetail2 #8
> [=C2=A0 =C2=A0 1.510918] Hardware name: Pe2204 DEMO DDR4 (DT)
> [=C2=A0 =C2=A0 1.510920] IRQ stage: Xenomai
> [=C2=A0 =C2=A0 1.510923] Call trace:
> [=C2=A0 =C2=A0 1.510926]=C2=A0 dump_backtrace+0x90/0xe4
> [=C2=A0 =C2=A0 1.510940]=C2=A0 show_stack+0x14/0x1c
> [=C2=A0 =C2=A0 1.510946]=C2=A0 dump_stack_lvl+0x84/0xc8
> [=C2=A0 =C2=A0 1.510953]=C2=A0 dump_stack+0x14/0x1c
> [=C2=A0 =C2=A0 1.510957]=C2=A0 check_inband_stage+0xb0/0xc8
> [=C2=A0 =C2=A0 1.510965]=C2=A0 inband_irq_save+0xc/0x28
> [=C2=A0 =C2=A0 1.510971]=C2=A0 _raw_spin_lock_irqsave+0x14/0x90
> [=C2=A0 =C2=A0 1.510977]=C2=A0 its_send_single_command+0x24/0x154
> [=C2=A0 =C2=A0 1.510984]=C2=A0 lpi_update_config+0x9c/0x144
> [=C2=A0 =C2=A0 1.510990]=C2=A0 its_mask_irq+0x2c/0x64
> [=C2=A0 =C2=A0 1.510997]=C2=A0 irq_chip_mask_parent+0x18/0x20
> [=C2=A0 =C2=A0 1.511005]=C2=A0 its_mask_msi_irq+0x1c/0x28
> [=C2=A0 =C2=A0 1.511012]=C2=A0 handle_fasteoi_irq+0x1cc/0x2b4
> [=C2=A0 =C2=A0 1.511016]=C2=A0 generic_pipeline_irq_desc+0x6c/0xa4
> [=C2=A0 =C2=A0 1.511021]=C2=A0 generic_handle_domain_irq+0x18/0x20
> [=C2=A0 =C2=A0 1.511028]=C2=A0 gic_handle_irq+0x4c/0x120
> [=C2=A0 =C2=A0 1.511032]=C2=A0 handle_irq_pipelined+0x40/0x64
> [=C2=A0 =C2=A0 1.511038]=C2=A0 call_on_irq_stack+0x24/0x30
> [=C2=A0 =C2=A0 1.511044]=C2=A0 do_interrupt_handler+0x138/0x158
> [=C2=A0 =C2=A0 1.511050]=C2=A0 el1_interrupt+0x40/0x110
> [=C2=A0 =C2=A0 1.511055]=C2=A0 el1h_64_irq_handler+0x14/0x1c
> [=C2=A0 =C2=A0 1.511061]=C2=A0 el1h_64_irq+0x64/0x68
> [=C2=A0 =C2=A0 1.511064]=C2=A0 default_idle_call+0x30/0x78
> [=C2=A0 =C2=A0 1.511071]=C2=A0 do_idle+0x128/0x150
> [=C2=A0 =C2=A0 1.511077]=C2=A0 cpu_startup_entry+0x34/0x38
> [=C2=A0 =C2=A0 1.511081]=C2=A0 kernel_init+0x0/0x1d4
> [=C2=A0 =C2=A0 1.511088]=C2=A0 arch_post_acpi_subsys_init+0x0/0x8
> [=C2=A0 =C2=A0 1.511096]=C2=A0 start_kernel+0x504/0x5cc
> [=C2=A0 =C2=A0 1.511103]=C2=A0 __primary_switched+0xbc/0xc4
>
> Signed-off-by: zhanglin <[email protected]>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v=
3-its.c
> index 0e57735eac..3f2b02b8f4 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -94,7 +94,7 @@ struct its_device;
>   * list.
>   */
>  struct its_node {
> -	raw_spinlock_t		lock;
> +	hard_spinlock_t		lock;
>  	struct mutex		dev_alloc_lock;
>  	struct list_head	entry;
>  	void __iomem		*base;

This is correct, but we need more changes for this irqchip driver,
particularly for handlers mentioned by this document [1], like irq_mask,
irq_unmask.

For instance, anything which calls lpi_update_config() needs to be made
oob-capable, so it looks like gic_data_rdist_cpu(cpu)->rd_lock needs
care too.

[1] https://v4.xenomai.org/dovetail/porting/irqflow/index.html#irqchip-fixup

--=20
Philippe.