From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [PATCH 4/5] KVM: vmx: Unavailable DR4/5 is checked before CPL Date: Mon, 6 Oct 2014 21:33:37 +0200 Message-ID: <20141006193337.GA2722@potion.brq.redhat.com> References: <1412287806-16016-1-git-send-email-namit@cs.technion.ac.il> <1412287806-16016-5-git-send-email-namit@cs.technion.ac.il> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: pbonzini@redhat.com, joro@8bytes.org, kvm@vger.kernel.org To: Nadav Amit Return-path: Received: from mx1.redhat.com ([209.132.183.28]:33474 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750780AbaJFTdt (ORCPT ); Mon, 6 Oct 2014 15:33:49 -0400 Content-Disposition: inline In-Reply-To: <1412287806-16016-5-git-send-email-namit@cs.technion.ac.il> Sender: kvm-owner@vger.kernel.org List-ID: 2014-10-03 01:10+0300, Nadav Amit: > If DR4/5 is accessed when it is unavailable (since CR4.DE is set), then #UD > should be generated even if CPL>0. This is according to Intel SDM Table 6-2: > "Priority Among Simultaneous Exceptions and Interrupts". > > Note, that this may happen on the first DR access, even if the host does not > sets debug breakpoints. Obviously, it occurs when the host debugs the guest. (This got me confused for a while; "first" because we disable DR exiting in the handler.) > This patch moves the DR4/5 checks from __kvm_set_dr/_kvm_get_dr to handle_dr. > The emulator already checks DR4/5 availability in check_dr_read. Nested > virutalization related calls to kvm_set_dr/kvm_get_dr would not like to inject > exceptions to the guest. > > As for SVM, the patch follows the previous logic as much as possible. Anyhow, > it appears the DR interception code might be buggy - even if the DR access > may cause an exception, the instruction is skipped. SVM likely injects GP (UD) before it intercepts DR. [2:Table 15-7]: All normal exception checks take precedence over the SVM intercepts. => no need to check even in our case. > Signed-off-by: Nadav Amit > --- > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 6857257..e903167 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -806,8 +816,6 @@ static int __kvm_set_dr(struct kvm_vcpu *vcpu, int dr, unsigned long val) > vcpu->arch.eff_db[dr] = val; > break; > case 4: > - if (kvm_read_cr4_bits(vcpu, X86_CR4_DE)) WARN_ONCE_ON() instead? > - return 1; /* #UD */ > /* fall through */ > case 6: > if (val & 0xffffffff00000000ULL)