qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH V5 0/4] tests: Add migration test for aarch64
@ 2018-02-23 21:58 Wei Huang
  2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 1/4] rules: Move cross compilation auto detection functions to rules.mak Wei Huang
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Wei Huang @ 2018-02-23 21:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: dgilbert, peter.maydell, quintela, drjones

This patchset adds a migration test for aarch64. It leverages
Dave Gilbert's recent patch "tests/migration: Add source to PC boot block"
to create a new test case for aarch64.

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):
  rules: Move cross compilation auto detection functions to rules.mak
  tests/migration: Convert the boot block compilation script into
    Makefile
  tests/migration: Add migration-test header file
  tests: Add migration test for aarch64

 roms/Makefile                                      | 24 ++-----
 rules.mak                                          | 15 +++++
 tests/Makefile.include                             |  1 +
 tests/migration-test.c                             | 74 ++++++++++++++++------
 tests/migration/Makefile                           | 44 +++++++++++++
 tests/migration/aarch64-a-b-kernel.S               | 71 +++++++++++++++++++++
 tests/migration/aarch64-a-b-kernel.h               | 19 ++++++
 tests/migration/migration-test.h                   | 23 +++++++
 tests/migration/rebuild-x86-bootblock.sh           | 33 ----------
 .../{x86-a-b-bootblock.s => x86-a-b-bootblock.S}   | 12 ++--
 tests/migration/x86-a-b-bootblock.h                |  4 +-
 11 files changed, 244 insertions(+), 76 deletions(-)
 create mode 100644 tests/migration/Makefile
 create mode 100644 tests/migration/aarch64-a-b-kernel.S
 create mode 100644 tests/migration/aarch64-a-b-kernel.h
 create mode 100644 tests/migration/migration-test.h
 delete mode 100755 tests/migration/rebuild-x86-bootblock.sh
 rename tests/migration/{x86-a-b-bootblock.s => x86-a-b-bootblock.S} (88%)

-- 
2.14.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH V5 1/4] rules: Move cross compilation auto detection functions to rules.mak
  2018-02-23 21:58 [Qemu-devel] [PATCH V5 0/4] tests: Add migration test for aarch64 Wei Huang
@ 2018-02-23 21:58 ` Wei Huang
  2018-02-26  9:07   ` Andrew Jones
  2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 2/4] tests/migration: Convert the boot block compilation script into Makefile Wei Huang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Wei Huang @ 2018-02-23 21:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: dgilbert, peter.maydell, quintela, drjones

This patch moves the auto detection functions for cross compilation from
roms/Makefile to rules.mak. So the functions can be shared among Makefiles
in QEMU.

Signed-off-by: Wei Huang <wei@redhat.com>
---
 roms/Makefile | 24 +++++++-----------------
 rules.mak     | 15 +++++++++++++++
 2 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/roms/Makefile b/roms/Makefile
index b5e5a69e91..e972c65333 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -21,23 +21,6 @@ pxe-rom-virtio   efi-rom-virtio   : DID := 1000
 pxe-rom-vmxnet3  efi-rom-vmxnet3  : VID := 15ad
 pxe-rom-vmxnet3  efi-rom-vmxnet3  : DID := 07b0
 
-#
-# cross compiler auto detection
-#
-path := $(subst :, ,$(PATH))
-system := $(shell uname -s | tr "A-Z" "a-z")
-
-# first find cross binutils in path
-find-cross-ld = $(firstword $(wildcard $(patsubst %,%/$(1)-*$(system)*-ld,$(path))))
-# then check we have cross gcc too
-find-cross-gcc = $(firstword $(wildcard $(patsubst %ld,%gcc,$(call find-cross-ld,$(1)))))
-# finally strip off path + toolname so we get the prefix
-find-cross-prefix = $(subst gcc,,$(notdir $(call find-cross-gcc,$(1))))
-
-powerpc64_cross_prefix := $(call find-cross-prefix,powerpc64)
-powerpc_cross_prefix := $(call find-cross-prefix,powerpc)
-x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
-
 # tag our seabios builds
 SEABIOS_EXTRAVERSION="-prebuilt.qemu-project.org"
 
@@ -66,6 +49,13 @@ default:
 	@echo "  skiboot        -- update skiboot.lid"
 	@echo "  u-boot.e500    -- update u-boot.e500"
 
+SRC_PATH=..
+include $(SRC_PATH)/rules.mak
+
+powerpc64_cross_prefix := $(call find-cross-prefix,powerpc64)
+powerpc_cross_prefix := $(call find-cross-prefix,powerpc)
+x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
+
 bios: build-seabios-config-seabios-128k build-seabios-config-seabios-256k
 	cp seabios/builds/seabios-128k/bios.bin ../pc-bios/bios.bin
 	cp seabios/builds/seabios-256k/bios.bin ../pc-bios/bios-256k.bin
diff --git a/rules.mak b/rules.mak
index 6e943335f3..ef8adee3f8 100644
--- a/rules.mak
+++ b/rules.mak
@@ -62,6 +62,21 @@ expand-objs = $(strip $(sort $(filter %.o,$1)) \
                   $(foreach o,$(filter %.mo,$1),$($o-objs)) \
                   $(filter-out %.o %.mo,$1))
 
+# Cross compilation auto detection. Use find-cross-prefix to detect the
+# target archtecture's prefix, and then append it to the build tool or pass
+# it to CROSS_COMPILE directly. Here is one example:
+#      x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
+#      $(x86_64_cross_prefix)gcc -c test.c -o test.o
+#      make -C testdir CROSS_COMPILE=$(x86_64_cross_prefix)
+cross-search-path := $(subst :, ,$(PATH))
+cross-host-system := $(shell uname -s | tr "A-Z" "a-z")
+
+find-cross-ld = $(firstword $(wildcard $(patsubst \
+                    %,%/$(1)-*$(cross-host-system)*-ld,$(cross-search-path))))
+find-cross-gcc = $(firstword $(wildcard \
+                    $(patsubst %ld,%gcc,$(call find-cross-ld,$(1)))))
+find-cross-prefix = $(subst gcc,,$(notdir $(call find-cross-gcc,$(1))))
+
 %.o: %.c
 	$(call quiet-command,$(CC) $(QEMU_LOCAL_INCLUDES) $(QEMU_INCLUDES) \
 	       $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) \
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH V5 2/4] tests/migration: Convert the boot block compilation script into Makefile
  2018-02-23 21:58 [Qemu-devel] [PATCH V5 0/4] tests: Add migration test for aarch64 Wei Huang
  2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 1/4] rules: Move cross compilation auto detection functions to rules.mak Wei Huang
@ 2018-02-23 21:58 ` Wei Huang
  2018-02-26  9:07   ` Andrew Jones
  2018-02-26 18:01   ` Dr. David Alan Gilbert
  2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 3/4] tests/migration: Add migration-test header file Wei Huang
  2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 4/4] tests: Add migration test for aarch64 Wei Huang
  3 siblings, 2 replies; 16+ messages in thread
From: Wei Huang @ 2018-02-23 21:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: dgilbert, peter.maydell, quintela, drjones

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.

Signed-off-by: Wei Huang <wei@redhat.com>
---
 tests/migration/Makefile                 | 36 ++++++++++++++++++++++++++++++++
 tests/migration/rebuild-x86-bootblock.sh | 33 -----------------------------
 tests/migration/x86-a-b-bootblock.h      |  2 +-
 tests/migration/x86-a-b-bootblock.s      |  5 ++---
 4 files changed, 39 insertions(+), 37 deletions(-)
 create mode 100644 tests/migration/Makefile
 delete mode 100755 tests/migration/rebuild-x86-bootblock.sh

diff --git a/tests/migration/Makefile b/tests/migration/Makefile
new file mode 100644
index 0000000000..8fbedaa8b8
--- /dev/null
+++ b/tests/migration/Makefile
@@ -0,0 +1,36 @@
+#
+# Copyright (c) 2016-2018 Red Hat, Inc. and/or its affiliates
+#
+# Authors:
+#   Dave Gilbert <dgilbert@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.
+#
+export __note
+override define __note
+/* This file is automatically generated from
+ * tests/migration/$<, edit that and then run
+ * "make $@" inside tests/migration to update,
+ * and then remember to send both in your patch submission.
+ */
+endef
+
+all: x86-a-b-bootblock.h
+# Dummy command so that make thinks it has done something
+	@true
+
+SRC_PATH=../..
+include $(SRC_PATH)/rules.mak
+
+x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
+
+x86-a-b-bootblock.h: x86-a-b-bootblock.s
+	$(x86_64_cross_prefix)as --32 -march=i486 $< -o x86.o
+	$(x86_64_cross_prefix)objcopy -O binary x86.o x86.boot
+	dd if=x86.boot of=x86.bootsect bs=256 count=2 skip=124
+	echo "$$__note" > $@
+	xxd -i x86.bootsect | sed -e 's/.*int.*//' >> $@
+
+clean:
+	rm -f *.bootsect *.boot *.o
diff --git a/tests/migration/rebuild-x86-bootblock.sh b/tests/migration/rebuild-x86-bootblock.sh
deleted file mode 100755
index 86cec5d284..0000000000
--- 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.h b/tests/migration/x86-a-b-bootblock.h
index 78a151fe2a..9e8e2e028b 100644
--- a/tests/migration/x86-a-b-bootblock.h
+++ b/tests/migration/x86-a-b-bootblock.h
@@ -1,6 +1,6 @@
 /* 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,
+ * "make x86-a-b-bootblock.h" inside tests/migration to update,
  * and then remember to send both in your patch submission.
  */
 unsigned char x86_bootsect[] = {
diff --git a/tests/migration/x86-a-b-bootblock.s b/tests/migration/x86-a-b-bootblock.s
index b1642641a7..98dbfab084 100644
--- a/tests/migration/x86-a-b-bootblock.s
+++ b/tests/migration/x86-a-b-bootblock.s
@@ -3,9 +3,8 @@
 #  range.
 #  Outputs an initial 'A' on serial followed by repeated 'B's
 #
-# run   tests/migration/rebuild-x86-bootblock.sh
-#   to regenerate the hex, and remember to include both the .h and .s
-#   in any patches.
+#  In tests/migration dir, run 'make x86-a-b-bootblock.h' to regenerate
+#  the hex, and remember to include both the .h and .s in any patches.
 #
 # Copyright (c) 2016 Red Hat, Inc. and/or its affiliates
 # This work is licensed under the terms of the GNU GPL, version 2 or later.
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH V5 3/4] tests/migration: Add migration-test header file
  2018-02-23 21:58 [Qemu-devel] [PATCH V5 0/4] tests: Add migration test for aarch64 Wei Huang
  2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 1/4] rules: Move cross compilation auto detection functions to rules.mak Wei Huang
  2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 2/4] tests/migration: Convert the boot block compilation script into Makefile Wei Huang
@ 2018-02-23 21:58 ` Wei Huang
  2018-02-26  9:05   ` Andrew Jones
  2018-02-26  9:38   ` Andrew Jones
  2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 4/4] tests: Add migration test for aarch64 Wei Huang
  3 siblings, 2 replies; 16+ messages in thread
