Re: [PATCH] KVM: PPC: Book3S HV: return htab entries in big endian

[Alexander Graf <agraf@suse.de>]

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;
>