[PATCH] vfio/pci: Fix version2 RC endpoint PCIe capability size

[PATCH] vfio/pci: Fix version2 RC endpoint PCIe capability size

[Andrew Jones]

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(-)

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)
-- 
2.13.4

Re: [PATCH] vfio/pci: Fix version2 RC endpoint PCIe capability size

[Alex Williamson]

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)