[PATCH v3 00/11] single-binary: hw/virtio

[PATCH v3 00/11] single-binary: hw/virtio

[Philippe Mathieu-Daudé]

This series eliminates target specifics in hw/virtio and replace them
with runtime functions where needed.

v3:
- include missing SPAPR_MAX_RAM_SLOTS patch
v2:
- Reduce runtime calls for performance
v1:
https://lore.kernel.org/qemu-devel/20260131020100.1115203-1-pierrick.bouvier@linaro.org/

Philippe Mathieu-Daudé (6):
  hw/virtio: Constify virtio_is_big_endian() argument
  hw/virtio: Introduce VirtIODevice::access_is_big_endian boolean field
  hw/virtio: Use VirtIODevice::access_is_big_endian field
  hw/virtio: Reduce virtio_access_is_big_endian() scope
  hw/virtio: Check target supports legacy bi-endianness at runtime
  hw/virtio: Replace TARGET_BIG_ENDIAN -> target_big_endian()

Pierrick Bouvier (5):
  target-info: add target_base_ppc, target_ppc and target_ppc64
  hw/ppc/spapr: extract SPAPR_MAX_RAM_SLOTS in a new header
  hw/virtio/vhost-user: make compilation unit common
  hw/virtio/virtio-qmp: make compilation unit common
  hw/virtio: make all compilation units common

 include/hw/ppc/spapr.h            |  8 +---
 include/hw/ppc/spapr_common.h     | 17 ++++++++
 include/hw/virtio/virtio-access.h | 46 ++++++--------------
 include/hw/virtio/virtio.h        |  7 +++-
 include/qemu/target-info.h        | 21 ++++++++++
 hw/virtio/vhost-user.c            | 11 ++---
 hw/virtio/virtio-qmp.c            | 70 -------------------------------
 hw/virtio/virtio.c                | 25 ++++++++++-
 target-info.c                     | 21 ++++++++++
 hw/virtio/meson.build             | 17 ++++----
 10 files changed, 113 insertions(+), 130 deletions(-)
 create mode 100644 include/hw/ppc/spapr_common.h

-- 
2.52.0

[PATCH v3 01/11] target-info: add target_base_ppc, target_ppc and target_ppc64

[Philippe Mathieu-Daudé]

From: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/qemu/target-info.h | 21 +++++++++++++++++++++
 target-info.c              | 21 +++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/include/qemu/target-info.h b/include/qemu/target-info.h
index 62359622232..e3287334304 100644
--- a/include/qemu/target-info.h
+++ b/include/qemu/target-info.h
@@ -71,4 +71,25 @@ bool target_arm(void);
  */
 bool target_aarch64(void);
 
+/**
+ * target_base_ppc:
+ *
+ * Returns whether the target architecture is PowerPC 32-bit or 64-bit.
+ */
+bool target_base_ppc(void);
+
+/**
+ * target_ppc:
+ *
+ * Returns whether the target architecture is PowerPC 32-bit.
+ */
+bool target_ppc(void);
+
+/**
+ * target_ppc64:
+ *
+ * Returns whether the target architecture is PowerPC 64-bit.
+ */
+bool target_ppc64(void);
+
 #endif
diff --git a/target-info.c b/target-info.c
index 24696ff4111..5a6d7282524 100644
--- a/target-info.c
+++ b/target-info.c
@@ -73,3 +73,24 @@ bool target_aarch64(void)
 {
     return target_arch() == SYS_EMU_TARGET_AARCH64;
 }
+
+bool target_base_ppc(void)
+{
+    switch (target_arch()) {
+    case SYS_EMU_TARGET_PPC:
+    case SYS_EMU_TARGET_PPC64:
+        return true;
+    default:
+        return false;
+    }
+}
+
+bool target_ppc(void)
+{
+    return target_arch() == SYS_EMU_TARGET_PPC;
+}
+
+bool target_ppc64(void)
+{
+    return target_arch() == SYS_EMU_TARGET_PPC64;
+}
-- 
2.52.0

[PATCH v3 02/11] hw/virtio: Constify virtio_is_big_endian() argument

[Philippe Mathieu-Daudé]

VirtIODevice argument is accessed read-only, make it const.

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

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index fcaa9b03b4a..17b105c146d 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -468,7 +468,7 @@ static inline bool virtio_host_has_feature(VirtIODevice *vdev,
     return virtio_has_feature(vdev->host_features, fbit);
 }
 
-static inline bool virtio_is_big_endian(VirtIODevice *vdev)
+static inline bool virtio_is_big_endian(const VirtIODevice *vdev)
 {
     if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
         assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN);
-- 
2.52.0

[PATCH v3 03/11] hw/virtio: Introduce VirtIODevice::access_is_big_endian boolean field

[Philippe Mathieu-Daudé]

VirtIODevice::access_is_big_endian boolean field, initialized on
device reset, represents whether load/store accesses are ordered
using big endianness.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/virtio/virtio.h | 5 +++++
 hw/virtio/virtio.c         | 1 +
 2 files changed, 6 insertions(+)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 17b105c146d..7a4f7de7475 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -154,6 +154,11 @@ struct VirtIODevice
     VMChangeStateEntry *vmstate;
     char *bus_name;
     uint8_t device_endian;
+    /**
+     * @access_is_big_endian: whether load/store memory accesses are ordered
+     * using big endianness. Might change during device resets.
+     */
+    bool access_is_big_endian;
     /**
      * @user_guest_notifier_mask: gate usage of ->guest_notifier_mask() callback.
      * This is used to suppress the masking of guest updates for
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 86fc0e262f7..242c207a591 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3247,6 +3247,7 @@ void virtio_reset(VirtIODevice *vdev)
         /* System reset */
         vdev->device_endian = virtio_default_endian();
     }
+    vdev->access_is_big_endian = virtio_access_is_big_endian(vdev);
 
     if (k->get_vhost) {
         struct vhost_dev *hdev = k->get_vhost(vdev);
-- 
2.52.0

Re: [PATCH v3 03/11] hw/virtio: Introduce VirtIODevice::access_is_big_endian boolean field

[Philippe Mathieu-Daudé]

On 2/2/26 00:29, Philippe Mathieu-Daudé wrote:
> VirtIODevice::access_is_big_endian boolean field, initialized on
> device reset, represents whether load/store accesses are ordered
> using big endianness.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/virtio/virtio.h | 5 +++++
>   hw/virtio/virtio.c         | 1 +
>   2 files changed, 6 insertions(+)
> 
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 17b105c146d..7a4f7de7475 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -154,6 +154,11 @@ struct VirtIODevice
>       VMChangeStateEntry *vmstate;
>       char *bus_name;
>       uint8_t device_endian;
> +    /**
> +     * @access_is_big_endian: whether load/store memory accesses are ordered
> +     * using big endianness. Might change during device resets.
> +     */
> +    bool access_is_big_endian;
>       /**
>        * @user_guest_notifier_mask: gate usage of ->guest_notifier_mask() callback.
>        * This is used to suppress the masking of guest updates for
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 86fc0e262f7..242c207a591 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3247,6 +3247,7 @@ void virtio_reset(VirtIODevice *vdev)
>           /* System reset */
>           vdev->device_endian = virtio_default_endian();

IIUC vdev->device_endian is only used in vhost_needs_vring_endian().

>       }
> +    vdev->access_is_big_endian = virtio_access_is_big_endian(vdev);
>   
>       if (k->get_vhost) {
>           struct vhost_dev *hdev = k->get_vhost(vdev);

[PATCH v3 04/11] hw/virtio: Use VirtIODevice::access_is_big_endian field

[Philippe Mathieu-Daudé]

Endianness access is constant between device resets.
Use the field instead of calling the same function.

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

diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
index cd17d0c87eb..f3b4d0075b5 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -42,7 +42,7 @@ static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
 
 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);
@@ -51,7 +51,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);
@@ -60,7 +60,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);
@@ -69,7 +69,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);
@@ -78,7 +78,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);
@@ -87,7 +87,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);
@@ -97,9 +97,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
 }
 
@@ -111,9 +111,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
 }
 
@@ -125,9 +125,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
 }
 
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 242c207a591..1dc60d37cb4 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -220,7 +220,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);
@@ -230,7 +230,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);
-- 
2.52.0

Re: [PATCH v3 04/11] hw/virtio: Use VirtIODevice::access_is_big_endian field

[Michael S. Tsirkin]

On Mon, Feb 02, 2026 at 12:29:16AM +0100, Philippe Mathieu-Daudé wrote:
> Endianness access is constant between device resets.
> Use the field instead of calling the same function.

yes but the function was inlined and often a NOP. worth mentioning.

> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/hw/virtio/virtio-access.h | 24 ++++++++++++------------
>  hw/virtio/virtio.c                |  4 ++--
>  2 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
> index cd17d0c87eb..f3b4d0075b5 100644
> --- a/include/hw/virtio/virtio-access.h
> +++ b/include/hw/virtio/virtio-access.h
> @@ -42,7 +42,7 @@ static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
>  
>  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);


So this is the main extra cost: on an LE host, we now have this branch on every
access where previously it was all optimized out.

Is it measureable? It is worth testing.



> @@ -51,7 +51,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);
> @@ -60,7 +60,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);
> @@ -69,7 +69,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);

and this one.


> @@ -78,7 +78,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);
> @@ -87,7 +87,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);
> @@ -97,9 +97,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
>  }
>  
> @@ -111,9 +111,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
>  }
>  
> @@ -125,9 +125,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
>  }
>  
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 242c207a591..1dc60d37cb4 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -220,7 +220,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);
> @@ -230,7 +230,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);
> -- 
> 2.52.0

Re: [PATCH v3 04/11] hw/virtio: Use VirtIODevice::access_is_big_endian field

[Alex Bennée]

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

