From patchwork Wed Oct 24 09:19:35 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Fritsch X-Patchwork-Id: 1636461 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id 44CA83FC36 for ; Wed, 24 Oct 2012 09:19:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932184Ab2JXJTl (ORCPT ); Wed, 24 Oct 2012 05:19:41 -0400 Received: from eru.sfritsch.de ([188.40.99.202]:46539 "EHLO eru.sfritsch.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754405Ab2JXJTi (ORCPT ); Wed, 24 Oct 2012 05:19:38 -0400 Received: from stf (helo=localhost) by eru.sfritsch.de with local-esmtp (Exim 4.72) (envelope-from ) id 1TQx7f-0003PC-GB; Wed, 24 Oct 2012 11:19:35 +0200 Date: Wed, 24 Oct 2012 11:19:35 +0200 (CEST) From: Stefan Fritsch X-X-Sender: stf@eru.sfritsch.de To: Avi Kivity cc: kvm@vger.kernel.org Subject: Re: How to do fast accesses to LAPIC TPR under kvm? In-Reply-To: <508553D6.8030709@redhat.com> Message-ID: References: <201210172124.26939.sf@sfritsch.de> <20121018093513.GF20788@redhat.com> <507FF5B9.6060605@redhat.com> <201210200039.16412.sf@sfritsch.de> <508553D6.8030709@redhat.com> User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) MIME-Version: 1.0 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Mon, 22 Oct 2012, Avi Kivity wrote: >>> Right. You will need to expose the alternate encoding of cr8 (IIRC >>> lock mov reg, cr0) on AMD via cpuid, but otherwise it should just >>> work. Be aware that this will break cross-vendor migration. >> >> I get an exception and I am not sure why: >> >> kvm_entry: vcpu 0 >> kvm_exit: reason write_cr8 rip 0xd0203788 info 0 0 >> kvm_emulate_insn: 0:d0203788: f0 0f 22 c0 (prot32) >> kvm_inj_exception: #UD (0x0) >> >> This is qemu-kvm 1.1.2 on Linux 3.2. >> >> When I look at arch/x86/kvm/emulate.c (both the current and the v3.2 >> version), I don't see any special case handling for "lock mov reg, >> cr0" to mean "mov reg, cr8". > > emulate.c will #UD is the Lock flag is missing in the instruction decode > table. > >> Before I spend lots of time on debugging my code, can you verify if >> the alternate encoding of cr8 is actually supported in kvm or if it is >> maybe missing? Thanks in advance. > > With the decode table fix I think it should work. It needs some more changes. The patch below did the trick for me. It is against 3.5, because I didn't want to build a whole new kernel (my test machine is a dead slow AMD E-350). The patch is definitely incomplete. It now allows the lock prefix for all mov operations on the cr1-7, which should not be the case. Apart from that, do the changes look reasonable? I have not checked that this is the minimal patch that works. But the LockReg bit was definitely necessary, that was the final piece to make it work. Cheers, Stefan --- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 4837375..c7f0ec7 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -128,6 +128,7 @@ #define Priv (1<<27) /* instruction generates #GP if current CPL != 0 */ #define No64 (1<<28) #define PageTable (1 << 29) /* instruction used to write page table */ +#define LockReg (1<<30) /* lock prefix is allowed for the instruction even for reg destination */ /* Source 2 operand type */ #define Src2Shift (30) #define Src2None (OpNone << Src2Shift) @@ -420,6 +421,7 @@ static int emulator_check_intercept(struct x86_emulate_ctxt *ctxt, struct x86_instruction_info info = { .intercept = intercept, .rep_prefix = ctxt->rep_prefix, + .lock_prefix = ctxt->lock_prefix, .modrm_mod = ctxt->modrm_mod, .modrm_reg = ctxt->modrm_reg, .modrm_rm = ctxt->modrm_rm, @@ -2874,7 +2876,10 @@ static int em_mov(struct x86_emulate_ctxt *ctxt) static int em_cr_write(struct x86_emulate_ctxt *ctxt) { - if (ctxt->ops->set_cr(ctxt, ctxt->modrm_reg, ctxt->src.val)) + int cr = ctxt->modrm_reg; + if (ctxt->lock_prefix && cr == 0) + cr = 8; + if (ctxt->ops->set_cr(ctxt, cr, ctxt->src.val)) return emulate_gp(ctxt, 0); /* Disable writeback. */ @@ -3177,6 +3182,8 @@ static int check_cr_write(struct x86_emulate_ctxt *ctxt) CR8_RESERVED_BITS, }; + if (ctxt->lock_prefix && cr == 0) + cr = 8; if (!valid_cr(cr)) return emulate_ud(ctxt); @@ -3599,9 +3606,9 @@ static struct opcode twobyte_table[256] = { /* 0x10 - 0x1F */ N, N, N, N, N, N, N, N, D(ImplicitOps | ModRM), N, N, N, N, N, N, N, /* 0x20 - 0x2F */ - DIP(ModRM | DstMem | Priv | Op3264, cr_read, check_cr_read), + DIP(Lock | LockReg | ModRM | DstMem | Priv | Op3264, cr_read, check_cr_read), DIP(ModRM | DstMem | Priv | Op3264, dr_read, check_dr_read), - IIP(ModRM | SrcMem | Priv | Op3264, em_cr_write, cr_write, check_cr_write), + IIP(Lock | LockReg | ModRM | SrcMem | Priv | Op3264, em_cr_write, cr_write, check_cr_write), IIP(ModRM | SrcMem | Priv | Op3264, em_dr_write, dr_write, check_dr_write), N, N, N, N, N, N, N, GP(ModRM | DstMem | SrcReg | Sse | Mov | Aligned, &pfx_vmovntpx), @@ -4130,7 +4137,8 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt) } /* LOCK prefix is allowed only with some instructions */ - if (ctxt->lock_prefix && (!(ctxt->d & Lock) || ctxt->dst.type != OP_MEM)) { + if (ctxt->lock_prefix && (!(ctxt->d & Lock) + || (ctxt->dst.type != OP_MEM && !(ctxt->d & LockReg)))) { rc = emulate_ud(ctxt); goto done; } diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index f75af40..13d4f3b 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -4146,8 +4146,13 @@ static int svm_check_intercept(struct kvm_vcpu *vcpu, unsigned long cr0, val; u64 intercept; - if (info->intercept == x86_intercept_cr_write) - icpt_info.exit_code += info->modrm_reg; + if (info->intercept == x86_intercept_cr_write) { + int cr = info->modrm_reg; + if (cr == 0 && info->lock_prefix) { + cr = 8; + } + icpt_info.exit_code += cr; + } if (icpt_info.exit_code != SVM_EXIT_WRITE_CR0) break; diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h index 15f960c..305d804 100644 --- a/arch/x86/include/asm/kvm_emulate.h +++ b/arch/x86/include/asm/kvm_emulate.h @@ -33,6 +33,7 @@ struct x86_exception { struct x86_instruction_info { u8 intercept; /* which intercept */ u8 rep_prefix; /* rep prefix? */ + u8 lock_prefix; /* lock prefix? */ u8 modrm_mod; /* mod part of modrm */ u8 modrm_reg; /* index of register used */ u8 modrm_rm; /* rm part of modrm */