[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3] pci : Add pba_offset PCI quirk for Chelsio T
From: |
Gabriel Laupre |
Subject: |
Re: [Qemu-devel] [PATCH v3] pci : Add pba_offset PCI quirk for Chelsio T5 devices |
Date: |
Wed, 1 Jul 2015 18:10:20 +0000 |
> What you are suggesting is:
> If table_offset is not as expected, then check if it's a chelsio device.
> If it's not, then print a message. On the other hand, if it's a chelsio
> device, then let msix_init() catch the error. Why ? And if we are sure that
> msix_init will error out, what's the purpose of the table_offset check ?
The test needs only to check the pba_offset and reduced as following is enough
...
/*
* Test the size of the pba_offset variable and catch if it extends outside
* of the specified BAR. If it is the case, we need to apply a hardware
* specific quirk if the device is known or we have a broken configuration.
*/
if (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;
}
}
...
As the hardware problem is only related with the pba_offset and the purpose of
the quirk is to correct the known hardware error. The table_offset has never
been seen as wrong. Therefore the msix_init() sanity check should take care of
a "rare" potential error as you mentioned.
This time I'll wait for ACKs from your side before submitting a new version :)
Gabriel
-----Original Message-----
From: Bandan Das [mailto:address@hidden
Sent: Tuesday, June 30, 2015 6:48 PM
To: Gabriel Laupre
Cc: address@hidden; Casey Leedom; address@hidden; address@hidden; Anish Bhatt;
Michael Boksanyi; address@hidden; address@hidden
Subject: Re: [Qemu-devel] [PATCH v3] pci : Add pba_offset PCI quirk for Chelsio
T5 devices
Gabriel Laupre <address@hidden> writes:
> @ Bandan ...
>> > + + /* 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 ?
>
> I don't know if it is relevant to do all the tests here because in the
> function msix_init() all size are checked. I would prefer keeping this
> test as this to simplify the quirk, i.e. just testing the device
> first, and if another size than the pba_offset is wrong, then the
> sanity check in the function msix_init() will catch the error.
Ok, here's the excerpt:
+ /* 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;
+ } else {
+ error_report("vfio: Hardware reports invalid configuration, "
+ "MSIX data outside of specified BAR");
+ return -EINVAL;
+ }
+ }
What you are suggesting is:
If table_offset is not as expected, then check if it's a chelsio device.
If it's not, then print a message. On the other hand, if it's a chelsio device,
then let msix_init() catch the error. Why ? And if we are sure that msix_init
will error out, what's the purpose of the table_offset check ?
> @ Alex I corrected what you pointed out. I will send the patch v4 in a
> minute.
>
> Thanks you
>
> Gabriel
- Re: [Qemu-devel] [PATCH v3] pci : Add pba_offset PCI quirk for Chelsio T5 devices,
Gabriel Laupre <=