diff mbox series

[kvm-unit-tests,v4,03/12] powerpc: Abstract H_CEDE calls into a sleep functions

Message ID 20230608075826.86217-4-npiggin@gmail.com (mailing list archive)
State New, archived
Headers show
Series powerpc: updates, P10, PNV support | expand

Commit Message

Nicholas Piggin June 8, 2023, 7:58 a.m. UTC 
This consolidates several implementations, and it no longer leaves
MSR[EE] enabled after the decrementer interrupt is handled, but
rather disables it on return.

The handler no longer allows a continuous ticking, but rather dec
has to be re-armed and EE re-enabled (e.g., via H_CEDE hcall) each
time.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 lib/powerpc/asm/handlers.h  |  2 +-
 lib/powerpc/asm/ppc_asm.h   |  1 +
 lib/powerpc/asm/processor.h |  7 ++++++
 lib/powerpc/handlers.c      | 10 ++++-----
 lib/powerpc/processor.c     | 43 +++++++++++++++++++++++++++++++++++++
 powerpc/sprs.c              |  6 +-----
 powerpc/tm.c                | 20 +----------------
 7 files changed, 58 insertions(+), 31 deletions(-)

Comments

Thomas Huth July 3, 2023, 1:13 p.m. UTC | #1 
On 08/06/2023 09.58, Nicholas Piggin wrote:
> This consolidates several implementations, and it no longer leaves
> MSR[EE] enabled after the decrementer interrupt is handled, but
> rather disables it on return.
> 
> The handler no longer allows a continuous ticking, but rather dec
> has to be re-armed and EE re-enabled (e.g., via H_CEDE hcall) each
> time.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   lib/powerpc/asm/handlers.h  |  2 +-
>   lib/powerpc/asm/ppc_asm.h   |  1 +
>   lib/powerpc/asm/processor.h |  7 ++++++
>   lib/powerpc/handlers.c      | 10 ++++-----
>   lib/powerpc/processor.c     | 43 +++++++++++++++++++++++++++++++++++++
>   powerpc/sprs.c              |  6 +-----
>   powerpc/tm.c                | 20 +----------------
>   7 files changed, 58 insertions(+), 31 deletions(-)
> 
> diff --git a/lib/powerpc/asm/handlers.h b/lib/powerpc/asm/handlers.h
> index 64ba727a..e4a0cd45 100644
> --- a/lib/powerpc/asm/handlers.h
> +++ b/lib/powerpc/asm/handlers.h
> @@ -3,6 +3,6 @@
>   
>   #include <asm/ptrace.h>
>   
> -void dec_except_handler(struct pt_regs *regs, void *data);
> +void dec_handler_oneshot(struct pt_regs *regs, void *data);
>   
>   #endif /* _ASMPOWERPC_HANDLERS_H_ */
> diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
> index 1b85f6bb..6299ff53 100644
> --- a/lib/powerpc/asm/ppc_asm.h
> +++ b/lib/powerpc/asm/ppc_asm.h
> @@ -36,6 +36,7 @@
>   #endif /* __BYTE_ORDER__ */
>   
>   /* Machine State Register definitions: */
> +#define MSR_EE_BIT	15			/* External Interrupts Enable */
>   #define MSR_SF_BIT	63			/* 64-bit mode */
>   
>   #endif /* _ASMPOWERPC_PPC_ASM_H */
> diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
> index ac001e18..ebfeff2b 100644
> --- a/lib/powerpc/asm/processor.h
> +++ b/lib/powerpc/asm/processor.h
> @@ -20,6 +20,8 @@ static inline uint64_t get_tb(void)
>   
>   extern void delay(uint64_t cycles);
>   extern void udelay(uint64_t us);
> +extern void sleep_tb(uint64_t cycles);
> +extern void usleep(uint64_t us);
>   
>   static inline void mdelay(uint64_t ms)
>   {
> @@ -27,4 +29,9 @@ static inline void mdelay(uint64_t ms)
>   		udelay(1000);
>   }
>   
> +static inline void msleep(uint64_t ms)
> +{
> +	usleep(ms * 1000);
> +}
> +
>   #endif /* _ASMPOWERPC_PROCESSOR_H_ */
> diff --git a/lib/powerpc/handlers.c b/lib/powerpc/handlers.c
> index c8721e0a..296f14ff 100644
> --- a/lib/powerpc/handlers.c
> +++ b/lib/powerpc/handlers.c
> @@ -9,15 +9,13 @@
>   #include <libcflat.h>
>   #include <asm/handlers.h>
>   #include <asm/ptrace.h>
> +#include <asm/ppc_asm.h>
>   
>   /*
>    * Generic handler for decrementer exceptions (0x900)
> - * Just reset the decrementer back to the value specified when registering the
> - * handler
> + * Return with MSR[EE] disabled.
>    */
> -void dec_except_handler(struct pt_regs *regs __unused, void *data)
> +void dec_handler_oneshot(struct pt_regs *regs, void *data)
>   {
> -	uint64_t dec = *((uint64_t *) data);
> -
> -	asm volatile ("mtdec %0" : : "r" (dec));
> +	regs->msr &= ~(1UL << MSR_EE_BIT);
>   }
> diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
> index 0550e4fc..aaf45b68 100644
> --- a/lib/powerpc/processor.c
> +++ b/lib/powerpc/processor.c
> @@ -10,6 +10,8 @@
>   #include <asm/ptrace.h>
>   #include <asm/setup.h>
>   #include <asm/barrier.h>
> +#include <asm/hcall.h>
> +#include <asm/handlers.h>
>   
>   static struct {
>   	void (*func)(struct pt_regs *, void *data);
> @@ -58,3 +60,44 @@ void udelay(uint64_t us)
>   {
>   	delay((us * tb_hz) / 1000000);
>   }
> +
> +void sleep_tb(uint64_t cycles)
> +{
> +	uint64_t start, end, now;
> +
> +	start = now = get_tb();
> +	end = start + cycles;
> +
> +	while (end > now) {
> +		uint64_t left = end - now;
> +
> +		/* TODO: Could support large decrementer */
> +		if (left > 0x7fffffff)
> +			left = 0x7fffffff;
> +
> +		/* DEC won't fire until H_CEDE is called because EE=0 */
> +		asm volatile ("mtdec %0" : : "r" (left));
> +		handle_exception(0x900, &dec_handler_oneshot, NULL);
> +		/*
> +		 * H_CEDE is called with MSR[EE] clear and enables it as part
> +		 * of the hcall, returning with EE enabled. The dec interrupt
> +		 * is then taken immediately and the handler disables EE.
> +		 *
> +		 * If H_CEDE returned for any other interrupt than dec
> +		 * expiring, that is considered an unhandled interrupt and
> +		 * the test case would be stopped.
> +		 */
> +		if (hcall(H_CEDE) != H_SUCCESS) {
> +			printf("H_CEDE failed\n");
> +			abort();
> +		}
> +		handle_exception(0x900, NULL, NULL);
> +
> +		now = get_tb();
> +	}
> +}
> +
> +void usleep(uint64_t us)
> +{
> +	sleep_tb((us * tb_hz) / 1000000);
> +}
> diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> index 5cc1cd16..ba4ddee4 100644
> --- a/powerpc/sprs.c
> +++ b/powerpc/sprs.c
> @@ -254,7 +254,6 @@ int main(int argc, char **argv)
>   		0x1234567890ABCDEFULL, 0xFEDCBA0987654321ULL,
>   		-1ULL,
>   	};
> -	static uint64_t decr = 0x7FFFFFFF; /* Max value */
>   
>   	for (i = 1; i < argc; i++) {
>   		if (!strcmp(argv[i], "-w")) {
> @@ -288,10 +287,7 @@ int main(int argc, char **argv)
>   	if (pause) {
>   		migrate_once();
>   	} else {
> -		puts("Sleeping...\n");
> -		handle_exception(0x900, &dec_except_handler, &decr);
> -		asm volatile ("mtdec %0" : : "r" (0x3FFFFFFF));
> -		hcall(H_CEDE);
> +		msleep(2000);
>   	}
>   
>   	get_sprs(after);
> diff --git a/powerpc/tm.c b/powerpc/tm.c
> index 65cacdf5..7fa91636 100644
> --- a/powerpc/tm.c
> +++ b/powerpc/tm.c
> @@ -48,17 +48,6 @@ static int count_cpus_with_tm(void)
>   	return available;
>   }
>   
> -static int h_cede(void)
> -{
> -	register uint64_t r3 asm("r3") = H_CEDE;
> -
> -	asm volatile ("sc 1" : "+r"(r3) :
> -			     : "r0", "r4", "r5", "r6", "r7", "r8", "r9",
> -			       "r10", "r11", "r12", "xer", "ctr", "cc");
> -
> -	return r3;
> -}
> -
>   /*
>    * Enable transactional memory
>    * Returns:	FALSE - Failure
> @@ -95,14 +84,10 @@ static bool enable_tm(void)
>   static void test_h_cede_tm(int argc, char **argv)
>   {
>   	int i;
> -	static uint64_t decr = 0x3FFFFF; /* ~10ms */
>   
>   	if (argc > 2)
>   		report_abort("Unsupported argument: '%s'", argv[2]);
>   
> -	handle_exception(0x900, &dec_except_handler, &decr);
> -	asm volatile ("mtdec %0" : : "r" (decr));
> -
>   	if (!start_all_cpus(halt, 0))
>   		report_abort("Failed to start secondary cpus");
>   
> @@ -120,10 +105,7 @@ static void test_h_cede_tm(int argc, char **argv)
>   		      "bf 2,1b" : : : "cr0");
>   
>   	for (i = 0; i < 500; i++) {
> -		uint64_t rval = h_cede();
> -
> -		if (rval != H_SUCCESS)
> -			break;
> +		msleep(10);
>   		mdelay(5);

msleep() directly followed by an mdelay() looks weird now. Do we still need 
the mdelay()? Or could we maybe at least get a comment here why there are 
now two different delaying calls here?

  Thomas


>   	}
>
diff mbox series