From: Wei Huang @ 2018-02-23 21:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: dgilbert, peter.maydell, quintela, drjones

This patch moves the settings related migration-test from the
migration-test.c file to a seperate header file. It also renames the
x86-a-b-bootblock.s file extension from .s to .S, allowing gcc
pre-processor to include the C-style header file correctly.

Signed-off-by: Wei Huang <wei@redhat.com>
---
 tests/migration-test.c                             | 28 +++++++++++-----------
 tests/migration/Makefile                           |  4 ++--
 tests/migration/migration-test.h                   | 18 ++++++++++++++
 .../{x86-a-b-bootblock.s => x86-a-b-bootblock.S}   |  7 +++---
 tests/migration/x86-a-b-bootblock.h                |  2 +-
 5 files changed, 39 insertions(+), 20 deletions(-)
 create mode 100644 tests/migration/migration-test.h
 rename tests/migration/{x86-a-b-bootblock.s => x86-a-b-bootblock.S} (94%)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 74f9361bdd..ce2922df6a 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -21,10 +21,10 @@
 #include "sysemu/sysemu.h"
 #include "hw/nvram/chrp_nvram.h"
 
-#define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */
+#include "migration/migration-test.h"
 
-const unsigned start_address = 1024 * 1024;
-const unsigned end_address = 100 * 1024 * 1024;
+const unsigned start_address = TEST_MEM_START;
+const unsigned end_address = TEST_MEM_END;
 bool got_stop;
 
 #if defined(__linux__)
@@ -77,8 +77,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-a-b-bootblock.h"
 
@@ -104,9 +104,8 @@ static void init_bootfile_ppc(const char *bootpath)
     memcpy(header->name, "common", 6);
     chrp_nvram_finish_partition(header, MIN_NVRAM_SIZE);
 
-    /* FW_MAX_SIZE is 4MB, but slof.bin is only 900KB,
-     * so let's modify memory between 1MB and 100MB
-     * to do like PC bootsector
+    /* FW_MAX_SIZE is 4MB, but slof.bin is only 900KB. So it is OK to modify
+     * memory between start_address and end_address like PC bootsector does.
      */
 
     sprintf(buf + 16,
@@ -263,11 +262,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;
@@ -278,7 +277,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);
diff --git a/tests/migration/Makefile b/tests/migration/Makefile
index 8fbedaa8b8..013b8d1f44 100644
--- a/tests/migration/Makefile
+++ b/tests/migration/Makefile
@@ -25,8 +25,8 @@ include $(SRC_PATH)/rules.mak
 
 x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
 
-x86-a-b-bootblock.h: x86-a-b-bootblock.s
-	$(x86_64_cross_prefix)as --32 -march=i486 $< -o x86.o
+x86-a-b-bootblock.h: x86-a-b-bootblock.S
+	$(x86_64_cross_prefix)gcc -m32 -march=i486 -c $< -o x86.o
 	$(x86_64_cross_prefix)objcopy -O binary x86.o x86.boot
 	dd if=x86.boot of=x86.bootsect bs=256 count=2 skip=124
 	echo "$$__note" > $@
diff --git a/tests/migration/migration-test.h b/tests/migration/migration-test.h
new file mode 100644
index 0000000000..48b59b3281
--- /dev/null
+++ b/tests/migration/migration-test.h
@@ -0,0 +1,18 @@
+/*
+ * 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_START      (1 * 1024 * 1024)
+#define TEST_MEM_END        (100 * 1024 * 1024)
+#define TEST_MEM_PAGE_SIZE  4096
+
+/* PPC */
+#define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */
+
+#endif /* _TEST_MIGRATION_H_ */
diff --git a/tests/migration/x86-a-b-bootblock.s b/tests/migration/x86-a-b-bootblock.S
similarity index 94%
rename from tests/migration/x86-a-b-bootblock.s
rename to tests/migration/x86-a-b-bootblock.S
index 98dbfab084..08b51f9e7f 100644
--- a/tests/migration/x86-a-b-bootblock.s
+++ b/tests/migration/x86-a-b-bootblock.S
@@ -12,6 +12,7 @@
 #
 # Author: dgilbert@redhat.com
 
+#include "migration-test.h"
 
 .code16
 .org 0x7c00
@@ -45,11 +46,11 @@ start:             # at 0x7c00 ?
         mov $0, %bl
 mainloop:
         # Start from 1MB
-        mov $(1024*1024),%eax
+        mov $TEST_MEM_START,%eax
 innerloop:
         incb (%eax)
-        add $4096,%eax
-        cmp $(100*1024*1024),%eax
+        add $TEST_MEM_PAGE_SIZE,%eax
+        cmp $TEST_MEM_END,%eax
         jl innerloop
 
         inc %bl
