Re: [PATCH v13 08/25] CXL/AER: Move AER drivers RCH error handling into pcie/aer_cxl_rch.c
From: Bjorn Helgaas
Date: Wed Dec 10 2025 - 18:13:09 EST
On Wed, Dec 10, 2025 at 03:57:57PM -0600, Bowman, Terry wrote:
> On 12/8/2025 3:28 PM, Bowman, Terry wrote:
> > On 12/8/2025 12:06 PM, Bjorn Helgaas wrote:
> >> On Mon, Nov 03, 2025 at 06:09:44PM -0600, Terry Bowman wrote:
> >>> The restricted CXL Host (RCH) AER error handling logic currently resides
> >>> in the AER driver file, drivers/pci/pcie/aer.c. CXL specific changes are
> >>> conditionally compiled using #ifdefs.
> ...
> >>> Export pci_aer_unmask_internal_errors() allowing for all
> >>> subsystems to use.
> ...
> >>> @@ -1143,116 +1143,25 @@ static void pci_aer_unmask_internal_errors(struct pci_dev *dev)
> >>> mask &= ~PCI_ERR_COR_INTERNAL;
> >>> pci_write_config_dword(dev, aer + PCI_ERR_COR_MASK, mask);
> >>> }
> >>> +EXPORT_SYMBOL_GPL(pci_aer_unmask_internal_errors);
> >>
> >> Not sure why these EXPORTs are needed. Is there a caller that
> >> can be a module? The callers I see look like they would be
> >> builtin. If you add callers later that need this, the export can
> >> be done then.
> >
> > pci_aer_unmask_internal_errors() is called by the cxl_core module
> > later in the 2nd to-last patch. I'll move the export change to the
> > later patch. At one point I was trying to avoid changes to same
> > definitions multiple times.
> >
> >>> +++ b/include/linux/aer.h
> >>> @@ -56,12 +56,20 @@ struct aer_capability_regs {
> >>> #if defined(CONFIG_PCIEAER)
> >>> int pci_aer_clear_nonfatal_status(struct pci_dev *dev);
> >>> int pcie_aer_is_native(struct pci_dev *dev);
> >>> +void pci_aer_unmask_internal_errors(struct pci_dev *dev);
> >>> #else
> >>> static inline int pci_aer_clear_nonfatal_status(struct pci_dev *dev)
> >>> {
> >>> return -EINVAL;
> >>> }
> >>> static inline int pcie_aer_is_native(struct pci_dev *dev) { return 0; }
> >>> +static inline void pci_aer_unmask_internal_errors(struct pci_dev *dev) { }
> >> These include/linux/aer.h changes look like a separate patch.
> >> Moving code from aer.c to aer_cxl_rch.c doesn't add any callers
> >> outside drivers/pci, so these shouldn't need to be in
> >> include/linux/.
> >
> > I'll remove these from here.
>
> I reviewed this more closely and recalled the reasoning behind the
> change. Lukas requested that pci_aer_unmask_internal_errors() be
> made available across the entire kernel. I already noted this in the
> commit message, but I can also include a link to Lukas’s request.
> Alternatively, I could split this into a separate patch with a
> Recommended-by tag, leave it as is, or make another adjustment.
> Additionally, I’ll update cxl_error_is_native() so it’s only
> included when necessary.
There's a lot going on in this patch, which makes the commit log long
and a bit cluttered.
Let's make the pci_aer_unmask_internal_errors() change a separate
patch, combined with or immediately before adding a new user outside
drivers/pci.
I assume you're referring to Lukas's email at [1], but he's responding
to a patch that exported pci_aer_unmask_internal_errors() for modules
but left the declaration in drivers/pci/pci.h.
[1] https://lore.kernel.org/all/aK66OcdL4Meb0wFt@xxxxxxxxx/