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

Re: [RFC 6/6] capabilities: convert attach debugger into a capability



Hi Daniel,

On 01/08/2023 21:20, Daniel P. Smith wrote:
Expresses the ability to attach a debugger as a capability that a domain can be
provisioned.

Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
---
  xen/arch/x86/hvm/svm/svm.c      |  8 ++++----
  xen/arch/x86/hvm/vmx/realmode.c |  2 +-
  xen/arch/x86/hvm/vmx/vmcs.c     |  2 +-
  xen/arch/x86/hvm/vmx/vmx.c      | 10 +++++-----
  xen/arch/x86/traps.c            |  6 ++++--
  xen/common/domctl.c             |  6 ++++--
  xen/include/xen/sched.h         |  9 ++++++---
  7 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 27170213ae..9872804d39 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -999,7 +999,7 @@ static void noreturn cf_check svm_do_resume(void)
  {
      struct vcpu *v = current;
      struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
-    bool debug_state = (v->domain->debugger_attached ||
+    bool debug_state = (domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) ||
                          v->domain->arch.monitor.software_breakpoint_enabled ||
                          v->domain->arch.monitor.debug_exception_enabled);
      bool_t vcpu_guestmode = 0;
@@ -1335,7 +1335,7 @@ static void cf_check svm_inject_event(const struct 
x86_event *event)
          }
          /* fall through */
      case X86_EXC_BP:
-        if ( curr->domain->debugger_attached )
+        if ( domain_has_cap(curr->domain, CAP_DEBUGGER_ATTACH) )
          {
              /* Debug/Int3: Trap to debugger. */
              domain_pause_for_debugger();
@@ -2732,7 +2732,7 @@ void svm_vmexit_handler(void)
case VMEXIT_ICEBP:
      case VMEXIT_EXCEPTION_DB:
-        if ( !v->domain->debugger_attached )
+        if ( !domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) )
          {
              unsigned int trap_type;
@@ -2769,7 +2769,7 @@ void svm_vmexit_handler(void)
          if ( insn_len == 0 )
               break;
- if ( v->domain->debugger_attached )
+        if ( domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) )
          {
              /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update 
RIP. */
              __update_guest_eip(regs, insn_len);
diff --git a/xen/arch/x86/hvm/vmx/realmode.c b/xen/arch/x86/hvm/vmx/realmode.c
index ff44ddcfa6..f761026a9d 100644
--- a/xen/arch/x86/hvm/vmx/realmode.c
+++ b/xen/arch/x86/hvm/vmx/realmode.c
@@ -121,7 +121,7 @@ void vmx_realmode_emulate_one(struct hvm_emulate_ctxt 
*hvmemul_ctxt)
if ( rc == X86EMUL_EXCEPTION )
      {
-        if ( unlikely(curr->domain->debugger_attached) &&
+        if ( unlikely(domain_has_cap(curr->domain, CAP_DEBUGGER_ATTACH)) &&
               ((hvmemul_ctxt->ctxt.event.vector == X86_EXC_DB) ||
                (hvmemul_ctxt->ctxt.event.vector == X86_EXC_BP)) )
          {
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 13719cc923..9474869018 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1912,7 +1912,7 @@ void cf_check vmx_do_resume(void)
          hvm_asid_flush_vcpu(v);
      }
- debug_state = v->domain->debugger_attached
+    debug_state = domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH)
                    || v->domain->arch.monitor.software_breakpoint_enabled
                    || v->domain->arch.monitor.singlestep_enabled;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 7ec44018d4..5069e3cbf3 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2041,7 +2041,7 @@ static void cf_check vmx_inject_event(const struct 
x86_event *event)
              break;
          /* fall through */
      case X86_EXC_BP:
-        if ( curr->domain->debugger_attached )
+        if ( domain_has_cap(curr->domain, CAP_DEBUGGER_ATTACH) )
          {
              /* Debug/Int3: Trap to debugger. */
              domain_pause_for_debugger();
@@ -2121,7 +2121,7 @@ static void cf_check vmx_set_info_guest(struct vcpu *v)
       * immediately vmexit and hence make no progress.
       */
      __vmread(GUEST_INTERRUPTIBILITY_INFO, &intr_shadow);
-    if ( v->domain->debugger_attached &&
+    if ( domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) &&
           (v->arch.user_regs.eflags & X86_EFLAGS_TF) &&
           (intr_shadow & VMX_INTR_SHADOW_STI) )
      {
@@ -4283,7 +4283,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
                  }
              }
- if ( !v->domain->debugger_attached )
+            if ( !domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) )
              {
                  unsigned long insn_len = 0;
                  int rc;
@@ -4307,7 +4307,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
              break;
          case X86_EXC_BP:
              HVMTRACE_1D(TRAP, vector);
-            if ( !v->domain->debugger_attached )
+            if ( !domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) )
              {
                  unsigned long insn_len;
                  int rc;
@@ -4647,7 +4647,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
                                HVM_MONITOR_SINGLESTEP_BREAKPOINT,
                                0, 0, 0);
- if ( v->domain->debugger_attached )
+            if ( domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) )
                  domain_pause_for_debugger();
          }
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 4229bda159..041ced35ea 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1214,7 +1214,8 @@ void do_int3(struct cpu_user_regs *regs)
          return;
      }
