All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH  v1 00/13] various virtio docs, fixes and tweaks
@ 2022-03-21 15:30 Alex Bennée
  2022-03-21 15:30   ` [Virtio-fs] " Alex Bennée
                   ` (14 more replies)
  0 siblings, 15 replies; 31+ messages in thread
From: Alex Bennée @ 2022-03-21 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, stefanha,
	marcandre.lureau, Alex Bennée

Hi,

This series is a sub-set of patches while I was trying to re-rev my
virtio-rpmb patches. It attempts to address a few things:

  - improve documentation for virtio/vhost/vhost-user
  - document some of the API
  - a hacky fix for F_CONFIG handling
  - putting VhostUserState on a diet, make VhostUserHostNotifier dynamic

In particular I've been trying to better understand how vhost-user
interactions are meant to work and why there are two different methods
for instantiating them. If my supposition is correct perhaps a number
of devices that don't have in-kernel vhost equivalents could be converted?

While working onthe VhostUserHostNotifier changes I found it quite
hard to trigger the code. Is this rarely used code or just requires
backends we don't see in the testing?

Alex Bennée (10):
  hw/virtio: move virtio-pci.h into shared include space
  virtio-pci: add notification trace points
  hw/virtio: add vhost_user_[read|write] trace points
  vhost-user.rst: add clarifying language about protocol negotiation
  libvhost-user: expose vu_request_to_string
  docs/devel: start documenting writing VirtIO devices
  include/hw: start documenting the vhost API
  contrib/vhost-user-blk: fix 32 bit build and enable
  hw/virtio/vhost-user: don't suppress F_CONFIG when supported
  virtio/vhost-user: dynamically assign VhostUserHostNotifiers

Paolo Bonzini (3):
  docs: vhost-user: clean up request/reply description
  docs: vhost-user: rewrite section on ring state machine
  docs: vhost-user: replace master/slave with front-end/back-end

 docs/devel/index-internals.rst            |   1 +
 docs/devel/virtio-backends.rst            | 214 +++++++++
 docs/interop/vhost-user-gpu.rst           |  10 +-
 docs/interop/vhost-user.rst               | 555 ++++++++++++----------
 meson.build                               |   2 +-
 include/hw/virtio/vhost-user.h            |  43 +-
 include/hw/virtio/vhost.h                 | 132 ++++-
 {hw => include/hw}/virtio/virtio-pci.h    |   0
 subprojects/libvhost-user/libvhost-user.h |   9 +
 contrib/vhost-user-blk/vhost-user-blk.c   |   6 +-
 hw/scsi/vhost-user-scsi.c                 |   1 +
 hw/virtio/vhost-scsi-pci.c                |   2 +-
 hw/virtio/vhost-user-blk-pci.c            |   2 +-
 hw/virtio/vhost-user-fs-pci.c             |   2 +-
 hw/virtio/vhost-user-i2c-pci.c            |   2 +-
 hw/virtio/vhost-user-input-pci.c          |   2 +-
 hw/virtio/vhost-user-rng-pci.c            |   2 +-
 hw/virtio/vhost-user-scsi-pci.c           |   2 +-
 hw/virtio/vhost-user-vsock-pci.c          |   2 +-
 hw/virtio/vhost-user.c                    | 133 ++++--
 hw/virtio/vhost-vsock-pci.c               |   2 +-
 hw/virtio/virtio-9p-pci.c                 |   2 +-
 hw/virtio/virtio-balloon-pci.c            |   2 +-
 hw/virtio/virtio-blk-pci.c                |   2 +-
 hw/virtio/virtio-input-host-pci.c         |   2 +-
 hw/virtio/virtio-input-pci.c              |   2 +-
 hw/virtio/virtio-iommu-pci.c              |   2 +-
 hw/virtio/virtio-net-pci.c                |   2 +-
 hw/virtio/virtio-pci.c                    |   5 +-
 hw/virtio/virtio-rng-pci.c                |   2 +-
 hw/virtio/virtio-scsi-pci.c               |   2 +-
 hw/virtio/virtio-serial-pci.c             |   2 +-
 subprojects/libvhost-user/libvhost-user.c |   2 +-
 contrib/vhost-user-blk/meson.build        |   3 +-
 hw/virtio/trace-events                    |  10 +-
 35 files changed, 831 insertions(+), 333 deletions(-)
 create mode 100644 docs/devel/virtio-backends.rst
 rename {hw => include/hw}/virtio/virtio-pci.h (100%)

-- 
2.30.2



^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v1 01/13] hw/virtio: move virtio-pci.h into shared include space
  2022-03-21 15:30 [PATCH v1 00/13] various virtio docs, fixes and tweaks Alex Bennée
@ 2022-03-21 15:30   ` Alex Bennée
  2022-03-21 15:30 ` [PATCH v1 02/13] virtio-pci: add notification trace points Alex Bennée
                     ` (13 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2022-03-21 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, David Hildenbrand,
	Dr. David Alan Gilbert, Raphael Norwitz, open list:virtiofs,
	Eric Auger, stefanha, marcandre.lureau, Alex Bennée

This allows other device classes that will be exposed via PCI to be
able to do so in the appropriate hw/ directory. I resisted the
temptation to re-order headers to be more aesthetically pleasing.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200925125147.26943-4-alex.bennee@linaro.org>

---
v2
  - add i2c/rng device to changes
---
 {hw => include/hw}/virtio/virtio-pci.h | 0
 hw/virtio/vhost-scsi-pci.c             | 2 +-
 hw/virtio/vhost-user-blk-pci.c         | 2 +-
 hw/virtio/vhost-user-fs-pci.c          | 2 +-
 hw/virtio/vhost-user-i2c-pci.c         | 2 +-
 hw/virtio/vhost-user-input-pci.c       | 2 +-
 hw/virtio/vhost-user-rng-pci.c         | 2 +-
 hw/virtio/vhost-user-scsi-pci.c        | 2 +-
 hw/virtio/vhost-user-vsock-pci.c       | 2 +-
 hw/virtio/vhost-vsock-pci.c            | 2 +-
 hw/virtio/virtio-9p-pci.c              | 2 +-
 hw/virtio/virtio-balloon-pci.c         | 2 +-
 hw/virtio/virtio-blk-pci.c             | 2 +-
 hw/virtio/virtio-input-host-pci.c      | 2 +-
 hw/virtio/virtio-input-pci.c           | 2 +-
 hw/virtio/virtio-iommu-pci.c           | 2 +-
 hw/virtio/virtio-net-pci.c             | 2 +-
 hw/virtio/virtio-pci.c                 | 2 +-
 hw/virtio/virtio-rng-pci.c             | 2 +-
 hw/virtio/virtio-scsi-pci.c            | 2 +-
 hw/virtio/virtio-serial-pci.c          | 2 +-
 21 files changed, 20 insertions(+), 20 deletions(-)
 rename {hw => include/hw}/virtio/virtio-pci.h (100%)

diff --git a/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
similarity index 100%
rename from hw/virtio/virtio-pci.h
rename to include/hw/virtio/virtio-pci.h
diff --git a/hw/virtio/vhost-scsi-pci.c b/hw/virtio/vhost-scsi-pci.c
index cb71a294fa..08980bc23b 100644
--- a/hw/virtio/vhost-scsi-pci.c
+++ b/hw/virtio/vhost-scsi-pci.c
@@ -21,7 +21,7 @@
 #include "hw/virtio/vhost-scsi.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qom/object.h"
 
 typedef struct VHostSCSIPCI VHostSCSIPCI;
diff --git a/hw/virtio/vhost-user-blk-pci.c b/hw/virtio/vhost-user-blk-pci.c
index 33b404d8a2..eef8641a98 100644
--- a/hw/virtio/vhost-user-blk-pci.c
+++ b/hw/virtio/vhost-user-blk-pci.c
@@ -26,7 +26,7 @@
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu/module.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qom/object.h"
 
 typedef struct VHostUserBlkPCI VHostUserBlkPCI;
diff --git a/hw/virtio/vhost-user-fs-pci.c b/hw/virtio/vhost-user-fs-pci.c
index 2ed8492b3f..6829b8b743 100644
--- a/hw/virtio/vhost-user-fs-pci.c
+++ b/hw/virtio/vhost-user-fs-pci.c
@@ -14,7 +14,7 @@
 #include "qemu/osdep.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/vhost-user-fs.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qom/object.h"
 
 struct VHostUserFSPCI {
diff --git a/hw/virtio/vhost-user-i2c-pci.c b/hw/virtio/vhost-user-i2c-pci.c
index 70b7b65fd9..00ac10941f 100644
--- a/hw/virtio/vhost-user-i2c-pci.c
+++ b/hw/virtio/vhost-user-i2c-pci.c
@@ -9,7 +9,7 @@
 #include "qemu/osdep.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/vhost-user-i2c.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 
 struct VHostUserI2CPCI {
     VirtIOPCIProxy parent_obj;
diff --git a/hw/virtio/vhost-user-input-pci.c b/hw/virtio/vhost-user-input-pci.c
index c9d3e9113a..b858898a36 100644
--- a/hw/virtio/vhost-user-input-pci.c
+++ b/hw/virtio/vhost-user-input-pci.c
@@ -9,7 +9,7 @@
 #include "hw/virtio/virtio-input.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qom/object.h"
 
 typedef struct VHostUserInputPCI VHostUserInputPCI;
diff --git a/hw/virtio/vhost-user-rng-pci.c b/hw/virtio/vhost-user-rng-pci.c
index c83dc86813..f64935453b 100644
--- a/hw/virtio/vhost-user-rng-pci.c
+++ b/hw/virtio/vhost-user-rng-pci.c
@@ -9,7 +9,7 @@
 #include "qemu/osdep.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/vhost-user-rng.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 
 struct VHostUserRNGPCI {
     VirtIOPCIProxy parent_obj;
diff --git a/hw/virtio/vhost-user-scsi-pci.c b/hw/virtio/vhost-user-scsi-pci.c
index d5343412a1..75882e3cf9 100644
--- a/hw/virtio/vhost-user-scsi-pci.c
+++ b/hw/virtio/vhost-user-scsi-pci.c
@@ -30,7 +30,7 @@
 #include "hw/pci/msix.h"
 #include "hw/loader.h"
 #include "sysemu/kvm.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qom/object.h"
 
 typedef struct VHostUserSCSIPCI VHostUserSCSIPCI;
diff --git a/hw/virtio/vhost-user-vsock-pci.c b/hw/virtio/vhost-user-vsock-pci.c
index 72a96199cd..e5a86e8013 100644
--- a/hw/virtio/vhost-user-vsock-pci.c
+++ b/hw/virtio/vhost-user-vsock-pci.c
@@ -10,7 +10,7 @@
 
 #include "qemu/osdep.h"
 
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/vhost-user-vsock.h"
 #include "qom/object.h"
diff --git a/hw/virtio/vhost-vsock-pci.c b/hw/virtio/vhost-vsock-pci.c
index 205da8d1f5..9f34414d38 100644
--- a/hw/virtio/vhost-vsock-pci.c
+++ b/hw/virtio/vhost-vsock-pci.c
@@ -13,7 +13,7 @@
 
 #include "qemu/osdep.h"
 
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/vhost-vsock.h"
 #include "qemu/module.h"
diff --git a/hw/virtio/virtio-9p-pci.c b/hw/virtio/virtio-9p-pci.c
index e07adcd9ea..94c14f0b98 100644
--- a/hw/virtio/virtio-9p-pci.c
+++ b/hw/virtio/virtio-9p-pci.c
@@ -15,7 +15,7 @@
 
 #include "qemu/osdep.h"
 
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/9pfs/virtio-9p.h"
 #include "hw/qdev-properties.h"
 #include "qemu/module.h"
diff --git a/hw/virtio/virtio-balloon-pci.c b/hw/virtio/virtio-balloon-pci.c
index 79a3ba979a..ce2645ba71 100644
--- a/hw/virtio/virtio-balloon-pci.c
+++ b/hw/virtio/virtio-balloon-pci.c
@@ -14,7 +14,7 @@
 
 #include "qemu/osdep.h"
 
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-balloon.h"
 #include "qapi/error.h"
diff --git a/hw/virtio/virtio-blk-pci.c b/hw/virtio/virtio-blk-pci.c
index 9d5795810c..9743bee965 100644
--- a/hw/virtio/virtio-blk-pci.c
+++ b/hw/virtio/virtio-blk-pci.c
@@ -19,7 +19,7 @@
 
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-blk.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
 #include "qom/object.h"
diff --git a/hw/virtio/virtio-input-host-pci.c b/hw/virtio/virtio-input-host-pci.c
index 0ac360de4f..cf8a9cf9e8 100644
--- a/hw/virtio/virtio-input-host-pci.c
+++ b/hw/virtio/virtio-input-host-pci.c
@@ -8,7 +8,7 @@
 
 #include "qemu/osdep.h"
 
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/virtio/virtio-input.h"
 #include "qemu/module.h"
 #include "qom/object.h"
diff --git a/hw/virtio/virtio-input-pci.c b/hw/virtio/virtio-input-pci.c
index 48e9ff38e2..a9d0992389 100644
--- a/hw/virtio/virtio-input-pci.c
+++ b/hw/virtio/virtio-input-pci.c
@@ -8,7 +8,7 @@
 
 #include "qemu/osdep.h"
 
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-input.h"
 #include "qemu/module.h"
diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
index 6a1df7fe50..844d647704 100644
--- a/hw/virtio/virtio-iommu-pci.c
+++ b/hw/virtio/virtio-iommu-pci.c
@@ -11,7 +11,7 @@
 
 #include "qemu/osdep.h"
 
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/virtio/virtio-iommu.h"
 #include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
diff --git a/hw/virtio/virtio-net-pci.c b/hw/virtio/virtio-net-pci.c
index aa0b3caecb..e03543a70a 100644
--- a/hw/virtio/virtio-net-pci.c
+++ b/hw/virtio/virtio-net-pci.c
@@ -19,7 +19,7 @@
 
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-net.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
 #include "qom/object.h"
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 7cf1231c1c..602be7f83d 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -33,7 +33,7 @@
 #include "hw/pci/msix.h"
 #include "hw/loader.h"
 #include "sysemu/kvm.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qemu/range.h"
 #include "hw/virtio/virtio-bus.h"
 #include "qapi/visitor.h"
diff --git a/hw/virtio/virtio-rng-pci.c b/hw/virtio/virtio-rng-pci.c
index c1f916268b..151ece6f94 100644
--- a/hw/virtio/virtio-rng-pci.c
+++ b/hw/virtio/virtio-rng-pci.c
@@ -11,7 +11,7 @@
 
 #include "qemu/osdep.h"
 
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/virtio/virtio-rng.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
diff --git a/hw/virtio/virtio-scsi-pci.c b/hw/virtio/virtio-scsi-pci.c
index 97fab74236..e8e3442f38 100644
--- a/hw/virtio/virtio-scsi-pci.c
+++ b/hw/virtio/virtio-scsi-pci.c
@@ -18,7 +18,7 @@
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-scsi.h"
 #include "qemu/module.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qom/object.h"
 
 typedef struct VirtIOSCSIPCI VirtIOSCSIPCI;
diff --git a/hw/virtio/virtio-serial-pci.c b/hw/virtio/virtio-serial-pci.c
index 35bcd961c9..cea31adcc4 100644
--- a/hw/virtio/virtio-serial-pci.c
+++ b/hw/virtio/virtio-serial-pci.c
@@ -20,7 +20,7 @@
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-serial.h"
 #include "qemu/module.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qom/object.h"
 
 typedef struct VirtIOSerialPCI VirtIOSerialPCI;
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Virtio-fs] [PATCH v1 01/13] hw/virtio: move virtio-pci.h into shared include space
@ 2022-03-21 15:30   ` Alex Bennée
  0 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2022-03-21 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: slp, mst, marcandre.lureau, stefanha, mathieu.poirier,
	viresh.kumar, Alex Bennée, Raphael Norwitz,
	Dr. David Alan Gilbert, David Hildenbrand, Eric Auger,
	open list:virtiofs

This allows other device classes that will be exposed via PCI to be
able to do so in the appropriate hw/ directory. I resisted the
temptation to re-order headers to be more aesthetically pleasing.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200925125147.26943-4-alex.bennee@linaro.org>

---
v2
  - add i2c/rng device to changes
---
 {hw => include/hw}/virtio/virtio-pci.h | 0
 hw/virtio/vhost-scsi-pci.c             | 2 +-
 hw/virtio/vhost-user-blk-pci.c         | 2 +-
 hw/virtio/vhost-user-fs-pci.c          | 2 +-
 hw/virtio/vhost-user-i2c-pci.c         | 2 +-
 hw/virtio/vhost-user-input-pci.c       | 2 +-
 hw/virtio/vhost-user-rng-pci.c         | 2 +-
 hw/virtio/vhost-user-scsi-pci.c        | 2 +-
 hw/virtio/vhost-user-vsock-pci.c       | 2 +-
 hw/virtio/vhost-vsock-pci.c            | 2 +-
 hw/virtio/virtio-9p-pci.c              | 2 +-
 hw/virtio/virtio-balloon-pci.c         | 2 +-
 hw/virtio/virtio-blk-pci.c             | 2 +-
 hw/virtio/virtio-input-host-pci.c      | 2 +-
 hw/virtio/virtio-input-pci.c           | 2 +-
 hw/virtio/virtio-iommu-pci.c           | 2 +-
 hw/virtio/virtio-net-pci.c             | 2 +-
 hw/virtio/virtio-pci.c                 | 2 +-
 hw/virtio/virtio-rng-pci.c             | 2 +-
 hw/virtio/virtio-scsi-pci.c            | 2 +-
 hw/virtio/virtio-serial-pci.c          | 2 +-
 21 files changed, 20 insertions(+), 20 deletions(-)
 rename {hw => include/hw}/virtio/virtio-pci.h (100%)

diff --git a/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
similarity index 100%
rename from hw/virtio/virtio-pci.h
rename to include/hw/virtio/virtio-pci.h
diff --git a/hw/virtio/vhost-scsi-pci.c b/hw/virtio/vhost-scsi-pci.c
index cb71a294fa..08980bc23b 100644
--- a/hw/virtio/vhost-scsi-pci.c
+++ b/hw/virtio/vhost-scsi-pci.c
@@ -21,7 +21,7 @@
 #include "hw/virtio/vhost-scsi.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qom/object.h"
 
 typedef struct VHostSCSIPCI VHostSCSIPCI;
diff --git a/hw/virtio/vhost-user-blk-pci.c b/hw/virtio/vhost-user-blk-pci.c
index 33b404d8a2..eef8641a98 100644
--- a/hw/virtio/vhost-user-blk-pci.c
+++ b/hw/virtio/vhost-user-blk-pci.c
@@ -26,7 +26,7 @@
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu/module.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qom/object.h"
 
 typedef struct VHostUserBlkPCI VHostUserBlkPCI;
diff --git a/hw/virtio/vhost-user-fs-pci.c b/hw/virtio/vhost-user-fs-pci.c
index 2ed8492b3f..6829b8b743 100644
--- a/hw/virtio/vhost-user-fs-pci.c
+++ b/hw/virtio/vhost-user-fs-pci.c
@@ -14,7 +14,7 @@
 #include "qemu/osdep.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/vhost-user-fs.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qom/object.h"
 
 struct VHostUserFSPCI {
diff --git a/hw/virtio/vhost-user-i2c-pci.c b/hw/virtio/vhost-user-i2c-pci.c
index 70b7b65fd9..00ac10941f 100644
--- a/hw/virtio/vhost-user-i2c-pci.c
+++ b/hw/virtio/vhost-user-i2c-pci.c
@@ -9,7 +9,7 @@
 #include "qemu/osdep.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/vhost-user-i2c.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 
 struct VHostUserI2CPCI {
     VirtIOPCIProxy parent_obj;
diff --git a/hw/virtio/vhost-user-input-pci.c b/hw/virtio/vhost-user-input-pci.c
index c9d3e9113a..b858898a36 100644
--- a/hw/virtio/vhost-user-input-pci.c
+++ b/hw/virtio/vhost-user-input-pci.c
@@ -9,7 +9,7 @@
 #include "hw/virtio/virtio-input.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qom/object.h"
 
 typedef struct VHostUserInputPCI VHostUserInputPCI;
diff --git a/hw/virtio/vhost-user-rng-pci.c b/hw/virtio/vhost-user-rng-pci.c
index c83dc86813..f64935453b 100644
--- a/hw/virtio/vhost-user-rng-pci.c
+++ b/hw/virtio/vhost-user-rng-pci.c
@@ -9,7 +9,7 @@
 #include "qemu/osdep.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/vhost-user-rng.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 
 struct VHostUserRNGPCI {
     VirtIOPCIProxy parent_obj;
diff --git a/hw/virtio/vhost-user-scsi-pci.c b/hw/virtio/vhost-user-scsi-pci.c
index d5343412a1..75882e3cf9 100644
--- a/hw/virtio/vhost-user-scsi-pci.c
+++ b/hw/virtio/vhost-user-scsi-pci.c
@@ -30,7 +30,7 @@
 #include "hw/pci/msix.h"
 #include "hw/loader.h"
 #include "sysemu/kvm.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qom/object.h"
 
 typedef struct VHostUserSCSIPCI VHostUserSCSIPCI;
diff --git a/hw/virtio/vhost-user-vsock-pci.c b/hw/virtio/vhost-user-vsock-pci.c
index 72a96199cd..e5a86e8013 100644
--- a/hw/virtio/vhost-user-vsock-pci.c
+++ b/hw/virtio/vhost-user-vsock-pci.c
@@ -10,7 +10,7 @@
 
 #include "qemu/osdep.h"
 
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/vhost-user-vsock.h"
 #include "qom/object.h"
diff --git a/hw/virtio/vhost-vsock-pci.c b/hw/virtio/vhost-vsock-pci.c
index 205da8d1f5..9f34414d38 100644
--- a/hw/virtio/vhost-vsock-pci.c
+++ b/hw/virtio/vhost-vsock-pci.c
@@ -13,7 +13,7 @@
 
 #include "qemu/osdep.h"
 
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/vhost-vsock.h"
 #include "qemu/module.h"
diff --git a/hw/virtio/virtio-9p-pci.c b/hw/virtio/virtio-9p-pci.c
index e07adcd9ea..94c14f0b98 100644
--- a/hw/virtio/virtio-9p-pci.c
+++ b/hw/virtio/virtio-9p-pci.c
@@ -15,7 +15,7 @@
 
 #include "qemu/osdep.h"
 
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/9pfs/virtio-9p.h"
 #include "hw/qdev-properties.h"
 #include "qemu/module.h"
diff --git a/hw/virtio/virtio-balloon-pci.c b/hw/virtio/virtio-balloon-pci.c
index 79a3ba979a..ce2645ba71 100644
--- a/hw/virtio/virtio-balloon-pci.c
+++ b/hw/virtio/virtio-balloon-pci.c
@@ -14,7 +14,7 @@
 
 #include "qemu/osdep.h"
 
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-balloon.h"
 #include "qapi/error.h"
diff --git a/hw/virtio/virtio-blk-pci.c b/hw/virtio/virtio-blk-pci.c
index 9d5795810c..9743bee965 100644
--- a/hw/virtio/virtio-blk-pci.c
+++ b/hw/virtio/virtio-blk-pci.c
@@ -19,7 +19,7 @@
 
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-blk.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
 #include "qom/object.h"
diff --git a/hw/virtio/virtio-input-host-pci.c b/hw/virtio/virtio-input-host-pci.c
index 0ac360de4f..cf8a9cf9e8 100644
--- a/hw/virtio/virtio-input-host-pci.c
+++ b/hw/virtio/virtio-input-host-pci.c
@@ -8,7 +8,7 @@
 
 #include "qemu/osdep.h"
 
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/virtio/virtio-input.h"
 #include "qemu/module.h"
 #include "qom/object.h"
diff --git a/hw/virtio/virtio-input-pci.c b/hw/virtio/virtio-input-pci.c
index 48e9ff38e2..a9d0992389 100644
--- a/hw/virtio/virtio-input-pci.c
+++ b/hw/virtio/virtio-input-pci.c
@@ -8,7 +8,7 @@
 
 #include "qemu/osdep.h"
 
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-input.h"
 #include "qemu/module.h"
diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
index 6a1df7fe50..844d647704 100644
--- a/hw/virtio/virtio-iommu-pci.c
+++ b/hw/virtio/virtio-iommu-pci.c
@@ -11,7 +11,7 @@
 
 #include "qemu/osdep.h"
 
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/virtio/virtio-iommu.h"
 #include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
diff --git a/hw/virtio/virtio-net-pci.c b/hw/virtio/virtio-net-pci.c
index aa0b3caecb..e03543a70a 100644
--- a/hw/virtio/virtio-net-pci.c
+++ b/hw/virtio/virtio-net-pci.c
@@ -19,7 +19,7 @@
 
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-net.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
 #include "qom/object.h"
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 7cf1231c1c..602be7f83d 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -33,7 +33,7 @@
 #include "hw/pci/msix.h"
 #include "hw/loader.h"
 #include "sysemu/kvm.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qemu/range.h"
 #include "hw/virtio/virtio-bus.h"
 #include "qapi/visitor.h"
diff --git a/hw/virtio/virtio-rng-pci.c b/hw/virtio/virtio-rng-pci.c
index c1f916268b..151ece6f94 100644
--- a/hw/virtio/virtio-rng-pci.c
+++ b/hw/virtio/virtio-rng-pci.c
@@ -11,7 +11,7 @@
 
 #include "qemu/osdep.h"
 
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/virtio/virtio-rng.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
diff --git a/hw/virtio/virtio-scsi-pci.c b/hw/virtio/virtio-scsi-pci.c
index 97fab74236..e8e3442f38 100644
--- a/hw/virtio/virtio-scsi-pci.c
+++ b/hw/virtio/virtio-scsi-pci.c
@@ -18,7 +18,7 @@
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-scsi.h"
 #include "qemu/module.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qom/object.h"
 
 typedef struct VirtIOSCSIPCI VirtIOSCSIPCI;
diff --git a/hw/virtio/virtio-serial-pci.c b/hw/virtio/virtio-serial-pci.c
index 35bcd961c9..cea31adcc4 100644
--- a/hw/virtio/virtio-serial-pci.c
+++ b/hw/virtio/virtio-serial-pci.c
@@ -20,7 +20,7 @@
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-serial.h"
 #include "qemu/module.h"
-#include "virtio-pci.h"
+#include "hw/virtio/virtio-pci.h"
 #include "qom/object.h"
 
 typedef struct VirtIOSerialPCI VirtIOSerialPCI;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH  v1 02/13] virtio-pci: add notification trace points
  2022-03-21 15:30 [PATCH v1 00/13] various virtio docs, fixes and tweaks Alex Bennée
  2022-03-21 15:30   ` [Virtio-fs] " Alex Bennée
@ 2022-03-21 15:30 ` Alex Bennée
  2022-03-21 15:30 ` [PATCH v1 03/13] hw/virtio: add vhost_user_[read|write] " Alex Bennée
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2022-03-21 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar,
	Philippe Mathieu-Daudé,
	stefanha, marcandre.lureau, Alex Bennée

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200925125147.26943-6-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 hw/virtio/virtio-pci.c | 3 +++
 hw/virtio/trace-events | 7 ++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 602be7f83d..0566ad7d00 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -38,6 +38,7 @@
 #include "hw/virtio/virtio-bus.h"
 #include "qapi/visitor.h"
 #include "sysemu/replay.h"
+#include "trace.h"
 
 #define VIRTIO_PCI_REGION_SIZE(dev)     VIRTIO_PCI_CONFIG_OFF(msix_present(dev))
 
@@ -1380,6 +1381,7 @@ static void virtio_pci_notify_write(void *opaque, hwaddr addr,
     unsigned queue = addr / virtio_pci_queue_mem_mult(proxy);
 
     if (vdev != NULL && queue < VIRTIO_QUEUE_MAX) {
+        trace_virtio_pci_notify_write(addr, val, size);
         virtio_queue_notify(vdev, queue);
     }
 }
@@ -1393,6 +1395,7 @@ static void virtio_pci_notify_write_pio(void *opaque, hwaddr addr,
     unsigned queue = val;
 
     if (vdev != NULL && queue < VIRTIO_QUEUE_MAX) {
+        trace_virtio_pci_notify_write_pio(addr, val, size);
         virtio_queue_notify(vdev, queue);
     }
 }
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index a5102eac9e..46851a7cd1 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -87,7 +87,12 @@ virtio_mmio_guest_page(uint64_t size, int shift) "guest page size 0x%" PRIx64 "
 virtio_mmio_queue_write(uint64_t value, int max_size) "mmio_queue write 0x%" PRIx64 " max %d"
 virtio_mmio_setting_irq(int level) "virtio_mmio setting IRQ %d"
 
-# virtio-iommu.c
+# virtio-pci.c
+virtio_pci_notify(uint16_t vector) "virtio_pci_notify vec 0x%x"
+virtio_pci_notify_write(uint64_t addr, uint64_t val, unsigned int size) "0x%" PRIx64" = 0x%" PRIx64 " (%d)"
+virtio_pci_notify_write_pio(uint64_t addr, uint64_t val, unsigned int size) "0x%" PRIx64" = 0x%" PRIx64 " (%d)"
+
+# hw/virtio/virtio-iommu.c
 virtio_iommu_device_reset(void) "reset!"
 virtio_iommu_system_reset(void) "system reset!"
 virtio_iommu_get_features(uint64_t features) "device supports features=0x%"PRIx64
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 03/13] hw/virtio: add vhost_user_[read|write] trace points
  2022-03-21 15:30 [PATCH v1 00/13] various virtio docs, fixes and tweaks Alex Bennée
  2022-03-21 15:30   ` [Virtio-fs] " Alex Bennée
  2022-03-21 15:30 ` [PATCH v1 02/13] virtio-pci: add notification trace points Alex Bennée
@ 2022-03-21 15:30 ` Alex Bennée
  2022-03-21 22:29   ` Philippe Mathieu-Daudé
  2022-03-21 15:30 ` [PATCH v1 04/13] docs: vhost-user: clean up request/reply description Alex Bennée
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2022-03-21 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, stefanha,
	marcandre.lureau, Alex Bennée

These are useful when trying to debug the initial vhost-user
negotiation, especially when it hard to get logging from the low level
library on the other side.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v2
  - fixed arguments
---
 hw/virtio/vhost-user.c | 4 ++++
 hw/virtio/trace-events | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 6abbc9da32..b27b8c56e2 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -489,6 +489,8 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
         return ret < 0 ? -saved_errno : -EIO;
     }
 
+    trace_vhost_user_write(msg->hdr.request, msg->hdr.flags);
+
     return 0;
 }
 
