| 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
| ||
|
Message-ID: <d5d2eff8-0c04-51c5-0128-b6c7e848552d@amd.com>
Date: Thu, 3 Oct 2024 12:31:48 -0700
From: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>
To: Ira Weiny <ira.weiny@...el.com>, linux-efi@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-cxl@...r.kernel.org
Cc: Ard Biesheuvel <ardb@...nel.org>,
Alison Schofield <alison.schofield@...el.com>,
Vishal Verma <vishal.l.verma@...el.com>,
Dan Williams <dan.j.williams@...el.com>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Yazen Ghannam <yazen.ghannam@....com>, Bowman Terry <terry.bowman@....com>
Subject: Re: [PATCH v2 4/4] acpi/ghes, cxl/pci: Trace FW-First CXL Protocol
Errors
On 10/1/2024 8:52 AM, Ira Weiny wrote:
> Smita Koralahalli wrote:
>> When PCIe AER is in FW-First, OS should process CXL Protocol errors from
>> CPER records.
>>
>> Reuse the existing work queue cxl_cper_work registered with GHES to notify
>> the CXL subsystem on a Protocol error.
>>
>> The defined trace events cxl_aer_uncorrectable_error and
>> cxl_aer_correctable_error currently trace native CXL AER errors. Reuse
>> them to trace FW-First Protocol Errors.
>>
>> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>
>> ---
>> v2:
>> Removed pr_warn for serial number.
>> p_err -> rec/p_rec.
>> ---
>> drivers/acpi/apei/ghes.c | 14 ++++++++++++++
>> drivers/cxl/core/pci.c | 24 ++++++++++++++++++++++++
>> drivers/cxl/cxlpci.h | 3 +++
>> drivers/cxl/pci.c | 20 ++++++++++++++++++--
>> include/cxl/event.h | 1 +
>> 5 files changed, 60 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>> index 9dcf0f78458f..5082885e1f2c 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -723,6 +723,20 @@ static void cxl_cper_handle_prot_err(struct acpi_hest_generic_data *gdata)
>>
>> if (cxl_cper_handle_prot_err_info(gdata, &wd.p_rec))
>> return;
>> +
>> + guard(spinlock_irqsave)(&cxl_cper_work_lock);
>> +
>> + if (!cxl_cper_work)
>> + return;
>> +
>> + wd.event_type = CXL_CPER_EVENT_PROT_ERR;
>> +
>> + if (!kfifo_put(&cxl_cper_fifo, wd)) {
>> + pr_err_ratelimited("CXL CPER kfifo overflow\n");
>> + return;
>> + }
>> +
>> + schedule_work(cxl_cper_work);
>> }
>>
>> int cxl_cper_register_work(struct work_struct *work)
>> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
>> index 5b46bc46aaa9..39ef24c8991f 100644
>> --- a/drivers/cxl/core/pci.c
>> +++ b/drivers/cxl/core/pci.c
>> @@ -650,6 +650,30 @@ void read_cdat_data(struct cxl_port *port)
>> }
>> EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);
>>
>> +void cxl_trace_prot_err(struct cxl_dev_state *cxlds,
>> + struct cxl_cper_prot_err *rec)
>> +{
>> + u32 status, fe;
>> +
>> + if (rec->severity == CXL_AER_CORRECTABLE) {
>> + status = rec->cxl_ras.cor_status & ~rec->cxl_ras.cor_mask;
>> +
>> + trace_cxl_aer_correctable_error(cxlds->cxlmd, status);
>> + } else {
>> + status = rec->cxl_ras.uncor_status & ~rec->cxl_ras.uncor_mask;
>> +
>> + if (hweight32(status) > 1)
>> + fe = BIT(FIELD_GET(CXL_RAS_CAP_CONTROL_FE_MASK,
>> + rec->cxl_ras.cap_control));
>> + else
>> + fe = status;
>> +
>> + trace_cxl_aer_uncorrectable_error(cxlds->cxlmd, status, fe,
>> + rec->cxl_ras.header_log);
>> + }
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_trace_prot_err, CXL);
>> +
>> static void __cxl_handle_cor_ras(struct cxl_dev_state *cxlds,
>> void __iomem *ras_base)
>> {
>> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
>> index 4da07727ab9c..8acd8f2c39c9 100644
>> --- a/drivers/cxl/cxlpci.h
>> +++ b/drivers/cxl/cxlpci.h
>> @@ -129,4 +129,7 @@ void read_cdat_data(struct cxl_port *port);
>> void cxl_cor_error_detected(struct pci_dev *pdev);
>> pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
>> pci_channel_state_t state);
>> +struct cxl_cper_prot_err;
>> +void cxl_trace_prot_err(struct cxl_dev_state *cxlds,
>> + struct cxl_cper_prot_err *rec);
>> #endif /* __CXL_PCI_H__ */
>> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
>> index 915102f5113f..0a29321921a0 100644
>> --- a/drivers/cxl/pci.c
>> +++ b/drivers/cxl/pci.c
>> @@ -1064,12 +1064,28 @@ static void cxl_handle_cper_event(enum cxl_event_type ev_type,
>> &uuid_null, &rec->event);
>> }
>>
>> +static void cxl_handle_prot_err(struct cxl_cper_prot_err *rec)
>> +{
>> + struct cxl_dev_state *cxlds;
>> +
>> + cxlds = get_cxl_devstate(rec->segment, rec->bus,
>> + rec->device, rec->function);
>
> As mentioned in 2/4 I don't think this is going to work correctly.
>
>> + if (!cxlds)
>> + return;
>> +
>> + cxl_trace_prot_err(cxlds, rec);
>> +}
>> +
>> static void cxl_cper_work_fn(struct work_struct *work)
>> {
>> struct cxl_cper_work_data wd;
>>
>> - while (cxl_cper_kfifo_get(&wd))
>> - cxl_handle_cper_event(wd.event_type, &wd.rec);
>> + while (cxl_cper_kfifo_get(&wd)) {
>> + if (wd.event_type == CXL_CPER_EVENT_PROT_ERR)
>> + cxl_handle_prot_err(&wd.p_rec);
>> + else
>> + cxl_handle_cper_event(wd.event_type, &wd.rec);
>
> Why does this need to change?
>
> Have cxl_handle_cper_event() decode the BDF as it does now and call
> cxl_handle_prot_err() there? I think you could drop patch 2/4 entirely
> and not have to duplicate the logic.
>
> Ira
Yeah works for me. My intention was to not disturb the
cxl_handle_cper_event() as that was focussed on handling device errors.
Thanks
Smita
>
>> + }
>> }
>> static DECLARE_WORK(cxl_cper_work, cxl_cper_work_fn);
>>
>> diff --git a/include/cxl/event.h b/include/cxl/event.h
>> index 5b316150556a..d854d8c435db 100644
>> --- a/include/cxl/event.h
>> +++ b/include/cxl/event.h
>> @@ -115,6 +115,7 @@ struct cxl_event_record_raw {
>> } __packed;
>>
>> enum cxl_event_type {
>> + CXL_CPER_EVENT_PROT_ERR,
>> CXL_CPER_EVENT_GENERIC,
>> CXL_CPER_EVENT_GEN_MEDIA,
>> CXL_CPER_EVENT_DRAM,
>> --
>> 2.17.1
>>
>
>
Powered by blists - more mailing lists