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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <653c7eb29265c_244c8f2944b@dwillia2-xfh.jf.intel.com.notmuch>
Date:   Fri, 27 Oct 2023 20:23:30 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     Dan Williams <dan.j.williams@...el.com>,
        Robert Richter <rrichter@....com>,
        Davidlohr Bueso <dave@...olabs.net>,
        Jonathan Cameron <jonathan.cameron@...wei.com>,
        Dave Jiang <dave.jiang@...el.com>,
        "Alison Schofield" <alison.schofield@...el.com>,
        Vishal Verma <vishal.l.verma@...el.com>,
        Ira Weiny <ira.weiny@...el.com>,
        Ben Widawsky <bwidawsk@...nel.org>
CC:     <linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Terry Bowman <terry.bowman@....com>,
        Robert Richter <rrichter@....com>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>
Subject: RE: [PATCH v12 14/20] cxl/pci: Map RCH downstream AER registers for
 logging protocol errors

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@...el.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@....com>
Cc: Robert Richter <rrichter@....com>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>
Signed-off-by: Dan Williams <dan.j.williams@...el.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