On 2/3/26 6:10 PM, Richard Henderson wrote:
On 2/4/26 06:56, Pierrick Bouvier wrote:
Signed-off-by: Pierrick Bouvier <[email protected]>
---
include/hw/virtio/virtio-access.h | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/include/hw/virtio/virtio-access.h
b/include/hw/virtio/virtio-access.h
index f3b4d0075b5..1bea3445979 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -17,27 +17,27 @@
#define QEMU_VIRTIO_ACCESS_H
#include "exec/hwaddr.h"
+#include "qemu/target-info.h"
#include "system/memory_cached.h"
#include "hw/virtio/virtio.h"
#include "hw/virtio/virtio-bus.h"
-#if defined(TARGET_PPC64) || defined(TARGET_ARM)
-#define LEGACY_VIRTIO_IS_BIENDIAN 1
-#endif
-
static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
{
-#if defined(LEGACY_VIRTIO_IS_BIENDIAN)
- return virtio_is_big_endian(vdev);
-#elif TARGET_BIG_ENDIAN
- if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
- /* Devices conforming to VIRTIO 1.0 or later are always LE. */
- return false;
+ if (target_ppc64() || target_base_arm()) {
+ return virtio_is_big_endian(vdev);
Why use two predicates like this, effectively calling target_arch() twice?
Surely it's better to do
Yes it can, I simply wrote the simplest change possible. Since this
function is called only once now at reset time, it definitely looks like
premature optimization IMHO.
switch (target_arch()) {
case SYS_EMU_TARGET_ARM:
case SYS_EMU_TARGET_AARCH64:
case SYS_EMU_TARGET_PPC64:
return virtio_is_big_endian(vdev);
default:
break;
}
Definitely with a comment saying this is about legacy behaviour.
And does VIRTIO_F_VERSION_1 really not take priority?
Not sure about this one, but current implementation has same behaviour
than previous ifdef based one.
In case that's wrong, I would be happy to change it.
r~
Regards,
Pierrick
