Re: [Xen-devel] [PATCH] x86/vmx: fix dr6 handling issue

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.05.14 at 10:00, <yang.z.zhang@xxxxxxxxx> wrote:
> @@ -1178,16 +1189,10 @@ static void vmx_update_host_cr3(struct vcpu *v)
>  
>  void vmx_update_debug_state(struct vcpu *v)
>  {
> -    unsigned long mask;
> -
> -    mask = 1u << TRAP_int3;
> -    if ( !cpu_has_monitor_trap_flag )
> -        mask |= 1u << TRAP_debug;

So how are things going to work now when !cpu_has_monitor_trap_flag?

> @@ -2717,10 +2722,18 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>               */
>              __vmread(EXIT_QUALIFICATION, &exit_qualification);
>              HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
> -            write_debugreg(6, exit_qualification | 0xffff0ff0);
> -            if ( !v->domain->debugger_attached || cpu_has_monitor_trap_flag )
> -                goto exit_and_crash;
> -            domain_pause_for_debugger();
> +            exit_qualification |= DR_STATUS_RESERVED_ONE;
> +            if ( v->domain->debugger_attached )
> +            {
> +                write_debugreg(6, exit_qualification);
> +                domain_pause_for_debugger();
> +            }
> +            else 
> +            {
> +                v->arch.debugreg[6] |= exit_qualification;
> +                __restore_debug_registers(v);
> +                hvm_inject_hw_exception(TRAP_debug, 
> HVM_DELIVER_NO_ERROR_CODE);
> +            }

I.e. is that really the only thing that needs doing when there' no MTF?

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -3763,8 +3763,8 @@ long set_debugreg(struct vcpu *v, unsigned int reg, 
> unsigned long value)
>           * DR6: Bits 4-11,16-31 reserved (set to 1).
>           *      Bit 12 reserved (set to 0).
>           */
> -        value &= 0xffffefff; /* reserved bits => 0 */
> -        value |= 0xffff0ff0; /* reserved bits => 1 */
> +        value &= ~DR_STATUS_RESERVED_ZERO;  /* reserved bits => 0 */

This is not equivalent to the old code, ...

> --- a/xen/include/asm-x86/debugreg.h
> +++ b/xen/include/asm-x86/debugreg.h
> @@ -76,4 +76,7 @@
>  long set_debugreg(struct vcpu *, unsigned int reg, unsigned long value);
>  void activate_debugregs(const struct vcpu *);
>  
> +#define DR_STATUS_RESERVED_ZERO  (0x00001000ul) /* Reserved, read as zero */

... because here you're introducing a problem similar to the one
which was the subject of XSA-12. This needs to be

#define DR_STATUS_RESERVED_ZERO  (~0xffffeffful) /* Reserved, read as zero */

matching DR_CONTROL_RESERVED_ZERO.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel