* [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
@ 2016-04-08 11:35 ` Thomas Huth
0 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2016-04-08 11:35 UTC (permalink / raw)
To: kvm; +Cc: kvm-ppc, pbonzini, lvivier, clg, dgibson, drjones
This test can be used to check whether the SPR (special purpose
registers) of the PowerPC CPU are migrated right. It first fills
the various SPRs with some non-zero value, then reads the values
back into a first array, then waits for a key (with the '-w' option)
so that it is possible to migrate the VM, and finally reads the
values from the SPRs back into another array and then compares it
with the initial values.
Currently the test only supports the SPRs from the 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 | 5 +-
powerpc/sprs.c | 230 ++++++++++++++++++++++++++++++++++++++++++++++++
powerpc/unittests.cfg | 4 +
3 files changed, 238 insertions(+), 1 deletion(-)
create mode 100644 powerpc/sprs.c
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 4449aec..43b2e49 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -8,7 +8,8 @@ tests-common = \
$(TEST_DIR)/selftest.elf \
$(TEST_DIR)/spapr_hcall.elf \
$(TEST_DIR)/rtas.elf \
- $(TEST_DIR)/emulator.elf
+ $(TEST_DIR)/emulator.elf \
+ $(TEST_DIR)/sprs.elf
all: $(TEST_DIR)/boot_rom.bin test_cases
@@ -77,3 +78,5 @@ $(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
$(TEST_DIR)/rtas.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/rtas.o
$(TEST_DIR)/emulator.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/emulator.o
+
+$(TEST_DIR)/sprs.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/sprs.o
diff --git a/powerpc/sprs.c b/powerpc/sprs.c
new file mode 100644
index 0000000..e99501f
--- /dev/null
+++ b/powerpc/sprs.c
@@ -0,0 +1,230 @@
+/*
+ * Test SPRs
+ *
+ * Copyright 2016 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 (if the '-w'
+ * option has been specified) so that the user has a chance to migrate the VM,
+ * and after a key has been pressed, we read back the values again and compare
+ * them with the values before the migration, reporting errors for mismatches.
+ * 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, so we have to omit those registers.
+ */
+#include <libcflat.h>
+#include <util.h>
+#include <alloc.h>
+#include <asm/hcall.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];
+
+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(1, val); /* XER */
+ mtspr(9, val); /* CTR */
+ mtspr(273, val); /* SPRG1 */
+ mtspr(274, val); /* SPRG2 */
+ mtspr(275, val); /* SPRG3 */
+}
+
+/* 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 */
+ mtspr(153, val); /* FSCR */
+ mtspr(157, val); /* UAMOR */
+ mtspr(159, val); /* PSPB */
+ mtspr(256, val); /* VRSAVE */
+ mtspr(272, val); /* SPRG0 */
+ mtspr(512, val); /* SPEFSCR */
+ 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); /* 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); /* MMCR0 */
+ mtspr(796, val); /* SIAR */
+ mtspr(798, val); /* SDAR */
+ 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 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[1] = mfspr(1); /* XER */
+ v[9] = mfspr(9); /* CTR */
+ v[273] = mfspr(273); /* SPRG1 */
+ v[274] = mfspr(274); /* SPRG2 */
+ v[275] = mfspr(275); /* SPRG3 */
+}
+
+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 */
+ 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 */
+ v[512] = mfspr(512); /* SPEFSCR */
+ 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[798] = mfspr(798); /* SDAR */
+ 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 0x4b: /* POWER8E */
+ case 0x4c: /* POWER8NVL */
+ case 0x4d: /* POWER8 */
+ get_sprs_book3s_207(v);
+ break;
+ }
+}
+
+int main(int argc, char **argv)
+{
+ int i;
+
+ if (argc > 1)
+ report_abort("Warning: Unsupported arguments!");
+
+ puts("Settings SPRs...\n");
+ set_sprs(0xcafefacec0debabeULL);
+
+ memset(before, 0, sizeof(before));
+ memset(after, 0, sizeof(after));
+
+ get_sprs(before);
+
+ if (argc > 0 && !strcmp(argv[0], "-w")) {
+ puts("Now migrate the VM, then press a key...\n");
+ while (h_get_term_char(0) = 0)
+ ;
+ } else {
+ puts("Parameter '-w' not specified - not waiting for a key.\n");
+ }
+
+ 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 ed4fdbe..5563cbe 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -50,3 +50,7 @@ groups = rtas
[emulator]
file = emulator.elf
+
+[sprs]
+file = sprs.elf
+#extra_params = -append '-w'
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
2016-04-08 11:35 ` Thomas Huth
@ 2016-04-08 12:14 ` Andrew Jones
-1 siblings, 0 replies; 19+ messages in thread
From: Andrew Jones @ 2016-04-08 12:14 UTC (permalink / raw)
To: Thomas Huth; +Cc: kvm, kvm-ppc, pbonzini, lvivier, clg, dgibson
On Fri, Apr 08, 2016 at 01:35:29PM +0200, Thomas Huth wrote:
> This test can be used to check whether the SPR (special purpose
> registers) of the PowerPC CPU are migrated right. It first fills
> the various SPRs with some non-zero value, then reads the values
> back into a first array, then waits for a key (with the '-w' option)
> so that it is possible to migrate the VM, and finally reads the
> values from the SPRs back into another array and then compares it
> with the initial values.
> Currently the test only supports the SPRs from the 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 | 5 +-
> powerpc/sprs.c | 230 ++++++++++++++++++++++++++++++++++++++++++++++++
> powerpc/unittests.cfg | 4 +
> 3 files changed, 238 insertions(+), 1 deletion(-)
> create mode 100644 powerpc/sprs.c
>
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 4449aec..43b2e49 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -8,7 +8,8 @@ tests-common = \
> $(TEST_DIR)/selftest.elf \
> $(TEST_DIR)/spapr_hcall.elf \
> $(TEST_DIR)/rtas.elf \
> - $(TEST_DIR)/emulator.elf
> + $(TEST_DIR)/emulator.elf \
> + $(TEST_DIR)/sprs.elf
>
> all: $(TEST_DIR)/boot_rom.bin test_cases
>
> @@ -77,3 +78,5 @@ $(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
> $(TEST_DIR)/rtas.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/rtas.o
>
> $(TEST_DIR)/emulator.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/emulator.o
> +
> +$(TEST_DIR)/sprs.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/sprs.o
> diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> new file mode 100644
> index 0000000..e99501f
> --- /dev/null
> +++ b/powerpc/sprs.c
> @@ -0,0 +1,230 @@
> +/*
> + * Test SPRs
> + *
> + * Copyright 2016 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 (if the '-w'
> + * option has been specified) so that the user has a chance to migrate the VM,
> + * and after a key has been pressed, we read back the values again and compare
> + * them with the values before the migration, reporting errors for mismatches.
> + * 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, so we have to omit those registers.
> + */
> +#include <libcflat.h>
> +#include <util.h>
> +#include <alloc.h>
> +#include <asm/hcall.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));
You may want these defines in processor.h to share with other tests.
> +
> +uint64_t before[1024], after[1024];
> +
> +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;
> +}
You may want this in hcall.h to share with other tests.
> +
> +/* Common SPRs for all PowerPC CPUs */
> +static void set_sprs_common(uint64_t val)
> +{
> + mtspr(1, val); /* XER */
> + mtspr(9, val); /* CTR */
> + mtspr(273, val); /* SPRG1 */
> + mtspr(274, val); /* SPRG2 */
> + mtspr(275, val); /* SPRG3 */
> +}
> +
> +/* 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 */
> + mtspr(153, val); /* FSCR */
> + mtspr(157, val); /* UAMOR */
> + mtspr(159, val); /* PSPB */
> + mtspr(256, val); /* VRSAVE */
> + mtspr(272, val); /* SPRG0 */
> + mtspr(512, val); /* SPEFSCR */
> + 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); /* 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); /* MMCR0 */
> + mtspr(796, val); /* SIAR */
> + mtspr(798, val); /* SDAR */
> + 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 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[1] = mfspr(1); /* XER */
> + v[9] = mfspr(9); /* CTR */
> + v[273] = mfspr(273); /* SPRG1 */
> + v[274] = mfspr(274); /* SPRG2 */
> + v[275] = mfspr(275); /* SPRG3 */
> +}
> +
> +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 */
> + 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 */
> + v[512] = mfspr(512); /* SPEFSCR */
> + 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[798] = mfspr(798); /* SDAR */
> + 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 0x4b: /* POWER8E */
> + case 0x4c: /* POWER8NVL */
> + case 0x4d: /* POWER8 */
> + get_sprs_book3s_207(v);
> + break;
> + }
> +}
> +
> +int main(int argc, char **argv)
> +{
> + int i;
> +
> + if (argc > 1)
> + report_abort("Warning: Unsupported arguments!");
> +
> + puts("Settings SPRs...\n");
> + set_sprs(0xcafefacec0debabeULL);
> +
> + memset(before, 0, sizeof(before));
> + memset(after, 0, sizeof(after));
before and after shouldn't need the memsets, as they're in the bss.
> +
> + get_sprs(before);
> +
> + if (argc > 0 && !strcmp(argv[0], "-w")) {
> + puts("Now migrate the VM, then press a key...\n");
> + while (h_get_term_char(0) == 0)
> + ;
> + } else {
> + puts("Parameter '-w' not specified - not waiting for a key.\n");
> + }
> +
> + 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 ed4fdbe..5563cbe 100644
> --- a/powerpc/unittests.cfg
> +++ b/powerpc/unittests.cfg
> @@ -50,3 +50,7 @@ groups = rtas
>
> [emulator]
> file = emulator.elf
> +
> +[sprs]
> +file = sprs.elf
> +#extra_params = -append '-w'
> --
> 1.8.3.1
>
This is an interesting use of kvm-unit-tests. Ideally we'd keep avoid
the key press, which could complicate an automated test. Above you say
some SPRs change their content automatically. Are any of those changes
something that can be predicted in the case of a migration? I.e. could
we monitor them to see if a migration occurred?
Thanks,
drew
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
@ 2016-04-08 12:14 ` Andrew Jones
0 siblings, 0 replies; 19+ messages in thread
From: Andrew Jones @ 2016-04-08 12:14 UTC (permalink / raw)
To: Thomas Huth; +Cc: kvm, kvm-ppc, pbonzini, lvivier, clg, dgibson
On Fri, Apr 08, 2016 at 01:35:29PM +0200, Thomas Huth wrote:
> This test can be used to check whether the SPR (special purpose
> registers) of the PowerPC CPU are migrated right. It first fills
> the various SPRs with some non-zero value, then reads the values
> back into a first array, then waits for a key (with the '-w' option)
> so that it is possible to migrate the VM, and finally reads the
> values from the SPRs back into another array and then compares it
> with the initial values.
> Currently the test only supports the SPRs from the 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 | 5 +-
> powerpc/sprs.c | 230 ++++++++++++++++++++++++++++++++++++++++++++++++
> powerpc/unittests.cfg | 4 +
> 3 files changed, 238 insertions(+), 1 deletion(-)
> create mode 100644 powerpc/sprs.c
>
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 4449aec..43b2e49 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -8,7 +8,8 @@ tests-common = \
> $(TEST_DIR)/selftest.elf \
> $(TEST_DIR)/spapr_hcall.elf \
> $(TEST_DIR)/rtas.elf \
> - $(TEST_DIR)/emulator.elf
> + $(TEST_DIR)/emulator.elf \
> + $(TEST_DIR)/sprs.elf
>
> all: $(TEST_DIR)/boot_rom.bin test_cases
>
> @@ -77,3 +78,5 @@ $(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
> $(TEST_DIR)/rtas.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/rtas.o
>
> $(TEST_DIR)/emulator.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/emulator.o
> +
> +$(TEST_DIR)/sprs.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/sprs.o
> diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> new file mode 100644
> index 0000000..e99501f
> --- /dev/null
> +++ b/powerpc/sprs.c
> @@ -0,0 +1,230 @@
> +/*
> + * Test SPRs
> + *
> + * Copyright 2016 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 (if the '-w'
> + * option has been specified) so that the user has a chance to migrate the VM,
> + * and after a key has been pressed, we read back the values again and compare
> + * them with the values before the migration, reporting errors for mismatches.
> + * 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, so we have to omit those registers.
> + */
> +#include <libcflat.h>
> +#include <util.h>
> +#include <alloc.h>
> +#include <asm/hcall.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));
You may want these defines in processor.h to share with other tests.
> +
> +uint64_t before[1024], after[1024];
> +
> +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;
> +}
You may want this in hcall.h to share with other tests.
> +
> +/* Common SPRs for all PowerPC CPUs */
> +static void set_sprs_common(uint64_t val)
> +{
> + mtspr(1, val); /* XER */
> + mtspr(9, val); /* CTR */
> + mtspr(273, val); /* SPRG1 */
> + mtspr(274, val); /* SPRG2 */
> + mtspr(275, val); /* SPRG3 */
> +}
> +
> +/* 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 */
> + mtspr(153, val); /* FSCR */
> + mtspr(157, val); /* UAMOR */
> + mtspr(159, val); /* PSPB */
> + mtspr(256, val); /* VRSAVE */
> + mtspr(272, val); /* SPRG0 */
> + mtspr(512, val); /* SPEFSCR */
> + 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); /* 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); /* MMCR0 */
> + mtspr(796, val); /* SIAR */
> + mtspr(798, val); /* SDAR */
> + 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 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[1] = mfspr(1); /* XER */
> + v[9] = mfspr(9); /* CTR */
> + v[273] = mfspr(273); /* SPRG1 */
> + v[274] = mfspr(274); /* SPRG2 */
> + v[275] = mfspr(275); /* SPRG3 */
> +}
> +
> +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 */
> + 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 */
> + v[512] = mfspr(512); /* SPEFSCR */
> + 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[798] = mfspr(798); /* SDAR */
> + 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 0x4b: /* POWER8E */
> + case 0x4c: /* POWER8NVL */
> + case 0x4d: /* POWER8 */
> + get_sprs_book3s_207(v);
> + break;
> + }
> +}
> +
> +int main(int argc, char **argv)
> +{
> + int i;
> +
> + if (argc > 1)
> + report_abort("Warning: Unsupported arguments!");
> +
> + puts("Settings SPRs...\n");
> + set_sprs(0xcafefacec0debabeULL);
> +
> + memset(before, 0, sizeof(before));
> + memset(after, 0, sizeof(after));
before and after shouldn't need the memsets, as they're in the bss.
> +
> + get_sprs(before);
> +
> + if (argc > 0 && !strcmp(argv[0], "-w")) {
> + puts("Now migrate the VM, then press a key...\n");
> + while (h_get_term_char(0) = 0)
> + ;
> + } else {
> + puts("Parameter '-w' not specified - not waiting for a key.\n");
> + }
> +
> + 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 ed4fdbe..5563cbe 100644
> --- a/powerpc/unittests.cfg
> +++ b/powerpc/unittests.cfg
> @@ -50,3 +50,7 @@ groups = rtas
>
> [emulator]
> file = emulator.elf
> +
> +[sprs]
> +file = sprs.elf
> +#extra_params = -append '-w'
> --
> 1.8.3.1
>
This is an interesting use of kvm-unit-tests. Ideally we'd keep avoid
the key press, which could complicate an automated test. Above you say
some SPRs change their content automatically. Are any of those changes
something that can be predicted in the case of a migration? I.e. could
we monitor them to see if a migration occurred?
Thanks,
drew
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
2016-04-08 12:14 ` Andrew Jones
@ 2016-04-08 14:08 ` Thomas Huth
-1 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2016-04-08 14:08 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, kvm-ppc, pbonzini, lvivier, clg, dgibson
On 08.04.2016 14:14, Andrew Jones wrote:
> On Fri, Apr 08, 2016 at 01:35:29PM +0200, Thomas Huth wrote:
>> This test can be used to check whether the SPR (special purpose
>> registers) of the PowerPC CPU are migrated right. It first fills
>> the various SPRs with some non-zero value, then reads the values
>> back into a first array, then waits for a key (with the '-w' option)
>> so that it is possible to migrate the VM, and finally reads the
>> values from the SPRs back into another array and then compares it
>> with the initial values.
>> Currently the test only supports the SPRs from the 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 | 5 +-
>> powerpc/sprs.c | 230 ++++++++++++++++++++++++++++++++++++++++++++++++
>> powerpc/unittests.cfg | 4 +
>> 3 files changed, 238 insertions(+), 1 deletion(-)
>> create mode 100644 powerpc/sprs.c
[...]
>> diff --git a/powerpc/sprs.c b/powerpc/sprs.c
>> new file mode 100644
>> index 0000000..e99501f
>> --- /dev/null
>> +++ b/powerpc/sprs.c
>> @@ -0,0 +1,230 @@
>> +/*
>> + * Test SPRs
>> + *
>> + * Copyright 2016 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 (if the '-w'
>> + * option has been specified) so that the user has a chance to migrate the VM,
>> + * and after a key has been pressed, we read back the values again and compare
>> + * them with the values before the migration, reporting errors for mismatches.
>> + * 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, so we have to omit those registers.
>> + */
>> +#include <libcflat.h>
>> +#include <util.h>
>> +#include <alloc.h>
>> +#include <asm/hcall.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));
>
> You may want these defines in processor.h to share with other tests.
Ok, mfspr() is already used in spapr_hcall.c, too, so this is a good idea.
>> +
>> +uint64_t before[1024], after[1024];
>> +
>> +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;
>> +}
>
> You may want this in hcall.h to share with other tests.
Not sure whether it's of any use to another test, since they normally
run automatically, without user interaction. It's just this test that
needs to wait a little bit for the end of migration ... so I'd rather
keep it here until we have another test that needs it.
[...]
>> +int main(int argc, char **argv)
>> +{
>> + int i;
>> +
>> + if (argc > 1)
>> + report_abort("Warning: Unsupported arguments!");
>> +
>> + puts("Settings SPRs...\n");
>> + set_sprs(0xcafefacec0debabeULL);
>> +
>> + memset(before, 0, sizeof(before));
>> + memset(after, 0, sizeof(after));
>
> before and after shouldn't need the memsets, as they're in the bss.
Ok.
>> +
>> + get_sprs(before);
>> +
>> + if (argc > 0 && !strcmp(argv[0], "-w")) {
>> + puts("Now migrate the VM, then press a key...\n");
>> + while (h_get_term_char(0) == 0)
>> + ;
>> + } else {
>> + puts("Parameter '-w' not specified - not waiting for a key.\n");
>> + }
>> +
>> + 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();
>> +}
[...]
> This is an interesting use of kvm-unit-tests. Ideally we'd keep avoid
> the key press, which could complicate an automated test. Above you say
> some SPRs change their content automatically. Are any of those changes
> something that can be predicted in the case of a migration? I.e. could
> we monitor them to see if a migration occurred?
I don't think that it is very easy to detect the end of the migration
like this... the registers I was talking about are things like the
decrementer or timebase registers.
If we really want to automate this, we could maybe use something like
the machine check interrupt instead (which can be triggered with the
"nmi" HMP command in QEMU, I think) to let the guest know about the end
of the migration. But all that would also require quite some additional
logic in the runner script, I think. Not sure whether it's worth the
effort just for this one small test, which can also be run manually from
time to time... But if you think we should really go this way, let me
know, then I can have a try...
Thomas
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
@ 2016-04-08 14:08 ` Thomas Huth
0 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2016-04-08 14:08 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, kvm-ppc, pbonzini, lvivier, clg, dgibson
On 08.04.2016 14:14, Andrew Jones wrote:
> On Fri, Apr 08, 2016 at 01:35:29PM +0200, Thomas Huth wrote:
>> This test can be used to check whether the SPR (special purpose
>> registers) of the PowerPC CPU are migrated right. It first fills
>> the various SPRs with some non-zero value, then reads the values
>> back into a first array, then waits for a key (with the '-w' option)
>> so that it is possible to migrate the VM, and finally reads the
>> values from the SPRs back into another array and then compares it
>> with the initial values.
>> Currently the test only supports the SPRs from the 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 | 5 +-
>> powerpc/sprs.c | 230 ++++++++++++++++++++++++++++++++++++++++++++++++
>> powerpc/unittests.cfg | 4 +
>> 3 files changed, 238 insertions(+), 1 deletion(-)
>> create mode 100644 powerpc/sprs.c
[...]
>> diff --git a/powerpc/sprs.c b/powerpc/sprs.c
>> new file mode 100644
>> index 0000000..e99501f
>> --- /dev/null
>> +++ b/powerpc/sprs.c
>> @@ -0,0 +1,230 @@
>> +/*
>> + * Test SPRs
>> + *
>> + * Copyright 2016 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 (if the '-w'
>> + * option has been specified) so that the user has a chance to migrate the VM,
>> + * and after a key has been pressed, we read back the values again and compare
>> + * them with the values before the migration, reporting errors for mismatches.
>> + * 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, so we have to omit those registers.
>> + */
>> +#include <libcflat.h>
>> +#include <util.h>
>> +#include <alloc.h>
>> +#include <asm/hcall.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));
>
> You may want these defines in processor.h to share with other tests.
Ok, mfspr() is already used in spapr_hcall.c, too, so this is a good idea.
>> +
>> +uint64_t before[1024], after[1024];
>> +
>> +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;
>> +}
>
> You may want this in hcall.h to share with other tests.
Not sure whether it's of any use to another test, since they normally
run automatically, without user interaction. It's just this test that
needs to wait a little bit for the end of migration ... so I'd rather
keep it here until we have another test that needs it.
[...]
>> +int main(int argc, char **argv)
>> +{
>> + int i;
>> +
>> + if (argc > 1)
>> + report_abort("Warning: Unsupported arguments!");
>> +
>> + puts("Settings SPRs...\n");
>> + set_sprs(0xcafefacec0debabeULL);
>> +
>> + memset(before, 0, sizeof(before));
>> + memset(after, 0, sizeof(after));
>
> before and after shouldn't need the memsets, as they're in the bss.
Ok.
>> +
>> + get_sprs(before);
>> +
>> + if (argc > 0 && !strcmp(argv[0], "-w")) {
>> + puts("Now migrate the VM, then press a key...\n");
>> + while (h_get_term_char(0) = 0)
>> + ;
>> + } else {
>> + puts("Parameter '-w' not specified - not waiting for a key.\n");
>> + }
>> +
>> + 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();
>> +}
[...]
> This is an interesting use of kvm-unit-tests. Ideally we'd keep avoid
> the key press, which could complicate an automated test. Above you say
> some SPRs change their content automatically. Are any of those changes
> something that can be predicted in the case of a migration? I.e. could
> we monitor them to see if a migration occurred?
I don't think that it is very easy to detect the end of the migration
like this... the registers I was talking about are things like the
decrementer or timebase registers.
If we really want to automate this, we could maybe use something like
the machine check interrupt instead (which can be triggered with the
"nmi" HMP command in QEMU, I think) to let the guest know about the end
of the migration. But all that would also require quite some additional
logic in the runner script, I think. Not sure whether it's worth the
effort just for this one small test, which can also be run manually from
time to time... But if you think we should really go this way, let me
know, then I can have a try...
Thomas
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
2016-04-08 14:08 ` Thomas Huth
@ 2016-04-08 15:20 ` Andrew Jones
-1 siblings, 0 replies; 19+ messages in thread
From: Andrew Jones @ 2016-04-08 15:20 UTC (permalink / raw)
To: Thomas Huth; +Cc: kvm, kvm-ppc, pbonzini, lvivier, clg, dgibson
On Fri, Apr 08, 2016 at 04:08:41PM +0200, Thomas Huth wrote:
> > This is an interesting use of kvm-unit-tests. Ideally we'd keep avoid
> > the key press, which could complicate an automated test. Above you say
> > some SPRs change their content automatically. Are any of those changes
> > something that can be predicted in the case of a migration? I.e. could
> > we monitor them to see if a migration occurred?
>
> I don't think that it is very easy to detect the end of the migration
> like this... the registers I was talking about are things like the
> decrementer or timebase registers.
>
> If we really want to automate this, we could maybe use something like
> the machine check interrupt instead (which can be triggered with the
> "nmi" HMP command in QEMU, I think) to let the guest know about the end
> of the migration. But all that would also require quite some additional
> logic in the runner script, I think. Not sure whether it's worth the
> effort just for this one small test, which can also be run manually from
> time to time... But if you think we should really go this way, let me
> know, then I can have a try...
I'm OK with time-to-time manual tests. I wonder if we should add
a run_tests.sh cmdline switch that enables all manual tests somehow.
This would allow us to run the complete collection of manual tests
easily, not forgetting any.
drew
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
@ 2016-04-08 15:20 ` Andrew Jones
0 siblings, 0 replies; 19+ messages in thread
From: Andrew Jones @ 2016-04-08 15:20 UTC (permalink / raw)
To: Thomas Huth; +Cc: kvm, kvm-ppc, pbonzini, lvivier, clg, dgibson
On Fri, Apr 08, 2016 at 04:08:41PM +0200, Thomas Huth wrote:
> > This is an interesting use of kvm-unit-tests. Ideally we'd keep avoid
> > the key press, which could complicate an automated test. Above you say
> > some SPRs change their content automatically. Are any of those changes
> > something that can be predicted in the case of a migration? I.e. could
> > we monitor them to see if a migration occurred?
>
> I don't think that it is very easy to detect the end of the migration
> like this... the registers I was talking about are things like the
> decrementer or timebase registers.
>
> If we really want to automate this, we could maybe use something like
> the machine check interrupt instead (which can be triggered with the
> "nmi" HMP command in QEMU, I think) to let the guest know about the end
> of the migration. But all that would also require quite some additional
> logic in the runner script, I think. Not sure whether it's worth the
> effort just for this one small test, which can also be run manually from
> time to time... But if you think we should really go this way, let me
> know, then I can have a try...
I'm OK with time-to-time manual tests. I wonder if we should add
a run_tests.sh cmdline switch that enables all manual tests somehow.
This would allow us to run the complete collection of manual tests
easily, not forgetting any.
drew
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
2016-04-08 11:35 ` Thomas Huth
@ 2016-04-11 1:55 ` David Gibson
-1 siblings, 0 replies; 19+ messages in thread
From: David Gibson @ 2016-04-11 1:55 UTC (permalink / raw)
To: Thomas Huth; +Cc: kvm, kvm-ppc, pbonzini, lvivier, clg, drjones
[-- Attachment #1: Type: text/plain, Size: 3378 bytes --]
On Fri, 8 Apr 2016 13:35:29 +0200
Thomas Huth <thuth@redhat.com> wrote:
> This test can be used to check whether the SPR (special purpose
> registers) of the PowerPC CPU are migrated right. It first fills
> the various SPRs with some non-zero value, then reads the values
> back into a first array, then waits for a key (with the '-w' option)
> so that it is possible to migrate the VM, and finally reads the
> values from the SPRs back into another array and then compares it
> with the initial values.
> Currently the test only supports the SPRs from the 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>
So, the main concern I have about this is that while writing an
arbitrary value is ok for quite a few SPRs, there's a significant
number where that's not safe: either because it could actually cause
some sort of exception interrupting the test, or because the value will
get masked or otherwise modifier on write.
I think at the very least we should separate out the code dealing with
the safe-to-write-anything SPRs from the others as a form of internal
documentation. Then we probably want some sort of table of safe
non-default values for the other SPRs.
[snip]
> +/* Common SPRs for all PowerPC CPUs */
> +static void set_sprs_common(uint64_t val)
> +{
> + mtspr(1, val); /* XER */
> + mtspr(9, val); /* CTR */
> + mtspr(273, val); /* SPRG1 */
> + mtspr(274, val); /* SPRG2 */
> + mtspr(275, val); /* SPRG3 */
> +}
> +
> +/* 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 */
AMR seems to be listed twice..
> + mtspr(61, val); /* IAMR */
> + mtspr(152, val); /* CTRL */
> + mtspr(153, val); /* FSCR */
> + mtspr(157, val); /* UAMOR */
> + mtspr(159, val); /* PSPB */
> + mtspr(256, val); /* VRSAVE */
> + mtspr(272, val); /* SPRG0 */
> + mtspr(512, val); /* SPEFSCR */
> + 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); /* 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); /* MMCR0 */
> + mtspr(796, val); /* SIAR */
> + mtspr(798, val); /* SDAR */
> + 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 */
> +}
At a glance SPRs above where writing an arbitrary value might not be
safe include AMR, IAMR, UAMOR, and MMCR*.
--
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
@ 2016-04-11 1:55 ` David Gibson
0 siblings, 0 replies; 19+ messages in thread
From: David Gibson @ 2016-04-11 1:55 UTC (permalink / raw)
To: Thomas Huth; +Cc: kvm, kvm-ppc, pbonzini, lvivier, clg, drjones
[-- Attachment #1: Type: text/plain, Size: 3378 bytes --]
On Fri, 8 Apr 2016 13:35:29 +0200
Thomas Huth <thuth@redhat.com> wrote:
> This test can be used to check whether the SPR (special purpose
> registers) of the PowerPC CPU are migrated right. It first fills
> the various SPRs with some non-zero value, then reads the values
> back into a first array, then waits for a key (with the '-w' option)
> so that it is possible to migrate the VM, and finally reads the
> values from the SPRs back into another array and then compares it
> with the initial values.
> Currently the test only supports the SPRs from the 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>
So, the main concern I have about this is that while writing an
arbitrary value is ok for quite a few SPRs, there's a significant
number where that's not safe: either because it could actually cause
some sort of exception interrupting the test, or because the value will
get masked or otherwise modifier on write.
I think at the very least we should separate out the code dealing with
the safe-to-write-anything SPRs from the others as a form of internal
documentation. Then we probably want some sort of table of safe
non-default values for the other SPRs.
[snip]
> +/* Common SPRs for all PowerPC CPUs */
> +static void set_sprs_common(uint64_t val)
> +{
> + mtspr(1, val); /* XER */
> + mtspr(9, val); /* CTR */
> + mtspr(273, val); /* SPRG1 */
> + mtspr(274, val); /* SPRG2 */
> + mtspr(275, val); /* SPRG3 */
> +}
> +
> +/* 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 */
AMR seems to be listed twice..
> + mtspr(61, val); /* IAMR */
> + mtspr(152, val); /* CTRL */
> + mtspr(153, val); /* FSCR */
> + mtspr(157, val); /* UAMOR */
> + mtspr(159, val); /* PSPB */
> + mtspr(256, val); /* VRSAVE */
> + mtspr(272, val); /* SPRG0 */
> + mtspr(512, val); /* SPEFSCR */
> + 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); /* 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); /* MMCR0 */
> + mtspr(796, val); /* SIAR */
> + mtspr(798, val); /* SDAR */
> + 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 */
> +}
At a glance SPRs above where writing an arbitrary value might not be
safe include AMR, IAMR, UAMOR, and MMCR*.
--
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
2016-04-11 1:55 ` David Gibson
@ 2016-04-11 9:23 ` Thomas Huth
-1 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2016-04-11 9:23 UTC (permalink / raw)
To: David Gibson; +Cc: kvm, kvm-ppc, pbonzini, lvivier, clg, drjones
[-- Attachment #1: Type: text/plain, Size: 4039 bytes --]
On 11.04.2016 03:55, David Gibson wrote:
> On Fri, 8 Apr 2016 13:35:29 +0200
> Thomas Huth <thuth@redhat.com> wrote:
>
>> This test can be used to check whether the SPR (special purpose
>> registers) of the PowerPC CPU are migrated right. It first fills
>> the various SPRs with some non-zero value, then reads the values
>> back into a first array, then waits for a key (with the '-w' option)
>> so that it is possible to migrate the VM, and finally reads the
>> values from the SPRs back into another array and then compares it
>> with the initial values.
>> Currently the test only supports the SPRs from the 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>
>
> So, the main concern I have about this is that while writing an
> arbitrary value is ok for quite a few SPRs, there's a significant
> number where that's not safe: either because it could actually cause
> some sort of exception interrupting the test, or because the value will
> get masked or otherwise modifier on write.
If a write to an SPR caused any exception or hang of the guest, I did
not include it in the list of SPRs to be tested here. Sorry, I should
have mentioned that somewhere in the commit message or comment of the
sprs.c file below.
Anyway, working out a detailed list of values that should be written
into each of the SPRs might get quite difficult/cumbersome since some of
these SPRs are not that well documented in the PowerISA. So I'd like to
keep the generic approach here instead. Anyway, if a write causes
trouble, it's not a big issue to take that SPR out of the list again.
And since the kvm-unit-tests are also a very isolated test (i.e. we're
not running a Linux kernel here which might expect certain values in
certain SPRs), it's IMHO also ok to mess up the state in the SPRs since
the VM will get destroyed after the test anyway.
About your second concern, that the values will get masked or modified:
Yes, you're right, of course! But that's not a problem here since I read
back the values from the SPRs before the migration - I do not use the
original value that I wrote into the SPRs for comparison!
> [snip]
>> +/* Common SPRs for all PowerPC CPUs */
>> +static void set_sprs_common(uint64_t val)
>> +{
>> + mtspr(1, val); /* XER */
>> + mtspr(9, val); /* CTR */
>> + mtspr(273, val); /* SPRG1 */
>> + mtspr(274, val); /* SPRG2 */
>> + mtspr(275, val); /* SPRG3 */
>> +}
>> +
>> +/* 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 */
>
> AMR seems to be listed twice..
That's because AMR is available via both SPR numbers - one time for
userspace mode (which can be disabled by the kernel if desired), and one
time for privileged mode.
> At a glance SPRs above where writing an arbitrary value might not be
> safe include AMR, IAMR, UAMOR, and MMCR*.
Well, you should not think of this test as a nice, behaving guest kernel
that only puts valid values into safe SPRs ... rather think of it as a
stress test for the host with a bad, misbehaving guest kernel.
So think what's the worst thing that could happen? Host crash? Right,
this test already helped to find one of those bugs:
http://www.spinics.net/lists/kvm/msg128989.html
... but then we've of course got to fix the host kernel (or QEMU), not
this test.
The other bad thing that could of course happen is a guest crash - but
then, as mentioned above, we can simply adjust the list of SPRs that we
test as soon as we see such a crash. With the current list, I do not get
any guest crashes - at least not with kvm-hv. ... but kvm-pr and tcg are
two other candidates that likely need some fixing for this test.
Thomas
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
@ 2016-04-11 9:23 ` Thomas Huth
0 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2016-04-11 9:23 UTC (permalink / raw)
To: David Gibson; +Cc: kvm, kvm-ppc, pbonzini, lvivier, clg, drjones
[-- Attachment #1: Type: text/plain, Size: 4039 bytes --]
On 11.04.2016 03:55, David Gibson wrote:
> On Fri, 8 Apr 2016 13:35:29 +0200
> Thomas Huth <thuth@redhat.com> wrote:
>
>> This test can be used to check whether the SPR (special purpose
>> registers) of the PowerPC CPU are migrated right. It first fills
>> the various SPRs with some non-zero value, then reads the values
>> back into a first array, then waits for a key (with the '-w' option)
>> so that it is possible to migrate the VM, and finally reads the
>> values from the SPRs back into another array and then compares it
>> with the initial values.
>> Currently the test only supports the SPRs from the 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>
>
> So, the main concern I have about this is that while writing an
> arbitrary value is ok for quite a few SPRs, there's a significant
> number where that's not safe: either because it could actually cause
> some sort of exception interrupting the test, or because the value will
> get masked or otherwise modifier on write.
If a write to an SPR caused any exception or hang of the guest, I did
not include it in the list of SPRs to be tested here. Sorry, I should
have mentioned that somewhere in the commit message or comment of the
sprs.c file below.
Anyway, working out a detailed list of values that should be written
into each of the SPRs might get quite difficult/cumbersome since some of
these SPRs are not that well documented in the PowerISA. So I'd like to
keep the generic approach here instead. Anyway, if a write causes
trouble, it's not a big issue to take that SPR out of the list again.
And since the kvm-unit-tests are also a very isolated test (i.e. we're
not running a Linux kernel here which might expect certain values in
certain SPRs), it's IMHO also ok to mess up the state in the SPRs since
the VM will get destroyed after the test anyway.
About your second concern, that the values will get masked or modified:
Yes, you're right, of course! But that's not a problem here since I read
back the values from the SPRs before the migration - I do not use the
original value that I wrote into the SPRs for comparison!
> [snip]
>> +/* Common SPRs for all PowerPC CPUs */
>> +static void set_sprs_common(uint64_t val)
>> +{
>> + mtspr(1, val); /* XER */
>> + mtspr(9, val); /* CTR */
>> + mtspr(273, val); /* SPRG1 */
>> + mtspr(274, val); /* SPRG2 */
>> + mtspr(275, val); /* SPRG3 */
>> +}
>> +
>> +/* 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 */
>
> AMR seems to be listed twice..
That's because AMR is available via both SPR numbers - one time for
userspace mode (which can be disabled by the kernel if desired), and one
time for privileged mode.
> At a glance SPRs above where writing an arbitrary value might not be
> safe include AMR, IAMR, UAMOR, and MMCR*.
Well, you should not think of this test as a nice, behaving guest kernel
that only puts valid values into safe SPRs ... rather think of it as a
stress test for the host with a bad, misbehaving guest kernel.
So think what's the worst thing that could happen? Host crash? Right,
this test already helped to find one of those bugs:
http://www.spinics.net/lists/kvm/msg128989.html
... but then we've of course got to fix the host kernel (or QEMU), not
this test.
The other bad thing that could of course happen is a guest crash - but
then, as mentioned above, we can simply adjust the list of SPRs that we
test as soon as we see such a crash. With the current list, I do not get
any guest crashes - at least not with kvm-hv. ... but kvm-pr and tcg are
two other candidates that likely need some fixing for this test.
Thomas
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
2016-04-11 9:23 ` Thomas Huth
@ 2016-04-12 1:21 ` David Gibson
-1 siblings, 0 replies; 19+ messages in thread
From: David Gibson @ 2016-04-12 1:21 UTC (permalink / raw)
To: Thomas Huth; +Cc: kvm, kvm-ppc, pbonzini, lvivier, clg, drjones
[-- Attachment #1: Type: text/plain, Size: 5157 bytes --]
On Mon, 11 Apr 2016 11:23:46 +0200
Thomas Huth <thuth@redhat.com> wrote:
> On 11.04.2016 03:55, David Gibson wrote:
> > On Fri, 8 Apr 2016 13:35:29 +0200
> > Thomas Huth <thuth@redhat.com> wrote:
> >
> >> This test can be used to check whether the SPR (special purpose
> >> registers) of the PowerPC CPU are migrated right. It first fills
> >> the various SPRs with some non-zero value, then reads the values
> >> back into a first array, then waits for a key (with the '-w' option)
> >> so that it is possible to migrate the VM, and finally reads the
> >> values from the SPRs back into another array and then compares it
> >> with the initial values.
> >> Currently the test only supports the SPRs from the 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>
> >
> > So, the main concern I have about this is that while writing an
> > arbitrary value is ok for quite a few SPRs, there's a significant
> > number where that's not safe: either because it could actually cause
> > some sort of exception interrupting the test, or because the value will
> > get masked or otherwise modifier on write.
>
> If a write to an SPR caused any exception or hang of the guest, I did
> not include it in the list of SPRs to be tested here. Sorry, I should
> have mentioned that somewhere in the commit message or comment of the
> sprs.c file below.
Hmm.. right, but that just means the particular value you're using
doesn't break - other values could cause guest crashes. But as it
appears now, it looks like the value written is arbitrary.
> Anyway, working out a detailed list of values that should be written
> into each of the SPRs might get quite difficult/cumbersome since some of
> these SPRs are not that well documented in the PowerISA. So I'd like to
> keep the generic approach here instead. Anyway, if a write causes
> trouble, it's not a big issue to take that SPR out of the list again.
> And since the kvm-unit-tests are also a very isolated test (i.e. we're
> not running a Linux kernel here which might expect certain values in
> certain SPRs), it's IMHO also ok to mess up the state in the SPRs since
> the VM will get destroyed after the test anyway.
Hm, yeah, I guess so. I'm just concerned about crashing the guest
before the test can even finish.
> About your second concern, that the values will get masked or modified:
> Yes, you're right, of course! But that's not a problem here since I read
> back the values from the SPRs before the migration - I do not use the
> original value that I wrote into the SPRs for comparison!
Ah, I missed that, sorry. Clever approach.
> > [snip]
> >> +/* Common SPRs for all PowerPC CPUs */
> >> +static void set_sprs_common(uint64_t val)
> >> +{
> >> + mtspr(1, val); /* XER */
> >> + mtspr(9, val); /* CTR */
> >> + mtspr(273, val); /* SPRG1 */
> >> + mtspr(274, val); /* SPRG2 */
> >> + mtspr(275, val); /* SPRG3 */
> >> +}
> >> +
> >> +/* 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 */
> >
> > AMR seems to be listed twice..
>
> That's because AMR is available via both SPR numbers - one time for
> userspace mode (which can be disabled by the kernel if desired), and one
> time for privileged mode.
>
> > At a glance SPRs above where writing an arbitrary value might not be
> > safe include AMR, IAMR, UAMOR, and MMCR*.
>
> Well, you should not think of this test as a nice, behaving guest kernel
> that only puts valid values into safe SPRs ... rather think of it as a
> stress test for the host with a bad, misbehaving guest kernel.
>
> So think what's the worst thing that could happen? Host crash? Right,
> this test already helped to find one of those bugs:
>
> http://www.spinics.net/lists/kvm/msg128989.html
>
> ... but then we've of course got to fix the host kernel (or QEMU), not
> this test.
>
> The other bad thing that could of course happen is a guest crash - but
> then, as mentioned above, we can simply adjust the list of SPRs that we
> test as soon as we see such a crash. With the current list, I do not get
> any guest crashes - at least not with kvm-hv. ... but kvm-pr and tcg are
> two other candidates that likely need some fixing for this test.
I'm still a little concerned about crashing the guest before the test
can complete. But as you say generating sane values for all SPRs is
quite a lot of work, and in the meantime the SPRs which could crash the
guest (or the host) are the ones that are especially important to test
migration of.
So, you've convinced me that this is a good idea on balance.
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
@ 2016-04-12 1:21 ` David Gibson
0 siblings, 0 replies; 19+ messages in thread
From: David Gibson @ 2016-04-12 1:21 UTC (permalink / raw)
To: Thomas Huth; +Cc: kvm, kvm-ppc, pbonzini, lvivier, clg, drjones
[-- Attachment #1: Type: text/plain, Size: 5157 bytes --]
On Mon, 11 Apr 2016 11:23:46 +0200
Thomas Huth <thuth@redhat.com> wrote:
> On 11.04.2016 03:55, David Gibson wrote:
> > On Fri, 8 Apr 2016 13:35:29 +0200
> > Thomas Huth <thuth@redhat.com> wrote:
> >
> >> This test can be used to check whether the SPR (special purpose
> >> registers) of the PowerPC CPU are migrated right. It first fills
> >> the various SPRs with some non-zero value, then reads the values
> >> back into a first array, then waits for a key (with the '-w' option)
> >> so that it is possible to migrate the VM, and finally reads the
> >> values from the SPRs back into another array and then compares it
> >> with the initial values.
> >> Currently the test only supports the SPRs from the 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>
> >
> > So, the main concern I have about this is that while writing an
> > arbitrary value is ok for quite a few SPRs, there's a significant
> > number where that's not safe: either because it could actually cause
> > some sort of exception interrupting the test, or because the value will
> > get masked or otherwise modifier on write.
>
> If a write to an SPR caused any exception or hang of the guest, I did
> not include it in the list of SPRs to be tested here. Sorry, I should
> have mentioned that somewhere in the commit message or comment of the
> sprs.c file below.
Hmm.. right, but that just means the particular value you're using
doesn't break - other values could cause guest crashes. But as it
appears now, it looks like the value written is arbitrary.
> Anyway, working out a detailed list of values that should be written
> into each of the SPRs might get quite difficult/cumbersome since some of
> these SPRs are not that well documented in the PowerISA. So I'd like to
> keep the generic approach here instead. Anyway, if a write causes
> trouble, it's not a big issue to take that SPR out of the list again.
> And since the kvm-unit-tests are also a very isolated test (i.e. we're
> not running a Linux kernel here which might expect certain values in
> certain SPRs), it's IMHO also ok to mess up the state in the SPRs since
> the VM will get destroyed after the test anyway.
Hm, yeah, I guess so. I'm just concerned about crashing the guest
before the test can even finish.
> About your second concern, that the values will get masked or modified:
> Yes, you're right, of course! But that's not a problem here since I read
> back the values from the SPRs before the migration - I do not use the
> original value that I wrote into the SPRs for comparison!
Ah, I missed that, sorry. Clever approach.
> > [snip]
> >> +/* Common SPRs for all PowerPC CPUs */
> >> +static void set_sprs_common(uint64_t val)
> >> +{
> >> + mtspr(1, val); /* XER */
> >> + mtspr(9, val); /* CTR */
> >> + mtspr(273, val); /* SPRG1 */
> >> + mtspr(274, val); /* SPRG2 */
> >> + mtspr(275, val); /* SPRG3 */
> >> +}
> >> +
> >> +/* 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 */
> >
> > AMR seems to be listed twice..
>
> That's because AMR is available via both SPR numbers - one time for
> userspace mode (which can be disabled by the kernel if desired), and one
> time for privileged mode.
>
> > At a glance SPRs above where writing an arbitrary value might not be
> > safe include AMR, IAMR, UAMOR, and MMCR*.
>
> Well, you should not think of this test as a nice, behaving guest kernel
> that only puts valid values into safe SPRs ... rather think of it as a
> stress test for the host with a bad, misbehaving guest kernel.
>
> So think what's the worst thing that could happen? Host crash? Right,
> this test already helped to find one of those bugs:
>
> http://www.spinics.net/lists/kvm/msg128989.html
>
> ... but then we've of course got to fix the host kernel (or QEMU), not
> this test.
>
> The other bad thing that could of course happen is a guest crash - but
> then, as mentioned above, we can simply adjust the list of SPRs that we
> test as soon as we see such a crash. With the current list, I do not get
> any guest crashes - at least not with kvm-hv. ... but kvm-pr and tcg are
> two other candidates that likely need some fixing for this test.
I'm still a little concerned about crashing the guest before the test
can complete. But as you say generating sane values for all SPRs is
quite a lot of work, and in the meantime the SPRs which could crash the
guest (or the host) are the ones that are especially important to test
migration of.
So, you've convinced me that this is a good idea on balance.
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
2016-04-12 1:21 ` David Gibson
(?)
@ 2016-04-12 21:32 ` Greg Harmon
2016-04-14 8:18 ` Thomas Huth
-1 siblings, 1 reply; 19+ messages in thread
From: Greg Harmon @ 2016-04-12 21:32 UTC (permalink / raw)
To: kvm
David Gibson <dgibson <at> redhat.com> writes:
>
> On Mon, 11 Apr 2016 11:23:46 +0200
> Thomas Huth <thuth <at> redhat.com> wrote:
>
> > On 11.04.2016 03:55, David Gibson wrote:
> > > On Fri, 8 Apr 2016 13:35:29 +0200
> > > Thomas Huth <thuth <at> redhat.com> wrote:
> > >
> > >> This test can be used to check whether the SPR (special purpose
> > >> registers) of the PowerPC CPU are migrated right. It first fills
> > >> the various SPRs with some non-zero value, then reads the values
> > >> back into a first array, then waits for a key (with the '-w' option)
> > >> so that it is possible to migrate the VM, and finally reads the
> > >> values from the SPRs back into another array and then compares it
> > >> with the initial values.
Hi,
In order to make this test automated, what if we extend the test device to
return a save/restore generation number? It can report that in a
register,and then the test can spin, waiting for that value to increase.
Thanks,
Greg
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
2016-04-12 21:32 ` Greg Harmon
@ 2016-04-14 8:18 ` Thomas Huth
2016-04-14 16:43 ` Greg Harmon
0 siblings, 1 reply; 19+ messages in thread
From: Thomas Huth @ 2016-04-14 8:18 UTC (permalink / raw)
To: Greg Harmon, kvm
On 12.04.2016 23:32, Greg Harmon wrote:
> David Gibson <dgibson <at> redhat.com> writes:
>> On Mon, 11 Apr 2016 11:23:46 +0200
>> Thomas Huth <thuth <at> redhat.com> wrote:
>>> On 11.04.2016 03:55, David Gibson wrote:
>>>> On Fri, 8 Apr 2016 13:35:29 +0200
>>>> Thomas Huth <thuth <at> redhat.com> wrote:
>>>>
>>>>> This test can be used to check whether the SPR (special purpose
>>>>> registers) of the PowerPC CPU are migrated right. It first fills
>>>>> the various SPRs with some non-zero value, then reads the values
>>>>> back into a first array, then waits for a key (with the '-w' option)
>>>>> so that it is possible to migrate the VM, and finally reads the
>>>>> values from the SPRs back into another array and then compares it
>>>>> with the initial values.
>
> Hi,
> In order to make this test automated, what if we extend the test device to
> return a save/restore generation number? It can report that in a
> register,and then the test can spin, waiting for that value to increase.
The test device only works on x86, as far as I know, so I'm afraid it's
not usable for this case here on PowerPC.
Thomas
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
2016-04-14 8:18 ` Thomas Huth
@ 2016-04-14 16:43 ` Greg Harmon
2016-04-15 8:43 ` Andrew Jones
2016-04-19 8:20 ` Thomas Huth
0 siblings, 2 replies; 19+ messages in thread
From: Greg Harmon @ 2016-04-14 16:43 UTC (permalink / raw)
To: Thomas Huth; +Cc: kvm
On Thu, Apr 14, 2016 at 1:18 AM, Thomas Huth <thuth@redhat.com> wrote:
> On 12.04.2016 23:32, Greg Harmon wrote:
>> David Gibson <dgibson <at> redhat.com> writes:
>>> On Mon, 11 Apr 2016 11:23:46 +0200
>>> Thomas Huth <thuth <at> redhat.com> wrote:
>>>> On 11.04.2016 03:55, David Gibson wrote:
>>>>> On Fri, 8 Apr 2016 13:35:29 +0200
>>>>> Thomas Huth <thuth <at> redhat.com> wrote:
>>>>>
>>>>>> This test can be used to check whether the SPR (special purpose
>>>>>> registers) of the PowerPC CPU are migrated right. It first fills
>>>>>> the various SPRs with some non-zero value, then reads the values
>>>>>> back into a first array, then waits for a key (with the '-w' option)
>>>>>> so that it is possible to migrate the VM, and finally reads the
>>>>>> values from the SPRs back into another array and then compares it
>>>>>> with the initial values.
>>
>> Hi,
>> In order to make this test automated, what if we extend the test device to
>> return a save/restore generation number? It can report that in a
>> register,and then the test can spin, waiting for that value to increase.
>
> The test device only works on x86, as far as I know, so I'm afraid it's
> not usable for this case here on PowerPC.
That's too bad that test device doesn't run on PowerPC. I'll try to
send a patch sometime to add this type of register to the test device.
Thanks,
Greg
>
> Thomas
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
2016-04-14 16:43 ` Greg Harmon
@ 2016-04-15 8:43 ` Andrew Jones
2016-04-19 8:20 ` Thomas Huth
1 sibling, 0 replies; 19+ messages in thread
From: Andrew Jones @ 2016-04-15 8:43 UTC (permalink / raw)
To: Greg Harmon; +Cc: Thomas Huth, kvm
On Thu, Apr 14, 2016 at 09:43:10AM -0700, Greg Harmon wrote:
> On Thu, Apr 14, 2016 at 1:18 AM, Thomas Huth <thuth@redhat.com> wrote:
> > On 12.04.2016 23:32, Greg Harmon wrote:
> >> David Gibson <dgibson <at> redhat.com> writes:
> >>> On Mon, 11 Apr 2016 11:23:46 +0200
> >>> Thomas Huth <thuth <at> redhat.com> wrote:
> >>>> On 11.04.2016 03:55, David Gibson wrote:
> >>>>> On Fri, 8 Apr 2016 13:35:29 +0200
> >>>>> Thomas Huth <thuth <at> redhat.com> wrote:
> >>>>>
> >>>>>> This test can be used to check whether the SPR (special purpose
> >>>>>> registers) of the PowerPC CPU are migrated right. It first fills
> >>>>>> the various SPRs with some non-zero value, then reads the values
> >>>>>> back into a first array, then waits for a key (with the '-w' option)
> >>>>>> so that it is possible to migrate the VM, and finally reads the
> >>>>>> values from the SPRs back into another array and then compares it
> >>>>>> with the initial values.
> >>
> >> Hi,
> >> In order to make this test automated, what if we extend the test device to
> >> return a save/restore generation number? It can report that in a
> >> register,and then the test can spin, waiting for that value to increase.
> >
> > The test device only works on x86, as far as I know, so I'm afraid it's
> > not usable for this case here on PowerPC.
>
> That's too bad that test device doesn't run on PowerPC. I'll try to
> send a patch sometime to add this type of register to the test device.
The current plan for powerpc testdev is to use chr-testdev (see
backends/testdev.c in qemu src), like arm does. Also the current plan
to use it is to bind it to an spapr-vty. Of course plans can change
in light of better proposals, but that's where things were left. chr-
testdev currently only has one function (debugexit), which doesn't
require a read function, so we'll need to add one to support a
generation number feature.
Thanks,
drew
>
> Thanks,
> Greg
>
> >
> > Thomas
> >
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [kvm-unit-tests PATCH] powerpc: Add SPRs migration test
2016-04-14 16:43 ` Greg Harmon
2016-04-15 8:43 ` Andrew Jones
@ 2016-04-19 8:20 ` Thomas Huth
1 sibling, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2016-04-19 8:20 UTC (permalink / raw)
To: Greg Harmon; +Cc: kvm
On 14.04.2016 18:43, Greg Harmon wrote:
> On Thu, Apr 14, 2016 at 1:18 AM, Thomas Huth <thuth@redhat.com> wrote:
>> On 12.04.2016 23:32, Greg Harmon wrote:
>>> David Gibson <dgibson <at> redhat.com> writes:
>>>> On Mon, 11 Apr 2016 11:23:46 +0200
>>>> Thomas Huth <thuth <at> redhat.com> wrote:
>>>>> On 11.04.2016 03:55, David Gibson wrote:
>>>>>> On Fri, 8 Apr 2016 13:35:29 +0200
>>>>>> Thomas Huth <thuth <at> redhat.com> wrote:
>>>>>>
>>>>>>> This test can be used to check whether the SPR (special purpose
>>>>>>> registers) of the PowerPC CPU are migrated right. It first fills
>>>>>>> the various SPRs with some non-zero value, then reads the values
>>>>>>> back into a first array, then waits for a key (with the '-w' option)
>>>>>>> so that it is possible to migrate the VM, and finally reads the
>>>>>>> values from the SPRs back into another array and then compares it
>>>>>>> with the initial values.
>>>
>>> Hi,
>>> In order to make this test automated, what if we extend the test device to
>>> return a save/restore generation number? It can report that in a
>>> register,and then the test can spin, waiting for that value to increase.
>>
>> The test device only works on x86, as far as I know, so I'm afraid it's
>> not usable for this case here on PowerPC.
>
> That's too bad that test device doesn't run on PowerPC. I'll try to
> send a patch sometime to add this type of register to the test device.
By the way, for x86, there is already such a mechanism specified, see:
https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg05600.html
... seems like it has just not been merged yet.
Thomas
^ permalink raw reply [flat|nested] 19+ messages in thread