diff mbox

[2/4] pci: Add generic pci_bus_force_mmconfig interface

Message ID 20170302232104.10136-2-andi@firstfloor.org
State Changes Requested
Headers show

Commit Message

Andi Kleen March 2, 2017, 11:21 p.m. UTC 
From: Andi Kleen <ak@linux.intel.com>

x86 traditionally used mmconfig only for extended config space accesses
with offsets larger than 256. For lower offsets it uses the classic
Type 1 IO port access. This is quite slow and also requires taking
a global spin lock to protect the Type 1 IO port mailbox.

IIRC (I added it originally) it was merely to be conservative;
I don't remember any actual cases where mmconfig did not work
after passing the other sanity checks. But most devices
don't use extended config space, so most devices were never
tested with MMCONFIG. Starting to use MMCONFIG everywhere
unconditionally seems somewhat risky as we never tested this

Also for most drivers it doesn't really matter because they only
use config accesses during driver initialization, which is
not performance critical.

But some drivers can do a lot of config bus accesses < 256.
I ran into this problem with the Intel uncore PMU drivers
when sampling uncore counters in many PMUs at a high frequency
on a 4S system. This showed significant spin lock contention
and also large overhead in accessing the IO port.

For drivers like this is useful to be able to force MMCONFIG
access. The interface is intended for devices integrated on a SOC,
so there is no concern that some strange PCI bridges may
incorrectly handle mmconfig.

This patch adds a new function pci_bus_force_mmconfig() where
a driver can force MMCONFIG for all accesses on the bus.

This is a stub implementation, but the next patch will
fill it in for x86.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 drivers/pci/pci.c   | 15 +++++++++++++++
 include/linux/pci.h |  2 ++
 2 files changed, 17 insertions(+)

Comments

H. Peter Anvin March 14, 2017, 5:34 p.m. UTC | #1 
On 03/02/17 15:21, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> x86 traditionally used mmconfig only for extended config space accesses
> with offsets larger than 256. For lower offsets it uses the classic
> Type 1 IO port access. This is quite slow and also requires taking
> a global spin lock to protect the Type 1 IO port mailbox.
> 
> IIRC (I added it originally) it was merely to be conservative;
> I don't remember any actual cases where mmconfig did not work
> after passing the other sanity checks. But most devices
> don't use extended config space, so most devices were never
> tested with MMCONFIG. Starting to use MMCONFIG everywhere
> unconditionally seems somewhat risky as we never tested this
> 

I would rather enable it by default, but having a command-line switch to
disable it.  This seems a helluva lot saner than mucking with the entire
PCI bus from an individual device driver.

Also, it seems crazy to say we will take this penalty forever.  My
opinion is we should enable it in the absence of evidence of
malfunctions, but if we end up having problems we could put an ACPI date
cutoff quirk in.

	-hpa
diff mbox

Patch

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7904d02ffdb9..af68f1d9eed7 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5236,6 +5236,21 @@  int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent)
 #endif
 
 /**
+ * pci_bus_force_mmconfig - enable mmconfig for all config accesses on bus.
+ *
+ * Allow mmconfig for all accesses to devices. This is mainly a performance
+ * optimization, and should be only used if the driver "knows" all the bridges
+ * involved (e.g. SOC on chip devices)
+ *
+ * Default implementation. Architectures can override this.
+ */
+__weak int pci_bus_force_mmconfig(struct pci_bus *bus)
+{
+	return 0;
+}
+EXPORT_SYMBOL(pci_bus_force_mmconfig);
+
+/**
  * pci_ext_cfg_avail - can we access extended PCI config space?
  *
  * Returns 1 if we can access PCI extended config space (offsets
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 9b234cbc7ae1..61b4437dc8e5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -891,6 +891,8 @@  void pci_sort_breadthfirst(void);
 
 /* Generic PCI functions exported to card drivers */
 
+int pci_bus_force_mmconfig(struct pci_bus *bus);
+
 enum pci_lost_interrupt_reason {
 	PCI_LOST_IRQ_NO_INFORMATION = 0,
 	PCI_LOST_IRQ_DISABLE_MSI,