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

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

Hi Jan,

On Fri, Aug 29, 2025 at 10:45 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 28.08.2025 18:32, Volodymyr Babchuk wrote:
> > Mykola Kvach <xakep.amatop@xxxxxxxxx> writes:
> >> @@ -880,6 +883,25 @@ void arch_domain_creation_finished(struct domain *d)
> >>      p2m_domain_creation_finished(d);
> >>  }
> >>
> >> +int arch_domain_resume(struct domain *d)
> >> +{
> >> +    int rc;
> >> +    typeof(d->arch.resume_ctx) *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",
> >> +                d, d->is_shutting_down, d->shutdown_code);
> >> +        return -EINVAL;
> >> +    }
> >
> > This check probably can go into common domain_resume() function, as
> > there is nothing arch-specific in here. Probably this can be done during
> > commit, to save us from v12? If commiters are okay with this.
>
> Seeing this, I'm learning that ->is_shutting_down can actually go from
> true to false. Which I think is a problem (not of this series, but a pre-
> existing one, see [1]). Code elsewhere assumes this to not be possible,
> see e.g. __domain_crash(), but I'm also unconvinced that e.g. the shutdown
> deferral logic is prepared to deal with such a case.

Should I add an extra patch to this series just for fixing this issue,
or can I do it within the scope of this one?

>
> Jan
>
> [1] e105919b0120 ("Add resumedomain domctl to resume a domain after 
> checkpoint")

Best regards,
Mykola