Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1602913

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

From Thomas Gleixner <tglx@linutronix.de>
Newsgroups linux.kernel
Subject [patch 3/7] x86/pci/ce4100: Properly lock accessor functions
Date 2017-03-17 00:20 +0100
Message-ID <tlMQx-5vf-11@gated-at.bofh.it> (permalink)
References <tlMQx-5vf-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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)

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread


Thread

[patch 0/7] x86/pci: Switch to lockless ECAM configuration mode Thomas Gleixner <tglx@linutronix.de> - 2017-03-17 00:20 +0100
  [patch 4/7] PCI: Provide Kconfig option for lockless config space  accessors Thomas Gleixner <tglx@linutronix.de> - 2017-03-17 00:20 +0100
  [patch 1/7] x86/pci: Remove duplicate defines Thomas Gleixner <tglx@linutronix.de> - 2017-03-17 00:20 +0100
  [patch 5/7] x86/pci: Select CONFIG_PCI_LOCKLESS_CONFIG Thomas Gleixner <tglx@linutronix.de> - 2017-03-17 00:20 +0100
  [patch 3/7] x86/pci/ce4100: Properly lock accessor functions Thomas Gleixner <tglx@linutronix.de> - 2017-03-17 00:20 +0100
    Re: [patch 3/7] x86/pci/ce4100: Properly lock accessor functions Andi Kleen <ak@linux.intel.com> - 2017-03-17 01:50 +0100
  [patch 6/7] x86/pci/mmcfg: Include 32/64 bit code into shared code Thomas Gleixner <tglx@linutronix.de> - 2017-03-17 00:20 +0100
    Re: [patch 6/7] x86/pci/mmcfg: Include 32/64 bit code into shared  code Andi Kleen <ak@linux.intel.com> - 2017-03-17 01:40 +0100
      Re: [patch 6/7] x86/pci/mmcfg: Include 32/64 bit code into shared  code Thomas Gleixner <tglx@linutronix.de> - 2017-03-17 09:50 +0100
  [patch 7/7] x86/pci/mmcfg: Switch to ECAM config mode if possible Thomas Gleixner <tglx@linutronix.de> - 2017-03-17 00:20 +0100
    Re: [patch 7/7] x86/pci/mmcfg: Switch to ECAM config mode if possible Andi Kleen <ak@linux.intel.com> - 2017-03-17 01:50 +0100
      Re: [patch 7/7] x86/pci/mmcfg: Switch to ECAM config mode if  possible Thomas Gleixner <tglx@linutronix.de> - 2017-03-17 07:20 +0100
  [patch 2/7] x86/pci: Abort if legacy init fails Thomas Gleixner <tglx@linutronix.de> - 2017-03-17 00:20 +0100

csiph-web