[PATCH v2 0/3] ACPI, APEI, EINJ: Add new CXL Error Types

[PATCH v2 0/3] ACPI, APEI, EINJ: Add new CXL Error Types

[Ben Cheatham]

Changes in V2:
	- remove unnecessary comments in patches 2 & 3 (Borislav)
	- change bitshit operation to just use BIT macro (Borislav)

Fix formatting errors alerted by checkpatch.pl, including missing 
lines and indentations to clean up for following patches.
 
Create an array to store error type descriptions for maintainability.

Add new CXL error types so that they are advertised.

Quick Note:	
I sent out an email last week explaining why I was taking over this
patch series, but just in case: Jay's internship at AMD ended a couple
of months ago and I was asked to pick up this patch set for her. I also
said I was going to add a patch in aforementioned email, but I didn't
have a machine to test it so I've left it out. Thanks

Jay Lu (3):
  ACPI, APEI, EINJ: Fix Formatting Errors
  ACPI, APEI, EINJ: Refactor available_error_type_show
  ACPI, APEI, EINJ: Add support for new CXL error types

 drivers/acpi/apei/einj.c | 62 ++++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

-- 
2.34.1

[PATCH v2 1/3] ACPI, APEI, EINJ: Fix Formatting Errors

[Ben Cheatham]

From: Jay Lu <jaylu102@amd.com>

Checkpatch reveals warnings and an error due to missing lines and
incorrect indentations. Add the missing lines after declarations and
fix the suspect indentations.

Signed-off-by: Jay Lu <jaylu102@amd.com>
Signed-off-by: Ben Cheatham <benjamin.cheatham@amd.com>
---
 drivers/acpi/apei/einj.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c
