[kvm-unit-tests PATCH v4 0/2] powerpc: Test SPR persistency during migration

[kvm-unit-tests PATCH v4 0/2] powerpc: Test SPR persistency during migration

[Thomas Huth]

Here's a KVM-unit-test to check the persistency of SPRs during migration.
First patch adds a helper script for doing migration tests in the
kvm-unit-test framework, and the second patch contains the SPR tester.

v4:
- Fix the error paths in run_migration() - use "exit 2" instead of "exit 1"
  to avoid trouble with the return code magic in the powerpc/run script
  and make sure to terminate the two QEMU instances in case of migration
  failure.

v3:
- Migration shell script code has been moved to a helper function in
  arch-run.bash, so that this now also works with the standalone tests.
- Introduced the helper function migration_cmd there, too, so that other
  architectures can use this, too.
- Use "$@" instead of $* when calling QEMU.

v2:
- Addressed review feedback from v1
- Now using cpu_relax() instead of asm volatile ("nop")

Thomas Huth (2):
  Add the possibility to do simple migration tests
  powerpc: Add Special Purpose Register persistency test

 powerpc/Makefile.common |   3 +-
 powerpc/cstart64.S      |   2 +
 powerpc/run             |   2 +-
 powerpc/sprs.c          | 304 ++++++++++++++++++++++++++++++++++++++++++++++++
 powerpc/unittests.cfg   |   5 +
 scripts/arch-run.bash   |  63 ++++++++++
 scripts/runtime.bash    |   3 +
 7 files changed, 380 insertions(+), 2 deletions(-)
 create mode 100644 powerpc/sprs.c

-- 
1.8.3.1

[kvm-unit-tests PATCH v4 0/2] powerpc: Test SPR persistency during migration

[Thomas Huth]

Here's a KVM-unit-test to check the persistency of SPRs during migration.
First patch adds a helper script for doing migration tests in the
kvm-unit-test framework, and the second patch contains the SPR tester.

v4:
- Fix the error paths in run_migration() - use "exit 2" instead of "exit 1"
  to avoid trouble with the return code magic in the powerpc/run script
  and make sure to terminate the two QEMU instances in case of migration
  failure.

v3:
- Migration shell script code has been moved to a helper function in
  arch-run.bash, so that this now also works with the standalone tests.
- Introduced the helper function migration_cmd there, too, so that other
  architectures can use this, too.
- Use "$@" instead of $* when calling QEMU.

v2:
- Addressed review feedback from v1
- Now using cpu_relax() instead of asm volatile ("nop")

Thomas Huth (2):
  Add the possibility to do simple migration tests
  powerpc: Add Special Purpose Register persistency test

 powerpc/Makefile.common |   3 +-
 powerpc/cstart64.S      |   2 +
 powerpc/run             |   2 +-
 powerpc/sprs.c          | 304 ++++++++++++++++++++++++++++++++++++++++++++++++
 powerpc/unittests.cfg   |   5 +
 scripts/arch-run.bash   |  63 ++++++++++
 scripts/runtime.bash    |   3 +
 7 files changed, 380 insertions(+), 2 deletions(-)
 create mode 100644 powerpc/sprs.c

-- 
1.8.3.1

[kvm-unit-tests PATCH v4 1/2] Add the possibility to do simple migration tests

[Thomas Huth]

To be able to do simple migration tests with kvm-unit-tests, too,
add a helper script that does all the necessary work: Start two
instances of QEMU (source and destination) with QMP sockets for
sending commands to them, then trigger the migration from one
instance to the other and finally signal the end of the migration
to the guest by injecting an NMI.
This helper script is now used automatically for powerpc tests
if the test is put into the "migration" group in the unittests.cfg
file.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 powerpc/run           |  2 +-
 scripts/arch-run.bash | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
 scripts/runtime.bash  |  3 +++
 3 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/powerpc/run b/powerpc/run
index 6269abb..d92608e 100755
--- a/powerpc/run
+++ b/powerpc/run
@@ -46,7 +46,7 @@ M+=",accel=$ACCEL"
 command="$qemu -nodefaults $M -bios $FIRMWARE"
 [ -f "$ENV" ] && command+=" -initrd $ENV"
 command+=" -display none -serial stdio -kernel"
-command="$(timeout_cmd) $command"
+command="$(migration_cmd) $(timeout_cmd) $command"
 echo $command "$@"
 
 # powerpc tests currently exit with rtas-poweroff, which exits with 0.
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 1610f3b..96bd5c4 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -70,3 +70,66 @@ timeout_cmd ()
 		echo "timeout -k 1s --foreground $TIMEOUT"
 	fi
 }
+
+qmp ()
+{
+	echo '{ "execute": "qmp_capabilities" }{ "execute":' "$2" '}' | nc -U $1
+}
+
+run_migration ()
+{
+	if ! command -v nc >/dev/null 2>&1; then
+		echo "$0 needs nc (netcat)" >&2
+		exit 2
+	fi
+
+	qemu=$1
+	shift
+
+	migsock=`mktemp -u -t mig-helper-socket.XXXXXXXXXX`
+	migout1=`mktemp -t mig-helper-stdout1.XXXXXXXXXX`
+	qmp1=`mktemp -u -t mig-helper-qmp1.XXXXXXXXXX`
+	qmp2=`mktemp -u -t mig-helper-qmp2.XXXXXXXXXX`
+	qmpout1=/dev/null
+	qmpout2=/dev/null
+
+	trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2}' EXIT
+
+	$qemu "$@" -chardev socket,id=mon1,path=${qmp1},server,nowait \
+		 -mon chardev=mon1,mode=control | tee ${migout1} &
+
+	$qemu "$@" -chardev socket,id=mon2,path=${qmp2},server,nowait \
+		 -mon chardev=mon2,mode=control -incoming unix:${migsock} &
+
+	# The test must prompt the user to migrate, so wait for the "migrate" keyword
+	while ! grep -q -i "migrate" < ${migout1} ; do
+		sleep 1
+	done
+
+	qmp ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > ${qmpout1}
+
+	# Wait for the migration to complete
+	migstatus=`qmp ${qmp1} '"query-migrate"' | grep return`
+	while ! grep -q '"completed"' <<<"$migstatus" ; do
+		sleep 1
+		migstatus=`qmp ${qmp1} '"query-migrate"' | grep return`
+		if grep -q '"failed"' <<<"$migstatus" ; then
+			echo "ERROR: Migration failed." >&2
+			qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
+			qmp ${qmp2} '"quit"'> ${qmpout2} 2>/dev/null
+			exit 2
+		fi
+	done
+	qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
+
+	qmp ${qmp2} '"inject-nmi"'> ${qmpout2}
+
+	wait
+}
+
+migration_cmd ()
+{
+	if [ "$MIGRATION" = "yes" ]; then
+		echo "run_migration"
+	fi
+}
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 98f1835..e630279 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -99,6 +99,9 @@ function run()
     }
 
     cmdline=$(get_cmdline $kernel)
+    if grep -qw "migration" <<<$groups ; then
+        cmdline="MIGRATION=yes $cmdline"
+    fi
     if [ "$verbose" = "yes" ]; then
         echo $cmdline
     fi
-- 
1.8.3.1

[kvm-unit-tests PATCH v4 1/2] Add the possibility to do simple migration tests

[Thomas Huth]

To be able to do simple migration tests with kvm-unit-tests, too,
add a helper script that does all the necessary work: Start two
instances of QEMU (source and destination) with QMP sockets for
sending commands to them, then trigger the migration from one
instance to the other and finally signal the end of the migration
to the guest by injecting an NMI.
This helper script is now used automatically for powerpc tests
if the test is put into the "migration" group in the unittests.cfg
file.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 powerpc/run           |  2 +-
 scripts/arch-run.bash | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
 scripts/runtime.bash  |  3 +++
 3 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/powerpc/run b/powerpc/run
