[Qemu-devel] [PATCH V8 0/4] tests: Add migration test for aarch64

[Qemu-devel] [PATCH V8 0/4] tests: Add migration test for aarch64

[Wei Huang]

This patchset adds a migration test for aarch64. It leverages
Dave Gilbert's migration boot block patches to create a new test case
for aarch64.

V7->V8:
 * Support cross compilation by searching for CROSS_CC_GUEST option,
   instead of using the find-cross-prefix defined in roms/Makefile
 * Use the "max" options for ARM guest VM's CPU and GIC types
 * $(TARGET)/Makefile rules are rewritten based on Laurent Vivier's comment
 * NOTE: because Patch 1/2 is re-written, I remove the "Reviewed-by" for
   reviewers to take a look again. Thanks.

V6->V7:
 * Define test memory start/end addresses for all architectures
 * Check aarch64 kernel binary size, limit under 512KB

V5->V6:
 * Add Reviewed-by to patch 1-3
 * Add more design notes in patch 4 (aarch64 assembly compilation, bin space)

V4->V5:
 * Extract cross compilation detection code into rules.mak for sharing
 * Minor comment and code revision in migration-test.c & aarch64-a-b-kernel.S
 
V3->V4:
 * Rename .s to .S, allowing assembly to include C-style header file
 * Move test defines into a new migration-test.h file
 * Use different cpu & gic settings for kvm and tcg modes on aarch64
 * Clean up aarch64-a-b-kernel.S based on Andrew Jones' comments
 
V2->V3:
 * Convert build script to Makefile
 * Add cross-compilation support
 * Fix CPU type for "tcg" machine type
 * Revise asm code and the compilation process from asm to header file

V1->V2:
 * Similar to Dave Gilbert's recent changes to migration-test, we
   provide the test source and a build script in V2.
 * aarch64 kernel blob is defined as "unsigned char" because the source
   is now provided in V2.
 * Add "-machine none" to test_deprecated() because aarch64 doesn't have
   a default machine type.

RFC->V1:
 * aarch64 kernel blob is defined as an uint32_t array
 * The test code is re-written to address a data caching issue under KVM.
   Tests passed under both x86 and aarch64.
 * Re-use init_bootfile_x86() for both x86 and aarch64
 * Other minor fixes

Thanks,
-Wei

Wei Huang (4):
  tests/migration: Convert x86 boot block compilation script into
    Makefile
  tests/migration: Support cross compiler in generating boot block
    header file
  tests/migration: Add migration-test header file
  tests: Add migration test for aarch64

 tests/Makefile.include                             |  1 +
 tests/migration-test.c                             | 68 +++++++++++++++-----
 tests/migration/Makefile                           | 34 ++++++++++
 tests/migration/aarch64/Makefile                   | 20 ++++++
 tests/migration/aarch64/aarch64-a-b-kernel.S       | 75 ++++++++++++++++++++++
 tests/migration/aarch64/aarch64-a-b-kernel.h       | 19 ++++++
 tests/migration/migration-test.h                   | 30 +++++++++
 tests/migration/rebuild-x86-bootblock.sh           | 33 ----------
 tests/migration/x86_64/Makefile                    | 22 +++++++
 .../x86-a-b-bootblock.S}                           |  0
 tests/migration/{ => x86_64}/x86-a-b-bootblock.h   |  8 +--
 11 files changed, 258 insertions(+), 52 deletions(-)
 create mode 100644 tests/migration/Makefile
 create mode 100644 tests/migration/aarch64/Makefile
 create mode 100644 tests/migration/aarch64/aarch64-a-b-kernel.S
 create mode 100644 tests/migration/aarch64/aarch64-a-b-kernel.h
 create mode 100644 tests/migration/migration-test.h
 delete mode 100755 tests/migration/rebuild-x86-bootblock.sh
 create mode 100644 tests/migration/x86_64/Makefile
 rename tests/migration/{x86-a-b-bootblock.s => x86_64/x86-a-b-bootblock.S} (100%)
 rename tests/migration/{ => x86_64}/x86-a-b-bootblock.h (93%)

-- 
1.8.3.1

[Qemu-devel] [PATCH V8 1/4] tests/migration: Convert x86 boot block compilation script into Makefile

[Wei Huang]

The x86 boot block header currently is generated with a shell script.
To better support other CPUs (e.g. aarch64), we convert the script
into Makefile. This allows us to 1) support cross-compilation easily,
and 2) avoid creating a script file for every architecture.

Note that, in the new design, the cross compiler prefix can be specified by
setting the CROSS_PREFIX in "make" command. Also to allow gcc pre-processor
to include the C-style file correctly, it also renames the
x86-a-b-bootblock.s file extension from .s to .S.

Signed-off-by: Wei Huang <wei@redhat.com>
---
 tests/migration/Makefile                           | 31 ++++++++++++++++++++
 tests/migration/rebuild-x86-bootblock.sh           | 33 ----------------------
 .../{x86-a-b-bootblock.s => x86-a-b-bootblock.S}   |  0
 3 files changed, 31 insertions(+), 33 deletions(-)
 create mode 100644 tests/migration/Makefile
 delete mode 100755 tests/migration/rebuild-x86-bootblock.sh
 rename tests/migration/{x86-a-b-bootblock.s => x86-a-b-bootblock.S} (100%)

