RE: [RFC PATCH v2 05/19] vdpa: introduce virtqueue groups

[Gautam Dawar <gdawar@xilinx.com>]

在 2022/2/25 上午5:22, Gautam Dawar 写道:
> This patch introduces virtqueue groups to vDPA device. The virtqueue 
> group is the minimal set of virtqueues that must share an address 
> space. And the address space identifier could only be attached to a 
> specific virtqueue group.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
> ---
>   drivers/vdpa/ifcvf/ifcvf_main.c   |  8 +++++++-
>   drivers/vdpa/mlx5/net/mlx5_vnet.c |  8 +++++++-
>   drivers/vdpa/vdpa.c               |  3 +++
>   drivers/vdpa/vdpa_sim/vdpa_sim.c  |  9 ++++++++-
>   drivers/vdpa/vdpa_sim/vdpa_sim.h  |  1 +
>   include/linux/vdpa.h              | 16 ++++++++++++----
>   6 files changed, 38 insertions(+), 7 deletions(-)


We had three more parents now, so we should convert them as well:

vp_vdpa, VDUSE and eni_vdpa.
[GD>>] I agree. However, the intention of publishing this as a RFC series is to gather feedback on the approach in the cover letter and at the same time to test its integration with both a simulator (vdpa_sim) and an actual (Xilinx SmartNIC) HW device with a development build of QEMU supporting Shadow virtqueue by Eugnio.
Once this is approved, we can move to the actual patch series covering all other vdpa parent drivers.

Thanks


