RE: [PATCH v2 16/40] xen/arm: introduce setup_mm_mappings

[Penny Zheng <Penny.Zheng@xxxxxxx>]

Hi Julien

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: Monday, February 6, 2023 5:32 AM
> To: Penny Zheng <Penny.Zheng@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Wei Chen <Wei.Chen@xxxxxxx>; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>; Bertrand Marquis <Bertrand.Marquis@xxxxxxx>;
> Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> Subject: Re: [PATCH v2 16/40] xen/arm: introduce setup_mm_mappings
> 
> Hi,
> 
> On 13/01/2023 05:28, Penny Zheng wrote:
> > Function setup_pagetables is responsible for boot-time pagetable setup
> > in MMU system.
> > But in MPU system, we have already built up start-of-day Xen MPU
> > memory region mapping at the very beginning in assembly.
> >
> > So in order to keep only one codeflow in arm/setup.c,
> > setup_mm_mappings , with a more generic name, is introduced and act as
> > an empty stub in MPU system.
> 
> is the empty stub temporarily?
> 
> >
> > Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> > Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> > ---
> >   xen/arch/arm/include/asm/mm.h     |  2 ++
> >   xen/arch/arm/include/asm/mm_mpu.h | 16 ++++++++++++++++
> >   xen/arch/arm/setup.c              |  2 +-
> >   3 files changed, 19 insertions(+), 1 deletion(-)
> >   create mode 100644 xen/arch/arm/include/asm/mm_mpu.h
> >
> > diff --git a/xen/arch/arm/include/asm/mm.h
> > b/xen/arch/arm/include/asm/mm.h index 1b9fdb6ff5..9b4c07d965 100644
> > --- a/xen/arch/arm/include/asm/mm.h
> > +++ b/xen/arch/arm/include/asm/mm.h
> > @@ -243,6 +243,8 @@ static inline void __iomem *ioremap_wc(paddr_t
> > start, size_t len)
> >
> >   #ifndef CONFIG_HAS_MPU
> >   #include <asm/mm_mmu.h>
> > +#else
> > +#include <asm/mm_mpu.h>
> >   #endif
> >
> >   /* Page-align address and convert to frame number format */ diff
> > --git a/xen/arch/arm/include/asm/mm_mpu.h
> > b/xen/arch/arm/include/asm/mm_mpu.h
> > new file mode 100644
> > index 0000000000..1f3cff7743
> > --- /dev/null
> > +++ b/xen/arch/arm/include/asm/mm_mpu.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */ #ifndef
> > +__ARCH_ARM_MM_MPU__ #define __ARCH_ARM_MM_MPU__
> > +
> > +#define setup_mm_mappings(boot_phys_offset)
> > +((void)(boot_phys_offset))
> > +
> > +#endif /* __ARCH_ARM_MM_MPU__ */
> > +
> > +/*
> > + * Local variables:
> > + * mode: C
> > + * c-file-style: "BSD"
> > + * c-basic-offset: 4
> > + * indent-tabs-mode: nil
> > + * End:
> > + */
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index
> > 1f26f67b90..d7d200179c 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -1003,7 +1003,7 @@ void __init start_xen(unsigned long
> boot_phys_offset,
> >       /* Initialize traps early allow us to get backtrace when an error 
> > occurred
> */
> >       init_traps();
> >
> > -    setup_pagetables(boot_phys_offset);
> > +    setup_mm_mappings(boot_phys_offset);
> 
> You are renaming the caller but not the function. Why?
> 

It is a reorg mistake.  MMU-related implementation has been mistakenly
put in previous commit "[PATCH v2 15/40] xen/arm: move MMU-specific
memory management code to 
mm_mmu.c/mm_mmu.h"(https://lists.xenproject.org/archives/html/xen-devel/2023-01/msg00776.html
 )
Sorry for that.
I'll extract the relative codes from the previous commit:
'''
+/* Boot-time pagetable setup */
+#define setup_mm_mappings(boot_phys_offset) setup_pagetables(boot_phys_offset)
'''

> >
> >       smp_clear_cpu_maps();
> >
> 
> Cheers,
> 
> --
> Julien Grall