[PATCH v6 06/11] xen, x86: factor out map and unmap from the memory_mapping DOMCTL

[Arianna Avanzini <avanzini.arianna@gmail.com>]

This commit factors out from the XEN_DOMCTL_memory_mapping hypercall
implementation, currently available only for x86, the operations
related to memory ranges map and unmap. The code is factored out
into two {map|unmap}_mmio_regions() functions for x86 , that will match
the corresponding pair of ARM functions when the DOMCTL will be moved
to common code in the following commit.

Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Julien Grall <julien.grall@citrix.com>
Cc: Ian Campbell <Ian.Campbell@eu.citrix.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Eric Trudeau <etrudeau@broadcom.com>
Cc: Viktor Kleinik <viktor.kleinik@globallogic.com>

---

    v6:
        - Fix uncorrect usage of [mg]fn_end which, in the initial checks,
          was subtracted 1 twice.
        - Remove useless comment about the handling of errors in the
          remove path of the hypercall.
        - Replace stray hard tab with spaces.

    v5:
        - Let the unmap_mmio_regions() function for x86 return a proper
          error code upon failure.
        - Restore correct handling of errors in the remove path of the
          hypercall, assigning to the "ret" local variable the error
          code returned by the unmap_mmio_regions() function only if
          iomem_deny_access() didn't return with an error.
        - Compute gfn_end - 1 and mfn_end - 1 only once in the DOMCTL.
        - Use a local variable to keep the return value of the function
          unmap_mmio_regions() instead of re-using the "add" variable.
        - Add a comment to make hopefully clearer how error values of
          functions are handled in the remove path of the DOMCTL.

    v4:
        - Fix type and signedness of local variables used as indexes in
          map_mmio_regions() and unmap_mmio_regions() for x86.
        - Clear p2m entries in map_mmio_regions() for x86 only if
          set_mmio_p2m_entry() returned with an error.
        - Make ranges inclusive of the end address in map_mmio_regions()
          and unmap_mmio_regions() for x86.
        - Turn hard tabs into spaces.

    v3:
        - Add map_mmio_regions() and unmap_mmio_regions() functions for x86;
        - Use pfn as parameters to the unmap_mmio_regions() function.
        - Compute gfn + nr_mfns and mfn + nr_mfns only once.

    v2:
        - Use the already-defined PADDR_BITS constant in the new DOMCTL.
        - Use paddr_t as arguments to the map_mmio_regions() function.
        - Page-align addresses given as arguments to map_mmio_regions() and
          unmap_mmio_regions().

---
 xen/arch/x86/domctl.c     | 51 ++++++++++++++++++++---------------------------
 xen/arch/x86/mm/p2m.c     | 42 ++++++++++++++++++++++++++++++++++++++
 xen/include/asm-x86/p2m.h | 12 +++++++++++
 3 files changed, 76 insertions(+), 29 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 33e31e3..04c6007 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -646,46 +646,45 @@ long arch_do_domctl(
         unsigned long gfn = domctl->u.memory_mapping.first_gfn;
         unsigned long mfn = domctl->u.memory_mapping.first_mfn;
         unsigned long nr_mfns = domctl->u.memory_mapping.nr_mfns;
+        unsigned long mfn_end = mfn + nr_mfns - 1;
+        unsigned long gfn_end = gfn + nr_mfns - 1;
         int add = domctl->u.memory_mapping.add_mapping;
-        unsigned long i;
 
         ret = -EINVAL;
-        if ( (mfn + nr_mfns - 1) < mfn || /* wrap? */
-             ((mfn | (mfn + nr_mfns - 1)) >> (paddr_bits - PAGE_SHIFT)) ||
-             (gfn + nr_mfns - 1) < gfn ) /* wrap? */
-            break;
+        if ( mfn_end < mfn || /* wrap? */
+             ((mfn | mfn_end) >> (paddr_bits - PAGE_SHIFT)) ||
+             gfn_end < gfn ) /* wrap? */
+            return ret;
 
         ret = -EPERM;
-        if ( !iomem_access_permitted(current->domain, mfn, mfn + nr_mfns - 1) )
+        if ( !iomem_access_permitted(current->domain, mfn, mfn_end) )
             break;
 
-        ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn + nr_mfns - 1, add);
+        ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn_end, add);
         if ( ret )
             break;
 
         if ( add )
         {
             printk(XENLOG_G_INFO
-                   "memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx\n",
+                   "memory_map: add: dom%d gfn=%lx mfn=%lx nr=%lx\n",
                    d->domain_id, gfn, mfn, nr_mfns);
 
-            ret = iomem_permit_access(d, mfn, mfn + nr_mfns - 1);
-            if ( !ret && paging_mode_translate(d) )
+            ret = iomem_permit_access(d, mfn, mfn_end);
+            if ( !ret )
             {
-                for ( i = 0; !ret && i < nr_mfns; i++ )
-                    ret = set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i));
+                ret = map_mmio_regions(d, gfn, gfn_end, mfn);
                 if ( ret )
                 {
                     printk(XENLOG_G_WARNING
-                           "memory_map:fail: dom%d gfn=%lx mfn=%lx ret:%ld\n",
-                           d->domain_id, gfn + i, mfn + i, ret);
-                    while ( i-- )
-                        clear_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i));
-                    if ( iomem_deny_access(d, mfn, mfn + nr_mfns - 1) &&
+                           "memory_map: fail: dom%d gfn=%lx mfn=%lx ret:%ld\n",
+                           d->domain_id, gfn, mfn, ret);
+                    if ( iomem_deny_access(d, mfn, mfn_end) &&
                          is_hardware_domain(current->domain) )
                         printk(XENLOG_ERR
-                               "memory_map: failed to deny dom%d access to [%lx,%lx]\n",
-                               d->domain_id, mfn, mfn + nr_mfns - 1);
+                               "memory_map: failed to deny dom%d access "
+                               "to [%lx,%lx]\n",
+                               d->domain_id, mfn, mfn_end);
                 }
             }
         }
