Re: [PATCH v5 07/12] xen/arm: Add support for system suspend triggered by hardware domain

[Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>]

Hi Jan,

Jan Beulich <jbeulich@suse.com> writes:

> On 11.08.2025 22:48, Mykola Kvach wrote:
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -1311,7 +1311,11 @@ int domain_shutdown(struct domain *d, u8 reason)
>>          d->shutdown_code = reason;
>>      reason = d->shutdown_code;
>>  
>> +#if defined(CONFIG_SYSTEM_SUSPEND) && defined(CONFIG_ARM)
>> +    if ( reason != SHUTDOWN_suspend && is_hardware_domain(d) )
>> +#else
>>      if ( is_hardware_domain(d) )
>> +#endif
>>          hwdom_shutdown(reason);
>
> There better wouldn't be any #ifdef here. Afaict you can easily use
> IS_ENABLED(CONFIG_SYSTEM_SUSPEND). For CONFIG_ARM, though, I think some
> new abstraction will need adding. E.g. HAS_HWDOM_SUSPEND (with want for
> a better, yet not overly long name).
>
> With the hwdom / ctldom separation work in mind I wonder though if it's
> really hwdom to be in charge of initiating system suspend. Imo
> conceptually that rather would want to be ctldom. Stefano?

I am not Stefano, but IMO, this should be ctldom. But only after hwdom
is already suspended, otherwise we'll have problems with devices.

It is unclear which entity should check if hwdom is already suspended,
though. Should be it be ctldom or Xen itself?

And if we already speaking of devices... There can be domains with
passed-through devices. Those domains should be suspended properly, not
just freezed. I think this requires another TODO.

-- 
WBR, Volodymyr