[kvm-unit-tests v2 00/10] powerpc: updates, P10, PNV support

[kvm-unit-tests v2 00/10] powerpc: updates, P10, PNV support

[Nicholas Piggin]

Since v1 series, I fixed the sleep API and implementation in patch 2
as noted by Thomas. Added usleep and msleep variants to match [um]delay
we already have.

Also some minor tidy ups and fixes mainly with reporting format in the
sprs test rework.

And added PowerNV support to the harness with the 3 new patches at the
end because it didn't turn out to be too hard. We could parse the dt to
get a console UART directly for a really minimal firmware, but it is
better for us to have a test harness like this that can also be used for
skiboot testing.

Thanks,
Nick

Nicholas Piggin (10):
  MAINTAINERS: Update powerpc list
  powerpc: add local variant of SPR test
  powerpc: abstract H_CEDE calls into a sleep functions
  powerpc: Add ISA v3.1 (POWER10) support to SPR test
  powerpc: Indirect SPR accessor functions
  powerpc/sprs: Specify SPRs with data rather than code
  powerpc/spapr_vpa: Add basic VPA tests
  powerpc: Discover runtime load address dynamically
  powerpc: Support powernv machine with QEMU TCG
  powerpc/sprs: Test hypervisor registers on powernv machine

 MAINTAINERS                 |   2 +-
 lib/linux/compiler.h        |   2 +
 lib/powerpc/asm/handlers.h  |   2 +-
 lib/powerpc/asm/hcall.h     |   1 +
 lib/powerpc/asm/ppc_asm.h   |   6 +
 lib/powerpc/asm/processor.h |  21 ++
 lib/powerpc/handlers.c      |  10 +-
 lib/powerpc/hcall.c         |   4 +-
 lib/powerpc/io.c            |  33 +-
 lib/powerpc/io.h            |   6 +
 lib/powerpc/processor.c     |  52 +++
 lib/powerpc/setup.c         |  10 +-
 lib/ppc64/asm/opal.h        |  11 +
 lib/ppc64/asm/vpa.h         |  62 ++++
 lib/ppc64/opal-calls.S      |  46 +++
 lib/ppc64/opal.c            |  67 ++++
 powerpc/Makefile.ppc64      |   4 +-
 powerpc/cstart64.S          |  26 +-
 powerpc/run                 |  30 +-
 powerpc/spapr_vpa.c         |  90 +++++
 powerpc/sprs.c              | 641 ++++++++++++++++++++++++++----------
 powerpc/tm.c                |  20 +-
 powerpc/unittests.cfg       |   4 +
 23 files changed, 925 insertions(+), 225 deletions(-)
 create mode 100644 lib/ppc64/asm/opal.h
 create mode 100644 lib/ppc64/asm/vpa.h
 create mode 100644 lib/ppc64/opal-calls.S
 create mode 100644 lib/ppc64/opal.c
 create mode 100644 powerpc/spapr_vpa.c

-- 
2.37.2

[kvm-unit-tests v2 01/10] MAINTAINERS: Update powerpc list

[Nicholas Piggin]

KVM development on powerpc has moved to the Linux on Power mailing list,
as per linux.git commit 19b27f37ca97d ("MAINTAINERS: Update powerpc KVM
entry").

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 649de50..b545a45 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -79,7 +79,7 @@ M: Laurent Vivier <lvivier@redhat.com>
 M: Thomas Huth <thuth@redhat.com>
 S: Maintained
 L: kvm@vger.kernel.org
-L: kvm-ppc@vger.kernel.org
+L: linuxppc-dev@lists.ozlabs.org
 F: powerpc/
 F: lib/powerpc/
 F: lib/ppc64/
-- 
2.37.2

Re: [kvm-unit-tests v2 01/10] MAINTAINERS: Update powerpc list

[Thomas Huth]

On 20/03/2023 08.03, Nicholas Piggin wrote:
> KVM development on powerpc has moved to the Linux on Power mailing list,
> as per linux.git commit 19b27f37ca97d ("MAINTAINERS: Update powerpc KVM
> entry").
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   MAINTAINERS | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 649de50..b545a45 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -79,7 +79,7 @@ M: Laurent Vivier <lvivier@redhat.com>
>   M: Thomas Huth <thuth@redhat.com>
>   S: Maintained
>   L: kvm@vger.kernel.org
> -L: kvm-ppc@vger.kernel.org
> +L: linuxppc-dev@lists.ozlabs.org
>   F: powerpc/
>   F: lib/powerpc/
>   F: lib/ppc64/

Reviewed-by: Thomas Huth <thuth@redhat.com>

[kvm-unit-tests v2 02/10] powerpc: add local variant of SPR test

[Nicholas Piggin]

This adds the non-migration variant of the SPR test to the matrix,
which can be simpler to run and debug.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 powerpc/unittests.cfg | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index 1e74948..3e41598 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -68,5 +68,9 @@ groups = h_cede_tm
 
 [sprs]
 file = sprs.elf
+groups = sprs
+
+[sprs-migration]
+file = sprs.elf
 extra_params = -append '-w'
 groups = migration
-- 
2.37.2

Re: [kvm-unit-tests v2 02/10] powerpc: add local variant of SPR test

[Thomas Huth]

On 20/03/2023 08.03, Nicholas Piggin wrote:
> This adds the non-migration variant of the SPR test to the matrix,
> which can be simpler to run and debug.
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   powerpc/unittests.cfg | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
> index 1e74948..3e41598 100644
> --- a/powerpc/unittests.cfg
> +++ b/powerpc/unittests.cfg
> @@ -68,5 +68,9 @@ groups = h_cede_tm
>   
>   [sprs]
>   file = sprs.elf
> +groups = sprs

Looking at this again, I think you don't really need a "groups =" entry here 
... I'd suggest to drop that line.

  Thomas


> +[sprs-migration]
> +file = sprs.elf
>   extra_params = -append '-w'
>   groups = migration

Re: [kvm-unit-tests v2 02/10] powerpc: add local variant of SPR test

[Nicholas Piggin]

On Thu Mar 23, 2023 at 9:26 PM AEST, Thomas Huth wrote:
> On 20/03/2023 08.03, Nicholas Piggin wrote:
> > This adds the non-migration variant of the SPR test to the matrix,
> > which can be simpler to run and debug.
> > 
> > Reviewed-by: Thomas Huth <thuth@redhat.com>
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   powerpc/unittests.cfg | 4 ++++
> >   1 file changed, 4 insertions(+)
> > 
> > diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
> > index 1e74948..3e41598 100644
> > --- a/powerpc/unittests.cfg
> > +++ b/powerpc/unittests.cfg
> > @@ -68,5 +68,9 @@ groups = h_cede_tm
> >   
> >   [sprs]
> >   file = sprs.elf
> > +groups = sprs
>
> Looking at this again, I think you don't really need a "groups =" entry here 
> ... I'd suggest to drop that line.

Okay, will do.

Thanks,
Nick

[kvm-unit-tests v2 03/10] powerpc: abstract H_CEDE calls into a sleep functions

[Nicholas Piggin]

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     | 42 +++++++++++++++++++++++++++++++++++++
 powerpc/sprs.c              |  6 +-----
 powerpc/tm.c                | 20 +-----------------
 7 files changed, 57 insertions(+), 31 deletions(-)

diff --git a/lib/powerpc/asm/handlers.h b/lib/powerpc/asm/handlers.h
index 64ba727..e4a0cd4 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 1b85f6b..6299ff5 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 ac001e1..ebfeff2 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 c8721e0..296f14f 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 ec85b9d..e77a240 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);
@@ -54,3 +56,43 @@ 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;
+
+		/* Could support large decrementer */
+		if (left > 0x7fffffff)
+			left = 0x7fffffff;
+
+		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 5cc1cd1..ba4ddee 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 65cacdf..7fa9163 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);
 	}
 
-- 
2.37.2

Re: [kvm-unit-tests v2 03/10] powerpc: abstract H_CEDE calls into a sleep functions

[Thomas Huth]

On 20/03/2023 08.03, 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     | 42 +++++++++++++++++++++++++++++++++++++
>   powerpc/sprs.c              |  6 +-----
>   powerpc/tm.c                | 20 +-----------------
>   7 files changed, 57 insertions(+), 31 deletions(-)
> 
> diff --git a/lib/powerpc/asm/handlers.h b/lib/powerpc/asm/handlers.h
> index 64ba727..e4a0cd4 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 1b85f6b..6299ff5 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 ac001e1..ebfeff2 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 c8721e0..296f14f 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 ec85b9d..e77a240 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);
> @@ -54,3 +56,43 @@ 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;
> +
> +		/* Could support large decrementer */
> +		if (left > 0x7fffffff)
> +			left = 0x7fffffff;
> +
> +		asm volatile ("mtdec %0" : : "r" (left));
> +		handle_exception(0x900, &dec_handler_oneshot, NULL);

Wouldn't it be better to first call handle_exception() before moving 
something into the decrementer?

> +		/*
> +		 * 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();
> +	}
> +}

  Thomas

Re: [kvm-unit-tests v2 03/10] powerpc: abstract H_CEDE calls into a sleep functions

[Nicholas Piggin]

On Thu Mar 23, 2023 at 10:12 PM AEST, Thomas Huth wrote:
> On 20/03/2023 08.03, 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     | 42 +++++++++++++++++++++++++++++++++++++
> >   powerpc/sprs.c              |  6 +-----
> >   powerpc/tm.c                | 20 +-----------------
> >   7 files changed, 57 insertions(+), 31 deletions(-)
> > 
> > diff --git a/lib/powerpc/asm/handlers.h b/lib/powerpc/asm/handlers.h
> > index 64ba727..e4a0cd4 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 1b85f6b..6299ff5 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 ac001e1..ebfeff2 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 c8721e0..296f14f 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 ec85b9d..e77a240 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);
> > @@ -54,3 +56,43 @@ 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;
> > +
> > +		/* Could support large decrementer */
> > +		if (left > 0x7fffffff)
> > +			left = 0x7fffffff;
> > +
> > +		asm volatile ("mtdec %0" : : "r" (left));
> > +		handle_exception(0x900, &dec_handler_oneshot, NULL);
>
> Wouldn't it be better to first call handle_exception() before moving 
> something into the decrementer?

It shouldn't really matter, the drecrementer could be anything here
(probably it's already -ve here if it starts at zero). It only matters
when MSR[EE]=1 or we make the H_CEDE call.

I'll add a comment at least.

Thanks,
Nick

[kvm-unit-tests v2 04/10] powerpc: Add ISA v3.1 (POWER10) support to SPR test

[Nicholas Piggin]

This is a very basic detection that does not include all new SPRs.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 powerpc/sprs.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index ba4ddee..6ee6dba 100644
--- a/powerpc/sprs.c
+++ b/powerpc/sprs.c
@@ -117,6 +117,15 @@ static void set_sprs_book3s_300(uint64_t val)
 	mtspr(823, val);	/* PSSCR */
 }
 
+/* SPRs from Power ISA Version 3.1B */
+static void set_sprs_book3s_31(uint64_t val)
+{
+	set_sprs_book3s_207(val);
+	mtspr(48, val);		/* PIDR */
+	/* 3.1 removes TIDR */
+	mtspr(823, val);	/* PSSCR */
+}
+
 static void set_sprs(uint64_t val)
 {
 	uint32_t pvr = mfspr(287);	/* Processor Version Register */
@@ -137,6 +146,9 @@ static void set_sprs(uint64_t val)
 	case 0x4e:			/* POWER9 */
 		set_sprs_book3s_300(val);
 		break;
+	case 0x80:                      /* POWER10 */
+		set_sprs_book3s_31(val);
+		break;
 	default:
 		puts("Warning: Unknown processor version!\n");
 	}
@@ -220,6 +232,13 @@ static void get_sprs_book3s_300(uint64_t *v)
 	v[823] = mfspr(823);	/* PSSCR */
 }
 
+static void get_sprs_book3s_31(uint64_t *v)
+{
+	get_sprs_book3s_207(v);
+	v[48] = mfspr(48);	/* PIDR */
+	v[823] = mfspr(823);	/* PSSCR */
+}
+
 static void get_sprs(uint64_t *v)
 {
 	uint32_t pvr = mfspr(287);	/* Processor Version Register */
@@ -240,6 +259,9 @@ static void get_sprs(uint64_t *v)
 	case 0x4e:			/* POWER9 */
 		get_sprs_book3s_300(v);
 		break;
+	case 0x80:                      /* POWER10 */
+		get_sprs_book3s_31(v);
+		break;
 	}
 }
 
-- 
2.37.2

Re: [kvm-unit-tests v2 04/10] powerpc: Add ISA v3.1 (POWER10) support to SPR test

[Thomas Huth]

On 20/03/2023 08.03, Nicholas Piggin wrote:
> This is a very basic detection that does not include all new SPRs.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   powerpc/sprs.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> index ba4ddee..6ee6dba 100644
> --- a/powerpc/sprs.c
> +++ b/powerpc/sprs.c
> @@ -117,6 +117,15 @@ static void set_sprs_book3s_300(uint64_t val)
>   	mtspr(823, val);	/* PSSCR */
>   }
>   
> +/* SPRs from Power ISA Version 3.1B */
> +static void set_sprs_book3s_31(uint64_t val)
> +{
> +	set_sprs_book3s_207(val);
> +	mtspr(48, val);		/* PIDR */
> +	/* 3.1 removes TIDR */
> +	mtspr(823, val);	/* PSSCR */
> +}
> +
>   static void set_sprs(uint64_t val)
>   {
>   	uint32_t pvr = mfspr(287);	/* Processor Version Register */
> @@ -137,6 +146,9 @@ static void set_sprs(uint64_t val)
>   	case 0x4e:			/* POWER9 */
>   		set_sprs_book3s_300(val);
>   		break;
> +	case 0x80:                      /* POWER10 */
> +		set_sprs_book3s_31(val);
> +		break;
>   	default:
>   		puts("Warning: Unknown processor version!\n");
>   	}
> @@ -220,6 +232,13 @@ static void get_sprs_book3s_300(uint64_t *v)
>   	v[823] = mfspr(823);	/* PSSCR */
>   }
>   
> +static void get_sprs_book3s_31(uint64_t *v)
> +{
> +	get_sprs_book3s_207(v);
> +	v[48] = mfspr(48);	/* PIDR */
> +	v[823] = mfspr(823);	/* PSSCR */
> +}
> +
>   static void get_sprs(uint64_t *v)
>   {
>   	uint32_t pvr = mfspr(287);	/* Processor Version Register */
> @@ -240,6 +259,9 @@ static void get_sprs(uint64_t *v)
>   	case 0x4e:			/* POWER9 */
>   		get_sprs_book3s_300(v);
>   		break;
> +	case 0x80:                      /* POWER10 */
> +		get_sprs_book3s_31(v);
> +		break;
>   	}
>   }

Looks like I accidentally replied to v1 a couple of minutes ago... I meant 
to reply here:

Reviewed-by: Thomas Huth <thuth@redhat.com>

[kvm-unit-tests v2 05/10] powerpc: Indirect SPR accessor functions

[Nicholas Piggin]

Make overly-clever SPR accessor functions that allow a non-constant
SPR number to be specified. This will be used to restructure test
in the next change.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 powerpc/sprs.c | 63 ++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 54 insertions(+), 9 deletions(-)

diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index 6ee6dba..db341a9 100644
--- a/powerpc/sprs.c
+++ b/powerpc/sprs.c
@@ -28,21 +28,66 @@
 #include <asm/processor.h>
 #include <asm/barrier.h>
 
-#define mfspr(nr) ({ \
-	uint64_t ret; \
-	asm volatile("mfspr %0,%1" : "=r"(ret) : "i"(nr)); \
-	ret; \
-})
+/* "Indirect" mfspr/mtspr which accept a non-constant spr number */
+static uint64_t mfspr(unsigned spr)
+{
+	uint64_t tmp;
+	uint64_t ret;
+
+	asm volatile(
+"	bcl	20, 31, 1f		\n"
+"1:	mflr	%0			\n"
+"	addi	%0, %0, (2f-1b)		\n"
+"	add	%0, %0, %2		\n"
+"	mtctr	%0			\n"
+"	bctr				\n"
+"2:					\n"
+".LSPR=0				\n"
+".rept 1024				\n"
+"	mfspr	%1, .LSPR		\n"
+"	b	3f			\n"
+"	.LSPR=.LSPR+1			\n"
+".endr					\n"
+"3:					\n"
+	: "=&r"(tmp),
+	  "=r"(ret)
+	: "r"(spr*8) /* 8 bytes per 'mfspr ; b' block */
+	: "lr", "ctr");
+
+	return ret;
+}
 
