Re: [PATCH V6 6/6] docs: sample driver to demonstrate how to implement virtio-mdev framework

2019-10-30 Thread Jason Wang


On 2019/10/31 上午5:23, Christoph Hellwig wrote:

On Wed, Oct 30, 2019 at 02:44:44PM +0800, Jason Wang wrote:

This sample driver creates mdev device that simulate virtio net device
over virtio mdev transport. The device is implemented through vringh
and workqueue. A device specific dma ops is to make sure HVA is used
directly as the IOVA. This should be sufficient for kernel virtio
driver to work.

Only 'virtio' type is supported right now. I plan to add 'vhost' type
on top which requires some virtual IOMMU implemented in this sample
driver.

Can we please submit a real driver for it?  A more or less useless
sample driver doesn't really qualify for our normal kernel requirements
that infrastructure should have a real user.



Intel posted a real driver here: https://lkml.org/lkml/2019/10/15/1226.

I plan to post another driver that wire virito-pci back to mdev bus on 
top of this series as well.


Thanks


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

Re: [RFC] vhost_mdev: add network control vq support

2019-10-30 Thread Jason Wang


On 2019/10/30 下午7:54, Tiwei Bie wrote:

On Wed, Oct 30, 2019 at 03:04:37PM +0800, Jason Wang wrote:

On 2019/10/30 下午2:17, Tiwei Bie wrote:

On Tue, Oct 29, 2019 at 06:51:32PM +0800, Jason Wang wrote:

On 2019/10/29 下午6:17, Tiwei Bie wrote:

This patch adds the network control vq support in vhost-mdev.
A vhost-mdev specific op is introduced to allow parent drivers
to handle the network control commands come from userspace.

Probably work for userspace driver but not kernel driver.

Exactly. This is only for userspace.

I got your point now. In virtio-mdev kernel driver case,
the ctrl-vq can be special as well.


Then maybe it's better to introduce vhost-mdev-net on top?

Looking at the other type of virtio device:

- console have two control virtqueues when multiqueue port is enabled

- SCSI has controlq + eventq

- GPU has controlq

- Crypto device has one controlq

- Socket has eventq

...

Thanks for the list! It looks dirty to define specific
commands and types in vhost UAPI for each of them in the
future. It's definitely much better to find an approach
to solve it once for all if possible..

Just a quick thought, considering all vhost-mdev does
is just to forward settings between parent and userspace,
I'm wondering whether it's possible to make the argp
opaque in vhost-mdev UAPI and just introduce one generic
ioctl command to deliver these device specific commands
(which are opaque in vhost-mdev as vhost-mdev just pass
the pointer -- argp) defined by spec.



It looks that using opaque pointer is probably not good for UAPI. And we 
need also invent API for eventq.





I'm also fine with exposing ctrlq to userspace directly.
PS. It's interesting that some devices have more than
one ctrlq. I need to take a close look first..



Thanks.






Thanks



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

Re: [PATCH V6 6/6] docs: sample driver to demonstrate how to implement virtio-mdev framework

2019-10-30 Thread Christoph Hellwig
On Wed, Oct 30, 2019 at 02:44:44PM +0800, Jason Wang wrote:
> This sample driver creates mdev device that simulate virtio net device
> over virtio mdev transport. The device is implemented through vringh
> and workqueue. A device specific dma ops is to make sure HVA is used
> directly as the IOVA. This should be sufficient for kernel virtio
> driver to work.
> 
> Only 'virtio' type is supported right now. I plan to add 'vhost' type
> on top which requires some virtual IOMMU implemented in this sample
> driver.

Can we please submit a real driver for it?  A more or less useless
sample driver doesn't really qualify for our normal kernel requirements
that infrastructure should have a real user.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: read_barrier_depends() usage in vhost.c

2019-10-30 Thread Will Deacon
Hi Jason,

On Mon, Oct 21, 2019 at 01:48:53PM +0800, Jason Wang wrote:
> On 2019/10/19 上午4:58, Will Deacon wrote:
> > Staring at the code some more, my best bet at the moment
> > is that the load of 'indirect->addr' is probably the one to worry about,
> > since it's part of the vring and can be updated concurrently.
> 
> I'm also confused about the barrier here, basically in driver side we did:
> 
> 1) allocate pages
> 
> 2) store pages in indirect->addr
> 
> 3) smp_wmb()
> 
> 4) increase the avail idx (somehow a tail pointer of vring)
> 
> in vhost we did:
> 
> 1) read avail idx
> 
> 2) smp_rmb()
> 
> 3) read indirect->addr
> 
> 4) read from indirect->addr
> 
> It looks to me even the data dependency barrier is not necessary since we
> have rmb() which is sufficient for us to the correct indirect->addr and
> driver are not expected to do any writing to indirect->addr after avail idx
> is increased ?

That makes sense to me: the synchronization is done via vq->avail_idx()
and so the subsequent access of the indirect pages doesn't need additional
barriers, even on Alpha. Thanks.

I'll write up a patch and adopt your explanation above.

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

RE: [PATCH net-next 11/14] vsock: add multi-transports support

