diff mbox series

[2/2] ACPI: extlog: Trace CPER PCI Express Error Section

Message ID 20240527144356.246220-3-fabio.m.de.francesco@linux.intel.com
State New
Headers show
Series Make ELOG log and trace consistently with GHES | expand

Commit Message

Fabio M. De Francesco May 27, 2024, 2:43 p.m. UTC
Currently, extlog_print() (ELOG) only reports CPER PCIe section (UEFI
v2.10, Appendix N.2.7) to the kernel log via print_extlog_rcd(). Instead,
the similar ghes_do_proc() (GHES) prints to kernel log and calls
pci_print_aer() to report via the ftrace infrastructure.

Add support to report the CPER PCIe Error section also via the ftrace
infrastructure by calling pci_print_aer() to make ELOG act consistently
with GHES.

Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Fabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>
---
 drivers/acpi/acpi_extlog.c | 30 ++++++++++++++++++++++++++++++
 drivers/pci/pcie/aer.c     |  2 +-
 include/linux/aer.h        | 13 +++++++++++--
 3 files changed, 42 insertions(+), 3 deletions(-)

Comments

Dan Williams Aug. 6, 2024, 7:56 p.m. UTC | #1
Fabio M. De Francesco wrote:
> Currently, extlog_print() (ELOG) only reports CPER PCIe section (UEFI
> v2.10, Appendix N.2.7) to the kernel log via print_extlog_rcd().

I think the critical detail is is that print_extlog_rcd() is only
triggered when ras_userspace_consumers() returns true. The observation
is that ras_userspace_consumers() hides information from the trace path
when the intended purpose of it was to hide duplicate emissions to the
kernel log when userspace is watching the tracepoints.

Setting aside whether ras_userspace_consumers() is still a good idea or
not, it is obvious that this patch as is may surprise environments that
start seeing kernel error logs where the kernel was silent before.

I think the path of least surprise would be to make sure that
pci_print_aer() optionally skips emitting to the kernel log when not
needed wanted.

So perhaps first do a lead-in patch to optionally quiet the print
messages in pci_print_aer() and then pass in KERN_DEBUG from the
extlog_print() path. Then we can decide later what to do about
ras_userspace_consumers().


> the similar ghes_do_proc() (GHES) prints to kernel log and calls
> pci_print_aer() to report via the ftrace infrastructure.
> 
> Add support to report the CPER PCIe Error section also via the ftrace
> infrastructure by calling pci_print_aer() to make ELOG act consistently
> with GHES.

You might also want to explain a bit about the motivation for this which
is that I/O Machine Check Arcitecture events may signal failing PCIe
components or links. The AER event contains details on what was
happening on the wire when the error was signaled.