> On Mon, Feb 02, 2026 at 12:29:16AM +0100, Philippe Mathieu-Daudé wrote:
>> Endianness access is constant between device resets.
>> Use the field instead of calling the same function.
>
> yes but the function was inlined and often a NOP. worth mentioning.
>
>> 
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>  include/hw/virtio/virtio-access.h | 24 ++++++++++++------------
>>  hw/virtio/virtio.c                |  4 ++--
>>  2 files changed, 14 insertions(+), 14 deletions(-)
>> 
>> diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
>> index cd17d0c87eb..f3b4d0075b5 100644
>> --- a/include/hw/virtio/virtio-access.h
>> +++ b/include/hw/virtio/virtio-access.h
>> @@ -42,7 +42,7 @@ static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
>>  
>>  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);
>
>
> So this is the main extra cost: on an LE host, we now have this branch on every
> access where previously it was all optimized out.
>
> Is it measureable? It is worth testing.

I suspect not given this is a well predicted branch on the control
plane. But we can certainly try an measure it. Do we have any virtio
performance benchmarks?

We only have a few in meson:

  ./pyvenv/bin/meson test --benchmark --list
  bufferiszero-bench
  benchmark-crypto-hash
  benchmark-crypto-hmac
  benchmark-crypto-cipher
  benchmark-crypto-akcipher

>
>
>
>> @@ -51,7 +51,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);
>> @@ -60,7 +60,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);
>> @@ -69,7 +69,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);
>
> and this one.
>
>
>> @@ -78,7 +78,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);
>> @@ -87,7 +87,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);
>> @@ -97,9 +97,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
>>  }
>>  
>> @@ -111,9 +111,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
>>  }
>>  
>> @@ -125,9 +125,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
>>  }
>>  
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index 242c207a591..1dc60d37cb4 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -220,7 +220,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);
>> @@ -230,7 +230,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);
>> -- 
>> 2.52.0

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v3 04/11] hw/virtio: Use VirtIODevice::access_is_big_endian field

[Michael S. Tsirkin]

On Mon, Feb 02, 2026 at 01:08:25PM +0000, Alex Bennée wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Mon, Feb 02, 2026 at 12:29:16AM +0100, Philippe Mathieu-Daudé wrote:
> >> Endianness access is constant between device resets.
> >> Use the field instead of calling the same function.
> >
> > yes but the function was inlined and often a NOP. worth mentioning.
> >
> >> 
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >> ---
> >>  include/hw/virtio/virtio-access.h | 24 ++++++++++++------------
> >>  hw/virtio/virtio.c                |  4 ++--
> >>  2 files changed, 14 insertions(+), 14 deletions(-)
> >> 
> >> diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
> >> index cd17d0c87eb..f3b4d0075b5 100644
> >> --- a/include/hw/virtio/virtio-access.h
> >> +++ b/include/hw/virtio/virtio-access.h
> >> @@ -42,7 +42,7 @@ static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
> >>  
> >>  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);
> >
> >
> > So this is the main extra cost: on an LE host, we now have this branch on every
> > access where previously it was all optimized out.
> >
> > Is it measureable? It is worth testing.
> 
> I suspect not given this is a well predicted branch on the control
> plane.

Actually you are right. I meant the cached variants below. Not this one.





> But we can certainly try an measure it. Do we have any virtio
> performance benchmarks?
> 
> We only have a few in meson:
> 
>   ./pyvenv/bin/meson test --benchmark --list
>   bufferiszero-bench
>   benchmark-crypto-hash
>   benchmark-crypto-hmac
>   benchmark-crypto-cipher
>   benchmark-crypto-akcipher

I usually use fio but whatever blk maintainers use is fine by me.


> >
> >
> >
> >> @@ -51,7 +51,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);
> >> @@ -60,7 +60,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);
> >> @@ -69,7 +69,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);
> >
> > and this one.
> >
> >
> >> @@ -78,7 +78,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);
> >> @@ -87,7 +87,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);
> >> @@ -97,9 +97,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
> >>  }
> >>  
> >> @@ -111,9 +111,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
> >>  }
> >>  
> >> @@ -125,9 +125,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
> >>  }
> >>  
> >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >> index 242c207a591..1dc60d37cb4 100644
> >> --- a/hw/virtio/virtio.c
> >> +++ b/hw/virtio/virtio.c
> >> @@ -220,7 +220,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);
> >> @@ -230,7 +230,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);
> >> -- 
> >> 2.52.0


I meant these ones.

> -- 
> Alex Bennée
> Virtualisation Tech Lead @ Linaro

Re: [PATCH v3 04/11] hw/virtio: Use VirtIODevice::access_is_big_endian field

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 8126 bytes --]

On Mon, Feb 02, 2026 at 11:04:08AM -0500, Michael S. Tsirkin wrote:
> On Mon, Feb 02, 2026 at 01:08:25PM +0000, Alex Bennée wrote:
> > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > 
> > > On Mon, Feb 02, 2026 at 12:29:16AM +0100, Philippe Mathieu-Daudé wrote:
> > >> Endianness access is constant between device resets.
> > >> Use the field instead of calling the same function.
> > >
> > > yes but the function was inlined and often a NOP. worth mentioning.
> > >
> > >> 
> > >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > >> ---
> > >>  include/hw/virtio/virtio-access.h | 24 ++++++++++++------------
> > >>  hw/virtio/virtio.c                |  4 ++--
> > >>  2 files changed, 14 insertions(+), 14 deletions(-)
> > >> 
> > >> diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
> > >> index cd17d0c87eb..f3b4d0075b5 100644
> > >> --- a/include/hw/virtio/virtio-access.h
> > >> +++ b/include/hw/virtio/virtio-access.h
> > >> @@ -42,7 +42,7 @@ static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
> > >>  
> > >>  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);
> > >
> > >
> > > So this is the main extra cost: on an LE host, we now have this branch on every
> > > access where previously it was all optimized out.
> > >
> > > Is it measureable? It is worth testing.
> > 
> > I suspect not given this is a well predicted branch on the control
> > plane.
> 
> Actually you are right. I meant the cached variants below. Not this one.
> 
> 
> 
> 
> 
> > But we can certainly try an measure it. Do we have any virtio
> > performance benchmarks?
> > 
> > We only have a few in meson:
> > 
> >   ./pyvenv/bin/meson test --benchmark --list
> >   bufferiszero-bench
> >   benchmark-crypto-hash
> >   benchmark-crypto-hmac
> >   benchmark-crypto-cipher
> >   benchmark-crypto-akcipher
> 
> I usually use fio but whatever blk maintainers use is fine by me.

This command-line lets you benchmark virtio-blk without actual I/O
slowing down the request processing:

  qemu-system-x86_64 \
      -M accel=kvm \
      -cpu host \
      -m 4G \
      --blockdev file,node-name=drive0,filename=boot.img,cache.direct=on,aio=native \
      --blockdev null-co,node-name=drive1,size=$((10 * 1024 * 1024 * 1024)) \
      --object iothread,id=iothread0 \
      --device virtio-blk-pci,drive=drive0,iothread=iothread0 \
      --device virtio-blk-pci,drive=drive1,iothread=iothread0

Here is a fio command-line for 4 KiB random reads:

  fio \
      --ioengine=libaio \
      --direct=1 \
      --runtime=30 \
      --ramp_time=10 \
      --rw=randread \
      --bs=4k \
      --iodepth=128 \
      --filename=/dev/vdb \
      --name=randread

This is just a single vCPU, but it should be enough to see if there is
any difference in I/O Operations Per Second (IOPS) or efficiency
(IOPS/CPU utilization).

> 
> 
> > >
> > >
> > >
> > >> @@ -51,7 +51,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);
> > >> @@ -60,7 +60,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);
> > >> @@ -69,7 +69,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);
> > >
> > > and this one.
> > >
> > >
> > >> @@ -78,7 +78,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);
> > >> @@ -87,7 +87,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);
> > >> @@ -97,9 +97,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
> > >>  }
> > >>  
> > >> @@ -111,9 +111,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
> > >>  }
> > >>  
> > >> @@ -125,9 +125,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
> > >>  }
> > >>  
> > >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > >> index 242c207a591..1dc60d37cb4 100644
> > >> --- a/hw/virtio/virtio.c
> > >> +++ b/hw/virtio/virtio.c
> > >> @@ -220,7 +220,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);
> > >> @@ -230,7 +230,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);
> > >> -- 
> > >> 2.52.0
> 
> 
> I meant these ones.
> 
> > -- 
> > Alex Bennée
> > Virtualisation Tech Lead @ Linaro
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 04/11] hw/virtio: Use VirtIODevice::access_is_big_endian field

[Pierrick Bouvier]

On 2/2/26 10:52 AM, Stefan Hajnoczi wrote:
> 
> This command-line lets you benchmark virtio-blk without actual I/O
> slowing down the request processing:
> 
>    qemu-system-x86_64 \
>        -M accel=kvm \
>        -cpu host \
>        -m 4G \
>        --blockdev file,node-name=drive0,filename=boot.img,cache.direct=on,aio=native \
>        --blockdev null-co,node-name=drive1,size=$((10 * 1024 * 1024 * 1024)) \
>        --object iothread,id=iothread0 \
>        --device virtio-blk-pci,drive=drive0,iothread=iothread0 \
>        --device virtio-blk-pci,drive=drive1,iothread=iothread0
> 
> Here is a fio command-line for 4 KiB random reads:
> 
>    fio \
>        --ioengine=libaio \
>        --direct=1 \
>        --runtime=30 \
>        --ramp_time=10 \
>        --rw=randread \
>        --bs=4k \
>        --iodepth=128 \
>        --filename=/dev/vdb \
>        --name=randread
> 
> This is just a single vCPU, but it should be enough to see if there is
> any difference in I/O Operations Per Second (IOPS) or efficiency
> (IOPS/CPU utilization).
>
Thanks very much for the info Stefan. I didn't even know null-co 
blockdev, so it definitely would have taken some time to find all this.

For what it's worth, I automated the benchmark here (need podman for 
build), so it can be reused for future changes:
https://github.com/pbo-linaro/qemu-linux-stack/tree/x86_64_io_benchmark

