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

[Alex Williamson]

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