@@ -542,6 +544,8 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,
         }
     }
 
+    trace_vhost_user_read(msg.hdr.request, msg.hdr.flags);
+
     return 0;
 }
 
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 46851a7cd1..fd213e2a27 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -21,6 +21,8 @@ vhost_user_set_mem_table_withfd(int index, const char *name, uint64_t memory_siz
 vhost_user_postcopy_waker(const char *rb, uint64_t rb_offset) "%s + 0x%"PRIx64
 vhost_user_postcopy_waker_found(uint64_t client_addr) "0x%"PRIx64
 vhost_user_postcopy_waker_nomatch(const char *rb, uint64_t rb_offset) "%s + 0x%"PRIx64
+vhost_user_read(uint32_t req, uint32_t flags) "req:%d flags:0x%"PRIx32""
+vhost_user_write(uint32_t req, uint32_t flags) "req:%d flags:0x%"PRIx32""
 
 # vhost-vdpa.c
 vhost_vdpa_dma_map(void *vdpa, int fd, uint32_t msg_type, uint64_t iova, uint64_t size, uint64_t uaddr, uint8_t perm, uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32" iova: 0x%"PRIx64" size: 0x%"PRIx64" uaddr: 0x%"PRIx64" perm: 0x%"PRIx8" type: %"PRIu8
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 04/13] docs: vhost-user: clean up request/reply description
  2022-03-21 15:30 [PATCH v1 00/13] various virtio docs, fixes and tweaks Alex Bennée
                   ` (2 preceding siblings ...)
  2022-03-21 15:30 ` [PATCH v1 03/13] hw/virtio: add vhost_user_[read|write] " Alex Bennée
@ 2022-03-21 15:30 ` Alex Bennée
  2022-03-21 22:30   ` Philippe Mathieu-Daudé
  2022-03-21 15:30 ` [PATCH v1 05/13] docs: vhost-user: rewrite section on ring state machine Alex Bennée
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2022-03-21 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, stefanha, Paolo Bonzini,
	marcandre.lureau

From: Paolo Bonzini <pbonzini@redhat.com>

It is not necessary to mention which side is sending/receiving
each payload; it is more interesting to say which is the request
and which is the reply.  This also matches what vhost-user-gpu.rst
already does.

While at it, ensure that all messages list both the request and
the reply payload.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210226143413.188046-2-pbonzini@redhat.com>
---
 docs/interop/vhost-user.rst | 163 +++++++++++++++++++++---------------
 1 file changed, 95 insertions(+), 68 deletions(-)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 4dbc84fd00..bb588c66fc 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -865,8 +865,8 @@ Master message types
 ``VHOST_USER_GET_FEATURES``
   :id: 1
   :equivalent ioctl: ``VHOST_GET_FEATURES``
-  :master payload: N/A
-  :slave payload: ``u64``
+  :request payload: N/A
+  :reply payload: ``u64``
 
   Get from the underlying vhost implementation the features bitmask.
   Feature bit ``VHOST_USER_F_PROTOCOL_FEATURES`` signals slave support
@@ -876,7 +876,8 @@ Master message types
 ``VHOST_USER_SET_FEATURES``
   :id: 2
   :equivalent ioctl: ``VHOST_SET_FEATURES``
-  :master payload: ``u64``
+  :request payload: ``u64``
+  :reply payload: N/A
 
   Enable features in the underlying vhost implementation using a
   bitmask.  Feature bit ``VHOST_USER_F_PROTOCOL_FEATURES`` signals
@@ -886,8 +887,8 @@ Master message types
 ``VHOST_USER_GET_PROTOCOL_FEATURES``
   :id: 15
   :equivalent ioctl: ``VHOST_GET_FEATURES``
-  :master payload: N/A
-  :slave payload: ``u64``
+  :request payload: N/A
+  :reply payload: ``u64``
 
   Get the protocol feature bitmask from the underlying vhost
   implementation.  Only legal if feature bit
@@ -902,7 +903,8 @@ Master message types
 ``VHOST_USER_SET_PROTOCOL_FEATURES``
   :id: 16
   :equivalent ioctl: ``VHOST_SET_FEATURES``
-  :master payload: ``u64``
+  :request payload: ``u64``
+  :reply payload: N/A
 
   Enable protocol features in the underlying vhost implementation.
 
@@ -916,7 +918,8 @@ Master message types
 ``VHOST_USER_SET_OWNER``
   :id: 3
   :equivalent ioctl: ``VHOST_SET_OWNER``
-  :master payload: N/A
+  :request payload: N/A
+  :reply payload: N/A
 
   Issued when a new connection is established. It sets the current
   *master* as an owner of the session. This can be used on the *slave*
@@ -924,7 +927,8 @@ Master message types
 
 ``VHOST_USER_RESET_OWNER``
   :id: 4
-  :master payload: N/A
+  :request payload: N/A
+  :reply payload: N/A
 
 .. admonition:: Deprecated
 
@@ -937,8 +941,8 @@ Master message types
 ``VHOST_USER_SET_MEM_TABLE``
   :id: 5
   :equivalent ioctl: ``VHOST_SET_MEM_TABLE``
-  :master payload: memory regions description
-  :slave payload: (postcopy only) memory regions description
+  :request payload: memory regions description
+  :reply payload: (postcopy only) memory regions description
 
   Sets the memory map regions on the slave so it can translate the
   vring addresses. In the ancillary data there is an array of file
@@ -961,8 +965,8 @@ Master message types
 ``VHOST_USER_SET_LOG_BASE``
   :id: 6
   :equivalent ioctl: ``VHOST_SET_LOG_BASE``
-  :master payload: u64
-  :slave payload: N/A
+  :request payload: u64
+  :reply payload: N/A
 
   Sets logging shared memory space.
 
@@ -974,44 +978,48 @@ Master message types
 ``VHOST_USER_SET_LOG_FD``
   :id: 7
   :equivalent ioctl: ``VHOST_SET_LOG_FD``
-  :master payload: N/A
+  :request payload: N/A
+  :reply payload: N/A
 
   Sets the logging file descriptor, which is passed as ancillary data.
 
 ``VHOST_USER_SET_VRING_NUM``
   :id: 8
   :equivalent ioctl: ``VHOST_SET_VRING_NUM``
-  :master payload: vring state description
+  :request payload: vring state description
+  :reply payload: N/A
 
   Set the size of the queue.
 
 ``VHOST_USER_SET_VRING_ADDR``
   :id: 9
   :equivalent ioctl: ``VHOST_SET_VRING_ADDR``
-  :master payload: vring address description
-  :slave payload: N/A
+  :request payload: vring address description
+  :reply payload: N/A
 
   Sets the addresses of the different aspects of the vring.
 
 ``VHOST_USER_SET_VRING_BASE``
   :id: 10
   :equivalent ioctl: ``VHOST_SET_VRING_BASE``
-  :master payload: vring state description
+  :request payload: vring state description
+  :reply payload: N/A
 
   Sets the base offset in the available vring.
 
 ``VHOST_USER_GET_VRING_BASE``
   :id: 11
   :equivalent ioctl: ``VHOST_USER_GET_VRING_BASE``
-  :master payload: vring state description
-  :slave payload: vring state description
+  :request payload: vring state description
+  :reply payload: vring state description
 
   Get the available vring base offset.
 
 ``VHOST_USER_SET_VRING_KICK``
   :id: 12
   :equivalent ioctl: ``VHOST_SET_VRING_KICK``
-  :master payload: ``u64``
+  :request payload: ``u64``
+  :reply payload: N/A
 
   Set the event file descriptor for adding buffers to the vring. It is
   passed in the ancillary data.
@@ -1029,7 +1037,8 @@ Master message types
 ``VHOST_USER_SET_VRING_CALL``
   :id: 13
   :equivalent ioctl: ``VHOST_SET_VRING_CALL``
-  :master payload: ``u64``
+  :request payload: ``u64``
+  :reply payload: N/A
 
   Set the event file descriptor to signal when buffers are used. It is
   passed in the ancillary data.
@@ -1047,7 +1056,8 @@ Master message types
 ``VHOST_USER_SET_VRING_ERR``
   :id: 14
   :equivalent ioctl: ``VHOST_SET_VRING_ERR``
-  :master payload: ``u64``
+  :request payload: ``u64``
+  :reply payload: N/A
 
   Set the event file descriptor to signal when error occurs. It is
   passed in the ancillary data.
@@ -1064,8 +1074,8 @@ Master message types
 ``VHOST_USER_GET_QUEUE_NUM``
   :id: 17
   :equivalent ioctl: N/A
-  :master payload: N/A
-  :slave payload: u64
+  :request payload: N/A
+  :reply payload: u64
 
   Query how many queues the backend supports.
 
@@ -1076,7 +1086,8 @@ Master message types
 ``VHOST_USER_SET_VRING_ENABLE``
   :id: 18
   :equivalent ioctl: N/A
-  :master payload: vring state description
+  :request payload: vring state description
+  :reply payload: N/A
 
   Signal slave to enable or disable corresponding vring.
 
@@ -1086,7 +1097,8 @@ Master message types
 ``VHOST_USER_SEND_RARP``
   :id: 19
   :equivalent ioctl: N/A
-  :master payload: ``u64``
+  :request payload: ``u64``
+  :reply payload: N/A
 
   Ask vhost user backend to broadcast a fake RARP to notify the migration
   is terminated for guest that does not support GUEST_ANNOUNCE.
@@ -1101,7 +1113,8 @@ Master message types
 ``VHOST_USER_NET_SET_MTU``
   :id: 20
   :equivalent ioctl: N/A
-  :master payload: ``u64``
+  :request payload: ``u64``
+  :reply payload: N/A
 
   Set host MTU value exposed to the guest.
 
@@ -1118,7 +1131,8 @@ Master message types
 ``VHOST_USER_SET_SLAVE_REQ_FD``
   :id: 21
   :equivalent ioctl: N/A
-  :master payload: N/A
+  :request payload: N/A
+  :reply payload: N/A
 
   Set the socket file descriptor for slave initiated requests. It is passed
   in the ancillary data.
@@ -1133,8 +1147,8 @@ Master message types
 ``VHOST_USER_IOTLB_MSG``
   :id: 22
   :equivalent ioctl: N/A (equivalent to ``VHOST_IOTLB_MSG`` message type)
-  :master payload: ``struct vhost_iotlb_msg``
-  :slave payload: ``u64``
+  :request payload: ``struct vhost_iotlb_msg``
+  :reply payload: ``u64``
 
   Send IOTLB messages with ``struct vhost_iotlb_msg`` as payload.
 
@@ -1148,7 +1162,8 @@ Master message types
 ``VHOST_USER_SET_VRING_ENDIAN``
   :id: 23
   :equivalent ioctl: ``VHOST_SET_VRING_ENDIAN``
-  :master payload: vring state description
+  :request payload: vring state description
+  :reply payload: N/A
 
   Set the endianness of a VQ for legacy devices. Little-endian is
   indicated with state.num set to 0 and big-endian is indicated with
@@ -1163,8 +1178,8 @@ Master message types
 ``VHOST_USER_GET_CONFIG``
   :id: 24
   :equivalent ioctl: N/A
-  :master payload: virtio device config space
-  :slave payload: virtio device config space
+  :request payload: virtio device config space
+  :reply payload: virtio device config space
 
   When ``VHOST_USER_PROTOCOL_F_CONFIG`` is negotiated, this message is
   submitted by the vhost-user master to fetch the contents of the
@@ -1177,8 +1192,8 @@ Master message types
 ``VHOST_USER_SET_CONFIG``
   :id: 25
   :equivalent ioctl: N/A
-  :master payload: virtio device config space
-  :slave payload: N/A
+  :request payload: virtio device config space
+  :reply payload: N/A
 
   When ``VHOST_USER_PROTOCOL_F_CONFIG`` is negotiated, this message is
   submitted by the vhost-user master when the Guest changes the virtio
@@ -1190,8 +1205,8 @@ Master message types
 ``VHOST_USER_CREATE_CRYPTO_SESSION``
   :id: 26
   :equivalent ioctl: N/A
-  :master payload: crypto session description
-  :slave payload: crypto session description
+  :request payload: crypto session description
+  :reply payload: crypto session description
 
   Create a session for crypto operation. The server side must return
   the session id, 0 or positive for success, negative for failure.
@@ -1203,7 +1218,8 @@ Master message types
 ``VHOST_USER_CLOSE_CRYPTO_SESSION``
   :id: 27
   :equivalent ioctl: N/A
-  :master payload: ``u64``
+  :request payload: ``u64``
+  :reply payload: N/A
 
   Close a session for crypto operation which was previously
   created by ``VHOST_USER_CREATE_CRYPTO_SESSION``.
@@ -1215,8 +1231,8 @@ Master message types
 
 ``VHOST_USER_POSTCOPY_ADVISE``
   :id: 28
-  :master payload: N/A
-  :slave payload: userfault fd
+  :request payload: N/A
+  :reply payload: userfault fd
 
   When ``VHOST_USER_PROTOCOL_F_PAGEFAULT`` is supported, the master
   advises slave that a migration with postcopy enabled is underway,
@@ -1225,7 +1241,8 @@ Master message types
 
 ``VHOST_USER_POSTCOPY_LISTEN``
   :id: 29
-  :master payload: N/A
+  :request payload: N/A
+  :reply payload: N/A
 
   Master advises slave that a transition to postcopy mode has
   happened.  The slave must ensure that shared memory is registered
@@ -1236,10 +1253,11 @@ Master message types
 
 ``VHOST_USER_POSTCOPY_END``
   :id: 30
-  :slave payload: ``u64``
+  :request payload: N/A
+  :reply payload: ``u64``
 
   Master advises that postcopy migration has now completed.  The slave
-  must disable the userfaultfd. The response is an acknowledgement
+  must disable the userfaultfd. The reply is an acknowledgement
   only.
 
   When ``VHOST_USER_PROTOCOL_F_PAGEFAULT`` is supported, this message
@@ -1251,7 +1269,8 @@ Master message types
 ``VHOST_USER_GET_INFLIGHT_FD``
   :id: 31
   :equivalent ioctl: N/A
-  :master payload: inflight description
+  :request payload: inflight description
+  :reply payload: N/A
 
   When ``VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD`` protocol feature has
   been successfully negotiated, this message is submitted by master to
@@ -1262,7 +1281,8 @@ Master message types
 ``VHOST_USER_SET_INFLIGHT_FD``
   :id: 32
   :equivalent ioctl: N/A
-  :master payload: inflight description
+  :request payload: inflight description
+  :reply payload: N/A
 
   When ``VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD`` protocol feature has
   been successfully negotiated, this message is submitted by master to
@@ -1272,7 +1292,8 @@ Master message types
 ``VHOST_USER_GPU_SET_SOCKET``
   :id: 33
   :equivalent ioctl: N/A
-  :master payload: N/A
+  :request payload: N/A
+  :reply payload: N/A
 
   Sets the GPU protocol socket file descriptor, which is passed as
   ancillary data. The GPU protocol is used to inform the master of
@@ -1281,8 +1302,8 @@ Master message types
 ``VHOST_USER_RESET_DEVICE``
   :id: 34
   :equivalent ioctl: N/A
-  :master payload: N/A
-  :slave payload: N/A
+  :request payload: N/A
+  :reply payload: N/A
 
   Ask the vhost user backend to disable all rings and reset all
   internal device state to the initial state, ready to be
@@ -1295,8 +1316,8 @@ Master message types
 ``VHOST_USER_VRING_KICK``
   :id: 35
   :equivalent ioctl: N/A
-  :slave payload: vring state description
-  :master payload: N/A
+  :request payload: vring state description
+  :reply payload: N/A
 
   When the ``VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS`` protocol
   feature has been successfully negotiated, this message may be
@@ -1309,7 +1330,8 @@ Master message types
 ``VHOST_USER_GET_MAX_MEM_SLOTS``
   :id: 36
   :equivalent ioctl: N/A
-  :slave payload: u64
+  :request payload: N/A
+  :reply payload: u64
 
   When the ``VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS`` protocol
   feature has been successfully negotiated, this message is submitted
@@ -1322,7 +1344,8 @@ Master message types
 ``VHOST_USER_ADD_MEM_REG``
   :id: 37
   :equivalent ioctl: N/A
-  :slave payload: single memory region description
+  :request payload: N/A
+  :reply payload: single memory region description
 
   When the ``VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS`` protocol
   feature has been successfully negotiated, this message is submitted
@@ -1337,7 +1360,8 @@ Master message types
 ``VHOST_USER_REM_MEM_REG``
   :id: 38
   :equivalent ioctl: N/A
-  :slave payload: single memory region description
+  :request payload: N/A
+  :reply payload: single memory region description
 
   When the ``VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS`` protocol
   feature has been successfully negotiated, this message is submitted
@@ -1352,8 +1376,8 @@ Master message types
 ``VHOST_USER_SET_STATUS``
   :id: 39
   :equivalent ioctl: VHOST_VDPA_SET_STATUS
-  :slave payload: N/A
-  :master payload: ``u64``
+  :request payload: ``u64``
+  :reply payload: N/A
 
   When the ``VHOST_USER_PROTOCOL_F_STATUS`` protocol feature has been
   successfully negotiated, this message is submitted by the master to
@@ -1363,8 +1387,8 @@ Master message types
 ``VHOST_USER_GET_STATUS``
   :id: 40
   :equivalent ioctl: VHOST_VDPA_GET_STATUS
-  :slave payload: ``u64``
-  :master payload: N/A
+  :request payload: N/A
+  :reply payload: ``u64``
 
   When the ``VHOST_USER_PROTOCOL_F_STATUS`` protocol feature has been
   successfully negotiated, this message is submitted by the master to
@@ -1375,11 +1399,14 @@ Master message types
 Slave message types
 -------------------
 
+For this type of message, the request is sent by the slave and the reply
+is sent by the master.
+
 ``VHOST_USER_SLAVE_IOTLB_MSG``
   :id: 1
   :equivalent ioctl: N/A (equivalent to ``VHOST_IOTLB_MSG`` message type)
-  :slave payload: ``struct vhost_iotlb_msg``
-  :master payload: N/A
+  :request payload: ``struct vhost_iotlb_msg``
+  :reply payload: N/A
 
   Send IOTLB messages with ``struct vhost_iotlb_msg`` as payload.
   Slave sends such requests to notify of an IOTLB miss, or an IOTLB
@@ -1393,8 +1420,8 @@ Slave message types
 ``VHOST_USER_SLAVE_CONFIG_CHANGE_MSG``
   :id: 2
   :equivalent ioctl: N/A
-  :slave payload: N/A
-  :master payload: N/A
+  :request payload: N/A
+  :reply payload: N/A
 
   When ``VHOST_USER_PROTOCOL_F_CONFIG`` is negotiated, vhost-user
   slave sends such messages to notify that the virtio device's
@@ -1408,8 +1435,8 @@ Slave message types
 ``VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG``
   :id: 3
   :equivalent ioctl: N/A
-  :slave payload: vring area description
-  :master payload: N/A
+  :request payload: vring area description
+  :reply payload: N/A
 
   Sets host notifier for a specified queue. The queue index is
   contained in the ``u64`` field of the vring area description. The
@@ -1431,8 +1458,8 @@ Slave message types
 ``VHOST_USER_SLAVE_VRING_CALL``
   :id: 4
   :equivalent ioctl: N/A
-  :slave payload: vring state description
-  :master payload: N/A
+  :request payload: vring state description
+  :reply payload: N/A
 
   When the ``VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS`` protocol
   feature has been successfully negotiated, this message may be
@@ -1445,8 +1472,8 @@ Slave message types
 ``VHOST_USER_SLAVE_VRING_ERR``
   :id: 5
   :equivalent ioctl: N/A
-  :slave payload: vring state description
-  :master payload: N/A
+  :request payload: vring state description
+  :reply payload: N/A
 
   When the ``VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS`` protocol
   feature has been successfully negotiated, this message may be
@@ -1472,7 +1499,7 @@ client MUST respond with a Payload ``VhostUserMsg`` indicating success
 or failure. The payload should be set to zero on success or non-zero
 on failure, unless the message already has an explicit reply body.
 
-The response payload gives QEMU a deterministic indication of the result
+The reply payload gives QEMU a deterministic indication of the result
 of the command. Today, QEMU is expected to terminate the main vhost-user
 loop upon receiving such errors. In future, qemu could be taught to be more
 resilient for selective requests.
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 05/13] docs: vhost-user: rewrite section on ring state machine
  2022-03-21 15:30 [PATCH v1 00/13] various virtio docs, fixes and tweaks Alex Bennée
                   ` (3 preceding siblings ...)
  2022-03-21 15:30 ` [PATCH v1 04/13] docs: vhost-user: clean up request/reply description Alex Bennée
@ 2022-03-21 15:30 ` Alex Bennée
  2022-03-21 15:30 ` [PATCH v1 06/13] docs: vhost-user: replace master/slave with front-end/back-end Alex Bennée
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2022-03-21 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, stefanha, Paolo Bonzini,
	marcandre.lureau

From: Paolo Bonzini <pbonzini@redhat.com>

This section is using the word "back-end" to refer to the
"slave's back-end", and talking about the "client" for
what the rest of the document calls the "slave".

Rework it to free the use of the term "back-end", which in
the next patch will replace "slave".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210226143413.188046-3-pbonzini@redhat.com>
---
 docs/interop/vhost-user.rst | 46 +++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index bb588c66fc..694a113e59 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -331,40 +331,36 @@ bit was dedicated for this purpose::
 
   #define VHOST_USER_F_PROTOCOL_FEATURES 30
 
-Starting and stopping rings
----------------------------
+Ring states
+-----------
 
-Client must only process each ring when it is started.
+Rings can be in one of three states:
 
-Client must only pass data between the ring and the backend, when the
-ring is enabled.
+* stopped: the slave must not process the ring at all.
 
-If ring is started but disabled, client must process the ring without
-talking to the backend.
+* started but disabled: the slave must process the ring without
+  causing any side effects.  For example, for a networking device,
+  in the disabled state the slave must not supply any new RX packets,
+  but must process and discard any TX packets.
 
-For example, for a networking device, in the disabled state client
-must not supply any new RX packets, but must process and discard any
-TX packets.
+* started and enabled.
 
-If ``VHOST_USER_F_PROTOCOL_FEATURES`` has not been negotiated, the
-ring is initialized in an enabled state.
+Each ring is initialized in a stopped state.  The slave 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``.
 
-If ``VHOST_USER_F_PROTOCOL_FEATURES`` has been negotiated, the ring is
-initialized in a disabled state. Client must not pass data to/from the
-backend until ring is enabled by ``VHOST_USER_SET_VRING_ENABLE`` with
-parameter 1, or after it has been disabled by
-``VHOST_USER_SET_VRING_ENABLE`` with parameter 0.
+Rings can be enabled or disabled by ``VHOST_USER_SET_VRING_ENABLE``.
 
-Each ring is initialized in a stopped state, client must not process
-it until ring is started, or after it has been stopped.
+If ``VHOST_USER_F_PROTOCOL_FEATURES`` has not been negotiated, the
+ring starts directly in the enabled state.
 
-Client 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``.
+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.
 
-While processing the rings (whether they are enabled or not), client
+While processing the rings (whether they are enabled or not), the slave
 must support changing some configuration aspects on the fly.
 
 Multiple queue support
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 06/13] docs: vhost-user: replace master/slave with front-end/back-end
  2022-03-21 15:30 [PATCH v1 00/13] various virtio docs, fixes and tweaks Alex Bennée
                   ` (4 preceding siblings ...)
  2022-03-21 15:30 ` [PATCH v1 05/13] docs: vhost-user: rewrite section on ring state machine Alex Bennée
@ 2022-03-21 15:30 ` Alex Bennée
  2022-03-21 15:30 ` [PATCH v1 07/13] vhost-user.rst: add clarifying language about protocol negotiation Alex Bennée
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2022-03-21 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, Gerd Hoffmann, stefanha,
	Paolo Bonzini, marcandre.lureau

From: Paolo Bonzini <pbonzini@redhat.com>

This matches the nomenclature that is generally used.  Also commonly used
is client/server, but it is not as clear because sometimes the front-end
exposes a passive (server) socket that the back-end connects to.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210226143413.188046-4-pbonzini@redhat.com>
---
 docs/interop/vhost-user-gpu.rst |  10 +-
 docs/interop/vhost-user.rst     | 342 ++++++++++++++++----------------
 2 files changed, 176 insertions(+), 176 deletions(-)

diff --git a/docs/interop/vhost-user-gpu.rst b/docs/interop/vhost-user-gpu.rst
index 71a2c52b31..1640553729 100644
--- a/docs/interop/vhost-user-gpu.rst
+++ b/docs/interop/vhost-user-gpu.rst
@@ -13,10 +13,10 @@ Introduction
 ============
 
 The vhost-user-gpu protocol is aiming at sharing the rendering result
-of a virtio-gpu, done from a vhost-user slave process to a vhost-user
-master process (such as QEMU). It bears a resemblance to a display
+of a virtio-gpu, done from a vhost-user back-end process to a vhost-user
+front-end process (such as QEMU). It bears a resemblance to a display
 server protocol, if you consider QEMU as the display server and the
-slave as the client, but in a very limited way. Typically, it will
+back-end as the client, but in a very limited way. Typically, it will
 work by setting a scanout/display configuration, before sending flush
 events for the display updates. It will also update the cursor shape
 and position.
@@ -26,8 +26,8 @@ socket ancillary data to share opened file descriptors (DMABUF fds or
 shared memory). The socket is usually obtained via
 ``VHOST_USER_GPU_SET_SOCKET``.
 
-Requests are sent by the *slave*, and the optional replies by the
-*master*.
+Requests are sent by the *back-end*, and the optional replies by the
+*front-end*.
 
 Wire format
 ===========
diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 694a113e59..08c4bf2ef7 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -23,19 +23,19 @@ space process on the same host. It uses communication over a Unix
 domain socket to share file descriptors in the ancillary data of the
 message.
 
-The protocol defines 2 sides of the communication, *master* and
-*slave*. *Master* is the application that shares its virtqueues, in
-our case QEMU. *Slave* is the consumer of the virtqueues.
+The protocol defines 2 sides of the communication, *front-end* and
+*back-end*. The *front-end* is the application that shares its virtqueues, in
+our case QEMU. The *back-end* is the consumer of the virtqueues.
 
-In the current implementation QEMU is the *master*, and the *slave* is
-the external process consuming the virtio queues, for example a
+In the current implementation QEMU is the *front-end*, and the *back-end*
+is the external process consuming the virtio queues, for example a
 software Ethernet switch running in user space, such as Snabbswitch,
-or a block device backend processing read & write to a virtual
-disk. In order to facilitate interoperability between various backend
+or a block device back-end processing read & write to a virtual
+disk. In order to facilitate interoperability between various back-end
 implementations, it is recommended to follow the :ref:`Backend program
 conventions <backend_conventions>`.
 
-*Master* and *slave* can be either a client (i.e. connecting) or
+The *front-end* and *back-end* can be either a client (i.e. connecting) or
 server (listening) in the socket communication.
 
 Support for platforms other than Linux
@@ -77,7 +77,7 @@ Header
 :flags: 32-bit bit field
 
 - Lower 2 bits are the version (currently 0x01)
-- Bit 2 is the reply flag - needs to be sent on each reply from the slave
+- Bit 2 is the reply flag - needs to be sent on each reply from the back-end
 - Bit 3 is the need_reply flag - see :ref:`REPLY_ACK <reply_ack>` for
   details.
 
@@ -222,8 +222,8 @@ Virtio device config space
 :size: a 32-bit configuration space access size in bytes
 
 :flags: a 32-bit value:
-  - 0: Vhost master messages used for writeable fields
-  - 1: Vhost master messages used for live migration
+  - 0: Vhost front-end messages used for writeable fields
+  - 1: Vhost front-end messages used for live migration
 
 :payload: Size bytes array holding the contents of the virtio
           device's configuration space
@@ -290,8 +290,8 @@ vhost for the Linux Kernel. Most messages that can be sent via the
 Unix domain socket implementing vhost-user have an equivalent ioctl to
 the kernel implementation.
 
-The communication consists of *master* sending message requests and
-*slave* sending message replies. Most of the requests don't require
+The communication consists of the *front-end* sending message requests and
+the *back-end* sending message replies. Most of the requests don't require
 replies. Here is a list of the ones that do:
 
 * ``VHOST_USER_GET_FEATURES``
@@ -305,7 +305,7 @@ replies. Here is a list of the ones that do:
    :ref:`REPLY_ACK <reply_ack>`
        The section on ``REPLY_ACK`` protocol extension.
 
-There are several messages that the master sends with file descriptors passed
+There are several messages that the front-end sends with file descriptors passed
 in the ancillary data:
 
 * ``VHOST_USER_SET_MEM_TABLE``
@@ -317,16 +317,16 @@ in the ancillary data:
 * ``VHOST_USER_SET_SLAVE_REQ_FD``
 * ``VHOST_USER_SET_INFLIGHT_FD`` (if ``VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD``)
 
-If *master* is unable to send the full message or receives a wrong
+If *front-end* is unable to send the full message or receives a wrong
 reply it will close the connection. An optional reconnection mechanism
 can be implemented.
 
-If *slave* detects some error such as incompatible features, it may also
+If *back-end* detects some error such as incompatible features, it may also
 close the connection. This should only happen in exceptional circumstances.
 
 Any protocol extensions are gated by protocol feature bits, which
-allows full backwards compatibility on both master and slave.  As
-older slaves don't support negotiating protocol features, a feature
+allows full backwards compatibility on both front-end and back-end.  As
+older back-ends don't support negotiating protocol features, a feature
 bit was dedicated for this purpose::
 
   #define VHOST_USER_F_PROTOCOL_FEATURES 30
@@ -336,16 +336,16 @@ Ring states
 
 Rings can be in one of three states:
 
-* stopped: the slave must not process the ring at all.
+* stopped: the back-end must not process the ring at all.
 
-* started but disabled: the slave must process the ring without
+* started but disabled: the back-end must process the ring without
   causing any side effects.  For example, for a networking device,
-  in the disabled state the slave must not supply any new RX packets,
+  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.
 
-Each ring is initialized in a stopped state.  The slave must start
+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,
@@ -360,53 +360,53 @@ 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.
 
-While processing the rings (whether they are enabled or not), the slave
+While processing the rings (whether they are enabled or not), the back-end
 must support changing some configuration aspects on the fly.
 
 Multiple queue support
 ----------------------
 
-Many devices have a fixed number of virtqueues.  In this case the master
+Many devices have a fixed number of virtqueues.  In this case the front-end
 already knows the number of available virtqueues without communicating with the
-slave.
+back-end.
 
 Some devices do not have a fixed number of virtqueues.  Instead the maximum
-number of virtqueues is chosen by the slave.  The number can depend on host
-resource availability or slave implementation details.  Such devices are called
+number of virtqueues is chosen by the back-end.  The number can depend on host
+resource availability or back-end implementation details.  Such devices are called
 multiple queue devices.
 
-Multiple queue support allows the slave to advertise the maximum number of
-queues.  This is treated as a protocol extension, hence the slave has to
+Multiple queue support allows the back-end to advertise the maximum number of
+queues.  This is treated as a protocol extension, hence the back-end has to
 implement protocol features first. The multiple queues feature is supported
 only when the protocol feature ``VHOST_USER_PROTOCOL_F_MQ`` (bit 0) is set.
 
-The max number of queues the slave supports can be queried with message
-``VHOST_USER_GET_QUEUE_NUM``. Master should stop when the number of requested
+The max number of queues the back-end supports can be queried with message
+``VHOST_USER_GET_QUEUE_NUM``. Front-end should stop when the number of requested
 queues is bigger than that.
 
-As all queues share one connection, the master uses a unique index for each
+As all queues share one connection, the front-end uses a unique index for each
 queue in the sent message to identify a specified queue.
 
-The master enables queues by sending message ``VHOST_USER_SET_VRING_ENABLE``.
+The front-end enables queues by sending message ``VHOST_USER_SET_VRING_ENABLE``.
 vhost-user-net has historically automatically enabled the first queue pair.
 
-Slaves should always implement the ``VHOST_USER_PROTOCOL_F_MQ`` protocol
+Back-ends should always implement the ``VHOST_USER_PROTOCOL_F_MQ`` protocol
 feature, even for devices with a fixed number of virtqueues, since it is simple
 to implement and offers a degree of introspection.
 
-Masters must not rely on the ``VHOST_USER_PROTOCOL_F_MQ`` protocol feature for
+Front-ends must not rely on the ``VHOST_USER_PROTOCOL_F_MQ`` protocol feature for
 devices with a fixed number of virtqueues.  Only true multiqueue devices
 require this protocol feature.
 
 Migration
 ---------
 
-During live migration, the master may need to track the modifications
-the slave makes to the memory mapped regions. The client should mark
+During live migration, the front-end may need to track the modifications
+the back-end makes to the memory mapped regions. The front-end should mark
 the dirty pages in a log. Once it complies to this logging, it may
 declare the ``VHOST_F_LOG_ALL`` vhost feature.
 
-To start/stop logging of data/used ring writes, server may send
+To start/stop logging of data/used ring writes, the front-end may send
 messages ``VHOST_USER_SET_FEATURES`` with ``VHOST_F_LOG_ALL`` and
 ``VHOST_USER_SET_VRING_ADDR`` with ``VHOST_VRING_F_LOG`` in ring's
 flags set to 1/0, respectively.
@@ -420,7 +420,7 @@ Dirty pages are of size::
   #define VHOST_LOG_PAGE 0x1000
 
 The log memory fd is provided in the ancillary data of
-``VHOST_USER_SET_LOG_BASE`` message when the slave has
+``VHOST_USER_SET_LOG_BASE`` message when the back-end has
 ``VHOST_USER_PROTOCOL_F_LOG_SHMFD`` protocol feature.
 
 The size of the log is supplied as part of ``VhostUserMsg`` which
@@ -446,26 +446,26 @@ the bit offset of the last byte of the ring must fall within the size
 supplied by ``VhostUserLog``.
 
 ``VHOST_USER_SET_LOG_FD`` is an optional message with an eventfd in
-ancillary data, it may be used to inform the master that the log has
+ancillary data, it may be used to inform the front-end that the log has
 been modified.
 
 Once the source has finished migration, rings will be stopped by the
 source. No further update must be done before rings are restarted.
 
-In postcopy migration the slave is started before all the memory has
+In postcopy migration the back-end is started before all the memory has
 been received from the source host, and care must be taken to avoid
-accessing pages that have yet to be received.  The slave opens a
+accessing pages that have yet to be received.  The back-end opens a
 'userfault'-fd and registers the memory with it; this fd is then
-passed back over to the master.  The master services requests on the
+passed back over to the front-end.  The front-end services requests on the
 userfaultfd for pages that are accessed and when the page is available
 it performs WAKE ioctl's on the userfaultfd to wake the stalled
-slave.  The client indicates support for this via the
+back-end.  The front-end indicates support for this via the
 ``VHOST_USER_PROTOCOL_F_PAGEFAULT`` feature.
 
 Memory access
 -------------
 
-The master sends a list of vhost memory regions to the slave using the
+The front-end sends a list of vhost memory regions to the back-end using the
 ``VHOST_USER_SET_MEM_TABLE`` message.  Each region has two base
 addresses: a guest address and a user address.
 
@@ -490,60 +490,60 @@ IOMMU support
 -------------
 
 When the ``VIRTIO_F_IOMMU_PLATFORM`` feature has been negotiated, the
-master sends IOTLB entries update & invalidation by sending
-``VHOST_USER_IOTLB_MSG`` requests to the slave with a ``struct
+front-end sends IOTLB entries update & invalidation by sending
+``VHOST_USER_IOTLB_MSG`` requests to the back-end with a ``struct
 vhost_iotlb_msg`` as payload. For update events, the ``iotlb`` payload
 has to be filled with the update message type (2), the I/O virtual
 address, the size, the user virtual address, and the permissions
 flags. Addresses and size must be within vhost memory regions set via
 the ``VHOST_USER_SET_MEM_TABLE`` request. For invalidation events, the
 ``iotlb`` payload has to be filled with the invalidation message type
-(3), the I/O virtual address and the size. On success, the slave is
+(3), the I/O virtual address and the size. On success, the back-end is
 expected to reply with a zero payload, non-zero otherwise.
 
-The slave relies on the slave communication channel (see :ref:`Slave
-communication <slave_communication>` section below) to send IOTLB miss
+The back-end relies on the back-end communication channel (see :ref:`Back-end
+communication <backend_communication>` section below) to send IOTLB miss
 and access failure events, by sending ``VHOST_USER_SLAVE_IOTLB_MSG``
-requests to the master with a ``struct vhost_iotlb_msg`` as
+requests to the front-end with a ``struct vhost_iotlb_msg`` as
 payload. For miss events, the iotlb payload has to be filled with the
 miss message type (1), the I/O virtual address and the permissions
 flags. For access failure event, the iotlb payload has to be filled
 with the access failure message type (4), the I/O virtual address and
-the permissions flags.  For synchronization purpose, the slave may
-rely on the reply-ack feature, so the master may send a reply when
+the permissions flags.  For synchronization purpose, the back-end may
+rely on the reply-ack feature, so the front-end may send a reply when
 operation is completed if the reply-ack feature is negotiated and
-slaves requests a reply. For miss events, completed operation means
-either master sent an update message containing the IOTLB entry
-containing requested address and permission, or master sent nothing if
+back-ends requests a reply. For miss events, completed operation means
+either front-end sent an update message containing the IOTLB entry
+containing requested address and permission, or front-end sent nothing if
 the IOTLB miss message is invalid (invalid IOVA or permission).
 
-The master isn't expected to take the initiative to send IOTLB update
-messages, as the slave sends IOTLB miss messages for the guest virtual
+The front-end isn't expected to take the initiative to send IOTLB update
+messages, as the back-end sends IOTLB miss messages for the guest virtual
 memory areas it needs to access.
 
-.. _slave_communication:
+.. _backend_communication:
 
-Slave communication
--------------------
+Back-end communication
+----------------------
 
-An optional communication channel is provided if the slave declares
+An optional communication channel is provided if the back-end declares
 ``VHOST_USER_PROTOCOL_F_SLAVE_REQ`` protocol feature, to allow the
-slave to make requests to the master.
+back-end to make requests to the front-end.
 
 The fd is provided via ``VHOST_USER_SET_SLAVE_REQ_FD`` ancillary data.
 
-A slave may then send ``VHOST_USER_SLAVE_*`` messages to the master
+A back-end may then send ``VHOST_USER_SLAVE_*`` messages to the front-end
 using this fd communication channel.
 
 If ``VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD`` protocol feature is
-negotiated, slave can send file descriptors (at most 8 descriptors in
-each message) to master via ancillary data using this fd communication
+negotiated, back-end can send file descriptors (at most 8 descriptors in
+each message) to front-end via ancillary data using this fd communication
 channel.
 
 Inflight I/O tracking
 ---------------------
 
-To support reconnecting after restart or crash, slave may need to
+To support reconnecting after restart or crash, back-end may need to
 resubmit inflight I/Os. If virtqueue is processed in order, we can
 easily achieve that by getting the inflight descriptors from
 descriptor table (split virtqueue) or descriptor ring (packed
@@ -551,18 +551,18 @@ virtqueue). However, it can't work when we process descriptors
 out-of-order because some entries which store the information of
 inflight descriptors in available ring (split virtqueue) or descriptor
 ring (packed virtqueue) might be overridden by new entries. To solve
-this problem, slave need to allocate an extra buffer to store this
-information of inflight descriptors and share it with master for
+this problem, the back-end need to allocate an extra buffer to store this
+information of inflight descriptors and share it with front-end for
 persistent. ``VHOST_USER_GET_INFLIGHT_FD`` and
 ``VHOST_USER_SET_INFLIGHT_FD`` are used to transfer this buffer
-between master and slave. And the format of this buffer is described
+between front-end and back-end. And the format of this buffer is described
 below:
 
 +---------------+---------------+-----+---------------+
 | queue0 region | queue1 region | ... | queueN region |
 +---------------+---------------+-----+---------------+
 
-N is the number of available virtqueues. Slave could get it from num
+N is the number of available virtqueues. The back-end could get it from num
 queues field of ``VhostUserInflight``.
 
 For split virtqueue, queue region can be implemented as:
@@ -594,8 +594,8 @@ For split virtqueue, queue region can be implemented as:
        * Zero value indicates an uninitialized buffer */
       uint16_t version;
 
-      /* The size of DescStateSplit array. It's equal to the virtqueue
-       * size. Slave could get it from queue size field of VhostUserInflight. */
+      /* The size of DescStateSplit array. It's equal to the virtqueue size.
+       * The back-end could get it from queue size field of VhostUserInflight. */
       uint16_t desc_num;
 
       /* The head of list that track the last batch of used descriptors. */
@@ -701,8 +701,8 @@ For packed virtqueue, queue region can be implemented as:
        * Zero value indicates an uninitialized buffer */
       uint16_t version;
 
-      /* The size of DescStatePacked array. It's equal to the virtqueue
-       * size. Slave could get it from queue size field of VhostUserInflight. */
+      /* The size of DescStatePacked array. It's equal to the virtqueue size.
+       * The back-end could get it from queue size field of VhostUserInflight. */
       uint16_t desc_num;
 
       /* The head of free DescStatePacked entry list */
@@ -794,7 +794,7 @@ When reconnecting:
    #. Use ``old_used_wrap_counter`` to calculate the available flags
 
    #. If ``d.flags`` is not equal to the calculated flags value (means
-      slave has submitted the buffer to guest driver before crash, so
+      back-end has submitted the buffer to guest driver before crash, so
       it has to commit the in-progres update), set ``old_free_head``,
       ``old_used_idx``, ``old_used_wrap_counter`` to ``free_head``,
       ``used_idx``, ``used_wrap_counter``
@@ -823,11 +823,11 @@ cause the sending application(s) to block, it is not advised to use
 this feature unless absolutely necessary. It is also considered an
 error to negotiate this feature without also negotiating
 ``VHOST_USER_PROTOCOL_F_SLAVE_REQ`` and ``VHOST_USER_PROTOCOL_F_REPLY_ACK``,
-the former is necessary for getting a message channel from the slave
-to the master, while the latter needs to be used with the in-band
+the former is necessary for getting a message channel from the back-end
+to the front-end, while the latter needs to be used with the in-band
 notification messages to block until they are processed, both to avoid
 blocking later and for proper processing (at least in the simulation
-use case.) As it has no other way of signalling this error, the slave
+use case.) As it has no other way of signalling this error, the back-end
 should close the connection as a response to a
 ``VHOST_USER_SET_PROTOCOL_FEATURES`` message that sets the in-band
 notifications feature flag without the other two.
@@ -855,8 +855,8 @@ Protocol features
   #define VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS  15
   #define VHOST_USER_PROTOCOL_F_STATUS               16
 
-Master message types
---------------------
+Front-end message types
+-----------------------
 
 ``VHOST_USER_GET_FEATURES``
   :id: 1
@@ -865,7 +865,7 @@ Master message types
   :reply payload: ``u64``
 
   Get from the underlying vhost implementation the features bitmask.
-  Feature bit ``VHOST_USER_F_PROTOCOL_FEATURES`` signals slave support
+  Feature bit ``VHOST_USER_F_PROTOCOL_FEATURES`` signals back-end support
   for ``VHOST_USER_GET_PROTOCOL_FEATURES`` and
   ``VHOST_USER_SET_PROTOCOL_FEATURES``.
 
@@ -877,7 +877,7 @@ Master message types
 
   Enable features in the underlying vhost implementation using a
   bitmask.  Feature bit ``VHOST_USER_F_PROTOCOL_FEATURES`` signals
-  slave support for ``VHOST_USER_GET_PROTOCOL_FEATURES`` and
+  back-end support for ``VHOST_USER_GET_PROTOCOL_FEATURES`` and
   ``VHOST_USER_SET_PROTOCOL_FEATURES``.
 
 ``VHOST_USER_GET_PROTOCOL_FEATURES``
@@ -892,7 +892,7 @@ Master message types
   ``VHOST_USER_GET_FEATURES``.
 
 .. Note::
-   Slave that reported ``VHOST_USER_F_PROTOCOL_FEATURES`` must
+   Back-ends that report ``VHOST_USER_F_PROTOCOL_FEATURES`` must
    support this message even before ``VHOST_USER_SET_FEATURES`` was
    called.
 
@@ -908,7 +908,7 @@ Master message types
   ``VHOST_USER_GET_FEATURES``.
 
 .. Note::
-   Slave that reported ``VHOST_USER_F_PROTOCOL_FEATURES`` must support
+   Back-ends that report ``VHOST_USER_F_PROTOCOL_FEATURES`` must support
    this message even before ``VHOST_USER_SET_FEATURES`` was called.
 
 ``VHOST_USER_SET_OWNER``
@@ -917,8 +917,8 @@ Master message types
   :request payload: N/A
   :reply payload: N/A
 
-  Issued when a new connection is established. It sets the current
-  *master* as an owner of the session. This can be used on the *slave*
+  Issued when a new connection is established. It marks the sender
+  as the front-end that owns of the session. This can be used on the *back-end*
   as a "session start" flag.
 
 ``VHOST_USER_RESET_OWNER``
@@ -929,9 +929,9 @@ Master message types
 .. admonition:: Deprecated
 
    This is no longer used. Used to be sent to request disabling all
-   rings, but some clients interpreted it to also discard connection
+   rings, but some back-ends interpreted it to also discard connection
    state (this interpretation would lead to bugs).  It is recommended
-   that clients either ignore this message, or use it to disable all
+   that back-ends either ignore this message, or use it to disable all
    rings.
 
 ``VHOST_USER_SET_MEM_TABLE``
@@ -940,14 +940,14 @@ Master message types
   :request payload: memory regions description
   :reply payload: (postcopy only) memory regions description
 
-  Sets the memory map regions on the slave so it can translate the
+  Sets the memory map regions on the back-end so it can translate the
   vring addresses. In the ancillary data there is an array of file
   descriptors for each memory mapped region. The size and ordering of
   the fds matches the number and ordering of memory regions.
 
   When ``VHOST_USER_POSTCOPY_LISTEN`` has been received,
   ``SET_MEM_TABLE`` replies with the bases of the memory mapped
-  regions to the master.  The slave must have mmap'd the regions but
+  regions to the front-end.  The back-end must have mmap'd the regions but
   not yet accessed them and should not yet generate a userfault
   event.
 
@@ -966,7 +966,7 @@ Master message types
 
   Sets logging shared memory space.
 
-  When slave has ``VHOST_USER_PROTOCOL_F_LOG_SHMFD`` protocol feature,
+  When the back-end has ``VHOST_USER_PROTOCOL_F_LOG_SHMFD`` protocol feature,
   the log memory fd is provided in the ancillary data of
   ``VHOST_USER_SET_LOG_BASE`` message, the size and offset of shared
   memory area provided in the message.
@@ -1073,7 +1073,7 @@ Master message types
   :request payload: N/A
   :reply payload: u64
 
-  Query how many queues the backend supports.
+  Query how many queues the back-end supports.
 
   This request should be sent only when ``VHOST_USER_PROTOCOL_F_MQ``
   is set in queried protocol features by
@@ -1085,7 +1085,7 @@ Master message types
   :request payload: vring state description
   :reply payload: N/A
 
-  Signal slave to enable or disable corresponding vring.
+  Signal the back-end to enable or disable corresponding vring.
 
   This request should be sent only when
   ``VHOST_USER_F_PROTOCOL_FEATURES`` has been negotiated.
@@ -1096,7 +1096,7 @@ Master message types
   :request payload: ``u64``
   :reply payload: N/A
 
-  Ask vhost user backend to broadcast a fake RARP to notify the migration
+  Ask vhost user back-end to broadcast a fake RARP to notify the migration
   is terminated for guest that does not support GUEST_ANNOUNCE.
 
   Only legal if feature bit ``VHOST_USER_F_PROTOCOL_FEATURES`` is
@@ -1104,7 +1104,7 @@ Master message types
   ``VHOST_USER_PROTOCOL_F_RARP`` is present in
   ``VHOST_USER_GET_PROTOCOL_FEATURES``.  The first 6 bytes of the
   payload contain the mac address of the guest to allow the vhost user
-  backend to construct and broadcast the fake RARP.
+  back-end to construct and broadcast the fake RARP.
 
 ``VHOST_USER_NET_SET_MTU``
   :id: 20
@@ -1120,7 +1120,7 @@ Master message types
   ``VHOST_USER_PROTOCOL_F_NET_MTU`` is present in
   ``VHOST_USER_GET_PROTOCOL_FEATURES``.
 
-  If ``VHOST_USER_PROTOCOL_F_REPLY_ACK`` is negotiated, slave must
+  If ``VHOST_USER_PROTOCOL_F_REPLY_ACK`` is negotiated, the back-end must
   respond with zero in case the specified MTU is valid, or non-zero
   otherwise.
 
@@ -1130,14 +1130,14 @@ Master message types
   :request payload: N/A
   :reply payload: N/A
 
-  Set the socket file descriptor for slave initiated requests. It is passed
+  Set the socket file descriptor for back-end initiated requests. It is passed
   in the ancillary data.
 
   This request should be sent only when
   ``VHOST_USER_F_PROTOCOL_FEATURES`` has been negotiated, and protocol
   feature bit ``VHOST_USER_PROTOCOL_F_SLAVE_REQ`` bit is present in
   ``VHOST_USER_GET_PROTOCOL_FEATURES``.  If
-  ``VHOST_USER_PROTOCOL_F_REPLY_ACK`` is negotiated, slave must
+  ``VHOST_USER_PROTOCOL_F_REPLY_ACK`` is negotiated, the back-end must
   respond with zero for success, non-zero otherwise.
 
 ``VHOST_USER_IOTLB_MSG``
@@ -1148,8 +1148,8 @@ Master message types
 
   Send IOTLB messages with ``struct vhost_iotlb_msg`` as payload.
 
-  Master sends such requests to update and invalidate entries in the
-  device IOTLB. The slave has to acknowledge the request with sending
+  The front-end sends such requests to update and invalidate entries in the
+  device IOTLB. The back-end has to acknowledge the request with sending
   zero as ``u64`` payload for success, non-zero otherwise.
 
   This request should be send only when ``VIRTIO_F_IOMMU_PLATFORM``
@@ -1169,7 +1169,7 @@ Master message types
   ``VHOST_USER_PROTOCOL_F_CROSS_ENDIAN`` has been negotiated.
   Backends that negotiated this feature should handle both
   endiannesses and expect this message once (per VQ) during device
-  configuration (ie. before the master starts the VQ).
+  configuration (ie. before the front-end starts the VQ).
 
 ``VHOST_USER_GET_CONFIG``
   :id: 24
@@ -1178,11 +1178,11 @@ Master message types
   :reply payload: virtio device config space
 
   When ``VHOST_USER_PROTOCOL_F_CONFIG`` is negotiated, this message is
-  submitted by the vhost-user master to fetch the contents of the
-  virtio device configuration space, vhost-user slave's payload size
-  MUST match master's request, vhost-user slave uses zero length of
-  payload to indicate an error to vhost-user master. The vhost-user
-  master may cache the contents to avoid repeated
+  submitted by the vhost-user front-end to fetch the contents of the
+  virtio device configuration space, vhost-user back-end's payload size
+  MUST match the front-end's request, vhost-user back-end uses zero length of
+  payload to indicate an error to the vhost-user front-end. The vhost-user
+  front-end may cache the contents to avoid repeated
   ``VHOST_USER_GET_CONFIG`` calls.
 
 ``VHOST_USER_SET_CONFIG``
@@ -1192,10 +1192,10 @@ Master message types
   :reply payload: N/A
 
   When ``VHOST_USER_PROTOCOL_F_CONFIG`` is negotiated, this message is
-  submitted by the vhost-user master when the Guest changes the virtio
+  submitted by the vhost-user front-end when the Guest changes the virtio
   device configuration space and also can be used for live migration
-  on the destination host. The vhost-user slave must check the flags
-  field, and slaves MUST NOT accept SET_CONFIG for read-only
+  on the destination host. The vhost-user back-end must check the flags
+  field, and back-ends MUST NOT accept SET_CONFIG for read-only
   configuration space fields unless the live migration bit is set.
 
 ``VHOST_USER_CREATE_CRYPTO_SESSION``
@@ -1204,7 +1204,7 @@ Master message types
   :request payload: crypto session description
   :reply payload: crypto session description
 
-  Create a session for crypto operation. The server side must return
+  Create a session for crypto operation. The back-end must return
   the session id, 0 or positive for success, negative for failure.
   This request should be sent only when
   ``VHOST_USER_PROTOCOL_F_CRYPTO_SESSION`` feature has been
@@ -1230,9 +1230,9 @@ Master message types
   :request payload: N/A
   :reply payload: userfault fd
 
-  When ``VHOST_USER_PROTOCOL_F_PAGEFAULT`` is supported, the master
-  advises slave that a migration with postcopy enabled is underway,
-  the slave must open a userfaultfd for later use.  Note that at this
+  When ``VHOST_USER_PROTOCOL_F_PAGEFAULT`` is supported, the front-end
+  advises back-end that a migration with postcopy enabled is underway,
+  the back-end must open a userfaultfd for later use.  Note that at this
   stage the migration is still in precopy mode.
 
 ``VHOST_USER_POSTCOPY_LISTEN``
@@ -1240,8 +1240,8 @@ Master message types
   :request payload: N/A
   :reply payload: N/A
 
-  Master advises slave that a transition to postcopy mode has
-  happened.  The slave must ensure that shared memory is registered
+  The front-end advises back-end that a transition to postcopy mode has
+  happened.  The back-end must ensure that shared memory is registered
   with userfaultfd to cause faulting of non-present pages.
 
   This is always sent sometime after a ``VHOST_USER_POSTCOPY_ADVISE``,
@@ -1252,7 +1252,7 @@ Master message types
   :request payload: N/A
   :reply payload: ``u64``
 
-  Master advises that postcopy migration has now completed.  The slave
+  The front-end advises that postcopy migration has now completed.  The back-end
   must disable the userfaultfd. The reply is an acknowledgement
   only.
 
@@ -1269,9 +1269,9 @@ Master message types
   :reply payload: N/A
 
   When ``VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD`` protocol feature has
-  been successfully negotiated, this message is submitted by master to
-  get a shared buffer from slave. The shared buffer will be used to
-  track inflight I/O by slave. QEMU should retrieve a new one when vm
+  been successfully negotiated, this message is submitted by the front-end to
+  get a shared buffer from back-end. The shared buffer will be used to
+  track inflight I/O by back-end. QEMU should retrieve a new one when vm
   reset.
 
 ``VHOST_USER_SET_INFLIGHT_FD``
@@ -1281,9 +1281,9 @@ Master message types
   :reply payload: N/A
 
   When ``VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD`` protocol feature has
-  been successfully negotiated, this message is submitted by master to
-  send the shared inflight buffer back to slave so that slave could
-  get inflight I/O after a crash or restart.
+  been successfully negotiated, this message is submitted by the front-end to
+  send the shared inflight buffer back to the back-end so that the back-end
+  could get inflight I/O after a crash or restart.
 
 ``VHOST_USER_GPU_SET_SOCKET``
   :id: 33
@@ -1292,7 +1292,7 @@ Master message types
   :reply payload: N/A
 
   Sets the GPU protocol socket file descriptor, which is passed as
-  ancillary data. The GPU protocol is used to inform the master of
+  ancillary data. The GPU protocol is used to inform the front-end of
   rendering state and updates. See vhost-user-gpu.rst for details.
 
 ``VHOST_USER_RESET_DEVICE``
@@ -1301,13 +1301,13 @@ Master message types
   :request payload: N/A
   :reply payload: N/A
 
-  Ask the vhost user backend to disable all rings and reset all
+  Ask the vhost user back-end to disable all rings and reset all
   internal device state to the initial state, ready to be
-  reinitialized. The backend retains ownership of the device
+  reinitialized. The back-end retains ownership of the device
   throughout the reset operation.
 
   Only valid if the ``VHOST_USER_PROTOCOL_F_RESET_DEVICE`` protocol
-  feature is set by the backend.
+  feature is set by the back-end.
 
 ``VHOST_USER_VRING_KICK``
   :id: 35
@@ -1317,9 +1317,9 @@ Master message types
 
   When the ``VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS`` protocol
   feature has been successfully negotiated, this message may be
-  submitted by the master to indicate that a buffer was added to
+  submitted by the front-end to indicate that a buffer was added to
   the vring instead of signalling it using the vring's kick file
-  descriptor or having the slave rely on polling.
+  descriptor or having the back-end rely on polling.
 
   The state.num field is currently reserved and must be set to 0.
 
@@ -1331,9 +1331,9 @@ Master message types
 
   When the ``VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS`` protocol
   feature has been successfully negotiated, this message is submitted
-  by master to the slave. The slave should return the message with a
+  by the front-end to the back-end. The back-end should return the message with a
   u64 payload containing the maximum number of memory slots for
-  QEMU to expose to the guest. The value returned by the backend
+  QEMU to expose to the guest. The value returned by the back-end
   will be capped at the maximum number of ram slots which can be
   supported by the target platform.
 
@@ -1345,13 +1345,13 @@ Master message types
 
   When the ``VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS`` protocol
   feature has been successfully negotiated, this message is submitted
-  by the master to the slave. The message payload contains a memory
+  by the front-end to the back-end. The message payload contains a memory
   region descriptor struct, describing a region of guest memory which
-  the slave device must map in. When the
+  the back-end device must map in. When the
   ``VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS`` protocol feature has
   been successfully negotiated, along with the
   ``VHOST_USER_REM_MEM_REG`` message, this message is used to set and
-  update the memory tables of the slave device.
+  update the memory tables of the back-end device.
 
 ``VHOST_USER_REM_MEM_REG``
   :id: 38
@@ -1361,13 +1361,13 @@ Master message types
 
   When the ``VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS`` protocol
   feature has been successfully negotiated, this message is submitted
-  by the master to the slave. The message payload contains a memory
+  by the front-end to the back-end. The message payload contains a memory
   region descriptor struct, describing a region of guest memory which
-  the slave device must unmap. When the
+  the back-end device must unmap. When the
   ``VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS`` protocol feature has
   been successfully negotiated, along with the
   ``VHOST_USER_ADD_MEM_REG`` message, this message is used to set and
-  update the memory tables of the slave device.
+  update the memory tables of the back-end device.
 
 ``VHOST_USER_SET_STATUS``
   :id: 39
@@ -1376,8 +1376,8 @@ Master message types
   :reply payload: N/A
 
   When the ``VHOST_USER_PROTOCOL_F_STATUS`` protocol feature has been
-  successfully negotiated, this message is submitted by the master to
-  notify the backend with updated device status as defined in the Virtio
+  successfully negotiated, this message is submitted by the front-end to
+  notify the back-end with updated device status as defined in the Virtio
   specification.
 
 ``VHOST_USER_GET_STATUS``
@@ -1387,16 +1387,16 @@ Master message types
   :reply payload: ``u64``
 
   When the ``VHOST_USER_PROTOCOL_F_STATUS`` protocol feature has been
-  successfully negotiated, this message is submitted by the master to
-  query the backend for its device status as defined in the Virtio
+  successfully negotiated, this message is submitted by the front-end to
+  query the back-end for its device status as defined in the Virtio
   specification.
 
 
-Slave message types
--------------------
+Back-end message types
+----------------------
 
-For this type of message, the request is sent by the slave and the reply
-is sent by the master.
+For this type of message, the request is sent by the back-end and the reply
+is sent by the front-end.
 
 ``VHOST_USER_SLAVE_IOTLB_MSG``
   :id: 1
@@ -1405,9 +1405,9 @@ is sent by the master.
   :reply payload: N/A
 
   Send IOTLB messages with ``struct vhost_iotlb_msg`` as payload.
-  Slave sends such requests to notify of an IOTLB miss, or an IOTLB
+  The back-end sends such requests to notify of an IOTLB miss, or an IOTLB
   access failure. If ``VHOST_USER_PROTOCOL_F_REPLY_ACK`` is
-  negotiated, and slave set the ``VHOST_USER_NEED_REPLY`` flag, master
+  negotiated, and back-end set the ``VHOST_USER_NEED_REPLY`` flag, the front-end
   must respond with zero when operation is successfully completed, or
   non-zero otherwise.  This request should be send only when
   ``VIRTIO_F_IOMMU_PLATFORM`` feature has been successfully
@@ -1420,12 +1420,12 @@ is sent by the master.
   :reply payload: N/A
 
   When ``VHOST_USER_PROTOCOL_F_CONFIG`` is negotiated, vhost-user
-  slave sends such messages to notify that the virtio device's
+  back-end sends such messages to notify that the virtio device's
   configuration space has changed, for those host devices which can
   support such feature, host driver can send ``VHOST_USER_GET_CONFIG``
-  message to slave to get the latest content. If
-  ``VHOST_USER_PROTOCOL_F_REPLY_ACK`` is negotiated, and slave set the
-  ``VHOST_USER_NEED_REPLY`` flag, master must respond with zero when
+  message to the back-end to get the latest content. If
+  ``VHOST_USER_PROTOCOL_F_REPLY_ACK`` is negotiated, and the back-end sets the
+  ``VHOST_USER_NEED_REPLY`` flag, the front-end must respond with zero when
   operation is successfully completed, or non-zero otherwise.
 
 ``VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG``
@@ -1443,7 +1443,7 @@ is sent by the master.
   description. QEMU can mmap the file descriptor based on the size and
   offset to get a memory range. Registering a host notifier means
   mapping this memory range to the VM as the specified queue's notify
-  MMIO region. Slave sends this request to tell QEMU to de-register
+  MMIO region. The back-end sends this request to tell QEMU to de-register
   the existing notifier if any and register the new notifier if the
   request is sent with a file descriptor.
 
@@ -1459,9 +1459,9 @@ is sent by the master.
 
   When the ``VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS`` protocol
   feature has been successfully negotiated, this message may be
-  submitted by the slave to indicate that a buffer was used from
+  submitted by the back-end to indicate that a buffer was used from
   the vring instead of signalling this using the vring's call file
-  descriptor or having the master relying on polling.
+  descriptor or having the front-end relying on polling.
 
   The state.num field is currently reserved and must be set to 0.
 
@@ -1473,9 +1473,9 @@ is sent by the master.
 
   When the ``VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS`` protocol
   feature has been successfully negotiated, this message may be
-  submitted by the slave to indicate that an error occurred on the
+  submitted by the back-end to indicate that an error occurred on the
   specific vring, instead of signalling the error file descriptor
-  set by the master via ``VHOST_USER_SET_VRING_ERR``.
+  set by the front-end via ``VHOST_USER_SET_VRING_ERR``.
 
   The state.num field is currently reserved and must be set to 0.
 
@@ -1486,12 +1486,12 @@ VHOST_USER_PROTOCOL_F_REPLY_ACK
 
 The original vhost-user specification only demands replies for certain
 commands. This differs from the vhost protocol implementation where
-commands are sent over an ``ioctl()`` call and block until the client
+commands are sent over an ``ioctl()`` call and block until the back-end
 has completed.
 
 With this protocol extension negotiated, the sender (QEMU) can set the
 ``need_reply`` [Bit 3] flag to any command. This indicates that the
-client MUST respond with a Payload ``VhostUserMsg`` indicating success
+back-end MUST respond with a Payload ``VhostUserMsg`` indicating success
 or failure. The payload should be set to zero on success or non-zero
 on failure, unless the message already has an explicit reply body.
 
@@ -1500,7 +1500,7 @@ of the command. Today, QEMU is expected to terminate the main vhost-user
 loop upon receiving such errors. In future, qemu could be taught to be more
 resilient for selective requests.
 
-For the message types that already solicit a reply from the client,
+For the message types that already solicit a reply from the back-end,
 the presence of ``VHOST_USER_PROTOCOL_F_REPLY_ACK`` or need_reply bit
 being set brings no behavioural change. (See the Communication_
 section for details.)
@@ -1510,26 +1510,26 @@ section for details.)
 Backend program conventions
 ===========================
 
-vhost-user backends can provide various devices & services and may
+vhost-user back-ends can provide various devices & services and may
 need to be configured manually depending on the use case. However, it
 is a good idea to follow the conventions listed here when
 possible. Users, QEMU or libvirt, can then rely on some common
 behaviour to avoid heterogeneous configuration and management of the
-backend programs and facilitate interoperability.
+back-end programs and facilitate interoperability.
 
-Each backend installed on a host system should come with at least one
+Each back-end installed on a host system should come with at least one
 JSON file that conforms to the vhost-user.json schema. Each file
-informs the management applications about the backend type, and binary
+informs the management applications about the back-end type, and binary
 location. In addition, it defines rules for management apps for
-picking the highest priority backend when multiple match the search
+picking the highest priority back-end when multiple match the search
 criteria (see ``@VhostUserBackend`` documentation in the schema file).
 
-If the backend is not capable of enabling a requested feature on the
+If the back-end is not capable of enabling a requested feature on the
 host (such as 3D acceleration with virgl), or the initialization
-failed, the backend should fail to start early and exit with a status
+failed, the back-end should fail to start early and exit with a status
 != 0. It may also print a message to stderr for further details.
 
-The backend program must not daemonize itself, but it may be
+The back-end program must not daemonize itself, but it may be
 daemonized by the management layer. It may also have a restricted
 access to the system.
 
@@ -1537,7 +1537,7 @@ File descriptors 0, 1 and 2 will exist, and have regular
 stdin/stdout/stderr usage (they may have been redirected to /dev/null
 by the management layer, or to a log handler).
 
-The backend program must end (as quickly and cleanly as possible) when
+The back-end program must end (as quickly and cleanly as possible) when
 the SIGTERM signal is received. Eventually, it may receive SIGKILL by
 the management layer after a few seconds.
 
@@ -1551,15 +1551,15 @@ are mandatory, unless explicitly said differently:
 
 --fd=FDNUM
 
-  When this argument is given, the backend program is started with the
+  When this argument is given, the back-end program is started with the
   vhost-user socket as file descriptor FDNUM. It is incompatible with
   --socket-path.
 
 --print-capabilities
 
-  Output to stdout the backend capabilities in JSON format, and then
+  Output to stdout the back-end capabilities in JSON format, and then
   exit successfully. Other options and arguments should be ignored, and
-  the backend program should not perform its normal function.  The
+  the back-end program should not perform its normal function.  The
   capabilities can be reported dynamically depending on the host
   capabilities.
 
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 07/13] vhost-user.rst: add clarifying language about protocol negotiation
  2022-03-21 15:30 [PATCH v1 00/13] various virtio docs, fixes and tweaks Alex Bennée
                   ` (5 preceding siblings ...)
  2022-03-21 15:30 ` [PATCH v1 06/13] docs: vhost-user: replace master/slave with front-end/back-end Alex Bennée
@ 2022-03-21 15:30 ` Alex Bennée
  2022-03-21 15:30 ` [PATCH v1 08/13] libvhost-user: expose vu_request_to_string Alex Bennée
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2022-03-21 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, stefanha,
	marcandre.lureau, Jiang Liu, Alex Bennée

Make the language about feature negotiation explicitly clear about the
handling of the VHOST_USER_F_PROTOCOL_FEATURES feature bit. Try and
avoid the sort of bug introduced in vhost.rs REPLY_ACK processing:

  https://github.com/rust-vmm/vhost/pull/24

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Jiang Liu <gerry@linux.alibaba.com>
Message-Id: <20210226111619.21178-1-alex.bennee@linaro.org>

---
v2
  - use Stefan's suggested wording
  - Be super explicit in the message descriptions
---
 docs/interop/vhost-user.rst | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 08c4bf2ef7..948d69c9ad 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -331,6 +331,18 @@ bit was dedicated for this purpose::
 
   #define VHOST_USER_F_PROTOCOL_FEATURES 30
 
+Note that VHOST_USER_F_PROTOCOL_FEATURES is the UNUSED (30) feature
+bit defined in `VIRTIO 1.1 6.3 Legacy Interface: Reserved Feature Bits
+<https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.html#x1-4130003>`_.
+VIRTIO devices do not advertise this feature bit and therefore VIRTIO
+drivers cannot negotiate it.
+
+This reserved feature bit was reused by the vhost-user protocol to add
+vhost-user protocol feature negotiation in a backwards compatible
+fashion. Old vhost-user master and slave implementations continue to
+work even though they are not aware of vhost-user protocol feature
+negotiation.
+
 Ring states
 -----------
 
@@ -889,7 +901,8 @@ Front-end message types
   Get the protocol feature bitmask from the underlying vhost
   implementation.  Only legal if feature bit
   ``VHOST_USER_F_PROTOCOL_FEATURES`` is present in
-  ``VHOST_USER_GET_FEATURES``.
+  ``VHOST_USER_GET_FEATURES``.  It does not need to be acknowledged by
+  ``VHOST_USER_SET_FEATURES``.
 
 .. Note::
    Back-ends that report ``VHOST_USER_F_PROTOCOL_FEATURES`` must
@@ -905,7 +918,8 @@ Front-end message types
   Enable protocol features in the underlying vhost implementation.
 
   Only legal if feature bit ``VHOST_USER_F_PROTOCOL_FEATURES`` is present in
-  ``VHOST_USER_GET_FEATURES``.
+  ``VHOST_USER_GET_FEATURES``.  It does not need to be acknowledged by
+  ``VHOST_USER_SET_FEATURES``.
 
 .. Note::
    Back-ends that report ``VHOST_USER_F_PROTOCOL_FEATURES`` must support
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH  v1 08/13] libvhost-user: expose vu_request_to_string
  2022-03-21 15:30 [PATCH v1 00/13] various virtio docs, fixes and tweaks Alex Bennée
                   ` (6 preceding siblings ...)
  2022-03-21 15:30 ` [PATCH v1 07/13] vhost-user.rst: add clarifying language about protocol negotiation Alex Bennée