./build.sh && ./run.sh path/to/qemu-system-x86_64

My initial testing showed a 50% slow down, which was more than 
surprising. After profiling, the extra time is spent here:
https://github.com/qemu/qemu/blob/587f4a1805c83a4e1d59dd43cb14e0a834843d1d/target-info.c#L30

When we merged target-info, there have been several versions over a long 
time, and I was 100% sure we were updating the target_info structure, 
instead of reparsing the target_name every time. Unfortunately, that's 
not the case. I'll fix that.

With that fix, there is no difference in performance (<1%).

I'll respin a v4 with the target info fix, initial v1 changes and 
benchmark results.

Thanks for pointing the performance issue, there was one for sure.

Regards,
Pierrick

Re: [PATCH v3 04/11] hw/virtio: Use VirtIODevice::access_is_big_endian field

[Pierrick Bouvier]

On 2/2/26 11:25 AM, Pierrick Bouvier wrote:
> On 2/2/26 10:52 AM, Stefan Hajnoczi wrote:
>>
>> This command-line lets you benchmark virtio-blk without actual I/O
>> slowing down the request processing:
>>
>>     qemu-system-x86_64 \
>>         -M accel=kvm \
>>         -cpu host \
>>         -m 4G \
>>         --blockdev file,node-name=drive0,filename=boot.img,cache.direct=on,aio=native \
>>         --blockdev null-co,node-name=drive1,size=$((10 * 1024 * 1024 * 1024)) \
>>         --object iothread,id=iothread0 \
>>         --device virtio-blk-pci,drive=drive0,iothread=iothread0 \
>>         --device virtio-blk-pci,drive=drive1,iothread=iothread0
>>
>> Here is a fio command-line for 4 KiB random reads:
>>
>>     fio \
>>         --ioengine=libaio \
>>         --direct=1 \
>>         --runtime=30 \
>>         --ramp_time=10 \
>>         --rw=randread \
>>         --bs=4k \
>>         --iodepth=128 \
>>         --filename=/dev/vdb \
>>         --name=randread
>>
>> This is just a single vCPU, but it should be enough to see if there is
>> any difference in I/O Operations Per Second (IOPS) or efficiency
>> (IOPS/CPU utilization).
>>
> Thanks very much for the info Stefan. I didn't even know null-co
> blockdev, so it definitely would have taken some time to find all this.
> 
> For what it's worth, I automated the benchmark here (need podman for
> build), so it can be reused for future changes:
> https://github.com/pbo-linaro/qemu-linux-stack/tree/x86_64_io_benchmark
> 
> ./build.sh && ./run.sh path/to/qemu-system-x86_64
> 
> My initial testing showed a 50% slow down, which was more than
> surprising. After profiling, the extra time is spent here:
> https://github.com/qemu/qemu/blob/587f4a1805c83a4e1d59dd43cb14e0a834843d1d/target-info.c#L30
> 
> When we merged target-info, there have been several versions over a long
> time, and I was 100% sure we were updating the target_info structure,
> instead of reparsing the target_name every time. Unfortunately, that's
> not the case. I'll fix that.
> 
> With that fix, there is no difference in performance (<1%).
> 
> I'll respin a v4 with the target info fix, initial v1 changes and
> benchmark results.
> 
> Thanks for pointing the performance issue, there was one for sure.
> 
> Regards,
> Pierrick

After proper benchmarking, I get those results (in kIOPS) over 20 runs, 
all including target_arch fix I mentioned.

reference: mean=239.2 var=12.16
v1: mean=232.2 var=37.06
v4-wip (optimized virtio_access_is_big_endian): mean=235.05 var=18.64

So basically, we have a 1.7% performance hit on a torture benchmark.
Is that acceptable for you, or should we dig more?

Regards,
Pierrick

Re: [PATCH v3 04/11] hw/virtio: Use VirtIODevice::access_is_big_endian field

[Michael S. Tsirkin]

On Mon, Feb 02, 2026 at 07:22:16PM -0800, Pierrick Bouvier wrote:
> On 2/2/26 11:25 AM, Pierrick Bouvier wrote:
> > On 2/2/26 10:52 AM, Stefan Hajnoczi wrote:
> > > 
> > > This command-line lets you benchmark virtio-blk without actual I/O
> > > slowing down the request processing:
> > > 
> > >     qemu-system-x86_64 \
> > >         -M accel=kvm \
> > >         -cpu host \
> > >         -m 4G \
> > >         --blockdev file,node-name=drive0,filename=boot.img,cache.direct=on,aio=native \
> > >         --blockdev null-co,node-name=drive1,size=$((10 * 1024 * 1024 * 1024)) \
> > >         --object iothread,id=iothread0 \
> > >         --device virtio-blk-pci,drive=drive0,iothread=iothread0 \
> > >         --device virtio-blk-pci,drive=drive1,iothread=iothread0
> > > 
> > > Here is a fio command-line for 4 KiB random reads:
> > > 
> > >     fio \
> > >         --ioengine=libaio \
> > >         --direct=1 \
> > >         --runtime=30 \
> > >         --ramp_time=10 \
> > >         --rw=randread \
> > >         --bs=4k \
> > >         --iodepth=128 \
> > >         --filename=/dev/vdb \
> > >         --name=randread
> > > 
> > > This is just a single vCPU, but it should be enough to see if there is
> > > any difference in I/O Operations Per Second (IOPS) or efficiency
> > > (IOPS/CPU utilization).
> > > 
> > Thanks very much for the info Stefan. I didn't even know null-co
> > blockdev, so it definitely would have taken some time to find all this.
> > 
> > For what it's worth, I automated the benchmark here (need podman for
> > build), so it can be reused for future changes:
> > https://github.com/pbo-linaro/qemu-linux-stack/tree/x86_64_io_benchmark
> > 
> > ./build.sh && ./run.sh path/to/qemu-system-x86_64
> > 
> > My initial testing showed a 50% slow down, which was more than
> > surprising. After profiling, the extra time is spent here:
> > https://github.com/qemu/qemu/blob/587f4a1805c83a4e1d59dd43cb14e0a834843d1d/target-info.c#L30
> > 
> > When we merged target-info, there have been several versions over a long
> > time, and I was 100% sure we were updating the target_info structure,
> > instead of reparsing the target_name every time. Unfortunately, that's
> > not the case. I'll fix that.
> > 
> > With that fix, there is no difference in performance (<1%).
> > 
> > I'll respin a v4 with the target info fix, initial v1 changes and
> > benchmark results.
> > 
> > Thanks for pointing the performance issue, there was one for sure.
> > 
> > Regards,
> > Pierrick
> 
> After proper benchmarking, I get those results (in kIOPS) over 20 runs, all
> including target_arch fix I mentioned.
> 
> reference: mean=239.2 var=12.16
> v1: mean=232.2 var=37.06
> v4-wip (optimized virtio_access_is_big_endian): mean=235.05 var=18.64
> 
> So basically, we have a 1.7% performance hit on a torture benchmark.
> Is that acceptable for you, or should we dig more?
> 
> Regards,
> Pierrick

Could we use some kind of linker trick?
Split just the ring manipulation code from
virtio.c and keep it target specific.


Or it could be that even just these two would be enough:

static inline uint16_t virtio_lduw_phys_cached(VirtIODevice *vdev,
                                               MemoryRegionCache *cache,
                                               hwaddr pa)
{
    if (virtio_access_is_big_endian(vdev)) {
        return lduw_be_phys_cached(cache, pa);
    }
    return lduw_le_phys_cached(cache, pa);
}

static inline void virtio_stw_phys_cached(VirtIODevice *vdev,
                                          MemoryRegionCache *cache,
                                          hwaddr pa, uint16_t value)
{
    if (virtio_access_is_big_endian(vdev)) {
        stw_be_phys_cached(cache, pa, value);
    } else {
        stw_le_phys_cached(cache, pa, value);
    }
}






-- 
MST

Re: [PATCH v3 04/11] hw/virtio: Use VirtIODevice::access_is_big_endian field

[Pierrick Bouvier]

On 2/3/26 3:07 AM, Michael S. Tsirkin wrote:
> On Mon, Feb 02, 2026 at 07:22:16PM -0800, Pierrick Bouvier wrote:
>> On 2/2/26 11:25 AM, Pierrick Bouvier wrote:
>>> On 2/2/26 10:52 AM, Stefan Hajnoczi wrote:
>>>>
>>>> This command-line lets you benchmark virtio-blk without actual I/O
>>>> slowing down the request processing:
>>>>
>>>>      qemu-system-x86_64 \
>>>>          -M accel=kvm \
>>>>          -cpu host \
>>>>          -m 4G \
>>>>          --blockdev file,node-name=drive0,filename=boot.img,cache.direct=on,aio=native \
>>>>          --blockdev null-co,node-name=drive1,size=$((10 * 1024 * 1024 * 1024)) \
>>>>          --object iothread,id=iothread0 \
>>>>          --device virtio-blk-pci,drive=drive0,iothread=iothread0 \
>>>>          --device virtio-blk-pci,drive=drive1,iothread=iothread0
>>>>
>>>> Here is a fio command-line for 4 KiB random reads:
>>>>
>>>>      fio \
>>>>          --ioengine=libaio \
>>>>          --direct=1 \
>>>>          --runtime=30 \
>>>>          --ramp_time=10 \
>>>>          --rw=randread \
>>>>          --bs=4k \
>>>>          --iodepth=128 \
>>>>          --filename=/dev/vdb \
>>>>          --name=randread
>>>>
>>>> This is just a single vCPU, but it should be enough to see if there is
>>>> any difference in I/O Operations Per Second (IOPS) or efficiency
>>>> (IOPS/CPU utilization).
>>>>
>>> Thanks very much for the info Stefan. I didn't even know null-co
>>> blockdev, so it definitely would have taken some time to find all this.
>>>
>>> For what it's worth, I automated the benchmark here (need podman for
>>> build), so it can be reused for future changes:
>>> https://github.com/pbo-linaro/qemu-linux-stack/tree/x86_64_io_benchmark
>>>
>>> ./build.sh && ./run.sh path/to/qemu-system-x86_64
>>>
>>> My initial testing showed a 50% slow down, which was more than
>>> surprising. After profiling, the extra time is spent here:
>>> https://github.com/qemu/qemu/blob/587f4a1805c83a4e1d59dd43cb14e0a834843d1d/target-info.c#L30
>>>
>>> When we merged target-info, there have been several versions over a long
>>> time, and I was 100% sure we were updating the target_info structure,
>>> instead of reparsing the target_name every time. Unfortunately, that's
>>> not the case. I'll fix that.
>>>
>>> With that fix, there is no difference in performance (<1%).
>>>
>>> I'll respin a v4 with the target info fix, initial v1 changes and
>>> benchmark results.
>>>
>>> Thanks for pointing the performance issue, there was one for sure.
>>>
>>> Regards,
>>> Pierrick
>>
>> After proper benchmarking, I get those results (in kIOPS) over 20 runs, all
>> including target_arch fix I mentioned.
>>
>> reference: mean=239.2 var=12.16
>> v1: mean=232.2 var=37.06
>> v4-wip (optimized virtio_access_is_big_endian): mean=235.05 var=18.64
>>
>> So basically, we have a 1.7% performance hit on a torture benchmark.
>> Is that acceptable for you, or should we dig more?
>>
>> Regards,
>> Pierrick
> 
> Could we use some kind of linker trick?
> Split just the ring manipulation code from
> virtio.c and keep it target specific.
>