@@ -694,24 +693,18 @@ long arch_do_domctl(
             int tmp_rc = 0;
 
             printk(XENLOG_G_INFO
-                   "memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
+                   "memory_map: remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
                    d->domain_id, gfn, mfn, nr_mfns);
 
-            if ( paging_mode_translate(d) )
-                for ( i = 0; i < nr_mfns; i++ )
-                {
-                    ret = clear_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i));
-                    if ( ret )
-                        tmp_rc = ret;
-                }
-            ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
+            tmp_rc = unmap_mmio_regions(d, gfn, gfn_end, mfn);
+            ret = iomem_deny_access(d, mfn, mfn_end);
             if ( !ret )
                 ret = tmp_rc;
             if ( ret && is_hardware_domain(current->domain) )
                 printk(XENLOG_ERR
                        "memory_map: error %ld %s dom%d access to [%lx,%lx]\n",
                        ret, tmp_rc ? "removing" : "denying", d->domain_id,
-                       mfn, mfn + nr_mfns - 1);
+                       mfn, mfn_end);
         }
         /* Do this unconditionally to cover errors on above failure paths. */
         memory_type_changed(d);
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 8489482..a112fe4 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1657,6 +1657,48 @@ unsigned long paging_gva_to_gfn(struct vcpu *v,
     return hostmode->gva_to_gfn(v, hostp2m, va, pfec);
 }
 
+int map_mmio_regions(struct domain *d,
+                     unsigned long start_gfn,
+                     unsigned long end_gfn,
+                     unsigned long mfn)
+{
+    int ret = 0;
+    unsigned long i;
+    unsigned long nr_mfns = end_gfn - start_gfn + 1;
+
+    if ( !paging_mode_translate(d) )
+        return 0;
+
+    for ( i = 0; !ret && i < nr_mfns; i++ )
+        if ( !set_mmio_p2m_entry(d, start_gfn + i, _mfn(mfn + i)) )
+            ret = -EIO;
+    if ( ret )
+        while ( i-- )
+            clear_mmio_p2m_entry(d, start_gfn + i, _mfn(mfn + i));
+
+    return ret;
+}
+
+int unmap_mmio_regions(struct domain *d,
+                       unsigned long start_gfn,
+                       unsigned long end_gfn,
+                       unsigned long mfn)
+{
+    int ret = 0;
+    unsigned long nr_mfns = end_gfn - start_gfn + 1;
+    unsigned long i;
+
+    if ( !paging_mode_translate(d) )
+        return 0;
+
+    for ( i = 0; i < nr_mfns; i++ )
+        ret |= !clear_mmio_p2m_entry(d, start_gfn + i, _mfn(mfn + i));
+
+    if ( ret )
+        return -EIO;
+    return 0;
+}
+
 /*** Audit ***/
 
 #if P2M_AUDIT
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 4a1c129..1098600 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -32,6 +32,18 @@
 #include <asm/mem_sharing.h>
 #include <asm/page.h>    /* for pagetable_t */
 
+/* Map MMIO regions in the p2m: start_gfn and end_gfn is the range, inclusive
+ * of the end_gfn frame number, in the guest physical address space to map,
+ * starting from the machine frame number mfn. */
+int map_mmio_regions(struct domain *d,
+                     unsigned long start_gfn,
+                     unsigned long end_gfn,
+                     unsigned long mfn);
+int unmap_mmio_regions(struct domain *d,
+                       unsigned long start_gfn,
+                       unsigned long end_gfn,
+                       unsigned long mfn);
+
 extern bool_t opt_hap_1gb, opt_hap_2mb;
 
 /*
-- 
1.9.1