Re: [PATCH v8 10/13] xen/arm: Resume memory management on Xen resume

[Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>]

On 5/8/26 23:59, Mykola Kvach wrote:

Hello Mykola

> Hi Volodymyr,
>
> Thank you for the feedback.
>
> On Fri, May 8, 2026 at 1:06 AM Volodymyr Babchuk
> <Volodymyr_Babchuk@xxxxxxxx> wrote:
> > Hi Mykola,
> >
> > Mykola Kvach <xakep.amatop@xxxxxxxxx> writes:
> >
> > > From: Mirela Simonovic <mirela.simonovic@xxxxxxxxxx>
> > >
> > > The MMU must be enabled during the resume path before restoring context,
> > > as virtual addresses are used to access the saved context data.
> > I agree with Luca, this patch does not makes sense as is. I don't see
> > why it should be separated from the rest of the resume path that is
> > added in the next patch
> Ack. I'll combine this with the next patch in v9.
>
> Best regards,
> Mykola
>
> > > This patch adds MMU setup during resume by reusing the existing
> > > enable_secondary_cpu_mm function, which enables data cache and the MMU.
> > > Before the MMU is enabled, the content of TTBR0_EL2 is changed to point
> > > to init_ttbr (page tables used at runtime).
> > >
> > > Signed-off-by: Mirela Simonovic <mirela.simonovic@xxxxxxxxxx>
> > > Signed-off-by: Saeed Nowshadi <saeed.nowshadi@xxxxxxxxxx>
> > > Signed-off-by: Mykyta Poturai <mykyta_poturai@xxxxxxxx>
> > > Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
> > > ---
> > > Changes in v7:
> > > - no functional changes, just moved commit
> > > ---
> > >   xen/arch/arm/arm64/head.S | 24 ++++++++++++++++++++++++
> > >   1 file changed, 24 insertions(+)
> > >
> > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> > > index 72c7b24498..596e960152 100644
> > > --- a/xen/arch/arm/arm64/head.S
> > > +++ b/xen/arch/arm/arm64/head.S
> > > @@ -561,6 +561,30 @@ END(efi_xen_start)
> > >
> > >   #endif /* CONFIG_ARM_EFI */
> > >
> > > +#ifdef CONFIG_SYSTEM_SUSPEND
> > > +
> > > +FUNC(hyp_resume)
> > > +        /* 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   x0, =start
> > > +        adr   x20, start             /* x20 := paddr (start) */
> > > +        sub   x20, x20, x0           /* x20 := phys-offset */
> > > +        ldr   lr, =mmu_resumed
> > > +        b     enable_secondary_cpu_mm
> > > +
> > > +mmu_resumed:
> > > +        b .
I also think this patch would be better squashed with the next one, as 
they are tightly coupled.
During the review of patch 11, I had to switch between patches 10 and 11 
several times to understand the full context—patch 10 sets up hyp_resume 
with a placeholder (b .), and patch 11 immediately fills in the actual 
context restore.

> > > +END(hyp_resume)
> > > +
> > > +#endif /* CONFIG_SYSTEM_SUSPEND */
> > > +
> > >   /*
> > >    * Local variables:
> > >    * mode: ASM
> > --
> > WBR, Volodymyr