[PATCH v2 0/5] Third series for R82 MPU support

[PATCH v2 0/5] Third series for R82 MPU support

[Hari Limaye]

Hi all,

This series is the third set of patches in the ongoing work to
introduce support for MPU systems and Cortex R82 in Xen.

The patches in this series implement the necessary logic to transiently
map and unmap the static memory banks for initialization.

Changes from v1:
- Changes mentioned in individual patches

Cheers,
Hari

Luca Fancellu (4):
  arm/mpu: Implement setup_frametable_mappings for MPU systems
  arm/mpu: Implement setup_mm for MPU systems
  arm/mpu: Implement transient mapping
  arm/mpu: Implement ioremap_attr for MPU

Penny Zheng (1):
  xen/arm: map static memory on demand

 xen/arch/arm/include/asm/arm32/mpu.h     |   2 +
 xen/arch/arm/include/asm/arm64/mpu.h     |   2 +
 xen/arch/arm/include/asm/mmu/mm.h        |   3 +
 xen/arch/arm/include/asm/mpu/mm.h        |  40 +++-
 xen/arch/arm/include/asm/mpu/regions.inc |  19 +-
 xen/arch/arm/mpu/mm.c                    | 262 +++++++++++++++++++++--
 xen/arch/arm/mpu/setup.c                 |  11 +
 xen/include/xen/static-memory.h          |   8 +
 8 files changed, 328 insertions(+), 19 deletions(-)

-- 
2.34.1

[PATCH v2 1/5] arm/mpu: Implement setup_frametable_mappings for MPU systems

[Hari Limaye]

From: Luca Fancellu <luca.fancellu@arm.com>

Implement the MPU variant of `setup_frametable_mappings`. This function
will be called by `setup_mm` when an implementation for MPU systems is
added in a follow up commit.

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Signed-off-by: Hari Limaye <hari.limaye@arm.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
---
Changes from v1:
- Align ps and pe to page size
- Add sanity checking for frametable size, as in MMU version
- Add Michal's R-b
---
 xen/arch/arm/mpu/mm.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
index c6891607ec..3f155b7db2 100644
--- a/xen/arch/arm/mpu/mm.c
+++ b/xen/arch/arm/mpu/mm.c
@@ -168,6 +168,39 @@ int mpumap_contains_region(pr_t *table, uint8_t nr_regions, paddr_t base,
     return MPUMAP_REGION_NOTFOUND;
 }
 
+/* Map a frame table to cover physical addresses ps through pe */
+void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
+{
+    mfn_t base_mfn;
+    paddr_t aligned_ps = ROUNDUP(ps, PAGE_SIZE);
+    paddr_t aligned_pe = ROUNDDOWN(pe, PAGE_SIZE);
+
+    unsigned long nr_pdxs = mfn_to_pdx(mfn_add(maddr_to_mfn(aligned_pe), -1)) -
+                            mfn_to_pdx(maddr_to_mfn(aligned_ps)) + 1;
+    unsigned long frametable_size = nr_pdxs * sizeof(struct page_info);
+
+    /*
+     * The size of paddr_t should be sufficient for the complete range of
+     * physical address.
+     */
+    BUILD_BUG_ON((sizeof(paddr_t) * BITS_PER_BYTE) < PADDR_BITS);
+    BUILD_BUG_ON(sizeof(struct page_info) != PAGE_INFO_SIZE);
+
+    if ( frametable_size > FRAMETABLE_SIZE )
+        panic("The frametable cannot cover the physical region %#"PRIpaddr" - %#"PRIpaddr"\n",
+              ps, pe);
+
+    frametable_base_pdx = paddr_to_pdx(aligned_ps);
+    frametable_size = ROUNDUP(frametable_size, PAGE_SIZE);
+
+    base_mfn = alloc_boot_pages(frametable_size >> PAGE_SHIFT, 1);
+    frame_table = (struct page_info *)mfn_to_virt(mfn_x(base_mfn));
+
+    memset(&frame_table[0], 0, nr_pdxs * sizeof(struct page_info));
+    memset(&frame_table[nr_pdxs], -1,
+           frametable_size - (nr_pdxs * sizeof(struct page_info)));
+}
+
 /*
  * Allocate an entry for a new EL2 MPU region in the bitmap xen_mpumap_mask.
  * @param idx   Set to the index of the allocated EL2 MPU region on success.
-- 
2.34.1

[PATCH v2 2/5] arm/mpu: Implement setup_mm for MPU systems

[Hari Limaye]

From: Luca Fancellu <luca.fancellu@arm.com>

Implement `setup_mm` for MPU systems. This variant doesn't need to set
up the direct map.

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Signed-off-by: Hari Limaye <hari.limaye@arm.com>
---
Changes from v1:
- Fix total_pages dead assignment
- Remove extraneous space
- Remove redundant max_page assignment
---
 xen/arch/arm/mpu/mm.c | 62 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
index 3f155b7db2..4c517d6e43 100644
--- a/xen/arch/arm/mpu/mm.c
+++ b/xen/arch/arm/mpu/mm.c
@@ -8,9 +8,12 @@
 #include <xen/sizes.h>
 #include <xen/spinlock.h>
 #include <xen/types.h>
+#include <xen/static-memory.h>
+#include <xen/static-shmem.h>
 #include <asm/mpu.h>
 #include <asm/mpu/mm.h>
 #include <asm/page.h>
+#include <asm/setup.h>
 #include <asm/sysregs.h>
 
 struct page_info *frame_table;
@@ -378,9 +381,66 @@ int map_pages_to_xen(unsigned long virt, mfn_t mfn, unsigned long nr_mfns,
     return xen_mpumap_update(virt, mfn_to_maddr(mfn_add(mfn, nr_mfns)), flags);
 }
 
+/*
+ * Heap must be statically configured in Device Tree through "xen,static-heap"
+ * on MPU systems.
+ */
+static void __init setup_staticheap_mappings(void)
+{
+    const struct membanks *reserved_mem = bootinfo_get_reserved_mem();
+    unsigned int bank = 0;
+
+    for ( ; bank < reserved_mem->nr_banks; bank++ )
+    {
+        if ( reserved_mem->bank[bank].type == MEMBANK_STATIC_HEAP )
+        {
+            paddr_t bank_start = round_pgup(reserved_mem->bank[bank].start);
+            paddr_t bank_size = round_pgdown(reserved_mem->bank[bank].size);
+            paddr_t bank_end = bank_start + bank_size;
+
+            /* Map static heap with one MPU protection region */
+            if ( xen_mpumap_update(bank_start, bank_end, PAGE_HYPERVISOR) )
+                panic("Failed to map static heap\n");
+
+            break;
+        }
+    }
+
+    if ( bank == reserved_mem->nr_banks )
+        panic("No static heap memory bank found\n");
+}
+
 void __init setup_mm(void)
 {
-    BUG_ON("unimplemented");
+    const struct membanks *mem = bootinfo_get_mem();
+    paddr_t ram_start = INVALID_PADDR, ram_end = 0, ram_size = 0;
+
+    if ( !mem->nr_banks )
+        panic("No memory bank\n");
+
+    init_pdx();
+
+    populate_boot_allocator();
+
+    for ( unsigned int bank = 0; bank < mem->nr_banks; bank++ )
+    {
+        paddr_t bank_start = round_pgup(mem->bank[bank].start);
+        paddr_t bank_size = round_pgdown(mem->bank[bank].size);
+        paddr_t bank_end = bank_start + bank_size;
+
+        ram_size = ram_size + bank_size;
+        ram_start = min(ram_start, bank_start);
+        ram_end = max(ram_end, bank_end);
+    }
+
+    setup_staticheap_mappings();
+
+    total_pages = ram_size >> PAGE_SHIFT;
+
+    setup_frametable_mappings(ram_start, ram_end);
+
+    init_staticmem_pages();
+    init_sharedmem_pages();
 }
 
 int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int nf)
