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, 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.

Remove the restriction in the vPSCI 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 v6:
- Minor changes in comments.
- The implementation now uses own tasklet instead of continue_hypercall_on_cpu,
  as the latter rewrites user registers and would tie system suspend status
  to guest suspend status.
- The order of calls when suspending devices has been updated.

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/include/asm/mm.h      |   2 +
 xen/arch/arm/include/asm/suspend.h |   2 +
 xen/arch/arm/mmu/smpboot.c         |   2 +-
 xen/arch/arm/suspend.c             | 150 +++++++++++++++++++++++++++++
 xen/arch/arm/vpsci.c               |   9 +-
 xen/common/domain.c                |   4 +
 7 files changed, 168 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 5355534f3d..fdad53fd68 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/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
index 7a93dad2ed..61e211d087 100644
--- a/xen/arch/arm/include/asm/mm.h
+++ b/xen/arch/arm/include/asm/mm.h
@@ -365,6 +365,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 29eed4ee7f..8d30b01b7c 100644
--- a/xen/arch/arm/include/asm/suspend.h
+++ b/xen/arch/arm/include/asm/suspend.h
@@ -29,6 +29,8 @@ extern struct cpu_context cpu_context;
 void hyp_resume(void);
 int prepare_resume_ctx(struct cpu_context *ptr);
 
+void host_system_suspend(void);
+
 #endif /* CONFIG_SYSTEM_SUSPEND */
 
 #endif /* __ASM_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 5093f1bf3d..35b20581f1 100644
--- a/xen/arch/arm/suspend.c
+++ b/xen/arch/arm/suspend.c
@@ -1,9 +1,159 @@
 /* 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/llc-coloring.h>
+#include <xen/sched.h>
+#include <xen/tasklet.h>
+
+/*
+ * TODO list:
+ *  - Decide which domain will trigger system suspend ctl or hw ?
+ *  - Test system suspend with LLC_COLORING enabled and verify functionality
+ *  - Implement IOMMU suspend/resume handlers and integrate them
+ *    into the suspend/resume path (SMMU)
+ *  - Enable "xl suspend" support on ARM architecture
+ *  - Properly disable Xen timer watchdog from relevant services (init.d left)
+ *  - Add suspend/resume CI test for ARM (QEMU if feasible)
+ *  - Investigate feasibility and need for implementing system suspend on ARM32
+ */
+
 struct cpu_context cpu_context;
 
+/* Xen suspend. Note: data is not used (suspend is the suspend to RAM) */
+static void cf_check system_suspend(void *data)
+{
+    int status;
+    unsigned long flags;
+    /* 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");
+        status = -ENOSYS;
+        goto dom_resume;
+    }
+
+    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();
+
+    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;
+    }
+
+    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);
+
+ resume_console:
+    console_resume();
+    console_end_sync();
+
+    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);
+
+ dom_resume:
+    /* The resume of hardware domain should always follow Xen's resume. */
+    domain_resume(hardware_domain);
+}
+
+static DECLARE_TASKLET(system_suspend_tasklet, system_suspend, NULL);
+
+void host_system_suspend(void)
+{
+    /*
+     * system_suspend should be called when hardware domain finalizes the
+     * suspend procedure from its boot core (VCPU#0). However, Dom0's vCPU#0
+     * could be mapped to any pCPU. 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 22c3a5f544..2f52aba48d 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>
@@ -221,9 +222,10 @@ 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 */
+#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);
@@ -249,6 +251,11 @@ static int32_t do_psci_1_0_system_suspend(register_t epoint, register_t cid)
             "SYSTEM_SUSPEND requested, epoint=0x%"PRIregister", cid=0x%"PRIregister"\n",
             epoint, cid);
 
+#ifdef CONFIG_SYSTEM_SUSPEND
+    if ( is_hardware_domain(d) )
+        host_system_suspend();
+#endif
+
     return rc;
 }
 
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 667017c5e1..5e224740d3 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1317,7 +1317,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

