2015-07-01 09:32:09

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PULL] virtio/vhost: cross endian support

The following changes since commit 8a7b19d8b542b87bccc3eaaf81dcc90a5ca48aea:

include/uapi/linux/virtio_balloon.h: include linux/virtio_types.h (2015-06-01 15:46:54 +0200)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus

for you to fetch changes up to 59a5b0f7bf74f88da6670bcbf924d8cc1e75b1ee:

virtio-pci: alloc only resources actually used. (2015-06-24 08:15:09 +0200)

----------------------------------------------------------------
virtio/vhost: cross endian support

I have just queued some more bugfix patches today but none fix regressions and
none are related to these ones, so it looks like a good time for a merge for
-rc1.

Signed-off-by: Michael S. Tsirkin <[email protected]>

----------------------------------------------------------------
Gerd Hoffmann (1):
virtio-pci: alloc only resources actually used.

Greg Kurz (8):
virtio: introduce virtio_is_little_endian() helper
tun: add tun_is_little_endian() helper
macvtap: introduce macvtap_is_little_endian() helper
vringh: introduce vringh_is_little_endian() helper
vhost: introduce vhost_is_little_endian() helper
virtio: add explicit big-endian support to memory accessors
vhost: cross-endian support for legacy devices
macvtap/tun: cross-endian support for little-endian hosts

drivers/vhost/vhost.h | 25 ++++++++---
drivers/virtio/virtio_pci_common.h | 2 +
include/linux/virtio_byteorder.h | 24 ++++++-----
include/linux/virtio_config.h | 18 +++++---
include/linux/vringh.h | 18 +++++---
include/uapi/linux/if_tun.h | 6 +++
include/uapi/linux/vhost.h | 14 +++++++
drivers/net/macvtap.c | 65 ++++++++++++++++++++++++++++-
drivers/net/tun.c | 67 +++++++++++++++++++++++++++++-
drivers/vhost/vhost.c | 85 +++++++++++++++++++++++++++++++++++++-
drivers/virtio/virtio_pci_common.c | 7 ----
drivers/virtio/virtio_pci_legacy.c | 13 +++++-
drivers/virtio/virtio_pci_modern.c | 24 ++++++++---
drivers/net/Kconfig | 14 +++++++
drivers/vhost/Kconfig | 15 +++++++
15 files changed, 350 insertions(+), 47 deletions(-)


2015-07-01 19:02:59

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PULL] virtio/vhost: cross endian support

On Wed, Jul 1, 2015 at 2:31 AM, Michael S. Tsirkin <[email protected]> wrote:
> virtio/vhost: cross endian support

Ugh. Does this really have to be dynamic?

Can't virtio do the sane thing, and just use a _fixed_ endianness?

Doing a unconditional byte swap is faster and simpler than the crazy
conditionals. That's true regardless of endianness, but gets to be
even more so if the fixed endianness is little-endian, since BE is
not-so-slowly fading from the world.

Linus

2015-07-01 19:04:08

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PULL] virtio/vhost: cross endian support

On Wed, Jul 1, 2015 at 12:02 PM, Linus Torvalds
<[email protected]> wrote:
>
> Doing a unconditional byte swap is faster and simpler than the crazy
> conditionals.

Unconditional endianness not only makes for simpler and faster code,
it also ends up being easier to debug and add things like type
annotations for sparse.

Linus

2015-07-02 06:02:13

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PULL] virtio/vhost: cross endian support

On Wed, Jul 01, 2015 at 12:02:50PM -0700, Linus Torvalds wrote:
> On Wed, Jul 1, 2015 at 2:31 AM, Michael S. Tsirkin <[email protected]> wrote:
> > virtio/vhost: cross endian support
>
> Ugh. Does this really have to be dynamic?
>
> Can't virtio do the sane thing, and just use a _fixed_ endianness?
>
> Doing a unconditional byte swap is faster and simpler than the crazy
> conditionals. That's true regardless of endianness, but gets to be
> even more so if the fixed endianness is little-endian, since BE is
> not-so-slowly fading from the world.
>
> Linus

Yea, well - support for legacy BE guests on the new LE hosts is
exactly the motivation for this.

I dislike it too, but there are two redeeming properties that
made me merge this:

1. It's a trivial amount of code: since we wrap host/guest accesses
anyway, almost all of it is well hidden from drivers.

2. Sane platforms would never set flags like VHOST_CROSS_ENDIAN_LEGACY -
and when it's clear, there's zero overhead (as some point it was
tested by compiling with and without the patches, got the same
stripped binary).

