* [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 22:27 ` Philippe Mathieu-Daudé
2022-03-21 15:30 ` [PATCH v1 02/13] virtio-pci: add notification trace points Alex Bennée
` (13 subsequent siblings)
14 siblings, 1 reply; 29+ 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] 29+ messages in thread* Re: [PATCH v1 01/13] hw/virtio: move virtio-pci.h into shared include space
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 22:27 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 29+ 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] 29+ 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 ` [PATCH v1 01/13] hw/virtio: move virtio-pci.h into shared include space 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; 29+ 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] 29+ 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 ` [PATCH v1 01/13] hw/virtio: move virtio-pci.h into shared include space 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ 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; 29+ 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] 29+ messages in thread