[patch 0/7] x86/pci: Switch to lockless ECAM configuration mode

[patch 0/7] x86/pci: Switch to lockless ECAM configuration mode

[Thomas Gleixner]

Andi reported that the Intel Uncore performance monitoring suffers from the
global pci_lock which protects the generic PCI config space accessors.

This lock can be removed on X86 because all architecture specific config
space accessors are either serialized by a seperate lock or can be
converted to have their own locking. The mmconfig based access (ECAM) does
not require serialization at the config read/write level because the access
is a simple byte/word/dword read from or write to the memory mapped
configuration space. In that case the lock does not provide any more
protection than the hardware provides already.

The series converts the CE4100 PCI low level accessors to use the x86
specific pci_config_lock and implements the switch over to lockless ECAM
configureation space accesors when the system is capable.

Thanks,

	tglx

---
 arch/x86/Kconfig               |    1 
 arch/x86/include/asm/pci.h     |    8 ---
 arch/x86/include/asm/pci_x86.h |   15 +++----
 arch/x86/pci/Makefile          |    2 
 arch/x86/pci/ce4100.c          |   87 ++++++++++++++++++++++-------------------
 arch/x86/pci/common.c          |   16 +++++++
 arch/x86/pci/legacy.c          |   19 +++++---
 arch/x86/pci/mmconfig-shared.c |   36 ++++++++++++++++
 arch/x86/pci/mmconfig_32.c     |   12 -----
 arch/x86/pci/mmconfig_64.c     |   16 +------
 drivers/pci/Kconfig            |    3 +
 drivers/pci/access.c           |   16 +++++--
 12 files changed, 142 insertions(+), 89 deletions(-)

[patch 1/7] x86/pci: Remove duplicate defines

[Thomas Gleixner]