Maybe we could create a Kconfig symbol to enforce point (2): prevent
people from enabling it e.g. on x86. I will look into this - but it can
be done by a patch on top, so I think this can be merged as is.

Or do you know of someone using kernel with all config options enabled
undiscriminately?

Thanks,

--
MST

2015-07-02 06:12:49

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PULL] virtio/vhost: cross endian support

On Wed, Jul 01, 2015 at 12:03:59PM -0700, Linus Torvalds wrote:
> On Wed, Jul 1, 2015 at 12:02 PM, Linus Torvalds
> <[email protected]> wrote:
> >
> > Doing a unconditional byte swap is faster and simpler than the crazy
> > conditionals.
>
> Unconditional endianness not only makes for simpler and faster code,
> it also ends up being easier to debug and add things like type
> annotations for sparse.
>
> Linus

At least this last one is well covered by these patches: this uses
separate sparse types so all accesses are statically verified by sparse
to use the correct accessor.

--
MST

2015-07-02 09:13:17

by Gerg Kurz

[permalink] [raw]
Subject: Re: [PULL] virtio/vhost: cross endian support

On Thu, 2 Jul 2015 08:01:28 +0200
"Michael S. Tsirkin" <[email protected]> wrote:

> On Wed, Jul 01, 2015 at 12:02:50PM -0700, Linus Torvalds wrote:
> > On Wed, Jul 1, 2015 at 2:31 AM, Michael S. Tsirkin <[email protected]> wrote:
> > > virtio/vhost: cross endian support
> >
> > Ugh. Does this really have to be dynamic?
> >
> > Can't virtio do the sane thing, and just use a _fixed_ endianness?
> >
> > Doing a unconditional byte swap is faster and simpler than the crazy
> > conditionals. That's true regardless of endianness, but gets to be
> > even more so if the fixed endianness is little-endian, since BE is
> > not-so-slowly fading from the world.
> >
> > Linus
>
> Yea, well - support for legacy BE guests on the new LE hosts is
> exactly the motivation for this.
>
> I dislike it too, but there are two redeeming properties that
> made me merge this:
>
> 1. It's a trivial amount of code: since we wrap host/guest accesses
> anyway, almost all of it is well hidden from drivers.
>
> 2. Sane platforms would never set flags like VHOST_CROSS_ENDIAN_LEGACY -
> and when it's clear, there's zero overhead (as some point it was
> tested by compiling with and without the patches, got the same
> stripped binary).
>
> Maybe we could create a Kconfig symbol to enforce point (2): prevent
> people from enabling it e.g. on x86. I will look into this - but it can
> be done by a patch on top, so I think this can be merged as is.
>

This cross-endian *oddity* is targeting PowerPC book3s_64 processors... I
am not aware of any other users. Maybe create a symbol that would
be only selected by PPC_BOOK3S_64 ?


> Or do you know of someone using kernel with all config options enabled
> undiscriminately?
>
> Thanks,
>

2015-07-02 09:33:06

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PULL] virtio/vhost: cross endian support

On Thu, Jul 02, 2015 at 11:12:56AM +0200, Greg Kurz wrote:
> On Thu, 2 Jul 2015 08:01:28 +0200
> "Michael S. Tsirkin" <[email protected]> wrote:
>
> > On Wed, Jul 01, 2015 at 12:02:50PM -0700, Linus Torvalds wrote:
> > > On Wed, Jul 1, 2015 at 2:31 AM, Michael S. Tsirkin <[email protected]> wrote:
> > > > virtio/vhost: cross endian support
> > >
> > > Ugh. Does this really have to be dynamic?
> > >
> > > Can't virtio do the sane thing, and just use a _fixed_ endianness?
> > >
> > > Doing a unconditional byte swap is faster and simpler than the crazy
> > > conditionals. That's true regardless of endianness, but gets to be
> > > even more so if the fixed endianness is little-endian, since BE is
> > > not-so-slowly fading from the world.
> > >
> > > Linus
> >
> > Yea, well - support for legacy BE guests on the new LE hosts is
> > exactly the motivation for this.
> >
> > I dislike it too, but there are two redeeming properties that
> > made me merge this:
> >
> > 1. It's a trivial amount of code: since we wrap host/guest accesses
> > anyway, almost all of it is well hidden from drivers.
> >
> > 2. Sane platforms would never set flags like VHOST_CROSS_ENDIAN_LEGACY -
> > and when it's clear, there's zero overhead (as some point it was
> > tested by compiling with and without the patches, got the same
> > stripped binary).
> >
> > Maybe we could create a Kconfig symbol to enforce point (2): prevent
> > people from enabling it e.g. on x86. I will look into this - but it can
> > be done by a patch on top, so I think this can be merged as is.
> >
>
> This cross-endian *oddity* is targeting PowerPC book3s_64 processors... I
> am not aware of any other users. Maybe create a symbol that would
> be only selected by PPC_BOOK3S_64 ?