While it would allow to extract the common part, unfortunately, as long 
as we keep a target specific part, we still have a problem to solve.
Indeed, some symbols (may be ring manipulation code, or just functions 
mentioned below) will still be duplicated, and in the end, when we'll 
link two targets needing different versions, we'll have a conflict.
Since our goal for a single-binary is to have at least arm and another 
base architecture, we are in this situation now.

So the only solutions left are:
- accept the 1.7% regression, and we can provide more "real world" 
benchmarks to show that it gets absorbed in all existing layers
- compute virtio_access_is_big_endian once and reuse this everywhere: 
that's what Philippe implemented in his patches.

What would you prefer?

> 
> Or it could be that even just these two would be enough:
> 
> static inline uint16_t virtio_lduw_phys_cached(VirtIODevice *vdev,
>                                                 MemoryRegionCache *cache,
>                                                 hwaddr pa)
> {
>      if (virtio_access_is_big_endian(vdev)) {
>          return lduw_be_phys_cached(cache, pa);
>      }
>      return lduw_le_phys_cached(cache, pa);
> }
> 
> static inline void virtio_stw_phys_cached(VirtIODevice *vdev,
>                                            MemoryRegionCache *cache,
>                                            hwaddr pa, uint16_t value)
> {
>      if (virtio_access_is_big_endian(vdev)) {
>          stw_be_phys_cached(cache, pa, value);
>      } else {
>          stw_le_phys_cached(cache, pa, value);
>      }
> }
> 
> 
> 
> 
> 
> 

Regards,
Pierrick

Re: [PATCH v3 04/11] hw/virtio: Use VirtIODevice::access_is_big_endian field

[Michael S. Tsirkin]

On Tue, Feb 03, 2026 at 09:31:04AM -0800, Pierrick Bouvier wrote:
> On 2/3/26 3:07 AM, Michael S. Tsirkin wrote:
> > On Mon, Feb 02, 2026 at 07:22:16PM -0800, Pierrick Bouvier wrote:
> > > On 2/2/26 11:25 AM, Pierrick Bouvier wrote:
> > > > On 2/2/26 10:52 AM, Stefan Hajnoczi wrote:
> > > > > 
> > > > > This command-line lets you benchmark virtio-blk without actual I/O
> > > > > slowing down the request processing:
> > > > > 
> > > > >      qemu-system-x86_64 \
> > > > >          -M accel=kvm \
> > > > >          -cpu host \
> > > > >          -m 4G \
> > > > >          --blockdev file,node-name=drive0,filename=boot.img,cache.direct=on,aio=native \
> > > > >          --blockdev null-co,node-name=drive1,size=$((10 * 1024 * 1024 * 1024)) \
> > > > >          --object iothread,id=iothread0 \
> > > > >          --device virtio-blk-pci,drive=drive0,iothread=iothread0 \
> > > > >          --device virtio-blk-pci,drive=drive1,iothread=iothread0
> > > > > 
> > > > > Here is a fio command-line for 4 KiB random reads:
> > > > > 
> > > > >      fio \
> > > > >          --ioengine=libaio \
> > > > >          --direct=1 \
> > > > >          --runtime=30 \
> > > > >          --ramp_time=10 \
> > > > >          --rw=randread \
> > > > >          --bs=4k \
> > > > >          --iodepth=128 \
> > > > >          --filename=/dev/vdb \
> > > > >          --name=randread
> > > > > 
> > > > > This is just a single vCPU, but it should be enough to see if there is
> > > > > any difference in I/O Operations Per Second (IOPS) or efficiency
> > > > > (IOPS/CPU utilization).
> > > > > 
> > > > Thanks very much for the info Stefan. I didn't even know null-co
> > > > blockdev, so it definitely would have taken some time to find all this.
> > > > 
> > > > For what it's worth, I automated the benchmark here (need podman for
> > > > build), so it can be reused for future changes:
> > > > https://github.com/pbo-linaro/qemu-linux-stack/tree/x86_64_io_benchmark
> > > > 
> > > > ./build.sh && ./run.sh path/to/qemu-system-x86_64
> > > > 
> > > > My initial testing showed a 50% slow down, which was more than
> > > > surprising. After profiling, the extra time is spent here:
> > > > https://github.com/qemu/qemu/blob/587f4a1805c83a4e1d59dd43cb14e0a834843d1d/target-info.c#L30
> > > > 
> > > > When we merged target-info, there have been several versions over a long
> > > > time, and I was 100% sure we were updating the target_info structure,
> > > > instead of reparsing the target_name every time. Unfortunately, that's
> > > > not the case. I'll fix that.
> > > > 
> > > > With that fix, there is no difference in performance (<1%).
> > > > 
> > > > I'll respin a v4 with the target info fix, initial v1 changes and
> > > > benchmark results.
> > > > 
> > > > Thanks for pointing the performance issue, there was one for sure.
> > > > 
> > > > Regards,
> > > > Pierrick
> > > 
> > > After proper benchmarking, I get those results (in kIOPS) over 20 runs, all
> > > including target_arch fix I mentioned.
> > > 
> > > reference: mean=239.2 var=12.16
> > > v1: mean=232.2 var=37.06
> > > v4-wip (optimized virtio_access_is_big_endian): mean=235.05 var=18.64
> > > 
> > > So basically, we have a 1.7% performance hit on a torture benchmark.
> > > Is that acceptable for you, or should we dig more?
> > > 
> > > Regards,
> > > Pierrick
> > 
> > Could we use some kind of linker trick?
> > Split just the ring manipulation code from
> > virtio.c and keep it target specific.
> > 
> 
> While it would allow to extract the common part, unfortunately, as long as
> we keep a target specific part, we still have a problem to solve.
>
>
> Indeed, some symbols (may be ring manipulation code, or just functions
> mentioned below) will still be duplicated, and in the end, when we'll link
> two targets needing different versions, we'll have a conflict.

Shrug. The generic slower one can be made stronger and be used then.
This is not an new or unsolveable problem.


> Since our goal for a single-binary is to have at least arm and another base
> architecture, we are in this situation now.
> 
> So the only solutions left are:
> - accept the 1.7% regression, and we can provide more "real world"
> benchmarks to show that it gets absorbed in all existing layers
> - compute virtio_access_is_big_endian once and reuse this everywhere: that's
> what Philippe implemented in his patches.
> 
> What would you prefer?


Let's find a way where people who are not interested in a single binary
do not have to pay the price, please.

> > 
> > Or it could be that even just these two would be enough:
> > 
> > static inline uint16_t virtio_lduw_phys_cached(VirtIODevice *vdev,
> >                                                 MemoryRegionCache *cache,
> >                                                 hwaddr pa)
> > {
> >      if (virtio_access_is_big_endian(vdev)) {
> >          return lduw_be_phys_cached(cache, pa);
> >      }
> >      return lduw_le_phys_cached(cache, pa);
> > }
> > 
> > static inline void virtio_stw_phys_cached(VirtIODevice *vdev,
> >                                            MemoryRegionCache *cache,
> >                                            hwaddr pa, uint16_t value)
> > {
> >      if (virtio_access_is_big_endian(vdev)) {
> >          stw_be_phys_cached(cache, pa, value);
> >      } else {
> >          stw_le_phys_cached(cache, pa, value);
> >      }
> > }
> > 
> > 
> > 
> > 
> > 
> > 
> 
> Regards,
> Pierrick

Re: [PATCH v3 04/11] hw/virtio: Use VirtIODevice::access_is_big_endian field

[Pierrick Bouvier]

