On 19/11/2025 12:32, Mykola Kvach wrote:
> On Wed, Nov 19, 2025 at 1:07 PM Orzel, Michal <[email protected]> wrote:
>>
>>
>>
>> On 19/11/2025 12:00, Mykola Kvach wrote:
>>> Hi Michal,
>>>
>>> Thanks for your review.
>>>
>>> On Wed, Nov 19, 2025 at 10:48 AM Orzel, Michal <[email protected]> wrote:
>>>>
>>>>
>>>>
>>>> On 18/11/2025 16:37, Mykola Kvach wrote:
>>>>> From: Mykola Kvach <[email protected]>
>>>>>
>>>>> Add support for the PSCI SYSTEM_SUSPEND function in the vPSCI interface,
>>>>> allowing guests to request suspend via the PSCI v1.0+ SYSTEM_SUSPEND call
>>>>> (both 32-bit and 64-bit variants).
>>>>>
>>>>> Implementation details:
>>>>> - Add SYSTEM_SUSPEND function IDs to PSCI definitions
>>>>> - Trap and handle SYSTEM_SUSPEND in vPSCI
>>>>> - Allow only non-hardware domains to invoke SYSTEM_SUSPEND; return
>>>>>   PSCI_NOT_SUPPORTED for the hardware domain to avoid halting the system
>>>>>   in hwdom_shutdown() via domain_shutdown
>>>>> - Require all secondary VCPUs of the calling domain to be offline before
>>>>>   suspend, as mandated by the PSCI specification
>>>>>
>>>>> The arch_domain_resume() function is an architecture-specific hook that is
>>>>> invoked during domain resume to perform any necessary setup or restoration
>>>>> steps required by the platform.
>>>>>
>>>>> The new vpsci_vcpu_up_prepare() helper is called on the resume path to 
>>>>> set up
>>>>> the vCPU context (such as entry point, some system regs and context ID) 
>>>>> before
>>>>> resuming a suspended guest. This keeps ARM/vPSCI-specific logic out of 
>>>>> common
>>>>> code and avoids intrusive changes to the generic resume flow.
>>>>>
>>>>> Usage:
>>>>>
>>>>> For Linux-based guests, suspend can be initiated with:
>>>>>     echo mem > /sys/power/state
>>>>> or via:
>>>>>     systemctl suspend
>>>>>
>>>>> Resuming the guest is performed from control domain using:
>>>>>       xl resume <domain>
>>>>>
>>>>> Signed-off-by: Mykola Kvach <[email protected]>
>>>>> ---
>>>>> Changes in V14:
>>>>> - move arch_domain_resume to a separate header
>>>>> - avoid usage of typeof for context struct
>>>>> - moved error printing from domain_resume to arch_domain_resume
>>>>>   in order to simplify common parts of code
>>>>> - minor changes after review
>>>>> ---
>>>>>  xen/arch/arm/domain.c                 |  41 +++++++++
>>>>>  xen/arch/arm/include/asm/domain.h     |   2 +
>>>>>  xen/arch/arm/include/asm/perfc_defn.h |   1 +
>>>>>  xen/arch/arm/include/asm/psci.h       |   2 +
>>>>>  xen/arch/arm/include/asm/suspend.h    |  24 ++++++
>>>>>  xen/arch/arm/include/asm/vpsci.h      |   5 +-
>>>>>  xen/arch/arm/vpsci.c                  | 116 +++++++++++++++++++++-----
>>>>>  xen/common/domain.c                   |   5 ++
>>>>>  xen/include/xen/suspend.h             |  15 ++++
>>>>>  9 files changed, 189 insertions(+), 22 deletions(-)
>>>>>  create mode 100644 xen/arch/arm/include/asm/suspend.h
>>>>>  create mode 100644 xen/include/xen/suspend.h
>>>>>
>>>>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>>>>> index e36719bce4..3c0170480b 100644
>>>>> --- a/xen/arch/arm/domain.c
>>>>> +++ b/xen/arch/arm/domain.c
>>>>> @@ -12,6 +12,8 @@
>>>>>  #include <xen/softirq.h>
>>>>>  #include <xen/wait.h>
>>>>>
>>>>> +#include <public/sched.h>
>>>>> +
>>>>>  #include <asm/arm64/sve.h>
>>>>>  #include <asm/cpuerrata.h>
>>>>>  #include <asm/cpufeature.h>
>>>>> @@ -24,10 +26,12 @@
>>>>>  #include <asm/platform.h>
>>>>>  #include <asm/procinfo.h>
>>>>>  #include <asm/regs.h>
>>>>> +#include <asm/suspend.h>
>>>>>  #include <asm/firmware/sci.h>
>>>>>  #include <asm/tee/tee.h>
>>>>>  #include <asm/vfp.h>
>>>>>  #include <asm/vgic.h>
>>>>> +#include <asm/vpsci.h>
>>>>>  #include <asm/vtimer.h>
>>>>>
>>>>>  #include "vpci.h"
>>>>> @@ -885,6 +889,43 @@ void arch_domain_creation_finished(struct domain *d)
>>>>>      p2m_domain_creation_finished(d);
>>>>>  }
>>>>>
>>>>> +int arch_domain_resume(struct domain *d)
>>>>> +{
>>>>> +    int rc;
>>>>> +    struct resume_info *ctx = &d->arch.resume_ctx;
>>>>> +
>>>>> +    if ( !d->is_shutting_down || d->shutdown_code != SHUTDOWN_suspend )
>>>>> +    {
>>>>> +        dprintk(XENLOG_WARNING,
>>>>> +                "%pd: Invalid domain state for resume: 
>>>>> is_shutting_down=%d, shutdown_code=%d\n",
>>>> These are bool and uint, so %u should be used.
>>>
>>> ack
>>>
>>>>
>>>>> +                d, d->is_shutting_down, d->shutdown_code);
>>>>> +        return -EINVAL;
>>>>> +    }
>>>>> +
>>>>> +    /*
>>>>> +     * It is still possible to call domain_shutdown() with a suspend 
>>>>> reason
>>>>> +     * via some hypercalls, such as SCHEDOP_shutdown or 
>>>>> SCHEDOP_remote_shutdown.
>>>>> +     * In these cases, the resume context will be empty.
>>>>> +     * This is not expected to cause any issues, so we just warn about 
>>>>> the
>>>>> +     * situation and return without error, allowing the existing logic to
>>>>> +     * proceed as expected.
>>>>> +     */
>>>>> +    if ( !ctx->wake_cpu )
>>>>> +    {
>>>>> +        dprintk(XENLOG_INFO, "%pd: Wake CPU pointer context was not 
>>>>> provided\n",
>>>>> +                d);
>>>>> +        return 0;
>>>>> +    }
>>>>> +
>>>>> +    rc = vpsci_vcpu_up_prepare(ctx->wake_cpu , ctx->ep, ctx->cid);
>>>>> +    if ( rc )
>>>>> +        printk("%pd: Failed to resume domain (ret %d)\n", d, rc);
>>>>> +
>>>>> +    memset(ctx, 0, sizeof(*ctx));
>>>>> +
>>>>> +    return rc;
>>>>> +}
>>>>> +
>>>>>  static int is_guest_pv32_psr(uint32_t psr)
>>>>>  {
>>>>>      switch (psr & PSR_MODE_MASK)
>>>>> diff --git a/xen/arch/arm/include/asm/domain.h 
>>>>> b/xen/arch/arm/include/asm/domain.h
>>>>> index af3e168374..e637cb4de0 100644
>>>>> --- a/xen/arch/arm/include/asm/domain.h
>>>>> +++ b/xen/arch/arm/include/asm/domain.h
>>>>> @@ -5,6 +5,7 @@
>>>>>  #include <xen/timer.h>
>>>>>  #include <asm/page.h>
>>>>>  #include <asm/p2m.h>
>>>>> +#include <asm/suspend.h>
>>>>>  #include <asm/vfp.h>
>>>>>  #include <asm/mmio.h>
>>>>>  #include <asm/gic.h>
>>>>> @@ -126,6 +127,7 @@ struct arch_domain
>>>>>      void *sci_data;
>>>>>  #endif
>>>>>
>>>>> +    struct resume_info resume_ctx;
>>>>>  }  __cacheline_aligned;
>>>>>
>>>>>  struct arch_vcpu
>>>>> diff --git a/xen/arch/arm/include/asm/perfc_defn.h 
>>>>> b/xen/arch/arm/include/asm/perfc_defn.h
>>>>> index effd25b69e..8dfcac7e3b 100644
>>>>> --- a/xen/arch/arm/include/asm/perfc_defn.h
>>>>> +++ b/xen/arch/arm/include/asm/perfc_defn.h
>>>>> @@ -33,6 +33,7 @@ PERFCOUNTER(vpsci_system_reset,        "vpsci: 
>>>>> system_reset")
>>>>>  PERFCOUNTER(vpsci_cpu_suspend,         "vpsci: cpu_suspend")
>>>>>  PERFCOUNTER(vpsci_cpu_affinity_info,   "vpsci: cpu_affinity_info")
>>>>>  PERFCOUNTER(vpsci_features,            "vpsci: features")
>>>>> +PERFCOUNTER(vpsci_system_suspend,      "vpsci: system_suspend")
>>>>>
>>>>>  PERFCOUNTER(vcpu_kick,                 "vcpu: notify other vcpu")
>>>>>
>>>>> diff --git a/xen/arch/arm/include/asm/psci.h 
>>>>> b/xen/arch/arm/include/asm/psci.h
>>>>> index 4780972621..48a93e6b79 100644
>>>>> --- a/xen/arch/arm/include/asm/psci.h
>>>>> +++ b/xen/arch/arm/include/asm/psci.h
>>>>> @@ -47,10 +47,12 @@ void call_psci_system_reset(void);
>>>>>  #define PSCI_0_2_FN32_SYSTEM_OFF          PSCI_0_2_FN32(8)
>>>>>  #define PSCI_0_2_FN32_SYSTEM_RESET        PSCI_0_2_FN32(9)
>>>>>  #define PSCI_1_0_FN32_PSCI_FEATURES       PSCI_0_2_FN32(10)
>>>>> +#define PSCI_1_0_FN32_SYSTEM_SUSPEND      PSCI_0_2_FN32(14)
>>>>>
>>>>>  #define PSCI_0_2_FN64_CPU_SUSPEND         PSCI_0_2_FN64(1)
>>>>>  #define PSCI_0_2_FN64_CPU_ON              PSCI_0_2_FN64(3)
>>>>>  #define PSCI_0_2_FN64_AFFINITY_INFO       PSCI_0_2_FN64(4)
>>>>> +#define PSCI_1_0_FN64_SYSTEM_SUSPEND      PSCI_0_2_FN64(14)
>>>>>
>>>>>  /* PSCI v0.2 affinity level state returned by AFFINITY_INFO */
>>>>>  #define PSCI_0_2_AFFINITY_LEVEL_ON      0
>>>>> diff --git a/xen/arch/arm/include/asm/suspend.h 
>>>>> b/xen/arch/arm/include/asm/suspend.h
>>>>> new file mode 100644
>>>>> index 0000000000..b991a94d5a
>>>>> --- /dev/null
>>>>> +++ b/xen/arch/arm/include/asm/suspend.h
>>>>> @@ -0,0 +1,24 @@
>>>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>>>> +
>>>>> +#ifndef __ARM_SUSPEND_H__
>>>> There should be no double underscores in guards (see examples in 
>>>> CODING_STYLE)
>>>
>>> I had initially followed the style used by some of the existing headers
>>> in this directory, which still use guards with double underscores.
>>>
>>> I'll remove underscores in the next series.
>>>
>>>>> +#define __ARM_SUSPEND_H__
>>>>> +
>>>>> +struct resume_info {
>>>>> +    register_t ep;
>>>>> +    register_t cid;
>>>>> +    struct vcpu *wake_cpu;
>>>>> +};
>>>>> +
>>>>> +int arch_domain_resume(struct domain *d);
>>>> If possible, headers should be standalone, meaning you should include 
>>>> necessary
>>>> headers or forward declare what's missing.
>>>
>>> Thanks for the comment.
>>>
>>> My initial intention was to avoid adding new dependencies from asm
>>> headers to higher-level Xen headers, so I did not include e.g.
>>> <xen/sched.h> directly here. In this header we only need pointers to
>>> struct domain and struct vcpu, we never dereference them, so forward
>>> declarations would be sufficient to make it standalone.
>>>
>>> I also noticed that some existing asm headers in this directory do
>>> include higher-level Xen headers, so both patterns exist in the tree.
>>>
>>> If you prefer, I can either:
>>>   - add forward declarations
>>>
>>>       struct domain;
>>>       struct vcpu;
>>>
>>>     at the top of this header and keep the full includes in the .c
>>>     files that actually dereference these types, or
>>>
>>>   - include the appropriate Xen header(s) directly in this header.
>>>
>>> Personally I slightly prefer the forward-declaration approach to keep
>>> this header lightweight and avoid tightening the layering, but I am
>>> happy to follow your preference.
>> My preference is also to forward declare these structs.
>>
>>>
>>>>
>>>>> +
>>>>> +#endif /* __ARM_SUSPEND_H__ */
>>>>> +
>>>>> + /*
>>>>> +  * Local variables:
>>>>> +  * mode: C
>>>>> +  * c-file-style: "BSD"
>>>>> +  * c-basic-offset: 4
>>>>> +  * tab-width: 4
>>>>> +  * indent-tabs-mode: nil
>>>>> +  * End:
>>>>> +  */
>>>>> diff --git a/xen/arch/arm/include/asm/vpsci.h 
>>>>> b/xen/arch/arm/include/asm/vpsci.h
>>>>> index 0cca5e6830..d790ab3715 100644
>>>>> --- a/xen/arch/arm/include/asm/vpsci.h
>>>>> +++ b/xen/arch/arm/include/asm/vpsci.h
>>>>> @@ -23,12 +23,15 @@
>>>>>  #include <asm/psci.h>
>>>>>
>>>>>  /* Number of function implemented by virtual PSCI (only 0.2 or later) */
>>>>> -#define VPSCI_NR_FUNCS  12
>>>>> +#define VPSCI_NR_FUNCS  14
>>>>>
>>>>>  /* Functions handle PSCI calls from the guests */
>>>>>  bool do_vpsci_0_1_call(struct cpu_user_regs *regs, uint32_t fid);
>>>>>  bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid);
>>>>>
>>>>> +int vpsci_vcpu_up_prepare(struct vcpu *v, register_t entry_point,
>>>>> +                          register_t context_id);
>>>>> +
>>>>>  #endif /* __ASM_VPSCI_H__ */
>>>>>
>>>>>  /*
>>>>> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
>>>>> index 7ba9ccd94b..22c3a5f544 100644
>>>>> --- a/xen/arch/arm/vpsci.c
>>>>> +++ b/xen/arch/arm/vpsci.c
>>>>> @@ -10,32 +10,16 @@
>>>>>
>>>>>  #include <public/sched.h>
>>>>>
>>>>> -static int do_common_cpu_on(register_t target_cpu, register_t 
>>>>> entry_point,
>>>>> -                            register_t context_id)
>>>>> +int vpsci_vcpu_up_prepare(struct vcpu *v, register_t entry_point,
>>>>> +                   register_t context_id)
>>>> NIT: incorrect parameter alignment
>>>
>>> ack
>>>
>>>>
>>>>>  {
>>>>> -    struct vcpu *v;
>>>>> -    struct domain *d = current->domain;
>>>>> -    struct vcpu_guest_context *ctxt;
>>>>>      int rc;
>>>>> +    struct domain *d = v->domain;
>>>>>      bool is_thumb = entry_point & 1;
>>>>> -    register_t vcpuid;
>>>>> -
>>>>> -    vcpuid = vaffinity_to_vcpuid(target_cpu);
>>>>> -
>>>>> -    if ( (v = domain_vcpu(d, vcpuid)) == NULL )
>>>>> -        return PSCI_INVALID_PARAMETERS;
>>>>> -
>>>>> -    /* THUMB set is not allowed with 64-bit domain */
>>>>> -    if ( is_64bit_domain(d) && is_thumb )
>>>>> -        return PSCI_INVALID_ADDRESS;
>>>>> -
>>>>> -    if ( !test_bit(_VPF_down, &v->pause_flags) )
>>>>> -        return PSCI_ALREADY_ON;
>>>>> +    struct vcpu_guest_context *ctxt;
>>>>>
>>>>>      if ( (ctxt = alloc_vcpu_guest_context()) == NULL )
>>>>> -        return PSCI_DENIED;
>>>>> -
>>>>> -    vgic_clear_pending_irqs(v);
>>>>> +        return -ENOMEM;
>>>>>
>>>>>      memset(ctxt, 0, sizeof(*ctxt));
>>>>>      ctxt->user_regs.pc64 = (u64) entry_point;
>>>>> @@ -76,8 +60,37 @@ static int do_common_cpu_on(register_t target_cpu, 
>>>>> register_t entry_point,
>>>>>      free_vcpu_guest_context(ctxt);
>>>>>
>>>>>      if ( rc < 0 )
>>>>> +        return rc;
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static int do_common_cpu_on(register_t target_cpu, register_t 
>>>>> entry_point,
>>>>> +                            register_t context_id)
>>>>> +{
>>>>> +    struct vcpu *v;
>>>>> +    struct domain *d = current->domain;
>>>>> +    int rc;
>>>>> +    bool is_thumb = entry_point & 1;
>>>>> +    register_t vcpuid;
>>>>> +
>>>>> +    vcpuid = vaffinity_to_vcpuid(target_cpu);
>>>>> +
>>>>> +    if ( (v = domain_vcpu(d, vcpuid)) == NULL )
>>>>> +        return PSCI_INVALID_PARAMETERS;
>>>>> +
>>>>> +    /* THUMB set is not allowed with 64-bit domain */
>>>>> +    if ( is_64bit_domain(d) && is_thumb )
>>>>> +        return PSCI_INVALID_ADDRESS;
>>>>> +
>>>>> +    if ( !test_bit(_VPF_down, &v->pause_flags) )
>>>>> +        return PSCI_ALREADY_ON;
>>>>> +
>>>>> +    rc = vpsci_vcpu_up_prepare(v, entry_point, context_id);
>>>>> +    if ( rc )
>>>>>          return PSCI_DENIED;
>>>>>
>>>>> +    vgic_clear_pending_irqs(v);
>>>>>      vcpu_wake(v);
>>>>>
>>>>>      return PSCI_SUCCESS;
>>>>> @@ -197,6 +210,48 @@ static void do_psci_0_2_system_reset(void)
>>>>>      domain_shutdown(d,SHUTDOWN_reboot);
>>>>>  }
>>>>>
>>>>> +static int32_t do_psci_1_0_system_suspend(register_t epoint, register_t 
>>>>> cid)
>>>>> +{
>>>>> +    int32_t rc;
>>>>> +    struct vcpu *v;
>>>>> +    struct domain *d = current->domain;
>>>>> +    bool is_thumb = epoint & 1;
>>>>> +
>>>>> +    /* THUMB set is not allowed with 64-bit domain */
>>>>> +    if ( is_64bit_domain(d) && is_thumb )
>>>>> +        return PSCI_INVALID_ADDRESS;
>>>>> +
>>>>> +    /* SYSTEM_SUSPEND is not supported for the hardware domain yet */
>>>>> +    if ( is_hardware_domain(d) )
>>>>> +        return PSCI_NOT_SUPPORTED;
>>>>> +
>>>>> +    /* Ensure that all CPUs other than the calling one are offline */
>>>>> +    domain_lock(d);
>>>>> +    for_each_vcpu ( d, v )
>>>>> +    {
>>>>> +        if ( v != current && is_vcpu_online(v) )
>>>>> +        {
>>>>> +            domain_unlock(d);
>>>>> +            return PSCI_DENIED;
>>>>> +        }
>>>>> +    }
>>>>> +    domain_unlock(d);
>>>>> +
>>>>> +    rc = domain_shutdown(d, SHUTDOWN_suspend);
>>>>> +    if ( rc )
>>>>> +        return PSCI_DENIED;
>>>>> +
>>>>> +    d->arch.resume_ctx.ep = epoint;
>>>>> +    d->arch.resume_ctx.cid = cid;
>>>>> +    d->arch.resume_ctx.wake_cpu = current;
>>>>> +
>>>>> +    gprintk(XENLOG_DEBUG,
>>>>> +            "SYSTEM_SUSPEND requested, epoint=0x%"PRIregister", 
>>>>> cid=0x%"PRIregister"\n",
>>>> NIT: %# is preffered over 0x%.
>>>
>>> ack
>>>
>>>>
>>>>> +            epoint, cid);
>>>>> +
>>>>> +    return rc;
>>>>> +}
>>>>> +
>>>>>  static int32_t do_psci_1_0_features(uint32_t psci_func_id)
>>>>>  {
>>>>>      /* /!\ Ordered by function ID and not name */
>>>>> @@ -214,6 +269,8 @@ static int32_t do_psci_1_0_features(uint32_t 
>>>>> psci_func_id)
>>>>>      case PSCI_0_2_FN32_SYSTEM_OFF:
>>>>>      case PSCI_0_2_FN32_SYSTEM_RESET:
>>>>>      case PSCI_1_0_FN32_PSCI_FEATURES:
>>>>> +    case PSCI_1_0_FN32_SYSTEM_SUSPEND:
>>>>> +    case PSCI_1_0_FN64_SYSTEM_SUSPEND:
>>>>>      case ARM_SMCCC_VERSION_FID:
>>>>>          return 0;
>>>>>      default:
>>>>> @@ -344,6 +401,23 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, 
>>>>> uint32_t fid)
>>>>>          return true;
>>>>>      }
>>>>>
>>>>> +    case PSCI_1_0_FN32_SYSTEM_SUSPEND:
>>>>> +    case PSCI_1_0_FN64_SYSTEM_SUSPEND:
>>>>> +    {
>>>>> +        register_t epoint = PSCI_ARG(regs, 1);
>>>>> +        register_t cid = PSCI_ARG(regs, 2);
>>>>> +
>>>>> +        if ( fid == PSCI_1_0_FN32_SYSTEM_SUSPEND )
>>>>> +        {
>>>>> +            epoint &= GENMASK(31, 0);
>>>>> +            cid &= GENMASK(31, 0);
>>>>> +        }
>>>>> +
>>>>> +        perfc_incr(vpsci_system_suspend);
>>>>> +        PSCI_SET_RESULT(regs, do_psci_1_0_system_suspend(epoint, cid));
>>>>> +        return true;
>>>>> +    }
>>>>> +
>>>>>      default:
>>>>>          return false;
>>>>>      }
>>>>> diff --git a/xen/common/domain.c b/xen/common/domain.c
>>>>> index 775c339285..6410d32970 100644
>>>>> --- a/xen/common/domain.c
>>>>> +++ b/xen/common/domain.c
>>>>> @@ -26,6 +26,7 @@
>>>>>  #include <xen/hypercall.h>
>>>>>  #include <xen/delay.h>
>>>>>  #include <xen/shutdown.h>
>>>>> +#include <xen/suspend.h>
>>>>>  #include <xen/percpu.h>
>>>>>  #include <xen/multicall.h>
>>>>>  #include <xen/rcupdate.h>
>>>>> @@ -1363,6 +1364,9 @@ void domain_resume(struct domain *d)
>>>>>
>>>>>      spin_lock(&d->shutdown_lock);
>>>>>
>>>>> +    if ( arch_domain_resume(d) )
>>>>> +        goto fail;
>>>>> +
>>>>>      d->is_shutting_down = d->is_shut_down = 0;
>>>>>      d->shutdown_code = SHUTDOWN_CODE_INVALID;
>>>>>
>>>>> @@ -1373,6 +1377,7 @@ void domain_resume(struct domain *d)
>>>>>          v->paused_for_shutdown = 0;
>>>>>      }
>>>>>
>>>>> + fail:
>>>>>      spin_unlock(&d->shutdown_lock);
>>>>>
>>>>>      domain_unpause(d);
>>>>> diff --git a/xen/include/xen/suspend.h b/xen/include/xen/suspend.h
>>>>> new file mode 100644
>>>>> index 0000000000..53f75fd6ad
>>>>> --- /dev/null
>>>>> +++ b/xen/include/xen/suspend.h
>>>>> @@ -0,0 +1,15 @@
>>>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>>>> +#ifndef __XEN_SUSPEND_H__
>>>> There should be no double underscores in guards
>>>
>>> I initially followed the style used by the existing headers in this
>>> directory, which still have include guards with double underscores.
>>>
>>> You are right that this does not match CODING_STYLE examples.
>>> I will update this header to use a proper guard name.
>>>
>>>>
>>>>> +#define __XEN_SUSPEND_H__
>>>>> +
>>>>> +#if __has_include(<asm/suspend.h>)
>>>>> +#include <asm/suspend.h>
>>>>> +#else
>>>>> +static inline int arch_domain_resume(struct domain *d)
>>>>> +{
>>>>> +    return 0;
>>>>> +}
>>>>> +#endif
>>>>> +
>>>> Stray whiteline?
>>>
>>> will drop
>>>
>>>>
>>>>> +
>>>>> +#endif /* __XEN_SUSPEND_H__ */
>>>> Missing emacs block?
>>>
>>> It is permitted but isn't necessary as far as know,
>>> but if it needed here per your opinion I'll add it
>>> just let me know
>> On Arm we usually require them and that's the overall preference I would say.
>>
>>>
>>>>
>>>> Did you run MISRA scan on this patch?
>>>
>>> Yes, I ran it with:
>>>
>>>   ./xen/scripts/xen-analysis.py \
>>>       --run-cppcheck --cppcheck-misra --cppcheck-html -- \
>>>       XEN_TARGET_ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-
>>>
>>> The analysis did not report any new MISRA violations in the code
>>> touched by this patch.
>> That's only cppcheck scan which is rather poor in finding violations. ECLAIR
>> scan is done through Gitlab CI and this one is what we rely on for taking the
>> series in.
> 
> Thanks for the clarification.
> 
> Is it possible to run the ECLAIR MISRA scan locally, or is it only
> available via the project Gitlab CI instance? If there is a way to run
> it on a developer machine, I would be happy to set it up and check this
> series with the same configuration.
It's not possible to run it locally. But you can use your (if you don't have,
ask on Matrix) Xen fork under https://gitlab.com/xen-project/people to push a
branch and trigger the CI (ECLAIR scan needs to be manually executed from the
pipeline page).

~Michal


Reply via email to