1. Patchew
  2. Xen
  3. [PATCH v4 0/6] Second series for R82 MPU Support
  4. View patch

[PATCH v4 4/6] arm/mpu: Destroy an existing entry in Xen MPU memory mapping table

Hari Limaye posted 6 patches 3 months, 1 week ago
[PATCH v4 4/6] arm/mpu: Destroy an existing entry in Xen MPU memory mapping table
Posted by Hari Limaye 3 months, 1 week ago 
From: Penny Zheng <Penny.Zheng@arm.com>

This commit expands xen_mpumap_update/xen_mpumap_update_entry to include
destroying an existing entry.

We define a new helper "disable_mpu_region_from_index" to disable the MPU
region based on index. If region is within [0, 31], we could quickly
disable the MPU region through PRENR_EL2 which provides direct access to the
PRLAR_EL2.EN bits of EL2 MPU regions.

Rignt now, we only support destroying a *WHOLE* MPU memory region,
part-region removing is not supported, as in worst case, it will
leave two fragments behind.

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>
---
Changes from v1:
- Move check for part-region removal outside if condition
- Use normal printk

Changes from v2:
- Fix assert from `ASSERT(s <= e)` -> `ASSERT(s < e)`
- Remove call to context_sync_mpu
- Use register_t
- Improve sanity checking to catch modification & removing non-existent
  entries
- Update check for MPUMAP_REGION_INCLUSIVE to be generic

Changes from v3:
- Move early return for modifying case above MPUMAP_REGION_INCLUSIVE
  check
- Add bool variable to store `flags & _PAGE_PRESENT`
- Remove calls to `virt_to_maddr`
---
 xen/arch/arm/include/asm/mpu.h        |  2 +
 xen/arch/arm/include/asm/mpu/cpregs.h |  4 ++
 xen/arch/arm/mpu/mm.c                 | 95 ++++++++++++++++++++++++---
 3 files changed, 92 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/include/asm/mpu.h b/xen/arch/arm/include/asm/mpu.h
index 63560c613b..5053edaf63 100644
--- a/xen/arch/arm/include/asm/mpu.h
+++ b/xen/arch/arm/include/asm/mpu.h
@@ -23,6 +23,8 @@
 #define NUM_MPU_REGIONS_MASK    (NUM_MPU_REGIONS - 1)
 #define MAX_MPU_REGION_NR       NUM_MPU_REGIONS_MASK
 
+#define PRENR_MASK  GENMASK(31, 0)
+
 #ifndef __ASSEMBLY__
 
 /*
diff --git a/xen/arch/arm/include/asm/mpu/cpregs.h b/xen/arch/arm/include/asm/mpu/cpregs.h
index bb15e02df6..9f3b32acd7 100644
--- a/xen/arch/arm/include/asm/mpu/cpregs.h
+++ b/xen/arch/arm/include/asm/mpu/cpregs.h
@@ -6,6 +6,9 @@
 /* CP15 CR0: MPU Type Register */
 #define HMPUIR          p15,4,c0,c0,4
 
+/* CP15 CR6: Protection Region Enable Register */
+#define HPRENR          p15,4,c6,c1,1
+
 /* CP15 CR6: MPU Protection Region Base/Limit/Select Address Register */
 #define HPRSELR         p15,4,c6,c2,1
 #define HPRBAR          p15,4,c6,c3,0
@@ -82,6 +85,7 @@
 /* Alphabetically... */
 #define MPUIR_EL2       HMPUIR
 #define PRBAR_EL2       HPRBAR
+#define PRENR_EL2       HPRENR
 #define PRLAR_EL2       HPRLAR
 #define PRSELR_EL2      HPRSELR
 #endif /* CONFIG_ARM_32 */
diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
index d5426525af..2154b3720d 100644
--- a/xen/arch/arm/mpu/mm.c
+++ b/xen/arch/arm/mpu/mm.c
@@ -189,6 +189,42 @@ static int xen_mpumap_alloc_entry(uint8_t *idx)
     return 0;
 }
 
