[PATCH v2 0/6] hw/virtio/virtio-access.h: remove target specific code

Philippe Mathieu-Daudé posted 6 patches 1 month, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260225041948.52929-1-philmd@linaro.org
Maintainers: Christian Schoenebeck <qemu_oss@crudebyte.com>, Greg Kurz <groug@kaod.org>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Jason Wang <jasowang@redhat.com>, Fam Zheng <fam@euphon.net>, "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>
include/hw/virtio/virtio-access.h | 43 ++++++-------------
include/hw/virtio/virtio.h        |  7 +++-
hw/virtio/vhost-user.c            | 11 ++---
hw/virtio/vhost.c                 | 12 +++---
hw/virtio/virtio-pci.c            |  6 +--
hw/virtio/virtio-qmp.c            | 70 -------------------------------
hw/virtio/virtio.c                | 12 +++---
hw/9pfs/meson.build               |  2 +-
hw/block/meson.build              |  4 +-
hw/char/meson.build               |  2 +-
hw/net/meson.build                |  2 +-
hw/scsi/meson.build               |  6 +--
hw/virtio/meson.build             | 17 ++++----
13 files changed, 50 insertions(+), 144 deletions(-)
[PATCH v2 0/6] hw/virtio/virtio-access.h: remove target specific code
Posted by Philippe Mathieu-Daudé 1 month, 2 weeks ago
v2:
- Rebased
- Removed virtio_vdev_is_modern (Zoltan, myself)
- Include Stefan's Acked-by tag
- Include "make compilation unit common" patches from [3]

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.

By diving into virtio initialization, I noticed that device_endian is always
storing endianness of cpu associated on device reset. The root issue, is when
dealing with target who dynamically change their endianness during execution.
ppc64 is the main use case, as cpu always boot in BE and most of OS use LE. arm
use case is more limited, as big endian systems are mostly dead nowadays.

Because of this, initialization is tricky, and goes through different hoops to
have the expected value. My first approach has been to try to change this, by
simply setting endianness on the first access. However, it fell flat, because
current_cpu is not always available at this time.
A second approach was to set endianness to LE by default, and change it only
when setting device features, and potentially discovering it's a legacy virtio
device. It brought other issues, that showed that current initialization code,
even though it's complex, does the right thing.
Thus, I focused on refactoring virtio_access_is_big_endian, and noticed that it
was not even needed at this point.

Patches 1 and 2 are refactoring names to clear some confusion.
Patch 3 eliminates virtio_access_is_big_endian, with a lenghty commit message
explaining the change. By doing this, we remove target specifics from
hw/virtio/virtio-access.h.

Of course, performance has been tested, and it is on par with upstream/master.

Results are on 20 runs and expressed in kIOPS:
reference: mean=239.2 std_dev=3.48
with_series: mean=238.1 std_dev=3.56

---

Performance has been measured with this automated fio benchmark [1], with
original instructions from Stefan [2].

Download artifacts and run benchmark with:
$ wget https://github.com/pbo-linaro/qemu-linux-stack/releases/download/build/x86_64_io_benchmark-a55f2d6.tar.xz
$ tar xvf x86_64_io_benchmark-a55f2d6.tar.xz
$ ./run.sh /path/to/qemu-system-x86_64

[0] https://lore.kernel.org/qemu-devel/20260206221908.1451528-1-pierrick.bouvier@linaro.org/
[1] https://github.com/pbo-linaro/qemu-linux-stack/tree/x86_64_io_benchmark
[2] https://lore.kernel.org/qemu-devel/20260202185233.GC405548@fedora/
[3] https://lore.kernel.org/qemu-devel/20260206221908.1451528-1-pierrick.bouvier@linaro.org/

Pierrick Bouvier (6):
  hw/virtio: Add virtio_vdev_is_legacy()
  hw/virtio: Simplify virtio_access_is_big_endian()
  hw/virtio: Inline virtio_access_is_big_endian()
  hw/virtio/vhost-user: make compilation unit common
  hw/virtio/virtio-qmp: make compilation unit common
  hw/virtio/: make all compilation units common

 include/hw/virtio/virtio-access.h | 43 ++++++-------------
 include/hw/virtio/virtio.h        |  7 +++-
 hw/virtio/vhost-user.c            | 11 ++---
 hw/virtio/vhost.c                 | 12 +++---
 hw/virtio/virtio-pci.c            |  6 +--
 hw/virtio/virtio-qmp.c            | 70 -------------------------------
 hw/virtio/virtio.c                | 12 +++---
 hw/9pfs/meson.build               |  2 +-
 hw/block/meson.build              |  4 +-
 hw/char/meson.build               |  2 +-
 hw/net/meson.build                |  2 +-
 hw/scsi/meson.build               |  6 +--
 hw/virtio/meson.build             | 17 ++++----
 13 files changed, 50 insertions(+), 144 deletions(-)

