[v7,2/9] eal: introduce RTE common initialization level

  Currently mlx5_common uses CLASS priority to initialize
common code before initializing the PMD.
However mlx5_common is not really a class, it is the pre-initialization
code needed for the PMDs.

In subsequent patch a needed initialization sequence is:
(a) Initialize bus (say pci)
(b) Initialize common code of a driver (mlx5_common)
(c) Register mlx5 class PMDs (mlx5 net, mlx5 vdpa)
Information registered by these PMDs is used by mlx5_bus_pci PMD.
This mlx5 class PMDs should not confused with rte_class.
(d) Register mlx5 PCI bus PMD

Hence, introduce a new RTE priority level RTE_PRIO_COMMON which
can be used for common initialization and RTE_PRIO_CLASS by mlx5 PMDs
for class driver initialization.

Signed-off-by: Parav Pandit <parav@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
---
Changelog:
v2->v3:
 - new patch
---
 lib/librte_eal/include/rte_common.h | 1 +
 1 file changed, 1 insertion(+)
  

On 7/17/2020 2:49 PM, Parav Pandit wrote:
> Currently mlx5_common uses CLASS priority to initialize
> common code before initializing the PMD.
> However mlx5_common is not really a class, it is the pre-initialization
> code needed for the PMDs.
> 
> In subsequent patch a needed initialization sequence is:
> (a) Initialize bus (say pci)
> (b) Initialize common code of a driver (mlx5_common)
> (c) Register mlx5 class PMDs (mlx5 net, mlx5 vdpa)
> Information registered by these PMDs is used by mlx5_bus_pci PMD.
> This mlx5 class PMDs should not confused with rte_class.
> (d) Register mlx5 PCI bus PMD
> 
> Hence, introduce a new RTE priority level RTE_PRIO_COMMON which
> can be used for common initialization and RTE_PRIO_CLASS by mlx5 PMDs
> for class driver initialization.
> 
> Signed-off-by: Parav Pandit <parav@mellanox.com>
> Acked-by: Matan Azrad <matan@mellanox.com>
> ---
> Changelog:
> v2->v3:
>  - new patch
> ---
>  lib/librte_eal/include/rte_common.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/librte_eal/include/rte_common.h b/lib/librte_eal/include/rte_common.h
> index 8f487a563..522afe58e 100644
> --- a/lib/librte_eal/include/rte_common.h
> +++ b/lib/librte_eal/include/rte_common.h
> @@ -135,6 +135,7 @@ typedef uint16_t unaligned_uint16_t;
>  
>  #define RTE_PRIORITY_LOG 101
>  #define RTE_PRIORITY_BUS 110
> +#define RTE_PRIORITY_COMMON 119
>  #define RTE_PRIORITY_CLASS 120
>  #define RTE_PRIORITY_LAST 65535
>  
> 

I guess the name "common" selected because of the intention to use it by the
common piece of the driver, but only from eal perspective the name
"PRIORITY_COMMON" looks so vague, it doesn't describe any purpose.

Also the value doesn't leave any gap between the class priority, what else can
be needed in the future in between, right?


@Thomas, @David, I am reluctant to get this eal change through the next-net, can
you please review/ack it first?

Thanks,
ferruh
  

20/07/2020 18:21, Ferruh Yigit:
> On 7/17/2020 2:49 PM, Parav Pandit wrote:
> > Currently mlx5_common uses CLASS priority to initialize
> > common code before initializing the PMD.
> > However mlx5_common is not really a class, it is the pre-initialization
> > code needed for the PMDs.
> > 
> > In subsequent patch a needed initialization sequence is:
> > (a) Initialize bus (say pci)
> > (b) Initialize common code of a driver (mlx5_common)
> > (c) Register mlx5 class PMDs (mlx5 net, mlx5 vdpa)
> > Information registered by these PMDs is used by mlx5_bus_pci PMD.
> > This mlx5 class PMDs should not confused with rte_class.
> > (d) Register mlx5 PCI bus PMD
> > 
> > Hence, introduce a new RTE priority level RTE_PRIO_COMMON which
> > can be used for common initialization and RTE_PRIO_CLASS by mlx5 PMDs
> > for class driver initialization.
> > 
> > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > Acked-by: Matan Azrad <matan@mellanox.com>
> > ---
> > Changelog:
> > v2->v3:
> >  - new patch
> > ---
> >  lib/librte_eal/include/rte_common.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/lib/librte_eal/include/rte_common.h b/lib/librte_eal/include/rte_common.h
> > index 8f487a563..522afe58e 100644
> > --- a/lib/librte_eal/include/rte_common.h
> > +++ b/lib/librte_eal/include/rte_common.h
> > @@ -135,6 +135,7 @@ typedef uint16_t unaligned_uint16_t;
> >  
> >  #define RTE_PRIORITY_LOG 101
> >  #define RTE_PRIORITY_BUS 110
> > +#define RTE_PRIORITY_COMMON 119
> >  #define RTE_PRIORITY_CLASS 120
> >  #define RTE_PRIORITY_LAST 65535
> >  
> > 
> 
> I guess the name "common" selected because of the intention to use it by the
> common piece of the driver, but only from eal perspective the name
> "PRIORITY_COMMON" looks so vague, it doesn't describe any purpose.

You're right.

> Also the value doesn't leave any gap between the class priority, what else can
> be needed in the future in between, right?

And we can imagine a bus requiring a common lib
to be initialized before.

> @Thomas, @David, I am reluctant to get this eal change through the next-net, can
> you please review/ack it first?

What about skipping this patch and using "RTE_PRIORITY_CLASS - 1"
in the code?
  

