Re: [PATCH v9 10/13] xen/arm64: Save/restore CPU context across SYSTEM_SUSPEND

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

Hi Luca,

Thank you for the detailed review.

On Thu, May 14, 2026 at 8:22 PM Luca Fancellu <Luca.Fancellu@xxxxxxx> wrote:
>
> Hi Mykola,
>
> >
> > +#ifdef CONFIG_SYSTEM_SUSPEND
> > +/*
> > + * int prepare_resume_ctx(void)
> > + *
> > + * CPU context saved here will be restored on resume in hyp_resume 
> > function.
> > + * prepare_resume_ctx shall return a non-zero value. Upon restoring context
> > + * hyp_resume shall return value zero instead. From C code that invokes
> > + * prepare_resume_ctx, the return value is interpreted to determine whether
> > + * the context is saved (prepare_resume_ctx) or restored (hyp_resume).
> > + */
> > +FUNC(prepare_resume_ctx)
> > +        ldr   x0, =resume_cpu_context
> > +
> > +        /* Store callee-saved registers */
> > +        stp   x19, x20, [x0, #RESUME_CTX_X19]
> > +        stp   x21, x22, [x0, #RESUME_CTX_X21]
> > +        stp   x23, x24, [x0, #RESUME_CTX_X23]
> > +        stp   x25, x26, [x0, #RESUME_CTX_X25]
> > +        stp   x27, x28, [x0, #RESUME_CTX_X27]
> > +        stp   x29, lr, [x0, #RESUME_CTX_X29]
> > +
> > +        /* Store stack-pointer */
> > +        mov   x2, sp
> > +        str   x2, [x0, #RESUME_CTX_SP]
> > +
> > +        /* Store system control registers */
> > +        mrs   x2, VBAR_EL2
> > +        str   x2, [x0, #RESUME_CTX_VBAR_EL2]
> > +        mrs   x2, VTCR_EL2
> > +        str   x2, [x0, #RESUME_CTX_VTCR_EL2]
> > +        mrs   x2, VTTBR_EL2
> > +        str   x2, [x0, #RESUME_CTX_VTTBR_EL2]
> > +        mrs   x2, TPIDR_EL2
> > +        str   x2, [x0, #RESUME_CTX_TPIDR_EL2]
> > +        mrs   x2, MDCR_EL2
> > +        str   x2, [x0, #RESUME_CTX_MDCR_EL2]
> > +        mrs   x2, HSTR_EL2
> > +        str   x2, [x0, #RESUME_CTX_HSTR_EL2]
> > +        mrs   x2, CPTR_EL2
> > +        str   x2, [x0, #RESUME_CTX_CPTR_EL2]
> > +        mrs   x2, HCR_EL2
> > +        str   x2, [x0, #RESUME_CTX_HCR_EL2]
>
> Do you think we should save also CNTHCTL_EL2? Apologies it escaped my first 
> review,
> but I see we program it in the boot cpu path + secondary cpu path: 
> init_timer_interrupt().
>
> The rest looks ok.

Ack. Good catch, thanks.

CNTHCTL_EL2 is programmed from init_timer_interrupt() for both the boot
CPU and secondary CPUs, so it should be part of the EL2 context saved and
restored across SYSTEM_SUSPEND. I will add save/restore for it.

I will also re-audit the context save/restore list against the current
boot path to check whether any other EL2 system registers are missing.

Best regards,
Mykola