On 2/20/24 3:55 PM, Bjorn Helgaas wrote: > From: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > > When booting with "pci=noaer", we don't request control of AER, but we > previously *did* request control of DPC, as in the dmesg log attached at > the bugzilla below: > > Command line: ... pci=noaer > acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM ClockPM Segments MSI EDR HPX-Type3] > acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug SHPCHotplug PME PCIeCapability LTR DPC] > > That's illegal per PCI Firmware Spec, r3.3, sec 4.5.1, table 4-5, which > says: > > If the operating system sets this bit [OSC_PCI_EXPRESS_DPC_CONTROL], it > must also set bit 7 of the Support field (indicating support for Error > Disconnect Recover notifications) and bits 3 and 4 of the Control field > (requesting control of PCI Express Advanced Error Reporting and the PCI > Express Capability Structure). > > Request DPC control only if we have also requested AER control. Can you also add similar check in calculate_support call? if (pci_aer_available() && IS_ENABLED(CONFIG_PCIE_EDR)) support |= OSC_PCI_EDR_SUPPORT; > > Fixes: ac1c8e35a326 ("PCI/DPC: Add Error Disconnect Recover (EDR) support") > Link: https://bugzilla.kernel.org/show_bug.cgi?id=218491#c12 > Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > Cc: <stable@xxxxxxxxxxxxxxx> # v5.7+ > Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> > Cc: Matthew W Carlis <mattc@xxxxxxxxxxxxxxx> > Cc: Keith Busch <kbusch@xxxxxxxxxx> > Cc: Lukas Wunner <lukas@xxxxxxxxx> > Cc: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> > Cc: Jesse Brandeburg <jesse.brandeburg@xxxxxxxxx> > --- > drivers/acpi/pci_root.c | 20 +++++++++++--------- > 1 file changed, 11 insertions(+), 9 deletions(-) > > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > index 58b89b8d950e..1c16965427b3 100644 > --- a/drivers/acpi/pci_root.c > +++ b/drivers/acpi/pci_root.c > @@ -518,17 +518,19 @@ static u32 calculate_control(void) > if (IS_ENABLED(CONFIG_HOTPLUG_PCI_SHPC)) > control |= OSC_PCI_SHPC_NATIVE_HP_CONTROL; > > - if (pci_aer_available()) > + if (pci_aer_available()) { > control |= OSC_PCI_EXPRESS_AER_CONTROL; > > - /* > - * Per the Downstream Port Containment Related Enhancements ECN to > - * the PCI Firmware Spec, r3.2, sec 4.5.1, table 4-5, > - * OSC_PCI_EXPRESS_DPC_CONTROL indicates the OS supports both DPC > - * and EDR. > - */ > - if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR)) > - control |= OSC_PCI_EXPRESS_DPC_CONTROL; > + /* > + * Per PCI Firmware Spec, r3.3, sec 4.5.1, table 4-5, the > + * OS can request DPC control only if it has advertised > + * OSC_PCI_EDR_SUPPORT and requested both > + * OSC_PCI_EXPRESS_CAPABILITY_CONTROL and I think you mean OSC_PCI_EXPRESS_DPC_CONTROL. > + * OSC_PCI_EXPRESS_AER_CONTROL. > + */ > + if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR)) > + control |= OSC_PCI_EXPRESS_DPC_CONTROL; Since you are cleaning up this part, why not add a patch to remove CONFIG_PCIE_EDR? > + } > > return control; > } -- Sathyanarayanan Kuppuswamy Linux Kernel Developer