Re: [RFC 1/3] vdpa: support exposing the config size to userspace

Re: [RFC 1/3] vdpa: support exposing the config size to userspace

[Jason Wang]

在 2022/1/17 下午5:29, Longpeng(Mike) 写道:
> From: Longpeng <longpeng2@huawei.com>
>
> - GET_CONFIG_SIZE: the size of the virtio config space


I think we need to be verbose here. And it would be better to quote what 
spec said:

"

The device MUST allow reading of any device-specific configuration field 
before FEATURES_OK is set by the driver. This includes fields which are 
conditional on feature bits, as long as those feature bits are offered 
by the device.

"

I guess the size should contain the conditional on features bits.

(Or maybe we need to tweak the comment for get_config_size as well).

Other looks good.

Thanks


>
> Signed-off-by: Longpeng <longpeng2@huawei.com>
> ---
>   drivers/vhost/vdpa.c       | 17 +++++++++++++++++
>   include/uapi/linux/vhost.h |  4 ++++
>   2 files changed, 21 insertions(+)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 29cced1cd277..1eea14a4ea56 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -355,6 +355,20 @@ static long vhost_vdpa_get_iova_range(struct vhost_vdpa *v, u32 __user *argp)
>   	return 0;
>   }
>   
> +static long vhost_vdpa_get_config_size(struct vhost_vdpa *v, u32 __user *argp)
> +{
> +	struct vdpa_device *vdpa = v->vdpa;
> +	const struct vdpa_config_ops *ops = vdpa->config;
> +	u32 size;
> +
> +	size = ops->get_config_size(vdpa);
> +
> +	if (copy_to_user(argp, &size, sizeof(size)))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +
>   static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
>   				   void __user *argp)
>   {
> @@ -492,6 +506,9 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
>   	case VHOST_VDPA_GET_IOVA_RANGE:
>   		r = vhost_vdpa_get_iova_range(v, argp);
>   		break;
> +	case VHOST_VDPA_GET_CONFIG_SIZE:
> +		r = vhost_vdpa_get_config_size(v, argp);
> +		break;
>   	default:
>   		r = vhost_dev_ioctl(&v->vdev, cmd, argp);
>   		if (r == -ENOIOCTLCMD)
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index c998860d7bbc..bc74e95a273a 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -150,4 +150,8 @@
>   /* Get the valid iova range */
>   #define VHOST_VDPA_GET_IOVA_RANGE	_IOR(VHOST_VIRTIO, 0x78, \
>   					     struct vhost_vdpa_iova_range)
> +
> +/* Get the config size */
> +#define VHOST_VDPA_GET_CONFIG_SIZE	_IOR(VHOST_VIRTIO, 0x79, __u32)
> +
>   #endif

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC 2/3] vdpa: support exposing the count of vqs to userspace

[Jason Wang]

在 2022/1/17 下午5:29, Longpeng(Mike) 写道:
> From: Longpeng <longpeng2@huawei.com>
>
> - GET_VQS_COUNT: the count of virtqueues that exposed
>
> Signed-off-by: Longpeng <longpeng2@huawei.com>
> ---
>   drivers/vhost/vdpa.c       | 13 +++++++++++++
>   include/uapi/linux/vhost.h |  3 +++
>   2 files changed, 16 insertions(+)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 1eea14a4ea56..c1074278fc6b 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -369,6 +369,16 @@ static long vhost_vdpa_get_config_size(struct vhost_vdpa *v, u32 __user *argp)
>   	return 0;
>   }
>   
> +static long vhost_vdpa_get_vqs_count(struct vhost_vdpa *v, u32 __user *argp)
> +{
> +	struct vdpa_device *vdpa = v->vdpa;


While at it, I think it's better to change vdpa->nvqs to use u32?

Thanks


> +
> +	if (copy_to_user(argp, &vdpa->nvqs, sizeof(vdpa->nvqs)))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +
>   static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
>   				   void __user *argp)
>   {
> @@ -509,6 +519,9 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
>   	case VHOST_VDPA_GET_CONFIG_SIZE:
>   		r = vhost_vdpa_get_config_size(v, argp);
>   		break;
> +	case VHOST_VDPA_GET_VQS_COUNT:
> +		r = vhost_vdpa_get_vqs_count(v, argp);
> +		break;
>   	default:
>   		r = vhost_dev_ioctl(&v->vdev, cmd, argp);
>   		if (r == -ENOIOCTLCMD)
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index bc74e95a273a..5d99e7c242a2 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -154,4 +154,7 @@
>   /* Get the config size */
>   #define VHOST_VDPA_GET_CONFIG_SIZE	_IOR(VHOST_VIRTIO, 0x79, __u32)
>   
> +/* Get the count of all virtqueues */
> +#define VHOST_VDPA_GET_VQS_COUNT	_IOR(VHOST_VIRTIO, 0x80, __u32)
> +
>   #endif

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC 3/3] vdpasim_net: control virtqueue support

