All of lore.kernel.org
 help / color / mirror / Atom feed
From: Koen Vandeputte <koen.vandeputte@ncentric.com>
To: linux-arm-kernel@lists.infradead.org
Cc: Koen Vandeputte <koen.vandeputte@ncentric.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Krzysztof Halasa <khalasa@piap.pl>,
	Olof Johansson <olof@lixom.net>,
	Robin Leblon <robin.leblon@ncentric.com>,
	Rob Herring <robh@kernel.org>,
	Russell King <linux@armlinux.org.uk>,
	Tim Harvey <tharvey@gateworks.com>,
	stable@vger.kernel.org
Subject: [PATCH] arm: cns3xxx: fix writing to wrong PCI registers after alignment
Date: Tue, 18 Dec 2018 12:17:43 +0100	[thread overview]
Message-ID: <20181218111743.25566-1-koen.vandeputte@ncentric.com> (raw)

Originally, cns3xxx used it's own functions for mapping, reading and writing registers.

Commit 802b7c06adc7 ("ARM: cns3xxx: Convert PCI to use generic config accessors")
removed the internal PCI config write function in favor of the generic one:

cns3xxx_pci_write_config() --> pci_generic_config_write()

cns3xxx_pci_write_config() expected aligned addresses, being produced by cns3xxx_pci_map_bus()
while the generic one pci_generic_config_write() actually expects the real address
as both the function and hardware are capable of byte-aligned writes.

This currently leads to pci_generic_config_write() writing
to the wrong registers on some ocasions.

First issue seen due to this:

- driver ath9k gets loaded
- The driver wants to write value 0xA8 to register PCI_LATENCY_TIMER, located at 0x0D
- cns3xxx_pci_map_bus() aligns the address to 0x0C
- pci_generic_config_write() effectively writes 0xA8 into register 0x0C (CACHE_LINE_SIZE)

This seems to cause some slight instability when certain PCI devices are used.

Another issue example caused by this this is the PCI bus numbering,
where the primary bus is higher than the secondary, which is impossible.

Before:

00:00.0 PCI bridge: Cavium, Inc. Device 3400 (rev 01) (prog-if 00 [Normal decode])
    Flags: bus master, fast devsel, latency 0, IRQ 255
    Bus: primary=02, secondary=01, subordinate=ff, sec-latency=0

After fix:

00:00.0 PCI bridge: Cavium, Inc. Device 3400 (rev 01) (prog-if 00 [Normal decode])
    Flags: bus master, fast devsel, latency 0, IRQ 255
    Bus: primary=00, secondary=01, subordinate=02, sec-latency=0

And very likely some more ..

Fix all by omitting the alignment being done in the mapping function.

Fixes: 802b7c06adc7 ("ARM: cns3xxx: Convert PCI to use generic config accessors")
Signed-off-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Bjorn Helgaas <bhelgaas@google.com>
CC: Krzysztof Halasa <khalasa@piap.pl>
CC: Olof Johansson <olof@lixom.net>
CC: Robin Leblon <robin.leblon@ncentric.com>
CC: Rob Herring <robh@kernel.org>
CC: Russell King <linux@armlinux.org.uk>
CC: Tim Harvey <tharvey@gateworks.com>
CC: stable@vger.kernel.org # v4.0+
---
 arch/arm/mach-cns3xxx/pcie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-cns3xxx/pcie.c b/arch/arm/mach-cns3xxx/pcie.c
index 318394ed5c7a..5e11ad3164e0 100644
--- a/arch/arm/mach-cns3xxx/pcie.c
+++ b/arch/arm/mach-cns3xxx/pcie.c
@@ -83,7 +83,7 @@ static void __iomem *cns3xxx_pci_map_bus(struct pci_bus *bus,
 	} else /* remote PCI bus */
 		base = cnspci->cfg1_regs + ((busno & 0xf) << 20);
 
