linux-kernel - RE: [PATCH v4] ACPI: AGDI: Add interrupt signaling mode 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] [day] [month] [year] [list]

Message-ID:
 <OSCPR01MB163100DEFA746D8A44FA0A1E3D5D6A@OSCPR01MB16310.jpnprd01.prod.outlook.com>
Date: Tue, 18 Nov 2025 10:09:23 +0000
From: "Kazuhiro Abe (Fujitsu)" <fj1078ii@...itsu.com>
To: 'Hanjun Guo' <guohanjun@...wei.com>, Will Deacon <will@...nel.org>
CC: Lorenzo Pieralisi <lpieralisi@...nel.org>, Sudeep Holla
	<sudeep.holla@....com>, "Rafael J. Wysocki" <rafael@...nel.org>, Len Brown
	<lenb@...nel.org>, "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, Ilkka Koskinen
	<ilkka@...amperecomputing.com>, Catalin Marinas <catalin.marinas@....com>
Subject: RE: [PATCH v4] ACPI: AGDI: Add interrupt signaling mode support

Hi Hanjun,

Thanks for your comments.

> >>> AGDI has two types of signaling modes: SDEI and interrupt.
> >>> Currently, the AGDI driver only supports SDEI.
> >>> Therefore, add support for interrupt signaling mode The interrupt
> >>> vector is retrieved from the AGDI table, and call panic function
> >>> when an interrupt occurs.
> >>>
> >>> Reviewed-by: Ilkka Koskinen <ilkka@...amperecomputing.com>
> >>> Signed-off-by: Kazuhiro Abe <fj1078ii@...jp.fujitsu.com>
> >>> ---
> >>> Hanjun, I have addressed all your comments.
> >>> Please review them.
> >>>
> >>> v3->v4
> >>>    - Add a comment to the flags member.
> >>>    - Fix agdi_interrupt_probe.
> >>>    - Fix agdi_interrupt_remove.
> >>>    - Add space in struct initializsation.
> >>>    - Delete curly braces.
> >>
> >> Looks good to me,
> >>
> >> Acked-by: Hanjun Guo <guohanjun@...wei.com>
> >
> > I wasn't cc'd on the original patch but I couldn't figure out why it
> > uses IRQF_NO_AUTOEN when requesting the irq given that the first thing
> > it does is enable it.
> 
> Sorry for the late reply, seems this comment was addressed.
> 
> But when I go back to read the latest AGDI spec [0], I see the signaling mode
> can be SDEI *and* interrupt:
> 
> Flags node structures 1 36 Bits [1:0]: Signaling mode:
> 
> 2: Both SDEI and Interrupt-based signaling is supported. While an SDEI event
> handler is registered, the platform is allowed to not generate the wired interrupt
> 
> So which means we need to support both SDEI and interrupt when the
> signaling mode is 2, but for now, the code is
> 
> +	if (agdi_table->flags & ACPI_AGDI_SIGNALING_MODE)
> +		pdata.gsiv = agdi_table->gsiv;
> +	else
> +		pdata.sdei_event = agdi_table->sdei_event;
> 
> Kazuhiro, could you take a look again?

I don't think enabling both interrupt and SDEI makes sense.
Therefore, we need to choose one but we can't know which one is preferred for each platform.
On the other hand, before this patch the code is:

       if (agdi_table->flags & ACPI_AGDI_SIGNALING_MODE) {
               pr_warn("Interrupt signaling is not supported");
               goto err_put_table;
       }

...and SDEI is used when mode is 2. This behavior will not be changed by this patch.
Therefore, It seems for me this patch is reasonable for now.
I would appreciate your thoughts on this.

Best Regards,
Kazuhiro Abe

> 
> Thanks
> Hanjun
> 
> [0]: https://developer.arm.com/documentation/den0093/latest/