I think some ARM systems are trying to support cross-endian
configurations as well.

Besides that, yes, this is more or less what I had in mind.

>
> > Or do you know of someone using kernel with all config options enabled
> > undiscriminately?
> >
> > Thanks,
> >

2015-07-03 07:59:17

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PULL] virtio/vhost: cross endian support

On Thu, Jul 02, 2015 at 08:01:28AM +0200, Michael S. Tsirkin wrote:
> On Wed, Jul 01, 2015 at 12:02:50PM -0700, Linus Torvalds wrote:
> > On Wed, Jul 1, 2015 at 2:31 AM, Michael S. Tsirkin <[email protected]> wrote:
> > > virtio/vhost: cross endian support
> >
> > Ugh. Does this really have to be dynamic?
> >
> > Can't virtio do the sane thing, and just use a _fixed_ endianness?
> >
> > Doing a unconditional byte swap is faster and simpler than the crazy
> > conditionals. That's true regardless of endianness, but gets to be
> > even more so if the fixed endianness is little-endian, since BE is
> > not-so-slowly fading from the world.
> >
> > Linus
>
> Yea, well - support for legacy BE guests on the new LE hosts is
> exactly the motivation for this.
>
> I dislike it too, but there are two redeeming properties that
> made me merge this:
>
> 1. It's a trivial amount of code: since we wrap host/guest accesses
> anyway, almost all of it is well hidden from drivers.
>
> 2. Sane platforms would never set flags like VHOST_CROSS_ENDIAN_LEGACY -
> and when it's clear, there's zero overhead (as some point it was
> tested by compiling with and without the patches, got the same
> stripped binary).
>
> Maybe we could create a Kconfig symbol to enforce point (2): prevent
> people from enabling it e.g. on x86. I will look into this - but it can
> be done by a patch on top, so I think this can be merged as is.

Linus, could you please clarify whether making the
feature depend on the cross-endian guest support
would address your comment, and whether you think
this can be merged as is, and the dependency added
after -rc1?


> Or do you know of someone using kernel with all config options enabled
> undiscriminately?
>
> Thanks,
>
> --
> MST

2015-07-03 22:26:10

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PULL] virtio/vhost: cross endian support

On Fri, Jul 3, 2015 at 12:59 AM, Michael S. Tsirkin <[email protected]> wrote:
>
> Linus, could you please clarify whether making the
> feature depend on the cross-endian guest support
> would address your comment, and whether you think
> this can be merged as is, and the dependency added
> after -rc1?

I'll take it. I still think dynamic byte order is a fundamental (and
much too common) mistake, but whatever.

Linus

2015-07-07 16:37:10

by Thomas Huth

[permalink] [raw]
Subject: Re: [PULL] virtio/vhost: cross endian support

On Thu, 2 Jul 2015 11:32:52 +0200
"Michael S. Tsirkin" <[email protected]> wrote:

> On Thu, Jul 02, 2015 at 11:12:56AM +0200, Greg Kurz wrote:
> > On Thu, 2 Jul 2015 08:01:28 +0200
> > "Michael S. Tsirkin" <[email protected]> wrote:
...
> > > Yea, well - support for legacy BE guests on the new LE hosts is
> > > exactly the motivation for this.
> > >
> > > I dislike it too, but there are two redeeming properties that
> > > made me merge this:
> > >
> > > 1. It's a trivial amount of code: since we wrap host/guest accesses
> > > anyway, almost all of it is well hidden from drivers.
> > >
> > > 2. Sane platforms would never set flags like VHOST_CROSS_ENDIAN_LEGACY -
> > > and when it's clear, there's zero overhead (as some point it was
> > > tested by compiling with and without the patches, got the same
> > > stripped binary).
> > >
> > > Maybe we could create a Kconfig symbol to enforce point (2): prevent
> > > people from enabling it e.g. on x86. I will look into this - but it can
> > > be done by a patch on top, so I think this can be merged as is.
> > >
> >
> > This cross-endian *oddity* is targeting PowerPC book3s_64 processors... I
> > am not aware of any other users. Maybe create a symbol that would
> > be only selected by PPC_BOOK3S_64 ?
>
> I think some ARM systems are trying to support cross-endian
> configurations as well.
>
> Besides that, yes, this is more or less what I had in mind.

