Re: [PATCH RFC v2 9/9] cxl/pci: Add (hopeful) error handling support

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

On 10/20/2022 6:45 AM, Jonathan Cameron wrote:
> On Fri, 16 Sep 2022 16:11:45 -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.
>>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ...
>
>> +/* CXL spec rev3.0 8.2.4.16.1 */
>> +#define DATA_HEADER_SIZE 16
>> +#define FLIT_SIZE (64 + 2)
>> +static int header_log_setup(struct cxl_dev_state *cxlds, u32 fe, u8 *log)
>> +{
>> +	void __iomem *addr;
>> +
>> +	addr = cxlds->regs.ras + CXL_RAS_HEADER_LOG_OFFSET;
>> +
>> +	if (fe & CXL_RAS_UC_CACHE_DATA_PARITY || fe & CXL_RAS_UC_CACHE_ADDR_PARITY ||
>> +	    fe & CXL_RAS_UC_CACHE_BE_PARITY || fe & CXL_RAS_UC_CACHE_DATA_ECC ||
>> +	    fe & CXL_RAS_UC_MEM_DATA_PARITY || fe & CXL_RAS_UC_MEM_ADDR_PARITY ||
>> +	    fe & CXL_RAS_UC_MEM_BE_PARITY || fe & CXL_RAS_UC_MEM_DATA_ECC) {
>> +		memcpy_fromio(log, addr, DATA_HEADER_SIZE);
> I'd forgotten his gremlin.
>
> You can't use memcpy_fromio() because on some architectures it will issue 8 byte
> reads and 8.2.4.16.7 states that the log shall be accessed as aligned 4-byte
> quantities.
Ok I'll fix.
>
>> +		return DATA_HEADER_SIZE;
>> +	}
>> +
>> +	if (fe & CXL_RAS_UC_RSVD_ENCODE) {
>> +		memcpy_fromio(log, addr, FLIT_SIZE);
>> +		return FLIT_SIZE;
>> +	}
>> +
>> +	if (fe & CXL_RAS_UC_RECV_OVERFLOW) {
>> +		*log = readb(addr);
> Also not valid for same reason.  Do a 32bit read and mask out the bottom byte.

Will fix. Thanks.


>
> That was a pain to track down (and worst of all we hit the same thing for another
> bit of CXL last year I'd forgotten about it :(
>
>> +		return sizeof(u8);
>> +	}
>> +
>> +	return 0;
>> +}
>> +