[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 3/8] x86/emul: Add pending_dbg field to x86_event


  • To: Jinoh Kang <jinoh.kang.kr@xxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 30 Aug 2023 15:30:56 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=zXqIK2G696FTBrTz2yHQoXEgl3saFIS6AI/J3t0xXw4=; b=a4vXeCecb9ZoiRiMJk2nB96nVuCCZCSL+hY7uyl1EyEEK+VdtE9SxGmdHHWSCvqxL24JQhchXiwNVklDInds8qyh9+2Xmqd40XS0tSq6/XhIBfzuF7HRe2G7x6DEg07bzEmeAGi694gtsTrW4nugI6gGAjP4OXsuIKHKLtABfpBbbDZ46hdlQH6yCbZFyLDVyLD0WHySJlbHKe1Cl32F9VV5kJ341j/cwi5/LzJafRkawzGUUhdizuZ/0ruwkds3NNcOjF/YDMjWxzyeGz8GPe+7+btVBCpwUZVsnwDT5uJmOq9j+9ofWAuhl1USFJ7BArnAQrImKzib/ZGYSyUYXA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=moO6DFhTzDO8F/em2CeMswJ0sZY87QNKFL9s0hF49jVYNYTfXquIJsVo/bQvw4xwPJJEGuJt0m6DRsNjp80IuO5TUrePH2P2ma4hYJWHZg3zMGnub9pQw4WOEJj/zeEdw18oNXzHxz7sI3QoWqHtXLe9zSi+efcUpPecjyhy8o1wyzFehpb4T/HZBECvOGEdeAlprAEmQN0ubKo255tAvvOBdzFiRubsz1mVvbM5lNgeKCrEch9pFobiyAtTH64l75P+hG1vGgkh5HK7zr5YG75Gsx3mbqoyW93gFojQcyrJrcehoKmnc40r8pBuFXMHPIemjB7W88+q75TdSF6xfA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Jun Nakajima <jun.nakajima@xxxxxxxxx>, Kevin Tian <kevin.tian@xxxxxxxxx>, Tim Deegan <tim@xxxxxxx>, Tamas K Lengyel <tamas@xxxxxxxxxxxxx>, Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>, Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Wed, 30 Aug 2023 13:31:56 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 24.08.2023 17:26, Jinoh Kang wrote:
> From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> 
> All #DB exceptions result in an update of %dr6, but this isn't captured in
> Xen's handling.
> 
> PV guests generally work by modifying %dr6 before raising #DB, whereas HVM
> guests do nothing and have a single-step special case in the lowest levels of
> {vmx,svm}_inject_event().  All of this is buggy, but in particular, task
> switches with the trace flag never end up signalling BT in %dr6.
> 
> To begin resolving this issue, add a new pending_dbg field to x86_event
> (unioned with cr2 to avoid taking any extra space), and introduce
> {pv,hvm}_inject_debug_exn() helpers to replace the current callers using
> {pv,hvm}_inject_hw_exception().
> 
> A key property is that pending_dbg is taken with positive polarity to deal
> with RTM sensibly.  Most callers pass in a constant, but callers passing in a
> hardware %dr6 value need to xor the value with X86_DR6_DEFAULT to flip the
> polarity of RTM and reserved fields.
> 
> For PV guests, move the ad-hoc updating of %dr6 into pv_inject_event().  This
> in principle breaks the handing of RTM in do_debug(), but PV guests can't
> actually enable MSR_DEBUGCTL.RTM yet, so this doesn't matter in practice.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> [ jinoh: Rebase onto staging, forward declare struct vcpu ]
> Signed-off-by: Jinoh Kang <jinoh.kang.kr@xxxxxxxxx>
> ---
> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Wei Liu <wl@xxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
> CC: Kevin Tian <kevin.tian@xxxxxxxxx>
> CC: Tim Deegan <tim@xxxxxxx>
> CC: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> CC: Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>
> CC: Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>
> 
> v1 -> v2: [S-o-b fixes. More details below.]
> 
> - Update DR6 for gdbsx when trapped in PV guest kernel mode

I take it that this refers to ...

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -1989,17 +1989,17 @@ void do_debug(struct cpu_user_regs *regs)
>          return;
>      }
>  
> -    /* Save debug status register where guest OS can peek at it */
> -    v->arch.dr6 |= (dr6 & ~X86_DR6_DEFAULT);
> -    v->arch.dr6 &= (dr6 | ~X86_DR6_DEFAULT);
> -
>      if ( guest_kernel_mode(v, regs) && v->domain->debugger_attached )
>      {
> +        /* Save debug status register where gdbsx can peek at it */
> +        v->arch.dr6 |= (dr6 & ~X86_DR6_DEFAULT);
> +        v->arch.dr6 &= (dr6 | ~X86_DR6_DEFAULT);
> +
>          domain_pause_for_debugger();
>          return;
>      }

... this code movement. I'm afraid this should have resulted in you
dropping the earlier R-b, and I'm further afraid I'm not convinced 
this is correct, despite seeing why you would want to do this. The
issue is that this way you also alter guest-visible state, when the
intention is that such now happen via ...

> -    pv_inject_hw_exception(X86_EXC_DB, X86_EVENT_NO_EC);
> +    pv_inject_debug_exn(dr6 ^ X86_DR6_DEFAULT);
>  }

... this call alone. I fear I can't currently see how to get both
aspects right, other than by breaking up 

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.