On 02.09.2025 00:10, Mykola Kvach wrote:
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1317,7 +1317,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);

I still don't follow why Arm-specific code needs to live here. If this
can't be properly abstracted, then at the very least I'd expect some
code comment here, or at the very, very least something in the description.

From looking at hwdom_shutdown() I get the impression that it doesn't
expect to be called with SHUTDOWN_suspend, yet then the question is why we
make it into domain_shutdown() with that reason code.

Jan

Hi Jan,

On Tue, Sep 2, 2025 at 5:33 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 02.09.2025 00:10, Mykola Kvach wrote:
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -1317,7 +1317,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);
>
> I still don't follow why Arm-specific code needs to live here. If this
> can't be properly abstracted, then at the very least I'd expect some
> code comment here, or at the very, very least something in the description.

Looks like I missed your comment about this in the previous version of
the patch series.

>
> From looking at hwdom_shutdown() I get the impression that it doesn't
> expect to be called with SHUTDOWN_suspend, yet then the question is why we
> make it into domain_shutdown() with that reason code.

Thank you for the question, it is a good one.

Thinking about it, with the current implementation (i.e. when the HW domain
requests system suspend), we don't really need to call domain_shutdown().
It would be enough to pause the last running vCPU (the current one) just to
make sure that we don't return control to the domain after exiting from the
hvc trap on the PSCI SYSTEM_SUSPEND command. We also need to set
shutting_down to ensure that any asynchronous code or timer callbacks
behave properly during suspend (i.e. skip their normal actions).

However, if we consider a setup with two separate domains -- one control and
one HW -- where the control domain makes the final decision about system
suspend, then we would need to call __domain_finalise_shutdown() during the
HW domain suspend in order to notify the control domain that the HW domain
state has changed. The control domain would then check this state and call
system suspend for itself after confirming that all other domains are in a
suspended state.

I already added a TODO about moving this logic to the control domain. So, at
first sight (unless I am missing something), we can avoid extra modifications
inside domain_shutdown() and simply avoid calling it in case of HW domain.

>
> Jan

Best regards,
Mykola

Hi Jan,

On Wed, Sep 3, 2025 at 7:31 AM Mykola Kvach <xakep.amatop@gmail.com> wrote:
>
> Hi Jan,
>
> On Tue, Sep 2, 2025 at 5:33 PM Jan Beulich <jbeulich@suse.com> wrote:
> >
> > On 02.09.2025 00:10, Mykola Kvach wrote:
> > > --- a/xen/common/domain.c
> > > +++ b/xen/common/domain.c
> > > @@ -1317,7 +1317,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);
> >
> > I still don't follow why Arm-specific code needs to live here. If this
> > can't be properly abstracted, then at the very least I'd expect some
> > code comment here, or at the very, very least something in the description.
>
> Looks like I missed your comment about this in the previous version of
> the patch series.
>
> >
> > From looking at hwdom_shutdown() I get the impression that it doesn't
> > expect to be called with SHUTDOWN_suspend, yet then the question is why we
> > make it into domain_shutdown() with that reason code.
>
> Thank you for the question, it is a good one.
>
> Thinking about it, with the current implementation (i.e. when the HW domain
> requests system suspend), we don't really need to call domain_shutdown().
> It would be enough to pause the last running vCPU (the current one) just to
> make sure that we don't return control to the domain after exiting from the
> hvc trap on the PSCI SYSTEM_SUSPEND command. We also need to set
> shutting_down to ensure that any asynchronous code or timer callbacks
> behave properly during suspend (i.e. skip their normal actions).

If we avoid calling domain_shutdown() for the hardware domain during
suspend, we would need to duplicate most of its logic except for the
hwdom_shutdown() call, which is not ideal.

To improve this, I suggest introducing a helper function:

    static inline bool need_hwdom_shutdown(const struct domain *d, u8 reason)
    {
        if ( IS_ENABLED(CONFIG_SYSTEM_SUSPEND) && IS_ENABLED(CONFIG_ARM) )
            return is_hardware_domain(d) && reason != SHUTDOWN_suspend;

        return is_hardware_domain(d);
    }

