|
[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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |