Hi Julien, > On Aug 9, 2023, at 20:28, Julien Grall <jul...@xen.org> wrote: > > Hi Henry, > > Title: NIT: It is more a fold rather than move.
You are correct, using “fold” here is more appropriate. > > On 01/08/2023 04:44, Henry Wang wrote: >> For the >> future MPU support work, the early UART mapping could then be moved >> in prepare_early_mappings(). > > I would drop this sentence as this is more related to the future > implementation of MPU rather than this patch itself. Sure, v5 will drop this. > >> No functional change intended. >> [1] >> https://lore.kernel.org/xen-devel/78862bb8-fd7f-5a51-a7ae-3c5b5998e...@xen.org/ >> Signed-off-by: Henry Wang <henry.w...@arm.com> >> --- >> v4: >> - Rework "[v3,12/52] xen/mmu: extract early uart mapping from setup_fixmap" >> --- >> xen/arch/arm/arm64/head.S | 1 - >> xen/arch/arm/arm64/mmu/head.S | 50 ++++++++++++----------------------- >> 2 files changed, 17 insertions(+), 34 deletions(-) >> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >> index e4f579a48e..56f68a8e37 100644 >> --- a/xen/arch/arm/arm64/head.S >> +++ b/xen/arch/arm/arm64/head.S >> @@ -275,7 +275,6 @@ real_start_efi: >> b enable_boot_cpu_mm >> primary_switched: >> - bl setup_fixmap >> #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 b7c3dd423a..6bd94c3a45 100644 >> --- a/xen/arch/arm/arm64/mmu/head.S >> +++ b/xen/arch/arm/arm64/mmu/head.S > > The second paragraph in create_page_tables() now needs to be dropped. Thanks for finding this! Yes this should be dropped. > >> @@ -231,6 +231,23 @@ link_from_second_id: >> create_table_entry boot_second_id, boot_third_id, x19, 2, x0, x1, x2 >> link_from_third_id: >> create_mapping_entry boot_third_id, x19, x19, x0, x1, x2 >> + >> +#ifdef CONFIG_EARLY_PRINTK >> + /* Add UART to the fixmap table */ >> + ldr x0, =EARLY_UART_VIRTUAL_ADDRESS >> + /* x23: Early UART base physical address */ >> + create_mapping_entry xen_fixmap, x0, x23, x1, x2, x3, type=PT_DEV_L3 >> +#endif >> + /* Map fixmap into boot_second */ >> + ldr x0, =FIXMAP_ADDR(0) >> + create_table_entry boot_second, xen_fixmap, x0, 2, x1, x2, x3 >> + /* Ensure any page table updates made above have occurred. */ >> + dsb nshst >> + /* >> + * The fixmap area will be used soon after. So ensure no hardware >> + * translation happens before the dsb completes. >> + */ >> + isb > > The DSB/ISB is not necessary anymore as you are not modifying live > page-tables. So I would prefer if this is removed. Sure. Kind regards, Henry > > Cheers, > > -- > Julien Grall
