Re: [PULL] virtio/vhost: cross endian support

Re: [PULL] virtio/vhost: cross endian support

[Linus Torvalds]

On Wed, Jul 1, 2015 at 2:31 AM, Michael S. Tsirkin <mst@redhat.com> 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

Re: [PULL] virtio/vhost: cross endian support

[Linus Torvalds]

On Wed, Jul 1, 2015 at 12:02 PM, Linus Torvalds
<torvalds@linux-foundation.org> 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

Re: [PULL] virtio/vhost: cross endian support

[Michael S. Tsirkin]

On Wed, Jul 01, 2015 at 12:03:59PM -0700, Linus Torvalds wrote:
> On Wed, Jul 1, 2015 at 12:02 PM, Linus Torvalds
> <torvalds@linux-foundation.org> 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

Re: [PULL] virtio/vhost: cross endian support

[Michael S. Tsirkin]

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 <mst@redhat.com> 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

Re: [PULL] virtio/vhost: cross endian support

[Greg Kurz]

On Thu, 2 Jul 2015 08:01:28 +0200
"Michael S. Tsirkin" <mst@redhat.com> 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 <mst@redhat.com> 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,
> 

Re: [PULL] virtio/vhost: cross endian support

[Michael S. Tsirkin]

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" <mst@redhat.com> 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 <mst@redhat.com> 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,
> > 

Re: [PULL] virtio/vhost: cross endian support

[Thomas Huth]

On Thu, 2 Jul 2015 11:32:52 +0200
"Michael S. Tsirkin" <mst@redhat.com> 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" <mst@redhat.com> 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

Re: [PULL] virtio/vhost: cross endian support

[Michael S. Tsirkin]

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" <mst@redhat.com> 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" <mst@redhat.com> 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

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

[Thomas Huth]

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 <thuth@redhat.com>
---
 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

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

[Laurent Vivier]

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 <thuth@redhat.com>
> ---
>  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
> 

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

[Christoffer Dall]

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 <thuth@redhat.com>

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

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

[Cornelia Huck]

On Thu,  9 Jul 2015 09:49:05 +0200
Thomas Huth <thuth@redhat.com> 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 <thuth@redhat.com>
> ---
>  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 <cornelia.huck@de.ibm.com>

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

[Paolo Bonzini]

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 <thuth@redhat.com>
>> ---
>>  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
>>

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

[Michael S. Tsirkin]

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 <thuth@redhat.com>
> >> ---
> >>  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
> >>

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

[Thomas Huth]

On Thu, 9 Jul 2015 16:07:47 +0300
"Michael S. Tsirkin" <mst@redhat.com> 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

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

[Greg Kurz]

On Thu,  9 Jul 2015 09:49:05 +0200
Thomas Huth <thuth@redhat.com> 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 <thuth@redhat.com>

Acked-by: Greg Kurz <gkurz@linux.vnet.ibm.com>

Re: [PULL] virtio/vhost: cross endian support

[Michael S. Tsirkin]

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 <mst@redhat.com> 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

Re: [PULL] virtio/vhost: cross endian support

[Linus Torvalds]

On Fri, Jul 3, 2015 at 12:59 AM, Michael S. Tsirkin <mst@redhat.com> 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