[PATCH 0/2] vhost user one time message fixes

[PATCH 0/2] vhost user one time message fixes

[Tom Lonergan]

Add a missing message to vhost_user_one_time_request, and fix naming for the
function and update the associated comment.

Patch 1:
vhost_user_one_time_request is actually catching messages that are sent once
per device, not only once for the lifetime of the machine. I've renamed the
function to match the functionality and changed the comment to reflect that
as described in [1].

Patch 2:
VHOST_USER_RESET_DEVICE is used in the same cases as VHOST_USER_RESET_OWNER,
so I've added it to per device messages.

[1] https://lore.kernel.org/qemu-devel/20230127083027-mutt-send-email-mst@kernel.org/

Tom Lonergan (2):
  vhost-user: Change one_time to per_device request
  vhost-user: Make RESET_DEVICE a per device message

 hw/virtio/vhost-user.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

-- 
2.22.3

[PATCH 1/2] vhost-user: Change one_time to per_device request

[Tom Lonergan]

Some devices, like virtio-scsi, consist of one vhost_dev, while others, like
virtio-net, contain multiple vhost_devs. The QEMU vhost-user code has a
concept of one-time messages which is misleading. One-time messages are sent
once per operation on the device, not once for the lifetime of the device.
Therefore, as discussed in [1], vhost_user_one_time_request should be
renamed to vhost_user_per_device_request and the relevant comments updated
to match the real functionality.

[1] https://lore.kernel.org/qemu-devel/20230127083027-mutt-send-email-mst@kernel.org/

Signed-off-by: Tom Lonergan <tom.lonergan@nutanix.com>
---
 hw/virtio/vhost-user.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index c4e0cbd702..65d6299343 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -367,7 +367,7 @@ static int process_message_reply(struct vhost_dev *dev,
     return msg_reply.payload.u64 ? -EIO : 0;
 }
 
-static bool vhost_user_one_time_request(VhostUserRequest request)
+static bool vhost_user_per_device_request(VhostUserRequest request)
 {
     switch (request) {
     case VHOST_USER_SET_OWNER:
@@ -392,11 +392,17 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
     int ret, size = VHOST_USER_HDR_SIZE + msg->hdr.size;
 
     /*
-     * For non-vring specific requests, like VHOST_USER_SET_MEM_TABLE,
-     * we just need send it once in the first time. For later such
-     * request, we just ignore it.
+     * Some devices, like virtio-scsi, are implemented as a single vhost_dev,
+     * while others, like virtio-net, contain multiple vhost_devs. For
+     * operations such as configuring device memory mappings or issuing device
+     * resets, which affect the whole device instead of individual VQs,
+     * vhost-user messages should only be sent once.
+     * 
+     * Devices with multiple vhost_devs are given an associated dev->vq_index
+     * so per_device requests are only sent if vq_index is 0.
      */
-    if (vhost_user_one_time_request(msg->hdr.request) && dev->vq_index != 0) {
+    if (vhost_user_per_device_request(msg->hdr.request)
+        && dev->vq_index != 0) {
         msg->hdr.flags &= ~VHOST_USER_NEED_REPLY_MASK;
         return 0;
     }
@@ -1256,7 +1262,7 @@ static int vhost_user_get_u64(struct vhost_dev *dev, int request, uint64_t *u64)
         .hdr.flags = VHOST_USER_VERSION,
     };
 
-    if (vhost_user_one_time_request(request) && dev->vq_index != 0) {
+    if (vhost_user_per_device_request(request) && dev->vq_index != 0) {
         return 0;
     }
 
-- 
2.22.3

[PATCH 2/2] vhost-user: Make RESET_DEVICE a per device message

[Tom Lonergan]

A device reset is issued per device, not per VQ. The legacy device reset
message, VHOST_USER_RESET_OWNER, is already a per device message. Therefore,
this change adds the proper message, VHOST_USER_RESET_DEVICE, to per device
messages.

Signed-off-by: Tom Lonergan <tom.lonergan@nutanix.com>
---
 hw/virtio/vhost-user.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 65d6299343..8dcf049d42 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -375,6 +375,7 @@ static bool vhost_user_per_device_request(VhostUserRequest request)
     case VHOST_USER_SET_MEM_TABLE:
     case VHOST_USER_GET_QUEUE_NUM:
     case VHOST_USER_NET_SET_MTU:
+    case VHOST_USER_RESET_DEVICE:
     case VHOST_USER_ADD_MEM_REG:
     case VHOST_USER_REM_MEM_REG:
         return true;
-- 
2.22.3

Re: [PATCH 1/2] vhost-user: Change one_time to per_device request

[Raphael Norwitz]

> On Jun 28, 2023, at 5:39 PM, Tom Lonergan <tom.lonergan@nutanix.com> wrote:
> 
> Some devices, like virtio-scsi, consist of one vhost_dev, while others, like
> virtio-net, contain multiple vhost_devs. The QEMU vhost-user code has a
> concept of one-time messages which is misleading. One-time messages are sent
> once per operation on the device, not once for the lifetime of the device.
> Therefore, as discussed in [1], vhost_user_one_time_request should be
> renamed to vhost_user_per_device_request and the relevant comments updated
> to match the real functionality.
> 
> [1] https://lore.kernel.org/qemu-devel/20230127083027-mutt-send-email-mst@kernel.org/
> 

Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>

> Signed-off-by: Tom Lonergan <tom.lonergan@nutanix.com>
> ---
> hw/virtio/vhost-user.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index c4e0cbd702..65d6299343 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -367,7 +367,7 @@ static int process_message_reply(struct vhost_dev *dev,
>     return msg_reply.payload.u64 ? -EIO : 0;
> }
> 
> -static bool vhost_user_one_time_request(VhostUserRequest request)
> +static bool vhost_user_per_device_request(VhostUserRequest request)
> {
>     switch (request) {
>     case VHOST_USER_SET_OWNER:
> @@ -392,11 +392,17 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
>     int ret, size = VHOST_USER_HDR_SIZE + msg->hdr.size;
> 
>     /*
> -     * For non-vring specific requests, like VHOST_USER_SET_MEM_TABLE,
> -     * we just need send it once in the first time. For later such
> -     * request, we just ignore it.
> +     * Some devices, like virtio-scsi, are implemented as a single vhost_dev,
> +     * while others, like virtio-net, contain multiple vhost_devs. For
> +     * operations such as configuring device memory mappings or issuing device
> +     * resets, which affect the whole device instead of individual VQs,
> +     * vhost-user messages should only be sent once.
> +     * 
> +     * Devices with multiple vhost_devs are given an associated dev->vq_index
> +     * so per_device requests are only sent if vq_index is 0.
>      */
> -    if (vhost_user_one_time_request(msg->hdr.request) && dev->vq_index != 0) {
> +    if (vhost_user_per_device_request(msg->hdr.request)
> +        && dev->vq_index != 0) {
>         msg->hdr.flags &= ~VHOST_USER_NEED_REPLY_MASK;
>         return 0;
>     }
> @@ -1256,7 +1262,7 @@ static int vhost_user_get_u64(struct vhost_dev *dev, int request, uint64_t *u64)
>         .hdr.flags = VHOST_USER_VERSION,
>     };
> 
> -    if (vhost_user_one_time_request(request) && dev->vq_index != 0) {
> +    if (vhost_user_per_device_request(request) && dev->vq_index != 0) {
>         return 0;
>     }
> 
> -- 
> 2.22.3
> 

Re: [PATCH 2/2] vhost-user: Make RESET_DEVICE a per device message

[Raphael Norwitz]

> On Jun 28, 2023, at 5:39 PM, Tom Lonergan <tom.lonergan@nutanix.com> wrote:
> 
> A device reset is issued per device, not per VQ. The legacy device reset
> message, VHOST_USER_RESET_OWNER, is already a per device message. Therefore,
> this change adds the proper message, VHOST_USER_RESET_DEVICE, to per device
> messages.
> 

Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>

> Signed-off-by: Tom Lonergan <tom.lonergan@nutanix.com>
> ---
> hw/virtio/vhost-user.c | 1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 65d6299343..8dcf049d42 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -375,6 +375,7 @@ static bool vhost_user_per_device_request(VhostUserRequest request)
>     case VHOST_USER_SET_MEM_TABLE:
>     case VHOST_USER_GET_QUEUE_NUM:
>     case VHOST_USER_NET_SET_MTU:
> +    case VHOST_USER_RESET_DEVICE:
>     case VHOST_USER_ADD_MEM_REG:
>     case VHOST_USER_REM_MEM_REG:
>         return true;
> -- 
> 2.22.3
>