xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Arianna Avanzini <avanzini.arianna@gmail.com>
To: xen-devel@lists.xen.org
Cc: Ian.Campbell@eu.citrix.com, paolo.valente@unimore.it,
	keir@xen.org, stefano.stabellini@eu.citrix.com,
	Ian.Jackson@eu.citrix.com, dario.faggioli@citrix.com,
	tim@xen.org, julien.grall@citrix.com, etrudeau@broadcom.com,
	andrew.cooper3@citrix.com, JBeulich@suse.com,
	avanzini.arianna@gmail.com, viktor.kleinik@globallogic.com
Subject: [PATCH RESEND v9 08/14] xen/x86: factor out map and unmap from the memory_mapping DOMCTL
Date: Mon, 14 Jul 2014 02:50:29 +0200	[thread overview]
Message-ID: <1405299035-2988-9-git-send-email-avanzini.arianna@gmail.com> (raw)
In-Reply-To: <1405299035-2988-1-git-send-email-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. 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@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: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Eric Trudeau <etrudeau@broadcom.com>
Cc: Viktor Kleinik <viktor.kleinik@globallogic.com>

---

    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     | 43 +++++++++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/p2m.h |  4 ++++
 xen/include/asm-x86/p2m.h | 12 ++++++++++++
 5 files changed, 76 insertions(+), 14 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 6add407..a85e58d 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -872,6 +872,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 f991bbb..15af792 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 f5feb70..29df616 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1704,6 +1704,49 @@ 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 ret = 0, err = 0;
