* Re: [PATCH v4 2/2] ACPI: APEI: EINJ: Fix EINJV2 memory error injection
2026-04-21 15:02 ` [PATCH v4 2/2] ACPI: APEI: EINJ: Fix EINJV2 memory error injection Tony Luck
@ 2026-04-21 17:42 ` Jiaqi Yan
2026-04-21 18:32 ` Zaid Alali
1 sibling, 0 replies; 5+ messages in thread
From: Jiaqi Yan @ 2026-04-21 17:42 UTC (permalink / raw)
To: Tony Luck
Cc: Rafael J. Wysocki, Herman Li, Lai, Yi1, Zaid Alali, linux-acpi,
linux-kernel, patches
On Tue, Apr 21, 2026 at 8:02 AM Tony Luck <tony.luck@intel.com> wrote:
>
> Error types in EINJV2 use different bit positions for each flavor of
> injection from legacy EINJ.
>
> Two issues:
>
> 1) The address sanity checks in einj_error_inject() were skipped for EINJV2
> injections. Noted by sashiko[1]
> 2) __einj_error_trigger() failed to drop the entry of the target
> physical address from the list of resources that need to be
> requested.
>
> Add a helper function that checks if an injection is to memory and use it
> to solve each of these issues.
>
> Note that the old test in __einj_error_trigger() checked that param2 was
> not zero. This isn't needed because the sanity checks in einj_error_inject()
> reject memory injections with param2 == 0.
>
> Fixes: b47610296d17 ("ACPI: APEI: EINJ: Enable EINJv2 error injections")
> #Reported-by: sashiko <sashiko@sashiko.dev>
> Reported-by: Herman Li <herman.li@intel.com>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> Tested-by: "Lai, Yi1" <yi1.lai@intel.com>
> Link: https://sashiko.dev/#/patchset/20260415163620.12957-1-tony.luck%40intel.com # [1]
> ---
> drivers/acpi/apei/einj-core.c | 45 +++++++++++++++++++----------------
> 1 file changed, 25 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index a9248af078f6..1f3fa2278584 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -401,8 +401,18 @@ static struct acpi_generic_address *einj_get_trigger_parameter_region(
>
> return NULL;
> }
> +
> +static bool is_memory_injection(u32 type, u32 flags)
> +{
> + if (flags & SETWA_FLAGS_EINJV2)
> + return !!(type & ACPI_EINJV2_MEMORY);
> + if (type & ACPI5_VENDOR_BIT)
> + return !!(vendor_flags & SETWA_FLAGS_MEM);
> + return !!(type & MEM_ERROR_MASK) || !!(flags & SETWA_FLAGS_MEM);
> +}
> +
> /* Execute instructions in trigger error action table */
> -static int __einj_error_trigger(u64 trigger_paddr, u32 type,
> +static int __einj_error_trigger(u64 trigger_paddr, u32 type, u32 flags,
> u64 param1, u64 param2)
> {
> struct acpi_einj_trigger trigger_tab;
> @@ -480,7 +490,7 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
> * This will cause resource conflict with regular memory. So
> * remove it from trigger table resources.
> */
> - if ((param_extension || acpi5) && (type & MEM_ERROR_MASK) && param2) {
> + if ((param_extension || acpi5) && is_memory_injection(type, flags)) {
> struct apei_resources addr_resources;
>
> apei_resources_init(&addr_resources);
> @@ -660,7 +670,7 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
> return rc;
> trigger_paddr = apei_exec_ctx_get_output(&ctx);
> if (notrigger == 0) {
> - rc = __einj_error_trigger(trigger_paddr, type, param1, param2);
> + rc = __einj_error_trigger(trigger_paddr, type, flags, param1, param2);
> if (rc)
> return rc;
> }
> @@ -718,35 +728,30 @@ int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
> SETWA_FLAGS_PCIE_SBDF | SETWA_FLAGS_EINJV2)))
> return -EINVAL;
>
> + /*
> + * Injections targeting a CXL 1.0/1.1 port have to be injected
> + * via the einj_cxl_rch_error_inject() path as that does the proper
> + * validation of the given RCRB base (MMIO) address.
> + */
> + if (einj_is_cxl_error_type(type) && (flags & SETWA_FLAGS_MEM))
> + return -EINVAL;
> +
> /* check if type is a valid EINJv2 error type */
> if (is_v2) {
> if (!(type & available_error_type_v2))
> return -EINVAL;
> }
> - /*
> - * We need extra sanity checks for memory errors.
> - * Other types leap directly to injection.
> - */
>
> /* ensure param1/param2 existed */
> if (!(param_extension || acpi5))
> goto inject;
>
> - /* ensure injection is memory related */
> - if (type & ACPI5_VENDOR_BIT) {
> - if (vendor_flags != SETWA_FLAGS_MEM)
> - goto inject;
> - } else if (!(type & MEM_ERROR_MASK) && !(flags & SETWA_FLAGS_MEM)) {
> - goto inject;
> - }
> -
> /*
> - * Injections targeting a CXL 1.0/1.1 port have to be injected
> - * via the einj_cxl_rch_error_inject() path as that does the proper
> - * validation of the given RCRB base (MMIO) address.
> + * We need extra sanity checks for memory errors.
> + * Other types leap directly to injection.
> */
> - if (einj_is_cxl_error_type(type) && (flags & SETWA_FLAGS_MEM))
> - return -EINVAL;
> + if (!is_memory_injection(type, flags))
> + goto inject;
>
> /*
> * Disallow crazy address masks that give BIOS leeway to pick
> --
> 2.53.0
>
Reviewed-by: Jiaqi Yan <jiaqiyan@google.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v4 2/2] ACPI: APEI: EINJ: Fix EINJV2 memory error injection
2026-04-21 15:02 ` [PATCH v4 2/2] ACPI: APEI: EINJ: Fix EINJV2 memory error injection Tony Luck
2026-04-21 17:42 ` Jiaqi Yan
@ 2026-04-21 18:32 ` Zaid Alali
1 sibling, 0 replies; 5+ messages in thread
From: Zaid Alali @ 2026-04-21 18:32 UTC (permalink / raw)
To: Tony Luck
Cc: Rafael J. Wysocki, Herman Li, Lai, Yi1, Jiaqi Yan, linux-acpi,
linux-kernel, patches
On Tue, Apr 21, 2026 at 08:02:16AM -0700, Tony Luck wrote:
> Error types in EINJV2 use different bit positions for each flavor of
> injection from legacy EINJ.
>
> Two issues:
>
> 1) The address sanity checks in einj_error_inject() were skipped for EINJV2
> injections. Noted by sashiko[1]
> 2) __einj_error_trigger() failed to drop the entry of the target
> physical address from the list of resources that need to be
> requested.
>
> Add a helper function that checks if an injection is to memory and use it
> to solve each of these issues.
>
> Note that the old test in __einj_error_trigger() checked that param2 was
> not zero. This isn't needed because the sanity checks in einj_error_inject()
> reject memory injections with param2 == 0.
>
> Fixes: b47610296d17 ("ACPI: APEI: EINJ: Enable EINJv2 error injections")
> #Reported-by: sashiko <sashiko@sashiko.dev>
> Reported-by: Herman Li <herman.li@intel.com>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> Tested-by: "Lai, Yi1" <yi1.lai@intel.com>
> Link: https://sashiko.dev/#/patchset/20260415163620.12957-1-tony.luck%40intel.com # [1]
> ---
> drivers/acpi/apei/einj-core.c | 45 +++++++++++++++++++----------------
> 1 file changed, 25 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index a9248af078f6..1f3fa2278584 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -401,8 +401,18 @@ static struct acpi_generic_address *einj_get_trigger_parameter_region(
>
> return NULL;
> }
> +
> +static bool is_memory_injection(u32 type, u32 flags)
> +{
> + if (flags & SETWA_FLAGS_EINJV2)
> + return !!(type & ACPI_EINJV2_MEMORY);
> + if (type & ACPI5_VENDOR_BIT)
> + return !!(vendor_flags & SETWA_FLAGS_MEM);
> + return !!(type & MEM_ERROR_MASK) || !!(flags & SETWA_FLAGS_MEM);
> +}
> +
> /* Execute instructions in trigger error action table */
> -static int __einj_error_trigger(u64 trigger_paddr, u32 type,
> +static int __einj_error_trigger(u64 trigger_paddr, u32 type, u32 flags,
> u64 param1, u64 param2)
> {
> struct acpi_einj_trigger trigger_tab;
> @@ -480,7 +490,7 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
> * This will cause resource conflict with regular memory. So
> * remove it from trigger table resources.
> */
> - if ((param_extension || acpi5) && (type & MEM_ERROR_MASK) && param2) {
> + if ((param_extension || acpi5) && is_memory_injection(type, flags)) {
> struct apei_resources addr_resources;
>
> apei_resources_init(&addr_resources);
> @@ -660,7 +670,7 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
> return rc;
> trigger_paddr = apei_exec_ctx_get_output(&ctx);
> if (notrigger == 0) {
> - rc = __einj_error_trigger(trigger_paddr, type, param1, param2);
> + rc = __einj_error_trigger(trigger_paddr, type, flags, param1, param2);
> if (rc)
> return rc;
> }
> @@ -718,35 +728,30 @@ int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
> SETWA_FLAGS_PCIE_SBDF | SETWA_FLAGS_EINJV2)))
> return -EINVAL;
>
> + /*
> + * Injections targeting a CXL 1.0/1.1 port have to be injected
> + * via the einj_cxl_rch_error_inject() path as that does the proper
> + * validation of the given RCRB base (MMIO) address.
> + */
> + if (einj_is_cxl_error_type(type) && (flags & SETWA_FLAGS_MEM))
> + return -EINVAL;
> +
> /* check if type is a valid EINJv2 error type */
> if (is_v2) {
> if (!(type & available_error_type_v2))
> return -EINVAL;
> }
> - /*
> - * We need extra sanity checks for memory errors.
> - * Other types leap directly to injection.
> - */
>
> /* ensure param1/param2 existed */
> if (!(param_extension || acpi5))
> goto inject;
>
> - /* ensure injection is memory related */
> - if (type & ACPI5_VENDOR_BIT) {
> - if (vendor_flags != SETWA_FLAGS_MEM)
> - goto inject;
> - } else if (!(type & MEM_ERROR_MASK) && !(flags & SETWA_FLAGS_MEM)) {
> - goto inject;
> - }
> -
> /*
> - * Injections targeting a CXL 1.0/1.1 port have to be injected
> - * via the einj_cxl_rch_error_inject() path as that does the proper
> - * validation of the given RCRB base (MMIO) address.
> + * We need extra sanity checks for memory errors.
> + * Other types leap directly to injection.
> */
> - if (einj_is_cxl_error_type(type) && (flags & SETWA_FLAGS_MEM))
> - return -EINVAL;
> + if (!is_memory_injection(type, flags))
> + goto inject;
>
> /*
> * Disallow crazy address masks that give BIOS leeway to pick
> --
> 2.53.0
>
Reviewed-by:Zaid Alali <zaidal@os.amperecomputing.com>
^ permalink raw reply [flat|nested] 5+ messages in thread