@ 2022-03-21 15:30 ` Alex Bennée
  2022-03-21 22:31   ` Philippe Mathieu-Daudé
  2022-03-21 15:30 ` [PATCH v1 09/13] docs/devel: start documenting writing VirtIO devices Alex Bennée
                   ` (6 subsequent siblings)
  14 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2022-03-21 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, stefanha,
	marcandre.lureau, Alex Bennée

This is useful for more human readable debug messages in vhost-user
programs.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 subprojects/libvhost-user/libvhost-user.h | 9 +++++++++
 subprojects/libvhost-user/libvhost-user.c | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h
index cde9f07bb3..aea7ec5061 100644
--- a/subprojects/libvhost-user/libvhost-user.h
+++ b/subprojects/libvhost-user/libvhost-user.h
@@ -473,6 +473,15 @@ bool vu_init(VuDev *dev,
  */
 void vu_deinit(VuDev *dev);
 
+
+/**
+ * vu_request_to_string: return string for vhost message request
+ * @req: VhostUserMsg request
+ *
+ * Returns a const string, do not free.
+ */
+const char *vu_request_to_string(unsigned int req);
+
 /**
  * vu_dispatch:
  * @dev: a VuDev context
diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index 47d2efc60f..c218f911e7 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -99,7 +99,7 @@ static inline bool vu_has_protocol_feature(VuDev *dev, unsigned int fbit)
     return has_feature(dev->protocol_features, fbit);
 }
 
-static const char *
+const char *
 vu_request_to_string(unsigned int req)
 {
 #define REQ(req) [req] = #req
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 09/13] docs/devel: start documenting writing VirtIO devices
  2022-03-21 15:30 [PATCH v1 00/13] various virtio docs, fixes and tweaks Alex Bennée
                   ` (7 preceding siblings ...)
  2022-03-21 15:30 ` [PATCH v1 08/13] libvhost-user: expose vu_request_to_string Alex Bennée
@ 2022-03-21 15:30 ` Alex Bennée
  2022-03-21 15:30 ` [PATCH v1 10/13] include/hw: start documenting the vhost API Alex Bennée
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2022-03-21 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, Dr . David Alan Gilbert,
	Gerd Hoffmann, stefanha, marcandre.lureau, Alex Bennée

While writing my own VirtIO devices I've gotten confused with how
things are structured and what sort of shared infrastructure there is.
If we can document how everything is supposed to work we can then
maybe start cleaning up inconsistencies in the code.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20220309164929.19395-1-alex.bennee@linaro.org>

---
v2
  - move more description to the leader text
  - try not to confuse backend and frontend terms
  - more explicit description of objects
  - try and tease apart vhost_dev_init vs QOM-ifed vhost-user-backend
---
 docs/devel/index-internals.rst |   1 +
 docs/devel/virtio-backends.rst | 214 +++++++++++++++++++++++++++++++++
 2 files changed, 215 insertions(+)
 create mode 100644 docs/devel/virtio-backends.rst

diff --git a/docs/devel/index-internals.rst b/docs/devel/index-internals.rst
index bb118b8eaf..5d9f95dd93 100644
--- a/docs/devel/index-internals.rst
+++ b/docs/devel/index-internals.rst
@@ -19,3 +19,4 @@ Details about QEMU's various subsystems including how to add features to them.
    tracing
    vfio-migration
    writing-monitor-commands
+   virtio-backends
diff --git a/docs/devel/virtio-backends.rst b/docs/devel/virtio-backends.rst
new file mode 100644
index 0000000000..9ff092e7a0
--- /dev/null
+++ b/docs/devel/virtio-backends.rst
@@ -0,0 +1,214 @@
+..
+   Copyright (c) 2022, Linaro Limited
+   Written by Alex Bennée
+
+Writing VirtIO backends for QEMU
+================================
+
+This document attempts to outline the information a developer needs to
+know to write device emulations in QEMU. It is specifically focused on
+implementing VirtIO devices. For VirtIO the frontend is the driver
+running on the guest. The backend is the everything that QEMU needs to
+do to handle the emulation of the VirtIO device. This can be done
+entirely in QEMU, divided between QEMU and the kernel (vhost) or
+handled by a separate process which is configured by QEMU
+(vhost-user).
+
+VirtIO Transports
+-----------------
+
+VirtIO supports a number of different transports. While the details of
+the configuration and operation of the device will generally be the
+same QEMU represents them as different devices depending on the
+transport they use. For example -device virtio-foo represents the foo
+device using mmio and -device virtio-foo-pci is the same class of
+device using the PCI transport.
+
+Using the QEMU Object Model (QOM)
+---------------------------------
+
+Generally all devices in QEMU are super classes of ``TYPE_DEVICE``
+however VirtIO devices should be based on ``TYPE_VIRTIO_DEVICE`` which
+itself is derived from the base class. For example:
+
+.. code:: c
+
+  static const TypeInfo virtio_blk_info = {
+      .name = TYPE_VIRTIO_BLK,
+      .parent = TYPE_VIRTIO_DEVICE,
+      .instance_size = sizeof(VirtIOBlock),
+      .instance_init = virtio_blk_instance_init,
+      .class_init = virtio_blk_class_init,
+  };
+
+The author may decide to have a more expansive class hierarchy to
+support multiple device types. For example the Virtio GPU device:
+
+.. code:: c
+
+  static const TypeInfo virtio_gpu_base_info = {
+      .name = TYPE_VIRTIO_GPU_BASE,
+      .parent = TYPE_VIRTIO_DEVICE,
+      .instance_size = sizeof(VirtIOGPUBase),
+      .class_size = sizeof(VirtIOGPUBaseClass),
+      .class_init = virtio_gpu_base_class_init,
+      .abstract = true
+  };
+
+  static const TypeInfo vhost_user_gpu_info = {
+      .name = TYPE_VHOST_USER_GPU,
+      .parent = TYPE_VIRTIO_GPU_BASE,
+      .instance_size = sizeof(VhostUserGPU),
+      .instance_init = vhost_user_gpu_instance_init,
+      .instance_finalize = vhost_user_gpu_instance_finalize,
+      .class_init = vhost_user_gpu_class_init,
+  };
+
+  static const TypeInfo virtio_gpu_info = {
+      .name = TYPE_VIRTIO_GPU,
+      .parent = TYPE_VIRTIO_GPU_BASE,
+      .instance_size = sizeof(VirtIOGPU),
+      .class_size = sizeof(VirtIOGPUClass),
+      .class_init = virtio_gpu_class_init,
+  };
+
+defines a base class for the VirtIO GPU and then specialises two
+versions, one for the internal implementation and the other for the
+vhost-user version.
+
+VirtIOPCIProxy
+^^^^^^^^^^^^^^
+
+[AJB: the following is supposition and welcomes more informed
+opinions]
+
+Probably due to legacy from the pre-QOM days PCI VirtIO devices don't
+follow the normal hierarchy. Instead the a standalone object is based
+on the VirtIOPCIProxy class and the specific VirtIO instance is
+manually instantiated:
+
+.. code:: c
+
+  /*
+   * virtio-blk-pci: This extends VirtioPCIProxy.
+   */
+  #define TYPE_VIRTIO_BLK_PCI "virtio-blk-pci-base"
+  DECLARE_INSTANCE_CHECKER(VirtIOBlkPCI, VIRTIO_BLK_PCI,
+                           TYPE_VIRTIO_BLK_PCI)
+
+  struct VirtIOBlkPCI {
+      VirtIOPCIProxy parent_obj;
+      VirtIOBlock vdev;
+  };
+
+  static Property virtio_blk_pci_properties[] = {
+      DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
+      DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
+                      VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
+      DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
+                         DEV_NVECTORS_UNSPECIFIED),
+      DEFINE_PROP_END_OF_LIST(),
+  };
+
+  static void virtio_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
+  {
+      VirtIOBlkPCI *dev = VIRTIO_BLK_PCI(vpci_dev);
+      DeviceState *vdev = DEVICE(&dev->vdev);
+
+      ...
+
+      qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
+  }
+
+  static void virtio_blk_pci_class_init(ObjectClass *klass, void *data)
+  {
+      DeviceClass *dc = DEVICE_CLASS(klass);
+      VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
+      PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
+
+      set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
+      device_class_set_props(dc, virtio_blk_pci_properties);
+      k->realize = virtio_blk_pci_realize;
+      pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
+      pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_BLOCK;
+      pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
+      pcidev_k->class_id = PCI_CLASS_STORAGE_SCSI;
+  }
+
+  static void virtio_blk_pci_instance_init(Object *obj)
+  {
+      VirtIOBlkPCI *dev = VIRTIO_BLK_PCI(obj);
+
+      virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                  TYPE_VIRTIO_BLK);
+      object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
+                                "bootindex");
+  }
+
+  static const VirtioPCIDeviceTypeInfo virtio_blk_pci_info = {
+      .base_name              = TYPE_VIRTIO_BLK_PCI,
+      .generic_name           = "virtio-blk-pci",
+      .transitional_name      = "virtio-blk-pci-transitional",
+      .non_transitional_name  = "virtio-blk-pci-non-transitional",
+      .instance_size = sizeof(VirtIOBlkPCI),
+      .instance_init = virtio_blk_pci_instance_init,
+      .class_init    = virtio_blk_pci_class_init,
+  };
+
+Here you can see the instance_init has to manually instantiate the
+underlying ``TYPE_VIRTIO_BLOCK`` object and link an alias for one of
+it's properties to the PCI device.
+
+  
+Back End Implementations
+------------------------
+
+There are a number of places where the implementation of the backend
+can be done:
+
+* in QEMU itself
+* in the host kernel (a.k.a vhost)
+* in a separate process (a.k.a. vhost-user)
+
+vhost_ops vs TYPE_VHOST_USER_BACKEND
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+There are two choices to how to implement vhost code. Most of the code
+which has to work with either vhost or vhost-user uses
+``vhost_dev_init()`` to instantiate the appropriate backend. This
+means including a ``struct vhost_dev`` in the main object structure.
+
+For vhost-user devices you also need to add code to track the
+initialisation of the ``chardev`` device used for the control socket
+between QEMU and the external vhost-user process.
+
+If you only need to implement a vhost-user backed the other option is
+a use a QOM-ified version of vhost-user.
+
+.. code:: c
+
+  static void
+  vhost_user_gpu_instance_init(Object *obj)
+  {
+      VhostUserGPU *g = VHOST_USER_GPU(obj);
+
+      g->vhost = VHOST_USER_BACKEND(object_new(TYPE_VHOST_USER_BACKEND));
+      object_property_add_alias(obj, "chardev",
+                                OBJECT(g->vhost), "chardev");
+  }
+
+  static const TypeInfo vhost_user_gpu_info = {
+      .name = TYPE_VHOST_USER_GPU,
+      .parent = TYPE_VIRTIO_GPU_BASE,
+      .instance_size = sizeof(VhostUserGPU),
+      .instance_init = vhost_user_gpu_instance_init,
+      .instance_finalize = vhost_user_gpu_instance_finalize,
+      .class_init = vhost_user_gpu_class_init,
+  };
+
+Using it this way entails adding a ``struct VhostUserBackend`` to your
+core object structure and manually instantiating the backend. This
+sub-structure tracks both the ``vhost_dev`` and ``CharDev`` types
+needed for the connection. Instead of calling ``vhost_dev_init`` you
+would call ``vhost_user_backend_dev_init`` which does what is needed
+on your behalf.
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH  v1 10/13] include/hw: start documenting the vhost API
  2022-03-21 15:30 [PATCH v1 00/13] various virtio docs, fixes and tweaks Alex Bennée
                   ` (8 preceding siblings ...)
  2022-03-21 15:30 ` [PATCH v1 09/13] docs/devel: start documenting writing VirtIO devices Alex Bennée
