From: Alexander Graf <agraf@suse.de>
To: Paul Mackerras <paulus@samba.org>, kvm-ppc@vger.kernel.org
Cc: kvm@vger.kernel.org
Subject: Re: [RFC PATCH 4/5] KVM: PPC: Use analyse_instr() in kvmppc_emulate_instruction()
Date: Mon, 28 Jul 2014 12:12:41 +0000 [thread overview]
Message-ID: <53D63E39.9000903@suse.de> (raw)
In-Reply-To: <1405764872-8744-5-git-send-email-paulus@samba.org>
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 <paulus@samba.org>
> ---
> 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
>
WARNING: multiple messages have this Message-ID (diff)
From: Alexander Graf <agraf@suse.de>
To: Paul Mackerras <paulus@samba.org>, kvm-ppc@vger.kernel.org
Cc: kvm@vger.kernel.org
Subject: Re: [RFC PATCH 4/5] KVM: PPC: Use analyse_instr() in kvmppc_emulate_instruction()
Date: Mon, 28 Jul 2014 14:12:41 +0200 [thread overview]
Message-ID: <53D63E39.9000903@suse.de> (raw)
In-Reply-To: <1405764872-8744-5-git-send-email-paulus@samba.org>
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 <paulus@samba.org>
> ---
> 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
>
next prev parent reply other threads:[~2014-07-28 12:12 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-19 10:14 [RFC PATCH 0/5] Improve PPC instruction emulation Paul Mackerras
2014-07-19 10:14 ` Paul Mackerras
2014-07-19 10:14 ` [RFC PATCH 1/5] powerpc: Split out instruction analysis part of emulate_step() Paul Mackerras
2014-07-19 10:14 ` Paul Mackerras
2014-07-28 12:01 ` Alexander Graf
2014-07-28 12:01 ` Alexander Graf
2014-07-19 10:14 ` [RFC PATCH 2/5] powerpc: Implement emulation of string loads and stores Paul Mackerras
2014-07-19 10:14 ` Paul Mackerras
2014-07-19 10:14 ` [RFC PATCH 3/5] KVM: PPC: Use pt_regs struct for integer registers in struct vcpu_arch Paul Mackerras
2014-07-19 10:14 ` Paul Mackerras
2014-07-19 10:14 ` [RFC PATCH 4/5] KVM: PPC: Use analyse_instr() in kvmppc_emulate_instruction() Paul Mackerras
2014-07-19 10:14 ` Paul Mackerras
2014-07-28 12:12 ` Alexander Graf [this message]
2014-07-28 12:12 ` Alexander Graf
2014-07-19 10:14 ` [RFC PATCH 5/5] KVM: PPC: Book3S: Make kvmppc_handle_load/store handle any load or store Paul Mackerras
2014-07-19 10:14 ` Paul Mackerras
2014-07-28 12:14 ` Alexander Graf
2014-07-28 12:14 ` Alexander Graf
2014-07-28 11:46 ` [RFC PATCH 0/5] Improve PPC instruction emulation Alexander Graf
2014-07-28 11:46 ` Alexander Graf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53D63E39.9000903@suse.de \
--to=agraf@suse.de \
--cc=kvm-ppc@vger.kernel.org \
--cc=kvm@vger.kernel.org \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.