index 6269abb..d92608e 100755
--- a/powerpc/run
+++ b/powerpc/run
@@ -46,7 +46,7 @@ M+=",accel=$ACCEL"
 command="$qemu -nodefaults $M -bios $FIRMWARE"
 [ -f "$ENV" ] && command+=" -initrd $ENV"
 command+=" -display none -serial stdio -kernel"
-command="$(timeout_cmd) $command"
+command="$(migration_cmd) $(timeout_cmd) $command"
 echo $command "$@"
 
 # powerpc tests currently exit with rtas-poweroff, which exits with 0.
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 1610f3b..96bd5c4 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -70,3 +70,66 @@ timeout_cmd ()
 		echo "timeout -k 1s --foreground $TIMEOUT"
 	fi
 }
+
+qmp ()
+{
+	echo '{ "execute": "qmp_capabilities" }{ "execute":' "$2" '}' | nc -U $1
+}
+
+run_migration ()
+{
+	if ! command -v nc >/dev/null 2>&1; then
+		echo "$0 needs nc (netcat)" >&2
+		exit 2
+	fi
+
+	qemu=$1
+	shift
+
+	migsock=`mktemp -u -t mig-helper-socket.XXXXXXXXXX`
+	migout1=`mktemp -t mig-helper-stdout1.XXXXXXXXXX`
+	qmp1=`mktemp -u -t mig-helper-qmp1.XXXXXXXXXX`
+	qmp2=`mktemp -u -t mig-helper-qmp2.XXXXXXXXXX`
+	qmpout1=/dev/null
+	qmpout2=/dev/null
+
+	trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2}' EXIT
+
+	$qemu "$@" -chardev socket,id=mon1,path=${qmp1},server,nowait \
+		 -mon chardev=mon1,mode=control | tee ${migout1} &
+
+	$qemu "$@" -chardev socket,id=mon2,path=${qmp2},server,nowait \
+		 -mon chardev=mon2,mode=control -incoming unix:${migsock} &
+
+	# The test must prompt the user to migrate, so wait for the "migrate" keyword
+	while ! grep -q -i "migrate" < ${migout1} ; do
+		sleep 1
+	done
+
+	qmp ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > ${qmpout1}
+
+	# Wait for the migration to complete
+	migstatus=`qmp ${qmp1} '"query-migrate"' | grep return`
+	while ! grep -q '"completed"' <<<"$migstatus" ; do
+		sleep 1
+		migstatus=`qmp ${qmp1} '"query-migrate"' | grep return`
+		if grep -q '"failed"' <<<"$migstatus" ; then
+			echo "ERROR: Migration failed." >&2
+			qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
+			qmp ${qmp2} '"quit"'> ${qmpout2} 2>/dev/null
+			exit 2
+		fi
+	done
+	qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
+
+	qmp ${qmp2} '"inject-nmi"'> ${qmpout2}
+
+	wait
+}
+
+migration_cmd ()
+{
+	if [ "$MIGRATION" = "yes" ]; then
+		echo "run_migration"
+	fi
+}
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 98f1835..e630279 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -99,6 +99,9 @@ function run()
     }
 
     cmdline=$(get_cmdline $kernel)
+    if grep -qw "migration" <<<$groups ; then
+        cmdline="MIGRATION=yes $cmdline"
+    fi
     if [ "$verbose" = "yes" ]; then
         echo $cmdline
     fi
-- 
1.8.3.1

[kvm-unit-tests PATCH v4 2/2] powerpc: Add Special Purpose Register persistency test

[Thomas Huth]

This test has two purposes: First, check whether the hypervisor can be
destabilized by writing random values into the SPRs of the PowerPC CPU
(this indeed revealed a bug last year, see CVE-2016-3044).
Second, this test can be used to check whether the SPRs are synchronized
properly between the KVM host CPU and QEMU, e.g. when migrating the VM
from one QEMU instance to another.
The test first fills the various SPRs with some non-zero value, then reads
the values back into a first array. It then either sleeps a short period
of time (for testing without migration, in the hope that we're rescheduled
on another host CPU), or it waits for a key or NMI (with the '-w' option)
so that it is possible to migrate the VM before continuing. The test then
finally reads the values from the SPRs back into another array and then
compares them with the initial values.
Currently the test only supports the SPRs from the PowerISA v2.01
(PowerPC 970) and PowerISA v2.07 specification (i.e. POWER8 CPUs),
but other versions should be pretty easy to add later.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 powerpc/Makefile.common |   3 +-
 powerpc/cstart64.S      |   2 +
 powerpc/sprs.c          | 304 ++++++++++++++++++++++++++++++++++++++++++++++++
 powerpc/unittests.cfg   |   5 +
 4 files changed, 313 insertions(+), 1 deletion(-)
 create mode 100644 powerpc/sprs.c

diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 1869f44..e536ffe 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -9,7 +9,8 @@ tests-common = \
 	$(TEST_DIR)/spapr_hcall.elf \
 	$(TEST_DIR)/rtas.elf \
 	$(TEST_DIR)/emulator.elf \
-	$(TEST_DIR)/tm.elf
+	$(TEST_DIR)/tm.elf \
+	$(TEST_DIR)/sprs.elf
 
 tests-all = $(tests-common) $(tests)
 all: $(TEST_DIR)/boot_rom.bin $(tests-all)
diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
index 2204e3b..ec673b3 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S
@@ -247,6 +247,8 @@ call_handler:
 	.globl __start_interrupts
 __start_interrupts:
 
+VECTOR(0x100)
+VECTOR(0x200)
 VECTOR(0x300)
 VECTOR(0x400)
 VECTOR(0x500)
