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>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
---
Changes from v2:
- Define offsets programmatically, rather than hard-coding these
- Add Michal's R-b

Changes from v1:
- Improve commit message
- Mark parameter in read helper as const
---
 xen/arch/arm/arm32/asm-offsets.c         |  3 ++-
 xen/arch/arm/arm64/asm-offsets.c         |  2 ++
 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 | 17 +++++++++++++----
 xen/arch/arm/mpu/mm.c                    | 23 ++++++++++++++---------
 7 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/xen/arch/arm/arm32/asm-offsets.c b/xen/arch/arm/arm32/asm-offsets.c
index c203ce269d..f354bf374d 100644
--- a/xen/arch/arm/arm32/asm-offsets.c
+++ b/xen/arch/arm/arm32/asm-offsets.c
@@ -43,7 +43,6 @@ void __dummy__(void)
    OFFSET(UREGS_SP_und, struct cpu_user_regs, sp_und);
    OFFSET(UREGS_LR_und, struct cpu_user_regs, lr_und);
    OFFSET(UREGS_SPSR_und, struct cpu_user_regs, spsr_und);
-
    OFFSET(UREGS_SP_irq, struct cpu_user_regs, sp_irq);
    OFFSET(UREGS_LR_irq, struct cpu_user_regs, lr_irq);
    OFFSET(UREGS_SPSR_irq, struct cpu_user_regs, spsr_irq);
@@ -79,6 +78,8 @@ void __dummy__(void)
 #ifdef CONFIG_MPU
    DEFINE(XEN_MPUMAP_MASK_sizeof, sizeof(xen_mpumap_mask));
    DEFINE(XEN_MPUMAP_sizeof, sizeof(xen_mpumap));
+   DEFINE(XEN_MPUMAP_ENTRY_SHIFT, ilog2(sizeof(pr_t)));
+   DEFINE(XEN_MPUMAP_ENTRY_ZERO_OFFSET, sizeof(prbar_t) + sizeof(prlar_t));
    BLANK();
 #endif
 }
diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-offsets.c
index 320289b281..38a3894a3b 100644
--- a/xen/arch/arm/arm64/asm-offsets.c
+++ b/xen/arch/arm/arm64/asm-offsets.c
@@ -73,6 +73,8 @@ void __dummy__(void)
 #ifdef CONFIG_MPU
    DEFINE(XEN_MPUMAP_MASK_sizeof, sizeof(xen_mpumap_mask));
    DEFINE(XEN_MPUMAP_sizeof, sizeof(xen_mpumap));
+   DEFINE(XEN_MPUMAP_ENTRY_SHIFT, ilog2(sizeof(pr_t)));
+   DEFINE(XEN_MPUMAP_ENTRY_ZERO_OFFSET, sizeof(prbar_t) + sizeof(prlar_t));
    BLANK();
 #endif
 }
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..09e1dcf03f 100644
--- a/xen/arch/arm/include/asm/mpu/regions.inc
+++ b/xen/arch/arm/include/asm/mpu/regions.inc
@@ -14,19 +14,25 @@
 #define PRLAR_ELx_EN            0x1
 
 #ifdef CONFIG_ARM_64
-#define XEN_MPUMAP_ENTRY_SHIFT  0x4     /* 16 byte structure */
-
 .macro store_pair reg1, reg2, dst
     stp \reg1, \reg2, [\dst]
 .endm
 
-#else
-#define XEN_MPUMAP_ENTRY_SHIFT  0x3     /* 8 byte structure */
+.macro zero_pair dst, offset, tmp1, tmp2
+    stp xzr, xzr, [\dst, \offset]
+.endm
 
+#else
 .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 +103,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

Hi,

