linux-kernel - Re: [PATCH v3 20/25] irqchip/gic-v5: Add GICv5 PPI support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <864ixvh4ss.wl-maz@kernel.org>
Date: Thu, 08 May 2025 09:42:27 +0100
From: Marc Zyngier <maz@...nel.org>
To: Lorenzo Pieralisi <lpieralisi@...nel.org>
Cc: Thomas Gleixner <tglx@...utronix.de>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will@...nel.org>,
	Arnd Bergmann <arnd@...db.de>,
	Sascha Bischoff <sascha.bischoff@....com>,
	Timothy Hayes <timothy.hayes@....com>,
	"Liam R. Howlett" <Liam.Howlett@...cle.com>,
	Mark Rutland <mark.rutland@....com>,
	Jiri Slaby <jirislaby@...nel.org>,
	linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org
Subject: Re: [PATCH v3 20/25] irqchip/gic-v5: Add GICv5 PPI support

On Thu, 08 May 2025 08:42:41 +0100,
Lorenzo Pieralisi <lpieralisi@...nel.org> wrote:
> 
> On Wed, May 07, 2025 at 04:57:07PM +0200, Thomas Gleixner wrote:
> > On Wed, May 07 2025 at 14:52, Marc Zyngier wrote:
> > > On Wed, 07 May 2025 14:42:42 +0100,
> > > Thomas Gleixner <tglx@...utronix.de> wrote:
> > >> 
> > >> On Wed, May 07 2025 at 10:14, Marc Zyngier wrote:
> > >> > On Tue, 06 May 2025 16:00:31 +0100,
> > >> > Thomas Gleixner <tglx@...utronix.de> wrote:
> > >> >> 
> > >> >> How does this test distinguish between LEVEL_LOW and LEVEL_HIGH? It only
> > >> >> tests for level, no? So the test is interesting at best ...
> > >> >
> > >> > There is no distinction between HIGH and LOW, RISING and FALLING, in
> > >> > any revision of the GIC architecture.
> > >> 
> > >> Then pretending that there is a set_type() functionality is pretty daft
> > >
> > > You still need to distinguish between level and edge when this is
> > > programmable (which is the case for a subset of the PPIs).
> > 
> > Fair enough, but can we please add a comment to this function which
> > explains this oddity.
> 
> Getting back to this, I would need your/Marc's input on this.
> 
> I think it is fair to remove the irq_set_type() irqchip callback for
> GICv5 PPIs because there is nothing to set, as I said handling mode
> for these IRQs is fixed. I don't think this can cause any trouble
> (IIUC a value within the IRQF_TRIGGER_MASK should be set on requesting
> an IRQ to "force" the trigger to be programmed and even then core code
> would not fail if the irq_set_type() irqchip callback is not
> implemented).
> 
> I am thinking about *existing* drivers that request GICv3 PPIs with
> values in IRQF_TRIGGER_MASK set (are there any ? Don't think so but you
> know better than I do), when we switch over to GICv5 we would have no
> irq_set_type() callback for PPIs but I think we are still fine, not
> implementing irqchip.irq_set_type() is correct IMO.

Nobody seems to use a hardcoded trigger (well, there is one exception,
but that's to paper over a firmware bug).

> On the other hand, given that on GICv5 PPI handling mode is fixed,
> do you think that in the ppi_irq_domain_ops.translate() callback,
> I should check the type the firmware provided and fail the translation
> if it does not match the HW hardcoded value ?

Why? The fact that the firmware is wrong doesn't change the hardware
integration. It just indicates that whoever wrote the firmware didn't
read the documentation.

Even more, I wonder what the benefit of having that information in the
firmware tables if the only thing that matters in the immutable HW
view. Yes, having it in the DT/ACPI simplifies the job of the kernel
(only one format to parse). But it is overall useless information.

> Obviously if firmware exposes the wrong type that's a firmware bug
> but I was wondering whether it is better to fail the firmware-to-Linux
> IRQ translation if the firmware provided type is wrong rather than carry
> on pretending that the type is correct (I was abusing the irq_set_type()
> callback to do just that - namely, check that the type provided by
> firmware matches HW but I think that's the wrong place to put it).

I don't think there is anything to do. Worse case, you spit a
pr_warn_once() and carry on.

Thanks,

	M.

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