Hi Julien, > On 27 Jan 2023, at 20:55, Julien Grall <[email protected]> wrote: > > From: Julien Grall <[email protected]> > > Switching TTBR while the MMU is on is not safe. Now that the identity > mapping will not clash with the rest of the memory layout, we can avoid > creating temporary page-tables every time a CPU is brought up. > > The arm32 code will use a different approach. So this issue is for now > only resolved on arm64. > > Signed-off-by: Julien Grall <[email protected]> > Reviewed-by: Luca Fancellu <[email protected]> > Tested-by: Luca Fancellu <[email protected]>
Reviewed-by: Bertrand Marquis <[email protected] <mailto:[email protected]>> Cheers Bertrand > > --- > Changes in v5: > - Add Luca's reviewed-by and tested-by tags. > > Changes in v4: > - Somehow I forgot to send it in v3. So re-include it. > > Changes in v2: > - Remove arm32 code > --- > xen/arch/arm/arm32/smpboot.c | 4 ++++ > xen/arch/arm/arm64/head.S | 29 +++++++++-------------------- > xen/arch/arm/arm64/smpboot.c | 15 ++++++++++++++- > xen/arch/arm/include/asm/smp.h | 1 + > xen/arch/arm/smpboot.c | 1 + > 5 files changed, 29 insertions(+), 21 deletions(-) > > diff --git a/xen/arch/arm/arm32/smpboot.c b/xen/arch/arm/arm32/smpboot.c > index e7368665d50d..518e9f9c7e70 100644 > --- a/xen/arch/arm/arm32/smpboot.c > +++ b/xen/arch/arm/arm32/smpboot.c > @@ -21,6 +21,10 @@ int arch_cpu_up(int cpu) > return platform_cpu_up(cpu); > } > > +void arch_cpu_up_finish(void) > +{ > +} > + > /* > * Local variables: > * mode: C > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 5efd442b24af..a61b4d3c2738 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -308,6 +308,7 @@ real_start_efi: > bl check_cpu_mode > bl cpu_init > bl create_page_tables > + load_paddr x0, boot_pgtable > bl enable_mmu > > /* We are still in the 1:1 mapping. Jump to the runtime Virtual > Address. */ > @@ -365,29 +366,14 @@ GLOBAL(init_secondary) > #endif > bl check_cpu_mode > bl cpu_init > - bl create_page_tables > + load_paddr x0, init_ttbr > + ldr x0, [x0] > bl enable_mmu > > /* We are still in the 1:1 mapping. Jump to the runtime Virtual > Address. */ > ldr x0, =secondary_switched > br x0 > secondary_switched: > - /* > - * Non-boot CPUs need to move on to the proper pagetables, which were > - * setup in init_secondary_pagetables. > - * > - * XXX: This is not compliant with the Arm Arm. > - */ > - ldr x4, =init_ttbr /* VA of TTBR0_EL2 stashed by CPU 0 */ > - ldr x4, [x4] /* Actual value */ > - dsb sy > - msr TTBR0_EL2, x4 > - dsb sy > - isb > - tlbi alle2 > - dsb sy /* Ensure completion of TLB flush */ > - isb > - > #ifdef CONFIG_EARLY_PRINTK > /* Use a virtual address to access the UART. */ > ldr x23, =EARLY_UART_VIRTUAL_ADDRESS > @@ -672,9 +658,13 @@ ENDPROC(create_page_tables) > * mapping. In other word, the caller is responsible to switch to the runtime > * mapping. > * > - * Clobbers x0 - x3 > + * Inputs: > + * x0 : Physical address of the page tables. > + * > + * Clobbers x0 - x4 > */ > enable_mmu: > + mov x4, x0 > PRINT("- Turning on paging -\r\n") > > /* > @@ -685,8 +675,7 @@ enable_mmu: > dsb nsh > > /* Write Xen's PT's paddr into TTBR0_EL2 */ > - load_paddr x0, boot_pgtable > - msr TTBR0_EL2, x0 > + msr TTBR0_EL2, x4 > isb > > mrs x0, SCTLR_EL2 > diff --git a/xen/arch/arm/arm64/smpboot.c b/xen/arch/arm/arm64/smpboot.c > index 694fbf67e62a..9637f424699e 100644 > --- a/xen/arch/arm/arm64/smpboot.c > +++ b/xen/arch/arm/arm64/smpboot.c > @@ -106,10 +106,23 @@ int __init arch_cpu_init(int cpu, struct dt_device_node > *dn) > > int arch_cpu_up(int cpu) > { > + int rc; > + > if ( !smp_enable_ops[cpu].prepare_cpu ) > return -ENODEV; > > - return smp_enable_ops[cpu].prepare_cpu(cpu); > + update_identity_mapping(true); > + > + rc = smp_enable_ops[cpu].prepare_cpu(cpu); > + if ( rc ) > + update_identity_mapping(false); > + > + return rc; > +} > + > +void arch_cpu_up_finish(void) > +{ > + update_identity_mapping(false); > } > > /* > diff --git a/xen/arch/arm/include/asm/smp.h b/xen/arch/arm/include/asm/smp.h > index 8133d5c29572..a37ca55bff2c 100644 > --- a/xen/arch/arm/include/asm/smp.h > +++ b/xen/arch/arm/include/asm/smp.h > @@ -25,6 +25,7 @@ extern void noreturn stop_cpu(void); > extern int arch_smp_init(void); > extern int arch_cpu_init(int cpu, struct dt_device_node *dn); > extern int arch_cpu_up(int cpu); > +extern void arch_cpu_up_finish(void); > > int cpu_up_send_sgi(int cpu); > > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c > index 412ae2286906..4a89b3a8345b 100644 > --- a/xen/arch/arm/smpboot.c > +++ b/xen/arch/arm/smpboot.c > @@ -500,6 +500,7 @@ int __cpu_up(unsigned int cpu) > init_data.cpuid = ~0; > smp_up_cpu = MPIDR_INVALID; > clean_dcache(smp_up_cpu); > + arch_cpu_up_finish(); > > if ( !cpu_online(cpu) ) > { > -- > 2.38.1 > >
