* [PATCH] KVM: PPC: Book3S HV: return htab entries in big endian
@ 2014-10-02 16:58 Cédric Le Goater
2014-10-02 17:06 ` Alexander Graf
0 siblings, 1 reply; 5+ messages in thread
From: Cédric Le Goater @ 2014-10-02 16:58 UTC (permalink / raw)
To: agraf; +Cc: paulus, kvm-ppc, kvm, Cédric Le Goater
Saving and restoring guests on a KVM little endian host is currently
broken because qemu assumes that htabs are big endian.
This patch modifies kvm_htab_read and kvm_htab_write to make sure
that the endianness expected by qemu is enforced on big and little
endian hosts.
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
Tested on 3.17-rc7 with LE and BE host.
Looking at the code, it is not very clear what we are doing
in terms of endianness. May be this needs more work on both
side, KVM and qemu, to remove the big endian assumptions ?
Thanks,
arch/powerpc/kvm/book3s_64_mmu_hv.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index 79294c4c5015..51dbf772158b 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -1463,6 +1463,9 @@ static ssize_t kvm_htab_read(struct file *file, char __user *buf,
}
if (hdr.n_valid || hdr.n_invalid) {
+ hdr.index = cpu_to_be32(hdr.index);
+ hdr.n_valid = cpu_to_be16(hdr.n_valid);
+ hdr.n_invalid = cpu_to_be16(hdr.n_invalid);
/* write back the header */
if (__copy_to_user(hptr, &hdr, sizeof(hdr)))
return -EFAULT;
@@ -1542,6 +1545,8 @@ static ssize_t kvm_htab_write(struct file *file, const char __user *buf,
err = -EFAULT;
if (__get_user(v, lbuf) || __get_user(r, lbuf + 1))
goto out;
+ v = be64_to_cpu(v);
+ r = be64_to_cpu(r);
err = -EINVAL;
if (!(v & HPTE_V_VALID))
goto out;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] KVM: PPC: Book3S HV: return htab entries in big endian
2014-10-02 16:58 [PATCH] KVM: PPC: Book3S HV: return htab entries in big endian Cédric Le Goater
@ 2014-10-02 17:06 ` Alexander Graf
2014-10-03 12:05 ` Paul Mackerras
0 siblings, 1 reply; 5+ messages in thread
From: Alexander Graf @ 2014-10-02 17:06 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: paulus, kvm-ppc, kvm
On 02.10.14 18:58, Cédric Le Goater wrote:
> Saving and restoring guests on a KVM little endian host is currently
> broken because qemu assumes that htabs are big endian.
>
> This patch modifies kvm_htab_read and kvm_htab_write to make sure
> that the endianness expected by qemu is enforced on big and little
> endian hosts.
>
> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> ---
>
> Tested on 3.17-rc7 with LE and BE host.
>
> Looking at the code, it is not very clear what we are doing
> in terms of endianness. May be this needs more work on both
> side, KVM and qemu, to remove the big endian assumptions ?
>
> Thanks,
>
> arch/powerpc/kvm/book3s_64_mmu_hv.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> index 79294c4c5015..51dbf772158b 100644
> --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> @@ -1463,6 +1463,9 @@ static ssize_t kvm_htab_read(struct file *file, char __user *buf,
> }
>
> if (hdr.n_valid || hdr.n_invalid) {
> + hdr.index = cpu_to_be32(hdr.index);
> + hdr.n_valid = cpu_to_be16(hdr.n_valid);
> + hdr.n_invalid = cpu_to_be16(hdr.n_invalid);
This breaks strict endianness checking. cpu_to_be16 returns a be16 and
takes a u16 as input. Same for the 32bit version.
I think we're best off to keep the user space API native endian. So
really we should only ever have to convert from big to native endian on
read and native to big on write.
With that QEMU should do the "right thing" already, no?
Alex
> /* write back the header */
> if (__copy_to_user(hptr, &hdr, sizeof(hdr)))
> return -EFAULT;
> @@ -1542,6 +1545,8 @@ static ssize_t kvm_htab_write(struct file *file, const char __user *buf,
> err = -EFAULT;
> if (__get_user(v, lbuf) || __get_user(r, lbuf + 1))
> goto out;
> + v = be64_to_cpu(v);
> + r = be64_to_cpu(r);
> err = -EINVAL;
> if (!(v & HPTE_V_VALID))
> goto out;
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] KVM: PPC: Book3S HV: return htab entries in big endian
2014-10-02 17:06 ` Alexander Graf
@ 2014-10-03 12:05 ` Paul Mackerras
2014-10-03 21:05 ` Alexander Graf
0 siblings, 1 reply; 5+ messages in thread
From: Paul Mackerras @ 2014-10-03 12:05 UTC (permalink / raw)
To: Alexander Graf; +Cc: Cédric Le Goater, kvm-ppc, kvm, Alexey Kardashevskiy
On Thu, Oct 02, 2014 at 07:06:40PM +0200, Alexander Graf wrote:
>
> I think we're best off to keep the user space API native endian. So
> really we should only ever have to convert from big to native endian on
> read and native to big on write.
>
> With that QEMU should do the "right thing" already, no?
I believe that when migrating a guest, QEMU just passes the byte
stream it gets from the htab fd along to the destination QEMU with
little or no interpretation, and the destination QEMU writes the byte
stream to the htab fd for the receiving VM. Alexey would be able to
say for sure.
If that's the case, then perhaps it's best to define that byte stream
to contain values of one specific endianness, and for compatibility
that would be big endian.
I assume we would want to be able to migrate a guest from a BE host to
an LE host or vice versa. If not then the question is moot.
Paul.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: PPC: Book3S HV: return htab entries in big endian
2014-10-03 12:05 ` Paul Mackerras
@ 2014-10-03 21:05 ` Alexander Graf
2014-10-04 3:42 ` Alexey Kardashevskiy
0 siblings, 1 reply; 5+ messages in thread
From: Alexander Graf @ 2014-10-03 21:05 UTC (permalink / raw)
To: Paul Mackerras
Cc: Cédric Le Goater, kvm-ppc@vger.kernel.org,
kvm@vger.kernel.org, Alexey Kardashevskiy
> Am 03.10.2014 um 14:05 schrieb Paul Mackerras <paulus@samba.org>:
>
>> On Thu, Oct 02, 2014 at 07:06:40PM +0200, Alexander Graf wrote:
>>
>> I think we're best off to keep the user space API native endian. So
>> really we should only ever have to convert from big to native endian on
>> read and native to big on write.
>>
>> With that QEMU should do the "right thing" already, no?
>
> I believe that when migrating a guest, QEMU just passes the byte
> stream it gets from the htab fd along to the destination QEMU with
> little or no interpretation, and the destination QEMU writes the byte
> stream to the htab fd for the receiving VM. Alexey would be able to
> say for sure.
>
> If that's the case, then perhaps it's best to define that byte stream
> to contain values of one specific endianness, and for compatibility
> that would be big endian.
>
> I assume we would want to be able to migrate a guest from a BE host to
> an LE host or vice versa. If not then the question is moot.
Yes, the migration stream needs to stay host endian agnostic, so it should be BE. Whether QEMU or kvm does the conversion is an implementation detail I don't mind much for either way. But keep in mind that qemu also uses the htab fd for htab introspection that it does for gdbstub emulation or the x monitor command.
Alex
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: PPC: Book3S HV: return htab entries in big endian
2014-10-03 21:05 ` Alexander Graf
@ 2014-10-04 3:42 ` Alexey Kardashevskiy
0 siblings, 0 replies; 5+ messages in thread
From: Alexey Kardashevskiy @ 2014-10-04 3:42 UTC (permalink / raw)
To: Alexander Graf, Paul Mackerras
Cc: Ce'dric Le Goater, kvm-ppc@vger.kernel.org,
kvm@vger.kernel.org
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 10/04/2014 07:05 AM, Alexander Graf wrote:
>
>
>
>> Am 03.10.2014 um 14:05 schrieb Paul Mackerras <paulus@samba.org>:
>>
>>> On Thu, Oct 02, 2014 at 07:06:40PM +0200, Alexander Graf wrote:
>>>
>>> I think we're best off to keep the user space API native endian.
>>> So really we should only ever have to convert from big to native
>>> endian on read and native to big on write.
>>>
>>> With that QEMU should do the "right thing" already, no?
>>
>> I believe that when migrating a guest, QEMU just passes the byte
>> stream it gets from the htab fd along to the destination QEMU with
>> little or no interpretation, and the destination QEMU writes the
>> byte stream to the htab fd for the receiving VM. Alexey would be
>> able to say for sure.
>>
>> If that's the case, then perhaps it's best to define that byte
>> stream to contain values of one specific endianness, and for
>> compatibility that would be big endian.
>>
>> I assume we would want to be able to migrate a guest from a BE host
>> to an LE host or vice versa. If not then the question is moot.
>
> Yes, the migration stream needs to stay host endian agnostic, so it
> should be BE. Whether QEMU or kvm does the conversion is an
> implementation detail I don't mind much for either way.
I find it lot easier if every binary interface has defined endianness,
using native endian just creates problems.
> But keep in mind that qemu also uses the htab fd for htab
> introspection that it does for gdbstub emulation or the x monitor
> command.
- --
Alexey
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQIcBAEBAgAGBQJUL2ygAAoJEIYTPdgrwSC5CcEP/REdOJ6XAqwC4fhUMK9b40FO
ef72ggABoHiG5DZkAjwd0iUmGYvRkDHHH8tbC4BEhnxRlN4OkHRzXA7CgXsZfBea
DwB9eH7IXOycGBNbFcSDfsnxJLdqU8XP1kFtOzV68YtNoGjV1lPLLJ241hGvmuIQ
h+JdD5tRrA2LeV52knxHMQyALYF769fGXCigSsTvY6/VXPoVeNB/tDELzxri/DUM
GntqSZJBGXSD1x4Q1oWopy+QefTP586EZz+sKjMRNLi4gIq8/Z0Do9TaMPq+zmH0
TOk6hukfBVmUXC26nv3Yyo0YZSPAprSd7kiy9TmLnfQPOIdO1Rdh6VaSniyWykyr
G/NDEXJsl1U5OqiWmyJIJssT3QZgYgsxzicwCpyIHe8R9checwONEDAj5gU9RfiG
DJ5WHbGqZ/wQ5G5o/BRYrORX3IfYhQp6sZ2EUXrCNCFBQhnGW9C8RGp2h0P3Ar6J
x9PmpgQsemSIwNdsMvIS2h5MoY5iRtq8D3yWiilAKvP8cEmKBhHTvduyjYDlUtgR
mylsOy1q2muwVX1+3bIM6d7TWtNhB2ewHjIYMaU5LEmJzBN5waVM0ucBKc5g4DVt
BBLhlc0goIGTtjStHrfDAjjZuIwgzre1Ir9QkLDJ2/Tpi2vx3SJpl+9Lp642HmIQ
LLbul3RzhfHn4w+CCRl6
=4+hf
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-10-04 3:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-02 16:58 [PATCH] KVM: PPC: Book3S HV: return htab entries in big endian Cédric Le Goater
2014-10-02 17:06 ` Alexander Graf
2014-10-03 12:05 ` Paul Mackerras
2014-10-03 21:05 ` Alexander Graf
2014-10-04 3:42 ` Alexey Kardashevskiy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox