Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <efaa9bf0-5b5a-a33e-ce38-44473159d44a@redhat.com>
Date:   Mon, 28 Feb 2022 16:07:27 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.6.1
Subject: Re: [RFC PATCH v2 05/19] vdpa: introduce virtqueue groups
Content-Language: en-US
To:     Gautam Dawar <gautam.dawar@xilinx.com>
Cc:     gdawar@xilinx.com, martinh@xilinx.com, hanand@xilinx.com,
        tanujk@xilinx.com, eperezma@redhat.com,
        "Michael S. Tsirkin" <mst@redhat.com>,
        Zhu Lingshan <lingshan.zhu@intel.com>,
        Stefano Garzarella <sgarzare@redhat.com>,
        Xie Yongji <xieyongji@bytedance.com>,
        Eli Cohen <elic@nvidia.com>,
        Si-Wei Liu <si-wei.liu@oracle.com>,
        Parav Pandit <parav@nvidia.com>,
        Longpeng <longpeng2@huawei.com>,
        virtualization@lists.linux-foundation.org,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        netdev@vger.kernel.org
References: <20201216064818.48239-1-jasowang@redhat.com>
 <20220224212314.1326-1-gdawar@xilinx.com>
 <20220224212314.1326-6-gdawar@xilinx.com>
From:   Jason Wang <jasowang@redhat.com>
In-Reply-To: <20220224212314.1326-6-gdawar@xilinx.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


在 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.

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_main.c
> +++ 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);