[PATCH 00/16] xen/arm: mm: Remove open-coding mappings

[PATCH 00/16] xen/arm: mm: Remove open-coding mappings

[Julien Grall]

From: Julien Grall <jgrall@amazon.com>

Hi all,

This series was originally sent as "xen/arm: mm: Add limited support
for superpages" [1] and finally has grown enough to remove most of
the open-coding mappings in the boot code.

This will help to:
    1) Get better compliance with the Arm memory model
    2) Pave the way to support other page size (64KB, 16KB)

Cheers,

[1] <20201119190751.22345-1-julien@xen.org>

Julien Grall (15):
  xen/arm: mm: Allow other mapping size in xen_pt_update_entry()
  xen/arm: mm: Add support for the contiguous bit
  xen/arm: mm: Avoid flushing the TLBs when mapping are inserted
  xen/arm: mm: Don't open-code Xen PT update in remove_early_mappings()
  xen/arm: mm: Re-implement early_fdt_map() using map_pages_to_xen()
  xen/arm32: mm: Re-implement setup_xenheap_mappings() using
    map_pages_to_xen()
  xen/arm: mm: Allocate xen page tables in domheap rather than xenheap
  xen/arm: mm: Allow page-table allocation from the boot allocator
  xen/arm: Move fixmap definitions in a separate header
  xen/arm: mm: Clean-up the includes and order them
  xen/arm: mm: Use the PMAP helpers in xen_{,un}map_table()
  xen/arm32: setup: Move out the code to populate the boot allocator
  xen/arm64: mm: Add memory to the boot allocator first
  xen/arm: mm: Rework setup_xenheap_mappings()
  xen/arm: mm: Re-implement setup_frame_table_mappings() with
    map_pages_to_xen()

Wei Liu (1):
  xen/arm: add Persistent Map (PMAP) infrastructure

 xen/arch/arm/Kconfig                    |   1 +
 xen/arch/arm/acpi/lib.c                 |   2 +
 xen/arch/arm/include/asm/config.h       |   6 -
 xen/arch/arm/include/asm/early_printk.h |   1 +
 xen/arch/arm/include/asm/fixmap.h       |  48 +++
 xen/arch/arm/include/asm/lpae.h         |   8 +
 xen/arch/arm/include/asm/mm.h           |   4 -
 xen/arch/arm/include/asm/page.h         |   8 +
 xen/arch/arm/include/asm/pmap.h         |  32 ++
 xen/arch/arm/kernel.c                   |   1 +
 xen/arch/arm/mm.c                       | 494 +++++++++++++-----------
 xen/arch/arm/setup.c                    | 141 +++----
 xen/common/Kconfig                      |   3 +
 xen/common/Makefile                     |   1 +
 xen/common/pmap.c                       |  72 ++++
 xen/include/xen/acpi.h                  |  18 +-
 xen/include/xen/pmap.h                  |  16 +
 17 files changed, 549 insertions(+), 307 deletions(-)
 create mode 100644 xen/arch/arm/include/asm/fixmap.h
 create mode 100644 xen/arch/arm/include/asm/pmap.h
 create mode 100644 xen/common/pmap.c
 create mode 100644 xen/include/xen/pmap.h

-- 
2.32.0

[PATCH 01/16] xen/arm: mm: Allow other mapping size in xen_pt_update_entry()

[Julien Grall]

From: Julien Grall <julien.grall@arm.com>

At the moment, xen_pt_update_entry() only supports mapping at level 3
(i.e 4KB mapping). While this is fine for most of the runtime helper,
the boot code will require to use superpage mapping.

We don't want to allow superpage mapping by default as some of the
callers may expect small mappings (i.e populate_pt_range()) or even
expect to unmap only a part of a superpage.

To keep the code simple, a new flag _PAGE_BLOCK is introduced to
allow the caller to enable superpage mapping.

As the code doesn't support all the combinations, xen_pt_check_entry()
is extended to take into account the cases we don't support when
using block mapping:
    - Replacing a table with a mapping. This may happen if region was
    first mapped with 4KB mapping and then later on replaced with a 2MB
    (or 1GB mapping).
    - Removing/modifying a table. This may happen if a caller try to
    remove a region with _PAGE_BLOCK set when it was created without it.

Note that the current restriction means that the caller must ensure that
_PAGE_BLOCK is consistently set/cleared across all the updates on a
given virtual region. This ought to be fine with the expected use-cases.

More rework will be necessary if we wanted to remove the restrictions.

Note that nr_mfns is now marked const as it is used for flushing the
TLBs and we don't want it to be modified.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Hongda Deng <Hongda.Heng@arm.com>

---
    Changes in v4:
        - Add Hongda's reviewed-by
        - Add a comment why nr_mfns is const
        - Open-code pfn_to_paddr()

    Changes in v3:
        - Fix clash after prefixing the PT macros with XEN_PT_
        - Fix typoes in the commit message
        - Support superpage mappings even if nr is not suitably aligned
        - Move the logic to find the level in a separate function

    Changes in v2:
        - Pass the target level rather than the order to
        xen_pt_update_entry()
        - Update some comments
        - Open-code paddr_to_pfn()
        - Add my AWS signed-off-by
---
 xen/arch/arm/include/asm/page.h |   4 ++
 xen/arch/arm/mm.c               | 109 ++++++++++++++++++++++++++------
 2 files changed, 95 insertions(+), 18 deletions(-)

diff --git a/xen/arch/arm/include/asm/page.h b/xen/arch/arm/include/asm/page.h
index c6f9fb0d4e0c..07998df47bac 100644
--- a/xen/arch/arm/include/asm/page.h
+++ b/xen/arch/arm/include/asm/page.h
@@ -69,6 +69,7 @@
  * [3:4] Permission flags
  * [5]   Page present
  * [6]   Only populate page tables
+ * [7]   Superpage mappings is allowed
  */
 #define PAGE_AI_MASK(x) ((x) & 0x7U)
 
@@ -82,6 +83,9 @@
 #define _PAGE_PRESENT    (1U << 5)
 #define _PAGE_POPULATE   (1U << 6)
 
+#define _PAGE_BLOCK_BIT     7
+#define _PAGE_BLOCK         (1U << _PAGE_BLOCK_BIT)
+
 /*
  * _PAGE_DEVICE and _PAGE_NORMAL are convenience defines. They are not
  * meant to be used outside of this header.
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 7b1f2f49060d..be2ac302d731 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1078,9 +1078,10 @@ static int xen_pt_next_level(bool read_only, unsigned int level,
 }
 
 /* Sanity check of the entry */
-static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
+static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int level,
+                               unsigned int flags)
 {
-    /* Sanity check when modifying a page. */
+    /* Sanity check when modifying an entry. */
     if ( (flags & _PAGE_PRESENT) && mfn_eq(mfn, INVALID_MFN) )
     {
         /* We don't allow modifying an invalid entry. */
@@ -1090,6 +1091,13 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
             return false;
         }
 
+        /* We don't allow modifying a table entry */
+        if ( !lpae_is_mapping(entry, level) )
+        {
+            mm_printk("Modifying a table entry is not allowed.\n");
+            return false;
+        }
+
         /* We don't allow changing memory attributes. */
         if ( entry.pt.ai != PAGE_AI_MASK(flags) )
         {
@@ -1105,7 +1113,7 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
             return false;
         }
     }
-    /* Sanity check when inserting a page */
+    /* Sanity check when inserting a mapping */
     else if ( flags & _PAGE_PRESENT )
     {
         /* We should be here with a valid MFN. */
@@ -1114,18 +1122,28 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
         /* We don't allow replacing any valid entry. */
         if ( lpae_is_valid(entry) )
         {
-            mm_printk("Changing MFN for a valid entry is not allowed (%#"PRI_mfn" -> %#"PRI_mfn").\n",
-                      mfn_x(lpae_get_mfn(entry)), mfn_x(mfn));
+            if ( lpae_is_mapping(entry, level) )
+                mm_printk("Changing MFN for a valid entry is not allowed (%#"PRI_mfn" -> %#"PRI_mfn").\n",
+                          mfn_x(lpae_get_mfn(entry)), mfn_x(mfn));
+            else
+                mm_printk("Trying to replace a table with a mapping.\n");
             return false;
         }
     }
