[PATCH net-next 0/4] chelsio: improve PCI VPD handling

[PATCH net-next 0/4] chelsio: improve PCI VPD handling

[Heiner Kallweit]

Working on PCI VPD core code I came across the Chelsio drivers.
Let's improve the way how they handle PCI VPD.

This series touches only device-specific quirks in the core code,
therefore I think it should go via the netdev tree.

Heiner Kallweit (4):
  PCI/VPD: Remove Chelsio T3 quirk
  cxgb4: remove unused vpd_cap_addr
  PCI/VPD: Change Chelsio T4 quirk to provide access to full virtual
    address space
  cxgb4: remove changing VPD len

 .../net/ethernet/chelsio/cxgb4/cudbg_entity.h |  1 -
 .../net/ethernet/chelsio/cxgb4/cudbg_lib.c    | 21 ++++---------------
 drivers/net/ethernet/chelsio/cxgb4/cxgb4.h    |  1 -
 .../net/ethernet/chelsio/cxgb4/cxgb4_main.c   |  2 --
 drivers/pci/vpd.c                             | 18 ++++------------
 5 files changed, 8 insertions(+), 35 deletions(-)

-- 
2.30.0

[PATCH net-next 1/4] PCI/VPD: Remove Chelsio T3 quirk

[Heiner Kallweit]

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@gmail.com>
---
 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);
-	else if (chip >= 0x4 && func < 0x8)
+	if (chip >= 0x4 && func < 0x8)
 		pci_set_vpd_size(dev, 2048);
 }
 
-- 
2.30.0

Re: [PATCH net-next 1/4] PCI/VPD: Remove Chelsio T3 quirk

[Jakub Kicinski]

On Tue, 2 Feb 2021 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@gmail.com>

Would this not affect the size of the file under sysfs?

> 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);
> -	else if (chip >= 0x4 && func < 0x8)
> +	if (chip >= 0x4 && func < 0x8)
>  		pci_set_vpd_size(dev, 2048);
>  }
>  

Re: [PATCH net-next 1/4] PCI/VPD: Remove Chelsio T3 quirk

[Heiner Kallweit]

On 04.02.2021 03:30, Jakub Kicinski wrote:
> On Tue, 2 Feb 2021 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@gmail.com>
> 
> Would this not affect the size of the file under sysfs?
> 
Good point. Not the size (because it's 0 = unlimited), but the exposed
data would be limited to what can be auto-detected from offset 0.
Most T3 devices have the VPD at offset 0x0c00, and I'd expect (don't have
test hw) that there's no valid VPD structure at offset 0.
Therefore no VPD data will be exposed via sysfs. But:

- VPD data starting at an offset doesn't follow PCI spec. Therefore it's
  questionable whether anybody can expect such data to be available via sysfs.

- Typical userspace tools like lspci start parsing VPD at offset 0,
  and therefore won't recognize the VPD data also as of today.
  (again: no test hw to verify this)

- There might be Chelsio-provided userspace tools that use sysfs VPD access
  and parse the data based on knowledge of the proprietary VPD layout.
  Such a usecase would be broken now, indeed. Not sure whether any such
  tool exists, maybe Raju can comment on this.



>> 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);
>> -	else if (chip >= 0x4 && func < 0x8)
>> +	if (chip >= 0x4 && func < 0x8)
>>  		pci_set_vpd_size(dev, 2048);
>>  }
>>  
> 

Re: [PATCH net-next 1/4] PCI/VPD: Remove Chelsio T3 quirk

[Raju Rangoju]

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@gmail.com>
> ---
>  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?

----------------------------------------------------------
commit 1c7de2b4ff886a45fbd2f4c3d4627e0f37a9dd77
Author: Alexey Kardashevskiy <aik@ozlabs.ru>
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@ozlabs.ru>
   Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

---------------------------------------------


> -	else if (chip >= 0x4 && func < 0x8)
> +	if (chip >= 0x4 && func < 0x8)
>  		pci_set_vpd_size(dev, 2048);
>  }
>  
> -- 
> 2.30.0
> 
> 

Re: [PATCH net-next 1/4] PCI/VPD: Remove Chelsio T3 quirk

[Heiner Kallweit]

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@gmail.com>
>> ---
>>  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@ozlabs.ru>
> 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@ozlabs.ru>
>    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> ---------------------------------------------
> 
> 
>> -	else if (chip >= 0x4 && func < 0x8)
>> +	if (chip >= 0x4 && func < 0x8)
>>  		pci_set_vpd_size(dev, 2048);
>>  }
>>  
>> -- 
>> 2.30.0
>>
>>

[PATCH net-next 2/4] cxgb4: remove unused vpd_cap_addr

[Heiner Kallweit]

Presumably this is a leftover from T3 driver heritage. cxgb4 uses the
PCI core VPD access code that handles detection of VPD capabilities.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4.h      | 1 -
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 2 --
 2 files changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index 8e681ce72..314f8d806 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -414,7 +414,6 @@ struct pf_resources {
 };
 
 struct pci_params {
-	unsigned int vpd_cap_addr;
 	unsigned char speed;
 	unsigned char width;
 };
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 9f1965c80..6264bc66a 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -3201,8 +3201,6 @@ static void cxgb4_mgmt_fill_vf_station_mac_addr(struct adapter *adap)
 	int err;
 	u8 *na;
 
