Re: [PATCH v3 5/5] xen/arm: map static memory on demand

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

On 28/08/2025 13:12, Hari Limaye wrote:
> From: Penny Zheng <Penny.Zheng@xxxxxxx>
> 
> In the function `init_staticmem_pages` we need to have mapped static
> memory banks for initialization. Unlike on an MMU system, we cannot map
> the entire RAM on an MPU system as we have a limited number of MPU
> memory regions. To solve this, transiently map the static memory banks
> for initialization.
> 
> Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> Signed-off-by: Hari Limaye <hari.limaye@xxxxxxx>
> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
Given that the freeze date will likely be extended, I'd prefer to take this tag
back and request this patch to be changed due to reasons listed below.

> ---
> Changes from v2:
> - Add Michal's R-b
> ---
>  xen/arch/arm/include/asm/mmu/mm.h |  3 +++
>  xen/arch/arm/include/asm/mpu/mm.h |  4 ++++
>  xen/arch/arm/mpu/setup.c          | 11 +++++++++++
>  xen/include/xen/static-memory.h   |  8 ++++++++
>  4 files changed, 26 insertions(+)
> 
> diff --git a/xen/arch/arm/include/asm/mmu/mm.h 
> b/xen/arch/arm/include/asm/mmu/mm.h
> index 7f4d59137d..645a0ea3cb 100644
> --- a/xen/arch/arm/include/asm/mmu/mm.h
> +++ b/xen/arch/arm/include/asm/mmu/mm.h
> @@ -110,6 +110,9 @@ void dump_pt_walk(paddr_t ttbr, paddr_t addr,
>  extern void switch_ttbr(uint64_t ttbr);
>  extern void relocate_and_switch_ttbr(uint64_t ttbr);
>  
> +static inline void map_staticmem_pages_to_xen(paddr_t start, paddr_t end) {}
> +static inline void unmap_staticmem_pages_to_xen(paddr_t start, paddr_t end) 
> {}
> +
>  #endif /* __ARM_MMU_MM_H__ */
>  
>  /*
> diff --git a/xen/arch/arm/include/asm/mpu/mm.h 
> b/xen/arch/arm/include/asm/mpu/mm.h
> index efb0680e39..4cc769418e 100644
> --- a/xen/arch/arm/include/asm/mpu/mm.h
> +++ b/xen/arch/arm/include/asm/mpu/mm.h
> @@ -123,6 +123,10 @@ void *map_mm_range(paddr_t base, paddr_t limit, unsigned 
> int flags);
>   */
>  void unmap_mm_range(paddr_t base);
>  
> +/* {un}map_staticmem_pages_to_xen used while initializing static memory 
> banks */
> +void map_staticmem_pages_to_xen(paddr_t start, paddr_t end);
> +void unmap_staticmem_pages_to_xen(paddr_t start, paddr_t end);
> +
>  /*
>   * Checks whether a given memory range is present in the provided table of
>   * MPU protection regions.
> diff --git a/xen/arch/arm/mpu/setup.c b/xen/arch/arm/mpu/setup.c
> index 163573b932..dbc3107333 100644
> --- a/xen/arch/arm/mpu/setup.c
> +++ b/xen/arch/arm/mpu/setup.c
> @@ -83,6 +83,17 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
>      return fdt_virt;
>  }
>  
> +void __init map_staticmem_pages_to_xen(paddr_t start, paddr_t end)
> +{
> +    if ( !map_mm_range(start, end, PAGE_HYPERVISOR) )
> +        panic("Unable to map staticmem pages to Xen!");
> +}
> +
> +void __init unmap_staticmem_pages_to_xen(paddr_t start, paddr_t end)
> +{
> +    unmap_mm_range(start);
> +}
> +
>  /*
>   * copy_from_paddr - copy data from a physical address
>   * @dst: destination virtual address
> diff --git a/xen/include/xen/static-memory.h b/xen/include/xen/static-memory.h
> index e445aa8057..d99abac113 100644
> --- a/xen/include/xen/static-memory.h
> +++ b/xen/include/xen/static-memory.h
> @@ -18,7 +18,15 @@ static inline void init_staticmem_bank(const struct 
> membank *bank)
>      if ( mfn_x(bank_end) <= mfn_x(bank_start) )
>          return;
>  
> +    /* Map temporarily before initialization */
> +    map_staticmem_pages_to_xen(mfn_to_maddr(bank_start),
> +                               mfn_to_maddr(bank_end));
Static memory is not Arm only feature, it is common and as such should not (and
does not) make calls to Arm only functions. If at all, such helpers should be
made generic so other arches that could enable static memory can re-define them
if needed (as you pointed out, on MMU you don't need to map/unmap this region
temporarily).

~Michal