- if ( guest_kernel_mode(curr, regs) && curr->domain->debugger_attached )
+    if ( guest_kernel_mode(curr, regs) &&
+         domain_has_cap(curr->domain, CAP_DEBUGGER_ATTACH) )
      {
          curr->arch.gdbsx_vcpu_event = X86_EXC_BP;
          domain_pause_for_debugger();
@@ -1995,7 +1996,8 @@ void do_debug(struct cpu_user_regs *regs)
      v->arch.dr6 |= (dr6 & ~X86_DR6_DEFAULT);
      v->arch.dr6 &= (dr6 | ~X86_DR6_DEFAULT);
- if ( guest_kernel_mode(v, regs) && v->domain->debugger_attached )
+    if ( guest_kernel_mode(v, regs) &&
+         domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) )
      {
          domain_pause_for_debugger();
          return;
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 505e29c0dc..895ddf0600 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -99,7 +99,8 @@ void getdomaininfo(struct domain *d, struct 
xen_domctl_getdomaininfo *info)
          ((d->is_dying == DOMDYING_dead) ? XEN_DOMINF_dying     : 0) |
          (d->is_shut_down                ? XEN_DOMINF_shutdown  : 0) |
          (d->controller_pause_count > 0  ? XEN_DOMINF_paused    : 0) |
-        (d->debugger_attached           ? XEN_DOMINF_debugged  : 0) |
+        (domain_has_cap(d, CAP_DEBUGGER_ATTACH) ?
+                                          XEN_DOMINF_debugged  : 0) |
          (is_xenstore_domain(d)          ? XEN_DOMINF_xs_domain : 0) |
          (is_hvm_domain(d)               ? XEN_DOMINF_hvm_guest : 0) |
          d->shutdown_code << XEN_DOMINF_shutdownshift;
@@ -643,7 +644,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
u_domctl)
          else
          {
              domain_pause(d);
-            d->debugger_attached = !!op->u.setdebugging.enable;
+            if ( !!op->u.setdebugging.enable )
+                domain_set_cap(d, CAP_DEBUGGER_ATTACH);

From my understanding, before this patch, it was possible to detach the debugger. But now, you don't seem to allow clearing. Is it intended? If so, can you explain why? (The outcome would want to be written down in the commit message).

              domain_unpause(d); /* causes guest to latch new status */
          }
          break;
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index ebfe65cd73..47eadb5008 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -474,9 +474,8 @@ struct domain
      uint8_t          role;
  #define CAP_CONSOLE_IO         (1U<<0)
  #define CAP_DISABLE_CPU_FAULT  (1U<<1)
-    uint8_t          capabilities;
-    /* Is this guest being debugged by dom0? */
-    bool             debugger_attached;
+#define CAP_DEBUGGER_ATTACH    (1U<<2)
+    uint16_t         capabilities;
      /*
       * Set to true at the very end of domain creation, when the domain is
       * unpaused for the first time by the systemcontroller.
@@ -1166,6 +1165,10 @@ static always_inline bool domain_set_cap(
          if ( is_pv_domain(d) && is_control_domain(d) )
              d->capabilities |= cap;
          break;
+    case CAP_DEBUGGER_ATTACH:
+        if ( !is_control_domain(d) )

This seems to be a new restriction. Can you explain why?

+            d->capabilities |= cap;
+        break;
      default:
          return false;
      }

Cheers,

--
Julien Grall



 


Rackspace

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