[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v7 12/12] xen/arm: Add support for system suspend triggered by control domain



Hi Jan,

On Mon, Dec 15, 2025 at 1:49 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 11.12.2025 19:43, Mykola Kvach wrote:
> > --- a/xen/common/Kconfig
> > +++ b/xen/common/Kconfig
> > @@ -137,6 +137,11 @@ config HAS_EX_TABLE
> >  config HAS_FAST_MULTIPLY
> >       bool
> >
> > +config HAS_HWDOM_SHUTDOWN_ON_SUSPEND
> > +     bool
> > +     default y
> > +     depends on !ARM_64 || !SYSTEM_SUSPEND
>
> As written, this would want to be "def_bool y". However, I think I indicated

OK, I’ll switch this to def_bool.

> previously that imo it would be nice if we could stop adding more "depends on"
> referencing particular architectures. Instead "select" or "imply" from
> xen/arch/<arch>/Kconfig appears more desirable to use in such cases. That way
> each arch can control what it wants without needing to touch common code.
>
> As an aside, in the context of PV_SHIM_EXCLUSIVE it was also said several
> times that negative dependencies aren't very nice to have. Here we have no
> prompt, so the "allyesconfig" concern doesn't apply, but I still thought I'd
> mention this.

I used the common-level dependency only to avoid adding selects in every
other arch Kconfig, as the only exception I need is
    ARM_64 && SYSTEM_SUSPEND.

If you still prefer keeping all arch-specific handling under
xen/arch/<arch>/Kconfig, I can rework it accordingly.

>
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -1324,6 +1324,11 @@ void __domain_crash(struct domain *d)
> >      domain_shutdown(d, SHUTDOWN_crash);
> >  }
> >
> > +static inline bool need_hwdom_shutdown(uint8_t reason)
>
> Personally I think "want" would better express things, but I don't really
> mind "need".

I'll change it to "want".

>
> > +{
> > +    return IS_ENABLED(CONFIG_HAS_HWDOM_SHUTDOWN_ON_SUSPEND) ||
> > +           reason != SHUTDOWN_suspend;
> > +}
>
> Seeing this in use, I wonder if HAS_ is really suitable here.

What name would you consider more suitable here?

Best regards,
Mykola

>
> Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.