Re: [PATCH v9] cxl: add RAS status unmasking for CXL

[Dan Williams <dan.j.williams@intel.com>]

Smita Koralahalli wrote:
> Hi all,
> 
> I understand this has been in upstream already. But I have a slight 
> confusion on one of the checks been done here.
> 
> On 2/21/2023 9:55 AM, Dave Jiang wrote:
> > By default the CXL RAS mask registers bits are defaulted to 1's and
> > suppress all error reporting. If the kernel has negotiated ownership
> > of error handling for CXL then unmask the mask registers by writing 0s.
> > 
> > PCI_EXP_DEVCTL capability is checked to see uncorrectable or correctable
> > errors bits are set before unmasking the respective errors.
> > 
> > Acked-by: Bjorn Helgaas <bhelgaas@google.com>  # pci_regs.h
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> > 
> > ---
> 
> > +static int cxl_pci_ras_unmask(struct pci_dev *pdev)
> > +{
> > +	struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus);
> > +	struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
> > +	void __iomem *addr;
> > +	u32 orig_val, val, mask;
> > +	u16 cap;
> > +	int rc;
> > +
> > +	if (!cxlds->regs.ras) {
> > +		dev_dbg(&pdev->dev, "No RAS registers.\n");
> > +		return 0;
> > +	}
> > +
> > +	/* BIOS has CXL error control */
> > +	if (!host_bridge->native_cxl_error)
> > +		return -EOPNOTSUPP;
> 
> Why are we checking for native_cxl_error (native_cxl_error is CXL Memory 
> Error Reporting Control _OSC bit..) while unmasking RAS status?
> 
> RAS registers will be reported on a protocol error and the protocol 
> error follows the PCIe AER. Should we check for AER _OSC instead of CXL 
> Memory Error _OSC?
> 
> Because atleast on AMD systems we log RAS registers only on Protocol 
> errors and we use this CXL Memory _OSC knob to report component errors. 
> Is it same across everywhere? And there might be cases where protocol 
> error reporting might be native (PCIe AER) and component/memory can be 
> FW-First which fails this check..

I think that's reasonable, it just was not clear from the specification
that CXL protocol errors are included in PCIe AER as far as _OSC is
concerned because they are conveyed as "internal" errors.

So I believe it was an "abundance of caution" more than a requirement
that Linux expects control of memory-errors before proceeding to touch
the CXL RAS registers.