* [PATCH net-next v2 0/3] cxgb4: improve PCI VPD handling
@ 2021-02-05 14:24 Heiner Kallweit
2021-02-05 14:25 ` [PATCH net-next v2 1/3] cxgb4: remove unused vpd_cap_addr Heiner Kallweit
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Heiner Kallweit @ 2021-02-05 14:24 UTC (permalink / raw)
To: Raju Rangoju, Jakub Kicinski, David Miller, Bjorn Helgaas
Cc: linux-pci, netdev
Working on PCI VPD core code I came across the Chelsio drivers.
Let's improve the way how cxgb4 handles PCI VPD.
One major goal is to eventually remove pci_set_vpd_size(),
cxgb4 is the only user. The amount of data exposed via the VPD
interface is fixed, therefore I see no benefit in providing
an interface for manipulating the VPD size.
This series touches only device-specific quirks in the core code,
therefore I think it should go via the netdev tree.
v2:
- remove patch 1 from the series
Heiner Kallweit (3):
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 | 7 +++----
5 files changed, 7 insertions(+), 25 deletions(-)
--
2.30.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH net-next v2 1/3] cxgb4: remove unused vpd_cap_addr 2021-02-05 14:24 [PATCH net-next v2 0/3] cxgb4: improve PCI VPD handling Heiner Kallweit @ 2021-02-05 14:25 ` Heiner Kallweit 2021-02-08 17:06 ` Alexander Duyck 2021-02-05 14:26 ` [PATCH net-next v2 2/3] PCI/VPD: Change Chelsio T4 quirk to provide access to full virtual address space Heiner Kallweit 2021-02-05 14:27 ` [PATCH net-next v2 3/3] cxgb4: remove changing VPD len Heiner Kallweit 2 siblings, 1 reply; 7+ messages in thread From: Heiner Kallweit @ 2021-02-05 14:25 UTC (permalink / raw) To: Raju Rangoju, Jakub Kicinski, David Miller, Bjorn Helgaas Cc: linux-pci, netdev Supposedly 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 1/3] cxgb4: remove unused vpd_cap_addr 2021-02-05 14:25 ` [PATCH net-next v2 1/3] cxgb4: remove unused vpd_cap_addr Heiner Kallweit @ 2021-02-08 17:06 ` Alexander Duyck 0 siblings, 0 replies; 7+ messages in thread From: Alexander Duyck @ 2021-02-08 17:06 UTC (permalink / raw) To: Heiner Kallweit Cc: Raju Rangoju, Jakub Kicinski, David Miller, Bjorn Helgaas, linux-pci, netdev On Fri, Feb 5, 2021 at 2:29 PM Heiner Kallweit <hkallweit1@gmail.com> wrote: > > Supposedly 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> Instead of starting with the "Supposedly this is" it might be better to word it along the lines of "This is likely". The "Supposedly" makes it sound like you heard this as a rumor from somebody else. Other than that nit about the description the change looks good to me. Reviewed-by: Alexander Duyck <alexanderduyck@fb.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 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next v2 2/3] PCI/VPD: Change Chelsio T4 quirk to provide access to full virtual address space 2021-02-05 14:24 [PATCH net-next v2 0/3] cxgb4: improve PCI VPD handling Heiner Kallweit 2021-02-05 14:25 ` [PATCH net-next v2 1/3] cxgb4: remove unused vpd_cap_addr Heiner Kallweit @ 2021-02-05 14:26 ` Heiner Kallweit 2021-02-08 16:59 ` Alexander Duyck 2021-02-05 14:27 ` [PATCH net-next v2 3/3] cxgb4: remove changing VPD len Heiner Kallweit 2 siblings, 1 reply; 7+ messages in thread From: Heiner Kallweit @ 2021-02-05 14:26 UTC (permalink / raw) To: Raju Rangoju, Jakub Kicinski, David Miller, Bjorn Helgaas Cc: linux-pci, netdev 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, -- 2.30.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 2/3] PCI/VPD: Change Chelsio T4 quirk to provide access to full virtual address space 2021-02-05 14:26 ` [PATCH net-next v2 2/3] PCI/VPD: Change Chelsio T4 quirk to provide access to full virtual address space Heiner Kallweit @ 2021-02-08 16:59 ` Alexander Duyck 0 siblings, 0 replies; 7+ messages in thread From: Alexander Duyck @ 2021-02-08 16:59 UTC (permalink / raw) To: Heiner Kallweit Cc: Raju Rangoju, Jakub Kicinski, David Miller, Bjorn Helgaas, linux-pci, netdev 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> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next v2 3/3] cxgb4: remove changing VPD len 2021-02-05 14:24 [PATCH net-next v2 0/3] cxgb4: improve PCI VPD handling Heiner Kallweit 2021-02-05 14:25 ` [PATCH net-next v2 1/3] cxgb4: remove unused vpd_cap_addr Heiner Kallweit 2021-02-05 14:26 ` [PATCH net-next v2 2/3] PCI/VPD: Change Chelsio T4 quirk to provide access to full virtual address space Heiner Kallweit @ 2021-02-05 14:27 ` Heiner Kallweit 2021-02-08 17:07 ` Alexander Duyck 2 siblings, 1 reply; 7+ messages in thread From: Heiner Kallweit @ 2021-02-05 14:27 UTC (permalink / raw) To: Raju Rangoju, Jakub Kicinski, David Miller, Bjorn Helgaas Cc: linux-pci, netdev Now that the PCI VPD for Chelsio devices from T4 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 3/3] cxgb4: remove changing VPD len 2021-02-05 14:27 ` [PATCH net-next v2 3/3] cxgb4: remove changing VPD len Heiner Kallweit @ 2021-02-08 17:07 ` Alexander Duyck 0 siblings, 0 replies; 7+ messages in thread From: Alexander Duyck @ 2021-02-08 17:07 UTC (permalink / raw) To: Heiner Kallweit Cc: Raju Rangoju, Jakub Kicinski, David Miller, Bjorn Helgaas, linux-pci, netdev On Fri, Feb 5, 2021 at 2:18 PM Heiner Kallweit <hkallweit1@gmail.com> wrote: > > Now that the PCI VPD for Chelsio devices from T4 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) Assuming that patch 2 is okay then this patch should be fine since it is just toggling back and forth between the same value anyway. Reviewed-by: Alexander Duyck <alexanderduyck@fb.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-02-08 17:09 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-05 14:24 [PATCH net-next v2 0/3] cxgb4: improve PCI VPD handling Heiner Kallweit 2021-02-05 14:25 ` [PATCH net-next v2 1/3] cxgb4: remove unused vpd_cap_addr Heiner Kallweit 2021-02-08 17:06 ` Alexander Duyck 2021-02-05 14:26 ` [PATCH net-next v2 2/3] PCI/VPD: Change Chelsio T4 quirk to provide access to full virtual address space Heiner Kallweit 2021-02-08 16:59 ` Alexander Duyck 2021-02-05 14:27 ` [PATCH net-next v2 3/3] cxgb4: remove changing VPD len Heiner Kallweit 2021-02-08 17:07 ` Alexander Duyck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).