qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH-for-8.0 00/10] hw/virtio: Build most objects as target independent units
@ 2022-12-12 23:05 Philippe Mathieu-Daudé
  2022-12-12 23:05 ` [PATCH-for-8.0 01/10] hw/virtio: Add missing "hw/core/cpu.h" include Philippe Mathieu-Daudé
                   ` (9 more replies)
  0 siblings, 10 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-12 23:05 UTC (permalink / raw)
  To: qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz,
	Philippe Mathieu-Daudé

Currently the inlined virtio_access_is_big_endian() function
"hw/virtio/virtio-access.h" which is used by all I/O accesses
force any virtio device to be built as target-dependent object.

This series isolates the few VirtIO target specific bits, trying
to not impact the performance (a function is un-inlined once
not in the hot path).

At the end only 6 files remain in the target specific source set,
all other files are built once.

On a Linux/x86_64 host when building QEMU configured with
--disable-user, before this series a "make clean all" builds
7713 objects, after it only build 6831... I don't think my maths
are correct, because that would save building a few hundreds objects.

RFC because only build-tested.

Regards,

Phil.

Philippe Mathieu-Daudé (10):
  hw/virtio: Add missing "hw/core/cpu.h" include
  hw/virtio: Rename virtio_ss[] -> specific_virtio_ss[]
  hw/virtio: Constify qmp_virtio_feature_map_t[]
  hw/virtio: Extract config read/write accessors to virtio-config.c
  hw/virtio: Extract QMP related code virtio-qmp.c
  hw/virtio: Cache access_is_big_endian value in VirtIODevice state
  hw/virtio: Directly access cached VirtIODevice::access_is_big_endian
  hw/virtio: Un-inline virtio_access_is_big_endian()
  hw/virtio: Extract vhost_user_ram_slots_max() to vhost-user-target.c
  hw/virtio: Make most of virtio devices target-independent

 hw/9pfs/meson.build               |   2 +-
 hw/block/dataplane/meson.build    |   2 +-
 hw/block/meson.build              |   4 +-
 hw/char/meson.build               |   2 +-
 hw/net/meson.build                |   2 +-
 hw/virtio/meson.build             |  45 +-
 hw/virtio/vhost-user-target.c     |  29 ++
 hw/virtio/vhost-user.c            |  26 +-
 hw/virtio/virtio-config.c         | 224 +++++++++
 hw/virtio/virtio-qmp.c            | 631 +++++++++++++++++++++++
 hw/virtio/virtio-qmp.h            |  20 +
 hw/virtio/virtio.c                | 799 +-----------------------------
 include/hw/virtio/vhost-user.h    |   7 +
 include/hw/virtio/virtio-access.h |  63 +--
 include/hw/virtio/virtio.h        |   1 +
 15 files changed, 974 insertions(+), 883 deletions(-)
 create mode 100644 hw/virtio/vhost-user-target.c
 create mode 100644 hw/virtio/virtio-config.c
 create mode 100644 hw/virtio/virtio-qmp.c
 create mode 100644 hw/virtio/virtio-qmp.h

-- 
2.38.1



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

* [PATCH-for-8.0 01/10] hw/virtio: Add missing "hw/core/cpu.h" include
  2022-12-12 23:05 [RFC PATCH-for-8.0 00/10] hw/virtio: Build most objects as target independent units Philippe Mathieu-Daudé
@ 2022-12-12 23:05 ` Philippe Mathieu-Daudé
  2022-12-12 23:05 ` [PATCH-for-8.0 02/10] hw/virtio: Rename virtio_ss[] -> specific_virtio_ss[] Philippe Mathieu-Daudé
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-12 23:05 UTC (permalink / raw)
  To: qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz,
	Philippe Mathieu-Daudé

