[1/6] net/qede: define PCI config space specific osals

Message ID 20200609194207.24328-2-manishc@marvell.com (mailing list archive)
State Changes Requested, archived
Delegated to: Jerin Jacob
Headers
Series qede: SR-IOV PF driver support |

Checks

Context Check Description
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing fail Testing issues
ci/checkpatch success coding style OK

Commit Message

Manish Chopra June 9, 2020, 7:42 p.m. UTC
  This patch defines various PCI config space access APIs
in order to read and find IOV specific PCI capabilities.

With these definitions implemented, it enables the base
driver to do SR-IOV specific initialization and HW specific
configuration required from PF-PMD driver instance.

Signed-off-by: Manish Chopra <manishc@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
Signed-off-by: Rasesh Mody <rmody@marvell.com>
---
 drivers/net/qede/base/bcm_osal.c | 38 ++++++++++++++++++++++++++++++++
 drivers/net/qede/base/bcm_osal.h | 15 +++++++++----
 drivers/net/qede/base/ecore.h    | 23 +++++++++++++++++++
 drivers/net/qede/qede_main.c     |  1 +
 4 files changed, 73 insertions(+), 4 deletions(-)
  

Comments

Jerin Jacob June 26, 2020, 4:53 a.m. UTC | #1
On Wed, Jun 10, 2020 at 1:13 AM Manish Chopra <manishc@marvell.com> wrote:
>
> This patch defines various PCI config space access APIs
> in order to read and find IOV specific PCI capabilities.
>
> With these definitions implemented, it enables the base
> driver to do SR-IOV specific initialization and HW specific
> configuration required from PF-PMD driver instance.
>
> Signed-off-by: Manish Chopra <manishc@marvell.com>
> Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
> Signed-off-by: Rasesh Mody <rmody@marvell.com>
> ---
> +
> +int osal_pci_find_next_ext_capability(struct rte_pci_device *dev,
> +                                     int cap)


+ Gaetan (PCI maintainer)

Manish,
It must be a candidate for a generic PCI API as it is nothing to do with qede.
Please move to common PCI code if such API is not already present.


> +{
> +       int pos = PCI_CFG_SPACE_SIZE;
> +       uint32_t header;
> +       int ttl;
> +
> +       /* minimum 8 bytes per capability */
> +       ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8;
> +
> +       if (rte_pci_read_config(dev, &header, 4, pos) < 0)
> +               return -1;
> +
> +       /*
> +        * If we have no capabilities, this is indicated by cap ID,
> +        * cap version and next pointer all being 0.
> +        */
> +       if (header == 0)
> +               return 0;
> +
> +       while (ttl-- > 0) {
> +               if (PCI_EXT_CAP_ID(header) == cap)
> +                       return pos;
> +
> +               pos = PCI_EXT_CAP_NEXT(header);
> +
> +               if (pos < PCI_CFG_SPACE_SIZE)
> +                       break;
> +
> +               if (rte_pci_read_config(dev, &header, 4, pos) < 0)
> +                       return -1;
> +       }
> +
> +       return 0;
> +}
>

>
> +#define PCICFG_VENDOR_ID_OFFSET 0x00
> +#define PCICFG_DEVICE_ID_OFFSET 0x02
> +#define PCI_CFG_SPACE_SIZE 256
> +#define PCI_EXP_DEVCTL 0x0008
> +#define PCI_EXT_CAP_ID(header) (int)((header) & 0x0000ffff)
> +#define PCI_EXT_CAP_NEXT(header) (((header) >> 20) & 0xffc)
> +#define PCI_CFG_SPACE_EXP_SIZE 4096
> +
> +#define PCI_SRIOV_CTRL 0x08 /* SR-IOV Control */
> +#define PCI_SRIOV_TOTAL_VF 0x0e /* Total VFs */
> +#define PCI_SRIOV_INITIAL_VF 0x0c /* Initial VFs */
> +#define PCI_SRIOV_NUM_VF 0x10 /* Number of VFs */
> +#define PCI_SRIOV_VF_OFFSET 0x14 /* First VF Offset */
> +#define PCI_SRIOV_VF_STRIDE 0x16 /* Following VF Stride */
> +#define PCI_SRIOV_VF_DID 0x1a
> +#define PCI_SRIOV_SUP_PGSIZE 0x1c
> +#define PCI_SRIOV_CAP 0x04
> +#define PCI_SRIOV_FUNC_LINK 0x12
> +#define PCI_EXT_CAP_ID_SRIOV 0x10

