From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33280) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZA3Xh-0000sj-8r for qemu-devel@nongnu.org; Tue, 30 Jun 2015 17:58:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZA3Xe-000898-0u for qemu-devel@nongnu.org; Tue, 30 Jun 2015 17:58:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38379) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZA3Xd-00088y-R1 for qemu-devel@nongnu.org; Tue, 30 Jun 2015 17:58:09 -0400 From: Bandan Das References: <1435698210-15999-1-git-send-email-glaupre@chelsio.com> Date: Tue, 30 Jun 2015 17:58:03 -0400 In-Reply-To: <1435698210-15999-1-git-send-email-glaupre@chelsio.com> (Gabriel Laupre's message of "Tue, 30 Jun 2015 14:03:30 -0700") Message-ID: MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v3] pci : Add pba_offset PCI quirk for Chelsio T5 devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gabriel Laupre Cc: jb-gnumlists@wisemo.com, leedom@chelsio.com, mst@redhat.com, qemu-devel@nongnu.org, anish@chelsio.com, mboksanyi@chelsio.com, alex.williamson@redhat.com, bsd@makefile.in Gabriel Laupre writes: ... > + /* Test the size of the pba variables and catch if they extend outside of > + * the specified BAR. If it is the case, we have a broken configuration or > + * we need to apply a hardware specific quirk. */ > + 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. 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; For the rare case where table_offset is wrong for the device being checked for above and pba_offset is actually correct, shouldn't we fail ? > + } else { > + error_report("vfio: Hardware reports invalid configuration, " > + "MSIX data outside of specified BAR"); Since we are printing anyway, and we have already made the check above, why not print exactly what's wrong instead of "MSIX data" ? > + 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