On 2/3/26 11:06 AM, Michael S. Tsirkin wrote:
> On Tue, Feb 03, 2026 at 09:31:04AM -0800, Pierrick Bouvier wrote:
>> On 2/3/26 3:07 AM, Michael S. Tsirkin wrote:
>>> On Mon, Feb 02, 2026 at 07:22:16PM -0800, Pierrick Bouvier wrote:
>>>> On 2/2/26 11:25 AM, Pierrick Bouvier wrote:
>>>>> On 2/2/26 10:52 AM, Stefan Hajnoczi wrote:
>>>>>>
>>>>>> This command-line lets you benchmark virtio-blk without actual I/O
>>>>>> slowing down the request processing:
>>>>>>
>>>>>>       qemu-system-x86_64 \
>>>>>>           -M accel=kvm \
>>>>>>           -cpu host \
>>>>>>           -m 4G \
>>>>>>           --blockdev file,node-name=drive0,filename=boot.img,cache.direct=on,aio=native \
>>>>>>           --blockdev null-co,node-name=drive1,size=$((10 * 1024 * 1024 * 1024)) \
>>>>>>           --object iothread,id=iothread0 \
>>>>>>           --device virtio-blk-pci,drive=drive0,iothread=iothread0 \
>>>>>>           --device virtio-blk-pci,drive=drive1,iothread=iothread0
>>>>>>
>>>>>> Here is a fio command-line for 4 KiB random reads:
>>>>>>
>>>>>>       fio \
>>>>>>           --ioengine=libaio \
>>>>>>           --direct=1 \
>>>>>>           --runtime=30 \
>>>>>>           --ramp_time=10 \
>>>>>>           --rw=randread \
>>>>>>           --bs=4k \
>>>>>>           --iodepth=128 \
>>>>>>           --filename=/dev/vdb \
>>>>>>           --name=randread
>>>>>>
>>>>>> This is just a single vCPU, but it should be enough to see if there is
>>>>>> any difference in I/O Operations Per Second (IOPS) or efficiency
>>>>>> (IOPS/CPU utilization).
>>>>>>
>>>>> Thanks very much for the info Stefan. I didn't even know null-co
>>>>> blockdev, so it definitely would have taken some time to find all this.
>>>>>
>>>>> For what it's worth, I automated the benchmark here (need podman for
>>>>> build), so it can be reused for future changes:
>>>>> https://github.com/pbo-linaro/qemu-linux-stack/tree/x86_64_io_benchmark
>>>>>
>>>>> ./build.sh && ./run.sh path/to/qemu-system-x86_64
>>>>>
>>>>> My initial testing showed a 50% slow down, which was more than
>>>>> surprising. After profiling, the extra time is spent here:
>>>>> https://github.com/qemu/qemu/blob/587f4a1805c83a4e1d59dd43cb14e0a834843d1d/target-info.c#L30
>>>>>
>>>>> When we merged target-info, there have been several versions over a long
>>>>> time, and I was 100% sure we were updating the target_info structure,
>>>>> instead of reparsing the target_name every time. Unfortunately, that's
>>>>> not the case. I'll fix that.
>>>>>
>>>>> With that fix, there is no difference in performance (<1%).
>>>>>
>>>>> I'll respin a v4 with the target info fix, initial v1 changes and
>>>>> benchmark results.
>>>>>
>>>>> Thanks for pointing the performance issue, there was one for sure.
>>>>>
>>>>> Regards,
>>>>> Pierrick
>>>>
>>>> After proper benchmarking, I get those results (in kIOPS) over 20 runs, all
>>>> including target_arch fix I mentioned.
>>>>
>>>> reference: mean=239.2 var=12.16
>>>> v1: mean=232.2 var=37.06
>>>> v4-wip (optimized virtio_access_is_big_endian): mean=235.05 var=18.64
>>>>
>>>> So basically, we have a 1.7% performance hit on a torture benchmark.
>>>> Is that acceptable for you, or should we dig more?
>>>>
>>>> Regards,
>>>> Pierrick
>>>
>>> Could we use some kind of linker trick?
>>> Split just the ring manipulation code from
>>> virtio.c and keep it target specific.
>>>
>>
>> While it would allow to extract the common part, unfortunately, as long as
>> we keep a target specific part, we still have a problem to solve.
>>
>>
>> Indeed, some symbols (may be ring manipulation code, or just functions
>> mentioned below) will still be duplicated, and in the end, when we'll link
>> two targets needing different versions, we'll have a conflict.
> 
> Shrug. The generic slower one can be made stronger and be used then.
> This is not an new or unsolveable problem.
>
> 
>> Since our goal for a single-binary is to have at least arm and another base
>> architecture, we are in this situation now.
>>
>> So the only solutions left are:
>> - accept the 1.7% regression, and we can provide more "real world"
>> benchmarks to show that it gets absorbed in all existing layers
>> - compute virtio_access_is_big_endian once and reuse this everywhere: that's
>> what Philippe implemented in his patches.
>>
>> What would you prefer?
> 
> 
> Let's find a way where people who are not interested in a single binary
> do not have to pay the price, please.
>

Sure, I'll try and benchmark with Philippe additional patches, it should 
remove the overhead completely.

Thanks,
Pierrick

Re: [PATCH v3 04/11] hw/virtio: Use VirtIODevice::access_is_big_endian field

[Pierrick Bouvier]

On 2/3/26 11:10 AM, Pierrick Bouvier wrote:
> On 2/3/26 11:06 AM, Michael S. Tsirkin wrote:
>> On Tue, Feb 03, 2026 at 09:31:04AM -0800, Pierrick Bouvier wrote:
>>> On 2/3/26 3:07 AM, Michael S. Tsirkin wrote:
>>>> On Mon, Feb 02, 2026 at 07:22:16PM -0800, Pierrick Bouvier wrote:
>>>>> On 2/2/26 11:25 AM, Pierrick Bouvier wrote:
>>>>>> On 2/2/26 10:52 AM, Stefan Hajnoczi wrote:
>>>>>>>
>>>>>>> This command-line lets you benchmark virtio-blk without actual I/O
>>>>>>> slowing down the request processing:
>>>>>>>
>>>>>>>        qemu-system-x86_64 \
>>>>>>>            -M accel=kvm \
>>>>>>>            -cpu host \
>>>>>>>            -m 4G \
>>>>>>>            --blockdev file,node-name=drive0,filename=boot.img,cache.direct=on,aio=native \
>>>>>>>            --blockdev null-co,node-name=drive1,size=$((10 * 1024 * 1024 * 1024)) \
>>>>>>>            --object iothread,id=iothread0 \
>>>>>>>            --device virtio-blk-pci,drive=drive0,iothread=iothread0 \
>>>>>>>            --device virtio-blk-pci,drive=drive1,iothread=iothread0
>>>>>>>
>>>>>>> Here is a fio command-line for 4 KiB random reads:
>>>>>>>
>>>>>>>        fio \
>>>>>>>            --ioengine=libaio \
>>>>>>>            --direct=1 \
>>>>>>>            --runtime=30 \
>>>>>>>            --ramp_time=10 \
>>>>>>>            --rw=randread \
>>>>>>>            --bs=4k \
>>>>>>>            --iodepth=128 \
>>>>>>>            --filename=/dev/vdb \
>>>>>>>            --name=randread
>>>>>>>
>>>>>>> This is just a single vCPU, but it should be enough to see if there is
>>>>>>> any difference in I/O Operations Per Second (IOPS) or efficiency
>>>>>>> (IOPS/CPU utilization).
>>>>>>>
>>>>>> Thanks very much for the info Stefan. I didn't even know null-co
>>>>>> blockdev, so it definitely would have taken some time to find all this.
>>>>>>
>>>>>> For what it's worth, I automated the benchmark here (need podman for
>>>>>> build), so it can be reused for future changes:
>>>>>> https://github.com/pbo-linaro/qemu-linux-stack/tree/x86_64_io_benchmark
>>>>>>
>>>>>> ./build.sh && ./run.sh path/to/qemu-system-x86_64
>>>>>>
>>>>>> My initial testing showed a 50% slow down, which was more than
>>>>>> surprising. After profiling, the extra time is spent here:
>>>>>> https://github.com/qemu/qemu/blob/587f4a1805c83a4e1d59dd43cb14e0a834843d1d/target-info.c#L30
>>>>>>
>>>>>> When we merged target-info, there have been several versions over a long
>>>>>> time, and I was 100% sure we were updating the target_info structure,
>>>>>> instead of reparsing the target_name every time. Unfortunately, that's
>>>>>> not the case. I'll fix that.
>>>>>>
>>>>>> With that fix, there is no difference in performance (<1%).
>>>>>>
>>>>>> I'll respin a v4 with the target info fix, initial v1 changes and
>>>>>> benchmark results.
>>>>>>
>>>>>> Thanks for pointing the performance issue, there was one for sure.
>>>>>>
>>>>>> Regards,
>>>>>> Pierrick
>>>>>
>>>>> After proper benchmarking, I get those results (in kIOPS) over 20 runs, all
>>>>> including target_arch fix I mentioned.
>>>>>
>>>>> reference: mean=239.2 var=12.16
>>>>> v1: mean=232.2 var=37.06
>>>>> v4-wip (optimized virtio_access_is_big_endian): mean=235.05 var=18.64
>>>>>
>>>>> So basically, we have a 1.7% performance hit on a torture benchmark.
>>>>> Is that acceptable for you, or should we dig more?
>>>>>
>>>>> Regards,
>>>>> Pierrick
>>>>
>>>> Could we use some kind of linker trick?
>>>> Split just the ring manipulation code from
>>>> virtio.c and keep it target specific.
>>>>
>>>
>>> While it would allow to extract the common part, unfortunately, as long as
>>> we keep a target specific part, we still have a problem to solve.
>>>
>>>
>>> Indeed, some symbols (may be ring manipulation code, or just functions
>>> mentioned below) will still be duplicated, and in the end, when we'll link
>>> two targets needing different versions, we'll have a conflict.
>>
>> Shrug. The generic slower one can be made stronger and be used then.
>> This is not an new or unsolveable problem.
>>
>>
>>> Since our goal for a single-binary is to have at least arm and another base
>>> architecture, we are in this situation now.
>>>
>>> So the only solutions left are:
>>> - accept the 1.7% regression, and we can provide more "real world"
>>> benchmarks to show that it gets absorbed in all existing layers
>>> - compute virtio_access_is_big_endian once and reuse this everywhere: that's
>>> what Philippe implemented in his patches.
>>>
>>> What would you prefer?
>>
>>
>> Let's find a way where people who are not interested in a single binary
>> do not have to pay the price, please.
>>
> 
> Sure, I'll try and benchmark with Philippe additional patches, it should
> remove the overhead completely.
> 
> Thanks,
> Pierrick