On 28/08/2025 12:12, 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>
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
> ---
> Changes from v2:
> - Define offsets programmatically, rather than hard-coding these
> - Add Michal's R-b
> 
> Changes from v1:
> - Improve commit message
> - Mark parameter in read helper as const
> ---
>   xen/arch/arm/arm32/asm-offsets.c         |  3 ++-
>   xen/arch/arm/arm64/asm-offsets.c         |  2 ++
>   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 | 17 +++++++++++++----
>   xen/arch/arm/mpu/mm.c                    | 23 ++++++++++++++---------
>   7 files changed, 48 insertions(+), 15 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/asm-offsets.c b/xen/arch/arm/arm32/asm-offsets.c
> index c203ce269d..f354bf374d 100644
> --- a/xen/arch/arm/arm32/asm-offsets.c
> +++ b/xen/arch/arm/arm32/asm-offsets.c
> @@ -43,7 +43,6 @@ void __dummy__(void)
>      OFFSET(UREGS_SP_und, struct cpu_user_regs, sp_und);
>      OFFSET(UREGS_LR_und, struct cpu_user_regs, lr_und);
>      OFFSET(UREGS_SPSR_und, struct cpu_user_regs, spsr_und);
> -

Spurious change?

>      OFFSET(UREGS_SP_irq, struct cpu_user_regs, sp_irq);
>      OFFSET(UREGS_LR_irq, struct cpu_user_regs, lr_irq);
>      OFFSET(UREGS_SPSR_irq, struct cpu_user_regs, spsr_irq);
> @@ -79,6 +78,8 @@ void __dummy__(void)
>   #ifdef CONFIG_MPU
>      DEFINE(XEN_MPUMAP_MASK_sizeof, sizeof(xen_mpumap_mask));
>      DEFINE(XEN_MPUMAP_sizeof, sizeof(xen_mpumap));
> +   DEFINE(XEN_MPUMAP_ENTRY_SHIFT, ilog2(sizeof(pr_t)));
> +   DEFINE(XEN_MPUMAP_ENTRY_ZERO_OFFSET, sizeof(prbar_t) + sizeof(prlar_t));
>      BLANK();
>   #endif
>   }
> diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-offsets.c
> index 320289b281..38a3894a3b 100644
> --- a/xen/arch/arm/arm64/asm-offsets.c
> +++ b/xen/arch/arm/arm64/asm-offsets.c
> @@ -73,6 +73,8 @@ void __dummy__(void)
>   #ifdef CONFIG_MPU
>      DEFINE(XEN_MPUMAP_MASK_sizeof, sizeof(xen_mpumap_mask));
>      DEFINE(XEN_MPUMAP_sizeof, sizeof(xen_mpumap));
> +   DEFINE(XEN_MPUMAP_ENTRY_SHIFT, ilog2(sizeof(pr_t)));
> +   DEFINE(XEN_MPUMAP_ENTRY_ZERO_OFFSET, sizeof(prbar_t) + sizeof(prlar_t));
>      BLANK();
>   #endif
>   }
> 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.

Why can't this be part of the flags?

>    * @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..09e1dcf03f 100644
> --- a/xen/arch/arm/include/asm/mpu/regions.inc
> +++ b/xen/arch/arm/include/asm/mpu/regions.inc
> @@ -14,19 +14,25 @@
>   #define PRLAR_ELx_EN            0x1
>   
>   #ifdef CONFIG_ARM_64
> -#define XEN_MPUMAP_ENTRY_SHIFT  0x4     /* 16 byte structure */
> -
>   .macro store_pair reg1, reg2, dst
>       stp \reg1, \reg2, [\dst]
>   .endm
>   
> -#else
> -#define XEN_MPUMAP_ENTRY_SHIFT  0x3     /* 8 byte structure */
> +.macro zero_pair dst, offset, tmp1, tmp2
 > +    stp xzr, xzr, [\dst, \offset]> +.endm
>   
> +#else
>   .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
 > +

TBH the addition of zero_pair feels a bit overkill when there is one 
user. Why can't we open-code the use case below and use ``store_pair``?

This would also avoid the "tmp1" and "tmp2" which are not used by arm64.

Cheers,

-- 
Julien Grall