On 15.01.2026 11:19, Mykola Kvach wrote:
> On Mon, Dec 15, 2025 at 1:49 PM Jan Beulich <[email protected]> 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.
Imo there are two options: Do as you suggest, but with an option not starting
HAS_*. Or use HAS_ with per-arch selects (which I think I'd prefer).
To limit the number of selects needed, perhaps the sense of the option may
want inverting? Otoh I don't think we know yet what RISC-V and PPC are going
to want?
>>> --- 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?
As per above, HAS_ dropped would be an option. Yet that goes against my
preference above. Maybe HAS_ really is okay-ish here, despite what I said
earlier.
Jan
