qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Hanna Czenczek <hreitz@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>
Subject: [PULL 02/63] vhost-user.rst: Clarify enabling/disabling vrings
Date: Tue, 7 Nov 2023 05:09:33 -0500	[thread overview]
Message-ID: <eae69cc36b278a3e84bb013909135e138b37855d.1699351720.git.mst@redhat.com> (raw)
In-Reply-To: <cover.1699351720.git.mst@redhat.com>

From: Hanna Czenczek <hreitz@redhat.com>

Currently, the vhost-user documentation says that rings are to be
initialized in a disabled state when VHOST_USER_F_PROTOCOL_FEATURES is
negotiated.  However, by the time of feature negotiation, all rings have
already been initialized, so it is not entirely clear what this means.

At least the vhost-user-backend Rust crate's implementation interpreted
it to mean that whenever this feature is negotiated, all rings are to
put into a disabled state, which means that every SET_FEATURES call
would disable all rings, effectively halting the device.  This is
problematic because the VHOST_F_LOG_ALL feature is also set or cleared
this way, which happens during migration.  Doing so should not halt the
device.

Other implementations have interpreted this to mean that the device is
to be initialized with all rings disabled, and a subsequent SET_FEATURES
call that does not set VHOST_USER_F_PROTOCOL_FEATURES will enable all of
them.  Here, SET_FEATURES will never disable any ring.

This interpretation does not suffer the problem of unintentionally
halting the device whenever features are set or cleared, so it seems
better and more reasonable.

We can clarify this in the documentation by making it explicit that the
enabled/disabled state is tracked even while the vring is stopped.
Every vring is initialized in a disabled state, and SET_FEATURES without
VHOST_USER_F_PROTOCOL_FEATURES simply becomes one way to enable all
vrings.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20231016134243.68248-3-hreitz@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 docs/interop/vhost-user.rst | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 9202b167dd..e5a04c04ed 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -411,31 +411,33 @@ negotiation.
 Ring states
 -----------
 
-Rings can be in one of three states:
+Rings have two independent states: started/stopped, and enabled/disabled.
 
-* stopped: the back-end must not process the ring at all.
+* While a ring is stopped, the back-end must not process the ring at
+  all, regardless of whether it is enabled or disabled.  The
+  enabled/disabled state should still be tracked, though, so it can come
+  into effect once the ring is started.
 
-* started but disabled: the back-end must process the ring without
+* started and disabled: The back-end must process the ring without
   causing any side effects.  For example, for a networking device,
   in the disabled state the back-end must not supply any new RX packets,
   but must process and discard any TX packets.
 
-* started and enabled.
+* started and enabled: The back-end must process the ring normally, i.e.
+  process all requests and execute them.
 
-Each ring is initialized in a stopped state.  The back-end must start
-ring upon receiving a kick (that is, detecting that file descriptor is
-readable) on the descriptor specified by ``VHOST_USER_SET_VRING_KICK``
-or receiving the in-band message ``VHOST_USER_VRING_KICK`` if negotiated,
-and stop ring upon receiving ``VHOST_USER_GET_VRING_BASE``.
+Each ring is initialized in a stopped and disabled state.  The back-end
+must start a ring upon receiving a kick (that is, detecting that file
+descriptor is readable) on the descriptor specified by
+``VHOST_USER_SET_VRING_KICK`` or receiving the in-band message
+``VHOST_USER_VRING_KICK`` if negotiated, and stop a ring upon receiving
+``VHOST_USER_GET_VRING_BASE``.
 
 Rings can be enabled or disabled by ``VHOST_USER_SET_VRING_ENABLE``.
 
-If ``VHOST_USER_F_PROTOCOL_FEATURES`` has not been negotiated, the
-ring starts directly in the enabled state.
-
-If ``VHOST_USER_F_PROTOCOL_FEATURES`` has been negotiated, the ring is
-initialized in a disabled state and is enabled by
-``VHOST_USER_SET_VRING_ENABLE`` with parameter 1.
+In addition, upon receiving a ``VHOST_USER_SET_FEATURES`` message from
+the front-end without ``VHOST_USER_F_PROTOCOL_FEATURES`` set, the
+back-end must enable all rings immediately.
 
 While processing the rings (whether they are enabled or not), the back-end
 must support changing some configuration aspects on the fly.
