[v4,3/6] arch/x86: rename debug.c to gdbsx.c

Commit Message

Bobby Eshleman Sept. 28, 2021, 8:30 p.m. UTC

This commit renames debug.c to gdbsx.c to clarify its purpose.

The function gdbsx_guest_mem_io() is moved from domctl.c to gdbsx.c.

Although gdbsx_guest_mem_io() is conditionally removed from its single
call site in domctl.c upon !CONFIG_GDBSX and so no stub is technically
necessary, this commit adds a stub that would preserve the functioning
of that call site if the #ifdef CONFIG_GDBSX were to ever be removed or
the function were to ever be called outside of such an ifdef block.

Signed-off-by: Bobby Eshleman <[e-mail]>
---
Changes in v4:
- Alphebetize Makefile addition
- Fix broken header guard
- Include errno.h in gdbsx.h

 xen/arch/x86/Makefile             |  2 +-
 xen/arch/x86/domctl.c             | 12 +-----------
 xen/arch/x86/{debug.c => gdbsx.c} | 12 ++++++++++--
 xen/include/asm-x86/debugger.h    |  6 ------
 xen/include/asm-x86/gdbsx.h       | 19 +++++++++++++++++++
 5 files changed, 31 insertions(+), 20 deletions(-)
 rename xen/arch/x86/{debug.c => gdbsx.c} (93%)
 create mode 100644 xen/include/asm-x86/gdbsx.h

Comments

Andrew Cooper Sept. 28, 2021, 9:09 p.m. UTC | #1

On 28/09/2021 21:30, Bobby Eshleman wrote:
> diff --git a/xen/include/asm-x86/gdbsx.h b/xen/include/asm-x86/gdbsx.h
> new file mode 100644
> index 0000000000..473229a7fb
> --- /dev/null
> +++ b/xen/include/asm-x86/gdbsx.h
> @@ -0,0 +1,19 @@
> +#ifndef __X86_GDBX_H__
> +#define __X86_GDBX_H__
> +
> +#include <xen/errno.h>

The errno include wants to move below....

However, you need to avoid latent build errors based on the order of
includes.  I'd include public/domctl.h which will get you both domid_t
and struct xen_domctl_gdbsx_memio.

> +
> +#ifdef CONFIG_GDBSX
> +
> +int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop);
> +
> +#else
> +

... specifically here.

~Andrew

> +static inline int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
> +{
> +    return -EOPNOTSUPP;
> +}
> +
> +#endif
> +
> +#endif

Jan Beulich Sept. 29, 2021, 8:09 a.m. UTC | #2

On 28.09.2021 23:09, Andrew Cooper wrote:
> On 28/09/2021 21:30, Bobby Eshleman wrote:
>> diff --git a/xen/include/asm-x86/gdbsx.h b/xen/include/asm-x86/gdbsx.h
>> new file mode 100644
>> index 0000000000..473229a7fb
>> --- /dev/null
>> +++ b/xen/include/asm-x86/gdbsx.h
>> @@ -0,0 +1,19 @@
>> +#ifndef __X86_GDBX_H__
>> +#define __X86_GDBX_H__
>> +
>> +#include <xen/errno.h>
> 
> The errno include wants to move below....
> 
> However, you need to avoid latent build errors based on the order of
> includes.  I'd include public/domctl.h which will get you both domid_t
> and struct xen_domctl_gdbsx_memio.

public/xen.h should suffice as ...

>> +#ifdef CONFIG_GDBSX
>> +
>> +int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop);
>> +
>> +#else
>> +
> 
> ... specifically here.
> 
> ~Andrew
> 
>> +static inline int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
>> +{
>> +    return -EOPNOTSUPP;
>> +}

... for struct xen_domctl_gdbsx_memio a forward declaration is all that's
needed here.

Jan

Patch

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index fe38cfd544..9fa2ea9aa1 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -20,7 +20,6 @@  obj-y += cpuid.o
 obj-$(CONFIG_PV) += compat.o
 obj-$(CONFIG_PV32) += x86_64/compat.o
 obj-$(CONFIG_KEXEC) += crash.o
-obj-$(CONFIG_GDBSX) += debug.o
 obj-y += delay.o
 obj-y += desc.o
 obj-bin-y += dmi_scan.init.o