-- 
2.34.1

[PATCH v2 3/5] arm/mpu: Implement transient mapping

[Hari Limaye]

From: Luca Fancellu <luca.fancellu@arm.com>

Add a scheme to distinguish transient MPU regions, to identify MPU
regions which will be mapped for a short period of time. This is needed
for the functions which transiently map and unmap memory ranges on
demand which will be introduced in a future commit.

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Signed-off-by: Hari Limaye <hari.limaye@arm.com>
---
Changes from v1:
- Improve commit message
- Mark parameter in read helper as const
---
 xen/arch/arm/include/asm/arm32/mpu.h     |  2 ++
 xen/arch/arm/include/asm/arm64/mpu.h     |  2 ++
 xen/arch/arm/include/asm/mpu/mm.h        | 14 +++++++++++++-
 xen/arch/arm/include/asm/mpu/regions.inc | 19 +++++++++++++++++--
 xen/arch/arm/mpu/mm.c                    | 23 ++++++++++++++---------
 5 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/xen/arch/arm/include/asm/arm32/mpu.h b/xen/arch/arm/include/asm/arm32/mpu.h
index 0a6930b3a0..9906d98809 100644
--- a/xen/arch/arm/include/asm/arm32/mpu.h
+++ b/xen/arch/arm/include/asm/arm32/mpu.h
@@ -39,6 +39,8 @@ typedef union {
 typedef struct {
     prbar_t prbar;
     prlar_t prlar;
+    bool transient;
+    uint8_t pad[7]; /* Pad structure to 16 Bytes */
 } pr_t;
 
 #endif /* __ASSEMBLY__ */
diff --git a/xen/arch/arm/include/asm/arm64/mpu.h b/xen/arch/arm/include/asm/arm64/mpu.h
index f0ce344e78..1d1843eda0 100644
--- a/xen/arch/arm/include/asm/arm64/mpu.h
+++ b/xen/arch/arm/include/asm/arm64/mpu.h
@@ -38,6 +38,8 @@ typedef union {
 typedef struct {
     prbar_t prbar;
     prlar_t prlar;
+    bool transient;
+    uint8_t pad[15]; /* Pad structure to 32 Bytes */
 } pr_t;
 
 #endif /* __ASSEMBLY__ */
diff --git a/xen/arch/arm/include/asm/mpu/mm.h b/xen/arch/arm/include/asm/mpu/mm.h
index e1ded6521d..566d338986 100644
--- a/xen/arch/arm/include/asm/mpu/mm.h
+++ b/xen/arch/arm/include/asm/mpu/mm.h
@@ -55,6 +55,16 @@ static inline void context_sync_mpu(void)
     isb();
 }
 
+static inline bool region_is_transient(const pr_t *pr)
+{
+    return pr->transient;
+}
+
+static inline void region_set_transient(pr_t *pr, bool transient)
+{
+    pr->transient = transient;
+}
+
 /*
  * The following API requires context_sync_mpu() after being used to modify MPU
  * regions:
@@ -75,9 +85,11 @@ void write_protection_region(const pr_t *pr_write, uint8_t sel);
  * @param base      Base address of the range to map (inclusive).
  * @param limit     Limit address of the range to map (exclusive).
  * @param flags     Flags for the memory range to map.
+ * @param transient True for a transient mapping, otherwise False.
  * @return          0 on success, negative on error.
  */
-int xen_mpumap_update(paddr_t base, paddr_t limit, unsigned int flags);
+int xen_mpumap_update(paddr_t base, paddr_t limit, unsigned int flags,
+                      bool transient);
 
 /*
  * Creates a pr_t structure describing a protection region.
diff --git a/xen/arch/arm/include/asm/mpu/regions.inc b/xen/arch/arm/include/asm/mpu/regions.inc
index 23fead3b21..f9892fe3d8 100644
--- a/xen/arch/arm/include/asm/mpu/regions.inc
+++ b/xen/arch/arm/include/asm/mpu/regions.inc
@@ -14,19 +14,31 @@
 #define PRLAR_ELx_EN            0x1
 
 #ifdef CONFIG_ARM_64
-#define XEN_MPUMAP_ENTRY_SHIFT  0x4     /* 16 byte structure */
+#define XEN_MPUMAP_ENTRY_SHIFT        0x5   /* 32 byte structure */
+#define XEN_MPUMAP_ENTRY_ZERO_OFFSET  0x10  /* {PRBAR, PRLAR} is 16 bytes  */
 
 .macro store_pair reg1, reg2, dst
     stp \reg1, \reg2, [\dst]
 .endm
 
+.macro zero_pair dst, offset, tmp1, tmp2
+    stp xzr, xzr, [\dst, \offset]
+.endm
+
 #else
-#define XEN_MPUMAP_ENTRY_SHIFT  0x3     /* 8 byte structure */
+#define XEN_MPUMAP_ENTRY_SHIFT        0x4   /* 16 byte structure */
+#define XEN_MPUMAP_ENTRY_ZERO_OFFSET  0x8   /* {PRBAR, PRLAR} is 8 bytes  */
 
 .macro store_pair reg1, reg2, dst
     strd  \reg1, \reg2, [\dst]
 .endm
 
+.macro zero_pair dst, offset, tmp1, tmp2
+    mov \tmp1, #0
+    mov \tmp2, #0
+    strd \tmp1, \tmp2, [\dst, \offset]
+.endm
+
 #endif
 
 /*
@@ -97,6 +109,9 @@
 
 3:
 
+    /* Clear the rest of the xen_mpumap entry. Clobbers prbar and prlar. */
+    zero_pair \base, #XEN_MPUMAP_ENTRY_ZERO_OFFSET, \prbar, \prlar
+
     add   \sel, \sel, #1
 
 1:
diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
index 4c517d6e43..33333181d5 100644
--- a/xen/arch/arm/mpu/mm.c
+++ b/xen/arch/arm/mpu/mm.c
@@ -265,13 +265,14 @@ static void disable_mpu_region_from_index(uint8_t index)
  * Update the entry in the MPU memory region mapping table (xen_mpumap) for the
  * given memory range and flags, creating one if none exists.
  *
- * @param base  Base address (inclusive).
- * @param limit Limit address (exclusive).
- * @param flags Region attributes (a combination of PAGE_HYPERVISOR_XXX)
+ * @param base      Base address (inclusive).
+ * @param limit     Limit address (exclusive).
+ * @param flags     Region attributes (a combination of PAGE_HYPERVISOR_XXX)
+ * @param transient True for a transient mapping, otherwise False.
  * @return      0 on success, otherwise negative on error.
  */
 static int xen_mpumap_update_entry(paddr_t base, paddr_t limit,
-                                   unsigned int flags)
+                                   unsigned int flags, bool transient)
 {
     bool flags_has_page_present;
     uint8_t idx;
@@ -311,6 +312,7 @@ static int xen_mpumap_update_entry(paddr_t base, paddr_t limit,
             return -ENOENT;
 
         xen_mpumap[idx] = pr_of_addr(base, limit, flags);
+        region_set_transient(&xen_mpumap[idx], transient);
 
         write_protection_region(&xen_mpumap[idx], idx);
     }
@@ -330,7 +332,8 @@ static int xen_mpumap_update_entry(paddr_t base, paddr_t limit,
     return 0;
 }
 
-int xen_mpumap_update(paddr_t base, paddr_t limit, unsigned int flags)
+int xen_mpumap_update(paddr_t base, paddr_t limit, unsigned int flags,
+                      bool transient)
 {
     int rc;
 
@@ -356,7 +359,7 @@ int xen_mpumap_update(paddr_t base, paddr_t limit, unsigned int flags)
 
     spin_lock(&xen_mpumap_lock);
 
-    rc = xen_mpumap_update_entry(base, limit, flags);
+    rc = xen_mpumap_update_entry(base, limit, flags, transient);
     if ( !rc )
         context_sync_mpu();
 
@@ -371,14 +374,15 @@ int destroy_xen_mappings(unsigned long s, unsigned long e)
     ASSERT(IS_ALIGNED(e, PAGE_SIZE));
     ASSERT(s < e);
 
-    return xen_mpumap_update(s, e, 0);
+    return xen_mpumap_update(s, e, 0, false);
 }
 
 int map_pages_to_xen(unsigned long virt, mfn_t mfn, unsigned long nr_mfns,
                      unsigned int flags)
 {
     /* MPU systems have no translation, ma == va, so pass virt directly */
-    return xen_mpumap_update(virt, mfn_to_maddr(mfn_add(mfn, nr_mfns)), flags);
+    return xen_mpumap_update(virt, mfn_to_maddr(mfn_add(mfn, nr_mfns)), flags,
+                             false);
 }
 
 /*
@@ -399,7 +403,8 @@ static void __init setup_staticheap_mappings(void)
             paddr_t bank_end = bank_start + bank_size;
 
             /* Map static heap with one MPU protection region */
-            if ( xen_mpumap_update(bank_start, bank_end, PAGE_HYPERVISOR) )
+            if ( xen_mpumap_update(bank_start, bank_end, PAGE_HYPERVISOR,
+                                   false) )
                 panic("Failed to map static heap\n");
 
             break;
-- 
2.34.1

[PATCH v2 4/5] arm/mpu: Implement ioremap_attr for MPU

[Hari Limaye]

From: Luca Fancellu <luca.fancellu@arm.com>

Introduce helpers (un)map_mm_range() in order to allow the transient
mapping of a range of memory, and use these to implement the function
`ioremap_attr` for MPU systems.

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Signed-off-by: Hari Limaye <hari.limaye@arm.com>
---
Changes from v1:
- Use transient instead of temporary, and improve wording of comments
  regarding transient mapping
- Rename start, end -> base, limit
---
 xen/arch/arm/include/asm/mpu/mm.h |  22 +++++
 xen/arch/arm/mpu/mm.c             | 150 ++++++++++++++++++++++++++++--
 2 files changed, 163 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/include/asm/mpu/mm.h b/xen/arch/arm/include/asm/mpu/mm.h
index 566d338986..efb0680e39 100644
--- a/xen/arch/arm/include/asm/mpu/mm.h
+++ b/xen/arch/arm/include/asm/mpu/mm.h
@@ -101,6 +101,28 @@ int xen_mpumap_update(paddr_t base, paddr_t limit, unsigned int flags,
  */
 pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags);
 
+/*
+ * Maps transiently a range of memory with attributes `flags`; if the range is
+ * already mapped with the same attributes, including an inclusive match, the
+ * existing mapping is returned. This API is intended for mappings that exist
+ * transiently for a short period between calls to this function and
+ * `unmap_mm_range`.
+ *
+ * @param base      Base address of the range to map (inclusive).
+ * @param limit     Limit address of the range to map (exclusive).
+ * @param flags     Flags for the memory range to map.
+ * @return          Pointer to base of region on success, NULL on error.
+ */
+void *map_mm_range(paddr_t base, paddr_t limit, unsigned int flags);
+
+/*
+ * Unmaps a range of memory if it was previously mapped by map_mm_range,
+ * otherwise it does not remove the mapping.
+ *
+ * @param base     Base address of the range to map (inclusive).
+ */
+void unmap_mm_range(paddr_t base);
+
 /*
  * Checks whether a given memory range is present in the provided table of
  * MPU protection regions.
diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
index 33333181d5..52c4c43827 100644
--- a/xen/arch/arm/mpu/mm.c
+++ b/xen/arch/arm/mpu/mm.c
@@ -332,31 +332,39 @@ static int xen_mpumap_update_entry(paddr_t base, paddr_t limit,
     return 0;
 }
 
-int xen_mpumap_update(paddr_t base, paddr_t limit, unsigned int flags,
-                      bool transient)
+static bool check_mpu_mapping(paddr_t base, paddr_t limit, unsigned int flags)
 {
-    int rc;
-
     if ( flags_has_rwx(flags) )
     {
         printk("Mappings should not be both Writeable and Executable\n");
-        return -EINVAL;
+        return false;
     }
 
     if ( base >= limit )
     {
         printk("Base address %#"PRIpaddr" must be smaller than limit address %#"PRIpaddr"\n",
                base, limit);
-        return -EINVAL;
+        return false;
     }
 
     if ( !IS_ALIGNED(base, PAGE_SIZE) || !IS_ALIGNED(limit, PAGE_SIZE) )
     {
         printk("base address %#"PRIpaddr", or limit address %#"PRIpaddr" is not page aligned\n",
                base, limit);
-        return -EINVAL;
+        return false;
     }
 
+    return true;
+}
+
+int xen_mpumap_update(paddr_t base, paddr_t limit, unsigned int flags,
+                      bool transient)
+{
+    int rc;
+
+    if ( !check_mpu_mapping(base, limit, flags) )
+        return -EINVAL;
+
     spin_lock(&xen_mpumap_lock);
 
     rc = xen_mpumap_update_entry(base, limit, flags, transient);
@@ -465,10 +473,134 @@ void free_init_memory(void)
     BUG_ON("unimplemented");
 }
 
+static uint8_t is_mm_range_mapped(paddr_t start, paddr_t end)
+{
+    int rc;
+    uint8_t idx;
+
+    ASSERT(spin_is_locked(&xen_mpumap_lock));
+
+    rc = mpumap_contains_region(xen_mpumap, max_mpu_regions, start, end, &idx);
+    if ( rc < 0 )
+         panic("Cannot handle overlapping MPU memory protection regions\n");
+
+    /*
+     * 'idx' will be INVALID_REGION_IDX for rc == MPUMAP_REGION_NOTFOUND and
+     * it will be a proper region index when rc >= MPUMAP_REGION_FOUND.
+     */
+    return idx;
+}
+
+static bool is_mm_attr_match(pr_t *region, unsigned int attributes)
+{
+    bool ret = true;
+
+    if ( region->prbar.reg.ro != PAGE_RO_MASK(attributes) )
+    {
+        printk(XENLOG_WARNING
+               "Mismatched Access Permission attributes (%#x0 instead of %#x0)\n",
+               region->prbar.reg.ro, PAGE_RO_MASK(attributes));
+        ret = false;
+    }
+
+    if ( region->prbar.reg.xn != PAGE_XN_MASK(attributes) )
+    {
+        printk(XENLOG_WARNING
+               "Mismatched Execute Never attributes (%#x instead of %#x)\n",
+               region->prbar.reg.xn, PAGE_XN_MASK(attributes));
+        ret = false;
+    }
+
+    if ( region->prlar.reg.ai != PAGE_AI_MASK(attributes) )
+    {
+        printk(XENLOG_WARNING
+               "Mismatched Memory Attribute Index (%#x instead of %#x)\n",
+               region->prlar.reg.ai, PAGE_AI_MASK(attributes));
+        ret = false;
+    }
+
+    return ret;
+}
+
+void *map_mm_range(paddr_t base, paddr_t limit, unsigned int flags)
+{
+    paddr_t start_pg = round_pgdown(base);
+    paddr_t end_pg = round_pgup(limit);
+    void *ret = NULL;
+    uint8_t idx;
+
+    if ( !check_mpu_mapping(start_pg, end_pg, flags) )
+        return NULL;
+
+    spin_lock(&xen_mpumap_lock);
+
+    idx = is_mm_range_mapped(start_pg, end_pg);
+    if ( idx != INVALID_REGION_IDX )
+    {
+        /* Already mapped with different attributes */
+        if ( !is_mm_attr_match(&xen_mpumap[idx], flags) )
+        {
+            printk(XENLOG_WARNING
+                   "Range %#"PRIpaddr"-%#"PRIpaddr" already mapped with different flags\n",
+                   start_pg, end_pg);
+            goto out;
+        }
+
+        /* Already mapped with same attributes */
+        ret = maddr_to_virt(base);
+        goto out;
+    }
+
+    if ( !xen_mpumap_update_entry(start_pg, end_pg, flags, true) )
+    {
+        context_sync_mpu();
+        ret = maddr_to_virt(base);
+    }
+
+ out:
+    spin_unlock(&xen_mpumap_lock);
+
+    return ret;
+}
+
+void unmap_mm_range(paddr_t base)
+{
+    uint8_t idx;
+
+    spin_lock(&xen_mpumap_lock);
+
+    /*
+     * Mappings created via map_mm_range are at least PAGE_SIZE. Find the idx
+     * of the MPU memory region containing `start` mapped through map_mm_range.
+     */
+    idx = is_mm_range_mapped(base, base + PAGE_SIZE);
+    if ( idx == INVALID_REGION_IDX )
+    {
+        printk(XENLOG_ERR
+               "Failed to unmap_mm_range MPU memory region at %#"PRIpaddr"\n",
+               base);
+        goto out;
+    }
+
+    /* This API is only meant to unmap transient regions */
+    if ( !region_is_transient(&xen_mpumap[idx]) )
+        goto out;
+
+    /* Disable MPU memory region and clear the associated entry in xen_mpumap */
+    disable_mpu_region_from_index(idx);
+    context_sync_mpu();
+
+ out:
+    spin_unlock(&xen_mpumap_lock);
+}
+
 void __iomem *ioremap_attr(paddr_t start, size_t len, unsigned int flags)
 {
-    BUG_ON("unimplemented");
-    return NULL;
+    if ( !map_mm_range(start, start + len, flags) )
+        return NULL;
+
+    /* Mapped or already mapped */
+    return maddr_to_virt(start);
 }
 
 /*
-- 
2.34.1

[PATCH v2 5/5] xen/arm: map static memory on demand

[Hari Limaye]

From: Penny Zheng <Penny.Zheng@arm.com>

In the function `init_staticmem_pages` we need to have mapped static
memory banks for initialization. Unlike on an MMU system, we cannot map
the entire RAM on an MPU system as we have a limited number of MPU
memory regions. To solve this, transiently map the static memory banks
for initialization.

Signed-off-by: Penny Zheng <penny.zheng@arm.com>
Signed-off-by: Wei Chen <wei.chen@arm.com>
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Signed-off-by: Hari Limaye <hari.limaye@arm.com>
---
 xen/arch/arm/include/asm/mmu/mm.h |  3 +++
 xen/arch/arm/include/asm/mpu/mm.h |  4 ++++
 xen/arch/arm/mpu/setup.c          | 11 +++++++++++
 xen/include/xen/static-memory.h   |  8 ++++++++
 4 files changed, 26 insertions(+)

diff --git a/xen/arch/arm/include/asm/mmu/mm.h b/xen/arch/arm/include/asm/mmu/mm.h
index 7f4d59137d..645a0ea3cb 100644
--- a/xen/arch/arm/include/asm/mmu/mm.h
+++ b/xen/arch/arm/include/asm/mmu/mm.h
@@ -110,6 +110,9 @@ void dump_pt_walk(paddr_t ttbr, paddr_t addr,
 extern void switch_ttbr(uint64_t ttbr);
 extern void relocate_and_switch_ttbr(uint64_t ttbr);
 
+static inline void map_staticmem_pages_to_xen(paddr_t start, paddr_t end) {}
+static inline void unmap_staticmem_pages_to_xen(paddr_t start, paddr_t end) {}
+
 #endif /* __ARM_MMU_MM_H__ */
 
 /*
diff --git a/xen/arch/arm/include/asm/mpu/mm.h b/xen/arch/arm/include/asm/mpu/mm.h
index efb0680e39..4cc769418e 100644
--- a/xen/arch/arm/include/asm/mpu/mm.h
+++ b/xen/arch/arm/include/asm/mpu/mm.h
@@ -123,6 +123,10 @@ void *map_mm_range(paddr_t base, paddr_t limit, unsigned int flags);
  */
 void unmap_mm_range(paddr_t base);
 