Would something simple like this already do the job:

diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
--- a/drivers/vhost/Kconfig
+++ b/drivers/vhost/Kconfig
@@ -35,6 +35,7 @@ config VHOST

config VHOST_CROSS_ENDIAN_LEGACY
bool "Cross-endian support for vhost"
+ depends on KVM_BOOK3S_64 || KVM_ARM_HOST
default n
---help---
This option allows vhost to support guests with a different byte

?

If that looks acceptable, I can submit a proper patch if you like.

Thomas

2015-07-07 16:52:00

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PULL] virtio/vhost: cross endian support

On Tue, Jul 07, 2015 at 06:36:53PM +0200, Thomas Huth wrote:
> On Thu, 2 Jul 2015 11:32:52 +0200
> "Michael S. Tsirkin" <[email protected]> wrote:
>
> > On Thu, Jul 02, 2015 at 11:12:56AM +0200, Greg Kurz wrote:
> > > On Thu, 2 Jul 2015 08:01:28 +0200
> > > "Michael S. Tsirkin" <[email protected]> wrote:
> ...
> > > > Yea, well - support for legacy BE guests on the new LE hosts is
> > > > exactly the motivation for this.
> > > >
> > > > I dislike it too, but there are two redeeming properties that
> > > > made me merge this:
> > > >
> > > > 1. It's a trivial amount of code: since we wrap host/guest accesses
> > > > anyway, almost all of it is well hidden from drivers.
> > > >
> > > > 2. Sane platforms would never set flags like VHOST_CROSS_ENDIAN_LEGACY -
> > > > and when it's clear, there's zero overhead (as some point it was
> > > > tested by compiling with and without the patches, got the same
> > > > stripped binary).
> > > >
> > > > Maybe we could create a Kconfig symbol to enforce point (2): prevent
> > > > people from enabling it e.g. on x86. I will look into this - but it can
> > > > be done by a patch on top, so I think this can be merged as is.
> > > >
> > >
> > > This cross-endian *oddity* is targeting PowerPC book3s_64 processors... I
> > > am not aware of any other users. Maybe create a symbol that would
> > > be only selected by PPC_BOOK3S_64 ?
> >
> > I think some ARM systems are trying to support cross-endian
> > configurations as well.
> >
> > Besides that, yes, this is more or less what I had in mind.
>
> Would something simple like this already do the job:
>
> diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
> --- a/drivers/vhost/Kconfig
> +++ b/drivers/vhost/Kconfig
> @@ -35,6 +35,7 @@ config VHOST
>
> config VHOST_CROSS_ENDIAN_LEGACY
> bool "Cross-endian support for vhost"
> + depends on KVM_BOOK3S_64 || KVM_ARM_HOST
> default n
> ---help---
> This option allows vhost to support guests with a different byte
>
> ?

Do all ARM hosts support this dynamic endian-ness?

> If that looks acceptable, I can submit a proper patch if you like.
>
> Thomas

I think I prefer some kind of symbol defined by these arches,
so I don't get to maintain an arch list in vhost.

--
MST

2015-07-09 07:49:22

by Thomas Huth

[permalink] [raw]
Subject: [PATCH] KVM: Add Kconfig option to signal cross-endian guests

The option for supporting cross-endianness legacy guests in
the vhost and tun code should only be available on systems
that support cross-endian guests.

Signed-off-by: Thomas Huth <[email protected]>
---
arch/arm/kvm/Kconfig | 1 +
arch/arm64/kvm/Kconfig | 1 +
arch/powerpc/kvm/Kconfig | 1 +
drivers/net/Kconfig | 1 +
drivers/vhost/Kconfig | 1 +
virt/kvm/Kconfig | 3 +++
6 files changed, 8 insertions(+)

diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
index bfb915d..9d8f363 100644
--- a/arch/arm/kvm/Kconfig
+++ b/arch/arm/kvm/Kconfig
@@ -31,6 +31,7 @@ config KVM
select KVM_VFIO
select HAVE_KVM_EVENTFD
select HAVE_KVM_IRQFD
+ select KVM_CROSS_ENDIAN_GUESTS
depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
---help---
Support hosting virtualized guest machines.
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index bfffe8f..9af39fe 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -31,6 +31,7 @@ config KVM
select KVM_VFIO
select HAVE_KVM_EVENTFD
select HAVE_KVM_IRQFD
+ select KVM_CROSS_ENDIAN_GUESTS
---help---
Support hosting virtualized guest machines.

diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
index 3caec2c..e028710 100644
--- a/arch/powerpc/kvm/Kconfig
+++ b/arch/powerpc/kvm/Kconfig
@@ -79,6 +79,7 @@ config KVM_BOOK3S_64_HV
select KVM_BOOK3S_HV_POSSIBLE
select MMU_NOTIFIER
select CMA
+ select KVM_CROSS_ENDIAN_GUESTS
---help---
Support running unmodified book3s_64 guest kernels in
virtual machines on POWER7 and PPC970 processors that have
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index c18f9e6..0c4ce47 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -261,6 +261,7 @@ config TUN
config TUN_VNET_CROSS_LE
bool "Support for cross-endian vnet headers on little-endian kernels"
default n
+ depends on KVM_CROSS_ENDIAN_GUESTS
---help---
This option allows TUN/TAP and MACVTAP device drivers in a
little-endian kernel to parse vnet headers that come from a
diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
index 533eaf0..4d8ae6b 100644
--- a/drivers/vhost/Kconfig
+++ b/drivers/vhost/Kconfig
@@ -35,6 +35,7 @@ config VHOST

config VHOST_CROSS_ENDIAN_LEGACY
bool "Cross-endian support for vhost"
+ depends on KVM_CROSS_ENDIAN_GUESTS
default n
---help---
This option allows vhost to support guests with a different byte
diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index e2c876d..cc7b28a 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig
@@ -47,3 +47,6 @@ config KVM_GENERIC_DIRTYLOG_READ_PROTECT
config KVM_COMPAT
def_bool y
depends on COMPAT && !S390
+
+config KVM_CROSS_ENDIAN_GUESTS
+ bool
--
1.8.3.1

2015-07-09 09:48:25

by Laurent Vivier

[permalink] [raw]
Subject: Re: [PATCH] KVM: Add Kconfig option to signal cross-endian guests



On 09/07/2015 09:49, Thomas Huth wrote:
> The option for supporting cross-endianness legacy guests in
> the vhost and tun code should only be available on systems
> that support cross-endian guests.

I'm sure I misunderstand something, but what happens if we use QEMU with
TCG instead of KVM, i.e. a big endian powerpc kernel guest on x86_64
little endian host ?
Do you forbid the use of vhost in this case ?

> Signed-off-by: Thomas Huth <[email protected]>
> ---
> arch/arm/kvm/Kconfig | 1 +
> arch/arm64/kvm/Kconfig | 1 +
> arch/powerpc/kvm/Kconfig | 1 +
> drivers/net/Kconfig | 1 +
> drivers/vhost/Kconfig | 1 +
> virt/kvm/Kconfig | 3 +++
> 6 files changed, 8 insertions(+)
>
> diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
> index bfb915d..9d8f363 100644
> --- a/arch/arm/kvm/Kconfig
> +++ b/arch/arm/kvm/Kconfig
> @@ -31,6 +31,7 @@ config KVM
> select KVM_VFIO
> select HAVE_KVM_EVENTFD
> select HAVE_KVM_IRQFD
> + select KVM_CROSS_ENDIAN_GUESTS
> depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
> ---help---
> Support hosting virtualized guest machines.
> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> index bfffe8f..9af39fe 100644
> --- a/arch/arm64/kvm/Kconfig
> +++ b/arch/arm64/kvm/Kconfig
> @@ -31,6 +31,7 @@ config KVM
> select KVM_VFIO
> select HAVE_KVM_EVENTFD
> select HAVE_KVM_IRQFD
> + select KVM_CROSS_ENDIAN_GUESTS
> ---help---
> Support hosting virtualized guest machines.
>
> diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
> index 3caec2c..e028710 100644
> --- a/arch/powerpc/kvm/Kconfig
> +++ b/arch/powerpc/kvm/Kconfig
> @@ -79,6 +79,7 @@ config KVM_BOOK3S_64_HV
> select KVM_BOOK3S_HV_POSSIBLE
> select MMU_NOTIFIER
> select CMA
> + select KVM_CROSS_ENDIAN_GUESTS
> ---help---
> Support running unmodified book3s_64 guest kernels in
> virtual machines on POWER7 and PPC970 processors that have
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index c18f9e6..0c4ce47 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -261,6 +261,7 @@ config TUN
> config TUN_VNET_CROSS_LE
> bool "Support for cross-endian vnet headers on little-endian kernels"
> default n
> + depends on KVM_CROSS_ENDIAN_GUESTS
> ---help---
> This option allows TUN/TAP and MACVTAP device drivers in a
> little-endian kernel to parse vnet headers that come from a
> diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
> index 533eaf0..4d8ae6b 100644
> --- a/drivers/vhost/Kconfig
> +++ b/drivers/vhost/Kconfig
> @@ -35,6 +35,7 @@ config VHOST
>
> config VHOST_CROSS_ENDIAN_LEGACY
> bool "Cross-endian support for vhost"
> + depends on KVM_CROSS_ENDIAN_GUESTS
> default n
> ---help---
> This option allows vhost to support guests with a different byte
> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
> index e2c876d..cc7b28a 100644
> --- a/virt/kvm/Kconfig
> +++ b/virt/kvm/Kconfig
> @@ -47,3 +47,6 @@ config KVM_GENERIC_DIRTYLOG_READ_PROTECT
> config KVM_COMPAT
> def_bool y
> depends on COMPAT && !S390
> +
> +config KVM_CROSS_ENDIAN_GUESTS
> + bool
>

