On 02/12/2018 11:31 AM, Andrew Jones wrote: > On Fri, Feb 09, 2018 at 04:42:42PM -0500, Wei Huang wrote: >> This patch adds migration test support for aarch64. The test code, which >> implements the same functionality as x86, is booted as a kernel in qemu. >> Here are the design choices we make for aarch64: >> >> * We choose this -kernel approach 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 change >> the start_address and end_address for aarch64. >> >> In addition to providing the binary, this patch also includes the test source >> and the build script in tests/migration. So users can change/re-compile >> the binary as they wish. >> >> Signed-off-by: Wei Huang <w...@redhat.com> >> --- >> tests/Makefile.include | 1 + >> tests/migration-test.c | 29 ++++++++++--- >> tests/migration/aarch64-a-b-kernel.h | 19 +++++++++ >> tests/migration/aarch64-a-b-kernel.s | 67 >> +++++++++++++++++++++++++++++++ >> tests/migration/rebuild-aarch64-kernel.sh | 67 >> +++++++++++++++++++++++++++++++ >> 5 files changed, 177 insertions(+), 6 deletions(-) >> create mode 100644 tests/migration/aarch64-a-b-kernel.h >> create mode 100644 tests/migration/aarch64-a-b-kernel.s >> create mode 100755 tests/migration/rebuild-aarch64-kernel.sh >> >> diff --git a/tests/Makefile.include b/tests/Makefile.include >> index f41da23..0fd18fd 100644 >> --- a/tests/Makefile.include >> +++ b/tests/Makefile.include >> @@ -369,6 +369,7 @@ gcov-files-arm-y += hw/timer/arm_mptimer.c >> check-qtest-arm-y += tests/boot-serial-test$(EXESUF) >> >> 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 85d4014..b16944c 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__) >> @@ -80,12 +80,13 @@ static const char *tmpfs; >> * outputing a 'B' every so often if it's still running. >> */ >> #include "tests/migration/x86-a-b-bootblock.h" >> +#include "tests/migration/aarch64-a-b-kernel.h" >> >> -static void init_bootfile_x86(const char *bootpath) >> +static void init_bootfile(const char *bootpath, void *content) >> { >> FILE *bootfile = fopen(bootpath, "wb"); >> >> - g_assert_cmpint(fwrite(x86_bootsect, 512, 1, bootfile), ==, 1); >> + g_assert_cmpint(fwrite(content, 512, 1, bootfile), ==, 1); >> fclose(bootfile); >> } >> >> @@ -391,7 +392,7 @@ static void test_migrate_start(QTestState **from, >> QTestState **to, >> got_stop = false; >> >> if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { >> - init_bootfile_x86(bootpath); >> + init_bootfile(bootpath, x86_bootsect); >> cmd_src = g_strdup_printf("-machine accel=%s -m 150M" >> " -name source,debug-threads=on" >> " -serial file:%s/src_serial" >> @@ -420,6 +421,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(bootpath, aarch64_kernel); >> + cmd_src = g_strdup_printf("-machine virt,accel=kvm:tcg -m 150M " >> + "-name vmsource,debug-threads=on -cpu >> host " > > We can't use '-cpu host' with tcg, so the accel fallback won't work.
Will fix > >> + "-serial file:%s/src_serial " >> + "-kernel %s ", >> + tmpfs, bootpath); >> + cmd_dst = g_strdup_printf("-machine virt,accel=kvm:tcg -m 150M " >> + "-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 */ > > s/mem started from/memory starts at/ will fix > >> + start_address += 0x40000000; >> + end_address += 0x40000000; > > Why is end_address == start_address? No, these two variables have been defined above. We just need to shift them to new locations. > >> } else { >> g_assert_not_reached(); >> } >> @@ -501,7 +518,7 @@ static void test_deprecated(void) >> { >> QTestState *from; >> >> - from = qtest_start(""); >> + from = qtest_start("-machine none"); >> >> deprecated_set_downtime(from, 0.12345); >> deprecated_set_speed(from, "12345"); >> diff --git a/tests/migration/aarch64-a-b-kernel.h >> b/tests/migration/aarch64-a-b-kernel.h >> new file mode 100644 >> index 0000000..5bdc74b >> --- /dev/null >> +++ b/tests/migration/aarch64-a-b-kernel.h >> @@ -0,0 +1,19 @@ >> +/* This file is automatically generated from >> + * tests/migration/aarch64-a-b-kernel.s, edit that and then run >> + * tests/migration/rebuild-aarch64-kernel.sh to update, and then >> + * remember to send both in your patch submission. >> + */ >> +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, 0x01, 0x00, 0x80, 0x52, 0x41, 0x00, 0x00, 0x39, >> + 0x42, 0x04, 0x40, 0x91, 0x5f, 0x00, 0x03, 0xeb, 0xad, 0xff, 0xff, 0x54, >> + 0x02, 0x02, 0xa8, 0xd2, 0x22, 0x7e, 0x0b, 0xd5, 0x41, 0x00, 0x40, 0x39, >> + 0x21, 0x04, 0x00, 0x11, 0x21, 0x1c, 0x00, 0x12, 0x41, 0x00, 0x00, 0x39, >> + 0x42, 0x04, 0x40, 0x91, 0x5f, 0x00, 0x03, 0xeb, 0x2b, 0xff, 0xff, 0x54, >> + 0xc6, 0x04, 0x00, 0x11, 0xc6, 0x1c, 0x00, 0x12, 0xdf, 0x00, 0x00, 0x71, >> + 0x81, 0xfe, 0xff, 0x54, 0x44, 0x08, 0x80, 0x52, 0x05, 0x20, 0xa1, 0xd2, >> + 0xa4, 0x00, 0x00, 0x39, 0xf0, 0xff, 0xff, 0x97 >> +}; >> + >> diff --git a/tests/migration/aarch64-a-b-kernel.s >> b/tests/migration/aarch64-a-b-kernel.s >> new file mode 100644 >> index 0000000..bc20c81 >> --- /dev/null >> +++ b/tests/migration/aarch64-a-b-kernel.s >> @@ -0,0 +1,67 @@ >> +#!/bin/sh >> +# Copyright (c) 2018 Red Hat, Inc. and/or its affiliates >> +# >> +# Authors: >> +# Wei Huang <w...@redhat.com> >> +# >> +# This work is licensed under the terms of the GNU GPL, version 2 or later. >> +# See the COPYING file in the top-level directory. >> + >> +.section .text >> + >> + .globl start >> + >> +start: >> + /* disable MMU to use phys mem address */ >> + mrs x0, sctlr_el1 >> + bic x0, x0, #(1<<0) >> + msr sctlr_el1, x0 >> + isb > > The MMU is always initially off, so this isn't really necessary, but OK. > >> + >> + /* output char 'A' to PL011 */ >> + mov w4, #65 >> + mov x5, #0x9000000 >> + strb w4, [x5] >> + >> + /* w6 keeps a counter so we can limit the output speed */ >> + mov w6, #0 >> + >> + /* phys mem base addr = 0x40000000 */ >> + mov x3, #(0x40000000 + 100 *1024*1024) /* traverse 1M-100M */ >> + mov x2, #(0x40000000 + 1 * 1024*1024) > > How about creating a tests include file that contains this memory base > address so we don't have to scatter it around too many files? We can > throw in other defines too. > > tests/arm-mach-virt.h: > #define ARM_MACH_VIRT_UART 0x09000000 > #define ARM_MACH_VIRT_PHYS_BASE 0x40000000 > #define ARM_MACH_VIRT_A_B_START 0x40000000 > #define ARM_MACH_VIRT_A_B_END (0x40000000 * 100) Will fix with ".set" > >> + >> + /* clean up memory first */ >> + mov w1, #0 >> +clean: >> + strb w1, [x2] >> + add x2, x2, #(4096) >> + cmp x2, x3 >> + ble clean > > Why are we only clearing the first byte of each 4K chunk? Anyway QEMU > always gives us clean memory, so this isn't really necessary. Unfortunately this isn't always true. I wrote a small program to check guest VM's memory on AArch64 (every 4KB). It turned that three locations have non-zero content: Addr=0x40100000, Content=0x10 Addr=0x40110000, Content=0x10 Addr=0x44c01000, Content=0x6d > >> + >> + /* main body */ >> +mainloop: >> + mov x2, #(0x40000000 + 1 * 1024*1024) > > nit: This reassignment of x2 would be unnecessary if you copied x2 into > a scratch register (x7) prior to the clean loop, and then used the > scratch register there. It's the same number of instructions, but > less hard coded constants floating around. Will fix > >> + >> +innerloop: >> + /* clean cache because el2 might still cache guest data under KVM */ >> + dc civac, x2 > > Blank line here and a comment introducing the next block would be nice. > >> + ldrb w1, [x2] >> + add w1, w1, #1 >> + and w1, w1, #(0xff) >> + strb w1, [x2] >> + >> + add x2, x2, #(4096) >> + cmp x2, x3 >> + blt innerloop > > OK, I guess we only care about the first byte of each 4K chunk. > >> + >> + add w6, w6, #1 >> + and w6, w6, #(0xff) >> + cmp w6, #0 >> + bne mainloop > > So we limit the output speed by doing the write loop 256 times? How was > the number 256 selected? If we want to delay the outputs we can use the > timer counter to ensure we only output every N us. I pulled together > some code that implements a 100 us delay: > > mrs x0, cntfrq_el0 > mov x1, #10000 > udiv x1, x0, x1 > mrs x0, cntvct_el0 > add x0, x0, x1 > 1: isb > mrs x1, cntvct_el0 > subs x1, x0, x1 > b.gt 1b > 256 is a number I picked after trying on aarch64 server. I think I will stick with the original design. The reason is that we don't want to rely on hardware availability (timer) in such a short program. 256 is a good enough choice which serves the purpose and still is very portable. >> + >> + /* output char 'B' to PL011 */ >> + mov w4, #66 >> + mov x5, #0x9000000 > > You never overwrote x5, so you don't need to rewrite the uart addr here. > >> + strb w4, [x5] >> + >> + bl mainloop >> diff --git a/tests/migration/rebuild-aarch64-kernel.sh >> b/tests/migration/rebuild-aarch64-kernel.sh >> new file mode 100755 >> index 0000000..0fbca99 >> --- /dev/null >> +++ b/tests/migration/rebuild-aarch64-kernel.sh >> @@ -0,0 +1,67 @@ >> +#!/bin/sh >> +# Copyright (c) 2018 Red Hat, Inc. and/or its affiliates >> +# >> +# Authors: >> +# Wei Huang <w...@redhat.com> >> +# >> +# This work is licensed under the terms of the GNU GPL, version 2 or later. >> +# See the COPYING file in the top-level directory. >> + >> +ASMFILE=tests/migration/aarch64-a-b-kernel.s >> +HEADER=tests/migration/aarch64-a-b-kernel.h >> + >> +if [ ! -e "$ASMFILE" ] >> +then >> + echo "Couldn't find $ASMFILE" >&2 >> + exit 1 >> +fi >> + >> +ASM_WORK_DIR=/tmp/AARCH64BB$$ > > Same mktemp comment as in David's patch. Will do > >> + >> +mkdir $ASM_WORK_DIR && >> +cat <<EOF > "$ASM_WORK_DIR/linker.lds" && >> +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) >> +EOF >> +as -march="armv8-a" -c "$ASMFILE" -o $ASM_WORK_DIR/kernel.o && >> +gcc -O2 -o $ASM_WORK_DIR/kernel.elf -nostdlib \ >> + -Wl,-T,$ASM_WORK_DIR/linker.lds,--build-id=none,-Ttext=40080000 \ >> + $ASM_WORK_DIR/kernel.o && > > You don't need the above linker script nor the 'as' line. Just do > > gcc -o $ASM_WORK_DIR/kernel.elf \ > -nostdlib -Wl,--build-id=none,-Ttext=40080000 $ASMFILE > Will do >> +objcopy -O binary $ASM_WORK_DIR/kernel.elf $ASM_WORK_DIR/kernel.flat && > > Shouldn't gcc and objcopy have a $CROSS_COMPILER prefix variable > prepended? Does the QEMU test build environment support cross > compiling? Can this script get build environment variables some how? > I think this is a good point. For cross compilation, I decided to change this re-build script into a Makefile (also merged with Dave Gilbert's version). >> +xxd -i $ASM_WORK_DIR/kernel.flat | >> +sed -e 's/_tmp.*_kernel_flat/aarch64_kernel/' -e 's/.*int.*//' > \ >> + $ASM_WORK_DIR/kernel.hex && >> +cat - $ASM_WORK_DIR/kernel.hex <<EOF > "$HEADER" >> +/* This file is automatically generated from >> + * tests/migration/aarch64-a-b-kernel.s, edit that and then run >> + * tests/migration/rebuild-aarch64-kernel.sh to update, and then >> + * remember to send both in your patch submission. >> + */ >> +EOF >> + >> +rm $ASM_WORK_DIR/kernel.hex $ASM_WORK_DIR/kernel.flat \ >> + $ASM_WORK_DIR/kernel.elf $ASM_WORK_DIR/kernel.o $ASM_WORK_DIR/linker.lds >> +rmdir $ASM_WORK_DIR >> -- >> 1.8.3.1 >> >> > > Thanks, > drew >