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 <[email protected]>
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 <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
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 <[email protected]>
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 <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
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 <[email protected]>
Date: Thu Feb 12 15:46:02 2026 -0800
hw/virtio: Inline virtio_access_is_big_endian()
Signed-off-by: Pierrick Bouvier <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
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.
