On 2/4/26 8:37 AM, Pierrick Bouvier wrote:
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
For what it's worth, that's the patch I wrote when I was optimizing this
function and found -1.7% for performance:
commit d497f71b5b1bcc46de97436e1ce31641ff792eb9
Author: Pierrick Bouvier <[email protected]>
Date: Mon Feb 2 13:53:06 2026 -0800
include/hw/virtio/virtio-access.h: optimize virtio_access_is_big_endian
By following benchmark recommended by Stefan [1], it was found that
previous commit introduced an overhead of 3% for this scenario.
By reading TargetInfo only once and using a jump table, we get to 1.7%.
[1] https://lore.kernel.org/qemu-devel/20260202185233.GC405548@fedora/
Signed-off-by: Pierrick Bouvier <[email protected]>
diff --git a/include/hw/virtio/virtio-access.h
b/include/hw/virtio/virtio-access.h
index 1bea3445979..b7695f56fed 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -24,11 +24,17 @@
static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
{
- if (target_ppc64() || target_base_arm()) {
+ const TargetInfo *info = target_info();
+ static const int may_be_big_endian[SYS_EMU_TARGET__MAX] = {
+ [SYS_EMU_TARGET_ARM] = 1,
+ [SYS_EMU_TARGET_AARCH64] = 1,
+ [SYS_EMU_TARGET_PPC64] = 1,
+ };
+ if (may_be_big_endian[info->target_arch]) {
return virtio_is_big_endian(vdev);
}
- if (target_big_endian()) {
+ if (info->endianness == ENDIAN_MODE_BIG) {
if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
/* Devices conforming to VIRTIO 1.0 or later are always LE. */
return false;