2015-07-09 10:03:02

by Christoffer Dall

[permalink] [raw]
Subject: Re: [PATCH] KVM: Add Kconfig option to signal cross-endian guests

On Thu, Jul 09, 2015 at 09:49:05AM +0200, Thomas Huth wrote:
> The option for supporting cross-endianness legacy guests in
> the vhost and tun code should only be available on systems
> that support cross-endian guests.
>
> Signed-off-by: Thomas Huth <[email protected]>

Acked-by: Christoffer Dall <[email protected]>

2015-07-09 12:22:11

by Cornelia Huck

[permalink] [raw]
Subject: Re: [PATCH] KVM: Add Kconfig option to signal cross-endian guests

On Thu, 9 Jul 2015 09:49:05 +0200
Thomas Huth <[email protected]> wrote:

> The option for supporting cross-endianness legacy guests in

s/cross-endianness/cross-endian/ ?

> the vhost and tun code should only be available on systems
> that support cross-endian guests.
>
> Signed-off-by: Thomas Huth <[email protected]>
> ---
> arch/arm/kvm/Kconfig | 1 +
> arch/arm64/kvm/Kconfig | 1 +
> arch/powerpc/kvm/Kconfig | 1 +
> drivers/net/Kconfig | 1 +
> drivers/vhost/Kconfig | 1 +
> virt/kvm/Kconfig | 3 +++
> 6 files changed, 8 insertions(+)

Acked-by: Cornelia Huck <[email protected]>

2015-07-09 12:57:54

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH] KVM: Add Kconfig option to signal cross-endian guests



On 09/07/2015 11:48, Laurent Vivier wrote:
>
>
> On 09/07/2015 09:49, Thomas Huth wrote:
>> The option for supporting cross-endianness legacy guests in
>> the vhost and tun code should only be available on systems
>> that support cross-endian guests.
>
> I'm sure I misunderstand something, but what happens if we use QEMU with
> TCG instead of KVM, i.e. a big endian powerpc kernel guest on x86_64
> little endian host ?

TCG does not yet support irqfd/ioeventfd, so it cannot be used with vhost.

Paolo