diff --git a/tests/migration/x86-a-b-bootblock.h b/tests/migration/x86-a-b-bootblock.h
index 9e8e2e028b..44e4b99506 100644
--- a/tests/migration/x86-a-b-bootblock.h
+++ b/tests/migration/x86-a-b-bootblock.h
@@ -1,5 +1,5 @@
 /* This file is automatically generated from
- * tests/migration/x86-a-b-bootblock.s, edit that and then run
+ * tests/migration/x86-a-b-bootblock.S, edit that and then run
  * "make x86-a-b-bootblock.h" inside tests/migration to update,
  * and then remember to send both in your patch submission.
  */
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH V5 4/4] tests: Add migration test for aarch64
  2018-02-23 21:58 [Qemu-devel] [PATCH V5 0/4] tests: Add migration test for aarch64 Wei Huang
                   ` (2 preceding siblings ...)
  2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 3/4] tests/migration: Add migration-test header file Wei Huang
@ 2018-02-23 21:58 ` Wei Huang
  2018-02-26  9:30   ` Andrew Jones
  3 siblings, 1 reply; 16+ messages in thread
From: Wei Huang @ 2018-02-23 21:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: dgilbert, peter.maydell, quintela, drjones

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/. 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               | 50 ++++++++++++++++++++++---
 tests/migration/Makefile             | 12 +++++-
 tests/migration/aarch64-a-b-kernel.S | 71 ++++++++++++++++++++++++++++++++++++
 tests/migration/aarch64-a-b-kernel.h | 19 ++++++++++
 tests/migration/migration-test.h     |  5 +++
 6 files changed, 150 insertions(+), 8 deletions(-)
 create mode 100644 tests/migration/aarch64-a-b-kernel.S
 create mode 100644 tests/migration/aarch64-a-b-kernel.h

diff --git a/tests/Makefile.include b/tests/Makefile.include
index a1bcbffe12..df9f64438f 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -372,6 +372,7 @@ check-qtest-arm-y += tests/sdhci-test$(EXESUF)
 check-qtest-aarch64-y = tests/numa-test$(EXESUF)
 check-qtest-aarch64-y += 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 ce2922df6a..d60e34c82d 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -11,6 +11,7 @@
  */
 
 #include "qemu/osdep.h"
+#include <sys/utsname.h>
 
 #include "libqtest.h"
 #include "qapi/qmp/qdict.h"
@@ -23,8 +24,8 @@
 
 #include "migration/migration-test.h"
 
-const unsigned start_address = TEST_MEM_START;
-const unsigned end_address = TEST_MEM_END;
+unsigned start_address = TEST_MEM_START;
+unsigned end_address = TEST_MEM_END;
 bool got_stop;
 
 #if defined(__linux__)
@@ -81,12 +82,13 @@ static const char *tmpfs;
  * repeatedly. It outputs a 'B' at a fixed rate while it's still running.
  */
 #include "tests/migration/x86-a-b-bootblock.h"
+#include "tests/migration/aarch64-a-b-kernel.h"
 
-static void init_bootfile_x86(const char *bootpath)
+static void init_bootfile(const char *bootpath, void *content)
 {
     FILE *bootfile = fopen(bootpath, "wb");
 
-    g_assert_cmpint(fwrite(x86_bootsect, 512, 1, bootfile), ==, 1);
+    g_assert_cmpint(fwrite(content, 512, 1, bootfile), ==, 1);
     fclose(bootfile);
 }
 
@@ -392,7 +394,7 @@ static void test_migrate_start(QTestState **from, QTestState **to,
     got_stop = false;
 
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
-        init_bootfile_x86(bootpath);
+        init_bootfile(bootpath, x86_bootsect);
         cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
                                   " -name source,debug-threads=on"
                                   " -serial file:%s/src_serial"
@@ -421,6 +423,42 @@ 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) {
+        const char *cpu;
+        const char *gic_ver;
+        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";
+            cpu = "host";
+            gic_ver = "host";
+        } else {
+            accel = "tcg";
+            cpu = "cortex-a57";
+            gic_ver = "2";
+        }
+
+        init_bootfile(bootpath, aarch64_kernel);
+        cmd_src = g_strdup_printf("-machine virt,accel=%s,gic-version=%s "
+                                  "-name vmsource,debug-threads=on -cpu %s "
+                                  "-m 150M -serial file:%s/src_serial "
+                                  "-kernel %s ",
+                                  accel, gic_ver, cpu, tmpfs, bootpath);
+        cmd_dst = g_strdup_printf("-machine virt,accel=%s,gic-version=%s "
+                                  "-name vmdest,debug-threads=on -cpu %s "
+                                  "-m 150M -serial file:%s/dest_serial "
+                                  "-kernel %s "
+                                  "-incoming %s ",
+                                  accel, gic_ver, cpu, tmpfs, bootpath, uri);
+
+        /* aarch64 virt machine physical memory starts at 0x40000000, which
+         * is also the kernel loader base address. It should be fine to
+         * allocate & modify the test memory 1MB away.
+         */
+        start_address = ARM_TEST_MEM_START;
+        end_address = ARM_TEST_MEM_END;
     } else {
         g_assert_not_reached();
     }
@@ -505,7 +543,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 013b8d1f44..1324427a93 100644
--- a/tests/migration/Makefile
+++ b/tests/migration/Makefile
@@ -16,7 +16,7 @@ override define __note
  */
 endef
 
-all: x86-a-b-bootblock.h
+all: x86-a-b-bootblock.h aarch64-a-b-kernel.h
 # Dummy command so that make thinks it has done something
 	@true
 
@@ -24,6 +24,7 @@ SRC_PATH=../..
 include $(SRC_PATH)/rules.mak
 
 x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
+aarch64_cross_prefix := $(call find-cross-prefix,aarch64)
 
 x86-a-b-bootblock.h: x86-a-b-bootblock.S
 	$(x86_64_cross_prefix)gcc -m32 -march=i486 -c $< -o x86.o
@@ -32,5 +33,12 @@ x86-a-b-bootblock.h: x86-a-b-bootblock.S
 	echo "$$__note" > $@
 	xxd -i x86.bootsect | sed -e 's/.*int.*//' >> $@
 
+aarch64-a-b-kernel.h: aarch64-a-b-kernel.S
+	$(aarch64_cross_prefix)gcc -o aarch64.elf -nostdlib \
+		-Wl,--build-id=none $<
+	$(aarch64_cross_prefix)objcopy -O binary aarch64.elf aarch64.kernel
+	echo "$$__note" > $@
+	xxd -i aarch64.kernel | sed -e 's/.*int.*//' >> $@
+
 clean:
-	rm -f *.bootsect *.boot *.o
+	rm -f *.bootsect *.boot *.o *.elf *.kernel
diff --git a/tests/migration/aarch64-a-b-kernel.S b/tests/migration/aarch64-a-b-kernel.S
new file mode 100644
index 0000000000..6640c906cf
--- /dev/null
+++ b/tests/migration/aarch64-a-b-kernel.S
@@ -0,0 +1,71 @@
+#
+# 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.
+
+#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-a-b-kernel.h b/tests/migration/aarch64-a-b-kernel.h
new file mode 100644
index 0000000000..53b44d3e99
--- /dev/null
+++ b/tests/migration/aarch64-a-b-kernel.h
@@ -0,0 +1,19 @@
+/* This file is automatically generated from
+ * tests/migration/aarch64-a-b-kernel.S, edit that and then run
+ * "make aarch64-a-b-kernel.h" 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 48b59b3281..be0771b35e 100644
--- a/tests/migration/migration-test.h
+++ b/tests/migration/migration-test.h
@@ -15,4 +15,9 @@
 /* PPC */
 #define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */
 
+/* AArch64 */
+#define ARM_MACH_VIRT_UART  0x09000000
+#define ARM_TEST_MEM_START  (0x40000000 + TEST_MEM_START)
+#define ARM_TEST_MEM_END    (0x40000000 + TEST_MEM_END)
+
 #endif /* _TEST_MIGRATION_H_ */
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH V5 3/4] tests/migration: Add migration-test header file
  2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 3/4] tests/migration: Add migration-test header file Wei Huang
@ 2018-02-26  9:05   ` Andrew Jones
  2018-02-26  9:38   ` Andrew Jones
  1 sibling, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2018-02-26  9:05 UTC (permalink / raw)
  To: Wei Huang; +Cc: qemu-devel, peter.maydell, dgilbert, quintela

On Fri, Feb 23, 2018 at 03:58:57PM -0600, Wei Huang wrote:
> This patch moves the settings related migration-test from the
> migration-test.c file to a seperate header file. It also renames the
> x86-a-b-bootblock.s file extension from .s to .S, allowing gcc
> pre-processor to include the C-style header file correctly.
> 
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>  tests/migration-test.c                             | 28 +++++++++++-----------
>  tests/migration/Makefile                           |  4 ++--
>  tests/migration/migration-test.h                   | 18 ++++++++++++++
>  .../{x86-a-b-bootblock.s => x86-a-b-bootblock.S}   |  7 +++---
>  tests/migration/x86-a-b-bootblock.h                |  2 +-
>  5 files changed, 39 insertions(+), 20 deletions(-)
>  create mode 100644 tests/migration/migration-test.h
>  rename tests/migration/{x86-a-b-bootblock.s => x86-a-b-bootblock.S} (94%)
>

I gave this my r-b last review. Do I have to review it again? 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH V5 1/4] rules: Move cross compilation auto detection functions to rules.mak
  2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 1/4] rules: Move cross compilation auto detection functions to rules.mak Wei Huang
@ 2018-02-26  9:07   ` Andrew Jones
  0 siblings, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2018-02-26  9:07 UTC (permalink / raw)
  To: Wei Huang; +Cc: qemu-devel, peter.maydell, dgilbert, quintela

On Fri, Feb 23, 2018 at 03:58:55PM -0600, Wei Huang wrote:
> This patch moves the auto detection functions for cross compilation from
> roms/Makefile to rules.mak. So the functions can be shared among Makefiles
> in QEMU.
> 
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>  roms/Makefile | 24 +++++++-----------------
>  rules.mak     | 15 +++++++++++++++
>  2 files changed, 22 insertions(+), 17 deletions(-)
>

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH V5 2/4] tests/migration: Convert the boot block compilation script into Makefile
  2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 2/4] tests/migration: Convert the boot block compilation script into Makefile Wei Huang
@ 2018-02-26  9:07   ` Andrew Jones
  2018-02-26 18:01   ` Dr. David Alan Gilbert
  1 sibling, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2018-02-26  9:07 UTC (permalink / raw)
  To: Wei Huang; +Cc: qemu-devel, peter.maydell, dgilbert, quintela

