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:
 <TY3PR01MB9983025D434CAA2CDF83656BD511A@TY3PR01MB9983.jpnprd01.prod.outlook.com>
Date: Fri, 19 Sep 2025 05:05:04 +0000
From: "Kazuhiro Abe (Fujitsu)" <fj1078ii@...itsu.com>
To: 'Ilkka Koskinen' <ilkka@...amperecomputing.com>, 'Hanjun Guo'
	<guohanjun@...wei.com>, 'Sudeep Holla' <sudeep.holla@....com>
CC: 'Lorenzo Pieralisi' <lpieralisi@...nel.org>, "'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>
Subject: RE: [PATCH v3] ACPI: AGDI: Add interrupt signaling mode support

Hi Hanjun & Sudeep

> Hi Ilkka
> 
> > Hi Kazuhiro,
> >
> > On Fri, 5 Sep 2025, Kazuhiro Abe wrote:
> > > 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.
> > >
> > > Signed-off-by: Kazuhiro Abe <fj1078ii@...jp.fujitsu.com>
> >
> >
> > Looks good to me.
> >
> > Reviewed-by: Ilkka Koskinen <ilkka@...amperecomputing.com>
> >
> 
> Thanks for your review.
> 
> Best Regards,
> Kazuhiro Abe
> 
> >
> > Hanjun & Sudeep, what's your thought on enabling the use of regular
> > interrupts here? I do agree the spec talks about non-maskable ones and
> > to my understanding that's what the idea was indeed.

Do you have any thoughts on this matter?

Best Regards,
Kazuhiro Abe

