[kvm-unit-tests PATCH v5 00/29] powerpc: updates, P10, PNV support

[kvm-unit-tests PATCH v5 00/29] powerpc: updates, P10, PNV support

[Nicholas Piggin]

This has taken quite a while to get right, but I think is getting
much better now.

There are several semi-related things here: multi-migration support,
bugfixing, adding powernv machine, expanding and adding test cases.
But I found it easiest to leave it as one series. Multi-migration I
added when testing and debugging time migration issues in QEMU, and
that has grown to quite a size, I can try to split that out if
preferred.

Since v4:
- Multi migration now seems quite solid, I haven't broken it. Arm64
  migration regression reported by Shaoqin is fixed, it was due to an exit
  status getting lost.
- Several other small migration fixes and cleanups that poppedup in
  testing.
- More complete SMP support including IPI for pseries and powernv.
- Made powernv a first-class citizen that can do run_tests.sh unit
  tests (with machine option).
- More polished, quiet warnings, skip unsupported tests, etc.
- Fix several powerpc bugs that got exposed (.got, stack backtrace,
  stack alignment).
- Added a bunch more tests I've been accumulating, atomics, smp,
  timebase, interrupts.

Note the arm64 psci cpu-on test is flakey with/without this series.

Thanks,
Nick


Nicholas Piggin (29):
  arch-run: Clean up temporary files properly
  arch-run: Clean up initrd cleanup
  migration: use a more robust way to wait for background job
  migration: Support multiple migrations
  arch-run: rename migration variables
  powerpc: Quiet QEMU TCG pseries capability warnings
  powerpc: Add a migration stress tester
  powerpc: Require KVM for the TM test
  powerpc: Fix interrupt stack alignment
  powerpc/sprs: Specify SPRs with data rather than code
  powerpc/sprs: Don't fail changed SPRs that are used by the test
    harness
  powerpc/sprs: Avoid taking async interrupts caused by register fuzzing
  powerpc: Make interrupt handler error more readable
  powerpc: Expand exception handler vector granularity
  powerpc: Add support for more interrupts including HV interrupts
  powerpc: Set .got section alignment to 256 bytes
  powerpc: Discover runtime load address dynamically
  powerpc: Fix stack backtrace termination
  scripts: allow machine option to be specified in unittests.cfg
  scripts: Accommodate powerpc powernv machine differences
  powerpc: Support powernv machine with QEMU TCG
  powerpc: Fix emulator illegal instruction test for powernv
  powerpc/sprs: Test hypervisor registers on powernv machine
  powerpc: interrupt tests
  powerpc: Add rtas stop-self support
  powerpc: add SMP and IPI support
  powerpc: Avoid using larx/stcx. in spinlocks when only one CPU is
    running
  powerpc: Add atomics tests
  powerpc: Add timebase tests

 lib/migrate.c               |   8 +-
 lib/migrate.h               |   1 +
 lib/powerpc/asm/ppc_asm.h   |  25 ++
 lib/powerpc/asm/processor.h |  41 +++
 lib/powerpc/asm/rtas.h      |   2 +
 lib/powerpc/asm/setup.h     |   3 +-
 lib/powerpc/asm/smp.h       |  24 +-
 lib/powerpc/hcall.c         |   4 +-
 lib/powerpc/io.c            |  27 +-
 lib/powerpc/io.h            |   6 +
 lib/powerpc/processor.c     |  55 ++-
 lib/powerpc/rtas.c          |  78 ++++-
 lib/powerpc/setup.c         |  50 ++-
 lib/powerpc/smp.c           | 270 +++++++++++++--
 lib/powerpc/spinlock.c      |  28 ++
 lib/ppc64/asm/atomic.h      |   6 +
 lib/ppc64/asm/opal.h        |  20 ++
 lib/ppc64/asm/ptrace.h      |   1 +
 lib/ppc64/asm/spinlock.h    |   7 +-
 lib/ppc64/opal-calls.S      |  46 +++
 lib/ppc64/opal.c            |  76 +++++
 powerpc/Makefile.common     |   8 +-
 powerpc/Makefile.ppc64      |   2 +
 powerpc/atomics.c           | 190 +++++++++++
 powerpc/cstart64.S          | 163 +++++++--
 powerpc/emulator.c          |  19 +-
 powerpc/flat.lds            |   3 +-
 powerpc/interrupts.c        | 412 +++++++++++++++++++++++
 powerpc/migrate.c           |  64 ++++
 powerpc/run                 |  39 ++-
 powerpc/smp.c               | 199 +++++++++++
 powerpc/sprs.c              | 642 +++++++++++++++++++++++++-----------
 powerpc/timebase.c          | 328 ++++++++++++++++++
 powerpc/tm.c                |   2 +-
 powerpc/unittests.cfg       |  37 +++
 scripts/arch-run.bash       | 181 +++++++---
 scripts/common.bash         |   8 +-
 scripts/runtime.bash        |  20 +-
 38 files changed, 2736 insertions(+), 359 deletions(-)
 create mode 100644 lib/powerpc/spinlock.c
 create mode 100644 lib/ppc64/asm/atomic.h
 create mode 100644 lib/ppc64/asm/opal.h
 create mode 100644 lib/ppc64/opal-calls.S
 create mode 100644 lib/ppc64/opal.c
 create mode 100644 powerpc/atomics.c
 create mode 100644 powerpc/interrupts.c
 create mode 100644 powerpc/migrate.c
 create mode 100644 powerpc/smp.c
 create mode 100644 powerpc/timebase.c

-- 
2.42.0

[kvm-unit-tests PATCH v5 01/29] arch-run: Clean up temporary files properly

[Nicholas Piggin]

Migration files weren't being removed when tests were interrupted.
This seems to improve the situation.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 scripts/arch-run.bash | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index d0864360..f22ead6f 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -134,12 +134,14 @@ run_migration ()
 	qmp1=$(mktemp -u -t mig-helper-qmp1.XXXXXXXXXX)
 	qmp2=$(mktemp -u -t mig-helper-qmp2.XXXXXXXXXX)
 	fifo=$(mktemp -u -t mig-helper-fifo.XXXXXXXXXX)
+
+	# race here between file creation and trap
+	trap "trap - TERM ; kill 0 ; exit 2" INT TERM
+	trap "rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}" RETURN EXIT
+
 	qmpout1=/dev/null
 	qmpout2=/dev/null
 
-	trap 'kill 0; exit 2' INT TERM
-	trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}' RETURN EXIT
-
 	eval "$@" -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \
 		-mon chardev=mon1,mode=control | tee ${migout1} &
 	live_pid=`jobs -l %+ | grep "eval" | awk '{print$2}'`
@@ -211,8 +213,8 @@ run_panic ()
 
 	qmp=$(mktemp -u -t panic-qmp.XXXXXXXXXX)
 
-	trap 'kill 0; exit 2' INT TERM
-	trap 'rm -f ${qmp}' RETURN EXIT
+	trap "trap - TERM ; kill 0 ; exit 2" INT TERM
+	trap "rm -f ${qmp}" RETURN EXIT
 
 	# start VM stopped so we don't miss any events
 	eval "$@" -chardev socket,id=mon1,path=${qmp},server=on,wait=off \
-- 
2.42.0

[kvm-unit-tests PATCH v5 02/29] arch-run: Clean up initrd cleanup

[Nicholas Piggin]

Rather than put a big script into the trap handler, have it call
a function.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 scripts/arch-run.bash | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index f22ead6f..cc7da7c5 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -271,10 +271,20 @@ search_qemu_binary ()
 	export PATH=$save_path
 }
 
