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: <6d110b4f-a1e6-2092-f529-bd527fc00f6d@huawei.com>
Date: Thu, 20 Nov 2025 21:27:26 +0800
From: Hanjun Guo <guohanjun@...wei.com>
To: "Kazuhiro Abe (Fujitsu)" <fj1078ii@...itsu.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

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.

Thanks
Hanjun