On 2/9/26 18:04, Pierrick Bouvier wrote:
On 2/8/26 9:19 PM, Richard Henderson wrote:
On 2/7/26 08:19, Pierrick Bouvier wrote:
virtio_is_big_endian already checks for VIRTIO_F_VERSION_1 feature, so
this is redundant with big_endian_code.
Also, clarify why we explicitly check for ppc64 and arm arch.
Signed-off-by: Pierrick Bouvier <[email protected]>
---
include/hw/virtio/virtio-access.h | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/include/hw/virtio/virtio-access.h
b/include/hw/virtio/virtio-access.h
index 1bea3445979..9aed3d1da4f 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -22,21 +22,16 @@
#include "hw/virtio/virtio.h"
#include "hw/virtio/virtio-bus.h"
+static inline bool legacy_virtio_is_biendian(void)
+{
+ return target_ppc64() || target_base_arm();
+}
+
static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
{
- if (target_ppc64() || target_base_arm()) {
+ if (target_big_endian() || legacy_virtio_is_biendian()) {
return virtio_is_big_endian(vdev);
}
This now calls the hook for all big-endian, not just bi-endian.
I don't think you need to change anything here...
If I'm correct, that's what original code was doing:
#if defined(LEGACY_VIRTIO_IS_BIENDIAN)
return virtio_is_big_endian(vdev);
#elif TARGET_BIG_ENDIAN
/* duplicated code for virtio_is_big_endian(vdev) */
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;
From what I understand, legacy virtio devices follow host endianness (thus considering
all big-endian arch), while virtio >= 1.0 devices are always little endian.
What did I miss to consider it should only be called for bi-endian arch?
if (target_big_endian() || ...) {
return virtio_is_big_endian(vdev);
}
invokes virtio_is_big_endian for e.g. sparc, which it never did before.
The first #if defined(LEGACY_VIRTIO_IS_BIENDIAN) is exactly your
legacy_virtio_is_biendian(), and is the only time that should ever be invoked.
Separately, you noticed that virtio_device_endian_needed already checks
VIRTIO_F_VERSION_1, so checking it a second time here (possibly with the rest of the
changes in this patch set), is unnecessary. Which allows you do remove the big-endian
check entirely.
I do think that (again, possibly with the rest of the changes in the patch set) the
function virtio_access_is_big_endian should be merged into its single caller, so that (1)
it's obvious no further check vs VERSION is required and (2) no further calls of
virtio_access_is_big_endian can be introduced.
r~
