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

Commit Message

Bobby Eshleman Aug. 18, 2021, 8:29 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]>
---
 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       | 17 +++++++++++++++++
 5 files changed, 29 insertions(+), 20 deletions(-)
 rename xen/arch/x86/{debug.c => gdbsx.c} (93%)
 create mode 100644 xen/include/asm-x86/gdbsx.h

Comments

Andrew Cooper Aug. 24, 2021, 11:31 a.m. UTC | #1

On 18/08/2021 21:29, Bobby Eshleman wrote:
> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index fe38cfd544..ef8c2c4770 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -20,7 +20,7 @@ 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-$(CONFIG_GDBSX) += gdbsx.o

This wants moving now to retain alphabetical order.

> diff --git a/xen/include/asm-x86/gdbsx.h b/xen/include/asm-x86/gdbsx.h
> new file mode 100644
> index 0000000000..2b8812881d
> --- /dev/null
> +++ b/xen/include/asm-x86/gdbsx.h
> @@ -0,0 +1,17 @@
> +#ifndef __X86_GDBX_H
> +#define __X86_GDBX_H__

Broken header guard.

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

Need to include <xen/errno.h> to avoid breaking the !GDBSX build.

Can fix all on commit.

~Andrew

Jan Beulich Aug. 24, 2021, 12:19 p.m. UTC | #2

On 18.08.2021 22:29, Bobby Eshleman wrote:
> --- /dev/null
> +++ b/xen/include/asm-x86/gdbsx.h
> @@ -0,0 +1,17 @@
> +#ifndef __X86_GDBX_H
> +#define __X86_GDBX_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

In addition to what Andrew has said, you also want to make sure
- domid_t is actually declared (need to include public/xen.h, I think),
- struct xen_domctl_gdbsx_memio has a forward declaration (ahead of the
  #ifdef).
This is so the header can actually be #include-d without needing to
worry about prereq #include-s.

Jan

Bobby Eshleman Aug. 24, 2021, 2:32 p.m. UTC | #3

On Tue, Aug 24, 2021, 2:20 PM Jan Beulich <[e-mail]> wrote:

> On 18.08.2021 22:29, Bobby Eshleman wrote:
> > --- /dev/null
> > +++ b/xen/include/asm-x86/gdbsx.h
> > @@ -0,0 +1,17 @@
> > +#ifndef __X86_GDBX_H
> > +#define __X86_GDBX_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
>
> In addition to what Andrew has said, you also want to make sure
> - domid_t is actually declared (need to include public/xen.h, I think),
> - struct xen_domctl_gdbsx_memio has a forward declaration (ahead of the
>   #ifdef).
> This is so the header can actually be #include-d without needing to
> worry about prereq #include-s.
>
> Jan
>

Roger that.

I'll be away from work on vacation until mid September but I'll get to
these changes and the others when I get back, if there are some pending /
still uncommitted.

Writing this from my phone, so sorry for gmail's email strangeness.

>

Patch

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index fe38cfd544..ef8c2c4770 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -20,7 +20,7 @@  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-$(CONFIG_GDBSX) += gdbsx.o
 obj-y += delay.o
 obj-y += desc.o
 obj-bin-y += dmi_scan.init.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..2b8812881d
--- /dev/null
+++ b/xen/include/asm-x86/gdbsx.h
@@ -0,0 +1,17 @@ 
+#ifndef __X86_GDBX_H
+#define __X86_GDBX_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