index 6b583373c58a..1a2641937eb6 100644
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -358,6 +358,7 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
 	 */
 	if ((param_extension || acpi5) && (type & MEM_ERROR_MASK) && param2) {
 		struct apei_resources addr_resources;
+
 		apei_resources_init(&addr_resources);
 		trigger_param_region = einj_get_trigger_parameter_region(
 			trigger_tab, param1, param2);
@@ -432,11 +433,11 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 			}
 			v5param->flags = vendor_flags;
 		} else if (flags) {
-				v5param->flags = flags;
-				v5param->memory_address = param1;
-				v5param->memory_address_range = param2;
-				v5param->apicid = param3;
-				v5param->pcie_sbdf = param4;
+			v5param->flags = flags;
+			v5param->memory_address = param1;
+			v5param->memory_address_range = param2;
+			v5param->apicid = param3;
+			v5param->pcie_sbdf = param4;
 		} else {
 			switch (type) {
 			case ACPI_EINJ_PROCESSOR_CORRECTABLE:
@@ -466,6 +467,7 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 			return rc;
 		if (einj_param) {
 			struct einj_parameter *v4param = einj_param;
+
 			v4param->param1 = param1;
 			v4param->param2 = param2;
 		}
@@ -689,8 +691,7 @@ static int __init einj_init(void)
 	if (status == AE_NOT_FOUND) {
 		pr_warn("EINJ table not found.\n");
 		return -ENODEV;
-	}
-	else if (ACPI_FAILURE(status)) {
+	} else if (ACPI_FAILURE(status)) {
 		pr_err("Failed to get EINJ table: %s\n",
 				acpi_format_exception(status));
 		return -EINVAL;
-- 
2.34.1

[PATCH v2 2/3] ACPI, APEI, EINJ: Refactor available_error_type_show

[Ben Cheatham]

From: Jay Lu <jaylu102@amd.com>

Move error type descriptions into an array and loop over error types
to improve readability and maintainability.

Replace seq_printf() with seq_puts() as recommended by checkpatch.pl.

Signed-off-by: Jay Lu <jaylu102@amd.com>
Co-developed-by: Ben Cheatham <benjamin.cheatham@amd.com>
Signed-off-by: Ben Cheatham <benjamin.cheatham@amd.com>
---
 drivers/acpi/apei/einj.c | 41 +++++++++++++++++-----------------------
 1 file changed, 17 insertions(+), 24 deletions(-)

diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c
index 1a2641937eb6..ab86b2f4e719 100644
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -571,6 +571,20 @@ static u64 error_param2;
 static u64 error_param3;
 static u64 error_param4;
 static struct dentry *einj_debug_dir;
+static const char * const einj_error_type_string[] = {
+	"0x00000001\tProcessor Correctable\n",
+	"0x00000002\tProcessor Uncorrectable non-fatal\n",
+	"0x00000004\tProcessor Uncorrectable fatal\n",
+	"0x00000008\tMemory Correctable\n",
+	"0x00000010\tMemory Uncorrectable non-fatal\n",
+	"0x00000020\tMemory Uncorrectable fatal\n",
+	"0x00000040\tPCI Express Correctable\n",
+	"0x00000080\tPCI Express Uncorrectable non-fatal\n",
+	"0x00000100\tPCI Express Uncorrectable fatal\n",
+	"0x00000200\tPlatform Correctable\n",
+	"0x00000400\tPlatform Uncorrectable non-fatal\n",
+	"0x00000800\tPlatform Uncorrectable fatal\n",
+};
 
 static int available_error_type_show(struct seq_file *m, void *v)
 {
@@ -580,30 +594,9 @@ static int available_error_type_show(struct seq_file *m, void *v)
 	rc = einj_get_available_error_type(&available_error_type);
 	if (rc)
 		return rc;
-	if (available_error_type & 0x0001)
-		seq_printf(m, "0x00000001\tProcessor Correctable\n");
-	if (available_error_type & 0x0002)
-		seq_printf(m, "0x00000002\tProcessor Uncorrectable non-fatal\n");
-	if (available_error_type & 0x0004)
-		seq_printf(m, "0x00000004\tProcessor Uncorrectable fatal\n");
-	if (available_error_type & 0x0008)
-		seq_printf(m, "0x00000008\tMemory Correctable\n");
-	if (available_error_type & 0x0010)
-		seq_printf(m, "0x00000010\tMemory Uncorrectable non-fatal\n");
-	if (available_error_type & 0x0020)
-		seq_printf(m, "0x00000020\tMemory Uncorrectable fatal\n");
-	if (available_error_type & 0x0040)
-		seq_printf(m, "0x00000040\tPCI Express Correctable\n");
-	if (available_error_type & 0x0080)
-		seq_printf(m, "0x00000080\tPCI Express Uncorrectable non-fatal\n");
-	if (available_error_type & 0x0100)
-		seq_printf(m, "0x00000100\tPCI Express Uncorrectable fatal\n");
-	if (available_error_type & 0x0200)
-		seq_printf(m, "0x00000200\tPlatform Correctable\n");
-	if (available_error_type & 0x0400)
-		seq_printf(m, "0x00000400\tPlatform Uncorrectable non-fatal\n");
-	if (available_error_type & 0x0800)
-		seq_printf(m, "0x00000800\tPlatform Uncorrectable fatal\n");
+	for (int pos = 0; pos < ARRAY_SIZE(einj_error_type_string); pos++)
+		if (available_error_type & BIT(pos))
+			seq_puts(m, einj_error_type_string[pos]);
 
 	return 0;
 }
-- 
2.34.1

[PATCH v2 3/3] ACPI, APEI, EINJ: Add support for new CXL error types

[Ben Cheatham]

From: Jay Lu <jaylu102@amd.com>

EINJ module allows new CXL error types to be passed through sysfs
interface, but it doesn't advertise new CXL error types in sysfs.
Update EINJ so that it displays new CXL errors.

Signed-off-by: Jay Lu <jaylu102@amd.com>
Co-developed-by: Ben Cheatham <benjamin.cheatham@amd.com>
Signed-off-by: Ben Cheatham <benjamin.cheatham@amd.com>
---
 drivers/acpi/apei/einj.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c
index ab86b2f4e719..82004abb9643 100644
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -584,6 +584,12 @@ static const char * const einj_error_type_string[] = {
 	"0x00000200\tPlatform Correctable\n",
 	"0x00000400\tPlatform Uncorrectable non-fatal\n",
 	"0x00000800\tPlatform Uncorrectable fatal\n",
+	"0x00001000\tCXL.cache Protocol Correctable\n",
+	"0x00002000\tCXL.cache Protocol Uncorrectable non-fatal\n",
+	"0x00004000\tCXL.cache Protocol Uncorrectable fatal\n",
+	"0x00008000\tCXL.mem Protocol Correctable\n",
+	"0x00010000\tCXL.mem Protocol Uncorrectable non-fatal\n",
+	"0x00020000\tCXL.mem Protocol Uncorrectable fatal\n",
 };
 
 static int available_error_type_show(struct seq_file *m, void *v)
-- 
2.34.1

RE: [PATCH v2 0/3] ACPI, APEI, EINJ: Add new CXL Error Types

[Luck, Tony]

> Add new CXL error types so that they are advertised.
>
> Quick Note:	
> I sent out an email last week explaining why I was taking over this
> patch series, but just in case: Jay's internship at AMD ended a couple
> of months ago and I was asked to pick up this patch set for her. I also
> said I was going to add a patch in aforementioned email, but I didn't
> have a machine to test it so I've left it out. Thanks

These look like a good start. But will there soon be changes to:

	drivers/acpi/apei/einj.c

to do something with these new error types?

-Tony

Re: [PATCH v2 0/3] ACPI, APEI, EINJ: Add new CXL Error Types

[Ben Cheatham]

On 12/6/22 3:11 PM, Luck, Tony wrote:
>> Add new CXL error types so that they are advertised.
>>
>> Quick Note:	
>> I sent out an email last week explaining why I was taking over this
>> patch series, but just in case: Jay's internship at AMD ended a couple
>> of months ago and I was asked to pick up this patch set for her. I also
>> said I was going to add a patch in aforementioned email, but I didn't
>> have a machine to test it so I've left it out. Thanks
> These look like a good start. But will there soon be changes to:
>
> 	drivers/acpi/apei/einj.c
>
> to do something with these new error types?
>
> -Tony

Hi Tony,

The last patch I mentioned leaving out added support for injecting CXL 
errors, but I don't have access to a machine that I can test it with at 
the moment so it'll probably have to wait.

RE: [PATCH v2 0/3] ACPI, APEI, EINJ: Add new CXL Error Types

[Luck, Tony]

Hi Ben,

> The last patch I mentioned leaving out added support for injecting CXL 
> errors, but I don't have access to a machine that I can test it with at 
> the moment so it'll probably have to wait.

Parts 1 & 2 of your series can be applied now (as nice cleanups).

But part 3 would just be confusing to users without the matching patch
to add CXL injection support.

I.e. a user might

# cat /sys/kernel/debug/apei/einj/available_error_type

and see:

0x00001000	CXL.cache Protocol Correctable

But:

# echo 0x1000 > /sys/kernel/debug/apei/einj/error_type

wouldn't do anything useful (may do weird stuff since the driver
doesn't appear to validate the "type" ... would be great if you fixed
that while you are digging around in this code :-).

So I'm happy to offer up a "Reviewed-by: Tony Luck <tony.luck@intel.com>"
for all three parts. I just think that part 3 should not be applied until the
rest of the code to go with it is ready.

-Tony

Re: [PATCH v2 0/3] ACPI, APEI, EINJ: Add new CXL Error Types

[Rafael J. Wysocki]

On Tue, Dec 6, 2022 at 10:37 PM Luck, Tony <tony.luck@intel.com> wrote:
>
> Hi Ben,
>
> > The last patch I mentioned leaving out added support for injecting CXL
> > errors, but I don't have access to a machine that I can test it with at
> > the moment so it'll probably have to wait.
>
> Parts 1 & 2 of your series can be applied now (as nice cleanups).
>
> But part 3 would just be confusing to users without the matching patch
> to add CXL injection support.
>
> I.e. a user might
>
> # cat /sys/kernel/debug/apei/einj/available_error_type
>
> and see:
>
> 0x00001000      CXL.cache Protocol Correctable
>
> But:
>
> # echo 0x1000 > /sys/kernel/debug/apei/einj/error_type
>
> wouldn't do anything useful (may do weird stuff since the driver
> doesn't appear to validate the "type" ... would be great if you fixed
> that while you are digging around in this code :-).
>
> So I'm happy to offer up a "Reviewed-by: Tony Luck <tony.luck@intel.com>"
> for all three parts. I just think that part 3 should not be applied until the
> rest of the code to go with it is ready.

I agree, so I've done accordingly.

Patches [1-2/3] have been applied as 6.2 material (with very minor
subject adjustments) and Ben please resend patch [3/3] when sending
the other material mentioned above (and please feel free to add the
tag from Tony to it when doing that).

Re: [PATCH v2 0/3] ACPI, APEI, EINJ: Add new CXL Error Types

[Ben Cheatham]

On 12/7/22 11:19 AM, Rafael J. Wysocki wrote:
> On Tue, Dec 6, 2022 at 10:37 PM Luck, Tony <tony.luck@intel.com> wrote:
>> Hi Ben,
>>
>>> The last patch I mentioned leaving out added support for injecting CXL
>>> errors, but I don't have access to a machine that I can test it with at
>>> the moment so it'll probably have to wait.
>> Parts 1 & 2 of your series can be applied now (as nice cleanups).
>>
>> But part 3 would just be confusing to users without the matching patch
>> to add CXL injection support.
>>
>> I.e. a user might
>>
>> # cat /sys/kernel/debug/apei/einj/available_error_type
>>
>> and see:
>>
>> 0x00001000      CXL.cache Protocol Correctable
>>
>> But:
>>
>> # echo 0x1000 > /sys/kernel/debug/apei/einj/error_type
>>
>> wouldn't do anything useful (may do weird stuff since the driver
>> doesn't appear to validate the "type" ... would be great if you fixed
>> that while you are digging around in this code :-).
>>
>> So I'm happy to offer up a "Reviewed-by: Tony Luck <tony.luck@intel.com>"
>> for all three parts. I just think that part 3 should not be applied until the
>> rest of the code to go with it is ready.
> I agree, so I've done accordingly.
>
> Patches [1-2/3] have been applied as 6.2 material (with very minor
> subject adjustments) and Ben please resend patch [3/3] when sending
> the other material mentioned above (and please feel free to add the
> tag from Tony to it when doing that).
Will do. Thanks for the review!