diff --git a/powerpc/sprs.c b/powerpc/sprs.c
new file mode 100644
index 0000000..9227eff
--- /dev/null
+++ b/powerpc/sprs.c
@@ -0,0 +1,304 @@
+/*
+ * Test Special Purpose Registers
+ *
+ * Copyright 2017  Thomas Huth, Red Hat Inc.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ *
+ * The basic idea of this test is to check whether the contents of the Special
+ * Purpose Registers (SPRs) are preserved correctly during migration. So we
+ * fill in the SPRs with a well-known value, read the values back (since not
+ * all bits might be retained in the SPRs), then wait for a key or NMI (if the
+ * '-w' option has been specified) so that the user has a chance to migrate the
+ * VM. Alternatively, the test can also simply sleep a little bit with the
+ * H_CEDE hypercall, in the hope that we'll get scheduled to another host CPU
+ * and thus register contents might have changed, too (in case of bugs).
+ * Finally, we read back the values from the SPRs and compare them with the
+ * values before the migration. Mismatches are reported as test failures.
+ * Note that we do not test all SPRs since some of the registers change their
+ * content automatically, and some are only accessible with hypervisor privi-
+ * leges or have bad side effects, so we have to omit those registers.
+ */
+#include <libcflat.h>
+#include <util.h>
+#include <alloc.h>
+#include <asm/handlers.h>
+#include <asm/hcall.h>
+#include <asm/processor.h>
+#include <asm/barrier.h>
+
+#define mfspr(nr) ({ \
+	uint64_t ret; \
+	asm volatile("mfspr %0,%1" : "=r"(ret) : "i"(nr)); \
+	ret; \
+})
+
+#define mtspr(nr, val) \
+	asm volatile("mtspr %0,%1" : : "i"(nr), "r"(val))
+
+uint64_t before[1024], after[1024];
+
+volatile int nmi_occurred;
+
+static void nmi_handler(struct pt_regs *regs __unused, void *opaque __unused)
+{
+	nmi_occurred = 1;
+}
+
+static int h_get_term_char(uint64_t termno)
+{
+	register uint64_t r3 asm("r3") = 0x54; /* H_GET_TERM_CHAR */
+	register uint64_t r4 asm("r4") = termno;
+	register uint64_t r5 asm("r5");
+
+	asm volatile (" sc 1 "	: "+r"(r3), "+r"(r4), "=r"(r5)
+				: "r"(r3),  "r"(r4));
+
+	return r3 == H_SUCCESS && r4 > 0 ? r5 >> 48 : 0;
+}
+
+/* Common SPRs for all PowerPC CPUs */
+static void set_sprs_common(uint64_t val)
+{
+	mtspr(9, val);		/* CTR */
+	// mtspr(273, val);	/* SPRG1 */  /* Used by our exception handler */
+	mtspr(274, val);	/* SPRG2 */
+	mtspr(275, val);	/* SPRG3 */
+}
+
+/* SPRs from PowerPC Operating Environment Architecture, Book III, Vers. 2.01 */
+static void set_sprs_book3s_201(uint64_t val)
+{
+	mtspr(18, val);		/* DSISR */
+	mtspr(19, val);		/* DAR */
+	mtspr(152, val);	/* CTRL */
+	mtspr(256, val);	/* VRSAVE */
+	mtspr(786, val);	/* MMCRA */
+	mtspr(795, val);	/* MMCR0 */
+	mtspr(798, val);	/* MMCR1 */
+}
+
+/* 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 void set_sprs(uint64_t val)
+{
+	uint32_t pvr = mfspr(287);	/* Processor Version Register */
+
+	set_sprs_common(val);
+
+	switch (pvr >> 16) {
+	case 0x39:			/* PPC970 */
+	case 0x3C:			/* PPC970FX */
+	case 0x44:			/* PPC970MP */
+		set_sprs_book3s_201(val);
+		break;
+	case 0x4b:			/* POWER8E */
+	case 0x4c:			/* POWER8NVL */
+	case 0x4d:			/* POWER8 */
+		set_sprs_book3s_207(val);
+		break;
+	default:
+		puts("Warning: Unknown processor version!\n");
+	}
+}
+
+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(uint64_t *v)
+{
+	uint32_t pvr = mfspr(287);	/* Processor Version Register */
+
+	get_sprs_common(v);
+
+	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;
+	}
+}
+
+int main(int argc, char **argv)
+{
+	int i;
+	bool pause = false;
+	uint64_t pat = 0xcafefacec0debabeULL;
+	const uint64_t patterns[] = {
+		0xcafefacec0debabeULL, ~0xcafefacec0debabeULL,
+		0xAAAA5555AAAA5555ULL, 0x5555AAAA5555AAAAULL,
+		0x1234567890ABCDEFULL, 0xFEDCBA0987654321ULL,
+		-1ULL,
+	};
+
+	for (i = 1; i < argc; i++) {
+		if (!strcmp(argv[i], "-w")) {
+			pause = true;
+		} else if (!strcmp(argv[i], "-p")) {
+			i += 1;
+			if (i >= argc || *argv[i] < '0'
+			    || *argv[i] >= '0' + ARRAY_SIZE(patterns))
+				report_abort("Error: bad value for -p");
+			pat ^= patterns[*argv[i] - '0'];
+		} else if (!strcmp(argv[i], "-t")) {
+			/* Randomize with timebase register */
+			asm volatile("mftb %0" : "=r"(i));
+			pat ^= i;
+			asm volatile("mftb %0" : "=r"(i));
+			pat ^= ~(uint64_t)i << 32;
+		} else {
+			report_abort("Warning: Unsupported argument: %s",
+			             argv[i]);
+		}
+	}
+
+	printf("Settings SPRs to 0x%lx...\n", pat);
+	set_sprs(pat);
+
+	memset(before, 0, sizeof(before));
+	memset(after, 0, sizeof(after));
+
+	get_sprs(before);
+
+	if (pause) {
+		handle_exception(0x100, &nmi_handler, NULL);
+		puts("Now migrate the VM, then press a key or send NMI...\n");
+		while (!nmi_occurred && h_get_term_char(0) == 0)
+			cpu_relax();
+	} else {
+		puts("Sleeping...\n");
+		handle_exception(0x900, &dec_except_handler, NULL);
+		asm volatile ("mtdec %0" : : "r" (0x3FFFFFFF));
+		hcall(H_CEDE);
+	}
+
+	get_sprs(after);
+
+	puts("Checking SPRs...\n");
+	for (i = 0; i < 1024; i++) {
+		if (before[i] != 0 || after[i] != 0)
+			report("SPR %d:\t0x%016lx <==> 0x%016lx",
+				before[i] == after[i], i, before[i], after[i]);
+	}
+
+	return report_summary();
+}
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index 20dbde6..fb6b70e 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -59,3 +59,8 @@ file = tm.elf
 smp = 2,threads=2
 extra_params = -append "h_cede_tm"
 groups = nodefault,h_cede_tm
+
+[sprs]
+file = sprs.elf
+extra_params = -append '-w'
+groups = migration
-- 
1.8.3.1

[kvm-unit-tests PATCH v4 2/2] powerpc: Add Special Purpose Register persistency test

[Thomas Huth]

This test has two purposes: First, check whether the hypervisor can be
destabilized by writing random values into the SPRs of the PowerPC CPU
(this indeed revealed a bug last year, see CVE-2016-3044).
Second, this test can be used to check whether the SPRs are synchronized
properly between the KVM host CPU and QEMU, e.g. when migrating the VM
from one QEMU instance to another.
The test first fills the various SPRs with some non-zero value, then reads
the values back into a first array. It then either sleeps a short period
of time (for testing without migration, in the hope that we're rescheduled
on another host CPU), or it waits for a key or NMI (with the '-w' option)
so that it is possible to migrate the VM before continuing. The test then
finally reads the values from the SPRs back into another array and then
compares them with the initial values.
Currently the test only supports the SPRs from the PowerISA v2.01
(PowerPC 970) and PowerISA v2.07 specification (i.e. POWER8 CPUs),
but other versions should be pretty easy to add later.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 powerpc/Makefile.common |   3 +-
 powerpc/cstart64.S      |   2 +
 powerpc/sprs.c          | 304 ++++++++++++++++++++++++++++++++++++++++++++++++
 powerpc/unittests.cfg   |   5 +
 4 files changed, 313 insertions(+), 1 deletion(-)
 create mode 100644 powerpc/sprs.c

diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 1869f44..e536ffe 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -9,7 +9,8 @@ tests-common = \
 	$(TEST_DIR)/spapr_hcall.elf \
 	$(TEST_DIR)/rtas.elf \
 	$(TEST_DIR)/emulator.elf \
-	$(TEST_DIR)/tm.elf
+	$(TEST_DIR)/tm.elf \
+	$(TEST_DIR)/sprs.elf
 
 tests-all = $(tests-common) $(tests)
 all: $(TEST_DIR)/boot_rom.bin $(tests-all)
diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
index 2204e3b..ec673b3 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S
@@ -247,6 +247,8 @@ call_handler:
 	.globl __start_interrupts
 __start_interrupts:
 
+VECTOR(0x100)
+VECTOR(0x200)
 VECTOR(0x300)
 VECTOR(0x400)
 VECTOR(0x500)
