| Message ID | 20161212183020.GA30274@obsidianresearch.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Dec 12, 2016 at 11:30:20AM -0700, Jason Gunthorpe wrote: > The PCI core will write to the bridge window config multiple times > while they are enabled. This can lead to mbus failures like: > > mvebu_mbus: cannot add window '4:e8', conflicts with another window > mvebu-pcie mbus:pex@e0000000: Could not create MBus window at [mem 0xe0000000-0xe00fffff]: -22 > > For me this is happening during a hotplug cycle. The PCI core is > not changing the values, just writing them twice while active. > > The patch addresses the general case of any change to an active window, > but not atomically. The code is slightly refactored so io and mem > can share more of the window logic. Looks good to me, but I'm waiting for an ack from Thomas or Jason (listed as maintainers and already cc'd). > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > --- > drivers/pci/host/pci-mvebu.c | 101 +++++++++++++++++++++++++------------------ > 1 file changed, 60 insertions(+), 41 deletions(-) > > diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c > index 307f81d6b479af..af724731b22f53 100644 > --- a/drivers/pci/host/pci-mvebu.c > +++ b/drivers/pci/host/pci-mvebu.c > @@ -133,6 +133,12 @@ struct mvebu_pcie { > int nports; > }; > > +struct mvebu_pcie_window { > + phys_addr_t base; > + phys_addr_t remap; > + size_t size; > +}; > + > /* Structure representing one PCIe interface */ > struct mvebu_pcie_port { > char *name; > @@ -150,10 +156,8 @@ struct mvebu_pcie_port { > struct mvebu_sw_pci_bridge bridge; > struct device_node *dn; > struct mvebu_pcie *pcie; > - phys_addr_t memwin_base; > - size_t memwin_size; > - phys_addr_t iowin_base; > - size_t iowin_size; > + struct mvebu_pcie_window memwin; > + struct mvebu_pcie_window iowin; > u32 saved_pcie_stat; > }; > > @@ -379,23 +383,45 @@ static void mvebu_pcie_add_windows(struct mvebu_pcie_port *port, > } > } > > +static void mvebu_pcie_set_window(struct mvebu_pcie_port *port, > + unsigned int target, unsigned int attribute, > + const struct mvebu_pcie_window *desired, > + struct mvebu_pcie_window *cur) > +{ > + if (desired->base == cur->base && desired->remap == cur->remap && > + desired->size == cur->size) > + return; > + > + if (cur->size != 0) { > + mvebu_pcie_del_windows(port, cur->base, cur->size); > + cur->size = 0; > + cur->base = 0; > + > + /* > + * If something tries to change the window while it is enabled > + * the change will not be done atomically. That would be > + * difficult to do in the general case. > + */ > + } > + > + if (desired->size == 0) > + return; > + > + mvebu_pcie_add_windows(port, target, attribute, desired->base, > + desired->size, desired->remap); > + *cur = *desired; > +} > + > static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port) > { > - phys_addr_t iobase; > + struct mvebu_pcie_window desired = {}; > > /* Are the new iobase/iolimit values invalid? */ > if (port->bridge.iolimit < port->bridge.iobase || > port->bridge.iolimitupper < port->bridge.iobaseupper || > !(port->bridge.command & PCI_COMMAND_IO)) { > - > - /* If a window was configured, remove it */ > - if (port->iowin_base) { > - mvebu_pcie_del_windows(port, port->iowin_base, > - port->iowin_size); > - port->iowin_base = 0; > - port->iowin_size = 0; > - } > - > + mvebu_pcie_set_window(port, port->io_target, port->io_attr, > + &desired, &port->iowin); > return; > } > > @@ -412,32 +438,27 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port) > * specifications. iobase is the bus address, port->iowin_base > * is the CPU address. > */ > - iobase = ((port->bridge.iobase & 0xF0) << 8) | > - (port->bridge.iobaseupper << 16); > - port->iowin_base = port->pcie->io.start + iobase; > - port->iowin_size = ((0xFFF | ((port->bridge.iolimit & 0xF0) << 8) | > - (port->bridge.iolimitupper << 16)) - > - iobase) + 1; > - > - mvebu_pcie_add_windows(port, port->io_target, port->io_attr, > - port->iowin_base, port->iowin_size, > - iobase); > + desired.remap = ((port->bridge.iobase & 0xF0) << 8) | > + (port->bridge.iobaseupper << 16); > + desired.base = port->pcie->io.start + desired.remap; > + desired.size = ((0xFFF | ((port->bridge.iolimit & 0xF0) << 8) | > + (port->bridge.iolimitupper << 16)) - > + desired.remap) + > + 1; > + > + mvebu_pcie_set_window(port, port->io_target, port->io_attr, &desired, > + &port->iowin); > } > > static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port) > { > + struct mvebu_pcie_window desired = {.remap = MVEBU_MBUS_NO_REMAP}; > + > /* Are the new membase/memlimit values invalid? */ > if (port->bridge.memlimit < port->bridge.membase || > !(port->bridge.command & PCI_COMMAND_MEMORY)) { > - > - /* If a window was configured, remove it */ > - if (port->memwin_base) { > - mvebu_pcie_del_windows(port, port->memwin_base, > - port->memwin_size); > - port->memwin_base = 0; > - port->memwin_size = 0; > - } > - > + mvebu_pcie_set_window(port, port->mem_target, port->mem_attr, > + &desired, &port->memwin); > return; > } > > @@ -447,14 +468,12 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port) > * window to setup, according to the PCI-to-PCI bridge > * specifications. > */ > - port->memwin_base = ((port->bridge.membase & 0xFFF0) << 16); > - port->memwin_size = > - (((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) - > - port->memwin_base + 1; > - > - mvebu_pcie_add_windows(port, port->mem_target, port->mem_attr, > - port->memwin_base, port->memwin_size, > - MVEBU_MBUS_NO_REMAP); > + desired.base = ((port->bridge.membase & 0xFFF0) << 16); > + desired.size = (((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) - > + desired.base + 1; > + > + mvebu_pcie_set_window(port, port->mem_target, port->mem_attr, &desired, > + &port->memwin); > } > > /* > -- > 2.7.4 > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 11, 2017 at 12:30:55PM -0600, Bjorn Helgaas wrote: > On Mon, Dec 12, 2016 at 11:30:20AM -0700, Jason Gunthorpe wrote: > > The PCI core will write to the bridge window config multiple times > > while they are enabled. This can lead to mbus failures like: > > > > mvebu_mbus: cannot add window '4:e8', conflicts with another window > > mvebu-pcie mbus:pex@e0000000: Could not create MBus window at [mem 0xe0000000-0xe00fffff]: -22 > > > > For me this is happening during a hotplug cycle. The PCI core is > > not changing the values, just writing them twice while active. > > > > The patch addresses the general case of any change to an active window, > > but not atomically. The code is slightly refactored so io and mem > > can share more of the window logic. > > Looks good to me, but I'm waiting for an ack from Thomas or Jason (listed > as maintainers and already cc'd). Ping, Thomas, Jason C? > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > --- > > drivers/pci/host/pci-mvebu.c | 101 +++++++++++++++++++++++++------------------ > > 1 file changed, 60 insertions(+), 41 deletions(-) > > > > diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c > > index 307f81d6b479af..af724731b22f53 100644 > > --- a/drivers/pci/host/pci-mvebu.c > > +++ b/drivers/pci/host/pci-mvebu.c > > @@ -133,6 +133,12 @@ struct mvebu_pcie { > > int nports; > > }; > > > > +struct mvebu_pcie_window { > > + phys_addr_t base; > > + phys_addr_t remap; > > + size_t size; > > +}; > > + > > /* Structure representing one PCIe interface */ > > struct mvebu_pcie_port { > > char *name; > > @@ -150,10 +156,8 @@ struct mvebu_pcie_port { > > struct mvebu_sw_pci_bridge bridge; > > struct device_node *dn; > > struct mvebu_pcie *pcie; > > - phys_addr_t memwin_base; > > - size_t memwin_size; > > - phys_addr_t iowin_base; > > - size_t iowin_size; > > + struct mvebu_pcie_window memwin; > > + struct mvebu_pcie_window iowin; > > u32 saved_pcie_stat; > > }; > > > > @@ -379,23 +383,45 @@ static void mvebu_pcie_add_windows(struct mvebu_pcie_port *port, > > } > > } > > > > +static void mvebu_pcie_set_window(struct mvebu_pcie_port *port, > > + unsigned int target, unsigned int attribute, > > + const struct mvebu_pcie_window *desired, > > + struct mvebu_pcie_window *cur) > > +{ > > + if (desired->base == cur->base && desired->remap == cur->remap && > > + desired->size == cur->size) > > + return; > > + > > + if (cur->size != 0) { > > + mvebu_pcie_del_windows(port, cur->base, cur->size); > > + cur->size = 0; > > + cur->base = 0; > > + > > + /* > > + * If something tries to change the window while it is enabled > > + * the change will not be done atomically. That would be > > + * difficult to do in the general case. > > + */ > > + } > > + > > + if (desired->size == 0) > > + return; > > + > > + mvebu_pcie_add_windows(port, target, attribute, desired->base, > > + desired->size, desired->remap); > > + *cur = *desired; > > +} > > + > > static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port) > > { > > - phys_addr_t iobase; > > + struct mvebu_pcie_window desired = {}; > > > > /* Are the new iobase/iolimit values invalid? */ > > if (port->bridge.iolimit < port->bridge.iobase || > > port->bridge.iolimitupper < port->bridge.iobaseupper || > > !(port->bridge.command & PCI_COMMAND_IO)) { > > - > > - /* If a window was configured, remove it */ > > - if (port->iowin_base) { > > - mvebu_pcie_del_windows(port, port->iowin_base, > > - port->iowin_size); > > - port->iowin_base = 0; > > - port->iowin_size = 0; > > - } > > - > > + mvebu_pcie_set_window(port, port->io_target, port->io_attr, > > + &desired, &port->iowin); > > return; > > } > > > > @@ -412,32 +438,27 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port) > > * specifications. iobase is the bus address, port->iowin_base > > * is the CPU address. > > */ > > - iobase = ((port->bridge.iobase & 0xF0) << 8) | > > - (port->bridge.iobaseupper << 16); > > - port->iowin_base = port->pcie->io.start + iobase; > > - port->iowin_size = ((0xFFF | ((port->bridge.iolimit & 0xF0) << 8) | > > - (port->bridge.iolimitupper << 16)) - > > - iobase) + 1; > > - > > - mvebu_pcie_add_windows(port, port->io_target, port->io_attr, > > - port->iowin_base, port->iowin_size, > > - iobase); > > + desired.remap = ((port->bridge.iobase & 0xF0) << 8) | > > + (port->bridge.iobaseupper << 16); > > + desired.base = port->pcie->io.start + desired.remap; > > + desired.size = ((0xFFF | ((port->bridge.iolimit & 0xF0) << 8) | > > + (port->bridge.iolimitupper << 16)) - > > + desired.remap) + > > + 1; > > + > > + mvebu_pcie_set_window(port, port->io_target, port->io_attr, &desired, > > + &port->iowin); > > } > > > > static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port) > > { > > + struct mvebu_pcie_window desired = {.remap = MVEBU_MBUS_NO_REMAP}; > > + > > /* Are the new membase/memlimit values invalid? */ > > if (port->bridge.memlimit < port->bridge.membase || > > !(port->bridge.command & PCI_COMMAND_MEMORY)) { > > - > > - /* If a window was configured, remove it */ > > - if (port->memwin_base) { > > - mvebu_pcie_del_windows(port, port->memwin_base, > > - port->memwin_size); > > - port->memwin_base = 0; > > - port->memwin_size = 0; > > - } > > - > > + mvebu_pcie_set_window(port, port->mem_target, port->mem_attr, > > + &desired, &port->memwin); > > return; > > } > > > > @@ -447,14 +468,12 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port) > > * window to setup, according to the PCI-to-PCI bridge > > * specifications. > > */ > > - port->memwin_base = ((port->bridge.membase & 0xFFF0) << 16); > > - port->memwin_size = > > - (((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) - > > - port->memwin_base + 1; > > - > > - mvebu_pcie_add_windows(port, port->mem_target, port->mem_attr, > > - port->memwin_base, port->memwin_size, > > - MVEBU_MBUS_NO_REMAP); > > + desired.base = ((port->bridge.membase & 0xFFF0) << 16); > > + desired.size = (((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) - > > + desired.base + 1; > > + > > + mvebu_pcie_set_window(port, port->mem_target, port->mem_attr, &desired, > > + &port->memwin); > > } > > > > /* > > -- > > 2.7.4 > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Bjorn, On Sat, Jan 28, 2017 at 03:17:28PM -0600, Bjorn Helgaas wrote: > On Wed, Jan 11, 2017 at 12:30:55PM -0600, Bjorn Helgaas wrote: > > On Mon, Dec 12, 2016 at 11:30:20AM -0700, Jason Gunthorpe wrote: > > > The PCI core will write to the bridge window config multiple times > > > while they are enabled. This can lead to mbus failures like: > > > > > > mvebu_mbus: cannot add window '4:e8', conflicts with another window > > > mvebu-pcie mbus:pex@e0000000: Could not create MBus window at [mem 0xe0000000-0xe00fffff]: -22 > > > > > > For me this is happening during a hotplug cycle. The PCI core is > > > not changing the values, just writing them twice while active. > > > > > > The patch addresses the general case of any change to an active window, > > > but not atomically. The code is slightly refactored so io and mem > > > can share more of the window logic. > > > > Looks good to me, but I'm waiting for an ack from Thomas or Jason (listed > > as maintainers and already cc'd). > > Ping, Thomas, Jason C? > > > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > > --- > > > drivers/pci/host/pci-mvebu.c | 101 +++++++++++++++++++++++++------------------ > > > 1 file changed, 60 insertions(+), 41 deletions(-) Sorry, been on travel and I think Thomas is on holiday. Acked-by: Jason Cooper <jason@lakedaemon.net> thx, Jason.
On Mon, Dec 12, 2016 at 11:30:20AM -0700, Jason Gunthorpe wrote: > The PCI core will write to the bridge window config multiple times > while they are enabled. This can lead to mbus failures like: > > mvebu_mbus: cannot add window '4:e8', conflicts with another window > mvebu-pcie mbus:pex@e0000000: Could not create MBus window at [mem 0xe0000000-0xe00fffff]: -22 > > For me this is happening during a hotplug cycle. The PCI core is > not changing the values, just writing them twice while active. > > The patch addresses the general case of any change to an active window, > but not atomically. The code is slightly refactored so io and mem > can share more of the window logic. > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> Applied with Jason C's ack to pci/host-mvebu for v4.11, thanks! > --- > drivers/pci/host/pci-mvebu.c | 101 +++++++++++++++++++++++++------------------ > 1 file changed, 60 insertions(+), 41 deletions(-) > > diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c > index 307f81d6b479af..af724731b22f53 100644 > --- a/drivers/pci/host/pci-mvebu.c > +++ b/drivers/pci/host/pci-mvebu.c > @@ -133,6 +133,12 @@ struct mvebu_pcie { > int nports; > }; > > +struct mvebu_pcie_window { > + phys_addr_t base; > + phys_addr_t remap; > + size_t size; > +}; > + > /* Structure representing one PCIe interface */ > struct mvebu_pcie_port { > char *name; > @@ -150,10 +156,8 @@ struct mvebu_pcie_port { > struct mvebu_sw_pci_bridge bridge; > struct device_node *dn; > struct mvebu_pcie *pcie; > - phys_addr_t memwin_base; > - size_t memwin_size; > - phys_addr_t iowin_base; > - size_t iowin_size; > + struct mvebu_pcie_window memwin; > + struct mvebu_pcie_window iowin; > u32 saved_pcie_stat; > }; > > @@ -379,23 +383,45 @@ static void mvebu_pcie_add_windows(struct mvebu_pcie_port *port, > } > } > > +static void mvebu_pcie_set_window(struct mvebu_pcie_port *port, > + unsigned int target, unsigned int attribute, > + const struct mvebu_pcie_window *desired, > + struct mvebu_pcie_window *cur) > +{ > + if (desired->base == cur->base && desired->remap == cur->remap && > + desired->size == cur->size) > + return; > + > + if (cur->size != 0) { > + mvebu_pcie_del_windows(port, cur->base, cur->size); > + cur->size = 0; > + cur->base = 0; > + > + /* > + * If something tries to change the window while it is enabled > + * the change will not be done atomically. That would be > + * difficult to do in the general case. > + */ > + } > + > + if (desired->size == 0) > + return; > + > + mvebu_pcie_add_windows(port, target, attribute, desired->base, > + desired->size, desired->remap); > + *cur = *desired; > +} > + > static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port) > { > - phys_addr_t iobase; > + struct mvebu_pcie_window desired = {}; > > /* Are the new iobase/iolimit values invalid? */ > if (port->bridge.iolimit < port->bridge.iobase || > port->bridge.iolimitupper < port->bridge.iobaseupper || > !(port->bridge.command & PCI_COMMAND_IO)) { > - > - /* If a window was configured, remove it */ > - if (port->iowin_base) { > - mvebu_pcie_del_windows(port, port->iowin_base, > - port->iowin_size); > - port->iowin_base = 0; > - port->iowin_size = 0; > - } > - > + mvebu_pcie_set_window(port, port->io_target, port->io_attr, > + &desired, &port->iowin); > return; > } > > @@ -412,32 +438,27 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port) > * specifications. iobase is the bus address, port->iowin_base > * is the CPU address. > */ > - iobase = ((port->bridge.iobase & 0xF0) << 8) | > - (port->bridge.iobaseupper << 16); > - port->iowin_base = port->pcie->io.start + iobase; > - port->iowin_size = ((0xFFF | ((port->bridge.iolimit & 0xF0) << 8) | > - (port->bridge.iolimitupper << 16)) - > - iobase) + 1; > - > - mvebu_pcie_add_windows(port, port->io_target, port->io_attr, > - port->iowin_base, port->iowin_size, > - iobase); > + desired.remap = ((port->bridge.iobase & 0xF0) << 8) | > + (port->bridge.iobaseupper << 16); > + desired.base = port->pcie->io.start + desired.remap; > + desired.size = ((0xFFF | ((port->bridge.iolimit & 0xF0) << 8) | > + (port->bridge.iolimitupper << 16)) - > + desired.remap) + > + 1; > + > + mvebu_pcie_set_window(port, port->io_target, port->io_attr, &desired, > + &port->iowin); > } > > static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port) > { > + struct mvebu_pcie_window desired = {.remap = MVEBU_MBUS_NO_REMAP}; > + > /* Are the new membase/memlimit values invalid? */ > if (port->bridge.memlimit < port->bridge.membase || > !(port->bridge.command & PCI_COMMAND_MEMORY)) { > - > - /* If a window was configured, remove it */ > - if (port->memwin_base) { > - mvebu_pcie_del_windows(port, port->memwin_base, > - port->memwin_size); > - port->memwin_base = 0; > - port->memwin_size = 0; > - } > - > + mvebu_pcie_set_window(port, port->mem_target, port->mem_attr, > + &desired, &port->memwin); > return; > } > > @@ -447,14 +468,12 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port) > * window to setup, according to the PCI-to-PCI bridge > * specifications. > */ > - port->memwin_base = ((port->bridge.membase & 0xFFF0) << 16); > - port->memwin_size = > - (((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) - > - port->memwin_base + 1; > - > - mvebu_pcie_add_windows(port, port->mem_target, port->mem_attr, > - port->memwin_base, port->memwin_size, > - MVEBU_MBUS_NO_REMAP); > + desired.base = ((port->bridge.membase & 0xFFF0) << 16); > + desired.size = (((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) - > + desired.base + 1; > + > + mvebu_pcie_set_window(port, port->mem_target, port->mem_attr, &desired, > + &port->memwin); > } > > /* > -- > 2.7.4 > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 30, 2017 at 09:41:36AM -0600, Bjorn Helgaas wrote: > On Mon, Dec 12, 2016 at 11:30:20AM -0700, Jason Gunthorpe wrote: > > The PCI core will write to the bridge window config multiple times > > while they are enabled. This can lead to mbus failures like: > > > > mvebu_mbus: cannot add window '4:e8', conflicts with another window > > mvebu-pcie mbus:pex@e0000000: Could not create MBus window at [mem 0xe0000000-0xe00fffff]: -22 > > > > For me this is happening during a hotplug cycle. The PCI core is > > not changing the values, just writing them twice while active. > > > > The patch addresses the general case of any change to an active window, > > but not atomically. The code is slightly refactored so io and mem > > can share more of the window logic. > > > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > Applied with Jason C's ack to pci/host-mvebu for v4.11, thanks! Thank you for keeping on top of this Bjorn! Jason
diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c index 307f81d6b479af..af724731b22f53 100644 --- a/drivers/pci/host/pci-mvebu.c +++ b/drivers/pci/host/pci-mvebu.c @@ -133,6 +133,12 @@ struct mvebu_pcie { int nports; }; +struct mvebu_pcie_window { + phys_addr_t base; + phys_addr_t remap; + size_t size; +}; + /* Structure representing one PCIe interface */ struct mvebu_pcie_port { char *name; @@ -150,10 +156,8 @@ struct mvebu_pcie_port { struct mvebu_sw_pci_bridge bridge; struct device_node *dn; struct mvebu_pcie *pcie; - phys_addr_t memwin_base; - size_t memwin_size; - phys_addr_t iowin_base; - size_t iowin_size; + struct mvebu_pcie_window memwin; + struct mvebu_pcie_window iowin; u32 saved_pcie_stat; }; @@ -379,23 +383,45 @@ static void mvebu_pcie_add_windows(struct mvebu_pcie_port *port, } } +static void mvebu_pcie_set_window(struct mvebu_pcie_port *port, + unsigned int target, unsigned int attribute, + const struct mvebu_pcie_window *desired, + struct mvebu_pcie_window *cur) +{ + if (desired->base == cur->base && desired->remap == cur->remap && + desired->size == cur->size) + return; + + if (cur->size != 0) { + mvebu_pcie_del_windows(port, cur->base, cur->size); + cur->size = 0; + cur->base = 0; + + /* + * If something tries to change the window while it is enabled + * the change will not be done atomically. That would be + * difficult to do in the general case. + */ + } + + if (desired->size == 0) + return; + + mvebu_pcie_add_windows(port, target, attribute, desired->base, + desired->size, desired->remap); + *cur = *desired; +} + static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port) { - phys_addr_t iobase; + struct mvebu_pcie_window desired = {}; /* Are the new iobase/iolimit values invalid? */ if (port->bridge.iolimit < port->bridge.iobase || port->bridge.iolimitupper < port->bridge.iobaseupper || !(port->bridge.command & PCI_COMMAND_IO)) { - - /* If a window was configured, remove it */ - if (port->iowin_base) { - mvebu_pcie_del_windows(port, port->iowin_base, - port->iowin_size); - port->iowin_base = 0; - port->iowin_size = 0; - } - + mvebu_pcie_set_window(port, port->io_target, port->io_attr, + &desired, &port->iowin); return; } @@ -412,32 +438,27 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port) * specifications. iobase is the bus address, port->iowin_base * is the CPU address. */ - iobase = ((port->bridge.iobase & 0xF0) << 8) | - (port->bridge.iobaseupper << 16); - port->iowin_base = port->pcie->io.start + iobase; - port->iowin_size = ((0xFFF | ((port->bridge.iolimit & 0xF0) << 8) | - (port->bridge.iolimitupper << 16)) - - iobase) + 1; - - mvebu_pcie_add_windows(port, port->io_target, port->io_attr, - port->iowin_base, port->iowin_size, - iobase); + desired.remap = ((port->bridge.iobase & 0xF0) << 8) | + (port->bridge.iobaseupper << 16); + desired.base = port->pcie->io.start + desired.remap; + desired.size = ((0xFFF | ((port->bridge.iolimit & 0xF0) << 8) | + (port->bridge.iolimitupper << 16)) - + desired.remap) + + 1; + + mvebu_pcie_set_window(port, port->io_target, port->io_attr, &desired, + &port->iowin); } static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port) { + struct mvebu_pcie_window desired = {.remap = MVEBU_MBUS_NO_REMAP}; + /* Are the new membase/memlimit values invalid? */ if (port->bridge.memlimit < port->bridge.membase || !(port->bridge.command & PCI_COMMAND_MEMORY)) { - - /* If a window was configured, remove it */ - if (port->memwin_base) { - mvebu_pcie_del_windows(port, port->memwin_base, - port->memwin_size); - port->memwin_base = 0; - port->memwin_size = 0; - } - + mvebu_pcie_set_window(port, port->mem_target, port->mem_attr, + &desired, &port->memwin); return; } @@ -447,14 +468,12 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port) * window to setup, according to the PCI-to-PCI bridge * specifications. */ - port->memwin_base = ((port->bridge.membase & 0xFFF0) << 16); - port->memwin_size = - (((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) - - port->memwin_base + 1; - - mvebu_pcie_add_windows(port, port->mem_target, port->mem_attr, - port->memwin_base, port->memwin_size, - MVEBU_MBUS_NO_REMAP); + desired.base = ((port->bridge.membase & 0xFFF0) << 16); + desired.size = (((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) - + desired.base + 1; + + mvebu_pcie_set_window(port, port->mem_target, port->mem_attr, &desired, + &port->memwin); } /*
The PCI core will write to the bridge window config multiple times while they are enabled. This can lead to mbus failures like: mvebu_mbus: cannot add window '4:e8', conflicts with another window mvebu-pcie mbus:pex@e0000000: Could not create MBus window at [mem 0xe0000000-0xe00fffff]: -22 For me this is happening during a hotplug cycle. The PCI core is not changing the values, just writing them twice while active. The patch addresses the general case of any change to an active window, but not atomically. The code is slightly refactored so io and mem can share more of the window logic. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> --- drivers/pci/host/pci-mvebu.c | 101 +++++++++++++++++++++++++------------------ 1 file changed, 60 insertions(+), 41 deletions(-)