From: Mirela Simonovic <mirela.simonovic@aggios.com>

Trigger Xen suspend when the hardware 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 and console
 - maintain system state before and after suspend

Remove the restriction in the PSCI interface preventing suspend from the
hardware domain.

Select HAS_SYSTEM_SUSPEND for ARM_64.

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 v5:
- select HAS_SYSTEM_SUSPEND in ARM_64 instead of ARM
- check llc_coloring_enabled instead of LLC_COLORING during the selection
  of HAS_SYSTEM_SUSPEND config
- call host_system_suspend from guest PSCI system suspend instead of
  arch_domain_shutdown, reducing the complexity of the new code
- update some comments

Changes introduced in V4:
  - drop code for saving and restoring VCPU context (for more information
    refer part 1 of patch series)
  - remove IOMMU suspend and resume calls until they will be implemented
  - move system suspend logic to arch_domain_shutdown, invoked from
    domain_shutdown
  - apply minor fixes such as renaming functions, updating comments, and
    modifying the commit message to reflect these changes
  - add console_end_sync to resume path after system suspend

Changes introduced in V3:
  - merge changes from other commits into this patch (previously stashed):
    1) disable/enable non-boot physical CPUs and freeze/thaw domains during
       suspend/resume
    2) suspend/resume the timer, GIC, console, IOMMU, and hardware domain
---
 xen/arch/arm/Kconfig               |   1 +
 xen/arch/arm/Makefile              |   1 +
 xen/arch/arm/include/asm/suspend.h |  22 +++++
 xen/arch/arm/suspend.c             | 151 +++++++++++++++++++++++++++++
 xen/arch/arm/vpsci.c               |  12 ++-
 xen/common/domain.c                |   4 +
 6 files changed, 190 insertions(+), 1 deletion(-)
 create mode 100644 xen/arch/arm/include/asm/suspend.h
 create mode 100644 xen/arch/arm/suspend.c

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index a0c8160474..ccdbaa5bc3 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 UNSUPPORTED
 	select HAS_VPCI_GUEST_SUPPORT if PCI_PASSTHROUGH
 
 config ARM
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index f833cdf207..3f6247adee 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -51,6 +51,7 @@ obj-y += setup.o
 obj-y += shutdown.o
 obj-y += smp.o
 obj-y += smpboot.o
+obj-$(CONFIG_SYSTEM_SUSPEND) += suspend.o
 obj-$(CONFIG_SYSCTL) += sysctl.o
 obj-y += time.o
 obj-y += traps.o