+/*
+ * Disable and remove an MPU region from the data structure and MPU registers.
+ *
+ * @param index Index of the MPU region to be disabled.
+ */
+static void disable_mpu_region_from_index(uint8_t index)
+{
+    ASSERT(spin_is_locked(&xen_mpumap_lock));
+    ASSERT(index != INVALID_REGION_IDX);
+
+    if ( !region_is_valid(&xen_mpumap[index]) )
+    {
+        printk(XENLOG_WARNING
+               "mpu: MPU memory region[%u] is already disabled\n", index);
+        return;
+    }
+
+    /* Zeroing the region will also zero the region enable */
+    memset(&xen_mpumap[index], 0, sizeof(pr_t));
+    clear_bit(index, xen_mpumap_mask);
+
+    /*
+     * Both Armv8-R AArch64 and AArch32 have direct access to the enable bit for
+     * MPU regions numbered from 0 to 31.
+     */
+    if ( (index & PRENR_MASK) != 0 )
+    {
+        /* Clear respective bit */
+        register_t val = READ_SYSREG(PRENR_EL2) & (~(1UL << index));
+
+        WRITE_SYSREG(val, PRENR_EL2);
+    }
+    else
+        write_protection_region(&xen_mpumap[index], 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.
@@ -201,27 +237,59 @@ static int xen_mpumap_alloc_entry(uint8_t *idx)
 static int xen_mpumap_update_entry(paddr_t base, paddr_t limit,
                                    unsigned int flags)
 {
+    bool flags_has_page_present;
     uint8_t idx;
     int rc;
 
     ASSERT(spin_is_locked(&xen_mpumap_lock));
 
-    /* Currently only region creation is supported. */
-    if ( !(flags & _PAGE_PRESENT) )
+    rc = mpumap_contains_region(xen_mpumap, max_mpu_regions, base, limit, &idx);
+    if ( rc < 0 )
         return -EINVAL;
 
-    rc = mpumap_contains_region(xen_mpumap, max_mpu_regions, base, limit, &idx);
-    if ( rc != MPUMAP_REGION_NOTFOUND )
+    flags_has_page_present = flags & _PAGE_PRESENT;
+
+    /* Currently we don't support modifying an existing entry. */
+    if ( flags_has_page_present && (rc >= MPUMAP_REGION_FOUND) )
+    {
+        printk("mpu: modifying an existing entry is not supported\n");
         return -EINVAL;
+    }
+
+    /*
+     * Currently, we only support removing/modifying a *WHOLE* MPU memory
+     * region. Part-region removal/modification is not supported as in the worst
+     * case it will leave two/three fragments behind.
+     */
+    if ( rc == MPUMAP_REGION_INCLUSIVE )
+    {
+        printk("mpu: part-region removal/modification is not supported\n");
+        return -EINVAL;
+    }
 
     /* We are inserting a mapping => Create new region. */
-    rc = xen_mpumap_alloc_entry(&idx);
-    if ( rc )
-        return -ENOENT;
+    if ( flags_has_page_present && (MPUMAP_REGION_NOTFOUND == rc) )
+    {
+        rc = xen_mpumap_alloc_entry(&idx);
+        if ( rc )
+            return -ENOENT;
+
+        xen_mpumap[idx] = pr_of_addr(base, limit, flags);
 
-    xen_mpumap[idx] = pr_of_addr(base, limit, flags);
+        write_protection_region(&xen_mpumap[idx], idx);
+    }
+
+    /* Removing a mapping */
+    if ( !flags_has_page_present )
+    {
+        if ( rc == MPUMAP_REGION_NOTFOUND )
+        {
+            printk("mpu: cannot remove an entry that does not exist\n");
+            return -EINVAL;
+        }
 
-    write_protection_region(&xen_mpumap[idx], idx);
+        disable_mpu_region_from_index(idx);
+    }
 
     return 0;
 }
@@ -261,6 +329,15 @@ int xen_mpumap_update(paddr_t base, paddr_t limit, unsigned int flags)
     return rc;
 }
 
+int destroy_xen_mappings(unsigned long s, unsigned long e)
+{
+    ASSERT(IS_ALIGNED(s, PAGE_SIZE));
+    ASSERT(IS_ALIGNED(e, PAGE_SIZE));
+    ASSERT(s < e);
+
+    return xen_mpumap_update(s, e, 0);
+}
+
 int map_pages_to_xen(unsigned long virt, mfn_t mfn, unsigned long nr_mfns,
                      unsigned int flags)
 {
-- 
2.34.1
Re: [PATCH v4 4/6] arm/mpu: Destroy an existing entry in Xen MPU memory mapping table
Posted by Orzel, Michal 3 months, 1 week ago 

On 21/07/2025 10:31, Hari Limaye wrote:
> From: Penny Zheng <Penny.Zheng@arm.com>
> 
> This commit expands xen_mpumap_update/xen_mpumap_update_entry to include
> destroying an existing entry.
> 
> We define a new helper "disable_mpu_region_from_index" to disable the MPU
> region based on index. If region is within [0, 31], we could quickly
> disable the MPU region through PRENR_EL2 which provides direct access to the
> PRLAR_EL2.EN bits of EL2 MPU regions.
> 
> Rignt now, we only support destroying a *WHOLE* MPU memory region,
> part-region removing is not supported, as in worst case, it will
> leave two fragments behind.
> 
> 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>
> ---
> Changes from v1:
> - Move check for part-region removal outside if condition
> - Use normal printk
> 
> Changes from v2:
> - Fix assert from `ASSERT(s <= e)` -> `ASSERT(s < e)`
> - Remove call to context_sync_mpu
> - Use register_t
> - Improve sanity checking to catch modification & removing non-existent
>   entries
> - Update check for MPUMAP_REGION_INCLUSIVE to be generic
> 
> Changes from v3:
> - Move early return for modifying case above MPUMAP_REGION_INCLUSIVE
>   check
> - Add bool variable to store `flags & _PAGE_PRESENT`
> - Remove calls to `virt_to_maddr`
> ---
>  xen/arch/arm/include/asm/mpu.h        |  2 +
>  xen/arch/arm/include/asm/mpu/cpregs.h |  4 ++
>  xen/arch/arm/mpu/mm.c                 | 95 ++++++++++++++++++++++++---
>  3 files changed, 92 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/mpu.h b/xen/arch/arm/include/asm/mpu.h
> index 63560c613b..5053edaf63 100644
> --- a/xen/arch/arm/include/asm/mpu.h
> +++ b/xen/arch/arm/include/asm/mpu.h
> @@ -23,6 +23,8 @@
>  #define NUM_MPU_REGIONS_MASK    (NUM_MPU_REGIONS - 1)
>  #define MAX_MPU_REGION_NR       NUM_MPU_REGIONS_MASK
>  
> +#define PRENR_MASK  GENMASK(31, 0)
> +
>  #ifndef __ASSEMBLY__
>  
>  /*
> diff --git a/xen/arch/arm/include/asm/mpu/cpregs.h b/xen/arch/arm/include/asm/mpu/cpregs.h
> index bb15e02df6..9f3b32acd7 100644
> --- a/xen/arch/arm/include/asm/mpu/cpregs.h
> +++ b/xen/arch/arm/include/asm/mpu/cpregs.h
> @@ -6,6 +6,9 @@
>  /* CP15 CR0: MPU Type Register */
>  #define HMPUIR          p15,4,c0,c0,4
>  
> +/* CP15 CR6: Protection Region Enable Register */
> +#define HPRENR          p15,4,c6,c1,1
> +
>  /* CP15 CR6: MPU Protection Region Base/Limit/Select Address Register */
>  #define HPRSELR         p15,4,c6,c2,1
>  #define HPRBAR          p15,4,c6,c3,0
> @@ -82,6 +85,7 @@
>  /* Alphabetically... */
>  #define MPUIR_EL2       HMPUIR
>  #define PRBAR_EL2       HPRBAR
> +#define PRENR_EL2       HPRENR
>  #define PRLAR_EL2       HPRLAR
>  #define PRSELR_EL2      HPRSELR
>  #endif /* CONFIG_ARM_32 */
> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
> index d5426525af..2154b3720d 100644
> --- a/xen/arch/arm/mpu/mm.c
> +++ b/xen/arch/arm/mpu/mm.c
> @@ -189,6 +189,42 @@ static int xen_mpumap_alloc_entry(uint8_t *idx)
>      return 0;
>  }
>  
> +/*
> + * Disable and remove an MPU region from the data structure and MPU registers.
> + *
> + * @param index Index of the MPU region to be disabled.
> + */
> +static void disable_mpu_region_from_index(uint8_t index)
> +{
> +    ASSERT(spin_is_locked(&xen_mpumap_lock));
> +    ASSERT(index != INVALID_REGION_IDX);
> +
> +    if ( !region_is_valid(&xen_mpumap[index]) )
> +    {
> +        printk(XENLOG_WARNING
> +               "mpu: MPU memory region[%u] is already disabled\n", index);
NIT: In this patch you start adding mpu: prefix, even though other messages do
not have it. I don't think it's needed.

Otherwise:
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal
Re: [PATCH v4 4/6] arm/mpu: Destroy an existing entry in Xen MPU memory mapping table
Posted by Hari Limaye 3 months, 1 week ago 
Hi Michal,

> NIT: In this patch you start adding mpu: prefix, even though other messages do
> not have it. I don't think it's needed.
> 
> Otherwise:
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
> 
> ~Michal
> 

Would you like me to respin for this change, or can it be addressed on commit?

Many thanks,
Hari
Re: [PATCH v4 4/6] arm/mpu: Destroy an existing entry in Xen MPU memory mapping table
Posted by Orzel, Michal 3 months, 1 week ago 

On 21/07/2025 11:31, Hari Limaye wrote:
> Hi Michal,
> 
>> NIT: In this patch you start adding mpu: prefix, even though other messages do
>> not have it. I don't think it's needed.
>>
>> Otherwise:
>> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
>>
>> ~Michal
>>
> 
> Would you like me to respin for this change, or can it be addressed on commit?
Provided no remarks from others in a few days, I can address it on commit.

~Michal

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.