> 
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Fabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>
> ---
>  drivers/acpi/acpi_extlog.c | 30 ++++++++++++++++++++++++++++++
>  drivers/pci/pcie/aer.c     |  2 +-
>  include/linux/aer.h        | 13 +++++++++++--
>  3 files changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index e025ae390737..007ce96f8672 100644
> --- a/drivers/acpi/acpi_extlog.c
> +++ b/drivers/acpi/acpi_extlog.c
> @@ -131,6 +131,32 @@ static int print_extlog_rcd(const char *pfx,
>  	return 1;
>  }
>  
> +static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
> +			      int severity)
> +{
> +	struct aer_capability_regs *aer;
> +	struct pci_dev *pdev;
> +	unsigned int devfn;
> +	unsigned int bus;
> +	int aer_severity;
> +	int domain;
> +
> +	if (pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID &&
> +	    pcie_err->validation_bits & CPER_PCIE_VALID_AER_INFO) {
> +		aer_severity = cper_severity_to_aer(severity);
> +		aer = (struct aer_capability_regs *)pcie_err->aer_info;
> +		domain = pcie_err->device_id.segment;
> +		bus = pcie_err->device_id.bus;
> +		devfn = PCI_DEVFN(pcie_err->device_id.device,
> +				  pcie_err->device_id.function);
> +		pdev = pci_get_domain_bus_and_slot(domain, bus, devfn);
> +		if (!pdev)
> +			return;
> +		pci_print_aer(pdev, aer_severity, aer);

...per above this would become:

    pci_print_aer(KERN_DEBUG, pdev, aer_severity, aer);

[..]

Rest of the changes look good to me.
Bjorn Helgaas Aug. 6, 2024, 9:31 p.m. UTC | #2
On Mon, May 27, 2024 at 04:43:41PM +0200, Fabio M. De Francesco wrote:
> Currently, extlog_print() (ELOG) only reports CPER PCIe section (UEFI
> v2.10, Appendix N.2.7) to the kernel log via print_extlog_rcd(). Instead,
> the similar ghes_do_proc() (GHES) prints to kernel log and calls
> pci_print_aer() to report via the ftrace infrastructure.
> 
> Add support to report the CPER PCIe Error section also via the ftrace
> infrastructure by calling pci_print_aer() to make ELOG act consistently
> with GHES.
> 
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Fabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>
> ---
>  drivers/acpi/acpi_extlog.c | 30 ++++++++++++++++++++++++++++++
>  drivers/pci/pcie/aer.c     |  2 +-
>  include/linux/aer.h        | 13 +++++++++++--
>  3 files changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index e025ae390737..007ce96f8672 100644
> --- a/drivers/acpi/acpi_extlog.c
> +++ b/drivers/acpi/acpi_extlog.c
> @@ -131,6 +131,32 @@ static int print_extlog_rcd(const char *pfx,
>  	return 1;
>  }
>  
> +static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
> +			      int severity)
> +{
> +	struct aer_capability_regs *aer;
> +	struct pci_dev *pdev;
> +	unsigned int devfn;
> +	unsigned int bus;
> +	int aer_severity;
> +	int domain;
> +
> +	if (pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID &&
> +	    pcie_err->validation_bits & CPER_PCIE_VALID_AER_INFO) {
> +		aer_severity = cper_severity_to_aer(severity);
> +		aer = (struct aer_capability_regs *)pcie_err->aer_info;
> +		domain = pcie_err->device_id.segment;
> +		bus = pcie_err->device_id.bus;
> +		devfn = PCI_DEVFN(pcie_err->device_id.device,
> +				  pcie_err->device_id.function);
> +		pdev = pci_get_domain_bus_and_slot(domain, bus, devfn);
> +		if (!pdev)
> +			return;
> +		pci_print_aer(pdev, aer_severity, aer);
> +		pci_dev_put(pdev);
> +	}

I'm 100% in favor of making error reporting work and look the same
across GHES and ELOG.  But I do have to gripe a bit...

It's already unfortunate that GHES and the native AER handling are
separate paths that lead to the same place (__aer_print_error()).

I'm sorry that we need to add a third path that again does
fundamentally the same thing.  The fact that they're separate means
all the design, reviewing, testing, and maintenance effort is diluted,
and error handling always gets too little love in the first place.
I think this is a recipe for confusion.

  ghes_do_proc                                        # GHES
    apei_estatus_for_each_section
      ...
      if (guid_equal(sec_type, &CPER_SEC_PCIE))
        ghes_handle_aer
          cper_severity_to_aer
          aer_recover_queue
            kfifo_in_spinlocked(&aer_recover_ring)    # add to queue
          aer_recover_work_func                       # another thread
            kfifo_get(&aer_recover_ring)              # remove from queue
            pci_print_aer
              __aer_print_error         <---

  aer_irq                                             # native AER
    kfifo_put(&aer_fifo)                              # add to queue
  aer_isr                                             # another thread
    kfifo_get(&aer_fifo)                              # remove from queue
    ...
    aer_isr_one_error
      aer_process_err_devices
        aer_print_error
          __aer_print_error             <---

  extlog_print                                        # extlog (x86 only)
    apei_estatus_for_each_section
      ...
      if (guid_equal(sec_type, &CPER_SEC_PCIE))
        extlog_print_pcie
          cper_severity_to_aer
          pci_get_domain_bus_and_slot
          pci_print_aer
            __aer_print_error           <---

And we also have CXL paths that lead to __aer_print_error(), although
it seems like they at least start in the native AER (and maybe GHES?)
path and branch out somewhere.  My head is spinning.

Do I *object* to this patch?  No, not really; it's a trivial change in
drivers/pci/, and Rafael can add my

  Acked-by: Bjorn Helgaas <bhelgaas@google.com>

as needed.  But I am afraid we're making ourselves a maintenance
headache.

> +}
> +
>  static int extlog_print(struct notifier_block *nb, unsigned long val,
>  			void *data)
>  {
> @@ -179,6 +205,10 @@ static int extlog_print(struct notifier_block *nb, unsigned long val,
>  			if (gdata->error_data_length >= sizeof(*mem))
>  				trace_extlog_mem_event(mem, err_seq, fru_id, fru_text,
>  						       (u8)gdata->error_severity);
> +		} else if (guid_equal(sec_type, &CPER_SEC_PCIE)) {
> +			struct cper_sec_pcie *pcie_err = acpi_hest_get_payload(gdata);
> +
> +			extlog_print_pcie(pcie_err, gdata->error_severity);
>  		} else {
>  			void *err = acpi_hest_get_payload(gdata);
>  
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index ac6293c24976..794aa15527ba 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -801,7 +801,7 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
>  	trace_aer_event(dev_name(&dev->dev), (status & ~mask),
>  			aer_severity, tlp_header_valid, &aer->header_log);
>  }
> -EXPORT_SYMBOL_NS_GPL(pci_print_aer, CXL);
> +EXPORT_SYMBOL_GPL(pci_print_aer);
>  
>  /**
>   * add_error_device - list device to be handled
> diff --git a/include/linux/aer.h b/include/linux/aer.h
> index 4b97f38f3fcf..fbc82206045c 100644
> --- a/include/linux/aer.h
> +++ b/include/linux/aer.h
> @@ -42,17 +42,26 @@ int pcie_read_tlp_log(struct pci_dev *dev, int where, struct pcie_tlp_log *log);
>  #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_print_aer(struct pci_dev *dev, int aer_severity,
> +		   struct aer_capability_regs *aer);
>  #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_print_aer(struct pci_dev *dev, int aer_severity,
> +				 struct aer_capability_regs *aer)
> +{ }
>  #endif
>  
> -void pci_print_aer(struct pci_dev *dev, int aer_severity,
> -		    struct aer_capability_regs *aer);
> +#if defined(CONFIG_ACPI_APEI_PCIEAER)
>  int cper_severity_to_aer(int cper_severity);
> +#else
> +static inline int cper_severity_to_aer(int cper_severity) { return 0; }
> +#endif
> +
>  void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn,
>  		       int severity, struct aer_capability_regs *aer_regs);
>  #endif //_AER_H_
> -- 
> 2.45.1
>
Dan Williams Aug. 7, 2024, 8:31 p.m. UTC | #3
Dan Williams wrote:
> [ add Boris ]

[ actually add Boris ]

Boris, see below, thoughts on deprecating acpi_extlog...

> Bjorn Helgaas wrote:
> > On Mon, May 27, 2024 at 04:43:41PM +0200, Fabio M. De Francesco wrote:
> > > Currently, extlog_print() (ELOG) only reports CPER PCIe section (UEFI
> > > v2.10, Appendix N.2.7) to the kernel log via print_extlog_rcd(). Instead,
> > > the similar ghes_do_proc() (GHES) prints to kernel log and calls
> > > pci_print_aer() to report via the ftrace infrastructure.
> > > 
> > > Add support to report the CPER PCIe Error section also via the ftrace
> > > infrastructure by calling pci_print_aer() to make ELOG act consistently
> > > with GHES.
> > > 
> > > Cc: Dan Williams <dan.j.williams@intel.com>
> > > Signed-off-by: Fabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>
> > > ---
> > >  drivers/acpi/acpi_extlog.c | 30 ++++++++++++++++++++++++++++++
> > >  drivers/pci/pcie/aer.c     |  2 +-
> > >  include/linux/aer.h        | 13 +++++++++++--
> > >  3 files changed, 42 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> > > index e025ae390737..007ce96f8672 100644
> > > --- a/drivers/acpi/acpi_extlog.c
> > > +++ b/drivers/acpi/acpi_extlog.c
> > > @@ -131,6 +131,32 @@ static int print_extlog_rcd(const char *pfx,
> > >  	return 1;
> > >  }
> > >  
> > > +static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
> > > +			      int severity)
> > > +{
> > > +	struct aer_capability_regs *aer;
> > > +	struct pci_dev *pdev;
> > > +	unsigned int devfn;
> > > +	unsigned int bus;
> > > +	int aer_severity;
> > > +	int domain;
> > > +
> > > +	if (pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID &&
> > > +	    pcie_err->validation_bits & CPER_PCIE_VALID_AER_INFO) {
> > > +		aer_severity = cper_severity_to_aer(severity);
> > > +		aer = (struct aer_capability_regs *)pcie_err->aer_info;
> > > +		domain = pcie_err->device_id.segment;
> > > +		bus = pcie_err->device_id.bus;
> > > +		devfn = PCI_DEVFN(pcie_err->device_id.device,
> > > +				  pcie_err->device_id.function);
> > > +		pdev = pci_get_domain_bus_and_slot(domain, bus, devfn);
> > > +		if (!pdev)
> > > +			return;
> > > +		pci_print_aer(pdev, aer_severity, aer);
> > > +		pci_dev_put(pdev);
> > > +	}
> > 
> > I'm 100% in favor of making error reporting work and look the same
> > across GHES and ELOG.  But I do have to gripe a bit...
> > 
> > It's already unfortunate that GHES and the native AER handling are
> > separate paths that lead to the same place (__aer_print_error()).
> > 
> > I'm sorry that we need to add a third path that again does
> > fundamentally the same thing.  The fact that they're separate means
> > all the design, reviewing, testing, and maintenance effort is diluted,
> > and error handling always gets too little love in the first place.
> > I think this is a recipe for confusion.
> > 
> >   ghes_do_proc                                        # GHES
> >     apei_estatus_for_each_section
> >       ...
> >       if (guid_equal(sec_type, &CPER_SEC_PCIE))
> >         ghes_handle_aer
> >           cper_severity_to_aer
> >           aer_recover_queue
> >             kfifo_in_spinlocked(&aer_recover_ring)    # add to queue
> >           aer_recover_work_func                       # another thread
> >             kfifo_get(&aer_recover_ring)              # remove from queue
> >             pci_print_aer
> >               __aer_print_error         <---
> > 
> >   aer_irq                                             # native AER
> >     kfifo_put(&aer_fifo)                              # add to queue
> >   aer_isr                                             # another thread
> >     kfifo_get(&aer_fifo)                              # remove from queue
> >     ...
> >     aer_isr_one_error
> >       aer_process_err_devices
> >         aer_print_error
> >           __aer_print_error             <---
> > 
> >   extlog_print                                        # extlog (x86 only)
> >     apei_estatus_for_each_section
> >       ...
> >       if (guid_equal(sec_type, &CPER_SEC_PCIE))
> >         extlog_print_pcie
> >           cper_severity_to_aer
> >           pci_get_domain_bus_and_slot
> >           pci_print_aer
> >             __aer_print_error           <---
> > 
> > And we also have CXL paths that lead to __aer_print_error(), although
> > it seems like they at least start in the native AER (and maybe GHES?)
> > path and branch out somewhere.  My head is spinning.
> > 
> > Do I *object* to this patch?  No, not really; it's a trivial change in
> > drivers/pci/, and Rafael can add my
> > 
> >   Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> > 
> > as needed.  But I am afraid we're making ourselves a maintenance
> > headache.
> 
> To be honest, I am too. Upon discovering that extlog_print() behaves
> differently than ghes_do_proc(), I had the snarky thought "great, can we
> now just go ahead and deprecate the extlog path, it's just a source of
> maintenance pain.".
> 
> So *if*we keep acpi_extlog it then I definitely think it should be
> consistent with other CPER handlers (needs this patch). But, I am also
> open to entertaining "deprecate it".
> 
> To me, the fact that ras_userspace_consumers() is only honored for
> acpi_extlog is clear evidence that the kernel has already painted itself
> into a confusing user ABI corner and maybe the proper path forward at
> this point is to cut acpi_extlog loose.
Fabio M. De Francesco Oct. 23, 2024, 1:35 p.m. UTC | #4
On Tuesday, August 6, 2024 9:56:24 PM GMT+2 Dan Williams wrote:
> Fabio M. De Francesco wrote:
> > Currently, extlog_print() (ELOG) only reports CPER PCIe section (UEFI
> > v2.10, Appendix N.2.7) to the kernel log via print_extlog_rcd().
> 
> I think the critical detail is is that print_extlog_rcd() is only
> triggered when ras_userspace_consumers() returns true. The observation
> is that ras_userspace_consumers() hides information from the trace path
> when the intended purpose of it was to hide duplicate emissions to the
> kernel log when userspace is watching the tracepoints.
>
> Setting aside whether ras_userspace_consumers() is still a good idea or
> not, it is obvious that this patch as is may surprise environments that
> start seeing kernel error logs where the kernel was silent before.
>
> I think the path of least surprise would be to make sure that
> pci_print_aer() optionally skips emitting to the kernel log when not
> needed wanted.

Sorry for replying so late...

I'm not entirely sure that users would not prefer to be surprised by 
_finally_ seeing kernel error logs for failing PCIe components. I suspect 
that users might have been confused by not seeing any output.
 
> So perhaps first do a lead-in patch to optionally quiet the print
> messages in pci_print_aer() and then pass in KERN_DEBUG from the
> extlog_print() path. Then we can decide later what to do about
> ras_userspace_consumers().

Anyway, I'll do it.

> > the similar ghes_do_proc() (GHES) prints to kernel log and calls
> > pci_print_aer() to report via the ftrace infrastructure.
> > 
> > Add support to report the CPER PCIe Error section also via the ftrace
> > infrastructure by calling pci_print_aer() to make ELOG act consistently
> > with GHES.
> 
> You might also want to explain a bit about the motivation for this which
> is that I/O Machine Check Arcitecture events may signal failing PCIe
> components or links. The AER event contains details on what was
> happening on the wire when the error was signaled.

Yes, I agree.

> > 
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Fabio M. De Francesco 
<fabio.m.de.francesco@linux.intel.com>
> > ---
> >  drivers/acpi/acpi_extlog.c | 30 ++++++++++++++++++++++++++++++
> >  drivers/pci/pcie/aer.c     |  2 +-
> >  include/linux/aer.h        | 13 +++++++++++--
> >  3 files changed, 42 insertions(+), 3 deletions(-)
> > 
> > [...]
> >
> > +		pci_print_aer(pdev, aer_severity, aer);
> 
> ...per above this would become:
> 
>     pci_print_aer(KERN_DEBUG, pdev, aer_severity, aer);
> 
> [..]
> 
> Rest of the changes look good to me.

I need to be sure that I understood...

void pci_print_aer(char *level, struct pci_dev *dev, int aer_severity,
                   struct aer_capability_regs *aer)
{
        [...]

        if (printk_get_level(level) <= console_loglevel) {
                pci_err(dev, "aer_status: 0x%08x, aer_mask: 0x%08x\n",
                        status, mask);
                __aer_print_error(dev, &info);
                pci_err(dev, "aer_layer=%s, aer_agent=%s\n",
                        aer_error_layer[layer], aer_agent_string[agent]);

                if (aer_severity != AER_CORRECTABLE)
                        pci_err(dev, "aer_uncor_severity: 0x%08x\n",
                                aer->uncor_severity);

                if (tlp_header_valid)
                        __print_tlp_header(dev, &aer->header_log);
        }

        [...]
}	

It would require changing a couple of call sites, like in    
aer_recover_work_func():

pci_print_aer(KERN_ERR, pdev, entry.severity, entry.regs);
 
Would you please confirm that the code shown above is what
you asked for?

Thanks,

Fabio
Dan Williams Dec. 11, 2024, 1:51 a.m. UTC | #5
Fabio M. De Francesco wrote:
> On Tuesday, August 6, 2024 9:56:24 PM GMT+2 Dan Williams wrote:
> > Fabio M. De Francesco wrote:
> > > Currently, extlog_print() (ELOG) only reports CPER PCIe section (UEFI
> > > v2.10, Appendix N.2.7) to the kernel log via print_extlog_rcd().
> > 
> > I think the critical detail is is that print_extlog_rcd() is only
> > triggered when ras_userspace_consumers() returns true. The observation
> > is that ras_userspace_consumers() hides information from the trace path
> > when the intended purpose of it was to hide duplicate emissions to the
> > kernel log when userspace is watching the tracepoints.
> >
> > Setting aside whether ras_userspace_consumers() is still a good idea or
> > not, it is obvious that this patch as is may surprise environments that
> > start seeing kernel error logs where the kernel was silent before.
> >
> > I think the path of least surprise would be to make sure that
> > pci_print_aer() optionally skips emitting to the kernel log when not
> > needed wanted.
> 
> Sorry for replying so late...
> 
> I'm not entirely sure that users would not prefer to be surprised by 
> _finally_ seeing kernel error logs for failing PCIe components. I suspect 
> that users might have been confused by not seeing any output.

2 notes:

* New KERN_ERR prints are often found to be unwelcome. When the kernel starts
  printing new error messages it causes sysadmins to scramble.

* The future of RAS is trace-events. Any new RAS messages to the kernel
  log need to ask the question, "is userspace better served by
  registering for a RAS trace event, rather than parsing kernel log
  messsages".

[..]
> I need to be sure that I understood...
> 
> void pci_print_aer(char *level, struct pci_dev *dev, int aer_severity,
>                    struct aer_capability_regs *aer)
> {
>         [...]
> 
>         if (printk_get_level(level) <= console_loglevel) {
>                 pci_err(dev, "aer_status: 0x%08x, aer_mask: 0x%08x\n",
>                         status, mask);

No, the code would be:

    pci_printk(level, dev, "aer_status: 0x%08x, aer_mask: 0x%08x\n", status, mask);

...i.e. just pass @level rather than open code "if
(printk_get_level(level) <= console_loglevel)".
diff mbox series

Patch

diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index e025ae390737..007ce96f8672 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -131,6 +131,32 @@  static int print_extlog_rcd(const char *pfx,
 	return 1;
 }
 
+static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
+			      int severity)
+{
+	struct aer_capability_regs *aer;
+	struct pci_dev *pdev;
+	unsigned int devfn;
+	unsigned int bus;
+	int aer_severity;
+	int domain;
+
+	if (pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID &&
+	    pcie_err->validation_bits & CPER_PCIE_VALID_AER_INFO) {
+		aer_severity = cper_severity_to_aer(severity);
+		aer = (struct aer_capability_regs *)pcie_err->aer_info;
+		domain = pcie_err->device_id.segment;
+		bus = pcie_err->device_id.bus;
+		devfn = PCI_DEVFN(pcie_err->device_id.device,
+				  pcie_err->device_id.function);
+		pdev = pci_get_domain_bus_and_slot(domain, bus, devfn);
+		if (!pdev)
+			return;
+		pci_print_aer(pdev, aer_severity, aer);
+		pci_dev_put(pdev);
+	}
+}
+
 static int extlog_print(struct notifier_block *nb, unsigned long val,
 			void *data)
 {
@@ -179,6 +205,10 @@  static int extlog_print(struct notifier_block *nb, unsigned long val,
 			if (gdata->error_data_length >= sizeof(*mem))
 				trace_extlog_mem_event(mem, err_seq, fru_id, fru_text,
 						       (u8)gdata->error_severity);
+		} else if (guid_equal(sec_type, &CPER_SEC_PCIE)) {
+			struct cper_sec_pcie *pcie_err = acpi_hest_get_payload(gdata);
+
+			extlog_print_pcie(pcie_err, gdata->error_severity);
 		} else {
 			void *err = acpi_hest_get_payload(gdata);
 
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index ac6293c24976..794aa15527ba 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -801,7 +801,7 @@  void pci_print_aer(struct pci_dev *dev, int aer_severity,
 	trace_aer_event(dev_name(&dev->dev), (status & ~mask),
 			aer_severity, tlp_header_valid, &aer->header_log);
 }
-EXPORT_SYMBOL_NS_GPL(pci_print_aer, CXL);
+EXPORT_SYMBOL_GPL(pci_print_aer);
 
 /**
  * add_error_device - list device to be handled
diff --git a/include/linux/aer.h b/include/linux/aer.h
index 4b97f38f3fcf..fbc82206045c 100644
--- a/include/linux/aer.h
+++ b/include/linux/aer.h
@@ -42,17 +42,26 @@  int pcie_read_tlp_log(struct pci_dev *dev, int where, struct pcie_tlp_log *log);
 #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_print_aer(struct pci_dev *dev, int aer_severity,
+		   struct aer_capability_regs *aer);
 #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_print_aer(struct pci_dev *dev, int aer_severity,
+				 struct aer_capability_regs *aer)
+{ }
 #endif
 
-void pci_print_aer(struct pci_dev *dev, int aer_severity,
-		    struct aer_capability_regs *aer);
+#if defined(CONFIG_ACPI_APEI_PCIEAER)
 int cper_severity_to_aer(int cper_severity);
+#else
+static inline int cper_severity_to_aer(int cper_severity) { return 0; }
+#endif
+
 void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn,
 		       int severity, struct aer_capability_regs *aer_regs);
 #endif //_AER_H_