linux-kernel - Re: [PATCH 2/4] acpi/ghes, efi/cper: Recognize and process CXL Protocol Errors.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 10 Jun 2024 12:07:48 -0700
From: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: Alison Schofield <alison.schofield@...el.com>, linux-efi@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-cxl@...r.kernel.org,
 Ard Biesheuvel <ardb@...nel.org>, Vishal Verma <vishal.l.verma@...el.com>,
 Ira Weiny <ira.weiny@...el.com>, Dan Williams <dan.j.williams@...el.com>,
 Yazen Ghannam <yazen.ghannam@....com>, Bowman Terry <terry.bowman@....com>
Subject: Re: [PATCH 2/4] acpi/ghes, efi/cper: Recognize and process CXL
 Protocol Errors.

On 6/7/2024 8:26 AM, Jonathan Cameron wrote:
> On Thu, 23 May 2024 14:21:40 -0700
> Smita Koralahalli <Smita.KoralahalliChannabasappa@....com> wrote:
> 
>> Hi Alison,
>>
>> On 5/22/2024 5:03 PM, Alison Schofield wrote:
>>> On Wed, May 22, 2024 at 03:08:37PM +0000, Smita Koralahalli wrote:
>>>> UEFI v2.10 section N.2.13 defines a CPER record for CXL Protocol errors.
>>>>
>>>> Add GHES support to detect CXL CPER Protocol Error Record and Cache Error
>>>> Severity, Device ID, Device Serial number and CXL RAS capability struct in
>>>> struct cxl_cper_prot_err. Include this struct as a member of struct
>>>> cxl_cper_work_data.
>>>>
>>>> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>

[snip]


>>>> +	 * The device ID or agent address is required for CXL RCD, CXL
>>>> +	 * SLD, CXL LD, CXL Fabric Manager Managed LD, CXL Root Port,
>>>> +	 * CXL Downstream Switch Port and CXL Upstream Switch Port.
>>>> +	 */
>>>> +	if (prot_err->agent_type <= 0x7 && prot_err->agent_type != RCH_DP) {
>>>
>>> For this check against agent_type, and the similar one below, would a boolean
>>> array indexed by the agent type work? That would avoid the <= 0x7 and > 0x4
>>> below. It seems one array would suffice for this case, but naming it isn't obvious
>>> to me. Maybe it'll be to you.
>>>
>>> Something similar to what is done for prot_err_agent_type_strs[]
>>>
>>> static const bool agent_requires_id_address_serial[] = {
>>> 	true,	/* RDC */ 	
>>> 	false,	/* RCH_DP */
> 	[RCD] = false,
> 
> etc rather than comments would be neater.
> Given two similar things already. Maybe time for a little structure.
> 
> //with better name than this
> struct agent_reqs {
> 	bool sn;
> 	bool sbdf;
> };
> 
> static const agent_reqs agent_reqs[] = {
> 	[RCD] = { .sn = false, .sbdf = true, },
> };
> 
> etc.
> 
> Maybe just bring the the string in as well
> 
> struct agent_info {
> 	const char *string;
> 	bool req_sn;
> 	bool req_sbdf;
> };
> 
> static const agent_info agent_info[] = {
> 	[RD] = {
> 		.string = "Restricted CXL Device",
> 		.req_sn = false,
> 		.req_sbdf = true,
> 	},
> };
> 
> Values made up, but hopefully conveys that moving to having
> all the data in one place makes it harder to forget stuff
> for new entries etc.
> 

Okay. I was considering 2D array without naming before. Something like:

static const bool agent_requires_id_address[][] = {
        /* Device_ID, Serial_num */
        {true, true},   /* RCD */
        {false, false}, /* RCH_DP */

Let me change to array of structures if naming helps.

Thanks,
Smita

[snip]