[PATCH] firmware: SDEI: Allow sdei initialization without ACPI_APEI_GHES

[PATCH] firmware: SDEI: Allow sdei initialization without ACPI_APEI_GHES

[Huang Yiwei]

SDEI usually initialize with the ACPI table, but on platforms where
ACPI is not used, the SDEI feature can still be used to handle
specific firmware calls or other customized purposes. Therefore, it
is not necessary for ARM_SDE_INTERFACE to depend on ACPI_APEI_GHES.

In commit dc4e8c07e9e2 ("ACPI: APEI: explicit init of HEST and GHES
in acpi_init()"), to make APEI ready earlier, sdei_init was moved
into acpi_ghes_init instead of being a standalone initcall, adding
ACPI_APEI_GHES dependency to ARM_SDE_INTERFACE. This restricts the
flexibility and usability of SDEI.

This patch corrects the dependency in Kconfig and allows the
initialization of SDEI without ACPI_APEI_GHES enabled.

Fixes: dc4e8c07e9e2 ("ACPI: APEI: explicit init of HEST and GHES in apci_init()")
Cc: Shuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: Huang Yiwei <quic_hyiwei@quicinc.com>
---
Link: https://lore.kernel.org/all/20230906130900.12218-1-schspa@gmail.com/

Current patch has been verified in the following scenarios:
  - ACPI_APEI_GHES enabled and ARM_SDE_INTERFACE enabled
  - ACPI_APEI_GHES disabled and ARM_SDE_INTERFACE enabled
  - ACPI_APEI_GHES disabled and ARM_SDE_INTERFACE disabled
  - SDEI works well with DT node and compatiable firmware when
    ACPI_APEI_GHES disabled

The scenario where CONFIG_ACPI enabled but not used has not been
considered in this patch due to the absence of such platform.

 drivers/acpi/apei/Kconfig   | 1 +
 drivers/firmware/Kconfig    | 1 -
 drivers/firmware/arm_sdei.c | 9 +++++++--
 include/linux/arm_sdei.h    | 4 ++--
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig
index 3cfe7e7475f2..070c07d68dfb 100644
--- a/drivers/acpi/apei/Kconfig
+++ b/drivers/acpi/apei/Kconfig
@@ -23,6 +23,7 @@ config ACPI_APEI_GHES
 	select ACPI_HED
 	select IRQ_WORK
 	select GENERIC_ALLOCATOR
+	select ARM_SDE_INTERFACE if ARM64
 	help
 	  Generic Hardware Error Source provides a way to report
 	  platform hardware errors (such as that from chipset). It
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index aadc395ee168..7df19d82aa68 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -31,7 +31,6 @@ config ARM_SCPI_PROTOCOL
 config ARM_SDE_INTERFACE
 	bool "ARM Software Delegated Exception Interface (SDEI)"
 	depends on ARM64
-	depends on ACPI_APEI_GHES
 	help
 	  The Software Delegated Exception Interface (SDEI) is an ARM
 	  standard for registering callbacks from the platform firmware
diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
index 3e8051fe8296..ddb10389b340 100644
--- a/drivers/firmware/arm_sdei.c
+++ b/drivers/firmware/arm_sdei.c
@@ -1062,14 +1062,14 @@ static bool __init sdei_present_acpi(void)
 	return true;
 }
 
-void __init sdei_init(void)
+int __init sdei_init(void)
 {
 	struct platform_device *pdev;
 	int ret;
 
 	ret = platform_driver_register(&sdei_driver);
 	if (ret || !sdei_present_acpi())
-		return;
+		return ret;
 
 	pdev = platform_device_register_simple(sdei_driver.driver.name,
 					       0, NULL, 0);
@@ -1079,7 +1079,12 @@ void __init sdei_init(void)
 		pr_info("Failed to register ACPI:SDEI platform device %d\n",
 			ret);
 	}
+
+	return ret;
 }
+#ifndef CONFIG_ACPI_APEI_GHES
+subsys_initcall_sync(sdei_init);
+#endif
 
 int sdei_event_handler(struct pt_regs *regs,
 		       struct sdei_registered_event *arg)
diff --git a/include/linux/arm_sdei.h b/include/linux/arm_sdei.h
index 255701e1251b..1a54f0eebb0d 100644
--- a/include/linux/arm_sdei.h
+++ b/include/linux/arm_sdei.h
@@ -46,12 +46,12 @@ int sdei_unregister_ghes(struct ghes *ghes);
 /* For use by arch code when CPU hotplug notifiers are not appropriate. */
 int sdei_mask_local_cpu(void);
 int sdei_unmask_local_cpu(void);
