[PATCH 0/3] hw/virtio/virtio-access.h: remove target specific code

[PATCH 0/3] hw/virtio/virtio-access.h: remove target specific code

[Pierrick Bouvier]

This series eliminates some target specifics in hw/virtio and replace them with
runtime functions where needed, to be able to link virtio code in single-binary.
After a first try on a series [0] doing this change and making all virtio files
common, Richard asked to refactor this part, thus this independent series.

By diving into virtio initialization, I noticed that device_endian is always
storing endianness of cpu associated on device reset. The root issue, is when
dealing with target who dynamically change their endianness during execution.
ppc64 is the main use case, as cpu always boot in BE and most of OS use LE. arm
use case is more limited, as big endian systems are mostly dead nowadays.

Because of this, initialization is tricky, and goes through different hoops to
have the expected value. My first approach has been to try to change this, by
simply setting endianness on the first access. However, it fell flat, because
current_cpu is not always available at this time.
A second approach was to set endianness to LE by default, and change it only
when setting device features, and potentially discovering it's a legacy virtio
device. It brought other issues, that showed that current initialization code,
even though it's complex, does the right thing.
Thus, I focused on refactoring virtio_access_is_big_endian, and noticed that it
was not even needed at this point.

Patches 1 and 2 are refactoring names to clear some confusion.
Patch 3 eliminates virtio_access_is_big_endian, with a lenghty commit message
explaining the change. By doing this, we remove target specifics from
hw/virtio/virtio-access.h.

Of course, performance has been tested, and it is on par with upstream/master.

Results are on 20 runs and expressed in kIOPS:
reference: mean=239.2 std_dev=3.48
with_series: mean=238.1 std_dev=3.56

---

Performance has been measured with this automated fio benchmark [1], with
original instructions from Stefan [2].

Download artifacts and run benchmark with:
$ wget https://github.com/pbo-linaro/qemu-linux-stack/releases/download/build/x86_64_io_benchmark-a55f2d6.tar.xz
$ tar xvf x86_64_io_benchmark-a55f2d6.tar.xz
$ ./run.sh /path/to/qemu-system-x86_64

[0] https://lore.kernel.org/qemu-devel/20260206221908.1451528-1-pierrick.bouvier@linaro.org/
[1] https://github.com/pbo-linaro/qemu-linux-stack/tree/x86_64_io_benchmark
[2] https://lore.kernel.org/qemu-devel/20260202185233.GC405548@fedora/

Pierrick Bouvier (3):
  hw/virtio: add virtio_vdev_is_{modern, legacy}
  hw/virtio: rename virtio_is_big_endian to virtio_vdev_is_big_endian
  hw/virtio: remove virtio_access_is_big_endian

 include/hw/virtio/virtio-access.h | 43 +++++++++----------------------
 include/hw/virtio/virtio.h        | 14 ++++++++--
 hw/net/virtio-net.c               |  6 ++---
 hw/virtio/vhost.c                 |  6 ++---
 hw/virtio/virtio-pci.c            | 10 +++----
 hw/virtio/virtio.c                | 20 +++++++-------
 6 files changed, 45 insertions(+), 54 deletions(-)

-- 
2.47.3

[PATCH 1/3] hw/virtio: add virtio_vdev_is_{modern, legacy}

[Pierrick Bouvier]

This simplifies code compared to having
virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) or
!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1).

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 include/hw/virtio/virtio-access.h |  3 +--
 include/hw/virtio/virtio.h        | 12 +++++++++++-
 hw/net/virtio-net.c               |  2 +-
 hw/virtio/vhost.c                 |  2 +-
 hw/virtio/virtio-pci.c            |  2 +-
 hw/virtio/virtio.c                | 16 ++++++++--------
 6 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
index cd17d0c87eb..324ba907e93 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -30,8 +30,7 @@ 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. */
+    if (virtio_vdev_is_modern(vdev)) {
         return false;
     }
     return true;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 27cd98d2fe1..8f1e4323599 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -468,9 +468,19 @@ static inline bool virtio_host_has_feature(VirtIODevice *vdev,
     return virtio_has_feature(vdev->host_features, fbit);
 }
 
+static inline bool virtio_vdev_is_modern(const VirtIODevice *vdev)
+{
+    return virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
+}
+
+static inline bool virtio_vdev_is_legacy(const VirtIODevice *vdev)
+{
+    return !virtio_vdev_is_modern(vdev);
+}
+
 static inline bool virtio_is_big_endian(VirtIODevice *vdev)
 {
-    if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+    if (virtio_vdev_is_legacy(vdev)) {
         assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN);
         return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG;
     }
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 512a7c02c93..313d3b88400 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -217,7 +217,7 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
     memcpy(&netcfg, config, n->config_size);
 
     if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR) &&
-        !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) &&
+        virtio_vdev_is_legacy(vdev) &&
         memcmp(netcfg.mac, n->mac, ETH_ALEN)) {
         memcpy(n->mac, netcfg.mac, ETH_ALEN);
         qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 52801c1796b..70a112242b0 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1168,7 +1168,7 @@ static void vhost_log_stop(MemoryListener *listener,
  */
 static inline bool vhost_needs_vring_endian(VirtIODevice *vdev)
 {
-    if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+    if (virtio_vdev_is_modern(vdev)) {
         return false;
     }
 #if HOST_BIG_ENDIAN
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 1e8f90df34e..6126187a164 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1449,7 +1449,7 @@ static bool virtio_pci_queue_enabled(DeviceState *d, int n)
     VirtIOPCIProxy *proxy = VIRTIO_PCI(d);
     VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
 
-    if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+    if (virtio_vdev_is_modern(vdev)) {
         return proxy->vqs[n].enabled;
     }
 
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 77ca54e5206..b0987e66d5b 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2279,7 +2279,7 @@ int virtio_set_status(VirtIODevice *vdev, uint8_t val)
     trace_virtio_set_status(vdev, val);
     int ret = 0;
 
-    if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+    if (virtio_vdev_is_modern(vdev)) {
         if (!(vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) &&
             val & VIRTIO_CONFIG_S_FEATURES_OK) {
             ret = virtio_validate_features(vdev);
@@ -2365,7 +2365,7 @@ void virtio_queue_enable(VirtIODevice *vdev, uint32_t queue_index)
      * be re-enabled for new machine types only, and also after
      * being converted to LOG_GUEST_ERROR.
      *
-    if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+    if (virtio_vdev_is_legacy(vdev)) {
         error_report("queue_enable is only supported in devices of virtio "
                      "1.0 or later.");
     }
@@ -2454,7 +2454,7 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align)
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
 
     /* virtio-1 compliant devices cannot change the alignment */
-    if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+    if (virtio_vdev_is_modern(vdev)) {
         error_report("tried to modify queue alignment for virtio-1 device");
         return;
     }
@@ -2753,7 +2753,7 @@ static bool virtio_device_endian_needed(void *opaque)
     VirtIODevice *vdev = opaque;
 
     assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN);
-    if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+    if (virtio_vdev_is_legacy(vdev)) {
         return vdev->device_endian != virtio_default_endian();
     }
     /* Devices conforming to VIRTIO 1.0 or later are always LE. */
@@ -3228,7 +3228,7 @@ int virtio_set_features_ex(VirtIODevice *vdev, const uint64_t *features)
     }
     if (!ret) {
         if (!virtio_device_started(vdev, vdev->status) &&
-            !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+            virtio_vdev_is_legacy(vdev)) {
             vdev->start_on_kick = true;
         }
     }
@@ -3446,7 +3446,7 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
     }
 
     if (!virtio_device_started(vdev, vdev->status) &&
-        !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+        virtio_vdev_is_legacy(vdev)) {
         vdev->start_on_kick = true;
     }
 
@@ -3461,7 +3461,7 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
              * to calculate used and avail ring addresses based on the desc
              * address.
              */
-            if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+            if (virtio_vdev_is_modern(vdev)) {
                 virtio_init_region_cache(vdev, i);
             } else {
                 virtio_queue_update_rings(vdev, i);
@@ -4019,7 +4019,7 @@ void G_GNUC_PRINTF(2, 3) virtio_error(VirtIODevice *vdev, const char *fmt, ...)
     error_vreport(fmt, ap);
     va_end(ap);
 
-    if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+    if (virtio_vdev_is_modern(vdev)) {
         vdev->status = vdev->status | VIRTIO_CONFIG_S_NEEDS_RESET;
         virtio_notify_config(vdev);
     }
-- 
2.47.3

Re: [PATCH 1/3] hw/virtio: add virtio_vdev_is_{modern, legacy}

[BALATON Zoltan]

On Thu, 12 Feb 2026, Pierrick Bouvier wrote:
> This simplifies code compared to having
> virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) or
> !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1).
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
> include/hw/virtio/virtio-access.h |  3 +--
> include/hw/virtio/virtio.h        | 12 +++++++++++-
> hw/net/virtio-net.c               |  2 +-
> hw/virtio/vhost.c                 |  2 +-
> hw/virtio/virtio-pci.c            |  2 +-
> hw/virtio/virtio.c                | 16 ++++++++--------
> 6 files changed, 23 insertions(+), 14 deletions(-)

It's longer than before with two more functions to look up when reading 
the code to understand it so I would not say it's simpler. Maybe more 
readable for some but I don't think it adds much more clarity and so patch 
could just be dropped. But I don't care so if others don't mind or think 
it's better then I don't mind.

Regards,
BALATON Zoltan

> diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
> index cd17d0c87eb..324ba907e93 100644
> --- a/include/hw/virtio/virtio-access.h
> +++ b/include/hw/virtio/virtio-access.h
> @@ -30,8 +30,7 @@ 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. */
> +    if (virtio_vdev_is_modern(vdev)) {
>         return false;
>     }
>     return true;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 27cd98d2fe1..8f1e4323599 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -468,9 +468,19 @@ static inline bool virtio_host_has_feature(VirtIODevice *vdev,
>     return virtio_has_feature(vdev->host_features, fbit);
> }
>
> +static inline bool virtio_vdev_is_modern(const VirtIODevice *vdev)
> +{
> +    return virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
> +}
> +
> +static inline bool virtio_vdev_is_legacy(const VirtIODevice *vdev)
> +{
> +    return !virtio_vdev_is_modern(vdev);
> +}
> +
> static inline bool virtio_is_big_endian(VirtIODevice *vdev)
> {
> -    if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> +    if (virtio_vdev_is_legacy(vdev)) {
>         assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN);
>         return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG;
>     }
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 512a7c02c93..313d3b88400 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -217,7 +217,7 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
>     memcpy(&netcfg, config, n->config_size);
>
>     if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR) &&
> -        !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) &&
> +        virtio_vdev_is_legacy(vdev) &&
>         memcmp(netcfg.mac, n->mac, ETH_ALEN)) {
>         memcpy(n->mac, netcfg.mac, ETH_ALEN);
>         qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 52801c1796b..70a112242b0 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1168,7 +1168,7 @@ static void vhost_log_stop(MemoryListener *listener,
>  */
> static inline bool vhost_needs_vring_endian(VirtIODevice *vdev)
> {
> -    if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> +    if (virtio_vdev_is_modern(vdev)) {
>         return false;
>     }
> #if HOST_BIG_ENDIAN
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 1e8f90df34e..6126187a164 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1449,7 +1449,7 @@ static bool virtio_pci_queue_enabled(DeviceState *d, int n)
>     VirtIOPCIProxy *proxy = VIRTIO_PCI(d);
>     VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
>
> -    if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> +    if (virtio_vdev_is_modern(vdev)) {
>         return proxy->vqs[n].enabled;
>     }
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 77ca54e5206..b0987e66d5b 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2279,7 +2279,7 @@ int virtio_set_status(VirtIODevice *vdev, uint8_t val)
>     trace_virtio_set_status(vdev, val);
>     int ret = 0;
>
> -    if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> +    if (virtio_vdev_is_modern(vdev)) {
>         if (!(vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) &&
>             val & VIRTIO_CONFIG_S_FEATURES_OK) {
>             ret = virtio_validate_features(vdev);
> @@ -2365,7 +2365,7 @@ void virtio_queue_enable(VirtIODevice *vdev, uint32_t queue_index)
>      * be re-enabled for new machine types only, and also after
>      * being converted to LOG_GUEST_ERROR.
>      *
> -    if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> +    if (virtio_vdev_is_legacy(vdev)) {
>         error_report("queue_enable is only supported in devices of virtio "
>                      "1.0 or later.");
>     }
> @@ -2454,7 +2454,7 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align)
>     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>
>     /* virtio-1 compliant devices cannot change the alignment */
> -    if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> +    if (virtio_vdev_is_modern(vdev)) {
>         error_report("tried to modify queue alignment for virtio-1 device");
>         return;
>     }
> @@ -2753,7 +2753,7 @@ static bool virtio_device_endian_needed(void *opaque)
>     VirtIODevice *vdev = opaque;
>
>     assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN);
> -    if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> +    if (virtio_vdev_is_legacy(vdev)) {
>         return vdev->device_endian != virtio_default_endian();
>     }
>     /* Devices conforming to VIRTIO 1.0 or later are always LE. */
> @@ -3228,7 +3228,7 @@ int virtio_set_features_ex(VirtIODevice *vdev, const uint64_t *features)
>     }
>     if (!ret) {
>         if (!virtio_device_started(vdev, vdev->status) &&
> -            !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> +            virtio_vdev_is_legacy(vdev)) {
>             vdev->start_on_kick = true;
>         }
>     }
> @@ -3446,7 +3446,7 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
>     }
>
>     if (!virtio_device_started(vdev, vdev->status) &&
> -        !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> +        virtio_vdev_is_legacy(vdev)) {
>         vdev->start_on_kick = true;
>     }
>
> @@ -3461,7 +3461,7 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
>              * to calculate used and avail ring addresses based on the desc
>              * address.
>              */
> -            if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> +            if (virtio_vdev_is_modern(vdev)) {
>                 virtio_init_region_cache(vdev, i);
>             } else {
>                 virtio_queue_update_rings(vdev, i);
> @@ -4019,7 +4019,7 @@ void G_GNUC_PRINTF(2, 3) virtio_error(VirtIODevice *vdev, const char *fmt, ...)
>     error_vreport(fmt, ap);
>     va_end(ap);
>
> -    if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> +    if (virtio_vdev_is_modern(vdev)) {
>         vdev->status = vdev->status | VIRTIO_CONFIG_S_NEEDS_RESET;
>         virtio_notify_config(vdev);
>     }
>

Re: [PATCH 1/3] hw/virtio: add virtio_vdev_is_{modern, legacy}

[Pierrick Bouvier]

Hi Zoltan,

On 2/12/26 5:19 PM, BALATON Zoltan wrote:
> On Thu, 12 Feb 2026, Pierrick Bouvier wrote:
>> This simplifies code compared to having
>> virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) or
>> !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1).
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>> include/hw/virtio/virtio-access.h |  3 +--
>> include/hw/virtio/virtio.h        | 12 +++++++++++-
>> hw/net/virtio-net.c               |  2 +-
>> hw/virtio/vhost.c                 |  2 +-
>> hw/virtio/virtio-pci.c            |  2 +-
>> hw/virtio/virtio.c                | 16 ++++++++--------
>> 6 files changed, 23 insertions(+), 14 deletions(-)
> 
> It's longer than before with two more functions to look up when reading
> the code to understand it so I would not say it's simpler. Maybe more
> readable for some but I don't think it adds much more clarity and so patch
> could just be dropped. But I don't care so if others don't mind or think
> it's better then I don't mind.
>

patches 1-2 helped me to reach patch 3 conclusion, which is the only one 
that really matters in the end. Hopefully it will help reviewers to 
reach the same conclusion.

If you and others are ok with it, we can just replace 
virtio_access_is_big_endian with virtio_is_big_endian and call it a day 
(even though I still think this is confusing regarding cpu ops having 
the same name).

> Regards,
> BALATON Zoltan
>

Thanks,
Pierrick

Re: [PATCH 1/3] hw/virtio: add virtio_vdev_is_{modern, legacy}

[Philippe Mathieu-Daudé]

On 13/2/26 00:46, Pierrick Bouvier wrote:
> This simplifies code compared to having
> virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) or
> !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1).
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   include/hw/virtio/virtio-access.h |  3 +--
>   include/hw/virtio/virtio.h        | 12 +++++++++++-
>   hw/net/virtio-net.c               |  2 +-
>   hw/virtio/vhost.c                 |  2 +-
>   hw/virtio/virtio-pci.c            |  2 +-
>   hw/virtio/virtio.c                | 16 ++++++++--------
>   6 files changed, 23 insertions(+), 14 deletions(-)


> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 27cd98d2fe1..8f1e4323599 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -468,9 +468,19 @@ static inline bool virtio_host_has_feature(VirtIODevice *vdev,
>       return virtio_has_feature(vdev->host_features, fbit);
>   }
>   
> +static inline bool virtio_vdev_is_modern(const VirtIODevice *vdev)
> +{
> +    return virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
> +}
> +
> +static inline bool virtio_vdev_is_legacy(const VirtIODevice *vdev)
> +{
> +    return !virtio_vdev_is_modern(vdev);
> +}

With retrospective, mentioning 'modern' was a mistake IMHO. But I
understand it was simpler to add 'modern' code without updating the
'legacy' code base. Today I'd rather have 'legacy' explicit in function
names, everything else being the normal / current / modern version,
without having to clarify it.

Thus back to this series I'd rather not introduce virtio_vdev_is_modern
and use !virtio_vdev_is_legacy instead.

Re: [PATCH 1/3] hw/virtio: add virtio_vdev_is_{modern, legacy}

[Pierrick Bouvier]

On 2/18/26 1:00 PM, Philippe Mathieu-Daudé wrote:
> On 13/2/26 00:46, Pierrick Bouvier wrote:
>> This simplifies code compared to having
>> virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) or
>> !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1).
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    include/hw/virtio/virtio-access.h |  3 +--
>>    include/hw/virtio/virtio.h        | 12 +++++++++++-
>>    hw/net/virtio-net.c               |  2 +-
>>    hw/virtio/vhost.c                 |  2 +-
>>    hw/virtio/virtio-pci.c            |  2 +-
>>    hw/virtio/virtio.c                | 16 ++++++++--------
>>    6 files changed, 23 insertions(+), 14 deletions(-)
> 
> 
>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>> index 27cd98d2fe1..8f1e4323599 100644
>> --- a/include/hw/virtio/virtio.h
>> +++ b/include/hw/virtio/virtio.h
>> @@ -468,9 +468,19 @@ static inline bool virtio_host_has_feature(VirtIODevice *vdev,
>>        return virtio_has_feature(vdev->host_features, fbit);
>>    }
>>    
>> +static inline bool virtio_vdev_is_modern(const VirtIODevice *vdev)
>> +{
>> +    return virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
>> +}
>> +
>> +static inline bool virtio_vdev_is_legacy(const VirtIODevice *vdev)
>> +{
>> +    return !virtio_vdev_is_modern(vdev);
>> +}
> 
> With retrospective, mentioning 'modern' was a mistake IMHO. But I
> understand it was simpler to add 'modern' code without updating the
> 'legacy' code base. Today I'd rather have 'legacy' explicit in function
> names, everything else being the normal / current / modern version,
> without having to clarify it.
>

Agree.

> Thus back to this series I'd rather not introduce virtio_vdev_is_modern
> and use !virtio_vdev_is_legacy instead.

Sounds good. I feel it's still easier and clearer to read than 
"!has_feature(1.0)". But as said in my previous answer to Zoltan, I 
don't mind this patch too much, only 3rd one really matter.

Re: [PATCH 1/3] hw/virtio: add virtio_vdev_is_{modern, legacy}

[Pierrick Bouvier]

