From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicholas Piggin Date: Fri, 06 Aug 2021 10:30:23 +0000 Subject: Re: [PATCH v1 11/55] powerpc/time: add API for KVM to re-arm the host timer/decrementer Message-Id: <1628245629.4fpdbi6ce8.astroid@bobo.none> List-Id: References: <20210726035036.739609-1-npiggin@gmail.com> <20210726035036.739609-12-npiggin@gmail.com> <370398a9-4429-285e-4a0f-33759f39b2fc@csgroup.eu> In-Reply-To: <370398a9-4429-285e-4a0f-33759f39b2fc@csgroup.eu> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: Christophe Leroy , kvm-ppc@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Excerpts from Christophe Leroy's message of August 5, 2021 5:22 pm: > > > Le 26/07/2021 à 05:49, Nicholas Piggin a écrit : >> Rather than have KVM look up the host timer and fiddle with the >> irq-work internal details, have the powerpc/time.c code provide a >> function for KVM to re-arm the Linux timer code when exiting a >> guest. >> >> This is implementation has an improvement over existing code of >> marking a decrementer interrupt as soft-pending if a timer has >> expired, rather than setting DEC to a -ve value, which tended to >> cause host timers to take two interrupts (first hdec to exit the >> guest, then the immediate dec). >> >> Signed-off-by: Nicholas Piggin >> --- >> arch/powerpc/include/asm/time.h | 16 +++------- >> arch/powerpc/kernel/time.c | 52 +++++++++++++++++++++++++++------ >> arch/powerpc/kvm/book3s_hv.c | 7 ++--- >> 3 files changed, 49 insertions(+), 26 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h >> index 69b6be617772..924b2157882f 100644 >> --- a/arch/powerpc/include/asm/time.h >> +++ b/arch/powerpc/include/asm/time.h >> @@ -99,18 +99,6 @@ extern void div128_by_32(u64 dividend_high, u64 dividend_low, >> extern void secondary_cpu_time_init(void); >> extern void __init time_init(void); >> >> -#ifdef CONFIG_PPC64 >> -static inline unsigned long test_irq_work_pending(void) >> -{ >> - unsigned long x; >> - >> - asm volatile("lbz %0,%1(13)" >> - : "=r" (x) >> - : "i" (offsetof(struct paca_struct, irq_work_pending))); >> - return x; >> -} >> -#endif >> - >> DECLARE_PER_CPU(u64, decrementers_next_tb); >> >> static inline u64 timer_get_next_tb(void) >> @@ -118,6 +106,10 @@ static inline u64 timer_get_next_tb(void) >> return __this_cpu_read(decrementers_next_tb); >> } >> >> +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE >> +void timer_rearm_host_dec(u64 now); >> +#endif >> + >> /* Convert timebase ticks to nanoseconds */ >> unsigned long long tb_to_ns(unsigned long long tb_ticks); >> >> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c >> index 72d872b49167..016828b7401b 100644 >> --- a/arch/powerpc/kernel/time.c >> +++ b/arch/powerpc/kernel/time.c >> @@ -499,6 +499,16 @@ EXPORT_SYMBOL(profile_pc); >> * 64-bit uses a byte in the PACA, 32-bit uses a per-cpu variable... >> */ >> #ifdef CONFIG_PPC64 >> +static inline unsigned long test_irq_work_pending(void) >> +{ >> + unsigned long x; >> + >> + asm volatile("lbz %0,%1(13)" >> + : "=r" (x) >> + : "i" (offsetof(struct paca_struct, irq_work_pending))); > > Can we just use READ_ONCE() instead of hard coding the read ? Good question, probably yes. Probably calls for its own patch series, e.g., hw_irq.h has all similar paca accesses. Thanks, Nick