On Mon, Sep 1, 2025 at 8:02 PM Mykola Kvach <xakep.ama...@gmail.com> wrote:
>
> Hi Jan,
>
> On Mon, Sep 1, 2025 at 5:29 PM Jan Beulich <jbeul...@suse.com> 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.
... while I understand your concern about redundancy and out-of-line
calls, I would appreciate more specific technical reasoning for why
this approach is undesirable.
Code review is most effective when it is based on objective criteria
and project guidelines, rather than personal preferences.
This helps contributors understand the rationale and make improvements
that benefit the whole project.
>
> >
> > > --- 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
