Re: [PATCH v3 5/5] xen/arm32: mpu: Stubs to build MPU for arm32

[Julien Grall <julien@xxxxxxx>]

Hi Ayan,

On 30/03/2025 19:03, Ayan Kumar Halder wrote:
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> ---
> Changes from :-
>
> v1, v2 -
> 1. New patch introduced in v3.
> 2. Should be applied on top of
> https://patchwork.kernel.org/project/xen-devel/cover/20250316192445.2376484-1-luca.fancellu@xxxxxxx/
>
>   xen/arch/arm/Kconfig             |  2 +-
>   xen/arch/arm/arm32/mpu/Makefile  |  2 ++
>   xen/arch/arm/arm32/mpu/p2m.c     | 18 ++++++++++++++++++
>   xen/arch/arm/arm32/mpu/smpboot.c | 23 +++++++++++++++++++++++
>   xen/arch/arm/include/asm/mm.h    |  5 +++++
>   5 files changed, 49 insertions(+), 1 deletion(-)
>   create mode 100644 xen/arch/arm/arm32/mpu/p2m.c
>   create mode 100644 xen/arch/arm/arm32/mpu/smpboot.c
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 565f288331..a1dd942091 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -1,7 +1,7 @@
>   config ARM_32
>         def_bool y
>         depends on "$(ARCH)" = "arm32"
> -       select ARCH_MAP_DOMAIN_PAGE
> +       select ARCH_MAP_DOMAIN_PAGE if MMU
If ARCH_MAP_DOMAIN_PAGE is not present, then the assumption is Xen will 
be able to access all the memory. Is this what we want long term?
If not, then I would consider providing stubs rather than disable 
ARCH_MAP_DOMAIN_PAGE.
>   
>   config ARM_64
>         def_bool y
> diff --git a/xen/arch/arm/arm32/mpu/Makefile b/xen/arch/arm/arm32/mpu/Makefile
> index 3340058c08..38797f28af 100644
> --- a/xen/arch/arm/arm32/mpu/Makefile
> +++ b/xen/arch/arm/arm32/mpu/Makefile
> @@ -1 +1,3 @@
>   obj-y += head.o
> +obj-y += smpboot.o
> +obj-y += p2m.o
> diff --git a/xen/arch/arm/arm32/mpu/p2m.c b/xen/arch/arm/arm32/mpu/p2m.c
> new file mode 100644
> index 0000000000..df8de5c7d8
> --- /dev/null
> +++ b/xen/arch/arm/arm32/mpu/p2m.c
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/init.h>
> +#include <asm/p2m.h>
> +
> +void __init setup_virt_paging(void)
> +{
> +    BUG_ON("unimplemented");
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/arm32/mpu/smpboot.c b/xen/arch/arm/arm32/mpu/smpboot.c
> new file mode 100644
> index 0000000000..3f3e54294e
> --- /dev/null
> +++ b/xen/arch/arm/arm32/mpu/smpboot.c
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/mm.h>
> +
> +int prepare_secondary_mm(int cpu)
> +{
> +    BUG_ON("unimplemented");
> +    return -EINVAL;
> +}
> +
> +void update_boot_mapping(bool enable)
> +{
> +    BUG_ON("unimplemented");
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
> index fbffaccef4..a894e28ac9 100644
> --- a/xen/arch/arm/include/asm/mm.h
> +++ b/xen/arch/arm/include/asm/mm.h
> @@ -171,12 +171,17 @@ struct page_info
>   #define PGC_need_scrub    PGC_allocated
>   
>   #ifdef CONFIG_ARM_32
> +#ifdef CONFIG_MPU
> +#define is_xen_heap_page(page) false
> +#define is_xen_heap_mfn(mfn) false
Can you clarify whether this is the intended implementation? If not then 
we can be use BUG_ON("unimplemented")?
Also...

> +#else
>   #define is_xen_heap_page(page) is_xen_heap_mfn(page_to_mfn(page))
>   #define is_xen_heap_mfn(mfn) ({                                 \
>       unsigned long mfn_ = mfn_x(mfn);                            \
>       (mfn_ >= mfn_x(directmap_mfn_start) &&                      \
>        mfn_ < mfn_x(directmap_mfn_end));                          \
>   })
> +#endif
... is the implementation will be similar to is_xen_heap_page() for MMU?

>   #else
>   #define is_xen_heap_page(page) ((page)->count_info & PGC_xen_heap)
>   #define is_xen_heap_mfn(mfn) \
Cheers,

--
Julien Grall