Re: [PATCH 2/7] x86/emul: Fix and extend #DB trap handling

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/09/2023 9:36 pm, Andrew Cooper wrote:
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c 
> b/xen/arch/x86/x86_emulate/x86_emulate.c
> index 94caec1d142c..de7f99500e3f 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -8379,13 +8379,6 @@ x86_emulate(
>      if ( !mode_64bit() )
>          _regs.r(ip) = (uint32_t)_regs.r(ip);
>  
> -    /* Should a singlestep #DB be raised? */
> -    if ( rc == X86EMUL_OKAY && singlestep && !ctxt->retire.mov_ss )
> -    {
> -        ctxt->retire.singlestep = true;
> -        ctxt->retire.sti = false;
> -    }
> -
>      if ( rc != X86EMUL_DONE )
>          *ctxt->regs = _regs;
>      else
> @@ -8394,6 +8387,19 @@ x86_emulate(
>          rc = X86EMUL_OKAY;
>      }
>  
> +    /* Should a singlestep #DB be raised? */
> +    if ( rc == X86EMUL_OKAY && singlestep )
> +    {
> +        ctxt->retire.pending_dbg |= X86_DR6_BS;
> +
> +        /* BROKEN - TODO, merge into pending_dbg. */
> +        if ( !ctxt->retire.mov_ss )
> +        {
> +            ctxt->retire.singlestep = true;
> +            ctxt->retire.sti = false;
> +        }

I occurs to me that setting X86_DR6_BS outside of the !mov_ss case will
break HVM (when HVM swaps from singlestep to pending_dbg) until one of
the further TODOs is addressed.

This will need bringing back within the conditional to avoid regressions
in the short term.

~Andrew