Re: [Qemu-devel] [PATCH v2] pci : Add pba_offset PCI quirk for Chelsio T5 devices

[Casey Leedom]

  Looks good to me.

Casey

________________________________________
From: Alex Williamson address@hidden
Sent: Friday, June 26, 2015 11:01 AM
To: Gabriel Laupre
Cc: address@hidden; address@hidden; address@hidden; address@hidden; Casey 
Leedom; Michael Boksanyi; Anish Bhatt
Subject: Re: [PATCH v2] pci : Add pba_offset PCI quirk for Chelsio T5 devices

On Thu, 2015-06-25 at 17:30 -0700, Gabriel Laupre wrote:
> Fix pba_offset initialization value for Chelsio T5 Virtual Function
> device. The T5 hardware has a bug in it where it reports a Pending Interrupt
> Bit Array Offset of 0x8000 for its SR-IOV Virtual Functions instead
> of the 0x1000 that the hardware actually uses internally. As the hardware
> doesn't return the correct pba_offset value, add a quirk to instead
> return a hardcoded value of 0x1000 when a Chelsio T5 VF device is
> detected.
>
> This bug has been fixed in the Chelsio's next chip series T6 but there are
> no plans to respin the T5 ASIC for this bug. It is just documented in the
> T5 Errata and left it at that.
>
> v2 : Replace and PCI_DEVICE_ID_CHELSIO_T5_SERIES_VF macro definition with
>   the Chelsio's T5 VF devices identifier schema of 0x58xx
>
> Signed-off-by: Gabriel Laupre <address@hidden>
> ---
>  hw/vfio/pci.c            | 12 ++++++++++++
>  include/hw/pci/pci_ids.h |  2 ++
>  2 files changed, 14 insertions(+)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index e0e339a..5f35f61 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2220,6 +2220,9 @@ static int vfio_early_setup_msix(VFIOPCIDevice *vdev)
>      uint16_t ctrl;
>      uint32_t table, pba;
>      int fd = vdev->vbasedev.fd;
> +    PCIDevice *pdev = &vdev->pdev;
> +    uint16_t vendor = pci_get_word(pdev->config + PCI_VENDOR_ID);
> +    uint16_t device = pci_get_word(pdev->config + PCI_DEVICE_ID);
>
>      pos = pci_find_capability(&vdev->pdev, PCI_CAP_ID_MSIX);
>      if (!pos) {
> @@ -2252,6 +2255,15 @@ static int vfio_early_setup_msix(VFIOPCIDevice *vdev)
>      vdev->msix->pba_offset = pba & ~PCI_MSIX_FLAGS_BIRMASK;
>      vdev->msix->entries = (ctrl & PCI_MSIX_FLAGS_QSIZE) + 1;
>
> +    /* Chelsio T5 Virtual Function devices are encoded as 0x58xx for T5
> +     * adapters.  The T5 hardware returns an incorrect value of 0x8000 for 
> the
> +     * VF PBA offset.  The correct value is 0x1000, so we hard code that 
> here.
> +     */
> +    if (vendor == PCI_VENDOR_ID_CHELSIO &&
> +        (device & 0xff00) == 0x5800) {
> +        vdev->msix->pba_offset = 0x1000;
> +    }
> +
>      trace_vfio_early_setup_msix(vdev->vbasedev.name, pos,
>                                  vdev->msix->table_bar,
>                                  vdev->msix->table_offset,

I believe I suggested something like this:

--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2252,6 +2252,33 @@ static int vfio_early_setup_msix(VFIOPCIDevice *vdev)
     vdev->msix->pba_offset = pba & ~PCI_MSIX_FLAGS_BIRMASK;
     vdev->msix->entries = (ctrl & PCI_MSIX_FLAGS_QSIZE) + 1;

+    /*
+     * We could be more thorough here, but msix_init() will do further
+     * testing, look for the obviously broken configs and apply any quirks.
+     */
+    if (vdev->msix->table_offset >=
+        vdev->bars[vdev->msix->table_bar].region.size ||
+        vdev->msix->pba_offset >=
+        vdev->bars[vdev->msix->pba_bar].region.size) {
+        PCIDevice *pdev = &vdev->pdev;
+        uint16_t vendor = pci_get_word(pdev->config + PCI_VENDOR_ID);
+        uint16_t device = pci_get_word(pdev->config + PCI_DEVICE_ID);
+
+        /*
+         * Chelsio T5 Virtual Function devices are encoded as 0x58xx for T5
+         * adapters.  The T5 hardware returns an incorrect value of 0x8000
+         * for the VF PBA offset while the BAR itself is only 8K.  The
+         * correct value is 0x1000, so we hard code that here.
+         */
+        if (vendor == PCI_VENDOR_ID_CHELSIO && (device & 0xff00) == 0x5800) {
+            vdev->msix->pba_offset = 0x1000;
+        } else {
+            error_report("vfio: Hardware reports invalid configuration, "
+                         "MSIX data outside of specified BAR");
+            return -EINVAL;
+        }
+    }
+
     trace_vfio_early_setup_msix(vdev->vbasedev.name, pos,
                                 vdev->msix->table_bar,
                                 vdev->msix->table_offset,

> diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
> index 49c062b..d98e6c9 100644
> --- a/include/hw/pci/pci_ids.h
> +++ b/include/hw/pci/pci_ids.h
> @@ -114,6 +114,8 @@
>  #define PCI_VENDOR_ID_ENSONIQ            0x1274
>  #define PCI_DEVICE_ID_ENSONIQ_ES1370     0x5000
>
> +#define PCI_VENDOR_ID_CHELSIO            0x1425
> +
>  #define PCI_VENDOR_ID_FREESCALE          0x1957
>  #define PCI_DEVICE_ID_MPC8533E           0x0030
>