On Fri, Feb 23, 2018 at 03:58:56PM -0600, 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.
> 
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>  tests/migration/Makefile                 | 36 ++++++++++++++++++++++++++++++++
>  tests/migration/rebuild-x86-bootblock.sh | 33 -----------------------------
>  tests/migration/x86-a-b-bootblock.h      |  2 +-
>  tests/migration/x86-a-b-bootblock.s      |  5 ++---
>  4 files changed, 39 insertions(+), 37 deletions(-)
>  create mode 100644 tests/migration/Makefile
>  delete mode 100755 tests/migration/rebuild-x86-bootblock.sh
>

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH V5 4/4] tests: Add migration test for aarch64
  2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 4/4] tests: Add migration test for aarch64 Wei Huang
@ 2018-02-26  9:30   ` Andrew Jones
  2018-02-26  9:43     ` Andrew Jones
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Jones @ 2018-02-26  9:30 UTC (permalink / raw)
  To: Wei Huang; +Cc: qemu-devel, peter.maydell, dgilbert, quintela

On Fri, Feb 23, 2018 at 03:58:58PM -0600, Wei Huang wrote:
> This patch adds migration test support for aarch64. The test code, which
> implements the same functionality as x86, is booted as a kernel in qemu.
> Here are the design choices we make for aarch64:
> 
>  * We choose this -kernel approach because aarch64 QEMU doesn't provide a
>    built-in fw like x86 does. So instead of relying on a boot loader, we
>    use -kernel approach for aarch64.
>  * The serial output is sent to PL011 directly.
>  * The physical memory base for mach-virt machine is 0x40000000. We change
>    the start_address and end_address for aarch64.
> 
> In addition to providing the binary, this patch also includes the source
> code and the build script in tests/migration/. 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               | 50 ++++++++++++++++++++++---
>  tests/migration/Makefile             | 12 +++++-
>  tests/migration/aarch64-a-b-kernel.S | 71 ++++++++++++++++++++++++++++++++++++
>  tests/migration/aarch64-a-b-kernel.h | 19 ++++++++++
>  tests/migration/migration-test.h     |  5 +++
>  6 files changed, 150 insertions(+), 8 deletions(-)
>  create mode 100644 tests/migration/aarch64-a-b-kernel.S
>  create mode 100644 tests/migration/aarch64-a-b-kernel.h
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index a1bcbffe12..df9f64438f 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -372,6 +372,7 @@ check-qtest-arm-y += tests/sdhci-test$(EXESUF)
>  check-qtest-aarch64-y = tests/numa-test$(EXESUF)
>  check-qtest-aarch64-y += 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 ce2922df6a..d60e34c82d 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -11,6 +11,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include <sys/utsname.h>
>  
>  #include "libqtest.h"
>  #include "qapi/qmp/qdict.h"
> @@ -23,8 +24,8 @@
>  
>  #include "migration/migration-test.h"
>  
> -const unsigned start_address = TEST_MEM_START;
> -const unsigned end_address = TEST_MEM_END;
> +unsigned start_address = TEST_MEM_START;
> +unsigned end_address = TEST_MEM_END;
>  bool got_stop;
>  
>  #if defined(__linux__)
> @@ -81,12 +82,13 @@ static const char *tmpfs;
>   * repeatedly. It outputs a 'B' at a fixed rate while it's still running.
>   */
>  #include "tests/migration/x86-a-b-bootblock.h"
> +#include "tests/migration/aarch64-a-b-kernel.h"
>  
> -static void init_bootfile_x86(const char *bootpath)
> +static void init_bootfile(const char *bootpath, void *content)
>  {
>      FILE *bootfile = fopen(bootpath, "wb");
>  
> -    g_assert_cmpint(fwrite(x86_bootsect, 512, 1, bootfile), ==, 1);
> +    g_assert_cmpint(fwrite(content, 512, 1, bootfile), ==, 1);
>      fclose(bootfile);
>  }
>  
> @@ -392,7 +394,7 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>      got_stop = false;
>  
>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> -        init_bootfile_x86(bootpath);
> +        init_bootfile(bootpath, x86_bootsect);
>          cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
>                                    " -name source,debug-threads=on"
>                                    " -serial file:%s/src_serial"
> @@ -421,6 +423,42 @@ 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) {
> +        const char *cpu;
> +        const char *gic_ver;
> +        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";
> +            cpu = "host";
> +            gic_ver = "host";
> +        } else {
> +            accel = "tcg";
> +            cpu = "cortex-a57";
> +            gic_ver = "2";
> +        }
> +
> +        init_bootfile(bootpath, aarch64_kernel);
> +        cmd_src = g_strdup_printf("-machine virt,accel=%s,gic-version=%s "
> +                                  "-name vmsource,debug-threads=on -cpu %s "
> +                                  "-m 150M -serial file:%s/src_serial "
> +                                  "-kernel %s ",
> +                                  accel, gic_ver, cpu, tmpfs, bootpath);
> +        cmd_dst = g_strdup_printf("-machine virt,accel=%s,gic-version=%s "
> +                                  "-name vmdest,debug-threads=on -cpu %s "
> +                                  "-m 150M -serial file:%s/dest_serial "
> +                                  "-kernel %s "
> +                                  "-incoming %s ",
> +                                  accel, gic_ver, cpu, tmpfs, bootpath, uri);
> +
> +        /* aarch64 virt machine physical memory starts at 0x40000000, which
> +         * is also the kernel loader base address. It should be fine to

It's not the kernel base address.

> +         * allocate & modify the test memory 1MB away.

It's only 512K away - which is still probably fine, but you
said he found data once when reading every 4K after 1M, so
maybe not.

> +         */
> +        start_address = ARM_TEST_MEM_START;
> +        end_address = ARM_TEST_MEM_END;
>      } else {
>          g_assert_not_reached();
>      }
> @@ -505,7 +543,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 013b8d1f44..1324427a93 100644
> --- a/tests/migration/Makefile
> +++ b/tests/migration/Makefile
> @@ -16,7 +16,7 @@ override define __note
>   */
>  endef
>  
> -all: x86-a-b-bootblock.h
> +all: x86-a-b-bootblock.h aarch64-a-b-kernel.h
>  # Dummy command so that make thinks it has done something
>  	@true
>  
> @@ -24,6 +24,7 @@ SRC_PATH=../..
>  include $(SRC_PATH)/rules.mak
>  
>  x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
> +aarch64_cross_prefix := $(call find-cross-prefix,aarch64)
>  
>  x86-a-b-bootblock.h: x86-a-b-bootblock.S
>  	$(x86_64_cross_prefix)gcc -m32 -march=i486 -c $< -o x86.o
> @@ -32,5 +33,12 @@ x86-a-b-bootblock.h: x86-a-b-bootblock.S
>  	echo "$$__note" > $@
>  	xxd -i x86.bootsect | sed -e 's/.*int.*//' >> $@
>  
> +aarch64-a-b-kernel.h: aarch64-a-b-kernel.S
> +	$(aarch64_cross_prefix)gcc -o aarch64.elf -nostdlib \
> +		-Wl,--build-id=none $<
> +	$(aarch64_cross_prefix)objcopy -O binary aarch64.elf aarch64.kernel
> +	echo "$$__note" > $@
> +	xxd -i aarch64.kernel | sed -e 's/.*int.*//' >> $@
> +
>  clean:
> -	rm -f *.bootsect *.boot *.o
> +	rm -f *.bootsect *.boot *.o *.elf *.kernel
> diff --git a/tests/migration/aarch64-a-b-kernel.S b/tests/migration/aarch64-a-b-kernel.S
> new file mode 100644
> index 0000000000..6640c906cf
> --- /dev/null
> +++ b/tests/migration/aarch64-a-b-kernel.S
> @@ -0,0 +1,71 @@
> +#
> +# 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.
> +
> +#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-a-b-kernel.h b/tests/migration/aarch64-a-b-kernel.h
> new file mode 100644
> index 0000000000..53b44d3e99
> --- /dev/null
> +++ b/tests/migration/aarch64-a-b-kernel.h
> @@ -0,0 +1,19 @@
> +/* This file is automatically generated from
> + * tests/migration/aarch64-a-b-kernel.S, edit that and then run
> + * "make aarch64-a-b-kernel.h" 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 48b59b3281..be0771b35e 100644
> --- a/tests/migration/migration-test.h
> +++ b/tests/migration/migration-test.h
> @@ -15,4 +15,9 @@
>  /* PPC */
>  #define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */
>  
> +/* AArch64 */
> +#define ARM_MACH_VIRT_UART  0x09000000
> +#define ARM_TEST_MEM_START  (0x40000000 + TEST_MEM_START)
> +#define ARM_TEST_MEM_END    (0x40000000 + TEST_MEM_END)
> +
>  #endif /* _TEST_MIGRATION_H_ */
> -- 
> 2.14.3
> 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH V5 3/4] tests/migration: Add migration-test header file
  2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 3/4] tests/migration: Add migration-test header file Wei Huang
  2018-02-26  9:05   ` Andrew Jones
@ 2018-02-26  9:38   ` Andrew Jones
  1 sibling, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2018-02-26  9:38 UTC (permalink / raw)
  To: Wei Huang; +Cc: qemu-devel, peter.maydell, dgilbert, quintela

