Hi all, Friendly ping on this series.
I believe all previously received comments have been addressed in this revision, and I’m not aware of any remaining open issues. Could you please take another look when you have a moment? If it looks good, an Acked-by or Reviewed-by would be very appreciated; otherwise, I’m happy to iterate on any further feedback. Best regards, Mykola On Fri, Dec 12, 2025 at 3:22 PM Mykola Kvach <[email protected]> wrote: > > From: Mykola Kvach <[email protected]> > > Add support for the PSCI SYSTEM_SUSPEND function in the vPSCI interface, > allowing guests to request suspend via the PSCI v1.0+ SYSTEM_SUSPEND call > (both 32-bit and 64-bit variants). > > Implementation details: > - Add SYSTEM_SUSPEND function IDs to PSCI definitions > - Trap and handle SYSTEM_SUSPEND in vPSCI > - Allow only non-hardware domains to invoke SYSTEM_SUSPEND; return > PSCI_NOT_SUPPORTED for the hardware domain to avoid halting the system > in hwdom_shutdown() via domain_shutdown > - Require all secondary VCPUs of the calling domain to be offline before > suspend, as mandated by the PSCI specification > > The arch_domain_resume() function is an architecture-specific hook that is > invoked during domain resume to perform any necessary setup or restoration > steps required by the platform. arch_domain_resume() stays int to propagate > errno-style detail into common logging; preserving the integer keeps the > reason visible and leaves room for future arch-specific failures or richer > handling. > > The new vpsci_vcpu_up_prepare() helper is called on the resume path to set up > the vCPU context (such as entry point, some system regs and context ID) before > resuming a suspended guest. This keeps ARM/vPSCI-specific logic out of common > code and avoids intrusive changes to the generic resume flow. > > Usage: > > For Linux-based guests, suspend can be initiated with: > echo mem > /sys/power/state > or via: > systemctl suspend > > Resuming the guest is performed from control domain using: > xl resume <domain> > > Signed-off-by: Mykola Kvach <[email protected]> > --- > Changes in V16: > - Refactor error handling in domain_resume: move logging to generic code, > use explicit return code checking. > - Make context clearing conditional on success in arch_domain_resume. > - The 'int' return type is retained for arch_domain_resume for consistency > with other arch hooks and to allow for specific negative error codes. > --- > xen/arch/arm/domain.c | 39 +++++++++ > xen/arch/arm/include/asm/domain.h | 2 + > xen/arch/arm/include/asm/perfc_defn.h | 1 + > xen/arch/arm/include/asm/psci.h | 2 + > xen/arch/arm/include/asm/suspend.h | 27 ++++++ > xen/arch/arm/include/asm/vpsci.h | 5 +- > xen/arch/arm/vpsci.c | 116 +++++++++++++++++++++----- > xen/common/domain.c | 10 +++ > xen/include/xen/suspend.h | 25 ++++++ > 9 files changed, 205 insertions(+), 22 deletions(-) > create mode 100644 xen/arch/arm/include/asm/suspend.h > create mode 100644 xen/include/xen/suspend.h > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 47973f99d9..f903e7d4f0 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -12,6 +12,8 @@ > #include <xen/softirq.h> > #include <xen/wait.h> > > +#include <public/sched.h> > + > #include <asm/arm64/sve.h> > #include <asm/cpuerrata.h> > #include <asm/cpufeature.h> > @@ -24,10 +26,12 @@ > #include <asm/platform.h> > #include <asm/procinfo.h> > #include <asm/regs.h> > +#include <asm/suspend.h> > #include <asm/firmware/sci.h> > #include <asm/tee/tee.h> > #include <asm/vfp.h> > #include <asm/vgic.h> > +#include <asm/vpsci.h> > #include <asm/vtimer.h> > > #include "vpci.h" > @@ -851,6 +855,41 @@ void arch_domain_creation_finished(struct domain *d) > p2m_domain_creation_finished(d); > } > > +int arch_domain_resume(struct domain *d) > +{ > + int rc; > + struct resume_info *ctx = &d->arch.resume_ctx; > + > + if ( !d->is_shutting_down || d->shutdown_code != SHUTDOWN_suspend ) > + { > + dprintk(XENLOG_WARNING, > + "%pd: Invalid domain state for resume: is_shutting_down=%u, > shutdown_code=%u\n", > + d, d->is_shutting_down, d->shutdown_code); > + return -EINVAL; > + } > + > + /* > + * It is still possible to call domain_shutdown() with a suspend reason > + * via some hypercalls, such as SCHEDOP_shutdown or > SCHEDOP_remote_shutdown. > + * In these cases, the resume context will be empty. > + * This is not expected to cause any issues, so we just notify about the > + * situation and return without error, allowing the existing logic to > + * proceed as expected. > + */ > + if ( !ctx->wake_cpu ) > + { > + dprintk(XENLOG_INFO, "%pd: Wake CPU pointer context was not > provided\n", > + d); > + return 0; > + } > + > + rc = vpsci_vcpu_up_prepare(ctx->wake_cpu , ctx->ep, ctx->cid); > + if ( !rc ) > + memset(ctx, 0, sizeof(*ctx)); > + > + return rc; > +} > + > static int is_guest_pv32_psr(uint32_t psr) > { > switch (psr & PSR_MODE_MASK) > diff --git a/xen/arch/arm/include/asm/domain.h > b/xen/arch/arm/include/asm/domain.h > index 758ad807e4..66b1246892 100644 > --- a/xen/arch/arm/include/asm/domain.h > +++ b/xen/arch/arm/include/asm/domain.h > @@ -5,6 +5,7 @@ > #include <xen/timer.h> > #include <asm/page.h> > #include <asm/p2m.h> > +#include <asm/suspend.h> > #include <asm/vfp.h> > #include <asm/mmio.h> > #include <asm/gic.h> > @@ -126,6 +127,7 @@ struct arch_domain > void *sci_data; > #endif > > + struct resume_info resume_ctx; > } __cacheline_aligned; > > struct arch_vcpu > diff --git a/xen/arch/arm/include/asm/perfc_defn.h > b/xen/arch/arm/include/asm/perfc_defn.h > index effd25b69e..8dfcac7e3b 100644 > --- a/xen/arch/arm/include/asm/perfc_defn.h > +++ b/xen/arch/arm/include/asm/perfc_defn.h > @@ -33,6 +33,7 @@ PERFCOUNTER(vpsci_system_reset, "vpsci: > system_reset") > PERFCOUNTER(vpsci_cpu_suspend, "vpsci: cpu_suspend") > PERFCOUNTER(vpsci_cpu_affinity_info, "vpsci: cpu_affinity_info") > PERFCOUNTER(vpsci_features, "vpsci: features") > +PERFCOUNTER(vpsci_system_suspend, "vpsci: system_suspend") > > PERFCOUNTER(vcpu_kick, "vcpu: notify other vcpu") > > diff --git a/xen/arch/arm/include/asm/psci.h b/xen/arch/arm/include/asm/psci.h > index 4780972621..48a93e6b79 100644 > --- a/xen/arch/arm/include/asm/psci.h > +++ b/xen/arch/arm/include/asm/psci.h > @@ -47,10 +47,12 @@ void call_psci_system_reset(void); > #define PSCI_0_2_FN32_SYSTEM_OFF PSCI_0_2_FN32(8) > #define PSCI_0_2_FN32_SYSTEM_RESET PSCI_0_2_FN32(9) > #define PSCI_1_0_FN32_PSCI_FEATURES PSCI_0_2_FN32(10) > +#define PSCI_1_0_FN32_SYSTEM_SUSPEND PSCI_0_2_FN32(14) > > #define PSCI_0_2_FN64_CPU_SUSPEND PSCI_0_2_FN64(1) > #define PSCI_0_2_FN64_CPU_ON PSCI_0_2_FN64(3) > #define PSCI_0_2_FN64_AFFINITY_INFO PSCI_0_2_FN64(4) > +#define PSCI_1_0_FN64_SYSTEM_SUSPEND PSCI_0_2_FN64(14) > > /* PSCI v0.2 affinity level state returned by AFFINITY_INFO */ > #define PSCI_0_2_AFFINITY_LEVEL_ON 0 > diff --git a/xen/arch/arm/include/asm/suspend.h > b/xen/arch/arm/include/asm/suspend.h > new file mode 100644 > index 0000000000..313d03ea59 > --- /dev/null > +++ b/xen/arch/arm/include/asm/suspend.h > @@ -0,0 +1,27 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > + > +#ifndef ARM_SUSPEND_H > +#define ARM_SUSPEND_H > + > +struct domain; > +struct vcpu; > + > +struct resume_info { > + register_t ep; > + register_t cid; > + struct vcpu *wake_cpu; > +}; > + > +int arch_domain_resume(struct domain *d); > + > +#endif /* ARM_SUSPEND_H */ > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/arch/arm/include/asm/vpsci.h > b/xen/arch/arm/include/asm/vpsci.h > index 0cca5e6830..d790ab3715 100644 > --- a/xen/arch/arm/include/asm/vpsci.h > +++ b/xen/arch/arm/include/asm/vpsci.h > @@ -23,12 +23,15 @@ > #include <asm/psci.h> > > /* Number of function implemented by virtual PSCI (only 0.2 or later) */ > -#define VPSCI_NR_FUNCS 12 > +#define VPSCI_NR_FUNCS 14 > > /* Functions handle PSCI calls from the guests */ > bool do_vpsci_0_1_call(struct cpu_user_regs *regs, uint32_t fid); > bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid); > > +int vpsci_vcpu_up_prepare(struct vcpu *v, register_t entry_point, > + register_t context_id); > + > #endif /* __ASM_VPSCI_H__ */ > > /* > diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c > index 7ba9ccd94b..c4d616ec68 100644 > --- a/xen/arch/arm/vpsci.c > +++ b/xen/arch/arm/vpsci.c > @@ -10,32 +10,16 @@ > > #include <public/sched.h> > > -static int do_common_cpu_on(register_t target_cpu, register_t entry_point, > - register_t context_id) > +int vpsci_vcpu_up_prepare(struct vcpu *v, register_t entry_point, > + register_t context_id) > { > - struct vcpu *v; > - struct domain *d = current->domain; > - struct vcpu_guest_context *ctxt; > int rc; > + struct domain *d = v->domain; > bool is_thumb = entry_point & 1; > - register_t vcpuid; > - > - vcpuid = vaffinity_to_vcpuid(target_cpu); > - > - if ( (v = domain_vcpu(d, vcpuid)) == NULL ) > - return PSCI_INVALID_PARAMETERS; > - > - /* THUMB set is not allowed with 64-bit domain */ > - if ( is_64bit_domain(d) && is_thumb ) > - return PSCI_INVALID_ADDRESS; > - > - if ( !test_bit(_VPF_down, &v->pause_flags) ) > - return PSCI_ALREADY_ON; > + struct vcpu_guest_context *ctxt; > > if ( (ctxt = alloc_vcpu_guest_context()) == NULL ) > - return PSCI_DENIED; > - > - vgic_clear_pending_irqs(v); > + return -ENOMEM; > > memset(ctxt, 0, sizeof(*ctxt)); > ctxt->user_regs.pc64 = (u64) entry_point; > @@ -76,8 +60,37 @@ static int do_common_cpu_on(register_t target_cpu, > register_t entry_point, > free_vcpu_guest_context(ctxt); > > if ( rc < 0 ) > + return rc; > + > + return 0; > +} > + > +static int do_common_cpu_on(register_t target_cpu, register_t entry_point, > + register_t context_id) > +{ > + struct vcpu *v; > + struct domain *d = current->domain; > + int rc; > + bool is_thumb = entry_point & 1; > + register_t vcpuid; > + > + vcpuid = vaffinity_to_vcpuid(target_cpu); > + > + if ( (v = domain_vcpu(d, vcpuid)) == NULL ) > + return PSCI_INVALID_PARAMETERS; > + > + /* THUMB set is not allowed with 64-bit domain */ > + if ( is_64bit_domain(d) && is_thumb ) > + return PSCI_INVALID_ADDRESS; > + > + if ( !test_bit(_VPF_down, &v->pause_flags) ) > + return PSCI_ALREADY_ON; > + > + rc = vpsci_vcpu_up_prepare(v, entry_point, context_id); > + if ( rc ) > return PSCI_DENIED; > > + vgic_clear_pending_irqs(v); > vcpu_wake(v); > > return PSCI_SUCCESS; > @@ -197,6 +210,48 @@ static void do_psci_0_2_system_reset(void) > domain_shutdown(d,SHUTDOWN_reboot); > } > > +static int32_t do_psci_1_0_system_suspend(register_t epoint, register_t cid) > +{ > + int32_t rc; > + struct vcpu *v; > + struct domain *d = current->domain; > + bool is_thumb = epoint & 1; > + > + /* THUMB set is not allowed with 64-bit domain */ > + if ( is_64bit_domain(d) && is_thumb ) > + return PSCI_INVALID_ADDRESS; > + > + /* SYSTEM_SUSPEND is not supported for the hardware domain yet */ > + if ( is_hardware_domain(d) ) > + return PSCI_NOT_SUPPORTED; > + > + /* Ensure that all CPUs other than the calling one are offline */ > + domain_lock(d); > + for_each_vcpu ( d, v ) > + { > + if ( v != current && is_vcpu_online(v) ) > + { > + domain_unlock(d); > + return PSCI_DENIED; > + } > + } > + domain_unlock(d); > + > + rc = domain_shutdown(d, SHUTDOWN_suspend); > + if ( rc ) > + return PSCI_DENIED; > + > + d->arch.resume_ctx.ep = epoint; > + d->arch.resume_ctx.cid = cid; > + d->arch.resume_ctx.wake_cpu = current; > + > + gprintk(XENLOG_DEBUG, > + "SYSTEM_SUSPEND requested, epoint=%#"PRIregister", > cid=%#"PRIregister"\n", > + epoint, cid); > + > + return rc; > +} > + > static int32_t do_psci_1_0_features(uint32_t psci_func_id) > { > /* /!\ Ordered by function ID and not name */ > @@ -214,6 +269,8 @@ static int32_t do_psci_1_0_features(uint32_t psci_func_id) > case PSCI_0_2_FN32_SYSTEM_OFF: > case PSCI_0_2_FN32_SYSTEM_RESET: > case PSCI_1_0_FN32_PSCI_FEATURES: > + case PSCI_1_0_FN32_SYSTEM_SUSPEND: > + case PSCI_1_0_FN64_SYSTEM_SUSPEND: > case ARM_SMCCC_VERSION_FID: > return 0; > default: > @@ -344,6 +401,23 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, > uint32_t fid) > return true; > } > > + case PSCI_1_0_FN32_SYSTEM_SUSPEND: > + case PSCI_1_0_FN64_SYSTEM_SUSPEND: > + { > + register_t epoint = PSCI_ARG(regs, 1); > + register_t cid = PSCI_ARG(regs, 2); > + > + if ( fid == PSCI_1_0_FN32_SYSTEM_SUSPEND ) > + { > + epoint &= GENMASK(31, 0); > + cid &= GENMASK(31, 0); > + } > + > + perfc_incr(vpsci_system_suspend); > + PSCI_SET_RESULT(regs, do_psci_1_0_system_suspend(epoint, cid)); > + return true; > + } > + > default: > return false; > } > diff --git a/xen/common/domain.c b/xen/common/domain.c > index 93c71bc766..09ad0a26ee 100644 > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -26,6 +26,7 @@ > #include <xen/hypercall.h> > #include <xen/delay.h> > #include <xen/shutdown.h> > +#include <xen/suspend.h> > #include <xen/percpu.h> > #include <xen/multicall.h> > #include <xen/rcupdate.h> > @@ -1374,6 +1375,7 @@ int domain_shutdown(struct domain *d, u8 reason) > void domain_resume(struct domain *d) > { > struct vcpu *v; > + int rc; > > /* > * Some code paths assume that shutdown status does not get reset under > @@ -1383,6 +1385,13 @@ void domain_resume(struct domain *d) > > spin_lock(&d->shutdown_lock); > > + rc = arch_domain_resume(d); > + if ( rc ) > + { > + printk("%pd: Failed to resume domain (ret %d)\n", d, rc); > + goto fail; > + } > + > d->is_shutting_down = d->is_shut_down = 0; > d->shutdown_code = SHUTDOWN_CODE_INVALID; > > @@ -1393,6 +1402,7 @@ void domain_resume(struct domain *d) > v->paused_for_shutdown = 0; > } > > + fail: > spin_unlock(&d->shutdown_lock); > > domain_unpause(d); > diff --git a/xen/include/xen/suspend.h b/xen/include/xen/suspend.h > new file mode 100644 > index 0000000000..528879c2a9 > --- /dev/null > +++ b/xen/include/xen/suspend.h > @@ -0,0 +1,25 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > + > +#ifndef XEN_SUSPEND_H > +#define XEN_SUSPEND_H > + > +#if __has_include(<asm/suspend.h>) > +#include <asm/suspend.h> > +#else > +static inline int arch_domain_resume(struct domain *d) > +{ > + return 0; > +} > +#endif > + > +#endif /* XEN_SUSPEND_H */ > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */ > -- > 2.43.0 >
