linux-kernel - Re: [PATCH v2 4/4] acpi/ghes, cxl/pci: Trace FW-First 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]

Message-ID: <1dd9cb74-5b88-06ca-81d2-97392fe14ce9@amd.com>
Date: Thu, 3 Oct 2024 13:03:00 -0700
From: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>
To: Dan Williams <dan.j.williams@...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>, Ira Weiny <ira.weiny@...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/2/2024 5:16 PM, Dan Williams 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);
> 
> The cxl_cper_work item is only for cases where the cxl_pci driver might care
> about annotating an error report with driver specific details like the
> impacted kernel object name, 'struct cxl_memdev', or address translation
> for DPA data.
> 
> Protocol errors that are not endpoint errors should never be placed in
> the cxl_cper_fifo. That is exclusively for errors that cxl_pci needs to
> consume.
> 
> My expectation is that similar to aer_recover_queue for PCIe protocol
> errors CXL needs to grow a cxl_recover_queue that at a minimum triggers
> new trace events to dump these records to RAS daemon.
> 
> I am struggling to think what useful information cxl_pci could ever
> append to a protocol error event.
> 
> What is more likely is that later when Terry adds port error handling a
> CPER protocol error record could trigger a new cxl_do_recovery() to
> react to CXL topology errors that might impact downstream CXL devices.
> In that case the notification will come through something like a new
> 'struct cxl_error_handlers *' hanging off 'struct pci_driver' since
> accelerator drivers are going to have distinct error handling from
> generic memory expanders.

Hmm, I agree handling device and protocol errors separately. Unless 
Terry has something to add here, I can start working on a minimal 
version of cxl_recover_queue as of now.

Thanks
Smita
>