| Message ID | 1441697927-16456-8-git-send-email-yuanhan.liu@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Sep 08, 2015 at 03:38:47PM +0800, Yuanhan Liu wrote: > From: Changchun Ouyang <changchun.ouyang@intel.com> > > Add a new message, VHOST_USER_SET_VRING_FLAG, to enable and disable > a specific virt queue, which is similar to attach/detach queue for > tap device. > > virtio driver on guest doesn't have to use max virt queue pair, it > could enable any number of virt queue ranging from 1 to max virt > queue pair. > > Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com> > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com> Fine, but I would like to make this dependent on the MQ flag. > --- > docs/specs/vhost-user.txt | 10 ++++++++++ > hw/net/vhost_net.c | 18 ++++++++++++++++++ > hw/net/virtio-net.c | 2 ++ > hw/virtio/vhost-user.c | 32 ++++++++++++++++++++++++++++---- > include/hw/virtio/vhost-backend.h | 2 ++ > include/net/vhost_net.h | 1 + > 6 files changed, 61 insertions(+), 4 deletions(-) > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt > index 99d79be..c2c38f3 100644 > --- a/docs/specs/vhost-user.txt > +++ b/docs/specs/vhost-user.txt > @@ -325,3 +325,13 @@ Message types > Query how many queues the backend supports. This request should be > sent only when VHOST_USER_PROTOCOL_F_MQ is set in quried protocol > features by VHOST_USER_GET_PROTOCOL_FEATURES. > + > + * VHOST_USER_SET_VRING_FLAG SET_VRIHG_ENABLE would be a better name. > + > + Id: 18 > + Equivalent ioctl: N/A > + Master payload: vring state description > + > + Set the flag(1 for enable and 0 for disable) to signal slave to enable Space before ( please. > + or disable corresponding virt queue. This request should be sent only > + when the protocol feature bit VHOST_USER_PROTOCOL_F_VRING_FLAG is set. > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c > index 141b557..7d9cc8d 100644 > --- a/hw/net/vhost_net.c > +++ b/hw/net/vhost_net.c > @@ -418,6 +418,19 @@ VHostNetState *get_vhost_net(NetClientState *nc) > > return vhost_net; > } > + > +int vhost_set_vring_flag(NetClientState *nc, int enable) > +{ > + if (nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) { > + VHostNetState *net = get_vhost_net(nc); > + const VhostOps *vhost_ops = net->dev.vhost_ops; add an empty line after declaration pls. > + if (vhost_ops->vhost_backend_mq_set_vring_flag) > + return vhost_ops->vhost_backend_mq_set_vring_flag(&net->dev, enable); Coding style violation: line too long, and should use {}. > + } > + > + return 0; > +} > + > #else > struct vhost_net *vhost_net_init(VhostNetOptions *options) > { > @@ -463,4 +476,9 @@ VHostNetState *get_vhost_net(NetClientState *nc) > { > return 0; > } > + > +int vhost_set_vring_flag(NetClientState *nc, int enable) > +{ > + return 0; > +} > #endif > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 8d28e45..53f93b1 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -407,6 +407,7 @@ static int peer_attach(VirtIONet *n, int index) > } > > if (nc->peer->info->type != NET_CLIENT_OPTIONS_KIND_TAP) { > + vhost_set_vring_flag(nc->peer, 1); > return 0; > } > > @@ -422,6 +423,7 @@ static int peer_detach(VirtIONet *n, int index) > } > > if (nc->peer->info->type != NET_CLIENT_OPTIONS_KIND_TAP) { > + vhost_set_vring_flag(nc->peer, 0); > return 0; > } > makes no sense to call it for !== tap only. Either call this unconditionally, or just for vhost user. > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index 11e46b5..ca6f7fa 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -25,9 +25,10 @@ > > #define VHOST_MEMORY_MAX_NREGIONS 8 > #define VHOST_USER_F_PROTOCOL_FEATURES 30 > -#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x1ULL > +#define VHOST_USER_PROTOCOL_FEATURE_MASK 0xfULL > > -#define VHOST_USER_PROTOCOL_F_MQ 0 > +#define VHOST_USER_PROTOCOL_F_MQ 0 > +#define VHOST_USER_PROTOCOL_F_VRING_FLAG 1 > > typedef enum VhostUserRequest { > VHOST_USER_NONE = 0, > @@ -48,6 +49,7 @@ typedef enum VhostUserRequest { > VHOST_USER_GET_PROTOCOL_FEATURES = 15, > VHOST_USER_SET_PROTOCOL_FEATURES = 16, > VHOST_USER_GET_QUEUE_NUM = 17, > + VHOST_USER_SET_VRING_FLAG = 18, > VHOST_USER_MAX > } VhostUserRequest; > > @@ -296,6 +298,11 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request, > msg.addr.index += dev->vq_index; > msg.size = sizeof(m.state); > break; > + case VHOST_USER_SET_VRING_FLAG: > + msg.state.index = dev->vq_index; > + msg.state.num = *(int*)arg; > + msg.size = sizeof(msg.state); > + break; > > case VHOST_USER_GET_VRING_BASE: > memcpy(&msg.state, arg, sizeof(struct vhost_vring_state)); > @@ -408,6 +415,22 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque) > return 0; > } > > +static int vhost_user_set_vring_flag(struct vhost_dev *dev, int enable) > +{ > + int err; > + > + assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); > + > + if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_VRING_FLAG))) > + return -1; > + > + err = vhost_user_call(dev, VHOST_USER_SET_VRING_FLAG, &enable); > + if (err < 0) > + return err; > + > + return 0; > +} > + > static int vhost_user_cleanup(struct vhost_dev *dev) > { > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); In fact, it's a per VQ pair call the way it's coded. So pls fix it, call it for all VQs. > @@ -421,5 +444,6 @@ const VhostOps user_ops = { > .backend_type = VHOST_BACKEND_TYPE_USER, > .vhost_call = vhost_user_call, > .vhost_backend_init = vhost_user_init, > - .vhost_backend_cleanup = vhost_user_cleanup > - }; > + .vhost_backend_cleanup = vhost_user_cleanup, > + .vhost_backend_mq_set_vring_flag = vhost_user_set_vring_flag, > +}; > diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h > index e472f29..6fc76b7 100644 > --- a/include/hw/virtio/vhost-backend.h > +++ b/include/hw/virtio/vhost-backend.h > @@ -24,12 +24,14 @@ typedef int (*vhost_call)(struct vhost_dev *dev, unsigned long int request, > void *arg); > typedef int (*vhost_backend_init)(struct vhost_dev *dev, void *opaque); > typedef int (*vhost_backend_cleanup)(struct vhost_dev *dev); > +typedef int (*vhost_backend_mq_set_vring_flag)(struct vhost_dev *dev, int enable); > > typedef struct VhostOps { > VhostBackendType backend_type; > vhost_call vhost_call; > vhost_backend_init vhost_backend_init; > vhost_backend_cleanup vhost_backend_cleanup; > + vhost_backend_mq_set_vring_flag vhost_backend_mq_set_vring_flag; That's quite ugly: mq is a vhost net thing. Why not enable each VQ? > } VhostOps; > > extern const VhostOps user_ops; > diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h > index 8db723e..3ec33ff 100644 > --- a/include/net/vhost_net.h > +++ b/include/net/vhost_net.h > @@ -28,4 +28,5 @@ bool vhost_net_virtqueue_pending(VHostNetState *net, int n); > void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev, > int idx, bool mask); > VHostNetState *get_vhost_net(NetClientState *nc); > +int vhost_set_vring_flag(NetClientState * nc, int enable); > #endif > -- > 1.9.0
On Wed, Sep 09, 2015 at 01:45:50PM +0300, Michael S. Tsirkin wrote: > On Tue, Sep 08, 2015 at 03:38:47PM +0800, Yuanhan Liu wrote: > > From: Changchun Ouyang <changchun.ouyang@intel.com> > > > > Add a new message, VHOST_USER_SET_VRING_FLAG, to enable and disable > > a specific virt queue, which is similar to attach/detach queue for > > tap device. > > > > virtio driver on guest doesn't have to use max virt queue pair, it > > could enable any number of virt queue ranging from 1 to max virt > > queue pair. > > > > Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com> > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com> > > Fine, but I would like to make this dependent on the MQ flag. Reasonable. > > > > --- > > docs/specs/vhost-user.txt | 10 ++++++++++ > > hw/net/vhost_net.c | 18 ++++++++++++++++++ > > hw/net/virtio-net.c | 2 ++ > > hw/virtio/vhost-user.c | 32 ++++++++++++++++++++++++++++---- > > include/hw/virtio/vhost-backend.h | 2 ++ > > include/net/vhost_net.h | 1 + > > 6 files changed, 61 insertions(+), 4 deletions(-) > > > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt > > index 99d79be..c2c38f3 100644 > > --- a/docs/specs/vhost-user.txt > > +++ b/docs/specs/vhost-user.txt > > @@ -325,3 +325,13 @@ Message types > > Query how many queues the backend supports. This request should be > > sent only when VHOST_USER_PROTOCOL_F_MQ is set in quried protocol > > features by VHOST_USER_GET_PROTOCOL_FEATURES. > > + > > + * VHOST_USER_SET_VRING_FLAG > > > SET_VRIHG_ENABLE would be a better name. Okay. > > > + > > + Id: 18 > > + Equivalent ioctl: N/A > > + Master payload: vring state description > > + > > + Set the flag(1 for enable and 0 for disable) to signal slave to enable > > Space before ( please. Okay. > > > + or disable corresponding virt queue. This request should be sent only > > + when the protocol feature bit VHOST_USER_PROTOCOL_F_VRING_FLAG is set. > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c > > index 141b557..7d9cc8d 100644 > > --- a/hw/net/vhost_net.c > > +++ b/hw/net/vhost_net.c > > @@ -418,6 +418,19 @@ VHostNetState *get_vhost_net(NetClientState *nc) > > > > return vhost_net; > > } > > + > > +int vhost_set_vring_flag(NetClientState *nc, int enable) > > +{ > > + if (nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) { > > + VHostNetState *net = get_vhost_net(nc); > > + const VhostOps *vhost_ops = net->dev.vhost_ops; > > add an empty line after declaration pls. Got it, thanks. > > > + if (vhost_ops->vhost_backend_mq_set_vring_flag) > > + return vhost_ops->vhost_backend_mq_set_vring_flag(&net->dev, enable); > > Coding style violation: > line too long, Yeah, 81 chars. I thougth that's kind of acceptable. But, anyway, since you pointed it out, I will fix it next time. > and should use {}. TBH, I seldom use {} for one statement, and you could see that a lot from this patch set. Is it a must in qemu community? If so, I could fix them all. > > > + } > > + > > + return 0; > > +} > > + > > #else > > struct vhost_net *vhost_net_init(VhostNetOptions *options) > > { > > @@ -463,4 +476,9 @@ VHostNetState *get_vhost_net(NetClientState *nc) > > { > > return 0; > > } > > + > > +int vhost_set_vring_flag(NetClientState *nc, int enable) > > +{ > > + return 0; > > +} > > #endif > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > > index 8d28e45..53f93b1 100644 > > --- a/hw/net/virtio-net.c > > +++ b/hw/net/virtio-net.c > > @@ -407,6 +407,7 @@ static int peer_attach(VirtIONet *n, int index) > > } > > > > if (nc->peer->info->type != NET_CLIENT_OPTIONS_KIND_TAP) { > > + vhost_set_vring_flag(nc->peer, 1); > > return 0; > > } > > > > @@ -422,6 +423,7 @@ static int peer_detach(VirtIONet *n, int index) > > } > > > > if (nc->peer->info->type != NET_CLIENT_OPTIONS_KIND_TAP) { > > + vhost_set_vring_flag(nc->peer, 0); > > return 0; > > } > > > > makes no sense to call it for !== tap only. > Either call this unconditionally, or just for vhost user. Agreed. I will make it vhost-usre specific then. > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > index 11e46b5..ca6f7fa 100644 > > --- a/hw/virtio/vhost-user.c > > +++ b/hw/virtio/vhost-user.c > > @@ -25,9 +25,10 @@ > > > > #define VHOST_MEMORY_MAX_NREGIONS 8 > > #define VHOST_USER_F_PROTOCOL_FEATURES 30 > > -#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x1ULL > > +#define VHOST_USER_PROTOCOL_FEATURE_MASK 0xfULL > > > > -#define VHOST_USER_PROTOCOL_F_MQ 0 > > +#define VHOST_USER_PROTOCOL_F_MQ 0 > > +#define VHOST_USER_PROTOCOL_F_VRING_FLAG 1 > > > > typedef enum VhostUserRequest { > > VHOST_USER_NONE = 0, > > @@ -48,6 +49,7 @@ typedef enum VhostUserRequest { > > VHOST_USER_GET_PROTOCOL_FEATURES = 15, > > VHOST_USER_SET_PROTOCOL_FEATURES = 16, > > VHOST_USER_GET_QUEUE_NUM = 17, > > + VHOST_USER_SET_VRING_FLAG = 18, > > VHOST_USER_MAX > > } VhostUserRequest; > > > > @@ -296,6 +298,11 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request, > > msg.addr.index += dev->vq_index; > > msg.size = sizeof(m.state); > > break; > > + case VHOST_USER_SET_VRING_FLAG: > > + msg.state.index = dev->vq_index; > > + msg.state.num = *(int*)arg; > > + msg.size = sizeof(msg.state); > > + break; > > > > case VHOST_USER_GET_VRING_BASE: > > memcpy(&msg.state, arg, sizeof(struct vhost_vring_state)); > > @@ -408,6 +415,22 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque) > > return 0; > > } > > > > +static int vhost_user_set_vring_flag(struct vhost_dev *dev, int enable) > > +{ > > + int err; > > + > > + assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); > > + > > + if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_VRING_FLAG))) > > + return -1; > > + > > + err = vhost_user_call(dev, VHOST_USER_SET_VRING_FLAG, &enable); > > + if (err < 0) > > + return err; > > + > > + return 0; > > +} > > + > > static int vhost_user_cleanup(struct vhost_dev *dev) > > { > > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); > > In fact, it's a per VQ pair call the way it's coded. > So pls fix it, call it for all VQs. Let me try. > > > > @@ -421,5 +444,6 @@ const VhostOps user_ops = { > > .backend_type = VHOST_BACKEND_TYPE_USER, > > .vhost_call = vhost_user_call, > > .vhost_backend_init = vhost_user_init, > > - .vhost_backend_cleanup = vhost_user_cleanup > > - }; > > + .vhost_backend_cleanup = vhost_user_cleanup, > > + .vhost_backend_mq_set_vring_flag = vhost_user_set_vring_flag, > > +}; > > diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h > > index e472f29..6fc76b7 100644 > > --- a/include/hw/virtio/vhost-backend.h > > +++ b/include/hw/virtio/vhost-backend.h > > @@ -24,12 +24,14 @@ typedef int (*vhost_call)(struct vhost_dev *dev, unsigned long int request, > > void *arg); > > typedef int (*vhost_backend_init)(struct vhost_dev *dev, void *opaque); > > typedef int (*vhost_backend_cleanup)(struct vhost_dev *dev); > > +typedef int (*vhost_backend_mq_set_vring_flag)(struct vhost_dev *dev, int enable); > > > > typedef struct VhostOps { > > VhostBackendType backend_type; > > vhost_call vhost_call; > > vhost_backend_init vhost_backend_init; > > vhost_backend_cleanup vhost_backend_cleanup; > > + vhost_backend_mq_set_vring_flag vhost_backend_mq_set_vring_flag; > > That's quite ugly: mq is a vhost net thing. > Why not enable each VQ? To define it as `vhost_backend_enable_vring'? Thanks. --yliu > > > } VhostOps; > > > > extern const VhostOps user_ops; > > diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h > > index 8db723e..3ec33ff 100644 > > --- a/include/net/vhost_net.h > > +++ b/include/net/vhost_net.h > > @@ -28,4 +28,5 @@ bool vhost_net_virtqueue_pending(VHostNetState *net, int n); > > void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev, > > int idx, bool mask); > > VHostNetState *get_vhost_net(NetClientState *nc); > > +int vhost_set_vring_flag(NetClientState * nc, int enable); > > #endif > > -- > > 1.9.0
On Wed, Sep 09, 2015 at 09:38:12PM +0800, Yuanhan Liu wrote: > On Wed, Sep 09, 2015 at 01:45:50PM +0300, Michael S. Tsirkin wrote: > > On Tue, Sep 08, 2015 at 03:38:47PM +0800, Yuanhan Liu wrote: > > > From: Changchun Ouyang <changchun.ouyang@intel.com> > > > > > > Add a new message, VHOST_USER_SET_VRING_FLAG, to enable and disable > > > a specific virt queue, which is similar to attach/detach queue for > > > tap device. > > > > > > virtio driver on guest doesn't have to use max virt queue pair, it > > > could enable any number of virt queue ranging from 1 to max virt > > > queue pair. > > > > > > Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com> > > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com> > > > > Fine, but I would like to make this dependent on the MQ flag. > > Reasonable. > > > > > > > > --- > > > docs/specs/vhost-user.txt | 10 ++++++++++ > > > hw/net/vhost_net.c | 18 ++++++++++++++++++ > > > hw/net/virtio-net.c | 2 ++ > > > hw/virtio/vhost-user.c | 32 ++++++++++++++++++++++++++++---- > > > include/hw/virtio/vhost-backend.h | 2 ++ > > > include/net/vhost_net.h | 1 + > > > 6 files changed, 61 insertions(+), 4 deletions(-) > > > > > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt > > > index 99d79be..c2c38f3 100644 > > > --- a/docs/specs/vhost-user.txt > > > +++ b/docs/specs/vhost-user.txt > > > @@ -325,3 +325,13 @@ Message types > > > Query how many queues the backend supports. This request should be > > > sent only when VHOST_USER_PROTOCOL_F_MQ is set in quried protocol > > > features by VHOST_USER_GET_PROTOCOL_FEATURES. > > > + > > > + * VHOST_USER_SET_VRING_FLAG > > > > > > SET_VRIHG_ENABLE would be a better name. > > Okay. > > > > > > + > > > + Id: 18 > > > + Equivalent ioctl: N/A > > > + Master payload: vring state description > > > + > > > + Set the flag(1 for enable and 0 for disable) to signal slave to enable > > > > Space before ( please. > > Okay. > > > > > > + or disable corresponding virt queue. This request should be sent only > > > + when the protocol feature bit VHOST_USER_PROTOCOL_F_VRING_FLAG is set. > > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c > > > index 141b557..7d9cc8d 100644 > > > --- a/hw/net/vhost_net.c > > > +++ b/hw/net/vhost_net.c > > > @@ -418,6 +418,19 @@ VHostNetState *get_vhost_net(NetClientState *nc) > > > > > > return vhost_net; > > > } > > > + > > > +int vhost_set_vring_flag(NetClientState *nc, int enable) > > > +{ > > > + if (nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) { > > > + VHostNetState *net = get_vhost_net(nc); > > > + const VhostOps *vhost_ops = net->dev.vhost_ops; > > > > add an empty line after declaration pls. > > Got it, thanks. > > > > > > + if (vhost_ops->vhost_backend_mq_set_vring_flag) > > > + return vhost_ops->vhost_backend_mq_set_vring_flag(&net->dev, enable); > > > > Coding style violation: > > line too long, > > Yeah, 81 chars. I thougth that's kind of acceptable. But, anyway, since > you pointed it out, I will fix it next time. > > > and should use {}. > > TBH, I seldom use {} for one statement, and you could see that a lot from > this patch set. Is it a must in qemu community? Yes it is. It's different from the kernel. > If so, I could fix them all. > > > > > > + } > > > + > > > + return 0; > > > +} > > > + > > > #else > > > struct vhost_net *vhost_net_init(VhostNetOptions *options) > > > { > > > @@ -463,4 +476,9 @@ VHostNetState *get_vhost_net(NetClientState *nc) > > > { > > > return 0; > > > } > > > + > > > +int vhost_set_vring_flag(NetClientState *nc, int enable) > > > +{ > > > + return 0; > > > +} > > > #endif > > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > > > index 8d28e45..53f93b1 100644 > > > --- a/hw/net/virtio-net.c > > > +++ b/hw/net/virtio-net.c > > > @@ -407,6 +407,7 @@ static int peer_attach(VirtIONet *n, int index) > > > } > > > > > > if (nc->peer->info->type != NET_CLIENT_OPTIONS_KIND_TAP) { > > > + vhost_set_vring_flag(nc->peer, 1); > > > return 0; > > > } > > > > > > @@ -422,6 +423,7 @@ static int peer_detach(VirtIONet *n, int index) > > > } > > > > > > if (nc->peer->info->type != NET_CLIENT_OPTIONS_KIND_TAP) { > > > + vhost_set_vring_flag(nc->peer, 0); > > > return 0; > > > } > > > > > > > makes no sense to call it for !== tap only. > > Either call this unconditionally, or just for vhost user. > > Agreed. I will make it vhost-usre specific then. > > > > > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > > index 11e46b5..ca6f7fa 100644 > > > --- a/hw/virtio/vhost-user.c > > > +++ b/hw/virtio/vhost-user.c > > > @@ -25,9 +25,10 @@ > > > > > > #define VHOST_MEMORY_MAX_NREGIONS 8 > > > #define VHOST_USER_F_PROTOCOL_FEATURES 30 > > > -#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x1ULL > > > +#define VHOST_USER_PROTOCOL_FEATURE_MASK 0xfULL > > > > > > -#define VHOST_USER_PROTOCOL_F_MQ 0 > > > +#define VHOST_USER_PROTOCOL_F_MQ 0 > > > +#define VHOST_USER_PROTOCOL_F_VRING_FLAG 1 > > > > > > typedef enum VhostUserRequest { > > > VHOST_USER_NONE = 0, > > > @@ -48,6 +49,7 @@ typedef enum VhostUserRequest { > > > VHOST_USER_GET_PROTOCOL_FEATURES = 15, > > > VHOST_USER_SET_PROTOCOL_FEATURES = 16, > > > VHOST_USER_GET_QUEUE_NUM = 17, > > > + VHOST_USER_SET_VRING_FLAG = 18, > > > VHOST_USER_MAX > > > } VhostUserRequest; > > > > > > @@ -296,6 +298,11 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request, > > > msg.addr.index += dev->vq_index; > > > msg.size = sizeof(m.state); > > > break; > > > + case VHOST_USER_SET_VRING_FLAG: > > > + msg.state.index = dev->vq_index; > > > + msg.state.num = *(int*)arg; > > > + msg.size = sizeof(msg.state); > > > + break; > > > > > > case VHOST_USER_GET_VRING_BASE: > > > memcpy(&msg.state, arg, sizeof(struct vhost_vring_state)); > > > @@ -408,6 +415,22 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque) > > > return 0; > > > } > > > > > > +static int vhost_user_set_vring_flag(struct vhost_dev *dev, int enable) > > > +{ > > > + int err; > > > + > > > + assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); > > > + > > > + if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_VRING_FLAG))) > > > + return -1; > > > + > > > + err = vhost_user_call(dev, VHOST_USER_SET_VRING_FLAG, &enable); > > > + if (err < 0) > > > + return err; > > > + > > > + return 0; > > > +} > > > + > > > static int vhost_user_cleanup(struct vhost_dev *dev) > > > { > > > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); > > > > In fact, it's a per VQ pair call the way it's coded. > > So pls fix it, call it for all VQs. > > Let me try. > > > > > > > > @@ -421,5 +444,6 @@ const VhostOps user_ops = { > > > .backend_type = VHOST_BACKEND_TYPE_USER, > > > .vhost_call = vhost_user_call, > > > .vhost_backend_init = vhost_user_init, > > > - .vhost_backend_cleanup = vhost_user_cleanup > > > - }; > > > + .vhost_backend_cleanup = vhost_user_cleanup, > > > + .vhost_backend_mq_set_vring_flag = vhost_user_set_vring_flag, > > > +}; > > > diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h > > > index e472f29..6fc76b7 100644 > > > --- a/include/hw/virtio/vhost-backend.h > > > +++ b/include/hw/virtio/vhost-backend.h > > > @@ -24,12 +24,14 @@ typedef int (*vhost_call)(struct vhost_dev *dev, unsigned long int request, > > > void *arg); > > > typedef int (*vhost_backend_init)(struct vhost_dev *dev, void *opaque); > > > typedef int (*vhost_backend_cleanup)(struct vhost_dev *dev); > > > +typedef int (*vhost_backend_mq_set_vring_flag)(struct vhost_dev *dev, int enable); > > > > > > typedef struct VhostOps { > > > VhostBackendType backend_type; > > > vhost_call vhost_call; > > > vhost_backend_init vhost_backend_init; > > > vhost_backend_cleanup vhost_backend_cleanup; > > > + vhost_backend_mq_set_vring_flag vhost_backend_mq_set_vring_flag; > > > > That's quite ugly: mq is a vhost net thing. > > Why not enable each VQ? > > To define it as `vhost_backend_enable_vring'? > > Thanks. > > --yliu Sure. > > > > > } VhostOps; > > > > > > extern const VhostOps user_ops; > > > diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h > > > index 8db723e..3ec33ff 100644 > > > --- a/include/net/vhost_net.h > > > +++ b/include/net/vhost_net.h > > > @@ -28,4 +28,5 @@ bool vhost_net_virtqueue_pending(VHostNetState *net, int n); > > > void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev, > > > int idx, bool mask); > > > VHostNetState *get_vhost_net(NetClientState *nc); > > > +int vhost_set_vring_flag(NetClientState * nc, int enable); > > > #endif > > > -- > > > 1.9.0
On Wed, Sep 09, 2015 at 09:38:12PM +0800, Yuanhan Liu wrote: > On Wed, Sep 09, 2015 at 01:45:50PM +0300, Michael S. Tsirkin wrote: > > On Tue, Sep 08, 2015 at 03:38:47PM +0800, Yuanhan Liu wrote: > > > From: Changchun Ouyang <changchun.ouyang@intel.com> > > > > > > Add a new message, VHOST_USER_SET_VRING_FLAG, to enable and disable > > > a specific virt queue, which is similar to attach/detach queue for > > > tap device. > > > > > > virtio driver on guest doesn't have to use max virt queue pair, it > > > could enable any number of virt queue ranging from 1 to max virt > > > queue pair. > > > > > > Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com> > > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com> > > > > Fine, but I would like to make this dependent on the MQ flag. > > Reasonable. > > > > > > > > --- > > > docs/specs/vhost-user.txt | 10 ++++++++++ > > > hw/net/vhost_net.c | 18 ++++++++++++++++++ > > > hw/net/virtio-net.c | 2 ++ > > > hw/virtio/vhost-user.c | 32 ++++++++++++++++++++++++++++---- > > > include/hw/virtio/vhost-backend.h | 2 ++ > > > include/net/vhost_net.h | 1 + > > > 6 files changed, 61 insertions(+), 4 deletions(-) > > > > > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt > > > index 99d79be..c2c38f3 100644 > > > --- a/docs/specs/vhost-user.txt > > > +++ b/docs/specs/vhost-user.txt > > > @@ -325,3 +325,13 @@ Message types > > > Query how many queues the backend supports. This request should be > > > sent only when VHOST_USER_PROTOCOL_F_MQ is set in quried protocol > > > features by VHOST_USER_GET_PROTOCOL_FEATURES. > > > + > > > + * VHOST_USER_SET_VRING_FLAG > > > > > > SET_VRIHG_ENABLE would be a better name. > > Okay. > > > > > > + > > > + Id: 18 > > > + Equivalent ioctl: N/A > > > + Master payload: vring state description > > > + > > > + Set the flag(1 for enable and 0 for disable) to signal slave to enable > > > > Space before ( please. > > Okay. > > > > > > + or disable corresponding virt queue. This request should be sent only > > > + when the protocol feature bit VHOST_USER_PROTOCOL_F_VRING_FLAG is set. > > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c > > > index 141b557..7d9cc8d 100644 > > > --- a/hw/net/vhost_net.c > > > +++ b/hw/net/vhost_net.c > > > @@ -418,6 +418,19 @@ VHostNetState *get_vhost_net(NetClientState *nc) > > > > > > return vhost_net; > > > } > > > + > > > +int vhost_set_vring_flag(NetClientState *nc, int enable) > > > +{ > > > + if (nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) { > > > + VHostNetState *net = get_vhost_net(nc); > > > + const VhostOps *vhost_ops = net->dev.vhost_ops; > > > > add an empty line after declaration pls. > > Got it, thanks. > > > > > > + if (vhost_ops->vhost_backend_mq_set_vring_flag) > > > + return vhost_ops->vhost_backend_mq_set_vring_flag(&net->dev, enable); > > > > Coding style violation: > > line too long, > > Yeah, 81 chars. I thougth that's kind of acceptable. But, anyway, since > you pointed it out, I will fix it next time. > > > and should use {}. > > TBH, I seldom use {} for one statement, and you could see that a lot from > this patch set. Is it a must in qemu community? Yes it is. It's different from the kernel. > If so, I could fix them all. > > > > > > + } > > > + > > > + return 0; > > > +} > > > + > > > #else > > > struct vhost_net *vhost_net_init(VhostNetOptions *options) > > > { > > > @@ -463,4 +476,9 @@ VHostNetState *get_vhost_net(NetClientState *nc) > > > { > > > return 0; > > > } > > > + > > > +int vhost_set_vring_flag(NetClientState *nc, int enable) > > > +{ > > > + return 0; > > > +} > > > #endif > > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > > > index 8d28e45..53f93b1 100644 > > > --- a/hw/net/virtio-net.c > > > +++ b/hw/net/virtio-net.c > > > @@ -407,6 +407,7 @@ static int peer_attach(VirtIONet *n, int index) > > > } > > > > > > if (nc->peer->info->type != NET_CLIENT_OPTIONS_KIND_TAP) { > > > + vhost_set_vring_flag(nc->peer, 1); > > > return 0; > > > } > > > > > > @@ -422,6 +423,7 @@ static int peer_detach(VirtIONet *n, int index) > > > } > > > > > > if (nc->peer->info->type != NET_CLIENT_OPTIONS_KIND_TAP) { > > > + vhost_set_vring_flag(nc->peer, 0); > > > return 0; > > > } > > > > > > > makes no sense to call it for !== tap only. > > Either call this unconditionally, or just for vhost user. > > Agreed. I will make it vhost-usre specific then. > > > > > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > > index 11e46b5..ca6f7fa 100644 > > > --- a/hw/virtio/vhost-user.c > > > +++ b/hw/virtio/vhost-user.c > > > @@ -25,9 +25,10 @@ > > > > > > #define VHOST_MEMORY_MAX_NREGIONS 8 > > > #define VHOST_USER_F_PROTOCOL_FEATURES 30 > > > -#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x1ULL > > > +#define VHOST_USER_PROTOCOL_FEATURE_MASK 0xfULL > > > > > > -#define VHOST_USER_PROTOCOL_F_MQ 0 > > > +#define VHOST_USER_PROTOCOL_F_MQ 0 > > > +#define VHOST_USER_PROTOCOL_F_VRING_FLAG 1 > > > > > > typedef enum VhostUserRequest { > > > VHOST_USER_NONE = 0, > > > @@ -48,6 +49,7 @@ typedef enum VhostUserRequest { > > > VHOST_USER_GET_PROTOCOL_FEATURES = 15, > > > VHOST_USER_SET_PROTOCOL_FEATURES = 16, > > > VHOST_USER_GET_QUEUE_NUM = 17, > > > + VHOST_USER_SET_VRING_FLAG = 18, > > > VHOST_USER_MAX > > > } VhostUserRequest; > > > > > > @@ -296,6 +298,11 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request, > > > msg.addr.index += dev->vq_index; > > > msg.size = sizeof(m.state); > > > break; > > > + case VHOST_USER_SET_VRING_FLAG: > > > + msg.state.index = dev->vq_index; > > > + msg.state.num = *(int*)arg; > > > + msg.size = sizeof(msg.state); > > > + break; > > > > > > case VHOST_USER_GET_VRING_BASE: > > > memcpy(&msg.state, arg, sizeof(struct vhost_vring_state)); > > > @@ -408,6 +415,22 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque) > > > return 0; > > > } > > > > > > +static int vhost_user_set_vring_flag(struct vhost_dev *dev, int enable) > > > +{ > > > + int err; > > > + > > > + assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); > > > + > > > + if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_VRING_FLAG))) > > > + return -1; > > > + > > > + err = vhost_user_call(dev, VHOST_USER_SET_VRING_FLAG, &enable); > > > + if (err < 0) > > > + return err; > > > + > > > + return 0; > > > +} > > > + > > > static int vhost_user_cleanup(struct vhost_dev *dev) > > > { > > > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); > > > > In fact, it's a per VQ pair call the way it's coded. > > So pls fix it, call it for all VQs. > > Let me try. > > > > > > > > @@ -421,5 +444,6 @@ const VhostOps user_ops = { > > > .backend_type = VHOST_BACKEND_TYPE_USER, > > > .vhost_call = vhost_user_call, > > > .vhost_backend_init = vhost_user_init, > > > - .vhost_backend_cleanup = vhost_user_cleanup > > > - }; > > > + .vhost_backend_cleanup = vhost_user_cleanup, > > > + .vhost_backend_mq_set_vring_flag = vhost_user_set_vring_flag, > > > +}; > > > diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h > > > index e472f29..6fc76b7 100644 > > > --- a/include/hw/virtio/vhost-backend.h > > > +++ b/include/hw/virtio/vhost-backend.h > > > @@ -24,12 +24,14 @@ typedef int (*vhost_call)(struct vhost_dev *dev, unsigned long int request, > > > void *arg); > > > typedef int (*vhost_backend_init)(struct vhost_dev *dev, void *opaque); > > > typedef int (*vhost_backend_cleanup)(struct vhost_dev *dev); > > > +typedef int (*vhost_backend_mq_set_vring_flag)(struct vhost_dev *dev, int enable); > > > > > > typedef struct VhostOps { > > > VhostBackendType backend_type; > > > vhost_call vhost_call; > > > vhost_backend_init vhost_backend_init; > > > vhost_backend_cleanup vhost_backend_cleanup; > > > + vhost_backend_mq_set_vring_flag vhost_backend_mq_set_vring_flag; > > > > That's quite ugly: mq is a vhost net thing. > > Why not enable each VQ? > > To define it as `vhost_backend_enable_vring'? > > Thanks. > > --yliu Sure. > > > > > } VhostOps; > > > > > > extern const VhostOps user_ops; > > > diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h > > > index 8db723e..3ec33ff 100644 > > > --- a/include/net/vhost_net.h > > > +++ b/include/net/vhost_net.h > > > @@ -28,4 +28,5 @@ bool vhost_net_virtqueue_pending(VHostNetState *net, int n); > > > void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev, > > > int idx, bool mask); > > > VHostNetState *get_vhost_net(NetClientState *nc); > > > +int vhost_set_vring_flag(NetClientState * nc, int enable); > > > #endif > > > -- > > > 1.9.0
diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt index 99d79be..c2c38f3 100644 --- a/docs/specs/vhost-user.txt +++ b/docs/specs/vhost-user.txt @@ -325,3 +325,13 @@ Message types Query how many queues the backend supports. This request should be sent only when VHOST_USER_PROTOCOL_F_MQ is set in quried protocol features by VHOST_USER_GET_PROTOCOL_FEATURES. + + * VHOST_USER_SET_VRING_FLAG + + Id: 18 + Equivalent ioctl: N/A + Master payload: vring state description + + Set the flag(1 for enable and 0 for disable) to signal slave to enable + or disable corresponding virt queue. This request should be sent only + when the protocol feature bit VHOST_USER_PROTOCOL_F_VRING_FLAG is set. diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 141b557..7d9cc8d 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -418,6 +418,19 @@ VHostNetState *get_vhost_net(NetClientState *nc) return vhost_net; } + +int vhost_set_vring_flag(NetClientState *nc, int enable) +{ + if (nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) { + VHostNetState *net = get_vhost_net(nc); + const VhostOps *vhost_ops = net->dev.vhost_ops; + if (vhost_ops->vhost_backend_mq_set_vring_flag) + return vhost_ops->vhost_backend_mq_set_vring_flag(&net->dev, enable); + } + + return 0; +} + #else struct vhost_net *vhost_net_init(VhostNetOptions *options) { @@ -463,4 +476,9 @@ VHostNetState *get_vhost_net(NetClientState *nc) { return 0; } + +int vhost_set_vring_flag(NetClientState *nc, int enable) +{ + return 0; +} #endif diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 8d28e45..53f93b1 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -407,6 +407,7 @@ static int peer_attach(VirtIONet *n, int index) } if (nc->peer->info->type != NET_CLIENT_OPTIONS_KIND_TAP) { + vhost_set_vring_flag(nc->peer, 1); return 0; } @@ -422,6 +423,7 @@ static int peer_detach(VirtIONet *n, int index) } if (nc->peer->info->type != NET_CLIENT_OPTIONS_KIND_TAP) { + vhost_set_vring_flag(nc->peer, 0); return 0; } diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 11e46b5..ca6f7fa 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -25,9 +25,10 @@ #define VHOST_MEMORY_MAX_NREGIONS 8 #define VHOST_USER_F_PROTOCOL_FEATURES 30 -#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x1ULL +#define VHOST_USER_PROTOCOL_FEATURE_MASK 0xfULL -#define VHOST_USER_PROTOCOL_F_MQ 0 +#define VHOST_USER_PROTOCOL_F_MQ 0 +#define VHOST_USER_PROTOCOL_F_VRING_FLAG 1 typedef enum VhostUserRequest { VHOST_USER_NONE = 0, @@ -48,6 +49,7 @@ typedef enum VhostUserRequest { VHOST_USER_GET_PROTOCOL_FEATURES = 15, VHOST_USER_SET_PROTOCOL_FEATURES = 16, VHOST_USER_GET_QUEUE_NUM = 17, + VHOST_USER_SET_VRING_FLAG = 18, VHOST_USER_MAX } VhostUserRequest; @@ -296,6 +298,11 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request, msg.addr.index += dev->vq_index; msg.size = sizeof(m.state); break; + case VHOST_USER_SET_VRING_FLAG: + msg.state.index = dev->vq_index; + msg.state.num = *(int*)arg; + msg.size = sizeof(msg.state); + break; case VHOST_USER_GET_VRING_BASE: memcpy(&msg.state, arg, sizeof(struct vhost_vring_state)); @@ -408,6 +415,22 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque) return 0; } +static int vhost_user_set_vring_flag(struct vhost_dev *dev, int enable) +{ + int err; + + assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); + + if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_VRING_FLAG))) + return -1; + + err = vhost_user_call(dev, VHOST_USER_SET_VRING_FLAG, &enable); + if (err < 0) + return err; + + return 0; +} + static int vhost_user_cleanup(struct vhost_dev *dev) { assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); @@ -421,5 +444,6 @@ const VhostOps user_ops = { .backend_type = VHOST_BACKEND_TYPE_USER, .vhost_call = vhost_user_call, .vhost_backend_init = vhost_user_init, - .vhost_backend_cleanup = vhost_user_cleanup - }; + .vhost_backend_cleanup = vhost_user_cleanup, + .vhost_backend_mq_set_vring_flag = vhost_user_set_vring_flag, +}; diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h index e472f29..6fc76b7 100644 --- a/include/hw/virtio/vhost-backend.h +++ b/include/hw/virtio/vhost-backend.h @@ -24,12 +24,14 @@ typedef int (*vhost_call)(struct vhost_dev *dev, unsigned long int request, void *arg); typedef int (*vhost_backend_init)(struct vhost_dev *dev, void *opaque); typedef int (*vhost_backend_cleanup)(struct vhost_dev *dev); +typedef int (*vhost_backend_mq_set_vring_flag)(struct vhost_dev *dev, int enable); typedef struct VhostOps { VhostBackendType backend_type; vhost_call vhost_call; vhost_backend_init vhost_backend_init; vhost_backend_cleanup vhost_backend_cleanup; + vhost_backend_mq_set_vring_flag vhost_backend_mq_set_vring_flag; } VhostOps; extern const VhostOps user_ops; diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h index 8db723e..3ec33ff 100644 --- a/include/net/vhost_net.h +++ b/include/net/vhost_net.h @@ -28,4 +28,5 @@ bool vhost_net_virtqueue_pending(VHostNetState *net, int n); void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev, int idx, bool mask); VHostNetState *get_vhost_net(NetClientState *nc); +int vhost_set_vring_flag(NetClientState * nc, int enable); #endif