diff --git a/xen/arch/arm/include/asm/suspend.h b/xen/arch/arm/include/asm/suspend.h
new file mode 100644
index 0000000000..78d0e2383b
--- /dev/null
+++ b/xen/arch/arm/include/asm/suspend.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __ASM_ARM_SUSPEND_H__
+#define __ASM_ARM_SUSPEND_H__
+
+#ifdef CONFIG_SYSTEM_SUSPEND
+
+int host_system_suspend(void);
+
+#endif /* CONFIG_SYSTEM_SUSPEND */
+
+#endif /* __ASM_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/suspend.c b/xen/arch/arm/suspend.c
new file mode 100644
index 0000000000..abf4b928ce
--- /dev/null
+++ b/xen/arch/arm/suspend.c
@@ -0,0 +1,151 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <xen/console.h>
+#include <xen/cpu.h>
+#include <xen/llc-coloring.h>
+#include <xen/sched.h>
+
+/*
+ * TODO list:
+ *  - Test system suspend with LLC_COLORING enabled and verify functionality
+ *  - Implement IOMMU suspend/resume handlers and integrate them
+ *    into the suspend/resume path (IPMMU and SMMU)
+ *  - Enable "xl suspend" support on ARM architecture
+ *  - Properly disable Xen timer watchdog from relevant services
+ *  - Add suspend/resume CI test for ARM (QEMU if feasible)
+ *  - Investigate feasibility and need for implementing system suspend on ARM32
+ */
+
+/* Xen suspend. Note: data is not used (suspend is the suspend to RAM) */
+static long system_suspend(void *data)
+{
+    int status;
+    unsigned long flags;
+
+    BUG_ON(system_state != SYS_STATE_active);
+
+    system_state = SYS_STATE_suspend;
+    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. Tested on Xilinx Zynq Ultrascale+ MPSoC (including power down of
+     * each non-boot CPU).
+     */
+    status = disable_nonboot_cpus();
+    if ( status )
+    {
+        system_state = SYS_STATE_resume;
+        goto resume_nonboot_cpus;
+    }
+
+    time_suspend();
+
+    local_irq_save(flags);
+    status = gic_suspend();
+    if ( status )
+    {
+        system_state = SYS_STATE_resume;
+        goto resume_irqs;
+    }
+
+    printk("Xen suspending...\n");
+
+    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_console;
+    }
+
+    /*
+     * Enable identity mapping before entering suspend to simplify
+     * the resume path
+     */
+    update_boot_mapping(true);
+
+    system_state = SYS_STATE_resume;
+    update_boot_mapping(false);
+
+ resume_console:
+    console_resume();
+    console_end_sync();
+
+    gic_resume();
+
+ resume_irqs:
+    local_irq_restore(flags);
+    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;
+
+    /* The resume of hardware domain should always follow Xen's resume. */
+    domain_resume(hardware_domain);
+
+    printk("Resume (status %d)\n", status);
+    return status;
+}
+
+int host_system_suspend(void)
+{
+    int status;
+
+    /* TODO: drop check after verification that features can work together */
+    if ( llc_coloring_enabled )
+    {
+        dprintk(XENLOG_ERR,
+                "System suspend is not supported with LLC_COLORING enabled\n");
+        return -ENOSYS;
+    }
+
+    /*
+     * system_suspend should be called when Dom0 finalizes the suspend
+     * procedure from its boot core (VCPU#0). However, Dom0's VCPU#0 could
+     * be mapped to any PCPU (this function could be executed by any PCPU).
+     * The suspend procedure has to be finalized by the PCPU#0 (non-boot
+     * PCPUs will be disabled during the suspend).
+     */
+    status = continue_hypercall_on_cpu(0, system_suspend, NULL);
+
+    /*
+     * If an error happened, there is nothing that needs to be done here
+     * because the system_suspend always returns in fully functional state
+     * no matter what the outcome of suspend procedure is. If the system
+     * suspended successfully the function will return 0 after the resume.
+     * Otherwise, if an error is returned it means Xen did not suspended,
+     * but it is still in the same state as if the system_suspend was never
+     * called. We dump a debug message in case of an error for debugging/
+     * logging purpose.
+     */
+    if ( status )
+        dprintk(XENLOG_ERR, "Failed to suspend, errno=%d\n", status);
+
+    return status;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
index 67d369a8a2..757e719ea7 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>
@@ -214,9 +215,10 @@ static int32_t do_psci_1_0_system_suspend(register_t epoint, register_t cid)
     struct vcpu *v;
     struct domain *d = current->domain;
 
-    /* SYSTEM_SUSPEND is not supported for the hardware domain yet */
+#ifndef CONFIG_SYSTEM_SUSPEND
     if ( is_hardware_domain(d) )
         return PSCI_NOT_SUPPORTED;
+#endif
 
     /* Ensure that all CPUs other than the calling one are offline */
     domain_lock(d);
@@ -234,6 +236,14 @@ static int32_t do_psci_1_0_system_suspend(register_t epoint, register_t cid)
     if ( rc )
         return PSCI_DENIED;
 
+#ifdef CONFIG_SYSTEM_SUSPEND
+    if ( is_hardware_domain(d) && host_system_suspend() )
+    {
+        domain_resume_nopause(d);
+        return PSCI_DENIED;
+    }
+#endif
+
     rc = do_setup_vcpu_ctx(current, epoint, cid);
     if ( rc != PSCI_SUCCESS )
     {
diff --git a/xen/common/domain.c b/xen/common/domain.c
index c3609b0cb0..414a691242 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1311,7 +1311,11 @@ int domain_shutdown(struct domain *d, u8 reason)
         d->shutdown_code = reason;
     reason = d->shutdown_code;
 
+#if defined(CONFIG_SYSTEM_SUSPEND) && defined(CONFIG_ARM)
+    if ( reason != SHUTDOWN_suspend && is_hardware_domain(d) )
+#else
     if ( is_hardware_domain(d) )
+#endif
         hwdom_shutdown(reason);
 
     if ( d->is_shutting_down )
-- 
2.48.1

Hi Mykola,

Mykola Kvach <xakep.amatop@gmail.com> writes:

> From: Mirela Simonovic <mirela.simonovic@aggios.com>
>
> Trigger Xen suspend when the hardware 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 and console
>  - maintain system state before and after suspend
>
> Remove the restriction in the PSCI interface preventing suspend from the
> hardware domain.
>
> Select HAS_SYSTEM_SUSPEND for ARM_64.
>
> 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 v5:
> - select HAS_SYSTEM_SUSPEND in ARM_64 instead of ARM
> - check llc_coloring_enabled instead of LLC_COLORING during the selection
>   of HAS_SYSTEM_SUSPEND config
> - call host_system_suspend from guest PSCI system suspend instead of
>   arch_domain_shutdown, reducing the complexity of the new code
> - update some comments
>
> Changes introduced in V4:
>   - drop code for saving and restoring VCPU context (for more information
>     refer part 1 of patch series)
>   - remove IOMMU suspend and resume calls until they will be implemented
>   - move system suspend logic to arch_domain_shutdown, invoked from
>     domain_shutdown
>   - apply minor fixes such as renaming functions, updating comments, and
>     modifying the commit message to reflect these changes
>   - add console_end_sync to resume path after system suspend
>
> Changes introduced in V3:
>   - merge changes from other commits into this patch (previously stashed):
>     1) disable/enable non-boot physical CPUs and freeze/thaw domains during
>        suspend/resume
>     2) suspend/resume the timer, GIC, console, IOMMU, and hardware domain
> ---
>  xen/arch/arm/Kconfig               |   1 +
>  xen/arch/arm/Makefile              |   1 +
>  xen/arch/arm/include/asm/suspend.h |  22 +++++
>  xen/arch/arm/suspend.c             | 151 +++++++++++++++++++++++++++++
>  xen/arch/arm/vpsci.c               |  12 ++-
>  xen/common/domain.c                |   4 +
>  6 files changed, 190 insertions(+), 1 deletion(-)
>  create mode 100644 xen/arch/arm/include/asm/suspend.h
>  create mode 100644 xen/arch/arm/suspend.c
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index a0c8160474..ccdbaa5bc3 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 UNSUPPORTED
>  	select HAS_VPCI_GUEST_SUPPORT if PCI_PASSTHROUGH
>  
>  config ARM
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index f833cdf207..3f6247adee 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -51,6 +51,7 @@ obj-y += setup.o
>  obj-y += shutdown.o
>  obj-y += smp.o
>  obj-y += smpboot.o
> +obj-$(CONFIG_SYSTEM_SUSPEND) += suspend.o
>  obj-$(CONFIG_SYSCTL) += sysctl.o
>  obj-y += time.o
>  obj-y += traps.o
> diff --git a/xen/arch/arm/include/asm/suspend.h b/xen/arch/arm/include/asm/suspend.h
> new file mode 100644
> index 0000000000..78d0e2383b
> --- /dev/null
> +++ b/xen/arch/arm/include/asm/suspend.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __ASM_ARM_SUSPEND_H__
> +#define __ASM_ARM_SUSPEND_H__
> +
> +#ifdef CONFIG_SYSTEM_SUSPEND
> +
> +int host_system_suspend(void);
> +
> +#endif /* CONFIG_SYSTEM_SUSPEND */
> +
> +#endif /* __ASM_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/suspend.c b/xen/arch/arm/suspend.c
> new file mode 100644
> index 0000000000..abf4b928ce
> --- /dev/null
> +++ b/xen/arch/arm/suspend.c
> @@ -0,0 +1,151 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/console.h>
> +#include <xen/cpu.h>
> +#include <xen/llc-coloring.h>
> +#include <xen/sched.h>
> +
> +/*
> + * TODO list:
> + *  - Test system suspend with LLC_COLORING enabled and verify functionality
> + *  - Implement IOMMU suspend/resume handlers and integrate them
> + *    into the suspend/resume path (IPMMU and SMMU)
> + *  - Enable "xl suspend" support on ARM architecture
> + *  - Properly disable Xen timer watchdog from relevant services
> + *  - Add suspend/resume CI test for ARM (QEMU if feasible)
> + *  - Investigate feasibility and need for implementing system suspend on ARM32
> + */
> +
> +/* Xen suspend. Note: data is not used (suspend is the suspend to RAM) */
> +static long system_suspend(void *data)
> +{
> +    int status;
> +    unsigned long flags;
> +
> +    BUG_ON(system_state != SYS_STATE_active);
> +
> +    system_state = SYS_STATE_suspend;
> +    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. Tested on Xilinx Zynq Ultrascale+ MPSoC (including power down of
> +     * each non-boot CPU).

I don't think that the last part of the comment is relevant in upstream.

> +     */
> +    status = disable_nonboot_cpus();
> +    if ( status )
> +    {
> +        system_state = SYS_STATE_resume;
> +        goto resume_nonboot_cpus;
> +    }
> +
> +    time_suspend();
> +
> +    local_irq_save(flags);
> +    status = gic_suspend();
> +    if ( status )
> +    {
> +        system_state = SYS_STATE_resume;
> +        goto resume_irqs;
> +    }
> +
> +    printk("Xen suspending...\n");
> +
> +    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_console;
> +    }
> +
> +    /*
> +     * Enable identity mapping before entering suspend to simplify
> +     * the resume path
> +     */
> +    update_boot_mapping(true);
> +

