| Message ID | 1466808476-32690-16-git-send-email-benh@kernel.crashing.org |
|---|---|
| State | Superseded |
| Headers | show |
On Sat, 2016-06-25 at 08:47 +1000, Benjamin Herrenschmidt wrote: > PCIe devices cache the bus number on the first config write, make sure > we restore them when at the same time Is this a fix that needs to go into some stable builds? > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > --- > core/pci.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/core/pci.c b/core/pci.c > index 9b65a49..7ba5169 100644 > --- a/core/pci.c > +++ b/core/pci.c > @@ -1605,8 +1605,14 @@ static int __pci_restore_bridge_buses(struct phb *phb, > struct pci_device *pd, > void *data __unused) > { > - if (!pd->is_bridge) > + if (!pd->is_bridge) { > + uint32_t vdid; > + > + /* Make all devices below a bridge "re-capture" the bdfn */ > + if (pci_cfg_read32(phb, pd->bdfn, 0, &vdid) == 0) > + pci_cfg_write32(phb, pd->bdfn, 0, vdid); Do we have a lock held here to ensure there is no race? > return 0; > + } > > pci_cfg_write8(phb, pd->bdfn, PCI_CFG_PRIMARY_BUS, > pd->primary_bus);
On Wed, 2016-07-06 at 17:24 +1000, Michael Neuling wrote: > On Sat, 2016-06-25 at 08:47 +1000, Benjamin Herrenschmidt wrote: > > PCIe devices cache the bus number on the first config write, make > > sure > > we restore them when at the same time > > Is this a fix that needs to go into some stable builds? Yes probably, not a huge issue though. > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > > > --- > > core/pci.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/core/pci.c b/core/pci.c > > index 9b65a49..7ba5169 100644 > > --- a/core/pci.c > > +++ b/core/pci.c > > @@ -1605,8 +1605,14 @@ static int __pci_restore_bridge_buses(struct > > phb *phb, > > struct pci_device *pd, > > void *data __unused) > > { > > - if (!pd->is_bridge) > > + if (!pd->is_bridge) { > > + uint32_t vdid; > > + > > + /* Make all devices below a bridge "re-capture" > > the bdfn */ > > + if (pci_cfg_read32(phb, pd->bdfn, 0, &vdid) == 0) > > + pci_cfg_write32(phb, pd->bdfn, 0, vdid); > > Do we have a lock held here to ensure there is no race? No no need. The register is read only, the write does nothing other than making the device capture it's own bus ID from the packet. > > return 0; > > + } > > > > pci_cfg_write8(phb, pd->bdfn, PCI_CFG_PRIMARY_BUS, > > pd->primary_bus);
On Wed, 2016-07-06 at 17:27 +1000, Benjamin Herrenschmidt wrote: > On Wed, 2016-07-06 at 17:24 +1000, Michael Neuling wrote: > > > > On Sat, 2016-06-25 at 08:47 +1000, Benjamin Herrenschmidt wrote: > > > > > > PCIe devices cache the bus number on the first config write, make > > > sure > > > we restore them when at the same time > > Is this a fix that needs to go into some stable builds? > Yes probably, not a huge issue though. Ok, Stewart? > > > > > > > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> Since there is no race below... Acked-by: Michael Neuling <mikey@neuling.org> > > > > > > --- > > > core/pci.c | 8 +++++++- > > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > > > diff --git a/core/pci.c b/core/pci.c > > > index 9b65a49..7ba5169 100644 > > > --- a/core/pci.c > > > +++ b/core/pci.c > > > @@ -1605,8 +1605,14 @@ static int __pci_restore_bridge_buses(struct > > > phb *phb, > > > struct pci_device *pd, > > > void *data __unused) > > > { > > > - if (!pd->is_bridge) > > > + if (!pd->is_bridge) { > > > + uint32_t vdid; > > > + > > > + /* Make all devices below a bridge "re-capture" > > > the bdfn */ > > > + if (pci_cfg_read32(phb, pd->bdfn, 0, &vdid) == 0) > > > + pci_cfg_write32(phb, pd->bdfn, 0, vdid); > > Do we have a lock held here to ensure there is no race? > No no need. The register is read only, the write does nothing other > than making the device capture it's own bus ID from the packet. OK if you respin, a comment would be nice. Mikey > > > > > > > > > return 0; > > > + } > > > > > > pci_cfg_write8(phb, pd->bdfn, PCI_CFG_PRIMARY_BUS, > > > pd->primary_bus); > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
diff --git a/core/pci.c b/core/pci.c index 9b65a49..7ba5169 100644 --- a/core/pci.c +++ b/core/pci.c @@ -1605,8 +1605,14 @@ static int __pci_restore_bridge_buses(struct phb *phb, struct pci_device *pd, void *data __unused) { - if (!pd->is_bridge) + if (!pd->is_bridge) { + uint32_t vdid; + + /* Make all devices below a bridge "re-capture" the bdfn */ + if (pci_cfg_read32(phb, pd->bdfn, 0, &vdid) == 0) + pci_cfg_write32(phb, pd->bdfn, 0, vdid); return 0; + } pci_cfg_write8(phb, pd->bdfn, PCI_CFG_PRIMARY_BUS, pd->primary_bus);
PCIe devices cache the bus number on the first config write, make sure we restore them when at the same time Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- core/pci.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)