Wei Huang <w...@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.
