Re: [dpdk-dev] [RFC PATCH 6/6] bus/mlx5_pci: enable net and vDPA to use mlx5 PCI bus driver

[Parav Pandit <parav@mellanox.com>]

> From: Gaëtan Rivet <grive@u256.net>
> Sent: Tuesday, June 16, 2020 3:26 AM
> 
> On 10/06/20 17:17 +0000, Parav Pandit wrote:
> > Enable class driver to match with the mlx5 pci devices.
> > Migrate mlx5 net PMD and vdpa PMD to start using mlx5 common class
> > driver.
> >
> > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > ---
> >  drivers/bus/Makefile                |  3 ++
> >  drivers/bus/mlx5_pci/mlx5_pci_bus.c | 60
> +++++++++++++++++++++++++++++
> >  drivers/net/mlx5/Makefile           |  3 +-
> >  drivers/net/mlx5/linux/mlx5_os.c    |  1 -
> >  drivers/net/mlx5/linux/mlx5_os.h    |  3 --
> >  drivers/net/mlx5/meson.build        |  2 +-
> >  drivers/net/mlx5/mlx5.c             |  7 ++--
> >  drivers/net/mlx5/mlx5.h             |  1 -
> >  drivers/vdpa/mlx5/Makefile          |  3 +-
> >  drivers/vdpa/mlx5/meson.build       |  2 +-
> >  drivers/vdpa/mlx5/mlx5_vdpa.c       | 10 ++---
> >  mk/rte.app.mk                       |  1 +
> >  12 files changed, 79 insertions(+), 17 deletions(-)
> >
> 
> [...]
> 
> > diff --git a/drivers/net/mlx5/linux/mlx5_os.h
> > b/drivers/net/mlx5/linux/mlx5_os.h
> > index f310f1773..70972244f 100644
> > --- a/drivers/net/mlx5/linux/mlx5_os.h
> > +++ b/drivers/net/mlx5/linux/mlx5_os.h
> > @@ -12,7 +12,4 @@ enum {
> >  	DEV_SYSFS_PATH_MAX = IBV_SYSFS_PATH_MAX  };
> >
> > -#define PCI_DRV_FLAGS  (RTE_PCI_DRV_INTR_LSC | \
> > -			RTE_PCI_DRV_INTR_RMV | \
> > -			RTE_PCI_DRV_PROBE_AGAIN)
> >  #endif /* RTE_PMD_MLX5_OS_H_ */
> > diff --git a/drivers/net/mlx5/meson.build
> > b/drivers/net/mlx5/meson.build index e71b2c515..fd9f41d6e 100644
> > --- a/drivers/net/mlx5/meson.build
> > +++ b/drivers/net/mlx5/meson.build
> > @@ -8,7 +8,7 @@ if not (is_linux or is_windows)
> >  	subdir_done()
> >  endif
> >
> > -deps += ['hash', 'common_mlx5']
> > +deps += ['hash', 'common_mlx5', 'bus_mlx5_pci']
> >  sources = files(
> >  	'mlx5.c',
> >  	'mlx5_ethdev.c',
> > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index
> > 7c5e23d9f..6b250771f 100644
> > --- a/drivers/net/mlx5/mlx5.c
> > +++ b/drivers/net/mlx5/mlx5.c
> > @@ -34,6 +34,7 @@
> >  #include <rte_spinlock.h>
> >  #include <rte_string_fns.h>
> >  #include <rte_alarm.h>
> > +#include <rte_bus_mlx5_pci.h>
> >
> >  #include <mlx5_glue.h>
> >  #include <mlx5_devx_cmds.h>
> > @@ -2106,16 +2107,16 @@ static const struct rte_pci_id
> mlx5_pci_id_map[] = {
> >  	}
> >  };
> >
> > -struct rte_pci_driver mlx5_driver = {
> > +static struct rte_mlx5_pci_driver mlx5_driver = {
> >  	.driver = {
> >  		.name = MLX5_DRIVER_NAME
> >  	},
> > +	.dev_class = MLX5_CLASS_NET,
> >  	.id_table = mlx5_pci_id_map,
> >  	.probe = mlx5_os_pci_probe,
> >  	.remove = mlx5_pci_remove,
> >  	.dma_map = mlx5_dma_map,
> >  	.dma_unmap = mlx5_dma_unmap,
> > -	.drv_flags = PCI_DRV_FLAGS,
> 
> You mask the drv_flags, do you plan on merging those flags in the demuxing
> drivers above? It seems LSC | RMV | PROBE_AGAIN is not used afterward,
> seems missing.
> 
> Using an rte_pci_driver instead of an rte_driver as core object for you driver
> would allow managing this by the way.
Yes. but how would mlx5_pci bus will process the drv_flags passed by the class drivers such a vdpa/net?

> 
> >  };
> >
> >  /**
> > @@ -2133,7 +2134,7 @@ RTE_INIT(rte_mlx5_pmd_init)
> >  	mlx5_set_cksum_table();
> >  	mlx5_set_swp_types_table();
> >  	if (mlx5_glue)
> > -		rte_pci_register(&mlx5_driver);
> > +		rte_mlx5_pci_driver_register(&mlx5_driver);
> >  }
> >
> >  RTE_PMD_EXPORT_NAME(net_mlx5, __COUNTER__); diff --git
> > a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> > 8c4b234e5..b357543dd 100644
> > --- a/drivers/net/mlx5/mlx5.h
> > +++ b/drivers/net/mlx5/mlx5.h
> > @@ -129,7 +129,6 @@ struct mlx5_local_data {  };
> >
> >  extern struct mlx5_shared_data *mlx5_shared_data; -extern struct
> > rte_pci_driver mlx5_driver;
> >
> >  /* Dev ops structs */
> >  extern const struct eth_dev_ops mlx5_dev_sec_ops; diff --git
> > a/drivers/vdpa/mlx5/Makefile b/drivers/vdpa/mlx5/Makefile index
> > 91c89d604..a2231f021 100644
> > --- a/drivers/vdpa/mlx5/Makefile
> > +++ b/drivers/vdpa/mlx5/Makefile
> > @@ -24,13 +24,14 @@ CFLAGS += -
> I$(RTE_SDK)/drivers/common/mlx5/linux
> >  CFLAGS += -I$(RTE_SDK)/drivers/net/mlx5_vdpa
> >  CFLAGS += -I$(RTE_SDK)/lib/librte_sched  CFLAGS +=
> > -I$(BUILDDIR)/drivers/common/mlx5
> > +CFLAGS += -I$(RTE_SDK)/drivers/bus/mlx5_pci
> >  CFLAGS += -D_BSD_SOURCE
> >  CFLAGS += -D_DEFAULT_SOURCE
> >  CFLAGS += -D_XOPEN_SOURCE=600
> >  CFLAGS += $(WERROR_FLAGS)
> >  CFLAGS += -Wno-strict-prototypes
> >  LDLIBS += -lrte_common_mlx5
> > -LDLIBS += -lrte_eal -lrte_vhost -lrte_kvargs -lrte_pci -lrte_bus_pci
> > -lrte_sched
> > +LDLIBS += -lrte_eal -lrte_vhost -lrte_kvargs -lrte_pci
> > +-lrte_bus_mlx5_pci -lrte_sched
> >
> >  # A few warnings cannot be avoided in external headers.
> >  CFLAGS += -Wno-error=cast-qual
> > diff --git a/drivers/vdpa/mlx5/meson.build
> > b/drivers/vdpa/mlx5/meson.build index 2963aad71..f4175c34e 100644
> > --- a/drivers/vdpa/mlx5/meson.build
> > +++ b/drivers/vdpa/mlx5/meson.build
> > @@ -8,7 +8,7 @@ if not is_linux
> >  endif
> >
> >  fmt_name = 'mlx5_vdpa'
> > -deps += ['hash', 'common_mlx5', 'vhost', 'pci', 'bus_pci', 'eal',
> > 'sched']
> > +deps += ['hash', 'common_mlx5', 'vhost', 'pci', 'bus_mlx5_pci',
> > +'eal', 'sched']
> 
> After reading it more, I think bus_pci_mlx5 would work better.
> From the more general to the specific (bus >> pci >> mlx5 demux).
> 
Ok. that is fine too.
Will change.