From 36e8164882ca6d3c41cb91e6f09a3ed236841f80 Mon Sep 17 00:00:00 2001 From: Myron Stowe Date: Thu, 30 Oct 2014 11:54:37 -0600 Subject: PCI: Restore detection of read-only BARs Commit 6ac665c63dca ("PCI: rewrite PCI BAR reading code") masked off low-order bits from 'l', but not from 'sz'. Both are passed to pci_size(), which compares 'base == maxbase' to check for read-only BARs. The masking of 'l' means that comparison will never be 'true', so the check for read-only BARs no longer works. Resolve this by also masking off the low-order bits of 'sz' before passing it into pci_size() as 'maxbase'. With this change, pci_size() will once again catch the problems that have been encountered to date: - AGP aperture BAR of AMD-7xx host bridges: if the AGP window is disabled, this BAR is read-only and read as 0x00000008 [1] - BARs 0-4 of ALi IDE controllers can be non-zero and read-only [1] - Intel Sandy Bridge - Thermal Management Controller [8086:0103]; BAR 0 returning 0xfed98004 [2] - Intel Xeon E5 v3/Core i7 Power Control Unit [8086:2fc0]; Bar 0 returning 0x00001a [3] Link: [1] https://git.kernel.org/cgit/linux/kernel/git/tglx/history.git/commit/drivers/pci/probe.c?id=1307ef6621991f1c4bc3cec1b5a4ebd6fd3d66b9 ("PCI: probing read-only BARs" (pre-git)) Link: [2] https://bugzilla.kernel.org/show_bug.cgi?id=43331 Link: [3] https://bugzilla.kernel.org/show_bug.cgi?id=85991 Reported-by: William Unruh Reported-by: Martin Lucina Signed-off-by: Myron Stowe Signed-off-by: Bjorn Helgaas CC: Matthew Wilcox CC: stable@vger.kernel.org # v2.6.27+ --- drivers/pci/probe.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/pci/probe.c') diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 5ed99309c758..19dc247618f8 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -216,14 +216,17 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type, res->flags |= IORESOURCE_SIZEALIGN; if (res->flags & IORESOURCE_IO) { l &= PCI_BASE_ADDRESS_IO_MASK; + sz &= PCI_BASE_ADDRESS_IO_MASK; mask = PCI_BASE_ADDRESS_IO_MASK & (u32) IO_SPACE_LIMIT; } else { l &= PCI_BASE_ADDRESS_MEM_MASK; + sz &= PCI_BASE_ADDRESS_MEM_MASK; mask = (u32)PCI_BASE_ADDRESS_MEM_MASK; } } else { res->flags |= (l & IORESOURCE_ROM_ENABLE); l &= PCI_ROM_ADDRESS_MASK; + sz &= PCI_ROM_ADDRESS_MASK; mask = (u32)PCI_ROM_ADDRESS_MASK; } -- cgit v1.2.2 From f795d86aaa578501551a2d1b463eac4bbea84db2 Mon Sep 17 00:00:00 2001 From: Myron Stowe Date: Thu, 30 Oct 2014 11:54:43 -0600 Subject: PCI: Shrink decoding-disabled window while sizing BARs __pci_read_base() disables decoding while sizing device BARs. We can't print while decoding is disabled, which leads to some rather messy exit logic. Coalesce the sizing logic to minimize the time decoding is disabled. This lets us print errors where they're detected. The refactoring also takes advantage of the symmetry of obtaining the BAR's extent (pci_size) and storing the result as the 'region' for both the 32-bit and 64-bit BARs, consolidating both cases. No functional change intended. [bhelgaas: move pci_size() up, per Thomas Petazzoni, Thierry Reding, Kevin Hilman] Signed-off-by: Myron Stowe Signed-off-by: Bjorn Helgaas CC: Matthew Wilcox --- drivers/pci/probe.c | 77 +++++++++++++++++++++-------------------------------- 1 file changed, 31 insertions(+), 46 deletions(-) (limited to 'drivers/pci/probe.c') diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 19dc247618f8..529fcd782e43 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -175,7 +175,6 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type, u64 l64, sz64, mask64; u16 orig_cmd; struct pci_bus_region region, inverted_region; - bool bar_too_big = false, bar_too_high = false, bar_invalid = false; mask = type ? PCI_ROM_ADDRESS_MASK : ~0; @@ -201,8 +200,8 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type, * memory BAR or a ROM, bit 0 must be clear; if it's an io BAR, bit * 1 must be clear. */ - if (!sz || sz == 0xffffffff) - goto fail; + if (sz == 0xffffffff) + sz = 0; /* * I don't know how l can have all bits set. Copied from old code. @@ -215,26 +214,22 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type, res->flags = decode_bar(dev, l); res->flags |= IORESOURCE_SIZEALIGN; if (res->flags & IORESOURCE_IO) { - l &= PCI_BASE_ADDRESS_IO_MASK; - sz &= PCI_BASE_ADDRESS_IO_MASK; - mask = PCI_BASE_ADDRESS_IO_MASK & (u32) IO_SPACE_LIMIT; + l64 = l & PCI_BASE_ADDRESS_IO_MASK; + sz64 = sz & PCI_BASE_ADDRESS_IO_MASK; + mask64 = PCI_BASE_ADDRESS_IO_MASK & (u32)IO_SPACE_LIMIT; } else { - l &= PCI_BASE_ADDRESS_MEM_MASK; - sz &= PCI_BASE_ADDRESS_MEM_MASK; - mask = (u32)PCI_BASE_ADDRESS_MEM_MASK; + l64 = l & PCI_BASE_ADDRESS_MEM_MASK; + sz64 = sz & PCI_BASE_ADDRESS_MEM_MASK; + mask64 = (u32)PCI_BASE_ADDRESS_MEM_MASK; } } else { res->flags |= (l & IORESOURCE_ROM_ENABLE); - l &= PCI_ROM_ADDRESS_MASK; - sz &= PCI_ROM_ADDRESS_MASK; - mask = (u32)PCI_ROM_ADDRESS_MASK; + l64 = l & PCI_ROM_ADDRESS_MASK; + sz64 = sz & PCI_ROM_ADDRESS_MASK; + mask64 = (u32)PCI_ROM_ADDRESS_MASK; } if (res->flags & IORESOURCE_MEM_64) { - l64 = l; - sz64 = sz; - mask64 = mask | (u64)~0 << 32; - pci_read_config_dword(dev, pos + 4, &l); pci_write_config_dword(dev, pos + 4, ~0); pci_read_config_dword(dev, pos + 4, &sz); @@ -242,18 +237,27 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type, l64 |= ((u64)l << 32); sz64 |= ((u64)sz << 32); + mask64 |= ((u64)~0 << 32); + } - sz64 = pci_size(l64, sz64, mask64); + if (!dev->mmio_always_on && (orig_cmd & PCI_COMMAND_DECODE_ENABLE)) + pci_write_config_word(dev, PCI_COMMAND, orig_cmd); - if (!sz64) - goto fail; + if (!sz64) + goto fail; + + sz64 = pci_size(l64, sz64, mask64); + if (!sz64) + goto fail; + if (res->flags & IORESOURCE_MEM_64) { if ((sizeof(dma_addr_t) < 8 || sizeof(resource_size_t) < 8) && sz64 > 0x100000000ULL) { res->flags |= IORESOURCE_UNSET | IORESOURCE_DISABLED; res->start = 0; res->end = 0; - bar_too_big = true; + dev_err(&dev->dev, "reg 0x%x: can't handle BAR larger than 4GB (size %#010llx)\n", + pos, (unsigned long long)sz64); goto out; } @@ -262,22 +266,15 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type, res->flags |= IORESOURCE_UNSET; res->start = 0; res->end = sz64; - bar_too_high = true; + dev_info(&dev->dev, "reg 0x%x: can't handle BAR above 4GB (bus address %#010llx)\n", + pos, (unsigned long long)l64); goto out; - } else { - region.start = l64; - region.end = l64 + sz64; } - } else { - sz = pci_size(l, sz, mask); - - if (!sz) - goto fail; - - region.start = l; - region.end = l + sz; } + region.start = l64; + region.end = l64 + sz64; + pcibios_bus_to_resource(dev->bus, res, ®ion); pcibios_resource_to_bus(dev->bus, &inverted_region, res); @@ -296,7 +293,8 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type, res->flags |= IORESOURCE_UNSET; res->start = 0; res->end = region.end - region.start; - bar_invalid = true; + dev_info(&dev->dev, "reg 0x%x: initial BAR value %#010llx invalid\n", + pos, (unsigned long long)region.start); } goto out; @@ -305,19 +303,6 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type, fail: res->flags = 0; out: - if (!dev->mmio_always_on && - (orig_cmd & PCI_COMMAND_DECODE_ENABLE)) - pci_write_config_word(dev, PCI_COMMAND, orig_cmd); - - if (bar_too_big) - dev_err(&dev->dev, "reg 0x%x: can't handle BAR larger than 4GB (size %#010llx)\n", - pos, (unsigned long long) sz64); - if (bar_too_high) - dev_info(&dev->dev, "reg 0x%x: can't handle BAR above 4G (bus address %#010llx)\n", - pos, (unsigned long long) l64); - if (bar_invalid) - dev_info(&dev->dev, "reg 0x%x: initial BAR value %#010llx invalid\n", - pos, (unsigned long long) region.start); if (res->flags) dev_printk(KERN_DEBUG, &dev->dev, "reg 0x%x: %pR\n", pos, res); -- cgit v1.2.2 From ff0387c3777f2438bad2088abe442c9f231b0be4 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Mon, 10 Nov 2014 21:02:17 -0700 Subject: PCI: Delete unnecessary NULL pointer checks The functions pci_dev_put(), pci_pme_wakeup_bus(), and put_device() return immediately if their argument is NULL. Thus the test before the call is not needed. Remove these unnecessary tests. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring Signed-off-by: Bjorn Helgaas --- drivers/pci/probe.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/pci/probe.c') diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 5ed99309c758..eb927c04e290 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -87,8 +87,7 @@ static void release_pcibus_dev(struct device *dev) { struct pci_bus *pci_bus = to_pci_bus(dev); - if (pci_bus->bridge) - put_device(pci_bus->bridge); + put_device(pci_bus->bridge); pci_bus_remove_resources(pci_bus); pci_release_bus_of_node(pci_bus); kfree(pci_bus); -- cgit v1.2.2 From 7e79c5f8cad2ac1dc26c03e1aa16216391a04dad Mon Sep 17 00:00:00 2001 From: Myron Stowe Date: Thu, 30 Oct 2014 11:54:50 -0600 Subject: PCI: Add informational printk for invalid BARs As a consequence of restoring the detection of invalid BARs, add a new informational printk like the following when such occurrences are encountered. pci ssss:bb:dd.f: [Firmware Bug]: reg 0xXX: invalid BAR (can't size) Reported-by: William Unruh Reported-by: Martin Lucina Signed-off-by: Myron Stowe Signed-off-by: Bjorn Helgaas CC: Matthew Wilcox --- drivers/pci/probe.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/pci/probe.c') diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 529fcd782e43..6029ad786146 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -247,8 +247,11 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type, goto fail; sz64 = pci_size(l64, sz64, mask64); - if (!sz64) + if (!sz64) { + dev_info(&dev->dev, FW_BUG "reg 0x%x: invalid BAR (can't size)\n", + pos); goto fail; + } if (res->flags & IORESOURCE_MEM_64) { if ((sizeof(dma_addr_t) < 8 || sizeof(resource_size_t) < 8) && -- cgit v1.2.2