Dont DEFINE PCI_ symbols in drivers, It may conflict with other PCI
definitions in the future.
Please move GENERIC PCI_ symbols to the generic PCI layer.



> +
  
Manish Chopra July 9, 2020, 3:05 p.m. UTC | #2
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Friday, June 26, 2020 10:24 AM
> To: Manish Chopra <manishc@marvell.com>; Gaetan Rivet
> <grive@u256.net>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ferruh Yigit
> <ferruh.yigit@intel.com>; dpdk-dev <dev@dpdk.org>; Igor Russkikh
> <irusskikh@marvell.com>; Rasesh Mody <rmody@marvell.com>; GR-Everest-
> DPDK-Dev <GR-Everest-DPDK-Dev@marvell.com>
> Subject: [EXT] Re: [PATCH 1/6] net/qede: define PCI config space specific
> osals
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Wed, Jun 10, 2020 at 1:13 AM Manish Chopra <manishc@marvell.com>
> wrote:
> >
> > This patch defines various PCI config space access APIs in order to
> > read and find IOV specific PCI capabilities.
> >
> > With these definitions implemented, it enables the base driver to do
> > SR-IOV specific initialization and HW specific configuration required
> > from PF-PMD driver instance.
> >
> > Signed-off-by: Manish Chopra <manishc@marvell.com>
> > Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
> > Signed-off-by: Rasesh Mody <rmody@marvell.com>
> > ---
> > +
> > +int osal_pci_find_next_ext_capability(struct rte_pci_device *dev,
> > +                                     int cap)
> 
> 
> + Gaetan (PCI maintainer)
> 
> Manish,
> It must be a candidate for a generic PCI API as it is nothing to do with qede.
> Please move to common PCI code if such API is not already present.
> 
> 
> > +{
> > +       int pos = PCI_CFG_SPACE_SIZE;
> > +       uint32_t header;
> > +       int ttl;
> > +
> > +       /* minimum 8 bytes per capability */
> > +       ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8;
> > +
> > +       if (rte_pci_read_config(dev, &header, 4, pos) < 0)
> > +               return -1;
> > +
> > +       /*
> > +        * If we have no capabilities, this is indicated by cap ID,
> > +        * cap version and next pointer all being 0.
> > +        */
> > +       if (header == 0)
> > +               return 0;
> > +
> > +       while (ttl-- > 0) {
> > +               if (PCI_EXT_CAP_ID(header) == cap)
> > +                       return pos;
> > +
> > +               pos = PCI_EXT_CAP_NEXT(header);
> > +
> > +               if (pos < PCI_CFG_SPACE_SIZE)
> > +                       break;
> > +
> > +               if (rte_pci_read_config(dev, &header, 4, pos) < 0)
> > +                       return -1;
> > +       }
> > +
> > +       return 0;
> > +}
> >
> 
> >
> > +#define PCICFG_VENDOR_ID_OFFSET 0x00
> > +#define PCICFG_DEVICE_ID_OFFSET 0x02
> > +#define PCI_CFG_SPACE_SIZE 256
> > +#define PCI_EXP_DEVCTL 0x0008
> > +#define PCI_EXT_CAP_ID(header) (int)((header) & 0x0000ffff) #define
> > +PCI_EXT_CAP_NEXT(header) (((header) >> 20) & 0xffc) #define
> > +PCI_CFG_SPACE_EXP_SIZE 4096
> > +
> > +#define PCI_SRIOV_CTRL 0x08 /* SR-IOV Control */ #define
> > +PCI_SRIOV_TOTAL_VF 0x0e /* Total VFs */ #define PCI_SRIOV_INITIAL_VF
> > +0x0c /* Initial VFs */ #define PCI_SRIOV_NUM_VF 0x10 /* Number of VFs
> > +*/ #define PCI_SRIOV_VF_OFFSET 0x14 /* First VF Offset */ #define
> > +PCI_SRIOV_VF_STRIDE 0x16 /* Following VF Stride */ #define
> > +PCI_SRIOV_VF_DID 0x1a #define PCI_SRIOV_SUP_PGSIZE 0x1c #define
> > +PCI_SRIOV_CAP 0x04 #define PCI_SRIOV_FUNC_LINK 0x12 #define
> > +PCI_EXT_CAP_ID_SRIOV 0x10
> 
> Dont DEFINE PCI_ symbols in drivers, It may conflict with other PCI
> definitions in the future.
> Please move GENERIC PCI_ symbols to the generic PCI layer.
> 
> 
> 

