Message-ID: <20251114160945.0000727d@huawei.com>
Date: Fri, 14 Nov 2025 16:09:45 +0000
From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: "Bowman, Terry" <terry.bowman@....com>
CC: Bjorn Helgaas <helgaas@...nel.org>, <dave@...olabs.net>,
	<dave.jiang@...el.com>, <alison.schofield@...el.com>,
	<dan.j.williams@...el.com>, <bhelgaas@...gle.com>, <shiju.jose@...wei.com>,
	<ming.li@...omail.com>, <Smita.KoralahalliChannabasappa@....com>,
	<rrichter@....com>, <dan.carpenter@...aro.org>,
	<PradeepVineshReddy.Kodamati@....com>, <lukas@...ner.de>,
	<Benjamin.Cheatham@....com>, <sathyanarayanan.kuppuswamy@...ux.intel.com>,
	<linux-cxl@...r.kernel.org>, <alucerop@....com>, <ira.weiny@...el.com>,
	<linux-kernel@...r.kernel.org>, <linux-pci@...r.kernel.org>
Subject: Re: [RESEND v13 22/25] CXL/PCI: Export and rename merge_result() to
 pci_ers_merge_result()

On Fri, 14 Nov 2025 09:20:08 -0600
"Bowman, Terry" <terry.bowman@....com> wrote:

> On 11/4/2025 1:03 PM, Bjorn Helgaas wrote:
> > On Tue, Nov 04, 2025 at 11:03:02AM -0600, Terry Bowman wrote:  
> >> CXL uncorrectable errors (UCE) will soon be handled separately from the PCI
> >> AER handling. The merge_result() function can be made common to use in both
> >> handling paths.
> >>
> >> Rename the PCI subsystem's merge_result() to be pci_ers_merge_result().
> >> Export pci_ers_merge_result() to make available for the CXL and other
> >> drivers to use.
> >>
> >> Update pci_ers_merge_result() to support recently introduced PCI_ERS_RESULT_PANIC
> >> result.  
> > Seems like this merge_result() change maybe should be in the same
> > patch that added PCI_ERS_RESULT_PANIC?  That would also solve the
> > problem that the subject line doesn't mention this important
> > functional change.
> >
> > I haven't seen the user(s) of pci_ers_merge_result() yet, but this
> > seems like it might be a little too low level to be exported to
> > modules and in include/linux/pci.h.  Maybe there's no other way.  
> 
> This is used in the UCE handling patch. I will move there.
> 
> Jonathan suggested updating |merge_result()| to handle both PCIe and CXL error 
> cases with shared logic. The only other option I see is to remove the export here 
> and duplicate the function in the CXL drivers?

I don't mind if turns out we do need to duplicate this little bit of code.

Jonathan
> /
> /
> 
> - Terry
> 
> 
> > Wrap commit log to fit in 75 columns.
> >
> > Suggest possible subject prefix of "PCI/ERR" since the only CXL
> > connection is that you want to *use* this from CXL.
> >  
> >> Signed-off-by: Terry Bowman <terry.bowman@....com>
> >> --- Changes in v12->v13: - Renamed pci_ers_merge_result() to pcie_ers_merge_result(). pci_ers_merge_result() is already used in eeh driver. (Bot) Changes in v11->v12: - Remove static inline pci_ers_merge_result() definition for !CONFIG_PCIEAER. Is not needed. (Lukas) Changes in v10->v11: - New patch - pci_ers_merge_result() - Change export to non-namespace and rename to be pci_ers_merge_result() - Move pci_ers_merge_result() definition to pci.h. Needs pci_ers_result --- drivers/pci/pcie/err.c | 14 +++++++++----- include/linux/pci.h | 7 +++++++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c index bebe4bc111d7..9394bbdcf0fb 100644 --- a/drivers/pci/pcie/err.c +++ b/drivers/pci/pcie/err.c @@ -21,9 +21,12 @@ #include "portdrv.h" #include "../pci.h" -static pci_ers_result_t merge_result(enum pci_ers_result orig, - enum pci_ers_result new) +pci_ers_result_t pcie_ers_merge_result(enum pci_ers_result orig, + enum
> >> pci_ers_result new) { + if (new == PCI_ERS_RESULT_PANIC) + return PCI_ERS_RESULT_PANIC; + if (new == PCI_ERS_RESULT_NO_AER_DRIVER) return PCI_ERS_RESULT_NO_AER_DRIVER; @@ -45,6 +48,7 @@ static pci_ers_result_t merge_result(enum pci_ers_result orig, return orig; } +EXPORT_SYMBOL(pcie_ers_merge_result); static int report_error_detected(struct pci_dev *dev, pci_channel_state_t state, @@ -81,7 +85,7 @@ static int report_error_detected(struct pci_dev *dev, vote = err_handler->error_detected(dev, state); } pci_uevent_ers(dev, vote); - *result = merge_result(*result, vote); + *result = pcie_ers_merge_result(*result, vote); device_unlock(&dev->dev); return 0; } @@ -139,7 +143,7 @@ static int report_mmio_enabled(struct pci_dev *dev, void *data) err_handler = pdrv->err_handler; vote = err_handler->mmio_enabled(dev); - *result = merge_result(*result, vote); + *result = pcie_ers_merge_result(*result, vote); out: device_unlock(&dev->dev); return 0; @@ -159,7 +163,7 @@ static int
> >> report_slot_reset(struct pci_dev *dev, void *data) err_handler = pdrv->err_handler; vote = err_handler->slot_reset(dev); - *result = merge_result(*result, vote); + *result = pcie_ers_merge_result(*result, vote); out: device_unlock(&dev->dev); return 0; diff --git a/include/linux/pci.h b/include/linux/pci.h index 33d16b212e0d..d3e3300f79ec 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1887,9 +1887,16 @@ static inline void pci_hp_unignore_link_change(struct pci_dev *pdev) { } #ifdef CONFIG_PCIEAER bool pci_aer_available(void); void pcie_clear_device_status(struct pci_dev *dev); +pci_ers_result_t pcie_ers_merge_result(enum pci_ers_result orig, + enum pci_ers_result new); #else static inline bool pci_aer_available(void) { return false; } static inline void pcie_clear_device_status(struct pci_dev *dev) { } +static inline pci_ers_result_t pcie_ers_merge_result(enum pci_ers_result orig, + enum pci_ers_result new) +{ + return PCI_ERS_RESULT_NONE; +} #endif bool
> >> pci_ats_disabled(void); -- 2.34.1   
> 

Powered by blists - more mailing lists