diff mbox series

[net-next,v2,2/3] PCI/VPD: Change Chelsio T4 quirk to provide access to full virtual address space

Message ID 0e55480b-67cb-8a2f-fb82-734d4b1b0eb0@gmail.com
State New
Headers show
Series cxgb4: improve PCI VPD handling | expand

Commit Message

Heiner Kallweit Feb. 5, 2021, 2:26 p.m. UTC
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 inofficial (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 | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Alexander Duyck Feb. 8, 2021, 4:59 p.m. UTC | #1
On Fri, Feb 5, 2021 at 2:15 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> 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 inofficial (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 | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c
> index 7915d10f9..06a7954d0 100644
> --- a/drivers/pci/vpd.c
> +++ b/drivers/pci/vpd.c
> @@ -633,9 +633,8 @@ static void quirk_chelsio_extend_vpd(struct pci_dev *dev)
>         /*
>          * 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
> +        * later based adapter, provide access to the full virtual EEPROM
> +        * address space. 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
> @@ -644,7 +643,7 @@ static void quirk_chelsio_extend_vpd(struct pci_dev *dev)
>         if (chip == 0x0 && prod >= 0x20)
>                 pci_set_vpd_size(dev, 8192);
>         else 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,

So as I recall the size value was added when some hardware was hanging
when an out-of-bounds read occured from various tools accessing the
VPD. I'm assuming if you are enabling full access the T4 hardware can
handle cases where an out-of-bounds read is requested?

Otherwise the code itself looks fine to me.

Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
diff mbox series

Patch

diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c
index 7915d10f9..06a7954d0 100644
--- a/drivers/pci/vpd.c
+++ b/drivers/pci/vpd.c
@@ -633,9 +633,8 @@  static void quirk_chelsio_extend_vpd(struct pci_dev *dev)
 	/*
 	 * 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
+	 * later based adapter, provide access to the full virtual EEPROM
+	 * address space. 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
@@ -644,7 +643,7 @@  static void quirk_chelsio_extend_vpd(struct pci_dev *dev)
 	if (chip == 0x0 && prod >= 0x20)
 		pci_set_vpd_size(dev, 8192);
 	else 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,