-void __init sdei_init(void);
+int __init sdei_init(void);
 void sdei_handler_abort(void);
 #else
 static inline int sdei_mask_local_cpu(void) { return 0; }
 static inline int sdei_unmask_local_cpu(void) { return 0; }
-static inline void sdei_init(void) { }
+static inline int sdei_init(void) { return 0; }
 static inline void sdei_handler_abort(void) { }
 #endif /* CONFIG_ARM_SDE_INTERFACE */
 
-- 
2.25.1

Re: [PATCH] firmware: SDEI: Allow sdei initialization without ACPI_APEI_GHES

[Konrad Dybcio]

On 4/28/25 11:56 AM, Huang Yiwei wrote:
> SDEI usually initialize with the ACPI table, but on platforms where
> ACPI is not used, the SDEI feature can still be used to handle
> specific firmware calls or other customized purposes. Therefore, it
> is not necessary for ARM_SDE_INTERFACE to depend on ACPI_APEI_GHES.
> 
> In commit dc4e8c07e9e2 ("ACPI: APEI: explicit init of HEST and GHES
> in acpi_init()"), to make APEI ready earlier, sdei_init was moved
> into acpi_ghes_init instead of being a standalone initcall, adding
> ACPI_APEI_GHES dependency to ARM_SDE_INTERFACE. This restricts the
> flexibility and usability of SDEI.
> 
> This patch corrects the dependency in Kconfig and allows the
> initialization of SDEI without ACPI_APEI_GHES enabled.
> 
> Fixes: dc4e8c07e9e2 ("ACPI: APEI: explicit init of HEST and GHES in apci_init()")
> Cc: Shuai Xue <xueshuai@linux.alibaba.com>
> Signed-off-by: Huang Yiwei <quic_hyiwei@quicinc.com>
> ---

[...]

> +#ifndef CONFIG_ACPI_APEI_GHES
> +subsys_initcall_sync(sdei_init);

I think it'd be good to leave a comment such as:

/* Initialized by acpi_ghes_init() when ACPI is present */

Konrad

Re: [PATCH] firmware: SDEI: Allow sdei initialization without ACPI_APEI_GHES

[Will Deacon]

On Mon, Apr 28, 2025 at 05:56:23PM +0800, Huang Yiwei wrote:
> SDEI usually initialize with the ACPI table, but on platforms where
> ACPI is not used, the SDEI feature can still be used to handle
> specific firmware calls or other customized purposes. Therefore, it
> is not necessary for ARM_SDE_INTERFACE to depend on ACPI_APEI_GHES.
> 
> In commit dc4e8c07e9e2 ("ACPI: APEI: explicit init of HEST and GHES
> in acpi_init()"), to make APEI ready earlier, sdei_init was moved
> into acpi_ghes_init instead of being a standalone initcall, adding
> ACPI_APEI_GHES dependency to ARM_SDE_INTERFACE. This restricts the
> flexibility and usability of SDEI.
> 
> This patch corrects the dependency in Kconfig and allows the
> initialization of SDEI without ACPI_APEI_GHES enabled.
> 
> Fixes: dc4e8c07e9e2 ("ACPI: APEI: explicit init of HEST and GHES in apci_init()")
> Cc: Shuai Xue <xueshuai@linux.alibaba.com>
> Signed-off-by: Huang Yiwei <quic_hyiwei@quicinc.com>
> ---
> Link: https://lore.kernel.org/all/20230906130900.12218-1-schspa@gmail.com/
> 
> Current patch has been verified in the following scenarios:
>   - ACPI_APEI_GHES enabled and ARM_SDE_INTERFACE enabled
>   - ACPI_APEI_GHES disabled and ARM_SDE_INTERFACE enabled
>   - ACPI_APEI_GHES disabled and ARM_SDE_INTERFACE disabled
>   - SDEI works well with DT node and compatiable firmware when
>     ACPI_APEI_GHES disabled
> 
> The scenario where CONFIG_ACPI enabled but not used has not been
> considered in this patch due to the absence of such platform.
> 
>  drivers/acpi/apei/Kconfig   | 1 +
>  drivers/firmware/Kconfig    | 1 -
>  drivers/firmware/arm_sdei.c | 9 +++++++--
>  include/linux/arm_sdei.h    | 4 ++--
>  4 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig
> index 3cfe7e7475f2..070c07d68dfb 100644
> --- a/drivers/acpi/apei/Kconfig
> +++ b/drivers/acpi/apei/Kconfig
> @@ -23,6 +23,7 @@ config ACPI_APEI_GHES
>  	select ACPI_HED
>  	select IRQ_WORK
>  	select GENERIC_ALLOCATOR
> +	select ARM_SDE_INTERFACE if ARM64
>  	help
>  	  Generic Hardware Error Source provides a way to report
>  	  platform hardware errors (such as that from chipset). It
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index aadc395ee168..7df19d82aa68 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -31,7 +31,6 @@ config ARM_SCPI_PROTOCOL
>  config ARM_SDE_INTERFACE
>  	bool "ARM Software Delegated Exception Interface (SDEI)"
>  	depends on ARM64
> -	depends on ACPI_APEI_GHES
>  	help
>  	  The Software Delegated Exception Interface (SDEI) is an ARM
>  	  standard for registering callbacks from the platform firmware
> diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
> index 3e8051fe8296..ddb10389b340 100644
> --- a/drivers/firmware/arm_sdei.c
> +++ b/drivers/firmware/arm_sdei.c
> @@ -1062,14 +1062,14 @@ static bool __init sdei_present_acpi(void)
>  	return true;
>  }
>  
> -void __init sdei_init(void)
> +int __init sdei_init(void)
>  {
>  	struct platform_device *pdev;
>  	int ret;
>  
>  	ret = platform_driver_register(&sdei_driver);
>  	if (ret || !sdei_present_acpi())
> -		return;
> +		return ret;
>  
>  	pdev = platform_device_register_simple(sdei_driver.driver.name,
>  					       0, NULL, 0);
> @@ -1079,7 +1079,12 @@ void __init sdei_init(void)
>  		pr_info("Failed to register ACPI:SDEI platform device %d\n",
>  			ret);
>  	}
> +
> +	return ret;
>  }
> +#ifndef CONFIG_ACPI_APEI_GHES
> +subsys_initcall_sync(sdei_init);
> +#endif

