[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c03b8d66-65d9-40fc-dd0a-1b8154e24f6e@amd.com>
Date: Tue, 28 Mar 2023 08:41:38 -0500
From: Terry Bowman <Terry.Bowman@....com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: alison.schofield@...el.com, vishal.l.verma@...el.com,
ira.weiny@...el.com, bwidawsk@...nel.org, dan.j.williams@...el.com,
dave.jiang@...el.com, Jonathan.Cameron@...wei.com,
linux-cxl@...r.kernel.org, rrichter@....com,
linux-kernel@...r.kernel.org, bhelgaas@...gle.com
Subject: Re: [PATCH v2 4/5] cxl/pci: Forward RCH downstream port-detected
errors to the CXL.mem dev handler
Hi Bjorn,
On 3/24/23 17:36, Bjorn Helgaas wrote:
> I'd call this a "PCI/AER: ..." patch since that's where all the
> changes are.
>
> On Thu, Mar 23, 2023 at 04:38:07PM -0500, Terry Bowman wrote:
>> From: Robert Richter <rrichter@....com>
>>
>> In RCD mode a CXL device (RCD) is exposed as an RCiEP, but CXL
>> downstream and upstream ports are not enumerated and not visible in
>> the PCIe hierarchy. Protocol and link errors are sent to an RCEC.
>
> "RCD" isn't a common term in drivers/pci; can you expand it once here?
>
>> Now, RCH downstream port-detected errors are signaled as internal AER
>> errors (UIE/CIE) with the RCEC's source ID. A CXL handler must then
>
> Similarly, "UIE" and "CIE" are new to drivers/pci; can you expand them
> before using? I assume Uncorrectable Internal Error (UIE) and
> Corrected Internal Error (CIE)? (Annoying that the PCIe spec uses
> "Correctable" in general, but "Corrected" for Internal Errors.)
>
>> inspect the error status in various CXL registers residing in the
>> dport's component register space (CXL RAS cap) or the dport's RCRB
>> (AER ext cap). [1]
>>
>> This patch connects errors showing up in the RCEC's error handler with
>
> "Connect errors ..." (we already know this text applies to *this
> patch*).
>
>> the CXL subsystem. Implement this by forwarding the error to all CXL
>> devices below the RCEC. Since the entire CXL device is controlled only
>> using PCIe Configuration Space of device 0, Function 0, only pass it
>> there [2]. These devices have the Memory Device class code set
>> (PCI_CLASS_MEMORY_CXL, 502h) and the existing cxl_pci driver can
>> implement the handler.
>
>> The CXL device driver is then responsible to
>> enable error reporting in the RCEC's AER cap
>
> I don't know exactly what you mean by "error reporting in the RCEC's
> AER cap", but IIUC, for non-Root Port devices, generation of ERR_COR/
> ERR_NONFATAL/ERR_FATAL messages is controlled by the Device Control
> register and should already be enabled by pci_aer_init().
>
> Maybe you mean setting AER mask/severity specifically for Internal
> Errors? I'm hoping to get as much of AER management as we can in the
> PCI core and out of drivers, so maybe we need a new PCI interface to
> do that.
>
> In any event, I assume this sort of configuration would be an
> enumeration-time thing, while *this* patch is a run-time thing, so
> maybe this information belongs with a different patch?
>
>> (esp. CIE and UIE) and to
>> inspect the dport's CXL registers in addition (CXL RAS cap and AER ext
>> cap).
>>
>> The reason for choosing this implementation is that a CXL RCEC device
>> is bound to the AER port driver, but the driver does not allow it to
>> register a custom specific handler to support CXL. Connecting the RCEC
>> hard-wired with a CXL handler does not work, as the CXL subsystem
>> might not be present all the time. The alternative to add an
>> implementation to the portdrv to allow the registration of a custom
>> RCEC error handler isn't worth doing it as CXL would be its only user.
>> Instead, just check for an CXL RCEC and pass it down to the connected
>> CXL device's error handler.
>>
>> [1] CXL 3.0 spec, 12.2.1.1 RCH Downstream Port-detected Errors
>> [2] CXL 3.0 spec, 8.1.3 PCIe DVSEC for CXL Devices
>>
>> Co-developed-by: Terry Bowman <terry.bowman@....com>
>> Signed-off-by: Terry Bowman <terry.bowman@....com>
>> Signed-off-by: Robert Richter <rrichter@....com>
>
> Since you're sending this patch (Terry) your Signed-off-by should be
> last.
>
I'll move my Signed-off-by to the last.
Regards,
Terry
Powered by blists - more mailing lists