From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] kvm/svm: fix DR interception handling on upcoming AMD CPUs Date: Wed, 09 Feb 2011 15:39:41 +0200 Message-ID: <4D52991D.4000107@redhat.com> References: <1297210949-13240-1-git-send-email-andre.przywara@amd.com> <20110209100115.GB26519@amd.com> <4D526FD0.8000802@amd.com> <20110209125600.GC26519@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Andre Przywara , "mtosatti@redhat.com" , "kvm@vger.kernel.org" To: "Roedel, Joerg" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:42565 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754796Ab1BINkn (ORCPT ); Wed, 9 Feb 2011 08:40:43 -0500 In-Reply-To: <20110209125600.GC26519@amd.com> Sender: kvm-owner@vger.kernel.org List-ID: On 02/09/2011 02:56 PM, Roedel, Joerg wrote: > On Wed, Feb 09, 2011 at 05:43:28AM -0500, Andre Przywara wrote: > > Roedel, Joerg wrote: > > > On Tue, Feb 08, 2011 at 07:22:29PM -0500, Andre Przywara wrote: > > >> Somehow the code line advancing the RIP and checking for exceptions > > >> got dropped between the post on the ML and the commit. > > >> Add it again to let guests boot on upcoming AMD CPUs again. > > >> > > >> Reported-by: Joerg Roedel > > >> Signed-off-by: Andre Przywara > > >> --- > > >> arch/x86/kvm/svm.c | 1 + > > >> 1 files changed, 1 insertions(+), 0 deletions(-) > > >> > > >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > > >> index 25bd1bc..be06e58 100644 > > >> --- a/arch/x86/kvm/svm.c > > >> +++ b/arch/x86/kvm/svm.c > > >> @@ -2776,6 +2776,7 @@ static int dr_interception(struct vcpu_svm *svm) > > >> if (!err) > > >> kvm_register_write(&svm->vcpu, reg, val); > > >> } > > >> + kvm_complete_insn_gp(&svm->vcpu, err); > > > > > > Any exception check takes presedence over the intercept. So a call to > > > skip_emulated_instruction should be sufficient here. > > > You are right, but I think this way the implementation is cleaner and > > more generic. If there is no exception condition detected (err==0), then > > the call will just do skip_emulated_instruction anyway. > > But as I write this, I see that err is not initialized, so I'd propose > > this corrected version of the patch instead. > > One function call is not less clean than another and this function call > has more overhead. > Beside that, looking at kvm_set_dr this function only returns 1 if one > tries to set dr4 or dr5 while CR4.DE is not set. In this case the > function you call here would inject a #GP but architecturally a #UD > is required. > This problem doesn't matter too much because the exceptions take > precedence over the intercept anyway so it is sufficient to call > skip_emulated_instruction here. Given that the exception is incorrect I agree skip_emulated_instruction() is a better choice here. -- error compiling committee.c: too many arguments to function