On 2/18/26 2:17 PM, Pierrick Bouvier wrote:
> On 2/18/26 1:00 PM, Philippe Mathieu-Daudé wrote:
>> On 13/2/26 00:46, Pierrick Bouvier wrote:
>>> This simplifies code compared to having
>>> virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) or
>>> !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1).
>>>
>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> ---
>>>     include/hw/virtio/virtio-access.h |  3 +--
>>>     include/hw/virtio/virtio.h        | 12 +++++++++++-
>>>     hw/net/virtio-net.c               |  2 +-
>>>     hw/virtio/vhost.c                 |  2 +-
>>>     hw/virtio/virtio-pci.c            |  2 +-
>>>     hw/virtio/virtio.c                | 16 ++++++++--------
>>>     6 files changed, 23 insertions(+), 14 deletions(-)
>>
>>
>>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>>> index 27cd98d2fe1..8f1e4323599 100644
>>> --- a/include/hw/virtio/virtio.h
>>> +++ b/include/hw/virtio/virtio.h
>>> @@ -468,9 +468,19 @@ static inline bool virtio_host_has_feature(VirtIODevice *vdev,
>>>         return virtio_has_feature(vdev->host_features, fbit);
>>>     }
>>>     
>>> +static inline bool virtio_vdev_is_modern(const VirtIODevice *vdev)
>>> +{
>>> +    return virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
>>> +}
>>> +
>>> +static inline bool virtio_vdev_is_legacy(const VirtIODevice *vdev)
>>> +{
>>> +    return !virtio_vdev_is_modern(vdev);
>>> +}
>>
>> With retrospective, mentioning 'modern' was a mistake IMHO. But I
>> understand it was simpler to add 'modern' code without updating the
>> 'legacy' code base. Today I'd rather have 'legacy' explicit in function
>> names, everything else being the normal / current / modern version,
>> without having to clarify it.
>>
> 
> Agree.
> 
>> Thus back to this series I'd rather not introduce virtio_vdev_is_modern
>> and use !virtio_vdev_is_legacy instead.
> 
> Sounds good. I feel it's still easier and clearer to read than
> "!has_feature(1.0)". But as said in my previous answer to Zoltan, I
> don't mind this patch too much, only 3rd one really matter.

s/!has_feature(1.0)/has_feature(1.0), thus showing I've been confused 
once more :).

Re: [PATCH 1/3] hw/virtio: add virtio_vdev_is_{modern, legacy}

[BALATON Zoltan]

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

On Wed, 18 Feb 2026, Pierrick Bouvier wrote:
> On 2/18/26 2:17 PM, Pierrick Bouvier wrote:
>> On 2/18/26 1:00 PM, Philippe Mathieu-Daudé wrote:
>>> On 13/2/26 00:46, Pierrick Bouvier wrote:
>>>> This simplifies code compared to having
>>>> virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) or
>>>> !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1).
>>>> 
>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>> ---
>>>>     include/hw/virtio/virtio-access.h |  3 +--
>>>>     include/hw/virtio/virtio.h        | 12 +++++++++++-
>>>>     hw/net/virtio-net.c               |  2 +-
>>>>     hw/virtio/vhost.c                 |  2 +-
>>>>     hw/virtio/virtio-pci.c            |  2 +-
>>>>     hw/virtio/virtio.c                | 16 ++++++++--------
>>>>     6 files changed, 23 insertions(+), 14 deletions(-)
>>> 
>>> 
>>>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>>>> index 27cd98d2fe1..8f1e4323599 100644
>>>> --- a/include/hw/virtio/virtio.h
>>>> +++ b/include/hw/virtio/virtio.h
>>>> @@ -468,9 +468,19 @@ static inline bool 
>>>> virtio_host_has_feature(VirtIODevice *vdev,
>>>>         return virtio_has_feature(vdev->host_features, fbit);
>>>>     }
>>>>     +static inline bool virtio_vdev_is_modern(const VirtIODevice *vdev)
>>>> +{
>>>> +    return virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
>>>> +}
>>>> +
>>>> +static inline bool virtio_vdev_is_legacy(const VirtIODevice *vdev)
>>>> +{
>>>> +    return !virtio_vdev_is_modern(vdev);
>>>> +}
>>> 
>>> With retrospective, mentioning 'modern' was a mistake IMHO. But I
>>> understand it was simpler to add 'modern' code without updating the
>>> 'legacy' code base. Today I'd rather have 'legacy' explicit in function
>>> names, everything else being the normal / current / modern version,
>>> without having to clarify it.
>>> 
>> 
>> Agree.
>> 
>>> Thus back to this series I'd rather not introduce virtio_vdev_is_modern
>>> and use !virtio_vdev_is_legacy instead.
>> 
>> Sounds good. I feel it's still easier and clearer to read than
>> "!has_feature(1.0)". But as said in my previous answer to Zoltan, I
>> don't mind this patch too much, only 3rd one really matter.
>
> s/!has_feature(1.0)/has_feature(1.0), thus showing I've been confused once 
> more :).

The has_feature(1.0) is more specific though. What if 1.0 becomes legacy 
too? Then modern and legacy is more confusing than testing for a specific 
feature. I still think not touching this would be the easiest as the 
introduced legacy/modern does not seem to make it much clearer.

Regards,
BALATON Zoltan

Re: [PATCH 1/3] hw/virtio: add virtio_vdev_is_{modern, legacy}

[Pierrick Bouvier]

On 2/18/26 2:28 PM, BALATON Zoltan wrote:
> On Wed, 18 Feb 2026, Pierrick Bouvier wrote:
>> On 2/18/26 2:17 PM, Pierrick Bouvier wrote:
>>> On 2/18/26 1:00 PM, Philippe Mathieu-Daudé wrote:
>>>> On 13/2/26 00:46, Pierrick Bouvier wrote:
>>>>> This simplifies code compared to having
>>>>> virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) or
>>>>> !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1).
>>>>>
>>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>> ---
>>>>>      include/hw/virtio/virtio-access.h |  3 +--
>>>>>      include/hw/virtio/virtio.h        | 12 +++++++++++-
>>>>>      hw/net/virtio-net.c               |  2 +-
>>>>>      hw/virtio/vhost.c                 |  2 +-
>>>>>      hw/virtio/virtio-pci.c            |  2 +-
>>>>>      hw/virtio/virtio.c                | 16 ++++++++--------
>>>>>      6 files changed, 23 insertions(+), 14 deletions(-)
>>>>
>>>>
>>>>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>>>>> index 27cd98d2fe1..8f1e4323599 100644
>>>>> --- a/include/hw/virtio/virtio.h
>>>>> +++ b/include/hw/virtio/virtio.h
>>>>> @@ -468,9 +468,19 @@ static inline bool
>>>>> virtio_host_has_feature(VirtIODevice *vdev,
>>>>>          return virtio_has_feature(vdev->host_features, fbit);
>>>>>      }
>>>>>      +static inline bool virtio_vdev_is_modern(const VirtIODevice *vdev)
>>>>> +{
>>>>> +    return virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
>>>>> +}
>>>>> +
>>>>> +static inline bool virtio_vdev_is_legacy(const VirtIODevice *vdev)
>>>>> +{
>>>>> +    return !virtio_vdev_is_modern(vdev);
>>>>> +}
>>>>
>>>> With retrospective, mentioning 'modern' was a mistake IMHO. But I
>>>> understand it was simpler to add 'modern' code without updating the
>>>> 'legacy' code base. Today I'd rather have 'legacy' explicit in function
>>>> names, everything else being the normal / current / modern version,
>>>> without having to clarify it.
>>>>
>>>
>>> Agree.
>>>
>>>> Thus back to this series I'd rather not introduce virtio_vdev_is_modern
>>>> and use !virtio_vdev_is_legacy instead.
>>>
>>> Sounds good. I feel it's still easier and clearer to read than
>>> "!has_feature(1.0)". But as said in my previous answer to Zoltan, I
>>> don't mind this patch too much, only 3rd one really matter.
>>
>> s/!has_feature(1.0)/has_feature(1.0), thus showing I've been confused once
>> more :).
> 
> The has_feature(1.0) is more specific though. What if 1.0 becomes legacy
> too? Then modern and legacy is more confusing than testing for a specific
> feature. I still think not touching this would be the easiest as the
> introduced legacy/modern does not seem to make it much clearer.
>

As you can find in official Virtio spec, "legacy" is not a random name 
or something I personally chose, it's the official term for pre 1.0 
devices/interfaces.

https://docs.oasis-open.org/virtio/virtio/v1.4/virtio-v1.4.html#x1-60001

```
1.3.1 Legacy Interface: Terminology

Legacy Interface
is an interface specified by an earlier draft of this specification 
(before 1.0)

Legacy Device
is a device implemented before this specification was released, and 
implementing a legacy interface on the host side
```

So even if VirtIO 2.0 comes out one day, they will have to define or 
change term accordingly.

Your comments are welcome also on patch 3 by the way.

> Regards,
> BALATON Zoltan

Re: [PATCH 1/3] hw/virtio: add virtio_vdev_is_{modern, legacy}

[BALATON Zoltan]

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

