On 05/01/2026 12:34, Harry Ramsey wrote:
> From: Luca Fancellu <[email protected]>
>
> HAS_VMAP is not enabled on MPU systems, but the vmap functions are used
> in places across common code. In order to keep the existing code and
> maintain correct functionality, implement the `vmap_contig` and `vunmap`
> functions for MPU systems.
>
> Introduce a helper function `destroy_xen_mapping_containing` to aid with
> unmapping an entire region when only the start address is known.
>
> Signed-off-by: Luca Fancellu <[email protected]>
> Signed-off-by: Harry Ramsey <[email protected]>
> ---
> v2:
> - Rename `destroy_entire_xen_mapping` to `destroy_xen_mapping_containing`
> - Improve code documentation.
> - Refactor nested code.
> - Fix ignored rc error code in `destroy_xen_mapping_containing`.
> ---
> xen/arch/arm/include/asm/mpu/mm.h | 10 +++++
> xen/arch/arm/mpu/mm.c | 74 ++++++++++++++++++++++++++-----
> xen/arch/arm/mpu/vmap.c | 14 ++++--
> 3 files changed, 83 insertions(+), 15 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/mpu/mm.h
> b/xen/arch/arm/include/asm/mpu/mm.h
> index e1ded6521d..1b5ffa5b64 100644
> --- a/xen/arch/arm/include/asm/mpu/mm.h
> +++ b/xen/arch/arm/include/asm/mpu/mm.h
> @@ -111,6 +111,16 @@ pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned
> int flags);
> int mpumap_contains_region(pr_t *table, uint8_t nr_regions, paddr_t base,
> paddr_t limit, uint8_t *index);
>
> +
> +/*
> + * Destroys and frees (if reference count is 0) an entire xen mapping on MPU
> + * systems where only the start address is known.
> + *
> + * @param s Start address of memory region to be destroyed.
> + * @return: 0 on success, negative on error.
> + */
> +int destroy_xen_mapping_containing(paddr_t s);
> +
> #endif /* __ARM_MPU_MM_H__ */
>
> /*
> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
> index 687dec3bc6..207e8d2d91 100644
> --- a/xen/arch/arm/mpu/mm.c
> +++ b/xen/arch/arm/mpu/mm.c
> @@ -290,6 +290,42 @@ static void disable_mpu_region_from_index(uint8_t index)
> write_protection_region(&xen_mpumap[index], index);
> }
>
> +/*
> + * Free a xen_mpumap entry given the index. A mpu region is actually disabled
> + * when the refcount is 0 and the region type is MPUMAP_REGION_FOUND.
> + *
> + * @param idx Index of the mpumap entry.
> + * @param region_found_type MPUMAP_REGION_* value.
> + * @return 0 on success, otherwise negative on error.
> + */
> +static int xen_mpumap_free_entry(uint8_t idx, int region_found_type)
> +{
> + ASSERT(spin_is_locked(&xen_mpumap_lock));
> + ASSERT(idx != INVALID_REGION_IDX);
> +
> + if ( MPUMAP_REGION_OVERLAP == region_found_type )
> + {
> + printk(XENLOG_ERR "Cannot remove an overlapping region\n");
> + return -EINVAL;
> + }
> +
> + if ( xen_mpumap[idx].refcount )
> + {
> + xen_mpumap[idx].refcount -= 1;
> + return 0;
> + }
> +
> + if ( MPUMAP_REGION_FOUND == region_found_type )
> + disable_mpu_region_from_index(idx);
> + else
> + {
> + printk(XENLOG_ERR "Cannot remove a partial region\n");
Shouldn't this be moved above refcount checking? Do we expect this function to
be called with region_found_type being MPUMAP_REGION_INCLUSIVE?
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> /*
> * Update the entry in the MPU memory region mapping table (xen_mpumap) for
> the
> * given memory range and flags, creating one if none exists.
> @@ -357,18 +393,7 @@ static int xen_mpumap_update_entry(paddr_t base, paddr_t
> limit,
> return -EINVAL;
> }
>
> - if ( xen_mpumap[idx].refcount == 0 )
> - {
> - if ( MPUMAP_REGION_FOUND == rc )
> - disable_mpu_region_from_index(idx);
> - else
> - {
> - printk("Cannot remove a partial region\n");
> - return -EINVAL;
> - }
> - }
> - else
> - xen_mpumap[idx].refcount -= 1;
> + return xen_mpumap_free_entry(idx, rc);
> }
>
> return 0;
> @@ -418,6 +443,31 @@ int destroy_xen_mappings(unsigned long s, unsigned long
> e)
> return xen_mpumap_update(s, e, 0);
> }
>
> +int destroy_xen_mapping_containing(paddr_t s)
> +{
> + int rc;
> + uint8_t idx;
> +
> + ASSERT(IS_ALIGNED(s, PAGE_SIZE));
> +
> + spin_lock(&xen_mpumap_lock);
> +
> + rc = mpumap_contains_region(xen_mpumap, max_mpu_regions, s, s +
> PAGE_SIZE,
> + &idx);
> + if ( rc == MPUMAP_REGION_NOTFOUND )
> + {
> + printk(XENLOG_ERR "Cannot remove entry that does not exist");
Why do we split sanity checking between this and xen_mpumap_free_entry?
What are the possible region types that xen_mpumap_free_entry is expected to
work with? I thought that it should only be MPUMAP_REGION_FOUND.
~Michal
