EINJ trigger actions for memory error injection often include access to
the target injection address. This address does not need to special
mapping.

The code to drop the target address from the resource list only checked
for V1 injection signature.

Add a test for the EINJ V2 memory injection signature.

Fixes: b47610296d17 ("ACPI: APEI: EINJ: Enable EINJv2 error injections")
Reported-by: Herman Li <herman.li@intel.com>
Signed-off-by: Tony Luck <tony.luck@intel.com>
---

V2: Add "Fixes:" tag

 drivers/acpi/apei/einj-core.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
index a9248af078f6..ba0ce71b7adb 100644
--- a/drivers/acpi/apei/einj-core.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -401,6 +401,17 @@ static struct acpi_generic_address *einj_get_trigger_parameter_region(
 
 	return NULL;
 }
+
+static bool is_memory_injection(u32 type, u64 param2)
+{
+	if (is_v2)
+		return type & BIT(1);
+	else if (param_extension || acpi5)
+		return (type & MEM_ERROR_MASK) && param2;
+
+	return false;
+}
+
 /* Execute instructions in trigger error action table */
 static int __einj_error_trigger(u64 trigger_paddr, u32 type,
 				u64 param1, u64 param2)
@@ -480,7 +491,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 (is_memory_injection(type, param2)) {
 		struct apei_resources addr_resources;
 
 		apei_resources_init(&addr_resources);
-- 
2.53.0

On Wed, Apr 15, 2026 at 10:37 AM Tony Luck <tony.luck@intel.com> wrote:
>
> EINJ trigger actions for memory error injection often include access to
> the target injection address. This address does not need to special
> mapping.
>
> The code to drop the target address from the resource list only checked
> for V1 injection signature.
>
> Add a test for the EINJ V2 memory injection signature.
>
> Fixes: b47610296d17 ("ACPI: APEI: EINJ: Enable EINJv2 error injections")
> Reported-by: Herman Li <herman.li@intel.com>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>
> V2: Add "Fixes:" tag
>
>  drivers/acpi/apei/einj-core.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index a9248af078f6..ba0ce71b7adb 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -401,6 +401,17 @@ static struct acpi_generic_address *einj_get_trigger_parameter_region(
>
>         return NULL;
>  }
> +
> +static bool is_memory_injection(u32 type, u64 param2)
> +{
> +       if (is_v2)
> +               return type & BIT(1);

Hi Tony, just a nit: for readability would it better to introduce a
macro for BIT(1)? like ACPI65_EINJV2_ERROR_TYPE_MEMORY

> +       else if (param_extension || acpi5)
> +               return (type & MEM_ERROR_MASK) && param2;
> +
> +       return false;
> +}
> +
>  /* Execute instructions in trigger error action table */
>  static int __einj_error_trigger(u64 trigger_paddr, u32 type,
>                                 u64 param1, u64 param2)
> @@ -480,7 +491,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 (is_memory_injection(type, param2)) {
>                 struct apei_resources addr_resources;
>
>                 apei_resources_init(&addr_resources);
> --
> 2.53.0
>

Anyway, the patch looks good to me.

Reviewed-by: Jiaqi Yan <jiaqiyan@google.com>

> > +static bool is_memory_injection(u32 type, u64 param2)
> > +{
> > +       if (is_v2)
> > +               return type & BIT(1);
>
> Hi Tony, just a nit: for readability would it better to introduce a
> macro for BIT(1)? like ACPI65_EINJV2_ERROR_TYPE_MEMORY

I thought about that. The old (v1?) EINJ type bits are in <acpi/actbl1.h>

So "the right thing"(TM) might be to add some V2 defines there:

#define ACPI_EINJV2_PROCESSOR	BIT(0)
#define ACPI_EINJV2_MEMORY		BIT(1)
#define ACPI_EINJV2_PCIX			BIT(2)

Rafael: Do you want that? I can start the process in the ACPICA tree if you do.

-Tony

On Wed, Apr 15, 2026 at 8:15 PM Luck, Tony <tony.luck@intel.com> wrote:
>
> > > +static bool is_memory_injection(u32 type, u64 param2)
> > > +{
> > > +       if (is_v2)
> > > +               return type & BIT(1);
> >
> > Hi Tony, just a nit: for readability would it better to introduce a
> > macro for BIT(1)? like ACPI65_EINJV2_ERROR_TYPE_MEMORY
>
> I thought about that. The old (v1?) EINJ type bits are in <acpi/actbl1.h>
>
> So "the right thing"(TM) might be to add some V2 defines there:
>
> #define ACPI_EINJV2_PROCESSOR   BIT(0)
> #define ACPI_EINJV2_MEMORY              BIT(1)
> #define ACPI_EINJV2_PCIX                        BIT(2)
>
> Rafael: Do you want that? I can start the process in the ACPICA tree if you do.

Yes, please!

Hi Tony,

On Wed, Apr 15, 2026 at 11:15 AM Luck, Tony <tony.luck@intel.com> wrote:
>
> > > +static bool is_memory_injection(u32 type, u64 param2)
> > > +{
> > > +       if (is_v2)
> > > +               return type & BIT(1);
> >
> > Hi Tony, just a nit: for readability would it better to introduce a
> > macro for BIT(1)? like ACPI65_EINJV2_ERROR_TYPE_MEMORY
>
> I thought about that. The old (v1?) EINJ type bits are in <acpi/actbl1.h>
>
> So "the right thing"(TM) might be to add some V2 defines there:
>
> #define ACPI_EINJV2_PROCESSOR   BIT(0)
> #define ACPI_EINJV2_MEMORY              BIT(1)
> #define ACPI_EINJV2_PCIX                        BIT(2)

Looks good to me. Let's see what Rafael thinks about adding them.

>
> Rafael: Do you want that? I can start the process in the ACPICA tree if you do.
>
> -Tony

> > > > +static bool is_memory_injection(u32 type, u64 param2)
> > > > +{
> > > > +       if (is_v2)
> > > > +               return type & BIT(1);
> > >
> > > Hi Tony, just a nit: for readability would it better to introduce a
> > > macro for BIT(1)? like ACPI65_EINJV2_ERROR_TYPE_MEMORY
> >
> > I thought about that. The old (v1?) EINJ type bits are in <acpi/actbl1.h>
> >
> > So "the right thing"(TM) might be to add some V2 defines there:
> >
> > #define ACPI_EINJV2_PROCESSOR   BIT(0)
> > #define ACPI_EINJV2_MEMORY              BIT(1)
> > #define ACPI_EINJV2_PCIX                        BIT(2)
>
> Looks good to me. Let's see what Rafael thinks about adding them.
>
> >
> > Rafael: Do you want that? I can start the process in the ACPICA tree if you do.

The sashiko AI review found another spot where code is checking for memory injection
but is unaware of EINJ V2. In einj_error_inject() there's a check to see if this is memory
injection, if so, validate the address to avoid problematic ranges:

Link: https://sashiko.dev/#/patchset/20260415163620.12957-1-tony.luck%40intel.com

I first tried another simple band-aid check for is_v2 ... but the problem seems more structural.
Code passes around "type" all over the place. Sometimes it might be a V1 bitmask,
and sometimes a V2 bitmask.

I'm going to think a bit more on this to see if there is a better solution that adding
checks for "is_v2" all over the place.

-Tony