On 05.02.2021 13:42, Raju Rangoju wrote:
> On Tuesday, February 02/02/21, 2021 at 21:35:55 +0100, Heiner Kallweit wrote:
>> cxgb3 driver doesn't use the PCI core code for VPD access, it has its own
>> implementation. Therefore we don't need a quirk for it in the core code.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
>> ---
>> drivers/pci/vpd.c | 13 ++++---------
>> 1 file changed, 4 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c
>> index 7915d10f9..db86fe226 100644
>> --- a/drivers/pci/vpd.c
>> +++ b/drivers/pci/vpd.c
>> @@ -628,22 +628,17 @@ static void quirk_chelsio_extend_vpd(struct pci_dev *dev)
>> {
>> int chip = (dev->device & 0xf000) >> 12;
>> int func = (dev->device & 0x0f00) >> 8;
>> - int prod = (dev->device & 0x00ff) >> 0;
>>
>> /*
>> - * If this is a T3-based adapter, there's a 1KB VPD area at offset
>> - * 0xc00 which contains the preferred VPD values. If this is a T4 or
>> - * later based adapter, the special VPD is at offset 0x400 for the
>> - * Physical Functions (the SR-IOV Virtual Functions have no VPD
>> - * Capabilities). The PCI VPD Access core routines will normally
>> + * If this is a T4 or later based adapter, the special VPD is at offset
>> + * 0x400 for the Physical Functions (the SR-IOV Virtual Functions have
>> + * no VPD Capabilities). The PCI VPD Access core routines will normally
>> * compute the size of the VPD by parsing the VPD Data Structure at
>> * offset 0x000. This will result in silent failures when attempting
>> * to accesses these other VPD areas which are beyond those computed
>> * limits.
>> */
>> - if (chip == 0x0 && prod >= 0x20)
>> - pci_set_vpd_size(dev, 8192);
>
> The above quirk has been added by the following commit to fix VPD access
> issue in the guest VM. Wouldn't removing this quirk reopen the original
> issue?
>
Indeed, I looked at cxgb3 and missed the problematic vfio-pci use case.
So let me remove patch 1 from the series.
> ----------------------------------------------------------
> commit 1c7de2b4ff886a45fbd2f4c3d4627e0f37a9dd77
> Author: Alexey Kardashevskiy <aik@xxxxxxxxx>
> Date: Mon Oct 24 18:04:17 2016 +1100
>
> PCI: Enable access to non-standard VPD for Chelsio devices (cxgb3)
>
> There is at least one Chelsio 10Gb card which uses VPD area to store
> some non-standard blocks (example below). However pci_vpd_size() returns the
> length of the first block only assuming that there can be only one VPD "End
> Tag".
>
> Since 4e1a635552d3 ("vfio/pci: Use kernel VPD access functions"), VFIO
> blocks access beyond that offset, which prevents the guest "cxgb3" driver
> from probing the device. The host system does not have this problem as its
> driver accesses the config space directly without pci_read_vpd().
>
> Add a quirk to override the VPD size to a bigger value. The maximum size
> is taken from EEPROMSIZE in drivers/net/ethernet/chelsio/cxgb3/common.h.
> We do not read the tag as the cxgb3 driver does as the driver supports
> writing to EEPROM/VPD and when it writes, it only checks for 8192 bytes
> boundary. The quirk is registered for all devices supported by the cxgb3
> driver.
>
> This adds a quirk to the PCI layer (not to the cxgb3 driver) as the cxgb3
> driver itself accesses VPD directly and the problem only exists with the
> vfio-pci driver (when cxgb3 is not running on the host and may not be even
> loaded) which blocks accesses beyond the first block of VPD data. However
> vfio-pci itself does not have quirks mechanism so we add it to PCI.
>
> This is the controller:
> Ethernet controller [0200]: Chelsio Communications Inc T310 10GbE Single
> Port Adapter [1425:0030]
>
> This is what I parsed from its VPD:
> ===
> b'\x82*\x0010 Gigabit Ethernet-SR PCI Express Adapter\x90J\x00EC\x07D76809
> FN\x0746K'
>
> 0000 Large item 42 bytes; name 0x2 Identifier String
> b'10 Gigabit Ethernet-SR PCI Express Adapter'
> 002d Large item 74 bytes; name 0x10
> #00 [EC] len=7: b'D76809'
> #0a [FN] len=7: b'46K7897'
> #14 [PN] len=7: b'46K7897'
> #1e [MN] len=4: b'1037'
> #25 [FC] len=4: b'5769'
> #2c [SN] len=12: b'YL102035603V'
> #3b [NA] len=12: b'00145E992ED1'
> 007a Small item 1 bytes; name 0xf End Tag
> 0c00 Large item 16 bytes; name 0x2 Identifier String
> b'S310E-SR-X '
> 0c13 Large item 234 bytes; name 0x10
> #00 [PN] len=16: b'TBD '
> #13 [EC] len=16: b'110107730D2 '
> #26 [SN] len=16: b'97YL102035603V '
> #39 [NA] len=12: b'00145E992ED1'
> #48 [V0] len=6: b'175000'
> #51 [V1] len=6: b'266666'
> #5a [V2] len=6: b'266666'
> #63 [V3] len=6: b'2000 '
> #6c [V4] len=2: b'1 '
> #71 [V5] len=6: b'c2 '
> #7a [V6] len=6: b'0 '
> #83 [V7] len=2: b'1 '
> #88 [V8] len=2: b'0 '
> #8d [V9] len=2: b'0 '
> #92 [VA] len=2: b'0 '
> #97 [RV] len=80:
> b's\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'...
> 0d00 Large item 252 bytes; name 0x11
> #00 [VC] len=16: b'122310_1222 dp '
> #13 [VD] len=16: b'610-0001-00 H1\x00\x00'
> #26 [VE] len=16: b'122310_1353 fp '
> #39 [VF] len=16: b'610-0001-00 H1\x00\x00'
> #4c [RW] len=173:
> b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'...
> 0dff Small item 0 bytes; name 0xf End Tag
>
> 10f3 Large item 13315 bytes; name 0x62
> !!! unknown item name 98:
> b'\xd0\x03\x00@`\x0c\x08\x00\x00\x00\x00\x00\x00\x00\x00\x00'
> ===
>
> Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxxxx>
> Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
>
> ---------------------------------------------
>
>
>> - else if (chip >= 0x4 && func < 0x8)
>> + if (chip >= 0x4 && func < 0x8)
>> pci_set_vpd_size(dev, 2048);
>> }
>>
>> --
>> 2.30.0
>>
>>