virtio.c uses target_words_bigendian() which is declared in
"hw/core/cpu.h". Add the missing header to avoid when refactoring:

  hw/virtio/virtio.c:2451:9: error: implicit declaration of function 'target_words_bigendian' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
    if (target_words_bigendian()) {
        ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/virtio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index eb6347ab5d..5817f4cbc9 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -25,6 +25,7 @@
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
 #include "qom/object_interfaces.h"
+#include "hw/core/cpu.h"
 #include "hw/virtio/virtio.h"
 #include "migration/qemu-file-types.h"
 #include "qemu/atomic.h"
-- 
2.38.1



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

* [PATCH-for-8.0 02/10] hw/virtio: Rename virtio_ss[] -> specific_virtio_ss[]
  2022-12-12 23:05 [RFC PATCH-for-8.0 00/10] hw/virtio: Build most objects as target independent units Philippe Mathieu-Daudé
  2022-12-12 23:05 ` [PATCH-for-8.0 01/10] hw/virtio: Add missing "hw/core/cpu.h" include Philippe Mathieu-Daudé
@ 2022-12-12 23:05 ` Philippe Mathieu-Daudé
  2022-12-12 23:05 ` [PATCH-for-8.0 03/10] hw/virtio: Constify qmp_virtio_feature_map_t[] Philippe Mathieu-Daudé
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-12 23:05 UTC (permalink / raw)
  To: qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz,
	Philippe Mathieu-Daudé

Since virtio_ss[] is added to specific_ss[], rename it as
specific_virtio_ss[] to make it clearer.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/meson.build | 41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index dfed1e7af5..23a980efaa 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -3,34 +3,34 @@ softmmu_virtio_ss.add(files('virtio-bus.c'))
 softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: files('virtio-pci.c'))
 softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_MMIO', if_true: files('virtio-mmio.c'))
 
-virtio_ss = ss.source_set()
-virtio_ss.add(files('virtio.c'))
+specific_virtio_ss = ss.source_set()
+specific_virtio_ss.add(files('virtio.c'))
 
 if have_vhost
-  virtio_ss.add(files('vhost.c', 'vhost-backend.c', 'vhost-iova-tree.c'))
+  specific_virtio_ss.add(files('vhost.c', 'vhost-backend.c', 'vhost-iova-tree.c'))
   if have_vhost_user
-    virtio_ss.add(files('vhost-user.c'))
+    specific_virtio_ss.add(files('vhost-user.c'))
   endif
   if have_vhost_vdpa
-    virtio_ss.add(files('vhost-vdpa.c', 'vhost-shadow-virtqueue.c'))
+    specific_virtio_ss.add(files('vhost-vdpa.c', 'vhost-shadow-virtqueue.c'))
   endif
 else
   softmmu_virtio_ss.add(files('vhost-stub.c'))
 endif
 
-virtio_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: files('virtio-balloon.c'))
-virtio_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('virtio-crypto.c'))
-virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: files('vhost-user-fs.c'))
-virtio_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: files('virtio-pmem.c'))
-virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock.c', 'vhost-vsock-common.c'))
-virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock.c', 'vhost-vsock-common.c'))
-virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
-virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu.c'))
-virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
-virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: files('vhost-user-i2c.c'))
-virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: files('vhost-user-rng.c'))
-virtio_ss.add(when: 'CONFIG_VHOST_USER_GPIO', if_true: files('vhost-user-gpio.c'))
-virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_GPIO'], if_true: files('vhost-user-gpio-pci.c'))
+specific_virtio_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: files('virtio-balloon.c'))
+specific_virtio_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('virtio-crypto.c'))
+specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: files('vhost-user-fs.c'))
+specific_virtio_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: files('virtio-pmem.c'))
+specific_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock.c', 'vhost-vsock-common.c'))
+specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock.c', 'vhost-vsock-common.c'))
+specific_virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
+specific_virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu.c'))
+specific_virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
+specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: files('vhost-user-i2c.c'))
+specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: files('vhost-user-rng.c'))
+specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_GPIO', if_true: files('vhost-user-gpio.c'))
+specific_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_GPIO'], if_true: files('vhost-user-gpio-pci.c'))
 
 virtio_pci_ss = ss.source_set()
 virtio_pci_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock-pci.c'))
@@ -57,11 +57,12 @@ virtio_pci_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: files('virtio-pmem-pci.c'
 virtio_pci_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem-pci.c'))
 
-virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
+specific_virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
 
-specific_ss.add_all(when: 'CONFIG_VIRTIO', if_true: virtio_ss)
 softmmu_ss.add_all(when: 'CONFIG_VIRTIO', if_true: softmmu_virtio_ss)
 softmmu_ss.add(when: 'CONFIG_VIRTIO', if_false: files('vhost-stub.c'))
 softmmu_ss.add(when: 'CONFIG_VIRTIO', if_false: files('virtio-stub.c'))
 softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('vhost-stub.c'))
 softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('virtio-stub.c'))
+
+specific_ss.add_all(when: 'CONFIG_VIRTIO', if_true: specific_virtio_ss)
-- 
2.38.1



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

* [PATCH-for-8.0 03/10] hw/virtio: Constify qmp_virtio_feature_map_t[]
  2022-12-12 23:05 [RFC PATCH-for-8.0 00/10] hw/virtio: Build most objects as target independent units Philippe Mathieu-Daudé
  2022-12-12 23:05 ` [PATCH-for-8.0 01/10] hw/virtio: Add missing "hw/core/cpu.h" include Philippe Mathieu-Daudé
  2022-12-12 23:05 ` [PATCH-for-8.0 02/10] hw/virtio: Rename virtio_ss[] -> specific_virtio_ss[] Philippe Mathieu-Daudé
@ 2022-12-12 23:05 ` Philippe Mathieu-Daudé
  2022-12-13  0:02   ` Richard Henderson
  2022-12-12 23:05 ` [PATCH-for-8.0 04/10] hw/virtio: Extract config read/write accessors to virtio-config.c Philippe Mathieu-Daudé
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-12 23:05 UTC (permalink / raw)
  To: qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz,
	Philippe Mathieu-Daudé

These arrays are only accessed read-only, move them to .rodata.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/virtio.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 5817f4cbc9..f54cc23304 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -80,7 +80,7 @@ enum VhostUserProtocolFeature {
 };
 
 /* Virtio transport features mapping */
-static qmp_virtio_feature_map_t virtio_transport_map[] = {
+static const qmp_virtio_feature_map_t virtio_transport_map[] = {
     /* Virtio device transport features */
 #ifndef VIRTIO_CONFIG_NO_LEGACY
     FEATURE_ENTRY(VIRTIO_F_NOTIFY_ON_EMPTY, \
@@ -111,7 +111,7 @@ static qmp_virtio_feature_map_t virtio_transport_map[] = {
 };
 
 /* Vhost-user protocol features mapping */
-static qmp_virtio_feature_map_t vhost_user_protocol_map[] = {
+static const qmp_virtio_feature_map_t vhost_user_protocol_map[] = {
     FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_MQ, \
             "VHOST_USER_PROTOCOL_F_MQ: Multiqueue protocol supported"),
     FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_LOG_SHMFD, \
@@ -161,7 +161,7 @@ static qmp_virtio_feature_map_t vhost_user_protocol_map[] = {
 };
 
 /* virtio device configuration statuses */
-static qmp_virtio_feature_map_t virtio_config_status_map[] = {
+static const qmp_virtio_feature_map_t virtio_config_status_map[] = {
     FEATURE_ENTRY(VIRTIO_CONFIG_S_DRIVER_OK, \
             "VIRTIO_CONFIG_S_DRIVER_OK: Driver setup and ready"),
     FEATURE_ENTRY(VIRTIO_CONFIG_S_FEATURES_OK, \
@@ -179,7 +179,7 @@ static qmp_virtio_feature_map_t virtio_config_status_map[] = {
 };
 
 /* virtio-blk features mapping */
-qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
     FEATURE_ENTRY(VIRTIO_BLK_F_SIZE_MAX, \
             "VIRTIO_BLK_F_SIZE_MAX: Max segment size is size_max"),
     FEATURE_ENTRY(VIRTIO_BLK_F_SEG_MAX, \
@@ -218,7 +218,7 @@ qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
 };
 
 /* virtio-serial features mapping */
-qmp_virtio_feature_map_t virtio_serial_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_serial_feature_map[] = {
     FEATURE_ENTRY(VIRTIO_CONSOLE_F_SIZE, \
             "VIRTIO_CONSOLE_F_SIZE: Host providing console size"),
     FEATURE_ENTRY(VIRTIO_CONSOLE_F_MULTIPORT, \
@@ -229,7 +229,7 @@ qmp_virtio_feature_map_t virtio_serial_feature_map[] = {
 };
 
 /* virtio-gpu features mapping */
-qmp_virtio_feature_map_t virtio_gpu_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_gpu_feature_map[] = {
     FEATURE_ENTRY(VIRTIO_GPU_F_VIRGL, \
             "VIRTIO_GPU_F_VIRGL: Virgl 3D mode supported"),
     FEATURE_ENTRY(VIRTIO_GPU_F_EDID, \
@@ -250,7 +250,7 @@ qmp_virtio_feature_map_t virtio_gpu_feature_map[] = {
 };
 
 /* virtio-input features mapping */
-qmp_virtio_feature_map_t virtio_input_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_input_feature_map[] = {
     FEATURE_ENTRY(VHOST_F_LOG_ALL, \
             "VHOST_F_LOG_ALL: Logging write descriptors supported"),
     FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
@@ -260,7 +260,7 @@ qmp_virtio_feature_map_t virtio_input_feature_map[] = {
 };
 
 /* virtio-net features mapping */
-qmp_virtio_feature_map_t virtio_net_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_net_feature_map[] = {
     FEATURE_ENTRY(VIRTIO_NET_F_CSUM, \
             "VIRTIO_NET_F_CSUM: Device handling packets with partial checksum "
             "supported"),
@@ -338,7 +338,7 @@ qmp_virtio_feature_map_t virtio_net_feature_map[] = {
 };
 
 /* virtio-scsi features mapping */
-qmp_virtio_feature_map_t virtio_scsi_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_scsi_feature_map[] = {
     FEATURE_ENTRY(VIRTIO_SCSI_F_INOUT, \
             "VIRTIO_SCSI_F_INOUT: Requests including read and writable data "
             "buffers suppoted"),
@@ -359,7 +359,7 @@ qmp_virtio_feature_map_t virtio_scsi_feature_map[] = {
 };
 
 /* virtio/vhost-user-fs features mapping */
-qmp_virtio_feature_map_t virtio_fs_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_fs_feature_map[] = {
     FEATURE_ENTRY(VHOST_F_LOG_ALL, \
             "VHOST_F_LOG_ALL: Logging write descriptors supported"),
     FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
@@ -369,7 +369,7 @@ qmp_virtio_feature_map_t virtio_fs_feature_map[] = {
 };
 
 /* virtio/vhost-user-i2c features mapping */
-qmp_virtio_feature_map_t virtio_i2c_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_i2c_feature_map[] = {
     FEATURE_ENTRY(VIRTIO_I2C_F_ZERO_LENGTH_REQUEST, \
             "VIRTIO_I2C_F_ZERO_LEGNTH_REQUEST: Zero length requests supported"),
     FEATURE_ENTRY(VHOST_F_LOG_ALL, \
@@ -381,7 +381,7 @@ qmp_virtio_feature_map_t virtio_i2c_feature_map[] = {
 };
 
 /* virtio/vhost-vsock features mapping */
-qmp_virtio_feature_map_t virtio_vsock_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_vsock_feature_map[] = {
     FEATURE_ENTRY(VIRTIO_VSOCK_F_SEQPACKET, \
             "VIRTIO_VSOCK_F_SEQPACKET: SOCK_SEQPACKET supported"),
     FEATURE_ENTRY(VHOST_F_LOG_ALL, \
@@ -393,7 +393,7 @@ qmp_virtio_feature_map_t virtio_vsock_feature_map[] = {
 };
 
 /* virtio-balloon features mapping */
-qmp_virtio_feature_map_t virtio_balloon_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_balloon_feature_map[] = {
     FEATURE_ENTRY(VIRTIO_BALLOON_F_MUST_TELL_HOST, \
             "VIRTIO_BALLOON_F_MUST_TELL_HOST: Tell host before reclaiming "
             "pages"),
@@ -411,14 +411,14 @@ qmp_virtio_feature_map_t virtio_balloon_feature_map[] = {
 };
 
 /* virtio-crypto features mapping */
-qmp_virtio_feature_map_t virtio_crypto_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_crypto_feature_map[] = {
     FEATURE_ENTRY(VHOST_F_LOG_ALL, \
             "VHOST_F_LOG_ALL: Logging write descriptors supported"),
     { -1, "" }
 };
 
 /* virtio-iommu features mapping */
-qmp_virtio_feature_map_t virtio_iommu_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_iommu_feature_map[] = {
     FEATURE_ENTRY(VIRTIO_IOMMU_F_INPUT_RANGE, \
             "VIRTIO_IOMMU_F_INPUT_RANGE: Range of available virtual addrs. "
             "available"),
@@ -441,7 +441,7 @@ qmp_virtio_feature_map_t virtio_iommu_feature_map[] = {
 };
 
 /* virtio-mem features mapping */
-qmp_virtio_feature_map_t virtio_mem_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_mem_feature_map[] = {
 #ifndef CONFIG_ACPI
     FEATURE_ENTRY(VIRTIO_MEM_F_ACPI_PXM, \
             "VIRTIO_MEM_F_ACPI_PXM: node_id is an ACPI PXM and is valid"),
@@ -453,7 +453,7 @@ qmp_virtio_feature_map_t virtio_mem_feature_map[] = {
 };
 
 /* virtio-rng features mapping */
-qmp_virtio_feature_map_t virtio_rng_feature_map[] = {
+const qmp_virtio_feature_map_t virtio_rng_feature_map[] = {
     FEATURE_ENTRY(VHOST_F_LOG_ALL, \
             "VHOST_F_LOG_ALL: Logging write descriptors supported"),
     FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
-- 
2.38.1



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

* [PATCH-for-8.0 04/10] hw/virtio: Extract config read/write accessors to virtio-config.c
  2022-12-12 23:05 [RFC PATCH-for-8.0 00/10] hw/virtio: Build most objects as target independent units Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2022-12-12 23:05 ` [PATCH-for-8.0 03/10] hw/virtio: Constify qmp_virtio_feature_map_t[] Philippe Mathieu-Daudé
@ 2022-12-12 23:05 ` Philippe Mathieu-Daudé
  2022-12-12 23:05 ` [PATCH-for-8.0 05/10] hw/virtio: Extract QMP related code virtio-qmp.c Philippe Mathieu-Daudé
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-12 23:05 UTC (permalink / raw)
  To: qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz,
	Philippe Mathieu-Daudé

These config helpers use the target-dependent LD/ST API.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/meson.build     |   1 +
 hw/virtio/virtio-config.c | 204 ++++++++++++++++++++++++++++++++++++++
 hw/virtio/virtio.c        | 190 -----------------------------------
 3 files changed, 205 insertions(+), 190 deletions(-)
 create mode 100644 hw/virtio/virtio-config.c

diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index 23a980efaa..097220b4b5 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -5,6 +5,7 @@ softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_MMIO', if_true: files('virtio-mmio.c'
 
 specific_virtio_ss = ss.source_set()
 specific_virtio_ss.add(files('virtio.c'))
+specific_virtio_ss.add(files('virtio-config.c'))
 
 if have_vhost
   specific_virtio_ss.add(files('vhost.c', 'vhost-backend.c', 'vhost-iova-tree.c'))
diff --git a/hw/virtio/virtio-config.c b/hw/virtio/virtio-config.c
new file mode 100644
index 0000000000..ad78e0b9bc
--- /dev/null
+++ b/hw/virtio/virtio-config.c
@@ -0,0 +1,204 @@
+/*
+ * Virtio Support
+ *
+ * Copyright IBM, Corp. 2007
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/virtio/virtio.h"
+#include "cpu.h"
+
+uint32_t virtio_config_readb(VirtIODevice *vdev, uint32_t addr)
+{
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+    uint8_t val;
+
+    if (addr + sizeof(val) > vdev->config_len) {
+        return (uint32_t)-1;
+    }
+
+    k->get_config(vdev, vdev->config);
+
+    val = ldub_p(vdev->config + addr);
+    return val;
+}
+
+uint32_t virtio_config_readw(VirtIODevice *vdev, uint32_t addr)
+{
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+    uint16_t val;
+
+    if (addr + sizeof(val) > vdev->config_len) {
+        return (uint32_t)-1;
+    }
+
+    k->get_config(vdev, vdev->config);
+
+    val = lduw_p(vdev->config + addr);
+    return val;
+}
+
+uint32_t virtio_config_readl(VirtIODevice *vdev, uint32_t addr)
+{
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+    uint32_t val;
+
+    if (addr + sizeof(val) > vdev->config_len) {
+        return (uint32_t)-1;
+    }
+
+    k->get_config(vdev, vdev->config);
+
+    val = ldl_p(vdev->config + addr);
+    return val;
+}
+
+void virtio_config_writeb(VirtIODevice *vdev, uint32_t addr, uint32_t data)
+{
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+    uint8_t val = data;
+
+    if (addr + sizeof(val) > vdev->config_len) {
+        return;
+    }
+
+    stb_p(vdev->config + addr, val);
+
+    if (k->set_config) {
+        k->set_config(vdev, vdev->config);
+    }
+}
+
+void virtio_config_writew(VirtIODevice *vdev, uint32_t addr, uint32_t data)
+{
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+    uint16_t val = data;
+
+    if (addr + sizeof(val) > vdev->config_len) {
+        return;
+    }
+
+    stw_p(vdev->config + addr, val);
+
+    if (k->set_config) {
+        k->set_config(vdev, vdev->config);
+    }
+}
+
+void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data)
+{
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+    uint32_t val = data;
+
+    if (addr + sizeof(val) > vdev->config_len) {
+        return;
+    }
+
+    stl_p(vdev->config + addr, val);
+
+    if (k->set_config) {
+        k->set_config(vdev, vdev->config);
+    }
+}
+
+uint32_t virtio_config_modern_readb(VirtIODevice *vdev, uint32_t addr)
+{
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+    uint8_t val;
+
+    if (addr + sizeof(val) > vdev->config_len) {
+        return (uint32_t)-1;
+    }
+
+    k->get_config(vdev, vdev->config);
+
+    val = ldub_p(vdev->config + addr);
+    return val;
+}
+
+uint32_t virtio_config_modern_readw(VirtIODevice *vdev, uint32_t addr)
+{
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+    uint16_t val;
+
+    if (addr + sizeof(val) > vdev->config_len) {
+        return (uint32_t)-1;
+    }
+
+    k->get_config(vdev, vdev->config);
+
+    val = lduw_le_p(vdev->config + addr);
+    return val;
+}
+
+uint32_t virtio_config_modern_readl(VirtIODevice *vdev, uint32_t addr)
+{
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+    uint32_t val;
+
+    if (addr + sizeof(val) > vdev->config_len) {
+        return (uint32_t)-1;
+    }
+
+    k->get_config(vdev, vdev->config);
+
+    val = ldl_le_p(vdev->config + addr);
+    return val;
+}
+
+void virtio_config_modern_writeb(VirtIODevice *vdev,
+                                 uint32_t addr, uint32_t data)
+{
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+    uint8_t val = data;
+
+    if (addr + sizeof(val) > vdev->config_len) {
+        return;
+    }
+
+    stb_p(vdev->config + addr, val);
+
+    if (k->set_config) {
+        k->set_config(vdev, vdev->config);
+    }
+}
+
+void virtio_config_modern_writew(VirtIODevice *vdev,
+                                 uint32_t addr, uint32_t data)
+{
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+    uint16_t val = data;
+
+    if (addr + sizeof(val) > vdev->config_len) {
+        return;
+    }
+
+    stw_le_p(vdev->config + addr, val);
+
+    if (k->set_config) {
+        k->set_config(vdev, vdev->config);
+    }
+}
+
+void virtio_config_modern_writel(VirtIODevice *vdev,
+                                 uint32_t addr, uint32_t data)
+{
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+    uint32_t val = data;
+
+    if (addr + sizeof(val) > vdev->config_len) {
+        return;
+    }
+
+    stl_le_p(vdev->config + addr, val);
+
+    if (k->set_config) {
+        k->set_config(vdev, vdev->config);
+    }
+}
+
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index f54cc23304..1f19c8e0f0 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -18,7 +18,6 @@
 #include "qapi/qapi-commands-qom.h"
 #include "qapi/qapi-visit-virtio.h"
 #include "qapi/qmp/qjson.h"
-#include "cpu.h"
 #include "trace.h"
 #include "qemu/error-report.h"
 #include "qemu/log.h"
@@ -2552,195 +2551,6 @@ void virtio_reset(void *opaque)
     }
 }
 
-uint32_t virtio_config_readb(VirtIODevice *vdev, uint32_t addr)
-{
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
-    uint8_t val;
-
-    if (addr + sizeof(val) > vdev->config_len) {
-        return (uint32_t)-1;
-    }
-
-    k->get_config(vdev, vdev->config);
-
-    val = ldub_p(vdev->config + addr);
-    return val;
-}
-
-uint32_t virtio_config_readw(VirtIODevice *vdev, uint32_t addr)
-{
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
-    uint16_t val;
-
-    if (addr + sizeof(val) > vdev->config_len) {
-        return (uint32_t)-1;
-    }
-
-    k->get_config(vdev, vdev->config);
-
-    val = lduw_p(vdev->config + addr);
-    return val;
-}
-
-uint32_t virtio_config_readl(VirtIODevice *vdev, uint32_t addr)
-{
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
-    uint32_t val;
-
-    if (addr + sizeof(val) > vdev->config_len) {
-        return (uint32_t)-1;
-    }
-
-    k->get_config(vdev, vdev->config);
-
-    val = ldl_p(vdev->config + addr);
-    return val;
-}
-
-void virtio_config_writeb(VirtIODevice *vdev, uint32_t addr, uint32_t data)
-{
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
-    uint8_t val = data;
-
-    if (addr + sizeof(val) > vdev->config_len) {
-        return;
-    }
-
-    stb_p(vdev->config + addr, val);
-
-    if (k->set_config) {
-        k->set_config(vdev, vdev->config);
-    }
-}
-
-void virtio_config_writew(VirtIODevice *vdev, uint32_t addr, uint32_t data)
-{
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
-    uint16_t val = data;
-
-    if (addr + sizeof(val) > vdev->config_len) {
-        return;
-    }
-
-    stw_p(vdev->config + addr, val);
-
-    if (k->set_config) {
-        k->set_config(vdev, vdev->config);
-    }
-}
-
-void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data)
-{
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
-    uint32_t val = data;
-
-    if (addr + sizeof(val) > vdev->config_len) {
-        return;
-    }
-
-    stl_p(vdev->config + addr, val);
-
-    if (k->set_config) {
-        k->set_config(vdev, vdev->config);
-    }
-}
-
-uint32_t virtio_config_modern_readb(VirtIODevice *vdev, uint32_t addr)
-{
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
-    uint8_t val;
-
-    if (addr + sizeof(val) > vdev->config_len) {
-        return (uint32_t)-1;
-    }
-
-    k->get_config(vdev, vdev->config);
-
-    val = ldub_p(vdev->config + addr);
-    return val;
-}
-
-uint32_t virtio_config_modern_readw(VirtIODevice *vdev, uint32_t addr)
-{
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
-    uint16_t val;
-
-    if (addr + sizeof(val) > vdev->config_len) {
-        return (uint32_t)-1;
-    }
-
-    k->get_config(vdev, vdev->config);
-
-    val = lduw_le_p(vdev->config + addr);
-    return val;
-}
-
-uint32_t virtio_config_modern_readl(VirtIODevice *vdev, uint32_t addr)
-{
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
-    uint32_t val;
-
-    if (addr + sizeof(val) > vdev->config_len) {
-        return (uint32_t)-1;
-    }
-
-    k->get_config(vdev, vdev->config);
-
-    val = ldl_le_p(vdev->config + addr);
-    return val;
-}
-
-void virtio_config_modern_writeb(VirtIODevice *vdev,
-                                 uint32_t addr, uint32_t data)
-{
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
-    uint8_t val = data;
-
-    if (addr + sizeof(val) > vdev->config_len) {
-        return;
-    }
-
-    stb_p(vdev->config + addr, val);
-
-    if (k->set_config) {
-        k->set_config(vdev, vdev->config);
-    }
-}
-
-void virtio_config_modern_writew(VirtIODevice *vdev,
-                                 uint32_t addr, uint32_t data)
-{
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
-    uint16_t val = data;
-
-    if (addr + sizeof(val) > vdev->config_len) {
-        return;
-    }
-
-    stw_le_p(vdev->config + addr, val);
-
-    if (k->set_config) {
-        k->set_config(vdev, vdev->config);
-    }
-}
-
-void virtio_config_modern_writel(VirtIODevice *vdev,
-                                 uint32_t addr, uint32_t data)
-{
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
-    uint32_t val = data;
-
-    if (addr + sizeof(val) > vdev->config_len) {
-        return;
-    }
-
-    stl_le_p(vdev->config + addr, val);
-
-    if (k->set_config) {
-        k->set_config(vdev, vdev->config);
-    }
-}
-
 void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr)
 {
     if (!vdev->vq[n].vring.num) {
-- 
2.38.1



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

* [PATCH-for-8.0 05/10] hw/virtio: Extract QMP related code virtio-qmp.c
  2022-12-12 23:05 [RFC PATCH-for-8.0 00/10] hw/virtio: Build most objects as target independent units Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2022-12-12 23:05 ` [PATCH-for-8.0 04/10] hw/virtio: Extract config read/write accessors to virtio-config.c Philippe Mathieu-Daudé
@ 2022-12-12 23:05 ` Philippe Mathieu-Daudé
  2022-12-12 23:05 ` [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-12 23:05 UTC (permalink / raw)
  To: qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz,
	Philippe Mathieu-Daudé

The monitor decoders are the only functions using the CONFIG_xxx
definitions declared in the target specific CONFIG_DEVICES header.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/meson.build  |   2 +-
 hw/virtio/virtio-qmp.c | 631 +++++++++++++++++++++++++++++++++++++++++
 hw/virtio/virtio-qmp.h |  20 ++
 hw/virtio/virtio.c     | 607 +--------------------------------------
 4 files changed, 654 insertions(+), 606 deletions(-)
 create mode 100644 hw/virtio/virtio-qmp.c
 create mode 100644 hw/virtio/virtio-qmp.h

diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index 097220b4b5..eb7ee8ea92 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -5,7 +5,7 @@ softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_MMIO', if_true: files('virtio-mmio.c'
 
 specific_virtio_ss = ss.source_set()
 specific_virtio_ss.add(files('virtio.c'))
-specific_virtio_ss.add(files('virtio-config.c'))
+specific_virtio_ss.add(files('virtio-config.c', 'virtio-qmp.c'))
 
 if have_vhost
   specific_virtio_ss.add(files('vhost.c', 'vhost-backend.c', 'vhost-iova-tree.c'))
diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
new file mode 100644
index 0000000000..63d03cc6db
--- /dev/null
+++ b/hw/virtio/virtio-qmp.c
@@ -0,0 +1,631 @@
+/*
+ * Virtio QMP helpers
+ *
+ * Copyright IBM, Corp. 2007
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/virtio/virtio.h"
+#include "virtio-qmp.h"
+
+#include "standard-headers/linux/virtio_ids.h"
+#include "standard-headers/linux/vhost_types.h"
+#include "standard-headers/linux/virtio_blk.h"
+#include "standard-headers/linux/virtio_console.h"
+#include "standard-headers/linux/virtio_gpu.h"
+#include "standard-headers/linux/virtio_net.h"
+#include "standard-headers/linux/virtio_scsi.h"
+#include "standard-headers/linux/virtio_i2c.h"
+#include "standard-headers/linux/virtio_balloon.h"
+#include "standard-headers/linux/virtio_iommu.h"
+#include "standard-headers/linux/virtio_mem.h"
+#include "standard-headers/linux/virtio_vsock.h"
+
+#include CONFIG_DEVICES
+
+#define FEATURE_ENTRY(name, desc) (qmp_virtio_feature_map_t) \
+    { .virtio_bit = name, .feature_desc = desc }
+
+enum VhostUserProtocolFeature {
+    VHOST_USER_PROTOCOL_F_MQ = 0,
+    VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
+    VHOST_USER_PROTOCOL_F_RARP = 2,
+    VHOST_USER_PROTOCOL_F_REPLY_ACK = 3,
+    VHOST_USER_PROTOCOL_F_NET_MTU = 4,
+    VHOST_USER_PROTOCOL_F_SLAVE_REQ = 5,
+    VHOST_USER_PROTOCOL_F_CROSS_ENDIAN = 6,
+    VHOST_USER_PROTOCOL_F_CRYPTO_SESSION = 7,
+    VHOST_USER_PROTOCOL_F_PAGEFAULT = 8,
+    VHOST_USER_PROTOCOL_F_CONFIG = 9,
+    VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD = 10,
+    VHOST_USER_PROTOCOL_F_HOST_NOTIFIER = 11,
+    VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD = 12,
+    VHOST_USER_PROTOCOL_F_RESET_DEVICE = 13,
+    VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS = 14,
+    VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS = 15,
+    VHOST_USER_PROTOCOL_F_MAX
+};
+
+/* Virtio transport features mapping */
+static const qmp_virtio_feature_map_t virtio_transport_map[] = {
+    /* Virtio device transport features */
+#ifndef VIRTIO_CONFIG_NO_LEGACY
+    FEATURE_ENTRY(VIRTIO_F_NOTIFY_ON_EMPTY, \
+            "VIRTIO_F_NOTIFY_ON_EMPTY: Notify when device runs out of avail. "
+            "descs. on VQ"),
+    FEATURE_ENTRY(VIRTIO_F_ANY_LAYOUT, \
+            "VIRTIO_F_ANY_LAYOUT: Device accepts arbitrary desc. layouts"),
+#endif /* !VIRTIO_CONFIG_NO_LEGACY */
+    FEATURE_ENTRY(VIRTIO_F_VERSION_1, \
+            "VIRTIO_F_VERSION_1: Device compliant for v1 spec (legacy)"),
+    FEATURE_ENTRY(VIRTIO_F_IOMMU_PLATFORM, \
+            "VIRTIO_F_IOMMU_PLATFORM: Device can be used on IOMMU platform"),
+    FEATURE_ENTRY(VIRTIO_F_RING_PACKED, \
+            "VIRTIO_F_RING_PACKED: Device supports packed VQ layout"),
+    FEATURE_ENTRY(VIRTIO_F_IN_ORDER, \
+            "VIRTIO_F_IN_ORDER: Device uses buffers in same order as made "
+            "available by driver"),
+    FEATURE_ENTRY(VIRTIO_F_ORDER_PLATFORM, \
+            "VIRTIO_F_ORDER_PLATFORM: Memory accesses ordered by platform"),
+    FEATURE_ENTRY(VIRTIO_F_SR_IOV, \
+            "VIRTIO_F_SR_IOV: Device supports single root I/O virtualization"),
+    /* Virtio ring transport features */
+    FEATURE_ENTRY(VIRTIO_RING_F_INDIRECT_DESC, \
+            "VIRTIO_RING_F_INDIRECT_DESC: Indirect descriptors supported"),
+    FEATURE_ENTRY(VIRTIO_RING_F_EVENT_IDX, \
+            "VIRTIO_RING_F_EVENT_IDX: Used & avail. event fields enabled"),
+    { -1, "" }
+};
+
+/* Vhost-user protocol features mapping */
+static const qmp_virtio_feature_map_t vhost_user_protocol_map[] = {
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_MQ, \
+            "VHOST_USER_PROTOCOL_F_MQ: Multiqueue protocol supported"),
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_LOG_SHMFD, \
+            "VHOST_USER_PROTOCOL_F_LOG_SHMFD: Shared log memory fd supported"),
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_RARP, \
+            "VHOST_USER_PROTOCOL_F_RARP: Vhost-user back-end RARP broadcasting "
+            "supported"),
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_REPLY_ACK, \
+            "VHOST_USER_PROTOCOL_F_REPLY_ACK: Requested operation status ack. "
+            "supported"),
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_NET_MTU, \
+            "VHOST_USER_PROTOCOL_F_NET_MTU: Expose host MTU to guest supported"),
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_SLAVE_REQ, \
+            "VHOST_USER_PROTOCOL_F_SLAVE_REQ: Socket fd for back-end initiated "
+            "requests supported"),
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_CROSS_ENDIAN, \
+            "VHOST_USER_PROTOCOL_F_CROSS_ENDIAN: Endianness of VQs for legacy "
+            "devices supported"),
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_CRYPTO_SESSION, \
+            "VHOST_USER_PROTOCOL_F_CRYPTO_SESSION: Session creation for crypto "
+            "operations supported"),
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_PAGEFAULT, \
+            "VHOST_USER_PROTOCOL_F_PAGEFAULT: Request servicing on userfaultfd "
+            "for accessed pages supported"),
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_CONFIG, \
+            "VHOST_USER_PROTOCOL_F_CONFIG: Vhost-user messaging for virtio "
+            "device configuration space supported"),
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD, \
+            "VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD: Slave fd communication "
+            "channel supported"),
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_HOST_NOTIFIER, \
+            "VHOST_USER_PROTOCOL_F_HOST_NOTIFIER: Host notifiers for specified "
+            "VQs supported"),
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD, \
+            "VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD: Shared inflight I/O buffers "
+            "supported"),
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_RESET_DEVICE, \
+            "VHOST_USER_PROTOCOL_F_RESET_DEVICE: Disabling all rings and "
+            "resetting internal device state supported"),
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS, \
+            "VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS: In-band messaging "
+            "supported"),
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS, \
+            "VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS: Configuration for "
+            "memory slots supported"),
+    { -1, "" }
+};
+
+/* virtio device configuration statuses */
+static const qmp_virtio_feature_map_t virtio_config_status_map[] = {
+    FEATURE_ENTRY(VIRTIO_CONFIG_S_DRIVER_OK, \
+            "VIRTIO_CONFIG_S_DRIVER_OK: Driver setup and ready"),
+    FEATURE_ENTRY(VIRTIO_CONFIG_S_FEATURES_OK, \
+            "VIRTIO_CONFIG_S_FEATURES_OK: Feature negotiation complete"),
+    FEATURE_ENTRY(VIRTIO_CONFIG_S_DRIVER, \
+            "VIRTIO_CONFIG_S_DRIVER: Guest OS compatible with device"),
+    FEATURE_ENTRY(VIRTIO_CONFIG_S_NEEDS_RESET, \
+            "VIRTIO_CONFIG_S_NEEDS_RESET: Irrecoverable error, device needs "
+            "reset"),
+    FEATURE_ENTRY(VIRTIO_CONFIG_S_FAILED, \
+            "VIRTIO_CONFIG_S_FAILED: Error in guest, device failed"),
+    FEATURE_ENTRY(VIRTIO_CONFIG_S_ACKNOWLEDGE, \
+            "VIRTIO_CONFIG_S_ACKNOWLEDGE: Valid virtio device found"),
+    { -1, "" }
+};
+
+/* virtio-blk features mapping */
+const qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
+    FEATURE_ENTRY(VIRTIO_BLK_F_SIZE_MAX, \
+            "VIRTIO_BLK_F_SIZE_MAX: Max segment size is size_max"),
+    FEATURE_ENTRY(VIRTIO_BLK_F_SEG_MAX, \
+            "VIRTIO_BLK_F_SEG_MAX: Max segments in a request is seg_max"),
+    FEATURE_ENTRY(VIRTIO_BLK_F_GEOMETRY, \
+            "VIRTIO_BLK_F_GEOMETRY: Legacy geometry available"),
+    FEATURE_ENTRY(VIRTIO_BLK_F_RO, \
+            "VIRTIO_BLK_F_RO: Device is read-only"),
+    FEATURE_ENTRY(VIRTIO_BLK_F_BLK_SIZE, \
+            "VIRTIO_BLK_F_BLK_SIZE: Block size of disk available"),
+    FEATURE_ENTRY(VIRTIO_BLK_F_TOPOLOGY, \
+            "VIRTIO_BLK_F_TOPOLOGY: Topology information available"),
+    FEATURE_ENTRY(VIRTIO_BLK_F_MQ, \
+            "VIRTIO_BLK_F_MQ: Multiqueue supported"),
+    FEATURE_ENTRY(VIRTIO_BLK_F_DISCARD, \
+            "VIRTIO_BLK_F_DISCARD: Discard command supported"),
+    FEATURE_ENTRY(VIRTIO_BLK_F_WRITE_ZEROES, \
+            "VIRTIO_BLK_F_WRITE_ZEROES: Write zeroes command supported"),
+#ifndef VIRTIO_BLK_NO_LEGACY
+    FEATURE_ENTRY(VIRTIO_BLK_F_BARRIER, \
+            "VIRTIO_BLK_F_BARRIER: Request barriers supported"),
+    FEATURE_ENTRY(VIRTIO_BLK_F_SCSI, \
+            "VIRTIO_BLK_F_SCSI: SCSI packet commands supported"),
+    FEATURE_ENTRY(VIRTIO_BLK_F_FLUSH, \
+            "VIRTIO_BLK_F_FLUSH: Flush command supported"),
+    FEATURE_ENTRY(VIRTIO_BLK_F_CONFIG_WCE, \
+            "VIRTIO_BLK_F_CONFIG_WCE: Cache writeback and writethrough modes "
+            "supported"),
+#endif /* !VIRTIO_BLK_NO_LEGACY */
+    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
+            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
+    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
+            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
+            "negotiation supported"),
+    { -1, "" }
+};
+
+/* virtio-serial features mapping */
+const qmp_virtio_feature_map_t virtio_serial_feature_map[] = {
+    FEATURE_ENTRY(VIRTIO_CONSOLE_F_SIZE, \
+            "VIRTIO_CONSOLE_F_SIZE: Host providing console size"),
+    FEATURE_ENTRY(VIRTIO_CONSOLE_F_MULTIPORT, \
+            "VIRTIO_CONSOLE_F_MULTIPORT: Multiple ports for device supported"),
+    FEATURE_ENTRY(VIRTIO_CONSOLE_F_EMERG_WRITE, \
+            "VIRTIO_CONSOLE_F_EMERG_WRITE: Emergency write supported"),
+    { -1, "" }
+};
+
+/* virtio-gpu features mapping */
+const qmp_virtio_feature_map_t virtio_gpu_feature_map[] = {
+    FEATURE_ENTRY(VIRTIO_GPU_F_VIRGL, \
+            "VIRTIO_GPU_F_VIRGL: Virgl 3D mode supported"),
+    FEATURE_ENTRY(VIRTIO_GPU_F_EDID, \
+            "VIRTIO_GPU_F_EDID: EDID metadata supported"),
+    FEATURE_ENTRY(VIRTIO_GPU_F_RESOURCE_UUID, \
+            "VIRTIO_GPU_F_RESOURCE_UUID: Resource UUID assigning supported"),
+    FEATURE_ENTRY(VIRTIO_GPU_F_RESOURCE_BLOB, \
+            "VIRTIO_GPU_F_RESOURCE_BLOB: Size-based blob resources supported"),
+    FEATURE_ENTRY(VIRTIO_GPU_F_CONTEXT_INIT, \
+            "VIRTIO_GPU_F_CONTEXT_INIT: Context types and synchronization "
+            "timelines supported"),
+    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
+            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
+    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
+            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
+            "negotiation supported"),
+    { -1, "" }
+};
+
+/* virtio-input features mapping */
+const qmp_virtio_feature_map_t virtio_input_feature_map[] = {
+    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
+            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
+    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
+            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
+            "negotiation supported"),
+    { -1, "" }
+};
+
+/* virtio-net features mapping */
+const qmp_virtio_feature_map_t virtio_net_feature_map[] = {
+    FEATURE_ENTRY(VIRTIO_NET_F_CSUM, \
+            "VIRTIO_NET_F_CSUM: Device handling packets with partial checksum "
+            "supported"),
+    FEATURE_ENTRY(VIRTIO_NET_F_GUEST_CSUM, \
+            "VIRTIO_NET_F_GUEST_CSUM: Driver handling packets with partial "
+            "checksum supported"),
+    FEATURE_ENTRY(VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
+            "VIRTIO_NET_F_CTRL_GUEST_OFFLOADS: Control channel offloading "
+            "reconfig. supported"),
+    FEATURE_ENTRY(VIRTIO_NET_F_MTU, \
+            "VIRTIO_NET_F_MTU: Device max MTU reporting supported"),
+    FEATURE_ENTRY(VIRTIO_NET_F_MAC, \
+            "VIRTIO_NET_F_MAC: Device has given MAC address"),
+    FEATURE_ENTRY(VIRTIO_NET_F_GUEST_TSO4, \
+            "VIRTIO_NET_F_GUEST_TSO4: Driver can receive TSOv4"),
+    FEATURE_ENTRY(VIRTIO_NET_F_GUEST_TSO6, \
+            "VIRTIO_NET_F_GUEST_TSO6: Driver can receive TSOv6"),
+    FEATURE_ENTRY(VIRTIO_NET_F_GUEST_ECN, \
+            "VIRTIO_NET_F_GUEST_ECN: Driver can receive TSO with ECN"),
+    FEATURE_ENTRY(VIRTIO_NET_F_GUEST_UFO, \
+            "VIRTIO_NET_F_GUEST_UFO: Driver can receive UFO"),
+    FEATURE_ENTRY(VIRTIO_NET_F_HOST_TSO4, \
+            "VIRTIO_NET_F_HOST_TSO4: Device can receive TSOv4"),
+    FEATURE_ENTRY(VIRTIO_NET_F_HOST_TSO6, \
+            "VIRTIO_NET_F_HOST_TSO6: Device can receive TSOv6"),
+    FEATURE_ENTRY(VIRTIO_NET_F_HOST_ECN, \
+            "VIRTIO_NET_F_HOST_ECN: Device can receive TSO with ECN"),
+    FEATURE_ENTRY(VIRTIO_NET_F_HOST_UFO, \
+            "VIRTIO_NET_F_HOST_UFO: Device can receive UFO"),
+    FEATURE_ENTRY(VIRTIO_NET_F_MRG_RXBUF, \
+            "VIRTIO_NET_F_MRG_RXBUF: Driver can merge receive buffers"),
+    FEATURE_ENTRY(VIRTIO_NET_F_STATUS, \
+            "VIRTIO_NET_F_STATUS: Configuration status field available"),
+    FEATURE_ENTRY(VIRTIO_NET_F_CTRL_VQ, \
+            "VIRTIO_NET_F_CTRL_VQ: Control channel available"),
+    FEATURE_ENTRY(VIRTIO_NET_F_CTRL_RX, \
+            "VIRTIO_NET_F_CTRL_RX: Control channel RX mode supported"),
+    FEATURE_ENTRY(VIRTIO_NET_F_CTRL_VLAN, \
+            "VIRTIO_NET_F_CTRL_VLAN: Control channel VLAN filtering supported"),
+    FEATURE_ENTRY(VIRTIO_NET_F_CTRL_RX_EXTRA, \
+            "VIRTIO_NET_F_CTRL_RX_EXTRA: Extra RX mode control supported"),
+    FEATURE_ENTRY(VIRTIO_NET_F_GUEST_ANNOUNCE, \
+            "VIRTIO_NET_F_GUEST_ANNOUNCE: Driver sending gratuitous packets "
+            "supported"),
+    FEATURE_ENTRY(VIRTIO_NET_F_MQ, \
+            "VIRTIO_NET_F_MQ: Multiqueue with automatic receive steering "
+            "supported"),
+    FEATURE_ENTRY(VIRTIO_NET_F_CTRL_MAC_ADDR, \
+            "VIRTIO_NET_F_CTRL_MAC_ADDR: MAC address set through control "
+            "channel"),
+    FEATURE_ENTRY(VIRTIO_NET_F_HASH_REPORT, \
+            "VIRTIO_NET_F_HASH_REPORT: Hash reporting supported"),
+    FEATURE_ENTRY(VIRTIO_NET_F_RSS, \
+            "VIRTIO_NET_F_RSS: RSS RX steering supported"),
+    FEATURE_ENTRY(VIRTIO_NET_F_RSC_EXT, \
+            "VIRTIO_NET_F_RSC_EXT: Extended coalescing info supported"),
+    FEATURE_ENTRY(VIRTIO_NET_F_STANDBY, \
+            "VIRTIO_NET_F_STANDBY: Device acting as standby for primary "
+            "device with same MAC addr. supported"),
+    FEATURE_ENTRY(VIRTIO_NET_F_SPEED_DUPLEX, \
+            "VIRTIO_NET_F_SPEED_DUPLEX: Device set linkspeed and duplex"),
+#ifndef VIRTIO_NET_NO_LEGACY
+    FEATURE_ENTRY(VIRTIO_NET_F_GSO, \
+            "VIRTIO_NET_F_GSO: Handling GSO-type packets supported"),
+#endif /* !VIRTIO_NET_NO_LEGACY */
+    FEATURE_ENTRY(VHOST_NET_F_VIRTIO_NET_HDR, \
+            "VHOST_NET_F_VIRTIO_NET_HDR: Virtio-net headers for RX and TX "
+            "packets supported"),
+    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
+            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
+    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
+            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
+            "negotiation supported"),
+    { -1, "" }
+};
+
+/* virtio-scsi features mapping */
+const qmp_virtio_feature_map_t virtio_scsi_feature_map[] = {
+    FEATURE_ENTRY(VIRTIO_SCSI_F_INOUT, \
+            "VIRTIO_SCSI_F_INOUT: Requests including read and writable data "
+            "buffers suppoted"),
+    FEATURE_ENTRY(VIRTIO_SCSI_F_HOTPLUG, \
+            "VIRTIO_SCSI_F_HOTPLUG: Reporting and handling hot-plug events "
+            "supported"),
+    FEATURE_ENTRY(VIRTIO_SCSI_F_CHANGE, \
+            "VIRTIO_SCSI_F_CHANGE: Reporting and handling LUN changes "
+            "supported"),
+    FEATURE_ENTRY(VIRTIO_SCSI_F_T10_PI, \
+            "VIRTIO_SCSI_F_T10_PI: T10 info included in request header"),
+    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
+            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
+    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
+            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
+            "negotiation supported"),
+    { -1, "" }
+};
+
+/* virtio/vhost-user-fs features mapping */
+const qmp_virtio_feature_map_t virtio_fs_feature_map[] = {
+    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
+            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
+    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
+            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
+            "negotiation supported"),
+    { -1, "" }
+};
+
+/* virtio/vhost-user-i2c features mapping */
+const qmp_virtio_feature_map_t virtio_i2c_feature_map[] = {
+    FEATURE_ENTRY(VIRTIO_I2C_F_ZERO_LENGTH_REQUEST, \
+            "VIRTIO_I2C_F_ZERO_LEGNTH_REQUEST: Zero length requests supported"),
+    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
+            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
+    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
+            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
+            "negotiation supported"),
+    { -1, "" }
+};
+
+/* virtio/vhost-vsock features mapping */
+const qmp_virtio_feature_map_t virtio_vsock_feature_map[] = {
+    FEATURE_ENTRY(VIRTIO_VSOCK_F_SEQPACKET, \
+            "VIRTIO_VSOCK_F_SEQPACKET: SOCK_SEQPACKET supported"),
+    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
+            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
+    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
+            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
+            "negotiation supported"),
+    { -1, "" }
+};
+
+/* virtio-balloon features mapping */
+const qmp_virtio_feature_map_t virtio_balloon_feature_map[] = {
+    FEATURE_ENTRY(VIRTIO_BALLOON_F_MUST_TELL_HOST, \
+            "VIRTIO_BALLOON_F_MUST_TELL_HOST: Tell host before reclaiming "
+            "pages"),
+    FEATURE_ENTRY(VIRTIO_BALLOON_F_STATS_VQ, \
+            "VIRTIO_BALLOON_F_STATS_VQ: Guest memory stats VQ available"),
+    FEATURE_ENTRY(VIRTIO_BALLOON_F_DEFLATE_ON_OOM, \
+            "VIRTIO_BALLOON_F_DEFLATE_ON_OOM: Deflate balloon when guest OOM"),
+    FEATURE_ENTRY(VIRTIO_BALLOON_F_FREE_PAGE_HINT, \
+            "VIRTIO_BALLOON_F_FREE_PAGE_HINT: VQ reporting free pages enabled"),
+    FEATURE_ENTRY(VIRTIO_BALLOON_F_PAGE_POISON, \
+            "VIRTIO_BALLOON_F_PAGE_POISON: Guest page poisoning enabled"),
+    FEATURE_ENTRY(VIRTIO_BALLOON_F_REPORTING, \
+            "VIRTIO_BALLOON_F_REPORTING: Page reporting VQ enabled"),
+    { -1, "" }
+};
+
+/* virtio-crypto features mapping */
+const qmp_virtio_feature_map_t virtio_crypto_feature_map[] = {
+    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
+            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
+    { -1, "" }
+};
+
+/* virtio-iommu features mapping */
+const qmp_virtio_feature_map_t virtio_iommu_feature_map[] = {
+    FEATURE_ENTRY(VIRTIO_IOMMU_F_INPUT_RANGE, \
+            "VIRTIO_IOMMU_F_INPUT_RANGE: Range of available virtual addrs. "
+            "available"),
+    FEATURE_ENTRY(VIRTIO_IOMMU_F_DOMAIN_RANGE, \
+            "VIRTIO_IOMMU_F_DOMAIN_RANGE: Number of supported domains "
+            "available"),
+    FEATURE_ENTRY(VIRTIO_IOMMU_F_MAP_UNMAP, \
+            "VIRTIO_IOMMU_F_MAP_UNMAP: Map and unmap requests available"),
+    FEATURE_ENTRY(VIRTIO_IOMMU_F_BYPASS, \
+            "VIRTIO_IOMMU_F_BYPASS: Endpoints not attached to domains are in "
+            "bypass mode"),
+    FEATURE_ENTRY(VIRTIO_IOMMU_F_PROBE, \
+            "VIRTIO_IOMMU_F_PROBE: Probe requests available"),
+    FEATURE_ENTRY(VIRTIO_IOMMU_F_MMIO, \
+            "VIRTIO_IOMMU_F_MMIO: VIRTIO_IOMMU_MAP_F_MMIO flag available"),
+    FEATURE_ENTRY(VIRTIO_IOMMU_F_BYPASS_CONFIG, \
+            "VIRTIO_IOMMU_F_BYPASS_CONFIG: Bypass field of IOMMU config "
+            "available"),
+    { -1, "" }
+};
+
+/* virtio-mem features mapping */
+const qmp_virtio_feature_map_t virtio_mem_feature_map[] = {
+#ifndef CONFIG_ACPI
+    FEATURE_ENTRY(VIRTIO_MEM_F_ACPI_PXM, \
+            "VIRTIO_MEM_F_ACPI_PXM: node_id is an ACPI PXM and is valid"),
+#endif /* !CONFIG_ACPI */
+    FEATURE_ENTRY(VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE, \
+            "VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE: Unplugged memory cannot be "
+            "accessed"),
+    { -1, "" }
+};
+
+/* virtio-rng features mapping */
+const qmp_virtio_feature_map_t virtio_rng_feature_map[] = {
+    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
+            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
+    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
+            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
+            "negotiation supported"),
+    { -1, "" }
+};
+
+#define CONVERT_FEATURES(type, map, is_status, bitmap)   \
+    ({                                                   \
+        type *list = NULL;                               \
+        type *node;                                      \
+        for (i = 0; map[i].virtio_bit != -1; i++) {      \
+            if (is_status) {                             \
+                bit = map[i].virtio_bit;                 \
+            }                                            \
+            else {                                       \
+                bit = 1ULL << map[i].virtio_bit;         \
+            }                                            \
+            if ((bitmap & bit) == 0) {                   \
+                continue;                                \
+            }                                            \
+            node = g_new0(type, 1);                      \
+            node->value = g_strdup(map[i].feature_desc); \
+            node->next = list;                           \
+            list = node;                                 \
+            bitmap ^= bit;                               \
+        }                                                \
+        list;                                            \
+    })
+
+VirtioDeviceStatus *qmp_decode_status(uint8_t bitmap)
+{
+    VirtioDeviceStatus *status;
+    uint8_t bit;
+    int i;
+
+    status = g_new0(VirtioDeviceStatus, 1);
+    status->statuses = CONVERT_FEATURES(strList, virtio_config_status_map,
+                                        1, bitmap);
+    status->has_unknown_statuses = bitmap != 0;
+    if (status->has_unknown_statuses) {
+        status->unknown_statuses = bitmap;
+    }
+
+    return status;
+}
+
+VhostDeviceProtocols *qmp_decode_protocols(uint64_t bitmap)
+{
+    VhostDeviceProtocols *vhu_protocols;
+    uint64_t bit;
+    int i;
+
+    vhu_protocols = g_new0(VhostDeviceProtocols, 1);
+    vhu_protocols->protocols =
+                    CONVERT_FEATURES(strList,
+                                     vhost_user_protocol_map, 0, bitmap);
+    vhu_protocols->has_unknown_protocols = bitmap != 0;
+    if (vhu_protocols->has_unknown_protocols) {
+        vhu_protocols->unknown_protocols = bitmap;
+    }
+
+    return vhu_protocols;
+}
+
+VirtioDeviceFeatures *qmp_decode_features(uint16_t device_id, uint64_t bitmap)
+{
+    VirtioDeviceFeatures *features;
+    uint64_t bit;
+    int i;
+
+    features = g_new0(VirtioDeviceFeatures, 1);
+    features->has_dev_features = true;
+
+    /* transport features */
+    features->transports = CONVERT_FEATURES(strList, virtio_transport_map, 0,
+                                            bitmap);
+
+    /* device features */
+    switch (device_id) {
+#ifdef CONFIG_VIRTIO_SERIAL
+    case VIRTIO_ID_CONSOLE:
+        features->dev_features =
+            CONVERT_FEATURES(strList, virtio_serial_feature_map, 0, bitmap);
+        break;
+#endif
+#ifdef CONFIG_VIRTIO_BLK
+    case VIRTIO_ID_BLOCK:
+        features->dev_features =
+            CONVERT_FEATURES(strList, virtio_blk_feature_map, 0, bitmap);
+        break;
+#endif
+#ifdef CONFIG_VIRTIO_GPU
+    case VIRTIO_ID_GPU:
+        features->dev_features =
+            CONVERT_FEATURES(strList, virtio_gpu_feature_map, 0, bitmap);
+        break;
+#endif
+#ifdef CONFIG_VIRTIO_NET
+    case VIRTIO_ID_NET:
+        features->dev_features =
+            CONVERT_FEATURES(strList, virtio_net_feature_map, 0, bitmap);
+        break;
+#endif
+#ifdef CONFIG_VIRTIO_SCSI
+    case VIRTIO_ID_SCSI:
+        features->dev_features =
+            CONVERT_FEATURES(strList, virtio_scsi_feature_map, 0, bitmap);
+        break;
+#endif
+#ifdef CONFIG_VIRTIO_BALLOON
+    case VIRTIO_ID_BALLOON:
+        features->dev_features =
+            CONVERT_FEATURES(strList, virtio_balloon_feature_map, 0, bitmap);
+        break;
+#endif
+#ifdef CONFIG_VIRTIO_IOMMU
+    case VIRTIO_ID_IOMMU:
+        features->dev_features =
+            CONVERT_FEATURES(strList, virtio_iommu_feature_map, 0, bitmap);
+        break;
+#endif
+#ifdef CONFIG_VIRTIO_INPUT
+    case VIRTIO_ID_INPUT:
+        features->dev_features =
+            CONVERT_FEATURES(strList, virtio_input_feature_map, 0, bitmap);
+        break;
+#endif
+#ifdef CONFIG_VHOST_USER_FS
+    case VIRTIO_ID_FS:
+        features->dev_features =
+            CONVERT_FEATURES(strList, virtio_fs_feature_map, 0, bitmap);
+        break;
+#endif
+#ifdef CONFIG_VHOST_VSOCK
+    case VIRTIO_ID_VSOCK:
+        features->dev_features =
+            CONVERT_FEATURES(strList, virtio_vsock_feature_map, 0, bitmap);
+        break;
+#endif
+#ifdef CONFIG_VIRTIO_CRYPTO
+    case VIRTIO_ID_CRYPTO:
+        features->dev_features =
+            CONVERT_FEATURES(strList, virtio_crypto_feature_map, 0, bitmap);
+        break;
+#endif
+#ifdef CONFIG_VIRTIO_MEM
+    case VIRTIO_ID_MEM:
+        features->dev_features =
+            CONVERT_FEATURES(strList, virtio_mem_feature_map, 0, bitmap);
+        break;
+#endif
+#ifdef CONFIG_VIRTIO_I2C_ADAPTER
+    case VIRTIO_ID_I2C_ADAPTER:
+        features->dev_features =
+            CONVERT_FEATURES(strList, virtio_i2c_feature_map, 0, bitmap);
+        break;
+#endif
+#ifdef CONFIG_VIRTIO_RNG
+    case VIRTIO_ID_RNG:
+        features->dev_features =
+            CONVERT_FEATURES(strList, virtio_rng_feature_map, 0, bitmap);
+        break;
+#endif
+    /* No features */
+    case VIRTIO_ID_9P:
+    case VIRTIO_ID_PMEM:
+    case VIRTIO_ID_IOMEM:
+    case VIRTIO_ID_RPMSG:
+    case VIRTIO_ID_CLOCK:
+    case VIRTIO_ID_MAC80211_WLAN:
+    case VIRTIO_ID_MAC80211_HWSIM:
+    case VIRTIO_ID_RPROC_SERIAL:
+    case VIRTIO_ID_MEMORY_BALLOON:
+    case VIRTIO_ID_CAIF:
+    case VIRTIO_ID_SIGNAL_DIST:
+    case VIRTIO_ID_PSTORE:
+    case VIRTIO_ID_SOUND:
+    case VIRTIO_ID_BT:
+    case VIRTIO_ID_RPMB:
+    case VIRTIO_ID_VIDEO_ENCODER:
+    case VIRTIO_ID_VIDEO_DECODER:
+    case VIRTIO_ID_SCMI:
+    case VIRTIO_ID_NITRO_SEC_MOD:
+    case VIRTIO_ID_WATCHDOG:
+    case VIRTIO_ID_CAN:
+    case VIRTIO_ID_DMABUF:
+    case VIRTIO_ID_PARAM_SERV:
+    case VIRTIO_ID_AUDIO_POLICY:
+    case VIRTIO_ID_GPIO:
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    features->has_unknown_dev_features = bitmap != 0;
+    if (features->has_unknown_dev_features) {
+        features->unknown_dev_features = bitmap;
+    }
+
+    return features;
+}
diff --git a/hw/virtio/virtio-qmp.h b/hw/virtio/virtio-qmp.h
new file mode 100644
index 0000000000..075fc27030
--- /dev/null
+++ b/hw/virtio/virtio-qmp.h
@@ -0,0 +1,20 @@
+/*
+ * Virtio QMP helpers
+ *
+ * Copyright IBM, Corp. 2007
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef HW_VIRTIO_QMP_H
+#define HW_VIRTIO_QMP_H
+
+#include "qapi/qapi-types-virtio.h"
+
+VirtioDeviceStatus *qmp_decode_status(uint8_t bitmap);
+VhostDeviceProtocols *qmp_decode_protocols(uint64_t bitmap);
+VirtioDeviceFeatures *qmp_decode_features(uint16_t device_id, uint64_t bitmap);
+
+#endif
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 1f19c8e0f0..09b1a0e3d9 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -16,7 +16,6 @@
 #include "qapi/qmp/qdict.h"
 #include "qapi/qapi-commands-virtio.h"
 #include "qapi/qapi-commands-qom.h"
-#include "qapi/qapi-visit-virtio.h"
 #include "qapi/qmp/qjson.h"
 #include "trace.h"
 #include "qemu/error-report.h"
@@ -33,6 +32,8 @@
 #include "hw/virtio/virtio-access.h"
 #include "sysemu/dma.h"
 #include "sysemu/runstate.h"
+#include "virtio-qmp.h"
+
 #include "standard-headers/linux/virtio_ids.h"
 #include "standard-headers/linux/vhost_types.h"
 #include "standard-headers/linux/virtio_blk.h"
@@ -45,7 +46,6 @@
 #include "standard-headers/linux/virtio_iommu.h"
 #include "standard-headers/linux/virtio_mem.h"
 #include "standard-headers/linux/virtio_vsock.h"
-#include CONFIG_DEVICES
 
 /* QAPI list of realized VirtIODevices */
 static QTAILQ_HEAD(, VirtIODevice) virtio_list;
@@ -55,412 +55,6 @@ static QTAILQ_HEAD(, VirtIODevice) virtio_list;
  */
 #define VHOST_USER_MAX_CONFIG_SIZE 256
 
-#define FEATURE_ENTRY(name, desc) (qmp_virtio_feature_map_t) \
-    { .virtio_bit = name, .feature_desc = desc }
-
-enum VhostUserProtocolFeature {
-    VHOST_USER_PROTOCOL_F_MQ = 0,
-    VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
-    VHOST_USER_PROTOCOL_F_RARP = 2,
-    VHOST_USER_PROTOCOL_F_REPLY_ACK = 3,
-    VHOST_USER_PROTOCOL_F_NET_MTU = 4,
-    VHOST_USER_PROTOCOL_F_SLAVE_REQ = 5,
-    VHOST_USER_PROTOCOL_F_CROSS_ENDIAN = 6,
-    VHOST_USER_PROTOCOL_F_CRYPTO_SESSION = 7,
-    VHOST_USER_PROTOCOL_F_PAGEFAULT = 8,
-    VHOST_USER_PROTOCOL_F_CONFIG = 9,
-    VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD = 10,
-    VHOST_USER_PROTOCOL_F_HOST_NOTIFIER = 11,
-    VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD = 12,
-    VHOST_USER_PROTOCOL_F_RESET_DEVICE = 13,
-    VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS = 14,
-    VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS = 15,
-    VHOST_USER_PROTOCOL_F_MAX
-};
-
-/* Virtio transport features mapping */
-static const qmp_virtio_feature_map_t virtio_transport_map[] = {
-    /* Virtio device transport features */
-#ifndef VIRTIO_CONFIG_NO_LEGACY
-    FEATURE_ENTRY(VIRTIO_F_NOTIFY_ON_EMPTY, \
-            "VIRTIO_F_NOTIFY_ON_EMPTY: Notify when device runs out of avail. "
-            "descs. on VQ"),
-    FEATURE_ENTRY(VIRTIO_F_ANY_LAYOUT, \
-            "VIRTIO_F_ANY_LAYOUT: Device accepts arbitrary desc. layouts"),
-#endif /* !VIRTIO_CONFIG_NO_LEGACY */
-    FEATURE_ENTRY(VIRTIO_F_VERSION_1, \
-            "VIRTIO_F_VERSION_1: Device compliant for v1 spec (legacy)"),
-    FEATURE_ENTRY(VIRTIO_F_IOMMU_PLATFORM, \
-            "VIRTIO_F_IOMMU_PLATFORM: Device can be used on IOMMU platform"),
-    FEATURE_ENTRY(VIRTIO_F_RING_PACKED, \
-            "VIRTIO_F_RING_PACKED: Device supports packed VQ layout"),
-    FEATURE_ENTRY(VIRTIO_F_IN_ORDER, \
-            "VIRTIO_F_IN_ORDER: Device uses buffers in same order as made "
-            "available by driver"),
-    FEATURE_ENTRY(VIRTIO_F_ORDER_PLATFORM, \
-            "VIRTIO_F_ORDER_PLATFORM: Memory accesses ordered by platform"),
-    FEATURE_ENTRY(VIRTIO_F_SR_IOV, \
-            "VIRTIO_F_SR_IOV: Device supports single root I/O virtualization"),
-    /* Virtio ring transport features */
-    FEATURE_ENTRY(VIRTIO_RING_F_INDIRECT_DESC, \
-            "VIRTIO_RING_F_INDIRECT_DESC: Indirect descriptors supported"),
-    FEATURE_ENTRY(VIRTIO_RING_F_EVENT_IDX, \
-            "VIRTIO_RING_F_EVENT_IDX: Used & avail. event fields enabled"),
-    { -1, "" }
-};
-
-/* Vhost-user protocol features mapping */
-static const qmp_virtio_feature_map_t vhost_user_protocol_map[] = {
-    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_MQ, \
-            "VHOST_USER_PROTOCOL_F_MQ: Multiqueue protocol supported"),
-    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_LOG_SHMFD, \
-            "VHOST_USER_PROTOCOL_F_LOG_SHMFD: Shared log memory fd supported"),
-    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_RARP, \
-            "VHOST_USER_PROTOCOL_F_RARP: Vhost-user back-end RARP broadcasting "
-            "supported"),
-    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_REPLY_ACK, \
-            "VHOST_USER_PROTOCOL_F_REPLY_ACK: Requested operation status ack. "
-            "supported"),
-    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_NET_MTU, \
-            "VHOST_USER_PROTOCOL_F_NET_MTU: Expose host MTU to guest supported"),
-    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_SLAVE_REQ, \
-            "VHOST_USER_PROTOCOL_F_SLAVE_REQ: Socket fd for back-end initiated "
-            "requests supported"),
-    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_CROSS_ENDIAN, \
-            "VHOST_USER_PROTOCOL_F_CROSS_ENDIAN: Endianness of VQs for legacy "
-            "devices supported"),
-    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_CRYPTO_SESSION, \
-            "VHOST_USER_PROTOCOL_F_CRYPTO_SESSION: Session creation for crypto "
-            "operations supported"),
-    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_PAGEFAULT, \
-            "VHOST_USER_PROTOCOL_F_PAGEFAULT: Request servicing on userfaultfd "
-            "for accessed pages supported"),
-    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_CONFIG, \
-            "VHOST_USER_PROTOCOL_F_CONFIG: Vhost-user messaging for virtio "
-            "device configuration space supported"),
-    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD, \
-            "VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD: Slave fd communication "
-            "channel supported"),
-    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_HOST_NOTIFIER, \
-            "VHOST_USER_PROTOCOL_F_HOST_NOTIFIER: Host notifiers for specified "
-            "VQs supported"),
-    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD, \
-            "VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD: Shared inflight I/O buffers "
-            "supported"),
-    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_RESET_DEVICE, \
-            "VHOST_USER_PROTOCOL_F_RESET_DEVICE: Disabling all rings and "
-            "resetting internal device state supported"),
-    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS, \
-            "VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS: In-band messaging "
-            "supported"),
-    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS, \
-            "VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS: Configuration for "
-            "memory slots supported"),
-    { -1, "" }
-};
-
-/* virtio device configuration statuses */
-static const qmp_virtio_feature_map_t virtio_config_status_map[] = {
-    FEATURE_ENTRY(VIRTIO_CONFIG_S_DRIVER_OK, \
-            "VIRTIO_CONFIG_S_DRIVER_OK: Driver setup and ready"),
-    FEATURE_ENTRY(VIRTIO_CONFIG_S_FEATURES_OK, \
-            "VIRTIO_CONFIG_S_FEATURES_OK: Feature negotiation complete"),
-    FEATURE_ENTRY(VIRTIO_CONFIG_S_DRIVER, \
-            "VIRTIO_CONFIG_S_DRIVER: Guest OS compatible with device"),
-    FEATURE_ENTRY(VIRTIO_CONFIG_S_NEEDS_RESET, \
-            "VIRTIO_CONFIG_S_NEEDS_RESET: Irrecoverable error, device needs "
-            "reset"),
-    FEATURE_ENTRY(VIRTIO_CONFIG_S_FAILED, \
-            "VIRTIO_CONFIG_S_FAILED: Error in guest, device failed"),
-    FEATURE_ENTRY(VIRTIO_CONFIG_S_ACKNOWLEDGE, \
-            "VIRTIO_CONFIG_S_ACKNOWLEDGE: Valid virtio device found"),
-    { -1, "" }
-};
-
-/* virtio-blk features mapping */
-const qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
-    FEATURE_ENTRY(VIRTIO_BLK_F_SIZE_MAX, \
-            "VIRTIO_BLK_F_SIZE_MAX: Max segment size is size_max"),
-    FEATURE_ENTRY(VIRTIO_BLK_F_SEG_MAX, \
-            "VIRTIO_BLK_F_SEG_MAX: Max segments in a request is seg_max"),
-    FEATURE_ENTRY(VIRTIO_BLK_F_GEOMETRY, \
-            "VIRTIO_BLK_F_GEOMETRY: Legacy geometry available"),
-    FEATURE_ENTRY(VIRTIO_BLK_F_RO, \
-            "VIRTIO_BLK_F_RO: Device is read-only"),
-    FEATURE_ENTRY(VIRTIO_BLK_F_BLK_SIZE, \
-            "VIRTIO_BLK_F_BLK_SIZE: Block size of disk available"),
-    FEATURE_ENTRY(VIRTIO_BLK_F_TOPOLOGY, \
-            "VIRTIO_BLK_F_TOPOLOGY: Topology information available"),
-    FEATURE_ENTRY(VIRTIO_BLK_F_MQ, \
-            "VIRTIO_BLK_F_MQ: Multiqueue supported"),
-    FEATURE_ENTRY(VIRTIO_BLK_F_DISCARD, \
-            "VIRTIO_BLK_F_DISCARD: Discard command supported"),
-    FEATURE_ENTRY(VIRTIO_BLK_F_WRITE_ZEROES, \
-            "VIRTIO_BLK_F_WRITE_ZEROES: Write zeroes command supported"),
-#ifndef VIRTIO_BLK_NO_LEGACY
-    FEATURE_ENTRY(VIRTIO_BLK_F_BARRIER, \
-            "VIRTIO_BLK_F_BARRIER: Request barriers supported"),
-    FEATURE_ENTRY(VIRTIO_BLK_F_SCSI, \
-            "VIRTIO_BLK_F_SCSI: SCSI packet commands supported"),
-    FEATURE_ENTRY(VIRTIO_BLK_F_FLUSH, \
-            "VIRTIO_BLK_F_FLUSH: Flush command supported"),
-    FEATURE_ENTRY(VIRTIO_BLK_F_CONFIG_WCE, \
-            "VIRTIO_BLK_F_CONFIG_WCE: Cache writeback and writethrough modes "
-            "supported"),
-#endif /* !VIRTIO_BLK_NO_LEGACY */
-    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
-            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
-    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
-            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
-            "negotiation supported"),
-    { -1, "" }
-};
-
-/* virtio-serial features mapping */
-const qmp_virtio_feature_map_t virtio_serial_feature_map[] = {
-    FEATURE_ENTRY(VIRTIO_CONSOLE_F_SIZE, \
-            "VIRTIO_CONSOLE_F_SIZE: Host providing console size"),
-    FEATURE_ENTRY(VIRTIO_CONSOLE_F_MULTIPORT, \
-            "VIRTIO_CONSOLE_F_MULTIPORT: Multiple ports for device supported"),
-    FEATURE_ENTRY(VIRTIO_CONSOLE_F_EMERG_WRITE, \
-            "VIRTIO_CONSOLE_F_EMERG_WRITE: Emergency write supported"),
-    { -1, "" }
-};
-
-/* virtio-gpu features mapping */
-const qmp_virtio_feature_map_t virtio_gpu_feature_map[] = {
-    FEATURE_ENTRY(VIRTIO_GPU_F_VIRGL, \
-            "VIRTIO_GPU_F_VIRGL: Virgl 3D mode supported"),
-    FEATURE_ENTRY(VIRTIO_GPU_F_EDID, \
-            "VIRTIO_GPU_F_EDID: EDID metadata supported"),
-    FEATURE_ENTRY(VIRTIO_GPU_F_RESOURCE_UUID, \
-            "VIRTIO_GPU_F_RESOURCE_UUID: Resource UUID assigning supported"),
-    FEATURE_ENTRY(VIRTIO_GPU_F_RESOURCE_BLOB, \
-            "VIRTIO_GPU_F_RESOURCE_BLOB: Size-based blob resources supported"),
-    FEATURE_ENTRY(VIRTIO_GPU_F_CONTEXT_INIT, \
-            "VIRTIO_GPU_F_CONTEXT_INIT: Context types and synchronization "
-            "timelines supported"),
-    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
-            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
-    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
-            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
-            "negotiation supported"),
-    { -1, "" }
-};
-
-/* virtio-input features mapping */
-const qmp_virtio_feature_map_t virtio_input_feature_map[] = {
-    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
-            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
-    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
-            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
-            "negotiation supported"),
-    { -1, "" }
-};
-
-/* virtio-net features mapping */
-const qmp_virtio_feature_map_t virtio_net_feature_map[] = {
-    FEATURE_ENTRY(VIRTIO_NET_F_CSUM, \
-            "VIRTIO_NET_F_CSUM: Device handling packets with partial checksum "
-            "supported"),
-    FEATURE_ENTRY(VIRTIO_NET_F_GUEST_CSUM, \
-            "VIRTIO_NET_F_GUEST_CSUM: Driver handling packets with partial "
-            "checksum supported"),
-    FEATURE_ENTRY(VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
-            "VIRTIO_NET_F_CTRL_GUEST_OFFLOADS: Control channel offloading "
-            "reconfig. supported"),
-    FEATURE_ENTRY(VIRTIO_NET_F_MTU, \
-            "VIRTIO_NET_F_MTU: Device max MTU reporting supported"),
-    FEATURE_ENTRY(VIRTIO_NET_F_MAC, \
-            "VIRTIO_NET_F_MAC: Device has given MAC address"),
-    FEATURE_ENTRY(VIRTIO_NET_F_GUEST_TSO4, \
-            "VIRTIO_NET_F_GUEST_TSO4: Driver can receive TSOv4"),
-    FEATURE_ENTRY(VIRTIO_NET_F_GUEST_TSO6, \
-            "VIRTIO_NET_F_GUEST_TSO6: Driver can receive TSOv6"),
-    FEATURE_ENTRY(VIRTIO_NET_F_GUEST_ECN, \
-            "VIRTIO_NET_F_GUEST_ECN: Driver can receive TSO with ECN"),
-    FEATURE_ENTRY(VIRTIO_NET_F_GUEST_UFO, \
-            "VIRTIO_NET_F_GUEST_UFO: Driver can receive UFO"),
-    FEATURE_ENTRY(VIRTIO_NET_F_HOST_TSO4, \
-            "VIRTIO_NET_F_HOST_TSO4: Device can receive TSOv4"),
-    FEATURE_ENTRY(VIRTIO_NET_F_HOST_TSO6, \
-            "VIRTIO_NET_F_HOST_TSO6: Device can receive TSOv6"),
-    FEATURE_ENTRY(VIRTIO_NET_F_HOST_ECN, \
-            "VIRTIO_NET_F_HOST_ECN: Device can receive TSO with ECN"),
-    FEATURE_ENTRY(VIRTIO_NET_F_HOST_UFO, \
-            "VIRTIO_NET_F_HOST_UFO: Device can receive UFO"),
-    FEATURE_ENTRY(VIRTIO_NET_F_MRG_RXBUF, \
-            "VIRTIO_NET_F_MRG_RXBUF: Driver can merge receive buffers"),
-    FEATURE_ENTRY(VIRTIO_NET_F_STATUS, \
-            "VIRTIO_NET_F_STATUS: Configuration status field available"),
-    FEATURE_ENTRY(VIRTIO_NET_F_CTRL_VQ, \
-            "VIRTIO_NET_F_CTRL_VQ: Control channel available"),
-    FEATURE_ENTRY(VIRTIO_NET_F_CTRL_RX, \
-            "VIRTIO_NET_F_CTRL_RX: Control channel RX mode supported"),
-    FEATURE_ENTRY(VIRTIO_NET_F_CTRL_VLAN, \
-            "VIRTIO_NET_F_CTRL_VLAN: Control channel VLAN filtering supported"),
-    FEATURE_ENTRY(VIRTIO_NET_F_CTRL_RX_EXTRA, \
-            "VIRTIO_NET_F_CTRL_RX_EXTRA: Extra RX mode control supported"),
-    FEATURE_ENTRY(VIRTIO_NET_F_GUEST_ANNOUNCE, \
-            "VIRTIO_NET_F_GUEST_ANNOUNCE: Driver sending gratuitous packets "
-            "supported"),
-    FEATURE_ENTRY(VIRTIO_NET_F_MQ, \
-            "VIRTIO_NET_F_MQ: Multiqueue with automatic receive steering "
-            "supported"),
-    FEATURE_ENTRY(VIRTIO_NET_F_CTRL_MAC_ADDR, \
-            "VIRTIO_NET_F_CTRL_MAC_ADDR: MAC address set through control "
-            "channel"),
-    FEATURE_ENTRY(VIRTIO_NET_F_HASH_REPORT, \
-            "VIRTIO_NET_F_HASH_REPORT: Hash reporting supported"),
-    FEATURE_ENTRY(VIRTIO_NET_F_RSS, \
-            "VIRTIO_NET_F_RSS: RSS RX steering supported"),
-    FEATURE_ENTRY(VIRTIO_NET_F_RSC_EXT, \
-            "VIRTIO_NET_F_RSC_EXT: Extended coalescing info supported"),
-    FEATURE_ENTRY(VIRTIO_NET_F_STANDBY, \
-            "VIRTIO_NET_F_STANDBY: Device acting as standby for primary "
-            "device with same MAC addr. supported"),
-    FEATURE_ENTRY(VIRTIO_NET_F_SPEED_DUPLEX, \
-            "VIRTIO_NET_F_SPEED_DUPLEX: Device set linkspeed and duplex"),
-#ifndef VIRTIO_NET_NO_LEGACY
-    FEATURE_ENTRY(VIRTIO_NET_F_GSO, \
-            "VIRTIO_NET_F_GSO: Handling GSO-type packets supported"),
-#endif /* !VIRTIO_NET_NO_LEGACY */
-    FEATURE_ENTRY(VHOST_NET_F_VIRTIO_NET_HDR, \
-            "VHOST_NET_F_VIRTIO_NET_HDR: Virtio-net headers for RX and TX "
-            "packets supported"),
-    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
-            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
-    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
-            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
-            "negotiation supported"),
-    { -1, "" }
-};
-
-/* virtio-scsi features mapping */
-const qmp_virtio_feature_map_t virtio_scsi_feature_map[] = {
-    FEATURE_ENTRY(VIRTIO_SCSI_F_INOUT, \
-            "VIRTIO_SCSI_F_INOUT: Requests including read and writable data "
-            "buffers suppoted"),
-    FEATURE_ENTRY(VIRTIO_SCSI_F_HOTPLUG, \
-            "VIRTIO_SCSI_F_HOTPLUG: Reporting and handling hot-plug events "
-            "supported"),
-    FEATURE_ENTRY(VIRTIO_SCSI_F_CHANGE, \
-            "VIRTIO_SCSI_F_CHANGE: Reporting and handling LUN changes "
-            "supported"),
-    FEATURE_ENTRY(VIRTIO_SCSI_F_T10_PI, \
-            "VIRTIO_SCSI_F_T10_PI: T10 info included in request header"),
-    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
-            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
-    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
-            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
-            "negotiation supported"),
-    { -1, "" }
-};
-
-/* virtio/vhost-user-fs features mapping */
-const qmp_virtio_feature_map_t virtio_fs_feature_map[] = {
-    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
-            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
-    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
-            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
-            "negotiation supported"),
-    { -1, "" }
-};
-
-/* virtio/vhost-user-i2c features mapping */
-const qmp_virtio_feature_map_t virtio_i2c_feature_map[] = {
-    FEATURE_ENTRY(VIRTIO_I2C_F_ZERO_LENGTH_REQUEST, \
-            "VIRTIO_I2C_F_ZERO_LEGNTH_REQUEST: Zero length requests supported"),
-    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
-            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
-    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
-            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
-            "negotiation supported"),
-    { -1, "" }
-};
-
-/* virtio/vhost-vsock features mapping */
-const qmp_virtio_feature_map_t virtio_vsock_feature_map[] = {
-    FEATURE_ENTRY(VIRTIO_VSOCK_F_SEQPACKET, \
-            "VIRTIO_VSOCK_F_SEQPACKET: SOCK_SEQPACKET supported"),
-    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
-            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
-    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
-            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
-            "negotiation supported"),
-    { -1, "" }
-};
-
-/* virtio-balloon features mapping */
-const qmp_virtio_feature_map_t virtio_balloon_feature_map[] = {
-    FEATURE_ENTRY(VIRTIO_BALLOON_F_MUST_TELL_HOST, \
-            "VIRTIO_BALLOON_F_MUST_TELL_HOST: Tell host before reclaiming "
-            "pages"),
-    FEATURE_ENTRY(VIRTIO_BALLOON_F_STATS_VQ, \
-            "VIRTIO_BALLOON_F_STATS_VQ: Guest memory stats VQ available"),
-    FEATURE_ENTRY(VIRTIO_BALLOON_F_DEFLATE_ON_OOM, \
-            "VIRTIO_BALLOON_F_DEFLATE_ON_OOM: Deflate balloon when guest OOM"),
-    FEATURE_ENTRY(VIRTIO_BALLOON_F_FREE_PAGE_HINT, \
-            "VIRTIO_BALLOON_F_FREE_PAGE_HINT: VQ reporting free pages enabled"),
-    FEATURE_ENTRY(VIRTIO_BALLOON_F_PAGE_POISON, \
-            "VIRTIO_BALLOON_F_PAGE_POISON: Guest page poisoning enabled"),
-    FEATURE_ENTRY(VIRTIO_BALLOON_F_REPORTING, \
-            "VIRTIO_BALLOON_F_REPORTING: Page reporting VQ enabled"),
-    { -1, "" }
-};
-
-/* virtio-crypto features mapping */
-const qmp_virtio_feature_map_t virtio_crypto_feature_map[] = {
-    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
-            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
-    { -1, "" }
-};
-
-/* virtio-iommu features mapping */
-const qmp_virtio_feature_map_t virtio_iommu_feature_map[] = {
-    FEATURE_ENTRY(VIRTIO_IOMMU_F_INPUT_RANGE, \
-            "VIRTIO_IOMMU_F_INPUT_RANGE: Range of available virtual addrs. "
-            "available"),
-    FEATURE_ENTRY(VIRTIO_IOMMU_F_DOMAIN_RANGE, \
-            "VIRTIO_IOMMU_F_DOMAIN_RANGE: Number of supported domains "
-            "available"),
-    FEATURE_ENTRY(VIRTIO_IOMMU_F_MAP_UNMAP, \
-            "VIRTIO_IOMMU_F_MAP_UNMAP: Map and unmap requests available"),
-    FEATURE_ENTRY(VIRTIO_IOMMU_F_BYPASS, \
-            "VIRTIO_IOMMU_F_BYPASS: Endpoints not attached to domains are in "
-            "bypass mode"),
-    FEATURE_ENTRY(VIRTIO_IOMMU_F_PROBE, \
-            "VIRTIO_IOMMU_F_PROBE: Probe requests available"),
-    FEATURE_ENTRY(VIRTIO_IOMMU_F_MMIO, \
-            "VIRTIO_IOMMU_F_MMIO: VIRTIO_IOMMU_MAP_F_MMIO flag available"),
-    FEATURE_ENTRY(VIRTIO_IOMMU_F_BYPASS_CONFIG, \
-            "VIRTIO_IOMMU_F_BYPASS_CONFIG: Bypass field of IOMMU config "
-            "available"),
-    { -1, "" }
-};
-
-/* virtio-mem features mapping */
-const qmp_virtio_feature_map_t virtio_mem_feature_map[] = {
-#ifndef CONFIG_ACPI
-    FEATURE_ENTRY(VIRTIO_MEM_F_ACPI_PXM, \
-            "VIRTIO_MEM_F_ACPI_PXM: node_id is an ACPI PXM and is valid"),
-#endif /* !CONFIG_ACPI */
-    FEATURE_ENTRY(VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE, \
-            "VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE: Unplugged memory cannot be "
-            "accessed"),
-    { -1, "" }
-};
-
-/* virtio-rng features mapping */
-const qmp_virtio_feature_map_t virtio_rng_feature_map[] = {
-    FEATURE_ENTRY(VHOST_F_LOG_ALL, \
-            "VHOST_F_LOG_ALL: Logging write descriptors supported"),
-    FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
-            "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
-            "negotiation supported"),
-    { -1, "" }
-};
-
 /*
  * The alignment to use between consumer and producer parts of vring.
  * x86 pagesize again. This is the default, used by transports like PCI
@@ -4268,203 +3862,6 @@ static VirtIODevice *virtio_device_find(const char *path)
     return NULL;
 }
 
-#define CONVERT_FEATURES(type, map, is_status, bitmap)   \
-    ({                                                   \
-        type *list = NULL;                               \
-        type *node;                                      \
-        for (i = 0; map[i].virtio_bit != -1; i++) {      \
-            if (is_status) {                             \
-                bit = map[i].virtio_bit;                 \
-            }                                            \
-            else {                                       \
-                bit = 1ULL << map[i].virtio_bit;         \
-            }                                            \
-            if ((bitmap & bit) == 0) {                   \
-                continue;                                \
-            }                                            \
-            node = g_new0(type, 1);                      \
-            node->value = g_strdup(map[i].feature_desc); \
-            node->next = list;                           \
-            list = node;                                 \
-            bitmap ^= bit;                               \
-        }                                                \
-        list;                                            \
-    })
-
-static VirtioDeviceStatus *qmp_decode_status(uint8_t bitmap)
-{
-    VirtioDeviceStatus *status;
-    uint8_t bit;
-    int i;
-
-    status = g_new0(VirtioDeviceStatus, 1);
-    status->statuses = CONVERT_FEATURES(strList, virtio_config_status_map,
-                                        1, bitmap);
-    status->has_unknown_statuses = bitmap != 0;
-    if (status->has_unknown_statuses) {
-        status->unknown_statuses = bitmap;
-    }
-
-    return status;
-}
-
-static VhostDeviceProtocols *qmp_decode_protocols(uint64_t bitmap)
-{
-    VhostDeviceProtocols *vhu_protocols;
-    uint64_t bit;
-    int i;
-
-    vhu_protocols = g_new0(VhostDeviceProtocols, 1);
-    vhu_protocols->protocols =
-                    CONVERT_FEATURES(strList,
-                                     vhost_user_protocol_map, 0, bitmap);
-    vhu_protocols->has_unknown_protocols = bitmap != 0;
-    if (vhu_protocols->has_unknown_protocols) {
-        vhu_protocols->unknown_protocols = bitmap;
-    }
-
-    return vhu_protocols;
-}
-
-static VirtioDeviceFeatures *qmp_decode_features(uint16_t device_id,
-                                                 uint64_t bitmap)
-{
-    VirtioDeviceFeatures *features;
-    uint64_t bit;
-    int i;
-
-    features = g_new0(VirtioDeviceFeatures, 1);
-    features->has_dev_features = true;
-
-    /* transport features */
-    features->transports = CONVERT_FEATURES(strList, virtio_transport_map, 0,
-                                            bitmap);
-
-    /* device features */
-    switch (device_id) {
-#ifdef CONFIG_VIRTIO_SERIAL
-    case VIRTIO_ID_CONSOLE:
-        features->dev_features =
-            CONVERT_FEATURES(strList, virtio_serial_feature_map, 0, bitmap);
-        break;
-#endif
-#ifdef CONFIG_VIRTIO_BLK
-    case VIRTIO_ID_BLOCK:
-        features->dev_features =
-            CONVERT_FEATURES(strList, virtio_blk_feature_map, 0, bitmap);
-        break;
-#endif
-#ifdef CONFIG_VIRTIO_GPU
-    case VIRTIO_ID_GPU:
-        features->dev_features =
-            CONVERT_FEATURES(strList, virtio_gpu_feature_map, 0, bitmap);
-        break;
-#endif
-#ifdef CONFIG_VIRTIO_NET
-    case VIRTIO_ID_NET:
-        features->dev_features =
-            CONVERT_FEATURES(strList, virtio_net_feature_map, 0, bitmap);
-        break;
-#endif
-#ifdef CONFIG_VIRTIO_SCSI
-    case VIRTIO_ID_SCSI:
-        features->dev_features =
-            CONVERT_FEATURES(strList, virtio_scsi_feature_map, 0, bitmap);
-        break;
-#endif
-#ifdef CONFIG_VIRTIO_BALLOON
-    case VIRTIO_ID_BALLOON:
-        features->dev_features =
-            CONVERT_FEATURES(strList, virtio_balloon_feature_map, 0, bitmap);
-        break;
-#endif
-#ifdef CONFIG_VIRTIO_IOMMU
-    case VIRTIO_ID_IOMMU:
-        features->dev_features =
-            CONVERT_FEATURES(strList, virtio_iommu_feature_map, 0, bitmap);
-        break;
-#endif
-#ifdef CONFIG_VIRTIO_INPUT
-    case VIRTIO_ID_INPUT:
-        features->dev_features =
-            CONVERT_FEATURES(strList, virtio_input_feature_map, 0, bitmap);
-        break;
-#endif
-#ifdef CONFIG_VHOST_USER_FS
-    case VIRTIO_ID_FS:
-        features->dev_features =
-            CONVERT_FEATURES(strList, virtio_fs_feature_map, 0, bitmap);
-        break;
-#endif
-#ifdef CONFIG_VHOST_VSOCK
-    case VIRTIO_ID_VSOCK:
-        features->dev_features =
-            CONVERT_FEATURES(strList, virtio_vsock_feature_map, 0, bitmap);
-        break;
-#endif
-#ifdef CONFIG_VIRTIO_CRYPTO
-    case VIRTIO_ID_CRYPTO:
-        features->dev_features =
-            CONVERT_FEATURES(strList, virtio_crypto_feature_map, 0, bitmap);
-        break;
-#endif
-#ifdef CONFIG_VIRTIO_MEM
-    case VIRTIO_ID_MEM:
-        features->dev_features =
-            CONVERT_FEATURES(strList, virtio_mem_feature_map, 0, bitmap);
-        break;
-#endif
-#ifdef CONFIG_VIRTIO_I2C_ADAPTER
-    case VIRTIO_ID_I2C_ADAPTER:
-        features->dev_features =
-            CONVERT_FEATURES(strList, virtio_i2c_feature_map, 0, bitmap);
-        break;
-#endif
-#ifdef CONFIG_VIRTIO_RNG
-    case VIRTIO_ID_RNG:
-        features->dev_features =
-            CONVERT_FEATURES(strList, virtio_rng_feature_map, 0, bitmap);
-        break;
-#endif
-    /* No features */
-    case VIRTIO_ID_9P:
-    case VIRTIO_ID_PMEM:
-    case VIRTIO_ID_IOMEM:
-    case VIRTIO_ID_RPMSG:
-    case VIRTIO_ID_CLOCK:
-    case VIRTIO_ID_MAC80211_WLAN:
-    case VIRTIO_ID_MAC80211_HWSIM:
-    case VIRTIO_ID_RPROC_SERIAL:
-    case VIRTIO_ID_MEMORY_BALLOON:
-    case VIRTIO_ID_CAIF:
-    case VIRTIO_ID_SIGNAL_DIST:
-    case VIRTIO_ID_PSTORE:
-    case VIRTIO_ID_SOUND:
-    case VIRTIO_ID_BT:
-    case VIRTIO_ID_RPMB:
-    case VIRTIO_ID_VIDEO_ENCODER:
-    case VIRTIO_ID_VIDEO_DECODER:
-    case VIRTIO_ID_SCMI:
-    case VIRTIO_ID_NITRO_SEC_MOD:
-    case VIRTIO_ID_WATCHDOG:
-    case VIRTIO_ID_CAN:
-    case VIRTIO_ID_DMABUF:
-    case VIRTIO_ID_PARAM_SERV:
-    case VIRTIO_ID_AUDIO_POLICY:
-    case VIRTIO_ID_GPIO:
-        break;
-    default:
-        g_assert_not_reached();
-    }
-
-    features->has_unknown_dev_features = bitmap != 0;
-    if (features->has_unknown_dev_features) {
-        features->unknown_dev_features = bitmap;
-    }
-
-    return features;
-}
-
 VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
 {
     VirtIODevice *vdev;
-- 
2.38.1



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

* [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state
  2022-12-12 23:05 [RFC PATCH-for-8.0 00/10] hw/virtio: Build most objects as target independent units Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
  2022-12-12 23:05 ` [PATCH-for-8.0 05/10] hw/virtio: Extract QMP related code virtio-qmp.c Philippe Mathieu-Daudé
@ 2022-12-12 23:05 ` Philippe Mathieu-Daudé
  2022-12-13  0:14   ` Richard Henderson
  2022-12-12 23:05 ` [RFC PATCH-for-8.0 07/10] hw/virtio: Directly access cached VirtIODevice::access_is_big_endian Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-12 23:05 UTC (permalink / raw)
  To: qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz,
	Philippe Mathieu-Daudé

The device endianness doesn't change during runtime.
Cache it in the VirtIODevice state.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
RFC: I'm not sure virtio_init() is the correct place to add this
     check. We want to initialize this field once the features are
     negociated.
---
 hw/virtio/virtio.c         | 1 +
 include/hw/virtio/virtio.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 09b1a0e3d9..dbb1fe33f7 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3193,6 +3193,7 @@ void virtio_init(VirtIODevice *vdev, uint16_t device_id, size_t config_size)
     vdev->vmstate = qdev_add_vm_change_state_handler(DEVICE(vdev),
             virtio_vmstate_change, vdev);
     vdev->device_endian = virtio_default_endian();
+    vdev->access_is_big_endian = virtio_access_is_big_endian(vdev);
     vdev->use_guest_notifier_mask = true;
 }
 
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index acfd4df125..5f28e51e93 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -130,6 +130,7 @@ struct VirtIODevice
     bool vhost_started;
     VMChangeStateEntry *vmstate;
     char *bus_name;
+    bool access_is_big_endian;
     uint8_t device_endian;
     bool use_guest_notifier_mask;
     AddressSpace *dma_as;
-- 
2.38.1



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

* [RFC PATCH-for-8.0 07/10] hw/virtio: Directly access cached VirtIODevice::access_is_big_endian
  2022-12-12 23:05 [RFC PATCH-for-8.0 00/10] hw/virtio: Build most objects as target independent units Philippe Mathieu-Daudé
                   ` (5 preceding siblings ...)
  2022-12-12 23:05 ` [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state Philippe Mathieu-Daudé
@ 2022-12-12 23:05 ` Philippe Mathieu-Daudé
  2022-12-13 10:50   ` Greg Kurz
  2022-12-12 23:05 ` [PATCH-for-8.0 08/10] hw/virtio: Un-inline virtio_access_is_big_endian() Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-12 23:05 UTC (permalink / raw)
  To: qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz,
	Philippe Mathieu-Daudé

Since the device endianness doesn't change at runtime,
use the cached value instead of evaluating it on each call.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/virtio/virtio-access.h | 44 +++++++++++++++----------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
index 07aae69042..985f39fe16 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -43,7 +43,7 @@ static inline uint16_t virtio_lduw_phys(VirtIODevice *vdev, hwaddr pa)
 {
     AddressSpace *dma_as = vdev->dma_as;
 
-    if (virtio_access_is_big_endian(vdev)) {
+    if (vdev->access_is_big_endian) {
         return lduw_be_phys(dma_as, pa);
     }
     return lduw_le_phys(dma_as, pa);
@@ -53,7 +53,7 @@ static inline uint32_t virtio_ldl_phys(VirtIODevice *vdev, hwaddr pa)
 {
     AddressSpace *dma_as = vdev->dma_as;
 
-    if (virtio_access_is_big_endian(vdev)) {
+    if (vdev->access_is_big_endian) {
         return ldl_be_phys(dma_as, pa);
     }
     return ldl_le_phys(dma_as, pa);
@@ -63,7 +63,7 @@ static inline uint64_t virtio_ldq_phys(VirtIODevice *vdev, hwaddr pa)
 {
     AddressSpace *dma_as = vdev->dma_as;
 
-    if (virtio_access_is_big_endian(vdev)) {
+    if (vdev->access_is_big_endian) {
         return ldq_be_phys(dma_as, pa);
     }
     return ldq_le_phys(dma_as, pa);
@@ -74,7 +74,7 @@ static inline void virtio_stw_phys(VirtIODevice *vdev, hwaddr pa,
 {
     AddressSpace *dma_as = vdev->dma_as;
 
-    if (virtio_access_is_big_endian(vdev)) {
+    if (vdev->access_is_big_endian) {
         stw_be_phys(dma_as, pa, value);
     } else {
         stw_le_phys(dma_as, pa, value);
@@ -86,7 +86,7 @@ static inline void virtio_stl_phys(VirtIODevice *vdev, hwaddr pa,
 {
     AddressSpace *dma_as = vdev->dma_as;
 
-    if (virtio_access_is_big_endian(vdev)) {
+    if (vdev->access_is_big_endian) {
         stl_be_phys(dma_as, pa, value);
     } else {
         stl_le_phys(dma_as, pa, value);
@@ -95,7 +95,7 @@ static inline void virtio_stl_phys(VirtIODevice *vdev, hwaddr pa,
 
 static inline void virtio_stw_p(VirtIODevice *vdev, void *ptr, uint16_t v)
 {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (vdev->access_is_big_endian) {
         stw_be_p(ptr, v);
     } else {
         stw_le_p(ptr, v);
@@ -104,7 +104,7 @@ static inline void virtio_stw_p(VirtIODevice *vdev, void *ptr, uint16_t v)
 
 static inline void virtio_stl_p(VirtIODevice *vdev, void *ptr, uint32_t v)
 {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (vdev->access_is_big_endian) {
         stl_be_p(ptr, v);
     } else {
         stl_le_p(ptr, v);
@@ -113,7 +113,7 @@ static inline void virtio_stl_p(VirtIODevice *vdev, void *ptr, uint32_t v)
 
 static inline void virtio_stq_p(VirtIODevice *vdev, void *ptr, uint64_t v)
 {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (vdev->access_is_big_endian) {
         stq_be_p(ptr, v);
     } else {
         stq_le_p(ptr, v);
@@ -122,7 +122,7 @@ static inline void virtio_stq_p(VirtIODevice *vdev, void *ptr, uint64_t v)
 
 static inline int virtio_lduw_p(VirtIODevice *vdev, const void *ptr)
 {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (vdev->access_is_big_endian) {
         return lduw_be_p(ptr);
     } else {
         return lduw_le_p(ptr);
@@ -131,7 +131,7 @@ static inline int virtio_lduw_p(VirtIODevice *vdev, const void *ptr)
 
 static inline int virtio_ldl_p(VirtIODevice *vdev, const void *ptr)
 {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (vdev->access_is_big_endian) {
         return ldl_be_p(ptr);
     } else {
         return ldl_le_p(ptr);
@@ -140,7 +140,7 @@ static inline int virtio_ldl_p(VirtIODevice *vdev, const void *ptr)
 
 static inline uint64_t virtio_ldq_p(VirtIODevice *vdev, const void *ptr)
 {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (vdev->access_is_big_endian) {
         return ldq_be_p(ptr);
     } else {
         return ldq_le_p(ptr);
@@ -150,9 +150,9 @@ static inline uint64_t virtio_ldq_p(VirtIODevice *vdev, const void *ptr)
 static inline uint16_t virtio_tswap16(VirtIODevice *vdev, uint16_t s)
 {
 #if HOST_BIG_ENDIAN
-    return virtio_access_is_big_endian(vdev) ? s : bswap16(s);
+    return vdev->access_is_big_endian ? s : bswap16(s);
 #else
-    return virtio_access_is_big_endian(vdev) ? bswap16(s) : s;
+    return vdev->access_is_big_endian ? bswap16(s) : s;
 #endif
 }
 
@@ -160,7 +160,7 @@ static inline uint16_t virtio_lduw_phys_cached(VirtIODevice *vdev,
                                                MemoryRegionCache *cache,
                                                hwaddr pa)
 {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (vdev->access_is_big_endian) {
         return lduw_be_phys_cached(cache, pa);
     }
     return lduw_le_phys_cached(cache, pa);
@@ -170,7 +170,7 @@ static inline uint32_t virtio_ldl_phys_cached(VirtIODevice *vdev,
                                               MemoryRegionCache *cache,
                                               hwaddr pa)
 {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (vdev->access_is_big_endian) {
         return ldl_be_phys_cached(cache, pa);
     }
     return ldl_le_phys_cached(cache, pa);
@@ -180,7 +180,7 @@ static inline uint64_t virtio_ldq_phys_cached(VirtIODevice *vdev,
                                               MemoryRegionCache *cache,
                                               hwaddr pa)
 {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (vdev->access_is_big_endian) {
         return ldq_be_phys_cached(cache, pa);
     }
     return ldq_le_phys_cached(cache, pa);
@@ -190,7 +190,7 @@ static inline void virtio_stw_phys_cached(VirtIODevice *vdev,
                                           MemoryRegionCache *cache,
                                           hwaddr pa, uint16_t value)
 {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (vdev->access_is_big_endian) {
         stw_be_phys_cached(cache, pa, value);
     } else {
         stw_le_phys_cached(cache, pa, value);
@@ -201,7 +201,7 @@ static inline void virtio_stl_phys_cached(VirtIODevice *vdev,
                                           MemoryRegionCache *cache,
                                           hwaddr pa, uint32_t value)
 {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (vdev->access_is_big_endian) {
         stl_be_phys_cached(cache, pa, value);
     } else {
         stl_le_phys_cached(cache, pa, value);
@@ -216,9 +216,9 @@ static inline void virtio_tswap16s(VirtIODevice *vdev, uint16_t *s)
 static inline uint32_t virtio_tswap32(VirtIODevice *vdev, uint32_t s)
 {
 #if HOST_BIG_ENDIAN
-    return virtio_access_is_big_endian(vdev) ? s : bswap32(s);
+    return vdev->access_is_big_endian ? s : bswap32(s);
 #else
-    return virtio_access_is_big_endian(vdev) ? bswap32(s) : s;
+    return vdev->access_is_big_endian ? bswap32(s) : s;
 #endif
 }
 
@@ -230,9 +230,9 @@ static inline void virtio_tswap32s(VirtIODevice *vdev, uint32_t *s)
 static inline uint64_t virtio_tswap64(VirtIODevice *vdev, uint64_t s)
 {
 #if HOST_BIG_ENDIAN
-    return virtio_access_is_big_endian(vdev) ? s : bswap64(s);
+    return vdev->access_is_big_endian ? s : bswap64(s);
 #else
-    return virtio_access_is_big_endian(vdev) ? bswap64(s) : s;
+    return vdev->access_is_big_endian ? bswap64(s) : s;
 #endif
 }
 
-- 
2.38.1



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

* [PATCH-for-8.0 08/10] hw/virtio: Un-inline virtio_access_is_big_endian()
  2022-12-12 23:05 [RFC PATCH-for-8.0 00/10] hw/virtio: Build most objects as target independent units Philippe Mathieu-Daudé
                   ` (6 preceding siblings ...)
  2022-12-12 23:05 ` [RFC PATCH-for-8.0 07/10] hw/virtio: Directly access cached VirtIODevice::access_is_big_endian Philippe Mathieu-Daudé
@ 2022-12-12 23:05 ` Philippe Mathieu-Daudé
  2022-12-12 23:05 ` [RFC PATCH-for-8.0 09/10] hw/virtio: Extract vhost_user_ram_slots_max() to vhost-user-target.c Philippe Mathieu-Daudé
  2022-12-12 23:05 ` [RFC PATCH-for-8.0 10/10] hw/virtio: Make most of virtio devices target-independent Philippe Mathieu-Daudé
  9 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-12 23:05 UTC (permalink / raw)
  To: qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz,
	Philippe Mathieu-Daudé

In order to avoid target-specific code in VirtIO headers,
move this particular function -- which is only called once
in virtio_init() -- in its own unit.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/virtio-config.c         | 20 ++++++++++++++++++++
 include/hw/virtio/virtio-access.h | 19 +------------------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/hw/virtio/virtio-config.c b/hw/virtio/virtio-config.c
index ad78e0b9bc..aca6ef5e1b 100644
--- a/hw/virtio/virtio-config.c
+++ b/hw/virtio/virtio-config.c
@@ -11,8 +11,28 @@
 
 #include "qemu/osdep.h"
 #include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-access.h"
 #include "cpu.h"
 
+#if defined(TARGET_PPC64) || defined(TARGET_ARM)
+#define LEGACY_VIRTIO_IS_BIENDIAN 1
+#endif
+
+bool virtio_access_is_big_endian(VirtIODevice *vdev)
+{
+#if defined(LEGACY_VIRTIO_IS_BIENDIAN)
+    return virtio_is_big_endian(vdev);
+#elif TARGET_BIG_ENDIAN
+    if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+        /* Devices conforming to VIRTIO 1.0 or later are always LE. */
+        return false;
+    }
+    return true;
+#else
+    return false;
+#endif
+}
+
 uint32_t virtio_config_readb(VirtIODevice *vdev, uint32_t addr)
 {
     VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
index 985f39fe16..7229088b7c 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -20,24 +20,7 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-bus.h"
 
-#if defined(TARGET_PPC64) || defined(TARGET_ARM)
-#define LEGACY_VIRTIO_IS_BIENDIAN 1
-#endif
-
-static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
-{
-#if defined(LEGACY_VIRTIO_IS_BIENDIAN)
-    return virtio_is_big_endian(vdev);
-#elif TARGET_BIG_ENDIAN
-    if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
-        /* Devices conforming to VIRTIO 1.0 or later are always LE. */
-        return false;
-    }
-    return true;
-#else
-    return false;
-#endif
-}
+bool virtio_access_is_big_endian(VirtIODevice *vdev);
 
 static inline uint16_t virtio_lduw_phys(VirtIODevice *vdev, hwaddr pa)
 {
-- 
2.38.1



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

* [RFC PATCH-for-8.0 09/10] hw/virtio: Extract vhost_user_ram_slots_max() to vhost-user-target.c
  2022-12-12 23:05 [RFC PATCH-for-8.0 00/10] hw/virtio: Build most objects as target independent units Philippe Mathieu-Daudé
                   ` (7 preceding siblings ...)
  2022-12-12 23:05 ` [PATCH-for-8.0 08/10] hw/virtio: Un-inline virtio_access_is_big_endian() Philippe Mathieu-Daudé
@ 2022-12-12 23:05 ` Philippe Mathieu-Daudé
  2025-04-10 12:14   ` Philippe Mathieu-Daudé
  2022-12-12 23:05 ` [RFC PATCH-for-8.0 10/10] hw/virtio: Make most of virtio devices target-independent Philippe Mathieu-Daudé
  9 siblings, 1 reply; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-12 23:05 UTC (permalink / raw)
  To: qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz,
	Philippe Mathieu-Daudé

The current definition of VHOST_USER_MAX_RAM_SLOTS is
target specific. By converting this definition to a runtime
vhost_user_ram_slots_max() helper declared in a target
specific unit, we can have the rest of vhost-user.c target
independent.

To avoid variable length array or using the heap to store
arrays of vhost_user_ram_slots_max() elements, we simply
declare an array of the biggest VHOST_USER_MAX_RAM_SLOTS,
and each target uses up to vhost_user_ram_slots_max()
elements of it. Ensure arrays are big enough by adding an
assertion in vhost_user_init().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
RFC: Should I add VHOST_USER_MAX_RAM_SLOTS to vhost-user.h
     or create an internal header for it?
---
 hw/virtio/meson.build          |  1 +
 hw/virtio/vhost-user-target.c  | 29 +++++++++++++++++++++++++++++
 hw/virtio/vhost-user.c         | 26 +++++---------------------
 include/hw/virtio/vhost-user.h |  7 +++++++
 4 files changed, 42 insertions(+), 21 deletions(-)
 create mode 100644 hw/virtio/vhost-user-target.c

diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index eb7ee8ea92..bf7e35fa8a 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -11,6 +11,7 @@ if have_vhost
   specific_virtio_ss.add(files('vhost.c', 'vhost-backend.c', 'vhost-iova-tree.c'))
   if have_vhost_user
     specific_virtio_ss.add(files('vhost-user.c'))
+    specific_virtio_ss.add(files('vhost-user-target.c'))
   endif
   if have_vhost_vdpa
     specific_virtio_ss.add(files('vhost-vdpa.c', 'vhost-shadow-virtqueue.c'))
diff --git a/hw/virtio/vhost-user-target.c b/hw/virtio/vhost-user-target.c
new file mode 100644
index 0000000000..6a0d0f53d0
--- /dev/null
+++ b/hw/virtio/vhost-user-target.c
@@ -0,0 +1,29 @@
+/*
+ * vhost-user target-specific helpers
+ *
+ * Copyright (c) 2013 Virtual Open Systems Sarl.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/virtio/vhost-user.h"
+
+#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
+    defined(TARGET_ARM) || defined(TARGET_ARM_64)
+#include "hw/acpi/acpi.h"
+#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
+#include "hw/ppc/spapr.h"
+#endif
+
+unsigned int vhost_user_ram_slots_max(void)
+{
+#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
+    defined(TARGET_ARM) || defined(TARGET_ARM_64)
+    return ACPI_MAX_RAM_SLOTS;
+#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
+    return SPAPR_MAX_RAM_SLOTS;
+#else
+    return 512;
+#endif
+}
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 8f635844af..21fc176725 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -41,24 +41,7 @@
 #define VHOST_MEMORY_BASELINE_NREGIONS    8
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 #define VHOST_USER_SLAVE_MAX_FDS     8
-
-/*
- * Set maximum number of RAM slots supported to
- * the maximum number supported by the target
- * hardware plaform.
- */
-#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
-    defined(TARGET_ARM) || defined(TARGET_ARM_64)
-#include "hw/acpi/acpi.h"
-#define VHOST_USER_MAX_RAM_SLOTS ACPI_MAX_RAM_SLOTS
-
-#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
-#include "hw/ppc/spapr.h"
-#define VHOST_USER_MAX_RAM_SLOTS SPAPR_MAX_RAM_SLOTS
-
-#else
 #define VHOST_USER_MAX_RAM_SLOTS 512
-#endif
 
 /*
  * Maximum size of virtio device config space
@@ -935,7 +918,7 @@ static int vhost_user_add_remove_regions(struct vhost_dev *dev,
 
     if (track_ramblocks) {
         memcpy(u->postcopy_client_bases, shadow_pcb,
-               sizeof(uint64_t) * VHOST_USER_MAX_RAM_SLOTS);
+               sizeof(uint64_t) * vhost_user_ram_slots_max());
         /*
          * Now we've registered this with the postcopy code, we ack to the
          * client, because now we're in the position to be able to deal with
@@ -956,7 +939,7 @@ static int vhost_user_add_remove_regions(struct vhost_dev *dev,
 err:
     if (track_ramblocks) {
         memcpy(u->postcopy_client_bases, shadow_pcb,
-               sizeof(uint64_t) * VHOST_USER_MAX_RAM_SLOTS);
+               sizeof(uint64_t) * vhost_user_ram_slots_max());
     }
 
     return ret;
@@ -1030,7 +1013,7 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev,
         }
 
         memset(u->postcopy_client_bases, 0,
-               sizeof(uint64_t) * VHOST_USER_MAX_RAM_SLOTS);
+               sizeof(uint64_t) * vhost_user_ram_slots_max());
 
         /*
          * They're in the same order as the regions that were sent
@@ -2169,7 +2152,7 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
                 return -EINVAL;
             }
 
-            u->user->memory_slots = MIN(ram_slots, VHOST_USER_MAX_RAM_SLOTS);
+            u->user->memory_slots = MIN(ram_slots, vhost_user_ram_slots_max());
         }
     }
 
@@ -2649,6 +2632,7 @@ static void vhost_user_state_destroy(gpointer data)
 
 bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp)
 {
+    assert(vhost_user_ram_slots_max() <= VHOST_USER_MAX_RAM_SLOTS);
     if (user->chr) {
         error_setg(errp, "Cannot initialize vhost-user state");
         return false;
diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
index 191216a74f..e13584ade8 100644
--- a/include/hw/virtio/vhost-user.h
+++ b/include/hw/virtio/vhost-user.h
@@ -86,4 +86,11 @@ void vhost_user_async_close(DeviceState *d,
                             CharBackend *chardev, struct vhost_dev *vhost,
                             vu_async_close_fn cb);
 
+/**
+ * vhost_user_ram_slots_max()
+ *
+ * Return: maximum number of RAM slots supported by the target hardware plaform.
+ */
+unsigned int vhost_user_ram_slots_max(void);
+
 #endif
-- 
2.38.1



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

* [RFC PATCH-for-8.0 10/10] hw/virtio: Make most of virtio devices target-independent
  2022-12-12 23:05 [RFC PATCH-for-8.0 00/10] hw/virtio: Build most objects as target independent units Philippe Mathieu-Daudé
                   ` (8 preceding siblings ...)
  2022-12-12 23:05 ` [RFC PATCH-for-8.0 09/10] hw/virtio: Extract vhost_user_ram_slots_max() to vhost-user-target.c Philippe Mathieu-Daudé
@ 2022-12-12 23:05 ` Philippe Mathieu-Daudé
  9 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-12 23:05 UTC (permalink / raw)
  To: qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz,
	Philippe Mathieu-Daudé

Except the following files:
- virtio-config.c
- virtio-qmp.c
- virtio-iommu.c
- virtio-mem.c
- vhost-user-target.c
- vhost-vdpa.c
all other virtio related files are target independent and
can be compiled only once for a system emulation build,
avoiding compiling hundreds of objects.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
RFC: Cross-built on ppc64le/s390x but not tested there.
---
 hw/9pfs/meson.build            |  2 +-
 hw/block/dataplane/meson.build |  2 +-
 hw/block/meson.build           |  4 ++--
 hw/char/meson.build            |  2 +-
 hw/net/meson.build             |  2 +-
 hw/virtio/meson.build          | 38 +++++++++++++++++-----------------
 6 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/hw/9pfs/meson.build b/hw/9pfs/meson.build
index 12443b6ad5..ef37532dbf 100644
--- a/hw/9pfs/meson.build
+++ b/hw/9pfs/meson.build
@@ -18,4 +18,4 @@ fs_ss.add(when: 'CONFIG_DARWIN', if_true: files('9p-util-darwin.c'))
 fs_ss.add(when: 'CONFIG_XEN', if_true: files('xen-9p-backend.c'))
 softmmu_ss.add_all(when: 'CONFIG_FSDEV_9P', if_true: fs_ss)
 
-specific_ss.add(when: 'CONFIG_VIRTIO_9P', if_true: files('virtio-9p-device.c'))
+softmmu_ss.add(when: 'CONFIG_VIRTIO_9P', if_true: files('virtio-9p-device.c'))
diff --git a/hw/block/dataplane/meson.build b/hw/block/dataplane/meson.build
index 12c6a264f1..e2f3721ce2 100644
--- a/hw/block/dataplane/meson.build
+++ b/hw/block/dataplane/meson.build
@@ -1,2 +1,2 @@
-specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
+softmmu_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
 specific_ss.add(when: 'CONFIG_XEN', if_true: files('xen-block.c'))
diff --git a/hw/block/meson.build b/hw/block/meson.build
index b434d5654c..8a3ca43a5c 100644
--- a/hw/block/meson.build
+++ b/hw/block/meson.build
@@ -17,7 +17,7 @@ softmmu_ss.add(when: 'CONFIG_SWIM', if_true: files('swim.c'))
 softmmu_ss.add(when: 'CONFIG_XEN', if_true: files('xen-block.c'))
 softmmu_ss.add(when: 'CONFIG_TC58128', if_true: files('tc58128.c'))
 
-specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c', 'virtio-blk-common.c'))
-specific_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: files('vhost-user-blk.c', 'virtio-blk-common.c'))
+softmmu_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c', 'virtio-blk-common.c'))
+softmmu_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: files('vhost-user-blk.c', 'virtio-blk-common.c'))
 
 subdir('dataplane')
