From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] svm: implement NEXTRIPsave SVM feature Date: Mon, 12 Apr 2010 13:34:56 +0300 Message-ID: <4BC2F750.4020007@redhat.com> References: <1271020048-10083-1-git-send-email-andre.przywara@amd.com> <4BC2F3F4.4030309@redhat.com> <204DAA35-1D05-4738-B65C-C68C622B52FF@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Andre Przywara , kvm@vger.kernel.org To: Alexander Graf Return-path: Received: from mx1.redhat.com ([209.132.183.28]:62603 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752133Ab0DLKfD (ORCPT ); Mon, 12 Apr 2010 06:35:03 -0400 In-Reply-To: <204DAA35-1D05-4738-B65C-C68C622B52FF@suse.de> Sender: kvm-owner@vger.kernel.org List-ID: On 04/12/2010 01:29 PM, Alexander Graf wrote: > On 12.04.2010, at 12:20, Avi Kivity wrote: > > >> On 04/12/2010 12:07 AM, Andre Przywara wrote: >> >>> On SVM we set the instruction length of skipped instructions >>> to hard-coded, well known values, which could be wrong when (bogus, >>> but valid) prefixes (REX, segment override) are used. >>> Newer AMD processors (Fam10h 45nm and better, aka. PhenomII or >>> AthlonII) have an explicit NEXTRIP field in the VMCB containing the >>> desired information. >>> Since it is cheap to do so, we use this field to override the guessed >>> value on newer processors. >>> A fix for older CPUs would be rather expensive, as it would require >>> to fetch and partially decode the instruction. As the problem is not >>> a security issue and needs special, handcrafted code to trigger >>> (no compiler will ever generate such code), I omit a fix for older >>> CPUs. >>> If someone is interested, I have both a patch for these CPUs as well as >>> demo code triggering this issue: It segfaults under KVM, but runs >>> perfectly on native Linux. >>> >>> >>> @@ -319,6 +319,9 @@ static void skip_emulated_instruction(struct kvm_vcpu *vcpu) >>> { >>> struct vcpu_svm *svm = to_svm(vcpu); >>> >>> + if (svm->vmcb->control.next_rip != 0) >>> + svm->next_rip = svm->vmcb->control.next_rip; >>> + >>> if (!svm->next_rip) { >>> if (emulate_instruction(vcpu, 0, 0, EMULTYPE_SKIP) != >>> EMULATE_DONE) >>> >>> >> How does this interact with nested svm? Suppose we exit a nested guest, then emulate a #VMEXIT, we'll have next_rip from a previous exit? >> > Since we don't call skip_emulated_instruction on #VMEXIT injection I think we're safe here. The instruction skip is done at the vmrun intercept. > > I think you're right. If the guest intercepts, we don't skip_emulated_instruction(). If the guest doesn't intercept, we keep the nested vmcb, so next_rip is right. Neat. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.