+initrd_cleanup ()
+{
+	if [ "$KVM_UNIT_TESTS_ENV_OLD" ]; then
+		export KVM_UNIT_TESTS_ENV="$KVM_UNIT_TESTS_ENV_OLD"
+	else
+		unset KVM_UNIT_TESTS_ENV
+		unset KVM_UNIT_TESTS_ENV_OLD
+	fi
+}
+
 initrd_create ()
 {
 	if [ "$ENVIRON_DEFAULT" = "yes" ]; then
-		trap_exit_push 'rm -f $KVM_UNIT_TESTS_ENV; [ "$KVM_UNIT_TESTS_ENV_OLD" ] && export KVM_UNIT_TESTS_ENV="$KVM_UNIT_TESTS_ENV_OLD" || unset KVM_UNIT_TESTS_ENV; unset KVM_UNIT_TESTS_ENV_OLD'
+		trap_exit_push 'rm -f $KVM_UNIT_TESTS_ENV; initrd_cleanup'
 		[ -f "$KVM_UNIT_TESTS_ENV" ] && export KVM_UNIT_TESTS_ENV_OLD="$KVM_UNIT_TESTS_ENV"
 		export KVM_UNIT_TESTS_ENV=$(mktemp)
 		env_params
-- 
2.42.0

[kvm-unit-tests PATCH v5 03/29] migration: use a more robust way to wait for background job

[Nicholas Piggin]

Starting a pipeline of jobs in the background seems to not have a
simple way to reliably find the pid of a particular process. The
way PID of QEMU is sometimes causes a failure waiting on a PID that
is not running when stressing migration with later changes to do
multiple migrations.

Changing this to use $! is more robust in testing, and simpler.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 scripts/arch-run.bash | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index cc7da7c5..4d4e791c 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -131,6 +131,7 @@ run_migration ()
 
 	migsock=$(mktemp -u -t mig-helper-socket.XXXXXXXXXX)
 	migout1=$(mktemp -t mig-helper-stdout1.XXXXXXXXXX)
+	migout_fifo1=$(mktemp -u -t mig-helper-fifo-stdout1.XXXXXXXXXX)
 	qmp1=$(mktemp -u -t mig-helper-qmp1.XXXXXXXXXX)
 	qmp2=$(mktemp -u -t mig-helper-qmp2.XXXXXXXXXX)
 	fifo=$(mktemp -u -t mig-helper-fifo.XXXXXXXXXX)
@@ -143,8 +144,9 @@ run_migration ()
 	qmpout2=/dev/null
 
 	eval "$@" -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \
-		-mon chardev=mon1,mode=control | tee ${migout1} &
-	live_pid=`jobs -l %+ | grep "eval" | awk '{print$2}'`
+		-mon chardev=mon1,mode=control > ${migout_fifo1} &
+	live_pid=$!
+	cat ${migout_fifo1} | tee ${migout1} &
 
 	# We have to use cat to open the named FIFO, because named FIFO's, unlike
 	# pipes, will block on open() until the other end is also opened, and that
@@ -152,7 +154,7 @@ run_migration ()
 	mkfifo ${fifo}
 	eval "$@" -chardev socket,id=mon2,path=${qmp2},server=on,wait=off \
 		-mon chardev=mon2,mode=control -incoming unix:${migsock} < <(cat ${fifo}) &
-	incoming_pid=`jobs -l %+ | awk '{print$2}'`
+	incoming_pid=$!
 
 	# The test must prompt the user to migrate, so wait for the "migrate" keyword
 	while ! grep -q -i "Now migrate the VM" < ${migout1} ; do
@@ -166,6 +168,9 @@ run_migration ()
 		sleep 1
 	done
 
+	# Wait until the destination has created the incoming socket
+	while ! [ -S ${migsock} ] ; do sleep 0.1 ; done
+
 	qmp ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > ${qmpout1}
 
 	# Wait for the migration to complete
-- 
2.42.0

[kvm-unit-tests PATCH v5 04/29] migration: Support multiple migrations

[Nicholas Piggin]

Support multiple migrations by flipping dest file/socket variables to
source after the migration is complete, ready to start again. A new
destination is created if the test outputs the migrate line again.
Test cases may now switch to calling migrate() one or more times.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 lib/migrate.c         |  8 ++--
 lib/migrate.h         |  1 +
 scripts/arch-run.bash | 94 +++++++++++++++++++++++++++++++++++++------
 3 files changed, 86 insertions(+), 17 deletions(-)

diff --git a/lib/migrate.c b/lib/migrate.c
index 527e63ae..b7721659 100644
--- a/lib/migrate.c
+++ b/lib/migrate.c
@@ -8,8 +8,10 @@
 #include <libcflat.h>
 #include "migrate.h"
 
-/* static for now since we only support migrating exactly once per test. */
-static void migrate(void)
+/*
+ * Initiate migration and wait for it to complete.
+ */
+void migrate(void)
 {
 	puts("Now migrate the VM, then press a key to continue...\n");
 	(void)getchar();
@@ -19,8 +21,6 @@ static void migrate(void)
 /*
  * Initiate migration and wait for it to complete.
  * If this function is called more than once, it is a no-op.
- * Since migrate_cmd can only migrate exactly once this function can
- * simplify the control flow, especially when skipping tests.
  */
 void migrate_once(void)
 {
diff --git a/lib/migrate.h b/lib/migrate.h
index 3c94e6af..2af06a72 100644
--- a/lib/migrate.h
+++ b/lib/migrate.h
@@ -6,4 +6,5 @@
  * Author: Nico Boehr <nrb@linux.ibm.com>
  */
 
+void migrate(void);
 void migrate_once(void);
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 4d4e791c..02b15b4b 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -132,29 +132,77 @@ run_migration ()
 	migsock=$(mktemp -u -t mig-helper-socket.XXXXXXXXXX)
 	migout1=$(mktemp -t mig-helper-stdout1.XXXXXXXXXX)
 	migout_fifo1=$(mktemp -u -t mig-helper-fifo-stdout1.XXXXXXXXXX)
+	migout2=$(mktemp -t mig-helper-stdout2.XXXXXXXXXX)
+	migout_fifo2=$(mktemp -u -t mig-helper-fifo-stdout2.XXXXXXXXXX)
 	qmp1=$(mktemp -u -t mig-helper-qmp1.XXXXXXXXXX)
 	qmp2=$(mktemp -u -t mig-helper-qmp2.XXXXXXXXXX)
 	fifo=$(mktemp -u -t mig-helper-fifo.XXXXXXXXXX)
 
 	# race here between file creation and trap
 	trap "trap - TERM ; kill 0 ; exit 2" INT TERM
-	trap "rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}" RETURN EXIT
+	trap "rm -f ${migout1} ${migout2} ${migout_fifo1} ${migout_fifo2} ${migsock} ${qmp1} ${qmp2} ${fifo}" RETURN EXIT
 
 	qmpout1=/dev/null
 	qmpout2=/dev/null
+	migcmdline=$@
 
-	eval "$@" -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \
+	mkfifo ${migout_fifo1}
+	mkfifo ${migout_fifo2}
+
+	eval "$migcmdline" \
+		-chardev socket,id=mon1,path=${qmp1},server=on,wait=off \
 		-mon chardev=mon1,mode=control > ${migout_fifo1} &
 	live_pid=$!
 	cat ${migout_fifo1} | tee ${migout1} &
 
-	# We have to use cat to open the named FIFO, because named FIFO's, unlike
-	# pipes, will block on open() until the other end is also opened, and that
-	# totally breaks QEMU...
+	# The test must prompt the user to migrate, so wait for the "migrate"
+	# keyword
+	while ! grep -q -i "Now migrate the VM" < ${migout1} ; do
+		if ! ps -p ${live_pid} > /dev/null ; then
+			echo "ERROR: Test exit before migration point." >&2
+			echo > ${fifo}
+			qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
+			return 3
+		fi
+		sleep 0.1
+	done
+
+	# This starts the first source QEMU in advance of the test reaching the
+	# migration point, since we expect at least one migration. Subsequent
+	# sources are started as the test hits migrate keywords.
+	do_migration || return $?
+
+	while ps -p ${live_pid} > /dev/null ; do
+		# Wait for EXIT or further migrations
+		if ! grep -q -i "Now migrate the VM" < ${migout1} ; then
+			sleep 0.1
+		else
+			do_migration || return $?
+		fi
+	done
+
+	wait ${live_pid}
+	ret=$?
+
+	while (( $(jobs -r | wc -l) > 0 )); do
+		sleep 0.1
+	done
+
+	return $ret
+}
+
+do_migration ()
+{
+	# We have to use cat to open the named FIFO, because named FIFO's,
+	# unlike pipes, will block on open() until the other end is also
+	# opened, and that totally breaks QEMU...
 	mkfifo ${fifo}
-	eval "$@" -chardev socket,id=mon2,path=${qmp2},server=on,wait=off \
-		-mon chardev=mon2,mode=control -incoming unix:${migsock} < <(cat ${fifo}) &
+	eval "$migcmdline" \
+		-chardev socket,id=mon2,path=${qmp2},server=on,wait=off \
+		-mon chardev=mon2,mode=control -incoming unix:${migsock} \
+		< <(cat ${fifo}) > ${migout_fifo2} &
 	incoming_pid=$!
+	cat ${migout_fifo2} | tee ${migout2} &
 
 	# The test must prompt the user to migrate, so wait for the "migrate" keyword
 	while ! grep -q -i "Now migrate the VM" < ${migout1} ; do
@@ -165,7 +213,7 @@ run_migration ()
 			qmp ${qmp2} '"quit"'> ${qmpout2} 2>/dev/null
 			return 3
 		fi
-		sleep 1
+		sleep 0.1
 	done
 
 	# Wait until the destination has created the incoming socket
@@ -176,7 +224,7 @@ run_migration ()
 	# Wait for the migration to complete
 	migstatus=`qmp ${qmp1} '"query-migrate"' | grep return`
 	while ! grep -q '"completed"' <<<"$migstatus" ; do
-		sleep 1
+		sleep 0.1
 		if ! migstatus=`qmp ${qmp1} '"query-migrate"'`; then
 			echo "ERROR: Querying migration state failed." >&2
 			echo > ${fifo}
@@ -192,14 +240,34 @@ run_migration ()
 			return 2
 		fi
 	done
+
 	qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
+
+	# keypress to dst so getchar completes and test continues
 	echo > ${fifo}
-	wait $incoming_pid
+	rm ${fifo}
+
+	# Ensure the incoming socket is removed, ready for next destination
+	if [ -S ${migsock} ] ; then
+		echo "ERROR: Incoming migration socket not removed after migration." >& 2
+		qmp ${qmp2} '"quit"'> ${qmpout2} 2>/dev/null
+		return 2
+	fi
+
+	wait ${live_pid}
 	ret=$?
 
-	while (( $(jobs -r | wc -l) > 0 )); do
-		sleep 0.5
-	done
+	# Now flip the variables because dest becomes source
+	live_pid=${incoming_pid}
+	tmp=${migout1}
+	migout1=${migout2}
+	migout2=${tmp}
+	tmp=${migout_fifo1}
+	migout_fifo1=${migout_fifo2}
+	migout_fifo2=${tmp}
+	tmp=${qmp1}
+	qmp1=${qmp2}
+	qmp2=${tmp}
 
 	return $ret
 }
-- 
2.42.0

[kvm-unit-tests PATCH v5 05/29] arch-run: rename migration variables

[Nicholas Piggin]

Using 1 and 2 for source and destination is confusing, particularly
now with multiple migrations that flip between them. Do a rename
pass to tidy things up.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 scripts/arch-run.bash | 110 +++++++++++++++++++++---------------------
 1 file changed, 56 insertions(+), 54 deletions(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 02b15b4b..c1095478 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -129,39 +129,40 @@ run_migration ()
 		return 77
 	fi
 
-	migsock=$(mktemp -u -t mig-helper-socket.XXXXXXXXXX)
-	migout1=$(mktemp -t mig-helper-stdout1.XXXXXXXXXX)
-	migout_fifo1=$(mktemp -u -t mig-helper-fifo-stdout1.XXXXXXXXXX)
-	migout2=$(mktemp -t mig-helper-stdout2.XXXXXXXXXX)
-	migout_fifo2=$(mktemp -u -t mig-helper-fifo-stdout2.XXXXXXXXXX)
-	qmp1=$(mktemp -u -t mig-helper-qmp1.XXXXXXXXXX)
-	qmp2=$(mktemp -u -t mig-helper-qmp2.XXXXXXXXXX)
-	fifo=$(mktemp -u -t mig-helper-fifo.XXXXXXXXXX)
+	dst_incoming=$(mktemp -u -t mig-helper-socket-incoming.XXXXXXXXXX)
+	src_out=$(mktemp -t mig-helper-stdout1.XXXXXXXXXX)
+	src_outfifo=$(mktemp -u -t mig-helper-fifo-stdout1.XXXXXXXXXX)
+	dst_out=$(mktemp -t mig-helper-stdout2.XXXXXXXXXX)
+	dst_outfifo=$(mktemp -u -t mig-helper-fifo-stdout2.XXXXXXXXXX)
+	src_qmp=$(mktemp -u -t mig-helper-qmp1.XXXXXXXXXX)
+	dst_qmp=$(mktemp -u -t mig-helper-qmp2.XXXXXXXXXX)
+	dst_infifo=$(mktemp -u -t mig-helper-fifo-stdin.XXXXXXXXXX)
 
 	# race here between file creation and trap
 	trap "trap - TERM ; kill 0 ; exit 2" INT TERM
-	trap "rm -f ${migout1} ${migout2} ${migout_fifo1} ${migout_fifo2} ${migsock} ${qmp1} ${qmp2} ${fifo}" RETURN EXIT
+	trap "rm -f ${src_out} ${dst_out} ${src_outfifo} ${dst_outfifo} ${dst_incoming} ${src_qmp} ${dst_qmp} ${dst_infifo}" RETURN EXIT
+
+	src_qmpout=/dev/null
+	dst_qmpout=/dev/null
 
-	qmpout1=/dev/null
-	qmpout2=/dev/null
 	migcmdline=$@
 
-	mkfifo ${migout_fifo1}
-	mkfifo ${migout_fifo2}
+	mkfifo ${src_outfifo}
+	mkfifo ${dst_outfifo}
 
 	eval "$migcmdline" \
-		-chardev socket,id=mon1,path=${qmp1},server=on,wait=off \
-		-mon chardev=mon1,mode=control > ${migout_fifo1} &
+		-chardev socket,id=mon,path=${src_qmp},server=on,wait=off \
+		-mon chardev=mon,mode=control > ${src_outfifo} &
 	live_pid=$!
-	cat ${migout_fifo1} | tee ${migout1} &
+	cat ${src_outfifo} | tee ${src_out} &
 
 	# The test must prompt the user to migrate, so wait for the "migrate"
 	# keyword
-	while ! grep -q -i "Now migrate the VM" < ${migout1} ; do
+	while ! grep -q -i "Now migrate the VM" < ${src_out} ; do
 		if ! ps -p ${live_pid} > /dev/null ; then
 			echo "ERROR: Test exit before migration point." >&2
 			echo > ${fifo}
-			qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
+			qmp ${src_qmp} '"quit"'> ${src_qmpout} 2>/dev/null
 			return 3
 		fi
 		sleep 0.1
@@ -174,7 +175,7 @@ run_migration ()
 
 	while ps -p ${live_pid} > /dev/null ; do
 		# Wait for EXIT or further migrations
-		if ! grep -q -i "Now migrate the VM" < ${migout1} ; then
+		if ! grep -q -i "Now migrate the VM" < ${src_out} ; then
 			sleep 0.1
 		else
 			do_migration || return $?
@@ -196,78 +197,79 @@ do_migration ()
 	# We have to use cat to open the named FIFO, because named FIFO's,
 	# unlike pipes, will block on open() until the other end is also
 	# opened, and that totally breaks QEMU...
-	mkfifo ${fifo}
+	mkfifo ${dst_infifo}
 	eval "$migcmdline" \
-		-chardev socket,id=mon2,path=${qmp2},server=on,wait=off \
-		-mon chardev=mon2,mode=control -incoming unix:${migsock} \
-		< <(cat ${fifo}) > ${migout_fifo2} &
+		-chardev socket,id=mon,path=${dst_qmp},server=on,wait=off \
+		-mon chardev=mon,mode=control -incoming unix:${dst_incoming} \
+		< <(cat ${dst_infifo}) > ${dst_outfifo} &
 	incoming_pid=$!
-	cat ${migout_fifo2} | tee ${migout2} &
+	cat ${dst_outfifo} | tee ${dst_out} &
 
 	# The test must prompt the user to migrate, so wait for the "migrate" keyword
-	while ! grep -q -i "Now migrate the VM" < ${migout1} ; do
+	while ! grep -q -i "Now migrate the VM" < ${src_out} ; do
 		if ! ps -p ${live_pid} > /dev/null ; then
 			echo "ERROR: Test exit before migration point." >&2
-			echo > ${fifo}
-			qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
-			qmp ${qmp2} '"quit"'> ${qmpout2} 2>/dev/null
+			echo > ${dst_infifo}
+			qmp ${src_qmp} '"quit"'> ${src_qmpout} 2>/dev/null
+			qmp ${dst_qmp} '"quit"'> ${dst_qmpout} 2>/dev/null
 			return 3
 		fi
 		sleep 0.1
 	done
 
 	# Wait until the destination has created the incoming socket
-	while ! [ -S ${migsock} ] ; do sleep 0.1 ; done
+	while ! [ -S ${dst_incoming} ] ; do sleep 0.1 ; done
 
-	qmp ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > ${qmpout1}
+	qmp ${src_qmp} '"migrate", "arguments": { "uri": "unix:'${dst_incoming}'" }' > ${src_qmpout}
 
 	# Wait for the migration to complete
-	migstatus=`qmp ${qmp1} '"query-migrate"' | grep return`
+	migstatus=`qmp ${src_qmp} '"query-migrate"' | grep return`
 	while ! grep -q '"completed"' <<<"$migstatus" ; do
 		sleep 0.1
-		if ! migstatus=`qmp ${qmp1} '"query-migrate"'`; then
+		if ! migstatus=`qmp ${src_qmp} '"query-migrate"'`; then
 			echo "ERROR: Querying migration state failed." >&2
-			echo > ${fifo}
-			qmp ${qmp2} '"quit"'> ${qmpout2} 2>/dev/null
+			echo > ${dst_infifo}
+			qmp ${dst_qmp} '"quit"'> ${dst_qmpout} 2>/dev/null
 			return 2
 		fi
 		migstatus=`grep return <<<"$migstatus"`
 		if grep -q '"failed"' <<<"$migstatus"; then
 			echo "ERROR: Migration failed." >&2
-			echo > ${fifo}
-			qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
-			qmp ${qmp2} '"quit"'> ${qmpout2} 2>/dev/null
+			echo > ${dst_infifo}
+			qmp ${src_qmp} '"quit"'> ${src_qmpout} 2>/dev/null
+			qmp ${dst_qmp} '"quit"'> ${dst_qmpout} 2>/dev/null
 			return 2
 		fi
 	done
 
-	qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
+	qmp ${src_qmp} '"quit"'> ${src_qmpout} 2>/dev/null
 
 	# keypress to dst so getchar completes and test continues
-	echo > ${fifo}
-	rm ${fifo}
+	echo > ${dst_infifo}
+	rm ${dst_infifo}
 
 	# Ensure the incoming socket is removed, ready for next destination
-	if [ -S ${migsock} ] ; then
+	if [ -S ${dst_incoming} ] ; then
 		echo "ERROR: Incoming migration socket not removed after migration." >& 2
-		qmp ${qmp2} '"quit"'> ${qmpout2} 2>/dev/null
+		qmp ${dst_qmp} '"quit"'> ${dst_qmpout} 2>/dev/null
 		return 2
 	fi
 
 	wait ${live_pid}
 	ret=$?
 
-	# Now flip the variables because dest becomes source
+	# Now flip the variables because destination machine becomes source
+	# for the next migration.
 	live_pid=${incoming_pid}
-	tmp=${migout1}
-	migout1=${migout2}
-	migout2=${tmp}
-	tmp=${migout_fifo1}
-	migout_fifo1=${migout_fifo2}
-	migout_fifo2=${tmp}
-	tmp=${qmp1}
-	qmp1=${qmp2}
-	qmp2=${tmp}
+	tmp=${src_out}
+	src_out=${dst_out}
+	dst_out=${tmp}
+	tmp=${src_outfifo}
+	src_outfifo=${dst_outfifo}
+	dst_outfifo=${tmp}
+	tmp=${src_qmp}
+	src_qmp=${dst_qmp}
+	dst_qmp=${tmp}
 
 	return $ret
 }
@@ -290,8 +292,8 @@ run_panic ()
 	trap "rm -f ${qmp}" RETURN EXIT
 
 	# start VM stopped so we don't miss any events
-	eval "$@" -chardev socket,id=mon1,path=${qmp},server=on,wait=off \
-		-mon chardev=mon1,mode=control -S &
+	eval "$@" -chardev socket,id=mon,path=${qmp},server=on,wait=off \
+		-mon chardev=mon,mode=control -S &
 
 	panic_event_count=$(qmp_events ${qmp} | jq -c 'select(.event == "GUEST_PANICKED")' | wc -l)
 	if [ "$panic_event_count" -lt 1 ]; then
-- 
2.42.0

[kvm-unit-tests PATCH v5 06/29] powerpc: Quiet QEMU TCG pseries capability warnings

[Nicholas Piggin]

Quieten this console spam.

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

diff --git a/powerpc/run b/powerpc/run
index b353169f..e469f1eb 100755
--- a/powerpc/run
+++ b/powerpc/run
@@ -21,6 +21,11 @@ fi
 
 M='-machine pseries'
 M+=",accel=$ACCEL$ACCEL_PROPS"
+
+if [[ "$ACCEL" == "tcg" ]] ; then
+	M+=",cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,cap-ccf-assist=off"
+fi
+
 command="$qemu -nodefaults $M -bios $FIRMWARE"
 command+=" -display none -serial stdio -kernel"
 command="$(migration_cmd) $(timeout_cmd) $command"
-- 
2.42.0

Re: [kvm-unit-tests PATCH v5 06/29] powerpc: Quiet QEMU TCG pseries capability warnings

[Thomas Huth]

On 16/12/2023 14.42, Nicholas Piggin wrote:
> Quieten this console spam.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   powerpc/run | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/powerpc/run b/powerpc/run
> index b353169f..e469f1eb 100755
> --- a/powerpc/run
> +++ b/powerpc/run
> @@ -21,6 +21,11 @@ fi
>   
>   M='-machine pseries'
>   M+=",accel=$ACCEL$ACCEL_PROPS"
> +
> +if [[ "$ACCEL" == "tcg" ]] ; then
> +	M+=",cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,cap-ccf-assist=off"
> +fi

This also bugs me when I run QEMU with TCG manually and IMHO it should be 
fixed in QEMU instead (e.g. by providing a different CPU model for TCG mode).
Anyway, for the time being:

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

[kvm-unit-tests PATCH v5 07/29] powerpc: Add a migration stress tester

[Nicholas Piggin]

This performs 1000 migrations a tight loop to flush out simple issues
in the multiple-migration code.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 powerpc/Makefile.common |  1 +
 powerpc/migrate.c       | 64 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)
 create mode 100644 powerpc/migrate.c

diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index f8f47490..a7af225b 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -5,6 +5,7 @@
 #
 
 tests-common = \
+	$(TEST_DIR)/migrate.elf \
 	$(TEST_DIR)/selftest.elf \
 	$(TEST_DIR)/spapr_hcall.elf \
 	$(TEST_DIR)/rtas.elf \
diff --git a/powerpc/migrate.c b/powerpc/migrate.c
new file mode 100644
index 00000000..a9f98c9f
--- /dev/null
+++ b/powerpc/migrate.c
@@ -0,0 +1,64 @@
+/*
+ * Stress Test Migration
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include <libcflat.h>
+#include <util.h>
+#include <migrate.h>
+#include <alloc.h>
+#include <asm/handlers.h>
+#include <devicetree.h>
+#include <asm/hcall.h>
+#include <asm/processor.h>
+#include <asm/barrier.h>
+
+static bool do_migrate = false;
+
+static void test_migration(int argc, char **argv)
+{
+	int i;
+
+	for (i = 0; i < 1000; i++) {
+		if (do_migrate)
+			migrate();
+	}
+}
+
+struct {
+	const char *name;
+	void (*func)(int argc, char **argv);
+} hctests[] = {
+	{ "migration", test_migration },
+	{ NULL, NULL }
+};
+
+int main(int argc, char **argv)
+{
+	bool all;
+	int i;
+
+	all = argc == 1 || !strcmp(argv[1], "all");
+
+	for (i = 1; i < argc; i++) {
+		if (!strcmp(argv[i], "-w")) {
+			do_migrate = true;
+			if (!all && argc == 2)
+				all = true;
+		}
+	}
+
+	report_prefix_push("migration");
+
+	for (i = 0; hctests[i].name != NULL; i++) {
+		if (all || strcmp(argv[1], hctests[i].name) == 0) {
+			report_prefix_push(hctests[i].name);
+			hctests[i].func(argc, argv);
+			report_prefix_pop();
+		}
+	}
+
+	report_prefix_pop();
+
+	return report_summary();
+}
-- 
2.42.0

Re: [kvm-unit-tests PATCH v5 07/29] powerpc: Add a migration stress tester

[Thomas Huth]

On 16/12/2023 14.42, Nicholas Piggin wrote:
> This performs 1000 migrations a tight loop to flush out simple issues
> in the multiple-migration code.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   powerpc/Makefile.common |  1 +
>   powerpc/migrate.c       | 64 +++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 65 insertions(+)
>   create mode 100644 powerpc/migrate.c

You should likely add an entry to powerpc/unittests.cfg ...
also, wouldn't it be better to extend the sprs.elf test for this, so that it 
e.g. changes some stuff for each migration?

  Thomas

Re: [kvm-unit-tests PATCH v5 07/29] powerpc: Add a migration stress tester

[Nicholas Piggin]

On Tue Dec 19, 2023 at 3:58 PM AEST, Thomas Huth wrote:
> On 16/12/2023 14.42, Nicholas Piggin wrote:
> > This performs 1000 migrations a tight loop to flush out simple issues
> > in the multiple-migration code.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   powerpc/Makefile.common |  1 +
> >   powerpc/migrate.c       | 64 +++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 65 insertions(+)
> >   create mode 100644 powerpc/migrate.c
>
> You should likely add an entry to powerpc/unittests.cfg ...
> also, wouldn't it be better to extend the sprs.elf test for this, so that it 
> e.g. changes some stuff for each migration?

It was supposed to be kind of a dumb stress tester for the harness
that just runs as quickly as possible for a long time :) I'll see if
it can be massaged into something more useful.

sprs (and others) probably should get better migration stressing
when multi-migration support lands in the harness, but I was
thinking that could be follow up patches.

Thanks,
Nick

[kvm-unit-tests PATCH v5 08/29] powerpc: Require KVM for the TM test

[Nicholas Piggin]

QEMU TCG does not support TM. Skip this test gracefully when not on KVM.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 powerpc/unittests.cfg | 1 +
 1 file changed, 1 insertion(+)

diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index dd5f361c..e71140aa 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -65,6 +65,7 @@ file = emulator.elf
 
 [h_cede_tm]
 file = tm.elf
+accel = kvm
 smp = 2,threads=2
 extra_params = -machine cap-htm=on -append "h_cede_tm"
 groups = h_cede_tm
-- 
2.42.0

Re: [kvm-unit-tests PATCH v5 08/29] powerpc: Require KVM for the TM test

[Thomas Huth]

On 16/12/2023 14.42, Nicholas Piggin wrote:
> QEMU TCG does not support TM. Skip this test gracefully when not on KVM.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   powerpc/unittests.cfg | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
> index dd5f361c..e71140aa 100644
> --- a/powerpc/unittests.cfg
> +++ b/powerpc/unittests.cfg
> @@ -65,6 +65,7 @@ file = emulator.elf
>   
>   [h_cede_tm]
>   file = tm.elf
> +accel = kvm
>   smp = 2,threads=2
>   extra_params = -machine cap-htm=on -append "h_cede_tm"
>   groups = h_cede_tm

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

[kvm-unit-tests PATCH v5 09/29] powerpc: Fix interrupt stack alignment

[Nicholas Piggin]

ppc64 requires the stack to be 16-byte aligned but the interrupt
stack frame has 8-byte aligned size. Add padding to fix.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 lib/powerpc/setup.c    | 3 +++
 lib/ppc64/asm/ptrace.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
index 1be4c030..d98f66fa 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/ptrace.h>
 #include <asm/hcall.h>
 #include "io.h"
 
@@ -195,6 +196,8 @@ void setup(const void *fdt)
 		freemem += initrd_size;
 	}
 
+	assert(STACK_INT_FRAME_SIZE % 16 == 0);
+
 	/* call init functions */
 	cpu_init();
 
diff --git a/lib/ppc64/asm/ptrace.h b/lib/ppc64/asm/ptrace.h
index 076c9d9c..12de7499 100644
--- a/lib/ppc64/asm/ptrace.h
+++ b/lib/ppc64/asm/ptrace.h
@@ -14,6 +14,7 @@ struct pt_regs {
 	unsigned long xer;
 	unsigned long ccr;
 	unsigned long trap;
+	unsigned long _pad; /* stack must be 16-byte aligned */
 };
 
 #define STACK_INT_FRAME_SIZE    (sizeof(struct pt_regs) + \
-- 
2.42.0

Re: [kvm-unit-tests PATCH v5 09/29] powerpc: Fix interrupt stack alignment

[Thomas Huth]

On 16/12/2023 14.42, Nicholas Piggin wrote:
> ppc64 requires the stack to be 16-byte aligned but the interrupt
> stack frame has 8-byte aligned size. Add padding to fix.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   lib/powerpc/setup.c    | 3 +++
>   lib/ppc64/asm/ptrace.h | 1 +
>   2 files changed, 4 insertions(+)

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

[kvm-unit-tests PATCH v5 10/29] 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.

Acked-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 powerpc/sprs.c | 643 ++++++++++++++++++++++++++++++++++---------------
 1 file changed, 450 insertions(+), 193 deletions(-)

diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index 57e487ce..cd8b472d 100644
--- a/powerpc/sprs.c
+++ b/powerpc/sprs.c
@@ -28,231 +28,465 @@
 #include <asm/processor.h>
 #include <asm/barrier.h>
 
-uint64_t before[1024], after[1024];
-
-/* Common SPRs for all PowerPC CPUs */
-static void set_sprs_common(uint64_t val)
+/* "Indirect" mfspr/mtspr which accept a non-constant spr number */
+static uint64_t __mfspr(unsigned spr)
 {
-	mtspr(9, val);		/* CTR */
-	// mtspr(273, val);	/* SPRG1 */  /* Used by our exception handler */
-	mtspr(274, val);	/* SPRG2 */
-	mtspr(275, val);	/* SPRG3 */
+	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;
 }
 
-/* SPRs from PowerPC Operating Environment Architecture, Book III, Vers. 2.01 */
-static void set_sprs_book3s_201(uint64_t val)
+static void __mtspr(unsigned spr, 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 */
+	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");
 }
 
+static uint64_t before[1024], after[1024];
+
+#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 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,		SPR_ASYNC, },
+[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,		SPR_INT, },
+};
 
 /* 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,		SPR_INT, },
+[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,		SPR_INT, },
+[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 POWER9, POWER10 User Manual */
+static const struct spr sprs_power9_10[1024] = {
+[276] = {"SPRC",	64,	HV_RW, },
+[277] = {"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_power9_10[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_power9_10[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_power9_10[i].name) {
+				assert(!sprs[i].name);
+				memcpy(&sprs[i], &sprs_power9_10[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 */
-	// 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);
+		}
 	}
 }
 
@@ -289,7 +523,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));
@@ -301,16 +537,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.42.0

Re: [kvm-unit-tests PATCH v5 10/29] powerpc/sprs: Specify SPRs with data rather than code

[Thomas Huth]

On 16/12/2023 14.42, 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.
> 
> Acked-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   powerpc/sprs.c | 643 ++++++++++++++++++++++++++++++++++---------------
>   1 file changed, 450 insertions(+), 193 deletions(-)
> 
> diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> index 57e487ce..cd8b472d 100644
> --- a/powerpc/sprs.c
> +++ b/powerpc/sprs.c
> @@ -28,231 +28,465 @@
>   #include <asm/processor.h>
>   #include <asm/barrier.h>
>   
> -uint64_t before[1024], after[1024];
> -
> -/* Common SPRs for all PowerPC CPUs */
> -static void set_sprs_common(uint64_t val)
> +/* "Indirect" mfspr/mtspr which accept a non-constant spr number */
> +static uint64_t __mfspr(unsigned spr)
>   {
> -	mtspr(9, val);		/* CTR */
> -	// mtspr(273, val);	/* SPRG1 */  /* Used by our exception handler */
> -	mtspr(274, val);	/* SPRG2 */
> -	mtspr(275, val);	/* SPRG3 */
> +	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;
>   }
>   
> -/* SPRs from PowerPC Operating Environment Architecture, Book III, Vers. 2.01 */
> -static void set_sprs_book3s_201(uint64_t val)
> +static void __mtspr(unsigned spr, 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 */
> +	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");
>   }
>   
> +static uint64_t before[1024], after[1024];
> +
> +#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, },

Just a little stylish nit: You've got a space before the closing "}", but no 
space after the opening "{". Looks a little bit weird to me. Maybe add a 
space after the "{", too?

  Thomas

Re: [kvm-unit-tests PATCH v5 10/29] powerpc/sprs: Specify SPRs with data rather than code

[Nicholas Piggin]

On Tue Dec 19, 2023 at 4:14 PM AEST, Thomas Huth wrote:
> On 16/12/2023 14.42, 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.
> > 
> > Acked-by: Thomas Huth <thuth@redhat.com>
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   powerpc/sprs.c | 643 ++++++++++++++++++++++++++++++++++---------------
> >   1 file changed, 450 insertions(+), 193 deletions(-)
> > 
> > diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> > index 57e487ce..cd8b472d 100644
> > --- a/powerpc/sprs.c
> > +++ b/powerpc/sprs.c
> > @@ -28,231 +28,465 @@
> >   #include <asm/processor.h>
> >   #include <asm/barrier.h>
> >   
> > -uint64_t before[1024], after[1024];
> > -
> > -/* Common SPRs for all PowerPC CPUs */
> > -static void set_sprs_common(uint64_t val)
> > +/* "Indirect" mfspr/mtspr which accept a non-constant spr number */
> > +static uint64_t __mfspr(unsigned spr)
> >   {
> > -	mtspr(9, val);		/* CTR */
> > -	// mtspr(273, val);	/* SPRG1 */  /* Used by our exception handler */
> > -	mtspr(274, val);	/* SPRG2 */
> > -	mtspr(275, val);	/* SPRG3 */
> > +	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;
> >   }
> >   
> > -/* SPRs from PowerPC Operating Environment Architecture, Book III, Vers. 2.01 */
> > -static void set_sprs_book3s_201(uint64_t val)
> > +static void __mtspr(unsigned spr, 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 */
> > +	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");
> >   }
> >   
> > +static uint64_t before[1024], after[1024];
> > +
> > +#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, },
>
> Just a little stylish nit: You've got a space before the closing "}", but no 
> space after the opening "{". Looks a little bit weird to me. Maybe add a 
> space after the "{", too?

Yes that is inconsistent. I'll fix.

Thanks,
Nick

[kvm-unit-tests PATCH v5 11/29] powerpc/sprs: Don't fail changed SPRs that are used by the test harness

[Nicholas Piggin]

SPRs annotated with SPR_HARNESS can change between consecutive reads
because the test harness code has changed them. Avoid failing the
test in this case.

[XER was observed to change after the next changeset to use mdelay.]

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

diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index cd8b472d..01041912 100644
--- a/powerpc/sprs.c
+++ b/powerpc/sprs.c
@@ -557,7 +557,7 @@ int main(int argc, char **argv)
 			if (before[i] >> 32)
 				pass = false;
 		}
-		if (!(sprs[i].type & SPR_ASYNC) && (before[i] != after[i]))
+		if (!(sprs[i].type & (SPR_HARNESS|SPR_ASYNC)) && (before[i] != after[i]))
 			pass = false;
 
 		if (sprs[i].width == 32 && !(before[i] >> 32) && !(after[i] >> 32))
-- 
2.42.0

Re: [kvm-unit-tests PATCH v5 11/29] powerpc/sprs: Don't fail changed SPRs that are used by the test harness

[Thomas Huth]

On 16/12/2023 14.42, Nicholas Piggin wrote:
> SPRs annotated with SPR_HARNESS can change between consecutive reads
> because the test harness code has changed them. Avoid failing the
> test in this case.
> 
> [XER was observed to change after the next changeset to use mdelay.]
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   powerpc/sprs.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> index cd8b472d..01041912 100644
> --- a/powerpc/sprs.c
> +++ b/powerpc/sprs.c
> @@ -557,7 +557,7 @@ int main(int argc, char **argv)
>   			if (before[i] >> 32)
>   				pass = false;
>   		}
> -		if (!(sprs[i].type & SPR_ASYNC) && (before[i] != after[i]))
> +		if (!(sprs[i].type & (SPR_HARNESS|SPR_ASYNC)) && (before[i] != after[i]))
>   			pass = false;

I guess you could even squash this into the previous patch.

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

[kvm-unit-tests PATCH v5 12/29] powerpc/sprs: Avoid taking async interrupts caused by register fuzzing

[Nicholas Piggin]

Storing certain values in some registers can cause asynchronous
interrupts that can crash the test case, for example decrementer
or PMU interrupts.

Change the msleep to mdelay which does not enable MSR[EE] and so
avoids the problem. This allows removing some of the SPR special
casing.

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

diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index 01041912..313698e0 100644
--- a/powerpc/sprs.c
+++ b/powerpc/sprs.c
@@ -481,12 +481,7 @@ static void set_sprs(uint64_t val)
 			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);
-		}
+		__mtspr(i, val);
 	}
 }
 
@@ -536,12 +531,7 @@ int main(int argc, char **argv)
 	if (pause) {
 		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;
+		mdelay(2000);
 	}
 
 	get_sprs(after);
-- 
2.42.0

Re: [kvm-unit-tests PATCH v5 12/29] powerpc/sprs: Avoid taking async interrupts caused by register fuzzing

[Thomas Huth]

On 16/12/2023 14.42, Nicholas Piggin wrote:
> Storing certain values in some registers can cause asynchronous
> interrupts that can crash the test case, for example decrementer
> or PMU interrupts.
> 
> Change the msleep to mdelay which does not enable MSR[EE] and so
> avoids the problem. This allows removing some of the SPR special
> casing.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   powerpc/sprs.c | 14 ++------------
>   1 file changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> index 01041912..313698e0 100644
> --- a/powerpc/sprs.c
> +++ b/powerpc/sprs.c
> @@ -481,12 +481,7 @@ static void set_sprs(uint64_t val)
>   			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);
> -		}
> +		__mtspr(i, val);
>   	}
>   }
>   
> @@ -536,12 +531,7 @@ int main(int argc, char **argv)
>   	if (pause) {
>   		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;
> +		mdelay(2000);
>   	}

