From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
Cc: <linux-efi@vger.kernel.org>, <linux-cxl@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, Ard Biesheuvel <ardb@kernel.org>,
"Alison Schofield" <alison.schofield@intel.com>,
Vishal Verma <vishal.l.verma@intel.com>,
Ira Weiny <ira.weiny@intel.com>,
Ben Widawsky <bwidawsk@kernel.org>,
Dan Williams <dan.j.williams@intel.com>,
"Yazen Ghannam" <yazen.ghannam@amd.com>
Subject: Re: [PATCH v2 1/2] efi/cper, cxl: Decode CXL Protocol Error Section
Date: Thu, 4 Jan 2024 12:06:09 +0000 [thread overview]
Message-ID: <20240104120609.00005ebd@Huawei.com> (raw)
In-Reply-To: <20221028200950.67505-2-Smita.KoralahalliChannabasappa@amd.com>
On Fri, 28 Oct 2022 20:09:49 +0000
Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com> wrote:
> Add support for decoding CXL Protocol Error Section as defined in UEFI 2.10
> Section N.2.13.
>
> Do the section decoding in a new cper_cxl.c file. This new file will be
> used in the future for more CXL CPERs decode support. Add this to the
> existing UEFI_CPER config.
>
> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
Obviously very late with this comment, but I now have a test setup running
so a minor formatting issue became apparent that would be nice to tidy up
I think.
> +void cper_print_prot_err(const char *pfx, const struct cper_sec_prot_err *prot_err)
> +{
> + if (prot_err->valid_bits & PROT_ERR_VALID_AGENT_TYPE)
> + pr_info("%s agent_type: %d, %s\n", pfx, prot_err->agent_type,
> + prot_err->agent_type < ARRAY_SIZE(prot_err_agent_type_strs)
> + ? prot_err_agent_type_strs[prot_err->agent_type]
> + : "unknown");
> +
> + if (prot_err->valid_bits & PROT_ERR_VALID_AGENT_ADDRESS) {
> + switch (prot_err->agent_type) {
> + /*
> + * According to UEFI 2.10 Section N.2.13, the term CXL Device
> + * is used to refer to Restricted CXL Device, CXL Device, CXL
> + * Logical Device or a CXL Fabric Manager Managed Logical
> + * Device.
> + */
> + case RCD:
> + case DEVICE:
> + case LD:
> + case FMLD:
> + case RP:
> + case DSP:
> + case USP:
> + pr_info("%s agent_address: %04x:%02x:%02x.%x\n",
> + pfx, prot_err->agent_addr.segment,
> + prot_err->agent_addr.bus,
> + prot_err->agent_addr.device,
> + prot_err->agent_addr.function);
> + break;
> + case RCH_DP:
> + pr_info("%s rcrb_base_address: 0x%016llx\n", pfx,
> + prot_err->agent_addr.rcrb_base_addr);
> + break;
> + default:
> + break;
> + }
> + }
> +
> + if (prot_err->valid_bits & PROT_ERR_VALID_DEVICE_ID) {
> + const __u8 *class_code;
> +
> + switch (prot_err->agent_type) {
> + case RCD:
> + case DEVICE:
> + case LD:
> + case FMLD:
> + case RP:
> + case DSP:
> + case USP:
> + pr_info("%s slot: %d\n", pfx,
> + prot_err->device_id.slot >> CPER_PCIE_SLOT_SHIFT);
> + pr_info("%s vendor_id: 0x%04x, device_id: 0x%04x\n",
> + pfx, prot_err->device_id.vendor_id,
> + prot_err->device_id.device_id);
> + pr_info("%s sub_vendor_id: 0x%04x, sub_device_id: 0x%04x\n",
> + pfx, prot_err->device_id.subsystem_vendor_id,
> + prot_err->device_id.subsystem_id);
> + class_code = prot_err->device_id.class_code;
> + pr_info("%s class_code: %02x%02x\n", pfx,
> + class_code[1], class_code[0]);
> + break;
> + default:
> + break;
> + }
> + }
> +
> + if (prot_err->valid_bits & PROT_ERR_VALID_SERIAL_NUMBER) {
> + switch (prot_err->agent_type) {
> + case RCD:
> + case DEVICE:
> + case LD:
> + case FMLD:
> + pr_info("%s lower_dw: 0x%08x, upper_dw: 0x%08x\n", pfx,
> + prot_err->dev_serial_num.lower_dw,
> + prot_err->dev_serial_num.upper_dw);
Doesn't say what it is the lower dw of. Good to add a mention of serial number.
> + break;
> + default:
> + break;
> + }
> + }
> +
> + if (prot_err->valid_bits & PROT_ERR_VALID_CAPABILITY) {
> + switch (prot_err->agent_type) {
> + case RCD:
> + case DEVICE:
> + case LD:
> + case FMLD:
> + case RP:
> + case DSP:
> + case USP:
No heading for this. Which means the output just ends up with something like:
lower_dw: 0x00000004, upper_dw = 0x00000000
00000000: 00020010 00008000 0000000f 00000411
00000010: 00110000 00000000 00000000 00000000
etc So also wants a space before the hex dump or indeed greater indent once that's
added.
Similar formatting things apply to later changes to the message such as the header
log register in the next patch.
Whilst not a bug as such, pretty printing is always nice :)
Jonathan
> + print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4,
> + prot_err->capability,
> + sizeof(prot_err->capability), 0);
> + break;
> + default:
> + break;
> + }
> + }
> +
> + if (prot_err->valid_bits & PROT_ERR_VALID_DVSEC) {
> + pr_info("%s DVSEC length: 0x%04x\n", pfx, prot_err->dvsec_len);
> +
> + pr_info("%s CXL DVSEC:\n", pfx);
> + print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, (prot_err + 1),
> + prot_err->dvsec_len, 0);
> + }
> +}
> diff --git a/drivers/firmware/efi/cper_cxl.h b/drivers/firmware/efi/cper_cxl.h
> new file mode 100644
> index 000000000000..86bfcf7909ec
> --- /dev/null
> +++ b/drivers/firmware/efi/cper_cxl.h
> @@ -0,0 +1,66 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * UEFI Common Platform Error Record (CPER) support for CXL Section.
> + *
> + * Copyright (C) 2022 Advanced Micro Devices, Inc.
> + *
> + * Author: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> + */
> +
> +#ifndef LINUX_CPER_CXL_H
> +#define LINUX_CPER_CXL_H
> +
> +/* CXL Protocol Error Section */
> +#define CPER_SEC_CXL_PROT_ERR \
> + GUID_INIT(0x80B9EFB4, 0x52B5, 0x4DE3, 0xA7, 0x77, 0x68, 0x78, \
> + 0x4B, 0x77, 0x10, 0x48)
> +
> +#pragma pack(1)
> +
> +/* Compute Express Link Protocol Error Section, UEFI v2.10 sec N.2.13 */
> +struct cper_sec_prot_err {
> + u64 valid_bits;
> + u8 agent_type;
> + u8 reserved[7];
> +
> + /*
> + * Except for RCH Downstream Port, all the remaining CXL Agent
> + * types are uniquely identified by the PCIe compatible SBDF number.
> + */
> + union {
> + u64 rcrb_base_addr;
> + struct {
> + u8 function;
> + u8 device;
> + u8 bus;
> + u16 segment;
> + u8 reserved_1[3];
> + };
> + } agent_addr;
> +
> + struct {
> + u16 vendor_id;
> + u16 device_id;
> + u16 subsystem_vendor_id;
> + u16 subsystem_id;
> + u8 class_code[2];
> + u16 slot;
> + u8 reserved_1[4];
> + } device_id;
> +
> + struct {
> + u32 lower_dw;
> + u32 upper_dw;
> + } dev_serial_num;
> +
> + u8 capability[60];
> + u16 dvsec_len;
> + u16 err_len;
> + u8 reserved_2[4];
> +};
> +
> +#pragma pack()
> +
> +void cper_print_prot_err(const char *pfx, const struct cper_sec_prot_err *prot_err);
> +
> +#endif //__CPER_CXL_
next prev parent reply other threads:[~2024-01-04 12:06 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-28 20:09 [PATCH v2 0/2] efi/cper, cxl: Decode CXL Protocol Errors CPER Smita Koralahalli
2022-10-28 20:09 ` [PATCH v2 1/2] efi/cper, cxl: Decode CXL Protocol Error Section Smita Koralahalli
2022-11-03 17:49 ` Jonathan Cameron
2024-01-04 12:06 ` Jonathan Cameron [this message]
2022-10-28 20:09 ` [PATCH v2 2/2] efi/cper, cxl: Decode CXL Error Log Smita Koralahalli
2022-11-03 17:57 ` Jonathan Cameron
2022-11-09 9:30 ` [PATCH v2 0/2] efi/cper, cxl: Decode CXL Protocol Errors CPER Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240104120609.00005ebd@Huawei.com \
--to=jonathan.cameron@huawei.com \
--cc=Smita.KoralahalliChannabasappa@amd.com \
--cc=alison.schofield@intel.com \
--cc=ardb@kernel.org \
--cc=bwidawsk@kernel.org \
--cc=dan.j.williams@intel.com \
--cc=ira.weiny@intel.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-efi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=vishal.l.verma@intel.com \
--cc=yazen.ghannam@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).