Hi Jan, On Tue, Sep 2, 2025 at 5:33 PM Jan Beulich <jbeul...@suse.com> wrote: > > On 02.09.2025 00:10, Mykola Kvach wrote: > > --- a/xen/common/domain.c > > +++ b/xen/common/domain.c > > @@ -1317,7 +1317,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); > > I still don't follow why Arm-specific code needs to live here. If this > can't be properly abstracted, then at the very least I'd expect some > code comment here, or at the very, very least something in the description.
Looks like I missed your comment about this in the previous version of the patch series. > > From looking at hwdom_shutdown() I get the impression that it doesn't > expect to be called with SHUTDOWN_suspend, yet then the question is why we > make it into domain_shutdown() with that reason code. Thank you for the question, it is a good one. Thinking about it, with the current implementation (i.e. when the HW domain requests system suspend), we don't really need to call domain_shutdown(). It would be enough to pause the last running vCPU (the current one) just to make sure that we don't return control to the domain after exiting from the hvc trap on the PSCI SYSTEM_SUSPEND command. We also need to set shutting_down to ensure that any asynchronous code or timer callbacks behave properly during suspend (i.e. skip their normal actions). However, if we consider a setup with two separate domains -- one control and one HW -- where the control domain makes the final decision about system suspend, then we would need to call __domain_finalise_shutdown() during the HW domain suspend in order to notify the control domain that the HW domain state has changed. The control domain would then check this state and call system suspend for itself after confirming that all other domains are in a suspended state. I already added a TODO about moving this logic to the control domain. So, at first sight (unless I am missing something), we can avoid extra modifications inside domain_shutdown() and simply avoid calling it in case of HW domain. > > Jan Best regards, Mykola