IIRC I used the H_CEDE stuff here on purpose to increase the possibility 
that the guest gets rescheduled onto another CPU core on the host, and thus 
that it uncovers sprs that are not saved and restored on the host more 
easily. So I'd rather keep the msleep() here.

  Thomas

Re: [kvm-unit-tests PATCH v5 12/29] powerpc/sprs: Avoid taking async interrupts caused by register fuzzing

[Nicholas Piggin]

On Tue Dec 19, 2023 at 9:47 PM AEST, Thomas Huth wrote:
> On 16/12/2023 14.42, Nicholas Piggin wrote:
> > Storing certain values in some registers can cause asynchronous
> > interrupts that can crash the test case, for example decrementer
> > or PMU interrupts.
> > 
> > Change the msleep to mdelay which does not enable MSR[EE] and so
> > avoids the problem. This allows removing some of the SPR special
> > casing.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   powerpc/sprs.c | 14 ++------------
> >   1 file changed, 2 insertions(+), 12 deletions(-)
> > 
> > diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> > index 01041912..313698e0 100644
> > --- a/powerpc/sprs.c
> > +++ b/powerpc/sprs.c
> > @@ -481,12 +481,7 @@ static void set_sprs(uint64_t val)
> >   			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);
> > -		}
> > +		__mtspr(i, val);
> >   	}
> >   }
> >   
> > @@ -536,12 +531,7 @@ int main(int argc, char **argv)
> >   	if (pause) {
> >   		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;
> > +		mdelay(2000);
> >   	}
>
> IIRC I used the H_CEDE stuff here on purpose to increase the possibility 
> that the guest gets rescheduled onto another CPU core on the host, and thus 
> that it uncovers sprs that are not saved and restored on the host more 
> easily. So I'd rather keep the msleep() here.

Ah. Not a bad idea. I'll see about making it deal with H_CEDE
instead.

I wonder if there would be a way to stress host CPU migration
in the test harness. Instead of waiting for QEMU to exit, the
script could run taskset in a loop to bounce it around.
Conceptually easy, probably fiddly to wire things up nicely.

I may try to take a look at adding that... but after this
series so it doesn't get any bigger :/

Thanks,
Nick

[kvm-unit-tests PATCH v5 13/29] powerpc: Make interrupt handler error more readable

[Nicholas Piggin]

Installing the same handler twice reports a shifted trap vector
address which is hard to decipher. Print the unshifed address.

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

diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
index aaf45b68..b4cd5b4c 100644
--- a/lib/powerpc/processor.c
+++ b/lib/powerpc/processor.c
@@ -26,7 +26,7 @@ void handle_exception(int trap, void (*func)(struct pt_regs *, void *),
 	trap >>= 8;
 
 	if (func && handlers[trap].func) {
-		printf("exception handler installed twice %#x\n", trap);
+		printf("exception handler installed twice %#x\n", trap << 5);
 		abort();
 	}
 	handlers[trap].func = func;
-- 
2.42.0

Re: [kvm-unit-tests PATCH v5 13/29] powerpc: Make interrupt handler error more readable

[Thomas Huth]

On 16/12/2023 14.42, Nicholas Piggin wrote:
> Installing the same handler twice reports a shifted trap vector
> address which is hard to decipher. Print the unshifed address.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   lib/powerpc/processor.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
> index aaf45b68..b4cd5b4c 100644
> --- a/lib/powerpc/processor.c
> +++ b/lib/powerpc/processor.c
> @@ -26,7 +26,7 @@ void handle_exception(int trap, void (*func)(struct pt_regs *, void *),
>   	trap >>= 8;

You only change this to >>= 5 in the next patch...

>   	if (func && handlers[trap].func) {
> -		printf("exception handler installed twice %#x\n", trap);
> +		printf("exception handler installed twice %#x\n", trap << 5);

... so I think you should move this patch here after the next one.

  Thomas


>   		abort();
>   	}
>   	handlers[trap].func = func;

Re: [kvm-unit-tests PATCH v5 13/29] powerpc: Make interrupt handler error more readable

[Nicholas Piggin]

On Tue Dec 19, 2023 at 9:53 PM AEST, Thomas Huth wrote:
> On 16/12/2023 14.42, Nicholas Piggin wrote:
> > Installing the same handler twice reports a shifted trap vector
> > address which is hard to decipher. Print the unshifed address.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   lib/powerpc/processor.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
> > index aaf45b68..b4cd5b4c 100644
> > --- a/lib/powerpc/processor.c
> > +++ b/lib/powerpc/processor.c
> > @@ -26,7 +26,7 @@ void handle_exception(int trap, void (*func)(struct pt_regs *, void *),
> >   	trap >>= 8;
>
> You only change this to >>= 5 in the next patch...
>
> >   	if (func && handlers[trap].func) {
> > -		printf("exception handler installed twice %#x\n", trap);
> > +		printf("exception handler installed twice %#x\n", trap << 5);
>
> ... so I think you should move this patch here after the next one.

Paper bag for me.

Thanks,
Nick

[kvm-unit-tests PATCH v5 14/29] powerpc: Expand exception handler vector granularity

[Nicholas Piggin]

Exception handlers are currently indexed in units of 0x100, but
powerpc can have vectors that are aligned to as little as 0x20
bytes. Increase granularity of the handler functions before
adding support for those vectors.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 lib/powerpc/processor.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
index b4cd5b4c..b224fc8e 100644
--- a/lib/powerpc/processor.c
+++ b/lib/powerpc/processor.c
@@ -16,19 +16,25 @@
 static struct {
 	void (*func)(struct pt_regs *, void *data);
 	void *data;
-} handlers[16];
+} handlers[128];
 
+/*
+ * Exception handlers span from 0x100 to 0x1000 and can have a granularity
+ * of 0x20 bytes in some cases. Indexing spans 0-0x1000 with 0x20 increments
+ * resulting in 128 slots.
+ */
 void handle_exception(int trap, void (*func)(struct pt_regs *, void *),
 		      void * data)
 {
-	assert(!(trap & ~0xf00));
+	assert(!(trap & ~0xfe0));
 
-	trap >>= 8;
+	trap >>= 5;
 
 	if (func && handlers[trap].func) {
 		printf("exception handler installed twice %#x\n", trap << 5);
 		abort();
 	}
+
 	handlers[trap].func = func;
 	handlers[trap].data = data;
 }
@@ -37,9 +43,9 @@ void do_handle_exception(struct pt_regs *regs)
 {
 	unsigned char v;
 
-	v = regs->trap >> 8;
+	v = regs->trap >> 5;
 
-	if (v < 16 && handlers[v].func) {
+	if (v < 128 && handlers[v].func) {
 		handlers[v].func(regs, handlers[v].data);
 		return;
 	}
-- 
2.42.0

Re: [kvm-unit-tests PATCH v5 14/29] powerpc: Expand exception handler vector granularity

[Thomas Huth]

On 16/12/2023 14.42, Nicholas Piggin wrote:
> Exception handlers are currently indexed in units of 0x100, but
> powerpc can have vectors that are aligned to as little as 0x20
> bytes. Increase granularity of the handler functions before
> adding support for those vectors.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   lib/powerpc/processor.c | 16 +++++++++++-----
>   1 file changed, 11 insertions(+), 5 deletions(-)

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

[kvm-unit-tests PATCH v5 15/29] powerpc: Add support for more interrupts including HV interrupts

[Nicholas Piggin]

Interrupt vectors were not being populated for all architected
interrupt types, which could lead to crashes rather than a message for
unhandled interrupts.

0x20 sized vectors require some reworking of the code to fit. This
also adds support for HV / HSRR type interrupts which will be used in
a later change.

Acked-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 lib/powerpc/asm/ppc_asm.h |  3 ++
 powerpc/cstart64.S        | 79 ++++++++++++++++++++++++++++++++-------
 2 files changed, 68 insertions(+), 14 deletions(-)

diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
index 6299ff53..46b4be00 100644
--- a/lib/powerpc/asm/ppc_asm.h
+++ b/lib/powerpc/asm/ppc_asm.h
@@ -35,6 +35,9 @@
 
 #endif /* __BYTE_ORDER__ */
 
+#define SPR_HSRR0	0x13A
+#define SPR_HSRR1	0x13B
+
 /* Machine State Register definitions: */
 #define MSR_EE_BIT	15			/* External Interrupts Enable */
 #define MSR_SF_BIT	63			/* 64-bit mode */
diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
index 34e39341..b7514100 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S
@@ -184,14 +184,6 @@ call_handler:
 	mfcr	r0
 	std	r0,_CCR(r1)
 
-	/* nip and msr */
-
-	mfsrr0	r0
-	std	r0, _NIP(r1)
-
-	mfsrr1	r0
-	std	r0, _MSR(r1)
-
 	/* restore TOC pointer */
 
 	LOAD_REG_IMMEDIATE(r31, SPAPR_KERNEL_LOAD_ADDR)
@@ -238,6 +230,7 @@ call_handler:
 
 .section .text.ex
 
+/* [H]VECTOR must not be more than 8 instructions to fit in 0x20 vectors */
 .macro VECTOR vec
 	. = \vec
 
@@ -246,19 +239,28 @@ call_handler:
 	subi	r1,r1, INT_FRAME_SIZE
 
 	/* save r0 and ctr to call generic handler */
-
 	SAVE_GPR(0,r1)
 
-	mfctr	r0
-	std	r0,_CTR(r1)
+	li	r0,\vec
+	std	r0,_TRAP(r1)
 
-	ld	r0, P_HANDLER(0)
-	mtctr	r0
+	b	handler_trampoline
+.endm
+
+.macro HVECTOR vec
+	. = \vec
+
+	mtsprg1	r1	/* save r1 */
+	mfsprg0	r1	/* get exception stack address */
+	subi	r1,r1, INT_FRAME_SIZE
+
+	/* save r0 and ctr to call generic handler */
+	SAVE_GPR(0,r1)
 
 	li	r0,\vec
 	std	r0,_TRAP(r1)
 
-	bctr
+	b	handler_htrampoline
 .endm
 
 	. = 0x100
@@ -268,12 +270,61 @@ __start_interrupts:
 VECTOR(0x100)
 VECTOR(0x200)
 VECTOR(0x300)
+VECTOR(0x380)
 VECTOR(0x400)
+VECTOR(0x480)
 VECTOR(0x500)
 VECTOR(0x600)
 VECTOR(0x700)
 VECTOR(0x800)
 VECTOR(0x900)
+HVECTOR(0x980)
+VECTOR(0xa00)
+VECTOR(0xc00)
+VECTOR(0xd00)
+HVECTOR(0xe00)
+HVECTOR(0xe20)
+HVECTOR(0xe40)
+HVECTOR(0xe60)
+HVECTOR(0xe80)
+HVECTOR(0xea0)
+VECTOR(0xf00)
+VECTOR(0xf20)
+VECTOR(0xf40)
+VECTOR(0xf60)
+HVECTOR(0xf80)
+
+handler_trampoline:
+	mfctr	r0
+	std	r0,_CTR(r1)
+
+	ld	r0, P_HANDLER(0)
+	mtctr	r0
+
+	/* nip and msr */
+	mfsrr0	r0
+	std	r0, _NIP(r1)
+
+	mfsrr1	r0
+	std	r0, _MSR(r1)
+
+	bctr
+
+handler_htrampoline:
+	mfctr	r0
+	std	r0,_CTR(r1)
+
+	ld	r0, P_HANDLER(0)
+	mtctr	r0
+
+	/* nip and msr */
+	mfspr	r0, SPR_HSRR0
+	std	r0, _NIP(r1)
+
+	mfspr	r0, SPR_HSRR1
+	std	r0, _MSR(r1)
+
+	bctr
 
 	.align 7
 	.globl __end_interrupts
-- 
2.42.0

[kvm-unit-tests PATCH v5 16/29] powerpc: Set .got section alignment to 256 bytes

[Nicholas Piggin]

Modern powerpc64 toolchains require the .got section have alignment of
256 bytes. Incorrect alignment ends up causing the .data section ELF
load address to move by 8 bytes from its file offset, relative to
previous sections. This is not a problem for the QEMU bios loader used
by the pseries machine, but it is a problem for the powernv machine
using skiboot as the bios and the test programs as a kernel, because the
skiboot ELF loader is crippled:

  * Note that we execute the kernel in-place, we don't actually
  * obey the load informations in the headers. This is expected
  * to work for the Linux Kernel because it's a fairly dumb ELF
  * but it will not work for any ELF binary.

This causes all references to data to be incorrect. Aligning the .got
to 256 bytes prevents this offset skew and allows the skiboot "flat"
loader to work. [I don't know why the .got alignment can cause this
difference in linking.]

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 powerpc/flat.lds | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/powerpc/flat.lds b/powerpc/flat.lds
index 5eed368d..e07b91c1 100644
--- a/powerpc/flat.lds
+++ b/powerpc/flat.lds
@@ -41,8 +41,7 @@ SECTIONS
     /*
      * tocptr is tocbase + 32K, allowing toc offsets to be +-32K
      */
-    tocptr = . + 32K;
-    .got : { *(.toc) *(.got) }
+    .got : ALIGN(256) { tocptr = . + 32K; *(.toc .got) }
     . = ALIGN(64K);
     edata = .;
     . += 64K;
-- 
2.42.0

Re: [kvm-unit-tests PATCH v5 16/29] powerpc: Set .got section alignment to 256 bytes

[Thomas Huth]

On 16/12/2023 14.42, Nicholas Piggin wrote:
> Modern powerpc64 toolchains require the .got section have alignment of
> 256 bytes. Incorrect alignment ends up causing the .data section ELF
> load address to move by 8 bytes from its file offset, relative to
> previous sections. This is not a problem for the QEMU bios loader used
> by the pseries machine, but it is a problem for the powernv machine
> using skiboot as the bios and the test programs as a kernel, because the
> skiboot ELF loader is crippled:
> 
>    * Note that we execute the kernel in-place, we don't actually
>    * obey the load informations in the headers. This is expected
>    * to work for the Linux Kernel because it's a fairly dumb ELF
>    * but it will not work for any ELF binary.
> 
> This causes all references to data to be incorrect. Aligning the .got
> to 256 bytes prevents this offset skew and allows the skiboot "flat"
> loader to work. [I don't know why the .got alignment can cause this
> difference in linking.]
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   powerpc/flat.lds | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/powerpc/flat.lds b/powerpc/flat.lds
> index 5eed368d..e07b91c1 100644
> --- a/powerpc/flat.lds
> +++ b/powerpc/flat.lds
> @@ -41,8 +41,7 @@ SECTIONS
>       /*
>        * tocptr is tocbase + 32K, allowing toc offsets to be +-32K
>        */
> -    tocptr = . + 32K;
> -    .got : { *(.toc) *(.got) }
> +    .got : ALIGN(256) { tocptr = . + 32K; *(.toc .got) }
>       . = ALIGN(64K);
>       edata = .;
>       . += 64K;

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

[kvm-unit-tests PATCH v5 17/29] 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.

Acked-by: Thomas Huth <thuth@redhat.com>
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 b7514100..e18ae9a2 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 addresses 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
@@ -185,9 +193,10 @@ call_handler:
 	std	r0,_CCR(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.42.0

[kvm-unit-tests PATCH v5 18/29] powerpc: Fix stack backtrace termination

[Nicholas Piggin]

The backtrace handler terminates when it sees a NULL caller address,
but the powerpc stack setup does not keep such a NULL caller frame
at the start of the stack.

This happens to work on pseries because the memory at 0 is mapped and
it contains 0 at the location of the return address pointer if it
were a stack frame. But this is fragile, and does not work with powernv
where address 0 contains firmware instructions.

Use the existing dummy frame on stack as the NULL caller, and create a
new frame on stack for the entry code.

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

diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
index e18ae9a2..14ab0c6c 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S
@@ -46,8 +46,16 @@ start:
 	add	r1, r1, r31
 	add	r2, r2, r31
 
+	/* Zero backpointers in initial stack frame so backtrace() stops */
+	li	r0,0
+	std	r0,0(r1)
+	std	r0,16(r1)
+
+	/* Create entry frame */
+	stdu	r1,-INT_FRAME_SIZE(r1)
+
 	/* save DTB pointer */
-	std	r3, 56(r1)
+	SAVE_GPR(3,r1)
 
 	/*
 	 * Call relocate. relocate is C code, but careful to not use
@@ -101,7 +109,7 @@ start:
 	stw	r4, 0(r3)
 
 	/* complete setup */
-1:	ld	r3, 56(r1)
+1:	REST_GPR(3, r1)
 	bl	setup
 
 	/* run the test */
-- 
2.42.0

Re: [kvm-unit-tests PATCH v5 18/29] powerpc: Fix stack backtrace termination

[Thomas Huth]

On 16/12/2023 14.42, Nicholas Piggin wrote:
> The backtrace handler terminates when it sees a NULL caller address,
> but the powerpc stack setup does not keep such a NULL caller frame
> at the start of the stack.
> 
> This happens to work on pseries because the memory at 0 is mapped and
> it contains 0 at the location of the return address pointer if it
> were a stack frame. But this is fragile, and does not work with powernv
> where address 0 contains firmware instructions.
> 
> Use the existing dummy frame on stack as the NULL caller, and create a
> new frame on stack for the entry code.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   powerpc/cstart64.S | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
> index e18ae9a2..14ab0c6c 100644
> --- a/powerpc/cstart64.S
> +++ b/powerpc/cstart64.S
> @@ -46,8 +46,16 @@ start:
>   	add	r1, r1, r31
>   	add	r2, r2, r31
>   
> +	/* Zero backpointers in initial stack frame so backtrace() stops */
> +	li	r0,0
> +	std	r0,0(r1)
> +	std	r0,16(r1)
> +
> +	/* Create entry frame */
> +	stdu	r1,-INT_FRAME_SIZE(r1)

Shouldn't that rather be STACK_FRAME_OVERHEAD instead of INT_FRAME_SIZE...

>   	/* save DTB pointer */
> -	std	r3, 56(r1)
> +	SAVE_GPR(3,r1)

... since SAVE_GPR uses STACK_FRAME_OVERHEAD (via GPR0), too?

  Thomas

Re: [kvm-unit-tests PATCH v5 18/29] powerpc: Fix stack backtrace termination

[Nicholas Piggin]

On Tue Dec 19, 2023 at 10:22 PM AEST, Thomas Huth wrote:
> On 16/12/2023 14.42, Nicholas Piggin wrote:
> > The backtrace handler terminates when it sees a NULL caller address,
> > but the powerpc stack setup does not keep such a NULL caller frame
> > at the start of the stack.
> > 
> > This happens to work on pseries because the memory at 0 is mapped and
> > it contains 0 at the location of the return address pointer if it
> > were a stack frame. But this is fragile, and does not work with powernv
> > where address 0 contains firmware instructions.
> > 
> > Use the existing dummy frame on stack as the NULL caller, and create a
> > new frame on stack for the entry code.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   powerpc/cstart64.S | 12 ++++++++++--
> >   1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
> > index e18ae9a2..14ab0c6c 100644
> > --- a/powerpc/cstart64.S
> > +++ b/powerpc/cstart64.S
> > @@ -46,8 +46,16 @@ start:
> >   	add	r1, r1, r31
> >   	add	r2, r2, r31
> >   
> > +	/* Zero backpointers in initial stack frame so backtrace() stops */
> > +	li	r0,0
> > +	std	r0,0(r1)
> > +	std	r0,16(r1)
> > +
> > +	/* Create entry frame */
> > +	stdu	r1,-INT_FRAME_SIZE(r1)
>
> Shouldn't that rather be STACK_FRAME_OVERHEAD instead of INT_FRAME_SIZE...
>
> >   	/* save DTB pointer */
> > -	std	r3, 56(r1)
> > +	SAVE_GPR(3,r1)
>
> ... since SAVE_GPR uses STACK_FRAME_OVERHEAD (via GPR0), too?

No I think it's correct. INT_FRAME_SIZE has STACK_FRAME_OVERHEAD and
struct pt_regs. The STACK_FRAME_OVERHEAD in GPR offsets is just to skip
that and get to pt_regs.gpr[].

Thanks,
Nick

[kvm-unit-tests PATCH v5 19/29] scripts: allow machine option to be specified in unittests.cfg

[Nicholas Piggin]

This allows different machines with different requirements to be
supported by run_tests.sh, similarly to how different accelerators
are handled.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 scripts/common.bash  |  8 ++++++--
 scripts/runtime.bash | 16 ++++++++++++----
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/scripts/common.bash b/scripts/common.bash
index b9413d68..ee1dd865 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -10,6 +10,7 @@ function for_each_unittest()
 	local opts
 	local groups
 	local arch
+	local machine
 	local check
 	local accel
 	local timeout
@@ -21,7 +22,7 @@ function for_each_unittest()
 		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
 			rematch=${BASH_REMATCH[1]}
 			if [ -n "${testname}" ]; then
-				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
 			fi
 			testname=$rematch
 			smp=1
@@ -29,6 +30,7 @@ function for_each_unittest()
 			opts=""
 			groups=""
 			arch=""
+			machine=""
 			check=""
 			accel=""
 			timeout=""
@@ -58,6 +60,8 @@ function for_each_unittest()
 			groups=${BASH_REMATCH[1]}
 		elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
 			arch=${BASH_REMATCH[1]}
+		elif [[ $line =~ ^machine\ *=\ *(.*)$ ]]; then
+			machine=${BASH_REMATCH[1]}
 		elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
 			check=${BASH_REMATCH[1]}
 		elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
@@ -67,7 +71,7 @@ function for_each_unittest()
 		fi
 	done
 	if [ -n "${testname}" ]; then
-		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
 	fi
 	exec {fd}<&-
 }
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index fc156f2f..0803d02a 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -30,7 +30,7 @@ premature_failure()
 get_cmdline()
 {
     local kernel=$1
-    echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
+    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
 }
 
 skip_nodefault()
@@ -78,9 +78,10 @@ function run()
     local kernel="$4"
     local opts="$5"
     local arch="$6"
-    local check="${CHECK:-$7}"
-    local accel="$8"
-    local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default
+    local machine="$7"
+    local check="${CHECK:-$8}"
+    local accel="$9"
+    local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
 
     if [ "${CONFIG_EFI}" == "y" ]; then
         kernel=${kernel/%.flat/.efi}
@@ -114,6 +115,13 @@ function run()
         return 2
     fi
 
+    if [ -n "$machine" ] && [ -n "$MACHINE" ] && [ "$machine" != "$MACHINE" ]; then
+        print_result "SKIP" $testname "" "$machine only"
+        return 2
+    elif [ -n "$MACHINE" ]; then
+        machine="$MACHINE"
+    fi
+
     if [ -n "$accel" ] && [ -n "$ACCEL" ] && [ "$accel" != "$ACCEL" ]; then
         print_result "SKIP" $testname "" "$accel only, but ACCEL=$ACCEL"
         return 2
-- 
2.42.0

Re: [kvm-unit-tests PATCH v5 19/29] scripts: allow machine option to be specified in unittests.cfg

[Thomas Huth]

On 16/12/2023 14.42, Nicholas Piggin wrote:
> This allows different machines with different requirements to be
> supported by run_tests.sh, similarly to how different accelerators
> are handled.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   scripts/common.bash  |  8 ++++++--
>   scripts/runtime.bash | 16 ++++++++++++----
>   2 files changed, 18 insertions(+), 6 deletions(-)

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

[kvm-unit-tests PATCH v5 20/29] scripts: Accommodate powerpc powernv machine differences

[Nicholas Piggin]

The QEMU powerpc powernv machine has minor differences that must be
accommodated for in output parsing:

- Summary parsing must search more lines of output for the summary
  line, to accommodate OPAL message on shutdown.
- Premature failure testing must tolerate case differences in kernel
  load error message.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 scripts/runtime.bash | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 0803d02a..96756a0d 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -9,7 +9,7 @@ FAIL() { echo -ne "\e[31mFAIL\e[0m"; }
 extract_summary()
 {
     local cr=$'\r'
-    tail -3 | grep '^SUMMARY: ' | sed 's/^SUMMARY: /(/;s/'"$cr"'\{0,1\}$/)/'
+    tail -5 | grep '^SUMMARY: ' | sed 's/^SUMMARY: /(/;s/'"$cr"'\{0,1\}$/)/'
 }
 
 # We assume that QEMU is going to work if it tried to load the kernel
@@ -18,7 +18,7 @@ premature_failure()
     local log="$(eval "$(get_cmdline _NO_FILE_4Uhere_)" 2>&1)"
 
     echo "$log" | grep "_NO_FILE_4Uhere_" |
-        grep -q -e "could not \(load\|open\) kernel" -e "error loading" &&
+        grep -q -e "[Cc]ould not \(load\|open\) kernel" -e "error loading" &&
         return 1
 
     RUNTIME_log_stderr <<< "$log"
-- 
2.42.0

Re: [kvm-unit-tests PATCH v5 20/29] scripts: Accommodate powerpc powernv machine differences

[Thomas Huth]

On 16/12/2023 14.42, Nicholas Piggin wrote:
> The QEMU powerpc powernv machine has minor differences that must be
> accommodated for in output parsing:
> 
> - Summary parsing must search more lines of output for the summary
>    line, to accommodate OPAL message on shutdown.
> - Premature failure testing must tolerate case differences in kernel
>    load error message.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   scripts/runtime.bash | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

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

[kvm-unit-tests PATCH v5 21/29] powerpc: Support powernv machine with QEMU TCG

[Nicholas Piggin]

Add support for QEMU's powernv machine. This uses standard firmware
(skiboot) rather than a minimal firmware shim.

Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 lib/powerpc/asm/ppc_asm.h   |  6 +++
 lib/powerpc/asm/processor.h | 13 +++++++
 lib/powerpc/hcall.c         |  4 +-
 lib/powerpc/io.c            | 27 ++++++++++++-
 lib/powerpc/io.h            |  6 +++
 lib/powerpc/processor.c     | 37 ++++++++++++++++++
 lib/powerpc/setup.c         | 10 +++--
 lib/ppc64/asm/opal.h        | 15 ++++++++
 lib/ppc64/opal-calls.S      | 46 ++++++++++++++++++++++
 lib/ppc64/opal.c            | 76 +++++++++++++++++++++++++++++++++++++
 powerpc/Makefile.ppc64      |  2 +
 powerpc/cstart64.S          |  7 ++++
 powerpc/run                 | 38 ++++++++++++++++---
 powerpc/unittests.cfg       |  8 ++++
 14 files changed, 282 insertions(+), 13 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 46b4be00..ef2d91dd 100644
--- a/lib/powerpc/asm/ppc_asm.h
+++ b/lib/powerpc/asm/ppc_asm.h
@@ -39,7 +39,13 @@
 #define SPR_HSRR1	0x13B
 
 /* 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 MSR_ME		0x1000ULL
+
+#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 4ad6612b..6368a9ee 100644
--- a/lib/powerpc/asm/processor.h
+++ b/lib/powerpc/asm/processor.h
@@ -3,6 +3,7 @@
 
 #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 *);
@@ -43,6 +44,15 @@ static inline void mtmsr(uint64_t msr)
 	asm volatile ("mtmsrd %[msr]" :: [msr] "r" (msr) : "memory");
 }
 
+/*
+ * 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)
+{
+	return !!(mfmsr() & (1ULL << MSR_HV_BIT));
+}
+
 static inline uint64_t get_tb(void)
 {
 	return mfspr(SPR_TB);
@@ -64,4 +74,7 @@ static inline void msleep(uint64_t ms)
 	usleep(ms * 1000);
 }
 
+void enable_mcheck(void);
+void disable_mcheck(void);
+
 #endif /* _ASMPOWERPC_PROCESSOR_H_ */
diff --git a/lib/powerpc/hcall.c b/lib/powerpc/hcall.c
index 711cb1b0..37e52f54 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 a381688b..ab7bb843 100644
--- a/lib/powerpc/io.c
+++ b/lib/powerpc/io.c
@@ -9,13 +9,33 @@
 #include <asm/spinlock.h>
 #include <asm/rtas.h>
 #include <asm/setup.h>
+#include <asm/processor.h>
 #include "io.h"
 
 static struct spinlock print_lock;
 
+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())
+		assert(!opal_init());
+	else
+		rtas_init();
 }
 
 void puts(const char *s)
@@ -38,7 +58,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 d4f21ba1..943bf142 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 int 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 b224fc8e..ef184d8d 100644
--- a/lib/powerpc/processor.c
+++ b/lib/powerpc/processor.c
@@ -71,6 +71,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;
 
@@ -107,3 +117,30 @@ void usleep(uint64_t us)
 {
 	sleep_tb((us * tb_hz) / 1000000);
 }
+
+static void rfid_msr(uint64_t msr)
+{
+	uint64_t tmp;
+
+	asm volatile(
+		"mtsrr1	%1		\n\
+		 bl	0f		\n\
+		 0:			\n\
+		 mflr	%0		\n\
+		 addi	%0,%0,1f-0b	\n\
+		 mtsrr0	%0		\n\
+		 rfid			\n\
+		 1:			\n"
+		: "=r"(tmp) : "r"(msr) : "lr");
+}
+
+void enable_mcheck(void)
+{
+	/* This is a no-op on pseries */
+	rfid_msr(mfmsr() | MSR_ME);
+}
+
+void disable_mcheck(void)
+{
+	rfid_msr(mfmsr() & ~MSR_ME);
+}
diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
index d98f66fa..2b7e27b0 100644
--- a/lib/powerpc/setup.c
+++ b/lib/powerpc/setup.c
@@ -19,6 +19,7 @@
 #include <asm/setup.h>
 #include <asm/page.h>
 #include <asm/ptrace.h>
+#include <asm/processor.h>
 #include <asm/hcall.h>
 #include "io.h"
 
@@ -98,12 +99,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)
@@ -159,6 +161,8 @@ void setup(const void *fdt)
 	u32 fdt_size;
 	int ret;
 
+	enable_mcheck();
+
 	/*
 	 * Before calling mem_init we need to move the fdt and initrd
 	 * to safe locations. We move them to construct the memory
diff --git a/lib/ppc64/asm/opal.h b/lib/ppc64/asm/opal.h
new file mode 100644
index 00000000..7b1299f7
--- /dev/null
+++ b/lib/ppc64/asm/opal.h
@@ -0,0 +1,15 @@
+/* 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
+#define OPAL_POLL_EVENTS			10
+#define OPAL_REINIT_CPUS			70
+# define OPAL_REINIT_CPUS_HILE_BE		(1 << 0)
+# define OPAL_REINIT_CPUS_HILE_LE		(1 << 1)
+
+#endif
diff --git a/lib/ppc64/opal-calls.S b/lib/ppc64/opal-calls.S
new file mode 100644
index 00000000..1833358c
--- /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 00000000..63fe42ae
--- /dev/null
+++ b/lib/ppc64/opal.c
@@ -0,0 +1,76 @@
+/* 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>
+#include <asm/io.h>
+#include "../powerpc/io.h"
+
+struct opal {
+	uint64_t base;
+	uint64_t entry;
+} opal;
+
+extern int64_t opal_call(int64_t token, int64_t arg1, int64_t arg2, int64_t arg3);
+
+int opal_init(void)
+{
+	const struct fdt_property *prop;
+	int node, len;
+
+	node = fdt_path_offset(dt_fdt(), "/ibm,opal");
+	if (node < 0)
+		return -1;
+
+	prop = fdt_get_property(dt_fdt(), node, "opal-base-address", &len);
+	if (!prop)
+		return -1;
+	opal.base = fdt64_to_cpu(*(uint64_t *)prop->data);
+
+	prop = fdt_get_property(dt_fdt(), node, "opal-entry-address", &len);
+	if (!prop)
+		return -1;
+	opal.entry = fdt64_to_cpu(*(uint64_t *)prop->data);
+
+#if  __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+	if (opal_call(OPAL_REINIT_CPUS, OPAL_REINIT_CPUS_HILE_LE, 0, 0) != OPAL_SUCCESS)
+		return -1;
+#endif
+
+	return 0;
+}
+
+extern void opal_power_off(void)
+{
+	opal_call(OPAL_CEC_POWER_DOWN, 0, 0, 0);
+	while (true)
+		opal_call(OPAL_POLL_EVENTS, 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;
+	if (nr_chars == 0)
+		return -1;
+
+	return ch;
+}
diff --git a/powerpc/Makefile.ppc64 b/powerpc/Makefile.ppc64
index b0ed2b10..06a7cf67 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 14ab0c6c..51d632da 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S
@@ -100,6 +100,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 e469f1eb..6fe6d31b 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"
@@ -11,22 +20,39 @@ fi
 
 set_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$ACCEL_PROPS"
+B=""
+D=""
+
+if [[ "$MACHINE" == "pseries"* ]] ; then
+	if [[ "$ACCEL" == "tcg" ]] ; then
+		M+=",cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,cap-ccf-assist=off"
+	fi
+	B+="-bios $FIRMWARE"
+fi
 
-if [[ "$ACCEL" == "tcg" ]] ; then
-	M+=",cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,cap-ccf-assist=off"
+if [[ "$MACHINE" == "powernv"* ]] ; then
+	D+="-device ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10"
 fi
 
-command="$qemu -nodefaults $M -bios $FIRMWARE"
+command="$qemu -nodefaults $M $B $D"
 command+=" -display none -serial stdio -kernel"
 command="$(migration_cmd) $(timeout_cmd) $command"
 
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index e71140aa..5d6ba852 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -32,30 +32,36 @@
 #
 [selftest-setup]
 file = selftest.elf
+machine = pseries
 smp = 2
 extra_params = -m 256 -append 'setup smp=2 mem=256'
 groups = selftest
 
 [spapr_hcall]
 file = spapr_hcall.elf
+machine = pseries
 
 [spapr_vpa]
 file = spapr_vpa.elf
+machine = pseries
 
 [rtas-get-time-of-day]
 file = rtas.elf
+machine = pseries
 timeout = 5
 extra_params = -append "get-time-of-day date=$(date +%s)"
 groups = rtas
 
 [rtas-get-time-of-day-base]
 file = rtas.elf
+machine = pseries
 timeout = 5
 extra_params = -rtc base="2006-06-17" -append "get-time-of-day date=$(date --date="2006-06-17 UTC" +%s)"
 groups = rtas
 
 [rtas-set-time-of-day]
 file = rtas.elf
+machine = pseries
 extra_params = -append "set-time-of-day"
 timeout = 5
 groups = rtas
@@ -65,6 +71,7 @@ file = emulator.elf
 
 [h_cede_tm]
 file = tm.elf
+machine = pseries
 accel = kvm
 smp = 2,threads=2
 extra_params = -machine cap-htm=on -append "h_cede_tm"
@@ -75,5 +82,6 @@ file = sprs.elf
 
 [sprs-migration]
 file = sprs.elf
+machine = pseries
 extra_params = -append '-w'
 groups = migration
-- 
2.42.0

[kvm-unit-tests PATCH v5 22/29] powerpc: Fix emulator illegal instruction test for powernv

[Nicholas Piggin]

Illegal instructions cause 0xe40 (HEAI) interrupts rather
than program interrupts.

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

diff --git a/powerpc/emulator.c b/powerpc/emulator.c
index 39dd5964..035a903c 100644
--- a/powerpc/emulator.c
+++ b/powerpc/emulator.c
@@ -31,6 +31,20 @@ static void program_check_handler(struct pt_regs *regs, void *opaque)
 	regs->nip += 4;
 }
 
+static void heai_handler(struct pt_regs *regs, void *opaque)
+{
+	int *data = opaque;
+
+	if (verbose) {
+		printf("Detected invalid instruction %#018lx: %08x\n",
+		       regs->nip, *(uint32_t*)regs->nip);
+	}
+
+	*data = 8; /* Illegal instruction */
+
+	regs->nip += 4;
+}
+
 static void alignment_handler(struct pt_regs *regs, void *opaque)
 {
 	int *data = opaque;
@@ -362,7 +376,10 @@ int main(int argc, char **argv)
 {
 	int i;
 
-	handle_exception(0x700, program_check_handler, (void *)&is_invalid);
+	if (machine_is_powernv())
+		handle_exception(0xe40, heai_handler, (void *)&is_invalid);
+	else
+		handle_exception(0x700, program_check_handler, (void *)&is_invalid);
 	handle_exception(0x600, alignment_handler, (void *)&alignment);
 
 	for (i = 1; i < argc; i++) {
-- 
2.42.0

Re: [kvm-unit-tests PATCH v5 22/29] powerpc: Fix emulator illegal instruction test for powernv

[Thomas Huth]

On 16/12/2023 14.42, Nicholas Piggin wrote:
> Illegal instructions cause 0xe40 (HEAI) interrupts rather
> than program interrupts.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   powerpc/emulator.c | 19 ++++++++++++++++++-
>   1 file changed, 18 insertions(+), 1 deletion(-)

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

[kvm-unit-tests PATCH v5 23/29] powerpc/sprs: Test hypervisor registers on powernv machine

[Nicholas Piggin]

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

Acked-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 powerpc/sprs.c | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index 313698e0..daaae3bc 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, },
@@ -306,7 +306,7 @@ static const struct spr sprs_power9_10[1024] = {
 [921] = {"TSCR",	32,	HV_RW, },
 [922] = {"TTR",		64,	HV_RW, },
 [1006]= {"TRACE",	64,	WO, },
-[1008]= {"HID",		64,	HV_RW, },
+[1008]= {"HID",		64,	HV_RW,		SPR_HARNESS, }, /* At least HILE would be unhelpful to change */
 };
 
 /* This covers POWER8 and POWER9 PMUs */
@@ -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 */
@@ -466,7 +482,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);
 	}
@@ -477,8 +493,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;
 		__mtspr(i, val);
@@ -540,7 +557,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.42.0

[kvm-unit-tests PATCH v5 24/29] powerpc: interrupt tests

[Nicholas Piggin]

Add basic testing of various kinds of interrupts, machine check,
page fault, illegal, decrementer, trace, syscall, etc.

This has a known failure on QEMU TCG pseries machines where MSR[ME]
can be incorrectly set to 0.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 lib/powerpc/asm/ppc_asm.h |  21 +-
 powerpc/Makefile.common   |   3 +-
 powerpc/interrupts.c      | 422 ++++++++++++++++++++++++++++++++++++++
 powerpc/unittests.cfg     |   3 +
 4 files changed, 445 insertions(+), 4 deletions(-)
 create mode 100644 powerpc/interrupts.c

diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
index ef2d91dd..778e78ee 100644
--- a/lib/powerpc/asm/ppc_asm.h
+++ b/lib/powerpc/asm/ppc_asm.h
@@ -35,17 +35,32 @@
 
 #endif /* __BYTE_ORDER__ */
 
+#define SPR_DSISR	0x012
+#define SPR_DAR		0x013
+#define SPR_DEC		0x016
+#define SPR_SRR0	0x01A
+#define SPR_SRR1	0x01B
+#define SPR_FSCR	0x099
+#define   FSCR_PREFIX	0x2000
+#define SPR_HDEC	0x136
 #define SPR_HSRR0	0x13A
 #define SPR_HSRR1	0x13B
+#define SPR_LPCR	0x13E
+#define   LPCR_HDICE	0x1UL
+#define SPR_HEIR	0x153
+#define SPR_SIAR	0x31C
 
 /* 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 MSR_ME		0x1000ULL
 
-#define SPR_HSRR0	0x13A
-#define SPR_HSRR1	0x13B
+#define MSR_DR		0x0010ULL
+#define MSR_IR		0x0020ULL
+#define MSR_BE		0x0200ULL		/* Branch Trace Enable */
+#define MSR_SE		0x0400ULL		/* Single Step Enable */
+#define MSR_EE		0x8000ULL
+#define MSR_ME		0x1000ULL
 
 #endif /* _ASMPOWERPC_PPC_ASM_H */
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index a7af225b..b340a53b 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -11,7 +11,8 @@ tests-common = \
 	$(TEST_DIR)/rtas.elf \
 	$(TEST_DIR)/emulator.elf \
 	$(TEST_DIR)/tm.elf \
-	$(TEST_DIR)/sprs.elf
+	$(TEST_DIR)/sprs.elf \
+	$(TEST_DIR)/interrupts.elf
 
 tests-all = $(tests-common) $(tests)
 all: directories $(TEST_DIR)/boot_rom.bin $(tests-all)
diff --git a/powerpc/interrupts.c b/powerpc/interrupts.c
new file mode 100644
index 00000000..3217b15e
--- /dev/null
+++ b/powerpc/interrupts.c
@@ -0,0 +1,422 @@
+/*
+ * Test interrupts
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include <libcflat.h>
+#include <util.h>
+#include <migrate.h>
+#include <alloc.h>
+#include <asm/handlers.h>
+#include <asm/hcall.h>
+#include <asm/processor.h>
+#include <asm/barrier.h>
+
+#define SPR_LPCR	0x13E
+#define LPCR_HDICE	0x1UL
+#define SPR_DEC		0x016
+#define SPR_HDEC	0x136
+
+#define MSR_DR		0x0010ULL
+#define MSR_IR		0x0020ULL
+#define MSR_EE		0x8000ULL
+#define MSR_ME		0x1000ULL
+
+static bool cpu_has_heir(void)
+{
+	uint32_t pvr = mfspr(287);	/* Processor Version Register */
+
+	if (!machine_is_powernv())
+		return false;
+
+	/* POWER6 has HEIR, but QEMU powernv support does not go that far */
+	switch (pvr >> 16) {
+	case 0x4b:			/* POWER8E */
+	case 0x4c:			/* POWER8NVL */
+	case 0x4d:			/* POWER8 */
+	case 0x4e:			/* POWER9 */
+	case 0x80:			/* POWER10 */
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool cpu_has_prefix(void)
+{
+	uint32_t pvr = mfspr(287);	/* Processor Version Register */
+	switch (pvr >> 16) {
+	case 0x80:			/* POWER10 */
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool cpu_has_lev_in_srr1(void)
+{
+	uint32_t pvr = mfspr(287);	/* Processor Version Register */
+	switch (pvr >> 16) {
+	case 0x80:			/* POWER10 */
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool regs_is_prefix(volatile struct pt_regs *regs)
+{
+	return (regs->msr >> (63-34)) & 1;
+}
+
+static void regs_advance_insn(struct pt_regs *regs)
+{
+	if (regs_is_prefix(regs))
+		regs->nip += 8;
+	else
+		regs->nip += 4;
+}
+
+static volatile bool got_interrupt;
+static volatile struct pt_regs recorded_regs;
+
+static void mce_handler(struct pt_regs *regs, void *opaque)
+{
+	got_interrupt = true;
+	memcpy((void *)&recorded_regs, regs, sizeof(struct pt_regs));
+	regs_advance_insn(regs);
+}
+
+static void test_mce(void)
+{
+	unsigned long addr = -4ULL;
+	uint8_t tmp;
+
+	handle_exception(0x200, mce_handler, NULL);
+
+	if (machine_is_powernv()) {
+		enable_mcheck();
+	} else {
+		report(mfmsr() & MSR_ME, "pseries machine has MSR[ME]=1");
+		if (!(mfmsr() & MSR_ME)) { /* try to fix it */
+			enable_mcheck();
+		}
+		if (mfmsr() & MSR_ME) {
+			disable_mcheck();
+			report(mfmsr() & MSR_ME, "pseries is unable to change MSR[ME]");
+			if (!(mfmsr() & MSR_ME)) { /* try to fix it */
+				enable_mcheck();
+			}
+		}
+	}
+
+	asm volatile("lbz %0,0(%1)" : "=r"(tmp) : "r"(addr));
+
+	report(got_interrupt, "MCE on access to invalid real address");
+	report(mfspr(SPR_DAR) == addr, "MCE sets DAR correctly");
+	got_interrupt = false;
+}
+
+static void dseg_handler(struct pt_regs *regs, void *data)
+{
+	got_interrupt = true;
+	memcpy((void *)&recorded_regs, regs, sizeof(struct pt_regs));
+	regs_advance_insn(regs);
+	regs->msr &= ~MSR_DR;
+}
+
+static void test_dseg(void)
+{
+	uint64_t msr, tmp;
+
+	report_prefix_push("data segment");
+
+	/* Some HV start in radix mode and need 0x300 */
+	handle_exception(0x300, &dseg_handler, NULL);
+	handle_exception(0x380, &dseg_handler, NULL);
+
+	asm volatile(
+"		mfmsr	%0		\n \
+		ori	%0,%0,%2	\n \
+		mtmsrd	%0		\n \
+		lbz	%1,0(0)		"
+		: "=r"(msr), "=r"(tmp) : "i"(MSR_DR): "memory");
+
+	report(got_interrupt, "interrupt on NULL dereference");
+	got_interrupt = false;
+
+	handle_exception(0x300, NULL, NULL);
+	handle_exception(0x380, NULL, NULL);
+
+	report_prefix_pop();
+}
+
+static void dec_handler(struct pt_regs *regs, void *data)
+{
+	got_interrupt = true;
+	memcpy((void *)&recorded_regs, regs, sizeof(struct pt_regs));
+	regs->msr &= ~MSR_EE;
+}
+
+static void test_dec(void)
+{
+	uint64_t msr;
+
+	report_prefix_push("decrementer");
+
+	handle_exception(0x900, &dec_handler, NULL);
+
+	asm volatile(
+"		mtdec	%1		\n \
+		mfmsr	%0		\n \
+		ori	%0,%0,%2	\n \
+		mtmsrd	%0,1		"
+		: "=r"(msr) : "r"(10000), "i"(MSR_EE): "memory");
+
+	while (!got_interrupt)
+		;
+
+	report(got_interrupt, "interrupt on decrementer underflow");
+	got_interrupt = false;
+
+	handle_exception(0x900, NULL, NULL);
+
+	if (!machine_is_powernv())
+		goto done;
+
+	handle_exception(0x980, &dec_handler, NULL);
+
+	mtspr(SPR_LPCR, mfspr(SPR_LPCR) | LPCR_HDICE);
+	asm volatile(
+"		mtspr	0x136,%1	\n \
+		mtdec	%3		\n \
+		mfmsr	%0		\n \
+		ori	%0,%0,%2	\n \
+		mtmsrd	%0,1		"
+		: "=r"(msr) : "r"(10000), "i"(MSR_EE), "r"(0x7fffffff): "memory");
+
+	while (!got_interrupt)
+		;
+
+	mtspr(SPR_LPCR, mfspr(SPR_LPCR) & ~LPCR_HDICE);
+
+	report(got_interrupt, "interrupt on hdecrementer underflow");
+	got_interrupt = false;
+
+	handle_exception(0x980, NULL, NULL);
+
+done:
+	report_prefix_pop();
+}
+
+
+static volatile uint64_t recorded_heir;
+
+static void heai_handler(struct pt_regs *regs, void *data)
+{
+	got_interrupt = true;
+	memcpy((void *)&recorded_regs, regs, sizeof(struct pt_regs));
+	regs_advance_insn(regs);
+	if (cpu_has_heir())
+		recorded_heir = mfspr(SPR_HEIR);
+}
+
+static void program_handler(struct pt_regs *regs, void *data)
+{
+	got_interrupt = true;
+	memcpy((void *)&recorded_regs, regs, sizeof(struct pt_regs));
+	regs_advance_insn(regs);
+}
+
+/*
+ * This tests invalid instruction handling. powernv (HV) should take an
+ * HEAI interrupt with the HEIR SPR set to the instruction image. pseries
+ * (guest) should take a program interrupt. CPUs which support prefix
+ * should report prefix instruction in (H)SRR1[34].
+ */
+static void test_illegal(void)
+{
+	report_prefix_push("illegal instruction");
+
+	if (machine_is_powernv()) {
+		handle_exception(0xe40, &heai_handler, NULL);
+	} else {
+		handle_exception(0x700, &program_handler, NULL);
+	}
+
+	asm volatile(".long 0x12345678" ::: "memory");
+	report(got_interrupt, "interrupt on invalid instruction");
+	got_interrupt = false;
+	if (cpu_has_heir())
+		report(recorded_heir == 0x12345678, "HEIR: 0x%08lx", recorded_heir);
+	report(!regs_is_prefix(&recorded_regs), "(H)SRR1 prefix bit: %d", regs_is_prefix(&recorded_regs));
+
+	if (cpu_has_prefix()) {
+		mtspr(SPR_FSCR, mfspr(SPR_FSCR) | FSCR_PREFIX);
+		asm volatile(".balign 8 ; .long 0x04000123; .long 0x00badc0d");
+		report(got_interrupt, "interrupt on invalid prefix instruction");
+		got_interrupt = false;
+		if (cpu_has_heir())
+			report(recorded_heir == 0x0400012300badc0d, "HEIR: 0x%08lx", recorded_heir);
+		report(regs_is_prefix(&recorded_regs), "(H)SRR1 prefix bit: %d", regs_is_prefix(&recorded_regs));
+	}
+
+	handle_exception(0xe40, NULL, NULL);
+	handle_exception(0x700, NULL, NULL);
+
+	report_prefix_pop();
+}
+
+static void sc_handler(struct pt_regs *regs, void *data)
+{
+	got_interrupt = true;
+	memcpy((void *)&recorded_regs, regs, sizeof(struct pt_regs));
+}
+
+static void test_sc(void)
+{
+	report_prefix_push("syscall");
+
+	handle_exception(0xc00, &sc_handler, NULL);
+
+	asm volatile("sc 0" ::: "memory");
+
+	report(got_interrupt, "interrupt on sc 0 instruction");
+	got_interrupt = false;
+	if (cpu_has_lev_in_srr1())
+		report(((recorded_regs.msr >> 20) & 0x3) == 0, "SRR1 set LEV=0");
+	if (machine_is_powernv()) {
+		asm volatile("sc 1" ::: "memory");
+
+		report(got_interrupt, "interrupt on sc 1 instruction");
+		got_interrupt = false;
+		if (cpu_has_lev_in_srr1())
+			report(((recorded_regs.msr >> 20) & 0x3) == 1, "SRR1 set LEV=1");
+	}
+
+	handle_exception(0xc00, NULL, NULL);
+
+	report_prefix_pop();
+}
+
+
+static void trace_handler(struct pt_regs *regs, void *data)
+{
+	got_interrupt = true;
+	memcpy((void *)&recorded_regs, regs, sizeof(struct pt_regs));
+	regs->msr &= ~(MSR_SE | MSR_BE);
+}
+
+static void program_trace_handler(struct pt_regs *regs, void *data)
+{
+	regs->msr &= ~(MSR_SE | MSR_BE);
+	regs->nip += 4;
+}
+
+extern char trace_insn[];
+extern char trace_insn2[];
+extern char trace_insn3[];
+extern char trace_rfid[];
+
+static void test_trace(void)
+{
+	unsigned long msr;
+
+	report_prefix_push("trace");
+
+	handle_exception(0xd00, &trace_handler, NULL);
+
+	msr = mfmsr() | MSR_SE;
+	asm volatile(
+	"	mtmsr	%0		\n"
+	".global trace_insn		\n"
+	"trace_insn:			\n"
+	"	nop			\n"
+	: : "r"(msr) : "memory");
+
+	report(got_interrupt, "interrupt on single step");
+	got_interrupt = false;
+	report(recorded_regs.nip == (unsigned long)trace_insn + 4,
+			"single step interrupt at the correct address");
+	report(mfspr(SPR_SIAR) == (unsigned long)trace_insn,
+			"single step recorded SIAR at the correct address");
+
+	msr = mfmsr() | MSR_SE;
+	asm volatile(
+	"	mtmsr	%0		\n"
+	".global trace_insn2		\n"
+	"trace_insn2:			\n"
+	"	b	1f		\n"
+	"	nop			\n"
+	"1:				\n"
+	: : "r"(msr) : "memory");
+
+	report(got_interrupt, "interrupt on single step branch");
+	got_interrupt = false;
+	report(recorded_regs.nip == (unsigned long)trace_insn2 + 8,
+			"single step interrupt at the correct address");
+	report(mfspr(SPR_SIAR) == (unsigned long)trace_insn2,
+			"single step recorded SIAR at the correct address");
+
+	msr = mfmsr() | MSR_BE;
+	asm volatile(
+	"	mtmsr	%0		\n"
+	".global trace_insn3		\n"
+	"trace_insn3:			\n"
+	"	nop			\n"
+	"	b	1f		\n"
+	"	nop			\n"
+	"1:				\n"
+	: : "r"(msr) : "memory");
+
+	report(got_interrupt, "interrupt on branch trace");
+	got_interrupt = false;
+	report(recorded_regs.nip == (unsigned long)trace_insn3 + 12,
+			"branch trace interrupt at the correct address");
+	report(mfspr(SPR_SIAR) == (unsigned long)trace_insn3 + 4,
+			"branch trace recorded SIAR at the correct address");
+
+	handle_exception(0x700, &program_trace_handler, NULL);
+	msr = mfmsr() | MSR_SE;
+	asm volatile(
+	"	mtmsr	%0		\n"
+	"	trap			\n"
+	: : "r"(msr) : "memory");
+
+	report(!got_interrupt, "no interrupt on single step trap");
+	got_interrupt = false;
+	handle_exception(0x700, NULL, NULL);
+
+	msr = mfmsr() | MSR_SE;
+	mtspr(SPR_SRR0, (unsigned long)trace_rfid);
+	mtspr(SPR_SRR1, mfmsr());
+	asm volatile(
+	"	mtmsr	%0		\n"
+	"	rfid			\n"
+	".global trace_rfid		\n"
+	"trace_rfid:			\n"
+	: : "r"(msr) : "memory");
+
+	report(!got_interrupt, "no interrupt on single step rfid");
+	got_interrupt = false;
+	handle_exception(0xd00, NULL, NULL);
+
+	report_prefix_pop();
+}
+
+
+int main(int argc, char **argv)
+{
+	report_prefix_push("interrupts");
+
+	test_mce();
+	test_dseg();
+	test_illegal();
+	test_dec();
+	test_sc();
+	test_trace();
+
+	report_prefix_pop();
+
+	return report_summary();
+}
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index 5d6ba852..1ae9a00e 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -69,6 +69,9 @@ groups = rtas
 [emulator]
 file = emulator.elf
 
+[interrupts]
+file = interrupts.elf
+
 [h_cede_tm]
 file = tm.elf
 machine = pseries
-- 
2.42.0

Re: [kvm-unit-tests PATCH v5 24/29] powerpc: interrupt tests

[Thomas Huth]

On 16/12/2023 14.42, Nicholas Piggin wrote:
> Add basic testing of various kinds of interrupts, machine check,
> page fault, illegal, decrementer, trace, syscall, etc.
> 
> This has a known failure on QEMU TCG pseries machines where MSR[ME]
> can be incorrectly set to 0.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   lib/powerpc/asm/ppc_asm.h |  21 +-
>   powerpc/Makefile.common   |   3 +-
>   powerpc/interrupts.c      | 422 ++++++++++++++++++++++++++++++++++++++
>   powerpc/unittests.cfg     |   3 +
>   4 files changed, 445 insertions(+), 4 deletions(-)
>   create mode 100644 powerpc/interrupts.c
> 
> diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
> index ef2d91dd..778e78ee 100644
> --- a/lib/powerpc/asm/ppc_asm.h
> +++ b/lib/powerpc/asm/ppc_asm.h
> @@ -35,17 +35,32 @@
>   
>   #endif /* __BYTE_ORDER__ */
>   
> +#define SPR_DSISR	0x012
> +#define SPR_DAR		0x013
> +#define SPR_DEC		0x016
> +#define SPR_SRR0	0x01A
> +#define SPR_SRR1	0x01B
> +#define SPR_FSCR	0x099
> +#define   FSCR_PREFIX	0x2000
> +#define SPR_HDEC	0x136
>   #define SPR_HSRR0	0x13A
>   #define SPR_HSRR1	0x13B
> +#define SPR_LPCR	0x13E
> +#define   LPCR_HDICE	0x1UL
> +#define SPR_HEIR	0x153
> +#define SPR_SIAR	0x31C
>   
>   /* 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 MSR_ME		0x1000ULL
>   
> -#define SPR_HSRR0	0x13A
> -#define SPR_HSRR1	0x13B
> +#define MSR_DR		0x0010ULL
> +#define MSR_IR		0x0020ULL
> +#define MSR_BE		0x0200ULL		/* Branch Trace Enable */
> +#define MSR_SE		0x0400ULL		/* Single Step Enable */
> +#define MSR_EE		0x8000ULL
> +#define MSR_ME		0x1000ULL
>   
>   #endif /* _ASMPOWERPC_PPC_ASM_H */
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index a7af225b..b340a53b 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -11,7 +11,8 @@ tests-common = \
>   	$(TEST_DIR)/rtas.elf \
>   	$(TEST_DIR)/emulator.elf \
>   	$(TEST_DIR)/tm.elf \
> -	$(TEST_DIR)/sprs.elf
> +	$(TEST_DIR)/sprs.elf \
> +	$(TEST_DIR)/interrupts.elf
>   
>   tests-all = $(tests-common) $(tests)
>   all: directories $(TEST_DIR)/boot_rom.bin $(tests-all)
> diff --git a/powerpc/interrupts.c b/powerpc/interrupts.c
> new file mode 100644
> index 00000000..3217b15e
> --- /dev/null
> +++ b/powerpc/interrupts.c
> @@ -0,0 +1,422 @@
> +/*
> + * Test interrupts
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include <libcflat.h>
> +#include <util.h>
> +#include <migrate.h>
> +#include <alloc.h>
> +#include <asm/handlers.h>
> +#include <asm/hcall.h>
> +#include <asm/processor.h>
> +#include <asm/barrier.h>
> +
> +#define SPR_LPCR	0x13E
> +#define LPCR_HDICE	0x1UL
> +#define SPR_DEC		0x016
> +#define SPR_HDEC	0x136
> +
> +#define MSR_DR		0x0010ULL
> +#define MSR_IR		0x0020ULL
> +#define MSR_EE		0x8000ULL
> +#define MSR_ME		0x1000ULL

Why don't you use the definitions from ppc_asm.h above?

> +static bool cpu_has_heir(void)
> +{
> +	uint32_t pvr = mfspr(287);	/* Processor Version Register */
> +
> +	if (!machine_is_powernv())
> +		return false;
> +
> +	/* POWER6 has HEIR, but QEMU powernv support does not go that far */
> +	switch (pvr >> 16) {
> +	case 0x4b:			/* POWER8E */
> +	case 0x4c:			/* POWER8NVL */
> +	case 0x4d:			/* POWER8 */
> +	case 0x4e:			/* POWER9 */
> +	case 0x80:			/* POWER10 */

I'd suggest to introduce some #defines for those PVR values instead of using 
magic numbers all over the place?

> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool cpu_has_prefix(void)
> +{
> +	uint32_t pvr = mfspr(287);	/* Processor Version Register */
> +	switch (pvr >> 16) {
> +	case 0x80:			/* POWER10 */
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool cpu_has_lev_in_srr1(void)
> +{
> +	uint32_t pvr = mfspr(287);	/* Processor Version Register */
> +	switch (pvr >> 16) {
> +	case 0x80:			/* POWER10 */
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool regs_is_prefix(volatile struct pt_regs *regs)
> +{
> +	return (regs->msr >> (63-34)) & 1;

You introduced a bunch of new #define MSR_xx statements ... why not for this 
one, too?

> +}
> +
> +static void regs_advance_insn(struct pt_regs *regs)
> +{
> +	if (regs_is_prefix(regs))
> +		regs->nip += 8;
> +	else
> +		regs->nip += 4;
> +}
> +
> +static volatile bool got_interrupt;
> +static volatile struct pt_regs recorded_regs;
> +
> +static void mce_handler(struct pt_regs *regs, void *opaque)
> +{
> +	got_interrupt = true;
> +	memcpy((void *)&recorded_regs, regs, sizeof(struct pt_regs));
> +	regs_advance_insn(regs);
> +}
> +
> +static void test_mce(void)
> +{
> +	unsigned long addr = -4ULL;
> +	uint8_t tmp;
> +
> +	handle_exception(0x200, mce_handler, NULL);
> +
> +	if (machine_is_powernv()) {
> +		enable_mcheck();
> +	} else {
> +		report(mfmsr() & MSR_ME, "pseries machine has MSR[ME]=1");
> +		if (!(mfmsr() & MSR_ME)) { /* try to fix it */
> +			enable_mcheck();
> +		}
> +		if (mfmsr() & MSR_ME) {
> +			disable_mcheck();
> +			report(mfmsr() & MSR_ME, "pseries is unable to change MSR[ME]");
> +			if (!(mfmsr() & MSR_ME)) { /* try to fix it */
> +				enable_mcheck();
> +			}
> +		}
> +	}
> +
> +	asm volatile("lbz %0,0(%1)" : "=r"(tmp) : "r"(addr));
> +
> +	report(got_interrupt, "MCE on access to invalid real address");
> +	report(mfspr(SPR_DAR) == addr, "MCE sets DAR correctly");
> +	got_interrupt = false;
> +}
> +
> +static void dseg_handler(struct pt_regs *regs, void *data)
> +{
> +	got_interrupt = true;
> +	memcpy((void *)&recorded_regs, regs, sizeof(struct pt_regs));
> +	regs_advance_insn(regs);
> +	regs->msr &= ~MSR_DR;
> +}
> +
> +static void test_dseg(void)
> +{
> +	uint64_t msr, tmp;
> +
> +	report_prefix_push("data segment");
> +
> +	/* Some HV start in radix mode and need 0x300 */
> +	handle_exception(0x300, &dseg_handler, NULL);
> +	handle_exception(0x380, &dseg_handler, NULL);
> +
> +	asm volatile(
> +"		mfmsr	%0		\n \
> +		ori	%0,%0,%2	\n \
> +		mtmsrd	%0		\n \
> +		lbz	%1,0(0)		"
> +		: "=r"(msr), "=r"(tmp) : "i"(MSR_DR): "memory");
> +
> +	report(got_interrupt, "interrupt on NULL dereference");
> +	got_interrupt = false;
> +
> +	handle_exception(0x300, NULL, NULL);
> +	handle_exception(0x380, NULL, NULL);
> +
> +	report_prefix_pop();
> +}
> +
> +static void dec_handler(struct pt_regs *regs, void *data)
> +{
> +	got_interrupt = true;
> +	memcpy((void *)&recorded_regs, regs, sizeof(struct pt_regs));
> +	regs->msr &= ~MSR_EE;
> +}
> +
> +static void test_dec(void)
> +{
> +	uint64_t msr;
> +
> +	report_prefix_push("decrementer");
> +
> +	handle_exception(0x900, &dec_handler, NULL);
> +
> +	asm volatile(
> +"		mtdec	%1		\n \
> +		mfmsr	%0		\n \
> +		ori	%0,%0,%2	\n \
> +		mtmsrd	%0,1		"
> +		: "=r"(msr) : "r"(10000), "i"(MSR_EE): "memory");
> +
> +	while (!got_interrupt)
> +		;

Maybe add a timeout (in case the interrupt never fires)?

> +	report(got_interrupt, "interrupt on decrementer underflow");
> +	got_interrupt = false;
> +
> +	handle_exception(0x900, NULL, NULL);
> +
> +	if (!machine_is_powernv())
> +		goto done;
> +
> +	handle_exception(0x980, &dec_handler, NULL);
> +
> +	mtspr(SPR_LPCR, mfspr(SPR_LPCR) | LPCR_HDICE);
> +	asm volatile(
> +"		mtspr	0x136,%1	\n \
> +		mtdec	%3		\n \
> +		mfmsr	%0		\n \
> +		ori	%0,%0,%2	\n \
> +		mtmsrd	%0,1		"
> +		: "=r"(msr) : "r"(10000), "i"(MSR_EE), "r"(0x7fffffff): "memory");
> +
> +	while (!got_interrupt)
> +		;

dito?

> +	mtspr(SPR_LPCR, mfspr(SPR_LPCR) & ~LPCR_HDICE);
> +
> +	report(got_interrupt, "interrupt on hdecrementer underflow");
> +	got_interrupt = false;
> +
> +	handle_exception(0x980, NULL, NULL);
> +
> +done:
> +	report_prefix_pop();
> +}

  Thomas

Re: [kvm-unit-tests PATCH v5 24/29] powerpc: interrupt tests

[Nicholas Piggin]

On Tue Dec 19, 2023 at 11:57 PM AEST, Thomas Huth wrote:
> On 16/12/2023 14.42, Nicholas Piggin wrote:
> > Add basic testing of various kinds of interrupts, machine check,
> > page fault, illegal, decrementer, trace, syscall, etc.
> > 
> > This has a known failure on QEMU TCG pseries machines where MSR[ME]
> > can be incorrectly set to 0.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   lib/powerpc/asm/ppc_asm.h |  21 +-
> >   powerpc/Makefile.common   |   3 +-
> >   powerpc/interrupts.c      | 422 ++++++++++++++++++++++++++++++++++++++
> >   powerpc/unittests.cfg     |   3 +
> >   4 files changed, 445 insertions(+), 4 deletions(-)
> >   create mode 100644 powerpc/interrupts.c
> > 
> > diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
> > index ef2d91dd..778e78ee 100644
> > --- a/lib/powerpc/asm/ppc_asm.h
> > +++ b/lib/powerpc/asm/ppc_asm.h
> > @@ -35,17 +35,32 @@
> >   
> >   #endif /* __BYTE_ORDER__ */
> >   
> > +#define SPR_DSISR	0x012
> > +#define SPR_DAR		0x013
> > +#define SPR_DEC		0x016
> > +#define SPR_SRR0	0x01A
> > +#define SPR_SRR1	0x01B
> > +#define SPR_FSCR	0x099
> > +#define   FSCR_PREFIX	0x2000
> > +#define SPR_HDEC	0x136
> >   #define SPR_HSRR0	0x13A
> >   #define SPR_HSRR1	0x13B
> > +#define SPR_LPCR	0x13E
> > +#define   LPCR_HDICE	0x1UL
> > +#define SPR_HEIR	0x153
> > +#define SPR_SIAR	0x31C
> >   
> >   /* 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 MSR_ME		0x1000ULL
> >   
> > -#define SPR_HSRR0	0x13A
> > -#define SPR_HSRR1	0x13B
> > +#define MSR_DR		0x0010ULL
> > +#define MSR_IR		0x0020ULL
> > +#define MSR_BE		0x0200ULL		/* Branch Trace Enable */
> > +#define MSR_SE		0x0400ULL		/* Single Step Enable */
> > +#define MSR_EE		0x8000ULL
> > +#define MSR_ME		0x1000ULL
> >   
> >   #endif /* _ASMPOWERPC_PPC_ASM_H */
> > diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> > index a7af225b..b340a53b 100644
> > --- a/powerpc/Makefile.common
> > +++ b/powerpc/Makefile.common
> > @@ -11,7 +11,8 @@ tests-common = \
> >   	$(TEST_DIR)/rtas.elf \
> >   	$(TEST_DIR)/emulator.elf \
> >   	$(TEST_DIR)/tm.elf \
> > -	$(TEST_DIR)/sprs.elf
> > +	$(TEST_DIR)/sprs.elf \
> > +	$(TEST_DIR)/interrupts.elf
> >   
> >   tests-all = $(tests-common) $(tests)
> >   all: directories $(TEST_DIR)/boot_rom.bin $(tests-all)
> > diff --git a/powerpc/interrupts.c b/powerpc/interrupts.c
> > new file mode 100644
> > index 00000000..3217b15e
> > --- /dev/null
> > +++ b/powerpc/interrupts.c
> > @@ -0,0 +1,422 @@
> > +/*
> > + * Test interrupts
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#include <libcflat.h>
> > +#include <util.h>
> > +#include <migrate.h>
> > +#include <alloc.h>
> > +#include <asm/handlers.h>
> > +#include <asm/hcall.h>
> > +#include <asm/processor.h>
> > +#include <asm/barrier.h>
> > +
> > +#define SPR_LPCR	0x13E
> > +#define LPCR_HDICE	0x1UL
> > +#define SPR_DEC		0x016
> > +#define SPR_HDEC	0x136
> > +
> > +#define MSR_DR		0x0010ULL
> > +#define MSR_IR		0x0020ULL
> > +#define MSR_EE		0x8000ULL
> > +#define MSR_ME		0x1000ULL
>
> Why don't you use the definitions from ppc_asm.h above?

Yeah, should be more consistent with those. I'll take a look.

>
> > +static bool cpu_has_heir(void)
> > +{
> > +	uint32_t pvr = mfspr(287);	/* Processor Version Register */
> > +
> > +	if (!machine_is_powernv())
> > +		return false;
> > +
> > +	/* POWER6 has HEIR, but QEMU powernv support does not go that far */
> > +	switch (pvr >> 16) {
> > +	case 0x4b:			/* POWER8E */
> > +	case 0x4c:			/* POWER8NVL */
> > +	case 0x4d:			/* POWER8 */
> > +	case 0x4e:			/* POWER9 */
> > +	case 0x80:			/* POWER10 */
>
> I'd suggest to introduce some #defines for those PVR values instead of using 
> magic numbers all over the place?

Yeah you're right.

>
> > +		return true;
> > +	default:
> > +		return false;
> > +	}
> > +}
> > +
> > +static bool cpu_has_prefix(void)
> > +{
> > +	uint32_t pvr = mfspr(287);	/* Processor Version Register */
> > +	switch (pvr >> 16) {
> > +	case 0x80:			/* POWER10 */
> > +		return true;
> > +	default:
> > +		return false;
> > +	}
> > +}
> > +
> > +static bool cpu_has_lev_in_srr1(void)
> > +{
> > +	uint32_t pvr = mfspr(287);	/* Processor Version Register */
> > +	switch (pvr >> 16) {
> > +	case 0x80:			/* POWER10 */
> > +		return true;
> > +	default:
> > +		return false;
> > +	}
> > +}
> > +
> > +static bool regs_is_prefix(volatile struct pt_regs *regs)
> > +{
> > +	return (regs->msr >> (63-34)) & 1;
>
> You introduced a bunch of new #define MSR_xx statements ... why not for this 
> one, too?

It's an interrupt-specific bit so SRR1_xx, but yes it should be a
define.

>
> > +}
> > +
> > +static void regs_advance_insn(struct pt_regs *regs)
> > +{
> > +	if (regs_is_prefix(regs))
> > +		regs->nip += 8;
> > +	else
> > +		regs->nip += 4;
> > +}
> > +
> > +static volatile bool got_interrupt;
> > +static volatile struct pt_regs recorded_regs;
> > +
> > +static void mce_handler(struct pt_regs *regs, void *opaque)
> > +{
> > +	got_interrupt = true;
> > +	memcpy((void *)&recorded_regs, regs, sizeof(struct pt_regs));
> > +	regs_advance_insn(regs);
> > +}
> > +
> > +static void test_mce(void)
> > +{
> > +	unsigned long addr = -4ULL;
> > +	uint8_t tmp;
> > +
> > +	handle_exception(0x200, mce_handler, NULL);
> > +
> > +	if (machine_is_powernv()) {
> > +		enable_mcheck();
> > +	} else {
> > +		report(mfmsr() & MSR_ME, "pseries machine has MSR[ME]=1");
> > +		if (!(mfmsr() & MSR_ME)) { /* try to fix it */
> > +			enable_mcheck();
> > +		}
> > +		if (mfmsr() & MSR_ME) {
> > +			disable_mcheck();
> > +			report(mfmsr() & MSR_ME, "pseries is unable to change MSR[ME]");
> > +			if (!(mfmsr() & MSR_ME)) { /* try to fix it */
> > +				enable_mcheck();
> > +			}
> > +		}
> > +	}
> > +
> > +	asm volatile("lbz %0,0(%1)" : "=r"(tmp) : "r"(addr));
> > +
> > +	report(got_interrupt, "MCE on access to invalid real address");
> > +	report(mfspr(SPR_DAR) == addr, "MCE sets DAR correctly");
> > +	got_interrupt = false;
> > +}
> > +
> > +static void dseg_handler(struct pt_regs *regs, void *data)
> > +{
> > +	got_interrupt = true;
> > +	memcpy((void *)&recorded_regs, regs, sizeof(struct pt_regs));
> > +	regs_advance_insn(regs);
> > +	regs->msr &= ~MSR_DR;
> > +}
> > +
> > +static void test_dseg(void)
> > +{
> > +	uint64_t msr, tmp;
> > +
> > +	report_prefix_push("data segment");
> > +
> > +	/* Some HV start in radix mode and need 0x300 */
> > +	handle_exception(0x300, &dseg_handler, NULL);
> > +	handle_exception(0x380, &dseg_handler, NULL);
> > +
> > +	asm volatile(
> > +"		mfmsr	%0		\n \
> > +		ori	%0,%0,%2	\n \
> > +		mtmsrd	%0		\n \
> > +		lbz	%1,0(0)		"
> > +		: "=r"(msr), "=r"(tmp) : "i"(MSR_DR): "memory");
> > +
> > +	report(got_interrupt, "interrupt on NULL dereference");
> > +	got_interrupt = false;
> > +
> > +	handle_exception(0x300, NULL, NULL);
> > +	handle_exception(0x380, NULL, NULL);
> > +
> > +	report_prefix_pop();
> > +}
> > +
> > +static void dec_handler(struct pt_regs *regs, void *data)
> > +{
> > +	got_interrupt = true;
> > +	memcpy((void *)&recorded_regs, regs, sizeof(struct pt_regs));
> > +	regs->msr &= ~MSR_EE;
> > +}
> > +
> > +static void test_dec(void)
> > +{
> > +	uint64_t msr;
> > +
> > +	report_prefix_push("decrementer");
> > +
> > +	handle_exception(0x900, &dec_handler, NULL);
> > +
> > +	asm volatile(
> > +"		mtdec	%1		\n \
> > +		mfmsr	%0		\n \
> > +		ori	%0,%0,%2	\n \
> > +		mtmsrd	%0,1		"
> > +		: "=r"(msr) : "r"(10000), "i"(MSR_EE): "memory");
> > +
> > +	while (!got_interrupt)
> > +		;
>
> Maybe add a timeout (in case the interrupt never fires)?

Yeah that would improve things.

> > +	report(got_interrupt, "interrupt on decrementer underflow");
> > +	got_interrupt = false;
> > +
> > +	handle_exception(0x900, NULL, NULL);
> > +
> > +	if (!machine_is_powernv())
> > +		goto done;
> > +
> > +	handle_exception(0x980, &dec_handler, NULL);
> > +
> > +	mtspr(SPR_LPCR, mfspr(SPR_LPCR) | LPCR_HDICE);
> > +	asm volatile(
> > +"		mtspr	0x136,%1	\n \
> > +		mtdec	%3		\n \
> > +		mfmsr	%0		\n \
> > +		ori	%0,%0,%2	\n \
> > +		mtmsrd	%0,1		"
> > +		: "=r"(msr) : "r"(10000), "i"(MSR_EE), "r"(0x7fffffff): "memory");
> > +
> > +	while (!got_interrupt)
> > +		;
>
> dito?

Will do.

Thanks,
Nick

[kvm-unit-tests PATCH v5 25/29] powerpc: Add rtas stop-self support

[Nicholas Piggin]

In preparation for improved SMP support, add stop-self support to the
harness. This is non-trivial because it requires an unlocked rtas
call: a CPU can't be holding a spin lock when it goes offline or it
will deadlock other CPUs. rtas permits stop-self to be called without
serialising all other rtas operations.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 lib/powerpc/asm/rtas.h |  2 ++
 lib/powerpc/rtas.c     | 78 +++++++++++++++++++++++++++++++++---------
 2 files changed, 64 insertions(+), 16 deletions(-)

diff --git a/lib/powerpc/asm/rtas.h b/lib/powerpc/asm/rtas.h
index 6fb407a1..364bf935 100644
--- a/lib/powerpc/asm/rtas.h
+++ b/lib/powerpc/asm/rtas.h
@@ -23,8 +23,10 @@ struct rtas_args {
 extern void rtas_init(void);
 extern int rtas_token(const char *service, uint32_t *token);
 extern int rtas_call(int token, int nargs, int nret, int *outputs, ...);
+extern int rtas_call_unlocked(struct rtas_args *args, int token, int nargs, int nret, int *outputs, ...);
 
 extern void rtas_power_off(void);
+extern void rtas_stop_self(void);
 #endif /* __ASSEMBLY__ */
 
 #define RTAS_MSR_MASK 0xfffffffffffffffe
diff --git a/lib/powerpc/rtas.c b/lib/powerpc/rtas.c
index 41c0a243..b77a60b0 100644
--- a/lib/powerpc/rtas.c
+++ b/lib/powerpc/rtas.c
@@ -87,40 +87,86 @@ int rtas_token(const char *service, uint32_t *token)
 	return 0;
 }
 
-int rtas_call(int token, int nargs, int nret, int *outputs, ...)
+static void __rtas_call(struct rtas_args *args)
 {
-	va_list list;
-	int ret, i;
+	enter_rtas(__pa(args));
+}
 
-	spin_lock(&rtas_lock);
+static int rtas_call_unlocked_va(struct rtas_args *args,
+			  int token, int nargs, int nret, int *outputs,
+			  va_list list)
+{
+	int ret, i;
 
-	rtas_args.token = cpu_to_be32(token);
-	rtas_args.nargs = cpu_to_be32(nargs);
-	rtas_args.nret = cpu_to_be32(nret);
-	rtas_args.rets = &rtas_args.args[nargs];
+	args->token = cpu_to_be32(token);
+	args->nargs = cpu_to_be32(nargs);
+	args->nret = cpu_to_be32(nret);
+	args->rets = &args->args[nargs];
 
-	va_start(list, outputs);
 	for (i = 0; i < nargs; ++i)
-		rtas_args.args[i] = cpu_to_be32(va_arg(list, u32));
-	va_end(list);
+		args->args[i] = cpu_to_be32(va_arg(list, u32));
 
 	for (i = 0; i < nret; ++i)
-		rtas_args.rets[i] = 0;
+		args->rets[i] = 0;
 
-	enter_rtas(__pa(&rtas_args));
+	__rtas_call(args);
 
 	if (nret > 1 && outputs != NULL)
 		for (i = 0; i < nret - 1; ++i)
-			outputs[i] = be32_to_cpu(rtas_args.rets[i + 1]);
+			outputs[i] = be32_to_cpu(args->rets[i + 1]);
+
+	ret = nret > 0 ? be32_to_cpu(args->rets[0]) : 0;
+
+	return ret;
+}
+
+int rtas_call_unlocked(struct rtas_args *args, int token, int nargs, int nret, int *outputs, ...)
+{
+	va_list list;
+	int ret;
 
-	ret = nret > 0 ? be32_to_cpu(rtas_args.rets[0]) : 0;
+	va_start(list, outputs);
+	ret = rtas_call_unlocked_va(args, token, nargs, nret, outputs, list);
+	va_end(list);
+
+	return ret;
+}
+
+int rtas_call(int token, int nargs, int nret, int *outputs, ...)
+{
+	va_list list;
+	int ret;
+
+	spin_lock(&rtas_lock);
+
+	va_start(list, outputs);
+	ret = rtas_call_unlocked_va(&rtas_args, token, nargs, nret, outputs, list);
+	va_end(list);
 
 	spin_unlock(&rtas_lock);
+
 	return ret;
 }
 
+void rtas_stop_self(void)
+{
+	struct rtas_args args;
+	uint32_t token;
+	int ret;
+
+	ret = rtas_token("stop-self", &token);
+	if (ret) {
+		puts("RTAS stop-self not available\n");
+		return;
+	}
+
+	ret = rtas_call_unlocked(&args, token, 0, 1, NULL);
+	printf("RTAS stop-self returnd %d\n", ret);
+}
+
 void rtas_power_off(void)
 {
+	struct rtas_args args;
 	uint32_t token;
 	int ret;
 
@@ -130,6 +176,6 @@ void rtas_power_off(void)
 		return;
 	}
 
-	ret = rtas_call(token, 2, 1, NULL, -1, -1);
+	ret = rtas_call_unlocked(&args, token, 2, 1, NULL, -1, -1);
 	printf("RTAS power-off returned %d\n", ret);
 }
-- 
2.42.0

Re: [kvm-unit-tests PATCH v5 25/29] powerpc: Add rtas stop-self support

[Thomas Huth]

On 16/12/2023 14.42, Nicholas Piggin wrote:
> In preparation for improved SMP support, add stop-self support to the
> harness. This is non-trivial because it requires an unlocked rtas
> call: a CPU can't be holding a spin lock when it goes offline or it
> will deadlock other CPUs. rtas permits stop-self to be called without
> serialising all other rtas operations.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   lib/powerpc/asm/rtas.h |  2 ++
>   lib/powerpc/rtas.c     | 78 +++++++++++++++++++++++++++++++++---------
>   2 files changed, 64 insertions(+), 16 deletions(-)
...
> +void rtas_stop_self(void)
> +{
> +	struct rtas_args args;
> +	uint32_t token;
> +	int ret;
> +
> +	ret = rtas_token("stop-self", &token);
> +	if (ret) {
> +		puts("RTAS stop-self not available\n");
> +		return;
> +	}
> +
> +	ret = rtas_call_unlocked(&args, token, 0, 1, NULL);
> +	printf("RTAS stop-self returnd %d\n", ret);

s/returnd/returned/

> +}

With the typo fixed:

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

[kvm-unit-tests PATCH v5 26/29] powerpc: add SMP and IPI support

[Nicholas Piggin]

powerpc SMP support is very primitive and does not set up a first-class
runtime environment for secondary CPUs.

This adds more complete support for a C environment and interrupt
handling, as well as IPI support.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 lib/powerpc/asm/processor.h |   6 +
 lib/powerpc/asm/setup.h     |   3 +-
 lib/powerpc/asm/smp.h       |  23 ++--
 lib/powerpc/setup.c         |  37 +++--
 lib/powerpc/smp.c           | 264 ++++++++++++++++++++++++++++++++----
 lib/ppc64/asm/atomic.h      |   6 +
 lib/ppc64/asm/opal.h        |   5 +
 powerpc/Makefile.common     |   1 +
 powerpc/cstart64.S          |  46 +++++++
 powerpc/interrupts.c        |  10 --
 powerpc/smp.c               | 221 ++++++++++++++++++++++++++++++
 powerpc/tm.c                |   2 +-
 powerpc/unittests.cfg       |   8 ++
 13 files changed, 570 insertions(+), 62 deletions(-)
 create mode 100644 lib/ppc64/asm/atomic.h
 create mode 100644 powerpc/smp.c

diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
index 6368a9ee..924451da 100644
--- a/lib/powerpc/asm/processor.h
+++ b/lib/powerpc/asm/processor.h
@@ -15,6 +15,7 @@ void do_handle_exception(struct pt_regs *regs);
 #define SPR_SPRG1	0x111
 #define SPR_SPRG2	0x112
 #define SPR_SPRG3	0x113
+#define SPR_TBU40	0x11e
 
 static inline uint64_t mfspr(int nr)
 {
@@ -53,6 +54,11 @@ static inline bool machine_is_powernv(void)
 	return !!(mfmsr() & (1ULL << MSR_HV_BIT));
 }
 
+static inline uint64_t smp_processor_id(void)
+{
+	return mfspr(SPR_SPRG2);
+}
+
 static inline uint64_t get_tb(void)
 {
 	return mfspr(SPR_TB);
diff --git a/lib/powerpc/asm/setup.h b/lib/powerpc/asm/setup.h
index cc7cf5e2..c0d892da 100644
--- a/lib/powerpc/asm/setup.h
+++ b/lib/powerpc/asm/setup.h
@@ -8,9 +8,10 @@
 #include <libcflat.h>
 
 #define NR_CPUS			8	/* arbitrarily set for now */
-extern u32 cpus[NR_CPUS];
 extern int nr_cpus;
 
+#define EXCEPTION_STACK_SIZE	(32*1024) /* 32kB */
+
 extern uint64_t tb_hz;
 
 #define NR_MEM_REGIONS		8
diff --git a/lib/powerpc/asm/smp.h b/lib/powerpc/asm/smp.h
index 21940b4b..163bbeec 100644
--- a/lib/powerpc/asm/smp.h
+++ b/lib/powerpc/asm/smp.h
@@ -3,20 +3,21 @@
 
 #include <libcflat.h>
 
-extern int nr_threads;
+typedef void (*secondary_entry_fn)(int cpu_id);
 
-struct start_threads {
-	int nr_threads;
-	int nr_started;
-};
+extern void halt(int cpu_id);
 
-typedef void (*secondary_entry_fn)(void);
+extern bool start_all_cpus(secondary_entry_fn entry);
+extern void stop_all_cpus(void);
 
-extern void halt(void);
+struct pt_regs;
+void register_ipi(void (*fn)(struct pt_regs *, void *), void *data);
+void unregister_ipi(void);
+void cpu_init_ipis(void);
+void local_ipi_enable(void);
+void local_ipi_disable(void);
+void send_ipi(int cpu_id);
 
-extern int start_thread(int cpu_id, secondary_entry_fn entry, uint32_t r3);
-extern struct start_threads start_cpu(int cpu_node, secondary_entry_fn entry,
-				      uint32_t r3);
-extern bool start_all_cpus(secondary_entry_fn entry, uint32_t r3);
+extern int nr_cpus_online;
 
 #endif /* _ASMPOWERPC_SMP_H_ */
diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
index 2b7e27b0..41830ead 100644
--- a/lib/powerpc/setup.c
+++ b/lib/powerpc/setup.c
@@ -17,6 +17,7 @@
 #include <alloc_phys.h>
 #include <argv.h>
 #include <asm/setup.h>
+#include <asm/smp.h>
 #include <asm/page.h>
 #include <asm/ptrace.h>
 #include <asm/processor.h>
@@ -28,7 +29,7 @@ extern unsigned long stacktop;
 char *initrd;
 u32 initrd_size;
 
-u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
+u32 cpu_to_hwid[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
 int nr_cpus;
 uint64_t tb_hz;
 
@@ -42,23 +43,31 @@ struct cpu_set_params {
 	uint64_t tb_hz;
 };
 
-#define EXCEPTION_STACK_SIZE	(32*1024) /* 32kB */
-
-static char exception_stack[NR_CPUS][EXCEPTION_STACK_SIZE];
+static char boot_exception_stack[EXCEPTION_STACK_SIZE];
 
 static void cpu_set(int fdtnode, u64 regval, void *info)
 {
+	const struct fdt_property *prop;
+	u32 *threads;
 	static bool read_common_info = false;
 	struct cpu_set_params *params = info;
-	int cpu = nr_cpus++;
+	int nr_threads;
+	int len, i;
 
-	assert_msg(cpu < NR_CPUS, "Number cpus exceeds maximum supported (%d).", NR_CPUS);
+	/* Get the id array of threads on this node */
+	prop = fdt_get_property(dt_fdt(), fdtnode,
+				"ibm,ppc-interrupt-server#s", &len);
+	assert(prop);
 
-	cpus[cpu] = regval;
+	nr_threads = len >> 2; /* Divide by 4 since 4 bytes per thread */
+	threads = (u32 *)prop->data; /* Array of valid ids */
+
+	assert_msg(nr_cpus + nr_threads <= NR_CPUS, "Number cpus exceeds maximum supported (%d).", NR_CPUS);
+
+	for (i = 0; i < nr_threads; i++) {
+		cpu_to_hwid[nr_cpus++] = fdt32_to_cpu(threads[i]);
+	}
 
-	/* set exception stack address for this CPU (in SPGR0) */
-	asm volatile ("mtsprg0 %[addr]" ::
-		      [addr] "r" (exception_stack[cpu + 1]));
 
 	if (!read_common_info) {
 		const struct fdt_property *prop;
@@ -161,6 +170,10 @@ void setup(const void *fdt)
 	u32 fdt_size;
 	int ret;
 
+	/* set exception stack address for this CPU (in SPGR0) */
+	asm volatile ("mtsprg0 %[addr]" :: [addr] "r" (boot_exception_stack +
+							EXCEPTION_STACK_SIZE));
+
 	enable_mcheck();
 
 	/*
@@ -202,6 +215,10 @@ void setup(const void *fdt)
 
 	assert(STACK_INT_FRAME_SIZE % 16 == 0);
 
+	mtspr(SPR_SPRG2, fdt_boot_cpuid_phys(dt_fdt()));
+
+	cpu_init_ipis();
+
 	/* call init functions */
 	cpu_init();
 
diff --git a/lib/powerpc/smp.c b/lib/powerpc/smp.c
index afe43617..96e3219a 100644
--- a/lib/powerpc/smp.c
+++ b/lib/powerpc/smp.c
@@ -6,55 +6,250 @@
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
 
+#include <alloc.h>
 #include <devicetree.h>
+#include <asm/atomic.h>
+#include <asm/barrier.h>
+#include <asm/processor.h>
 #include <asm/setup.h>
+#include <asm/opal.h>
+#include <asm/hcall.h>
 #include <asm/rtas.h>
 #include <asm/smp.h>
 
-int nr_threads;
+struct start_threads {
+	int nr_threads;
+	int nr_started;
+};
 
 struct secondary_entry_data {
 	secondary_entry_fn entry;
-	uint64_t r3;
 	int nr_started;
 };
 
+struct cpu {
+	uint64_t server_no;
+	unsigned long stack;
+	unsigned long exception_stack;
+	secondary_entry_fn entry;
+} __attribute__((packed)); /* used by asm */
+
+int nr_cpus_online = 1;
+
+struct cpu start_secondary_cpus[NR_CPUS];
+
+static void stop_self(int cpu_id)
+{
+	if (machine_is_powernv()) {
+		if (opal_call(OPAL_RETURN_CPU, 0, 0, 0) != OPAL_SUCCESS) {
+			printf("OPAL_RETURN_CPU failed\n");
+		}
+	} else {
+		rtas_stop_self();
+	}
+
+	printf("failed to stop cpu %d\n", cpu_id);
+	assert(0);
+}
+
+void main_secondary(struct cpu *cpu);
+void main_secondary(struct cpu *cpu)
+{
+	asm volatile ("mtsprg0 %[addr]" :: [addr] "r" (cpu->exception_stack +
+							EXCEPTION_STACK_SIZE));
+	mtspr(SPR_SPRG2, cpu->server_no);
+
+	enable_mcheck();
+
+	cpu_init_ipis();
+
+	atomic_fetch_inc(&nr_cpus_online);
+
+	cpu->entry(cpu->server_no);
+
+	atomic_fetch_dec(&nr_cpus_online);
+
+	stop_self(cpu->server_no);
+}
+
+#define OPAL_START_CPU				41
+#define OPAL_QUERY_CPU_STATUS			42
+#define OPAL_RETURN_CPU				69
+
+enum OpalThreadStatus {
+        OPAL_THREAD_INACTIVE = 0x0,
+        OPAL_THREAD_STARTED = 0x1,
+        OPAL_THREAD_UNAVAILABLE = 0x2 /* opal-v3 */
+};
+
+#define H_EOI		0x64
+#define H_CPPR		0x68
+#define H_IPI		0x6c
+#define H_XIRR		0x74
+
+static void (*ipi_fn)(struct pt_regs *regs, void *data);
+
+static void dbell_handler(struct pt_regs *regs, void *data)
+{
+	/* sync */
+	ipi_fn(regs, data);
+}
+
+static void extint_handler(struct pt_regs *regs, void *data)
+{
+	int32_t xirr;
+	int32_t xisr;
+	int64_t rc;
+
+	asm volatile("mr r3,%1 ; sc 1 ; mr %0,r4" : "=r"(xirr) : "r"(H_XIRR));
+	xisr = xirr & 0xffffff;
+
+	if (xisr == 2) { /* IPI */
+		rc = hcall(H_IPI, smp_processor_id(), 0xff);
+		assert(rc == H_SUCCESS);
+	}
+
+	xirr |= (5 << 24);
+	rc = hcall(H_EOI, xirr);
+	assert(rc == H_SUCCESS);
+
+	/* lower IPI */
+	ipi_fn(regs, data);
+}
+
+void cpu_init_ipis(void)
+{
+	if (machine_is_powernv()) {
+		/* skiboot can leave some messages set */
+		unsigned long rb = (5 << (63-36));
+		asm volatile("msgclr	%0" :: "r"(rb) : "memory");
+	}
+}
+
+void local_ipi_enable(void)
+{
+	if (!machine_is_powernv()) {
+		hcall(H_CPPR, 5);
+	}
+}
+
+void local_ipi_disable(void)
+{
+	if (!machine_is_powernv()) {
+		hcall(H_CPPR, 0);
+	}
+}
+
+void register_ipi(void (*fn)(struct pt_regs *, void *), void *data)
+{
+	ipi_fn = fn;
+	if (machine_is_powernv()) {
+		handle_exception(0xe80, &dbell_handler, data);
+	} else {
+		handle_exception(0x500, &extint_handler, data);
+	}
+}
+
+void unregister_ipi(void)
+{
+	if (machine_is_powernv()) {
+		handle_exception(0xe80, NULL, NULL);
+	} else {
+		handle_exception(0x500, NULL, NULL);
+	}
+}
+
+void send_ipi(int cpu_id)
+{
+	if (machine_is_powernv()) {
+		unsigned long rb = (5 << (63-36)) | cpu_id;
+		asm volatile("lwsync" ::: "memory");
+		asm volatile("msgsnd	%0" :: "r"(rb) : "memory");
+	} else {
+		hcall(H_IPI, cpu_id, 4);
+	}
+}
+
+static int nr_started = 0;
+extern void start_secondary(uint64_t server_no); /* asm entry point */
+
 /*
  * Start stopped thread cpu_id at entry
  * Returns:	<0 on failure to start stopped cpu
  *		0  on success
  *		>0 on cpu not in stopped state
  */
-int start_thread(int cpu_id, secondary_entry_fn entry, uint32_t r3)
+static int start_thread(int cpu_id, secondary_entry_fn entry)
 {
-	uint32_t query_token, start_token;
-	int outputs[1], ret;
+	struct cpu *cpu;
 
-	ret = rtas_token("query-cpu-stopped-state", &query_token);
-	assert(ret == 0);
-	ret = rtas_token("start-cpu", &start_token);
-	assert(ret == 0);
+	cpu = &start_secondary_cpus[nr_started];
+	nr_started++;
 
-	ret = rtas_call(query_token, 1, 2, outputs, cpu_id);
-	if (ret) {
-		printf("query-cpu-stopped-state failed for cpu %d\n", cpu_id);
-	} else if (!outputs[0]) { /* cpu in stopped state */
-		ret = rtas_call(start_token, 3, 1, NULL, cpu_id, entry, r3);
-		if (ret)
+	cpu->server_no = cpu_id;
+	if (cpu_id == smp_processor_id()) {
+		/* Boot CPU already started */
+		return -1;
+	}
+
+	cpu->stack = (unsigned long)memalign(4096, 64*1024);
+	cpu->stack += 64*1024 - 64;
+	cpu->exception_stack = (unsigned long)memalign(4096, EXCEPTION_STACK_SIZE);
+	cpu->entry = entry;
+
+	if (machine_is_powernv()) {
+		int64_t ret;
+		uint8_t status;
+
+		ret = opal_call(OPAL_QUERY_CPU_STATUS, cpu_id, (unsigned long)&status, 0);
+		if (ret != OPAL_SUCCESS) {
+			printf("OPAL_QUERY_CPU_STATUS failed for cpu %d\n", cpu_id);
+			return -1;
+		}
+		if (status != OPAL_THREAD_INACTIVE) {
+			printf("cpu %d not in stopped state\n", cpu_id);
+			return -1;
+		}
+		if (opal_call(OPAL_START_CPU, cpu_id, (unsigned long)start_secondary, 0) != OPAL_SUCCESS) {
 			printf("failed to start cpu %d\n", cpu_id);
-	} else { /* cpu not in stopped state */
-		ret = outputs[0];
+			return -1;
+		}
+	} else {
+		uint32_t query_token, start_token;
+		int outputs[1], ret;
+
+		ret = rtas_token("query-cpu-stopped-state", &query_token);
+		assert(ret == 0);
+		ret = rtas_token("start-cpu", &start_token);
+		assert(ret == 0);
+
+		ret = rtas_call(query_token, 1, 2, outputs, cpu_id);
+		if (ret) {
+			printf("query-cpu-stopped-state failed for cpu %d\n", cpu_id);
+			return ret;
+		}
+		if (outputs[0]) { /* cpu not in stopped state */
+			ret = outputs[0];
+			printf("cpu %d not in stopped state\n", cpu_id);
+			return ret;
+		}
+		ret = rtas_call(start_token, 3, 1, NULL, cpu_id, start_secondary, cpu_id);
+		if (ret) {
+			printf("failed to start cpu %d\n", cpu_id);
+			return ret;
+		}
 	}
 
-	return ret;
+	return 0;
 }
 
+static int nr_cpus_present;
+
 /*
  * Start all stopped threads (vcpus) on cpu_node
  * Returns: Number of stopped cpus which were successfully started
  */
-struct start_threads start_cpu(int cpu_node, secondary_entry_fn entry,
-			       uint32_t r3)
+static int start_core(int cpu_node, secondary_entry_fn entry)
 {
 	int len, i, nr_threads, nr_started = 0;
 	const struct fdt_property *prop;
@@ -66,23 +261,23 @@ struct start_threads start_cpu(int cpu_node, secondary_entry_fn entry,
 	assert(prop);
 
 	nr_threads = len >> 2; /* Divide by 4 since 4 bytes per thread */
+	nr_cpus_present += nr_threads;
+
 	threads = (u32 *)prop->data; /* Array of valid ids */
 
 	for (i = 0; i < nr_threads; i++) {
-		if (!start_thread(fdt32_to_cpu(threads[i]), entry, r3))
+		if (!start_thread(fdt32_to_cpu(threads[i]), entry))
 			nr_started++;
 	}
 
-	return (struct start_threads) { nr_threads, nr_started };
+	return nr_started;
 }
 
 static void start_each_secondary(int fdtnode, u64 regval __unused, void *info)
 {
 	struct secondary_entry_data *datap = info;
-	struct start_threads ret = start_cpu(fdtnode, datap->entry, datap->r3);
 
-	nr_threads += ret.nr_threads;
-	datap->nr_started += ret.nr_started;
+	datap->nr_started += start_core(fdtnode, datap->entry);
 }
 
 /*
@@ -91,14 +286,25 @@ static void start_each_secondary(int fdtnode, u64 regval __unused, void *info)
  * Returns:	TRUE on success
  *		FALSE on failure
  */
-bool start_all_cpus(secondary_entry_fn entry, uint32_t r3)
+bool start_all_cpus(secondary_entry_fn entry)
 {
-	struct secondary_entry_data data = { entry, r3,	0 };
+	struct secondary_entry_data data = { entry, 0 };
 	int ret;
 
+	memset(start_secondary_cpus, 0xff, sizeof(start_secondary_cpus));
+
+	assert(nr_cpus_online == 1);
+
+	nr_started = 0;
+	nr_cpus_present = 0;
 	ret = dt_for_each_cpu_node(start_each_secondary, &data);
 	assert(ret == 0);
 
-	/* We expect that we come in with one thread already started */
-	return data.nr_started == nr_threads - 1;
+	return 1;
+}
+
+void stop_all_cpus(void)
+{
+	while (nr_cpus_online > 1)
+		cpu_relax();
 }
diff --git a/lib/ppc64/asm/atomic.h b/lib/ppc64/asm/atomic.h
new file mode 100644
index 00000000..0f461b9c
--- /dev/null
+++ b/lib/ppc64/asm/atomic.h
@@ -0,0 +1,6 @@
+#ifndef _POWERPC_ATOMIC_H_
+#define _POWERPC_ATOMIC_H_
+
+#include "asm-generic/atomic.h"
+
+#endif /* _POWERPC_ATOMIC_H_ */
diff --git a/lib/ppc64/asm/opal.h b/lib/ppc64/asm/opal.h
index 7b1299f7..ce6dbb16 100644
--- a/lib/ppc64/asm/opal.h
+++ b/lib/ppc64/asm/opal.h
@@ -2,14 +2,19 @@
 #ifndef _ASMPPC64_HCALL_H_
 #define _ASMPPC64_HCALL_H_
 
+#include <stdint.h>
+
 #define OPAL_SUCCESS				0
 
 #define OPAL_CONSOLE_WRITE			1
 #define OPAL_CONSOLE_READ			2
 #define OPAL_CEC_POWER_DOWN			5
 #define OPAL_POLL_EVENTS			10
+#define OPAL_RETURN_CPU				69
 #define OPAL_REINIT_CPUS			70
 # define OPAL_REINIT_CPUS_HILE_BE		(1 << 0)
 # define OPAL_REINIT_CPUS_HILE_LE		(1 << 1)
 
+int64_t opal_call(int64_t token, int64_t arg1, int64_t arg2, int64_t arg3);
+
 #endif
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index b340a53b..f9dd937a 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -11,6 +11,7 @@ tests-common = \
 	$(TEST_DIR)/rtas.elf \
 	$(TEST_DIR)/emulator.elf \
 	$(TEST_DIR)/tm.elf \
+	$(TEST_DIR)/smp.elf \
 	$(TEST_DIR)/sprs.elf \
 	$(TEST_DIR)/interrupts.elf
 
diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
index 51d632da..57edd0ae 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S
@@ -128,6 +128,52 @@ start:
 	bl	exit
 	b	halt
 
+/*
+ * start_secondary is the secondary entry point. r3 contains the cpu id
+ */
+.globl start_secondary
+start_secondary:
+	FIXUP_ENDIAN
+	/* Switch to 64-bit mode */
+	mfmsr	r1
+	li	r2,1
+	sldi	r2,r2,MSR_SF_BIT
+	or	r1,r1,r2
+	mtmsrd	r1
+
+	bl	0f
+0:	mflr	r31
+	subi	r31, r31, 0b - start	/* QEMU's kernel load address */
+
+	ld	r2, (p_toc - start)(r31)
+
+	LOAD_REG_ADDR(r9, start_secondary_cpus)
+	li	r8,0
+	li	r7,0
+1:	ldx	r6,r9,r7
+	cmpd	r6,r3
+	beq	2f
+	addi	r7,r7,32 /* sizeof (struct cpu) */
+	addi	r8,r8,1
+	cmpdi	r8,16 /* MAX_CPUS */
+	bne	1b
+	b	.
+
+2:	add	r3,r9,r7
+	ld	r1,8(r3)
+
+	/* Zero backpointers in initial stack frame so backtrace() stops */
+	li	r0,0
+	std	r0,0(r1)
+	std	r0,16(r1)
+
+	/* Create entry frame */
+	stdu	r1,-INT_FRAME_SIZE(r1)
+
+	bl	main_secondary
+	bl	exit
+	b	halt
+
 .align 3
 p_stack:	.llong  stackptr
 p_toc:		.llong  tocptr
diff --git a/powerpc/interrupts.c b/powerpc/interrupts.c
index 3217b15e..14805d76 100644
--- a/powerpc/interrupts.c
+++ b/powerpc/interrupts.c
@@ -12,16 +12,6 @@
 #include <asm/processor.h>
 #include <asm/barrier.h>
 
-#define SPR_LPCR	0x13E
-#define LPCR_HDICE	0x1UL
-#define SPR_DEC		0x016
-#define SPR_HDEC	0x136
-
-#define MSR_DR		0x0010ULL
-#define MSR_IR		0x0020ULL
-#define MSR_EE		0x8000ULL
-#define MSR_ME		0x1000ULL
-
 static bool cpu_has_heir(void)
 {
 	uint32_t pvr = mfspr(287);	/* Processor Version Register */
diff --git a/powerpc/smp.c b/powerpc/smp.c
new file mode 100644
index 00000000..b0a99069
--- /dev/null
+++ b/powerpc/smp.c
@@ -0,0 +1,221 @@
+/*
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ * SMP Tests
+ */
+#include <libcflat.h>
+#include <asm/atomic.h>
+#include <asm/barrier.h>
+#include <asm/processor.h>
+#include <asm/smp.h>
+#include <asm/setup.h>
+#include <asm/ppc_asm.h>
+#include <devicetree.h>
+
+static volatile int nr_cpus_started;
+static volatile int secondary_cpu_id;
+
+static void start_fn(int cpu_id)
+{
+	atomic_fetch_inc(&nr_cpus_started);
+}
+
+static void test_start_cpus(int argc, char **argv)
+{
+	if (argc > 2)
+		report_abort("Unsupported argument: '%s'", argv[2]);
+
+	nr_cpus_started = 1;
+	if (!start_all_cpus(start_fn))
+		report_abort("Failed to start secondary cpus");
+
+	while (nr_cpus_started < nr_cpus)
+		cpu_relax();
+
+	stop_all_cpus();
+
+	report(true, "start cpus");
+}
+
+static volatile int nr_cpus_ipi = 0;
+
+static void ipi_handler(struct pt_regs *regs, void *data)
+{
+	atomic_fetch_inc(&nr_cpus_ipi);
+}
+
+static void local_irq_enable(void)
+{
+	unsigned long msr;
+
+	asm volatile(
+"		mfmsr	%0		\n \
+		ori	%0,%0,%1	\n \
+		mtmsrd	%0,1		"
+		: "=r"(msr) : "i"(MSR_EE): "memory");
+}
+
+static void local_irq_disable(void)
+{
+	unsigned long msr;
+
+	asm volatile(
+"		mfmsr	%0		\n \
+		andc	%0,%0,%1	\n \
+		mtmsrd	%0,1		"
+		: "=r"(msr) : "r"(MSR_EE): "memory");
+}
+
+static volatile bool ipi_test_running = true;
+
+static void ipi_fn(int cpu_id)
+{
+	local_ipi_enable();
+
+	secondary_cpu_id = cpu_id;
+	atomic_fetch_inc(&nr_cpus_started);
+	mtspr(SPR_DEC, 0x7fffffff);
+	local_irq_enable();
+	while (ipi_test_running)
+		cpu_relax();
+	local_irq_disable();
+
+	local_ipi_disable();
+	atomic_fetch_dec(&nr_cpus_started);
+}
+
+static void test_ipi_cpus(int argc, char **argv)
+{
+	if (argc > 2)
+		report_abort("Unsupported argument: '%s'", argv[2]);
+
+	register_ipi(ipi_handler, NULL);
+
+	nr_cpus_started = 1;
+	if (!start_all_cpus(ipi_fn))
+		report_abort("Failed to start secondary cpus");
+
+	while (nr_cpus_started < nr_cpus)
+		cpu_relax();
+
+	send_ipi(secondary_cpu_id);
+
+	while (nr_cpus_ipi < 1)
+		cpu_relax();
+
+	send_ipi(secondary_cpu_id);
+
+	while (nr_cpus_ipi < 2)
+		cpu_relax();
+
+	ipi_test_running = false;
+
+	while (nr_cpus_started > 1)
+		cpu_relax();
+
+	msleep(1000);
+
+	stop_all_cpus();
+
+	assert(nr_cpus_ipi == 2);
+
+	unregister_ipi();
+
+	report(true, "IPI cpus");
+}
+
+static uint64_t time;
+
+static void check_and_record_time(void)
+{
+	uint64_t tb;
+	uint64_t t;
+	uint64_t old;
+
+	t = time;
+again:
+	barrier();
+	tb = get_tb();
+	asm volatile("1: ldarx %0,0,%1 ; cmpd %0,%2 ; bne 2f ; stdcx. %3,0,%1 ; bne- 1b; 2:" : "=&r"(old) : "r"(&time), "r"(t), "r"(tb) : "memory", "cr0");
+	assert(tb >= t);
+	if (old != t) {
+		t = old;
+		goto again;
+	}
+	assert(old <= tb);
+}
+
+static void update_time(int64_t tb_offset)
+{
+	uint64_t new_tb;
+
+	new_tb = get_tb() + tb_offset;
+	mtspr(SPR_TBU40, new_tb);
+	if ((get_tb() & 0xFFFFFF) < (new_tb & 0xFFFFFF)) {
+		new_tb += 0x1000000;
+		mtspr(SPR_TBU40, new_tb);
+	}
+}
+
+static void time_sync_fn(int cpu_id)
+{
+	uint64_t start = get_tb();
+
+	while (get_tb() - start < tb_hz*3) {
+		check_and_record_time();
+		update_time(0x1234000000);
+		cpu_relax();
+		update_time(-0x1234000000);
+		udelay(1);
+	}
+}
+
+static void test_time_sync(int argc, char **argv)
+{
+	if (argc > 2)
+		report_abort("Unsupported argument: '%s'", argv[2]);
+
+	if (!machine_is_powernv()) {
+		report_skip("requires powernv");
+		return;
+	}
+
+	if (!start_all_cpus(time_sync_fn))
+		report_abort("Failed to start secondary cpus");
+
+	time_sync_fn(-1);
+
+	stop_all_cpus();
+
+	report(true, "time sync");
+}
+
+struct {
+	const char *name;
+	void (*func)(int argc, char **argv);
+} hctests[] = {
+	{ "start_cpus", test_start_cpus },
+	{ "ipi_cpus", test_ipi_cpus },
+	{ "time_sync", test_time_sync },
+	{ NULL, NULL }
+};
+
+int main(int argc, char **argv)
+{
+	bool all;
+	int i;
+
+	all = argc == 1 || !strcmp(argv[1], "all");
+
+	report_prefix_push("smp");
+
+	for (i = 0; hctests[i].name != NULL; i++) {
+		if (all || strcmp(argv[1], hctests[i].name) == 0) {
+			report_prefix_push(hctests[i].name);
+			hctests[i].func(argc, argv);
+			report_prefix_pop();
+		}
+	}
+
+	report_prefix_pop();
+	return report_summary();
+}
diff --git a/powerpc/tm.c b/powerpc/tm.c
index 7fa91636..84ab3dc9 100644
--- a/powerpc/tm.c
+++ b/powerpc/tm.c
@@ -88,7 +88,7 @@ static void test_h_cede_tm(int argc, char **argv)
 	if (argc > 2)
 		report_abort("Unsupported argument: '%s'", argv[2]);
 
-	if (!start_all_cpus(halt, 0))
+	if (!start_all_cpus(halt))
 		report_abort("Failed to start secondary cpus");
 
 	if (!enable_tm())
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index 1ae9a00e..727712bb 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -72,6 +72,14 @@ file = emulator.elf
 [interrupts]
 file = interrupts.elf
 
+[smp]
+file = smp.elf
+smp = 2
+
+[smp-smt]
+file = smp.elf
+smp = 8,threads=4
+
 [h_cede_tm]
 file = tm.elf
 machine = pseries
-- 
2.42.0

[kvm-unit-tests PATCH v5 27/29] powerpc: Avoid using larx/stcx. in spinlocks when only one CPU is running

[Nicholas Piggin]

The test harness uses spinlocks if they are implemented with larx/stcx.
it can prevent some test scenarios such as testing migration of a
reservation.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 lib/powerpc/asm/smp.h    |  1 +
 lib/powerpc/smp.c        |  6 ++++++
 lib/powerpc/spinlock.c   | 28 ++++++++++++++++++++++++++++
 lib/ppc64/asm/spinlock.h |  7 ++++++-
 powerpc/Makefile.common  |  1 +
 5 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100644 lib/powerpc/spinlock.c

diff --git a/lib/powerpc/asm/smp.h b/lib/powerpc/asm/smp.h
index 163bbeec..e2c03295 100644
--- a/lib/powerpc/asm/smp.h
+++ b/lib/powerpc/asm/smp.h
@@ -19,5 +19,6 @@ void local_ipi_disable(void);
 void send_ipi(int cpu_id);
 
 extern int nr_cpus_online;
+extern bool multithreaded;
 
 #endif /* _ASMPOWERPC_SMP_H_ */
diff --git a/lib/powerpc/smp.c b/lib/powerpc/smp.c
index 96e3219a..b473ba41 100644
--- a/lib/powerpc/smp.c
+++ b/lib/powerpc/smp.c
@@ -280,6 +280,8 @@ static void start_each_secondary(int fdtnode, u64 regval __unused, void *info)
 	datap->nr_started += start_core(fdtnode, datap->entry);
 }
 
+bool multithreaded = false;
+
 /*
  * Start all stopped cpus on the guest at entry with register 3 set to r3
  * We expect that we come in with only one thread currently started
@@ -293,8 +295,10 @@ bool start_all_cpus(secondary_entry_fn entry)
 
 	memset(start_secondary_cpus, 0xff, sizeof(start_secondary_cpus));
 
+	assert(!multithreaded);
 	assert(nr_cpus_online == 1);
 
+	multithreaded = true;
 	nr_started = 0;
 	nr_cpus_present = 0;
 	ret = dt_for_each_cpu_node(start_each_secondary, &data);
@@ -305,6 +309,8 @@ bool start_all_cpus(secondary_entry_fn entry)
 
 void stop_all_cpus(void)
 {
+	assert(multithreaded);
 	while (nr_cpus_online > 1)
 		cpu_relax();
+	multithreaded = false;
 }
diff --git a/lib/powerpc/spinlock.c b/lib/powerpc/spinlock.c
new file mode 100644
index 00000000..238549f1
--- /dev/null
+++ b/lib/powerpc/spinlock.c
@@ -0,0 +1,28 @@
+#include <asm/spinlock.h>
+#include <asm/smp.h>
+
+/*
+ * Skip the atomic when single-threaded, which helps avoid larx/stcx. in
+ * the harness when testing tricky larx/stcx. sequences (e.g., migration
+ * vs reservation).
+ */
+void spin_lock(struct spinlock *lock)
+{
+	if (!multithreaded) {
+		assert(lock->v == 0);
+		lock->v = 1;
+	} else {
+		while (__sync_lock_test_and_set(&lock->v, 1))
+			;
+	}
+}
+
+void spin_unlock(struct spinlock *lock)
+{
+	assert(lock->v == 1);
+	if (!multithreaded) {
+		lock->v = 0;
+	} else {
+		__sync_lock_release(&lock->v);
+	}
+}
diff --git a/lib/ppc64/asm/spinlock.h b/lib/ppc64/asm/spinlock.h
index f59eed19..b952386d 100644
--- a/lib/ppc64/asm/spinlock.h
+++ b/lib/ppc64/asm/spinlock.h
@@ -1,6 +1,11 @@
 #ifndef _ASMPPC64_SPINLOCK_H_
 #define _ASMPPC64_SPINLOCK_H_
 
-#include <asm-generic/spinlock.h>
+struct spinlock {
+	unsigned int v;
+};
+
+void spin_lock(struct spinlock *lock);
+void spin_unlock(struct spinlock *lock);
 
 #endif /* _ASMPPC64_SPINLOCK_H_ */
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index f9dd937a..caa807f2 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -47,6 +47,7 @@ cflatobjs += lib/powerpc/rtas.o
 cflatobjs += lib/powerpc/processor.o
 cflatobjs += lib/powerpc/handlers.o
 cflatobjs += lib/powerpc/smp.o
+cflatobjs += lib/powerpc/spinlock.o
 
 OBJDIRS += lib/powerpc
 
-- 
2.42.0

[kvm-unit-tests PATCH v5 28/29] powerpc: Add atomics tests

[Nicholas Piggin]

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 powerpc/Makefile.common |   1 +
 powerpc/atomics.c       | 190 ++++++++++++++++++++++++++++++++++++++++
 powerpc/unittests.cfg   |   9 ++
 3 files changed, 200 insertions(+)
 create mode 100644 powerpc/atomics.c

diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index caa807f2..697f5735 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -10,6 +10,7 @@ tests-common = \
 	$(TEST_DIR)/spapr_hcall.elf \
 	$(TEST_DIR)/rtas.elf \
 	$(TEST_DIR)/emulator.elf \
+	$(TEST_DIR)/atomics.elf \
 	$(TEST_DIR)/tm.elf \
 	$(TEST_DIR)/smp.elf \
 	$(TEST_DIR)/sprs.elf \
diff --git a/powerpc/atomics.c b/powerpc/atomics.c
new file mode 100644
index 00000000..f2e7a3e3
--- /dev/null
+++ b/powerpc/atomics.c
@@ -0,0 +1,190 @@
+/*
+ * Test some powerpc instructions
+ */
+#include <stdint.h>
+#include <libcflat.h>
+#include <migrate.h>
+#include <asm/processor.h>
+
+static int verbose;
+
+#define RSV_SIZE 128
+
+static uint8_t granule[RSV_SIZE] __attribute((__aligned__(RSV_SIZE)));
+
+static void test_lwarx_stwcx(void)
+{
+	unsigned int *var = (unsigned int *)granule;
+	unsigned int old;
+	unsigned int result;
+
+	report_prefix_push("lwarx/stwcx.");
+
+	*var = 0;
+	asm volatile ("1:"
+		      "lwarx	%0,0,%2;"
+		      "stwcx.	%1,0,%2;"
+		      "bne-	1b;"
+		      : "=&r"(old) : "r"(1), "r"(var) : "cr0", "memory");
+	report(old == 0 && *var == 1, "simple update");
+
+	*var = 0;
+	asm volatile ("li	%0,0;"
+		      "stwcx.	%1,0,%2;"
+		      "stwcx.	%1,0,%2;"
+		      "bne-	1f;"
+		      "li	%0,1;"
+		      "1:"
+		      : "=&r"(result)
+		      : "r"(1), "r"(var) : "cr0", "memory");
+	report(result == 0 && *var == 0, "failed stwcx. (no reservation)");
+
+	*var = 0;
+	asm volatile ("li	%0,0;"
+		      "lwarx	%1,0,%4;"
+		      "stw	%3,0(%4);"
+		      "stwcx.	%2,0,%4;"
+		      "bne-	1f;"
+		      "li	%0,1;"
+		      "1:"
+		      : "=&r"(result), "=&r"(old)
+		      : "r"(1), "r"(2), "r"(var) : "cr0", "memory");
+	report(result == 0 && *var == 2, "failed stwcx. (intervening store)");
+
+	report_prefix_pop();
+}
+
+static void test_lqarx_stqcx(void)
+{
+	union {
+		__int128_t var;
+		struct {
+#if  __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+			unsigned long var1;
+			unsigned long var2;
+#else
+			unsigned long var2;
+			unsigned long var1;
+#endif
+		};
+	} var __attribute__((aligned(16)));
+	register unsigned long new1 asm("r8");
+	register unsigned long new2 asm("r9");
+	register unsigned long old1 asm("r10");
+	register unsigned long old2 asm("r11");
+	unsigned int result;
+
+	var.var1 = 1;
+	var.var2 = 2;
+
+	(void)new2;
+	(void)old2;
+
+	report_prefix_push("lqarx/stqcx.");
+
+	old1 = 0;
+	old2 = 0;
+	new1 = 3;
+	new2 = 4;
+	asm volatile ("1:"
+		      "lqarx	%0,0,%4;"
+		      "stqcx.	%2,0,%4;"
+		      "bne-	1b;"
+		      : "=&r"(old1), "=&r"(old2)
+		      : "r"(new1), "r"(new2), "r"(&var)
+		      : "cr0", "memory");
+
+	report(old1 == 2 && old2 == 1 && var.var1 == 4 && var.var2 == 3,
+			"simple update");
+
+	var.var1 = 1;
+	var.var2 = 2;
+	new1 = 3;
+	new2 = 4;
+	asm volatile ("li	%0,0;"
+		      "stqcx.	%1,0,%3;"
+		      "stqcx.	%1,0,%3;"
+		      "bne-	1f;"
+		      "li	%0,1;"
+		      "1:"
+		      : "=&r"(result)
+		      : "r"(new1), "r"(new2), "r"(&var)
+		      : "cr0", "memory");
+	report(result == 0 && var.var1 == 1 && var.var2 == 2,
+			"failed stqcx. (no reservation)");
+
+	var.var1 = 1;
+	var.var2 = 2;
+	new1 = 3;
+	new2 = 4;
+	asm volatile ("li	%0,0;"
+		      "lqarx	%1,0,%6;"
+		      "std	%5,0(%6);"
+		      "stqcx.	%3,0,%6;"
+		      "bne-	1f;"
+		      "li	%0,1;"
+		      "1:"
+		      : "=&r"(result), "=&r"(old1), "=&r"(old2)
+		      : "r"(new1), "r"(new2), "r"(0), "r"(&var)
+		      : "cr0", "memory");
+	report(result == 0 && (var.var1 == 0 || var.var2 == 0),
+			"failed stqcx. (intervening store)");
+
+	report_prefix_pop();
+}
+
+static void test_migrate_reserve(void)
+{
+	unsigned int *var = (unsigned int *)granule;
+	unsigned int old;
+
+	/* ensure incorrect value does not succeed */
+	report_prefix_push("migrate reservation");
+
+	*var = 0x12345;
+	asm volatile ("lwarx	%0,0,%1" : "=&r"(old) : "r"(var) : "memory");
+	migrate();
+	*var = 0;
+	asm volatile ("stwcx.	%0,0,%1" : : "r"(0xbad), "r"(var) : "cr0", "memory");
+	report(*var == 0, "migrate reserve update fails with concurrently modified value");
+
+#if 0
+XXX this will not work with KVM and for QEMU it only works with record/replay -
+something the harness is not equipped to test.
+
+	/* ensure reservation succeds */
+	report_prefix_push("migrate reservation");
+
+	*var = 0x12345;
+	asm volatile ("lwarx	%0,0,%1" : "=&r"(old) : "r"(var) : "memory");
+	migrate();
+	asm volatile ("stwcx.	%0,0,%1" : : "r"(0xf070), "r"(var) : "cr0", "memory");
+	report(*var == 0xf070, "migrate reserve update succeeds with unmodified value");
+#endif
+}
+
+int main(int argc, char **argv)
+{
+	int i;
+	bool migrate = false;
+
+	for (i = 1; i < argc; i++) {
+		if (strcmp(argv[i], "-v") == 0) {
+			verbose = 1;
+		}
+		if (strcmp(argv[i], "-m") == 0) {
+			migrate = true;
+		}
+	}
+
+	report_prefix_push("atomics");
+
+	test_lwarx_stwcx();
+	test_lqarx_stqcx();
+	if (migrate)
+		test_migrate_reserve();
+
+	report_prefix_pop();
+
+	return report_summary();
+}
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index 727712bb..9f71ea93 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -80,6 +80,15 @@ smp = 2
 file = smp.elf
 smp = 8,threads=4
 
+[atomics]
+file = atomics.elf
+
+[atomics-migration]
+file = atomics.elf
+machine = pseries
+extra_params = -append '-m'
+groups = migration
+
 [h_cede_tm]
 file = tm.elf
 machine = pseries
-- 
2.42.0

[kvm-unit-tests PATCH v5 29/29] powerpc: Add timebase tests

[Nicholas Piggin]

This has a known failure on QEMU TCG machines where the decrementer
interrupt is not lowered when the DEC wraps from -ve to +ve.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 lib/powerpc/asm/ppc_asm.h   |   1 +
 lib/powerpc/asm/processor.h |  22 +++
 powerpc/Makefile.common     |   1 +
 powerpc/smp.c               |  22 ---
 powerpc/timebase.c          | 328 ++++++++++++++++++++++++++++++++++++
 powerpc/unittests.cfg       |   8 +
 6 files changed, 360 insertions(+), 22 deletions(-)
 create mode 100644 powerpc/timebase.c

diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
index 778e78ee..18b25fe0 100644
--- a/lib/powerpc/asm/ppc_asm.h
+++ b/lib/powerpc/asm/ppc_asm.h
@@ -47,6 +47,7 @@
 #define SPR_HSRR1	0x13B
 #define SPR_LPCR	0x13E
 #define   LPCR_HDICE	0x1UL
+#define   LPCR_LD	0x20000UL
 #define SPR_HEIR	0x153
 #define SPR_SIAR	0x31C
 
diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
index 924451da..995656b6 100644
--- a/lib/powerpc/asm/processor.h
+++ b/lib/powerpc/asm/processor.h
@@ -45,6 +45,28 @@ static inline void mtmsr(uint64_t msr)
 	asm volatile ("mtmsrd %[msr]" :: [msr] "r" (msr) : "memory");
 }
 
+static inline void local_irq_enable(void)
+{
+	unsigned long msr;
+
+	asm volatile(
+"		mfmsr	%0		\n \
+		ori	%0,%0,%1	\n \
+		mtmsrd	%0,1		"
+		: "=r"(msr) : "i"(MSR_EE): "memory");
+}
+
+static inline void local_irq_disable(void)
+{
+	unsigned long msr;
+
+	asm volatile(
+"		mfmsr	%0		\n \
+		andc	%0,%0,%1	\n \
+		mtmsrd	%0,1		"
+		: "=r"(msr) : "r"(MSR_EE): "memory");
+}
+
 /*
  * This returns true on PowerNV / OPAL machines which run in hypervisor
  * mode. False on pseries / PAPR machines that run in guest mode.
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 697f5735..448b0ded 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -14,6 +14,7 @@ tests-common = \
 	$(TEST_DIR)/tm.elf \
 	$(TEST_DIR)/smp.elf \
 	$(TEST_DIR)/sprs.elf \
+	$(TEST_DIR)/timebase.elf \
 	$(TEST_DIR)/interrupts.elf
 
 tests-all = $(tests-common) $(tests)
diff --git a/powerpc/smp.c b/powerpc/smp.c
index b0a99069..60503977 100644
--- a/powerpc/smp.c
+++ b/powerpc/smp.c
@@ -43,28 +43,6 @@ static void ipi_handler(struct pt_regs *regs, void *data)
 	atomic_fetch_inc(&nr_cpus_ipi);
 }
 
-static void local_irq_enable(void)
-{
-	unsigned long msr;
-
-	asm volatile(
-"		mfmsr	%0		\n \
-		ori	%0,%0,%1	\n \
-		mtmsrd	%0,1		"
-		: "=r"(msr) : "i"(MSR_EE): "memory");
-}
-
-static void local_irq_disable(void)
-{
-	unsigned long msr;
-
-	asm volatile(
-"		mfmsr	%0		\n \
-		andc	%0,%0,%1	\n \
-		mtmsrd	%0,1		"
-		: "=r"(msr) : "r"(MSR_EE): "memory");
-}
-
 static volatile bool ipi_test_running = true;
 
 static void ipi_fn(int cpu_id)
diff --git a/powerpc/timebase.c b/powerpc/timebase.c
new file mode 100644
index 00000000..4d80ea09
--- /dev/null
+++ b/powerpc/timebase.c
@@ -0,0 +1,328 @@
+/*
+ * Test Timebase
+ *
+ * Copyright 2017  Thomas Huth, Red Hat Inc.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ *
+ * This contains tests of timebase facility, TB, DEC, etc.
+ */
+#include <libcflat.h>
+#include <util.h>
+#include <migrate.h>
+#include <alloc.h>
+#include <asm/handlers.h>
+#include <devicetree.h>
+#include <asm/hcall.h>
+#include <asm/processor.h>
+#include <asm/barrier.h>
+
+static int dec_bits = 0;
+
+static void cpu_dec_bits(int fdtnode, u64 regval __unused, void *arg __unused)
+{
+	const struct fdt_property *prop;
+	int plen;
+
+	prop = fdt_get_property(dt_fdt(), fdtnode, "ibm,dec-bits", &plen);
+	if (!prop) {
+		dec_bits = 32;
+		return;
+	}
+
+	/* Sanity check for the property layout (first two bytes are header) */
+	assert(plen == 4);
+
+	dec_bits = fdt32_to_cpu(*(uint32_t *)prop->data);
+}
+
+/* Check amount of CPUs nodes that have the TM flag */
+static int find_dec_bits(void)
+{
+	int ret;
+
+	ret = dt_for_each_cpu_node(cpu_dec_bits, NULL);
+	if (ret < 0)
+		return ret;
+
+	return dec_bits;
+}
+
+
+static bool do_migrate = false;
+static volatile bool got_interrupt;
+static volatile struct pt_regs recorded_regs;
+
+static uint64_t dec_max;
+static uint64_t dec_min;
+
+static void test_tb(int argc, char **argv)
+{
+	uint64_t tb;
+
+	tb = get_tb();
+	if (do_migrate)
+		migrate();
+	report(get_tb() >= tb, "timebase is incrementing");
+}
+
+static void dec_stop_handler(struct pt_regs *regs, void *data)
+{
+	mtspr(SPR_DEC, dec_max);
+}
+
+static void dec_handler(struct pt_regs *regs, void *data)
+{
+	got_interrupt = true;
+	memcpy((void *)&recorded_regs, regs, sizeof(struct pt_regs));
+	regs->msr &= ~MSR_EE;
+}
+
+static void test_dec(int argc, char **argv)
+{
+	uint64_t tb1, tb2, dec;
+
+	handle_exception(0x900, &dec_handler, NULL);
+
+	tb1 = get_tb();
+	mtspr(SPR_DEC, dec_max);
+	dec = mfspr(SPR_DEC);
+	tb2 = get_tb();
+	report(tb2 - tb1 >= dec_max - dec, "decrementer remains within TB");
+	assert(tb2 - tb1 >= dec_max - dec);
+
+	tb1 = get_tb();
+	mtspr(SPR_DEC, dec_max);
+	mdelay(1000);
+	dec = mfspr(SPR_DEC);
+	tb2 = get_tb();
+	report(tb2 - tb1 >= dec_max - dec, "decrementer remains within TB after 1s");
+	assert(tb2 - tb1 >= dec_max - dec);
+
+	mtspr(SPR_DEC, dec_max);
+	local_irq_enable();
+	local_irq_disable();
+	if (mfspr(SPR_DEC) <= dec_max) {
+		report(!got_interrupt, "no interrupt on decrementer positive");
+	}
+	got_interrupt = false;
+
+	mtspr(SPR_DEC, 1);
+	mdelay(100); /* Give the timer a chance to run */
+	if (do_migrate)
+		migrate();
+	local_irq_enable();
+	local_irq_disable();
+	report(got_interrupt, "interrupt on decrementer underflow");
+	got_interrupt = false;
+
+	if (do_migrate)
+		migrate();
+	local_irq_enable();
+	local_irq_disable();
+	report(got_interrupt, "interrupt on decrementer still underflown");
+	got_interrupt = false;
+
+	mtspr(SPR_DEC, 0);
+	mdelay(100); /* Give the timer a chance to run */
+	if (do_migrate)
+		migrate();
+	local_irq_enable();
+	local_irq_disable();
+	report(got_interrupt, "DEC deal with set to 0");
+	got_interrupt = false;
+
+	/* Test for level-triggered decrementer */
+	mtspr(SPR_DEC, -1ULL);
+	if (do_migrate)
+		migrate();
+	local_irq_enable();
+	local_irq_disable();
+	report(got_interrupt, "interrupt on decrementer write MSB");
+	got_interrupt = false;
+
+	mtspr(SPR_DEC, dec_max);
+	local_irq_enable();
+	if (do_migrate)
+		migrate();
+	mtspr(SPR_DEC, -1);
+	local_irq_disable();
+	report(got_interrupt, "interrupt on decrementer write MSB with irqs on");
+	got_interrupt = false;
+
+	mtspr(SPR_DEC, dec_min + 1);
+	mdelay(100);
+	local_irq_enable();
+	local_irq_disable();
+	report(!got_interrupt, "no interrupt after wrap to positive");
+	got_interrupt = false;
+
+	handle_exception(0x900, NULL, NULL);
+}
+
+static void test_hdec(int argc, char **argv)
+{
+	uint64_t tb1, tb2, hdec;
+
+	if (!machine_is_powernv()) {
+		report_skip("skipping on !powernv machine");
+		return;
+	}
+
+	handle_exception(0x900, &dec_stop_handler, NULL);
+	handle_exception(0x980, &dec_handler, NULL);
+
+	mtspr(SPR_HDEC, dec_max);
+	mtspr(SPR_LPCR, mfspr(SPR_LPCR) | LPCR_HDICE);
+
+	tb1 = get_tb();
+	mtspr(SPR_HDEC, dec_max);
+	hdec = mfspr(SPR_HDEC);
+	tb2 = get_tb();
+	report(tb2 - tb1 >= dec_max - hdec, "hdecrementer remains within TB");
+	assert(tb2 - tb1 >= dec_max - hdec);
+
+	tb1 = get_tb();
+	mtspr(SPR_HDEC, dec_max);
+	mdelay(1000);
+	hdec = mfspr(SPR_HDEC);
+	tb2 = get_tb();
+	report(tb2 - tb1 >= dec_max - hdec, "hdecrementer remains within TB after 1s");
+	assert(tb2 - tb1 >= dec_max - hdec);
+
+	mtspr(SPR_HDEC, dec_max);
+	local_irq_enable();
+	local_irq_disable();
+	if (mfspr(SPR_HDEC) <= dec_max) {
+		report(!got_interrupt, "no interrupt on decrementer positive");
+	}
+	got_interrupt = false;
+
+	mtspr(SPR_HDEC, 1);
+	mdelay(100); /* Give the timer a chance to run */
+	if (do_migrate)
+		migrate();
+	/* HDEC is edge triggered so ensure it still fires */
+	mtspr(SPR_HDEC, dec_max);
+	local_irq_enable();
+	local_irq_disable();
+	report(got_interrupt, "interrupt on hdecrementer underflow");
+	got_interrupt = false;
+
+	if (do_migrate)
+		migrate();
+	local_irq_enable();
+	local_irq_disable();
+	report(!got_interrupt, "no interrupt on hdecrementer still underflown");
+	got_interrupt = false;
+
+	mtspr(SPR_HDEC, -1ULL);
+	if (do_migrate)
+		migrate();
+	local_irq_enable();
+	local_irq_disable();
+	report(got_interrupt, "no interrupt on hdecrementer underflown write MSB");
+	got_interrupt = false;
+
+	mtspr(SPR_HDEC, 0);
+	mdelay(100); /* Give the timer a chance to run */
+	if (do_migrate)
+		migrate();
+	/* HDEC is edge triggered so ensure it still fires */
+	mtspr(SPR_HDEC, dec_max);
+	local_irq_enable();
+	local_irq_disable();
+	report(got_interrupt, "HDEC deal with set to 0");
+	got_interrupt = false;
+
+	mtspr(SPR_HDEC, dec_max);
+	local_irq_enable();
+	if (do_migrate)
+		migrate();
+	mtspr(SPR_HDEC, -1ULL);
+	local_irq_disable();
+	report(got_interrupt, "interrupt on hdecrementer write MSB with irqs on");
+	got_interrupt = false;
+
+	mtspr(SPR_HDEC, dec_max);
+	got_interrupt = false;
+	mtspr(SPR_HDEC, dec_min + 1);
+	if (do_migrate)
+		migrate();
+	mdelay(100);
+	local_irq_enable();
+	local_irq_disable();
+	report(got_interrupt, "got interrupt after wrap to positive");
+	got_interrupt = false;
+
+	mtspr(SPR_HDEC, -1ULL);
+	local_irq_enable();
+	local_irq_disable();
+	got_interrupt = false;
+	mtspr(SPR_HDEC, dec_min + 1000000);
+	if (do_migrate)
+		migrate();
+	mdelay(100);
+	mtspr(SPR_HDEC, -1ULL);
+	local_irq_enable();
+	local_irq_disable();
+	report(got_interrupt, "edge re-armed after wrap to positive");
+	got_interrupt = false;
+
+	mtspr(SPR_LPCR, mfspr(SPR_LPCR) & ~LPCR_HDICE);
+
+	handle_exception(0x900, NULL, NULL);
+	handle_exception(0x980, NULL, NULL);
+}
+
+struct {
+	const char *name;
+	void (*func)(int argc, char **argv);
+} hctests[] = {
+	{ "tb", test_tb },
+	{ "dec", test_dec },
+	{ "hdec", test_hdec },
+	{ NULL, NULL }
+};
+
+int main(int argc, char **argv)
+{
+	bool all;
+	int i;
+
+	all = argc == 1 || !strcmp(argv[1], "all");
+
+	for (i = 1; i < argc; i++) {
+		if (!strcmp(argv[i], "-w")) {
+			do_migrate = true;
+			if (!all && argc == 2)
+				all = true;
+		}
+	}
+
+	find_dec_bits();
+	dec_max = (1ULL << (dec_bits - 1)) - 1;
+	dec_min = (1ULL << (dec_bits - 1));
+
+	if (machine_is_powernv() && dec_bits > 32) {
+		mtspr(SPR_LPCR, mfspr(SPR_LPCR) | LPCR_LD);
+	}
+
+	report_prefix_push("timebase");
+
+	for (i = 0; hctests[i].name != NULL; i++) {
+		if (all || strcmp(argv[1], hctests[i].name) == 0) {
+			report_prefix_push(hctests[i].name);
+			hctests[i].func(argc, argv);
+			report_prefix_pop();
+		}
+	}
+
+	report_prefix_pop();
+
+	if (machine_is_powernv() && dec_bits > 32) {
+		mtspr(SPR_LPCR, mfspr(SPR_LPCR) & ~LPCR_LD);
+	}
+
+	return report_summary();
+}
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index 9f71ea93..882d48da 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -89,6 +89,14 @@ machine = pseries
 extra_params = -append '-m'
 groups = migration
 
+[timebase]
+file = timebase.elf
+
+[timebase-icount]
+file = timebase.elf
+accel = tcg
+extra_params = -icount shift=0
+
 [h_cede_tm]
 file = tm.elf
 machine = pseries
-- 
2.42.0

Re: [kvm-unit-tests PATCH v5 29/29] powerpc: Add timebase tests

[Thomas Huth]

On 16/12/2023 14.42, Nicholas Piggin wrote:
> This has a known failure on QEMU TCG machines where the decrementer
> interrupt is not lowered when the DEC wraps from -ve to +ve.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   lib/powerpc/asm/ppc_asm.h   |   1 +
>   lib/powerpc/asm/processor.h |  22 +++
>   powerpc/Makefile.common     |   1 +
>   powerpc/smp.c               |  22 ---
>   powerpc/timebase.c          | 328 ++++++++++++++++++++++++++++++++++++
>   powerpc/unittests.cfg       |   8 +
>   6 files changed, 360 insertions(+), 22 deletions(-)
>   create mode 100644 powerpc/timebase.c
...
> diff --git a/powerpc/timebase.c b/powerpc/timebase.c
> new file mode 100644
> index 00000000..4d80ea09
> --- /dev/null
> +++ b/powerpc/timebase.c
> @@ -0,0 +1,328 @@
> +/*
> + * Test Timebase
> + *
> + * Copyright 2017  Thomas Huth, Red Hat Inc.

No, not really. Please update ;-)

  Thomas