[-- Attachment #1: x86-pci--Remove-duplicate-defines.patch --]
[-- Type: text/plain, Size: 763 bytes --]

For some historic reason these defines are duplicated. Remove them.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/pci.h |    8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -76,14 +76,8 @@ static inline bool is_vmd(struct pci_bus
 
 extern unsigned int pcibios_assign_all_busses(void);
 extern int pci_legacy_init(void);
-# ifdef CONFIG_ACPI
-#  define x86_default_pci_init pci_acpi_init
-# else
-#  define x86_default_pci_init pci_legacy_init
-# endif
 #else
-# define pcibios_assign_all_busses()	0
-# define x86_default_pci_init		NULL
+static inline int pcibios_assign_all_busses(void) { return 0; }
 #endif
 
 extern unsigned long pci_mem_start;

Re: [patch 1/7] x86/pci: Remove duplicate defines

[Bjorn Helgaas]

[+cc linux-pci]

On Thu, Mar 16, 2017 at 10:50:03PM +0100, Thomas Gleixner wrote:
> For some historic reason these defines are duplicated. Remove them.

Duplicated in arch/x86/include/asm/pci.h and
arch/x86/include/asm/pci_x86.h, I guess.

Good to remove the duplication, for sure.  I wonder why we didn't see
duplicate definition warnings.

> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/x86/include/asm/pci.h |    8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> --- a/arch/x86/include/asm/pci.h
> +++ b/arch/x86/include/asm/pci.h
> @@ -76,14 +76,8 @@ static inline bool is_vmd(struct pci_bus
>  
>  extern unsigned int pcibios_assign_all_busses(void);
>  extern int pci_legacy_init(void);
> -# ifdef CONFIG_ACPI
> -#  define x86_default_pci_init pci_acpi_init
> -# else
> -#  define x86_default_pci_init pci_legacy_init
> -# endif
>  #else
> -# define pcibios_assign_all_busses()	0
> -# define x86_default_pci_init		NULL
> +static inline int pcibios_assign_all_busses(void) { return 0; }
>  #endif
>  
>  extern unsigned long pci_mem_start;
> 
> 

[tip:x86/platform] x86/PCI: Remove duplicate defines

[tip-bot for Thomas Gleixner]

Commit-ID:  9304d1621e6019c63497f8a4aad09d003916dbe9
Gitweb:     http://git.kernel.org/tip/9304d1621e6019c63497f8a4aad09d003916dbe9
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 16 Mar 2017 22:50:03 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 28 Jun 2017 22:32:55 +0200

x86/PCI: Remove duplicate defines

For some historic reason these defines are duplicated and also available in
arch/x86/include/asm/pci_x86.h,

Remove them.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Bjorn Helgaas <helgaas@kernel.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: linux-pci@vger.kernel.org
Link: http://lkml.kernel.org/r/20170316215056.967808646@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/include/asm/pci.h | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index f513cc2..473a729 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -77,14 +77,8 @@ static inline bool is_vmd(struct pci_bus *bus)
 
 extern unsigned int pcibios_assign_all_busses(void);
 extern int pci_legacy_init(void);
-# ifdef CONFIG_ACPI
-#  define x86_default_pci_init pci_acpi_init
-# else
-#  define x86_default_pci_init pci_legacy_init
-# endif
 #else
-# define pcibios_assign_all_busses()	0
-# define x86_default_pci_init		NULL
+static inline int pcibios_assign_all_busses(void) { return 0; }
 #endif
 
 extern unsigned long pci_mem_start;

[patch 2/7] x86/pci: Abort if legacy init fails

[Thomas Gleixner]

[-- Attachment #1: x86-pci--Abort-if-legacy-init-fails.patch --]
[-- Type: text/plain, Size: 1642 bytes --]

If the legacy pci init fails, then there are no pci config space accesors
available, but the code continues and tries to scan the busses, which fails
due to the lack of config space accessors.

Return right away, if the last init fallback fails.

Switch the few printks to pr_info while at it.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/pci/legacy.c |   18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

--- a/arch/x86/pci/legacy.c
+++ b/arch/x86/pci/legacy.c
@@ -24,12 +24,10 @@ static void pcibios_fixup_peer_bridges(v
 
 int __init pci_legacy_init(void)
 {
-	if (!raw_pci_ops) {
-		printk("PCI: System does not support PCI\n");
-		return 0;
-	}
+	if (!raw_pci_ops)
+		return 1;
 
-	printk("PCI: Probing PCI hardware\n");
+	pr_info("PCI: Probing PCI hardware\n");
 	pcibios_scan_root(0);
 	return 0;
 }
@@ -46,7 +44,7 @@ void pcibios_scan_specific_bus(int busn)
 		if (!raw_pci_read(0, busn, devfn, PCI_VENDOR_ID, 2, &l) &&
 		    l != 0x0000 && l != 0xffff) {
 			DBG("Found device at %02x:%02x [%04x]\n", busn, devfn, l);
-			printk(KERN_INFO "PCI: Discovered peer bus %02x\n", busn);
+			pr_info("PCI: Discovered peer bus %02x\n", busn);
 			pcibios_scan_root(busn);
 			return;
 		}
@@ -60,8 +58,12 @@ static int __init pci_subsys_init(void)
 	 * The init function returns an non zero value when
 	 * pci_legacy_init should be invoked.
 	 */
-	if (x86_init.pci.init())
-		pci_legacy_init();
+	if (x86_init.pci.init()) {
+		if (pci_legacy_init()) {
+			pr_info("PCI: System does not support PCI\n");
+			return -ENODEV;
+		}
+	}
 
 	pcibios_fixup_peer_bridges();
 	x86_init.pci.init_irq();

Re: [patch 2/7] x86/pci: Abort if legacy init fails

[Bjorn Helgaas]

[+cc linux-pci]

s|x86/pci: |x86/PCI: | for these three, to follow previous convention.

On Thu, Mar 16, 2017 at 10:50:04PM +0100, Thomas Gleixner wrote:
> If the legacy pci init fails, then there are no pci config space accesors
> available, but the code continues and tries to scan the busses, which fails
> due to the lack of config space accessors.

s/pci/PCI/ above.

> Return right away, if the last init fallback fails.
> 
> Switch the few printks to pr_info while at it.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/x86/pci/legacy.c |   18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> --- a/arch/x86/pci/legacy.c
> +++ b/arch/x86/pci/legacy.c
> @@ -24,12 +24,10 @@ static void pcibios_fixup_peer_bridges(v
>  
>  int __init pci_legacy_init(void)
>  {
> -	if (!raw_pci_ops) {
> -		printk("PCI: System does not support PCI\n");
> -		return 0;
> -	}
> +	if (!raw_pci_ops)
> +		return 1;
>  
> -	printk("PCI: Probing PCI hardware\n");
> +	pr_info("PCI: Probing PCI hardware\n");
>  	pcibios_scan_root(0);
>  	return 0;
>  }
> @@ -46,7 +44,7 @@ void pcibios_scan_specific_bus(int busn)
>  		if (!raw_pci_read(0, busn, devfn, PCI_VENDOR_ID, 2, &l) &&
>  		    l != 0x0000 && l != 0xffff) {
>  			DBG("Found device at %02x:%02x [%04x]\n", busn, devfn, l);
> -			printk(KERN_INFO "PCI: Discovered peer bus %02x\n", busn);
> +			pr_info("PCI: Discovered peer bus %02x\n", busn);
>  			pcibios_scan_root(busn);
>  			return;
>  		}
> @@ -60,8 +58,12 @@ static int __init pci_subsys_init(void)
>  	 * The init function returns an non zero value when
>  	 * pci_legacy_init should be invoked.
>  	 */
> -	if (x86_init.pci.init())
> -		pci_legacy_init();
> +	if (x86_init.pci.init()) {
> +		if (pci_legacy_init()) {
> +			pr_info("PCI: System does not support PCI\n");
> +			return -ENODEV;
> +		}
> +	}
>  
>  	pcibios_fixup_peer_bridges();
>  	x86_init.pci.init_irq();
> 
> 

[tip:x86/platform] x86/PCI: Abort if legacy init fails

[tip-bot for Thomas Gleixner]

Commit-ID:  aae3e318d012e76211f34bb65754f3d4d2a8c93d
Gitweb:     http://git.kernel.org/tip/aae3e318d012e76211f34bb65754f3d4d2a8c93d
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 16 Mar 2017 22:50:04 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 28 Jun 2017 22:32:55 +0200

x86/PCI: Abort if legacy init fails

If the legacy PCI init fails, then there are no PCI config space accesors
available, but the code continues and tries to scan the busses, which fails
due to the lack of config space accessors.

Return right away, if the last init fallback fails.

Switch the few printks to pr_info while at it.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Bjorn Helgaas <helgaas@kernel.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: linux-pci@vger.kernel.org
Link: http://lkml.kernel.org/r/20170316215057.047576516@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/pci/legacy.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/x86/pci/legacy.c b/arch/x86/pci/legacy.c
index ea6f380..1cb01ab 100644
--- a/arch/x86/pci/legacy.c
+++ b/arch/x86/pci/legacy.c
@@ -24,12 +24,10 @@ static void pcibios_fixup_peer_bridges(void)
 
 int __init pci_legacy_init(void)
 {
-	if (!raw_pci_ops) {
-		printk("PCI: System does not support PCI\n");
-		return 0;
-	}
+	if (!raw_pci_ops)
+		return 1;
 
-	printk("PCI: Probing PCI hardware\n");
+	pr_info("PCI: Probing PCI hardware\n");
 	pcibios_scan_root(0);
 	return 0;
 }
@@ -46,7 +44,7 @@ void pcibios_scan_specific_bus(int busn)
 		if (!raw_pci_read(0, busn, devfn, PCI_VENDOR_ID, 2, &l) &&
 		    l != 0x0000 && l != 0xffff) {
 			DBG("Found device at %02x:%02x [%04x]\n", busn, devfn, l);
-			printk(KERN_INFO "PCI: Discovered peer bus %02x\n", busn);
+			pr_info("PCI: Discovered peer bus %02x\n", busn);
 			pcibios_scan_root(busn);
 			return;
 		}
@@ -60,8 +58,12 @@ static int __init pci_subsys_init(void)
 	 * The init function returns an non zero value when
 	 * pci_legacy_init should be invoked.
 	 */
-	if (x86_init.pci.init())
-		pci_legacy_init();
+	if (x86_init.pci.init()) {
+		if (pci_legacy_init()) {
+			pr_info("PCI: System does not support PCI\n");
+			return -ENODEV;
+		}
+	}
 
 	pcibios_fixup_peer_bridges();
 	x86_init.pci.init_irq();

[patch 3/7] x86/pci/ce4100: Properly lock accessor functions

[Thomas Gleixner]

[-- Attachment #1: x86-pci-ce4100--Properly-lock-accessor-functions.patch --]
[-- Type: text/plain, Size: 4970 bytes --]

x86 wants to get rid of the global pci_lock protecting the config space
accessors so ECAM mode can operate completely lockless, but the CE4100 pci
code relies on that to protect the simulation registers.

Restructure the code so it uses the x86 specific pci_config_lock to
serialize the inner workings of the CE4100 PCI magic. That allows to remove
the global locking via pci_lock later.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/pci/ce4100.c |   87 +++++++++++++++++++++++++++-----------------------
 1 file changed, 48 insertions(+), 39 deletions(-)

--- a/arch/x86/pci/ce4100.c
+++ b/arch/x86/pci/ce4100.c
@@ -65,6 +65,9 @@ struct sim_reg_op {
 { PCI_DEVFN(device, func), offset, init_op, read_op, write_op,\
 	{0, SIZE_TO_MASK(size)} },
 
+/*
+ * All read/write functions are called with pci_config_lock held.
+ */
 static void reg_init(struct sim_dev_reg *reg)
 {
 	pci_direct_conf1.read(0, 1, reg->dev_func, reg->reg, 4,
@@ -73,21 +76,13 @@ static void reg_init(struct sim_dev_reg
 
 static void reg_read(struct sim_dev_reg *reg, u32 *value)
 {
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&pci_config_lock, flags);
 	*value = reg->sim_reg.value;
-	raw_spin_unlock_irqrestore(&pci_config_lock, flags);
 }
 
 static void reg_write(struct sim_dev_reg *reg, u32 value)
 {
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&pci_config_lock, flags);
 	reg->sim_reg.value = (value & reg->sim_reg.mask) |
 		(reg->sim_reg.value & ~reg->sim_reg.mask);
-	raw_spin_unlock_irqrestore(&pci_config_lock, flags);
 }
 
 static void sata_reg_init(struct sim_dev_reg *reg)
@@ -117,12 +112,8 @@ static void sata_revid_read(struct sim_d
 
 static void reg_noirq_read(struct sim_dev_reg *reg, u32 *value)
 {
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&pci_config_lock, flags);
 	/* force interrupt pin value to 0 */
 	*value = reg->sim_reg.value & 0xfff00ff;
-	raw_spin_unlock_irqrestore(&pci_config_lock, flags);
 }
 
 static struct sim_dev_reg bus1_fixups[] = {
@@ -265,24 +256,33 @@ int bridge_read(unsigned int devfn, int
 	return retval;
 }
 
-static int ce4100_conf_read(unsigned int seg, unsigned int bus,
-			    unsigned int devfn, int reg, int len, u32 *value)
+static int ce4100_bus1_read(unsigned int devfn, int reg, int len, u32 *value)
 {
+	unsigned long flags;
 	int i;
 
-	WARN_ON(seg);
-	if (bus == 1) {
-		for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
-			if (bus1_fixups[i].dev_func == devfn &&
-			    bus1_fixups[i].reg == (reg & ~3) &&
-			    bus1_fixups[i].read) {
-				bus1_fixups[i].read(&(bus1_fixups[i]),
-						    value);
-				extract_bytes(value, reg, len);
-				return 0;
-			}
+	for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
+		if (bus1_fixups[i].dev_func == devfn &&
+		    bus1_fixups[i].reg == (reg & ~3) &&
+		    bus1_fixups[i].read) {
+
+			raw_spin_lock_irqsave(&pci_config_lock, flags);
+			bus1_fixups[i].read(&(bus1_fixups[i]), value);
+			raw_spin_unlock_irqrestore(&pci_config_lock, flags);
+			extract_bytes(value, reg, len);
+			return 0;
 		}
 	}
+	return -1;
+}
+
+static int ce4100_conf_read(unsigned int seg, unsigned int bus,
+			    unsigned int devfn, int reg, int len, u32 *value)
+{
+	WARN_ON(seg);
+
+	if (bus == 1 && !ce4100_bus1_read(devfn, reg, len, value))
+		return 0;
 
 	if (bus == 0 && (PCI_DEVFN(1, 0) == devfn) &&
 	    !bridge_read(devfn, reg, len, value))
@@ -291,23 +291,32 @@ static int ce4100_conf_read(unsigned int
 	return pci_direct_conf1.read(seg, bus, devfn, reg, len, value);
 }
 
-static int ce4100_conf_write(unsigned int seg, unsigned int bus,
-			     unsigned int devfn, int reg, int len, u32 value)
+static int ce4100_bus1_write(unsigned int devfn, int reg, int len, u32 value)
 {
+	unsigned long flags;
 	int i;
 
-	WARN_ON(seg);
-	if (bus == 1) {
-		for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
-			if (bus1_fixups[i].dev_func == devfn &&
-			    bus1_fixups[i].reg == (reg & ~3) &&
-			    bus1_fixups[i].write) {
-				bus1_fixups[i].write(&(bus1_fixups[i]),
-						     value);
-				return 0;
-			}
+	for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
+		if (bus1_fixups[i].dev_func == devfn &&
+		    bus1_fixups[i].reg == (reg & ~3) &&
+		    bus1_fixups[i].write) {
+
+			raw_spin_lock_irqsave(&pci_config_lock, flags);
+			bus1_fixups[i].write(&(bus1_fixups[i]), value);
+			raw_spin_unlock_irqrestore(&pci_config_lock, flags);
+			return 0;
 		}
 	}
+	return -1;
+}
+
+static int ce4100_conf_write(unsigned int seg, unsigned int bus,
+			     unsigned int devfn, int reg, int len, u32 value)
+{
+	WARN_ON(seg);
+
+	if (bus == 1 && !ce4100_bus1_write(devfn, reg, len, value))
+		return 0;
 
 	/* Discard writes to A/V bridge BAR. */
 	if (bus == 0 && PCI_DEVFN(1, 0) == devfn &&
@@ -318,8 +327,8 @@ static int ce4100_conf_write(unsigned in
 }
 
 static const struct pci_raw_ops ce4100_pci_conf = {
-	.read =	ce4100_conf_read,
-	.write = ce4100_conf_write,
+	.read	= ce4100_conf_read,
+	.write	= ce4100_conf_write,
 };
 
 int __init ce4100_pci_init(void)

Re: [patch 3/7] x86/pci/ce4100: Properly lock accessor functions

[Andi Kleen]

Except for the comments the patches look good to me. Haven't tested so far.

Reviewed-by: Andi Kleen <ak@linux.intel.com>

Re: [patch 3/7] x86/pci/ce4100: Properly lock accessor functions

[Bjorn Helgaas]

[+cc linux-pci]

On Thu, Mar 16, 2017 at 10:50:05PM +0100, Thomas Gleixner wrote:
> x86 wants to get rid of the global pci_lock protecting the config space
> accessors so ECAM mode can operate completely lockless, but the CE4100 pci
> code relies on that to protect the simulation registers.

s/pci/PCI/

> Restructure the code so it uses the x86 specific pci_config_lock to
> serialize the inner workings of the CE4100 PCI magic. That allows to remove
> the global locking via pci_lock later.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/x86/pci/ce4100.c |   87 +++++++++++++++++++++++++++-----------------------
>  1 file changed, 48 insertions(+), 39 deletions(-)
> 
> --- a/arch/x86/pci/ce4100.c
> +++ b/arch/x86/pci/ce4100.c
> @@ -65,6 +65,9 @@ struct sim_reg_op {
>  { PCI_DEVFN(device, func), offset, init_op, read_op, write_op,\
>  	{0, SIZE_TO_MASK(size)} },
>  
> +/*
> + * All read/write functions are called with pci_config_lock held.
> + */
>  static void reg_init(struct sim_dev_reg *reg)
>  {
>  	pci_direct_conf1.read(0, 1, reg->dev_func, reg->reg, 4,
> @@ -73,21 +76,13 @@ static void reg_init(struct sim_dev_reg
>  
>  static void reg_read(struct sim_dev_reg *reg, u32 *value)
>  {
> -	unsigned long flags;
> -
> -	raw_spin_lock_irqsave(&pci_config_lock, flags);
>  	*value = reg->sim_reg.value;
> -	raw_spin_unlock_irqrestore(&pci_config_lock, flags);
>  }
>  
>  static void reg_write(struct sim_dev_reg *reg, u32 value)
>  {
> -	unsigned long flags;
> -
> -	raw_spin_lock_irqsave(&pci_config_lock, flags);
>  	reg->sim_reg.value = (value & reg->sim_reg.mask) |
>  		(reg->sim_reg.value & ~reg->sim_reg.mask);
> -	raw_spin_unlock_irqrestore(&pci_config_lock, flags);
>  }
>  
>  static void sata_reg_init(struct sim_dev_reg *reg)
> @@ -117,12 +112,8 @@ static void sata_revid_read(struct sim_d
>  
>  static void reg_noirq_read(struct sim_dev_reg *reg, u32 *value)
>  {
> -	unsigned long flags;
> -
> -	raw_spin_lock_irqsave(&pci_config_lock, flags);
>  	/* force interrupt pin value to 0 */
>  	*value = reg->sim_reg.value & 0xfff00ff;
> -	raw_spin_unlock_irqrestore(&pci_config_lock, flags);
>  }
>  
>  static struct sim_dev_reg bus1_fixups[] = {
> @@ -265,24 +256,33 @@ int bridge_read(unsigned int devfn, int
>  	return retval;
>  }
>  
> -static int ce4100_conf_read(unsigned int seg, unsigned int bus,
> -			    unsigned int devfn, int reg, int len, u32 *value)
> +static int ce4100_bus1_read(unsigned int devfn, int reg, int len, u32 *value)
>  {
> +	unsigned long flags;
>  	int i;
>  
> -	WARN_ON(seg);
> -	if (bus == 1) {
> -		for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
> -			if (bus1_fixups[i].dev_func == devfn &&
> -			    bus1_fixups[i].reg == (reg & ~3) &&
> -			    bus1_fixups[i].read) {
> -				bus1_fixups[i].read(&(bus1_fixups[i]),
> -						    value);
> -				extract_bytes(value, reg, len);
> -				return 0;
> -			}
> +	for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
> +		if (bus1_fixups[i].dev_func == devfn &&
> +		    bus1_fixups[i].reg == (reg & ~3) &&
> +		    bus1_fixups[i].read) {
> +
> +			raw_spin_lock_irqsave(&pci_config_lock, flags);
> +			bus1_fixups[i].read(&(bus1_fixups[i]), value);
> +			raw_spin_unlock_irqrestore(&pci_config_lock, flags);
> +			extract_bytes(value, reg, len);
> +			return 0;
>  		}
>  	}
> +	return -1;
> +}
> +
> +static int ce4100_conf_read(unsigned int seg, unsigned int bus,
> +			    unsigned int devfn, int reg, int len, u32 *value)
> +{
> +	WARN_ON(seg);
> +
> +	if (bus == 1 && !ce4100_bus1_read(devfn, reg, len, value))
> +		return 0;
>  
>  	if (bus == 0 && (PCI_DEVFN(1, 0) == devfn) &&
>  	    !bridge_read(devfn, reg, len, value))
> @@ -291,23 +291,32 @@ static int ce4100_conf_read(unsigned int
>  	return pci_direct_conf1.read(seg, bus, devfn, reg, len, value);
>  }
>  
> -static int ce4100_conf_write(unsigned int seg, unsigned int bus,
> -			     unsigned int devfn, int reg, int len, u32 value)
> +static int ce4100_bus1_write(unsigned int devfn, int reg, int len, u32 value)
>  {
> +	unsigned long flags;
>  	int i;
>  
> -	WARN_ON(seg);
> -	if (bus == 1) {
> -		for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
> -			if (bus1_fixups[i].dev_func == devfn &&
> -			    bus1_fixups[i].reg == (reg & ~3) &&
> -			    bus1_fixups[i].write) {
> -				bus1_fixups[i].write(&(bus1_fixups[i]),
> -						     value);
> -				return 0;
> -			}
> +	for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
> +		if (bus1_fixups[i].dev_func == devfn &&
> +		    bus1_fixups[i].reg == (reg & ~3) &&
> +		    bus1_fixups[i].write) {
> +
> +			raw_spin_lock_irqsave(&pci_config_lock, flags);
> +			bus1_fixups[i].write(&(bus1_fixups[i]), value);
> +			raw_spin_unlock_irqrestore(&pci_config_lock, flags);
> +			return 0;
>  		}
>  	}
> +	return -1;
> +}
> +
> +static int ce4100_conf_write(unsigned int seg, unsigned int bus,
> +			     unsigned int devfn, int reg, int len, u32 value)
> +{
> +	WARN_ON(seg);
> +
> +	if (bus == 1 && !ce4100_bus1_write(devfn, reg, len, value))
> +		return 0;
>  
>  	/* Discard writes to A/V bridge BAR. */
>  	if (bus == 0 && PCI_DEVFN(1, 0) == devfn &&
> @@ -318,8 +327,8 @@ static int ce4100_conf_write(unsigned in
>  }
>  
>  static const struct pci_raw_ops ce4100_pci_conf = {
> -	.read =	ce4100_conf_read,
> -	.write = ce4100_conf_write,
> +	.read	= ce4100_conf_read,
> +	.write	= ce4100_conf_write,
>  };
>  
>  int __init ce4100_pci_init(void)
> 
> 

[tip:x86/platform] x86/PCI/ce4100: Properly lock accessor functions

[tip-bot for Thomas Gleixner]

Commit-ID:  bb290fda879ffd1f6f6b0869bf7335554093f4bd
Gitweb:     http://git.kernel.org/tip/bb290fda879ffd1f6f6b0869bf7335554093f4bd
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 16 Mar 2017 22:50:05 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 28 Jun 2017 22:32:55 +0200

x86/PCI/ce4100: Properly lock accessor functions

x86 wants to get rid of the global pci_lock protecting the config space
accessors so ECAM mode can operate completely lockless, but the CE4100 PCI
code relies on that to protect the simulation registers.

Restructure the code so it uses the x86 specific pci_config_lock to
serialize the inner workings of the CE4100 PCI magic. That allows to remove
the global locking via pci_lock later.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Bjorn Helgaas <helgaas@kernel.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: linux-pci@vger.kernel.org
Link: http://lkml.kernel.org/r/20170316215057.126873574@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/pci/ce4100.c | 87 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 48 insertions(+), 39 deletions(-)

diff --git a/arch/x86/pci/ce4100.c b/arch/x86/pci/ce4100.c
index b914e20..3353b76d 100644
--- a/arch/x86/pci/ce4100.c
+++ b/arch/x86/pci/ce4100.c
@@ -65,6 +65,9 @@ struct sim_reg_op {
 { PCI_DEVFN(device, func), offset, init_op, read_op, write_op,\
 	{0, SIZE_TO_MASK(size)} },
 
+/*
+ * All read/write functions are called with pci_config_lock held.
+ */
 static void reg_init(struct sim_dev_reg *reg)
 {
 	pci_direct_conf1.read(0, 1, reg->dev_func, reg->reg, 4,
@@ -73,21 +76,13 @@ static void reg_init(struct sim_dev_reg *reg)
 
 static void reg_read(struct sim_dev_reg *reg, u32 *value)
 {
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&pci_config_lock, flags);
 	*value = reg->sim_reg.value;
-	raw_spin_unlock_irqrestore(&pci_config_lock, flags);
 }
 
 static void reg_write(struct sim_dev_reg *reg, u32 value)
 {
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&pci_config_lock, flags);
 	reg->sim_reg.value = (value & reg->sim_reg.mask) |
 		(reg->sim_reg.value & ~reg->sim_reg.mask);
-	raw_spin_unlock_irqrestore(&pci_config_lock, flags);
 }
 
 static void sata_reg_init(struct sim_dev_reg *reg)
@@ -117,12 +112,8 @@ static void sata_revid_read(struct sim_dev_reg *reg, u32 *value)
 
 static void reg_noirq_read(struct sim_dev_reg *reg, u32 *value)
 {
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&pci_config_lock, flags);
 	/* force interrupt pin value to 0 */
 	*value = reg->sim_reg.value & 0xfff00ff;
-	raw_spin_unlock_irqrestore(&pci_config_lock, flags);
 }
 
 static struct sim_dev_reg bus1_fixups[] = {
@@ -265,24 +256,33 @@ int bridge_read(unsigned int devfn, int reg, int len, u32 *value)
 	return retval;
 }
 
-static int ce4100_conf_read(unsigned int seg, unsigned int bus,
-			    unsigned int devfn, int reg, int len, u32 *value)
+static int ce4100_bus1_read(unsigned int devfn, int reg, int len, u32 *value)
 {
+	unsigned long flags;
 	int i;
 
-	WARN_ON(seg);
-	if (bus == 1) {
-		for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
-			if (bus1_fixups[i].dev_func == devfn &&
-			    bus1_fixups[i].reg == (reg & ~3) &&
-			    bus1_fixups[i].read) {
-				bus1_fixups[i].read(&(bus1_fixups[i]),
-						    value);
-				extract_bytes(value, reg, len);
-				return 0;
-			}
+	for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
+		if (bus1_fixups[i].dev_func == devfn &&
+		    bus1_fixups[i].reg == (reg & ~3) &&
+		    bus1_fixups[i].read) {
+
+			raw_spin_lock_irqsave(&pci_config_lock, flags);
+			bus1_fixups[i].read(&(bus1_fixups[i]), value);
+			raw_spin_unlock_irqrestore(&pci_config_lock, flags);
+			extract_bytes(value, reg, len);
+			return 0;
 		}
 	}
+	return -1;
+}
+
+static int ce4100_conf_read(unsigned int seg, unsigned int bus,
+			    unsigned int devfn, int reg, int len, u32 *value)
+{
+	WARN_ON(seg);
+
+	if (bus == 1 && !ce4100_bus1_read(devfn, reg, len, value))
+		return 0;
 
 	if (bus == 0 && (PCI_DEVFN(1, 0) == devfn) &&
 	    !bridge_read(devfn, reg, len, value))
@@ -291,23 +291,32 @@ static int ce4100_conf_read(unsigned int seg, unsigned int bus,
 	return pci_direct_conf1.read(seg, bus, devfn, reg, len, value);
 }
 
-static int ce4100_conf_write(unsigned int seg, unsigned int bus,
-			     unsigned int devfn, int reg, int len, u32 value)
+static int ce4100_bus1_write(unsigned int devfn, int reg, int len, u32 value)
 {
+	unsigned long flags;
 	int i;
 
-	WARN_ON(seg);
-	if (bus == 1) {
-		for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
-			if (bus1_fixups[i].dev_func == devfn &&
-			    bus1_fixups[i].reg == (reg & ~3) &&
-			    bus1_fixups[i].write) {
-				bus1_fixups[i].write(&(bus1_fixups[i]),
-						     value);
-				return 0;
-			}
+	for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
+		if (bus1_fixups[i].dev_func == devfn &&
+		    bus1_fixups[i].reg == (reg & ~3) &&
+		    bus1_fixups[i].write) {
+
+			raw_spin_lock_irqsave(&pci_config_lock, flags);
+			bus1_fixups[i].write(&(bus1_fixups[i]), value);
+			raw_spin_unlock_irqrestore(&pci_config_lock, flags);
+			return 0;
 		}
 	}
+	return -1;
+}
+
+static int ce4100_conf_write(unsigned int seg, unsigned int bus,
+			     unsigned int devfn, int reg, int len, u32 value)
+{
+	WARN_ON(seg);
+
+	if (bus == 1 && !ce4100_bus1_write(devfn, reg, len, value))
+		return 0;
 
 	/* Discard writes to A/V bridge BAR. */
 	if (bus == 0 && PCI_DEVFN(1, 0) == devfn &&
@@ -318,8 +327,8 @@ static int ce4100_conf_write(unsigned int seg, unsigned int bus,
 }
 
 static const struct pci_raw_ops ce4100_pci_conf = {
-	.read =	ce4100_conf_read,
-	.write = ce4100_conf_write,
+	.read	= ce4100_conf_read,
+	.write	= ce4100_conf_write,
 };
 
 int __init ce4100_pci_init(void)

[patch 4/7] PCI: Provide Kconfig option for lockless config space accessors

[Thomas Gleixner]

[-- Attachment #1: PCI--Provide-config-option-for-lockless-accessors.patch --]
[-- Type: text/plain, Size: 2349 bytes --]

The generic pci configuration space accessors are globally serialized via
pci_lock. On larger systems this causes massive lock contention when the
configuration space has to be accessed frequently. One such access pattern
is the Intel Uncore performance counter unit.

Provide a kernel config option which can be selected by an architecture
when the low level PCI configuration space accessors in the architecture
use their own serialization or can operate completely lockless.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/pci/Kconfig  |    3 +++
 drivers/pci/access.c |   16 ++++++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -86,6 +86,9 @@ config PCI_ATS
 config PCI_ECAM
 	bool
 
+config PCI_LOCKLESS_CONFIG
+	bool
+
 config PCI_IOV
 	bool "PCI IOV support"
 	depends on PCI
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -25,6 +25,14 @@ DEFINE_RAW_SPINLOCK(pci_lock);
 #define PCI_word_BAD (pos & 1)
 #define PCI_dword_BAD (pos & 3)
 
+#ifdef CONFIG_PCI_LOCKLESS_CONFIG
+# define pci_lock_config(f)	do { (void)(f); } while (0)
+# define pci_unlock_config(f)	do { (void)(f); } while (0)
+#else
+# define pci_lock_config(f)	raw_spin_lock_irqsave(&pci_lock, f)
+# define pci_unlock_config(f)	raw_spin_unlock_irqrestore(&pci_lock, f)
+#endif
+
 #define PCI_OP_READ(size, type, len) \
 int pci_bus_read_config_##size \
 	(struct pci_bus *bus, unsigned int devfn, int pos, type *value)	\
@@ -33,10 +41,10 @@ int pci_bus_read_config_##size \
 	unsigned long flags;						\
 	u32 data = 0;							\
 	if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER;	\
-	raw_spin_lock_irqsave(&pci_lock, flags);			\
+	pci_lock_config(flags);						\
 	res = bus->ops->read(bus, devfn, pos, len, &data);		\
 	*value = (type)data;						\
-	raw_spin_unlock_irqrestore(&pci_lock, flags);		\
+	pci_unlock_config(flags);					\
 	return res;							\
 }
 
@@ -47,9 +55,9 @@ int pci_bus_write_config_##size \
 	int res;							\
 	unsigned long flags;						\
 	if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER;	\
-	raw_spin_lock_irqsave(&pci_lock, flags);			\
+	pci_lock_config(flags);						\
 	res = bus->ops->write(bus, devfn, pos, len, value);		\
-	raw_spin_unlock_irqrestore(&pci_lock, flags);		\
+	pci_unlock_config(flags);					\
 	return res;							\
 }
 

Re: [patch 4/7] PCI: Provide Kconfig option for lockless config space accessors

[Bjorn Helgaas]

[+cc linux-pci]

On Thu, Mar 16, 2017 at 10:50:06PM +0100, Thomas Gleixner wrote:
> The generic pci configuration space accessors are globally serialized via
> pci_lock. On larger systems this causes massive lock contention when the
> configuration space has to be accessed frequently. One such access pattern
> is the Intel Uncore performance counter unit.

s/pci/PCI/ above.

> Provide a kernel config option which can be selected by an architecture
> when the low level PCI configuration space accessors in the architecture
> use their own serialization or can operate completely lockless.

The arch/x86/pci/common.c comment:

  /*
   * This interrupt-safe spinlock protects all accesses to PCI
   * configuration space.
   */
  DEFINE_RAW_SPINLOCK(pci_config_lock);

is no longer quite correct.

I think the raw_pci_read() and raw_pci_write() implementations are
such that we use the old locked accessors for the first 256 bytes,
even when ECAM is available.  Not necessarily a problem, just an
observation.  I guess the uncore PMU registers are in the extended
config space.

> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  drivers/pci/Kconfig  |    3 +++
>  drivers/pci/access.c |   16 ++++++++++++----
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -86,6 +86,9 @@ config PCI_ATS
>  config PCI_ECAM
>  	bool
>  
> +config PCI_LOCKLESS_CONFIG
> +	bool

It's conceivable that this could be a per-host bridge property, but
not worth worrying about for now.

>  config PCI_IOV
>  	bool "PCI IOV support"
>  	depends on PCI
> --- a/drivers/pci/access.c
> +++ b/drivers/pci/access.c
> @@ -25,6 +25,14 @@ DEFINE_RAW_SPINLOCK(pci_lock);
>  #define PCI_word_BAD (pos & 1)
>  #define PCI_dword_BAD (pos & 3)
>  
> +#ifdef CONFIG_PCI_LOCKLESS_CONFIG
> +# define pci_lock_config(f)	do { (void)(f); } while (0)
> +# define pci_unlock_config(f)	do { (void)(f); } while (0)
> +#else
> +# define pci_lock_config(f)	raw_spin_lock_irqsave(&pci_lock, f)
> +# define pci_unlock_config(f)	raw_spin_unlock_irqrestore(&pci_lock, f)
> +#endif
> +
>  #define PCI_OP_READ(size, type, len) \
>  int pci_bus_read_config_##size \
>  	(struct pci_bus *bus, unsigned int devfn, int pos, type *value)	\
> @@ -33,10 +41,10 @@ int pci_bus_read_config_##size \
>  	unsigned long flags;						\
>  	u32 data = 0;							\
>  	if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER;	\
> -	raw_spin_lock_irqsave(&pci_lock, flags);			\
> +	pci_lock_config(flags);						\
>  	res = bus->ops->read(bus, devfn, pos, len, &data);		\
>  	*value = (type)data;						\
> -	raw_spin_unlock_irqrestore(&pci_lock, flags);		\
> +	pci_unlock_config(flags);					\
>  	return res;							\
>  }
>  
> @@ -47,9 +55,9 @@ int pci_bus_write_config_##size \
>  	int res;							\
>  	unsigned long flags;						\
>  	if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER;	\
> -	raw_spin_lock_irqsave(&pci_lock, flags);			\
> +	pci_lock_config(flags);						\
>  	res = bus->ops->write(bus, devfn, pos, len, value);		\
> -	raw_spin_unlock_irqrestore(&pci_lock, flags);		\
> +	pci_unlock_config(flags);					\
>  	return res;							\
>  }
>  
> 
> 

Re: [patch 4/7] PCI: Provide Kconfig option for lockless config space accessors

[Thomas Gleixner]

On Tue, 27 Jun 2017, Bjorn Helgaas wrote:
> On Thu, Mar 16, 2017 at 10:50:06PM +0100, Thomas Gleixner wrote:
> > Provide a kernel config option which can be selected by an architecture
> > when the low level PCI configuration space accessors in the architecture
> > use their own serialization or can operate completely lockless.
> 
> The arch/x86/pci/common.c comment:
> 
>   /*
>    * This interrupt-safe spinlock protects all accesses to PCI
>    * configuration space.
>    */
>   DEFINE_RAW_SPINLOCK(pci_config_lock);
> 
> is no longer quite correct.

Yes. I updated it to:

 * This interrupt-safe spinlock protects all accesses to PCI configuration
 * space, except for the mmconfig (ECAM) based operations.

> I think the raw_pci_read() and raw_pci_write() implementations are
> such that we use the old locked accessors for the first 256 bytes,
> even when ECAM is available.  Not necessarily a problem, just an
> observation.

No, we actually don't after the next patch, which replaces the
pci_root_ops.read/write function pointers with the lockless ECAM accessors
after the end of the initialization, if none of the special quirks replaced
raw_pci_ext_ops and mmconfig/ECAM is available.

> I guess the uncore PMU registers are in the extended config space.

Yes.

Thanks,

	tglx

[tip:x86/platform] PCI: Provide Kconfig option for lockless config space accessors

[tip-bot for Thomas Gleixner]

Commit-ID:  714fe383d6c9bd95d0d2cad8cbeff3688342d025
Gitweb:     http://git.kernel.org/tip/714fe383d6c9bd95d0d2cad8cbeff3688342d025
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 16 Mar 2017 22:50:06 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 28 Jun 2017 22:32:56 +0200

PCI: Provide Kconfig option for lockless config space accessors

The generic PCI configuration space accessors are globally serialized via
pci_lock. On larger systems this causes massive lock contention when the
configuration space has to be accessed frequently. One such access pattern
is the Intel Uncore performance counter unit.

Provide a kernel config option which can be selected by an architecture
when the low level PCI configuration space accessors in the architecture
use their own serialization or can operate completely lockless.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Bjorn Helgaas <helgaas@kernel.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: linux-pci@vger.kernel.org
Link: http://lkml.kernel.org/r/20170316215057.205961140@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 drivers/pci/Kconfig  |  3 +++
 drivers/pci/access.c | 16 ++++++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index e0cacb7..c32a77f 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -86,6 +86,9 @@ config PCI_ATS
 config PCI_ECAM
 	bool
 
+config PCI_LOCKLESS_CONFIG
+	bool
+
 config PCI_IOV
 	bool "PCI IOV support"
 	depends on PCI
diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index c80e37a..913d672 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -25,6 +25,14 @@ DEFINE_RAW_SPINLOCK(pci_lock);
 #define PCI_word_BAD (pos & 1)
 #define PCI_dword_BAD (pos & 3)
 
+#ifdef CONFIG_PCI_LOCKLESS_CONFIG
+# define pci_lock_config(f)	do { (void)(f); } while (0)
+# define pci_unlock_config(f)	do { (void)(f); } while (0)
+#else
+# define pci_lock_config(f)	raw_spin_lock_irqsave(&pci_lock, f)
+# define pci_unlock_config(f)	raw_spin_unlock_irqrestore(&pci_lock, f)
+#endif
+
 #define PCI_OP_READ(size, type, len) \
 int pci_bus_read_config_##size \
 	(struct pci_bus *bus, unsigned int devfn, int pos, type *value)	\
@@ -33,10 +41,10 @@ int pci_bus_read_config_##size \
 	unsigned long flags;						\
 	u32 data = 0;							\
 	if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER;	\
-	raw_spin_lock_irqsave(&pci_lock, flags);			\
+	pci_lock_config(flags);						\
 	res = bus->ops->read(bus, devfn, pos, len, &data);		\
 	*value = (type)data;						\
-	raw_spin_unlock_irqrestore(&pci_lock, flags);		\
+	pci_unlock_config(flags);					\
 	return res;							\
 }
 
@@ -47,9 +55,9 @@ int pci_bus_write_config_##size \
 	int res;							\
 	unsigned long flags;						\
 	if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER;	\
-	raw_spin_lock_irqsave(&pci_lock, flags);			\
+	pci_lock_config(flags);						\
 	res = bus->ops->write(bus, devfn, pos, len, value);		\
-	raw_spin_unlock_irqrestore(&pci_lock, flags);		\
+	pci_unlock_config(flags);					\
 	return res;							\
 }
 

[patch 5/7] x86/pci: Select CONFIG_PCI_LOCKLESS_CONFIG

[Thomas Gleixner]

[-- Attachment #1: x86-pci--Select-CONFIG_PCI_LOCKLESS_CONFIG.patch --]
[-- Type: text/plain, Size: 583 bytes --]

All x86 PCI configuration space accessors have either their own
serialization or can operate completely lockless (ECAM).

Disable the global lock in the generic PCI configuration space accessors.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/Kconfig |    1 +
 1 file changed, 1 insertion(+)

--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -165,6 +165,7 @@ config X86
 	select HAVE_UNSTABLE_SCHED_CLOCK
 	select HAVE_USER_RETURN_NOTIFIER
 	select IRQ_FORCED_THREADING
+	select PCI_LOCKLESS_CONFIG
 	select PERF_EVENTS
 	select RTC_LIB
 	select RTC_MC146818_LIB

[tip:x86/platform] x86/PCI: Select CONFIG_PCI_LOCKLESS_CONFIG

[tip-bot for Thomas Gleixner]

Commit-ID:  df65c1bcd9b7b639177a5a15da1b8dc3bee4f5fa
Gitweb:     http://git.kernel.org/tip/df65c1bcd9b7b639177a5a15da1b8dc3bee4f5fa
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 16 Mar 2017 22:50:07 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 28 Jun 2017 22:32:56 +0200

x86/PCI: Select CONFIG_PCI_LOCKLESS_CONFIG

All x86 PCI configuration space accessors have either their own
serialization or can operate completely lockless (ECAM).

Disable the global lock in the generic PCI configuration space accessors.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Bjorn Helgaas <helgaas@kernel.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: linux-pci@vger.kernel.org
Link: http://lkml.kernel.org/r/20170316215057.295079391@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/Kconfig      | 1 +
 arch/x86/pci/common.c | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0efb4c9..0652c9f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -166,6 +166,7 @@ config X86
 	select HAVE_UNSTABLE_SCHED_CLOCK
 	select HAVE_USER_RETURN_NOTIFIER
 	select IRQ_FORCED_THREADING
+	select PCI_LOCKLESS_CONFIG
 	select PERF_EVENTS
 	select RTC_LIB
 	select RTC_MC146818_LIB
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 190e718..cfd1a89 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -75,8 +75,8 @@ struct pci_ops pci_root_ops = {
 };
 
 /*
- * This interrupt-safe spinlock protects all accesses to PCI
- * configuration space.
+ * This interrupt-safe spinlock protects all accesses to PCI configuration
+ * space, except for the mmconfig (ECAM) based operations.
  */
 DEFINE_RAW_SPINLOCK(pci_config_lock);
 

[patch 6/7] x86/pci/mmcfg: Include 32/64 bit code into shared code

[Thomas Gleixner]

[-- Attachment #1: x86-pci-mmcfg--Include-32-64-bit-code-into-shared-code.patch --]
[-- Type: text/plain, Size: 2936 bytes --]

Preparatory patch so the shared mmconfig code can access static functions
in the 32/64bit specific mmconfig implementations.

This is useful for the upcoming implementation of mmconfig based (ECAM) PCI
configuration space access for both the legacy part (0-0xff) and the
extended part (0x100-0xfff). Being able to access the low level mmconfig
read/write functions directly avoids extra indirections.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/pci/Makefile          |    2 +-
 arch/x86/pci/mmconfig-shared.c |    6 ++++++
 arch/x86/pci/mmconfig_32.c     |   12 ++----------
 arch/x86/pci/mmconfig_64.c     |   16 ++++------------
 4 files changed, 13 insertions(+), 23 deletions(-)

--- a/arch/x86/pci/Makefile
+++ b/arch/x86/pci/Makefile
@@ -1,7 +1,7 @@
 obj-y				:= i386.o init.o
 
 obj-$(CONFIG_PCI_BIOS)		+= pcbios.o
-obj-$(CONFIG_PCI_MMCONFIG)	+= mmconfig_$(BITS).o direct.o mmconfig-shared.o
+obj-$(CONFIG_PCI_MMCONFIG)	+= direct.o mmconfig-shared.o
 obj-$(CONFIG_PCI_DIRECT)	+= direct.o
 obj-$(CONFIG_PCI_OLPC)		+= olpc.o
 obj-$(CONFIG_PCI_XEN)		+= xen.o
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -24,6 +24,12 @@
 
 #define PREFIX "PCI: "
 
+#ifdef CONFIG_X86_64
+# include "mmconfig_64.c"
+#else
+# include "mmconfig_32.c"
+#endif
+
 /* Indicate if the mmcfg resources have been placed into the resource table. */
 static bool pci_mmcfg_running_state;
 static bool pci_mmcfg_arch_init_failed;
--- a/arch/x86/pci/mmconfig_32.c
+++ b/arch/x86/pci/mmconfig_32.c
@@ -3,18 +3,10 @@
  * Copyright (C) 2004 Intel Corp.
  *
  * This code is released under the GNU General Public License version 2.
+ *
+ * Low-level direct PCI config space access via MMCONFIG, 32bit version.
  */
 
-/*
- * mmconfig.c - Low-level direct PCI config space access via MMCONFIG
- */
-
-#include <linux/pci.h>
-#include <linux/init.h>
-#include <linux/rcupdate.h>
-#include <asm/e820.h>
-#include <asm/pci_x86.h>
-
 /* Assume systems with more busses have correct MCFG */
 #define mmcfg_virt_addr ((void __iomem *) fix_to_virt(FIX_PCIE_MCFG))
 
--- a/arch/x86/pci/mmconfig_64.c
+++ b/arch/x86/pci/mmconfig_64.c
@@ -1,20 +1,12 @@
 /*
- * mmconfig.c - Low-level direct PCI config space access via MMCONFIG
+ * Low-level direct PCI config space access via MMCONFIG
  *
  * This is an 64bit optimized version that always keeps the full mmconfig
  * space mapped. This allows lockless config space operation.
+ *
+ * This file is included into the 32/64bit shared code so the read/write
+ * accessors are directly reachable there.
  */
-
-#include <linux/pci.h>
-#include <linux/init.h>
-#include <linux/acpi.h>
-#include <linux/bitmap.h>
-#include <linux/rcupdate.h>
-#include <asm/e820.h>
-#include <asm/pci_x86.h>
-
-#define PREFIX "PCI: "
-
 static char __iomem *pci_dev_base(unsigned int seg, unsigned int bus, unsigned int devfn)
 {
 	struct pci_mmcfg_region *cfg = pci_mmconfig_lookup(seg, bus);

Re: [patch 6/7] x86/pci/mmcfg: Include 32/64 bit code into shared code

[Andi Kleen]

> --- a/arch/x86/pci/mmconfig-shared.c
> +++ b/arch/x86/pci/mmconfig-shared.c
> @@ -24,6 +24,12 @@
>  
>  #define PREFIX "PCI: "
>  
> +#ifdef CONFIG_X86_64
> +# include "mmconfig_64.c"
> +#else
> +# include "mmconfig_32.c"
> +#endif

This seems like a bad hack. If you want to access something from
multiple files just make it global, don't play preprocessor tricks.

-Andi

Re: [patch 6/7] x86/pci/mmcfg: Include 32/64 bit code into shared code

[Thomas Gleixner]

On Thu, 16 Mar 2017, Andi Kleen wrote:
> > --- a/arch/x86/pci/mmconfig-shared.c
> > +++ b/arch/x86/pci/mmconfig-shared.c
> > @@ -24,6 +24,12 @@
> >  
> >  #define PREFIX "PCI: "
> >  
> > +#ifdef CONFIG_X86_64
> > +# include "mmconfig_64.c"
> > +#else
> > +# include "mmconfig_32.c"
> > +#endif
> 
> This seems like a bad hack. If you want to access something from
> multiple files just make it global, don't play preprocessor tricks.

That's a leftover from an earlier iteration of the patches, where including
the code made a significant text size difference. Forgot to recheck. With
the current version the difference is minimal. I'll drop it.

Thanks,

	tglx

[patch 7/7] x86/pci/mmcfg: Switch to ECAM config mode if possible

[Thomas Gleixner]

[-- Attachment #1: x86-pci-mmcfg--Switch-to-ECAM-config-mode-if-possible.patch --]
[-- Type: text/plain, Size: 4282 bytes --]

To allow lockless access to the whole PCI configuration space the mmconfig
based accessor functions need to be propagated to the pci_root_ops.

Unfortunatly this cannot be done before the PCI subsystem initialization
happens even if mmconfig access is already available. The reason is that
some of the special platform PCI implementations must be able to overrule
that setting before further accesses happen.

The earliest possible point is after x86_init.pci.init() has been run. This
is at a point in the boot process where nothing actually uses the PCI
devices so the accessor function pointers can be updated lockless w/o risk.

The switch to full ECAM mode depends on the availability of mmconfig and
unchanged default accessors.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/pci_x86.h |   15 +++++++--------
 arch/x86/pci/common.c          |   16 ++++++++++++++++
 arch/x86/pci/legacy.c          |    1 +
 arch/x86/pci/mmconfig-shared.c |   30 ++++++++++++++++++++++++++++++
 4 files changed, 54 insertions(+), 8 deletions(-)

--- a/arch/x86/include/asm/pci_x86.h
+++ b/arch/x86/include/asm/pci_x86.h
@@ -46,20 +46,14 @@ enum pci_bf_sort_state {
 	pci_dmi_bf,
 };
 
-/* pci-i386.c */
-
 void pcibios_resource_survey(void);
 void pcibios_set_cache_line_size(void);
 
-/* pci-pc.c */
-
 extern int pcibios_last_bus;
 extern struct pci_ops pci_root_ops;
 
 void pcibios_scan_specific_bus(int busn);
 
-/* pci-irq.c */
-
 struct irq_info {
 	u8 bus, devfn;			/* Bus, device and function */
 	struct {
@@ -120,11 +114,10 @@ extern void __init dmi_check_skip_isa_al
 extern int __init pci_acpi_init(void);
 extern void __init pcibios_irq_init(void);
 extern int __init pcibios_init(void);
+extern void __init pcibios_select_ops(void);
 extern int pci_legacy_init(void);
 extern void pcibios_fixup_irqs(void);
 
-/* pci-mmconfig.c */
-
 /* "PCI MMCONFIG %04x [bus %02x-%02x]" */
 #define PCI_MMCFG_RESOURCE_NAME_LEN (22 + 4 + 2 + 2)
 
@@ -139,6 +132,12 @@ struct pci_mmcfg_region {
 	char name[PCI_MMCFG_RESOURCE_NAME_LEN];
 };
 
+#ifdef CONFIG_PCI_MMCONFIG
+extern void __init pci_mmcfg_select_ops(void);
+#else
+static inline void pci_mmcfg_select_ops(void) { }
+#endif
+
 extern int __init pci_mmcfg_arch_init(void);
 extern void __init pci_mmcfg_arch_free(void);
 extern int pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg);
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -157,6 +157,22 @@ static void pcibios_fixup_device_resourc
 }
 
 /*
+ * Called after the last possible modification to raw_pci_[ext_]ops.
+ *
+ * Verify that root_pci_ops have not been overwritten by any implementation
+ * of x86_init.pci.arch_init() and x86_init.pci.init().
+ *
+ * If not, let the mmconfig code decide whether the ops can be switched
+ * over to the ECAM accessor functions.
+ */
+void __init pcibios_select_ops(void)
+{
+	if (pci_root_ops.read != pci_read || pci_root_ops.write != pci_write)
+		return;
+	pci_mmcfg_select_ops();
+}
+
+/*
  *  Called after each bus is probed, but before its children
  *  are examined.
  */
--- a/arch/x86/pci/legacy.c
+++ b/arch/x86/pci/legacy.c
@@ -65,6 +65,7 @@ static int __init pci_subsys_init(void)
 		}
 	}
 
+	pcibios_select_ops();
 	pcibios_fixup_peer_bridges();
 	x86_init.pci.init_irq();
 	pcibios_init();
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -822,3 +822,33 @@ int pci_mmconfig_delete(u16 seg, u8 star
 
 	return -ENOENT;
 }
+
+static int pci_ecam_read(struct pci_bus *bus, unsigned int devfn, int reg,
+			 int size, u32 *value)
+{
+	return pci_mmcfg_read(pci_domain_nr(bus), bus->number, devfn, reg,
+			      size, value);
+}
+
+static int pci_ecam_write(struct pci_bus *bus, unsigned int devfn, int reg,
+			  int size, u32 value)
+{
+	return pci_mmcfg_write(pci_domain_nr(bus), bus->number, devfn, reg,
+			       size, value);
+}
+
+void __init pci_mmcfg_select_ops(void)
+{
+	if (raw_pci_ext_ops != &pci_mmcfg)
+		return;
+
+	/*
+	 * The pointer to root_pci_ops has been handed in to ACPI already
+	 * and is already set in the busses.
+	 *
+	 * Switch the functions over to ECAM for all config space accesses.
+	 */
+	pci_root_ops.read = pci_ecam_read;
+	pci_root_ops.write = pci_ecam_write;
+	pr_info("PCI: Switch to ECAM configuration mode\n");
+}

Re: [patch 7/7] x86/pci/mmcfg: Switch to ECAM config mode if possible

[Andi Kleen]

> +	/*
> +	 * The pointer to root_pci_ops has been handed in to ACPI already
> +	 * and is already set in the busses.
> +	 *
> +	 * Switch the functions over to ECAM for all config space accesses.
> +	 */
> +	pci_root_ops.read = pci_ecam_read;
> +	pci_root_ops.write = pci_ecam_write;
> +	pr_info("PCI: Switch to ECAM configuration mode\n");

That patch is fine, but it's generally called MMCONFIG (don't know
where this ECAM term comes from). So please use MMCONFIG or MCFG everywhere,
not ECAM.

-Andi

Re: [patch 7/7] x86/pci/mmcfg: Switch to ECAM config mode if possible

[Thomas Gleixner]

On Thu, 16 Mar 2017, Andi Kleen wrote:

> > +	/*
> > +	 * The pointer to root_pci_ops has been handed in to ACPI already
> > +	 * and is already set in the busses.
> > +	 *
> > +	 * Switch the functions over to ECAM for all config space accesses.
> > +	 */
> > +	pci_root_ops.read = pci_ecam_read;
> > +	pci_root_ops.write = pci_ecam_write;
> > +	pr_info("PCI: Switch to ECAM configuration mode\n");
> 
> That patch is fine, but it's generally called MMCONFIG (don't know
> where this ECAM term comes from).

ECAM is the official name for the memory mapped configuration mechanism
according to the PCI express specification.

> So please use MMCONFIG or MCFG everywhere, not ECAM.

While I prefer using names which match specifications, I let Bjorn decide
on that one.

Thanks,

	tglx

Re: [patch 7/7] x86/pci/mmcfg: Switch to ECAM config mode if possible

[Bjorn Helgaas]

[+cc linux-pci]

On Fri, Mar 17, 2017 at 07:15:21AM +0100, Thomas Gleixner wrote:
> On Thu, 16 Mar 2017, Andi Kleen wrote:
> 
> > > +	/*
> > > +	 * The pointer to root_pci_ops has been handed in to ACPI already
> > > +	 * and is already set in the busses.
> > > +	 *
> > > +	 * Switch the functions over to ECAM for all config space accesses.
> > > +	 */
> > > +	pci_root_ops.read = pci_ecam_read;
> > > +	pci_root_ops.write = pci_ecam_write;
> > > +	pr_info("PCI: Switch to ECAM configuration mode\n");
> > 
> > That patch is fine, but it's generally called MMCONFIG (don't know
> > where this ECAM term comes from).
> 
> ECAM is the official name for the memory mapped configuration mechanism
> according to the PCI express specification.
> 
> > So please use MMCONFIG or MCFG everywhere, not ECAM.
> 
> While I prefer using names which match specifications, I let Bjorn decide
> on that one.

I've definitely seen "MMCONFIG" used many places, especially
internally.  I've been trying to use names that correspond to the
public specs in an arch-independent way, so here's my thinking:

"MCFG" is the ACPI table name and appears in the PCI Firmware spec.
The x86-specific MCFG parser is called arch/x86/pci/mmconfig-shared.c,
but we named the arch-independent parser drivers/acpi/pci_mcfg.c.

"MMCONFIG" appears only in an implementation note in the PCI Firmware
spec r3.2, sec 4.1.4.  The x86 ECAM/MMCONFIG/MCFG code is in
arch/x86/pci/mmconfig*.  I wouldn't name it that way today because I
don't see "MMCONFIG" used much in public specs, but it doesn't seem
worth changing to me.

The PCIe spec doesn't mention MCFG or MMCONFIG, but it uses "ECAM" many
times, so that's what we used for the arch-independent code in
drivers/pci/ecam.c.

Bottom line, I'm fine with the names in this patch as-is.

Bjorn

[tip:x86/platform] x86/PCI/mmcfg: Switch to ECAM config mode if possible

[tip-bot for Thomas Gleixner]

Commit-ID:  5d381c2e053918bd67c2d1cc50fc73c35bd547f7
Gitweb:     http://git.kernel.org/tip/5d381c2e053918bd67c2d1cc50fc73c35bd547f7
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 16 Mar 2017 22:50:09 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 28 Jun 2017 22:32:57 +0200

x86/PCI/mmcfg: Switch to ECAM config mode if possible

To allow lockless access to the whole PCI configuration space the mmconfig
based accessor functions need to be propagated to the pci_root_ops.

Unfortunatly this cannot be done before the PCI subsystem initialization
happens even if mmconfig access is already available. The reason is that
some of the special platform PCI implementations must be able to overrule
that setting before further accesses happen.

The earliest possible point is after x86_init.pci.init() has been run. This
is at a point in the boot process where nothing actually uses the PCI
devices so the accessor function pointers can be updated lockless w/o risk.

The switch to full ECAM mode depends on the availability of mmconfig and
unchanged default accessors.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Bjorn Helgaas <helgaas@kernel.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: linux-pci@vger.kernel.org
Link: http://lkml.kernel.org/r/20170316215057.452220163@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/include/asm/pci_x86.h | 15 +++++++--------
 arch/x86/pci/common.c          | 16 ++++++++++++++++
 arch/x86/pci/legacy.c          |  1 +
 arch/x86/pci/mmconfig-shared.c | 30 ++++++++++++++++++++++++++++++
 4 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h
index 9f1b21f..ad518a9 100644
--- a/arch/x86/include/asm/pci_x86.h
+++ b/arch/x86/include/asm/pci_x86.h
@@ -48,20 +48,14 @@ enum pci_bf_sort_state {
 	pci_dmi_bf,
 };
 
-/* pci-i386.c */
-
 void pcibios_resource_survey(void);
 void pcibios_set_cache_line_size(void);
 
-/* pci-pc.c */
-
 extern int pcibios_last_bus;
 extern struct pci_ops pci_root_ops;
 
 void pcibios_scan_specific_bus(int busn);
 
-/* pci-irq.c */
-
 struct irq_info {
 	u8 bus, devfn;			/* Bus, device and function */
 	struct {
@@ -122,11 +116,10 @@ extern void __init dmi_check_skip_isa_align(void);
 extern int __init pci_acpi_init(void);
 extern void __init pcibios_irq_init(void);
 extern int __init pcibios_init(void);
+extern void __init pcibios_select_ops(void);
 extern int pci_legacy_init(void);
 extern void pcibios_fixup_irqs(void);
 
-/* pci-mmconfig.c */
-
 /* "PCI MMCONFIG %04x [bus %02x-%02x]" */
 #define PCI_MMCFG_RESOURCE_NAME_LEN (22 + 4 + 2 + 2)
 
@@ -141,6 +134,12 @@ struct pci_mmcfg_region {
 	char name[PCI_MMCFG_RESOURCE_NAME_LEN];
 };
 
+#ifdef CONFIG_PCI_MMCONFIG
+extern void __init pci_mmcfg_select_ops(void);
+#else
+static inline void pci_mmcfg_select_ops(void) { }
+#endif
+
 extern int __init pci_mmcfg_arch_init(void);
 extern void __init pci_mmcfg_arch_free(void);
 extern int pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg);
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index cfd1a89..81e4d21 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -157,6 +157,22 @@ static void pcibios_fixup_device_resources(struct pci_dev *dev)
 }
 
 /*
+ * Called after the last possible modification to raw_pci_[ext_]ops.
+ *
+ * Verify that root_pci_ops have not been overwritten by any implementation
+ * of x86_init.pci.arch_init() and x86_init.pci.init().
+ *
+ * If not, let the mmconfig code decide whether the ops can be switched
+ * over to the ECAM accessor functions.
+ */
+void __init pcibios_select_ops(void)
+{
+	if (pci_root_ops.read != pci_read || pci_root_ops.write != pci_write)
+		return;
+	pci_mmcfg_select_ops();
+}
+
+/*
  *  Called after each bus is probed, but before its children
  *  are examined.
  */
diff --git a/arch/x86/pci/legacy.c b/arch/x86/pci/legacy.c
index 1cb01ab..80ea40e 100644
--- a/arch/x86/pci/legacy.c
+++ b/arch/x86/pci/legacy.c
@@ -65,6 +65,7 @@ static int __init pci_subsys_init(void)
 		}
 	}
 
+	pcibios_select_ops();
 	pcibios_fixup_peer_bridges();
 	x86_init.pci.init_irq();
 	pcibios_init();
diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index d1b47d5..6af6351 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -816,3 +816,33 @@ int pci_mmconfig_delete(u16 seg, u8 start, u8 end)
 
 	return -ENOENT;
 }
+
+static int pci_ecam_read(struct pci_bus *bus, unsigned int devfn, int reg,
+			 int size, u32 *value)
+{
+	return pci_mmcfg_read(pci_domain_nr(bus), bus->number, devfn, reg,
+			      size, value);
+}
+
+static int pci_ecam_write(struct pci_bus *bus, unsigned int devfn, int reg,
+			  int size, u32 value)
+{
+	return pci_mmcfg_write(pci_domain_nr(bus), bus->number, devfn, reg,
+			       size, value);
+}
+
+void __init pci_mmcfg_select_ops(void)
+{
+	if (raw_pci_ext_ops != &pci_mmcfg)
+		return;
+
+	/*
+	 * The pointer to root_pci_ops has been handed in to ACPI already
+	 * and is already set in the busses.
+	 *
+	 * Switch the functions over to ECAM for all config space accesses.
+	 */
+	pci_root_ops.read = pci_ecam_read;
+	pci_root_ops.write = pci_ecam_write;
+	pr_info("PCI: Switch to ECAM configuration mode\n");
+}

[tip:x86/platform] x86/PCI/mmcfg: Switch to ECAM config mode if possible

[tip-bot for Thomas Gleixner]

Commit-ID:  b5b0f00c760b6e9673ab79b88ede2f3c7a039f74
Gitweb:     http://git.kernel.org/tip/b5b0f00c760b6e9673ab79b88ede2f3c7a039f74
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 16 Mar 2017 22:50:09 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 29 Jun 2017 08:41:54 +0200

x86/PCI/mmcfg: Switch to ECAM config mode if possible

To allow lockless access to the whole PCI configuration space the mmconfig
based accessor functions need to be propagated to the pci_root_ops.

Unfortunatly this cannot be done before the PCI subsystem initialization
happens even if mmconfig access is already available. The reason is that
some of the special platform PCI implementations must be able to overrule
that setting before further accesses happen.

The earliest possible point is after x86_init.pci.init() has been run. This
is at a point in the boot process where nothing actually uses the PCI
devices so the accessor function pointers can be updated lockless w/o risk.

The switch to full ECAM mode depends on the availability of mmconfig and
unchanged default accessors.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Bjorn Helgaas <helgaas@kernel.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: linux-pci@vger.kernel.org
Link: http://lkml.kernel.org/r/20170316215057.452220163@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/pci_x86.h | 20 ++++++++++++--------
 arch/x86/pci/common.c          | 16 ++++++++++++++++
 arch/x86/pci/legacy.c          |  1 +
 arch/x86/pci/mmconfig-shared.c | 30 ++++++++++++++++++++++++++++++
 arch/x86/pci/mmconfig_32.c     |  8 ++++----
 arch/x86/pci/mmconfig_64.c     |  8 ++++----
 6 files changed, 67 insertions(+), 16 deletions(-)

diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h
index 9f1b21f..65e1303 100644
--- a/arch/x86/include/asm/pci_x86.h
+++ b/arch/x86/include/asm/pci_x86.h
@@ -48,20 +48,14 @@ enum pci_bf_sort_state {
 	pci_dmi_bf,
 };
 
-/* pci-i386.c */
-
 void pcibios_resource_survey(void);
 void pcibios_set_cache_line_size(void);
 
-/* pci-pc.c */
-
 extern int pcibios_last_bus;
 extern struct pci_ops pci_root_ops;
 
 void pcibios_scan_specific_bus(int busn);
 
-/* pci-irq.c */
-
 struct irq_info {
 	u8 bus, devfn;			/* Bus, device and function */
 	struct {
@@ -122,11 +116,10 @@ extern void __init dmi_check_skip_isa_align(void);
 extern int __init pci_acpi_init(void);
 extern void __init pcibios_irq_init(void);
 extern int __init pcibios_init(void);
+extern void __init pcibios_select_ops(void);
 extern int pci_legacy_init(void);
 extern void pcibios_fixup_irqs(void);
 
-/* pci-mmconfig.c */
-
 /* "PCI MMCONFIG %04x [bus %02x-%02x]" */
 #define PCI_MMCFG_RESOURCE_NAME_LEN (22 + 4 + 2 + 2)
 
@@ -141,6 +134,12 @@ struct pci_mmcfg_region {
 	char name[PCI_MMCFG_RESOURCE_NAME_LEN];
 };
 
+#ifdef CONFIG_PCI_MMCONFIG
+extern void __init pci_mmcfg_select_ops(void);
+#else
+static inline void pci_mmcfg_select_ops(void) { }
+#endif
+
 extern int __init pci_mmcfg_arch_init(void);
 extern void __init pci_mmcfg_arch_free(void);
 extern int pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg);
@@ -197,6 +196,11 @@ static inline void mmio_config_writel(void __iomem *pos, u32 val)
 	asm volatile("movl %%eax,(%1)" : : "a" (val), "r" (pos) : "memory");
 }
 
+int pci_mmcfg_read(unsigned int seg, unsigned int bus,
+		   unsigned int devfn, int reg, int len, u32 *value);
+int pci_mmcfg_write(unsigned int seg, unsigned int bus,
+		    unsigned int devfn, int reg, int len, u32 value);
+
 #ifdef CONFIG_PCI
 # ifdef CONFIG_ACPI
 #  define x86_default_pci_init		pci_acpi_init
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index cfd1a89..81e4d21 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -157,6 +157,22 @@ static void pcibios_fixup_device_resources(struct pci_dev *dev)
 }
 
 /*
+ * Called after the last possible modification to raw_pci_[ext_]ops.
+ *
+ * Verify that root_pci_ops have not been overwritten by any implementation
+ * of x86_init.pci.arch_init() and x86_init.pci.init().
+ *
+ * If not, let the mmconfig code decide whether the ops can be switched
+ * over to the ECAM accessor functions.
+ */
+void __init pcibios_select_ops(void)
+{
+	if (pci_root_ops.read != pci_read || pci_root_ops.write != pci_write)
+		return;
+	pci_mmcfg_select_ops();
+}
+
+/*
  *  Called after each bus is probed, but before its children
  *  are examined.
  */
diff --git a/arch/x86/pci/legacy.c b/arch/x86/pci/legacy.c
index 1cb01ab..80ea40e 100644
--- a/arch/x86/pci/legacy.c
+++ b/arch/x86/pci/legacy.c
@@ -65,6 +65,7 @@ static int __init pci_subsys_init(void)
 		}
 	}
 
+	pcibios_select_ops();
 	pcibios_fixup_peer_bridges();
 	x86_init.pci.init_irq();
 	pcibios_init();
diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index d1b47d5..6af6351 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -816,3 +816,33 @@ int pci_mmconfig_delete(u16 seg, u8 start, u8 end)
 
 	return -ENOENT;
 }
+
+static int pci_ecam_read(struct pci_bus *bus, unsigned int devfn, int reg,
+			 int size, u32 *value)
+{
+	return pci_mmcfg_read(pci_domain_nr(bus), bus->number, devfn, reg,
+			      size, value);
+}
+
+static int pci_ecam_write(struct pci_bus *bus, unsigned int devfn, int reg,
+			  int size, u32 value)
+{
+	return pci_mmcfg_write(pci_domain_nr(bus), bus->number, devfn, reg,
+			       size, value);
+}
+
+void __init pci_mmcfg_select_ops(void)
+{
+	if (raw_pci_ext_ops != &pci_mmcfg)
+		return;
+
+	/*
+	 * The pointer to root_pci_ops has been handed in to ACPI already
+	 * and is already set in the busses.
+	 *
+	 * Switch the functions over to ECAM for all config space accesses.
+	 */
+	pci_root_ops.read = pci_ecam_read;
+	pci_root_ops.write = pci_ecam_write;
+	pr_info("PCI: Switch to ECAM configuration mode\n");
+}
diff --git a/arch/x86/pci/mmconfig_32.c b/arch/x86/pci/mmconfig_32.c
index 3e9e166..d0975f1 100644
--- a/arch/x86/pci/mmconfig_32.c
+++ b/arch/x86/pci/mmconfig_32.c
@@ -49,8 +49,8 @@ static void pci_exp_set_dev_base(unsigned int base, int bus, int devfn)
 	}
 }
 
-static int pci_mmcfg_read(unsigned int seg, unsigned int bus,
-			  unsigned int devfn, int reg, int len, u32 *value)
+int pci_mmcfg_read(unsigned int seg, unsigned int bus,
+		   unsigned int devfn, int reg, int len, u32 *value)
 {
 	unsigned long flags;
 	u32 base;
@@ -88,8 +88,8 @@ err:		*value = -1;
 	return 0;
 }
 
-static int pci_mmcfg_write(unsigned int seg, unsigned int bus,
-			   unsigned int devfn, int reg, int len, u32 value)
+int pci_mmcfg_write(unsigned int seg, unsigned int bus,
+		    unsigned int devfn, int reg, int len, u32 value)
 {
 	unsigned long flags;
 	u32 base;
diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c
index f1c1aa0..64949dd 100644
--- a/arch/x86/pci/mmconfig_64.c
+++ b/arch/x86/pci/mmconfig_64.c
@@ -24,8 +24,8 @@ static char __iomem *pci_dev_base(unsigned int seg, unsigned int bus, unsigned i
 	return NULL;
 }
 
-static int pci_mmcfg_read(unsigned int seg, unsigned int bus,
-			  unsigned int devfn, int reg, int len, u32 *value)
+int pci_mmcfg_read(unsigned int seg, unsigned int bus,
+		   unsigned int devfn, int reg, int len, u32 *value)
 {
 	char __iomem *addr;
 
@@ -58,8 +58,8 @@ err:		*value = -1;
 	return 0;
 }
 
-static int pci_mmcfg_write(unsigned int seg, unsigned int bus,
-			   unsigned int devfn, int reg, int len, u32 value)
+int pci_mmcfg_write(unsigned int seg, unsigned int bus,
+		    unsigned int devfn, int reg, int len, u32 value)
 {
 	char __iomem *addr;
 

Re: [tip:x86/platform] x86/PCI/mmcfg: Switch to ECAM config mode if possible

[Yinghai Lu]

On Wed, Jun 28, 2017 at 11:45 PM, tip-bot for Thomas Gleixner
<tipbot@zytor.com> wrote:
> Commit-ID:  b5b0f00c760b6e9673ab79b88ede2f3c7a039f74
> Gitweb:     http://git.kernel.org/tip/b5b0f00c760b6e9673ab79b88ede2f3c7a039f74
> Author:     Thomas Gleixner <tglx@linutronix.de>
> AuthorDate: Thu, 16 Mar 2017 22:50:09 +0100
> Committer:  Thomas Gleixner <tglx@linutronix.de>
> CommitDate: Thu, 29 Jun 2017 08:41:54 +0200
>
> x86/PCI/mmcfg: Switch to ECAM config mode if possible
>
> To allow lockless access to the whole PCI configuration space the mmconfig
> based accessor functions need to be propagated to the pci_root_ops.
>
> Unfortunatly this cannot be done before the PCI subsystem initialization
> happens even if mmconfig access is already available. The reason is that
> some of the special platform PCI implementations must be able to overrule
> that setting before further accesses happen.
>
> The earliest possible point is after x86_init.pci.init() has been run. This
> is at a point in the boot process where nothing actually uses the PCI
> devices so the accessor function pointers can be updated lockless w/o risk.
>
> The switch to full ECAM mode depends on the availability of mmconfig and
> unchanged default accessors.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Acked-by: Bjorn Helgaas <helgaas@kernel.org>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Stephane Eranian <eranian@google.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: linux-pci@vger.kernel.org
> Link: http://lkml.kernel.org/r/20170316215057.452220163@linutronix.de
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/x86/include/asm/pci_x86.h | 20 ++++++++++++--------
>  arch/x86/pci/common.c          | 16 ++++++++++++++++
>  arch/x86/pci/legacy.c          |  1 +
>  arch/x86/pci/mmconfig-shared.c | 30 ++++++++++++++++++++++++++++++
>  arch/x86/pci/mmconfig_32.c     |  8 ++++----
>  arch/x86/pci/mmconfig_64.c     |  8 ++++----
>  6 files changed, 67 insertions(+), 16 deletions(-)
>
>  /*
> + * Called after the last possible modification to raw_pci_[ext_]ops.
> + *
> + * Verify that root_pci_ops have not been overwritten by any implementation
> + * of x86_init.pci.arch_init() and x86_init.pci.init().
> + *
> + * If not, let the mmconfig code decide whether the ops can be switched
> + * over to the ECAM accessor functions.
> + */
> +void __init pcibios_select_ops(void)
> +{
> +       if (pci_root_ops.read != pci_read || pci_root_ops.write != pci_write)
> +               return;
> +       pci_mmcfg_select_ops();
> +}
> +
> +/*
>   *  Called after each bus is probed, but before its children
>   *  are examined.
>   */
> diff --git a/arch/x86/pci/legacy.c b/arch/x86/pci/legacy.c
> index 1cb01ab..80ea40e 100644
> --- a/arch/x86/pci/legacy.c
> +++ b/arch/x86/pci/legacy.c
> @@ -65,6 +65,7 @@ static int __init pci_subsys_init(void)
>                 }
>         }
>
> +       pcibios_select_ops();
>         pcibios_fixup_peer_bridges();
>         x86_init.pci.init_irq();
>         pcibios_init();
> diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
> index d1b47d5..6af6351 100644
> --- a/arch/x86/pci/mmconfig-shared.c
> +++ b/arch/x86/pci/mmconfig-shared.c
> @@ -816,3 +816,33 @@ int pci_mmconfig_delete(u16 seg, u8 start, u8 end)
>
>         return -ENOENT;
>  }
> +
> +static int pci_ecam_read(struct pci_bus *bus, unsigned int devfn, int reg,
> +                        int size, u32 *value)
> +{
> +       return pci_mmcfg_read(pci_domain_nr(bus), bus->number, devfn, reg,
> +                             size, value);
> +}
> +
> +static int pci_ecam_write(struct pci_bus *bus, unsigned int devfn, int reg,
> +                         int size, u32 value)
> +{
> +       return pci_mmcfg_write(pci_domain_nr(bus), bus->number, devfn, reg,
> +                              size, value);
> +}
> +
> +void __init pci_mmcfg_select_ops(void)
> +{
> +       if (raw_pci_ext_ops != &pci_mmcfg)
> +               return;
> +
> +       /*
> +        * The pointer to root_pci_ops has been handed in to ACPI already
> +        * and is already set in the busses.
> +        *
> +        * Switch the functions over to ECAM for all config space accesses.
> +        */
> +       pci_root_ops.read = pci_ecam_read;
> +       pci_root_ops.write = pci_ecam_write;
> +       pr_info("PCI: Switch to ECAM configuration mode\n");
> +}

Hi Thomas,

Would this patch actually void the commit:

commit a0ca9909609470ad779b9b9cc68ce96e975afff7
Author: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Date:   Mon Jan 14 17:31:09 2008 -0500

    PCI x86: always use conf1 to access config space below 256 bytes

    Thanks to Loic Prylli <loic@myri.com>, who originally proposed
    this idea.

    Always using legacy configuration mechanism for the legacy config space
    and extended mechanism (mmconf) for the extended config space is
    a simple and very logical approach. It's supposed to resolve all
    known mmconf problems. It still allows per-device quirks (tweaking
    dev->cfg_size). It also allows to get rid of mmconf fallback code.

    Signed-off-by: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
    Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Re: [tip:x86/platform] x86/PCI/mmcfg: Switch to ECAM config mode if possible

[Andi Kleen]

> Hi Thomas,
> 
> Would this patch actually void the commit:

Yes it does. My origina; patches were opt-in for this reason, but Thomas doesn't
believe in historical or other people's experience.

But MCFG problems were a long time ago and noone uses these systems anymore,
so perhaps he is right.

-Andi

> 
> commit a0ca9909609470ad779b9b9cc68ce96e975afff7
> Author: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
> Date:   Mon Jan 14 17:31:09 2008 -0500
> 
>     PCI x86: always use conf1 to access config space below 256 bytes
> 
>     Thanks to Loic Prylli <loic@myri.com>, who originally proposed
>     this idea.
> 
>     Always using legacy configuration mechanism for the legacy config space
>     and extended mechanism (mmconf) for the extended config space is
>     a simple and very logical approach. It's supposed to resolve all
>     known mmconf problems. It still allows per-device quirks (tweaking
>     dev->cfg_size). It also allows to get rid of mmconf fallback code.
> 
>     Signed-off-by: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
>     Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
>     Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Re: [tip:x86/platform] x86/PCI/mmcfg: Switch to ECAM config mode if possible

[Thomas Gleixner]

On Thu, 29 Jun 2017, Andi Kleen wrote:
> > Would this patch actually void the commit:
> 
> Yes it does. My origina; patches were opt-in for this reason, but Thomas doesn't
> believe in historical or other people's experience.

I did not realize that particular wreckage.

> But MCFG problems were a long time ago and noone uses these systems anymore,
> so perhaps he is right.

The obvious solution to this is to force type 1 for older machines, i.e. <=
K8. Some day we should stop supporting 15+ years old crap just because we
can.

Thanks,

	tglx

Re: [tip:x86/platform] x86/PCI/mmcfg: Switch to ECAM config mode if possible

[Linus Torvalds]

On Fri, Jun 30, 2017 at 7:30 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> But MCFG problems were a long time ago and noone uses these systems anymore,
>> so perhaps he is right.
>
> The obvious solution to this is to force type 1 for older machines, i.e. <=
> K8. Some day we should stop supporting 15+ years old crap just because we
> can.

No.

The fact is, type 1 is the *good* thing.  It's the standard thing that
has worked pretty much forever, and that is not just tested, but has
good semantics.

The new stuff is the crazy crap. It's crazy crap in so many ways:

 - non-deterministic memory addresses found in firmware tables that
have had bugs

 - using MMIO means that there are lots of basic issues with
fundamental things like write gathering and ordering

The right thing to do is to just admit that the extended mmcfg isn't
actually the rigth thjing to do by default, and only use it when you
have to.

"Newer" does not always mean "better", and there's been lots of bad
hardware (and bad firmware).

So use the "enhanced" one for stuff above the 256-byte limit. Not for
basic probing.

Anmd don't think that it should be phased out just because it's old.

Old is often *good*. Old means stable. Old means tested. Old means
simple. Those are all *good* thing.

                 Linus

Re: [tip:x86/platform] x86/PCI/mmcfg: Switch to ECAM config mode if possible

[Ivan Kokshaysky]

On Fri, Jun 30, 2017 at 10:16:15AM -0700, Linus Torvalds wrote:
> On Fri, Jun 30, 2017 at 7:30 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >> But MCFG problems were a long time ago and noone uses these systems anymore,
> >> so perhaps he is right.
> >
> > The obvious solution to this is to force type 1 for older machines, i.e. <=
> > K8. Some day we should stop supporting 15+ years old crap just because we
> > can.
> 
> No.
> 
> The fact is, type 1 is the *good* thing.  It's the standard thing that
> has worked pretty much forever, and that is not just tested, but has
> good semantics.

Fully agreed.

Also, config space accesses supposed to be *safe* rather than *fast*.

There was quite a heated discussion on this very subject back in 2008.
Thought that I have a full copy of that, but no, surprisingly...
Google still gives a sufficient part of that:

http://linux-kernel.2935.n7.nabble.com/Patch-v2-Make-PCI-extended-config-space-MMCONFIG-a-driver-opt-in-td243861i60.html

Ivan.

Re: [tip:x86/platform] x86/PCI/mmcfg: Switch to ECAM config mode if possible

[Thomas Gleixner]

On Fri, 30 Jun 2017, Linus Torvalds wrote:
> So use the "enhanced" one for stuff above the 256-byte limit. Not for
> basic probing.

The probing itself uses type1 unconditionally. It just switches over when
mmcfg is the default and no other quirk has been applied.

Nevertheless I zap that commit in question. The rest of the series is still
valid and does not affect that. The main goal of distangling the stuff from
the global PCI lock and avoiding pointless nested locking is still reached.

Thanks,

	tglx

Re: [patch 0/7] x86/pci: Switch to lockless ECAM configuration mode

[Andi Kleen]

On Thu, Mar 16, 2017 at 10:50:02PM +0100, Thomas Gleixner wrote:
> Andi reported that the Intel Uncore performance monitoring suffers from the
> global pci_lock which protects the generic PCI config space accessors.
> 
> This lock can be removed on X86 because all architecture specific config
> space accessors are either serialized by a seperate lock or can be
> converted to have their own locking. The mmconfig based access (ECAM) does
> not require serialization at the config read/write level because the access
> is a simple byte/word/dword read from or write to the memory mapped
> configuration space. In that case the lock does not provide any more
> protection than the hardware provides already.
> 
> The series converts the CE4100 PCI low level accessors to use the x86
> specific pci_config_lock and implements the switch over to lockless ECAM
> configureation space accesors when the system is capable.

What's the status of this patch series? The lock contention
problem is still there.

Thanks,

-Andi

Re: [patch 0/7] x86/pci: Switch to lockless ECAM configuration mode

[Thomas Gleixner]

Andi,

On Mon, 12 Jun 2017, Andi Kleen wrote:
> On Thu, Mar 16, 2017 at 10:50:02PM +0100, Thomas Gleixner wrote:
> > Andi reported that the Intel Uncore performance monitoring suffers from the
> > global pci_lock which protects the generic PCI config space accessors.
> > 
> > This lock can be removed on X86 because all architecture specific config
> > space accessors are either serialized by a seperate lock or can be
> > converted to have their own locking. The mmconfig based access (ECAM) does
> > not require serialization at the config read/write level because the access
> > is a simple byte/word/dword read from or write to the memory mapped
> > configuration space. In that case the lock does not provide any more
> > protection than the hardware provides already.
> > 
> > The series converts the CE4100 PCI low level accessors to use the x86
> > specific pci_config_lock and implements the switch over to lockless ECAM
> > configureation space accesors when the system is capable.
> 
> What's the status of this patch series? The lock contention
> problem is still there.

Just rechecked. It still applies and boots (patch 6/7) dropped.

If Bjorn has no objections I can route it through tip tomorrow.

Thanks,

	tglx

Re: [patch 0/7] x86/pci: Switch to lockless ECAM configuration mode

[Bjorn Helgaas]

[+cc linux-pci]

On Thu, Jun 22, 2017 at 12:28:09AM +0200, Thomas Gleixner wrote:
> Andi,
> 
> On Mon, 12 Jun 2017, Andi Kleen wrote:
> > On Thu, Mar 16, 2017 at 10:50:02PM +0100, Thomas Gleixner wrote:
> > > Andi reported that the Intel Uncore performance monitoring suffers from the
> > > global pci_lock which protects the generic PCI config space accessors.
> > > 
> > > This lock can be removed on X86 because all architecture specific config
> > > space accessors are either serialized by a seperate lock or can be
> > > converted to have their own locking. The mmconfig based access (ECAM) does
> > > not require serialization at the config read/write level because the access
> > > is a simple byte/word/dword read from or write to the memory mapped
> > > configuration space. In that case the lock does not provide any more
> > > protection than the hardware provides already.
> > > 
> > > The series converts the CE4100 PCI low level accessors to use the x86
> > > specific pci_config_lock and implements the switch over to lockless ECAM
> > > configureation space accesors when the system is capable.
> > 
> > What's the status of this patch series? The lock contention
> > problem is still there.
> 
> Just rechecked. It still applies and boots (patch 6/7) dropped.
> 
> If Bjorn has no objections I can route it through tip tomorrow.

Sorry for the delay.  I use patchwork as my to-do list, and it only
looks at linux-pci.  This series wasn't posted there, so I didn't see
it until now.

I have a couple trivial comments but I think it looks great overall.
Here's my ack if you want to apply it via tip:

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

Bjorn