diff --git a/hw/char/meson.build b/hw/char/meson.build
index 7b594f51b8..afd35649cd 100644
--- a/hw/char/meson.build
+++ b/hw/char/meson.build
@@ -18,6 +18,7 @@ softmmu_ss.add(when: 'CONFIG_SERIAL_PCI', if_true: files('serial-pci.c'))
 softmmu_ss.add(when: 'CONFIG_SERIAL_PCI_MULTI', if_true: files('serial-pci-multi.c'))
 softmmu_ss.add(when: 'CONFIG_SHAKTI_UART', if_true: files('shakti_uart.c'))
 softmmu_ss.add(when: 'CONFIG_VIRTIO_SERIAL', if_true: files('virtio-console.c'))
+softmmu_ss.add(when: 'CONFIG_VIRTIO', if_true: files('virtio-serial-bus.c'))
 softmmu_ss.add(when: 'CONFIG_XEN', if_true: files('xen_console.c'))
 softmmu_ss.add(when: 'CONFIG_XILINX', if_true: files('xilinx_uartlite.c'))
 
@@ -35,7 +36,6 @@ softmmu_ss.add(when: 'CONFIG_MCHP_PFSOC_MMUART', if_true: files('mchp_pfsoc_mmua
 
 specific_ss.add(when: 'CONFIG_HTIF', if_true: files('riscv_htif.c'))
 specific_ss.add(when: 'CONFIG_TERMINAL3270', if_true: files('terminal3270.c'))
-specific_ss.add(when: 'CONFIG_VIRTIO', if_true: files('virtio-serial-bus.c'))
 specific_ss.add(when: 'CONFIG_PSERIES', if_true: files('spapr_vty.c'))
 
 specific_ss.add(when: 'CONFIG_GOLDFISH_TTY', if_true: files('goldfish_tty.c'))
diff --git a/hw/net/meson.build b/hw/net/meson.build
index ebac261542..8035aaa560 100644
--- a/hw/net/meson.build
+++ b/hw/net/meson.build
@@ -44,7 +44,7 @@ specific_ss.add(when: 'CONFIG_PSERIES', if_true: files('spapr_llan.c'))
 specific_ss.add(when: 'CONFIG_XILINX_ETHLITE', if_true: files('xilinx_ethlite.c'))
 
 softmmu_ss.add(when: 'CONFIG_VIRTIO_NET', if_true: files('net_rx_pkt.c'))
-specific_ss.add(when: 'CONFIG_VIRTIO_NET', if_true: files('virtio-net.c'))
+softmmu_ss.add(when: 'CONFIG_VIRTIO_NET', if_true: files('virtio-net.c'))
 
 if have_vhost_net
   softmmu_ss.add(when: 'CONFIG_VIRTIO_NET', if_true: files('vhost_net.c'), if_false: files('vhost_net-stub.c'))
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index bf7e35fa8a..23be895c8e 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -1,38 +1,38 @@
 softmmu_virtio_ss = ss.source_set()
-softmmu_virtio_ss.add(files('virtio-bus.c'))
+softmmu_virtio_ss.add(files('virtio.c', 'virtio-bus.c'))
 softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: files('virtio-pci.c'))
 softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_MMIO', if_true: files('virtio-mmio.c'))
 
 specific_virtio_ss = ss.source_set()
-specific_virtio_ss.add(files('virtio.c'))
 specific_virtio_ss.add(files('virtio-config.c', 'virtio-qmp.c'))
+specific_virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu.c'))
+specific_virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
 
 if have_vhost
-  specific_virtio_ss.add(files('vhost.c', 'vhost-backend.c', 'vhost-iova-tree.c'))
+  softmmu_virtio_ss.add(files('vhost.c', 'vhost-backend.c', 'vhost-iova-tree.c'))
   if have_vhost_user
-    specific_virtio_ss.add(files('vhost-user.c'))
+    softmmu_virtio_ss.add(files('vhost-user.c'))
     specific_virtio_ss.add(files('vhost-user-target.c'))
   endif
   if have_vhost_vdpa
-    specific_virtio_ss.add(files('vhost-vdpa.c', 'vhost-shadow-virtqueue.c'))
+    specific_virtio_ss.add(files('vhost-vdpa.c'))
+    softmmu_virtio_ss.add(files('vhost-shadow-virtqueue.c'))
   endif
 else
   softmmu_virtio_ss.add(files('vhost-stub.c'))
 endif
 
-specific_virtio_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: files('virtio-balloon.c'))
-specific_virtio_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('virtio-crypto.c'))
-specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: files('vhost-user-fs.c'))
-specific_virtio_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: files('virtio-pmem.c'))
-specific_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock.c', 'vhost-vsock-common.c'))
-specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock.c', 'vhost-vsock-common.c'))
-specific_virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
-specific_virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu.c'))
-specific_virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
-specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: files('vhost-user-i2c.c'))
-specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: files('vhost-user-rng.c'))
-specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_GPIO', if_true: files('vhost-user-gpio.c'))
-specific_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_GPIO'], if_true: files('vhost-user-gpio-pci.c'))
+softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: files('virtio-balloon.c'))
+softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('virtio-crypto.c'))
+softmmu_virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: files('vhost-user-fs.c'))
+softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: files('virtio-pmem.c'))
+softmmu_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock.c', 'vhost-vsock-common.c'))
+softmmu_virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock.c', 'vhost-vsock-common.c'))
+softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
+softmmu_virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: files('vhost-user-i2c.c'))
+softmmu_virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: files('vhost-user-rng.c'))
+softmmu_virtio_ss.add(when: 'CONFIG_VHOST_USER_GPIO', if_true: files('vhost-user-gpio.c'))
+softmmu_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_GPIO'], if_true: files('vhost-user-gpio-pci.c'))
 
 virtio_pci_ss = ss.source_set()
 virtio_pci_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock-pci.c'))
