program_hpx_type2() is today unconditionally called, despite the fact
that when the _HPX was added to the ACPI spec. v3.0, the description
stated:
OSPM [1] will only evaluate _HPX with Setting Record – Type 2 if OSPM
is not controlling the PCI Express Advanced Error Reporting
capability.
Hence, we only call program_hpx_type2() when the OSPM owns the PCIe
hotplug capability but not the AER.
The Advanced Configuration and Power Interface (ACPI) Specification
version 6.6 has a provision that gives the OSPM the ability to control
the other PCIe Device Control bits any way. In a note in section
6.2.9, it is stated:
"OSPM may override the settings provided by the _HPX object's Type2
record (PCI Express Settings) or Type3 record (PCI Express Descriptor
Settings) when OSPM has assumed native control of the corresponding
feature."
So, in order to preserve the non-AER bits in PCIe Device Control, in
particular the performance sensitive ExtTag and RO, we make sure
program_hpx_type2() if called, doesn't modify any non-AER bits.
Also, when program_hpx_type2() is called, we completely avoid
modifying any bits in the Link Control register. However, if the _HPX
type 2 records contains bits indicating such modifications, we print
an info message.
[1] Operating System-directed configuration and Power Management
Fixes: 40abb96c51bb ("[PATCH] pciehp: Fix programming hotplug parameters")
Signed-off-by: Håkon Bugge <[email protected]>
---
v1 -> v2:
* Fixed comment style
* Simplified the and/or logic when programming the Device Control
register
* Fixed the incorrect and brutal warning about Link Control
register bits set and changed it to an info message about _HPX
attempting to set/reset bits therein.
* Removed the RCB programming from program_hpx_type2()
* Moved the PCI_EXP_AER_FLAGS definition from
drivers/pci/pcie/aer.c to drivers/pci/pci.h
---
drivers/pci/pci-acpi.c | 61 +++++++++++++++++++-----------------------
drivers/pci/pci.h | 3 +++
drivers/pci/pcie/aer.c | 3 ---
3 files changed, 30 insertions(+), 37 deletions(-)
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 9369377725fa0..34ea22f65a410 100644
--- 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);
+
+ /*
+ * We only do the HP programming if we own the PCIe native
+ * hotplug and not the AER ownership
+ */
+ 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,31 @@ 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
+ * We only allow _HPX to program the AER registers, namely
+ * PCI_EXP_DEVCTL_CERE, PCI_EXP_DEVCTL_NFERE,
+ * PCI_EXP_DEVCTL_FERE, and PCI_EXP_DEVCTL_URRE.
+ *
+ * The other settings in PCIe DEVCTL are managed by OS in
+ * order to make sure they're 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 PCIe 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)
+ pci_info(dev,
+ "_HPX attempts to reset the following bits in
PCIe Link Control: 0x%04x\n",
+ hpx->pci_exp_lnkctl_and);
+ if (hpx->pci_exp_lnkctl_or)
+ pci_info(dev,
+ "_HPX attempts to set the following bits in
PCIe Link Control: 0x%04x\n",
+ hpx->pci_exp_lnkctl_or);
}
/* Find Advanced Error Reporting Enhanced Capability */
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 0e67014aa0013..f388d4414dd3a 100644
--- 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 e0bcaa896803c..9472d86cef552 100644
--- 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);
--
2.43.5