>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c 
> b/drivers/vdpa/ifcvf/ifcvf_main.c index d1a6b5ab543c..c815a2e62440 
> 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_m
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -378,6 +378,11 @@ static size_t ifcvf_vdpa_get_config_size(struct vdpa_device *vdpa_dev)
>   	return  vf->config_size;
>   }
>   
> +static u32 ifcvf_vdpa_get_vq_group(struct vdpa_device *vdpa, u16 idx) 
> +{
> +	return 0;
> +}
> +
>   static void ifcvf_vdpa_get_config(struct vdpa_device *vdpa_dev,
>   				  unsigned int offset,
>   				  void *buf, unsigned int len)
> @@ -453,6 +458,7 @@ static const struct vdpa_config_ops ifc_vdpa_ops = {
>   	.get_device_id	= ifcvf_vdpa_get_device_id,
>   	.get_vendor_id	= ifcvf_vdpa_get_vendor_id,
>   	.get_vq_align	= ifcvf_vdpa_get_vq_align,
> +	.get_vq_group	= ifcvf_vdpa_get_vq_group,
>   	.get_config_size	= ifcvf_vdpa_get_config_size,
>   	.get_config	= ifcvf_vdpa_get_config,
>   	.set_config	= ifcvf_vdpa_set_config,
> @@ -507,7 +513,7 @@ static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
>   	pdev = ifcvf_mgmt_dev->pdev;
>   	dev = &pdev->dev;
>   	adapter = vdpa_alloc_device(struct ifcvf_adapter, vdpa,
> -				    dev, &ifc_vdpa_ops, name, false);
> +				    dev, &ifc_vdpa_ops, 1, name, false);
>   	if (IS_ERR(adapter)) {
>   		IFCVF_ERR(pdev, "Failed to allocate vDPA structure");
>   		return PTR_ERR(adapter);
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
> b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index b53603d94082..fcfc28460b72 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -1847,6 +1847,11 @@ static u32 mlx5_vdpa_get_vq_align(struct vdpa_device *vdev)
>   	return PAGE_SIZE;
>   }
>   
> +static u32 mlx5_vdpa_get_vq_group(struct vdpa_device *vdpa, u16 idx) 
> +{
> +	return 0;
> +}
> +
>   enum { MLX5_VIRTIO_NET_F_GUEST_CSUM = 1 << 9,
>   	MLX5_VIRTIO_NET_F_CSUM = 1 << 10,
>   	MLX5_VIRTIO_NET_F_HOST_TSO6 = 1 << 11, @@ -2363,6 +2368,7 @@ static 
> const struct vdpa_config_ops mlx5_vdpa_ops = {
>   	.get_vq_notification = mlx5_get_vq_notification,
>   	.get_vq_irq = mlx5_get_vq_irq,
>   	.get_vq_align = mlx5_vdpa_get_vq_align,
> +	.get_vq_group = mlx5_vdpa_get_vq_group,
>   	.get_device_features = mlx5_vdpa_get_device_features,
>   	.set_driver_features = mlx5_vdpa_set_driver_features,
>   	.get_driver_features = mlx5_vdpa_get_driver_features, @@ -2575,7 
> +2581,7 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
>   	}
>   
>   	ndev = vdpa_alloc_device(struct mlx5_vdpa_net, mvdev.vdev, mdev->device, &mlx5_vdpa_ops,
> -				 name, false);
> +				 1, name, false);
>   	if (IS_ERR(ndev))
>   		return PTR_ERR(ndev);
>   
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index 
> 9846c9de4bfa..a07bf0130559 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -159,6 +159,7 @@ static void vdpa_release_dev(struct device *d)
>    * initialized but before registered.
>    * @parent: the parent device
>    * @config: the bus operations that is supported by this device
> + * @ngroups: number of groups supported by this device
>    * @size: size of the parent structure that contains private data
>    * @name: name of the vdpa device; optional.
>    * @use_va: indicate whether virtual address must be used by this 
> device @@ -171,6 +172,7 @@ static void vdpa_release_dev(struct device *d)
>    */
>   struct vdpa_device *__vdpa_alloc_device(struct device *parent,
>   					const struct vdpa_config_ops *config,
> +					unsigned int ngroups,
>   					size_t size, const char *name,
>   					bool use_va)
>   {
> @@ -203,6 +205,7 @@ struct vdpa_device *__vdpa_alloc_device(struct device *parent,
>   	vdev->config = config;
>   	vdev->features_valid = false;
>   	vdev->use_va = use_va;
> +	vdev->ngroups = ngroups;
>   
>   	if (name)
>   		err = dev_set_name(&vdev->dev, "%s", name); diff --git 
> a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index ddbe142af09a..c98cb1f869fa 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -250,7 +250,7 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
>   	else
>   		ops = &vdpasim_config_ops;
>   
> -	vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops,
> +	vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops, 1,
>   				    dev_attr->name, false);
>   	if (IS_ERR(vdpasim)) {
>   		ret = PTR_ERR(vdpasim);
> @@ -399,6 +399,11 @@ static u32 vdpasim_get_vq_align(struct vdpa_device *vdpa)
>   	return VDPASIM_QUEUE_ALIGN;
>   }
>   
> +static u32 vdpasim_get_vq_group(struct vdpa_device *vdpa, u16 idx) {
> +	return 0;
> +}
> +
>   static u64 vdpasim_get_device_features(struct vdpa_device *vdpa)
>   {
>   	struct vdpasim *vdpasim = vdpa_to_sim(vdpa); @@ -620,6 +625,7 @@ 
> static const struct vdpa_config_ops vdpasim_config_ops = {
>   	.set_vq_state           = vdpasim_set_vq_state,
>   	.get_vq_state           = vdpasim_get_vq_state,
>   	.get_vq_align           = vdpasim_get_vq_align,
> +	.get_vq_group           = vdpasim_get_vq_group,
>   	.get_device_features    = vdpasim_get_device_features,
>   	.set_driver_features    = vdpasim_set_driver_features,
>   	.get_driver_features    = vdpasim_get_driver_features,
> @@ -650,6 +656,7 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = {
>   	.set_vq_state           = vdpasim_set_vq_state,
>   	.get_vq_state           = vdpasim_get_vq_state,
>   	.get_vq_align           = vdpasim_get_vq_align,
> +	.get_vq_group           = vdpasim_get_vq_group,
>   	.get_device_features    = vdpasim_get_device_features,
>   	.set_driver_features    = vdpasim_set_driver_features,
>   	.get_driver_features    = vdpasim_get_driver_features,
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h 
> b/drivers/vdpa/vdpa_sim/vdpa_sim.h
> index cd58e888bcf3..0be7c1e7ef80 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.h
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h
> @@ -63,6 +63,7 @@ struct vdpasim {
>   	u32 status;
>   	u32 generation;
>   	u64 features;
> +	u32 groups;
>   	/* spinlock to synchronize iommu table */
>   	spinlock_t iommu_lock;
>   };
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h index 
> 2de442ececae..026b7ad72ed7 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -85,6 +85,7 @@ struct vdpa_device {
>   	bool use_va;
>   	int nvqs;
>   	struct vdpa_mgmt_dev *mdev;
> +	unsigned int ngroups;
>   };
>   
>   /**
> @@ -172,6 +173,10 @@ struct vdpa_map_file {
>    *				for the device
>    *				@vdev: vdpa device
>    *				Returns virtqueue algin requirement
> + * @get_vq_group:		Get the group id for a specific virtqueue
> + *				@vdev: vdpa device
> + *				@idx: virtqueue index
> + *				Returns u32: group id for this virtqueue
>    * @get_device_features:	Get virtio features supported by the device
>    *				@vdev: vdpa device
>    *				Returns the virtio features support by the
> @@ -282,6 +287,7 @@ struct vdpa_config_ops {
>   
>   	/* Device ops */
>   	u32 (*get_vq_align)(struct vdpa_device *vdev);
> +	u32 (*get_vq_group)(struct vdpa_device *vdev, u16 idx);
>   	u64 (*get_device_features)(struct vdpa_device *vdev);
>   	int (*set_driver_features)(struct vdpa_device *vdev, u64 features);
>   	u64 (*get_driver_features)(struct vdpa_device *vdev); @@ -314,6 
> +320,7 @@ struct vdpa_config_ops {
>   
>   struct vdpa_device *__vdpa_alloc_device(struct device *parent,
>   					const struct vdpa_config_ops *config,
> +					unsigned int ngroups,
>   					size_t size, const char *name,
>   					bool use_va);
>   
> @@ -324,17 +331,18 @@ struct vdpa_device *__vdpa_alloc_device(struct device *parent,
>    * @member: the name of struct vdpa_device within the @dev_struct
>    * @parent: the parent device
>    * @config: the bus operations that is supported by this device
> + * @ngroups: the number of virtqueue groups supported by this device
>    * @name: name of the vdpa device
>    * @use_va: indicate whether virtual address must be used by this device
>    *
>    * Return allocated data structure or ERR_PTR upon error
>    */
> -#define vdpa_alloc_device(dev_struct, member, parent, config, name, use_va)   \
> -			  container_of(__vdpa_alloc_device( \
> -				       parent, config, \
> +#define vdpa_alloc_device(dev_struct, member, parent, config, ngroups, name, use_va)   \
> +			  container_of((__vdpa_alloc_device( \
> +				       parent, config, ngroups, \
>   				       sizeof(dev_struct) + \
>   				       BUILD_BUG_ON_ZERO(offsetof( \
> -				       dev_struct, member)), name, use_va), \
> +				       dev_struct, member)), name, use_va)), \
>   				       dev_struct, member)
>   
>   int vdpa_register_device(struct vdpa_device *vdev, int nvqs);