[PATCH v12 14/20] cxl/pci: Map RCH downstream AER registers for logging protocol errors

Robert Richter posted 20 patches 2 years, 2 months ago
[PATCH v12 14/20] cxl/pci: Map RCH downstream AER registers for logging protocol errors
Posted by Robert Richter 2 years, 2 months ago
From: Terry Bowman <terry.bowman@amd.com>

The restricted CXL host (RCH) error handler will log protocol errors
using AER and RAS status registers. The AER and RAS registers need to
be virtually memory mapped before enabling interrupts. Create the
initializer function devm_cxl_setup_parent_dport() for this when the
endpoint is connected with the dport. The initialization sets up the
RCH RAS and AER mappings.

Add 'struct cxl_regs' to 'struct cxl_dport' for saving a pointer to
the RCH downstream port's AER and RAS registers.

Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Co-developed-by: Robert Richter <rrichter@amd.com>
Signed-off-by: Robert Richter <rrichter@amd.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/cxl/core/pci.c | 36 ++++++++++++++++++++++++++++++++++++
 drivers/cxl/cxl.h      | 10 ++++++++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index d101fdafb07c..3b4bb8d05035 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -5,6 +5,7 @@
 #include <linux/delay.h>
 #include <linux/pci.h>
 #include <linux/pci-doe.h>
+#include <linux/aer.h>
 #include <cxlpci.h>
 #include <cxlmem.h>
 #include <cxl.h>
@@ -730,6 +731,38 @@ static bool cxl_handle_endpoint_ras(struct cxl_dev_state *cxlds)
 
 #ifdef CONFIG_PCIEAER_CXL
 