Hi Jerin/Gaetan,

Which generic PCI code/files these defines/API should be added to ? (lib/librte_pci/rte_pci.[c|h]) ?
Just FYI, note that it can't be done without cleaning up other vendors, as I can see that various other vendors have also
defined this function to find pci extended cap and some of these PCI_* macro defines as well in their respective drivers.

Thanks,
Manish
  
Jerin Jacob July 9, 2020, 4:11 p.m. UTC | #3
On Thu, Jul 9, 2020 at 8:35 PM Manish Chopra <manishc@marvell.com> wrote:
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Friday, June 26, 2020 10:24 AM
> > To: Manish Chopra <manishc@marvell.com>; Gaetan Rivet
> > <grive@u256.net>
> > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ferruh Yigit
> > <ferruh.yigit@intel.com>; dpdk-dev <dev@dpdk.org>; Igor Russkikh
> > <irusskikh@marvell.com>; Rasesh Mody <rmody@marvell.com>; GR-Everest-
> > DPDK-Dev <GR-Everest-DPDK-Dev@marvell.com>
> > Subject: [EXT] Re: [PATCH 1/6] net/qede: define PCI config space specific
> > osals
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > On Wed, Jun 10, 2020 at 1:13 AM Manish Chopra <manishc@marvell.com>
> > wrote:
> > >
> > > This patch defines various PCI config space access APIs in order to
> > > read and find IOV specific PCI capabilities.
> > >
> > > With these definitions implemented, it enables the base driver to do
> > > SR-IOV specific initialization and HW specific configuration required
> > > from PF-PMD driver instance.
> > >
> > > Signed-off-by: Manish Chopra <manishc@marvell.com>
> > > Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
> > > Signed-off-by: Rasesh Mody <rmody@marvell.com>
> > > ---
> > > +
> > > +int osal_pci_find_next_ext_capability(struct rte_pci_device *dev,
> > > +                                     int cap)
> >
> >
> > + Gaetan (PCI maintainer)
> >
> > Manish,
> > It must be a candidate for a generic PCI API as it is nothing to do with qede.
> > Please move to common PCI code if such API is not already present.
> >
> >
> > > +{
> > > +       int pos = PCI_CFG_SPACE_SIZE;
> > > +       uint32_t header;
> > > +       int ttl;
> > > +
> > > +       /* minimum 8 bytes per capability */
> > > +       ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8;
> > > +
> > > +       if (rte_pci_read_config(dev, &header, 4, pos) < 0)
> > > +               return -1;
> > > +
> > > +       /*
> > > +        * If we have no capabilities, this is indicated by cap ID,
> > > +        * cap version and next pointer all being 0.
> > > +        */
> > > +       if (header == 0)
> > > +               return 0;
> > > +
> > > +       while (ttl-- > 0) {
> > > +               if (PCI_EXT_CAP_ID(header) == cap)
> > > +                       return pos;
> > > +
> > > +               pos = PCI_EXT_CAP_NEXT(header);
> > > +
> > > +               if (pos < PCI_CFG_SPACE_SIZE)
> > > +                       break;
> > > +
> > > +               if (rte_pci_read_config(dev, &header, 4, pos) < 0)
> > > +                       return -1;
> > > +       }
> > > +
> > > +       return 0;
> > > +}
> > >
> >
> > >
> > > +#define PCICFG_VENDOR_ID_OFFSET 0x00
> > > +#define PCICFG_DEVICE_ID_OFFSET 0x02
> > > +#define PCI_CFG_SPACE_SIZE 256
> > > +#define PCI_EXP_DEVCTL 0x0008
> > > +#define PCI_EXT_CAP_ID(header) (int)((header) & 0x0000ffff) #define
> > > +PCI_EXT_CAP_NEXT(header) (((header) >> 20) & 0xffc) #define
> > > +PCI_CFG_SPACE_EXP_SIZE 4096
> > > +
> > > +#define PCI_SRIOV_CTRL 0x08 /* SR-IOV Control */ #define
> > > +PCI_SRIOV_TOTAL_VF 0x0e /* Total VFs */ #define PCI_SRIOV_INITIAL_VF
> > > +0x0c /* Initial VFs */ #define PCI_SRIOV_NUM_VF 0x10 /* Number of VFs
> > > +*/ #define PCI_SRIOV_VF_OFFSET 0x14 /* First VF Offset */ #define
> > > +PCI_SRIOV_VF_STRIDE 0x16 /* Following VF Stride */ #define
> > > +PCI_SRIOV_VF_DID 0x1a #define PCI_SRIOV_SUP_PGSIZE 0x1c #define
> > > +PCI_SRIOV_CAP 0x04 #define PCI_SRIOV_FUNC_LINK 0x12 #define
> > > +PCI_EXT_CAP_ID_SRIOV 0x10
> >
> > Dont DEFINE PCI_ symbols in drivers, It may conflict with other PCI
> > definitions in the future.
> > Please move GENERIC PCI_ symbols to the generic PCI layer.
> >
> >
> >
>
> Hi Jerin/Gaetan,
>
> Which generic PCI code/files these defines/API should be added to ? (lib/librte_pci/rte_pci.[c|h]) ?

