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

[Julien Grall <julien@xxxxxxx>]

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?

> 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.
> 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.

> 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?
[...]

> 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 */

Cheers,

--
Julien Grall