Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 23 Aug 2021 10:33:40 +0100
Message-ID: <87czq4qzd7.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Valentin Schneider <valentin.schneider@arm.com>
Cc:     Guenter Roeck <linux@roeck-us.net>, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] irqchip/gic: Convert to handle_strict_flow_irq()
In-Reply-To: <878s0t6s7p.mognet@arm.com>
References: <20210814194737.GA3951530@roeck-us.net>
        <87sfzb7jeo.mognet@arm.com>
        <87eeav19mc.wl-maz@kernel.org>
        <87k0kk7w0c.mognet@arm.com>
        <87czqasn9u.wl-maz@kernel.org>
        <878s0t6s7p.mognet@arm.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Sun, 22 Aug 2021 23:16:10 +0100,
Valentin Schneider <valentin.schneider@arm.com> wrote:
> 
> On 18/08/21 17:58, Marc Zyngier wrote:
> > On Tue, 17 Aug 2021 01:30:43 +0100,
> > Valentin Schneider <valentin.schneider@arm.com> wrote:
> >> Are we guaranteed to have
> >>
> >>   .irq_ack \in {NULL, irq_chip_ack_parent}
> >>
> >> for all intermediate (!root) irqchips? I don't see why that wouldn't
> >> be the case, and with that in mind what you described makes sense to
> >> me.
> >
> > An intermediate layer is allowed to implement its own irq_ack that is
> > not irq_chip_ack_parent, but it then has to call irq_chip_ack_parent
> > itself.
> >
> 
> Right, makes sense.
> 
> > There is the bizarre case of drivers/gpio/gpio-thunderx.c that changes
> > the irqchip flow to use either handle_fasteoi_ack_irq or
> > handle_fasteoi_mask_irq, which won't play very nicely with this.
> > Someone said Cavium?
> >
> 
> Humph...
> 
> I'm not familiar at all with the gpiolib irqchips, but I was under the
> impression those would involve chained IRQs (it does appear to be the case
> for the pl061 GPIOs on a Juno). For those, the innermost desc would be handled
> via chained_irq_{enter, exit}() [!!!], and the outermost one via whatever
> flow was installed by the relevant driver.

Not all of them are built like this. There is actually a bunch of
these build as full hierarchies (QC, nvidia and some others).

> I can't easily grok what goes on between that gpio-thunderx.c driver and
> gpiolib, but since that GPIO chip has
> 
>         .irq_eoi		= irq_chip_eoi_parent,
> 
> and
> 
>         girq->parent_domain =
>                 irq_get_irq_data(txgpio->msix_entries[0].vector)->domain;
> 
> (GPIOs hooked to MSI-X? Do I want to know?)

It's good, isn't it? TX1 has all its HW appearing as PCI, even if it
clearly isn't PCI underneath.

> 
> I'm guessing it is *not* chained, which means the irq_set_handler_locked()
> affects the entire stack :/

It does. We can probably fix that, but I won't be able to test (my TX1
was taken away a few months ago...). I'll accept body donations, for
scientific purposes.

> 
> [!!!] Speaking of chained IRQs, I'm now thinking this series breaks them;
> chained_irq_enter() + chained_irq_exit() will only issue an ->irq_eoi(),
> skipping the ->irq_ack()... One more thing to add to the list!

Urghh... Yeah, that's awful.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.