* [Qemu-devel] [PATCH v2 0/3] virtio: cross-endian helpers fixes
@ 2015-12-17 8:52 Greg Kurz
2015-12-17 8:52 ` [Qemu-devel] [PATCH v2 1/3] virtio: move cross-endian helper to vhost Greg Kurz
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Greg Kurz @ 2015-12-17 8:52 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Cornelia Huck, qemu-devel
This series tries to rework cross-endian helpers for better clarity.
It does not change behaviour, except perhaps patch 3/3 even if I could not
measure any performance gain.
---
Greg Kurz (3):
virtio: move cross-endian helper to vhost
vhost: move virtio 1.0 check to cross-endian helper
virtio: optimize virtio_access_is_big_endian() for little-endian targets
hw/virtio/vhost.c | 22 ++++++++++++++++++----
include/hw/virtio/virtio-access.h | 16 +++-------------
2 files changed, 21 insertions(+), 17 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 1/3] virtio: move cross-endian helper to vhost
2015-12-17 8:52 [Qemu-devel] [PATCH v2 0/3] virtio: cross-endian helpers fixes Greg Kurz
@ 2015-12-17 8:52 ` Greg Kurz
2015-12-17 8:53 ` [Qemu-devel] [PATCH v2 2/3] vhost: move virtio 1.0 check to cross-endian helper Greg Kurz
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Greg Kurz @ 2015-12-17 8:52 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Cornelia Huck, qemu-devel
If target is bi-endian (ppc64, arm), the virtio_legacy_is_cross_endian()
indeed returns the runtime state of the virtio device. However, it returns
false unconditionally in the general case. This sounds a bit strange
given the name of the function.
This helper is only useful for vhost actually, where indeed non bi-endian
targets don't have to deal with cross-endian issues.
This patch moves the helper to vhost.c and gives it a more appropriate name.
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
hw/virtio/vhost.c | 17 +++++++++++++++--
include/hw/virtio/virtio-access.h | 13 -------------
2 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index de29968a7945..2e1e792d599e 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -748,6 +748,19 @@ static void vhost_log_stop(MemoryListener *listener,
/* FIXME: implement */
}
+static inline bool vhost_needs_vring_endian(VirtIODevice *vdev)
+{
+#ifdef TARGET_IS_BIENDIAN
+#ifdef HOST_WORDS_BIGENDIAN
+ return !virtio_is_big_endian(vdev);
+#else
+ return virtio_is_big_endian(vdev);
+#endif
+#else
+ return false;
+#endif
+}
+
static int vhost_virtqueue_set_vring_endian_legacy(struct vhost_dev *dev,
bool is_big_endian,
int vhost_vq_index)
@@ -799,7 +812,7 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
}
if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) &&
- virtio_legacy_is_cross_endian(vdev)) {
+ vhost_needs_vring_endian(vdev)) {
r = vhost_virtqueue_set_vring_endian_legacy(dev,
virtio_is_big_endian(vdev),
vhost_vq_index);
@@ -896,7 +909,7 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
* native as legacy devices expect so by default.
*/
if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) &&
- virtio_legacy_is_cross_endian(vdev)) {
+ vhost_needs_vring_endian(vdev)) {
r = vhost_virtqueue_set_vring_endian_legacy(dev,
!virtio_is_big_endian(vdev),
vhost_vq_index);
diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
index a01fff2e51d7..f1f12afe9089 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -32,19 +32,6 @@ static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
#endif
}
-static inline bool virtio_legacy_is_cross_endian(VirtIODevice *vdev)
-{
-#ifdef TARGET_IS_BIENDIAN
-#ifdef HOST_WORDS_BIGENDIAN
- return !virtio_is_big_endian(vdev);
-#else
- return virtio_is_big_endian(vdev);
-#endif
-#else
- return false;
-#endif
-}
-
static inline uint16_t virtio_lduw_phys(VirtIODevice *vdev, hwaddr pa)
{
if (virtio_access_is_big_endian(vdev)) {
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 2/3] vhost: move virtio 1.0 check to cross-endian helper
2015-12-17 8:52 [Qemu-devel] [PATCH v2 0/3] virtio: cross-endian helpers fixes Greg Kurz
2015-12-17 8:52 ` [Qemu-devel] [PATCH v2 1/3] virtio: move cross-endian helper to vhost Greg Kurz
@ 2015-12-17 8:53 ` Greg Kurz
2015-12-17 8:55 ` [Qemu-devel] [PATCH v2 3/3] virtio: optimize virtio_access_is_big_endian() for little-endian targets Greg Kurz
2015-12-23 13:47 ` [Qemu-devel] [PATCH v2 0/3] virtio: cross-endian helpers fixes Michael S. Tsirkin
3 siblings, 0 replies; 8+ messages in thread
From: Greg Kurz @ 2015-12-17 8:53 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Cornelia Huck, qemu-devel
Indeed vhost doesn't need to ask for vring endian fixing if the device is
virtio 1.0, since it is already handled by the in-kernel vhost driver. This
patch simply consolidates the logic into the existing helper.
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
hw/virtio/vhost.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 2e1e792d599e..aef750df22ad 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -750,6 +750,9 @@ static void vhost_log_stop(MemoryListener *listener,
static inline bool vhost_needs_vring_endian(VirtIODevice *vdev)
{
+ if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+ return false;
+ }
#ifdef TARGET_IS_BIENDIAN
#ifdef HOST_WORDS_BIGENDIAN
return !virtio_is_big_endian(vdev);
@@ -811,8 +814,7 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
return -errno;
}
- if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) &&
- vhost_needs_vring_endian(vdev)) {
+ if (vhost_needs_vring_endian(vdev)) {
r = vhost_virtqueue_set_vring_endian_legacy(dev,
virtio_is_big_endian(vdev),
vhost_vq_index);
@@ -908,8 +910,7 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
/* In the cross-endian case, we need to reset the vring endianness to
* native as legacy devices expect so by default.
*/
- if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1) &&
- vhost_needs_vring_endian(vdev)) {
+ if (vhost_needs_vring_endian(vdev)) {
r = vhost_virtqueue_set_vring_endian_legacy(dev,
!virtio_is_big_endian(vdev),
vhost_vq_index);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 3/3] virtio: optimize virtio_access_is_big_endian() for little-endian targets
2015-12-17 8:52 [Qemu-devel] [PATCH v2 0/3] virtio: cross-endian helpers fixes Greg Kurz
2015-12-17 8:52 ` [Qemu-devel] [PATCH v2 1/3] virtio: move cross-endian helper to vhost Greg Kurz
2015-12-17 8:53 ` [Qemu-devel] [PATCH v2 2/3] vhost: move virtio 1.0 check to cross-endian helper Greg Kurz
@ 2015-12-17 8:55 ` Greg Kurz
2015-12-23 13:47 ` [Qemu-devel] [PATCH v2 0/3] virtio: cross-endian helpers fixes Michael S. Tsirkin
3 siblings, 0 replies; 8+ messages in thread
From: Greg Kurz @ 2015-12-17 8:55 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Cornelia Huck, qemu-devel
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 <gkurz@linux.vnet.ibm.com>
---
v2: - don't rename helper to virtio_is_big_endian_fast()
---
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
+
#if defined(TARGET_IS_BIENDIAN)
return virtio_is_big_endian(vdev);
#elif defined(TARGET_WORDS_BIGENDIAN)
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] virtio: cross-endian helpers fixes
2015-12-17 8:52 [Qemu-devel] [PATCH v2 0/3] virtio: cross-endian helpers fixes Greg Kurz
` (2 preceding siblings ...)
2015-12-17 8:55 ` [Qemu-devel] [PATCH v2 3/3] virtio: optimize virtio_access_is_big_endian() for little-endian targets Greg Kurz
@ 2015-12-23 13:47 ` Michael S. Tsirkin
2015-12-23 16:28 ` Greg Kurz
3 siblings, 1 reply; 8+ messages in thread
From: Michael S. Tsirkin @ 2015-12-23 13:47 UTC (permalink / raw)
To: Greg Kurz; +Cc: Cornelia Huck, qemu-devel
On Thu, Dec 17, 2015 at 09:52:46AM +0100, Greg Kurz wrote:
> This series tries to rework cross-endian helpers for better clarity.
> It does not change behaviour, except perhaps patch 3/3 even if I could not
> measure any performance gain.
Breaks build:
CC mips64-softmmu/hw/mips/mips_malta.o
/home/mst/scm/qemu/hw/net/vhost_net.c: In function
‘vhost_net_set_vnet_endian’:
/home/mst/scm/qemu/hw/net/vhost_net.c:208:10: error: implicit
declaration of function ‘virtio_legacy_is_cross_endian’
[-Werror=implicit-function-declaration]
(virtio_legacy_is_cross_endian(dev) &&
!virtio_is_big_endian(dev))) {
^
/home/mst/scm/qemu/hw/net/vhost_net.c:208:9: error: nested extern
declaration of ‘virtio_legacy_is_cross_endian’ [-Werror=nested-externs]
(virtio_legacy_is_cross_endian(dev) &&
!virtio_is_big_endian(dev))) {
^
cc1: all warnings being treated as errors
/home/mst/scm/qemu/rules.mak:57: recipe for target 'hw/net/vhost_net.o'
failed
make[1]: *** [hw/net/vhost_net.o] Error 1
Makefile:186: recipe for target 'subdir-i386-softmmu' failed
make: *** [subdir-i386-softmmu] Error 2
please always build all architectures.
> ---
>
> Greg Kurz (3):
> virtio: move cross-endian helper to vhost
> vhost: move virtio 1.0 check to cross-endian helper
> virtio: optimize virtio_access_is_big_endian() for little-endian targets
>
>
> hw/virtio/vhost.c | 22 ++++++++++++++++++----
> include/hw/virtio/virtio-access.h | 16 +++-------------
> 2 files changed, 21 insertions(+), 17 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] virtio: cross-endian helpers fixes
2015-12-23 13:47 ` [Qemu-devel] [PATCH v2 0/3] virtio: cross-endian helpers fixes Michael S. Tsirkin
@ 2015-12-23 16:28 ` Greg Kurz
2016-01-05 19:19 ` Greg Kurz
0 siblings, 1 reply; 8+ messages in thread
From: Greg Kurz @ 2015-12-23 16:28 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Cornelia Huck, qemu-devel
On Wed, 23 Dec 2015 15:47:00 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Thu, Dec 17, 2015 at 09:52:46AM +0100, Greg Kurz wrote:
> > This series tries to rework cross-endian helpers for better clarity.
> > It does not change behaviour, except perhaps patch 3/3 even if I could not
> > measure any performance gain.
>
> Breaks build:
>
> CC mips64-softmmu/hw/mips/mips_malta.o
> /home/mst/scm/qemu/hw/net/vhost_net.c: In function
> ‘vhost_net_set_vnet_endian’:
> /home/mst/scm/qemu/hw/net/vhost_net.c:208:10: error: implicit
> declaration of function ‘virtio_legacy_is_cross_endian’
> [-Werror=implicit-function-declaration]
> (virtio_legacy_is_cross_endian(dev) &&
> !virtio_is_big_endian(dev))) {
> ^
> /home/mst/scm/qemu/hw/net/vhost_net.c:208:9: error: nested extern
> declaration of ‘virtio_legacy_is_cross_endian’ [-Werror=nested-externs]
> (virtio_legacy_is_cross_endian(dev) &&
> !virtio_is_big_endian(dev))) {
> ^
> cc1: all warnings being treated as errors
> /home/mst/scm/qemu/rules.mak:57: recipe for target 'hw/net/vhost_net.o'
> failed
> make[1]: *** [hw/net/vhost_net.o] Error 1
> Makefile:186: recipe for target 'subdir-i386-softmmu' failed
> make: *** [subdir-i386-softmmu] Error 2
>
>
> please always build all architectures.
>
Ok. I'll do so from now on.
> > ---
> >
> > Greg Kurz (3):
> > virtio: move cross-endian helper to vhost
> > vhost: move virtio 1.0 check to cross-endian helper
> > virtio: optimize virtio_access_is_big_endian() for little-endian targets
> >
> >
> > hw/virtio/vhost.c | 22 ++++++++++++++++++----
> > include/hw/virtio/virtio-access.h | 16 +++-------------
> > 2 files changed, 21 insertions(+), 17 deletions(-)
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] virtio: cross-endian helpers fixes
2015-12-23 16:28 ` Greg Kurz
@ 2016-01-05 19:19 ` Greg Kurz
2016-01-06 11:09 ` Michael S. Tsirkin
0 siblings, 1 reply; 8+ messages in thread
From: Greg Kurz @ 2016-01-05 19:19 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Cornelia Huck, qemu-devel
On Wed, 23 Dec 2015 17:28:23 +0100
Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
> On Wed, 23 Dec 2015 15:47:00 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Thu, Dec 17, 2015 at 09:52:46AM +0100, Greg Kurz wrote:
> > > This series tries to rework cross-endian helpers for better clarity.
> > > It does not change behaviour, except perhaps patch 3/3 even if I could not
> > > measure any performance gain.
> >
> > Breaks build:
> >
> > CC mips64-softmmu/hw/mips/mips_malta.o
> > /home/mst/scm/qemu/hw/net/vhost_net.c: In function
> > ‘vhost_net_set_vnet_endian’:
> > /home/mst/scm/qemu/hw/net/vhost_net.c:208:10: error: implicit
> > declaration of function ‘virtio_legacy_is_cross_endian’
> > [-Werror=implicit-function-declaration]
> > (virtio_legacy_is_cross_endian(dev) &&
> > !virtio_is_big_endian(dev))) {
> > ^
> > /home/mst/scm/qemu/hw/net/vhost_net.c:208:9: error: nested extern
> > declaration of ‘virtio_legacy_is_cross_endian’ [-Werror=nested-externs]
> > (virtio_legacy_is_cross_endian(dev) &&
> > !virtio_is_big_endian(dev))) {
> > ^
> > cc1: all warnings being treated as errors
> > /home/mst/scm/qemu/rules.mak:57: recipe for target 'hw/net/vhost_net.o'
> > failed
> > make[1]: *** [hw/net/vhost_net.o] Error 1
> > Makefile:186: recipe for target 'subdir-i386-softmmu' failed
> > make: *** [subdir-i386-softmmu] Error 2
> >
> >
> > please always build all architectures.
> >
>
> Ok. I'll do so from now on.
>
The break isn't architecture related actually. It is because this series
depends on the "virtio-net/vhost-net: share cross-endian enablement" series
I had posted before... my bad. Since most of these series is cleanup of the
cross-endian code, I'll repost a single series with all the patches.
> > > ---
> > >
> > > Greg Kurz (3):
> > > virtio: move cross-endian helper to vhost
> > > vhost: move virtio 1.0 check to cross-endian helper
> > > virtio: optimize virtio_access_is_big_endian() for little-endian targets
> > >
> > >
> > > hw/virtio/vhost.c | 22 ++++++++++++++++++----
> > > include/hw/virtio/virtio-access.h | 16 +++-------------
> > > 2 files changed, 21 insertions(+), 17 deletions(-)
> >
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] virtio: cross-endian helpers fixes
2016-01-05 19:19 ` Greg Kurz
@ 2016-01-06 11:09 ` Michael S. Tsirkin
0 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2016-01-06 11:09 UTC (permalink / raw)
To: Greg Kurz; +Cc: Cornelia Huck, qemu-devel
On Tue, Jan 05, 2016 at 08:19:22PM +0100, Greg Kurz wrote:
> On Wed, 23 Dec 2015 17:28:23 +0100
> Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
>
> > On Wed, 23 Dec 2015 15:47:00 +0200
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >
> > > On Thu, Dec 17, 2015 at 09:52:46AM +0100, Greg Kurz wrote:
> > > > This series tries to rework cross-endian helpers for better clarity.
> > > > It does not change behaviour, except perhaps patch 3/3 even if I could not
> > > > measure any performance gain.
> > >
> > > Breaks build:
> > >
> > > CC mips64-softmmu/hw/mips/mips_malta.o
> > > /home/mst/scm/qemu/hw/net/vhost_net.c: In function
> > > ‘vhost_net_set_vnet_endian’:
> > > /home/mst/scm/qemu/hw/net/vhost_net.c:208:10: error: implicit
> > > declaration of function ‘virtio_legacy_is_cross_endian’
> > > [-Werror=implicit-function-declaration]
> > > (virtio_legacy_is_cross_endian(dev) &&
> > > !virtio_is_big_endian(dev))) {
> > > ^
> > > /home/mst/scm/qemu/hw/net/vhost_net.c:208:9: error: nested extern
> > > declaration of ‘virtio_legacy_is_cross_endian’ [-Werror=nested-externs]
> > > (virtio_legacy_is_cross_endian(dev) &&
> > > !virtio_is_big_endian(dev))) {
> > > ^
> > > cc1: all warnings being treated as errors
> > > /home/mst/scm/qemu/rules.mak:57: recipe for target 'hw/net/vhost_net.o'
> > > failed
> > > make[1]: *** [hw/net/vhost_net.o] Error 1
> > > Makefile:186: recipe for target 'subdir-i386-softmmu' failed
> > > make: *** [subdir-i386-softmmu] Error 2
> > >
> > >
> > > please always build all architectures.
> > >
> >
> > Ok. I'll do so from now on.
> >
>
> The break isn't architecture related actually. It is because this series
> depends on the "virtio-net/vhost-net: share cross-endian enablement" series
> I had posted before... my bad. Since most of these series is cleanup of the
> cross-endian code,
Oh. And I thought it's the reverse order somehow.
> I'll repost a single series with all the patches.
That's probably best.
> > > > ---
> > > >
> > > > Greg Kurz (3):
> > > > virtio: move cross-endian helper to vhost
> > > > vhost: move virtio 1.0 check to cross-endian helper
> > > > virtio: optimize virtio_access_is_big_endian() for little-endian targets
> > > >
> > > >
> > > > hw/virtio/vhost.c | 22 ++++++++++++++++++----
> > > > include/hw/virtio/virtio-access.h | 16 +++-------------
> > > > 2 files changed, 21 insertions(+), 17 deletions(-)
> > >
> >
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-01-06 11:09 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-17 8:52 [Qemu-devel] [PATCH v2 0/3] virtio: cross-endian helpers fixes Greg Kurz
2015-12-17 8:52 ` [Qemu-devel] [PATCH v2 1/3] virtio: move cross-endian helper to vhost Greg Kurz
2015-12-17 8:53 ` [Qemu-devel] [PATCH v2 2/3] vhost: move virtio 1.0 check to cross-endian helper Greg Kurz
2015-12-17 8:55 ` [Qemu-devel] [PATCH v2 3/3] virtio: optimize virtio_access_is_big_endian() for little-endian targets Greg Kurz
2015-12-23 13:47 ` [Qemu-devel] [PATCH v2 0/3] virtio: cross-endian helpers fixes Michael S. Tsirkin
2015-12-23 16:28 ` Greg Kurz
2016-01-05 19:19 ` Greg Kurz
2016-01-06 11:09 ` Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.