[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7fe041ed-9ee3-7cbe-ddce-fb727fba12c7@amd.com>
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]
Powered by blists - more mailing lists