@ 2022-03-21 15:30 ` Alex Bennée
  2022-03-21 15:30 ` [PATCH v1 11/13] contrib/vhost-user-blk: fix 32 bit build and enable Alex Bennée
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2022-03-21 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, stefanha,
	marcandre.lureau, Alex Bennée

While trying to get my head around the nest of interactions for vhost
devices I though I could start by documenting the key API functions.
This patch documents the main API hooks for creating and starting a
vhost device as well as how the configuration changes are handled.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/hw/virtio/vhost.h | 132 +++++++++++++++++++++++++++++++++++---
 1 file changed, 122 insertions(+), 10 deletions(-)

diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 58a73e7b7a..b291fe4e24 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -61,6 +61,12 @@ typedef struct VhostDevConfigOps {
 } VhostDevConfigOps;
 
 struct vhost_memory;
+
+/**
+ * struct vhost_dev - common vhost_dev structure
+ * @vhost_ops: backend specific ops
+ * @config_ops: ops for config changes (see @vhost_dev_set_config_notifier)
+ */
 struct vhost_dev {
     VirtIODevice *vdev;
     MemoryListener memory_listener;
@@ -108,15 +114,129 @@ struct vhost_net {
     NetClientState *nc;
 };
 
+/**
+ * vhost_dev_init() - initialise the vhost interface
+ * @hdev: the common vhost_dev structure
+ * @opaque: opaque ptr passed to backend (vhost/vhost-user/vdpa)
+ * @backend_type: type of backend
+ * @busyloop_timeout: timeout for polling virtqueue
+ * @errp: error handle
+ *
+ * The initialisation of the vhost device will trigger the
+ * initialisation of the backend and potentially capability
+ * negotiation of backend interface. Configuration of the VirtIO
+ * itself won't happen until the interface is started.
+ *
+ * Return: 0 on success, non-zero on error while setting errp.
+ */
 int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
                    VhostBackendType backend_type,
                    uint32_t busyloop_timeout, Error **errp);
