| Message ID | 2ef15cb605f987eb087c5496d123c47c01cc0ae7.1741164138.git.xakep.amatop@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Suspend to RAM support for Xen on arm64 | expand |
Hi, On 05/03/2025 09:11, Mykola Kvach wrote: > From: Mirela Simonovic <mirela.simonovic@aggios.com> > > The MMU needs to be enabled in the resume flow before the context > can be restored (we need to be able to access the context data by > virtual address in order to restore it). The configuration of system > registers prior to branching to the routine that sets up the page > tables is copied from xen/arch/arm/arm64/head.S. > After the MMU is enabled, the content of TTBR0_EL2 is changed to > point to init_ttbr (page tables used at runtime). > > At boot the init_ttbr variable is updated when a secondary CPU is > hotplugged. In the scenario where there is only one physical CPU in > the system, the init_ttbr would not be initialized for the use in > resume flow. To get the variable initialized in all scenarios in this > patch we add that the boot CPU updates init_ttbr after it sets the > page tables for runtime. > > Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com> > Signed-off-by: Saeed Nowshadi <saeed.nowshadi@xilinx.com> > Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com> > --- > Changes in V3: > - updated commit message > - instead of using create_page_tables, enable_mmu, and mmu_init_secondary_cpu, > the existing function enable_secondary_cpu_mm is now used > - prepare_secondary_mm (previously init_secondary_pagetables in the previous > patch series) is now called at the end of start_xen instead of > setup_pagetables. Calling it in the previous location caused a crash > - add early printk init during resume > > Changes in V2: > - moved hyp_resume to head.S to place it near the rest of the start code > - simplified the code in hyp_resume by using existing functions such as > check_cpu_mode, cpu_init, create_page_tables, and enable_mmu > --- > xen/arch/arm/arm64/head.S | 23 +++++++++++++++++++++++ > xen/arch/arm/setup.c | 8 ++++++++ > 2 files changed, 31 insertions(+) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 3522c497c5..fab2812e54 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -564,6 +564,29 @@ END(efi_xen_start) > #ifdef CONFIG_SYSTEM_SUSPEND > > FUNC(hyp_resume) > + msr DAIFSet, 0xf /* Disable all interrupts */ Surely we should be here with interrupts disabled. No? > + > + tlbi alle2 > + dsb sy /* Ensure completion of TLB flush */ This doesn't exist when booting Xen and I am not sure why we would need it for resume as we already nuke the TLbs in enable_mmu. Can you clarify? > + isb > + > + ldr x0, =start > + adr x19, start /* x19 := paddr (start) */ > + sub x20, x19, x0 /* x20 := phys-offset */ Looking at the code, I wonder if it is still necessary to set x19 and x20 for booting secondary CPUs and resume. There doesn't seem to be any use of the registers. > + > + /* Initialize the UART if earlyprintk has been enabled. */ > +#ifdef CONFIG_EARLY_PRINTK > + bl init_uart > +#endif > + PRINT_ID("- Xen resuming -\r\n") > + > + bl check_cpu_mode > + bl cpu_init > + > + ldr lr, =mmu_resumed > + b enable_secondary_cpu_mm > + > +mmu_resumed: > b . > END(hyp_resume) > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index ffcae900d7..3a89ac436b 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -508,6 +508,14 @@ void asmlinkage __init start_xen(unsigned long fdt_paddr) > for_each_domain( d ) > domain_unpause_by_systemcontroller(d); > > +#ifdef CONFIG_SYSTEM_SUSPEND > + /* > + * It is called to initialize init_ttbr. > + * Without this call, Xen gets stuck after resuming. This is not a very descriptive comment. But, you seem to make the assumption that prepare_secondary_mm() can be called on the boot CPU. This is not always the case. For instance on arm32, it will allocate memory and overwrite the per-cpu page-tables pointer (not great). This will also soon be the case for arm64. Furthermore, this call reminded me that the secondary CPU page-tables are not freed when turning off a CPU. So they will leak. Not yet a problem for arm64 though. So overall, I think we need a separate function that will be prepare init_ttbr for a given CPU (not allocate any memory). This will then need to be called from the suspend helper. > + */ > + prepare_secondary_mm(0);> +#endif > + > /* Switch on to the dynamically allocated stack for the idle vcpu > * since the static one we're running on is about to be freed. */ > memcpy(idle_vcpu[0]->arch.cpu_info, get_cpu_info(), Cheers,
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 3522c497c5..fab2812e54 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -564,6 +564,29 @@ END(efi_xen_start) #ifdef CONFIG_SYSTEM_SUSPEND FUNC(hyp_resume) + msr DAIFSet, 0xf /* Disable all interrupts */ + + tlbi alle2 + dsb sy /* Ensure completion of TLB flush */ + isb + + ldr x0, =start + adr x19, start /* x19 := paddr (start) */ + sub x20, x19, x0 /* x20 := phys-offset */ + + /* Initialize the UART if earlyprintk has been enabled. */ +#ifdef CONFIG_EARLY_PRINTK + bl init_uart +#endif + PRINT_ID("- Xen resuming -\r\n") + + bl check_cpu_mode + bl cpu_init + + ldr lr, =mmu_resumed + b enable_secondary_cpu_mm + +mmu_resumed: b . END(hyp_resume) diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index ffcae900d7..3a89ac436b 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -508,6 +508,14 @@ void asmlinkage __init start_xen(unsigned long fdt_paddr) for_each_domain( d ) domain_unpause_by_systemcontroller(d); +#ifdef CONFIG_SYSTEM_SUSPEND + /* + * It is called to initialize init_ttbr. + * Without this call, Xen gets stuck after resuming. + */ + prepare_secondary_mm(0); +#endif + /* Switch on to the dynamically allocated stack for the idle vcpu * since the static one we're running on is about to be freed. */ memcpy(idle_vcpu[0]->arch.cpu_info, get_cpu_info(),