| Message ID | 20190311133122.11417-4-s.miroshnichenko@yadro.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: Allow BAR movement during hotplug | expand |
On Mon, Mar 11, 2019 at 04:31:04PM +0300, Sergey Miroshnichenko wrote: > After updating the bridge window resources, the PCI_COMMAND_IO and > PCI_COMMAND_MEMORY bits of the bridge must be addressed as well. > > Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com> > --- > drivers/pci/pci.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 895201d4c9e6..69898fe5255e 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -1622,6 +1622,14 @@ static void pci_enable_bridge(struct pci_dev *dev) > pci_enable_bridge(bridge); > > if (pci_is_enabled(dev)) { > + int i, bars = 0; > + > + for (i = PCI_BRIDGE_RESOURCES; i < DEVICE_COUNT_RESOURCE; i++) { > + if (dev->resource[i].flags & (IORESOURCE_MEM | IORESOURCE_IO)) > + bars |= (1 << i); > + } > + do_pci_enable_device(dev, bars); In what situation is this needed, exactly? This code already exists in pci_enable_device_flags(). Why isn't that enough? I guess maybe there's some case where we enable the bridge, then assign bridge windows, then enable a downstream device? Does this fix a bug with current hotplug? > if (!dev->is_busmaster) > pci_set_master(dev); > mutex_unlock(&dev->enable_mutex); > -- > 2.20.1 >
On 3/26/19 10:13 PM, Bjorn Helgaas wrote: > On Mon, Mar 11, 2019 at 04:31:04PM +0300, Sergey Miroshnichenko wrote: >> After updating the bridge window resources, the PCI_COMMAND_IO and >> PCI_COMMAND_MEMORY bits of the bridge must be addressed as well. >> >> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com> >> --- >> drivers/pci/pci.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c >> index 895201d4c9e6..69898fe5255e 100644 >> --- a/drivers/pci/pci.c >> +++ b/drivers/pci/pci.c >> @@ -1622,6 +1622,14 @@ static void pci_enable_bridge(struct pci_dev *dev) >> pci_enable_bridge(bridge); >> >> if (pci_is_enabled(dev)) { >> + int i, bars = 0; >> + >> + for (i = PCI_BRIDGE_RESOURCES; i < DEVICE_COUNT_RESOURCE; i++) { >> + if (dev->resource[i].flags & (IORESOURCE_MEM | IORESOURCE_IO)) >> + bars |= (1 << i); >> + } >> + do_pci_enable_device(dev, bars); > > In what situation is this needed, exactly? This code already exists > in pci_enable_device_flags(). Why isn't that enough? > > I guess maybe there's some case where we enable the bridge, then > assign bridge windows, then enable a downstream device? > > Does this fix a bug with current hotplug? > Sure, this change was implemented because of the issue: pci_enable_device_flags() returns early if the device is already pci_is_enabled(), so if a bridge was already enabled before the hotplug event, but without MEM and/or IO being set, these bits will not be set even if a new device wants them. I've chosen the pci_enable_bridge() for this snippet because it recursively updates all the parent bridges. Serge >> if (!dev->is_busmaster) >> pci_set_master(dev); >> mutex_unlock(&dev->enable_mutex); >> -- >> 2.20.1 >>
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 895201d4c9e6..69898fe5255e 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1622,6 +1622,14 @@ static void pci_enable_bridge(struct pci_dev *dev) pci_enable_bridge(bridge); if (pci_is_enabled(dev)) { + int i, bars = 0; + + for (i = PCI_BRIDGE_RESOURCES; i < DEVICE_COUNT_RESOURCE; i++) { + if (dev->resource[i].flags & (IORESOURCE_MEM | IORESOURCE_IO)) + bars |= (1 << i); + } + do_pci_enable_device(dev, bars); + if (!dev->is_busmaster) pci_set_master(dev); mutex_unlock(&dev->enable_mutex);
After updating the bridge window resources, the PCI_COMMAND_IO and PCI_COMMAND_MEMORY bits of the bridge must be addressed as well. Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com> --- drivers/pci/pci.c | 8 ++++++++ 1 file changed, 8 insertions(+)