[PATCH v5 5/8] xen, common: add the XEN_DOMCTL_memory_mapping hypercall

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

This commit introduces a first attempt of implementation of the
XEN_DOMCTL_memory_mapping hypercall for ARM. As the implementation
would have been almost identical to the one for x86, this code has
been made common to both the architectures. The only difference
between the two procedures lies in the arch-specific implementation
of the map_mmio_regions() and unmap_mmio_regions() functions.

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: 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>

---

    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.
        - Rename new header to p2m-common.h.

    v4:
        - Use a define for paddr_bits instead of a new variable.
        - Define prototypes for common functions map_mmio_regions() and
          unmap_mmio_regions() only once in a common header.
        - 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:
        - Move code to xen/common/domctl.c; abstract out differences between
          the x86 and ARM code:
          . add map_mmio_regions() and unmap_mmio_regions() functions for x86;
          . add a paddr_bits variable for ARM.
        - Use pfn as parameters to the unmap_mmio_regions() function.
        - Compute gfn + nr_mfns and mfn + nr_mfns only once.

    v2:
        - Move code to xen/arm/domctl.c.
        - 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/arm/p2m.c           | 12 ++++++++
 xen/arch/x86/domctl.c        | 70 -------------------------------------------
 xen/arch/x86/mm/p2m.c        | 42 ++++++++++++++++++++++++++
 xen/common/domctl.c          | 71 ++++++++++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/p2m.h    |  9 ++----
 xen/include/asm-x86/p2m.h    |  1 +
 xen/include/xen/p2m-common.h | 16 ++++++++++
 7 files changed, 145 insertions(+), 76 deletions(-)
 create mode 100644 xen/include/xen/p2m-common.h

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 110b63a..cc71a5d 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -529,6 +529,18 @@ int map_mmio_regions(struct domain *d,
                              MATTR_DEV, p2m_mmio_direct);
 }
 
+int unmap_mmio_regions(struct domain *d,
+                       unsigned long start_gfn,
+                       unsigned long end_gfn,
+                       unsigned long mfn)
+{
+    return apply_p2m_changes(d, REMOVE,
+                             pfn_to_paddr(start_gfn),
+                             pfn_to_paddr(end_gfn),
+                             pfn_to_paddr(mfn),
+                             MATTR_DEV, p2m_mmio_direct);
+}
+
 int guest_physmap_add_entry(struct domain *d,
                             unsigned long gpfn,
                             unsigned long mfn,
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 1e51289..69a7fbf 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -639,76 +639,6 @@ long arch_do_domctl(
     }
     break;
 
-    case XEN_DOMCTL_memory_mapping:
-    {
-        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;
-        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;
-
-        ret = -EPERM;
-        if ( !iomem_access_permitted(current->domain, mfn, mfn + nr_mfns - 1) )
-            break;
-
-        ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn + nr_mfns - 1, add);
-        if ( ret )
-            break;
-
-        if ( add )
-        {
-            printk(XENLOG_G_INFO
-                   "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) )
-            {
-                for ( i = 0; !ret && i < nr_mfns; i++ )
-                    if ( !set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i)) )
-                        ret = -EIO;
-                if ( ret )
-                {
-                    printk(XENLOG_G_WARNING
-                           "memory_map:fail: dom%d gfn=%lx mfn=%lx\n",
-                           d->domain_id, gfn + i, mfn + i);
-                    while ( i-- )
-                        clear_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i));
-                    if ( iomem_deny_access(d, mfn, mfn + nr_mfns - 1) &&
-                         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);
-                }
-            }
-        }
-        else
-        {
-            printk(XENLOG_G_INFO
-                   "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++ )
-                    add |= !clear_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i));
-            ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
-            if ( !ret && add )
-                ret = -EIO;
-            if ( ret && is_hardware_domain(current->domain) )
-                printk(XENLOG_ERR
-                       "memory_map: error %ld %s dom%d access to [%lx,%lx]\n",
-                       ret, add ? "removing" : "denying", d->domain_id,
-                       mfn, mfn + nr_mfns - 1);
-        }
-    }
-    break;
-
     case XEN_DOMCTL_ioport_mapping:
     {
 #define MAX_IOPORTS    0x10000
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 2c894b8..2c6ef24 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1631,6 +1631,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/common/domctl.c b/xen/common/domctl.c
index 5342e5d..fe228fb 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -818,6 +818,77 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
     }
     break;
 
