[PATCH kvm-unit-tests 0/8] arm/arm64: extend psci tests

[PATCH kvm-unit-tests 0/8] arm/arm64: extend psci tests

[Andrew Jones]

Add migration support for ARM and extend the PSCI tests with some
checking pre/post migration. Also extend PSCI tests with tests for
the functionality (emulating PSCI 1.1) getting merged upstream now.

Andrew Jones (8):
  virtio-mmio: fix queue allocation
  scripts/arch-run: run_migration improvements
  powerpc: don't use NMI's to signal end of migration
  powerpc: Introduce getchar
  lib: Introduce do_migration
  arm/arm64: Add support for migration tests
  arm/arm64: psci: add migration test
  arm/psci: add smccc 1.1 tests

 arm/psci.c              | 191 +++++++++++++++++++++++++++++++++++++++++-------
 arm/run                 |   2 +-
 arm/unittests.cfg       |   7 ++
 lib/arm/io.c            |   5 ++
 lib/libcflat.h          |   8 ++
 lib/powerpc/asm/hcall.h |   1 +
 lib/powerpc/hcall.c     |  12 +++
 lib/virtio-mmio.c       |   3 +-
 powerpc/sprs.c          |  38 ++--------
 scripts/arch-run.bash   |  37 ++++++----
 10 files changed, 231 insertions(+), 73 deletions(-)

-- 
2.13.6

[PATCH kvm-unit-tests 1/8] virtio-mmio: fix queue allocation

[Andrew Jones]

Before 031755db ("arm: enable vmalloc") we were allocating the
queue with two pages of zeroed memory using memalign(), but
afterwards with only one uninitialized page using alloc_pages().
We can keep alloc_pages(), but we need two pages, and they need
to be clean, otherwise QEMU gets angry when we attempt to migrate
a unit test as the used vring index is corrupted by the page
allocator's next page link.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 lib/virtio-mmio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/virtio-mmio.c b/lib/virtio-mmio.c
index e5e8f660b5cd..cbc9e6217bbe 100644
--- a/lib/virtio-mmio.c
+++ b/lib/virtio-mmio.c
@@ -55,7 +55,8 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev,
 
 	vq = calloc(1, sizeof(*vq));
 	assert(VIRTIO_MMIO_QUEUE_SIZE_MIN <= 2*PAGE_SIZE);
-	queue = alloc_pages(1);
+	queue = alloc_pages(2);
+	memset(queue, 0, 2*PAGE_SIZE);
 	assert(vq && queue);
 
 	writel(index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL);
-- 
2.13.6

[PATCH kvm-unit-tests 2/8] scripts/arch-run: run_migration improvements