-#define mtspr(nr, val) \
-	asm volatile("mtspr %0,%1" : : "i"(nr), "r"(val))
+static void mtspr(unsigned spr, uint64_t val)
+{
+	uint64_t tmp;
+
+	asm volatile(
+"	bcl	20, 31, 1f		\n"
+"1:	mflr	%0			\n"
+"	addi	%0, %0, (2f-1b)		\n"
+"	add	%0, %0, %2		\n"
+"	mtctr	%0			\n"
+"	bctr				\n"
+"2:					\n"
+".LSPR=0				\n"
+".rept 1024				\n"
+"	mtspr	.LSPR, %1		\n"
+"	b	3f			\n"
+"	.LSPR=.LSPR+1			\n"
+".endr					\n"
+"3:					\n"
+	: "=&r"(tmp)
+	: "r"(val),
+	  "r"(spr*8) /* 8 bytes per 'mfspr ; b' block */
+	: "lr", "ctr", "xer");
+}
 
 uint64_t before[1024], after[1024];
 
 /* Common SPRs for all PowerPC CPUs */
 static void set_sprs_common(uint64_t val)
 {
-	mtspr(9, val);		/* CTR */
+	// mtspr(9, val);	/* CTR */ /* Used by mfspr/mtspr */
 	// mtspr(273, val);	/* SPRG1 */  /* Used by our exception handler */
 	mtspr(274, val);	/* SPRG2 */
 	mtspr(275, val);	/* SPRG3 */
@@ -156,7 +201,7 @@ static void set_sprs(uint64_t val)
 
 static void get_sprs_common(uint64_t *v)
 {
-	v[9] = mfspr(9);	/* CTR */
+	v[9] = mfspr(9);	/* CTR */ /* Used by mfspr/mtspr */
 	// v[273] = mfspr(273);	/* SPRG1 */ /* Used by our exception handler */
 	v[274] = mfspr(274);	/* SPRG2 */
 	v[275] = mfspr(275);	/* SPRG3 */
-- 
2.37.2

[kvm-unit-tests v2 06/10] powerpc/sprs: Specify SPRs with data rather than code

[Nicholas Piggin]

A significant rework that builds an array of 'struct spr', where each
element describes an SPR. This makes various metadata about the SPR
like name and access type easier to carry and use.

Hypervisor privileged registers are described despite not being used
at the moment for completeness, but also the code might one day be
reused for a hypervisor-privileged test.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

This ended up a little over-engineered perhaps, but there are lots of
SPRs, lots of access types, lots of changes between processor and ISA
versions, and lots of places they are implemented and used, so lots of
room for mistakes. There is not a good system in place to easily
see that userspace, supervisor, etc., switches perform all the right
SPR context switching so this is a nice test case to have. The sprs test
quickly caught a few QEMU TCG SPR bugs which really motivated me to
improve the SPR coverage.
---
 powerpc/sprs.c | 589 +++++++++++++++++++++++++++++++++----------------
 1 file changed, 394 insertions(+), 195 deletions(-)

diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index db341a9..dd83dac 100644
--- a/powerpc/sprs.c
+++ b/powerpc/sprs.c
@@ -82,231 +82,407 @@ static void mtspr(unsigned spr, uint64_t val)
 	: "lr", "ctr", "xer");
 }
 
-uint64_t before[1024], after[1024];
+static uint64_t before[1024], after[1024];
 
-/* Common SPRs for all PowerPC CPUs */
-static void set_sprs_common(uint64_t val)
-{
-	// mtspr(9, val);	/* CTR */ /* Used by mfspr/mtspr */
-	// mtspr(273, val);	/* SPRG1 */  /* Used by our exception handler */
-	mtspr(274, val);	/* SPRG2 */
-	mtspr(275, val);	/* SPRG3 */
-}
+#define SPR_PR_READ	0x0001
+#define SPR_PR_WRITE	0x0002
+#define SPR_OS_READ	0x0010
+#define SPR_OS_WRITE	0x0020
+#define SPR_HV_READ	0x0100
+#define SPR_HV_WRITE	0x0200
+
+#define RW		0x333
+#define RO		0x111
+#define WO		0x222
+#define OS_RW		0x330
+#define OS_RO		0x110
+#define OS_WO		0x220
+#define HV_RW		0x300
+#define HV_RO		0x100
+#define HV_WO		0x200
+
+#define SPR_ASYNC	0x1000	/* May be updated asynchronously */
+#define SPR_INT		0x2000	/* May be updated by synchronous interrupt */
+#define SPR_HARNESS	0x4000	/* Test harness uses the register */
+
+struct spr {
+	const char	*name;
+	uint8_t		width;
+	uint16_t	access;
+	uint16_t	type;
+};
+
+/* SPRs common denominator back to PowerPC Operating Environment Architecture */
+static const struct spr sprs_common[1024] = {
+  [1] = {"XER",		64,	RW,		SPR_HARNESS, }, /* Compiler */
+  [8] = {"LR", 		64,	RW,		SPR_HARNESS, }, /* Compiler, mfspr/mtspr */
+  [9] = {"CTR",		64,	RW,		SPR_HARNESS, }, /* Compiler, mfspr/mtspr */
+ [18] = {"DSISR",	32,	OS_RW,		SPR_INT, },
+ [19] = {"DAR",		64,	OS_RW,		SPR_INT, },
+ [26] = {"SRR0",	64,	OS_RW,		SPR_INT, },
+ [27] = {"SRR1",	64,	OS_RW,		SPR_INT, },
+[268] = {"TB",		64,	RO	,	SPR_ASYNC, },
+[269] = {"TBU",		32,	RO,		SPR_ASYNC, },
+[272] = {"SPRG0",	64,	OS_RW,		SPR_HARNESS, }, /* Int stack */
+[273] = {"SPRG1",	64,	OS_RW,		SPR_HARNESS, }, /* Scratch */
+[274] = {"SPRG2",	64,	OS_RW, },
+[275] = {"SPRG3",	64,	OS_RW, },
+[287] = {"PVR",		32,	OS_RO, },
+};
 
 /* SPRs from PowerPC Operating Environment Architecture, Book III, Vers. 2.01 */
-static void set_sprs_book3s_201(uint64_t val)
-{
-	mtspr(18, val);		/* DSISR */
-	mtspr(19, val);		/* DAR */
-	mtspr(152, val);	/* CTRL */
-	mtspr(256, val);	/* VRSAVE */
-	mtspr(786, val);	/* MMCRA */
-	mtspr(795, val);	/* MMCR0 */
-	mtspr(798, val);	/* MMCR1 */
-}
+static const struct spr sprs_201[1024] = {
+ [22] = {"DEC",		32,	OS_RW,		SPR_ASYNC, },
+ [25] = {"SDR1",	64,	HV_RW | OS_RO, },
+ [29] = {"ACCR",	64,	OS_RW, },
+[136] = {"CTRL",	32,	RO, },
+[152] = {"CTRL",	32,	OS_WO, },
+[259] = {"SPRG3",	64,	RO, },
+/* ASR, EAR omitted */
+[268] = {"TB",		64,	RO, },
+[269] = {"TBU",		32,	RO, },
+[284] = {"TBL",		32,	HV_WO, },
+[285] = {"TBU",		32,	HV_WO, },
+[310] = {"HDEC",	32,	HV_RW, },
+[1013]= {"DABR",	64,	HV_RW | OS_RO, },
+[1023]= {"PIR",		32,	OS_RO, },
+};
+
+static const struct spr sprs_970_pmu[1024] = {
+/* POWER4+ PMU, should find PPC970 and confirm */
+[770] = {"MMCRA",	64,	RO, },
+[771] = {"PMC1",	32,	RO, },
+[772] = {"PMC2",	32,	RO, },
+[773] = {"PMC3",	32,	RO, },
+[774] = {"PMC4",	32,	RO, },
+[775] = {"PMC5",	32,	RO, },
+[776] = {"PMC6",	32,	RO, },
+[777] = {"PMC7",	32,	RO, },
+[778] = {"PMC8",	32,	RO, },
+[779] = {"MMCR0",	64,	RO, },
+[780] = {"SIAR",	64,	RO, },
+[781] = {"SDAR",	64,	RO, },
+[782] = {"MMCR1",	64,	RO, },
+[786] = {"MMCRA",	64,	OS_RW, },
+[787] = {"PMC1",	32,	OS_RW, },
+[788] = {"PMC2",	32,	OS_RW, },
+[789] = {"PMC3",	32,	OS_RW, },
+[790] = {"PMC4",	32,	OS_RW, },
+[791] = {"PMC5",	32,	OS_RW, },
+[792] = {"PMC6",	32,	OS_RW, },
+[793] = {"PMC7",	32,	OS_RW, },
+[794] = {"PMC8",	32,	OS_RW, },
+[795] = {"MMCR0",	64,	OS_RW, },
+[796] = {"SIAR",	64,	OS_RW, },
+[797] = {"SDAR",	64,	OS_RW, },
+[798] = {"MMCR1",	64,	OS_RW, },
+};
+
+/* These are common SPRs from 2.07S onward (POWER CPUs that support KVM HV) */
+static const struct spr sprs_power_common[1024] = {
+  [3] = {"DSCR",	64,	RW, },
+ [13] = {"AMR",		64,	RW, },
+ [17] = {"DSCR",	64,	OS_RW, },
+ [28] = {"CFAR",	64,	OS_RW,		SPR_ASYNC, }, /* Effectively async */
+ [29] = {"AMR",		64,	OS_RW, },
+ [61] = {"IAMR",	64,	OS_RW, },
+[136] = {"CTRL",	32,	RO, },
+[152] = {"CTRL",	32,	OS_WO, },
+[153] = {"FSCR",	64,	OS_RW, },
+[157] = {"UAMOR",	64,	OS_RW, },
+[159] = {"PSPB",	32,	OS_RW, },
+[176] = {"DPDES",	64,	HV_RW | OS_RO, },
+[180] = {"DAWR0",	64,	HV_RW, },
+[186] = {"RPR",		64,	HV_RW, },
+[187] = {"CIABR",	64,	HV_RW, },
+[188] = {"DAWRX0",	32,	HV_RW, },
+[190] = {"HFSCR",	64,	HV_RW, },
+[256] = {"VRSAVE",	32,	RW, },
+[259] = {"SPRG3",	64,	RO, },
+[284] = {"TBL",		32,	HV_WO, },
+[285] = {"TBU",		32,	HV_WO, },
+[286] = {"TBU40",	64,	HV_WO, },
+[304] = {"HSPRG0",	64,	HV_RW, },
+[305] = {"HSPRG1",	64,	HV_RW, },
+[306] = {"HDSISR",	32,	HV_RW,		SPR_INT, },
+[307] = {"HDAR",	64,	HV_RW,		SPR_INT, },
+[308] = {"SPURR",	64,	HV_RW | OS_RO,	SPR_ASYNC, },
+[309] = {"PURR",	64,	HV_RW | OS_RO,	SPR_ASYNC, },
+[313] = {"HRMOR",	64,	HV_RW, },
+[314] = {"HSRR0",	64,	HV_RW,		SPR_INT, },
+[315] = {"HSRR1",	64,	HV_RW,		SPR_INT, },
+[318] = {"LPCR",	64,	HV_RW, },
+[319] = {"LPIDR",	32,	HV_RW, },
+[336] = {"HMER",	64,	HV_RW, },
+[337] = {"HMEER",	64,	HV_RW, },
+[338] = {"PCR",		64,	HV_RW, },
+[349] = {"AMOR",	64,	HV_RW, },
+[446] = {"TIR",		64,	OS_RO, },
+[800] = {"BESCRS",	64,	RW, },
+[801] = {"BESCRSU",	32,	RW, },
+[802] = {"BESCRR",	64,	RW, },
+[803] = {"BESCRRU",	32,	RW, },
+[804] = {"EBBHR",	64,	RW, },
+[805] = {"EBBRR",	64,	RW, },
+[806] = {"BESCR",	64,	RW, },
+[815] = {"TAR",		64,	RW, },
+[848] = {"IC",		64,	HV_RW | OS_RO,	SPR_ASYNC, },
+[849] = {"VTB",		64,	HV_RW | OS_RO,	SPR_ASYNC, },
+[896] = {"PPR",		64,	RW, },
+[898] = {"PPR32",	32,	RW, },
+[1023]= {"PIR",		32,	OS_RO, },
+};
+
+static const struct spr sprs_tm[1024] = {
+#if 0
+	/* XXX: leave these out until enabling TM facility (and more testing) */
+[128] = {"TFHAR",	64,	RW, },
+[129] = {"TFIAR",	64,	RW, },
+[130] = {"TEXASR",	64,	RW, },
+[131] = {"TEXASRU",	32,	RW, },
+#endif
+};
 
 /* SPRs from PowerISA 2.07 Book III-S */
-static void set_sprs_book3s_207(uint64_t val)
-{
-	mtspr(3, val);		/* DSCR */
-	mtspr(13, val);		/* AMR */
-	mtspr(17, val);		/* DSCR */
-	mtspr(18, val);		/* DSISR */
-	mtspr(19, val);		/* DAR */
-	mtspr(29, val);		/* AMR */
-	mtspr(61, val);		/* IAMR */
-	// mtspr(152, val);	/* CTRL */  /* TODO: Needs a fix in KVM */
-	mtspr(153, val);	/* FSCR */
-	mtspr(157, val);	/* UAMOR */
-	mtspr(159, val);	/* PSPB */
-	mtspr(256, val);	/* VRSAVE */
-	// mtspr(272, val);	/* SPRG0 */ /* Used by our exception handler */
-	mtspr(769, val);	/* MMCR2 */
-	mtspr(770, val);	/* MMCRA */
-	mtspr(771, val);	/* PMC1 */
-	mtspr(772, val);	/* PMC2 */
-	mtspr(773, val);	/* PMC3 */
-	mtspr(774, val);	/* PMC4 */
-	mtspr(775, val);	/* PMC5 */
-	mtspr(776, val);	/* PMC6 */
-	mtspr(779, (val & 0xfffffffffbab3fffULL) | 0xfa0b2070);	/* MMCR0 */
-	mtspr(784, val);	/* SIER */
-	mtspr(785, val);	/* MMCR2 */
-	mtspr(786, val);	/* MMCRA */
-	mtspr(787, val);	/* PMC1 */
-	mtspr(788, val);	/* PMC2 */
-	mtspr(789, val);	/* PMC3 */
-	mtspr(790, val);	/* PMC4 */
-	mtspr(791, val);	/* PMC5 */
-	mtspr(792, val);	/* PMC6 */
-	mtspr(795, (val & 0xfffffffffbab3fffULL) | 0xfa0b2070);	/* MMCR0 */
-	mtspr(796, val);	/* SIAR */
-	mtspr(797, val);	/* SDAR */
-	mtspr(798, val);	/* MMCR1 */
-	mtspr(800, val);	/* BESCRS */
-	mtspr(801, val);	/* BESCCRSU */
-	mtspr(802, val);	/* BESCRR */
-	mtspr(803, val);	/* BESCRRU */
-	mtspr(804, val);	/* EBBHR */
-	mtspr(805, val);	/* EBBRR */
-	mtspr(806, val);	/* BESCR */
-	mtspr(815, val);	/* TAR */
-}
+static const struct spr sprs_207[1024] = {
+ [22] = {"DEC",		32,	OS_RW,		SPR_ASYNC, },
+ [25] = {"SDR1",	64,	HV_RW, },
+[177] = {"DHDES",	64,	HV_RW, },
+[283] = {"CIR",		32,	OS_RO, },
+[310] = {"HDEC",	32,	HV_RW,		SPR_ASYNC, },
+[312] = {"RMOR",	64,	HV_RW, },
+[339] = {"HEIR",	32,	HV_RW, },
+};
 
 /* SPRs from PowerISA 3.00 Book III */
-static void set_sprs_book3s_300(uint64_t val)
-{
-	set_sprs_book3s_207(val);
-	mtspr(48, val);		/* PIDR */
-	mtspr(144, val);	/* TIDR */
-	mtspr(823, val);	/* PSSCR */
-}
+static const struct spr sprs_300[1024] = {
+ [22] = {"DEC",		64,	OS_RW,		SPR_ASYNC, },
+ [48] = {"PIDR",	32,	OS_RW, },
+[144] = {"TIDR",	64,	OS_RW, },
+[283] = {"CIR",		32,	OS_RO, },
+[310] = {"HDEC",	64,	HV_RW,		SPR_ASYNC, },
+[339] = {"HEIR",	32,	HV_RW, },
+[464] = {"PTCR",	64,	HV_RW, },
+[816] = {"ASDR",	64,	HV_RW,		SPR_INT},
+[823] = {"PSSCR",	64,	OS_RW, },
+[855] = {"PSSCR",	64,	HV_RW, },
+};
 