+    unsigned long i;
+
+    if ( !paging_mode_translate(d) )
+        return 0;
+
+    for ( i = 0; i < nr; i++ ) {
+        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;
 
 /*
-- 
1.9.3

  parent reply	other threads:[~2014-07-14  0:50 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-02 18:42 [PATCH v9 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
2014-07-02 18:42 ` [PATCH v9 01/14] arch/arm: add consistency check to REMOVE p2m changes Arianna Avanzini
2014-07-03 10:24   ` Julien Grall
2014-07-03 11:03   ` Julien Grall
2014-07-03 14:28     ` Ian Campbell
2014-07-03 14:44       ` Julien Grall
2014-07-02 18:42 ` [PATCH v9 02/14] arch/arm: let map_mmio_regions() take pfn as parameters Arianna Avanzini
2014-07-03 10:26   ` Julien Grall
2014-07-02 18:42 ` [PATCH v9 03/14] arch/arm: let map_mmio_regions() use start and count Arianna Avanzini
2014-07-02 18:42 ` [PATCH v9 04/14] arch/arm: unmap partially-mapped I/O-memory regions Arianna Avanzini
2014-07-03 14:40   ` Ian Campbell
2014-07-03 15:00     ` Julien Grall
2014-07-03 15:15       ` Ian Campbell
2014-07-02 18:42 ` [PATCH v9 05/14] arch/x86: warn if to-be-removed mapping does not exist Arianna Avanzini
2014-07-02 18:42 ` [PATCH v9 06/14] arch/x86: cleanup memory_mapping DOMCTL Arianna Avanzini
2014-07-02 18:42 ` [PATCH v9 07/14] xen/common: add ARM stub for the function memory_type_changed() Arianna Avanzini
2014-07-03 14:41   ` Ian Campbell
2014-07-02 18:42 ` [PATCH v9 08/14] xen/x86: factor out map and unmap from the memory_mapping DOMCTL Arianna Avanzini
2014-07-03 10:43   ` Jan Beulich
2014-07-02 18:42 ` [PATCH v9 09/14] xen/common: move the memory_mapping DOMCTL hypercall to common code Arianna Avanzini
2014-07-02 18:42 ` [PATCH v9 10/14] tools/libxl: parse optional start gfn from the iomem config option Arianna Avanzini
2014-07-02 18:42 ` [PATCH v9 11/14] tools/libxl: handle the iomem parameter with the memory_mapping hcall Arianna Avanzini
2014-07-02 18:42 ` [PATCH v9 12/14] tools/libxl: read from config if passing through primary GPU Arianna Avanzini
2014-07-02 19:00   ` Sander Eikelenboom
2014-07-03 10:30     ` Jan Beulich
2014-07-03 10:49       ` Sander Eikelenboom
2014-07-03 10:58         ` Jan Beulich
2014-07-03 14:50       ` Ian Campbell
2014-07-03 14:53         ` Ian Campbell
2014-07-03 15:17           ` Jan Beulich
2014-07-03 15:25             ` Ian Campbell
2014-07-03 15:37               ` Jan Beulich
2014-07-03 16:11                 ` Ian Campbell
2014-07-03 15:45               ` Sander Eikelenboom
2014-07-03 14:47   ` Ian Campbell
2014-07-02 18:42 ` [PATCH v9 13/14] tools/libxl: explicitly grant access to needed I/O-memory ranges Arianna Avanzini
2014-07-02 18:42 ` [PATCH v9 14/14] xen/common: do not implicitly permit access to mapped I/O memory Arianna Avanzini
2014-07-11 13:51 ` [PATCH v9 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Julien Grall
2014-07-11 14:24   ` Ian Campbell
2014-07-11 17:00     ` Arianna Avanzini
2014-07-14  8:54       ` Ian Campbell
2014-07-14  9:22         ` Arianna Avanzini
2014-07-14  0:50   ` [PATCH RESEND " Arianna Avanzini
2014-07-14  0:50     ` [PATCH RESEND v9 01/14] arch/arm: add consistency check to REMOVE p2m changes Arianna Avanzini
2014-07-14 10:57       ` Julien Grall
2014-07-17 12:56         ` Ian Campbell
2014-07-14  0:50     ` [PATCH RESEND v9 02/14] arch/arm: let map_mmio_regions() take pfn as parameters Arianna Avanzini
2014-07-17 12:57       ` Ian Campbell
2014-07-18 12:43         ` Ian Campbell
2014-07-14  0:50     ` [PATCH RESEND v9 03/14] arch/arm: let map_mmio_regions() use start and count Arianna Avanzini
2014-07-14  0:50     ` [PATCH RESEND v9 04/14] arch/arm: unmap partially-mapped I/O-memory regions Arianna Avanzini
2014-07-14  0:50     ` [PATCH RESEND v9 05/14] arch/x86: warn if to-be-removed mapping does not exist Arianna Avanzini
2014-07-14  0:50     ` [PATCH RESEND v9 06/14] arch/x86: cleanup memory_mapping DOMCTL Arianna Avanzini
2014-07-14  0:50     ` [PATCH RESEND v9 07/14] xen/common: add ARM stub for the function memory_type_changed() Arianna Avanzini
2014-07-17 12:59       ` Ian Campbell
2014-07-24 14:07       ` Julien Grall
2014-07-14  0:50     ` Arianna Avanzini [this message]
2014-07-17 13:00       ` [PATCH RESEND v9 08/14] xen/x86: factor out map and unmap from the memory_mapping DOMCTL Ian Campbell
2014-07-23 11:59       ` Jan Beulich
2014-07-23 12:54         ` Arianna Avanzini
2014-07-23 13:04           ` Ian Campbell
2014-07-14  0:50     ` [PATCH RESEND v9 09/14] xen/common: move the memory_mapping DOMCTL hypercall to common code Arianna Avanzini
2014-07-24 14:10       ` Julien Grall
2014-07-14  0:50     ` [PATCH RESEND v9 10/14] tools/libxl: parse optional start gfn from the iomem config option Arianna Avanzini
2014-07-14  0:50     ` [PATCH RESEND v9 11/14] tools/libxl: handle the iomem parameter with the memory_mapping hcall Arianna Avanzini
2014-07-24 14:12       ` Julien Grall
2014-07-14  0:50     ` [PATCH RESEND v9 12/14] tools/libxl: read from config if passing through primary GPU Arianna Avanzini
2014-07-14  0:50     ` [PATCH RESEND v9 13/14] tools/libxl: explicitly grant access to needed I/O-memory ranges Arianna Avanzini
2014-07-14  0:50     ` [PATCH RESEND v9 14/14] xen/common: do not implicitly permit access to mapped I/O memory Arianna Avanzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1405299035-2988-9-git-send-email-avanzini.arianna@gmail.com \
    --to=avanzini.arianna@gmail.com \
    --cc=Ian.Campbell@eu.citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=JBeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=dario.faggioli@citrix.com \
    --cc=etrudeau@broadcom.com \
    --cc=julien.grall@citrix.com \
    --cc=keir@xen.org \
    --cc=paolo.valente@unimore.it \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=tim@xen.org \
    --cc=viktor.kleinik@globallogic.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).