On 11/04/2025 16:56, Luca Fancellu wrote:
> Introduce few utility function to manipulate and handle the
> pr_t type.
>
> Signed-off-by: Luca Fancellu <luca.fance...@arm.com>
> ---
> xen/arch/arm/include/asm/mpu.h | 40 ++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/xen/arch/arm/include/asm/mpu.h b/xen/arch/arm/include/asm/mpu.h
> index 59ff22c804c1..6971507457fb 100644
> --- a/xen/arch/arm/include/asm/mpu.h
> +++ b/xen/arch/arm/include/asm/mpu.h
> @@ -20,6 +20,46 @@
> #define NUM_MPU_REGIONS_MASK (NUM_MPU_REGIONS - 1)
> #define MAX_MPU_REGIONS NUM_MPU_REGIONS_MASK
>
> +#ifndef __ASSEMBLY__
> +
> +/* Set base address of MPU protection region(pr_t). */
What's the use of (pr_t) in this comment? pr_t is a data type. If at all, it
would want to be ...region @pr but I think you can skip it.
> +static inline void pr_set_base(pr_t *pr, paddr_t base)
> +{
> + pr->prbar.reg.base = (base >> MPU_REGION_SHIFT);
Looking at pr_t definition, base/limit is 46 bits wide. However the spec says
that last 4bits are reserved (i.e. you should not write to them) unless FEAT_LPA
is implemented. What's our plan here?
> +}
> +
> +/* Set limit address of MPU protection region(pr_t). */
> +static inline void pr_set_limit(pr_t *pr, paddr_t limit)
> +{
> + pr->prlar.reg.limit = ((limit - 1) >> MPU_REGION_SHIFT);
Why -1? AFAIR these registers take inclusive addresses, so is it because you
want caller to pass limit as exclusive and you convert it to inclusive? I think
it's quite error prone.
~Michal