On 7/20/2020 5:48 PM, Thomas Monjalon wrote:
> 20/07/2020 18:21, Ferruh Yigit:
>> On 7/17/2020 2:49 PM, Parav Pandit wrote:
>>> Currently mlx5_common uses CLASS priority to initialize
>>> common code before initializing the PMD.
>>> However mlx5_common is not really a class, it is the pre-initialization
>>> code needed for the PMDs.
>>>
>>> In subsequent patch a needed initialization sequence is:
>>> (a) Initialize bus (say pci)
>>> (b) Initialize common code of a driver (mlx5_common)
>>> (c) Register mlx5 class PMDs (mlx5 net, mlx5 vdpa)
>>> Information registered by these PMDs is used by mlx5_bus_pci PMD.
>>> This mlx5 class PMDs should not confused with rte_class.
>>> (d) Register mlx5 PCI bus PMD
>>>
>>> Hence, introduce a new RTE priority level RTE_PRIO_COMMON which
>>> can be used for common initialization and RTE_PRIO_CLASS by mlx5 PMDs
>>> for class driver initialization.
>>>
>>> Signed-off-by: Parav Pandit <parav@mellanox.com>
>>> Acked-by: Matan Azrad <matan@mellanox.com>
>>> ---
>>> Changelog:
>>> v2->v3:
>>>  - new patch
>>> ---
>>>  lib/librte_eal/include/rte_common.h | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/lib/librte_eal/include/rte_common.h b/lib/librte_eal/include/rte_common.h
>>> index 8f487a563..522afe58e 100644
>>> --- a/lib/librte_eal/include/rte_common.h
>>> +++ b/lib/librte_eal/include/rte_common.h
>>> @@ -135,6 +135,7 @@ typedef uint16_t unaligned_uint16_t;
>>>  
>>>  #define RTE_PRIORITY_LOG 101
>>>  #define RTE_PRIORITY_BUS 110
>>> +#define RTE_PRIORITY_COMMON 119
>>>  #define RTE_PRIORITY_CLASS 120
>>>  #define RTE_PRIORITY_LAST 65535
>>>  
>>>
>>
>> I guess the name "common" selected because of the intention to use it by the
>> common piece of the driver, but only from eal perspective the name
>> "PRIORITY_COMMON" looks so vague, it doesn't describe any purpose.
> 
> You're right.
> 
>> Also the value doesn't leave any gap between the class priority, what else can
>> be needed in the future in between, right?
> 
> And we can imagine a bus requiring a common lib
> to be initialized before.
> 
>> @Thomas, @David, I am reluctant to get this eal change through the next-net, can
>> you please review/ack it first?
> 
> What about skipping this patch and using "RTE_PRIORITY_CLASS - 1"
> in the code?
> 

For now I think it is OK, in the future if more priority dependency involved we
can define the macro.
  

Hi

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> On 7/20/2020 5:48 PM, Thomas Monjalon wrote:
> > 20/07/2020 18:21, Ferruh Yigit:
> >> On 7/17/2020 2:49 PM, Parav Pandit wrote:
> >>> Currently mlx5_common uses CLASS priority to initialize
> >>> common code before initializing the PMD.
> >>> However mlx5_common is not really a class, it is the pre-initialization
> >>> code needed for the PMDs.
> >>>
> >>> In subsequent patch a needed initialization sequence is:
> >>> (a) Initialize bus (say pci)
> >>> (b) Initialize common code of a driver (mlx5_common)
> >>> (c) Register mlx5 class PMDs (mlx5 net, mlx5 vdpa)
> >>> Information registered by these PMDs is used by mlx5_bus_pci PMD.
> >>> This mlx5 class PMDs should not confused with rte_class.
> >>> (d) Register mlx5 PCI bus PMD
> >>>
> >>> Hence, introduce a new RTE priority level RTE_PRIO_COMMON which
> >>> can be used for common initialization and RTE_PRIO_CLASS by mlx5 PMDs
> >>> for class driver initialization.
> >>>
> >>> Signed-off-by: Parav Pandit <parav@mellanox.com>
> >>> Acked-by: Matan Azrad <matan@mellanox.com>
> >>> ---
> >>> Changelog:
> >>> v2->v3:
> >>>  - new patch
> >>> ---
> >>>  lib/librte_eal/include/rte_common.h | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/lib/librte_eal/include/rte_common.h
> b/lib/librte_eal/include/rte_common.h
> >>> index 8f487a563..522afe58e 100644
> >>> --- a/lib/librte_eal/include/rte_common.h
> >>> +++ b/lib/librte_eal/include/rte_common.h
> >>> @@ -135,6 +135,7 @@ typedef uint16_t unaligned_uint16_t;
> >>>
> >>>  #define RTE_PRIORITY_LOG 101
> >>>  #define RTE_PRIORITY_BUS 110
> >>> +#define RTE_PRIORITY_COMMON 119
> >>>  #define RTE_PRIORITY_CLASS 120
> >>>  #define RTE_PRIORITY_LAST 65535
> >>>
> >>>
> >>
> >> I guess the name "common" selected because of the intention to use it by
> the
> >> common piece of the driver, but only from eal perspective the name
> >> "PRIORITY_COMMON" looks so vague, it doesn't describe any purpose.
> >
> > You're right.
> >
> >> Also the value doesn't leave any gap between the class priority, what else
> can
> >> be needed in the future in between, right?
> >
> > And we can imagine a bus requiring a common lib
> > to be initialized before.
> >
> >> @Thomas, @David, I am reluctant to get this eal change through the next-
> net, can
> >> you please review/ack it first?
> >
> > What about skipping this patch and using "RTE_PRIORITY_CLASS - 1"
> > in the code?
> >
> 
> For now I think it is OK, in the future if more priority dependency involved we
> can define the macro.
> 
I'm concerned what if someone else will add priority there may be conflict and.
Also using -1 means that no one knows that there is use in such priority.
What about setting the value to 115?
  