-/* SPRs from Power ISA Version 3.1B */
-static void set_sprs_book3s_31(uint64_t val)
-{
-	set_sprs_book3s_207(val);
-	mtspr(48, val);		/* PIDR */
-	/* 3.1 removes TIDR */
-	mtspr(823, val);	/* PSSCR */
-}
+/* SPRs from PowerISA 3.1B Book III */
+static const struct spr sprs_31[1024] = {
+ [22] = {"DEC",		64,	OS_RW,		SPR_ASYNC, },
+ [48] = {"PIDR",	32,	OS_RW, },
+[181] = {"DAWR1",	64,	HV_RW, },
+[189] = {"DAWRX1",	32,	HV_RW, },
+[310] = {"HDEC",	64,	HV_RW,		SPR_ASYNC, },
+[339] = {"HEIR",	64,	HV_RW, },
+[455] = {"HDEXCR",	32,	RO, },
+[464] = {"PTCR",	64,	HV_RW, },
+[468] = {"HASHKEYR",	64,	OS_RW, },
+[469] = {"HASHPKEYR",	64,	HV_RW, },
+[471] = {"HDEXCR",	64,	HV_RW, },
+[812] = {"DEXCR",	32,	RO, },
+[816] = {"ASDR",	64,	HV_RW,		SPR_INT},
+[823] = {"PSSCR",	64,	OS_RW, },
+[828] = {"DEXCR",	64,	OS_RW, },
+[855] = {"PSSCR",	64,	HV_RW, },
+};
 
-static void set_sprs(uint64_t val)
+/* SPRs POWER10 User Manual */
+static const struct spr sprs_power10[1024] = {
+[276] = {"SPRC",	64,	HV_RW, },
+[266] = {"SPRD",	64,	HV_RW, },
+[317] = {"TFMR",	64,	HV_RW, },
+[799] = {"IMC",		64,	HV_RW, },
+[850] = {"LDBAR",	64,	HV_RO, },
+[851] = {"MMCRC",	32,	HV_RW, },
+[853] = {"PMSR",	32,	HV_RO, },
+[861] = {"L2QOSR",	64,	HV_WO, },
+[881] = {"TRIG1",	64,	OS_WO, },
+[882] = {"TRIG2",	64,	OS_WO, },
+[884] = {"PMCR",	64,	HV_RW, },
+[885] = {"RWMR",	64,	HV_RW, },
+[895] = {"WORT",	64,	OS_RW, }, /* UM says 18-bits! */
+[921] = {"TSCR",	32,	HV_RW, },
+[922] = {"TTR",		64,	HV_RW, },
+[1006]= {"TRACE",	64,	WO, },
+[1008]= {"HID",		64,	HV_RW, },
+};
+
+/* This covers POWER8 and POWER9 PMUs */
+static const struct spr sprs_power_common_pmu[1024] = {
+[768] = {"SIER",	64,	RO, },
+[769] = {"MMCR2",	64,	RW, },
+[770] = {"MMCRA",	64,	RW, },
+[771] = {"PMC1",	32,	RW, },
+[772] = {"PMC2",	32,	RW, },
+[773] = {"PMC3",	32,	RW, },
+[774] = {"PMC4",	32,	RW, },
+[775] = {"PMC5",	32,	RW, },
+[776] = {"PMC6",	32,	RW, },
+[779] = {"MMCR0",	64,	RW, },
+[780] = {"SIAR",	64,	RO, },
+[781] = {"SDAR",	64,	RO, },
+[782] = {"MMCR1",	64,	RO, },
+[784] = {"SIER",	64,	OS_RW, },
+[785] = {"MMCR2",	64,	OS_RW, },
+[786] = {"MMCRA",	64,	OS_RW, },
+[787] = {"PMC1",	32,	OS_RW, },
+[788] = {"PMC2",	32,	OS_RW, },
+[789] = {"PMC3",	32,	OS_RW, },
+[790] = {"PMC4",	32,	OS_RW, },
+[791] = {"PMC5",	32,	OS_RW, },
+[792] = {"PMC6",	32,	OS_RW, },
+[795] = {"MMCR0",	64,	OS_RW, },
+[796] = {"SIAR",	64,	OS_RW, },
+[797] = {"SDAR",	64,	OS_RW, },
+[798] = {"MMCR1",	64,	OS_RW, },
+};
+
+static const struct spr sprs_power10_pmu[1024] = {
+[736] = {"SEIR2",	64,	RO, },
+[737] = {"SEIR3",	64,	RO, },
+[738] = {"MMCR3",	64,	RO, },
+[752] = {"SEIR2",	64,	OS_RW, },
+[753] = {"SEIR3",	64,	OS_RW, },
+[754] = {"MMCR3",	64,	OS_RW, },
+};
+
+static struct spr sprs[1024];
+
+static void setup_sprs(void)
 {
 	uint32_t pvr = mfspr(287);	/* Processor Version Register */
+	int i;
 
-	set_sprs_common(val);
+	for (i = 0; i < 1024; i++) {
+		if (sprs_common[i].name) {
+			memcpy(&sprs[i], &sprs_common[i], sizeof(struct spr));
+		}
+	}
 
 	switch (pvr >> 16) {
 	case 0x39:			/* PPC970 */
 	case 0x3C:			/* PPC970FX */
 	case 0x44:			/* PPC970MP */
-		set_sprs_book3s_201(val);
+		for (i = 0; i < 1024; i++) {
+			if (sprs_power_common[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_power_common[i], sizeof(struct spr));
+			}
+			if (sprs_201[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_201[i], sizeof(struct spr));
+			}
+			if (sprs_970_pmu[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_power_common_pmu[i], sizeof(struct spr));
+			}
+		}
 		break;
+
 	case 0x4b:			/* POWER8E */
 	case 0x4c:			/* POWER8NVL */
 	case 0x4d:			/* POWER8 */
-		set_sprs_book3s_207(val);
+		for (i = 0; i < 1024; i++) {
+			if (sprs_power_common[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_power_common[i], sizeof(struct spr));
+			}
+			if (sprs_207[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_207[i], sizeof(struct spr));
+			}
+			if (sprs_tm[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_tm[i], sizeof(struct spr));
+			}
+			if (sprs_power_common_pmu[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_power_common_pmu[i], sizeof(struct spr));
+			}
+		}
 		break;
+
 	case 0x4e:			/* POWER9 */
-		set_sprs_book3s_300(val);
+		for (i = 0; i < 1024; i++) {
+			if (sprs_power_common[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_power_common[i], sizeof(struct spr));
+			}
+			if (sprs_300[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_300[i], sizeof(struct spr));
+			}
+			if (sprs_tm[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_tm[i], sizeof(struct spr));
+			}
+			if (sprs_power_common_pmu[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_power_common_pmu[i], sizeof(struct spr));
+			}
+		}
 		break;
-	case 0x80:                      /* POWER10 */
-		set_sprs_book3s_31(val);
+
+	case 0x80:			/* POWER10 */
+		for (i = 0; i < 1024; i++) {
+			if (sprs_power_common[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_power_common[i], sizeof(struct spr));
+			}
+			if (sprs_31[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_31[i], sizeof(struct spr));
+			}
+			if (sprs_power10[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_power10[i], sizeof(struct spr));
+			}
+			if (sprs_power_common_pmu[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_power_common_pmu[i], sizeof(struct spr));
+			}
+			if (sprs_power10_pmu[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_power10_pmu[i], sizeof(struct spr));
+			}
+		}
 		break;
+
 	default:
-		puts("Warning: Unknown processor version!\n");
+		memcpy(sprs, sprs_common, sizeof(sprs));
+		puts("Warning: Unknown processor version, falling back to common SPRs!\n");
+		break;
 	}
 }
 
-static void get_sprs_common(uint64_t *v)
-{
-	v[9] = mfspr(9);	/* CTR */ /* Used by mfspr/mtspr */
-	// v[273] = mfspr(273);	/* SPRG1 */ /* Used by our exception handler */
-	v[274] = mfspr(274);	/* SPRG2 */
-	v[275] = mfspr(275);	/* SPRG3 */
-}
-
-static void get_sprs_book3s_201(uint64_t *v)
-{
-	v[18] = mfspr(18);	/* DSISR */
-	v[19] = mfspr(19);	/* DAR */
-	v[136] = mfspr(136);	/* CTRL */
-	v[256] = mfspr(256);	/* VRSAVE */
-	v[786] = mfspr(786);	/* MMCRA */
-	v[795] = mfspr(795);	/* MMCR0 */
-	v[798] = mfspr(798);	/* MMCR1 */
-}
-
-static void get_sprs_book3s_207(uint64_t *v)
-{
-	v[3] = mfspr(3);	/* DSCR */
-	v[13] = mfspr(13);	/* AMR */
-	v[17] = mfspr(17);	/* DSCR */
-	v[18] = mfspr(18);	/* DSISR */
-	v[19] = mfspr(19);	/* DAR */
-	v[29] = mfspr(29);	/* AMR */
-	v[61] = mfspr(61);	/* IAMR */
-	// v[136] = mfspr(136);	/* CTRL */  /* TODO: Needs a fix in KVM */
-	v[153] = mfspr(153);	/* FSCR */
-	v[157] = mfspr(157);	/* UAMOR */
-	v[159] = mfspr(159);	/* PSPB */
-	v[256] = mfspr(256);	/* VRSAVE */
-	v[259] = mfspr(259);	/* SPRG3 (read only) */
-	// v[272] = mfspr(272);	/* SPRG0 */  /* Used by our exception handler */
-	v[769] = mfspr(769);	/* MMCR2 */
-	v[770] = mfspr(770);	/* MMCRA */
-	v[771] = mfspr(771);	/* PMC1 */
-	v[772] = mfspr(772);	/* PMC2 */
-	v[773] = mfspr(773);	/* PMC3 */
-	v[774] = mfspr(774);	/* PMC4 */
-	v[775] = mfspr(775);	/* PMC5 */
-	v[776] = mfspr(776);	/* PMC6 */
-	v[779] = mfspr(779);	/* MMCR0 */
-	v[780] = mfspr(780);	/* SIAR (read only) */
-	v[781] = mfspr(781);	/* SDAR (read only) */
-	v[782] = mfspr(782);	/* MMCR1 (read only) */
-	v[784] = mfspr(784);	/* SIER */
-	v[785] = mfspr(785);	/* MMCR2 */
-	v[786] = mfspr(786);	/* MMCRA */
-	v[787] = mfspr(787);	/* PMC1 */
-	v[788] = mfspr(788);	/* PMC2 */
-	v[789] = mfspr(789);	/* PMC3 */
-	v[790] = mfspr(790);	/* PMC4 */
-	v[791] = mfspr(791);	/* PMC5 */
-	v[792] = mfspr(792);	/* PMC6 */
-	v[795] = mfspr(795);	/* MMCR0 */
-	v[796] = mfspr(796);	/* SIAR */
-	v[797] = mfspr(797);	/* SDAR */
-	v[798] = mfspr(798);	/* MMCR1 */
-	v[800] = mfspr(800);	/* BESCRS */
-	v[801] = mfspr(801);	/* BESCCRSU */
-	v[802] = mfspr(802);	/* BESCRR */
-	v[803] = mfspr(803);	/* BESCRRU */
-	v[804] = mfspr(804);	/* EBBHR */
-	v[805] = mfspr(805);	/* EBBRR */
-	v[806] = mfspr(806);	/* BESCR */
-	v[815] = mfspr(815);	/* TAR */
-}
-
-static void get_sprs_book3s_300(uint64_t *v)
+static void get_sprs(uint64_t *v)
 {
-	get_sprs_book3s_207(v);
-	v[48] = mfspr(48);	/* PIDR */
-	v[144] = mfspr(144);	/* TIDR */
-	v[823] = mfspr(823);	/* PSSCR */
-}
+	int i;
 
-static void get_sprs_book3s_31(uint64_t *v)
-{
-	get_sprs_book3s_207(v);
-	v[48] = mfspr(48);	/* PIDR */
-	v[823] = mfspr(823);	/* PSSCR */
+	for (i = 0; i < 1024; i++) {
+		if (!(sprs[i].access & SPR_OS_READ))
+			continue;
+		v[i] = mfspr(i);
+	}
 }
 
-static void get_sprs(uint64_t *v)
+static void set_sprs(uint64_t val)
 {
-	uint32_t pvr = mfspr(287);	/* Processor Version Register */
-
-	get_sprs_common(v);
+	int i;
 
-	switch (pvr >> 16) {
-	case 0x39:			/* PPC970 */
-	case 0x3C:			/* PPC970FX */
-	case 0x44:			/* PPC970MP */
-		get_sprs_book3s_201(v);
-		break;
-	case 0x4b:			/* POWER8E */
-	case 0x4c:			/* POWER8NVL */
-	case 0x4d:			/* POWER8 */
-		get_sprs_book3s_207(v);
-		break;
-	case 0x4e:			/* POWER9 */
-		get_sprs_book3s_300(v);
-		break;
-	case 0x80:                      /* POWER10 */
-		get_sprs_book3s_31(v);
-		break;
+	for (i = 0; i < 1024; i++) {
+		if (!(sprs[i].access & SPR_OS_WRITE))
+			continue;
+		if (sprs[i].type & SPR_HARNESS)
+			continue;
+		if (!strcmp(sprs[i].name, "MMCR0")) {
+			/* XXX: could use a comment or better abstraction! */
+			mtspr(i, (val & 0xfffffffffbab3fffULL) | 0xfa0b2070);
+		} else {
+			mtspr(i, val);
+		}
 	}
 }
 
@@ -343,7 +519,9 @@ int main(int argc, char **argv)
 		}
 	}
 
-	printf("Settings SPRs to %#lx...\n", pat);
+	setup_sprs();
+
+	printf("Setting SPRs to 0x%lx...\n", pat);
 	set_sprs(pat);
 
 	memset(before, 0, sizeof(before));
@@ -355,16 +533,37 @@ int main(int argc, char **argv)
 		migrate_once();
 	} else {
 		msleep(2000);
+
+		/* Taking a dec updates SRR0, SRR1, SPRG1, so don't fail. */
+		sprs[26].type |= SPR_ASYNC;
+		sprs[27].type |= SPR_ASYNC;
+		sprs[273].type |= SPR_ASYNC;
 	}
 
 	get_sprs(after);
 
 	puts("Checking SPRs...\n");
 	for (i = 0; i < 1024; i++) {
-		if (before[i] != 0 || after[i] != 0)
-			report(before[i] == after[i],
-			       "SPR %d:\t%#018lx <==> %#018lx", i, before[i],
-			       after[i]);
+		bool pass = true;
+
+		if (!(sprs[i].access & SPR_OS_READ))
+			continue;
+
+		if (sprs[i].width == 32) {
+			if (before[i] >> 32)
+				pass = false;
+		}
+		if (!(sprs[i].type & SPR_ASYNC) && (before[i] != after[i]))
+			pass = false;
+
+		if (sprs[i].width == 32 && !(before[i] >> 32) && !(after[i] >> 32))
+			report(pass, "%-10s(%4d):\t        0x%08lx <==>         0x%08lx",
+				sprs[i].name, i,
+				before[i], after[i]);
+		else
+			report(pass, "%-10s(%4d):\t0x%016lx <==> 0x%016lx",
+				sprs[i].name, i,
+				before[i], after[i]);
 	}
 
 	return report_summary();
-- 
2.37.2

Re: [kvm-unit-tests v2 06/10] powerpc/sprs: Specify SPRs with data rather than code

[Thomas Huth]