Patch

diff --git a/lib/powerpc/asm/handlers.h b/lib/powerpc/asm/handlers.h
index 64ba727a..e4a0cd45 100644
--- a/lib/powerpc/asm/handlers.h
+++ b/lib/powerpc/asm/handlers.h
@@ -3,6 +3,6 @@ 
 
 #include <asm/ptrace.h>
 
-void dec_except_handler(struct pt_regs *regs, void *data);
+void dec_handler_oneshot(struct pt_regs *regs, void *data);
 
 #endif /* _ASMPOWERPC_HANDLERS_H_ */
diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
index 1b85f6bb..6299ff53 100644
--- a/lib/powerpc/asm/ppc_asm.h
+++ b/lib/powerpc/asm/ppc_asm.h
@@ -36,6 +36,7 @@ 
 #endif /* __BYTE_ORDER__ */
 
 /* Machine State Register definitions: */
+#define MSR_EE_BIT	15			/* External Interrupts Enable */
 #define MSR_SF_BIT	63			/* 64-bit mode */
 
 #endif /* _ASMPOWERPC_PPC_ASM_H */
diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
index ac001e18..ebfeff2b 100644
--- a/lib/powerpc/asm/processor.h
+++ b/lib/powerpc/asm/processor.h
@@ -20,6 +20,8 @@  static inline uint64_t get_tb(void)
 
 extern void delay(uint64_t cycles);
 extern void udelay(uint64_t us);
