From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andre Przywara Subject: Re: [PATCH v2 05/30] KVM: Provide callback to get/set control registers in emulator ops. Date: Mon, 15 Mar 2010 14:06:48 +0100 Message-ID: <4B9E30E8.5000506@amd.com> References: <1268583675-3101-1-git-send-email-gleb@redhat.com> <1268583675-3101-6-git-send-email-gleb@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Cc: avi@redhat.com, mtosatti@redhat.com, kvm@vger.kernel.org To: Gleb Natapov Return-path: Received: from tx2ehsobe001.messaging.microsoft.com ([65.55.88.11]:9893 "EHLO TX2EHSOBE002.bigfish.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934247Ab0CONHY (ORCPT ); Mon, 15 Mar 2010 09:07:24 -0400 In-Reply-To: <1268583675-3101-6-git-send-email-gleb@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Gleb Natapov wrote: > Use this callback instead of directly call kvm function. Also rename > realmode_(set|get)_cr to emulator_(set|get)_cr since function has nothing > to do with real mode. Do you mind removing the static before emulator_{set,get}_cr and marking it EXPORT_SYMBOL? Then one could use it in vmx.c (and soon in svm.c ;-) while handling MOV-CR intercepts. Currently most of the code is actually duplicated. Also, shouldn't mk_cr_64() not be called mask_cr_64() for better readability? Regards, Andre. > Signed-off-by: Gleb Natapov > --- > arch/x86/include/asm/kvm_emulate.h | 3 +- > arch/x86/include/asm/kvm_host.h | 2 - > arch/x86/kvm/emulate.c | 7 +- > arch/x86/kvm/x86.c | 114 ++++++++++++++++++------------------ > 4 files changed, 63 insertions(+), 63 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h > index 2666d7a..0c5caa4 100644 > --- a/arch/x86/include/asm/kvm_emulate.h > +++ b/arch/x86/include/asm/kvm_emulate.h > @@ -108,7 +108,8 @@ struct x86_emulate_ops { > const void *new, > unsigned int bytes, > struct kvm_vcpu *vcpu); > - > + ulong (*get_cr)(int cr, struct kvm_vcpu *vcpu); > + void (*set_cr)(int cr, ulong val, struct kvm_vcpu *vcpu); > }; > > /* Type, address-of, and value of an instruction's operand. */ > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 3b178d8..e8e108a 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -585,8 +585,6 @@ void realmode_lidt(struct kvm_vcpu *vcpu, u16 size, unsigned long address); > void realmode_lmsw(struct kvm_vcpu *vcpu, unsigned long msw, > unsigned long *rflags); > > -unsigned long realmode_get_cr(struct kvm_vcpu *vcpu, int cr); > -void realmode_set_cr(struct kvm_vcpu *vcpu, int cr, unsigned long value); > void kvm_enable_efer_bits(u64); > int kvm_get_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *data); > int kvm_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data); > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index 91450b5..5b060e4 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -2483,7 +2483,7 @@ twobyte_insn: > break; > case 4: /* smsw */ > c->dst.bytes = 2; > - c->dst.val = realmode_get_cr(ctxt->vcpu, 0); > + c->dst.val = ops->get_cr(0, ctxt->vcpu); > break; > case 6: /* lmsw */ > realmode_lmsw(ctxt->vcpu, (u16)c->src.val, > @@ -2519,8 +2519,7 @@ twobyte_insn: > case 0x20: /* mov cr, reg */ > if (c->modrm_mod != 3) > goto cannot_emulate; > - c->regs[c->modrm_rm] = > - realmode_get_cr(ctxt->vcpu, c->modrm_reg); > + c->regs[c->modrm_rm] = ops->get_cr(c->modrm_reg, ctxt->vcpu); > c->dst.type = OP_NONE; /* no writeback */ > break; > case 0x21: /* mov from dr to reg */ > @@ -2534,7 +2533,7 @@ twobyte_insn: > case 0x22: /* mov reg, cr */ > if (c->modrm_mod != 3) > goto cannot_emulate; > - realmode_set_cr(ctxt->vcpu, c->modrm_reg, c->modrm_val); > + ops->set_cr(c->modrm_reg, c->modrm_val, ctxt->vcpu); > c->dst.type = OP_NONE; > break; > case 0x23: /* mov from reg to dr */ > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index a1e671a..bf714df 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3370,12 +3370,70 @@ void kvm_report_emulation_failure(struct kvm_vcpu *vcpu, const char *context) > } > EXPORT_SYMBOL_GPL(kvm_report_emulation_failure); > > +static u64 mk_cr_64(u64 curr_cr, u32 new_val) > +{ > + return (curr_cr & ~((1ULL << 32) - 1)) | new_val; > +} > + > +static unsigned long emulator_get_cr(int cr, struct kvm_vcpu *vcpu) > +{ > + unsigned long value; > + > + switch (cr) { > + case 0: > + value = kvm_read_cr0(vcpu); > + break; > + case 2: > + value = vcpu->arch.cr2; > + break; > + case 3: > + value = vcpu->arch.cr3; > + break; > + case 4: > + value = kvm_read_cr4(vcpu); > + break; > + case 8: > + value = kvm_get_cr8(vcpu); > + break; > + default: > + vcpu_printf(vcpu, "%s: unexpected cr %u\n", __func__, cr); > + return 0; > + } > + > + return value; > +} > + > +static void emulator_set_cr(int cr, unsigned long val, struct kvm_vcpu *vcpu) > +{ > + switch (cr) { > + case 0: > + kvm_set_cr0(vcpu, mk_cr_64(kvm_read_cr0(vcpu), val)); > + break; > + case 2: > + vcpu->arch.cr2 = val; > + break; > + case 3: > + kvm_set_cr3(vcpu, val); > + break; > + case 4: > + kvm_set_cr4(vcpu, mk_cr_64(kvm_read_cr4(vcpu), val)); > + break; > + case 8: > + kvm_set_cr8(vcpu, val & 0xfUL); > + break; > + default: > + vcpu_printf(vcpu, "%s: unexpected cr %u\n", __func__, cr); > + } > +} > + > static struct x86_emulate_ops emulate_ops = { > .read_std = kvm_read_guest_virt_system, > .fetch = kvm_fetch_guest_virt, > .read_emulated = emulator_read_emulated, > .write_emulated = emulator_write_emulated, > .cmpxchg_emulated = emulator_cmpxchg_emulated, > + .get_cr = emulator_get_cr, > + .set_cr = emulator_set_cr, > }; > > static void cache_all_regs(struct kvm_vcpu *vcpu) > @@ -3973,11 +4031,6 @@ int kvm_fix_hypercall(struct kvm_vcpu *vcpu) > return emulator_write_emulated(rip, instruction, 3, vcpu); > } > > -static u64 mk_cr_64(u64 curr_cr, u32 new_val) > -{ > - return (curr_cr & ~((1ULL << 32) - 1)) | new_val; > -} > - > void realmode_lgdt(struct kvm_vcpu *vcpu, u16 limit, unsigned long base) > { > struct desc_ptr dt = { limit, base }; > @@ -3999,57 +4052,6 @@ void realmode_lmsw(struct kvm_vcpu *vcpu, unsigned long msw, > *rflags = kvm_get_rflags(vcpu); > } > > -unsigned long realmode_get_cr(struct kvm_vcpu *vcpu, int cr) > -{ > - unsigned long value; > - > - switch (cr) { > - case 0: > - value = kvm_read_cr0(vcpu); > - break; > - case 2: > - value = vcpu->arch.cr2; > - break; > - case 3: > - value = vcpu->arch.cr3; > - break; > - case 4: > - value = kvm_read_cr4(vcpu); > - break; > - case 8: > - value = kvm_get_cr8(vcpu); > - break; > - default: > - vcpu_printf(vcpu, "%s: unexpected cr %u\n", __func__, cr); > - return 0; > - } > - > - return value; > -} > - > -void realmode_set_cr(struct kvm_vcpu *vcpu, int cr, unsigned long val) > -{ > - switch (cr) { > - case 0: > - kvm_set_cr0(vcpu, mk_cr_64(kvm_read_cr0(vcpu), val)); > - break; > - case 2: > - vcpu->arch.cr2 = val; > - break; > - case 3: > - kvm_set_cr3(vcpu, val); > - break; > - case 4: > - kvm_set_cr4(vcpu, mk_cr_64(kvm_read_cr4(vcpu), val)); > - break; > - case 8: > - kvm_set_cr8(vcpu, val & 0xfUL); > - break; > - default: > - vcpu_printf(vcpu, "%s: unexpected cr %u\n", __func__, cr); > - } > -} > - > static int move_to_next_stateful_cpuid_entry(struct kvm_vcpu *vcpu, int i) > { > struct kvm_cpuid_entry2 *e = &vcpu->arch.cpuid_entries[i]; -- Andre Przywara AMD-OSRC (Dresden) Tel: x29712