[Xen-devel] [PATCH v10 06/12] xen/x86: factor out map and unmap from the memory_mapping DOMCTL

[Arianna Avanzini <avanzini.arianna@xxxxxxxxx>]

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. This commit also adds an
unmap_mmio_regions() function for ARM so that the following transition
to common code is cleaner.

Signed-off-by: Arianna Avanzini <avanzini.arianna@xxxxxxxxx>
Acked-by: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
Cc: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
Cc: Paolo Valente <paolo.valente@xxxxxxxxxx>
Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Cc: Julien Grall <julien.grall@xxxxxxxxxx>
Cc: Jan Beulich <JBeulich@xxxxxxxx>
Cc: Keir Fraser <keir@xxxxxxx>
Cc: Tim Deegan <tim@xxxxxxx>
Cc: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Cc: Eric Trudeau <etrudeau@xxxxxxxxxxxx>
Cc: Viktor Kleinik <viktor.kleinik@xxxxxxxxxxxxxxx>

---

    v10:
        - Address coding-style issues in unmap_mmio_regions() for x86.

    v9:
        - Don't ignore an error if it doesn't happen while unmapping the last
          I/O-memory mapping in unmap_mmio_regions() for x86.
        - Fix wrong argument in the call to unmap_mmio_regions() performed on
          failure by map_mmio_regions() for x86.
        - Explicitly specify nr_mfns in the commit message emitted when the
          function map_mmio_regions() fails in the DOMCTL handling code.
        - Change "nr_mfns" to "nr" in the arch-specific map/unmap functions
          for x86.

    v8:
        - Latch the ret variable in {map|unmap}_mmio_regions() to the
          first or last errors encountered during mapping or unmapping
          operations.
        - Separate cleanup-related changes from refactoring ones.
        - Use correct count in unmap_mmio_regions().

    v7:
        - Use unmap_mmio_regions() for the error recovery path in
          map_mmio_regions().
        - Fix wrong use of memory-handling functions.
        - Use mfn_t as last parameter to {map|unmap}_mmio_regions().
        - Don't split format strings.
        - Remove spurious changes, such as those to already-existing format
          strings that are not necessary.

    v6:
        - Fix uncorrect usage of [mg]fn_end which, in the initial checks,
          was subtracted 1 twice.
        - Pass p2m_invalid as last parameter to unmap_mmio_regions() for ARM
          as it is not (and currently must not be) used.
        - 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/arm/p2m.c        | 12 ++++++++++++
 xen/arch/x86/domctl.c     | 19 +++++--------------
 xen/arch/x86/mm/p2m.c     | 45 +++++++++++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/p2m.h |  4 ++++
 xen/include/asm-x86/p2m.h | 12 ++++++++++++
 5 files changed, 78 insertions(+), 14 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index ffce7d6..753c8d6 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -878,6 +878,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 nr_mfns,
+                       unsigned long mfn)
+{
+    return apply_p2m_changes(d, REMOVE,
+                             pfn_to_paddr(start_gfn),
+                             pfn_to_paddr(start_gfn + nr_mfns),
+                             pfn_to_paddr(mfn),
+                             MATTR_DEV, p2m_invalid);
+}
+
 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 9cdbc3d..caa3be6 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -670,17 +670,14 @@ long arch_do_domctl(
                    d->domain_id, gfn, mfn, nr_mfns);
 
             ret = iomem_permit_access(d, mfn, mfn_end);
-            if ( !ret && paging_mode_translate(d) )
+            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, nr_mfns, 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));
+                           "memory_map:fail: dom%d gfn=%lx mfn=%lx nr=%lx 
ret:%ld\n",
+                           d->domain_id, gfn, mfn, nr_mfns, ret);
                     if ( iomem_deny_access(d, mfn, mfn_end) &&
                          is_hardware_domain(current->domain) )
                         printk(XENLOG_ERR
@@ -697,13 +694,7 @@ long arch_do_domctl(
                    "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 )
-                        rc = ret;
-                }
+            rc = unmap_mmio_regions(d, gfn, nr_mfns, mfn);
             ret = iomem_deny_access(d, mfn, mfn_end);
             if ( !ret )
                 ret = rc;
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 297c940..116d895 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1722,6 +1722,51 @@ 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 nr,
+                     unsigned long mfn)
+{
+    int ret = 0;
+    unsigned long i;
+
+    if ( !paging_mode_translate(d) )
+        return 0;
+
+    for ( i = 0; !ret && i < nr; i++ )
+    {
+        ret = set_mmio_p2m_entry(d, start_gfn + i, _mfn(mfn + i));
+        if ( ret )
+        {
+            unmap_mmio_regions(d, start_gfn, i, mfn);
+            break;
+        }
+    }
+
+    return ret;
+}
+
+int unmap_mmio_regions(struct domain *d,
+                       unsigned long start_gfn,
+                       unsigned long nr,
+                       unsigned long mfn)
+{
+    int err = 0;
+    unsigned long i;
+
+    if ( !paging_mode_translate(d) )
+        return 0;
+
+    for ( i = 0; i < nr; i++ )
+    {
+        int ret = clear_mmio_p2m_entry(d, start_gfn + i, _mfn(mfn + i));
+        if ( ret )
+            err = ret;
+    }
+
+    return err;
+}
+
 /*** Audit ***/
 
 #if P2M_AUDIT
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 13fea36..648144f 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -108,6 +108,10 @@ int map_mmio_regions(struct domain *d,
                      unsigned long start_gfn,
                      unsigned long nr_mfns,
                      unsigned long mfn);
+int unmap_mmio_regions(struct domain *d,
+                       unsigned long start_gfn,
+                       unsigned long nr_mfns,
+                       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 5ab7cb4..cea46e1 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 nr describe 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 nr,
+                     unsigned long mfn);
+int unmap_mmio_regions(struct domain *d,
+                       unsigned long start_gfn,
+                       unsigned long nr,
+                       unsigned long mfn);
+
 extern bool_t opt_hap_1gb, opt_hap_2mb;
 
 /*
-- 
2.0.3


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel