From: "Alex Bennée" <alex.bennee@linaro.org> To: Stefano Garzarella <sgarzare@redhat.com> Cc: qemu-devel@nongnu.org, virtio-dev@lists.oasis-open.org, slp@redhat.com, mst@redhat.com, marcandre.lureau@redhat.com, stefanha@redhat.com, viresh.kumar@linaro.org, takahiro.akashi@linaro.org, erik.schilling@linaro.org, manos.pitsidianakis@linaro.org, mathieu.poirier@linaro.org Subject: Re: [virtio-dev] [RFC PATCH] docs/interop: define STANDALONE protocol feature for vhost-user Date: Tue, 04 Jul 2023 16:02:42 +0100 [thread overview] Message-ID: <87o7krg0sn.fsf@linaro.org> (raw) In-Reply-To: <3ogh7u3ezp7vlrp3ticquoajgsnpnglplm44osrsd7gvxv2lyn@g22qgf4vwgp5> Stefano Garzarella <sgarzare@redhat.com> writes: > On Tue, Jul 04, 2023 at 01:36:00PM +0100, Alex Bennée wrote: >>Currently QEMU has to know some details about the back-end to be able >>to setup the guest. While various parts of the setup can be delegated >>to the backend (for example config handling) this is a very piecemeal >>approach. >> >>This patch suggests a new feature flag (VHOST_USER_PROTOCOL_F_STANDALONE) >>which the back-end can advertise which allows a probe message to be >>sent to get all the details QEMU needs to know in one message. >> >>Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> >>--- >>Initial RFC for discussion. I intend to prototype this work with QEMU >>and one of the rust-vmm vhost-user daemons. > > Thanks for starting this discussion! > > I'm comparing with vhost-vdpa IOCTLs, so my questions may be > superficial, but they help me understand the differences. I did have a quick read-through to get a handle on vhost-vdpa but the docs are fairly empty. However I see there is more detail in the linux commit so after looking at that I do wonder: * The kernel commit defines a subset of VIRTIO_F feature flags. Should we do the same for this interface? * The VDPA GET/SET STATUS and GET/SET CONFIG ioctls are already covered by the equivalent VHOST_USER messages? >>--- >> docs/interop/vhost-user.rst | 37 +++++++++++++++++++++++++++++++++++++ >> hw/virtio/vhost-user.c | 8 ++++++++ >> 2 files changed, 45 insertions(+) >> >>diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst >>index 5a070adbc1..85b1b1583a 100644 >>--- a/docs/interop/vhost-user.rst >>+++ b/docs/interop/vhost-user.rst >>@@ -275,6 +275,21 @@ Inflight description >> >> :queue size: a 16-bit size of virtqueues >> >>+Backend specifications >>+^^^^^^^^^^^^^^^^^^^^^^ >>+ >>++-----------+-------------+------------+------------+ >>+| device id | config size | min_vqs | max_vqs | >>++-----------+-------------+------------+------------+ >>+ >>+:device id: a 32-bit value holding the VirtIO device ID >>+ >>+:config size: a 32-bit value holding the config size (see ``VHOST_USER_GET_CONFIG``) >>+ >>+:min_vqs: a 32-bit value holding the minimum number of vqs supported > > Why do we need the minimum? We need to know the minimum number because some devices have fixed VQs that must be present. > >>+ >>+:max_vqs: a 32-bit value holding the maximum number of vqs supported, must be >= min_vqs > > Is this overlap with VHOST_USER_GET_QUEUE_NUM? Yes it does and I considered implementing a bunch of messages to fill in around what we already have. However that seemed like it would add a bunch of complexity to the interface when we could get all the initial data in one go. > >>+ >> C structure >> ----------- >> >>@@ -296,6 +311,7 @@ In QEMU the vhost-user message is implemented with the following struct: >> VhostUserConfig config; >> VhostUserVringArea area; >> VhostUserInflight inflight; >>+ VhostUserBackendSpecs specs; >> }; >> } QEMU_PACKED VhostUserMsg; >> >>@@ -316,6 +332,7 @@ replies. Here is a list of the ones that do: >> * ``VHOST_USER_GET_VRING_BASE`` >> * ``VHOST_USER_SET_LOG_BASE`` (if ``VHOST_USER_PROTOCOL_F_LOG_SHMFD``) >> * ``VHOST_USER_GET_INFLIGHT_FD`` (if ``VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD``) >>+* ``VHOST_USER_GET_BACKEND_SPECS`` (if ``VHOST_USER_PROTOCOL_F_STANDALONE``) >> >> .. seealso:: >> >>@@ -885,6 +902,13 @@ Protocol features >> #define VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS 15 >> #define VHOST_USER_PROTOCOL_F_STATUS 16 >> #define VHOST_USER_PROTOCOL_F_XEN_MMAP 17 >>+ #define VHOST_USER_PROTOCOL_F_STANDALONE 18 >>+ >>+Some features are only valid in the presence of other supporting >>+features. In the case of ``VHOST_USER_PROTOCOL_F_STANDALONE`` the >>+backend must also support ``VHOST_USER_PROTOCOL_F_CONFIG`` and >>+``VHOST_USER_PROTOCOL_F_STATUS``. >>+ > > What about adding a new section where we will describe what we mean > with "standalone" devices? > > For example that the entire virtio device is emulated in the backend, > etc. > > By the way, I was thinking more about F_FULL_DEVICE, but I'm not good > with names, so I'll just throw out an idea :-) Naming things is hard ;-) I could add a new section although AIUI there is nothing really in existing daemons which is split between the front-end and back-end. The stubs are basically boilerplate and ensure DT/PCIe hubs make the device appear so once things are discovered QEMU never really gets involved aside from being a dumb relay. > > Thanks, > Stefano > >> >> Front-end message types >> ----------------------- >>@@ -1440,6 +1464,19 @@ Front-end message types >> query the back-end for its device status as defined in the Virtio >> specification. >> >>+``VHOST_USER_GET_BACKEND_SPECS`` >>+ :id: 41 >>+ :request payload: N/A >>+ :reply payload: ``Backend specifications`` >>+ >>+ When the ``VHOST_USER_PROTOCOL_F_STANDALONE`` protocol feature has been >>+ successfully negotiated, this message is submitted by the front-end to >>+ query the back-end for its capabilities. This is intended to remove >>+ the need for the front-end to know ahead of time what the VirtIO >>+ device the backend emulates is. >>+ >>+ The reply contains the device id, size of the config space and the >>+ range of VirtQueues the backend supports. >> >> Back-end message types >> ---------------------- >>diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c >>index c4e0cbd702..28b021d5d3 100644 >>--- a/hw/virtio/vhost-user.c >>+++ b/hw/virtio/vhost-user.c >>@@ -202,6 +202,13 @@ typedef struct VhostUserInflight { >> uint16_t queue_size; >> } VhostUserInflight; >> >>+typedef struct VhostUserBackendSpecs { >>+ uint32_t device_id; >>+ uint32_t config_size; >>+ uint32_t min_vqs; >>+ uint32_t max_vqs; >>+} VhostUserBackendSpecs; >>+ >> typedef struct { >> VhostUserRequest request; >> >>@@ -226,6 +233,7 @@ typedef union { >> VhostUserCryptoSession session; >> VhostUserVringArea area; >> VhostUserInflight inflight; >>+ VhostUserBackendSpecs specs; >> } VhostUserPayload; >> >> typedef struct VhostUserMsg { >> -- 2.39.2 >> >> >>--------------------------------------------------------------------- >>To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org >>For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org >> -- Alex Bennée Virtualisation Tech Lead @ Linaro
WARNING: multiple messages have this Message-ID (diff)
From: "Alex Bennée" <alex.bennee@linaro.org> To: Stefano Garzarella <sgarzare@redhat.com> Cc: qemu-devel@nongnu.org, virtio-dev@lists.oasis-open.org, slp@redhat.com, mst@redhat.com, marcandre.lureau@redhat.com, stefanha@redhat.com, viresh.kumar@linaro.org, takahiro.akashi@linaro.org, erik.schilling@linaro.org, manos.pitsidianakis@linaro.org, mathieu.poirier@linaro.org Subject: Re: [virtio-dev] [RFC PATCH] docs/interop: define STANDALONE protocol feature for vhost-user Date: Tue, 04 Jul 2023 16:02:42 +0100 [thread overview] Message-ID: <87o7krg0sn.fsf@linaro.org> (raw) In-Reply-To: <3ogh7u3ezp7vlrp3ticquoajgsnpnglplm44osrsd7gvxv2lyn@g22qgf4vwgp5> Stefano Garzarella <sgarzare@redhat.com> writes: > On Tue, Jul 04, 2023 at 01:36:00PM +0100, Alex Bennée wrote: >>Currently QEMU has to know some details about the back-end to be able >>to setup the guest. While various parts of the setup can be delegated >>to the backend (for example config handling) this is a very piecemeal >>approach. >> >>This patch suggests a new feature flag (VHOST_USER_PROTOCOL_F_STANDALONE) >>which the back-end can advertise which allows a probe message to be >>sent to get all the details QEMU needs to know in one message. >> >>Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> >>--- >>Initial RFC for discussion. I intend to prototype this work with QEMU >>and one of the rust-vmm vhost-user daemons. > > Thanks for starting this discussion! > > I'm comparing with vhost-vdpa IOCTLs, so my questions may be > superficial, but they help me understand the differences. I did have a quick read-through to get a handle on vhost-vdpa but the docs are fairly empty. However I see there is more detail in the linux commit so after looking at that I do wonder: * The kernel commit defines a subset of VIRTIO_F feature flags. Should we do the same for this interface? * The VDPA GET/SET STATUS and GET/SET CONFIG ioctls are already covered by the equivalent VHOST_USER messages? >>--- >> docs/interop/vhost-user.rst | 37 +++++++++++++++++++++++++++++++++++++ >> hw/virtio/vhost-user.c | 8 ++++++++ >> 2 files changed, 45 insertions(+) >> >>diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst >>index 5a070adbc1..85b1b1583a 100644 >>--- a/docs/interop/vhost-user.rst >>+++ b/docs/interop/vhost-user.rst >>@@ -275,6 +275,21 @@ Inflight description >> >> :queue size: a 16-bit size of virtqueues >> >>+Backend specifications >>+^^^^^^^^^^^^^^^^^^^^^^ >>+ >>++-----------+-------------+------------+------------+ >>+| device id | config size | min_vqs | max_vqs | >>++-----------+-------------+------------+------------+ >>+ >>+:device id: a 32-bit value holding the VirtIO device ID >>+ >>+:config size: a 32-bit value holding the config size (see ``VHOST_USER_GET_CONFIG``) >>+ >>+:min_vqs: a 32-bit value holding the minimum number of vqs supported > > Why do we need the minimum? We need to know the minimum number because some devices have fixed VQs that must be present. > >>+ >>+:max_vqs: a 32-bit value holding the maximum number of vqs supported, must be >= min_vqs > > Is this overlap with VHOST_USER_GET_QUEUE_NUM? Yes it does and I considered implementing a bunch of messages to fill in around what we already have. However that seemed like it would add a bunch of complexity to the interface when we could get all the initial data in one go. > >>+ >> C structure >> ----------- >> >>@@ -296,6 +311,7 @@ In QEMU the vhost-user message is implemented with the following struct: >> VhostUserConfig config; >> VhostUserVringArea area; >> VhostUserInflight inflight; >>+ VhostUserBackendSpecs specs; >> }; >> } QEMU_PACKED VhostUserMsg; >> >>@@ -316,6 +332,7 @@ replies. Here is a list of the ones that do: >> * ``VHOST_USER_GET_VRING_BASE`` >> * ``VHOST_USER_SET_LOG_BASE`` (if ``VHOST_USER_PROTOCOL_F_LOG_SHMFD``) >> * ``VHOST_USER_GET_INFLIGHT_FD`` (if ``VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD``) >>+* ``VHOST_USER_GET_BACKEND_SPECS`` (if ``VHOST_USER_PROTOCOL_F_STANDALONE``) >> >> .. seealso:: >> >>@@ -885,6 +902,13 @@ Protocol features >> #define VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS 15 >> #define VHOST_USER_PROTOCOL_F_STATUS 16 >> #define VHOST_USER_PROTOCOL_F_XEN_MMAP 17 >>+ #define VHOST_USER_PROTOCOL_F_STANDALONE 18 >>+ >>+Some features are only valid in the presence of other supporting >>+features. In the case of ``VHOST_USER_PROTOCOL_F_STANDALONE`` the >>+backend must also support ``VHOST_USER_PROTOCOL_F_CONFIG`` and >>+``VHOST_USER_PROTOCOL_F_STATUS``. >>+ > > What about adding a new section where we will describe what we mean > with "standalone" devices? > > For example that the entire virtio device is emulated in the backend, > etc. > > By the way, I was thinking more about F_FULL_DEVICE, but I'm not good > with names, so I'll just throw out an idea :-) Naming things is hard ;-) I could add a new section although AIUI there is nothing really in existing daemons which is split between the front-end and back-end. The stubs are basically boilerplate and ensure DT/PCIe hubs make the device appear so once things are discovered QEMU never really gets involved aside from being a dumb relay. > > Thanks, > Stefano > >> >> Front-end message types >> ----------------------- >>@@ -1440,6 +1464,19 @@ Front-end message types >> query the back-end for its device status as defined in the Virtio >> specification. >> >>+``VHOST_USER_GET_BACKEND_SPECS`` >>+ :id: 41 >>+ :request payload: N/A >>+ :reply payload: ``Backend specifications`` >>+ >>+ When the ``VHOST_USER_PROTOCOL_F_STANDALONE`` protocol feature has been >>+ successfully negotiated, this message is submitted by the front-end to >>+ query the back-end for its capabilities. This is intended to remove >>+ the need for the front-end to know ahead of time what the VirtIO >>+ device the backend emulates is. >>+ >>+ The reply contains the device id, size of the config space and the >>+ range of VirtQueues the backend supports. >> >> Back-end message types >> ---------------------- >>diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c >>index c4e0cbd702..28b021d5d3 100644 >>--- a/hw/virtio/vhost-user.c >>+++ b/hw/virtio/vhost-user.c >>@@ -202,6 +202,13 @@ typedef struct VhostUserInflight { >> uint16_t queue_size; >> } VhostUserInflight; >> >>+typedef struct VhostUserBackendSpecs { >>+ uint32_t device_id; >>+ uint32_t config_size; >>+ uint32_t min_vqs; >>+ uint32_t max_vqs; >>+} VhostUserBackendSpecs; >>+ >> typedef struct { >> VhostUserRequest request; >> >>@@ -226,6 +233,7 @@ typedef union { >> VhostUserCryptoSession session; >> VhostUserVringArea area; >> VhostUserInflight inflight; >>+ VhostUserBackendSpecs specs; >> } VhostUserPayload; >> >> typedef struct VhostUserMsg { >> -- 2.39.2 >> >> >>--------------------------------------------------------------------- >>To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org >>For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org >> -- Alex Bennée Virtualisation Tech Lead @ Linaro --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
next prev parent reply other threads:[~2023-07-04 15:22 UTC|newest] Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-07-04 12:36 [RFC PATCH] docs/interop: define STANDALONE protocol feature for vhost-user Alex Bennée 2023-07-04 12:36 ` [virtio-dev] " Alex Bennée 2023-07-04 14:54 ` Stefano Garzarella 2023-07-04 14:54 ` Stefano Garzarella 2023-07-04 15:02 ` Alex Bennée [this message] 2023-07-04 15:02 ` Alex Bennée 2023-07-07 10:27 ` Stefano Garzarella 2023-07-07 10:27 ` Stefano Garzarella 2023-07-20 19:36 ` Stefan Hajnoczi 2023-07-26 16:01 ` Michael S. Tsirkin 2023-07-26 16:01 ` Michael S. Tsirkin 2023-07-26 14:33 ` Erik Schilling 2023-07-26 15:51 ` Stefan Hajnoczi 2023-07-06 16:31 ` Alex Bennée 2023-07-06 16:31 ` [virtio-dev] " Alex Bennée 2023-07-07 10:35 ` Stefano Garzarella 2023-07-07 10:35 ` [virtio-dev] " Stefano Garzarella 2023-07-06 16:48 ` Michael S. Tsirkin 2023-07-06 16:48 ` Michael S. Tsirkin 2023-07-07 7:58 ` [virtio-dev] " Alex Bennée 2023-07-07 7:58 ` Alex Bennée 2023-07-07 9:57 ` [virtio-dev] " Michael S. Tsirkin 2023-07-07 9:57 ` Michael S. Tsirkin 2023-07-07 13:12 ` Alex Bennée 2023-07-07 13:12 ` [virtio-dev] " Alex Bennée 2023-07-20 19:58 ` Stefan Hajnoczi 2023-07-20 19:58 ` Stefan Hajnoczi 2023-07-20 21:14 ` [virtio-dev] " Michael S. Tsirkin 2023-07-20 21:14 ` Michael S. Tsirkin 2023-07-20 21:31 ` Stefan Hajnoczi 2023-07-20 22:22 ` [virtio-dev] " Michael S. Tsirkin 2023-07-20 22:22 ` Michael S. Tsirkin 2023-07-24 18:08 ` [virtio-dev] " Stefan Hajnoczi 2023-07-24 18:08 ` Stefan Hajnoczi 2023-07-26 16:02 ` [virtio-dev] " Michael S. Tsirkin 2023-07-26 16:02 ` Michael S. Tsirkin 2023-07-26 17:37 ` [virtio-dev] " Stefan Hajnoczi 2023-07-26 17:37 ` Stefan Hajnoczi 2023-07-20 19:32 ` Stefan Hajnoczi 2023-07-20 19:32 ` [virtio-dev] " Stefan Hajnoczi 2023-07-20 19:34 ` Stefan Hajnoczi 2023-07-20 19:34 ` [virtio-dev] " Stefan Hajnoczi
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=87o7krg0sn.fsf@linaro.org \ --to=alex.bennee@linaro.org \ --cc=erik.schilling@linaro.org \ --cc=manos.pitsidianakis@linaro.org \ --cc=marcandre.lureau@redhat.com \ --cc=mathieu.poirier@linaro.org \ --cc=mst@redhat.com \ --cc=qemu-devel@nongnu.org \ --cc=sgarzare@redhat.com \ --cc=slp@redhat.com \ --cc=stefanha@redhat.com \ --cc=takahiro.akashi@linaro.org \ --cc=viresh.kumar@linaro.org \ --cc=virtio-dev@lists.oasis-open.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.