diff --git a/powerpc/sprs.c b/powerpc/sprs.c
new file mode 100644
index 0000000..9227eff
--- /dev/null
+++ b/powerpc/sprs.c
@@ -0,0 +1,304 @@
+/*
+ * Test Special Purpose Registers
+ *
+ * Copyright 2017  Thomas Huth, Red Hat Inc.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ *
+ * The basic idea of this test is to check whether the contents of the Special
+ * Purpose Registers (SPRs) are preserved correctly during migration. So we
+ * fill in the SPRs with a well-known value, read the values back (since not
+ * all bits might be retained in the SPRs), then wait for a key or NMI (if the
+ * '-w' option has been specified) so that the user has a chance to migrate the
+ * VM. Alternatively, the test can also simply sleep a little bit with the
+ * H_CEDE hypercall, in the hope that we'll get scheduled to another host CPU
+ * and thus register contents might have changed, too (in case of bugs).
+ * Finally, we read back the values from the SPRs and compare them with the
+ * values before the migration. Mismatches are reported as test failures.
+ * Note that we do not test all SPRs since some of the registers change their
+ * content automatically, and some are only accessible with hypervisor privi-
+ * leges or have bad side effects, so we have to omit those registers.
+ */
+#include <libcflat.h>
+#include <util.h>
+#include <alloc.h>
+#include <asm/handlers.h>
+#include <asm/hcall.h>
+#include <asm/processor.h>
+#include <asm/barrier.h>
+
+#define mfspr(nr) ({ \
+	uint64_t ret; \
+	asm volatile("mfspr %0,%1" : "=r"(ret) : "i"(nr)); \
+	ret; \
+})
+
+#define mtspr(nr, val) \
+	asm volatile("mtspr %0,%1" : : "i"(nr), "r"(val))
+
+uint64_t before[1024], after[1024];
+
+volatile int nmi_occurred;
+
+static void nmi_handler(struct pt_regs *regs __unused, void *opaque __unused)
+{
+	nmi_occurred = 1;
+}
+
+static int h_get_term_char(uint64_t termno)
+{
+	register uint64_t r3 asm("r3") = 0x54; /* H_GET_TERM_CHAR */
+	register uint64_t r4 asm("r4") = termno;
+	register uint64_t r5 asm("r5");
+
+	asm volatile (" sc 1 "	: "+r"(r3), "+r"(r4), "=r"(r5)
+				: "r"(r3),  "r"(r4));
+
+	return r3 = H_SUCCESS && r4 > 0 ? r5 >> 48 : 0;
+}
+
+/* Common SPRs for all PowerPC CPUs */
+static void set_sprs_common(uint64_t val)
+{
+	mtspr(9, val);		/* CTR */
+	// mtspr(273, val);	/* SPRG1 */  /* Used by our exception handler */
+	mtspr(274, val);	/* SPRG2 */
+	mtspr(275, val);	/* SPRG3 */
+}
+
+/* SPRs from PowerPC Operating Environment Architecture, Book III, Vers. 2.01 */
+static void set_sprs_book3s_201(uint64_t val)
+{
+	mtspr(18, val);		/* DSISR */
+	mtspr(19, val);		/* DAR */
+	mtspr(152, val);	/* CTRL */
+	mtspr(256, val);	/* VRSAVE */
+	mtspr(786, val);	/* MMCRA */
+	mtspr(795, val);	/* MMCR0 */
+	mtspr(798, val);	/* MMCR1 */
+}
+
+/* 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 void set_sprs(uint64_t val)
+{
+	uint32_t pvr = mfspr(287);	/* Processor Version Register */
+
+	set_sprs_common(val);
+
+	switch (pvr >> 16) {
+	case 0x39:			/* PPC970 */
+	case 0x3C:			/* PPC970FX */
+	case 0x44:			/* PPC970MP */
+		set_sprs_book3s_201(val);
+		break;
+	case 0x4b:			/* POWER8E */
+	case 0x4c:			/* POWER8NVL */
+	case 0x4d:			/* POWER8 */
+		set_sprs_book3s_207(val);
+		break;
+	default:
+		puts("Warning: Unknown processor version!\n");
+	}
+}
+
+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(uint64_t *v)
+{
+	uint32_t pvr = mfspr(287);	/* Processor Version Register */
+
+	get_sprs_common(v);
+
+	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;
+	}
+}
+
+int main(int argc, char **argv)
+{
+	int i;
+	bool pause = false;
+	uint64_t pat = 0xcafefacec0debabeULL;
+	const uint64_t patterns[] = {
+		0xcafefacec0debabeULL, ~0xcafefacec0debabeULL,
+		0xAAAA5555AAAA5555ULL, 0x5555AAAA5555AAAAULL,
+		0x1234567890ABCDEFULL, 0xFEDCBA0987654321ULL,
+		-1ULL,
+	};
+
+	for (i = 1; i < argc; i++) {
+		if (!strcmp(argv[i], "-w")) {
+			pause = true;
+		} else if (!strcmp(argv[i], "-p")) {
+			i += 1;
+			if (i >= argc || *argv[i] < '0'
+			    || *argv[i] >= '0' + ARRAY_SIZE(patterns))
+				report_abort("Error: bad value for -p");
+			pat ^= patterns[*argv[i] - '0'];
+		} else if (!strcmp(argv[i], "-t")) {
+			/* Randomize with timebase register */
+			asm volatile("mftb %0" : "=r"(i));
+			pat ^= i;
+			asm volatile("mftb %0" : "=r"(i));
+			pat ^= ~(uint64_t)i << 32;
+		} else {
+			report_abort("Warning: Unsupported argument: %s",
+			             argv[i]);
+		}
+	}
+
+	printf("Settings SPRs to 0x%lx...\n", pat);
+	set_sprs(pat);
+
+	memset(before, 0, sizeof(before));
+	memset(after, 0, sizeof(after));
+
+	get_sprs(before);
+
+	if (pause) {
+		handle_exception(0x100, &nmi_handler, NULL);
+		puts("Now migrate the VM, then press a key or send NMI...\n");
+		while (!nmi_occurred && h_get_term_char(0) = 0)
+			cpu_relax();
+	} else {
+		puts("Sleeping...\n");
+		handle_exception(0x900, &dec_except_handler, NULL);
+		asm volatile ("mtdec %0" : : "r" (0x3FFFFFFF));
+		hcall(H_CEDE);
+	}
+
+	get_sprs(after);
+
+	puts("Checking SPRs...\n");
+	for (i = 0; i < 1024; i++) {
+		if (before[i] != 0 || after[i] != 0)
+			report("SPR %d:\t0x%016lx <=> 0x%016lx",
+				before[i] = after[i], i, before[i], after[i]);
+	}
+
+	return report_summary();
+}
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index 20dbde6..fb6b70e 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -59,3 +59,8 @@ file = tm.elf
 smp = 2,threads=2
 extra_params = -append "h_cede_tm"
 groups = nodefault,h_cede_tm
+
+[sprs]
+file = sprs.elf
+extra_params = -append '-w'
+groups = migration
-- 
1.8.3.1

Re: [kvm-unit-tests PATCH v4 1/2] Add the possibility to do simple migration tests

[Andrew Jones]

On Wed, Mar 22, 2017 at 09:28:06AM +0100, Thomas Huth wrote:
> To be able to do simple migration tests with kvm-unit-tests, too,
> add a helper script that does all the necessary work: Start two
> instances of QEMU (source and destination) with QMP sockets for
> sending commands to them, then trigger the migration from one
> instance to the other and finally signal the end of the migration
> to the guest by injecting an NMI.
> This helper script is now used automatically for powerpc tests
> if the test is put into the "migration" group in the unittests.cfg
> file.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  powerpc/run           |  2 +-
>  scripts/arch-run.bash | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  scripts/runtime.bash  |  3 +++
>  3 files changed, 67 insertions(+), 1 deletion(-)
> 
> diff --git a/powerpc/run b/powerpc/run
> index 6269abb..d92608e 100755
> --- a/powerpc/run
> +++ b/powerpc/run
> @@ -46,7 +46,7 @@ M+=",accel=$ACCEL"
>  command="$qemu -nodefaults $M -bios $FIRMWARE"
>  [ -f "$ENV" ] && command+=" -initrd $ENV"
>  command+=" -display none -serial stdio -kernel"
> -command="$(timeout_cmd) $command"
> +command="$(migration_cmd) $(timeout_cmd) $command"
>  echo $command "$@"
>  
>  # powerpc tests currently exit with rtas-poweroff, which exits with 0.
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 1610f3b..96bd5c4 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -70,3 +70,66 @@ timeout_cmd ()
>  		echo "timeout -k 1s --foreground $TIMEOUT"
>  	fi
>  }
> +
> +qmp ()
> +{
> +	echo '{ "execute": "qmp_capabilities" }{ "execute":' "$2" '}' | nc -U $1
> +}
> +
> +run_migration ()
> +{
> +	if ! command -v nc >/dev/null 2>&1; then
> +		echo "$0 needs nc (netcat)" >&2
> +		exit 2

I would sort of still prefer $FUNCNAME over $0 here, but I don't feel
that strongly about it. Nice catch on the 'exit 2'. 2 is indeed our
designated error code for run-script failures.

> +	fi
> +
> +	qemu=$1
> +	shift
> +
> +	migsock=`mktemp -u -t mig-helper-socket.XXXXXXXXXX`
> +	migout1=`mktemp -t mig-helper-stdout1.XXXXXXXXXX`
> +	qmp1=`mktemp -u -t mig-helper-qmp1.XXXXXXXXXX`
> +	qmp2=`mktemp -u -t mig-helper-qmp2.XXXXXXXXXX`
> +	qmpout1=/dev/null
> +	qmpout2=/dev/null
> +
> +	trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2}' EXIT
> +
> +	$qemu "$@" -chardev socket,id=mon1,path=${qmp1},server,nowait \
> +		 -mon chardev=mon1,mode=control | tee ${migout1} &
> +
> +	$qemu "$@" -chardev socket,id=mon2,path=${qmp2},server,nowait \
> +		 -mon chardev=mon2,mode=control -incoming unix:${migsock} &
> +
> +	# The test must prompt the user to migrate, so wait for the "migrate" keyword
> +	while ! grep -q -i "migrate" < ${migout1} ; do
> +		sleep 1
> +	done
> +
> +	qmp ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > ${qmpout1}
> +
> +	# Wait for the migration to complete
> +	migstatus=`qmp ${qmp1} '"query-migrate"' | grep return`
> +	while ! grep -q '"completed"' <<<"$migstatus" ; do
> +		sleep 1
> +		migstatus=`qmp ${qmp1} '"query-migrate"' | grep return`
> +		if grep -q '"failed"' <<<"$migstatus" ; then
> +			echo "ERROR: Migration failed." >&2
> +			qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
> +			qmp ${qmp2} '"quit"'> ${qmpout2} 2>/dev/null
> +			exit 2