-    /* Sanity check when removing a page. */
+    /* Sanity check when removing a mapping. */
     else if ( (flags & (_PAGE_PRESENT|_PAGE_POPULATE)) == 0 )
     {
         /* We should be here with an invalid MFN. */
         ASSERT(mfn_eq(mfn, INVALID_MFN));
 
-        /* We don't allow removing page with contiguous bit set. */
+        /* We don't allow removing a table */
+        if ( lpae_is_table(entry, level) )
+        {
+            mm_printk("Removing a table is not allowed.\n");
+            return false;
+        }
+
+        /* We don't allow removing a mapping with contiguous bit set. */
         if ( entry.pt.contig )
         {
             mm_printk("Removing entry with contiguous bit set is not allowed.\n");
@@ -1143,13 +1161,13 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
     return true;
 }
 
+/* Update an entry at the level @target. */
 static int xen_pt_update_entry(mfn_t root, unsigned long virt,
-                               mfn_t mfn, unsigned int flags)
+                               mfn_t mfn, unsigned int target,
+                               unsigned int flags)
 {
     int rc;
     unsigned int level;
-    /* We only support 4KB mapping (i.e level 3) for now */
-    unsigned int target = 3;
     lpae_t *table;
     /*
      * The intermediate page tables are read-only when the MFN is not valid
@@ -1204,7 +1222,7 @@ static int xen_pt_update_entry(mfn_t root, unsigned long virt,
     entry = table + offsets[level];
 
     rc = -EINVAL;
-    if ( !xen_pt_check_entry(*entry, mfn, flags) )
+    if ( !xen_pt_check_entry(*entry, mfn, level, flags) )
         goto out;
 
     /* If we are only populating page-table, then we are done. */
@@ -1222,8 +1240,11 @@ static int xen_pt_update_entry(mfn_t root, unsigned long virt,
         {
             pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
 
-            /* Third level entries set pte.pt.table = 1 */
-            pte.pt.table = 1;
+            /*
+             * First and second level pages set pte.pt.table = 0, but
+             * third level entries set pte.pt.table = 1.
+             */
+            pte.pt.table = (level == 3);
         }
         else /* We are updating the permission => Copy the current pte. */
             pte = *entry;
@@ -1243,15 +1264,57 @@ out:
     return rc;
 }
 
+/* Return the level where mapping should be done */
+static int xen_pt_mapping_level(unsigned long vfn, mfn_t mfn, unsigned long nr,
+                                unsigned int flags)
+{
+    unsigned int level;
+    unsigned long mask;
+
+    /*
+      * Don't take into account the MFN when removing mapping (i.e
+      * MFN_INVALID) to calculate the correct target order.
+      *
+      * Per the Arm Arm, `vfn` and `mfn` must be both superpage aligned.
+      * They are or-ed together and then checked against the size of
+      * each level.
+      *
+      * `left` is not included and checked separately to allow
+      * superpage mapping even if it is not properly aligned (the
+      * user may have asked to map 2MB + 4k).
+      */
+     mask = !mfn_eq(mfn, INVALID_MFN) ? mfn_x(mfn) : 0;
+     mask |= vfn;
+
+     /*
+      * Always use level 3 mapping unless the caller request block
+      * mapping.
+      */
+     if ( likely(!(flags & _PAGE_BLOCK)) )
+         level = 3;
+     else if ( !(mask & (BIT(FIRST_ORDER, UL) - 1)) &&
+               (nr >= BIT(FIRST_ORDER, UL)) )
+         level = 1;
+     else if ( !(mask & (BIT(SECOND_ORDER, UL) - 1)) &&
+               (nr >= BIT(SECOND_ORDER, UL)) )
+         level = 2;
+     else
+         level = 3;
+
+     return level;
+}
+
 static DEFINE_SPINLOCK(xen_pt_lock);
 
 static int xen_pt_update(unsigned long virt,
                          mfn_t mfn,
-                         unsigned long nr_mfns,
+                         /* const on purpose as it is used for TLB flush */
+                         const unsigned long nr_mfns,
                          unsigned int flags)
 {
     int rc = 0;
-    unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
+    unsigned long vfn = virt >> PAGE_SHIFT;
+    unsigned long left = nr_mfns;
 
     /*
      * For arm32, page-tables are different on each CPUs. Yet, they share
@@ -1283,14 +1346,24 @@ static int xen_pt_update(unsigned long virt,
 
     spin_lock(&xen_pt_lock);
 
-    for ( ; addr < addr_end; addr += PAGE_SIZE )
+    while ( left )
     {
-        rc = xen_pt_update_entry(root, addr, mfn, flags);
+        unsigned int order, level;
+
+        level = xen_pt_mapping_level(vfn, mfn, left, flags);
+        order = XEN_PT_LEVEL_ORDER(level);
+
+        ASSERT(left >= BIT(order, UL));
+
+        rc = xen_pt_update_entry(root, vfn << PAGE_SHIFT, mfn, level, flags);
         if ( rc )
             break;
 
+        vfn += 1U << order;
         if ( !mfn_eq(mfn, INVALID_MFN) )
-            mfn = mfn_add(mfn, 1);
+            mfn = mfn_add(mfn, 1U << order);
+
+        left -= (1U << order);
     }
 
     /*
-- 
2.32.0

[PATCH 02/16] xen/arm: mm: Add support for the contiguous bit

[Julien Grall]

From: Julien Grall <jgrall@amazon.com>

In follow-up patches, we will use xen_pt_update() (or its callers)
to handle large mappings (e.g. frametable, xenheap). They are also
not going to be modified once created.

The page-table entries have an hint to indicate that whether an
entry is contiguous to another 16 entries (assuming 4KB). When the
processor support the hint, one TLB entry will be created per
contiguous region.

For now this is tied to _PAGE_BLOCK. We can untie it in the future
if there are use-cases where we may want to use _PAGE_BLOCK without
setting the contiguous (couldn't think of any yet).

Note that to avoid extra complexity, mappings with the contiguous
bit set cannot be removed. Given the expected use, this restriction
ought to be fine.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Hongda Deng <Hongda.Deng@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

---
    Changes in v4:
        - Add Hongda and Stefano's reviewed-by
        - Exit the outer loop as soon as there is an error.

    Changes in v3:
        - New patch
---
 xen/arch/arm/include/asm/page.h |  4 ++
 xen/arch/arm/mm.c               | 83 +++++++++++++++++++++++++++++----
 2 files changed, 79 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/include/asm/page.h b/xen/arch/arm/include/asm/page.h
index 07998df47bac..e7cd62190c7f 100644
--- a/xen/arch/arm/include/asm/page.h
+++ b/xen/arch/arm/include/asm/page.h
@@ -70,6 +70,7 @@
  * [5]   Page present
  * [6]   Only populate page tables
  * [7]   Superpage mappings is allowed
+ * [8]   Set contiguous bit (internal flag)
  */
 #define PAGE_AI_MASK(x) ((x) & 0x7U)
 
@@ -86,6 +87,9 @@
 #define _PAGE_BLOCK_BIT     7
 #define _PAGE_BLOCK         (1U << _PAGE_BLOCK_BIT)
 
+#define _PAGE_CONTIG_BIT    8
+#define _PAGE_CONTIG        (1U << _PAGE_CONTIG_BIT)
+
 /*
  * _PAGE_DEVICE and _PAGE_NORMAL are convenience defines. They are not
  * meant to be used outside of this header.
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index be2ac302d731..c4487dd7fc46 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1252,6 +1252,8 @@ static int xen_pt_update_entry(mfn_t root, unsigned long virt,
         /* Set permission */
         pte.pt.ro = PAGE_RO_MASK(flags);
         pte.pt.xn = PAGE_XN_MASK(flags);
+        /* Set contiguous bit */
+        pte.pt.contig = !!(flags & _PAGE_CONTIG);
     }
 
     write_pte(entry, pte);
@@ -1304,6 +1306,51 @@ static int xen_pt_mapping_level(unsigned long vfn, mfn_t mfn, unsigned long nr,
      return level;
 }
 
+#define XEN_PT_4K_NR_CONTIG 16
+
+/*
+ * Check whether the contiguous bit can be set. Return the number of
+ * contiguous entry allowed. If not allowed, return 1.
+ */
+static unsigned int xen_pt_check_contig(unsigned long vfn, mfn_t mfn,
+                                        unsigned int level, unsigned long left,
+                                        unsigned int flags)
+{
+    unsigned long nr_contig;
+
+    /*
+     * Allow the contiguous bit to set when the caller requests block
+     * mapping.
+     */
+    if ( !(flags & _PAGE_BLOCK) )
+        return 1;
+
+    /*
+     * We don't allow to remove mapping with the contiguous bit set.
+     * So shortcut the logic and directly return 1.
+     */
+    if ( mfn_eq(mfn, INVALID_MFN) )
+        return 1;
+
+    /*
+     * The number of contiguous entries varies depending on the page
+     * granularity used. The logic below assumes 4KB.
+     */
+    BUILD_BUG_ON(PAGE_SIZE != SZ_4K);
+
+    /*
+     * In order to enable the contiguous bit, we should have enough entries
+     * to map left and both the virtual and physical address should be
+     * aligned to the size of 16 translation tables entries.
+     */
+    nr_contig = BIT(XEN_PT_LEVEL_ORDER(level), UL) * XEN_PT_4K_NR_CONTIG;
+
+    if ( (left < nr_contig) || ((mfn_x(mfn) | vfn) & (nr_contig - 1)) )
+        return 1;
+
+    return XEN_PT_4K_NR_CONTIG;
+}
+
 static DEFINE_SPINLOCK(xen_pt_lock);
 
 static int xen_pt_update(unsigned long virt,
@@ -1338,6 +1385,12 @@ static int xen_pt_update(unsigned long virt,
         return -EINVAL;
     }
 
+    if ( flags & _PAGE_CONTIG )
+    {
+        mm_printk("_PAGE_CONTIG is an internal only flag.\n");
+        return -EINVAL;
+    }
+
     if ( !IS_ALIGNED(virt, PAGE_SIZE) )
     {
         mm_printk("The virtual address is not aligned to the page-size.\n");
@@ -1348,22 +1401,36 @@ static int xen_pt_update(unsigned long virt,
 
     while ( left )
     {
-        unsigned int order, level;
+        unsigned int order, level, nr_contig, new_flags;
 
         level = xen_pt_mapping_level(vfn, mfn, left, flags);
         order = XEN_PT_LEVEL_ORDER(level);
 
         ASSERT(left >= BIT(order, UL));
 
-        rc = xen_pt_update_entry(root, vfn << PAGE_SHIFT, mfn, level, flags);
-        if ( rc )
-            break;
+        /*
+         * Check if we can set the contiguous mapping and update the
+         * flags accordingly.
+         */
+        nr_contig = xen_pt_check_contig(vfn, mfn, level, left, flags);
+        new_flags = flags | ((nr_contig > 1) ? _PAGE_CONTIG : 0);
 
-        vfn += 1U << order;
-        if ( !mfn_eq(mfn, INVALID_MFN) )
-            mfn = mfn_add(mfn, 1U << order);
+        for ( ; nr_contig > 0; nr_contig-- )
+        {
+            rc = xen_pt_update_entry(root, vfn << PAGE_SHIFT, mfn, level,
+                                     new_flags);
+            if ( rc )
+                break;
+
+            vfn += 1U << order;
+            if ( !mfn_eq(mfn, INVALID_MFN) )
+                mfn = mfn_add(mfn, 1U << order);
 
-        left -= (1U << order);
+            left -= (1U << order);
+        }
+
+        if ( rc )
+            break;
     }
 
     /*
-- 
2.32.0

[PATCH 03/16] xen/arm: mm: Avoid flushing the TLBs when mapping are inserted

[Julien Grall]

From: Julien Grall <jgrall@amazon.com>

Currently, the function xen_pt_update() will flush the TLBs even when
the mappings are inserted. This is a bit wasteful because we don't
allow mapping replacement. Even if we were, the flush would need to
happen earlier because mapping replacement should use Break-Before-Make
when updating the entry.

A single call to xen_pt_update() can perform a single action. IOW, it
is not possible to, for instance, mix inserting and removing mappings.
Therefore, we can use `flags` to determine what action is performed.

This change will be particularly help to limit the impact of switching
boot time mapping to use xen_pt_update().

Signed-off-by: Julien Grall <jgrall@amazon.com>

---
    Changes in v4:
        - Switch the check to a different expression that will still
        result to the same truth table.

    Changes in v2:
        - New patch
---
 xen/arch/arm/mm.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index c4487dd7fc46..747083d820dd 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1119,7 +1119,13 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int level,
         /* We should be here with a valid MFN. */
         ASSERT(!mfn_eq(mfn, INVALID_MFN));
 
-        /* We don't allow replacing any valid entry. */
+        /*
+         * We don't allow replacing any valid entry.
+         *
+         * Note that the function xen_pt_update() relies on this
+         * assumption and will skip the TLB flush. The function will need
+         * to be updated if the check is relaxed.
+         */
         if ( lpae_is_valid(entry) )
         {
             if ( lpae_is_mapping(entry, level) )
@@ -1434,11 +1440,16 @@ static int xen_pt_update(unsigned long virt,
     }
 
     /*
-     * Flush the TLBs even in case of failure because we may have
+     * The TLBs flush can be safely skipped when a mapping is inserted
+     * as we don't allow mapping replacement (see xen_pt_check_entry()).
+     *
+     * For all the other cases, the TLBs will be flushed unconditionally
+     * even if the mapping has failed. This is because we may have
      * partially modified the PT. This will prevent any unexpected
      * behavior afterwards.
      */
-    flush_xen_tlb_range_va(virt, PAGE_SIZE * nr_mfns);
+    if ( !((flags & _PAGE_PRESENT) && !mfn_eq(mfn, INVALID_MFN)) )
+        flush_xen_tlb_range_va(virt, PAGE_SIZE * nr_mfns);
 
     spin_unlock(&xen_pt_lock);
 
-- 
2.32.0

[PATCH 04/16] xen/arm: mm: Don't open-code Xen PT update in remove_early_mappings()

[Julien Grall]

From: Julien Grall <julien.grall@arm.com>

Now that xen_pt_update_entry() is able to deal with different mapping
size, we can replace the open-coding of the page-tables update by a call
to modify_xen_mappings().

As the function is not meant to fail, a BUG_ON() is added to check the
return.

Note that we don't use destroy_xen_mappings() because the helper doesn't
allow us to pass a flags. In theory we could add an extra parameter to
the function, however there are no other expected users. Hence why
modify_xen_mappings() is used.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Hongda Deng <Hongda.Heng@arm.com>

---
    Changes in v4:
        - Add Hongda's reviewed-by
        - Add a comment to explain what modify_xen_mappings() does.
        - Clarify in the commit message hwy modify_xen_mappings() is
          used rather than destroy_xen_mappings().

    Changes in v2:
        - Stay consistent with how function name are used in the commit
        message
        - Add my AWS signed-off-by
---
 xen/arch/arm/mm.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 747083d820dd..64a79d45b38c 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -614,11 +614,12 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
 
 void __init remove_early_mappings(void)
 {
-    lpae_t pte = {0};
-    write_pte(xen_second + second_table_offset(BOOT_FDT_VIRT_START), pte);
-    write_pte(xen_second + second_table_offset(BOOT_FDT_VIRT_START + SZ_2M),
-              pte);
-    flush_xen_tlb_range_va(BOOT_FDT_VIRT_START, BOOT_FDT_SLOT_SIZE);
+    int rc;
+
+    /* destroy the _PAGE_BLOCK mapping */
+    rc = modify_xen_mappings(BOOT_FDT_VIRT_START, BOOT_FDT_VIRT_END,
+                             _PAGE_BLOCK);
+    BUG_ON(rc);
 }
 
 /*
-- 
2.32.0

[PATCH 05/16] xen/arm: mm: Re-implement early_fdt_map() using map_pages_to_xen()

[Julien Grall]

From: Julien Grall <julien.grall@arm.com>

Now that map_pages_to_xen() has been extended to support 2MB mappings,
we can replace the create_mappings() calls by map_pages_to_xen() calls.

The mapping can also be marked read-only as Xen should not modify
the host Device Tree during boot.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Hongda Deng <Hongda.Heng@arm.com>

---
    Changes in v4:
        - Fix typo in the commit message
        - Add Hongda's reviewed-by

    Changes in v2:
        - Add my AWS signed-off-by
        - Fix typo in the commit message
---
 xen/arch/arm/mm.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 64a79d45b38c..03f970e4d10b 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -574,6 +574,7 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
     paddr_t offset;
     void *fdt_virt;
     uint32_t size;
+    int rc;
 
     /*
      * Check whether the physical FDT address is set and meets the minimum
@@ -589,8 +590,12 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
     /* The FDT is mapped using 2MB superpage */
     BUILD_BUG_ON(BOOT_FDT_VIRT_START % SZ_2M);
 
-    create_mappings(xen_second, BOOT_FDT_VIRT_START, paddr_to_pfn(base_paddr),
-                    SZ_2M >> PAGE_SHIFT, SZ_2M);
+    rc = map_pages_to_xen(BOOT_FDT_VIRT_START, maddr_to_mfn(base_paddr),
+                          SZ_2M >> PAGE_SHIFT,
+                          PAGE_HYPERVISOR_RO | _PAGE_BLOCK);
+    if ( rc )
+        panic("Unable to map the device-tree.\n");
+
 
     offset = fdt_paddr % SECOND_SIZE;
     fdt_virt = (void *)BOOT_FDT_VIRT_START + offset;
@@ -604,9 +609,12 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
 
     if ( (offset + size) > SZ_2M )
     {
-        create_mappings(xen_second, BOOT_FDT_VIRT_START + SZ_2M,
-                        paddr_to_pfn(base_paddr + SZ_2M),
-                        SZ_2M >> PAGE_SHIFT, SZ_2M);
+        rc = map_pages_to_xen(BOOT_FDT_VIRT_START + SZ_2M,
+                              maddr_to_mfn(base_paddr + SZ_2M),
+                              SZ_2M >> PAGE_SHIFT,
+                              PAGE_HYPERVISOR_RO | _PAGE_BLOCK);
+        if ( rc )
+            panic("Unable to map the device-tree\n");
     }
 
     return fdt_virt;
-- 
2.32.0

[PATCH 06/16] xen/arm32: mm: Re-implement setup_xenheap_mappings() using map_pages_to_xen()

[Julien Grall]

From: Julien Grall <jgrall@amazon.com>

Now that map_pages_to_xen() has been extended to support 2MB mappings,
we can replace the create_mappings() call by map_pages_to_xen() call.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

---
    Changes in v4:
        - Add Stefano's reviewed-by

    Changes in v3:
        - Fix build when CONFIG_DEBUG=y

    Changes in v2:
        - New patch
---
 xen/arch/arm/mm.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 03f970e4d10b..47c2111c36a4 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -825,7 +825,12 @@ void mmu_init_secondary_cpu(void)
 void __init setup_xenheap_mappings(unsigned long base_mfn,
                                    unsigned long nr_mfns)
 {
-    create_mappings(xen_second, XENHEAP_VIRT_START, base_mfn, nr_mfns, MB(32));
+    int rc;
+
+    rc = map_pages_to_xen(XENHEAP_VIRT_START, _mfn(base_mfn), nr_mfns,
+                          PAGE_HYPERVISOR_RW | _PAGE_BLOCK);
+    if ( rc )
+        panic("Unable to setup the xenheap mappings.\n");
 
     /* Record where the xenheap is, for translation routines. */
     xenheap_virt_end = XENHEAP_VIRT_START + nr_mfns * PAGE_SIZE;
-- 
2.32.0

[PATCH 07/16] xen/arm: mm: Allocate xen page tables in domheap rather than xenheap

[Julien Grall]

From: Julien Grall <jgrall@amazon.com>

xen_{un,}map_table() already uses the helper to map/unmap pages
on-demand (note this is currently a NOP on arm64). So switching to
domheap don't have any disavantage.

But this as the benefit:
    - to keep the page tables unmapped if an arch decided to do so
    - reduce xenheap use on arm32 which can be pretty small

Signed-off-by: Julien Grall <jgrall@amazon.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>

---
    Changes in v3:
        - Add Stefano's acked-by

    Changes in v2:
        - New patch
---
 xen/arch/arm/mm.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 47c2111c36a4..252114d67df5 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -988,21 +988,6 @@ void *ioremap(paddr_t pa, size_t len)
     return ioremap_attr(pa, len, PAGE_HYPERVISOR_NOCACHE);
 }
 
-static int create_xen_table(lpae_t *entry)
-{
-    void *p;
-    lpae_t pte;
-
-    p = alloc_xenheap_page();
-    if ( p == NULL )
-        return -ENOMEM;
-    clear_page(p);
-    pte = mfn_to_xen_entry(virt_to_mfn(p), MT_NORMAL);
-    pte.pt.table = 1;
-    write_pte(entry, pte);
-    return 0;
-}
-
 static lpae_t *xen_map_table(mfn_t mfn)
 {
     /*
@@ -1043,6 +1028,27 @@ static void xen_unmap_table(const lpae_t *table)
     unmap_domain_page(table);
 }
 
+static int create_xen_table(lpae_t *entry)
+{
+    struct page_info *pg;
+    void *p;
+    lpae_t pte;
+
+    pg = alloc_domheap_page(NULL, 0);
+    if ( pg == NULL )
+        return -ENOMEM;
+
+    p = xen_map_table(page_to_mfn(pg));
+    clear_page(p);
+    xen_unmap_table(p);
+
+    pte = mfn_to_xen_entry(page_to_mfn(pg), MT_NORMAL);
+    pte.pt.table = 1;
+    write_pte(entry, pte);
+
+    return 0;
+}
+
 #define XEN_TABLE_MAP_FAILED 0
 #define XEN_TABLE_SUPER_PAGE 1
 #define XEN_TABLE_NORMAL_PAGE 2
-- 
2.32.0

[PATCH 08/16] xen/arm: mm: Allow page-table allocation from the boot allocator

[Julien Grall]

From: Julien Grall <julien.grall@arm.com>

At the moment, page-table can only be allocated from domheap. This means
it is not possible to create mapping in the page-tables via
map_pages_to_xen() if page-table needs to be allocated.

In order to avoid open-coding page-tables update in early boot, we need
to be able to allocate page-tables much earlier. Thankfully, we have the
boot allocator for those cases.

create_xen_table() is updated to cater early boot allocation by using
alloc_boot_pages().

Note, this is not sufficient to bootstrap the page-tables (i.e mapping
before any memory is actually mapped). This will be addressed
separately.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

---
    Changes in v4:
        - Add Stefano's reviewed-by

    Changes in v2:
        - New patch
---
 xen/arch/arm/mm.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 252114d67df5..6b7b72de27fe 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1030,19 +1030,27 @@ static void xen_unmap_table(const lpae_t *table)
 
 static int create_xen_table(lpae_t *entry)
 {
-    struct page_info *pg;
+    mfn_t mfn;
     void *p;
     lpae_t pte;
 
-    pg = alloc_domheap_page(NULL, 0);
-    if ( pg == NULL )
-        return -ENOMEM;
+    if ( system_state != SYS_STATE_early_boot )
+    {
+        struct page_info *pg = alloc_domheap_page(NULL, 0);
+
+        if ( pg == NULL )
+            return -ENOMEM;
+
+        mfn = page_to_mfn(pg);
+    }
+    else
+        mfn = alloc_boot_pages(1, 1);
 
-    p = xen_map_table(page_to_mfn(pg));
+    p = xen_map_table(mfn);
     clear_page(p);
     xen_unmap_table(p);
 
-    pte = mfn_to_xen_entry(page_to_mfn(pg), MT_NORMAL);
+    pte = mfn_to_xen_entry(mfn, MT_NORMAL);
     pte.pt.table = 1;
     write_pte(entry, pte);
 
-- 
2.32.0

[PATCH 09/16] xen/arm: Move fixmap definitions in a separate header

[Julien Grall]

From: Julien Grall <jgrall@amazon.com>

To use properly the fixmap definitions, their user would need
also new to include <xen/acpi.h>. This is not very great when
the user itself is not meant to directly use ACPI definitions.

Including <xen/acpi.h> in <asm/config.h> is not option because
the latter header is included by everyone. So move out the fixmap
entries definition in a new header.

Take the opportunity to also move {set, clear}_fixmap() prototypes
in the new header.

Note that most of the definitions in <xen/acpi.h> now need to be
surrounded with #ifndef __ASSEMBLY__ because <asm/fixmap.h> will
be used in assembly (see EARLY_UART_VIRTUAL_ADDRESS).

The split will become more helpful in a follow-up patch where new
fixmap entries will be defined.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Acked-by: Jan Beulich <jbeulich@suse.com>

---
    There was some disagreement with Stefano on whether fixmap.h
    should include acpi.h or this should be the other way around.

    I chose the former because each components should decide how
    much entries in the fixmap they need and also because this is
    the current behavior on x86. We should stay consitent
    between arch to avoid any headers mess.

    Jan acked this patch, so I am assuming he is happy with this
    approach. I would be OK to rework it if others agree with
    Stefano's view.

    Changes in v4:
        - Add Jan's acked-by
        - Record Stefano's disagreement on the approach

    Changes in v3:
        - Patch added
---
 xen/arch/arm/acpi/lib.c                 |  2 ++
 xen/arch/arm/include/asm/config.h       |  6 ------
 xen/arch/arm/include/asm/early_printk.h |  1 +
 xen/arch/arm/include/asm/fixmap.h       | 24 ++++++++++++++++++++++++
 xen/arch/arm/include/asm/mm.h           |  4 ----
 xen/arch/arm/kernel.c                   |  1 +
 xen/arch/arm/mm.c                       |  1 +
 xen/include/xen/acpi.h                  | 18 +++++++++++-------
 8 files changed, 40 insertions(+), 17 deletions(-)
 create mode 100644 xen/arch/arm/include/asm/fixmap.h

diff --git a/xen/arch/arm/acpi/lib.c b/xen/arch/arm/acpi/lib.c
index a59cc4074cfb..41d521f720ac 100644
--- a/xen/arch/arm/acpi/lib.c
+++ b/xen/arch/arm/acpi/lib.c
@@ -25,6 +25,8 @@
 #include <xen/init.h>
 #include <xen/mm.h>
 
+#include <asm/fixmap.h>
+
 static bool fixmap_inuse;
 
 char *__acpi_map_table(paddr_t phys, unsigned long size)
diff --git a/xen/arch/arm/include/asm/config.h b/xen/arch/arm/include/asm/config.h
index b25c9d39bb32..3e2a55a91058 100644
--- a/xen/arch/arm/include/asm/config.h
+++ b/xen/arch/arm/include/asm/config.h
@@ -169,12 +169,6 @@
 
 #endif
 
-/* Fixmap slots */
-#define FIXMAP_CONSOLE  0  /* The primary UART */
-#define FIXMAP_MISC     1  /* Ephemeral mappings of hardware */
-#define FIXMAP_ACPI_BEGIN  2  /* Start mappings of ACPI tables */
-#define FIXMAP_ACPI_END    (FIXMAP_ACPI_BEGIN + NUM_FIXMAP_ACPI_PAGES - 1)  /* End mappings of ACPI tables */
-
 #define NR_hypercalls 64
 
 #define STACK_ORDER 3
diff --git a/xen/arch/arm/include/asm/early_printk.h b/xen/arch/arm/include/asm/early_printk.h
index 8dc911cf48a3..c5149b2976da 100644
--- a/xen/arch/arm/include/asm/early_printk.h
+++ b/xen/arch/arm/include/asm/early_printk.h
@@ -11,6 +11,7 @@
 #define __ARM_EARLY_PRINTK_H__
 
 #include <xen/page-size.h>
+#include <asm/fixmap.h>
 
 #ifdef CONFIG_EARLY_PRINTK
 
diff --git a/xen/arch/arm/include/asm/fixmap.h b/xen/arch/arm/include/asm/fixmap.h
new file mode 100644
index 000000000000..1cee51e52ab9
--- /dev/null
+++ b/xen/arch/arm/include/asm/fixmap.h
@@ -0,0 +1,24 @@
+/*
+ * fixmap.h: compile-time virtual memory allocation
+ */
+#ifndef __ASM_FIXMAP_H
+#define __ASM_FIXMAP_H
+
+#include <xen/acpi.h>
+
+/* Fixmap slots */
+#define FIXMAP_CONSOLE  0  /* The primary UART */
+#define FIXMAP_MISC     1  /* Ephemeral mappings of hardware */
+#define FIXMAP_ACPI_BEGIN  2  /* Start mappings of ACPI tables */
+#define FIXMAP_ACPI_END    (FIXMAP_ACPI_BEGIN + NUM_FIXMAP_ACPI_PAGES - 1)  /* End mappings of ACPI tables */
+
+#ifndef __ASSEMBLY__
+
+/* Map a page in a fixmap entry */
+extern void set_fixmap(unsigned map, mfn_t mfn, unsigned attributes);
+/* Remove a mapping from a fixmap entry */
+extern void clear_fixmap(unsigned map);
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __ASM_FIXMAP_H */
diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
index 424aaf28230b..045a8ba4bb63 100644
--- a/xen/arch/arm/include/asm/mm.h
+++ b/xen/arch/arm/include/asm/mm.h
@@ -191,10 +191,6 @@ extern void mmu_init_secondary_cpu(void);
 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, 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 */
 void __iomem *ioremap_attr(paddr_t start, size_t len, unsigned attributes);
 
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 8f43caa1866d..25ded1c056d9 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -15,6 +15,7 @@
 #include <xen/vmap.h>
 
 #include <asm/byteorder.h>
+#include <asm/fixmap.h>
 #include <asm/kernel.h>
 #include <asm/setup.h>
 
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 6b7b72de27fe..52b2a0394047 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -41,6 +41,7 @@
 #include <xen/sizes.h>
 #include <xen/libfdt/libfdt.h>
 
+#include <asm/fixmap.h>
 #include <asm/setup.h>
 
 /* Override macros from asm/page.h to make them work with mfn_t */
diff --git a/xen/include/xen/acpi.h b/xen/include/xen/acpi.h
index 39d51fcd01dd..1b9c75e68fc4 100644
--- a/xen/include/xen/acpi.h
+++ b/xen/include/xen/acpi.h
@@ -28,6 +28,15 @@
 #define _LINUX
 #endif
 
+/*
+ * Fixmap pages to reserve for ACPI boot-time tables (see
+ * arch/x86/include/asm/fixmap.h or arch/arm/include/asm/fixmap.h),
+ * 64 pages(256KB) is large enough for most cases.)
+ */
+#define NUM_FIXMAP_ACPI_PAGES  64
+
+#ifndef __ASSEMBLY__
+
 #include <xen/list.h>
 
 #include <acpi/acpi.h>
@@ -39,13 +48,6 @@
 #define ACPI_MADT_GET_POLARITY(inti)	ACPI_MADT_GET_(POLARITY, inti)
 #define ACPI_MADT_GET_TRIGGER(inti)	ACPI_MADT_GET_(TRIGGER, inti)
 
-/*
- * Fixmap pages to reserve for ACPI boot-time tables (see
- * arch/x86/include/asm/fixmap.h or arch/arm/include/asm/config.h,
- * 64 pages(256KB) is large enough for most cases.)
- */
-#define NUM_FIXMAP_ACPI_PAGES  64
-
 #define BAD_MADT_ENTRY(entry, end) (                                        \
                 (!(entry)) || (unsigned long)(entry) + sizeof(*(entry)) > (end) ||  \
                 (entry)->header.length < sizeof(*(entry)))
@@ -207,4 +209,6 @@ void acpi_reboot(void);
 void acpi_dmar_zap(void);
 void acpi_dmar_reinstate(void);
 
+#endif /* __ASSEMBLY__ */
+
 #endif /*_LINUX_ACPI_H*/
-- 
2.32.0

[PATCH 10/16] xen/arm: add Persistent Map (PMAP) infrastructure

[Julien Grall]

From: Wei Liu <wei.liu2@citrix.com>

The basic idea is like Persistent Kernel Map (PKMAP) in Linux. We
pre-populate all the relevant page tables before the system is fully
set up.

We will need it on Arm in order to rework the arm64 version of
xenheap_setup_mappings() as we may need to use pages allocated from
the boot allocator before they are effectively mapped.

This infrastructure is not lock-protected therefore can only be used
before smpboot. After smpboot, map_domain_page() has to be used.

This is based on the x86 version [1] that was originally implemented
by Wei Liu.

The PMAP infrastructure is implemented in common code with some
arch helpers to set/clear the page-table entries and convertion
between a fixmap slot to a virtual address...

As mfn_to_xen_entry() now needs to be exported, take the opportunity
to swich the parameter attr from unsigned to unsigned int.

[1] <e92da4ad6015b6089737fcccba3ec1d6424649a5.1588278317.git.hongyxia@amazon.com>

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Hongyan Xia <hongyxia@amazon.com>
[julien: Adapted for Arm]
Signed-off-by: Julien Grall <jgrall@amazon.com>

---
    Changes in v4:
        - Move xen_fixmap in fixmap.h and add a comment about its usage.
        - Update comments
        - Use DECLARE_BITMAP()
        - Replace local_irq_{enable, disable} with an ASSERT() as there
          should be no user of pmap() in interrupt context.

    Changes in v3:
        - s/BITS_PER_LONG/BITS_PER_BYTE/
        - Move pmap to common code

    Changes in v2:
        - New patch

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Wei Liu <wl@xen.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/arm/Kconfig              |  1 +
 xen/arch/arm/include/asm/fixmap.h | 24 +++++++++++
 xen/arch/arm/include/asm/lpae.h   |  8 ++++
 xen/arch/arm/include/asm/pmap.h   | 32 ++++++++++++++
 xen/arch/arm/mm.c                 |  7 +--
 xen/common/Kconfig                |  3 ++
 xen/common/Makefile               |  1 +
 xen/common/pmap.c                 | 72 +++++++++++++++++++++++++++++++
 xen/include/xen/pmap.h            | 16 +++++++
 9 files changed, 158 insertions(+), 6 deletions(-)
 create mode 100644 xen/arch/arm/include/asm/pmap.h
 create mode 100644 xen/common/pmap.c
 create mode 100644 xen/include/xen/pmap.h

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index ecfa6822e4d3..a89a67802aa9 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -14,6 +14,7 @@ config ARM
 	select HAS_DEVICE_TREE
 	select HAS_PASSTHROUGH
 	select HAS_PDX
+	select HAS_PMAP
 	select IOMMU_FORCE_PT_SHARE
 
 config ARCH_DEFCONFIG
diff --git a/xen/arch/arm/include/asm/fixmap.h b/xen/arch/arm/include/asm/fixmap.h
index 1cee51e52ab9..365a2385a087 100644
--- a/xen/arch/arm/include/asm/fixmap.h
+++ b/xen/arch/arm/include/asm/fixmap.h
@@ -5,20 +5,44 @@
 #define __ASM_FIXMAP_H
 
 #include <xen/acpi.h>
+#include <xen/pmap.h>
 
 /* Fixmap slots */
 #define FIXMAP_CONSOLE  0  /* The primary UART */
 #define FIXMAP_MISC     1  /* Ephemeral mappings of hardware */
 #define FIXMAP_ACPI_BEGIN  2  /* Start mappings of ACPI tables */
 #define FIXMAP_ACPI_END    (FIXMAP_ACPI_BEGIN + NUM_FIXMAP_ACPI_PAGES - 1)  /* End mappings of ACPI tables */
+#define FIXMAP_PMAP_BEGIN (FIXMAP_ACPI_END + 1) /* Start of PMAP */
+#define FIXMAP_PMAP_END (FIXMAP_PMAP_BEGIN + NUM_FIX_PMAP - 1) /* End of PMAP */
+
+#define FIXMAP_LAST FIXMAP_PMAP_END
+
+#define FIXADDR_START FIXMAP_ADDR(0)
+#define FIXADDR_TOP FIXMAP_ADDR(FIXMAP_LAST)
 
 #ifndef __ASSEMBLY__
 
+/*
+ * Direct access to xen_fixmap[] should only happen when {set,
+ * clear}_fixmap() is unusable (e.g. where we would end up to
+ * recursively call the helpers).
+ */
+extern lpae_t xen_fixmap[XEN_PT_LPAE_ENTRIES];
+
 /* Map a page in a fixmap entry */
 extern void set_fixmap(unsigned map, mfn_t mfn, unsigned attributes);
 /* Remove a mapping from a fixmap entry */
 extern void clear_fixmap(unsigned map);
 
+#define fix_to_virt(slot) ((void *)FIXMAP_ADDR(slot))
+
+static inline unsigned int virt_to_fix(vaddr_t vaddr)
+{
+    BUG_ON(vaddr >= FIXADDR_TOP || vaddr < FIXADDR_START);
+
+    return ((vaddr - FIXADDR_START) >> PAGE_SHIFT);
+}
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* __ASM_FIXMAP_H */
diff --git a/xen/arch/arm/include/asm/lpae.h b/xen/arch/arm/include/asm/lpae.h
index aecb320dec45..fc19cbd84772 100644
--- a/xen/arch/arm/include/asm/lpae.h
+++ b/xen/arch/arm/include/asm/lpae.h
@@ -4,6 +4,7 @@
 #ifndef __ASSEMBLY__
 
 #include <xen/page-defs.h>
+#include <xen/mm-frame.h>
 
 /*
  * WARNING!  Unlike the x86 pagetable code, where l1 is the lowest level and
@@ -168,6 +169,13 @@ static inline bool lpae_is_superpage(lpae_t pte, unsigned int level)
         third_table_offset(addr)            \
     }
 
+/*
+ * 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.
+ */
+lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned int attr);
+
 #endif /* __ASSEMBLY__ */
 
 /*
diff --git a/xen/arch/arm/include/asm/pmap.h b/xen/arch/arm/include/asm/pmap.h
new file mode 100644
index 000000000000..74398b4c4fe6
--- /dev/null
+++ b/xen/arch/arm/include/asm/pmap.h
@@ -0,0 +1,32 @@
+#ifndef __ASM_PMAP_H__
+#define __ASM_PMAP_H__
+
+#include <xen/mm.h>
+
+#include <asm/fixmap.h>
+
+static inline void arch_pmap_map(unsigned int slot, mfn_t mfn)
+{
+    lpae_t *entry = &xen_fixmap[slot];
+    lpae_t pte;
+
+    ASSERT(!lpae_is_valid(*entry));
+
+    pte = mfn_to_xen_entry(mfn, PAGE_HYPERVISOR_RW);
+    pte.pt.table = 1;
+    write_pte(entry, pte);
+}
+
+static inline void arch_pmap_unmap(unsigned int slot)
+{
+    lpae_t pte = {};
+
+    write_pte(&xen_fixmap[slot], pte);
+
+    flush_xen_tlb_range_va_local(FIXMAP_ADDR(slot), PAGE_SIZE);
+}
+
+void arch_pmap_map_slot(unsigned int slot, mfn_t mfn);
+void arch_pmap_clear_slot(void *ptr);
+
+#endif /* __ASM_PMAP_H__ */
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 52b2a0394047..bd1348a99716 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -305,12 +305,7 @@ 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(mfn_t mfn, unsigned attr)
+lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned int attr)
 {
     lpae_t e = (lpae_t) {
         .pt = {
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index d921c74d615e..5b6b2406c028 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -49,6 +49,9 @@ config HAS_KEXEC
 config HAS_PDX
 	bool
 
+config HAS_PMAP
+	bool
+
 config HAS_SCHED_GRANULARITY
 	bool
 
diff --git a/xen/common/Makefile b/xen/common/Makefile
index b1e076c30b81..3baf83d527d8 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -29,6 +29,7 @@ obj-y += notifier.o
 obj-y += page_alloc.o
 obj-$(CONFIG_HAS_PDX) += pdx.o
 obj-$(CONFIG_PERF_COUNTERS) += perfc.o
+obj-bin-$(CONFIG_HAS_PMAP) += pmap.init.o
 obj-y += preempt.o
 obj-y += random.o
 obj-y += rangeset.o
diff --git a/xen/common/pmap.c b/xen/common/pmap.c
new file mode 100644
index 000000000000..9355cacb7373
--- /dev/null
+++ b/xen/common/pmap.c
@@ -0,0 +1,72 @@
+#include <xen/bitops.h>
+#include <xen/init.h>
+#include <xen/irq.h>
+#include <xen/pmap.h>
+
+#include <asm/pmap.h>
+#include <asm/fixmap.h>
+
+/*
+ * Simple mapping infrastructure to map / unmap pages in fixed map.
+ * This is used to set the page table before the map domain page infrastructure
+ * is initialized.
+ *
+ * This structure is not protected by any locks, so it must not be used after
+ * smp bring-up.
+ */
+
+/* Bitmap to track which slot is used */
+static __initdata DECLARE_BITMAP(inuse, NUM_FIX_PMAP);
+
+void *__init pmap_map(mfn_t mfn)
+{
+    unsigned int idx;
+    unsigned int slot;
+
+    ASSERT(system_state < SYS_STATE_smp_boot);
+    ASSERT(!in_irq());
+
+    idx = find_first_zero_bit(inuse, NUM_FIX_PMAP);
+    if ( idx == NUM_FIX_PMAP )
+        panic("Out of PMAP slots\n");
+
+    __set_bit(idx, inuse);
+
+    slot = idx + FIXMAP_PMAP_BEGIN;
+    ASSERT(slot >= FIXMAP_PMAP_BEGIN && slot <= FIXMAP_PMAP_END);
+
+    /*
+     * We cannot use set_fixmap() here. We use PMAP when the domain map
+     * page infrastructure is not yet initialized, so map_pages_to_xen() called
+     * by set_fixmap() needs to map pages on demand, which then calls pmap()
+     * again, resulting in a loop. Modify the PTEs directly instead. The same
+     * is true for pmap_unmap().
+     */
+    arch_pmap_map(slot, mfn);
+
+    return fix_to_virt(slot);
+}
+
+void __init pmap_unmap(const void *p)
+{
+    unsigned int idx;
+    unsigned int slot = virt_to_fix((unsigned long)p);
+
+    ASSERT(system_state < SYS_STATE_smp_boot);
+    ASSERT(slot >= FIXMAP_PMAP_BEGIN && slot <= FIXMAP_PMAP_END);
+    ASSERT(in_irq());
+
+    idx = slot - FIXMAP_PMAP_BEGIN;
+
+    __clear_bit(idx, inuse);
+    arch_pmap_unmap(slot);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/xen/pmap.h b/xen/include/xen/pmap.h
new file mode 100644
index 000000000000..93e61b10870e
--- /dev/null
+++ b/xen/include/xen/pmap.h
@@ -0,0 +1,16 @@
+#ifndef __XEN_PMAP_H__
+#define __XEN_PMAP_H__
+
+/* Large enough for mapping 5 levels of page tables with some headroom */
+#define NUM_FIX_PMAP 8
+
+#ifndef __ASSEMBLY__
+
+#include <xen/mm-frame.h>
+
+void *pmap_map(mfn_t mfn);
+void pmap_unmap(const void *p);
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __XEN_PMAP_H__ */
-- 
2.32.0

[PATCH 11/16] xen/arm: mm: Clean-up the includes and order them

[Julien Grall]

From: Julien Grall <jgrall@amazon.com>

The numbers of includes in mm.c has been growing quite a lot. However
some of them (e.g. xen/device_tree.h, xen/softirq.h) doesn't look
to be directly used by the file or other will be included by
larger headers (e.g asm/flushtlb.h will be included by xen/mm.h).

So trim down the number of includes. Take the opportunity to order
them with the xen headers first, then asm headers and last public
headers.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>

---

    Changes in v4:
        - Add Stefano's acked-by

    Changes in v3:
        - Patch added
---
 xen/arch/arm/mm.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index bd1348a99716..d40dd4e6c9e6 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -17,33 +17,26 @@
  * GNU General Public License for more details.
  */
 
-#include <xen/compile.h>
-#include <xen/types.h>
-#include <xen/device_tree.h>
-#include <xen/init.h>
-#include <xen/mm.h>
-#include <xen/preempt.h>
+#include <xen/domain_page.h>
 #include <xen/errno.h>
 #include <xen/grant_table.h>
-#include <xen/softirq.h>
-#include <xen/event.h>
 #include <xen/guest_access.h>
-#include <xen/domain_page.h>
-#include <xen/err.h>
-#include <asm/page.h>
-#include <asm/current.h>
-#include <asm/flushtlb.h>
-#include <public/memory.h>
+#include <xen/init.h>
+#include <xen/libfdt/libfdt.h>
+#include <xen/mm.h>
+#include <xen/pfn.h>
 #include <xen/sched.h>
+#include <xen/sizes.h>
+#include <xen/types.h>
 #include <xen/vmap.h>
+
 #include <xsm/xsm.h>
-#include <xen/pfn.h>
-#include <xen/sizes.h>
-#include <xen/libfdt/libfdt.h>
 
 #include <asm/fixmap.h>
 #include <asm/setup.h>
 
+#include <public/memory.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))
-- 
2.32.0

[PATCH 12/16] xen/arm: mm: Use the PMAP helpers in xen_{,un}map_table()

[Julien Grall]

From: Julien Grall <jgrall@amazon.com>

During early boot, it is not possible to use xen_{,un}map_table()
if the page tables are not residing the Xen binary.

This is a blocker to switch some of the helpers to use xen_pt_update()
as we may need to allocate extra page tables and access them before
the domheap has been initialized (see setup_xenheap_mappings()).

xen_{,un}map_table() are now updated to use the PMAP helpers for early
boot map/unmap. Note that the special case for page-tables residing
in Xen binary has been dropped because it is "complex" and was
only added as a workaround in 8d4f1b8878e0 ("xen/arm: mm: Allow
generic xen page-tables helpers to be called early").

Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

---
    Changes in v4:
        - Add Stefano's reviewed-by

    Changes in v2:
        - New patch
---
 xen/arch/arm/mm.c | 33 +++++++++------------------------
 1 file changed, 9 insertions(+), 24 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index d40dd4e6c9e6..b019e4b35b55 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -25,6 +25,7 @@
 #include <xen/libfdt/libfdt.h>
 #include <xen/mm.h>
 #include <xen/pfn.h>
+#include <xen/pmap.h>
 #include <xen/sched.h>
 #include <xen/sizes.h>
 #include <xen/types.h>
@@ -980,27 +981,11 @@ void *ioremap(paddr_t pa, size_t len)
 static lpae_t *xen_map_table(mfn_t mfn)
 {
     /*
-     * We may require to map the page table before map_domain_page() is
-     * useable. The requirements here is it must be useable as soon as
-     * page-tables are allocated dynamically via alloc_boot_pages().
-     *
-     * We need to do the check on physical address rather than virtual
-     * address to avoid truncation on Arm32. Therefore is_kernel() cannot
-     * be used.
+     * During early boot, map_domain_page() may be unusable. Use the
+     * PMAP to map temporarily a page-table.
      */
     if ( system_state == SYS_STATE_early_boot )
-    {
-        if ( is_xen_fixed_mfn(mfn) )
-        {
-            /*
-             * It is fine to demote the type because the size of Xen
-             * will always fit in vaddr_t.
-             */
-            vaddr_t offset = mfn_to_maddr(mfn) - virt_to_maddr(&_start);
-
-            return (lpae_t *)(XEN_VIRT_START + offset);
-        }
-    }
+        return pmap_map(mfn);
 
     return map_domain_page(mfn);
 }
@@ -1009,12 +994,12 @@ static void xen_unmap_table(const lpae_t *table)
 {
     /*
      * During early boot, xen_map_table() will not use map_domain_page()
-     * for page-tables residing in Xen binary. So skip the unmap part.
+     * but the PMAP.
      */
-    if ( system_state == SYS_STATE_early_boot && is_kernel(table) )
-        return;
-
-    unmap_domain_page(table);
+    if ( system_state == SYS_STATE_early_boot )
+        pmap_unmap(table);
+    else
+        unmap_domain_page(table);
 }
 
 static int create_xen_table(lpae_t *entry)
-- 
2.32.0

[PATCH 13/16] xen/arm32: setup: Move out the code to populate the boot allocator

[Julien Grall]

From: Julien Grall <jgrall@amazon.com>

In a follow-up patch, we will want to populate the boot allocator
separately for arm64. The code will end up to be very similar to the one
on arm32. So move out the code in a new helper populate_boot_allocator().

For now the code is still protected by CONFIG_ARM_32 to avoid any build
failure on arm64.

Take the opportunity to replace mfn_add(xen_mfn_start, xenheap_pages) with
xenheap_mfn_end as they are equivalent.

Signed-off-by: Julien Grall <jgrall@amazon.com>

---

    Changes in v4:
        - Patch added
---
 xen/arch/arm/setup.c | 90 +++++++++++++++++++++++++-------------------
 1 file changed, 51 insertions(+), 39 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index d5d0792ed48a..3d5a2283d4ef 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -637,10 +637,58 @@ static void __init init_staticmem_pages(void)
 }
 
 #ifdef CONFIG_ARM_32
+/*
+ * Populate the boot allocator. All the RAM but the following regions
+ * will be added:
+ *  - Modules (e.g., Xen, Kernel)
+ *  - Reserved regions
+ *  - Xenheap
+ */
+static void __init populate_boot_allocator(void)
+{
+    unsigned int i;
+    const struct meminfo *banks = &bootinfo.mem;
+
+    for ( i = 0; i < banks->nr_banks; i++ )
+    {
+        const struct membank *bank = &banks->bank[i];
+        paddr_t bank_end = bank->start + bank->size;
+        paddr_t s, e;
+
+        s = bank->start;
+        while ( s < bank_end )
+        {
+            paddr_t n = bank_end;
+
+            e = next_module(s, &n);
+
+            if ( e == ~(paddr_t)0 )
+                e = n = bank_end;
+
+            /*
+             * Module in a RAM bank other than the one which we are
+             * not dealing with here.
+             */
+            if ( e > bank_end )
+                e = bank_end;
+
+            /* Avoid the xenheap */
+            if ( s < mfn_to_maddr(xenheap_mfn_end) &&
+                 mfn_to_maddr(xenheap_mfn_start) < e )
+            {
+                e = mfn_to_maddr(xenheap_mfn_start);
+                n = mfn_to_maddr(xenheap_mfn_end);
+            }
+
+            fw_unreserved_regions(s, e, init_boot_pages, 0);
+            s = n;
+        }
+    }
+}
+
 static void __init setup_mm(void)
 {
-    paddr_t ram_start, ram_end, ram_size;
-    paddr_t s, e;
+    paddr_t ram_start, ram_end, ram_size, e;
     unsigned long ram_pages;
     unsigned long heap_pages, xenheap_pages, domheap_pages;
     int i;
@@ -718,43 +766,7 @@ static void __init setup_mm(void)
     setup_xenheap_mappings((e >> PAGE_SHIFT) - xenheap_pages, xenheap_pages);
 
     /* Add non-xenheap memory */
-    for ( i = 0; i < bootinfo.mem.nr_banks; i++ )
-    {
-        paddr_t bank_start = bootinfo.mem.bank[i].start;
-        paddr_t bank_end = bank_start + bootinfo.mem.bank[i].size;
-
-        s = bank_start;
-        while ( s < bank_end )
-        {
-            paddr_t n = bank_end;
-
-            e = next_module(s, &n);
-
-            if ( e == ~(paddr_t)0 )
-            {
-                e = n = ram_end;
-            }
-
-            /*
-             * Module in a RAM bank other than the one which we are
-             * not dealing with here.
-             */
-            if ( e > bank_end )
-                e = bank_end;
-
-            /* Avoid the xenheap */
-            if ( s < mfn_to_maddr(mfn_add(xenheap_mfn_start, xenheap_pages))
-                 && mfn_to_maddr(xenheap_mfn_start) < e )
-            {
-                e = mfn_to_maddr(xenheap_mfn_start);
-                n = mfn_to_maddr(mfn_add(xenheap_mfn_start, xenheap_pages));
-            }
-
-            fw_unreserved_regions(s, e, init_boot_pages, 0);
-
-            s = n;
-        }
-    }
+    populate_boot_allocator();
 
     /* Frame table covers all of RAM region, including holes */
     setup_frametable_mappings(ram_start, ram_end);
-- 
2.32.0

[PATCH 14/16] xen/arm64: mm: Add memory to the boot allocator first

[Julien Grall]

From: Julien Grall <jgrall@amazon.com>

Currently, memory is added to the boot allocator after the xenheap
mappings are done. This will break if the first mapping is more than
512GB of RAM.

In addition to that, a follow-up patch will rework setup_xenheap_mappings()
to use smaller mappings (e.g. 2MB, 4KB). So it will be necessary to have
memory in the boot allocator earlier.

Only free memory (e.g. not reserved or modules) can be added to the boot
allocator. It might be possible that some regions (including the first
one) will have no free memory.

So we need to add all the free memory to the boot allocator first
and then add do the mappings.

Populating the boot allocator is nearly the same between arm32 and
arm64. The only difference is on the former we need to exclude the
xenheap for the boot allocator. Gate the difference with CONFIG_ARM_32
so the code be re-used on arm64.

Signed-off-by: Julien Grall <jgrall@amazon.com>

---
    Changes in v4:
        - The implementation of populate_boot_allocator() has been
          moved in a separate patch.
        - Fix typo

    Changes in v3:
        - Patch added
---
 xen/arch/arm/setup.c | 55 +++++++++++++++++++-------------------------
 1 file changed, 24 insertions(+), 31 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 3d5a2283d4ef..db1768c03f03 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -636,13 +636,12 @@ static void __init init_staticmem_pages(void)
 #endif
 }
 
-#ifdef CONFIG_ARM_32
 /*
  * Populate the boot allocator. All the RAM but the following regions
  * will be added:
  *  - Modules (e.g., Xen, Kernel)
  *  - Reserved regions
- *  - Xenheap
+ *  - Xenheap (arm32 only)
  */
 static void __init populate_boot_allocator(void)
 {
@@ -672,6 +671,7 @@ static void __init populate_boot_allocator(void)
             if ( e > bank_end )
                 e = bank_end;
 
+#ifdef CONFIG_ARM_32
             /* Avoid the xenheap */
             if ( s < mfn_to_maddr(xenheap_mfn_end) &&
                  mfn_to_maddr(xenheap_mfn_start) < e )
@@ -679,6 +679,7 @@ static void __init populate_boot_allocator(void)
                 e = mfn_to_maddr(xenheap_mfn_start);
                 n = mfn_to_maddr(xenheap_mfn_end);
             }
+#endif
 
             fw_unreserved_regions(s, e, init_boot_pages, 0);
             s = n;
@@ -686,6 +687,7 @@ static void __init populate_boot_allocator(void)
     }
 }
 
+#ifdef CONFIG_ARM_32
 static void __init setup_mm(void)
 {
     paddr_t ram_start, ram_end, ram_size, e;
@@ -781,45 +783,36 @@ static void __init setup_mm(void)
 #else /* CONFIG_ARM_64 */
 static void __init setup_mm(void)
 {
+    const struct meminfo *banks = &bootinfo.mem;
     paddr_t ram_start = ~0;
     paddr_t ram_end = 0;
     paddr_t ram_size = 0;
-    int bank;
+    unsigned int i;
 
     init_pdx();
 
-    total_pages = 0;
-    for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
-    {
-        paddr_t bank_start = bootinfo.mem.bank[bank].start;
-        paddr_t bank_size = bootinfo.mem.bank[bank].size;
-        paddr_t bank_end = bank_start + bank_size;
-        paddr_t s, e;
-
-        ram_size = ram_size + bank_size;
-        ram_start = min(ram_start,bank_start);
-        ram_end = max(ram_end,bank_end);
-
-        setup_xenheap_mappings(bank_start>>PAGE_SHIFT, bank_size>>PAGE_SHIFT);
-
-        s = bank_start;
-        while ( s < bank_end )
-        {
-            paddr_t n = bank_end;
+    /*
+     * We need some memory to allocate the page-tables used for the xenheap
+     * mappings. But some regions may contain memory already allocated
+     * for other uses (e.g. modules, reserved-memory...).
+     *
+     * For simplicity, add all the free regions in the boot allocator.
+     */
+    populate_boot_allocator();
 
-            e = next_module(s, &n);
+    total_pages = 0;
 
-            if ( e == ~(paddr_t)0 )
-            {
-                e = n = bank_end;
-            }
+    for ( i = 0; i < banks->nr_banks; i++ )
+    {
+        const struct membank *bank = &banks->bank[i];
+        paddr_t bank_end = bank->start + bank->size;
 
-            if ( e > bank_end )
-                e = bank_end;
+        ram_size = ram_size + bank->size;
+        ram_start = min(ram_start, bank->start);
+        ram_end = max(ram_end, bank_end);
 
-            fw_unreserved_regions(s, e, init_boot_pages, 0);
-            s = n;
-        }
+        setup_xenheap_mappings(PFN_DOWN(bank->start),
+                               PFN_DOWN(bank->size));
     }
 
     total_pages += ram_size >> PAGE_SHIFT;
-- 
2.32.0

[PATCH 15/16] xen/arm: mm: Rework setup_xenheap_mappings()

[Julien Grall]

From: Julien Grall <julien.grall@arm.com>

The current implementation of setup_xenheap_mappings() is using 1GB
mappings. This can lead to unexpected result because the mapping
may alias a non-cachable region (such as device or reserved regions).
For more details see B2.8 in ARM DDI 0487H.a.

map_pages_to_xen() was recently reworked to allow superpage mappings,
support contiguous mapping and deal with the use of page-tables before
they are mapped.

Most of the code in setup_xenheap_mappings() is now replaced with a
single call to map_pages_to_xen().

Signed-off-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

---
    Changes in v4:
        - Fix typo
        - Add Stefano's reviewed-by

    Changes in v3:
        - Don't use 1GB mapping
        - Re-order code in setup_mm() in a separate patch

    Changes in v2:
        - New patch
---
 xen/arch/arm/mm.c | 87 ++++++++++-------------------------------------
 1 file changed, 18 insertions(+), 69 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index b019e4b35b55..65af44f42232 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -138,17 +138,6 @@ static DEFINE_PAGE_TABLE(cpu0_pgtable);
 static DEFINE_PAGE_TABLES(cpu0_dommap, DOMHEAP_SECOND_PAGES);
 #endif
 
-#ifdef CONFIG_ARM_64
-/* The first page of the first level mapping of the xenheap. The
- * subsequent xenheap first level pages are dynamically allocated, but
- * we need this one to bootstrap ourselves. */
-static DEFINE_PAGE_TABLE(xenheap_first_first);
-/* The zeroeth level slot which uses xenheap_first_first. Used because
- * setup_xenheap_mappings otherwise relies on mfn_to_virt which isn't
- * valid for a non-xenheap mapping. */
-static __initdata int xenheap_first_first_slot = -1;
-#endif
-
 /* Common pagetable leaves */
 /* Second level page tables.
  *
@@ -831,77 +820,37 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
 void __init setup_xenheap_mappings(unsigned long base_mfn,
                                    unsigned long nr_mfns)
 {
-    lpae_t *first, pte;
-    unsigned long mfn, end_mfn;
-    vaddr_t vaddr;
-
-    /* Align to previous 1GB boundary */
-    mfn = base_mfn & ~((FIRST_SIZE>>PAGE_SHIFT)-1);
+    int rc;
 
     /* First call sets the xenheap physical and virtual offset. */
     if ( mfn_eq(xenheap_mfn_start, INVALID_MFN) )
     {
+        unsigned long mfn_gb = base_mfn & ~((FIRST_SIZE >> PAGE_SHIFT) - 1);
+
         xenheap_mfn_start = _mfn(base_mfn);
         xenheap_base_pdx = mfn_to_pdx(_mfn(base_mfn));
+        /*
+         * The base address may not be aligned to the first level
+         * size (e.g. 1GB when using 4KB pages). This would prevent
+         * superpage mappings for all the regions because the virtual
+         * address and machine address should both be suitably aligned.
+         *
+         * Prevent that by offsetting the start of the xenheap virtual
+         * address.
+         */
         xenheap_virt_start = DIRECTMAP_VIRT_START +
-            (base_mfn - mfn) * PAGE_SIZE;
+            (base_mfn - mfn_gb) * PAGE_SIZE;
     }
 
     if ( base_mfn < mfn_x(xenheap_mfn_start) )
         panic("cannot add xenheap mapping at %lx below heap start %lx\n",
               base_mfn, mfn_x(xenheap_mfn_start));
 
-    end_mfn = base_mfn + nr_mfns;
-
-    /*
-     * Virtual address aligned to previous 1GB to match physical
-     * address alignment done above.
-     */
-    vaddr = (vaddr_t)__mfn_to_virt(base_mfn) & FIRST_MASK;
-
-    while ( mfn < end_mfn )
-    {
-        int slot = zeroeth_table_offset(vaddr);
-        lpae_t *p = &xen_pgtable[slot];
-
-        if ( p->pt.valid )
-        {
-            /* mfn_to_virt is not valid on the 1st 1st mfn, since it
-             * is not within the xenheap. */
-            first = slot == xenheap_first_first_slot ?
-                xenheap_first_first : mfn_to_virt(lpae_get_mfn(*p));
-        }
-        else if ( xenheap_first_first_slot == -1)
-        {
-            /* Use xenheap_first_first to bootstrap the mappings */
-            first = xenheap_first_first;
-
-            pte = pte_of_xenaddr((vaddr_t)xenheap_first_first);
-            pte.pt.table = 1;
-            write_pte(p, pte);
-
-            xenheap_first_first_slot = slot;
-        }
-        else
-        {
-            mfn_t first_mfn = alloc_boot_pages(1, 1);
-
-            clear_page(mfn_to_virt(first_mfn));
-            pte = mfn_to_xen_entry(first_mfn, MT_NORMAL);
-            pte.pt.table = 1;
-            write_pte(p, pte);
-            first = mfn_to_virt(first_mfn);
-        }
-
-        pte = mfn_to_xen_entry(_mfn(mfn), MT_NORMAL);
-        /* TODO: Set pte.pt.contig when appropriate. */
-        write_pte(&first[first_table_offset(vaddr)], pte);
-
-        mfn += FIRST_SIZE>>PAGE_SHIFT;
-        vaddr += FIRST_SIZE;
-    }
-
-    flush_xen_tlb_local();
+    rc = map_pages_to_xen((vaddr_t)__mfn_to_virt(base_mfn),
+                          _mfn(base_mfn), nr_mfns,
+                          PAGE_HYPERVISOR_RW | _PAGE_BLOCK);
+    if ( rc )
+        panic("Unable to setup the xenheap mappings.\n");
 }
 #endif
 
-- 
2.32.0

[PATCH 16/16] xen/arm: mm: Re-implement setup_frame_table_mappings() with map_pages_to_xen()

[Julien Grall]

From: Julien Grall <julien.grall@arm.com>

Now that map_pages_to_xen() has been extended to support 2MB mappings,
we can replace the create_mappings() call by map_pages_to_xen() call.

This has the advantage to remove the differences between 32-bit and
64-bit code.

Lastly remove create_mappings() as there is no more callers.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>

---
    Changes in v4:
        - Add missing _PAGE_BLOCK

    Changes in v3:
        - Fix typo in the commit message
        - Remove the TODO regarding contiguous bit

    Changes in v2:
        - New patch
---
 xen/arch/arm/mm.c | 64 +++++------------------------------------------
 1 file changed, 6 insertions(+), 58 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 65af44f42232..be37176a4725 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -369,40 +369,6 @@ void clear_fixmap(unsigned map)
     BUG_ON(res != 0);
 }
 
-/* Create Xen's mappings of memory.
- * Mapping_size must be either 2MB or 32MB.
- * Base and virt must be mapping_size aligned.
- * Size must be a multiple of mapping_size.
- * second must be a contiguous set of second level page tables
- * covering the region starting at virt_offset. */
-static void __init create_mappings(lpae_t *second,
-                                   unsigned long virt_offset,
-                                   unsigned long base_mfn,
-                                   unsigned long nr_mfns,
-                                   unsigned int mapping_size)
-{
-    unsigned long i, count;
-    const unsigned long granularity = mapping_size >> PAGE_SHIFT;
-    lpae_t pte, *p;
-
-    ASSERT((mapping_size == MB(2)) || (mapping_size == MB(32)));
-    ASSERT(!((virt_offset >> PAGE_SHIFT) % granularity));
-    ASSERT(!(base_mfn % granularity));
-    ASSERT(!(nr_mfns % granularity));
-
-    count = nr_mfns / XEN_PT_LPAE_ENTRIES;
-    p = second + second_linear_offset(virt_offset);
-    pte = mfn_to_xen_entry(_mfn(base_mfn), MT_NORMAL);
-    if ( granularity == 16 * XEN_PT_LPAE_ENTRIES )
-        pte.pt.contig = 1;  /* These maps are in 16-entry contiguous chunks. */
-    for ( i = 0; i < count; i++ )
-    {
-        write_pte(p + i, pte);
-        pte.pt.base += 1 << XEN_PT_LPAE_SHIFT;
-    }
-    flush_xen_tlb_local();
-}
-
 #ifdef CONFIG_DOMAIN_PAGE
 void *map_domain_page_global(mfn_t mfn)
 {
@@ -862,36 +828,18 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
     unsigned long frametable_size = nr_pdxs * sizeof(struct page_info);
     mfn_t base_mfn;
     const unsigned long mapping_size = frametable_size < MB(32) ? MB(2) : MB(32);
-#ifdef CONFIG_ARM_64
-    lpae_t *second, pte;
-    unsigned long nr_second;
-    mfn_t second_base;
-    int i;
-#endif
+    int rc;
 
     frametable_base_pdx = mfn_to_pdx(maddr_to_mfn(ps));
     /* Round up to 2M or 32M boundary, as appropriate. */
     frametable_size = ROUNDUP(frametable_size, mapping_size);
     base_mfn = alloc_boot_pages(frametable_size >> PAGE_SHIFT, 32<<(20-12));
 
-#ifdef CONFIG_ARM_64
-    /* Compute the number of second level pages. */
-    nr_second = ROUNDUP(frametable_size, FIRST_SIZE) >> FIRST_SHIFT;
-    second_base = alloc_boot_pages(nr_second, 1);
-    second = mfn_to_virt(second_base);
-    for ( i = 0; i < nr_second; i++ )
-    {
-        clear_page(mfn_to_virt(mfn_add(second_base, i)));
-        pte = mfn_to_xen_entry(mfn_add(second_base, i), MT_NORMAL);
-        pte.pt.table = 1;
-        write_pte(&xen_first[first_table_offset(FRAMETABLE_VIRT_START)+i], pte);
-    }
-    create_mappings(second, 0, mfn_x(base_mfn), frametable_size >> PAGE_SHIFT,
-                    mapping_size);
-#else
-    create_mappings(xen_second, FRAMETABLE_VIRT_START, mfn_x(base_mfn),
-                    frametable_size >> PAGE_SHIFT, mapping_size);
-#endif
+    rc = map_pages_to_xen(FRAMETABLE_VIRT_START, base_mfn,
+                          frametable_size >> PAGE_SHIFT,
+                          PAGE_HYPERVISOR_RW | _PAGE_BLOCK);
+    if ( rc )
+        panic("Unable to setup the frametable mappings.\n");
 
     memset(&frame_table[0], 0, nr_pdxs * sizeof(struct page_info));
     memset(&frame_table[nr_pdxs], -1,
-- 
2.32.0

Re: [PATCH 10/16] xen/arm: add Persistent Map (PMAP) infrastructure

[Julien Grall]

Hi,

On 20/05/2022 13:09, Julien Grall wrote:
> +void __init pmap_unmap(const void *p)
> +{
> +    unsigned int idx;
> +    unsigned int slot = virt_to_fix((unsigned long)p);
> +
> +    ASSERT(system_state < SYS_STATE_smp_boot);
> +    ASSERT(slot >= FIXMAP_PMAP_BEGIN && slot <= FIXMAP_PMAP_END);
> +    ASSERT(in_irq());

This needs to be ASSERT(!in_irq()).

Cheers,

-- 
Julien Grall

Re: [PATCH 13/16] xen/arm32: setup: Move out the code to populate the boot allocator

[Michal Orzel]

Hi Julien,

On 20.05.2022 14:09, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> In a follow-up patch, we will want to populate the boot allocator
> separately for arm64. The code will end up to be very similar to the one
> on arm32. So move out the code in a new helper populate_boot_allocator().
> 
> For now the code is still protected by CONFIG_ARM_32 to avoid any build
> failure on arm64.
> 
> Take the opportunity to replace mfn_add(xen_mfn_start, xenheap_pages) with
> xenheap_mfn_end as they are equivalent.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> ---
> 
>     Changes in v4:
>         - Patch added
> ---
>  xen/arch/arm/setup.c | 90 +++++++++++++++++++++++++-------------------
>  1 file changed, 51 insertions(+), 39 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index d5d0792ed48a..3d5a2283d4ef 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -637,10 +637,58 @@ static void __init init_staticmem_pages(void)
>  }
>  
>  #ifdef CONFIG_ARM_32
> +/*
> + * Populate the boot allocator. All the RAM but the following regions
> + * will be added:
> + *  - Modules (e.g., Xen, Kernel)
> + *  - Reserved regions
> + *  - Xenheap
> + */
> +static void __init populate_boot_allocator(void)
> +{
> +    unsigned int i;
Shouldn't this be an int (as it was previously) because ...
> +    const struct meminfo *banks = &bootinfo.mem;
> +
> +    for ( i = 0; i < banks->nr_banks; i++ )
... nr_banks is int ?

Apart from that:
Reviewed-by: Michal Orzel <michal.orzel@arm.com>

Cheers,
Michal

Re: [PATCH 13/16] xen/arm32: setup: Move out the code to populate the boot allocator

[Julien Grall]

On 23/05/2022 08:28, Michal Orzel wrote:
> Hi Julien,

Hi Michal,

> 
> On 20.05.2022 14:09, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> In a follow-up patch, we will want to populate the boot allocator
>> separately for arm64. The code will end up to be very similar to the one
>> on arm32. So move out the code in a new helper populate_boot_allocator().
>>
>> For now the code is still protected by CONFIG_ARM_32 to avoid any build
>> failure on arm64.
>>
>> Take the opportunity to replace mfn_add(xen_mfn_start, xenheap_pages) with
>> xenheap_mfn_end as they are equivalent.
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>>
>> ---
>>
>>      Changes in v4:
>>          - Patch added
>> ---
>>   xen/arch/arm/setup.c | 90 +++++++++++++++++++++++++-------------------
>>   1 file changed, 51 insertions(+), 39 deletions(-)
>>
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index d5d0792ed48a..3d5a2283d4ef 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -637,10 +637,58 @@ static void __init init_staticmem_pages(void)
>>   }
>>   
>>   #ifdef CONFIG_ARM_32
>> +/*
>> + * Populate the boot allocator. All the RAM but the following regions
>> + * will be added:
>> + *  - Modules (e.g., Xen, Kernel)
>> + *  - Reserved regions
>> + *  - Xenheap
>> + */
>> +static void __init populate_boot_allocator(void)
>> +{
>> +    unsigned int i;
> Shouldn't this be an int (as it was previously) because ...
>> +    const struct meminfo *banks = &bootinfo.mem;
>> +
>> +    for ( i = 0; i < banks->nr_banks; i++ )
> ... nr_banks is int ?

Hmmm... AFAIK banks->nr_banks never hold a negative value, so I am not 
sure why it was introduced as an "int".

Looking through the code, we seem to have a mix of "unsigned int" and 
"int". There seem to be less on the latter, so I have sent a patch to 
switch nr_banks to "unsigned int" [1].

This is based on this series thought and I would like to keep the 
"unsigned int" here.

> 
> Apart from that:
> Reviewed-by: Michal Orzel <michal.orzel@arm.com>

Thanks! Please let me know if this reviewed-by hold.

Cheers,

[1] https://lore.kernel.org/xen-devel/20220523194631.66262-1-julien@xen.org

-- 
Julien Grall

Re: [PATCH 10/16] xen/arm: add Persistent Map (PMAP) infrastructure

[Wei Chen]

Hi Julien,

On 2022/5/20 20:09, Julien Grall wrote:
> From: Wei Liu <wei.liu2@citrix.com>
> 
> The basic idea is like Persistent Kernel Map (PKMAP) in Linux. We
> pre-populate all the relevant page tables before the system is fully
> set up.
> 
> We will need it on Arm in order to rework the arm64 version of
> xenheap_setup_mappings() as we may need to use pages allocated from
> the boot allocator before they are effectively mapped.
> 
> This infrastructure is not lock-protected therefore can only be used
> before smpboot. After smpboot, map_domain_page() has to be used.
> 
> This is based on the x86 version [1] that was originally implemented
> by Wei Liu.
> 
> The PMAP infrastructure is implemented in common code with some
> arch helpers to set/clear the page-table entries and convertion
> between a fixmap slot to a virtual address...
> 
> As mfn_to_xen_entry() now needs to be exported, take the opportunity
> to swich the parameter attr from unsigned to unsigned int.
> 
> [1] <e92da4ad6015b6089737fcccba3ec1d6424649a5.1588278317.git.hongyxia@amazon.com>
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Signed-off-by: Hongyan Xia <hongyxia@amazon.com>
> [julien: Adapted for Arm]
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> ---
>      Changes in v4:
>          - Move xen_fixmap in fixmap.h and add a comment about its usage.
>          - Update comments
>          - Use DECLARE_BITMAP()
>          - Replace local_irq_{enable, disable} with an ASSERT() as there
>            should be no user of pmap() in interrupt context.
> 
>      Changes in v3:
>          - s/BITS_PER_LONG/BITS_PER_BYTE/
>          - Move pmap to common code
> 
>      Changes in v2:
>          - New patch
> 
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Wei Liu <wl@xen.org>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Roger Pau Monné <roger.pau@citrix.com>
> ---
>   xen/arch/arm/Kconfig              |  1 +
>   xen/arch/arm/include/asm/fixmap.h | 24 +++++++++++
>   xen/arch/arm/include/asm/lpae.h   |  8 ++++
>   xen/arch/arm/include/asm/pmap.h   | 32 ++++++++++++++
>   xen/arch/arm/mm.c                 |  7 +--
>   xen/common/Kconfig                |  3 ++
>   xen/common/Makefile               |  1 +
>   xen/common/pmap.c                 | 72 +++++++++++++++++++++++++++++++
>   xen/include/xen/pmap.h            | 16 +++++++
>   9 files changed, 158 insertions(+), 6 deletions(-)
>   create mode 100644 xen/arch/arm/include/asm/pmap.h
>   create mode 100644 xen/common/pmap.c
>   create mode 100644 xen/include/xen/pmap.h
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index ecfa6822e4d3..a89a67802aa9 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -14,6 +14,7 @@ config ARM
>   	select HAS_DEVICE_TREE
>   	select HAS_PASSTHROUGH
>   	select HAS_PDX
> +	select HAS_PMAP
>   	select IOMMU_FORCE_PT_SHARE
>   
>   config ARCH_DEFCONFIG
> diff --git a/xen/arch/arm/include/asm/fixmap.h b/xen/arch/arm/include/asm/fixmap.h
> index 1cee51e52ab9..365a2385a087 100644
> --- a/xen/arch/arm/include/asm/fixmap.h
> +++ b/xen/arch/arm/include/asm/fixmap.h
> @@ -5,20 +5,44 @@
>   #define __ASM_FIXMAP_H
>   
>   #include <xen/acpi.h>
> +#include <xen/pmap.h>
>   
>   /* Fixmap slots */
>   #define FIXMAP_CONSOLE  0  /* The primary UART */
>   #define FIXMAP_MISC     1  /* Ephemeral mappings of hardware */
>   #define FIXMAP_ACPI_BEGIN  2  /* Start mappings of ACPI tables */
>   #define FIXMAP_ACPI_END    (FIXMAP_ACPI_BEGIN + NUM_FIXMAP_ACPI_PAGES - 1)  /* End mappings of ACPI tables */
> +#define FIXMAP_PMAP_BEGIN (FIXMAP_ACPI_END + 1) /* Start of PMAP */
> +#define FIXMAP_PMAP_END (FIXMAP_PMAP_BEGIN + NUM_FIX_PMAP - 1) /* End of PMAP */
> +
> +#define FIXMAP_LAST FIXMAP_PMAP_END
> +
> +#define FIXADDR_START FIXMAP_ADDR(0)
> +#define FIXADDR_TOP FIXMAP_ADDR(FIXMAP_LAST)
>   
>   #ifndef __ASSEMBLY__
>   
> +/*
> + * Direct access to xen_fixmap[] should only happen when {set,
> + * clear}_fixmap() is unusable (e.g. where we would end up to
> + * recursively call the helpers).
> + */
> +extern lpae_t xen_fixmap[XEN_PT_LPAE_ENTRIES];
> +
>   /* Map a page in a fixmap entry */
>   extern void set_fixmap(unsigned map, mfn_t mfn, unsigned attributes);
>   /* Remove a mapping from a fixmap entry */
>   extern void clear_fixmap(unsigned map);
>   
> +#define fix_to_virt(slot) ((void *)FIXMAP_ADDR(slot))
> +
> +static inline unsigned int virt_to_fix(vaddr_t vaddr)
> +{
> +    BUG_ON(vaddr >= FIXADDR_TOP || vaddr < FIXADDR_START);
> +
> +    return ((vaddr - FIXADDR_START) >> PAGE_SHIFT);
> +}
> +
>   #endif /* __ASSEMBLY__ */
>   
>   #endif /* __ASM_FIXMAP_H */
> diff --git a/xen/arch/arm/include/asm/lpae.h b/xen/arch/arm/include/asm/lpae.h
> index aecb320dec45..fc19cbd84772 100644
> --- a/xen/arch/arm/include/asm/lpae.h
> +++ b/xen/arch/arm/include/asm/lpae.h
> @@ -4,6 +4,7 @@
>   #ifndef __ASSEMBLY__
>   
>   #include <xen/page-defs.h>
> +#include <xen/mm-frame.h>
>   
>   /*
>    * WARNING!  Unlike the x86 pagetable code, where l1 is the lowest level and
> @@ -168,6 +169,13 @@ static inline bool lpae_is_superpage(lpae_t pte, unsigned int level)
>           third_table_offset(addr)            \
>       }
>   
> +/*
> + * 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.
> + */
> +lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned int attr);
> +
>   #endif /* __ASSEMBLY__ */
>   
>   /*
> diff --git a/xen/arch/arm/include/asm/pmap.h b/xen/arch/arm/include/asm/pmap.h
> new file mode 100644
> index 000000000000..74398b4c4fe6
> --- /dev/null
> +++ b/xen/arch/arm/include/asm/pmap.h
> @@ -0,0 +1,32 @@
> +#ifndef __ASM_PMAP_H__
> +#define __ASM_PMAP_H__
> +
> +#include <xen/mm.h>
> +
> +#include <asm/fixmap.h>
> +
> +static inline void arch_pmap_map(unsigned int slot, mfn_t mfn)
> +{
> +    lpae_t *entry = &xen_fixmap[slot];
> +    lpae_t pte;
> +
> +    ASSERT(!lpae_is_valid(*entry));
> +

Sometimes it is very difficult for me to determine whether to
use ASSERT or fixed check in this situation. In debug=n config,
is there any risk of pte override of arch_pmap_map should be
prevented? IMO, it's better to provide a return value for this
function and use a fixed check here.

> +    pte = mfn_to_xen_entry(mfn, PAGE_HYPERVISOR_RW);
> +    pte.pt.table = 1;
> +    write_pte(entry, pte);
> +}
> +
> +static inline void arch_pmap_unmap(unsigned int slot)
> +{
> +    lpae_t pte = {};
> +

We have checked lpae_is_valid() in arch_pmap_map. So can we add a
!lpae_is_valid check here and return directly?

> +    write_pte(&xen_fixmap[slot], pte);
> +
> +    flush_xen_tlb_range_va_local(FIXMAP_ADDR(slot), PAGE_SIZE);
> +}
> +
> +void arch_pmap_map_slot(unsigned int slot, mfn_t mfn);
> +void arch_pmap_clear_slot(void *ptr);
> +
> +#endif /* __ASM_PMAP_H__ */
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 52b2a0394047..bd1348a99716 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -305,12 +305,7 @@ 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(mfn_t mfn, unsigned attr)
> +lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned int attr)
>   {
>       lpae_t e = (lpae_t) {
>           .pt = {
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index d921c74d615e..5b6b2406c028 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -49,6 +49,9 @@ config HAS_KEXEC
>   config HAS_PDX
>   	bool
>   
> +config HAS_PMAP
> +	bool
> +
>   config HAS_SCHED_GRANULARITY
>   	bool
>   
> diff --git a/xen/common/Makefile b/xen/common/Makefile
> index b1e076c30b81..3baf83d527d8 100644
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -29,6 +29,7 @@ obj-y += notifier.o
>   obj-y += page_alloc.o
>   obj-$(CONFIG_HAS_PDX) += pdx.o
>   obj-$(CONFIG_PERF_COUNTERS) += perfc.o
> +obj-bin-$(CONFIG_HAS_PMAP) += pmap.init.o
>   obj-y += preempt.o
>   obj-y += random.o
>   obj-y += rangeset.o
> diff --git a/xen/common/pmap.c b/xen/common/pmap.c
> new file mode 100644
> index 000000000000..9355cacb7373
> --- /dev/null
> +++ b/xen/common/pmap.c
> @@ -0,0 +1,72 @@
> +#include <xen/bitops.h>
> +#include <xen/init.h>
> +#include <xen/irq.h>
> +#include <xen/pmap.h>
> +
> +#include <asm/pmap.h>
> +#include <asm/fixmap.h>
> +
> +/*
> + * Simple mapping infrastructure to map / unmap pages in fixed map.
> + * This is used to set the page table before the map domain page infrastructure
> + * is initialized.
> + *
> + * This structure is not protected by any locks, so it must not be used after
> + * smp bring-up.
> + */
> +
> +/* Bitmap to track which slot is used */
> +static __initdata DECLARE_BITMAP(inuse, NUM_FIX_PMAP);
> +
> +void *__init pmap_map(mfn_t mfn)
> +{
> +    unsigned int idx;
> +    unsigned int slot;
> +
> +    ASSERT(system_state < SYS_STATE_smp_boot);
> +    ASSERT(!in_irq());
> +
> +    idx = find_first_zero_bit(inuse, NUM_FIX_PMAP);
> +    if ( idx == NUM_FIX_PMAP )
> +        panic("Out of PMAP slots\n");
> +
> +    __set_bit(idx, inuse);
> +
> +    slot = idx + FIXMAP_PMAP_BEGIN;
> +    ASSERT(slot >= FIXMAP_PMAP_BEGIN && slot <= FIXMAP_PMAP_END);
> +
> +    /*
> +     * We cannot use set_fixmap() here. We use PMAP when the domain map
> +     * page infrastructure is not yet initialized, so map_pages_to_xen() called
> +     * by set_fixmap() needs to map pages on demand, which then calls pmap()
> +     * again, resulting in a loop. Modify the PTEs directly instead. The same
> +     * is true for pmap_unmap().
> +     */
> +    arch_pmap_map(slot, mfn);
> +
> +    return fix_to_virt(slot);
> +}
> +
> +void __init pmap_unmap(const void *p)
> +{
> +    unsigned int idx;
> +    unsigned int slot = virt_to_fix((unsigned long)p);
> +
> +    ASSERT(system_state < SYS_STATE_smp_boot);
> +    ASSERT(slot >= FIXMAP_PMAP_BEGIN && slot <= FIXMAP_PMAP_END);
> +    ASSERT(in_irq());
> +

Why this condition is in_irq? Is it for TLB operation in arch_pmap_unmap?

Cheers,
Wei Chen

> +    idx = slot - FIXMAP_PMAP_BEGIN;
> +
> +    __clear_bit(idx, inuse);
> +    arch_pmap_unmap(slot);
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/xen/pmap.h b/xen/include/xen/pmap.h
> new file mode 100644
> index 000000000000..93e61b10870e
> --- /dev/null
> +++ b/xen/include/xen/pmap.h
> @@ -0,0 +1,16 @@
> +#ifndef __XEN_PMAP_H__
> +#define __XEN_PMAP_H__
> +
> +/* Large enough for mapping 5 levels of page tables with some headroom */
> +#define NUM_FIX_PMAP 8
> +
> +#ifndef __ASSEMBLY__
> +
> +#include <xen/mm-frame.h>
> +
> +void *pmap_map(mfn_t mfn);
> +void pmap_unmap(const void *p);
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* __XEN_PMAP_H__ */

Re: [PATCH 13/16] xen/arm32: setup: Move out the code to populate the boot allocator

[Michal Orzel]

On 23.05.2022 21:51, Julien Grall wrote:
> 
> 
> On 23/05/2022 08:28, Michal Orzel wrote:
>> Hi Julien,
> 
> Hi Michal,
> 
>>
>> On 20.05.2022 14:09, Julien Grall wrote:
>>> From: Julien Grall <jgrall@amazon.com>
>>>
>>> In a follow-up patch, we will want to populate the boot allocator
>>> separately for arm64. The code will end up to be very similar to the one
>>> on arm32. So move out the code in a new helper populate_boot_allocator().
>>>
>>> For now the code is still protected by CONFIG_ARM_32 to avoid any build
>>> failure on arm64.
>>>
>>> Take the opportunity to replace mfn_add(xen_mfn_start, xenheap_pages) with
>>> xenheap_mfn_end as they are equivalent.
>>>
>>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>>>
>>> ---
>>>
>>>      Changes in v4:
>>>          - Patch added
>>> ---
>>>   xen/arch/arm/setup.c | 90 +++++++++++++++++++++++++-------------------
>>>   1 file changed, 51 insertions(+), 39 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>>> index d5d0792ed48a..3d5a2283d4ef 100644
>>> --- a/xen/arch/arm/setup.c
>>> +++ b/xen/arch/arm/setup.c
>>> @@ -637,10 +637,58 @@ static void __init init_staticmem_pages(void)
>>>   }
>>>     #ifdef CONFIG_ARM_32
>>> +/*
>>> + * Populate the boot allocator. All the RAM but the following regions
>>> + * will be added:
>>> + *  - Modules (e.g., Xen, Kernel)
>>> + *  - Reserved regions
>>> + *  - Xenheap
>>> + */
>>> +static void __init populate_boot_allocator(void)
>>> +{
>>> +    unsigned int i;
>> Shouldn't this be an int (as it was previously) because ...
>>> +    const struct meminfo *banks = &bootinfo.mem;
>>> +
>>> +    for ( i = 0; i < banks->nr_banks; i++ )
>> ... nr_banks is int ?
> 
> Hmmm... AFAIK banks->nr_banks never hold a negative value, so I am not sure why it was introduced as an "int".
> 
> Looking through the code, we seem to have a mix of "unsigned int" and "int". There seem to be less on the latter, so I have sent a patch to switch nr_banks to "unsigned int" [1].
That's great, thanks.

> 
> This is based on this series thought and I would like to keep the "unsigned int" here.
> 
>>
>> Apart from that:
>> Reviewed-by: Michal Orzel <michal.orzel@arm.com>
> 
> Thanks! Please let me know if this reviewed-by hold.
Definitely yes.
> 
> Cheers,
> 
> [1] https://lore.kernel.org/xen-devel/20220523194631.66262-1-julien@xen.org
> 

Cheers,
Michal

Re: [PATCH 13/16] xen/arm32: setup: Move out the code to populate the boot allocator

[Bertrand Marquis]

Hi Julien,

> On 20 May 2022, at 13:09, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> In a follow-up patch, we will want to populate the boot allocator
> separately for arm64. The code will end up to be very similar to the one
> on arm32. So move out the code in a new helper populate_boot_allocator().
> 
> For now the code is still protected by CONFIG_ARM_32 to avoid any build
> failure on arm64.
> 
> Take the opportunity to replace mfn_add(xen_mfn_start, xenheap_pages) with
> xenheap_mfn_end as they are equivalent.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> 
> ---
> 
>    Changes in v4:
>        - Patch added
> ---
> xen/arch/arm/setup.c | 90 +++++++++++++++++++++++++-------------------
> 1 file changed, 51 insertions(+), 39 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index d5d0792ed48a..3d5a2283d4ef 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -637,10 +637,58 @@ static void __init init_staticmem_pages(void)
> }
> 
> #ifdef CONFIG_ARM_32
> +/*
> + * Populate the boot allocator. All the RAM but the following regions
> + * will be added:
> + *  - Modules (e.g., Xen, Kernel)
> + *  - Reserved regions
> + *  - Xenheap
> + */
> +static void __init populate_boot_allocator(void)
> +{
> +    unsigned int i;
> +    const struct meminfo *banks = &bootinfo.mem;
> +
> +    for ( i = 0; i < banks->nr_banks; i++ )
> +    {
> +        const struct membank *bank = &banks->bank[i];
> +        paddr_t bank_end = bank->start + bank->size;
> +        paddr_t s, e;
> +
> +        s = bank->start;
> +        while ( s < bank_end )
> +        {
> +            paddr_t n = bank_end;
> +
> +            e = next_module(s, &n);
> +
> +            if ( e == ~(paddr_t)0 )
> +                e = n = bank_end;
> +
> +            /*
> +             * Module in a RAM bank other than the one which we are
> +             * not dealing with here.
> +             */
> +            if ( e > bank_end )
> +                e = bank_end;
> +
> +            /* Avoid the xenheap */
> +            if ( s < mfn_to_maddr(xenheap_mfn_end) &&
> +                 mfn_to_maddr(xenheap_mfn_start) < e )
> +            {
> +                e = mfn_to_maddr(xenheap_mfn_start);
> +                n = mfn_to_maddr(xenheap_mfn_end);
> +            }
> +
> +            fw_unreserved_regions(s, e, init_boot_pages, 0);
> +            s = n;
> +        }
> +    }
> +}
> +
> static void __init setup_mm(void)
> {
> -    paddr_t ram_start, ram_end, ram_size;
> -    paddr_t s, e;
> +    paddr_t ram_start, ram_end, ram_size, e;
>     unsigned long ram_pages;
>     unsigned long heap_pages, xenheap_pages, domheap_pages;
>     int i;
> @@ -718,43 +766,7 @@ static void __init setup_mm(void)
>     setup_xenheap_mappings((e >> PAGE_SHIFT) - xenheap_pages, xenheap_pages);
> 
>     /* Add non-xenheap memory */
> -    for ( i = 0; i < bootinfo.mem.nr_banks; i++ )
> -    {
> -        paddr_t bank_start = bootinfo.mem.bank[i].start;
> -        paddr_t bank_end = bank_start + bootinfo.mem.bank[i].size;
> -
> -        s = bank_start;
> -        while ( s < bank_end )
> -        {
> -            paddr_t n = bank_end;
> -
> -            e = next_module(s, &n);
> -
> -            if ( e == ~(paddr_t)0 )
> -            {
> -                e = n = ram_end;
> -            }
> -
> -            /*
> -             * Module in a RAM bank other than the one which we are
> -             * not dealing with here.
> -             */
> -            if ( e > bank_end )
> -                e = bank_end;
> -
> -            /* Avoid the xenheap */
> -            if ( s < mfn_to_maddr(mfn_add(xenheap_mfn_start, xenheap_pages))
> -                 && mfn_to_maddr(xenheap_mfn_start) < e )
> -            {
> -                e = mfn_to_maddr(xenheap_mfn_start);
> -                n = mfn_to_maddr(mfn_add(xenheap_mfn_start, xenheap_pages));
> -            }
> -
> -            fw_unreserved_regions(s, e, init_boot_pages, 0);
> -
> -            s = n;
> -        }
> -    }
> +    populate_boot_allocator();
> 
>     /* Frame table covers all of RAM region, including holes */
>     setup_frametable_mappings(ram_start, ram_end);
> -- 
> 2.32.0
> 

Re: [PATCH 10/16] xen/arm: add Persistent Map (PMAP) infrastructure

[Julien Grall]

On 24/05/2022 03:11, Wei Chen wrote:
> Hi Julien,

Hi Wei,

>> diff --git a/xen/arch/arm/include/asm/pmap.h 
>> b/xen/arch/arm/include/asm/pmap.h
>> new file mode 100644
>> index 000000000000..74398b4c4fe6
>> --- /dev/null
>> +++ b/xen/arch/arm/include/asm/pmap.h
>> @@ -0,0 +1,32 @@
>> +#ifndef __ASM_PMAP_H__
>> +#define __ASM_PMAP_H__
>> +
>> +#include <xen/mm.h>
>> +
>> +#include <asm/fixmap.h>
>> +
>> +static inline void arch_pmap_map(unsigned int slot, mfn_t mfn)
>> +{
>> +    lpae_t *entry = &xen_fixmap[slot];
>> +    lpae_t pte;
>> +
>> +    ASSERT(!lpae_is_valid(*entry));
>> +
> 
> Sometimes it is very difficult for me to determine whether to
> use ASSERT or fixed check in this situation. In debug=n config,
> is there any risk of pte override of arch_pmap_map should be
> prevented? 

There is always a risk :). In this case, this would be a programming 
error if the slot contains a valid entry. Hence why an ASSERT() (They 
tend to be use for programming error).

> IMO, it's better to provide a return value for this
> function and use a fixed check here.
As I wrote above, arch_pmap_map() is not meant to be called in such 
situation. If we return an error, then there are a lot more churn 
necessary (pmap_map() would now need to return NULL...) for something 
that is never meant to happen.

> 
>> +    pte = mfn_to_xen_entry(mfn, PAGE_HYPERVISOR_RW);
>> +    pte.pt.table = 1;
>> +    write_pte(entry, pte);
>> +}
>> +
>> +static inline void arch_pmap_unmap(unsigned int slot)
>> +{
>> +    lpae_t pte = {};
>> +
> 
> We have checked lpae_is_valid() in arch_pmap_map. So can we add a
> !lpae_is_valid check here and return directly?
The code below can work with invalid entry and this function is not 
meant to be called in such case.

So to me this is sounds like an unnecessary optimization.

>> +void __init pmap_unmap(const void *p)
>> +{
>> +    unsigned int idx;
>> +    unsigned int slot = virt_to_fix((unsigned long)p);
>> +
>> +    ASSERT(system_state < SYS_STATE_smp_boot);
>> +    ASSERT(slot >= FIXMAP_PMAP_BEGIN && slot <= FIXMAP_PMAP_END);
>> +    ASSERT(in_irq());
>> +
> 
> Why this condition is in_irq?

This should be !in_irq().

> Is it for TLB operation in arch_pmap_unmap?

No. pmap_{map, unmap} are not re-entreant. So we have two choices here:
  1) Forbid the helpers to be used in IRQ context
  2) Use local_irq_{disable, enable}

I originally used the caller but given that are no users in IRQ 
contexts, I went with 1.

Cheers,

-- 
Julien Grall

Re: [PATCH 03/16] xen/arm: mm: Avoid flushing the TLBs when mapping are inserted

[Luca Fancellu]

> On 20 May 2022, at 13:09, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> Currently, the function xen_pt_update() will flush the TLBs even when
> the mappings are inserted. This is a bit wasteful because we don't
> allow mapping replacement. Even if we were, the flush would need to
> happen earlier because mapping replacement should use Break-Before-Make
> when updating the entry.
> 
> A single call to xen_pt_update() can perform a single action. IOW, it
> is not possible to, for instance, mix inserting and removing mappings.
> Therefore, we can use `flags` to determine what action is performed.
> 
> This change will be particularly help to limit the impact of switching
> boot time mapping to use xen_pt_update().
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Hi Julien,

It looks ok to me. I’ve also tested it starting, console-ing, destroying few guests
and I’ve got no problem

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>

Re: [PATCH 10/16] xen/arm: add Persistent Map (PMAP) infrastructure

[Luca Fancellu]

> On 20 May 2022, at 13:09, Julien Grall <julien@xen.org> wrote:
> 
> From: Wei Liu <wei.liu2@citrix.com>
> 
> The basic idea is like Persistent Kernel Map (PKMAP) in Linux. We
> pre-populate all the relevant page tables before the system is fully
> set up.
> 
> We will need it on Arm in order to rework the arm64 version of
> xenheap_setup_mappings() as we may need to use pages allocated from
> the boot allocator before they are effectively mapped.
> 
> This infrastructure is not lock-protected therefore can only be used
> before smpboot. After smpboot, map_domain_page() has to be used.
> 
> This is based on the x86 version [1] that was originally implemented
> by Wei Liu.
> 
> The PMAP infrastructure is implemented in common code with some
> arch helpers to set/clear the page-table entries and convertion
> between a fixmap slot to a virtual address...
> 
> As mfn_to_xen_entry() now needs to be exported, take the opportunity
> to swich the parameter attr from unsigned to unsigned int.
> 
> [1] <e92da4ad6015b6089737fcccba3ec1d6424649a5.1588278317.git.hongyxia@amazon.com>
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Signed-off-by: Hongyan Xia <hongyxia@amazon.com>
> [julien: Adapted for Arm]
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Hi Julien,

with ASSERT(!in_irq()) in pmap_unmap(const void *p) as you previously say.

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

I’ve also tested patches up to this one, start/destroying/connecting-to few guests
and no problem.

Tested-by: Luca Fancellu <luca.fancellu@arm.com>

Re: [PATCH 14/16] xen/arm64: mm: Add memory to the boot allocator first

[Luca Fancellu]

> On 20 May 2022, at 13:09, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> Currently, memory is added to the boot allocator after the xenheap
> mappings are done. This will break if the first mapping is more than
> 512GB of RAM.
> 
> In addition to that, a follow-up patch will rework setup_xenheap_mappings()
> to use smaller mappings (e.g. 2MB, 4KB). So it will be necessary to have
> memory in the boot allocator earlier.
> 
> Only free memory (e.g. not reserved or modules) can be added to the boot
> allocator. It might be possible that some regions (including the first
> one) will have no free memory.
> 
> So we need to add all the free memory to the boot allocator first
> and then add do the mappings.
> 
> Populating the boot allocator is nearly the same between arm32 and
> arm64. The only difference is on the former we need to exclude the
> xenheap for the boot allocator. Gate the difference with CONFIG_ARM_32
> so the code be re-used on arm64.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 

Hi Julien,

Seems ok to me!

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

I’ve also tested on arm64 patches until this one and no problem.

Tested-by: Luca Fancellu <luca.fancellu@arm.com>

Re: [PATCH 16/16] xen/arm: mm: Re-implement setup_frame_table_mappings() with map_pages_to_xen()

[Luca Fancellu]

> On 20 May 2022, at 13:09, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <julien.grall@arm.com>
> 
> Now that map_pages_to_xen() has been extended to support 2MB mappings,
> we can replace the create_mappings() call by map_pages_to_xen() call.
> 
> This has the advantage to remove the differences between 32-bit and
> 64-bit code.
> 
> Lastly remove create_mappings() as there is no more callers.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 

Hi Julien,

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

I’ve also tested all patches including this one on arm64, booting Xen+Dom0
and starting few guests, connecting consoles, destroying, doing networking,
no problem so far.

Tested-by: Luca Fancellu <luca.fancellu@arm.com>

Re: [PATCH 01/16] xen/arm: mm: Allow other mapping size in xen_pt_update_entry()

[Stefano Stabellini]

On Fri, 20 May 2022, Julien Grall wrote:
> From: Julien Grall <julien.grall@arm.com>
> 
> At the moment, xen_pt_update_entry() only supports mapping at level 3
> (i.e 4KB mapping). While this is fine for most of the runtime helper,
> the boot code will require to use superpage mapping.
> 
> We don't want to allow superpage mapping by default as some of the
> callers may expect small mappings (i.e populate_pt_range()) or even
> expect to unmap only a part of a superpage.
> 
> To keep the code simple, a new flag _PAGE_BLOCK is introduced to
> allow the caller to enable superpage mapping.
> 
> As the code doesn't support all the combinations, xen_pt_check_entry()
> is extended to take into account the cases we don't support when
> using block mapping:
>     - Replacing a table with a mapping. This may happen if region was
>     first mapped with 4KB mapping and then later on replaced with a 2MB
>     (or 1GB mapping).
>     - Removing/modifying a table. This may happen if a caller try to
>     remove a region with _PAGE_BLOCK set when it was created without it.
> 
> Note that the current restriction means that the caller must ensure that
> _PAGE_BLOCK is consistently set/cleared across all the updates on a
> given virtual region. This ought to be fine with the expected use-cases.
> 
> More rework will be necessary if we wanted to remove the restrictions.
> 
> Note that nr_mfns is now marked const as it is used for flushing the
> TLBs and we don't want it to be modified.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> Reviewed-by: Hongda Deng <Hongda.Heng@arm.com>

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


> ---
>     Changes in v4:
>         - Add Hongda's reviewed-by
>         - Add a comment why nr_mfns is const
>         - Open-code pfn_to_paddr()
> 
>     Changes in v3:
>         - Fix clash after prefixing the PT macros with XEN_PT_
>         - Fix typoes in the commit message
>         - Support superpage mappings even if nr is not suitably aligned
>         - Move the logic to find the level in a separate function
> 
>     Changes in v2:
>         - Pass the target level rather than the order to
>         xen_pt_update_entry()
>         - Update some comments
>         - Open-code paddr_to_pfn()
>         - Add my AWS signed-off-by
> ---
>  xen/arch/arm/include/asm/page.h |   4 ++
>  xen/arch/arm/mm.c               | 109 ++++++++++++++++++++++++++------
>  2 files changed, 95 insertions(+), 18 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/page.h b/xen/arch/arm/include/asm/page.h
> index c6f9fb0d4e0c..07998df47bac 100644
> --- a/xen/arch/arm/include/asm/page.h
> +++ b/xen/arch/arm/include/asm/page.h
> @@ -69,6 +69,7 @@
>   * [3:4] Permission flags
>   * [5]   Page present
>   * [6]   Only populate page tables
> + * [7]   Superpage mappings is allowed
>   */
>  #define PAGE_AI_MASK(x) ((x) & 0x7U)
>  
> @@ -82,6 +83,9 @@
>  #define _PAGE_PRESENT    (1U << 5)
>  #define _PAGE_POPULATE   (1U << 6)
>  
> +#define _PAGE_BLOCK_BIT     7
> +#define _PAGE_BLOCK         (1U << _PAGE_BLOCK_BIT)
> +
>  /*
>   * _PAGE_DEVICE and _PAGE_NORMAL are convenience defines. They are not
>   * meant to be used outside of this header.
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 7b1f2f49060d..be2ac302d731 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1078,9 +1078,10 @@ static int xen_pt_next_level(bool read_only, unsigned int level,
>  }
>  
>  /* Sanity check of the entry */
> -static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
> +static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int level,
> +                               unsigned int flags)
>  {
> -    /* Sanity check when modifying a page. */
> +    /* Sanity check when modifying an entry. */
>      if ( (flags & _PAGE_PRESENT) && mfn_eq(mfn, INVALID_MFN) )
>      {
>          /* We don't allow modifying an invalid entry. */
> @@ -1090,6 +1091,13 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
>              return false;
>          }
>  
> +        /* We don't allow modifying a table entry */
> +        if ( !lpae_is_mapping(entry, level) )
> +        {
> +            mm_printk("Modifying a table entry is not allowed.\n");
> +            return false;
> +        }
> +
>          /* We don't allow changing memory attributes. */
>          if ( entry.pt.ai != PAGE_AI_MASK(flags) )
>          {
> @@ -1105,7 +1113,7 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
>              return false;
>          }
>      }
> -    /* Sanity check when inserting a page */
> +    /* Sanity check when inserting a mapping */
>      else if ( flags & _PAGE_PRESENT )
>      {
>          /* We should be here with a valid MFN. */
> @@ -1114,18 +1122,28 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
>          /* We don't allow replacing any valid entry. */
>          if ( lpae_is_valid(entry) )
>          {
> -            mm_printk("Changing MFN for a valid entry is not allowed (%#"PRI_mfn" -> %#"PRI_mfn").\n",
> -                      mfn_x(lpae_get_mfn(entry)), mfn_x(mfn));
> +            if ( lpae_is_mapping(entry, level) )
> +                mm_printk("Changing MFN for a valid entry is not allowed (%#"PRI_mfn" -> %#"PRI_mfn").\n",
> +                          mfn_x(lpae_get_mfn(entry)), mfn_x(mfn));
> +            else
> +                mm_printk("Trying to replace a table with a mapping.\n");
>              return false;
>          }
>      }
> -    /* Sanity check when removing a page. */
> +    /* Sanity check when removing a mapping. */
>      else if ( (flags & (_PAGE_PRESENT|_PAGE_POPULATE)) == 0 )
>      {
>          /* We should be here with an invalid MFN. */
>          ASSERT(mfn_eq(mfn, INVALID_MFN));
>  
> -        /* We don't allow removing page with contiguous bit set. */
> +        /* We don't allow removing a table */
> +        if ( lpae_is_table(entry, level) )
> +        {
> +            mm_printk("Removing a table is not allowed.\n");
> +            return false;
> +        }
> +
> +        /* We don't allow removing a mapping with contiguous bit set. */
>          if ( entry.pt.contig )
>          {
>              mm_printk("Removing entry with contiguous bit set is not allowed.\n");
> @@ -1143,13 +1161,13 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
>      return true;
>  }
>  
> +/* Update an entry at the level @target. */
>  static int xen_pt_update_entry(mfn_t root, unsigned long virt,
> -                               mfn_t mfn, unsigned int flags)
> +                               mfn_t mfn, unsigned int target,
> +                               unsigned int flags)
>  {
>      int rc;
>      unsigned int level;
> -    /* We only support 4KB mapping (i.e level 3) for now */
> -    unsigned int target = 3;
>      lpae_t *table;
>      /*
>       * The intermediate page tables are read-only when the MFN is not valid
> @@ -1204,7 +1222,7 @@ static int xen_pt_update_entry(mfn_t root, unsigned long virt,
>      entry = table + offsets[level];
>  
>      rc = -EINVAL;
> -    if ( !xen_pt_check_entry(*entry, mfn, flags) )
> +    if ( !xen_pt_check_entry(*entry, mfn, level, flags) )
>          goto out;
>  
>      /* If we are only populating page-table, then we are done. */
> @@ -1222,8 +1240,11 @@ static int xen_pt_update_entry(mfn_t root, unsigned long virt,
>          {
>              pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
>  
> -            /* Third level entries set pte.pt.table = 1 */
> -            pte.pt.table = 1;
> +            /*
> +             * First and second level pages set pte.pt.table = 0, but
> +             * third level entries set pte.pt.table = 1.
> +             */
> +            pte.pt.table = (level == 3);
>          }
>          else /* We are updating the permission => Copy the current pte. */
>              pte = *entry;
> @@ -1243,15 +1264,57 @@ out:
>      return rc;
>  }
>  
> +/* Return the level where mapping should be done */
> +static int xen_pt_mapping_level(unsigned long vfn, mfn_t mfn, unsigned long nr,
> +                                unsigned int flags)
> +{
> +    unsigned int level;
> +    unsigned long mask;
> +
> +    /*
> +      * Don't take into account the MFN when removing mapping (i.e
> +      * MFN_INVALID) to calculate the correct target order.
> +      *
> +      * Per the Arm Arm, `vfn` and `mfn` must be both superpage aligned.
> +      * They are or-ed together and then checked against the size of
> +      * each level.
> +      *
> +      * `left` is not included and checked separately to allow
> +      * superpage mapping even if it is not properly aligned (the
> +      * user may have asked to map 2MB + 4k).
> +      */
> +     mask = !mfn_eq(mfn, INVALID_MFN) ? mfn_x(mfn) : 0;
> +     mask |= vfn;
> +
> +     /*
> +      * Always use level 3 mapping unless the caller request block
> +      * mapping.
> +      */
> +     if ( likely(!(flags & _PAGE_BLOCK)) )
> +         level = 3;
> +     else if ( !(mask & (BIT(FIRST_ORDER, UL) - 1)) &&
> +               (nr >= BIT(FIRST_ORDER, UL)) )
> +         level = 1;
> +     else if ( !(mask & (BIT(SECOND_ORDER, UL) - 1)) &&
> +               (nr >= BIT(SECOND_ORDER, UL)) )
> +         level = 2;
> +     else
> +         level = 3;
> +
> +     return level;
> +}
> +
>  static DEFINE_SPINLOCK(xen_pt_lock);
>  
>  static int xen_pt_update(unsigned long virt,
>                           mfn_t mfn,
> -                         unsigned long nr_mfns,
> +                         /* const on purpose as it is used for TLB flush */
> +                         const unsigned long nr_mfns,
>                           unsigned int flags)
>  {
>      int rc = 0;
> -    unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
> +    unsigned long vfn = virt >> PAGE_SHIFT;
> +    unsigned long left = nr_mfns;
>  
>      /*
>       * For arm32, page-tables are different on each CPUs. Yet, they share
> @@ -1283,14 +1346,24 @@ static int xen_pt_update(unsigned long virt,
>  
>      spin_lock(&xen_pt_lock);
>  
> -    for ( ; addr < addr_end; addr += PAGE_SIZE )
> +    while ( left )
>      {
> -        rc = xen_pt_update_entry(root, addr, mfn, flags);
> +        unsigned int order, level;
> +
> +        level = xen_pt_mapping_level(vfn, mfn, left, flags);
> +        order = XEN_PT_LEVEL_ORDER(level);
> +
> +        ASSERT(left >= BIT(order, UL));
> +
> +        rc = xen_pt_update_entry(root, vfn << PAGE_SHIFT, mfn, level, flags);
>          if ( rc )
>              break;
>  
> +        vfn += 1U << order;
>          if ( !mfn_eq(mfn, INVALID_MFN) )
> -            mfn = mfn_add(mfn, 1);
> +            mfn = mfn_add(mfn, 1U << order);
> +
> +        left -= (1U << order);
>      }
>  
>      /*
> -- 
> 2.32.0
> 

Re: [PATCH 03/16] xen/arm: mm: Avoid flushing the TLBs when mapping are inserted

[Stefano Stabellini]

On Fri, 20 May 2022, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Currently, the function xen_pt_update() will flush the TLBs even when
> the mappings are inserted. This is a bit wasteful because we don't
> allow mapping replacement. Even if we were, the flush would need to
> happen earlier because mapping replacement should use Break-Before-Make
> when updating the entry.
> 
> A single call to xen_pt_update() can perform a single action. IOW, it
> is not possible to, for instance, mix inserting and removing mappings.
> Therefore, we can use `flags` to determine what action is performed.
> 
> This change will be particularly help to limit the impact of switching
> boot time mapping to use xen_pt_update().
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

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


> ---
>     Changes in v4:
>         - Switch the check to a different expression that will still
>         result to the same truth table.
> 
>     Changes in v2:
>         - New patch
> ---
>  xen/arch/arm/mm.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index c4487dd7fc46..747083d820dd 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1119,7 +1119,13 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int level,
>          /* We should be here with a valid MFN. */
>          ASSERT(!mfn_eq(mfn, INVALID_MFN));
>  
> -        /* We don't allow replacing any valid entry. */
> +        /*
> +         * We don't allow replacing any valid entry.
> +         *
> +         * Note that the function xen_pt_update() relies on this
> +         * assumption and will skip the TLB flush. The function will need
> +         * to be updated if the check is relaxed.
> +         */
>          if ( lpae_is_valid(entry) )
>          {
>              if ( lpae_is_mapping(entry, level) )
> @@ -1434,11 +1440,16 @@ static int xen_pt_update(unsigned long virt,
>      }
>  
>      /*
> -     * Flush the TLBs even in case of failure because we may have
> +     * The TLBs flush can be safely skipped when a mapping is inserted
> +     * as we don't allow mapping replacement (see xen_pt_check_entry()).
> +     *
> +     * For all the other cases, the TLBs will be flushed unconditionally
> +     * even if the mapping has failed. This is because we may have
>       * partially modified the PT. This will prevent any unexpected
>       * behavior afterwards.
>       */
> -    flush_xen_tlb_range_va(virt, PAGE_SIZE * nr_mfns);
> +    if ( !((flags & _PAGE_PRESENT) && !mfn_eq(mfn, INVALID_MFN)) )
> +        flush_xen_tlb_range_va(virt, PAGE_SIZE * nr_mfns);
>  
>      spin_unlock(&xen_pt_lock);
>  
> -- 
> 2.32.0
> 

Re: [PATCH 04/16] xen/arm: mm: Don't open-code Xen PT update in remove_early_mappings()

[Stefano Stabellini]

On Fri, 20 May 2022, Julien Grall wrote:
> From: Julien Grall <julien.grall@arm.com>
> 
> Now that xen_pt_update_entry() is able to deal with different mapping
> size, we can replace the open-coding of the page-tables update by a call
> to modify_xen_mappings().
> 
> As the function is not meant to fail, a BUG_ON() is added to check the
> return.
> 
> Note that we don't use destroy_xen_mappings() because the helper doesn't
> allow us to pass a flags. In theory we could add an extra parameter to
> the function, however there are no other expected users. Hence why
> modify_xen_mappings() is used.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> Reviewed-by: Hongda Deng <Hongda.Heng@arm.com>

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


> ---
>     Changes in v4:
>         - Add Hongda's reviewed-by
>         - Add a comment to explain what modify_xen_mappings() does.
>         - Clarify in the commit message hwy modify_xen_mappings() is
>           used rather than destroy_xen_mappings().
> 
>     Changes in v2:
>         - Stay consistent with how function name are used in the commit
>         message
>         - Add my AWS signed-off-by
> ---
>  xen/arch/arm/mm.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 747083d820dd..64a79d45b38c 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -614,11 +614,12 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
>  
>  void __init remove_early_mappings(void)
>  {
> -    lpae_t pte = {0};
> -    write_pte(xen_second + second_table_offset(BOOT_FDT_VIRT_START), pte);
> -    write_pte(xen_second + second_table_offset(BOOT_FDT_VIRT_START + SZ_2M),
> -              pte);
> -    flush_xen_tlb_range_va(BOOT_FDT_VIRT_START, BOOT_FDT_SLOT_SIZE);
> +    int rc;
> +
> +    /* destroy the _PAGE_BLOCK mapping */
> +    rc = modify_xen_mappings(BOOT_FDT_VIRT_START, BOOT_FDT_VIRT_END,
> +                             _PAGE_BLOCK);
> +    BUG_ON(rc);
>  }
>  
>  /*
> -- 
> 2.32.0
> 

Re: [PATCH 05/16] xen/arm: mm: Re-implement early_fdt_map() using map_pages_to_xen()

[Stefano Stabellini]

On Fri, 20 May 2022, Julien Grall wrote:
> From: Julien Grall <julien.grall@arm.com>
> 
> Now that map_pages_to_xen() has been extended to support 2MB mappings,
> we can replace the create_mappings() calls by map_pages_to_xen() calls.
> 
> The mapping can also be marked read-only as Xen should not modify
> the host Device Tree during boot.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> Reviewed-by: Hongda Deng <Hongda.Heng@arm.com>

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


> ---
>     Changes in v4:
>         - Fix typo in the commit message
>         - Add Hongda's reviewed-by
> 
>     Changes in v2:
>         - Add my AWS signed-off-by
>         - Fix typo in the commit message
> ---
>  xen/arch/arm/mm.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 64a79d45b38c..03f970e4d10b 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -574,6 +574,7 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
>      paddr_t offset;
>      void *fdt_virt;
>      uint32_t size;
> +    int rc;
>  
>      /*
>       * Check whether the physical FDT address is set and meets the minimum
> @@ -589,8 +590,12 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
>      /* The FDT is mapped using 2MB superpage */
>      BUILD_BUG_ON(BOOT_FDT_VIRT_START % SZ_2M);
>  
> -    create_mappings(xen_second, BOOT_FDT_VIRT_START, paddr_to_pfn(base_paddr),
> -                    SZ_2M >> PAGE_SHIFT, SZ_2M);
> +    rc = map_pages_to_xen(BOOT_FDT_VIRT_START, maddr_to_mfn(base_paddr),
> +                          SZ_2M >> PAGE_SHIFT,
> +                          PAGE_HYPERVISOR_RO | _PAGE_BLOCK);
> +    if ( rc )
> +        panic("Unable to map the device-tree.\n");
> +
>  
>      offset = fdt_paddr % SECOND_SIZE;
>      fdt_virt = (void *)BOOT_FDT_VIRT_START + offset;
> @@ -604,9 +609,12 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
>  
>      if ( (offset + size) > SZ_2M )
>      {
> -        create_mappings(xen_second, BOOT_FDT_VIRT_START + SZ_2M,
> -                        paddr_to_pfn(base_paddr + SZ_2M),
> -                        SZ_2M >> PAGE_SHIFT, SZ_2M);
> +        rc = map_pages_to_xen(BOOT_FDT_VIRT_START + SZ_2M,
> +                              maddr_to_mfn(base_paddr + SZ_2M),
> +                              SZ_2M >> PAGE_SHIFT,
> +                              PAGE_HYPERVISOR_RO | _PAGE_BLOCK);
> +        if ( rc )
> +            panic("Unable to map the device-tree\n");
>      }
>  
>      return fdt_virt;
> -- 
> 2.32.0
> 

Re: [PATCH 13/16] xen/arm32: setup: Move out the code to populate the boot allocator

[Stefano Stabellini]

On Fri, 20 May 2022, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> In a follow-up patch, we will want to populate the boot allocator
> separately for arm64. The code will end up to be very similar to the one
> on arm32. So move out the code in a new helper populate_boot_allocator().
> 
> For now the code is still protected by CONFIG_ARM_32 to avoid any build
> failure on arm64.
> 
> Take the opportunity to replace mfn_add(xen_mfn_start, xenheap_pages) with
> xenheap_mfn_end as they are equivalent.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

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


> ---
> 
>     Changes in v4:
>         - Patch added
> ---
>  xen/arch/arm/setup.c | 90 +++++++++++++++++++++++++-------------------
>  1 file changed, 51 insertions(+), 39 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index d5d0792ed48a..3d5a2283d4ef 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -637,10 +637,58 @@ static void __init init_staticmem_pages(void)
>  }
>  
>  #ifdef CONFIG_ARM_32
> +/*
> + * Populate the boot allocator. All the RAM but the following regions
> + * will be added:
> + *  - Modules (e.g., Xen, Kernel)
> + *  - Reserved regions
> + *  - Xenheap
> + */
> +static void __init populate_boot_allocator(void)
> +{
> +    unsigned int i;
> +    const struct meminfo *banks = &bootinfo.mem;
> +
> +    for ( i = 0; i < banks->nr_banks; i++ )
> +    {
> +        const struct membank *bank = &banks->bank[i];
> +        paddr_t bank_end = bank->start + bank->size;
> +        paddr_t s, e;
> +
> +        s = bank->start;
> +        while ( s < bank_end )
> +        {
> +            paddr_t n = bank_end;
> +
> +            e = next_module(s, &n);
> +
> +            if ( e == ~(paddr_t)0 )
> +                e = n = bank_end;
> +
> +            /*
> +             * Module in a RAM bank other than the one which we are
> +             * not dealing with here.
> +             */
> +            if ( e > bank_end )
> +                e = bank_end;
> +
> +            /* Avoid the xenheap */
> +            if ( s < mfn_to_maddr(xenheap_mfn_end) &&
> +                 mfn_to_maddr(xenheap_mfn_start) < e )
> +            {
> +                e = mfn_to_maddr(xenheap_mfn_start);
> +                n = mfn_to_maddr(xenheap_mfn_end);
> +            }
> +
> +            fw_unreserved_regions(s, e, init_boot_pages, 0);
> +            s = n;
> +        }
> +    }
> +}
> +
>  static void __init setup_mm(void)
>  {
> -    paddr_t ram_start, ram_end, ram_size;
> -    paddr_t s, e;
> +    paddr_t ram_start, ram_end, ram_size, e;
>      unsigned long ram_pages;
>      unsigned long heap_pages, xenheap_pages, domheap_pages;
>      int i;
> @@ -718,43 +766,7 @@ static void __init setup_mm(void)
>      setup_xenheap_mappings((e >> PAGE_SHIFT) - xenheap_pages, xenheap_pages);
>  
>      /* Add non-xenheap memory */
> -    for ( i = 0; i < bootinfo.mem.nr_banks; i++ )
> -    {
> -        paddr_t bank_start = bootinfo.mem.bank[i].start;
> -        paddr_t bank_end = bank_start + bootinfo.mem.bank[i].size;
> -
> -        s = bank_start;
> -        while ( s < bank_end )
> -        {
> -            paddr_t n = bank_end;
> -
> -            e = next_module(s, &n);
> -
> -            if ( e == ~(paddr_t)0 )
> -            {
> -                e = n = ram_end;
> -            }
> -
> -            /*
> -             * Module in a RAM bank other than the one which we are
> -             * not dealing with here.
> -             */
> -            if ( e > bank_end )
> -                e = bank_end;
> -
> -            /* Avoid the xenheap */
> -            if ( s < mfn_to_maddr(mfn_add(xenheap_mfn_start, xenheap_pages))
> -                 && mfn_to_maddr(xenheap_mfn_start) < e )
> -            {
> -                e = mfn_to_maddr(xenheap_mfn_start);
> -                n = mfn_to_maddr(mfn_add(xenheap_mfn_start, xenheap_pages));
> -            }
> -
> -            fw_unreserved_regions(s, e, init_boot_pages, 0);
> -
> -            s = n;
> -        }
> -    }
> +    populate_boot_allocator();
>  
>      /* Frame table covers all of RAM region, including holes */
>      setup_frametable_mappings(ram_start, ram_end);
> -- 
> 2.32.0
> 

Re: [PATCH 14/16] xen/arm64: mm: Add memory to the boot allocator first

[Stefano Stabellini]

On Fri, 20 May 2022, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Currently, memory is added to the boot allocator after the xenheap
> mappings are done. This will break if the first mapping is more than
> 512GB of RAM.
> 
> In addition to that, a follow-up patch will rework setup_xenheap_mappings()
> to use smaller mappings (e.g. 2MB, 4KB). So it will be necessary to have
> memory in the boot allocator earlier.
> 
> Only free memory (e.g. not reserved or modules) can be added to the boot
> allocator. It might be possible that some regions (including the first
> one) will have no free memory.
> 
> So we need to add all the free memory to the boot allocator first
> and then add do the mappings.
> 
> Populating the boot allocator is nearly the same between arm32 and
> arm64. The only difference is on the former we need to exclude the
> xenheap for the boot allocator. Gate the difference with CONFIG_ARM_32
> so the code be re-used on arm64.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

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


> ---
>     Changes in v4:
>         - The implementation of populate_boot_allocator() has been
>           moved in a separate patch.
>         - Fix typo
> 
>     Changes in v3:
>         - Patch added
> ---
>  xen/arch/arm/setup.c | 55 +++++++++++++++++++-------------------------
>  1 file changed, 24 insertions(+), 31 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 3d5a2283d4ef..db1768c03f03 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -636,13 +636,12 @@ static void __init init_staticmem_pages(void)
>  #endif
>  }
>  
> -#ifdef CONFIG_ARM_32
>  /*
>   * Populate the boot allocator. All the RAM but the following regions
>   * will be added:
>   *  - Modules (e.g., Xen, Kernel)
>   *  - Reserved regions
> - *  - Xenheap
> + *  - Xenheap (arm32 only)
>   */
>  static void __init populate_boot_allocator(void)
>  {
> @@ -672,6 +671,7 @@ static void __init populate_boot_allocator(void)
>              if ( e > bank_end )
>                  e = bank_end;
>  
> +#ifdef CONFIG_ARM_32
>              /* Avoid the xenheap */
>              if ( s < mfn_to_maddr(xenheap_mfn_end) &&
>                   mfn_to_maddr(xenheap_mfn_start) < e )
> @@ -679,6 +679,7 @@ static void __init populate_boot_allocator(void)
>                  e = mfn_to_maddr(xenheap_mfn_start);
>                  n = mfn_to_maddr(xenheap_mfn_end);
>              }
> +#endif
>  
>              fw_unreserved_regions(s, e, init_boot_pages, 0);
>              s = n;
> @@ -686,6 +687,7 @@ static void __init populate_boot_allocator(void)
>      }
>  }
>  
> +#ifdef CONFIG_ARM_32
>  static void __init setup_mm(void)
>  {
>      paddr_t ram_start, ram_end, ram_size, e;
> @@ -781,45 +783,36 @@ static void __init setup_mm(void)
>  #else /* CONFIG_ARM_64 */
>  static void __init setup_mm(void)
>  {
> +    const struct meminfo *banks = &bootinfo.mem;
>      paddr_t ram_start = ~0;
>      paddr_t ram_end = 0;
>      paddr_t ram_size = 0;
> -    int bank;
> +    unsigned int i;
>  
>      init_pdx();
>  
> -    total_pages = 0;
> -    for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
> -    {
> -        paddr_t bank_start = bootinfo.mem.bank[bank].start;
> -        paddr_t bank_size = bootinfo.mem.bank[bank].size;
> -        paddr_t bank_end = bank_start + bank_size;
> -        paddr_t s, e;
> -
> -        ram_size = ram_size + bank_size;
> -        ram_start = min(ram_start,bank_start);
> -        ram_end = max(ram_end,bank_end);
> -
> -        setup_xenheap_mappings(bank_start>>PAGE_SHIFT, bank_size>>PAGE_SHIFT);
> -
> -        s = bank_start;
> -        while ( s < bank_end )
> -        {
> -            paddr_t n = bank_end;
> +    /*
> +     * We need some memory to allocate the page-tables used for the xenheap
> +     * mappings. But some regions may contain memory already allocated
> +     * for other uses (e.g. modules, reserved-memory...).
> +     *
> +     * For simplicity, add all the free regions in the boot allocator.
> +     */
> +    populate_boot_allocator();
>  
> -            e = next_module(s, &n);
> +    total_pages = 0;
>  
> -            if ( e == ~(paddr_t)0 )
> -            {
> -                e = n = bank_end;
> -            }
> +    for ( i = 0; i < banks->nr_banks; i++ )
> +    {
> +        const struct membank *bank = &banks->bank[i];
> +        paddr_t bank_end = bank->start + bank->size;
>  
> -            if ( e > bank_end )
> -                e = bank_end;
> +        ram_size = ram_size + bank->size;
> +        ram_start = min(ram_start, bank->start);
> +        ram_end = max(ram_end, bank_end);
>  
> -            fw_unreserved_regions(s, e, init_boot_pages, 0);
> -            s = n;
> -        }
> +        setup_xenheap_mappings(PFN_DOWN(bank->start),
> +                               PFN_DOWN(bank->size));
>      }
>  
>      total_pages += ram_size >> PAGE_SHIFT;
> -- 
> 2.32.0
> 

Re: [PATCH 16/16] xen/arm: mm: Re-implement setup_frame_table_mappings() with map_pages_to_xen()

[Stefano Stabellini]

On Fri, 20 May 2022, Julien Grall wrote:
> From: Julien Grall <julien.grall@arm.com>
> 
> Now that map_pages_to_xen() has been extended to support 2MB mappings,
> we can replace the create_mappings() call by map_pages_to_xen() call.
> 
> This has the advantage to remove the differences between 32-bit and
> 64-bit code.
> 
> Lastly remove create_mappings() as there is no more callers.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Julien Grall <jgrall@amazon.com>

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


> ---
>     Changes in v4:
>         - Add missing _PAGE_BLOCK
> 
>     Changes in v3:
>         - Fix typo in the commit message
>         - Remove the TODO regarding contiguous bit
> 
>     Changes in v2:
>         - New patch
> ---
>  xen/arch/arm/mm.c | 64 +++++------------------------------------------
>  1 file changed, 6 insertions(+), 58 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 65af44f42232..be37176a4725 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -369,40 +369,6 @@ void clear_fixmap(unsigned map)
>      BUG_ON(res != 0);
>  }
>  
> -/* Create Xen's mappings of memory.
> - * Mapping_size must be either 2MB or 32MB.
> - * Base and virt must be mapping_size aligned.
> - * Size must be a multiple of mapping_size.
> - * second must be a contiguous set of second level page tables
> - * covering the region starting at virt_offset. */
> -static void __init create_mappings(lpae_t *second,
> -                                   unsigned long virt_offset,
> -                                   unsigned long base_mfn,
> -                                   unsigned long nr_mfns,
> -                                   unsigned int mapping_size)
> -{
> -    unsigned long i, count;
> -    const unsigned long granularity = mapping_size >> PAGE_SHIFT;
> -    lpae_t pte, *p;
> -
> -    ASSERT((mapping_size == MB(2)) || (mapping_size == MB(32)));
> -    ASSERT(!((virt_offset >> PAGE_SHIFT) % granularity));
> -    ASSERT(!(base_mfn % granularity));
> -    ASSERT(!(nr_mfns % granularity));
> -
> -    count = nr_mfns / XEN_PT_LPAE_ENTRIES;
> -    p = second + second_linear_offset(virt_offset);
> -    pte = mfn_to_xen_entry(_mfn(base_mfn), MT_NORMAL);
> -    if ( granularity == 16 * XEN_PT_LPAE_ENTRIES )
> -        pte.pt.contig = 1;  /* These maps are in 16-entry contiguous chunks. */
> -    for ( i = 0; i < count; i++ )
> -    {
> -        write_pte(p + i, pte);
> -        pte.pt.base += 1 << XEN_PT_LPAE_SHIFT;
> -    }
> -    flush_xen_tlb_local();
> -}
> -
>  #ifdef CONFIG_DOMAIN_PAGE
>  void *map_domain_page_global(mfn_t mfn)
>  {
> @@ -862,36 +828,18 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
>      unsigned long frametable_size = nr_pdxs * sizeof(struct page_info);
>      mfn_t base_mfn;
>      const unsigned long mapping_size = frametable_size < MB(32) ? MB(2) : MB(32);
> -#ifdef CONFIG_ARM_64
> -    lpae_t *second, pte;
> -    unsigned long nr_second;
> -    mfn_t second_base;
> -    int i;
> -#endif
> +    int rc;
>  
>      frametable_base_pdx = mfn_to_pdx(maddr_to_mfn(ps));
>      /* Round up to 2M or 32M boundary, as appropriate. */
>      frametable_size = ROUNDUP(frametable_size, mapping_size);
>      base_mfn = alloc_boot_pages(frametable_size >> PAGE_SHIFT, 32<<(20-12));
>  
> -#ifdef CONFIG_ARM_64
> -    /* Compute the number of second level pages. */
> -    nr_second = ROUNDUP(frametable_size, FIRST_SIZE) >> FIRST_SHIFT;
> -    second_base = alloc_boot_pages(nr_second, 1);
> -    second = mfn_to_virt(second_base);
> -    for ( i = 0; i < nr_second; i++ )
> -    {
> -        clear_page(mfn_to_virt(mfn_add(second_base, i)));
> -        pte = mfn_to_xen_entry(mfn_add(second_base, i), MT_NORMAL);
> -        pte.pt.table = 1;
> -        write_pte(&xen_first[first_table_offset(FRAMETABLE_VIRT_START)+i], pte);
> -    }
> -    create_mappings(second, 0, mfn_x(base_mfn), frametable_size >> PAGE_SHIFT,
> -                    mapping_size);
> -#else
> -    create_mappings(xen_second, FRAMETABLE_VIRT_START, mfn_x(base_mfn),
> -                    frametable_size >> PAGE_SHIFT, mapping_size);
> -#endif
> +    rc = map_pages_to_xen(FRAMETABLE_VIRT_START, base_mfn,
> +                          frametable_size >> PAGE_SHIFT,
> +                          PAGE_HYPERVISOR_RW | _PAGE_BLOCK);
> +    if ( rc )
> +        panic("Unable to setup the frametable mappings.\n");
>  
>      memset(&frame_table[0], 0, nr_pdxs * sizeof(struct page_info));
>      memset(&frame_table[nr_pdxs], -1,
> -- 
> 2.32.0
> 

Re: [PATCH 10/16] xen/arm: add Persistent Map (PMAP) infrastructure

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 10747 bytes --]

On Fri, 20 May 2022, Julien Grall wrote:
> From: Wei Liu <wei.liu2@citrix.com>
> 
> The basic idea is like Persistent Kernel Map (PKMAP) in Linux. We
> pre-populate all the relevant page tables before the system is fully
> set up.
> 
> We will need it on Arm in order to rework the arm64 version of
> xenheap_setup_mappings() as we may need to use pages allocated from
> the boot allocator before they are effectively mapped.
> 
> This infrastructure is not lock-protected therefore can only be used
> before smpboot. After smpboot, map_domain_page() has to be used.
> 
> This is based on the x86 version [1] that was originally implemented
> by Wei Liu.
> 
> The PMAP infrastructure is implemented in common code with some
> arch helpers to set/clear the page-table entries and convertion
> between a fixmap slot to a virtual address...
> 
> As mfn_to_xen_entry() now needs to be exported, take the opportunity
> to swich the parameter attr from unsigned to unsigned int.
> 
> [1] <e92da4ad6015b6089737fcccba3ec1d6424649a5.1588278317.git.hongyxia@amazon.com>
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Signed-off-by: Hongyan Xia <hongyxia@amazon.com>
> [julien: Adapted for Arm]
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> ---
>     Changes in v4:
>         - Move xen_fixmap in fixmap.h and add a comment about its usage.
>         - Update comments
>         - Use DECLARE_BITMAP()
>         - Replace local_irq_{enable, disable} with an ASSERT() as there
>           should be no user of pmap() in interrupt context.
> 
>     Changes in v3:
>         - s/BITS_PER_LONG/BITS_PER_BYTE/
>         - Move pmap to common code
> 
>     Changes in v2:
>         - New patch
> 
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Wei Liu <wl@xen.org>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Roger Pau Monné <roger.pau@citrix.com>
> ---
>  xen/arch/arm/Kconfig              |  1 +
>  xen/arch/arm/include/asm/fixmap.h | 24 +++++++++++
>  xen/arch/arm/include/asm/lpae.h   |  8 ++++
>  xen/arch/arm/include/asm/pmap.h   | 32 ++++++++++++++
>  xen/arch/arm/mm.c                 |  7 +--
>  xen/common/Kconfig                |  3 ++
>  xen/common/Makefile               |  1 +
>  xen/common/pmap.c                 | 72 +++++++++++++++++++++++++++++++
>  xen/include/xen/pmap.h            | 16 +++++++
>  9 files changed, 158 insertions(+), 6 deletions(-)
>  create mode 100644 xen/arch/arm/include/asm/pmap.h
>  create mode 100644 xen/common/pmap.c
>  create mode 100644 xen/include/xen/pmap.h
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index ecfa6822e4d3..a89a67802aa9 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -14,6 +14,7 @@ config ARM
>  	select HAS_DEVICE_TREE
>  	select HAS_PASSTHROUGH
>  	select HAS_PDX
> +	select HAS_PMAP
>  	select IOMMU_FORCE_PT_SHARE
>  
>  config ARCH_DEFCONFIG
> diff --git a/xen/arch/arm/include/asm/fixmap.h b/xen/arch/arm/include/asm/fixmap.h
> index 1cee51e52ab9..365a2385a087 100644
> --- a/xen/arch/arm/include/asm/fixmap.h
> +++ b/xen/arch/arm/include/asm/fixmap.h
> @@ -5,20 +5,44 @@
>  #define __ASM_FIXMAP_H
>  
>  #include <xen/acpi.h>
> +#include <xen/pmap.h>
>  
>  /* Fixmap slots */
>  #define FIXMAP_CONSOLE  0  /* The primary UART */
>  #define FIXMAP_MISC     1  /* Ephemeral mappings of hardware */
>  #define FIXMAP_ACPI_BEGIN  2  /* Start mappings of ACPI tables */
>  #define FIXMAP_ACPI_END    (FIXMAP_ACPI_BEGIN + NUM_FIXMAP_ACPI_PAGES - 1)  /* End mappings of ACPI tables */
> +#define FIXMAP_PMAP_BEGIN (FIXMAP_ACPI_END + 1) /* Start of PMAP */
> +#define FIXMAP_PMAP_END (FIXMAP_PMAP_BEGIN + NUM_FIX_PMAP - 1) /* End of PMAP */
> +
> +#define FIXMAP_LAST FIXMAP_PMAP_END
> +
> +#define FIXADDR_START FIXMAP_ADDR(0)
> +#define FIXADDR_TOP FIXMAP_ADDR(FIXMAP_LAST)
>  
>  #ifndef __ASSEMBLY__
>  
> +/*
> + * Direct access to xen_fixmap[] should only happen when {set,
> + * clear}_fixmap() is unusable (e.g. where we would end up to
> + * recursively call the helpers).
> + */
> +extern lpae_t xen_fixmap[XEN_PT_LPAE_ENTRIES];
> +
>  /* Map a page in a fixmap entry */
>  extern void set_fixmap(unsigned map, mfn_t mfn, unsigned attributes);
>  /* Remove a mapping from a fixmap entry */
>  extern void clear_fixmap(unsigned map);
>  
> +#define fix_to_virt(slot) ((void *)FIXMAP_ADDR(slot))
> +
> +static inline unsigned int virt_to_fix(vaddr_t vaddr)
> +{
> +    BUG_ON(vaddr >= FIXADDR_TOP || vaddr < FIXADDR_START);
> +
> +    return ((vaddr - FIXADDR_START) >> PAGE_SHIFT);
> +}
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif /* __ASM_FIXMAP_H */
> diff --git a/xen/arch/arm/include/asm/lpae.h b/xen/arch/arm/include/asm/lpae.h
> index aecb320dec45..fc19cbd84772 100644
> --- a/xen/arch/arm/include/asm/lpae.h
> +++ b/xen/arch/arm/include/asm/lpae.h
> @@ -4,6 +4,7 @@
>  #ifndef __ASSEMBLY__
>  
>  #include <xen/page-defs.h>
> +#include <xen/mm-frame.h>
>  
>  /*
>   * WARNING!  Unlike the x86 pagetable code, where l1 is the lowest level and
> @@ -168,6 +169,13 @@ static inline bool lpae_is_superpage(lpae_t pte, unsigned int level)
>          third_table_offset(addr)            \
>      }
>  
> +/*
> + * 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.
> + */
> +lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned int attr);
> +
>  #endif /* __ASSEMBLY__ */
>  
>  /*
> diff --git a/xen/arch/arm/include/asm/pmap.h b/xen/arch/arm/include/asm/pmap.h
> new file mode 100644
> index 000000000000..74398b4c4fe6
> --- /dev/null
> +++ b/xen/arch/arm/include/asm/pmap.h
> @@ -0,0 +1,32 @@
> +#ifndef __ASM_PMAP_H__
> +#define __ASM_PMAP_H__
> +
> +#include <xen/mm.h>
> +
> +#include <asm/fixmap.h>
> +
> +static inline void arch_pmap_map(unsigned int slot, mfn_t mfn)
> +{
> +    lpae_t *entry = &xen_fixmap[slot];
> +    lpae_t pte;
> +
> +    ASSERT(!lpae_is_valid(*entry));
> +
> +    pte = mfn_to_xen_entry(mfn, PAGE_HYPERVISOR_RW);
> +    pte.pt.table = 1;
> +    write_pte(entry, pte);

Here we don't need a tlb flush because we never go from a valid mapping
to another valid mapping. We also go through arch_pmap_unmap which
clears the mapping and also flushes the tlb. Is that right?


> +}
> +
> +static inline void arch_pmap_unmap(unsigned int slot)
> +{
> +    lpae_t pte = {};
> +
> +    write_pte(&xen_fixmap[slot], pte);
> +
> +    flush_xen_tlb_range_va_local(FIXMAP_ADDR(slot), PAGE_SIZE);
> +}
> +
> +void arch_pmap_map_slot(unsigned int slot, mfn_t mfn);
> +void arch_pmap_clear_slot(void *ptr);

What are these two? They are not defined anywhere?


> +#endif /* __ASM_PMAP_H__ */
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 52b2a0394047..bd1348a99716 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -305,12 +305,7 @@ 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(mfn_t mfn, unsigned attr)
> +lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned int attr)
>  {
>      lpae_t e = (lpae_t) {
>          .pt = {
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index d921c74d615e..5b6b2406c028 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -49,6 +49,9 @@ config HAS_KEXEC
>  config HAS_PDX
>  	bool
>  
> +config HAS_PMAP
> +	bool
> +
>  config HAS_SCHED_GRANULARITY
>  	bool
>  
> diff --git a/xen/common/Makefile b/xen/common/Makefile
> index b1e076c30b81..3baf83d527d8 100644
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -29,6 +29,7 @@ obj-y += notifier.o
>  obj-y += page_alloc.o
>  obj-$(CONFIG_HAS_PDX) += pdx.o
>  obj-$(CONFIG_PERF_COUNTERS) += perfc.o
> +obj-bin-$(CONFIG_HAS_PMAP) += pmap.init.o
>  obj-y += preempt.o
>  obj-y += random.o
>  obj-y += rangeset.o
> diff --git a/xen/common/pmap.c b/xen/common/pmap.c
> new file mode 100644
> index 000000000000..9355cacb7373
> --- /dev/null
> +++ b/xen/common/pmap.c
> @@ -0,0 +1,72 @@
> +#include <xen/bitops.h>
> +#include <xen/init.h>
> +#include <xen/irq.h>
> +#include <xen/pmap.h>
> +
> +#include <asm/pmap.h>
> +#include <asm/fixmap.h>
> +
> +/*
> + * Simple mapping infrastructure to map / unmap pages in fixed map.
> + * This is used to set the page table before the map domain page infrastructure
> + * is initialized.
> + *
> + * This structure is not protected by any locks, so it must not be used after
> + * smp bring-up.
> + */
> +
> +/* Bitmap to track which slot is used */
> +static __initdata DECLARE_BITMAP(inuse, NUM_FIX_PMAP);
> +
> +void *__init pmap_map(mfn_t mfn)
> +{
> +    unsigned int idx;
> +    unsigned int slot;
> +
> +    ASSERT(system_state < SYS_STATE_smp_boot);
> +    ASSERT(!in_irq());
> +
> +    idx = find_first_zero_bit(inuse, NUM_FIX_PMAP);
> +    if ( idx == NUM_FIX_PMAP )
> +        panic("Out of PMAP slots\n");
> +
> +    __set_bit(idx, inuse);
> +
> +    slot = idx + FIXMAP_PMAP_BEGIN;
> +    ASSERT(slot >= FIXMAP_PMAP_BEGIN && slot <= FIXMAP_PMAP_END);
> +
> +    /*
> +     * We cannot use set_fixmap() here. We use PMAP when the domain map
> +     * page infrastructure is not yet initialized, so map_pages_to_xen() called
> +     * by set_fixmap() needs to map pages on demand, which then calls pmap()
> +     * again, resulting in a loop. Modify the PTEs directly instead. The same
> +     * is true for pmap_unmap().
> +     */
> +    arch_pmap_map(slot, mfn);
> +
> +    return fix_to_virt(slot);
> +}
> +
> +void __init pmap_unmap(const void *p)
> +{
> +    unsigned int idx;
> +    unsigned int slot = virt_to_fix((unsigned long)p);
> +
> +    ASSERT(system_state < SYS_STATE_smp_boot);
> +    ASSERT(slot >= FIXMAP_PMAP_BEGIN && slot <= FIXMAP_PMAP_END);
> +    ASSERT(in_irq());
> +
> +    idx = slot - FIXMAP_PMAP_BEGIN;
> +
> +    __clear_bit(idx, inuse);
> +    arch_pmap_unmap(slot);
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/xen/pmap.h b/xen/include/xen/pmap.h
> new file mode 100644
> index 000000000000..93e61b10870e
> --- /dev/null
> +++ b/xen/include/xen/pmap.h
> @@ -0,0 +1,16 @@
> +#ifndef __XEN_PMAP_H__
> +#define __XEN_PMAP_H__
> +
> +/* Large enough for mapping 5 levels of page tables with some headroom */
> +#define NUM_FIX_PMAP 8
> +
> +#ifndef __ASSEMBLY__
> +
> +#include <xen/mm-frame.h>
> +
> +void *pmap_map(mfn_t mfn);
> +void pmap_unmap(const void *p);
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* __XEN_PMAP_H__ */
> -- 
> 2.32.0
> 

Re: [PATCH 09/16] xen/arm: Move fixmap definitions in a separate header

[Stefano Stabellini]

On Fri, 20 May 2022, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> To use properly the fixmap definitions, their user would need
> also new to include <xen/acpi.h>. This is not very great when
> the user itself is not meant to directly use ACPI definitions.
> 
> Including <xen/acpi.h> in <asm/config.h> is not option because
> the latter header is included by everyone. So move out the fixmap
> entries definition in a new header.
> 
> Take the opportunity to also move {set, clear}_fixmap() prototypes
> in the new header.
> 
> Note that most of the definitions in <xen/acpi.h> now need to be
> surrounded with #ifndef __ASSEMBLY__ because <asm/fixmap.h> will
> be used in assembly (see EARLY_UART_VIRTUAL_ADDRESS).
> 
> The split will become more helpful in a follow-up patch where new
> fixmap entries will be defined.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> ---
>     There was some disagreement with Stefano on whether fixmap.h
>     should include acpi.h or this should be the other way around.
> 
>     I chose the former because each components should decide how
>     much entries in the fixmap they need and also because this is
>     the current behavior on x86. We should stay consitent
>     between arch to avoid any headers mess.
> 
>     Jan acked this patch, so I am assuming he is happy with this
>     approach. I would be OK to rework it if others agree with
>     Stefano's view.

noone is speaking so:

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



>     Changes in v4:
>         - Add Jan's acked-by
>         - Record Stefano's disagreement on the approach
> 
>     Changes in v3:
>         - Patch added
> ---
>  xen/arch/arm/acpi/lib.c                 |  2 ++
>  xen/arch/arm/include/asm/config.h       |  6 ------
>  xen/arch/arm/include/asm/early_printk.h |  1 +
>  xen/arch/arm/include/asm/fixmap.h       | 24 ++++++++++++++++++++++++
>  xen/arch/arm/include/asm/mm.h           |  4 ----
>  xen/arch/arm/kernel.c                   |  1 +
>  xen/arch/arm/mm.c                       |  1 +
>  xen/include/xen/acpi.h                  | 18 +++++++++++-------
>  8 files changed, 40 insertions(+), 17 deletions(-)
>  create mode 100644 xen/arch/arm/include/asm/fixmap.h
> 
> diff --git a/xen/arch/arm/acpi/lib.c b/xen/arch/arm/acpi/lib.c
> index a59cc4074cfb..41d521f720ac 100644
> --- a/xen/arch/arm/acpi/lib.c
> +++ b/xen/arch/arm/acpi/lib.c
> @@ -25,6 +25,8 @@
>  #include <xen/init.h>
>  #include <xen/mm.h>
>  
> +#include <asm/fixmap.h>
> +
>  static bool fixmap_inuse;
>  
>  char *__acpi_map_table(paddr_t phys, unsigned long size)
> diff --git a/xen/arch/arm/include/asm/config.h b/xen/arch/arm/include/asm/config.h
> index b25c9d39bb32..3e2a55a91058 100644
> --- a/xen/arch/arm/include/asm/config.h
> +++ b/xen/arch/arm/include/asm/config.h
> @@ -169,12 +169,6 @@
>  
>  #endif
>  
> -/* Fixmap slots */
> -#define FIXMAP_CONSOLE  0  /* The primary UART */
> -#define FIXMAP_MISC     1  /* Ephemeral mappings of hardware */
> -#define FIXMAP_ACPI_BEGIN  2  /* Start mappings of ACPI tables */
> -#define FIXMAP_ACPI_END    (FIXMAP_ACPI_BEGIN + NUM_FIXMAP_ACPI_PAGES - 1)  /* End mappings of ACPI tables */
> -
>  #define NR_hypercalls 64
>  
>  #define STACK_ORDER 3
> diff --git a/xen/arch/arm/include/asm/early_printk.h b/xen/arch/arm/include/asm/early_printk.h
> index 8dc911cf48a3..c5149b2976da 100644
> --- a/xen/arch/arm/include/asm/early_printk.h
> +++ b/xen/arch/arm/include/asm/early_printk.h
> @@ -11,6 +11,7 @@
>  #define __ARM_EARLY_PRINTK_H__
>  
>  #include <xen/page-size.h>
> +#include <asm/fixmap.h>
>  
>  #ifdef CONFIG_EARLY_PRINTK
>  
> diff --git a/xen/arch/arm/include/asm/fixmap.h b/xen/arch/arm/include/asm/fixmap.h
> new file mode 100644
> index 000000000000..1cee51e52ab9
> --- /dev/null
> +++ b/xen/arch/arm/include/asm/fixmap.h
> @@ -0,0 +1,24 @@
> +/*
> + * fixmap.h: compile-time virtual memory allocation
> + */
> +#ifndef __ASM_FIXMAP_H
> +#define __ASM_FIXMAP_H
> +
> +#include <xen/acpi.h>
> +
> +/* Fixmap slots */
> +#define FIXMAP_CONSOLE  0  /* The primary UART */
> +#define FIXMAP_MISC     1  /* Ephemeral mappings of hardware */
> +#define FIXMAP_ACPI_BEGIN  2  /* Start mappings of ACPI tables */
> +#define FIXMAP_ACPI_END    (FIXMAP_ACPI_BEGIN + NUM_FIXMAP_ACPI_PAGES - 1)  /* End mappings of ACPI tables */
> +
> +#ifndef __ASSEMBLY__
> +
> +/* Map a page in a fixmap entry */
> +extern void set_fixmap(unsigned map, mfn_t mfn, unsigned attributes);
> +/* Remove a mapping from a fixmap entry */
> +extern void clear_fixmap(unsigned map);
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* __ASM_FIXMAP_H */
> diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
> index 424aaf28230b..045a8ba4bb63 100644
> --- a/xen/arch/arm/include/asm/mm.h
> +++ b/xen/arch/arm/include/asm/mm.h
> @@ -191,10 +191,6 @@ extern void mmu_init_secondary_cpu(void);
>  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, 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 */
>  void __iomem *ioremap_attr(paddr_t start, size_t len, unsigned attributes);
>  
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index 8f43caa1866d..25ded1c056d9 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -15,6 +15,7 @@
>  #include <xen/vmap.h>
>  
>  #include <asm/byteorder.h>
> +#include <asm/fixmap.h>
>  #include <asm/kernel.h>
>  #include <asm/setup.h>
>  
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 6b7b72de27fe..52b2a0394047 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -41,6 +41,7 @@
>  #include <xen/sizes.h>
>  #include <xen/libfdt/libfdt.h>
>  
> +#include <asm/fixmap.h>
>  #include <asm/setup.h>
>  
>  /* Override macros from asm/page.h to make them work with mfn_t */
> diff --git a/xen/include/xen/acpi.h b/xen/include/xen/acpi.h
> index 39d51fcd01dd..1b9c75e68fc4 100644
> --- a/xen/include/xen/acpi.h
> +++ b/xen/include/xen/acpi.h
> @@ -28,6 +28,15 @@
>  #define _LINUX
>  #endif
>  
> +/*
> + * Fixmap pages to reserve for ACPI boot-time tables (see
> + * arch/x86/include/asm/fixmap.h or arch/arm/include/asm/fixmap.h),
> + * 64 pages(256KB) is large enough for most cases.)
> + */
> +#define NUM_FIXMAP_ACPI_PAGES  64
> +
> +#ifndef __ASSEMBLY__
> +
>  #include <xen/list.h>
>  
>  #include <acpi/acpi.h>
> @@ -39,13 +48,6 @@
>  #define ACPI_MADT_GET_POLARITY(inti)	ACPI_MADT_GET_(POLARITY, inti)
>  #define ACPI_MADT_GET_TRIGGER(inti)	ACPI_MADT_GET_(TRIGGER, inti)
>  
> -/*
> - * Fixmap pages to reserve for ACPI boot-time tables (see
> - * arch/x86/include/asm/fixmap.h or arch/arm/include/asm/config.h,
> - * 64 pages(256KB) is large enough for most cases.)
> - */
> -#define NUM_FIXMAP_ACPI_PAGES  64
> -
>  #define BAD_MADT_ENTRY(entry, end) (                                        \
>                  (!(entry)) || (unsigned long)(entry) + sizeof(*(entry)) > (end) ||  \
>                  (entry)->header.length < sizeof(*(entry)))
> @@ -207,4 +209,6 @@ void acpi_reboot(void);
>  void acpi_dmar_zap(void);
>  void acpi_dmar_reinstate(void);
>  
> +#endif /* __ASSEMBLY__ */
> +
>  #endif /*_LINUX_ACPI_H*/
> -- 
> 2.32.0
> 

Re: [PATCH 16/16] xen/arm: mm: Re-implement setup_frame_table_mappings() with map_pages_to_xen()

[Stefano Stabellini]

On Fri, 20 May 2022, Julien Grall wrote:
> From: Julien Grall <julien.grall@arm.com>
> 
> Now that map_pages_to_xen() has been extended to support 2MB mappings,
> we can replace the create_mappings() call by map_pages_to_xen() call.
> 
> This has the advantage to remove the differences between 32-bit and
> 64-bit code.
> 
> Lastly remove create_mappings() as there is no more callers.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Julien Grall <jgrall@amazon.com>

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


> ---
>     Changes in v4:
>         - Add missing _PAGE_BLOCK
> 
>     Changes in v3:
>         - Fix typo in the commit message
>         - Remove the TODO regarding contiguous bit
> 
>     Changes in v2:
>         - New patch
> ---
>  xen/arch/arm/mm.c | 64 +++++------------------------------------------
>  1 file changed, 6 insertions(+), 58 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 65af44f42232..be37176a4725 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -369,40 +369,6 @@ void clear_fixmap(unsigned map)
>      BUG_ON(res != 0);
>  }
>  
> -/* Create Xen's mappings of memory.
> - * Mapping_size must be either 2MB or 32MB.
> - * Base and virt must be mapping_size aligned.
> - * Size must be a multiple of mapping_size.
> - * second must be a contiguous set of second level page tables
> - * covering the region starting at virt_offset. */
> -static void __init create_mappings(lpae_t *second,
> -                                   unsigned long virt_offset,
> -                                   unsigned long base_mfn,
> -                                   unsigned long nr_mfns,
> -                                   unsigned int mapping_size)
> -{
> -    unsigned long i, count;
> -    const unsigned long granularity = mapping_size >> PAGE_SHIFT;
> -    lpae_t pte, *p;
> -
> -    ASSERT((mapping_size == MB(2)) || (mapping_size == MB(32)));
> -    ASSERT(!((virt_offset >> PAGE_SHIFT) % granularity));
> -    ASSERT(!(base_mfn % granularity));
> -    ASSERT(!(nr_mfns % granularity));
> -
> -    count = nr_mfns / XEN_PT_LPAE_ENTRIES;
> -    p = second + second_linear_offset(virt_offset);
> -    pte = mfn_to_xen_entry(_mfn(base_mfn), MT_NORMAL);
> -    if ( granularity == 16 * XEN_PT_LPAE_ENTRIES )
> -        pte.pt.contig = 1;  /* These maps are in 16-entry contiguous chunks. */
> -    for ( i = 0; i < count; i++ )
> -    {
> -        write_pte(p + i, pte);
> -        pte.pt.base += 1 << XEN_PT_LPAE_SHIFT;
> -    }
> -    flush_xen_tlb_local();
> -}
> -
>  #ifdef CONFIG_DOMAIN_PAGE
>  void *map_domain_page_global(mfn_t mfn)
>  {
> @@ -862,36 +828,18 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
>      unsigned long frametable_size = nr_pdxs * sizeof(struct page_info);
>      mfn_t base_mfn;
>      const unsigned long mapping_size = frametable_size < MB(32) ? MB(2) : MB(32);
> -#ifdef CONFIG_ARM_64
> -    lpae_t *second, pte;
> -    unsigned long nr_second;
> -    mfn_t second_base;
> -    int i;
> -#endif
> +    int rc;
>  
>      frametable_base_pdx = mfn_to_pdx(maddr_to_mfn(ps));
>      /* Round up to 2M or 32M boundary, as appropriate. */
>      frametable_size = ROUNDUP(frametable_size, mapping_size);
>      base_mfn = alloc_boot_pages(frametable_size >> PAGE_SHIFT, 32<<(20-12));
>  
> -#ifdef CONFIG_ARM_64
> -    /* Compute the number of second level pages. */
> -    nr_second = ROUNDUP(frametable_size, FIRST_SIZE) >> FIRST_SHIFT;
> -    second_base = alloc_boot_pages(nr_second, 1);
> -    second = mfn_to_virt(second_base);
> -    for ( i = 0; i < nr_second; i++ )
> -    {
> -        clear_page(mfn_to_virt(mfn_add(second_base, i)));
> -        pte = mfn_to_xen_entry(mfn_add(second_base, i), MT_NORMAL);
> -        pte.pt.table = 1;
> -        write_pte(&xen_first[first_table_offset(FRAMETABLE_VIRT_START)+i], pte);
> -    }
> -    create_mappings(second, 0, mfn_x(base_mfn), frametable_size >> PAGE_SHIFT,
> -                    mapping_size);
> -#else
> -    create_mappings(xen_second, FRAMETABLE_VIRT_START, mfn_x(base_mfn),
> -                    frametable_size >> PAGE_SHIFT, mapping_size);
> -#endif
> +    rc = map_pages_to_xen(FRAMETABLE_VIRT_START, base_mfn,
> +                          frametable_size >> PAGE_SHIFT,
> +                          PAGE_HYPERVISOR_RW | _PAGE_BLOCK);
> +    if ( rc )
> +        panic("Unable to setup the frametable mappings.\n");
>  
>      memset(&frame_table[0], 0, nr_pdxs * sizeof(struct page_info));
>      memset(&frame_table[nr_pdxs], -1,
> -- 
> 2.32.0
> 

Re: [PATCH 10/16] xen/arm: add Persistent Map (PMAP) infrastructure

[Julien Grall]

Hi Stefano,

On 08/06/2022 02:08, Stefano Stabellini wrote:
>> diff --git a/xen/arch/arm/include/asm/pmap.h b/xen/arch/arm/include/asm/pmap.h
>> new file mode 100644
>> index 000000000000..74398b4c4fe6
>> --- /dev/null
>> +++ b/xen/arch/arm/include/asm/pmap.h
>> @@ -0,0 +1,32 @@
>> +#ifndef __ASM_PMAP_H__
>> +#define __ASM_PMAP_H__
>> +
>> +#include <xen/mm.h>
>> +
>> +#include <asm/fixmap.h>
>> +
>> +static inline void arch_pmap_map(unsigned int slot, mfn_t mfn)
>> +{
>> +    lpae_t *entry = &xen_fixmap[slot];
>> +    lpae_t pte;
>> +
>> +    ASSERT(!lpae_is_valid(*entry));
>> +
>> +    pte = mfn_to_xen_entry(mfn, PAGE_HYPERVISOR_RW);
>> +    pte.pt.table = 1;
>> +    write_pte(entry, pte);
> 
> Here we don't need a tlb flush because we never go from a valid mapping
> to another valid mapping.

A TLB flush would not be sufficient here. You would need to follow the 
break-before-make sequence in order to replace a valid mapping with 
another valid mapping.

> We also go through arch_pmap_unmap which
> clears the mapping and also flushes the tlb. Is that right?

The PMAP code is using a bitmap to know which entry is used. So when 
arch_pmap_map() is called, we also guarantees the entry will be invalid 
(hence the ASSERT(!lpae_is_valid()).

The bit in the bitmap will only be cleared by pmap_unmap() which will 
result to a TLB flush.

>> +}
>> +
>> +static inline void arch_pmap_unmap(unsigned int slot)
>> +{
>> +    lpae_t pte = {};
>> +
>> +    write_pte(&xen_fixmap[slot], pte);
>> +
>> +    flush_xen_tlb_range_va_local(FIXMAP_ADDR(slot), PAGE_SIZE);
>> +}
>> +
>> +void arch_pmap_map_slot(unsigned int slot, mfn_t mfn);
>> +void arch_pmap_clear_slot(void *ptr);
> 
> What are these two? They are not defined anywhere?

It is left-over. I will drop them.

Cheers,

-- 
Julien Grall

Re: [PATCH 10/16] xen/arm: add Persistent Map (PMAP) infrastructure

[Stefano Stabellini]

On Wed, 8 Jun 2022, Julien Grall wrote:
> On 08/06/2022 02:08, Stefano Stabellini wrote:
> > > diff --git a/xen/arch/arm/include/asm/pmap.h
> > > b/xen/arch/arm/include/asm/pmap.h
> > > new file mode 100644
> > > index 000000000000..74398b4c4fe6
> > > --- /dev/null
> > > +++ b/xen/arch/arm/include/asm/pmap.h
> > > @@ -0,0 +1,32 @@
> > > +#ifndef __ASM_PMAP_H__
> > > +#define __ASM_PMAP_H__
> > > +
> > > +#include <xen/mm.h>
> > > +
> > > +#include <asm/fixmap.h>
> > > +
> > > +static inline void arch_pmap_map(unsigned int slot, mfn_t mfn)
> > > +{
> > > +    lpae_t *entry = &xen_fixmap[slot];
> > > +    lpae_t pte;
> > > +
> > > +    ASSERT(!lpae_is_valid(*entry));
> > > +
> > > +    pte = mfn_to_xen_entry(mfn, PAGE_HYPERVISOR_RW);
> > > +    pte.pt.table = 1;
> > > +    write_pte(entry, pte);
> > 
> > Here we don't need a tlb flush because we never go from a valid mapping
> > to another valid mapping.
> 
> A TLB flush would not be sufficient here. You would need to follow the
> break-before-make sequence in order to replace a valid mapping with another
> valid mapping.
> 
> > We also go through arch_pmap_unmap which
> > clears the mapping and also flushes the tlb. Is that right?
> 
> The PMAP code is using a bitmap to know which entry is used. So when
> arch_pmap_map() is called, we also guarantees the entry will be invalid (hence
> the ASSERT(!lpae_is_valid()).
> 
> The bit in the bitmap will only be cleared by pmap_unmap() which will result
> to a TLB flush.
> 
> > > +}
> > > +
> > > +static inline void arch_pmap_unmap(unsigned int slot)
> > > +{
> > > +    lpae_t pte = {};
> > > +
> > > +    write_pte(&xen_fixmap[slot], pte);
> > > +
> > > +    flush_xen_tlb_range_va_local(FIXMAP_ADDR(slot), PAGE_SIZE);
> > > +}
> > > +
> > > +void arch_pmap_map_slot(unsigned int slot, mfn_t mfn);
> > > +void arch_pmap_clear_slot(void *ptr);
> > 
> > What are these two? They are not defined anywhere?
> 
> It is left-over. I will drop them.

With those two functions removed, you can add my 

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

Re: [PATCH 00/16] xen/arm: mm: Remove open-coding mappings

[Julien Grall]

Hi,

On 20/05/2022 13:09, Julien Grall wrote:
> Julien Grall (15):
>    xen/arm: mm: Allow other mapping size in xen_pt_update_entry()
>    xen/arm: mm: Add support for the contiguous bit
>    xen/arm: mm: Avoid flushing the TLBs when mapping are inserted
>    xen/arm: mm: Don't open-code Xen PT update in remove_early_mappings()
>    xen/arm: mm: Re-implement early_fdt_map() using map_pages_to_xen()
>    xen/arm32: mm: Re-implement setup_xenheap_mappings() using
>      map_pages_to_xen()
>    xen/arm: mm: Allocate xen page tables in domheap rather than xenheap
>    xen/arm: mm: Allow page-table allocation from the boot allocator
>    xen/arm: Move fixmap definitions in a separate header
>    xen/arm: mm: Clean-up the includes and order them
>    xen/arm: mm: Use the PMAP helpers in xen_{,un}map_table()
>    xen/arm32: setup: Move out the code to populate the boot allocator
>    xen/arm64: mm: Add memory to the boot allocator first
>    xen/arm: mm: Rework setup_xenheap_mappings()
>    xen/arm: mm: Re-implement setup_frame_table_mappings() with
>      map_pages_to_xen()
> 
> Wei Liu (1):
>    xen/arm: add Persistent Map (PMAP) infrastructure

I have now committed the full series.

Cheers,

-- 
Julien Grall