> Do you forbid the use of vhost in this case ?
>
>> Signed-off-by: Thomas Huth <[email protected]>
>> ---
>> arch/arm/kvm/Kconfig | 1 +
>> arch/arm64/kvm/Kconfig | 1 +
>> arch/powerpc/kvm/Kconfig | 1 +
>> drivers/net/Kconfig | 1 +
>> drivers/vhost/Kconfig | 1 +
>> virt/kvm/Kconfig | 3 +++
>> 6 files changed, 8 insertions(+)
>>
>> diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
>> index bfb915d..9d8f363 100644
>> --- a/arch/arm/kvm/Kconfig
>> +++ b/arch/arm/kvm/Kconfig
>> @@ -31,6 +31,7 @@ config KVM
>> select KVM_VFIO
>> select HAVE_KVM_EVENTFD
>> select HAVE_KVM_IRQFD
>> + select KVM_CROSS_ENDIAN_GUESTS
>> depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
>> ---help---
>> Support hosting virtualized guest machines.
>> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
>> index bfffe8f..9af39fe 100644
>> --- a/arch/arm64/kvm/Kconfig
>> +++ b/arch/arm64/kvm/Kconfig
>> @@ -31,6 +31,7 @@ config KVM
>> select KVM_VFIO
>> select HAVE_KVM_EVENTFD
>> select HAVE_KVM_IRQFD
>> + select KVM_CROSS_ENDIAN_GUESTS
>> ---help---
>> Support hosting virtualized guest machines.
>>
>> diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
>> index 3caec2c..e028710 100644
>> --- a/arch/powerpc/kvm/Kconfig
>> +++ b/arch/powerpc/kvm/Kconfig
>> @@ -79,6 +79,7 @@ config KVM_BOOK3S_64_HV
>> select KVM_BOOK3S_HV_POSSIBLE
>> select MMU_NOTIFIER
>> select CMA
>> + select KVM_CROSS_ENDIAN_GUESTS
>> ---help---
>> Support running unmodified book3s_64 guest kernels in
>> virtual machines on POWER7 and PPC970 processors that have
>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>> index c18f9e6..0c4ce47 100644
>> --- a/drivers/net/Kconfig
>> +++ b/drivers/net/Kconfig
>> @@ -261,6 +261,7 @@ config TUN
>> config TUN_VNET_CROSS_LE
>> bool "Support for cross-endian vnet headers on little-endian kernels"
>> default n
>> + depends on KVM_CROSS_ENDIAN_GUESTS
>> ---help---
>> This option allows TUN/TAP and MACVTAP device drivers in a
>> little-endian kernel to parse vnet headers that come from a
>> diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
>> index 533eaf0..4d8ae6b 100644
>> --- a/drivers/vhost/Kconfig
>> +++ b/drivers/vhost/Kconfig
>> @@ -35,6 +35,7 @@ config VHOST
>>
>> config VHOST_CROSS_ENDIAN_LEGACY
>> bool "Cross-endian support for vhost"
>> + depends on KVM_CROSS_ENDIAN_GUESTS
>> default n
>> ---help---
>> This option allows vhost to support guests with a different byte
>> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
>> index e2c876d..cc7b28a 100644
>> --- a/virt/kvm/Kconfig
>> +++ b/virt/kvm/Kconfig
>> @@ -47,3 +47,6 @@ config KVM_GENERIC_DIRTYLOG_READ_PROTECT
>> config KVM_COMPAT
>> def_bool y
>> depends on COMPAT && !S390
>> +
>> +config KVM_CROSS_ENDIAN_GUESTS
>> + bool
>>

2015-07-09 13:08:11

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PATCH] KVM: Add Kconfig option to signal cross-endian guests

On Thu, Jul 09, 2015 at 02:57:33PM +0200, Paolo Bonzini wrote:
>
>
> On 09/07/2015 11:48, Laurent Vivier wrote:
> >
> >
> > On 09/07/2015 09:49, Thomas Huth wrote:
> >> The option for supporting cross-endianness legacy guests in
> >> the vhost and tun code should only be available on systems
> >> that support cross-endian guests.
> >
> > I'm sure I misunderstand something, but what happens if we use QEMU with
> > TCG instead of KVM, i.e. a big endian powerpc kernel guest on x86_64
> > little endian host ?
>
> TCG does not yet support irqfd/ioeventfd, so it cannot be used with vhost.
>
> Paolo

vhost does not require irqfd anymore. I think ioeventfd actually works
fine though I didn't try, it would be easy to support.