diff --git a/tests/migration/Makefile b/tests/migration/Makefile
new file mode 100644
index 0000000..5d5fa07
--- /dev/null
+++ b/tests/migration/Makefile
@@ -0,0 +1,31 @@
+# To specify cross compiler prefix, use CROSS_PREFIX=
+#   > make CROSS_PREFIX=x86_64-linux-gnu-
+
+override define __note
+/* This file is automatically generated from
+ * tests/migration/x86-a-b-bootblock.s, edit that and then run
+ * tests/migration/rebuild-x86-bootblock.sh to update,
+ * and then remember to send both in your patch submission.
+ */
+endef
+export __note
+
+.PHONY: all clean
+all: x86-a-b-bootblock.h
+
+x86-a-b-bootblock.h: x86.bootsect
+	echo "$$__note" > header.tmp
+	xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
+	mv header.tmp $@
+
+x86.bootsect: x86.boot
+	dd if=$< of=$@ bs=256 count=2 skip=124
+
+x86.boot: x86.o
+	$(CROSS_PREFIX)objcopy -O binary $< $@
+
+x86.o: x86-a-b-bootblock.S
+	$(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
+
+clean:
+	@rm -rf *.boot *.o *.bootsect
diff --git a/tests/migration/rebuild-x86-bootblock.sh b/tests/migration/rebuild-x86-bootblock.sh
deleted file mode 100755
index 86cec5d..0000000
--- a/tests/migration/rebuild-x86-bootblock.sh
+++ /dev/null
@@ -1,33 +0,0 @@
-#!/bin/sh
-# Copyright (c) 2016-2018 Red Hat, Inc. and/or its affiliates
-# This work is licensed under the terms of the GNU GPL, version 2 or later.
-# See the COPYING file in the top-level directory.
-#
-# Author: dgilbert@redhat.com
-
-ASMFILE=$PWD/tests/migration/x86-a-b-bootblock.s
-HEADER=$PWD/tests/migration/x86-a-b-bootblock.h
-
-if [ ! -e "$ASMFILE" ]
-then
-  echo "Couldn't find $ASMFILE" >&2
-  exit 1
-fi
-
-ASM_WORK_DIR=$(mktemp -d --tmpdir X86BB.XXXXXX)
-cd "$ASM_WORK_DIR" &&
-as --32 -march=i486 "$ASMFILE" -o x86.o &&
-objcopy -O binary x86.o x86.boot &&
-dd if=x86.boot of=x86.bootsect bs=256 count=2 skip=124 &&
-xxd -i x86.bootsect |
-sed -e 's/.*int.*//' > x86.hex &&
-cat - x86.hex <<HERE > "$HEADER"
-/* This file is automatically generated from
- * tests/migration/x86-a-b-bootblock.s, edit that and then run
- * tests/migration/rebuild-x86-bootblock.sh to update,
- * and then remember to send both in your patch submission.
- */
-HERE
-
-rm x86.hex x86.bootsect x86.boot x86.o
-cd .. && rmdir "$ASM_WORK_DIR"
diff --git a/tests/migration/x86-a-b-bootblock.s b/tests/migration/x86-a-b-bootblock.S
similarity index 100%
rename from tests/migration/x86-a-b-bootblock.s
rename to tests/migration/x86-a-b-bootblock.S
-- 
1.8.3.1

[Qemu-devel] [PATCH V8 2/4] tests/migration: Support cross compilation in generating boot header file

[Wei Huang]

Recently a new configure option, CROSS_CC_GUEST, was added to
$(TARGET)-softmmu/config-target.mak to support TCG-related tests. This
patch tries to leverage this option to support cross compilation when the
migration boot block file is being re-generated:

 * The x86_64 related files are moved to a new sub-dir (named ./x86_64).
 * A new top-layer Makefile is created in tests/migration/ directory.
   This Makefile searches and parses CROSS_CC_GUEST to generate CROSS_PREFIX.
   The CROSS_PREFIX, if available, is then passed to migration/$ARCH/Makefile.

Signed-off-by: Wei Huang <wei@redhat.com>
---
 tests/migration-test.c                           |  2 +-
 tests/migration/Makefile                         | 43 +++++++++++++-----------
 tests/migration/x86_64/Makefile                  | 22 ++++++++++++
 tests/migration/{ => x86_64}/x86-a-b-bootblock.S |  0
 tests/migration/{ => x86_64}/x86-a-b-bootblock.h |  8 ++---
 5 files changed, 50 insertions(+), 25 deletions(-)
 create mode 100644 tests/migration/x86_64/Makefile
 rename tests/migration/{ => x86_64}/x86-a-b-bootblock.S (100%)
 rename tests/migration/{ => x86_64}/x86-a-b-bootblock.h (93%)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 0e687b7..c4d79e9 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -83,7 +83,7 @@ static const char *tmpfs;
 /* A simple PC boot sector that modifies memory (1-100MB) quickly
  * outputting a 'B' every so often if it's still running.
  */
-#include "tests/migration/x86-a-b-bootblock.h"
+#include "tests/migration/x86_64/x86-a-b-bootblock.h"
 
 static void init_bootfile_x86(const char *bootpath)
 {
diff --git a/tests/migration/Makefile b/tests/migration/Makefile
index 5d5fa07..1de480e 100644
--- a/tests/migration/Makefile
+++ b/tests/migration/Makefile
@@ -1,31 +1,34 @@
-# To specify cross compiler prefix, use CROSS_PREFIX=
-#   > make CROSS_PREFIX=x86_64-linux-gnu-
+#
+# Copyright (c) 2018 Red Hat, Inc. and/or its affiliates
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+# See the COPYING file in the top-level directory.
+#
+
+TARGET_LIST = x86_64
+
+SRC_PATH = ../..
 
 override define __note
-/* This file is automatically generated from
- * tests/migration/x86-a-b-bootblock.s, edit that and then run
- * tests/migration/rebuild-x86-bootblock.sh to update,
- * and then remember to send both in your patch submission.
+/* This file is automatically generated from the assembly file in
+ * tests/migration/$@, edit that and then run "make all"
+ * inside tests/migration to update, and then remember to send both
+ * in your patch submission.
  */
 endef
 export __note
 
-.PHONY: all clean
-all: x86-a-b-bootblock.h
-
-x86-a-b-bootblock.h: x86.bootsect
-	echo "$$__note" > header.tmp
-	xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
-	mv header.tmp $@
+parse-cross-prefix = $(subst gcc,,$(patsubst cc,gcc,$(patsubst CROSS_CC_GUEST="%",%,$(shell grep "CROSS_CC_GUEST=" $(SRC_PATH)/$(1)-softmmu/config-target.mak))))
+gen-cross-prefix = $(patsubst %-,CROSS_PREFIX=%-,$(call parse-cross-prefix,$(1)))
 
-x86.bootsect: x86.boot
-	dd if=$< of=$@ bs=256 count=2 skip=124
+.PHONY: all $(TARGET_LIST)
 
-x86.boot: x86.o
-	$(CROSS_PREFIX)objcopy -O binary $< $@
+all: $(TARGET_LIST)
 
-x86.o: x86-a-b-bootblock.S
-	$(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
+$(TARGET_LIST):
+	$(MAKE) -C $@ $(call gen-cross-prefix,$@)
 
 clean:
-	@rm -rf *.boot *.o *.bootsect
+	for target in $(TARGET_LIST); do \
+		$(MAKE) -C $$target clean; \
+	done
diff --git a/tests/migration/x86_64/Makefile b/tests/migration/x86_64/Makefile
new file mode 100644
index 0000000..6dc849f
--- /dev/null
+++ b/tests/migration/x86_64/Makefile
@@ -0,0 +1,22 @@
+# To specify cross compiler prefix, use CROSS_PREFIX=
+#   > make CROSS_PREFIX=x86_64-linux-gnu-
+
+.PHONY: all clean
+all: x86-a-b-bootblock.h
+
+x86-a-b-bootblock.h: x86.bootsect
+	echo "$$__note" > header.tmp
+	xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
+	mv header.tmp $@
+
+x86.bootsect: x86.boot
+	dd if=$< of=$@ bs=256 count=2 skip=124
+
+x86.boot: x86.o
+	$(CROSS_PREFIX)objcopy -O binary $< $@
+
+x86.o: x86-a-b-bootblock.S
+	$(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
+
+clean:
+	@rm -rf *.boot *.o *.bootsect
diff --git a/tests/migration/x86-a-b-bootblock.S b/tests/migration/x86_64/x86-a-b-bootblock.S
similarity index 100%
rename from tests/migration/x86-a-b-bootblock.S
rename to tests/migration/x86_64/x86-a-b-bootblock.S
diff --git a/tests/migration/x86-a-b-bootblock.h b/tests/migration/x86_64/x86-a-b-bootblock.h
similarity index 93%
rename from tests/migration/x86-a-b-bootblock.h
rename to tests/migration/x86_64/x86-a-b-bootblock.h
index 78a151f..73670ef 100644
--- a/tests/migration/x86-a-b-bootblock.h
+++ b/tests/migration/x86_64/x86-a-b-bootblock.h
@@ -1,7 +1,7 @@
-/* This file is automatically generated from
- * tests/migration/x86-a-b-bootblock.s, edit that and then run
- * tests/migration/rebuild-x86-bootblock.sh to update,
- * and then remember to send both in your patch submission.
+/* This file is automatically generated from the assembly file in
+ * tests/migration/x86_64, edit that and then run "make all"
+ * inside tests/migration to update, and then remember to send both
+ * in your patch submission.
  */
 unsigned char x86_bootsect[] = {
   0xfa, 0x0f, 0x01, 0x16, 0x74, 0x7c, 0x66, 0xb8, 0x01, 0x00, 0x00, 0x00,
-- 
1.8.3.1

[Qemu-devel] [PATCH V8 3/4] tests/migration: Add migration-test header file

[Wei Huang]

This patch moves the settings related migration-test from the
migration-test.c file to a new header file.

Signed-off-by: Wei Huang <wei@redhat.com>
---
 tests/migration-test.c           | 28 ++++++++++++++++++----------
 tests/migration/migration-test.h | 21 +++++++++++++++++++++
 2 files changed, 39 insertions(+), 10 deletions(-)
 create mode 100644 tests/migration/migration-test.h

diff --git a/tests/migration-test.c b/tests/migration-test.c
index c4d79e9..bf60fa0 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -21,11 +21,13 @@
 #include "chardev/char.h"
 #include "sysemu/sysemu.h"
 
+#include "migration/migration-test.h"
+
 /* TODO actually test the results and get rid of this */
 #define qtest_qmp_discard_response(...) qobject_unref(qtest_qmp(__VA_ARGS__))
 
-const unsigned start_address = 1024 * 1024;
-const unsigned end_address = 100 * 1024 * 1024;
+unsigned start_address;
+unsigned end_address;
 bool got_stop;
 static bool uffd_feature_thread_id;
 
@@ -80,8 +82,8 @@ static bool ufd_version_check(void)
 
 static const char *tmpfs;
 
-/* A simple PC boot sector that modifies memory (1-100MB) quickly
- * outputting a 'B' every so often if it's still running.
+/* The boot file modifies memory area in [start_address, end_address)
+ * repeatedly. It outputs a 'B' at a fixed rate while it's still running.
  */
 #include "tests/migration/x86_64/x86-a-b-bootblock.h"
 
@@ -270,11 +272,11 @@ static void wait_for_migration_pass(QTestState *who)
 static void check_guests_ram(QTestState *who)
 {
     /* Our ASM test will have been incrementing one byte from each page from
-     * 1MB to <100MB in order.
-     * This gives us a constraint that any page's byte should be equal or less
-     * than the previous pages byte (mod 256); and they should all be equal
-     * except for one transition at the point where we meet the incrementer.
-     * (We're running this with the guest stopped).
+     * start_address to < end_address in order. This gives us a constraint
+     * that any page's byte should be equal or less than the previous pages
+     * byte (mod 256); and they should all be equal except for one transition
+     * at the point where we meet the incrementer. (We're running this with
+     * the guest stopped).
      */
     unsigned address;
     uint8_t first_byte;
@@ -285,7 +287,8 @@ static void check_guests_ram(QTestState *who)
     qtest_memread(who, start_address, &first_byte, 1);
     last_byte = first_byte;
 
-    for (address = start_address + 4096; address < end_address; address += 4096)
+    for (address = start_address + TEST_MEM_PAGE_SIZE; address < end_address;
+         address += TEST_MEM_PAGE_SIZE)
     {
         uint8_t b;
         qtest_memread(who, address, &b, 1);
@@ -437,6 +440,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
                                   " -drive file=%s,format=raw"
                                   " -incoming %s",
                                   accel, tmpfs, bootpath, uri);
+        start_address = X86_TEST_MEM_START;
+        end_address = X86_TEST_MEM_END;
     } else if (strcmp(arch, "ppc64") == 0) {
         cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
                                   " -name source,debug-threads=on"
@@ -451,6 +456,9 @@ static int test_migrate_start(QTestState **from, QTestState **to,
                                   " -serial file:%s/dest_serial"
                                   " -incoming %s",
                                   accel, tmpfs, uri);
+
+        start_address = PPC_TEST_MEM_START;
+        end_address = PPC_TEST_MEM_END;
     } else {
         g_assert_not_reached();
     }
diff --git a/tests/migration/migration-test.h b/tests/migration/migration-test.h
new file mode 100644
index 0000000..c4c0c52
--- /dev/null
+++ b/tests/migration/migration-test.h
@@ -0,0 +1,21 @@
+/*
+ * Copyright (c) 2018 Red Hat, Inc. and/or its affiliates
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef _TEST_MIGRATION_H_
+#define _TEST_MIGRATION_H_
+
+/* Common */
+#define TEST_MEM_PAGE_SIZE 4096
+
+/* x86 */
+#define X86_TEST_MEM_START (1 * 1024 * 1024)
+#define X86_TEST_MEM_END   (100 * 1024 * 1024)
+
+/* PPC */
+#define PPC_TEST_MEM_START (1 * 1024 * 1024)
+#define PPC_TEST_MEM_END   (100 * 1024 * 1024)
+
+#endif /* _TEST_MIGRATION_H_ */
-- 
1.8.3.1

[Qemu-devel] [PATCH V8 4/4] tests: Add migration test for aarch64

[Wei Huang]

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 source
code and the build script in tests/migration/aarch64. So users can change
the source and/or re-compile the binary as they wish.

Signed-off-by: Wei Huang <wei@redhat.com>
---
 tests/Makefile.include                       |  1 +
 tests/migration-test.c                       | 38 ++++++++++++--
 tests/migration/Makefile                     |  2 +-
 tests/migration/aarch64/Makefile             | 20 ++++++++
 tests/migration/aarch64/aarch64-a-b-kernel.S | 75 ++++++++++++++++++++++++++++
 tests/migration/aarch64/aarch64-a-b-kernel.h | 19 +++++++
 tests/migration/migration-test.h             |  9 ++++
 7 files changed, 159 insertions(+), 5 deletions(-)
 create mode 100644 tests/migration/aarch64/Makefile
 create mode 100644 tests/migration/aarch64/aarch64-a-b-kernel.S
 create mode 100644 tests/migration/aarch64/aarch64-a-b-kernel.h

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 87c81d1..fab8fb9 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -390,6 +390,7 @@ check-qtest-arm-y += tests/hexloader-test$(EXESUF)
 check-qtest-aarch64-y = tests/numa-test$(EXESUF)
 check-qtest-aarch64-$(CONFIG_SDHCI) += tests/sdhci-test$(EXESUF)
 check-qtest-aarch64-y += tests/boot-serial-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 bf60fa0..9df7bce 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -34,6 +34,7 @@ static bool uffd_feature_thread_id;
 #if defined(__linux__)
 #include <sys/syscall.h>
 #include <sys/vfs.h>
+#include <sys/utsname.h>
 #endif
 
 #if defined(__linux__) && defined(__NR_userfaultfd) && defined(CONFIG_EVENTFD)
@@ -86,12 +87,13 @@ static const char *tmpfs;
  * repeatedly. It outputs a 'B' at a fixed rate while it's still running.
  */
 #include "tests/migration/x86_64/x86-a-b-bootblock.h"
+#include "tests/migration/aarch64/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);
 }
 
@@ -428,7 +430,7 @@ static int 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"
@@ -459,6 +461,34 @@ static int test_migrate_start(QTestState **from, QTestState **to,
 
         start_address = PPC_TEST_MEM_START;
         end_address = PPC_TEST_MEM_END;
+    } else if (strcmp(arch, "aarch64") == 0) {
+        struct utsname utsname;
+
+        /* kvm and tcg need different cpu and gic-version configs */
+        if (access("/dev/kvm", F_OK) == 0 && uname(&utsname) == 0 &&
+            strcmp(utsname.machine, "aarch64") == 0) {
+            accel = "kvm";
+        } else {
+            accel = "tcg";
+        }
+
+        init_bootfile(bootpath, aarch64_kernel);
+        cmd_src = g_strdup_printf("-machine virt,accel=%s,gic-version=max "
+                                  "-name vmsource,debug-threads=on -cpu max "
+                                  "-m 150M -serial file:%s/src_serial "
+                                  "-kernel %s ",
+                                  accel, tmpfs, bootpath);
+        cmd_dst = g_strdup_printf("-machine virt,accel=%s,gic-version=max "
+                                  "-name vmdest,debug-threads=on -cpu max "
+                                  "-m 150M -serial file:%s/dest_serial "
+                                  "-kernel %s "
+                                  "-incoming %s ",
+                                  accel, tmpfs, bootpath, uri);
+
+        start_address = ARM_TEST_MEM_START;
+        end_address = ARM_TEST_MEM_END;
+
+        g_assert(sizeof(aarch64_kernel) <= ARM_TEST_MAX_KERNEL_SIZE);
     } else {
         g_assert_not_reached();
     }
@@ -545,7 +575,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/Makefile b/tests/migration/Makefile
index 1de480e..bbc11cb 100644
--- a/tests/migration/Makefile
+++ b/tests/migration/Makefile
@@ -5,7 +5,7 @@
 # See the COPYING file in the top-level directory.
 #
 
-TARGET_LIST = x86_64
+TARGET_LIST = x86_64 aarch64
 
 SRC_PATH = ../..
 
diff --git a/tests/migration/aarch64/Makefile b/tests/migration/aarch64/Makefile
new file mode 100644
index 0000000..e88e546
--- /dev/null
+++ b/tests/migration/aarch64/Makefile
@@ -0,0 +1,20 @@
+# To specify cross compiler prefix, use CROSS_PREFIX=
+#   > make CROSS_PREFIX=aarch64-linux-gnu-
+
+.PHONY: all clean
+all: aarch64-a-b-kernel.h
+
+aarch64-a-b-kernel.h: aarch64.kernel
+	echo "$$__note" > header.tmp
+	xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
+	mv header.tmp $@
+
+aarch64.kernel: aarch64.elf
+	$(CROSS_PREFIX)objcopy -O binary $< $@
+
+aarch64.elf: aarch64-a-b-kernel.S
+	$(CROSS_PREFIX)gcc -o $@ -nostdlib -Wl,--build-id=none $<
+
+clean:
+	@rm -rf *.kernel *.elf
+
diff --git a/tests/migration/aarch64/aarch64-a-b-kernel.S b/tests/migration/aarch64/aarch64-a-b-kernel.S
new file mode 100644
index 0000000..507af30
--- /dev/null
+++ b/tests/migration/aarch64/aarch64-a-b-kernel.S
@@ -0,0 +1,75 @@
+#
+# Copyright (c) 2018 Red Hat, Inc. and/or its affiliates
+#
+# Author:
+#   Wei Huang <wei@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.
+#
+# Note: Please make sure the compiler compiles the assembly code below with
+# pc-relative address. Also the branch instructions should use relative
+# addresses only.
+
+#include "../migration-test.h"
+
+.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
+
+        /* traverse test memory region */
+        mov     x0, #ARM_TEST_MEM_START
+        mov     x1, #ARM_TEST_MEM_END
+
+        /* output char 'A' to PL011 */
+        mov     w3, 'A'
+        mov     x2, #ARM_MACH_VIRT_UART
+        strb    w3, [x2]
+
+        /* clean up memory */
+        mov     w3, #0
+        mov     x4, x0
+clean:
+        strb    w3, [x4]
+        add     x4, x4, #TEST_MEM_PAGE_SIZE
+        cmp     x4, x1
+        ble     clean
+
+        /* w5 keeps a counter so we can limit the output speed */
+        mov     w5, #0
+
+        /* main body */
+mainloop:
+        mov     x4, x0
+
+innerloop:
+        /* clean cache because el2 might still cache guest data under KVM */
+        dc      civac, x4
+
+        /* increment the first byte of each page by 1 */
+        ldrb    w3, [x4]
+        add     w3, w3, #1
+        and     w3, w3, #0xff
+        strb    w3, [x4]
+
+        add     x4, x4, #TEST_MEM_PAGE_SIZE
+        cmp     x4, x1
+        blt     innerloop
+
+        add     w5, w5, #1
+        and     w5, w5, #0xff
+        cmp     w5, #0
+        bne     mainloop
+
+        /* output char 'B' to PL011 */
+        mov     w3, 'B'
+        strb    w3, [x2]
+
+        b       mainloop
diff --git a/tests/migration/aarch64/aarch64-a-b-kernel.h b/tests/migration/aarch64/aarch64-a-b-kernel.h
new file mode 100644
index 0000000..36e76ff
--- /dev/null
+++ b/tests/migration/aarch64/aarch64-a-b-kernel.h
@@ -0,0 +1,19 @@
+/* This file is automatically generated from the assembly file in
+ * tests/migration/aarch64, edit that and then run "make all"
+ * inside tests/migration 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, 0x00, 0x02, 0xa8, 0xd2, 0x01, 0xc8, 0xa8, 0xd2,
+  0x23, 0x08, 0x80, 0x52, 0x02, 0x20, 0xa1, 0xd2, 0x43, 0x00, 0x00, 0x39,
+  0x03, 0x00, 0x80, 0x52, 0xe4, 0x03, 0x00, 0xaa, 0x83, 0x00, 0x00, 0x39,
+  0x84, 0x04, 0x40, 0x91, 0x9f, 0x00, 0x01, 0xeb, 0xad, 0xff, 0xff, 0x54,
+  0x05, 0x00, 0x80, 0x52, 0xe4, 0x03, 0x00, 0xaa, 0x24, 0x7e, 0x0b, 0xd5,
+  0x83, 0x00, 0x40, 0x39, 0x63, 0x04, 0x00, 0x11, 0x63, 0x1c, 0x00, 0x12,
+  0x83, 0x00, 0x00, 0x39, 0x84, 0x04, 0x40, 0x91, 0x9f, 0x00, 0x01, 0xeb,
+  0x2b, 0xff, 0xff, 0x54, 0xa5, 0x04, 0x00, 0x11, 0xa5, 0x1c, 0x00, 0x12,
+  0xbf, 0x00, 0x00, 0x71, 0x81, 0xfe, 0xff, 0x54, 0x43, 0x08, 0x80, 0x52,
+  0x43, 0x00, 0x00, 0x39, 0xf1, 0xff, 0xff, 0x17
+};
+
diff --git a/tests/migration/migration-test.h b/tests/migration/migration-test.h
index c4c0c52..6939a13 100644
--- a/tests/migration/migration-test.h
+++ b/tests/migration/migration-test.h
@@ -18,4 +18,13 @@
 #define PPC_TEST_MEM_START (1 * 1024 * 1024)
 #define PPC_TEST_MEM_END   (100 * 1024 * 1024)
 
+/* ARM */
+#define ARM_TEST_MEM_START (0x40000000 + 1 * 1024 * 1024)
+#define ARM_TEST_MEM_END   (0x40000000 + 100 * 1024 * 1024)
+#define ARM_MACH_VIRT_UART 0x09000000
+/* AArch64 kernel load address is 0x40080000, and the test memory starts at
+ * 0x40100000. So the maximum allowable kernel size is 512KB.
+ */
+#define ARM_TEST_MAX_KERNEL_SIZE (512 * 1024)
+
 #endif /* _TEST_MIGRATION_H_ */
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH V8 4/4] tests: Add migration test for aarch64

[Peter Maydell]

On 1 September 2018 at 06:11, Wei Huang <wei@redhat.com> 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 source
> code and the build script in tests/migration/aarch64. So users can change
> the source and/or re-compile the binary as they wish.
> +    } else if (strcmp(arch, "aarch64") == 0) {
> +        struct utsname utsname;
> +

The comment says:

> +        /* kvm and tcg need different cpu and gic-version configs */

...but the code below it doesn't change the cpu or gic-version
based on 'accel'.

> +        if (access("/dev/kvm", F_OK) == 0 && uname(&utsname) == 0 &&
> +            strcmp(utsname.machine, "aarch64") == 0) {
> +            accel = "kvm";
> +        } else {
> +            accel = "tcg";
> +        }

Maybe we could just use -accel=kvm,tcg rather than guessing
whether kvm is going to work ? If this is how the x86 code
works then probably best to do the same as that, though.

> +
> +        init_bootfile(bootpath, aarch64_kernel);
> +        cmd_src = g_strdup_printf("-machine virt,accel=%s,gic-version=max "
> +                                  "-name vmsource,debug-threads=on -cpu max "
> +                                  "-m 150M -serial file:%s/src_serial "
> +                                  "-kernel %s ",
> +                                  accel, tmpfs, bootpath);
> +        cmd_dst = g_strdup_printf("-machine virt,accel=%s,gic-version=max "
> +                                  "-name vmdest,debug-threads=on -cpu max "
> +                                  "-m 150M -serial file:%s/dest_serial "
> +                                  "-kernel %s "
> +                                  "-incoming %s ",
> +                                  accel, tmpfs, bootpath, uri);
> +
> +        start_address = ARM_TEST_MEM_START;
> +        end_address = ARM_TEST_MEM_END;
> +
> +        g_assert(sizeof(aarch64_kernel) <= ARM_TEST_MAX_KERNEL_SIZE);

thanks
-- PMM

Re: [Qemu-devel] [PATCH V8 4/4] tests: Add migration test for aarch64

[Wei Huang]

On 09/01/2018 05:07 AM, Peter Maydell wrote:
> On 1 September 2018 at 06:11, Wei Huang <wei@redhat.com> 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 source
>> code and the build script in tests/migration/aarch64. So users can change
>> the source and/or re-compile the binary as they wish.
>> +    } else if (strcmp(arch, "aarch64") == 0) {
>> +        struct utsname utsname;
>> +
> 
> The comment says:
> 
>> +        /* kvm and tcg need different cpu and gic-version configs */
> 
> ...but the code below it doesn't change the cpu or gic-version
> based on 'accel'.

My bad. The code below can be removed.

> 
>> +        if (access("/dev/kvm", F_OK) == 0 && uname(&utsname) == 0 &&
>> +            strcmp(utsname.machine, "aarch64") == 0) {
>> +            accel = "kvm";
>> +        } else {
>> +            accel = "tcg";
>> +        }
> 
> Maybe we could just use -accel=kvm,tcg rather than guessing
> whether kvm is going to work ? If this is how the x86 code
> works then probably best to do the same as that, though.

Yes, "kvm,tcg" work work (I just verified with "tcg" only). I will
update the code after more comments are collected.

> 
>> +
>> +        init_bootfile(bootpath, aarch64_kernel);
>> +        cmd_src = g_strdup_printf("-machine virt,accel=%s,gic-version=max "
>> +                                  "-name vmsource,debug-threads=on -cpu max "
>> +                                  "-m 150M -serial file:%s/src_serial "
>> +                                  "-kernel %s ",
>> +                                  accel, tmpfs, bootpath);
>> +        cmd_dst = g_strdup_printf("-machine virt,accel=%s,gic-version=max "
>> +                                  "-name vmdest,debug-threads=on -cpu max "
>> +                                  "-m 150M -serial file:%s/dest_serial "
>> +                                  "-kernel %s "
>> +                                  "-incoming %s ",
>> +                                  accel, tmpfs, bootpath, uri);
>> +
>> +        start_address = ARM_TEST_MEM_START;
>> +        end_address = ARM_TEST_MEM_END;
>> +
>> +        g_assert(sizeof(aarch64_kernel) <= ARM_TEST_MAX_KERNEL_SIZE);
> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [PATCH V8 1/4] tests/migration: Convert x86 boot block compilation script into Makefile

[Juan Quintela]

Wei Huang <wei@redhat.com> wrote:
> The x86 boot block header currently is generated with a shell script.
> To better support other CPUs (e.g. aarch64), we convert the script
> into Makefile. This allows us to 1) support cross-compilation easily,
> and 2) avoid creating a script file for every architecture.
>
> Note that, in the new design, the cross compiler prefix can be specified by
> setting the CROSS_PREFIX in "make" command. Also to allow gcc pre-processor
> to include the C-style file correctly, it also renames the
> x86-a-b-bootblock.s file extension from .s to .S.
>
> Signed-off-by: Wei Huang <wei@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Re: [Qemu-devel] [PATCH V8 3/4] tests/migration: Add migration-test header file

[Juan Quintela]

Wei Huang <wei@redhat.com> wrote:
> This patch moves the settings related migration-test from the
> migration-test.c file to a new header file.
>
> Signed-off-by: Wei Huang <wei@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Re: [Qemu-devel] [PATCH V8 4/4] tests: Add migration test for aarch64

[Juan Quintela]

Wei Huang <wei@redhat.com> 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 source
> code and the build script in tests/migration/aarch64. So users can change
> the source and/or re-compile the binary as they wish.
>
> Signed-off-by: Wei Huang <wei@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

with the change spotted by Peter.

Thanks a lot for getting this done.  I was "porting" your previous
version to 3.0 because I thought that you had already gone through too
many "posts" just to get "general", not aarch64 bits fixed.

I have no idea about arm asm, so the reviewed by is for the other bits O:-)