Since it generic, To me, lib/librte_pci/rte_pci.[c|h]) is the correct place.

> Just FYI, note that it can't be done without cleaning up other vendors, as I can see that various other vendors have also
> defined this function to find pci extended cap and some of these PCI_* macro defines as well in their respective drivers.
>
> Thanks,
> Manish
  
Manish Chopra July 9, 2020, 10:28 p.m. UTC | #4
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Thursday, July 9, 2020 9:41 PM
> To: Manish Chopra <manishc@marvell.com>
> Cc: Gaetan Rivet <grive@u256.net>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Ferruh Yigit <ferruh.yigit@intel.com>; dpdk-dev
> <dev@dpdk.org>; Igor Russkikh <irusskikh@marvell.com>; Rasesh Mody
> <rmody@marvell.com>; GR-Everest-DPDK-Dev <GR-Everest-DPDK-
> Dev@marvell.com>
> Subject: Re: [EXT] Re: [PATCH 1/6] net/qede: define PCI config space specific
> osals
> 
> On Thu, Jul 9, 2020 at 8:35 PM Manish Chopra <manishc@marvell.com>
> wrote:
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > Sent: Friday, June 26, 2020 10:24 AM
> > > To: Manish Chopra <manishc@marvell.com>; Gaetan Rivet
> > > <grive@u256.net>
> > > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ferruh Yigit
> > > <ferruh.yigit@intel.com>; dpdk-dev <dev@dpdk.org>; Igor Russkikh
> > > <irusskikh@marvell.com>; Rasesh Mody <rmody@marvell.com>;
> > > GR-Everest- DPDK-Dev <GR-Everest-DPDK-Dev@marvell.com>
> > > Subject: [EXT] Re: [PATCH 1/6] net/qede: define PCI config space
> > > specific osals
> > >
> > > External Email
> > >
> > > --------------------------------------------------------------------
> > > -- On Wed, Jun 10, 2020 at 1:13 AM Manish Chopra
> > > <manishc@marvell.com>
> > > wrote:
> > > >
> > > > This patch defines various PCI config space access APIs in order
> > > > to read and find IOV specific PCI capabilities.
> > > >
> > > > With these definitions implemented, it enables the base driver to
> > > > do SR-IOV specific initialization and HW specific configuration
> > > > required from PF-PMD driver instance.
> > > >
> > > > Signed-off-by: Manish Chopra <manishc@marvell.com>
> > > > Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
> > > > Signed-off-by: Rasesh Mody <rmody@marvell.com>
> > > > ---
> > > > +
> > > > +int osal_pci_find_next_ext_capability(struct rte_pci_device *dev,
> > > > +                                     int cap)
> > >
> > >
> > > + Gaetan (PCI maintainer)
> > >
> > > Manish,
> > > It must be a candidate for a generic PCI API as it is nothing to do with
> qede.
> > > Please move to common PCI code if such API is not already present.
> > >
> > >
> > > > +{
> > > > +       int pos = PCI_CFG_SPACE_SIZE;
> > > > +       uint32_t header;
> > > > +       int ttl;
> > > > +
> > > > +       /* minimum 8 bytes per capability */
> > > > +       ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8;
> > > > +
> > > > +       if (rte_pci_read_config(dev, &header, 4, pos) < 0)
> > > > +               return -1;
> > > > +
> > > > +       /*
> > > > +        * If we have no capabilities, this is indicated by cap ID,
> > > > +        * cap version and next pointer all being 0.
> > > > +        */
> > > > +       if (header == 0)
> > > > +               return 0;
> > > > +
> > > > +       while (ttl-- > 0) {
> > > > +               if (PCI_EXT_CAP_ID(header) == cap)
> > > > +                       return pos;
> > > > +
> > > > +               pos = PCI_EXT_CAP_NEXT(header);
> > > > +
> > > > +               if (pos < PCI_CFG_SPACE_SIZE)
> > > > +                       break;
> > > > +
> > > > +               if (rte_pci_read_config(dev, &header, 4, pos) < 0)
> > > > +                       return -1;
> > > > +       }
> > > > +
> > > > +       return 0;
> > > > +}
> > > >
> > >
> > > >
> > > > +#define PCICFG_VENDOR_ID_OFFSET 0x00 #define
> > > > +PCICFG_DEVICE_ID_OFFSET 0x02 #define PCI_CFG_SPACE_SIZE 256
> > > > +#define PCI_EXP_DEVCTL 0x0008 #define PCI_EXT_CAP_ID(header)
> > > > +(int)((header) & 0x0000ffff) #define
> > > > +PCI_EXT_CAP_NEXT(header) (((header) >> 20) & 0xffc) #define
> > > > +PCI_CFG_SPACE_EXP_SIZE 4096
> > > > +
> > > > +#define PCI_SRIOV_CTRL 0x08 /* SR-IOV Control */ #define
> > > > +PCI_SRIOV_TOTAL_VF 0x0e /* Total VFs */ #define
> > > > +PCI_SRIOV_INITIAL_VF 0x0c /* Initial VFs */ #define
> > > > +PCI_SRIOV_NUM_VF 0x10 /* Number of VFs */ #define
> > > > +PCI_SRIOV_VF_OFFSET 0x14 /* First VF Offset */ #define
> > > > +PCI_SRIOV_VF_STRIDE 0x16 /* Following VF Stride */ #define
> > > > +PCI_SRIOV_VF_DID 0x1a #define PCI_SRIOV_SUP_PGSIZE 0x1c #define
> > > > +PCI_SRIOV_CAP 0x04 #define PCI_SRIOV_FUNC_LINK 0x12 #define
> > > > +PCI_EXT_CAP_ID_SRIOV 0x10
> > >
> > > Dont DEFINE PCI_ symbols in drivers, It may conflict with other PCI
> > > definitions in the future.
> > > Please move GENERIC PCI_ symbols to the generic PCI layer.
> > >
> > >
> > >
> >
> > Hi Jerin/Gaetan,
> >
> > Which generic PCI code/files these defines/API should be added to ?
> (lib/librte_pci/rte_pci.[c|h]) ?
> 
> Since it generic, To me, lib/librte_pci/rte_pci.[c|h]) is the correct place.
> 
> > Just FYI, note that it can't be done without cleaning up other
> > vendors, as I can see that various other vendors have also defined this
> function to find pci extended cap and some of these PCI_* macro defines as
> well in their respective drivers.
> >
> > Thanks,
> > Manish

