* [Qemu-devel] [PATCH RFC 1/1] tests: Add migration test for aarch64
@ 2017-12-15 20:37 Wei Huang
2017-12-16 13:49 ` Peter Maydell
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Wei Huang @ 2017-12-15 20:37 UTC (permalink / raw
To: qemu-devel; +Cc: dgilbert, quintela, wei
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(-)
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 1/1] tests: Add migration test for aarch64
2017-12-15 20:37 [Qemu-devel] [PATCH RFC 1/1] tests: Add migration test for aarch64 Wei Huang
@ 2017-12-16 13:49 ` Peter Maydell
2017-12-18 4:54 ` Wei Huang
2017-12-18 9:54 ` Dr. David Alan Gilbert
2017-12-18 10:03 ` Dr. David Alan Gilbert
2018-01-04 20:10 ` Juan Quintela
2 siblings, 2 replies; 8+ messages in thread
From: Peter Maydell @ 2017-12-16 13:49 UTC (permalink / raw
To: Wei Huang; +Cc: QEMU Developers, Dr. David Alan Gilbert, Juan Quintela
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 1/1] tests: Add migration test for aarch64
2017-12-16 13:49 ` Peter Maydell
@ 2017-12-18 4:54 ` Wei Huang
2017-12-18 9:54 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 8+ messages in thread
From: Wei Huang @ 2017-12-18 4:54 UTC (permalink / raw
To: Peter Maydell; +Cc: Juan Quintela, QEMU Developers, Dr. David Alan Gilbert
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 1/1] tests: Add migration test for aarch64
2017-12-16 13:49 ` Peter Maydell
2017-12-18 4:54 ` Wei Huang
@ 2017-12-18 9:54 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 8+ messages in thread
From: Dr. David Alan Gilbert @ 2017-12-18 9:54 UTC (permalink / raw
To: Peter Maydell; +Cc: Wei Huang, QEMU Developers, Juan Quintela
* 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 1/1] tests: Add migration test for aarch64
2017-12-15 20:37 [Qemu-devel] [PATCH RFC 1/1] tests: Add migration test for aarch64 Wei Huang
2017-12-16 13:49 ` Peter Maydell
@ 2017-12-18 10:03 ` Dr. David Alan Gilbert
2018-01-04 20:10 ` Juan Quintela
2 siblings, 0 replies; 8+ messages in thread
From: Dr. David Alan Gilbert @ 2017-12-18 10:03 UTC (permalink / raw
To: Wei Huang; +Cc: qemu-devel, quintela
* 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 1/1] tests: Add migration test for aarch64
2017-12-15 20:37 [Qemu-devel] [PATCH RFC 1/1] tests: Add migration test for aarch64 Wei Huang
2017-12-16 13:49 ` Peter Maydell
2017-12-18 10:03 ` Dr. David Alan Gilbert
@ 2018-01-04 20:10 ` Juan Quintela
2018-01-05 17:39 ` Wei Huang
2 siblings, 1 reply; 8+ messages in thread
From: Juan Quintela @ 2018-01-04 20:10 UTC (permalink / raw
To: Wei Huang; +Cc: qemu-devel, dgilbert
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 1/1] tests: Add migration test for aarch64
2018-01-04 20:10 ` Juan Quintela
@ 2018-01-05 17:39 ` Wei Huang
2018-01-05 17:57 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 8+ messages in thread
From: Wei Huang @ 2018-01-05 17:39 UTC (permalink / raw
To: quintela; +Cc: qemu-devel, Dr. David Alan Gilbert, Laurent Vivier
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.
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 1/1] tests: Add migration test for aarch64
2018-01-05 17:39 ` Wei Huang
@ 2018-01-05 17:57 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 8+ messages in thread
From: Dr. David Alan Gilbert @ 2018-01-05 17:57 UTC (permalink / raw
To: Wei Huang; +Cc: quintela, Laurent Vivier, qemu-devel
* 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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-01-05 17:58 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-15 20:37 [Qemu-devel] [PATCH RFC 1/1] tests: Add migration test for aarch64 Wei Huang
2017-12-16 13:49 ` Peter Maydell
2017-12-18 4:54 ` Wei Huang
2017-12-18 9:54 ` Dr. David Alan Gilbert
2017-12-18 10:03 ` Dr. David Alan Gilbert
2018-01-04 20:10 ` Juan Quintela
2018-01-05 17:39 ` Wei Huang
2018-01-05 17:57 ` Dr. David Alan Gilbert
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.