Later, Juan.

Re: [Qemu-devel] [PATCH V8 2/4] tests/migration: Support cross compilation in generating boot header file

[Juan Quintela]

Wei Huang <wei@redhat.com> wrote:
> Recently a new configure option, CROSS_CC_GUEST, was added to
> $(TARGET)-softmmu/config-target.mak to support TCG-related tests. This
> patch tries to leverage this option to support cross compilation when the
> migration boot block file is being re-generated:
>
>  * The x86_64 related files are moved to a new sub-dir (named ./x86_64).
>  * A new top-layer Makefile is created in tests/migration/ directory.
>    This Makefile searches and parses CROSS_CC_GUEST to generate CROSS_PREFIX.
>    The CROSS_PREFIX, if available, is then passed to migration/$ARCH/Makefile.
>
> Signed-off-by: Wei Huang <wei@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Re: [Qemu-devel] [PATCH V8 1/4] tests/migration: Convert x86 boot block compilation script into Makefile

[Andrew Jones]

On Sat, Sep 01, 2018 at 01:11:12AM -0400, Wei Huang wrote:
> The x86 boot block header currently is generated with a shell script.
> To better support other CPUs (e.g. aarch64), we convert the script
> into Makefile. This allows us to 1) support cross-compilation easily,
> and 2) avoid creating a script file for every architecture.
> 
> Note that, in the new design, the cross compiler prefix can be specified by
> setting the CROSS_PREFIX in "make" command. Also to allow gcc pre-processor
> to include the C-style file correctly, it also renames the
> x86-a-b-bootblock.s file extension from .s to .S.
> 
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>  tests/migration/Makefile                           | 31 ++++++++++++++++++++
>  tests/migration/rebuild-x86-bootblock.sh           | 33 ----------------------
>  .../{x86-a-b-bootblock.s => x86-a-b-bootblock.S}   |  0
>  3 files changed, 31 insertions(+), 33 deletions(-)
>  create mode 100644 tests/migration/Makefile
>  delete mode 100755 tests/migration/rebuild-x86-bootblock.sh
>  rename tests/migration/{x86-a-b-bootblock.s => x86-a-b-bootblock.S} (100%)
> 
> diff --git a/tests/migration/Makefile b/tests/migration/Makefile
> new file mode 100644
> index 0000000..5d5fa07
> --- /dev/null
> +++ b/tests/migration/Makefile
> @@ -0,0 +1,31 @@
> +# To specify cross compiler prefix, use CROSS_PREFIX=
> +#   > make CROSS_PREFIX=x86_64-linux-gnu-
       ^ nit: this prompt symbol is weird (at least to me, what shell uses
              this?) To me it looks like a sh/bash redirect symbol. Can
              we change it to '$' or use nothing at all?