Also a nice improvement, ensuring the qemu processes quit.

> +		fi
> +	done
> +	qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
> +
> +	qmp ${qmp2} '"inject-nmi"'> ${qmpout2}
> +
> +	wait
> +}
> +
> +migration_cmd ()
> +{
> +	if [ "$MIGRATION" = "yes" ]; then
> +		echo "run_migration"
> +	fi
> +}
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 98f1835..e630279 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -99,6 +99,9 @@ function run()
>      }
>  
>      cmdline=$(get_cmdline $kernel)
> +    if grep -qw "migration" <<<$groups ; then
> +        cmdline="MIGRATION=yes $cmdline"
> +    fi
>      if [ "$verbose" = "yes" ]; then
>          echo $cmdline
>      fi
> -- 
> 1.8.3.1
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [kvm-unit-tests PATCH v4 1/2] Add the possibility to do simple migration tests

[Andrew Jones]

On Wed, Mar 22, 2017 at 09:28:06AM +0100, Thomas Huth wrote:
> To be able to do simple migration tests with kvm-unit-tests, too,
> add a helper script that does all the necessary work: Start two
> instances of QEMU (source and destination) with QMP sockets for
> sending commands to them, then trigger the migration from one
> instance to the other and finally signal the end of the migration
> to the guest by injecting an NMI.
> This helper script is now used automatically for powerpc tests
> if the test is put into the "migration" group in the unittests.cfg
> file.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  powerpc/run           |  2 +-
>  scripts/arch-run.bash | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  scripts/runtime.bash  |  3 +++
>  3 files changed, 67 insertions(+), 1 deletion(-)
> 
> diff --git a/powerpc/run b/powerpc/run
> index 6269abb..d92608e 100755
> --- a/powerpc/run
> +++ b/powerpc/run
> @@ -46,7 +46,7 @@ M+=",accel=$ACCEL"
>  command="$qemu -nodefaults $M -bios $FIRMWARE"
>  [ -f "$ENV" ] && command+=" -initrd $ENV"
>  command+=" -display none -serial stdio -kernel"
> -command="$(timeout_cmd) $command"
> +command="$(migration_cmd) $(timeout_cmd) $command"
>  echo $command "$@"
>  
>  # powerpc tests currently exit with rtas-poweroff, which exits with 0.
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 1610f3b..96bd5c4 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -70,3 +70,66 @@ timeout_cmd ()
>  		echo "timeout -k 1s --foreground $TIMEOUT"
>  	fi
>  }
> +
> +qmp ()
> +{
> +	echo '{ "execute": "qmp_capabilities" }{ "execute":' "$2" '}' | nc -U $1
> +}
> +
> +run_migration ()
> +{
> +	if ! command -v nc >/dev/null 2>&1; then
> +		echo "$0 needs nc (netcat)" >&2
> +		exit 2

I would sort of still prefer $FUNCNAME over $0 here, but I don't feel
that strongly about it. Nice catch on the 'exit 2'. 2 is indeed our
designated error code for run-script failures.

> +	fi
> +
> +	qemu=$1
> +	shift
> +
> +	migsock=`mktemp -u -t mig-helper-socket.XXXXXXXXXX`
> +	migout1=`mktemp -t mig-helper-stdout1.XXXXXXXXXX`
> +	qmp1=`mktemp -u -t mig-helper-qmp1.XXXXXXXXXX`
> +	qmp2=`mktemp -u -t mig-helper-qmp2.XXXXXXXXXX`
> +	qmpout1=/dev/null
> +	qmpout2=/dev/null
> +
> +	trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2}' EXIT
> +
> +	$qemu "$@" -chardev socket,id=mon1,path=${qmp1},server,nowait \
> +		 -mon chardev=mon1,mode=control | tee ${migout1} &
> +
> +	$qemu "$@" -chardev socket,id=mon2,path=${qmp2},server,nowait \
> +		 -mon chardev=mon2,mode=control -incoming unix:${migsock} &
> +
> +	# The test must prompt the user to migrate, so wait for the "migrate" keyword
> +	while ! grep -q -i "migrate" < ${migout1} ; do
> +		sleep 1
> +	done
> +
> +	qmp ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > ${qmpout1}
> +
> +	# Wait for the migration to complete
> +	migstatus=`qmp ${qmp1} '"query-migrate"' | grep return`
> +	while ! grep -q '"completed"' <<<"$migstatus" ; do
> +		sleep 1
> +		migstatus=`qmp ${qmp1} '"query-migrate"' | grep return`
> +		if grep -q '"failed"' <<<"$migstatus" ; then
> +			echo "ERROR: Migration failed." >&2
> +			qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
> +			qmp ${qmp2} '"quit"'> ${qmpout2} 2>/dev/null
> +			exit 2

Also a nice improvement, ensuring the qemu processes quit.

> +		fi
> +	done
> +	qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
> +
> +	qmp ${qmp2} '"inject-nmi"'> ${qmpout2}
> +
> +	wait
> +}
> +
> +migration_cmd ()
> +{
> +	if [ "$MIGRATION" = "yes" ]; then
> +		echo "run_migration"
> +	fi
> +}
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 98f1835..e630279 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -99,6 +99,9 @@ function run()
>      }
>  
>      cmdline=$(get_cmdline $kernel)
> +    if grep -qw "migration" <<<$groups ; then
> +        cmdline="MIGRATION=yes $cmdline"
> +    fi
>      if [ "$verbose" = "yes" ]; then
>          echo $cmdline
>      fi
> -- 
> 1.8.3.1
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [kvm-unit-tests PATCH v4 0/2] powerpc: Test SPR persistency during migration

[Laurent Vivier]

On 22/03/2017 09:28, Thomas Huth wrote:
> Here's a KVM-unit-test to check the persistency of SPRs during migration.
> First patch adds a helper script for doing migration tests in the
> kvm-unit-test framework, and the second patch contains the SPR tester.
> 
> v4:
> - Fix the error paths in run_migration() - use "exit 2" instead of "exit 1"
>   to avoid trouble with the return code magic in the powerpc/run script
>   and make sure to terminate the two QEMU instances in case of migration
>   failure.
> 
> v3:
> - Migration shell script code has been moved to a helper function in
>   arch-run.bash, so that this now also works with the standalone tests.
> - Introduced the helper function migration_cmd there, too, so that other
>   architectures can use this, too.
> - Use "$@" instead of $* when calling QEMU.
> 
> v2:
> - Addressed review feedback from v1
> - Now using cpu_relax() instead of asm volatile ("nop")
> 
> Thomas Huth (2):
>   Add the possibility to do simple migration tests
>   powerpc: Add Special Purpose Register persistency test
> 
>  powerpc/Makefile.common |   3 +-
>  powerpc/cstart64.S      |   2 +
>  powerpc/run             |   2 +-
>  powerpc/sprs.c          | 304 ++++++++++++++++++++++++++++++++++++++++++++++++
>  powerpc/unittests.cfg   |   5 +
>  scripts/arch-run.bash   |  63 ++++++++++
>  scripts/runtime.bash    |   3 +
>  7 files changed, 380 insertions(+), 2 deletions(-)
>  create mode 100644 powerpc/sprs.c
> 