+/* {un}map_staticmem_pages_to_xen used while initializing static memory banks */
+void map_staticmem_pages_to_xen(paddr_t start, paddr_t end);
+void unmap_staticmem_pages_to_xen(paddr_t start, paddr_t end);
+
 /*
  * Checks whether a given memory range is present in the provided table of
  * MPU protection regions.
diff --git a/xen/arch/arm/mpu/setup.c b/xen/arch/arm/mpu/setup.c
index 163573b932..dbc3107333 100644
--- a/xen/arch/arm/mpu/setup.c
+++ b/xen/arch/arm/mpu/setup.c
@@ -83,6 +83,17 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
     return fdt_virt;
 }
 
+void __init map_staticmem_pages_to_xen(paddr_t start, paddr_t end)
+{
+    if ( !map_mm_range(start, end, PAGE_HYPERVISOR) )
+        panic("Unable to map staticmem pages to Xen!");
+}
+
+void __init unmap_staticmem_pages_to_xen(paddr_t start, paddr_t end)
+{
+    unmap_mm_range(start);
+}
+
 /*
  * copy_from_paddr - copy data from a physical address
  * @dst: destination virtual address
diff --git a/xen/include/xen/static-memory.h b/xen/include/xen/static-memory.h
index e445aa8057..d99abac113 100644
--- a/xen/include/xen/static-memory.h
+++ b/xen/include/xen/static-memory.h
@@ -18,7 +18,15 @@ static inline void init_staticmem_bank(const struct membank *bank)
     if ( mfn_x(bank_end) <= mfn_x(bank_start) )
         return;
 
+    /* Map temporarily before initialization */
+    map_staticmem_pages_to_xen(mfn_to_maddr(bank_start),
+                               mfn_to_maddr(bank_end));
+
     unprepare_staticmem_pages(mfn_to_page(bank_start), bank_pages, false);
