* [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
@ 2016-07-21 16:47 Laurent Vivier
2016-07-21 19:08 ` Dr. David Alan Gilbert
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: Laurent Vivier @ 2016-07-21 16:47 UTC (permalink / raw)
To: qemu-devel; +Cc: dgilbert, thuth, dgibson, Laurent Vivier
As userfaultfd syscall is available on powerpc, migration
postcopy can be used.
This patch adds the support needed to test this on powerpc,
instead of using a bootsector to run code to modify memory,
we use a FORTH script in "boot-command" property.
As spapr machine doesn't support "-prom-env" argument
(the nvram is initialized by SLOF and not by QEMU),
"boot-command" is provided to SLOF via a file mapped nvram
(with "-drive file=...,if=pflash")
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
v2: move FORTH script directly in sprintf()
use openbios_firmware_abi.h
remove useless "default" case
tests/Makefile.include | 1 +
tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
2 files changed, 98 insertions(+), 19 deletions(-)
diff --git a/tests/Makefile.include b/tests/Makefile.include
index e7e50d6..e2d1885 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -268,6 +268,7 @@ check-qtest-sparc-y += tests/prom-env-test$(EXESUF)
#check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
+check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
check-qtest-generic-y += tests/qom-test$(EXESUF)
diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c
index 16465ab..229e9e9 100644
--- a/tests/postcopy-test.c
+++ b/tests/postcopy-test.c
@@ -18,6 +18,9 @@
#include "qemu/sockets.h"
#include "sysemu/char.h"
#include "sysemu/sysemu.h"
+#include "hw/nvram/openbios_firmware_abi.h"
+
+#define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */
const unsigned start_address = 1024 * 1024;
const unsigned end_address = 100 * 1024 * 1024;
@@ -122,6 +125,44 @@ unsigned char bootsect[] = {
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa
};
+static void init_bootfile_x86(const char *bootpath)
+{
+ FILE *bootfile = fopen(bootpath, "wb");
+
+ g_assert_cmpint(fwrite(bootsect, 512, 1, bootfile), ==, 1);
+ fclose(bootfile);
+}
+
+static void init_bootfile_ppc(const char *bootpath)
+{
+ FILE *bootfile;
+ char buf[MIN_NVRAM_SIZE];
+ struct OpenBIOS_nvpart_v1 *header = (struct OpenBIOS_nvpart_v1 *)buf;
+
+ memset(buf, 0, MIN_NVRAM_SIZE);
+
+ /* Create a "common" partition in nvram to store boot-command property */
+
+ header->signature = OPENBIOS_PART_SYSTEM;
+ memcpy(header->name, "common", 6);
+ OpenBIOS_finish_partition(header, MIN_NVRAM_SIZE);
+
+ /* FW_MAX_SIZE is 4MB, but slof.bin is only 900KB,
+ * so let's modify memory between 1MB and 100MB
+ * to do like PC bootsector
+ */
+
+ sprintf(buf + 16,
+ "boot-command=hex .\" _\" begin %x %x do i c@ 1 + i c! 1000 +loop "
+ ".\" B\" 0 until", end_address, start_address);
+
+ /* Write partition to the NVRAM file */
+
+ bootfile = fopen(bootpath, "wb");
+ g_assert_cmpint(fwrite(buf, MIN_NVRAM_SIZE, 1, bootfile), ==, 1);
+ fclose(bootfile);
+}
+
/*
* Wait for some output in the serial output file,
* we get an 'A' followed by an endless string of 'B's
@@ -131,10 +172,29 @@ static void wait_for_serial(const char *side)
{
char *serialpath = g_strdup_printf("%s/%s", tmpfs, side);
FILE *serialfile = fopen(serialpath, "r");
+ const char *arch = qtest_get_arch();
+ int started = (strcmp(side, "src_serial") == 0 &&
+ strcmp(arch, "ppc64") == 0) ? 0 : 1;
do {
int readvalue = fgetc(serialfile);
+ if (!started) {
+ /* SLOF prints its banner before starting test,
+ * to ignore it, mark the start of the test with '_',
+ * ignore all characters until this marker
+ */
+ switch (readvalue) {
+ case '_':
+ started = 1;
+ break;
+ case EOF:
+ fseek(serialfile, 0, SEEK_SET);
+ usleep(1000);
+ break;
+ }
+ continue;
+ }
switch (readvalue) {
case 'A':
/* Fine */
@@ -147,6 +207,8 @@ static void wait_for_serial(const char *side)
return;
case EOF:
+ started = (strcmp(side, "src_serial") == 0 &&
+ strcmp(arch, "ppc64") == 0) ? 0 : 1;
fseek(serialfile, 0, SEEK_SET);
usleep(1000);
break;
@@ -295,32 +357,48 @@ static void test_migrate(void)
char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
QTestState *global = global_qtest, *from, *to;
unsigned char dest_byte_a, dest_byte_b, dest_byte_c, dest_byte_d;
- gchar *cmd;
+ gchar *cmd, *cmd_src, *cmd_dst;
QDict *rsp;
char *bootpath = g_strdup_printf("%s/bootsect", tmpfs);
- FILE *bootfile = fopen(bootpath, "wb");
+ const char *arch = qtest_get_arch();
got_stop = false;
- g_assert_cmpint(fwrite(bootsect, 512, 1, bootfile), ==, 1);
- fclose(bootfile);
- cmd = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
- " -name pcsource,debug-threads=on"
- " -serial file:%s/src_serial"
- " -drive file=%s,format=raw",
- tmpfs, bootpath);
- from = qtest_start(cmd);
- g_free(cmd);
+ if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+ init_bootfile_x86(bootpath);
+ cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
+ " -name pcsource,debug-threads=on"
+ " -serial file:%s/src_serial"
+ " -drive file=%s,format=raw",
+ tmpfs, bootpath);
+ cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
+ " -name pcdest,debug-threads=on"
+ " -serial file:%s/dest_serial"
+ " -drive file=%s,format=raw"
+ " -incoming %s",
+ tmpfs, bootpath, uri);
+ } else if (strcmp(arch, "ppc64") == 0) {
+ init_bootfile_ppc(bootpath);
+ cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
+ " -name pcsource,debug-threads=on"
+ " -serial file:%s/src_serial"
+ " -drive file=%s,if=pflash,format=raw",
+ tmpfs, bootpath);
+ cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
+ " -name pcdest,debug-threads=on"
+ " -serial file:%s/dest_serial"
+ " -incoming %s",
+ tmpfs, uri);
+ } else {
+ g_assert_not_reached();
+ }
- cmd = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
- " -name pcdest,debug-threads=on"
- " -serial file:%s/dest_serial"
- " -drive file=%s,format=raw"
- " -incoming %s",
- tmpfs, bootpath, uri);
- to = qtest_init(cmd);
- g_free(cmd);
+ from = qtest_start(cmd_src);
+ g_free(cmd_src);
+
+ to = qtest_init(cmd_dst);
+ g_free(cmd_dst);
global_qtest = from;
rsp = qmp("{ 'execute': 'migrate-set-capabilities',"
--
2.5.5
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-21 16:47 [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64 Laurent Vivier
@ 2016-07-21 19:08 ` Dr. David Alan Gilbert
2016-07-21 20:57 ` Thomas Huth
` (2 subsequent siblings)
3 siblings, 0 replies; 19+ messages in thread
From: Dr. David Alan Gilbert @ 2016-07-21 19:08 UTC (permalink / raw)
To: Laurent Vivier; +Cc: qemu-devel, thuth, dgibson
* Laurent Vivier (lvivier@redhat.com) wrote:
> As userfaultfd syscall is available on powerpc, migration
> postcopy can be used.
>
> This patch adds the support needed to test this on powerpc,
> instead of using a bootsector to run code to modify memory,
> we use a FORTH script in "boot-command" property.
>
> As spapr machine doesn't support "-prom-env" argument
> (the nvram is initialized by SLOF and not by QEMU),
> "boot-command" is provided to SLOF via a file mapped nvram
> (with "-drive file=...,if=pflash")
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Thanks for doing this!
> ---
> v2: move FORTH script directly in sprintf()
> use openbios_firmware_abi.h
> remove useless "default" case
>
> tests/Makefile.include | 1 +
> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
> 2 files changed, 98 insertions(+), 19 deletions(-)
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index e7e50d6..e2d1885 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -268,6 +268,7 @@ check-qtest-sparc-y += tests/prom-env-test$(EXESUF)
> #check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
> check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
> check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
> +check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
>
> check-qtest-generic-y += tests/qom-test$(EXESUF)
>
> diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c
> index 16465ab..229e9e9 100644
> --- a/tests/postcopy-test.c
> +++ b/tests/postcopy-test.c
> @@ -18,6 +18,9 @@
> #include "qemu/sockets.h"
> #include "sysemu/char.h"
> #include "sysemu/sysemu.h"
> +#include "hw/nvram/openbios_firmware_abi.h"
> +
> +#define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */
>
> const unsigned start_address = 1024 * 1024;
> const unsigned end_address = 100 * 1024 * 1024;
> @@ -122,6 +125,44 @@ unsigned char bootsect[] = {
> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa
> };
>
> +static void init_bootfile_x86(const char *bootpath)
> +{
> + FILE *bootfile = fopen(bootpath, "wb");
> +
> + g_assert_cmpint(fwrite(bootsect, 512, 1, bootfile), ==, 1);
> + fclose(bootfile);
> +}
> +
> +static void init_bootfile_ppc(const char *bootpath)
> +{
> + FILE *bootfile;
> + char buf[MIN_NVRAM_SIZE];
> + struct OpenBIOS_nvpart_v1 *header = (struct OpenBIOS_nvpart_v1 *)buf;
> +
> + memset(buf, 0, MIN_NVRAM_SIZE);
> +
> + /* Create a "common" partition in nvram to store boot-command property */
> +
> + header->signature = OPENBIOS_PART_SYSTEM;
> + memcpy(header->name, "common", 6);
> + OpenBIOS_finish_partition(header, MIN_NVRAM_SIZE);
> +
> + /* FW_MAX_SIZE is 4MB, but slof.bin is only 900KB,
> + * so let's modify memory between 1MB and 100MB
> + * to do like PC bootsector
> + */
> +
> + sprintf(buf + 16,
> + "boot-command=hex .\" _\" begin %x %x do i c@ 1 + i c! 1000 +loop "
> + ".\" B\" 0 until", end_address, start_address);
Very nice; took me a while do decode but yes I think that's doing
the same as my x86.
Dave
> + /* Write partition to the NVRAM file */
> +
> + bootfile = fopen(bootpath, "wb");
> + g_assert_cmpint(fwrite(buf, MIN_NVRAM_SIZE, 1, bootfile), ==, 1);
> + fclose(bootfile);
> +}
> +
> /*
> * Wait for some output in the serial output file,
> * we get an 'A' followed by an endless string of 'B's
> @@ -131,10 +172,29 @@ static void wait_for_serial(const char *side)
> {
> char *serialpath = g_strdup_printf("%s/%s", tmpfs, side);
> FILE *serialfile = fopen(serialpath, "r");
> + const char *arch = qtest_get_arch();
> + int started = (strcmp(side, "src_serial") == 0 &&
> + strcmp(arch, "ppc64") == 0) ? 0 : 1;
>
> do {
> int readvalue = fgetc(serialfile);
>
> + if (!started) {
> + /* SLOF prints its banner before starting test,
> + * to ignore it, mark the start of the test with '_',
> + * ignore all characters until this marker
> + */
> + switch (readvalue) {
> + case '_':
> + started = 1;
> + break;
> + case EOF:
> + fseek(serialfile, 0, SEEK_SET);
> + usleep(1000);
> + break;
> + }
> + continue;
> + }
> switch (readvalue) {
> case 'A':
> /* Fine */
> @@ -147,6 +207,8 @@ static void wait_for_serial(const char *side)
> return;
>
> case EOF:
> + started = (strcmp(side, "src_serial") == 0 &&
> + strcmp(arch, "ppc64") == 0) ? 0 : 1;
> fseek(serialfile, 0, SEEK_SET);
> usleep(1000);
> break;
> @@ -295,32 +357,48 @@ static void test_migrate(void)
> char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> QTestState *global = global_qtest, *from, *to;
> unsigned char dest_byte_a, dest_byte_b, dest_byte_c, dest_byte_d;
> - gchar *cmd;
> + gchar *cmd, *cmd_src, *cmd_dst;
> QDict *rsp;
>
> char *bootpath = g_strdup_printf("%s/bootsect", tmpfs);
> - FILE *bootfile = fopen(bootpath, "wb");
> + const char *arch = qtest_get_arch();
>
> got_stop = false;
> - g_assert_cmpint(fwrite(bootsect, 512, 1, bootfile), ==, 1);
> - fclose(bootfile);
>
> - cmd = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> - " -name pcsource,debug-threads=on"
> - " -serial file:%s/src_serial"
> - " -drive file=%s,format=raw",
> - tmpfs, bootpath);
> - from = qtest_start(cmd);
> - g_free(cmd);
> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> + init_bootfile_x86(bootpath);
> + cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> + " -name pcsource,debug-threads=on"
> + " -serial file:%s/src_serial"
> + " -drive file=%s,format=raw",
> + tmpfs, bootpath);
> + cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> + " -name pcdest,debug-threads=on"
> + " -serial file:%s/dest_serial"
> + " -drive file=%s,format=raw"
> + " -incoming %s",
> + tmpfs, bootpath, uri);
> + } else if (strcmp(arch, "ppc64") == 0) {
> + init_bootfile_ppc(bootpath);
> + cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
> + " -name pcsource,debug-threads=on"
> + " -serial file:%s/src_serial"
> + " -drive file=%s,if=pflash,format=raw",
> + tmpfs, bootpath);
> + cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
> + " -name pcdest,debug-threads=on"
> + " -serial file:%s/dest_serial"
> + " -incoming %s",
> + tmpfs, uri);
> + } else {
> + g_assert_not_reached();
> + }
>
> - cmd = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> - " -name pcdest,debug-threads=on"
> - " -serial file:%s/dest_serial"
> - " -drive file=%s,format=raw"
> - " -incoming %s",
> - tmpfs, bootpath, uri);
> - to = qtest_init(cmd);
> - g_free(cmd);
> + from = qtest_start(cmd_src);
> + g_free(cmd_src);
> +
> + to = qtest_init(cmd_dst);
> + g_free(cmd_dst);
>
> global_qtest = from;
> rsp = qmp("{ 'execute': 'migrate-set-capabilities',"
> --
> 2.5.5
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-21 16:47 [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64 Laurent Vivier
2016-07-21 19:08 ` Dr. David Alan Gilbert
@ 2016-07-21 20:57 ` Thomas Huth
2016-07-22 6:43 ` David Gibson
2016-07-27 0:43 ` David Gibson
3 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2016-07-21 20:57 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: dgilbert, dgibson
On 21.07.2016 18:47, Laurent Vivier wrote:
> As userfaultfd syscall is available on powerpc, migration
> postcopy can be used.
>
> This patch adds the support needed to test this on powerpc,
> instead of using a bootsector to run code to modify memory,
> we use a FORTH script in "boot-command" property.
>
> As spapr machine doesn't support "-prom-env" argument
> (the nvram is initialized by SLOF and not by QEMU),
> "boot-command" is provided to SLOF via a file mapped nvram
> (with "-drive file=...,if=pflash")
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> v2: move FORTH script directly in sprintf()
> use openbios_firmware_abi.h
> remove useless "default" case
>
> tests/Makefile.include | 1 +
> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
> 2 files changed, 98 insertions(+), 19 deletions(-)
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index e7e50d6..e2d1885 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -268,6 +268,7 @@ check-qtest-sparc-y += tests/prom-env-test$(EXESUF)
> #check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
> check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
> check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
> +check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
>
> check-qtest-generic-y += tests/qom-test$(EXESUF)
>
> diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c
> index 16465ab..229e9e9 100644
> --- a/tests/postcopy-test.c
> +++ b/tests/postcopy-test.c
> @@ -18,6 +18,9 @@
> #include "qemu/sockets.h"
> #include "sysemu/char.h"
> #include "sysemu/sysemu.h"
> +#include "hw/nvram/openbios_firmware_abi.h"
> +
> +#define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */
>
> const unsigned start_address = 1024 * 1024;
> const unsigned end_address = 100 * 1024 * 1024;
> @@ -122,6 +125,44 @@ unsigned char bootsect[] = {
> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa
> };
>
> +static void init_bootfile_x86(const char *bootpath)
> +{
> + FILE *bootfile = fopen(bootpath, "wb");
> +
> + g_assert_cmpint(fwrite(bootsect, 512, 1, bootfile), ==, 1);
> + fclose(bootfile);
> +}
> +
> +static void init_bootfile_ppc(const char *bootpath)
> +{
> + FILE *bootfile;
> + char buf[MIN_NVRAM_SIZE];
> + struct OpenBIOS_nvpart_v1 *header = (struct OpenBIOS_nvpart_v1 *)buf;
> +
> + memset(buf, 0, MIN_NVRAM_SIZE);
> +
> + /* Create a "common" partition in nvram to store boot-command property */
> +
> + header->signature = OPENBIOS_PART_SYSTEM;
> + memcpy(header->name, "common", 6);
> + OpenBIOS_finish_partition(header, MIN_NVRAM_SIZE);
> +
> + /* FW_MAX_SIZE is 4MB, but slof.bin is only 900KB,
> + * so let's modify memory between 1MB and 100MB
> + * to do like PC bootsector
> + */
> +
> + sprintf(buf + 16,
> + "boot-command=hex .\" _\" begin %x %x do i c@ 1 + i c! 1000 +loop "
> + ".\" B\" 0 until", end_address, start_address);
> +
> + /* Write partition to the NVRAM file */
> +
> + bootfile = fopen(bootpath, "wb");
> + g_assert_cmpint(fwrite(buf, MIN_NVRAM_SIZE, 1, bootfile), ==, 1);
> + fclose(bootfile);
> +}
> +
> /*
> * Wait for some output in the serial output file,
> * we get an 'A' followed by an endless string of 'B's
> @@ -131,10 +172,29 @@ static void wait_for_serial(const char *side)
> {
> char *serialpath = g_strdup_printf("%s/%s", tmpfs, side);
> FILE *serialfile = fopen(serialpath, "r");
> + const char *arch = qtest_get_arch();
> + int started = (strcmp(side, "src_serial") == 0 &&
> + strcmp(arch, "ppc64") == 0) ? 0 : 1;
>
> do {
> int readvalue = fgetc(serialfile);
>
> + if (!started) {
> + /* SLOF prints its banner before starting test,
> + * to ignore it, mark the start of the test with '_',
> + * ignore all characters until this marker
> + */
> + switch (readvalue) {
> + case '_':
> + started = 1;
> + break;
> + case EOF:
> + fseek(serialfile, 0, SEEK_SET);
> + usleep(1000);
> + break;
> + }
> + continue;
> + }
> switch (readvalue) {
> case 'A':
> /* Fine */
> @@ -147,6 +207,8 @@ static void wait_for_serial(const char *side)
> return;
>
> case EOF:
> + started = (strcmp(side, "src_serial") == 0 &&
> + strcmp(arch, "ppc64") == 0) ? 0 : 1;
> fseek(serialfile, 0, SEEK_SET);
> usleep(1000);
> break;
> @@ -295,32 +357,48 @@ static void test_migrate(void)
> char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> QTestState *global = global_qtest, *from, *to;
> unsigned char dest_byte_a, dest_byte_b, dest_byte_c, dest_byte_d;
> - gchar *cmd;
> + gchar *cmd, *cmd_src, *cmd_dst;
> QDict *rsp;
>
> char *bootpath = g_strdup_printf("%s/bootsect", tmpfs);
> - FILE *bootfile = fopen(bootpath, "wb");
> + const char *arch = qtest_get_arch();
>
> got_stop = false;
> - g_assert_cmpint(fwrite(bootsect, 512, 1, bootfile), ==, 1);
> - fclose(bootfile);
>
> - cmd = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> - " -name pcsource,debug-threads=on"
> - " -serial file:%s/src_serial"
> - " -drive file=%s,format=raw",
> - tmpfs, bootpath);
> - from = qtest_start(cmd);
> - g_free(cmd);
> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> + init_bootfile_x86(bootpath);
> + cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> + " -name pcsource,debug-threads=on"
> + " -serial file:%s/src_serial"
> + " -drive file=%s,format=raw",
> + tmpfs, bootpath);
> + cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> + " -name pcdest,debug-threads=on"
> + " -serial file:%s/dest_serial"
> + " -drive file=%s,format=raw"
> + " -incoming %s",
> + tmpfs, bootpath, uri);
> + } else if (strcmp(arch, "ppc64") == 0) {
> + init_bootfile_ppc(bootpath);
> + cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
> + " -name pcsource,debug-threads=on"
> + " -serial file:%s/src_serial"
> + " -drive file=%s,if=pflash,format=raw",
> + tmpfs, bootpath);
> + cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
> + " -name pcdest,debug-threads=on"
> + " -serial file:%s/dest_serial"
> + " -incoming %s",
> + tmpfs, uri);
> + } else {
> + g_assert_not_reached();
> + }
>
> - cmd = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> - " -name pcdest,debug-threads=on"
> - " -serial file:%s/dest_serial"
> - " -drive file=%s,format=raw"
> - " -incoming %s",
> - tmpfs, bootpath, uri);
> - to = qtest_init(cmd);
> - g_free(cmd);
> + from = qtest_start(cmd_src);
> + g_free(cmd_src);
> +
> + to = qtest_init(cmd_dst);
> + g_free(cmd_dst);
>
> global_qtest = from;
> rsp = qmp("{ 'execute': 'migrate-set-capabilities',"
>
Looks fine to me now, thanks for the update!
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-21 16:47 [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64 Laurent Vivier
2016-07-21 19:08 ` Dr. David Alan Gilbert
2016-07-21 20:57 ` Thomas Huth
@ 2016-07-22 6:43 ` David Gibson
2016-07-22 7:28 ` Laurent Vivier
2016-07-27 0:43 ` David Gibson
3 siblings, 1 reply; 19+ messages in thread
From: David Gibson @ 2016-07-22 6:43 UTC (permalink / raw)
To: Laurent Vivier; +Cc: qemu-devel, dgilbert, thuth, dgibson
[-- Attachment #1: Type: text/plain, Size: 8507 bytes --]
On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote:
> As userfaultfd syscall is available on powerpc, migration
> postcopy can be used.
>
> This patch adds the support needed to test this on powerpc,
> instead of using a bootsector to run code to modify memory,
> we use a FORTH script in "boot-command" property.
>
> As spapr machine doesn't support "-prom-env" argument
> (the nvram is initialized by SLOF and not by QEMU),
> "boot-command" is provided to SLOF via a file mapped nvram
> (with "-drive file=...,if=pflash")
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> v2: move FORTH script directly in sprintf()
> use openbios_firmware_abi.h
> remove useless "default" case
>
> tests/Makefile.include | 1 +
> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
> 2 files changed, 98 insertions(+), 19 deletions(-)
There's a mostly cosmetic problem with this. If you run make check
for a ppc64 target on an x86 machine, you get:
GTESTER check-qtest-ppc64
"kvm" accelerator not found.
"kvm" accelerator not found.
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index e7e50d6..e2d1885 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -268,6 +268,7 @@ check-qtest-sparc-y += tests/prom-env-test$(EXESUF)
> #check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
> check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
> check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
> +check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
>
> check-qtest-generic-y += tests/qom-test$(EXESUF)
>
> diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c
> index 16465ab..229e9e9 100644
> --- a/tests/postcopy-test.c
> +++ b/tests/postcopy-test.c
> @@ -18,6 +18,9 @@
> #include "qemu/sockets.h"
> #include "sysemu/char.h"
> #include "sysemu/sysemu.h"
> +#include "hw/nvram/openbios_firmware_abi.h"
> +
> +#define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */
>
> const unsigned start_address = 1024 * 1024;
> const unsigned end_address = 100 * 1024 * 1024;
> @@ -122,6 +125,44 @@ unsigned char bootsect[] = {
> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa
> };
>
> +static void init_bootfile_x86(const char *bootpath)
> +{
> + FILE *bootfile = fopen(bootpath, "wb");
> +
> + g_assert_cmpint(fwrite(bootsect, 512, 1, bootfile), ==, 1);
> + fclose(bootfile);
> +}
> +
> +static void init_bootfile_ppc(const char *bootpath)
> +{
> + FILE *bootfile;
> + char buf[MIN_NVRAM_SIZE];
> + struct OpenBIOS_nvpart_v1 *header = (struct OpenBIOS_nvpart_v1 *)buf;
> +
> + memset(buf, 0, MIN_NVRAM_SIZE);
> +
> + /* Create a "common" partition in nvram to store boot-command property */
> +
> + header->signature = OPENBIOS_PART_SYSTEM;
> + memcpy(header->name, "common", 6);
> + OpenBIOS_finish_partition(header, MIN_NVRAM_SIZE);
> +
> + /* FW_MAX_SIZE is 4MB, but slof.bin is only 900KB,
> + * so let's modify memory between 1MB and 100MB
> + * to do like PC bootsector
> + */
> +
> + sprintf(buf + 16,
> + "boot-command=hex .\" _\" begin %x %x do i c@ 1 + i c! 1000 +loop "
> + ".\" B\" 0 until", end_address, start_address);
> +
> + /* Write partition to the NVRAM file */
> +
> + bootfile = fopen(bootpath, "wb");
> + g_assert_cmpint(fwrite(buf, MIN_NVRAM_SIZE, 1, bootfile), ==, 1);
> + fclose(bootfile);
> +}
> +
> /*
> * Wait for some output in the serial output file,
> * we get an 'A' followed by an endless string of 'B's
> @@ -131,10 +172,29 @@ static void wait_for_serial(const char *side)
> {
> char *serialpath = g_strdup_printf("%s/%s", tmpfs, side);
> FILE *serialfile = fopen(serialpath, "r");
> + const char *arch = qtest_get_arch();
> + int started = (strcmp(side, "src_serial") == 0 &&
> + strcmp(arch, "ppc64") == 0) ? 0 : 1;
>
> do {
> int readvalue = fgetc(serialfile);
>
> + if (!started) {
> + /* SLOF prints its banner before starting test,
> + * to ignore it, mark the start of the test with '_',
> + * ignore all characters until this marker
> + */
> + switch (readvalue) {
> + case '_':
> + started = 1;
> + break;
> + case EOF:
> + fseek(serialfile, 0, SEEK_SET);
> + usleep(1000);
> + break;
> + }
> + continue;
> + }
> switch (readvalue) {
> case 'A':
> /* Fine */
> @@ -147,6 +207,8 @@ static void wait_for_serial(const char *side)
> return;
>
> case EOF:
> + started = (strcmp(side, "src_serial") == 0 &&
> + strcmp(arch, "ppc64") == 0) ? 0 : 1;
> fseek(serialfile, 0, SEEK_SET);
> usleep(1000);
> break;
> @@ -295,32 +357,48 @@ static void test_migrate(void)
> char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> QTestState *global = global_qtest, *from, *to;
> unsigned char dest_byte_a, dest_byte_b, dest_byte_c, dest_byte_d;
> - gchar *cmd;
> + gchar *cmd, *cmd_src, *cmd_dst;
> QDict *rsp;
>
> char *bootpath = g_strdup_printf("%s/bootsect", tmpfs);
> - FILE *bootfile = fopen(bootpath, "wb");
> + const char *arch = qtest_get_arch();
>
> got_stop = false;
> - g_assert_cmpint(fwrite(bootsect, 512, 1, bootfile), ==, 1);
> - fclose(bootfile);
>
> - cmd = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> - " -name pcsource,debug-threads=on"
> - " -serial file:%s/src_serial"
> - " -drive file=%s,format=raw",
> - tmpfs, bootpath);
> - from = qtest_start(cmd);
> - g_free(cmd);
> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> + init_bootfile_x86(bootpath);
> + cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> + " -name pcsource,debug-threads=on"
> + " -serial file:%s/src_serial"
> + " -drive file=%s,format=raw",
> + tmpfs, bootpath);
> + cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> + " -name pcdest,debug-threads=on"
> + " -serial file:%s/dest_serial"
> + " -drive file=%s,format=raw"
> + " -incoming %s",
> + tmpfs, bootpath, uri);
> + } else if (strcmp(arch, "ppc64") == 0) {
> + init_bootfile_ppc(bootpath);
> + cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
> + " -name pcsource,debug-threads=on"
> + " -serial file:%s/src_serial"
> + " -drive file=%s,if=pflash,format=raw",
> + tmpfs, bootpath);
> + cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
> + " -name pcdest,debug-threads=on"
> + " -serial file:%s/dest_serial"
> + " -incoming %s",
> + tmpfs, uri);
> + } else {
> + g_assert_not_reached();
> + }
>
> - cmd = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> - " -name pcdest,debug-threads=on"
> - " -serial file:%s/dest_serial"
> - " -drive file=%s,format=raw"
> - " -incoming %s",
> - tmpfs, bootpath, uri);
> - to = qtest_init(cmd);
> - g_free(cmd);
> + from = qtest_start(cmd_src);
> + g_free(cmd_src);
> +
> + to = qtest_init(cmd_dst);
> + g_free(cmd_dst);
>
> global_qtest = from;
> rsp = qmp("{ 'execute': 'migrate-set-capabilities',"
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-22 6:43 ` David Gibson
@ 2016-07-22 7:28 ` Laurent Vivier
2016-07-23 6:30 ` David Gibson
0 siblings, 1 reply; 19+ messages in thread
From: Laurent Vivier @ 2016-07-22 7:28 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-devel, dgilbert, thuth, dgibson
On 22/07/2016 08:43, David Gibson wrote:
> On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote:
>> As userfaultfd syscall is available on powerpc, migration
>> postcopy can be used.
>>
>> This patch adds the support needed to test this on powerpc,
>> instead of using a bootsector to run code to modify memory,
>> we use a FORTH script in "boot-command" property.
>>
>> As spapr machine doesn't support "-prom-env" argument
>> (the nvram is initialized by SLOF and not by QEMU),
>> "boot-command" is provided to SLOF via a file mapped nvram
>> (with "-drive file=...,if=pflash")
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>> v2: move FORTH script directly in sprintf()
>> use openbios_firmware_abi.h
>> remove useless "default" case
>>
>> tests/Makefile.include | 1 +
>> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
>> 2 files changed, 98 insertions(+), 19 deletions(-)
>
> There's a mostly cosmetic problem with this. If you run make check
> for a ppc64 target on an x86 machine, you get:
>
> GTESTER check-qtest-ppc64
> "kvm" accelerator not found.
> "kvm" accelerator not found.
I think this is because of "-machine accel=kvm:tcg", it tries to use kvm
and fall back to tcg.
accel.c:
80 void configure_accelerator(MachineState *ms)
81 {
...
100 acc = accel_find(buf);
101 if (!acc) {
102 fprintf(stderr, "\"%s\" accelerator not found.\n", buf);
103 continue;
104 }
We can remove the "-machine" argument to use the default instead (tcg or
kvm).
Laurent
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-22 7:28 ` Laurent Vivier
@ 2016-07-23 6:30 ` David Gibson
2016-07-26 9:23 ` Laurent Vivier
0 siblings, 1 reply; 19+ messages in thread
From: David Gibson @ 2016-07-23 6:30 UTC (permalink / raw)
To: Laurent Vivier; +Cc: qemu-devel, dgilbert, thuth, dgibson
[-- Attachment #1: Type: text/plain, Size: 2074 bytes --]
On Fri, Jul 22, 2016 at 09:28:58AM +0200, Laurent Vivier wrote:
>
>
> On 22/07/2016 08:43, David Gibson wrote:
> > On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote:
> >> As userfaultfd syscall is available on powerpc, migration
> >> postcopy can be used.
> >>
> >> This patch adds the support needed to test this on powerpc,
> >> instead of using a bootsector to run code to modify memory,
> >> we use a FORTH script in "boot-command" property.
> >>
> >> As spapr machine doesn't support "-prom-env" argument
> >> (the nvram is initialized by SLOF and not by QEMU),
> >> "boot-command" is provided to SLOF via a file mapped nvram
> >> (with "-drive file=...,if=pflash")
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >> ---
> >> v2: move FORTH script directly in sprintf()
> >> use openbios_firmware_abi.h
> >> remove useless "default" case
> >>
> >> tests/Makefile.include | 1 +
> >> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
> >> 2 files changed, 98 insertions(+), 19 deletions(-)
> >
> > There's a mostly cosmetic problem with this. If you run make check
> > for a ppc64 target on an x86 machine, you get:
> >
> > GTESTER check-qtest-ppc64
> > "kvm" accelerator not found.
> > "kvm" accelerator not found.
>
> I think this is because of "-machine accel=kvm:tcg", it tries to use kvm
> and fall back to tcg.
>
> accel.c:
>
> 80 void configure_accelerator(MachineState *ms)
> 81 {
> ...
> 100 acc = accel_find(buf);
> 101 if (!acc) {
> 102 fprintf(stderr, "\"%s\" accelerator not found.\n", buf);
> 103 continue;
> 104 }
>
> We can remove the "-machine" argument to use the default instead (tcg or
> kvm).
That sounds like a good option for a general test.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-23 6:30 ` David Gibson
@ 2016-07-26 9:23 ` Laurent Vivier
2016-07-26 9:28 ` Thomas Huth
0 siblings, 1 reply; 19+ messages in thread
From: Laurent Vivier @ 2016-07-26 9:23 UTC (permalink / raw)
To: David Gibson; +Cc: dgibson, thuth, qemu-devel, dgilbert
On 23/07/2016 08:30, David Gibson wrote:
> On Fri, Jul 22, 2016 at 09:28:58AM +0200, Laurent Vivier wrote:
>>
>>
>> On 22/07/2016 08:43, David Gibson wrote:
>>> On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote:
>>>> As userfaultfd syscall is available on powerpc, migration
>>>> postcopy can be used.
>>>>
>>>> This patch adds the support needed to test this on powerpc,
>>>> instead of using a bootsector to run code to modify memory,
>>>> we use a FORTH script in "boot-command" property.
>>>>
>>>> As spapr machine doesn't support "-prom-env" argument
>>>> (the nvram is initialized by SLOF and not by QEMU),
>>>> "boot-command" is provided to SLOF via a file mapped nvram
>>>> (with "-drive file=...,if=pflash")
>>>>
>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>> ---
>>>> v2: move FORTH script directly in sprintf()
>>>> use openbios_firmware_abi.h
>>>> remove useless "default" case
>>>>
>>>> tests/Makefile.include | 1 +
>>>> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
>>>> 2 files changed, 98 insertions(+), 19 deletions(-)
>>>
>>> There's a mostly cosmetic problem with this. If you run make check
>>> for a ppc64 target on an x86 machine, you get:
>>>
>>> GTESTER check-qtest-ppc64
>>> "kvm" accelerator not found.
>>> "kvm" accelerator not found.
>>
>> I think this is because of "-machine accel=kvm:tcg", it tries to use kvm
>> and fall back to tcg.
>>
>> accel.c:
>>
>> 80 void configure_accelerator(MachineState *ms)
>> 81 {
>> ...
>> 100 acc = accel_find(buf);
>> 101 if (!acc) {
>> 102 fprintf(stderr, "\"%s\" accelerator not found.\n", buf);
>> 103 continue;
>> 104 }
>>
>> We can remove the "-machine" argument to use the default instead (tcg or
>> kvm).
>
> That sounds like a good option for a general test.
>
In fact, we can't: we need to add a "-machine accel=XXXX" to our command
line to override the "-machine accel=qtest" provided by the qtest
framework. If we don't override it, the machine doesn't start.
Laurent
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-26 9:23 ` Laurent Vivier
@ 2016-07-26 9:28 ` Thomas Huth
2016-07-26 9:39 ` Laurent Vivier
0 siblings, 1 reply; 19+ messages in thread
From: Thomas Huth @ 2016-07-26 9:28 UTC (permalink / raw)
To: Laurent Vivier, David Gibson; +Cc: dgibson, qemu-devel, dgilbert
On 26.07.2016 11:23, Laurent Vivier wrote:
>
>
> On 23/07/2016 08:30, David Gibson wrote:
>> On Fri, Jul 22, 2016 at 09:28:58AM +0200, Laurent Vivier wrote:
>>>
>>>
>>> On 22/07/2016 08:43, David Gibson wrote:
>>>> On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote:
>>>>> As userfaultfd syscall is available on powerpc, migration
>>>>> postcopy can be used.
>>>>>
>>>>> This patch adds the support needed to test this on powerpc,
>>>>> instead of using a bootsector to run code to modify memory,
>>>>> we use a FORTH script in "boot-command" property.
>>>>>
>>>>> As spapr machine doesn't support "-prom-env" argument
>>>>> (the nvram is initialized by SLOF and not by QEMU),
>>>>> "boot-command" is provided to SLOF via a file mapped nvram
>>>>> (with "-drive file=...,if=pflash")
>>>>>
>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>>> ---
>>>>> v2: move FORTH script directly in sprintf()
>>>>> use openbios_firmware_abi.h
>>>>> remove useless "default" case
>>>>>
>>>>> tests/Makefile.include | 1 +
>>>>> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
>>>>> 2 files changed, 98 insertions(+), 19 deletions(-)
>>>>
>>>> There's a mostly cosmetic problem with this. If you run make check
>>>> for a ppc64 target on an x86 machine, you get:
>>>>
>>>> GTESTER check-qtest-ppc64
>>>> "kvm" accelerator not found.
>>>> "kvm" accelerator not found.
>>>
>>> I think this is because of "-machine accel=kvm:tcg", it tries to use kvm
>>> and fall back to tcg.
>>>
>>> accel.c:
>>>
>>> 80 void configure_accelerator(MachineState *ms)
>>> 81 {
>>> ...
>>> 100 acc = accel_find(buf);
>>> 101 if (!acc) {
>>> 102 fprintf(stderr, "\"%s\" accelerator not found.\n", buf);
>>> 103 continue;
>>> 104 }
>>>
>>> We can remove the "-machine" argument to use the default instead (tcg or
>>> kvm).
>>
>> That sounds like a good option for a general test.
>
> In fact, we can't: we need to add a "-machine accel=XXXX" to our command
> line to override the "-machine accel=qtest" provided by the qtest
> framework. If we don't override it, the machine doesn't start.
Would it work if you'd added some magic with "#ifdef CONFIG_KVM" here?
Thomas
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-26 9:28 ` Thomas Huth
@ 2016-07-26 9:39 ` Laurent Vivier
2016-07-26 9:53 ` Laurent Vivier
0 siblings, 1 reply; 19+ messages in thread
From: Laurent Vivier @ 2016-07-26 9:39 UTC (permalink / raw)
To: Thomas Huth, David Gibson; +Cc: dgibson, qemu-devel, dgilbert
On 26/07/2016 11:28, Thomas Huth wrote:
> On 26.07.2016 11:23, Laurent Vivier wrote:
>>
>>
>> On 23/07/2016 08:30, David Gibson wrote:
>>> On Fri, Jul 22, 2016 at 09:28:58AM +0200, Laurent Vivier wrote:
>>>>
>>>>
>>>> On 22/07/2016 08:43, David Gibson wrote:
>>>>> On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote:
>>>>>> As userfaultfd syscall is available on powerpc, migration
>>>>>> postcopy can be used.
>>>>>>
>>>>>> This patch adds the support needed to test this on powerpc,
>>>>>> instead of using a bootsector to run code to modify memory,
>>>>>> we use a FORTH script in "boot-command" property.
>>>>>>
>>>>>> As spapr machine doesn't support "-prom-env" argument
>>>>>> (the nvram is initialized by SLOF and not by QEMU),
>>>>>> "boot-command" is provided to SLOF via a file mapped nvram
>>>>>> (with "-drive file=...,if=pflash")
>>>>>>
>>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>>>> ---
>>>>>> v2: move FORTH script directly in sprintf()
>>>>>> use openbios_firmware_abi.h
>>>>>> remove useless "default" case
>>>>>>
>>>>>> tests/Makefile.include | 1 +
>>>>>> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
>>>>>> 2 files changed, 98 insertions(+), 19 deletions(-)
>>>>>
>>>>> There's a mostly cosmetic problem with this. If you run make check
>>>>> for a ppc64 target on an x86 machine, you get:
>>>>>
>>>>> GTESTER check-qtest-ppc64
>>>>> "kvm" accelerator not found.
>>>>> "kvm" accelerator not found.
>>>>
>>>> I think this is because of "-machine accel=kvm:tcg", it tries to use kvm
>>>> and fall back to tcg.
>>>>
>>>> accel.c:
>>>>
>>>> 80 void configure_accelerator(MachineState *ms)
>>>> 81 {
>>>> ...
>>>> 100 acc = accel_find(buf);
>>>> 101 if (!acc) {
>>>> 102 fprintf(stderr, "\"%s\" accelerator not found.\n", buf);
>>>> 103 continue;
>>>> 104 }
>>>>
>>>> We can remove the "-machine" argument to use the default instead (tcg or
>>>> kvm).
>>>
>>> That sounds like a good option for a general test.
>>
>> In fact, we can't: we need to add a "-machine accel=XXXX" to our command
>> line to override the "-machine accel=qtest" provided by the qtest
>> framework. If we don't override it, the machine doesn't start.
>
> Would it work if you'd added some magic with "#ifdef CONFIG_KVM" here?
I think it needs to be dynamic as the same binary test is used on x86 to
test x86 and ppc64, and vice-versa. I'm going to check if we have
something like "qtest_get_accel()"...
Thanks,
Laurent
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-26 9:39 ` Laurent Vivier
@ 2016-07-26 9:53 ` Laurent Vivier
2016-07-26 9:54 ` Dr. David Alan Gilbert
2016-07-26 10:02 ` Thomas Huth
0 siblings, 2 replies; 19+ messages in thread
From: Laurent Vivier @ 2016-07-26 9:53 UTC (permalink / raw)
To: Thomas Huth, David Gibson; +Cc: dgibson, qemu-devel, dgilbert
On 26/07/2016 11:39, Laurent Vivier wrote:
>
>
> On 26/07/2016 11:28, Thomas Huth wrote:
>> On 26.07.2016 11:23, Laurent Vivier wrote:
>>>
>>>
>>> On 23/07/2016 08:30, David Gibson wrote:
>>>> On Fri, Jul 22, 2016 at 09:28:58AM +0200, Laurent Vivier wrote:
>>>>>
>>>>>
>>>>> On 22/07/2016 08:43, David Gibson wrote:
>>>>>> On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote:
>>>>>>> As userfaultfd syscall is available on powerpc, migration
>>>>>>> postcopy can be used.
>>>>>>>
>>>>>>> This patch adds the support needed to test this on powerpc,
>>>>>>> instead of using a bootsector to run code to modify memory,
>>>>>>> we use a FORTH script in "boot-command" property.
>>>>>>>
>>>>>>> As spapr machine doesn't support "-prom-env" argument
>>>>>>> (the nvram is initialized by SLOF and not by QEMU),
>>>>>>> "boot-command" is provided to SLOF via a file mapped nvram
>>>>>>> (with "-drive file=...,if=pflash")
>>>>>>>
>>>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>>>>> ---
>>>>>>> v2: move FORTH script directly in sprintf()
>>>>>>> use openbios_firmware_abi.h
>>>>>>> remove useless "default" case
>>>>>>>
>>>>>>> tests/Makefile.include | 1 +
>>>>>>> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
>>>>>>> 2 files changed, 98 insertions(+), 19 deletions(-)
>>>>>>
>>>>>> There's a mostly cosmetic problem with this. If you run make check
>>>>>> for a ppc64 target on an x86 machine, you get:
>>>>>>
>>>>>> GTESTER check-qtest-ppc64
>>>>>> "kvm" accelerator not found.
>>>>>> "kvm" accelerator not found.
>>>>>
>>>>> I think this is because of "-machine accel=kvm:tcg", it tries to use kvm
>>>>> and fall back to tcg.
>>>>>
>>>>> accel.c:
>>>>>
>>>>> 80 void configure_accelerator(MachineState *ms)
>>>>> 81 {
>>>>> ...
>>>>> 100 acc = accel_find(buf);
>>>>> 101 if (!acc) {
>>>>> 102 fprintf(stderr, "\"%s\" accelerator not found.\n", buf);
>>>>> 103 continue;
>>>>> 104 }
>>>>>
>>>>> We can remove the "-machine" argument to use the default instead (tcg or
>>>>> kvm).
>>>>
>>>> That sounds like a good option for a general test.
>>>
>>> In fact, we can't: we need to add a "-machine accel=XXXX" to our command
>>> line to override the "-machine accel=qtest" provided by the qtest
>>> framework. If we don't override it, the machine doesn't start.
>>
>> Would it work if you'd added some magic with "#ifdef CONFIG_KVM" here?
>
> I think it needs to be dynamic as the same binary test is used on x86 to
> test x86 and ppc64, and vice-versa. I'm going to check if we have
> something like "qtest_get_accel()"...
Something like that should work:
--- a/tests/postcopy-test.c
+++ b/tests/postcopy-test.c
@@ -380,12 +380,17 @@ static void test_migrate(void)
tmpfs, bootpath, uri);
} else if (strcmp(arch, "ppc64") == 0) {
init_bootfile_ppc(bootpath);
- cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
+#ifdef _ARCH_PPC64
+#define QEMU_CMD_ACCEL "-machine accel=kvm:tcg"
+#else
+#define QEMU_CMD_ACCEL "-machine accel=tcg"
+#endif
+ cmd_src = g_strdup_printf(QEMU_CMD_ACCEL " -m 256M"
" -name pcsource,debug-threads=on"
" -serial file:%s/src_serial"
" -drive file=%s,if=pflash,format=raw",
tmpfs, bootpath);
- cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
+ cmd_dst = g_strdup_printf(QEMU_CMD_ACCEL " -m 256M"
" -name pcdest,debug-threads=on"
" -serial file:%s/dest_serial"
" -incoming %s",
Laurent
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-26 9:53 ` Laurent Vivier
@ 2016-07-26 9:54 ` Dr. David Alan Gilbert
2016-07-26 9:58 ` Laurent Vivier
2016-07-26 10:02 ` Thomas Huth
1 sibling, 1 reply; 19+ messages in thread
From: Dr. David Alan Gilbert @ 2016-07-26 9:54 UTC (permalink / raw)
To: Laurent Vivier; +Cc: Thomas Huth, David Gibson, dgibson, qemu-devel
* Laurent Vivier (lvivier@redhat.com) wrote:
>
>
> On 26/07/2016 11:39, Laurent Vivier wrote:
> >
> >
> > On 26/07/2016 11:28, Thomas Huth wrote:
> >> On 26.07.2016 11:23, Laurent Vivier wrote:
> >>>
> >>>
> >>> On 23/07/2016 08:30, David Gibson wrote:
> >>>> On Fri, Jul 22, 2016 at 09:28:58AM +0200, Laurent Vivier wrote:
> >>>>>
> >>>>>
> >>>>> On 22/07/2016 08:43, David Gibson wrote:
> >>>>>> On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote:
> >>>>>>> As userfaultfd syscall is available on powerpc, migration
> >>>>>>> postcopy can be used.
> >>>>>>>
> >>>>>>> This patch adds the support needed to test this on powerpc,
> >>>>>>> instead of using a bootsector to run code to modify memory,
> >>>>>>> we use a FORTH script in "boot-command" property.
> >>>>>>>
> >>>>>>> As spapr machine doesn't support "-prom-env" argument
> >>>>>>> (the nvram is initialized by SLOF and not by QEMU),
> >>>>>>> "boot-command" is provided to SLOF via a file mapped nvram
> >>>>>>> (with "-drive file=...,if=pflash")
> >>>>>>>
> >>>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >>>>>>> ---
> >>>>>>> v2: move FORTH script directly in sprintf()
> >>>>>>> use openbios_firmware_abi.h
> >>>>>>> remove useless "default" case
> >>>>>>>
> >>>>>>> tests/Makefile.include | 1 +
> >>>>>>> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
> >>>>>>> 2 files changed, 98 insertions(+), 19 deletions(-)
> >>>>>>
> >>>>>> There's a mostly cosmetic problem with this. If you run make check
> >>>>>> for a ppc64 target on an x86 machine, you get:
> >>>>>>
> >>>>>> GTESTER check-qtest-ppc64
> >>>>>> "kvm" accelerator not found.
> >>>>>> "kvm" accelerator not found.
> >>>>>
> >>>>> I think this is because of "-machine accel=kvm:tcg", it tries to use kvm
> >>>>> and fall back to tcg.
> >>>>>
> >>>>> accel.c:
> >>>>>
> >>>>> 80 void configure_accelerator(MachineState *ms)
> >>>>> 81 {
> >>>>> ...
> >>>>> 100 acc = accel_find(buf);
> >>>>> 101 if (!acc) {
> >>>>> 102 fprintf(stderr, "\"%s\" accelerator not found.\n", buf);
> >>>>> 103 continue;
> >>>>> 104 }
> >>>>>
> >>>>> We can remove the "-machine" argument to use the default instead (tcg or
> >>>>> kvm).
> >>>>
> >>>> That sounds like a good option for a general test.
> >>>
> >>> In fact, we can't: we need to add a "-machine accel=XXXX" to our command
> >>> line to override the "-machine accel=qtest" provided by the qtest
> >>> framework. If we don't override it, the machine doesn't start.
> >>
> >> Would it work if you'd added some magic with "#ifdef CONFIG_KVM" here?
> >
> > I think it needs to be dynamic as the same binary test is used on x86 to
> > test x86 and ppc64, and vice-versa. I'm going to check if we have
> > something like "qtest_get_accel()"...
>
> Something like that should work:
>
> --- a/tests/postcopy-test.c
> +++ b/tests/postcopy-test.c
> @@ -380,12 +380,17 @@ static void test_migrate(void)
> tmpfs, bootpath, uri);
> } else if (strcmp(arch, "ppc64") == 0) {
> init_bootfile_ppc(bootpath);
> - cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
> +#ifdef _ARCH_PPC64
> +#define QEMU_CMD_ACCEL "-machine accel=kvm:tcg"
> +#else
> +#define QEMU_CMD_ACCEL "-machine accel=tcg"
> +#endif
> + cmd_src = g_strdup_printf(QEMU_CMD_ACCEL " -m 256M"
> " -name pcsource,debug-threads=on"
> " -serial file:%s/src_serial"
> " -drive file=%s,if=pflash,format=raw",
> tmpfs, bootpath);
> - cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
> + cmd_dst = g_strdup_printf(QEMU_CMD_ACCEL " -m 256M"
> " -name pcdest,debug-threads=on"
> " -serial file:%s/dest_serial"
> " -incoming %s",
>
> Laurent
Is it worth the hastle to just get rid of the two warnings?
Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-26 9:54 ` Dr. David Alan Gilbert
@ 2016-07-26 9:58 ` Laurent Vivier
2016-07-26 11:50 ` David Gibson
0 siblings, 1 reply; 19+ messages in thread
From: Laurent Vivier @ 2016-07-26 9:58 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: Thomas Huth, David Gibson, dgibson, qemu-devel
On 26/07/2016 11:54, Dr. David Alan Gilbert wrote:
> * Laurent Vivier (lvivier@redhat.com) wrote:
>>
>>
>> On 26/07/2016 11:39, Laurent Vivier wrote:
>>>
>>>
>>> On 26/07/2016 11:28, Thomas Huth wrote:
>>>> On 26.07.2016 11:23, Laurent Vivier wrote:
>>>>>
>>>>>
>>>>> On 23/07/2016 08:30, David Gibson wrote:
>>>>>> On Fri, Jul 22, 2016 at 09:28:58AM +0200, Laurent Vivier wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 22/07/2016 08:43, David Gibson wrote:
>>>>>>>> On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote:
>>>>>>>>> As userfaultfd syscall is available on powerpc, migration
>>>>>>>>> postcopy can be used.
>>>>>>>>>
>>>>>>>>> This patch adds the support needed to test this on powerpc,
>>>>>>>>> instead of using a bootsector to run code to modify memory,
>>>>>>>>> we use a FORTH script in "boot-command" property.
>>>>>>>>>
>>>>>>>>> As spapr machine doesn't support "-prom-env" argument
>>>>>>>>> (the nvram is initialized by SLOF and not by QEMU),
>>>>>>>>> "boot-command" is provided to SLOF via a file mapped nvram
>>>>>>>>> (with "-drive file=...,if=pflash")
>>>>>>>>>
>>>>>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>>>>>>> ---
>>>>>>>>> v2: move FORTH script directly in sprintf()
>>>>>>>>> use openbios_firmware_abi.h
>>>>>>>>> remove useless "default" case
>>>>>>>>>
>>>>>>>>> tests/Makefile.include | 1 +
>>>>>>>>> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
>>>>>>>>> 2 files changed, 98 insertions(+), 19 deletions(-)
>>>>>>>>
>>>>>>>> There's a mostly cosmetic problem with this. If you run make check
>>>>>>>> for a ppc64 target on an x86 machine, you get:
>>>>>>>>
>>>>>>>> GTESTER check-qtest-ppc64
>>>>>>>> "kvm" accelerator not found.
>>>>>>>> "kvm" accelerator not found.
>>>>>>>
>>>>>>> I think this is because of "-machine accel=kvm:tcg", it tries to use kvm
>>>>>>> and fall back to tcg.
>>>>>>>
>>>>>>> accel.c:
>>>>>>>
>>>>>>> 80 void configure_accelerator(MachineState *ms)
>>>>>>> 81 {
>>>>>>> ...
>>>>>>> 100 acc = accel_find(buf);
>>>>>>> 101 if (!acc) {
>>>>>>> 102 fprintf(stderr, "\"%s\" accelerator not found.\n", buf);
>>>>>>> 103 continue;
>>>>>>> 104 }
>>>>>>>
>>>>>>> We can remove the "-machine" argument to use the default instead (tcg or
>>>>>>> kvm).
>>>>>>
>>>>>> That sounds like a good option for a general test.
>>>>>
>>>>> In fact, we can't: we need to add a "-machine accel=XXXX" to our command
>>>>> line to override the "-machine accel=qtest" provided by the qtest
>>>>> framework. If we don't override it, the machine doesn't start.
>>>>
>>>> Would it work if you'd added some magic with "#ifdef CONFIG_KVM" here?
>>>
>>> I think it needs to be dynamic as the same binary test is used on x86 to
>>> test x86 and ppc64, and vice-versa. I'm going to check if we have
>>> something like "qtest_get_accel()"...
>>
>> Something like that should work:
>>
>> --- a/tests/postcopy-test.c
>> +++ b/tests/postcopy-test.c
>> @@ -380,12 +380,17 @@ static void test_migrate(void)
>> tmpfs, bootpath, uri);
>> } else if (strcmp(arch, "ppc64") == 0) {
>> init_bootfile_ppc(bootpath);
>> - cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
>> +#ifdef _ARCH_PPC64
>> +#define QEMU_CMD_ACCEL "-machine accel=kvm:tcg"
>> +#else
>> +#define QEMU_CMD_ACCEL "-machine accel=tcg"
>> +#endif
>> + cmd_src = g_strdup_printf(QEMU_CMD_ACCEL " -m 256M"
>> " -name pcsource,debug-threads=on"
>> " -serial file:%s/src_serial"
>> " -drive file=%s,if=pflash,format=raw",
>> tmpfs, bootpath);
>> - cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
>> + cmd_dst = g_strdup_printf(QEMU_CMD_ACCEL " -m 256M"
>> " -name pcdest,debug-threads=on"
>> " -serial file:%s/dest_serial"
>> " -incoming %s",
>>
>> Laurent
>
> Is it worth the hastle to just get rid of the two warnings?
I don't know, it's why I'd like to have the opinion of David.
Laurent
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-26 9:53 ` Laurent Vivier
2016-07-26 9:54 ` Dr. David Alan Gilbert
@ 2016-07-26 10:02 ` Thomas Huth
2016-07-26 12:53 ` Laurent Vivier
` (2 more replies)
1 sibling, 3 replies; 19+ messages in thread
From: Thomas Huth @ 2016-07-26 10:02 UTC (permalink / raw)
To: Laurent Vivier, David Gibson; +Cc: dgibson, qemu-devel, dgilbert
On 26.07.2016 11:53, Laurent Vivier wrote:
>
>
> On 26/07/2016 11:39, Laurent Vivier wrote:
>>
>>
>> On 26/07/2016 11:28, Thomas Huth wrote:
>>> On 26.07.2016 11:23, Laurent Vivier wrote:
>>>>
>>>>
>>>> On 23/07/2016 08:30, David Gibson wrote:
>>>>> On Fri, Jul 22, 2016 at 09:28:58AM +0200, Laurent Vivier wrote:
>>>>>>
>>>>>>
>>>>>> On 22/07/2016 08:43, David Gibson wrote:
>>>>>>> On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote:
>>>>>>>> As userfaultfd syscall is available on powerpc, migration
>>>>>>>> postcopy can be used.
>>>>>>>>
>>>>>>>> This patch adds the support needed to test this on powerpc,
>>>>>>>> instead of using a bootsector to run code to modify memory,
>>>>>>>> we use a FORTH script in "boot-command" property.
>>>>>>>>
>>>>>>>> As spapr machine doesn't support "-prom-env" argument
>>>>>>>> (the nvram is initialized by SLOF and not by QEMU),
>>>>>>>> "boot-command" is provided to SLOF via a file mapped nvram
>>>>>>>> (with "-drive file=...,if=pflash")
>>>>>>>>
>>>>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>>>>>> ---
>>>>>>>> v2: move FORTH script directly in sprintf()
>>>>>>>> use openbios_firmware_abi.h
>>>>>>>> remove useless "default" case
>>>>>>>>
>>>>>>>> tests/Makefile.include | 1 +
>>>>>>>> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
>>>>>>>> 2 files changed, 98 insertions(+), 19 deletions(-)
>>>>>>>
>>>>>>> There's a mostly cosmetic problem with this. If you run make check
>>>>>>> for a ppc64 target on an x86 machine, you get:
>>>>>>>
>>>>>>> GTESTER check-qtest-ppc64
>>>>>>> "kvm" accelerator not found.
>>>>>>> "kvm" accelerator not found.
>>>>>>
>>>>>> I think this is because of "-machine accel=kvm:tcg", it tries to use kvm
>>>>>> and fall back to tcg.
>>>>>>
>>>>>> accel.c:
>>>>>>
>>>>>> 80 void configure_accelerator(MachineState *ms)
>>>>>> 81 {
>>>>>> ...
>>>>>> 100 acc = accel_find(buf);
>>>>>> 101 if (!acc) {
>>>>>> 102 fprintf(stderr, "\"%s\" accelerator not found.\n", buf);
>>>>>> 103 continue;
>>>>>> 104 }
>>>>>>
>>>>>> We can remove the "-machine" argument to use the default instead (tcg or
>>>>>> kvm).
>>>>>
>>>>> That sounds like a good option for a general test.
>>>>
>>>> In fact, we can't: we need to add a "-machine accel=XXXX" to our command
>>>> line to override the "-machine accel=qtest" provided by the qtest
>>>> framework. If we don't override it, the machine doesn't start.
>>>
>>> Would it work if you'd added some magic with "#ifdef CONFIG_KVM" here?
>>
>> I think it needs to be dynamic as the same binary test is used on x86 to
>> test x86 and ppc64, and vice-versa. I'm going to check if we have
>> something like "qtest_get_accel()"...
>
> Something like that should work:
>
> --- a/tests/postcopy-test.c
> +++ b/tests/postcopy-test.c
> @@ -380,12 +380,17 @@ static void test_migrate(void)
> tmpfs, bootpath, uri);
> } else if (strcmp(arch, "ppc64") == 0) {
> init_bootfile_ppc(bootpath);
> - cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
> +#ifdef _ARCH_PPC64
I think you'd need to test CONFIG_KVM, too, since it could also have
been disabled on on PPC, couldn't it?
> +#define QEMU_CMD_ACCEL "-machine accel=kvm:tcg"
> +#else
> +#define QEMU_CMD_ACCEL "-machine accel=tcg"
> +#endif
Alternatively, what about shutting up the message in accel.c by changing
it like that:
if (!qtest_enabled()) {
error_report("\"%s\" accelerator not found.\n", buf);
}
?
Thomas
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-26 9:58 ` Laurent Vivier
@ 2016-07-26 11:50 ` David Gibson
0 siblings, 0 replies; 19+ messages in thread
From: David Gibson @ 2016-07-26 11:50 UTC (permalink / raw)
To: Laurent Vivier; +Cc: Dr. David Alan Gilbert, Thomas Huth, dgibson, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5052 bytes --]
On Tue, Jul 26, 2016 at 11:58:17AM +0200, Laurent Vivier wrote:
>
>
> On 26/07/2016 11:54, Dr. David Alan Gilbert wrote:
> > * Laurent Vivier (lvivier@redhat.com) wrote:
> >>
> >>
> >> On 26/07/2016 11:39, Laurent Vivier wrote:
> >>>
> >>>
> >>> On 26/07/2016 11:28, Thomas Huth wrote:
> >>>> On 26.07.2016 11:23, Laurent Vivier wrote:
> >>>>>
> >>>>>
> >>>>> On 23/07/2016 08:30, David Gibson wrote:
> >>>>>> On Fri, Jul 22, 2016 at 09:28:58AM +0200, Laurent Vivier wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> On 22/07/2016 08:43, David Gibson wrote:
> >>>>>>>> On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote:
> >>>>>>>>> As userfaultfd syscall is available on powerpc, migration
> >>>>>>>>> postcopy can be used.
> >>>>>>>>>
> >>>>>>>>> This patch adds the support needed to test this on powerpc,
> >>>>>>>>> instead of using a bootsector to run code to modify memory,
> >>>>>>>>> we use a FORTH script in "boot-command" property.
> >>>>>>>>>
> >>>>>>>>> As spapr machine doesn't support "-prom-env" argument
> >>>>>>>>> (the nvram is initialized by SLOF and not by QEMU),
> >>>>>>>>> "boot-command" is provided to SLOF via a file mapped nvram
> >>>>>>>>> (with "-drive file=...,if=pflash")
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >>>>>>>>> ---
> >>>>>>>>> v2: move FORTH script directly in sprintf()
> >>>>>>>>> use openbios_firmware_abi.h
> >>>>>>>>> remove useless "default" case
> >>>>>>>>>
> >>>>>>>>> tests/Makefile.include | 1 +
> >>>>>>>>> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
> >>>>>>>>> 2 files changed, 98 insertions(+), 19 deletions(-)
> >>>>>>>>
> >>>>>>>> There's a mostly cosmetic problem with this. If you run make check
> >>>>>>>> for a ppc64 target on an x86 machine, you get:
> >>>>>>>>
> >>>>>>>> GTESTER check-qtest-ppc64
> >>>>>>>> "kvm" accelerator not found.
> >>>>>>>> "kvm" accelerator not found.
> >>>>>>>
> >>>>>>> I think this is because of "-machine accel=kvm:tcg", it tries to use kvm
> >>>>>>> and fall back to tcg.
> >>>>>>>
> >>>>>>> accel.c:
> >>>>>>>
> >>>>>>> 80 void configure_accelerator(MachineState *ms)
> >>>>>>> 81 {
> >>>>>>> ...
> >>>>>>> 100 acc = accel_find(buf);
> >>>>>>> 101 if (!acc) {
> >>>>>>> 102 fprintf(stderr, "\"%s\" accelerator not found.\n", buf);
> >>>>>>> 103 continue;
> >>>>>>> 104 }
> >>>>>>>
> >>>>>>> We can remove the "-machine" argument to use the default instead (tcg or
> >>>>>>> kvm).
> >>>>>>
> >>>>>> That sounds like a good option for a general test.
> >>>>>
> >>>>> In fact, we can't: we need to add a "-machine accel=XXXX" to our command
> >>>>> line to override the "-machine accel=qtest" provided by the qtest
> >>>>> framework. If we don't override it, the machine doesn't start.
> >>>>
> >>>> Would it work if you'd added some magic with "#ifdef CONFIG_KVM" here?
> >>>
> >>> I think it needs to be dynamic as the same binary test is used on x86 to
> >>> test x86 and ppc64, and vice-versa. I'm going to check if we have
> >>> something like "qtest_get_accel()"...
> >>
> >> Something like that should work:
> >>
> >> --- a/tests/postcopy-test.c
> >> +++ b/tests/postcopy-test.c
> >> @@ -380,12 +380,17 @@ static void test_migrate(void)
> >> tmpfs, bootpath, uri);
> >> } else if (strcmp(arch, "ppc64") == 0) {
> >> init_bootfile_ppc(bootpath);
> >> - cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
> >> +#ifdef _ARCH_PPC64
> >> +#define QEMU_CMD_ACCEL "-machine accel=kvm:tcg"
> >> +#else
> >> +#define QEMU_CMD_ACCEL "-machine accel=tcg"
> >> +#endif
> >> + cmd_src = g_strdup_printf(QEMU_CMD_ACCEL " -m 256M"
> >> " -name pcsource,debug-threads=on"
> >> " -serial file:%s/src_serial"
> >> " -drive file=%s,if=pflash,format=raw",
> >> tmpfs, bootpath);
> >> - cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
> >> + cmd_dst = g_strdup_printf(QEMU_CMD_ACCEL " -m 256M"
> >> " -name pcdest,debug-threads=on"
> >> " -serial file:%s/dest_serial"
> >> " -incoming %s",
> >>
> >> Laurent
> >
> > Is it worth the hastle to just get rid of the two warnings?
>
> I don't know, it's why I'd like to have the opinion of David.
I'm not really sure either. I do dislike leaving warnings as a rule,
because for someone not familiar with the details of the test it may
not be obvious whether a warning is harmless or not.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-26 10:02 ` Thomas Huth
@ 2016-07-26 12:53 ` Laurent Vivier
2016-07-26 12:59 ` Laurent Vivier
2016-07-26 16:15 ` Laurent Vivier
2016-07-26 16:29 ` Laurent Vivier
2 siblings, 1 reply; 19+ messages in thread
From: Laurent Vivier @ 2016-07-26 12:53 UTC (permalink / raw)
To: Thomas Huth, David Gibson; +Cc: dgibson, qemu-devel, dgilbert
On 26/07/2016 12:02, Thomas Huth wrote:
> On 26.07.2016 11:53, Laurent Vivier wrote:
>>
>>
>> On 26/07/2016 11:39, Laurent Vivier wrote:
>>>
>>>
>>> On 26/07/2016 11:28, Thomas Huth wrote:
>>>> On 26.07.2016 11:23, Laurent Vivier wrote:
>>>>>
>>>>>
>>>>> On 23/07/2016 08:30, David Gibson wrote:
>>>>>> On Fri, Jul 22, 2016 at 09:28:58AM +0200, Laurent Vivier wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 22/07/2016 08:43, David Gibson wrote:
>>>>>>>> On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote:
>>>>>>>>> As userfaultfd syscall is available on powerpc, migration
>>>>>>>>> postcopy can be used.
>>>>>>>>>
>>>>>>>>> This patch adds the support needed to test this on powerpc,
>>>>>>>>> instead of using a bootsector to run code to modify memory,
>>>>>>>>> we use a FORTH script in "boot-command" property.
>>>>>>>>>
>>>>>>>>> As spapr machine doesn't support "-prom-env" argument
>>>>>>>>> (the nvram is initialized by SLOF and not by QEMU),
>>>>>>>>> "boot-command" is provided to SLOF via a file mapped nvram
>>>>>>>>> (with "-drive file=...,if=pflash")
>>>>>>>>>
>>>>>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>>>>>>> ---
>>>>>>>>> v2: move FORTH script directly in sprintf()
>>>>>>>>> use openbios_firmware_abi.h
>>>>>>>>> remove useless "default" case
>>>>>>>>>
>>>>>>>>> tests/Makefile.include | 1 +
>>>>>>>>> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
>>>>>>>>> 2 files changed, 98 insertions(+), 19 deletions(-)
>>>>>>>>
>>>>>>>> There's a mostly cosmetic problem with this. If you run make check
>>>>>>>> for a ppc64 target on an x86 machine, you get:
>>>>>>>>
>>>>>>>> GTESTER check-qtest-ppc64
>>>>>>>> "kvm" accelerator not found.
>>>>>>>> "kvm" accelerator not found.
>>>>>>>
>>>>>>> I think this is because of "-machine accel=kvm:tcg", it tries to use kvm
>>>>>>> and fall back to tcg.
>>>>>>>
>>>>>>> accel.c:
>>>>>>>
>>>>>>> 80 void configure_accelerator(MachineState *ms)
>>>>>>> 81 {
>>>>>>> ...
>>>>>>> 100 acc = accel_find(buf);
>>>>>>> 101 if (!acc) {
>>>>>>> 102 fprintf(stderr, "\"%s\" accelerator not found.\n", buf);
>>>>>>> 103 continue;
>>>>>>> 104 }
>>>>>>>
>>>>>>> We can remove the "-machine" argument to use the default instead (tcg or
>>>>>>> kvm).
>>>>>>
>>>>>> That sounds like a good option for a general test.
>>>>>
>>>>> In fact, we can't: we need to add a "-machine accel=XXXX" to our command
>>>>> line to override the "-machine accel=qtest" provided by the qtest
>>>>> framework. If we don't override it, the machine doesn't start.
>>>>
>>>> Would it work if you'd added some magic with "#ifdef CONFIG_KVM" here?
>>>
>>> I think it needs to be dynamic as the same binary test is used on x86 to
>>> test x86 and ppc64, and vice-versa. I'm going to check if we have
>>> something like "qtest_get_accel()"...
>>
>> Something like that should work:
>>
>> --- a/tests/postcopy-test.c
>> +++ b/tests/postcopy-test.c
>> @@ -380,12 +380,17 @@ static void test_migrate(void)
>> tmpfs, bootpath, uri);
>> } else if (strcmp(arch, "ppc64") == 0) {
>> init_bootfile_ppc(bootpath);
>> - cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
>> +#ifdef _ARCH_PPC64
>
> I think you'd need to test CONFIG_KVM, too, since it could also have
> been disabled on on PPC, couldn't it?
Sure.
>> +#define QEMU_CMD_ACCEL "-machine accel=kvm:tcg"
>> +#else
>> +#define QEMU_CMD_ACCEL "-machine accel=tcg"
>> +#endif
>
> Alternatively, what about shutting up the message in accel.c by changing
> it like that:
>
> if (!qtest_enabled()) {
> error_report("\"%s\" accelerator not found.\n", buf);
> }
>
I've tried that, and we always get the messages in the "make check" output.
Laurent
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-26 12:53 ` Laurent Vivier
@ 2016-07-26 12:59 ` Laurent Vivier
0 siblings, 0 replies; 19+ messages in thread
From: Laurent Vivier @ 2016-07-26 12:59 UTC (permalink / raw)
To: Thomas Huth, David Gibson; +Cc: dgibson, qemu-devel, dgilbert
On 26/07/2016 14:53, Laurent Vivier wrote:
>
>
> On 26/07/2016 12:02, Thomas Huth wrote:
>> On 26.07.2016 11:53, Laurent Vivier wrote:
>>>
>>>
>>> On 26/07/2016 11:39, Laurent Vivier wrote:
>>>>
>>>>
>>>> On 26/07/2016 11:28, Thomas Huth wrote:
>>>>> On 26.07.2016 11:23, Laurent Vivier wrote:
>>>>>>
>>>>>>
>>>>>> On 23/07/2016 08:30, David Gibson wrote:
>>>>>>> On Fri, Jul 22, 2016 at 09:28:58AM +0200, Laurent Vivier wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 22/07/2016 08:43, David Gibson wrote:
>>>>>>>>> On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote:
>>>>>>>>>> As userfaultfd syscall is available on powerpc, migration
>>>>>>>>>> postcopy can be used.
>>>>>>>>>>
>>>>>>>>>> This patch adds the support needed to test this on powerpc,
>>>>>>>>>> instead of using a bootsector to run code to modify memory,
>>>>>>>>>> we use a FORTH script in "boot-command" property.
>>>>>>>>>>
>>>>>>>>>> As spapr machine doesn't support "-prom-env" argument
>>>>>>>>>> (the nvram is initialized by SLOF and not by QEMU),
>>>>>>>>>> "boot-command" is provided to SLOF via a file mapped nvram
>>>>>>>>>> (with "-drive file=...,if=pflash")
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>>>>>>>> ---
>>>>>>>>>> v2: move FORTH script directly in sprintf()
>>>>>>>>>> use openbios_firmware_abi.h
>>>>>>>>>> remove useless "default" case
>>>>>>>>>>
>>>>>>>>>> tests/Makefile.include | 1 +
>>>>>>>>>> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
>>>>>>>>>> 2 files changed, 98 insertions(+), 19 deletions(-)
>>>>>>>>>
>>>>>>>>> There's a mostly cosmetic problem with this. If you run make check
>>>>>>>>> for a ppc64 target on an x86 machine, you get:
>>>>>>>>>
>>>>>>>>> GTESTER check-qtest-ppc64
>>>>>>>>> "kvm" accelerator not found.
>>>>>>>>> "kvm" accelerator not found.
>>>>>>>>
>>>>>>>> I think this is because of "-machine accel=kvm:tcg", it tries to use kvm
>>>>>>>> and fall back to tcg.
>>>>>>>>
>>>>>>>> accel.c:
>>>>>>>>
>>>>>>>> 80 void configure_accelerator(MachineState *ms)
>>>>>>>> 81 {
>>>>>>>> ...
>>>>>>>> 100 acc = accel_find(buf);
>>>>>>>> 101 if (!acc) {
>>>>>>>> 102 fprintf(stderr, "\"%s\" accelerator not found.\n", buf);
>>>>>>>> 103 continue;
>>>>>>>> 104 }
>>>>>>>>
>>>>>>>> We can remove the "-machine" argument to use the default instead (tcg or
>>>>>>>> kvm).
>>>>>>>
>>>>>>> That sounds like a good option for a general test.
>>>>>>
>>>>>> In fact, we can't: we need to add a "-machine accel=XXXX" to our command
>>>>>> line to override the "-machine accel=qtest" provided by the qtest
>>>>>> framework. If we don't override it, the machine doesn't start.
>>>>>
>>>>> Would it work if you'd added some magic with "#ifdef CONFIG_KVM" here?
>>>>
>>>> I think it needs to be dynamic as the same binary test is used on x86 to
>>>> test x86 and ppc64, and vice-versa. I'm going to check if we have
>>>> something like "qtest_get_accel()"...
>>>
>>> Something like that should work:
>>>
>>> --- a/tests/postcopy-test.c
>>> +++ b/tests/postcopy-test.c
>>> @@ -380,12 +380,17 @@ static void test_migrate(void)
>>> tmpfs, bootpath, uri);
>>> } else if (strcmp(arch, "ppc64") == 0) {
>>> init_bootfile_ppc(bootpath);
>>> - cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
>>> +#ifdef _ARCH_PPC64
>>
>> I think you'd need to test CONFIG_KVM, too, since it could also have
>> been disabled on on PPC, couldn't it?
>
> Sure.
>
>>> +#define QEMU_CMD_ACCEL "-machine accel=kvm:tcg"
>>> +#else
>>> +#define QEMU_CMD_ACCEL "-machine accel=tcg"
>>> +#endif
>>
>> Alternatively, what about shutting up the message in accel.c by changing
>> it like that:
>>
>> if (!qtest_enabled()) {
>> error_report("\"%s\" accelerator not found.\n", buf);
>> }
>>
>
> I've tried that, and we always get the messages in the "make check" output.
No, I'm wrong: I didn't add the "qtest_enabled()", only replace the
fprintf() by an "error_report()"... it should work.
Laurent
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-26 10:02 ` Thomas Huth
2016-07-26 12:53 ` Laurent Vivier
@ 2016-07-26 16:15 ` Laurent Vivier
2016-07-26 16:29 ` Laurent Vivier
2 siblings, 0 replies; 19+ messages in thread
From: Laurent Vivier @ 2016-07-26 16:15 UTC (permalink / raw)
To: Thomas Huth, David Gibson; +Cc: dgibson, qemu-devel, dgilbert
On 26/07/2016 12:02, Thomas Huth wrote:
> On 26.07.2016 11:53, Laurent Vivier wrote:
>>
>>
>> On 26/07/2016 11:39, Laurent Vivier wrote:
>>>
>>>
>>> On 26/07/2016 11:28, Thomas Huth wrote:
>>>> On 26.07.2016 11:23, Laurent Vivier wrote:
>>>>>
>>>>>
>>>>> On 23/07/2016 08:30, David Gibson wrote:
>>>>>> On Fri, Jul 22, 2016 at 09:28:58AM +0200, Laurent Vivier wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 22/07/2016 08:43, David Gibson wrote:
>>>>>>>> On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote:
>>>>>>>>> As userfaultfd syscall is available on powerpc, migration
>>>>>>>>> postcopy can be used.
>>>>>>>>>
>>>>>>>>> This patch adds the support needed to test this on powerpc,
>>>>>>>>> instead of using a bootsector to run code to modify memory,
>>>>>>>>> we use a FORTH script in "boot-command" property.
>>>>>>>>>
>>>>>>>>> As spapr machine doesn't support "-prom-env" argument
>>>>>>>>> (the nvram is initialized by SLOF and not by QEMU),
>>>>>>>>> "boot-command" is provided to SLOF via a file mapped nvram
>>>>>>>>> (with "-drive file=...,if=pflash")
>>>>>>>>>
>>>>>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>>>>>>> ---
>>>>>>>>> v2: move FORTH script directly in sprintf()
>>>>>>>>> use openbios_firmware_abi.h
>>>>>>>>> remove useless "default" case
>>>>>>>>>
>>>>>>>>> tests/Makefile.include | 1 +
>>>>>>>>> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
>>>>>>>>> 2 files changed, 98 insertions(+), 19 deletions(-)
>>>>>>>>
>>>>>>>> There's a mostly cosmetic problem with this. If you run make check
>>>>>>>> for a ppc64 target on an x86 machine, you get:
>>>>>>>>
>>>>>>>> GTESTER check-qtest-ppc64
>>>>>>>> "kvm" accelerator not found.
>>>>>>>> "kvm" accelerator not found.
>>>>>>>
>>>>>>> I think this is because of "-machine accel=kvm:tcg", it tries to use kvm
>>>>>>> and fall back to tcg.
>>>>>>>
>>>>>>> accel.c:
>>>>>>>
>>>>>>> 80 void configure_accelerator(MachineState *ms)
>>>>>>> 81 {
>>>>>>> ...
>>>>>>> 100 acc = accel_find(buf);
>>>>>>> 101 if (!acc) {
>>>>>>> 102 fprintf(stderr, "\"%s\" accelerator not found.\n", buf);
>>>>>>> 103 continue;
>>>>>>> 104 }
>>>>>>>
>>>>>>> We can remove the "-machine" argument to use the default instead (tcg or
>>>>>>> kvm).
>>>>>>
>>>>>> That sounds like a good option for a general test.
>>>>>
>>>>> In fact, we can't: we need to add a "-machine accel=XXXX" to our command
>>>>> line to override the "-machine accel=qtest" provided by the qtest
>>>>> framework. If we don't override it, the machine doesn't start.
>>>>
>>>> Would it work if you'd added some magic with "#ifdef CONFIG_KVM" here?
>>>
>>> I think it needs to be dynamic as the same binary test is used on x86 to
>>> test x86 and ppc64, and vice-versa. I'm going to check if we have
>>> something like "qtest_get_accel()"...
>>
>> Something like that should work:
>>
>> --- a/tests/postcopy-test.c
>> +++ b/tests/postcopy-test.c
>> @@ -380,12 +380,17 @@ static void test_migrate(void)
>> tmpfs, bootpath, uri);
>> } else if (strcmp(arch, "ppc64") == 0) {
>> init_bootfile_ppc(bootpath);
>> - cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
>> +#ifdef _ARCH_PPC64
>
> I think you'd need to test CONFIG_KVM, too, since it could also have
> been disabled on on PPC, couldn't it?
I've tried to use CONFIG_KVM, but as it is defined per target in
config-target.h we can't use it in qtest sources. I think we can only
use CONFIGs defined in config-host.h (as the same binary is used for all
the targets).
Laurent
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-26 10:02 ` Thomas Huth
2016-07-26 12:53 ` Laurent Vivier
2016-07-26 16:15 ` Laurent Vivier
@ 2016-07-26 16:29 ` Laurent Vivier
2 siblings, 0 replies; 19+ messages in thread
From: Laurent Vivier @ 2016-07-26 16:29 UTC (permalink / raw)
To: Thomas Huth, David Gibson; +Cc: dgibson, qemu-devel, dgilbert
On 26/07/2016 12:02, Thomas Huth wrote:
> On 26.07.2016 11:53, Laurent Vivier wrote:
>>
>>
>> On 26/07/2016 11:39, Laurent Vivier wrote:
>>>
>>>
>>> On 26/07/2016 11:28, Thomas Huth wrote:
>>>> On 26.07.2016 11:23, Laurent Vivier wrote:
>>>>>
>>>>>
>>>>> On 23/07/2016 08:30, David Gibson wrote:
>>>>>> On Fri, Jul 22, 2016 at 09:28:58AM +0200, Laurent Vivier wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 22/07/2016 08:43, David Gibson wrote:
>>>>>>>> On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote:
>>>>>>>>> As userfaultfd syscall is available on powerpc, migration
>>>>>>>>> postcopy can be used.
>>>>>>>>>
>>>>>>>>> This patch adds the support needed to test this on powerpc,
>>>>>>>>> instead of using a bootsector to run code to modify memory,
>>>>>>>>> we use a FORTH script in "boot-command" property.
>>>>>>>>>
>>>>>>>>> As spapr machine doesn't support "-prom-env" argument
>>>>>>>>> (the nvram is initialized by SLOF and not by QEMU),
>>>>>>>>> "boot-command" is provided to SLOF via a file mapped nvram
>>>>>>>>> (with "-drive file=...,if=pflash")
>>>>>>>>>
>>>>>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>>>>>>> ---
>>>>>>>>> v2: move FORTH script directly in sprintf()
>>>>>>>>> use openbios_firmware_abi.h
>>>>>>>>> remove useless "default" case
>>>>>>>>>
>>>>>>>>> tests/Makefile.include | 1 +
>>>>>>>>> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
>>>>>>>>> 2 files changed, 98 insertions(+), 19 deletions(-)
>>>>>>>>
>>>>>>>> There's a mostly cosmetic problem with this. If you run make check
>>>>>>>> for a ppc64 target on an x86 machine, you get:
>>>>>>>>
>>>>>>>> GTESTER check-qtest-ppc64
>>>>>>>> "kvm" accelerator not found.
>>>>>>>> "kvm" accelerator not found.
>>>>>>>
>>>>>>> I think this is because of "-machine accel=kvm:tcg", it tries to use kvm
>>>>>>> and fall back to tcg.
>>>>>>>
>>>>>>> accel.c:
>>>>>>>
>>>>>>> 80 void configure_accelerator(MachineState *ms)
>>>>>>> 81 {
>>>>>>> ...
>>>>>>> 100 acc = accel_find(buf);
>>>>>>> 101 if (!acc) {
>>>>>>> 102 fprintf(stderr, "\"%s\" accelerator not found.\n", buf);
>>>>>>> 103 continue;
>>>>>>> 104 }
>>>>>>>
>>>>>>> We can remove the "-machine" argument to use the default instead (tcg or
>>>>>>> kvm).
>>>>>>
>>>>>> That sounds like a good option for a general test.
>>>>>
>>>>> In fact, we can't: we need to add a "-machine accel=XXXX" to our command
>>>>> line to override the "-machine accel=qtest" provided by the qtest
>>>>> framework. If we don't override it, the machine doesn't start.
>>>>
>>>> Would it work if you'd added some magic with "#ifdef CONFIG_KVM" here?
>>>
>>> I think it needs to be dynamic as the same binary test is used on x86 to
>>> test x86 and ppc64, and vice-versa. I'm going to check if we have
>>> something like "qtest_get_accel()"...
>>
>> Something like that should work:
>>
>> --- a/tests/postcopy-test.c
>> +++ b/tests/postcopy-test.c
>> @@ -380,12 +380,17 @@ static void test_migrate(void)
>> tmpfs, bootpath, uri);
>> } else if (strcmp(arch, "ppc64") == 0) {
>> init_bootfile_ppc(bootpath);
>> - cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
>> +#ifdef _ARCH_PPC64
>
> I think you'd need to test CONFIG_KVM, too, since it could also have
> been disabled on on PPC, couldn't it?
>
>> +#define QEMU_CMD_ACCEL "-machine accel=kvm:tcg"
>> +#else
>> +#define QEMU_CMD_ACCEL "-machine accel=tcg"
>> +#endif
>
> Alternatively, what about shutting up the message in accel.c by changing
> it like that:
>
> if (!qtest_enabled()) {
> error_report("\"%s\" accelerator not found.\n", buf);
> }
>
We can't use this as we overwrite "-machine accel=qtest" with our
"-machine accel=kvm:tcg": qtest accelerator is not initialized and
qtest_enabled() is false.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
2016-07-21 16:47 [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64 Laurent Vivier
` (2 preceding siblings ...)
2016-07-22 6:43 ` David Gibson
@ 2016-07-27 0:43 ` David Gibson
3 siblings, 0 replies; 19+ messages in thread
From: David Gibson @ 2016-07-27 0:43 UTC (permalink / raw)
To: Laurent Vivier; +Cc: qemu-devel, dgilbert, thuth, dgibson
[-- Attachment #1: Type: text/plain, Size: 8555 bytes --]
On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote:
> As userfaultfd syscall is available on powerpc, migration
> postcopy can be used.
>
> This patch adds the support needed to test this on powerpc,
> instead of using a bootsector to run code to modify memory,
> we use a FORTH script in "boot-command" property.
>
> As spapr machine doesn't support "-prom-env" argument
> (the nvram is initialized by SLOF and not by QEMU),
> "boot-command" is provided to SLOF via a file mapped nvram
> (with "-drive file=...,if=pflash")
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
So, the warning message is a bit unfortunate, but the following
discussion seems to show that removing it isn't trivial. If we can do
so in a follow up patch, that would be nice, but for the time being,
I've merged this patch to ppc-for-2.7 anyway.
> ---
> v2: move FORTH script directly in sprintf()
> use openbios_firmware_abi.h
> remove useless "default" case
>
> tests/Makefile.include | 1 +
> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++--------
> 2 files changed, 98 insertions(+), 19 deletions(-)
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index e7e50d6..e2d1885 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -268,6 +268,7 @@ check-qtest-sparc-y += tests/prom-env-test$(EXESUF)
> #check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
> check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
> check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
> +check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
>
> check-qtest-generic-y += tests/qom-test$(EXESUF)
>
> diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c
> index 16465ab..229e9e9 100644
> --- a/tests/postcopy-test.c
> +++ b/tests/postcopy-test.c
> @@ -18,6 +18,9 @@
> #include "qemu/sockets.h"
> #include "sysemu/char.h"
> #include "sysemu/sysemu.h"
> +#include "hw/nvram/openbios_firmware_abi.h"
> +
> +#define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */
>
> const unsigned start_address = 1024 * 1024;
> const unsigned end_address = 100 * 1024 * 1024;
> @@ -122,6 +125,44 @@ unsigned char bootsect[] = {
> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa
> };
>
> +static void init_bootfile_x86(const char *bootpath)
> +{
> + FILE *bootfile = fopen(bootpath, "wb");
> +
> + g_assert_cmpint(fwrite(bootsect, 512, 1, bootfile), ==, 1);
> + fclose(bootfile);
> +}
> +
> +static void init_bootfile_ppc(const char *bootpath)
> +{
> + FILE *bootfile;
> + char buf[MIN_NVRAM_SIZE];
> + struct OpenBIOS_nvpart_v1 *header = (struct OpenBIOS_nvpart_v1 *)buf;
> +
> + memset(buf, 0, MIN_NVRAM_SIZE);
> +
> + /* Create a "common" partition in nvram to store boot-command property */
> +
> + header->signature = OPENBIOS_PART_SYSTEM;
> + memcpy(header->name, "common", 6);
> + OpenBIOS_finish_partition(header, MIN_NVRAM_SIZE);
> +
> + /* FW_MAX_SIZE is 4MB, but slof.bin is only 900KB,
> + * so let's modify memory between 1MB and 100MB
> + * to do like PC bootsector
> + */
> +
> + sprintf(buf + 16,
> + "boot-command=hex .\" _\" begin %x %x do i c@ 1 + i c! 1000 +loop "
> + ".\" B\" 0 until", end_address, start_address);
> +
> + /* Write partition to the NVRAM file */
> +
> + bootfile = fopen(bootpath, "wb");
> + g_assert_cmpint(fwrite(buf, MIN_NVRAM_SIZE, 1, bootfile), ==, 1);
> + fclose(bootfile);
> +}
> +
> /*
> * Wait for some output in the serial output file,
> * we get an 'A' followed by an endless string of 'B's
> @@ -131,10 +172,29 @@ static void wait_for_serial(const char *side)
> {
> char *serialpath = g_strdup_printf("%s/%s", tmpfs, side);
> FILE *serialfile = fopen(serialpath, "r");
> + const char *arch = qtest_get_arch();
> + int started = (strcmp(side, "src_serial") == 0 &&
> + strcmp(arch, "ppc64") == 0) ? 0 : 1;
>
> do {
> int readvalue = fgetc(serialfile);
>
> + if (!started) {
> + /* SLOF prints its banner before starting test,
> + * to ignore it, mark the start of the test with '_',
> + * ignore all characters until this marker
> + */
> + switch (readvalue) {
> + case '_':
> + started = 1;
> + break;
> + case EOF:
> + fseek(serialfile, 0, SEEK_SET);
> + usleep(1000);
> + break;
> + }
> + continue;
> + }
> switch (readvalue) {
> case 'A':
> /* Fine */
> @@ -147,6 +207,8 @@ static void wait_for_serial(const char *side)
> return;
>
> case EOF:
> + started = (strcmp(side, "src_serial") == 0 &&
> + strcmp(arch, "ppc64") == 0) ? 0 : 1;
> fseek(serialfile, 0, SEEK_SET);
> usleep(1000);
> break;
> @@ -295,32 +357,48 @@ static void test_migrate(void)
> char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> QTestState *global = global_qtest, *from, *to;
> unsigned char dest_byte_a, dest_byte_b, dest_byte_c, dest_byte_d;
> - gchar *cmd;
> + gchar *cmd, *cmd_src, *cmd_dst;
> QDict *rsp;
>
> char *bootpath = g_strdup_printf("%s/bootsect", tmpfs);
> - FILE *bootfile = fopen(bootpath, "wb");
> + const char *arch = qtest_get_arch();
>
> got_stop = false;
> - g_assert_cmpint(fwrite(bootsect, 512, 1, bootfile), ==, 1);
> - fclose(bootfile);
>
> - cmd = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> - " -name pcsource,debug-threads=on"
> - " -serial file:%s/src_serial"
> - " -drive file=%s,format=raw",
> - tmpfs, bootpath);
> - from = qtest_start(cmd);
> - g_free(cmd);
> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> + init_bootfile_x86(bootpath);
> + cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> + " -name pcsource,debug-threads=on"
> + " -serial file:%s/src_serial"
> + " -drive file=%s,format=raw",
> + tmpfs, bootpath);
> + cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> + " -name pcdest,debug-threads=on"
> + " -serial file:%s/dest_serial"
> + " -drive file=%s,format=raw"
> + " -incoming %s",
> + tmpfs, bootpath, uri);
> + } else if (strcmp(arch, "ppc64") == 0) {
> + init_bootfile_ppc(bootpath);
> + cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
> + " -name pcsource,debug-threads=on"
> + " -serial file:%s/src_serial"
> + " -drive file=%s,if=pflash,format=raw",
> + tmpfs, bootpath);
> + cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
> + " -name pcdest,debug-threads=on"
> + " -serial file:%s/dest_serial"
> + " -incoming %s",
> + tmpfs, uri);
> + } else {
> + g_assert_not_reached();
> + }
>
> - cmd = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> - " -name pcdest,debug-threads=on"
> - " -serial file:%s/dest_serial"
> - " -drive file=%s,format=raw"
> - " -incoming %s",
> - tmpfs, bootpath, uri);
> - to = qtest_init(cmd);
> - g_free(cmd);
> + from = qtest_start(cmd_src);
> + g_free(cmd_src);
> +
> + to = qtest_init(cmd_dst);
> + g_free(cmd_dst);
>
> global_qtest = from;
> rsp = qmp("{ 'execute': 'migrate-set-capabilities',"
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2016-07-27 0:45 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-21 16:47 [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64 Laurent Vivier
2016-07-21 19:08 ` Dr. David Alan Gilbert
2016-07-21 20:57 ` Thomas Huth
2016-07-22 6:43 ` David Gibson
2016-07-22 7:28 ` Laurent Vivier
2016-07-23 6:30 ` David Gibson
2016-07-26 9:23 ` Laurent Vivier
2016-07-26 9:28 ` Thomas Huth
2016-07-26 9:39 ` Laurent Vivier
2016-07-26 9:53 ` Laurent Vivier
2016-07-26 9:54 ` Dr. David Alan Gilbert
2016-07-26 9:58 ` Laurent Vivier
2016-07-26 11:50 ` David Gibson
2016-07-26 10:02 ` Thomas Huth
2016-07-26 12:53 ` Laurent Vivier
2016-07-26 12:59 ` Laurent Vivier
2016-07-26 16:15 ` Laurent Vivier
2016-07-26 16:29 ` Laurent Vivier
2016-07-27 0:43 ` David Gibson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.