From: Mirela Simonovic <mirela.simonovic@aggios.com>
Trigger Xen suspend when the control domain initiates suspend via
SHUTDOWN_suspend. Redirect system suspend to CPU#0 to ensure the
suspend logic runs on the boot CPU, as required.
Introduce full suspend/resume infrastructure gated by CONFIG_SYSTEM_SUSPEND,
including logic to:
- disable and enable non-boot physical CPUs
- freeze and thaw domains
- suspend and resume the GIC, timer, iommu and console
- maintain system state before and after suspend
On boot, init_ttbr is normally initialized during secondary CPU hotplug.
On uniprocessor systems, this would leave init_ttbr uninitialized,
causing resume to fail. To address this, the boot CPU now sets init_ttbr
during suspend.
Select HAS_SYSTEM_SUSPEND for ARM_64.
Introduce CONFIG_HWDOM_SHUTDOWN_ON_SUSPEND (default y, disabled only on ARM_64
with SYSTEM_SUSPEND) so we skip hwdom_shutdown() for SHUTDOWN_suspend when the
hardware domain survives suspend. Non-ARM behaviour is unchanged, and the
ARM/system-suspend case no longer needs arch-specific checks in the code.
Note: the code is guarded by CONFIG_SYSTEM_SUSPEND, which is currently only
selected when UNSUPPORTED is set, and thus the functionality is neither enabled
by default nor even built.
Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>
Signed-off-by: Saeed Nowshadi <saeed.nowshadi@xilinx.com>
Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
---
Changes in V7:
- Control domain is responsible for host suspend
- Add an empty inline host_system_suspend() function when SYSTEM_SUSPEND
config is disabled
- Use IS_ENABLED() for config checking instead of #ifdef
- Replace #ifdef checks in domain_shutdown() with IS_ENABLED() to simplify
control flow.
- Factor hardware domain shutdown condition into a helper
(need_hwdom_shutdown()) to avoid preprocessor directives inside the function.
- Squash with iommu suspend/resume commit
---
xen/arch/arm/Kconfig | 1 +
xen/arch/arm/include/asm/mm.h | 2 +
xen/arch/arm/include/asm/suspend.h | 7 +-
xen/arch/arm/mmu/smpboot.c | 2 +-
xen/arch/arm/suspend.c | 139 +++++++++++++++++++++++++++++
xen/arch/arm/vpsci.c | 12 ++-
xen/common/Kconfig | 5 ++
xen/common/domain.c | 7 +-
xen/drivers/passthrough/arm/smmu.c | 10 +++
9 files changed, 180 insertions(+), 5 deletions(-)
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index cf6af68299..86fa0b762a 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -8,6 +8,7 @@ config ARM_64
depends on !ARM_32
select 64BIT
select HAS_FAST_MULTIPLY
+ select HAS_SYSTEM_SUSPEND if !MPU && UNSUPPORTED
select HAS_VPCI_GUEST_SUPPORT if PCI_PASSTHROUGH
config ARM
diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
index ec2d2dc537..e4e795fa1f 100644
--- a/xen/arch/arm/include/asm/mm.h
+++ b/xen/arch/arm/include/asm/mm.h
@@ -364,6 +364,8 @@ static inline void page_set_xenheap_gfn(struct page_info *p, gfn_t gfn)
} while ( (y = cmpxchg(&p->u.inuse.type_info, x, nx)) != x );
}
+void set_init_ttbr(lpae_t *root);
+
#endif /* __ARCH_ARM_MM__ */
/*
* Local variables:
diff --git a/xen/arch/arm/include/asm/suspend.h b/xen/arch/arm/include/asm/suspend.h
index d3a88ea979..2fb7cd8d56 100644
--- a/xen/arch/arm/include/asm/suspend.h
+++ b/xen/arch/arm/include/asm/suspend.h
@@ -38,7 +38,12 @@ extern struct cpu_context cpu_context;
int prepare_resume_ctx(struct cpu_context *ptr);
void hyp_resume(void);
-#endif /* CONFIG_SYSTEM_SUSPEND */
+void host_system_suspend(struct domain *d);
+
+#else /* !CONFIG_SYSTEM_SUSPEND */
+
+static inline void host_system_suspend(struct domain *d) { (void)d; }
+#endif
#endif /* ARM_SUSPEND_H */
diff --git a/xen/arch/arm/mmu/smpboot.c b/xen/arch/arm/mmu/smpboot.c
index 37e91d72b7..ff508ecf40 100644
--- a/xen/arch/arm/mmu/smpboot.c
+++ b/xen/arch/arm/mmu/smpboot.c
@@ -72,7 +72,7 @@ static void clear_boot_pagetables(void)
clear_table(boot_third);
}
-static void set_init_ttbr(lpae_t *root)
+void set_init_ttbr(lpae_t *root)
{
/*
* init_ttbr is part of the identity mapping which is read-only. So
diff --git a/xen/arch/arm/suspend.c b/xen/arch/arm/suspend.c
index e38566b0b7..c34b578e18 100644
--- a/xen/arch/arm/suspend.c
+++ b/xen/arch/arm/suspend.c
@@ -1,9 +1,148 @@
/* SPDX-License-Identifier: GPL-2.0-only */
+#include <asm/psci.h>
#include <asm/suspend.h>
+#include <xen/console.h>
+#include <xen/cpu.h>
+#include <xen/iommu.h>
+#include <xen/llc-coloring.h>
+#include <xen/sched.h>
+#include <xen/tasklet.h>
+
struct cpu_context cpu_context = {};
+/* Xen suspend. data identifies the domain that initiated suspend. */
+static void system_suspend(void *data)
+{
+ int status;
+ unsigned long flags;
+ struct domain *d = (struct domain *)data;
+
+ BUG_ON(system_state != SYS_STATE_active);
+
+ system_state = SYS_STATE_suspend;
+
+ printk("Xen suspending...\n");
+
+ freeze_domains();
+ scheduler_disable();
+
+ /*
+ * Non-boot CPUs have to be disabled on suspend and enabled on resume
+ * (hotplug-based mechanism). Disabling non-boot CPUs will lead to PSCI
+ * CPU_OFF to be called by each non-boot CPU. Depending on the underlying
+ * platform capabilities, this may lead to the physical powering down of
+ * CPUs.
+ */
+ status = disable_nonboot_cpus();
+ if ( status )
+ {
+ system_state = SYS_STATE_resume;
+ goto resume_nonboot_cpus;
+ }
+
+ time_suspend();
+
+ status = iommu_suspend();
+ if ( status )
+ {
+ system_state = SYS_STATE_resume;
+ goto resume_time;
+ }
+
+ console_start_sync();
+ status = console_suspend();
+ if ( status )
+ {
+ dprintk(XENLOG_ERR, "Failed to suspend the console, err=%d\n", status);
+ system_state = SYS_STATE_resume;
+ goto resume_end_sync;
+ }
+
+ local_irq_save(flags);
+ status = gic_suspend();
+ if ( status )
+ {
+ system_state = SYS_STATE_resume;
+ goto resume_irqs;
+ }
+
+ set_init_ttbr(xen_pgtable);
+
+ /*
+ * Enable identity mapping before entering suspend to simplify
+ * the resume path
+ */
+ update_boot_mapping(true);
+
+ if ( prepare_resume_ctx(&cpu_context) )
+ {
+ status = call_psci_system_suspend();
+ /*
+ * If suspend is finalized properly by above system suspend PSCI call,
+ * the code below in this 'if' branch will never execute. Execution
+ * will continue from hyp_resume which is the hypervisor's resume point.
+ * In hyp_resume CPU context will be restored and since link-register is
+ * restored as well, it will appear to return from prepare_resume_ctx.
+ * The difference in returning from prepare_resume_ctx on system suspend
+ * versus resume is in function's return value: on suspend, the return
+ * value is a non-zero value, on resume it is zero. That is why the
+ * control flow will not re-enter this 'if' branch on resume.
+ */
+ if ( status )
+ dprintk(XENLOG_WARNING, "PSCI system suspend failed, err=%d\n",
+ status);
+ }
+
+ system_state = SYS_STATE_resume;
+ update_boot_mapping(false);
+
+ gic_resume();
+
+ resume_irqs:
+ local_irq_restore(flags);
+
+ console_resume();
+ resume_end_sync:
+ console_end_sync();
+
+ iommu_resume();
+
+ resume_time:
+ time_resume();
+
+ resume_nonboot_cpus:
+ /*
+ * The rcu_barrier() has to be added to ensure that the per cpu area is
+ * freed before a non-boot CPU tries to initialize it (_free_percpu_area()
+ * has to be called before the init_percpu_area()). This scenario occurs
+ * when non-boot CPUs are hot-unplugged on suspend and hotplugged on resume.
+ */
+ rcu_barrier();
+ enable_nonboot_cpus();
+ scheduler_enable();
+ thaw_domains();
+
+ system_state = SYS_STATE_active;
+
+ printk("Resume (status %d)\n", status);
+
+ domain_resume(d);
+}
+
+static DECLARE_TASKLET(system_suspend_tasklet, system_suspend, NULL);
+
+void host_system_suspend(struct domain *d)
+{
+ system_suspend_tasklet.data = (void *)d;
+ /*
+ * The suspend procedure has to be finalized by the pCPU#0 (non-boot pCPUs
+ * will be disabled during the suspend).
+ */
+ tasklet_schedule_on_cpu(&system_suspend_tasklet, 0);
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
index c4d616ec68..891d9f2cb9 100644
--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -4,6 +4,7 @@
#include <xen/types.h>
#include <asm/current.h>
+#include <asm/suspend.h>
#include <asm/vgic.h>
#include <asm/vpsci.h>
#include <asm/event.h>
@@ -210,6 +211,11 @@ static void do_psci_0_2_system_reset(void)
domain_shutdown(d,SHUTDOWN_reboot);
}
+static bool can_trigger_host_suspend(struct domain *d)
+{
+ return is_control_domain(d);
+}
+
static int32_t do_psci_1_0_system_suspend(register_t epoint, register_t cid)
{
int32_t rc;
@@ -221,8 +227,7 @@ static int32_t do_psci_1_0_system_suspend(register_t epoint, register_t cid)
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) )
+ if ( !IS_ENABLED(CONFIG_SYSTEM_SUSPEND) && can_trigger_host_suspend(d) )
return PSCI_NOT_SUPPORTED;
/* Ensure that all CPUs other than the calling one are offline */
@@ -249,6 +254,9 @@ static int32_t do_psci_1_0_system_suspend(register_t epoint, register_t cid)
"SYSTEM_SUSPEND requested, epoint=%#"PRIregister", cid=%#"PRIregister"\n",
epoint, cid);
+ if ( can_trigger_host_suspend(d) )
+ host_system_suspend(d);
+
return rc;
}
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 401d5046f6..31f54def0b 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -137,6 +137,11 @@ config HAS_EX_TABLE
config HAS_FAST_MULTIPLY
bool
+config HAS_HWDOM_SHUTDOWN_ON_SUSPEND
+ bool
+ default y
+ depends on !ARM_64 || !SYSTEM_SUSPEND
+
config HAS_IOPORTS
bool
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 8f895108fd..b8a8c68428 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1324,6 +1324,11 @@ void __domain_crash(struct domain *d)
domain_shutdown(d, SHUTDOWN_crash);
}
+static inline bool need_hwdom_shutdown(uint8_t reason)
+{
+ return IS_ENABLED(CONFIG_HAS_HWDOM_SHUTDOWN_ON_SUSPEND) ||
+ reason != SHUTDOWN_suspend;
+}
int domain_shutdown(struct domain *d, u8 reason)
{
@@ -1340,7 +1345,7 @@ int domain_shutdown(struct domain *d, u8 reason)
d->shutdown_code = reason;
reason = d->shutdown_code;
- if ( is_hardware_domain(d) )
+ if ( is_hardware_domain(d) && need_hwdom_shutdown(reason) )
hwdom_shutdown(reason);
if ( d->is_shutting_down )
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index 22d306d0cb..45f29ef8ec 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2947,6 +2947,13 @@ static void arm_smmu_iommu_domain_teardown(struct domain *d)
xfree(xen_domain);
}
+#ifdef CONFIG_SYSTEM_SUSPEND
+static int arm_smmu_suspend(void)
+{
+ return -ENOSYS;
+}
+#endif
+
static const struct iommu_ops arm_smmu_iommu_ops = {
.page_sizes = PAGE_SIZE_4K,
.init = arm_smmu_iommu_domain_init,
@@ -2960,6 +2967,9 @@ static const struct iommu_ops arm_smmu_iommu_ops = {
.map_page = arm_iommu_map_page,
.unmap_page = arm_iommu_unmap_page,
.dt_xlate = arm_smmu_dt_xlate_generic,
+#ifdef CONFIG_SYSTEM_SUSPEND
+ .suspend = arm_smmu_suspend,
+#endif
};
static struct arm_smmu_device *find_smmu(const struct device *dev)
--
2.43.0
On 11.12.2025 19:43, Mykola Kvach wrote:
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -137,6 +137,11 @@ config HAS_EX_TABLE
> config HAS_FAST_MULTIPLY
> bool
>
> +config HAS_HWDOM_SHUTDOWN_ON_SUSPEND
> + bool
> + default y
> + depends on !ARM_64 || !SYSTEM_SUSPEND
As written, this would want to be "def_bool y". However, I think I indicated
previously that imo it would be nice if we could stop adding more "depends on"
referencing particular architectures. Instead "select" or "imply" from
xen/arch/<arch>/Kconfig appears more desirable to use in such cases. That way
each arch can control what it wants without needing to touch common code.
As an aside, in the context of PV_SHIM_EXCLUSIVE it was also said several
times that negative dependencies aren't very nice to have. Here we have no
prompt, so the "allyesconfig" concern doesn't apply, but I still thought I'd
mention this.
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1324,6 +1324,11 @@ void __domain_crash(struct domain *d)
> domain_shutdown(d, SHUTDOWN_crash);
> }
>
> +static inline bool need_hwdom_shutdown(uint8_t reason)
Personally I think "want" would better express things, but I don't really
mind "need".
> +{
> + return IS_ENABLED(CONFIG_HAS_HWDOM_SHUTDOWN_ON_SUSPEND) ||
> + reason != SHUTDOWN_suspend;
> +}
Seeing this in use, I wonder if HAS_ is really suitable here.
Jan
Hi Jan,
On Mon, Dec 15, 2025 at 1:49 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 11.12.2025 19:43, Mykola Kvach wrote:
> > --- a/xen/common/Kconfig
> > +++ b/xen/common/Kconfig
> > @@ -137,6 +137,11 @@ config HAS_EX_TABLE
> > config HAS_FAST_MULTIPLY
> > bool
> >
> > +config HAS_HWDOM_SHUTDOWN_ON_SUSPEND
> > + bool
> > + default y
> > + depends on !ARM_64 || !SYSTEM_SUSPEND
>
> As written, this would want to be "def_bool y". However, I think I indicated
OK, I’ll switch this to def_bool.
> previously that imo it would be nice if we could stop adding more "depends on"
> referencing particular architectures. Instead "select" or "imply" from
> xen/arch/<arch>/Kconfig appears more desirable to use in such cases. That way
> each arch can control what it wants without needing to touch common code.
>
> As an aside, in the context of PV_SHIM_EXCLUSIVE it was also said several
> times that negative dependencies aren't very nice to have. Here we have no
> prompt, so the "allyesconfig" concern doesn't apply, but I still thought I'd
> mention this.
I used the common-level dependency only to avoid adding selects in every
other arch Kconfig, as the only exception I need is
ARM_64 && SYSTEM_SUSPEND.
If you still prefer keeping all arch-specific handling under
xen/arch/<arch>/Kconfig, I can rework it accordingly.
>
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -1324,6 +1324,11 @@ void __domain_crash(struct domain *d)
> > domain_shutdown(d, SHUTDOWN_crash);
> > }
> >
> > +static inline bool need_hwdom_shutdown(uint8_t reason)
>
> Personally I think "want" would better express things, but I don't really
> mind "need".
I'll change it to "want".
>
> > +{
> > + return IS_ENABLED(CONFIG_HAS_HWDOM_SHUTDOWN_ON_SUSPEND) ||
> > + reason != SHUTDOWN_suspend;
> > +}
>
> Seeing this in use, I wonder if HAS_ is really suitable here.
What name would you consider more suitable here?
Best regards,
Mykola
>
> Jan
On 15.01.2026 11:19, Mykola Kvach wrote:
> On Mon, Dec 15, 2025 at 1:49 PM Jan Beulich <jbeulich@suse.com> wrote:
>> On 11.12.2025 19:43, Mykola Kvach wrote:
>>> --- a/xen/common/Kconfig
>>> +++ b/xen/common/Kconfig
>>> @@ -137,6 +137,11 @@ config HAS_EX_TABLE
>>> config HAS_FAST_MULTIPLY
>>> bool
>>>
>>> +config HAS_HWDOM_SHUTDOWN_ON_SUSPEND
>>> + bool
>>> + default y
>>> + depends on !ARM_64 || !SYSTEM_SUSPEND
>>
>> As written, this would want to be "def_bool y". However, I think I indicated
>
> OK, I’ll switch this to def_bool.
>
>> previously that imo it would be nice if we could stop adding more "depends on"
>> referencing particular architectures. Instead "select" or "imply" from
>> xen/arch/<arch>/Kconfig appears more desirable to use in such cases. That way
>> each arch can control what it wants without needing to touch common code.
>>
>> As an aside, in the context of PV_SHIM_EXCLUSIVE it was also said several
>> times that negative dependencies aren't very nice to have. Here we have no
>> prompt, so the "allyesconfig" concern doesn't apply, but I still thought I'd
>> mention this.
>
> I used the common-level dependency only to avoid adding selects in every
> other arch Kconfig, as the only exception I need is
> ARM_64 && SYSTEM_SUSPEND.
>
> If you still prefer keeping all arch-specific handling under
> xen/arch/<arch>/Kconfig, I can rework it accordingly.
Imo there are two options: Do as you suggest, but with an option not starting
HAS_*. Or use HAS_ with per-arch selects (which I think I'd prefer).
To limit the number of selects needed, perhaps the sense of the option may
want inverting? Otoh I don't think we know yet what RISC-V and PPC are going
to want?
>>> --- a/xen/common/domain.c
>>> +++ b/xen/common/domain.c
>>> @@ -1324,6 +1324,11 @@ void __domain_crash(struct domain *d)
>>> domain_shutdown(d, SHUTDOWN_crash);
>>> }
>>>
>>> +static inline bool need_hwdom_shutdown(uint8_t reason)
>>
>> Personally I think "want" would better express things, but I don't really
>> mind "need".
>
> I'll change it to "want".
>
>>
>>> +{
>>> + return IS_ENABLED(CONFIG_HAS_HWDOM_SHUTDOWN_ON_SUSPEND) ||
>>> + reason != SHUTDOWN_suspend;
>>> +}
>>
>> Seeing this in use, I wonder if HAS_ is really suitable here.
>
> What name would you consider more suitable here?
As per above, HAS_ dropped would be an option. Yet that goes against my
preference above. Maybe HAS_ really is okay-ish here, despite what I said
earlier.
Jan
© 2016 - 2026 Red Hat, Inc.