On 20/03/2023 08.03, Nicholas Piggin wrote:
> A significant rework that builds an array of 'struct spr', where each
> element describes an SPR. This makes various metadata about the SPR
> like name and access type easier to carry and use.
> 
> Hypervisor privileged registers are described despite not being used
> at the moment for completeness, but also the code might one day be
> reused for a hypervisor-privileged test.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> 
> This ended up a little over-engineered perhaps, but there are lots of
> SPRs, lots of access types, lots of changes between processor and ISA
> versions, and lots of places they are implemented and used, so lots of
> room for mistakes. There is not a good system in place to easily
> see that userspace, supervisor, etc., switches perform all the right
> SPR context switching so this is a nice test case to have. The sprs test
> quickly caught a few QEMU TCG SPR bugs which really motivated me to
> improve the SPR coverage.
> ---
>   powerpc/sprs.c | 589 +++++++++++++++++++++++++++++++++----------------
>   1 file changed, 394 insertions(+), 195 deletions(-)
> 
> diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> index db341a9..dd83dac 100644
> --- a/powerpc/sprs.c
> +++ b/powerpc/sprs.c
> @@ -82,231 +82,407 @@ static void mtspr(unsigned spr, uint64_t val)
>   	: "lr", "ctr", "xer");
>   }
>   
> -uint64_t before[1024], after[1024];
> +static uint64_t before[1024], after[1024];
>   
> -/* Common SPRs for all PowerPC CPUs */
> -static void set_sprs_common(uint64_t val)
> -{
> -	// mtspr(9, val);	/* CTR */ /* Used by mfspr/mtspr */
> -	// mtspr(273, val);	/* SPRG1 */  /* Used by our exception handler */
> -	mtspr(274, val);	/* SPRG2 */
> -	mtspr(275, val);	/* SPRG3 */
> -}
> +#define SPR_PR_READ	0x0001
> +#define SPR_PR_WRITE	0x0002
> +#define SPR_OS_READ	0x0010
> +#define SPR_OS_WRITE	0x0020
> +#define SPR_HV_READ	0x0100
> +#define SPR_HV_WRITE	0x0200
> +
> +#define RW		0x333
> +#define RO		0x111
> +#define WO		0x222
> +#define OS_RW		0x330
> +#define OS_RO		0x110
> +#define OS_WO		0x220
> +#define HV_RW		0x300
> +#define HV_RO		0x100
> +#define HV_WO		0x200
> +
> +#define SPR_ASYNC	0x1000	/* May be updated asynchronously */
> +#define SPR_INT		0x2000	/* May be updated by synchronous interrupt */
> +#define SPR_HARNESS	0x4000	/* Test harness uses the register */
> +
> +struct spr {
> +	const char	*name;
> +	uint8_t		width;
> +	uint16_t	access;
> +	uint16_t	type;
> +};
> +
> +/* SPRs common denominator back to PowerPC Operating Environment Architecture */
> +static const struct spr sprs_common[1024] = {
> +  [1] = {"XER",		64,	RW,		SPR_HARNESS, }, /* Compiler */
> +  [8] = {"LR", 		64,	RW,		SPR_HARNESS, }, /* Compiler, mfspr/mtspr */
> +  [9] = {"CTR",		64,	RW,		SPR_HARNESS, }, /* Compiler, mfspr/mtspr */
> + [18] = {"DSISR",	32,	OS_RW,		SPR_INT, },
> + [19] = {"DAR",		64,	OS_RW,		SPR_INT, },
> + [26] = {"SRR0",	64,	OS_RW,		SPR_INT, },
> + [27] = {"SRR1",	64,	OS_RW,		SPR_INT, },
> +[268] = {"TB",		64,	RO	,	SPR_ASYNC, },
> +[269] = {"TBU",		32,	RO,		SPR_ASYNC, },
> +[272] = {"SPRG0",	64,	OS_RW,		SPR_HARNESS, }, /* Int stack */
> +[273] = {"SPRG1",	64,	OS_RW,		SPR_HARNESS, }, /* Scratch */
> +[274] = {"SPRG2",	64,	OS_RW, },
> +[275] = {"SPRG3",	64,	OS_RW, },
> +[287] = {"PVR",		32,	OS_RO, },
> +};

Using a size of 1024 for each of these arrays looks weird. Why don't you add 
a "nr" field to struct spr and specify the register number via that field 
instead of using the index into the array as register number?

  Thomas

Re: [kvm-unit-tests v2 06/10] powerpc/sprs: Specify SPRs with data rather than code

[Nicholas Piggin]

On Thu Mar 23, 2023 at 10:36 PM AEST, Thomas Huth wrote:
> On 20/03/2023 08.03, Nicholas Piggin wrote:
> > +/* SPRs common denominator back to PowerPC Operating Environment Architecture */
> > +static const struct spr sprs_common[1024] = {
> > +  [1] = {"XER",		64,	RW,		SPR_HARNESS, }, /* Compiler */
> > +  [8] = {"LR", 		64,	RW,		SPR_HARNESS, }, /* Compiler, mfspr/mtspr */
> > +  [9] = {"CTR",		64,	RW,		SPR_HARNESS, }, /* Compiler, mfspr/mtspr */
> > + [18] = {"DSISR",	32,	OS_RW,		SPR_INT, },
> > + [19] = {"DAR",		64,	OS_RW,		SPR_INT, },
> > + [26] = {"SRR0",	64,	OS_RW,		SPR_INT, },
> > + [27] = {"SRR1",	64,	OS_RW,		SPR_INT, },
> > +[268] = {"TB",		64,	RO	,	SPR_ASYNC, },
> > +[269] = {"TBU",		32,	RO,		SPR_ASYNC, },
> > +[272] = {"SPRG0",	64,	OS_RW,		SPR_HARNESS, }, /* Int stack */
> > +[273] = {"SPRG1",	64,	OS_RW,		SPR_HARNESS, }, /* Scratch */
> > +[274] = {"SPRG2",	64,	OS_RW, },
> > +[275] = {"SPRG3",	64,	OS_RW, },
> > +[287] = {"PVR",		32,	OS_RO, },
> > +};
>
> Using a size of 1024 for each of these arrays looks weird. Why don't you add 
> a "nr" field to struct spr and specify the register number via that field 
> instead of using the index into the array as register number?

Oh I meant to reply to this. I did try it that way at first. When it
came manipulating the arrays like merging them or adding and
subtracing some SPRs, it required a bit of code to search, sort, add,
remove, etc. This way takes almost nothing. It is a dumb data structure
but it works okay here.

Thanks,
Nick

[kvm-unit-tests v2 07/10] powerpc/spapr_vpa: Add basic VPA tests

[Nicholas Piggin]

The VPA is a(n optional) memory structure shared between the hypervisor
and operating system, defined by PAPR. This test defines the structure
and adds registration, deregistration, and a few simple sanity tests.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 lib/linux/compiler.h    |  2 +
 lib/powerpc/asm/hcall.h |  1 +
 lib/ppc64/asm/vpa.h     | 62 ++++++++++++++++++++++++++++
 powerpc/Makefile.ppc64  |  2 +-
 powerpc/spapr_vpa.c     | 90 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 156 insertions(+), 1 deletion(-)
 create mode 100644 lib/ppc64/asm/vpa.h
 create mode 100644 powerpc/spapr_vpa.c

diff --git a/lib/linux/compiler.h b/lib/linux/compiler.h
index 6f565e4..c9d205e 100644
--- a/lib/linux/compiler.h
+++ b/lib/linux/compiler.h
@@ -45,7 +45,9 @@
 
 #define barrier()	asm volatile("" : : : "memory")
 
+#ifndef __always_inline
 #define __always_inline	inline __attribute__((always_inline))
+#endif
 #define noinline __attribute__((noinline))
 #define __unused __attribute__((__unused__))
 
diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h
index 1173fea..e0f5009 100644
--- a/lib/powerpc/asm/hcall.h
+++ b/lib/powerpc/asm/hcall.h
@@ -18,6 +18,7 @@
 #define H_SET_SPRG0		0x24
 #define H_SET_DABR		0x28
 #define H_PAGE_INIT		0x2c
+#define H_REGISTER_VPA		0xDC
 #define H_CEDE			0xE0
 #define H_GET_TERM_CHAR		0x54
 #define H_PUT_TERM_CHAR		0x58
diff --git a/lib/ppc64/asm/vpa.h b/lib/ppc64/asm/vpa.h
new file mode 100644
index 0000000..11dde01
--- /dev/null
+++ b/lib/ppc64/asm/vpa.h
@@ -0,0 +1,62 @@
+#ifndef _ASMPOWERPC_VPA_H_
+#define _ASMPOWERPC_VPA_H_
+/*
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+
+#ifndef __ASSEMBLY__
+
+struct vpa {
+	uint32_t	descriptor;
+	uint16_t	size;
+	uint8_t		reserved1[3];
+	uint8_t		status;
+	uint8_t		reserved2[14];
+	uint32_t	fru_node_id;
+	uint32_t	fru_proc_id;
+	uint8_t		reserved3[56];
+	uint8_t		vhpn_change_counters[8];
+	uint8_t		reserved4[80];
+	uint8_t		cede_latency;
+	uint8_t		maintain_ebb;
+	uint8_t		reserved5[6];
+	uint8_t		dtl_enable_mask;
+	uint8_t		dedicated_cpu_donate;
+	uint8_t		maintain_fpr;
+	uint8_t		maintain_pmc;
+	uint8_t		reserved6[28];
+	uint64_t	idle_estimate_purr;
+	uint8_t		reserved7[28];
+	uint16_t	maintain_nr_slb;
+	uint8_t		idle;
+	uint8_t		maintain_vmx;
+	uint32_t	vp_dispatch_count;
+	uint32_t	vp_dispatch_dispersion;
+	uint64_t	vp_fault_count;
+	uint64_t	vp_fault_tb;
+	uint64_t	purr_exprop_idle;
+	uint64_t	spurr_exprop_idle;
+	uint64_t	purr_exprop_busy;
+	uint64_t	spurr_exprop_busy;
+	uint64_t	purr_donate_idle;
+	uint64_t	spurr_donate_idle;
+	uint64_t	purr_donate_busy;
+	uint64_t	spurr_donate_busy;
+	uint64_t	vp_wait3_tb;
+	uint64_t	vp_wait2_tb;
+	uint64_t	vp_wait1_tb;
+	uint64_t	purr_exprop_adjunct_busy;
+	uint64_t	spurr_exprop_adjunct_busy;
+	uint32_t	supervisor_pagein_count;
+	uint8_t		reserved8[4];
+	uint64_t	purr_exprop_adjunct_idle;
+	uint64_t	spurr_exprop_adjunct_idle;
+	uint64_t	adjunct_insns_executed;
+	uint8_t		reserved9[120];
+	uint64_t	dtl_index;
+	uint8_t		reserved10[96];
+};
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _ASMPOWERPC_VPA_H_ */
diff --git a/powerpc/Makefile.ppc64 b/powerpc/Makefile.ppc64
index ea68447..b0ed2b1 100644
--- a/powerpc/Makefile.ppc64
+++ b/powerpc/Makefile.ppc64
@@ -19,7 +19,7 @@ reloc.o  = $(TEST_DIR)/reloc64.o
 OBJDIRS += lib/ppc64
 
 # ppc64 specific tests
-tests =
+tests = $(TEST_DIR)/spapr_vpa.elf
 
 include $(SRCDIR)/$(TEST_DIR)/Makefile.common
 
diff --git a/powerpc/spapr_vpa.c b/powerpc/spapr_vpa.c
new file mode 100644
index 0000000..45688fe
--- /dev/null
+++ b/powerpc/spapr_vpa.c
@@ -0,0 +1,90 @@
+/*
+ * Test sPAPR hypervisor calls (aka. h-calls)
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include <libfdt/libfdt.h>
+#include <devicetree.h>
+#include <libcflat.h>
+#include <util.h>
+#include <alloc.h>
+#include <asm/processor.h>
+#include <asm/hcall.h>
+#include <asm/vpa.h>
+#include <asm/io.h> /* for endian accessors */
+
+static void print_vpa(struct vpa *vpa)
+{
+	printf("VPA\n");
+	printf("descriptor:			0x%08x\n", be32_to_cpu(vpa->descriptor));
+	printf("size:				    0x%04x\n", be16_to_cpu(vpa->size));
+	printf("status:				      0x%02x\n", vpa->status);
+	printf("fru_node_id:			0x%08x\n", be32_to_cpu(vpa->fru_node_id));
+	printf("fru_proc_id:			0x%08x\n", be32_to_cpu(vpa->fru_proc_id));
+	printf("vhpn_change_counters:		0x%02x %02x %02x %02x %02x %02x %02x %02x\n", vpa->vhpn_change_counters[0], vpa->vhpn_change_counters[1], vpa->vhpn_change_counters[2], vpa->vhpn_change_counters[3], vpa->vhpn_change_counters[4], vpa->vhpn_change_counters[5], vpa->vhpn_change_counters[6], vpa->vhpn_change_counters[7]);
+	printf("vp_dispatch_count:		0x%08x\n", be32_to_cpu(vpa->vp_dispatch_count));
+	printf("vp_dispatch_dispersion:		0x%08x\n", be32_to_cpu(vpa->vp_dispatch_dispersion));
+	printf("vp_fault_count:			0x%08lx\n", be64_to_cpu(vpa->vp_fault_count));
+	printf("vp_fault_tb:			0x%08lx\n", be64_to_cpu(vpa->vp_fault_tb));
+	printf("purr_exprop_idle:		0x%08lx\n", be64_to_cpu(vpa->purr_exprop_idle));
+	printf("spurr_exprop_idle:		0x%08lx\n", be64_to_cpu(vpa->spurr_exprop_idle));
+	printf("purr_exprop_busy:		0x%08lx\n", be64_to_cpu(vpa->purr_exprop_busy));
+	printf("spurr_exprop_busy:		0x%08lx\n", be64_to_cpu(vpa->spurr_exprop_busy));
+	printf("purr_donate_idle:		0x%08lx\n", be64_to_cpu(vpa->purr_donate_idle));
+	printf("spurr_donate_idle:		0x%08lx\n", be64_to_cpu(vpa->spurr_donate_idle));
+	printf("purr_donate_busy:		0x%08lx\n", be64_to_cpu(vpa->purr_donate_busy));
+	printf("spurr_donate_busy:		0x%08lx\n", be64_to_cpu(vpa->spurr_donate_busy));
+	printf("vp_wait3_tb:			0x%08lx\n", be64_to_cpu(vpa->vp_wait3_tb));
+	printf("vp_wait2_tb:			0x%08lx\n", be64_to_cpu(vpa->vp_wait2_tb));
+	printf("vp_wait1_tb:			0x%08lx\n", be64_to_cpu(vpa->vp_wait1_tb));
+	printf("purr_exprop_adjunct_busy:	0x%08lx\n", be64_to_cpu(vpa->purr_exprop_adjunct_busy));
+	printf("spurr_exprop_adjunct_busy:	0x%08lx\n", be64_to_cpu(vpa->spurr_exprop_adjunct_busy));
+	printf("purr_exprop_adjunct_idle:	0x%08lx\n", be64_to_cpu(vpa->purr_exprop_adjunct_idle));
+	printf("spurr_exprop_adjunct_idle:	0x%08lx\n", be64_to_cpu(vpa->spurr_exprop_adjunct_idle));
+	printf("adjunct_insns_executed:		0x%08lx\n", be64_to_cpu(vpa->adjunct_insns_executed));
+	printf("dtl_index:			0x%08lx\n", be64_to_cpu(vpa->dtl_index));
+}
+
+/**
+ * Test the H_REGISTER_VPA h-call register/deregister.
+ */
+static void register_vpa(struct vpa *vpa)
+{
+	uint32_t cpuid = fdt_boot_cpuid_phys(dt_fdt());
+	int disp_count1, disp_count2;
+	int rc;
+
+	rc = hcall(H_REGISTER_VPA, 1ULL << 45, cpuid, vpa);
+	report(rc == H_SUCCESS, "VPA registered");
+
+	print_vpa(vpa);
+
+	disp_count1 = be32_to_cpu(vpa->vp_dispatch_count);
+	report(disp_count1 % 2 == 0, "Dispatch count is even while running");
+	msleep(100);
+	disp_count2 = be32_to_cpu(vpa->vp_dispatch_count);
+	report(disp_count1 != disp_count2, "Dispatch count increments over H_CEDE");
+
+	rc = hcall(H_REGISTER_VPA, 5ULL << 45, cpuid, vpa);
+	report(rc == H_SUCCESS, "VPA deregistered");
+
+	disp_count1 = be32_to_cpu(vpa->vp_dispatch_count);
+	report(disp_count1 % 2 == 1, "Dispatch count is odd after deregister");
+}
+
+int main(int argc, char **argv)
+{
+	struct vpa *vpa;
+
+	vpa = memalign(4096, sizeof(*vpa));
+
+	memset(vpa, 0, sizeof(*vpa));
+
+	vpa->size = cpu_to_be16(sizeof(*vpa));
+
+	report_prefix_push("vpa");
+
+	register_vpa(vpa);
+
+	return report_summary();
+}
-- 
2.37.2

Re: [kvm-unit-tests v2 07/10] powerpc/spapr_vpa: Add basic VPA tests

[Thomas Huth]

On 20/03/2023 08.03, Nicholas Piggin wrote:
> The VPA is a(n optional) memory structure shared between the hypervisor
> and operating system, defined by PAPR. This test defines the structure
> and adds registration, deregistration, and a few simple sanity tests.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   lib/linux/compiler.h    |  2 +
>   lib/powerpc/asm/hcall.h |  1 +
>   lib/ppc64/asm/vpa.h     | 62 ++++++++++++++++++++++++++++
>   powerpc/Makefile.ppc64  |  2 +-
>   powerpc/spapr_vpa.c     | 90 +++++++++++++++++++++++++++++++++++++++++

Please add the new test to powerpc/unittests.cfg, otherwise it won't get 
picked up by the run_tests.sh script.

> diff --git a/lib/linux/compiler.h b/lib/linux/compiler.h
> index 6f565e4..c9d205e 100644
> --- a/lib/linux/compiler.h
> +++ b/lib/linux/compiler.h
> @@ -45,7 +45,9 @@
>   
>   #define barrier()	asm volatile("" : : : "memory")
>   
> +#ifndef __always_inline
>   #define __always_inline	inline __attribute__((always_inline))
> +#endif

What's this change good for? ... it doesn't seem to be related to this patch?

> diff --git a/lib/ppc64/asm/vpa.h b/lib/ppc64/asm/vpa.h
> new file mode 100644
> index 0000000..11dde01
> --- /dev/null
> +++ b/lib/ppc64/asm/vpa.h
> @@ -0,0 +1,62 @@
> +#ifndef _ASMPOWERPC_VPA_H_
> +#define _ASMPOWERPC_VPA_H_
> +/*
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +
> +#ifndef __ASSEMBLY__
> +
> +struct vpa {
> +	uint32_t	descriptor;
> +	uint16_t	size;
> +	uint8_t		reserved1[3];
> +	uint8_t		status;

Where does this status field come from? ... My LoPAPR only says that there 
are 18 "reserved" bytes in total here.

> +	uint8_t		reserved2[14];
> +	uint32_t	fru_node_id;
> +	uint32_t	fru_proc_id;
> +	uint8_t		reserved3[56];
> +	uint8_t		vhpn_change_counters[8];
> +	uint8_t		reserved4[80];
> +	uint8_t		cede_latency;
> +	uint8_t		maintain_ebb;
> +	uint8_t		reserved5[6];
> +	uint8_t		dtl_enable_mask;
> +	uint8_t		dedicated_cpu_donate;
> +	uint8_t		maintain_fpr;
> +	uint8_t		maintain_pmc;
> +	uint8_t		reserved6[28];
> +	uint64_t	idle_estimate_purr;
> +	uint8_t		reserved7[28];
> +	uint16_t	maintain_nr_slb;
> +	uint8_t		idle;
> +	uint8_t		maintain_vmx;
> +	uint32_t	vp_dispatch_count;
> +	uint32_t	vp_dispatch_dispersion;
> +	uint64_t	vp_fault_count;
> +	uint64_t	vp_fault_tb;
> +	uint64_t	purr_exprop_idle;
> +	uint64_t	spurr_exprop_idle;
> +	uint64_t	purr_exprop_busy;
> +	uint64_t	spurr_exprop_busy;
> +	uint64_t	purr_donate_idle;
> +	uint64_t	spurr_donate_idle;
> +	uint64_t	purr_donate_busy;
> +	uint64_t	spurr_donate_busy;
> +	uint64_t	vp_wait3_tb;
> +	uint64_t	vp_wait2_tb;
> +	uint64_t	vp_wait1_tb;
> +	uint64_t	purr_exprop_adjunct_busy;
> +	uint64_t	spurr_exprop_adjunct_busy;

The above two fields are also marked as "reserved" in my LoPAPR ... which 
version did you use?

> +	uint32_t	supervisor_pagein_count;
> +	uint8_t		reserved8[4];
> +	uint64_t	purr_exprop_adjunct_idle;
> +	uint64_t	spurr_exprop_adjunct_idle;
> +	uint64_t	adjunct_insns_executed;

dito for the above three lines... I guess my LoPAPR is too old...

> +	uint8_t		reserved9[120];
> +	uint64_t	dtl_index;
> +	uint8_t		reserved10[96];
> +};
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* _ASMPOWERPC_VPA_H_ */
> diff --git a/powerpc/Makefile.ppc64 b/powerpc/Makefile.ppc64
> index ea68447..b0ed2b1 100644
> --- a/powerpc/Makefile.ppc64
> +++ b/powerpc/Makefile.ppc64
> @@ -19,7 +19,7 @@ reloc.o  = $(TEST_DIR)/reloc64.o
>   OBJDIRS += lib/ppc64
>   
>   # ppc64 specific tests
> -tests =
> +tests = $(TEST_DIR)/spapr_vpa.elf
>   
>   include $(SRCDIR)/$(TEST_DIR)/Makefile.common
>   
> diff --git a/powerpc/spapr_vpa.c b/powerpc/spapr_vpa.c
> new file mode 100644
> index 0000000..45688fe
> --- /dev/null
> +++ b/powerpc/spapr_vpa.c
> @@ -0,0 +1,90 @@
> +/*
> + * Test sPAPR hypervisor calls (aka. h-calls)

Adjust to "Test sPAPR H_REGISTER_VPA hypervisor call" ?

> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include <libfdt/libfdt.h>
> +#include <devicetree.h>
> +#include <libcflat.h>
> +#include <util.h>
> +#include <alloc.h>
> +#include <asm/processor.h>
> +#include <asm/hcall.h>
> +#include <asm/vpa.h>
> +#include <asm/io.h> /* for endian accessors */
> +
> +static void print_vpa(struct vpa *vpa)
> +{
> +	printf("VPA\n");
> +	printf("descriptor:			0x%08x\n", be32_to_cpu(vpa->descriptor));
> +	printf("size:				    0x%04x\n", be16_to_cpu(vpa->size));
> +	printf("status:				      0x%02x\n", vpa->status);
> +	printf("fru_node_id:			0x%08x\n", be32_to_cpu(vpa->fru_node_id));
> +	printf("fru_proc_id:			0x%08x\n", be32_to_cpu(vpa->fru_proc_id));
> +	printf("vhpn_change_counters:		0x%02x %02x %02x %02x %02x %02x %02x %02x\n", vpa->vhpn_change_counters[0], vpa->vhpn_change_counters[1], vpa->vhpn_change_counters[2], vpa->vhpn_change_counters[3], vpa->vhpn_change_counters[4], vpa->vhpn_change_counters[5], vpa->vhpn_change_counters[6], vpa->vhpn_change_counters[7]);
> +	printf("vp_dispatch_count:		0x%08x\n", be32_to_cpu(vpa->vp_dispatch_count));
> +	printf("vp_dispatch_dispersion:		0x%08x\n", be32_to_cpu(vpa->vp_dispatch_dispersion));
> +	printf("vp_fault_count:			0x%08lx\n", be64_to_cpu(vpa->vp_fault_count));
> +	printf("vp_fault_tb:			0x%08lx\n", be64_to_cpu(vpa->vp_fault_tb));
> +	printf("purr_exprop_idle:		0x%08lx\n", be64_to_cpu(vpa->purr_exprop_idle));
> +	printf("spurr_exprop_idle:		0x%08lx\n", be64_to_cpu(vpa->spurr_exprop_idle));
> +	printf("purr_exprop_busy:		0x%08lx\n", be64_to_cpu(vpa->purr_exprop_busy));
> +	printf("spurr_exprop_busy:		0x%08lx\n", be64_to_cpu(vpa->spurr_exprop_busy));
> +	printf("purr_donate_idle:		0x%08lx\n", be64_to_cpu(vpa->purr_donate_idle));
> +	printf("spurr_donate_idle:		0x%08lx\n", be64_to_cpu(vpa->spurr_donate_idle));
> +	printf("purr_donate_busy:		0x%08lx\n", be64_to_cpu(vpa->purr_donate_busy));
> +	printf("spurr_donate_busy:		0x%08lx\n", be64_to_cpu(vpa->spurr_donate_busy));
> +	printf("vp_wait3_tb:			0x%08lx\n", be64_to_cpu(vpa->vp_wait3_tb));
> +	printf("vp_wait2_tb:			0x%08lx\n", be64_to_cpu(vpa->vp_wait2_tb));
> +	printf("vp_wait1_tb:			0x%08lx\n", be64_to_cpu(vpa->vp_wait1_tb));
> +	printf("purr_exprop_adjunct_busy:	0x%08lx\n", be64_to_cpu(vpa->purr_exprop_adjunct_busy));
> +	printf("spurr_exprop_adjunct_busy:	0x%08lx\n", be64_to_cpu(vpa->spurr_exprop_adjunct_busy));
> +	printf("purr_exprop_adjunct_idle:	0x%08lx\n", be64_to_cpu(vpa->purr_exprop_adjunct_idle));
> +	printf("spurr_exprop_adjunct_idle:	0x%08lx\n", be64_to_cpu(vpa->spurr_exprop_adjunct_idle));
> +	printf("adjunct_insns_executed:		0x%08lx\n", be64_to_cpu(vpa->adjunct_insns_executed));
> +	printf("dtl_index:			0x%08lx\n", be64_to_cpu(vpa->dtl_index));
> +}
> +
> +/**
> + * Test the H_REGISTER_VPA h-call register/deregister.
> + */
> +static void register_vpa(struct vpa *vpa)
> +{
> +	uint32_t cpuid = fdt_boot_cpuid_phys(dt_fdt());
> +	int disp_count1, disp_count2;
> +	int rc;
> +
> +	rc = hcall(H_REGISTER_VPA, 1ULL << 45, cpuid, vpa);
> +	report(rc == H_SUCCESS, "VPA registered");
> +
> +	print_vpa(vpa);
> +
> +	disp_count1 = be32_to_cpu(vpa->vp_dispatch_count);
> +	report(disp_count1 % 2 == 0, "Dispatch count is even while running");
> +	msleep(100);
> +	disp_count2 = be32_to_cpu(vpa->vp_dispatch_count);
> +	report(disp_count1 != disp_count2, "Dispatch count increments over H_CEDE");
> +
> +	rc = hcall(H_REGISTER_VPA, 5ULL << 45, cpuid, vpa);
> +	report(rc == H_SUCCESS, "VPA deregistered");
> +
> +	disp_count1 = be32_to_cpu(vpa->vp_dispatch_count);
> +	report(disp_count1 % 2 == 1, "Dispatch count is odd after deregister");
> +}

Now that was a very tame amount of tests ;-)

I'd suggest to add some more:

- Check hcall(H_REGISTER_VPA, 0, ...);
- Check hcall(H_REGISTER_VPA, ..., bad-cpu-id, ...)
- Check hcall(H_REGISTER_VPA, ..., ..., unaligned-address)
- Check hcall(H_REGISTER_VPA, ..., ..., illegal-address)
- Check registration with vpa->size being too small
- Check registration where the vpa crosses the 4k boundary

What do you think?

> +int main(int argc, char **argv)
> +{
> +	struct vpa *vpa;
> +
> +	vpa = memalign(4096, sizeof(*vpa));
> +
> +	memset(vpa, 0, sizeof(*vpa));
> +
> +	vpa->size = cpu_to_be16(sizeof(*vpa));
> +
> +	report_prefix_push("vpa");

This lacks the corresponding report_prefix_pop() later.

> +	register_vpa(vpa);
> +
> +	return report_summary();
> +}

  Thomas

Re: [kvm-unit-tests v2 07/10] powerpc/spapr_vpa: Add basic VPA tests

[Nicholas Piggin]

On Fri Mar 24, 2023 at 12:07 AM AEST, Thomas Huth wrote:
> On 20/03/2023 08.03, Nicholas Piggin wrote:
> > The VPA is a(n optional) memory structure shared between the hypervisor
> > and operating system, defined by PAPR. This test defines the structure
> > and adds registration, deregistration, and a few simple sanity tests.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   lib/linux/compiler.h    |  2 +
> >   lib/powerpc/asm/hcall.h |  1 +
> >   lib/ppc64/asm/vpa.h     | 62 ++++++++++++++++++++++++++++
> >   powerpc/Makefile.ppc64  |  2 +-
> >   powerpc/spapr_vpa.c     | 90 +++++++++++++++++++++++++++++++++++++++++
>
> Please add the new test to powerpc/unittests.cfg, otherwise it won't get 
> picked up by the run_tests.sh script.

Ah good point.

> > diff --git a/lib/linux/compiler.h b/lib/linux/compiler.h
> > index 6f565e4..c9d205e 100644
> > --- a/lib/linux/compiler.h
> > +++ b/lib/linux/compiler.h
> > @@ -45,7 +45,9 @@
> >   
> >   #define barrier()	asm volatile("" : : : "memory")
> >   
> > +#ifndef __always_inline
> >   #define __always_inline	inline __attribute__((always_inline))
> > +#endif
>
> What's this change good for? ... it doesn't seem to be related to this patch?

Some header ordering issue I forgot about, thanks for reminding. I think it
should be split it out. See /usr/include/<arch>/sys/cdefs.h:

/* Forces a function to be always inlined.  */
#if __GNUC_PREREQ (3,2) || __glibc_has_attribute (__always_inline__)
/* The Linux kernel defines __always_inline in stddef.h (283d7573), and
   it conflicts with this definition.  Therefore undefine it first to
   allow either header to be included first.  */
# undef __always_inline
# define __always_inline __inline __attribute__ ((__always_inline__))
#else
# undef __always_inline
# define __always_inline __inline
#endif

> > diff --git a/lib/ppc64/asm/vpa.h b/lib/ppc64/asm/vpa.h
> > new file mode 100644
> > index 0000000..11dde01
> > --- /dev/null
> > +++ b/lib/ppc64/asm/vpa.h
> > @@ -0,0 +1,62 @@
> > +#ifndef _ASMPOWERPC_VPA_H_
> > +#define _ASMPOWERPC_VPA_H_
> > +/*
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +
> > +#ifndef __ASSEMBLY__
> > +
> > +struct vpa {
> > +	uint32_t	descriptor;
> > +	uint16_t	size;
> > +	uint8_t		reserved1[3];
> > +	uint8_t		status;
>
> Where does this status field come from? ... My LoPAPR only says that there 
> are 18 "reserved" bytes in total here.

Hmm, I'm not sure why that was left out of LoPAPR, Linux has been using
it for a long time. It basically just tells you if you are on a
dedicated or shared partition (hard partitioned or timesliced CPUs).
Possibly an oversight.

>
> > +	uint8_t		reserved2[14];
> > +	uint32_t	fru_node_id;
> > +	uint32_t	fru_proc_id;
> > +	uint8_t		reserved3[56];
> > +	uint8_t		vhpn_change_counters[8];
> > +	uint8_t		reserved4[80];
> > +	uint8_t		cede_latency;
> > +	uint8_t		maintain_ebb;
> > +	uint8_t		reserved5[6];
> > +	uint8_t		dtl_enable_mask;
> > +	uint8_t		dedicated_cpu_donate;
> > +	uint8_t		maintain_fpr;
> > +	uint8_t		maintain_pmc;
> > +	uint8_t		reserved6[28];
> > +	uint64_t	idle_estimate_purr;
> > +	uint8_t		reserved7[28];
> > +	uint16_t	maintain_nr_slb;
> > +	uint8_t		idle;
> > +	uint8_t		maintain_vmx;
> > +	uint32_t	vp_dispatch_count;
> > +	uint32_t	vp_dispatch_dispersion;
> > +	uint64_t	vp_fault_count;
> > +	uint64_t	vp_fault_tb;
> > +	uint64_t	purr_exprop_idle;
> > +	uint64_t	spurr_exprop_idle;
> > +	uint64_t	purr_exprop_busy;
> > +	uint64_t	spurr_exprop_busy;
> > +	uint64_t	purr_donate_idle;
> > +	uint64_t	spurr_donate_idle;
> > +	uint64_t	purr_donate_busy;
> > +	uint64_t	spurr_donate_busy;
> > +	uint64_t	vp_wait3_tb;
> > +	uint64_t	vp_wait2_tb;
> > +	uint64_t	vp_wait1_tb;
> > +	uint64_t	purr_exprop_adjunct_busy;
> > +	uint64_t	spurr_exprop_adjunct_busy;
>
> The above two fields are also marked as "reserved" in my LoPAPR ... which 
> version did you use?
>
> > +	uint32_t	supervisor_pagein_count;
> > +	uint8_t		reserved8[4];
> > +	uint64_t	purr_exprop_adjunct_idle;
> > +	uint64_t	spurr_exprop_adjunct_idle;
> > +	uint64_t	adjunct_insns_executed;
>
> dito for the above three lines... I guess my LoPAPR is too old...

Ah, I'm guessing the "adjunct" option isn't relevant to Linux/KVM so it
was probably left out (it's much older than LoPAPR).

Generally LoPAPR is still pretty up to date, but we should do better at
keeping it current IMO. I've made some more noises about that, but
can't make any promises here.

> > +	uint8_t		reserved9[120];
> > +	uint64_t	dtl_index;
> > +	uint8_t		reserved10[96];
> > +};
> > +
> > +#endif /* __ASSEMBLY__ */
> > +
> > +#endif /* _ASMPOWERPC_VPA_H_ */
> > diff --git a/powerpc/Makefile.ppc64 b/powerpc/Makefile.ppc64
> > index ea68447..b0ed2b1 100644
> > --- a/powerpc/Makefile.ppc64
> > +++ b/powerpc/Makefile.ppc64
> > @@ -19,7 +19,7 @@ reloc.o  = $(TEST_DIR)/reloc64.o
> >   OBJDIRS += lib/ppc64
> >   
> >   # ppc64 specific tests
> > -tests =
> > +tests = $(TEST_DIR)/spapr_vpa.elf
> >   
> >   include $(SRCDIR)/$(TEST_DIR)/Makefile.common
> >   
> > diff --git a/powerpc/spapr_vpa.c b/powerpc/spapr_vpa.c
> > new file mode 100644
> > index 0000000..45688fe
> > --- /dev/null
> > +++ b/powerpc/spapr_vpa.c
> > @@ -0,0 +1,90 @@
> > +/*
> > + * Test sPAPR hypervisor calls (aka. h-calls)
>
> Adjust to "Test sPAPR H_REGISTER_VPA hypervisor call" ?

Yes.

> > +	rc = hcall(H_REGISTER_VPA, 5ULL << 45, cpuid, vpa);
> > +	report(rc == H_SUCCESS, "VPA deregistered");
> > +
> > +	disp_count1 = be32_to_cpu(vpa->vp_dispatch_count);
> > +	report(disp_count1 % 2 == 1, "Dispatch count is odd after deregister");
> > +}
>
> Now that was a very tame amount of tests ;-)

Yeah it was just a start. I was going to add a few more scheduling
type ones if I can improve SMP support as well.

> I'd suggest to add some more:
>
> - Check hcall(H_REGISTER_VPA, 0, ...);
> - Check hcall(H_REGISTER_VPA, ..., bad-cpu-id, ...)
> - Check hcall(H_REGISTER_VPA, ..., ..., unaligned-address)
> - Check hcall(H_REGISTER_VPA, ..., ..., illegal-address)
> - Check registration with vpa->size being too small
> - Check registration where the vpa crosses the 4k boundary
>
> What do you think?

Good idea.

> > +int main(int argc, char **argv)
> > +{
> > +	struct vpa *vpa;
> > +
> > +	vpa = memalign(4096, sizeof(*vpa));
> > +
> > +	memset(vpa, 0, sizeof(*vpa));
> > +
> > +	vpa->size = cpu_to_be16(sizeof(*vpa));
> > +
> > +	report_prefix_push("vpa");
>
> This lacks the corresponding report_prefix_pop() later.

Got it.

Thanks,
Nick

[kvm-unit-tests v2 08/10] powerpc: Discover runtime load address dynamically

[Nicholas Piggin]

The next change will load the kernels at different addresses depending
on test options, so this needs to be reverted back to dynamic
discovery.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 powerpc/cstart64.S | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
index 34e3934..069d991 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S
@@ -33,9 +33,14 @@ start:
 	 * We were loaded at QEMU's kernel load address, but we're not
 	 * allowed to link there due to how QEMU deals with linker VMAs,
 	 * so we just linked at zero. This means the first thing to do is
-	 * to find our stack and toc, and then do a relocate.
+	 * to find our stack and toc, and then do a relocate. powernv and
+	 * pseries load addreses are not the same, so find the address
+	 * dynamically:
 	 */
