[<prev] [next>] [<thread-prev] [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
Powered by blists - more mailing lists