On Fri, Feb 23, 2018 at 03:58:57PM -0600, Wei Huang wrote:
> This patch moves the settings related migration-test from the
> migration-test.c file to a seperate header file. It also renames the
> x86-a-b-bootblock.s file extension from .s to .S, allowing gcc
> pre-processor to include the C-style header file correctly.
> 
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>  tests/migration-test.c                             | 28 +++++++++++-----------
>  tests/migration/Makefile                           |  4 ++--
>  tests/migration/migration-test.h                   | 18 ++++++++++++++
>  .../{x86-a-b-bootblock.s => x86-a-b-bootblock.S}   |  7 +++---
>  tests/migration/x86-a-b-bootblock.h                |  2 +-
>  5 files changed, 39 insertions(+), 20 deletions(-)
>  create mode 100644 tests/migration/migration-test.h
>  rename tests/migration/{x86-a-b-bootblock.s => x86-a-b-bootblock.S} (94%)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 74f9361bdd..ce2922df6a 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -21,10 +21,10 @@
>  #include "sysemu/sysemu.h"
>  #include "hw/nvram/chrp_nvram.h"
>  
> -#define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */
> +#include "migration/migration-test.h"
>  
> -const unsigned start_address = 1024 * 1024;
> -const unsigned end_address = 100 * 1024 * 1024;
> +const unsigned start_address = TEST_MEM_START;
> +const unsigned end_address = TEST_MEM_END;
>  bool got_stop;
>  
>  #if defined(__linux__)
> @@ -77,8 +77,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-a-b-bootblock.h"
>  
> @@ -104,9 +104,8 @@ static void init_bootfile_ppc(const char *bootpath)
>      memcpy(header->name, "common", 6);
>      chrp_nvram_finish_partition(header, MIN_NVRAM_SIZE);
>  
> -    /* FW_MAX_SIZE is 4MB, but slof.bin is only 900KB,
> -     * so let's modify memory between 1MB and 100MB
> -     * to do like PC bootsector
> +    /* FW_MAX_SIZE is 4MB, but slof.bin is only 900KB. So it is OK to modify
> +     * memory between start_address and end_address like PC bootsector does.
>       */
>  
>      sprintf(buf + 16,
> @@ -263,11 +262,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

need space before end_address

> +     * 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;
> @@ -278,7 +277,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);
> diff --git a/tests/migration/Makefile b/tests/migration/Makefile
> index 8fbedaa8b8..013b8d1f44 100644
> --- a/tests/migration/Makefile
> +++ b/tests/migration/Makefile
> @@ -25,8 +25,8 @@ include $(SRC_PATH)/rules.mak
>  
>  x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
>  
> -x86-a-b-bootblock.h: x86-a-b-bootblock.s
> -	$(x86_64_cross_prefix)as --32 -march=i486 $< -o x86.o
> +x86-a-b-bootblock.h: x86-a-b-bootblock.S
> +	$(x86_64_cross_prefix)gcc -m32 -march=i486 -c $< -o x86.o
>  	$(x86_64_cross_prefix)objcopy -O binary x86.o x86.boot
>  	dd if=x86.boot of=x86.bootsect bs=256 count=2 skip=124
>  	echo "$$__note" > $@
> diff --git a/tests/migration/migration-test.h b/tests/migration/migration-test.h
> new file mode 100644
> index 0000000000..48b59b3281
> --- /dev/null
> +++ b/tests/migration/migration-test.h
> @@ -0,0 +1,18 @@
> +/*
> + * 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_START      (1 * 1024 * 1024)
> +#define TEST_MEM_END        (100 * 1024 * 1024)
> +#define TEST_MEM_PAGE_SIZE  4096
> +
> +/* PPC */
> +#define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */
> +
> +#endif /* _TEST_MIGRATION_H_ */
> diff --git a/tests/migration/x86-a-b-bootblock.s b/tests/migration/x86-a-b-bootblock.S
> similarity index 94%
> rename from tests/migration/x86-a-b-bootblock.s
> rename to tests/migration/x86-a-b-bootblock.S
> index 98dbfab084..08b51f9e7f 100644
> --- a/tests/migration/x86-a-b-bootblock.s
> +++ b/tests/migration/x86-a-b-bootblock.S
> @@ -12,6 +12,7 @@
>  #
>  # Author: dgilbert@redhat.com
>  
> +#include "migration-test.h"
>  
>  .code16
>  .org 0x7c00
> @@ -45,11 +46,11 @@ start:             # at 0x7c00 ?
>          mov $0, %bl
>  mainloop:
>          # Start from 1MB
> -        mov $(1024*1024),%eax
> +        mov $TEST_MEM_START,%eax
>  innerloop:
>          incb (%eax)
> -        add $4096,%eax
> -        cmp $(100*1024*1024),%eax
> +        add $TEST_MEM_PAGE_SIZE,%eax
> +        cmp $TEST_MEM_END,%eax
>          jl innerloop
>  
>          inc %bl
> diff --git a/tests/migration/x86-a-b-bootblock.h b/tests/migration/x86-a-b-bootblock.h
> index 9e8e2e028b..44e4b99506 100644
> --- a/tests/migration/x86-a-b-bootblock.h
> +++ b/tests/migration/x86-a-b-bootblock.h
> @@ -1,5 +1,5 @@
>  /* This file is automatically generated from
> - * tests/migration/x86-a-b-bootblock.s, edit that and then run
> + * tests/migration/x86-a-b-bootblock.S, edit that and then run
>   * "make x86-a-b-bootblock.h" inside tests/migration to update,
>   * and then remember to send both in your patch submission.
>   */
> -- 
> 2.14.3
> 
>

So I reviewed again and see the comments got updated. You could have put
my r-b on anyway. You can again, but I'd still recommend fixing the '<'

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH V5 4/4] tests: Add migration test for aarch64
  2018-02-26  9:30   ` Andrew Jones
@ 2018-02-26  9:43     ` Andrew Jones
  0 siblings, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2018-02-26  9:43 UTC (permalink / raw)
  To: Wei Huang; +Cc: peter.maydell, quintela, qemu-devel, dgilbert

On Mon, Feb 26, 2018 at 10:30:31AM +0100, Andrew Jones wrote:
> On Fri, Feb 23, 2018 at 03:58:58PM -0600, Wei Huang wrote:
> > +        /* aarch64 virt machine physical memory starts at 0x40000000, which
> > +         * is also the kernel loader base address. It should be fine to
> 
> It's not the kernel base address.
> 
> > +         * allocate & modify the test memory 1MB away.
> 
> It's only 512K away - which is still probably fine, but you
> said he found data once when reading every 4K after 1M, so
> maybe not.
>

BTW, I'd just drop this comment altogether, rather than fix it.
Or fix it, but put it in the header. The point of the header
is to avoid these addresses spreading around too much. Putting
them in comments doesn't help.

drew

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH V5 2/4] tests/migration: Convert the boot block compilation script into Makefile
  2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 2/4] tests/migration: Convert the boot block compilation script into Makefile Wei Huang
  2018-02-26  9:07   ` Andrew Jones
@ 2018-02-26 18:01   ` Dr. David Alan Gilbert
  2018-02-26 18:28     ` Wei Huang
  1 sibling, 1 reply; 16+ messages in thread
From: Dr. David Alan Gilbert @ 2018-02-26 18:01 UTC (permalink / raw)
  To: Wei Huang; +Cc: qemu-devel, peter.maydell, quintela, drjones

* 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.
> 
> Signed-off-by: Wei Huang <wei@redhat.com>

But what happens if you're on a system that doesn't have the cross
compilers?  I found it's too easy to get stuck with it trying
to rebuild the header when there's no need.

Dave

> ---
>  tests/migration/Makefile                 | 36 ++++++++++++++++++++++++++++++++
>  tests/migration/rebuild-x86-bootblock.sh | 33 -----------------------------
>  tests/migration/x86-a-b-bootblock.h      |  2 +-
>  tests/migration/x86-a-b-bootblock.s      |  5 ++---
>  4 files changed, 39 insertions(+), 37 deletions(-)
>  create mode 100644 tests/migration/Makefile
>  delete mode 100755 tests/migration/rebuild-x86-bootblock.sh
> 
> diff --git a/tests/migration/Makefile b/tests/migration/Makefile
> new file mode 100644
> index 0000000000..8fbedaa8b8
> --- /dev/null
> +++ b/tests/migration/Makefile
> @@ -0,0 +1,36 @@
> +#
> +# Copyright (c) 2016-2018 Red Hat, Inc. and/or its affiliates
> +#
> +# Authors:
> +#   Dave Gilbert <dgilbert@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.
> +#
> +export __note
> +override define __note
> +/* This file is automatically generated from
> + * tests/migration/$<, edit that and then run
> + * "make $@" inside tests/migration to update,
> + * and then remember to send both in your patch submission.
> + */
> +endef
> +
> +all: x86-a-b-bootblock.h
> +# Dummy command so that make thinks it has done something
> +	@true
> +
> +SRC_PATH=../..
> +include $(SRC_PATH)/rules.mak
> +
> +x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
> +
> +x86-a-b-bootblock.h: x86-a-b-bootblock.s
> +	$(x86_64_cross_prefix)as --32 -march=i486 $< -o x86.o
> +	$(x86_64_cross_prefix)objcopy -O binary x86.o x86.boot
> +	dd if=x86.boot of=x86.bootsect bs=256 count=2 skip=124
> +	echo "$$__note" > $@
> +	xxd -i x86.bootsect | sed -e 's/.*int.*//' >> $@
> +
> +clean:
> +	rm -f *.bootsect *.boot *.o
> diff --git a/tests/migration/rebuild-x86-bootblock.sh b/tests/migration/rebuild-x86-bootblock.sh
> deleted file mode 100755
> index 86cec5d284..0000000000
> --- 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.h b/tests/migration/x86-a-b-bootblock.h
> index 78a151fe2a..9e8e2e028b 100644
> --- a/tests/migration/x86-a-b-bootblock.h
> +++ b/tests/migration/x86-a-b-bootblock.h
> @@ -1,6 +1,6 @@
>  /* 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,
> + * "make x86-a-b-bootblock.h" inside tests/migration to update,
>   * and then remember to send both in your patch submission.
>   */
>  unsigned char x86_bootsect[] = {
> diff --git a/tests/migration/x86-a-b-bootblock.s b/tests/migration/x86-a-b-bootblock.s
> index b1642641a7..98dbfab084 100644
> --- a/tests/migration/x86-a-b-bootblock.s
> +++ b/tests/migration/x86-a-b-bootblock.s
> @@ -3,9 +3,8 @@
>  #  range.
>  #  Outputs an initial 'A' on serial followed by repeated 'B's
>  #
> -# run   tests/migration/rebuild-x86-bootblock.sh
> -#   to regenerate the hex, and remember to include both the .h and .s
> -#   in any patches.
> +#  In tests/migration dir, run 'make x86-a-b-bootblock.h' to regenerate
> +#  the hex, and remember to include both the .h and .s in any patches.
>  #
>  # Copyright (c) 2016 Red Hat, Inc. and/or its affiliates
>  # This work is licensed under the terms of the GNU GPL, version 2 or later.
> -- 
> 2.14.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH V5 2/4] tests/migration: Convert the boot block compilation script into Makefile
  2018-02-26 18:01   ` Dr. David Alan Gilbert
@ 2018-02-26 18:28     ` Wei Huang
  2018-02-27 11:38       ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 16+ messages in thread
