[PATCH 00/24] xen/arm: Extend the usage of typesafe MFN

[PATCH 00/24] xen/arm: Extend the usage of typesafe MFN

[Julien Grall]

Hello all,

This patch series extend the usage of typesafe MFN in the ARM code. _mfn(...)
and mfn_x(...) are pushed further down in the call stack.

Cheers,

Julien Grall (24):
  xen/mm: Don't use _{g,m}fn for defining INVALID_{G,M}FN
  xen/arm: gic-v2: Fix indentation in gicv2_map_hwdom_extra_mappings
  xen/arm: setup: Remove bogus xenheap_mfn_end in setup_mm for arm64
  xen/arm: mm: Introduce clear_table and use it
  xen/arm: mm: Move mfn_to_xen_entry from page.h to mm.c
  xen/arm: mm: Fix coding style of mfn_to_xen_entry
  xen/arm: mm: Clean-up mfn_to_xen_entry
  xen/arm: mm: Use typesafe MFN in mfn_to_xen_entry
  xen/arm: Define mfn_to_page/page_to_mfn in term of
    __mfn_to_page/__page_to_mfn
  xen/arm: domain_build: Replace paddr_to_pfn(virt_to_maddr(.)) by
    virt_to_mfn(.)
  xen/arm: mm: Replace __va(pfn_to_paddr(...)) by mfn_to_virt
  xen/arm: Replace DIV_ROUND_UP(..., PAGE_SIZE) by PFN_UP(...)
  xen/arm: traps: Replace p2m_lookup(..., ..., NULL) by gfn_to_mfn(...,
    ...)
  xen/arm: Introduce wrappers for MFN <-> MADDR and GFN <-> GADDR
  xen/arm: Use the newly introduced MFN <-> MADDR and GFN <-> MADDR
    helpers
  xen/arm: mm: Use typesafe mfn for xenheap_mfn_*
  xen/arm: mm: Use typesafe MFN in set_fixmap
  xen/arm: mm: Use typesafe MFN in dump_pt_walk
  xen/arm: p2m: Redefine mfn_to_page and page_to_mfn to use typesafe
  xen/arm: mm: Redefine virt_to_mfn to support typesafe
  xen/arm: domain_build: Redefine virt_to_mfn to support typesafe
  xen/arm: alternative: Redefine virt_to_mfn to support typesafe
  xen/arm: livepatch: Redefine virt_to_mfn to support typesafe
  xen/arm: create_xen_entries: Use typesafe MFN

 xen/arch/arm/acpi/lib.c           |   4 +-
 xen/arch/arm/alternative.c        |   6 +-
 xen/arch/arm/domain_build.c       |  22 ++---
 xen/arch/arm/gic-v2.c             |   6 +-
 xen/arch/arm/gic-v3.c             |   8 +-
 xen/arch/arm/kernel.c             |   8 +-
 xen/arch/arm/livepatch.c          |   6 +-
 xen/arch/arm/mem_access.c         |  10 +--
 xen/arch/arm/mm.c                 | 166 +++++++++++++++++++++++++++-----------
 xen/arch/arm/p2m.c                |  28 ++++---
 xen/arch/arm/platforms/exynos5.c  |   8 +-
 xen/arch/arm/platforms/omap5.c    |  16 ++--
 xen/arch/arm/platforms/vexpress.c |   2 +-
 xen/arch/arm/setup.c              |  20 +++--
 xen/arch/arm/traps.c              |  16 ++--
 xen/arch/arm/vgic-v2.c            |   4 +-
 xen/drivers/video/arm_hdlcd.c     |   2 +-
 xen/include/asm-arm/mm.h          |  33 +++++---
 xen/include/asm-arm/page.h        |  65 ---------------
 xen/include/xen/mm.h              |   4 +-
 20 files changed, 235 insertions(+), 199 deletions(-)

-- 
2.11.0


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

[PATCH 01/24] xen/mm: Don't use _{g, m}fn for defining INVALID_{G, M}FN

[Julien Grall]

INVALID_{G,M}FN are defined using static inline helpers _{g,m}fn.
This means, they cannot be used to initialize a build time static variable:

In file included from mm.c:24:0:
xen/xen/include/xen/mm.h:59:26: error: initializer element is not constant
 #define INVALID_MFN      _mfn(~0UL)

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>

I know that this solution will not work for non-debug build. I would
like input from the community on way to fix it nicely.
---
 xen/include/xen/mm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index ee50d4cd7b..f0daae6b5c 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -56,7 +56,7 @@
 
 TYPE_SAFE(unsigned long, mfn);
 #define PRI_mfn          "05lx"
-#define INVALID_MFN      _mfn(~0UL)
+#define INVALID_MFN      (mfn_t){ ~0UL }
 
 #ifndef mfn_t
 #define mfn_t /* Grep fodder: mfn_t, _mfn() and mfn_x() are defined above */
@@ -85,7 +85,7 @@ static inline bool_t mfn_eq(mfn_t x, mfn_t y)
 
 TYPE_SAFE(unsigned long, gfn);
 #define PRI_gfn          "05lx"
-#define INVALID_GFN      _gfn(~0UL)
+#define INVALID_GFN      (gfn_t){ ~0UL }
 
 #ifndef gfn_t
 #define gfn_t /* Grep fodder: gfn_t, _gfn() and gfn_x() are defined above */
-- 
2.11.0


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

[PATCH 02/24] xen/arm: gic-v2: Fix indentation in gicv2_map_hwdom_extra_mappings

[Julien Grall]

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/gic-v2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 270a1362ec..f8124e5e54 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -598,8 +598,8 @@ static int gicv2_map_hwdown_extra_mappings(struct domain *d)
                v2m_data->spi_start, v2m_data->nr_spis);
 
         ret = map_mmio_regions(d, _gfn(paddr_to_pfn(v2m_data->addr)),
-                            DIV_ROUND_UP(v2m_data->size, PAGE_SIZE),
-                            _mfn(paddr_to_pfn(v2m_data->addr)));
+                               DIV_ROUND_UP(v2m_data->size, PAGE_SIZE),
+                               _mfn(paddr_to_pfn(v2m_data->addr)));
         if ( ret )
         {
             printk(XENLOG_ERR "GICv2: Map v2m frame to d%d failed.\n",
-- 
2.11.0


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

[PATCH 03/24] xen/arm: setup: Remove bogus xenheap_mfn_end in setup_mm for arm64

[Julien Grall]

xenheap_mfn_end is storing an MFN and not a physical address. Thankfully
xenheap_mfn_end is not used in the arm64 code. So drop it.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/setup.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index f00f29a45b..ab4d8e4218 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -654,8 +654,6 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
             if ( e > bank_end )
                 e = bank_end;
 
-            xenheap_mfn_end = e;
-
             dt_unreserved_regions(s, e, init_boot_pages, 0);
             s = n;
         }
-- 
2.11.0


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

[PATCH 04/24] xen/arm: mm: Introduce clear_table and use it

[Julien Grall]

Add a new helper clear_table to clear a page table entry and invalidate
the cache.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/mm.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 082c872c72..b4ff777b55 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -529,6 +529,13 @@ void __init remove_early_mappings(void)
 
 extern void relocate_xen(uint64_t ttbr, void *src, void *dst, size_t len);
 
+/* Clear a translation table and clean & invalidate the cache */
+static void clear_table(void *table)
+{
+    clear_page(table);
+    clean_and_invalidate_dcache_va_range(table, PAGE_SIZE);
+}
+
 /* Boot-time pagetable setup.
  * Changes here may need matching changes in head.S */
 void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
@@ -604,18 +611,13 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
 
     /* Clear the copy of the boot pagetables. Each secondary CPU
      * rebuilds these itself (see head.S) */
-    memset(boot_pgtable, 0x0, PAGE_SIZE);
-    clean_and_invalidate_dcache(boot_pgtable);
+    clear_table(boot_pgtable);
 #ifdef CONFIG_ARM_64
-    memset(boot_first, 0x0, PAGE_SIZE);
-    clean_and_invalidate_dcache(boot_first);
-    memset(boot_first_id, 0x0, PAGE_SIZE);
-    clean_and_invalidate_dcache(boot_first_id);
+    clear_table(boot_first);
+    clear_table(boot_first_id);
 #endif
-    memset(boot_second, 0x0, PAGE_SIZE);
-    clean_and_invalidate_dcache(boot_second);
-    memset(boot_third, 0x0, PAGE_SIZE);
-    clean_and_invalidate_dcache(boot_third);
+    clear_table(boot_second);
+    clear_table(boot_third);
 
     /* Break up the Xen mapping into 4k pages and protect them separately. */
     for ( i = 0; i < LPAE_ENTRIES; i++ )
-- 
2.11.0


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

[PATCH 05/24] xen/arm: mm: Move mfn_to_xen_entry from page.h to mm.c

[Julien Grall]

The file mm.c is the only user of mfn_to_xen_entry. This will also help
to use the typesafe MFN.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/mm.c          | 65 ++++++++++++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/page.h | 65 ----------------------------------------------
 2 files changed, 65 insertions(+), 65 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index b4ff777b55..587a6b3975 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -254,6 +254,71 @@ void dump_hyp_walk(vaddr_t addr)
     dump_pt_walk(ttbr, addr, HYP_PT_ROOT_LEVEL, 1);
 }
 
+/* Standard entry type that we'll use to build Xen's own pagetables.
+ * We put the same permissions at every level, because they're ignored
+ * by the walker in non-leaf entries. */
+static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
+{
+    paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT;
+    lpae_t e = (lpae_t) {
+        .pt = {
+            .valid = 1,           /* Mappings are present */
+            .table = 0,           /* Set to 1 for links and 4k maps */
+            .ai = attr,
+            .ns = 1,              /* Hyp mode is in the non-secure world */
+            .user = 1,            /* See below */
+            .ro = 0,              /* Assume read-write */
+            .af = 1,              /* No need for access tracking */
+            .ng = 1,              /* Makes TLB flushes easier */
+            .contig = 0,          /* Assume non-contiguous */
+            .xn = 1,              /* No need to execute outside .text */
+            .avail = 0,           /* Reference count for domheap mapping */
+        }};;
+    /* Setting the User bit is strange, but the ATS1H[RW] instructions
+     * don't seem to work otherwise, and since we never run on Xen
+     * pagetables in User mode it's OK.  If this changes, remember
+     * to update the hard-coded values in head.S too */
+
+    switch ( attr )
+    {
+    case BUFFERABLE:
+        /*
+         * ARM ARM: Overlaying the shareability attribute (DDI
+         * 0406C.b B3-1376 to 1377)
+         *
+         * A memory region with a resultant memory type attribute of Normal,
+         * and a resultant cacheability attribute of Inner Non-cacheable,
+         * Outer Non-cacheable, must have a resultant shareability attribute
+         * of Outer Shareable, otherwise shareability is UNPREDICTABLE.
+         *
+         * On ARMv8 sharability is ignored and explicitly treated as Outer
+         * Shareable for Normal Inner Non_cacheable, Outer Non-cacheable.
+         */
+        e.pt.sh = LPAE_SH_OUTER;
+        break;
+    case UNCACHED:
+    case DEV_SHARED:
+        /* Shareability is ignored for non-Normal memory, Outer is as
+         * good as anything.
+         *
+         * On ARMv8 sharability is ignored and explicitly treated as Outer
+         * Shareable for any device memory type.
+         */
+        e.pt.sh = LPAE_SH_OUTER;
+        break;
+    default:
+        e.pt.sh = LPAE_SH_INNER;  /* Xen mappings are SMP coherent */
+        break;
+    }
+
+    ASSERT(!(pa & ~PAGE_MASK));
+    ASSERT(!(pa & ~PADDR_MASK));
+
+    // XXX shifts
+    e.bits |= pa;
+    return e;
+}
+
 /* Map a 4k page in a fixmap entry */
 void set_fixmap(unsigned map, unsigned long mfn, unsigned attributes)
 {
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 497b4c86ad..3670ab665d 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -205,71 +205,6 @@ typedef union {
     lpae_walk_t walk;
 } lpae_t;
 
-/* Standard entry type that we'll use to build Xen's own pagetables.
- * We put the same permissions at every level, because they're ignored
- * by the walker in non-leaf entries. */
-static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
-{
-    paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT;
-    lpae_t e = (lpae_t) {
-        .pt = {
-            .valid = 1,           /* Mappings are present */
-            .table = 0,           /* Set to 1 for links and 4k maps */
-            .ai = attr,
-            .ns = 1,              /* Hyp mode is in the non-secure world */
-            .user = 1,            /* See below */
-            .ro = 0,              /* Assume read-write */
-            .af = 1,              /* No need for access tracking */
-            .ng = 1,              /* Makes TLB flushes easier */
-            .contig = 0,          /* Assume non-contiguous */
-            .xn = 1,              /* No need to execute outside .text */
-            .avail = 0,           /* Reference count for domheap mapping */
-        }};;
-    /* Setting the User bit is strange, but the ATS1H[RW] instructions
-     * don't seem to work otherwise, and since we never run on Xen
-     * pagetables in User mode it's OK.  If this changes, remember
-     * to update the hard-coded values in head.S too */
-
-    switch ( attr )
-    {
-    case BUFFERABLE:
-        /*
-         * ARM ARM: Overlaying the shareability attribute (DDI
-         * 0406C.b B3-1376 to 1377)
-         *
-         * A memory region with a resultant memory type attribute of Normal,
-         * and a resultant cacheability attribute of Inner Non-cacheable,
-         * Outer Non-cacheable, must have a resultant shareability attribute
-         * of Outer Shareable, otherwise shareability is UNPREDICTABLE.
-         *
-         * On ARMv8 sharability is ignored and explicitly treated as Outer
-         * Shareable for Normal Inner Non_cacheable, Outer Non-cacheable.
-         */
-        e.pt.sh = LPAE_SH_OUTER;
-        break;
-    case UNCACHED:
-    case DEV_SHARED:
-        /* Shareability is ignored for non-Normal memory, Outer is as
-         * good as anything.
-         *
-         * On ARMv8 sharability is ignored and explicitly treated as Outer
-         * Shareable for any device memory type.
-         */
-        e.pt.sh = LPAE_SH_OUTER;
-        break;
-    default:
-        e.pt.sh = LPAE_SH_INNER;  /* Xen mappings are SMP coherent */
-        break;
-    }
-
-    ASSERT(!(pa & ~PAGE_MASK));
-    ASSERT(!(pa & ~PADDR_MASK));
-
-    // XXX shifts
-    e.bits |= pa;
-    return e;
-}
-
 #if defined(CONFIG_ARM_32)
 # include <asm/arm32/page.h>
 #elif defined(CONFIG_ARM_64)
-- 
2.11.0


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

[PATCH 06/24] xen/arm: mm: Fix coding style of mfn_to_xen_entry

[Julien Grall]

Fix the comment coding style and add a newline before the return.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/mm.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 587a6b3975..6f63e4315a 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -254,9 +254,11 @@ void dump_hyp_walk(vaddr_t addr)
     dump_pt_walk(ttbr, addr, HYP_PT_ROOT_LEVEL, 1);
 }
 
-/* Standard entry type that we'll use to build Xen's own pagetables.
+/*
+ * Standard entry type that we'll use to build Xen's own pagetables.
  * We put the same permissions at every level, because they're ignored
- * by the walker in non-leaf entries. */
+ * by the walker in non-leaf entries.
+ */
 static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
 {
     paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT;
@@ -274,10 +276,12 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
             .xn = 1,              /* No need to execute outside .text */
             .avail = 0,           /* Reference count for domheap mapping */
         }};;
-    /* Setting the User bit is strange, but the ATS1H[RW] instructions
+    /*
+     * Setting the User bit is strange, but the ATS1H[RW] instructions
      * don't seem to work otherwise, and since we never run on Xen
      * pagetables in User mode it's OK.  If this changes, remember
-     * to update the hard-coded values in head.S too */
+     * to update the hard-coded values in head.S too.
+     */
 
     switch ( attr )
     {
@@ -298,7 +302,8 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
         break;
     case UNCACHED:
     case DEV_SHARED:
-        /* Shareability is ignored for non-Normal memory, Outer is as
+        /*
+         * Shareability is ignored for non-Normal memory, Outer is as
          * good as anything.
          *
          * On ARMv8 sharability is ignored and explicitly treated as Outer
@@ -314,8 +319,9 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
     ASSERT(!(pa & ~PAGE_MASK));
     ASSERT(!(pa & ~PADDR_MASK));
 
-    // XXX shifts
+    /* XXX shifts */
     e.bits |= pa;
+
     return e;
 }
 
-- 
2.11.0


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

[PATCH 07/24] xen/arm: mm: Clean-up mfn_to_xen_entry

[Julien Grall]

The physical address is computed from the machine frame number, so
checking if the physical address is page aligned is pointless.

