linux-kernel - Re: [PATCH] firmware: SDEI: Allow sdei initialization without ACPI_APEI

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250430102440.GA27570@willie-the-truck>
Date: Wed, 30 Apr 2025 11:24:41 +0100
From: Will Deacon <will@...nel.org>
To: Huang Yiwei <quic_hyiwei@...cinc.com>
Cc: rafael@...nel.org, lenb@...nel.org, james.morse@....com,
	tony.luck@...el.com, bp@...en8.de, xueshuai@...ux.alibaba.com,
	quic_aiquny@...cinc.com, quic_satyap@...cinc.com,
	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, kernel@...cinc.com,
	kernel@....qualcomm.com
Subject: Re: [PATCH] firmware: SDEI: Allow sdei initialization without
 ACPI_APEI_GHES

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@...ux.alibaba.com>
> Signed-off-by: Huang Yiwei <quic_hyiwei@...cinc.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