On Wed, 18 Feb 2026, Pierrick Bouvier wrote:
> On 2/18/26 2:28 PM, BALATON Zoltan wrote:
>> On Wed, 18 Feb 2026, Pierrick Bouvier wrote:
>>> On 2/18/26 2:17 PM, Pierrick Bouvier wrote:
>>>> On 2/18/26 1:00 PM, Philippe Mathieu-Daudé wrote:
>>>>> On 13/2/26 00:46, Pierrick Bouvier wrote:
>>>>>> This simplifies code compared to having
>>>>>> virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) or
>>>>>> !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1).
>>>>>> 
>>>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>>> ---
>>>>>>      include/hw/virtio/virtio-access.h |  3 +--
>>>>>>      include/hw/virtio/virtio.h        | 12 +++++++++++-
>>>>>>      hw/net/virtio-net.c               |  2 +-
>>>>>>      hw/virtio/vhost.c                 |  2 +-
>>>>>>      hw/virtio/virtio-pci.c            |  2 +-
>>>>>>      hw/virtio/virtio.c                | 16 ++++++++--------
>>>>>>      6 files changed, 23 insertions(+), 14 deletions(-)
>>>>> 
>>>>> 
>>>>>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>>>>>> index 27cd98d2fe1..8f1e4323599 100644
>>>>>> --- a/include/hw/virtio/virtio.h
>>>>>> +++ b/include/hw/virtio/virtio.h
>>>>>> @@ -468,9 +468,19 @@ static inline bool
>>>>>> virtio_host_has_feature(VirtIODevice *vdev,
>>>>>>          return virtio_has_feature(vdev->host_features, fbit);
>>>>>>      }
>>>>>>      +static inline bool virtio_vdev_is_modern(const VirtIODevice 
>>>>>> *vdev)
>>>>>> +{
>>>>>> +    return virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
>>>>>> +}
>>>>>> +
>>>>>> +static inline bool virtio_vdev_is_legacy(const VirtIODevice *vdev)
>>>>>> +{
>>>>>> +    return !virtio_vdev_is_modern(vdev);
>>>>>> +}
>>>>> 
>>>>> With retrospective, mentioning 'modern' was a mistake IMHO. But I
>>>>> understand it was simpler to add 'modern' code without updating the
>>>>> 'legacy' code base. Today I'd rather have 'legacy' explicit in function
>>>>> names, everything else being the normal / current / modern version,
>>>>> without having to clarify it.
>>>>> 
>>>> 
>>>> Agree.
>>>> 
>>>>> Thus back to this series I'd rather not introduce virtio_vdev_is_modern
>>>>> and use !virtio_vdev_is_legacy instead.
>>>> 
>>>> Sounds good. I feel it's still easier and clearer to read than
>>>> "!has_feature(1.0)". But as said in my previous answer to Zoltan, I
>>>> don't mind this patch too much, only 3rd one really matter.
>>> 
>>> s/!has_feature(1.0)/has_feature(1.0), thus showing I've been confused once
>>> more :).
>> 
>> The has_feature(1.0) is more specific though. What if 1.0 becomes legacy
>> too? Then modern and legacy is more confusing than testing for a specific
>> feature. I still think not touching this would be the easiest as the
>> introduced legacy/modern does not seem to make it much clearer.
>> 
>
> As you can find in official Virtio spec, "legacy" is not a random name or 
> something I personally chose, it's the official term for pre 1.0 
> devices/interfaces.
>
> https://docs.oasis-open.org/virtio/virtio/v1.4/virtio-v1.4.html#x1-60001
>
> ```
> 1.3.1 Legacy Interface: Terminology
>
> Legacy Interface
> is an interface specified by an earlier draft of this specification (before 
> 1.0)
>
> Legacy Device
> is a device implemented before this specification was released, and 
> implementing a legacy interface on the host side
> ```
>
> So even if VirtIO 2.0 comes out one day, they will have to define or change 
> term accordingly.

OK in that case it's better but modern would still be ambiguous. But as I 
said I think current test for feature is OK in my opinion. That's all I 
can add to this.

> Your comments are welcome also on patch 3 by the way.

Sorry I can't comment on that as I don't know this code. I also don't use 
virtio with PPC machines but I know some people use it so they'll probably 
complain if this broke something.

But maybe everybody uses current virtio now and the old one is only there 
for compatibility. Is this maybe referenced from hw/core/machine.c in a 
compatibility property that could be removed or is it user settable and 
still supported? There was another thread on these compatibility 
properties and adding a way to mark and deprecate compatibility properties 
for easier removal but that did not end yet. You'd probably just want to 
make this series do the least changes needed and then move on.

Regards,
BALATON Zoltan

Re: [PATCH 1/3] hw/virtio: add virtio_vdev_is_{modern, legacy}

[Pierrick Bouvier]

On 2/18/26 3:46 PM, BALATON Zoltan wrote:
> On Wed, 18 Feb 2026, Pierrick Bouvier wrote:
>> On 2/18/26 2:28 PM, BALATON Zoltan wrote:
>>> On Wed, 18 Feb 2026, Pierrick Bouvier wrote:
>>>> On 2/18/26 2:17 PM, Pierrick Bouvier wrote:
>>>>> On 2/18/26 1:00 PM, Philippe Mathieu-Daudé wrote:
>>>>>> On 13/2/26 00:46, Pierrick Bouvier wrote:
>>>>>>> This simplifies code compared to having
>>>>>>> virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) or
>>>>>>> !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1).
>>>>>>>
>>>>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>>>> ---
>>>>>>>       include/hw/virtio/virtio-access.h |  3 +--
>>>>>>>       include/hw/virtio/virtio.h        | 12 +++++++++++-
>>>>>>>       hw/net/virtio-net.c               |  2 +-
>>>>>>>       hw/virtio/vhost.c                 |  2 +-
>>>>>>>       hw/virtio/virtio-pci.c            |  2 +-
>>>>>>>       hw/virtio/virtio.c                | 16 ++++++++--------
>>>>>>>       6 files changed, 23 insertions(+), 14 deletions(-)
>>>>>>
>>>>>>
>>>>>>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>>>>>>> index 27cd98d2fe1..8f1e4323599 100644
>>>>>>> --- a/include/hw/virtio/virtio.h
>>>>>>> +++ b/include/hw/virtio/virtio.h
>>>>>>> @@ -468,9 +468,19 @@ static inline bool
>>>>>>> virtio_host_has_feature(VirtIODevice *vdev,
>>>>>>>           return virtio_has_feature(vdev->host_features, fbit);
>>>>>>>       }
>>>>>>>       +static inline bool virtio_vdev_is_modern(const VirtIODevice
>>>>>>> *vdev)
>>>>>>> +{
>>>>>>> +    return virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
>>>>>>> +}
>>>>>>> +
>>>>>>> +static inline bool virtio_vdev_is_legacy(const VirtIODevice *vdev)
>>>>>>> +{
>>>>>>> +    return !virtio_vdev_is_modern(vdev);
>>>>>>> +}
>>>>>>
>>>>>> With retrospective, mentioning 'modern' was a mistake IMHO. But I
>>>>>> understand it was simpler to add 'modern' code without updating the
>>>>>> 'legacy' code base. Today I'd rather have 'legacy' explicit in function
>>>>>> names, everything else being the normal / current / modern version,
>>>>>> without having to clarify it.
>>>>>>
>>>>>
>>>>> Agree.
>>>>>
>>>>>> Thus back to this series I'd rather not introduce virtio_vdev_is_modern
>>>>>> and use !virtio_vdev_is_legacy instead.
>>>>>
>>>>> Sounds good. I feel it's still easier and clearer to read than
>>>>> "!has_feature(1.0)". But as said in my previous answer to Zoltan, I
>>>>> don't mind this patch too much, only 3rd one really matter.
>>>>
>>>> s/!has_feature(1.0)/has_feature(1.0), thus showing I've been confused once
>>>> more :).
>>>
>>> The has_feature(1.0) is more specific though. What if 1.0 becomes legacy
>>> too? Then modern and legacy is more confusing than testing for a specific
>>> feature. I still think not touching this would be the easiest as the
>>> introduced legacy/modern does not seem to make it much clearer.
>>>
>>
>> As you can find in official Virtio spec, "legacy" is not a random name or
>> something I personally chose, it's the official term for pre 1.0
>> devices/interfaces.
>>
>> https://docs.oasis-open.org/virtio/virtio/v1.4/virtio-v1.4.html#x1-60001
>>
>> ```
>> 1.3.1 Legacy Interface: Terminology
>>
>> Legacy Interface
>> is an interface specified by an earlier draft of this specification (before
>> 1.0)
>>
>> Legacy Device
>> is a device implemented before this specification was released, and
>> implementing a legacy interface on the host side
>> ```
>>
>> So even if VirtIO 2.0 comes out one day, they will have to define or change
>> term accordingly.
> 
> OK in that case it's better but modern would still be ambiguous. But as I
> said I think current test for feature is OK in my opinion. That's all I
> can add to this.
> 
>> Your comments are welcome also on patch 3 by the way.
> 
> Sorry I can't comment on that as I don't know this code. I also don't use
> virtio with PPC machines but I know some people use it so they'll probably
> complain if this broke something.
>

I did my fair share of tests, including reproducing a ppc64el 
environment to make sure migration was not broken. I hope someone 
comfortable with this can give a reviewed-by and that Michael will 
happily pick the patch.

> But maybe everybody uses current virtio now and the old one is only there
> for compatibility. Is this maybe referenced from hw/core/machine.c in a
> compatibility property that could be removed or is it user settable and
> still supported? There was another thread on these compatibility
> properties and adding a way to mark and deprecate compatibility properties
> for easier removal but that did not end yet. You'd probably just want to
> make this series do the least changes needed and then move on.
>

Philippe went down this way, trying to remove legacy support completely, 
but the community answered it had to stay and could not be 
deprecated/removed. This position might change in the future, but we 
need to find a solution now to remove target specifics, thus the current 
solution.

> Regards,
> BALATON Zoltan

Thanks,
Pierrick

Re: [PATCH 1/3] hw/virtio: add virtio_vdev_is_{modern, legacy}

[BALATON Zoltan]

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

