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 05:46, Julien Grall wrote:
> Hi Penny,
>
> 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.
> Aren't those macros already shared between head.S and mmu/head.S?
Correct me if I understand wrongly, so you want to remove the mention of 
MPU, and rephrase it to
"
As some assembly macros need to be shared by head.S and mmu/head.S,
we will 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.
> As mentionned in an earlier patch, I would prefer if the name was 
> correct from the beginning. So this patch will be merely code movement.
Sure. will fix.


> > Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> > Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> I think this will need a rebase on top of the recent changes in head.S.
Will do in the next version.

> > 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
> [...]
>
> > -/*
> > - * Macro to print the value of register \xb
> > - *
> > - * Clobbers x0 - x4
> > - */
> > -.macro print_reg xb
> > -        mov   x0, \xb
> > -        mov   x4, lr
> > -        bl    putn
> > -        mov   lr, x4
> > -.endm
> I can't find any use of print_reg() in mmu/head.S. So is it necessary to 
> move right now?
True, it is only used in head.S. I'll leave it here.

> [...]
>
> > diff --git a/xen/arch/arm/arm64/mmu/head.S 
> > b/xen/arch/arm/arm64/mmu/head.S
> > new file mode 100644
> > index 0000000000..2b209fc3ce
> > --- /dev/null
> > +++ b/xen/arch/arm/arm64/mmu/head.S
> > @@ -0,0 +1,453 @@
> > +/*
> > + * xen/arch/arm/mmu/head.S
> > + *
> > + * Start-of-day code for an ARMv8.
> > + *
> > + * Ian Campbell <ian.campbell@xxxxxxxxxx>
> > + * Copyright (c) 2012 Citrix Systems.
> > + *
> > + * Based on ARMv7-A head.S by
> > + * Tim Deegan <tim@xxxxxxx>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> For new file (even split code), please use the SPDX tag. In this case, 
> the following line should be added at the top of the file:
> /* SPDX-License-Identifier: GPL-2.0-or-later */
Sure.

> Cheers,