All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM
@ 2014-08-24 20:29 Arianna Avanzini
  2014-08-24 20:29 ` [PATCH v11 01/14] arch/arm: add consistency check to REMOVE p2m changes Arianna Avanzini
                   ` (14 more replies)
  0 siblings, 15 replies; 21+ messages in thread
From: Arianna Avanzini @ 2014-08-24 20:29 UTC (permalink / raw
  To: xen-devel
  Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
	Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
	andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
	andrii.tseglytskyi

Hello,

here is the eleventh version of my implementation proposal for the DOMCTL
memory_mapping for the ARM architecture. As usual, I'll try to keep this
as short as I can by just going through the most relevant changes. A more
detailed changelog is in the description of each of the commits, while an
extensive summary of the patch series' genesis can be found in the last
full cover letter ([1]).
Please note that this patchset includes a contribution from Andrii Tseglytskyi,
as you may also see from the shortlog below.

Patch 0001 has been fixed according to suggestions given by Julien Grall and
approved by Ian Campbell. Now it handles even level 3 mappings. Also, as
suggested by Andrew Cooper, the printk uses a XENLOG_G_WARNING loglevel so
that it can be rate-limited appropriately.

Patch 0002 has also been changed according to directives provided by Julien
Grall. Now partially-mapped memory regions are unmapped regardless of their
being I/O-memory or not.

Patch 0008 has been fixed according to directive from Julien Grall.

Patch 0010 has been added to allow FLASK to handle the memory_mapping case
also on ARM architecture, therefore preventing wrong error messages from
being printed, as indicated by Julien Grall.

A new patch 0011, contributed by Andrii Tseglytskyi, has been merged into
the series. It adds a FLASK policy rule to allow a domU to access previously-
mapped I/O-memory regions.

Patch 0012 has been modified according to suggestions provided by Ian Campbell.
The VGA-related bits have been moved to a libxl_pci helper. Non-functional
changes have been split into patch 0013 to facilitate review. A leftover debug
print has been removed.

As usual, the code has again been tested on a cubieboard2, with Linux v3.15-rc3
as a dom0 and ERIKA Enterprise ([3]) as a domU. The x86 bits have been tested on
an x86_64 machine with Linux 3.15.0-rc5 as both dom0 and domU.
Any feedback about this new version of the patchset is more than welcome,
Arianna

[1] http://lists.xen.org/archives/html/xen-devel/2014-03/msg01724.html
[2] http://markmail.org/message/7t6erxfy7pgorakb
[3] http://erika.tuxfamily.org/drupal/

Andrii Tseglytskyi (1):
  flask/policy: allow domU to use previously-mapped I/O-memory

Arianna Avanzini (13):
  arch/arm: add consistency check to REMOVE p2m changes
  arch/arm: unmap partially-mapped memory regions
  arch/x86: warn if to-be-removed mapping does not exist
  arch/x86: cleanup memory_mapping DOMCTL
  xen/common: add ARM stub for the function memory_type_changed()
  xen/x86: factor out map and unmap from the memory_mapping DOMCTL
  xen/common: move the memory_mapping DOMCTL hypercall to common code
  tools/libxl: parse optional start gfn from the iomem config option
  tools/libxl: handle the iomem parameter with the memory_mapping hcall
  xsm/flask: avoid spurious error messages when mapping I/O-memory
  tools/libxl: explicitly grant access to needed I/O-memory ranges
  tools/libxl: cleanup the do_pci_add() function
  xen/common: do not implicitly permit access to mapped I/O memory

 docs/man/xl.cfg.pod.5                        |  18 ++-
 tools/flask/policy/policy/modules/xen/xen.te |   1 +
 tools/libxc/xc_domain.c                      |  10 ++
 tools/libxl/libxl.h                          |  10 ++
 tools/libxl/libxl_create.c                   |  28 +++-
 tools/libxl/libxl_internal.h                 |   3 +
 tools/libxl/libxl_pci.c                      | 213 +++++++++++++++++----------
 tools/libxl/libxl_types.idl                  |   4 +
 tools/libxl/xl_cmdimpl.c                     |  17 ++-
 xen/arch/arm/p2m.c                           |  44 +++++-
 xen/arch/x86/domctl.c                        |  76 ----------
 xen/arch/x86/mm/p2m.c                        |  57 ++++++-
 xen/common/domctl.c                          |  54 ++++++-
 xen/common/memory.c                          |   2 +-
 xen/include/asm-arm/p2m.h                    |  13 +-
 xen/include/asm-x86/p2m.h                    |   3 +-
 xen/include/xen/p2m-common.h                 |  16 ++
 xen/xsm/flask/hooks.c                        |   2 +-
 18 files changed, 385 insertions(+), 186 deletions(-)
 create mode 100644 xen/include/xen/p2m-common.h

-- 
2.1.0

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v11 01/14] arch/arm: add consistency check to REMOVE p2m changes
  2014-08-24 20:29 [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
@ 2014-08-24 20:29 ` Arianna Avanzini
  2014-08-27 14:46   ` Julien Grall
  2014-08-24 20:29 ` [PATCH v11 02/14] arch/arm: unmap partially-mapped memory regions Arianna Avanzini
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 21+ messages in thread
From: Arianna Avanzini @ 2014-08-24 20:29 UTC (permalink / raw
  To: xen-devel
  Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
	Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
	andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
	andrii.tseglytskyi

Currently, the REMOVE case of the switch in apply_p2m_changes()
does not perform any consistency check on the mapping to be removed.
More in detail, the code does not check if the guest address to be
unmapped is actually mapped to the machine address given as a
parameter.
This commit adds the above-described consistency check to the REMOVE
path of apply_p2m_changes() and lets a warning be emitted when trying
to remove a non-existent mapping. This is instrumental to one of the
following commits, which implements the possibility to trigger the
removal of p2m ranges via the memory_mapping DOMCTL for ARM.

Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>

---

    v11:
        - Handle also level 3 mappings by performing the check outside
          the ( level < 3 ) branch.
        - Use XENLOG_G_WARNING as a loglevel to allow for appropriate rate-
          limiting of messages.

    v10:
        - Emit a warning and still unmap the mapping when failing to remove
          a mapping.
        - Correctly place the check for an unexpected mapping in the REMOVE
          case of apply_one_level().
        - Drop the check for non-present entries which is redundant.
        - Print the domain id when emitting a warning message.

    v9:
        - Don't return with an error when failing to remove a mapping, but
          simply keep unmapping.
        - Don't force assignment to the flush variable, as it is already
          set before the switch.
        - Change warning message to be more appropriate and clear; use the
          correct format for paddr_t and gdprintk(), which is more restricted
          than regular printk()s.
        - Adapt to rework of p2m-related functions for ARM.

    v8:
        - Re-add erroneously-removed increments to the maddr variable.
        - When failing to remove a mapping, add previously-mapped PT entry,
          unlock the p2m_lock and flush TLBs if necessary.
        - Emit an error message when failing to remove a mapping.
        - Remove tentative phrases from commit description.

    v7:
        - Silently ignore the fact that, when removing a mapping, the specified
          gfn is not mapped at all.
        - Remove spurious spacing change.

    v6:
        - Don't update "count" on REMOVE as it is only used inside the
          RELINQUISH case of the switch in apply_p2m_changes().
        - Return with an error if removal of a page fails instead of just
          skipping the page.

    v5:
        - Do not use a temporary variable to hold the machine address:
          use the "maddr" function parameter itself.
        - Increment the machine address also when first and second level
          mappings are not valid.
        - Get the actual machine frame number mapped to the guest frame
          number given as parameter to the function directly in the
          REMOVE case of the switch construct, as it might not be valid
          in other cases and its value might be uncorrectly used in the
          future.
        - Remove useless and/or harmful ASSERT; check however if the
          mapping is valid and skip the page if it is not.

    v4:
        - Remove useless and slow lookup and use already-available
          data from pte instead.
        - Correctly increment the local variable used to keep the
          machine address whose mapping is currently being removed.
        - Return with an error upon finding a mismatch between the
          actual machine address mapped to the guest address and
          the machine address passed as parameter, instead of just
          skipping the page.

---
 xen/arch/arm/p2m.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 143199b..8f83d17 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -601,6 +601,7 @@ static int apply_one_level(struct domain *d,
         {
             /* Progress up to next boundary */
             *addr = (*addr + level_size) & level_mask;
+            *maddr = (*maddr + level_size) & level_mask;
             return P2M_ONE_PROGRESS_NOP;
         }
 
@@ -632,12 +633,29 @@ static int apply_one_level(struct domain *d,
             }
         }
 
+        /*
+         * Ensure that the guest address addr currently being
+         * handled (that is in the range given as argument to
+         * this function) is actually mapped to the corresponding
+         * machine address in the specified range. maddr here is
+         * the machine address given to the function, while
+         * orig_pte.p2m.base is the machine frame number actually
+         * mapped to the guest address: check if the two correspond.
+         */
+         if ( op == REMOVE &&
+              pfn_to_paddr(orig_pte.p2m.base) != *maddr )
+             printk(XENLOG_G_WARNING
+                    "p2m_remove dom%d: mapping at %"PRIpaddr" is of maddr %"PRIpaddr" not %"PRIpaddr" as expected\n",
+                    d->domain_id, *addr, pfn_to_paddr(orig_pte.p2m.base),
+                    *maddr);
+
         *flush = true;
 
         memset(&pte, 0x00, sizeof(pte));
         p2m_write_pte(entry, pte, flush_cache);
 
         *addr += level_size;
+        *maddr += level_size;
 
         p2m->stats.mappings[level]--;
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v11 02/14] arch/arm: unmap partially-mapped memory regions
  2014-08-24 20:29 [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
  2014-08-24 20:29 ` [PATCH v11 01/14] arch/arm: add consistency check to REMOVE p2m changes Arianna Avanzini
@ 2014-08-24 20:29 ` Arianna Avanzini
  2014-08-27 16:35   ` Julien Grall
  2014-08-24 20:29 ` [PATCH v11 03/14] arch/x86: warn if to-be-removed mapping does not exist Arianna Avanzini
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 21+ messages in thread
From: Arianna Avanzini @ 2014-08-24 20:29 UTC (permalink / raw
  To: xen-devel
  Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
	Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
	andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
	andrii.tseglytskyi

This commit changes the function apply_p2m_changes() to destroy changes
performed while mapping a memory region, if errors are seen during
the operation. The implemented behaviour is the counterpart of the x86
code, which destroys the whole range: this is useful to avoid that
memory areas remain partially accessible to guests.

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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>

---

    v11:
        - Handle partially-mapped memory regions regardless of their being
          I/O-memory regions or not.

    v10:
        - Recursively call apply_p2m_changes() on the whole I/O-memory range
          when unmapping a partially-mapped I/O-memory region.

    v9:
        - Let apply_p2m_ranges() unwind its own progress instead of relying on
          the caller to unmap partially-mapped I/O-memory regions.
        - Adapt to rework of p2m-related functions for ARM.

---
 xen/arch/arm/p2m.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index b9129a0..0520f84 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -713,7 +713,7 @@ static int apply_p2m_changes(struct domain *d,
     int rc, ret;
     struct p2m_domain *p2m = &d->arch.p2m;
     lpae_t *first = NULL, *second = NULL, *third = NULL;
-    paddr_t addr;
+    paddr_t addr, orig_maddr = maddr;
     unsigned long cur_first_page = ~0,
                   cur_first_offset = ~0,
                   cur_second_offset = ~0;
@@ -844,6 +844,10 @@ out:
     if (third) unmap_domain_page(third);
     if (second) unmap_domain_page(second);
     if (first) unmap_domain_page(first);
+    if ( rc < 0 && ( op == INSERT || op == ALLOCATE ) )
+        apply_p2m_changes(d, REMOVE, start_gpaddr,
+                          end_gpaddr, orig_maddr,
+                          MATTR_DEV, p2m_invalid);
 
     spin_unlock(&p2m->lock);
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v11 03/14] arch/x86: warn if to-be-removed mapping does not exist
  2014-08-24 20:29 [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
  2014-08-24 20:29 ` [PATCH v11 01/14] arch/arm: add consistency check to REMOVE p2m changes Arianna Avanzini
  2014-08-24 20:29 ` [PATCH v11 02/14] arch/arm: unmap partially-mapped memory regions Arianna Avanzini
@ 2014-08-24 20:29 ` Arianna Avanzini
  2014-08-24 20:29 ` [PATCH v11 04/14] arch/x86: cleanup memory_mapping DOMCTL Arianna Avanzini
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Arianna Avanzini @ 2014-08-24 20:29 UTC (permalink / raw
  To: xen-devel
  Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
	Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
	andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
	andrii.tseglytskyi

Currently, when a memory mapping is removed with the memory_mapping
DOMCTL, no check is performed on the existence of such a mapping.
This commit adds such a consistency check to the code performing the
unmap, emitting a warning message if the check fails.

Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.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: 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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>

---

    v8:
        - Fix commit description and subject to better fit the contents of
          the patch.
        - Remove tentative phrases from commit description.

    v7:
        - Do not fail with an error when attempting to remove a non-existent
          mapping: just emit a warning.

---
 xen/arch/x86/domctl.c     |  4 ++--
 xen/arch/x86/mm/p2m.c     | 12 ++++++++----
 xen/common/memory.c       |  2 +-
 xen/include/asm-x86/p2m.h |  2 +-
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index d1517c4..1ead690 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -679,7 +679,7 @@ long arch_do_domctl(
                            "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);
+                        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
@@ -699,7 +699,7 @@ long arch_do_domctl(
             if ( paging_mode_translate(d) )
                 for ( i = 0; i < nr_mfns; i++ )
                 {
-                    ret = clear_mmio_p2m_entry(d, gfn + i);
+                    ret = clear_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i));
                     if ( ret )
                         tmp_rc = ret;
                 }
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index bca9f0f..1f6955b 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -859,10 +859,10 @@ int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn)
 }
 
 /* Returns: 0 for success, -errno for failure */
-int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn)
+int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn)
 {
     int rc = -EINVAL;
-    mfn_t mfn;
+    mfn_t actual_mfn;
     p2m_access_t a;
     p2m_type_t t;
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
@@ -871,15 +871,19 @@ int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn)
         return -EIO;
 
     gfn_lock(p2m, gfn, 0);
-    mfn = p2m->get_entry(p2m, gfn, &t, &a, 0, NULL);
+    actual_mfn = p2m->get_entry(p2m, gfn, &t, &a, 0, NULL);
 
     /* Do not use mfn_valid() here as it will usually fail for MMIO pages. */
-    if ( (INVALID_MFN == mfn_x(mfn)) || (t != p2m_mmio_direct) )
+    if ( (INVALID_MFN == mfn_x(actual_mfn)) || (t != p2m_mmio_direct) )
     {
         gdprintk(XENLOG_ERR,
                  "gfn_to_mfn failed! gfn=%08lx type:%d\n", gfn, t);
         goto out;
     }
+    if ( mfn_x(mfn) != mfn_x(actual_mfn) )
+        gdprintk(XENLOG_WARNING,
+                 "no mapping between mfn %08lx and gfn %08lx\n",
+                 mfn_x(mfn), gfn);
     rc = p2m_set_entry(p2m, gfn, _mfn(INVALID_MFN), PAGE_ORDER_4K, p2m_invalid,
                        p2m->default_access);
 
diff --git a/xen/common/memory.c b/xen/common/memory.c
index c2dd31b..2433111 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -206,7 +206,7 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
     }
     if ( p2mt == p2m_mmio_direct )
     {
-        clear_mmio_p2m_entry(d, gmfn);
+        clear_mmio_p2m_entry(d, gmfn, _mfn(mfn));
         put_gfn(d, gmfn);
         return 1;
     }
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 0ddbadb..5ab7cb4 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -534,7 +534,7 @@ int p2m_is_logdirty_range(struct p2m_domain *, unsigned long start,
 
 /* Set mmio addresses in the p2m table (for pass-through) */
 int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn);
-int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn);
+int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn);
 
 /* Add foreign mapping to the guest's p2m table. */
 int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v11 04/14] arch/x86: cleanup memory_mapping DOMCTL
  2014-08-24 20:29 [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
                   ` (2 preceding siblings ...)
  2014-08-24 20:29 ` [PATCH v11 03/14] arch/x86: warn if to-be-removed mapping does not exist Arianna Avanzini
@ 2014-08-24 20:29 ` Arianna Avanzini
  2014-08-24 20:29 ` [PATCH v11 05/14] xen/common: add ARM stub for the function memory_type_changed() Arianna Avanzini
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Arianna Avanzini @ 2014-08-24 20:29 UTC (permalink / raw
  To: xen-devel
  Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
	Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
	andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
	andrii.tseglytskyi

This commit lets the end mfn be computed only once while handling a
XEN_DOMCTL_memory_mapping hypercall. Also, the name of the tmp_rc
local variable is changed to rc.

Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Jan Beulich <JBeulich@suse.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: 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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
 xen/arch/x86/domctl.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 1ead690..9cdbc3d 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -646,19 +646,20 @@ 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;
         int add = domctl->u.memory_mapping.add_mapping;
 
         ret = -EINVAL;
-        if ( (mfn + nr_mfns - 1) < mfn || /* wrap? */
-             ((mfn | (mfn + nr_mfns - 1)) >> (paddr_bits - PAGE_SHIFT)) ||
+        if ( mfn_end < mfn || /* wrap? */
+             ((mfn | mfn_end) >> (paddr_bits - PAGE_SHIFT)) ||
              (gfn + nr_mfns - 1) < gfn ) /* wrap? */
             break;
 
         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;
 
@@ -668,7 +669,7 @@ long arch_do_domctl(
                    "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);
+            ret = iomem_permit_access(d, mfn, mfn_end);
             if ( !ret && paging_mode_translate(d) )
             {
                 for ( i = 0; !ret && i < nr_mfns; i++ )
@@ -680,17 +681,17 @@ long arch_do_domctl(
                            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) &&
+                    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);
+                               d->domain_id, mfn, mfn_end);
                 }
             }
         }
         else
         {
-            int tmp_rc = 0;
+            int rc = 0;
 
             printk(XENLOG_G_INFO
                    "memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
@@ -701,16 +702,16 @@ long arch_do_domctl(
                 {
                     ret = clear_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i));
                     if ( ret )
-                        tmp_rc = ret;
+                        rc = ret;
                 }
-            ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
+            ret = iomem_deny_access(d, mfn, mfn_end);
             if ( !ret )
-                ret = tmp_rc;
+                ret = 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);
+                       ret, rc ? "removing" : "denying", d->domain_id,
+                       mfn, mfn_end);
         }
         /* Do this unconditionally to cover errors on above failure paths. */
         memory_type_changed(d);
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v11 05/14] xen/common: add ARM stub for the function memory_type_changed()
  2014-08-24 20:29 [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
                   ` (3 preceding siblings ...)
  2014-08-24 20:29 ` [PATCH v11 04/14] arch/x86: cleanup memory_mapping DOMCTL Arianna Avanzini
@ 2014-08-24 20:29 ` Arianna Avanzini
  2014-08-24 20:29 ` [PATCH v11 06/14] xen/x86: factor out map and unmap from the memory_mapping DOMCTL Arianna Avanzini
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Arianna Avanzini @ 2014-08-24 20:29 UTC (permalink / raw
  To: xen-devel
  Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
	Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
	andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
	andrii.tseglytskyi

MTRR-related code is not available for the ARM architecture. Given
that the memory_type_changed() function would be called also from
common code, its invocation is currently ifdef'd out to be only
compiled in on an x86 machine. This commit adds an empty stub for ARM.

Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Ian Campbell <Ian.Campbell@eu.citrix.com>
Acked-by: Julien Grall <julien.grall@citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>

---

    v9:
        - Don't expose the memory_type_changed() function to common code,
          just add an empty stub in arch/arm/p2m.c and in the related header
          for ARM.

---
 xen/arch/arm/p2m.c        | 4 ++++
 xen/common/domctl.c       | 2 --
 xen/include/asm-arm/p2m.h | 2 ++
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 0520f84..ea9a884 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -47,6 +47,10 @@ void p2m_dump_info(struct domain *d)
     spin_unlock(&p2m->lock);
 }
 
+void memory_type_changed(struct domain *d)
+{
+}
+
 void dump_p2m_lookup(struct domain *d, paddr_t addr)
 {
     struct p2m_domain *p2m = &d->arch.p2m;
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index c326aba..24102c0 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -897,10 +897,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
             ret = iomem_permit_access(d, mfn, mfn + nr_mfns - 1);
         else
             ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
-#ifdef CONFIG_X86
         if ( !ret )
             memory_type_changed(d);
-#endif
     }
     break;
 
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 06c93a0..13fea36 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -5,6 +5,8 @@
 
 struct domain;
 
+extern void memory_type_changed(struct domain *);
+
 /* Per-p2m-table state */
 struct p2m_domain {
     /* Lock that protects updates to the p2m */
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v11 06/14] xen/x86: factor out map and unmap from the memory_mapping DOMCTL
  2014-08-24 20:29 [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
                   ` (4 preceding siblings ...)
  2014-08-24 20:29 ` [PATCH v11 05/14] xen/common: add ARM stub for the function memory_type_changed() Arianna Avanzini
@ 2014-08-24 20:29 ` Arianna Avanzini
  2014-08-24 20:30 ` [PATCH v11 07/14] xen/common: move the memory_mapping DOMCTL hypercall to common code Arianna Avanzini
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Arianna Avanzini @ 2014-08-24 20:29 UTC (permalink / raw
  To: xen-devel
  Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
	Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
	andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
	andrii.tseglytskyi

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>
Acked-by: Ian Campbell <Ian.Campbell@eu.citrix.com>
Acked-by: Jan Beulich <JBeulich@suse.com>
Acked-by: Julien Grall <julien.grall@citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>

---

    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 ea9a884..67077c4 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 1f6955b..fa81036 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.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v11 07/14] xen/common: move the memory_mapping DOMCTL hypercall to common code
  2014-08-24 20:29 [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
                   ` (5 preceding siblings ...)
  2014-08-24 20:29 ` [PATCH v11 06/14] xen/x86: factor out map and unmap from the memory_mapping DOMCTL Arianna Avanzini
@ 2014-08-24 20:30 ` Arianna Avanzini
  2014-08-24 20:30 ` [PATCH v11 08/14] tools/libxl: parse optional start gfn from the iomem config option Arianna Avanzini
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Arianna Avanzini @ 2014-08-24 20:30 UTC (permalink / raw
  To: xen-devel
  Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
	Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
	andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
	andrii.tseglytskyi

This commit moves to common code the implementation of the memory_mapping
DOMCTL, currently available only for the x86 architecture. It also adds
a definition for the PADDR_BITS constant for ARM, that is to be used in
common code and currently not available for the ARM architecture.

Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Jan Beulich <JBeulich@suse.com>
Acked-by: Julien Grall <julien.grall@citrix.com>
Tested-by: Julien Grall <julien.grall@citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <Ian.Campbell@eu.citrix.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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>

---

    v9:
        - Don't expose the type mfn_t to common code; let the x86-specific map/
          unmap functions perform the type cast and use unsigned long in common
          code.

    v7:
        - Change the name of tmp_rc to rc. Also, do not uselessly initialize it.
        - ifdef out the invocation of memory_type_changed() to be called only
          if the architecture is x86 instead of adding an useless empty stub
          for ARM.

    v6:
        - Add an empty definition of the memory_type_changed() function for ARM.

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

    v3:
        - Add a paddr_bits variable for ARM.

    v2:
        - Move code to xen/arm/domctl.c.

---
 xen/arch/arm/p2m.c           |  8 +++---
 xen/arch/x86/domctl.c        | 68 --------------------------------------------
 xen/common/domctl.c          | 68 ++++++++++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/p2m.h    | 15 +++-------
 xen/include/asm-x86/p2m.h    | 13 +--------
 xen/include/xen/p2m-common.h | 16 +++++++++++
 6 files changed, 93 insertions(+), 95 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 67077c4..af8f5c9 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -868,24 +868,24 @@ int p2m_populate_ram(struct domain *d,
 
 int map_mmio_regions(struct domain *d,
                      unsigned long start_gfn,
-                     unsigned long nr_mfns,
+                     unsigned long nr,
                      unsigned long mfn)
 {
     return apply_p2m_changes(d, INSERT,
                              pfn_to_paddr(start_gfn),
-                             pfn_to_paddr(start_gfn + nr_mfns),
+                             pfn_to_paddr(start_gfn + nr),
                              pfn_to_paddr(mfn),
                              MATTR_DEV, p2m_mmio_direct);
 }
 
 int unmap_mmio_regions(struct domain *d,
                        unsigned long start_gfn,
-                       unsigned long nr_mfns,
+                       unsigned long nr,
                        unsigned long mfn)
 {
     return apply_p2m_changes(d, REMOVE,
                              pfn_to_paddr(start_gfn),
-                             pfn_to_paddr(start_gfn + nr_mfns),
+                             pfn_to_paddr(start_gfn + nr),
                              pfn_to_paddr(mfn),
                              MATTR_DEV, p2m_invalid);
 }
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index caa3be6..7a5de43 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -641,74 +641,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;
-        unsigned long mfn_end = mfn + nr_mfns - 1;
-        int add = domctl->u.memory_mapping.add_mapping;
-
-        ret = -EINVAL;
-        if ( mfn_end < mfn || /* wrap? */
-             ((mfn | mfn_end) >> (paddr_bits - PAGE_SHIFT)) ||
-             (gfn + nr_mfns - 1) < gfn ) /* wrap? */
-            break;
-
-        ret = -EPERM;
-        if ( !iomem_access_permitted(current->domain, mfn, mfn_end) )
-            break;
-
-        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",
-                   d->domain_id, gfn, mfn, nr_mfns);
-
-            ret = iomem_permit_access(d, mfn, mfn_end);
-            if ( !ret )
-            {
-                ret = map_mmio_regions(d, gfn, nr_mfns, mfn);
-                if ( ret )
-                {
-                    printk(XENLOG_G_WARNING
-                           "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
-                               "memory_map: failed to deny dom%d access to [%lx,%lx]\n",
-                               d->domain_id, mfn, mfn_end);
-                }
-            }
-        }
-        else
-        {
-            int rc = 0;
-
-            printk(XENLOG_G_INFO
-                   "memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
-                   d->domain_id, gfn, mfn, nr_mfns);
-
-            rc = unmap_mmio_regions(d, gfn, nr_mfns, mfn);
-            ret = iomem_deny_access(d, mfn, mfn_end);
-            if ( !ret )
-                ret = rc;
-            if ( ret && is_hardware_domain(current->domain) )
-                printk(XENLOG_ERR
-                       "memory_map: error %ld %s dom%d access to [%lx,%lx]\n",
-                       ret, rc ? "removing" : "denying", d->domain_id,
-                       mfn, mfn_end);
-        }
-        /* Do this unconditionally to cover errors on above failure paths. */
-        memory_type_changed(d);
-    }
-    break;
-
     case XEN_DOMCTL_ioport_mapping:
     {
         struct hvm_iommu *hd;
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 24102c0..80b7800 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -902,6 +902,74 @@ 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;
+        int add = op->u.memory_mapping.add_mapping;
+
+        ret = -EINVAL;
+        if ( mfn_end < mfn || /* wrap? */
+             ((mfn | mfn_end) >> (paddr_bits - PAGE_SHIFT)) ||
+             (gfn + nr_mfns - 1) < gfn ) /* wrap? */
+            break;
+
+        ret = -EPERM;
+        if ( !iomem_access_permitted(current->domain, mfn, mfn_end) )
+            break;
+
+        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",
+                   d->domain_id, gfn, mfn, nr_mfns);
+
+            ret = iomem_permit_access(d, mfn, mfn_end);
+            if ( !ret )
+            {
+                ret = map_mmio_regions(d, gfn, nr_mfns, mfn);
+                if ( ret )
+                {
+                    printk(XENLOG_G_WARNING
+                           "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
+                               "memory_map: failed to deny dom%d access to [%lx,%lx]\n",
+                               d->domain_id, mfn, mfn_end);
+                }
+            }
+        }
+        else
+        {
+            int rc = 0;
+
+            printk(XENLOG_G_INFO
+                   "memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
+                   d->domain_id, gfn, mfn, nr_mfns);
+
+            rc = unmap_mmio_regions(d, gfn, nr_mfns, mfn);
+            ret = iomem_deny_access(d, mfn, mfn_end);
+            if ( !ret )
+                ret = rc;
+            if ( ret && is_hardware_domain(current->domain) )
+                printk(XENLOG_ERR
+                       "memory_map: error %ld %s dom%d access to [%lx,%lx]\n",
+                       ret, rc ? "removing" : "denying", d->domain_id,
+                       mfn, mfn_end);
+        }
+        /* Do this unconditionally to cover errors on above failure paths. */
+        memory_type_changed(d);
+    }
+    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 648144f..08ce07b 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -3,6 +3,10 @@
 
 #include <xen/mm.h>
 
+#include <xen/p2m-common.h>
+
+#define paddr_bits PADDR_BITS
+
 struct domain;
 
 extern void memory_type_changed(struct domain *);
@@ -101,17 +105,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 nr_mfns 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_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 cea46e1..2a65848 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -29,21 +29,10 @@
 
 #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 */
 
-/* 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;
 
 /*
diff --git a/xen/include/xen/p2m-common.h b/xen/include/xen/p2m-common.h
new file mode 100644
index 0000000..9f1b771
--- /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 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);
+
+#endif /* _XEN_P2M_COMMON_H */
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v11 08/14] tools/libxl: parse optional start gfn from the iomem config option
  2014-08-24 20:29 [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
                   ` (6 preceding siblings ...)
  2014-08-24 20:30 ` [PATCH v11 07/14] xen/common: move the memory_mapping DOMCTL hypercall to common code Arianna Avanzini
@ 2014-08-24 20:30 ` Arianna Avanzini
  2014-08-24 20:30 ` [PATCH v11 09/14] tools/libxl: handle the iomem parameter with the memory_mapping hcall Arianna Avanzini
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Arianna Avanzini @ 2014-08-24 20:30 UTC (permalink / raw
  To: xen-devel
  Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
	Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
	andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
	andrii.tseglytskyi

Currently, the "iomem" domU config option allows to specify a machine
address range to be mapped to the domU. However, there is no way to
specify the guest address range used for the mapping. This commit
extends the iomem option handling code to parse an additional, optional
parameter: this parameter, if given, specifies the start guest address
used for the mapping; if no start guest address is given, a 1:1 mapping
is performed as default.

Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Ian Campbell <Ian.Campbell@eu.citrix.com>
Acked-by: Julien Grall <julien.grall@citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>

---

    v11:
        - Avoid shadowed declaration warning by moving to function scope an
          index variable.

    v7:
        - Use LIBXL_INVALID_GFN when needed.
        - Add to the manpage a note about the mapping defaulting to 1:1 when
          the gfn parameter is not specified, and about the option being for
          auto-translated guests.
        - Remove spurious change.

    v6:
        - Document what happens if the new "gfn" parameter is omitted
          while specifying the "iomem" option in the domain configuration.
        - Add LIBXL_INVALID_GFN macro with value "~(uint64_t)0".
        - Fix typo in the comment to the LIBXL_HAVE_BUILDINFO_IOMEM_START_GFN
          macro.

    v5:
        - Initialize the gfn field of the iomem structure with the
          value "(uint64_t)-1".
        - Defer the assignment of the value of "start" to "gfn", if
          "gfn" has been initialized to the default value, in libxl.
        - Use a local variable to keep the return value of sscanf()
          for better code readability.

    v4:
        - Add definition of a LIBXL_HAVE_BUILDINFO_IOMEM_START_GFN macro
          to indicate the availability of the new option.
        - Simplify the code parsing the new optional parameter by using
          the switch construct on the return value of the sscanf() function
          instead of calling the function twice.
        - Add a short paragraph to the manpage about the use of the iomem
          option to passthrough devices protected by an IOMMU.
        - Add comments to the fields of the "iomem" structure.

---
 docs/man/xl.cfg.pod.5        | 18 +++++++++++++-----
 tools/libxl/libxl.h          | 10 ++++++++++
 tools/libxl/libxl_create.c   |  8 ++++++--
 tools/libxl/libxl_internal.h |  1 +
 tools/libxl/libxl_types.idl  |  4 ++++
 tools/libxl/xl_cmdimpl.c     | 17 ++++++++++-------
 6 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 1e04eed..6d184d6 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -632,12 +632,20 @@ is given in hexadecimal and may either a span e.g. C<2f8-2ff>
 It is recommended to use this option only for trusted VMs under
 administrator control.
 
-=item B<iomem=[ "IOMEM_START,NUM_PAGES", "IOMEM_START,NUM_PAGES", ... ]>
+=item B<iomem=[ "IOMEM_START,NUM_PAGES[@GFN]", "IOMEM_START,NUM_PAGES[@GFN]", ... ]>
 
-Allow guest to access specific hardware I/O memory pages. B<IOMEM_START>
-is a physical page number. B<NUM_PAGES> is the number
-of pages beginning with B<START_PAGE> to allow access. Both values
-must be given in hexadecimal.
+Allow auto-translated domains to access specific hardware I/O memory pages.
+
+B<IOMEM_START> is a physical page number. B<NUM_PAGES> is the number of pages
+beginning with B<START_PAGE> to allow access. B<GFN> specifies the guest frame
+number where the mapping will start in the domU's address space. If B<GFN> is
+not given, the mapping will be performed using B<IOMEM_START> as a start in the
+domU's address space, therefore performing an 1:1 mapping as default.
+All of these values must be given in hexadecimal.
+
+Note that the IOMMU won't be updated with the mappings specified with this
+option. This option therefore should not be used to passthrough any
+IOMMU-protected device.
 
 It is recommended to use this option only for trusted VMs under
 administrator control.
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index bfeb3bc..41b6836 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -128,6 +128,16 @@
 #define LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_ENABLE 1
 
 /*
+ * LIBXL_HAVE_BUILDINFO_IOMEM_START_GFN indicates that it is possible
+ * to specify the start guest frame number used to map a range of I/O
+ * memory machine frame numbers via the 'gfn' field (of type uint64)
+ * of the 'iomem' structure. An array of iomem structures is embedded
+ * in libxl_domain_build_info and used to map the indicated memory
+ * ranges during domain build.
+ */
+#define LIBXL_HAVE_BUILDINFO_IOMEM_START_GFN 1
+
+/*
  * libxl ABI compatibility
  *
  * The only guarantee which libxl makes regarding ABI compatibility
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 9b66294..3450337 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -102,6 +102,8 @@ static int sched_params_valid(libxl__gc *gc,
 int libxl__domain_build_info_setdefault(libxl__gc *gc,
                                         libxl_domain_build_info *b_info)
 {
+    int i;
+
     if (b_info->type != LIBXL_DOMAIN_TYPE_HVM &&
         b_info->type != LIBXL_DOMAIN_TYPE_PV)
         return ERROR_INVAL;
@@ -189,8 +191,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
 
     /* In libxl internals, we want to deal with vcpu_hard_affinity only! */
     if (b_info->cpumap.size && !b_info->num_vcpu_hard_affinity) {
-        int i;
-
         b_info->vcpu_hard_affinity = libxl__calloc(gc, b_info->max_vcpus,
                                                    sizeof(libxl_bitmap));
         for (i = 0; i < b_info->max_vcpus; i++) {
@@ -215,6 +215,10 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
 
     libxl_defbool_setdefault(&b_info->disable_migrate, false);
 
+    for (i = 0 ; i < b_info->num_iomem; i++)
+        if (b_info->iomem[i].gfn == LIBXL_INVALID_GFN)
+            b_info->iomem[i].gfn = b_info->iomem[i].start;
+
     if (!b_info->event_channels)
         b_info->event_channels = 1023;
 
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index beb052e..04c9378 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -91,6 +91,7 @@
 #define LIBXL_PV_EXTRA_MEMORY 1024
 #define LIBXL_HVM_EXTRA_MEMORY 2048
 #define LIBXL_MIN_DOM0_MEM (128*1024)
+#define LIBXL_INVALID_GFN (~(uint64_t)0)
 /* use 0 as the domid of the toolstack domain for now */
 #define LIBXL_TOOLSTACK_DOMID 0
 #define QEMU_SIGNATURE "DeviceModelRecord0002"
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 0b3496f..874cc49 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -185,8 +185,12 @@ libxl_ioport_range = Struct("ioport_range", [
     ])
 
 libxl_iomem_range = Struct("iomem_range", [
+    # start host frame number to be mapped to the guest
     ("start", uint64),
+    # number of frames to be mapped
     ("number", uint64),
+    # guest frame number used as a start for the mapping
+    ("gfn", uint64, {'init_val': "LIBXL_INVALID_GFN"}),
     ])
 
 libxl_vga_interface_info = Struct("vga_interface_info", [
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index f1c136a..8aae9df 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1182,6 +1182,7 @@ static void parse_config_data(const char *config_source,
     }
 
     if (!xlu_cfg_get_list(config, "iomem", &iomem, &num_iomem, 0)) {
+        int ret;
         b_info->num_iomem = num_iomem;
         b_info->iomem = calloc(num_iomem, sizeof(*b_info->iomem));
         if (b_info->iomem == NULL) {
@@ -1195,13 +1196,15 @@ static void parse_config_data(const char *config_source,
                         "xl: Unable to get element %d in iomem list\n", i);
                 exit(1);
             }
-            if(sscanf(buf, "%" SCNx64",%" SCNx64,
-                     &b_info->iomem[i].start,
-                     &b_info->iomem[i].number)
-                  != 2) {
-               fprintf(stderr,
-                       "xl: Invalid argument parsing iomem: %s\n", buf);
-               exit(1);
+            libxl_iomem_range_init(&b_info->iomem[i]);
+            ret = sscanf(buf, "%" SCNx64",%" SCNx64"@%" SCNx64,
+                         &b_info->iomem[i].start,
+                         &b_info->iomem[i].number,
+                         &b_info->iomem[i].gfn);
+            if (ret < 2) {
+                fprintf(stderr,
+                        "xl: Invalid argument parsing iomem: %s\n", buf);
+                exit(1);
             }
         }
     }
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v11 09/14] tools/libxl: handle the iomem parameter with the memory_mapping hcall
  2014-08-24 20:29 [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
                   ` (7 preceding siblings ...)
  2014-08-24 20:30 ` [PATCH v11 08/14] tools/libxl: parse optional start gfn from the iomem config option Arianna Avanzini
@ 2014-08-24 20:30 ` Arianna Avanzini
  2014-08-24 20:30 ` [PATCH v11 10/14] xsm/flask: avoid spurious error messages when mapping I/O-memory Arianna Avanzini
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Arianna Avanzini @ 2014-08-24 20:30 UTC (permalink / raw
  To: xen-devel
  Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
	Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
	andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
	andrii.tseglytskyi

Currently, the configuration-parsing code concerning the handling of the
iomem parameter only invokes the XEN_DOMCTL_iomem_permission hypercall.
This commit lets the XEN_DOMCTL_memory_mapping hypercall be invoked
after XEN_DOMCTL_iomem_permission when the iomem parameter is parsed
from a domU configuration file, so that the address range can be mapped
to the address space of the domU. The hypercall is invoked only in case
of domains using an auto-translated physmap.

Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Ian Campbell <Ian.Campbell@eu.citrix.com>
Acked-by: Julien Grall <julien.grall@citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>

---

    v9:
        - Enforce checks on return value of xc_domain_getinfo() as it could
          return information concerning the wrong domain.
        - Don't use PERROR for xc_core_* functions.
        - Avoid emitting an error message if xc_core_auto_translated_physmap()
          fails.

    v8:
        - Fix wrong error message emitted when the memory_mapping DOMCTL is
          called by a domain that is not auto-translated.

    v7:
        - Move the check for an auto-translated physmap to xc to avoid having
          to expose the information to libxl.

    v4:
        - Let the hypercall be called only in case the guest uses an
          auto-translated physmap.

    v3:
        - Add ifdefs to let the hypercall be called only by ARM or x86
          HVM guests, with a whitelist approach.
        - Remove the NOTE comment.

    v2:
        - Add a comment explaining outstanding issues.

---
 tools/libxc/xc_domain.c    | 10 ++++++++++
 tools/libxl/libxl_create.c | 11 +++++++++++
 2 files changed, 21 insertions(+)

diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index c67ac9a..1eba393 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -1960,6 +1960,16 @@ int xc_domain_memory_mapping(
     uint32_t add_mapping)
 {
     DECLARE_DOMCTL;
+    xc_dominfo_t info;
+
+    if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 ||
+         info.domid != domid )
+    {
+        PERROR("Could not get info for domain");
+        return -EINVAL;
+    }
+    if ( !xc_core_arch_auto_translated_physmap(&info) )
+        return 0;
 
     domctl.cmd = XEN_DOMCTL_memory_mapping;
     domctl.domain = domid;
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 3450337..3b1ec91 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1161,6 +1161,17 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
                  "failed give dom%d access to iomem range %"PRIx64"-%"PRIx64,
                  domid, io->start, io->start + io->number - 1);
             ret = ERROR_FAIL;
+            continue;
+        }
+        ret = xc_domain_memory_mapping(CTX->xch, domid,
+                                       io->gfn, io->start,
+                                       io->number, 1);
+        if (ret < 0) {
+            LOGE(ERROR,
+                 "failed to map to dom%d iomem range %"PRIx64"-%"PRIx64
+                 " to guest address %"PRIx64,
+                 domid, io->start, io->start + io->number - 1, io->gfn);
+            ret = ERROR_FAIL;
         }
     }
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v11 10/14] xsm/flask: avoid spurious error messages when mapping I/O-memory
  2014-08-24 20:29 [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
                   ` (8 preceding siblings ...)
  2014-08-24 20:30 ` [PATCH v11 09/14] tools/libxl: handle the iomem parameter with the memory_mapping hcall Arianna Avanzini
@ 2014-08-24 20:30 ` Arianna Avanzini
  2014-08-27 16:40   ` Julien Grall
  2014-08-24 20:30 ` [PATCH v11 11/14] flask/policy: allow domU to use previously-mapped I/O-memory Arianna Avanzini
                   ` (4 subsequent siblings)
  14 siblings, 1 reply; 21+ messages in thread
From: Arianna Avanzini @ 2014-08-24 20:30 UTC (permalink / raw
  To: xen-devel
  Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
	Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
	andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
	andrii.tseglytskyi

Currently, FLASK only handles the memory_mapping hypercall for the
x86 architecture. In case the architecture is ARM, an error message
will be printed regardless of how XSM has been configured. This
commit lets the memory_mapping case be handled also for architectures
other than x86.

Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Julien Grall <julien.grall@citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
 xen/xsm/flask/hooks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index f2f59ea..a0e4ae0 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -570,6 +570,7 @@ static int flask_domctl(struct domain *d, int cmd)
     case XEN_DOMCTL_scheduler_op:
     case XEN_DOMCTL_irq_permission:
     case XEN_DOMCTL_iomem_permission:
+    case XEN_DOMCTL_memory_mapping:
     case XEN_DOMCTL_set_target:
 #ifdef CONFIG_X86
     /* These have individual XSM hooks (arch/x86/domctl.c) */
@@ -577,7 +578,6 @@ static int flask_domctl(struct domain *d, int cmd)
     case XEN_DOMCTL_ioport_permission:
     case XEN_DOMCTL_bind_pt_irq:
     case XEN_DOMCTL_unbind_pt_irq:
-    case XEN_DOMCTL_memory_mapping:
     case XEN_DOMCTL_ioport_mapping:
     case XEN_DOMCTL_mem_event_op:
     /* These have individual XSM hooks (drivers/passthrough/iommu.c) */
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v11 11/14] flask/policy: allow domU to use previously-mapped I/O-memory
  2014-08-24 20:29 [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
                   ` (9 preceding siblings ...)
  2014-08-24 20:30 ` [PATCH v11 10/14] xsm/flask: avoid spurious error messages when mapping I/O-memory Arianna Avanzini
@ 2014-08-24 20:30 ` Arianna Avanzini
  2014-08-24 20:30 ` [PATCH v11 12/14] tools/libxl: explicitly grant access to needed I/O-memory ranges Arianna Avanzini
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Arianna Avanzini @ 2014-08-24 20:30 UTC (permalink / raw
  To: xen-devel
  Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
	Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
	andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
	andrii.tseglytskyi

From: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>

This commit allows the domU to access previously-mapped I/O-memory
even if XSM is enabled and FLASK is enforced.

Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
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>
---
 tools/flask/policy/policy/modules/xen/xen.te | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/flask/policy/policy/modules/xen/xen.te b/tools/flask/policy/policy/modules/xen/xen.te
index bb59fe8..34b5bfa 100644
--- a/tools/flask/policy/policy/modules/xen/xen.te
+++ b/tools/flask/policy/policy/modules/xen/xen.te
@@ -107,6 +107,7 @@ admin_device(dom0_t, device_t)
 admin_device(dom0_t, irq_t)
 admin_device(dom0_t, ioport_t)
 admin_device(dom0_t, iomem_t)
+admin_device(domU_t, iomem_t)
 
 domain_comms(dom0_t, dom0_t)
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v11 12/14] tools/libxl: explicitly grant access to needed I/O-memory ranges
  2014-08-24 20:29 [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
                   ` (10 preceding siblings ...)
  2014-08-24 20:30 ` [PATCH v11 11/14] flask/policy: allow domU to use previously-mapped I/O-memory Arianna Avanzini
@ 2014-08-24 20:30 ` Arianna Avanzini
  2014-08-24 20:30 ` [PATCH v11 13/14] tools/libxl: cleanup the do_pci_add() function Arianna Avanzini
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Arianna Avanzini @ 2014-08-24 20:30 UTC (permalink / raw
  To: xen-devel
  Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
	Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
	andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
	andrii.tseglytskyi

This commit changes the existing libxl code to be sure to grant access
permission to PCI-related I/O memory ranges, while setting up passthrough
of PCI devices specified in the domain's configuration, and to VGA-related
memory ranges, while setting up VGA passthrough (if gfx_passthru = 1 and
gfx_passthru_primary = 1 in the domain's configuration).
As for the latter, the newly-added code does not replace any existing one,
but instead matches the calls to xc_domain_memory_mapping() performed by
QEMU on the path that is executed if gfx passthru is enabled and follows
the registration of a new VGA controller (in register_vga_regions(),
defined in hw/pt-graphics.c). In fact, VGA needs some extra memory
ranges to be mapped with respect to PCI; QEMU expects that access to those
memory ranges is implicitly granted when he calls the hypervisor with the
function xc_domain_memory_mapping(): this commit calls iomem_permission
for it when needed for a primary GPU by checking the passthru PCI devices'
class.

This commit is instrumental to the last one in the series, which will
separate the functions of the iomem_permission and memory_mapping DOMCTLs,
so that requesting an I/O-memory range will not imply that access to such
a range is implicitly granted.

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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>

---

    v11:
        - Move the VGA-related hunk to a libxl helper; evaluate gfx_passthru.val
          only once as it is static in the scope of the helper.
        - Remove leftover debug fprintf().
        - Improve code readability by separating blocks of code.

    v10:
        - Use a class-based check on the PCI device to determine if VGA-related
          I/O-memory ranges must be made accessible to a guest that needs gfx
          passthrough.

    v9:
        - Allow a domain access to the VGA framebuffer only if the user has
          signaled that one of the passthru GPUs is primary via domain config.

    v8:
        - Explain better in the commit description VGA-related code additions.
        - Fix v6 changelog which, being too generic, ended up to uncorrectly
          state that one of the requests had been addressed.
        - Remove tentative phrases from commit description.

    v7:
        - Let libxl not handle I/O ports and I/O memory differently when access
          to a PCI device is allowed to a domain.
        - Change the construct used by libxl during PCI-related initialization
          from a switch to an if to better suit the new execution flow.

---
 tools/libxl/libxl_create.c   |  9 +++++
 tools/libxl/libxl_internal.h |  2 +
 tools/libxl/libxl_pci.c      | 91 ++++++++++++++++++++++++++++++++++++--------
 3 files changed, 86 insertions(+), 16 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 3b1ec91..7c02e20 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1197,6 +1197,15 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
             libxl__spawn_stub_dm(egc, &dcs->dmss);
         else
             libxl__spawn_local_dm(egc, &dcs->dmss.dm);
+
+        /*
+         * Handle the domain's (and the related stubdomain's) access to
+         * the VGA framebuffer.
+         */
+        ret = libxl__grant_vga_iomem_permission(gc, domid, d_config);
+        if ( ret )
+            goto error_out;
+
         return;
     }
     case LIBXL_DOMAIN_TYPE_PV:
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 04c9378..e27861b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -951,6 +951,8 @@ _hidden int libxl__domain_cpupool(libxl__gc *gc, uint32_t domid);
 _hidden libxl_scheduler libxl__domain_scheduler(libxl__gc *gc, uint32_t domid);
 _hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
                                     libxl_domain_sched_params *scparams);
+_hidden int libxl__grant_vga_iomem_permission(libxl__gc *gc, const uint32_t domid,
+                                              libxl_domain_config *const d_config);
 
 typedef struct {
     uint32_t store_port;
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 2782d0e..4fc9218 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -846,10 +846,13 @@ static int qemu_pci_add_xenstore(libxl__gc *gc, uint32_t domid,
 static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
+    libxl_domain_type type = libxl__domain_type(gc, domid);
     int rc, hvm = 0;
 
-    switch (libxl__domain_type(gc, domid)) {
-    case LIBXL_DOMAIN_TYPE_HVM:
+    if (type == LIBXL_DOMAIN_TYPE_INVALID)
+        return ERROR_FAIL;
+
+    if (type == LIBXL_DOMAIN_TYPE_HVM) {
         hvm = 1;
         if (libxl__wait_for_device_model_deprecated(gc, domid, "running",
                                          NULL, NULL, NULL) < 0) {
@@ -867,8 +870,8 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
         }
         if ( rc )
             return ERROR_FAIL;
-        break;
-    case LIBXL_DOMAIN_TYPE_PV:
+    }
+
     {
         char *sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
                                          pcidev->bus, pcidev->dev, pcidev->func);
@@ -937,11 +940,8 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
                 return ERROR_FAIL;
             }
         }
-        break;
-    }
-    case LIBXL_DOMAIN_TYPE_INVALID:
-        return ERROR_FAIL;
     }
+
 out:
     if (!libxl_is_stubdom(ctx, domid, NULL)) {
         rc = xc_assign_device(ctx->xch, domid, pcidev_encode_bdf(pcidev));
@@ -1166,6 +1166,7 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     libxl_device_pci *assigned;
+    libxl_domain_type type = libxl__domain_type(gc, domid);
     int hvm = 0, rc, num;
     int stubdomid = 0;
 
@@ -1181,8 +1182,7 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
     }
 
     rc = ERROR_FAIL;
-    switch (libxl__domain_type(gc, domid)) {
-    case LIBXL_DOMAIN_TYPE_HVM:
+    if (type == LIBXL_DOMAIN_TYPE_HVM) {
         hvm = 1;
         if (libxl__wait_for_device_model_deprecated(gc, domid, "running",
                                          NULL, NULL, NULL) < 0)
@@ -1203,8 +1203,8 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
             rc = ERROR_FAIL;
             goto out_fail;
         }
-        break;
-    case LIBXL_DOMAIN_TYPE_PV:
+    } else if (type != LIBXL_DOMAIN_TYPE_PV)
+        abort();
     {
         char *sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
                                          pcidev->bus, pcidev->dev, pcidev->func);
@@ -1254,10 +1254,6 @@ skip1:
             }
         }
         fclose(f);
-        break;
-    }
-    default:
-        abort();
     }
 out:
     /* don't do multiple resets while some functions are still passed through */
@@ -1435,6 +1431,69 @@ int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid)
     return 0;
 }
 
+int libxl__grant_vga_iomem_permission(libxl__gc *gc, const uint32_t domid,
+                                      libxl_domain_config *const d_config)
+{
+    int i, ret;
+
+    if (!d_config->b_info.u.hvm.gfx_passthru.val)
+        return 0;
+
+    for (i = 0 ; i < d_config->num_pcidevs ; i++) {
+        uint64_t vga_iomem_start = 0xa0000 >> XC_PAGE_SHIFT;
+        uint32_t stubdom_domid;
+        libxl_device_pci *pcidev = &d_config->pcidevs[i];
+        char *pci_device_class_path =
+            libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/class",
+                           pcidev->domain, pcidev->bus, pcidev->dev,
+                           pcidev->func);
+        int read_items;
+        unsigned long pci_device_class;
+
+        FILE *f = fopen(pci_device_class_path, "r");
+        if (!f) {
+            LOGE(ERROR,
+                 "pci device "PCI_BDF" does not have class attribute",
+                 pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+            continue;
+        }
+        read_items = fscanf(f, "0x%lx\n", &pci_device_class);
+        fclose(f);
+        if (read_items != 1) {
+            LOGE(ERROR,
+                 "cannot read class of pci device "PCI_BDF,
+                 pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+            continue;
+        }
+        if (pci_device_class != 0x030000) /* VGA class */
+            continue;
+
+        stubdom_domid = libxl_get_stubdom_id(CTX, domid);
+        ret = xc_domain_iomem_permission(CTX->xch, stubdom_domid,
+                                         vga_iomem_start, 0x20, 1);
+        if (ret < 0) {
+            LOGE(ERROR,
+                 "failed to give stubdom%d access to iomem range "
+                 "%"PRIx64"-%"PRIx64" for VGA passthru",
+                 stubdom_domid,
+                 vga_iomem_start, (vga_iomem_start + 0x20 - 1));
+            return ret;
+        }
+        ret = xc_domain_iomem_permission(CTX->xch, domid,
+                                         vga_iomem_start, 0x20, 1);
+        if (ret < 0) {
+            LOGE(ERROR,
+                 "failed to give dom%d access to iomem range "
+                 "%"PRIx64"-%"PRIx64" for VGA passthru",
+                 domid, vga_iomem_start, (vga_iomem_start + 0x20 - 1));
+            return ret;
+        }
+        break;
+    }
+
+    return 0;
+}
+
 /*
  * Local variables:
  * mode: C
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v11 13/14] tools/libxl: cleanup the do_pci_add() function
  2014-08-24 20:29 [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
                   ` (11 preceding siblings ...)
  2014-08-24 20:30 ` [PATCH v11 12/14] tools/libxl: explicitly grant access to needed I/O-memory ranges Arianna Avanzini
@ 2014-08-24 20:30 ` Arianna Avanzini
  2014-08-24 20:30 ` [PATCH v11 14/14] xen/common: do not implicitly permit access to mapped I/O memory Arianna Avanzini
  2014-08-29 13:41 ` [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Andrii Tseglytskyi
  14 siblings, 0 replies; 21+ messages in thread
From: Arianna Avanzini @ 2014-08-24 20:30 UTC (permalink / raw
  To: xen-devel
  Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
	Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
	andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
	andrii.tseglytskyi

This function modifies the do_pci_add() function in libxl_pci.c
by bringing back a code block whose condition was removed in the
previous commit. The block was left as is to facilitate functional
review of the previous commit; this commit cleans it up.
This commit introduces no functional change.

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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
 tools/libxl/libxl_pci.c | 122 ++++++++++++++++++++++++------------------------
 1 file changed, 61 insertions(+), 61 deletions(-)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 4fc9218..ef8cdaa 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -847,7 +847,10 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     libxl_domain_type type = libxl__domain_type(gc, domid);
-    int rc, hvm = 0;
+    char *sysfs_path;
+    FILE *f;
+    unsigned long long start, end, flags, size;
+    int irq, i, rc, hvm = 0;
 
     if (type == LIBXL_DOMAIN_TYPE_INVALID)
         return ERROR_FAIL;
@@ -872,73 +875,70 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
             return ERROR_FAIL;
     }
 
-    {
-        char *sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
-                                         pcidev->bus, pcidev->dev, pcidev->func);
-        FILE *f = fopen(sysfs_path, "r");
-        unsigned long long start = 0, end = 0, flags = 0, size = 0;
-        int irq = 0;
-        int i;
+    sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
+                                pcidev->bus, pcidev->dev, pcidev->func);
+    f = fopen(sysfs_path, "r");
+    start = end = flags = size = 0;
+    irq = 0;
 
-        if (f == NULL) {
-            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s", sysfs_path);
-            return ERROR_FAIL;
-        }
-        for (i = 0; i < PROC_PCI_NUM_RESOURCES; i++) {
-            if (fscanf(f, "0x%llx 0x%llx 0x%llx\n", &start, &end, &flags) != 3)
-                continue;
-            size = end - start + 1;
-            if (start) {
-                if (flags & PCI_BAR_IO) {
-                    rc = xc_domain_ioport_permission(ctx->xch, domid, start, size, 1);
-                    if (rc < 0) {
-                        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_ioport_permission error 0x%llx/0x%llx", start, size);
-                        fclose(f);
-                        return ERROR_FAIL;
-                    }
-                } else {
-                    rc = xc_domain_iomem_permission(ctx->xch, domid, start>>XC_PAGE_SHIFT,
-                                                    (size+(XC_PAGE_SIZE-1))>>XC_PAGE_SHIFT, 1);
-                    if (rc < 0) {
-                        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_iomem_permission error 0x%llx/0x%llx", start, size);
-                        fclose(f);
-                        return ERROR_FAIL;
-                    }
+    if (f == NULL) {
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s", sysfs_path);
+        return ERROR_FAIL;
+    }
+    for (i = 0; i < PROC_PCI_NUM_RESOURCES; i++) {
+        if (fscanf(f, "0x%llx 0x%llx 0x%llx\n", &start, &end, &flags) != 3)
+            continue;
+        size = end - start + 1;
+        if (start) {
+            if (flags & PCI_BAR_IO) {
+                rc = xc_domain_ioport_permission(ctx->xch, domid, start, size, 1);
+                if (rc < 0) {
+                    LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_ioport_permission error 0x%llx/0x%llx", start, size);
+                    fclose(f);
+                    return ERROR_FAIL;
+                }
+            } else {
+                rc = xc_domain_iomem_permission(ctx->xch, domid, start>>XC_PAGE_SHIFT,
+                                                (size+(XC_PAGE_SIZE-1))>>XC_PAGE_SHIFT, 1);
+                if (rc < 0) {
+                    LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_iomem_permission error 0x%llx/0x%llx", start, size);
+                    fclose(f);
+                    return ERROR_FAIL;
                 }
             }
         }
-        fclose(f);
-        sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/irq", pcidev->domain,
-                                   pcidev->bus, pcidev->dev, pcidev->func);
-        f = fopen(sysfs_path, "r");
-        if (f == NULL) {
-            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s", sysfs_path);
-            goto out;
+    }
+    fclose(f);
+    sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/irq", pcidev->domain,
+                                pcidev->bus, pcidev->dev, pcidev->func);
+    f = fopen(sysfs_path, "r");
+    if (f == NULL) {
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s", sysfs_path);
+        goto out;
+    }
+    if ((fscanf(f, "%u", &irq) == 1) && irq) {
+        rc = xc_physdev_map_pirq(ctx->xch, domid, irq, &irq);
+        if (rc < 0) {
+            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_physdev_map_pirq irq=%d", irq);
+            fclose(f);
+            return ERROR_FAIL;
         }
-        if ((fscanf(f, "%u", &irq) == 1) && irq) {
-            rc = xc_physdev_map_pirq(ctx->xch, domid, irq, &irq);
-            if (rc < 0) {
-                LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_physdev_map_pirq irq=%d", irq);
-                fclose(f);
-                return ERROR_FAIL;
-            }
-            rc = xc_domain_irq_permission(ctx->xch, domid, irq, 1);
-            if (rc < 0) {
-                LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_irq_permission irq=%d", irq);
-                fclose(f);
-                return ERROR_FAIL;
-            }
+        rc = xc_domain_irq_permission(ctx->xch, domid, irq, 1);
+        if (rc < 0) {
+            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_irq_permission irq=%d", irq);
+            fclose(f);
+            return ERROR_FAIL;
         }
-        fclose(f);
+    }
+    fclose(f);
 
-        /* Don't restrict writes to the PCI config space from this VM */
-        if (pcidev->permissive) {
-            if ( sysfs_write_bdf(gc, SYSFS_PCIBACK_DRIVER"/permissive",
-                                 pcidev) < 0 ) {
-                LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-                           "Setting permissive for device");
-                return ERROR_FAIL;
-            }
+    /* Don't restrict writes to the PCI config space from this VM */
+    if (pcidev->permissive) {
+        if ( sysfs_write_bdf(gc, SYSFS_PCIBACK_DRIVER"/permissive",
+                             pcidev) < 0 ) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                       "Setting permissive for device");
+            return ERROR_FAIL;
         }
     }
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v11 14/14] xen/common: do not implicitly permit access to mapped I/O memory
  2014-08-24 20:29 [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
                   ` (12 preceding siblings ...)
  2014-08-24 20:30 ` [PATCH v11 13/14] tools/libxl: cleanup the do_pci_add() function Arianna Avanzini
@ 2014-08-24 20:30 ` Arianna Avanzini
  2014-08-29 13:41 ` [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Andrii Tseglytskyi
  14 siblings, 0 replies; 21+ messages in thread
From: Arianna Avanzini @ 2014-08-24 20:30 UTC (permalink / raw
  To: xen-devel
  Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
	Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
	andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
	andrii.tseglytskyi

Currently, the XEN_DOMCTL_memory_mapping hypercall implicitly grants
to a domain access permission to the I/O memory areas mapped in its
guest address space. This conflicts with the presence of a specific
hypercall (XEN_DOMCTL_iomem_permission) used to grant such a permission
to a domain.
This commit separates the functions of the two hypercalls by having only
the latter be able to permit I/O memory access to a domain, and the
former just performing the mapping after a permissions check on both the
granting and the grantee domains.

Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Jan Beulich <JBeulich@suse.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: 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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>

---

    v8:
        - Drop iomem_permission-related changes.
        - Conservatively check both the granting and the grantee domains'
          permissions in the memory_mapping DOMCTL.
        - Remove tentative phrases from commit description.

    v7:
        - Let iomem_permission check if the calling domain is allowed to access
          memory ranges to be mapped to a domain. Remove such a check from the
          memory_mapping hypercall.

---
 xen/common/domctl.c | 36 ++++++++++--------------------------
 1 file changed, 10 insertions(+), 26 deletions(-)

diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 80b7800..04ecd53 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -917,7 +917,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
             break;
 
         ret = -EPERM;
-        if ( !iomem_access_permitted(current->domain, mfn, mfn_end) )
+        if ( !iomem_access_permitted(current->domain, mfn, mfn_end) ||
+             !iomem_access_permitted(d, mfn, mfn_end) )
             break;
 
         ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn_end, add);
@@ -930,40 +931,23 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
                    "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, nr_mfns, mfn);
-                if ( ret )
-                {
-                    printk(XENLOG_G_WARNING
-                           "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
-                               "memory_map: failed to deny dom%d access to [%lx,%lx]\n",
-                               d->domain_id, mfn, mfn_end);
-                }
-            }
+            ret = map_mmio_regions(d, gfn, nr_mfns, mfn);
+            if ( ret )
+                printk(XENLOG_G_WARNING
+                       "memory_map:fail: dom%d gfn=%lx mfn=%lx nr=%lx ret:%ld\n",
+                       d->domain_id, gfn, mfn, nr_mfns, ret);
         }
         else
         {
-            int rc = 0;
-
             printk(XENLOG_G_INFO
                    "memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
                    d->domain_id, gfn, mfn, nr_mfns);
 
-            rc = unmap_mmio_regions(d, gfn, nr_mfns, mfn);
-            ret = iomem_deny_access(d, mfn, mfn_end);
-            if ( !ret )
-                ret = rc;
+            ret = unmap_mmio_regions(d, gfn, nr_mfns, mfn);
             if ( ret && is_hardware_domain(current->domain) )
                 printk(XENLOG_ERR
-                       "memory_map: error %ld %s dom%d access to [%lx,%lx]\n",
-                       ret, rc ? "removing" : "denying", d->domain_id,
-                       mfn, mfn_end);
+                       "memory_map: error %ld removing dom%d access to [%lx,%lx]\n",
+                       ret, d->domain_id, mfn, mfn_end);
         }
         /* Do this unconditionally to cover errors on above failure paths. */
         memory_type_changed(d);
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v11 01/14] arch/arm: add consistency check to REMOVE p2m changes
  2014-08-24 20:29 ` [PATCH v11 01/14] arch/arm: add consistency check to REMOVE p2m changes Arianna Avanzini
@ 2014-08-27 14:46   ` Julien Grall
  0 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2014-08-27 14:46 UTC (permalink / raw
  To: Arianna Avanzini, xen-devel
  Cc: julien.grall, paolo.valente, keir, stefano.stabellini, tim,
	dario.faggioli, Ian.Jackson, Ian.Campbell, etrudeau, JBeulich,
	andrew.cooper3, viktor.kleinik, andrii.tseglytskyi

Hi Arianna,

On 24/08/14 16:29, Arianna Avanzini wrote:
> Currently, the REMOVE case of the switch in apply_p2m_changes()
> does not perform any consistency check on the mapping to be removed.
> More in detail, the code does not check if the guest address to be
> unmapped is actually mapped to the machine address given as a
> parameter.
> This commit adds the above-described consistency check to the REMOVEHi
> path of apply_p2m_changes() and lets a warning be emitted when trying
> to remove a non-existent mapping. This is instrumental to one of the
> following commits, which implements the possibility to trigger the
> removal of p2m ranges via the memory_mapping DOMCTL for ARM.
>
> Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Dario Faggioli <dario.faggioli@citrix.com>
> Cc: Paolo Valente <paolo.valente@unimore.it>

Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v11 02/14] arch/arm: unmap partially-mapped memory regions
  2014-08-24 20:29 ` [PATCH v11 02/14] arch/arm: unmap partially-mapped memory regions Arianna Avanzini
@ 2014-08-27 16:35   ` Julien Grall
  2014-08-28  9:19     ` Arianna Avanzini
  0 siblings, 1 reply; 21+ messages in thread
From: Julien Grall @ 2014-08-27 16:35 UTC (permalink / raw
  To: Arianna Avanzini, xen-devel
  Cc: julien.grall, paolo.valente, keir, stefano.stabellini, tim,
	dario.faggioli, Ian.Jackson, Ian.Campbell, etrudeau, JBeulich,
	andrew.cooper3, viktor.kleinik, andrii.tseglytskyi

Hi Arianna,

On 24/08/14 16:29, Arianna Avanzini wrote:
>   xen/arch/arm/p2m.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index b9129a0..0520f84 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -713,7 +713,7 @@ static int apply_p2m_changes(struct domain *d,
>       int rc, ret;
>       struct p2m_domain *p2m = &d->arch.p2m;
>       lpae_t *first = NULL, *second = NULL, *third = NULL;
> -    paddr_t addr;
> +    paddr_t addr, orig_maddr = maddr;
>       unsigned long cur_first_page = ~0,
>                     cur_first_offset = ~0,
>                     cur_second_offset = ~0;
> @@ -844,6 +844,10 @@ out:
>       if (third) unmap_domain_page(third);
>       if (second) unmap_domain_page(second);
>       if (first) unmap_domain_page(first);
> +    if ( rc < 0 && ( op == INSERT || op == ALLOCATE ) )
> +        apply_p2m_changes(d, REMOVE, start_gpaddr,
> +                          end_gpaddr, orig_maddr,
> +                          MATTR_DEV, p2m_invalid);

s/MATTR_DEV/mattr/

Also, with unmapping the whole range you may hit your warning added in 
the previous patch in some corner case.
It could happen when the end of range already contains another mapping. 
But, I don't think it happen often. Maybe we can live with it for now?

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v11 10/14] xsm/flask: avoid spurious error messages when mapping I/O-memory
  2014-08-24 20:30 ` [PATCH v11 10/14] xsm/flask: avoid spurious error messages when mapping I/O-memory Arianna Avanzini
