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

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

Hi Luca,

Thank you for the review.

On Mon, Apr 27, 2026 at 5:51 PM Luca Fancellu <Luca.Fancellu@xxxxxxx> wrote:
>
> Hi Mykola,
>
> > On 2 Apr 2026, at 11:45, Mykola Kvach <xakep.amatop@xxxxxxxxx> wrote:
> >
> > 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.
> >
> > This patch adds MMU setup during resume by reusing the existing
> > enable_secondary_cpu_mm function, which enables data cache and the MMU.
>
> I don’t understand where this last part happen in this commit:

This is a leftover from before the commits were reorganized. I will update the
commit message in v9 so that it only describes what this patch actually does.

>
> > 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)
>
> I think we should mask all exceptions here:
> msr DAIFSet, 0xf
>
> until we resume correctly the status (VBAR_EL2, etc).

This was discussed in an earlier version:

https://patchew.org/Xen/cover.1741164138.git.xakep.amatop@xxxxxxxxx/2ef15cb605f987eb087c5496d123c47c01cc0ae7.1741164138.git.xakep.amatop@xxxxxxxxx/#CAGeoDV97no7mXSKd7auFu5E85wSXAHKWvqGW2=-VEAbkrTyU8Q@xxxxxxxxxxxxxx

For SYSTEM_SUSPEND, PSCI ties the call semantics to CPU_SUSPEND. In
particular, section 5.20.2 says that the caller must observe all the rules
described for CPU_SUSPEND, and section 6.4 explicitly says that the initial
state rules also apply to SYSTEM_SUSPEND.

For the return Exception level on AArch64, section 6.4.3.3 requires
SPSR_ELx.{D,A,I,F} to be set to {1, 1, 1, 1}. Therefore Xen expects to enter
this resume path with DAIF already masked by PSCI-compliant firmware.

I agree this assumption is not obvious from the code, so I will add a comment
at the resume entry point to document that this path relies on the PSCI initial
core configuration requirements.

>
> > +        /* 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 .
> > +END(hyp_resume)
> > +
> > +#endif /* CONFIG_SYSTEM_SUSPEND */
> > +
> > /*
> >  * Local variables:
> >  * mode: ASM
> >
>
> This is more a trampoline for the core resuming, not sure if it could be 
> better to squash this
> into the following patch, the maintainer could provide their preference.

Yes, this patch is only the low-level resume trampoline before the context
restore code is added by the following patch. I do not have a strong preference
between keeping it separate and squashing it into the next patch. I can squash
them in v9 unless the maintainers prefer to keep the trampoline separate.

Best regards,
Mykola

>
> Cheers,
> Luca
>
>
>