These are intended to fix some corner-case BAR sizing issues: cases where
resource_size_t or dma_addr_t is too small to handle a BAR larger than 4GB
or one placed above 4GB. There's also a message clarification in the
resource sanity check and a subtractive bridge window fix.
Comments welcome.
---
Bjorn Helgaas (7):
PCI: Fail safely if we can't handle BARs larger than 4GB
PCI: Reject BAR above 4GB if dma_addr_t is too small
PCI: Don't convert BAR address to resource if dma_addr_t is too small
PCI: Don't set BAR to zero if dma_addr_t is too small
PCI: Don't print anything while decoding is disabled
resources: Clarify sanity check message
PCI: Don't add disabled subtractive decode bus resources
drivers/pci/probe.c | 48 +++++++++++++++++++++++++++++-------------------
kernel/resource.c | 7 ++-----
2 files changed, 31 insertions(+), 24 deletions(-)
We can only handle BARs larger than 4GB if both dma_addr_t and
resource_size_t are 64 bits wide. If dma_addr_t is 32 bits, we can't
represent all the bus addresses, and if resource_size_t is 32 bits, we
can't represent all the CPU addresses.
Previously we cleared res->flags (at "fail:") for resources that were too
large. That means we think the BAR doesn't exist at all, which in turn
means that we could enable the device even though we can't keep track of
where the BAR is and we can't make sure it doesn't overlap something else.
This preserves the type flags (MEM/IO) so we can keep from enabling the
device.
Signed-off-by: Bjorn Helgaas <[email protected]>
---
drivers/pci/probe.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ef09f5f2fe6c..c7f8b717c2e7 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -171,6 +171,7 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
struct resource *res, unsigned int pos)
{
u32 l, sz, mask;
+ u64 l64, sz64, mask64;
u16 orig_cmd;
struct pci_bus_region region, inverted_region;
bool bar_too_big = false, bar_disabled = false;
@@ -226,9 +227,9 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
}
if (res->flags & IORESOURCE_MEM_64) {
- u64 l64 = l;
- u64 sz64 = sz;
- u64 mask64 = mask | (u64)~0 << 32;
+ 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);
@@ -243,9 +244,13 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
if (!sz64)
goto fail;
- if ((sizeof(resource_size_t) < 8) && (sz64 > 0x100000000ULL)) {
+ 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;
- goto fail;
+ goto out;
}
if ((sizeof(resource_size_t) < 8) && l) {
@@ -303,7 +308,8 @@ out:
pci_write_config_word(dev, PCI_COMMAND, orig_cmd);
if (bar_too_big)
- dev_err(&dev->dev, "reg 0x%x: can't handle 64-bit BAR\n", pos);
+ dev_err(&dev->dev, "reg 0x%x: can't handle BAR larger than 4GB (size %#010llx)\n",
+ pos, (unsigned long long) sz64);
if (res->flags && !bar_disabled)
dev_printk(KERN_DEBUG, &dev->dev, "reg 0x%x: %pR\n", pos, res);
We can only handle BARs above 4GB if dma_addr_t (not resource_size_t) is 64
bits wide. If we have a 64-bit resource_size_t and a 32-bit dma_addr_t,
we can't deal with BARs above 4GB.
Signed-off-by: Bjorn Helgaas <[email protected]>
---
drivers/pci/probe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index c7f8b717c2e7..afae3bf405fa 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -253,7 +253,7 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
goto out;
}
- if ((sizeof(resource_size_t) < 8) && l) {
+ if ((sizeof(dma_addr_t) < 8) && l) {
/* Address above 32-bit boundary; disable the BAR */
pci_write_config_dword(dev, pos, 0);
pci_write_config_dword(dev, pos + 4, 0);
If dma_addr_t is too small to represent the BAR value,
pcibios_bus_to_resource() will fail, so just remember the BAR size directly
in the resource. The resource is already marked UNSET, so we know the
address isn't valid anyway.
Signed-off-by: Bjorn Helgaas <[email protected]>
---
drivers/pci/probe.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index afae3bf405fa..82cd75f6118a 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -258,9 +258,10 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
pci_write_config_dword(dev, pos, 0);
pci_write_config_dword(dev, pos + 4, 0);
res->flags |= IORESOURCE_UNSET;
- region.start = 0;
- region.end = sz64;
+ res->start = 0;
+ res->end = sz64;
bar_disabled = true;
+ goto out;
} else {
region.start = l64;
region.end = l64 + sz64;
If the console is a PCI device, and we try to print to it while its
decoding is disabled, the system will hang. This particular printk hasn't
caused a problem yet, but it could, so this fixes it.
See also 0ff9514b579b ("PCI: Don't print anything while decoding is
disabled").
Signed-off-by: Bjorn Helgaas <[email protected]>
---
drivers/pci/probe.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index dd710b12d34c..3bc149b848a8 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -174,7 +174,7 @@ 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;
+ bool bar_too_big = false, bar_too_high = false, bar_invalid = false;
mask = type ? PCI_ROM_ADDRESS_MASK : ~0;
@@ -289,11 +289,10 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
* be claimed by the device.
*/
if (inverted_region.start != region.start) {
- dev_info(&dev->dev, "reg 0x%x: initial BAR value %pa invalid; forcing reassignment\n",
- pos, ®ion.start);
res->flags |= IORESOURCE_UNSET;
- res->end -= res->start;
res->start = 0;
+ res->end = region.end - region.start;
+ bar_invalid = true;
}
goto out;
@@ -312,6 +311,9 @@ out:
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);
The resource map sanity check message is a bit confusing. Change it to be
more readable:
-resource map sanity check conflict: 0xfed10000 0xfed15fff 0xfed10000 0xfed13fff pnp 00:01
+resource sanity check: requesting [mem 0xfed10000-0xfed15fff], which spans more than pnp 00:01 [mem 0xfed10000-0xfed13fff]
Signed-off-by: Bjorn Helgaas <[email protected]>
---
kernel/resource.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/kernel/resource.c b/kernel/resource.c
index 8957d686e29b..3c2237ac32db 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1288,13 +1288,10 @@ int iomem_map_sanity_check(resource_size_t addr, unsigned long size)
if (p->flags & IORESOURCE_BUSY)
continue;
- printk(KERN_WARNING "resource map sanity check conflict: "
- "0x%llx 0x%llx 0x%llx 0x%llx %s\n",
+ printk(KERN_WARNING "resource sanity check: requesting [mem %#010llx-%#010llx], which spans more than %s %pR\n",
(unsigned long long)addr,
(unsigned long long)(addr + size - 1),
- (unsigned long long)p->start,
- (unsigned long long)p->end,
- p->name);
+ p->name, p);
err = -1;
break;
}
For a subtractive decode bridge, we previously added and printed all
resources of the primary bus, even if they were not valid. In the example
below, the bridge 00:1c.3 has no windows enabled, so there are no valid
resources on bus 02. But since 02:00.0 is subtractive decode bridge, we
add and print all those invalid resources, which don't really make sense:
pci 0000:00:1c.3: PCI bridge to [bus 02-03]
pci 0000:02:00.0: PCI bridge to [bus 03] (subtractive decode)
pci 0000:02:00.0: bridge window [??? 0x00000000 flags 0x0] (subtractive decode)
Add and print the subtractively-decoded resources only if they are valid.
There's an example in the dmesg log attached to the bugzilla below (but
this patch doesn't fix the bug reported there).
Link: https://bugzilla.kernel.org/show_bug.cgi?id=73141
Signed-off-by: Bjorn Helgaas <[email protected]>
---
drivers/pci/probe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 3bc149b848a8..966514010974 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -475,7 +475,7 @@ void pci_read_bridge_bases(struct pci_bus *child)
if (dev->transparent) {
pci_bus_for_each_resource(child->parent, res, i) {
- if (res) {
+ if (res && res->flags) {
pci_bus_add_resource(child, res,
PCI_SUBTRACTIVE_DECODE);
dev_printk(KERN_DEBUG, &dev->dev,
If a BAR is above 4GB and our dma_addr_t is too small, don't clear the BAR
to zero: that doesn't disable the BAR, and it makes it more likely that the
BAR will conflict with things if we turn on the memory enable bit (as we
will at "out:" if the BIOS left it enabled).
We should also print the BAR info and its original size so we can see what
happens when we try to assign space to it.
Signed-off-by: Bjorn Helgaas <[email protected]>
---
drivers/pci/probe.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 82cd75f6118a..dd710b12d34c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -174,7 +174,7 @@ 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_disabled = false;
+ bool bar_too_big = false, bar_too_high = false;
mask = type ? PCI_ROM_ADDRESS_MASK : ~0;
@@ -254,13 +254,11 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
}
if ((sizeof(dma_addr_t) < 8) && l) {
- /* Address above 32-bit boundary; disable the BAR */
- pci_write_config_dword(dev, pos, 0);
- pci_write_config_dword(dev, pos + 4, 0);
+ /* Above 32-bit boundary; try to reallocate */
res->flags |= IORESOURCE_UNSET;
res->start = 0;
res->end = sz64;
- bar_disabled = true;
+ bar_too_high = true;
goto out;
} else {
region.start = l64;
@@ -311,7 +309,10 @@ out:
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 (res->flags && !bar_disabled)
+ 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 (res->flags)
dev_printk(KERN_DEBUG, &dev->dev, "reg 0x%x: %pR\n", pos, res);
return (res->flags & IORESOURCE_MEM_64) ? 1 : 0;