Using an initcall purely for the non-ACPI case feels like a hack to me.
Could we instead just call sdei_init() from the arch code (and remove
the call from acpi_ghes_init()) so that the platform device is
registered at the same time, regardless of the firmware?

Will

Re: [PATCH] firmware: SDEI: Allow sdei initialization without ACPI_APEI_GHES

[Huang Yiwei]

> On Mon, Apr 28, 2025 at 05:56:23PM +0800, Huang Yiwei wrote:
>> SDEI usually initialize with the ACPI table, but on platforms where
>> ACPI is not used, the SDEI feature can still be used to handle
>> specific firmware calls or other customized purposes. Therefore, it
>> ......
>>   	}
>> +
>> +	return ret;
>>   }
>> +#ifndef CONFIG_ACPI_APEI_GHES
>> +subsys_initcall_sync(sdei_init);
>> +#endif
> 
> Using an initcall purely for the non-ACPI case feels like a hack to me.

Yeah, I agree with you actually, but to address the dependency chain 
issue highlighted in commit dc4e8c07e9e2 ("ACPI: APEI: explicit init of 
HEST and GHES in acpi_init()"), where the following relationships need 
to be maintained:

     ghes_init() => acpi_hest_init() => acpi_bus_init() => acpi_init()
     ghes_init() => sdei_init()

> Could we instead just call sdei_init() from the arch code (and remove
> the call from acpi_ghes_init()) so that the platform device is
> registered at the same time, regardless of the firmware?
> 
> Will

I propose splitting sdei_init() into two separate functions: sdei_init() 
and acpi_sdei_init(). This way, sdei_init() will be called by 
arch_initcall and will only initialize the platform driver, while 
acpi_sdei_init() will initialize the device from acpi_ghes_init() when 
ACPI is ready. This approach should help maintain the dependency chain 
without causing any breaks.

     sdei_init --> platform_driver_register
     arch_initcall(sdei_init)

     acpi_init
         acpi_bus_init();
         acpi_hest_init();
         acpi_ghes_init();
             acpi_sdei_init(); --> platform_device_register_simple
     subsys_initcall(acpi_init);