[Andrew Jones]

 - Don't assume the first argument is QEMU, it's actually most
   likely 'timeout'.
 - Don't assume run_migration is called from a subshell (even
   though it is) and use return and a RETURN trap handler instead
   (and make this change in env_generate_errata() as well).
 - Cleanup all child processes on termination
 - Make sure the exit code of the unit test is propagated.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 scripts/arch-run.bash | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 1e41eb7bd4ab..565e299295db 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -108,12 +108,9 @@ run_migration ()
 {
 	if ! command -v nc >/dev/null 2>&1; then
 		echo "${FUNCNAME[0]} needs nc (netcat)" >&2
-		exit 2
+		return 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`
@@ -121,13 +118,15 @@ run_migration ()
 	qmpout1=/dev/null
 	qmpout2=/dev/null
 
-	trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2}' EXIT
+	trap 'kill 0; exit 2' INT TERM
+	trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2}' RETURN EXIT
 
-	$qemu "$@" -chardev socket,id=mon1,path=${qmp1},server,nowait \
-		 -mon chardev=mon1,mode=control | tee ${migout1} &
+	eval "$@" -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} &
+	eval "$@" -chardev socket,id=mon2,path=${qmp2},server,nowait \
+		-mon chardev=mon2,mode=control -incoming unix:${migsock} &
+	incoming_pid=`jobs -l %+ | awk '{print$2}'`
 
 	# The test must prompt the user to migrate, so wait for the "migrate" keyword
 	while ! grep -q -i "migrate" < ${migout1} ; do
@@ -145,14 +144,17 @@ run_migration ()
 			echo "ERROR: Migration failed." >&2
 			qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
 			qmp ${qmp2} '"quit"'> ${qmpout2} 2>/dev/null
-			exit 2
+			return 2
 		fi
 	done
 	qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
 
 	qmp ${qmp2} '"inject-nmi"'> ${qmpout2}
 
+	wait $incoming_pid
+	ret=$?
 	wait
+	return $ret
 }
 
 migration_cmd ()
@@ -254,7 +256,7 @@ env_generate_errata ()
 
 		if ! [[ $v =~ ^[0-9]+$ ]] || ! [[ $p =~ ^[0-9]+$ ]]; then
 			echo "Bad minimum kernel version in $ERRATATXT, $minver"
-			exit 2
+			return 2
 		fi
 		! [[ $s =~ ^[0-9]+$ ]] && unset $s
 		! [[ $x =~ ^[0-9]+$ ]] && unset $x
-- 
2.13.6

[PATCH kvm-unit-tests 3/8] powerpc: don't use NMI's to signal end of migration

[Andrew Jones]

The SPRs test already supports using serial input as the trigger to
proceed after waiting for migration to complete, but the general
test framework uses NMI's (which the SPR test also supported before
this patch). ARM doesn't support NMI injection, so change the general
framework to use serial input instead, and drop the NMI support from
the SPR test.

Cc: Thomas Huth <thuth@redhat.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 powerpc/sprs.c        | 26 +++++++++-----------------
 scripts/arch-run.bash | 13 ++++++++-----
 2 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index c02bcc9fcf32..3b920e9f929a 100644
--- a/powerpc/sprs.c
+++ b/powerpc/sprs.c
@@ -8,13 +8,13 @@
  * 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.
+ * all bits might be retained in the SPRs), then wait for migration to complete
+ * (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.
@@ -38,13 +38,6 @@
 
 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 */
@@ -303,9 +296,8 @@ int main(int argc, char **argv)
 	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)
+		puts("Now migrate the VM, then press a key to continue...\n");
+		while (h_get_term_char(0) == 0)
 			cpu_relax();
 	} else {
 		puts("Sleeping...\n");
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 565e299295db..e13af8e8064a 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -115,17 +115,22 @@ run_migration ()
 	migout1=`mktemp -t mig-helper-stdout1.XXXXXXXXXX`
 	qmp1=`mktemp -u -t mig-helper-qmp1.XXXXXXXXXX`
 	qmp2=`mktemp -u -t mig-helper-qmp2.XXXXXXXXXX`
+	fifo=`mktemp -u -t mig-helper-fifo.XXXXXXXXXX`
 	qmpout1=/dev/null
 	qmpout2=/dev/null
 
 	trap 'kill 0; exit 2' INT TERM
-	trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2}' RETURN EXIT
+	trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}' RETURN EXIT
 
 	eval "$@" -chardev socket,id=mon1,path=${qmp1},server,nowait \
 		-mon chardev=mon1,mode=control | tee ${migout1} &
 
+	# We have to use cat to open the named FIFO, because named FIFO's, unlike
+	# pipes, will block on open() until the other end is also opened, and that
+	# totally breaks QEMU...
+	mkfifo ${fifo}
 	eval "$@" -chardev socket,id=mon2,path=${qmp2},server,nowait \
-		-mon chardev=mon2,mode=control -incoming unix:${migsock} &
+		-mon chardev=mon2,mode=control -incoming unix:${migsock} < <(cat ${fifo}) &
 	incoming_pid=`jobs -l %+ | awk '{print$2}'`
 
 	# The test must prompt the user to migrate, so wait for the "migrate" keyword
@@ -148,9 +153,7 @@ run_migration ()
 		fi
 	done
 	qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
-
-	qmp ${qmp2} '"inject-nmi"'> ${qmpout2}
-
+	echo > ${fifo}
 	wait $incoming_pid
 	ret=$?
 	wait
-- 
2.13.6

[PATCH kvm-unit-tests 4/8] powerpc: Introduce getchar

[Andrew Jones]

Move the unit test specific h_get_term_char() to powerpc common code
and call it getchar(). Other architectures may want to implement
getchar() too (ARM will in a coming patch), so put the prototype in
libcflat.h

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 lib/libcflat.h          |  1 +
 lib/powerpc/asm/hcall.h |  1 +
 lib/powerpc/hcall.c     | 12 ++++++++++++
 powerpc/sprs.c          | 14 +-------------
 4 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index fb465415c14a..c680b69a926e 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -82,6 +82,7 @@ typedef u64			phys_addr_t;
 #define INVALID_PHYS_ADDR	(~(phys_addr_t)0)
 
 extern void puts(const char *s);
+extern int getchar(void);
 extern void exit(int code);
 extern void abort(void);
 extern long atol(const char *ptr);
diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h
index 80aa3e304382..a8bd7e3afc37 100644
--- a/lib/powerpc/asm/hcall.h
+++ b/lib/powerpc/asm/hcall.h
@@ -19,6 +19,7 @@
 #define H_SET_DABR		0x28
 #define H_PAGE_INIT		0x2c
 #define H_CEDE			0xE0
+#define H_GET_TERM_CHAR		0x54
 #define H_PUT_TERM_CHAR		0x58
 #define H_RANDOM		0x300
 #define H_SET_MODE		0x31C
diff --git a/lib/powerpc/hcall.c b/lib/powerpc/hcall.c
index cd6d26680f7c..d65af93697e4 100644
--- a/lib/powerpc/hcall.c
+++ b/lib/powerpc/hcall.c
@@ -31,3 +31,15 @@ void putchar(int c)
 
 	hcall(H_PUT_TERM_CHAR, vty, nr_chars, chars);
 }
+
+int getchar(void)
+{
+	register unsigned long r3 asm("r3") = H_GET_TERM_CHAR;
+	register unsigned long r4 asm("r4") = 0; /* 0 == default vty */
+	register unsigned long 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;
+}
diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index 3b920e9f929a..5b5a54032819 100644
--- a/powerpc/sprs.c
+++ b/powerpc/sprs.c
@@ -38,18 +38,6 @@
 
 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)
 {
@@ -297,7 +285,7 @@ int main(int argc, char **argv)
 
 	if (pause) {
 		puts("Now migrate the VM, then press a key to continue...\n");
-		while (h_get_term_char(0) == 0)
+		while (getchar() == 0)
 			cpu_relax();
 	} else {
 		puts("Sleeping...\n");
-- 
2.13.6

[PATCH kvm-unit-tests 5/8] lib: Introduce do_migration

[Andrew Jones]

Wrap migration starting and waiting into a function in order to
encapsulate the protocol.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 lib/libcflat.h        | 7 +++++++
 powerpc/sprs.c        | 4 +---
 scripts/arch-run.bash | 4 ++--
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index c680b69a926e..29b39a54eb00 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -159,4 +159,11 @@ void print_binstr(unsigned long x);
 
 extern void setup_vm(void);
 
+static inline void do_migration(void)
+{
+	report_info("Migration Start, migrate the VM and then press a key to continue...");
+	while (getchar() == 0);
+	report_info("Migration Complete");
+}
+
 #endif
diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index 5b5a54032819..a75abdd7f8c1 100644
--- a/powerpc/sprs.c
+++ b/powerpc/sprs.c
@@ -284,9 +284,7 @@ int main(int argc, char **argv)
 	get_sprs(before);
 
 	if (pause) {
-		puts("Now migrate the VM, then press a key to continue...\n");
-		while (getchar() == 0)
-			cpu_relax();
+		do_migration();
 	} else {
 		puts("Sleeping...\n");
 		handle_exception(0x900, &dec_except_handler, NULL);
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index e13af8e8064a..9d8687c42f2c 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -133,8 +133,8 @@ run_migration ()
 		-mon chardev=mon2,mode=control -incoming unix:${migsock} < <(cat ${fifo}) &
 	incoming_pid=`jobs -l %+ | awk '{print$2}'`
 
-	# The test must prompt the user to migrate, so wait for the "migrate" keyword
-	while ! grep -q -i "migrate" < ${migout1} ; do
+	# Wait for the unit test to execute do_migration()
+	while ! grep -q "Migration Start" < ${migout1} ; do
 		sleep 1
 	done
 
-- 
2.13.6

[PATCH kvm-unit-tests 6/8] arm/arm64: Add support for migration tests

[Andrew Jones]

We only need to implement getchar() and pass the test command
line to run_migration when the unittests.cfg file tells us to.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/run      | 2 +-
 lib/arm/io.c | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arm/run b/arm/run
index fd280ee19837..1dd11b950447 100755
--- a/arm/run
+++ b/arm/run
@@ -57,6 +57,6 @@ fi
 M+=",accel=$ACCEL"
 command="$qemu -nodefaults $M -cpu $processor $chr_testdev $pci_testdev"
 command+=" -display none -serial stdio -kernel"
-command="$(timeout_cmd) $command"
+command="$(migration_cmd) $(timeout_cmd) $command"
 
 run_qemu $command "$@"
diff --git a/lib/arm/io.c b/lib/arm/io.c
index a111530f4802..f6b9a9012776 100644
--- a/lib/arm/io.c
+++ b/lib/arm/io.c
@@ -77,6 +77,11 @@ void puts(const char *s)
 	spin_unlock(&uart_lock);
 }
 
+int getchar(void)
+{
+	return readb(uart0_base);
+}
+
 void exit(int code)
 {
 	chr_testdev_exit(code);
-- 
2.13.6

[PATCH kvm-unit-tests 7/8] arm/arm64: psci: add migration test

[Andrew Jones]

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/psci.c        | 132 ++++++++++++++++++++++++++++++++++++++++++------------
 arm/unittests.cfg |   7 +++
 2 files changed, 111 insertions(+), 28 deletions(-)

diff --git a/arm/psci.c b/arm/psci.c
index 5cb4d5c7c233..a092f00c2cab 100644
--- a/arm/psci.c
+++ b/arm/psci.c
@@ -12,6 +12,9 @@
 #include <asm/processor.h>
 #include <asm/smp.h>
 #include <asm/psci.h>
+#include <asm/delay.h>
+
+static cpumask_t halted;
 
 static bool invalid_function_exception;
 
@@ -49,25 +52,15 @@ static bool psci_invalid_function(void)
 	return pass;
 }
 
-static int psci_affinity_info(unsigned long target_affinity, uint32_t lowest_affinity_level)
+static int psci_affinity_info(unsigned int cpu)
 {
 #ifdef __arm__
-	return psci_invoke(PSCI_0_2_FN_AFFINITY_INFO, target_affinity, lowest_affinity_level, 0);
+	return psci_invoke(PSCI_0_2_FN_AFFINITY_INFO, cpus[cpu], 0, 0);
 #else
-	return psci_invoke(PSCI_0_2_FN64_AFFINITY_INFO, target_affinity, lowest_affinity_level, 0);
+	return psci_invoke(PSCI_0_2_FN64_AFFINITY_INFO, cpus[cpu], 0, 0);
 #endif
 }
 
-static bool psci_affinity_info_on(void)
-{
-	return psci_affinity_info(cpus[0], 0) == PSCI_0_2_AFFINITY_LEVEL_ON;
-}
-
-static bool psci_affinity_info_off(void)
-{
-	return psci_affinity_info(cpus[1], 0) == PSCI_0_2_AFFINITY_LEVEL_OFF;
-}
-
 static int cpu_on_ret[NR_CPUS];
 static cpumask_t cpu_on_ready, cpu_on_done;
 static volatile int cpu_on_start;
@@ -122,33 +115,116 @@ static bool psci_cpu_on_test(void)
 	return !failed;
 }
 
-int main(void)
+static void general_check(void)
 {
-	int ver = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
-
-	if (nr_cpus < 2) {
-		report_skip("At least 2 cpus required");
-		goto done;
-	}
-
-	report_info("PSCI version %d.%d", PSCI_VERSION_MAJOR(ver),
-					  PSCI_VERSION_MINOR(ver));
 	report("invalid-function", psci_invalid_function());
-	report("affinity-info-on", psci_affinity_info_on());
-	report("affinity-info-off", psci_affinity_info_off());
+	report("affinity-info-on", psci_affinity_info(0) == PSCI_0_2_AFFINITY_LEVEL_ON);
+	report("affinity-info-off", psci_affinity_info(1) == PSCI_0_2_AFFINITY_LEVEL_OFF);
 
 	if (ERRATA(6c7a5dce22b3))
 		report("cpu-on", psci_cpu_on_test());
 	else
 		report_skip("Skipping unsafe cpu-on test. Set ERRATA_6c7a5dce22b3=y to enable.");
+}
 
-done:
+static void general_exit(void)
+{
 #if 0
 	report_summary();
 	psci_invoke(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
 	report("system-off", false);
 	return 1; /* only reaches here if system-off fails */
-#else
-	return report_summary();
 #endif
 }
+
+static void do_wfi(void *data)
+{
+	cpumask_set_cpu(smp_processor_id(), &halted);
+	wfi();
+	cpumask_clear_cpu(smp_processor_id(), &halted);
+}
+
+static void migrate_prepare(void)
+{
+	/*
+	 * CPU0 is "runnable" now and should remain so after migration.
+	 * CPU1 is "stopped" now and should remain so after migration.
+	 * CPU2 will execute WFI before migration, so it should be "halted", both
+	 * before and after the migration.
+	 */
+	assert(psci_affinity_info(0) == PSCI_0_2_AFFINITY_LEVEL_ON);
+	assert(psci_affinity_info(1) == PSCI_0_2_AFFINITY_LEVEL_OFF);
+	on_cpu_async(2, do_wfi, NULL);
+	while (!cpumask_test_cpu(2, &halted))
+		cpu_relax();
+	assert(psci_affinity_info(2) == PSCI_0_2_AFFINITY_LEVEL_ON);
+
+	mdelay(500);
+	do_migration();
+	mdelay(500);
+}
+
+static void migrate_check(void)
+{
+	report("CPU0 runnable", psci_affinity_info(0) == PSCI_0_2_AFFINITY_LEVEL_ON);
+	report("CPU1 stopped", psci_affinity_info(1) == PSCI_0_2_AFFINITY_LEVEL_OFF);
+	report("CPU2 not stopped", psci_affinity_info(2) == PSCI_0_2_AFFINITY_LEVEL_ON);
+	report("CPU2 still halted", cpumask_test_cpu(2, &halted));
+}
+
+static struct test {
+	const char *name;
+	int nr_cpus;
+	void (*prepare)(void);
+	void (*check)(void);
+	void (*exit)(void);
+} tests[] = {
+{
+	.name = "general",
+	.nr_cpus = 2,
+	.check = general_check,
+	.exit = general_exit,
+},
+{
+	.name = "migrate",
+	.nr_cpus = 3,
+	.prepare = migrate_prepare,
+	.check = migrate_check,
+},
+	{},
+};
+
+static struct test *get_test(const char *name)
+{
+	struct test *test = &tests[0];
+	while (test->name && strcmp(test->name, name))
+		++test;
+	return test->name ? test : NULL;
+}
+
+int main(int ac, char **av)
+{
+	int psci_version = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
+	struct test *test = get_test("general");
+
+	report_info("PSCI version %d.%d", PSCI_VERSION_MAJOR(psci_version),
+					  PSCI_VERSION_MINOR(psci_version));
+
+	if (ac > 1)
+		test = get_test(av[1]);
+
+	if (nr_cpus < test->nr_cpus) {
+		report_skip("At least %d cpus required", test->nr_cpus);
+		goto done;
+	}
+
+	if (test->prepare)
+		test->prepare();
+	if (test->check)
+		test->check();
+	if (test->exit)
+		test->exit();
+
+done:
+	return report_summary();
+}
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index 44b98cfc7afd..cf5a3f5c51d2 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -110,6 +110,13 @@ file = psci.flat
 smp = $MAX_SMP
 groups = psci
 
+[psci-migration]
+file = psci.flat
+smp = $MAX_SMP
+extra_params = -append 'migrate'
+timeout = 15s
+groups = psci,migration
+
 # Timer tests
 [timer]
 file = timer.flat
-- 
2.13.6

[PATCH kvm-unit-tests 8/8] arm/psci: add smccc 1.1 tests

[Andrew Jones]

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/psci.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 67 insertions(+), 2 deletions(-)

diff --git a/arm/psci.c b/arm/psci.c
index a092f00c2cab..af96a82cedd0 100644
--- a/arm/psci.c
+++ b/arm/psci.c
@@ -14,9 +14,14 @@
 #include <asm/psci.h>
 #include <asm/delay.h>
 
-static cpumask_t halted;
+#define ARM_SMCCC_VERSION_FUNC_ID		((1U << 31) | 0x0000)
+#define ARM_SMCCC_ARCH_FEATURES_FUNC_ID		((1U << 31) | 0x0001)
+#define ARM_SMCCC_ARCH_WORKAROUND_1		((1U << 31) | 0x8000)
 
+static int psci_version;
+static cpumask_t halted;
 static bool invalid_function_exception;
+static bool have_workaround1;
 
 #ifdef __arm__
 static void invalid_function_handler(struct pt_regs *regs __unused)
@@ -115,12 +120,60 @@ static bool psci_cpu_on_test(void)
 	return !failed;
 }
 
+static void workaround1_test(void)
+{
+#ifdef __aarch64__
+	unsigned long ret;
+
+	if (!have_workaround1)
+		return;
+
+	asm volatile(
+	"	mov	w0, " xstr(ARM_SMCCC_ARCH_WORKAROUND_1) "\n"
+	"	hvc	#0\n"
+	"	mov	%0, x0\n"
+	: "=r" (ret) : : "x0");
+
+	report("workaround1_test", ret == 0);
+#endif
+}
+
+static void smccc11_test(void)
+{
+	int ret;
+
+	ret = psci_invoke(PSCI_1_0_FN_PSCI_FEATURES, PSCI_1_0_FN_PSCI_FEATURES, 0, 0);
+	if (ret != 0) {
+		ret = 0;
+		goto report_version;
+	}
+
+	ret = psci_invoke(PSCI_1_0_FN_PSCI_FEATURES, ARM_SMCCC_VERSION_FUNC_ID, 0, 0);
+	if (ret != 0) {
+		ret = 0;
+		goto report_version;
+	}
+
+	ret = psci_invoke(ARM_SMCCC_VERSION_FUNC_ID, 0, 0, 0);
+	report_info("SMCCC version %d.%d", PSCI_VERSION_MAJOR(ret), PSCI_VERSION_MINOR(ret));
+	ret = PSCI_VERSION_MAJOR(ret) >= 1 && PSCI_VERSION_MINOR(ret) >= 1;
+
+report_version:
+	report("smccc: version >= 1.1", ret);
+	have_workaround1 = ret && psci_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID, ARM_SMCCC_ARCH_WORKAROUND_1, 0, 0);
+}
+
 static void general_check(void)
 {
 	report("invalid-function", psci_invalid_function());
 	report("affinity-info-on", psci_affinity_info(0) == PSCI_0_2_AFFINITY_LEVEL_ON);
 	report("affinity-info-off", psci_affinity_info(1) == PSCI_0_2_AFFINITY_LEVEL_OFF);
 
+	if (PSCI_VERSION_MAJOR(psci_version) >= 1)
+		smccc11_test();
+
+	workaround1_test();
+
 	if (ERRATA(6c7a5dce22b3))
 		report("cpu-on", psci_cpu_on_test());
 	else
@@ -159,6 +212,11 @@ static void migrate_prepare(void)
 		cpu_relax();
 	assert(psci_affinity_info(2) == PSCI_0_2_AFFINITY_LEVEL_ON);
 
+	if (PSCI_VERSION_MAJOR(psci_version) >= 1)
+		smccc11_test();
+
+	workaround1_test();
+
 	mdelay(500);
 	do_migration();
 	mdelay(500);
@@ -170,6 +228,13 @@ static void migrate_check(void)
 	report("CPU1 stopped", psci_affinity_info(1) == PSCI_0_2_AFFINITY_LEVEL_OFF);
 	report("CPU2 not stopped", psci_affinity_info(2) == PSCI_0_2_AFFINITY_LEVEL_ON);
 	report("CPU2 still halted", cpumask_test_cpu(2, &halted));
+
+	int version = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
+	report_info("PSCI version %d.%d", PSCI_VERSION_MAJOR(version),
+					  PSCI_VERSION_MINOR(version));
+	if (PSCI_VERSION_MAJOR(psci_version) >= 1 || PSCI_VERSION_MAJOR(version) >= 1)
+		smccc11_test();
+	workaround1_test();
 }
 
 static struct test {
@@ -204,9 +269,9 @@ static struct test *get_test(const char *name)
 
 int main(int ac, char **av)
 {
-	int psci_version = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
 	struct test *test = get_test("general");
 
+	psci_version = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
 	report_info("PSCI version %d.%d", PSCI_VERSION_MAJOR(psci_version),
 					  PSCI_VERSION_MINOR(psci_version));
 
-- 
2.13.6

Re: [PATCH kvm-unit-tests 1/8] virtio-mmio: fix queue allocation

[Andrew Jones]

On Wed, Feb 07, 2018 at 08:03:27PM +0100, Andrew Jones wrote:
> Before 031755db ("arm: enable vmalloc") we were allocating the
> queue with two pages of zeroed memory using memalign(), but
> afterwards with only one uninitialized page using alloc_pages().
> We can keep alloc_pages(), but we need two pages, and they need
> to be clean, otherwise QEMU gets angry when we attempt to migrate
> a unit test as the used vring index is corrupted by the page
> allocator's next page link.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  lib/virtio-mmio.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/virtio-mmio.c b/lib/virtio-mmio.c
> index e5e8f660b5cd..cbc9e6217bbe 100644
> --- a/lib/virtio-mmio.c
> +++ b/lib/virtio-mmio.c
> @@ -55,7 +55,8 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev,
>  
>  	vq = calloc(1, sizeof(*vq));
>  	assert(VIRTIO_MMIO_QUEUE_SIZE_MIN <= 2*PAGE_SIZE);
> -	queue = alloc_pages(1);
> +	queue = alloc_pages(2);

Whoops. As usual my brain engages right after posting patches.
The 1 was ok here because alloc_pages() takes an order, not a
number. The memset below is still key to avoid the migration
bug though.

I'll send a v2 of this patch - mostly to fix the commit message.

drew

> +	memset(queue, 0, 2*PAGE_SIZE);
>  	assert(vq && queue);
>  
>  	writel(index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL);
> -- 
> 2.13.6
> 

[PATCH kvm-unit-tests v2 1/8] virtio-mmio: fix queue allocation

[Andrew Jones]

Ensure we clear the queue memory we get from alloc_pages().
Otherwise QEMU gets angry when we attempt to migrate a unit
test as the used vring index is corrupted by the page
allocator's next page link.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 lib/virtio-mmio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/virtio-mmio.c b/lib/virtio-mmio.c
index e5e8f660b5cd..57fe78ecbb6c 100644
--- a/lib/virtio-mmio.c
+++ b/lib/virtio-mmio.c
@@ -56,6 +56,7 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev,
 	vq = calloc(1, sizeof(*vq));
 	assert(VIRTIO_MMIO_QUEUE_SIZE_MIN <= 2*PAGE_SIZE);
 	queue = alloc_pages(1);
+	memset(queue, 0, 2*PAGE_SIZE);
 	assert(vq && queue);
 
 	writel(index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL);
-- 
2.13.6

Re: [PATCH kvm-unit-tests 3/8] powerpc: don't use NMI's to signal end of migration

[Thomas Huth]

[-- Attachment #1.1: Type: text/plain, Size: 719 bytes --]

On 07.02.2018 20:03, Andrew Jones wrote:
> The SPRs test already supports using serial input as the trigger to
> proceed after waiting for migration to complete, but the general
> test framework uses NMI's (which the SPR test also supported before
> this patch). ARM doesn't support NMI injection, so change the general
> framework to use serial input instead, and drop the NMI support from
> the SPR test.
> 
> Cc: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  powerpc/sprs.c        | 26 +++++++++-----------------
>  scripts/arch-run.bash | 13 ++++++++-----
>  2 files changed, 17 insertions(+), 22 deletions(-)

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH kvm-unit-tests 4/8] powerpc: Introduce getchar

[Thomas Huth]

On 07.02.2018 20:03, Andrew Jones wrote:
> Move the unit test specific h_get_term_char() to powerpc common code
> and call it getchar(). Other architectures may want to implement
> getchar() too (ARM will in a coming patch), so put the prototype in
> libcflat.h

Using the libc name getchar() here sounds wrong. h_get_term_char()
behaves differently compared to the libc getchar(), e.g. it does not
block if there are no characters available.
I think you should either name this function differently, or implement a
behavior that is more close to the libc getchar() to avoid future confusion.

 Thomas

Re: [PATCH kvm-unit-tests 4/8] powerpc: Introduce getchar

[Andrew Jones]

On Thu, Feb 08, 2018 at 12:06:07PM +0100, Thomas Huth wrote:
> On 07.02.2018 20:03, Andrew Jones wrote:
> > Move the unit test specific h_get_term_char() to powerpc common code
> > and call it getchar(). Other architectures may want to implement
> > getchar() too (ARM will in a coming patch), so put the prototype in
> > libcflat.h
> 
> Using the libc name getchar() here sounds wrong. h_get_term_char()
> behaves differently compared to the libc getchar(), e.g. it does not
> block if there are no characters available.
> I think you should either name this function differently, or implement a
> behavior that is more close to the libc getchar() to avoid future confusion.
>

Good point. I guess I should rename it, because we'd need to anyway in
order to share getchar()'s implementation among other architectures,
unless we duplicated the looping and EOF returning. Any suggestion for
the name? __getchar() or stdin_readb()?

Note, I see we got things wrong for puts() too. Maybe we should do a
quick libc function audit and send a cleanup series.

Thanks,
drew

Re: [PATCH kvm-unit-tests 4/8] powerpc: Introduce getchar

[Paolo Bonzini]

On 08/02/2018 13:59, Andrew Jones wrote:
> On Thu, Feb 08, 2018 at 12:06:07PM +0100, Thomas Huth wrote:
>> On 07.02.2018 20:03, Andrew Jones wrote:
>>> Move the unit test specific h_get_term_char() to powerpc common code
>>> and call it getchar(). Other architectures may want to implement
>>> getchar() too (ARM will in a coming patch), so put the prototype in
>>> libcflat.h
>>
>> Using the libc name getchar() here sounds wrong. h_get_term_char()
>> behaves differently compared to the libc getchar(), e.g. it does not
>> block if there are no characters available.
>> I think you should either name this function differently, or implement a
>> behavior that is more close to the libc getchar() to avoid future confusion.
> 
> Good point. I guess I should rename it, because we'd need to anyway in
> order to share getchar()'s implementation among other architectures,
> unless we duplicated the looping and EOF returning. Any suggestion for
> the name? __getchar() or stdin_readb()?

Like this?

diff --git a/lib/getchar.c b/lib/getchar.c
new file mode 100644
index 0000000..26f6b6b
--- /dev/null
+++ b/lib/getchar.c
@@ -0,0 +1,11 @@
+#include "libcflat.h"
+#include "asm/barrier.h"
+
+int getchar(void)
+{
+	int c;
+
+	while ((c = __getchar()) == -1)
+		cpu_relax();
+	return c;
+}
diff --git a/lib/libcflat.h b/lib/libcflat.h
index c680b69..cc56553 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -82,6 +82,7 @@ typedef u64			phys_addr_t;
 #define INVALID_PHYS_ADDR	(~(phys_addr_t)0)
 
 extern void puts(const char *s);
+extern int __getchar(void);
 extern int getchar(void);
 extern void exit(int code);
 extern void abort(void);
diff --git a/lib/powerpc/hcall.c b/lib/powerpc/hcall.c
index d65af93..7b05265 100644
--- a/lib/powerpc/hcall.c
+++ b/lib/powerpc/hcall.c
@@ -32,7 +32,7 @@ void putchar(int c)
 	hcall(H_PUT_TERM_CHAR, vty, nr_chars, chars);
 }
 
-int getchar(void)
+int __getchar(void)
 {
 	register unsigned long r3 asm("r3") = H_GET_TERM_CHAR;
 	register unsigned long r4 asm("r4") = 0; /* 0 == default vty */
@@ -41,5 +41,5 @@ int getchar(void)
 	asm volatile (" sc 1 "  : "+r"(r3), "+r"(r4), "=r"(r5)
 				: "r"(r3),  "r"(r4));
 
-	return r3 == H_SUCCESS && r4 > 0 ? r5 >> 48 : 0;
+	return r3 == H_SUCCESS && r4 > 0 ? r5 >> 48 : -1;
 }
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 21dea40..63dc166 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -30,6 +30,7 @@ asm-offsets = lib/$(ARCH)/asm-offsets.h
 include $(SRCDIR)/scripts/asm-offsets.mak
 
 cflatobjs += lib/util.o
+cflatobjs += lib/getchar.o
 cflatobjs += lib/alloc_phys.o
 cflatobjs += lib/alloc.o
 cflatobjs += lib/devicetree.o
diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index 5b5a540..6744bd8 100644
--- a/powerpc/sprs.c
+++ b/powerpc/sprs.c
@@ -285,8 +285,7 @@ int main(int argc, char **argv)
 
 	if (pause) {
 		puts("Now migrate the VM, then press a key to continue...\n");
-		while (getchar() == 0)
-			cpu_relax();
+		(void) getchar();
 	} else {
 		puts("Sleeping...\n");
 		handle_exception(0x900, &dec_except_handler, NULL);


Paolo

Re: [PATCH kvm-unit-tests 5/8] lib: Introduce do_migration

[Paolo Bonzini]

On 07/02/2018 20:03, Andrew Jones wrote:
> Wrap migration starting and waiting into a function in order to
> encapsulate the protocol.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  lib/libcflat.h        | 7 +++++++
>  powerpc/sprs.c        | 4 +---
>  scripts/arch-run.bash | 4 ++--
>  3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index c680b69a926e..29b39a54eb00 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -159,4 +159,11 @@ void print_binstr(unsigned long x);
>  
>  extern void setup_vm(void);
>  
> +static inline void do_migration(void)
> +{
> +	report_info("Migration Start, migrate the VM and then press a key to continue...");
> +	while (getchar() == 0);
> +	report_info("Migration Complete");
> +}
> +
>  #endif

I don't think this belongs in libcflat.h.  Maybe userinput.h?

Paolo

Re: [PATCH kvm-unit-tests 0/8] arm/arm64: extend psci tests

[Paolo Bonzini]

On 07/02/2018 20:03, Andrew Jones wrote:
> Add migration support for ARM and extend the PSCI tests with some
> checking pre/post migration. Also extend PSCI tests with tests for
> the functionality (emulating PSCI 1.1) getting merged upstream now.
> 
> Andrew Jones (8):
>   virtio-mmio: fix queue allocation
>   scripts/arch-run: run_migration improvements
>   powerpc: don't use NMI's to signal end of migration
>   powerpc: Introduce getchar
>   lib: Introduce do_migration
>   arm/arm64: Add support for migration tests
>   arm/arm64: psci: add migration test
>   arm/psci: add smccc 1.1 tests
> 
>  arm/psci.c              | 191 +++++++++++++++++++++++++++++++++++++++++-------
>  arm/run                 |   2 +-
>  arm/unittests.cfg       |   7 ++
>  lib/arm/io.c            |   5 ++
>  lib/libcflat.h          |   8 ++
>  lib/powerpc/asm/hcall.h |   1 +
>  lib/powerpc/hcall.c     |  12 +++
>  lib/virtio-mmio.c       |   3 +-
>  powerpc/sprs.c          |  38 ++--------
>  scripts/arch-run.bash   |  37 ++++++----
>  10 files changed, 231 insertions(+), 73 deletions(-)
> 

I'm applying patches 1-3, and 4 if you ack my changes.

Paolo

Re: [PATCH kvm-unit-tests 4/8] powerpc: Introduce getchar

[Andrew Jones]

On Wed, Feb 14, 2018 at 12:43:40PM +0100, Paolo Bonzini wrote:
> On 08/02/2018 13:59, Andrew Jones wrote:
> > On Thu, Feb 08, 2018 at 12:06:07PM +0100, Thomas Huth wrote:
> >> On 07.02.2018 20:03, Andrew Jones wrote:
> >>> Move the unit test specific h_get_term_char() to powerpc common code
> >>> and call it getchar(). Other architectures may want to implement
> >>> getchar() too (ARM will in a coming patch), so put the prototype in
> >>> libcflat.h
> >>
> >> Using the libc name getchar() here sounds wrong. h_get_term_char()
> >> behaves differently compared to the libc getchar(), e.g. it does not
> >> block if there are no characters available.
> >> I think you should either name this function differently, or implement a
> >> behavior that is more close to the libc getchar() to avoid future confusion.
> > 
> > Good point. I guess I should rename it, because we'd need to anyway in
> > order to share getchar()'s implementation among other architectures,
> > unless we duplicated the looping and EOF returning. Any suggestion for
> > the name? __getchar() or stdin_readb()?
> 
> Like this?

Precisely, although I'm not sure it needs its own file. Since it might be
nice to eventually clean up (remove?) libcflat.h, maybe we should create
lib/stdio.[ch] and lib/stdlib.[ch] files. Implementations for libc
functions (like getchar), that have their prototypes in lib/stdio.h, could
all get thrown together in lib/stdio.c.

Could be nice to define EOF in lib/stdio.h too.

Thanks,
drew

> 
> diff --git a/lib/getchar.c b/lib/getchar.c
> new file mode 100644
> index 0000000..26f6b6b
> --- /dev/null
> +++ b/lib/getchar.c
> @@ -0,0 +1,11 @@
> +#include "libcflat.h"
> +#include "asm/barrier.h"
> +
> +int getchar(void)
> +{
> +	int c;
> +
> +	while ((c = __getchar()) == -1)
> +		cpu_relax();
> +	return c;
> +}
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index c680b69..cc56553 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -82,6 +82,7 @@ typedef u64			phys_addr_t;
>  #define INVALID_PHYS_ADDR	(~(phys_addr_t)0)
>  
>  extern void puts(const char *s);
> +extern int __getchar(void);
>  extern int getchar(void);
>  extern void exit(int code);
>  extern void abort(void);
> diff --git a/lib/powerpc/hcall.c b/lib/powerpc/hcall.c
> index d65af93..7b05265 100644
> --- a/lib/powerpc/hcall.c
> +++ b/lib/powerpc/hcall.c
> @@ -32,7 +32,7 @@ void putchar(int c)
>  	hcall(H_PUT_TERM_CHAR, vty, nr_chars, chars);
>  }
>  
> -int getchar(void)
> +int __getchar(void)
>  {
>  	register unsigned long r3 asm("r3") = H_GET_TERM_CHAR;
>  	register unsigned long r4 asm("r4") = 0; /* 0 == default vty */
> @@ -41,5 +41,5 @@ int getchar(void)
>  	asm volatile (" sc 1 "  : "+r"(r3), "+r"(r4), "=r"(r5)
>  				: "r"(r3),  "r"(r4));
>  
> -	return r3 == H_SUCCESS && r4 > 0 ? r5 >> 48 : 0;
> +	return r3 == H_SUCCESS && r4 > 0 ? r5 >> 48 : -1;
>  }
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 21dea40..63dc166 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -30,6 +30,7 @@ asm-offsets = lib/$(ARCH)/asm-offsets.h
>  include $(SRCDIR)/scripts/asm-offsets.mak
>  
>  cflatobjs += lib/util.o
> +cflatobjs += lib/getchar.o
>  cflatobjs += lib/alloc_phys.o
>  cflatobjs += lib/alloc.o
>  cflatobjs += lib/devicetree.o
> diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> index 5b5a540..6744bd8 100644
> --- a/powerpc/sprs.c
> +++ b/powerpc/sprs.c
> @@ -285,8 +285,7 @@ int main(int argc, char **argv)
>  
>  	if (pause) {
>  		puts("Now migrate the VM, then press a key to continue...\n");
> -		while (getchar() == 0)
> -			cpu_relax();
> +		(void) getchar();
>  	} else {
>  		puts("Sleeping...\n");
>  		handle_exception(0x900, &dec_except_handler, NULL);
> 
> 
> Paolo

Re: [PATCH kvm-unit-tests 5/8] lib: Introduce do_migration

[Andrew Jones]

On Wed, Feb 14, 2018 at 12:45:31PM +0100, Paolo Bonzini wrote:
> On 07/02/2018 20:03, Andrew Jones wrote:
> > Wrap migration starting and waiting into a function in order to
> > encapsulate the protocol.
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  lib/libcflat.h        | 7 +++++++
> >  powerpc/sprs.c        | 4 +---
> >  scripts/arch-run.bash | 4 ++--
> >  3 files changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/lib/libcflat.h b/lib/libcflat.h
> > index c680b69a926e..29b39a54eb00 100644
> > --- a/lib/libcflat.h
> > +++ b/lib/libcflat.h
> > @@ -159,4 +159,11 @@ void print_binstr(unsigned long x);
> >  
> >  extern void setup_vm(void);
> >  
> > +static inline void do_migration(void)
> > +{
> > +	report_info("Migration Start, migrate the VM and then press a key to continue...");
> > +	while (getchar() == 0);
> > +	report_info("Migration Complete");
> > +}
> > +
> >  #endif
> 
> I don't think this belongs in libcflat.h.  Maybe userinput.h?
>

Yeah, IMO it'd be nice to completely kill libcflat.h. I'm not sure how
many user input type functions we'll add to the framework, so I'm a bit
concerned userinput.h will be a one prototype header. Well, unless you
want to throw command line parsing in there too. How about we try to
better use the concept of util.h? Maybe rename it to test-util.h or
something else (userinput.h?) first?

(Side note, we could definitely use a lib/report.h for all the report
 prototypes.)

Thanks,
drew

Re: [PATCH kvm-unit-tests 5/8] lib: Introduce do_migration

[Paolo Bonzini]

On 14/02/2018 13:44, Andrew Jones wrote:
> Yeah, IMO it'd be nice to completely kill libcflat.h. I'm not sure how
> many user input type functions we'll add to the framework, so I'm a bit
> concerned userinput.h will be a one prototype header. Well, unless you
> want to throw command line parsing in there too. How about we try to
> better use the concept of util.h? Maybe rename it to test-util.h or
> something else (userinput.h?) first?

test-util.[hc] gets my vote.

Paolo

Re: [PATCH kvm-unit-tests 4/8] powerpc: Introduce getchar

[Paolo Bonzini]

On 14/02/2018 13:38, Andrew Jones wrote:
> Precisely, although I'm not sure it needs its own file. Since it might be
> nice to eventually clean up (remove?) libcflat.h, maybe we should create
> lib/stdio.[ch] and lib/stdlib.[ch] files. Implementations for libc
> functions (like getchar), that have their prototypes in lib/stdio.h, could
> all get thrown together in lib/stdio.c.
> 
> Could be nice to define EOF in lib/stdio.h too.

Yes, I agree on all accounts!

Paolo

Re: [PATCH kvm-unit-tests 4/8] powerpc: Introduce getchar

[Andrew Jones]

On Wed, Feb 14, 2018 at 02:25:07PM +0100, Paolo Bonzini wrote:
> On 14/02/2018 13:38, Andrew Jones wrote:
> > Precisely, although I'm not sure it needs its own file. Since it might be
> > nice to eventually clean up (remove?) libcflat.h, maybe we should create
> > lib/stdio.[ch] and lib/stdlib.[ch] files. Implementations for libc
> > functions (like getchar), that have their prototypes in lib/stdio.h, could
> > all get thrown together in lib/stdio.c.
> > 
> > Could be nice to define EOF in lib/stdio.h too.
> 
> Yes, I agree on all accounts!
>

OK, I'll send a libc-ifying clean up series and then rebase patches 5-8
of this series on that series.

Thanks,
drew