-	LOAD_REG_IMMEDIATE(r31, SPAPR_KERNEL_LOAD_ADDR)
+	bl	0f
+0:	mflr	r31
+	subi	r31, r31, 0b - start	/* QEMU's kernel load address */
+
 	ld	r1, (p_stack - start)(r31)
 	ld	r2, (p_toc - start)(r31)
 	add	r1, r1, r31
@@ -114,8 +119,11 @@ p_toc:		.llong  tocptr
 p_dyn:		.llong  dynamic_start
 
 .text
+start_text:
 .align 3
+p_toc_text:	.llong	tocptr
 
+.align 3
 .globl hcall
 hcall:
 	sc	1
@@ -193,9 +201,10 @@ call_handler:
 	std	r0, _MSR(r1)
 
 	/* restore TOC pointer */
-
-	LOAD_REG_IMMEDIATE(r31, SPAPR_KERNEL_LOAD_ADDR)
-	ld	r2, (p_toc - start)(r31)
+	bl	0f
+0:	mflr	r31
+	subi	r31, r31, 0b - start_text
+	ld	r2, (p_toc_text - start_text)(r31)
 
 	/* FIXME: build stack frame */
 
-- 
2.37.2

[kvm-unit-tests v2 09/10] powerpc: Support powernv machine with QEMU TCG

[Nicholas Piggin]

This is a basic first pass at powernv support using OPAL (skiboot)
firmware.

The ACCEL is a bit clunky, now defaulting to tcg for powernv machine.
It also does not yet run in the run_tests.sh batch process, more work
is needed to exclude certain tests (e.g., rtas) and adjust parameters
(e.g., increase memory size) to allow powernv to work. For now it
can run single test cases.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 lib/powerpc/asm/ppc_asm.h   |  5 +++
 lib/powerpc/asm/processor.h | 14 ++++++++
 lib/powerpc/hcall.c         |  4 +--
 lib/powerpc/io.c            | 33 ++++++++++++++++--
 lib/powerpc/io.h            |  6 ++++
 lib/powerpc/processor.c     | 10 ++++++
 lib/powerpc/setup.c         | 10 ++++--
 lib/ppc64/asm/opal.h        | 11 ++++++
 lib/ppc64/opal-calls.S      | 46 +++++++++++++++++++++++++
 lib/ppc64/opal.c            | 67 +++++++++++++++++++++++++++++++++++++
 powerpc/Makefile.ppc64      |  2 ++
 powerpc/cstart64.S          |  7 ++++
 powerpc/run                 | 30 ++++++++++++++---
 13 files changed, 234 insertions(+), 11 deletions(-)
 create mode 100644 lib/ppc64/asm/opal.h
 create mode 100644 lib/ppc64/opal-calls.S
 create mode 100644 lib/ppc64/opal.c

diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
index 6299ff5..5eec9d3 100644
--- a/lib/powerpc/asm/ppc_asm.h
+++ b/lib/powerpc/asm/ppc_asm.h
@@ -36,7 +36,12 @@
 #endif /* __BYTE_ORDER__ */
 
 /* Machine State Register definitions: */
+#define MSR_LE_BIT	0
 #define MSR_EE_BIT	15			/* External Interrupts Enable */
+#define MSR_HV_BIT	60			/* Hypervisor mode */
 #define MSR_SF_BIT	63			/* 64-bit mode */
 
+#define SPR_HSRR0	0x13A
+#define SPR_HSRR1	0x13B
+
 #endif /* _ASMPOWERPC_PPC_ASM_H */
diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
index ebfeff2..8084787 100644
--- a/lib/powerpc/asm/processor.h
+++ b/lib/powerpc/asm/processor.h
@@ -3,12 +3,26 @@
 
 #include <libcflat.h>
 #include <asm/ptrace.h>
+#include <asm/ppc_asm.h>
 
 #ifndef __ASSEMBLY__
 void handle_exception(int trap, void (*func)(struct pt_regs *, void *), void *);
 void do_handle_exception(struct pt_regs *regs);
 #endif /* __ASSEMBLY__ */
 
+/*
+ * If this returns true on PowerNV / OPAL machines which run in hypervisor
+ * mode. False on pseries / PAPR machines that run in guest mode.
+ */
+static inline bool machine_is_powernv(void)
+{
+	uint64_t msr;
+
+	asm volatile ("mfmsr %[msr]" : [msr] "=r" (msr));
+
+	return !!(msr & (1ULL << MSR_HV_BIT));
+}
+
 static inline uint64_t get_tb(void)
 {
 	uint64_t tb;
diff --git a/lib/powerpc/hcall.c b/lib/powerpc/hcall.c
index 711cb1b..37e52f5 100644
--- a/lib/powerpc/hcall.c
+++ b/lib/powerpc/hcall.c
@@ -25,7 +25,7 @@ int hcall_have_broken_sc1(void)
 	return r3 == (unsigned long)H_PRIVILEGE;
 }
 
-void putchar(int c)
+void papr_putchar(int c)
 {
 	unsigned long vty = 0;		/* 0 == default */
 	unsigned long nr_chars = 1;
@@ -34,7 +34,7 @@ void putchar(int c)
 	hcall(H_PUT_TERM_CHAR, vty, nr_chars, chars);
 }
 
-int __getchar(void)
+int __papr_getchar(void)
 {
 	register unsigned long r3 asm("r3") = H_GET_TERM_CHAR;
 	register unsigned long r4 asm("r4") = 0; /* 0 == default vty */
diff --git a/lib/powerpc/io.c b/lib/powerpc/io.c
index a381688..a3a64ce 100644
--- a/lib/powerpc/io.c
+++ b/lib/powerpc/io.c
@@ -9,13 +9,39 @@
 #include <asm/spinlock.h>
 #include <asm/rtas.h>
 #include <asm/setup.h>
+#include <asm/processor.h>
 #include "io.h"
 
 static struct spinlock print_lock;
 
+struct opal {
+	uint64_t base;
+	uint64_t entry;
+};
+extern struct opal opal;
+
+void putchar(int c)
+{
+	if (machine_is_powernv())
+		opal_putchar(c);
+	else
+		papr_putchar(c);
+}
+
+int __getchar(void)
+{
+	if (machine_is_powernv())
+		return __opal_getchar();
+	else
+		return __papr_getchar();
+}
+
 void io_init(void)
 {
-	rtas_init();
+	if (machine_is_powernv())
+		opal_init();
+	else
+		rtas_init();
 }
 
 void puts(const char *s)
@@ -38,7 +64,10 @@ void exit(int code)
 // FIXME: change this print-exit/rtas-poweroff to chr_testdev_exit(),
 //        maybe by plugging chr-testdev into a spapr-vty.
 	printf("\nEXIT: STATUS=%d\n", ((code) << 1) | 1);
-	rtas_power_off();
+	if (machine_is_powernv())
+		opal_power_off();
+	else
+		rtas_power_off();
 	halt(code);
 	__builtin_unreachable();
 }
diff --git a/lib/powerpc/io.h b/lib/powerpc/io.h
index d4f21ba..a2aed7b 100644
--- a/lib/powerpc/io.h
+++ b/lib/powerpc/io.h
@@ -8,6 +8,12 @@
 #define _POWERPC_IO_H_
 
 extern void io_init(void);
+extern void opal_init(void);
+extern void opal_power_off(void);
 extern void putchar(int c);
+extern void opal_putchar(int c);
+extern void papr_putchar(int c);
+extern int __opal_getchar(void);
+extern int __papr_getchar(void);
 
 #endif
diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
index e77a240..06f6db3 100644
--- a/lib/powerpc/processor.c
+++ b/lib/powerpc/processor.c
@@ -61,6 +61,16 @@ void sleep_tb(uint64_t cycles)
 {
 	uint64_t start, end, now;
 
+	if (machine_is_powernv()) {
+		/*
+		 * Could use 'stop' to sleep here which would be interesting.
+		 * stop with ESL=0 should be simple enough, ESL=1 would require
+		 * SRESET based wakeup which is more involved.
+		 */
+		delay(cycles);
+		return;
+	}
+
 	start = now = get_tb();
 	end = start + cycles;
 
diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
index 1be4c03..f327540 100644
--- a/lib/powerpc/setup.c
+++ b/lib/powerpc/setup.c
@@ -18,6 +18,7 @@
 #include <argv.h>
 #include <asm/setup.h>
 #include <asm/page.h>
+#include <asm/processor.h>
 #include <asm/hcall.h>
 #include "io.h"
 
@@ -97,12 +98,13 @@ static void cpu_init(void)
 	tb_hz = params.tb_hz;
 
 	/* Interrupt Endianness */
-
+	if (!machine_is_powernv()) {
 #if  __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
-        hcall(H_SET_MODE, 1, 4, 0, 0);
+		hcall(H_SET_MODE, 1, 4, 0, 0);
 #else
-        hcall(H_SET_MODE, 0, 4, 0, 0);
+		hcall(H_SET_MODE, 0, 4, 0, 0);
 #endif
+	}
 }
 
 static void mem_init(phys_addr_t freemem_start)
@@ -195,6 +197,8 @@ void setup(const void *fdt)
 		freemem += initrd_size;
 	}
 
+	opal_init();
+
 	/* call init functions */
 	cpu_init();
 
diff --git a/lib/ppc64/asm/opal.h b/lib/ppc64/asm/opal.h
new file mode 100644
index 0000000..243a3c6
--- /dev/null
+++ b/lib/ppc64/asm/opal.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef _ASMPPC64_HCALL_H_
+#define _ASMPPC64_HCALL_H_
+
+#define OPAL_SUCCESS				0
+
+#define OPAL_CONSOLE_WRITE			1
+#define OPAL_CONSOLE_READ			2
+#define OPAL_CEC_POWER_DOWN			5
+
+#endif
diff --git a/lib/ppc64/opal-calls.S b/lib/ppc64/opal-calls.S
new file mode 100644
index 0000000..1833358
--- /dev/null
+++ b/lib/ppc64/opal-calls.S
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2016 IBM Corporation.
+ */
+
+#include <asm/ppc_asm.h>
+
+	.text
+	.globl opal_call
+opal_call:
+	mr	r0,r3
+	mr	r3,r4
+	mr	r4,r5
+	mr	r5,r6
+	mr	r6,r7
+	mflr	r11
+	std	r11,16(r1)
+	mfcr	r12
+	stw	r12,8(r1)
+	mr	r13,r2
+
+	/* Set opal return address */
+	LOAD_REG_ADDR(r11, opal_return)
+	mtlr	r11
+	mfmsr	r12
+
+	/* switch to BE when we enter OPAL */
+	li	r11,(1 << MSR_LE_BIT)
+	andc	r12,r12,r11
+	mtspr	SPR_HSRR1,r12
+
+	/* load the opal call entry point and base */
+	LOAD_REG_ADDR(r11, opal)
+	ld	r12,8(r11)
+	ld	r2,0(r11)
+	mtspr	SPR_HSRR0,r12
+	hrfid
+
+opal_return:
+	FIXUP_ENDIAN
+	mr	r2,r13;
+	lwz	r11,8(r1);
+	ld	r12,16(r1)
+	mtcr	r11;
+	mtlr	r12
+	blr
diff --git a/lib/ppc64/opal.c b/lib/ppc64/opal.c
new file mode 100644
index 0000000..515e9ab
--- /dev/null
+++ b/lib/ppc64/opal.c
@@ -0,0 +1,67 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * OPAL call helpers
+ */
+#include <asm/opal.h>
+#include <libcflat.h>
+#include <libfdt/libfdt.h>
+#include <devicetree.h>
+//libfdt_env.h>
+#include <asm/io.h>
+#include "../powerpc/io.h"
+
+struct opal {
+	uint64_t base;
+	uint64_t entry;
+} opal;
+
+void opal_init(void)
+{
+	const struct fdt_property *prop;
+	int node, len;
+
+	node = fdt_path_offset(dt_fdt(), "/ibm,opal");
+	if (node < 0) {
+		return;// node;
+	}
+
+	prop = fdt_get_property(dt_fdt(), node, "opal-base-address", &len);
+	if (!prop)
+		return;// len;
+	opal.base = fdt64_to_cpu(*(uint64_t *)prop->data);
+
+	prop = fdt_get_property(dt_fdt(), node, "opal-entry-address", &len);
+	if (!prop)
+		return;// len;
+	opal.entry = fdt64_to_cpu(*(uint64_t *)prop->data);
+}
+
+extern int64_t opal_call(int64_t token, int64_t arg1, int64_t arg2, int64_t arg3);
+
+extern void opal_power_off(void)
+{
+	opal_call(OPAL_CEC_POWER_DOWN, 0, 0, 0);
+}
+
+void opal_putchar(int c)
+{
+	unsigned long vty = 0;		/* 0 == default */
+	unsigned long nr_chars = cpu_to_be64(1);
+	char ch = c;
+
+	opal_call(OPAL_CONSOLE_WRITE, (int64_t)vty, (int64_t)&nr_chars, (int64_t)&ch);
+}
+
+int __opal_getchar(void)
+{
+	unsigned long vty = 0;		/* 0 == default */
+	unsigned long nr_chars = cpu_to_be64(1);
+	char ch;
+	int rc;
+
+	rc = opal_call(OPAL_CONSOLE_READ, (int64_t)vty, (int64_t)&nr_chars, (int64_t)&ch);
+	if (rc != OPAL_SUCCESS)
+		return -1;
+
+	return ch;
+}
diff --git a/powerpc/Makefile.ppc64 b/powerpc/Makefile.ppc64
index b0ed2b1..06a7cf6 100644
--- a/powerpc/Makefile.ppc64
+++ b/powerpc/Makefile.ppc64
@@ -17,6 +17,8 @@ cstart.o = $(TEST_DIR)/cstart64.o
 reloc.o  = $(TEST_DIR)/reloc64.o
 
 OBJDIRS += lib/ppc64
