Message ID | 1524657284-16706-4-git-send-email-wei.guo.simon@gmail.com (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
Series | KVM: PPC: reconstruct mmio emulation with analyse_instr() | expand |
On Wed, Apr 25, 2018 at 07:54:36PM +0800, wei.guo.simon@gmail.com wrote: > From: Simon Guo <wei.guo.simon@gmail.com> > > When KVM emulates VMX store, it will invoke kvmppc_get_vmx_data() to > retrieve VMX reg val. kvmppc_get_vmx_data() will check mmio_host_swabbed > to decide which double word of vr[] to be used. But the > mmio_host_swabbed can be uninitiazed during VMX store procedure: > > kvmppc_emulate_loadstore > \- kvmppc_handle_store128_by2x64 > \- kvmppc_get_vmx_data > > This patch corrects this by using kvmppc_need_byteswap() to choose > double word of vr[] and initialized mmio_host_swabbed to avoid invisble > trouble. > > Signed-off-by: Simon Guo <wei.guo.simon@gmail.com> The patch is correct, but I think the patch description needs to say that vcpu->arch.mmio_host_swabbed is not meant to be used at all for emulation of store instructions, and this patch makes that true for VMX stores. Paul.
On Thu, May 03, 2018 at 03:48:26PM +1000, Paul Mackerras wrote: > On Wed, Apr 25, 2018 at 07:54:36PM +0800, wei.guo.simon@gmail.com wrote: > > From: Simon Guo <wei.guo.simon@gmail.com> > > > > When KVM emulates VMX store, it will invoke kvmppc_get_vmx_data() to > > retrieve VMX reg val. kvmppc_get_vmx_data() will check mmio_host_swabbed > > to decide which double word of vr[] to be used. But the > > mmio_host_swabbed can be uninitiazed during VMX store procedure: > > > > kvmppc_emulate_loadstore > > \- kvmppc_handle_store128_by2x64 > > \- kvmppc_get_vmx_data > > > > This patch corrects this by using kvmppc_need_byteswap() to choose > > double word of vr[] and initialized mmio_host_swabbed to avoid invisble > > trouble. > > > > Signed-off-by: Simon Guo <wei.guo.simon@gmail.com> > > The patch is correct, but I think the patch description needs to say > that vcpu->arch.mmio_host_swabbed is not meant to be used at all for > emulation of store instructions, and this patch makes that true for > VMX stores. I will revise the commit message accordingly. > > Paul. Thanks, - Simon
diff --git a/arch/powerpc/kvm/emulate_loadstore.c b/arch/powerpc/kvm/emulate_loadstore.c index a382e15..b8a3aef 100644 --- a/arch/powerpc/kvm/emulate_loadstore.c +++ b/arch/powerpc/kvm/emulate_loadstore.c @@ -111,6 +111,7 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu) vcpu->arch.mmio_sp64_extend = 0; vcpu->arch.mmio_sign_extend = 0; vcpu->arch.mmio_vmx_copy_nums = 0; + vcpu->arch.mmio_host_swabbed = 0; switch (get_op(inst)) { case 31: diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 4e38764..bef27b1 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -1374,7 +1374,7 @@ static inline int kvmppc_get_vmx_data(struct kvm_vcpu *vcpu, int rs, u64 *val) if (di > 1) return -1; - if (vcpu->arch.mmio_host_swabbed) + if (kvmppc_need_byteswap(vcpu)) di = 1 - di; w0 = vrs.u[di * 2];