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

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

Hi Jan,

On Mon, Sep 1, 2025 at 5:29 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 31.08.2025 00:10, Mykola Kvach wrote:
> > --- a/xen/arch/ppc/stubs.c
> > +++ b/xen/arch/ppc/stubs.c
> > @@ -224,6 +224,11 @@ void arch_domain_creation_finished(struct domain *d)
> >      BUG_ON("unimplemented");
> >  }
> >
> > +int arch_domain_resume(struct domain *d)
> > +{
> > +    return 0;
> > +}
> > +
> >  int arch_set_info_guest(struct vcpu *v, vcpu_guest_context_u c)
> >  {
> >      BUG_ON("unimplemented");
> > diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
> > index 1a8c86cd8d..52532ae14d 100644
> > --- a/xen/arch/riscv/stubs.c
> > +++ b/xen/arch/riscv/stubs.c
> > @@ -198,6 +198,11 @@ void arch_domain_creation_finished(struct domain *d)
> >      BUG_ON("unimplemented");
> >  }
> >
> > +int arch_domain_resume(struct domain *d)
> > +{
> > +    return 0;
> > +}
> > +
> >  int arch_set_info_guest(struct vcpu *v, vcpu_guest_context_u c)
> >  {
> >      BUG_ON("unimplemented");
> > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> > index 19fd86ce88..94a06bc697 100644
> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -1138,6 +1138,11 @@ void arch_domain_creation_finished(struct domain *d)
> >          hvm_domain_creation_finished(d);
> >  }
> >
> > +int arch_domain_resume(struct domain *d)
> > +{
> > +    return 0;
> > +}
> > +
> >  #ifdef CONFIG_COMPAT
> >  #define xen_vcpu_guest_context vcpu_guest_context
> >  #define fpu_ctxt fpu_ctxt.x
>
> I definitely don't like this redundancy, and even less so that you introduce 
> out-
> of-line calls.

Thank you for your feedback.
I followed the existing pattern used in other architecture stubs.

>
> > --- a/xen/include/xen/domain.h
> > +++ b/xen/include/xen/domain.h
> > @@ -109,6 +109,8 @@ int arch_domain_soft_reset(struct domain *d);
> >
> >  void arch_domain_creation_finished(struct domain *d);
> >
> > +int arch_domain_resume(struct domain *d);
>
> I think this wants to move to a per-arch header, presence of which is checked 
> by
> has_include(), with an inline fallback define once centrally here.

Would it be acceptable to use a weak function as the default
implementation instead? This way, architectures needing a real
implementation could override it, while others would use the weak
default.

>
> Jan

Best regards,
Mykola