Then, in domain_shutdown(), we can call need_hwdom_shutdown() instead
of directly checking is_hardware_domain(d). This keeps the logic
readable and avoids code duplication.

What do you think about this approach?

>
> However, if we consider a setup with two separate domains -- one control and
> one HW -- where the control domain makes the final decision about system
> suspend, then we would need to call __domain_finalise_shutdown() during the
> HW domain suspend in order to notify the control domain that the HW domain
> state has changed. The control domain would then check this state and call
> system suspend for itself after confirming that all other domains are in a
> suspended state.
>
> I already added a TODO about moving this logic to the control domain. So, at
> first sight (unless I am missing something), we can avoid extra modifications
> inside domain_shutdown() and simply avoid calling it in case of HW domain.
>
> >
> > Jan
>
> Best regards,
> Mykola

Best regards,
Mykola

On 09.09.2025 08:29, Mykola Kvach wrote:
> On Wed, Sep 3, 2025 at 7:31 AM Mykola Kvach <xakep.amatop@gmail.com> wrote:
>> On Tue, Sep 2, 2025 at 5:33 PM Jan Beulich <jbeulich@suse.com> wrote:
>>> On 02.09.2025 00:10, Mykola Kvach wrote:
>>>> --- a/xen/common/domain.c
>>>> +++ b/xen/common/domain.c
>>>> @@ -1317,7 +1317,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);
>>>
>>> I still don't follow why Arm-specific code needs to live here. If this
>>> can't be properly abstracted, then at the very least I'd expect some
>>> code comment here, or at the very, very least something in the description.
>>
>> Looks like I missed your comment about this in the previous version of
>> the patch series.
>>
>>>
>>> From looking at hwdom_shutdown() I get the impression that it doesn't
>>> expect to be called with SHUTDOWN_suspend, yet then the question is why we
>>> make it into domain_shutdown() with that reason code.
>>
>> Thank you for the question, it is a good one.
>>
>> Thinking about it, with the current implementation (i.e. when the HW domain
>> requests system suspend), we don't really need to call domain_shutdown().
>> It would be enough to pause the last running vCPU (the current one) just to
>> make sure that we don't return control to the domain after exiting from the
>> hvc trap on the PSCI SYSTEM_SUSPEND command. We also need to set
>> shutting_down to ensure that any asynchronous code or timer callbacks
>> behave properly during suspend (i.e. skip their normal actions).
> 
> If we avoid calling domain_shutdown() for the hardware domain during
> suspend, we would need to duplicate most of its logic except for the
> hwdom_shutdown() call, which is not ideal.

That is, you effectively take back what you said earlier (as to not needing
to call domain_shutdown())?

> To improve this, I suggest introducing a helper function:
> 
>     static inline bool need_hwdom_shutdown(const struct domain *d, u8 reason)
>     {
>         if ( IS_ENABLED(CONFIG_SYSTEM_SUSPEND) && IS_ENABLED(CONFIG_ARM) )
>             return is_hardware_domain(d) && reason != SHUTDOWN_suspend;
> 
>         return is_hardware_domain(d);
>     }

If I see a call to a function of this name, I'd expect the "hardware
domain" nature already having been checked. I.e. a call site would
rather look like

    if ( is_hardware_domain(d) && need_hwdom_shutdown(d, reason) )
        ...;

> Then, in domain_shutdown(), we can call need_hwdom_shutdown() instead
> of directly checking is_hardware_domain(d). This keeps the logic
> readable and avoids code duplication.
> 
> What do you think about this approach?

Well, there's still the CONFIG_ARM check in there that I would like to
see gone. (As a nit, the use of u8 would also want to go away.)