From: Wei Huang @ 2018-02-26 18:28 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: qemu-devel, peter.maydell, quintela, drjones



On 02/26/2018 12:01 PM, Dr. David Alan Gilbert wrote:
> * 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.
>>
>> Signed-off-by: Wei Huang <wei@redhat.com>
> 
> But what happens if you're on a system that doesn't have the cross
> compilers?  I found it's too easy to get stuck with it trying
> to rebuild the header when there's no need.

I think it is fine. If no cross compiler is present, NNN_cross_prefix
won't find anything. So it ends up with using the gcc tools on host
systems. Obviously the compilation will fail. This should become the
problem to solve for whoever wants to do cross-compile. For us, we have
done our part because the .h files have been provided.

> 
> Dave
> 
>> ---
>>  tests/migration/Makefile                 | 36 ++++++++++++++++++++++++++++++++
>>  tests/migration/rebuild-x86-bootblock.sh | 33 -----------------------------
>>  tests/migration/x86-a-b-bootblock.h      |  2 +-
>>  tests/migration/x86-a-b-bootblock.s      |  5 ++---
>>  4 files changed, 39 insertions(+), 37 deletions(-)
>>  create mode 100644 tests/migration/Makefile
>>  delete mode 100755 tests/migration/rebuild-x86-bootblock.sh
>>
>> diff --git a/tests/migration/Makefile b/tests/migration/Makefile
>> new file mode 100644
>> index 0000000000..8fbedaa8b8
>> --- /dev/null
>> +++ b/tests/migration/Makefile
>> @@ -0,0 +1,36 @@
>> +#
>> +# Copyright (c) 2016-2018 Red Hat, Inc. and/or its affiliates
>> +#
>> +# Authors:
>> +#   Dave Gilbert <dgilbert@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.
>> +#
>> +export __note
>> +override define __note
>> +/* This file is automatically generated from
>> + * tests/migration/$<, edit that and then run
>> + * "make $@" inside tests/migration to update,
>> + * and then remember to send both in your patch submission.
>> + */
>> +endef
>> +
>> +all: x86-a-b-bootblock.h
>> +# Dummy command so that make thinks it has done something
>> +	@true
>> +
>> +SRC_PATH=../..
>> +include $(SRC_PATH)/rules.mak
>> +
>> +x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
>> +
>> +x86-a-b-bootblock.h: x86-a-b-bootblock.s
>> +	$(x86_64_cross_prefix)as --32 -march=i486 $< -o x86.o
>> +	$(x86_64_cross_prefix)objcopy -O binary x86.o x86.boot
>> +	dd if=x86.boot of=x86.bootsect bs=256 count=2 skip=124
>> +	echo "$$__note" > $@
>> +	xxd -i x86.bootsect | sed -e 's/.*int.*//' >> $@
>> +
>> +clean:
>> +	rm -f *.bootsect *.boot *.o
>> diff --git a/tests/migration/rebuild-x86-bootblock.sh b/tests/migration/rebuild-x86-bootblock.sh
>> deleted file mode 100755
>> index 86cec5d284..0000000000
>> --- 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.h b/tests/migration/x86-a-b-bootblock.h
>> index 78a151fe2a..9e8e2e028b 100644
>> --- a/tests/migration/x86-a-b-bootblock.h
>> +++ b/tests/migration/x86-a-b-bootblock.h
>> @@ -1,6 +1,6 @@
>>  /* 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,
>> + * "make x86-a-b-bootblock.h" inside tests/migration to update,
>>   * and then remember to send both in your patch submission.
>>   */
>>  unsigned char x86_bootsect[] = {
>> diff --git a/tests/migration/x86-a-b-bootblock.s b/tests/migration/x86-a-b-bootblock.s
>> index b1642641a7..98dbfab084 100644
>> --- a/tests/migration/x86-a-b-bootblock.s
>> +++ b/tests/migration/x86-a-b-bootblock.s
>> @@ -3,9 +3,8 @@
>>  #  range.
>>  #  Outputs an initial 'A' on serial followed by repeated 'B's
>>  #
>> -# run   tests/migration/rebuild-x86-bootblock.sh
>> -#   to regenerate the hex, and remember to include both the .h and .s
>> -#   in any patches.
>> +#  In tests/migration dir, run 'make x86-a-b-bootblock.h' to regenerate
>> +#  the hex, and remember to include both the .h and .s in any patches.
>>  #
>>  # Copyright (c) 2016 Red Hat, Inc. and/or its affiliates
>>  # This work is licensed under the terms of the GNU GPL, version 2 or later.
>> -- 
>> 2.14.3
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH V5 2/4] tests/migration: Convert the boot block compilation script into Makefile
  2018-02-26 18:28     ` Wei Huang
@ 2018-02-27 11:38       ` Dr. David Alan Gilbert
  2018-02-27 15:21         ` Wei Huang
  0 siblings, 1 reply; 16+ messages in thread
From: Dr. David Alan Gilbert @ 2018-02-27 11:38 UTC (permalink / raw)
  To: Wei Huang; +Cc: qemu-devel, peter.maydell, quintela, drjones

* Wei Huang (wei@redhat.com) wrote:
> 
> 
> On 02/26/2018 12:01 PM, Dr. David Alan Gilbert wrote:
> > * 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.
> >>
> >> Signed-off-by: Wei Huang <wei@redhat.com>
> > 
> > But what happens if you're on a system that doesn't have the cross
> > compilers?  I found it's too easy to get stuck with it trying
> > to rebuild the header when there's no need.
> 
> I think it is fine. If no cross compiler is present, NNN_cross_prefix
> won't find anything. So it ends up with using the gcc tools on host
> systems. Obviously the compilation will fail. This should become the
> problem to solve for whoever wants to do cross-compile. For us, we have
> done our part because the .h files have been provided.

But isn't it a problem for people just trying to make check?
One of the things that convinced me to move it out of the makefile and
into a separate script was that it was too easy to get into a situation
where Make wanted to rebuild the headerilfe on a system.

Dave

> > 
> > Dave
> > 
> >> ---
> >>  tests/migration/Makefile                 | 36 ++++++++++++++++++++++++++++++++
> >>  tests/migration/rebuild-x86-bootblock.sh | 33 -----------------------------
> >>  tests/migration/x86-a-b-bootblock.h      |  2 +-
> >>  tests/migration/x86-a-b-bootblock.s      |  5 ++---
> >>  4 files changed, 39 insertions(+), 37 deletions(-)
> >>  create mode 100644 tests/migration/Makefile
> >>  delete mode 100755 tests/migration/rebuild-x86-bootblock.sh
> >>
> >> diff --git a/tests/migration/Makefile b/tests/migration/Makefile
> >> new file mode 100644
> >> index 0000000000..8fbedaa8b8
> >> --- /dev/null
> >> +++ b/tests/migration/Makefile
> >> @@ -0,0 +1,36 @@
> >> +#
> >> +# Copyright (c) 2016-2018 Red Hat, Inc. and/or its affiliates
> >> +#
> >> +# Authors:
> >> +#   Dave Gilbert <dgilbert@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.
> >> +#
> >> +export __note
> >> +override define __note
> >> +/* This file is automatically generated from
> >> + * tests/migration/$<, edit that and then run
> >> + * "make $@" inside tests/migration to update,
> >> + * and then remember to send both in your patch submission.
> >> + */
> >> +endef
> >> +
> >> +all: x86-a-b-bootblock.h
> >> +# Dummy command so that make thinks it has done something
> >> +	@true
> >> +
> >> +SRC_PATH=../..
> >> +include $(SRC_PATH)/rules.mak
> >> +
> >> +x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
> >> +
> >> +x86-a-b-bootblock.h: x86-a-b-bootblock.s
> >> +	$(x86_64_cross_prefix)as --32 -march=i486 $< -o x86.o
> >> +	$(x86_64_cross_prefix)objcopy -O binary x86.o x86.boot
> >> +	dd if=x86.boot of=x86.bootsect bs=256 count=2 skip=124
> >> +	echo "$$__note" > $@
> >> +	xxd -i x86.bootsect | sed -e 's/.*int.*//' >> $@
> >> +
> >> +clean:
> >> +	rm -f *.bootsect *.boot *.o
> >> diff --git a/tests/migration/rebuild-x86-bootblock.sh b/tests/migration/rebuild-x86-bootblock.sh
> >> deleted file mode 100755
> >> index 86cec5d284..0000000000
> >> --- 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.h b/tests/migration/x86-a-b-bootblock.h
> >> index 78a151fe2a..9e8e2e028b 100644
> >> --- a/tests/migration/x86-a-b-bootblock.h
> >> +++ b/tests/migration/x86-a-b-bootblock.h
> >> @@ -1,6 +1,6 @@
> >>  /* 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,
> >> + * "make x86-a-b-bootblock.h" inside tests/migration to update,
> >>   * and then remember to send both in your patch submission.
> >>   */
> >>  unsigned char x86_bootsect[] = {
> >> diff --git a/tests/migration/x86-a-b-bootblock.s b/tests/migration/x86-a-b-bootblock.s
> >> index b1642641a7..98dbfab084 100644
> >> --- a/tests/migration/x86-a-b-bootblock.s
> >> +++ b/tests/migration/x86-a-b-bootblock.s
> >> @@ -3,9 +3,8 @@
> >>  #  range.
> >>  #  Outputs an initial 'A' on serial followed by repeated 'B's
> >>  #
> >> -# run   tests/migration/rebuild-x86-bootblock.sh
> >> -#   to regenerate the hex, and remember to include both the .h and .s
> >> -#   in any patches.
> >> +#  In tests/migration dir, run 'make x86-a-b-bootblock.h' to regenerate
> >> +#  the hex, and remember to include both the .h and .s in any patches.
> >>  #
> >>  # Copyright (c) 2016 Red Hat, Inc. and/or its affiliates
> >>  # This work is licensed under the terms of the GNU GPL, version 2 or later.
> >> -- 
> >> 2.14.3
> >>
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH V5 2/4] tests/migration: Convert the boot block compilation script into Makefile
  2018-02-27 11:38       ` Dr. David Alan Gilbert
