Re: [PATCH v3 1/6] arm/mpu: Find MPU region by range

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 15/07/2025 09:45, Hari Limaye wrote:
> From: Luca Fancellu <luca.fancellu@xxxxxxx>
> 
> Implement a function to find the index of a MPU region in the xen_mpumap
> MPU region array. This function will be used in future commits to
> implement creating and destroying MPU regions.
> 
> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> Signed-off-by: Hari Limaye <hari.limaye@xxxxxxx>
> ---
> Changes from v1:
> - Update commit message
> - Remove internal _index variable
> - Simplify logic by disallowing NULL index parameter
> - Use normal printk
> - Reorder conditional checks
> - Update some comments
> 
> Changes from v2:
> - Replace conditional with assert
> - Improve comments regarding base and limit
> - Space between ( and uint8_t.
> ---
>  xen/arch/arm/include/asm/mpu/mm.h | 29 ++++++++++++++++
>  xen/arch/arm/mpu/mm.c             | 55 +++++++++++++++++++++++++++++++
>  2 files changed, 84 insertions(+)
> 
> diff --git a/xen/arch/arm/include/asm/mpu/mm.h 
> b/xen/arch/arm/include/asm/mpu/mm.h
> index a7f970b465..5a2b9b498b 100644
> --- a/xen/arch/arm/include/asm/mpu/mm.h
> +++ b/xen/arch/arm/include/asm/mpu/mm.h
> @@ -10,6 +10,13 @@
>  #include <asm/mm.h>
>  #include <asm/mpu.h>
>  
> +#define MPUMAP_REGION_OVERLAP      -1
> +#define MPUMAP_REGION_NOTFOUND      0
> +#define MPUMAP_REGION_FOUND         1
> +#define MPUMAP_REGION_INCLUSIVE     2
> +
> +#define INVALID_REGION_IDX     0xFFU
> +
>  extern struct page_info *frame_table;
>  
>  extern uint8_t max_mpu_regions;
> @@ -75,6 +82,28 @@ void write_protection_region(const pr_t *pr_write, uint8_t 
> sel);
>   */
>  pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags);
>  
> +/*
> + * Checks whether a given memory range is present in the provided table of
> + * MPU protection regions.
> + *
> + * @param table         Array of pr_t protection regions.
> + * @param r_regions     Number of elements in `table`.
> + * @param base          Start of the memory region to be checked (inclusive).
> + * @param limit         End of the memory region to be checked (exclusive).
> + * @param index         Set to the index of the region if an exact or 
> inclusive
> + *                      match is found, and INVALID_REGION otherwise.
> + * @return: Return code indicating the result of the search:
> + *          MPUMAP_REGION_NOTFOUND: no part of the range is present in 
> `table`
> + *          MPUMAP_REGION_FOUND: found an exact match in `table`
> + *          MPUMAP_REGION_INCLUSIVE: found an inclusive match in `table`
> + *          MPUMAP_REGION_OVERLAP: found an overlap with a mapping in `table`
> + *
> + * Note: make sure that the range [`base`, `limit`) refers to the memory 
> region
> + * inclusive of `base` and exclusive of `limit`.
> + */
> +int mpumap_contains_region(pr_t *table, uint8_t nr_regions, paddr_t base,
> +                           paddr_t limit, uint8_t *index);
> +
>  #endif /* __ARM_MPU_MM_H__ */
>  
>  /*
> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
> index ccfb37a67b..407264a88c 100644
> --- a/xen/arch/arm/mpu/mm.c
> +++ b/xen/arch/arm/mpu/mm.c
> @@ -110,6 +110,61 @@ pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned 
> int flags)
>      return region;
>  }
>  
> +int mpumap_contains_region(pr_t *table, uint8_t nr_regions, paddr_t base,
> +                           paddr_t limit, uint8_t *index)
> +{
> +    ASSERT(index);
> +    *index = INVALID_REGION_IDX;
> +
> +    /*
> +     * The caller supplies a half-open interval [base, limit), i.e. limit is 
> the
> +     * first byte *after* the region. Require limit strictly greater than 
> base,
> +     * which is necessarily a non-empty region.
> +     */
> +    ASSERT(base < limit);
Well, that does not guarantee a non-empty region.
Consider passing [x, x+1). The assert will pass, even though the region is 
empty.

~Michal