On 18.08.2025 14:43, Mykola Kvach wrote:
> On Mon, Aug 18, 2025 at 1:15 PM Jan Beulich <jbeul...@suse.com> wrote:
>> On 18.08.2025 10:49, Mykola Kvach wrote:
>>> @@ -1360,13 +1357,33 @@ void domain_resume(struct domain *d)
>>>
>>> for_each_vcpu ( d, v )
>>> {
>>> + /*
>>> + * No need to conditionally clear _VPF_suspended here:
>>> + * - This bit is only set on Arm64, and only after a successful
>>> suspend.
>
> Note to self: s/Arm64/Arm/g
>
>>> + * - domain_resume_nopause() may also be called from paths other
>>> than
>>> + * the suspend/resume flow, such as "soft-reset" actions (e.g.,
>>> + * on_poweroff), as part of the Xenstore control/shutdown
>>> protocol.
>>> + * These require guest acknowledgement to complete the operation.
>>> + * So clearing the bit unconditionally is safe.
>>> + */
>>> + clear_bit(_VPF_suspended, &v->pause_flags);
>>
>> Seeing that you set this bit for a single vCPU in a domain only, I wonder why
>> it needs to be a per-vCPU flag.
>
> That's a good question. In earlier versions of this patch series, both I and
> some other contributors used existed fields from the domain structure, such as
> shutdown_code and is_shutting_down, for this purpose. However, I recall that
> in a previous review, this approach was not well received. See:
> https://lore.kernel.org/all/d24be446-af5a-7881-2db4-b25afac3e...@citrix.com/
>
> Technically, there is nothing preventing me from storing this information in
> the domain structure. However, I do not see much benefit in introducing a new
> field to the domain struct when there is already an existing per-vCPU flags
> field that tracks powerdown and pause states. Using one more bit in the
> pause_flags field seems sufficient and avoids further bloating the domain
> structure.
Hmm, yes, I was mis-remembering something here: I thought that much like we
have pause_count both for vCPU-s and for domains, we'd also have pause_flags
for both. Perhaps indeed okay as is then, as far as where to put the flag
goes.
Jan