@ 2018-02-27 15:21         ` Wei Huang
  2018-02-27 16:19           ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 16+ messages in thread
From: Wei Huang @ 2018-02-27 15:21 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: qemu-devel, peter.maydell, quintela, drjones



On 02/27/2018 05:38 AM, Dr. David Alan Gilbert wrote:
> * Wei Huang (wei@redhat.com) wrote:
>>
>>
>> On 02/26/2018 12:01 PM, Dr. David Alan Gilbert wrote:
>>> * 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.
>>>>
>>>> Signed-off-by: Wei Huang <wei@redhat.com>
>>>
>>> But what happens if you're on a system that doesn't have the cross
>>> compilers?  I found it's too easy to get stuck with it trying
>>> to rebuild the header when there's no need.
>>
>> I think it is fine. If no cross compiler is present, NNN_cross_prefix
>> won't find anything. So it ends up with using the gcc tools on host
>> systems. Obviously the compilation will fail. This should become the
>> problem to solve for whoever wants to do cross-compile. For us, we have
>> done our part because the .h files have been provided.
> 
> But isn't it a problem for people just trying to make check?

"make check" doesn't invoke the Makefile in tests/migration/. This
Makefile is only invoked when people manually run "make" or "make
x86-a-b-bootblock.h" inside the tests/migration/ directory.

> One of the things that convinced me to move it out of the makefile and
> into a separate script was that it was too easy to get into a situation
> where Make wanted to rebuild the headerilfe on a system.

This could be potentially one concern. But the real reason behind this
concern is the cross-compilation doesn't always work (it depends on the
availability of cross-compiler tools). My argument is this problem also
exists in the original script approach. As long as QEMU doesn't include
tests/migration/Makefile in "make" or "make check", I think this rebuild
headerfile situation won't happen.

Also, using Makefile, we don't need to write a script for each single
architecture. And it is easier to support cross-compilation detection
(than script).

Thanks,
-Wei

> 
> Dave
> 
>>>
>>> Dave
>>>
>>>> ---
>>>>  tests/migration/Makefile                 | 36 ++++++++++++++++++++++++++++++++
>>>>  tests/migration/rebuild-x86-bootblock.sh | 33 -----------------------------
>>>>  tests/migration/x86-a-b-bootblock.h      |  2 +-
>>>>  tests/migration/x86-a-b-bootblock.s      |  5 ++---
>>>>  4 files changed, 39 insertions(+), 37 deletions(-)
>>>>  create mode 100644 tests/migration/Makefile
>>>>  delete mode 100755 tests/migration/rebuild-x86-bootblock.sh
>>>>
>>>> diff --git a/tests/migration/Makefile b/tests/migration/Makefile
>>>> new file mode 100644
>>>> index 0000000000..8fbedaa8b8
>>>> --- /dev/null
>>>> +++ b/tests/migration/Makefile
>>>> @@ -0,0 +1,36 @@
>>>> +#
>>>> +# Copyright (c) 2016-2018 Red Hat, Inc. and/or its affiliates
>>>> +#
>>>> +# Authors:
>>>> +#   Dave Gilbert <dgilbert@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.
>>>> +#
>>>> +export __note
>>>> +override define __note
>>>> +/* This file is automatically generated from
>>>> + * tests/migration/$<, edit that and then run
>>>> + * "make $@" inside tests/migration to update,
>>>> + * and then remember to send both in your patch submission.
>>>> + */
>>>> +endef
>>>> +
>>>> +all: x86-a-b-bootblock.h
>>>> +# Dummy command so that make thinks it has done something
>>>> +	@true
>>>> +
>>>> +SRC_PATH=../..
>>>> +include $(SRC_PATH)/rules.mak
>>>> +
>>>> +x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
>>>> +
>>>> +x86-a-b-bootblock.h: x86-a-b-bootblock.s
>>>> +	$(x86_64_cross_prefix)as --32 -march=i486 $< -o x86.o
>>>> +	$(x86_64_cross_prefix)objcopy -O binary x86.o x86.boot
>>>> +	dd if=x86.boot of=x86.bootsect bs=256 count=2 skip=124
>>>> +	echo "$$__note" > $@
>>>> +	xxd -i x86.bootsect | sed -e 's/.*int.*//' >> $@
>>>> +
>>>> +clean:
>>>> +	rm -f *.bootsect *.boot *.o
>>>> diff --git a/tests/migration/rebuild-x86-bootblock.sh b/tests/migration/rebuild-x86-bootblock.sh
>>>> deleted file mode 100755
>>>> index 86cec5d284..0000000000
>>>> --- 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.h b/tests/migration/x86-a-b-bootblock.h
>>>> index 78a151fe2a..9e8e2e028b 100644
>>>> --- a/tests/migration/x86-a-b-bootblock.h
>>>> +++ b/tests/migration/x86-a-b-bootblock.h
>>>> @@ -1,6 +1,6 @@
>>>>  /* 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,
>>>> + * "make x86-a-b-bootblock.h" inside tests/migration to update,
>>>>   * and then remember to send both in your patch submission.
>>>>   */
>>>>  unsigned char x86_bootsect[] = {
>>>> diff --git a/tests/migration/x86-a-b-bootblock.s b/tests/migration/x86-a-b-bootblock.s
>>>> index b1642641a7..98dbfab084 100644
>>>> --- a/tests/migration/x86-a-b-bootblock.s
>>>> +++ b/tests/migration/x86-a-b-bootblock.s
>>>> @@ -3,9 +3,8 @@
>>>>  #  range.
>>>>  #  Outputs an initial 'A' on serial followed by repeated 'B's
>>>>  #
>>>> -# run   tests/migration/rebuild-x86-bootblock.sh
>>>> -#   to regenerate the hex, and remember to include both the .h and .s
>>>> -#   in any patches.
>>>> +#  In tests/migration dir, run 'make x86-a-b-bootblock.h' to regenerate
>>>> +#  the hex, and remember to include both the .h and .s in any patches.
>>>>  #
>>>>  # Copyright (c) 2016 Red Hat, Inc. and/or its affiliates
>>>>  # This work is licensed under the terms of the GNU GPL, version 2 or later.
>>>> -- 
>>>> 2.14.3
>>>>
>>> --
>>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH V5 2/4] tests/migration: Convert the boot block compilation script into Makefile
  2018-02-27 15:21         ` Wei Huang
