From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joerg Roedel Subject: Re: [PATCH] kvm: svm: Only propagate next_rip when guest supports it Date: Wed, 7 Oct 2015 18:14:31 +0200 Message-ID: <20151007161431.GJ28811@8bytes.org> References: <560D1C6E.2060803@suse.com> <560D278F.10801@redhat.com> <20151006102838.GD20886@8bytes.org> <20151007110335.GA28811@8bytes.org> <20151007124700.GE28811@8bytes.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Paolo Bonzini , kvm@vger.kernel.org, Dirk =?iso-8859-1?Q?M=FCller?= To: Bandan Das Return-path: Received: from 8bytes.org ([81.169.241.247]:48782 "EHLO theia.8bytes.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754985AbbJGQOd (ORCPT ); Wed, 7 Oct 2015 12:14:33 -0400 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Oct 07, 2015 at 11:48:36AM -0400, Bandan Das wrote: > Ok, understood now. The warn_on would trigger in L1 only if it has > decided to disable nrips for some reason as was the case here. So, > my reasoning behind putting the warning was incorrect. Okay, so I think the warning can be removed. > > + > > + if (guest_cpuid_has_nrips(vcpu)) > > + nested_vmcb->control.next_rip = vmcb->control.next_rip; Note that there is a bug here, instead of vcpu it must be &svm->vcpu. Somehow I missed to at least compile-test this. Dirk is currently testing whether this (fixed) patch solves the problem in his setup. > > > > /* > > * If we emulate a VMRUN/#VMEXIT in the same host #vmexit cycle we have > > @@ -2714,6 +2716,9 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm) > > svm->vmcb->control.event_inj = nested_vmcb->control.event_inj; > > svm->vmcb->control.event_inj_err = nested_vmcb->control.event_inj_err; > > > > + /* Clear next_rip, as real hardware would do */ > > + nested_vmcb->control.next_rip = 0; > > + > > Why do we need this ? And are you sure this is what real hardware does ? > I couldn't find anything in the spec. Yeah, probably right. Since we only write guests next_rip when the guest supports it via cpuid, there is probably no point in resetting it at vmrun emulation. Joerg