[PATCH v3] ACPI: AGDI: Add interrupt signaling mode support

[PATCH v3] ACPI: AGDI: Add interrupt signaling mode support

[Kazuhiro Abe]

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@aa.jp.fujitsu.com>
---
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.fujitsu.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

Re: [PATCH v3] ACPI: AGDI: Add interrupt signaling mode support

[Ilkka Koskinen]

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@aa.jp.fujitsu.com>


Looks good to me.

Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>


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.

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.fujitsu.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
>
>

RE: [PATCH v3] ACPI: AGDI: Add interrupt signaling mode support

[Kazuhiro Abe (Fujitsu)]

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@aa.jp.fujitsu.com>
> 
> 
> Looks good to me.
> 
> Reviewed-by: Ilkka Koskinen <ilkka@os.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.
> 
> 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
> >
> >

RE: [PATCH v3] ACPI: AGDI: Add interrupt signaling mode support

[Kazuhiro Abe (Fujitsu)]

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@aa.jp.fujitsu.com>
> >
> >
> > Looks good to me.
> >
> > Reviewed-by: Ilkka Koskinen <ilkka@os.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
> > >
> > >

Re: [PATCH v3] ACPI: AGDI: Add interrupt signaling mode support

[Hanjun Guo]

Hi Kazuhiro,

Sorry for the late reply, some comments below.

On 2025/9/5 12:27, 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@aa.jp.fujitsu.com>
> ---
> 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.fujitsu.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;

Adding a comment here for what's the falgs used for,
multi flags in this file such as irq_flags, just
make the code easy to understand.

>   	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;
> +	}

if (!ret) {
	/* NMI handling code */
}

/* Then try normal interrupt */
ret = request_irq();
...

This makes code better organized.

> +
> +	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);
> +	}

if (adata->irq == -1)
	return;

...

To save extra { }.

> +}
> +
> +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 agdi_data pdata = { 0 };

Thanks
Hanjun

Re: [PATCH v3] ACPI: AGDI: Add interrupt signaling mode support

[Hanjun Guo]

On 2025/9/19 13:05, Kazuhiro Abe (Fujitsu) wrote:
> 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@aa.jp.fujitsu.com>
>>>
>>>
>>> Looks good to me.
>>>
>>> Reviewed-by: Ilkka Koskinen <ilkka@os.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?

For the real use case, if the system is in failure state, for example,
the system is panic, the CPU will not handle regular interrupts, so
what's the use case do you have to use regular interrupt?

Thanks
Hanjun

RE: [PATCH v3] ACPI: AGDI: Add interrupt signaling mode support

[Kazuhiro Abe (Fujitsu)]

Hi Hanjun,

Thank you for your review.

> Hi Kazuhiro,
> 
> Sorry for the late reply, some comments below.
> 
> On 2025/9/5 12:27, 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@aa.jp.fujitsu.com>
> > ---
> > 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;
> 
> Adding a comment here for what's the falgs used for, multi flags in this file such as
> irq_flags, just make the code easy to understand.

Understood.
I will add the following comment:
unsigned char flags; /* AGDI Signaling Mode (0=SDEI-based, 1=Interrupt-based) */

> 
> >   	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;
> > +	}
> 
> if (!ret) {
> 	/* NMI handling code */
> }
> 
> /* Then try normal interrupt */
> ret = request_irq();
> ...
> 
> This makes code better organized.

Understood. I will fix it for better organization as you suggested.


> 
> > +
> > +	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);
> > +	}
> 
> if (adata->irq == -1)
> 	return;
> 
> ...
> 
> To save extra { }.

Understood. I will fix it as suggested.


> 
> > +}
> > +
> > +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 agdi_data pdata = { 0 };

Understood, I'll do that.


Best Regards,
Kazuhiro Abe

> 
> Thanks
> Hanjun

RE: [PATCH v3] ACPI: AGDI: Add interrupt signaling mode support

[Kazuhiro Abe (Fujitsu)]

Hi Hanjun,

> > 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@aa.jp.fujitsu.com>
> >>>
> >>>
> >>> Looks good to me.
> >>>
> >>> Reviewed-by: Ilkka Koskinen <ilkka@os.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?
> 
> For the real use case, if the system is in failure state, for example, the system is
> panic, the CPU will not handle regular interrupts, so what's the use case do you
> have to use regular interrupt?

This driver is designed to operate with NMI enabled.

However, on current ARM64 platforms, NMI functionality may not be active unless
"pseudo NMI" is explicitly specified via a kernel command-line parameter.
In such scenarios, we've included regular interrupt handling as a last resort,
anticipating rare cases where some cores might still be able to receive regular
interrupts even if other cores are unresponsive.

To reiterate, the underlying assumption is that NMI(now pseudo NMI) is enabled for use.


Best Regards,
Kazuhiro Abe

> 
> Thanks
> Hanjun

Re: [PATCH v3] ACPI: AGDI: Add interrupt signaling mode support

[Hanjun Guo]

On 2025/9/29 15:32, Kazuhiro Abe (Fujitsu) wrote:
> Hi Hanjun,
> 
>>> 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@aa.jp.fujitsu.com>
>>>>>
>>>>> Looks good to me.
>>>>>
>>>>> Reviewed-by: Ilkka Koskinen<ilkka@os.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?
>> For the real use case, if the system is in failure state, for example, the system is
>> panic, the CPU will not handle regular interrupts, so what's the use case do you
>> have to use regular interrupt?
> This driver is designed to operate with NMI enabled.
> 
> However, on current ARM64 platforms, NMI functionality may not be active unless
> "pseudo NMI" is explicitly specified via a kernel command-line parameter.
> In such scenarios, we've included regular interrupt handling as a last resort,
> anticipating rare cases where some cores might still be able to receive regular
> interrupts even if other cores are unresponsive.
> 
> To reiterate, the underlying assumption is that NMI(now pseudo NMI) is enabled for use.

OK. Please send a new version, I will ack it
if you address my comments.

Thanks
Hanjun

RE: [PATCH v3] ACPI: AGDI: Add interrupt signaling mode support

[Kazuhiro Abe (Fujitsu)]

Hi Hanjun,

> > Hi Hanjun,
> >
> >>> 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@aa.jp.fujitsu.com>
> >>>>>
> >>>>> Looks good to me.
> >>>>>
> >>>>> Reviewed-by: Ilkka Koskinen<ilkka@os.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?
> >> For the real use case, if the system is in failure state, for example, the system
> is
> >> panic, the CPU will not handle regular interrupts, so what's the use case do
> you
> >> have to use regular interrupt?
> > This driver is designed to operate with NMI enabled.
> >
> > However, on current ARM64 platforms, NMI functionality may not be active
> unless
> > "pseudo NMI" is explicitly specified via a kernel command-line parameter.
> > In such scenarios, we've included regular interrupt handling as a last resort,
> > anticipating rare cases where some cores might still be able to receive regular
> > interrupts even if other cores are unresponsive.
> >
> > To reiterate, the underlying assumption is that NMI(now pseudo NMI) is
> enabled for use.
> 
> OK. Please send a new version, I will ack it
> if you address my comments.

Thanks for your comment.
I will send a new version with your comments addressed after 6.18-rc1 is released.

Best Regards,
Kazuhiro Abe

> 
> Thanks
> Hanjun