Re: [PATCH 4/5] KVM: vmx: Unavailable DR4/5 is checked before CPL

["Radim Krčmář" <rkrcmar@redhat.com>]

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 <namit@cs.technion.ac.il>
> ---
> 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)