* [PATCH] PCI: Probe bridge window attributes once at enumeration-time @ 2019-01-22 19:02 Bjorn Helgaas 2019-01-29 22:43 ` Bjorn Helgaas 0 siblings, 1 reply; 7+ messages in thread From: Bjorn Helgaas @ 2019-01-22 19:02 UTC (permalink / raw) To: linux-pci Cc: Michael S. Tsirkin, xuyandong, Yinghai Lu, Jesse Barnes, Sagi Grimberg, Ofer Hayut, Roy Shterman, Keith Busch, Wangzhou (B), linux-kernel From: Bjorn Helgaas <bhelgaas@google.com> pci_bridge_check_ranges() determines whether a bridge supports the optional I/O and prefetchable memory windows and sets the flag bits in the bridge resources. This *could* be done once during enumeration except that the resource allocation code completely clears the flag bits, e.g., in the pci_assign_unassigned_bridge_resources() path. The problem with pci_bridge_check_ranges() in the resource allocation path is that we may allocate resources after devices have been claimed by drivers, and pci_bridge_check_ranges() *changes* the window registers to determine whether they're writable. This may break concurrent accesses to devices behind the bridge. Add a new pci_read_bridge_windows() to determine whether a bridge supports the optional windows, call it once during enumeration, remember the results, and change pci_bridge_check_ranges() so it doesn't touch the bridge windows but sets the flag bits based on those remembered results. Link: https://lore.kernel.org/linux-pci/1506151482-113560-1-git-send-email-wangzhou1@hisilicon.com Link: https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg02082.html Reported-by: xuyandong <xuyandong2@huawei.com> Tested-by: xuyandong <xuyandong2@huawei.com> Cc: Sagi Grimberg <sagi@grimberg.me> Cc: Ofer Hayut <ofer@lightbitslabs.com> Cc: Roy Shterman <roys@lightbitslabs.com> Cc: Keith Busch <keith.busch@intel.com> Cc: Zhou Wang <wangzhou1@hisilicon.com> --- drivers/pci/probe.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++ drivers/pci/setup-bus.c | 45 ++++------------------------------------- include/linux/pci.h | 3 +++ 3 files changed, 59 insertions(+), 41 deletions(-) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 257b9f6f2ebb..2ef8b954c65a 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -348,6 +348,57 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom) } } +static void pci_read_bridge_windows(struct pci_dev *bridge) +{ + u16 io; + u32 pmem, tmp; + + pci_read_config_word(bridge, PCI_IO_BASE, &io); + if (!io) { + pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0); + pci_read_config_word(bridge, PCI_IO_BASE, &io); + pci_write_config_word(bridge, PCI_IO_BASE, 0x0); + } + if (io) + bridge->io_window = 1; + + /* + * DECchip 21050 pass 2 errata: the bridge may miss an address + * disconnect boundary by one PCI data phase. Workaround: do not + * use prefetching on this device. + */ + if (bridge->vendor == PCI_VENDOR_ID_DEC && bridge->device == 0x0001) + return; + + pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); + if (!pmem) { + pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, + 0xffe0fff0); + pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); + pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, 0x0); + } + if (!pmem) + return; + + bridge->pref_window = 1; + + if ((pmem & PCI_PREF_RANGE_TYPE_MASK) == PCI_PREF_RANGE_TYPE_64) { + + /* + * Bridge claims to have a 64-bit prefetchable memory + * window; verify that the upper bits are actually + * writable. + */ + pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &pmem); + pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, + 0xffffffff); + pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp); + pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, pmem); + if (tmp) + bridge->pref_64_window = 1; + } +} + static void pci_read_bridge_io(struct pci_bus *child) { struct pci_dev *dev = child->self; @@ -1739,6 +1790,7 @@ int pci_setup_device(struct pci_dev *dev) pci_read_irq(dev); dev->transparent = ((dev->class & 0xff) == 1); pci_read_bases(dev, 2, PCI_ROM_ADDRESS1); + pci_read_bridge_windows(dev); set_pcie_hotplug_bridge(dev); pos = pci_find_capability(dev, PCI_CAP_ID_SSVID); if (pos) { diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index ed960436df5e..1941bb0a6c13 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -735,58 +735,21 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i) base/limit registers must be read-only and read as 0. */ static void pci_bridge_check_ranges(struct pci_bus *bus) { - u16 io; - u32 pmem; struct pci_dev *bridge = bus->self; - struct resource *b_res; + struct resource *b_res = &bridge->resource[PCI_BRIDGE_RESOURCES]; - b_res = &bridge->resource[PCI_BRIDGE_RESOURCES]; b_res[1].flags |= IORESOURCE_MEM; - pci_read_config_word(bridge, PCI_IO_BASE, &io); - if (!io) { - pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0); - pci_read_config_word(bridge, PCI_IO_BASE, &io); - pci_write_config_word(bridge, PCI_IO_BASE, 0x0); - } - if (io) + if (bridge->io_window) b_res[0].flags |= IORESOURCE_IO; - /* DECchip 21050 pass 2 errata: the bridge may miss an address - disconnect boundary by one PCI data phase. - Workaround: do not use prefetching on this device. */ - if (bridge->vendor == PCI_VENDOR_ID_DEC && bridge->device == 0x0001) - return; - - pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); - if (!pmem) { - pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, - 0xffe0fff0); - pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); - pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, 0x0); - } - if (pmem) { + if (bridge->pref_window) { b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH; - if ((pmem & PCI_PREF_RANGE_TYPE_MASK) == - PCI_PREF_RANGE_TYPE_64) { + if (bridge->pref_64_window) { b_res[2].flags |= IORESOURCE_MEM_64; b_res[2].flags |= PCI_PREF_RANGE_TYPE_64; } } - - /* double check if bridge does support 64 bit pref */ - if (b_res[2].flags & IORESOURCE_MEM_64) { - u32 mem_base_hi, tmp; - pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, - &mem_base_hi); - pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, - 0xffffffff); - pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp); - if (!tmp) - b_res[2].flags &= ~IORESOURCE_MEM_64; - pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, - mem_base_hi); - } } /* Helper function for sizing routines: find first available diff --git a/include/linux/pci.h b/include/linux/pci.h index 65f1d8c2f082..40b327b814aa 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -373,6 +373,9 @@ struct pci_dev { bool match_driver; /* Skip attaching driver */ unsigned int transparent:1; /* Subtractive decode bridge */ + unsigned int io_window:1; /* Bridge has I/O window */ + unsigned int pref_window:1; /* Bridge has pref mem window */ + unsigned int pref_64_window:1; /* Pref mem window is 64-bit */ unsigned int multifunction:1; /* Multi-function device */ unsigned int is_busmaster:1; /* Is busmaster */ ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: Probe bridge window attributes once at enumeration-time 2019-01-22 19:02 [PATCH] PCI: Probe bridge window attributes once at enumeration-time Bjorn Helgaas @ 2019-01-29 22:43 ` Bjorn Helgaas 2019-01-29 22:47 ` Michael S. Tsirkin 0 siblings, 1 reply; 7+ messages in thread From: Bjorn Helgaas @ 2019-01-29 22:43 UTC (permalink / raw) To: linux-pci Cc: Michael S. Tsirkin, xuyandong, Yinghai Lu, Jesse Barnes, Sagi Grimberg, Ofer Hayut, Roy Shterman, Keith Busch, Wangzhou (B), linux-kernel On Tue, Jan 22, 2019 at 01:02:54PM -0600, Bjorn Helgaas wrote: > From: Bjorn Helgaas <bhelgaas@google.com> > > pci_bridge_check_ranges() determines whether a bridge supports the optional > I/O and prefetchable memory windows and sets the flag bits in the bridge > resources. This *could* be done once during enumeration except that the > resource allocation code completely clears the flag bits, e.g., in the > pci_assign_unassigned_bridge_resources() path. > > The problem with pci_bridge_check_ranges() in the resource allocation path > is that we may allocate resources after devices have been claimed by > drivers, and pci_bridge_check_ranges() *changes* the window registers to > determine whether they're writable. This may break concurrent accesses to > devices behind the bridge. > > Add a new pci_read_bridge_windows() to determine whether a bridge supports > the optional windows, call it once during enumeration, remember the > results, and change pci_bridge_check_ranges() so it doesn't touch the > bridge windows but sets the flag bits based on those remembered results. > > Link: https://lore.kernel.org/linux-pci/1506151482-113560-1-git-send-email-wangzhou1@hisilicon.com > Link: https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg02082.html > Reported-by: xuyandong <xuyandong2@huawei.com> > Tested-by: xuyandong <xuyandong2@huawei.com> > Cc: Sagi Grimberg <sagi@grimberg.me> > Cc: Ofer Hayut <ofer@lightbitslabs.com> > Cc: Roy Shterman <roys@lightbitslabs.com> > Cc: Keith Busch <keith.busch@intel.com> > Cc: Zhou Wang <wangzhou1@hisilicon.com> Applied to pci/enumeration for v5.1. This is fairly simple in concept, but doesn't meet the letter of the restrictions in Documentation/process/stable-kernel-rules.rst, so I didn't tag it for stable. Is there a compelling argument to mark it for stable? > --- > drivers/pci/probe.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++ > drivers/pci/setup-bus.c | 45 ++++------------------------------------- > include/linux/pci.h | 3 +++ > 3 files changed, 59 insertions(+), 41 deletions(-) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 257b9f6f2ebb..2ef8b954c65a 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -348,6 +348,57 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom) > } > } > > +static void pci_read_bridge_windows(struct pci_dev *bridge) > +{ > + u16 io; > + u32 pmem, tmp; > + > + pci_read_config_word(bridge, PCI_IO_BASE, &io); > + if (!io) { > + pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0); > + pci_read_config_word(bridge, PCI_IO_BASE, &io); > + pci_write_config_word(bridge, PCI_IO_BASE, 0x0); > + } > + if (io) > + bridge->io_window = 1; > + > + /* > + * DECchip 21050 pass 2 errata: the bridge may miss an address > + * disconnect boundary by one PCI data phase. Workaround: do not > + * use prefetching on this device. > + */ > + if (bridge->vendor == PCI_VENDOR_ID_DEC && bridge->device == 0x0001) > + return; > + > + pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); > + if (!pmem) { > + pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, > + 0xffe0fff0); > + pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); > + pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, 0x0); > + } > + if (!pmem) > + return; > + > + bridge->pref_window = 1; > + > + if ((pmem & PCI_PREF_RANGE_TYPE_MASK) == PCI_PREF_RANGE_TYPE_64) { > + > + /* > + * Bridge claims to have a 64-bit prefetchable memory > + * window; verify that the upper bits are actually > + * writable. > + */ > + pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &pmem); > + pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, > + 0xffffffff); > + pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp); > + pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, pmem); > + if (tmp) > + bridge->pref_64_window = 1; > + } > +} > + > static void pci_read_bridge_io(struct pci_bus *child) > { > struct pci_dev *dev = child->self; > @@ -1739,6 +1790,7 @@ int pci_setup_device(struct pci_dev *dev) > pci_read_irq(dev); > dev->transparent = ((dev->class & 0xff) == 1); > pci_read_bases(dev, 2, PCI_ROM_ADDRESS1); > + pci_read_bridge_windows(dev); > set_pcie_hotplug_bridge(dev); > pos = pci_find_capability(dev, PCI_CAP_ID_SSVID); > if (pos) { > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > index ed960436df5e..1941bb0a6c13 100644 > --- a/drivers/pci/setup-bus.c > +++ b/drivers/pci/setup-bus.c > @@ -735,58 +735,21 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i) > base/limit registers must be read-only and read as 0. */ > static void pci_bridge_check_ranges(struct pci_bus *bus) > { > - u16 io; > - u32 pmem; > struct pci_dev *bridge = bus->self; > - struct resource *b_res; > + struct resource *b_res = &bridge->resource[PCI_BRIDGE_RESOURCES]; > > - b_res = &bridge->resource[PCI_BRIDGE_RESOURCES]; > b_res[1].flags |= IORESOURCE_MEM; > > - pci_read_config_word(bridge, PCI_IO_BASE, &io); > - if (!io) { > - pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0); > - pci_read_config_word(bridge, PCI_IO_BASE, &io); > - pci_write_config_word(bridge, PCI_IO_BASE, 0x0); > - } > - if (io) > + if (bridge->io_window) > b_res[0].flags |= IORESOURCE_IO; > > - /* DECchip 21050 pass 2 errata: the bridge may miss an address > - disconnect boundary by one PCI data phase. > - Workaround: do not use prefetching on this device. */ > - if (bridge->vendor == PCI_VENDOR_ID_DEC && bridge->device == 0x0001) > - return; > - > - pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); > - if (!pmem) { > - pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, > - 0xffe0fff0); > - pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); > - pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, 0x0); > - } > - if (pmem) { > + if (bridge->pref_window) { > b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH; > - if ((pmem & PCI_PREF_RANGE_TYPE_MASK) == > - PCI_PREF_RANGE_TYPE_64) { > + if (bridge->pref_64_window) { > b_res[2].flags |= IORESOURCE_MEM_64; > b_res[2].flags |= PCI_PREF_RANGE_TYPE_64; > } > } > - > - /* double check if bridge does support 64 bit pref */ > - if (b_res[2].flags & IORESOURCE_MEM_64) { > - u32 mem_base_hi, tmp; > - pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, > - &mem_base_hi); > - pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, > - 0xffffffff); > - pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp); > - if (!tmp) > - b_res[2].flags &= ~IORESOURCE_MEM_64; > - pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, > - mem_base_hi); > - } > } > > /* Helper function for sizing routines: find first available > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 65f1d8c2f082..40b327b814aa 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -373,6 +373,9 @@ struct pci_dev { > bool match_driver; /* Skip attaching driver */ > > unsigned int transparent:1; /* Subtractive decode bridge */ > + unsigned int io_window:1; /* Bridge has I/O window */ > + unsigned int pref_window:1; /* Bridge has pref mem window */ > + unsigned int pref_64_window:1; /* Pref mem window is 64-bit */ > unsigned int multifunction:1; /* Multi-function device */ > > unsigned int is_busmaster:1; /* Is busmaster */ > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: Probe bridge window attributes once at enumeration-time 2019-01-29 22:43 ` Bjorn Helgaas @ 2019-01-29 22:47 ` Michael S. Tsirkin 2019-01-29 23:02 ` Bjorn Helgaas 0 siblings, 1 reply; 7+ messages in thread From: Michael S. Tsirkin @ 2019-01-29 22:47 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-pci, xuyandong, Yinghai Lu, Jesse Barnes, Sagi Grimberg, Ofer Hayut, Roy Shterman, Keith Busch, Wangzhou (B), linux-kernel On Tue, Jan 29, 2019 at 04:43:33PM -0600, Bjorn Helgaas wrote: > On Tue, Jan 22, 2019 at 01:02:54PM -0600, Bjorn Helgaas wrote: > > From: Bjorn Helgaas <bhelgaas@google.com> > > > > pci_bridge_check_ranges() determines whether a bridge supports the optional > > I/O and prefetchable memory windows and sets the flag bits in the bridge > > resources. This *could* be done once during enumeration except that the > > resource allocation code completely clears the flag bits, e.g., in the > > pci_assign_unassigned_bridge_resources() path. > > > > The problem with pci_bridge_check_ranges() in the resource allocation path > > is that we may allocate resources after devices have been claimed by > > drivers, and pci_bridge_check_ranges() *changes* the window registers to > > determine whether they're writable. This may break concurrent accesses to > > devices behind the bridge. > > > > Add a new pci_read_bridge_windows() to determine whether a bridge supports > > the optional windows, call it once during enumeration, remember the > > results, and change pci_bridge_check_ranges() so it doesn't touch the > > bridge windows but sets the flag bits based on those remembered results. > > > > Link: https://lore.kernel.org/linux-pci/1506151482-113560-1-git-send-email-wangzhou1@hisilicon.com > > Link: https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg02082.html > > Reported-by: xuyandong <xuyandong2@huawei.com> > > Tested-by: xuyandong <xuyandong2@huawei.com> > > Cc: Sagi Grimberg <sagi@grimberg.me> > > Cc: Ofer Hayut <ofer@lightbitslabs.com> > > Cc: Roy Shterman <roys@lightbitslabs.com> > > Cc: Keith Busch <keith.busch@intel.com> > > Cc: Zhou Wang <wangzhou1@hisilicon.com> > > Applied to pci/enumeration for v5.1. > > This is fairly simple in concept, but doesn't meet the letter of the > restrictions in Documentation/process/stable-kernel-rules.rst, so I > didn't tag it for stable. > > Is there a compelling argument to mark it for stable? Well it's needed downstream. It's a bit above 100 lines with context. Is this what you mean? If yes how about using my patch for stable as an alternative? The rules allow for equivalent patches. > > --- > > drivers/pci/probe.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++ > > drivers/pci/setup-bus.c | 45 ++++------------------------------------- > > include/linux/pci.h | 3 +++ > > 3 files changed, 59 insertions(+), 41 deletions(-) > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > > index 257b9f6f2ebb..2ef8b954c65a 100644 > > --- a/drivers/pci/probe.c > > +++ b/drivers/pci/probe.c > > @@ -348,6 +348,57 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom) > > } > > } > > > > +static void pci_read_bridge_windows(struct pci_dev *bridge) > > +{ > > + u16 io; > > + u32 pmem, tmp; > > + > > + pci_read_config_word(bridge, PCI_IO_BASE, &io); > > + if (!io) { > > + pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0); > > + pci_read_config_word(bridge, PCI_IO_BASE, &io); > > + pci_write_config_word(bridge, PCI_IO_BASE, 0x0); > > + } > > + if (io) > > + bridge->io_window = 1; > > + > > + /* > > + * DECchip 21050 pass 2 errata: the bridge may miss an address > > + * disconnect boundary by one PCI data phase. Workaround: do not > > + * use prefetching on this device. > > + */ > > + if (bridge->vendor == PCI_VENDOR_ID_DEC && bridge->device == 0x0001) > > + return; > > + > > + pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); > > + if (!pmem) { > > + pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, > > + 0xffe0fff0); > > + pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); > > + pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, 0x0); > > + } > > + if (!pmem) > > + return; > > + > > + bridge->pref_window = 1; > > + > > + if ((pmem & PCI_PREF_RANGE_TYPE_MASK) == PCI_PREF_RANGE_TYPE_64) { > > + > > + /* > > + * Bridge claims to have a 64-bit prefetchable memory > > + * window; verify that the upper bits are actually > > + * writable. > > + */ > > + pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &pmem); > > + pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, > > + 0xffffffff); > > + pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp); > > + pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, pmem); > > + if (tmp) > > + bridge->pref_64_window = 1; > > + } > > +} > > + > > static void pci_read_bridge_io(struct pci_bus *child) > > { > > struct pci_dev *dev = child->self; > > @@ -1739,6 +1790,7 @@ int pci_setup_device(struct pci_dev *dev) > > pci_read_irq(dev); > > dev->transparent = ((dev->class & 0xff) == 1); > > pci_read_bases(dev, 2, PCI_ROM_ADDRESS1); > > + pci_read_bridge_windows(dev); > > set_pcie_hotplug_bridge(dev); > > pos = pci_find_capability(dev, PCI_CAP_ID_SSVID); > > if (pos) { > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > > index ed960436df5e..1941bb0a6c13 100644 > > --- a/drivers/pci/setup-bus.c > > +++ b/drivers/pci/setup-bus.c > > @@ -735,58 +735,21 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i) > > base/limit registers must be read-only and read as 0. */ > > static void pci_bridge_check_ranges(struct pci_bus *bus) > > { > > - u16 io; > > - u32 pmem; > > struct pci_dev *bridge = bus->self; > > - struct resource *b_res; > > + struct resource *b_res = &bridge->resource[PCI_BRIDGE_RESOURCES]; > > > > - b_res = &bridge->resource[PCI_BRIDGE_RESOURCES]; > > b_res[1].flags |= IORESOURCE_MEM; > > > > - pci_read_config_word(bridge, PCI_IO_BASE, &io); > > - if (!io) { > > - pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0); > > - pci_read_config_word(bridge, PCI_IO_BASE, &io); > > - pci_write_config_word(bridge, PCI_IO_BASE, 0x0); > > - } > > - if (io) > > + if (bridge->io_window) > > b_res[0].flags |= IORESOURCE_IO; > > > > - /* DECchip 21050 pass 2 errata: the bridge may miss an address > > - disconnect boundary by one PCI data phase. > > - Workaround: do not use prefetching on this device. */ > > - if (bridge->vendor == PCI_VENDOR_ID_DEC && bridge->device == 0x0001) > > - return; > > - > > - pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); > > - if (!pmem) { > > - pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, > > - 0xffe0fff0); > > - pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); > > - pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, 0x0); > > - } > > - if (pmem) { > > + if (bridge->pref_window) { > > b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH; > > - if ((pmem & PCI_PREF_RANGE_TYPE_MASK) == > > - PCI_PREF_RANGE_TYPE_64) { > > + if (bridge->pref_64_window) { > > b_res[2].flags |= IORESOURCE_MEM_64; > > b_res[2].flags |= PCI_PREF_RANGE_TYPE_64; > > } > > } > > - > > - /* double check if bridge does support 64 bit pref */ > > - if (b_res[2].flags & IORESOURCE_MEM_64) { > > - u32 mem_base_hi, tmp; > > - pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, > > - &mem_base_hi); > > - pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, > > - 0xffffffff); > > - pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp); > > - if (!tmp) > > - b_res[2].flags &= ~IORESOURCE_MEM_64; > > - pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, > > - mem_base_hi); > > - } > > } > > > > /* Helper function for sizing routines: find first available > > diff --git a/include/linux/pci.h b/include/linux/pci.h > > index 65f1d8c2f082..40b327b814aa 100644 > > --- a/include/linux/pci.h > > +++ b/include/linux/pci.h > > @@ -373,6 +373,9 @@ struct pci_dev { > > bool match_driver; /* Skip attaching driver */ > > > > unsigned int transparent:1; /* Subtractive decode bridge */ > > + unsigned int io_window:1; /* Bridge has I/O window */ > > + unsigned int pref_window:1; /* Bridge has pref mem window */ > > + unsigned int pref_64_window:1; /* Pref mem window is 64-bit */ > > unsigned int multifunction:1; /* Multi-function device */ > > > > unsigned int is_busmaster:1; /* Is busmaster */ > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: Probe bridge window attributes once at enumeration-time 2019-01-29 22:47 ` Michael S. Tsirkin @ 2019-01-29 23:02 ` Bjorn Helgaas 2019-02-05 18:24 ` Michael S. Tsirkin 0 siblings, 1 reply; 7+ messages in thread From: Bjorn Helgaas @ 2019-01-29 23:02 UTC (permalink / raw) To: Michael S. Tsirkin Cc: linux-pci, xuyandong, Yinghai Lu, Jesse Barnes, Sagi Grimberg, Ofer Hayut, Roy Shterman, Keith Busch, Wangzhou (B), linux-kernel On Tue, Jan 29, 2019 at 05:47:32PM -0500, Michael S. Tsirkin wrote: > On Tue, Jan 29, 2019 at 04:43:33PM -0600, Bjorn Helgaas wrote: > > On Tue, Jan 22, 2019 at 01:02:54PM -0600, Bjorn Helgaas wrote: > > > From: Bjorn Helgaas <bhelgaas@google.com> > > > > > > pci_bridge_check_ranges() determines whether a bridge supports the optional > > > I/O and prefetchable memory windows and sets the flag bits in the bridge > > > resources. This *could* be done once during enumeration except that the > > > resource allocation code completely clears the flag bits, e.g., in the > > > pci_assign_unassigned_bridge_resources() path. > > > > > > The problem with pci_bridge_check_ranges() in the resource allocation path > > > is that we may allocate resources after devices have been claimed by > > > drivers, and pci_bridge_check_ranges() *changes* the window registers to > > > determine whether they're writable. This may break concurrent accesses to > > > devices behind the bridge. > > > > > > Add a new pci_read_bridge_windows() to determine whether a bridge supports > > > the optional windows, call it once during enumeration, remember the > > > results, and change pci_bridge_check_ranges() so it doesn't touch the > > > bridge windows but sets the flag bits based on those remembered results. > > > > > > Link: https://lore.kernel.org/linux-pci/1506151482-113560-1-git-send-email-wangzhou1@hisilicon.com > > > Link: https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg02082.html > > > Reported-by: xuyandong <xuyandong2@huawei.com> > > > Tested-by: xuyandong <xuyandong2@huawei.com> > > > Cc: Sagi Grimberg <sagi@grimberg.me> > > > Cc: Ofer Hayut <ofer@lightbitslabs.com> > > > Cc: Roy Shterman <roys@lightbitslabs.com> > > > Cc: Keith Busch <keith.busch@intel.com> > > > Cc: Zhou Wang <wangzhou1@hisilicon.com> > > > > Applied to pci/enumeration for v5.1. > > > > This is fairly simple in concept, but doesn't meet the letter of the > > restrictions in Documentation/process/stable-kernel-rules.rst, so I > > didn't tag it for stable. > > > > Is there a compelling argument to mark it for stable? > > Well it's needed downstream. > It's a bit above 100 lines with context. Is this what you mean? Yep, I should have been explicit about that. > If yes how about using my patch for stable as an alternative? > The rules allow for equivalent patches. Well, yes, that would be a possibility. I would rather develop an argument for marking *this* patch for stable. Even though it is technically too large, I think we could make it work if we have compelling reasons. Those would need to be fleshed out a little more than "needed downstream" -- something about the scenarios where this happens, what the serious problem is, etc. The *ideal* thing would be to have an actual bugzilla.kernel.org report of the problem with a way to reproduce it and dmesg illustrating the problem. > > > --- > > > drivers/pci/probe.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++ > > > drivers/pci/setup-bus.c | 45 ++++------------------------------------- > > > include/linux/pci.h | 3 +++ > > > 3 files changed, 59 insertions(+), 41 deletions(-) > > > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > > > index 257b9f6f2ebb..2ef8b954c65a 100644 > > > --- a/drivers/pci/probe.c > > > +++ b/drivers/pci/probe.c > > > @@ -348,6 +348,57 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom) > > > } > > > } > > > > > > +static void pci_read_bridge_windows(struct pci_dev *bridge) > > > +{ > > > + u16 io; > > > + u32 pmem, tmp; > > > + > > > + pci_read_config_word(bridge, PCI_IO_BASE, &io); > > > + if (!io) { > > > + pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0); > > > + pci_read_config_word(bridge, PCI_IO_BASE, &io); > > > + pci_write_config_word(bridge, PCI_IO_BASE, 0x0); > > > + } > > > + if (io) > > > + bridge->io_window = 1; > > > + > > > + /* > > > + * DECchip 21050 pass 2 errata: the bridge may miss an address > > > + * disconnect boundary by one PCI data phase. Workaround: do not > > > + * use prefetching on this device. > > > + */ > > > + if (bridge->vendor == PCI_VENDOR_ID_DEC && bridge->device == 0x0001) > > > + return; > > > + > > > + pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); > > > + if (!pmem) { > > > + pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, > > > + 0xffe0fff0); > > > + pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); > > > + pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, 0x0); > > > + } > > > + if (!pmem) > > > + return; > > > + > > > + bridge->pref_window = 1; > > > + > > > + if ((pmem & PCI_PREF_RANGE_TYPE_MASK) == PCI_PREF_RANGE_TYPE_64) { > > > + > > > + /* > > > + * Bridge claims to have a 64-bit prefetchable memory > > > + * window; verify that the upper bits are actually > > > + * writable. > > > + */ > > > + pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &pmem); > > > + pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, > > > + 0xffffffff); > > > + pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp); > > > + pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, pmem); > > > + if (tmp) > > > + bridge->pref_64_window = 1; > > > + } > > > +} > > > + > > > static void pci_read_bridge_io(struct pci_bus *child) > > > { > > > struct pci_dev *dev = child->self; > > > @@ -1739,6 +1790,7 @@ int pci_setup_device(struct pci_dev *dev) > > > pci_read_irq(dev); > > > dev->transparent = ((dev->class & 0xff) == 1); > > > pci_read_bases(dev, 2, PCI_ROM_ADDRESS1); > > > + pci_read_bridge_windows(dev); > > > set_pcie_hotplug_bridge(dev); > > > pos = pci_find_capability(dev, PCI_CAP_ID_SSVID); > > > if (pos) { > > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > > > index ed960436df5e..1941bb0a6c13 100644 > > > --- a/drivers/pci/setup-bus.c > > > +++ b/drivers/pci/setup-bus.c > > > @@ -735,58 +735,21 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i) > > > base/limit registers must be read-only and read as 0. */ > > > static void pci_bridge_check_ranges(struct pci_bus *bus) > > > { > > > - u16 io; > > > - u32 pmem; > > > struct pci_dev *bridge = bus->self; > > > - struct resource *b_res; > > > + struct resource *b_res = &bridge->resource[PCI_BRIDGE_RESOURCES]; > > > > > > - b_res = &bridge->resource[PCI_BRIDGE_RESOURCES]; > > > b_res[1].flags |= IORESOURCE_MEM; > > > > > > - pci_read_config_word(bridge, PCI_IO_BASE, &io); > > > - if (!io) { > > > - pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0); > > > - pci_read_config_word(bridge, PCI_IO_BASE, &io); > > > - pci_write_config_word(bridge, PCI_IO_BASE, 0x0); > > > - } > > > - if (io) > > > + if (bridge->io_window) > > > b_res[0].flags |= IORESOURCE_IO; > > > > > > - /* DECchip 21050 pass 2 errata: the bridge may miss an address > > > - disconnect boundary by one PCI data phase. > > > - Workaround: do not use prefetching on this device. */ > > > - if (bridge->vendor == PCI_VENDOR_ID_DEC && bridge->device == 0x0001) > > > - return; > > > - > > > - pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); > > > - if (!pmem) { > > > - pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, > > > - 0xffe0fff0); > > > - pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); > > > - pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, 0x0); > > > - } > > > - if (pmem) { > > > + if (bridge->pref_window) { > > > b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH; > > > - if ((pmem & PCI_PREF_RANGE_TYPE_MASK) == > > > - PCI_PREF_RANGE_TYPE_64) { > > > + if (bridge->pref_64_window) { > > > b_res[2].flags |= IORESOURCE_MEM_64; > > > b_res[2].flags |= PCI_PREF_RANGE_TYPE_64; > > > } > > > } > > > - > > > - /* double check if bridge does support 64 bit pref */ > > > - if (b_res[2].flags & IORESOURCE_MEM_64) { > > > - u32 mem_base_hi, tmp; > > > - pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, > > > - &mem_base_hi); > > > - pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, > > > - 0xffffffff); > > > - pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp); > > > - if (!tmp) > > > - b_res[2].flags &= ~IORESOURCE_MEM_64; > > > - pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, > > > - mem_base_hi); > > > - } > > > } > > > > > > /* Helper function for sizing routines: find first available > > > diff --git a/include/linux/pci.h b/include/linux/pci.h > > > index 65f1d8c2f082..40b327b814aa 100644 > > > --- a/include/linux/pci.h > > > +++ b/include/linux/pci.h > > > @@ -373,6 +373,9 @@ struct pci_dev { > > > bool match_driver; /* Skip attaching driver */ > > > > > > unsigned int transparent:1; /* Subtractive decode bridge */ > > > + unsigned int io_window:1; /* Bridge has I/O window */ > > > + unsigned int pref_window:1; /* Bridge has pref mem window */ > > > + unsigned int pref_64_window:1; /* Pref mem window is 64-bit */ > > > unsigned int multifunction:1; /* Multi-function device */ > > > > > > unsigned int is_busmaster:1; /* Is busmaster */ > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: Probe bridge window attributes once at enumeration-time 2019-01-29 23:02 ` Bjorn Helgaas @ 2019-02-05 18:24 ` Michael S. Tsirkin 2019-02-11 1:57 ` Xuyandong (Yandong Xu, Euler5) 0 siblings, 1 reply; 7+ messages in thread From: Michael S. Tsirkin @ 2019-02-05 18:24 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-pci, xuyandong, Yinghai Lu, Jesse Barnes, Sagi Grimberg, Ofer Hayut, Roy Shterman, Keith Busch, Wangzhou (B), linux-kernel On Tue, Jan 29, 2019 at 05:02:26PM -0600, Bjorn Helgaas wrote: > On Tue, Jan 29, 2019 at 05:47:32PM -0500, Michael S. Tsirkin wrote: > > On Tue, Jan 29, 2019 at 04:43:33PM -0600, Bjorn Helgaas wrote: > > > On Tue, Jan 22, 2019 at 01:02:54PM -0600, Bjorn Helgaas wrote: > > > > From: Bjorn Helgaas <bhelgaas@google.com> > > > > > > > > pci_bridge_check_ranges() determines whether a bridge supports the optional > > > > I/O and prefetchable memory windows and sets the flag bits in the bridge > > > > resources. This *could* be done once during enumeration except that the > > > > resource allocation code completely clears the flag bits, e.g., in the > > > > pci_assign_unassigned_bridge_resources() path. > > > > > > > > The problem with pci_bridge_check_ranges() in the resource allocation path > > > > is that we may allocate resources after devices have been claimed by > > > > drivers, and pci_bridge_check_ranges() *changes* the window registers to > > > > determine whether they're writable. This may break concurrent accesses to > > > > devices behind the bridge. > > > > > > > > Add a new pci_read_bridge_windows() to determine whether a bridge supports > > > > the optional windows, call it once during enumeration, remember the > > > > results, and change pci_bridge_check_ranges() so it doesn't touch the > > > > bridge windows but sets the flag bits based on those remembered results. > > > > > > > > Link: https://lore.kernel.org/linux-pci/1506151482-113560-1-git-send-email-wangzhou1@hisilicon.com > > > > Link: https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg02082.html > > > > Reported-by: xuyandong <xuyandong2@huawei.com> > > > > Tested-by: xuyandong <xuyandong2@huawei.com> > > > > Cc: Sagi Grimberg <sagi@grimberg.me> > > > > Cc: Ofer Hayut <ofer@lightbitslabs.com> > > > > Cc: Roy Shterman <roys@lightbitslabs.com> > > > > Cc: Keith Busch <keith.busch@intel.com> > > > > Cc: Zhou Wang <wangzhou1@hisilicon.com> > > > > > > Applied to pci/enumeration for v5.1. > > > > > > This is fairly simple in concept, but doesn't meet the letter of the > > > restrictions in Documentation/process/stable-kernel-rules.rst, so I > > > didn't tag it for stable. > > > > > > Is there a compelling argument to mark it for stable? > > > > Well it's needed downstream. > > It's a bit above 100 lines with context. Is this what you mean? > > Yep, I should have been explicit about that. > > > If yes how about using my patch for stable as an alternative? > > The rules allow for equivalent patches. > > Well, yes, that would be a possibility. > > I would rather develop an argument for marking *this* patch for > stable. Even though it is technically too large, I think we could > make it work if we have compelling reasons. > > Those would need to be fleshed out a little more than "needed > downstream" -- something about the scenarios where this happens, what > the serious problem is, etc. > > The *ideal* thing would be to have an actual bugzilla.kernel.org > report of the problem with a way to reproduce it and dmesg > illustrating the problem. xuyandong would you like to take a stub at creating the bugzilla entry? If not let me know and I'll do it. Thanks! > > > > --- > > > > drivers/pci/probe.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++ > > > > drivers/pci/setup-bus.c | 45 ++++------------------------------------- > > > > include/linux/pci.h | 3 +++ > > > > 3 files changed, 59 insertions(+), 41 deletions(-) > > > > > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > > > > index 257b9f6f2ebb..2ef8b954c65a 100644 > > > > --- a/drivers/pci/probe.c > > > > +++ b/drivers/pci/probe.c > > > > @@ -348,6 +348,57 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom) > > > > } > > > > } > > > > > > > > +static void pci_read_bridge_windows(struct pci_dev *bridge) > > > > +{ > > > > + u16 io; > > > > + u32 pmem, tmp; > > > > + > > > > + pci_read_config_word(bridge, PCI_IO_BASE, &io); > > > > + if (!io) { > > > > + pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0); > > > > + pci_read_config_word(bridge, PCI_IO_BASE, &io); > > > > + pci_write_config_word(bridge, PCI_IO_BASE, 0x0); > > > > + } > > > > + if (io) > > > > + bridge->io_window = 1; > > > > + > > > > + /* > > > > + * DECchip 21050 pass 2 errata: the bridge may miss an address > > > > + * disconnect boundary by one PCI data phase. Workaround: do not > > > > + * use prefetching on this device. > > > > + */ > > > > + if (bridge->vendor == PCI_VENDOR_ID_DEC && bridge->device == 0x0001) > > > > + return; > > > > + > > > > + pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); > > > > + if (!pmem) { > > > > + pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, > > > > + 0xffe0fff0); > > > > + pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); > > > > + pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, 0x0); > > > > + } > > > > + if (!pmem) > > > > + return; > > > > + > > > > + bridge->pref_window = 1; > > > > + > > > > + if ((pmem & PCI_PREF_RANGE_TYPE_MASK) == PCI_PREF_RANGE_TYPE_64) { > > > > + > > > > + /* > > > > + * Bridge claims to have a 64-bit prefetchable memory > > > > + * window; verify that the upper bits are actually > > > > + * writable. > > > > + */ > > > > + pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &pmem); > > > > + pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, > > > > + 0xffffffff); > > > > + pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp); > > > > + pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, pmem); > > > > + if (tmp) > > > > + bridge->pref_64_window = 1; > > > > + } > > > > +} > > > > + > > > > static void pci_read_bridge_io(struct pci_bus *child) > > > > { > > > > struct pci_dev *dev = child->self; > > > > @@ -1739,6 +1790,7 @@ int pci_setup_device(struct pci_dev *dev) > > > > pci_read_irq(dev); > > > > dev->transparent = ((dev->class & 0xff) == 1); > > > > pci_read_bases(dev, 2, PCI_ROM_ADDRESS1); > > > > + pci_read_bridge_windows(dev); > > > > set_pcie_hotplug_bridge(dev); > > > > pos = pci_find_capability(dev, PCI_CAP_ID_SSVID); > > > > if (pos) { > > > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > > > > index ed960436df5e..1941bb0a6c13 100644 > > > > --- a/drivers/pci/setup-bus.c > > > > +++ b/drivers/pci/setup-bus.c > > > > @@ -735,58 +735,21 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i) > > > > base/limit registers must be read-only and read as 0. */ > > > > static void pci_bridge_check_ranges(struct pci_bus *bus) > > > > { > > > > - u16 io; > > > > - u32 pmem; > > > > struct pci_dev *bridge = bus->self; > > > > - struct resource *b_res; > > > > + struct resource *b_res = &bridge->resource[PCI_BRIDGE_RESOURCES]; > > > > > > > > - b_res = &bridge->resource[PCI_BRIDGE_RESOURCES]; > > > > b_res[1].flags |= IORESOURCE_MEM; > > > > > > > > - pci_read_config_word(bridge, PCI_IO_BASE, &io); > > > > - if (!io) { > > > > - pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0); > > > > - pci_read_config_word(bridge, PCI_IO_BASE, &io); > > > > - pci_write_config_word(bridge, PCI_IO_BASE, 0x0); > > > > - } > > > > - if (io) > > > > + if (bridge->io_window) > > > > b_res[0].flags |= IORESOURCE_IO; > > > > > > > > - /* DECchip 21050 pass 2 errata: the bridge may miss an address > > > > - disconnect boundary by one PCI data phase. > > > > - Workaround: do not use prefetching on this device. */ > > > > - if (bridge->vendor == PCI_VENDOR_ID_DEC && bridge->device == 0x0001) > > > > - return; > > > > - > > > > - pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); > > > > - if (!pmem) { > > > > - pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, > > > > - 0xffe0fff0); > > > > - pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem); > > > > - pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, 0x0); > > > > - } > > > > - if (pmem) { > > > > + if (bridge->pref_window) { > > > > b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH; > > > > - if ((pmem & PCI_PREF_RANGE_TYPE_MASK) == > > > > - PCI_PREF_RANGE_TYPE_64) { > > > > + if (bridge->pref_64_window) { > > > > b_res[2].flags |= IORESOURCE_MEM_64; > > > > b_res[2].flags |= PCI_PREF_RANGE_TYPE_64; > > > > } > > > > } > > > > - > > > > - /* double check if bridge does support 64 bit pref */ > > > > - if (b_res[2].flags & IORESOURCE_MEM_64) { > > > > - u32 mem_base_hi, tmp; > > > > - pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, > > > > - &mem_base_hi); > > > > - pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, > > > > - 0xffffffff); > > > > - pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp); > > > > - if (!tmp) > > > > - b_res[2].flags &= ~IORESOURCE_MEM_64; > > > > - pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, > > > > - mem_base_hi); > > > > - } > > > > } > > > > > > > > /* Helper function for sizing routines: find first available > > > > diff --git a/include/linux/pci.h b/include/linux/pci.h > > > > index 65f1d8c2f082..40b327b814aa 100644 > > > > --- a/include/linux/pci.h > > > > +++ b/include/linux/pci.h > > > > @@ -373,6 +373,9 @@ struct pci_dev { > > > > bool match_driver; /* Skip attaching driver */ > > > > > > > > unsigned int transparent:1; /* Subtractive decode bridge */ > > > > + unsigned int io_window:1; /* Bridge has I/O window */ > > > > + unsigned int pref_window:1; /* Bridge has pref mem window */ > > > > + unsigned int pref_64_window:1; /* Pref mem window is 64-bit */ > > > > unsigned int multifunction:1; /* Multi-function device */ > > > > > > > > unsigned int is_busmaster:1; /* Is busmaster */ > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] PCI: Probe bridge window attributes once at enumeration-time 2019-02-05 18:24 ` Michael S. Tsirkin @ 2019-02-11 1:57 ` Xuyandong (Yandong Xu, Euler5) 2019-02-11 3:48 ` Michael S. Tsirkin 0 siblings, 1 reply; 7+ messages in thread From: Xuyandong (Yandong Xu, Euler5) @ 2019-02-11 1:57 UTC (permalink / raw) To: 'Michael S. Tsirkin', Bjorn Helgaas Cc: linux-pci, Yinghai Lu, Jesse Barnes, Sagi Grimberg, Ofer Hayut, Roy Shterman, Keith Busch, Wangzhou (B), linux-kernel > On Tue, Jan 29, 2019 at 05:02:26PM -0600, Bjorn Helgaas wrote: > > On Tue, Jan 29, 2019 at 05:47:32PM -0500, Michael S. Tsirkin wrote: > > > On Tue, Jan 29, 2019 at 04:43:33PM -0600, Bjorn Helgaas wrote: > > > > On Tue, Jan 22, 2019 at 01:02:54PM -0600, Bjorn Helgaas wrote: > > > > > From: Bjorn Helgaas <bhelgaas@google.com> > > > > > > > > > > pci_bridge_check_ranges() determines whether a bridge supports > > > > > the optional I/O and prefetchable memory windows and sets the > > > > > flag bits in the bridge resources. This *could* be done once > > > > > during enumeration except that the resource allocation code > > > > > completely clears the flag bits, e.g., in the > > > > > pci_assign_unassigned_bridge_resources() path. > > > > > > > > > > The problem with pci_bridge_check_ranges() in the resource > > > > > allocation path is that we may allocate resources after devices > > > > > have been claimed by drivers, and pci_bridge_check_ranges() > > > > > *changes* the window registers to determine whether they're > > > > > writable. This may break concurrent accesses to devices behind the > bridge. > > > > > > > > > > Add a new pci_read_bridge_windows() to determine whether a > > > > > bridge supports the optional windows, call it once during > > > > > enumeration, remember the results, and change > > > > > pci_bridge_check_ranges() so it doesn't touch the bridge windows but > sets the flag bits based on those remembered results. > > > > > > > > > > Link: > > > > > https://lore.kernel.org/linux-pci/1506151482-113560-1-git-send-e > > > > > mail-wangzhou1@hisilicon.com > > > > > Link: > > > > > https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg02082.h > > > > > tml > > > > > Reported-by: xuyandong <xuyandong2@huawei.com> > > > > > Tested-by: xuyandong <xuyandong2@huawei.com> > > > > > Cc: Sagi Grimberg <sagi@grimberg.me> > > > > > Cc: Ofer Hayut <ofer@lightbitslabs.com> > > > > > Cc: Roy Shterman <roys@lightbitslabs.com> > > > > > Cc: Keith Busch <keith.busch@intel.com> > > > > > Cc: Zhou Wang <wangzhou1@hisilicon.com> > > > > > > > > Applied to pci/enumeration for v5.1. > > > > > > > > This is fairly simple in concept, but doesn't meet the letter of > > > > the restrictions in Documentation/process/stable-kernel-rules.rst, > > > > so I didn't tag it for stable. > > > > > > > > Is there a compelling argument to mark it for stable? > > > > > > Well it's needed downstream. > > > It's a bit above 100 lines with context. Is this what you mean? > > > > Yep, I should have been explicit about that. > > > > > If yes how about using my patch for stable as an alternative? > > > The rules allow for equivalent patches. > > > > Well, yes, that would be a possibility. > > > > I would rather develop an argument for marking *this* patch for > > stable. Even though it is technically too large, I think we could > > make it work if we have compelling reasons. > > > > Those would need to be fleshed out a little more than "needed > > downstream" -- something about the scenarios where this happens, what > > the serious problem is, etc. > > > > The *ideal* thing would be to have an actual bugzilla.kernel.org > > report of the problem with a way to reproduce it and dmesg > > illustrating the problem. > > xuyandong would you like to take a stub at creating the bugzilla entry? > If not let me know and I'll do it. > Thanks! > Thanks, but I do not want to create the bugzilla entry. > > > > > > --- > > > > > drivers/pci/probe.c | 52 > +++++++++++++++++++++++++++++++++++++++++++++++ > > > > > drivers/pci/setup-bus.c | 45 ++++------------------------------------- > > > > > include/linux/pci.h | 3 +++ > > > > > 3 files changed, 59 insertions(+), 41 deletions(-) > > > > > > > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index > > > > > 257b9f6f2ebb..2ef8b954c65a 100644 > > > > > --- a/drivers/pci/probe.c > > > > > +++ b/drivers/pci/probe.c > > > > > @@ -348,6 +348,57 @@ static void pci_read_bases(struct pci_dev *dev, > unsigned int howmany, int rom) > > > > > } > > > > > } > > > > > > > > > > +static void pci_read_bridge_windows(struct pci_dev *bridge) { > > > > > + u16 io; > > > > > + u32 pmem, tmp; > > > > > + > > > > > + pci_read_config_word(bridge, PCI_IO_BASE, &io); > > > > > + if (!io) { > > > > > + pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0); > > > > > + pci_read_config_word(bridge, PCI_IO_BASE, &io); > > > > > + pci_write_config_word(bridge, PCI_IO_BASE, 0x0); > > > > > + } > > > > > + if (io) > > > > > + bridge->io_window = 1; > > > > > + > > > > > + /* > > > > > + * DECchip 21050 pass 2 errata: the bridge may miss an > address > > > > > + * disconnect boundary by one PCI data phase. Workaround: > do not > > > > > + * use prefetching on this device. > > > > > + */ > > > > > + if (bridge->vendor == PCI_VENDOR_ID_DEC && bridge->device > == 0x0001) > > > > > + return; > > > > > + > > > > > + pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, > &pmem); > > > > > + if (!pmem) { > > > > > + pci_write_config_dword(bridge, > PCI_PREF_MEMORY_BASE, > > > > > + 0xffe0fff0); > > > > > + pci_read_config_dword(bridge, > PCI_PREF_MEMORY_BASE, &pmem); > > > > > + pci_write_config_dword(bridge, > PCI_PREF_MEMORY_BASE, 0x0); > > > > > + } > > > > > + if (!pmem) > > > > > + return; > > > > > + > > > > > + bridge->pref_window = 1; > > > > > + > > > > > + if ((pmem & PCI_PREF_RANGE_TYPE_MASK) == > > > > > +PCI_PREF_RANGE_TYPE_64) { > > > > > + > > > > > + /* > > > > > + * Bridge claims to have a 64-bit prefetchable memory > > > > > + * window; verify that the upper bits are actually > > > > > + * writable. > > > > > + */ > > > > > + pci_read_config_dword(bridge, > PCI_PREF_BASE_UPPER32, &pmem); > > > > > + pci_write_config_dword(bridge, > PCI_PREF_BASE_UPPER32, > > > > > + 0xffffffff); > > > > > + pci_read_config_dword(bridge, > PCI_PREF_BASE_UPPER32, &tmp); > > > > > + pci_write_config_dword(bridge, > PCI_PREF_BASE_UPPER32, pmem); > > > > > + if (tmp) > > > > > + bridge->pref_64_window = 1; > > > > > + } > > > > > +} > > > > > + > > > > > static void pci_read_bridge_io(struct pci_bus *child) { > > > > > struct pci_dev *dev = child->self; @@ -1739,6 +1790,7 @@ int > > > > > pci_setup_device(struct pci_dev *dev) > > > > > pci_read_irq(dev); > > > > > dev->transparent = ((dev->class & 0xff) == 1); > > > > > pci_read_bases(dev, 2, PCI_ROM_ADDRESS1); > > > > > + pci_read_bridge_windows(dev); > > > > > set_pcie_hotplug_bridge(dev); > > > > > pos = pci_find_capability(dev, PCI_CAP_ID_SSVID); > > > > > if (pos) { > > > > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > > > > > index ed960436df5e..1941bb0a6c13 100644 > > > > > --- a/drivers/pci/setup-bus.c > > > > > +++ b/drivers/pci/setup-bus.c > > > > > @@ -735,58 +735,21 @@ int pci_claim_bridge_resource(struct pci_dev > *bridge, int i) > > > > > base/limit registers must be read-only and read as 0. */ > > > > > static void pci_bridge_check_ranges(struct pci_bus *bus) { > > > > > - u16 io; > > > > > - u32 pmem; > > > > > struct pci_dev *bridge = bus->self; > > > > > - struct resource *b_res; > > > > > + struct resource *b_res = > > > > > +&bridge->resource[PCI_BRIDGE_RESOURCES]; > > > > > > > > > > - b_res = &bridge->resource[PCI_BRIDGE_RESOURCES]; > > > > > b_res[1].flags |= IORESOURCE_MEM; > > > > > > > > > > - pci_read_config_word(bridge, PCI_IO_BASE, &io); > > > > > - if (!io) { > > > > > - pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0); > > > > > - pci_read_config_word(bridge, PCI_IO_BASE, &io); > > > > > - pci_write_config_word(bridge, PCI_IO_BASE, 0x0); > > > > > - } > > > > > - if (io) > > > > > + if (bridge->io_window) > > > > > b_res[0].flags |= IORESOURCE_IO; > > > > > > > > > > - /* DECchip 21050 pass 2 errata: the bridge may miss an > address > > > > > - disconnect boundary by one PCI data phase. > > > > > - Workaround: do not use prefetching on this device. */ > > > > > - if (bridge->vendor == PCI_VENDOR_ID_DEC && bridge->device > == 0x0001) > > > > > - return; > > > > > - > > > > > - pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, > &pmem); > > > > > - if (!pmem) { > > > > > - pci_write_config_dword(bridge, > PCI_PREF_MEMORY_BASE, > > > > > - 0xffe0fff0); > > > > > - pci_read_config_dword(bridge, > PCI_PREF_MEMORY_BASE, &pmem); > > > > > - pci_write_config_dword(bridge, > PCI_PREF_MEMORY_BASE, 0x0); > > > > > - } > > > > > - if (pmem) { > > > > > + if (bridge->pref_window) { > > > > > b_res[2].flags |= IORESOURCE_MEM | > IORESOURCE_PREFETCH; > > > > > - if ((pmem & PCI_PREF_RANGE_TYPE_MASK) == > > > > > - PCI_PREF_RANGE_TYPE_64) { > > > > > + if (bridge->pref_64_window) { > > > > > b_res[2].flags |= IORESOURCE_MEM_64; > > > > > b_res[2].flags |= PCI_PREF_RANGE_TYPE_64; > > > > > } > > > > > } > > > > > - > > > > > - /* double check if bridge does support 64 bit pref */ > > > > > - if (b_res[2].flags & IORESOURCE_MEM_64) { > > > > > - u32 mem_base_hi, tmp; > > > > > - pci_read_config_dword(bridge, > PCI_PREF_BASE_UPPER32, > > > > > - &mem_base_hi); > > > > > - pci_write_config_dword(bridge, > PCI_PREF_BASE_UPPER32, > > > > > - 0xffffffff); > > > > > - pci_read_config_dword(bridge, > PCI_PREF_BASE_UPPER32, &tmp); > > > > > - if (!tmp) > > > > > - b_res[2].flags &= ~IORESOURCE_MEM_64; > > > > > - pci_write_config_dword(bridge, > PCI_PREF_BASE_UPPER32, > > > > > - mem_base_hi); > > > > > - } > > > > > } > > > > > > > > > > /* Helper function for sizing routines: find first available > > > > > diff --git a/include/linux/pci.h b/include/linux/pci.h index > > > > > 65f1d8c2f082..40b327b814aa 100644 > > > > > --- a/include/linux/pci.h > > > > > +++ b/include/linux/pci.h > > > > > @@ -373,6 +373,9 @@ struct pci_dev { > > > > > bool match_driver; /* Skip attaching > driver */ > > > > > > > > > > unsigned int transparent:1; /* Subtractive decode > bridge */ > > > > > + unsigned int io_window:1; /* Bridge has I/O > window */ > > > > > + unsigned int pref_window:1; /* Bridge has pref > mem window */ > > > > > + unsigned int pref_64_window:1; /* Pref mem window > is 64-bit */ > > > > > unsigned int multifunction:1; /* Multi-function device */ > > > > > > > > > > unsigned int is_busmaster:1; /* Is busmaster */ > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: Probe bridge window attributes once at enumeration-time 2019-02-11 1:57 ` Xuyandong (Yandong Xu, Euler5) @ 2019-02-11 3:48 ` Michael S. Tsirkin 0 siblings, 0 replies; 7+ messages in thread From: Michael S. Tsirkin @ 2019-02-11 3:48 UTC (permalink / raw) To: Xuyandong (Yandong Xu, Euler5) Cc: Bjorn Helgaas, linux-pci, Yinghai Lu, Jesse Barnes, Sagi Grimberg, Ofer Hayut, Roy Shterman, Keith Busch, Wangzhou (B), linux-kernel On Mon, Feb 11, 2019 at 01:57:20AM +0000, Xuyandong (Yandong Xu, Euler5) wrote: > > On Tue, Jan 29, 2019 at 05:02:26PM -0600, Bjorn Helgaas wrote: > > > On Tue, Jan 29, 2019 at 05:47:32PM -0500, Michael S. Tsirkin wrote: > > > > On Tue, Jan 29, 2019 at 04:43:33PM -0600, Bjorn Helgaas wrote: > > > > > On Tue, Jan 22, 2019 at 01:02:54PM -0600, Bjorn Helgaas wrote: > > > > > > From: Bjorn Helgaas <bhelgaas@google.com> > > > > > > > > > > > > pci_bridge_check_ranges() determines whether a bridge supports > > > > > > the optional I/O and prefetchable memory windows and sets the > > > > > > flag bits in the bridge resources. This *could* be done once > > > > > > during enumeration except that the resource allocation code > > > > > > completely clears the flag bits, e.g., in the > > > > > > pci_assign_unassigned_bridge_resources() path. > > > > > > > > > > > > The problem with pci_bridge_check_ranges() in the resource > > > > > > allocation path is that we may allocate resources after devices > > > > > > have been claimed by drivers, and pci_bridge_check_ranges() > > > > > > *changes* the window registers to determine whether they're > > > > > > writable. This may break concurrent accesses to devices behind the > > bridge. > > > > > > > > > > > > Add a new pci_read_bridge_windows() to determine whether a > > > > > > bridge supports the optional windows, call it once during > > > > > > enumeration, remember the results, and change > > > > > > pci_bridge_check_ranges() so it doesn't touch the bridge windows but > > sets the flag bits based on those remembered results. > > > > > > > > > > > > Link: > > > > > > https://lore.kernel.org/linux-pci/1506151482-113560-1-git-send-e > > > > > > mail-wangzhou1@hisilicon.com > > > > > > Link: > > > > > > https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg02082.h > > > > > > tml > > > > > > Reported-by: xuyandong <xuyandong2@huawei.com> > > > > > > Tested-by: xuyandong <xuyandong2@huawei.com> > > > > > > Cc: Sagi Grimberg <sagi@grimberg.me> > > > > > > Cc: Ofer Hayut <ofer@lightbitslabs.com> > > > > > > Cc: Roy Shterman <roys@lightbitslabs.com> > > > > > > Cc: Keith Busch <keith.busch@intel.com> > > > > > > Cc: Zhou Wang <wangzhou1@hisilicon.com> > > > > > > > > > > Applied to pci/enumeration for v5.1. > > > > > > > > > > This is fairly simple in concept, but doesn't meet the letter of > > > > > the restrictions in Documentation/process/stable-kernel-rules.rst, > > > > > so I didn't tag it for stable. > > > > > > > > > > Is there a compelling argument to mark it for stable? > > > > > > > > Well it's needed downstream. > > > > It's a bit above 100 lines with context. Is this what you mean? > > > > > > Yep, I should have been explicit about that. > > > > > > > If yes how about using my patch for stable as an alternative? > > > > The rules allow for equivalent patches. > > > > > > Well, yes, that would be a possibility. > > > > > > I would rather develop an argument for marking *this* patch for > > > stable. Even though it is technically too large, I think we could > > > make it work if we have compelling reasons. > > > > > > Those would need to be fleshed out a little more than "needed > > > downstream" -- something about the scenarios where this happens, what > > > the serious problem is, etc. > > > > > > The *ideal* thing would be to have an actual bugzilla.kernel.org > > > report of the problem with a way to reproduce it and dmesg > > > illustrating the problem. > > > > xuyandong would you like to take a stub at creating the bugzilla entry? > > If not let me know and I'll do it. > > Thanks! > > > Thanks, but I do not want to create the bugzilla entry. OK that's fair, you did quite a lot already. Xuyandong thanks a lot for your contribution in reporting the problem, proposing fixes and then testing patches, it is very much appreciated! > > > > > > > > > --- > > > > > > drivers/pci/probe.c | 52 > > +++++++++++++++++++++++++++++++++++++++++++++++ > > > > > > drivers/pci/setup-bus.c | 45 ++++------------------------------------- > > > > > > include/linux/pci.h | 3 +++ > > > > > > 3 files changed, 59 insertions(+), 41 deletions(-) > > > > > > > > > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index > > > > > > 257b9f6f2ebb..2ef8b954c65a 100644 > > > > > > --- a/drivers/pci/probe.c > > > > > > +++ b/drivers/pci/probe.c > > > > > > @@ -348,6 +348,57 @@ static void pci_read_bases(struct pci_dev *dev, > > unsigned int howmany, int rom) > > > > > > } > > > > > > } > > > > > > > > > > > > +static void pci_read_bridge_windows(struct pci_dev *bridge) { > > > > > > + u16 io; > > > > > > + u32 pmem, tmp; > > > > > > + > > > > > > + pci_read_config_word(bridge, PCI_IO_BASE, &io); > > > > > > + if (!io) { > > > > > > + pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0); > > > > > > + pci_read_config_word(bridge, PCI_IO_BASE, &io); > > > > > > + pci_write_config_word(bridge, PCI_IO_BASE, 0x0); > > > > > > + } > > > > > > + if (io) > > > > > > + bridge->io_window = 1; > > > > > > + > > > > > > + /* > > > > > > + * DECchip 21050 pass 2 errata: the bridge may miss an > > address > > > > > > + * disconnect boundary by one PCI data phase. Workaround: > > do not > > > > > > + * use prefetching on this device. > > > > > > + */ > > > > > > + if (bridge->vendor == PCI_VENDOR_ID_DEC && bridge->device > > == 0x0001) > > > > > > + return; > > > > > > + > > > > > > + pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, > > &pmem); > > > > > > + if (!pmem) { > > > > > > + pci_write_config_dword(bridge, > > PCI_PREF_MEMORY_BASE, > > > > > > + 0xffe0fff0); > > > > > > + pci_read_config_dword(bridge, > > PCI_PREF_MEMORY_BASE, &pmem); > > > > > > + pci_write_config_dword(bridge, > > PCI_PREF_MEMORY_BASE, 0x0); > > > > > > + } > > > > > > + if (!pmem) > > > > > > + return; > > > > > > + > > > > > > + bridge->pref_window = 1; > > > > > > + > > > > > > + if ((pmem & PCI_PREF_RANGE_TYPE_MASK) == > > > > > > +PCI_PREF_RANGE_TYPE_64) { > > > > > > + > > > > > > + /* > > > > > > + * Bridge claims to have a 64-bit prefetchable memory > > > > > > + * window; verify that the upper bits are actually > > > > > > + * writable. > > > > > > + */ > > > > > > + pci_read_config_dword(bridge, > > PCI_PREF_BASE_UPPER32, &pmem); > > > > > > + pci_write_config_dword(bridge, > > PCI_PREF_BASE_UPPER32, > > > > > > + 0xffffffff); > > > > > > + pci_read_config_dword(bridge, > > PCI_PREF_BASE_UPPER32, &tmp); > > > > > > + pci_write_config_dword(bridge, > > PCI_PREF_BASE_UPPER32, pmem); > > > > > > + if (tmp) > > > > > > + bridge->pref_64_window = 1; > > > > > > + } > > > > > > +} > > > > > > + > > > > > > static void pci_read_bridge_io(struct pci_bus *child) { > > > > > > struct pci_dev *dev = child->self; @@ -1739,6 +1790,7 @@ int > > > > > > pci_setup_device(struct pci_dev *dev) > > > > > > pci_read_irq(dev); > > > > > > dev->transparent = ((dev->class & 0xff) == 1); > > > > > > pci_read_bases(dev, 2, PCI_ROM_ADDRESS1); > > > > > > + pci_read_bridge_windows(dev); > > > > > > set_pcie_hotplug_bridge(dev); > > > > > > pos = pci_find_capability(dev, PCI_CAP_ID_SSVID); > > > > > > if (pos) { > > > > > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > > > > > > index ed960436df5e..1941bb0a6c13 100644 > > > > > > --- a/drivers/pci/setup-bus.c > > > > > > +++ b/drivers/pci/setup-bus.c > > > > > > @@ -735,58 +735,21 @@ int pci_claim_bridge_resource(struct pci_dev > > *bridge, int i) > > > > > > base/limit registers must be read-only and read as 0. */ > > > > > > static void pci_bridge_check_ranges(struct pci_bus *bus) { > > > > > > - u16 io; > > > > > > - u32 pmem; > > > > > > struct pci_dev *bridge = bus->self; > > > > > > - struct resource *b_res; > > > > > > + struct resource *b_res = > > > > > > +&bridge->resource[PCI_BRIDGE_RESOURCES]; > > > > > > > > > > > > - b_res = &bridge->resource[PCI_BRIDGE_RESOURCES]; > > > > > > b_res[1].flags |= IORESOURCE_MEM; > > > > > > > > > > > > - pci_read_config_word(bridge, PCI_IO_BASE, &io); > > > > > > - if (!io) { > > > > > > - pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0); > > > > > > - pci_read_config_word(bridge, PCI_IO_BASE, &io); > > > > > > - pci_write_config_word(bridge, PCI_IO_BASE, 0x0); > > > > > > - } > > > > > > - if (io) > > > > > > + if (bridge->io_window) > > > > > > b_res[0].flags |= IORESOURCE_IO; > > > > > > > > > > > > - /* DECchip 21050 pass 2 errata: the bridge may miss an > > address > > > > > > - disconnect boundary by one PCI data phase. > > > > > > - Workaround: do not use prefetching on this device. */ > > > > > > - if (bridge->vendor == PCI_VENDOR_ID_DEC && bridge->device > > == 0x0001) > > > > > > - return; > > > > > > - > > > > > > - pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, > > &pmem); > > > > > > - if (!pmem) { > > > > > > - pci_write_config_dword(bridge, > > PCI_PREF_MEMORY_BASE, > > > > > > - 0xffe0fff0); > > > > > > - pci_read_config_dword(bridge, > > PCI_PREF_MEMORY_BASE, &pmem); > > > > > > - pci_write_config_dword(bridge, > > PCI_PREF_MEMORY_BASE, 0x0); > > > > > > - } > > > > > > - if (pmem) { > > > > > > + if (bridge->pref_window) { > > > > > > b_res[2].flags |= IORESOURCE_MEM | > > IORESOURCE_PREFETCH; > > > > > > - if ((pmem & PCI_PREF_RANGE_TYPE_MASK) == > > > > > > - PCI_PREF_RANGE_TYPE_64) { > > > > > > + if (bridge->pref_64_window) { > > > > > > b_res[2].flags |= IORESOURCE_MEM_64; > > > > > > b_res[2].flags |= PCI_PREF_RANGE_TYPE_64; > > > > > > } > > > > > > } > > > > > > - > > > > > > - /* double check if bridge does support 64 bit pref */ > > > > > > - if (b_res[2].flags & IORESOURCE_MEM_64) { > > > > > > - u32 mem_base_hi, tmp; > > > > > > - pci_read_config_dword(bridge, > > PCI_PREF_BASE_UPPER32, > > > > > > - &mem_base_hi); > > > > > > - pci_write_config_dword(bridge, > > PCI_PREF_BASE_UPPER32, > > > > > > - 0xffffffff); > > > > > > - pci_read_config_dword(bridge, > > PCI_PREF_BASE_UPPER32, &tmp); > > > > > > - if (!tmp) > > > > > > - b_res[2].flags &= ~IORESOURCE_MEM_64; > > > > > > - pci_write_config_dword(bridge, > > PCI_PREF_BASE_UPPER32, > > > > > > - mem_base_hi); > > > > > > - } > > > > > > } > > > > > > > > > > > > /* Helper function for sizing routines: find first available > > > > > > diff --git a/include/linux/pci.h b/include/linux/pci.h index > > > > > > 65f1d8c2f082..40b327b814aa 100644 > > > > > > --- a/include/linux/pci.h > > > > > > +++ b/include/linux/pci.h > > > > > > @@ -373,6 +373,9 @@ struct pci_dev { > > > > > > bool match_driver; /* Skip attaching > > driver */ > > > > > > > > > > > > unsigned int transparent:1; /* Subtractive decode > > bridge */ > > > > > > + unsigned int io_window:1; /* Bridge has I/O > > window */ > > > > > > + unsigned int pref_window:1; /* Bridge has pref > > mem window */ > > > > > > + unsigned int pref_64_window:1; /* Pref mem window > > is 64-bit */ > > > > > > unsigned int multifunction:1; /* Multi-function device */ > > > > > > > > > > > > unsigned int is_busmaster:1; /* Is busmaster */ > > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-02-11 3:48 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-22 19:02 [PATCH] PCI: Probe bridge window attributes once at enumeration-time Bjorn Helgaas 2019-01-29 22:43 ` Bjorn Helgaas 2019-01-29 22:47 ` Michael S. Tsirkin 2019-01-29 23:02 ` Bjorn Helgaas 2019-02-05 18:24 ` Michael S. Tsirkin 2019-02-11 1:57 ` Xuyandong (Yandong Xu, Euler5) 2019-02-11 3:48 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).