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 for Android: free password hash cracker in your pocket

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

Message-ID:
 <OSCPR01MB163106E2269F8EB49E507A1F3D5D5A@OSCPR01MB16310.jpnprd01.prod.outlook.com>
Date: Fri, 21 Nov 2025 02:31:34 +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>,
	"Koichi Okuno (Fujitsu)" <fj2767dz@...itsu.com>
Subject: RE: [PATCH v4] ACPI: AGDI: Add interrupt signaling mode support

Hi Hanjun and Will,

> On 2025/11/18 18:09, Kazuhiro Abe (Fujitsu) wrote:
> > 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.
> 
> I had a talk to firmware guy about this, they say even with both supported, the
> platform will use the same interrupt number to trigger, and the spec says:
> 
> While an SDEI event handler is registered, the platform is allowed to not
> generate the wired interrupt
> 
> I think this patch is ready to go.

Hanjun, Thank you for your review.

Will, as all comments have been addressed, please consider applying this patch.

Best Regards,
Kazuhiro Abe

> 
> Thanks
> Hanjun