"Michael S. Tsirkin" <[email protected]> writes:
> On Mon, Feb 02, 2026 at 12:29:16AM +0100, Philippe Mathieu-Daudé wrote:
>> Endianness access is constant between device resets.
>> Use the field instead of calling the same function.
>
> yes but the function was inlined and often a NOP. worth mentioning.
>
>>
>> Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
>> ---
>> include/hw/virtio/virtio-access.h | 24 ++++++++++++------------
>> hw/virtio/virtio.c | 4 ++--
>> 2 files changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/include/hw/virtio/virtio-access.h
>> b/include/hw/virtio/virtio-access.h
>> index cd17d0c87eb..f3b4d0075b5 100644
>> --- a/include/hw/virtio/virtio-access.h
>> +++ b/include/hw/virtio/virtio-access.h
>> @@ -42,7 +42,7 @@ static inline bool
>> virtio_access_is_big_endian(VirtIODevice *vdev)
>>
>> static inline void virtio_stw_p(VirtIODevice *vdev, void *ptr, uint16_t v)
>> {
>> - if (virtio_access_is_big_endian(vdev)) {
>> + if (vdev->access_is_big_endian) {
>> stw_be_p(ptr, v);
>> } else {
>> stw_le_p(ptr, v);
>
>
> So this is the main extra cost: on an LE host, we now have this branch on
> every
> access where previously it was all optimized out.
>
> Is it measureable? It is worth testing.
I suspect not given this is a well predicted branch on the control
plane. But we can certainly try an measure it. Do we have any virtio
performance benchmarks?
We only have a few in meson:
./pyvenv/bin/meson test --benchmark --list
bufferiszero-bench
benchmark-crypto-hash
benchmark-crypto-hmac
benchmark-crypto-cipher
benchmark-crypto-akcipher
>
>
>
>> @@ -51,7 +51,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 (vdev->access_is_big_endian) {
>> stl_be_p(ptr, v);
>> } else {
>> stl_le_p(ptr, v);
>> @@ -60,7 +60,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 (vdev->access_is_big_endian) {
>> stq_be_p(ptr, v);
>> } else {
>> stq_le_p(ptr, v);
>> @@ -69,7 +69,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 (vdev->access_is_big_endian) {
>> return lduw_be_p(ptr);
>> } else {
>> return lduw_le_p(ptr);
>
> and this one.
>
>
>> @@ -78,7 +78,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 (vdev->access_is_big_endian) {
>> return ldl_be_p(ptr);
>> } else {
>> return ldl_le_p(ptr);
>> @@ -87,7 +87,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 (vdev->access_is_big_endian) {
>> return ldq_be_p(ptr);
>> } else {
>> return ldq_le_p(ptr);
>> @@ -97,9 +97,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 vdev->access_is_big_endian ? s : bswap16(s);
>> #else
>> - return virtio_access_is_big_endian(vdev) ? bswap16(s) : s;
>> + return vdev->access_is_big_endian ? bswap16(s) : s;
>> #endif
>> }
>>
>> @@ -111,9 +111,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 vdev->access_is_big_endian ? s : bswap32(s);
>> #else
>> - return virtio_access_is_big_endian(vdev) ? bswap32(s) : s;
>> + return vdev->access_is_big_endian ? bswap32(s) : s;
>> #endif
>> }
>>
>> @@ -125,9 +125,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 vdev->access_is_big_endian ? s : bswap64(s);
>> #else
>> - return virtio_access_is_big_endian(vdev) ? bswap64(s) : s;
>> + return vdev->access_is_big_endian ? bswap64(s) : s;
>> #endif
>> }
>>
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index 242c207a591..1dc60d37cb4 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -220,7 +220,7 @@ static inline uint16_t
>> virtio_lduw_phys_cached(VirtIODevice *vdev,
>> MemoryRegionCache *cache,
>> hwaddr pa)
>> {
>> - if (virtio_access_is_big_endian(vdev)) {
>> + if (vdev->access_is_big_endian) {
>> return lduw_be_phys_cached(cache, pa);
>> }
>> return lduw_le_phys_cached(cache, pa);
>> @@ -230,7 +230,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 (vdev->access_is_big_endian) {
>> stw_be_phys_cached(cache, pa, value);
>> } else {
>> stw_le_phys_cached(cache, pa, value);
>> --
>> 2.52.0
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