> +
> +override define __note
> +/* This file is automatically generated from
> + * tests/migration/x86-a-b-bootblock.s, edit that and then run
                                        ^ should be 'S'

> + * tests/migration/rebuild-x86-bootblock.sh to update,
> + * and then remember to send both in your patch submission.
> + */
> +endef
> +export __note
> +
> +.PHONY: all clean
> +all: x86-a-b-bootblock.h
> +
> +x86-a-b-bootblock.h: x86.bootsect
> +	echo "$$__note" > header.tmp
> +	xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
> +	mv header.tmp $@

The shell script this Makefile is replacing used mktemp
for a randomly named tmp dir. Shouldn't we continue to
use random names?

> +
> +x86.bootsect: x86.boot
> +	dd if=$< of=$@ bs=256 count=2 skip=124
> +
> +x86.boot: x86.o
> +	$(CROSS_PREFIX)objcopy -O binary $< $@
> +
> +x86.o: x86-a-b-bootblock.S
> +	$(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
> +
> +clean:
> +	@rm -rf *.boot *.o *.bootsect

We don't want to remove the generated header file when cleaning?

> diff --git a/tests/migration/rebuild-x86-bootblock.sh b/tests/migration/rebuild-x86-bootblock.sh
> deleted file mode 100755
> index 86cec5d..0000000
> --- a/tests/migration/rebuild-x86-bootblock.sh
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -#!/bin/sh
> -# Copyright (c) 2016-2018 Red Hat, Inc. and/or its affiliates
> -# This work is licensed under the terms of the GNU GPL, version 2 or later.
> -# See the COPYING file in the top-level directory.
> -#
> -# Author: dgilbert@redhat.com
> -
> -ASMFILE=$PWD/tests/migration/x86-a-b-bootblock.s
> -HEADER=$PWD/tests/migration/x86-a-b-bootblock.h
> -
> -if [ ! -e "$ASMFILE" ]
> -then
> -  echo "Couldn't find $ASMFILE" >&2
> -  exit 1
> -fi
> -
> -ASM_WORK_DIR=$(mktemp -d --tmpdir X86BB.XXXXXX)
> -cd "$ASM_WORK_DIR" &&
> -as --32 -march=i486 "$ASMFILE" -o x86.o &&
> -objcopy -O binary x86.o x86.boot &&
> -dd if=x86.boot of=x86.bootsect bs=256 count=2 skip=124 &&
> -xxd -i x86.bootsect |
> -sed -e 's/.*int.*//' > x86.hex &&
> -cat - x86.hex <<HERE > "$HEADER"
> -/* This file is automatically generated from
> - * tests/migration/x86-a-b-bootblock.s, edit that and then run
> - * tests/migration/rebuild-x86-bootblock.sh to update,
> - * and then remember to send both in your patch submission.
> - */
> -HERE
> -
> -rm x86.hex x86.bootsect x86.boot x86.o
> -cd .. && rmdir "$ASM_WORK_DIR"
> diff --git a/tests/migration/x86-a-b-bootblock.s b/tests/migration/x86-a-b-bootblock.S
> similarity index 100%
> rename from tests/migration/x86-a-b-bootblock.s
> rename to tests/migration/x86-a-b-bootblock.S
> -- 
> 1.8.3.1
> 
>

Thanks,
drew 

Re: [Qemu-devel] [PATCH V8 2/4] tests/migration: Support cross compilation in generating boot header file

[Andrew Jones]

On Sat, Sep 01, 2018 at 01:11:13AM -0400, Wei Huang wrote:
> Recently a new configure option, CROSS_CC_GUEST, was added to
> $(TARGET)-softmmu/config-target.mak to support TCG-related tests. This
> patch tries to leverage this option to support cross compilation when the
> migration boot block file is being re-generated:
> 
>  * The x86_64 related files are moved to a new sub-dir (named ./x86_64).
>  * A new top-layer Makefile is created in tests/migration/ directory.
>    This Makefile searches and parses CROSS_CC_GUEST to generate CROSS_PREFIX.
>    The CROSS_PREFIX, if available, is then passed to migration/$ARCH/Makefile.
> 
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>  tests/migration-test.c                           |  2 +-
>  tests/migration/Makefile                         | 43 +++++++++++++-----------
>  tests/migration/x86_64/Makefile                  | 22 ++++++++++++
>  tests/migration/{ => x86_64}/x86-a-b-bootblock.S |  0
>  tests/migration/{ => x86_64}/x86-a-b-bootblock.h |  8 ++---
>  5 files changed, 50 insertions(+), 25 deletions(-)
>  create mode 100644 tests/migration/x86_64/Makefile
>  rename tests/migration/{ => x86_64}/x86-a-b-bootblock.S (100%)
>  rename tests/migration/{ => x86_64}/x86-a-b-bootblock.h (93%)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 0e687b7..c4d79e9 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -83,7 +83,7 @@ static const char *tmpfs;
>  /* A simple PC boot sector that modifies memory (1-100MB) quickly
>   * outputting a 'B' every so often if it's still running.
>   */
> -#include "tests/migration/x86-a-b-bootblock.h"
> +#include "tests/migration/x86_64/x86-a-b-bootblock.h"

Is x86-a-b-bootblock.h specific to x86_64, or both i386 and x86_64?
I think we want the dir name to be i386 if it's the later. Also,
we could drop the 'x86-' prefix from the filename, now that the
directory identifies the arch.

>  
>  static void init_bootfile_x86(const char *bootpath)
>  {
> diff --git a/tests/migration/Makefile b/tests/migration/Makefile
> index 5d5fa07..1de480e 100644
> --- a/tests/migration/Makefile
> +++ b/tests/migration/Makefile
> @@ -1,31 +1,34 @@
> -# To specify cross compiler prefix, use CROSS_PREFIX=
> -#   > make CROSS_PREFIX=x86_64-linux-gnu-
> +#
> +# Copyright (c) 2018 Red Hat, Inc. and/or its affiliates
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or later.
> +# See the COPYING file in the top-level directory.
> +#
> +
> +TARGET_LIST = x86_64
> +
> +SRC_PATH = ../..
>  
>  override define __note
> -/* This file is automatically generated from
> - * tests/migration/x86-a-b-bootblock.s, edit that and then run
> - * tests/migration/rebuild-x86-bootblock.sh to update,
> - * and then remember to send both in your patch submission.
> +/* This file is automatically generated from the assembly file in
> + * tests/migration/$@, edit that and then run "make all"

nit: /, edit that/. Edit that file/

> + * inside tests/migration to update, and then remember to send both
> + * in your patch submission.
nit: ^ the header and the assembler differences

>   */
>  endef
>  export __note
>  
> -.PHONY: all clean
> -all: x86-a-b-bootblock.h
> -
> -x86-a-b-bootblock.h: x86.bootsect
> -	echo "$$__note" > header.tmp
> -	xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
> -	mv header.tmp $@
> +parse-cross-prefix = $(subst gcc,,$(patsubst cc,gcc,$(patsubst CROSS_CC_GUEST="%",%,$(shell grep "CROSS_CC_GUEST=" $(SRC_PATH)/$(1)-softmmu/config-target.mak))))
> +gen-cross-prefix = $(patsubst %-,CROSS_PREFIX=%-,$(call parse-cross-prefix,$(1)))
>  
> -x86.bootsect: x86.boot
> -	dd if=$< of=$@ bs=256 count=2 skip=124
> +.PHONY: all $(TARGET_LIST)
>  
> -x86.boot: x86.o
> -	$(CROSS_PREFIX)objcopy -O binary $< $@
> +all: $(TARGET_LIST)
>  
> -x86.o: x86-a-b-bootblock.S
> -	$(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
> +$(TARGET_LIST):
> +	$(MAKE) -C $@ $(call gen-cross-prefix,$@)
>  
>  clean:
> -	@rm -rf *.boot *.o *.bootsect
> +	for target in $(TARGET_LIST); do \
> +		$(MAKE) -C $$target clean; \
> +	done

The above seems overly complicated. Does the following work?

$(TARGET_LIST): CROSS_PREFIX=$(CROSS_CC_GUEST)
$(TARGET_LIST):
	$(MAKE) -C $@


> diff --git a/tests/migration/x86_64/Makefile b/tests/migration/x86_64/Makefile
> new file mode 100644
> index 0000000..6dc849f
> --- /dev/null
> +++ b/tests/migration/x86_64/Makefile
> @@ -0,0 +1,22 @@
> +# To specify cross compiler prefix, use CROSS_PREFIX=
> +#   > make CROSS_PREFIX=x86_64-linux-gnu-
> +
> +.PHONY: all clean
> +all: x86-a-b-bootblock.h
> +
> +x86-a-b-bootblock.h: x86.bootsect
> +	echo "$$__note" > header.tmp
> +	xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
> +	mv header.tmp $@
> +
> +x86.bootsect: x86.boot
> +	dd if=$< of=$@ bs=256 count=2 skip=124
> +
> +x86.boot: x86.o
> +	$(CROSS_PREFIX)objcopy -O binary $< $@
> +
> +x86.o: x86-a-b-bootblock.S
> +	$(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
> +
> +clean:
> +	@rm -rf *.boot *.o *.bootsect
> diff --git a/tests/migration/x86-a-b-bootblock.S b/tests/migration/x86_64/x86-a-b-bootblock.S
> similarity index 100%
> rename from tests/migration/x86-a-b-bootblock.S
> rename to tests/migration/x86_64/x86-a-b-bootblock.S
> diff --git a/tests/migration/x86-a-b-bootblock.h b/tests/migration/x86_64/x86-a-b-bootblock.h
> similarity index 93%
> rename from tests/migration/x86-a-b-bootblock.h
> rename to tests/migration/x86_64/x86-a-b-bootblock.h
> index 78a151f..73670ef 100644
> --- a/tests/migration/x86-a-b-bootblock.h
> +++ b/tests/migration/x86_64/x86-a-b-bootblock.h
> @@ -1,7 +1,7 @@
> -/* This file is automatically generated from
> - * tests/migration/x86-a-b-bootblock.s, edit that and then run
> - * tests/migration/rebuild-x86-bootblock.sh to update,
> - * and then remember to send both in your patch submission.
> +/* This file is automatically generated from the assembly file in
> + * tests/migration/x86_64, edit that and then run "make all"
> + * inside tests/migration to update, and then remember to send both
> + * in your patch submission.
>   */
>  unsigned char x86_bootsect[] = {
>    0xfa, 0x0f, 0x01, 0x16, 0x74, 0x7c, 0x66, 0xb8, 0x01, 0x00, 0x00, 0x00,
> -- 
> 1.8.3.1
> 
>

Thanks,
drew 

Re: [Qemu-devel] [PATCH V8 3/4] tests/migration: Add migration-test header file

[Andrew Jones]

On Sat, Sep 01, 2018 at 01:11:14AM -0400, Wei Huang wrote:
> This patch moves the settings related migration-test from the
> migration-test.c file to a new header file.
> 
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>  tests/migration-test.c           | 28 ++++++++++++++++++----------
>  tests/migration/migration-test.h | 21 +++++++++++++++++++++
>  2 files changed, 39 insertions(+), 10 deletions(-)
>  create mode 100644 tests/migration/migration-test.h
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [Qemu-devel] [PATCH V8 1/4] tests/migration: Convert x86 boot block compilation script into Makefile

[Juan Quintela]

Andrew Jones <drjones@redhat.com> wrote:
> On Sat, Sep 01, 2018 at 01:11:12AM -0400, Wei Huang wrote:
>> The x86 boot block header currently is generated with a shell script.
>> To better support other CPUs (e.g. aarch64), we convert the script
>> into Makefile. This allows us to 1) support cross-compilation easily,
>> and 2) avoid creating a script file for every architecture.
>> 
>> Note that, in the new design, the cross compiler prefix can be specified by
>> setting the CROSS_PREFIX in "make" command. Also to allow gcc pre-processor
>> to include the C-style file correctly, it also renames the
>> x86-a-b-bootblock.s file extension from .s to .S.
>> + * tests/migration/rebuild-x86-bootblock.sh to update,
>> + * and then remember to send both in your patch submission.
>> + */
>> +endef
>> +export __note
>> +
>> +.PHONY: all clean
>> +all: x86-a-b-bootblock.h
>> +
>> +x86-a-b-bootblock.h: x86.bootsect
>> +	echo "$$__note" > header.tmp
>> +	xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
>> +	mv header.tmp $@
>
> The shell script this Makefile is replacing used mktemp
> for a randomly named tmp dir. Shouldn't we continue to
> use random names?

We shouldn't be trying to create the file twice in parallel, no?

>
>> +
>> +x86.bootsect: x86.boot
>> +	dd if=$< of=$@ bs=256 count=2 skip=124
>> +
>> +x86.boot: x86.o
>> +	$(CROSS_PREFIX)objcopy -O binary $< $@
>> +
>> +x86.o: x86-a-b-bootblock.S
>> +	$(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
>> +
>> +clean:
>> +	@rm -rf *.boot *.o *.bootsect
>
> We don't want to remove the generated header file when cleaning?

No, because we only generated the header file when running native, have
the right crosscompilers, etc.  In general we only recompile it when we
change the test file, otherwise we store the generated ones.

Idea is:
We have a c file that does what we want.
We compile it (when we have a propper compiler) and store it on the
tree.
We compile it for each test.

This is more "like firmware" than normal "qemu" source code.

Later, Juan.

Re: [Qemu-devel] [PATCH V8 4/4] tests: Add migration test for aarch64

[Andrew Jones]

On Sat, Sep 01, 2018 at 11:07:26AM +0100, Peter Maydell wrote:
> On 1 September 2018 at 06:11, Wei Huang <wei@redhat.com> 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 source
> > code and the build script in tests/migration/aarch64. So users can change
> > the source and/or re-compile the binary as they wish.
> > +    } else if (strcmp(arch, "aarch64") == 0) {
> > +        struct utsname utsname;
> > +
> 
> The comment says:
> 
> > +        /* kvm and tcg need different cpu and gic-version configs */
> 
> ...but the code below it doesn't change the cpu or gic-version
> based on 'accel'.
> 
> > +        if (access("/dev/kvm", F_OK) == 0 && uname(&utsname) == 0 &&
> > +            strcmp(utsname.machine, "aarch64") == 0) {
> > +            accel = "kvm";
> > +        } else {
> > +            accel = "tcg";
> > +        }
> 
> Maybe we could just use -accel=kvm,tcg rather than guessing
> whether kvm is going to work ? If this is how the x86 code
> works then probably best to do the same as that, though.

Yeah, using '-accel=kvm:tcg' is what I suggested in the v7 review when
suggesting to switch to -cpu max and gic-version=max. That allows the
uname() use and the new utsname.h include to go away.

Thanks,
drew

> 
> > +
> > +        init_bootfile(bootpath, aarch64_kernel);
> > +        cmd_src = g_strdup_printf("-machine virt,accel=%s,gic-version=max "
> > +                                  "-name vmsource,debug-threads=on -cpu max "
> > +                                  "-m 150M -serial file:%s/src_serial "
> > +                                  "-kernel %s ",
> > +                                  accel, tmpfs, bootpath);
> > +        cmd_dst = g_strdup_printf("-machine virt,accel=%s,gic-version=max "
> > +                                  "-name vmdest,debug-threads=on -cpu max "
> > +                                  "-m 150M -serial file:%s/dest_serial "
> > +                                  "-kernel %s "
> > +                                  "-incoming %s ",
> > +                                  accel, tmpfs, bootpath, uri);
> > +
> > +        start_address = ARM_TEST_MEM_START;
> > +        end_address = ARM_TEST_MEM_END;
> > +
> > +        g_assert(sizeof(aarch64_kernel) <= ARM_TEST_MAX_KERNEL_SIZE);
> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [PATCH V8 4/4] tests: Add migration test for aarch64

[Andrew Jones]

On Sat, Sep 01, 2018 at 01:11:15AM -0400, Wei Huang wrote:
> +.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
> +
> +        /* traverse test memory region */
> +        mov     x0, #ARM_TEST_MEM_START
> +        mov     x1, #ARM_TEST_MEM_END

I don't think there's any reason to require the start and
end addresses to be hard coded. We should be able to get
the start address with

#define KERNEL_OFFSET 0x80000
           adr     x0, _start
           add     x0, x0, #(1024 * 1024 - KERNEL_OFFSET)

and the end address with

           add     x1, x0, #(99 * 1024 * 1024)

Thanks,
drew

Re: [Qemu-devel] [PATCH V8 1/4] tests/migration: Convert x86 boot block compilation script into Makefile

[Andrew Jones]

On Mon, Sep 03, 2018 at 01:45:33PM +0200, Juan Quintela wrote:
> Andrew Jones <drjones@redhat.com> wrote:
> > On Sat, Sep 01, 2018 at 01:11:12AM -0400, Wei Huang wrote:
> >> The x86 boot block header currently is generated with a shell script.
> >> To better support other CPUs (e.g. aarch64), we convert the script
> >> into Makefile. This allows us to 1) support cross-compilation easily,
> >> and 2) avoid creating a script file for every architecture.
> >> 
> >> Note that, in the new design, the cross compiler prefix can be specified by
> >> setting the CROSS_PREFIX in "make" command. Also to allow gcc pre-processor
> >> to include the C-style file correctly, it also renames the
> >> x86-a-b-bootblock.s file extension from .s to .S.
> >> + * tests/migration/rebuild-x86-bootblock.sh to update,
> >> + * and then remember to send both in your patch submission.
> >> + */
> >> +endef
> >> +export __note
> >> +
> >> +.PHONY: all clean
> >> +all: x86-a-b-bootblock.h
> >> +
> >> +x86-a-b-bootblock.h: x86.bootsect
> >> +	echo "$$__note" > header.tmp
> >> +	xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
> >> +	mv header.tmp $@
> >
> > The shell script this Makefile is replacing used mktemp
> > for a randomly named tmp dir. Shouldn't we continue to
> > use random names?
> 
> We shouldn't be trying to create the file twice in parallel, no?

Yeah, you're right. This should be safe.

> 
> >
> >> +
> >> +x86.bootsect: x86.boot
> >> +	dd if=$< of=$@ bs=256 count=2 skip=124
> >> +
> >> +x86.boot: x86.o
> >> +	$(CROSS_PREFIX)objcopy -O binary $< $@
> >> +
> >> +x86.o: x86-a-b-bootblock.S
> >> +	$(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
> >> +
> >> +clean:
> >> +	@rm -rf *.boot *.o *.bootsect
> >
> > We don't want to remove the generated header file when cleaning?
> 
> No, because we only generated the header file when running native, have
> the right crosscompilers, etc.  In general we only recompile it when we
> change the test file, otherwise we store the generated ones.
> 
> Idea is:
> We have a c file that does what we want.
> We compile it (when we have a propper compiler) and store it on the
> tree.
> We compile it for each test.
> 
> This is more "like firmware" than normal "qemu" source code.
>

Right. I momentarily forget that when replying to this patch.

Thanks,
drew

Re: [Qemu-devel] [PATCH V8 1/4] tests/migration: Convert x86 boot block compilation script into Makefile

[Wei Huang]

On 09/03/2018 06:08 AM, Andrew Jones wrote:
> On Sat, Sep 01, 2018 at 01:11:12AM -0400, Wei Huang wrote:
>> The x86 boot block header currently is generated with a shell script.
>> To better support other CPUs (e.g. aarch64), we convert the script
>> into Makefile. This allows us to 1) support cross-compilation easily,
>> and 2) avoid creating a script file for every architecture.
>>
>> Note that, in the new design, the cross compiler prefix can be specified by
>> setting the CROSS_PREFIX in "make" command. Also to allow gcc pre-processor
>> to include the C-style file correctly, it also renames the
>> x86-a-b-bootblock.s file extension from .s to .S.
>>
>> Signed-off-by: Wei Huang <wei@redhat.com>
>> ---
>>  tests/migration/Makefile                           | 31 ++++++++++++++++++++
>>  tests/migration/rebuild-x86-bootblock.sh           | 33 ----------------------
>>  .../{x86-a-b-bootblock.s => x86-a-b-bootblock.S}   |  0
>>  3 files changed, 31 insertions(+), 33 deletions(-)
>>  create mode 100644 tests/migration/Makefile
>>  delete mode 100755 tests/migration/rebuild-x86-bootblock.sh
>>  rename tests/migration/{x86-a-b-bootblock.s => x86-a-b-bootblock.S} (100%)
>>
>> diff --git a/tests/migration/Makefile b/tests/migration/Makefile
>> new file mode 100644
>> index 0000000..5d5fa07
>> --- /dev/null
>> +++ b/tests/migration/Makefile
>> @@ -0,0 +1,31 @@
>> +# To specify cross compiler prefix, use CROSS_PREFIX=
>> +#   > make CROSS_PREFIX=x86_64-linux-gnu-
>        ^ nit: this prompt symbol is weird (at least to me, what shell uses
>               this?) To me it looks like a sh/bash redirect symbol. Can
>               we change it to '$' or use nothing at all?

I will replace '>' with '$', which is indeed the commonly-used one.

>> +
>> +override define __note
>> +/* This file is automatically generated from
>> + * tests/migration/x86-a-b-bootblock.s, edit that and then run
>                                         ^ should be 'S'

OK, will do.

> 
>> + * tests/migration/rebuild-x86-bootblock.sh to update,
>> + * and then remember to send both in your patch submission.
>> + */
>> +endef
>> +export __note
>> +
>> +.PHONY: all clean
>> +all: x86-a-b-bootblock.h
>> +
>> +x86-a-b-bootblock.h: x86.bootsect
>> +	echo "$$__note" > header.tmp
>> +	xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
>> +	mv header.tmp $@
> 
> The shell script this Makefile is replacing used mktemp
> for a randomly named tmp dir. Shouldn't we continue to
> use random names?>
>> +
>> +x86.bootsect: x86.boot
>> +	dd if=$< of=$@ bs=256 count=2 skip=124
>> +
>> +x86.boot: x86.o
>> +	$(CROSS_PREFIX)objcopy -O binary $< $@
>> +
>> +x86.o: x86-a-b-bootblock.S
>> +	$(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
>> +
>> +clean:
>> +	@rm -rf *.boot *.o *.bootsect
> 
> We don't want to remove the generated header file when cleaning?
> 
>> diff --git a/tests/migration/rebuild-x86-bootblock.sh b/tests/migration/rebuild-x86-bootblock.sh
>> deleted file mode 100755
>> index 86cec5d..0000000
>> --- a/tests/migration/rebuild-x86-bootblock.sh
>> +++ /dev/null
>> @@ -1,33 +0,0 @@
>> -#!/bin/sh
>> -# Copyright (c) 2016-2018 Red Hat, Inc. and/or its affiliates
>> -# This work is licensed under the terms of the GNU GPL, version 2 or later.
>> -# See the COPYING file in the top-level directory.
>> -#
>> -# Author: dgilbert@redhat.com
>> -
>> -ASMFILE=$PWD/tests/migration/x86-a-b-bootblock.s
>> -HEADER=$PWD/tests/migration/x86-a-b-bootblock.h
>> -
>> -if [ ! -e "$ASMFILE" ]
>> -then
>> -  echo "Couldn't find $ASMFILE" >&2
>> -  exit 1
>> -fi
>> -
>> -ASM_WORK_DIR=$(mktemp -d --tmpdir X86BB.XXXXXX)
>> -cd "$ASM_WORK_DIR" &&
>> -as --32 -march=i486 "$ASMFILE" -o x86.o &&
>> -objcopy -O binary x86.o x86.boot &&
>> -dd if=x86.boot of=x86.bootsect bs=256 count=2 skip=124 &&
>> -xxd -i x86.bootsect |
>> -sed -e 's/.*int.*//' > x86.hex &&
>> -cat - x86.hex <<HERE > "$HEADER"
>> -/* This file is automatically generated from
>> - * tests/migration/x86-a-b-bootblock.s, edit that and then run
>> - * tests/migration/rebuild-x86-bootblock.sh to update,
>> - * and then remember to send both in your patch submission.
>> - */
>> -HERE
>> -
>> -rm x86.hex x86.bootsect x86.boot x86.o
>> -cd .. && rmdir "$ASM_WORK_DIR"
>> diff --git a/tests/migration/x86-a-b-bootblock.s b/tests/migration/x86-a-b-bootblock.S
>> similarity index 100%
>> rename from tests/migration/x86-a-b-bootblock.s
>> rename to tests/migration/x86-a-b-bootblock.S
>> -- 
>> 1.8.3.1
>>
>>
> 
> Thanks,
> drew 
> 

Re: [Qemu-devel] [PATCH V8 2/4] tests/migration: Support cross compilation in generating boot header file

[Wei Huang]

----- Original Message -----
> From: "Andrew Jones" <drjones@redhat.com>
> To: "Wei Huang" <wei@redhat.com>
> Cc: qemu-devel@nongnu.org, lvivier@redhat.com, "peter maydell" <peter.maydell@linaro.org>, quintela@redhat.com,
> dgilbert@redhat.com
> Sent: Monday, September 3, 2018 6:26:39 AM
> Subject: Re: [Qemu-devel] [PATCH V8 2/4] tests/migration: Support cross compilation in generating boot header file
> 
> On Sat, Sep 01, 2018 at 01:11:13AM -0400, Wei Huang wrote:
> > Recently a new configure option, CROSS_CC_GUEST, was added to
> > $(TARGET)-softmmu/config-target.mak to support TCG-related tests. This
> > patch tries to leverage this option to support cross compilation when the
> > migration boot block file is being re-generated:
> > 
> >  * The x86_64 related files are moved to a new sub-dir (named ./x86_64).
> >  * A new top-layer Makefile is created in tests/migration/ directory.
> >    This Makefile searches and parses CROSS_CC_GUEST to generate
> >    CROSS_PREFIX.
> >    The CROSS_PREFIX, if available, is then passed to
> >    migration/$ARCH/Makefile.
> > 
> > Signed-off-by: Wei Huang <wei@redhat.com>
> > ---
> >  tests/migration-test.c                           |  2 +-
> >  tests/migration/Makefile                         | 43
> >  +++++++++++++-----------
> >  tests/migration/x86_64/Makefile                  | 22 ++++++++++++
> >  tests/migration/{ => x86_64}/x86-a-b-bootblock.S |  0
> >  tests/migration/{ => x86_64}/x86-a-b-bootblock.h |  8 ++---
> >  5 files changed, 50 insertions(+), 25 deletions(-)
> >  create mode 100644 tests/migration/x86_64/Makefile
> >  rename tests/migration/{ => x86_64}/x86-a-b-bootblock.S (100%)
> >  rename tests/migration/{ => x86_64}/x86-a-b-bootblock.h (93%)
> > 
> > diff --git a/tests/migration-test.c b/tests/migration-test.c
> > index 0e687b7..c4d79e9 100644
> > --- a/tests/migration-test.c
> > +++ b/tests/migration-test.c
> > @@ -83,7 +83,7 @@ static const char *tmpfs;
> >  /* A simple PC boot sector that modifies memory (1-100MB) quickly
> >   * outputting a 'B' every so often if it's still running.
> >   */
> > -#include "tests/migration/x86-a-b-bootblock.h"
> > +#include "tests/migration/x86_64/x86-a-b-bootblock.h"
> 
> Is x86-a-b-bootblock.h specific to x86_64, or both i386 and x86_64?
> I think we want the dir name to be i386 if it's the later. Also,
> we could drop the 'x86-' prefix from the filename, now that the
> directory identifies the arch.

Well, I can try it. But this will make the Makefile more complicated as we have to detect x86_64 compiler for i386 binary (most distros only ship x86_64 cross compiler).

> 
> >  
> >  static void init_bootfile_x86(const char *bootpath)
> >  {
> > diff --git a/tests/migration/Makefile b/tests/migration/Makefile
> > index 5d5fa07..1de480e 100644
> > --- a/tests/migration/Makefile
> > +++ b/tests/migration/Makefile
> > @@ -1,31 +1,34 @@
> > -# To specify cross compiler prefix, use CROSS_PREFIX=
> > -#   > make CROSS_PREFIX=x86_64-linux-gnu-
> > +#
> > +# Copyright (c) 2018 Red Hat, Inc. and/or its affiliates
> > +#
> > +# This work is licensed under the terms of the GNU GPL, version 2 or
> > later.
> > +# See the COPYING file in the top-level directory.
> > +#
> > +
> > +TARGET_LIST = x86_64
> > +
> > +SRC_PATH = ../..
> >  
> >  override define __note
> > -/* This file is automatically generated from
> > - * tests/migration/x86-a-b-bootblock.s, edit that and then run
> > - * tests/migration/rebuild-x86-bootblock.sh to update,
> > - * and then remember to send both in your patch submission.
> > +/* This file is automatically generated from the assembly file in
> > + * tests/migration/$@, edit that and then run "make all"
> 
> nit: /, edit that/. Edit that file/
> 
> > + * inside tests/migration to update, and then remember to send both
> > + * in your patch submission.
> nit: ^ the header and the assembler differences
> 
> >   */
> >  endef
> >  export __note
> >  
> > -.PHONY: all clean
> > -all: x86-a-b-bootblock.h
> > -
> > -x86-a-b-bootblock.h: x86.bootsect
> > -	echo "$$__note" > header.tmp
> > -	xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
> > -	mv header.tmp $@
> > +parse-cross-prefix = $(subst gcc,,$(patsubst cc,gcc,$(patsubst
> > CROSS_CC_GUEST="%",%,$(shell grep "CROSS_CC_GUEST="
> > $(SRC_PATH)/$(1)-softmmu/config-target.mak))))
> > +gen-cross-prefix = $(patsubst %-,CROSS_PREFIX=%-,$(call
> > parse-cross-prefix,$(1)))
> >  
> > -x86.bootsect: x86.boot
> > -	dd if=$< of=$@ bs=256 count=2 skip=124
> > +.PHONY: all $(TARGET_LIST)
> >  
> > -x86.boot: x86.o
> > -	$(CROSS_PREFIX)objcopy -O binary $< $@
> > +all: $(TARGET_LIST)
> >  
> > -x86.o: x86-a-b-bootblock.S
> > -	$(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
> > +$(TARGET_LIST):
> > +	$(MAKE) -C $@ $(call gen-cross-prefix,$@)
> >  
> >  clean:
> > -	@rm -rf *.boot *.o *.bootsect
> > +	for target in $(TARGET_LIST); do \
> > +		$(MAKE) -C $$target clean; \
> > +	done
> 
> The above seems overly complicated. Does the following work?
> 
> $(TARGET_LIST): CROSS_PREFIX=$(CROSS_CC_GUEST)
> $(TARGET_LIST):
> 	$(MAKE) -C $@

No, it doen't work for different reasons.

First, to have CROSS_CC_GUEST defined, we normally would use "-include" to include config-target.mak file. But the end result is, the CROSS_CC_GUEST only has the same definition for both architectures (due to make command's multi-iteration processing and include is processed first). Secondly, CROSS_CC_GUEST needs to be polished because it can be: i) not defined; ii) "cc" without prefix; or iii) used-define value. We can't use it directly.



> 
> 
> > diff --git a/tests/migration/x86_64/Makefile
> > b/tests/migration/x86_64/Makefile
> > new file mode 100644
> > index 0000000..6dc849f
> > --- /dev/null
> > +++ b/tests/migration/x86_64/Makefile
> > @@ -0,0 +1,22 @@
> > +# To specify cross compiler prefix, use CROSS_PREFIX=
> > +#   > make CROSS_PREFIX=x86_64-linux-gnu-
> > +
> > +.PHONY: all clean
> > +all: x86-a-b-bootblock.h
> > +
> > +x86-a-b-bootblock.h: x86.bootsect
> > +	echo "$$__note" > header.tmp
> > +	xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
> > +	mv header.tmp $@
> > +
> > +x86.bootsect: x86.boot
> > +	dd if=$< of=$@ bs=256 count=2 skip=124
> > +
> > +x86.boot: x86.o
> > +	$(CROSS_PREFIX)objcopy -O binary $< $@
> > +
> > +x86.o: x86-a-b-bootblock.S
> > +	$(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
> > +
> > +clean:
> > +	@rm -rf *.boot *.o *.bootsect
> > diff --git a/tests/migration/x86-a-b-bootblock.S
> > b/tests/migration/x86_64/x86-a-b-bootblock.S
> > similarity index 100%
> > rename from tests/migration/x86-a-b-bootblock.S
> > rename to tests/migration/x86_64/x86-a-b-bootblock.S
> > diff --git a/tests/migration/x86-a-b-bootblock.h
> > b/tests/migration/x86_64/x86-a-b-bootblock.h
> > similarity index 93%
> > rename from tests/migration/x86-a-b-bootblock.h
> > rename to tests/migration/x86_64/x86-a-b-bootblock.h
> > index 78a151f..73670ef 100644
> > --- a/tests/migration/x86-a-b-bootblock.h
> > +++ b/tests/migration/x86_64/x86-a-b-bootblock.h
> > @@ -1,7 +1,7 @@
> > -/* This file is automatically generated from
> > - * tests/migration/x86-a-b-bootblock.s, edit that and then run
> > - * tests/migration/rebuild-x86-bootblock.sh to update,
> > - * and then remember to send both in your patch submission.
> > +/* This file is automatically generated from the assembly file in
> > + * tests/migration/x86_64, edit that and then run "make all"
> > + * inside tests/migration to update, and then remember to send both
> > + * in your patch submission.
> >   */
> >  unsigned char x86_bootsect[] = {
> >    0xfa, 0x0f, 0x01, 0x16, 0x74, 0x7c, 0x66, 0xb8, 0x01, 0x00, 0x00, 0x00,
> > --
> > 1.8.3.1
> > 
> >
> 
> Thanks,
> drew
> 

Re: [Qemu-devel] [PATCH V8 4/4] tests: Add migration test for aarch64

[Wei Huang]

----- Original Message -----
> From: "Andrew Jones" <drjones@redhat.com>
> To: "Wei Huang" <wei@redhat.com>
> Cc: qemu-devel@nongnu.org, lvivier@redhat.com, "peter maydell" <peter.maydell@linaro.org>, quintela@redhat.com,
> dgilbert@redhat.com
> Sent: Monday, September 3, 2018 6:53:35 AM
> Subject: Re: [Qemu-devel] [PATCH V8 4/4] tests: Add migration test for aarch64
> 
> On Sat, Sep 01, 2018 at 01:11:15AM -0400, Wei Huang wrote:
> > +.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
> > +
> > +        /* traverse test memory region */
> > +        mov     x0, #ARM_TEST_MEM_START
> > +        mov     x1, #ARM_TEST_MEM_END
> 
> I don't think there's any reason to require the start and
> end addresses to be hard coded. We should be able to get
> the start address with

Are you saying that we should make the .S file as independent as possible? These variables need to be defined for migration-test.c anyway. Why can't we just use them here?

> 
> #define KERNEL_OFFSET 0x80000
>            adr     x0, _start
>            add     x0, x0, #(1024 * 1024 - KERNEL_OFFSET)
> 
> and the end address with
> 
>            add     x1, x0, #(99 * 1024 * 1024)
> 
> Thanks,
> drew
> 

Re: [Qemu-devel] [PATCH V8 4/4] tests: Add migration test for aarch64

[Andrew Jones]

On Tue, Sep 04, 2018 at 01:07:56PM -0400, Wei Huang wrote:
> 
> 
> ----- Original Message -----
> > From: "Andrew Jones" <drjones@redhat.com>
> > To: "Wei Huang" <wei@redhat.com>
> > Cc: qemu-devel@nongnu.org, lvivier@redhat.com, "peter maydell" <peter.maydell@linaro.org>, quintela@redhat.com,
> > dgilbert@redhat.com
> > Sent: Monday, September 3, 2018 6:53:35 AM
> > Subject: Re: [Qemu-devel] [PATCH V8 4/4] tests: Add migration test for aarch64
> > 
> > On Sat, Sep 01, 2018 at 01:11:15AM -0400, Wei Huang wrote:
> > > +.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
> > > +
> > > +        /* traverse test memory region */
> > > +        mov     x0, #ARM_TEST_MEM_START
> > > +        mov     x1, #ARM_TEST_MEM_END
> > 
> > I don't think there's any reason to require the start and
> > end addresses to be hard coded. We should be able to get
> > the start address with
> 
> Are you saying that we should make the .S file as independent as possible? These variables need to be defined for migration-test.c anyway. Why can't we just use them here?

If we ever change the base address of mach-virt, or want to reuse this
code for another AArch64 machine model that has a different base address,
then it would just work, when we don't hard code discoverable variables.
But, you're right, migration-test.c, would need to change in some way
to be reusable, or to handle different base addresses anyway, so I
guess I'm OK with just hard coding them here too.

Thanks,
drew

> 
> > 
> > #define KERNEL_OFFSET 0x80000
> >            adr     x0, _start
> >            add     x0, x0, #(1024 * 1024 - KERNEL_OFFSET)
> > 
> > and the end address with
> > 
> >            add     x1, x0, #(99 * 1024 * 1024)
> > 
> > Thanks,
> > drew
> > 

Re: [Qemu-devel] [PATCH V8 2/4] tests/migration: Support cross compilation in generating boot header file

[Andrew Jones]

On Tue, Sep 04, 2018 at 01:04:31PM -0400, Wei Huang wrote:
> > > +parse-cross-prefix = $(subst gcc,,$(patsubst cc,gcc,$(patsubst
> > > CROSS_CC_GUEST="%",%,$(shell grep "CROSS_CC_GUEST="
> > > $(SRC_PATH)/$(1)-softmmu/config-target.mak))))
> > > +gen-cross-prefix = $(patsubst %-,CROSS_PREFIX=%-,$(call
> > > parse-cross-prefix,$(1)))
> > >  
> > > -x86.bootsect: x86.boot
> > > -	dd if=$< of=$@ bs=256 count=2 skip=124
> > > +.PHONY: all $(TARGET_LIST)
> > >  
> > > -x86.boot: x86.o
> > > -	$(CROSS_PREFIX)objcopy -O binary $< $@
> > > +all: $(TARGET_LIST)
> > >  
> > > -x86.o: x86-a-b-bootblock.S
> > > -	$(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
> > > +$(TARGET_LIST):
> > > +	$(MAKE) -C $@ $(call gen-cross-prefix,$@)
> > >  
> > >  clean:
> > > -	@rm -rf *.boot *.o *.bootsect
> > > +	for target in $(TARGET_LIST); do \
> > > +		$(MAKE) -C $$target clean; \
> > > +	done
> > 
> > The above seems overly complicated. Does the following work?
> > 
> > $(TARGET_LIST): CROSS_PREFIX=$(CROSS_CC_GUEST)
> > $(TARGET_LIST):
> > 	$(MAKE) -C $@
> 
> No, it doen't work for different reasons.
> 
> First, to have CROSS_CC_GUEST defined, we normally would use "-include" to include config-target.mak file. But the end result is, the CROSS_CC_GUEST only has the same definition for both architectures (due to make command's multi-iteration processing and include is processed first). Secondly, CROSS_CC_GUEST needs to be polished because it can be: i) not defined; ii) "cc" without prefix; or iii) used-define value. We can't use it directly.
>

OK. Since I don't have any better suggestions. Then this is good
enough for me.

Thanks,
drew