Really nice work.

I've tested with TCG and KVM HV and it works fine.

It hangs with KVM PR, but I have an "emulation at f60 failed (00000000)"
in kernel logs, did you see that?

Laurent

Re: [kvm-unit-tests PATCH v4 0/2] powerpc: Test SPR persistency during migration

[Laurent Vivier]

On 22/03/2017 09:28, Thomas Huth wrote:
> Here's a KVM-unit-test to check the persistency of SPRs during migration.
> First patch adds a helper script for doing migration tests in the
> kvm-unit-test framework, and the second patch contains the SPR tester.
> 
> v4:
> - Fix the error paths in run_migration() - use "exit 2" instead of "exit 1"
>   to avoid trouble with the return code magic in the powerpc/run script
>   and make sure to terminate the two QEMU instances in case of migration
>   failure.
> 
> v3:
> - Migration shell script code has been moved to a helper function in
>   arch-run.bash, so that this now also works with the standalone tests.
> - Introduced the helper function migration_cmd there, too, so that other
>   architectures can use this, too.
> - Use "$@" instead of $* when calling QEMU.
> 
> v2:
> - Addressed review feedback from v1
> - Now using cpu_relax() instead of asm volatile ("nop")
> 
> Thomas Huth (2):
>   Add the possibility to do simple migration tests
>   powerpc: Add Special Purpose Register persistency test
> 
>  powerpc/Makefile.common |   3 +-
>  powerpc/cstart64.S      |   2 +
>  powerpc/run             |   2 +-
>  powerpc/sprs.c          | 304 ++++++++++++++++++++++++++++++++++++++++++++++++
>  powerpc/unittests.cfg   |   5 +
>  scripts/arch-run.bash   |  63 ++++++++++
>  scripts/runtime.bash    |   3 +
>  7 files changed, 380 insertions(+), 2 deletions(-)
>  create mode 100644 powerpc/sprs.c
> 

Really nice work.

I've tested with TCG and KVM HV and it works fine.

It hangs with KVM PR, but I have an "emulation at f60 failed (00000000)"
in kernel logs, did you see that?

Laurent

Re: [kvm-unit-tests PATCH v4 0/2] powerpc: Test SPR persistency during migration

[Thomas Huth]

On 22.03.2017 16:20, Laurent Vivier wrote:
> On 22/03/2017 09:28, Thomas Huth wrote:
>> Here's a KVM-unit-test to check the persistency of SPRs during migration.
>> First patch adds a helper script for doing migration tests in the
>> kvm-unit-test framework, and the second patch contains the SPR tester.
>>
>> v4:
>> - Fix the error paths in run_migration() - use "exit 2" instead of "exit 1"
>>   to avoid trouble with the return code magic in the powerpc/run script
>>   and make sure to terminate the two QEMU instances in case of migration
>>   failure.
>>
>> v3:
>> - Migration shell script code has been moved to a helper function in
>>   arch-run.bash, so that this now also works with the standalone tests.
>> - Introduced the helper function migration_cmd there, too, so that other
>>   architectures can use this, too.
>> - Use "$@" instead of $* when calling QEMU.
>>
>> v2:
>> - Addressed review feedback from v1
>> - Now using cpu_relax() instead of asm volatile ("nop")
>>
>> Thomas Huth (2):
>>   Add the possibility to do simple migration tests
>>   powerpc: Add Special Purpose Register persistency test
>>
>>  powerpc/Makefile.common |   3 +-
>>  powerpc/cstart64.S      |   2 +
>>  powerpc/run             |   2 +-
>>  powerpc/sprs.c          | 304 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  powerpc/unittests.cfg   |   5 +
>>  scripts/arch-run.bash   |  63 ++++++++++
>>  scripts/runtime.bash    |   3 +
>>  7 files changed, 380 insertions(+), 2 deletions(-)
>>  create mode 100644 powerpc/sprs.c
>>
> 
> Really nice work.
> 
> I've tested with TCG and KVM HV and it works fine.
> 
> It hangs with KVM PR, but I have an "emulation at f60 failed (00000000)"
> in kernel logs, did you see that?

Yes, it's because KVM-PR does not support some of the SPRs yet and
apparently always injects a program exception in that case. So there is
something left to do for KVM-PR here...

I think KVM-PR should not inject a program interrupt here, since
according to the PowerISA (v2.07), the mtspr/mfspr should be treated as
no-op for unknown SPRs when the CPU is not running in PRoblem state...

 Thomas

Re: [kvm-unit-tests PATCH v4 0/2] powerpc: Test SPR persistency during migration

[Thomas Huth]

On 22.03.2017 16:20, Laurent Vivier wrote:
> On 22/03/2017 09:28, Thomas Huth wrote:
>> Here's a KVM-unit-test to check the persistency of SPRs during migration.
>> First patch adds a helper script for doing migration tests in the
>> kvm-unit-test framework, and the second patch contains the SPR tester.
>>
>> v4:
>> - Fix the error paths in run_migration() - use "exit 2" instead of "exit 1"
>>   to avoid trouble with the return code magic in the powerpc/run script
>>   and make sure to terminate the two QEMU instances in case of migration
>>   failure.
>>
>> v3:
>> - Migration shell script code has been moved to a helper function in
>>   arch-run.bash, so that this now also works with the standalone tests.
>> - Introduced the helper function migration_cmd there, too, so that other
>>   architectures can use this, too.
>> - Use "$@" instead of $* when calling QEMU.
>>
>> v2:
>> - Addressed review feedback from v1
>> - Now using cpu_relax() instead of asm volatile ("nop")
>>
>> Thomas Huth (2):
>>   Add the possibility to do simple migration tests
>>   powerpc: Add Special Purpose Register persistency test
>>
>>  powerpc/Makefile.common |   3 +-
>>  powerpc/cstart64.S      |   2 +
>>  powerpc/run             |   2 +-
>>  powerpc/sprs.c          | 304 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  powerpc/unittests.cfg   |   5 +
>>  scripts/arch-run.bash   |  63 ++++++++++
>>  scripts/runtime.bash    |   3 +
>>  7 files changed, 380 insertions(+), 2 deletions(-)
>>  create mode 100644 powerpc/sprs.c
>>
> 
> Really nice work.
> 
> I've tested with TCG and KVM HV and it works fine.
> 
> It hangs with KVM PR, but I have an "emulation at f60 failed (00000000)"
> in kernel logs, did you see that?

Yes, it's because KVM-PR does not support some of the SPRs yet and
apparently always injects a program exception in that case. So there is
something left to do for KVM-PR here...

I think KVM-PR should not inject a program interrupt here, since
according to the PowerISA (v2.07), the mtspr/mfspr should be treated as
no-op for unknown SPRs when the CPU is not running in PRoblem state...

 Thomas

Re: [kvm-unit-tests PATCH v4 0/2] powerpc: Test SPR persistency during migration

[Cédric Le Goater]