+
+/**
+ * vhost_dev_cleanup() - tear down and cleanup vhost interface
+ * @hdev: the common vhost_dev structure
+ */
 void vhost_dev_cleanup(struct vhost_dev *hdev);
-int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev);
-void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev);
+
+/**
+ * vhost_dev_enable_notifiers() - enable event notifiers
+ * @hdev: common vhost_dev structure
+ * @vdev: the VirtIODevice structure
+ *
+ * Enable notifications directly to the vhost device rather than being
+ * triggered by QEMU itself. Notifications should be enabled before
+ * the vhost device is started via @vhost_dev_start.
+ *
+ * Return: 0 on success, < 0 on error.
+ */
 int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev);
+
+/**
+ * vhost_dev_disable_notifiers - disable event notifications
+ * @hdev: common vhost_dev structure
+ * @vdev: the VirtIODevice structure
+ *
+ * Disable direct notifications to vhost device.
+ */
 void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev);
 
+/**
+ * vhost_dev_start() - start the vhost device
+ * @hdev: common vhost_dev structure
+ * @vdev: the VirtIODevice structure
+ *
+ * Starts the vhost device. From this point VirtIO feature negotiation
+ * can start and the device can start processing VirtIO transactions.
+ *
+ * Return: 0 on success, < 0 on error.
+ */
+int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev);
+
+/**
+ * vhost_dev_stop() - stop the vhost device
+ * @hdev: common vhost_dev structure
+ * @vdev: the VirtIODevice structure
+ *
+ * Stop the vhost device. After the device is stopped the notifiers
+ * can be disabled (@vhost_dev_disable_notifiers) and the device can
+ * be torn down (@vhost_dev_cleanup).
+ */
+void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev);
+
+/**
+ * DOC: vhost device configuration handling
+ *
+ * The VirtIO device configuration space is used for rarely changing
+ * or initialisation time parameters. The configuration can be updated
+ * by either the guest driver or the device itself. If the device can
+ * change the configuration over time the vhost handler should
+ * register a @VhostDevConfigOps structure with
+ * @vhost_dev_set_config_notifier so the guest can be notified. Some
+ * devices register a handler anyway and will signal an error if an
+ * unexpected config change happens.
+ */
+
+/**
+ * vhost_dev_get_config() - fetch device configuration
+ * @hdev: common vhost_dev_structure
+ * @config: pointer to device appropriate config structure
+ * @config_len: size of device appropriate config structure
+ *
+ * Return: 0 on success, < 0 on error while setting errp
+ */
+int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config,
+                         uint32_t config_len, Error **errp);
+
+/**
+ * vhost_dev_set_config() - set device configuration
+ * @hdev: common vhost_dev_structure
+ * @data: pointer to data to set
+ * @offset: offset into configuration space
+ * @size: length of set
+ * @flags: @VhostSetConfigType flags
+ *
+ * By use of @offset/@size a subset of the configuration space can be
+ * written to. The @flags are used to indicate if it is a normal
+ * transaction or related to migration.
+ *
+ * Return: 0 on success, non-zero on error
+ */
+int vhost_dev_set_config(struct vhost_dev *dev, const uint8_t *data,
+                         uint32_t offset, uint32_t size, uint32_t flags);
+
+/**
+ * vhost_dev_set_config_notifier() - register VhostDevConfigOps
+ * @hdev: common vhost_dev_structure
+ * @ops: notifier ops
+ *
+ * If the device is expected to change configuration a notifier can be
+ * setup to handle the case.
+ */
+void vhost_dev_set_config_notifier(struct vhost_dev *dev,
+                                   const VhostDevConfigOps *ops);
+
+
 /* Test and clear masked event pending status.
  * Should be called after unmask to avoid losing events.
  */
@@ -136,14 +256,6 @@ int vhost_net_set_backend(struct vhost_dev *hdev,
                           struct vhost_vring_file *file);
 
 int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write);
-int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config,
-                         uint32_t config_len, Error **errp);
-int vhost_dev_set_config(struct vhost_dev *dev, const uint8_t *data,
-                         uint32_t offset, uint32_t size, uint32_t flags);
-/* notifier callback in case vhost device config space changed
- */
-void vhost_dev_set_config_notifier(struct vhost_dev *dev,
-                                   const VhostDevConfigOps *ops);
 
 void vhost_dev_reset_inflight(struct vhost_inflight *inflight);
 void vhost_dev_free_inflight(struct vhost_inflight *inflight);
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 11/13] contrib/vhost-user-blk: fix 32 bit build and enable
  2022-03-21 15:30 [PATCH v1 00/13] various virtio docs, fixes and tweaks Alex Bennée
                   ` (9 preceding siblings ...)
  2022-03-21 15:30 ` [PATCH v1 10/13] include/hw: start documenting the vhost API Alex Bennée
@ 2022-03-21 15:30 ` Alex Bennée
  2022-03-21 22:32   ` Philippe Mathieu-Daudé
  2022-03-21 15:30 ` [PATCH v1 12/13] hw/virtio/vhost-user: don't suppress F_CONFIG when supported Alex Bennée
                   ` (3 subsequent siblings)
  14 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2022-03-21 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, Raphael Norwitz,
	stefanha, marcandre.lureau, Alex Bennée

We were not building the vhost-user-blk server due to 32 bit
compilation problems. The problem was due to format string types so
fix that and then enable the build. Tweak the rule to follow the same
rules as other vhost-user daemons.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 meson.build                             | 2 +-
 contrib/vhost-user-blk/vhost-user-blk.c | 6 +++---
 contrib/vhost-user-blk/meson.build      | 3 +--
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/meson.build b/meson.build
index 282e7c4650..0435419307 100644
--- a/meson.build
+++ b/meson.build
@@ -1326,7 +1326,7 @@ have_vhost_user_blk_server = get_option('vhost_user_blk_server') \
            error_message: 'vhost_user_blk_server requires linux') \
   .require('CONFIG_VHOST_USER' in config_host,
            error_message: 'vhost_user_blk_server requires vhost-user support') \
-  .disable_auto_if(not have_system) \
+  .disable_auto_if(not have_tools and not have_system) \
   .allowed()
 
 if get_option('fuse').disabled() and get_option('fuse_lseek').enabled()
diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
index d14b2896bf..0bee79360f 100644
--- a/contrib/vhost-user-blk/vhost-user-blk.c
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -146,7 +146,7 @@ vub_readv(VubReq *req, struct iovec *iov, uint32_t iovcnt)
     req->size = vub_iov_size(iov, iovcnt);
     rc = preadv(vdev_blk->blk_fd, iov, iovcnt, req->sector_num * 512);
     if (rc < 0) {
-        fprintf(stderr, "%s, Sector %"PRIu64", Size %lu failed with %s\n",
+        fprintf(stderr, "%s, Sector %"PRIu64", Size %zu failed with %s\n",
                 vdev_blk->blk_name, req->sector_num, req->size,
                 strerror(errno));
         return -1;
@@ -169,7 +169,7 @@ vub_writev(VubReq *req, struct iovec *iov, uint32_t iovcnt)
     req->size = vub_iov_size(iov, iovcnt);
     rc = pwritev(vdev_blk->blk_fd, iov, iovcnt, req->sector_num * 512);
     if (rc < 0) {
-        fprintf(stderr, "%s, Sector %"PRIu64", Size %lu failed with %s\n",
+        fprintf(stderr, "%s, Sector %"PRIu64", Size %zu failed with %s\n",
                 vdev_blk->blk_name, req->sector_num, req->size,
                 strerror(errno));
         return -1;
@@ -188,7 +188,7 @@ vub_discard_write_zeroes(VubReq *req, struct iovec *iov, uint32_t iovcnt,
 
     size = vub_iov_size(iov, iovcnt);
     if (size != sizeof(*desc)) {
-        fprintf(stderr, "Invalid size %ld, expect %ld\n", size, sizeof(*desc));
+        fprintf(stderr, "Invalid size %zd, expect %zd\n", size, sizeof(*desc));
         return -1;
     }
     buf = g_new0(char, size);
diff --git a/contrib/vhost-user-blk/meson.build b/contrib/vhost-user-blk/meson.build
index 601ea15ef5..dcb9e2ffcd 100644
--- a/contrib/vhost-user-blk/meson.build
+++ b/contrib/vhost-user-blk/meson.build
@@ -1,5 +1,4 @@
-# FIXME: broken on 32-bit architectures
 executable('vhost-user-blk', files('vhost-user-blk.c'),
            dependencies: [qemuutil, vhost_user],
-           build_by_default: false,
+           build_by_default: targetos == 'linux',
            install: false)
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 12/13] hw/virtio/vhost-user: don't suppress F_CONFIG when supported
  2022-03-21 15:30 [PATCH v1 00/13] various virtio docs, fixes and tweaks Alex Bennée
                   ` (10 preceding siblings ...)
  2022-03-21 15:30 ` [PATCH v1 11/13] contrib/vhost-user-blk: fix 32 bit build and enable Alex Bennée
@ 2022-03-21 15:30 ` Alex Bennée
  2022-03-22 14:02   ` Michael S. Tsirkin
  2022-03-21 15:30 ` [PATCH v1 13/13] virtio/vhost-user: dynamically assign VhostUserHostNotifiers Alex Bennée
                   ` (2 subsequent siblings)
  14 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2022-03-21 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, slp, mathieu.poirier, mst, viresh.kumar,
	Raphael Norwitz, Maxime Coquelin, stefanha, Paolo Bonzini,
	marcandre.lureau, Alex Bennée

Previously we would silently suppress VHOST_USER_PROTOCOL_F_CONFIG
during the protocol negotiation if the QEMU stub hadn't implemented
the vhost_dev_config_notifier. However this isn't the only way we can
handle config messages, the existing vdc->get/set_config can do this
as well.

Lightly re-factor the code to check for both potential methods and
instead of silently squashing the feature error out. It is unlikely
that a vhost-user backend expecting to handle CONFIG messages will
behave correctly if they never get sent.

Fixes: 1c3e5a2617 ("vhost-user: back SET/GET_CONFIG requests with a protocol feature")
Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
  - we can't check for get_config/set_config as the stack squashed vdev
  - use vhost-user-state to transmit this
---
 include/hw/virtio/vhost-user.h |  1 +
 hw/scsi/vhost-user-scsi.c      |  1 +
 hw/virtio/vhost-user.c         | 46 ++++++++++++++++++++++++----------
 3 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
index e44a41bb70..6e0e8a71a3 100644
--- a/include/hw/virtio/vhost-user.h
+++ b/include/hw/virtio/vhost-user.h
@@ -22,6 +22,7 @@ typedef struct VhostUserState {
     CharBackend *chr;
     VhostUserHostNotifier notifier[VIRTIO_QUEUE_MAX];
     int memory_slots;
+    bool supports_config;
 } VhostUserState;
 
 bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp);
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 1b2f7eed98..9be21d07ee 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -121,6 +121,7 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp)
     vsc->dev.backend_features = 0;
     vqs = vsc->dev.vqs;
 
+    s->vhost_user.supports_config = true;
     ret = vhost_dev_init(&vsc->dev, &s->vhost_user,
                          VHOST_BACKEND_TYPE_USER, 0, errp);
     if (ret < 0) {
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index b27b8c56e2..6ce082861b 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1949,14 +1949,15 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
 static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
                                    Error **errp)
 {
-    uint64_t features, protocol_features, ram_slots;
+    uint64_t features, ram_slots;
     struct vhost_user *u;
+    VhostUserState *vus = (VhostUserState *) opaque;
     int err;
 
     assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
 
     u = g_new0(struct vhost_user, 1);
-    u->user = opaque;
+    u->user = vus;
     u->dev = dev;
     dev->opaque = u;
 
@@ -1967,6 +1968,10 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
     }
 
     if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
+        bool supports_f_config = vus->supports_config ||
+            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
+        uint64_t protocol_features;
+
         dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
 
         err = vhost_user_get_u64(dev, VHOST_USER_GET_PROTOCOL_FEATURES,
@@ -1976,19 +1981,34 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
             return -EPROTO;
         }
 
-        dev->protocol_features =
-            protocol_features & VHOST_USER_PROTOCOL_FEATURE_MASK;
-
-        if (!dev->config_ops || !dev->config_ops->vhost_dev_config_notifier) {
-            /* Don't acknowledge CONFIG feature if device doesn't support it */
-            dev->protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
-        } else if (!(protocol_features &
-                    (1ULL << VHOST_USER_PROTOCOL_F_CONFIG))) {
-            error_setg(errp, "Device expects VHOST_USER_PROTOCOL_F_CONFIG "
-                       "but backend does not support it.");
-            return -EINVAL;
+        /*
+         * We will use all the protocol features we support - although
+         * we suppress F_CONFIG if we know QEMUs internal code can not support
+         * it.
+         */
+        protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
+
+        if (supports_f_config) {
+            if (!virtio_has_feature(protocol_features,
+                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
+                error_setg(errp, "vhost-user device %s expecting "
+                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
+                           "not support it.", dev->vdev->name);
+                return -EPROTO;
+            }
+        } else {
+            if (virtio_has_feature(protocol_features,
+                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
+                warn_reportf_err(*errp, "vhost-user backend supports "
+                                 "VHOST_USER_PROTOCOL_F_CONFIG for "
+                                 "device %s but QEMU does not.",
+                                 dev->vdev->name);
+                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
+            }
         }
 
+        /* final set of protocol features */
+        dev->protocol_features = protocol_features;
         err = vhost_user_set_protocol_features(dev, dev->protocol_features);
         if (err < 0) {
             error_setg_errno(errp, EPROTO, "vhost_backend_init failed");
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v1 13/13] virtio/vhost-user: dynamically assign VhostUserHostNotifiers
  2022-03-21 15:30 [PATCH v1 00/13] various virtio docs, fixes and tweaks Alex Bennée
                   ` (11 preceding siblings ...)
  2022-03-21 15:30 ` [PATCH v1 12/13] hw/virtio/vhost-user: don't suppress F_CONFIG when supported Alex Bennée
@ 2022-03-21 15:30 ` Alex Bennée
  2022-12-06 10:54   ` Philippe Mathieu-Daudé
  2022-12-06 15:45   ` Stefan Hajnoczi
  2022-03-22 13:56 ` [PATCH v1 00/13] various virtio docs, fixes and tweaks Michael S. Tsirkin
  2022-05-13 10:15 ` Michael S. Tsirkin
  14 siblings, 2 replies; 31+ messages in thread
From: Alex Bennée @ 2022-03-21 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, stefanha,
	marcandre.lureau, Alex Bennée

At a couple of hundred bytes per notifier allocating one for every
potential queue is very wasteful as most devices only have a few
queues. Instead of having this handled statically dynamically assign
them and track in a GPtrArray.

[AJB: it's hard to trigger the vhost notifiers code, I assume as it
requires a KVM guest with appropriate backend]

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/hw/virtio/vhost-user.h | 42 ++++++++++++++++-
 hw/virtio/vhost-user.c         | 83 +++++++++++++++++++++++++++-------
 hw/virtio/trace-events         |  1 +
 3 files changed, 108 insertions(+), 18 deletions(-)

diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
index 6e0e8a71a3..c6e693cd3f 100644
--- a/include/hw/virtio/vhost-user.h
+++ b/include/hw/virtio/vhost-user.h
@@ -11,21 +11,61 @@
 #include "chardev/char-fe.h"
 #include "hw/virtio/virtio.h"
 
+/**
+ * VhostUserHostNotifier - notifier information for one queue
+ * @rcu: rcu_head for cleanup
+ * @mr: memory region of notifier
+ * @addr: current mapped address
+ * @unmap_addr: address to be un-mapped
+ * @idx: virtioqueue index
+ *
+ * The VhostUserHostNotifier entries are re-used. When an old mapping
+ * is to be released it is moved to @unmap_addr and @addr is replaced.
+ * Once the RCU process has completed the unmap @unmap_addr is
+ * cleared.
+ */
 typedef struct VhostUserHostNotifier {
     struct rcu_head rcu;
     MemoryRegion mr;
     void *addr;
     void *unmap_addr;
+    int idx;
 } VhostUserHostNotifier;
 
+/**
+ * VhostUserState - shared state for all vhost-user devices
+ * @chr: the character backend for the socket
+ * @notifiers: GPtrArray of @VhostUserHostnotifier
+ * @memory_slots:
+ */
 typedef struct VhostUserState {
     CharBackend *chr;
-    VhostUserHostNotifier notifier[VIRTIO_QUEUE_MAX];
+    GPtrArray *notifiers;
     int memory_slots;
     bool supports_config;
 } VhostUserState;
 
+/**
+ * vhost_user_init() - initialise shared vhost_user state
+ * @user: allocated area for storing shared state
+ * @chr: the chardev for the vhost socket
+ * @errp: error handle
+ *
+ * User can either directly g_new() space for the state or embed
+ * VhostUserState in their larger device structure and just point to
+ * it.
+ *
+ * Return: true on success, false on error while setting errp.
+ */
 bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp);
+
+/**
+ * vhost_user_cleanup() - cleanup state
+ * @user: ptr to use state
+ *
+ * Cleans up shared state and notifiers, callee is responsible for
+ * freeing the @VhostUserState memory itself.
+ */
 void vhost_user_cleanup(VhostUserState *user);
 
 #endif
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 6ce082861b..4c0423de55 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1174,14 +1174,16 @@ static void vhost_user_host_notifier_free(VhostUserHostNotifier *n)
     n->unmap_addr = NULL;
 }
 
-static void vhost_user_host_notifier_remove(VhostUserState *user,
-                                            VirtIODevice *vdev, int queue_idx)
+/*
+ * clean-up function for notifier, will finally free the structure
+ * under rcu.
+ */
+static void vhost_user_host_notifier_remove(VhostUserHostNotifier *n,
+                                            VirtIODevice *vdev)
 {
-    VhostUserHostNotifier *n = &user->notifier[queue_idx];
-
     if (n->addr) {
         if (vdev) {
-            virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false);
+            virtio_queue_set_host_notifier_mr(vdev, n->idx, &n->mr, false);
         }
         assert(!n->unmap_addr);
         n->unmap_addr = n->addr;
@@ -1225,6 +1227,15 @@ static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
     return 0;
 }
 
+static VhostUserHostNotifier *fetch_notifier(VhostUserState *u,
+                                             int idx)
+{
+    if (idx >= u->notifiers->len) {
+        return NULL;
+    }
+    return g_ptr_array_index(u->notifiers, idx);
+}
+
 static int vhost_user_get_vring_base(struct vhost_dev *dev,
                                      struct vhost_vring_state *ring)
 {
@@ -1237,7 +1248,10 @@ static int vhost_user_get_vring_base(struct vhost_dev *dev,
     };
     struct vhost_user *u = dev->opaque;
 
-    vhost_user_host_notifier_remove(u->user, dev->vdev, ring->index);
+    VhostUserHostNotifier *n = fetch_notifier(u->user, ring->index);
+    if (n) {
+        vhost_user_host_notifier_remove(n, dev->vdev);
+    }
 
     ret = vhost_user_write(dev, &msg, NULL, 0);
     if (ret < 0) {
@@ -1502,6 +1516,29 @@ static int vhost_user_slave_handle_config_change(struct vhost_dev *dev)
     return dev->config_ops->vhost_dev_config_notifier(dev);
 }
 
+/*
+ * Fetch or create the notifier for a given idx. Newly created
+ * notifiers are added to the pointer array that tracks them.
+ */
+static VhostUserHostNotifier *fetch_or_create_notifier(VhostUserState *u,
+                                                       int idx)
+{
+    VhostUserHostNotifier *n = NULL;
+    if (idx >= u->notifiers->len) {
+        g_ptr_array_set_size(u->notifiers, idx);
+    }
+
+    n = g_ptr_array_index(u->notifiers, idx);
+    if (!n) {
+        n = g_new0(VhostUserHostNotifier, 1);
+        n->idx = idx;
+        g_ptr_array_insert(u->notifiers, idx, n);
+        trace_vhost_user_create_notifier(idx, n);
+    }
+
+    return n;
+}
+
 static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
                                                        VhostUserVringArea *area,
                                                        int fd)
@@ -1521,9 +1558,12 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
         return -EINVAL;
     }
 
-    n = &user->notifier[queue_idx];
-
-    vhost_user_host_notifier_remove(user, vdev, queue_idx);
+    /*
+     * Fetch notifier and invalidate any old data before setting up
+     * new mapped address.
+     */
+    n = fetch_or_create_notifier(user, queue_idx);
+    vhost_user_host_notifier_remove(n, vdev);
 
     if (area->u64 & VHOST_USER_VRING_NOFD_MASK) {
         return 0;
@@ -2526,6 +2566,20 @@ static int vhost_user_set_inflight_fd(struct vhost_dev *dev,
     return vhost_user_write(dev, &msg, &inflight->fd, 1);
 }
 
+static void vhost_user_state_destroy(gpointer data)
+{
+    VhostUserHostNotifier *n = (VhostUserHostNotifier *) data;
+    if (n) {
+        vhost_user_host_notifier_remove(n, NULL);
+        object_unparent(OBJECT(&n->mr));
+        /*
+         * We can't free until vhost_user_host_notifier_remove has
+         * done it's thing so schedule the free with RCU.
+         */
+        g_free_rcu(n, rcu);
+    }
+}
+
 bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp)
 {
     if (user->chr) {
@@ -2534,23 +2588,18 @@ bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp)
     }
     user->chr = chr;
     user->memory_slots = 0;
+    user->notifiers = g_ptr_array_new_full(VIRTIO_QUEUE_MAX / 4,
+                                           &vhost_user_state_destroy);
     return true;
 }
 
 void vhost_user_cleanup(VhostUserState *user)
 {
-    int i;
-    VhostUserHostNotifier *n;
-
     if (!user->chr) {
         return;
     }
     memory_region_transaction_begin();
-    for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
-        n = &user->notifier[i];
-        vhost_user_host_notifier_remove(user, NULL, i);
-        object_unparent(OBJECT(&n->mr));
-    }
+    user->notifiers = (GPtrArray *) g_ptr_array_free(user->notifiers, true);
     memory_region_transaction_commit();
     user->chr = NULL;
 }
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index fd213e2a27..b40392a593 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -23,6 +23,7 @@ vhost_user_postcopy_waker_found(uint64_t client_addr) "0x%"PRIx64
 vhost_user_postcopy_waker_nomatch(const char *rb, uint64_t rb_offset) "%s + 0x%"PRIx64
 vhost_user_read(uint32_t req, uint32_t flags) "req:%d flags:0x%"PRIx32""
 vhost_user_write(uint32_t req, uint32_t flags) "req:%d flags:0x%"PRIx32""
+vhost_user_create_notifier(int idx, void *n) "idx:%d n:%p"
 
 # vhost-vdpa.c
 vhost_vdpa_dma_map(void *vdpa, int fd, uint32_t msg_type, uint64_t iova, uint64_t size, uint64_t uaddr, uint8_t perm, uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32" iova: 0x%"PRIx64" size: 0x%"PRIx64" uaddr: 0x%"PRIx64" perm: 0x%"PRIx8" type: %"PRIu8
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH v1 01/13] hw/virtio: move virtio-pci.h into shared include space
  2022-03-21 15:30   ` [Virtio-fs] " Alex Bennée
@ 2022-03-21 22:27     ` Philippe Mathieu-Daudé
  -1 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-21 22:27 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: slp, mathieu.poirier, David Hildenbrand, viresh.kumar, mst,
	Dr. David Alan Gilbert, Raphael Norwitz, open list:virtiofs,
	Eric Auger, stefanha, marcandre.lureau

On 21/3/22 16:30, Alex Bennée wrote:
> This allows other device classes that will be exposed via PCI to be
> able to do so in the appropriate hw/ directory. I resisted the
> temptation to re-order headers to be more aesthetically pleasing.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20200925125147.26943-4-alex.bennee@linaro.org>
> 
> ---
> v2
>    - add i2c/rng device to changes
> ---
>   {hw => include/hw}/virtio/virtio-pci.h | 0
>   hw/virtio/vhost-scsi-pci.c             | 2 +-
>   hw/virtio/vhost-user-blk-pci.c         | 2 +-
>   hw/virtio/vhost-user-fs-pci.c          | 2 +-
>   hw/virtio/vhost-user-i2c-pci.c         | 2 +-
>   hw/virtio/vhost-user-input-pci.c       | 2 +-
>   hw/virtio/vhost-user-rng-pci.c         | 2 +-
>   hw/virtio/vhost-user-scsi-pci.c        | 2 +-
>   hw/virtio/vhost-user-vsock-pci.c       | 2 +-
>   hw/virtio/vhost-vsock-pci.c            | 2 +-
>   hw/virtio/virtio-9p-pci.c              | 2 +-
>   hw/virtio/virtio-balloon-pci.c         | 2 +-
>   hw/virtio/virtio-blk-pci.c             | 2 +-
>   hw/virtio/virtio-input-host-pci.c      | 2 +-
>   hw/virtio/virtio-input-pci.c           | 2 +-
>   hw/virtio/virtio-iommu-pci.c           | 2 +-
>   hw/virtio/virtio-net-pci.c             | 2 +-
>   hw/virtio/virtio-pci.c                 | 2 +-
>   hw/virtio/virtio-rng-pci.c             | 2 +-
>   hw/virtio/virtio-scsi-pci.c            | 2 +-
>   hw/virtio/virtio-serial-pci.c          | 2 +-
>   21 files changed, 20 insertions(+), 20 deletions(-)
>   rename {hw => include/hw}/virtio/virtio-pci.h (100%)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Virtio-fs] [PATCH v1 01/13] hw/virtio: move virtio-pci.h into shared include space
@ 2022-03-21 22:27     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-21 22:27 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, David Hildenbrand,
	Dr. David Alan Gilbert, Raphael Norwitz, open list:virtiofs,
	Eric Auger, stefanha, marcandre.lureau

On 21/3/22 16:30, Alex Bennée wrote:
> This allows other device classes that will be exposed via PCI to be
> able to do so in the appropriate hw/ directory. I resisted the
> temptation to re-order headers to be more aesthetically pleasing.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20200925125147.26943-4-alex.bennee@linaro.org>
> 
> ---
> v2
>    - add i2c/rng device to changes
> ---
>   {hw => include/hw}/virtio/virtio-pci.h | 0
>   hw/virtio/vhost-scsi-pci.c             | 2 +-
>   hw/virtio/vhost-user-blk-pci.c         | 2 +-
>   hw/virtio/vhost-user-fs-pci.c          | 2 +-
>   hw/virtio/vhost-user-i2c-pci.c         | 2 +-
>   hw/virtio/vhost-user-input-pci.c       | 2 +-
>   hw/virtio/vhost-user-rng-pci.c         | 2 +-
>   hw/virtio/vhost-user-scsi-pci.c        | 2 +-
>   hw/virtio/vhost-user-vsock-pci.c       | 2 +-
>   hw/virtio/vhost-vsock-pci.c            | 2 +-
>   hw/virtio/virtio-9p-pci.c              | 2 +-
>   hw/virtio/virtio-balloon-pci.c         | 2 +-
>   hw/virtio/virtio-blk-pci.c             | 2 +-
>   hw/virtio/virtio-input-host-pci.c      | 2 +-
>   hw/virtio/virtio-input-pci.c           | 2 +-
>   hw/virtio/virtio-iommu-pci.c           | 2 +-
>   hw/virtio/virtio-net-pci.c             | 2 +-
>   hw/virtio/virtio-pci.c                 | 2 +-
>   hw/virtio/virtio-rng-pci.c             | 2 +-
>   hw/virtio/virtio-scsi-pci.c            | 2 +-
>   hw/virtio/virtio-serial-pci.c          | 2 +-
>   21 files changed, 20 insertions(+), 20 deletions(-)
>   rename {hw => include/hw}/virtio/virtio-pci.h (100%)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v1 03/13] hw/virtio: add vhost_user_[read|write] trace points
  2022-03-21 15:30 ` [PATCH v1 03/13] hw/virtio: add vhost_user_[read|write] " Alex Bennée
@ 2022-03-21 22:29   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-21 22:29 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, stefanha, marcandre.lureau

On 21/3/22 16:30, Alex Bennée wrote:
> These are useful when trying to debug the initial vhost-user
> negotiation, especially when it hard to get logging from the low level
> library on the other side.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> v2
>    - fixed arguments
> ---
>   hw/virtio/vhost-user.c | 4 ++++
>   hw/virtio/trace-events | 2 ++
>   2 files changed, 6 insertions(+)
> 
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 6abbc9da32..b27b8c56e2 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -489,6 +489,8 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
>           return ret < 0 ? -saved_errno : -EIO;
>       }
>   
> +    trace_vhost_user_write(msg->hdr.request, msg->hdr.flags);
> +
>       return 0;
>   }
>   
> @@ -542,6 +544,8 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,
>           }
>       }
>   
> +    trace_vhost_user_read(msg.hdr.request, msg.hdr.flags);
> +
>       return 0;
>   }
>   
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 46851a7cd1..fd213e2a27 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -21,6 +21,8 @@ vhost_user_set_mem_table_withfd(int index, const char *name, uint64_t memory_siz
>   vhost_user_postcopy_waker(const char *rb, uint64_t rb_offset) "%s + 0x%"PRIx64
>   vhost_user_postcopy_waker_found(uint64_t client_addr) "0x%"PRIx64
>   vhost_user_postcopy_waker_nomatch(const char *rb, uint64_t rb_offset) "%s + 0x%"PRIx64
> +vhost_user_read(uint32_t req, uint32_t flags) "req:%d flags:0x%"PRIx32""
> +vhost_user_write(uint32_t req, uint32_t flags) "req:%d flags:0x%"PRIx32""

"req:%u", otherwise:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v1 04/13] docs: vhost-user: clean up request/reply description
  2022-03-21 15:30 ` [PATCH v1 04/13] docs: vhost-user: clean up request/reply description Alex Bennée
@ 2022-03-21 22:30   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-21 22:30 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, stefanha,
	marcandre.lureau, Paolo Bonzini

On 21/3/22 16:30, Alex Bennée wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> It is not necessary to mention which side is sending/receiving
> each payload; it is more interesting to say which is the request
> and which is the reply.  This also matches what vhost-user-gpu.rst
> already does.
> 
> While at it, ensure that all messages list both the request and
> the reply payload.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Message-Id: <20210226143413.188046-2-pbonzini@redhat.com>
> ---
>   docs/interop/vhost-user.rst | 163 +++++++++++++++++++++---------------
>   1 file changed, 95 insertions(+), 68 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v1 08/13] libvhost-user: expose vu_request_to_string
  2022-03-21 15:30 ` [PATCH v1 08/13] libvhost-user: expose vu_request_to_string Alex Bennée
@ 2022-03-21 22:31   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-21 22:31 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, stefanha, marcandre.lureau

On 21/3/22 16:30, Alex Bennée wrote:
> This is useful for more human readable debug messages in vhost-user
> programs.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   subprojects/libvhost-user/libvhost-user.h | 9 +++++++++
>   subprojects/libvhost-user/libvhost-user.c | 2 +-
>   2 files changed, 10 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v1 11/13] contrib/vhost-user-blk: fix 32 bit build and enable
  2022-03-21 15:30 ` [PATCH v1 11/13] contrib/vhost-user-blk: fix 32 bit build and enable Alex Bennée
@ 2022-03-21 22:32   ` Philippe Mathieu-Daudé
  2022-05-16 10:46     ` Alex Bennée
  0 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-21 22:32 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, Raphael Norwitz,
	stefanha, marcandre.lureau

On 21/3/22 16:30, Alex Bennée wrote:
> We were not building the vhost-user-blk server due to 32 bit
> compilation problems. The problem was due to format string types so
> fix that and then enable the build. Tweak the rule to follow the same
> rules as other vhost-user daemons.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   meson.build                             | 2 +-
>   contrib/vhost-user-blk/vhost-user-blk.c | 6 +++---
>   contrib/vhost-user-blk/meson.build      | 3 +--
>   3 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 282e7c4650..0435419307 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1326,7 +1326,7 @@ have_vhost_user_blk_server = get_option('vhost_user_blk_server') \
>              error_message: 'vhost_user_blk_server requires linux') \
>     .require('CONFIG_VHOST_USER' in config_host,
>              error_message: 'vhost_user_blk_server requires vhost-user support') \
> -  .disable_auto_if(not have_system) \
> +  .disable_auto_if(not have_tools and not have_system) \

s/and/or/?

>     .allowed()


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH  v1 00/13] various virtio docs, fixes and tweaks
  2022-03-21 15:30 [PATCH v1 00/13] various virtio docs, fixes and tweaks Alex Bennée
                   ` (12 preceding siblings ...)
  2022-03-21 15:30 ` [PATCH v1 13/13] virtio/vhost-user: dynamically assign VhostUserHostNotifiers Alex Bennée
@ 2022-03-22 13:56 ` Michael S. Tsirkin
  2022-03-22 15:50   ` Alex Bennée
  2022-05-13 10:15 ` Michael S. Tsirkin
  14 siblings, 1 reply; 31+ messages in thread
From: Michael S. Tsirkin @ 2022-03-22 13:56 UTC (permalink / raw)
  To: Alex Bennée
  Cc: slp, mathieu.poirier, viresh.kumar, qemu-devel, stefanha,
	marcandre.lureau

On Mon, Mar 21, 2022 at 03:30:24PM +0000, Alex Bennée wrote:
> Hi,
> 
> This series is a sub-set of patches while I was trying to re-rev my
> virtio-rpmb patches. It attempts to address a few things:
> 
>   - improve documentation for virtio/vhost/vhost-user
>   - document some of the API
>   - a hacky fix for F_CONFIG handling
>   - putting VhostUserState on a diet, make VhostUserHostNotifier dynamic

So I think this is best deferred until after the release,
more of a cleanup than a bugfix.

I will tag this series, but please do remind me after the release
to help make sure it does not get lost.


> In particular I've been trying to better understand how vhost-user
> interactions are meant to work and why there are two different methods
> for instantiating them. If my supposition is correct perhaps a number
> of devices that don't have in-kernel vhost equivalents could be converted?

Hope I understand your question.  Well we started off with saying
vhost-user is just a backend, so should not affect the frontend device.
This is clean and makes migration work e.g. you can migrate between
different backends, but it makes adding features more work.


> While working onthe VhostUserHostNotifier changes I found it quite
> hard to trigger the code. Is this rarely used code or just requires
> backends we don't see in the testing?

Which function are you asking about exactly?