Furthermore with continuing to (ab)use domain_shutdown() also for the
suspend case (Dom0 isn't really shut down when suspending, aiui), you
retain the widening of the issue with the bogus setting of
d->is_shutting_down (and hence the need for later clearing the flag
again) that I mentioned elsewhere. (Yes, I remain of the opinion that
you don't need to sort that as a prereq to your work, yet at the same
time I think the goal should be to at least not make a bad situation
worse.)

Jan

Thank you for the fast response.

On Tue, Sep 9, 2025 at 9:57 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 09.09.2025 08:29, Mykola Kvach wrote:
> > On Wed, Sep 3, 2025 at 7:31 AM Mykola Kvach <xakep.amatop@gmail.com> wrote:
> >> On Tue, Sep 2, 2025 at 5:33 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>> On 02.09.2025 00:10, Mykola Kvach wrote:
> >>>> --- a/xen/common/domain.c
> >>>> +++ b/xen/common/domain.c
> >>>> @@ -1317,7 +1317,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);
> >>>
> >>> I still don't follow why Arm-specific code needs to live here. If this
> >>> can't be properly abstracted, then at the very least I'd expect some
> >>> code comment here, or at the very, very least something in the description.
> >>
> >> Looks like I missed your comment about this in the previous version of
> >> the patch series.
> >>
> >>>
> >>> From looking at hwdom_shutdown() I get the impression that it doesn't
> >>> expect to be called with SHUTDOWN_suspend, yet then the question is why we
> >>> make it into domain_shutdown() with that reason code.
> >>
> >> Thank you for the question, it is a good one.
> >>
> >> Thinking about it, with the current implementation (i.e. when the HW domain
> >> requests system suspend), we don't really need to call domain_shutdown().
> >> It would be enough to pause the last running vCPU (the current one) just to
> >> make sure that we don't return control to the domain after exiting from the
> >> hvc trap on the PSCI SYSTEM_SUSPEND command. We also need to set
> >> shutting_down to ensure that any asynchronous code or timer callbacks
> >> behave properly during suspend (i.e. skip their normal actions).
> >
> > If we avoid calling domain_shutdown() for the hardware domain during
> > suspend, we would need to duplicate most of its logic except for the
> > hwdom_shutdown() call, which is not ideal.
>
> That is, you effectively take back what you said earlier (as to not needing
> to call domain_shutdown())?

Sure. Looking more closely, I see that for the vCPUs, for example, many flags
are checked. In the case of the control domain initializing shutdown, I need
to see the __domain_finalise_shutdown() call.

We currently don’t have any functionality inside arch_domain_shutdown()
for ARM, but it would be nice to have it in the future. Calling
domain_shutdown() for every domain makes the code more consistent.

The flow for all domains will be the same during suspend, at least within
Xen’s internal code.

>
> > To improve this, I suggest introducing a helper function:
> >
> >     static inline bool need_hwdom_shutdown(const struct domain *d, u8 reason)
> >     {
> >         if ( IS_ENABLED(CONFIG_SYSTEM_SUSPEND) && IS_ENABLED(CONFIG_ARM) )
> >             return is_hardware_domain(d) && reason != SHUTDOWN_suspend;
> >
> >         return is_hardware_domain(d);
> >     }
>
> If I see a call to a function of this name, I'd expect the "hardware
> domain" nature already having been checked. I.e. a call site would
> rather look like
>
>     if ( is_hardware_domain(d) && need_hwdom_shutdown(d, reason) )
>         ...;
>

For me, the name simply indicates whether we need to call
hwdom_shutdown() or not, and I expect it to perform the check for whether
the domain is a hardware domain inside the function itself.

> > Then, in domain_shutdown(), we can call need_hwdom_shutdown() instead
> > of directly checking is_hardware_domain(d). This keeps the logic
> > readable and avoids code duplication.
> >
> > What do you think about this approach?
>
> Well, there's still the CONFIG_ARM check in there that I would like to
> see gone. (As a nit, the use of u8 would also want to go away.)

We could combine your proposal from v5 of this patch series, i.e., using the
HAS_HWDOM_SUSPEND extra config together with this helper function:

    static inline bool need_hwdom_shutdown(const struct domain *d)
    {
        bool is_hw_dom = is_hardware_domain(d);

        if ( !IS_ENABLED(CONFIG_HAS_HWDOM_SUSPEND) )
            return is_hw_dom && d->shutdown_code != SHUTDOWN_suspend;

        return is_hw_dom;
    }