[Jason Wang]

在 2022/1/17 下午5:29, Longpeng(Mike) 写道:
> From: Longpeng <longpeng2@huawei.com>
>
> Introduces the control virtqueue support for vdpasim_net, based on
> Jason's RFC [1].
>
> [1] https://patchwork.kernel.org/project/kvm/patch/20200924032125.18619-25-jasowang@redhat.com/


I'd expect to implement the receive filter as well[1]. This gives us a 
chance to test this.

Thanks

[1] https://lkml.org/lkml/2020/9/23/1269


>
> Signed-off-by: Longpeng <longpeng2@huawei.com>
> ---
>   drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 83 +++++++++++++++++++++++++++-
>   1 file changed, 81 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> index 76dd24abc791..e9e388fd3cff 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> @@ -26,9 +26,85 @@
>   #define DRV_LICENSE  "GPL v2"
>   
>   #define VDPASIM_NET_FEATURES	(VDPASIM_FEATURES | \
> -				 (1ULL << VIRTIO_NET_F_MAC))
> +				 (1ULL << VIRTIO_NET_F_MAC) | \
> +				 (1ULL << VIRTIO_NET_F_CTRL_VQ) | \
> +				 (1ULL << VIRTIO_NET_F_CTRL_MAC_ADDR))
>   
> -#define VDPASIM_NET_VQ_NUM	2
> +#define VDPASIM_NET_VQ_NUM	3
> +
> +virtio_net_ctrl_ack vdpasim_net_handle_ctrl_mac(struct vdpasim *vdpasim,
> +						u8 cmd)
> +{
> +	struct vdpasim_virtqueue *cvq = &vdpasim->vqs[2];
> +	virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
> +	struct virtio_net_config *config = vdpasim->config;
> +	size_t read;
> +
> +	switch (cmd) {
> +	case VIRTIO_NET_CTRL_MAC_ADDR_SET:
> +		read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->out_iov,
> +					     (void *)config->mac, ETH_ALEN);
> +		if (read == ETH_ALEN)
> +			status = VIRTIO_NET_OK;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return status;
> +}
> +
> +static void vdpasim_net_handle_cvq(struct vdpasim *vdpasim)
> +{
> +	struct vdpasim_virtqueue *cvq = &vdpasim->vqs[2];
> +	virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
> +	struct virtio_net_ctrl_hdr ctrl;
> +	size_t read, write;
> +	int err;
> +
> +	if (!(vdpasim->features & (1ULL << VIRTIO_NET_F_CTRL_VQ)))
> +		return;
> +
> +	if (!cvq->ready)
> +		return;
> +
> +	while (true) {
> +		err = vringh_getdesc_iotlb(&cvq->vring, &cvq->out_iov, &cvq->in_iov,
> +					   &cvq->head, GFP_ATOMIC);
> +		if (err <= 0)
> +			break;
> +
> +		read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->in_iov, &ctrl,
> +					     sizeof(ctrl));
> +		if (read != sizeof(ctrl))
> +			break;
> +
> +		switch (ctrl.class) {
> +		case VIRTIO_NET_CTRL_MAC:
> +			status = vdpasim_net_handle_ctrl_mac(vdpasim, ctrl.cmd);
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		/* Make sure data is wrote before advancing index */
> +		smp_wmb();
> +
> +		write = vringh_iov_push_iotlb(&cvq->vring, &cvq->out_iov,
> +					      &status, sizeof (status));
> +		vringh_complete_iotlb(&cvq->vring, cvq->head, write);
> +		vringh_kiov_cleanup(&cvq->in_iov);
> +		vringh_kiov_cleanup(&cvq->out_iov);
> +
> +		/* Make sure used is visible before rasing the interrupt. */
> +		smp_wmb();
> +
> +		local_bh_disable();
> +		if (vringh_need_notify_iotlb(&cvq->vring) > 0)
> +			vringh_notify(&cvq->vring);
> +		local_bh_enable();
> +	}
> +}
>   
>   static void vdpasim_net_work(struct work_struct *work)
>   {
> @@ -42,6 +118,9 @@ static void vdpasim_net_work(struct work_struct *work)
>   
>   	spin_lock(&vdpasim->lock);
>   
> +	/* process ctrl vq first */
> +	vdpasim_net_handle_cvq(vdpasim);
> +
>   	if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK))
>   		goto out;
>   

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC 2/3] vdpa: support exposing the count of vqs to userspace

