Re: [PATCH v4 09/11] cxl/pci: Add (hopeful) error handling support

[Dave Jiang <dave.jiang@intel.com>]

On 1/6/23 9:05 AM, Jonathan Cameron wrote:
> On Tue, 29 Nov 2022 10:48:59 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
>> From: Dan Williams <dan.j.williams@intel.com>
>>
>> Add nominal error handling that tears down CXL.mem in response to error
>> notifications that imply a device reset. Given some CXL.mem may be
>> operating as System RAM, there is a high likelihood that these error
>> events are fatal. However, if the system survives the notification the
>> expectation is that the driver behavior is equivalent to a hot-unplug
>> and re-plug of an endpoint.
>>
>> Note that this does not change the mask values from the default. That
>> awaits CXL _OSC support to determine whether platform firmware is in
>> control of the mask registers.
>>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
> I've been messing around with improving the qemu injection to do multiple
> errors and ran into a bug...
> 
> I'll send a patch next week, but in meantime...
> 
> 
>> ---
> 
>> +/*
>> + * Log the state of the RAS status registers and prepare them to log the
>> + * next error status. Return 1 if reset needed.
>> + */
>> +static bool cxl_report_and_clear(struct cxl_dev_state *cxlds)
>> +{
>> +	struct cxl_memdev *cxlmd = cxlds->cxlmd;
>> +	struct device *dev = &cxlmd->dev;
>> +	u32 hl[CXL_HEADERLOG_SIZE_U32];
>> +	void __iomem *addr;
>> +	u32 status;
>> +	u32 fe;
>> +
>> +	if (!cxlds->regs.ras)
>> +		return false;
>> +
>> +	addr = cxlds->regs.ras + CXL_RAS_UNCORRECTABLE_STATUS_OFFSET;
>> +	status = le32_to_cpu((__force __le32)readl(addr));
>> +	if (!(status & CXL_RAS_UNCORRECTABLE_STATUS_MASK))
>> +		return false;
>> +
>> +	/* If multiple errors, log header points to first error from ctrl reg */
>> +	if (hweight32(status) > 1) {
>> +		addr = cxlds->regs.ras + CXL_RAS_CAP_CONTROL_OFFSET;
>> +		fe = BIT(le32_to_cpu((__force __le32)readl(addr)) &
>> +				     CXL_RAS_CAP_CONTROL_FE_MASK);
>> +	} else {
>> +		fe = status;
>> +	}
>> +
>> +	header_log_copy(cxlds, hl);
>> +	trace_cxl_aer_uncorrectable_error(dev_name(dev), status, fe, hl);
>> +	writel(status & CXL_RAS_UNCORRECTABLE_STATUS_MASK, addr);
> 
> This address is meant to be that of the CXL_RAS_UNCORRECTABLE_STATUS register
> but in the event hweight32(status) > 1 it's been ovewritten with the
> address of CXL_RAS_CAP_CONTROL.

Great catch! I'll send out a fix.

> 
> 
>> +
>> +	return true;
>> +}
>> +