Hi Jerin,

It seems like adding these in lib/librte_pci/rte_pci.[c|h]) is not straight w/o doing forward declarations of func/structs
like (strcut rte_pci_device, rte_pci_read_config()) which are being referenced in pci_find_next_ext_capability(),
as rte_bus_pci.h already have include of rte_pci.h

So, how about adding them in drivers/bus/pci/pci_common.c and drivers/bus/pci/rte_bus_pci.h files directly ?

Also, most of the PCI* defines above are already available from /usr/include/pci_regs.h so I think we don't need them to re-define any again in DPDK tree's headers.
(Assuming that all supported kernels would have latest /usr/include/pci_regs.h)

Thanks,
Manish
  
Jerin Jacob July 10, 2020, 5:03 a.m. UTC | #5
On Fri, Jul 10, 2020 at 3:58 AM Manish Chopra <manishc@marvell.com> wrote:
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Thursday, July 9, 2020 9:41 PM
> > To: Manish Chopra <manishc@marvell.com>
> > Cc: Gaetan Rivet <grive@u256.net>; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; Ferruh Yigit <ferruh.yigit@intel.com>; dpdk-dev
> > <dev@dpdk.org>; Igor Russkikh <irusskikh@marvell.com>; Rasesh Mody
> > <rmody@marvell.com>; GR-Everest-DPDK-Dev <GR-Everest-DPDK-
> > Dev@marvell.com>
> > Subject: Re: [EXT] Re: [PATCH 1/6] net/qede: define PCI config space specific
> > osals
> >
> > On Thu, Jul 9, 2020 at 8:35 PM Manish Chopra <manishc@marvell.com>
> > wrote:
> > >
> > > > -----Original Message-----
> > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > Sent: Friday, June 26, 2020 10:24 AM
> > > > To: Manish Chopra <manishc@marvell.com>; Gaetan Rivet
> > > > <grive@u256.net>
> > > > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ferruh Yigit
> > > > <ferruh.yigit@intel.com>; dpdk-dev <dev@dpdk.org>; Igor Russkikh
> > > > <irusskikh@marvell.com>; Rasesh Mody <rmody@marvell.com>;
> > > > GR-Everest- DPDK-Dev <GR-Everest-DPDK-Dev@marvell.com>
> > > > Subject: [EXT] Re: [PATCH 1/6] net/qede: define PCI config space
> > > > specific osals
> > > >
> > > > External Email
> > > >
> > > > --------------------------------------------------------------------
> > > > -- On Wed, Jun 10, 2020 at 1:13 AM Manish Chopra
> > > > <manishc@marvell.com>
> > > > wrote:
> > > > >
> > > > > This patch defines various PCI config space access APIs in order
> > > > > to read and find IOV specific PCI capabilities.
> > > > >
> > > > > With these definitions implemented, it enables the base driver to
> > > > > do SR-IOV specific initialization and HW specific configuration
> > > > > required from PF-PMD driver instance.
> > > > >
> > > > > Signed-off-by: Manish Chopra <manishc@marvell.com>
> > > > > Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
> > > > > Signed-off-by: Rasesh Mody <rmody@marvell.com>
> > > > > ---
> > > > > +
> > > > > +int osal_pci_find_next_ext_capability(struct rte_pci_device *dev,
> > > > > +                                     int cap)
> > > >
> > > >
> > > > + Gaetan (PCI maintainer)
> > > >
> > > > Manish,
> > > > It must be a candidate for a generic PCI API as it is nothing to do with
> > qede.
> > > > Please move to common PCI code if such API is not already present.
> > > >
> > > >
> > > > > +{
> > > > > +       int pos = PCI_CFG_SPACE_SIZE;
> > > > > +       uint32_t header;
> > > > > +       int ttl;
> > > > > +
> > > > > +       /* minimum 8 bytes per capability */
> > > > > +       ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8;
> > > > > +
> > > > > +       if (rte_pci_read_config(dev, &header, 4, pos) < 0)
> > > > > +               return -1;
> > > > > +
> > > > > +       /*
> > > > > +        * If we have no capabilities, this is indicated by cap ID,
> > > > > +        * cap version and next pointer all being 0.
> > > > > +        */
> > > > > +       if (header == 0)
> > > > > +               return 0;
> > > > > +
> > > > > +       while (ttl-- > 0) {
> > > > > +               if (PCI_EXT_CAP_ID(header) == cap)
> > > > > +                       return pos;
> > > > > +
> > > > > +               pos = PCI_EXT_CAP_NEXT(header);
> > > > > +
> > > > > +               if (pos < PCI_CFG_SPACE_SIZE)
> > > > > +                       break;
> > > > > +
> > > > > +               if (rte_pci_read_config(dev, &header, 4, pos) < 0)
> > > > > +                       return -1;
> > > > > +       }
> > > > > +
> > > > > +       return 0;
> > > > > +}
> > > > >
> > > >
> > > > >
> > > > > +#define PCICFG_VENDOR_ID_OFFSET 0x00 #define
> > > > > +PCICFG_DEVICE_ID_OFFSET 0x02 #define PCI_CFG_SPACE_SIZE 256
> > > > > +#define PCI_EXP_DEVCTL 0x0008 #define PCI_EXT_CAP_ID(header)
> > > > > +(int)((header) & 0x0000ffff) #define
> > > > > +PCI_EXT_CAP_NEXT(header) (((header) >> 20) & 0xffc) #define
> > > > > +PCI_CFG_SPACE_EXP_SIZE 4096
> > > > > +
> > > > > +#define PCI_SRIOV_CTRL 0x08 /* SR-IOV Control */ #define
> > > > > +PCI_SRIOV_TOTAL_VF 0x0e /* Total VFs */ #define
> > > > > +PCI_SRIOV_INITIAL_VF 0x0c /* Initial VFs */ #define
> > > > > +PCI_SRIOV_NUM_VF 0x10 /* Number of VFs */ #define
> > > > > +PCI_SRIOV_VF_OFFSET 0x14 /* First VF Offset */ #define
> > > > > +PCI_SRIOV_VF_STRIDE 0x16 /* Following VF Stride */ #define
> > > > > +PCI_SRIOV_VF_DID 0x1a #define PCI_SRIOV_SUP_PGSIZE 0x1c #define
> > > > > +PCI_SRIOV_CAP 0x04 #define PCI_SRIOV_FUNC_LINK 0x12 #define
> > > > > +PCI_EXT_CAP_ID_SRIOV 0x10
> > > >
> > > > Dont DEFINE PCI_ symbols in drivers, It may conflict with other PCI
> > > > definitions in the future.
> > > > Please move GENERIC PCI_ symbols to the generic PCI layer.
> > > >
> > > >
> > > >
> > >
> > > Hi Jerin/Gaetan,
> > >
> > > Which generic PCI code/files these defines/API should be added to ?
> > (lib/librte_pci/rte_pci.[c|h]) ?
> >
> > Since it generic, To me, lib/librte_pci/rte_pci.[c|h]) is the correct place.
> >
> > > Just FYI, note that it can't be done without cleaning up other
> > > vendors, as I can see that various other vendors have also defined this
> > function to find pci extended cap and some of these PCI_* macro defines as
> > well in their respective drivers.
> > >
> > > Thanks,
> > > Manish
>
> Hi Jerin,
>
> It seems like adding these in lib/librte_pci/rte_pci.[c|h]) is not straight w/o doing forward declarations of func/structs
> like (strcut rte_pci_device, rte_pci_read_config()) which are being referenced in pci_find_next_ext_capability(),
> as rte_bus_pci.h already have include of rte_pci.h
>
> So, how about adding them in drivers/bus/pci/pci_common.c and drivers/bus/pci/rte_bus_pci.h files directly ?