On 03/22/2017 04:20 PM, Laurent Vivier wrote:
> On 22/03/2017 09:28, Thomas Huth wrote:
>> Here's a KVM-unit-test to check the persistency of SPRs during migration.
>> First patch adds a helper script for doing migration tests in the
>> kvm-unit-test framework, and the second patch contains the SPR tester.
>>
>> v4:
>> - Fix the error paths in run_migration() - use "exit 2" instead of "exit 1"
>>   to avoid trouble with the return code magic in the powerpc/run script
>>   and make sure to terminate the two QEMU instances in case of migration
>>   failure.
>>
>> v3:
>> - Migration shell script code has been moved to a helper function in
>>   arch-run.bash, so that this now also works with the standalone tests.
>> - Introduced the helper function migration_cmd there, too, so that other
>>   architectures can use this, too.
>> - Use "$@" instead of $* when calling QEMU.
>>
>> v2:
>> - Addressed review feedback from v1
>> - Now using cpu_relax() instead of asm volatile ("nop")
>>
>> Thomas Huth (2):
>>   Add the possibility to do simple migration tests
>>   powerpc: Add Special Purpose Register persistency test
>>
>>  powerpc/Makefile.common |   3 +-
>>  powerpc/cstart64.S      |   2 +
>>  powerpc/run             |   2 +-
>>  powerpc/sprs.c          | 304 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  powerpc/unittests.cfg   |   5 +
>>  scripts/arch-run.bash   |  63 ++++++++++
>>  scripts/runtime.bash    |   3 +
>>  7 files changed, 380 insertions(+), 2 deletions(-)
>>  create mode 100644 powerpc/sprs.c
>>
> 
> Really nice work.

Yes. 

I hope to have some time to hack a little your script to use 
a minimal image with a guest agent and do a ping to check that 
migration has succeeded. That would increase the scope of what 
is being migrated. Good for regression tests, specially when
we are modifying the interrupt or mmu models.   

Thanks,

C. 

> I've tested with TCG and KVM HV and it works fine.
> 
> It hangs with KVM PR, but I have an "emulation at f60 failed (00000000)"
> in kernel logs, did you see that?
> 
> Laurent
> 

Re: [kvm-unit-tests PATCH v4 0/2] powerpc: Test SPR persistency during migration

[Cédric Le Goater]

On 03/22/2017 04:20 PM, Laurent Vivier wrote:
> On 22/03/2017 09:28, Thomas Huth wrote:
>> Here's a KVM-unit-test to check the persistency of SPRs during migration.
>> First patch adds a helper script for doing migration tests in the
>> kvm-unit-test framework, and the second patch contains the SPR tester.
>>
>> v4:
>> - Fix the error paths in run_migration() - use "exit 2" instead of "exit 1"
>>   to avoid trouble with the return code magic in the powerpc/run script
>>   and make sure to terminate the two QEMU instances in case of migration
>>   failure.
>>
>> v3:
>> - Migration shell script code has been moved to a helper function in
>>   arch-run.bash, so that this now also works with the standalone tests.
>> - Introduced the helper function migration_cmd there, too, so that other
>>   architectures can use this, too.
>> - Use "$@" instead of $* when calling QEMU.
>>
>> v2:
>> - Addressed review feedback from v1
>> - Now using cpu_relax() instead of asm volatile ("nop")
>>
>> Thomas Huth (2):
>>   Add the possibility to do simple migration tests
>>   powerpc: Add Special Purpose Register persistency test
>>
>>  powerpc/Makefile.common |   3 +-
>>  powerpc/cstart64.S      |   2 +
>>  powerpc/run             |   2 +-
>>  powerpc/sprs.c          | 304 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  powerpc/unittests.cfg   |   5 +
>>  scripts/arch-run.bash   |  63 ++++++++++
>>  scripts/runtime.bash    |   3 +
>>  7 files changed, 380 insertions(+), 2 deletions(-)
>>  create mode 100644 powerpc/sprs.c
>>
> 
> Really nice work.

Yes. 

I hope to have some time to hack a little your script to use 
a minimal image with a guest agent and do a ping to check that 
migration has succeeded. That would increase the scope of what 
is being migrated. Good for regression tests, specially when
we are modifying the interrupt or mmu models.   

Thanks,

C. 

> I've tested with TCG and KVM HV and it works fine.
> 
> It hangs with KVM PR, but I have an "emulation at f60 failed (00000000)"
> in kernel logs, did you see that?
> 
> Laurent
> 

Re: [kvm-unit-tests PATCH v4 0/2] powerpc: Test SPR persistency during migration

[Paul Mackerras]

On Wed, Mar 22, 2017 at 06:27:20PM +0100, Thomas Huth wrote:
> On 22.03.2017 16:20, Laurent Vivier wrote:
> > On 22/03/2017 09:28, Thomas Huth wrote:
> >> Here's a KVM-unit-test to check the persistency of SPRs during migration.
> >> First patch adds a helper script for doing migration tests in the
> >> kvm-unit-test framework, and the second patch contains the SPR tester.
> >>
> >> v4:
> >> - Fix the error paths in run_migration() - use "exit 2" instead of "exit 1"
> >>   to avoid trouble with the return code magic in the powerpc/run script
> >>   and make sure to terminate the two QEMU instances in case of migration
> >>   failure.
> >>
> >> v3:
> >> - Migration shell script code has been moved to a helper function in
> >>   arch-run.bash, so that this now also works with the standalone tests.
> >> - Introduced the helper function migration_cmd there, too, so that other
> >>   architectures can use this, too.
> >> - Use "$@" instead of $* when calling QEMU.
> >>
> >> v2:
> >> - Addressed review feedback from v1
> >> - Now using cpu_relax() instead of asm volatile ("nop")
> >>
> >> Thomas Huth (2):
> >>   Add the possibility to do simple migration tests
> >>   powerpc: Add Special Purpose Register persistency test
> >>
> >>  powerpc/Makefile.common |   3 +-
> >>  powerpc/cstart64.S      |   2 +
> >>  powerpc/run             |   2 +-
> >>  powerpc/sprs.c          | 304 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>  powerpc/unittests.cfg   |   5 +
> >>  scripts/arch-run.bash   |  63 ++++++++++
> >>  scripts/runtime.bash    |   3 +
> >>  7 files changed, 380 insertions(+), 2 deletions(-)
> >>  create mode 100644 powerpc/sprs.c
> >>
> > 
> > Really nice work.
> > 
> > I've tested with TCG and KVM HV and it works fine.
> > 
> > It hangs with KVM PR, but I have an "emulation at f60 failed (00000000)"
> > in kernel logs, did you see that?
> 
> Yes, it's because KVM-PR does not support some of the SPRs yet and
> apparently always injects a program exception in that case. So there is
> something left to do for KVM-PR here...
> 
> I think KVM-PR should not inject a program interrupt here, since
> according to the PowerISA (v2.07), the mtspr/mfspr should be treated as
> no-op for unknown SPRs when the CPU is not running in PRoblem state...

I agree, let's fix it.

I did try to fix it some time ago, but at that stage Alex Graf didn't
want to change the behaviour (I think because he thought the existing
behaviour was better for debugging).  However, we could have the
architecturally correct behaviour but still have a way to log the
unimplemented SPR accesses for debugging.

Paul.

Re: [kvm-unit-tests PATCH v4 0/2] powerpc: Test SPR persistency during migration

[Paul Mackerras]

