Re: [PATCH 1/5] arm/mpu: Implement setup_frametable_mappings for MPU systems

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

On 30/07/2025 10:45, Hari Limaye wrote:
> From: Luca Fancellu <luca.fancellu@xxxxxxx>
> 
> Implement the MPU variant of `setup_frametable_mappings`. This function
> will be called by `setup_mm` when an implementation for MPU systems is
> added in a follow up commit.
> 
> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> Signed-off-by: Hari Limaye <hari.limaye@xxxxxxx>
> ---
>  xen/arch/arm/mpu/mm.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
> index c6891607ec..6a16fa348d 100644
> --- a/xen/arch/arm/mpu/mm.c
> +++ b/xen/arch/arm/mpu/mm.c
> @@ -168,6 +168,25 @@ int mpumap_contains_region(pr_t *table, uint8_t 
> nr_regions, paddr_t base,
>      return MPUMAP_REGION_NOTFOUND;
>  }
>  
> +/* Map a frame table to cover physical addresses ps through pe */
> +void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
> +{
> +    mfn_t base_mfn;
> +    unsigned long nr_pdxs = mfn_to_pdx(mfn_add(maddr_to_mfn(pe), -1)) -
> +                            mfn_to_pdx(maddr_to_mfn(ps)) + 1;
Don't you need to make sure that ps is rounded up to page size and e rounded 
down?

> +    unsigned long frametable_size = nr_pdxs * sizeof(struct page_info);
Why don't you need sanity checking with BUILD_BUG_ON to check frametable size?

> +
> +    frametable_base_pdx = paddr_to_pdx(ps);
> +    frametable_size = ROUNDUP(frametable_size, PAGE_SIZE);
> +
> +    base_mfn = alloc_boot_pages(frametable_size >> PAGE_SHIFT, 1);
> +    frame_table = (struct page_info *)mfn_to_virt(mfn_x(base_mfn));
> +
> +    memset(&frame_table[0], 0, nr_pdxs * sizeof(struct page_info));
> +    memset(&frame_table[nr_pdxs], -1,
> +           frametable_size - (nr_pdxs * sizeof(struct page_info)));
> +}
> +
>  /*
>   * Allocate an entry for a new EL2 MPU region in the bitmap xen_mpumap_mask.
>   * @param idx   Set to the index of the allocated EL2 MPU region on success.
Other than that:
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

~Michal