This resulted in a +1.4% speedup vs existing code, so it sounds like we 
have a solution.
I'll assemble and post a v4 for this.

Regards,
Pierrick

Re: [PATCH v3 04/11] hw/virtio: Use VirtIODevice::access_is_big_endian field

[Philippe Mathieu-Daudé]

On 2/2/26 20:25, Pierrick Bouvier wrote:

> My initial testing showed a 50% slow down, which was more than 
> surprising. After profiling, the extra time is spent here:
> https://github.com/qemu/qemu/ 
> blob/587f4a1805c83a4e1d59dd43cb14e0a834843d1d/target-info.c#L30
> 
> When we merged target-info, there have been several versions over a long 
> time, and I was 100% sure we were updating the target_info structure, 
> instead of reparsing the target_name every time. Unfortunately, that's 
> not the case. I'll fix that.

We (Richard, you, myself) knew this patch was bad performance wise, but
back then it was decided to be good enough for the first conversion of
build-time target specific definitions to runtime methods.

As a second step, Richard asked for the target-info.c file to get
absorbed with page-vary-common.c, compiling it with CFLAG=-fno-lto.
This wasn't a priority until TargetInfo API got involved in hot path.

[PATCH v3 05/11] hw/virtio: Reduce virtio_access_is_big_endian() scope

[Philippe Mathieu-Daudé]

virtio_access_is_big_endian() is only used once in virtio_reset().
Make the VirtIODevice argument const.

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

diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
index f3b4d0075b5..c6b76979749 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -16,29 +16,9 @@
 #ifndef QEMU_VIRTIO_ACCESS_H
 #define QEMU_VIRTIO_ACCESS_H
 
-#include "exec/hwaddr.h"
+#include "qemu/bswap.h"
 #include "system/memory_cached.h"
 #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
-}
 
 static inline void virtio_stw_p(VirtIODevice *vdev, void *ptr, uint16_t v)
 {
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 1dc60d37cb4..627a929b063 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3233,6 +3233,25 @@ int virtio_set_features_ex(VirtIODevice *vdev, const uint64_t *features)
     return ret;
 }
 
+#if defined(TARGET_PPC64) || defined(TARGET_ARM)
+#define LEGACY_VIRTIO_IS_BIENDIAN 1
+#endif
+
+static bool virtio_access_is_big_endian(const 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
+}
+
 void virtio_reset(VirtIODevice *vdev)
 {
     VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
-- 
2.52.0

[PATCH v3 06/11] hw/virtio: Check target supports legacy bi-endianness at runtime

[Philippe Mathieu-Daudé]

Convert build-time check by a runtime one using the TargetInfo API.

Inspired-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/virtio.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 627a929b063..d64ecc29a89 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3233,15 +3233,17 @@ int virtio_set_features_ex(VirtIODevice *vdev, const uint64_t *features)
     return ret;
 }
 
-#if defined(TARGET_PPC64) || defined(TARGET_ARM)
-#define LEGACY_VIRTIO_IS_BIENDIAN 1
-#endif
+static bool target_is_legacy_virtio_biendian(void)
+{
+    return target_ppc64() || target_base_arm();
+}
 
 static bool virtio_access_is_big_endian(const VirtIODevice *vdev)
 {
-#if defined(LEGACY_VIRTIO_IS_BIENDIAN)
-    return virtio_is_big_endian(vdev);
-#elif TARGET_BIG_ENDIAN
+    if (target_is_legacy_virtio_biendian()) {
+        return virtio_is_big_endian(vdev);
+    }
+#if 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;
-- 
2.52.0

[PATCH v3 07/11] hw/virtio: Replace TARGET_BIG_ENDIAN -> target_big_endian()

[Philippe Mathieu-Daudé]

Check endianness at runtime to remove the target-specific
TARGET_BIG_ENDIAN definition.

Inspired-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/virtio.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index d64ecc29a89..9c5563f9bec 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3243,15 +3243,14 @@ static bool virtio_access_is_big_endian(const VirtIODevice *vdev)
     if (target_is_legacy_virtio_biendian()) {
         return virtio_is_big_endian(vdev);
     }
-#if TARGET_BIG_ENDIAN
+    if (!target_big_endian()) {
+        return false;
+    }
     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
 }
 
 void virtio_reset(VirtIODevice *vdev)
-- 
2.52.0

[PATCH v3 08/11] hw/ppc/spapr: extract SPAPR_MAX_RAM_SLOTS in a new header

[Philippe Mathieu-Daudé]

From: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Allow to include it from common code (vhost-user, in next commit),
else it pulls ppc/cpu.h which has target specifics.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/ppc/spapr.h        |  8 +-------
 include/hw/ppc/spapr_common.h | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 7 deletions(-)
 create mode 100644 include/hw/ppc/spapr_common.h

diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 5476ac7ce7b..b022f8dd25d 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -4,6 +4,7 @@
 #include "qemu/units.h"
 #include "system/dma.h"
 #include "hw/core/boards.h"
+#include "hw/ppc/spapr_common.h"
 #include "hw/ppc/spapr_drc.h"
 #include "hw/mem/pc-dimm.h"
 #include "hw/ppc/spapr_ovec.h"
@@ -946,13 +947,6 @@ int spapr_rtc_import_offset(SpaprRtcState *rtc, int64_t legacy_offset);
 
 #define SPAPR_MEMORY_BLOCK_SIZE ((hwaddr)1 << 28) /* 256MB */
 
-/*
- * This defines the maximum number of DIMM slots we can have for sPAPR
- * guest. This is not defined by sPAPR but we are defining it to 32 slots
- * based on default number of slots provided by PowerPC kernel.
- */
-#define SPAPR_MAX_RAM_SLOTS     32
-
 /* 1GB alignment for hotplug memory region */
 #define SPAPR_DEVICE_MEM_ALIGN (1 * GiB)
 
diff --git a/include/hw/ppc/spapr_common.h b/include/hw/ppc/spapr_common.h
new file mode 100644
index 00000000000..a315b685805
--- /dev/null
+++ b/include/hw/ppc/spapr_common.h
@@ -0,0 +1,17 @@
+/*
+ * Common definitions for PPC sPAPR
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_SPAPR_COMMON_H
+#define HW_SPAPR_COMMON_H
+
+/*
+ * The maximum number of DIMM slots we can have for sPAPR guest.
+ * This is not defined by sPAPR but we are defining it to 32 slots
+ * based on default number of slots provided by PowerPC kernel.
+ */
+#define SPAPR_MAX_RAM_SLOTS     32
+
+#endif /* HW_SPAPR_COMMON_H */
-- 
2.52.0

[PATCH v3 09/11] hw/virtio/vhost-user: make compilation unit common

[Philippe Mathieu-Daudé]

From: Pierrick Bouvier <pierrick.bouvier@linaro.org>

PPC architectures use a custom value for VHOST_USER_MAX_RAM_SLOTS (32
instead of 512).

vhost_user struct and several functions use VHOST_USER_MAX_RAM_SLOTS to
define stack allocated buffers. To avoid changing all functions to use
heap allocated buffers, we keep this max, and simply add a
target_base_ppc() conditional for the single place where size really
matters.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/vhost-user.c | 11 ++++-------
 hw/virtio/meson.build  |  3 +--
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 63fa9a1b4b1..c8a004c6d2d 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -44,13 +44,8 @@
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 #define VHOST_USER_BACKEND_MAX_FDS     8
 
-#if defined(TARGET_PPC) || defined(TARGET_PPC64)
-#include "hw/ppc/spapr.h"
-#define VHOST_USER_MAX_RAM_SLOTS SPAPR_MAX_RAM_SLOTS
-
-#else
+#include "hw/ppc/spapr_common.h"
 #define VHOST_USER_MAX_RAM_SLOTS 512
-#endif
 
 /*
  * Maximum size of virtio device config space
@@ -2280,7 +2275,9 @@ 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);
+            const uint64_t vhost_user_max_ram_slots = target_base_ppc() ?
+                SPAPR_MAX_RAM_SLOTS : VHOST_USER_MAX_RAM_SLOTS;
+            u->user->memory_slots = MIN(ram_slots, vhost_user_max_ram_slots);
         }
     }
 
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index affd66887db..ee397aaf196 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -17,8 +17,7 @@ if have_vhost
   system_virtio_ss.add(files('vhost.c'))
   system_virtio_ss.add(files('vhost-backend.c', 'vhost-iova-tree.c'))
   if have_vhost_user
-    # fixme - this really should be generic
-    specific_virtio_ss.add(files('vhost-user.c'))
+    system_virtio_ss.add(files('vhost-user.c'))
     system_virtio_ss.add(files('vhost-user-base.c'))
 
     # MMIO Stubs
-- 
2.52.0

[PATCH v3 10/11] hw/virtio/virtio-qmp: make compilation unit common

[Philippe Mathieu-Daudé]

From: Pierrick Bouvier <pierrick.bouvier@linaro.org>

All compile time conditionals have no impact at runtime, since they are
representing only possible features for devices present at runtime.

In case they are not present, associated features table will never be
used. In case they are present but some features are not, matching bits
will never be enabled, so those entries will be unused.

Thus, simply expose everything and call it a day.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/virtio-qmp.c | 70 ------------------------------------------
 hw/virtio/meson.build  |  2 +-
 2 files changed, 1 insertion(+), 71 deletions(-)

diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
index 968299fda0c..f9cdca50d99 100644
--- a/hw/virtio/virtio-qmp.c
+++ b/hw/virtio/virtio-qmp.c
@@ -33,21 +33,17 @@
 #include "standard-headers/linux/virtio_vsock.h"
 #include "standard-headers/linux/virtio_gpio.h"
 
-#include CONFIG_DEVICES
-
 #define FEATURE_ENTRY(name, desc) (qmp_virtio_feature_map_t) \
     { .virtio_bit = name, .feature_desc = desc }
 
 /* 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, \
@@ -149,7 +145,6 @@ static const qmp_virtio_feature_map_t virtio_config_status_map[] = {
 };
 
 /* virtio-blk features mapping */