+cflatobjs += lib/ppc64/opal.o
+cflatobjs += lib/ppc64/opal-calls.o
 
 # ppc64 specific tests
 tests = $(TEST_DIR)/spapr_vpa.elf
diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
index 069d991..b5b3ec7 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S
@@ -92,6 +92,13 @@ start:
 	sync
 	isync
 
+	/* powernv machine does not check broken_sc1 */
+	mfmsr	r3
+	li	r4,1
+	sldi	r4,r4,MSR_HV_BIT
+	and.	r3,r3,r4
+	bne	1f
+
 	/* patch sc1 if needed */
 	bl	hcall_have_broken_sc1
 	cmpwi	r3, 0
diff --git a/powerpc/run b/powerpc/run
index ee38e07..2ec3df8 100755
--- a/powerpc/run
+++ b/powerpc/run
@@ -1,5 +1,14 @@
 #!/usr/bin/env bash
 
+get_qemu_machine ()
+{
+	if [ "$MACHINE" ]; then
+		echo $MACHINE
+	else
+		echo pseries
+	fi
+}
+
 if [ -z "$KUT_STANDALONE" ]; then
 	if [ ! -f config.mak ]; then
 		echo "run ./configure && make first. See ./configure -h"
@@ -12,17 +21,30 @@ fi
 ACCEL=$(get_qemu_accelerator) ||
 	exit $?
 
+MACHINE=$(get_qemu_machine) ||
+	exit $?
+
+if [ "$MACHINE" = "powernv" ] && [ "$ACCEL" = "kvm" ]; then
+	echo "powernv machine does not support KVM. ACCEL=tcg must be specified."
+	exit 2
+fi
+
 qemu=$(search_qemu_binary) ||
 	exit $?
 
-if ! $qemu -machine '?' 2>&1 | grep 'pseries' > /dev/null; then
-	echo "$qemu doesn't support pSeries ('-machine pseries'). Exiting."
+if ! $qemu -machine '?' 2>&1 | grep $MACHINE > /dev/null; then
+	echo "$qemu doesn't support '-machine $MACHINE'. Exiting."
 	exit 2
 fi
 
-M='-machine pseries'
+M="-machine $MACHINE"
 M+=",accel=$ACCEL"
-command="$qemu -nodefaults $M -bios $FIRMWARE"
+B=""
+if [ "$MACHINE" = "pseries" ] ; then
+	B+="-bios $FIRMWARE"
+fi
+
+command="$qemu -nodefaults $M $B"
 command+=" -display none -serial stdio -kernel"
 command="$(migration_cmd) $(timeout_cmd) $command"
 
-- 
2.37.2

Re: [kvm-unit-tests v2 09/10] powerpc: Support powernv machine with QEMU TCG

[Cédric Le Goater]

Hello Nick,

On 3/20/23 08:03, Nicholas Piggin wrote:
> This is a basic first pass at powernv support using OPAL (skiboot)
> firmware.
> 
> The ACCEL is a bit clunky, now defaulting to tcg for powernv machine.
> It also does not yet run in the run_tests.sh batch process, more work
> is needed to exclude certain tests (e.g., rtas) and adjust parameters
> (e.g., increase memory size) to allow powernv to work. For now it
> can run single test cases.

Why do you need to load OPAL ? for the shutdown ? because the UART ops
could be done directly using MMIOs on the LPC IO space.

> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   lib/powerpc/asm/ppc_asm.h   |  5 +++
>   lib/powerpc/asm/processor.h | 14 ++++++++
>   lib/powerpc/hcall.c         |  4 +--
>   lib/powerpc/io.c            | 33 ++++++++++++++++--
>   lib/powerpc/io.h            |  6 ++++
>   lib/powerpc/processor.c     | 10 ++++++
>   lib/powerpc/setup.c         | 10 ++++--
>   lib/ppc64/asm/opal.h        | 11 ++++++
>   lib/ppc64/opal-calls.S      | 46 +++++++++++++++++++++++++
>   lib/ppc64/opal.c            | 67 +++++++++++++++++++++++++++++++++++++
>   powerpc/Makefile.ppc64      |  2 ++
>   powerpc/cstart64.S          |  7 ++++
>   powerpc/run                 | 30 ++++++++++++++---
>   13 files changed, 234 insertions(+), 11 deletions(-)
>   create mode 100644 lib/ppc64/asm/opal.h
>   create mode 100644 lib/ppc64/opal-calls.S
>   create mode 100644 lib/ppc64/opal.c
> 
> diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
> index 6299ff5..5eec9d3 100644
> --- a/lib/powerpc/asm/ppc_asm.h
> +++ b/lib/powerpc/asm/ppc_asm.h
> @@ -36,7 +36,12 @@
>   #endif /* __BYTE_ORDER__ */
>   
>   /* Machine State Register definitions: */
> +#define MSR_LE_BIT	0
>   #define MSR_EE_BIT	15			/* External Interrupts Enable */
> +#define MSR_HV_BIT	60			/* Hypervisor mode */
>   #define MSR_SF_BIT	63			/* 64-bit mode */
>   
> +#define SPR_HSRR0	0x13A
> +#define SPR_HSRR1	0x13B
> +
>   #endif /* _ASMPOWERPC_PPC_ASM_H */
> diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
> index ebfeff2..8084787 100644
> --- a/lib/powerpc/asm/processor.h
> +++ b/lib/powerpc/asm/processor.h
> @@ -3,12 +3,26 @@
>   
>   #include <libcflat.h>
>   #include <asm/ptrace.h>
> +#include <asm/ppc_asm.h>
>   
>   #ifndef __ASSEMBLY__
>   void handle_exception(int trap, void (*func)(struct pt_regs *, void *), void *);
>   void do_handle_exception(struct pt_regs *regs);
>   #endif /* __ASSEMBLY__ */
>   
> +/*
> + * If this returns true on PowerNV / OPAL machines which run in hypervisor
> + * mode. False on pseries / PAPR machines that run in guest mode.
> + */
> +static inline bool machine_is_powernv(void)
> +{
> +	uint64_t msr;
> +
> +	asm volatile ("mfmsr %[msr]" : [msr] "=r" (msr));
> +
> +	return !!(msr & (1ULL << MSR_HV_BIT));
> +}
> +
>   static inline uint64_t get_tb(void)
>   {
>   	uint64_t tb;
> diff --git a/lib/powerpc/hcall.c b/lib/powerpc/hcall.c
> index 711cb1b..37e52f5 100644
> --- a/lib/powerpc/hcall.c
> +++ b/lib/powerpc/hcall.c
> @@ -25,7 +25,7 @@ int hcall_have_broken_sc1(void)
>   	return r3 == (unsigned long)H_PRIVILEGE;
>   }
>   
> -void putchar(int c)
> +void papr_putchar(int c)
>   {
>   	unsigned long vty = 0;		/* 0 == default */
>   	unsigned long nr_chars = 1;
> @@ -34,7 +34,7 @@ void putchar(int c)
>   	hcall(H_PUT_TERM_CHAR, vty, nr_chars, chars);
>   }
>   
> -int __getchar(void)
> +int __papr_getchar(void)
>   {
>   	register unsigned long r3 asm("r3") = H_GET_TERM_CHAR;
>   	register unsigned long r4 asm("r4") = 0; /* 0 == default vty */
> diff --git a/lib/powerpc/io.c b/lib/powerpc/io.c
> index a381688..a3a64ce 100644
> --- a/lib/powerpc/io.c
> +++ b/lib/powerpc/io.c
> @@ -9,13 +9,39 @@
>   #include <asm/spinlock.h>
>   #include <asm/rtas.h>
>   #include <asm/setup.h>
> +#include <asm/processor.h>
>   #include "io.h"
>   
>   static struct spinlock print_lock;
>   
> +struct opal {
> +	uint64_t base;
> +	uint64_t entry;
> +};
> +extern struct opal opal;
> +
> +void putchar(int c)
> +{
> +	if (machine_is_powernv())
> +		opal_putchar(c);
> +	else
> +		papr_putchar(c);
> +}
> +
> +int __getchar(void)
> +{
> +	if (machine_is_powernv())
> +		return __opal_getchar();
> +	else
> +		return __papr_getchar();
> +}
> +
>   void io_init(void)
>   {
> -	rtas_init();
> +	if (machine_is_powernv())
> +		opal_init();
> +	else
> +		rtas_init();
>   }
>   
>   void puts(const char *s)
> @@ -38,7 +64,10 @@ void exit(int code)
>   // FIXME: change this print-exit/rtas-poweroff to chr_testdev_exit(),
>   //        maybe by plugging chr-testdev into a spapr-vty.
>   	printf("\nEXIT: STATUS=%d\n", ((code) << 1) | 1);
> -	rtas_power_off();
> +	if (machine_is_powernv())
> +		opal_power_off();
> +	else
> +		rtas_power_off();
>   	halt(code);
>   	__builtin_unreachable();
>   }
> diff --git a/lib/powerpc/io.h b/lib/powerpc/io.h
> index d4f21ba..a2aed7b 100644
> --- a/lib/powerpc/io.h
> +++ b/lib/powerpc/io.h
> @@ -8,6 +8,12 @@
>   #define _POWERPC_IO_H_
>   
>   extern void io_init(void);
> +extern void opal_init(void);
> +extern void opal_power_off(void);
>   extern void putchar(int c);
> +extern void opal_putchar(int c);
> +extern void papr_putchar(int c);
> +extern int __opal_getchar(void);
> +extern int __papr_getchar(void);
>   
>   #endif
> diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
> index e77a240..06f6db3 100644
> --- a/lib/powerpc/processor.c
> +++ b/lib/powerpc/processor.c
> @@ -61,6 +61,16 @@ void sleep_tb(uint64_t cycles)
>   {
>   	uint64_t start, end, now;
>   
> +	if (machine_is_powernv()) {
> +		/*
> +		 * Could use 'stop' to sleep here which would be interesting.
> +		 * stop with ESL=0 should be simple enough, ESL=1 would require
> +		 * SRESET based wakeup which is more involved.
> +		 */
> +		delay(cycles);
> +		return;
> +	}
> +
>   	start = now = get_tb();
>   	end = start + cycles;
>   
> diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
> index 1be4c03..f327540 100644
> --- a/lib/powerpc/setup.c
> +++ b/lib/powerpc/setup.c
> @@ -18,6 +18,7 @@
>   #include <argv.h>
>   #include <asm/setup.h>
>   #include <asm/page.h>
> +#include <asm/processor.h>
>   #include <asm/hcall.h>
>   #include "io.h"
>   
> @@ -97,12 +98,13 @@ static void cpu_init(void)
>   	tb_hz = params.tb_hz;
>   
>   	/* Interrupt Endianness */
> -
> +	if (!machine_is_powernv()) {
>   #if  __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> -        hcall(H_SET_MODE, 1, 4, 0, 0);
> +		hcall(H_SET_MODE, 1, 4, 0, 0);
>   #else
> -        hcall(H_SET_MODE, 0, 4, 0, 0);
> +		hcall(H_SET_MODE, 0, 4, 0, 0);
>   #endif
> +	}
>   }
>   
>   static void mem_init(phys_addr_t freemem_start)
> @@ -195,6 +197,8 @@ void setup(const void *fdt)
>   		freemem += initrd_size;
>   	}
>   
> +	opal_init();
> +

This opal_init() call seems redundant with io_init().

C.

>   	/* call init functions */
>   	cpu_init();
>   
> diff --git a/lib/ppc64/asm/opal.h b/lib/ppc64/asm/opal.h
> new file mode 100644
> index 0000000..243a3c6
> --- /dev/null
> +++ b/lib/ppc64/asm/opal.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef _ASMPPC64_HCALL_H_
> +#define _ASMPPC64_HCALL_H_
> +
> +#define OPAL_SUCCESS				0
> +
> +#define OPAL_CONSOLE_WRITE			1
> +#define OPAL_CONSOLE_READ			2
> +#define OPAL_CEC_POWER_DOWN			5
> +
> +#endif
> diff --git a/lib/ppc64/opal-calls.S b/lib/ppc64/opal-calls.S
> new file mode 100644
> index 0000000..1833358
> --- /dev/null
> +++ b/lib/ppc64/opal-calls.S
> @@ -0,0 +1,46 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2016 IBM Corporation.
> + */
> +
> +#include <asm/ppc_asm.h>
> +
> +	.text
> +	.globl opal_call
> +opal_call:
> +	mr	r0,r3
> +	mr	r3,r4
> +	mr	r4,r5
> +	mr	r5,r6
> +	mr	r6,r7
> +	mflr	r11
> +	std	r11,16(r1)
> +	mfcr	r12
> +	stw	r12,8(r1)
> +	mr	r13,r2
> +
> +	/* Set opal return address */
> +	LOAD_REG_ADDR(r11, opal_return)
> +	mtlr	r11
> +	mfmsr	r12
> +
> +	/* switch to BE when we enter OPAL */
> +	li	r11,(1 << MSR_LE_BIT)
> +	andc	r12,r12,r11
> +	mtspr	SPR_HSRR1,r12
> +
> +	/* load the opal call entry point and base */
> +	LOAD_REG_ADDR(r11, opal)
> +	ld	r12,8(r11)
> +	ld	r2,0(r11)
> +	mtspr	SPR_HSRR0,r12
> +	hrfid
> +
> +opal_return:
> +	FIXUP_ENDIAN
> +	mr	r2,r13;
> +	lwz	r11,8(r1);
> +	ld	r12,16(r1)
> +	mtcr	r11;
> +	mtlr	r12
> +	blr
> diff --git a/lib/ppc64/opal.c b/lib/ppc64/opal.c
> new file mode 100644
> index 0000000..515e9ab
> --- /dev/null
> +++ b/lib/ppc64/opal.c
> @@ -0,0 +1,67 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * OPAL call helpers
> + */
> +#include <asm/opal.h>
> +#include <libcflat.h>
> +#include <libfdt/libfdt.h>
> +#include <devicetree.h>
> +//libfdt_env.h>
> +#include <asm/io.h>
> +#include "../powerpc/io.h"
> +
> +struct opal {
> +	uint64_t base;
> +	uint64_t entry;
> +} opal;
> +
> +void opal_init(void)
> +{
> +	const struct fdt_property *prop;
> +	int node, len;
> +
> +	node = fdt_path_offset(dt_fdt(), "/ibm,opal");
> +	if (node < 0) {
> +		return;// node;
> +	}
> +
> +	prop = fdt_get_property(dt_fdt(), node, "opal-base-address", &len);
> +	if (!prop)
> +		return;// len;
> +	opal.base = fdt64_to_cpu(*(uint64_t *)prop->data);
> +
> +	prop = fdt_get_property(dt_fdt(), node, "opal-entry-address", &len);
> +	if (!prop)
> +		return;// len;
> +	opal.entry = fdt64_to_cpu(*(uint64_t *)prop->data);
> +}
> +
> +extern int64_t opal_call(int64_t token, int64_t arg1, int64_t arg2, int64_t arg3);
> +
> +extern void opal_power_off(void)
> +{
> +	opal_call(OPAL_CEC_POWER_DOWN, 0, 0, 0);
> +}
> +
> +void opal_putchar(int c)
> +{
> +	unsigned long vty = 0;		/* 0 == default */
> +	unsigned long nr_chars = cpu_to_be64(1);
> +	char ch = c;
> +
> +	opal_call(OPAL_CONSOLE_WRITE, (int64_t)vty, (int64_t)&nr_chars, (int64_t)&ch);
> +}
> +
> +int __opal_getchar(void)
> +{
> +	unsigned long vty = 0;		/* 0 == default */
> +	unsigned long nr_chars = cpu_to_be64(1);
> +	char ch;
> +	int rc;
> +
> +	rc = opal_call(OPAL_CONSOLE_READ, (int64_t)vty, (int64_t)&nr_chars, (int64_t)&ch);
> +	if (rc != OPAL_SUCCESS)
> +		return -1;
> +
> +	return ch;
> +}
> diff --git a/powerpc/Makefile.ppc64 b/powerpc/Makefile.ppc64
> index b0ed2b1..06a7cf6 100644
> --- a/powerpc/Makefile.ppc64
> +++ b/powerpc/Makefile.ppc64
> @@ -17,6 +17,8 @@ cstart.o = $(TEST_DIR)/cstart64.o
>   reloc.o  = $(TEST_DIR)/reloc64.o
>   
>   OBJDIRS += lib/ppc64
> +cflatobjs += lib/ppc64/opal.o
> +cflatobjs += lib/ppc64/opal-calls.o
>   
>   # ppc64 specific tests
>   tests = $(TEST_DIR)/spapr_vpa.elf
> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
> index 069d991..b5b3ec7 100644
> --- a/powerpc/cstart64.S
> +++ b/powerpc/cstart64.S
> @@ -92,6 +92,13 @@ start:
>   	sync
>   	isync
>   
> +	/* powernv machine does not check broken_sc1 */
> +	mfmsr	r3
> +	li	r4,1
> +	sldi	r4,r4,MSR_HV_BIT
> +	and.	r3,r3,r4
> +	bne	1f
> +
>   	/* patch sc1 if needed */
>   	bl	hcall_have_broken_sc1
>   	cmpwi	r3, 0
> diff --git a/powerpc/run b/powerpc/run
> index ee38e07..2ec3df8 100755
> --- a/powerpc/run
> +++ b/powerpc/run
> @@ -1,5 +1,14 @@
>   #!/usr/bin/env bash
>   
> +get_qemu_machine ()
> +{
> +	if [ "$MACHINE" ]; then
> +		echo $MACHINE
> +	else
> +		echo pseries
> +	fi
> +}
> +
>   if [ -z "$KUT_STANDALONE" ]; then
>   	if [ ! -f config.mak ]; then
>   		echo "run ./configure && make first. See ./configure -h"
> @@ -12,17 +21,30 @@ fi
>   ACCEL=$(get_qemu_accelerator) ||
>   	exit $?
>   
> +MACHINE=$(get_qemu_machine) ||
> +	exit $?
> +
> +if [ "$MACHINE" = "powernv" ] && [ "$ACCEL" = "kvm" ]; then
> +	echo "powernv machine does not support KVM. ACCEL=tcg must be specified."
> +	exit 2
> +fi
> +
>   qemu=$(search_qemu_binary) ||
>   	exit $?
>   
> -if ! $qemu -machine '?' 2>&1 | grep 'pseries' > /dev/null; then
> -	echo "$qemu doesn't support pSeries ('-machine pseries'). Exiting."
> +if ! $qemu -machine '?' 2>&1 | grep $MACHINE > /dev/null; then
> +	echo "$qemu doesn't support '-machine $MACHINE'. Exiting."
>   	exit 2
>   fi
>   
> -M='-machine pseries'
> +M="-machine $MACHINE"
>   M+=",accel=$ACCEL"
> -command="$qemu -nodefaults $M -bios $FIRMWARE"
> +B=""
> +if [ "$MACHINE" = "pseries" ] ; then
> +	B+="-bios $FIRMWARE"
> +fi
> +
> +command="$qemu -nodefaults $M $B"
>   command+=" -display none -serial stdio -kernel"
>   command="$(migration_cmd) $(timeout_cmd) $command"
>   