+
+    /* Unmap immediately after initialization */
+    unmap_staticmem_pages_to_xen(mfn_to_maddr(bank_start),
+                                 mfn_to_maddr(bank_end));
 }
 
 void allocate_static_memory(struct domain *d, struct kernel_info *kinfo,
-- 
2.34.1

Re: [PATCH v2 2/5] arm/mpu: Implement setup_mm for MPU systems

[Orzel, Michal]

On 27/08/2025 18:35, Hari Limaye wrote:
> From: Luca Fancellu <luca.fancellu@arm.com>
> 
> Implement `setup_mm` for MPU systems. This variant doesn't need to set
> up the direct map.
> 
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> Signed-off-by: Hari Limaye <hari.limaye@arm.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal

Re: [PATCH v2 3/5] arm/mpu: Implement transient mapping

[Orzel, Michal]

On 27/08/2025 18:35, Hari Limaye wrote:
> From: Luca Fancellu <luca.fancellu@arm.com>
> 
> Add a scheme to distinguish transient MPU regions, to identify MPU
> regions which will be mapped for a short period of time. This is needed
> for the functions which transiently map and unmap memory ranges on
> demand which will be introduced in a future commit.
> 
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> Signed-off-by: Hari Limaye <hari.limaye@arm.com>
> ---
> Changes from v1:
> - Improve commit message
> - Mark parameter in read helper as const
> ---
>  xen/arch/arm/include/asm/arm32/mpu.h     |  2 ++
>  xen/arch/arm/include/asm/arm64/mpu.h     |  2 ++
>  xen/arch/arm/include/asm/mpu/mm.h        | 14 +++++++++++++-
>  xen/arch/arm/include/asm/mpu/regions.inc | 19 +++++++++++++++++--
>  xen/arch/arm/mpu/mm.c                    | 23 ++++++++++++++---------
>  5 files changed, 48 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/arm32/mpu.h b/xen/arch/arm/include/asm/arm32/mpu.h
> index 0a6930b3a0..9906d98809 100644
> --- a/xen/arch/arm/include/asm/arm32/mpu.h
> +++ b/xen/arch/arm/include/asm/arm32/mpu.h
> @@ -39,6 +39,8 @@ typedef union {
>  typedef struct {
>      prbar_t prbar;
>      prlar_t prlar;
> +    bool transient;
Do we expect to have any other flags? If so, it could make sense to use a
bitfield right away.

> +    uint8_t pad[7]; /* Pad structure to 16 Bytes */
>  } pr_t;
>  
>  #endif /* __ASSEMBLY__ */
> diff --git a/xen/arch/arm/include/asm/arm64/mpu.h b/xen/arch/arm/include/asm/arm64/mpu.h
> index f0ce344e78..1d1843eda0 100644
> --- a/xen/arch/arm/include/asm/arm64/mpu.h
> +++ b/xen/arch/arm/include/asm/arm64/mpu.h
> @@ -38,6 +38,8 @@ typedef union {
>  typedef struct {
>      prbar_t prbar;
>      prlar_t prlar;
> +    bool transient;
> +    uint8_t pad[15]; /* Pad structure to 32 Bytes */
>  } pr_t;
>  
>  #endif /* __ASSEMBLY__ */
> diff --git a/xen/arch/arm/include/asm/mpu/mm.h b/xen/arch/arm/include/asm/mpu/mm.h
> index e1ded6521d..566d338986 100644
> --- a/xen/arch/arm/include/asm/mpu/mm.h
> +++ b/xen/arch/arm/include/asm/mpu/mm.h
> @@ -55,6 +55,16 @@ static inline void context_sync_mpu(void)
>      isb();
>  }
>  
> +static inline bool region_is_transient(const pr_t *pr)
> +{
> +    return pr->transient;
> +}
> +
> +static inline void region_set_transient(pr_t *pr, bool transient)
> +{
> +    pr->transient = transient;
> +}
> +
>  /*
>   * The following API requires context_sync_mpu() after being used to modify MPU
>   * regions:
> @@ -75,9 +85,11 @@ void write_protection_region(const pr_t *pr_write, uint8_t sel);
>   * @param base      Base address of the range to map (inclusive).
>   * @param limit     Limit address of the range to map (exclusive).
>   * @param flags     Flags for the memory range to map.
> + * @param transient True for a transient mapping, otherwise False.
>   * @return          0 on success, negative on error.
>   */
> -int xen_mpumap_update(paddr_t base, paddr_t limit, unsigned int flags);
> +int xen_mpumap_update(paddr_t base, paddr_t limit, unsigned int flags,
> +                      bool transient);
>  
>  /*
>   * Creates a pr_t structure describing a protection region.
> diff --git a/xen/arch/arm/include/asm/mpu/regions.inc b/xen/arch/arm/include/asm/mpu/regions.inc
> index 23fead3b21..f9892fe3d8 100644
> --- a/xen/arch/arm/include/asm/mpu/regions.inc
> +++ b/xen/arch/arm/include/asm/mpu/regions.inc
> @@ -14,19 +14,31 @@
>  #define PRLAR_ELx_EN            0x1
>  
>  #ifdef CONFIG_ARM_64
> -#define XEN_MPUMAP_ENTRY_SHIFT  0x4     /* 16 byte structure */
> +#define XEN_MPUMAP_ENTRY_SHIFT        0x5   /* 32 byte structure */
> +#define XEN_MPUMAP_ENTRY_ZERO_OFFSET  0x10  /* {PRBAR, PRLAR} is 16 bytes  */
To avoid having to update this over and over again, maybe define a macro using
OFFSET?

Other than that:
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal

Re: [PATCH v2 4/5] arm/mpu: Implement ioremap_attr for MPU

[Orzel, Michal]

On 27/08/2025 18:35, Hari Limaye wrote:
> From: Luca Fancellu <luca.fancellu@arm.com>
> 
> Introduce helpers (un)map_mm_range() in order to allow the transient
> mapping of a range of memory, and use these to implement the function
> `ioremap_attr` for MPU systems.
> 
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> Signed-off-by: Hari Limaye <hari.limaye@arm.com>
> ---
> Changes from v1:
> - Use transient instead of temporary, and improve wording of comments
>   regarding transient mapping
> - Rename start, end -> base, limit
> ---
>  xen/arch/arm/include/asm/mpu/mm.h |  22 +++++
>  xen/arch/arm/mpu/mm.c             | 150 ++++++++++++++++++++++++++++--
>  2 files changed, 163 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/mpu/mm.h b/xen/arch/arm/include/asm/mpu/mm.h
> index 566d338986..efb0680e39 100644
> --- a/xen/arch/arm/include/asm/mpu/mm.h
> +++ b/xen/arch/arm/include/asm/mpu/mm.h
> @@ -101,6 +101,28 @@ int xen_mpumap_update(paddr_t base, paddr_t limit, unsigned int flags,
>   */
>  pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags);
>  
> +/*
> + * Maps transiently a range of memory with attributes `flags`; if the range is
> + * already mapped with the same attributes, including an inclusive match, the
> + * existing mapping is returned. This API is intended for mappings that exist
> + * transiently for a short period between calls to this function and
> + * `unmap_mm_range`.
> + *
> + * @param base      Base address of the range to map (inclusive).
> + * @param limit     Limit address of the range to map (exclusive).
> + * @param flags     Flags for the memory range to map.
> + * @return          Pointer to base of region on success, NULL on error.
> + */
> +void *map_mm_range(paddr_t base, paddr_t limit, unsigned int flags);
> +
> +/*
> + * Unmaps a range of memory if it was previously mapped by map_mm_range,
> + * otherwise it does not remove the mapping.
> + *
> + * @param base     Base address of the range to map (inclusive).
> + */
> +void unmap_mm_range(paddr_t base);
> +
>  /*
>   * Checks whether a given memory range is present in the provided table of
>   * MPU protection regions.
> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
> index 33333181d5..52c4c43827 100644
> --- a/xen/arch/arm/mpu/mm.c
> +++ b/xen/arch/arm/mpu/mm.c
> @@ -332,31 +332,39 @@ static int xen_mpumap_update_entry(paddr_t base, paddr_t limit,
>      return 0;
>  }
>  
> -int xen_mpumap_update(paddr_t base, paddr_t limit, unsigned int flags,
> -                      bool transient)
> +static bool check_mpu_mapping(paddr_t base, paddr_t limit, unsigned int flags)
>  {
> -    int rc;
> -
>      if ( flags_has_rwx(flags) )
>      {
>          printk("Mappings should not be both Writeable and Executable\n");
> -        return -EINVAL;
> +        return false;
>      }
>  
>      if ( base >= limit )
>      {
>          printk("Base address %#"PRIpaddr" must be smaller than limit address %#"PRIpaddr"\n",
>                 base, limit);
> -        return -EINVAL;
> +        return false;
>      }
>  
>      if ( !IS_ALIGNED(base, PAGE_SIZE) || !IS_ALIGNED(limit, PAGE_SIZE) )
>      {
>          printk("base address %#"PRIpaddr", or limit address %#"PRIpaddr" is not page aligned\n",
>                 base, limit);
> -        return -EINVAL;
> +        return false;
>      }
>  
> +    return true;
> +}
> +
> +int xen_mpumap_update(paddr_t base, paddr_t limit, unsigned int flags,
> +                      bool transient)
> +{
> +    int rc;
> +
> +    if ( !check_mpu_mapping(base, limit, flags) )
> +        return -EINVAL;
> +
>      spin_lock(&xen_mpumap_lock);
>  
>      rc = xen_mpumap_update_entry(base, limit, flags, transient);
> @@ -465,10 +473,134 @@ void free_init_memory(void)
>      BUG_ON("unimplemented");
>  }
>  
> +static uint8_t is_mm_range_mapped(paddr_t start, paddr_t end)
> +{
> +    int rc;
> +    uint8_t idx;
> +
> +    ASSERT(spin_is_locked(&xen_mpumap_lock));
> +
> +    rc = mpumap_contains_region(xen_mpumap, max_mpu_regions, start, end, &idx);
> +    if ( rc < 0 )
> +         panic("Cannot handle overlapping MPU memory protection regions\n");
Why panic? This function is not used only at boot time and should propagate
error to the caller, it's also within a spin lock.

> +
> +    /*
> +     * 'idx' will be INVALID_REGION_IDX for rc == MPUMAP_REGION_NOTFOUND and
> +     * it will be a proper region index when rc >= MPUMAP_REGION_FOUND.
> +     */
> +    return idx;
> +}
> +
> +static bool is_mm_attr_match(pr_t *region, unsigned int attributes)
> +{
> +    bool ret = true;
> +
> +    if ( region->prbar.reg.ro != PAGE_RO_MASK(attributes) )
> +    {
> +        printk(XENLOG_WARNING
> +               "Mismatched Access Permission attributes (%#x0 instead of %#x0)\n",
> +               region->prbar.reg.ro, PAGE_RO_MASK(attributes));
> +        ret = false;
> +    }
> +
> +    if ( region->prbar.reg.xn != PAGE_XN_MASK(attributes) )
> +    {
> +        printk(XENLOG_WARNING
> +               "Mismatched Execute Never attributes (%#x instead of %#x)\n",
> +               region->prbar.reg.xn, PAGE_XN_MASK(attributes));
> +        ret = false;
> +    }
> +
> +    if ( region->prlar.reg.ai != PAGE_AI_MASK(attributes) )
> +    {
> +        printk(XENLOG_WARNING
> +               "Mismatched Memory Attribute Index (%#x instead of %#x)\n",
> +               region->prlar.reg.ai, PAGE_AI_MASK(attributes));
> +        ret = false;
> +    }
> +
> +    return ret;
> +}
> +
> +void *map_mm_range(paddr_t base, paddr_t limit, unsigned int flags)
> +{
> +    paddr_t start_pg = round_pgdown(base);
> +    paddr_t end_pg = round_pgup(limit);
> +    void *ret = NULL;
> +    uint8_t idx;
> +
> +    if ( !check_mpu_mapping(start_pg, end_pg, flags) )
> +        return NULL;
> +
> +    spin_lock(&xen_mpumap_lock);
> +
> +    idx = is_mm_range_mapped(start_pg, end_pg);
> +    if ( idx != INVALID_REGION_IDX )
> +    {
> +        /* Already mapped with different attributes */
> +        if ( !is_mm_attr_match(&xen_mpumap[idx], flags) )
> +        {
> +            printk(XENLOG_WARNING
> +                   "Range %#"PRIpaddr"-%#"PRIpaddr" already mapped with different flags\n",
> +                   start_pg, end_pg);
> +            goto out;
> +        }
> +
> +        /* Already mapped with same attributes */
> +        ret = maddr_to_virt(base);
> +        goto out;
> +    }
> +
> +    if ( !xen_mpumap_update_entry(start_pg, end_pg, flags, true) )
> +    {
> +        context_sync_mpu();
> +        ret = maddr_to_virt(base);
> +    }
> +
> + out:
> +    spin_unlock(&xen_mpumap_lock);
> +
> +    return ret;
> +}
> +
> +void unmap_mm_range(paddr_t base)
> +{
> +    uint8_t idx;
> +
> +    spin_lock(&xen_mpumap_lock);
> +
> +    /*
> +     * Mappings created via map_mm_range are at least PAGE_SIZE. Find the idx
> +     * of the MPU memory region containing `start` mapped through map_mm_range.
> +     */
> +    idx = is_mm_range_mapped(base, base + PAGE_SIZE);
> +    if ( idx == INVALID_REGION_IDX )
> +    {
> +        printk(XENLOG_ERR
> +               "Failed to unmap_mm_range MPU memory region at %#"PRIpaddr"\n",
> +               base);
> +        goto out;
> +    }
> +
> +    /* This API is only meant to unmap transient regions */
> +    if ( !region_is_transient(&xen_mpumap[idx]) )
So is this the only purpose of the transient flag? To check that unmap_mm_range
is used on the range that was mapped with map_mm_range? What would happen
without introducing this flag? You already check for the matching attributes.

~Michal

Re: [PATCH v2 5/5] xen/arm: map static memory on demand

[Orzel, Michal]

On 27/08/2025 18:35, Hari Limaye wrote:
> From: Penny Zheng <Penny.Zheng@arm.com>
> 
> In the function `init_staticmem_pages` we need to have mapped static
> memory banks for initialization. Unlike on an MMU system, we cannot map
> the entire RAM on an MPU system as we have a limited number of MPU
> memory regions. To solve this, transiently map the static memory banks
> for initialization.
> 
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> Signed-off-by: Wei Chen <wei.chen@arm.com>
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> Signed-off-by: Hari Limaye <hari.limaye@arm.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal

Re: [PATCH v2 4/5] arm/mpu: Implement ioremap_attr for MPU

[Hari Limaye]

Hi Michal,

>> 
>> +    rc = mpumap_contains_region(xen_mpumap, max_mpu_regions, start, end, &idx);
>> +    if ( rc < 0 )
>> +         panic("Cannot handle overlapping MPU memory protection regions\n");
> Why panic? This function is not used only at boot time and should propagate
> error to the caller, it's also within a spin lock.

Good point - I will update this to propagate the error in the next version of the series.

>> +    /* This API is only meant to unmap transient regions */
>> +    if ( !region_is_transient(&xen_mpumap[idx]) )
> So is this the only purpose of the transient flag? To check that unmap_mm_range
> is used on the range that was mapped with map_mm_range? What would happen
> without introducing this flag? You already check for the matching attributes.
> 
> ~Michal
> 

Yes this is the purpose of the transient flag - we want to ensure that a call to unmap_mm_range only destroys a mapping that was created by a matching call to map_mm_range. Due to the fact that map_mm_range may not create a mapping in the instance that one already exists - `/* Already mapped with same attributes */` - we need this check to ensure that unmap_mm_range will not destroy a pre-existing mapping.

Many thanks,
Hari

Re: [PATCH v2 3/5] arm/mpu: Implement transient mapping

[Hari Limaye]

Hi Michal,

>> +++ b/xen/arch/arm/include/asm/arm32/mpu.h
>> @@ -39,6 +39,8 @@ typedef union {
>> typedef struct {
>>     prbar_t prbar;
>>     prlar_t prlar;
>> +    bool transient;
> Do we expect to have any other flags? If so, it could make sense to use a
> bitfield right away.
Yes, at least one more flag at present - I will update this to use a bitfield in the next version of the series.


>> #ifdef CONFIG_ARM_64
>> -#define XEN_MPUMAP_ENTRY_SHIFT  0x4     /* 16 byte structure */
>> +#define XEN_MPUMAP_ENTRY_SHIFT        0x5   /* 32 byte structure */
>> +#define XEN_MPUMAP_ENTRY_ZERO_OFFSET  0x10  /* {PRBAR, PRLAR} is 16 bytes  */
> To avoid having to update this over and over again, maybe define a macro using
> OFFSET?
> 
> Other than that:
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
> 
> ~Michal

Good point - I will do this in the next version of the series.

Many thanks,
Hari

Re: [PATCH v2 4/5] arm/mpu: Implement ioremap_attr for MPU

[Orzel, Michal]

On 28/08/2025 11:37, Hari Limaye wrote:
> Hi Michal,
> 
>>>
>>> +    rc = mpumap_contains_region(xen_mpumap, max_mpu_regions, start, end, &idx);
>>> +    if ( rc < 0 )
>>> +         panic("Cannot handle overlapping MPU memory protection regions\n");
>> Why panic? This function is not used only at boot time and should propagate
>> error to the caller, it's also within a spin lock.
> 
> Good point - I will update this to propagate the error in the next version of the series.
> 
>>> +    /* This API is only meant to unmap transient regions */
>>> +    if ( !region_is_transient(&xen_mpumap[idx]) )
>> So is this the only purpose of the transient flag? To check that unmap_mm_range
>> is used on the range that was mapped with map_mm_range? What would happen
>> without introducing this flag? You already check for the matching attributes.
>>
>> ~Michal
>>
> 
> Yes this is the purpose of the transient flag - we want to ensure that a call to unmap_mm_range only destroys a mapping that was created by a matching call to map_mm_range. Due to the fact that map_mm_range may not create a mapping in the instance that one already exists - `/* Already mapped with same attributes */` - we need this check to ensure that unmap_mm_range will not destroy a pre-existing mapping.
Ok, understood.

~Michal