From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andre Przywara Subject: Re: [PATCH] svm: implement NEXTRIPsave SVM feature Date: Sun, 11 Apr 2010 23:51:52 +0200 Message-ID: <4BC24478.5060803@amd.com> References: <1271020048-10083-1-git-send-email-andre.przywara@amd.com> <98EA2068-1878-4698-945B-10BACCB0631F@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Cc: Avi Kivity , kvm-devel list To: Alexander Graf Return-path: Received: from va3ehsobe002.messaging.microsoft.com ([216.32.180.12]:37971 "EHLO VA3EHSOBE002.bigfish.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753074Ab0DKVwG (ORCPT ); Sun, 11 Apr 2010 17:52:06 -0400 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: Alexander Graf wrote: > On 11.04.2010, at 23:40, Alexander Graf wrote: > >> /* Either adds offset n to the instruction counter or takes the next >> instruction pointer from the vmcb if the CPU supports it */ >> >> static u64 svm_next_rip(struct kvm_vcpu *vcpu, int add) >> { >> if (svm->vmcb->control.next_rip != 0) > > In fact, that if should probably be: > > if (svm_has(SVM_FEATURE_NRIP)) This is not sufficient. The next RIP is only provided for some intercepts (namely instruction intercepts), so one would need to check the validity of this field anyway. By definition reserved VMCB fields are 0, and as 0 is never a valid _next_ RIP, this is a quick and correct check. Regards, Andre. P.S. I don't have a strong opinion about your proposed refactoring, if Avi agrees I will rework it. I only found the current fix small and easy, and the mentioned patch for older CPUs removed the add line anyway, so the concerns you rose did not apply to the original version of the patch. -- Andre Przywara AMD-Operating System Research Center (OSRC), Dresden, Germany Tel: +49 351 488-3567-12