PCI/ACPI: Restrict program_hpx_type2() to AER bits

Previously program_hpx_type2() applied PCIe settings unconditionally,
which could incorrectly change bits like Extended Tag Field Enable and
Enable Relaxed Ordering.

When _HPX was added to ACPI r3.0, the intent of the PCIe Setting
Record (Type 2) in sec 6.2.7.3 was to configure AER registers when the
OS does not own the AER Capability:

  The PCI Express setting record contains ... [the AER] Uncorrectable
  Error Mask, Uncorrectable Error Severity, Correctable Error Mask
  ... to be used when configuring registers in the Advanced Error
  Reporting Extended Capability Structure ...

  OSPM [1] will only evaluate _HPX with Setting Record – Type 2 if
  OSPM is not controlling the PCI Express Advanced Error Reporting
  capability.

ACPI r3.0b, sec 6.2.7.3, added more AER registers, including registers
in the PCIe Capability with AER-related bits, and the restriction that
the OS use this only when it owns PCIe native hotplug:

  ... when configuring PCI Express registers in the Advanced Error
  Reporting Extended Capability Structure *or PCI Express Capability
  Structure* ...

  An OS that has assumed ownership of native hot plug but does not
  ... have ownership of the AER register set must use ... the Type 2
  record to program the AER registers ...

  However, since the Type 2 record also includes register bits that
  have functions other than AER, the OS must ignore values ... that
  are not applicable.

Restrict program_hpx_type2() to only the intended purpose:

  - Apply settings only when OS owns PCIe native hotplug but not AER,

  - Only touch the AER-related bits (Error Reporting Enables) in Device
    Control

  - Don't touch Link Control at all, since nothing there seems AER-related,
    but log _HPX settings for debugging purposes

Note that Read Completion Boundary is now configured elsewhere, since it is
unrelated to _HPX.

[1] Operating System-directed configuration and Power Management

Fixes: 40abb96c51bb ("[PATCH] pciehp: Fix programming hotplug parameters")
Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Link: https://patch.msgid.link/20260129175237.727059-3-haakon.bugge@oracle.com

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 9369377725fa030b6137df19caab0e298de87e60..0162acfb57896b81173c6680f923fdd2b1dd8e5c 100644 (file)
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -271,21 +271,6 @@ static acpi_status decode_type1_hpx_record(union acpi_object *record,
        return AE_OK;
 }
 
-static bool pcie_root_rcb_set(struct pci_dev *dev)
-{
-       struct pci_dev *rp = pcie_find_root_port(dev);
-       u16 lnkctl;
-
-       if (!rp)
-               return false;
-
-       pcie_capability_read_word(rp, PCI_EXP_LNKCTL, &lnkctl);
-       if (lnkctl & PCI_EXP_LNKCTL_RCB)
-               return true;
-
-       return false;
-}
-
 /* _HPX PCI Express Setting Record (Type 2) */
 struct hpx_type2 {
        u32 revision;
@@ -311,6 +296,7 @@ static void program_hpx_type2(struct pci_dev *dev, struct hpx_type2 *hpx)
 {
        int pos;
        u32 reg32;
+       const struct pci_host_bridge *host;
 
        if (!hpx)
                return;
@@ -318,6 +304,15 @@ static void program_hpx_type2(struct pci_dev *dev, struct hpx_type2 *hpx)
        if (!pci_is_pcie(dev))
                return;
 
+       host = pci_find_host_bridge(dev->bus);
+
+       /*
+        * Only do the _HPX Type 2 programming if OS owns PCIe native
+        * hotplug but not AER.
+        */
+       if (!host->native_pcie_hotplug || host->native_aer)
+               return;
+
        if (hpx->revision > 1) {
                pci_warn(dev, "PCIe settings rev %d not supported\n",
                         hpx->revision);
@@ -325,33 +320,27 @@ static void program_hpx_type2(struct pci_dev *dev, struct hpx_type2 *hpx)
        }
 
        /*
-        * Don't allow _HPX to change MPS or MRRS settings.  We manage
-        * those to make sure they're consistent with the rest of the
-        * platform.
+        * We only allow _HPX to program DEVCTL bits related to AER, namely
+        * PCI_EXP_DEVCTL_CERE, PCI_EXP_DEVCTL_NFERE, PCI_EXP_DEVCTL_FERE,
+        * and PCI_EXP_DEVCTL_URRE.
+        *
+        * The rest of DEVCTL is managed by the OS to make sure it's
+        * consistent with the rest of the platform.
         */
-       hpx->pci_exp_devctl_and |= PCI_EXP_DEVCTL_PAYLOAD |
-                                   PCI_EXP_DEVCTL_READRQ;
-       hpx->pci_exp_devctl_or &= ~(PCI_EXP_DEVCTL_PAYLOAD |
-                                   PCI_EXP_DEVCTL_READRQ);
+       hpx->pci_exp_devctl_and |= ~PCI_EXP_AER_FLAGS;
+       hpx->pci_exp_devctl_or &= PCI_EXP_AER_FLAGS;
 
        /* Initialize Device Control Register */
        pcie_capability_clear_and_set_word(dev, PCI_EXP_DEVCTL,
                        ~hpx->pci_exp_devctl_and, hpx->pci_exp_devctl_or);
 
-       /* Initialize Link Control Register */
+       /* Log if _HPX attempts to modify Link Control Register */
        if (pcie_cap_has_lnkctl(dev)) {
-
-               /*
-                * If the Root Port supports Read Completion Boundary of
-                * 128, set RCB to 128.  Otherwise, clear it.
-                */
-               hpx->pci_exp_lnkctl_and |= PCI_EXP_LNKCTL_RCB;
-               hpx->pci_exp_lnkctl_or &= ~PCI_EXP_LNKCTL_RCB;
-               if (pcie_root_rcb_set(dev))
-                       hpx->pci_exp_lnkctl_or |= PCI_EXP_LNKCTL_RCB;
-
-               pcie_capability_clear_and_set_word(dev, PCI_EXP_LNKCTL,
-                       ~hpx->pci_exp_lnkctl_and, hpx->pci_exp_lnkctl_or);
+               if (hpx->pci_exp_lnkctl_and != 0xffff ||
+                   hpx->pci_exp_lnkctl_or != 0)
+                       pci_info(dev, "_HPX attempts Link Control setting (AND %#06x OR %#06x)\n",
+                                hpx->pci_exp_lnkctl_and,
+                                hpx->pci_exp_lnkctl_or);
        }
 
        /* Find Advanced Error Reporting Enhanced Capability */

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 0e67014aa0013a7086c3a45d576d4b1ca2bb159f..e3c2852c80fbd30a5a7ea69e79753ce39f7a6a87 100644 (file)
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -88,6 +88,9 @@ struct pcie_tlp_log;
 #define PCI_BUS_BRIDGE_MEM_WINDOW      1
 #define PCI_BUS_BRIDGE_PREF_MEM_WINDOW 2
 
+#define PCI_EXP_AER_FLAGS      (PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | \
+                                PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE)
+
 extern const unsigned char pcie_link_speed[];
 extern bool pci_early_dump;
 

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index e0bcaa896803c91e7451e57fed06a725c90c6114..9472d86cef552488dbfbf1192574119a3f671754 100644 (file)
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -239,9 +239,6 @@ void pcie_ecrc_get_policy(char *str)
 }
 #endif /* CONFIG_PCIE_ECRC */
 
-#define        PCI_EXP_AER_FLAGS       (PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | \
-                                PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE)
-
 int pcie_aer_is_native(struct pci_dev *dev)
 {
        struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);