> Alex Bennée (10):
>   hw/virtio: move virtio-pci.h into shared include space
>   virtio-pci: add notification trace points
>   hw/virtio: add vhost_user_[read|write] trace points
>   vhost-user.rst: add clarifying language about protocol negotiation
>   libvhost-user: expose vu_request_to_string
>   docs/devel: start documenting writing VirtIO devices
>   include/hw: start documenting the vhost API
>   contrib/vhost-user-blk: fix 32 bit build and enable
>   hw/virtio/vhost-user: don't suppress F_CONFIG when supported
>   virtio/vhost-user: dynamically assign VhostUserHostNotifiers
> 
> Paolo Bonzini (3):
>   docs: vhost-user: clean up request/reply description
>   docs: vhost-user: rewrite section on ring state machine
>   docs: vhost-user: replace master/slave with front-end/back-end
> 
>  docs/devel/index-internals.rst            |   1 +
>  docs/devel/virtio-backends.rst            | 214 +++++++++
>  docs/interop/vhost-user-gpu.rst           |  10 +-
>  docs/interop/vhost-user.rst               | 555 ++++++++++++----------
>  meson.build                               |   2 +-
>  include/hw/virtio/vhost-user.h            |  43 +-
>  include/hw/virtio/vhost.h                 | 132 ++++-
>  {hw => include/hw}/virtio/virtio-pci.h    |   0
>  subprojects/libvhost-user/libvhost-user.h |   9 +
>  contrib/vhost-user-blk/vhost-user-blk.c   |   6 +-
>  hw/scsi/vhost-user-scsi.c                 |   1 +
>  hw/virtio/vhost-scsi-pci.c                |   2 +-
>  hw/virtio/vhost-user-blk-pci.c            |   2 +-
>  hw/virtio/vhost-user-fs-pci.c             |   2 +-
>  hw/virtio/vhost-user-i2c-pci.c            |   2 +-
>  hw/virtio/vhost-user-input-pci.c          |   2 +-
>  hw/virtio/vhost-user-rng-pci.c            |   2 +-
>  hw/virtio/vhost-user-scsi-pci.c           |   2 +-
>  hw/virtio/vhost-user-vsock-pci.c          |   2 +-
>  hw/virtio/vhost-user.c                    | 133 ++++--
>  hw/virtio/vhost-vsock-pci.c               |   2 +-
>  hw/virtio/virtio-9p-pci.c                 |   2 +-
>  hw/virtio/virtio-balloon-pci.c            |   2 +-
>  hw/virtio/virtio-blk-pci.c                |   2 +-
>  hw/virtio/virtio-input-host-pci.c         |   2 +-
>  hw/virtio/virtio-input-pci.c              |   2 +-
>  hw/virtio/virtio-iommu-pci.c              |   2 +-
>  hw/virtio/virtio-net-pci.c                |   2 +-
>  hw/virtio/virtio-pci.c                    |   5 +-
>  hw/virtio/virtio-rng-pci.c                |   2 +-
>  hw/virtio/virtio-scsi-pci.c               |   2 +-
>  hw/virtio/virtio-serial-pci.c             |   2 +-
>  subprojects/libvhost-user/libvhost-user.c |   2 +-
>  contrib/vhost-user-blk/meson.build        |   3 +-
>  hw/virtio/trace-events                    |  10 +-
>  35 files changed, 831 insertions(+), 333 deletions(-)
>  create mode 100644 docs/devel/virtio-backends.rst
>  rename {hw => include/hw}/virtio/virtio-pci.h (100%)
> 
> -- 
> 2.30.2



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH  v1 12/13] hw/virtio/vhost-user: don't suppress F_CONFIG when supported
  2022-03-21 15:30 ` [PATCH v1 12/13] hw/virtio/vhost-user: don't suppress F_CONFIG when supported Alex Bennée
@ 2022-03-22 14:02   ` Michael S. Tsirkin
  2022-03-22 15:54     ` Alex Bennée
  0 siblings, 1 reply; 31+ messages in thread
From: Michael S. Tsirkin @ 2022-03-22 14:02 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Fam Zheng, slp, mathieu.poirier, viresh.kumar, qemu-devel,
	Raphael Norwitz, Maxime Coquelin, stefanha, Paolo Bonzini,
	marcandre.lureau

On Mon, Mar 21, 2022 at 03:30:36PM +0000, Alex Bennée wrote:
> Previously we would silently suppress VHOST_USER_PROTOCOL_F_CONFIG
> during the protocol negotiation if the QEMU stub hadn't implemented
> the vhost_dev_config_notifier. However this isn't the only way we can
> handle config messages, the existing vdc->get/set_config can do this
> as well.


Could you give an example where the problem is encountered please?

> Lightly re-factor the code to check for both potential methods and
> instead of silently squashing the feature error out. It is unlikely
> that a vhost-user backend expecting to handle CONFIG messages will
> behave correctly if they never get sent.

Hmm but are you sure? Most devices work mostly fine without CONFIG
messages, there's a chance a backend set this flag just in case
without much thought ...

> Fixes: 1c3e5a2617 ("vhost-user: back SET/GET_CONFIG requests with a protocol feature")

I'm not sure whether something is broken or this is a cleanup patch.
Fixes tag means "if you have 1c3e5a2617 you should pick this patch", so
cleanups don't need a fixes: tag.


> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
>   - we can't check for get_config/set_config as the stack squashed vdev
>   - use vhost-user-state to transmit this
> ---
>  include/hw/virtio/vhost-user.h |  1 +
>  hw/scsi/vhost-user-scsi.c      |  1 +
>  hw/virtio/vhost-user.c         | 46 ++++++++++++++++++++++++----------
>  3 files changed, 35 insertions(+), 13 deletions(-)
> 
> diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
> index e44a41bb70..6e0e8a71a3 100644
> --- a/include/hw/virtio/vhost-user.h
> +++ b/include/hw/virtio/vhost-user.h
> @@ -22,6 +22,7 @@ typedef struct VhostUserState {
>      CharBackend *chr;
>      VhostUserHostNotifier notifier[VIRTIO_QUEUE_MAX];
>      int memory_slots;
> +    bool supports_config;
>  } VhostUserState;
>  
>  bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp);
> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
> index 1b2f7eed98..9be21d07ee 100644
> --- a/hw/scsi/vhost-user-scsi.c
> +++ b/hw/scsi/vhost-user-scsi.c
> @@ -121,6 +121,7 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp)
>      vsc->dev.backend_features = 0;
>      vqs = vsc->dev.vqs;
>  
> +    s->vhost_user.supports_config = true;
>      ret = vhost_dev_init(&vsc->dev, &s->vhost_user,
>                           VHOST_BACKEND_TYPE_USER, 0, errp);
>      if (ret < 0) {
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index b27b8c56e2..6ce082861b 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1949,14 +1949,15 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
>  static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>                                     Error **errp)
>  {
> -    uint64_t features, protocol_features, ram_slots;
> +    uint64_t features, ram_slots;
>      struct vhost_user *u;
> +    VhostUserState *vus = (VhostUserState *) opaque;
>      int err;
>  
>      assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
>  
>      u = g_new0(struct vhost_user, 1);
> -    u->user = opaque;
> +    u->user = vus;
>      u->dev = dev;
>      dev->opaque = u;
>  
> @@ -1967,6 +1968,10 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>      }
>  
>      if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> +        bool supports_f_config = vus->supports_config ||
> +            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
> +        uint64_t protocol_features;
> +
>          dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
>  
>          err = vhost_user_get_u64(dev, VHOST_USER_GET_PROTOCOL_FEATURES,
> @@ -1976,19 +1981,34 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>              return -EPROTO;
>          }
>  
> -        dev->protocol_features =
> -            protocol_features & VHOST_USER_PROTOCOL_FEATURE_MASK;
> -
> -        if (!dev->config_ops || !dev->config_ops->vhost_dev_config_notifier) {
> -            /* Don't acknowledge CONFIG feature if device doesn't support it */
> -            dev->protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> -        } else if (!(protocol_features &
> -                    (1ULL << VHOST_USER_PROTOCOL_F_CONFIG))) {
> -            error_setg(errp, "Device expects VHOST_USER_PROTOCOL_F_CONFIG "
> -                       "but backend does not support it.");
> -            return -EINVAL;
> +        /*
> +         * We will use all the protocol features we support - although
> +         * we suppress F_CONFIG if we know QEMUs internal code can not support
> +         * it.
> +         */
> +        protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
> +
> +        if (supports_f_config) {
> +            if (!virtio_has_feature(protocol_features,
> +                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
> +                error_setg(errp, "vhost-user device %s expecting "
> +                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
> +                           "not support it.", dev->vdev->name);
> +                return -EPROTO;
> +            }
> +        } else {
> +            if (virtio_has_feature(protocol_features,
> +                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
> +                warn_reportf_err(*errp, "vhost-user backend supports "
> +                                 "VHOST_USER_PROTOCOL_F_CONFIG for "
> +                                 "device %s but QEMU does not.",
> +                                 dev->vdev->name);
> +                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> +            }
>          }
>  
> +        /* final set of protocol features */
> +        dev->protocol_features = protocol_features;
>          err = vhost_user_set_protocol_features(dev, dev->protocol_features);
>          if (err < 0) {
>              error_setg_errno(errp, EPROTO, "vhost_backend_init failed");
> -- 
> 2.30.2



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH  v1 00/13] various virtio docs, fixes and tweaks
  2022-03-22 13:56 ` [PATCH v1 00/13] various virtio docs, fixes and tweaks Michael S. Tsirkin
@ 2022-03-22 15:50   ` Alex Bennée
  2022-03-22 16:13     ` Michael S. Tsirkin
  0 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2022-03-22 15:50 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: slp, mathieu.poirier, viresh.kumar, qemu-devel, stefanha,
	marcandre.lureau


"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Mon, Mar 21, 2022 at 03:30:24PM +0000, Alex Bennée wrote:
>> Hi,
>> 
>> This series is a sub-set of patches while I was trying to re-rev my
>> virtio-rpmb patches. It attempts to address a few things:
>> 
>>   - improve documentation for virtio/vhost/vhost-user
>>   - document some of the API
>>   - a hacky fix for F_CONFIG handling
>>   - putting VhostUserState on a diet, make VhostUserHostNotifier dynamic
>
> So I think this is best deferred until after the release,
> more of a cleanup than a bugfix.

Sorry I should have made it clearer - I wasn't intending this for 7.0
but I also didn't want it bound up with the rpmb changes which will take
longer to land.

>
> I will tag this series, but please do remind me after the release
> to help make sure it does not get lost.
>
>
>> In particular I've been trying to better understand how vhost-user
>> interactions are meant to work and why there are two different methods
>> for instantiating them. If my supposition is correct perhaps a number
>> of devices that don't have in-kernel vhost equivalents could be converted?
>
> Hope I understand your question.  Well we started off with saying
> vhost-user is just a backend, so should not affect the frontend device.
> This is clean and makes migration work e.g. you can migrate between
> different backends, but it makes adding features more work.

This is covered in the doc patch, specifically:

  vhost_ops vs TYPE_VHOST_USER_BACKEND

>> While working onthe VhostUserHostNotifier changes I found it quite
>> hard to trigger the code. Is this rarely used code or just requires
>> backends we don't see in the testing?
>
> Which function are you asking about exactly?

  vhost_user_slave_handle_vring_host_notifier

which is the only place where a mapping is set up AFAICT.

>
>> Alex Bennée (10):
>>   hw/virtio: move virtio-pci.h into shared include space
>>   virtio-pci: add notification trace points
>>   hw/virtio: add vhost_user_[read|write] trace points
>>   vhost-user.rst: add clarifying language about protocol negotiation
>>   libvhost-user: expose vu_request_to_string
>>   docs/devel: start documenting writing VirtIO devices
>>   include/hw: start documenting the vhost API
>>   contrib/vhost-user-blk: fix 32 bit build and enable
>>   hw/virtio/vhost-user: don't suppress F_CONFIG when supported
>>   virtio/vhost-user: dynamically assign VhostUserHostNotifiers
>> 
>> Paolo Bonzini (3):
>>   docs: vhost-user: clean up request/reply description
>>   docs: vhost-user: rewrite section on ring state machine
>>   docs: vhost-user: replace master/slave with front-end/back-end
>> 
>>  docs/devel/index-internals.rst            |   1 +
>>  docs/devel/virtio-backends.rst            | 214 +++++++++
>>  docs/interop/vhost-user-gpu.rst           |  10 +-
>>  docs/interop/vhost-user.rst               | 555 ++++++++++++----------
>>  meson.build                               |   2 +-
>>  include/hw/virtio/vhost-user.h            |  43 +-
>>  include/hw/virtio/vhost.h                 | 132 ++++-
>>  {hw => include/hw}/virtio/virtio-pci.h    |   0
>>  subprojects/libvhost-user/libvhost-user.h |   9 +
>>  contrib/vhost-user-blk/vhost-user-blk.c   |   6 +-
>>  hw/scsi/vhost-user-scsi.c                 |   1 +
>>  hw/virtio/vhost-scsi-pci.c                |   2 +-
>>  hw/virtio/vhost-user-blk-pci.c            |   2 +-
>>  hw/virtio/vhost-user-fs-pci.c             |   2 +-
>>  hw/virtio/vhost-user-i2c-pci.c            |   2 +-
>>  hw/virtio/vhost-user-input-pci.c          |   2 +-
>>  hw/virtio/vhost-user-rng-pci.c            |   2 +-
>>  hw/virtio/vhost-user-scsi-pci.c           |   2 +-
>>  hw/virtio/vhost-user-vsock-pci.c          |   2 +-
>>  hw/virtio/vhost-user.c                    | 133 ++++--
>>  hw/virtio/vhost-vsock-pci.c               |   2 +-
>>  hw/virtio/virtio-9p-pci.c                 |   2 +-
>>  hw/virtio/virtio-balloon-pci.c            |   2 +-
>>  hw/virtio/virtio-blk-pci.c                |   2 +-
>>  hw/virtio/virtio-input-host-pci.c         |   2 +-
>>  hw/virtio/virtio-input-pci.c              |   2 +-
>>  hw/virtio/virtio-iommu-pci.c              |   2 +-
>>  hw/virtio/virtio-net-pci.c                |   2 +-
>>  hw/virtio/virtio-pci.c                    |   5 +-
>>  hw/virtio/virtio-rng-pci.c                |   2 +-
>>  hw/virtio/virtio-scsi-pci.c               |   2 +-
>>  hw/virtio/virtio-serial-pci.c             |   2 +-
>>  subprojects/libvhost-user/libvhost-user.c |   2 +-
>>  contrib/vhost-user-blk/meson.build        |   3 +-
>>  hw/virtio/trace-events                    |  10 +-
>>  35 files changed, 831 insertions(+), 333 deletions(-)
>>  create mode 100644 docs/devel/virtio-backends.rst
>>  rename {hw => include/hw}/virtio/virtio-pci.h (100%)
>> 
>> -- 
>> 2.30.2


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH  v1 12/13] hw/virtio/vhost-user: don't suppress F_CONFIG when supported
  2022-03-22 14:02   ` Michael S. Tsirkin
@ 2022-03-22 15:54     ` Alex Bennée
  2022-03-22 16:22       ` Michael S. Tsirkin
  0 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2022-03-22 15:54 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Fam Zheng, slp, mathieu.poirier, viresh.kumar, qemu-devel,
	Raphael Norwitz, Maxime Coquelin, stefanha, Paolo Bonzini,
	marcandre.lureau


"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Mon, Mar 21, 2022 at 03:30:36PM +0000, Alex Bennée wrote:
>> Previously we would silently suppress VHOST_USER_PROTOCOL_F_CONFIG
>> during the protocol negotiation if the QEMU stub hadn't implemented
>> the vhost_dev_config_notifier. However this isn't the only way we can
>> handle config messages, the existing vdc->get/set_config can do this
>> as well.
>
> Could you give an example where the problem is encountered please?

Well I only came across it when I realised by rpmb implementation wasn't
sending config updates to the vhost-user daemon because it only
implemented set/get_config methods.

I think vhost-user-scsi suffers from this but I'm not able to test it as
it's not clear how to do it. The vhost-user-scsi daemon want to attach
to real SCSI devices rather than a file for the block device. I guess I
need to somehow add "fake" scsi nodes to my host system which SCSI
commands can be passed to?

When was the last time vhost-user-scsi was tested with anything?

>> Lightly re-factor the code to check for both potential methods and
>> instead of silently squashing the feature error out. It is unlikely
>> that a vhost-user backend expecting to handle CONFIG messages will
>> behave correctly if they never get sent.
>
> Hmm but are you sure? Most devices work mostly fine without CONFIG
> messages, there's a chance a backend set this flag just in case
> without much thought ...

But that would be a bug right? Certainly a mismatch if something really
does want to see config messages. Again RPMB needs this because it's the
vhost-user daemon that knows the size of the device.

>> Fixes: 1c3e5a2617 ("vhost-user: back SET/GET_CONFIG requests with a protocol feature")
>
> I'm not sure whether something is broken or this is a cleanup patch.
> Fixes tag means "if you have 1c3e5a2617 you should pick this patch", so
> cleanups don't need a fixes: tag.

No I think it's broken, we just didn't notice because as you say most
devices don't need to care.

>
>
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> 
>> ---
>>   - we can't check for get_config/set_config as the stack squashed vdev
>>   - use vhost-user-state to transmit this
>> ---
>>  include/hw/virtio/vhost-user.h |  1 +
>>  hw/scsi/vhost-user-scsi.c      |  1 +
>>  hw/virtio/vhost-user.c         | 46 ++++++++++++++++++++++++----------
>>  3 files changed, 35 insertions(+), 13 deletions(-)
>> 
>> diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
>> index e44a41bb70..6e0e8a71a3 100644
>> --- a/include/hw/virtio/vhost-user.h
>> +++ b/include/hw/virtio/vhost-user.h
>> @@ -22,6 +22,7 @@ typedef struct VhostUserState {
>>      CharBackend *chr;
>>      VhostUserHostNotifier notifier[VIRTIO_QUEUE_MAX];
>>      int memory_slots;
>> +    bool supports_config;
>>  } VhostUserState;
>>  
>>  bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp);
>> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
>> index 1b2f7eed98..9be21d07ee 100644
>> --- a/hw/scsi/vhost-user-scsi.c
>> +++ b/hw/scsi/vhost-user-scsi.c
>> @@ -121,6 +121,7 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp)
>>      vsc->dev.backend_features = 0;
>>      vqs = vsc->dev.vqs;
>>  
>> +    s->vhost_user.supports_config = true;
>>      ret = vhost_dev_init(&vsc->dev, &s->vhost_user,
>>                           VHOST_BACKEND_TYPE_USER, 0, errp);
>>      if (ret < 0) {
>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>> index b27b8c56e2..6ce082861b 100644
>> --- a/hw/virtio/vhost-user.c
>> +++ b/hw/virtio/vhost-user.c
>> @@ -1949,14 +1949,15 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
>>  static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>>                                     Error **errp)
>>  {
>> -    uint64_t features, protocol_features, ram_slots;
>> +    uint64_t features, ram_slots;
>>      struct vhost_user *u;
>> +    VhostUserState *vus = (VhostUserState *) opaque;
>>      int err;
>>  
>>      assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
>>  
>>      u = g_new0(struct vhost_user, 1);
>> -    u->user = opaque;
>> +    u->user = vus;
>>      u->dev = dev;
>>      dev->opaque = u;
>>  
>> @@ -1967,6 +1968,10 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>>      }
>>  
>>      if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
>> +        bool supports_f_config = vus->supports_config ||
>> +            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
>> +        uint64_t protocol_features;
>> +
>>          dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
>>  
>>          err = vhost_user_get_u64(dev, VHOST_USER_GET_PROTOCOL_FEATURES,
>> @@ -1976,19 +1981,34 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>>              return -EPROTO;
>>          }
>>  
>> -        dev->protocol_features =
>> -            protocol_features & VHOST_USER_PROTOCOL_FEATURE_MASK;
>> -
>> -        if (!dev->config_ops || !dev->config_ops->vhost_dev_config_notifier) {
>> -            /* Don't acknowledge CONFIG feature if device doesn't support it */
>> -            dev->protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
>> -        } else if (!(protocol_features &
>> -                    (1ULL << VHOST_USER_PROTOCOL_F_CONFIG))) {
>> -            error_setg(errp, "Device expects VHOST_USER_PROTOCOL_F_CONFIG "
>> -                       "but backend does not support it.");
>> -            return -EINVAL;
>> +        /*
>> +         * We will use all the protocol features we support - although
>> +         * we suppress F_CONFIG if we know QEMUs internal code can not support
>> +         * it.
>> +         */
>> +        protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
>> +
>> +        if (supports_f_config) {
>> +            if (!virtio_has_feature(protocol_features,
>> +                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
>> +                error_setg(errp, "vhost-user device %s expecting "
>> +                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
>> +                           "not support it.", dev->vdev->name);
>> +                return -EPROTO;
>> +            }
>> +        } else {
>> +            if (virtio_has_feature(protocol_features,
>> +                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
>> +                warn_reportf_err(*errp, "vhost-user backend supports "
>> +                                 "VHOST_USER_PROTOCOL_F_CONFIG for "
>> +                                 "device %s but QEMU does not.",
>> +                                 dev->vdev->name);
>> +                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
>> +            }
>>          }
>>  
>> +        /* final set of protocol features */
>> +        dev->protocol_features = protocol_features;
>>          err = vhost_user_set_protocol_features(dev, dev->protocol_features);
>>          if (err < 0) {
>>              error_setg_errno(errp, EPROTO, "vhost_backend_init failed");
>> -- 
>> 2.30.2


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH  v1 00/13] various virtio docs, fixes and tweaks
  2022-03-22 15:50   ` Alex Bennée
@ 2022-03-22 16:13     ` Michael S. Tsirkin
  0 siblings, 0 replies; 31+ messages in thread
From: Michael S. Tsirkin @ 2022-03-22 16:13 UTC (permalink / raw)
  To: Alex Bennée
  Cc: slp, mathieu.poirier, viresh.kumar, qemu-devel, stefanha,
	marcandre.lureau

On Tue, Mar 22, 2022 at 03:50:28PM +0000, Alex Bennée wrote:
> 
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Mon, Mar 21, 2022 at 03:30:24PM +0000, Alex Bennée wrote:
> >> Hi,
> >> 
> >> This series is a sub-set of patches while I was trying to re-rev my
> >> virtio-rpmb patches. It attempts to address a few things:
> >> 
> >>   - improve documentation for virtio/vhost/vhost-user
> >>   - document some of the API
> >>   - a hacky fix for F_CONFIG handling
> >>   - putting VhostUserState on a diet, make VhostUserHostNotifier dynamic
> >
> > So I think this is best deferred until after the release,
> > more of a cleanup than a bugfix.
> 
> Sorry I should have made it clearer - I wasn't intending this for 7.0
> but I also didn't want it bound up with the rpmb changes which will take
> longer to land.

Makes sense, I was just making sure.

> >
> > I will tag this series, but please do remind me after the release
> > to help make sure it does not get lost.
> >
> >
> >> In particular I've been trying to better understand how vhost-user
> >> interactions are meant to work and why there are two different methods
> >> for instantiating them. If my supposition is correct perhaps a number
> >> of devices that don't have in-kernel vhost equivalents could be converted?
> >
> > Hope I understand your question.  Well we started off with saying
> > vhost-user is just a backend, so should not affect the frontend device.
> > This is clean and makes migration work e.g. you can migrate between
> > different backends, but it makes adding features more work.
> 
> This is covered in the doc patch, specifically:
> 
>   vhost_ops vs TYPE_VHOST_USER_BACKEND

I guess I misunderstood then - what was the question again?

> >> hard to trigger the code. Is this rarely used code or just requires
> >> backends we don't see in the testing?
> >
> > Which function are you asking about exactly?
> 
>   vhost_user_slave_handle_vring_host_notifier
> 
> which is the only place where a mapping is set up AFAICT.

Well it's mostly for use with vdpa, but I think vhost user bridge
is supposed to support that with a special flag (-H use host notifier).
It's for debugging purposes as it wastes lots of CPU polling
the notification memory.

> >
> >> Alex Bennée (10):
> >>   hw/virtio: move virtio-pci.h into shared include space
> >>   virtio-pci: add notification trace points
> >>   hw/virtio: add vhost_user_[read|write] trace points
> >>   vhost-user.rst: add clarifying language about protocol negotiation
> >>   libvhost-user: expose vu_request_to_string
> >>   docs/devel: start documenting writing VirtIO devices
> >>   include/hw: start documenting the vhost API
> >>   contrib/vhost-user-blk: fix 32 bit build and enable
> >>   hw/virtio/vhost-user: don't suppress F_CONFIG when supported
> >>   virtio/vhost-user: dynamically assign VhostUserHostNotifiers
> >> 
> >> Paolo Bonzini (3):
> >>   docs: vhost-user: clean up request/reply description
> >>   docs: vhost-user: rewrite section on ring state machine
> >>   docs: vhost-user: replace master/slave with front-end/back-end
> >> 
> >>  docs/devel/index-internals.rst            |   1 +
> >>  docs/devel/virtio-backends.rst            | 214 +++++++++
> >>  docs/interop/vhost-user-gpu.rst           |  10 +-
> >>  docs/interop/vhost-user.rst               | 555 ++++++++++++----------
> >>  meson.build                               |   2 +-
> >>  include/hw/virtio/vhost-user.h            |  43 +-
> >>  include/hw/virtio/vhost.h                 | 132 ++++-
> >>  {hw => include/hw}/virtio/virtio-pci.h    |   0
> >>  subprojects/libvhost-user/libvhost-user.h |   9 +
> >>  contrib/vhost-user-blk/vhost-user-blk.c   |   6 +-
> >>  hw/scsi/vhost-user-scsi.c                 |   1 +
> >>  hw/virtio/vhost-scsi-pci.c                |   2 +-
> >>  hw/virtio/vhost-user-blk-pci.c            |   2 +-
> >>  hw/virtio/vhost-user-fs-pci.c             |   2 +-
> >>  hw/virtio/vhost-user-i2c-pci.c            |   2 +-
> >>  hw/virtio/vhost-user-input-pci.c          |   2 +-
> >>  hw/virtio/vhost-user-rng-pci.c            |   2 +-
> >>  hw/virtio/vhost-user-scsi-pci.c           |   2 +-
> >>  hw/virtio/vhost-user-vsock-pci.c          |   2 +-
> >>  hw/virtio/vhost-user.c                    | 133 ++++--
> >>  hw/virtio/vhost-vsock-pci.c               |   2 +-
> >>  hw/virtio/virtio-9p-pci.c                 |   2 +-
> >>  hw/virtio/virtio-balloon-pci.c            |   2 +-
> >>  hw/virtio/virtio-blk-pci.c                |   2 +-
> >>  hw/virtio/virtio-input-host-pci.c         |   2 +-
> >>  hw/virtio/virtio-input-pci.c              |   2 +-
> >>  hw/virtio/virtio-iommu-pci.c              |   2 +-
> >>  hw/virtio/virtio-net-pci.c                |   2 +-
> >>  hw/virtio/virtio-pci.c                    |   5 +-
> >>  hw/virtio/virtio-rng-pci.c                |   2 +-
> >>  hw/virtio/virtio-scsi-pci.c               |   2 +-
> >>  hw/virtio/virtio-serial-pci.c             |   2 +-
> >>  subprojects/libvhost-user/libvhost-user.c |   2 +-
> >>  contrib/vhost-user-blk/meson.build        |   3 +-
> >>  hw/virtio/trace-events                    |  10 +-
> >>  35 files changed, 831 insertions(+), 333 deletions(-)
> >>  create mode 100644 docs/devel/virtio-backends.rst
> >>  rename {hw => include/hw}/virtio/virtio-pci.h (100%)
> >> 
> >> -- 
> >> 2.30.2
> 
> 
> -- 
> Alex Bennée



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH  v1 12/13] hw/virtio/vhost-user: don't suppress F_CONFIG when supported
  2022-03-22 15:54     ` Alex Bennée
