On Wed, Mar 04, 2015 at 03:12:04PM +0800, Daniel J Blueman wrote: > Your patch solves the conflicts nicely [1] with: > > From f835b16b0758a1dde6042a0e4c8aa5a2e8be5f21 Mon Sep 17 00:00:00 2001 > From: Daniel J Blueman <daniel@xxxxxxxxxxxxx> > Date: Wed, 4 Mar 2015 14:53:00 +0800 > Subject: [PATCH] Mark PCI BARs with address 0 as unset > > Allow the kernel to activate the unset flag for PCI BAR resources if > the firmware assigns address 0 (invalid as legacy IO is in this range). > > This allows preventing conflicts with legacy IO/ACPI PNP resources in > this range. > > Signed-off-by: Daniel J Blueman <daniel@xxxxxxxxxxxxx> > --- > drivers/pci/probe.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 8d2f400..ef43652 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -281,6 +281,13 @@ int __pci_read_base(struct pci_dev *dev, enum > pci_bar_type type, > pcibios_resource_to_bus(dev->bus, &inverted_region, res); > > /* > + * If firmware doesn't assign a valid PCI address (as legacy IO is below > + * PCI IO), mark resource unset to prevent later resource conflicts > + */ > + if (region.start == 0) > + res->flags |= IORESOURCE_UNSET; It's true that an uninitialized BAR should contain zero. But an initialized BAR may also contain zero, since zero is a valid PCI memory or I/O address, so I don't really want to preclude that here. On large systems with host bridges that support address translation, it would be reasonable to have something like this: pci_bus 0001:00: root bus resource [mem 0x100000000-0x1ffffffff] (bus address [0x00000000-0xffffffff]) In that case, an initialized BAR may contain zero and that should not be an error. On your system, I don't think you advertise an I/O aperture to bus 0001:00. I'd like to make the PCI core smart enough to notice that and just ignore any I/O BARs on that bus. There's an argument for doing this immediately, here inside __pci_read_base(): we could look for an upstream window that contains the BAR we're reading. I'd like to be able to do that someday, but I'm not sure we have enough of the upstream topology set up to do that. Can you try the patch below, which tries to do it a little later? > + /* > * If "A" is a BAR value (a bus address), "bus_to_resource(A)" is > * the corresponding resource address (the physical address used by > * the CPU. Converting that resource address back to a bus address > > [1] https://resource.numascale.com/dmesg-4.0.0-rc2.txt This URL doesn't work for me. Bjorn commit 66c15b678466cb217f2615d4078d12a2ee4c99ac Author: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> Date: Wed Mar 4 10:47:35 2015 -0600 PCI: Mark invalid BARs as unassigned If a BAR is not inside any upstream bridge window, or if it conflicts with another resource, mark it as IORESOURCE_UNSET so we don't try to use it. We may be able to assign a different address for it. Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c index b7c3a5ea1fca..232f9254c11a 100644 --- a/drivers/pci/setup-res.c +++ b/drivers/pci/setup-res.c @@ -120,6 +120,7 @@ int pci_claim_resource(struct pci_dev *dev, int resource) if (!root) { dev_info(&dev->dev, "can't claim BAR %d %pR: no compatible bridge window\n", resource, res); + res->flags |= IORESOURCE_UNSET; return -EINVAL; } @@ -127,6 +128,7 @@ int pci_claim_resource(struct pci_dev *dev, int resource) if (conflict) { dev_info(&dev->dev, "can't claim BAR %d %pR: address conflict with %s %pR\n", resource, res, conflict->name, conflict); + res->flags |= IORESOURCE_UNSET; return -EBUSY; } -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
