| Message ID | 15604985aae5333670467a84cccbaaa403a10000.1741164138.git.xakep.amatop@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Suspend to RAM support for Xen on arm64 | expand |
Hi Mykola, On 05/03/2025 09:11, Mykola Kvach wrote: > From: Mykola Kvach <mykola_kvach@epam.com> > > This patch implements suspend/resume helpers for the watchdog. > While a domain is suspended its watchdogs must be paused. Otherwise, > if the domain stays in the suspend state for a longer period of time > compared to the watchdog period, the domain would be shutdown on resume. > Proper solution to this problem is to stop (suspend) the watchdog timers > after the domain suspends and to restart (resume) the watchdog timers > before the domain resumes. The suspend/resume of watchdog timers is done > in Xen and is invisible to the guests. > > Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com> > Signed-off-by: Saeed Nowshadi <saeed.nowshadi@xilinx.com> > Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com> > --- > Changes in v3: > - cover the code with CONFIG_SYSTEM_SUSPEND > > Changes in v2: > - drop suspended field from timer structure > - drop the call of watchdog_domain_resume from ctxt_switch_to > --- > xen/common/sched/core.c | 39 +++++++++++++++++++++++++++++++++++++++ > xen/include/xen/sched.h | 9 +++++++++ > 2 files changed, 48 insertions(+) > > diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c > index b1c6b6b9fa..6c2231826a 100644 > --- a/xen/common/sched/core.c > +++ b/xen/common/sched/core.c > @@ -1605,6 +1605,45 @@ void watchdog_domain_destroy(struct domain *d) > kill_timer(&d->watchdog_timer[i].timer); > } > > +#ifdef CONFIG_SYSTEM_SUSPEND The config option is Arm specific, yet this is common code. As x86, already suspend/resume, then shouldn't the config option be common? But more importantly, why do we need to save/restore the watchdogs for Arm but not x86? Is this a latent issue or design choice? > + > +void watchdog_domain_suspend(struct domain *d) > +{ > + unsigned int i; > + > + spin_lock(&d->watchdog_lock); > + > + for ( i = 0; i < NR_DOMAIN_WATCHDOG_TIMERS; i++ ) > + { > + if ( test_bit(i, &d->watchdog_inuse_map) ) > + { > + stop_timer(&d->watchdog_timer[i].timer); > + } > + } > + > + spin_unlock(&d->watchdog_lock); > +} > + > +void watchdog_domain_resume(struct domain *d) > +{ > + unsigned int i; > + > + spin_lock(&d->watchdog_lock); > + > + for ( i = 0; i < NR_DOMAIN_WATCHDOG_TIMERS; i++ ) > + { > + if ( test_bit(i, &d->watchdog_inuse_map) ) > + { > + set_timer(&d->watchdog_timer[i].timer, > + NOW() + SECONDS(d->watchdog_timer[i].timeout)); > + } > + } > + > + spin_unlock(&d->watchdog_lock); > +} > + > +#endif /* CONFIG_SYSTEM_SUSPEND */ > + > /* > * Pin a vcpu temporarily to a specific CPU (or restore old pinning state if > * cpu is NR_CPUS). > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h > index d0d10612ce..caab4aad93 100644 > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -1109,6 +1109,15 @@ void scheduler_disable(void); > void watchdog_domain_init(struct domain *d); > void watchdog_domain_destroy(struct domain *d); > > +#ifdef CONFIG_SYSTEM_SUSPEND > +/* > + * Suspend/resume watchdogs of domain (while the domain is suspended its > + * watchdogs should be on pause) > + */ > +void watchdog_domain_suspend(struct domain *d); > +void watchdog_domain_resume(struct domain *d); > +#endif /* CONFIG_SYSTEM_SUSPEND */ > + > /* > * Use this check when the following are both true: > * - Using this feature or interface requires full access to the hardware Cheers,
diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c index b1c6b6b9fa..6c2231826a 100644 --- a/xen/common/sched/core.c +++ b/xen/common/sched/core.c @@ -1605,6 +1605,45 @@ void watchdog_domain_destroy(struct domain *d) kill_timer(&d->watchdog_timer[i].timer); } +#ifdef CONFIG_SYSTEM_SUSPEND + +void watchdog_domain_suspend(struct domain *d) +{ + unsigned int i; + + spin_lock(&d->watchdog_lock); + + for ( i = 0; i < NR_DOMAIN_WATCHDOG_TIMERS; i++ ) + { + if ( test_bit(i, &d->watchdog_inuse_map) ) + { + stop_timer(&d->watchdog_timer[i].timer); + } + } + + spin_unlock(&d->watchdog_lock); +} + +void watchdog_domain_resume(struct domain *d) +{ + unsigned int i; + + spin_lock(&d->watchdog_lock); + + for ( i = 0; i < NR_DOMAIN_WATCHDOG_TIMERS; i++ ) + { + if ( test_bit(i, &d->watchdog_inuse_map) ) + { + set_timer(&d->watchdog_timer[i].timer, + NOW() + SECONDS(d->watchdog_timer[i].timeout)); + } + } + + spin_unlock(&d->watchdog_lock); +} + +#endif /* CONFIG_SYSTEM_SUSPEND */ + /* * Pin a vcpu temporarily to a specific CPU (or restore old pinning state if * cpu is NR_CPUS). diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index d0d10612ce..caab4aad93 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -1109,6 +1109,15 @@ void scheduler_disable(void); void watchdog_domain_init(struct domain *d); void watchdog_domain_destroy(struct domain *d); +#ifdef CONFIG_SYSTEM_SUSPEND +/* + * Suspend/resume watchdogs of domain (while the domain is suspended its + * watchdogs should be on pause) + */ +void watchdog_domain_suspend(struct domain *d); +void watchdog_domain_resume(struct domain *d); +#endif /* CONFIG_SYSTEM_SUSPEND */ + /* * Use this check when the following are both true: * - Using this feature or interface requires full access to the hardware