Good to me.

>
> Also, most of the PCI* defines above are already available from /usr/include/pci_regs.h so I think we don't need them to re-define any again in DPDK tree's headers.
> (Assuming that all supported kernels would have latest /usr/include/pci_regs.h)

I think, we can avoid that path to avoid Linux kernel or specific
package dependency

>
> Thanks,
> Manish
  

Patch

diff --git a/drivers/net/qede/base/bcm_osal.c b/drivers/net/qede/base/bcm_osal.c
index 48d016e24..3cf33a9a7 100644
--- a/drivers/net/qede/base/bcm_osal.c
+++ b/drivers/net/qede/base/bcm_osal.c
@@ -14,6 +14,44 @@ 
 #include "ecore_iov_api.h"
 #include "ecore_mcp_api.h"
 #include "ecore_l2_api.h"
+#include <rte_bus_pci.h>
+#include <rte_io.h>
+
+int osal_pci_find_next_ext_capability(struct rte_pci_device *dev,
+				      int cap)
+{
+	int pos = PCI_CFG_SPACE_SIZE;
+	uint32_t header;
+	int ttl;
+
+	/* minimum 8 bytes per capability */
+	ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8;
+
+	if (rte_pci_read_config(dev, &header, 4, pos) < 0)
+		return -1;
+
+	/*
+	 * If we have no capabilities, this is indicated by cap ID,
+	 * cap version and next pointer all being 0.
+	 */
+	if (header == 0)
+		return 0;
+
+	while (ttl-- > 0) {
+		if (PCI_EXT_CAP_ID(header) == cap)
+			return pos;
+
+		pos = PCI_EXT_CAP_NEXT(header);
+
+		if (pos < PCI_CFG_SPACE_SIZE)
+			break;
+
+		if (rte_pci_read_config(dev, &header, 4, pos) < 0)
+			return -1;
+	}
+
+	return 0;
+}
 
 /* Array of memzone pointers */
 static const struct rte_memzone *ecore_mz_mapping[RTE_MAX_MEMZONE];