On 7/20/2020 6:26 PM, Ori Kam wrote:
> Hi
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> On 7/20/2020 5:48 PM, Thomas Monjalon wrote:
>>> 20/07/2020 18:21, Ferruh Yigit:
>>>> On 7/17/2020 2:49 PM, Parav Pandit wrote:
>>>>> Currently mlx5_common uses CLASS priority to initialize
>>>>> common code before initializing the PMD.
>>>>> However mlx5_common is not really a class, it is the pre-initialization
>>>>> code needed for the PMDs.
>>>>>
>>>>> In subsequent patch a needed initialization sequence is:
>>>>> (a) Initialize bus (say pci)
>>>>> (b) Initialize common code of a driver (mlx5_common)
>>>>> (c) Register mlx5 class PMDs (mlx5 net, mlx5 vdpa)
>>>>> Information registered by these PMDs is used by mlx5_bus_pci PMD.
>>>>> This mlx5 class PMDs should not confused with rte_class.
>>>>> (d) Register mlx5 PCI bus PMD
>>>>>
>>>>> Hence, introduce a new RTE priority level RTE_PRIO_COMMON which
>>>>> can be used for common initialization and RTE_PRIO_CLASS by mlx5 PMDs
>>>>> for class driver initialization.
>>>>>
>>>>> Signed-off-by: Parav Pandit <parav@mellanox.com>
>>>>> Acked-by: Matan Azrad <matan@mellanox.com>
>>>>> ---
>>>>> Changelog:
>>>>> v2->v3:
>>>>>  - new patch
>>>>> ---
>>>>>  lib/librte_eal/include/rte_common.h | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/lib/librte_eal/include/rte_common.h
>> b/lib/librte_eal/include/rte_common.h
>>>>> index 8f487a563..522afe58e 100644
>>>>> --- a/lib/librte_eal/include/rte_common.h
>>>>> +++ b/lib/librte_eal/include/rte_common.h
>>>>> @@ -135,6 +135,7 @@ typedef uint16_t unaligned_uint16_t;
>>>>>
>>>>>  #define RTE_PRIORITY_LOG 101
>>>>>  #define RTE_PRIORITY_BUS 110
>>>>> +#define RTE_PRIORITY_COMMON 119
>>>>>  #define RTE_PRIORITY_CLASS 120
>>>>>  #define RTE_PRIORITY_LAST 65535
>>>>>
>>>>>
>>>>
>>>> I guess the name "common" selected because of the intention to use it by
>> the
>>>> common piece of the driver, but only from eal perspective the name
>>>> "PRIORITY_COMMON" looks so vague, it doesn't describe any purpose.
>>>
>>> You're right.
>>>
>>>> Also the value doesn't leave any gap between the class priority, what else
>> can
>>>> be needed in the future in between, right?
>>>
>>> And we can imagine a bus requiring a common lib
>>> to be initialized before.
>>>
>>>> @Thomas, @David, I am reluctant to get this eal change through the next-
>> net, can
>>>> you please review/ack it first?
>>>
>>> What about skipping this patch and using "RTE_PRIORITY_CLASS - 1"
>>> in the code?
>>>
>>
>> For now I think it is OK, in the future if more priority dependency involved we
>> can define the macro.
>>
> I'm concerned what if someone else will add priority there may be conflict and.
> Also using -1 means that no one knows that there is use in such priority.

Is the new constructor priority level a common need, or just specific to this
"mlx5 pci" bus usage?
I understand the need of new constructor priority level, but it seems it is not
clear enough to define it as a generic level, that is why I believe this can be
local to PMD for now, otherwise your concerns looks valid.

Also I am thinking if the multi class support can be done as generic bus
feature, instead of defining a new PMD specific bus for it, but I am aware it is
too late for this question.

> What about setting the value to 115?
>
  

On Mon, Jul 20, 2020 at 6:48 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 20/07/2020 18:21, Ferruh Yigit:
> > On 7/17/2020 2:49 PM, Parav Pandit wrote:
> > > Currently mlx5_common uses CLASS priority to initialize
> > > common code before initializing the PMD.
> > > However mlx5_common is not really a class, it is the pre-initialization
> > > code needed for the PMDs.
> > >
> > > In subsequent patch a needed initialization sequence is:
> > > (a) Initialize bus (say pci)
> > > (b) Initialize common code of a driver (mlx5_common)
> > > (c) Register mlx5 class PMDs (mlx5 net, mlx5 vdpa)
> > > Information registered by these PMDs is used by mlx5_bus_pci PMD.
> > > This mlx5 class PMDs should not confused with rte_class.
> > > (d) Register mlx5 PCI bus PMD
> > >
> > > Hence, introduce a new RTE priority level RTE_PRIO_COMMON which
> > > can be used for common initialization and RTE_PRIO_CLASS by mlx5 PMDs
> > > for class driver initialization.
> > >
> > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > Acked-by: Matan Azrad <matan@mellanox.com>
> > > ---
> > > Changelog:
> > > v2->v3:
> > >  - new patch
> > > ---
> > >  lib/librte_eal/include/rte_common.h | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/lib/librte_eal/include/rte_common.h b/lib/librte_eal/include/rte_common.h
> > > index 8f487a563..522afe58e 100644
> > > --- a/lib/librte_eal/include/rte_common.h
> > > +++ b/lib/librte_eal/include/rte_common.h
> > > @@ -135,6 +135,7 @@ typedef uint16_t unaligned_uint16_t;
> > >
> > >  #define RTE_PRIORITY_LOG 101
> > >  #define RTE_PRIORITY_BUS 110
> > > +#define RTE_PRIORITY_COMMON 119
> > >  #define RTE_PRIORITY_CLASS 120
> > >  #define RTE_PRIORITY_LAST 65535
> > >
> > >
> >
> > I guess the name "common" selected because of the intention to use it by the
> > common piece of the driver, but only from eal perspective the name
> > "PRIORITY_COMMON" looks so vague, it doesn't describe any purpose.
>
> You're right.
>
> > Also the value doesn't leave any gap between the class priority, what else can
> > be needed in the future in between, right?
>
> And we can imagine a bus requiring a common lib
> to be initialized before.
>
> > @Thomas, @David, I am reluctant to get this eal change through the next-net, can
> > you please review/ack it first?
>
> What about skipping this patch and using "RTE_PRIORITY_CLASS - 1"
> in the code?

net and vdpa code expect the common code being initialised.
It is a dependency internal to mlx5 drivers, I see nothing generic.

The common driver can provide a init function called by net and vdpa
drivers in constructor context (not that I like calling complex init,
but it should be equivalent to current state).
It expresses a clear dependency and there is no constructor semantic
to invent, nor runtime breakage possible because someone tweaks the
priority order in the future.

There is also some hack about a haswell/broadwell detection in LOG
priority that makes no sense.

I compile-tested following:

$ git diff next-net-mlx/master -- lib/librte_eal/ drivers/*/mlx5/
diff --git a/drivers/common/mlx5/mlx5_common.c
b/drivers/common/mlx5/mlx5_common.c
index 79cd5ba344..3fb9a8ab89 100644
--- a/drivers/common/mlx5/mlx5_common.c
+++ b/drivers/common/mlx5/mlx5_common.c
@@ -49,14 +49,6 @@ RTE_INIT_PRIO(mlx5_log_init, LOG)
                rte_log_set_level(mlx5_common_logtype, RTE_LOG_NOTICE);
 }

-/**
- * Initialization routine for run-time dependency on glue library.
- */
-RTE_INIT_PRIO(mlx5_glue_init, COMMON)
-{
-       mlx5_glue_constructor();
-}
-
 /**
  * This function is responsible of initializing the variable
  *  haswell_broadwell_cpu by checking if the cpu is intel
@@ -67,7 +59,7 @@ RTE_INIT_PRIO(mlx5_glue_init, COMMON)
  *  if the cpu is haswell or broadwell the variable will be set to 1
  *  otherwise it will be 0.
  */
