Re: [PATCH v3 08/52] xen/arm64: move MMU related code from head.S to mmu/head.S

[Penny Zheng <penny.zheng@xxxxxxx>]

Hi Julien

On 2023/7/5 18:43, Julien Grall wrote:
> Hi Penny,
>
> One more remark.
>
> On 26/06/2023 04:33, Penny Zheng wrote:
> > From: Wei Chen <wei.chen@xxxxxxx>
> >
> > There are lots of MMU specific code in head.S. This code will not
> > be used in MPU systems. If we use #ifdef to gate them, the code
> > will become messy and hard to maintain. So we move MMU related
> > code to mmu/head.S, and keep common code still in head.S. We also
> > add .text.idmap in mmu/head.S to make all code in this new file
> > are still in identity map page but will be linked after head.S.
> >
> > As "fail" in head.S is very simple and this name is too easy to
> > be conflicted, so duplicate it in mmu/head.S instead of exporting
> > it.
> >
> > And some assembly macros that will be shared by MMU and MPU later,
> > we move them to macros.h.
> >
> > Rename enable_boot_mmu()/enable_runtime_mmu() to a more generic name
> > enable_boot_mm()/enable_runtime_mm(), in order to make them common 
> > interfaces
> > to be used for both MMU and later MPU system.
> >
> > Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> > Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> > ---
> > v1 -> v2:
> > 1. Move macros to macros.h
> > 2. Remove the indention modification
> > 3. Duplicate "fail" instead of exporting it.
> > ---
> > v3:
> > - Rename enable_boot_mmu()/enable_runtime_mmu() to a more generic name
> > enable_boot_mm()/enable_runtime_mm()
> > ---
> >   xen/arch/arm/arm64/Makefile             |   3 +
> >   xen/arch/arm/arm64/head.S               | 469 +-----------------------
> >   xen/arch/arm/arm64/mmu/head.S           | 453 +++++++++++++++++++++++
> >   xen/arch/arm/include/asm/arm64/macros.h |  51 +++
> >   4 files changed, 509 insertions(+), 467 deletions(-)
> >   create mode 100644 xen/arch/arm/arm64/mmu/head.S
> >
> > diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
> > index 54ad55c75c..0c4b177be9 100644
> > --- a/xen/arch/arm/arm64/Makefile
> > +++ b/xen/arch/arm/arm64/Makefile
> > @@ -8,6 +8,9 @@ obj-y += domctl.o
> >   obj-y += domain.o
> >   obj-y += entry.o
> >   obj-y += head.o
> > +ifeq ($(CONFIG_HAS_MMU),y)
> > +obj-y += mmu/head.o
> > +endif
> You could use obj-$(CONFIG_HAS_MMU) += mmu/head.o.
>
> But in this case, I would rather prefer if we match how other 
> subdirectory are added. I.e. on the parent's directory Makefile you add:
> obj-$(CONFIG_MMU) += mmu/
>
> And in the directory you add a Makefile which list the files to compile.
Understood. thanks for the instruction.

> Cheers,