-	return base + (where & 0xffc) + (devfn << 12);
+	return base + where + (devfn << 12);
 }
 
 static int cns3xxx_pci_read_config(struct pci_bus *bus, unsigned int devfn,
-- 
2.17.1

WARNING: multiple messages have this Message-ID (diff)
From: Koen Vandeputte <koen.vandeputte@ncentric.com>
To: linux-arm-kernel@lists.infradead.org
Cc: Rob Herring <robh@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	Tim Harvey <tharvey@gateworks.com>,
	Russell King <linux@armlinux.org.uk>,
	stable@vger.kernel.org, Robin Leblon <robin.leblon@ncentric.com>,
	Olof Johansson <olof@lixom.net>,
	Krzysztof Halasa <khalasa@piap.pl>,
	Koen Vandeputte <koen.vandeputte@ncentric.com>,
	Bjorn Helgaas <bhelgaas@google.com>
Subject: [PATCH] arm: cns3xxx: fix writing to wrong PCI registers after alignment
Date: Tue, 18 Dec 2018 12:17:43 +0100	[thread overview]
Message-ID: <20181218111743.25566-1-koen.vandeputte@ncentric.com> (raw)

Originally, cns3xxx used it's own functions for mapping, reading and writing registers.

Commit 802b7c06adc7 ("ARM: cns3xxx: Convert PCI to use generic config accessors")
removed the internal PCI config write function in favor of the generic one:

cns3xxx_pci_write_config() --> pci_generic_config_write()

cns3xxx_pci_write_config() expected aligned addresses, being produced by cns3xxx_pci_map_bus()
while the generic one pci_generic_config_write() actually expects the real address
as both the function and hardware are capable of byte-aligned writes.

This currently leads to pci_generic_config_write() writing
to the wrong registers on some ocasions.

First issue seen due to this:

- driver ath9k gets loaded
- The driver wants to write value 0xA8 to register PCI_LATENCY_TIMER, located at 0x0D
- cns3xxx_pci_map_bus() aligns the address to 0x0C
- pci_generic_config_write() effectively writes 0xA8 into register 0x0C (CACHE_LINE_SIZE)

This seems to cause some slight instability when certain PCI devices are used.

Another issue example caused by this this is the PCI bus numbering,
where the primary bus is higher than the secondary, which is impossible.

Before:

00:00.0 PCI bridge: Cavium, Inc. Device 3400 (rev 01) (prog-if 00 [Normal decode])
    Flags: bus master, fast devsel, latency 0, IRQ 255
    Bus: primary=02, secondary=01, subordinate=ff, sec-latency=0

After fix:

00:00.0 PCI bridge: Cavium, Inc. Device 3400 (rev 01) (prog-if 00 [Normal decode])
    Flags: bus master, fast devsel, latency 0, IRQ 255
    Bus: primary=00, secondary=01, subordinate=02, sec-latency=0

And very likely some more ..

Fix all by omitting the alignment being done in the mapping function.

Fixes: 802b7c06adc7 ("ARM: cns3xxx: Convert PCI to use generic config accessors")
Signed-off-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Bjorn Helgaas <bhelgaas@google.com>
CC: Krzysztof Halasa <khalasa@piap.pl>
CC: Olof Johansson <olof@lixom.net>
CC: Robin Leblon <robin.leblon@ncentric.com>
CC: Rob Herring <robh@kernel.org>
CC: Russell King <linux@armlinux.org.uk>
CC: Tim Harvey <tharvey@gateworks.com>
CC: stable@vger.kernel.org # v4.0+
---
 arch/arm/mach-cns3xxx/pcie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-cns3xxx/pcie.c b/arch/arm/mach-cns3xxx/pcie.c
index 318394ed5c7a..5e11ad3164e0 100644
--- a/arch/arm/mach-cns3xxx/pcie.c
+++ b/arch/arm/mach-cns3xxx/pcie.c
@@ -83,7 +83,7 @@ static void __iomem *cns3xxx_pci_map_bus(struct pci_bus *bus,
 	} else /* remote PCI bus */
 		base = cnspci->cfg1_regs + ((busno & 0xf) << 20);
 
-	return base + (where & 0xffc) + (devfn << 12);
+	return base + where + (devfn << 12);
 }
 
 static int cns3xxx_pci_read_config(struct pci_bus *bus, unsigned int devfn,
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

             reply	other threads:[~2018-12-18 11:19 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-18 11:17 Koen Vandeputte [this message]
2018-12-18 11:17 ` [PATCH] arm: cns3xxx: fix writing to wrong PCI registers after alignment Koen Vandeputte
2018-12-18 16:04 ` Tim Harvey
2018-12-18 16:04   ` Tim Harvey
2018-12-20  5:34 ` Krzysztof Hałasa
2018-12-20  5:34   ` Krzysztof Hałasa
2018-12-29 14:40 ` Koen Vandeputte
2018-12-29 14:40   ` Koen Vandeputte
2018-12-30  1:06 ` Bjorn Helgaas
2018-12-30  1:06   ` Bjorn Helgaas
2018-12-31 10:14   ` Krzysztof Hałasa
2018-12-31 10:14     ` Krzysztof Hałasa
2018-12-31 15:29     ` Bjorn Helgaas
2018-12-31 15:29       ` Bjorn Helgaas
2019-01-07  8:58       ` Koen Vandeputte
2019-01-07  8:58         ` Koen Vandeputte
2019-01-07 13:24         ` Bjorn Helgaas
2019-01-07 13:24           ` Bjorn Helgaas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181218111743.25566-1-koen.vandeputte@ncentric.com \
    --to=koen.vandeputte@ncentric.com \
    --cc=arnd@arndb.de \
    --cc=bhelgaas@google.com \
    --cc=khalasa@piap.pl \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux@armlinux.org.uk \
    --cc=olof@lixom.net \
    --cc=robh@kernel.org \
    --cc=robin.leblon@ncentric.com \
    --cc=stable@vger.kernel.org \
    --cc=tharvey@gateworks.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.