Re: [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest

[Paolo Bonzini <pbonzini@redhat.com>]

Il 08/11/2013 11:07, Marc Zyngier ha scritto:
> Do the necessary byteswap when host and guest have different
> views of the universe. Actually, the only case we need to take
> care of is when the guest is BE. All the other cases are naturally
> handled.
> 
> Also be careful about endianness when the data is being memcopy-ed
> from/to the run buffer.
> 
> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/include/asm/kvm_emulate.h   | 41 +++++++++++++++++
>  arch/arm/kvm/mmio.c                  | 86 +++++++++++++++++++++++++++++++-----
>  arch/arm64/include/asm/kvm_emulate.h | 48 ++++++++++++++++++++
>  3 files changed, 164 insertions(+), 11 deletions(-)

Christoffer and Marc, please coordinate so that arm+arm64 patch go only
through one person.  The conflict between your two pull requests was not
really necessary.

I'm pulling both anyway.

Paolo

> diff --git a/arch/arm/include/asm/kvm_emulate.h b/arch/arm/include/asm/kvm_emulate.h
> index a464e8d..8a6be05 100644
> --- a/arch/arm/include/asm/kvm_emulate.h
> +++ b/arch/arm/include/asm/kvm_emulate.h
> @@ -157,4 +157,45 @@ static inline u32 kvm_vcpu_hvc_get_imm(struct kvm_vcpu *vcpu)
>  	return kvm_vcpu_get_hsr(vcpu) & HSR_HVC_IMM_MASK;
>  }
>  
> +static inline bool kvm_vcpu_is_be(struct kvm_vcpu *vcpu)
> +{
> +	return !!(*vcpu_cpsr(vcpu) & PSR_E_BIT);
> +}
> +
> +static inline unsigned long vcpu_data_guest_to_host(struct kvm_vcpu *vcpu,
> +						    unsigned long data,
> +						    unsigned int len)
> +{
> +	if (kvm_vcpu_is_be(vcpu)) {
> +		switch (len) {
> +		case 1:
> +			return data & 0xff;
> +		case 2:
> +			return be16_to_cpu(data & 0xffff);
> +		default:
> +			return be32_to_cpu(data);
> +		}
> +	}
> +
> +	return data;		/* Leave LE untouched */
> +}
> +
> +static inline unsigned long vcpu_data_host_to_guest(struct kvm_vcpu *vcpu,
> +						    unsigned long data,
> +						    unsigned int len)
> +{
> +	if (kvm_vcpu_is_be(vcpu)) {
> +		switch (len) {
> +		case 1:
> +			return data & 0xff;
> +		case 2:
> +			return cpu_to_be16(data & 0xffff);
> +		default:
> +			return cpu_to_be32(data);
> +		}
> +	}
> +
> +	return data;		/* Leave LE untouched */
> +}
> +
>  #endif /* __ARM_KVM_EMULATE_H__ */
> diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c
> index 0c25d94..4cb5a93 100644
> --- a/arch/arm/kvm/mmio.c
> +++ b/arch/arm/kvm/mmio.c
> @@ -23,6 +23,68 @@
>  
>  #include "trace.h"
>  
> +static void mmio_write_buf(char *buf, unsigned int len, unsigned long data)
> +{
> +	void *datap = NULL;
> +	union {
> +		u8	byte;
> +		u16	hword;
> +		u32	word;
> +		u64	dword;
> +	} tmp;
> +
> +	switch (len) {
> +	case 1:
> +		tmp.byte	= data;
> +		datap		= &tmp.byte;
> +		break;
> +	case 2:
> +		tmp.hword	= data;
> +		datap		= &tmp.hword;
> +		break;
> +	case 4:
> +		tmp.word	= data;
> +		datap		= &tmp.word;
> +		break;
> +	case 8:
> +		tmp.dword	= data;
> +		datap		= &tmp.dword;
> +		break;
> +	}
> +
> +	memcpy(buf, datap, len);
> +}
> +
> +static unsigned long mmio_read_buf(char *buf, unsigned int len)
> +{
> +	unsigned long data = 0;
> +	union {
> +		u16	hword;
> +		u32	word;
> +		u64	dword;
> +	} tmp;
> +
> +	switch (len) {
> +	case 1:
> +		data = buf[0];
> +		break;
> +	case 2:
> +		memcpy(&tmp.hword, buf, len);
> +		data = tmp.hword;
> +		break;
> +	case 4:
> +		memcpy(&tmp.word, buf, len);
> +		data = tmp.word;
> +		break;
> +	case 8:
> +		memcpy(&tmp.dword, buf, len);
> +		data = tmp.dword;
> +		break;
> +	}
> +
> +	return data;
> +}
> +
>  /**
>   * kvm_handle_mmio_return -- Handle MMIO loads after user space emulation
>   * @vcpu: The VCPU pointer
> @@ -33,28 +95,27 @@
>   */
>  int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  {
> -	unsigned long *dest;
> +	unsigned long data;
>  	unsigned int len;
>  	int mask;
>  
>  	if (!run->mmio.is_write) {
> -		dest = vcpu_reg(vcpu, vcpu->arch.mmio_decode.rt);
> -		*dest = 0;
> -
>  		len = run->mmio.len;
>  		if (len > sizeof(unsigned long))
>  			return -EINVAL;
>  
> -		memcpy(dest, run->mmio.data, len);
> -
> -		trace_kvm_mmio(KVM_TRACE_MMIO_READ, len, run->mmio.phys_addr,
> -				*((u64 *)run->mmio.data));
> +		data = mmio_read_buf(run->mmio.data, len);
>  
>  		if (vcpu->arch.mmio_decode.sign_extend &&
>  		    len < sizeof(unsigned long)) {
>  			mask = 1U << ((len * 8) - 1);
> -			*dest = (*dest ^ mask) - mask;
> +			data = (data ^ mask) - mask;
>  		}
> +
> +		trace_kvm_mmio(KVM_TRACE_MMIO_READ, len, run->mmio.phys_addr,
> +			       data);
> +		data = vcpu_data_host_to_guest(vcpu, data, len);
> +		*vcpu_reg(vcpu, vcpu->arch.mmio_decode.rt) = data;
>  	}
>  
>  	return 0;
> @@ -105,6 +166,7 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
>  		 phys_addr_t fault_ipa)
>  {
>  	struct kvm_exit_mmio mmio;
> +	unsigned long data;
>  	unsigned long rt;
>  	int ret;
>  
> @@ -125,13 +187,15 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
>  	}
>  
>  	rt = vcpu->arch.mmio_decode.rt;
> +	data = vcpu_data_guest_to_host(vcpu, *vcpu_reg(vcpu, rt), mmio.len);
> +
>  	trace_kvm_mmio((mmio.is_write) ? KVM_TRACE_MMIO_WRITE :
>  					 KVM_TRACE_MMIO_READ_UNSATISFIED,
>  			mmio.len, fault_ipa,
> -			(mmio.is_write) ? *vcpu_reg(vcpu, rt) : 0);
> +			(mmio.is_write) ? data : 0);
>  
>  	if (mmio.is_write)
> -		memcpy(mmio.data, vcpu_reg(vcpu, rt), mmio.len);
> +		mmio_write_buf(mmio.data, mmio.len, data);
>  
>  	if (vgic_handle_mmio(vcpu, run, &mmio))
>  		return 1;
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index eec0738..b016577 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -177,4 +177,52 @@ static inline u8 kvm_vcpu_trap_get_fault(const struct kvm_vcpu *vcpu)
>  	return kvm_vcpu_get_hsr(vcpu) & ESR_EL2_FSC_TYPE;
>  }
>  
> +static inline bool kvm_vcpu_is_be(struct kvm_vcpu *vcpu)
> +{
> +	if (vcpu_mode_is_32bit(vcpu))
> +		return !!(*vcpu_cpsr(vcpu) & COMPAT_PSR_E_BIT);
> +
> +	return !!(vcpu_sys_reg(vcpu, SCTLR_EL1) & (1 << 25));
> +}
> +
> +static inline unsigned long vcpu_data_guest_to_host(struct kvm_vcpu *vcpu,
> +						    unsigned long data,
> +						    unsigned int len)
> +{
> +	if (kvm_vcpu_is_be(vcpu)) {
> +		switch (len) {
> +		case 1:
> +			return data & 0xff;
> +		case 2:
> +			return be16_to_cpu(data & 0xffff);
> +		case 4:
> +			return be32_to_cpu(data & 0xffffffff);
> +		default:
> +			return be64_to_cpu(data);
> +		}
> +	}
> +
> +	return data;		/* Leave LE untouched */
> +}
> +
> +static inline unsigned long vcpu_data_host_to_guest(struct kvm_vcpu *vcpu,
> +						    unsigned long data,
> +						    unsigned int len)
> +{
> +	if (kvm_vcpu_is_be(vcpu)) {
> +		switch (len) {
> +		case 1:
> +			return data & 0xff;
> +		case 2:
> +			return cpu_to_be16(data & 0xffff);
> +		case 4:
> +			return cpu_to_be32(data & 0xffffffff);
> +		default:
> +			return cpu_to_be64(data);
> +		}
> +	}
> +
> +	return data;		/* Leave LE untouched */
> +}
> +
>  #endif /* __ARM64_KVM_EMULATE_H__ */
>