[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 05/40] xen/arm64: prepare for moving MMU related code from head.S
On 18/01/2023 03:09, Wei Chen wrote:
Hi Julien,
-----Original Message-----
From: Julien Grall <julien@xxxxxxx>
Sent: 2023年1月18日 7:37
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 05/40] xen/arm64: prepare for moving MMU related
code from head.S
Hi Penny,
On 13/01/2023 05:28, Penny Zheng wrote:
From: Wei Chen <wei.chen@xxxxxxx>
We want to reuse head.S for MPU systems, but there are some
code implemented for MMU systems only. We will move such
code to another MMU specific file. But before that, we will
do some preparations in this patch to make them easier
for reviewing:
Well, I agree that...
1. Fix the indentations of code comments.
... changing the indentation is better here. But...
2. Export some symbols that will be accessed out of file
scope.
... I have no idea which functions are going to be used in a separate
file. So I think they should belong to the patch moving the code.
Ok, I will move these changes to the moving code patches.
Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
Your signed-off-by is missing.
---
v1 -> v2:
1. New patch.
---
xen/arch/arm/arm64/head.S | 40 +++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index 93f9b0b9d5..b2214bc5e3 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -136,22 +136,22 @@
add \xb, \xb, x20
.endm
- .section .text.header, "ax", %progbits
- /*.aarch64*/
+.section .text.header, "ax", %progbits
+/*.aarch64*/
This change is not mentioned.
I will add the description in commit message.
- /*
- * Kernel startup entry point.
- * ---------------------------
- *
- * The requirements are:
- * MMU = off, D-cache = off, I-cache = on or off,
- * x0 = physical address to the FDT blob.
- *
- * This must be the very first address in the loaded image.
- * It should be linked at XEN_VIRT_START, and loaded at any
- * 4K-aligned address. All of text+data+bss must fit in 2MB,
- * or the initial pagetable code below will need adjustment.
- */
+/*
+ * Kernel startup entry point.
+ * ---------------------------
+ *
+ * The requirements are:
+ * MMU = off, D-cache = off, I-cache = on or off,
+ * x0 = physical address to the FDT blob.
+ *
+ * This must be the very first address in the loaded image.
+ * It should be linked at XEN_VIRT_START, and loaded at any
+ * 4K-aligned address. All of text+data+bss must fit in 2MB,
+ * or the initial pagetable code below will need adjustment.
+ */
GLOBAL(start)
/*
@@ -586,7 +586,7 @@ ENDPROC(cpu_init)
*
* Clobbers x0 - x4
*/
-create_page_tables:
+ENTRY(create_page_tables)
I am not sure about keeping this name. Now we have create_page_tables()
and arch_setup_page_tables().
I would conside to name it create_boot_page_tables().
Do you need me to rename it in this patch?
So looking at the rest of the series, I see you are already renaming the
helper in patch #11. I think it would be better if the naming is done
earlier.
That said, I am not convinced that create_page_tables() should actually
be called externally.
In fact, you have something like:
bl create_page_tables
bl enable_mmu
Both will need a MMU/MPU specific implementation. So it would be better
if we provide a wrapper to limit the number of external functions.
Cheers,
--
Julien Grall
|
