| Message ID | 63e5551cc906d8603abfbe9596403fdd8107c849.1665137247.git.mykyta_poturai@epam.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/19] xen/arm: Implement PSCI system suspend | expand |
Hi, On 07/10/2022 11:32, Mykyta Poturai wrote: > From: Mirela Simonovic <mirela.simonovic@aggios.com> > > Timer interrupts have to be disabled while the system is in suspend. > Otherwise, a timer interrupt would fire and wake-up the system. > Suspending the timer interrupts consists of disabling physical EL1 > and EL2 timers. The resume consists only of raising timer softirq, > which will trigger the generic timer code to reprogram the EL2 timer > as needed. Enabling of EL1 physical timer will be triggered by an > entity which uses it. > > Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com> > Signed-off-by: Saeed Nowshadi <saeed.nowshadi@xilinx.com> > --- > xen/arch/arm/suspend.c | 4 ++++ > xen/arch/arm/time.c | 22 ++++++++++++++++++++++ > xen/include/asm-arm/time.h | 3 +++ > 3 files changed, 29 insertions(+) > > diff --git a/xen/arch/arm/suspend.c b/xen/arch/arm/suspend.c > index b94a6df86d..05c43ce502 100644 > --- a/xen/arch/arm/suspend.c > +++ b/xen/arch/arm/suspend.c > @@ -151,6 +151,8 @@ static long system_suspend(void *data) > goto resume_nonboot_cpus; > } > > + time_suspend(); > + > local_irq_save(flags); > status = gic_suspend(); > if ( status ) > @@ -166,6 +168,8 @@ static long system_suspend(void *data) > resume_irqs: > local_irq_restore(flags); > > + time_resume(); > + > resume_nonboot_cpus: > rcu_barrier(); > enable_nonboot_cpus(); > diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c > index dec53b5f7d..ca54bcfe68 100644 > --- a/xen/arch/arm/time.c > +++ b/xen/arch/arm/time.c > @@ -363,6 +363,28 @@ void domain_set_time_offset(struct domain *d, int64_t time_offset_seconds) > /* XXX update guest visible wallclock time */ > } > > +void time_suspend(void) > +{ > + /* Disable physical EL1 timer */ > + WRITE_SYSREG(0, CNTP_CTL_EL0); > + > + /* Disable hypervisor's timer */ > + WRITE_SYSREG(0, CNTHP_CTL_EL2); > + isb(); > +} > + > +void time_resume(void) > +{ > + /* > + * Raising timer softirq will trigger generic timer code to reprogram_timer > + * with the correct timeout value (which is not known here). There is no > + * need to do anything else in order to recover the time keeping from power > + * down, because the system counter is not affected by the power down (it > + * resides out of the ARM's cluster in an always-on part of the SoC). > + */ Can you point me to the section in the Arm Arm stating the system counter is in an always-on part of the SoC? > + raise_softirq(TIMER_SOFTIRQ); My expectation is that the time_resume() would closely match the x86 version (aside the arch specific part). I can't see the raise_softirq(). So why do we need it here? Also, there is a missing call to do_settime() and update_vcpu_system_time(). > +} > + > static int cpu_time_callback(struct notifier_block *nfb, > unsigned long action, > void *hcpu) > diff --git a/xen/include/asm-arm/time.h b/xen/include/asm-arm/time.h > index 4b401c1110..ded93b490a 100644 > --- a/xen/include/asm-arm/time.h > +++ b/xen/include/asm-arm/time.h > @@ -107,6 +107,9 @@ void preinit_xen_time(void); > > void force_update_vcpu_system_time(struct vcpu *v); > > +void time_suspend(void); > +void time_resume(void); > + > #endif /* __ARM_TIME_H__ */ > /* > * Local variables: Cheers,
diff --git a/xen/arch/arm/suspend.c b/xen/arch/arm/suspend.c index b94a6df86d..05c43ce502 100644 --- a/xen/arch/arm/suspend.c +++ b/xen/arch/arm/suspend.c @@ -151,6 +151,8 @@ static long system_suspend(void *data) goto resume_nonboot_cpus; } + time_suspend(); + local_irq_save(flags); status = gic_suspend(); if ( status ) @@ -166,6 +168,8 @@ static long system_suspend(void *data) resume_irqs: local_irq_restore(flags); + time_resume(); + resume_nonboot_cpus: rcu_barrier(); enable_nonboot_cpus(); diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c index dec53b5f7d..ca54bcfe68 100644 --- a/xen/arch/arm/time.c +++ b/xen/arch/arm/time.c @@ -363,6 +363,28 @@ void domain_set_time_offset(struct domain *d, int64_t time_offset_seconds) /* XXX update guest visible wallclock time */ } +void time_suspend(void) +{ + /* Disable physical EL1 timer */ + WRITE_SYSREG(0, CNTP_CTL_EL0); + + /* Disable hypervisor's timer */ + WRITE_SYSREG(0, CNTHP_CTL_EL2); + isb(); +} + +void time_resume(void) +{ + /* + * Raising timer softirq will trigger generic timer code to reprogram_timer + * with the correct timeout value (which is not known here). There is no + * need to do anything else in order to recover the time keeping from power + * down, because the system counter is not affected by the power down (it + * resides out of the ARM's cluster in an always-on part of the SoC). + */ + raise_softirq(TIMER_SOFTIRQ); +} + static int cpu_time_callback(struct notifier_block *nfb, unsigned long action, void *hcpu) diff --git a/xen/include/asm-arm/time.h b/xen/include/asm-arm/time.h index 4b401c1110..ded93b490a 100644 --- a/xen/include/asm-arm/time.h +++ b/xen/include/asm-arm/time.h @@ -107,6 +107,9 @@ void preinit_xen_time(void); void force_update_vcpu_system_time(struct vcpu *v); +void time_suspend(void); +void time_resume(void); + #endif /* __ARM_TIME_H__ */ /* * Local variables: