Re: [PATCH v2 0/3] PCI/DPC: Clean up DPC vs AER/EDR ownership and Kconfig

[Ethan Zhao]

On 2/27/2024 2:35 PM, Kuppuswamy Sathyanarayanan wrote:
> Hi,
>
> On 2/26/24 10:18 PM, Ethan Zhao wrote:
> > On 2/23/2024 6:15 AM, Bjorn Helgaas wrote:
> > > From: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> > >
> > > Previously we could request control of DPC without AER, which is illegal
> > > per spec.  Also, we could enable CONFIG_PCIE_DPC without CONFIG_PCIE_EDR,
> > > which is also illegal.  This series addresses both.
> > I have a question here, how to understand the relationship EDR & AER ?
> > somewhere EDR touches AER status without checking _OSC granted bits,
> > such as
> >     pci_aer_raw_clear_status(edev);
>
> Which_OSC bits?
>
> EDR code will only get triggered if OS advertises the EDR support (which
> also means OS supports AER and DPC), and both AER and DPC is owned by
> the firmware. During the EDR notification, the OS is allowed to touch AER

Means no need to check if host->native_aer ? why checked in
pcie_do_recovery() ?

Thanks,
Ethan

> and DPC registers. So there is no problem with EDR code using AER routines.
>
>
> > sometimes EDR calling AER with host->native_aer checked, like
> >
> > pcie_do_recovery()
> > {
> >   ...
> >   if (host->native_aer || pcie_ports_native) {
> >          pcie_clear_device_status(dev);
> >          pci_aer_clear_nonfatal_status(dev);
> >      }
> >   ...
> > }
> >
> > That is really confusing. could we do some cleanup to eliminate it ?
> > such as seperate AER code into common code and runtime part.
> >
> >
> > Thanks,
> > Ethan
> >   
> >
> > > Bjorn Helgaas (3):
> > >     PCI/DPC: Request DPC only if also requesting AER
> > >     PCI/DPC: Remove CONFIG_PCIE_EDR
> > >     PCI/DPC: Encapsulate pci_acpi_add_edr_notifier()
> > >
> > >    drivers/acpi/pci_root.c   | 22 ++++++++++++----------
> > >    drivers/pci/pci.h         |  4 ++++
> > >    drivers/pci/pcie/Kconfig  | 14 ++++----------
> > >    drivers/pci/pcie/Makefile |  5 ++++-
> > >    drivers/pci/pcie/dpc.c    | 10 ----------
> > >    include/linux/pci-acpi.h  |  8 --------
> > >    6 files changed, 24 insertions(+), 39 deletions(-)