Re: [PATCH] VMX: fix/adjust trap injection

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 23/11/15 12:49, Jan Beulich wrote:
> In the course of investigating the 4.1.6 backport issue of the XSA-156
> patch I realized that #DB injection has always been broken, but with it
> now getting always intercepted the problem has got worse: Documentation
> clearly states that neither DR7.GD nor DebugCtl.LBR get cleared before
> the intercept, so this is something we need to do before reflecting the
> intercepted exception.
>
> While adjusting this (and also with 4.1.6's strange use of
> X86_EVENTTYPE_SW_EXCEPTION for #DB in mind) I further realized that
> the special casing of individual vectors shouldn't be done for
> software interrupts (resulting from INT $nn).
>
> And then some code movement: Setting of CR2 for #PF can be done in the
> same switch() statement (no need for a separate if()), and reading of
> intr_info is better done close the the consumption of the variable
> (allowing the compiler to generate better code / use fewer registers
> for variables).
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -1508,16 +1508,7 @@ static void vmx_inject_trap(struct hvm_t
>      struct vcpu *curr = current;
>      struct hvm_trap _trap = *trap;
>  
> -    if ( (_trap.vector == TRAP_page_fault) &&
> -         (_trap.type == X86_EVENTTYPE_HW_EXCEPTION) )
> -        curr->arch.hvm_vcpu.guest_cr[2] = _trap.cr2;
> -
> -    if ( nestedhvm_vcpu_in_guestmode(curr) )
> -        intr_info = vcpu_2_nvmx(curr).intr.intr_info;
> -    else
> -        __vmread(VM_ENTRY_INTR_INFO, &intr_info);
> -
> -    switch ( _trap.vector )
> +    switch ( _trap.vector | -(_trap.type == X86_EVENTTYPE_SW_INTERRUPT) )
>      {
>      case TRAP_debug:
>          if ( guest_cpu_user_regs()->eflags & X86_EFLAGS_TF )
> @@ -1525,6 +1516,16 @@ static void vmx_inject_trap(struct hvm_t
>              __restore_debug_registers(curr);
>              write_debugreg(6, read_debugreg(6) | DR_STEP);
>          }
> +        if ( !nestedhvm_vcpu_in_guestmode(curr) ||
> +             !nvmx_intercepts_exception(curr, TRAP_debug, _trap.error_code) )
> +        {
> +            unsigned long val;
> +
> +            __vmread(GUEST_DR7, &val);
> +            __vmwrite(GUEST_DR7, val & ~DR_GENERAL_DETECT);
> +            __vmread(GUEST_IA32_DEBUGCTL, &val);
> +            __vmwrite(GUEST_IA32_DEBUGCTL, val & ~IA32_DEBUGCTLMSR_LBR);

Is it worth eliding the __vmwrite's if possible?  It will be fewer
VMexits if this Xen is running nested, and ISTR it will avoid slowing
down the vmentry with further consistency checks.

~Andrew