-	adap->params.pci.vpd_cap_addr = pci_find_capability(adap->pdev,
-							    PCI_CAP_ID_VPD);
 	err = t4_get_raw_vpd_params(adap, &adap->params.vpd);
 	if (err)
 		return;
-- 
2.30.0

[PATCH net-next 3/4] PCI/VPD: Change Chelsio T4 quirk to provide access to full virtual EEPROM address space

[Heiner Kallweit]

cxgb4 uses the full VPD address space for accessing its EEPROM (with some
mapping, see t4_eeprom_ptov()). In cudbg_collect_vpd_data() it sets the
VPD len to 32K (PCI_VPD_MAX_SIZE), and then back to 2K (CUDBG_VPD_PF_SIZE).
Having official (structured) and unofficial (unstructured) VPD data
violates the PCI spec, let's set VPD len according to all data that can be
accessed via PCI VPD access, no matter of its structure.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/pci/vpd.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c
index db86fe226..90f17f3b7 100644
--- a/drivers/pci/vpd.c
+++ b/drivers/pci/vpd.c
@@ -630,16 +630,11 @@ static void quirk_chelsio_extend_vpd(struct pci_dev *dev)
 	int func = (dev->device & 0x0f00) >>  8;
 
 	/*
-	 * 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 this is a T4 or later based adapter, provide access to the full
+	 * virtual EEPROM address space.
 	 */
 	if (chip >= 0x4 && func < 0x8)
-		pci_set_vpd_size(dev, 2048);
+		pci_set_vpd_size(dev, PCI_VPD_MAX_SIZE);
 }
 
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, PCI_ANY_ID,
-- 
2.30.0

[PATCH net-next 4/4] cxgb4: remove changing VPD len

[Heiner Kallweit]

Now that the PCI VPD for Chelsio devices from T4 on has been changed and
VPD len is set to PCI_VPD_MAX_SIZE (32K), we don't have to change the
VPD len any longer.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 .../net/ethernet/chelsio/cxgb4/cudbg_entity.h |  1 -
 .../net/ethernet/chelsio/cxgb4/cudbg_lib.c    | 21 ++++---------------
 2 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cudbg_entity.h b/drivers/net/ethernet/chelsio/cxgb4/cudbg_entity.h
index 876f90e57..02ccb610a 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cudbg_entity.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cudbg_entity.h
@@ -220,7 +220,6 @@ struct cudbg_mps_tcam {
 	u8 reserved[2];
 };
 
-#define CUDBG_VPD_PF_SIZE 0x800
 #define CUDBG_SCFG_VER_ADDR 0x06
 #define CUDBG_SCFG_VER_LEN 4
 #define CUDBG_VPD_VER_ADDR 0x18c7
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c b/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c
index 75474f810..addac5518 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c
@@ -2689,7 +2689,7 @@ int cudbg_collect_vpd_data(struct cudbg_init *pdbg_init,
 	u32 scfg_vers, vpd_vers, fw_vers;
 	struct cudbg_vpd_data *vpd_data;
 	struct vpd_params vpd = { 0 };
-	int rc, ret;
+	int rc;
 
 	rc = t4_get_raw_vpd_params(padap, &vpd);
 	if (rc)
@@ -2699,24 +2699,11 @@ int cudbg_collect_vpd_data(struct cudbg_init *pdbg_init,
 	if (rc)
 		return rc;
 
-	/* Serial Configuration Version is located beyond the PF's vpd size.
-	 * Temporarily give access to entire EEPROM to get it.
-	 */
-	rc = pci_set_vpd_size(padap->pdev, EEPROMVSIZE);
-	if (rc < 0)
-		return rc;
-
-	ret = cudbg_read_vpd_reg(padap, CUDBG_SCFG_VER_ADDR, CUDBG_SCFG_VER_LEN,
-				 &scfg_vers);
-
-	/* Restore back to original PF's vpd size */
-	rc = pci_set_vpd_size(padap->pdev, CUDBG_VPD_PF_SIZE);
-	if (rc < 0)
+	rc = cudbg_read_vpd_reg(padap, CUDBG_SCFG_VER_ADDR, CUDBG_SCFG_VER_LEN,
+				&scfg_vers);
+	if (rc)
 		return rc;
 
-	if (ret)
-		return ret;
-
 	rc = cudbg_read_vpd_reg(padap, CUDBG_VPD_VER_ADDR, CUDBG_VPD_VER_LEN,
 				vpd_str);
 	if (rc)
-- 
2.30.0

Re: [PATCH net-next 0/4] chelsio: improve PCI VPD handling

[Jakub Kicinski]

On Tue, 2 Feb 2021 21:34:49 +0100 Heiner Kallweit wrote:
> Working on PCI VPD core code I came across the Chelsio drivers.
> Let's improve the way how they handle PCI VPD.
> 
> This series touches only device-specific quirks in the core code,
> therefore I think it should go via the netdev tree.

Raju, please take a look.