-RTE_INIT_PRIO(mlx5_is_haswell_broadwell_cpu, LOG)
+static void mlx5_is_haswell_broadwell_cpu(void)
 {
 #ifdef RTE_ARCH_X86_64
        unsigned int broadwell_models[4] = {0x3d, 0x47, 0x4F, 0x56};
@@ -114,6 +106,21 @@ RTE_INIT_PRIO(mlx5_is_haswell_broadwell_cpu, LOG)
        haswell_broadwell_cpu = 0;
 }

+/**
+ * Initialization routine for run-time dependency on glue library.
+ */
+void mlx5_common_init(void)
+{
+       static bool init_once = false;
+
+       if (init_once)
+               return;
+
+       mlx5_glue_constructor();
+       mlx5_is_haswell_broadwell_cpu();
+       init_once = true;
+}
+
 /**
  * Allocate page of door-bells and register it using DevX API.
  *
diff --git a/drivers/common/mlx5/mlx5_common.h
b/drivers/common/mlx5/mlx5_common.h
index 5b9b7bd5a9..30961bc8cc 100644
--- a/drivers/common/mlx5/mlx5_common.h
+++ b/drivers/common/mlx5/mlx5_common.h
@@ -254,6 +254,10 @@ int64_t mlx5_get_dbr(void *ctx,  struct
mlx5_dbr_page_list *head,
 __rte_internal
 int32_t mlx5_release_dbr(struct mlx5_dbr_page_list *head, uint32_t umem_id,
                         uint64_t offset);
+
+__rte_internal
+void mlx5_common_init(void);
+
 extern uint8_t haswell_broadwell_cpu;

 #endif /* RTE_PMD_MLX5_COMMON_H_ */