Furthermore, directly assigned the MFN to the corresponding field in the
entry rather than converting to a physical address and orring the value.
It will avoid to rely on the field position and make the code clearer.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/mm.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 6f63e4315a..d164ed2eda 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -261,7 +261,6 @@ void dump_hyp_walk(vaddr_t addr)
  */
 static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
 {
-    paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT;
     lpae_t e = (lpae_t) {
         .pt = {
             .valid = 1,           /* Mappings are present */
@@ -316,11 +315,9 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
         break;
     }
 
-    ASSERT(!(pa & ~PAGE_MASK));
-    ASSERT(!(pa & ~PADDR_MASK));
+    ASSERT(!(pfn_to_paddr(mfn) & ~PADDR_MASK));
 
-    /* XXX shifts */
-    e.bits |= pa;
+    e.pt.base = mfn;
 
     return e;
 }
-- 
2.11.0


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

[PATCH 08/24] xen/arm: mm: Use typesafe MFN in mfn_to_xen_entry

[Julien Grall]

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/mm.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index d164ed2eda..08116679ec 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -259,7 +259,7 @@ void dump_hyp_walk(vaddr_t addr)
  * We put the same permissions at every level, because they're ignored
  * by the walker in non-leaf entries.
  */
-static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
+static inline lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned attr)
 {
     lpae_t e = (lpae_t) {
         .pt = {
@@ -315,9 +315,9 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
         break;
     }
 
-    ASSERT(!(pfn_to_paddr(mfn) & ~PADDR_MASK));
+    ASSERT(!(pfn_to_paddr(mfn_x(mfn)) & ~PADDR_MASK));
 
-    e.pt.base = mfn;
+    e.pt.base = mfn_x(mfn);
 
     return e;
 }
@@ -325,7 +325,7 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
 /* Map a 4k page in a fixmap entry */
 void set_fixmap(unsigned map, unsigned long mfn, unsigned attributes)
 {
-    lpae_t pte = mfn_to_xen_entry(mfn, attributes);
+    lpae_t pte = mfn_to_xen_entry(_mfn(mfn), attributes);
     pte.pt.table = 1; /* 4k mappings always have this bit set */
     pte.pt.xn = 1;
     write_pte(xen_fixmap + third_table_offset(FIXMAP_ADDR(map)), pte);
@@ -363,7 +363,7 @@ static void __init create_mappings(lpae_t *second,
 
     count = nr_mfns / LPAE_ENTRIES;
     p = second + second_linear_offset(virt_offset);
-    pte = mfn_to_xen_entry(base_mfn, WRITEALLOC);
+    pte = mfn_to_xen_entry(_mfn(base_mfn), WRITEALLOC);
     if ( granularity == 16 * LPAE_ENTRIES )
         pte.pt.contig = 1;  /* These maps are in 16-entry contiguous chunks. */
     for ( i = 0; i < count; i++ )
@@ -416,7 +416,7 @@ void *map_domain_page(mfn_t mfn)
         else if ( map[slot].pt.avail == 0 )
         {
             /* Commandeer this 2MB slot */
-            pte = mfn_to_xen_entry(slot_mfn, WRITEALLOC);
+            pte = mfn_to_xen_entry(_mfn(slot_mfn), WRITEALLOC);
             pte.pt.avail = 1;
             write_pte(map + slot, pte);
             break;
@@ -537,7 +537,7 @@ static inline lpae_t pte_of_xenaddr(vaddr_t va)
 {
     paddr_t ma = va + phys_offset;
     unsigned long mfn = ma >> PAGE_SHIFT;
-    return mfn_to_xen_entry(mfn, WRITEALLOC);
+    return mfn_to_xen_entry(_mfn(mfn), WRITEALLOC);
 }
 
 /* Map the FDT in the early boot page table */
@@ -646,7 +646,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
     /* Initialise xen second level entries ... */
     /* ... Xen's text etc */
 
-    pte = mfn_to_xen_entry(xen_paddr>>PAGE_SHIFT, WRITEALLOC);
+    pte = mfn_to_xen_entry(_mfn(xen_paddr>>PAGE_SHIFT), WRITEALLOC);
     pte.pt.xn = 0;/* Contains our text mapping! */
     xen_second[second_table_offset(XEN_VIRT_START)] = pte;
 
@@ -663,7 +663,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
 
     /* ... Boot Misc area for xen relocation */
     dest_va = BOOT_RELOC_VIRT_START;
-    pte = mfn_to_xen_entry(xen_paddr >> PAGE_SHIFT, WRITEALLOC);
+    pte = mfn_to_xen_entry(_mfn(xen_paddr >> PAGE_SHIFT), WRITEALLOC);
     /* Map the destination in xen_second. */
     xen_second[second_table_offset(dest_va)] = pte;
     /* Map the destination in boot_second. */
@@ -694,7 +694,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
         unsigned long va = XEN_VIRT_START + (i << PAGE_SHIFT);
         if ( !is_kernel(va) )
             break;
-        pte = mfn_to_xen_entry(mfn, WRITEALLOC);
+        pte = mfn_to_xen_entry(_mfn(mfn), WRITEALLOC);
         pte.pt.table = 1; /* 4k mappings always have this bit set */
         if ( is_kernel_text(va) || is_kernel_inittext(va) )
         {
@@ -764,7 +764,8 @@ int init_secondary_pagetables(int cpu)
      * domheap mapping pages. */
     for ( i = 0; i < DOMHEAP_SECOND_PAGES; i++ )
     {
-        pte = mfn_to_xen_entry(virt_to_mfn(domheap+i*LPAE_ENTRIES), WRITEALLOC);
+        pte = mfn_to_xen_entry(_mfn(virt_to_mfn(domheap+i*LPAE_ENTRIES)),
+                               WRITEALLOC);
         pte.pt.table = 1;
         write_pte(&first[first_table_offset(DOMHEAP_VIRT_START+i*FIRST_SIZE)], pte);
     }
@@ -862,13 +863,13 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
             unsigned long first_mfn = alloc_boot_pages(1, 1);
 
             clear_page(mfn_to_virt(first_mfn));
-            pte = mfn_to_xen_entry(first_mfn, WRITEALLOC);
+            pte = mfn_to_xen_entry(_mfn(first_mfn), WRITEALLOC);
             pte.pt.table = 1;
             write_pte(p, pte);
             first = mfn_to_virt(first_mfn);
         }
 
-        pte = mfn_to_xen_entry(mfn, WRITEALLOC);
+        pte = mfn_to_xen_entry(_mfn(mfn), WRITEALLOC);
         /* TODO: Set pte.pt.contig when appropriate. */
         write_pte(&first[first_table_offset(vaddr)], pte);
 
@@ -907,7 +908,7 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
     for ( i = 0; i < nr_second; i++ )
     {
         clear_page(mfn_to_virt(second_base + i));
-        pte = mfn_to_xen_entry(second_base + i, WRITEALLOC);
+        pte = mfn_to_xen_entry(_mfn(second_base + i), WRITEALLOC);
         pte.pt.table = 1;
         write_pte(&xen_first[first_table_offset(FRAMETABLE_VIRT_START)+i], pte);
     }
@@ -960,7 +961,7 @@ static int create_xen_table(lpae_t *entry)
     if ( p == NULL )
         return -ENOMEM;
     clear_page(p);
-    pte = mfn_to_xen_entry(virt_to_mfn(p), WRITEALLOC);
+    pte = mfn_to_xen_entry(_mfn(virt_to_mfn(p)), WRITEALLOC);
     pte.pt.table = 1;
     write_pte(entry, pte);
     return 0;
@@ -1011,7 +1012,7 @@ static int create_xen_entries(enum xenmap_operation op,
                 }
                 if ( op == RESERVE )
                     break;
-                pte = mfn_to_xen_entry(mfn, ai);
+                pte = mfn_to_xen_entry(_mfn(mfn), ai);
                 pte.pt.table = 1;
                 write_pte(&third[third_table_offset(addr)], pte);
                 break;
-- 
2.11.0


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

[PATCH 09/24] xen/arm: Define mfn_to_page/page_to_mfn in term of __mfn_to_page/__page_to_mfn

[Julien Grall]

This is matching the x86 side where the __* version is used if you need
to override the helpers in source files.

At the same time, move the non-underscore version at the end of the
defintion and add a comment to explain them.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/include/asm-arm/mm.h | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index f6915ad882..cc3220a6b7 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -203,10 +203,8 @@ static inline void __iomem *ioremap_wc(paddr_t start, size_t len)
 })
 
 /* Convert between machine frame numbers and page-info structures. */
-#define mfn_to_page(mfn)  (frame_table + (pfn_to_pdx(mfn) - frametable_base_pdx))
-#define page_to_mfn(pg)   pdx_to_pfn((unsigned long)((pg) - frame_table) + frametable_base_pdx)
-#define __page_to_mfn(pg)  page_to_mfn(pg)
-#define __mfn_to_page(mfn) mfn_to_page(mfn)
+#define __mfn_to_page(mfn)  (frame_table + (pfn_to_pdx(mfn) - frametable_base_pdx))
+#define __page_to_mfn(pg)   pdx_to_pfn((unsigned long)((pg) - frame_table) + frametable_base_pdx)
 
 /* Convert between machine addresses and page-info structures. */
 #define maddr_to_page(ma) __mfn_to_page((ma) >> PAGE_SHIFT)
@@ -264,6 +262,13 @@ static inline int gvirt_to_maddr(vaddr_t va, paddr_t *pa, unsigned int flags)
 #define virt_to_mfn(va)   (virt_to_maddr(va) >> PAGE_SHIFT)
 #define mfn_to_virt(mfn)  (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT))
 
+/*
+ * We define non-underscored wrappers for above conversion functions.
+ * These are overriden in various source files while underscored version
+ * remain intact.
+ */
+#define mfn_to_page(mfn)    __mfn_to_page(mfn)
+#define page_to_mfn(pg)     __page_to_mfn(pg)
 
 /* Convert between Xen-heap virtual addresses and page-info structures. */
 static inline struct page_info *virt_to_page(const void *v)
-- 
2.11.0


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

[PATCH 10/24] xen/arm: domain_build: Replace paddr_to_pfn(virt_to_maddr(.)) by virt_to_mfn(.)

[Julien Grall]

paddr_to_pfn(virt_to_maddr(.)) and virt_to_mfn(.) are equivalent.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/domain_build.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 3abacc0d80..a04c8862db 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1903,7 +1903,7 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
     rc = map_regions_p2mt(d,
                           _gfn(paddr_to_pfn(d->arch.efi_acpi_gpa)),
                           PFN_UP(d->arch.efi_acpi_len),
-                          _mfn(paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table))),
+                          _mfn(virt_to_mfn(d->arch.efi_acpi_table)),
                           p2m_mmio_direct_c);
     if ( rc != 0 )
     {
-- 
2.11.0


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

[PATCH 11/24] xen/arm: mm: Replace __va(pfn_to_paddr(...)) by mfn_to_virt

[Julien Grall]

__va(pfn_to_paddr(...)) and mfn_to_virt are equivalent.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/mm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 08116679ec..f5df669d92 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -999,7 +999,7 @@ static int create_xen_entries(enum xenmap_operation op,
 
         BUG_ON(!xen_second[second_linear_offset(addr)].pt.valid);
 
-        third = __va(pfn_to_paddr(xen_second[second_linear_offset(addr)].pt.base));
+        third = mfn_to_virt(xen_second[second_linear_offset(addr)].pt.base);
 
         switch ( op ) {
             case INSERT:
-- 
2.11.0


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

[PATCH 12/24] xen/arm: Replace DIV_ROUND_UP(..., PAGE_SIZE) by PFN_UP(...)

[Julien Grall]

DIV_ROUND_UP(..., PAGE_SIZE) and PFN_UP(...) are equivalent.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/domain_build.c | 4 ++--
 xen/arch/arm/gic-v2.c       | 2 +-
 xen/arch/arm/gic-v3.c       | 8 ++++----
 xen/arch/arm/kernel.c       | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index a04c8862db..a3243bdb5d 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1008,7 +1008,7 @@ static int map_range_to_domain(const struct dt_device_node *dev,
     {
         res = map_regions_p2mt(d,
                                _gfn(paddr_to_pfn(addr)),
-                               DIV_ROUND_UP(len, PAGE_SIZE),
+                               PFN_UP(len),
                                _mfn(paddr_to_pfn(addr)),
                                mr_data->p2mt);
 
@@ -1545,7 +1545,7 @@ static void acpi_map_other_tables(struct domain *d)
         size = acpi_gbl_root_table_list.tables[i].length;
         res = map_regions_p2mt(d,
                                _gfn(paddr_to_pfn(addr)),
-                               DIV_ROUND_UP(size, PAGE_SIZE),
+                               PFN_UP(size),
                                _mfn(paddr_to_pfn(addr)),
                                p2m_mmio_direct_c);
         if ( res )
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index f8124e5e54..0482b1fe32 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -598,7 +598,7 @@ static int gicv2_map_hwdown_extra_mappings(struct domain *d)
                v2m_data->spi_start, v2m_data->nr_spis);
 
         ret = map_mmio_regions(d, _gfn(paddr_to_pfn(v2m_data->addr)),
-                               DIV_ROUND_UP(v2m_data->size, PAGE_SIZE),
+                               PFN_UP(v2m_data->size),
                                _mfn(paddr_to_pfn(v2m_data->addr)));
         if ( ret )
         {
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index a559e5e260..bb845e955d 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1283,7 +1283,7 @@ static int gicv3_iomem_deny_access(const struct domain *d)
     unsigned long mfn, nr;
 
     mfn = dbase >> PAGE_SHIFT;
-    nr = DIV_ROUND_UP(SZ_64K, PAGE_SIZE);
+    nr = PFN_UP(SZ_64K);
     rc = iomem_deny_access(d, mfn, mfn + nr);
     if ( rc )
         return rc;
@@ -1291,7 +1291,7 @@ static int gicv3_iomem_deny_access(const struct domain *d)
     for ( i = 0; i < gicv3.rdist_count; i++ )
     {
         mfn = gicv3.rdist_regions[i].base >> PAGE_SHIFT;
-        nr = DIV_ROUND_UP(gicv3.rdist_regions[i].size, PAGE_SIZE);
+        nr = PFN_UP(gicv3.rdist_regions[i].size);
         rc = iomem_deny_access(d, mfn, mfn + nr);
         if ( rc )
             return rc;
@@ -1300,7 +1300,7 @@ static int gicv3_iomem_deny_access(const struct domain *d)
     if ( cbase != INVALID_PADDR )
     {
         mfn = cbase >> PAGE_SHIFT;
-        nr = DIV_ROUND_UP(csize, PAGE_SIZE);
+        nr = PFN_UP(csize);
         rc = iomem_deny_access(d, mfn, mfn + nr);
         if ( rc )
             return rc;
@@ -1309,7 +1309,7 @@ static int gicv3_iomem_deny_access(const struct domain *d)
     if ( vbase != INVALID_PADDR )
     {
         mfn = vbase >> PAGE_SHIFT;
-        nr = DIV_ROUND_UP(csize, PAGE_SIZE);
+        nr = PFN_UP(csize);
         return iomem_deny_access(d, mfn, mfn + nr);
     }
 
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index e2512c4612..0ed8b6005c 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -312,7 +312,7 @@ static __init int kernel_decompress(struct bootmodule *mod)
      * Need to free pages after output_size here because they won't be
      * freed by discard_initial_modules
      */
-    i = DIV_ROUND_UP(output_size, PAGE_SIZE);
+    i = PFN_UP(output_size);
     for ( ; i < (1 << kernel_order_out); i++ )
         free_domheap_page(pages + i);
 
-- 
2.11.0


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

[PATCH 13/24] xen/arm: traps: Replace p2m_lookup(..., ..., NULL) by gfn_to_mfn(..., ...)

[Julien Grall]

gfn_to_mfn is a wrapper of p2m_lookup which does not return the
p2m_type.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/traps.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 6cf9ee7244..ce19021f01 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2481,7 +2481,7 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
     uint32_t *first = NULL, *second = NULL;
     mfn_t mfn;
 
-    mfn = p2m_lookup(d, _gfn(paddr_to_pfn(ttbr0)), NULL);
+    mfn = gfn_to_mfn(d, _gfn(paddr_to_pfn(ttbr0)));
 
     printk("dom%d VA 0x%08"PRIvaddr"\n", d->domain_id, addr);
     printk("    TTBCR: 0x%08"PRIregister"\n", ttbcr);
@@ -2513,7 +2513,7 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
           (first[offset] & 0x2) )
         goto done;
 
-    mfn = p2m_lookup(d, _gfn(paddr_to_pfn(first[offset])), NULL);
+    mfn = gfn_to_mfn(d, _gfn(paddr_to_pfn(first[offset])));
 
     if ( mfn_eq(mfn, INVALID_MFN) )
     {
@@ -2619,7 +2619,7 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
          * with the Stage-2 page table. Walk the Stage-2 PT to check
          * if the entry exists. If it's the case, return to the guest
          */
-        mfn = p2m_lookup(current->domain, _gfn(paddr_to_pfn(gpa)), NULL);
+        mfn = gfn_to_mfn(current->domain, _gfn(paddr_to_pfn(gpa)));
         if ( !mfn_eq(mfn, INVALID_MFN) )
             return;
     }
@@ -2759,7 +2759,7 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
          * with the Stage-2 page table. Walk the Stage-2 PT to check
          * if the entry exists. If it's the case, return to the guest
          */
-        mfn = p2m_lookup(current->domain, _gfn(paddr_to_pfn(info.gpa)), NULL);
+        mfn = gfn_to_mfn(current->domain, _gfn(paddr_to_pfn(info.gpa)));
         if ( !mfn_eq(mfn, INVALID_MFN) )
             return;
 
-- 
2.11.0


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

[PATCH 14/24] xen/arm: Introduce wrappers for MFN <-> MADDR and GFN <-> GADDR

[Julien Grall]

The new wrappers will add more safety when converting an address to a
frame number (either machine or guest). A follow-up patch will use them
to simplify the code.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/include/asm-arm/mm.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index cc3220a6b7..a42da20f0a 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -214,6 +214,10 @@ static inline void __iomem *ioremap_wc(paddr_t start, size_t len)
 #define pfn_to_paddr(pfn) ((paddr_t)(pfn) << PAGE_SHIFT)
 #define paddr_to_pfn(pa)  ((unsigned long)((pa) >> PAGE_SHIFT))
 #define paddr_to_pdx(pa)    pfn_to_pdx(paddr_to_pfn(pa))
+#define gfn_to_gaddr(gfn)   pfn_to_paddr(gfn_x(gfn))
+#define gaddr_to_gfn(ga)    _gfn(paddr_to_pfn(ga))
+#define mfn_to_maddr(mfn)   pfn_to_paddr(mfn_x(mfn))
+#define maddr_to_mfn(ma)    _mfn(paddr_to_pfn(ma))
 #define vmap_to_mfn(va)     paddr_to_pfn(virt_to_maddr((vaddr_t)va))
 #define vmap_to_page(va)    mfn_to_page(vmap_to_mfn(va))
 
-- 
2.11.0


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

[PATCH 15/24] xen/arm: Use the newly introduced MFN <-> MADDR and GFN <-> MADDR helpers

[Julien Grall]

Replace the following constructions:
    - _gfn(paddr_to_pfn(...))   => gaddr_to_gfn(...)
    - _mfn(paddr_to_pfn(...))   => maddr_to_mfn(...)
    - pfn_to_paddr(mfn_x(...))  => mfn_to_maddr(...)
    - pfn_to_paddr(gfn_x(...))  => gfn_to_gaddr(...)
    - _mfn(... >> PAGE_SHIFT)   => maddr_to_mfn(...)

Signed-off-by: Julien Grall <julien.grall@arm.com>
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
Cc: Tamas K Lengyel <tamas@tklengyel.com>
---
 xen/arch/arm/domain_build.c      | 12 ++++++------
 xen/arch/arm/gic-v2.c            |  4 ++--
 xen/arch/arm/kernel.c            |  2 +-
 xen/arch/arm/mem_access.c        | 10 +++++-----
 xen/arch/arm/mm.c                | 14 +++++++-------
 xen/arch/arm/p2m.c               | 10 +++++-----
 xen/arch/arm/platforms/exynos5.c |  8 ++++----
 xen/arch/arm/platforms/omap5.c   | 16 ++++++++--------
 xen/arch/arm/traps.c             | 14 +++++++-------
 xen/arch/arm/vgic-v2.c           |  4 ++--
 10 files changed, 47 insertions(+), 47 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index a3243bdb5d..c6776d76fc 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1007,9 +1007,9 @@ static int map_range_to_domain(const struct dt_device_node *dev,
     if ( need_mapping )
     {
         res = map_regions_p2mt(d,
-                               _gfn(paddr_to_pfn(addr)),
+                               gaddr_to_gfn(addr),
                                PFN_UP(len),
-                               _mfn(paddr_to_pfn(addr)),
+                               maddr_to_mfn(addr),
                                mr_data->p2mt);
 
         if ( res < 0 )
@@ -1544,9 +1544,9 @@ static void acpi_map_other_tables(struct domain *d)
         addr = acpi_gbl_root_table_list.tables[i].address;
         size = acpi_gbl_root_table_list.tables[i].length;
         res = map_regions_p2mt(d,
-                               _gfn(paddr_to_pfn(addr)),
+                               gaddr_to_gfn(addr),
                                PFN_UP(size),
-                               _mfn(paddr_to_pfn(addr)),
+                               maddr_to_mfn(addr),
                                p2m_mmio_direct_c);
         if ( res )
         {
@@ -1901,7 +1901,7 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
 
     /* Map the EFI and ACPI tables to Dom0 */
     rc = map_regions_p2mt(d,
-                          _gfn(paddr_to_pfn(d->arch.efi_acpi_gpa)),
+                          gaddr_to_gfn(d->arch.efi_acpi_gpa),
                           PFN_UP(d->arch.efi_acpi_len),
                           _mfn(virt_to_mfn(d->arch.efi_acpi_table)),
                           p2m_mmio_direct_c);
@@ -2014,7 +2014,7 @@ static void initrd_load(struct kernel_info *kinfo)
             return;
         }
 
-        dst = map_domain_page(_mfn(paddr_to_pfn(ma)));
+        dst = map_domain_page(maddr_to_mfn(ma));
 
         copy_from_paddr(dst + s, paddr + offs, l);
 
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 0482b1fe32..5bf7d35a7e 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -597,9 +597,9 @@ static int gicv2_map_hwdown_extra_mappings(struct domain *d)
                d->domain_id, v2m_data->addr, v2m_data->size,
                v2m_data->spi_start, v2m_data->nr_spis);
 
-        ret = map_mmio_regions(d, _gfn(paddr_to_pfn(v2m_data->addr)),
+        ret = map_mmio_regions(d, gaddr_to_gfn(v2m_data->addr),
                                PFN_UP(v2m_data->size),
-                               _mfn(paddr_to_pfn(v2m_data->addr)));
+                               maddr_to_mfn(v2m_data->addr));
         if ( ret )
         {
             printk(XENLOG_ERR "GICv2: Map v2m frame to d%d failed.\n",
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 0ed8b6005c..1b32d55284 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -183,7 +183,7 @@ static void kernel_zimage_load(struct kernel_info *info)
             return;
         }
 
-        dst = map_domain_page(_mfn(paddr_to_pfn(ma)));
+        dst = map_domain_page(maddr_to_mfn(ma));
 
         copy_from_paddr(dst + s, paddr + offs, l);
 
diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
index 04b1506b00..bcf49f5c15 100644
--- a/xen/arch/arm/mem_access.c
+++ b/xen/arch/arm/mem_access.c
@@ -113,7 +113,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag,
     if ( rc < 0 )
         goto err;
 
-    gfn = _gfn(paddr_to_pfn(ipa));
+    gfn = gaddr_to_gfn(ipa);
 
     /*
      * We do this first as this is faster in the default case when no
@@ -203,7 +203,7 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec)
     if ( !p2m->mem_access_enabled )
         return true;
 
-    rc = p2m_get_mem_access(v->domain, _gfn(paddr_to_pfn(gpa)), &xma);
+    rc = p2m_get_mem_access(v->domain, gaddr_to_gfn(gpa), &xma);
     if ( rc )
         return true;
 
@@ -245,13 +245,13 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec)
     /* First, handle rx2rw and n2rwx conversion automatically. */
     if ( npfec.write_access && xma == XENMEM_access_rx2rw )
     {
-        rc = p2m_set_mem_access(v->domain, _gfn(paddr_to_pfn(gpa)), 1,
+        rc = p2m_set_mem_access(v->domain, gaddr_to_gfn(gpa), 1,
                                 0, ~0, XENMEM_access_rw, 0);
         return false;
     }
     else if ( xma == XENMEM_access_n2rwx )
     {
-        rc = p2m_set_mem_access(v->domain, _gfn(paddr_to_pfn(gpa)), 1,
+        rc = p2m_set_mem_access(v->domain, gaddr_to_gfn(gpa), 1,
                                 0, ~0, XENMEM_access_rwx, 0);
     }
 
@@ -273,7 +273,7 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec)
             {
                 /* A listener is not required, so clear the access
                  * restrictions. */
-                rc = p2m_set_mem_access(v->domain, _gfn(paddr_to_pfn(gpa)), 1,
+                rc = p2m_set_mem_access(v->domain, gaddr_to_gfn(gpa), 1,
                                         0, ~0, XENMEM_access_rwx, 0);
             }
         }
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index f5df669d92..0014c24ecc 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -315,7 +315,7 @@ static inline lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned attr)
         break;
     }
 
-    ASSERT(!(pfn_to_paddr(mfn_x(mfn)) & ~PADDR_MASK));
+    ASSERT(!(mfn_to_maddr(mfn) & ~PADDR_MASK));
 
     e.pt.base = mfn_x(mfn);
 
@@ -536,8 +536,8 @@ void __init arch_init_memory(void)
 static inline lpae_t pte_of_xenaddr(vaddr_t va)
 {
     paddr_t ma = va + phys_offset;
-    unsigned long mfn = ma >> PAGE_SHIFT;
-    return mfn_to_xen_entry(_mfn(mfn), WRITEALLOC);
+
+    return mfn_to_xen_entry(maddr_to_mfn(ma), WRITEALLOC);
 }
 
 /* Map the FDT in the early boot page table */
@@ -646,7 +646,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
     /* Initialise xen second level entries ... */
     /* ... Xen's text etc */
 
-    pte = mfn_to_xen_entry(_mfn(xen_paddr>>PAGE_SHIFT), WRITEALLOC);
+    pte = mfn_to_xen_entry(maddr_to_mfn(xen_paddr), WRITEALLOC);
     pte.pt.xn = 0;/* Contains our text mapping! */
     xen_second[second_table_offset(XEN_VIRT_START)] = pte;
 
@@ -663,7 +663,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
 
     /* ... Boot Misc area for xen relocation */
     dest_va = BOOT_RELOC_VIRT_START;
-    pte = mfn_to_xen_entry(_mfn(xen_paddr >> PAGE_SHIFT), WRITEALLOC);
+    pte = mfn_to_xen_entry(maddr_to_mfn(xen_paddr), WRITEALLOC);
     /* Map the destination in xen_second. */
     xen_second[second_table_offset(dest_va)] = pte;
     /* Map the destination in boot_second. */
@@ -690,11 +690,11 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
     /* Break up the Xen mapping into 4k pages and protect them separately. */
     for ( i = 0; i < LPAE_ENTRIES; i++ )
     {
-        unsigned long mfn = paddr_to_pfn(xen_paddr) + i;
+        mfn_t mfn = mfn_add(maddr_to_mfn(xen_paddr), i);
         unsigned long va = XEN_VIRT_START + (i << PAGE_SHIFT);
         if ( !is_kernel(va) )
             break;
-        pte = mfn_to_xen_entry(_mfn(mfn), WRITEALLOC);
+        pte = mfn_to_xen_entry(mfn, WRITEALLOC);
         pte.pt.table = 1; /* 4k mappings always have this bit set */
         if ( is_kernel_text(va) || is_kernel_inittext(va) )
         {
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index b7bbea1d81..266d1c3bd6 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -311,7 +311,7 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
                     p2m_type_t *t, p2m_access_t *a,
                     unsigned int *page_order)
 {
-    paddr_t addr = pfn_to_paddr(gfn_x(gfn));
+    paddr_t addr = gfn_to_gaddr(gfn);
     unsigned int level = 0;
     lpae_t entry, *table;
     int rc;
@@ -542,7 +542,7 @@ static lpae_t mfn_to_p2m_entry(mfn_t mfn, p2m_type_t t, p2m_access_t a)
 
     p2m_set_permission(&e, t, a);
 
-    ASSERT(!(pfn_to_paddr(mfn_x(mfn)) & ~PADDR_MASK));
+    ASSERT(!(mfn_to_maddr(mfn) & ~PADDR_MASK));
 
     e.p2m.base = mfn_x(mfn);
 
@@ -803,7 +803,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
                            p2m_type_t t,
                            p2m_access_t a)
 {
-    paddr_t addr = pfn_to_paddr(gfn_x(sgfn));
+    paddr_t addr = gfn_to_gaddr(sgfn);
     unsigned int level = 0;
     unsigned int target = 3 - (page_order / LPAE_SHIFT);
     lpae_t *entry, *table, orig_pte;
@@ -1440,10 +1440,10 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
     if ( rc )
         goto err;
 
-    if ( !mfn_valid(_mfn(maddr >> PAGE_SHIFT)) )
+    if ( !mfn_valid(maddr_to_mfn(maddr)) )
         goto err;
 
-    page = mfn_to_page(maddr >> PAGE_SHIFT);
+    page = mfn_to_page(mfn_x(maddr_to_mfn(maddr)));
     ASSERT(page);
 
     if ( unlikely(!get_page(page, d)) )
diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
index 492cd3e11f..2ae5fa66e0 100644
--- a/xen/arch/arm/platforms/exynos5.c
+++ b/xen/arch/arm/platforms/exynos5.c
@@ -82,12 +82,12 @@ static int exynos5_init_time(void)
 static int exynos5250_specific_mapping(struct domain *d)
 {
     /* Map the chip ID */
-    map_mmio_regions(d, _gfn(paddr_to_pfn(EXYNOS5_PA_CHIPID)), 1,
-                     _mfn(paddr_to_pfn(EXYNOS5_PA_CHIPID)));
+    map_mmio_regions(d, gaddr_to_gfn(EXYNOS5_PA_CHIPID), 1,
+                     maddr_to_mfn(EXYNOS5_PA_CHIPID));
 
     /* Map the PWM region */
-    map_mmio_regions(d, _gfn(paddr_to_pfn(EXYNOS5_PA_TIMER)), 2,
-                     _mfn(paddr_to_pfn(EXYNOS5_PA_TIMER)));
+    map_mmio_regions(d, gaddr_to_gfn(EXYNOS5_PA_TIMER), 2,
+                     maddr_to_mfn(EXYNOS5_PA_TIMER));
 
     return 0;
 }
diff --git a/xen/arch/arm/platforms/omap5.c b/xen/arch/arm/platforms/omap5.c
index eadc4f8382..1e1f9fa970 100644
--- a/xen/arch/arm/platforms/omap5.c
+++ b/xen/arch/arm/platforms/omap5.c
@@ -101,20 +101,20 @@ static int omap5_init_time(void)
 static int omap5_specific_mapping(struct domain *d)
 {
     /* Map the PRM module */
-    map_mmio_regions(d, _gfn(paddr_to_pfn(OMAP5_PRM_BASE)), 2,
-                     _mfn(paddr_to_pfn(OMAP5_PRM_BASE)));
+    map_mmio_regions(d, gaddr_to_gfn(OMAP5_PRM_BASE), 2,
+                     maddr_to_mfn(OMAP5_PRM_BASE));
 
     /* Map the PRM_MPU */
-    map_mmio_regions(d, _gfn(paddr_to_pfn(OMAP5_PRCM_MPU_BASE)), 1,
-                     _mfn(paddr_to_pfn(OMAP5_PRCM_MPU_BASE)));
+    map_mmio_regions(d, gaddr_to_gfn(OMAP5_PRCM_MPU_BASE), 1,
+                     maddr_to_mfn(OMAP5_PRCM_MPU_BASE));
 
     /* Map the Wakeup Gen */
-    map_mmio_regions(d, _gfn(paddr_to_pfn(OMAP5_WKUPGEN_BASE)), 1,
-                     _mfn(paddr_to_pfn(OMAP5_WKUPGEN_BASE)));
+    map_mmio_regions(d, gaddr_to_gfn(OMAP5_WKUPGEN_BASE), 1,
+                     maddr_to_mfn(OMAP5_WKUPGEN_BASE));
 
     /* Map the on-chip SRAM */
-    map_mmio_regions(d, _gfn(paddr_to_pfn(OMAP5_SRAM_PA)), 32,
-                     _mfn(paddr_to_pfn(OMAP5_SRAM_PA)));
+    map_mmio_regions(d, gaddr_to_gfn(OMAP5_SRAM_PA), 32,
+                     maddr_to_mfn(OMAP5_SRAM_PA));
 
     return 0;
 }
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index ce19021f01..c07999b518 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2481,12 +2481,12 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
     uint32_t *first = NULL, *second = NULL;
     mfn_t mfn;
 
-    mfn = gfn_to_mfn(d, _gfn(paddr_to_pfn(ttbr0)));
+    mfn = gfn_to_mfn(d, gaddr_to_gfn(ttbr0));
 
     printk("dom%d VA 0x%08"PRIvaddr"\n", d->domain_id, addr);
     printk("    TTBCR: 0x%08"PRIregister"\n", ttbcr);
     printk("    TTBR0: 0x%016"PRIx64" = 0x%"PRIpaddr"\n",
-           ttbr0, pfn_to_paddr(mfn_x(mfn)));
+           ttbr0, mfn_to_maddr(mfn));
 
     if ( ttbcr & TTBCR_EAE )
     {
@@ -2508,12 +2508,12 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
 
     offset = addr >> (12+8);
     printk("1ST[0x%"PRIx32"] (0x%"PRIpaddr") = 0x%08"PRIx32"\n",
-           offset, pfn_to_paddr(mfn_x(mfn)), first[offset]);
+           offset, mfn_to_maddr(mfn), first[offset]);
     if ( !(first[offset] & 0x1) ||
           (first[offset] & 0x2) )
         goto done;
 
-    mfn = gfn_to_mfn(d, _gfn(paddr_to_pfn(first[offset])));
+    mfn = gfn_to_mfn(d, gaddr_to_gfn(first[offset]));
 
     if ( mfn_eq(mfn, INVALID_MFN) )
     {
@@ -2523,7 +2523,7 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
     second = map_domain_page(mfn);
     offset = (addr >> 12) & 0x3FF;
     printk("2ND[0x%"PRIx32"] (0x%"PRIpaddr") = 0x%08"PRIx32"\n",
-           offset, pfn_to_paddr(mfn_x(mfn)), second[offset]);
+           offset, mfn_to_maddr(mfn), second[offset]);
 
 done:
     if (second) unmap_domain_page(second);
@@ -2759,11 +2759,11 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
          * with the Stage-2 page table. Walk the Stage-2 PT to check
          * if the entry exists. If it's the case, return to the guest
          */
-        mfn = gfn_to_mfn(current->domain, _gfn(paddr_to_pfn(info.gpa)));
+        mfn = gfn_to_mfn(current->domain, gaddr_to_gfn(info.gpa));
         if ( !mfn_eq(mfn, INVALID_MFN) )
             return;
 
-        if ( try_map_mmio(_gfn(paddr_to_pfn(info.gpa))) )
+        if ( try_map_mmio(gaddr_to_gfn(info.gpa)) )
             return;
 
         break;
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index dc9f95b948..e5cfa33d8a 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -686,8 +686,8 @@ static int vgic_v2_domain_init(struct domain *d)
      * Map the gic virtual cpu interface in the gic cpu interface
      * region of the guest.
      */
-    ret = map_mmio_regions(d, _gfn(paddr_to_pfn(cbase)), csize / PAGE_SIZE,
-                           _mfn(paddr_to_pfn(vbase)));
+    ret = map_mmio_regions(d, gaddr_to_gfn(cbase), csize / PAGE_SIZE,
+                           maddr_to_mfn(vbase));
     if ( ret )
         return ret;
 
-- 
2.11.0


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

[PATCH 16/24] xen/arm: mm: Use typesafe mfn for xenheap_mfn_*

[Julien Grall]

Add more safety when using xenheap_mfn_*.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/mm.c        | 16 ++++++++--------
 xen/arch/arm/setup.c     | 18 +++++++++---------
 xen/include/asm-arm/mm.h | 11 ++++++-----
 3 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 0014c24ecc..fb01f01879 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -138,8 +138,8 @@ uint64_t init_ttbr;
 static paddr_t phys_offset;
 
 /* Limits of the Xen heap */
-unsigned long xenheap_mfn_start __read_mostly = ~0UL;
-unsigned long xenheap_mfn_end __read_mostly;
+mfn_t xenheap_mfn_start __read_mostly = INVALID_MFN;
+mfn_t xenheap_mfn_end __read_mostly;
 vaddr_t xenheap_virt_end __read_mostly;
 #ifdef CONFIG_ARM_64
 vaddr_t xenheap_virt_start __read_mostly;
@@ -801,8 +801,8 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
 
     /* Record where the xenheap is, for translation routines. */
     xenheap_virt_end = XENHEAP_VIRT_START + nr_mfns * PAGE_SIZE;
-    xenheap_mfn_start = base_mfn;
-    xenheap_mfn_end = base_mfn + nr_mfns;
+    xenheap_mfn_start = _mfn(base_mfn);
+    xenheap_mfn_end = _mfn(base_mfn + nr_mfns);
 }
 #else /* CONFIG_ARM_64 */
 void __init setup_xenheap_mappings(unsigned long base_mfn,
@@ -816,16 +816,16 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
     mfn = base_mfn & ~((FIRST_SIZE>>PAGE_SHIFT)-1);
 
     /* First call sets the xenheap physical and virtual offset. */
-    if ( xenheap_mfn_start == ~0UL )
+    if ( mfn_eq(xenheap_mfn_start, INVALID_MFN) )
     {
-        xenheap_mfn_start = base_mfn;
+        xenheap_mfn_start = _mfn(base_mfn);
         xenheap_virt_start = DIRECTMAP_VIRT_START +
             (base_mfn - mfn) * PAGE_SIZE;
     }
 
-    if ( base_mfn < xenheap_mfn_start )
+    if ( base_mfn < mfn_x(xenheap_mfn_start) )
         panic("cannot add xenheap mapping at %lx below heap start %lx",
-              base_mfn, xenheap_mfn_start);
+              base_mfn, mfn_x(xenheap_mfn_start));
 
     end_mfn = base_mfn + nr_mfns;
 
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index ab4d8e4218..3b34855668 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -555,8 +555,8 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
      * and enough mapped pages for copying the DTB.
      */
     dtb_pages = (dtb_size + PAGE_SIZE-1) >> PAGE_SHIFT;
-    boot_mfn_start = xenheap_mfn_end - dtb_pages - 1;
-    boot_mfn_end = xenheap_mfn_end;
+    boot_mfn_start = mfn_x(xenheap_mfn_end) - dtb_pages - 1;
+    boot_mfn_end = mfn_x(xenheap_mfn_end);
 
     init_boot_pages(pfn_to_paddr(boot_mfn_start), pfn_to_paddr(boot_mfn_end));
 
@@ -591,11 +591,11 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
                 e = bank_end;
 
             /* Avoid the xenheap */
-            if ( s < pfn_to_paddr(xenheap_mfn_start+xenheap_pages)
-                 && pfn_to_paddr(xenheap_mfn_start) < e )
+            if ( s < mfn_to_maddr(mfn_add(xenheap_mfn_start, xenheap_pages))
+                 && mfn_to_maddr(xenheap_mfn_start) < e )
             {
-                e = pfn_to_paddr(xenheap_mfn_start);
-                n = pfn_to_paddr(xenheap_mfn_start+xenheap_pages);
+                e = mfn_to_maddr(xenheap_mfn_start);
+                n = mfn_to_maddr(mfn_add(xenheap_mfn_start, xenheap_pages));
             }
 
             dt_unreserved_regions(s, e, init_boot_pages, 0);
@@ -610,7 +610,7 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
 
     /* Add xenheap memory that was not already added to the boot
        allocator. */
-    init_xenheap_pages(pfn_to_paddr(xenheap_mfn_start),
+    init_xenheap_pages(mfn_to_maddr(xenheap_mfn_start),
                        pfn_to_paddr(boot_mfn_start));
 }
 #else /* CONFIG_ARM_64 */
@@ -662,8 +662,8 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
     total_pages += ram_size >> PAGE_SHIFT;
 
     xenheap_virt_end = XENHEAP_VIRT_START + ram_end - ram_start;
-    xenheap_mfn_start = ram_start >> PAGE_SHIFT;
-    xenheap_mfn_end = ram_end >> PAGE_SHIFT;
+    xenheap_mfn_start = maddr_to_mfn(ram_start);
+    xenheap_mfn_end = maddr_to_mfn(ram_end);
 
     /*
      * Need enough mapped pages for copying the DTB.
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index a42da20f0a..3dab6dc9a1 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -115,7 +115,7 @@ struct page_info
 #define PGC_count_width   PG_shift(9)
 #define PGC_count_mask    ((1UL<<PGC_count_width)-1)
 
-extern unsigned long xenheap_mfn_start, xenheap_mfn_end;
+extern mfn_t xenheap_mfn_start, xenheap_mfn_end;
 extern vaddr_t xenheap_virt_end;
 #ifdef CONFIG_ARM_64
 extern vaddr_t xenheap_virt_start;
@@ -125,7 +125,8 @@ extern vaddr_t xenheap_virt_start;
 #define is_xen_heap_page(page) is_xen_heap_mfn(page_to_mfn(page))
 #define is_xen_heap_mfn(mfn) ({                                 \
     unsigned long _mfn = (mfn);                                 \
-    (_mfn >= xenheap_mfn_start && _mfn < xenheap_mfn_end);      \
+    (_mfn >= mfn_x(xenheap_mfn_start) &&                        \
+     _mfn < mfn_x(xenheap_mfn_end));                            \
 })
 #else
 #define is_xen_heap_page(page) ((page)->count_info & PGC_xen_heap)
@@ -235,7 +236,7 @@ static inline paddr_t __virt_to_maddr(vaddr_t va)
 static inline void *maddr_to_virt(paddr_t ma)
 {
     ASSERT(is_xen_heap_mfn(ma >> PAGE_SHIFT));
-    ma -= pfn_to_paddr(xenheap_mfn_start);
+    ma -= mfn_to_maddr(xenheap_mfn_start);
     return (void *)(unsigned long) ma + XENHEAP_VIRT_START;
 }
 #else
@@ -243,7 +244,7 @@ static inline void *maddr_to_virt(paddr_t ma)
 {
     ASSERT(pfn_to_pdx(ma >> PAGE_SHIFT) < (DIRECTMAP_SIZE >> PAGE_SHIFT));
     return (void *)(XENHEAP_VIRT_START -
-                    pfn_to_paddr(xenheap_mfn_start) +
+                    mfn_to_maddr(xenheap_mfn_start) +
                     ((ma & ma_va_bottom_mask) |
                      ((ma & ma_top_mask) >> pfn_pdx_hole_shift)));
 }
@@ -284,7 +285,7 @@ static inline struct page_info *virt_to_page(const void *v)
     ASSERT(va < xenheap_virt_end);
 
     pdx = (va - XENHEAP_VIRT_START) >> PAGE_SHIFT;
-    pdx += pfn_to_pdx(xenheap_mfn_start);
+    pdx += pfn_to_pdx(mfn_x(xenheap_mfn_start));
     return frame_table + pdx - frametable_base_pdx;
 }
 
-- 
2.11.0


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

[PATCH 17/24] xen/arm: mm: Use typesafe MFN in set_fixmap

[Julien Grall]

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/acpi/lib.c           | 4 ++--
 xen/arch/arm/kernel.c             | 4 +---
 xen/arch/arm/mm.c                 | 4 ++--
 xen/arch/arm/platforms/vexpress.c | 2 +-
 xen/drivers/video/arm_hdlcd.c     | 2 +-
 xen/include/asm-arm/mm.h          | 2 +-
 6 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/acpi/lib.c b/xen/arch/arm/acpi/lib.c
index 9bd769cff6..70131b0736 100644
--- a/xen/arch/arm/acpi/lib.c
+++ b/xen/arch/arm/acpi/lib.c
@@ -31,7 +31,7 @@ char *__acpi_map_table(paddr_t phys, unsigned long size)
 
     offset = phys & (PAGE_SIZE - 1);
     mapped_size = PAGE_SIZE - offset;
-    set_fixmap(FIXMAP_ACPI_BEGIN, phys >> PAGE_SHIFT, PAGE_HYPERVISOR);
+    set_fixmap(FIXMAP_ACPI_BEGIN, maddr_to_mfn(phys), PAGE_HYPERVISOR);
     base = FIXMAP_ADDR(FIXMAP_ACPI_BEGIN);
 
     /* Most cases can be covered by the below. */
@@ -41,7 +41,7 @@ char *__acpi_map_table(paddr_t phys, unsigned long size)
         if ( ++idx > FIXMAP_ACPI_END )
             return NULL;    /* cannot handle this */
         phys += PAGE_SIZE;
-        set_fixmap(idx, phys >> PAGE_SHIFT, PAGE_HYPERVISOR);
+        set_fixmap(idx, maddr_to_mfn(phys), PAGE_HYPERVISOR);
         mapped_size += PAGE_SIZE;
     }
 
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 1b32d55284..7403ec0c0e 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -49,14 +49,12 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len)
     void *src = (void *)FIXMAP_ADDR(FIXMAP_MISC);
 
     while (len) {
-        paddr_t p;
         unsigned long l, s;
 
-        p = paddr >> PAGE_SHIFT;
         s = paddr & (PAGE_SIZE-1);
         l = min(PAGE_SIZE - s, len);
 
-        set_fixmap(FIXMAP_MISC, p, BUFFERABLE);
+        set_fixmap(FIXMAP_MISC, maddr_to_mfn(paddr), BUFFERABLE);
         memcpy(dst, src + s, l);
         clean_dcache_va_range(dst, l);
 
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index fb01f01879..8573192e3a 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -323,9 +323,9 @@ static inline lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned attr)
 }
 
 /* Map a 4k page in a fixmap entry */
-void set_fixmap(unsigned map, unsigned long mfn, unsigned attributes)
+void set_fixmap(unsigned map, mfn_t mfn, unsigned attributes)
 {
-    lpae_t pte = mfn_to_xen_entry(_mfn(mfn), attributes);
+    lpae_t pte = mfn_to_xen_entry(mfn, attributes);
     pte.pt.table = 1; /* 4k mappings always have this bit set */
     pte.pt.xn = 1;
     write_pte(xen_fixmap + third_table_offset(FIXMAP_ADDR(map)), pte);
diff --git a/xen/arch/arm/platforms/vexpress.c b/xen/arch/arm/platforms/vexpress.c
index 8e6a4eaa32..a26ac324ba 100644
--- a/xen/arch/arm/platforms/vexpress.c
+++ b/xen/arch/arm/platforms/vexpress.c
@@ -65,7 +65,7 @@ int vexpress_syscfg(int write, int function, int device, uint32_t *data)
     uint32_t *syscfg = (uint32_t *) FIXMAP_ADDR(FIXMAP_MISC);
     int ret = -1;
 
-    set_fixmap(FIXMAP_MISC, V2M_SYS_MMIO_BASE >> PAGE_SHIFT, DEV_SHARED);
+    set_fixmap(FIXMAP_MISC, maddr_to_mfn(V2M_SYS_MMIO_BASE), DEV_SHARED);
 
     if ( syscfg[V2M_SYS_CFGCTRL/4] & V2M_SYS_CFG_START )
         goto out;
diff --git a/xen/drivers/video/arm_hdlcd.c b/xen/drivers/video/arm_hdlcd.c
index 8241cae117..3915f731f5 100644
--- a/xen/drivers/video/arm_hdlcd.c
+++ b/xen/drivers/video/arm_hdlcd.c
@@ -227,7 +227,7 @@ void __init video_init(void)
     /* uses FIXMAP_MISC */
     set_pixclock(videomode->pixclock);
 
-    set_fixmap(FIXMAP_MISC, hdlcd_start >> PAGE_SHIFT, DEV_SHARED);
+    set_fixmap(FIXMAP_MISC, maddr_to_mfn(hdlcd_start), DEV_SHARED);
     HDLCD[HDLCD_COMMAND] = 0;
 
     HDLCD[HDLCD_LINELENGTH] = videomode->xres * bytes_per_pixel;
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index 3dab6dc9a1..b2e7ea7761 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -176,7 +176,7 @@ extern void setup_xenheap_mappings(unsigned long base_mfn, unsigned long nr_mfns
 /* Map a frame table to cover physical addresses ps through pe */
 extern void setup_frametable_mappings(paddr_t ps, paddr_t pe);
 /* Map a 4k page in a fixmap entry */
-extern void set_fixmap(unsigned map, unsigned long mfn, unsigned attributes);
+extern void set_fixmap(unsigned map, mfn_t mfn, unsigned attributes);
 /* Remove a mapping from a fixmap entry */
 extern void clear_fixmap(unsigned map);
 /* map a physical range in virtual memory */
-- 
2.11.0


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

[PATCH 18/24] xen/arm: mm: Use typesafe MFN in dump_pt_walk

[Julien Grall]

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/mm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 8573192e3a..452c1e26c3 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -181,7 +181,7 @@ void dump_pt_walk(paddr_t ttbr, paddr_t addr,
                   unsigned int nr_root_tables)
 {
     static const char *level_strs[4] = { "0TH", "1ST", "2ND", "3RD" };
-    const unsigned long root_pfn = paddr_to_pfn(ttbr);
+    const mfn_t root_mfn = maddr_to_mfn(ttbr);
     const unsigned int offsets[4] = {
         zeroeth_table_offset(addr),
         first_table_offset(addr),
@@ -215,7 +215,7 @@ void dump_pt_walk(paddr_t ttbr, paddr_t addr,
     else
         root_table = 0;
 
-    mapping = map_domain_page(_mfn(root_pfn + root_table));
+    mapping = map_domain_page(mfn_add(root_mfn, root_table));
 
     for ( level = root_level; ; level++ )
     {
-- 
2.11.0


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

[PATCH 19/24] xen/arm: p2m: Redefine mfn_to_page and page_to_mfn to use typesafe

[Julien Grall]

The file xen/arch/arm/p2m.c is using typesafe MFN in most of the place.
This requires caller to mfn_to_page and page_to_mfn to use _mfn/mfn_x.

To avoid extra _mfn/mfn_x, re-define mfn_to_page and page_to_mfn within
xen/arch/arm/p2m.c to handle typesafe MFN.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/p2m.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 266d1c3bd6..6c1ac70044 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -38,6 +38,12 @@ static unsigned int __read_mostly max_vmid = MAX_VMID_8_BIT;
 
 #define P2M_ROOT_PAGES    (1<<P2M_ROOT_ORDER)
 
+/* Override macros from asm/mm.h to make them work with mfn_t */
+#undef mfn_to_page
+#define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn))
+#undef page_to_mfn
+#define page_to_mfn(pg) _mfn(__page_to_mfn(pg))
+
 unsigned int __read_mostly p2m_ipa_bits;
 
 /* Helpers to lookup the properties of each level */
@@ -115,7 +121,7 @@ void dump_p2m_lookup(struct domain *d, paddr_t addr)
     printk("dom%d IPA 0x%"PRIpaddr"\n", d->domain_id, addr);
 
     printk("P2M @ %p mfn:0x%lx\n",
-           p2m->root, page_to_mfn(p2m->root));
+           p2m->root, mfn_x(page_to_mfn(p2m->root)));
 
     dump_pt_walk(page_to_maddr(p2m->root), addr,
                  P2M_ROOT_LEVEL, P2M_ROOT_PAGES);
@@ -591,7 +597,7 @@ static int p2m_create_table(struct p2m_domain *p2m, lpae_t *entry)
      * The access value does not matter because the hardware will ignore
      * the permission fields for table entry.
      */
-    pte = mfn_to_p2m_entry(_mfn(page_to_mfn(page)), p2m_invalid,
+    pte = mfn_to_p2m_entry(page_to_mfn(page), p2m_invalid,
                            p2m->default_access);
 
     p2m_write_pte(entry, pte, p2m->clean_pte);
@@ -650,9 +656,9 @@ static void p2m_put_l3_page(const lpae_t pte)
      */
     if ( p2m_is_foreign(pte.p2m.type) )
     {
-        unsigned long mfn = pte.p2m.base;
+        mfn_t mfn = _mfn(pte.p2m.base);
 
-        ASSERT(mfn_valid(_mfn(mfn)));
+        ASSERT(mfn_valid(mfn));
         put_page(mfn_to_page(mfn));
     }
 }
@@ -702,7 +708,7 @@ static void p2m_free_entry(struct p2m_domain *p2m,
     mfn = _mfn(entry.p2m.base);
     ASSERT(mfn_valid(mfn));
 
-    pg = mfn_to_page(mfn_x(mfn));
+    pg = mfn_to_page(mfn);
 
     page_list_del(pg, &p2m->pages);
     free_domheap_page(pg);
@@ -780,7 +786,7 @@ static bool p2m_split_superpage(struct p2m_domain *p2m, lpae_t *entry,
 
     unmap_domain_page(table);
 
-    pte = mfn_to_p2m_entry(_mfn(page_to_mfn(page)), p2m_invalid,
+    pte = mfn_to_p2m_entry(page_to_mfn(page), p2m_invalid,
                            p2m->default_access);
 
     /*
@@ -1443,7 +1449,7 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
     if ( !mfn_valid(maddr_to_mfn(maddr)) )
         goto err;
 
-    page = mfn_to_page(mfn_x(maddr_to_mfn(maddr)));
+    page = mfn_to_page(maddr_to_mfn(maddr));
     ASSERT(page);
 
     if ( unlikely(!get_page(page, d)) )
-- 
2.11.0


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

[PATCH 20/24] xen/arm: mm: Redefine virt_to_mfn to support typesafe

[Julien Grall]

The file xen/arch/arm/mm.c is using the typesafe MFN in most of the
place. This requires all caller of virt_to_mfn to prefixed by _mfn(...).

To avoid the extra _mfn(...), re-defined virt_to_mfn within arch/arm/mm.c
to handle typesafe MFN.

This patch also introduce __virt_to_mfn, so virt_to_mfn can be
re-defined easily.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/mm.c        | 16 ++++++++++------
 xen/include/asm-arm/mm.h |  3 ++-
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 452c1e26c3..2ff1688f3f 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -44,6 +44,10 @@
 
 struct domain *dom_xen, *dom_io, *dom_cow;
 
+/* Override macros from asm/page.h to make them work with mfn_t */
+#undef virt_to_mfn
+#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
+
 /* Static start-of-day pagetables that we use before the allocators
  * are up. These are used by all CPUs during bringup before switching
  * to the CPUs own pagetables.
@@ -479,7 +483,7 @@ unsigned long domain_page_map_to_mfn(const void *ptr)
     unsigned long offset = (va>>THIRD_SHIFT) & LPAE_ENTRY_MASK;
 
     if ( va >= VMAP_VIRT_START && va < VMAP_VIRT_END )
-        return virt_to_mfn(va);
+        return mfn_x(virt_to_mfn(va));
 
     ASSERT(slot >= 0 && slot < DOMHEAP_ENTRIES);
     ASSERT(map[slot].pt.avail != 0);
@@ -764,7 +768,7 @@ int init_secondary_pagetables(int cpu)
      * domheap mapping pages. */
     for ( i = 0; i < DOMHEAP_SECOND_PAGES; i++ )
     {
-        pte = mfn_to_xen_entry(_mfn(virt_to_mfn(domheap+i*LPAE_ENTRIES)),
+        pte = mfn_to_xen_entry(virt_to_mfn(domheap+i*LPAE_ENTRIES),
                                WRITEALLOC);
         pte.pt.table = 1;
         write_pte(&first[first_table_offset(DOMHEAP_VIRT_START+i*FIRST_SIZE)], pte);
@@ -961,7 +965,7 @@ static int create_xen_table(lpae_t *entry)
     if ( p == NULL )
         return -ENOMEM;
     clear_page(p);
-    pte = mfn_to_xen_entry(_mfn(virt_to_mfn(p)), WRITEALLOC);
+    pte = mfn_to_xen_entry(virt_to_mfn(p), WRITEALLOC);
     pte.pt.table = 1;
     write_pte(entry, pte);
     return 0;
@@ -1216,7 +1220,7 @@ int xenmem_add_to_physmap_one(
     unsigned long idx,
     gfn_t gfn)
 {
-    unsigned long mfn = 0;
+    mfn_t mfn = INVALID_MFN;
     int rc;
     p2m_type_t t;
     struct page_info *page = NULL;
@@ -1302,7 +1306,7 @@ int xenmem_add_to_physmap_one(
             return -EINVAL;
         }
 
-        mfn = page_to_mfn(page);
+        mfn = _mfn(page_to_mfn(page));
         t = p2m_map_foreign;
 
         rcu_unlock_domain(od);
@@ -1321,7 +1325,7 @@ int xenmem_add_to_physmap_one(
     }
 
     /* Map at new location. */
-    rc = guest_physmap_add_entry(d, gfn, _mfn(mfn), 0, t);
+    rc = guest_physmap_add_entry(d, gfn, mfn, 0, t);
 
     /* If we fail to add the mapping, we need to drop the reference we
      * took earlier on foreign pages */
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index b2e7ea7761..6e2b3c7f2b 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -264,7 +264,7 @@ static inline int gvirt_to_maddr(vaddr_t va, paddr_t *pa, unsigned int flags)
 #define __va(x)             (maddr_to_virt(x))
 
 /* Convert between Xen-heap virtual addresses and machine frame numbers. */
-#define virt_to_mfn(va)   (virt_to_maddr(va) >> PAGE_SHIFT)
+#define __virt_to_mfn(va) (virt_to_maddr(va) >> PAGE_SHIFT)
 #define mfn_to_virt(mfn)  (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT))
 
 /*
@@ -274,6 +274,7 @@ static inline int gvirt_to_maddr(vaddr_t va, paddr_t *pa, unsigned int flags)
  */
 #define mfn_to_page(mfn)    __mfn_to_page(mfn)
 #define page_to_mfn(pg)     __page_to_mfn(pg)
+#define virt_to_mfn(va)     __virt_to_mfn(va)
 
 /* Convert between Xen-heap virtual addresses and page-info structures. */
 static inline struct page_info *virt_to_page(const void *v)
-- 
2.11.0


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

[PATCH 21/24] xen/arm: domain_build: Redefine virt_to_mfn to support typesafe

[Julien Grall]

The file xen/arch/arm/domain_build.c is using typesafe MFN in most of
the place. The only caller to virt_to_mfn is using prefixed with
_mfn(...).

To avoid extra _mfn(...), re-define virt_to_mfn within
arch/arm/domain_build.c to handle typesafe MFN.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/domain_build.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index c6776d76fc..1bec4fa23d 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -45,6 +45,10 @@ struct map_range_data
     p2m_type_t p2mt;
 };
 
+/* Override macros from asm/page.h to make them work with mfn_t */
+#undef virt_to_mfn
+#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
+
 //#define DEBUG_11_ALLOCATION
 #ifdef DEBUG_11_ALLOCATION
 # define D11PRINT(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
@@ -1903,7 +1907,7 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
     rc = map_regions_p2mt(d,
                           gaddr_to_gfn(d->arch.efi_acpi_gpa),
                           PFN_UP(d->arch.efi_acpi_len),
-                          _mfn(virt_to_mfn(d->arch.efi_acpi_table)),
+                          virt_to_mfn(d->arch.efi_acpi_table),
                           p2m_mmio_direct_c);
     if ( rc != 0 )
     {
-- 
2.11.0


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

[PATCH 22/24] xen/arm: alternative: Redefine virt_to_mfn to support typesafe

[Julien Grall]

The file xen/arch/arm/alternative.c is using typesafe MFN in most of
the place. The only caller to virt_to_mfn is using with _mfn(...).

To avoid extra _mfn(...), re-define virt_to_mfn within
xen/arch/arm/alternative.c to handle typesafe MFN.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/alternative.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
index 4d7e5b6155..a3bcda3117 100644
--- a/xen/arch/arm/alternative.c
+++ b/xen/arch/arm/alternative.c
@@ -32,6 +32,10 @@
 #include <asm/insn.h>
 #include <asm/page.h>
 
+/* Override macros from asm/page.h to make them work with mfn_t */
+#undef virt_to_mfn
+#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
+
 extern const struct alt_instr __alt_instructions[], __alt_instructions_end[];
 
 struct alt_region {
@@ -154,7 +158,7 @@ static int __apply_alternatives_multi_stop(void *unused)
     {
         int ret;
         struct alt_region region;
-        mfn_t xen_mfn = _mfn(virt_to_mfn(_start));
+        mfn_t xen_mfn = virt_to_mfn(_start);
         paddr_t xen_size = _end - _start;
         unsigned int xen_order = get_order_from_bytes(xen_size);
         void *xenmap;
-- 
2.11.0


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

[PATCH 23/24] xen/arm: livepatch: Redefine virt_to_mfn to support typesafe

[Julien Grall]

The file xen/arch/arm/livepatch.c is using typesafe MFN in most of
the place. The only caller to virt_to_mfn is using with _mfn(...).

To avoid extra _mfn(...), re-define virt_to_mfn within
xen/arch/arm/livepatch.c to handle typesafe MFN.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 xen/arch/arm/livepatch.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
index de95e54744..3e53524365 100644
--- a/xen/arch/arm/livepatch.c
+++ b/xen/arch/arm/livepatch.c
@@ -12,6 +12,10 @@
 #include <asm/livepatch.h>
 #include <asm/mm.h>
 
+/* Override macros from asm/page.h to make them work with mfn_t */
+#undef virt_to_mfn
+#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
+
 void *vmap_of_xen_text;
 
 int arch_livepatch_quiesce(void)
@@ -22,7 +26,7 @@ int arch_livepatch_quiesce(void)
     if ( vmap_of_xen_text )
         return -EINVAL;
 
-    text_mfn = _mfn(virt_to_mfn(_start));
+    text_mfn = virt_to_mfn(_start);
     text_order = get_order_from_bytes(_end - _start);
 
     /*
-- 
2.11.0


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

[PATCH 24/24] xen/arm: create_xen_entries: Use typesafe MFN

[Julien Grall]

Add a bit more safety when using create_xen_entries.

Also when destroying/modifying mapping, the MFN is currently not used.
Rather than passing _mfn(0) use INVALID_MFN to stay consistent with the
other usage.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/mm.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 2ff1688f3f..8cb0559972 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -980,7 +980,7 @@ enum xenmap_operation {
 
 static int create_xen_entries(enum xenmap_operation op,
                               unsigned long virt,
-                              unsigned long mfn,
+                              mfn_t mfn,
                               unsigned long nr_mfns,
                               unsigned int ai)
 {
@@ -989,7 +989,7 @@ static int create_xen_entries(enum xenmap_operation op,
     lpae_t pte;
     lpae_t *third = NULL;
 
-    for(; addr < addr_end; addr += PAGE_SIZE, mfn++)
+    for(; addr < addr_end; addr += PAGE_SIZE, mfn = mfn_add(mfn, 1))
     {
         if ( !xen_second[second_linear_offset(addr)].pt.valid ||
              !xen_second[second_linear_offset(addr)].pt.table )
@@ -1010,13 +1010,13 @@ static int create_xen_entries(enum xenmap_operation op,
             case RESERVE:
                 if ( third[third_table_offset(addr)].pt.valid )
                 {
-                    printk("create_xen_entries: trying to replace an existing mapping addr=%lx mfn=%lx\n",
-                           addr, mfn);
+                    printk("create_xen_entries: trying to replace an existing mapping addr=%lx mfn=%"PRI_mfn"\n",
+                           addr, mfn_x(mfn));
                     return -EINVAL;
                 }
                 if ( op == RESERVE )
                     break;
-                pte = mfn_to_xen_entry(_mfn(mfn), ai);
+                pte = mfn_to_xen_entry(mfn, ai);
                 pte.pt.table = 1;
                 write_pte(&third[third_table_offset(addr)], pte);
                 break;
@@ -1061,24 +1061,25 @@ int map_pages_to_xen(unsigned long virt,
                      unsigned long nr_mfns,
                      unsigned int flags)
 {
-    return create_xen_entries(INSERT, virt, mfn, nr_mfns, flags);
+    return create_xen_entries(INSERT, virt, _mfn(mfn), nr_mfns, flags);
 }
 
 int populate_pt_range(unsigned long virt, unsigned long mfn,
                       unsigned long nr_mfns)
 {
-    return create_xen_entries(RESERVE, virt, mfn, nr_mfns, 0);
+    return create_xen_entries(RESERVE, virt, _mfn(mfn), nr_mfns, 0);
 }
 
 int destroy_xen_mappings(unsigned long v, unsigned long e)
 {
-    return create_xen_entries(REMOVE, v, 0, (e - v) >> PAGE_SHIFT, 0);
+    return create_xen_entries(REMOVE, v, INVALID_MFN, (e - v) >> PAGE_SHIFT, 0);
 }
 
 int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags)
 {
     ASSERT((flags & (PTE_NX | PTE_RO)) == flags);
-    return create_xen_entries(MODIFY, s, 0, (e - s) >> PAGE_SHIFT, flags);
+    return create_xen_entries(MODIFY, s, INVALID_MFN, (e - s) >> PAGE_SHIFT,
+                              flags);
 }
 
 enum mg { mg_clear, mg_ro, mg_rw, mg_rx };
-- 
2.11.0


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

Re: [PATCH 01/24] xen/mm: Don't use _{g, m}fn for defining INVALID_{G, M}FN

[Andrew Cooper]

On 13/06/17 17:13, Julien Grall wrote:
> INVALID_{G,M}FN are defined using static inline helpers _{g,m}fn.
> This means, they cannot be used to initialize a build time static variable:
>
> In file included from mm.c:24:0:
> xen/xen/include/xen/mm.h:59:26: error: initializer element is not constant
>  #define INVALID_MFN      _mfn(~0UL)
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
>
> I know that this solution will not work for non-debug build. I would
> like input from the community on way to fix it nicely.

Hmm - a proper typedef gets inserted.  I presume the compiler objects to
(unsigned long){ ~0UL } to initialise a scalar?

It might be better to move the definition of INVALID_$FOO into the
TYPE_SAFE() declaration so we can create an appropriate initialiser for
each both builds.

~Andrew

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

Re: [PATCH 01/24] xen/mm: Don't use _{g, m}fn for defining INVALID_{G, M}FN

[Jan Beulich]

>>> On 13.06.17 at 18:20, <andrew.cooper3@citrix.com> wrote:
> On 13/06/17 17:13, Julien Grall wrote:
>> INVALID_{G,M}FN are defined using static inline helpers _{g,m}fn.
>> This means, they cannot be used to initialize a build time static variable:
>>
>> In file included from mm.c:24:0:
>> xen/xen/include/xen/mm.h:59:26: error: initializer element is not constant
>>  #define INVALID_MFN      _mfn(~0UL)
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> ---
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>> Cc: Tim Deegan <tim@xen.org>
>> Cc: Wei Liu <wei.liu2@citrix.com>
>>
>> I know that this solution will not work for non-debug build. I would
>> like input from the community on way to fix it nicely.
> 
> Hmm - a proper typedef gets inserted.  I presume the compiler objects to
> (unsigned long){ ~0UL } to initialise a scalar?
> 
> It might be better to move the definition of INVALID_$FOO into the
> TYPE_SAFE() declaration so we can create an appropriate initialiser for
> each both builds.

Except that you can't put a #define in a macro definition, and producing
a static const wouldn't help Julien's case of wanting it in the initializer of
a static variable. So best I can come up with right now would be to
introduce a separate

#define TYPE_SAFE_CONSTANT(name, val) \
    (name##_t){ val }

and

#define TYPE_SAFE_CONSTANT(name, val) \
    (name##_t)(val)

for the !NDEBUG / NDEBUG cases respectively and use it as

#define INVALID_MFN TYPE_SAFE_CONSTANT(mfn, ~0UL)

Jan


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

Re: [PATCH 01/24] xen/mm: Don't use _{g, m}fn for defining INVALID_{G, M}FN

[Tim Deegan]

At 17:13 +0100 on 13 Jun (1497373980), Julien Grall wrote:
> INVALID_{G,M}FN are defined using static inline helpers _{g,m}fn.
> This means, they cannot be used to initialize a build time static variable:
> 
> In file included from mm.c:24:0:
> xen/xen/include/xen/mm.h:59:26: error: initializer element is not constant
>  #define INVALID_MFN      _mfn(~0UL)
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Acked-by: Tim Deegan <tim@xen.org>

> I know that this solution will not work for non-debug build. I would
> like input from the community on way to fix it nicely.

It seems to WFM: https://godbolt.org/g/vEVNY3

Tim.

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

Re: [PATCH 01/24] xen/mm: Don't use _{g, m}fn for defining INVALID_{G, M}FN

[Julien Grall]

Hi Tim,

On 06/14/2017 09:49 AM, Tim Deegan wrote:
> At 17:13 +0100 on 13 Jun (1497373980), Julien Grall wrote:
>> INVALID_{G,M}FN are defined using static inline helpers _{g,m}fn.
>> This means, they cannot be used to initialize a build time static variable:
>>
>> In file included from mm.c:24:0:
>> xen/xen/include/xen/mm.h:59:26: error: initializer element is not constant
>>   #define INVALID_MFN      _mfn(~0UL)
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> Acked-by: Tim Deegan <tim@xen.org>
> 
>> I know that this solution will not work for non-debug build. I would
>> like input from the community on way to fix it nicely.
> 
> It seems to WFM: https://godbolt.org/g/vEVNY3

To be honest, I thought it would not work and a bit surprised that it 
actually works.

I am happy to keep like that or introduced a new define (such as 
TYPE_SAFE_CONSTANT suggested by Jan).

Cheers,

-- 
Julien Grall

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

Re: [PATCH 01/24] xen/mm: Don't use _{g, m}fn for defining INVALID_{G, M}FN

[Jan Beulich]

>>> On 14.06.17 at 11:40, <julien.grall@arm.com> wrote:
> On 06/14/2017 09:49 AM, Tim Deegan wrote:
>> At 17:13 +0100 on 13 Jun (1497373980), Julien Grall wrote:
>>> INVALID_{G,M}FN are defined using static inline helpers _{g,m}fn.
>>> This means, they cannot be used to initialize a build time static variable:
>>>
>>> In file included from mm.c:24:0:
>>> xen/xen/include/xen/mm.h:59:26: error: initializer element is not constant
>>>   #define INVALID_MFN      _mfn(~0UL)
>>>
>>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> 
>> Acked-by: Tim Deegan <tim@xen.org>
>> 
>>> I know that this solution will not work for non-debug build. I would
>>> like input from the community on way to fix it nicely.
>> 
>> It seems to WFM: https://godbolt.org/g/vEVNY3 
> 
> To be honest, I thought it would not work and a bit surprised that it 
> actually works.

Me too - it didn't occur to me that the extension "Compound Literals"
also, contrary to its name, covers scalar types. But it's indeed
documented, so we don't need to be afraid of it going away.
Remains the question whether our oldest supported gcc allows it.
The oldest one I can easily try (4.3.something) works fine.

Jan


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

Re: [PATCH 15/24] xen/arm: Use the newly introduced MFN <-> MADDR and GFN <-> MADDR helpers

[Tamas K Lengyel]

On Tue, Jun 13, 2017 at 10:13 AM, Julien Grall <julien.grall@arm.com> wrote:
> Replace the following constructions:
>     - _gfn(paddr_to_pfn(...))   => gaddr_to_gfn(...)
>     - _mfn(paddr_to_pfn(...))   => maddr_to_mfn(...)
>     - pfn_to_paddr(mfn_x(...))  => mfn_to_maddr(...)
>     - pfn_to_paddr(gfn_x(...))  => gfn_to_gaddr(...)
>     - _mfn(... >> PAGE_SHIFT)   => maddr_to_mfn(...)
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Cc: Tamas K Lengyel <tamas@tklengyel.com>

Cool, this makes things a lot more readable!

For the mem_access bits:
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>

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

Re: [PATCH 02/24] xen/arm: gic-v2: Fix indentation in gicv2_map_hwdom_extra_mappings

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/gic-v2.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 270a1362ec..f8124e5e54 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -598,8 +598,8 @@ static int gicv2_map_hwdown_extra_mappings(struct domain *d)
>                 v2m_data->spi_start, v2m_data->nr_spis);
>  
>          ret = map_mmio_regions(d, _gfn(paddr_to_pfn(v2m_data->addr)),
> -                            DIV_ROUND_UP(v2m_data->size, PAGE_SIZE),
> -                            _mfn(paddr_to_pfn(v2m_data->addr)));
> +                               DIV_ROUND_UP(v2m_data->size, PAGE_SIZE),
> +                               _mfn(paddr_to_pfn(v2m_data->addr)));
>          if ( ret )
>          {
>              printk(XENLOG_ERR "GICv2: Map v2m frame to d%d failed.\n",
> -- 
> 2.11.0
> 

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

Re: [PATCH 03/24] xen/arm: setup: Remove bogus xenheap_mfn_end in setup_mm for arm64

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> xenheap_mfn_end is storing an MFN and not a physical address. Thankfully
> xenheap_mfn_end is not used in the arm64 code. So drop it.

That's fine, but in that case I would prefer to move the definition of
xenheap_mfn_end under #ifdef CONFIG_ARM_32. In fact, there is another
assignment of xenheap_mfn_end few lines below in the arm64 version of
setup_mm: don't we need to remove that too?


> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/setup.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index f00f29a45b..ab4d8e4218 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -654,8 +654,6 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
>              if ( e > bank_end )
>                  e = bank_end;
>  
> -            xenheap_mfn_end = e;
> -
>              dt_unreserved_regions(s, e, init_boot_pages, 0);
>              s = n;
>          }
> -- 
> 2.11.0
> 

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

Re: [PATCH 04/24] xen/arm: mm: Introduce clear_table and use it

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> Add a new helper clear_table to clear a page table entry and invalidate
> the cache.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/mm.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 082c872c72..b4ff777b55 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -529,6 +529,13 @@ void __init remove_early_mappings(void)
>  
>  extern void relocate_xen(uint64_t ttbr, void *src, void *dst, size_t len);
>  
> +/* Clear a translation table and clean & invalidate the cache */
> +static void clear_table(void *table)

This could be a static inline. In any case:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> +{
> +    clear_page(table);
> +    clean_and_invalidate_dcache_va_range(table, PAGE_SIZE);
> +}
> +
>  /* Boot-time pagetable setup.
>   * Changes here may need matching changes in head.S */
>  void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
> @@ -604,18 +611,13 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
>  
>      /* Clear the copy of the boot pagetables. Each secondary CPU
>       * rebuilds these itself (see head.S) */
> -    memset(boot_pgtable, 0x0, PAGE_SIZE);
> -    clean_and_invalidate_dcache(boot_pgtable);
> +    clear_table(boot_pgtable);
>  #ifdef CONFIG_ARM_64
> -    memset(boot_first, 0x0, PAGE_SIZE);
> -    clean_and_invalidate_dcache(boot_first);
> -    memset(boot_first_id, 0x0, PAGE_SIZE);
> -    clean_and_invalidate_dcache(boot_first_id);
> +    clear_table(boot_first);
> +    clear_table(boot_first_id);
>  #endif
> -    memset(boot_second, 0x0, PAGE_SIZE);
> -    clean_and_invalidate_dcache(boot_second);
> -    memset(boot_third, 0x0, PAGE_SIZE);
> -    clean_and_invalidate_dcache(boot_third);
> +    clear_table(boot_second);
> +    clear_table(boot_third);
>  
>      /* Break up the Xen mapping into 4k pages and protect them separately. */
>      for ( i = 0; i < LPAE_ENTRIES; i++ )
> -- 
> 2.11.0
> 

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

Re: [PATCH 05/24] xen/arm: mm: Move mfn_to_xen_entry from page.h to mm.c

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> The file mm.c is the only user of mfn_to_xen_entry. This will also help
> to use the typesafe MFN.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/mm.c          | 65 ++++++++++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/page.h | 65 ----------------------------------------------
>  2 files changed, 65 insertions(+), 65 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index b4ff777b55..587a6b3975 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -254,6 +254,71 @@ void dump_hyp_walk(vaddr_t addr)
>      dump_pt_walk(ttbr, addr, HYP_PT_ROOT_LEVEL, 1);
>  }
>  
> +/* Standard entry type that we'll use to build Xen's own pagetables.
> + * We put the same permissions at every level, because they're ignored
> + * by the walker in non-leaf entries. */
> +static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
> +{
> +    paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT;
> +    lpae_t e = (lpae_t) {
> +        .pt = {
> +            .valid = 1,           /* Mappings are present */
> +            .table = 0,           /* Set to 1 for links and 4k maps */
> +            .ai = attr,
> +            .ns = 1,              /* Hyp mode is in the non-secure world */
> +            .user = 1,            /* See below */
> +            .ro = 0,              /* Assume read-write */
> +            .af = 1,              /* No need for access tracking */
> +            .ng = 1,              /* Makes TLB flushes easier */
> +            .contig = 0,          /* Assume non-contiguous */
> +            .xn = 1,              /* No need to execute outside .text */
> +            .avail = 0,           /* Reference count for domheap mapping */
> +        }};;
> +    /* Setting the User bit is strange, but the ATS1H[RW] instructions
> +     * don't seem to work otherwise, and since we never run on Xen
> +     * pagetables in User mode it's OK.  If this changes, remember
> +     * to update the hard-coded values in head.S too */
> +
> +    switch ( attr )
> +    {
> +    case BUFFERABLE:
> +        /*
> +         * ARM ARM: Overlaying the shareability attribute (DDI
> +         * 0406C.b B3-1376 to 1377)
> +         *
> +         * A memory region with a resultant memory type attribute of Normal,
> +         * and a resultant cacheability attribute of Inner Non-cacheable,
> +         * Outer Non-cacheable, must have a resultant shareability attribute
> +         * of Outer Shareable, otherwise shareability is UNPREDICTABLE.
> +         *
> +         * On ARMv8 sharability is ignored and explicitly treated as Outer
> +         * Shareable for Normal Inner Non_cacheable, Outer Non-cacheable.
> +         */
> +        e.pt.sh = LPAE_SH_OUTER;
> +        break;
> +    case UNCACHED:
> +    case DEV_SHARED:
> +        /* Shareability is ignored for non-Normal memory, Outer is as
> +         * good as anything.
> +         *
> +         * On ARMv8 sharability is ignored and explicitly treated as Outer
> +         * Shareable for any device memory type.
> +         */
> +        e.pt.sh = LPAE_SH_OUTER;
> +        break;
> +    default:
> +        e.pt.sh = LPAE_SH_INNER;  /* Xen mappings are SMP coherent */
> +        break;
> +    }
> +
> +    ASSERT(!(pa & ~PAGE_MASK));
> +    ASSERT(!(pa & ~PADDR_MASK));
> +
> +    // XXX shifts
> +    e.bits |= pa;
> +    return e;
> +}
> +
>  /* Map a 4k page in a fixmap entry */
>  void set_fixmap(unsigned map, unsigned long mfn, unsigned attributes)
>  {
> diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> index 497b4c86ad..3670ab665d 100644
> --- a/xen/include/asm-arm/page.h
> +++ b/xen/include/asm-arm/page.h
> @@ -205,71 +205,6 @@ typedef union {
>      lpae_walk_t walk;
>  } lpae_t;
>  
> -/* Standard entry type that we'll use to build Xen's own pagetables.
> - * We put the same permissions at every level, because they're ignored
> - * by the walker in non-leaf entries. */
> -static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
> -{
> -    paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT;
> -    lpae_t e = (lpae_t) {
> -        .pt = {
> -            .valid = 1,           /* Mappings are present */
> -            .table = 0,           /* Set to 1 for links and 4k maps */
> -            .ai = attr,
> -            .ns = 1,              /* Hyp mode is in the non-secure world */
> -            .user = 1,            /* See below */
> -            .ro = 0,              /* Assume read-write */
> -            .af = 1,              /* No need for access tracking */
> -            .ng = 1,              /* Makes TLB flushes easier */
> -            .contig = 0,          /* Assume non-contiguous */
> -            .xn = 1,              /* No need to execute outside .text */
> -            .avail = 0,           /* Reference count for domheap mapping */
> -        }};;
> -    /* Setting the User bit is strange, but the ATS1H[RW] instructions
> -     * don't seem to work otherwise, and since we never run on Xen
> -     * pagetables in User mode it's OK.  If this changes, remember
> -     * to update the hard-coded values in head.S too */
> -
> -    switch ( attr )
> -    {
> -    case BUFFERABLE:
> -        /*
> -         * ARM ARM: Overlaying the shareability attribute (DDI
> -         * 0406C.b B3-1376 to 1377)
> -         *
> -         * A memory region with a resultant memory type attribute of Normal,
> -         * and a resultant cacheability attribute of Inner Non-cacheable,
> -         * Outer Non-cacheable, must have a resultant shareability attribute
> -         * of Outer Shareable, otherwise shareability is UNPREDICTABLE.
> -         *
> -         * On ARMv8 sharability is ignored and explicitly treated as Outer
> -         * Shareable for Normal Inner Non_cacheable, Outer Non-cacheable.
> -         */
> -        e.pt.sh = LPAE_SH_OUTER;
> -        break;
> -    case UNCACHED:
> -    case DEV_SHARED:
> -        /* Shareability is ignored for non-Normal memory, Outer is as
> -         * good as anything.
> -         *
> -         * On ARMv8 sharability is ignored and explicitly treated as Outer
> -         * Shareable for any device memory type.
> -         */
> -        e.pt.sh = LPAE_SH_OUTER;
> -        break;
> -    default:
> -        e.pt.sh = LPAE_SH_INNER;  /* Xen mappings are SMP coherent */
> -        break;
> -    }
> -
> -    ASSERT(!(pa & ~PAGE_MASK));
> -    ASSERT(!(pa & ~PADDR_MASK));
> -
> -    // XXX shifts
> -    e.bits |= pa;
> -    return e;
> -}
> -
>  #if defined(CONFIG_ARM_32)
>  # include <asm/arm32/page.h>
>  #elif defined(CONFIG_ARM_64)
> -- 
> 2.11.0
> 

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

Re: [PATCH 06/24] xen/arm: mm: Fix coding style of mfn_to_xen_entry

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> Fix the comment coding style and add a newline before the return.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>  xen/arch/arm/mm.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 587a6b3975..6f63e4315a 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -254,9 +254,11 @@ void dump_hyp_walk(vaddr_t addr)
>      dump_pt_walk(ttbr, addr, HYP_PT_ROOT_LEVEL, 1);
>  }
>  
> -/* Standard entry type that we'll use to build Xen's own pagetables.
> +/*
> + * Standard entry type that we'll use to build Xen's own pagetables.
>   * We put the same permissions at every level, because they're ignored
> - * by the walker in non-leaf entries. */
> + * by the walker in non-leaf entries.
> + */
>  static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
>  {
>      paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT;
> @@ -274,10 +276,12 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
>              .xn = 1,              /* No need to execute outside .text */
>              .avail = 0,           /* Reference count for domheap mapping */
>          }};;
> -    /* Setting the User bit is strange, but the ATS1H[RW] instructions
> +    /*
> +     * Setting the User bit is strange, but the ATS1H[RW] instructions
>       * don't seem to work otherwise, and since we never run on Xen
>       * pagetables in User mode it's OK.  If this changes, remember
> -     * to update the hard-coded values in head.S too */
> +     * to update the hard-coded values in head.S too.
> +     */
>  
>      switch ( attr )
>      {
> @@ -298,7 +302,8 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
>          break;
>      case UNCACHED:
>      case DEV_SHARED:
> -        /* Shareability is ignored for non-Normal memory, Outer is as
> +        /*
> +         * Shareability is ignored for non-Normal memory, Outer is as
>           * good as anything.
>           *
>           * On ARMv8 sharability is ignored and explicitly treated as Outer
> @@ -314,8 +319,9 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
>      ASSERT(!(pa & ~PAGE_MASK));
>      ASSERT(!(pa & ~PADDR_MASK));
>  
> -    // XXX shifts
> +    /* XXX shifts */
>      e.bits |= pa;
> +
>      return e;
>  }
>  
> -- 
> 2.11.0
> 

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

Re: [PATCH 07/24] xen/arm: mm: Clean-up mfn_to_xen_entry

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> The physical address is computed from the machine frame number, so
> checking if the physical address is page aligned is pointless.
> 
> Furthermore, directly assigned the MFN to the corresponding field in the
> entry rather than converting to a physical address and orring the value.
> It will avoid to rely on the field position and make the code clearer.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/mm.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 6f63e4315a..d164ed2eda 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -261,7 +261,6 @@ void dump_hyp_walk(vaddr_t addr)
>   */
>  static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
>  {
> -    paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT;
>      lpae_t e = (lpae_t) {
>          .pt = {
>              .valid = 1,           /* Mappings are present */
> @@ -316,11 +315,9 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
>          break;
>      }
>  
> -    ASSERT(!(pa & ~PAGE_MASK));
> -    ASSERT(!(pa & ~PADDR_MASK));
> +    ASSERT(!(pfn_to_paddr(mfn) & ~PADDR_MASK));
>  
> -    /* XXX shifts */
> -    e.bits |= pa;
> +    e.pt.base = mfn;
>  
>      return e;
>  }
> -- 
> 2.11.0
> 

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

Re: [PATCH 08/24] xen/arm: mm: Use typesafe MFN in mfn_to_xen_entry

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/mm.c | 33 +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index d164ed2eda..08116679ec 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -259,7 +259,7 @@ void dump_hyp_walk(vaddr_t addr)
>   * We put the same permissions at every level, because they're ignored
>   * by the walker in non-leaf entries.
>   */
> -static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
> +static inline lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned attr)
>  {
>      lpae_t e = (lpae_t) {
>          .pt = {
> @@ -315,9 +315,9 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
>          break;
>      }
>  
> -    ASSERT(!(pfn_to_paddr(mfn) & ~PADDR_MASK));
> +    ASSERT(!(pfn_to_paddr(mfn_x(mfn)) & ~PADDR_MASK));
>  
> -    e.pt.base = mfn;
> +    e.pt.base = mfn_x(mfn);
>  
>      return e;
>  }
> @@ -325,7 +325,7 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
>  /* Map a 4k page in a fixmap entry */
>  void set_fixmap(unsigned map, unsigned long mfn, unsigned attributes)
>  {
> -    lpae_t pte = mfn_to_xen_entry(mfn, attributes);
> +    lpae_t pte = mfn_to_xen_entry(_mfn(mfn), attributes);
>      pte.pt.table = 1; /* 4k mappings always have this bit set */
>      pte.pt.xn = 1;
>      write_pte(xen_fixmap + third_table_offset(FIXMAP_ADDR(map)), pte);
> @@ -363,7 +363,7 @@ static void __init create_mappings(lpae_t *second,
>  
>      count = nr_mfns / LPAE_ENTRIES;
>      p = second + second_linear_offset(virt_offset);
> -    pte = mfn_to_xen_entry(base_mfn, WRITEALLOC);
> +    pte = mfn_to_xen_entry(_mfn(base_mfn), WRITEALLOC);
>      if ( granularity == 16 * LPAE_ENTRIES )
>          pte.pt.contig = 1;  /* These maps are in 16-entry contiguous chunks. */
>      for ( i = 0; i < count; i++ )
> @@ -416,7 +416,7 @@ void *map_domain_page(mfn_t mfn)
>          else if ( map[slot].pt.avail == 0 )
>          {
>              /* Commandeer this 2MB slot */
> -            pte = mfn_to_xen_entry(slot_mfn, WRITEALLOC);
> +            pte = mfn_to_xen_entry(_mfn(slot_mfn), WRITEALLOC);
>              pte.pt.avail = 1;
>              write_pte(map + slot, pte);
>              break;
> @@ -537,7 +537,7 @@ static inline lpae_t pte_of_xenaddr(vaddr_t va)
>  {
>      paddr_t ma = va + phys_offset;
>      unsigned long mfn = ma >> PAGE_SHIFT;
> -    return mfn_to_xen_entry(mfn, WRITEALLOC);
> +    return mfn_to_xen_entry(_mfn(mfn), WRITEALLOC);
>  }
>  
>  /* Map the FDT in the early boot page table */
> @@ -646,7 +646,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
>      /* Initialise xen second level entries ... */
>      /* ... Xen's text etc */
>  
> -    pte = mfn_to_xen_entry(xen_paddr>>PAGE_SHIFT, WRITEALLOC);
> +    pte = mfn_to_xen_entry(_mfn(xen_paddr>>PAGE_SHIFT), WRITEALLOC);
>      pte.pt.xn = 0;/* Contains our text mapping! */
>      xen_second[second_table_offset(XEN_VIRT_START)] = pte;
>  
> @@ -663,7 +663,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
>  
>      /* ... Boot Misc area for xen relocation */
>      dest_va = BOOT_RELOC_VIRT_START;
> -    pte = mfn_to_xen_entry(xen_paddr >> PAGE_SHIFT, WRITEALLOC);
> +    pte = mfn_to_xen_entry(_mfn(xen_paddr >> PAGE_SHIFT), WRITEALLOC);
>      /* Map the destination in xen_second. */
>      xen_second[second_table_offset(dest_va)] = pte;
>      /* Map the destination in boot_second. */
> @@ -694,7 +694,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
>          unsigned long va = XEN_VIRT_START + (i << PAGE_SHIFT);
>          if ( !is_kernel(va) )
>              break;
> -        pte = mfn_to_xen_entry(mfn, WRITEALLOC);
> +        pte = mfn_to_xen_entry(_mfn(mfn), WRITEALLOC);
>          pte.pt.table = 1; /* 4k mappings always have this bit set */
>          if ( is_kernel_text(va) || is_kernel_inittext(va) )
>          {
> @@ -764,7 +764,8 @@ int init_secondary_pagetables(int cpu)
>       * domheap mapping pages. */
>      for ( i = 0; i < DOMHEAP_SECOND_PAGES; i++ )
>      {
> -        pte = mfn_to_xen_entry(virt_to_mfn(domheap+i*LPAE_ENTRIES), WRITEALLOC);
> +        pte = mfn_to_xen_entry(_mfn(virt_to_mfn(domheap+i*LPAE_ENTRIES)),
> +                               WRITEALLOC);
>          pte.pt.table = 1;
>          write_pte(&first[first_table_offset(DOMHEAP_VIRT_START+i*FIRST_SIZE)], pte);
>      }
> @@ -862,13 +863,13 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
>              unsigned long first_mfn = alloc_boot_pages(1, 1);
>  
>              clear_page(mfn_to_virt(first_mfn));
> -            pte = mfn_to_xen_entry(first_mfn, WRITEALLOC);
> +            pte = mfn_to_xen_entry(_mfn(first_mfn), WRITEALLOC);
>              pte.pt.table = 1;
>              write_pte(p, pte);
>              first = mfn_to_virt(first_mfn);
>          }
>  
> -        pte = mfn_to_xen_entry(mfn, WRITEALLOC);
> +        pte = mfn_to_xen_entry(_mfn(mfn), WRITEALLOC);
>          /* TODO: Set pte.pt.contig when appropriate. */
>          write_pte(&first[first_table_offset(vaddr)], pte);
>  
> @@ -907,7 +908,7 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
>      for ( i = 0; i < nr_second; i++ )
>      {
>          clear_page(mfn_to_virt(second_base + i));
> -        pte = mfn_to_xen_entry(second_base + i, WRITEALLOC);
> +        pte = mfn_to_xen_entry(_mfn(second_base + i), WRITEALLOC);
>          pte.pt.table = 1;
>          write_pte(&xen_first[first_table_offset(FRAMETABLE_VIRT_START)+i], pte);
>      }
> @@ -960,7 +961,7 @@ static int create_xen_table(lpae_t *entry)
>      if ( p == NULL )
>          return -ENOMEM;
>      clear_page(p);
> -    pte = mfn_to_xen_entry(virt_to_mfn(p), WRITEALLOC);
> +    pte = mfn_to_xen_entry(_mfn(virt_to_mfn(p)), WRITEALLOC);
>      pte.pt.table = 1;
>      write_pte(entry, pte);
>      return 0;
> @@ -1011,7 +1012,7 @@ static int create_xen_entries(enum xenmap_operation op,
>                  }
>                  if ( op == RESERVE )
>                      break;
> -                pte = mfn_to_xen_entry(mfn, ai);
> +                pte = mfn_to_xen_entry(_mfn(mfn), ai);
>                  pte.pt.table = 1;
>                  write_pte(&third[third_table_offset(addr)], pte);
>                  break;
> -- 
> 2.11.0
> 

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

Re: [PATCH 09/24] xen/arm: Define mfn_to_page/page_to_mfn in term of __mfn_to_page/__page_to_mfn

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> This is matching the x86 side where the __* version is used if you need
> to override the helpers in source files.
> 
> At the same time, move the non-underscore version at the end of the
> defintion and add a comment to explain them.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/include/asm-arm/mm.h | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index f6915ad882..cc3220a6b7 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -203,10 +203,8 @@ static inline void __iomem *ioremap_wc(paddr_t start, size_t len)
>  })
>  
>  /* Convert between machine frame numbers and page-info structures. */
> -#define mfn_to_page(mfn)  (frame_table + (pfn_to_pdx(mfn) - frametable_base_pdx))
> -#define page_to_mfn(pg)   pdx_to_pfn((unsigned long)((pg) - frame_table) + frametable_base_pdx)
> -#define __page_to_mfn(pg)  page_to_mfn(pg)
> -#define __mfn_to_page(mfn) mfn_to_page(mfn)
> +#define __mfn_to_page(mfn)  (frame_table + (pfn_to_pdx(mfn) - frametable_base_pdx))
> +#define __page_to_mfn(pg)   pdx_to_pfn((unsigned long)((pg) - frame_table) + frametable_base_pdx)
>  
>  /* Convert between machine addresses and page-info structures. */
>  #define maddr_to_page(ma) __mfn_to_page((ma) >> PAGE_SHIFT)
> @@ -264,6 +262,13 @@ static inline int gvirt_to_maddr(vaddr_t va, paddr_t *pa, unsigned int flags)
>  #define virt_to_mfn(va)   (virt_to_maddr(va) >> PAGE_SHIFT)
>  #define mfn_to_virt(mfn)  (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT))
>  
> +/*
> + * We define non-underscored wrappers for above conversion functions.
> + * These are overriden in various source files while underscored version
> + * remain intact.
> + */
> +#define mfn_to_page(mfn)    __mfn_to_page(mfn)
> +#define page_to_mfn(pg)     __page_to_mfn(pg)
>  
>  /* Convert between Xen-heap virtual addresses and page-info structures. */
>  static inline struct page_info *virt_to_page(const void *v)
> -- 
> 2.11.0
> 

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

Re: [PATCH 10/24] xen/arm: domain_build: Replace paddr_to_pfn(virt_to_maddr(.)) by virt_to_mfn(.)

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> paddr_to_pfn(virt_to_maddr(.)) and virt_to_mfn(.) are equivalent.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>  xen/arch/arm/domain_build.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 3abacc0d80..a04c8862db 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1903,7 +1903,7 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
>      rc = map_regions_p2mt(d,
>                            _gfn(paddr_to_pfn(d->arch.efi_acpi_gpa)),
>                            PFN_UP(d->arch.efi_acpi_len),
> -                          _mfn(paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table))),
> +                          _mfn(virt_to_mfn(d->arch.efi_acpi_table)),
>                            p2m_mmio_direct_c);
>      if ( rc != 0 )
>      {
> -- 
> 2.11.0
> 

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

Re: [PATCH 11/24] xen/arm: mm: Replace __va(pfn_to_paddr(...)) by mfn_to_virt

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> __va(pfn_to_paddr(...)) and mfn_to_virt are equivalent.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/mm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 08116679ec..f5df669d92 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -999,7 +999,7 @@ static int create_xen_entries(enum xenmap_operation op,
>  
>          BUG_ON(!xen_second[second_linear_offset(addr)].pt.valid);
>  
> -        third = __va(pfn_to_paddr(xen_second[second_linear_offset(addr)].pt.base));
> +        third = mfn_to_virt(xen_second[second_linear_offset(addr)].pt.base);
>  
>          switch ( op ) {
>              case INSERT:
> -- 
> 2.11.0
> 

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

Re: [PATCH 12/24] xen/arm: Replace DIV_ROUND_UP(..., PAGE_SIZE) by PFN_UP(...)

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> DIV_ROUND_UP(..., PAGE_SIZE) and PFN_UP(...) are equivalent.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/domain_build.c | 4 ++--
>  xen/arch/arm/gic-v2.c       | 2 +-
>  xen/arch/arm/gic-v3.c       | 8 ++++----
>  xen/arch/arm/kernel.c       | 2 +-
>  4 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index a04c8862db..a3243bdb5d 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1008,7 +1008,7 @@ static int map_range_to_domain(const struct dt_device_node *dev,
>      {
>          res = map_regions_p2mt(d,
>                                 _gfn(paddr_to_pfn(addr)),
> -                               DIV_ROUND_UP(len, PAGE_SIZE),
> +                               PFN_UP(len),
>                                 _mfn(paddr_to_pfn(addr)),
>                                 mr_data->p2mt);
>  
> @@ -1545,7 +1545,7 @@ static void acpi_map_other_tables(struct domain *d)
>          size = acpi_gbl_root_table_list.tables[i].length;
>          res = map_regions_p2mt(d,
>                                 _gfn(paddr_to_pfn(addr)),
> -                               DIV_ROUND_UP(size, PAGE_SIZE),
> +                               PFN_UP(size),
>                                 _mfn(paddr_to_pfn(addr)),
>                                 p2m_mmio_direct_c);
>          if ( res )
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index f8124e5e54..0482b1fe32 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -598,7 +598,7 @@ static int gicv2_map_hwdown_extra_mappings(struct domain *d)
>                 v2m_data->spi_start, v2m_data->nr_spis);
>  
>          ret = map_mmio_regions(d, _gfn(paddr_to_pfn(v2m_data->addr)),
> -                               DIV_ROUND_UP(v2m_data->size, PAGE_SIZE),
> +                               PFN_UP(v2m_data->size),
>                                 _mfn(paddr_to_pfn(v2m_data->addr)));
>          if ( ret )
>          {
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index a559e5e260..bb845e955d 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1283,7 +1283,7 @@ static int gicv3_iomem_deny_access(const struct domain *d)
>      unsigned long mfn, nr;
>  
>      mfn = dbase >> PAGE_SHIFT;
> -    nr = DIV_ROUND_UP(SZ_64K, PAGE_SIZE);
> +    nr = PFN_UP(SZ_64K);
>      rc = iomem_deny_access(d, mfn, mfn + nr);
>      if ( rc )
>          return rc;
> @@ -1291,7 +1291,7 @@ static int gicv3_iomem_deny_access(const struct domain *d)
>      for ( i = 0; i < gicv3.rdist_count; i++ )
>      {
>          mfn = gicv3.rdist_regions[i].base >> PAGE_SHIFT;
> -        nr = DIV_ROUND_UP(gicv3.rdist_regions[i].size, PAGE_SIZE);
> +        nr = PFN_UP(gicv3.rdist_regions[i].size);
>          rc = iomem_deny_access(d, mfn, mfn + nr);
>          if ( rc )
>              return rc;
> @@ -1300,7 +1300,7 @@ static int gicv3_iomem_deny_access(const struct domain *d)
>      if ( cbase != INVALID_PADDR )
>      {
>          mfn = cbase >> PAGE_SHIFT;
> -        nr = DIV_ROUND_UP(csize, PAGE_SIZE);
> +        nr = PFN_UP(csize);
>          rc = iomem_deny_access(d, mfn, mfn + nr);
>          if ( rc )
>              return rc;
> @@ -1309,7 +1309,7 @@ static int gicv3_iomem_deny_access(const struct domain *d)
>      if ( vbase != INVALID_PADDR )
>      {
>          mfn = vbase >> PAGE_SHIFT;
> -        nr = DIV_ROUND_UP(csize, PAGE_SIZE);
> +        nr = PFN_UP(csize);
>          return iomem_deny_access(d, mfn, mfn + nr);
>      }
>  
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index e2512c4612..0ed8b6005c 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -312,7 +312,7 @@ static __init int kernel_decompress(struct bootmodule *mod)
>       * Need to free pages after output_size here because they won't be
>       * freed by discard_initial_modules
>       */
> -    i = DIV_ROUND_UP(output_size, PAGE_SIZE);
> +    i = PFN_UP(output_size);
>      for ( ; i < (1 << kernel_order_out); i++ )
>          free_domheap_page(pages + i);
>  
> -- 
> 2.11.0
> 

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

Re: [PATCH 13/24] xen/arm: traps: Replace p2m_lookup(..., ..., NULL) by gfn_to_mfn(..., ...)

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> gfn_to_mfn is a wrapper of p2m_lookup which does not return the
> p2m_type.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/traps.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 6cf9ee7244..ce19021f01 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2481,7 +2481,7 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
>      uint32_t *first = NULL, *second = NULL;
>      mfn_t mfn;
>  
> -    mfn = p2m_lookup(d, _gfn(paddr_to_pfn(ttbr0)), NULL);
> +    mfn = gfn_to_mfn(d, _gfn(paddr_to_pfn(ttbr0)));
>  
>      printk("dom%d VA 0x%08"PRIvaddr"\n", d->domain_id, addr);
>      printk("    TTBCR: 0x%08"PRIregister"\n", ttbcr);
> @@ -2513,7 +2513,7 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
>            (first[offset] & 0x2) )
>          goto done;
>  
> -    mfn = p2m_lookup(d, _gfn(paddr_to_pfn(first[offset])), NULL);
> +    mfn = gfn_to_mfn(d, _gfn(paddr_to_pfn(first[offset])));
>  
>      if ( mfn_eq(mfn, INVALID_MFN) )
>      {
> @@ -2619,7 +2619,7 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
>           * with the Stage-2 page table. Walk the Stage-2 PT to check
>           * if the entry exists. If it's the case, return to the guest
>           */
> -        mfn = p2m_lookup(current->domain, _gfn(paddr_to_pfn(gpa)), NULL);
> +        mfn = gfn_to_mfn(current->domain, _gfn(paddr_to_pfn(gpa)));
>          if ( !mfn_eq(mfn, INVALID_MFN) )
>              return;
>      }
> @@ -2759,7 +2759,7 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
>           * with the Stage-2 page table. Walk the Stage-2 PT to check
>           * if the entry exists. If it's the case, return to the guest
>           */
> -        mfn = p2m_lookup(current->domain, _gfn(paddr_to_pfn(info.gpa)), NULL);
> +        mfn = gfn_to_mfn(current->domain, _gfn(paddr_to_pfn(info.gpa)));
>          if ( !mfn_eq(mfn, INVALID_MFN) )
>              return;
>  
> -- 
> 2.11.0
> 

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

Re: [PATCH 14/24] xen/arm: Introduce wrappers for MFN <-> MADDR and GFN <-> GADDR

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> The new wrappers will add more safety when converting an address to a
> frame number (either machine or guest). A follow-up patch will use them
> to simplify the code.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/include/asm-arm/mm.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index cc3220a6b7..a42da20f0a 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -214,6 +214,10 @@ static inline void __iomem *ioremap_wc(paddr_t start, size_t len)
>  #define pfn_to_paddr(pfn) ((paddr_t)(pfn) << PAGE_SHIFT)
>  #define paddr_to_pfn(pa)  ((unsigned long)((pa) >> PAGE_SHIFT))
>  #define paddr_to_pdx(pa)    pfn_to_pdx(paddr_to_pfn(pa))
> +#define gfn_to_gaddr(gfn)   pfn_to_paddr(gfn_x(gfn))
> +#define gaddr_to_gfn(ga)    _gfn(paddr_to_pfn(ga))
> +#define mfn_to_maddr(mfn)   pfn_to_paddr(mfn_x(mfn))
> +#define maddr_to_mfn(ma)    _mfn(paddr_to_pfn(ma))
>  #define vmap_to_mfn(va)     paddr_to_pfn(virt_to_maddr((vaddr_t)va))
>  #define vmap_to_page(va)    mfn_to_page(vmap_to_mfn(va))
>  
> -- 
> 2.11.0
> 

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

Re: [PATCH 15/24] xen/arm: Use the newly introduced MFN <-> MADDR and GFN <-> MADDR helpers

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> Replace the following constructions:
>     - _gfn(paddr_to_pfn(...))   => gaddr_to_gfn(...)
>     - _mfn(paddr_to_pfn(...))   => maddr_to_mfn(...)
>     - pfn_to_paddr(mfn_x(...))  => mfn_to_maddr(...)
>     - pfn_to_paddr(gfn_x(...))  => gfn_to_gaddr(...)
>     - _mfn(... >> PAGE_SHIFT)   => maddr_to_mfn(...)
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Cc: Tamas K Lengyel <tamas@tklengyel.com>
> ---
>  xen/arch/arm/domain_build.c      | 12 ++++++------
>  xen/arch/arm/gic-v2.c            |  4 ++--
>  xen/arch/arm/kernel.c            |  2 +-
>  xen/arch/arm/mem_access.c        | 10 +++++-----
>  xen/arch/arm/mm.c                | 14 +++++++-------
>  xen/arch/arm/p2m.c               | 10 +++++-----
>  xen/arch/arm/platforms/exynos5.c |  8 ++++----
>  xen/arch/arm/platforms/omap5.c   | 16 ++++++++--------
>  xen/arch/arm/traps.c             | 14 +++++++-------
>  xen/arch/arm/vgic-v2.c           |  4 ++--
>  10 files changed, 47 insertions(+), 47 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index a3243bdb5d..c6776d76fc 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1007,9 +1007,9 @@ static int map_range_to_domain(const struct dt_device_node *dev,
>      if ( need_mapping )
>      {
>          res = map_regions_p2mt(d,
> -                               _gfn(paddr_to_pfn(addr)),
> +                               gaddr_to_gfn(addr),
>                                 PFN_UP(len),
> -                               _mfn(paddr_to_pfn(addr)),
> +                               maddr_to_mfn(addr),
>                                 mr_data->p2mt);
>  
>          if ( res < 0 )
> @@ -1544,9 +1544,9 @@ static void acpi_map_other_tables(struct domain *d)
>          addr = acpi_gbl_root_table_list.tables[i].address;
>          size = acpi_gbl_root_table_list.tables[i].length;
>          res = map_regions_p2mt(d,
> -                               _gfn(paddr_to_pfn(addr)),
> +                               gaddr_to_gfn(addr),
>                                 PFN_UP(size),
> -                               _mfn(paddr_to_pfn(addr)),
> +                               maddr_to_mfn(addr),
>                                 p2m_mmio_direct_c);
>          if ( res )
>          {
> @@ -1901,7 +1901,7 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
>  
>      /* Map the EFI and ACPI tables to Dom0 */
>      rc = map_regions_p2mt(d,
> -                          _gfn(paddr_to_pfn(d->arch.efi_acpi_gpa)),
> +                          gaddr_to_gfn(d->arch.efi_acpi_gpa),
>                            PFN_UP(d->arch.efi_acpi_len),
>                            _mfn(virt_to_mfn(d->arch.efi_acpi_table)),
>                            p2m_mmio_direct_c);
> @@ -2014,7 +2014,7 @@ static void initrd_load(struct kernel_info *kinfo)
>              return;
>          }
>  
> -        dst = map_domain_page(_mfn(paddr_to_pfn(ma)));
> +        dst = map_domain_page(maddr_to_mfn(ma));
>  
>          copy_from_paddr(dst + s, paddr + offs, l);
>  
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 0482b1fe32..5bf7d35a7e 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -597,9 +597,9 @@ static int gicv2_map_hwdown_extra_mappings(struct domain *d)
>                 d->domain_id, v2m_data->addr, v2m_data->size,
>                 v2m_data->spi_start, v2m_data->nr_spis);
>  
> -        ret = map_mmio_regions(d, _gfn(paddr_to_pfn(v2m_data->addr)),
> +        ret = map_mmio_regions(d, gaddr_to_gfn(v2m_data->addr),
>                                 PFN_UP(v2m_data->size),
> -                               _mfn(paddr_to_pfn(v2m_data->addr)));
> +                               maddr_to_mfn(v2m_data->addr));
>          if ( ret )
>          {
>              printk(XENLOG_ERR "GICv2: Map v2m frame to d%d failed.\n",
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index 0ed8b6005c..1b32d55284 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -183,7 +183,7 @@ static void kernel_zimage_load(struct kernel_info *info)
>              return;
>          }
>  
> -        dst = map_domain_page(_mfn(paddr_to_pfn(ma)));
> +        dst = map_domain_page(maddr_to_mfn(ma));
>  
>          copy_from_paddr(dst + s, paddr + offs, l);
>  
> diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
> index 04b1506b00..bcf49f5c15 100644
> --- a/xen/arch/arm/mem_access.c
> +++ b/xen/arch/arm/mem_access.c
> @@ -113,7 +113,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag,
>      if ( rc < 0 )
>          goto err;
>  
> -    gfn = _gfn(paddr_to_pfn(ipa));
> +    gfn = gaddr_to_gfn(ipa);
>  
>      /*
>       * We do this first as this is faster in the default case when no
> @@ -203,7 +203,7 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec)
>      if ( !p2m->mem_access_enabled )
>          return true;
>  
> -    rc = p2m_get_mem_access(v->domain, _gfn(paddr_to_pfn(gpa)), &xma);
> +    rc = p2m_get_mem_access(v->domain, gaddr_to_gfn(gpa), &xma);
>      if ( rc )
>          return true;
>  
> @@ -245,13 +245,13 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec)
>      /* First, handle rx2rw and n2rwx conversion automatically. */
>      if ( npfec.write_access && xma == XENMEM_access_rx2rw )
>      {
> -        rc = p2m_set_mem_access(v->domain, _gfn(paddr_to_pfn(gpa)), 1,
> +        rc = p2m_set_mem_access(v->domain, gaddr_to_gfn(gpa), 1,
>                                  0, ~0, XENMEM_access_rw, 0);
>          return false;
>      }
>      else if ( xma == XENMEM_access_n2rwx )
>      {
> -        rc = p2m_set_mem_access(v->domain, _gfn(paddr_to_pfn(gpa)), 1,
> +        rc = p2m_set_mem_access(v->domain, gaddr_to_gfn(gpa), 1,
>                                  0, ~0, XENMEM_access_rwx, 0);
>      }
>  
> @@ -273,7 +273,7 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec)
>              {
>                  /* A listener is not required, so clear the access
>                   * restrictions. */
> -                rc = p2m_set_mem_access(v->domain, _gfn(paddr_to_pfn(gpa)), 1,
> +                rc = p2m_set_mem_access(v->domain, gaddr_to_gfn(gpa), 1,
>                                          0, ~0, XENMEM_access_rwx, 0);
>              }
>          }
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index f5df669d92..0014c24ecc 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -315,7 +315,7 @@ static inline lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned attr)
>          break;
>      }
>  
> -    ASSERT(!(pfn_to_paddr(mfn_x(mfn)) & ~PADDR_MASK));
> +    ASSERT(!(mfn_to_maddr(mfn) & ~PADDR_MASK));
>  
>      e.pt.base = mfn_x(mfn);
>  
> @@ -536,8 +536,8 @@ void __init arch_init_memory(void)
>  static inline lpae_t pte_of_xenaddr(vaddr_t va)
>  {
>      paddr_t ma = va + phys_offset;
> -    unsigned long mfn = ma >> PAGE_SHIFT;
> -    return mfn_to_xen_entry(_mfn(mfn), WRITEALLOC);
> +
> +    return mfn_to_xen_entry(maddr_to_mfn(ma), WRITEALLOC);
>  }
>  
>  /* Map the FDT in the early boot page table */
> @@ -646,7 +646,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
>      /* Initialise xen second level entries ... */
>      /* ... Xen's text etc */
>  
> -    pte = mfn_to_xen_entry(_mfn(xen_paddr>>PAGE_SHIFT), WRITEALLOC);
> +    pte = mfn_to_xen_entry(maddr_to_mfn(xen_paddr), WRITEALLOC);
>      pte.pt.xn = 0;/* Contains our text mapping! */
>      xen_second[second_table_offset(XEN_VIRT_START)] = pte;
>  
> @@ -663,7 +663,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
>  
>      /* ... Boot Misc area for xen relocation */
>      dest_va = BOOT_RELOC_VIRT_START;
> -    pte = mfn_to_xen_entry(_mfn(xen_paddr >> PAGE_SHIFT), WRITEALLOC);
> +    pte = mfn_to_xen_entry(maddr_to_mfn(xen_paddr), WRITEALLOC);
>      /* Map the destination in xen_second. */
>      xen_second[second_table_offset(dest_va)] = pte;
>      /* Map the destination in boot_second. */
> @@ -690,11 +690,11 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
>      /* Break up the Xen mapping into 4k pages and protect them separately. */
>      for ( i = 0; i < LPAE_ENTRIES; i++ )
>      {
> -        unsigned long mfn = paddr_to_pfn(xen_paddr) + i;
> +        mfn_t mfn = mfn_add(maddr_to_mfn(xen_paddr), i);
>          unsigned long va = XEN_VIRT_START + (i << PAGE_SHIFT);
>          if ( !is_kernel(va) )
>              break;
> -        pte = mfn_to_xen_entry(_mfn(mfn), WRITEALLOC);
> +        pte = mfn_to_xen_entry(mfn, WRITEALLOC);
>          pte.pt.table = 1; /* 4k mappings always have this bit set */
>          if ( is_kernel_text(va) || is_kernel_inittext(va) )
>          {
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index b7bbea1d81..266d1c3bd6 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -311,7 +311,7 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
>                      p2m_type_t *t, p2m_access_t *a,
>                      unsigned int *page_order)
>  {
> -    paddr_t addr = pfn_to_paddr(gfn_x(gfn));
> +    paddr_t addr = gfn_to_gaddr(gfn);
>      unsigned int level = 0;
>      lpae_t entry, *table;
>      int rc;
> @@ -542,7 +542,7 @@ static lpae_t mfn_to_p2m_entry(mfn_t mfn, p2m_type_t t, p2m_access_t a)
>  
>      p2m_set_permission(&e, t, a);
>  
> -    ASSERT(!(pfn_to_paddr(mfn_x(mfn)) & ~PADDR_MASK));
> +    ASSERT(!(mfn_to_maddr(mfn) & ~PADDR_MASK));
>  
>      e.p2m.base = mfn_x(mfn);
>  
> @@ -803,7 +803,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
>                             p2m_type_t t,
>                             p2m_access_t a)
>  {
> -    paddr_t addr = pfn_to_paddr(gfn_x(sgfn));
> +    paddr_t addr = gfn_to_gaddr(sgfn);
>      unsigned int level = 0;
>      unsigned int target = 3 - (page_order / LPAE_SHIFT);
>      lpae_t *entry, *table, orig_pte;
> @@ -1440,10 +1440,10 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
>      if ( rc )
>          goto err;
>  
> -    if ( !mfn_valid(_mfn(maddr >> PAGE_SHIFT)) )
> +    if ( !mfn_valid(maddr_to_mfn(maddr)) )
>          goto err;
>  
> -    page = mfn_to_page(maddr >> PAGE_SHIFT);
> +    page = mfn_to_page(mfn_x(maddr_to_mfn(maddr)));
>
>      ASSERT(page);
>  
>      if ( unlikely(!get_page(page, d)) )
> diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
> index 492cd3e11f..2ae5fa66e0 100644
> --- a/xen/arch/arm/platforms/exynos5.c
> +++ b/xen/arch/arm/platforms/exynos5.c
> @@ -82,12 +82,12 @@ static int exynos5_init_time(void)
>  static int exynos5250_specific_mapping(struct domain *d)
>  {
>      /* Map the chip ID */
> -    map_mmio_regions(d, _gfn(paddr_to_pfn(EXYNOS5_PA_CHIPID)), 1,
> -                     _mfn(paddr_to_pfn(EXYNOS5_PA_CHIPID)));
> +    map_mmio_regions(d, gaddr_to_gfn(EXYNOS5_PA_CHIPID), 1,
> +                     maddr_to_mfn(EXYNOS5_PA_CHIPID));
>  
>      /* Map the PWM region */
> -    map_mmio_regions(d, _gfn(paddr_to_pfn(EXYNOS5_PA_TIMER)), 2,
> -                     _mfn(paddr_to_pfn(EXYNOS5_PA_TIMER)));
> +    map_mmio_regions(d, gaddr_to_gfn(EXYNOS5_PA_TIMER), 2,
> +                     maddr_to_mfn(EXYNOS5_PA_TIMER));
>  
>      return 0;
>  }
> diff --git a/xen/arch/arm/platforms/omap5.c b/xen/arch/arm/platforms/omap5.c
> index eadc4f8382..1e1f9fa970 100644
> --- a/xen/arch/arm/platforms/omap5.c
> +++ b/xen/arch/arm/platforms/omap5.c
> @@ -101,20 +101,20 @@ static int omap5_init_time(void)
>  static int omap5_specific_mapping(struct domain *d)
>  {
>      /* Map the PRM module */
> -    map_mmio_regions(d, _gfn(paddr_to_pfn(OMAP5_PRM_BASE)), 2,
> -                     _mfn(paddr_to_pfn(OMAP5_PRM_BASE)));
> +    map_mmio_regions(d, gaddr_to_gfn(OMAP5_PRM_BASE), 2,
> +                     maddr_to_mfn(OMAP5_PRM_BASE));
>  
>      /* Map the PRM_MPU */
> -    map_mmio_regions(d, _gfn(paddr_to_pfn(OMAP5_PRCM_MPU_BASE)), 1,
> -                     _mfn(paddr_to_pfn(OMAP5_PRCM_MPU_BASE)));
> +    map_mmio_regions(d, gaddr_to_gfn(OMAP5_PRCM_MPU_BASE), 1,
> +                     maddr_to_mfn(OMAP5_PRCM_MPU_BASE));
>  
>      /* Map the Wakeup Gen */
> -    map_mmio_regions(d, _gfn(paddr_to_pfn(OMAP5_WKUPGEN_BASE)), 1,
> -                     _mfn(paddr_to_pfn(OMAP5_WKUPGEN_BASE)));
> +    map_mmio_regions(d, gaddr_to_gfn(OMAP5_WKUPGEN_BASE), 1,
> +                     maddr_to_mfn(OMAP5_WKUPGEN_BASE));
>  
>      /* Map the on-chip SRAM */
> -    map_mmio_regions(d, _gfn(paddr_to_pfn(OMAP5_SRAM_PA)), 32,
> -                     _mfn(paddr_to_pfn(OMAP5_SRAM_PA)));
> +    map_mmio_regions(d, gaddr_to_gfn(OMAP5_SRAM_PA), 32,
> +                     maddr_to_mfn(OMAP5_SRAM_PA));
>  
>      return 0;
>  }
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index ce19021f01..c07999b518 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2481,12 +2481,12 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
>      uint32_t *first = NULL, *second = NULL;
>      mfn_t mfn;
>  
> -    mfn = gfn_to_mfn(d, _gfn(paddr_to_pfn(ttbr0)));
> +    mfn = gfn_to_mfn(d, gaddr_to_gfn(ttbr0));
>  
>      printk("dom%d VA 0x%08"PRIvaddr"\n", d->domain_id, addr);
>      printk("    TTBCR: 0x%08"PRIregister"\n", ttbcr);
>      printk("    TTBR0: 0x%016"PRIx64" = 0x%"PRIpaddr"\n",
> -           ttbr0, pfn_to_paddr(mfn_x(mfn)));
> +           ttbr0, mfn_to_maddr(mfn));
>  
>      if ( ttbcr & TTBCR_EAE )
>      {
> @@ -2508,12 +2508,12 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
>  
>      offset = addr >> (12+8);
>      printk("1ST[0x%"PRIx32"] (0x%"PRIpaddr") = 0x%08"PRIx32"\n",
> -           offset, pfn_to_paddr(mfn_x(mfn)), first[offset]);
> +           offset, mfn_to_maddr(mfn), first[offset]);
>      if ( !(first[offset] & 0x1) ||
>            (first[offset] & 0x2) )
>          goto done;
>  
> -    mfn = gfn_to_mfn(d, _gfn(paddr_to_pfn(first[offset])));
> +    mfn = gfn_to_mfn(d, gaddr_to_gfn(first[offset]));
>  
>      if ( mfn_eq(mfn, INVALID_MFN) )
>      {
> @@ -2523,7 +2523,7 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
>      second = map_domain_page(mfn);
>      offset = (addr >> 12) & 0x3FF;
>      printk("2ND[0x%"PRIx32"] (0x%"PRIpaddr") = 0x%08"PRIx32"\n",
> -           offset, pfn_to_paddr(mfn_x(mfn)), second[offset]);
> +           offset, mfn_to_maddr(mfn), second[offset]);
>  
>  done:
>      if (second) unmap_domain_page(second);
> @@ -2759,11 +2759,11 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
>           * with the Stage-2 page table. Walk the Stage-2 PT to check
>           * if the entry exists. If it's the case, return to the guest
>           */
> -        mfn = gfn_to_mfn(current->domain, _gfn(paddr_to_pfn(info.gpa)));
> +        mfn = gfn_to_mfn(current->domain, gaddr_to_gfn(info.gpa));
>          if ( !mfn_eq(mfn, INVALID_MFN) )
>              return;
>  
> -        if ( try_map_mmio(_gfn(paddr_to_pfn(info.gpa))) )
> +        if ( try_map_mmio(gaddr_to_gfn(info.gpa)) )
>              return;
>  
>          break;
> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
> index dc9f95b948..e5cfa33d8a 100644
> --- a/xen/arch/arm/vgic-v2.c
> +++ b/xen/arch/arm/vgic-v2.c
> @@ -686,8 +686,8 @@ static int vgic_v2_domain_init(struct domain *d)
>       * Map the gic virtual cpu interface in the gic cpu interface
>       * region of the guest.
>       */
> -    ret = map_mmio_regions(d, _gfn(paddr_to_pfn(cbase)), csize / PAGE_SIZE,
> -                           _mfn(paddr_to_pfn(vbase)));
> +    ret = map_mmio_regions(d, gaddr_to_gfn(cbase), csize / PAGE_SIZE,
> +                           maddr_to_mfn(vbase));
>      if ( ret )
>          return ret;
>  
> -- 
> 2.11.0
> 

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

Re: [PATCH 16/24] xen/arm: mm: Use typesafe mfn for xenheap_mfn_*

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> Add more safety when using xenheap_mfn_*.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/mm.c        | 16 ++++++++--------
>  xen/arch/arm/setup.c     | 18 +++++++++---------
>  xen/include/asm-arm/mm.h | 11 ++++++-----
>  3 files changed, 23 insertions(+), 22 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 0014c24ecc..fb01f01879 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -138,8 +138,8 @@ uint64_t init_ttbr;
>  static paddr_t phys_offset;
>  
>  /* Limits of the Xen heap */
> -unsigned long xenheap_mfn_start __read_mostly = ~0UL;
> -unsigned long xenheap_mfn_end __read_mostly;
> +mfn_t xenheap_mfn_start __read_mostly = INVALID_MFN;
> +mfn_t xenheap_mfn_end __read_mostly;

The patch is fine, but given that xenheap_mfn_end is unused on arm64, I
would like to make it arm32 only.


>  vaddr_t xenheap_virt_end __read_mostly;
>  #ifdef CONFIG_ARM_64
>  vaddr_t xenheap_virt_start __read_mostly;
> @@ -801,8 +801,8 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
>  
>      /* Record where the xenheap is, for translation routines. */
>      xenheap_virt_end = XENHEAP_VIRT_START + nr_mfns * PAGE_SIZE;
> -    xenheap_mfn_start = base_mfn;
> -    xenheap_mfn_end = base_mfn + nr_mfns;
> +    xenheap_mfn_start = _mfn(base_mfn);
> +    xenheap_mfn_end = _mfn(base_mfn + nr_mfns);
>  }
>  #else /* CONFIG_ARM_64 */
>  void __init setup_xenheap_mappings(unsigned long base_mfn,
> @@ -816,16 +816,16 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
>      mfn = base_mfn & ~((FIRST_SIZE>>PAGE_SHIFT)-1);
>  
>      /* First call sets the xenheap physical and virtual offset. */
> -    if ( xenheap_mfn_start == ~0UL )
> +    if ( mfn_eq(xenheap_mfn_start, INVALID_MFN) )
>      {
> -        xenheap_mfn_start = base_mfn;
> +        xenheap_mfn_start = _mfn(base_mfn);
>          xenheap_virt_start = DIRECTMAP_VIRT_START +
>              (base_mfn - mfn) * PAGE_SIZE;
>      }
>  
> -    if ( base_mfn < xenheap_mfn_start )
> +    if ( base_mfn < mfn_x(xenheap_mfn_start) )
>          panic("cannot add xenheap mapping at %lx below heap start %lx",
> -              base_mfn, xenheap_mfn_start);
> +              base_mfn, mfn_x(xenheap_mfn_start));
>  
>      end_mfn = base_mfn + nr_mfns;
>  
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index ab4d8e4218..3b34855668 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -555,8 +555,8 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
>       * and enough mapped pages for copying the DTB.
>       */
>      dtb_pages = (dtb_size + PAGE_SIZE-1) >> PAGE_SHIFT;
> -    boot_mfn_start = xenheap_mfn_end - dtb_pages - 1;
> -    boot_mfn_end = xenheap_mfn_end;
> +    boot_mfn_start = mfn_x(xenheap_mfn_end) - dtb_pages - 1;
> +    boot_mfn_end = mfn_x(xenheap_mfn_end);
>  
>      init_boot_pages(pfn_to_paddr(boot_mfn_start), pfn_to_paddr(boot_mfn_end));
>  
> @@ -591,11 +591,11 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
>                  e = bank_end;
>  
>              /* Avoid the xenheap */
> -            if ( s < pfn_to_paddr(xenheap_mfn_start+xenheap_pages)
> -                 && pfn_to_paddr(xenheap_mfn_start) < e )
> +            if ( s < mfn_to_maddr(mfn_add(xenheap_mfn_start, xenheap_pages))
> +                 && mfn_to_maddr(xenheap_mfn_start) < e )
>              {
> -                e = pfn_to_paddr(xenheap_mfn_start);
> -                n = pfn_to_paddr(xenheap_mfn_start+xenheap_pages);
> +                e = mfn_to_maddr(xenheap_mfn_start);
> +                n = mfn_to_maddr(mfn_add(xenheap_mfn_start, xenheap_pages));
>              }
>  
>              dt_unreserved_regions(s, e, init_boot_pages, 0);
> @@ -610,7 +610,7 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
>  
>      /* Add xenheap memory that was not already added to the boot
>         allocator. */
> -    init_xenheap_pages(pfn_to_paddr(xenheap_mfn_start),
> +    init_xenheap_pages(mfn_to_maddr(xenheap_mfn_start),
>                         pfn_to_paddr(boot_mfn_start));
>  }
>  #else /* CONFIG_ARM_64 */
> @@ -662,8 +662,8 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
>      total_pages += ram_size >> PAGE_SHIFT;
>  
>      xenheap_virt_end = XENHEAP_VIRT_START + ram_end - ram_start;
> -    xenheap_mfn_start = ram_start >> PAGE_SHIFT;
> -    xenheap_mfn_end = ram_end >> PAGE_SHIFT;
> +    xenheap_mfn_start = maddr_to_mfn(ram_start);
> +    xenheap_mfn_end = maddr_to_mfn(ram_end);
>  
>      /*
>       * Need enough mapped pages for copying the DTB.
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index a42da20f0a..3dab6dc9a1 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -115,7 +115,7 @@ struct page_info
>  #define PGC_count_width   PG_shift(9)
>  #define PGC_count_mask    ((1UL<<PGC_count_width)-1)
>  
> -extern unsigned long xenheap_mfn_start, xenheap_mfn_end;
> +extern mfn_t xenheap_mfn_start, xenheap_mfn_end;
>  extern vaddr_t xenheap_virt_end;
>  #ifdef CONFIG_ARM_64
>  extern vaddr_t xenheap_virt_start;
> @@ -125,7 +125,8 @@ extern vaddr_t xenheap_virt_start;
>  #define is_xen_heap_page(page) is_xen_heap_mfn(page_to_mfn(page))
>  #define is_xen_heap_mfn(mfn) ({                                 \
>      unsigned long _mfn = (mfn);                                 \
> -    (_mfn >= xenheap_mfn_start && _mfn < xenheap_mfn_end);      \
> +    (_mfn >= mfn_x(xenheap_mfn_start) &&                        \
> +     _mfn < mfn_x(xenheap_mfn_end));                            \

Do you think that mfn_less_than() and mfn_greater_than() would be helpful?


>  })
>  #else
>  #define is_xen_heap_page(page) ((page)->count_info & PGC_xen_heap)
> @@ -235,7 +236,7 @@ static inline paddr_t __virt_to_maddr(vaddr_t va)
>  static inline void *maddr_to_virt(paddr_t ma)
>  {
>      ASSERT(is_xen_heap_mfn(ma >> PAGE_SHIFT));
> -    ma -= pfn_to_paddr(xenheap_mfn_start);
> +    ma -= mfn_to_maddr(xenheap_mfn_start);
>      return (void *)(unsigned long) ma + XENHEAP_VIRT_START;
>  }
>  #else
> @@ -243,7 +244,7 @@ static inline void *maddr_to_virt(paddr_t ma)
>  {
>      ASSERT(pfn_to_pdx(ma >> PAGE_SHIFT) < (DIRECTMAP_SIZE >> PAGE_SHIFT));
>      return (void *)(XENHEAP_VIRT_START -
> -                    pfn_to_paddr(xenheap_mfn_start) +
> +                    mfn_to_maddr(xenheap_mfn_start) +
>                      ((ma & ma_va_bottom_mask) |
>                       ((ma & ma_top_mask) >> pfn_pdx_hole_shift)));
>  }
> @@ -284,7 +285,7 @@ static inline struct page_info *virt_to_page(const void *v)
>      ASSERT(va < xenheap_virt_end);
>  
>      pdx = (va - XENHEAP_VIRT_START) >> PAGE_SHIFT;
> -    pdx += pfn_to_pdx(xenheap_mfn_start);
> +    pdx += pfn_to_pdx(mfn_x(xenheap_mfn_start));
>      return frame_table + pdx - frametable_base_pdx;
>  }
>  
> -- 
> 2.11.0
> 

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

Re: [PATCH 17/24] xen/arm: mm: Use typesafe MFN in set_fixmap

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>  xen/arch/arm/acpi/lib.c           | 4 ++--
>  xen/arch/arm/kernel.c             | 4 +---
>  xen/arch/arm/mm.c                 | 4 ++--
>  xen/arch/arm/platforms/vexpress.c | 2 +-
>  xen/drivers/video/arm_hdlcd.c     | 2 +-
>  xen/include/asm-arm/mm.h          | 2 +-
>  6 files changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/arm/acpi/lib.c b/xen/arch/arm/acpi/lib.c
> index 9bd769cff6..70131b0736 100644
> --- a/xen/arch/arm/acpi/lib.c
> +++ b/xen/arch/arm/acpi/lib.c
> @@ -31,7 +31,7 @@ char *__acpi_map_table(paddr_t phys, unsigned long size)
>  
>      offset = phys & (PAGE_SIZE - 1);
>      mapped_size = PAGE_SIZE - offset;
> -    set_fixmap(FIXMAP_ACPI_BEGIN, phys >> PAGE_SHIFT, PAGE_HYPERVISOR);
> +    set_fixmap(FIXMAP_ACPI_BEGIN, maddr_to_mfn(phys), PAGE_HYPERVISOR);
>      base = FIXMAP_ADDR(FIXMAP_ACPI_BEGIN);
>  
>      /* Most cases can be covered by the below. */
> @@ -41,7 +41,7 @@ char *__acpi_map_table(paddr_t phys, unsigned long size)
>          if ( ++idx > FIXMAP_ACPI_END )
>              return NULL;    /* cannot handle this */
>          phys += PAGE_SIZE;
> -        set_fixmap(idx, phys >> PAGE_SHIFT, PAGE_HYPERVISOR);
> +        set_fixmap(idx, maddr_to_mfn(phys), PAGE_HYPERVISOR);
>          mapped_size += PAGE_SIZE;
>      }
>  
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index 1b32d55284..7403ec0c0e 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -49,14 +49,12 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len)
>      void *src = (void *)FIXMAP_ADDR(FIXMAP_MISC);
>  
>      while (len) {
> -        paddr_t p;
>          unsigned long l, s;
>  
> -        p = paddr >> PAGE_SHIFT;
>          s = paddr & (PAGE_SIZE-1);
>          l = min(PAGE_SIZE - s, len);
>  
> -        set_fixmap(FIXMAP_MISC, p, BUFFERABLE);
> +        set_fixmap(FIXMAP_MISC, maddr_to_mfn(paddr), BUFFERABLE);
>          memcpy(dst, src + s, l);
>          clean_dcache_va_range(dst, l);
>  
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index fb01f01879..8573192e3a 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -323,9 +323,9 @@ static inline lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned attr)
>  }
>  
>  /* Map a 4k page in a fixmap entry */
> -void set_fixmap(unsigned map, unsigned long mfn, unsigned attributes)
> +void set_fixmap(unsigned map, mfn_t mfn, unsigned attributes)
>  {
> -    lpae_t pte = mfn_to_xen_entry(_mfn(mfn), attributes);
> +    lpae_t pte = mfn_to_xen_entry(mfn, attributes);
>      pte.pt.table = 1; /* 4k mappings always have this bit set */
>      pte.pt.xn = 1;
>      write_pte(xen_fixmap + third_table_offset(FIXMAP_ADDR(map)), pte);
> diff --git a/xen/arch/arm/platforms/vexpress.c b/xen/arch/arm/platforms/vexpress.c
> index 8e6a4eaa32..a26ac324ba 100644
> --- a/xen/arch/arm/platforms/vexpress.c
> +++ b/xen/arch/arm/platforms/vexpress.c
> @@ -65,7 +65,7 @@ int vexpress_syscfg(int write, int function, int device, uint32_t *data)
>      uint32_t *syscfg = (uint32_t *) FIXMAP_ADDR(FIXMAP_MISC);
>      int ret = -1;
>  
> -    set_fixmap(FIXMAP_MISC, V2M_SYS_MMIO_BASE >> PAGE_SHIFT, DEV_SHARED);
> +    set_fixmap(FIXMAP_MISC, maddr_to_mfn(V2M_SYS_MMIO_BASE), DEV_SHARED);
>  
>      if ( syscfg[V2M_SYS_CFGCTRL/4] & V2M_SYS_CFG_START )
>          goto out;
> diff --git a/xen/drivers/video/arm_hdlcd.c b/xen/drivers/video/arm_hdlcd.c
> index 8241cae117..3915f731f5 100644
> --- a/xen/drivers/video/arm_hdlcd.c
> +++ b/xen/drivers/video/arm_hdlcd.c
> @@ -227,7 +227,7 @@ void __init video_init(void)
>      /* uses FIXMAP_MISC */
>      set_pixclock(videomode->pixclock);
>  
> -    set_fixmap(FIXMAP_MISC, hdlcd_start >> PAGE_SHIFT, DEV_SHARED);
> +    set_fixmap(FIXMAP_MISC, maddr_to_mfn(hdlcd_start), DEV_SHARED);
>      HDLCD[HDLCD_COMMAND] = 0;
>  
>      HDLCD[HDLCD_LINELENGTH] = videomode->xres * bytes_per_pixel;
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index 3dab6dc9a1..b2e7ea7761 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -176,7 +176,7 @@ extern void setup_xenheap_mappings(unsigned long base_mfn, unsigned long nr_mfns
>  /* Map a frame table to cover physical addresses ps through pe */
>  extern void setup_frametable_mappings(paddr_t ps, paddr_t pe);
>  /* Map a 4k page in a fixmap entry */
> -extern void set_fixmap(unsigned map, unsigned long mfn, unsigned attributes);
> +extern void set_fixmap(unsigned map, mfn_t mfn, unsigned attributes);
>  /* Remove a mapping from a fixmap entry */
>  extern void clear_fixmap(unsigned map);
>  /* map a physical range in virtual memory */
> -- 
> 2.11.0

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

Re: [PATCH 18/24] xen/arm: mm: Use typesafe MFN in dump_pt_walk

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>  xen/arch/arm/mm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 8573192e3a..452c1e26c3 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -181,7 +181,7 @@ void dump_pt_walk(paddr_t ttbr, paddr_t addr,
>                    unsigned int nr_root_tables)
>  {
>      static const char *level_strs[4] = { "0TH", "1ST", "2ND", "3RD" };
> -    const unsigned long root_pfn = paddr_to_pfn(ttbr);
> +    const mfn_t root_mfn = maddr_to_mfn(ttbr);
>      const unsigned int offsets[4] = {
>          zeroeth_table_offset(addr),
>          first_table_offset(addr),
> @@ -215,7 +215,7 @@ void dump_pt_walk(paddr_t ttbr, paddr_t addr,
>      else
>          root_table = 0;
>  
> -    mapping = map_domain_page(_mfn(root_pfn + root_table));
> +    mapping = map_domain_page(mfn_add(root_mfn, root_table));
>  
>      for ( level = root_level; ; level++ )
>      {
> -- 
> 2.11.0
> 

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

Re: [PATCH 19/24] xen/arm: p2m: Redefine mfn_to_page and page_to_mfn to use typesafe

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> The file xen/arch/arm/p2m.c is using typesafe MFN in most of the place.
> This requires caller to mfn_to_page and page_to_mfn to use _mfn/mfn_x.
> 
> To avoid extra _mfn/mfn_x, re-define mfn_to_page and page_to_mfn within
> xen/arch/arm/p2m.c to handle typesafe MFN.

This is the same thing that x86/p2m.c does. At some point one has to
wonder if it makes sense to just bite the bullet and change mfn_to_page
everywhere.


> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/p2m.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 266d1c3bd6..6c1ac70044 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -38,6 +38,12 @@ static unsigned int __read_mostly max_vmid = MAX_VMID_8_BIT;
>  
>  #define P2M_ROOT_PAGES    (1<<P2M_ROOT_ORDER)
>  
> +/* Override macros from asm/mm.h to make them work with mfn_t */
> +#undef mfn_to_page
> +#define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn))
> +#undef page_to_mfn
> +#define page_to_mfn(pg) _mfn(__page_to_mfn(pg))
> +
>  unsigned int __read_mostly p2m_ipa_bits;
>  
>  /* Helpers to lookup the properties of each level */
> @@ -115,7 +121,7 @@ void dump_p2m_lookup(struct domain *d, paddr_t addr)
>      printk("dom%d IPA 0x%"PRIpaddr"\n", d->domain_id, addr);
>  
>      printk("P2M @ %p mfn:0x%lx\n",
> -           p2m->root, page_to_mfn(p2m->root));
> +           p2m->root, mfn_x(page_to_mfn(p2m->root)));

__page_to_mfn(pg) ?

The rest looks good


>      dump_pt_walk(page_to_maddr(p2m->root), addr,
>                   P2M_ROOT_LEVEL, P2M_ROOT_PAGES);
> @@ -591,7 +597,7 @@ static int p2m_create_table(struct p2m_domain *p2m, lpae_t *entry)
>       * The access value does not matter because the hardware will ignore
>       * the permission fields for table entry.
>       */
> -    pte = mfn_to_p2m_entry(_mfn(page_to_mfn(page)), p2m_invalid,
> +    pte = mfn_to_p2m_entry(page_to_mfn(page), p2m_invalid,
>                             p2m->default_access);
>  
>      p2m_write_pte(entry, pte, p2m->clean_pte);
> @@ -650,9 +656,9 @@ static void p2m_put_l3_page(const lpae_t pte)
>       */
>      if ( p2m_is_foreign(pte.p2m.type) )
>      {
> -        unsigned long mfn = pte.p2m.base;
> +        mfn_t mfn = _mfn(pte.p2m.base);
>  
> -        ASSERT(mfn_valid(_mfn(mfn)));
> +        ASSERT(mfn_valid(mfn));
>          put_page(mfn_to_page(mfn));
>      }
>  }
> @@ -702,7 +708,7 @@ static void p2m_free_entry(struct p2m_domain *p2m,
>      mfn = _mfn(entry.p2m.base);
>      ASSERT(mfn_valid(mfn));
>  
> -    pg = mfn_to_page(mfn_x(mfn));
> +    pg = mfn_to_page(mfn);
>  
>      page_list_del(pg, &p2m->pages);
>      free_domheap_page(pg);
> @@ -780,7 +786,7 @@ static bool p2m_split_superpage(struct p2m_domain *p2m, lpae_t *entry,
>  
>      unmap_domain_page(table);
>  
> -    pte = mfn_to_p2m_entry(_mfn(page_to_mfn(page)), p2m_invalid,
> +    pte = mfn_to_p2m_entry(page_to_mfn(page), p2m_invalid,
>                             p2m->default_access);
>  
>      /*
> @@ -1443,7 +1449,7 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
>      if ( !mfn_valid(maddr_to_mfn(maddr)) )
>          goto err;
>  
> -    page = mfn_to_page(mfn_x(maddr_to_mfn(maddr)));
> +    page = mfn_to_page(maddr_to_mfn(maddr));
>      ASSERT(page);
>  
>      if ( unlikely(!get_page(page, d)) )
> -- 
> 2.11.0
> 

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

Re: [PATCH 20/24] xen/arm: mm: Redefine virt_to_mfn to support typesafe

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> The file xen/arch/arm/mm.c is using the typesafe MFN in most of the
> place. This requires all caller of virt_to_mfn to prefixed by _mfn(...).
> 
> To avoid the extra _mfn(...), re-defined virt_to_mfn within arch/arm/mm.c
> to handle typesafe MFN.
> 
> This patch also introduce __virt_to_mfn, so virt_to_mfn can be
> re-defined easily.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/mm.c        | 16 ++++++++++------
>  xen/include/asm-arm/mm.h |  3 ++-
>  2 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 452c1e26c3..2ff1688f3f 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -44,6 +44,10 @@
>  
>  struct domain *dom_xen, *dom_io, *dom_cow;
>  
> +/* Override macros from asm/page.h to make them work with mfn_t */
> +#undef virt_to_mfn
> +#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
> +
>  /* Static start-of-day pagetables that we use before the allocators
>   * are up. These are used by all CPUs during bringup before switching
>   * to the CPUs own pagetables.
> @@ -479,7 +483,7 @@ unsigned long domain_page_map_to_mfn(const void *ptr)
>      unsigned long offset = (va>>THIRD_SHIFT) & LPAE_ENTRY_MASK;
>  
>      if ( va >= VMAP_VIRT_START && va < VMAP_VIRT_END )
> -        return virt_to_mfn(va);
> +        return mfn_x(virt_to_mfn(va));

__virt_to_mfn?


>      ASSERT(slot >= 0 && slot < DOMHEAP_ENTRIES);
>      ASSERT(map[slot].pt.avail != 0);
> @@ -764,7 +768,7 @@ int init_secondary_pagetables(int cpu)
>       * domheap mapping pages. */
>      for ( i = 0; i < DOMHEAP_SECOND_PAGES; i++ )
>      {
> -        pte = mfn_to_xen_entry(_mfn(virt_to_mfn(domheap+i*LPAE_ENTRIES)),
> +        pte = mfn_to_xen_entry(virt_to_mfn(domheap+i*LPAE_ENTRIES),
>                                 WRITEALLOC);
>          pte.pt.table = 1;
>          write_pte(&first[first_table_offset(DOMHEAP_VIRT_START+i*FIRST_SIZE)], pte);
> @@ -961,7 +965,7 @@ static int create_xen_table(lpae_t *entry)
>      if ( p == NULL )
>          return -ENOMEM;
>      clear_page(p);
> -    pte = mfn_to_xen_entry(_mfn(virt_to_mfn(p)), WRITEALLOC);
> +    pte = mfn_to_xen_entry(virt_to_mfn(p), WRITEALLOC);
>      pte.pt.table = 1;
>      write_pte(entry, pte);
>      return 0;
> @@ -1216,7 +1220,7 @@ int xenmem_add_to_physmap_one(
>      unsigned long idx,
>      gfn_t gfn)
>  {
> -    unsigned long mfn = 0;
> +    mfn_t mfn = INVALID_MFN;
>      int rc;
>      p2m_type_t t;
>      struct page_info *page = NULL;
> @@ -1302,7 +1306,7 @@ int xenmem_add_to_physmap_one(
>              return -EINVAL;
>          }
>  
> -        mfn = page_to_mfn(page);
> +        mfn = _mfn(page_to_mfn(page));
>          t = p2m_map_foreign;
>  
>          rcu_unlock_domain(od);
> @@ -1321,7 +1325,7 @@ int xenmem_add_to_physmap_one(
>      }
>  
>      /* Map at new location. */
> -    rc = guest_physmap_add_entry(d, gfn, _mfn(mfn), 0, t);
> +    rc = guest_physmap_add_entry(d, gfn, mfn, 0, t);
>  
>      /* If we fail to add the mapping, we need to drop the reference we
>       * took earlier on foreign pages */
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index b2e7ea7761..6e2b3c7f2b 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -264,7 +264,7 @@ static inline int gvirt_to_maddr(vaddr_t va, paddr_t *pa, unsigned int flags)
>  #define __va(x)             (maddr_to_virt(x))
>  
>  /* Convert between Xen-heap virtual addresses and machine frame numbers. */
> -#define virt_to_mfn(va)   (virt_to_maddr(va) >> PAGE_SHIFT)
> +#define __virt_to_mfn(va) (virt_to_maddr(va) >> PAGE_SHIFT)
>  #define mfn_to_virt(mfn)  (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT))
>  
>  /*
> @@ -274,6 +274,7 @@ static inline int gvirt_to_maddr(vaddr_t va, paddr_t *pa, unsigned int flags)
>   */
>  #define mfn_to_page(mfn)    __mfn_to_page(mfn)
>  #define page_to_mfn(pg)     __page_to_mfn(pg)
> +#define virt_to_mfn(va)     __virt_to_mfn(va)
>  
>  /* Convert between Xen-heap virtual addresses and page-info structures. */
>  static inline struct page_info *virt_to_page(const void *v)
> -- 
> 2.11.0
> 

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

Re: [PATCH 21/24] xen/arm: domain_build: Redefine virt_to_mfn to support typesafe

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> The file xen/arch/arm/domain_build.c is using typesafe MFN in most of
> the place. The only caller to virt_to_mfn is using prefixed with
> _mfn(...).
> 
> To avoid extra _mfn(...), re-define virt_to_mfn within
> arch/arm/domain_build.c to handle typesafe MFN.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/domain_build.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index c6776d76fc..1bec4fa23d 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -45,6 +45,10 @@ struct map_range_data
>      p2m_type_t p2mt;
>  };
>  
> +/* Override macros from asm/page.h to make them work with mfn_t */
> +#undef virt_to_mfn
> +#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
> +
>  //#define DEBUG_11_ALLOCATION
>  #ifdef DEBUG_11_ALLOCATION
>  # define D11PRINT(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
> @@ -1903,7 +1907,7 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
>      rc = map_regions_p2mt(d,
>                            gaddr_to_gfn(d->arch.efi_acpi_gpa),
>                            PFN_UP(d->arch.efi_acpi_len),
> -                          _mfn(virt_to_mfn(d->arch.efi_acpi_table)),
> +                          virt_to_mfn(d->arch.efi_acpi_table),
>                            p2m_mmio_direct_c);
>      if ( rc != 0 )
>      {
> -- 
> 2.11.0
> 

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

Re: [PATCH 22/24] xen/arm: alternative: Redefine virt_to_mfn to support typesafe

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> The file xen/arch/arm/alternative.c is using typesafe MFN in most of
> the place. The only caller to virt_to_mfn is using with _mfn(...).
> 
> To avoid extra _mfn(...), re-define virt_to_mfn within
> xen/arch/arm/alternative.c to handle typesafe MFN.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>  xen/arch/arm/alternative.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
> index 4d7e5b6155..a3bcda3117 100644
> --- a/xen/arch/arm/alternative.c
> +++ b/xen/arch/arm/alternative.c
> @@ -32,6 +32,10 @@
>  #include <asm/insn.h>
>  #include <asm/page.h>
>  
> +/* Override macros from asm/page.h to make them work with mfn_t */
> +#undef virt_to_mfn
> +#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
> +
>  extern const struct alt_instr __alt_instructions[], __alt_instructions_end[];
>  
>  struct alt_region {
> @@ -154,7 +158,7 @@ static int __apply_alternatives_multi_stop(void *unused)
>      {
>          int ret;
>          struct alt_region region;
> -        mfn_t xen_mfn = _mfn(virt_to_mfn(_start));
> +        mfn_t xen_mfn = virt_to_mfn(_start);
>          paddr_t xen_size = _end - _start;
>          unsigned int xen_order = get_order_from_bytes(xen_size);
>          void *xenmap;
> -- 
> 2.11.0
> 

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

Re: [PATCH 23/24] xen/arm: livepatch: Redefine virt_to_mfn to support typesafe

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> The file xen/arch/arm/livepatch.c is using typesafe MFN in most of
> the place. The only caller to virt_to_mfn is using with _mfn(...).
> 
> To avoid extra _mfn(...), re-define virt_to_mfn within
> xen/arch/arm/livepatch.c to handle typesafe MFN.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/livepatch.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
> index de95e54744..3e53524365 100644
> --- a/xen/arch/arm/livepatch.c
> +++ b/xen/arch/arm/livepatch.c
> @@ -12,6 +12,10 @@
>  #include <asm/livepatch.h>
>  #include <asm/mm.h>
>  
> +/* Override macros from asm/page.h to make them work with mfn_t */
> +#undef virt_to_mfn
> +#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
> +
>  void *vmap_of_xen_text;
>  
>  int arch_livepatch_quiesce(void)
> @@ -22,7 +26,7 @@ int arch_livepatch_quiesce(void)
>      if ( vmap_of_xen_text )
>          return -EINVAL;
>  
> -    text_mfn = _mfn(virt_to_mfn(_start));
> +    text_mfn = virt_to_mfn(_start);
>      text_order = get_order_from_bytes(_end - _start);
>  
>      /*
> -- 
> 2.11.0
> 

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

Re: [PATCH 24/24] xen/arm: create_xen_entries: Use typesafe MFN

[Stefano Stabellini]

On Tue, 13 Jun 2017, Julien Grall wrote:
> Add a bit more safety when using create_xen_entries.
> 
> Also when destroying/modifying mapping, the MFN is currently not used.
> Rather than passing _mfn(0) use INVALID_MFN to stay consistent with the
> other usage.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/mm.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 2ff1688f3f..8cb0559972 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -980,7 +980,7 @@ enum xenmap_operation {
>  
>  static int create_xen_entries(enum xenmap_operation op,
>                                unsigned long virt,
> -                              unsigned long mfn,
> +                              mfn_t mfn,
>                                unsigned long nr_mfns,
>                                unsigned int ai)
>  {
> @@ -989,7 +989,7 @@ static int create_xen_entries(enum xenmap_operation op,
>      lpae_t pte;
>      lpae_t *third = NULL;
>  
> -    for(; addr < addr_end; addr += PAGE_SIZE, mfn++)
> +    for(; addr < addr_end; addr += PAGE_SIZE, mfn = mfn_add(mfn, 1))
>      {
>          if ( !xen_second[second_linear_offset(addr)].pt.valid ||
>               !xen_second[second_linear_offset(addr)].pt.table )
> @@ -1010,13 +1010,13 @@ static int create_xen_entries(enum xenmap_operation op,
>              case RESERVE:
>                  if ( third[third_table_offset(addr)].pt.valid )
>                  {
> -                    printk("create_xen_entries: trying to replace an existing mapping addr=%lx mfn=%lx\n",
> -                           addr, mfn);
> +                    printk("create_xen_entries: trying to replace an existing mapping addr=%lx mfn=%"PRI_mfn"\n",
> +                           addr, mfn_x(mfn));
>                      return -EINVAL;
>                  }
>                  if ( op == RESERVE )
>                      break;
> -                pte = mfn_to_xen_entry(_mfn(mfn), ai);
> +                pte = mfn_to_xen_entry(mfn, ai);
>                  pte.pt.table = 1;
>                  write_pte(&third[third_table_offset(addr)], pte);
>                  break;
> @@ -1061,24 +1061,25 @@ int map_pages_to_xen(unsigned long virt,
>                       unsigned long nr_mfns,
>                       unsigned int flags)
>  {
> -    return create_xen_entries(INSERT, virt, mfn, nr_mfns, flags);
> +    return create_xen_entries(INSERT, virt, _mfn(mfn), nr_mfns, flags);
>  }
>  
>  int populate_pt_range(unsigned long virt, unsigned long mfn,
>                        unsigned long nr_mfns)
>  {
> -    return create_xen_entries(RESERVE, virt, mfn, nr_mfns, 0);
> +    return create_xen_entries(RESERVE, virt, _mfn(mfn), nr_mfns, 0);
>  }
>  
>  int destroy_xen_mappings(unsigned long v, unsigned long e)
>  {
> -    return create_xen_entries(REMOVE, v, 0, (e - v) >> PAGE_SHIFT, 0);
> +    return create_xen_entries(REMOVE, v, INVALID_MFN, (e - v) >> PAGE_SHIFT, 0);
>  }
>  
>  int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags)
>  {
>      ASSERT((flags & (PTE_NX | PTE_RO)) == flags);
> -    return create_xen_entries(MODIFY, s, 0, (e - s) >> PAGE_SHIFT, flags);
> +    return create_xen_entries(MODIFY, s, INVALID_MFN, (e - s) >> PAGE_SHIFT,
> +                              flags);
>  }
>  
>  enum mg { mg_clear, mg_ro, mg_rw, mg_rx };
> -- 
> 2.11.0
> 

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

Re: [PATCH 00/24] xen/arm: Extend the usage of typesafe MFN

[Stefano Stabellini]

Hi Julien,

thanks for the series!

I committed patches 2, 4-15, 17,18.

Cheers,

Stefano

On Tue, 13 Jun 2017, Julien Grall wrote:
> Hello all,
> 
> This patch series extend the usage of typesafe MFN in the ARM code. _mfn(...)
> and mfn_x(...) are pushed further down in the call stack.
> 
> Cheers,
> 
> Julien Grall (24):
>   xen/mm: Don't use _{g,m}fn for defining INVALID_{G,M}FN
>   xen/arm: gic-v2: Fix indentation in gicv2_map_hwdom_extra_mappings
>   xen/arm: setup: Remove bogus xenheap_mfn_end in setup_mm for arm64
>   xen/arm: mm: Introduce clear_table and use it
>   xen/arm: mm: Move mfn_to_xen_entry from page.h to mm.c
>   xen/arm: mm: Fix coding style of mfn_to_xen_entry
>   xen/arm: mm: Clean-up mfn_to_xen_entry
>   xen/arm: mm: Use typesafe MFN in mfn_to_xen_entry
>   xen/arm: Define mfn_to_page/page_to_mfn in term of
>     __mfn_to_page/__page_to_mfn
>   xen/arm: domain_build: Replace paddr_to_pfn(virt_to_maddr(.)) by
>     virt_to_mfn(.)
>   xen/arm: mm: Replace __va(pfn_to_paddr(...)) by mfn_to_virt
>   xen/arm: Replace DIV_ROUND_UP(..., PAGE_SIZE) by PFN_UP(...)
>   xen/arm: traps: Replace p2m_lookup(..., ..., NULL) by gfn_to_mfn(...,
>     ...)
>   xen/arm: Introduce wrappers for MFN <-> MADDR and GFN <-> GADDR
>   xen/arm: Use the newly introduced MFN <-> MADDR and GFN <-> MADDR
>     helpers
>   xen/arm: mm: Use typesafe mfn for xenheap_mfn_*
>   xen/arm: mm: Use typesafe MFN in set_fixmap
>   xen/arm: mm: Use typesafe MFN in dump_pt_walk
>   xen/arm: p2m: Redefine mfn_to_page and page_to_mfn to use typesafe
>   xen/arm: mm: Redefine virt_to_mfn to support typesafe
>   xen/arm: domain_build: Redefine virt_to_mfn to support typesafe
>   xen/arm: alternative: Redefine virt_to_mfn to support typesafe
>   xen/arm: livepatch: Redefine virt_to_mfn to support typesafe
>   xen/arm: create_xen_entries: Use typesafe MFN
> 
>  xen/arch/arm/acpi/lib.c           |   4 +-
>  xen/arch/arm/alternative.c        |   6 +-
>  xen/arch/arm/domain_build.c       |  22 ++---
>  xen/arch/arm/gic-v2.c             |   6 +-
>  xen/arch/arm/gic-v3.c             |   8 +-
>  xen/arch/arm/kernel.c             |   8 +-
>  xen/arch/arm/livepatch.c          |   6 +-
>  xen/arch/arm/mem_access.c         |  10 +--
>  xen/arch/arm/mm.c                 | 166 +++++++++++++++++++++++++++-----------
>  xen/arch/arm/p2m.c                |  28 ++++---
>  xen/arch/arm/platforms/exynos5.c  |   8 +-
>  xen/arch/arm/platforms/omap5.c    |  16 ++--
>  xen/arch/arm/platforms/vexpress.c |   2 +-
>  xen/arch/arm/setup.c              |  20 +++--
>  xen/arch/arm/traps.c              |  16 ++--
>  xen/arch/arm/vgic-v2.c            |   4 +-
>  xen/drivers/video/arm_hdlcd.c     |   2 +-
>  xen/include/asm-arm/mm.h          |  33 +++++---
>  xen/include/asm-arm/page.h        |  65 ---------------
>  xen/include/xen/mm.h              |   4 +-
>  20 files changed, 235 insertions(+), 199 deletions(-)
> 
> -- 
> 2.11.0
> 

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

Re: [PATCH 03/24] xen/arm: setup: Remove bogus xenheap_mfn_end in setup_mm for arm64

[Julien Grall]

Hi Stefano,

On 15/06/2017 23:28, Stefano Stabellini wrote:
> On Tue, 13 Jun 2017, Julien Grall wrote:
>> xenheap_mfn_end is storing an MFN and not a physical address. Thankfully
>> xenheap_mfn_end is not used in the arm64 code. So drop it.
>
> That's fine, but in that case I would prefer to move the definition of
> xenheap_mfn_end under #ifdef CONFIG_ARM_32. In fact, there is another
> assignment of xenheap_mfn_end few lines below in the arm64 version of
> setup_mm: don't we need to remove that too?

The other xenheap_mfn_end contains valid mfn that point to the end and I 
didn't want to #ifdef it because:
	1) It complexify the code
	2) All regions should be bound with start/end to simplify potential use.

Cheers,

-- 
Julien Grall

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

Re: [PATCH 04/24] xen/arm: mm: Introduce clear_table and use it

[Julien Grall]

Hi Stefano,

On 15/06/2017 23:31, Stefano Stabellini wrote:
> On Tue, 13 Jun 2017, Julien Grall wrote:
>> Add a new helper clear_table to clear a page table entry and invalidate
>> the cache.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> ---
>>  xen/arch/arm/mm.c | 22 ++++++++++++----------
>>  1 file changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
>> index 082c872c72..b4ff777b55 100644
>> --- a/xen/arch/arm/mm.c
>> +++ b/xen/arch/arm/mm.c
>> @@ -529,6 +529,13 @@ void __init remove_early_mappings(void)
>>
>>  extern void relocate_xen(uint64_t ttbr, void *src, void *dst, size_t len);
>>
>> +/* Clear a translation table and clean & invalidate the cache */
>> +static void clear_table(void *table)
>
> This could be a static inline. In any case:

I see you already committed. I see very limited use here for static 
inline as this function might quite big (clear_page and 
clean_and_invalidate_dcache_va_range). The compiler is in better 
position to decide what to do than us.

IHMO, static inline should only be used in header and very small 
function (basically just checking a condition).

Cheers,

-- 
Julien Grall

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

Re: [PATCH 16/24] xen/arm: mm: Use typesafe mfn for xenheap_mfn_*

[Julien Grall]

Hi Stefano,

On 16/06/2017 00:12, Stefano Stabellini wrote:
> On Tue, 13 Jun 2017, Julien Grall wrote:
>> Add more safety when using xenheap_mfn_*.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> ---
>>  xen/arch/arm/mm.c        | 16 ++++++++--------
>>  xen/arch/arm/setup.c     | 18 +++++++++---------
>>  xen/include/asm-arm/mm.h | 11 ++++++-----
>>  3 files changed, 23 insertions(+), 22 deletions(-)
>>
>> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
>> index 0014c24ecc..fb01f01879 100644
>> --- a/xen/arch/arm/mm.c
>> +++ b/xen/arch/arm/mm.c
>> @@ -138,8 +138,8 @@ uint64_t init_ttbr;
>>  static paddr_t phys_offset;
>>
>>  /* Limits of the Xen heap */
>> -unsigned long xenheap_mfn_start __read_mostly = ~0UL;
>> -unsigned long xenheap_mfn_end __read_mostly;
>> +mfn_t xenheap_mfn_start __read_mostly = INVALID_MFN;
>> +mfn_t xenheap_mfn_end __read_mostly;
>
> The patch is fine, but given that xenheap_mfn_end is unused on arm64, I
> would like to make it arm32 only.

See my answer on patch #3, I would like to limit the #ifdefery and it is 
better to bound a region with start/end.

Hopefully, at some point we could make the xenheap code very similar on 
both ARM64 and ARM32.

Cheers,

-- 
Julien Grall

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

Re: [PATCH 19/24] xen/arm: p2m: Redefine mfn_to_page and page_to_mfn to use typesafe

[Julien Grall]

Hi Stefano,

On 16/06/2017 00:37, Stefano Stabellini wrote:
> On Tue, 13 Jun 2017, Julien Grall wrote:
>> The file xen/arch/arm/p2m.c is using typesafe MFN in most of the place.
>> This requires caller to mfn_to_page and page_to_mfn to use _mfn/mfn_x.
>>
>> To avoid extra _mfn/mfn_x, re-define mfn_to_page and page_to_mfn within
>> xen/arch/arm/p2m.c to handle typesafe MFN.
>
> This is the same thing that x86/p2m.c does. At some point one has to
> wonder if it makes sense to just bite the bullet and change mfn_to_page
> everywhere.

The point of those re-definition is to split the work in smaller series 
and between multiple people rather than requiring one to do the full 
clean-up. It is quite a boring task but not as mechanical as you think 
because often you would need to rework a bit the code around.

When we end-up to have the source code using typesafe MFN then we can 
have a patch dropping the definition. Ideally all the mfn_to_* and 
*_to_mfn should be typesafe. But this is a longer term goal and not my 
plan here.

Cheers,

-- 
Julien Grall

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

Re: [PATCH 20/24] xen/arm: mm: Redefine virt_to_mfn to support typesafe

[Julien Grall]

Hi Stefano,

On 16/06/2017 00:44, Stefano Stabellini wrote:
> On Tue, 13 Jun 2017, Julien Grall wrote:
>> The file xen/arch/arm/mm.c is using the typesafe MFN in most of the
>> place. This requires all caller of virt_to_mfn to prefixed by _mfn(...).
>>
>> To avoid the extra _mfn(...), re-defined virt_to_mfn within arch/arm/mm.c
>> to handle typesafe MFN.
>>
>> This patch also introduce __virt_to_mfn, so virt_to_mfn can be
>> re-defined easily.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> ---
>>  xen/arch/arm/mm.c        | 16 ++++++++++------
>>  xen/include/asm-arm/mm.h |  3 ++-
>>  2 files changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
>> index 452c1e26c3..2ff1688f3f 100644
>> --- a/xen/arch/arm/mm.c
>> +++ b/xen/arch/arm/mm.c
>> @@ -44,6 +44,10 @@
>>
>>  struct domain *dom_xen, *dom_io, *dom_cow;
>>
>> +/* Override macros from asm/page.h to make them work with mfn_t */
>> +#undef virt_to_mfn
>> +#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
>> +
>>  /* Static start-of-day pagetables that we use before the allocators
>>   * are up. These are used by all CPUs during bringup before switching
>>   * to the CPUs own pagetables.
>> @@ -479,7 +483,7 @@ unsigned long domain_page_map_to_mfn(const void *ptr)
>>      unsigned long offset = (va>>THIRD_SHIFT) & LPAE_ENTRY_MASK;
>>
>>      if ( va >= VMAP_VIRT_START && va < VMAP_VIRT_END )
>> -        return virt_to_mfn(va);
>> +        return mfn_x(virt_to_mfn(va));
>
> __virt_to_mfn?

Ok.

Cheers,

-- 
Julien Grall

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

Re: [PATCH 03/24] xen/arm: setup: Remove bogus xenheap_mfn_end in setup_mm for arm64

[Stefano Stabellini]

On Fri, 16 Jun 2017, Julien Grall wrote:
> Hi Stefano,
> 
> On 15/06/2017 23:28, Stefano Stabellini wrote:
> > On Tue, 13 Jun 2017, Julien Grall wrote:
> > > xenheap_mfn_end is storing an MFN and not a physical address. Thankfully
> > > xenheap_mfn_end is not used in the arm64 code. So drop it.
> > 
> > That's fine, but in that case I would prefer to move the definition of
> > xenheap_mfn_end under #ifdef CONFIG_ARM_32. In fact, there is another
> > assignment of xenheap_mfn_end few lines below in the arm64 version of
> > setup_mm: don't we need to remove that too?
> 
> The other xenheap_mfn_end contains valid mfn that point to the end and I
> didn't want to #ifdef it because:
> 	1) It complexify the code
> 	2) All regions should be bound with start/end to simplify potential
> use.

I am only suggesting to move its definition and declaration under #ifdef
CONFIG_ARM_32 in xen/include/asm-arm/mm.h and xen/arch/arm/mm.c.

After that, all users of xenheap_mfn_end are already #ifdef
CONFIG_ARM_32, except for xen/arch/arm/setup.c:setup_mm. The setup_mm
under #ifdef CONFIG_ARM_32 will be fine. The setup_mm under
#ifdef CONFIG_ARM_64, doesn't need xenheap_mfn_end and we could just
remove it from there.

Does it make sense? Am I missing something?

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

Re: [PATCH 16/24] xen/arm: mm: Use typesafe mfn for xenheap_mfn_*

[Julien Grall]

Hi Stefano,

On 06/16/2017 12:12 AM, Stefano Stabellini wrote:
> On Tue, 13 Jun 2017, Julien Grall wrote:
>>        * Need enough mapped pages for copying the DTB.
>> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
>> index a42da20f0a..3dab6dc9a1 100644
>> --- a/xen/include/asm-arm/mm.h
>> +++ b/xen/include/asm-arm/mm.h
>> @@ -115,7 +115,7 @@ struct page_info
>>   #define PGC_count_width   PG_shift(9)
>>   #define PGC_count_mask    ((1UL<<PGC_count_width)-1)
>>   
>> -extern unsigned long xenheap_mfn_start, xenheap_mfn_end;
>> +extern mfn_t xenheap_mfn_start, xenheap_mfn_end;
>>   extern vaddr_t xenheap_virt_end;
>>   #ifdef CONFIG_ARM_64
>>   extern vaddr_t xenheap_virt_start;
>> @@ -125,7 +125,8 @@ extern vaddr_t xenheap_virt_start;
>>   #define is_xen_heap_page(page) is_xen_heap_mfn(page_to_mfn(page))
>>   #define is_xen_heap_mfn(mfn) ({                                 \
>>       unsigned long _mfn = (mfn);                                 \
>> -    (_mfn >= xenheap_mfn_start && _mfn < xenheap_mfn_end);      \
>> +    (_mfn >= mfn_x(xenheap_mfn_start) &&                        \
>> +     _mfn < mfn_x(xenheap_mfn_end));                            \
> 
> Do you think that mfn_less_than() and mfn_greater_than() would be helpful?

I forgot to answer this one. I have looked at the place comparing (other 
than = and !=) typesafe MFN and I haven't found many. So I didn't see 
the need to introduce helpers for that.

If notice an increase of them, we can decide to introduce one. It will 
be easy to find as they would have explicit mfn_x in the code.

Cheers,

-- 
Julien Grall

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

Re: [PATCH 03/24] xen/arm: setup: Remove bogus xenheap_mfn_end in setup_mm for arm64

[Julien Grall]

Hi Stefano,

On 06/16/2017 06:33 PM, Stefano Stabellini wrote:
> On Fri, 16 Jun 2017, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 15/06/2017 23:28, Stefano Stabellini wrote:
>>> On Tue, 13 Jun 2017, Julien Grall wrote:
>>>> xenheap_mfn_end is storing an MFN and not a physical address. Thankfully
>>>> xenheap_mfn_end is not used in the arm64 code. So drop it.
>>>
>>> That's fine, but in that case I would prefer to move the definition of
>>> xenheap_mfn_end under #ifdef CONFIG_ARM_32. In fact, there is another
>>> assignment of xenheap_mfn_end few lines below in the arm64 version of
>>> setup_mm: don't we need to remove that too?
>>
>> The other xenheap_mfn_end contains valid mfn that point to the end and I
>> didn't want to #ifdef it because:
>> 	1) It complexify the code
>> 	2) All regions should be bound with start/end to simplify potential
>> use.
> 
> I am only suggesting to move its definition and declaration under #ifdef
> CONFIG_ARM_32 in xen/include/asm-arm/mm.h and xen/arch/arm/mm.c.
> 
> After that, all users of xenheap_mfn_end are already #ifdef
> CONFIG_ARM_32, except for xen/arch/arm/setup.c:setup_mm. The setup_mm
> under #ifdef CONFIG_ARM_32 will be fine. The setup_mm under
> #ifdef CONFIG_ARM_64, doesn't need xenheap_mfn_end and we could just
> remove it from there.
> 
> Does it make sense? Am I missing something?

To be honest, I really want to limit the ifdefery in the mm code. This 
is a bit complex to follow. One of my side project is to look at that.

Also, even if xenheap_mfn_end today is not used, I think the current 
value is valid and could be helpful to have in hand. For instance, it 
does not seem justify to have different implementation of at least 
is_xen_heap_page for arm32 and arm64.

So I am not in favor of dropping xenheap_mfn_end at the moment.

Cheers,

-- 
Julien Grall

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

Re: [PATCH 03/24] xen/arm: setup: Remove bogus xenheap_mfn_end in setup_mm for arm64

[Stefano Stabellini]

On Fri, 16 Jun 2017, Julien Grall wrote:
> Hi Stefano,
> 
> On 06/16/2017 06:33 PM, Stefano Stabellini wrote:
> > On Fri, 16 Jun 2017, Julien Grall wrote:
> > > Hi Stefano,
> > > 
> > > On 15/06/2017 23:28, Stefano Stabellini wrote:
> > > > On Tue, 13 Jun 2017, Julien Grall wrote:
> > > > > xenheap_mfn_end is storing an MFN and not a physical address.
> > > > > Thankfully
> > > > > xenheap_mfn_end is not used in the arm64 code. So drop it.
> > > > 
> > > > That's fine, but in that case I would prefer to move the definition of
> > > > xenheap_mfn_end under #ifdef CONFIG_ARM_32. In fact, there is another
> > > > assignment of xenheap_mfn_end few lines below in the arm64 version of
> > > > setup_mm: don't we need to remove that too?
> > > 
> > > The other xenheap_mfn_end contains valid mfn that point to the end and I
> > > didn't want to #ifdef it because:
> > > 	1) It complexify the code
> > > 	2) All regions should be bound with start/end to simplify potential
> > > use.
> > 
> > I am only suggesting to move its definition and declaration under #ifdef
> > CONFIG_ARM_32 in xen/include/asm-arm/mm.h and xen/arch/arm/mm.c.
> > 
> > After that, all users of xenheap_mfn_end are already #ifdef
> > CONFIG_ARM_32, except for xen/arch/arm/setup.c:setup_mm. The setup_mm
> > under #ifdef CONFIG_ARM_32 will be fine. The setup_mm under
> > #ifdef CONFIG_ARM_64, doesn't need xenheap_mfn_end and we could just
> > remove it from there.
> > 
> > Does it make sense? Am I missing something?
> 
> To be honest, I really want to limit the ifdefery in the mm code. This is a
> bit complex to follow. One of my side project is to look at that.
> 
> Also, even if xenheap_mfn_end today is not used, I think the current value is
> valid and could be helpful to have in hand. For instance, it does not seem
> justify to have different implementation of at least is_xen_heap_page for
> arm32 and arm64.
> 
> So I am not in favor of dropping xenheap_mfn_end at the moment.

All right, then if we are going to keep xenheap_mfn_end around on arm64,
please update the commit message of this patch because it is confusing. 
It is just this one instance of xenheap_mfn_end in setup_mm which is
superfluous on arm64 because we are setting it again later.

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