| Message ID | 1469095928-5816-1-git-send-email-lvivier@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 21.07.2016 12:12, Laurent Vivier wrote: > As userfaultfd syscall is available on powerpc, migration > postcopy can be used. Good idea to add this test for PPC, too! > 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") I wonder whether we could easily add support for the "-prom-env" parameter for the sPAPR machine, too, since the NVRAM layout seems to be pretty much the same as on the old CHRP Mac machines...? > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > tests/Makefile.include | 1 + > tests/postcopy-test.c | 141 ++++++++++++++++++++++++++++++++++++++++++------- > 2 files changed, 123 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..439afd9 100644 > --- a/tests/postcopy-test.c > +++ b/tests/postcopy-test.c > @@ -19,6 +19,24 @@ > #include "sysemu/char.h" > #include "sysemu/sysemu.h" > > +/* These structures are already defined by OpenBIOS and usable with SLOF */ > +#define NVRAM_PART_SYSTEM 0x70 > +struct nvpart { > + uint8_t signature; > + uint8_t checksum; > + uint16_t len; /* BE, length divided by 16 */ > + char name[12]; > + char content[0]; IIRC zero-sized arrays are a GCC extension ... for valid C99, it might be better to use "char content[]" instead? > +}; > > +#define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */ > + > +/* 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 > + */ > +#define FORTH_BOOTSCRIPT "boot-command=hex .\" _\" begin %x %x do i c@ 1 + i c! 1000 +loop .\" B\" 0 until" Just a matter of taste, but I somewhat dislike the idea of hiding a string with format parameters in a macro ... I think I'd rather place this string directly into the corresponding sprintf() statement below instead. > const unsigned start_address = 1024 * 1024; > const unsigned end_address = 100 * 1024 * 1024; > bool got_stop; > @@ -122,6 +140,52 @@ 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 nvpart_checksum(struct nvpart *header) > +{ > + unsigned int i, sum; > + uint8_t *tmpptr; > + > + tmpptr = (uint8_t *)header; > + sum = *tmpptr; > + for (i = 0; i < 14; i++) { > + sum += tmpptr[2 + i]; > + sum = (sum + ((sum & 0xff00) >> 8)) & 0xff; > + } > + header->checksum = sum & 0xff; > +} Have you tried to include openbios_firmware_abi.h instead and use OpenBIOS_finish_partition() here? That would avoid to have this code duplicated. > +static void init_bootfile_ppc(const char *bootpath) > +{ > + FILE *bootfile; > + char buf[MIN_NVRAM_SIZE]; > + struct nvpart *header = (struct nvpart *)buf; > + > + memset(buf, 0, MIN_NVRAM_SIZE); > + > + /* Create a "common" partition in nvram to store boot-command property */ > + > + header->signature = NVRAM_PART_SYSTEM; > + memcpy(header->name, "common", 6); > + header->len = cpu_to_be16(MIN_NVRAM_SIZE >> 4); > + nvpart_checksum(header); /* can change if we change header->len */ > + > + sprintf(header->content, FORTH_BOOTSCRIPT, 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 +195,31 @@ 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; > + default: > + break; I think you could remove that default case. > + } > + continue; > + } > switch (readvalue) { > case 'A': > /* Fine */ > @@ -147,6 +232,8 @@ static void wait_for_serial(const char *side) > return; > > case EOF: > + started = (strcmp(side, "src_serial") == 0 && > + strcmp(arch, "ppc64") == 0) ? 0 : 1; I somehow fail to see why is this needed here again? Isn't the initial setup of "started" at the beginning of the function enough? > fseek(serialfile, 0, SEEK_SET); > usleep(1000); > break; > @@ -295,32 +382,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'," > Thomas
On 21/07/2016 15:16, Thomas Huth wrote: > On 21.07.2016 12:12, Laurent Vivier wrote: >> As userfaultfd syscall is available on powerpc, migration >> postcopy can be used. > > Good idea to add this test for PPC, too! The idea comes from David Gilbert :) Thanks David! > >> 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") > > I wonder whether we could easily add support for the "-prom-env" > parameter for the sPAPR machine, too, since the NVRAM layout seems to be > pretty much the same as on the old CHRP Mac machines...? The only thing we need to have "-prom-env" is to initialize the nvram partitions in QEMU if they don't exist. I didn't follow this way because this work is normally to be done by the firmware not by QEMU. If we do it in QEMU, SLOF will do not, and we can't be sure we are doing the things well, as SLOF does. On Mac, it is another history, originally, firmware was OpenHackware, not OpenBIOS, and OH was only here to allow to boot the kernel nothing more (it was really a hack: there was not forth interpreter [oh, my god!]). So all the important stuffs were done by QEMU, like to initialize the NVRAM, the PCI bus, ... and this allows to set values in the environment variables. But if you think it is viable, it should be really easy to do (like cut'n'paste from OpenBIOS ABI)... Thanks, Laurent > >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >> --- >> tests/Makefile.include | 1 + >> tests/postcopy-test.c | 141 ++++++++++++++++++++++++++++++++++++++++++------- >> 2 files changed, 123 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..439afd9 100644 >> --- a/tests/postcopy-test.c >> +++ b/tests/postcopy-test.c >> @@ -19,6 +19,24 @@ >> #include "sysemu/char.h" >> #include "sysemu/sysemu.h" >> >> +/* These structures are already defined by OpenBIOS and usable with SLOF */ >> +#define NVRAM_PART_SYSTEM 0x70 >> +struct nvpart { >> + uint8_t signature; >> + uint8_t checksum; >> + uint16_t len; /* BE, length divided by 16 */ >> + char name[12]; >> + char content[0]; > > IIRC zero-sized arrays are a GCC extension ... for valid C99, it might > be better to use "char content[]" instead? > >> +}; >> >> +#define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */ >> + >> +/* 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 >> + */ >> +#define FORTH_BOOTSCRIPT "boot-command=hex .\" _\" begin %x %x do i c@ 1 + i c! 1000 +loop .\" B\" 0 until" > > Just a matter of taste, but I somewhat dislike the idea of hiding a > string with format parameters in a macro ... I think I'd rather place > this string directly into the corresponding sprintf() statement below > instead. > >> const unsigned start_address = 1024 * 1024; >> const unsigned end_address = 100 * 1024 * 1024; >> bool got_stop; >> @@ -122,6 +140,52 @@ 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 nvpart_checksum(struct nvpart *header) >> +{ >> + unsigned int i, sum; >> + uint8_t *tmpptr; >> + >> + tmpptr = (uint8_t *)header; >> + sum = *tmpptr; >> + for (i = 0; i < 14; i++) { >> + sum += tmpptr[2 + i]; >> + sum = (sum + ((sum & 0xff00) >> 8)) & 0xff; >> + } >> + header->checksum = sum & 0xff; >> +} > > Have you tried to include openbios_firmware_abi.h instead and use > OpenBIOS_finish_partition() here? That would avoid to have this code > duplicated. > >> +static void init_bootfile_ppc(const char *bootpath) >> +{ >> + FILE *bootfile; >> + char buf[MIN_NVRAM_SIZE]; >> + struct nvpart *header = (struct nvpart *)buf; >> + >> + memset(buf, 0, MIN_NVRAM_SIZE); >> + >> + /* Create a "common" partition in nvram to store boot-command property */ >> + >> + header->signature = NVRAM_PART_SYSTEM; >> + memcpy(header->name, "common", 6); >> + header->len = cpu_to_be16(MIN_NVRAM_SIZE >> 4); >> + nvpart_checksum(header); /* can change if we change header->len */ >> + >> + sprintf(header->content, FORTH_BOOTSCRIPT, 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 +195,31 @@ 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; >> + default: >> + break; > > I think you could remove that default case. > >> + } >> + continue; >> + } >> switch (readvalue) { >> case 'A': >> /* Fine */ >> @@ -147,6 +232,8 @@ static void wait_for_serial(const char *side) >> return; >> >> case EOF: >> + started = (strcmp(side, "src_serial") == 0 && >> + strcmp(arch, "ppc64") == 0) ? 0 : 1; > > I somehow fail to see why is this needed here again? Isn't the initial > setup of "started" at the beginning of the function enough? > >> fseek(serialfile, 0, SEEK_SET); >> usleep(1000); >> break; >> @@ -295,32 +382,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'," >> > > Thomas >
On 21/07/2016 15:16, Thomas Huth wrote: > On 21.07.2016 12:12, Laurent Vivier wrote: ... >> 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..439afd9 100644 >> --- a/tests/postcopy-test.c >> +++ b/tests/postcopy-test.c >> @@ -19,6 +19,24 @@ >> #include "sysemu/char.h" >> #include "sysemu/sysemu.h" >> >> +/* These structures are already defined by OpenBIOS and usable with SLOF */ >> +#define NVRAM_PART_SYSTEM 0x70 >> +struct nvpart { >> + uint8_t signature; >> + uint8_t checksum; >> + uint16_t len; /* BE, length divided by 16 */ >> + char name[12]; >> + char content[0]; > > IIRC zero-sized arrays are a GCC extension ... for valid C99, it might > be better to use "char content[]" instead? You're right: I though [0] was more generic than []. > >> +}; >> >> +#define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */ >> + >> +/* 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 >> + */ >> +#define FORTH_BOOTSCRIPT "boot-command=hex .\" _\" begin %x %x do i c@ 1 + i c! 1000 +loop .\" B\" 0 until" > > Just a matter of taste, but I somewhat dislike the idea of hiding a > string with format parameters in a macro ... I think I'd rather place > this string directly into the corresponding sprintf() statement below > instead. Right > >> const unsigned start_address = 1024 * 1024; >> const unsigned end_address = 100 * 1024 * 1024; >> bool got_stop; >> @@ -122,6 +140,52 @@ 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 nvpart_checksum(struct nvpart *header) >> +{ >> + unsigned int i, sum; >> + uint8_t *tmpptr; >> + >> + tmpptr = (uint8_t *)header; >> + sum = *tmpptr; >> + for (i = 0; i < 14; i++) { >> + sum += tmpptr[2 + i]; >> + sum = (sum + ((sum & 0xff00) >> 8)) & 0xff; >> + } >> + header->checksum = sum & 0xff; >> +} > > Have you tried to include openbios_firmware_abi.h instead and use > OpenBIOS_finish_partition() here? That would avoid to have this code > duplicated. Right again >> +static void init_bootfile_ppc(const char *bootpath) >> +{ >> + FILE *bootfile; >> + char buf[MIN_NVRAM_SIZE]; >> + struct nvpart *header = (struct nvpart *)buf; >> + >> + memset(buf, 0, MIN_NVRAM_SIZE); >> + >> + /* Create a "common" partition in nvram to store boot-command property */ >> + >> + header->signature = NVRAM_PART_SYSTEM; >> + memcpy(header->name, "common", 6); >> + header->len = cpu_to_be16(MIN_NVRAM_SIZE >> 4); >> + nvpart_checksum(header); /* can change if we change header->len */ >> + >> + sprintf(header->content, FORTH_BOOTSCRIPT, 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 +195,31 @@ 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; >> + default: >> + break; > > I think you could remove that default case. ok > >> + } >> + continue; >> + } >> switch (readvalue) { >> case 'A': >> /* Fine */ >> @@ -147,6 +232,8 @@ static void wait_for_serial(const char *side) >> return; >> >> case EOF: >> + started = (strcmp(side, "src_serial") == 0 && >> + strcmp(arch, "ppc64") == 0) ? 0 : 1; > > I somehow fail to see why is this needed here again? Isn't the initial > setup of "started" at the beginning of the function enough? No, because we read a file and this file is growing while we are reading it, so if we restart to read the file (fseek() below) because we are at the end, we must skip again the banner. > >> fseek(serialfile, 0, SEEK_SET); >> usleep(1000); >> break; >> @@ -295,32 +382,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'," >> > > Thomas I'm going to send a v2... Thanks, Laurent
On 21.07.2016 15:46, Laurent Vivier wrote: > > > On 21/07/2016 15:16, Thomas Huth wrote: >> On 21.07.2016 12:12, Laurent Vivier wrote: >>> As userfaultfd syscall is available on powerpc, migration >>> postcopy can be used. >> >> Good idea to add this test for PPC, too! > > The idea comes from David Gilbert :) > Thanks David! > >> >>> 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") >> >> I wonder whether we could easily add support for the "-prom-env" >> parameter for the sPAPR machine, too, since the NVRAM layout seems to be >> pretty much the same as on the old CHRP Mac machines...? > > The only thing we need to have "-prom-env" is to initialize the nvram > partitions in QEMU if they don't exist. > > I didn't follow this way because this work is normally to be done by the > firmware not by QEMU. If we do it in QEMU, SLOF will do not, and we > can't be sure we are doing the things well, as SLOF does. I'm slightly familiar with the NVRAM code in SLOF ... shouldn't be too hard to get a similar layout done with QEMU, especially if we can re-use some of the OpenBIOS NVRAM functions in QEMU. > On Mac, it is another history, originally, firmware was OpenHackware, > not OpenBIOS, and OH was only here to allow to boot the kernel nothing > more (it was really a hack: there was not forth interpreter [oh, my god!]). > So all the important stuffs were done by QEMU, like to initialize the > NVRAM, the PCI bus, ... and this allows to set values in the environment > variables. > > But if you think it is viable, it should be really easy to do (like > cut'n'paste from OpenBIOS ABI)... Having -prom-env for SLOF, too, would be really posh, so I think I'll have a try when I got some spare minutes ... hmm, my TODO list seems to keep rather growing than shrinking ... maybe I'm doing something wrong ;-) Thomas
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..439afd9 100644 --- a/tests/postcopy-test.c +++ b/tests/postcopy-test.c @@ -19,6 +19,24 @@ #include "sysemu/char.h" #include "sysemu/sysemu.h" +/* These structures are already defined by OpenBIOS and usable with SLOF */ +#define NVRAM_PART_SYSTEM 0x70 +struct nvpart { + uint8_t signature; + uint8_t checksum; + uint16_t len; /* BE, length divided by 16 */ + char name[12]; + char content[0]; +}; + +#define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */ + +/* 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 + */ +#define FORTH_BOOTSCRIPT "boot-command=hex .\" _\" begin %x %x do i c@ 1 + i c! 1000 +loop .\" B\" 0 until" + const unsigned start_address = 1024 * 1024; const unsigned end_address = 100 * 1024 * 1024; bool got_stop; @@ -122,6 +140,52 @@ 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 nvpart_checksum(struct nvpart *header) +{ + unsigned int i, sum; + uint8_t *tmpptr; + + tmpptr = (uint8_t *)header; + sum = *tmpptr; + for (i = 0; i < 14; i++) { + sum += tmpptr[2 + i]; + sum = (sum + ((sum & 0xff00) >> 8)) & 0xff; + } + header->checksum = sum & 0xff; +} + +static void init_bootfile_ppc(const char *bootpath) +{ + FILE *bootfile; + char buf[MIN_NVRAM_SIZE]; + struct nvpart *header = (struct nvpart *)buf; + + memset(buf, 0, MIN_NVRAM_SIZE); + + /* Create a "common" partition in nvram to store boot-command property */ + + header->signature = NVRAM_PART_SYSTEM; + memcpy(header->name, "common", 6); + header->len = cpu_to_be16(MIN_NVRAM_SIZE >> 4); + nvpart_checksum(header); /* can change if we change header->len */ + + sprintf(header->content, FORTH_BOOTSCRIPT, 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 +195,31 @@ 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; + default: + break; + } + continue; + } switch (readvalue) { case 'A': /* Fine */ @@ -147,6 +232,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 +382,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',"
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> --- tests/Makefile.include | 1 + tests/postcopy-test.c | 141 ++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 123 insertions(+), 19 deletions(-)