Hi Penny,
On 05/07/2023 10:03, Penny Zheng wrote:
On 2023/7/5 06:25, Julien Grall wrote:
Hi Penny,
Title: You want to clarify that this change is arm64 only. So:
xen/arm64: mmu: ...
On 26/06/2023 04:34, Penny Zheng wrote:
Original setup_fixmap is actually doing two seperate tasks, one is
enabling
the early UART when earlyprintk on, and the other is to set up the
fixmap
even when earlyprintk is not configured.
To be more dedicated and precise, the old function shall be split
into two
functions, setup_early_uart and new setup_fixmap.
While some of the split before would be warrant even without the MPU
support. This one is not because there is limited point to have 3
lines function. So I think you want to justify based on what you plan
to do with the MPU code.
That said, I don't think we need to introduce setup_fixmap. See below.
Signed-off-by: Penny Zheng <penny.zh...@arm.com>
Signed-off-by: Wei Chen <wei.c...@arm.com>
---
v3:
- new patch
---
xen/arch/arm/arm64/head.S | 3 +++
xen/arch/arm/arm64/mmu/head.S | 24 +++++++++++++++++-------
2 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index e63886b037..55a4cfe69e 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -258,7 +258,10 @@ real_start_efi:
b enable_boot_mm
primary_switched:
+ bl setup_early_uart
+#ifdef CONFIG_HAS_FIXMAP
bl setup_fixmap
+#endif
#ifdef CONFIG_EARLY_PRINTK
/* Use a virtual address to access the UART. */
ldr x23, =EARLY_UART_VIRTUAL_ADDRESS
diff --git a/xen/arch/arm/arm64/mmu/head.S
b/xen/arch/arm/arm64/mmu/head.S
index 2b209fc3ce..295596aca1 100644
--- a/xen/arch/arm/arm64/mmu/head.S
+++ b/xen/arch/arm/arm64/mmu/head.S
@@ -367,24 +367,34 @@ identity_mapping_removed:
ENDPROC(remove_identity_mapping)
/*
- * Map the UART in the fixmap (when earlyprintk is used) and hook the
- * fixmap table in the page tables.
- *
- * The fixmap cannot be mapped in create_page_tables because it may
- * clash with the 1:1 mapping.
Since commit 9d267c049d92 ("xen/arm64: Rework the memory layout"),
there is no chance that the fixmap will clash with the 1:1 mapping. So
rather than introducing a new function, I would move the creation of
the fixmap in create_pagetables().
Understood. I'll move the creation of the fixmap in create_pagetables().
This would avoid the #ifdef CONFIG_HAS_FIXMAP in head.S.
+ * Map the UART in the fixmap (when earlyprintk is used)
*
* Inputs:
- * x20: Physical offset
* x23: Early UART base physical address
*
* Clobbers x0 - x3
*/
-ENTRY(setup_fixmap)
+ENTRY(setup_early_uart)
#ifdef CONFIG_EARLY_PRINTK
/* Add UART to the fixmap table */
ldr x0, =EARLY_UART_VIRTUAL_ADDRESS
create_mapping_entry xen_fixmap, x0, x23, x1, x2, x3,
type=PT_DEV_L3
+ /* Ensure any page table updates made above have occurred. */
+ dsb nshst
+
+ ret
The 'ret' needs to be outside of the '#ifdef' block. But, in this
case, I would prefer if we don't call setup_early_uart() when
!CONFIG_EARLY_PRINTK in head.S
okay. I'll move the #ifdef to the caller in head.S.
Thinking about this again. I think you can actually move the mapping of
the UART in create_pagetables() because it will also not clash with the 1:1.
For the MPU, the mapping could then be moved in prepare_early_mappings().
This would reduce the number of functions exposed.
Cheers,
--
Julien Grall
