2022-07-29 15:39:27

by Jay Lu

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

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.

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.27.0


2022-07-29 15:40:08

by Jay Lu

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

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 <[email protected]>
---
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 a68103280f74..4aed0eab329e 100644
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -582,6 +582,12 @@ static const char * const einj_error_type_string[] = {
"0x00000200\tPlatform Correctable\n", /* bit 9 */
"0x00000400\tPlatform Uncorrectable non-fatal\n", /* bit 10 */
"0x00000800\tPlatform Uncorrectable fatal\n", /* bit 11 */
+ "0x00001000\tCXL.cache Protocol Correctable\n", /* bit 12 */
+ "0x00002000\tCXL.cache Protocol Uncorrectable non-fatal\n", /* bit 13 */
+ "0x00004000\tCXL.cache Protocol Uncorrectable fatal\n", /* bit 14 */
+ "0x00008000\tCXL.mem Protocol Correctable\n", /* bit 15 */
+ "0x00010000\tCXL.mem Protocol Uncorrectable non-fatal\n", /* bit 16 */
+ "0x00020000\tCXL.mem Protocol Uncorrectable fatal\n", /* bit 17 */
};

static int available_error_type_show(struct seq_file *m, void *v)
--
2.27.0

2022-07-29 16:13:42

by Jay Lu

[permalink] [raw]
Subject: [PATCH 1/3] ACPI, APEI, EINJ: Fix Formatting Errors

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 <[email protected]>
---
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 d4326ec12d29..da039c630fd0 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;
}
@@ -687,8 +689,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.27.0

2022-07-29 16:24:51

by Jay Lu

[permalink] [raw]
Subject: [PATCH 2/3] ACPI, APEI, EINJ: Refactor available_error_type_show

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 <[email protected]>
---
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 da039c630fd0..a68103280f74 100644
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -569,6 +569,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", /* bit 0 */
+ "0x00000002\tProcessor Uncorrectable non-fatal\n", /* bit 1 */
+ "0x00000004\tProcessor Uncorrectable fatal\n", /* bit 2 */
+ "0x00000008\tMemory Correctable\n", /* bit 3 */
+ "0x00000010\tMemory Uncorrectable non-fatal\n", /* bit 4 */
+ "0x00000020\tMemory Uncorrectable fatal\n", /* bit 5 */
+ "0x00000040\tPCI Express Correctable\n", /* bit 6 */
+ "0x00000080\tPCI Express Uncorrectable non-fatal\n", /* bit 7 */
+ "0x00000100\tPCI Express Uncorrectable fatal\n", /* bit 8 */
+ "0x00000200\tPlatform Correctable\n", /* bit 9 */
+ "0x00000400\tPlatform Uncorrectable non-fatal\n", /* bit 10 */
+ "0x00000800\tPlatform Uncorrectable fatal\n", /* bit 11 */
+};

static int available_error_type_show(struct seq_file *m, void *v)
{
@@ -578,30 +592,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(0) << pos))
+ seq_puts(m, einj_error_type_string[pos]);

return 0;
}
--
2.27.0

2022-09-20 10:08:02

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 2/3] ACPI, APEI, EINJ: Refactor available_error_type_show

On Fri, Jul 29, 2022 at 10:35:49AM -0500, Jay Lu wrote:
> 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 <[email protected]>
> ---
> 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 da039c630fd0..a68103280f74 100644
> --- a/drivers/acpi/apei/einj.c
> +++ b/drivers/acpi/apei/einj.c
> @@ -569,6 +569,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", /* bit 0 */
> + "0x00000002\tProcessor Uncorrectable non-fatal\n", /* bit 1 */
> + "0x00000004\tProcessor Uncorrectable fatal\n", /* bit 2 */
> + "0x00000008\tMemory Correctable\n", /* bit 3 */
> + "0x00000010\tMemory Uncorrectable non-fatal\n", /* bit 4 */
> + "0x00000020\tMemory Uncorrectable fatal\n", /* bit 5 */
> + "0x00000040\tPCI Express Correctable\n", /* bit 6 */
> + "0x00000080\tPCI Express Uncorrectable non-fatal\n", /* bit 7 */
> + "0x00000100\tPCI Express Uncorrectable fatal\n", /* bit 8 */
> + "0x00000200\tPlatform Correctable\n", /* bit 9 */
> + "0x00000400\tPlatform Uncorrectable non-fatal\n", /* bit 10 */
> + "0x00000800\tPlatform Uncorrectable fatal\n", /* bit 11 */
^^^^^^^^^^^^

Those comments look useless - you have the bit numbers in front already.


> static int available_error_type_show(struct seq_file *m, void *v)
> {
> @@ -578,30 +592,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(0) << pos))
^^^^^^^^^^^^^^^

That's a weird way of saying:

BIT(pos)

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2022-11-30 21:07:31

by Ben Cheatham

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

Hi all,

Jay's internship at AMD ended a couple of months ago and I was asked to
pick this up for her. I made the suggested changes and also have a EINJ
patch given to me by Yazen that has to do with this one so I'll be
adding that in as well. I'm still waiting for a machine to test with,
but I expect to send out v2 later this week. Thanks

On 7/29/22 10:35 AM, Jay Lu wrote:
> 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.
>
> 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(-)
>