From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Subject: Re: [RFC PATCH 4/5] KVM: PPC: Use analyse_instr() in kvmppc_emulate_instruction() Date: Mon, 28 Jul 2014 14:12:41 +0200 Message-ID: <53D63E39.9000903@suse.de> References: <1405764872-8744-1-git-send-email-paulus@samba.org> <1405764872-8744-5-git-send-email-paulus@samba.org> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Paul Mackerras , kvm-ppc@vger.kernel.org Return-path: Received: from cantor2.suse.de ([195.135.220.15]:51008 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752936AbaG1MMp (ORCPT ); Mon, 28 Jul 2014 08:12:45 -0400 In-Reply-To: <1405764872-8744-5-git-send-email-paulus@samba.org> Sender: kvm-owner@vger.kernel.org List-ID: On 19.07.14 12:14, Paul Mackerras wrote: > This changes kvmppc_emulate_instruction() to use the common instruction > decoding code from arch/powerpc/lib/sstep.c. This expands the set of > instructions that we recognize to include all of the integer load and > store instructions except for the string (lsw*, stsw*) and multiple > (lmw, stmw) instructions and reduces the total amount of code. > > This removes kvmppc_handle_loads() and instead adds a 'sign_extend' > parameter to kvmppc_handle_load(). (In fact kvmppc_handle_loads() > could not have worked previously; it sets vcpu->arch.mmio_sign_extend > to 1 before calling kvmppc_handle_load, which sets it back to 0.) > > The instruction emulation for specific CPU flavours is largely unchanged, > except that emulation of mfmsr, eieio, and (for book 3S) mtmsr[d] has > moved into the generic code, and tlbsync emulation into the CPU-specific > code. > > At this point the code still assumes that the instruction caused the > most recent guest exit, and that if the instruction is a load or store, > it must have been an emulated MMIO access and vcpu->arch.paddr_accessed > already contains the guest physical address being accessed. > > Signed-off-by: Paul Mackerras > --- > arch/powerpc/include/asm/kvm_ppc.h | 5 +- > arch/powerpc/kvm/book3s_emulate.c | 22 +-- > arch/powerpc/kvm/book3s_paired_singles.c | 6 +- > arch/powerpc/kvm/booke_emulate.c | 8 +- > arch/powerpc/kvm/emulate.c | 317 ++++++++++--------------------- > arch/powerpc/kvm/powerpc.c | 17 +- > 6 files changed, 110 insertions(+), 265 deletions(-) > > diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h > index 246fb9a..9318cf3 100644 > --- a/arch/powerpc/include/asm/kvm_ppc.h > +++ b/arch/powerpc/include/asm/kvm_ppc.h > @@ -54,10 +54,7 @@ extern void kvmppc_handler_highmem(void); > extern void kvmppc_dump_vcpu(struct kvm_vcpu *vcpu); > extern int kvmppc_handle_load(struct kvm_run *run, struct kvm_vcpu *vcpu, > unsigned int rt, unsigned int bytes, > - int is_default_endian); > -extern int kvmppc_handle_loads(struct kvm_run *run, struct kvm_vcpu *vcpu, > - unsigned int rt, unsigned int bytes, > - int is_default_endian); > + int is_default_endian, int sign_extend); > 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/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c > index 84fddcd..f74b8d5 100644 > --- a/arch/powerpc/kvm/book3s_emulate.c > +++ b/arch/powerpc/kvm/book3s_emulate.c > @@ -129,24 +129,6 @@ int kvmppc_core_emulate_op_pr(struct kvm_run *run, struct kvm_vcpu *vcpu, > break; > case 31: > switch (get_xop(inst)) { > - case OP_31_XOP_MFMSR: > - kvmppc_set_gpr(vcpu, rt, kvmppc_get_msr(vcpu)); > - break; > - case OP_31_XOP_MTMSRD: > - { > - ulong rs_val = kvmppc_get_gpr(vcpu, rs); > - if (inst & 0x10000) { > - ulong new_msr = kvmppc_get_msr(vcpu); > - new_msr &= ~(MSR_RI | MSR_EE); > - new_msr |= rs_val & (MSR_RI | MSR_EE); > - kvmppc_set_msr_fast(vcpu, new_msr); > - } else > - kvmppc_set_msr(vcpu, rs_val); > - break; > - } > - case OP_31_XOP_MTMSR: > - kvmppc_set_msr(vcpu, kvmppc_get_gpr(vcpu, rs)); > - break; > case OP_31_XOP_MFSR: > { > int srnum; > @@ -189,6 +171,8 @@ int kvmppc_core_emulate_op_pr(struct kvm_run *run, struct kvm_vcpu *vcpu, > vcpu->arch.mmu.tlbie(vcpu, addr, large); > break; > } > + case OP_31_XOP_TLBSYNC: > + break; > #ifdef CONFIG_PPC_BOOK3S_64 > case OP_31_XOP_FAKE_SC1: > { > @@ -217,8 +201,6 @@ int kvmppc_core_emulate_op_pr(struct kvm_run *run, struct kvm_vcpu *vcpu, > break; > } > #endif > - case OP_31_XOP_EIOIO: > - break; > case OP_31_XOP_SLBMTE: > if (!vcpu->arch.mmu.slbmte) > return EMULATE_FAIL; > diff --git a/arch/powerpc/kvm/book3s_paired_singles.c b/arch/powerpc/kvm/book3s_paired_singles.c > index 6c8011f..a5bde19 100644 > --- a/arch/powerpc/kvm/book3s_paired_singles.c > +++ b/arch/powerpc/kvm/book3s_paired_singles.c > @@ -200,7 +200,7 @@ static int kvmppc_emulate_fpr_load(struct kvm_run *run, struct kvm_vcpu *vcpu, > goto done_load; > } else if (r == EMULATE_DO_MMIO) { > emulated = kvmppc_handle_load(run, vcpu, KVM_MMIO_REG_FPR | rs, > - len, 1); > + len, 1, 0); > goto done_load; > } > > @@ -291,12 +291,12 @@ static int kvmppc_emulate_psq_load(struct kvm_run *run, struct kvm_vcpu *vcpu, > goto done_load; > } else if ((r == EMULATE_DO_MMIO) && w) { > emulated = kvmppc_handle_load(run, vcpu, KVM_MMIO_REG_FPR | rs, > - 4, 1); > + 4, 1, 0); > vcpu->arch.qpr[rs] = tmp[1]; > goto done_load; > } else if (r == EMULATE_DO_MMIO) { > emulated = kvmppc_handle_load(run, vcpu, KVM_MMIO_REG_FQPR | rs, > - 8, 1); > + 8, 1, 0); > goto done_load; > } > > diff --git a/arch/powerpc/kvm/booke_emulate.c b/arch/powerpc/kvm/booke_emulate.c > index 6bef2c7..81519b3 100644 > --- a/arch/powerpc/kvm/booke_emulate.c > +++ b/arch/powerpc/kvm/booke_emulate.c > @@ -74,11 +74,6 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu, > case 31: > switch (get_xop(inst)) { > > - case OP_31_XOP_MFMSR: > - kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->msr); > - kvmppc_set_exit_type(vcpu, EMULATED_MFMSR_EXITS); > - break; > - > case OP_31_XOP_MTMSR: > kvmppc_set_exit_type(vcpu, EMULATED_MTMSR_EXITS); > kvmppc_set_msr(vcpu, kvmppc_get_gpr(vcpu, rs)); > @@ -96,6 +91,9 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu, > kvmppc_set_exit_type(vcpu, EMULATED_WRTEE_EXITS); > break; > > + case OP_31_XOP_TLBSYNC: > + break; > + > default: > emulated = EMULATE_FAIL; > } > diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c > index da86d9b..0e66230 100644 > --- a/arch/powerpc/kvm/emulate.c > +++ b/arch/powerpc/kvm/emulate.c > @@ -31,6 +31,7 @@ [...] > - case OP_LHAU: > - emulated = kvmppc_handle_loads(run, vcpu, rt, 2, 1); > - kvmppc_set_gpr(vcpu, ra, vcpu->arch.vaddr_accessed); > + case MFMSR: > + kvmppc_set_gpr(vcpu, op.reg, kvmppc_get_msr(vcpu)); > + kvmppc_set_exit_type(vcpu, EMULATED_MFMSR_EXITS); > break; > > - case OP_STH: > - emulated = kvmppc_handle_store(run, vcpu, > - kvmppc_get_gpr(vcpu, rs), > - 2, 1); > +#ifdef CONFIG_PPC_BOOK3S Why can't we handle this in the book3s emulation file? We could just pass op into it, no? Alex > + case MTMSR: > + val = kvmppc_get_gpr(vcpu, op.reg); > + val = (val & op.val) | (kvmppc_get_msr(vcpu) & ~op.val); > + kvmppc_set_exit_type(vcpu, EMULATED_MTMSR_EXITS); > + kvmppc_set_msr(vcpu, val); > break; > +#endif >