* [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions @ 2018-02-01 18:15 Jose Ricardo Ziviani 2018-02-01 18:15 ` [PATCH v5 1/1] KVM: PPC: Book3S: Add MMIO emulation for VMX instructions Jose Ricardo Ziviani 2018-02-02 0:30 ` [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions Paul Mackerras 0 siblings, 2 replies; 5+ messages in thread From: Jose Ricardo Ziviani @ 2018-02-01 18:15 UTC (permalink / raw) To: linuxppc-dev; +Cc: kvm-ppc, paulus, lvivier v5: - Fixed the mask off of the effective address v4: - Changed KVM_MMIO_REG_VMX to 0xc0 because there are 64 VSX registers v3: - Added Reported-by in the commit message v2: - kvmppc_get_vsr_word_offset() moved back to its original place - EA AND ~0xF, following ISA. - fixed BE/LE cases TESTS: For testing purposes I wrote a small program that performs stvx/lvx using the program's virtual memory and using MMIO. Load/Store into virtual memory is the model I use to check if MMIO results are correct (because only MMIO is emulated by KVM). Results: HOST LE - GUEST BE address: 0x10034850010 0x21436587bbbbaaaa4444555578563412 io_address: 0x3fff89a20000 0x21436587bbbbaaaa4444555578563412 HOST LE - GUEST LE address: 0x10033a20010 0x1234567855554444aaaabbbb87654321 io_address: 0x3fffb5380000 0x1234567855554444aaaabbbb87654321 HOST BE - GUEST BE address: 0x1002c4a0010 0x21436587bbbbaaaa4444555578563412 io_address: 0x3ffface40000 0x21436587bbbbaaaa4444555578563412 HOST BR - GUEST LE address: 0x100225e0010 0x1234567855554444aaaabbbb87654321 io_address: 0x3fff7fcb0000 0x1234567855554444aaaabbbb87654321 This patch implements MMIO emulation for two instructions: lvx and stvx. Jose Ricardo Ziviani (1): KVM: PPC: Book3S: Add MMIO emulation for VMX instructions arch/powerpc/include/asm/kvm_host.h | 2 + arch/powerpc/include/asm/kvm_ppc.h | 4 + arch/powerpc/include/asm/ppc-opcode.h | 6 ++ arch/powerpc/kvm/emulate_loadstore.c | 34 ++++++++ arch/powerpc/kvm/powerpc.c | 153 +++++++++++++++++++++++++++++++++- 5 files changed, 198 insertions(+), 1 deletion(-) -- 2.14.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v5 1/1] KVM: PPC: Book3S: Add MMIO emulation for VMX instructions 2018-02-01 18:15 [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions Jose Ricardo Ziviani @ 2018-02-01 18:15 ` Jose Ricardo Ziviani 2018-02-02 2:55 ` Paul Mackerras 2018-02-02 0:30 ` [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions Paul Mackerras 1 sibling, 1 reply; 5+ messages in thread From: Jose Ricardo Ziviani @ 2018-02-01 18:15 UTC (permalink / raw) To: linuxppc-dev; +Cc: kvm-ppc, paulus, lvivier This patch provides the MMIO load/store vector indexed X-Form emulation. Instructions implemented: lvx: the quadword in storage addressed by the result of EA & 0xffff_ffff_ffff_fff0 is loaded into VRT. stvx: the contents of VRS are stored into the quadword in storage addressed by the result of EA & 0xffff_ffff_ffff_fff0. Reported-by: Gopesh Kumar Chaudhary <gopchaud@in.ibm.com> Reported-by: Balamuruhan S <bala24@linux.vnet.ibm.com> Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- arch/powerpc/include/asm/kvm_host.h | 2 + arch/powerpc/include/asm/kvm_ppc.h | 4 + arch/powerpc/include/asm/ppc-opcode.h | 6 ++ arch/powerpc/kvm/emulate_loadstore.c | 34 ++++++++ arch/powerpc/kvm/powerpc.c | 153 +++++++++++++++++++++++++++++++++- 5 files changed, 198 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 3aa5b577cd60..045acc843e98 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -690,6 +690,7 @@ struct kvm_vcpu_arch { u8 mmio_vsx_offset; u8 mmio_vsx_copy_type; u8 mmio_vsx_tx_sx_enabled; + u8 mmio_vmx_copy_nums; u8 osi_needed; u8 osi_enabled; u8 papr_enabled; @@ -800,6 +801,7 @@ struct kvm_vcpu_arch { #define KVM_MMIO_REG_QPR 0x0040 #define KVM_MMIO_REG_FQPR 0x0060 #define KVM_MMIO_REG_VSX 0x0080 +#define KVM_MMIO_REG_VMX 0x00c0 #define __KVM_HAVE_ARCH_WQP #define __KVM_HAVE_CREATE_DEVICE diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h index 9db18287b5f4..7765a800ddae 100644 --- a/arch/powerpc/include/asm/kvm_ppc.h +++ b/arch/powerpc/include/asm/kvm_ppc.h @@ -81,6 +81,10 @@ extern int kvmppc_handle_loads(struct kvm_run *run, struct kvm_vcpu *vcpu, extern int kvmppc_handle_vsx_load(struct kvm_run *run, struct kvm_vcpu *vcpu, unsigned int rt, unsigned int bytes, int is_default_endian, int mmio_sign_extend); +extern int kvmppc_handle_load128_by2x64(struct kvm_run *run, + struct kvm_vcpu *vcpu, unsigned int rt, int is_default_endian); +extern int kvmppc_handle_store128_by2x64(struct kvm_run *run, + struct kvm_vcpu *vcpu, unsigned int rs, int is_default_endian); extern int kvmppc_handle_store(struct kvm_run *run, struct kvm_vcpu *vcpu, u64 val, unsigned int bytes, int is_default_endian); diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h index ab5c1588b487..f1083bcf449c 100644 --- a/arch/powerpc/include/asm/ppc-opcode.h +++ b/arch/powerpc/include/asm/ppc-opcode.h @@ -156,6 +156,12 @@ #define OP_31_XOP_LFDX 599 #define OP_31_XOP_LFDUX 631 +/* VMX Vector Load Instructions */ +#define OP_31_XOP_LVX 103 + +/* VMX Vector Store Instructions */ +#define OP_31_XOP_STVX 231 + #define OP_LWZ 32 #define OP_STFS 52 #define OP_STFSU 53 diff --git a/arch/powerpc/kvm/emulate_loadstore.c b/arch/powerpc/kvm/emulate_loadstore.c index af833531af31..332b82eafd48 100644 --- a/arch/powerpc/kvm/emulate_loadstore.c +++ b/arch/powerpc/kvm/emulate_loadstore.c @@ -58,6 +58,18 @@ static bool kvmppc_check_vsx_disabled(struct kvm_vcpu *vcpu) } #endif /* CONFIG_VSX */ +#ifdef CONFIG_ALTIVEC +static bool kvmppc_check_altivec_disabled(struct kvm_vcpu *vcpu) +{ + if (!(kvmppc_get_msr(vcpu) & MSR_VEC)) { + kvmppc_core_queue_vec_unavail(vcpu); + return true; + } + + return false; +} +#endif /* CONFIG_ALTIVEC */ + /* * XXX to do: * lfiwax, lfiwzx @@ -98,6 +110,7 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu) vcpu->arch.mmio_vsx_copy_type = KVMPPC_VSX_COPY_NONE; vcpu->arch.mmio_sp64_extend = 0; vcpu->arch.mmio_sign_extend = 0; + vcpu->arch.mmio_vmx_copy_nums = 0; switch (get_op(inst)) { case 31: @@ -459,6 +472,27 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu) rs, 4, 1); break; #endif /* CONFIG_VSX */ + +#ifdef CONFIG_ALTIVEC + case OP_31_XOP_LVX: + if (kvmppc_check_altivec_disabled(vcpu)) + return EMULATE_DONE; + vcpu->arch.vaddr_accessed &= ~0xFULL; + vcpu->arch.mmio_vmx_copy_nums = 2; + emulated = kvmppc_handle_load128_by2x64(run, vcpu, + KVM_MMIO_REG_VMX|rt, 1); + break; + + case OP_31_XOP_STVX: + if (kvmppc_check_altivec_disabled(vcpu)) + return EMULATE_DONE; + vcpu->arch.vaddr_accessed &= ~0xFULL; + vcpu->arch.mmio_vmx_copy_nums = 2; + emulated = kvmppc_handle_store128_by2x64(run, vcpu, + rs, 1); + break; +#endif /* CONFIG_ALTIVEC */ + default: emulated = EMULATE_FAIL; break; diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 1915e86cef6f..a19f42120b38 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -832,7 +832,7 @@ void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *cons, kvm->arch.kvm_ops->irq_bypass_del_producer(cons, prod); } -#ifdef CONFIG_VSX +#ifdef CONFIG_ALTIVEC static inline int kvmppc_get_vsr_dword_offset(int index) { int offset; @@ -848,7 +848,9 @@ static inline int kvmppc_get_vsr_dword_offset(int index) return offset; } +#endif /* CONFIG_ALTIVEC */ +#ifdef CONFIG_VSX static inline int kvmppc_get_vsr_word_offset(int index) { int offset; @@ -925,6 +927,31 @@ static inline void kvmppc_set_vsr_word(struct kvm_vcpu *vcpu, } #endif /* CONFIG_VSX */ +#ifdef CONFIG_ALTIVEC +static inline void kvmppc_set_vmx_dword(struct kvm_vcpu *vcpu, + u64 gpr) +{ + int index = vcpu->arch.io_gpr & KVM_MMIO_REG_MASK; + u32 hi, lo; + +#ifdef __BIG_ENDIAN + hi = gpr >> 32; + lo = gpr & 0xffffffff; +#else + lo = gpr >> 32; + hi = gpr & 0xffffffff; +#endif + + if (vcpu->arch.mmio_vmx_copy_nums == 1) { + VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(2)] = lo; + VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(3)] = hi; + } else if (vcpu->arch.mmio_vmx_copy_nums == 2) { + VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(0)] = lo; + VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(1)] = hi; + } +} +#endif /* CONFIG_ALTIVEC */ + #ifdef CONFIG_PPC_FPU static inline u64 sp_to_dp(u32 fprs) { @@ -1027,6 +1054,11 @@ static void kvmppc_complete_mmio_load(struct kvm_vcpu *vcpu, KVMPPC_VSX_COPY_DWORD_LOAD_DUMP) kvmppc_set_vsr_dword_dump(vcpu, gpr); break; +#endif +#ifdef CONFIG_ALTIVEC + case KVM_MMIO_REG_VMX: + kvmppc_set_vmx_dword(vcpu, gpr); + break; #endif default: BUG(); @@ -1307,6 +1339,113 @@ static int kvmppc_emulate_mmio_vsx_loadstore(struct kvm_vcpu *vcpu, } #endif /* CONFIG_VSX */ +#ifdef CONFIG_ALTIVEC +/* handle quadword load access in two halves */ +int kvmppc_handle_load128_by2x64(struct kvm_run *run, struct kvm_vcpu *vcpu, + unsigned int rt, int is_default_endian) +{ + enum emulation_result emulated; + + while (vcpu->arch.mmio_vmx_copy_nums) { + emulated = __kvmppc_handle_load(run, vcpu, rt, 8, + is_default_endian, 0); + + if (emulated != EMULATE_DONE) + break; + + vcpu->arch.paddr_accessed += run->mmio.len; + vcpu->arch.mmio_vmx_copy_nums--; + } + + return emulated; +} + +static inline int kvmppc_get_vmx_data(struct kvm_vcpu *vcpu, int rs, u64 *val) +{ + vector128 vrs = VCPU_VSX_VR(vcpu, rs); + + if (vcpu->arch.mmio_vmx_copy_nums == 1) { +#ifdef __BIG_ENDIAN + *val = vrs.u[kvmppc_get_vsr_word_offset(3)]; + *val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(2)]; +#else + *val = vrs.u[kvmppc_get_vsr_word_offset(2)]; + *val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(3)]; +#endif + return 0; + } else if (vcpu->arch.mmio_vmx_copy_nums == 2) { +#ifdef __BIG_ENDIAN + *val = vrs.u[kvmppc_get_vsr_word_offset(1)]; + *val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(0)]; +#else + *val = vrs.u[kvmppc_get_vsr_word_offset(0)]; + *val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(1)]; +#endif + return 0; + } + return -1; +} + +/* handle quadword store in two halves */ +int kvmppc_handle_store128_by2x64(struct kvm_run *run, struct kvm_vcpu *vcpu, + unsigned int rs, int is_default_endian) +{ + u64 val = 0; + enum emulation_result emulated = EMULATE_DONE; + + vcpu->arch.io_gpr = rs; + + while (vcpu->arch.mmio_vmx_copy_nums) { + if (kvmppc_get_vmx_data(vcpu, rs, &val) == -1) + return EMULATE_FAIL; + + emulated = kvmppc_handle_store(run, vcpu, val, 8, + is_default_endian); + if (emulated != EMULATE_DONE) + break; + + vcpu->arch.paddr_accessed += run->mmio.len; + vcpu->arch.mmio_vmx_copy_nums--; + } + + return emulated; +} + +static int kvmppc_emulate_mmio_vmx_loadstore(struct kvm_vcpu *vcpu, + struct kvm_run *run) +{ + enum emulation_result emulated = EMULATE_FAIL; + int r; + + vcpu->arch.paddr_accessed += run->mmio.len; + + if (!vcpu->mmio_is_write) { + emulated = kvmppc_handle_load128_by2x64(run, vcpu, + vcpu->arch.io_gpr, 1); + } else { + emulated = kvmppc_handle_store128_by2x64(run, vcpu, + vcpu->arch.io_gpr, 1); + } + + switch (emulated) { + case EMULATE_DO_MMIO: + run->exit_reason = KVM_EXIT_MMIO; + r = RESUME_HOST; + break; + case EMULATE_FAIL: + pr_info("KVM: MMIO emulation failed (VMX repeat)\n"); + run->exit_reason = KVM_EXIT_INTERNAL_ERROR; + run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION; + r = RESUME_HOST; + break; + default: + r = RESUME_GUEST; + break; + } + return r; +} +#endif /* CONFIG_ALTIVEC */ + int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg) { int r = 0; @@ -1425,6 +1564,18 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run) return r; } } +#endif +#ifdef CONFIG_ALTIVEC + if (vcpu->arch.mmio_vmx_copy_nums > 0) + vcpu->arch.mmio_vmx_copy_nums--; + + if (vcpu->arch.mmio_vmx_copy_nums > 0) { + r = kvmppc_emulate_mmio_vmx_loadstore(vcpu, run); + if (r == RESUME_HOST) { + vcpu->mmio_needed = 1; + return r; + } + } #endif } else if (vcpu->arch.osi_needed) { u64 *gprs = run->osi.gprs; -- 2.14.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v5 1/1] KVM: PPC: Book3S: Add MMIO emulation for VMX instructions 2018-02-01 18:15 ` [PATCH v5 1/1] KVM: PPC: Book3S: Add MMIO emulation for VMX instructions Jose Ricardo Ziviani @ 2018-02-02 2:55 ` Paul Mackerras 0 siblings, 0 replies; 5+ messages in thread From: Paul Mackerras @ 2018-02-02 2:55 UTC (permalink / raw) To: Jose Ricardo Ziviani; +Cc: linuxppc-dev, kvm-ppc, lvivier On Thu, Feb 01, 2018 at 04:15:39PM -0200, Jose Ricardo Ziviani wrote: > This patch provides the MMIO load/store vector indexed > X-Form emulation. > > Instructions implemented: > lvx: the quadword in storage addressed by the result of EA & > 0xffff_ffff_ffff_fff0 is loaded into VRT. > > stvx: the contents of VRS are stored into the quadword in storage > addressed by the result of EA & 0xffff_ffff_ffff_fff0. > > Reported-by: Gopesh Kumar Chaudhary <gopchaud@in.ibm.com> > Reported-by: Balamuruhan S <bala24@linux.vnet.ibm.com> > Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> > --- > arch/powerpc/include/asm/kvm_host.h | 2 + > arch/powerpc/include/asm/kvm_ppc.h | 4 + > arch/powerpc/include/asm/ppc-opcode.h | 6 ++ > arch/powerpc/kvm/emulate_loadstore.c | 34 ++++++++ > arch/powerpc/kvm/powerpc.c | 153 +++++++++++++++++++++++++++++++++- > 5 files changed, 198 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h > index 3aa5b577cd60..045acc843e98 100644 > --- a/arch/powerpc/include/asm/kvm_host.h > +++ b/arch/powerpc/include/asm/kvm_host.h > @@ -690,6 +690,7 @@ struct kvm_vcpu_arch { > u8 mmio_vsx_offset; > u8 mmio_vsx_copy_type; > u8 mmio_vsx_tx_sx_enabled; > + u8 mmio_vmx_copy_nums; > u8 osi_needed; > u8 osi_enabled; > u8 papr_enabled; > @@ -800,6 +801,7 @@ struct kvm_vcpu_arch { > #define KVM_MMIO_REG_QPR 0x0040 > #define KVM_MMIO_REG_FQPR 0x0060 > #define KVM_MMIO_REG_VSX 0x0080 > +#define KVM_MMIO_REG_VMX 0x00c0 > > #define __KVM_HAVE_ARCH_WQP > #define __KVM_HAVE_CREATE_DEVICE > diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h > index 9db18287b5f4..7765a800ddae 100644 > --- a/arch/powerpc/include/asm/kvm_ppc.h > +++ b/arch/powerpc/include/asm/kvm_ppc.h > @@ -81,6 +81,10 @@ extern int kvmppc_handle_loads(struct kvm_run *run, struct kvm_vcpu *vcpu, > extern int kvmppc_handle_vsx_load(struct kvm_run *run, struct kvm_vcpu *vcpu, > unsigned int rt, unsigned int bytes, > int is_default_endian, int mmio_sign_extend); > +extern int kvmppc_handle_load128_by2x64(struct kvm_run *run, > + struct kvm_vcpu *vcpu, unsigned int rt, int is_default_endian); > +extern int kvmppc_handle_store128_by2x64(struct kvm_run *run, > + struct kvm_vcpu *vcpu, unsigned int rs, int is_default_endian); > extern int kvmppc_handle_store(struct kvm_run *run, struct kvm_vcpu *vcpu, > u64 val, unsigned int bytes, > int is_default_endian); > diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h > index ab5c1588b487..f1083bcf449c 100644 > --- a/arch/powerpc/include/asm/ppc-opcode.h > +++ b/arch/powerpc/include/asm/ppc-opcode.h > @@ -156,6 +156,12 @@ > #define OP_31_XOP_LFDX 599 > #define OP_31_XOP_LFDUX 631 > > +/* VMX Vector Load Instructions */ > +#define OP_31_XOP_LVX 103 > + > +/* VMX Vector Store Instructions */ > +#define OP_31_XOP_STVX 231 > + > #define OP_LWZ 32 > #define OP_STFS 52 > #define OP_STFSU 53 > diff --git a/arch/powerpc/kvm/emulate_loadstore.c b/arch/powerpc/kvm/emulate_loadstore.c > index af833531af31..332b82eafd48 100644 > --- a/arch/powerpc/kvm/emulate_loadstore.c > +++ b/arch/powerpc/kvm/emulate_loadstore.c > @@ -58,6 +58,18 @@ static bool kvmppc_check_vsx_disabled(struct kvm_vcpu *vcpu) > } > #endif /* CONFIG_VSX */ > > +#ifdef CONFIG_ALTIVEC > +static bool kvmppc_check_altivec_disabled(struct kvm_vcpu *vcpu) > +{ > + if (!(kvmppc_get_msr(vcpu) & MSR_VEC)) { > + kvmppc_core_queue_vec_unavail(vcpu); > + return true; > + } > + > + return false; > +} > +#endif /* CONFIG_ALTIVEC */ > + > /* > * XXX to do: > * lfiwax, lfiwzx > @@ -98,6 +110,7 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu) > vcpu->arch.mmio_vsx_copy_type = KVMPPC_VSX_COPY_NONE; > vcpu->arch.mmio_sp64_extend = 0; > vcpu->arch.mmio_sign_extend = 0; > + vcpu->arch.mmio_vmx_copy_nums = 0; > > switch (get_op(inst)) { > case 31: > @@ -459,6 +472,27 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu) > rs, 4, 1); > break; > #endif /* CONFIG_VSX */ > + > +#ifdef CONFIG_ALTIVEC > + case OP_31_XOP_LVX: > + if (kvmppc_check_altivec_disabled(vcpu)) > + return EMULATE_DONE; > + vcpu->arch.vaddr_accessed &= ~0xFULL; > + vcpu->arch.mmio_vmx_copy_nums = 2; > + emulated = kvmppc_handle_load128_by2x64(run, vcpu, > + KVM_MMIO_REG_VMX|rt, 1); > + break; > + > + case OP_31_XOP_STVX: > + if (kvmppc_check_altivec_disabled(vcpu)) > + return EMULATE_DONE; > + vcpu->arch.vaddr_accessed &= ~0xFULL; > + vcpu->arch.mmio_vmx_copy_nums = 2; > + emulated = kvmppc_handle_store128_by2x64(run, vcpu, > + rs, 1); > + break; > +#endif /* CONFIG_ALTIVEC */ > + > default: > emulated = EMULATE_FAIL; > break; > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 1915e86cef6f..a19f42120b38 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -832,7 +832,7 @@ void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *cons, > kvm->arch.kvm_ops->irq_bypass_del_producer(cons, prod); > } > > -#ifdef CONFIG_VSX > +#ifdef CONFIG_ALTIVEC > static inline int kvmppc_get_vsr_dword_offset(int index) > { > int offset; > @@ -848,7 +848,9 @@ static inline int kvmppc_get_vsr_dword_offset(int index) > > return offset; > } > +#endif /* CONFIG_ALTIVEC */ > > +#ifdef CONFIG_VSX > static inline int kvmppc_get_vsr_word_offset(int index) You make the dword version available with ALTIVEC && ~VSX, but in fact it's the word version that you use below. However, I don't think we actually want either of them (see below). > { > int offset; > @@ -925,6 +927,31 @@ static inline void kvmppc_set_vsr_word(struct kvm_vcpu *vcpu, > } > #endif /* CONFIG_VSX */ > > +#ifdef CONFIG_ALTIVEC > +static inline void kvmppc_set_vmx_dword(struct kvm_vcpu *vcpu, > + u64 gpr) > +{ > + int index = vcpu->arch.io_gpr & KVM_MMIO_REG_MASK; > + u32 hi, lo; > + > +#ifdef __BIG_ENDIAN > + hi = gpr >> 32; > + lo = gpr & 0xffffffff; > +#else > + lo = gpr >> 32; > + hi = gpr & 0xffffffff; > +#endif > + > + if (vcpu->arch.mmio_vmx_copy_nums == 1) { > + VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(2)] = lo; > + VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(3)] = hi; > + } else if (vcpu->arch.mmio_vmx_copy_nums == 2) { > + VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(0)] = lo; > + VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(1)] = hi; > + } Since what we're doing is a 16-byte load, the main thing we have to do here in handling a cross-endian situation is to swap the two 8-byte halves. The byte-swapping within each 8-byte half has already been handled more generically. I suggest the following code. It is simpler and passes my test case. static inline void kvmppc_set_vmx_dword(struct kvm_vcpu *vcpu, u64 gpr) { int index = vcpu->arch.io_gpr & KVM_MMIO_REG_MASK; u32 hi, lo; u32 di; #ifdef __BIG_ENDIAN hi = gpr >> 32; lo = gpr & 0xffffffff; #else lo = gpr >> 32; hi = gpr & 0xffffffff; #endif di = 2 - vcpu->arch.mmio_vmx_copy_nums; /* doubleword index */ if (di > 1) return; if (vcpu->arch.mmio_host_swabbed) di = 1 - di; VCPU_VSX_VR(vcpu, index).u[di * 2] = hi; VCPU_VSX_VR(vcpu, index).u[di * 2 + 1] = lo; } > +static inline int kvmppc_get_vmx_data(struct kvm_vcpu *vcpu, int rs, u64 *val) > +{ > + vector128 vrs = VCPU_VSX_VR(vcpu, rs); > + > + if (vcpu->arch.mmio_vmx_copy_nums == 1) { > +#ifdef __BIG_ENDIAN > + *val = vrs.u[kvmppc_get_vsr_word_offset(3)]; > + *val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(2)]; > +#else > + *val = vrs.u[kvmppc_get_vsr_word_offset(2)]; > + *val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(3)]; > +#endif > + return 0; > + } else if (vcpu->arch.mmio_vmx_copy_nums == 2) { > +#ifdef __BIG_ENDIAN > + *val = vrs.u[kvmppc_get_vsr_word_offset(1)]; > + *val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(0)]; > +#else > + *val = vrs.u[kvmppc_get_vsr_word_offset(0)]; > + *val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(1)]; > +#endif > + return 0; > + } > + return -1; Once again the main thing is to swap the two halves. My suggested code is: static inline int kvmppc_get_vmx_data(struct kvm_vcpu *vcpu, int rs, u64 *val) { vector128 vrs = VCPU_VSX_VR(vcpu, rs); u32 di; u64 w0, w1; di = 2 - vcpu->arch.mmio_vmx_copy_nums; /* doubleword index */ if (di > 1) return -1; if (vcpu->arch.mmio_host_swabbed) di = 1 - di; w0 = vrs.u[di * 2]; w1 = vrs.u[di * 2 + 1]; #ifdef __BIG_ENDIAN *val = (w0 << 32) | w1; #else *val = (w1 << 32) | w0; #endif return 0; } Paul. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions 2018-02-01 18:15 [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions Jose Ricardo Ziviani 2018-02-01 18:15 ` [PATCH v5 1/1] KVM: PPC: Book3S: Add MMIO emulation for VMX instructions Jose Ricardo Ziviani @ 2018-02-02 0:30 ` Paul Mackerras 2018-02-03 0:02 ` joserz 1 sibling, 1 reply; 5+ messages in thread From: Paul Mackerras @ 2018-02-02 0:30 UTC (permalink / raw) To: Jose Ricardo Ziviani; +Cc: linuxppc-dev, kvm-ppc, lvivier On Thu, Feb 01, 2018 at 04:15:38PM -0200, Jose Ricardo Ziviani wrote: > v5: > - Fixed the mask off of the effective address > > v4: > - Changed KVM_MMIO_REG_VMX to 0xc0 because there are 64 VSX registers > > v3: > - Added Reported-by in the commit message > > v2: > - kvmppc_get_vsr_word_offset() moved back to its original place > - EA AND ~0xF, following ISA. > - fixed BE/LE cases > > TESTS: > > For testing purposes I wrote a small program that performs stvx/lvx using the > program's virtual memory and using MMIO. Load/Store into virtual memory is the > model I use to check if MMIO results are correct (because only MMIO is emulated > by KVM). I'd be interested to see your test program because in my testing it's still not right, unfortunately. Interestingly, it is right for the BE guest on LE host case. However, with a LE guest on a LE host the two halves are swapped, both for lvx and stvx: error in lvx at byte 0 was: -> 62 69 70 77 7e 85 8c 93 2a 31 38 3f 46 4d 54 5b ref: -> 2a 31 38 3f 46 4d 54 5b 62 69 70 77 7e 85 8c 93 error in stvx at byte 0 was: -> 49 50 57 5e 65 6c 73 7a 11 18 1f 26 2d 34 3b 42 ref: -> 11 18 1f 26 2d 34 3b 42 49 50 57 5e 65 6c 73 7a The byte order within each 8-byte half is correct but the two halves are swapped. ("was" is what was in memory and "ref" is the correct value. For lvx it does lvx from emulated MMIO and stvx to ordinary memory, and for stvx it does lvx from ordinary memory and stvx to emulated MMIO. In both cases the checking is done with a byte by byte comparison.) Paul. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions 2018-02-02 0:30 ` [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions Paul Mackerras @ 2018-02-03 0:02 ` joserz 0 siblings, 0 replies; 5+ messages in thread From: joserz @ 2018-02-03 0:02 UTC (permalink / raw) To: Paul Mackerras; +Cc: lvivier, linuxppc-dev, kvm-ppc On Fri, Feb 02, 2018 at 11:30:18AM +1100, Paul Mackerras wrote: > On Thu, Feb 01, 2018 at 04:15:38PM -0200, Jose Ricardo Ziviani wrote: > > v5: > > - Fixed the mask off of the effective address > > > > v4: > > - Changed KVM_MMIO_REG_VMX to 0xc0 because there are 64 VSX registers > > > > v3: > > - Added Reported-by in the commit message > > > > v2: > > - kvmppc_get_vsr_word_offset() moved back to its original place > > - EA AND ~0xF, following ISA. > > - fixed BE/LE cases > > > > TESTS: > > > > For testing purposes I wrote a small program that performs stvx/lvx using the > > program's virtual memory and using MMIO. Load/Store into virtual memory is the > > model I use to check if MMIO results are correct (because only MMIO is emulated > > by KVM). > > I'd be interested to see your test program because in my testing it's > still not right, unfortunately. Interestingly, it is right for the BE > guest on LE host case. However, with a LE guest on a LE host the two > halves are swapped, both for lvx and stvx: Absolutely, here it's: https://gist.github.com/jrziviani/a65e71c5d661bffa8afcd6710fedd520 It basically maps an IO region and also allocates some memory from the program's address space. Then I store to/load from both addresses and compare the results. Because only the mmio load/store are emulated, I use the regular load/store as a model. > > error in lvx at byte 0 > was: -> 62 69 70 77 7e 85 8c 93 2a 31 38 3f 46 4d 54 5b > ref: -> 2a 31 38 3f 46 4d 54 5b 62 69 70 77 7e 85 8c 93 > error in stvx at byte 0 > was: -> 49 50 57 5e 65 6c 73 7a 11 18 1f 26 2d 34 3b 42 > ref: -> 11 18 1f 26 2d 34 3b 42 49 50 57 5e 65 6c 73 7a > > The byte order within each 8-byte half is correct but the two halves > are swapped. ("was" is what was in memory and "ref" is the correct > value. For lvx it does lvx from emulated MMIO and stvx to ordinary > memory, and for stvx it does lvx from ordinary memory and stvx to > emulated MMIO. In both cases the checking is done with a byte by byte > comparison.) The funny thing is that I still see it right in both cases, so I believe that my test case is incorrect. Example (host LE, guest LE): ====> VR0 after lvx (gdb) p $vr0 {uint128 = 0x1234567855554444aaaabbbb87654321, v4_float = { -1.72477726e-34, -3.03283305e-13, 1.46555735e+13, 5.69045661e-28}, v4_int32 = {-2023406815, -1431651397, 1431651396, 305419896}, v8_int16 = { 17185, -30875, -17477, -21846, 17476, 21845, 22136, 4660}, v16_int8 = {33, 67, 101, -121, -69, -69, -86, -86, 68, 68, 85, 85, 120, 86, 52, 18}} address: 0x10030010 0x1234567855554444aaaabbbb87654321 ====> VR0 after lvx from MMIO (gdb) p $vr0 $3 = {uint128 = 0x1234567855554444aaaabbbb87654321, v4_float = { -1.72477726e-34, -3.03283305e-13, 1.46555735e+13, 5.69045661e-28}, v4_int32 = {-2023406815, -1431651397, 1431651396, 305419896}, v8_int16 = { 17185, -30875, -17477, -21846, 17476, 21845, 22136, 4660}, v16_int8 = {33, 67, 101, -121, -69, -69, -86, -86, 68, 68, 85, 85, 120, 86, 52, 18}} io_address: 0x3fffb7f70000 0x1234567855554444aaaabbbb87654321 I only see it wrong when I mess with copy order: if (vcpu->arch.mmio_vmx_copy_nums == /*from 1 to */ 2) { VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(2)] = lo; VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(3)] = hi; } else if (vcpu->arch.mmio_vmx_copy_nums == /*from 2 to */ 1) { VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(0)] = lo; VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(1)] = hi; } then I get: address: 0x1003b530010 0x1234567855554444aaaabbbb87654321 io_address: 0x3fff811a0000 0xaaaabbbb876543211234567855554444 Anyway, your suggestion works great and it's a way more elegant/easy to understand. I'll send the next version with it. Thank you very much for your patience and hints that helped me to understand KVM better. :-) > > Paul. > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-02-03 0:02 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-02-01 18:15 [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions Jose Ricardo Ziviani 2018-02-01 18:15 ` [PATCH v5 1/1] KVM: PPC: Book3S: Add MMIO emulation for VMX instructions Jose Ricardo Ziviani 2018-02-02 2:55 ` Paul Mackerras 2018-02-02 0:30 ` [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions Paul Mackerras 2018-02-03 0:02 ` joserz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).