[v3,4/6] x86/gdbsx: expand dbg_rw_mem() inline

Commit Message

Bobby Eshleman Aug. 18, 2021, 8:29 p.m. UTC

Because dbg_rw_mem() has only a single call site, this commit
expands it inline.
---
 xen/arch/x86/gdbsx.c | 32 +++++++++++---------------------
 1 file changed, 11 insertions(+), 21 deletions(-)

Comments

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

On 18/08/2021 21:29, Bobby Eshleman wrote:
> Because dbg_rw_mem() has only a single call site, this commit
> expands it inline.

Missing a SoB.

Otherwise, looks ok.

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

On 18.08.2021 22:29, Bobby Eshleman wrote:
> --- a/xen/arch/x86/gdbsx.c
> +++ b/xen/arch/x86/gdbsx.c
> @@ -151,33 +151,23 @@ static unsigned int dbg_rw_guest_mem(struct domain *dp, unsigned long addr,
>      return len;
>  }
>  
> -/*
> - * addr is guest addr
> - * buf is debugger buffer.
> - * if toaddr, then addr = buf (write to addr), else buf = addr (rd from guest)
> - * pgd3: value of init_mm.pgd[3] in guest. see above.
> - * Returns: number of bytes remaining to be copied.
> - */
> -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)
> +int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
>  {
>      struct domain *d = rcu_lock_domain_by_id(domid);
>  
> -    if ( d )
> +    if ( d && !d->is_dying )
>      {
> -        if ( !d->is_dying )
> -            len = dbg_rw_guest_mem(d, gva, buf, len, toaddr, pgd3);
> -        rcu_unlock_domain(d);
> +        iop->remain = dbg_rw_guest_mem(
> +                d, iop->gva, guest_handle_from_ptr(iop->uva, void),
> +                iop->len, domid, iop->pgd3val);
> +    }
> +    else
> +    {
> +        iop->remain = iop->len;
>      }

Nit: Generally we omit the braces in cases like this one.

Jan

Patch

diff --git a/xen/arch/x86/gdbsx.c b/xen/arch/x86/gdbsx.c
index adea0f017b..4cb8e042f9 100644
--- a/xen/arch/x86/gdbsx.c
+++ b/xen/arch/x86/gdbsx.c
@@ -151,33 +151,23 @@  static unsigned int dbg_rw_guest_mem(struct domain *dp, unsigned long addr,
     return len;
 }
 
-/*
- * addr is guest addr
- * buf is debugger buffer.
- * if toaddr, then addr = buf (write to addr), else buf = addr (rd from guest)
- * pgd3: value of init_mm.pgd[3] in guest. see above.
- * Returns: number of bytes remaining to be copied.
- */
-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)
+int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
 {
     struct domain *d = rcu_lock_domain_by_id(domid);
 
-    if ( d )
+    if ( d && !d->is_dying )
     {
-        if ( !d->is_dying )
-            len = dbg_rw_guest_mem(d, gva, buf, len, toaddr, pgd3);
-        rcu_unlock_domain(d);
+        iop->remain = dbg_rw_guest_mem(
+                d, iop->gva, guest_handle_from_ptr(iop->uva, void),
+                iop->len, domid, iop->pgd3val);
+    }
+    else
+    {
+        iop->remain = iop->len;
     }
 
-    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);
+    if ( d )
+        rcu_unlock_domain(d);
 
     return iop->remain ? -EFAULT : 0;
 }