@@ -59,7 +59,7 @@ virtio_pci_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: files('virtio-pmem-pci.c'
 virtio_pci_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem-pci.c'))
 
-specific_virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
+softmmu_virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
 
 softmmu_ss.add_all(when: 'CONFIG_VIRTIO', if_true: softmmu_virtio_ss)
 softmmu_ss.add(when: 'CONFIG_VIRTIO', if_false: files('vhost-stub.c'))
-- 
2.38.1



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

* Re: [PATCH-for-8.0 03/10] hw/virtio: Constify qmp_virtio_feature_map_t[]
  2022-12-12 23:05 ` [PATCH-for-8.0 03/10] hw/virtio: Constify qmp_virtio_feature_map_t[] Philippe Mathieu-Daudé
@ 2022-12-13  0:02   ` Richard Henderson
  2022-12-13  7:35     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 26+ messages in thread
From: Richard Henderson @ 2022-12-13  0:02 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz

On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:
> @@ -161,7 +161,7 @@ static qmp_virtio_feature_map_t vhost_user_protocol_map[] = {
>   };
>   
>   /* virtio device configuration statuses */
> -static qmp_virtio_feature_map_t virtio_config_status_map[] = {
> +static const qmp_virtio_feature_map_t virtio_config_status_map[] = {
>       FEATURE_ENTRY(VIRTIO_CONFIG_S_DRIVER_OK, \
>               "VIRTIO_CONFIG_S_DRIVER_OK: Driver setup and ready"),
>       FEATURE_ENTRY(VIRTIO_CONFIG_S_FEATURES_OK, \
> @@ -179,7 +179,7 @@ static qmp_virtio_feature_map_t virtio_config_status_map[] = {
>   };
>   
>   /* virtio-blk features mapping */
> -qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
> +const qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
>       FEATURE_ENTRY(VIRTIO_BLK_F_SIZE_MAX, \
>               "VIRTIO_BLK_F_SIZE_MAX: Max segment size is size_max"),
>       FEATURE_ENTRY(VIRTIO_BLK_F_SEG_MAX, \

It appears all of the ones not marked static can be?
Otherwise you should have needed to adjust some header file as well.


r~


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

* Re: [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state
  2022-12-12 23:05 ` [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state Philippe Mathieu-Daudé
@ 2022-12-13  0:14   ` Richard Henderson
  2022-12-13  7:30     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 26+ messages in thread
From: Richard Henderson @ 2022-12-13  0:14 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz

On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:
> The device endianness doesn't change during runtime.

What are you talking about?  Of course it does.

I mean, it doesn't often in practice, because the Linux kernel is compiled for one 
endianness and doesn't keep toggling state, but the hooks that you're replacing test for 
the *current* endianness state of the cpu.  So this is a behaviour change.

Have you considered that the bootloader and the kernel may use different endianness?


r~


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

* Re: [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state
  2022-12-13  0:14   ` Richard Henderson
@ 2022-12-13  7:30     ` Philippe Mathieu-Daudé
  2022-12-13  8:03       ` Thomas Huth
                         ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-13  7:30 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz

On 13/12/22 01:14, Richard Henderson wrote:
> On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:
>> The device endianness doesn't change during runtime.
> 
> What are you talking about?  Of course it does.

The host CPU certainly does, but the virtio device doesn't... Does it?

This check only consider the device, not the CPU:

     bool virtio_access_is_big_endian(VirtIODevice *vdev)
     {
     #if defined(LEGACY_VIRTIO_IS_BIENDIAN)
         return virtio_is_big_endian(vdev);
     #elif TARGET_BIG_ENDIAN
         if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
             /*Devices conforming to VIRTIO 1.0 or later are always LE.*/
             return false;
         }
         return true;
     #else
         return false;
     #endif
     }

     static inline bool virtio_is_big_endian(VirtIODevice *vdev)
     {
         if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
             assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN);
             return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG;
         }
         /* Devices conforming to VIRTIO 1.0 or later are always LE. */
         return false;
     }

and once the features are negotiated it doesn't seem to change.

> I mean, it doesn't often in practice, because the Linux kernel is 
> compiled for one endianness and doesn't keep toggling state, but the 
> hooks that you're replacing test for the *current* endianness state of 
> the cpu.  So this is a behaviour change.

I agree. Note however currently the CPU endianness is only checked once
upon virtio device reset (or from a migration stream):

     void virtio_reset(void *opaque)
     {
         VirtIODevice *vdev = opaque;
         VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
         int i;

         virtio_set_status(vdev, 0);
         if (current_cpu) {
             /* Guest initiated reset */
             vdev->device_endian = virtio_current_cpu_endian();
         } else {
             /* System reset */
             vdev->device_endian = virtio_default_endian();
         }

     bool cpu_virtio_is_big_endian(CPUState *cpu)
     {
         CPUClass *cc = CPU_GET_CLASS(cpu);

         if (cc->sysemu_ops->virtio_is_big_endian) {
             return cc->sysemu_ops->virtio_is_big_endian(cpu);
         }
         return target_words_bigendian();
     }

ARM being the single arch implementing a runtime endianness check:

     static bool arm_cpu_virtio_is_big_endian(CPUState *cs)
     {
         ARMCPU *cpu = ARM_CPU(cs);
         CPUARMState *env = &cpu->env;

         cpu_synchronize_state(cs);
         return arm_cpu_data_is_big_endian(env);
     }

> Have you considered that the bootloader and the kernel may use different 
> endianness?

Certainly, but I'll revisit the code more thoughtfully.

Thanks,

Phil.


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

* Re: [PATCH-for-8.0 03/10] hw/virtio: Constify qmp_virtio_feature_map_t[]
  2022-12-13  0:02   ` Richard Henderson
@ 2022-12-13  7:35     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-13  7:35 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz

On 13/12/22 01:02, Richard Henderson wrote:
> On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:
>> @@ -161,7 +161,7 @@ static qmp_virtio_feature_map_t 
>> vhost_user_protocol_map[] = {
>>   };
>>   /* virtio device configuration statuses */
>> -static qmp_virtio_feature_map_t virtio_config_status_map[] = {
>> +static const qmp_virtio_feature_map_t virtio_config_status_map[] = {
>>       FEATURE_ENTRY(VIRTIO_CONFIG_S_DRIVER_OK, \
>>               "VIRTIO_CONFIG_S_DRIVER_OK: Driver setup and ready"),
>>       FEATURE_ENTRY(VIRTIO_CONFIG_S_FEATURES_OK, \
>> @@ -179,7 +179,7 @@ static qmp_virtio_feature_map_t 
>> virtio_config_status_map[] = {
>>   };
>>   /* virtio-blk features mapping */
>> -qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
>> +const qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
>>       FEATURE_ENTRY(VIRTIO_BLK_F_SIZE_MAX, \
>>               "VIRTIO_BLK_F_SIZE_MAX: Max segment size is size_max"),
>>       FEATURE_ENTRY(VIRTIO_BLK_F_SEG_MAX, \
> 
> It appears all of the ones not marked static can be?

It seems some are not static to avoid "declared static but not used"
warnings due to how they are conditionally used with the
CONFIG_VIRTIO_xxx guards in qmp_decode_features():

     /* device features */
     switch (device_id) {
#ifdef CONFIG_VIRTIO_SERIAL
     case VIRTIO_ID_CONSOLE:
         features->dev_features =
             CONVERT_FEATURES(strList, virtio_serial_feature_map, 0, 
bitmap);
         break;
#endif
#ifdef CONFIG_VIRTIO_BLK
     case VIRTIO_ID_BLOCK:
         features->dev_features =
             CONVERT_FEATURES(strList, virtio_blk_feature_map, 0, bitmap);
         break;
#endif
#ifdef CONFIG_VIRTIO_GPU
     case VIRTIO_ID_GPU:
         features->dev_features =
             CONVERT_FEATURES(strList, virtio_gpu_feature_map, 0, bitmap);
         break;
#endif

> Otherwise you should have needed to adjust some header file as well.

OK I'll guard them with the corresponding #ifdef'ry.


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

* Re: [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state
  2022-12-13  7:30     ` Philippe Mathieu-Daudé
@ 2022-12-13  8:03       ` Thomas Huth
  2022-12-13  8:32         ` Philippe Mathieu-Daudé
  2022-12-13  8:22       ` Philippe Mathieu-Daudé
  2022-12-13 15:41       ` Richard Henderson
  2 siblings, 1 reply; 26+ messages in thread
From: Thomas Huth @ 2022-12-13  8:03 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Richard Henderson, qemu-devel
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz

On 13/12/2022 08.30, Philippe Mathieu-Daudé wrote:
> On 13/12/22 01:14, Richard Henderson wrote:
>> On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:
>>> The device endianness doesn't change during runtime.
>>
>> What are you talking about?  Of course it does.
> 
> The host CPU certainly does, but the virtio device doesn't... Does it?
> 
> This check only consider the device, not the CPU:
> 
>      bool virtio_access_is_big_endian(VirtIODevice *vdev)
>      {
>      #if defined(LEGACY_VIRTIO_IS_BIENDIAN)
>          return virtio_is_big_endian(vdev);
>      #elif TARGET_BIG_ENDIAN
>          if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
>              /*Devices conforming to VIRTIO 1.0 or later are always LE.*/
>              return false;
>          }
>          return true;

Well, this part here means that the endianness can indeed change on the 
device side during runtime. Depending on whether VIRTIO_F_VERSION_1 is 
negotiated or not, the device is little or big endian. Happens on s390x for 
example - for legacy virtio, big endian is used, and for modern virtio, 
little endian is used instead.

  Thomas



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

* Re: [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state
  2022-12-13  7:30     ` Philippe Mathieu-Daudé
  2022-12-13  8:03       ` Thomas Huth
@ 2022-12-13  8:22       ` Philippe Mathieu-Daudé
  2022-12-13 15:41       ` Richard Henderson
  2 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-13  8:22 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz

On 13/12/22 08:30, Philippe Mathieu-Daudé wrote:
> On 13/12/22 01:14, Richard Henderson wrote:
>> On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:
>>> The device endianness doesn't change during runtime.
>>
>> What are you talking about?  Of course it does.
> 
> The host CPU certainly does, but the virtio device doesn't... Does it?
> 
> This check only consider the device, not the CPU:
> 
>      bool virtio_access_is_big_endian(VirtIODevice *vdev)
>      {
>      #if defined(LEGACY_VIRTIO_IS_BIENDIAN)
>          return virtio_is_big_endian(vdev);
>      #elif TARGET_BIG_ENDIAN
>          if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
>              /*Devices conforming to VIRTIO 1.0 or later are always LE.*/
>              return false;
>          }
>          return true;
>      #else
>          return false;
>      #endif
>      }
> 
>      static inline bool virtio_is_big_endian(VirtIODevice *vdev)
>      {
>          if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
>              assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN);
>              return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG;
>          }
>          /* Devices conforming to VIRTIO 1.0 or later are always LE. */
>          return false;
>      }
> 
> and once the features are negotiated it doesn't seem to change.

Per the spec, if the device changes its endianness, it must set the
VIRTIO_CONFIG_S_NEEDS_RESET bit:

   3.2.1 Notification of Device Configuration Changes

   For devices where the device-specific configuration information
   can be changed, a configuration change notification is sent when
   a device-specific configuration change occurs.
   In addition, this notification is triggered by the device setting
   DEVICE_NEEDS_RESET

However QEMU doesn't read this bit, only sets it:

hw/virtio/virtio.c:3551:        vdev->status = vdev->status | 
VIRTIO_CONFIG_S_NEEDS_RESET;
include/standard-headers/linux/virtio_config.h:44:#define 
VIRTIO_CONFIG_S_NEEDS_RESET   0x40

>> I mean, it doesn't often in practice, because the Linux kernel is 
>> compiled for one endianness and doesn't keep toggling state, but the 
>> hooks that you're replacing test for the *current* endianness state of 
>> the cpu.  So this is a behaviour change.
> 
> I agree. Note however currently the CPU endianness is only checked once
> upon virtio device reset (or from a migration stream):
> 
>      void virtio_reset(void *opaque)
>      {
>          VirtIODevice *vdev = opaque;
>          VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
>          int i;
> 
>          virtio_set_status(vdev, 0);
>          if (current_cpu) {
>              /* Guest initiated reset */
>              vdev->device_endian = virtio_current_cpu_endian();
>          } else {
>              /* System reset */
>              vdev->device_endian = virtio_default_endian();
>          }
> 
>      bool cpu_virtio_is_big_endian(CPUState *cpu)
>      {
>          CPUClass *cc = CPU_GET_CLASS(cpu);
> 
>          if (cc->sysemu_ops->virtio_is_big_endian) {
>              return cc->sysemu_ops->virtio_is_big_endian(cpu);
>          }
>          return target_words_bigendian();
>      }
> 
> ARM being the single arch implementing a runtime endianness check:
> 
>      static bool arm_cpu_virtio_is_big_endian(CPUState *cs)
>      {
>          ARMCPU *cpu = ARM_CPU(cs);
>          CPUARMState *env = &cpu->env;
> 
>          cpu_synchronize_state(cs);
>          return arm_cpu_data_is_big_endian(env);
>      }

virtio_reset() is only called by virtio_bus_reset().

$ git grep -w virtio_bus_reset
hw/s390x/virtio-ccw.c:256:    virtio_bus_reset(&dev->bus);
hw/virtio/virtio-bus.c:102:void virtio_bus_reset(VirtioBusState *bus)
hw/virtio/virtio-mmio.c:75:    virtio_bus_reset(&proxy->bus);
hw/virtio/virtio-pci.c:1998:    virtio_bus_reset(bus);

So the result of virtio_access_is_big_endian() is only updated there,
after a virtio_bus_reset() call, and IIUC  isn't dependent on the CPU
endianness state, thus can be cached in VirtIODevice. But maybe the
current implementation is incomplete and my change is simply making
it worst...


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

* Re: [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state
  2022-12-13  8:03       ` Thomas Huth
@ 2022-12-13  8:32         ` Philippe Mathieu-Daudé
  2022-12-13  8:47           ` Thomas Huth
  0 siblings, 1 reply; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-13  8:32 UTC (permalink / raw)
  To: Thomas Huth, Richard Henderson, qemu-devel
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz

On 13/12/22 09:03, Thomas Huth wrote:
> On 13/12/2022 08.30, Philippe Mathieu-Daudé wrote:
>> On 13/12/22 01:14, Richard Henderson wrote:
>>> On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:
>>>> The device endianness doesn't change during runtime.
>>>
>>> What are you talking about?  Of course it does.
>>
>> The host CPU certainly does, but the virtio device doesn't... Does it?
>>
>> This check only consider the device, not the CPU:
>>
>>      bool virtio_access_is_big_endian(VirtIODevice *vdev)
>>      {
>>      #if defined(LEGACY_VIRTIO_IS_BIENDIAN)
>>          return virtio_is_big_endian(vdev);
>>      #elif TARGET_BIG_ENDIAN
>>          if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
>>              /*Devices conforming to VIRTIO 1.0 or later are always LE.*/
>>              return false;
>>          }
>>          return true;
> 
> Well, this part here means that the endianness can indeed change on the 
> device side during runtime. Depending on whether VIRTIO_F_VERSION_1 is 
> negotiated or not, the device is little or big endian. Happens on s390x 
> for example - for legacy virtio, big endian is used, and for modern 
> virtio, little endian is used instead.

virtio_is_big_endian() depends on vdev->device_endian which is set in:

1) virtio_init()

     void virtio_init(VirtIODevice *vdev, uint16_t device_id,
                      size_t config_size)
     {
         ....
         vdev->device_endian = virtio_default_endian();

2) virtio_load()

     int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
     {
         int i, ret;
         int32_t config_len;
         uint32_t num;
         uint32_t features;
         BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
         VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
         VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);

         /*
          * We poison the endianness to ensure it does not get
          * used before subsections have been loaded.
          */
         vdev->device_endian = VIRTIO_DEVICE_ENDIAN_UNKNOWN;
         ....

         if (vdev->device_endian == VIRTIO_DEVICE_ENDIAN_UNKNOWN) {
             vdev->device_endian = virtio_default_endian();
         }

3) virtio_reset()

     void virtio_reset(void *opaque)
     {
         VirtIODevice *vdev = opaque;
         VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
         int i;

         virtio_set_status(vdev, 0);
         if (current_cpu) {
             /* Guest initiated reset */
             vdev->device_endian = virtio_current_cpu_endian();
         } else {
             /* System reset */
             vdev->device_endian = virtio_default_endian();
         }

So the current patch is not complete and should be:

-- >8 --
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 09b1a0e3d9..b02b9058f9 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2124,6 +2124,7 @@ void virtio_reset(void *opaque)
          /* System reset */
          vdev->device_endian = virtio_default_endian();
      }
+    vdev->access_is_big_endian = virtio_access_is_big_endian(vdev);

      if (k->reset) {
          k->reset(vdev);
@@ -3018,6 +3019,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, 
int version_id)

      if (vdev->device_endian == VIRTIO_DEVICE_ENDIAN_UNKNOWN) {
          vdev->device_endian = virtio_default_endian();
+        vdev->access_is_big_endian = virtio_access_is_big_endian(vdev);
      }

      if (virtio_64bit_features_needed(vdev)) {
@@ -3193,6 +3195,7 @@ void virtio_init(VirtIODevice *vdev, uint16_t 
device_id, size_t config_size)
      vdev->vmstate = qdev_add_vm_change_state_handler(DEVICE(vdev),
              virtio_vmstate_change, vdev);
      vdev->device_endian = virtio_default_endian();
+    vdev->access_is_big_endian = virtio_access_is_big_endian(vdev);
      vdev->use_guest_notifier_mask = true;
  }
---

Still, the result of virtio_access_is_big_endian() doesn't depend on
the CPU endianness in my analysis... What am I missing?

Thanks,

Phil.


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

* Re: [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state
  2022-12-13  8:32         ` Philippe Mathieu-Daudé
@ 2022-12-13  8:47           ` Thomas Huth
  0 siblings, 0 replies; 26+ messages in thread
From: Thomas Huth @ 2022-12-13  8:47 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Richard Henderson, qemu-devel
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz

On 13/12/2022 09.32, Philippe Mathieu-Daudé wrote:
> On 13/12/22 09:03, Thomas Huth wrote:
>> On 13/12/2022 08.30, Philippe Mathieu-Daudé wrote:
>>> On 13/12/22 01:14, Richard Henderson wrote:
>>>> On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:
>>>>> The device endianness doesn't change during runtime.
>>>>
>>>> What are you talking about?  Of course it does.
>>>
>>> The host CPU certainly does, but the virtio device doesn't... Does it?
>>>
>>> This check only consider the device, not the CPU:
>>>
>>>      bool virtio_access_is_big_endian(VirtIODevice *vdev)
>>>      {
>>>      #if defined(LEGACY_VIRTIO_IS_BIENDIAN)
>>>          return virtio_is_big_endian(vdev);
>>>      #elif TARGET_BIG_ENDIAN
>>>          if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
>>>              /*Devices conforming to VIRTIO 1.0 or later are always LE.*/
>>>              return false;
>>>          }
>>>          return true;
>>
>> Well, this part here means that the endianness can indeed change on the 
>> device side during runtime. Depending on whether VIRTIO_F_VERSION_1 is 
>> negotiated or not, the device is little or big endian. Happens on s390x 
>> for example - for legacy virtio, big endian is used, and for modern 
>> virtio, little endian is used instead.
> 
> virtio_is_big_endian() depends on vdev->device_endian which is set in:
> 
> 1) virtio_init()
> 
>      void virtio_init(VirtIODevice *vdev, uint16_t device_id,
>                       size_t config_size)
>      {
>          ....
>          vdev->device_endian = virtio_default_endian();
> 
> 2) virtio_load()
> 
>      int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
>      {
>          int i, ret;
>          int32_t config_len;
>          uint32_t num;
>          uint32_t features;
>          BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>          VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>          VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
> 
>          /*
>           * We poison the endianness to ensure it does not get
>           * used before subsections have been loaded.
>           */
>          vdev->device_endian = VIRTIO_DEVICE_ENDIAN_UNKNOWN;
>          ....
> 
>          if (vdev->device_endian == VIRTIO_DEVICE_ENDIAN_UNKNOWN) {
>              vdev->device_endian = virtio_default_endian();
>          }
> 
> 3) virtio_reset()
> 
>      void virtio_reset(void *opaque)
>      {
>          VirtIODevice *vdev = opaque;
>          VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
>          int i;
> 
>          virtio_set_status(vdev, 0);
>          if (current_cpu) {
>              /* Guest initiated reset */
>              vdev->device_endian = virtio_current_cpu_endian();

This is where the virtio endianness depends on the CPU endianness. Looks 
like it is fortunately only taken into account after a device reset, and not 
for every access (as I originally thought).

  Thomas



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

* Re: [RFC PATCH-for-8.0 07/10] hw/virtio: Directly access cached VirtIODevice::access_is_big_endian
  2022-12-12 23:05 ` [RFC PATCH-for-8.0 07/10] hw/virtio: Directly access cached VirtIODevice::access_is_big_endian Philippe Mathieu-Daudé
@ 2022-12-13 10:50   ` Greg Kurz
  0 siblings, 0 replies; 26+ messages in thread
From: Greg Kurz @ 2022-12-13 10:50 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Thomas Huth, Stefan Hajnoczi, Kevin Wolf, qemu-block,
	Paolo Bonzini, Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz

On Tue, 13 Dec 2022 00:05:14 +0100
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> Since the device endianness doesn't change at runtime,
> use the cached value instead of evaluating it on each call.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/hw/virtio/virtio-access.h | 44 +++++++++++++++----------------
>  1 file changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
> index 07aae69042..985f39fe16 100644
> --- a/include/hw/virtio/virtio-access.h
> +++ b/include/hw/virtio/virtio-access.h
> @@ -43,7 +43,7 @@ static inline uint16_t virtio_lduw_phys(VirtIODevice *vdev, hwaddr pa)
>  {
>      AddressSpace *dma_as = vdev->dma_as;
>  
> -    if (virtio_access_is_big_endian(vdev)) {
> +    if (vdev->access_is_big_endian) {

For x86, virtio_access_is_big_endian() expands to :

static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
{
    return false;
}

When I added these memory accessors, there was a strong requirement from MST
that x86 shouldn't have to pay for an out-of-line check when it is known that
everything is always little endian. Not sure exactly what you're trying to
achieve with VirtIODevice::access_is_big_endian but this shouldn't mess with
this fast path.

>          return lduw_be_phys(dma_as, pa);
>      }
>      return lduw_le_phys(dma_as, pa);
> @@ -53,7 +53,7 @@ static inline uint32_t virtio_ldl_phys(VirtIODevice *vdev, hwaddr pa)
>  {
>      AddressSpace *dma_as = vdev->dma_as;
>  
> -    if (virtio_access_is_big_endian(vdev)) {
> +    if (vdev->access_is_big_endian) {
>          return ldl_be_phys(dma_as, pa);
>      }
>      return ldl_le_phys(dma_as, pa);
> @@ -63,7 +63,7 @@ static inline uint64_t virtio_ldq_phys(VirtIODevice *vdev, hwaddr pa)
>  {
>      AddressSpace *dma_as = vdev->dma_as;
>  
> -    if (virtio_access_is_big_endian(vdev)) {
> +    if (vdev->access_is_big_endian) {
>          return ldq_be_phys(dma_as, pa);
>      }
>      return ldq_le_phys(dma_as, pa);
> @@ -74,7 +74,7 @@ static inline void virtio_stw_phys(VirtIODevice *vdev, hwaddr pa,
>  {
>      AddressSpace *dma_as = vdev->dma_as;
>  
> -    if (virtio_access_is_big_endian(vdev)) {
> +    if (vdev->access_is_big_endian) {
>          stw_be_phys(dma_as, pa, value);
>      } else {
>          stw_le_phys(dma_as, pa, value);
> @@ -86,7 +86,7 @@ static inline void virtio_stl_phys(VirtIODevice *vdev, hwaddr pa,
>  {
>      AddressSpace *dma_as = vdev->dma_as;
>  
> -    if (virtio_access_is_big_endian(vdev)) {
> +    if (vdev->access_is_big_endian) {
>          stl_be_phys(dma_as, pa, value);
>      } else {
>          stl_le_phys(dma_as, pa, value);
> @@ -95,7 +95,7 @@ static inline void virtio_stl_phys(VirtIODevice *vdev, hwaddr pa,
>  
>  static inline void virtio_stw_p(VirtIODevice *vdev, void *ptr, uint16_t v)
>  {
> -    if (virtio_access_is_big_endian(vdev)) {
> +    if (vdev->access_is_big_endian) {
>          stw_be_p(ptr, v);
>      } else {
>          stw_le_p(ptr, v);
> @@ -104,7 +104,7 @@ static inline void virtio_stw_p(VirtIODevice *vdev, void *ptr, uint16_t v)
>  
>  static inline void virtio_stl_p(VirtIODevice *vdev, void *ptr, uint32_t v)
>  {
> -    if (virtio_access_is_big_endian(vdev)) {
> +    if (vdev->access_is_big_endian) {
>          stl_be_p(ptr, v);
>      } else {
>          stl_le_p(ptr, v);
> @@ -113,7 +113,7 @@ static inline void virtio_stl_p(VirtIODevice *vdev, void *ptr, uint32_t v)
>  
>  static inline void virtio_stq_p(VirtIODevice *vdev, void *ptr, uint64_t v)
>  {
> -    if (virtio_access_is_big_endian(vdev)) {
> +    if (vdev->access_is_big_endian) {
>          stq_be_p(ptr, v);
>      } else {
>          stq_le_p(ptr, v);
> @@ -122,7 +122,7 @@ static inline void virtio_stq_p(VirtIODevice *vdev, void *ptr, uint64_t v)
>  
>  static inline int virtio_lduw_p(VirtIODevice *vdev, const void *ptr)
>  {
> -    if (virtio_access_is_big_endian(vdev)) {
> +    if (vdev->access_is_big_endian) {
>          return lduw_be_p(ptr);
>      } else {
>          return lduw_le_p(ptr);
> @@ -131,7 +131,7 @@ static inline int virtio_lduw_p(VirtIODevice *vdev, const void *ptr)
>  
>  static inline int virtio_ldl_p(VirtIODevice *vdev, const void *ptr)
>  {
> -    if (virtio_access_is_big_endian(vdev)) {
> +    if (vdev->access_is_big_endian) {
>          return ldl_be_p(ptr);
>      } else {
>          return ldl_le_p(ptr);
> @@ -140,7 +140,7 @@ static inline int virtio_ldl_p(VirtIODevice *vdev, const void *ptr)
>  
>  static inline uint64_t virtio_ldq_p(VirtIODevice *vdev, const void *ptr)
>  {
> -    if (virtio_access_is_big_endian(vdev)) {
> +    if (vdev->access_is_big_endian) {
>          return ldq_be_p(ptr);
>      } else {
>          return ldq_le_p(ptr);
> @@ -150,9 +150,9 @@ static inline uint64_t virtio_ldq_p(VirtIODevice *vdev, const void *ptr)
>  static inline uint16_t virtio_tswap16(VirtIODevice *vdev, uint16_t s)
>  {
>  #if HOST_BIG_ENDIAN
> -    return virtio_access_is_big_endian(vdev) ? s : bswap16(s);
> +    return vdev->access_is_big_endian ? s : bswap16(s);
>  #else
> -    return virtio_access_is_big_endian(vdev) ? bswap16(s) : s;
> +    return vdev->access_is_big_endian ? bswap16(s) : s;
>  #endif
>  }
>  
> @@ -160,7 +160,7 @@ static inline uint16_t virtio_lduw_phys_cached(VirtIODevice *vdev,
>                                                 MemoryRegionCache *cache,
>                                                 hwaddr pa)
>  {
> -    if (virtio_access_is_big_endian(vdev)) {
> +    if (vdev->access_is_big_endian) {
>          return lduw_be_phys_cached(cache, pa);
>      }
>      return lduw_le_phys_cached(cache, pa);
> @@ -170,7 +170,7 @@ static inline uint32_t virtio_ldl_phys_cached(VirtIODevice *vdev,
>                                                MemoryRegionCache *cache,
>                                                hwaddr pa)
>  {
> -    if (virtio_access_is_big_endian(vdev)) {
> +    if (vdev->access_is_big_endian) {
>          return ldl_be_phys_cached(cache, pa);
>      }
>      return ldl_le_phys_cached(cache, pa);
> @@ -180,7 +180,7 @@ static inline uint64_t virtio_ldq_phys_cached(VirtIODevice *vdev,
>                                                MemoryRegionCache *cache,
>                                                hwaddr pa)
>  {
> -    if (virtio_access_is_big_endian(vdev)) {
> +    if (vdev->access_is_big_endian) {
>          return ldq_be_phys_cached(cache, pa);
>      }
>      return ldq_le_phys_cached(cache, pa);
> @@ -190,7 +190,7 @@ static inline void virtio_stw_phys_cached(VirtIODevice *vdev,
>                                            MemoryRegionCache *cache,
>                                            hwaddr pa, uint16_t value)
>  {
> -    if (virtio_access_is_big_endian(vdev)) {
> +    if (vdev->access_is_big_endian) {
>          stw_be_phys_cached(cache, pa, value);
>      } else {
>          stw_le_phys_cached(cache, pa, value);
> @@ -201,7 +201,7 @@ static inline void virtio_stl_phys_cached(VirtIODevice *vdev,
>                                            MemoryRegionCache *cache,
>                                            hwaddr pa, uint32_t value)
>  {
> -    if (virtio_access_is_big_endian(vdev)) {
> +    if (vdev->access_is_big_endian) {
>          stl_be_phys_cached(cache, pa, value);
>      } else {
>          stl_le_phys_cached(cache, pa, value);
> @@ -216,9 +216,9 @@ static inline void virtio_tswap16s(VirtIODevice *vdev, uint16_t *s)
>  static inline uint32_t virtio_tswap32(VirtIODevice *vdev, uint32_t s)
>  {
>  #if HOST_BIG_ENDIAN
> -    return virtio_access_is_big_endian(vdev) ? s : bswap32(s);
> +    return vdev->access_is_big_endian ? s : bswap32(s);
>  #else
> -    return virtio_access_is_big_endian(vdev) ? bswap32(s) : s;
> +    return vdev->access_is_big_endian ? bswap32(s) : s;
>  #endif
>  }
>  
> @@ -230,9 +230,9 @@ static inline void virtio_tswap32s(VirtIODevice *vdev, uint32_t *s)
>  static inline uint64_t virtio_tswap64(VirtIODevice *vdev, uint64_t s)
>  {
>  #if HOST_BIG_ENDIAN
> -    return virtio_access_is_big_endian(vdev) ? s : bswap64(s);
> +    return vdev->access_is_big_endian ? s : bswap64(s);
>  #else
> -    return virtio_access_is_big_endian(vdev) ? bswap64(s) : s;
> +    return vdev->access_is_big_endian ? bswap64(s) : s;
>  #endif
>  }
>  



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

* Re: [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state
  2022-12-13  7:30     ` Philippe Mathieu-Daudé
  2022-12-13  8:03       ` Thomas Huth
  2022-12-13  8:22       ` Philippe Mathieu-Daudé
@ 2022-12-13 15:41       ` Richard Henderson
  2 siblings, 0 replies; 26+ messages in thread
From: Richard Henderson @ 2022-12-13 15:41 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz

On 12/13/22 01:30, Philippe Mathieu-Daudé wrote:
> On 13/12/22 01:14, Richard Henderson wrote:
>> On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:
>>> The device endianness doesn't change during runtime.
>>
>> What are you talking about?  Of course it does.
> 
> The host CPU certainly does, but the virtio device doesn't... Does it?
> 
> This check only consider the device, not the CPU:
> 
>      bool virtio_access_is_big_endian(VirtIODevice *vdev)
>      {
>      #if defined(LEGACY_VIRTIO_IS_BIENDIAN)
>          return virtio_is_big_endian(vdev);

This is set for both ARM and PPC, which checks current cpu endianness in both cases.

> and once the features are negotiated it doesn't seem to change.

Incorrect.


r~


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

* Re: [RFC PATCH-for-8.0 09/10] hw/virtio: Extract vhost_user_ram_slots_max() to vhost-user-target.c
  2022-12-12 23:05 ` [RFC PATCH-for-8.0 09/10] hw/virtio: Extract vhost_user_ram_slots_max() to vhost-user-target.c Philippe Mathieu-Daudé
@ 2025-04-10 12:14   ` Philippe Mathieu-Daudé
  2025-04-10 14:36     ` Pierrick Bouvier
  0 siblings, 1 reply; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-04-10 12:14 UTC (permalink / raw)
  To: qemu-devel, Thomas Huth, Pierrick Bouvier
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz

Hi Pierrick,

On 13/12/22 00:05, Philippe Mathieu-Daudé wrote:
> The current definition of VHOST_USER_MAX_RAM_SLOTS is
> target specific. By converting this definition to a runtime
> vhost_user_ram_slots_max() helper declared in a target
> specific unit, we can have the rest of vhost-user.c target
> independent.
> 
> To avoid variable length array or using the heap to store
> arrays of vhost_user_ram_slots_max() elements, we simply
> declare an array of the biggest VHOST_USER_MAX_RAM_SLOTS,
> and each target uses up to vhost_user_ram_slots_max()
> elements of it. Ensure arrays are big enough by adding an
> assertion in vhost_user_init().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> RFC: Should I add VHOST_USER_MAX_RAM_SLOTS to vhost-user.h
>       or create an internal header for it?
> ---
>   hw/virtio/meson.build          |  1 +
>   hw/virtio/vhost-user-target.c  | 29 +++++++++++++++++++++++++++++
>   hw/virtio/vhost-user.c         | 26 +++++---------------------
>   include/hw/virtio/vhost-user.h |  7 +++++++
>   4 files changed, 42 insertions(+), 21 deletions(-)
>   create mode 100644 hw/virtio/vhost-user-target.c
> 
> diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
> index eb7ee8ea92..bf7e35fa8a 100644
> --- a/hw/virtio/meson.build
> +++ b/hw/virtio/meson.build
> @@ -11,6 +11,7 @@ if have_vhost
>     specific_virtio_ss.add(files('vhost.c', 'vhost-backend.c', 'vhost-iova-tree.c'))
>     if have_vhost_user
>       specific_virtio_ss.add(files('vhost-user.c'))
> +    specific_virtio_ss.add(files('vhost-user-target.c'))
>     endif
>     if have_vhost_vdpa
>       specific_virtio_ss.add(files('vhost-vdpa.c', 'vhost-shadow-virtqueue.c'))
> diff --git a/hw/virtio/vhost-user-target.c b/hw/virtio/vhost-user-target.c
> new file mode 100644
> index 0000000000..6a0d0f53d0
> --- /dev/null
> +++ b/hw/virtio/vhost-user-target.c
> @@ -0,0 +1,29 @@
> +/*
> + * vhost-user target-specific helpers
> + *
> + * Copyright (c) 2013 Virtual Open Systems Sarl.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/virtio/vhost-user.h"
> +
> +#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
> +    defined(TARGET_ARM) || defined(TARGET_ARM_64)
> +#include "hw/acpi/acpi.h"
> +#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
> +#include "hw/ppc/spapr.h"
> +#endif
> +
> +unsigned int vhost_user_ram_slots_max(void)
> +{
> +#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
> +    defined(TARGET_ARM) || defined(TARGET_ARM_64)
> +    return ACPI_MAX_RAM_SLOTS;
> +#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
> +    return SPAPR_MAX_RAM_SLOTS;
> +#else
> +    return 512;

Should vhost_user_ram_slots_max be another TargetInfo field?

> +#endif
> +}
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 8f635844af..21fc176725 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -41,24 +41,7 @@
>   #define VHOST_MEMORY_BASELINE_NREGIONS    8
>   #define VHOST_USER_F_PROTOCOL_FEATURES 30
>   #define VHOST_USER_SLAVE_MAX_FDS     8
> -
> -/*
> - * Set maximum number of RAM slots supported to
> - * the maximum number supported by the target
> - * hardware plaform.
> - */
> -#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
> -    defined(TARGET_ARM) || defined(TARGET_ARM_64)
> -#include "hw/acpi/acpi.h"
> -#define VHOST_USER_MAX_RAM_SLOTS ACPI_MAX_RAM_SLOTS
> -
> -#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
> -#include "hw/ppc/spapr.h"
> -#define VHOST_USER_MAX_RAM_SLOTS SPAPR_MAX_RAM_SLOTS
> -
> -#else
>   #define VHOST_USER_MAX_RAM_SLOTS 512
> -#endif
>   
>   /*
>    * Maximum size of virtio device config space
> @@ -935,7 +918,7 @@ static int vhost_user_add_remove_regions(struct vhost_dev *dev,
>   
>       if (track_ramblocks) {
>           memcpy(u->postcopy_client_bases, shadow_pcb,
> -               sizeof(uint64_t) * VHOST_USER_MAX_RAM_SLOTS);
> +               sizeof(uint64_t) * vhost_user_ram_slots_max());
>           /*
>            * Now we've registered this with the postcopy code, we ack to the
>            * client, because now we're in the position to be able to deal with
> @@ -956,7 +939,7 @@ static int vhost_user_add_remove_regions(struct vhost_dev *dev,
>   err:
>       if (track_ramblocks) {
>           memcpy(u->postcopy_client_bases, shadow_pcb,
> -               sizeof(uint64_t) * VHOST_USER_MAX_RAM_SLOTS);
> +               sizeof(uint64_t) * vhost_user_ram_slots_max());
>       }
>   
>       return ret;
> @@ -1030,7 +1013,7 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev,
>           }
>   
>           memset(u->postcopy_client_bases, 0,
> -               sizeof(uint64_t) * VHOST_USER_MAX_RAM_SLOTS);
> +               sizeof(uint64_t) * vhost_user_ram_slots_max());
>   
>           /*
>            * They're in the same order as the regions that were sent
> @@ -2169,7 +2152,7 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>                   return -EINVAL;
>               }
>   
> -            u->user->memory_slots = MIN(ram_slots, VHOST_USER_MAX_RAM_SLOTS);
> +            u->user->memory_slots = MIN(ram_slots, vhost_user_ram_slots_max());
>           }
>       }
>   
> @@ -2649,6 +2632,7 @@ static void vhost_user_state_destroy(gpointer data)
>   
>   bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp)
>   {
> +    assert(vhost_user_ram_slots_max() <= VHOST_USER_MAX_RAM_SLOTS);
>       if (user->chr) {
>           error_setg(errp, "Cannot initialize vhost-user state");
>           return false;
> diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
> index 191216a74f..e13584ade8 100644
> --- a/include/hw/virtio/vhost-user.h
> +++ b/include/hw/virtio/vhost-user.h
> @@ -86,4 +86,11 @@ void vhost_user_async_close(DeviceState *d,
>                               CharBackend *chardev, struct vhost_dev *vhost,
>                               vu_async_close_fn cb);
>   
> +/**
> + * vhost_user_ram_slots_max()
> + *
> + * Return: maximum number of RAM slots supported by the target hardware plaform.
> + */
> +unsigned int vhost_user_ram_slots_max(void);
> +
>   #endif



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

* Re: [RFC PATCH-for-8.0 09/10] hw/virtio: Extract vhost_user_ram_slots_max() to vhost-user-target.c
  2025-04-10 12:14   ` Philippe Mathieu-Daudé
@ 2025-04-10 14:36     ` Pierrick Bouvier
  2025-04-10 17:21       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 26+ messages in thread
From: Pierrick Bouvier @ 2025-04-10 14:36 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz

On 4/10/25 05:14, Philippe Mathieu-Daudé wrote:
> Hi Pierrick,
> 
> On 13/12/22 00:05, Philippe Mathieu-Daudé wrote:
>> The current definition of VHOST_USER_MAX_RAM_SLOTS is
>> target specific. By converting this definition to a runtime
>> vhost_user_ram_slots_max() helper declared in a target
>> specific unit, we can have the rest of vhost-user.c target
>> independent.
>>
>> To avoid variable length array or using the heap to store
>> arrays of vhost_user_ram_slots_max() elements, we simply
>> declare an array of the biggest VHOST_USER_MAX_RAM_SLOTS,
>> and each target uses up to vhost_user_ram_slots_max()
>> elements of it. Ensure arrays are big enough by adding an
>> assertion in vhost_user_init().
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> RFC: Should I add VHOST_USER_MAX_RAM_SLOTS to vhost-user.h
>>        or create an internal header for it?
>> ---
>>    hw/virtio/meson.build          |  1 +
>>    hw/virtio/vhost-user-target.c  | 29 +++++++++++++++++++++++++++++
>>    hw/virtio/vhost-user.c         | 26 +++++---------------------
>>    include/hw/virtio/vhost-user.h |  7 +++++++
>>    4 files changed, 42 insertions(+), 21 deletions(-)
>>    create mode 100644 hw/virtio/vhost-user-target.c
>>
>> diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
>> index eb7ee8ea92..bf7e35fa8a 100644
>> --- a/hw/virtio/meson.build
>> +++ b/hw/virtio/meson.build
>> @@ -11,6 +11,7 @@ if have_vhost
>>      specific_virtio_ss.add(files('vhost.c', 'vhost-backend.c', 'vhost-iova-tree.c'))
>>      if have_vhost_user
>>        specific_virtio_ss.add(files('vhost-user.c'))
>> +    specific_virtio_ss.add(files('vhost-user-target.c'))
>>      endif
>>      if have_vhost_vdpa
>>        specific_virtio_ss.add(files('vhost-vdpa.c', 'vhost-shadow-virtqueue.c'))
>> diff --git a/hw/virtio/vhost-user-target.c b/hw/virtio/vhost-user-target.c
>> new file mode 100644
>> index 0000000000..6a0d0f53d0
>> --- /dev/null
>> +++ b/hw/virtio/vhost-user-target.c
>> @@ -0,0 +1,29 @@
>> +/*
>> + * vhost-user target-specific helpers
>> + *
>> + * Copyright (c) 2013 Virtual Open Systems Sarl.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/virtio/vhost-user.h"
>> +
>> +#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
>> +    defined(TARGET_ARM) || defined(TARGET_ARM_64)
>> +#include "hw/acpi/acpi.h"
>> +#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
>> +#include "hw/ppc/spapr.h"
>> +#endif
>> +
>> +unsigned int vhost_user_ram_slots_max(void)
>> +{
>> +#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
>> +    defined(TARGET_ARM) || defined(TARGET_ARM_64)
>> +    return ACPI_MAX_RAM_SLOTS;
>> +#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
>> +    return SPAPR_MAX_RAM_SLOTS;
>> +#else
>> +    return 512;
> 
> Should vhost_user_ram_slots_max be another TargetInfo field?
> 

I don't think so, it would be better to transform the existing function 
in something like:

switch (target_current()) {
case TARGET_X86:
case TARGET_ARM:
case TARGET_X86_64:
case TARGET_ARM_64:
	return ACPI_MAX_RAM_SLOTS;
case TARGET PPC:
case TARGET PPC64:
	return SPAPR_MAX_RAM_SLOTS;
default:
	return 512;
}

We should not add anything possible to TargetInfo, just for the sake of 
it. Especially becomes it's hard to follow values set per architecture.
In a case like this, a switch is much more readable and located in one 
place. With a generated jump table, it's quite efficient also.

In my opinion, it's another proof we need to have TARGET_X, and 
target_X() available at runtime.

>> +#endif
>> +}
>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>> index 8f635844af..21fc176725 100644
>> --- a/hw/virtio/vhost-user.c
>> +++ b/hw/virtio/vhost-user.c
>> @@ -41,24 +41,7 @@
>>    #define VHOST_MEMORY_BASELINE_NREGIONS    8
>>    #define VHOST_USER_F_PROTOCOL_FEATURES 30
>>    #define VHOST_USER_SLAVE_MAX_FDS     8
>> -
>> -/*
>> - * Set maximum number of RAM slots supported to
>> - * the maximum number supported by the target
>> - * hardware plaform.
>> - */
>> -#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
>> -    defined(TARGET_ARM) || defined(TARGET_ARM_64)
>> -#include "hw/acpi/acpi.h"
>> -#define VHOST_USER_MAX_RAM_SLOTS ACPI_MAX_RAM_SLOTS
>> -
>> -#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
>> -#include "hw/ppc/spapr.h"
>> -#define VHOST_USER_MAX_RAM_SLOTS SPAPR_MAX_RAM_SLOTS
>> -
>> -#else
>>    #define VHOST_USER_MAX_RAM_SLOTS 512
>> -#endif
>>    
>>    /*
>>     * Maximum size of virtio device config space
>> @@ -935,7 +918,7 @@ static int vhost_user_add_remove_regions(struct vhost_dev *dev,
>>    
>>        if (track_ramblocks) {
>>            memcpy(u->postcopy_client_bases, shadow_pcb,
>> -               sizeof(uint64_t) * VHOST_USER_MAX_RAM_SLOTS);
>> +               sizeof(uint64_t) * vhost_user_ram_slots_max());
>>            /*
>>             * Now we've registered this with the postcopy code, we ack to the
>>             * client, because now we're in the position to be able to deal with
>> @@ -956,7 +939,7 @@ static int vhost_user_add_remove_regions(struct vhost_dev *dev,
>>    err:
>>        if (track_ramblocks) {
>>            memcpy(u->postcopy_client_bases, shadow_pcb,
>> -               sizeof(uint64_t) * VHOST_USER_MAX_RAM_SLOTS);
>> +               sizeof(uint64_t) * vhost_user_ram_slots_max());
>>        }
>>    
>>        return ret;
>> @@ -1030,7 +1013,7 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev,
>>            }
>>    
>>            memset(u->postcopy_client_bases, 0,
>> -               sizeof(uint64_t) * VHOST_USER_MAX_RAM_SLOTS);
>> +               sizeof(uint64_t) * vhost_user_ram_slots_max());
>>    
>>            /*
>>             * They're in the same order as the regions that were sent
>> @@ -2169,7 +2152,7 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>>                    return -EINVAL;
>>                }
>>    
>> -            u->user->memory_slots = MIN(ram_slots, VHOST_USER_MAX_RAM_SLOTS);
>> +            u->user->memory_slots = MIN(ram_slots, vhost_user_ram_slots_max());
>>            }
>>        }
>>    
>> @@ -2649,6 +2632,7 @@ static void vhost_user_state_destroy(gpointer data)
>>    
>>    bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp)
>>    {
>> +    assert(vhost_user_ram_slots_max() <= VHOST_USER_MAX_RAM_SLOTS);
>>        if (user->chr) {
>>            error_setg(errp, "Cannot initialize vhost-user state");
>>            return false;
>> diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
>> index 191216a74f..e13584ade8 100644
>> --- a/include/hw/virtio/vhost-user.h
>> +++ b/include/hw/virtio/vhost-user.h
>> @@ -86,4 +86,11 @@ void vhost_user_async_close(DeviceState *d,
>>                                CharBackend *chardev, struct vhost_dev *vhost,
>>                                vu_async_close_fn cb);
>>    
>> +/**
>> + * vhost_user_ram_slots_max()
>> + *
>> + * Return: maximum number of RAM slots supported by the target hardware plaform.
>> + */
>> +unsigned int vhost_user_ram_slots_max(void);
>> +
>>    #endif
> 


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

* Re: [RFC PATCH-for-8.0 09/10] hw/virtio: Extract vhost_user_ram_slots_max() to vhost-user-target.c
  2025-04-10 14:36     ` Pierrick Bouvier
@ 2025-04-10 17:21       ` Philippe Mathieu-Daudé
  2025-04-10 17:29         ` Pierrick Bouvier
  0 siblings, 1 reply; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-04-10 17:21 UTC (permalink / raw)
  To: Pierrick Bouvier, qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz

On 10/4/25 16:36, Pierrick Bouvier wrote:
> On 4/10/25 05:14, Philippe Mathieu-Daudé wrote:
>> Hi Pierrick,
>>
>> On 13/12/22 00:05, Philippe Mathieu-Daudé wrote:
>>> The current definition of VHOST_USER_MAX_RAM_SLOTS is
>>> target specific. By converting this definition to a runtime
>>> vhost_user_ram_slots_max() helper declared in a target
>>> specific unit, we can have the rest of vhost-user.c target
>>> independent.
>>>
>>> To avoid variable length array or using the heap to store
>>> arrays of vhost_user_ram_slots_max() elements, we simply
>>> declare an array of the biggest VHOST_USER_MAX_RAM_SLOTS,
>>> and each target uses up to vhost_user_ram_slots_max()
>>> elements of it. Ensure arrays are big enough by adding an
>>> assertion in vhost_user_init().
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> RFC: Should I add VHOST_USER_MAX_RAM_SLOTS to vhost-user.h
>>>        or create an internal header for it?
>>> ---
>>>    hw/virtio/meson.build          |  1 +
>>>    hw/virtio/vhost-user-target.c  | 29 +++++++++++++++++++++++++++++
>>>    hw/virtio/vhost-user.c         | 26 +++++---------------------
>>>    include/hw/virtio/vhost-user.h |  7 +++++++
>>>    4 files changed, 42 insertions(+), 21 deletions(-)
>>>    create mode 100644 hw/virtio/vhost-user-target.c
>>>
>>> diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
>>> index eb7ee8ea92..bf7e35fa8a 100644
>>> --- a/hw/virtio/meson.build
>>> +++ b/hw/virtio/meson.build
>>> @@ -11,6 +11,7 @@ if have_vhost
>>>      specific_virtio_ss.add(files('vhost.c', 'vhost-backend.c', 
>>> 'vhost-iova-tree.c'))
>>>      if have_vhost_user
>>>        specific_virtio_ss.add(files('vhost-user.c'))
>>> +    specific_virtio_ss.add(files('vhost-user-target.c'))
>>>      endif
>>>      if have_vhost_vdpa
>>>        specific_virtio_ss.add(files('vhost-vdpa.c', 'vhost-shadow- 
>>> virtqueue.c'))
>>> diff --git a/hw/virtio/vhost-user-target.c b/hw/virtio/vhost-user- 
>>> target.c
>>> new file mode 100644
>>> index 0000000000..6a0d0f53d0
>>> --- /dev/null
>>> +++ b/hw/virtio/vhost-user-target.c
>>> @@ -0,0 +1,29 @@
>>> +/*
>>> + * vhost-user target-specific helpers
>>> + *
>>> + * Copyright (c) 2013 Virtual Open Systems Sarl.
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "hw/virtio/vhost-user.h"
>>> +
>>> +#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
>>> +    defined(TARGET_ARM) || defined(TARGET_ARM_64)
>>> +#include "hw/acpi/acpi.h"
>>> +#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
>>> +#include "hw/ppc/spapr.h"
>>> +#endif
>>> +
>>> +unsigned int vhost_user_ram_slots_max(void)
>>> +{
>>> +#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
>>> +    defined(TARGET_ARM) || defined(TARGET_ARM_64)
>>> +    return ACPI_MAX_RAM_SLOTS;
>>> +#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
>>> +    return SPAPR_MAX_RAM_SLOTS;
>>> +#else
>>> +    return 512;
>>
>> Should vhost_user_ram_slots_max be another TargetInfo field?
>>
> 
> I don't think so, it would be better to transform the existing function 
> in something like:
> 
> switch (target_current()) {
> case TARGET_X86:
> case TARGET_ARM:
> case TARGET_X86_64:
> case TARGET_ARM_64:
>      return ACPI_MAX_RAM_SLOTS;
> case TARGET PPC:
> case TARGET PPC64:
>      return SPAPR_MAX_RAM_SLOTS;
> default:
>      return 512;
> }

Clever, I like it, thanks!


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

* Re: [RFC PATCH-for-8.0 09/10] hw/virtio: Extract vhost_user_ram_slots_max() to vhost-user-target.c
  2025-04-10 17:21       ` Philippe Mathieu-Daudé
@ 2025-04-10 17:29         ` Pierrick Bouvier
  2025-04-11 10:15           ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 26+ messages in thread
From: Pierrick Bouvier @ 2025-04-10 17:29 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz

On 4/10/25 10:21, Philippe Mathieu-Daudé wrote:
> On 10/4/25 16:36, Pierrick Bouvier wrote:
>> On 4/10/25 05:14, Philippe Mathieu-Daudé wrote:
>>> Hi Pierrick,
>>>
>>> On 13/12/22 00:05, Philippe Mathieu-Daudé wrote:
>>>> The current definition of VHOST_USER_MAX_RAM_SLOTS is
>>>> target specific. By converting this definition to a runtime
>>>> vhost_user_ram_slots_max() helper declared in a target
>>>> specific unit, we can have the rest of vhost-user.c target
>>>> independent.
>>>>
>>>> To avoid variable length array or using the heap to store
>>>> arrays of vhost_user_ram_slots_max() elements, we simply
>>>> declare an array of the biggest VHOST_USER_MAX_RAM_SLOTS,
>>>> and each target uses up to vhost_user_ram_slots_max()
>>>> elements of it. Ensure arrays are big enough by adding an
>>>> assertion in vhost_user_init().
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>> RFC: Should I add VHOST_USER_MAX_RAM_SLOTS to vhost-user.h
>>>>         or create an internal header for it?
>>>> ---
>>>>     hw/virtio/meson.build          |  1 +
>>>>     hw/virtio/vhost-user-target.c  | 29 +++++++++++++++++++++++++++++
>>>>     hw/virtio/vhost-user.c         | 26 +++++---------------------
>>>>     include/hw/virtio/vhost-user.h |  7 +++++++
>>>>     4 files changed, 42 insertions(+), 21 deletions(-)
>>>>     create mode 100644 hw/virtio/vhost-user-target.c
>>>>
>>>> diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
>>>> index eb7ee8ea92..bf7e35fa8a 100644
>>>> --- a/hw/virtio/meson.build
>>>> +++ b/hw/virtio/meson.build
>>>> @@ -11,6 +11,7 @@ if have_vhost
>>>>       specific_virtio_ss.add(files('vhost.c', 'vhost-backend.c',
>>>> 'vhost-iova-tree.c'))
>>>>       if have_vhost_user
>>>>         specific_virtio_ss.add(files('vhost-user.c'))
>>>> +    specific_virtio_ss.add(files('vhost-user-target.c'))
>>>>       endif
>>>>       if have_vhost_vdpa
>>>>         specific_virtio_ss.add(files('vhost-vdpa.c', 'vhost-shadow-
>>>> virtqueue.c'))
>>>> diff --git a/hw/virtio/vhost-user-target.c b/hw/virtio/vhost-user-
>>>> target.c
>>>> new file mode 100644
>>>> index 0000000000..6a0d0f53d0
>>>> --- /dev/null
>>>> +++ b/hw/virtio/vhost-user-target.c
>>>> @@ -0,0 +1,29 @@
>>>> +/*
>>>> + * vhost-user target-specific helpers
>>>> + *
>>>> + * Copyright (c) 2013 Virtual Open Systems Sarl.
>>>> + *
>>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>>> + */
>>>> +
>>>> +#include "qemu/osdep.h"
>>>> +#include "hw/virtio/vhost-user.h"
>>>> +
>>>> +#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
>>>> +    defined(TARGET_ARM) || defined(TARGET_ARM_64)
>>>> +#include "hw/acpi/acpi.h"
>>>> +#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
>>>> +#include "hw/ppc/spapr.h"
>>>> +#endif
>>>> +
>>>> +unsigned int vhost_user_ram_slots_max(void)
>>>> +{
>>>> +#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
>>>> +    defined(TARGET_ARM) || defined(TARGET_ARM_64)
>>>> +    return ACPI_MAX_RAM_SLOTS;
>>>> +#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
>>>> +    return SPAPR_MAX_RAM_SLOTS;
>>>> +#else
>>>> +    return 512;
>>>
>>> Should vhost_user_ram_slots_max be another TargetInfo field?
>>>
>>
>> I don't think so, it would be better to transform the existing function
>> in something like:
>>
>> switch (target_current()) {
>> case TARGET_X86:
>> case TARGET_ARM:
>> case TARGET_X86_64:
>> case TARGET_ARM_64:
>>       return ACPI_MAX_RAM_SLOTS;
>> case TARGET PPC:
>> case TARGET PPC64:
>>       return SPAPR_MAX_RAM_SLOTS;
>> default:
>>       return 512;
>> }
> 
> Clever, I like it, thanks!

It's a pattern we can reuse in all places where it'll be needed.
It's better if we keep in TargetInfo only global information, that is 
used through all the codebase, and not specifics about a given 
subsystem/device/file.

By the way, TARGET_ARM_64 is probably TARGET_AARCH64.

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

* Re: [RFC PATCH-for-8.0 09/10] hw/virtio: Extract vhost_user_ram_slots_max() to vhost-user-target.c
  2025-04-10 17:29         ` Pierrick Bouvier
@ 2025-04-11 10:15           ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-04-11 10:15 UTC (permalink / raw)
  To: Pierrick Bouvier, qemu-devel, Thomas Huth
  Cc: Greg Kurz, Stefan Hajnoczi, Kevin Wolf, qemu-block, Paolo Bonzini,
	Michael S. Tsirkin, Alex Bennée, Jason Wang,
	Marc-André Lureau, Christian Schoenebeck, Hanna Reitz,
	Akihiko Odaki

On 10/4/25 19:29, Pierrick Bouvier wrote:
> On 4/10/25 10:21, Philippe Mathieu-Daudé wrote:
>> On 10/4/25 16:36, Pierrick Bouvier wrote:
>>> On 4/10/25 05:14, Philippe Mathieu-Daudé wrote:
>>>> Hi Pierrick,
>>>>
>>>> On 13/12/22 00:05, Philippe Mathieu-Daudé wrote:
>>>>> The current definition of VHOST_USER_MAX_RAM_SLOTS is
>>>>> target specific. By converting this definition to a runtime
>>>>> vhost_user_ram_slots_max() helper declared in a target
>>>>> specific unit, we can have the rest of vhost-user.c target
>>>>> independent.
>>>>>
>>>>> To avoid variable length array or using the heap to store
>>>>> arrays of vhost_user_ram_slots_max() elements, we simply
>>>>> declare an array of the biggest VHOST_USER_MAX_RAM_SLOTS,
>>>>> and each target uses up to vhost_user_ram_slots_max()
>>>>> elements of it. Ensure arrays are big enough by adding an
>>>>> assertion in vhost_user_init().
>>>>>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>> ---
>>>>> RFC: Should I add VHOST_USER_MAX_RAM_SLOTS to vhost-user.h
>>>>>         or create an internal header for it?
>>>>> ---
>>>>>     hw/virtio/meson.build          |  1 +
>>>>>     hw/virtio/vhost-user-target.c  | 29 +++++++++++++++++++++++++++++
>>>>>     hw/virtio/vhost-user.c         | 26 +++++---------------------
>>>>>     include/hw/virtio/vhost-user.h |  7 +++++++
>>>>>     4 files changed, 42 insertions(+), 21 deletions(-)
>>>>>     create mode 100644 hw/virtio/vhost-user-target.c
>>>>>
>>>>> diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
>>>>> index eb7ee8ea92..bf7e35fa8a 100644
>>>>> --- a/hw/virtio/meson.build
>>>>> +++ b/hw/virtio/meson.build
>>>>> @@ -11,6 +11,7 @@ if have_vhost
>>>>>       specific_virtio_ss.add(files('vhost.c', 'vhost-backend.c',
>>>>> 'vhost-iova-tree.c'))
>>>>>       if have_vhost_user
>>>>>         specific_virtio_ss.add(files('vhost-user.c'))
>>>>> +    specific_virtio_ss.add(files('vhost-user-target.c'))
>>>>>       endif
>>>>>       if have_vhost_vdpa
>>>>>         specific_virtio_ss.add(files('vhost-vdpa.c', 'vhost-shadow-
>>>>> virtqueue.c'))
>>>>> diff --git a/hw/virtio/vhost-user-target.c b/hw/virtio/vhost-user-
>>>>> target.c
>>>>> new file mode 100644
>>>>> index 0000000000..6a0d0f53d0
>>>>> --- /dev/null
>>>>> +++ b/hw/virtio/vhost-user-target.c
>>>>> @@ -0,0 +1,29 @@
>>>>> +/*
>>>>> + * vhost-user target-specific helpers
>>>>> + *
>>>>> + * Copyright (c) 2013 Virtual Open Systems Sarl.
>>>>> + *
>>>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>>>> + */
>>>>> +
>>>>> +#include "qemu/osdep.h"
>>>>> +#include "hw/virtio/vhost-user.h"
>>>>> +
>>>>> +#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
>>>>> +    defined(TARGET_ARM) || defined(TARGET_ARM_64)
>>>>> +#include "hw/acpi/acpi.h"
>>>>> +#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
>>>>> +#include "hw/ppc/spapr.h"
>>>>> +#endif
>>>>> +
>>>>> +unsigned int vhost_user_ram_slots_max(void)
>>>>> +{
>>>>> +#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
>>>>> +    defined(TARGET_ARM) || defined(TARGET_ARM_64)
>>>>> +    return ACPI_MAX_RAM_SLOTS;
>>>>> +#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
>>>>> +    return SPAPR_MAX_RAM_SLOTS;
>>>>> +#else
>>>>> +    return 512;
>>>>
>>>> Should vhost_user_ram_slots_max be another TargetInfo field?
>>>>
>>>
>>> I don't think so, it would be better to transform the existing function
>>> in something like:
>>>
>>> switch (target_current()) {
>>> case TARGET_X86:
>>> case TARGET_ARM:
>>> case TARGET_X86_64:
>>> case TARGET_ARM_64:
>>>       return ACPI_MAX_RAM_SLOTS;
>>> case TARGET PPC:
>>> case TARGET PPC64:
>>>       return SPAPR_MAX_RAM_SLOTS;
>>> default:
>>>       return 512;
>>> }
>>
>> Clever, I like it, thanks!
> 
> It's a pattern we can reuse in all places where it'll be needed.
> It's better if we keep in TargetInfo only global information, that is 
> used through all the codebase, and not specifics about a given 
> subsystem/device/file.
> 
> By the way, TARGET_ARM_64 is probably TARGET_AARCH64.

Correct, it has been fixed by Akihiko:

commit 744734ccc9eff28394a453de462b2a155f364118
Author: Akihiko Odaki <akihiko.odaki@daynix.com>
Date:   Mon Jan 9 15:31:30 2023 +0900

     vhost-user: Correct a reference of TARGET_AARCH64

     Presumably TARGET_ARM_64 should be a mistake of TARGET_AARCH64.

     Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
     Message-Id: <20230109063130.81296-1-akihiko.odaki@daynix.com>
     Fixes: 27598393a2 ("Lift max memory slots limit imposed by vhost-user")
     Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
     Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
     Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
     Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index d9ce0501b2c..6c79da953b3 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -48,7 +48,7 @@
   * hardware plaform.
   */
  #if defined(TARGET_X86) || defined(TARGET_X86_64) || \
-    defined(TARGET_ARM) || defined(TARGET_ARM_64)
+    defined(TARGET_ARM) || defined(TARGET_AARCH64)
  #include "hw/acpi/acpi.h"
  #define VHOST_USER_MAX_RAM_SLOTS ACPI_MAX_RAM_SLOTS




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

end of thread, other threads:[~2025-04-11 10:16 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-12 23:05 [RFC PATCH-for-8.0 00/10] hw/virtio: Build most objects as target independent units Philippe Mathieu-Daudé
2022-12-12 23:05 ` [PATCH-for-8.0 01/10] hw/virtio: Add missing "hw/core/cpu.h" include Philippe Mathieu-Daudé
2022-12-12 23:05 ` [PATCH-for-8.0 02/10] hw/virtio: Rename virtio_ss[] -> specific_virtio_ss[] Philippe Mathieu-Daudé
2022-12-12 23:05 ` [PATCH-for-8.0 03/10] hw/virtio: Constify qmp_virtio_feature_map_t[] Philippe Mathieu-Daudé
2022-12-13  0:02   ` Richard Henderson
2022-12-13  7:35     ` Philippe Mathieu-Daudé
2022-12-12 23:05 ` [PATCH-for-8.0 04/10] hw/virtio: Extract config read/write accessors to virtio-config.c Philippe Mathieu-Daudé
2022-12-12 23:05 ` [PATCH-for-8.0 05/10] hw/virtio: Extract QMP related code virtio-qmp.c Philippe Mathieu-Daudé
2022-12-12 23:05 ` [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state Philippe Mathieu-Daudé
2022-12-13  0:14   ` Richard Henderson
2022-12-13  7:30     ` Philippe Mathieu-Daudé
2022-12-13  8:03       ` Thomas Huth
2022-12-13  8:32         ` Philippe Mathieu-Daudé
2022-12-13  8:47           ` Thomas Huth
2022-12-13  8:22       ` Philippe Mathieu-Daudé
2022-12-13 15:41       ` Richard Henderson
2022-12-12 23:05 ` [RFC PATCH-for-8.0 07/10] hw/virtio: Directly access cached VirtIODevice::access_is_big_endian Philippe Mathieu-Daudé
2022-12-13 10:50   ` Greg Kurz
2022-12-12 23:05 ` [PATCH-for-8.0 08/10] hw/virtio: Un-inline virtio_access_is_big_endian() Philippe Mathieu-Daudé
2022-12-12 23:05 ` [RFC PATCH-for-8.0 09/10] hw/virtio: Extract vhost_user_ram_slots_max() to vhost-user-target.c Philippe Mathieu-Daudé
2025-04-10 12:14   ` Philippe Mathieu-Daudé
2025-04-10 14:36     ` Pierrick Bouvier
2025-04-10 17:21       ` Philippe Mathieu-Daudé
2025-04-10 17:29         ` Pierrick Bouvier
2025-04-11 10:15           ` Philippe Mathieu-Daudé
2022-12-12 23:05 ` [RFC PATCH-for-8.0 10/10] hw/virtio: Make most of virtio devices target-independent Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).