Re: [PATCH 06/16] xen/arm: Introduce system suspend config option

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

HI,

On Fri, Mar 21, 2025 at 4:58 PM Grygorii Strashko
<grygorii_strashko@xxxxxxxx> wrote:
>
>
>
> On 21.03.25 11:48, Mykola Kvach wrote:
> > Hi,
> >
> > On Wed, Mar 12, 2025 at 12:29 AM Julien Grall <julien@xxxxxxx> wrote:
> >>
> >> Hi,
> >>
> >> On 05/03/2025 09:11, Mykola Kvach wrote:
> >>> From: Mykola Kvach <mykola_kvach@xxxxxxxx>
> >>>
> >>> This option enables the system suspend support. This is the
> >>> mechanism that allows the system to be suspended to RAM and
> >>> later resumed.
> >>>
> >>> Signed-off-by: Mykyta Poturai <mykyta_poturai@xxxxxxxx>
> >>> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
> >>> ---
> >>>    xen/arch/arm/Kconfig | 11 +++++++++++
> >>>    1 file changed, 11 insertions(+)
> >>>
> >>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> >>> index a26d3e1182..5834af16ab 100644
> >>> --- a/xen/arch/arm/Kconfig
> >>> +++ b/xen/arch/arm/Kconfig
> >>> @@ -475,6 +475,17 @@ config ARM64_HARDEN_BRANCH_PREDICTOR
> >>>    config ARM32_HARDEN_BRANCH_PREDICTOR
> >>>        def_bool y if ARM_32 && HARDEN_BRANCH_PREDICTOR
> >>>
> >>> +config SYSTEM_SUSPEND
> >>> +     bool "System suspend support"
> >>> +     default y
> >>
> >> The default should likely be no until everything is working.
> >
> > got it!
> >
> >>
> >>> +     depends on ARM_64
> >>
> >> I think this also needs to depends on !LLC_COLORING (unless you
> >> confirmed cache coloring is working) and UNSUPPORTED.
> >
> > Sure! I'll add the dependency.
> >
> >>
> >>> +     help
> >>> +       This option enables the system suspend support. This is the
> >>> +       mechanism that allows the system to be suspended to RAM and
> >>> +       later resumed.
> >>
> >> You seem to also tie guest suspend/resunme to this option. Is it intended?
> >
> >  From the guest's perspective, it is a system suspend. However, it looks 
> > like the
> > description should be enhanced. Thank you for pointing that out.
>
> s2r = "suspend to ram"
>
> You definitely need consider and clarify ARM64 Guest System s2r and
> XEN system s2r. First can be supported without second, while the XEN system 
> s2r
> depends on Guests System s2r support and required guests to be properly 
> suspended
> before allowing XEN to enter system s2r.
>

This is exactly what...

> You can't call freeze_domains() and blindly pause some domain, because if 
> it's not
> suspend and has passed through HW which is in the middle of transaction -> 
> DEADBEEF.

... should happen. x86 works in the same way—we call domain_pause when
performing system suspend. All domains are paused except the one that
is eligible to request system suspend.

>
> --
> Best regards,
> -grygorii

Best regards,
Mykola