From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andre Przywara Subject: Re: [PATCH 2/5] kvm/svm: enhance MOV CR intercept handler Date: Mon, 20 Dec 2010 14:20:10 +0100 Message-ID: <4D0F580A.9050503@amd.com> References: <1291989088-1380-1-git-send-email-andre.przywara@amd.com> <1291989088-1380-3-git-send-email-andre.przywara@amd.com> <20101220115607.GC21124@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Cc: "avi@redhat.com" , "kvm@vger.kernel.org" To: Marcelo Tosatti Return-path: Received: from va3ehsobe001.messaging.microsoft.com ([216.32.180.11]:31065 "EHLO VA3EHSOBE008.bigfish.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753518Ab0LTNXI (ORCPT ); Mon, 20 Dec 2010 08:23:08 -0500 In-Reply-To: <20101220115607.GC21124@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: Marcelo Tosatti wrote: > On Fri, Dec 10, 2010 at 02:51:25PM +0100, Andre Przywara wrote: >> Newer SVM implementations provide the GPR number in the VMCB, so >> that the emulation path is no longer necesarry to handle CR >> register access intercepts. Implement the handling in svm.c and >> use it when the info is provided. >> >> Signed-off-by: Andre Przywara >> --- >> arch/x86/include/asm/svm.h | 2 + >> arch/x86/kvm/svm.c | 91 ++++++++++++++++++++++++++++++++++++++----- >> 2 files changed, 82 insertions(+), 11 deletions(-) >> >> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h >> index 11dbca7..589fc25 100644 >> --- a/arch/x86/include/asm/svm.h >> +++ b/arch/x86/include/asm/svm.h >> @@ -256,6 +256,8 @@ struct __attribute__ ((__packed__)) vmcb { >> #define SVM_EXITINFOSHIFT_TS_REASON_JMP 38 >> #define SVM_EXITINFOSHIFT_TS_HAS_ERROR_CODE 44 >> >> +#define SVM_EXITINFO_REG_MASK 0x0F >> + >> #define SVM_EXIT_READ_CR0 0x000 >> #define SVM_EXIT_READ_CR3 0x003 >> #define SVM_EXIT_READ_CR4 0x004 >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> index 298ff79..ee5f100 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -2594,12 +2594,81 @@ static int emulate_on_interception(struct vcpu_svm *svm) >> return emulate_instruction(&svm->vcpu, 0, 0, 0) == EMULATE_DONE; >> } >> >> +static int cr_interception(struct vcpu_svm *svm) >> +{ >> + int reg, cr; >> + unsigned long val; >> + int err; >> + >> + if (!static_cpu_has(X86_FEATURE_DECODEASSISTS)) >> + return emulate_on_interception(svm); >> + >> + /* bit 63 is the valid bit, as not all instructions (like lmsw) >> + provide the information */ >> + if (unlikely((svm->vmcb->control.exit_info_1 & (1ULL << 63)) == 0)) >> + return emulate_on_interception(svm); >> + >> + reg = svm->vmcb->control.exit_info_1 & SVM_EXITINFO_REG_MASK; >> + cr = svm->vmcb->control.exit_code - SVM_EXIT_READ_CR0; >> + >> + err = 0; >> + if (cr >= 16) { /* mov to cr */ >> + cr -= 16; >> + val = kvm_register_read(&svm->vcpu, reg); >> + switch (cr) { >> + case 0: >> + err = kvm_set_cr0(&svm->vcpu, val); >> + break; >> + case 3: >> + err = kvm_set_cr3(&svm->vcpu, val); >> + break; >> + case 4: >> + err = kvm_set_cr4(&svm->vcpu, val); >> + break; >> + case 8: >> + err = kvm_set_cr8(&svm->vcpu, val); >> + break; >> + default: >> + WARN(1, "unhandled write to CR%d", cr); >> + return EMULATE_FAIL; >> + } > > Wrong return value? Thats right, thanks for catching this. I copied this from somewhere else ;-( > Is WARN() really wanted? Well, this is a rather pathological code path. Illegal CR accesses generate #UD before interception, and additionally we don't set the intercept bits for these, so this should really never occur. But just for the sake of completeness I can inject an #UD here and return 1. So I leave the WARN in to inform the user that something stupid is going on, but it could also be BUG(). Regards, Andre. -- Andre Przywara AMD-OSRC (Dresden) Tel: x29712