| Message ID | 1513370232-25515-1-git-send-email-wei@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 15 December 2017 at 20:37, Wei Huang <wei@redhat.com> wrote: > This patch adds the migration test support for aarch64. The test code, > which implements the same functionality as x86, is compiled into a binary > and booted as a kernel to qemu. Here are the design ideas: > > * We choose this -kernel design because aarch64 QEMU doesn't provide a > built-in fw like x86 does. So instead of relying on a boot loader, we > use -kernel approach for aarch64. > * The serial output is sent to PL011 directly. > * The physical memory base for mach-virt machine is 0x40000000. We have > to change the start_address and end_address for aarch64. > * The downtime is changed from 0.001 to 0.1. Without this change, we saw > migration stalled. This problem is still under analysis and needs to be > resolved before removing RFC for this patch. > > The test code is as the following: > > .section .text > #if defined(__linux__) > @@ -125,6 +125,18 @@ unsigned char bootsect[] = { > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa > }; > > +unsigned char aarch64_kernel[] = { > + 0x00, 0x10, 0x38, 0xd5, 0x00, 0xf8, 0x7f, 0x92, 0x00, 0x10, 0x18, 0xd5, > + 0xdf, 0x3f, 0x03, 0xd5, 0x24, 0x08, 0x80, 0x52, 0x05, 0x20, 0xa1, 0xd2, > + 0xa4, 0x00, 0x00, 0x39, 0x06, 0x00, 0x80, 0x52, 0x03, 0xc8, 0xa8, 0xd2, > + 0x02, 0x02, 0xa8, 0xd2, 0x41, 0x00, 0x40, 0x39, 0x21, 0x04, 0x00, 0x11, > + 0x41, 0x00, 0x00, 0x39, 0x42, 0x04, 0x40, 0x91, 0x5f, 0x00, 0x03, 0xeb, > + 0x6b, 0xff, 0xff, 0x54, 0xc6, 0x04, 0x00, 0x11, 0xc6, 0x1c, 0x00, 0x12, > + 0xdf, 0x00, 0x00, 0x71, 0xc1, 0xfe, 0xff, 0x54, 0x44, 0x08, 0x80, 0x52, > + 0x05, 0x20, 0xa1, 0xd2, 0xa4, 0x00, 0x00, 0x39, 0xf2, 0xff, 0xff, 0x97 > +}; > +unsigned int aarch64_kernel_len = 96; I'm not really a fan of this steadily increasing number of hex-encoded target binary blobs in the tests directory, but if we must, I think this would be easier to read as an array of uint32_t, so that each entry is one instruction word. (If your claim is that nobody cares about the hex because they'll just rebuild from the source code in the commit message, I would suggest that that makes the source code the 'preferred form for modification' under the GPL...) But I think at some point we really need to stop doing this and instead figure out a mechanism for building target code as part of the QEMU build-and-test. It is coming up increasingly often: * code to run on the guest in tests * the bios blobs (at the moment we special case the x86 bios blobs and assume we can compile them with the host C compiler, which is not great) * for linux-user on several architectures we would like to properly implement a guest VDSO thanks -- PMM
On 12/16/2017 07:49 AM, Peter Maydell wrote: > On 15 December 2017 at 20:37, Wei Huang <wei@redhat.com> wrote: >> This patch adds the migration test support for aarch64. The test code, >> which implements the same functionality as x86, is compiled into a binary >> and booted as a kernel to qemu. Here are the design ideas: >> >> * We choose this -kernel design because aarch64 QEMU doesn't provide a >> built-in fw like x86 does. So instead of relying on a boot loader, we >> use -kernel approach for aarch64. >> * The serial output is sent to PL011 directly. >> * The physical memory base for mach-virt machine is 0x40000000. We have >> to change the start_address and end_address for aarch64. >> * The downtime is changed from 0.001 to 0.1. Without this change, we saw >> migration stalled. This problem is still under analysis and needs to be >> resolved before removing RFC for this patch. >> >> The test code is as the following: >> >> .section .text > > >> #if defined(__linux__) >> @@ -125,6 +125,18 @@ unsigned char bootsect[] = { >> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa >> }; >> >> +unsigned char aarch64_kernel[] = { >> + 0x00, 0x10, 0x38, 0xd5, 0x00, 0xf8, 0x7f, 0x92, 0x00, 0x10, 0x18, 0xd5, >> + 0xdf, 0x3f, 0x03, 0xd5, 0x24, 0x08, 0x80, 0x52, 0x05, 0x20, 0xa1, 0xd2, >> + 0xa4, 0x00, 0x00, 0x39, 0x06, 0x00, 0x80, 0x52, 0x03, 0xc8, 0xa8, 0xd2, >> + 0x02, 0x02, 0xa8, 0xd2, 0x41, 0x00, 0x40, 0x39, 0x21, 0x04, 0x00, 0x11, >> + 0x41, 0x00, 0x00, 0x39, 0x42, 0x04, 0x40, 0x91, 0x5f, 0x00, 0x03, 0xeb, >> + 0x6b, 0xff, 0xff, 0x54, 0xc6, 0x04, 0x00, 0x11, 0xc6, 0x1c, 0x00, 0x12, >> + 0xdf, 0x00, 0x00, 0x71, 0xc1, 0xfe, 0xff, 0x54, 0x44, 0x08, 0x80, 0x52, >> + 0x05, 0x20, 0xa1, 0xd2, 0xa4, 0x00, 0x00, 0x39, 0xf2, 0xff, 0xff, 0x97 >> +}; >> +unsigned int aarch64_kernel_len = 96; > > I'm not really a fan of this steadily increasing number of hex-encoded > target binary blobs in the tests directory, but if we must, I > think this would be easier to read as an array of uint32_t, > so that each entry is one instruction word. This make sense given that ARM has fixed-length instructions. > > (If your claim is that nobody cares about the hex because > they'll just rebuild from the source code in the commit > message, I would suggest that that makes the source code > the 'preferred form for modification' under the GPL...) > Overall I agree with your comments. For this specific migration-test, PPC's approach is my preferred one. But unfortunately, ARM doesn't have a built-in firmware in QEMU to run a scripting test. So either we have to use a per-compiled binary blob, or find a way to build this blob from source inside QEMU. The second one might take a while to complete though. > But I think at some point we really need to stop doing > this and instead figure out a mechanism for building > target code as part of the QEMU build-and-test. > It is coming up increasingly often: > * code to run on the guest in tests > * the bios blobs (at the moment we special case > the x86 bios blobs and assume we can compile them > with the host C compiler, which is not great) > * for linux-user on several architectures we would like > to properly implement a guest VDSO > > thanks > -- PMM >
* Peter Maydell (peter.maydell@linaro.org) wrote: > On 15 December 2017 at 20:37, Wei Huang <wei@redhat.com> wrote: > > This patch adds the migration test support for aarch64. The test code, > > which implements the same functionality as x86, is compiled into a binary > > and booted as a kernel to qemu. Here are the design ideas: > > > > * We choose this -kernel design because aarch64 QEMU doesn't provide a > > built-in fw like x86 does. So instead of relying on a boot loader, we > > use -kernel approach for aarch64. > > * The serial output is sent to PL011 directly. > > * The physical memory base for mach-virt machine is 0x40000000. We have > > to change the start_address and end_address for aarch64. > > * The downtime is changed from 0.001 to 0.1. Without this change, we saw > > migration stalled. This problem is still under analysis and needs to be > > resolved before removing RFC for this patch. > > > > The test code is as the following: > > > > .section .text > > > > #if defined(__linux__) > > @@ -125,6 +125,18 @@ unsigned char bootsect[] = { > > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa > > }; > > > > +unsigned char aarch64_kernel[] = { > > + 0x00, 0x10, 0x38, 0xd5, 0x00, 0xf8, 0x7f, 0x92, 0x00, 0x10, 0x18, 0xd5, > > + 0xdf, 0x3f, 0x03, 0xd5, 0x24, 0x08, 0x80, 0x52, 0x05, 0x20, 0xa1, 0xd2, > > + 0xa4, 0x00, 0x00, 0x39, 0x06, 0x00, 0x80, 0x52, 0x03, 0xc8, 0xa8, 0xd2, > > + 0x02, 0x02, 0xa8, 0xd2, 0x41, 0x00, 0x40, 0x39, 0x21, 0x04, 0x00, 0x11, > > + 0x41, 0x00, 0x00, 0x39, 0x42, 0x04, 0x40, 0x91, 0x5f, 0x00, 0x03, 0xeb, > > + 0x6b, 0xff, 0xff, 0x54, 0xc6, 0x04, 0x00, 0x11, 0xc6, 0x1c, 0x00, 0x12, > > + 0xdf, 0x00, 0x00, 0x71, 0xc1, 0xfe, 0xff, 0x54, 0x44, 0x08, 0x80, 0x52, > > + 0x05, 0x20, 0xa1, 0xd2, 0xa4, 0x00, 0x00, 0x39, 0xf2, 0xff, 0xff, 0x97 > > +}; > > +unsigned int aarch64_kernel_len = 96; > > I'm not really a fan of this steadily increasing number of hex-encoded > target binary blobs in the tests directory, but if we must, I > think this would be easier to read as an array of uint32_t, > so that each entry is one instruction word. > > (If your claim is that nobody cares about the hex because > they'll just rebuild from the source code in the commit > message, I would suggest that that makes the source code > the 'preferred form for modification' under the GPL...) > > But I think at some point we really need to stop doing > this and instead figure out a mechanism for building > target code as part of the QEMU build-and-test. > It is coming up increasingly often: > * code to run on the guest in tests > * the bios blobs (at the moment we special case > the x86 bios blobs and assume we can compile them > with the host C compiler, which is not great) > * for linux-user on several architectures we would like > to properly implement a guest VDSO We could have: a) A source file b) A makefile rule to regenerate a #include chunk of hex c) The result of running (b) all committed, so that most people use the pregenerated (c) and don't have to worry about having cross-assemblers for everything, but those who want can regenerate using a+b. Dave > thanks > -- PMM -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
* Wei Huang (wei@redhat.com) wrote: > This patch adds the migration test support for aarch64. The test code, > which implements the same functionality as x86, is compiled into a binary > and booted as a kernel to qemu. Here are the design ideas: > > * We choose this -kernel design because aarch64 QEMU doesn't provide a > built-in fw like x86 does. So instead of relying on a boot loader, we > use -kernel approach for aarch64. > * The serial output is sent to PL011 directly. > * The physical memory base for mach-virt machine is 0x40000000. We have > to change the start_address and end_address for aarch64. > * The downtime is changed from 0.001 to 0.1. Without this change, we saw > migration stalled. This problem is still under analysis and needs to be > resolved before removing RFC for this patch. Where does the stall happen and what state is it in? That downtime is set really small so that migration does *not* complete without entering postcopy, so you should see it enter postcopy and then complete. Dave > The test code is as the following: > > .section .text > > .globl start > > start: > /* disable MMU to use phys mem address, just in case */ > mrs x0, sctlr_el1 > bic x0, x0, #(1<<0) > msr sctlr_el1, x0 > isb > > /* output char 'A' to PL011 */ > mov w4, #65 > mov x5, #0x9000000 > strb w4, [x5] > > /* w6 keeps a counter so we limit the output speed */ > mov w6, #0 > > mov x3, #(0x40000000 + 100 * 1024 * 1024) > mainloop: > mov x2, #(0x40000000 + 1024*1024) /* base addr = 0x40000000 */ > > innerloop: > ldrb w1, [x2] > add w1, w1, #1 > strb w1, [x2] > > add x2, x2, #(4096) > cmp x2, x3 > blt innerloop > > add w6, w6, #1 > and w6, w6, #(0xff) > cmp w6, #0 > bne mainloop > > /* output char 'B' to PL011 */ > mov w4, #66 > mov x5, #0x9000000 > strb w4, [x5] > > bl mainloop > > The code is compiled with the following command: > * gcc -c -o fill.o fill.s > * gcc -O2 -o fill.elf -Wl,-T,/tmp/flat.lds,--build-id=none,-Ttext=40080000 \ > -nostdlib fill.o > * objcopy -O binary fill.elf fill.flat > * xxd -i fill.flat > > The linker file (borrowed from KVM unit test) is defined as: > > SECTIONS > { > .text : { *(.init) *(.text) *(.text.*) } > . = ALIGN(64K); > etext = .; > .data : { > *(.data) > } > . = ALIGN(16); > .rodata : { *(.rodata) } > . = ALIGN(16); > .bss : { *(.bss) } > . = ALIGN(64K); > edata = .; > . += 64K; > . = ALIGN(64K); > /* > * stack depth is 16K for arm and PAGE_SIZE for arm64, see THREAD_SIZE > * sp must be 16 byte aligned for arm64, and 8 byte aligned for arm > * sp must always be strictly less than the true stacktop > */ > stackptr = . - 16; > stacktop = .; > } > > ENTRY(start) > > Signed-off-by: Wei Huang <wei@redhat.com> > --- > tests/Makefile.include | 1 + > tests/migration-test.c | 43 ++++++++++++++++++++++++++++++++++++++++--- > 2 files changed, 41 insertions(+), 3 deletions(-) > > diff --git a/tests/Makefile.include b/tests/Makefile.include > index c002352..d5828c4 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -357,6 +357,7 @@ check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF) > gcov-files-arm-y += hw/timer/arm_mptimer.c > > check-qtest-aarch64-y = tests/numa-test$(EXESUF) > +check-qtest-aarch64-y += tests/migration-test$(EXESUF) > > check-qtest-microblazeel-y = $(check-qtest-microblaze-y) > > diff --git a/tests/migration-test.c b/tests/migration-test.c > index be598d3..b0dd365 100644 > --- a/tests/migration-test.c > +++ b/tests/migration-test.c > @@ -22,8 +22,8 @@ > > #define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */ > > -const unsigned start_address = 1024 * 1024; > -const unsigned end_address = 100 * 1024 * 1024; > +unsigned start_address = 1024 * 1024; > +unsigned end_address = 100 * 1024 * 1024; > bool got_stop; > > #if defined(__linux__) > @@ -125,6 +125,18 @@ unsigned char bootsect[] = { > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa > }; > > +unsigned char aarch64_kernel[] = { > + 0x00, 0x10, 0x38, 0xd5, 0x00, 0xf8, 0x7f, 0x92, 0x00, 0x10, 0x18, 0xd5, > + 0xdf, 0x3f, 0x03, 0xd5, 0x24, 0x08, 0x80, 0x52, 0x05, 0x20, 0xa1, 0xd2, > + 0xa4, 0x00, 0x00, 0x39, 0x06, 0x00, 0x80, 0x52, 0x03, 0xc8, 0xa8, 0xd2, > + 0x02, 0x02, 0xa8, 0xd2, 0x41, 0x00, 0x40, 0x39, 0x21, 0x04, 0x00, 0x11, > + 0x41, 0x00, 0x00, 0x39, 0x42, 0x04, 0x40, 0x91, 0x5f, 0x00, 0x03, 0xeb, > + 0x6b, 0xff, 0xff, 0x54, 0xc6, 0x04, 0x00, 0x11, 0xc6, 0x1c, 0x00, 0x12, > + 0xdf, 0x00, 0x00, 0x71, 0xc1, 0xfe, 0xff, 0x54, 0x44, 0x08, 0x80, 0x52, > + 0x05, 0x20, 0xa1, 0xd2, 0xa4, 0x00, 0x00, 0x39, 0xf2, 0xff, 0xff, 0x97 > +}; > +unsigned int aarch64_kernel_len = 96; > + > static void init_bootfile_x86(const char *bootpath) > { > FILE *bootfile = fopen(bootpath, "wb"); > @@ -163,6 +175,15 @@ static void init_bootfile_ppc(const char *bootpath) > fclose(bootfile); > } > > +static void init_bootfile_aarch64(const char *bootpath) > +{ > + FILE *kernelfile = fopen(bootpath, "wb"); > + > + g_assert_cmpint(fwrite(aarch64_kernel, aarch64_kernel_len, 1, kernelfile), > + ==, 1); > + fclose(kernelfile); > +} > + > /* > * Wait for some output in the serial output file, > * we get an 'A' followed by an endless string of 'B's > @@ -470,6 +491,22 @@ static void test_migrate_start(QTestState **from, QTestState **to, > " -serial file:%s/dest_serial" > " -incoming %s", > accel, tmpfs, uri); > + } else if (strcmp(arch, "aarch64") == 0) { > + init_bootfile_aarch64(bootpath); > + cmd_src = g_strdup_printf("-machine virt,accel=kvm:cg -m 1024M" > + " -name vmsource,debug-threads=on -cpu host" > + " -serial file:%s/src_serial " > + " -kernel %s ", > + tmpfs, bootpath); > + cmd_dst = g_strdup_printf("-machine virt,accel=kvm:tcg -m 1024M" > + " -name vmdest,debug-threads=on -cpu host" > + " -serial file:%s/dest_serial" > + " -kernel %s" > + " -incoming %s", > + tmpfs, bootpath, uri); > + /* aarch64 virt machine physical mem started from 0x40000000 */ > + start_address += 0x40000000; > + end_address += 0x40000000; > } else { > g_assert_not_reached(); > } > @@ -530,7 +567,7 @@ static void test_migrate(void) > * machine, so also set the downtime. > */ > migrate_set_speed(from, "100000000"); > - migrate_set_downtime(from, 0.001); > + migrate_set_downtime(from, 0.1); > > /* Wait for the first serial output from the source */ > wait_for_serial("src_serial"); > -- > 1.8.3.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Wei Huang <wei@redhat.com> wrote: > This patch adds the migration test support for aarch64. The test code, > which implements the same functionality as x86, is compiled into a binary > and booted as a kernel to qemu. Here are the design ideas: > > * We choose this -kernel design because aarch64 QEMU doesn't provide a > built-in fw like x86 does. So instead of relying on a boot loader, we > use -kernel approach for aarch64. > * The serial output is sent to PL011 directly. > * The physical memory base for mach-virt machine is 0x40000000. We have > to change the start_address and end_address for aarch64. > * The downtime is changed from 0.001 to 0.1. Without this change, we saw > migration stalled. This problem is still under analysis and needs to be > resolved before removing RFC for this patch. Hi I don't have a good solution for how to go from ASM -> binary array, I think that Dave suggestion is good. > check-qtest-aarch64-y = tests/numa-test$(EXESUF) > +check-qtest-aarch64-y += tests/migration-test$(EXESUF) > > check-qtest-microblazeel-y = $(check-qtest-microblaze-y) > > diff --git a/tests/migration-test.c b/tests/migration-test.c > index be598d3..b0dd365 100644 > --- a/tests/migration-test.c > +++ b/tests/migration-test.c > @@ -22,8 +22,8 @@ > > #define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */ > > -const unsigned start_address = 1024 * 1024; > -const unsigned end_address = 100 * 1024 * 1024; > +unsigned start_address = 1024 * 1024; > +unsigned end_address = 100 * 1024 * 1024; > bool got_stop; > > #if defined(__linux__) > @@ -125,6 +125,18 @@ unsigned char bootsect[] = { > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa > }; > > +unsigned char aarch64_kernel[] = { > + 0x00, 0x10, 0x38, 0xd5, 0x00, 0xf8, 0x7f, 0x92, 0x00, 0x10, 0x18, 0xd5, > + 0xdf, 0x3f, 0x03, 0xd5, 0x24, 0x08, 0x80, 0x52, 0x05, 0x20, 0xa1, 0xd2, > + 0xa4, 0x00, 0x00, 0x39, 0x06, 0x00, 0x80, 0x52, 0x03, 0xc8, 0xa8, 0xd2, > + 0x02, 0x02, 0xa8, 0xd2, 0x41, 0x00, 0x40, 0x39, 0x21, 0x04, 0x00, 0x11, > + 0x41, 0x00, 0x00, 0x39, 0x42, 0x04, 0x40, 0x91, 0x5f, 0x00, 0x03, 0xeb, > + 0x6b, 0xff, 0xff, 0x54, 0xc6, 0x04, 0x00, 0x11, 0xc6, 0x1c, 0x00, 0x12, > + 0xdf, 0x00, 0x00, 0x71, 0xc1, 0xfe, 0xff, 0x54, 0x44, 0x08, 0x80, 0x52, > + 0x05, 0x20, 0xa1, 0xd2, 0xa4, 0x00, 0x00, 0x39, 0xf2, 0xff, 0xff, 0x97 > +}; > +unsigned int aarch64_kernel_len = 96; Just wondering, what is wrong with sizeof(aarch64_kernel)? (Yes, existing code already do it this bad.) > + > static void init_bootfile_x86(const char *bootpath) > { > FILE *bootfile = fopen(bootpath, "wb"); > @@ -163,6 +175,15 @@ static void init_bootfile_ppc(const char *bootpath) > fclose(bootfile); > } > > +static void init_bootfile_aarch64(const char *bootpath) > +{ > + FILE *kernelfile = fopen(bootpath, "wb"); > + > + g_assert_cmpint(fwrite(aarch64_kernel, aarch64_kernel_len, 1, kernelfile), > + ==, 1); > + fclose(kernelfile); > +} This function is identical to init_bootfile_x86(), just that we are using aarch64_kernel instead of bootsect. Would be a good idea to abstract it? > @@ -470,6 +491,22 @@ static void test_migrate_start(QTestState **from, QTestState **to, > " -serial file:%s/dest_serial" > " -incoming %s", > accel, tmpfs, uri); > + } else if (strcmp(arch, "aarch64") == 0) { > + init_bootfile_aarch64(bootpath); > + cmd_src = g_strdup_printf("-machine virt,accel=kvm:cg -m > 1024M" We are missing a "t" in "tcg", right? I see that PC uses 150M, PPC uses 256M and now arm uses 1G. Any reason for that? > + " -name vmsource,debug-threads=on -cpu host" > + " -serial file:%s/src_serial " > + " -kernel %s ", > + x tmpfs, bootpath); > + cmd_dst = g_strdup_printf("-machine virt,accel=kvm:tcg -m 1024M" > + " -name vmdest,debug-threads=on -cpu host" > + " -serial file:%s/dest_serial" > + " -kernel %s" > + " -incoming %s", > + tmpfs, bootpath, uri); > + /* aarch64 virt machine physical mem started from 0x40000000 */ > + start_address += 0x40000000; > + end_address += 0x40000000; > } else { > g_assert_not_reached(); > } > @@ -530,7 +567,7 @@ static void test_migrate(void) > * machine, so also set the downtime. > */ > migrate_set_speed(from, "100000000"); > - migrate_set_downtime(from, 0.001); > + migrate_set_downtime(from, 0.1); Why? I guess this is a leftover from previous versions of upstream and can be dropped. > /* Wait for the first serial output from the source */ > wait_for_serial("src_serial"); I agree with the patch in general, only the two nickpits that I pointed need to be changed. But once that everybody is looking, I would like to open a discussion about how to make more abstract this test, and not adding so many bits each time that we need to create a new machine. And once that we are here, I *think* that the ppc test is wrong, it is missing the -drive-file on destination, no? And once here, does -cpu host make sense only for arm, or should we do it for all archs? Thanks, Juan.
On 01/04/2018 02:10 PM, Juan Quintela wrote: > Wei Huang <wei@redhat.com> wrote: >> This patch adds the migration test support for aarch64. The test code, >> which implements the same functionality as x86, is compiled into a binary >> and booted as a kernel to qemu. Here are the design ideas: >> >> * We choose this -kernel design because aarch64 QEMU doesn't provide a >> built-in fw like x86 does. So instead of relying on a boot loader, we >> use -kernel approach for aarch64. >> * The serial output is sent to PL011 directly. >> * The physical memory base for mach-virt machine is 0x40000000. We have >> to change the start_address and end_address for aarch64. >> * The downtime is changed from 0.001 to 0.1. Without this change, we saw >> migration stalled. This problem is still under analysis and needs to be >> resolved before removing RFC for this patch. > > > Hi > > I don't have a good solution for how to go from ASM -> binary array, I > think that Dave suggestion is good. This is related to a bug Dave Gilbert recently fixed. The reason is because there were 16 4K dirty pages unaccounted during migration. It is impossible to migrate them under 0.001 downtime. We are good now with this issue now. > >> check-qtest-aarch64-y = tests/numa-test$(EXESUF) >> +check-qtest-aarch64-y += tests/migration-test$(EXESUF) >> >> check-qtest-microblazeel-y = $(check-qtest-microblaze-y) >> >> diff --git a/tests/migration-test.c b/tests/migration-test.c >> index be598d3..b0dd365 100644 >> --- a/tests/migration-test.c >> +++ b/tests/migration-test.c >> @@ -22,8 +22,8 @@ >> >> #define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */ >> >> -const unsigned start_address = 1024 * 1024; >> -const unsigned end_address = 100 * 1024 * 1024; >> +unsigned start_address = 1024 * 1024; >> +unsigned end_address = 100 * 1024 * 1024; >> bool got_stop; >> >> #if defined(__linux__) >> @@ -125,6 +125,18 @@ unsigned char bootsect[] = { >> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa >> }; >> >> +unsigned char aarch64_kernel[] = { >> + 0x00, 0x10, 0x38, 0xd5, 0x00, 0xf8, 0x7f, 0x92, 0x00, 0x10, 0x18, 0xd5, >> + 0xdf, 0x3f, 0x03, 0xd5, 0x24, 0x08, 0x80, 0x52, 0x05, 0x20, 0xa1, 0xd2, >> + 0xa4, 0x00, 0x00, 0x39, 0x06, 0x00, 0x80, 0x52, 0x03, 0xc8, 0xa8, 0xd2, >> + 0x02, 0x02, 0xa8, 0xd2, 0x41, 0x00, 0x40, 0x39, 0x21, 0x04, 0x00, 0x11, >> + 0x41, 0x00, 0x00, 0x39, 0x42, 0x04, 0x40, 0x91, 0x5f, 0x00, 0x03, 0xeb, >> + 0x6b, 0xff, 0xff, 0x54, 0xc6, 0x04, 0x00, 0x11, 0xc6, 0x1c, 0x00, 0x12, >> + 0xdf, 0x00, 0x00, 0x71, 0xc1, 0xfe, 0xff, 0x54, 0x44, 0x08, 0x80, 0x52, >> + 0x05, 0x20, 0xa1, 0xd2, 0xa4, 0x00, 0x00, 0x39, 0xf2, 0xff, 0xff, 0x97 >> +}; >> +unsigned int aarch64_kernel_len = 96; > > Just wondering, what is wrong with sizeof(aarch64_kernel)? > > (Yes, existing code already do it this bad.) > Yes, we can fix both of them together. > > > >> + >> static void init_bootfile_x86(const char *bootpath) >> { >> FILE *bootfile = fopen(bootpath, "wb"); >> @@ -163,6 +175,15 @@ static void init_bootfile_ppc(const char *bootpath) >> fclose(bootfile); >> } >> >> +static void init_bootfile_aarch64(const char *bootpath) >> +{ >> + FILE *kernelfile = fopen(bootpath, "wb"); >> + >> + g_assert_cmpint(fwrite(aarch64_kernel, aarch64_kernel_len, 1, kernelfile), >> + ==, 1); >> + fclose(kernelfile); >> +} > > This function is identical to init_bootfile_x86(), just that we are > using aarch64_kernel instead of bootsect. Would be a good idea to > abstract it? Sure > >> @@ -470,6 +491,22 @@ static void test_migrate_start(QTestState **from, QTestState **to, >> " -serial file:%s/dest_serial" >> " -incoming %s", >> accel, tmpfs, uri); >> + } else if (strcmp(arch, "aarch64") == 0) { >> + init_bootfile_aarch64(bootpath); >> + cmd_src = g_strdup_printf("-machine virt,accel=kvm:cg -m >> 1024M" > > We are missing a "t" in "tcg", right? > > I see that PC uses 150M, PPC uses 256M and now arm uses 1G. Any reason > for that? It should be kvm:tcg in this case. Also my goal is to use 150M for AArch64. But I was facing another issue (under debug) which made me to try different memory sizes. The final version will use 150M. > > >> + " -name vmsource,debug-threads=on -cpu host" >> + " -serial file:%s/src_serial " >> + " -kernel %s ", >> + x tmpfs, bootpath); >> + cmd_dst = g_strdup_printf("-machine virt,accel=kvm:tcg -m 1024M" >> + " -name vmdest,debug-threads=on -cpu host" >> + " -serial file:%s/dest_serial" >> + " -kernel %s" >> + " -incoming %s", >> + tmpfs, bootpath, uri); >> + /* aarch64 virt machine physical mem started from 0x40000000 */ >> + start_address += 0x40000000; >> + end_address += 0x40000000; >> } else { >> g_assert_not_reached(); >> } >> @@ -530,7 +567,7 @@ static void test_migrate(void) >> * machine, so also set the downtime. >> */ >> migrate_set_speed(from, "100000000"); >> - migrate_set_downtime(from, 0.001); >> + migrate_set_downtime(from, 0.1); > > Why? I guess this is a leftover from previous versions of upstream and > can be dropped. According to Dave, migrate_set_downtime(from, 0.001) is to force postcopy. But on aarch64, it can't pass. So I had to use 0.1 temporarily. Dave had fixed it (see above). > > >> /* Wait for the first serial output from the source */ >> wait_for_serial("src_serial"); > > I agree with the patch in general, only the two nickpits that I pointed > need to be changed. > > > But once that everybody is looking, I would like to open a discussion > about how to make more abstract this test, and not adding so many bits > each time that we need to create a new machine. The test cases themselves are the most annoying ones. x86/aarch64 use assembly (converted to binaries); but ppc uses scripts. How to find a common solution to suite all arch's? > > And once that we are here, I *think* that the ppc test is wrong, it is > missing the -drive-file on destination, no? [cc'ing Laurent] > > And once here, does -cpu host make sense only for arm, or should we do > it for all archs? I think x86 and aarch64 are OK with it. But I am not sure about PPC. > > Thanks, Juan. >
* Wei Huang (wei@redhat.com) wrote: > > > On 01/04/2018 02:10 PM, Juan Quintela wrote: > > Wei Huang <wei@redhat.com> wrote: > > But once that everybody is looking, I would like to open a discussion > > about how to make more abstract this test, and not adding so many bits > > each time that we need to create a new machine. > > The test cases themselves are the most annoying ones. x86/aarch64 use > assembly (converted to binaries); but ppc uses scripts. How to find a > common solution to suite all arch's? The asm/binaries is the main issue since reading/editing the binary is horrible. Power's forth is tiny and almost readable. If we want we could move it into a separate .c file somewhere just as a string but tat's a minor issue. Dave > > > > And once that we are here, I *think* that the ppc test is wrong, it is > > missing the -drive-file on destination, no? > > [cc'ing Laurent] > > > > > And once here, does -cpu host make sense only for arm, or should we do > > it for all archs? > > I think x86 and aarch64 are OK with it. But I am not sure about PPC. > > > > > Thanks, Juan. > > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/tests/Makefile.include b/tests/Makefile.include index c002352..d5828c4 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -357,6 +357,7 @@ check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF) gcov-files-arm-y += hw/timer/arm_mptimer.c check-qtest-aarch64-y = tests/numa-test$(EXESUF) +check-qtest-aarch64-y += tests/migration-test$(EXESUF) check-qtest-microblazeel-y = $(check-qtest-microblaze-y) diff --git a/tests/migration-test.c b/tests/migration-test.c index be598d3..b0dd365 100644 --- a/tests/migration-test.c +++ b/tests/migration-test.c @@ -22,8 +22,8 @@ #define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */ -const unsigned start_address = 1024 * 1024; -const unsigned end_address = 100 * 1024 * 1024; +unsigned start_address = 1024 * 1024; +unsigned end_address = 100 * 1024 * 1024; bool got_stop; #if defined(__linux__) @@ -125,6 +125,18 @@ unsigned char bootsect[] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa }; +unsigned char aarch64_kernel[] = { + 0x00, 0x10, 0x38, 0xd5, 0x00, 0xf8, 0x7f, 0x92, 0x00, 0x10, 0x18, 0xd5, + 0xdf, 0x3f, 0x03, 0xd5, 0x24, 0x08, 0x80, 0x52, 0x05, 0x20, 0xa1, 0xd2, + 0xa4, 0x00, 0x00, 0x39, 0x06, 0x00, 0x80, 0x52, 0x03, 0xc8, 0xa8, 0xd2, + 0x02, 0x02, 0xa8, 0xd2, 0x41, 0x00, 0x40, 0x39, 0x21, 0x04, 0x00, 0x11, + 0x41, 0x00, 0x00, 0x39, 0x42, 0x04, 0x40, 0x91, 0x5f, 0x00, 0x03, 0xeb, + 0x6b, 0xff, 0xff, 0x54, 0xc6, 0x04, 0x00, 0x11, 0xc6, 0x1c, 0x00, 0x12, + 0xdf, 0x00, 0x00, 0x71, 0xc1, 0xfe, 0xff, 0x54, 0x44, 0x08, 0x80, 0x52, + 0x05, 0x20, 0xa1, 0xd2, 0xa4, 0x00, 0x00, 0x39, 0xf2, 0xff, 0xff, 0x97 +}; +unsigned int aarch64_kernel_len = 96; + static void init_bootfile_x86(const char *bootpath) { FILE *bootfile = fopen(bootpath, "wb"); @@ -163,6 +175,15 @@ static void init_bootfile_ppc(const char *bootpath) fclose(bootfile); } +static void init_bootfile_aarch64(const char *bootpath) +{ + FILE *kernelfile = fopen(bootpath, "wb"); + + g_assert_cmpint(fwrite(aarch64_kernel, aarch64_kernel_len, 1, kernelfile), + ==, 1); + fclose(kernelfile); +} + /* * Wait for some output in the serial output file, * we get an 'A' followed by an endless string of 'B's @@ -470,6 +491,22 @@ static void test_migrate_start(QTestState **from, QTestState **to, " -serial file:%s/dest_serial" " -incoming %s", accel, tmpfs, uri); + } else if (strcmp(arch, "aarch64") == 0) { + init_bootfile_aarch64(bootpath); + cmd_src = g_strdup_printf("-machine virt,accel=kvm:cg -m 1024M" + " -name vmsource,debug-threads=on -cpu host" + " -serial file:%s/src_serial " + " -kernel %s ", + tmpfs, bootpath); + cmd_dst = g_strdup_printf("-machine virt,accel=kvm:tcg -m 1024M" + " -name vmdest,debug-threads=on -cpu host" + " -serial file:%s/dest_serial" + " -kernel %s" + " -incoming %s", + tmpfs, bootpath, uri); + /* aarch64 virt machine physical mem started from 0x40000000 */ + start_address += 0x40000000; + end_address += 0x40000000; } else { g_assert_not_reached(); } @@ -530,7 +567,7 @@ static void test_migrate(void) * machine, so also set the downtime. */ migrate_set_speed(from, "100000000"); - migrate_set_downtime(from, 0.001); + migrate_set_downtime(from, 0.1); /* Wait for the first serial output from the source */ wait_for_serial("src_serial");
This patch adds the migration test support for aarch64. The test code, which implements the same functionality as x86, is compiled into a binary and booted as a kernel to qemu. Here are the design ideas: * We choose this -kernel design because aarch64 QEMU doesn't provide a built-in fw like x86 does. So instead of relying on a boot loader, we use -kernel approach for aarch64. * The serial output is sent to PL011 directly. * The physical memory base for mach-virt machine is 0x40000000. We have to change the start_address and end_address for aarch64. * The downtime is changed from 0.001 to 0.1. Without this change, we saw migration stalled. This problem is still under analysis and needs to be resolved before removing RFC for this patch. The test code is as the following: .section .text .globl start start: /* disable MMU to use phys mem address, just in case */ mrs x0, sctlr_el1 bic x0, x0, #(1<<0) msr sctlr_el1, x0 isb /* output char 'A' to PL011 */ mov w4, #65 mov x5, #0x9000000 strb w4, [x5] /* w6 keeps a counter so we limit the output speed */ mov w6, #0 mov x3, #(0x40000000 + 100 * 1024 * 1024) mainloop: mov x2, #(0x40000000 + 1024*1024) /* base addr = 0x40000000 */ innerloop: ldrb w1, [x2] add w1, w1, #1 strb w1, [x2] add x2, x2, #(4096) cmp x2, x3 blt innerloop add w6, w6, #1 and w6, w6, #(0xff) cmp w6, #0 bne mainloop /* output char 'B' to PL011 */ mov w4, #66 mov x5, #0x9000000 strb w4, [x5] bl mainloop The code is compiled with the following command: * gcc -c -o fill.o fill.s * gcc -O2 -o fill.elf -Wl,-T,/tmp/flat.lds,--build-id=none,-Ttext=40080000 \ -nostdlib fill.o * objcopy -O binary fill.elf fill.flat * xxd -i fill.flat The linker file (borrowed from KVM unit test) is defined as: SECTIONS { .text : { *(.init) *(.text) *(.text.*) } . = ALIGN(64K); etext = .; .data : { *(.data) } . = ALIGN(16); .rodata : { *(.rodata) } . = ALIGN(16); .bss : { *(.bss) } . = ALIGN(64K); edata = .; . += 64K; . = ALIGN(64K); /* * stack depth is 16K for arm and PAGE_SIZE for arm64, see THREAD_SIZE * sp must be 16 byte aligned for arm64, and 8 byte aligned for arm * sp must always be strictly less than the true stacktop */ stackptr = . - 16; stacktop = .; } ENTRY(start) Signed-off-by: Wei Huang <wei@redhat.com> --- tests/Makefile.include | 1 + tests/migration-test.c | 43 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 41 insertions(+), 3 deletions(-)