On Wed, 18 Feb 2026, Pierrick Bouvier wrote:
> On 2/18/26 3:46 PM, BALATON Zoltan wrote:
>> On Wed, 18 Feb 2026, Pierrick Bouvier wrote:
>>> On 2/18/26 2:28 PM, BALATON Zoltan wrote:
>>>> On Wed, 18 Feb 2026, Pierrick Bouvier wrote:
>>>>> On 2/18/26 2:17 PM, Pierrick Bouvier wrote:
>>>>>> On 2/18/26 1:00 PM, Philippe Mathieu-Daudé wrote:
>>>>>>> On 13/2/26 00:46, Pierrick Bouvier wrote:
>>>>>>>> This simplifies code compared to having
>>>>>>>> virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) or
>>>>>>>> !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1).
>>>>>>>> 
>>>>>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>>>>> ---
>>>>>>>>       include/hw/virtio/virtio-access.h |  3 +--
>>>>>>>>       include/hw/virtio/virtio.h        | 12 +++++++++++-
>>>>>>>>       hw/net/virtio-net.c               |  2 +-
>>>>>>>>       hw/virtio/vhost.c                 |  2 +-
>>>>>>>>       hw/virtio/virtio-pci.c            |  2 +-
>>>>>>>>       hw/virtio/virtio.c                | 16 ++++++++--------
>>>>>>>>       6 files changed, 23 insertions(+), 14 deletions(-)
>>>>>>> 
>>>>>>> 
>>>>>>>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>>>>>>>> index 27cd98d2fe1..8f1e4323599 100644
>>>>>>>> --- a/include/hw/virtio/virtio.h
>>>>>>>> +++ b/include/hw/virtio/virtio.h
>>>>>>>> @@ -468,9 +468,19 @@ static inline bool
>>>>>>>> virtio_host_has_feature(VirtIODevice *vdev,
>>>>>>>>           return virtio_has_feature(vdev->host_features, fbit);
>>>>>>>>       }
>>>>>>>>       +static inline bool virtio_vdev_is_modern(const VirtIODevice
>>>>>>>> *vdev)
>>>>>>>> +{
>>>>>>>> +    return virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static inline bool virtio_vdev_is_legacy(const VirtIODevice *vdev)
>>>>>>>> +{
>>>>>>>> +    return !virtio_vdev_is_modern(vdev);
>>>>>>>> +}
>>>>>>> 
>>>>>>> With retrospective, mentioning 'modern' was a mistake IMHO. But I
>>>>>>> understand it was simpler to add 'modern' code without updating the
>>>>>>> 'legacy' code base. Today I'd rather have 'legacy' explicit in 
>>>>>>> function
>>>>>>> names, everything else being the normal / current / modern version,
>>>>>>> without having to clarify it.
>>>>>>> 
>>>>>> 
>>>>>> Agree.
>>>>>> 
>>>>>>> Thus back to this series I'd rather not introduce 
>>>>>>> virtio_vdev_is_modern
>>>>>>> and use !virtio_vdev_is_legacy instead.
>>>>>> 
>>>>>> Sounds good. I feel it's still easier and clearer to read than
>>>>>> "!has_feature(1.0)". But as said in my previous answer to Zoltan, I
>>>>>> don't mind this patch too much, only 3rd one really matter.
>>>>> 
>>>>> s/!has_feature(1.0)/has_feature(1.0), thus showing I've been confused 
>>>>> once
>>>>> more :).
>>>> 
>>>> The has_feature(1.0) is more specific though. What if 1.0 becomes legacy
>>>> too? Then modern and legacy is more confusing than testing for a specific
>>>> feature. I still think not touching this would be the easiest as the
>>>> introduced legacy/modern does not seem to make it much clearer.
>>>> 
>>> 
>>> As you can find in official Virtio spec, "legacy" is not a random name or
>>> something I personally chose, it's the official term for pre 1.0
>>> devices/interfaces.
>>> 
>>> https://docs.oasis-open.org/virtio/virtio/v1.4/virtio-v1.4.html#x1-60001
>>> 
>>> ```
>>> 1.3.1 Legacy Interface: Terminology
>>> 
>>> Legacy Interface
>>> is an interface specified by an earlier draft of this specification 
>>> (before
>>> 1.0)
>>> 
>>> Legacy Device
>>> is a device implemented before this specification was released, and
>>> implementing a legacy interface on the host side
>>> ```
>>> 
>>> So even if VirtIO 2.0 comes out one day, they will have to define or 
>>> change
>>> term accordingly.
>> 
>> OK in that case it's better but modern would still be ambiguous. But as I
>> said I think current test for feature is OK in my opinion. That's all I
>> can add to this.
>> 
>>> Your comments are welcome also on patch 3 by the way.
>> 
>> Sorry I can't comment on that as I don't know this code. I also don't use
>> virtio with PPC machines but I know some people use it so they'll probably
>> complain if this broke something.
>> 
>
> I did my fair share of tests, including reproducing a ppc64el environment to 
> make sure migration was not broken. I hope someone comfortable with this can 
> give a reviewed-by and that Michael will happily pick the patch.
>
>> But maybe everybody uses current virtio now and the old one is only there
>> for compatibility. Is this maybe referenced from hw/core/machine.c in a
>> compatibility property that could be removed or is it user settable and
>> still supported? There was another thread on these compatibility
>> properties and adding a way to mark and deprecate compatibility properties
>> for easier removal but that did not end yet. You'd probably just want to
>> make this series do the least changes needed and then move on.
>> 
>
> Philippe went down this way, trying to remove legacy support completely, but 
> the community answered it had to stay and could not be deprecated/removed. 
> This position might change in the future, but we need to find a solution now 
> to remove target specifics, thus the current solution.

I don't know if that's related but in hw/core/machine.c:

GlobalProperty hw_compat_5_0[] = {
...
     { "virtio-device", "x-disable-legacy-check", "true" },
};

And the oldest visible machine is now 5.1 with everything older than 8.0 
deprecated due to auto deprecation. But we're behind with removing older 
code which is now at 2.8 so maybe we're deprecating things faster than we 
can remove and clean up old code so that may cause users problems before 
the code can benefit from it. Maybe we could slow down with auto 
deprecation until removal of old machine is catching up?

Regards,
BALATON Zoltan

[PATCH 2/3] hw/virtio: rename virtio_is_big_endian to virtio_vdev_is_big_endian

[Pierrick Bouvier]

Renaming this function removes the confusion with
existing virtio_is_big_endian cpu ops.