diff --git a/drivers/common/mlx5/rte_common_mlx5_version.map
b/drivers/common/mlx5/rte_common_mlx5_version.map
index fe62fa2b2f..039d221333 100644
--- a/drivers/common/mlx5/rte_common_mlx5_version.map
+++ b/drivers/common/mlx5/rte_common_mlx5_version.map
@@ -1,6 +1,7 @@
 INTERNAL {
        global:

+       mlx5_common_init;
        mlx5_common_verbs_reg_mr;
        mlx5_common_verbs_dereg_mr;

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 846398dd3d..d0e9345a55 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -1989,6 +1989,8 @@ RTE_LOG_REGISTER(mlx5_logtype, pmd.net.mlx5, NOTICE)
  */
 RTE_INIT_PRIO(rte_mlx5_pmd_init, CLASS)
 {
+       mlx5_common_init();
+
        /* Build the static tables for Verbs conversion. */
        mlx5_set_ptype_table();
        mlx5_set_cksum_table();
diff --git a/drivers/vdpa/mlx5/mlx5_vdpa.c b/drivers/vdpa/mlx5/mlx5_vdpa.c
index 70692ea1d2..9dc3e8fa56 100644
--- a/drivers/vdpa/mlx5/mlx5_vdpa.c
+++ b/drivers/vdpa/mlx5/mlx5_vdpa.c
@@ -844,6 +844,8 @@ RTE_LOG_REGISTER(mlx5_vdpa_logtype, pmd.vdpa.mlx5, NOTICE)
  */
 RTE_INIT_PRIO(rte_mlx5_vdpa_init, CLASS)
 {
+       mlx5_common_init();
+
        if (mlx5_glue)
                rte_mlx5_pci_driver_register(&mlx5_vdpa_driver);
 }
diff --git a/lib/librte_eal/include/rte_common.h
b/lib/librte_eal/include/rte_common.h
index 522afe58ed..8f487a563d 100644
--- a/lib/librte_eal/include/rte_common.h
+++ b/lib/librte_eal/include/rte_common.h
@@ -135,7 +135,6 @@ typedef uint16_t unaligned_uint16_t;

 #define RTE_PRIORITY_LOG 101
 #define RTE_PRIORITY_BUS 110
-#define RTE_PRIORITY_COMMON 119
 #define RTE_PRIORITY_CLASS 120
 #define RTE_PRIORITY_LAST 65535
  

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> On 7/20/2020 6:26 PM, Ori Kam wrote:
> > Hi
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >> On 7/20/2020 5:48 PM, Thomas Monjalon wrote:
> >>> 20/07/2020 18:21, Ferruh Yigit:
> >>>> On 7/17/2020 2:49 PM, Parav Pandit wrote:
> >>>>> Currently mlx5_common uses CLASS priority to initialize
> >>>>> common code before initializing the PMD.
> >>>>> However mlx5_common is not really a class, it is the pre-initialization
> >>>>> code needed for the PMDs.
> >>>>>
> >>>>> In subsequent patch a needed initialization sequence is:
> >>>>> (a) Initialize bus (say pci)
> >>>>> (b) Initialize common code of a driver (mlx5_common)
> >>>>> (c) Register mlx5 class PMDs (mlx5 net, mlx5 vdpa)
> >>>>> Information registered by these PMDs is used by mlx5_bus_pci PMD.
> >>>>> This mlx5 class PMDs should not confused with rte_class.
> >>>>> (d) Register mlx5 PCI bus PMD
> >>>>>
> >>>>> Hence, introduce a new RTE priority level RTE_PRIO_COMMON which
> >>>>> can be used for common initialization and RTE_PRIO_CLASS by mlx5
> PMDs
> >>>>> for class driver initialization.
> >>>>>
> >>>>> Signed-off-by: Parav Pandit <parav@mellanox.com>
> >>>>> Acked-by: Matan Azrad <matan@mellanox.com>
> >>>>> ---
> >>>>> Changelog:
> >>>>> v2->v3:
> >>>>>  - new patch
> >>>>> ---
> >>>>>  lib/librte_eal/include/rte_common.h | 1 +
> >>>>>  1 file changed, 1 insertion(+)
> >>>>>
> >>>>> diff --git a/lib/librte_eal/include/rte_common.h
> >> b/lib/librte_eal/include/rte_common.h
> >>>>> index 8f487a563..522afe58e 100644
> >>>>> --- a/lib/librte_eal/include/rte_common.h
> >>>>> +++ b/lib/librte_eal/include/rte_common.h
> >>>>> @@ -135,6 +135,7 @@ typedef uint16_t unaligned_uint16_t;
> >>>>>
> >>>>>  #define RTE_PRIORITY_LOG 101
> >>>>>  #define RTE_PRIORITY_BUS 110
> >>>>> +#define RTE_PRIORITY_COMMON 119
> >>>>>  #define RTE_PRIORITY_CLASS 120
> >>>>>  #define RTE_PRIORITY_LAST 65535
> >>>>>
> >>>>>
> >>>>
> >>>> I guess the name "common" selected because of the intention to use it by
> >> the
> >>>> common piece of the driver, but only from eal perspective the name
> >>>> "PRIORITY_COMMON" looks so vague, it doesn't describe any purpose.
> >>>
> >>> You're right.
> >>>
> >>>> Also the value doesn't leave any gap between the class priority, what else
> >> can
> >>>> be needed in the future in between, right?
> >>>
> >>> And we can imagine a bus requiring a common lib
> >>> to be initialized before.
> >>>
> >>>> @Thomas, @David, I am reluctant to get this eal change through the
> next-
> >> net, can
> >>>> you please review/ack it first?
> >>>
> >>> What about skipping this patch and using "RTE_PRIORITY_CLASS - 1"
> >>> in the code?
> >>>
> >>
> >> For now I think it is OK, in the future if more priority dependency involved
> we
> >> can define the macro.
> >>
> > I'm concerned what if someone else will add priority there may be conflict
> and.
> > Also using -1 means that no one knows that there is use in such priority.
> 
> Is the new constructor priority level a common need, or just specific to this
> "mlx5 pci" bus usage?

I think it can be used by any other vendor that needs a bus for his own PMDs.

> I understand the need of new constructor priority level, but it seems it is not
> clear enough to define it as a generic level, that is why I believe this can be
> local to PMD for now, otherwise your concerns looks valid.
> 
If I understand it can be local to the bus, it is not exposed to the PMD.
But like I said above this may be used by other vendors that wish to combine
some of their devices.

> Also I am thinking if the multi class support can be done as generic bus
> feature, instead of defining a new PMD specific bus for it, but I am aware it is
> too late for this question.
> 
> > What about setting the value to 115?
> >
  

Hi David,

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> 
> On Mon, Jul 20, 2020 at 6:48 PM Thomas Monjalon <thomas@monjalon.net>
> wrote:
> >
> > 20/07/2020 18:21, Ferruh Yigit:
> > > On 7/17/2020 2:49 PM, Parav Pandit wrote:
> > > > Currently mlx5_common uses CLASS priority to initialize
> > > > common code before initializing the PMD.
> > > > However mlx5_common is not really a class, it is the pre-initialization
> > > > code needed for the PMDs.
> > > >
> > > > In subsequent patch a needed initialization sequence is:
> > > > (a) Initialize bus (say pci)
> > > > (b) Initialize common code of a driver (mlx5_common)
> > > > (c) Register mlx5 class PMDs (mlx5 net, mlx5 vdpa)
> > > > Information registered by these PMDs is used by mlx5_bus_pci PMD.
> > > > This mlx5 class PMDs should not confused with rte_class.
> > > > (d) Register mlx5 PCI bus PMD
> > > >
> > > > Hence, introduce a new RTE priority level RTE_PRIO_COMMON which
> > > > can be used for common initialization and RTE_PRIO_CLASS by mlx5
> PMDs
> > > > for class driver initialization.
> > > >
> > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > > Acked-by: Matan Azrad <matan@mellanox.com>
> > > > ---
> > > > Changelog:
> > > > v2->v3:
> > > >  - new patch
> > > > ---
> > > >  lib/librte_eal/include/rte_common.h | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/lib/librte_eal/include/rte_common.h
> b/lib/librte_eal/include/rte_common.h
> > > > index 8f487a563..522afe58e 100644
> > > > --- a/lib/librte_eal/include/rte_common.h
> > > > +++ b/lib/librte_eal/include/rte_common.h
> > > > @@ -135,6 +135,7 @@ typedef uint16_t unaligned_uint16_t;
> > > >
> > > >  #define RTE_PRIORITY_LOG 101
> > > >  #define RTE_PRIORITY_BUS 110
> > > > +#define RTE_PRIORITY_COMMON 119
> > > >  #define RTE_PRIORITY_CLASS 120
> > > >  #define RTE_PRIORITY_LAST 65535
> > > >
> > > >
> > >
> > > I guess the name "common" selected because of the intention to use it by
> the
> > > common piece of the driver, but only from eal perspective the name
> > > "PRIORITY_COMMON" looks so vague, it doesn't describe any purpose.
> >
> > You're right.
> >
> > > Also the value doesn't leave any gap between the class priority, what else
> can
> > > be needed in the future in between, right?
> >
> > And we can imagine a bus requiring a common lib
> > to be initialized before.
> >
> > > @Thomas, @David, I am reluctant to get this eal change through the next-
> net, can
> > > you please review/ack it first?
> >
> > What about skipping this patch and using "RTE_PRIORITY_CLASS - 1"
> > in the code?
> 
> net and vdpa code expect the common code being initialised.
> It is a dependency internal to mlx5 drivers, I see nothing generic.
> 
First the idea was to declare a new bus not a PMD.
The issue is not from common code but from loading more than one
device on the same PCI.
So the logic is Mellanox PMD are registering to the new bus, the new bus
register to the PCI one.
So the new bus must have middle priority between the PMDs (class)  and the PCI (bus) bus.

May be a better name should have been:
RTE_PRIORITY_ MANUFACTORE _BUS  

> The common driver can provide a init function called by net and vdpa
> drivers in constructor context (not that I like calling complex init,
> but it should be equivalent to current state).
> It expresses a clear dependency and there is no constructor semantic
> to invent, nor runtime breakage possible because someone tweaks the
> priority order in the future.
> 
> There is also some hack about a haswell/broadwell detection in LOG
> priority that makes no sense.
> 
> I compile-tested following:
> 
> $ git diff next-net-mlx/master -- lib/librte_eal/ drivers/*/mlx5/
> diff --git a/drivers/common/mlx5/mlx5_common.c
> b/drivers/common/mlx5/mlx5_common.c
> index 79cd5ba344..3fb9a8ab89 100644
> --- a/drivers/common/mlx5/mlx5_common.c
> +++ b/drivers/common/mlx5/mlx5_common.c
> @@ -49,14 +49,6 @@ RTE_INIT_PRIO(mlx5_log_init, LOG)
>                 rte_log_set_level(mlx5_common_logtype, RTE_LOG_NOTICE);
>  }
> 
> -/**
> - * Initialization routine for run-time dependency on glue library.
> - */
> -RTE_INIT_PRIO(mlx5_glue_init, COMMON)
> -{
> -       mlx5_glue_constructor();
> -}
> -
>  /**
>   * This function is responsible of initializing the variable
>   *  haswell_broadwell_cpu by checking if the cpu is intel
> @@ -67,7 +59,7 @@ RTE_INIT_PRIO(mlx5_glue_init, COMMON)
>   *  if the cpu is haswell or broadwell the variable will be set to 1
>   *  otherwise it will be 0.
>   */
> -RTE_INIT_PRIO(mlx5_is_haswell_broadwell_cpu, LOG)
> +static void mlx5_is_haswell_broadwell_cpu(void)
>  {
>  #ifdef RTE_ARCH_X86_64
>         unsigned int broadwell_models[4] = {0x3d, 0x47, 0x4F, 0x56};
> @@ -114,6 +106,21 @@ RTE_INIT_PRIO(mlx5_is_haswell_broadwell_cpu,
> LOG)
>         haswell_broadwell_cpu = 0;
>  }
> 
> +/**
> + * Initialization routine for run-time dependency on glue library.
> + */
> +void mlx5_common_init(void)
> +{
> +       static bool init_once = false;
> +
> +       if (init_once)
> +               return;
> +
> +       mlx5_glue_constructor();
> +       mlx5_is_haswell_broadwell_cpu();
> +       init_once = true;
> +}
> +
>  /**
>   * Allocate page of door-bells and register it using DevX API.
>   *
> diff --git a/drivers/common/mlx5/mlx5_common.h
> b/drivers/common/mlx5/mlx5_common.h
> index 5b9b7bd5a9..30961bc8cc 100644
> --- a/drivers/common/mlx5/mlx5_common.h
> +++ b/drivers/common/mlx5/mlx5_common.h
> @@ -254,6 +254,10 @@ int64_t mlx5_get_dbr(void *ctx,  struct
> mlx5_dbr_page_list *head,
>  __rte_internal
>  int32_t mlx5_release_dbr(struct mlx5_dbr_page_list *head, uint32_t
> umem_id,
>                          uint64_t offset);
> +
> +__rte_internal
> +void mlx5_common_init(void);
> +
>  extern uint8_t haswell_broadwell_cpu;
> 
>  #endif /* RTE_PMD_MLX5_COMMON_H_ */
> diff --git a/drivers/common/mlx5/rte_common_mlx5_version.map
> b/drivers/common/mlx5/rte_common_mlx5_version.map
> index fe62fa2b2f..039d221333 100644
> --- a/drivers/common/mlx5/rte_common_mlx5_version.map
> +++ b/drivers/common/mlx5/rte_common_mlx5_version.map
> @@ -1,6 +1,7 @@
>  INTERNAL {
>         global:
> 
> +       mlx5_common_init;
>         mlx5_common_verbs_reg_mr;
>         mlx5_common_verbs_dereg_mr;
> 
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index 846398dd3d..d0e9345a55 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -1989,6 +1989,8 @@ RTE_LOG_REGISTER(mlx5_logtype, pmd.net.mlx5,
> NOTICE)
>   */
>  RTE_INIT_PRIO(rte_mlx5_pmd_init, CLASS)
>  {
> +       mlx5_common_init();
> +
>         /* Build the static tables for Verbs conversion. */
>         mlx5_set_ptype_table();
>         mlx5_set_cksum_table();
> diff --git a/drivers/vdpa/mlx5/mlx5_vdpa.c b/drivers/vdpa/mlx5/mlx5_vdpa.c
> index 70692ea1d2..9dc3e8fa56 100644
> --- a/drivers/vdpa/mlx5/mlx5_vdpa.c
> +++ b/drivers/vdpa/mlx5/mlx5_vdpa.c
> @@ -844,6 +844,8 @@ RTE_LOG_REGISTER(mlx5_vdpa_logtype,
> pmd.vdpa.mlx5, NOTICE)
>   */
>  RTE_INIT_PRIO(rte_mlx5_vdpa_init, CLASS)
>  {
> +       mlx5_common_init();
> +
>         if (mlx5_glue)
>                 rte_mlx5_pci_driver_register(&mlx5_vdpa_driver);
>  }
> diff --git a/lib/librte_eal/include/rte_common.h
> b/lib/librte_eal/include/rte_common.h
> index 522afe58ed..8f487a563d 100644
> --- a/lib/librte_eal/include/rte_common.h
> +++ b/lib/librte_eal/include/rte_common.h
> @@ -135,7 +135,6 @@ typedef uint16_t unaligned_uint16_t;
> 
>  #define RTE_PRIORITY_LOG 101
>  #define RTE_PRIORITY_BUS 110
> -#define RTE_PRIORITY_COMMON 119
>  #define RTE_PRIORITY_CLASS 120
>  #define RTE_PRIORITY_LAST 65535
> 
> 
> --
> David Marchand
  

On Mon, Jul 20, 2020 at 9:30 PM Ori Kam <orika@mellanox.com> wrote:
> > net and vdpa code expect the common code being initialised.
> > It is a dependency internal to mlx5 drivers, I see nothing generic.
> >
> First the idea was to declare a new bus not a PMD.
> The issue is not from common code but from loading more than one
> device on the same PCI.
> So the logic is Mellanox PMD are registering to the new bus, the new bus
> register to the PCI one.

- Is there a problem with the snippet I sent yesterday that removes
the COMMON priority?


- Currently in the main branch, the net/mlx5 driver is a pci driver
registered to the pci bus driver.
This registration happens in a constructor which puts the net/mlx5 pci
driver struct (with a populated id map) into the pci driver list.
Later (out of constructor context), a probe on the pci bus uses this
driver list.


A new pci driver is introduced by Mellanox to solve resources
dispatching between mlx5 drivers.
I'd rather not call this new pci driver a bus, it is not registered as
such (no rte_bus object) => no reason its constructor should be called
from the RTE_PRIO_BUS priority.

This driver not being a bus, I would not put it in drivers/bus/,
rather in the common code.
If there is a build dependency issue with the pci bus driver, you can
probably use the trick from qat
(https://git.dpdk.org/dpdk/tree/drivers/Makefile#n17) and write an
explicit dependency in drivers/Makefile.
meson should be just fine.


net/mlx5 and vdpa/mlx5 are not classes => no reason their constructor
should be called from the RTE_PRIO_CLASS priority.
The only classes we have are ethdev and vdpa.


The new pci driver simply serves as a proxy for mlx5 net and vdpa drivers.
From what I see, the new pci driver constructs its pci device id
map/drv_flags when registering to the pci bus driver based on what
mlx5 net and vdpa drivers registered before.
This is where a dependency was created.

You can move the id_table/drv_flags construction from
RTE_INIT(mlx5_bus_pci) to rte_mlx5_pci_register() itself.
This way, net/mlx5 and vdpa/mlx5 will pass their id_map to the mlx5
pci driver whether it is registered to the pci bus or not yet.
  

Hi David,

On 7/21/2020 3:04 PM, David Marchand wrote:
> On Mon, Jul 20, 2020 at 9:30 PM Ori Kam <orika@mellanox.com> wrote:
>>> net and vdpa code expect the common code being initialised.
>>> It is a dependency internal to mlx5 drivers, I see nothing generic.
>>>
>> First the idea was to declare a new bus not a PMD.
>> The issue is not from common code but from loading more than one
>> device on the same PCI.
>> So the logic is Mellanox PMD are registering to the new bus, the new bus
>> register to the PCI one.
> 
> - Is there a problem with the snippet I sent yesterday that removes
> the COMMON priority?
> 
> 
No. The problem as you describe below is in handling the inter
dependencies between mlx5_common, rte_pci and mlx5_{net/vdpa}.

mlx5_net/vdpa demands common to be loaded first.
mlx5_pci_bus expects mlx5_net_vdpa to register first.
More below.

> - Currently in the main branch, the net/mlx5 driver is a pci driver
> registered to the pci bus driver.
> This registration happens in a constructor which puts the net/mlx5 pci
> driver struct (with a populated id map) into the pci driver list.
> Later (out of constructor context), a probe on the pci bus uses this
> driver list.
> 
> 
> A new pci driver is introduced by Mellanox to solve resources
> dispatching between mlx5 drivers.
> I'd rather not call this new pci driver a bus, it is not registered as
> such (no rte_bus object) => no reason its constructor should be called
> from the RTE_PRIO_BUS priority.
> 
> This driver not being a bus, I would not put it in drivers/bus/,
> rather in the common code.
> If there is a build dependency issue with the pci bus driver, you can
> probably use the trick from qat
> (https://git.dpdk.org/dpdk/tree/drivers/Makefile#n17) and write an
> explicit dependency in drivers/Makefile.
> meson should be just fine.
> 
mlx5_common to depend on pci bus. (new Makefile change)
mlx5_net and mlx5_vdpa to depend on mlx5_common. (already there)

> 
> net/mlx5 and vdpa/mlx5 are not classes => no reason their constructor
> should be called from the RTE_PRIO_CLASS priority.
> The only classes we have are ethdev and vdpa.
> 
> 
> The new pci driver simply serves as a proxy for mlx5 net and vdpa drivers.
> From what I see, the new pci driver constructs its pci device id
> map/drv_flags when registering to the pci bus driver based on what
> mlx5 net and vdpa drivers registered before.
> This is where a dependency was created.
> 
> You can move the id_table/drv_flags construction from
> RTE_INIT(mlx5_bus_pci) to rte_mlx5_pci_register() itself.
ok. This is what were also discussing internally as well.

> This way, net/mlx5 and vdpa/mlx5 will pass their id_map to the mlx5
> pci driver whether it is registered to the pci bus or not yet.

And here pci_id_table will not be const *.
Is that ok?

Basically mlx5_pci_bus, mlx5_net and mlx5_vdpa will be called with same
priority RTE_PRIO_LAST.
driver id table and drv_flags dynamically updated as PMDs register.

Should we have another API after rte_pci_register() to indicate that
some field of the driver are updated, instead of just silently updating
it in a PMD?
  

On Tue, Jul 21, 2020 at 1:19 PM Parav Pandit <parav@mellanox.com> wrote:
> > This way, net/mlx5 and vdpa/mlx5 will pass their id_map to the mlx5
> > pci driver whether it is registered to the pci bus or not yet.
>
> And here pci_id_table will not be const *.
> Is that ok?

This is already the case in the current patch.
I see nothing wrong with it.
The pci code expects this pointer to be const and will it treat it as such.


On the other hand, you may remove the pci table exported by the common
driver as it is useless.
pmdinfogen only parses the .o and extracts the table symbol, so it
won't load other mlx drivers to generate the .pmd.c file.

I.e. with current patch,
$ ./usertools/dpdk-pmdinfo.py ./build/drivers/librte_bus_mlx5_pci.so
PMD NAME: mlx5_bus_pci


>
> Basically mlx5_pci_bus, mlx5_net and mlx5_vdpa will be called with same
> priority RTE_PRIO_LAST.
> driver id table and drv_flags dynamically updated as PMDs register.
>
> Should we have another API after rte_pci_register() to indicate that
> some field of the driver are updated, instead of just silently updating
> it in a PMD?

All of this happens in constructor context, this is a really early
stage used to resolve code dependencies.
No big init should happen there.

We can document this use of the existing API but I don't see which API to add.
  

21/07/2020 13:29, David Marchand:
> On Tue, Jul 21, 2020 at 1:19 PM Parav Pandit <parav@mellanox.com> wrote:
> > Basically mlx5_pci_bus, mlx5_net and mlx5_vdpa will be called with same
> > priority RTE_PRIO_LAST.
> > driver id table and drv_flags dynamically updated as PMDs register.
> >
> > Should we have another API after rte_pci_register() to indicate that
> > some field of the driver are updated, instead of just silently updating
> > it in a PMD?
> 
> All of this happens in constructor context, this is a really early
> stage used to resolve code dependencies.
> No big init should happen there.

In general, there is a misunderstanding of what should be done
in a constructor. I think we should document constructor usage properly.

For the record, init done in constructors should be small.
If the application is linked with DPDK but choose to not initialize it,
it should not have driver initialized (until it calls rte_eal_init).
Constructor init should be limited to adding a driver struct to a list.
  

> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, July 21, 2020 4:59 PM
> 
> On Tue, Jul 21, 2020 at 1:19 PM Parav Pandit <parav@mellanox.com> wrote:
> > > This way, net/mlx5 and vdpa/mlx5 will pass their id_map to the mlx5
> > > pci driver whether it is registered to the pci bus or not yet.
> >
> > And here pci_id_table will not be const *.
> > Is that ok?
> 
> This is already the case in the current patch.
> I see nothing wrong with it.
> The pci code expects this pointer to be const and will it treat it as such.
> 
o.k.

Gaetan, Ferruh, Thomas,
Can you please ack as well?

> 
> On the other hand, you may remove the pci table exported by the common
> driver as it is useless.
> pmdinfogen only parses the .o and extracts the table symbol, so it won't load
> other mlx drivers to generate the .pmd.c file.
> 
Ok.

> I.e. with current patch,
> $ ./usertools/dpdk-pmdinfo.py ./build/drivers/librte_bus_mlx5_pci.so
> PMD NAME: mlx5_bus_pci
> 
> 
> >
> > Basically mlx5_pci_bus, mlx5_net and mlx5_vdpa will be called with
> > same priority RTE_PRIO_LAST.
> > driver id table and drv_flags dynamically updated as PMDs register.
> >
> > Should we have another API after rte_pci_register() to indicate that
> > some field of the driver are updated, instead of just silently
> > updating it in a PMD?
> 
> All of this happens in constructor context, this is a really early stage used to
> resolve code dependencies.
> No big init should happen there.
> 
Right.
Only init() is to dynamically build the pci id table to remove duplicates, how its done today.
And it requires reallocating new table as part of mlx5_pci_bus_register() and updating new id table pointer in the driver structure.

> We can document this use of the existing API but I don't see which API to
> add.
> 
Alright.
Will wait for ack on pci id table from Ferruh, Gaetan and Thomas to post v8.
Thanks a lot David for the review.
  

21/07/2020 14:13, Parav Pandit:
> From: David Marchand <david.marchand@redhat.com>
> > On Tue, Jul 21, 2020 at 1:19 PM Parav Pandit <parav@mellanox.com> wrote:
> > > > This way, net/mlx5 and vdpa/mlx5 will pass their id_map to the mlx5
> > > > pci driver whether it is registered to the pci bus or not yet.
> > >
> > > And here pci_id_table will not be const *.
> > > Is that ok?
> > 
> > This is already the case in the current patch.
> > I see nothing wrong with it.
> > The pci code expects this pointer to be const and will it treat it as such.
> > 
> o.k.
> 
> Gaetan, Ferruh, Thomas,
> Can you please ack as well?

Yes of course it is OK updating the PCI table of the common layer
in runtime. The most important is to keep the fixed PCI table of the PMDs
the same as registered for pmdinfo usage.

> > On the other hand, you may remove the pci table exported by the common
> > driver as it is useless.
> > pmdinfogen only parses the .o and extracts the table symbol, so it won't load
> > other mlx drivers to generate the .pmd.c file.
> 
> Ok.

One more nit: please don't use "class" as variable name for drivers.
  

> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, July 21, 2020 5:57 PM
> 
> 21/07/2020 14:13, Parav Pandit:
> > From: David Marchand <david.marchand@redhat.com>
> > > On Tue, Jul 21, 2020 at 1:19 PM Parav Pandit <parav@mellanox.com>
> wrote:
> > > > > This way, net/mlx5 and vdpa/mlx5 will pass their id_map to the
> > > > > mlx5 pci driver whether it is registered to the pci bus or not yet.
> > > >
> > > > And here pci_id_table will not be const *.
> > > > Is that ok?
> > >
> > > This is already the case in the current patch.
> > > I see nothing wrong with it.
> > > The pci code expects this pointer to be const and will it treat it as such.
> > >
> > o.k.
> >
> > Gaetan, Ferruh, Thomas,
> > Can you please ack as well?
> 
> Yes of course it is OK updating the PCI table of the common layer in runtime.
> The most important is to keep the fixed PCI table of the PMDs the same as
> registered for pmdinfo usage.
o.k. pci id table registered with PCI bus will be build dynamically, similar to how its done in v7.
Instead of doing it in constructor of mlx5_pci_bus, it will be done inside rte_mlx5_pci_bus_register().
Right?

And since we don't register it as rte_bus, shall I name it as rte_mlx5_pci_driver_register()/unregister().
Right?

> 
> > > On the other hand, you may remove the pci table exported by the
> > > common driver as it is useless.
> > > pmdinfogen only parses the .o and extracts the table symbol, so it
> > > won't load other mlx drivers to generate the .pmd.c file.
> >
> > Ok.
> 
> One more nit: please don't use "class" as variable name for drivers.
Ok. I will address that.
  

21/07/2020 14:51, Parav Pandit:
> 
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Tuesday, July 21, 2020 5:57 PM
> > 
> > 21/07/2020 14:13, Parav Pandit:
> > > From: David Marchand <david.marchand@redhat.com>
> > > > On Tue, Jul 21, 2020 at 1:19 PM Parav Pandit <parav@mellanox.com>
> > wrote:
> > > > > > This way, net/mlx5 and vdpa/mlx5 will pass their id_map to the
> > > > > > mlx5 pci driver whether it is registered to the pci bus or not yet.
> > > > >
> > > > > And here pci_id_table will not be const *.
> > > > > Is that ok?
> > > >
> > > > This is already the case in the current patch.
> > > > I see nothing wrong with it.
> > > > The pci code expects this pointer to be const and will it treat it as such.
> > > >
> > > o.k.
> > >
> > > Gaetan, Ferruh, Thomas,
> > > Can you please ack as well?
> > 
> > Yes of course it is OK updating the PCI table of the common layer in runtime.
> > The most important is to keep the fixed PCI table of the PMDs the same as
> > registered for pmdinfo usage.
> 
> o.k. pci id table registered with PCI bus will be build dynamically, similar to how its done in v7.
> Instead of doing it in constructor of mlx5_pci_bus, it will be done inside rte_mlx5_pci_bus_register().
> Right?

Yes

> And since we don't register it as rte_bus, shall I name it as rte_mlx5_pci_driver_register()/unregister().
> Right?

Yes, if merging code in the common driver, no need to keep "bus" wording.