[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 7/7] hw/mem/cxl_type3: Add CXL RAS Error Injection Support.
From: |
Jonathan Cameron |
Subject: |
Re: [PATCH 7/7] hw/mem/cxl_type3: Add CXL RAS Error Injection Support. |
Date: |
Mon, 16 Jan 2023 11:04:02 +0000 |
Hi Mike,
> > +static int ct3d_qmp_uncor_err_to_cxl(CxlUncorErrorType qmp_err)
> > +{
> > + switch (qmp_err) {
> > + case CXL_UNCOR_ERROR_TYPE_CACHE_DATA_PARITY:
> > + return CXL_RAS_UNC_ERR_CACHE_DATA_PARITY;
> > + case CXL_UNCOR_ERROR_TYPE_CACHE_ADDRESS_PARITY:
> > + return CXL_RAS_UNC_ERR_CACHE_ADDRESS_PARITY;
> > + case CXL_UNCOR_ERROR_TYPE_CACHE_BE_PARITY:
> > + return CXL_RAS_UNC_ERR_CACHE_BE_PARITY;
> > + case CXL_UNCOR_ERROR_TYPE_CACHE_DATA_ECC:
> > + return CXL_RAS_UNC_ERR_CACHE_DATA_ECC;
> > + case CXL_UNCOR_ERROR_TYPE_MEM_DATA_PARITY:
> > + return CXL_RAS_UNC_ERR_MEM_DATA_PARITY;
> > + case CXL_UNCOR_ERROR_TYPE_MEM_ADDRESS_PARITY:
> > + return CXL_RAS_UNC_ERR_MEM_ADDRESS_PARITY;
> > + case CXL_UNCOR_ERROR_TYPE_MEM_BE_PARITY:
> > + return CXL_RAS_UNC_ERR_MEM_BE_PARITY;
> > + case CXL_UNCOR_ERROR_TYPE_MEM_DATA_ECC:
> > + return CXL_RAS_UNC_ERR_MEM_DATA_ECC;
> > + case CXL_UNCOR_ERROR_TYPE_REINIT_THRESHOLD:
> > + return CXL_RAS_UNC_ERR_REINIT_THRESHOLD;
> > + case CXL_UNCOR_ERROR_TYPE_RSVD_ENCODING:
> > + return CXL_RAS_UNC_ERR_RSVD_ENCODING;
> > + case CXL_UNCOR_ERROR_TYPE_POISON_RECEIVED:
> > + return CXL_RAS_UNC_ERR_POISON_RECEIVED;
> > + case CXL_UNCOR_ERROR_TYPE_RECEIVER_OVERFLOW:
> > + return CXL_RAS_UNC_ERR_RECEIVER_OVERFLOW;
> > + case CXL_UNCOR_ERROR_TYPE_INTERNAL:
> > + return CXL_RAS_UNC_ERR_INTERNAL;
> > + case CXL_UNCOR_ERROR_TYPE_CXL_IDE_TX:
> > + return CXL_RAS_UNC_ERR_CXL_IDE_TX;
> > + case CXL_UNCOR_ERROR_TYPE_CXL_IDE_RX:
> > + return CXL_RAS_UNC_ERR_CXL_IDE_RX;
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> > +static int ct3d_qmp_cor_err_to_cxl(CxlUncorErrorType qmp_err)
>
> CxlCorErrorType type is required.
>
> Compiler warns here:
> ../hw/mem/cxl_type3.c:1263:44: error: implicit conversion from
> enumeration type 'CxlCorErrorType' (aka 'enum CxlCorErrorType') to
> different enumeration type 'CxlUncorErrorType' (aka 'enum
> CxlUncorErrorType') [-Werror,-Wenum-conversion]
>
> cxl_err_type = ct3d_qmp_cor_err_to_cxl(type);
>
> ~~~~~~~~~~~~~~~~~~~~~~~ ^~~~
> 1 error generated.
Yikes. Not sure how I missed that!
>
> > +{
> > + switch (qmp_err) {
> > + case CXL_COR_ERROR_TYPE_CACHE_DATA_ECC:
> > + return CXL_RAS_COR_ERR_CACHE_DATA_ECC;
> > + case CXL_COR_ERROR_TYPE_MEM_DATA_ECC:
> > + return CXL_RAS_COR_ERR_MEM_DATA_ECC;
> > + case CXL_COR_ERROR_TYPE_CRC_THRESHOLD:
> > + return CXL_RAS_COR_ERR_CRC_THRESHOLD;
> > + case CXL_COR_ERROR_TYPE_RETRY_THRESHOLD:
> > + return CXL_RAS_COR_ERR_RETRY_THRESHOLD;
> > + case CXL_COR_ERROR_TYPE_CACHE_POISON_RECEIVED:
> > + return CXL_RAS_COR_ERR_CACHE_POISON_RECEIVED;
> > + case CXL_COR_ERROR_TYPE_MEM_POISON_RECEIVED:
> > + return CXL_RAS_COR_ERR_MEM_POISON_RECEIVED;
> > + case CXL_COR_ERROR_TYPE_PHYSICAL:
> > + return CXL_RAS_COR_ERR_PHYSICAL;
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> > static void ct3d_reg_write(void *opaque, hwaddr offset, uint64_t value,
> > unsigned size)
> > {
> > @@ -341,6 +402,84 @@ static void ct3d_reg_write(void *opaque, hwaddr
> > offset, uint64_t value,
> > should_commit = FIELD_EX32(value, CXL_HDM_DECODER0_CTRL, COMMIT);
> > which_hdm = 0;
> > break;
> > + case A_CXL_RAS_UNC_ERR_STATUS:
> > + {
> > + uint32_t capctrl = ldl_le_p(cache_mem + R_CXL_RAS_ERR_CAP_CTRL);
> > + uint32_t fe = FIELD_EX32(capctrl, CXL_RAS_ERR_CAP_CTRL,
> > FIRST_ERROR_POINTER);
> > + CXLError *cxl_err;
> > + uint32_t unc_err;
> > +
> > + /*
> > + * If single bit written that corresponds to the first error
> > + * pointer being cleared, update the status and header log.
> > + */
> > + if (!QTAILQ_EMPTY(&ct3d->error_list)) {
> > + CXLError *cxl_err = QTAILQ_FIRST(&ct3d->error_list);
>
> Is it ok that "CXLError *cxl_err" definition clobbers previous one above?
It isn't a bug as the external one is only used much later in a QTAILQ_FOREACH()
to build the resulting error status register, but it's certainly inelegant
and there is no need for the internal definition so I'll drop it.
I also moved the assignment to the else leg which is the only place that
specific assignment is used.
Thanks for quick review! I'll hold off sending a v2 out for a day or two
to let any other early comments come in.
Jonathan
- [PATCH 0/7] hw/cxl: RAS error emulation and injection, Jonathan Cameron, 2023/01/13
- [PATCH 1/7] hw/pci/aer: Implement PCI_ERR_UNCOR_MASK register, Jonathan Cameron, 2023/01/13
- [PATCH 2/7] hw/pci/aer: Add missing routing for AER errors, Jonathan Cameron, 2023/01/13
- [PATCH 3/7] hw/pci-bridge/cxl_root_port: Wire up AER, Jonathan Cameron, 2023/01/13
- [PATCH 4/7] hw/pci-bridge/cxl_root_port: Wire up MSI, Jonathan Cameron, 2023/01/13
- [PATCH 5/7] hw/mem/cxl-type3: Add AER extended capability, Jonathan Cameron, 2023/01/13
- [PATCH 6/7] hw/pci/aer: Make PCIE AER error injection facility available for other emulation to use., Jonathan Cameron, 2023/01/13
- [PATCH 7/7] hw/mem/cxl_type3: Add CXL RAS Error Injection Support., Jonathan Cameron, 2023/01/13