Re: [PATCH v1 11/55] powerpc/time: add API for KVM to re-arm the host timer/decrementer

Excerpts from Christophe Leroy's message of August 5, 2021 5:22 pm:

Le 26/07/2021 à 05:49, Nicholas Piggin a écrit :

quoted

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 <npiggin@gmail.com>
---
  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