-#ifdef CONFIG_VIRTIO_BLK
 static 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"),
@@ -173,7 +168,6 @@ static const qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
             "VIRTIO_BLK_F_SECURE_ERASE: Secure erase supported"),
     FEATURE_ENTRY(VIRTIO_BLK_F_ZONED, \
             "VIRTIO_BLK_F_ZONED: Zoned block devices"),
-#ifndef VIRTIO_BLK_NO_LEGACY
     FEATURE_ENTRY(VIRTIO_BLK_F_BARRIER, \
             "VIRTIO_BLK_F_BARRIER: Request barriers supported"),
     FEATURE_ENTRY(VIRTIO_BLK_F_SCSI, \
@@ -183,7 +177,6 @@ static const qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
     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, \
@@ -191,10 +184,8 @@ static const qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
             "negotiation supported"),
     { -1, "" }
 };
-#endif
 
 /* virtio-serial features mapping */
-#ifdef CONFIG_VIRTIO_SERIAL
 static const qmp_virtio_feature_map_t virtio_serial_feature_map[] = {
     FEATURE_ENTRY(VIRTIO_CONSOLE_F_SIZE, \
             "VIRTIO_CONSOLE_F_SIZE: Host providing console size"),
@@ -204,10 +195,8 @@ static const qmp_virtio_feature_map_t virtio_serial_feature_map[] = {
             "VIRTIO_CONSOLE_F_EMERG_WRITE: Emergency write supported"),
     { -1, "" }
 };
-#endif
 
 /* virtio-gpu features mapping */
-#ifdef CONFIG_VIRTIO_GPU
 static const qmp_virtio_feature_map_t virtio_gpu_feature_map[] = {
     FEATURE_ENTRY(VIRTIO_GPU_F_VIRGL, \
             "VIRTIO_GPU_F_VIRGL: Virgl 3D mode supported"),
@@ -227,10 +216,8 @@ static const qmp_virtio_feature_map_t virtio_gpu_feature_map[] = {
             "negotiation supported"),
     { -1, "" }
 };
-#endif
 
 /* virtio-input features mapping */
-#ifdef CONFIG_VIRTIO_INPUT
 static const qmp_virtio_feature_map_t virtio_input_feature_map[] = {
     FEATURE_ENTRY(VHOST_F_LOG_ALL, \
             "VHOST_F_LOG_ALL: Logging write descriptors supported"),
@@ -239,10 +226,8 @@ static const qmp_virtio_feature_map_t virtio_input_feature_map[] = {
             "negotiation supported"),
     { -1, "" }
 };
-#endif
 
 /* virtio-net features mapping */
-#ifdef CONFIG_VIRTIO_NET
 static 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 "
@@ -313,10 +298,8 @@ static const qmp_virtio_feature_map_t virtio_net_feature_map[] = {
             "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"),
@@ -341,10 +324,8 @@ static const qmp_virtio_feature_map_t virtio_net_feature_map[] = {
             "header"),
     { -1, "" }
 };
-#endif
 
 /* virtio-scsi features mapping */
-#ifdef CONFIG_VIRTIO_SCSI
 static 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 "
@@ -364,10 +345,8 @@ static const qmp_virtio_feature_map_t virtio_scsi_feature_map[] = {
             "negotiation supported"),
     { -1, "" }
 };
-#endif
 
 /* virtio/vhost-user-fs features mapping */
-#ifdef CONFIG_VHOST_USER_FS
 static const qmp_virtio_feature_map_t virtio_fs_feature_map[] = {
     FEATURE_ENTRY(VHOST_F_LOG_ALL, \
             "VHOST_F_LOG_ALL: Logging write descriptors supported"),
@@ -376,10 +355,8 @@ static const qmp_virtio_feature_map_t virtio_fs_feature_map[] = {
             "negotiation supported"),
     { -1, "" }
 };
-#endif
 
 /* virtio/vhost-user-i2c features mapping */
-#ifdef CONFIG_VIRTIO_I2C_ADAPTER
 static 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"),
@@ -390,10 +367,8 @@ static const qmp_virtio_feature_map_t virtio_i2c_feature_map[] = {
             "negotiation supported"),
     { -1, "" }
 };
-#endif
 
 /* virtio/vhost-vsock features mapping */
-#ifdef CONFIG_VHOST_VSOCK
 static const qmp_virtio_feature_map_t virtio_vsock_feature_map[] = {
     FEATURE_ENTRY(VIRTIO_VSOCK_F_SEQPACKET, \
             "VIRTIO_VSOCK_F_SEQPACKET: SOCK_SEQPACKET supported"),
@@ -404,10 +379,8 @@ static const qmp_virtio_feature_map_t virtio_vsock_feature_map[] = {
             "negotiation supported"),
     { -1, "" }
 };
-#endif
 
 /* virtio-balloon features mapping */
-#ifdef CONFIG_VIRTIO_BALLOON
 static 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 "
@@ -424,19 +397,15 @@ static const qmp_virtio_feature_map_t virtio_balloon_feature_map[] = {
             "VIRTIO_BALLOON_F_REPORTING: Page reporting VQ enabled"),
     { -1, "" }
 };
-#endif
 
 /* virtio-crypto features mapping */
-#ifdef CONFIG_VIRTIO_CRYPTO
 static 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, "" }
 };
-#endif
 
 /* virtio-iommu features mapping */
-#ifdef CONFIG_VIRTIO_IOMMU
 static 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. "
@@ -458,15 +427,11 @@ static const qmp_virtio_feature_map_t virtio_iommu_feature_map[] = {
             "available"),
     { -1, "" }
 };
-#endif
 
 /* virtio-mem features mapping */
-#ifdef CONFIG_VIRTIO_MEM
 static 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"),
@@ -475,10 +440,8 @@ static const qmp_virtio_feature_map_t virtio_mem_feature_map[] = {
             "plugged when suspending+resuming"),
     { -1, "" }
 };
-#endif
 
 /* virtio-rng features mapping */
-#ifdef CONFIG_VIRTIO_RNG
 static const qmp_virtio_feature_map_t virtio_rng_feature_map[] = {
     FEATURE_ENTRY(VHOST_F_LOG_ALL, \
             "VHOST_F_LOG_ALL: Logging write descriptors supported"),
@@ -487,10 +450,8 @@ static const qmp_virtio_feature_map_t virtio_rng_feature_map[] = {
             "negotiation supported"),
     { -1, "" }
 };
-#endif
 
 /* virtio/vhost-gpio features mapping */
-#ifdef CONFIG_VHOST_USER_GPIO
 static const qmp_virtio_feature_map_t virtio_gpio_feature_map[] = {
     FEATURE_ENTRY(VIRTIO_GPIO_F_IRQ, \
             "VIRTIO_GPIO_F_IRQ: Device supports interrupts on GPIO lines"),
@@ -499,7 +460,6 @@ static const qmp_virtio_feature_map_t virtio_gpio_feature_map[] = {
             "negotiation supported"),
     { -1, "" }
 };