+    case XEN_DOMCTL_memory_mapping:
+    {
+        unsigned long gfn = op->u.memory_mapping.first_gfn;
+        unsigned long mfn = op->u.memory_mapping.first_mfn;
+        unsigned long nr_mfns = op->u.memory_mapping.nr_mfns;
+        unsigned long mfn_end = mfn + nr_mfns - 1;
+        unsigned long gfn_end = gfn + nr_mfns - 1;
+        int add = op->u.memory_mapping.add_mapping;
+
+        ret = -EINVAL;
+        if ( (mfn_end - 1) < mfn || /* wrap? */
+             ((mfn | (mfn_end - 1)) >> (paddr_bits - PAGE_SHIFT)) ||
+             (gfn_end - 1) < gfn ) /* wrap? */
+            return ret;
+
+        ret = -EPERM;
+        if ( !iomem_access_permitted(current->domain, mfn, mfn_end) )
+            return ret;
+
+        ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn_end, add);
+        if ( ret )
+            return ret;
+
+        if ( add )
+        {
+            printk(XENLOG_G_INFO
+                   "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_end);
+            if ( !ret )
+            {
+                ret = map_mmio_regions(d, gfn, gfn_end, mfn);
+                if ( ret )
+                {
+                    printk(XENLOG_G_WARNING
+                           "memory_map: fail: dom%d gfn=%lx mfn=%lx\n",
+                           d->domain_id, gfn, mfn);
+                    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_end);
+                }
+            }
+        }
+        else
+        {
+	    long unmap_ret;
+
+            printk(XENLOG_G_INFO
+                   "memory_map: remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
+                   d->domain_id, gfn, mfn, nr_mfns);
+
+            unmap_ret = unmap_mmio_regions(d, gfn, gfn_end, mfn);
+            ret = iomem_deny_access(d, mfn, mfn_end);
+            /*
+             * Let an error value returned by iomem_deny_access() prevail on
+             * the one possibly returned by unmap_mmio_regions().
+             */
+            if ( !ret && unmap_ret )
+                ret = unmap_ret;
+            if ( ret && is_hardware_domain(current->domain) )
+                printk(XENLOG_ERR
+                       "memory_map: error %ld %s dom%d access to [%lx,%lx]\n",
+                       ret, unmap_ret ? "removing" : "denying", d->domain_id,
+                       mfn, mfn_end);
+        }
+    }
+    break;
+
     case XEN_DOMCTL_settimeoffset:
     {
         domain_set_time_offset(d, op->u.settimeoffset.time_offset_seconds);
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index c7dd6aa..e9ece8d 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -2,6 +2,9 @@
 #define _XEN_P2M_H
 
 #include <xen/mm.h>
+#include <xen/p2m-common.h>
+
+#define paddr_bits PADDR_BITS
 
 struct domain;
 
@@ -84,12 +87,6 @@ int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn);
 
 /* Setup p2m RAM mapping for domain d from start-end. */
 int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);
-/* Map MMIO regions in the p2m: start_gfn and end_gfn is the range 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 guest_physmap_add_entry(struct domain *d,
                             unsigned long gfn,
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index c403534..af53816 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -29,6 +29,7 @@
 
 #include <xen/config.h>
 #include <xen/paging.h>
+#include <xen/p2m-common.h>
 #include <asm/mem_sharing.h>
 #include <asm/page.h>    /* for pagetable_t */
 
diff --git a/xen/include/xen/p2m-common.h b/xen/include/xen/p2m-common.h
new file mode 100644
index 0000000..2be9a86
--- /dev/null
+++ b/xen/include/xen/p2m-common.h
@@ -0,0 +1,16 @@
+#ifndef _XEN_P2M_COMMON_H
+#define _XEN_P2M_COMMON_H
+
+/* Map MMIO regions in the p2m: start_gfn and end_gfn is the range
+ * 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);
+
+#endif /* _XEN_P2M_COMMON_H */
-- 
1.9.1