diff --git a/drivers/net/qede/base/bcm_osal.h b/drivers/net/qede/base/bcm_osal.h
index 8b2faec5b..7cb887409 100644
--- a/drivers/net/qede/base/bcm_osal.h
+++ b/drivers/net/qede/base/bcm_osal.h
@@ -18,6 +18,7 @@ 
 #include <rte_debug.h>
 #include <rte_ether.h>
 #include <rte_io.h>
+#include <rte_bus_pci.h>
 
 /* Forward declaration */
 struct ecore_dev;
@@ -284,10 +285,16 @@  typedef struct osal_list_t {
 
 /* PCI config space */
 
-#define OSAL_PCI_READ_CONFIG_BYTE(dev, address, dst) nothing
-#define OSAL_PCI_READ_CONFIG_WORD(dev, address, dst) nothing
-#define OSAL_PCI_READ_CONFIG_DWORD(dev, address, dst) nothing
-#define OSAL_PCI_FIND_EXT_CAPABILITY(dev, pcie_id) 0
+int osal_pci_find_next_ext_capability(struct rte_pci_device *dev,
+				       int cap);
+#define OSAL_PCI_READ_CONFIG_BYTE(dev, address, dst) \
+	rte_pci_read_config((dev)->pci_dev, dst, 1, address)
+#define OSAL_PCI_READ_CONFIG_WORD(dev, address, dst) \
+	rte_pci_read_config((dev)->pci_dev, dst, 2, address)
+#define OSAL_PCI_READ_CONFIG_DWORD(dev, address, dst) \
+	rte_pci_read_config((dev)->pci_dev, dst, 4, address)
+#define OSAL_PCI_FIND_EXT_CAPABILITY(dev, cap) \
+	osal_pci_find_next_ext_capability((dev)->pci_dev, cap)
 #define OSAL_PCI_FIND_CAPABILITY(dev, pcie_id) 0
 #define OSAL_PCI_WRITE_CONFIG_WORD(dev, address, val) nothing
 #define OSAL_BAR_SIZE(dev, bar_id) 0
diff --git a/drivers/net/qede/base/ecore.h b/drivers/net/qede/base/ecore.h
index b2077bc46..386348e68 100644
--- a/drivers/net/qede/base/ecore.h
+++ b/drivers/net/qede/base/ecore.h
@@ -27,6 +27,26 @@ 
 #include "ecore_proto_if.h"
 #include "mcp_public.h"
 
+#define PCICFG_VENDOR_ID_OFFSET 0x00
+#define PCICFG_DEVICE_ID_OFFSET 0x02
+#define PCI_CFG_SPACE_SIZE 256
+#define PCI_EXP_DEVCTL 0x0008
+#define PCI_EXT_CAP_ID(header) (int)((header) & 0x0000ffff)
+#define PCI_EXT_CAP_NEXT(header) (((header) >> 20) & 0xffc)
+#define PCI_CFG_SPACE_EXP_SIZE 4096
+
+#define PCI_SRIOV_CTRL 0x08 /* SR-IOV Control */
+#define PCI_SRIOV_TOTAL_VF 0x0e /* Total VFs */
+#define PCI_SRIOV_INITIAL_VF 0x0c /* Initial VFs */
+#define PCI_SRIOV_NUM_VF 0x10 /* Number of VFs */
+#define PCI_SRIOV_VF_OFFSET 0x14 /* First VF Offset */
+#define PCI_SRIOV_VF_STRIDE 0x16 /* Following VF Stride */
+#define PCI_SRIOV_VF_DID 0x1a
+#define PCI_SRIOV_SUP_PGSIZE 0x1c
+#define PCI_SRIOV_CAP 0x04
+#define PCI_SRIOV_FUNC_LINK 0x12
+#define PCI_EXT_CAP_ID_SRIOV 0x10
+
 #define ECORE_MAJOR_VERSION		8
 #define ECORE_MINOR_VERSION		40
 #define ECORE_REVISION_VERSION		26
@@ -916,6 +936,9 @@  struct ecore_dev {
 	/* @DPDK */
 	struct ecore_dbg_feature	dbg_features[DBG_FEATURE_NUM];
 	u8				engine_for_debug;
+
+	/* DPDK specific ecore field */
+	struct rte_pci_device		*pci_dev;
 };
 
 enum ecore_hsi_def_type {
diff --git a/drivers/net/qede/qede_main.c b/drivers/net/qede/qede_main.c
index 70357ebb6..62039af6f 100644
--- a/drivers/net/qede/qede_main.c
+++ b/drivers/net/qede/qede_main.c
@@ -36,6 +36,7 @@  static void qed_init_pci(struct ecore_dev *edev, struct rte_pci_device *pci_dev)
 	edev->regview = pci_dev->mem_resource[0].addr;
 	edev->doorbells = pci_dev->mem_resource[2].addr;
 	edev->db_size = pci_dev->mem_resource[2].len;
+	edev->pci_dev = pci_dev;
 }
 
 static int