On Wed, Mar 22, 2017 at 06:27:20PM +0100, Thomas Huth wrote:
> On 22.03.2017 16:20, Laurent Vivier wrote:
> > On 22/03/2017 09:28, Thomas Huth wrote:
> >> Here's a KVM-unit-test to check the persistency of SPRs during migration.
> >> First patch adds a helper script for doing migration tests in the
> >> kvm-unit-test framework, and the second patch contains the SPR tester.
> >>
> >> v4:
> >> - Fix the error paths in run_migration() - use "exit 2" instead of "exit 1"
> >>   to avoid trouble with the return code magic in the powerpc/run script
> >>   and make sure to terminate the two QEMU instances in case of migration
> >>   failure.
> >>
> >> v3:
> >> - Migration shell script code has been moved to a helper function in
> >>   arch-run.bash, so that this now also works with the standalone tests.
> >> - Introduced the helper function migration_cmd there, too, so that other
> >>   architectures can use this, too.
> >> - Use "$@" instead of $* when calling QEMU.
> >>
> >> v2:
> >> - Addressed review feedback from v1
> >> - Now using cpu_relax() instead of asm volatile ("nop")
> >>
> >> Thomas Huth (2):
> >>   Add the possibility to do simple migration tests
> >>   powerpc: Add Special Purpose Register persistency test
> >>
> >>  powerpc/Makefile.common |   3 +-
> >>  powerpc/cstart64.S      |   2 +
> >>  powerpc/run             |   2 +-
> >>  powerpc/sprs.c          | 304 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>  powerpc/unittests.cfg   |   5 +
> >>  scripts/arch-run.bash   |  63 ++++++++++
> >>  scripts/runtime.bash    |   3 +
> >>  7 files changed, 380 insertions(+), 2 deletions(-)
> >>  create mode 100644 powerpc/sprs.c
> >>
> > 
> > Really nice work.
> > 
> > I've tested with TCG and KVM HV and it works fine.
> > 
> > It hangs with KVM PR, but I have an "emulation at f60 failed (00000000)"
> > in kernel logs, did you see that?
> 
> Yes, it's because KVM-PR does not support some of the SPRs yet and
> apparently always injects a program exception in that case. So there is
> something left to do for KVM-PR here...
> 
> I think KVM-PR should not inject a program interrupt here, since
> according to the PowerISA (v2.07), the mtspr/mfspr should be treated as
> no-op for unknown SPRs when the CPU is not running in PRoblem state...

I agree, let's fix it.

I did try to fix it some time ago, but at that stage Alex Graf didn't
want to change the behaviour (I think because he thought the existing
behaviour was better for debugging).  However, we could have the
architecturally correct behaviour but still have a way to log the
unimplemented SPR accesses for debugging.

Paul.

Re: [kvm-unit-tests PATCH v4 0/2] powerpc: Test SPR persistency during migration

[Thomas Huth]

On 23.03.2017 23:41, Paul Mackerras wrote:
> On Wed, Mar 22, 2017 at 06:27:20PM +0100, Thomas Huth wrote:
>> On 22.03.2017 16:20, Laurent Vivier wrote:
>>> On 22/03/2017 09:28, Thomas Huth wrote:
>>>> Here's a KVM-unit-test to check the persistency of SPRs during migration.
>>>> First patch adds a helper script for doing migration tests in the
>>>> kvm-unit-test framework, and the second patch contains the SPR tester.
>>>>
>>>> v4:
>>>> - Fix the error paths in run_migration() - use "exit 2" instead of "exit 1"
>>>>   to avoid trouble with the return code magic in the powerpc/run script
>>>>   and make sure to terminate the two QEMU instances in case of migration
>>>>   failure.
>>>>
>>>> v3:
>>>> - Migration shell script code has been moved to a helper function in
>>>>   arch-run.bash, so that this now also works with the standalone tests.
>>>> - Introduced the helper function migration_cmd there, too, so that other
>>>>   architectures can use this, too.
>>>> - Use "$@" instead of $* when calling QEMU.
>>>>
>>>> v2:
>>>> - Addressed review feedback from v1
>>>> - Now using cpu_relax() instead of asm volatile ("nop")
>>>>
>>>> Thomas Huth (2):
>>>>   Add the possibility to do simple migration tests
>>>>   powerpc: Add Special Purpose Register persistency test
>>>>
>>>>  powerpc/Makefile.common |   3 +-
>>>>  powerpc/cstart64.S      |   2 +
>>>>  powerpc/run             |   2 +-
>>>>  powerpc/sprs.c          | 304 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  powerpc/unittests.cfg   |   5 +
>>>>  scripts/arch-run.bash   |  63 ++++++++++
>>>>  scripts/runtime.bash    |   3 +
>>>>  7 files changed, 380 insertions(+), 2 deletions(-)
>>>>  create mode 100644 powerpc/sprs.c
>>>>
>>>
>>> Really nice work.
>>>
>>> I've tested with TCG and KVM HV and it works fine.
>>>
>>> It hangs with KVM PR, but I have an "emulation at f60 failed (00000000)"
>>> in kernel logs, did you see that?
>>
>> Yes, it's because KVM-PR does not support some of the SPRs yet and
>> apparently always injects a program exception in that case. So there is
>> something left to do for KVM-PR here...
>>
>> I think KVM-PR should not inject a program interrupt here, since
>> according to the PowerISA (v2.07), the mtspr/mfspr should be treated as
>> no-op for unknown SPRs when the CPU is not running in PRoblem state...
> 
> I agree, let's fix it.
> 
> I did try to fix it some time ago, but at that stage Alex Graf didn't
> want to change the behaviour (I think because he thought the existing
> behaviour was better for debugging).  However, we could have the
> architecturally correct behaviour but still have a way to log the
> unimplemented SPR accesses for debugging.

Right, it should be enough for debugging to have a way to log this.
Could you dig out your patch, or shall I try to come up with a new one?

 Thomas

Re: [kvm-unit-tests PATCH v4 0/2] powerpc: Test SPR persistency during migration

[Thomas Huth]

On 23.03.2017 23:41, Paul Mackerras wrote:
> On Wed, Mar 22, 2017 at 06:27:20PM +0100, Thomas Huth wrote:
>> On 22.03.2017 16:20, Laurent Vivier wrote:
>>> On 22/03/2017 09:28, Thomas Huth wrote:
>>>> Here's a KVM-unit-test to check the persistency of SPRs during migration.
>>>> First patch adds a helper script for doing migration tests in the
>>>> kvm-unit-test framework, and the second patch contains the SPR tester.
>>>>
>>>> v4:
>>>> - Fix the error paths in run_migration() - use "exit 2" instead of "exit 1"
>>>>   to avoid trouble with the return code magic in the powerpc/run script
>>>>   and make sure to terminate the two QEMU instances in case of migration
>>>>   failure.
>>>>
>>>> v3:
>>>> - Migration shell script code has been moved to a helper function in
>>>>   arch-run.bash, so that this now also works with the standalone tests.
>>>> - Introduced the helper function migration_cmd there, too, so that other
>>>>   architectures can use this, too.
>>>> - Use "$@" instead of $* when calling QEMU.
>>>>
>>>> v2:
>>>> - Addressed review feedback from v1
>>>> - Now using cpu_relax() instead of asm volatile ("nop")
>>>>
>>>> Thomas Huth (2):
>>>>   Add the possibility to do simple migration tests
>>>>   powerpc: Add Special Purpose Register persistency test
>>>>
>>>>  powerpc/Makefile.common |   3 +-
>>>>  powerpc/cstart64.S      |   2 +
>>>>  powerpc/run             |   2 +-
>>>>  powerpc/sprs.c          | 304 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  powerpc/unittests.cfg   |   5 +
>>>>  scripts/arch-run.bash   |  63 ++++++++++
>>>>  scripts/runtime.bash    |   3 +
>>>>  7 files changed, 380 insertions(+), 2 deletions(-)
>>>>  create mode 100644 powerpc/sprs.c
>>>>
>>>
>>> Really nice work.
>>>
>>> I've tested with TCG and KVM HV and it works fine.
>>>
>>> It hangs with KVM PR, but I have an "emulation at f60 failed (00000000)"
>>> in kernel logs, did you see that?
>>
>> Yes, it's because KVM-PR does not support some of the SPRs yet and
>> apparently always injects a program exception in that case. So there is
>> something left to do for KVM-PR here...
>>
>> I think KVM-PR should not inject a program interrupt here, since
>> according to the PowerISA (v2.07), the mtspr/mfspr should be treated as
>> no-op for unknown SPRs when the CPU is not running in PRoblem state...
> 
> I agree, let's fix it.
> 
> I did try to fix it some time ago, but at that stage Alex Graf didn't
> want to change the behaviour (I think because he thought the existing
> behaviour was better for debugging).  However, we could have the
> architecturally correct behaviour but still have a way to log the
> unimplemented SPR accesses for debugging.

Right, it should be enough for debugging to have a way to log this.
Could you dig out your patch, or shall I try to come up with a new one?

 Thomas