Re: [PATCH v4] irqchip: gic-v3-its: irq_pipeline: Fix OOB context in-band lock warning for ITS lock
Philippe Gerum <[email protected]>
| Newsgroups | dev.linux.lists.xenomai |
|---|---|
| Message-ID | <[email protected]> |
Florian Bezdeka <[email protected]> writes: > On Tue, 2026-08-04 at 17:30 +0800, linz wrote: >> At 2026-08-04 16:59:00, "Florian Bezdeka" <[email protected]> wrote: >> > On Tue, 2026-08-04 at 16:53 +0800, linz wrote: >> > > 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. >> > > >> > > [ 1.510903] IRQ pipeline: some code running in oob context 'Xenomai' >> > > called an in-band only routine >> > > [ 1.510912] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G S 6.6.63-dovetail2 #8 >> > > [ 1.510918] Hardware name: Pe2204 DEMO DDR4 (DT) >> > > [ 1.510920] IRQ stage: Xenomai >> > > [ 1.510923] Call trace: >> > > [ 1.510926] dump_backtrace+0x90/0xe4 >> > > [ 1.510940] show_stack+0x14/0x1c >> > > [ 1.510946] dump_stack_lvl+0x84/0xc8 >> > > [ 1.510953] dump_stack+0x14/0x1c >> > > [ 1.510957] check_inband_stage+0xb0/0xc8 >> > > [ 1.510965] inband_irq_save+0xc/0x28 >> > > [ 1.510971] _raw_spin_lock_irqsave+0x14/0x90 >> > > [ 1.510977] its_send_single_command+0x24/0x154 >> > > [ 1.510984] lpi_update_config+0x9c/0x144 >> > > [ 1.510990] its_mask_irq+0x2c/0x64 >> > > [ 1.510997] irq_chip_mask_parent+0x18/0x20 >> > > [ 1.511005] its_mask_msi_irq+0x1c/0x28 >> > > [ 1.511012] handle_fasteoi_irq+0x1cc/0x2b4 >> > > [ 1.511016] generic_pipeline_irq_desc+0x6c/0xa4 >> > > [ 1.511021] generic_handle_domain_irq+0x18/0x20 >> > > [ 1.511028] gic_handle_irq+0x4c/0x120 >> > > [ 1.511032] handle_irq_pipelined+0x40/0x64 >> > > [ 1.511038] call_on_irq_stack+0x24/0x30 >> > > [ 1.511044] do_interrupt_handler+0x138/0x158 >> > > [ 1.511050] el1_interrupt+0x40/0x110 >> > > [ 1.511055] el1h_64_irq_handler+0x14/0x1c >> > > [ 1.511061] el1h_64_irq+0x64/0x68 >> > > [ 1.511064] default_idle_call+0x30/0x78 >> > > [ 1.511071] do_idle+0x128/0x150 >> > > [ 1.511077] cpu_startup_entry+0x34/0x38 >> > > [ 1.511081] kernel_init+0x0/0x1d4 >> > > [ 1.511088] arch_post_acpi_subsys_init+0x0/0x8 >> > > [ 1.511096] start_kernel+0x504/0x5cc >> > > [ 1.511103] __primary_switched+0xbc/0xc4 >> > > >> > > Beyond that, note that IRQ chip handlers such as irq_mask / irq_unmask run in >> > > OOB context. For instance, any code path calling lpi_update_config() must >> > > be oob‑capable, so gic_data_rdist_cpu(cpu)->rd_lock and its_vpe->vpe_lock are >> > > modified in this patch. >> > > >> > > Convert: >> > > - its_node->lock >> > > - rdists.rd_lock >> > > - its_vpe->vpe_lock >> > > from raw_spinlock_t to hard_spinlock_t. >> > > >> > > Signed-off-by: linz <[email protected]> >> > > --- >> > > drivers/irqchip/irq-gic-v3-its.c | 2 +- >> > > include/linux/irqchip/arm-gic-v3.h | 2 +- >> > > include/linux/irqchip/arm-gic-v4.h | 2 +- >> > > 3 files changed, 3 insertions(+), 3 deletions(-) >> > > >> > > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-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; >> > > diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h >> > > index 7286913654..9dcbf4b331 100644 >> > > --- a/include/linux/irqchip/arm-gic-v3.h >> > > +++ b/include/linux/irqchip/arm-gic-v3.h >> > > @@ -613,7 +613,7 @@ >> > > >> > > struct rdists { >> > > struct { >> > > - raw_spinlock_t rd_lock; >> > > + hard_spinlock_t rd_lock; >> > > void __iomem *rd_base; >> > > struct page *pend_page; >> > > phys_addr_t phys_base; >> > > diff --git a/include/linux/irqchip/arm-gic-v4.h b/include/linux/irqchip/arm-gic-v4.h >> > > index bf9e064028..d7afd9d2cd 100644 >> > > --- a/include/linux/irqchip/arm-gic-v4.h >> > > +++ b/include/linux/irqchip/arm-gic-v4.h >> > > @@ -68,7 +68,7 @@ struct its_vpe { >> > > * Ensures mutual exclusion between affinity setting of the >> > > * vPE and vLPI operations using vpe->col_idx. >> > > */ >> > > - raw_spinlock_t vpe_lock; >> > > + hard_spinlock_t vpe_lock; >> > >> > Can you please share the call stack where this lock is involved in an >> > OOB code path? >> > >> > The other two locks are fine. I'm unsure about this one here. >> >> > >> The vpe_lock can be taken in OOB context via the following call chain when >> handling vLPI interrupt masking: >> >> its_mask_irq() >> lpi_update_config() >> direct_lpi_inv() > > This one here is protected by is_v4_1(), which seems gic-v4 specific. > The location (arm-gic-v4.h) is also pointing in that direction. Most > likely the reason why you never saw any warning for that lock. You are > on v3, right? > > The change itself is correct but v4 specific and merged into a patch > titled v3. That's confusing. > > Proposal: Skip that lock for now and revisit it once the v4 irq chip is > made OOB aware. (Nice additional contribution possibility ;-)) > > Philippe, any comments from your side? > gic-v3 and v4 implementations share some type definitions via arm-gic-v4.h. Since we need to fix up anything down the lpi_update_config() path, it looks like the impact of enabling v3 for oob context is going to spread to v4. Looking at the its_vm definition in arm-gic-v4.h, there is a helpful comment stating that the following lock order applies: vmapp_lock -> vpe_lock ->vmovp_lock. For this reason, if we are going to convert vpe_lock to a hard lock, then vmovp_lock must be converted in the same move too due to nesting constraints. Which means in turn that any code path going through its_send_vmovp() needs to be inspected. -- Philippe.