Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <aeecb9a0-b3b2-cfa2-e5b7-a64d1ffe1c0c@amd.com>
Date:   Thu, 8 Jun 2023 13:48:51 +1000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.5.1
Subject: Re: [RFC PATCH v2 1/3] ACPI: APEI: EINJ: Refactor
 available_error_type_show()
Content-Language: en-US
To:     Yazen Ghannam <yazen.ghannam@amd.com>,
        Avadhut Naik <Avadhut.Naik@amd.com>, rafael@kernel.org,
        gregkh@linuxfoundation.org, lenb@kernel.org,
        linux-acpi@vger.kernel.org, linux-fsdevel@vger.kernel.org
Cc:     avadnaik@amd.com, alexey.kardashevskiy@amd.com,
        linux-kernel@vger.kernel.org
References: <20230525204422.4754-1-Avadhut.Naik@amd.com>
 <20230525204422.4754-2-Avadhut.Naik@amd.com>
 <37f64467-c9d7-826d-de41-aa571b2df0ec@amd.com>
From:   Alexey Kardashevskiy <aik@amd.com>
In-Reply-To: <37f64467-c9d7-826d-de41-aa571b2df0ec@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZXZiMmxWWlFuNnBZVmZrTjNsb3JJaDBWMldvWFRBRmFZVldDT3BHS2cyUDlF?=
 =?utf-8?B?S3ROeXJiVXZ5eVh4b2dXc1laT1AwS2RabjdtendXcWxObit3Tk81bGpYa2ZJ?=
 =?utf-8?B?VEYxTm5reEJYazVsWWFMQTFQeWdWSXBmdEd4NzFhMDFYWGFtNjJ0Skladjk5?=
 =?utf-8?B?UDJ4OHQ5RjRkcmRQZXY5d0hJdFU5ZzRHWUVQNGJHUmwvNGh5cDh2dmZXWWIz?=
 =?utf-8?B?VjRZRzlJcGNDWUdPMjFsOWFLUlpkR05OUEtFVitCd3Zhak9QMU81d1dBQVRh?=
 =?utf-8?B?VzJ0UlU0WjVPOWQxQ3NpemNDb3ZqMFd6cWZERmpITTVhOWhFL0ZaTGxuUlkz?=
 =?utf-8?B?UENUU3NuaEhoK1gzMkRWOW5uMlM0c3ppem9KVG5XVWl0QWdONzJPZ0UwUXZY?=
 =?utf-8?B?RGpLdmZEUXNOL0pVSUhsMndJUUlPaHVjRS9CTHc2LytXOERhc2FMK1Fnci9B?=
 =?utf-8?B?L2RESFRacVZMc2Mzc0IrK1dNdURtSjZGMzBLR2d2OU5XOFg5R2VVc1hsamlT?=
 =?utf-8?B?YmlZUkpOQUxqVlFaR000WVplbEE2bXRDdXZnL0J2c2lScEVZL1N2b1ZkT2sx?=
 =?utf-8?B?RlZUdW5QSWVpUnhIMUQ4dndFMHFQTGxhUlFTRUt1T1ZDQVQ3WjlmS1lmNzFQ?=
 =?utf-8?B?aDJ5WFd1YjlTZW1HSnFnUUluK1NvZHR5eDhPUzNXTEMyYW5kVUFzS3JNamtM?=
 =?utf-8?B?b09FbjJSQ1d6UllhV3ltaW8vaFJpK2tDOUpHdjhkelVLVDBkbnFzeFl2VXg3?=
 =?utf-8?B?V25BSEdsNUNsVGI2TGlvcktnbEduOXdkdHBFSzhMemFleGt0N01rajJ0R0tm?=
 =?utf-8?B?dUZ5dFhTcmdNWE11Y0VhbXNsS213QkVyQXJucSt3bWhvUVJFWDlOOTBvMmRO?=
 =?utf-8?B?dTk2S29nUEh0K2FJQ3R6LzRHdklJNFJvWk1HUGlqWGpUMVNRUkN1SVJIYUNo?=
 =?utf-8?B?UjlYTUlBYU9vTUxxOXJXcmtmaklNTUlWK2xnT09wbm8wL0lZdkd5SVBHRnNH?=
 =?utf-8?B?MmZ5U041SnZFRkxuRjBhZExrYm84Nnl2VmFuVmpvUTVNTFd5UWFiYUdJMG1D?=
 =?utf-8?B?ZzVIZXhlanByallKTG1UWU5qaFFZTFBGZFNFZ1ZJdWhJMlZNN0g2U3ZSOW9m?=
 =?utf-8?B?RngxUUZjSzM1ZC96YStPMGZId2w3WUFtMStOdEFUaDZwamh4Q0Rnd2Vmc3lQ?=
 =?utf-8?B?REFxK0RzVGRBQkJwN2oxNkVHVnNvRVZ0OEZpODVybVppV2dUZDJIYWJVSU5s?=
 =?utf-8?B?eHZxb0ZKT2JicWhaQjR0SDBrYm1HYURFcjdyZkFjRVNCR0JHd2ZjaVBIZGYw?=
 =?utf-8?B?THdiOWZvMXR0Snk1OEVPVkltZzFFZWF3dCtDNHNjNGJBenhlR3J0d0MyTEtv?=
 =?utf-8?B?aTVaN0p1WThoNTQvaUlPRnlzVWo3c0JBWU45TEtUSDUzdGV3ejd4QU9ZNnZM?=
 =?utf-8?B?c2haRENablQ4eHpDR0J5SFNRb1laeUNYNGJUSHluYWlRYWxBOEJJMS9ldkc3?=
 =?utf-8?B?bVUxQk9LWlpaMkVyK3JOdEkxcjBzS0cwL0Npb01xUWRWazZTTUxKYkRUei9w?=
 =?utf-8?B?QmFZdGhuYVlRbFprMmMwWlJ0VkZIWXAyaXhlUFovZ1FRdi8xVWtqeURVMkFZ?=
 =?utf-8?B?TkkxNklPSjRpQjBLbG9zVmxtakJxS0crY29nSmt6a2kwMW1qejVvbklXcjcr?=
 =?utf-8?B?aGE1T0Z2SFZyQm9KMExYOE12SzRFbkVOZTQ1MGhINnBQTnl5c0xXc1lhVTE0?=
 =?utf-8?B?SWI4SDRDM0x1Y0hPYnhoY0NMWXMyRCtCYm1RMmJub1VHNmJqaHJIbFRSdlYv?=
 =?utf-8?B?ZmJaSENFTnVEcGpGUXdaUHEybzF4Sng0TjI5cjdxK2pLMW1qUWpwUFpBa1g3?=
 =?utf-8?B?NTZFanVNWlprV0ZibGErZ1IyY01vOFhIdisrYnBBazBQaWNBZHpqbWpHRGJy?=
 =?utf-8?B?R3VlZW01alE1UDJQZmhxTUtRc3FmOTh5REt5ZG1xU2EwVVJTZnVnYWFIVzVw?=
 =?utf-8?B?SkNvNnlaNGdMbVpES0doZWlOWXpKUUdRNVlhYXFEYnhtbmIwQjI5UGwvQUk2?=
 =?utf-8?B?Z1E3cXh1YUdDTjlBOWpxNkpJYW4xaVZlWW53OWZnM3k4QWUzdEphdG5GSjRo?=
 =?utf-8?Q?At2ZNzmR34J/QmwKT06fJGdmg?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4a54ad92-fcfa-434b-45ad-08db67d34b98
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB2843.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Jun 2023 03:49:01.3620
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: lzT/J9NaoJC2LMqJREdHvkI7QPGDUeJafDY8400cx17Sr2tqCN3pRy0i3ShAaDC3n3cP3aTTnt+iWmobS1fhIQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR12MB7885
X-Spam-Status: No, score=-1.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,NICE_REPLY_A,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 8/6/23 00:20, Yazen Ghannam wrote:
> On 5/25/23 4:44 PM, Avadhut Naik wrote:
>> 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(-)
>>
>> diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c
>> index 013eb621dc92..d5f8dc4df7a5 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[] = {
>> +	{0x00000001, "Processor Correctable"},
>> +	{0x00000002, "Processor Uncorrectable non-fatal"},
>> +	{0x00000004, "Processor Uncorrectable fatal"},
>> +	{0x00000008, "Memory Correctable"},
>> +	{0x00000010, "Memory Uncorrectable non-fatal"},
>> +	{0x00000020, "Memory Uncorrectable fatal"},
>> +	{0x00000040, "PCI Express Correctable"},
>> +	{0x00000080, "PCI Express Uncorrectable non-fatal"},
>> +	{0x00000100, "PCI Express Uncorrectable fatal"},
>> +	{0x00000200, "Platform Correctable"},
>> +	{0x00000400, "Platform Uncorrectable non-fatal"},
>> +	{0x00000800, "Platform Uncorrectable fatal"},
>> +	{0x00001000, "CXL.cache Protocol Correctable"},
>> +	{0x00002000, "CXL.cache Protocol Uncorrectable non-fatal"},
>> +	{0x00004000, "CXL.cache Protocol Uncorrectable fatal"},
>> +	{0x00008000, "CXL.mem Protocol Correctable"},
>> +	{0x00010000, "CXL.mem Protocol Uncorrectable non-fatal"},
>> +	{0x00020000, "CXL.mem Protocol Uncorrectable fatal"},
>>   };
>>
> 
> I think it'd be easier to read if the masks used the BIT() macro rather
> than a hex value.

Makes sense but I'd say because it is easier to match the APCI spec 
which uses the bit numbers, not easier to read (which is arguable).


> 
> Thanks,
> Yazen

-- 
Alexey