> >
> > Cheers, Ilkka
> >
> >
> > > ---
> > > I keep normal IRQ code when NMI cannot be used.
> > > If there is any concern, please let me know.
> > >
> > > v2->v3
> > > - Fix bug in the return value of agdi_probe function.
> > > - Remove unnecessary curly braces in the agdi_remove function.
> > >
> > > v2:
> > > https://lore.kernel.org/all/20250829101154.2377800-1-fj1078ii@aa.jp.
> > > fu
> > > jitsu.com/
> > > v1->v2
> > > - Remove acpica update since there is no need to update define value
> > >   for this patch.
> > > ---
> > > drivers/acpi/arm64/agdi.c | 95
> > ++++++++++++++++++++++++++++++++++++---
> > > 1 file changed, 88 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/acpi/arm64/agdi.c b/drivers/acpi/arm64/agdi.c
> > > index e0df3daa4abf..2313a46f01cd 100644
> > > --- a/drivers/acpi/arm64/agdi.c
> > > +++ b/drivers/acpi/arm64/agdi.c
> > > @@ -16,7 +16,11 @@
> > > #include "init.h"
> > >
> > > struct agdi_data {
> > > +	unsigned char flags;
> > > 	int sdei_event;
> > > +	unsigned int gsiv;
> > > +	bool use_nmi;
> > > +	int irq;
> > > };
> > >
> > > static int agdi_sdei_handler(u32 sdei_event, struct pt_regs *regs,
> > > void *arg) @@ -48,6 +52,55 @@ static int agdi_sdei_probe(struct
> > platform_device *pdev,
> > > 	return 0;
> > > }
> > >
> > > +static irqreturn_t agdi_interrupt_handler_nmi(int irq, void
> > > +*dev_id) {
> > > +	nmi_panic(NULL, "Arm Generic Diagnostic Dump and Reset NMI
> > Interrupt event issued\n");
> > > +	return IRQ_HANDLED;
> > > +}
> > > +
> > > +static irqreturn_t agdi_interrupt_handler_irq(int irq, void
> > > +*dev_id) {
> > > +	panic("Arm Generic Diagnostic Dump and Reset Interrupt event
> > issued\n");
> > > +	return IRQ_HANDLED;
> > > +}
> > > +
> > > +static int agdi_interrupt_probe(struct platform_device *pdev,
> > > +				struct agdi_data *adata)
> > > +{
> > > +	unsigned long irq_flags;
> > > +	int ret;
> > > +	int irq;
> > > +
> > > +	irq = acpi_register_gsi(NULL, adata->gsiv, ACPI_EDGE_SENSITIVE,
> > ACPI_ACTIVE_HIGH);
> > > +	if (irq < 0) {
> > > +		dev_err(&pdev->dev, "cannot register GSI#%d (%d)\n",
> > adata->gsiv, irq);
> > > +		return irq;
> > > +	}
> > > +
> > > +	irq_flags = IRQF_PERCPU | IRQF_NOBALANCING |
> IRQF_NO_AUTOEN |
> > > +		    IRQF_NO_THREAD;
> > > +	/* try NMI first */
> > > +	ret = request_nmi(irq, &agdi_interrupt_handler_nmi, irq_flags,
> > > +			  "agdi_interrupt_nmi", NULL);
> > > +	if (ret) {
> > > +		ret = request_irq(irq, &agdi_interrupt_handler_irq,
> > > +				  irq_flags, "agdi_interrupt_irq", NULL);
> > > +		if (ret) {
> > > +			dev_err(&pdev->dev, "cannot register IRQ %d\n",
> ret);
> > > +			acpi_unregister_gsi(adata->gsiv);
> > > +			return ret;
> > > +		}
> > > +		enable_irq(irq);
> > > +		adata->irq = irq;
> > > +	} else {
> > > +		enable_nmi(irq);
> > > +		adata->irq = irq;
> > > +		adata->use_nmi = true;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > static int agdi_probe(struct platform_device *pdev) {
> > > 	struct agdi_data *adata = dev_get_platdata(&pdev->dev); @@ -55,12
> > > +108,15 @@ static int agdi_probe(struct platform_device *pdev)
> > > 	if (!adata)
> > > 		return -EINVAL;
> > >
> > > -	return agdi_sdei_probe(pdev, adata);
> > > +	if (adata->flags & ACPI_AGDI_SIGNALING_MODE)
> > > +		return agdi_interrupt_probe(pdev, adata);
> > > +	else
> > > +		return agdi_sdei_probe(pdev, adata);
> > > }
> > >
> > > -static void agdi_remove(struct platform_device *pdev)
> > > +static void agdi_sdei_remove(struct platform_device *pdev,
> > > +			     struct agdi_data *adata)
> > > {
> > > -	struct agdi_data *adata = dev_get_platdata(&pdev->dev);
> > > 	int err, i;
> > >
> > > 	err = sdei_event_disable(adata->sdei_event);
> > > @@ -83,6 +139,29 @@ static void agdi_remove(struct platform_device
> *pdev)
> > > 			adata->sdei_event, ERR_PTR(err)); }
> > >
> > > +static void agdi_interrupt_remove(struct platform_device *pdev,
> > > +				  struct agdi_data *adata)
> > > +{
> > > +	if (adata->irq != -1) {
> > > +		if (adata->use_nmi)
> > > +			free_nmi(adata->irq, NULL);
> > > +		else
> > > +			free_irq(adata->irq, NULL);
> > > +
> > > +		acpi_unregister_gsi(adata->gsiv);
> > > +	}
> > > +}
> > > +
> > > +static void agdi_remove(struct platform_device *pdev) {
> > > +	struct agdi_data *adata = dev_get_platdata(&pdev->dev);
> > > +
> > > +	if (adata->flags & ACPI_AGDI_SIGNALING_MODE)
> > > +		agdi_interrupt_remove(pdev, adata);
> > > +	else
> > > +		agdi_sdei_remove(pdev, adata);
> > > +}
> > > +
> > > static struct platform_driver agdi_driver = {
> > > 	.driver = {
> > > 		.name = "agdi",
> > > @@ -94,7 +173,7 @@ static struct platform_driver agdi_driver = {
> > > void __init acpi_agdi_init(void) {
> > > 	struct acpi_table_agdi *agdi_table;
> > > -	struct agdi_data pdata;
> > > +	struct agdi_data pdata = {0};
> > > 	struct platform_device *pdev;
> > > 	acpi_status status;
> > >
> > > @@ -104,11 +183,13 @@ void __init acpi_agdi_init(void)
> > > 		return;
> > >
> > > 	if (agdi_table->flags & ACPI_AGDI_SIGNALING_MODE) {
> > > -		pr_warn("Interrupt signaling is not supported");
> > > -		goto err_put_table;
> > > +		pdata.gsiv = agdi_table->gsiv;
> > > +	} else {
> > > +		pdata.sdei_event = agdi_table->sdei_event;
> > > 	}
> > >
> > > -	pdata.sdei_event = agdi_table->sdei_event;
> > > +	pdata.irq = -1;
> > > +	pdata.flags = agdi_table->flags;
> > >
> > > 	pdev = platform_device_register_data(NULL, "agdi", 0, &pdata,
> > sizeof(pdata));
> > > 	if (IS_ERR(pdev))
> > > --
> > > 2.43.0
> > >
> > >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