@ 2014-08-27 16:40   ` Julien Grall
  0 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2014-08-27 16:40 UTC (permalink / raw
  To: Arianna Avanzini, xen-devel
  Cc: julien.grall, paolo.valente, keir, stefano.stabellini, tim,
	dario.faggioli, Ian.Jackson, Ian.Campbell, etrudeau, JBeulich,
	andrew.cooper3, viktor.kleinik, andrii.tseglytskyi

Hi Arianna,

On 24/08/14 16:30, Arianna Avanzini wrote:
> Currently, FLASK only handles the memory_mapping hypercall for the
> x86 architecture. In case the architecture is ARM, an error message
> will be printed regardless of how XSM has been configured. This
> commit lets the memory_mapping case be handled also for architectures
> other than x86.
>
> Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
> Cc: Dario Faggioli <dario.faggioli@citrix.com>
> Cc: Paolo Valente <paolo.valente@unimore.it>

Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v11 02/14] arch/arm: unmap partially-mapped memory regions
  2014-08-27 16:35   ` Julien Grall
@ 2014-08-28  9:19     ` Arianna Avanzini
  2014-08-29 19:19       ` Julien Grall
  0 siblings, 1 reply; 21+ messages in thread
From: Arianna Avanzini @ 2014-08-28  9:19 UTC (permalink / raw
  To: Julien Grall
  Cc: julien.grall, paolo.valente, keir, stefano.stabellini, tim,
	dario.faggioli, Ian.Jackson, xen-devel, Ian.Campbell, etrudeau,
	JBeulich, andrew.cooper3, viktor.kleinik, andrii.tseglytskyi

On Wed, Aug 27, 2014 at 12:35:54PM -0400, Julien Grall wrote:
> Hi Arianna,

Hello!

> 
> On 24/08/14 16:29, Arianna Avanzini wrote:
> >  xen/arch/arm/p2m.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> >diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> >index b9129a0..0520f84 100644
> >--- a/xen/arch/arm/p2m.c
> >+++ b/xen/arch/arm/p2m.c
> >@@ -713,7 +713,7 @@ static int apply_p2m_changes(struct domain *d,
> >      int rc, ret;
> >      struct p2m_domain *p2m = &d->arch.p2m;
> >      lpae_t *first = NULL, *second = NULL, *third = NULL;
> >-    paddr_t addr;
> >+    paddr_t addr, orig_maddr = maddr;
> >      unsigned long cur_first_page = ~0,
> >                    cur_first_offset = ~0,
> >                    cur_second_offset = ~0;
> >@@ -844,6 +844,10 @@ out:
> >      if (third) unmap_domain_page(third);
> >      if (second) unmap_domain_page(second);
> >      if (first) unmap_domain_page(first);
> >+    if ( rc < 0 && ( op == INSERT || op == ALLOCATE ) )
> >+        apply_p2m_changes(d, REMOVE, start_gpaddr,
> >+                          end_gpaddr, orig_maddr,
> >+                          MATTR_DEV, p2m_invalid);
> 
> s/MATTR_DEV/mattr/
> 
> Also, with unmapping the whole range you may hit your warning added in the
> previous patch in some corner case.
> It could happen when the end of range already contains another mapping. But,
> I don't think it happen often. Maybe we can live with it for now?
> 

Thank you for the correction and for pointing that corner case out. Since I
have to send a new version perhaps I'll try to handle also the case you
mentioned. Just to see if I understood things correctly: the correct behavior
would be to leave the mapping already existing at the end of the range as it
is?

I'll try to send a v12 within this week however.

Thank you,
Arianna


> Regards,
> 
> -- 
> Julien Grall

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM
  2014-08-24 20:29 [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
                   ` (13 preceding siblings ...)
  2014-08-24 20:30 ` [PATCH v11 14/14] xen/common: do not implicitly permit access to mapped I/O memory Arianna Avanzini
@ 2014-08-29 13:41 ` Andrii Tseglytskyi
  14 siblings, 0 replies; 21+ messages in thread
From: Andrii Tseglytskyi @ 2014-08-29 13:41 UTC (permalink / raw
  To: Arianna Avanzini
  Cc: Ian Campbell, paolo.valente, Keir Fraser, Stefano Stabellini,
	Ian Jackson, Dario Faggioli, Tim Deegan, xen-devel@lists.xen.org,
	Julien Grall, etrudeau, Jan Beulich, Andrew Cooper,
	Viktor Kleinik

Whole patch series is additionally verified on the following setup:

Platform - Jacinto6 (OMAP5) with two ARMv7 cores (Cortex A15), 1.5 Gb of RAM
Hypervisor - Xen 4.5 unstable (latest master branch)
Dom0 - Linux 3.8 running with 2 vcpus, 128 Mb RAM
DomU - Android 4.3

the following memory regions were successfully mapped to domU:

libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 48034-48037
(XEN) memory_map:add: dom1 gfn=48034 mfn=48034 nr=4
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 48243-48243
(XEN) memory_map:add: dom1 gfn=48243 mfn=48243 nr=1
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 48281-48281
(XEN) memory_map:add: dom1 gfn=48281 mfn=48281 nr=1
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 48842-48845
(XEN) memory_map:add: dom1 gfn=48842 mfn=48842 nr=4
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 4a002-4a002
(XEN) memory_map:add: dom1 gfn=4a002 mfn=4a002 nr=1
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 4a005-4a005
(XEN) memory_map:add: dom1 gfn=4a005 mfn=4a005 nr=1
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 4a008-4a009
(XEN) memory_map:add: dom1 gfn=4a008 mfn=4a008 nr=2
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 4ae06-4ae07
(XEN) memory_map:add: dom1 gfn=4ae06 mfn=4ae06 nr=2
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 4ae0c-4ae0c
(XEN) memory_map:add: dom1 gfn=4ae0c mfn=4ae0c nr=1
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 55080-55080
(XEN) memory_map:add: dom1 gfn=55080 mfn=55080 nr=1
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 55083-55083
(XEN) memory_map:add: dom1 gfn=55083 mfn=55083 nr=1
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 56001-5600f
(XEN) memory_map:add: dom1 gfn=56001 mfn=56001 nr=f
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 95900-95eff
(XEN) memory_map:add: dom1 gfn=95900 mfn=95900 nr=600
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 95f00-989ff
(XEN) memory_map:add: dom1 gfn=95f00 mfn=95f00 nr=2b00
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 98a00-98aff
(XEN) memory_map:add: dom1 gfn=98a00 mfn=98a00 nr=100
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 95800-958ff
(XEN) memory_map:add: dom1 gfn=95800 mfn=95800 nr=100
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem ba300-bfcff
(XEN) memory_map:add: dom1 gfn=ba300 mfn=ba300 nr=5a00
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 60000-6ffff
(XEN) memory_map:add: dom1 gfn=60000 mfn=60000 nr=10000
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 70000-77fff
(XEN) memory_map:add: dom1 gfn=70000 mfn=70000 nr=8000
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 78000-7ffff
(XEN) memory_map:add: dom1 gfn=78000 mfn=78000 nr=8000
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 4a000-4afff
(XEN) memory_map:add: dom1 gfn=4a000 mfn=4a000 nr=1000
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 48000-481ff
(XEN) memory_map:add: dom1 gfn=48000 mfn=48000 nr=200
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 48400-487ff
(XEN) memory_map:add: dom1 gfn=48400 mfn=48400 nr=400
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 48800-48fff
(XEN) memory_map:add: dom1 gfn=48800 mfn=48800 nr=800
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 54000-54fff
(XEN) memory_map:add: dom1 gfn=54000 mfn=54000 nr=1000
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 5a000-5afff
(XEN) memory_map:add: dom1 gfn=5a000 mfn=5a000 nr=1000
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 5b000-5bfff
(XEN) memory_map:add: dom1 gfn=5b000 mfn=5b000 nr=1000
libxl: debug: libxl_create.c:1171:domcreate_launch_dm: dom1 iomem 4e000-4e0ff
(XEN) memory_map:add: dom1 gfn=4e000 mfn=4e000 nr=100

Regards,
Andrii

On Sun, Aug 24, 2014 at 11:29 PM, Arianna Avanzini
<avanzini.arianna@gmail.com> wrote:
> Hello,
>
> here is the eleventh version of my implementation proposal for the DOMCTL
> memory_mapping for the ARM architecture. As usual, I'll try to keep this
> as short as I can by just going through the most relevant changes. A more
> detailed changelog is in the description of each of the commits, while an
> extensive summary of the patch series' genesis can be found in the last
> full cover letter ([1]).
> Please note that this patchset includes a contribution from Andrii Tseglytskyi,
> as you may also see from the shortlog below.
>
> Patch 0001 has been fixed according to suggestions given by Julien Grall and
> approved by Ian Campbell. Now it handles even level 3 mappings. Also, as
> suggested by Andrew Cooper, the printk uses a XENLOG_G_WARNING loglevel so
> that it can be rate-limited appropriately.
>
> Patch 0002 has also been changed according to directives provided by Julien
> Grall. Now partially-mapped memory regions are unmapped regardless of their
> being I/O-memory or not.
>
> Patch 0008 has been fixed according to directive from Julien Grall.
>
> Patch 0010 has been added to allow FLASK to handle the memory_mapping case
> also on ARM architecture, therefore preventing wrong error messages from
> being printed, as indicated by Julien Grall.
>
> A new patch 0011, contributed by Andrii Tseglytskyi, has been merged into
> the series. It adds a FLASK policy rule to allow a domU to access previously-
> mapped I/O-memory regions.
>
> Patch 0012 has been modified according to suggestions provided by Ian Campbell.
> The VGA-related bits have been moved to a libxl_pci helper. Non-functional
> changes have been split into patch 0013 to facilitate review. A leftover debug
> print has been removed.
>
> As usual, the code has again been tested on a cubieboard2, with Linux v3.15-rc3
> as a dom0 and ERIKA Enterprise ([3]) as a domU. The x86 bits have been tested on
> an x86_64 machine with Linux 3.15.0-rc5 as both dom0 and domU.
> Any feedback about this new version of the patchset is more than welcome,
> Arianna
>
> [1] http://lists.xen.org/archives/html/xen-devel/2014-03/msg01724.html
> [2] http://markmail.org/message/7t6erxfy7pgorakb
> [3] http://erika.tuxfamily.org/drupal/
>
> Andrii Tseglytskyi (1):
>   flask/policy: allow domU to use previously-mapped I/O-memory
>
> Arianna Avanzini (13):
>   arch/arm: add consistency check to REMOVE p2m changes
>   arch/arm: unmap partially-mapped memory regions
>   arch/x86: warn if to-be-removed mapping does not exist
>   arch/x86: cleanup memory_mapping DOMCTL
>   xen/common: add ARM stub for the function memory_type_changed()
>   xen/x86: factor out map and unmap from the memory_mapping DOMCTL
>   xen/common: move the memory_mapping DOMCTL hypercall to common code
>   tools/libxl: parse optional start gfn from the iomem config option
>   tools/libxl: handle the iomem parameter with the memory_mapping hcall
>   xsm/flask: avoid spurious error messages when mapping I/O-memory
>   tools/libxl: explicitly grant access to needed I/O-memory ranges
>   tools/libxl: cleanup the do_pci_add() function
>   xen/common: do not implicitly permit access to mapped I/O memory
>
>  docs/man/xl.cfg.pod.5                        |  18 ++-
>  tools/flask/policy/policy/modules/xen/xen.te |   1 +
>  tools/libxc/xc_domain.c                      |  10 ++
>  tools/libxl/libxl.h                          |  10 ++
>  tools/libxl/libxl_create.c                   |  28 +++-
>  tools/libxl/libxl_internal.h                 |   3 +
>  tools/libxl/libxl_pci.c                      | 213 +++++++++++++++++----------
>  tools/libxl/libxl_types.idl                  |   4 +
>  tools/libxl/xl_cmdimpl.c                     |  17 ++-
>  xen/arch/arm/p2m.c                           |  44 +++++-
>  xen/arch/x86/domctl.c                        |  76 ----------
>  xen/arch/x86/mm/p2m.c                        |  57 ++++++-
>  xen/common/domctl.c                          |  54 ++++++-
>  xen/common/memory.c                          |   2 +-
>  xen/include/asm-arm/p2m.h                    |  13 +-
>  xen/include/asm-x86/p2m.h                    |   3 +-
>  xen/include/xen/p2m-common.h                 |  16 ++
>  xen/xsm/flask/hooks.c                        |   2 +-
>  18 files changed, 385 insertions(+), 186 deletions(-)
>  create mode 100644 xen/include/xen/p2m-common.h
>
> --
> 2.1.0
>



-- 

Andrii Tseglytskyi | Embedded Dev
GlobalLogic
www.globallogic.com

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v11 02/14] arch/arm: unmap partially-mapped memory regions
  2014-08-28  9:19     ` Arianna Avanzini
@ 2014-08-29 19:19       ` Julien Grall
  0 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2014-08-29 19:19 UTC (permalink / raw
  To: Arianna Avanzini
  Cc: julien.grall, paolo.valente, keir, stefano.stabellini, tim,
	dario.faggioli, Ian.Jackson, xen-devel, Ian.Campbell, etrudeau,
	JBeulich, andrew.cooper3, viktor.kleinik, andrii.tseglytskyi



On 28/08/14 05:19, Arianna Avanzini wrote:
> On Wed, Aug 27, 2014 at 12:35:54PM -0400, Julien Grall wrote:
>> Hi Arianna,
>
> Hello!

Hi Arianna,

> Thank you for the correction and for pointing that corner case out. Since I
> have to send a new version perhaps I'll try to handle also the case you
> mentioned. Just to see if I understood things correctly: the correct behavior
> would be to leave the mapping already existing at the end of the range as it
> is?

The main point is too unmap only what you have mapped during the call of 
apply_p2m_changes.

Otherwise you may get spurious warning message on Xen console and/or 
remove mapping that you haven't mapped.

But, for the latter case the mapping would have been override if the 
call was succeeded.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2014-08-29 19:19 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-24 20:29 [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
2014-08-24 20:29 ` [PATCH v11 01/14] arch/arm: add consistency check to REMOVE p2m changes Arianna Avanzini
2014-08-27 14:46   ` Julien Grall
2014-08-24 20:29 ` [PATCH v11 02/14] arch/arm: unmap partially-mapped memory regions Arianna Avanzini
2014-08-27 16:35   ` Julien Grall
2014-08-28  9:19     ` Arianna Avanzini
2014-08-29 19:19       ` Julien Grall
2014-08-24 20:29 ` [PATCH v11 03/14] arch/x86: warn if to-be-removed mapping does not exist Arianna Avanzini
2014-08-24 20:29 ` [PATCH v11 04/14] arch/x86: cleanup memory_mapping DOMCTL Arianna Avanzini
2014-08-24 20:29 ` [PATCH v11 05/14] xen/common: add ARM stub for the function memory_type_changed() Arianna Avanzini
2014-08-24 20:29 ` [PATCH v11 06/14] xen/x86: factor out map and unmap from the memory_mapping DOMCTL Arianna Avanzini
2014-08-24 20:30 ` [PATCH v11 07/14] xen/common: move the memory_mapping DOMCTL hypercall to common code Arianna Avanzini
2014-08-24 20:30 ` [PATCH v11 08/14] tools/libxl: parse optional start gfn from the iomem config option Arianna Avanzini
2014-08-24 20:30 ` [PATCH v11 09/14] tools/libxl: handle the iomem parameter with the memory_mapping hcall Arianna Avanzini
2014-08-24 20:30 ` [PATCH v11 10/14] xsm/flask: avoid spurious error messages when mapping I/O-memory Arianna Avanzini
2014-08-27 16:40   ` Julien Grall
2014-08-24 20:30 ` [PATCH v11 11/14] flask/policy: allow domU to use previously-mapped I/O-memory Arianna Avanzini
2014-08-24 20:30 ` [PATCH v11 12/14] tools/libxl: explicitly grant access to needed I/O-memory ranges Arianna Avanzini
2014-08-24 20:30 ` [PATCH v11 13/14] tools/libxl: cleanup the do_pci_add() function Arianna Avanzini
2014-08-24 20:30 ` [PATCH v11 14/14] xen/common: do not implicitly permit access to mapped I/O memory Arianna Avanzini
2014-08-29 13:41 ` [PATCH v11 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Andrii Tseglytskyi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.