[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 6/6] virtio: optimize virtio_access_is_big_endia
From: |
Greg Kurz |
Subject: |
Re: [Qemu-devel] [PATCH 6/6] virtio: optimize virtio_access_is_big_endian() for little-endian targets |
Date: |
Fri, 8 Jan 2016 10:27:20 +0100 |
On Thu, 7 Jan 2016 21:25:19 +0100
Laurent Vivier <address@hidden> wrote:
>
>
> On 07/01/2016 12:32, Greg Kurz wrote:
> > When adding cross-endian support, we introduced the TARGET_IS_BIENDIAN macro
> > and the virtio_access_is_big_endian() helper to have a branchless fast path
> > in the virtio memory accessors for targets that don't switch endian.
> >
> > This was considered as a strong requirement at the time.
> >
> > Now we have added a runtime check for virtio 1.0, which ruins the benefit
> > of the virtio_access_is_big_endian() helper for always little-endian
> > targets.
> >
> > With this patch, always little-endian targets stop checking for virtio 1.0,
> > since the result is little-endian in all cases.
> >
> > Signed-off-by: Greg Kurz <address@hidden>
> > ---
> > include/hw/virtio/virtio-access.h | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/include/hw/virtio/virtio-access.h
> > b/include/hw/virtio/virtio-access.h
> > index f1f12afe9089..fba4b4a4e1b2 100644
> > --- a/include/hw/virtio/virtio-access.h
> > +++ b/include/hw/virtio/virtio-access.h
> > @@ -19,10 +19,13 @@
> >
> > static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
> > {
> > +#if defined(TARGET_IS_BIENDIAN) || defined(TARGET_WORDS_BIGENDIAN)
> > if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> > /* Devices conforming to VIRTIO 1.0 or later are always LE. */
> > return false;
> > }
> > +#endif
>
>
> virtio_is_big_endian() checks VIRTIO_F_VERSION_1, so you don't need to
> check it here if TARGET_IS_BIENDIAN is defined.
>
> > +
> > #if defined(TARGET_IS_BIENDIAN)
> > return virtio_is_big_endian(vdev);
> > #elif defined(TARGET_WORDS_BIGENDIAN)
> >
> >
> So you can move virtio_vdev_has_feature() here.
>
> Laurent
>
>
Indeed this is one step further and results in:
static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
{
+#if defined(TARGET_IS_BIENDIAN)
+ return virtio_is_big_endian(vdev);
+#elif defined(TARGET_WORDS_BIGENDIAN)
if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
/* Devices conforming to VIRTIO 1.0 or later are always LE. */
return false;
}
-#if defined(TARGET_IS_BIENDIAN)
- return virtio_is_big_endian(vdev);
-#elif defined(TARGET_WORDS_BIGENDIAN)
return true;
#else
return false;
which looks nicer: slowest path (2 branches) for bi-endian targets,
slow path (1 branch) for big endian targets and fast path for little
endian targets.
Thanks for the suggestion !
--
Greg
- Re: [Qemu-devel] [PATCH 2/6] Revert "vhost-net: tell tap backend about the vnet endianness", (continued)
[Qemu-devel] [PATCH 3/6] virtio: drop the virtio_needs_swap() helper, Greg Kurz, 2016/01/07
[Qemu-devel] [PATCH 4/6] virtio: move cross-endian helper to vhost, Greg Kurz, 2016/01/07
[Qemu-devel] [PATCH 6/6] virtio: optimize virtio_access_is_big_endian() for little-endian targets, Greg Kurz, 2016/01/07
[Qemu-devel] [PATCH 5/6] vhost: move virtio 1.0 check to cross-endian helper, Greg Kurz, 2016/01/07