2019-10-30 Thread Jorgen Hansen via Virtualization
> From: Stefano Garzarella [mailto:sgarz...@redhat.com]
> > +/* Assign a transport to a socket and call the .init transport callback.
> > + *
> > + * Note: for stream socket this must be called when vsk->remote_addr
> > +is set
> > + * (e.g. during the connect() or when a connection request on a
> > +listener
> > + * socket is received).
> > + * The vsk->remote_addr is used to decide which transport to use:
> > + *  - remote CID > VMADDR_CID_HOST will use host->guest transport
> > + *  - remote CID <= VMADDR_CID_HOST will use guest->host transport
> > +*/ int vsock_assign_transport(struct vsock_sock *vsk, struct
> > +vsock_sock *psk) {
> > +   const struct vsock_transport *new_transport;
> > +   struct sock *sk = sk_vsock(vsk);
> > +
> > +   switch (sk->sk_type) {
> > +   case SOCK_DGRAM:
> > +   new_transport = transport_dgram;
> > +   break;
> > +   case SOCK_STREAM:
> > +   if (vsk->remote_addr.svm_cid > VMADDR_CID_HOST)
> > +   new_transport = transport_h2g;
> > +   else
> > +   new_transport = transport_g2h;
> 
> I just noticed that this break the loopback in the guest.
> As a fix, we should use 'transport_g2h' when remote_cid <=
> VMADDR_CID_HOST or remote_cid is the id of 'transport_g2h'.
> 
> To do that we also need to avoid that L2 guests can have the same CID of L1.
> For vhost_vsock I can call vsock_find_cid() in vhost_vsock_set_cid()
> 
> @Jorgen: for vmci we need to do the same? or it is guaranteed, since it's
> already support nested VMs, that a L2 guests cannot have the same CID as
> the L1.

As far as I can tell, we have the same issue with the current support for 
nested VMs in
VMCI. If we have an L2 guest with the same CID as the L1 guest, we will always 
send to
the L2 guest, and we may assign an L2 guest the same CID as L1. It should be 
straight
forward to avoid this, though.

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


RE: [PATCH net-next 09/14] vsock: move vsock_insert_unbound() in the vsock_create()

2019-10-30 Thread Jorgen Hansen via Virtualization
> From: Stefano Garzarella [mailto:sgarz...@redhat.com]
> Sent: Wednesday, October 23, 2019 11:56 AM
> Subject: [PATCH net-next 09/14] vsock: move vsock_insert_unbound() in the
> vsock_create()
> 
> vsock_insert_unbound() was called only when 'sock' parameter of
> __vsock_create() was not null. This only happened when
> __vsock_create() was called by vsock_create().
> 
> In order to simplify the multi-transports support, this patch moves
> vsock_insert_unbound() at the end of vsock_create().
> 
> Reviewed-by: Dexuan Cui 
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Stefano Garzarella 
> ---
>  net/vmw_vsock/af_vsock.c | 13 +

Reviewed-by: Jorgen Hansen 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


RE: [PATCH net-next 08/14] vsock: add vsock_create_connected() called by transports

2019-10-30 Thread Jorgen Hansen via Virtualization
> From: Stefano Garzarella [mailto:sgarz...@redhat.com]
> Sent: Wednesday, October 23, 2019 11:56 AM
> Subject: [PATCH net-next 08/14] vsock: add vsock_create_connected() called
> by transports
> 
> All transports call __vsock_create() with the same parameters,
> most of them depending on the parent socket. In order to simplify
> the VSOCK core APIs exposed to the transports, this patch adds
> the vsock_create_connected() callable from transports to create
> a new socket when a connection request is received.
> We also unexported the __vsock_create().
> 
> Suggested-by: Stefan Hajnoczi 
> Signed-off-by: Stefano Garzarella 
> ---
>  include/net/af_vsock.h  |  5 +
>  net/vmw_vsock/af_vsock.c| 20 +---
>  net/vmw_vsock/hyperv_transport.c|  3 +--
>  net/vmw_vsock/virtio_transport_common.c |  3 +--
>  net/vmw_vsock/vmci_transport.c  |  3 +--
>  5 files changed, 17 insertions(+), 17 deletions(-)

Reviewed-by: Jorgen Hansen 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


RE: [PATCH net-next 07/14] vsock: handle buffer_size sockopts in the core

2019-10-30 Thread Jorgen Hansen via Virtualization
> From: Stefano Garzarella [mailto:sgarz...@redhat.com]
> Sent: Wednesday, October 23, 2019 11:56 AM
> Subject: [PATCH net-next 07/14] vsock: handle buffer_size sockopts in the
> core
> 
> virtio_transport and vmci_transport handle the buffer_size sockopts in a
> very similar way.
> 
> In order to support multiple transports, this patch moves this handling in the
> core to allow the user to change the options also if the socket is not yet
> assigned to any transport.
> 
> This patch also adds the '.notify_buffer_size' callback in the 'struct
> virtio_transport' in order to inform the transport, when the buffer_size is
> changed by the user. It is also useful to limit the 'buffer_size' requested 
> (e.g.
> virtio transports).
> 
> Acked-by: Dexuan Cui 
> Signed-off-by: Stefano Garzarella 
> ---
> RFC -> v1:
> - changed .notify_buffer_size return to void (Stefan)
> - documented that .notify_buffer_size is called with sk_lock held (Stefan)
> ---
>  drivers/vhost/vsock.c   |  7 +-
>  include/linux/virtio_vsock.h| 15 +
>  include/net/af_vsock.h  | 15 ++---
>  net/vmw_vsock/af_vsock.c| 43 ++---
>  net/vmw_vsock/hyperv_transport.c| 36 ---
>  net/vmw_vsock/virtio_transport.c|  8 +--
>  net/vmw_vsock/virtio_transport_common.c | 79 ---
>  net/vmw_vsock/vmci_transport.c  | 86 +++--
>  net/vmw_vsock/vmci_transport.h  |  3 -
>  9 files changed, 65 insertions(+), 227 deletions(-)
> 
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c index
> 92ab3852c954..6d7e4f022748 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -418,13 +418,8 @@ static struct virtio_transport vhost_transport = {
>   .notify_send_pre_block=
> virtio_transport_notify_send_pre_block,
>   .notify_send_pre_enqueue  =
> virtio_transport_notify_send_pre_enqueue,
>   .notify_send_post_enqueue =
> virtio_transport_notify_send_post_enqueue,
> + .notify_buffer_size   = virtio_transport_notify_buffer_size,
> 
> - .set_buffer_size  = virtio_transport_set_buffer_size,
> - .set_min_buffer_size  =
> virtio_transport_set_min_buffer_size,
> - .set_max_buffer_size  =
> virtio_transport_set_max_buffer_size,
> - .get_buffer_size  = virtio_transport_get_buffer_size,
> - .get_min_buffer_size  =
> virtio_transport_get_min_buffer_size,
> - .get_max_buffer_size  =
> virtio_transport_get_max_buffer_size,
>   },
> 
>   .send_pkt = vhost_transport_send_pkt,
> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h index
> 96d8132acbd7..b79befd2a5a4 100644
> --- a/include/linux/virtio_vsock.h
> +++ b/include/linux/virtio_vsock.h
> @@ -7,9 +7,6 @@
>  #include 
>  #include 
> 
> -#define VIRTIO_VSOCK_DEFAULT_MIN_BUF_SIZE128
> -#define VIRTIO_VSOCK_DEFAULT_BUF_SIZE(1024 * 256)
> -#define VIRTIO_VSOCK_DEFAULT_MAX_BUF_SIZE(1024 * 256)
>  #define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE (1024 * 4)
>  #define VIRTIO_VSOCK_MAX_BUF_SIZE0xUL
>  #define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE(1024 * 64)
> @@ -25,11 +22,6 @@ enum {
>  struct virtio_vsock_sock {
>   struct vsock_sock *vsk;
> 
> - /* Protected by lock_sock(sk_vsock(trans->vsk)) */
> - u32 buf_size;
> - u32 buf_size_min;
> - u32 buf_size_max;
> -
>   spinlock_t tx_lock;
>   spinlock_t rx_lock;
> 
> @@ -93,12 +85,6 @@ s64 virtio_transport_stream_has_space(struct
> vsock_sock *vsk);
> 
>  int virtio_transport_do_socket_init(struct vsock_sock *vsk,
>struct vsock_sock *psk);
> -u64 virtio_transport_get_buffer_size(struct vsock_sock *vsk);
> -u64 virtio_transport_get_min_buffer_size(struct vsock_sock *vsk);
> -u64 virtio_transport_get_max_buffer_size(struct vsock_sock *vsk); -void
> virtio_transport_set_buffer_size(struct vsock_sock *vsk, u64 val); -void
> virtio_transport_set_min_buffer_size(struct vsock_sock *vsk, u64 val); -void
> virtio_transport_set_max_buffer_size(struct vsock_sock *vs, u64 val);  int
> virtio_transport_notify_poll_in(struct vsock_sock *vsk,
>   size_t target,
> @@ -125,6 +111,7 @@ int
> virtio_transport_notify_send_pre_enqueue(struct vsock_sock *vsk,
>   struct vsock_transport_send_notify_data *data);  int
> virtio_transport_notify_send_post_enqueue(struct vsock_sock *vsk,
>   ssize_t written, struct vsock_transport_send_notify_data *data);
> +void virtio_transport_notify_buffer_size(struct vsock_sock *vsk, u64
> +*val);
> 
>  u64 virtio_transport_stream_rcvhiwat(struct vsock_sock *vsk);  bool
> virtio_transport_stream_is_active(struct vsock_sock *vsk); diff --git
> a/include/net/af_vsock.h b/include/net/af_vsock.h index
> 2ca67d048de4..4b5d16840fd4 100644
> --- a/include/net/af_vsock.h
> +++ b/include/net

[PATCH 0/3] virtiofs: Small Cleanups for 5.5

2019-10-30 Thread Vivek Goyal
Hi Miklos,

Here are few small cleanups for virtiofs for 5.5. I had received some
comments from Michael Tsirkin on original virtiofs patches and these
cleanups are result of these comments.

Thanks
Vivek

Vivek Goyal (3):
  virtiofs: Use a common function to send forget
  virtiofs: Do not send forget request "struct list_head" element
  virtiofs: Use completions while waiting for queue to be drained

 fs/fuse/virtio_fs.c | 204 ++--
 1 file changed, 103 insertions(+), 101 deletions(-)

-- 
2.20.1

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


[PATCH 3/3] virtiofs: Use completions while waiting for queue to be drained

2019-10-30 Thread Vivek Goyal
While we wait for queue to finish draining, use completions instead of
uslee_range(). This is better way of waiting for event.

Signed-off-by: Vivek Goyal 
---
 fs/fuse/virtio_fs.c | 39 +--
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 43224db8d9ed..b5ba83ef1914 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -35,6 +35,7 @@ struct virtio_fs_vq {
struct fuse_dev *fud;
bool connected;
long in_flight;
+   struct completion in_flight_zero; /* No inflight requests */
char name[24];
 } cacheline_aligned_in_smp;
 
@@ -85,6 +86,8 @@ static inline void dec_in_flight_req(struct virtio_fs_vq 
*fsvq)
 {
WARN_ON(fsvq->in_flight <= 0);
fsvq->in_flight--;
+   if (!fsvq->in_flight)
+   complete(&fsvq->in_flight_zero);
 }
 
 static void release_virtio_fs_obj(struct kref *ref)
@@ -115,22 +118,23 @@ static void virtio_fs_drain_queue(struct virtio_fs_vq 
*fsvq)
WARN_ON(fsvq->in_flight < 0);
 
/* Wait for in flight requests to finish.*/
-   while (1) {
-   spin_lock(&fsvq->lock);
-   if (!fsvq->in_flight) {
-   spin_unlock(&fsvq->lock);
-   break;
-   }
+   spin_lock(&fsvq->lock);
+   if (fsvq->in_flight) {
+   /* We are holding virtio_fs_mutex. There should not be any
+* waiters waiting for completion.
+*/
+   reinit_completion(&fsvq->in_flight_zero);
+   spin_unlock(&fsvq->lock);
+   wait_for_completion(&fsvq->in_flight_zero);
+   } else {
spin_unlock(&fsvq->lock);
-   /* TODO use completion instead of timeout */
-   usleep_range(1000, 2000);
}
 
flush_work(&fsvq->done_work);
flush_delayed_work(&fsvq->dispatch_work);
 }
 
-static void virtio_fs_drain_all_queues(struct virtio_fs *fs)
+static void virtio_fs_drain_all_queues_locked(struct virtio_fs *fs)
 {
struct virtio_fs_vq *fsvq;
int i;
@@ -141,6 +145,19 @@ static void virtio_fs_drain_all_queues(struct virtio_fs 
*fs)
}
 }
 
+static void virtio_fs_drain_all_queues(struct virtio_fs *fs)
+{
+   /* Provides mutual exclusion between ->remove and ->kill_sb
+* paths. We don't want both of these draining queue at the
+* same time. Current completion logic reinits completion
+* and that means there should not be any other thread
+* doing reinit or waiting for completion already.
+*/
+   mutex_lock(&virtio_fs_mutex);
+   virtio_fs_drain_all_queues_locked(fs);
+   mutex_unlock(&virtio_fs_mutex);
+}
+
 static void virtio_fs_start_all_queues(struct virtio_fs *fs)
 {
struct virtio_fs_vq *fsvq;
@@ -581,6 +598,7 @@ static int virtio_fs_setup_vqs(struct virtio_device *vdev,
INIT_LIST_HEAD(&fs->vqs[VQ_HIPRIO].end_reqs);
INIT_DELAYED_WORK(&fs->vqs[VQ_HIPRIO].dispatch_work,
virtio_fs_hiprio_dispatch_work);
+   init_completion(&fs->vqs[VQ_HIPRIO].in_flight_zero);
spin_lock_init(&fs->vqs[VQ_HIPRIO].lock);
 
/* Initialize the requests virtqueues */
@@ -591,6 +609,7 @@ static int virtio_fs_setup_vqs(struct virtio_device *vdev,
  virtio_fs_request_dispatch_work);
INIT_LIST_HEAD(&fs->vqs[i].queued_reqs);
INIT_LIST_HEAD(&fs->vqs[i].end_reqs);
+   init_completion(&fs->vqs[i].in_flight_zero);
snprintf(fs->vqs[i].name, sizeof(fs->vqs[i].name),
 "requests.%u", i - VQ_REQUEST);
callbacks[i] = virtio_fs_vq_done;
@@ -684,7 +703,7 @@ static void virtio_fs_remove(struct virtio_device *vdev)
/* This device is going away. No one should get new reference */
list_del_init(&fs->list);
virtio_fs_stop_all_queues(fs);
-   virtio_fs_drain_all_queues(fs);
+   virtio_fs_drain_all_queues_locked(fs);
vdev->config->reset(vdev);
virtio_fs_cleanup_vqs(vdev, fs);
 
-- 
2.20.1

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


[PATCH 1/3] virtiofs: Use a common function to send forget

2019-10-30 Thread Vivek Goyal
Currently we are duplicating logic to send forgets at two places. Consolidate
the code by calling one helper function.

This also uses virtqueue_add_outbuf() instead of virtqueue_add_sgs(). Former
is simpler to call.

Signed-off-by: Vivek Goyal 
---
 fs/fuse/virtio_fs.c | 150 +++-
 1 file changed, 63 insertions(+), 87 deletions(-)

diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index a5c86048b96e..6cc7be170cb8 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -313,17 +313,71 @@ static void virtio_fs_request_dispatch_work(struct 
work_struct *work)
}
 }
 
+/*
+ * Returns 1 if queue is full and sender should wait a bit before sending
+ * next request, 0 otherwise.
+ */
+static int send_forget_request(struct virtio_fs_vq *fsvq,
+  struct virtio_fs_forget *forget,
+  bool in_flight)
+{
+   struct scatterlist sg;
+   struct virtqueue *vq;
+   int ret = 0;
+   bool notify;
+
+   spin_lock(&fsvq->lock);
+   if (!fsvq->connected) {
+   if (in_flight)
+   dec_in_flight_req(fsvq);
+   kfree(forget);
+   goto out;
+   }
+
+   sg_init_one(&sg, forget, sizeof(*forget));
+   vq = fsvq->vq;
+   dev_dbg(&vq->vdev->dev, "%s\n", __func__);
+
+   ret = virtqueue_add_outbuf(vq, &sg, 1, forget, GFP_ATOMIC);
+   if (ret < 0) {
+   if (ret == -ENOMEM || ret == -ENOSPC) {
+   pr_debug("virtio-fs: Could not queue FORGET: err=%d."
+" Will try later\n", ret);
+   list_add_tail(&forget->list, &fsvq->queued_reqs);
+   schedule_delayed_work(&fsvq->dispatch_work,
+ msecs_to_jiffies(1));
+   if (!in_flight)
+   inc_in_flight_req(fsvq);
+   /* Queue is full */
+   ret = 1;
+   } else {
+   pr_debug("virtio-fs: Could not queue FORGET: err=%d."
+" Dropping it.\n", ret);
+   kfree(forget);
+   if (in_flight)
+   dec_in_flight_req(fsvq);
+   }
+   goto out;
+   }
+
+   if (!in_flight)
+   inc_in_flight_req(fsvq);
+   notify = virtqueue_kick_prepare(vq);
+   spin_unlock(&fsvq->lock);
+
+   if (notify)
+   virtqueue_notify(vq);
+   return ret;
+out:
+   spin_unlock(&fsvq->lock);
+   return ret;
+}
+
 static void virtio_fs_hiprio_dispatch_work(struct work_struct *work)
 {
struct virtio_fs_forget *forget;
struct virtio_fs_vq *fsvq = container_of(work, struct virtio_fs_vq,
 dispatch_work.work);
-   struct virtqueue *vq = fsvq->vq;
-   struct scatterlist sg;
-   struct scatterlist *sgs[] = {&sg};
-   bool notify;
-   int ret;
-
pr_debug("virtio-fs: worker %s called.\n", __func__);
while (1) {
spin_lock(&fsvq->lock);
@@ -335,43 +389,9 @@ static void virtio_fs_hiprio_dispatch_work(struct 
work_struct *work)
}
 
list_del(&forget->list);
-   if (!fsvq->connected) {
-   dec_in_flight_req(fsvq);
-   spin_unlock(&fsvq->lock);
-   kfree(forget);
-   continue;
-   }
-
-   sg_init_one(&sg, forget, sizeof(*forget));
-
-   /* Enqueue the request */
-   dev_dbg(&vq->vdev->dev, "%s\n", __func__);
-   ret = virtqueue_add_sgs(vq, sgs, 1, 0, forget, GFP_ATOMIC);
-   if (ret < 0) {
-   if (ret == -ENOMEM || ret == -ENOSPC) {
-   pr_debug("virtio-fs: Could not queue FORGET: 
err=%d. Will try later\n",
-ret);
-   list_add_tail(&forget->list,
-   &fsvq->queued_reqs);
-   schedule_delayed_work(&fsvq->dispatch_work,
-   msecs_to_jiffies(1));
-   } else {
-   pr_debug("virtio-fs: Could not queue FORGET: 
err=%d. Dropping it.\n",
-ret);
-   dec_in_flight_req(fsvq);
-   kfree(forget);
-   }
-   spin_unlock(&fsvq->lock);
-   return;
-   }
-
-   notify = virtqueue_kick_prepare(vq);
spin_unlock(&fsvq->lock);
-
-   if (notify)
-   virtqueue_notify(vq);
-   pr_debug("virtio-fs: worker %s dispatched o

[PATCH 2/3] virtiofs: Do not send forget request "struct list_head" element

2019-10-30 Thread Vivek Goyal
We are sending whole of virtio_fs_foreget struct to the other end over
virtqueue. Other end does not need to see elements like "struct list".
That's internal detail of guest kernel. Fix it.

Signed-off-by: Vivek Goyal 
---
 fs/fuse/virtio_fs.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 6cc7be170cb8..43224db8d9ed 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -48,11 +48,15 @@ struct virtio_fs {
unsigned int num_request_queues; /* number of request queues */
 };
 
-struct virtio_fs_forget {
+struct virtio_fs_forget_req {
struct fuse_in_header ih;
struct fuse_forget_in arg;
+};
+
+struct virtio_fs_forget {
/* This request can be temporarily queued on virt queue */
struct list_head list;
+   struct virtio_fs_forget_req req;
 };
 
 static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq,
@@ -325,6 +329,7 @@ static int send_forget_request(struct virtio_fs_vq *fsvq,
struct virtqueue *vq;
int ret = 0;
bool notify;
+   struct virtio_fs_forget_req *req = &forget->req;
 
spin_lock(&fsvq->lock);
if (!fsvq->connected) {
@@ -334,7 +339,7 @@ static int send_forget_request(struct virtio_fs_vq *fsvq,
goto out;
}
 
-   sg_init_one(&sg, forget, sizeof(*forget));
+   sg_init_one(&sg, req, sizeof(*req));
vq = fsvq->vq;
dev_dbg(&vq->vdev->dev, "%s\n", __func__);
 
@@ -730,6 +735,7 @@ __releases(fiq->lock)
 {
struct fuse_forget_link *link;
struct virtio_fs_forget *forget;
+   struct virtio_fs_forget_req *req;
struct virtio_fs *fs;
struct virtio_fs_vq *fsvq;
u64 unique;
@@ -743,14 +749,15 @@ __releases(fiq->lock)
 
/* Allocate a buffer for the request */
forget = kmalloc(sizeof(*forget), GFP_NOFS | __GFP_NOFAIL);
+   req = &forget->req;
 
-   forget->ih = (struct fuse_in_header){
+   req->ih = (struct fuse_in_header){
.opcode = FUSE_FORGET,
.nodeid = link->forget_one.nodeid,
.unique = unique,
-   .len = sizeof(*forget),
+   .len = sizeof(*req),
};
-   forget->arg = (struct fuse_forget_in){
+   req->arg = (struct fuse_forget_in){
.nlookup = link->forget_one.nlookup,
};
 
-- 
2.20.1

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


RE: [PATCH net-next 06/14] vsock: add 'struct vsock_sock *' param to vsock_core_get_transport()

2019-10-30 Thread Jorgen Hansen via Virtualization
> From: Stefano Garzarella [mailto:sgarz...@redhat.com]
> Sent: Wednesday, October 23, 2019 11:56 AM
> Subject: [PATCH net-next 06/14] vsock: add 'struct vsock_sock *' param to
> vsock_core_get_transport()
> 
> Since now the 'struct vsock_sock' object contains a pointer to the transport,
> this patch adds a parameter to the
> vsock_core_get_transport() to return the right transport assigned to the
> socket.
> 
> This patch modifies also the virtio_transport_get_ops(), that uses the
> vsock_core_get_transport(), adding the 'struct vsock_sock *' parameter.
> 
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Stefano Garzarella 
> ---
> RFC -> v1:
> - Removed comment about protecting transport_single (Stefan)
> ---
>  include/net/af_vsock.h  | 2 +-
>  net/vmw_vsock/af_vsock.c| 7 ++-
>  net/vmw_vsock/virtio_transport_common.c | 9 +
>  3 files changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h index
> a5e1e134261d..2ca67d048de4 100644
> --- a/include/net/af_vsock.h
> +++ b/include/net/af_vsock.h
> @@ -166,7 +166,7 @@ static inline int vsock_core_init(const struct
> vsock_transport *t)  void vsock_core_exit(void);
> 
>  /* The transport may downcast this to access transport-specific functions */
> -const struct vsock_transport *vsock_core_get_transport(void);
> +const struct vsock_transport *vsock_core_get_transport(struct
> +vsock_sock *vsk);
> 
>  / UTILS /
> 
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index
> c3a14f853eb0..eaea159006c8 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -2001,12 +2001,9 @@ void vsock_core_exit(void)  }
> EXPORT_SYMBOL_GPL(vsock_core_exit);
> 
> -const struct vsock_transport *vsock_core_get_transport(void)
> +const struct vsock_transport *vsock_core_get_transport(struct
> +vsock_sock *vsk)
>  {
> - /* vsock_register_mutex not taken since only the transport uses this
> -  * function and only while registered.
> -  */
> - return transport_single;
> + return vsk->transport;
>  }
>  EXPORT_SYMBOL_GPL(vsock_core_get_transport);
> 
> diff --git a/net/vmw_vsock/virtio_transport_common.c
> b/net/vmw_vsock/virtio_transport_common.c
> index 9763394f7a61..37a1c7e7c7fe 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -29,9 +29,10 @@
>  /* Threshold for detecting small packets to copy */  #define
> GOOD_COPY_LEN  128
> 
> -static const struct virtio_transport *virtio_transport_get_ops(void)
> +static const struct virtio_transport *
> +virtio_transport_get_ops(struct vsock_sock *vsk)
>  {
> - const struct vsock_transport *t = vsock_core_get_transport();
> + const struct vsock_transport *t = vsock_core_get_transport(vsk);
> 
>   return container_of(t, struct virtio_transport, transport);  } @@ -
> 168,7 +169,7 @@ static int virtio_transport_send_pkt_info(struct vsock_sock
> *vsk,
>   struct virtio_vsock_pkt *pkt;
>   u32 pkt_len = info->pkt_len;
> 
> - src_cid = virtio_transport_get_ops()->transport.get_local_cid();
> + src_cid = virtio_transport_get_ops(vsk)->transport.get_local_cid();
>   src_port = vsk->local_addr.svm_port;
>   if (!info->remote_cid) {
>   dst_cid = vsk->remote_addr.svm_cid;
> @@ -201,7 +202,7 @@ static int virtio_transport_send_pkt_info(struct
> vsock_sock *vsk,
> 
>   virtio_transport_inc_tx_pkt(vvs, pkt);
> 
> - return virtio_transport_get_ops()->send_pkt(pkt);
> + return virtio_transport_get_ops(vsk)->send_pkt(pkt);
>  }
> 
>  static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
> --
> 2.21.0

Reviewed-by: Jorgen Hansen 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


RE: [PATCH net-next 04/14] vsock: add 'transport' member in the struct vsock_sock

2019-10-30 Thread Jorgen Hansen via Virtualization
> From: Stefano Garzarella [mailto:sgarz...@redhat.com]
> Sent: Wednesday, October 23, 2019 11:56 AM
> Subject: [PATCH net-next 04/14] vsock: add 'transport' member in the struct
> vsock_sock
> 
> As a preparation to support multiple transports, this patch adds the
> 'transport' member at the 'struct vsock_sock'.
> This new field is initialized during the creation in the
> __vsock_create() function.
> 
> This patch also renames the global 'transport' pointer to 'transport_single',
> since for now we're only supporting a single transport registered at run-time.
> 
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Stefano Garzarella 
> ---
>  include/net/af_vsock.h   |  1 +
>  net/vmw_vsock/af_vsock.c | 56 +++--
> ---
>  2 files changed, 39 insertions(+), 18 deletions(-)
> 
> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h index
> c660402b10f2..a5e1e134261d 100644
> --- a/include/net/af_vsock.h
> +++ b/include/net/af_vsock.h
> @@ -27,6 +27,7 @@ extern spinlock_t vsock_table_lock;  struct vsock_sock {
>   /* sk must be the first member. */
>   struct sock sk;
> + const struct vsock_transport *transport;
>   struct sockaddr_vm local_addr;
>   struct sockaddr_vm remote_addr;
>   /* Links for the global tables of bound and connected sockets. */ diff
> --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index
> 2f2582fb7fdd..c3a14f853eb0 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -126,7 +126,7 @@ static struct proto vsock_proto = {
>   */
>  #define VSOCK_DEFAULT_CONNECT_TIMEOUT (2 * HZ)
> 
> -static const struct vsock_transport *transport;
> +static const struct vsock_transport *transport_single;
>  static DEFINE_MUTEX(vsock_register_mutex);
> 
>  / UTILS /
> @@ -408,7 +408,9 @@ static bool vsock_is_pending(struct sock *sk)
> 
>  static int vsock_send_shutdown(struct sock *sk, int mode)  {
> - return transport->shutdown(vsock_sk(sk), mode);
> + struct vsock_sock *vsk = vsock_sk(sk);
> +
> + return vsk->transport->shutdown(vsk, mode);
>  }
> 
>  static void vsock_pending_work(struct work_struct *work) @@ -518,7
> +520,7 @@ static int __vsock_bind_stream(struct vsock_sock *vsk,  static int
> __vsock_bind_dgram(struct vsock_sock *vsk,
> struct sockaddr_vm *addr)
>  {
> - return transport->dgram_bind(vsk, addr);
> + return vsk->transport->dgram_bind(vsk, addr);
>  }
> 
>  static int __vsock_bind(struct sock *sk, struct sockaddr_vm *addr) @@ -
> 536,7 +538,7 @@ static int __vsock_bind(struct sock *sk, struct sockaddr_vm
> *addr)
>* like AF_INET prevents binding to a non-local IP address (in most
>* cases), we only allow binding to the local CID.
>*/
> - cid = transport->get_local_cid();
> + cid = vsk->transport->get_local_cid();
>   if (addr->svm_cid != cid && addr->svm_cid != VMADDR_CID_ANY)
>   return -EADDRNOTAVAIL;
> 
> @@ -586,6 +588,7 @@ struct sock *__vsock_create(struct net *net,
>   sk->sk_type = type;
> 
>   vsk = vsock_sk(sk);
> + vsk->transport = transport_single;
>   vsock_addr_init(&vsk->local_addr, VMADDR_CID_ANY,
> VMADDR_PORT_ANY);
>   vsock_addr_init(&vsk->remote_addr, VMADDR_CID_ANY,
> VMADDR_PORT_ANY);
> 
> @@ -616,7 +619,7 @@ struct sock *__vsock_create(struct net *net,
>   vsk->connect_timeout =
> VSOCK_DEFAULT_CONNECT_TIMEOUT;
>   }
> 
> - if (transport->init(vsk, psk) < 0) {
> + if (vsk->transport->init(vsk, psk) < 0) {
>   sk_free(sk);
>   return NULL;
>   }
> @@ -641,7 +644,7 @@ static void __vsock_release(struct sock *sk, int level)
>   /* The release call is supposed to use lock_sock_nested()
>* rather than lock_sock(), if a sock lock should be acquired.
>*/
> - transport->release(vsk);
> + vsk->transport->release(vsk);
> 
>   /* When "level" is SINGLE_DEPTH_NESTING, use the nested
>* version to avoid the warning "possible recursive locking
> @@ -670,7 +673,7 @@ static void vsock_sk_destruct(struct sock *sk)  {
>   struct vsock_sock *vsk = vsock_sk(sk);
> 
> - transport->destruct(vsk);
> + vsk->transport->destruct(vsk);
> 
>   /* When clearing these addresses, there's no need to set the family
> and
>* possibly register the address family with the kernel.
> @@ -694,13 +697,13 @@ static int vsock_queue_rcv_skb(struct sock *sk,
> struct sk_buff *skb)
> 
>  s64 vsock_stream_has_data(struct vsock_sock *vsk)  {
> - return transport->stream_has_data(vsk);
> + return vsk->transport->stream_has_data(vsk);
>  }
>  EXPORT_SYMBOL_GPL(vsock_stream_has_data);
> 
>  s64 vsock_stream_has_space(struct vsock_sock *vsk)  {
> - return transport->stream_has_space(vsk);
> + return vsk->transport->stream_has_space(vsk);
>  }
>  EXPORT_SYMBOL_GPL(vsock_stream_has_space);
> 
> @@ -869,6 +87

RE: [PATCH net-next 02/14] vsock: remove vm_sockets_get_local_cid()

2019-10-30 Thread Jorgen Hansen via Virtualization
> -Original Message-
> From: Stefano Garzarella [mailto:sgarz...@redhat.com]
> Sent: Wednesday, October 23, 2019 11:56 AM
> To: net...@vger.kernel.org
> Subject: [PATCH net-next 02/14] vsock: remove vm_sockets_get_local_cid()
> 
> vm_sockets_get_local_cid() is only used in virtio_transport_common.c.
> We can replace it calling the virtio_transport_get_ops() and using the
> get_local_cid() callback registered by the transport.
> 
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Stefano Garzarella 
> ---
>  include/linux/vm_sockets.h  |  2 --
>  net/vmw_vsock/af_vsock.c| 10 --
>  net/vmw_vsock/virtio_transport_common.c |  2 +-
>  3 files changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/include/linux/vm_sockets.h b/include/linux/vm_sockets.h index
> 33f1a2ecd905..7dd899ccb920 100644
> --- a/include/linux/vm_sockets.h
> +++ b/include/linux/vm_sockets.h
> @@ -10,6 +10,4 @@
> 
>  #include 
> 
> -int vm_sockets_get_local_cid(void);
> -
>  #endif /* _VM_SOCKETS_H */
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index
> 2ab43b2bba31..2f2582fb7fdd 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -129,16 +129,6 @@ static struct proto vsock_proto = {  static const struct
> vsock_transport *transport;  static DEFINE_MUTEX(vsock_register_mutex);
> 
> -/ EXPORTS /
> -
> -/* Get the ID of the local context.  This is transport dependent. */
> -
> -int vm_sockets_get_local_cid(void)
> -{
> - return transport->get_local_cid();
> -}
> -EXPORT_SYMBOL_GPL(vm_sockets_get_local_cid);
> -
>  / UTILS /
> 
>  /* Each bound VSocket is stored in the bind hash table and each connected
> diff --git a/net/vmw_vsock/virtio_transport_common.c
> b/net/vmw_vsock/virtio_transport_common.c
> index d02c9b41a768..b1cd16ed66ea 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -168,7 +168,7 @@ static int virtio_transport_send_pkt_info(struct
> vsock_sock *vsk,
>   struct virtio_vsock_pkt *pkt;
>   u32 pkt_len = info->pkt_len;
> 
> - src_cid = vm_sockets_get_local_cid();
> + src_cid = virtio_transport_get_ops()->transport.get_local_cid();
>   src_port = vsk->local_addr.svm_port;
>   if (!info->remote_cid) {
>   dst_cid = vsk->remote_addr.svm_cid;
> --
> 2.21.0

Reviewed-by: Jorgen Hansen 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


RE: [PATCH net-next 03/14] vsock: remove include/linux/vm_sockets.h file

2019-10-30 Thread Jorgen Hansen via Virtualization
> From: Stefano Garzarella [mailto:sgarz...@redhat.com]
> Sent: Wednesday, October 23, 2019 11:56 AM
> Subject: [PATCH net-next 03/14] vsock: remove include/linux/vm_sockets.h
> file
> 
> This header file now only includes the "uapi/linux/vm_sockets.h".
> We can include directly it when needed.
> 
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Stefano Garzarella 
> ---
>  include/linux/vm_sockets.h| 13 -
>  include/net/af_vsock.h|  2 +-
>  include/net/vsock_addr.h  |  2 +-
>  net/vmw_vsock/vmci_transport_notify.h |  1 -
>  4 files changed, 2 insertions(+), 16 deletions(-)  delete mode 100644
> include/linux/vm_sockets.h
> 
> diff --git a/include/linux/vm_sockets.h b/include/linux/vm_sockets.h
> deleted file mode 100644 index 7dd899ccb920..
> --- a/include/linux/vm_sockets.h
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/*
> - * VMware vSockets Driver
> - *
> - * Copyright (C) 2007-2013 VMware, Inc. All rights reserved.
> - */
> -
> -#ifndef _VM_SOCKETS_H
> -#define _VM_SOCKETS_H
> -
> -#include 
> -
> -#endif /* _VM_SOCKETS_H */
> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h index
> 80ea0f93d3f7..c660402b10f2 100644
> --- a/include/net/af_vsock.h
> +++ b/include/net/af_vsock.h
> @@ -10,7 +10,7 @@
> 
>  #include 
>  #include 
> -#include 
> +#include 
> 
>  #include "vsock_addr.h"
> 
> diff --git a/include/net/vsock_addr.h b/include/net/vsock_addr.h index
> 57d2db5c4bdf..cf8cc140d68d 100644
> --- a/include/net/vsock_addr.h
> +++ b/include/net/vsock_addr.h
> @@ -8,7 +8,7 @@
>  #ifndef _VSOCK_ADDR_H_
>  #define _VSOCK_ADDR_H_
> 
> -#include 
> +#include 
> 
>  void vsock_addr_init(struct sockaddr_vm *addr, u32 cid, u32 port);  int
> vsock_addr_validate(const struct sockaddr_vm *addr); diff --git
> a/net/vmw_vsock/vmci_transport_notify.h
> b/net/vmw_vsock/vmci_transport_notify.h
> index 7843f08d4290..a1aa5a998c0e 100644
> --- a/net/vmw_vsock/vmci_transport_notify.h
> +++ b/net/vmw_vsock/vmci_transport_notify.h
> @@ -11,7 +11,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
> 
>  #include "vmci_transport.h"
> 
> --
> 2.21.0

Reviewed-by: Jorgen Hansen 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


RE: [PATCH net-next 01/14] vsock/vmci: remove unused VSOCK_DEFAULT_CONNECT_TIMEOUT

2019-10-30 Thread Jorgen Hansen via Virtualization
> From: Stefano Garzarella [mailto:sgarz...@redhat.com]
> Sent: Wednesday, October 23, 2019 11:56 AM
> Subject: [PATCH net-next 01/14] vsock/vmci: remove unused
> VSOCK_DEFAULT_CONNECT_TIMEOUT
> 
> The VSOCK_DEFAULT_CONNECT_TIMEOUT definition was introduced with
> commit d021c344051af ("VSOCK: Introduce VM Sockets"), but it is never used
> in the net/vmw_vsock/vmci_transport.c.
> 
> VSOCK_DEFAULT_CONNECT_TIMEOUT is used and defined in
> net/vmw_vsock/af_vsock.c
> 
> Cc: Jorgen Hansen 
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Stefano Garzarella 
> ---
>  net/vmw_vsock/vmci_transport.c | 5 -
>  1 file changed, 5 deletions(-)
> 
> diff --git a/net/vmw_vsock/vmci_transport.c
> b/net/vmw_vsock/vmci_transport.c index 8c9c4ed90fa7..f8e3131ac480
> 100644
> --- a/net/vmw_vsock/vmci_transport.c
> +++ b/net/vmw_vsock/vmci_transport.c
> @@ -78,11 +78,6 @@ static int PROTOCOL_OVERRIDE = -1;
>  #define VMCI_TRANSPORT_DEFAULT_QP_SIZE   262144
>  #define VMCI_TRANSPORT_DEFAULT_QP_SIZE_MAX   262144
> 
> -/* The default peer timeout indicates how long we will wait for a peer
> response
> - * to a control message.
> - */
> -#define VSOCK_DEFAULT_CONNECT_TIMEOUT (2 * HZ)
> -
>  /* Helper function to convert from a VMCI error code to a VSock error code.
> */
> 
>  static s32 vmci_transport_error_to_vsock_error(s32 vmci_error)
> --
> 2.21.0

Reviewed-by: Jorgen Hansen 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC] vhost_mdev: add network control vq support

2019-10-30 Thread Tiwei Bie
On Wed, Oct 30, 2019 at 03:04:37PM +0800, Jason Wang wrote:
> On 2019/10/30 下午2:17, Tiwei Bie wrote:
> > On Tue, Oct 29, 2019 at 06:51:32PM +0800, Jason Wang wrote:
> >> On 2019/10/29 下午6:17, Tiwei Bie wrote:
> >>> This patch adds the network control vq support in vhost-mdev.
> >>> A vhost-mdev specific op is introduced to allow parent drivers
> >>> to handle the network control commands come from userspace.
> >> Probably work for userspace driver but not kernel driver.
> > Exactly. This is only for userspace.
> >
> > I got your point now. In virtio-mdev kernel driver case,
> > the ctrl-vq can be special as well.
> >
> 
> Then maybe it's better to introduce vhost-mdev-net on top?
> 
> Looking at the other type of virtio device:
> 
> - console have two control virtqueues when multiqueue port is enabled
> 
> - SCSI has controlq + eventq
> 
> - GPU has controlq
> 
> - Crypto device has one controlq
> 
> - Socket has eventq
> 
> ...

Thanks for the list! It looks dirty to define specific
commands and types in vhost UAPI for each of them in the
future. It's definitely much better to find an approach
to solve it once for all if possible..

Just a quick thought, considering all vhost-mdev does
is just to forward settings between parent and userspace,
I'm wondering whether it's possible to make the argp
opaque in vhost-mdev UAPI and just introduce one generic
ioctl command to deliver these device specific commands
(which are opaque in vhost-mdev as vhost-mdev just pass
the pointer -- argp) defined by spec.

I'm also fine with exposing ctrlq to userspace directly.
PS. It's interesting that some devices have more than
one ctrlq. I need to take a close look first..


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

Re: [PATCH] iommu/virtio: Remove unused variable

2019-10-30 Thread Joerg Roedel
On Fri, Oct 25, 2019 at 01:13:40PM -0300, Cristiane Naves wrote:
> Remove the variable of return. Issue found by
> coccicheck(scripts/coccinelle/misc/returnvar.cocci)
> 
> Signed-off-by: Cristiane Naves 
> ---
>  drivers/iommu/virtio-iommu.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Applied for v5.5, thanks.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3] vhost: introduce mdev based hardware backend

2019-10-30 Thread Jason Wang

On 2019/10/30 下午1:52, Tiwei Bie wrote:
> On Wed, Oct 30, 2019 at 09:55:57AM +0800, Jason Wang wrote:
>> On 2019/10/29 下午6:07, Tiwei Bie wrote:
>>> This patch introduces a mdev based hardware vhost backend.
>>> This backend is built on top of the same abstraction used
>>> in virtio-mdev and provides a generic vhost interface for
>>> userspace to accelerate the virtio devices in guest.
>>>
>>> This backend is implemented as a mdev device driver on top
>>> of the same mdev device ops used in virtio-mdev but using
>>> a different mdev class id, and it will register the device
>>> as a VFIO device for userspace to use. Userspace can setup
>>> the IOMMU with the existing VFIO container/group APIs and
>>> then get the device fd with the device name. After getting
>>> the device fd of this device, userspace can use vhost ioctls
>>> to setup the backend.
>>
>> Hi Tiwei:
>>
>> The patch looks good overall, just few comments & nits.
> Thanks for the review! I do appreciate it.


You're welcome :)


>
>>
>>> Signed-off-by: Tiwei Bie 
>>> ---
>>> This patch depends on below series:
>>> https://lkml.org/lkml/2019/10/23/614
>>>
>>> v2 -> v3:
>>> - Fix the return value (Jason);
>>> - Don't cache unnecessary information in vhost-mdev (Jason);
>>> - Get rid of the memset in open (Jason);
>>> - Add comments for VHOST_SET_MEM_TABLE, ... (Jason);
>>> - Filter out unsupported features in vhost-mdev (Jason);
>>> - Add _GET_DEVICE_ID ioctl (Jason);
>>> - Add _GET_CONFIG/_SET_CONFIG ioctls (Jason);
>>> - Drop _GET_QUEUE_NUM ioctl (Jason);
>>> - Fix the copy-paste errors in _IOW/_IOR usage;
>>> - Some minor fixes and improvements;
>>>
>>> v1 -> v2:
>>> - Replace _SET_STATE with _SET_STATUS (MST);
>>> - Check status bits at each step (MST);
>>> - Report the max ring size and max number of queues (MST);
>>> - Add missing MODULE_DEVICE_TABLE (Jason);
>>> - Only support the network backend w/o multiqueue for now;
>>> - Some minor fixes and improvements;
>>> - Rebase on top of virtio-mdev series v4;
>>>
>>> RFC v4 -> v1:
>>> - Implement vhost-mdev as a mdev device driver directly and
>>>   connect it to VFIO container/group. (Jason);
>>> - Pass ring addresses as GPAs/IOVAs in vhost-mdev to avoid
>>>   meaningless HVA->GPA translations (Jason);
>>>
>>> RFC v3 -> RFC v4:
>>> - Build vhost-mdev on top of the same abstraction used by
>>>   virtio-mdev (Jason);
>>> - Introduce vhost fd and pass VFIO fd via SET_BACKEND ioctl (MST);
>>>
>>> RFC v2 -> RFC v3:
>>> - Reuse vhost's ioctls instead of inventing a VFIO regions/irqs
>>>   based vhost protocol on top of vfio-mdev (Jason);
>>>
>>> RFC v1 -> RFC v2:
>>> - Introduce a new VFIO device type to build a vhost protocol
>>>   on top of vfio-mdev;
>>>
>>>  drivers/vfio/mdev/mdev_core.c|  20 ++
>>>  drivers/vfio/mdev/mdev_private.h |   1 +
>>>  drivers/vhost/Kconfig|  12 +
>>>  drivers/vhost/Makefile   |   3 +
>>>  drivers/vhost/mdev.c | 554 +++
>>>  include/linux/mdev.h |   5 +
>>>  include/uapi/linux/vhost.h   |  18 +
>>>  include/uapi/linux/vhost_types.h |   8 +
>>>  8 files changed, 621 insertions(+)
>>>  create mode 100644 drivers/vhost/mdev.c
>>>
>>> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
>>> index 9b00c3513120..3cfd787d605c 100644
>>> --- a/drivers/vfio/mdev/mdev_core.c
>>> +++ b/drivers/vfio/mdev/mdev_core.c
>>> @@ -96,6 +96,26 @@ mdev_get_virtio_ops(struct mdev_device *mdev)
>>>  }
>>>  EXPORT_SYMBOL(mdev_get_virtio_ops);
>>>  
>>> +/* Specify the vhost device ops for the mdev device, this
>>> + * must be called during create() callback for vhost mdev device.
>>> + */
>>> +void mdev_set_vhost_ops(struct mdev_device *mdev,
>>> +   const struct virtio_mdev_device_ops *vhost_ops)
>>> +{
>>> +   mdev_set_class(mdev, MDEV_CLASS_ID_VHOST);
>>> +   mdev->vhost_ops = vhost_ops;
>>> +}
>>> +EXPORT_SYMBOL(mdev_set_vhost_ops);
>>> +
>>> +/* Get the vhost device ops for the mdev device. */
>>> +const struct virtio_mdev_device_ops *
>>> +mdev_get_vhost_ops(struct mdev_device *mdev)
>>> +{
>>> +   WARN_ON(mdev->class_id != MDEV_CLASS_ID_VHOST);
>>> +   return mdev->vhost_ops;
>>> +}
>>> +EXPORT_SYMBOL(mdev_get_vhost_ops);
>>> +
>>>  struct device *mdev_dev(struct mdev_device *mdev)
>>>  {
>>> return &mdev->dev;
>>> diff --git a/drivers/vfio/mdev/mdev_private.h 
>>> b/drivers/vfio/mdev/mdev_private.h
>>> index 7b47890c34e7..5597c846e52f 100644
>>> --- a/drivers/vfio/mdev/mdev_private.h
>>> +++ b/drivers/vfio/mdev/mdev_private.h
>>> @@ -40,6 +40,7 @@ struct mdev_device {
>>> union {
>>> const struct vfio_mdev_device_ops *vfio_ops;
>>> const struct virtio_mdev_device_ops *virtio_ops;
>>> +   const struct virtio_mdev_device_ops *vhost_ops;
>>> };
>>>  };
>>>  
>>> diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
>>> index 3d03ccbd1adc..062cada28f89 100644
>>> --- a/drivers/vhost/Kconfig
>>> +++ b/drivers/vhost/Kconfig
>>>

Re: [RFC] vhost_mdev: add network control vq support

2019-10-30 Thread Jason Wang

On 2019/10/30 下午2:17, Tiwei Bie wrote:
> On Tue, Oct 29, 2019 at 06:51:32PM +0800, Jason Wang wrote:
>> On 2019/10/29 下午6:17, Tiwei Bie wrote:
>>> This patch adds the network control vq support in vhost-mdev.
>>> A vhost-mdev specific op is introduced to allow parent drivers
>>> to handle the network control commands come from userspace.
>> Probably work for userspace driver but not kernel driver.
> Exactly. This is only for userspace.
>
> I got your point now. In virtio-mdev kernel driver case,
> the ctrl-vq can be special as well.
>

Then maybe it's better to introduce vhost-mdev-net on top?

Looking at the other type of virtio device:

- console have two control virtqueues when multiqueue port is enabled

- SCSI has controlq + eventq

- GPU has controlq

- Crypto device has one controlq

- Socket has eventq

...

Thanks

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