+extern void sleep_tb(uint64_t cycles);
+extern void usleep(uint64_t us);
 
 static inline void mdelay(uint64_t ms)
 {
@@ -27,4 +29,9 @@  static inline void mdelay(uint64_t ms)
 		udelay(1000);
 }
 
+static inline void msleep(uint64_t ms)
+{
+	usleep(ms * 1000);
+}
+
 #endif /* _ASMPOWERPC_PROCESSOR_H_ */
diff --git a/lib/powerpc/handlers.c b/lib/powerpc/handlers.c
index c8721e0a..296f14ff 100644
--- a/lib/powerpc/handlers.c
+++ b/lib/powerpc/handlers.c
@@ -9,15 +9,13 @@ 
 #include <libcflat.h>
 #include <asm/handlers.h>
 #include <asm/ptrace.h>
+#include <asm/ppc_asm.h>
 
 /*
  * Generic handler for decrementer exceptions (0x900)
- * Just reset the decrementer back to the value specified when registering the
- * handler
+ * Return with MSR[EE] disabled.
  */
-void dec_except_handler(struct pt_regs *regs __unused, void *data)
+void dec_handler_oneshot(struct pt_regs *regs, void *data)
 {
-	uint64_t dec = *((uint64_t *) data);
-
-	asm volatile ("mtdec %0" : : "r" (dec));
+	regs->msr &= ~(1UL << MSR_EE_BIT);
 }
diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
index 0550e4fc..aaf45b68 100644
--- a/lib/powerpc/processor.c
+++ b/lib/powerpc/processor.c
@@ -10,6 +10,8 @@ 
 #include <asm/ptrace.h>
 #include <asm/setup.h>
 #include <asm/barrier.h>
+#include <asm/hcall.h>
+#include <asm/handlers.h>
 
 static struct {
 	void (*func)(struct pt_regs *, void *data);
@@ -58,3 +60,44 @@  void udelay(uint64_t us)
 {
 	delay((us * tb_hz) / 1000000);
 }
+
+void sleep_tb(uint64_t cycles)
+{
+	uint64_t start, end, now;
+
+	start = now = get_tb();
+	end = start + cycles;
+
+	while (end > now) {
+		uint64_t left = end - now;
+
+		/* TODO: Could support large decrementer */
+		if (left > 0x7fffffff)
+			left = 0x7fffffff;
+
+		/* DEC won't fire until H_CEDE is called because EE=0 */
+		asm volatile ("mtdec %0" : : "r" (left));
+		handle_exception(0x900, &dec_handler_oneshot, NULL);
+		/*
+		 * H_CEDE is called with MSR[EE] clear and enables it as part
+		 * of the hcall, returning with EE enabled. The dec interrupt
+		 * is then taken immediately and the handler disables EE.
+		 *
+		 * If H_CEDE returned for any other interrupt than dec
+		 * expiring, that is considered an unhandled interrupt and
+		 * the test case would be stopped.
+		 */
+		if (hcall(H_CEDE) != H_SUCCESS) {
+			printf("H_CEDE failed\n");
+			abort();
+		}
+		handle_exception(0x900, NULL, NULL);
+
+		now = get_tb();
+	}
+}
+
+void usleep(uint64_t us)
+{
+	sleep_tb((us * tb_hz) / 1000000);
+}
diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index 5cc1cd16..ba4ddee4 100644
--- a/powerpc/sprs.c
+++ b/powerpc/sprs.c
@@ -254,7 +254,6 @@  int main(int argc, char **argv)
 		0x1234567890ABCDEFULL, 0xFEDCBA0987654321ULL,
 		-1ULL,
 	};
-	static uint64_t decr = 0x7FFFFFFF; /* Max value */
 
 	for (i = 1; i < argc; i++) {
 		if (!strcmp(argv[i], "-w")) {
@@ -288,10 +287,7 @@  int main(int argc, char **argv)
 	if (pause) {
 		migrate_once();
 	} else {
-		puts("Sleeping...\n");
-		handle_exception(0x900, &dec_except_handler, &decr);
-		asm volatile ("mtdec %0" : : "r" (0x3FFFFFFF));
-		hcall(H_CEDE);
+		msleep(2000);
 	}
 
 	get_sprs(after);
diff --git a/powerpc/tm.c b/powerpc/tm.c
index 65cacdf5..7fa91636 100644
--- a/powerpc/tm.c
+++ b/powerpc/tm.c
@@ -48,17 +48,6 @@  static int count_cpus_with_tm(void)
 	return available;
 }
 
-static int h_cede(void)
-{
-	register uint64_t r3 asm("r3") = H_CEDE;
-
-	asm volatile ("sc 1" : "+r"(r3) :
-			     : "r0", "r4", "r5", "r6", "r7", "r8", "r9",
-			       "r10", "r11", "r12", "xer", "ctr", "cc");
-
-	return r3;
-}
-
 /*
  * Enable transactional memory
  * Returns:	FALSE - Failure
@@ -95,14 +84,10 @@  static bool enable_tm(void)
 static void test_h_cede_tm(int argc, char **argv)
 {
 	int i;
-	static uint64_t decr = 0x3FFFFF; /* ~10ms */
 
 	if (argc > 2)
 		report_abort("Unsupported argument: '%s'", argv[2]);
 
-	handle_exception(0x900, &dec_except_handler, &decr);
-	asm volatile ("mtdec %0" : : "r" (decr));
-
 	if (!start_all_cpus(halt, 0))
 		report_abort("Failed to start secondary cpus");
 
@@ -120,10 +105,7 @@  static void test_h_cede_tm(int argc, char **argv)
 		      "bf 2,1b" : : : "cr0");
 
 	for (i = 0; i < 500; i++) {
-		uint64_t rval = h_cede();
-
-		if (rval != H_SUCCESS)
-			break;
+		msleep(10);
 		mdelay(5);
 	}