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.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.