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

[Casey Leedom]

  Hi Alex, the issue is that 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.  (There was a 
mistaken <<3 used in the IP Glue Logic for the PCI Configuration Space of the 
VFs.)

  This bug has been fixed in our next chip — unimaginatively enough called T6 — 
but there are no plans to respin the T5 ASIC for this bug.  We've just 
documented it in our T5 Errata and left it at that.

  Prior to this issue that Gabriel is tackling, the mis-reported PBA Offset 
wasn't a problem because no Host Software that we were aware of ever used that 
value.  Apparently the software that Gabriel is using does look at the PBA 
Value and complains that the 0x8000 is outside the BAR4/5 MSI-X range, which is 
itself only 8KB in length.  (Although it's still not clear to me that the 
software in question ever bothers doing anything with the PBA Offset.)

  So I suggested to Gabriel that he follow the Linux kernel model and provide a 
Device Specific "Quirk" which would detect a T5 VF and hard wire the PBA Offset 
to 0x1000 for such devices.

  Please let me know if you'd like any more background information.

Casey

________________________________________
From: Alex Williamson address@hidden
Sent: Thursday, June 25, 2015 12:19 PM
To: Gabriel Laupre
Cc: address@hidden; address@hidden; address@hidden; address@hidden; Casey 
Leedom; Michael Boksanyi; Anish Bhatt
Subject: Re: [PATCH] pci : Add pba_offset PCI quirk for Chelsio T5 devices

On Thu, 2015-06-25 at 19:00 +0000, Gabriel Laupre wrote:
> @Bandan
> > Is the array offset guaranteed to always be the same ?
> The returned value depends on the physical function and should be 0x1000 for 
> the T5 series. Therefore this offset is guaranteed to always be the same.
>
> > What are the chances of this getting fixed by a firmware update ? :)
> It isn't a firmware issue, therefore can't be fixed via firmware update. This 
> will be resolved for the next versions.
>
> @Alex
> > Can we detect that the pba_offset is wrong?  Is it a consistent and 
> > obviously incorrect value?
> The pba_offset returned will always be 0x8000 for a virtual function for a T5 
> device. It is obviously too big. It is an hardware problem in the T5 series.


Is the current failure mode that msix_init() fails because pba_offset
(or pba_offset + pba_size) extends outside of the specified BAR?  If the
problem is going to be resolved in the next version, will that be
different PCI device IDs?  Ideally we'd only enable the quirk if we knew
we were in a bogus situation, so we can let new hardware work as it
describes itself if it gets fixed.  Thanks,

Alex

> Also I will need to correct the patch as the virtual function devices are 
> encoded as 0x58xx . This quirk is only related with the virtual functions.
>
> Gabriel
>
> -----Original Message-----
> From: Alex Williamson [mailto:address@hidden
> Sent: Thursday, June 25, 2015 7:08 AM
> To: Gabriel Laupre
> Cc: address@hidden; address@hidden; address@hidden; address@hidden; Casey 
> Leedom; Michael Boksanyi; Anish Bhatt
> Subject: Re: [PATCH] pci : Add pba_offset PCI quirk for Chelsio T5 devices
>
> On Wed, 2015-06-24 at 19:04 -0700, Gabriel Laupre wrote:
> > Fix pba_offset initialization value for Chelsio T5 devices.  The
> > hardware doesn't return the correct pba_offset value, so add a quirk
> > to instead return a hardcoded value of 0x1000 when a Chelsio
> > T5 device is detected.
> >
> > Signed-off-by: Gabriel Laupre <address@hidden>
> > ---
> >  hw/vfio/pci.c            | 12 ++++++++++++
> >  include/hw/pci/pci_ids.h |  3 +++
> >  2 files changed, 15 insertions(+)
> >
> > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index e0e339a..8a4c7cd
> > 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;
> >
> > +    /* Quirk to set the pba_offset value for Chelsio T5
> > +     * devices. Since hardware does not return value correctly,
> > +     * we override with a hardcoded value instead.
> > +     */
> > +    if (vendor == PCI_VENDOR_ID_CHELSIO &&
> > +        (device & 0xf000) == PCI_DEVICE_ID_CHELSIO_T5_SERIES) {
> > +        vdev->msix->pba_offset = 0x1000;
> > +    }
>
> Wow, so we're writing off a whole 1/16th of the Chelsio device ID space as 
> broken?  Can we detect that the pba_offset is wrong?  Is it a consistent and 
> obviously incorrect value?  Thanks,
>
> Alex
>
> > +
> >      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..9f649da 100644
> > --- a/include/hw/pci/pci_ids.h
> > +++ b/include/hw/pci/pci_ids.h
> > @@ -114,6 +114,9 @@
> >  #define PCI_VENDOR_ID_ENSONIQ            0x1274
> >  #define PCI_DEVICE_ID_ENSONIQ_ES1370     0x5000
> >
> > +#define PCI_VENDOR_ID_CHELSIO            0x1425
> > +#define PCI_DEVICE_ID_CHELSIO_T5_SERIES  0x5000
> > +
> >  #define PCI_VENDOR_ID_FREESCALE          0x1957
> >  #define PCI_DEVICE_ID_MPC8533E           0x0030
> >
>
>
>