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

Paolo Bonzini
2013-11-11T11:05:07Z

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__ */
>