[Jason Wang]

[-- Attachment #1.1: Type: text/plain, Size: 3273 bytes --]

On Thu, Mar 10, 2022 at 11:16 AM Longpeng (Mike, Cloud Infrastructure
Service Product Dept.) <longpeng2@huawei.com> wrote:

> Hi Jason,
>
> > -----Original Message-----
> > From: Jason Wang [mailto:jasowang@redhat.com]
> > Sent: Tuesday, January 18, 2022 11:08 AM
> > To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
> > <longpeng2@huawei.com>; mst@redhat.com; sgarzare@redhat.com;
> > stefanha@redhat.com
> > Cc: virtualization@lists.linux-foundation.org; kvm@vger.kernel.org;
> > linux-kernel@vger.kernel.org; netdev@vger.kernel.org; Gonglei (Arei)
> > <arei.gonglei@huawei.com>; Yechuan <yechuan@huawei.com>; Huangzhichao
> > <huangzhichao@huawei.com>
> > Subject: Re: [RFC 2/3] vdpa: support exposing the count of vqs to
> userspace
> >
> >
> > 在 2022/1/17 下午5:29, Longpeng(Mike) 写道:
> > > From: Longpeng <longpeng2@huawei.com>
> > >
> > > - GET_VQS_COUNT: the count of virtqueues that exposed
> > >
> > > Signed-off-by: Longpeng <longpeng2@huawei.com>
> > > ---
> > >   drivers/vhost/vdpa.c       | 13 +++++++++++++
> > >   include/uapi/linux/vhost.h |  3 +++
> > >   2 files changed, 16 insertions(+)
> > >
> > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> > > index 1eea14a4ea56..c1074278fc6b 100644
> > > --- a/drivers/vhost/vdpa.c
> > > +++ b/drivers/vhost/vdpa.c
> > > @@ -369,6 +369,16 @@ static long vhost_vdpa_get_config_size(struct
> vhost_vdpa
> > *v, u32 __user *argp)
> > >     return 0;
> > >   }
> > >
> > > +static long vhost_vdpa_get_vqs_count(struct vhost_vdpa *v, u32 __user
> *argp)
> > > +{
> > > +   struct vdpa_device *vdpa = v->vdpa;
> >
> >
> > While at it, I think it's better to change vdpa->nvqs to use u32?
> >
>
> struct vhost_vdpa {
>     ...
>         int nvqs;
>     ...
> };
>
> struct vdpa_device {
>     ...
>         int nvqs;
>     ...
> };
>
> I think we should change both to u32?
>

Yes.

Thanks


>
>
> > Thanks
> >
> >
> > > +
> > > +   if (copy_to_user(argp, &vdpa->nvqs, sizeof(vdpa->nvqs)))
> > > +           return -EFAULT;
> > > +
> > > +   return 0;
> > > +}
> > > +
> > >   static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned
> int cmd,
> > >                                void __user *argp)
> > >   {
> > > @@ -509,6 +519,9 @@ static long vhost_vdpa_unlocked_ioctl(struct file
> *filep,
> > >     case VHOST_VDPA_GET_CONFIG_SIZE:
> > >             r = vhost_vdpa_get_config_size(v, argp);
> > >             break;
> > > +   case VHOST_VDPA_GET_VQS_COUNT:
> > > +           r = vhost_vdpa_get_vqs_count(v, argp);
> > > +           break;
> > >     default:
> > >             r = vhost_dev_ioctl(&v->vdev, cmd, argp);
> > >             if (r == -ENOIOCTLCMD)
> > > diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> > > index bc74e95a273a..5d99e7c242a2 100644
> > > --- a/include/uapi/linux/vhost.h
> > > +++ b/include/uapi/linux/vhost.h
> > > @@ -154,4 +154,7 @@
> > >   /* Get the config size */
> > >   #define VHOST_VDPA_GET_CONFIG_SIZE        _IOR(VHOST_VIRTIO, 0x79,
> __u32)
> > >
> > > +/* Get the count of all virtqueues */
> > > +#define VHOST_VDPA_GET_VQS_COUNT   _IOR(VHOST_VIRTIO, 0x80, __u32)
> > > +
> > >   #endif
>
>

[-- Attachment #1.2: Type: text/html, Size: 5350 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization