x86: Fix XEN_DOMCTL_gdbsx_guestmemio crash

Prior to commit 9cd7e31b3f58 ("xen/iommu: cleanup iommu related domctl
handling"), when CONFIG_GDBSX was compiled out, iommu_do_domctl() fell over a
NULL pointer.  One of several bugs here is known-but-compiled-out subops
falling into the default chain and hitting unrelated logic.

Remove the CONFIG_GDBSX ifdefary in arch_do_domctl() by implementing
gdbsx_domctl() and moving the logic across.

As minor cleanup,
 * gdbsx_guest_mem_io() can become static
 * Remove opencoding of domain_vcpu() and %pd

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 1a8b4cff48ee699abb173d7259ad410deecc5b41..3c0ea8554107e50241d4f23b42d390a00830e322 100644 (file)
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -814,71 +814,12 @@ long arch_do_domctl(
     }
 #endif
 
-#ifdef CONFIG_GDBSX
     case XEN_DOMCTL_gdbsx_guestmemio:
-        ret = gdbsx_guest_mem_io(d, &domctl->u.gdbsx_guest_memio);
-        if ( !ret )
-           copyback = true;
-        break;
-
     case XEN_DOMCTL_gdbsx_pausevcpu:
-    {
-        struct vcpu *v;
-
-        ret = -EBUSY;
-        if ( !d->controller_pause_count )
-            break;
-        ret = -EINVAL;
-        if ( domctl->u.gdbsx_pauseunp_vcpu.vcpu >= d->max_vcpus ||
-             (v = d->vcpu[domctl->u.gdbsx_pauseunp_vcpu.vcpu]) == NULL )
-            break;
-        ret = vcpu_pause_by_systemcontroller(v);
-        break;
-    }
-
     case XEN_DOMCTL_gdbsx_unpausevcpu:
-    {
-        struct vcpu *v;
-
-        ret = -EBUSY;
-        if ( !d->controller_pause_count )
-            break;
-        ret = -EINVAL;
-        if ( domctl->u.gdbsx_pauseunp_vcpu.vcpu >= d->max_vcpus ||
-             (v = d->vcpu[domctl->u.gdbsx_pauseunp_vcpu.vcpu]) == NULL )
-            break;
-        ret = vcpu_unpause_by_systemcontroller(v);
-        if ( ret == -EINVAL )
-            printk(XENLOG_G_WARNING
-                   "WARN: d%d attempting to unpause %pv which is not paused\n",
-                   currd->domain_id, v);
-        break;
-    }
-
     case XEN_DOMCTL_gdbsx_domstatus:
-    {
-        struct vcpu *v;
-
-        domctl->u.gdbsx_domstatus.vcpu_id = -1;
-        domctl->u.gdbsx_domstatus.paused = d->controller_pause_count > 0;
-        if ( domctl->u.gdbsx_domstatus.paused )
-        {
-            for_each_vcpu ( d, v )
-            {
-                if ( v->arch.gdbsx_vcpu_event )
-                {
-                    domctl->u.gdbsx_domstatus.vcpu_id = v->vcpu_id;
-                    domctl->u.gdbsx_domstatus.vcpu_ev =
-                        v->arch.gdbsx_vcpu_event;
-                    v->arch.gdbsx_vcpu_event = 0;
-                    break;
-                }
-            }
-        }
-        copyback = true;
+        ret = gdbsx_domctl(d, domctl, &copyback);
         break;
-    }
-#endif
 
     case XEN_DOMCTL_setvcpuextstate:
     case XEN_DOMCTL_getvcpuextstate:

diff --git a/xen/arch/x86/gdbsx.c b/xen/arch/x86/gdbsx.c
index 6ef46e8ea77d8de3178ff50340bdf4fa2d0b9d95..21442f5dff1aa1ba64eeb2d68f33920b59bfbfd3 100644 (file)
--- a/xen/arch/x86/gdbsx.c
+++ b/xen/arch/x86/gdbsx.c
@@ -152,7 +152,8 @@ static unsigned int dbg_rw_guest_mem(struct domain *dp, unsigned long addr,
     return len;
 }
 
-int gdbsx_guest_mem_io(struct domain *d, struct xen_domctl_gdbsx_memio *iop)
+static int gdbsx_guest_mem_io(
+    struct domain *d, struct xen_domctl_gdbsx_memio *iop)
 {
     if ( d && !d->is_dying )
     {
@@ -178,6 +179,73 @@ void domain_pause_for_debugger(void)
         send_global_virq(VIRQ_DEBUGGER);
 }
 
+int gdbsx_domctl(struct domain *d, struct xen_domctl *domctl, bool *copyback)
+{
+    struct vcpu *v;
+    int ret;
+
+    switch ( domctl->cmd )
+    {
+    case XEN_DOMCTL_gdbsx_guestmemio:
+        ret = gdbsx_guest_mem_io(d, &domctl->u.gdbsx_guest_memio);
+        if ( !ret )
+            *copyback = true;
+        break;
+
+    case XEN_DOMCTL_gdbsx_pausevcpu:
+        ret = -EBUSY;
+        if ( !d->controller_pause_count )
+            break;
+        ret = -EINVAL;
+        if ( (v = domain_vcpu(d, domctl->u.gdbsx_pauseunp_vcpu.vcpu)) == NULL )
+            break;
+        ret = vcpu_pause_by_systemcontroller(v);
+        break;
+
+    case XEN_DOMCTL_gdbsx_unpausevcpu:
+        ret = -EBUSY;
+        if ( !d->controller_pause_count )
+            break;
+        ret = -EINVAL;
+        if ( (v = domain_vcpu(d, domctl->u.gdbsx_pauseunp_vcpu.vcpu)) == NULL )
+            break;
+        ret = vcpu_unpause_by_systemcontroller(v);
+        if ( ret == -EINVAL )
+            printk(XENLOG_G_WARNING
+                   "WARN: %pd attempting to unpause %pv which is not paused\n",
+                   current->domain, v);
+        break;
+
+    case XEN_DOMCTL_gdbsx_domstatus:
+        ret = 0;
+        domctl->u.gdbsx_domstatus.vcpu_id = -1;
+        domctl->u.gdbsx_domstatus.paused = d->controller_pause_count > 0;
+        if ( domctl->u.gdbsx_domstatus.paused )
+        {
+            for_each_vcpu ( d, v )
+            {
+                if ( v->arch.gdbsx_vcpu_event )
+                {
+                    domctl->u.gdbsx_domstatus.vcpu_id = v->vcpu_id;
+                    domctl->u.gdbsx_domstatus.vcpu_ev =
+                        v->arch.gdbsx_vcpu_event;
+                    v->arch.gdbsx_vcpu_event = 0;
+                    break;
+                }
+            }
+        }
+        *copyback = true;
+        break;
+
+    default:
+        ASSERT_UNREACHABLE();
+        ret = -ENOSYS;
+        break;
+    }
+
+    return ret;
+}
+
 /*
  * Local variables:
  * mode: C

diff --git a/xen/arch/x86/include/asm/gdbsx.h b/xen/arch/x86/include/asm/gdbsx.h
index 938eb74e2e258c3ce1f692924e797c7c63911ccd..e906be9ea31878c161f991a19ad82b0191f3a30a 100644 (file)
--- a/xen/arch/x86/include/asm/gdbsx.h
+++ b/xen/arch/x86/include/asm/gdbsx.h
@@ -2,18 +2,28 @@
 #ifndef __X86_GDBX_H__
 #define __X86_GDBX_H__
 
-#ifdef CONFIG_GDBSX
+#include <xen/stdbool.h>
 
 struct domain;
-struct xen_domctl_gdbsx_memio;
+struct xen_domctl;
 
-int gdbsx_guest_mem_io(struct domain *d, struct xen_domctl_gdbsx_memio *iop);
+#ifdef CONFIG_GDBSX
 
 void domain_pause_for_debugger(void);
 
+int gdbsx_domctl(struct domain *d, struct xen_domctl *domctl, bool *copyback);
+
 #else
 
+#include <xen/errno.h>
+
 static inline void domain_pause_for_debugger(void) {}
 
+static inline int gdbsx_domctl(
+    struct domain *d, struct xen_domctl *domctl, bool *copyback)
+{
+    return -ENOSYS;
+}
+
 #endif /* CONFIG_GDBSX */
 #endif /* __X86_GDBX_H__ */