Re: [kvm-unit-tests v2 09/10] powerpc: Support powernv machine with QEMU TCG

[Nicholas Piggin]

On Mon Mar 20, 2023 at 7:47 PM AEST, Cédric Le Goater wrote:
> Hello Nick,
>
> On 3/20/23 08:03, Nicholas Piggin wrote:
> > This is a basic first pass at powernv support using OPAL (skiboot)
> > firmware.
> > 
> > The ACCEL is a bit clunky, now defaulting to tcg for powernv machine.
> > It also does not yet run in the run_tests.sh batch process, more work
> > is needed to exclude certain tests (e.g., rtas) and adjust parameters
> > (e.g., increase memory size) to allow powernv to work. For now it
> > can run single test cases.
>
> Why do you need to load OPAL ? for the shutdown ? because the UART ops
> could be done directly using MMIOs on the LPC IO space.

Don't really need it but I thought it would be easier to begin with, and
then I thought actually it's nice to have this kind of test harness for
skiboot as well. So I would hope to keep the skiboot bios option even if
a no-bios version was done.

[...]

> >   void io_init(void)
> >   {
> > -	rtas_init();
> > +	if (machine_is_powernv())
> > +		opal_init();
> > +	else
> > +		rtas_init();
> >   }

[...]

> > @@ -195,6 +197,8 @@ void setup(const void *fdt)
> >   		freemem += initrd_size;
> >   	}
> >   
> > +	opal_init();
> > +
>
> This opal_init() call seems redundant with io_init().

Oh you're right good catch, that might be an old piece before I cleaned
it up. I'll have to fix that and re-test it.

Thanks,
Nick

Re: [kvm-unit-tests v2 09/10] powerpc: Support powernv machine with QEMU TCG

[Thomas Huth]

On 20/03/2023 08.03, Nicholas Piggin wrote:
> This is a basic first pass at powernv support using OPAL (skiboot)
> firmware.
> 
> The ACCEL is a bit clunky, now defaulting to tcg for powernv machine.
> It also does not yet run in the run_tests.sh batch process, more work
> is needed to exclude certain tests (e.g., rtas) and adjust parameters
> (e.g., increase memory size) to allow powernv to work. For now it
> can run single test cases.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   lib/powerpc/asm/ppc_asm.h   |  5 +++
>   lib/powerpc/asm/processor.h | 14 ++++++++
>   lib/powerpc/hcall.c         |  4 +--
>   lib/powerpc/io.c            | 33 ++++++++++++++++--
>   lib/powerpc/io.h            |  6 ++++
>   lib/powerpc/processor.c     | 10 ++++++
>   lib/powerpc/setup.c         | 10 ++++--
>   lib/ppc64/asm/opal.h        | 11 ++++++
>   lib/ppc64/opal-calls.S      | 46 +++++++++++++++++++++++++
>   lib/ppc64/opal.c            | 67 +++++++++++++++++++++++++++++++++++++
>   powerpc/Makefile.ppc64      |  2 ++
>   powerpc/cstart64.S          |  7 ++++
>   powerpc/run                 | 30 ++++++++++++++---
>   13 files changed, 234 insertions(+), 11 deletions(-)
>   create mode 100644 lib/ppc64/asm/opal.h
>   create mode 100644 lib/ppc64/opal-calls.S
>   create mode 100644 lib/ppc64/opal.c
> 
> diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
> index 6299ff5..5eec9d3 100644
> --- a/lib/powerpc/asm/ppc_asm.h
> +++ b/lib/powerpc/asm/ppc_asm.h
> @@ -36,7 +36,12 @@
>   #endif /* __BYTE_ORDER__ */
>   
>   /* Machine State Register definitions: */
> +#define MSR_LE_BIT	0
>   #define MSR_EE_BIT	15			/* External Interrupts Enable */
> +#define MSR_HV_BIT	60			/* Hypervisor mode */
>   #define MSR_SF_BIT	63			/* 64-bit mode */
>   
> +#define SPR_HSRR0	0x13A
> +#define SPR_HSRR1	0x13B
> +
>   #endif /* _ASMPOWERPC_PPC_ASM_H */
> diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
> index ebfeff2..8084787 100644
> --- a/lib/powerpc/asm/processor.h
> +++ b/lib/powerpc/asm/processor.h
> @@ -3,12 +3,26 @@
>   
>   #include <libcflat.h>
>   #include <asm/ptrace.h>
> +#include <asm/ppc_asm.h>
>   
>   #ifndef __ASSEMBLY__
>   void handle_exception(int trap, void (*func)(struct pt_regs *, void *), void *);
>   void do_handle_exception(struct pt_regs *regs);
>   #endif /* __ASSEMBLY__ */
>   
> +/*
> + * If this returns true on PowerNV / OPAL machines which run in hypervisor
> + * mode. False on pseries / PAPR machines that run in guest mode.

s/If this/This/

  Thomas

[kvm-unit-tests v2 10/10] powerpc/sprs: Test hypervisor registers on powernv machine

[Nicholas Piggin]

This enables HV privilege registers to be tested with the powernv
machine.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 powerpc/sprs.c | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index dd83dac..a7878ff 100644
--- a/powerpc/sprs.c
+++ b/powerpc/sprs.c
@@ -199,16 +199,16 @@ static const struct spr sprs_power_common[1024] = {
 [190] = {"HFSCR",	64,	HV_RW, },
 [256] = {"VRSAVE",	32,	RW, },
 [259] = {"SPRG3",	64,	RO, },
-[284] = {"TBL",		32,	HV_WO, },
-[285] = {"TBU",		32,	HV_WO, },
-[286] = {"TBU40",	64,	HV_WO, },
+[284] = {"TBL",		32,	HV_WO, }, /* Things can go a bit wonky with */
+[285] = {"TBU",		32,	HV_WO, }, /* Timebase changing. Should save */
+[286] = {"TBU40",	64,	HV_WO, }, /* and restore it. */
 [304] = {"HSPRG0",	64,	HV_RW, },
 [305] = {"HSPRG1",	64,	HV_RW, },
 [306] = {"HDSISR",	32,	HV_RW,		SPR_INT, },
 [307] = {"HDAR",	64,	HV_RW,		SPR_INT, },
 [308] = {"SPURR",	64,	HV_RW | OS_RO,	SPR_ASYNC, },
 [309] = {"PURR",	64,	HV_RW | OS_RO,	SPR_ASYNC, },
-[313] = {"HRMOR",	64,	HV_RW, },
+[313] = {"HRMOR",	64,	HV_RW,		SPR_HARNESS, }, /* Harness can't cope with HRMOR changing */
 [314] = {"HSRR0",	64,	HV_RW,		SPR_INT, },
 [315] = {"HSRR1",	64,	HV_RW,		SPR_INT, },
 [318] = {"LPCR",	64,	HV_RW, },
@@ -350,6 +350,22 @@ static const struct spr sprs_power10_pmu[1024] = {
 
 static struct spr sprs[1024];
 
+static bool spr_read_perms(int spr)
+{
+	if (machine_is_powernv())
+		return !!(sprs[spr].access & SPR_HV_READ);
+	else
+		return !!(sprs[spr].access & SPR_OS_READ);
+}
+
+static bool spr_write_perms(int spr)
+{
+	if (machine_is_powernv())
+		return !!(sprs[spr].access & SPR_HV_WRITE);
+	else
+		return !!(sprs[spr].access & SPR_OS_WRITE);
+}
+
 static void setup_sprs(void)
 {
 	uint32_t pvr = mfspr(287);	/* Processor Version Register */
@@ -462,7 +478,7 @@ static void get_sprs(uint64_t *v)
 	int i;
 
 	for (i = 0; i < 1024; i++) {
-		if (!(sprs[i].access & SPR_OS_READ))
+		if (!spr_read_perms(i))
 			continue;
 		v[i] = mfspr(i);
 	}
@@ -473,8 +489,9 @@ static void set_sprs(uint64_t val)
 	int i;
 
 	for (i = 0; i < 1024; i++) {
-		if (!(sprs[i].access & SPR_OS_WRITE))
+		if (!spr_write_perms(i))
 			continue;
+
 		if (sprs[i].type & SPR_HARNESS)
 			continue;
 		if (!strcmp(sprs[i].name, "MMCR0")) {
@@ -546,7 +563,7 @@ int main(int argc, char **argv)
 	for (i = 0; i < 1024; i++) {
 		bool pass = true;
 
-		if (!(sprs[i].access & SPR_OS_READ))
+		if (!spr_read_perms(i))
 			continue;
 
 		if (sprs[i].width == 32) {
-- 
2.37.2

Re: [kvm-unit-tests v2 10/10] powerpc/sprs: Test hypervisor registers on powernv machine

[Thomas Huth]

On 20/03/2023 08.03, Nicholas Piggin wrote:
> This enables HV privilege registers to be tested with the powernv
> machine.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   powerpc/sprs.c | 31 ++++++++++++++++++++++++-------
>   1 file changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> index dd83dac..a7878ff 100644
> --- a/powerpc/sprs.c
> +++ b/powerpc/sprs.c
> @@ -199,16 +199,16 @@ static const struct spr sprs_power_common[1024] = {
>   [190] = {"HFSCR",	64,	HV_RW, },
>   [256] = {"VRSAVE",	32,	RW, },
>   [259] = {"SPRG3",	64,	RO, },
> -[284] = {"TBL",		32,	HV_WO, },
> -[285] = {"TBU",		32,	HV_WO, },
> -[286] = {"TBU40",	64,	HV_WO, },
> +[284] = {"TBL",		32,	HV_WO, }, /* Things can go a bit wonky with */
> +[285] = {"TBU",		32,	HV_WO, }, /* Timebase changing. Should save */
> +[286] = {"TBU40",	64,	HV_WO, }, /* and restore it. */
>   [304] = {"HSPRG0",	64,	HV_RW, },
>   [305] = {"HSPRG1",	64,	HV_RW, },
>   [306] = {"HDSISR",	32,	HV_RW,		SPR_INT, },
>   [307] = {"HDAR",	64,	HV_RW,		SPR_INT, },
>   [308] = {"SPURR",	64,	HV_RW | OS_RO,	SPR_ASYNC, },
>   [309] = {"PURR",	64,	HV_RW | OS_RO,	SPR_ASYNC, },
> -[313] = {"HRMOR",	64,	HV_RW, },
> +[313] = {"HRMOR",	64,	HV_RW,		SPR_HARNESS, }, /* Harness can't cope with HRMOR changing */
>   [314] = {"HSRR0",	64,	HV_RW,		SPR_INT, },
>   [315] = {"HSRR1",	64,	HV_RW,		SPR_INT, },
>   [318] = {"LPCR",	64,	HV_RW, },
> @@ -350,6 +350,22 @@ static const struct spr sprs_power10_pmu[1024] = {
>   
>   static struct spr sprs[1024];
>   
> +static bool spr_read_perms(int spr)
> +{
> +	if (machine_is_powernv())
> +		return !!(sprs[spr].access & SPR_HV_READ);
> +	else
> +		return !!(sprs[spr].access & SPR_OS_READ);
> +}
> +
> +static bool spr_write_perms(int spr)
> +{
> +	if (machine_is_powernv())
> +		return !!(sprs[spr].access & SPR_HV_WRITE);
> +	else
> +		return !!(sprs[spr].access & SPR_OS_WRITE);
> +}
> +
>   static void setup_sprs(void)
>   {
>   	uint32_t pvr = mfspr(287);	/* Processor Version Register */
> @@ -462,7 +478,7 @@ static void get_sprs(uint64_t *v)
>   	int i;
>   
>   	for (i = 0; i < 1024; i++) {
> -		if (!(sprs[i].access & SPR_OS_READ))
> +		if (!spr_read_perms(i))
>   			continue;
>   		v[i] = mfspr(i);
>   	}
> @@ -473,8 +489,9 @@ static void set_sprs(uint64_t val)
>   	int i;
>   
>   	for (i = 0; i < 1024; i++) {
> -		if (!(sprs[i].access & SPR_OS_WRITE))
> +		if (!spr_write_perms(i))
>   			continue;
> +
>   		if (sprs[i].type & SPR_HARNESS)
>   			continue;
>   		if (!strcmp(sprs[i].name, "MMCR0")) {
> @@ -546,7 +563,7 @@ int main(int argc, char **argv)
>   	for (i = 0; i < 1024; i++) {
>   		bool pass = true;
>   
> -		if (!(sprs[i].access & SPR_OS_READ))
> +		if (!spr_read_perms(i))
>   			continue;
>   
>   		if (sprs[i].width == 32) {

Acked-by: Thomas Huth <thuth@redhat.com>

Re: [kvm-unit-tests v2 00/10] powerpc: updates, P10, PNV support

[Nicholas Piggin]

On Mon Mar 20, 2023 at 5:03 PM AEST, Nicholas Piggin wrote:
> Since v1 series, I fixed the sleep API and implementation in patch 2
> as noted by Thomas. Added usleep and msleep variants to match [um]delay
> we already have.
>
> Also some minor tidy ups and fixes mainly with reporting format in the
> sprs test rework.
>
> And added PowerNV support to the harness with the 3 new patches at the
> end because it didn't turn out to be too hard. We could parse the dt to
> get a console UART directly for a really minimal firmware, but it is
> better for us to have a test harness like this that can also be used for
> skiboot testing.

I'll send out one more series, I have a couple of fixes for PowerNV code
(I didn't make the OPAL call to set interrupts little endian for LE
builds, for one). I'll wait for a week or so for more feedback though.

Thanks,
Nick