| Message ID | 20170811100335.22716-1-drjones@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 11 Aug 2017 12:03:35 +0200 Andrew Jones <drjones@redhat.com> wrote: > Fix a typo which resulted in returning the same value for the length > of version 2 root complex integrated endpoints as other version 2 > devices, even though RC's don't have a link. It seems this fixes the wrong bug. The bug is in the definition of PCI_CAP_EXP_ENDPOINT_SIZEOF_V2, which does not include the v2 link information. This is why they're the same size currently. I've posted this patch which resolves that issue: http://www.spinics.net/lists/linux-pci/msg63762.html Once accepted we can incorporate the new macros rather than the hard coded values used here, but the existing code will automatically start using the correct offsets. The proposal below is wrong without the patch above, making the v2 RC endpoint too short. Thanks, Alex > Signed-off-by: Andrew Jones <drjones@redhat.com> > > --- > > The issue was only found by review, so I've only compile-tested the fix. > Actually, TBH, I don't know anything about PCI or RC config space, so I'm > just assuming link-size-v2 == link-size-v1 and that link-size-v1 is 8, > since the code was using 0xc == PCI_CAP_EXP_ENDPOINT_SIZEOF_V1 - 8 for it. > > drivers/vfio/pci/vfio_pci_config.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c > index 5628fe114347..afe29cd88f1f 100644 > --- a/drivers/vfio/pci/vfio_pci_config.c > +++ b/drivers/vfio/pci/vfio_pci_config.c > @@ -1244,15 +1244,15 @@ static int vfio_cap_len(struct vfio_pci_device *vdev, u8 cap, u8 pos) > } > > /* length based on version and type */ > - if ((pcie_caps_reg(pdev) & PCI_EXP_FLAGS_VERS) == 1) { > - if (pci_pcie_type(pdev) == PCI_EXP_TYPE_RC_END) > - return 0xc; /* "All Devices" only, no link */ > - return PCI_CAP_EXP_ENDPOINT_SIZEOF_V1; > - } else { > - if (pci_pcie_type(pdev) == PCI_EXP_TYPE_RC_END) > - return 0x2c; /* No link */ > - return PCI_CAP_EXP_ENDPOINT_SIZEOF_V2; > - } > + if ((pcie_caps_reg(pdev) & PCI_EXP_FLAGS_VERS) == 1) > + ret = PCI_CAP_EXP_ENDPOINT_SIZEOF_V1; > + else > + ret = PCI_CAP_EXP_ENDPOINT_SIZEOF_V2; > + > + if (pci_pcie_type(pdev) == PCI_EXP_TYPE_RC_END) > + ret -= 8; /* "All Devices" only, no link */ > + > + return ret; > case PCI_CAP_ID_HT: > ret = pci_read_config_byte(pdev, pos + 3, &byte); > if (ret)
diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c index 5628fe114347..afe29cd88f1f 100644 --- a/drivers/vfio/pci/vfio_pci_config.c +++ b/drivers/vfio/pci/vfio_pci_config.c @@ -1244,15 +1244,15 @@ static int vfio_cap_len(struct vfio_pci_device *vdev, u8 cap, u8 pos) } /* length based on version and type */ - if ((pcie_caps_reg(pdev) & PCI_EXP_FLAGS_VERS) == 1) { - if (pci_pcie_type(pdev) == PCI_EXP_TYPE_RC_END) - return 0xc; /* "All Devices" only, no link */ - return PCI_CAP_EXP_ENDPOINT_SIZEOF_V1; - } else { - if (pci_pcie_type(pdev) == PCI_EXP_TYPE_RC_END) - return 0x2c; /* No link */ - return PCI_CAP_EXP_ENDPOINT_SIZEOF_V2; - } + if ((pcie_caps_reg(pdev) & PCI_EXP_FLAGS_VERS) == 1) + ret = PCI_CAP_EXP_ENDPOINT_SIZEOF_V1; + else + ret = PCI_CAP_EXP_ENDPOINT_SIZEOF_V2; + + if (pci_pcie_type(pdev) == PCI_EXP_TYPE_RC_END) + ret -= 8; /* "All Devices" only, no link */ + + return ret; case PCI_CAP_ID_HT: ret = pci_read_config_byte(pdev, pos + 3, &byte); if (ret)
Fix a typo which resulted in returning the same value for the length of version 2 root complex integrated endpoints as other version 2 devices, even though RC's don't have a link. Signed-off-by: Andrew Jones <drjones@redhat.com> --- The issue was only found by review, so I've only compile-tested the fix. Actually, TBH, I don't know anything about PCI or RC config space, so I'm just assuming link-size-v2 == link-size-v1 and that link-size-v1 is 8, since the code was using 0xc == PCI_CAP_EXP_ENDPOINT_SIZEOF_V1 - 8 for it. drivers/vfio/pci/vfio_pci_config.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)