-#endif
 
 #define CONVERT_FEATURES(type, map, is_status, bitmap)   \
     ({                                                   \
@@ -595,96 +555,66 @@ VirtioDeviceFeatures *qmp_decode_features(uint16_t device_id,
 
     /* device features */
     switch (device_id) {
-#ifdef CONFIG_VIRTIO_SERIAL
     case VIRTIO_ID_CONSOLE:
         features->dev_features =
             CONVERT_FEATURES_EX(strList, virtio_serial_feature_map, bitmap);
         break;
-#endif
-#ifdef CONFIG_VIRTIO_BLK
     case VIRTIO_ID_BLOCK:
         features->dev_features =
             CONVERT_FEATURES_EX(strList, virtio_blk_feature_map, bitmap);
         break;
-#endif
-#ifdef CONFIG_VIRTIO_GPU
     case VIRTIO_ID_GPU:
         features->dev_features =
             CONVERT_FEATURES_EX(strList, virtio_gpu_feature_map, bitmap);
         break;
-#endif
-#ifdef CONFIG_VIRTIO_NET
     case VIRTIO_ID_NET:
         features->dev_features =
             CONVERT_FEATURES_EX(strList, virtio_net_feature_map, bitmap);
         break;
-#endif
-#ifdef CONFIG_VIRTIO_SCSI
     case VIRTIO_ID_SCSI:
         features->dev_features =
             CONVERT_FEATURES_EX(strList, virtio_scsi_feature_map, bitmap);
         break;
-#endif
-#ifdef CONFIG_VIRTIO_BALLOON
     case VIRTIO_ID_BALLOON:
         features->dev_features =
             CONVERT_FEATURES_EX(strList, virtio_balloon_feature_map, bitmap);
         break;
-#endif
-#ifdef CONFIG_VIRTIO_IOMMU
     case VIRTIO_ID_IOMMU:
         features->dev_features =
             CONVERT_FEATURES_EX(strList, virtio_iommu_feature_map, bitmap);
         break;
-#endif
-#ifdef CONFIG_VIRTIO_INPUT
     case VIRTIO_ID_INPUT:
         features->dev_features =
             CONVERT_FEATURES_EX(strList, virtio_input_feature_map, bitmap);
         break;
-#endif
-#ifdef CONFIG_VHOST_USER_FS
     case VIRTIO_ID_FS:
         features->dev_features =
             CONVERT_FEATURES_EX(strList, virtio_fs_feature_map, bitmap);
         break;
-#endif
-#ifdef CONFIG_VHOST_VSOCK
     case VIRTIO_ID_VSOCK:
         features->dev_features =
             CONVERT_FEATURES_EX(strList, virtio_vsock_feature_map, bitmap);
         break;
-#endif
-#ifdef CONFIG_VIRTIO_CRYPTO
     case VIRTIO_ID_CRYPTO:
         features->dev_features =
             CONVERT_FEATURES_EX(strList, virtio_crypto_feature_map, bitmap);
         break;
-#endif
-#ifdef CONFIG_VIRTIO_MEM
     case VIRTIO_ID_MEM:
         features->dev_features =
             CONVERT_FEATURES_EX(strList, virtio_mem_feature_map, bitmap);
         break;
-#endif
-#ifdef CONFIG_VIRTIO_I2C_ADAPTER
     case VIRTIO_ID_I2C_ADAPTER:
         features->dev_features =
             CONVERT_FEATURES_EX(strList, virtio_i2c_feature_map, bitmap);
         break;
-#endif
-#ifdef CONFIG_VIRTIO_RNG
     case VIRTIO_ID_RNG:
         features->dev_features =
             CONVERT_FEATURES_EX(strList, virtio_rng_feature_map, bitmap);
         break;
-#endif
-#ifdef CONFIG_VHOST_USER_GPIO
     case VIRTIO_ID_GPIO:
         features->dev_features =
             CONVERT_FEATURES_EX(strList, virtio_gpio_feature_map, bitmap);
         break;
-#endif
     /* No features */
     case VIRTIO_ID_9P:
     case VIRTIO_ID_PMEM:
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index ee397aaf196..d913b94e1ca 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -11,7 +11,7 @@ system_virtio_ss.add(when: 'CONFIG_VHOST_VDPA_DEV', if_true: files('vdpa-dev.c')
 
 specific_virtio_ss = ss.source_set()
 specific_virtio_ss.add(files('virtio.c'))
-specific_virtio_ss.add(files('virtio-qmp.c'))
+system_virtio_ss.add(files('virtio-qmp.c'))
 
 if have_vhost
   system_virtio_ss.add(files('vhost.c'))
-- 
2.52.0

[PATCH v3 11/11] hw/virtio: make all compilation units common

[Philippe Mathieu-Daudé]

From: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/meson.build | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index d913b94e1ca..4f308beb961 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -9,8 +9,7 @@ system_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK_COMMON', if_true: files('vhost-vs
 system_virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu.c'))
 system_virtio_ss.add(when: 'CONFIG_VHOST_VDPA_DEV', if_true: files('vdpa-dev.c'))
 
-specific_virtio_ss = ss.source_set()
-specific_virtio_ss.add(files('virtio.c'))
+system_virtio_ss.add(files('virtio.c'))
 system_virtio_ss.add(files('virtio-qmp.c'))
 
 if have_vhost
@@ -52,10 +51,10 @@ endif
 system_virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock.c'))
 system_virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
 
-specific_virtio_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: files('virtio-balloon.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'))
+system_virtio_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: files('virtio-balloon.c'))
+system_virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: files('vhost-user-fs.c'))
+system_virtio_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: files('virtio-pmem.c'))
+system_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock.c'))
 system_virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
 system_virtio_ss.add(when: 'CONFIG_VIRTIO_NSM', if_true: files('virtio-nsm.c'))
 system_virtio_ss.add(when: 'CONFIG_VIRTIO_NSM', if_true: [files('cbor-helpers.c'), libcbor])
@@ -97,5 +96,4 @@ system_ss.add(when: ['CONFIG_VIRTIO_MD', 'CONFIG_VIRTIO_PCI'],
 
 system_ss.add(files('virtio-hmp-cmds.c'))
 
-specific_ss.add_all(when: 'CONFIG_VIRTIO', if_true: specific_virtio_ss)
 system_ss.add(when: 'CONFIG_ACPI', if_true: files('virtio-acpi.c'))
-- 
2.52.0

Re: [PATCH v3 00/11] single-binary: hw/virtio

[Michael S. Tsirkin]

On Mon, Feb 02, 2026 at 12:29:12AM +0100, Philippe Mathieu-Daudé wrote:
> This series eliminates target specifics in hw/virtio and replace them
> with runtime functions where needed.


This is not bad at all. I feel we do need to have the performance impact
tested, however, just to be sure, since it's probably the hottest path
for I/O.


> v3:
> - include missing SPAPR_MAX_RAM_SLOTS patch
> v2:
> - Reduce runtime calls for performance
> v1:
> https://lore.kernel.org/qemu-devel/20260131020100.1115203-1-pierrick.bouvier@linaro.org/
> 
> Philippe Mathieu-Daudé (6):
>   hw/virtio: Constify virtio_is_big_endian() argument
>   hw/virtio: Introduce VirtIODevice::access_is_big_endian boolean field
>   hw/virtio: Use VirtIODevice::access_is_big_endian field
>   hw/virtio: Reduce virtio_access_is_big_endian() scope
>   hw/virtio: Check target supports legacy bi-endianness at runtime
>   hw/virtio: Replace TARGET_BIG_ENDIAN -> target_big_endian()
> 
> Pierrick Bouvier (5):
>   target-info: add target_base_ppc, target_ppc and target_ppc64
>   hw/ppc/spapr: extract SPAPR_MAX_RAM_SLOTS in a new header
>   hw/virtio/vhost-user: make compilation unit common
>   hw/virtio/virtio-qmp: make compilation unit common
>   hw/virtio: make all compilation units common
> 
>  include/hw/ppc/spapr.h            |  8 +---
>  include/hw/ppc/spapr_common.h     | 17 ++++++++
>  include/hw/virtio/virtio-access.h | 46 ++++++--------------
>  include/hw/virtio/virtio.h        |  7 +++-
>  include/qemu/target-info.h        | 21 ++++++++++
>  hw/virtio/vhost-user.c            | 11 ++---
>  hw/virtio/virtio-qmp.c            | 70 -------------------------------
>  hw/virtio/virtio.c                | 25 ++++++++++-
>  target-info.c                     | 21 ++++++++++
>  hw/virtio/meson.build             | 17 ++++----
>  10 files changed, 113 insertions(+), 130 deletions(-)
>  create mode 100644 include/hw/ppc/spapr_common.h
> 
> -- 
> 2.52.0

Re: [PATCH v3 00/11] single-binary: hw/virtio

[Philippe Mathieu-Daudé]

On 2/2/26 00:29, Philippe Mathieu-Daudé wrote:
> This series eliminates target specifics in hw/virtio and replace them
> with runtime functions where needed.


> Philippe Mathieu-Daudé (6):
>    hw/virtio: Constify virtio_is_big_endian() argument
>    hw/virtio: Introduce VirtIODevice::access_is_big_endian boolean field
>    hw/virtio: Use VirtIODevice::access_is_big_endian field
>    hw/virtio: Reduce virtio_access_is_big_endian() scope
>    hw/virtio: Check target supports legacy bi-endianness at runtime
>    hw/virtio: Replace TARGET_BIG_ENDIAN -> target_big_endian()
> 
> Pierrick Bouvier (5):
>    target-info: add target_base_ppc, target_ppc and target_ppc64
>    hw/ppc/spapr: extract SPAPR_MAX_RAM_SLOTS in a new header
>    hw/virtio/vhost-user: make compilation unit common
>    hw/virtio/virtio-qmp: make compilation unit common
>    hw/virtio: make all compilation units common

FTR I worked on v2 and v3 without previously discussing with Pierrick,
hoping it would help to get a closure on this painful (for me, since
I try to get that fixed since at least 3 years now) topic. I didn't
meant to be disrepectful, and understand this is not a proper teamwork
approach. I will let Pierrick, who kindly offered to help here, follow
his line of work without intervening. Hopefully I won't have to touch
this virtio code ever again.

Regards,

Phil.

Re: [PATCH v3 00/11] single-binary: hw/virtio

[Pierrick Bouvier]

On 2/2/26 3:16 AM, Philippe Mathieu-Daudé wrote:
> On 2/2/26 00:29, Philippe Mathieu-Daudé wrote:
>> This series eliminates target specifics in hw/virtio and replace them
>> with runtime functions where needed.
> 
> 
>> Philippe Mathieu-Daudé (6):
>>     hw/virtio: Constify virtio_is_big_endian() argument
>>     hw/virtio: Introduce VirtIODevice::access_is_big_endian boolean field
>>     hw/virtio: Use VirtIODevice::access_is_big_endian field
>>     hw/virtio: Reduce virtio_access_is_big_endian() scope
>>     hw/virtio: Check target supports legacy bi-endianness at runtime
>>     hw/virtio: Replace TARGET_BIG_ENDIAN -> target_big_endian()
>>
>> Pierrick Bouvier (5):
>>     target-info: add target_base_ppc, target_ppc and target_ppc64
>>     hw/ppc/spapr: extract SPAPR_MAX_RAM_SLOTS in a new header
>>     hw/virtio/vhost-user: make compilation unit common
>>     hw/virtio/virtio-qmp: make compilation unit common
>>     hw/virtio: make all compilation units common
> 
> FTR I worked on v2 and v3 without previously discussing with Pierrick,
> hoping it would help to get a closure on this painful (for me, since
> I try to get that fixed since at least 3 years now) topic. I didn't
> meant to be disrepectful, and understand this is not a proper teamwork
> approach. I will let Pierrick, who kindly offered to help here, follow
> his line of work without intervening. Hopefully I won't have to touch
> this virtio code ever again.
>

Please do not feel bad about it Philippe, we (including myself) now you 
just want to help here. It might just not be the best way to collaborate 
though, since we are not sure of which direction things should take from 
there.

As said previously, your change is valid and worth testing in addition 
to the single-binary series: it just seems to be two different topics 
accidently reaching the same goal.

For the first step, I still would prefer to stick to the simple "convert 
to runtime" approach, benchmark it, and get it merged asap if Michael is 
happy with it. In parallel, it's totally valid to send a separate series 
taking the approach you offered. First merged in, and we all win.

I'll benchmark the v1, and eventually repost this as v4 with the results 
to keep it clear for mailing list users.

> Regards,
> 
> Phil.

Regards,
Pierrick