Re: [PATCH v7 04/17] PCI/AER: Modify AER driver logging to report CXL or PCIe bus error type

[Dan Williams <dan.j.williams@xxxxxxxxx>]

Terry Bowman wrote:
> The AER driver and aer_event tracing currently log 'PCIe Bus Type'
> for all errors.
> 
> Update the driver and aer_event tracing to log 'CXL Bus Type' for CXL
> device errors.
> 
> Signed-off-by: Terry Bowman <terry.bowman@xxxxxxx>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>
> Reviewed-by: Fan Ni <fan.ni@xxxxxxxxxxx>
> Reviewed-by: Ira Weiny <ira.weiny@xxxxxxxxx>
> Reviewed-by: Gregory Price <gourry@xxxxxxxxxx>

> ---
>  drivers/pci/pcie/aer.c  | 14 ++++++++------
>  include/ras/ras_event.h |  9 ++++++---
>  2 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 6e8de77d0fc4..f99a1c6fb274 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -694,13 +694,14 @@ static void __aer_print_error(struct pci_dev *dev,
>  
>  void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
>  {
> +	const char *bus_type = pcie_is_cxl(dev) ? "CXL"  : "PCIe";

I was expecting that this would be more than just a CXL link check
because CXL AER events are only a subset of the events that can trigger
an AER interrupt on a CXL device.

Also, with CXL protocol errors the TLP log is sourced from CXL RAS
registers and is distinct from the PCIe ->tlp in 'struct aer_err_info'.

All that to say that I think this patch probably wants to decorate the
bus type in 'struct aer_err_info' and then use that rather than just the ->is_cxl
flag of the device.

In the interest of moving the patch set along perhaps just do something
like this for now and circle back to make it more sophisticated later:

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 01e51db8d285..eed098c134a6 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -533,6 +533,7 @@ static inline bool pci_dev_test_and_set_removed(struct pci_dev *dev)
 struct aer_err_info {
        struct pci_dev *dev[AER_MAX_MULTI_ERR_DEVICES];
        int error_dev_num;
+       bool is_cxl;
 
        unsigned int id:16;
 
@@ -549,6 +550,11 @@ struct aer_err_info {
        struct pcie_tlp_log tlp;        /* TLP Header */
 };
 
+static inline const char *aer_err_bus(struct aer_err_info *info)
+{
+       return info->is_cxl ? "CXL" : "PCIe";
+}
+
 int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info);
 void aer_print_error(struct pci_dev *dev, struct aer_err_info *info);
 
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 508474e17183..405f15c878ff 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1211,6 +1211,7 @@ int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
        /* Must reset in this function */
        info->status = 0;
        info->tlp_header_valid = 0;
+       info->is_cxl = dev->is_cxl;
 
        /* The device might not support AER */
        if (!aer)

Other than that, this patch looks good to me:

Reviewed-by: Dan Williams <dan.j.williams@xxxxxxxxx>