@@ -32,6 +31,7 @@  obj-y += emul-i8254.o
 obj-y += extable.o
 obj-y += flushtlb.o
 obj-$(CONFIG_CRASH_DEBUG) += gdbstub.o
+obj-$(CONFIG_GDBSX) += gdbsx.o
 obj-y += hypercall.o
 obj-y += i387.o
 obj-y += i8259.o
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 26a76d2be9..a492fe140e 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -20,6 +20,7 @@ 
 #include <xen/console.h>
 #include <xen/iocap.h>
 #include <xen/paging.h>
+#include <asm/gdbsx.h>
 #include <asm/irq.h>
 #include <asm/hvm/emulate.h>
 #include <asm/hvm/hvm.h>
@@ -33,20 +34,9 @@ 
 #include <public/vm_event.h>
 #include <asm/mem_sharing.h>
 #include <asm/xstate.h>
-#include <asm/debugger.h>
 #include <asm/psr.h>
 #include <asm/cpuid.h>
 
-#ifdef CONFIG_GDBSX
-static int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
-{
-    iop->remain = dbg_rw_mem(iop->gva, guest_handle_from_ptr(iop->uva, void),
-                             iop->len, domid, iop->gwr, iop->pgd3val);
-
-    return iop->remain ? -EFAULT : 0;
-}
-#endif
-
 static int update_domain_cpu_policy(struct domain *d,
                                     xen_domctl_cpu_policy_t *xdpc)
 {
diff --git a/xen/arch/x86/debug.c b/xen/arch/x86/gdbsx.c
similarity index 93%
rename from xen/arch/x86/debug.c
rename to xen/arch/x86/gdbsx.c
index d90dc93056..adea0f017b 100644
--- a/xen/arch/x86/debug.c
+++ b/xen/arch/x86/gdbsx.c
@@ -19,7 +19,7 @@ 
 #include <xen/mm.h>
 #include <xen/domain_page.h>
 #include <xen/guest_access.h>
-#include <asm/debugger.h>
+#include <asm/gdbsx.h>
 #include <asm/p2m.h>
 
 typedef unsigned long dbgva_t;
@@ -158,7 +158,7 @@  static unsigned int dbg_rw_guest_mem(struct domain *dp, unsigned long addr,
  * pgd3: value of init_mm.pgd[3] in guest. see above.
  * Returns: number of bytes remaining to be copied.
  */
-unsigned int dbg_rw_mem(unsigned long gva, XEN_GUEST_HANDLE_PARAM(void) buf,
+static unsigned int dbg_rw_mem(unsigned long gva, XEN_GUEST_HANDLE_PARAM(void) buf,
                         unsigned int len, domid_t domid, bool toaddr,
                         uint64_t pgd3)
 {
@@ -174,6 +174,14 @@  unsigned int dbg_rw_mem(unsigned long gva, XEN_GUEST_HANDLE_PARAM(void) buf,
     return len;
 }
 
+int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
+{
+    iop->remain = dbg_rw_mem(iop->gva, guest_handle_from_ptr(iop->uva, void),
+                             iop->len, domid, iop->gwr, iop->pgd3val);
+
+    return iop->remain ? -EFAULT : 0;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-x86/debugger.h b/xen/include/asm-x86/debugger.h
index cd6b9477f7..ed4d5c829b 100644
--- a/xen/include/asm-x86/debugger.h
+++ b/xen/include/asm-x86/debugger.h
@@ -54,10 +54,4 @@  static inline bool debugger_trap_fatal(
 
 #endif
 
-#ifdef CONFIG_GDBSX
-unsigned int dbg_rw_mem(unsigned long gva, XEN_GUEST_HANDLE_PARAM(void) buf,
-                        unsigned int len, domid_t domid, bool toaddr,
-                        uint64_t pgd3);
-#endif
-
 #endif /* __X86_DEBUGGER_H__ */
diff --git a/xen/include/asm-x86/gdbsx.h b/xen/include/asm-x86/gdbsx.h
new file mode 100644
index 0000000000..473229a7fb
--- /dev/null
+++ b/xen/include/asm-x86/gdbsx.h
@@ -0,0 +1,19 @@ 
+#ifndef __X86_GDBX_H__
+#define __X86_GDBX_H__
+
+#include <xen/errno.h>
+
+#ifdef CONFIG_GDBSX
+
+int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop);
+
+#else
+
+static inline int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
+{
+    return -EOPNOTSUPP;
+}
+
+#endif
+
+#endif