-- 
2.52.0
Re: [PATCH v2 0/6] hw/virtio/virtio-access.h: remove target specific code
Posted by Pierrick Bouvier 1 month ago
On 2/24/26 8:19 PM, Philippe Mathieu-Daudé wrote:
> v2:
> - Rebased
> - Removed virtio_vdev_is_modern (Zoltan, myself)
> - Include Stefan's Acked-by tag
> - Include "make compilation unit common" patches from [3]
> 
> 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.
> 
> By diving into virtio initialization, I noticed that device_endian is always
> storing endianness of cpu associated on device reset. The root issue, is when
> dealing with target who dynamically change their endianness during execution.
> ppc64 is the main use case, as cpu always boot in BE and most of OS use LE. arm
> use case is more limited, as big endian systems are mostly dead nowadays.
> 
> Because of this, initialization is tricky, and goes through different hoops to
> have the expected value. My first approach has been to try to change this, by
> simply setting endianness on the first access. However, it fell flat, because
> current_cpu is not always available at this time.
> A second approach was to set endianness to LE by default, and change it only
> when setting device features, and potentially discovering it's a legacy virtio
> device. It brought other issues, that showed that current initialization code,
> even though it's complex, does the right thing.
> Thus, I focused on refactoring virtio_access_is_big_endian, and noticed that it
> was not even needed at this point.
> 
> Patches 1 and 2 are refactoring names to clear some confusion.
> Patch 3 eliminates virtio_access_is_big_endian, with a lenghty commit message
> explaining the change. By doing this, we remove target specifics from
> hw/virtio/virtio-access.h.
> 
> Of course, performance has been tested, and it is on par with upstream/master.
> 
> Results are on 20 runs and expressed in kIOPS:
> reference: mean=239.2 std_dev=3.48
> with_series: mean=238.1 std_dev=3.56
> 
> ---
> 
> Performance has been measured with this automated fio benchmark [1], with
> original instructions from Stefan [2].
> 
> Download artifacts and run benchmark with:
> $ wget https://github.com/pbo-linaro/qemu-linux-stack/releases/download/build/x86_64_io_benchmark-a55f2d6.tar.xz
> $ tar xvf x86_64_io_benchmark-a55f2d6.tar.xz
> $ ./run.sh /path/to/qemu-system-x86_64
> 
> [0] https://lore.kernel.org/qemu-devel/20260206221908.1451528-1-pierrick.bouvier@linaro.org/
> [1] https://github.com/pbo-linaro/qemu-linux-stack/tree/x86_64_io_benchmark
> [2] https://lore.kernel.org/qemu-devel/20260202185233.GC405548@fedora/
> [3] https://lore.kernel.org/qemu-devel/20260206221908.1451528-1-pierrick.bouvier@linaro.org/
> 
> Pierrick Bouvier (6):
>    hw/virtio: Add virtio_vdev_is_legacy()
>    hw/virtio: Simplify virtio_access_is_big_endian()
>    hw/virtio: Inline virtio_access_is_big_endian()
>    hw/virtio/vhost-user: make compilation unit common
>    hw/virtio/virtio-qmp: make compilation unit common
>    hw/virtio/: make all compilation units common
> 
>   include/hw/virtio/virtio-access.h | 43 ++++++-------------
>   include/hw/virtio/virtio.h        |  7 +++-
>   hw/virtio/vhost-user.c            | 11 ++---
>   hw/virtio/vhost.c                 | 12 +++---
>   hw/virtio/virtio-pci.c            |  6 +--
>   hw/virtio/virtio-qmp.c            | 70 -------------------------------
>   hw/virtio/virtio.c                | 12 +++---
>   hw/9pfs/meson.build               |  2 +-
>   hw/block/meson.build              |  4 +-
>   hw/char/meson.build               |  2 +-
>   hw/net/meson.build                |  2 +-
>   hw/scsi/meson.build               |  6 +--
>   hw/virtio/meson.build             | 17 ++++----
>   13 files changed, 50 insertions(+), 144 deletions(-)
> 


This was merged into master (2940018747e31842cf34334394c7d32517e73fbd).

Regards,
Pierrick