@ 2022-03-22 16:22       ` Michael S. Tsirkin
  0 siblings, 0 replies; 31+ messages in thread
From: Michael S. Tsirkin @ 2022-03-22 16:22 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Fam Zheng, slp, mathieu.poirier, viresh.kumar, felipe,
	qemu-devel, Raphael Norwitz, Maxime Coquelin, stefanha,
	Paolo Bonzini, marcandre.lureau

On Tue, Mar 22, 2022 at 03:54:32PM +0000, Alex Bennée wrote:
> 
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Mon, Mar 21, 2022 at 03:30:36PM +0000, Alex Bennée wrote:
> >> Previously we would silently suppress VHOST_USER_PROTOCOL_F_CONFIG
> >> during the protocol negotiation if the QEMU stub hadn't implemented
> >> the vhost_dev_config_notifier. However this isn't the only way we can
> >> handle config messages, the existing vdc->get/set_config can do this
> >> as well.
> >
> > Could you give an example where the problem is encountered please?
> 
> Well I only came across it when I realised by rpmb implementation wasn't
> sending config updates to the vhost-user daemon because it only
> implemented set/get_config methods.
> 
> I think vhost-user-scsi suffers from this but I'm not able to test it as
> it's not clear how to do it. The vhost-user-scsi daemon want to attach
> to real SCSI devices rather than a file for the block device. I guess I
> need to somehow add "fake" scsi nodes to my host system which SCSI
> commands can be passed to?
> 
> When was the last time vhost-user-scsi was tested with anything?

It's for passthrough mostly, yes. I think you can use tcm_loop
on top of loopback on top of a file. Didn't try myself.

> >> Lightly re-factor the code to check for both potential methods and
> >> instead of silently squashing the feature error out. It is unlikely
> >> that a vhost-user backend expecting to handle CONFIG messages will
> >> behave correctly if they never get sent.
> >
> > Hmm but are you sure? Most devices work mostly fine without CONFIG
> > messages, there's a chance a backend set this flag just in case
> > without much thought ...
> 
> But that would be a bug right? Certainly a mismatch if something really
> does want to see config messages. Again RPMB needs this because it's the
> vhost-user daemon that knows the size of the device.


Could you pls clarify how do you create a config that's broken with RPMB?

> >> Fixes: 1c3e5a2617 ("vhost-user: back SET/GET_CONFIG requests with a protocol feature")
> >
> > I'm not sure whether something is broken or this is a cleanup patch.
> > Fixes tag means "if you have 1c3e5a2617 you should pick this patch", so
> > cleanups don't need a fixes: tag.
> 
> No I think it's broken, we just didn't notice because as you say most
> devices don't need to care.

I was mostly asking about the fixes tag.  If no existing user cares then
it's best to avoid the fixes tag, it's a hint for backporters which
versions need the patch.


> >
> >
> >> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> Cc: Michael S. Tsirkin <mst@redhat.com>
> >> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >> 
> >> ---
> >>   - we can't check for get_config/set_config as the stack squashed vdev
> >>   - use vhost-user-state to transmit this
> >> ---
> >>  include/hw/virtio/vhost-user.h |  1 +
> >>  hw/scsi/vhost-user-scsi.c      |  1 +
> >>  hw/virtio/vhost-user.c         | 46 ++++++++++++++++++++++++----------
> >>  3 files changed, 35 insertions(+), 13 deletions(-)
> >> 
> >> diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
> >> index e44a41bb70..6e0e8a71a3 100644
> >> --- a/include/hw/virtio/vhost-user.h
> >> +++ b/include/hw/virtio/vhost-user.h
> >> @@ -22,6 +22,7 @@ typedef struct VhostUserState {
> >>      CharBackend *chr;
> >>      VhostUserHostNotifier notifier[VIRTIO_QUEUE_MAX];
> >>      int memory_slots;
> >> +    bool supports_config;
> >>  } VhostUserState;
> >>  
> >>  bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp);
> >> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
> >> index 1b2f7eed98..9be21d07ee 100644
> >> --- a/hw/scsi/vhost-user-scsi.c
> >> +++ b/hw/scsi/vhost-user-scsi.c
> >> @@ -121,6 +121,7 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp)
> >>      vsc->dev.backend_features = 0;
> >>      vqs = vsc->dev.vqs;
> >>  
> >> +    s->vhost_user.supports_config = true;
> >>      ret = vhost_dev_init(&vsc->dev, &s->vhost_user,
> >>                           VHOST_BACKEND_TYPE_USER, 0, errp);
> >>      if (ret < 0) {
> >> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> >> index b27b8c56e2..6ce082861b 100644
> >> --- a/hw/virtio/vhost-user.c
> >> +++ b/hw/virtio/vhost-user.c
> >> @@ -1949,14 +1949,15 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
> >>  static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> >>                                     Error **errp)
> >>  {
> >> -    uint64_t features, protocol_features, ram_slots;
> >> +    uint64_t features, ram_slots;
> >>      struct vhost_user *u;
> >> +    VhostUserState *vus = (VhostUserState *) opaque;
> >>      int err;
> >>  
> >>      assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
> >>  
> >>      u = g_new0(struct vhost_user, 1);
> >> -    u->user = opaque;
> >> +    u->user = vus;
> >>      u->dev = dev;
> >>      dev->opaque = u;
> >>  
> >> @@ -1967,6 +1968,10 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> >>      }
> >>  
> >>      if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> >> +        bool supports_f_config = vus->supports_config ||
> >> +            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
> >> +        uint64_t protocol_features;
> >> +
> >>          dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> >>  
> >>          err = vhost_user_get_u64(dev, VHOST_USER_GET_PROTOCOL_FEATURES,
> >> @@ -1976,19 +1981,34 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> >>              return -EPROTO;
> >>          }
> >>  
> >> -        dev->protocol_features =
> >> -            protocol_features & VHOST_USER_PROTOCOL_FEATURE_MASK;
> >> -
> >> -        if (!dev->config_ops || !dev->config_ops->vhost_dev_config_notifier) {
> >> -            /* Don't acknowledge CONFIG feature if device doesn't support it */
> >> -            dev->protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> >> -        } else if (!(protocol_features &
> >> -                    (1ULL << VHOST_USER_PROTOCOL_F_CONFIG))) {
> >> -            error_setg(errp, "Device expects VHOST_USER_PROTOCOL_F_CONFIG "
> >> -                       "but backend does not support it.");
> >> -            return -EINVAL;
> >> +        /*
> >> +         * We will use all the protocol features we support - although
> >> +         * we suppress F_CONFIG if we know QEMUs internal code can not support
> >> +         * it.
> >> +         */
> >> +        protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
> >> +
> >> +        if (supports_f_config) {
> >> +            if (!virtio_has_feature(protocol_features,
> >> +                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
> >> +                error_setg(errp, "vhost-user device %s expecting "
> >> +                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
> >> +                           "not support it.", dev->vdev->name);
> >> +                return -EPROTO;
> >> +            }
> >> +        } else {
> >> +            if (virtio_has_feature(protocol_features,
> >> +                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
> >> +                warn_reportf_err(*errp, "vhost-user backend supports "
> >> +                                 "VHOST_USER_PROTOCOL_F_CONFIG for "
> >> +                                 "device %s but QEMU does not.",
> >> +                                 dev->vdev->name);
> >> +                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> >> +            }
> >>          }
> >>  
> >> +        /* final set of protocol features */
> >> +        dev->protocol_features = protocol_features;
> >>          err = vhost_user_set_protocol_features(dev, dev->protocol_features);
> >>          if (err < 0) {
> >>              error_setg_errno(errp, EPROTO, "vhost_backend_init failed");
> >> -- 
> >> 2.30.2
> 
> 
> -- 
> Alex Bennée



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH  v1 00/13] various virtio docs, fixes and tweaks
  2022-03-21 15:30 [PATCH v1 00/13] various virtio docs, fixes and tweaks Alex Bennée
                   ` (13 preceding siblings ...)
  2022-03-22 13:56 ` [PATCH v1 00/13] various virtio docs, fixes and tweaks Michael S. Tsirkin
@ 2022-05-13 10:15 ` Michael S. Tsirkin
  14 siblings, 0 replies; 31+ messages in thread
From: Michael S. Tsirkin @ 2022-05-13 10:15 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, slp, marcandre.lureau, stefanha, mathieu.poirier,
	viresh.kumar

On Mon, Mar 21, 2022 at 03:30:24PM +0000, Alex Bennée wrote:
>   contrib/vhost-user-blk: fix 32 bit build and enable

I applied up to this point. Pls respond to Philippe's comment.

-- 
MST



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v1 11/13] contrib/vhost-user-blk: fix 32 bit build and enable
  2022-03-21 22:32   ` Philippe Mathieu-Daudé
@ 2022-05-16 10:46     ` Alex Bennée
  0 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2022-05-16 10:46 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, slp, mathieu.poirier, mst, viresh.kumar,
	Raphael Norwitz, stefanha, marcandre.lureau


Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com> writes:

> On 21/3/22 16:30, Alex Bennée wrote:
>> We were not building the vhost-user-blk server due to 32 bit
>> compilation problems. The problem was due to format string types so
>> fix that and then enable the build. Tweak the rule to follow the same
>> rules as other vhost-user daemons.
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   meson.build                             | 2 +-
>>   contrib/vhost-user-blk/vhost-user-blk.c | 6 +++---
>>   contrib/vhost-user-blk/meson.build      | 3 +--
>>   3 files changed, 5 insertions(+), 6 deletions(-)
>> diff --git a/meson.build b/meson.build
>> index 282e7c4650..0435419307 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -1326,7 +1326,7 @@ have_vhost_user_blk_server = get_option('vhost_user_blk_server') \
>>              error_message: 'vhost_user_blk_server requires linux') \
>>     .require('CONFIG_VHOST_USER' in config_host,
>>              error_message: 'vhost_user_blk_server requires vhost-user support') \
>> -  .disable_auto_if(not have_system) \
>> +  .disable_auto_if(not have_tools and not have_system) \
>
> s/and/or/?

AIUI this is for auto-enabling so you probably want to build if you have
system build or are explicitly building tools. I think, it's confusing
being in a double negative.

>
>>     .allowed()


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v1 13/13] virtio/vhost-user: dynamically assign VhostUserHostNotifiers
  2022-03-21 15:30 ` [PATCH v1 13/13] virtio/vhost-user: dynamically assign VhostUserHostNotifiers Alex Bennée
@ 2022-12-06 10:54   ` Philippe Mathieu-Daudé
  2022-12-06 15:45   ` Stefan Hajnoczi
  1 sibling, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-06 10:54 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: slp, mathieu.poirier, mst, viresh.kumar, stefanha, marcandre.lureau

Hi Alex,

On 21/3/22 16:30, Alex Bennée wrote:
> At a couple of hundred bytes per notifier allocating one for every
> potential queue is very wasteful as most devices only have a few
> queues. Instead of having this handled statically dynamically assign
> them and track in a GPtrArray.
> 
> [AJB: it's hard to trigger the vhost notifiers code, I assume as it
> requires a KVM guest with appropriate backend]
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   include/hw/virtio/vhost-user.h | 42 ++++++++++++++++-
>   hw/virtio/vhost-user.c         | 83 +++++++++++++++++++++++++++-------
>   hw/virtio/trace-events         |  1 +
>   3 files changed, 108 insertions(+), 18 deletions(-)


>   void vhost_user_cleanup(VhostUserState *user)
>   {
> -    int i;
> -    VhostUserHostNotifier *n;
> -
>       if (!user->chr) {
>           return;
>       }
>       memory_region_transaction_begin();
> -    for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
> -        n = &user->notifier[i];
> -        vhost_user_host_notifier_remove(user, NULL, i);
> -        object_unparent(OBJECT(&n->mr));
> -    }
> +    user->notifiers = (GPtrArray *) g_ptr_array_free(user->notifiers, true);

Since you replaced an iteration by a single function call, the MR 
transaction guards can be removed now, right?

>       memory_region_transaction_commit();
>       user->chr = NULL;
>   }


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v1 13/13] virtio/vhost-user: dynamically assign VhostUserHostNotifiers
  2022-03-21 15:30 ` [PATCH v1 13/13] virtio/vhost-user: dynamically assign VhostUserHostNotifiers Alex Bennée
  2022-12-06 10:54   ` Philippe Mathieu-Daudé
@ 2022-12-06 15:45   ` Stefan Hajnoczi
  1 sibling, 0 replies; 31+ messages in thread
From: Stefan Hajnoczi @ 2022-12-06 15:45 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, slp, mathieu.poirier, mst, viresh.kumar, stefanha,
	marcandre.lureau

On Mon, 21 Mar 2022 at 11:59, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> At a couple of hundred bytes per notifier allocating one for every
> potential queue is very wasteful as most devices only have a few
> queues. Instead of having this handled statically dynamically assign
> them and track in a GPtrArray.
>
> [AJB: it's hard to trigger the vhost notifiers code, I assume as it
> requires a KVM guest with appropriate backend]

I think vhost works with TCG. There is ioeventfd emulation in QEMU's
memory dispatch code, see memory_region_dispatch_write_eventfds().
There is irqfd emulation code for VIRTIO devices in
virtio_queue_set_guest_notifier_fd_handler().

Why do you say it's hard to trigger?

Stefan

>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  include/hw/virtio/vhost-user.h | 42 ++++++++++++++++-
>  hw/virtio/vhost-user.c         | 83 +++++++++++++++++++++++++++-------
>  hw/virtio/trace-events         |  1 +
>  3 files changed, 108 insertions(+), 18 deletions(-)
>
> diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
> index 6e0e8a71a3..c6e693cd3f 100644
> --- a/include/hw/virtio/vhost-user.h
> +++ b/include/hw/virtio/vhost-user.h
> @@ -11,21 +11,61 @@
>  #include "chardev/char-fe.h"
>  #include "hw/virtio/virtio.h"
>
> +/**
> + * VhostUserHostNotifier - notifier information for one queue
> + * @rcu: rcu_head for cleanup
> + * @mr: memory region of notifier
> + * @addr: current mapped address
> + * @unmap_addr: address to be un-mapped
> + * @idx: virtioqueue index
> + *
> + * The VhostUserHostNotifier entries are re-used. When an old mapping
> + * is to be released it is moved to @unmap_addr and @addr is replaced.
> + * Once the RCU process has completed the unmap @unmap_addr is
> + * cleared.
> + */
>  typedef struct VhostUserHostNotifier {
>      struct rcu_head rcu;
>      MemoryRegion mr;
>      void *addr;
>      void *unmap_addr;
> +    int idx;
>  } VhostUserHostNotifier;
>
> +/**
> + * VhostUserState - shared state for all vhost-user devices
> + * @chr: the character backend for the socket
> + * @notifiers: GPtrArray of @VhostUserHostnotifier
> + * @memory_slots:
> + */
>  typedef struct VhostUserState {
>      CharBackend *chr;
> -    VhostUserHostNotifier notifier[VIRTIO_QUEUE_MAX];
> +    GPtrArray *notifiers;
>      int memory_slots;
>      bool supports_config;
>  } VhostUserState;
>
> +/**
> + * vhost_user_init() - initialise shared vhost_user state
> + * @user: allocated area for storing shared state
> + * @chr: the chardev for the vhost socket
> + * @errp: error handle
> + *
> + * User can either directly g_new() space for the state or embed
> + * VhostUserState in their larger device structure and just point to
> + * it.
> + *
> + * Return: true on success, false on error while setting errp.
> + */
>  bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp);
> +
> +/**
> + * vhost_user_cleanup() - cleanup state
> + * @user: ptr to use state
> + *
> + * Cleans up shared state and notifiers, callee is responsible for
> + * freeing the @VhostUserState memory itself.
> + */
>  void vhost_user_cleanup(VhostUserState *user);
>
>  #endif
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 6ce082861b..4c0423de55 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1174,14 +1174,16 @@ static void vhost_user_host_notifier_free(VhostUserHostNotifier *n)
>      n->unmap_addr = NULL;
>  }
>
> -static void vhost_user_host_notifier_remove(VhostUserState *user,
> -                                            VirtIODevice *vdev, int queue_idx)
> +/*
> + * clean-up function for notifier, will finally free the structure
> + * under rcu.
> + */
> +static void vhost_user_host_notifier_remove(VhostUserHostNotifier *n,
> +                                            VirtIODevice *vdev)
>  {
> -    VhostUserHostNotifier *n = &user->notifier[queue_idx];
> -
>      if (n->addr) {
>          if (vdev) {
> -            virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false);
> +            virtio_queue_set_host_notifier_mr(vdev, n->idx, &n->mr, false);
>          }
>          assert(!n->unmap_addr);
>          n->unmap_addr = n->addr;
> @@ -1225,6 +1227,15 @@ static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
>      return 0;
>  }
>
> +static VhostUserHostNotifier *fetch_notifier(VhostUserState *u,
> +                                             int idx)
> +{
> +    if (idx >= u->notifiers->len) {
> +        return NULL;
> +    }
> +    return g_ptr_array_index(u->notifiers, idx);
> +}
> +
>  static int vhost_user_get_vring_base(struct vhost_dev *dev,
>                                       struct vhost_vring_state *ring)
>  {
> @@ -1237,7 +1248,10 @@ static int vhost_user_get_vring_base(struct vhost_dev *dev,
>      };
>      struct vhost_user *u = dev->opaque;
>
> -    vhost_user_host_notifier_remove(u->user, dev->vdev, ring->index);
> +    VhostUserHostNotifier *n = fetch_notifier(u->user, ring->index);
> +    if (n) {
> +        vhost_user_host_notifier_remove(n, dev->vdev);
> +    }
>
>      ret = vhost_user_write(dev, &msg, NULL, 0);
>      if (ret < 0) {
> @@ -1502,6 +1516,29 @@ static int vhost_user_slave_handle_config_change(struct vhost_dev *dev)
>      return dev->config_ops->vhost_dev_config_notifier(dev);
>  }
>
> +/*
> + * Fetch or create the notifier for a given idx. Newly created
> + * notifiers are added to the pointer array that tracks them.
> + */
> +static VhostUserHostNotifier *fetch_or_create_notifier(VhostUserState *u,
> +                                                       int idx)
> +{
> +    VhostUserHostNotifier *n = NULL;
> +    if (idx >= u->notifiers->len) {
> +        g_ptr_array_set_size(u->notifiers, idx);
> +    }
> +
> +    n = g_ptr_array_index(u->notifiers, idx);
> +    if (!n) {
> +        n = g_new0(VhostUserHostNotifier, 1);
> +        n->idx = idx;
> +        g_ptr_array_insert(u->notifiers, idx, n);
> +        trace_vhost_user_create_notifier(idx, n);
> +    }
> +
> +    return n;
> +}
> +
>  static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
>                                                         VhostUserVringArea *area,
>                                                         int fd)
> @@ -1521,9 +1558,12 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
>          return -EINVAL;
>      }
>
> -    n = &user->notifier[queue_idx];
> -
> -    vhost_user_host_notifier_remove(user, vdev, queue_idx);
> +    /*
> +     * Fetch notifier and invalidate any old data before setting up
> +     * new mapped address.
> +     */
> +    n = fetch_or_create_notifier(user, queue_idx);
> +    vhost_user_host_notifier_remove(n, vdev);
>
>      if (area->u64 & VHOST_USER_VRING_NOFD_MASK) {
>          return 0;
> @@ -2526,6 +2566,20 @@ static int vhost_user_set_inflight_fd(struct vhost_dev *dev,
>      return vhost_user_write(dev, &msg, &inflight->fd, 1);
>  }
>
> +static void vhost_user_state_destroy(gpointer data)
> +{
> +    VhostUserHostNotifier *n = (VhostUserHostNotifier *) data;
> +    if (n) {
> +        vhost_user_host_notifier_remove(n, NULL);
> +        object_unparent(OBJECT(&n->mr));
> +        /*
> +         * We can't free until vhost_user_host_notifier_remove has
> +         * done it's thing so schedule the free with RCU.
> +         */
> +        g_free_rcu(n, rcu);
> +    }
> +}
> +
>  bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp)
>  {
>      if (user->chr) {
> @@ -2534,23 +2588,18 @@ bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp)
>      }
>      user->chr = chr;
>      user->memory_slots = 0;
> +    user->notifiers = g_ptr_array_new_full(VIRTIO_QUEUE_MAX / 4,
> +                                           &vhost_user_state_destroy);
>      return true;
>  }
>
>  void vhost_user_cleanup(VhostUserState *user)
>  {
> -    int i;
> -    VhostUserHostNotifier *n;
> -
>      if (!user->chr) {
>          return;
>      }
>      memory_region_transaction_begin();
> -    for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
> -        n = &user->notifier[i];
> -        vhost_user_host_notifier_remove(user, NULL, i);
> -        object_unparent(OBJECT(&n->mr));
> -    }
> +    user->notifiers = (GPtrArray *) g_ptr_array_free(user->notifiers, true);
>      memory_region_transaction_commit();
>      user->chr = NULL;
>  }
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index fd213e2a27..b40392a593 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -23,6 +23,7 @@ vhost_user_postcopy_waker_found(uint64_t client_addr) "0x%"PRIx64
>  vhost_user_postcopy_waker_nomatch(const char *rb, uint64_t rb_offset) "%s + 0x%"PRIx64
>  vhost_user_read(uint32_t req, uint32_t flags) "req:%d flags:0x%"PRIx32""
>  vhost_user_write(uint32_t req, uint32_t flags) "req:%d flags:0x%"PRIx32""
> +vhost_user_create_notifier(int idx, void *n) "idx:%d n:%p"
>
>  # vhost-vdpa.c
>  vhost_vdpa_dma_map(void *vdpa, int fd, uint32_t msg_type, uint64_t iova, uint64_t size, uint64_t uaddr, uint8_t perm, uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32" iova: 0x%"PRIx64" size: 0x%"PRIx64" uaddr: 0x%"PRIx64" perm: 0x%"PRIx8" type: %"PRIu8
> --
> 2.30.2
>
>


^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2022-12-06 15:46 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-21 15:30 [PATCH v1 00/13] various virtio docs, fixes and tweaks Alex Bennée
2022-03-21 15:30 ` [PATCH v1 01/13] hw/virtio: move virtio-pci.h into shared include space Alex Bennée
2022-03-21 15:30   ` [Virtio-fs] " Alex Bennée
2022-03-21 22:27   ` Philippe Mathieu-Daudé
2022-03-21 22:27     ` [Virtio-fs] " Philippe Mathieu-Daudé
2022-03-21 15:30 ` [PATCH v1 02/13] virtio-pci: add notification trace points Alex Bennée
2022-03-21 15:30 ` [PATCH v1 03/13] hw/virtio: add vhost_user_[read|write] " Alex Bennée
2022-03-21 22:29   ` Philippe Mathieu-Daudé
2022-03-21 15:30 ` [PATCH v1 04/13] docs: vhost-user: clean up request/reply description Alex Bennée
2022-03-21 22:30   ` Philippe Mathieu-Daudé
2022-03-21 15:30 ` [PATCH v1 05/13] docs: vhost-user: rewrite section on ring state machine Alex Bennée
2022-03-21 15:30 ` [PATCH v1 06/13] docs: vhost-user: replace master/slave with front-end/back-end Alex Bennée
2022-03-21 15:30 ` [PATCH v1 07/13] vhost-user.rst: add clarifying language about protocol negotiation Alex Bennée
2022-03-21 15:30 ` [PATCH v1 08/13] libvhost-user: expose vu_request_to_string Alex Bennée
2022-03-21 22:31   ` Philippe Mathieu-Daudé
2022-03-21 15:30 ` [PATCH v1 09/13] docs/devel: start documenting writing VirtIO devices Alex Bennée
2022-03-21 15:30 ` [PATCH v1 10/13] include/hw: start documenting the vhost API Alex Bennée
2022-03-21 15:30 ` [PATCH v1 11/13] contrib/vhost-user-blk: fix 32 bit build and enable Alex Bennée
2022-03-21 22:32   ` Philippe Mathieu-Daudé
2022-05-16 10:46     ` Alex Bennée
2022-03-21 15:30 ` [PATCH v1 12/13] hw/virtio/vhost-user: don't suppress F_CONFIG when supported Alex Bennée
2022-03-22 14:02   ` Michael S. Tsirkin
2022-03-22 15:54     ` Alex Bennée
2022-03-22 16:22       ` Michael S. Tsirkin
2022-03-21 15:30 ` [PATCH v1 13/13] virtio/vhost-user: dynamically assign VhostUserHostNotifiers Alex Bennée
2022-12-06 10:54   ` Philippe Mathieu-Daudé
2022-12-06 15:45   ` Stefan Hajnoczi
2022-03-22 13:56 ` [PATCH v1 00/13] various virtio docs, fixes and tweaks Michael S. Tsirkin
2022-03-22 15:50   ` Alex Bennée
2022-03-22 16:13     ` Michael S. Tsirkin
2022-05-13 10:15 ` Michael S. Tsirkin

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.