On 18/08/2021 21:29, Bobby Eshleman wrote:
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index e60af16ddd..d0a4c0ea74 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;
> + }
There's no need for this. Both TRAP_debug and TRAP_int3 have their own
custom handers, and don't use do_trap().
> @@ -1215,6 +1218,12 @@ void do_int3(struct cpu_user_regs *regs)
>
> return;
> }
> + else if ( guest_kernel_mode(curr, regs) &&
> curr->domain->debugger_attached )
if ( foo ) { ...; return; } else if ( bar ) is a dangerous anti-pattern.
This should just be a plain if().
> @@ -1994,6 +1994,11 @@ void do_debug(struct cpu_user_regs *regs)
>
> return;
> }
> + else if ( guest_kernel_mode(v, regs) && v->domain->debugger_attached )
Same here.
> @@ -2028,6 +2030,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;
> + }
> gprintk(XENLOG_ERR, "Hit #CP[%04x] in guest context %04x:%p\n",
> ec, regs->cs, _p(regs->rip));
> ASSERT_UNREACHABLE();
This path is fatal to Xen, because it should be impossible. If we ever
get around to supporting CET for PV guests, #CP still isn't #DB/#BP so
shouldn't pause for the debugger.
I can fix all of these on commit.
~Andrew
