[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJZ5v0hxsqMFgseYkRvfvGeQ22FQRTniAjP8vwNWYsEfrbkA-g@mail.gmail.com>
Date: Wed, 19 Nov 2025 13:34:53 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Tony Luck <tony.luck@...el.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, bp@...en8.de, guohanjun@...wei.com, lenb@...nel.org,
mchehab@...nel.org, xueshuai@...ux.alibaba.com, yi1.lai@...el.com,
zaidal@...amperecomputing.com, linux-acpi@...r.kernel.org,
linux-kernel@...r.kernel.org, patches@...ts.linux.dev
Subject: Re: [PATCH] ACPI: APEI: EINJ: Fix EINJV2 initialization and injection
On Wed, Nov 19, 2025 at 2:27 AM Tony Luck <tony.luck@...el.com> wrote:
>
> ACPI 6.6 specification for EINJV2 appends an extra structure to
> the end of the existing struct set_error_type_with_address.
>
> Several issues showed up in testing.
>
> 1) Initialization was broken by an earlier fix[1] since is_v2 is only set
> while performing an injection, not during initialization.
>
> 2) A buggy BIOS provided invalid "revision" and "length" for the extension
> structure. Add several sanity checks.
>
> 3) When injecting legacy error types on an EINJV2 capable system don't
> copy the component arrays.
>
> Fixes: 6c7058514991 ("ACPI: APEI: EINJ: Check if user asked for EINJV2 injection") # [1]
> Fixes: b47610296d17 ("ACPI: APEI: EINJ: Enable EINJv2 error injections")
> Signed-off-by: Tony Luck <tony.luck@...el.com>
> ---
> drivers/acpi/apei/einj-core.c | 64 ++++++++++++++++++++++-------------
> 1 file changed, 41 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index 3c87953dbd19..305c240a303f 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -182,6 +182,7 @@ bool einj_initialized __ro_after_init;
>
> static void __iomem *einj_param;
> static u32 v5param_size;
> +static u32 v66param_size;
> static bool is_v2;
>
> static void einj_exec_ctx_init(struct apei_exec_context *ctx)
> @@ -283,6 +284,24 @@ static void check_vendor_extension(u64 paddr,
> acpi_os_unmap_iomem(p, sizeof(v));
> }
>
> +static u32 einjv2_init(struct einjv2_extension_struct *e)
> +{
> + if (e->revision != 1) {
> + pr_info("Unknown v2 extension revision %u\n", e->revision);
> + return 0;
> + }
> + if (e->length < sizeof(*e) || e->length > PAGE_SIZE) {
> + pr_info(FW_BUG "Bad1 v2 extension length %u\n", e->length);
> + return 0;
> + }
> + if ((e->length - sizeof(*e)) % sizeof(e->component_arr[0])) {
> + pr_info(FW_BUG "Bad2 v2 extension length %u\n", e->length);
> + return 0;
> + }
> +
> + return (e->length - sizeof(*e)) / sizeof(e->component_arr[0]);
> +}
> +
> static void __iomem *einj_get_parameter_address(void)
> {
> int i;
> @@ -310,28 +329,21 @@ static void __iomem *einj_get_parameter_address(void)
> v5param_size = sizeof(v5param);
> p = acpi_os_map_iomem(pa_v5, sizeof(*p));
> if (p) {
> - int offset, len;
> -
> memcpy_fromio(&v5param, p, v5param_size);
> acpi5 = 1;
> check_vendor_extension(pa_v5, &v5param);
> - if (is_v2 && available_error_type & ACPI65_EINJV2_SUPP) {
> - len = v5param.einjv2_struct.length;
> - offset = offsetof(struct einjv2_extension_struct, component_arr);
> - max_nr_components = (len - offset) /
> - sizeof(v5param.einjv2_struct.component_arr[0]);
> - /*
> - * The first call to acpi_os_map_iomem above does not include the
> - * component array, instead it is used to read and calculate maximum
> - * number of components supported by the system. Below, the mapping
> - * is expanded to include the component array.
> - */
> + if (available_error_type & ACPI65_EINJV2_SUPP) {
> + struct einjv2_extension_struct *e;
> +
> + e = &v5param.einjv2_struct;
> + max_nr_components = einjv2_init(e);
> +
> + /* remap including einjv2_extension_struct */
> acpi_os_unmap_iomem(p, v5param_size);
> - offset = offsetof(struct set_error_type_with_address, einjv2_struct);
> - v5param_size = offset + struct_size(&v5param.einjv2_struct,
> - component_arr, max_nr_components);
> - p = acpi_os_map_iomem(pa_v5, v5param_size);
> + v66param_size = v5param_size - sizeof(*e) + e->length;
> + p = acpi_os_map_iomem(pa_v5, v66param_size);
> }
> +
> return p;
> }
> }
> @@ -527,6 +539,7 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
> u64 param3, u64 param4)
> {
> struct apei_exec_context ctx;
> + u32 param_size = is_v2 ? v66param_size : v5param_size;
> u64 val, trigger_paddr, timeout = FIRMWARE_TIMEOUT;
> int i, rc;
>
> @@ -539,11 +552,11 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
> if (acpi5) {
> struct set_error_type_with_address *v5param;
>
> - v5param = kmalloc(v5param_size, GFP_KERNEL);
> + v5param = kmalloc(param_size, GFP_KERNEL);
> if (!v5param)
> return -ENOMEM;
>
> - memcpy_fromio(v5param, einj_param, v5param_size);
> + memcpy_fromio(v5param, einj_param, param_size);
> v5param->type = type;
> if (type & ACPI5_VENDOR_BIT) {
> switch (vendor_flags) {
> @@ -601,7 +614,7 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
> break;
> }
> }
> - memcpy_toio(einj_param, v5param, v5param_size);
> + memcpy_toio(einj_param, v5param, param_size);
> kfree(v5param);
> } else {
> rc = apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE);
> @@ -1132,9 +1145,14 @@ static void einj_remove(struct faux_device *fdev)
> struct apei_exec_context ctx;
>
> if (einj_param) {
> - acpi_size size = (acpi5) ?
> - v5param_size :
> - sizeof(struct einj_parameter);
> + acpi_size size;
> +
> + if (v66param_size)
> + size = v66param_size;
> + else if (acpi5)
> + size = v5param_size;
> + else
> + size = sizeof(struct einj_parameter);
>
> acpi_os_unmap_iomem(einj_param, size);
> if (vendor_errors.size)
> --
Applied as 6.18-rc material, thanks!
Powered by blists - more mailing lists