This puzzles me. I expected actually PSCI suspend call here.

> +    system_state = SYS_STATE_resume;
> +    update_boot_mapping(false);
> +
> + resume_console:
> +    console_resume();
> +    console_end_sync();
> +
> +    gic_resume();
> +
> + resume_irqs:
> +    local_irq_restore(flags);
> +    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;
> +
> +    /* The resume of hardware domain should always follow Xen's resume. */
> +    domain_resume(hardware_domain);
> +
> +    printk("Resume (status %d)\n", status);
> +    return status;
> +}
> +
> +int host_system_suspend(void)
> +{
> +    int status;
> +
> +    /* TODO: drop check after verification that features can work together */
> +    if ( llc_coloring_enabled )
> +    {
> +        dprintk(XENLOG_ERR,
> +                "System suspend is not supported with LLC_COLORING enabled\n");
> +        return -ENOSYS;
> +    }
> +
> +    /*
> +     * system_suspend should be called when Dom0 finalizes the suspend
> +     * procedure from its boot core (VCPU#0). However, Dom0's VCPU#0 could
> +     * be mapped to any PCPU (this function could be executed by any PCPU).
> +     * The suspend procedure has to be finalized by the PCPU#0 (non-boot
> +     * PCPUs will be disabled during the suspend).
> +     */
> +    status = continue_hypercall_on_cpu(0, system_suspend, NULL);
> +
> +    /*
> +     * If an error happened, there is nothing that needs to be done here
> +     * because the system_suspend always returns in fully functional state
> +     * no matter what the outcome of suspend procedure is. If the system
> +     * suspended successfully the function will return 0 after the resume.
> +     * Otherwise, if an error is returned it means Xen did not suspended,
> +     * but it is still in the same state as if the system_suspend was never
> +     * called. We dump a debug message in case of an error for debugging/
> +     * logging purpose.
> +     */
> +    if ( status )
> +        dprintk(XENLOG_ERR, "Failed to suspend, errno=%d\n", status);
> +
> +    return status;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index 67d369a8a2..757e719ea7 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>
> @@ -214,9 +215,10 @@ static int32_t do_psci_1_0_system_suspend(register_t epoint, register_t cid)
>      struct vcpu *v;
>      struct domain *d = current->domain;
>  
> -    /* SYSTEM_SUSPEND is not supported for the hardware domain yet */
> +#ifndef CONFIG_SYSTEM_SUSPEND
>      if ( is_hardware_domain(d) )
>          return PSCI_NOT_SUPPORTED;
> +#endif
>  
>      /* Ensure that all CPUs other than the calling one are offline */
>      domain_lock(d);
> @@ -234,6 +236,14 @@ static int32_t do_psci_1_0_system_suspend(register_t epoint, register_t cid)
>      if ( rc )
>          return PSCI_DENIED;
>  
> +#ifdef CONFIG_SYSTEM_SUSPEND
> +    if ( is_hardware_domain(d) && host_system_suspend() )
> +    {
> +        domain_resume_nopause(d);
> +        return PSCI_DENIED;
> +    }
> +#endif
> +
>      rc = do_setup_vcpu_ctx(current, epoint, cid);
>      if ( rc != PSCI_SUCCESS )
>      {
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index c3609b0cb0..414a691242 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1311,7 +1311,11 @@ int domain_shutdown(struct domain *d, u8 reason)
>          d->shutdown_code = reason;
>      reason = d->shutdown_code;
>  
> +#if defined(CONFIG_SYSTEM_SUSPEND) && defined(CONFIG_ARM)
> +    if ( reason != SHUTDOWN_suspend && is_hardware_domain(d) )
> +#else
>      if ( is_hardware_domain(d) )
> +#endif
>          hwdom_shutdown(reason);
>  
>      if ( d->is_shutting_down )

-- 
WBR, Volodymyr

Hi Volodymyr,

On Sat, Aug 23, 2025 at 4:00 AM Volodymyr Babchuk
<Volodymyr_Babchuk@epam.com> wrote:
>
> Hi Mykola,
>
> Mykola Kvach <xakep.amatop@gmail.com> writes:
>
> > From: Mirela Simonovic <mirela.simonovic@aggios.com>
> >
> > Trigger Xen suspend when the hardware 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 and console
> >  - maintain system state before and after suspend
> >
> > Remove the restriction in the PSCI interface preventing suspend from the
> > hardware domain.
> >
> > Select HAS_SYSTEM_SUSPEND for ARM_64.
> >
> > 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 v5:
> > - select HAS_SYSTEM_SUSPEND in ARM_64 instead of ARM
> > - check llc_coloring_enabled instead of LLC_COLORING during the selection
> >   of HAS_SYSTEM_SUSPEND config
> > - call host_system_suspend from guest PSCI system suspend instead of
> >   arch_domain_shutdown, reducing the complexity of the new code
> > - update some comments
> >
> > Changes introduced in V4:
> >   - drop code for saving and restoring VCPU context (for more information
> >     refer part 1 of patch series)
> >   - remove IOMMU suspend and resume calls until they will be implemented
> >   - move system suspend logic to arch_domain_shutdown, invoked from
> >     domain_shutdown
> >   - apply minor fixes such as renaming functions, updating comments, and
> >     modifying the commit message to reflect these changes
> >   - add console_end_sync to resume path after system suspend
> >
> > Changes introduced in V3:
> >   - merge changes from other commits into this patch (previously stashed):
> >     1) disable/enable non-boot physical CPUs and freeze/thaw domains during
> >        suspend/resume
> >     2) suspend/resume the timer, GIC, console, IOMMU, and hardware domain
> > ---
> >  xen/arch/arm/Kconfig               |   1 +
> >  xen/arch/arm/Makefile              |   1 +
> >  xen/arch/arm/include/asm/suspend.h |  22 +++++
> >  xen/arch/arm/suspend.c             | 151 +++++++++++++++++++++++++++++
> >  xen/arch/arm/vpsci.c               |  12 ++-
> >  xen/common/domain.c                |   4 +
> >  6 files changed, 190 insertions(+), 1 deletion(-)
> >  create mode 100644 xen/arch/arm/include/asm/suspend.h
> >  create mode 100644 xen/arch/arm/suspend.c
> >
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index a0c8160474..ccdbaa5bc3 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 UNSUPPORTED
> >       select HAS_VPCI_GUEST_SUPPORT if PCI_PASSTHROUGH
> >
> >  config ARM
> > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> > index f833cdf207..3f6247adee 100644
> > --- a/xen/arch/arm/Makefile
> > +++ b/xen/arch/arm/Makefile
> > @@ -51,6 +51,7 @@ obj-y += setup.o
> >  obj-y += shutdown.o
> >  obj-y += smp.o
> >  obj-y += smpboot.o
> > +obj-$(CONFIG_SYSTEM_SUSPEND) += suspend.o
> >  obj-$(CONFIG_SYSCTL) += sysctl.o
> >  obj-y += time.o
> >  obj-y += traps.o
> > diff --git a/xen/arch/arm/include/asm/suspend.h b/xen/arch/arm/include/asm/suspend.h
> > new file mode 100644
> > index 0000000000..78d0e2383b
> > --- /dev/null
> > +++ b/xen/arch/arm/include/asm/suspend.h
> > @@ -0,0 +1,22 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#ifndef __ASM_ARM_SUSPEND_H__
> > +#define __ASM_ARM_SUSPEND_H__
> > +
> > +#ifdef CONFIG_SYSTEM_SUSPEND
> > +
> > +int host_system_suspend(void);
> > +
> > +#endif /* CONFIG_SYSTEM_SUSPEND */
> > +
> > +#endif /* __ASM_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/suspend.c b/xen/arch/arm/suspend.c
> > new file mode 100644
> > index 0000000000..abf4b928ce
> > --- /dev/null
> > +++ b/xen/arch/arm/suspend.c
> > @@ -0,0 +1,151 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#include <xen/console.h>
> > +#include <xen/cpu.h>
> > +#include <xen/llc-coloring.h>
> > +#include <xen/sched.h>
> > +
> > +/*
> > + * TODO list:
> > + *  - Test system suspend with LLC_COLORING enabled and verify functionality
> > + *  - Implement IOMMU suspend/resume handlers and integrate them
> > + *    into the suspend/resume path (IPMMU and SMMU)
> > + *  - Enable "xl suspend" support on ARM architecture
> > + *  - Properly disable Xen timer watchdog from relevant services
> > + *  - Add suspend/resume CI test for ARM (QEMU if feasible)
> > + *  - Investigate feasibility and need for implementing system suspend on ARM32
> > + */
> > +
> > +/* Xen suspend. Note: data is not used (suspend is the suspend to RAM) */
> > +static long system_suspend(void *data)
> > +{
> > +    int status;
> > +    unsigned long flags;
> > +
> > +    BUG_ON(system_state != SYS_STATE_active);
> > +
> > +    system_state = SYS_STATE_suspend;
> > +    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. Tested on Xilinx Zynq Ultrascale+ MPSoC (including power down of
> > +     * each non-boot CPU).
>
> I don't think that the last part of the comment is relevant in upstream.

I’ll drop that part, as it’s not relevant for upstream.

>
> > +     */
> > +    status = disable_nonboot_cpus();
> > +    if ( status )
> > +    {
> > +        system_state = SYS_STATE_resume;
> > +        goto resume_nonboot_cpus;
> > +    }
> > +
> > +    time_suspend();
> > +
> > +    local_irq_save(flags);
> > +    status = gic_suspend();
> > +    if ( status )
> > +    {
> > +        system_state = SYS_STATE_resume;
> > +        goto resume_irqs;
> > +    }
> > +
> > +    printk("Xen suspending...\n");
> > +
> > +    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_console;
> > +    }
> > +
> > +    /*
> > +     * Enable identity mapping before entering suspend to simplify
> > +     * the resume path
> > +     */
> > +    update_boot_mapping(true);
> > +
>
> This puzzles me. I expected actually PSCI suspend call here.
>
> > +    system_state = SYS_STATE_resume;
> > +    update_boot_mapping(false);
> > +
> > + resume_console:
> > +    console_resume();
> > +    console_end_sync();
> > +
> > +    gic_resume();
> > +
> > + resume_irqs:
> > +    local_irq_restore(flags);
> > +    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;
> > +
> > +    /* The resume of hardware domain should always follow Xen's resume. */
> > +    domain_resume(hardware_domain);
> > +
> > +    printk("Resume (status %d)\n", status);
> > +    return status;
> > +}
> > +
> > +int host_system_suspend(void)
> > +{
> > +    int status;
> > +
> > +    /* TODO: drop check after verification that features can work together */
> > +    if ( llc_coloring_enabled )
> > +    {
> > +        dprintk(XENLOG_ERR,
> > +                "System suspend is not supported with LLC_COLORING enabled\n");
> > +        return -ENOSYS;
> > +    }
> > +
> > +    /*
> > +     * system_suspend should be called when Dom0 finalizes the suspend
> > +     * procedure from its boot core (VCPU#0). However, Dom0's VCPU#0 could
> > +     * be mapped to any PCPU (this function could be executed by any PCPU).
> > +     * The suspend procedure has to be finalized by the PCPU#0 (non-boot
> > +     * PCPUs will be disabled during the suspend).
> > +     */
> > +    status = continue_hypercall_on_cpu(0, system_suspend, NULL);
> > +
> > +    /*
> > +     * If an error happened, there is nothing that needs to be done here
> > +     * because the system_suspend always returns in fully functional state
> > +     * no matter what the outcome of suspend procedure is. If the system
> > +     * suspended successfully the function will return 0 after the resume.
> > +     * Otherwise, if an error is returned it means Xen did not suspended,
> > +     * but it is still in the same state as if the system_suspend was never
> > +     * called. We dump a debug message in case of an error for debugging/
> > +     * logging purpose.
> > +     */
> > +    if ( status )
> > +        dprintk(XENLOG_ERR, "Failed to suspend, errno=%d\n", status);
> > +
> > +    return status;
> > +}
> > +
> > +/*
> > + * Local variables:
> > + * mode: C
> > + * c-file-style: "BSD"
> > + * c-basic-offset: 4
> > + * indent-tabs-mode: nil
> > + * End:
> > + */
> > diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> > index 67d369a8a2..757e719ea7 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>
> > @@ -214,9 +215,10 @@ static int32_t do_psci_1_0_system_suspend(register_t epoint, register_t cid)
> >      struct vcpu *v;
> >      struct domain *d = current->domain;
> >
> > -    /* SYSTEM_SUSPEND is not supported for the hardware domain yet */
> > +#ifndef CONFIG_SYSTEM_SUSPEND
> >      if ( is_hardware_domain(d) )
> >          return PSCI_NOT_SUPPORTED;
> > +#endif
> >
> >      /* Ensure that all CPUs other than the calling one are offline */
> >      domain_lock(d);
> > @@ -234,6 +236,14 @@ static int32_t do_psci_1_0_system_suspend(register_t epoint, register_t cid)
> >      if ( rc )
> >          return PSCI_DENIED;
> >
> > +#ifdef CONFIG_SYSTEM_SUSPEND
> > +    if ( is_hardware_domain(d) && host_system_suspend() )
> > +    {
> > +        domain_resume_nopause(d);
> > +        return PSCI_DENIED;
> > +    }
> > +#endif
> > +
> >      rc = do_setup_vcpu_ctx(current, epoint, cid);
> >      if ( rc != PSCI_SUCCESS )
> >      {
> > diff --git a/xen/common/domain.c b/xen/common/domain.c
> > index c3609b0cb0..414a691242 100644
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -1311,7 +1311,11 @@ int domain_shutdown(struct domain *d, u8 reason)
> >          d->shutdown_code = reason;
> >      reason = d->shutdown_code;
> >
> > +#if defined(CONFIG_SYSTEM_SUSPEND) && defined(CONFIG_ARM)
> > +    if ( reason != SHUTDOWN_suspend && is_hardware_domain(d) )
> > +#else
> >      if ( is_hardware_domain(d) )
> > +#endif
> >          hwdom_shutdown(reason);
> >
> >      if ( d->is_shutting_down )
>
> --
> WBR, Volodymyr

Best regards,
Mykola

On 11.08.2025 22:48, Mykola Kvach wrote:
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1311,7 +1311,11 @@ int domain_shutdown(struct domain *d, u8 reason)
>          d->shutdown_code = reason;
>      reason = d->shutdown_code;
>  
> +#if defined(CONFIG_SYSTEM_SUSPEND) && defined(CONFIG_ARM)
> +    if ( reason != SHUTDOWN_suspend && is_hardware_domain(d) )
> +#else
>      if ( is_hardware_domain(d) )
> +#endif
>          hwdom_shutdown(reason);

There better wouldn't be any #ifdef here. Afaict you can easily use
IS_ENABLED(CONFIG_SYSTEM_SUSPEND). For CONFIG_ARM, though, I think some
new abstraction will need adding. E.g. HAS_HWDOM_SUSPEND (with want for
a better, yet not overly long name).

With the hwdom / ctldom separation work in mind I wonder though if it's
really hwdom to be in charge of initiating system suspend. Imo
conceptually that rather would want to be ctldom. Stefano?

Jan

Hi Jan,

Jan Beulich <jbeulich@suse.com> writes:

> On 11.08.2025 22:48, Mykola Kvach wrote:
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -1311,7 +1311,11 @@ int domain_shutdown(struct domain *d, u8 reason)
>>          d->shutdown_code = reason;
>>      reason = d->shutdown_code;
>>  
>> +#if defined(CONFIG_SYSTEM_SUSPEND) && defined(CONFIG_ARM)
>> +    if ( reason != SHUTDOWN_suspend && is_hardware_domain(d) )
>> +#else
>>      if ( is_hardware_domain(d) )
>> +#endif
>>          hwdom_shutdown(reason);
>
> There better wouldn't be any #ifdef here. Afaict you can easily use
> IS_ENABLED(CONFIG_SYSTEM_SUSPEND). For CONFIG_ARM, though, I think some
> new abstraction will need adding. E.g. HAS_HWDOM_SUSPEND (with want for
> a better, yet not overly long name).
>
> With the hwdom / ctldom separation work in mind I wonder though if it's
> really hwdom to be in charge of initiating system suspend. Imo
> conceptually that rather would want to be ctldom. Stefano?

I am not Stefano, but IMO, this should be ctldom. But only after hwdom
is already suspended, otherwise we'll have problems with devices.

It is unclear which entity should check if hwdom is already suspended,
though. Should be it be ctldom or Xen itself?

And if we already speaking of devices... There can be domains with
passed-through devices. Those domains should be suspended properly, not
just freezed. I think this requires another TODO.

-- 
WBR, Volodymyr