Indeed, virtio_vdev_is_big_endian is *not* calling cpu
virtio_is_big_endian everytime.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 include/hw/virtio/virtio-access.h | 2 +-
 include/hw/virtio/virtio.h        | 2 +-
 hw/net/virtio-net.c               | 4 ++--
 hw/virtio/vhost.c                 | 4 ++--
 hw/virtio/virtio-pci.c            | 8 ++++----
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
index 324ba907e93..0c5514178e4 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -28,7 +28,7 @@
 static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
 {
 #if defined(LEGACY_VIRTIO_IS_BIENDIAN)
-    return virtio_is_big_endian(vdev);
+    return virtio_vdev_is_big_endian(vdev);
 #elif TARGET_BIG_ENDIAN
     if (virtio_vdev_is_modern(vdev)) {
         return false;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 8f1e4323599..ed1a0967ed8 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -478,7 +478,7 @@ static inline bool virtio_vdev_is_legacy(const VirtIODevice *vdev)
     return !virtio_vdev_is_modern(vdev);
 }
 
-static inline bool virtio_is_big_endian(VirtIODevice *vdev)
+static inline bool virtio_vdev_is_big_endian(VirtIODevice *vdev)
 {
     if (virtio_vdev_is_legacy(vdev)) {
         assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN);
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 313d3b88400..ec85e5a949a 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -301,7 +301,7 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
         if (n->needs_vnet_hdr_swap) {
             error_report("backend does not support %s vnet headers; "
                          "falling back on userspace virtio",
-                         virtio_is_big_endian(vdev) ? "BE" : "LE");
+                         virtio_vdev_is_big_endian(vdev) ? "BE" : "LE");
             return;
         }
 
@@ -343,7 +343,7 @@ static int virtio_net_set_vnet_endian_one(VirtIODevice *vdev,
                                           NetClientState *peer,
                                           bool enable)
 {
-    if (virtio_is_big_endian(vdev)) {
+    if (virtio_vdev_is_big_endian(vdev)) {
         return qemu_set_vnet_be(peer, enable);
     } else {
         return qemu_set_vnet_le(peer, enable);
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 70a112242b0..1f5e70ad197 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1306,7 +1306,7 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
 
     if (vhost_needs_vring_endian(vdev)) {
         r = vhost_virtqueue_set_vring_endian_legacy(dev,
-                                                    virtio_is_big_endian(vdev),
+                                                    virtio_vdev_is_big_endian(vdev),
                                                     vhost_vq_index);
         if (r) {
             return r;
@@ -1423,7 +1423,7 @@ static int do_vhost_virtqueue_stop(struct vhost_dev *dev,
      */
     if (vhost_needs_vring_endian(vdev)) {
         vhost_virtqueue_set_vring_endian_legacy(dev,
-                                                !virtio_is_big_endian(vdev),
+                                                !virtio_vdev_is_big_endian(vdev),
                                                 vhost_vq_index);
     }
 
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 6126187a164..33c5cec01cd 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -586,13 +586,13 @@ static uint64_t virtio_pci_config_read(void *opaque, hwaddr addr,
         break;
     case 2:
         val = virtio_config_readw(vdev, addr);
-        if (virtio_is_big_endian(vdev)) {
+        if (virtio_vdev_is_big_endian(vdev)) {
             val = bswap16(val);
         }
         break;
     case 4:
         val = virtio_config_readl(vdev, addr);
-        if (virtio_is_big_endian(vdev)) {
+        if (virtio_vdev_is_big_endian(vdev)) {
             val = bswap32(val);
         }
         break;
@@ -625,13 +625,13 @@ static void virtio_pci_config_write(void *opaque, hwaddr addr,
         virtio_config_writeb(vdev, addr, val);
         break;
     case 2:
-        if (virtio_is_big_endian(vdev)) {
+        if (virtio_vdev_is_big_endian(vdev)) {
             val = bswap16(val);
         }
         virtio_config_writew(vdev, addr, val);
         break;
     case 4:
-        if (virtio_is_big_endian(vdev)) {
+        if (virtio_vdev_is_big_endian(vdev)) {
             val = bswap32(val);
         }
         virtio_config_writel(vdev, addr, val);
-- 
2.47.3

Re: [PATCH 2/3] hw/virtio: rename virtio_is_big_endian to virtio_vdev_is_big_endian

[Philippe Mathieu-Daudé]

On 13/2/26 00:46, Pierrick Bouvier wrote:
> Renaming this function removes the confusion with
> existing virtio_is_big_endian cpu ops.
> 
> Indeed, virtio_vdev_is_big_endian is *not* calling cpu
> virtio_is_big_endian everytime.
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   include/hw/virtio/virtio-access.h | 2 +-
>   include/hw/virtio/virtio.h        | 2 +-
>   hw/net/virtio-net.c               | 4 ++--
>   hw/virtio/vhost.c                 | 4 ++--
>   hw/virtio/virtio-pci.c            | 8 ++++----
>   5 files changed, 10 insertions(+), 10 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 3/3] hw/virtio: remove virtio_access_is_big_endian

[Pierrick Bouvier]

Thanks to previous refactoring, we can see more easily it is strictly
equivalent to always call virtio_vdev_is_big_endian.

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

The key is to understand that vdev->device_endian is initialized as
expected. It always contains cpu endianness, and not
device endianness, ignoring if device is legacy or not.

By default, it's initialized to vdev->device_endian =
virtio_default_endian(), which matches target default endianness.
Then, on virtio_reset, it will be initialized with current_cpu
endianness (if there is one current_cpu).

void virtio_reset() {
    ...
    if (current_cpu) {
        /* Guest initiated reset */
        vdev->device_endian = virtio_current_cpu_endian();
    } else {
        /* System reset */
        vdev->device_endian = virtio_default_endian();
    }

Now, we can see how existing virtio_access_is_big_endian is equivalent
to virtio_vdev_is_big_endian. Let's break the existing function in its 3
variants, and compare that to virtio_vdev_is_big_endian.

static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
- #if defined(LEGACY_VIRTIO_IS_BIENDIAN)
    return virtio_vdev_is_big_endian(vdev);
  This is the exact replacement we did, so equivalent.
- #elif TARGET_BIG_ENDIAN
    if (virtio_vdev_is_modern(vdev)) {
        return false;
    }
    return true;

  we know target_is_big_endian(), so
  vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG.
  if (virtio_vdev_is_legacy(vdev)) {
      return VIRTIO_DEVICE_ENDIAN_BIG == VIRTIO_DEVICE_ENDIAN_BIG;
  }
  return false;
  It's written in opposite style compared to existing code (if legacy vs
  if modern), but it's strictly equivalent.
- #else
    return false;
  we know !target_is_big_endian(), so
  vdev->device_endian == VIRTIO_DEVICE_ENDIAN_LITTLE.
  if virtio_vdev_is_legacy(vdev) {
    return VIRTIO_DEVICE_ENDIAN_LITTLE == VIRTIO_DEVICE_ENDIAN_BIG;
  }
  return false;
  So it always return false, as expected.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 include/hw/virtio/virtio-access.h | 42 +++++++++----------------------
 hw/virtio/virtio.c                |  4 +--
 2 files changed, 14 insertions(+), 32 deletions(-)

diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
index 0c5514178e4..506be642c9f 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -21,27 +21,9 @@
 #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_vdev_is_big_endian(vdev);
-#elif TARGET_BIG_ENDIAN
-    if (virtio_vdev_is_modern(vdev)) {
-        return false;
-    }
-    return true;
-#else
-    return false;
-#endif
-}
-
 static inline void virtio_stw_p(VirtIODevice *vdev, void *ptr, uint16_t v)
 {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (virtio_vdev_is_big_endian(vdev)) {
         stw_be_p(ptr, v);
     } else {
         stw_le_p(ptr, v);
@@ -50,7 +32,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 (virtio_vdev_is_big_endian(vdev)) {
         stl_be_p(ptr, v);
     } else {
         stl_le_p(ptr, v);
@@ -59,7 +41,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 (virtio_vdev_is_big_endian(vdev)) {
         stq_be_p(ptr, v);
     } else {
         stq_le_p(ptr, v);
@@ -68,7 +50,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 (virtio_vdev_is_big_endian(vdev)) {
         return lduw_be_p(ptr);
     } else {
         return lduw_le_p(ptr);
@@ -77,7 +59,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 (virtio_vdev_is_big_endian(vdev)) {
         return ldl_be_p(ptr);
     } else {
         return ldl_le_p(ptr);
@@ -86,7 +68,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 (virtio_vdev_is_big_endian(vdev)) {
         return ldq_be_p(ptr);
     } else {
         return ldq_le_p(ptr);
@@ -96,9 +78,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 virtio_vdev_is_big_endian(vdev) ? s : bswap16(s);
 #else
-    return virtio_access_is_big_endian(vdev) ? bswap16(s) : s;
+    return virtio_vdev_is_big_endian(vdev) ? bswap16(s) : s;
 #endif
 }
 
@@ -110,9 +92,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 virtio_vdev_is_big_endian(vdev) ? s : bswap32(s);
 #else
-    return virtio_access_is_big_endian(vdev) ? bswap32(s) : s;
+    return virtio_vdev_is_big_endian(vdev) ? bswap32(s) : s;
 #endif
 }
 
@@ -124,9 +106,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 virtio_vdev_is_big_endian(vdev) ? s : bswap64(s);
 #else
-    return virtio_access_is_big_endian(vdev) ? bswap64(s) : s;
+    return virtio_vdev_is_big_endian(vdev) ? bswap64(s) : s;
 #endif
 }
 
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index b0987e66d5b..2ae238001dc 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -222,7 +222,7 @@ static inline uint16_t virtio_lduw_phys_cached(VirtIODevice *vdev,
                                                MemoryRegionCache *cache,
                                                hwaddr pa)
 {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (virtio_vdev_is_big_endian(vdev)) {
         return lduw_be_phys_cached(cache, pa);
     }
     return lduw_le_phys_cached(cache, pa);
@@ -232,7 +232,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 (virtio_vdev_is_big_endian(vdev)) {
         stw_be_phys_cached(cache, pa, value);
     } else {
         stw_le_phys_cached(cache, pa, value);
-- 
2.47.3

Re: [PATCH 3/3] hw/virtio: remove virtio_access_is_big_endian

[Philippe Mathieu-Daudé]

On 13/2/26 00:46, Pierrick Bouvier wrote:
> Thanks to previous refactoring, we can see more easily it is strictly
> equivalent to always call virtio_vdev_is_big_endian.


> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   include/hw/virtio/virtio-access.h | 42 +++++++++----------------------
>   hw/virtio/virtio.c                |  4 +--
>   2 files changed, 14 insertions(+), 32 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 0/3] hw/virtio/virtio-access.h: remove target specific code

[Pierrick Bouvier]

On 2/12/26 3:45 PM, Pierrick Bouvier wrote:
> This series eliminates some target specifics in hw/virtio and replace them with
> runtime functions where needed, to be able to link virtio code in single-binary.
> After a first try on a series [0] doing this change and making all virtio files
> common, Richard asked to refactor this part, thus this independent series.
> 
> By diving into virtio initialization, I noticed that device_endian is always
> storing endianness of cpu associated on device reset. The root issue, is when
> dealing with target who dynamically change their endianness during execution.
> ppc64 is the main use case, as cpu always boot in BE and most of OS use LE. arm
> use case is more limited, as big endian systems are mostly dead nowadays.
> 
> Because of this, initialization is tricky, and goes through different hoops to
> have the expected value. My first approach has been to try to change this, by
> simply setting endianness on the first access. However, it fell flat, because
> current_cpu is not always available at this time.
> A second approach was to set endianness to LE by default, and change it only
> when setting device features, and potentially discovering it's a legacy virtio
> device. It brought other issues, that showed that current initialization code,
> even though it's complex, does the right thing.
> Thus, I focused on refactoring virtio_access_is_big_endian, and noticed that it
> was not even needed at this point.
> 
> Patches 1 and 2 are refactoring names to clear some confusion.
> Patch 3 eliminates virtio_access_is_big_endian, with a lenghty commit message
> explaining the change. By doing this, we remove target specifics from
> hw/virtio/virtio-access.h.
> 
> Of course, performance has been tested, and it is on par with upstream/master.
> 
> Results are on 20 runs and expressed in kIOPS:
> reference: mean=239.2 std_dev=3.48
> with_series: mean=238.1 std_dev=3.56
> 
> ---
> 
> Performance has been measured with this automated fio benchmark [1], with
> original instructions from Stefan [2].
> 
> Download artifacts and run benchmark with:
> $ wget https://github.com/pbo-linaro/qemu-linux-stack/releases/download/build/x86_64_io_benchmark-a55f2d6.tar.xz
> $ tar xvf x86_64_io_benchmark-a55f2d6.tar.xz
> $ ./run.sh /path/to/qemu-system-x86_64
> 
> [0] https://lore.kernel.org/qemu-devel/20260206221908.1451528-1-pierrick.bouvier@linaro.org/
> [1] https://github.com/pbo-linaro/qemu-linux-stack/tree/x86_64_io_benchmark
> [2] https://lore.kernel.org/qemu-devel/20260202185233.GC405548@fedora/
> 
> Pierrick Bouvier (3):
>    hw/virtio: add virtio_vdev_is_{modern, legacy}
>    hw/virtio: rename virtio_is_big_endian to virtio_vdev_is_big_endian
>    hw/virtio: remove virtio_access_is_big_endian
> 
>   include/hw/virtio/virtio-access.h | 43 +++++++++----------------------
>   include/hw/virtio/virtio.h        | 14 ++++++++--
>   hw/net/virtio-net.c               |  6 ++---
>   hw/virtio/vhost.c                 |  6 ++---
>   hw/virtio/virtio-pci.c            | 10 +++----
>   hw/virtio/virtio.c                | 20 +++++++-------
>   6 files changed, 45 insertions(+), 54 deletions(-)
> 

polite ping on this short series, that is needed to integrate virtio in 
the future single-binary
.
Richard, does it clarify for you what the original version was doing?

Regards,
Pierrick

Re: [PATCH 0/3] hw/virtio/virtio-access.h: remove target specific code

[Philippe Mathieu-Daudé]

Hi Pierrick,

On 13/2/26 00:45, Pierrick Bouvier wrote:
> This series eliminates some target specifics in hw/virtio and replace them with
> runtime functions where needed, to be able to link virtio code in single-binary.
> After a first try on a series [0] doing this change and making all virtio files
> common, Richard asked to refactor this part, thus this independent series.

> Pierrick Bouvier (3):
>    hw/virtio: add virtio_vdev_is_{modern, legacy}
>    hw/virtio: rename virtio_is_big_endian to virtio_vdev_is_big_endian
>    hw/virtio: remove virtio_access_is_big_endian

Patch #2 has been merged as commit 6325407f67d.

Since we don't have feedback from the maintainers Cc'ed, I took
the liberty to rebase your series, trying to address Zoltan's
concerns on patch #1. Patch #3 is split between the complex and
trivial changes. The patches look like:

-- >8 --
commit 817ee1acda278b484b750b6abbba7d829bb124bc
Author: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Date:   Tue Feb 24 13:00:28 2026 +0100

     hw/virtio: Add virtio_vdev_is_legacy()

     This simplifies code compared to having
     virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) or
     !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1).

     Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
     Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 9dd93cf965a..42d20899390 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -470,5 +470,10 @@ static inline bool 
virtio_host_has_feature(VirtIODevice *vdev,

+static inline bool virtio_vdev_is_legacy(const VirtIODevice *vdev)
+{
+    return !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
+}
+
  static inline bool virtio_vdev_is_big_endian(const VirtIODevice *vdev)
  {
-    if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+    if (virtio_vdev_is_legacy(vdev)) {
          assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN);
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index b4cdb7762f9..b9dc4ed13ba 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1170,10 +1170,8 @@ static inline bool 
vhost_needs_vring_endian(VirtIODevice *vdev)
  {
-    if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
-        return false;
+    if (virtio_vdev_is_legacy(vdev)) {
+        return vdev->device_endian == (HOST_BIG_ENDIAN
+                                       ? VIRTIO_DEVICE_ENDIAN_LITTLE
+                                       : VIRTIO_DEVICE_ENDIAN_BIG);
      }
-#if HOST_BIG_ENDIAN
-    return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_LITTLE;
-#else
-    return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG;
-#endif
+    return false;
  }
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index c7b5a79b936..31b566c6ae3 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1451,7 +1451,7 @@ static bool virtio_pci_queue_enabled(DeviceState 
*d, int n)

-    if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
-        return proxy->vqs[n].enabled;
+    if (virtio_vdev_is_legacy(vdev)) {
+        return virtio_queue_enabled_legacy(vdev, n);
      }

-    return virtio_queue_enabled_legacy(vdev, n);
+    return proxy->vqs[n].enabled;
  }
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index e9d55329525..c0c4599b586 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2755,3 +2755,3 @@ static bool virtio_device_endian_needed(void *opaque)
      assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN);
-    if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+    if (virtio_vdev_is_legacy(vdev)) {
          return vdev->device_endian != virtio_default_endian();
@@ -3462,6 +3462,6 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int 
version_id)
               */
-            if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
-                virtio_init_region_cache(vdev, i);
-            } else {
+            if (virtio_vdev_is_legacy(vdev)) {
                  virtio_queue_update_rings(vdev, i);
+            } else {
+                virtio_init_region_cache(vdev, i);
              }

commit 2beb9dea190039355af8a8fa16c389eed45491ca
Author: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Date:   Tue Feb 24 13:07:02 2026 +0100

     hw/virtio: Simplify virtio_access_is_big_endian()

     Thanks to previous refactoring, we can see more easily it is strictly
     equivalent to always call virtio_vdev_is_big_endian.

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

     The key is to understand that vdev->device_endian is initialized as
     expected. It always contains cpu endianness, and not
     device endianness, ignoring if device is legacy or not.

     By default, it's initialized to vdev->device_endian =
     virtio_default_endian(), which matches target default endianness.
     Then, on virtio_reset, it will be initialized with current_cpu
     endianness (if there is one current_cpu).

     void virtio_reset() {
         ...
         if (current_cpu) {
             /* Guest initiated reset */
             vdev->device_endian = virtio_current_cpu_endian();
         } else {
             /* System reset */
             vdev->device_endian = virtio_default_endian();
         }

     Now, we can see how existing virtio_access_is_big_endian is equivalent
     to virtio_vdev_is_big_endian. Let's break the existing function in 
its 3
     variants, and compare that to virtio_vdev_is_big_endian.

     static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
     - #if defined(LEGACY_VIRTIO_IS_BIENDIAN)
         return virtio_vdev_is_big_endian(vdev);
       This is the exact replacement we did, so equivalent.
     - #elif TARGET_BIG_ENDIAN
         if (virtio_vdev_is_modern(vdev)) {
             return false;
         }
         return true;

       we know target_is_big_endian(), so
       vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG.
       if (virtio_vdev_is_legacy(vdev)) {
           return VIRTIO_DEVICE_ENDIAN_BIG == VIRTIO_DEVICE_ENDIAN_BIG;
       }
       return false;
       It's written in opposite style compared to existing code (if 
legacy vs
       if modern), but it's strictly equivalent.
     - #else
         return false;
       we know !target_is_big_endian(), so
       vdev->device_endian == VIRTIO_DEVICE_ENDIAN_LITTLE.
       if virtio_vdev_is_legacy(vdev) {
         return VIRTIO_DEVICE_ENDIAN_LITTLE == VIRTIO_DEVICE_ENDIAN_BIG;
       }
       return false;
       So it always return false, as expected.

     Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
     Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
     Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

diff --git a/include/hw/virtio/virtio-access.h 
b/include/hw/virtio/virtio-access.h
index b58fb6ed7ea..e3148c23881 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -23,19 +23,5 @@

-#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_vdev_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
  }

commit 8f907ce751e9922ede323af8d63a21c42392a7d1
Author: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Date:   Thu Feb 12 15:46:02 2026 -0800

     hw/virtio: Inline virtio_access_is_big_endian()

     Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
     Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
     Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

diff --git a/include/hw/virtio/virtio-access.h 
b/include/hw/virtio/virtio-access.h
index e3148c23881..506be642c9f 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -23,10 +23,5 @@

-static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
-{
-    return virtio_vdev_is_big_endian(vdev);
-}
-
  static inline void virtio_stw_p(VirtIODevice *vdev, void *ptr, uint16_t v)
  {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (virtio_vdev_is_big_endian(vdev)) {
          stw_be_p(ptr, v);
@@ -39,3 +34,3 @@ static inline void virtio_stl_p(VirtIODevice *vdev, 
void *ptr, uint32_t v)
  {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (virtio_vdev_is_big_endian(vdev)) {
          stl_be_p(ptr, v);
@@ -48,3 +43,3 @@ static inline void virtio_stq_p(VirtIODevice *vdev, 
void *ptr, uint64_t v)
  {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (virtio_vdev_is_big_endian(vdev)) {
          stq_be_p(ptr, v);
@@ -57,3 +52,3 @@ static inline int virtio_lduw_p(VirtIODevice *vdev, 
const void *ptr)
  {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (virtio_vdev_is_big_endian(vdev)) {
          return lduw_be_p(ptr);
@@ -66,3 +61,3 @@ static inline int virtio_ldl_p(VirtIODevice *vdev, 
const void *ptr)
  {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (virtio_vdev_is_big_endian(vdev)) {
          return ldl_be_p(ptr);
@@ -75,3 +70,3 @@ static inline uint64_t virtio_ldq_p(VirtIODevice 
*vdev, const void *ptr)
  {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (virtio_vdev_is_big_endian(vdev)) {
          return ldq_be_p(ptr);
@@ -85,5 +80,5 @@ 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 virtio_vdev_is_big_endian(vdev) ? s : bswap16(s);
  #else
-    return virtio_access_is_big_endian(vdev) ? bswap16(s) : s;
+    return virtio_vdev_is_big_endian(vdev) ? bswap16(s) : s;
  #endif
@@ -99,5 +94,5 @@ 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 virtio_vdev_is_big_endian(vdev) ? s : bswap32(s);
  #else
-    return virtio_access_is_big_endian(vdev) ? bswap32(s) : s;
+    return virtio_vdev_is_big_endian(vdev) ? bswap32(s) : s;
  #endif
@@ -113,5 +108,5 @@ 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 virtio_vdev_is_big_endian(vdev) ? s : bswap64(s);
  #else
-    return virtio_access_is_big_endian(vdev) ? bswap64(s) : s;
+    return virtio_vdev_is_big_endian(vdev) ? bswap64(s) : s;
  #endif
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index c0c4599b586..22d798e6cdd 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -224,3 +224,3 @@ static inline uint16_t 
virtio_lduw_phys_cached(VirtIODevice *vdev,
  {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (virtio_vdev_is_big_endian(vdev)) {
          return lduw_be_phys_cached(cache, pa);
@@ -234,3 +234,3 @@ static inline void 
virtio_stw_phys_cached(VirtIODevice *vdev,
  {
-    if (virtio_access_is_big_endian(vdev)) {
+    if (virtio_vdev_is_big_endian(vdev)) {
          stw_be_phys_cached(cache, pa, value);
---

Do you want me to post as v2 for easier review?

Regards,

Phil.

Re: [PATCH 0/3] hw/virtio/virtio-access.h: remove target specific code

[Michael S. Tsirkin]

On Tue, Feb 24, 2026 at 07:33:14PM +0100, Philippe Mathieu-Daudé wrote:
> Hi Pierrick,
> 
> On 13/2/26 00:45, Pierrick Bouvier wrote:
> > This series eliminates some target specifics in hw/virtio and replace them with
> > runtime functions where needed, to be able to link virtio code in single-binary.
> > After a first try on a series [0] doing this change and making all virtio files
> > common, Richard asked to refactor this part, thus this independent series.
> 
> > Pierrick Bouvier (3):
> >    hw/virtio: add virtio_vdev_is_{modern, legacy}
> >    hw/virtio: rename virtio_is_big_endian to virtio_vdev_is_big_endian
> >    hw/virtio: remove virtio_access_is_big_endian
> 
> Patch #2 has been merged as commit 6325407f67d.
> 
> Since we don't have feedback from the maintainers Cc'ed, I took
> the liberty to rebase your series, trying to address Zoltan's
> concerns on patch #1. 


So as I said, main use-case where we would worry about perf is
virtio storage. So it's mostly for storage guys to ack.
Did not get Stefan's feedback yet on whether he is happy.

I don't understand if we always want to build a single binary though.
If not we could address all perf worries by having both  generic
and target specific implementations by using linker tricks (weak symbols or
whatnot).


-- 
MST

Re: [PATCH 0/3] hw/virtio/virtio-access.h: remove target specific code

[Pierrick Bouvier]

On 2/24/26 11:07 AM, Michael S. Tsirkin wrote:
> On Tue, Feb 24, 2026 at 07:33:14PM +0100, Philippe Mathieu-Daudé wrote:
>> Hi Pierrick,
>>
>> On 13/2/26 00:45, Pierrick Bouvier wrote:
>>> This series eliminates some target specifics in hw/virtio and replace them with
>>> runtime functions where needed, to be able to link virtio code in single-binary.
>>> After a first try on a series [0] doing this change and making all virtio files
>>> common, Richard asked to refactor this part, thus this independent series.
>>
>>> Pierrick Bouvier (3):
>>>     hw/virtio: add virtio_vdev_is_{modern, legacy}
>>>     hw/virtio: rename virtio_is_big_endian to virtio_vdev_is_big_endian
>>>     hw/virtio: remove virtio_access_is_big_endian
>>
>> Patch #2 has been merged as commit 6325407f67d.
>>
>> Since we don't have feedback from the maintainers Cc'ed, I took
>> the liberty to rebase your series, trying to address Zoltan's
>> concerns on patch #1.
> 
> 
> So as I said, main use-case where we would worry about perf is
> virtio storage. So it's mostly for storage guys to ack.
> Did not get Stefan's feedback yet on whether he is happy.
> 
> I don't understand if we always want to build a single binary though.
> If not we could address all perf worries by having both  generic
> and target specific implementations by using linker tricks (weak symbols or
> whatnot).
>

This is not possible unfortunately, since concerned function is declared 
static inline. If we want to follow the linker approach, we would still 
have to pay for an indirect function call. That's why we insist so much 
to change the current definition, as it's the only way to move forward.

The other solution is to exclude virtio completely from single-binary, 
but we don't want to have specific config for this, and we worked hard 
to keep our promise over all other subsystems we touched with satisfying 
compromises for concerned maintainers.
Virtio and vfio are the last two left. After that, there is nothing 
really blocking the path towards the goal.

I invite you to watch the presentation we did with Philippe in latest 
KVM Forum, it will probably answers on some of your questions.

Especially, you'll see we insist on single binary being a recollection 
of existing compilation units, and not yet another configuration to 
build (and potentially fail).

Regards,
Pierrick

Re: [PATCH 0/3] hw/virtio/virtio-access.h: remove target specific code

[Philippe Mathieu-Daudé]

Hi Stefan,

On 24/2/26 20:21, Pierrick Bouvier wrote:
> On 2/24/26 11:07 AM, Michael S. Tsirkin wrote:
>> On Tue, Feb 24, 2026 at 07:33:14PM +0100, Philippe Mathieu-Daudé wrote:
>>> Hi Pierrick,
>>>
>>> On 13/2/26 00:45, Pierrick Bouvier wrote:
>>>> This series eliminates some target specifics in hw/virtio and 
>>>> replace them with
>>>> runtime functions where needed, to be able to link virtio code in 
>>>> single-binary.
>>>> After a first try on a series [0] doing this change and making all 
>>>> virtio files
>>>> common, Richard asked to refactor this part, thus this independent 
>>>> series.
>>>
>>>> Pierrick Bouvier (3):
>>>>     hw/virtio: add virtio_vdev_is_{modern, legacy}
>>>>     hw/virtio: rename virtio_is_big_endian to virtio_vdev_is_big_endian
>>>>     hw/virtio: remove virtio_access_is_big_endian
>>>
>>> Patch #2 has been merged as commit 6325407f67d.
>>>
>>> Since we don't have feedback from the maintainers Cc'ed, I took
>>> the liberty to rebase your series, trying to address Zoltan's
>>> concerns on patch #1.
>>
>>
>> So as I said, main use-case where we would worry about perf is
>> virtio storage. So it's mostly for storage guys to ack.
>> Did not get Stefan's feedback yet on whether he is happy.

Are you available to discuss this topic during our next community
call (Tue March 3, 2pm UTC)?

>> I don't understand if we always want to build a single binary though.
>> If not we could address all perf worries by having both  generic
>> and target specific implementations by using linker tricks (weak 
>> symbols or
>> whatnot).
>>
> 
> This is not possible unfortunately, since concerned function is declared 
> static inline. If we want to follow the linker approach, we would still 
> have to pay for an indirect function call. That's why we insist so much 
> to change the current definition, as it's the only way to move forward.
> 
> The other solution is to exclude virtio completely from single-binary, 
> but we don't want to have specific config for this, and we worked hard 
> to keep our promise over all other subsystems we touched with satisfying 
> compromises for concerned maintainers.
> Virtio and vfio are the last two left. After that, there is nothing 
> really blocking the path towards the goal.
> 
> I invite you to watch the presentation we did with Philippe in latest 
> KVM Forum, it will probably answers on some of your questions.
> 
> Especially, you'll see we insist on single binary being a recollection 
> of existing compilation units, and not yet another configuration to 
> build (and potentially fail).
> 
> Regards,
> Pierrick

Re: [PATCH 0/3] hw/virtio/virtio-access.h: remove target specific code

[Stefan Hajnoczi]

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

On Tue, Feb 24, 2026 at 08:36:36PM +0100, Philippe Mathieu-Daudé wrote:
> Hi Stefan,
> 
> On 24/2/26 20:21, Pierrick Bouvier wrote:
> > On 2/24/26 11:07 AM, Michael S. Tsirkin wrote:
> > > On Tue, Feb 24, 2026 at 07:33:14PM +0100, Philippe Mathieu-Daudé wrote:
> > > > Hi Pierrick,
> > > > 
> > > > On 13/2/26 00:45, Pierrick Bouvier wrote:
> > > > > This series eliminates some target specifics in hw/virtio
> > > > > and replace them with
> > > > > runtime functions where needed, to be able to link virtio
> > > > > code in single-binary.
> > > > > After a first try on a series [0] doing this change and
> > > > > making all virtio files
> > > > > common, Richard asked to refactor this part, thus this
> > > > > independent series.
> > > > 
> > > > > Pierrick Bouvier (3):
> > > > >     hw/virtio: add virtio_vdev_is_{modern, legacy}
> > > > >     hw/virtio: rename virtio_is_big_endian to virtio_vdev_is_big_endian
> > > > >     hw/virtio: remove virtio_access_is_big_endian
> > > > 
> > > > Patch #2 has been merged as commit 6325407f67d.
> > > > 
> > > > Since we don't have feedback from the maintainers Cc'ed, I took
> > > > the liberty to rebase your series, trying to address Zoltan's
> > > > concerns on patch #1.
> > > 
> > > 
> > > So as I said, main use-case where we would worry about perf is
> > > virtio storage. So it's mostly for storage guys to ack.
> > > Did not get Stefan's feedback yet on whether he is happy.
> 
> Are you available to discuss this topic during our next community
> call (Tue March 3, 2pm UTC)?

If this revision of the patch series still has the same performance in
the cover letter, then I'm happy. I'll be on vacation next week and
can't make the community call.

Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

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

Re: [PATCH 0/3] hw/virtio/virtio-access.h: remove target specific code

[Michael S. Tsirkin]

On Tue, Feb 24, 2026 at 11:21:09AM -0800, Pierrick Bouvier wrote:
> On 2/24/26 11:07 AM, Michael S. Tsirkin wrote:
> > On Tue, Feb 24, 2026 at 07:33:14PM +0100, Philippe Mathieu-Daudé wrote:
> > > Hi Pierrick,
> > > 
> > > On 13/2/26 00:45, Pierrick Bouvier wrote:
> > > > This series eliminates some target specifics in hw/virtio and replace them with
> > > > runtime functions where needed, to be able to link virtio code in single-binary.
> > > > After a first try on a series [0] doing this change and making all virtio files
> > > > common, Richard asked to refactor this part, thus this independent series.
> > > 
> > > > Pierrick Bouvier (3):
> > > >     hw/virtio: add virtio_vdev_is_{modern, legacy}
> > > >     hw/virtio: rename virtio_is_big_endian to virtio_vdev_is_big_endian
> > > >     hw/virtio: remove virtio_access_is_big_endian
> > > 
> > > Patch #2 has been merged as commit 6325407f67d.
> > > 
> > > Since we don't have feedback from the maintainers Cc'ed, I took
> > > the liberty to rebase your series, trying to address Zoltan's
> > > concerns on patch #1.
> > 
> > 
> > So as I said, main use-case where we would worry about perf is
> > virtio storage. So it's mostly for storage guys to ack.
> > Did not get Stefan's feedback yet on whether he is happy.
> > 
> > I don't understand if we always want to build a single binary though.
> > If not we could address all perf worries by having both  generic
> > and target specific implementations by using linker tricks (weak symbols or
> > whatnot).
> > 
> 
> This is not possible unfortunately, since concerned function is declared
> static inline.

I refer to all of affected device here. Not just the single function.

> If we want to follow the linker approach, we would still have
> to pay for an indirect function call. That's why we insist so much to change
> the current definition, as it's the only way to move forward.
> 
> The other solution is to exclude virtio completely from single-binary,

Not exclude - have a separate slower version of virtio for that.

> but
> we don't want to have specific config for this, and we worked hard to keep
> our promise over all other subsystems we touched with satisfying compromises
> for concerned maintainers.

ok

> Virtio and vfio are the last two left. After that, there is nothing really
> blocking the path towards the goal.
> 
> I invite you to watch the presentation we did with Philippe in latest KVM
> Forum, it will probably answers on some of your questions.
> 
> Especially, you'll see we insist on single binary being a recollection of
> existing compilation units, and not yet another configuration to build (and
> potentially fail).
> 
> Regards,
> Pierrick

Merely pointing out a fallback approach.
-- 
MST