+static void cxl_dport_map_rch_aer(struct cxl_dport *dport)
+{
+	struct cxl_rcrb_info *ri = &dport->rcrb;
+	void __iomem *dport_aer = NULL;
+	resource_size_t aer_phys;
+	struct device *host;
+
+	if (dport->rch && ri->aer_cap) {
+		host = dport->reg_map.host;
+		aer_phys = ri->aer_cap + ri->base;
+		dport_aer = devm_cxl_iomap_block(host, aer_phys,
+				sizeof(struct aer_capability_regs));
+	}
+
+	dport->regs.dport_aer = dport_aer;
+}
+
+static void cxl_dport_map_regs(struct cxl_dport *dport)
+{
+	struct cxl_register_map *map = &dport->reg_map;
+	struct device *dev = dport->dport_dev;
+
+	if (!map->component_map.ras.valid)
+		dev_dbg(dev, "RAS registers not found\n");
+	else if (cxl_map_component_regs(map, &dport->regs.component,
+					BIT(CXL_CM_CAP_CAP_ID_RAS)))
+		dev_dbg(dev, "Failed to map RAS capability.\n");
+
+	if (dport->rch)
+		cxl_dport_map_rch_aer(dport);
+}
+
 void cxl_setup_parent_dport(struct device *host, struct cxl_dport *dport)
 {
 	struct device *dport_dev = dport->dport_dev;
@@ -738,6 +771,9 @@ void cxl_setup_parent_dport(struct device *host, struct cxl_dport *dport)
 	host_bridge = to_pci_host_bridge(dport_dev);
 	if (host_bridge->native_cxl_error)
 		dport->rcrb.aer_cap = cxl_rcrb_to_aer(dport_dev, dport->rcrb.base);
+
+	dport->reg_map.host = host;
+	cxl_dport_map_regs(dport);
 }
 EXPORT_SYMBOL_NS_GPL(cxl_setup_parent_dport, CXL);
 
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index cdb2ade6ba29..3b09286d9d52 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -221,6 +221,14 @@ struct cxl_regs {
 	struct_group_tagged(cxl_pmu_regs, pmu_regs,
 		void __iomem *pmu;
 	);
+
+	/*
+	 * RCH downstream port specific RAS register
+	 * @aer: CXL 3.0 8.2.1.1 RCH Downstream Port RCRB
+	 */
+	struct_group_tagged(cxl_rch_regs, rch_regs,
+		void __iomem *dport_aer;
+	);
 };
 
 struct cxl_reg_map {
@@ -623,6 +631,7 @@ struct cxl_rcrb_info {
  * @rcrb: Data about the Root Complex Register Block layout
  * @rch: Indicate whether this dport was enumerated in RCH or VH mode
  * @port: reference to cxl_port that contains this downstream port
+ * @regs: Dport parsed register blocks
  */
 struct cxl_dport {
 	struct device *dport_dev;
@@ -631,6 +640,7 @@ struct cxl_dport {
 	struct cxl_rcrb_info rcrb;
 	bool rch;
 	struct cxl_port *port;
+	struct cxl_regs regs;
 };
 
 /**
-- 
2.30.2
RE: [PATCH v12 14/20] cxl/pci: Map RCH downstream AER registers for logging protocol errors
Posted by Dan Williams 2 years, 1 month ago
Robert Richter wrote:
> From: Terry Bowman <terry.bowman@amd.com>
> 
> The restricted CXL host (RCH) error handler will log protocol errors
> using AER and RAS status registers. The AER and RAS registers need to
> be virtually memory mapped before enabling interrupts. Create the
> initializer function devm_cxl_setup_parent_dport() for this when the
> endpoint is connected with the dport. The initialization sets up the
> RCH RAS and AER mappings.
> 
> Add 'struct cxl_regs' to 'struct cxl_dport' for saving a pointer to
> the RCH downstream port's AER and RAS registers.
> 
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Co-developed-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Robert Richter <rrichter@amd.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/cxl/core/pci.c | 36 ++++++++++++++++++++++++++++++++++++
>  drivers/cxl/cxl.h      | 10 ++++++++++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index d101fdafb07c..3b4bb8d05035 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -5,6 +5,7 @@
>  #include <linux/delay.h>
>  #include <linux/pci.h>
>  #include <linux/pci-doe.h>
> +#include <linux/aer.h>
>  #include <cxlpci.h>
>  #include <cxlmem.h>
>  #include <cxl.h>
> @@ -730,6 +731,38 @@ static bool cxl_handle_endpoint_ras(struct cxl_dev_state *cxlds)
>  
>  #ifdef CONFIG_PCIEAER_CXL

Here is more code in an ifdef block that has no compile time dependency
on the config symbol. Please do not use ifdef blocks for runtime
dependencies.

Again, this will need to be a post -rc1 cleanup.
RE: [PATCH v12 14/20] cxl/pci: Map RCH downstream AER registers for logging protocol errors
Posted by Dan Williams 2 years, 1 month ago
Dan Williams wrote:
> Robert Richter wrote:
> > @@ -730,6 +731,38 @@ static bool cxl_handle_endpoint_ras(struct cxl_dev_state *cxlds)
> >  
> >  #ifdef CONFIG_PCIEAER_CXL
> 
> Here is more code in an ifdef block that has no compile time dependency
> on the config symbol. Please do not use ifdef blocks for runtime
> dependencies.
> 
> Again, this will need to be a post -rc1 cleanup.

Here is that patch:

-- >8 --
Subject: PCI/AER: Increase compile coverage of CONFIG_PCIEAER_CXL implementation

From: Dan Williams <dan.j.williams@intel.com>

Per coding-style, avoid usage of conditional compilation for
CONFIG_PCIEAER_CXL related helpers. Instead use
IS_ENABLED(CONFIG_PCIEAER_CXL) to check when CXL error handling is
compile-time disabled.

Cc: Terry Bowman <terry.bowman@amd.com>
Cc: Robert Richter <rrichter@amd.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/core/pci.c |    9 +++------
 drivers/cxl/cxl.h      |    6 ------
 drivers/pci/pcie/aer.c |   15 +++++----------
 3 files changed, 8 insertions(+), 22 deletions(-)

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 3da195caa4ad..2d7ba1899ea2 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -729,8 +729,6 @@ static bool cxl_handle_endpoint_ras(struct cxl_dev_state *cxlds)
 	return __cxl_handle_ras(cxlds, cxlds->regs.ras);
 }
 
-#ifdef CONFIG_PCIEAER_CXL
-
 static void cxl_dport_map_rch_aer(struct cxl_dport *dport)
 {
 	struct cxl_rcrb_info *ri = &dport->rcrb;
@@ -797,6 +795,9 @@ void cxl_setup_parent_dport(struct device *host, struct cxl_dport *dport)
 	struct device *dport_dev = dport->dport_dev;
 	struct pci_host_bridge *host_bridge;
 
+	if (!IS_ENABLED(CONFIG_PCIEAER_CXL))
+		return;
+
 	host_bridge = to_pci_host_bridge(dport_dev);
 	if (host_bridge->native_cxl_error)
 		dport->rcrb.aer_cap = cxl_rcrb_to_aer(dport_dev, dport->rcrb.base);
@@ -897,10 +898,6 @@ static void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
 		cxl_handle_rdport_ras(cxlds, dport);
 }
 
-#else
-static void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds) { }
-#endif
-
 void cxl_cor_error_detected(struct pci_dev *pdev)
 {
 	struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 378fc96ff7ff..0bf0d13346c2 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -712,13 +712,7 @@ struct cxl_dport *devm_cxl_add_dport(struct cxl_port *port,
 struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
 					 struct device *dport_dev, int port_id,
 					 resource_size_t rcrb);
-
-#ifdef CONFIG_PCIEAER_CXL
 void cxl_setup_parent_dport(struct device *host, struct cxl_dport *dport);
-#else
-static inline void cxl_setup_parent_dport(struct device *host,
-					  struct cxl_dport *dport) { }
-#endif
 
 struct cxl_decoder *to_cxl_decoder(struct device *dev);
 struct cxl_root_decoder *to_cxl_root_decoder(struct device *dev);
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 41076cb2956e..36541bfab688 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -934,8 +934,6 @@ static bool find_source_device(struct pci_dev *parent,
 	return true;
 }
 
-#ifdef CONFIG_PCIEAER_CXL
-
 /**
  * pci_aer_unmask_internal_errors - unmask internal errors
  * @dev: pointer to the pcie_dev data structure
@@ -1000,7 +998,8 @@ static int cxl_rch_handle_error_iter(struct pci_dev *dev, void *data)
 	struct aer_err_info *info = (struct aer_err_info *)data;
 	const struct pci_error_handlers *err_handler;
 
-	if (!is_cxl_mem_dev(dev) || !cxl_error_is_native(dev))
+	if (!is_cxl_mem_dev(dev) || !cxl_error_is_native(dev) ||
+	    !IS_ENABLED(CONFIG_PCIEAER_CXL))
 		return 0;
 
 	/* protect dev->driver */
@@ -1041,7 +1040,9 @@ static int handles_cxl_error_iter(struct pci_dev *dev, void *data)
 	bool *handles_cxl = data;
 
 	if (!*handles_cxl)
-		*handles_cxl = is_cxl_mem_dev(dev) && cxl_error_is_native(dev);
+		*handles_cxl = is_cxl_mem_dev(dev) &&
+			       cxl_error_is_native(dev) &&
+			       IS_ENABLED(CONFIG_PCIEAER_CXL);
 
 	/* Non-zero terminates iteration */
 	return *handles_cxl;
@@ -1067,12 +1068,6 @@ static void cxl_rch_enable_rcec(struct pci_dev *rcec)
 	pci_info(rcec, "CXL: Internal errors unmasked");
 }
 
-#else
-static inline void cxl_rch_enable_rcec(struct pci_dev *dev) { }
-static inline void cxl_rch_handle_error(struct pci_dev *dev,
-					struct aer_err_info *info) { }
-#endif
-
 /**
  * pci_aer_handle_error - handle logging error into an event log
  * @dev: pointer to pci_dev data structure of error source device