diff mbox series

[RESEND,v5,1/4] ACPI: APEI: EINJ: Refactor available_error_type_show()

Message ID 20231107213647.1405493-2-avadhut.naik@amd.com (mailing list archive)
State Superseded, archived
Headers show
Series Add support for Vendor Defined Error Types in Einj Module | expand

Commit Message

Avadhut Naik Nov. 7, 2023, 9:36 p.m. UTC 
From: Avadhut Naik <Avadhut.Naik@amd.com>

OSPM can discover the error injection capabilities of the platform by
executing GET_ERROR_TYPE error injection action.[1] The action returns
a DWORD representing a bitmap of platform supported error injections.[2]

The available_error_type_show() function determines the bits set within
this DWORD and provides a verbose output, from einj_error_type_string
array, through /sys/kernel/debug/apei/einj/available_error_type file.

The function however, assumes one to one correspondence between an error's
position in the bitmap and its array entry offset. Consequently, some
errors like Vendor Defined Error Type fail this assumption and will
incorrectly be shown as not supported, even if their corresponding bit is
set in the bitmap and they have an entry in the array.

Navigate around the issue by converting einj_error_type_string into an
array of structures with a predetermined mask for all error types
corresponding to their bit position in the DWORD returned by GET_ERROR_TYPE
action. The same breaks the aforementioned assumption resulting in all
supported error types by a platform being outputted through the above
available_error_type file.

[1] ACPI specification 6.5, Table 18.25
[2] ACPI specification 6.5, Table 18.30

Suggested-by: Alexey Kardashevskiy <alexey.kardashevskiy@amd.com>
Signed-off-by: Avadhut Naik <Avadhut.Naik@amd.com>
---
 drivers/acpi/apei/einj.c | 43 ++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

Comments

Borislav Petkov Nov. 8, 2023, 8:19 p.m. UTC | #1 
On Tue, Nov 07, 2023 at 03:36:44PM -0600, Avadhut Naik wrote:
> +static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
> +	{BIT(0), "Processor Correctable"},
> +	{BIT(1), "Processor Uncorrectable non-fatal"},
> +	{BIT(2), "Processor Uncorrectable fatal"},
> +	{BIT(3), "Memory Correctable"},
> +	{BIT(4), "Memory Uncorrectable non-fatal"},
> +	{BIT(5), "Memory Uncorrectable fatal"},
> +	{BIT(6), "PCI Express Correctable"},
> +	{BIT(7), "PCI Express Uncorrectable non-fatal"},
> +	{BIT(8), "PCI Express Uncorrectable fatal"},
> +	{BIT(9), "Platform Correctable"},
> +	{BIT(10), "Platform Uncorrectable non-fatal"},
> +	{BIT(11), "Platform Uncorrectable fatal"},
> +	{BIT(12), "CXL.cache Protocol Correctable"},
> +	{BIT(13), "CXL.cache Protocol Uncorrectable non-fatal"},
> +	{BIT(14), "CXL.cache Protocol Uncorrectable fatal"},
> +	{BIT(15), "CXL.mem Protocol Correctable"},
> +	{BIT(16), "CXL.mem Protocol Uncorrectable non-fatal"},
> +	{BIT(17), "CXL.mem Protocol Uncorrectable fatal"},

Might as well put spaces between the '{' and '}' brackets for better
readability.

>  static int available_error_type_show(struct seq_file *m, void *v)
> @@ -607,8 +607,9 @@ static int available_error_type_show(struct seq_file *m, void *v)
>  	if (rc)
>  		return rc;
>  	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]);
> +		if (available_error_type & einj_error_type_string[pos].mask)

Call that variable simply "error_type". Those are simple functions, one
can see that it is the available error type.

> +			seq_printf(m, "0x%08x\t%s\n", einj_error_type_string[pos].mask,
> +				   einj_error_type_string[pos].str);
>  
>  	return 0;

But those are just nitpicks.

Reviewed-by: Borislav Petkov (AMD) <bp@alien8.de>

Thx.
Naik, Avadhut Nov. 16, 2023, 9:46 p.m. UTC | #2 
Hi,