-- 
MST



  parent reply	other threads:[~2023-11-07 10:11 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-07 10:09 [PULL 00/63] virtio,pc,pci: features, fixes Michael S. Tsirkin
2023-11-07 10:09 ` [PULL 01/63] vhost-user.rst: Improve [GS]ET_VRING_BASE doc Michael S. Tsirkin
2023-11-07 10:09 ` Michael S. Tsirkin [this message]
2023-11-07 10:09 ` [PULL 03/63] vhost-user.rst: Introduce suspended state Michael S. Tsirkin
2023-11-07 10:09 ` [PULL 04/63] vhost-user.rst: Migrating back-end-internal state Michael S. Tsirkin
2023-11-07 10:09 ` [PULL 05/63] vhost-user: Interface for migration state transfer Michael S. Tsirkin
2023-11-07 10:09 ` [PULL 06/63] vhost: Add high-level state save/load functions Michael S. Tsirkin
2023-11-07 10:09 ` [PULL 07/63] vhost-user-fs: Implement internal migration Michael S. Tsirkin
2023-11-07 10:09 ` [PULL 08/63] Add virtio-sound device stub Michael S. Tsirkin
2023-11-09 14:30   ` Peter Maydell
2023-11-09 15:50     ` Manos Pitsidianakis
2023-11-09 16:06       ` Peter Maydell
2023-11-09 16:10       ` Alex Bennée
2023-11-07 10:10 ` [PULL 09/63] Add virtio-sound-pci device Michael S. Tsirkin
2023-11-07 10:10 ` [PULL 10/63] virtio-sound: handle control messages and streams Michael S. Tsirkin
2023-11-07 10:10 ` [PULL 11/63] virtio-sound: handle VIRTIO_SND_R_PCM_INFO request Michael S. Tsirkin
2023-11-07 10:10 ` [PULL 12/63] virtio-sound: handle VIRTIO_SND_R_PCM_{START,STOP} Michael S. Tsirkin
2023-11-07 10:10 ` [PULL 13/63] virtio-sound: handle VIRTIO_SND_R_PCM_SET_PARAMS Michael S. Tsirkin
2023-11-07 10:10 ` [PULL 14/63] virtio-sound: handle VIRTIO_SND_R_PCM_PREPARE Michael S. Tsirkin
2023-11-07 10:10 ` [PULL 15/63] virtio-sound: handle VIRTIO_SND_R_PCM_RELEASE Michael S. Tsirkin
2023-11-07 10:10 ` [PULL 16/63] virtio-sound: implement audio output (TX) Michael S. Tsirkin
2023-11-07 10:10 ` [PULL 17/63] virtio-sound: implement audio capture (RX) Michael S. Tsirkin
2023-11-07 10:10 ` [PULL 18/63] docs/system: add basic virtio-snd documentation Michael S. Tsirkin
2023-11-07 10:10 ` [PULL 19/63] vdpa: Restore hash calculation state Michael S. Tsirkin
2023-11-07 10:10 ` [PULL 20/63] vdpa: Allow VIRTIO_NET_F_HASH_REPORT in SVQ Michael S. Tsirkin
2023-11-07 10:11 ` [PULL 21/63] vdpa: Add SetSteeringEBPF method for NetClientState Michael S. Tsirkin
2023-11-07 10:11 ` [PULL 22/63] vdpa: Restore receive-side scaling state Michael S. Tsirkin
2023-11-07 10:11 ` [PULL 23/63] vdpa: Allow VIRTIO_NET_F_RSS in SVQ Michael S. Tsirkin
2023-11-07 10:11 ` [PULL 24/63] tests: test-smp-parse: Add the test for cores/threads per socket helpers Michael S. Tsirkin
2023-11-07 10:11 ` [PULL 25/63] tests: bios-tables-test: Prepare the ACPI table change for smbios type4 count test Michael S. Tsirkin
2023-11-07 10:11 ` [PULL 26/63] tests: bios-tables-test: Add test for smbios type4 count Michael S. Tsirkin
2023-11-07 10:11 ` [PULL 27/63] tests: bios-tables-test: Add ACPI table binaries for smbios type4 count test Michael S. Tsirkin
2023-11-07 10:11 ` [PULL 28/63] tests: bios-tables-test: Prepare the ACPI table change for smbios type4 core " Michael S. Tsirkin
2023-11-07 10:11 ` [PULL 29/63] tests: bios-tables-test: Add test for smbios type4 core count Michael S. Tsirkin
2023-11-07 10:11 ` [PULL 30/63] tests: bios-tables-test: Add ACPI table binaries for smbios type4 core count test Michael S. Tsirkin
2023-11-07 10:11 ` [PULL 31/63] tests: bios-tables-test: Prepare the ACPI table change for smbios type4 core count2 test Michael S. Tsirkin
2023-11-07 10:11 ` [PULL 32/63] tests: bios-tables-test: Extend smbios core count2 test to cover general topology Michael S. Tsirkin
2023-11-07 10:11 ` [PULL 33/63] tests: bios-tables-test: Update ACPI table binaries for smbios core count2 test Michael S. Tsirkin
2023-11-07 10:11 ` [PULL 34/63] tests: bios-tables-test: Prepare the ACPI table change for smbios type4 thread count test Michael S. Tsirkin
2023-11-07 10:12 ` [PULL 35/63] tests: bios-tables-test: Add test for smbios type4 thread count Michael S. Tsirkin
2023-11-07 10:12 ` [PULL 36/63] tests: bios-tables-test: Add ACPI table binaries for smbios type4 thread count test Michael S. Tsirkin
2023-11-07 10:12 ` [PULL 37/63] tests: bios-tables-test: Prepare the ACPI table change for smbios type4 thread count2 test Michael S. Tsirkin
2023-11-07 10:12 ` [PULL 38/63] tests: bios-tables-test: Add test for smbios type4 thread count2 Michael S. Tsirkin
2023-11-07 10:12 ` [PULL 39/63] tests: bios-tables-test: Add ACPI table binaries for smbios type4 thread count2 test Michael S. Tsirkin
2023-11-07 10:12 ` [PULL 40/63] hw/cxl: Use a switch to explicitly check size in caps_reg_read() Michael S. Tsirkin
2023-11-07 10:12 ` [PULL 41/63] hw/cxl: Use switch statements for read and write of cachemem registers Michael S. Tsirkin
2023-11-07 10:12 ` [PULL 42/63] hw/cxl: CXLDVSECPortExtensions renamed to CXLDVSECPortExt Michael S. Tsirkin
2023-11-07 10:12 ` [PULL 43/63] hw/cxl: Line length reductions Michael S. Tsirkin
2023-11-07 10:12 ` [PULL 44/63] hw/cxl: Fix a QEMU_BUILD_BUG_ON() in switch statement scope issue Michael S. Tsirkin
2023-11-07 10:12 ` [PULL 45/63] hw/cxl/mbox: Pull the payload out of struct cxl_cmd and make instances constant Michael S. Tsirkin
2023-11-07 10:12 ` [PULL 46/63] hw/cxl/mbox: Split mailbox command payload into separate input and output Michael S. Tsirkin
2023-11-07 10:12 ` [PULL 47/63] hw/cxl/mbox: Pull the CCI definition out of the CXLDeviceState Michael S. Tsirkin
2023-11-07 10:13 ` [PULL 48/63] hw/cxl/mbox: Generalize the CCI command processing Michael S. Tsirkin
2023-11-07 10:13 ` [PULL 49/63] hw/pci-bridge/cxl_upstream: Move defintion of device to header Michael S. Tsirkin
2023-11-07 10:13 ` [PULL 50/63] hw/cxl: Add a switch mailbox CCI function Michael S. Tsirkin
2023-11-07 10:13 ` [PULL 51/63] hw/cxl/mbox: Add Information and Status / Identify command Michael S. Tsirkin
2023-11-07 10:13 ` [PULL 52/63] hw/cxl/mbox: Add Physical Switch " Michael S. Tsirkin
2023-11-07 10:13 ` [PULL 53/63] hw/pci-bridge/cxl_downstream: Set default link width and link speed Michael S. Tsirkin
2023-11-07 10:13 ` [PULL 54/63] hw/cxl: Implement Physical Ports status retrieval Michael S. Tsirkin
2023-11-07 10:13 ` [PULL 55/63] hw/cxl/mbox: Add support for background operations Michael S. Tsirkin
2023-11-09 14:44   ` Peter Maydell
2023-11-10  4:25     ` Davidlohr Bueso
2023-11-07 10:13 ` [PULL 56/63] hw/cxl/mbox: Wire up interrupts for background completion Michael S. Tsirkin
2023-11-07 10:13 ` [PULL 57/63] hw/cxl: Add support for device sanitation Michael S. Tsirkin
2023-11-09 14:39   ` Peter Maydell
2023-11-10  4:14     ` Davidlohr Bueso
2023-11-07 10:13 ` [PULL 58/63] hw/cxl/mbox: Add Get Background Operation Status Command Michael S. Tsirkin
2023-11-07 10:13 ` [PULL 59/63] hw/cxl/type3: Cleanup multiple CXL_TYPE3() calls in read/write functions Michael S. Tsirkin
2023-11-07 10:13 ` [PULL 60/63] hw/cxl: Add dummy security state get Michael S. Tsirkin
2023-11-07 10:13 ` [PULL 61/63] hw/cxl: Add tunneled command support to mailbox for switch cci Michael S. Tsirkin
2023-11-07 10:13 ` [PULL 62/63] acpi/tests/avocado/bits: enforce 32-bit SMBIOS entry point Michael S. Tsirkin
2023-11-07 10:14 ` [PULL 63/63] acpi/tests/avocado/bits: enable console logging from bits VM Michael S. Tsirkin
2023-11-07 13:40 ` [PULL 00/63] virtio,pc,pci: features, fixes 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=eae69cc36b278a3e84bb013909135e138b37855d.1699351720.git.mst@redhat.com \
    --to=mst@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).