Re: [PATCH v7 1/4] xen/arm: Implement PSCI SYSTEM_SUSPEND call for guests

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.07.2025 10:52, Mykola Kvach wrote:
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1334,16 +1334,15 @@ int domain_shutdown(struct domain *d, u8 reason)
>      return 0;
>  }
>  
> -void domain_resume(struct domain *d)
> +#ifdef CONFIG_ARM
> +void domain_resume_nopause(struct domain *d)
> +#else
> +static void domain_resume_nopause(struct domain *d)
> +#endif

#ifndef CONFIG_ARM
static
#endif
void domain_resume_nopause(struct domain *d)

to have as little redundancy as possible.

>  {
>      struct vcpu *v;
>  
> -    /*
> -     * Some code paths assume that shutdown status does not get reset under
> -     * their feet (e.g., some assertions make this assumption).
> -     */
> -    domain_pause(d);
> -
> +    domain_lock(d);

This addition of locking affects domain_resume() as well. Neither need nor
correctness are discussed in the description. If the locking was really
needed for domain_resume() as well, I suppose adding that would better be
a separate change anyway.

The addition of this locking is particularly interesting considering that
...

>      spin_lock(&d->shutdown_lock);

... is what follows right after.

> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -814,6 +814,9 @@ void domain_destroy(struct domain *d);
>  int domain_kill(struct domain *d);
>  int domain_shutdown(struct domain *d, u8 reason);
>  void domain_resume(struct domain *d);
> +#ifdef CONFIG_ARM
> +void domain_resume_nopause(struct domain *d);
> +#endif
>  
>  int domain_soft_reset(struct domain *d, bool resuming);
>  
> @@ -1010,6 +1013,10 @@ static inline struct domain *next_domain_in_cpupool(
>  /* VCPU is parked. */
>  #define _VPF_parked          8
>  #define VPF_parked           (1UL<<_VPF_parked)
> +/* VCPU is suspended. */
> +#define _VPF_suspended       9
> +#define VPF_suspended        (1UL<<_VPF_suspended)

Irrespective of the style violations in pre-existing code, can you please
not add further violations, by inserting the missing blanks?

> +
>  

Please also don't introduce double blank lines.

Jan