On 11/8/2023 14:19, Borislav Petkov wrote:
> On Tue, Nov 07, 2023 at 03:36:44PM -0600, Avadhut Naik wrote:
>> +static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
>> +	{BIT(0), "Processor Correctable"},
>> +	{BIT(1), "Processor Uncorrectable non-fatal"},
>> +	{BIT(2), "Processor Uncorrectable fatal"},
>> +	{BIT(3), "Memory Correctable"},
>> +	{BIT(4), "Memory Uncorrectable non-fatal"},
>> +	{BIT(5), "Memory Uncorrectable fatal"},
>> +	{BIT(6), "PCI Express Correctable"},
>> +	{BIT(7), "PCI Express Uncorrectable non-fatal"},
>> +	{BIT(8), "PCI Express Uncorrectable fatal"},
>> +	{BIT(9), "Platform Correctable"},
>> +	{BIT(10), "Platform Uncorrectable non-fatal"},
>> +	{BIT(11), "Platform Uncorrectable fatal"},
>> +	{BIT(12), "CXL.cache Protocol Correctable"},
>> +	{BIT(13), "CXL.cache Protocol Uncorrectable non-fatal"},
>> +	{BIT(14), "CXL.cache Protocol Uncorrectable fatal"},
>> +	{BIT(15), "CXL.mem Protocol Correctable"},
>> +	{BIT(16), "CXL.mem Protocol Uncorrectable non-fatal"},
>> +	{BIT(17), "CXL.mem Protocol Uncorrectable fatal"},
> 
> Might as well put spaces between the '{' and '}' brackets for better
> readability.
> 
>>  static int available_error_type_show(struct seq_file *m, void *v)
>> @@ -607,8 +607,9 @@ static int available_error_type_show(struct seq_file *m, void *v)
>>  	if (rc)
>>  		return rc;
>>  	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]);
>> +		if (available_error_type & einj_error_type_string[pos].mask)
> 
> Call that variable simply "error_type". Those are simple functions, one
> can see that it is the available error type.
> 
>> +			seq_printf(m, "0x%08x\t%s\n", einj_error_type_string[pos].mask,
>> +				   einj_error_type_string[pos].str);
>>  
>>  	return 0;
> 
> But those are just nitpicks.
> 
> Reviewed-by: Borislav Petkov (AMD) <bp@alien8.de>
> 
Thanks for reviewing. Will address this in v6.
> Thx.
>
diff mbox series

Patch

diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c
index 013eb621dc92..ee360fcb1618 100644
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -577,25 +577,25 @@  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",
-	"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 struct { u32 mask; const char *str; } const einj_error_type_string[] = {
+	{BIT(0), "Processor Correctable"},
+	{BIT(1), "Processor Uncorrectable non-fatal"},
+	{BIT(2), "Processor Uncorrectable fatal"},
+	{BIT(3), "Memory Correctable"},
+	{BIT(4), "Memory Uncorrectable non-fatal"},
+	{BIT(5), "Memory Uncorrectable fatal"},
+	{BIT(6), "PCI Express Correctable"},
+	{BIT(7), "PCI Express Uncorrectable non-fatal"},
+	{BIT(8), "PCI Express Uncorrectable fatal"},
+	{BIT(9), "Platform Correctable"},
+	{BIT(10), "Platform Uncorrectable non-fatal"},
+	{BIT(11), "Platform Uncorrectable fatal"},
+	{BIT(12), "CXL.cache Protocol Correctable"},
+	{BIT(13), "CXL.cache Protocol Uncorrectable non-fatal"},
+	{BIT(14), "CXL.cache Protocol Uncorrectable fatal"},
+	{BIT(15), "CXL.mem Protocol Correctable"},
+	{BIT(16), "CXL.mem Protocol Uncorrectable non-fatal"},
+	{BIT(17), "CXL.mem Protocol Uncorrectable fatal"},
 };
 
 static int available_error_type_show(struct seq_file *m, void *v)
@@ -607,8 +607,9 @@  static int available_error_type_show(struct seq_file *m, void *v)
 	if (rc)
 		return rc;
 	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]);
+		if (available_error_type & einj_error_type_string[pos].mask)
+			seq_printf(m, "0x%08x\t%s\n", einj_error_type_string[pos].mask,
+				   einj_error_type_string[pos].str);
 
 	return 0;
 }