As for the second argument (reason), I can extract it directly from the
domain structure, as is done in the function above.

>
> Furthermore with continuing to (ab)use domain_shutdown() also for the
> suspend case (Dom0 isn't really shut down when suspending, aiui), you
> retain the widening of the issue with the bogus setting of
> d->is_shutting_down (and hence the need for later clearing the flag
> again) that I mentioned elsewhere. (Yes, I remain of the opinion that
> you don't need to sort that as a prereq to your work, yet at the same
> time I think the goal should be to at least not make a bad situation
> worse.)

From the perspective of ARM logic inside Xen, we perform the exact same
shutdown steps as for other domains, except that in the end we need to
call Xen suspend.

For a domain with a toolstack, it is possible to have a running Xen
watchdog service. For example, if we have systemd, it can be easily stopped
from the guest because we have hooks and can perform some actions before
suspend.

The same story applies to a Linux kernel driver: if it has PM ops installed
for the Xen watchdog driver, nothing bad happens.

However, in the case of using init.d, it isn’t easy to stop the Xen WDT
automatically right before suspend. Therefore, Xen code has an extra check
(see domain_watchdog_timeout) where it checks the is_shutting_down flag
and does nothing if it is set.

The is_shutting_down flag is easily reset on Xen resume via a
domain_resume call, so I don’t see any problems with that.

>
> Jan

Best regards,
Mykola

On 09.09.2025 10:14, Mykola Kvach wrote:
> On Tue, Sep 9, 2025 at 9:57 AM Jan Beulich <jbeulich@suse.com> wrote:
>> On 09.09.2025 08:29, Mykola Kvach wrote:
>>> Then, in domain_shutdown(), we can call need_hwdom_shutdown() instead
>>> of directly checking is_hardware_domain(d). This keeps the logic
>>> readable and avoids code duplication.
>>>
>>> What do you think about this approach?
>>
>> Well, there's still the CONFIG_ARM check in there that I would like to
>> see gone. (As a nit, the use of u8 would also want to go away.)
> 
> We could combine your proposal from v5 of this patch series, i.e., using the
> HAS_HWDOM_SUSPEND extra config together with this helper function:
> 
>     static inline bool need_hwdom_shutdown(const struct domain *d)
>     {
>         bool is_hw_dom = is_hardware_domain(d);
> 
>         if ( !IS_ENABLED(CONFIG_HAS_HWDOM_SUSPEND) )
>             return is_hw_dom && d->shutdown_code != SHUTDOWN_suspend;
> 
>         return is_hw_dom;
>     }

Maybe. Yet then the next thing striking me as odd is the redundant
checking of is_hw_dom. Why not simply

{
    if ( !is_hardware_domain(d) )
        return false;

    return IS_ENABLED(CONFIG_HAS_HWDOM_SUSPEND) || d->shutdown_code != SHUTDOWN_suspend;
}

Yet as said - my expectation is anyway that the is_hardware_domain() check
would live in the caller.

> As for the second argument (reason), I can extract it directly from the
> domain structure, as is done in the function above.

Looks like a misunderstanding: I don't mind the function parameter. But
the "u8" type shouldn't be used anymore in new code; that's uint8_t now.

>> Furthermore with continuing to (ab)use domain_shutdown() also for the
>> suspend case (Dom0 isn't really shut down when suspending, aiui), you
>> retain the widening of the issue with the bogus setting of
>> d->is_shutting_down (and hence the need for later clearing the flag
>> again) that I mentioned elsewhere. (Yes, I remain of the opinion that
>> you don't need to sort that as a prereq to your work, yet at the same
>> time I think the goal should be to at least not make a bad situation
>> worse.)
> 
> From the perspective of ARM logic inside Xen, we perform the exact same
> shutdown steps as for other domains, except that in the end we need to
> call Xen suspend.

Which, as said, feels wrong. Domains to be revived after resume aren't
really shut down, so imo they should never have ->is_shutting_down set.

> For a domain with a toolstack, it is possible to have a running Xen
> watchdog service. For example, if we have systemd, it can be easily stopped
> from the guest because we have hooks and can perform some actions before
> suspend.
> 
> The same story applies to a Linux kernel driver: if it has PM ops installed
> for the Xen watchdog driver, nothing bad happens.
> 
> However, in the case of using init.d, it isn’t easy to stop the Xen WDT
> automatically right before suspend. Therefore, Xen code has an extra check
> (see domain_watchdog_timeout) where it checks the is_shutting_down flag
> and does nothing if it is set.

I don't understand how these watchdog considerations come into play here.

> The is_shutting_down flag is easily reset on Xen resume via a
> domain_resume call, so I don’t see any problems with that.

You did read my earlier mail though, regarding concerns towards the clearing
of that flag once it was set? (You must have, since iirc you even asked [1]
whether you're expected to address those issues up front.)

Jan

[1] https://lists.xen.org/archives/html/xen-devel/2025-08/msg02057.html

Thanks for your detailed comments and suggestions — much appreciated.

On Tue, Sep 9, 2025 at 12:14 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 09.09.2025 10:14, Mykola Kvach wrote:
> > On Tue, Sep 9, 2025 at 9:57 AM Jan Beulich <jbeulich@suse.com> wrote:
> >> On 09.09.2025 08:29, Mykola Kvach wrote:
> >>> Then, in domain_shutdown(), we can call need_hwdom_shutdown() instead
> >>> of directly checking is_hardware_domain(d). This keeps the logic
> >>> readable and avoids code duplication.
> >>>
> >>> What do you think about this approach?
> >>
> >> Well, there's still the CONFIG_ARM check in there that I would like to
> >> see gone. (As a nit, the use of u8 would also want to go away.)
> >
> > We could combine your proposal from v5 of this patch series, i.e., using the
> > HAS_HWDOM_SUSPEND extra config together with this helper function:
> >
> >     static inline bool need_hwdom_shutdown(const struct domain *d)
> >     {
> >         bool is_hw_dom = is_hardware_domain(d);
> >
> >         if ( !IS_ENABLED(CONFIG_HAS_HWDOM_SUSPEND) )
> >             return is_hw_dom && d->shutdown_code != SHUTDOWN_suspend;
> >
> >         return is_hw_dom;
> >     }
>
> Maybe. Yet then the next thing striking me as odd is the redundant
> checking of is_hw_dom. Why not simply
>
> {
>     if ( !is_hardware_domain(d) )
>         return false;
>
>     return IS_ENABLED(CONFIG_HAS_HWDOM_SUSPEND) || d->shutdown_code != SHUTDOWN_suspend;
> }
>
> Yet as said - my expectation is anyway that the is_hardware_domain() check
> would live in the caller.

Ack.

>
> > As for the second argument (reason), I can extract it directly from the
> > domain structure, as is done in the function above.
>
> Looks like a misunderstanding: I don't mind the function parameter. But
> the "u8" type shouldn't be used anymore in new code; that's uint8_t now.

Oh, got it.
I just used the same type as in domain_shutdown().

>
> >> Furthermore with continuing to (ab)use domain_shutdown() also for the
> >> suspend case (Dom0 isn't really shut down when suspending, aiui), you
> >> retain the widening of the issue with the bogus setting of
> >> d->is_shutting_down (and hence the need for later clearing the flag
> >> again) that I mentioned elsewhere. (Yes, I remain of the opinion that
> >> you don't need to sort that as a prereq to your work, yet at the same
> >> time I think the goal should be to at least not make a bad situation
> >> worse.)
> >
> > From the perspective of ARM logic inside Xen, we perform the exact same
> > shutdown steps as for other domains, except that in the end we need to
> > call Xen suspend.
>
> Which, as said, feels wrong. Domains to be revived after resume aren't
> really shut down, so imo they should never have ->is_shutting_down set.

I believe this is out of scope for this series;
actually, the same applies to shutdown_code.

>
> > For a domain with a toolstack, it is possible to have a running Xen
> > watchdog service. For example, if we have systemd, it can be easily stopped
> > from the guest because we have hooks and can perform some actions before
> > suspend.
> >
> > The same story applies to a Linux kernel driver: if it has PM ops installed
> > for the Xen watchdog driver, nothing bad happens.
> >
> > However, in the case of using init.d, it isn’t easy to stop the Xen WDT
> > automatically right before suspend. Therefore, Xen code has an extra check
> > (see domain_watchdog_timeout) where it checks the is_shutting_down flag
> > and does nothing if it is set.
>
> I don't understand how these watchdog considerations come into play here.

I’m just trying to explain why we still need to set this flag
even for HW domain.

>
> > The is_shutting_down flag is easily reset on Xen resume via a
> > domain_resume call, so I don’t see any problems with that.
>
> You did read my earlier mail though, regarding concerns towards the clearing
> of that flag once it was set? (You must have, since iirc you even asked [1]
> whether you're expected to address those issues up front.)

As far as I understand, this issue is relevant to x86, and I believe
it is out of scope for this series.

See my previous message here:
https://lists.xen.org/archives/html/xen-devel/2025-08/msg02127.html

I will prepare a separate patch series to address it.

>
> Jan
>
> [1] https://lists.xen.org/archives/html/xen-devel/2025-08/msg02057.html

Best regards,
Mykola

On 09.09.2025 11:55, Mykola Kvach wrote:
> On Tue, Sep 9, 2025 at 12:14 PM Jan Beulich <jbeulich@suse.com> wrote:
>> On 09.09.2025 10:14, Mykola Kvach wrote:
>>> On Tue, Sep 9, 2025 at 9:57 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>> Furthermore with continuing to (ab)use domain_shutdown() also for the
>>>> suspend case (Dom0 isn't really shut down when suspending, aiui), you
>>>> retain the widening of the issue with the bogus setting of
>>>> d->is_shutting_down (and hence the need for later clearing the flag
>>>> again) that I mentioned elsewhere. (Yes, I remain of the opinion that
>>>> you don't need to sort that as a prereq to your work, yet at the same
>>>> time I think the goal should be to at least not make a bad situation
>>>> worse.)
>>>
>>> From the perspective of ARM logic inside Xen, we perform the exact same
>>> shutdown steps as for other domains, except that in the end we need to
>>> call Xen suspend.
>>
>> Which, as said, feels wrong. Domains to be revived after resume aren't
>> really shut down, so imo they should never have ->is_shutting_down set.
> 
> I believe this is out of scope for this series;

Yes, but see at the bottom.

> actually, the same applies to shutdown_code.

Not quite sure there.

>>> The is_shutting_down flag is easily reset on Xen resume via a
>>> domain_resume call, so I don’t see any problems with that.
>>
>> You did read my earlier mail though, regarding concerns towards the clearing
>> of that flag once it was set? (You must have, since iirc you even asked [1]
>> whether you're expected to address those issues up front.)
> 
> As far as I understand, this issue is relevant to x86, and I believe
> it is out of scope for this series.

Yes and ...

> See my previous message here:
> https://lists.xen.org/archives/html/xen-devel/2025-08/msg02127.html
> 
> I will prepare a separate patch series to address it.

... thanks. My request to not extend the badness remains though, as to the
series here.

Jan

Hi everyone,

This is the first commit in the second part of the patch series that
cannot exist without part 1. All previous commits are independent and
do not depend on part 1.

On Tue, Sep 2, 2025 at 1:10 AM Mykola Kvach <xakep.amatop@gmail.com> wrote:
>
> 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, 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.
>
> Remove the restriction in the vPSCI 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 v6:
> - Minor changes in comments.
> - The implementation now uses own tasklet instead of continue_hypercall_on_cpu,
>   as the latter rewrites user registers and would tie system suspend status
>   to guest suspend status.
> - The order of calls when suspending devices has been updated.
>
> 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/include/asm/mm.h      |   2 +
>  xen/arch/arm/include/asm/suspend.h |   2 +
>  xen/arch/arm/mmu/smpboot.c         |   2 +-
>  xen/arch/arm/suspend.c             | 150 +++++++++++++++++++++++++++++
>  xen/arch/arm/vpsci.c               |   9 +-
>  xen/common/domain.c                |   4 +
>  7 files changed, 168 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 5355534f3d..fdad53fd68 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/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
> index 7a93dad2ed..61e211d087 100644
> --- a/xen/arch/arm/include/asm/mm.h
> +++ b/xen/arch/arm/include/asm/mm.h
> @@ -365,6 +365,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 29eed4ee7f..8d30b01b7c 100644
> --- a/xen/arch/arm/include/asm/suspend.h
> +++ b/xen/arch/arm/include/asm/suspend.h
> @@ -29,6 +29,8 @@ extern struct cpu_context cpu_context;
>  void hyp_resume(void);
>  int prepare_resume_ctx(struct cpu_context *ptr);
>
> +void host_system_suspend(void);
> +
>  #endif /* CONFIG_SYSTEM_SUSPEND */
>
>  #endif /* __ASM_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 5093f1bf3d..35b20581f1 100644
> --- a/xen/arch/arm/suspend.c
> +++ b/xen/arch/arm/suspend.c
> @@ -1,9 +1,159 @@
>  /* 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/llc-coloring.h>
> +#include <xen/sched.h>
> +#include <xen/tasklet.h>
> +
> +/*
> + * TODO list:
> + *  - Decide which domain will trigger system suspend ctl or hw ?
> + *  - Test system suspend with LLC_COLORING enabled and verify functionality
> + *  - Implement IOMMU suspend/resume handlers and integrate them
> + *    into the suspend/resume path (SMMU)
> + *  - Enable "xl suspend" support on ARM architecture
> + *  - Properly disable Xen timer watchdog from relevant services (init.d left)
> + *  - Add suspend/resume CI test for ARM (QEMU if feasible)
> + *  - Investigate feasibility and need for implementing system suspend on ARM32
> + */
> +
>  struct cpu_context cpu_context;
>
> +/* Xen suspend. Note: data is not used (suspend is the suspend to RAM) */
> +static void cf_check system_suspend(void *data)
> +{
> +    int status;
> +    unsigned long flags;
> +    /* 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");
> +        status = -ENOSYS;
> +        goto dom_resume;
> +    }
> +
> +    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();
> +
> +    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;
> +    }
> +
> +    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);
> +
> + resume_console:
> +    console_resume();
> +    console_end_sync();
> +
> +    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);
> +
> + dom_resume:
> +    /* The resume of hardware domain should always follow Xen's resume. */
> +    domain_resume(hardware_domain);
> +}
> +
> +static DECLARE_TASKLET(system_suspend_tasklet, system_suspend, NULL);
> +
> +void host_system_suspend(void)
> +{
> +    /*
> +     * system_suspend should be called when hardware domain finalizes the
> +     * suspend procedure from its boot core (VCPU#0). However, Dom0's vCPU#0
> +     * could be mapped to any pCPU. 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 22c3a5f544..2f52aba48d 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>
> @@ -221,9 +222,10 @@ 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 */
> +#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);
> @@ -249,6 +251,11 @@ static int32_t do_psci_1_0_system_suspend(register_t epoint, register_t cid)
>              "SYSTEM_SUSPEND requested, epoint=0x%"PRIregister", cid=0x%"PRIregister"\n",
>              epoint, cid);
>
> +#ifdef CONFIG_SYSTEM_SUSPEND
> +    if ( is_hardware_domain(d) )
> +        host_system_suspend();
> +#endif
> +
>      return rc;
>  }
>
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 667017c5e1..5e224740d3 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1317,7 +1317,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
>

Best regards,
Mykola