Re: [PATCH v4 2/6] x86/debugger: separate Xen and guest debugging debugger_trap_* functions

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 28/09/2021 21:30, Bobby Eshleman wrote:
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index e60af16ddd..772e2a5bfc 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -858,13 +858,20 @@ static void do_trap(struct cpu_user_regs *regs)
>      if ( regs->error_code & X86_XEC_EXT )
>          goto hardware_trap;
>  
> -    if ( debugger_trap_entry(trapnr, regs) )
> -        return;
> -
>      ASSERT(trapnr < 32);
>  
>      if ( guest_mode(regs) )
>      {
> +        struct vcpu *curr = current;
> +        if ( (trapnr == TRAP_debug || trapnr == TRAP_int3) &&
> +              guest_kernel_mode(curr, regs) &&
> +              curr->domain->debugger_attached )
> +        {
> +            if ( trapnr != TRAP_debug )
> +                curr->arch.gdbsx_vcpu_event = trapnr;
> +            domain_pause_for_debugger();
> +            return;
> +        }

This is unreachable.  do_trap() isn't used for TRAP_debug or TRAP_int3.

> @@ -2014,9 +2021,6 @@ void do_entry_CP(struct cpu_user_regs *regs)
>      const char *err = "??";
>      unsigned int ec = regs->error_code;
>  
> -    if ( debugger_trap_entry(TRAP_debug, regs) )
> -        return;
> -
>      /* Decode ec if possible */
>      if ( ec < ARRAY_SIZE(errors) && errors[ec][0] )
>          err = errors[ec];
> @@ -2028,6 +2032,12 @@ void do_entry_CP(struct cpu_user_regs *regs)
>       */
>      if ( guest_mode(regs) )
>      {
> +        struct vcpu *curr = current;
> +        if ( guest_kernel_mode(curr, regs) && 
> curr->domain->debugger_attached )
> +        {
> +            domain_pause_for_debugger();
> +            return;
> +        }

Urgh.  The TRAP_debug above was a copy/paste error.

I'll submit a patch, as it wants backporting for a couple of releases,
after which there should be no additions in do_entry_CP().

Everything else looks good.

~Andrew