@ 2018-02-27 16:19           ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 16+ messages in thread
From: Dr. David Alan Gilbert @ 2018-02-27 16:19 UTC (permalink / raw)
  To: Wei Huang; +Cc: qemu-devel, peter.maydell, quintela, drjones

* Wei Huang (wei@redhat.com) wrote:
> 
> 
> On 02/27/2018 05:38 AM, Dr. David Alan Gilbert wrote:
> > * Wei Huang (wei@redhat.com) wrote:
> >>
> >>
> >> On 02/26/2018 12:01 PM, Dr. David Alan Gilbert wrote:
> >>> * 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.
> >>>>
> >>>> Signed-off-by: Wei Huang <wei@redhat.com>
> >>>
> >>> But what happens if you're on a system that doesn't have the cross
> >>> compilers?  I found it's too easy to get stuck with it trying
> >>> to rebuild the header when there's no need.
> >>
> >> I think it is fine. If no cross compiler is present, NNN_cross_prefix
> >> won't find anything. So it ends up with using the gcc tools on host
> >> systems. Obviously the compilation will fail. This should become the
> >> problem to solve for whoever wants to do cross-compile. For us, we have
> >> done our part because the .h files have been provided.
> > 
> > But isn't it a problem for people just trying to make check?
> 
> "make check" doesn't invoke the Makefile in tests/migration/. This
> Makefile is only invoked when people manually run "make" or "make
> x86-a-b-bootblock.h" inside the tests/migration/ directory.

Ah! That's what I hadn't spotted; I'd not noticed you'd created a
separate non-included Makefile.

In that case, yes:


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> > One of the things that convinced me to move it out of the makefile and
> > into a separate script was that it was too easy to get into a situation
> > where Make wanted to rebuild the headerilfe on a system.
> 
> This could be potentially one concern. But the real reason behind this
> concern is the cross-compilation doesn't always work (it depends on the
> availability of cross-compiler tools). My argument is this problem also
> exists in the original script approach. As long as QEMU doesn't include
> tests/migration/Makefile in "make" or "make check", I think this rebuild
> headerfile situation won't happen.

Yes.

> Also, using Makefile, we don't need to write a script for each single
> architecture. And it is easier to support cross-compilation detection
> (than script).

That doesn't make too much difference to me; but as long as it's a
separate Makefile that's OK.

Dave

> Thanks,
> -Wei
> 
> > 
> > Dave
> > 
> >>>
> >>> Dave
> >>>
> >>>> ---
> >>>>  tests/migration/Makefile                 | 36 ++++++++++++++++++++++++++++++++
> >>>>  tests/migration/rebuild-x86-bootblock.sh | 33 -----------------------------
> >>>>  tests/migration/x86-a-b-bootblock.h      |  2 +-
> >>>>  tests/migration/x86-a-b-bootblock.s      |  5 ++---
> >>>>  4 files changed, 39 insertions(+), 37 deletions(-)
> >>>>  create mode 100644 tests/migration/Makefile
> >>>>  delete mode 100755 tests/migration/rebuild-x86-bootblock.sh
> >>>>
> >>>> diff --git a/tests/migration/Makefile b/tests/migration/Makefile
> >>>> new file mode 100644
> >>>> index 0000000000..8fbedaa8b8
> >>>> --- /dev/null
> >>>> +++ b/tests/migration/Makefile
> >>>> @@ -0,0 +1,36 @@
> >>>> +#
> >>>> +# Copyright (c) 2016-2018 Red Hat, Inc. and/or its affiliates
> >>>> +#
> >>>> +# Authors:
> >>>> +#   Dave Gilbert <dgilbert@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.
> >>>> +#
> >>>> +export __note
> >>>> +override define __note
> >>>> +/* This file is automatically generated from
> >>>> + * tests/migration/$<, edit that and then run
> >>>> + * "make $@" inside tests/migration to update,
> >>>> + * and then remember to send both in your patch submission.
> >>>> + */
> >>>> +endef
> >>>> +
> >>>> +all: x86-a-b-bootblock.h
> >>>> +# Dummy command so that make thinks it has done something
> >>>> +	@true
> >>>> +
> >>>> +SRC_PATH=../..
> >>>> +include $(SRC_PATH)/rules.mak
> >>>> +
> >>>> +x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
> >>>> +
> >>>> +x86-a-b-bootblock.h: x86-a-b-bootblock.s
> >>>> +	$(x86_64_cross_prefix)as --32 -march=i486 $< -o x86.o
> >>>> +	$(x86_64_cross_prefix)objcopy -O binary x86.o x86.boot
> >>>> +	dd if=x86.boot of=x86.bootsect bs=256 count=2 skip=124
> >>>> +	echo "$$__note" > $@
> >>>> +	xxd -i x86.bootsect | sed -e 's/.*int.*//' >> $@
> >>>> +
> >>>> +clean:
> >>>> +	rm -f *.bootsect *.boot *.o
> >>>> diff --git a/tests/migration/rebuild-x86-bootblock.sh b/tests/migration/rebuild-x86-bootblock.sh
> >>>> deleted file mode 100755
> >>>> index 86cec5d284..0000000000
> >>>> --- 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.h b/tests/migration/x86-a-b-bootblock.h
> >>>> index 78a151fe2a..9e8e2e028b 100644
> >>>> --- a/tests/migration/x86-a-b-bootblock.h
> >>>> +++ b/tests/migration/x86-a-b-bootblock.h
> >>>> @@ -1,6 +1,6 @@
> >>>>  /* 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,
> >>>> + * "make x86-a-b-bootblock.h" inside tests/migration to update,
> >>>>   * and then remember to send both in your patch submission.
> >>>>   */
> >>>>  unsigned char x86_bootsect[] = {
> >>>> diff --git a/tests/migration/x86-a-b-bootblock.s b/tests/migration/x86-a-b-bootblock.s
> >>>> index b1642641a7..98dbfab084 100644
> >>>> --- a/tests/migration/x86-a-b-bootblock.s
> >>>> +++ b/tests/migration/x86-a-b-bootblock.s
> >>>> @@ -3,9 +3,8 @@
> >>>>  #  range.
> >>>>  #  Outputs an initial 'A' on serial followed by repeated 'B's
> >>>>  #
> >>>> -# run   tests/migration/rebuild-x86-bootblock.sh
> >>>> -#   to regenerate the hex, and remember to include both the .h and .s
> >>>> -#   in any patches.
> >>>> +#  In tests/migration dir, run 'make x86-a-b-bootblock.h' to regenerate
> >>>> +#  the hex, and remember to include both the .h and .s in any patches.
> >>>>  #
> >>>>  # Copyright (c) 2016 Red Hat, Inc. and/or its affiliates
> >>>>  # This work is licensed under the terms of the GNU GPL, version 2 or later.
> >>>> -- 
> >>>> 2.14.3
> >>>>
> >>> --
> >>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> >>>
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2018-02-27 16:20 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-23 21:58 [Qemu-devel] [PATCH V5 0/4] tests: Add migration test for aarch64 Wei Huang
2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 1/4] rules: Move cross compilation auto detection functions to rules.mak Wei Huang
2018-02-26  9:07   ` Andrew Jones
2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 2/4] tests/migration: Convert the boot block compilation script into Makefile Wei Huang
2018-02-26  9:07   ` Andrew Jones
2018-02-26 18:01   ` Dr. David Alan Gilbert
2018-02-26 18:28     ` Wei Huang
2018-02-27 11:38       ` Dr. David Alan Gilbert
2018-02-27 15:21         ` Wei Huang
2018-02-27 16:19           ` Dr. David Alan Gilbert
2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 3/4] tests/migration: Add migration-test header file Wei Huang
2018-02-26  9:05   ` Andrew Jones
2018-02-26  9:38   ` Andrew Jones
2018-02-23 21:58 ` [Qemu-devel] [PATCH V5 4/4] tests: Add migration test for aarch64 Wei Huang
2018-02-26  9:30   ` Andrew Jones
2018-02-26  9:43     ` Andrew Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).