> > Do you forbid the use of vhost in this case ?
> >
> >> Signed-off-by: Thomas Huth <[email protected]>
> >> ---
> >> arch/arm/kvm/Kconfig | 1 +
> >> arch/arm64/kvm/Kconfig | 1 +
> >> arch/powerpc/kvm/Kconfig | 1 +
> >> drivers/net/Kconfig | 1 +
> >> drivers/vhost/Kconfig | 1 +
> >> virt/kvm/Kconfig | 3 +++
> >> 6 files changed, 8 insertions(+)
> >>
> >> diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
> >> index bfb915d..9d8f363 100644
> >> --- a/arch/arm/kvm/Kconfig
> >> +++ b/arch/arm/kvm/Kconfig
> >> @@ -31,6 +31,7 @@ config KVM
> >> select KVM_VFIO
> >> select HAVE_KVM_EVENTFD
> >> select HAVE_KVM_IRQFD
> >> + select KVM_CROSS_ENDIAN_GUESTS
> >> depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
> >> ---help---
> >> Support hosting virtualized guest machines.
> >> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> >> index bfffe8f..9af39fe 100644
> >> --- a/arch/arm64/kvm/Kconfig
> >> +++ b/arch/arm64/kvm/Kconfig
> >> @@ -31,6 +31,7 @@ config KVM
> >> select KVM_VFIO
> >> select HAVE_KVM_EVENTFD
> >> select HAVE_KVM_IRQFD
> >> + select KVM_CROSS_ENDIAN_GUESTS
> >> ---help---
> >> Support hosting virtualized guest machines.
> >>
> >> diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
> >> index 3caec2c..e028710 100644
> >> --- a/arch/powerpc/kvm/Kconfig
> >> +++ b/arch/powerpc/kvm/Kconfig
> >> @@ -79,6 +79,7 @@ config KVM_BOOK3S_64_HV
> >> select KVM_BOOK3S_HV_POSSIBLE
> >> select MMU_NOTIFIER
> >> select CMA
> >> + select KVM_CROSS_ENDIAN_GUESTS
> >> ---help---
> >> Support running unmodified book3s_64 guest kernels in
> >> virtual machines on POWER7 and PPC970 processors that have
> >> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> >> index c18f9e6..0c4ce47 100644
> >> --- a/drivers/net/Kconfig
> >> +++ b/drivers/net/Kconfig
> >> @@ -261,6 +261,7 @@ config TUN
> >> config TUN_VNET_CROSS_LE
> >> bool "Support for cross-endian vnet headers on little-endian kernels"
> >> default n
> >> + depends on KVM_CROSS_ENDIAN_GUESTS
> >> ---help---
> >> This option allows TUN/TAP and MACVTAP device drivers in a
> >> little-endian kernel to parse vnet headers that come from a
> >> diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
> >> index 533eaf0..4d8ae6b 100644
> >> --- a/drivers/vhost/Kconfig
> >> +++ b/drivers/vhost/Kconfig
> >> @@ -35,6 +35,7 @@ config VHOST
> >>
> >> config VHOST_CROSS_ENDIAN_LEGACY
> >> bool "Cross-endian support for vhost"
> >> + depends on KVM_CROSS_ENDIAN_GUESTS
> >> default n
> >> ---help---
> >> This option allows vhost to support guests with a different byte
> >> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
> >> index e2c876d..cc7b28a 100644
> >> --- a/virt/kvm/Kconfig
> >> +++ b/virt/kvm/Kconfig
> >> @@ -47,3 +47,6 @@ config KVM_GENERIC_DIRTYLOG_READ_PROTECT
> >> config KVM_COMPAT
> >> def_bool y
> >> depends on COMPAT && !S390
> >> +
> >> +config KVM_CROSS_ENDIAN_GUESTS
> >> + bool
> >>

2015-07-09 18:59:46

by Thomas Huth

[permalink] [raw]
Subject: Re: [PATCH] KVM: Add Kconfig option to signal cross-endian guests

On Thu, 9 Jul 2015 16:07:47 +0300
"Michael S. Tsirkin" <[email protected]> wrote:

> On Thu, Jul 09, 2015 at 02:57:33PM +0200, Paolo Bonzini wrote:
> >
> >
> > On 09/07/2015 11:48, Laurent Vivier wrote:
> > >
> > >
> > > On 09/07/2015 09:49, Thomas Huth wrote:
> > >> The option for supporting cross-endianness legacy guests in
> > >> the vhost and tun code should only be available on systems
> > >> that support cross-endian guests.
> > >
> > > I'm sure I misunderstand something, but what happens if we use QEMU with
> > > TCG instead of KVM, i.e. a big endian powerpc kernel guest on x86_64
> > > little endian host ?
> >
> > TCG does not yet support irqfd/ioeventfd, so it cannot be used with vhost.
> >
> > Paolo
>
> vhost does not require irqfd anymore. I think ioeventfd actually works
> fine though I didn't try, it would be easy to support.

That's an interesting issue, thanks for pointing this out, Laurent! So
do we now rather want to leave everything as it currently is, in case
somebody wants to use vhost-net with a cross-endian TCG guest one day?

Or do we assume that either
a) TCG is so slow anyway that nobody wants to accelerate it with vhost
or
b) TCG vhost likely won't happen that soon so we hope that everybody
will already be using virtio 1.0 at that point in time (with a fixed
endianness)
?
... then I think we should go on and include this patch.

Thomas

2015-07-13 09:30:11

by Gerg Kurz

[permalink] [raw]
Subject: Re: [PATCH] KVM: Add Kconfig option to signal cross-endian guests

On Thu, 9 Jul 2015 09:49:05 +0200
Thomas Huth <[email protected]> wrote:

> The option for supporting cross-endianness legacy guests in
> the vhost and tun code should only be available on systems
> that support cross-endian guests.
>
> Signed-off-by: Thomas Huth <[email protected]>

Acked-by: Greg Kurz <[email protected]>