| Message ID | 1465396497.2948.4.camel@kernel.crashing.org |
|---|---|
| State | Accepted |
| Headers | show |
On Thu, Jun 09, 2016 at 12:34:57AM +1000, Benjamin Herrenschmidt wrote: >On PCI Express, devices need to know their own bus number in order >to provide the correct source identification (aka RID) in upstream >packets they might send, such as error messages or DMAs. > >However while devices know (and hard wire) their own device and >function number, they know nothing about bus numbers by default, those >are decoded by bridges for routing. All they know is that if their >parent bridge sends a "type 0" configuration access, they should decode >it provided the device and function numbers match. > >The PCIe spec thus defines that when a device receive such a configuration >access and it's a write, it should "capture" the bus number in the source >field of the packet, and re-use as the originator bus number of all >subsequent outgoing requests. > >In order to ensure that a device has this bus number firmly established >before it's likely to send error packets upstream, we should thus do a >dummy configuration write to it as soon as possible after probing. > >Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >--- > >diff --git a/core/pci.c b/core/pci.c >index 9b238d0..bce65fb 100644 >--- a/core/pci.c >+++ b/core/pci.c >@@ -156,6 +156,13 @@ static struct pci_device *pci_scan_one(struct phb *phb, struct pci_device *paren > } > if (had_crs) > PCIDBG(phb, bdfn, "Probe success after CRS\n"); >+ >+ /* Perform a dummy write to the device in order for it to >+ * capture it's own bus number, so any subsequent error >+ * messages will be properly tagged >+ */ >+ pci_cfg_write32(phb, bdfn, 0, &vdid); >+ Ben, I guess it's vdid, not &vdid though the target register is read-only. Thanks, Gavin > pd = zalloc(sizeof(struct pci_device)); > if (!pd) { > PCIERR(phb, bdfn,"Failed to allocate structure pci_device !\n"); >_______________________________________________ >Skiboot mailing list >Skiboot@lists.ozlabs.org >https://lists.ozlabs.org/listinfo/skiboot
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: > On PCI Express, devices need to know their own bus number in order > to provide the correct source identification (aka RID) in upstream > packets they might send, such as error messages or DMAs. > > However while devices know (and hard wire) their own device and > function number, they know nothing about bus numbers by default, those > are decoded by bridges for routing. All they know is that if their > parent bridge sends a "type 0" configuration access, they should decode > it provided the device and function numbers match. > > The PCIe spec thus defines that when a device receive such a configuration > access and it's a write, it should "capture" the bus number in the source > field of the packet, and re-use as the originator bus number of all > subsequent outgoing requests. > > In order to ensure that a device has this bus number firmly established > before it's likely to send error packets upstream, we should thus do a > dummy configuration write to it as soon as possible after probing. > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> You need to fix your mailer, it's sending corrupted patches. But luckily, due to youthful indescretions, I know more about unified diff than it's sane to and fixed it. I took Gavin's suggestion of vdid rather than &vdid as that's at least a bit more obvious that it's not incorrect... and possibly guards against something beind weird?
Stewart Smith <stewart@linux.vnet.ibm.com> writes: > Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: >> On PCI Express, devices need to know their own bus number in order >> to provide the correct source identification (aka RID) in upstream >> packets they might send, such as error messages or DMAs. >> >> However while devices know (and hard wire) their own device and >> function number, they know nothing about bus numbers by default, those >> are decoded by bridges for routing. All they know is that if their >> parent bridge sends a "type 0" configuration access, they should decode >> it provided the device and function numbers match. >> >> The PCIe spec thus defines that when a device receive such a configuration >> access and it's a write, it should "capture" the bus number in the source >> field of the packet, and re-use as the originator bus number of all >> subsequent outgoing requests. >> >> In order to ensure that a device has this bus number firmly established >> before it's likely to send error packets upstream, we should thus do a >> dummy configuration write to it as soon as possible after probing. >> >> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > You need to fix your mailer, it's sending corrupted patches. But > luckily, due to youthful indescretions, I know more about unified diff > than it's sane to and fixed it. > > I took Gavin's suggestion of vdid rather than &vdid as that's at least a > bit more obvious that it's not incorrect... and possibly guards against > something beind weird? Oh, and merged to master as of f46c1e5. Should this also go to stable?
On Thu, 2016-06-09 at 15:43 +1000, Gavin Shan wrote: > >+ > >+ /* Perform a dummy write to the device in order for it to > >+ * capture it's own bus number, so any subsequent error > >+ * messages will be properly tagged > >+ */ > >+ pci_cfg_write32(phb, bdfn, 0, &vdid); > >+ > > Ben, I guess it's vdid, not &vdid though the target register is read- > only. Hah, oops, forgot to commt the compile fix before sending the patch. Yes, it is. I will resend tmr. Cheers, Ben.
On Thu, 2016-06-09 at 16:07 +1000, Stewart Smith wrote: > > I took Gavin's suggestion of vdid rather than &vdid as that's at least a > > bit more obvious that it's not incorrect... and possibly guards against > > something beind weird? > > Oh, and merged to master as of f46c1e5. > > Should this also go to stable? Won't hurt. The only user visible difference is going to be that if a device goes bonkers before Linux has a chance to write something to it, we'll get a better sourced EEH error. Cheers, Ben.
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: > On Thu, 2016-06-09 at 16:07 +1000, Stewart Smith wrote: >> > I took Gavin's suggestion of vdid rather than &vdid as that's at least a >> > bit more obvious that it's not incorrect... and possibly guards against >> > something beind weird? >> >> Oh, and merged to master as of f46c1e5. >> >> Should this also go to stable? > > Won't hurt. The only user visible difference is going to be that if a > device goes bonkers before Linux has a chance to write something to > it, we'll get a better sourced EEH error. and done - skiboot 5.1.x and 5.2.x
diff --git a/core/pci.c b/core/pci.c index 9b238d0..bce65fb 100644 --- a/core/pci.c +++ b/core/pci.c @@ -156,6 +156,13 @@ static struct pci_device *pci_scan_one(struct phb *phb, struct pci_device *paren } if (had_crs) PCIDBG(phb, bdfn, "Probe success after CRS\n"); + + /* Perform a dummy write to the device in order for it to + * capture it's own bus number, so any subsequent error + * messages will be properly tagged + */ + pci_cfg_write32(phb, bdfn, 0, &vdid); + pd = zalloc(sizeof(struct pci_device)); if (!pd) { PCIERR(phb, bdfn,"Failed to allocate structure pci_device !\n");
On PCI Express, devices need to know their own bus number in order to provide the correct source identification (aka RID) in upstream packets they might send, such as error messages or DMAs. However while devices know (and hard wire) their own device and function number, they know nothing about bus numbers by default, those are decoded by bridges for routing. All they know is that if their parent bridge sends a "type 0" configuration access, they should decode it provided the device and function numbers match. The PCIe spec thus defines that when a device receive such a configuration access and it's a write, it should "capture" the bus number in the source field of the packet, and re-use as the originator bus number of all subsequent outgoing requests. In order to ensure that a device has this bus number firmly established before it's likely to send error packets upstream, we should thus do a dummy configuration write to it as soon as possible after probing. Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> ---