Re: [PATCH v9 12/13] xen/arm: Add vPSCI SYSTEM_SUSPEND policy

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.05.2026 19:07, Mykola Kvach wrote:
> From: Mykola Kvach <mykola_kvach@xxxxxxxx>
> 
> Introduce CONFIG_HAS_HWDOM_SYSTEM_SUSPEND as an architecture-selected
> capability for platforms where the hardware domain can be parked with
> SHUTDOWN_suspend without calling hwdom_shutdown().
> 
> Expose PSCI SYSTEM_SUSPEND as a vPSCI operation for all domains. For
> non-control domains, including the hardware domain when it is not acting as a
> control domain, the call is handled as a guest/domain suspend request and
> parks the domain in SHUTDOWN_suspend.
> 
> Control domains need additional sequencing because their SYSTEM_SUSPEND
> request is used to coordinate host-wide suspend. A non-last awake control
> domain may be parked in SHUTDOWN_suspend without requiring the host suspend
> path to be available. The last awake control domain is treated as the point
> where the request becomes a host-suspend request, and it may only proceed
> when all non-control domains are already in SHUTDOWN_suspend and the host
> suspend path is available.
> 
> Keep the control-domain sequencing and domain-readiness checks out of
> PSCI_FEATURES. They are per-attempt runtime conditions rather than stable PSCI
> function availability. Advertise SYSTEM_SUSPEND as implemented by vPSCI and
> enforce the sequencing policy in the call handler.
> 
> Select HAS_HWDOM_SYSTEM_SUSPEND independently from CONFIG_SYSTEM_SUSPEND so
> that SHUTDOWN_suspend from the hardware domain can be treated as a domain
> suspend state rather than as a hardware-domain initiated host shutdown. This
> does not by itself imply that host-wide suspend is available.
> 
> Add host_system_suspend_allowed() to combine the host PSCI SYSTEM_SUSPEND
> capability with runtime blockers reported by Xen-owned subsystems. Add
> runtime blockers for registered serial, IOMMU, GIC and SMMUv3 MSI IRQ paths
> lacking suspend/resume support. These blockers are runtime based, so they
> only apply to drivers or paths that Xen actually uses on the platform. For
> SMMUv3, the blocker applies only when Xen actually uses the MSI IRQ path,
> since resume does not restore the SMMU *_IRQ_CFGn MSI registers yet.
> 
> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
> ---
>  xen/arch/arm/Kconfig                  |   1 +
>  xen/arch/arm/gic.c                    |   6 ++
>  xen/arch/arm/include/asm/psci.h       |   3 +
>  xen/arch/arm/include/asm/suspend.h    |  10 ++-
>  xen/arch/arm/psci.c                   |   7 ++
>  xen/arch/arm/suspend.c                |  40 +++++++++
>  xen/arch/arm/vpsci.c                  | 114 +++++++++++++++++++++++---
>  xen/common/Kconfig                    |   3 +
>  xen/common/domain.c                   |   7 +-
>  xen/drivers/char/serial.c             |  12 +++
>  xen/drivers/passthrough/arm/iommu.c   |   4 +
>  xen/drivers/passthrough/arm/smmu-v3.c |   4 +
>  xen/include/xen/serial.h              |   1 +
>  xen/include/xen/suspend.h             |   2 +
>  14 files changed, 201 insertions(+), 13 deletions(-)
> 

Contrary to what the cover letter says, there's no revlog here.

> --- a/xen/arch/arm/suspend.c
> +++ b/xen/arch/arm/suspend.c
> @@ -1,9 +1,49 @@
>  /* SPDX-License-Identifier: GPL-2.0-only */
>  
> +#include <asm/psci.h>
>  #include <asm/suspend.h>
>  
> +#include <xen/lib.h>
> +#include <xen/serial.h>
> +
>  struct resume_cpu_context resume_cpu_context;
>  
> +/*
> + * Non-PSCI infrastructure can make host suspend impossible even when the 
> PSCI
> + * SYSTEM_SUSPEND conduit is present, e.g. when a Xen-owned driver has no 
> valid
> + * suspend/resume path.
> + *
> + * This gate is checked only when the last awake control domain attempts to
> + * turn a guest SYSTEM_SUSPEND request into a host-suspend request.
> + */
> +static bool host_system_suspend_runtime_allowed = true;
> +
> +static bool host_serial_suspend_allowed(void)
> +{
> +    if ( serial_suspend_supported() )
> +        return true;
> +
> +    printk_once(XENLOG_INFO
> +                "Host SYSTEM_SUSPEND blocked: serial driver lacks 
> suspend/resume support\n");

Please try to keep log messages down to a reasonable size. In the case here,
what value does "suspend/resume" add?

> +static int32_t domain_psci_system_suspend_policy(struct domain *d)
> +{
> +    struct domain *other;
> +    bool last_awake_control_domain = true;
> +    bool awake_non_control_domain = false;
> +
> +    /* Only control domains participate in sequencing policy. */
> +    if ( !is_control_domain(d) )
> +        return 0;
> +
> +    rcu_read_lock(&domlist_read_lock);
> +
> +    for_each_domain ( other )
> +    {
> +        bool suspended;
> +
> +        if ( other == d )
> +            continue;
> +
> +        suspended = domain_in_suspend_state(other);
> +        if ( suspended )
> +            continue;
> +
> +        if ( is_control_domain(other) )
> +        {
> +            last_awake_control_domain = false;
> +            break;
> +        }
> +
> +        awake_non_control_domain = true;
> +    }
> +
> +    rcu_read_unlock(&domlist_read_lock);
> +
> +    /*
> +     * Another control domain is still awake. This request is only the first
> +     * phase of the sequencing: park this control domain and leave the host
> +     * running. Host-wide suspend gates must not block this intermediate 
> state.
> +     */
> +    if ( !last_awake_control_domain )
> +        return 0;
> +
> +    /*
> +     * This is the last awake control domain. It must not be parked unless 
> the
> +     * request can proceed as a host-suspend request; otherwise Xen would 
> lose
> +     * the last domain that can coordinate the system suspend.
> +     */
> +    if ( awake_non_control_domain )
> +    {
> +        printk(XENLOG_DEBUG
> +               "SYSTEM_SUSPEND denied: last awake control domain dom%u 
> requested host suspend while non-control domains are still awake\n",
> +               d->domain_id);

Same here, plus please use %pd.

> --- a/xen/drivers/char/serial.c
> +++ b/xen/drivers/char/serial.c
> @@ -497,6 +497,8 @@ const struct vuart_info *serial_vuart_info(int idx)
>  
>  #ifdef CONFIG_SYSTEM_SUSPEND
>  
> +static bool __read_mostly serial_suspend_available = true;

__ro_after_init?

Jan