[PATCH 00/11] Run cross-compilation build tests in the gitlab-CI

[PATCH 00/11] Run cross-compilation build tests in the gitlab-CI

[Thomas Huth]

Now that we can use all our QEMU build containers in the gitlab-CI,
we can also run the cross-compilation jobs there. Of course, some
problems have to be fixed first, since apparently nobody was running
"make check-build" for QEMU for 32-bit big endian targets or MinGW
recently...

As a bonus, the last two patches also enable WHPX builds with our
debian-win64-cross container, so that we can compile-test this accelerator
code now, too.

Bruce Rogers (1):
  virtio-mem: Correct format specifier mismatch for RISC-V

Thomas Huth (10):
  target/riscv/vector_helper: Fix build on 32-bit big endian targets
  tests/Makefile: test-image-locking needs CONFIG_POSIX
  tests/Makefile: test-replication needs CONFIG_POSIX
  tests/Makefile: Only build usable targets during 'make check-build'
  tests/Makefile: Add $(EXESUF) to fp-test target
  Get rid of the libqemustub.a remainders
  stubs/notify-event: Mark qemu_notify_event() stub as "weak"
  gitlab-ci: Add cross-compiling build tests
  configure: Allow automatic WHPX detection
  dockerfiles/debian-win64-cross: Download WHPX MinGW headers

 .gitlab-ci.d/crossbuilds.yml                  | 113 ++++++++++++++++++
 .gitlab-ci.yml                                |   1 +
 MAINTAINERS                                   |   1 +
 Makefile                                      |   2 +-
 configure                                     |   1 +
 hw/virtio/virtio-mem.c                        |   2 +-
 scripts/coverity-scan/run-coverity-scan       |   3 -
 stubs/notify-event.c                          |   2 +-
 target/riscv/vector_helper.c                  |   4 +-
 tests/Makefile.include                        |  13 +-
 .../dockerfiles/debian-win64-cross.docker     |   9 +-
 tests/test-util-sockets.c                     |   3 +-
 12 files changed, 141 insertions(+), 13 deletions(-)
 create mode 100644 .gitlab-ci.d/crossbuilds.yml

-- 
2.18.1

[PATCH 01/11] virtio-mem: Correct format specifier mismatch for RISC-V

[Thomas Huth]

From: Bruce Rogers <brogers@suse.com>

This likely affects other, less popular host architectures as well.
Less common host architectures under linux get QEMU_VMALLOC_ALIGN (from
which VIRTIO_MEM_MIN_BLOCK_SIZE is derived) define to a variable of
type uintptr, which isn't compatible with the format specifier used to
print a user message. Since this particular usage of the underlying data
seems unique to this file, the simple fix is to just cast
QEMU_VMALLOC_ALIGN to uint32_t, which corresponds to the format specifier
used.

Signed-off-by: Bruce Rogers <brogers@suse.com>
Message-Id: <20200730130519.168475-1-brogers@suse.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/virtio/virtio-mem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index c12e9f79b0..7740fc613f 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -36,7 +36,7 @@
  * Use QEMU_VMALLOC_ALIGN, so no THP will have to be split when unplugging
  * memory (e.g., 2MB on x86_64).
  */
-#define VIRTIO_MEM_MIN_BLOCK_SIZE QEMU_VMALLOC_ALIGN
+#define VIRTIO_MEM_MIN_BLOCK_SIZE ((uint32_t)QEMU_VMALLOC_ALIGN)
 /*
  * Size the usable region bigger than the requested size if possible. Esp.
  * Linux guests will only add (aligned) memory blocks in case they fully
-- 
2.18.1

[PATCH 02/11] target/riscv/vector_helper: Fix build on 32-bit big endian targets

[Thomas Huth]

The code currently fails to compile on 32-bit big endian targets:

 target/riscv/vector_helper.c: In function 'vext_clear':
 target/riscv/vector_helper.c:154:16: error: cast to pointer from integer
 of different size [-Werror=int-to-pointer-cast]
         memset((void *)((uintptr_t)tail & ~(7ULL)), 0, part1);
                ^
 target/riscv/vector_helper.c:155:16: error: cast to pointer from integer
 of different size [-Werror=int-to-pointer-cast]
         memset((void *)(((uintptr_t)tail + 8) & ~(7ULL)), 0, part2);
                ^
 cc1: all warnings being treated as errors

We should not use "long long" (i.e. 64-bit) values here to avoid the
problem. Switch to our QEMU_ALIGN_PTR_DOWN/UP macros instead.

Fixes: 751538d5da ("add vector stride load and store instructions")
Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 target/riscv/vector_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 39f44d1029..793af99067 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -151,8 +151,8 @@ static void vext_clear(void *tail, uint32_t cnt, uint32_t tot)
     if (cnt % 8) {
         part1 = 8 - (cnt % 8);
         part2 = tot - cnt - part1;
-        memset((void *)((uintptr_t)tail & ~(7ULL)), 0, part1);
-        memset((void *)(((uintptr_t)tail + 8) & ~(7ULL)), 0, part2);
+        memset(QEMU_ALIGN_PTR_DOWN(tail, 8), 0, part1);
+        memset(QEMU_ALIGN_PTR_UP(tail, 8), 0, part2);
     } else {
         memset(tail, 0, part2);
     }
-- 
2.18.1

[PATCH 03/11] tests/Makefile: test-image-locking needs CONFIG_POSIX

[Thomas Huth]

test-image-locking.c uses the qemu_lock_fd_test() function which is
only available on POSIX-like systems.

Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index c7e4646ded..1e5ca3b585 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -87,7 +87,9 @@ check-unit-$(CONFIG_BLOCK) += tests/test-blockjob$(EXESUF)
 check-unit-$(CONFIG_BLOCK) += tests/test-blockjob-txn$(EXESUF)
 check-unit-$(CONFIG_BLOCK) += tests/test-block-backend$(EXESUF)
 check-unit-$(CONFIG_BLOCK) += tests/test-block-iothread$(EXESUF)
+ifeq ($(CONFIG_POSIX),y)
 check-unit-$(CONFIG_BLOCK) += tests/test-image-locking$(EXESUF)
+endif
 check-unit-y += tests/test-x86-cpuid$(EXESUF)
 # all code tested by test-x86-cpuid is inside topology.h
 ifeq ($(CONFIG_SOFTMMU),y)
-- 
2.18.1

[PATCH 04/11] tests/Makefile: test-replication needs CONFIG_POSIX

[Thomas Huth]

test-replication uses sigaction() and friends which are only available
on POSIX-like systems.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 1e5ca3b585..430119db74 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -155,11 +155,13 @@ check-unit-$(CONFIG_BLOCK)  += tests/test-crypto-afsplit$(EXESUF)
 check-unit-$(call land,$(CONFIG_BLOCK),$(CONFIG_QEMU_PRIVATE_XTS)) += tests/test-crypto-xts$(EXESUF)
 check-unit-$(CONFIG_BLOCK)  += tests/test-crypto-block$(EXESUF)
 check-unit-y += tests/test-logging$(EXESUF)
-check-unit-$(call land,$(CONFIG_BLOCK),$(CONFIG_REPLICATION)) += tests/test-replication$(EXESUF)
 check-unit-$(CONFIG_SOFTMMU) += tests/test-bufferiszero$(EXESUF)
 check-unit-y += tests/test-uuid$(EXESUF)
 check-unit-y += tests/ptimer-test$(EXESUF)
 check-unit-y += tests/test-qapi-util$(EXESUF)
+ifeq ($(CONFIG_BLOCK)$(CONFIG_REPLICATION)$(CONFIG_POSIX),yyy)
+check-unit-y += tests/test-replication$(EXESUF)
+endif
 
 check-block-$(call land,$(CONFIG_POSIX),$(CONFIG_SOFTMMU)) += tests/check-block.sh
 
-- 
2.18.1

[PATCH 05/11] tests/Makefile: Only build usable targets during 'make check-build'

[Thomas Huth]

If the softmmu targets are not enabled, "make check-build" fails to
build the qtests (which could not be run anyway without softmmu binary).
And the softfloat tests can not be compiled with MinGW yet (the macro
LITTLEENDIAN is re-defined in one of the system headers there, so this
needs some work first before it can be compiled).

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 430119db74..2806e062d0 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -965,7 +965,10 @@ check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
 ifeq ($(CONFIG_TOOLS),y)
 check-block: $(patsubst %,check-%, $(check-block-y))
 endif
-check-build: build-unit build-softfloat build-qtest
+check-build-y = build-unit
+check-build-$(CONFIG_POSIX) += build-softfloat
+check-build-$(CONFIG_SOFTMMU) += build-qtest
+check-build: $(check-build-y)
 
 check-clean:
 	rm -rf $(check-unit-y) tests/*.o tests/*/*.o $(QEMU_IOTESTS_HELPERS-y)
-- 
2.18.1

[PATCH 06/11] tests/Makefile: Add $(EXESUF) to fp-test target

[Thomas Huth]

In case we ever want to compile this for Windows, we need the $(EXESUF)
here.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 2806e062d0..e2532e12e2 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -687,7 +687,7 @@ check-report.tap: $(patsubst %,check-report-qtest-%.tap, $(QTEST_TARGETS)) check
 # generic Makefile expansions. Once we are cleanly passing all
 # the tests we can simplify the make syntax.
 
-FP_TEST_BIN=$(BUILD_DIR)/tests/fp/fp-test
+FP_TEST_BIN=$(BUILD_DIR)/tests/fp/fp-test$(EXESUF)
 
 # the build dir is created by configure
 $(FP_TEST_BIN): config-host.h $(test-util-obj-y)
-- 
2.18.1

[PATCH 07/11] Get rid of the libqemustub.a remainders

[Thomas Huth]

libqemustub.a has been removed in commit ebedb37c8d ("Makefile: Remove
libqemustub.a"). Some remainders have been missed. Remove them now.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 Makefile                                | 2 +-
 scripts/coverity-scan/run-coverity-scan | 3 ---
 tests/test-util-sockets.c               | 3 ++-
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index c2120d8d48..13dd708c4a 100644
--- a/Makefile
+++ b/Makefile
@@ -726,7 +726,7 @@ virtiofsd$(EXESUF): $(virtiofsd-obj-y) libvhost-user.a $(COMMON_LDADDS)
 	$(call LINK, $^)
 endif
 
-vhost-user-gpu$(EXESUF): $(vhost-user-gpu-obj-y) $(libvhost-user-obj-y) libqemuutil.a libqemustub.a
+vhost-user-gpu$(EXESUF): $(vhost-user-gpu-obj-y) $(libvhost-user-obj-y) libqemuutil.a
 	$(call LINK, $^)
 
 ifdef CONFIG_VHOST_USER_INPUT
diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan
index 03a791dec9..6eefb4b558 100755
--- a/scripts/coverity-scan/run-coverity-scan
+++ b/scripts/coverity-scan/run-coverity-scan
@@ -403,9 +403,6 @@ echo "Configuring..."
     --enable-mpath --enable-libxml2 --enable-glusterfs \
     --enable-virtfs --enable-zstd
 
-echo "Making libqemustub.a..."
-make libqemustub.a
-
 echo "Running cov-build..."
 rm -rf cov-int
 mkdir cov-int
diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c
index 2ca1e99f17..261dc48c03 100644
--- a/tests/test-util-sockets.c
+++ b/tests/test-util-sockets.c
@@ -64,7 +64,8 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
     return dup(mon_fd);
 }
 
-/* Syms in libqemustub.a are discarded at .o file granularity.
+/*
+ * Syms of stubs in libqemuutil.a are discarded at .o file granularity.
  * To replace monitor_get_fd() we must ensure everything in
  * stubs/monitor.c is defined, to make sure monitor.o is discarded
  * otherwise we get duplicate syms at link time.
-- 
2.18.1

[PATCH 08/11] stubs/notify-event: Mark qemu_notify_event() stub as "weak"

[Thomas Huth]

Otherwise there is a linker error with MinGW while compiling the tests:

  LINK    tests/test-timed-average.exe
 libqemuutil.a(main-loop.o): In function `qemu_notify_event':
 /builds/huth/qemu/util/main-loop.c:139: multiple definition of
  `qemu_notify_event'
 tests/test-timed-average.o:/builds/huth/qemu/tests/../stubs/notify-event.c:5:
  first defined here
 collect2: error: ld returned 1 exit status
 /builds/huth/qemu/rules.mak:124: recipe for target
  'tests/test-timed-average.exe' failed

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 stubs/notify-event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/stubs/notify-event.c b/stubs/notify-event.c
index 827bb52d1a..75d98f4a79 100644
--- a/stubs/notify-event.c
+++ b/stubs/notify-event.c
@@ -1,6 +1,6 @@
 #include "qemu/osdep.h"
 #include "qemu/main-loop.h"
 
-void qemu_notify_event(void)
+void __attribute__((weak)) qemu_notify_event(void)
 {
 }
-- 
2.18.1

[PATCH 09/11] gitlab-ci: Add cross-compiling build tests

[Thomas Huth]

Now that we can use all our QEMU test containers in the gitlab-CI, we can
easily add some jobs that test cross-compilation for various architectures.
There is just only small ugliness: Since the shared runners on gitlab.com
are single-threaded, we have to split each compilation job into two parts
(--disable-user and --disable-system), and exclude some additional targets,
to avoid that the jobs are running too long and hitting the timeout of 1 h.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .gitlab-ci.d/crossbuilds.yml | 113 +++++++++++++++++++++++++++++++++++
 .gitlab-ci.yml               |   1 +
 MAINTAINERS                  |   1 +
 3 files changed, 115 insertions(+)
 create mode 100644 .gitlab-ci.d/crossbuilds.yml

diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
new file mode 100644
index 0000000000..4ec7226b5c
--- /dev/null
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -0,0 +1,113 @@
+
+.cross_system_build_job_template: &cross_system_build_job_definition
+  stage: build
+  image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
+  script:
+    - mkdir build
+    - cd build
+    - PKG_CONFIG_PATH=$PKG_CONFIG_PATH
+      ../configure --enable-werror $QEMU_CONFIGURE_OPTS --disable-user
+        --target-list-exclude="aarch64-softmmu i386-softmmu microblaze-softmmu
+          mips-softmmu mipsel-softmmu mips64-softmmu ppc64-softmmu sh4-softmmu
+          xtensa-softmmu"
+    - make -j$(expr $(nproc) + 1) all check-build
+
+.cross_user_build_job_template: &cross_user_build_job_definition
+  stage: build
+  image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
+  script:
+    - mkdir build
+    - cd build
+    - PKG_CONFIG_PATH=$PKG_CONFIG_PATH
+      ../configure --enable-werror $QEMU_CONFIGURE_OPTS --disable-system
+    - make -j$(expr $(nproc) + 1) all check-build
+
+cross-armel-system:
+  <<: *cross_system_build_job_definition
+  variables:
+    IMAGE: debian-armel-cross
+
+cross-armel-user:
+  <<: *cross_user_build_job_definition
+  variables:
+    IMAGE: debian-armel-cross
+
+cross-armhf-system:
+  <<: *cross_system_build_job_definition
+  variables:
+    IMAGE: debian-armhf-cross
+
+cross-armhf-user:
+  <<: *cross_user_build_job_definition
+  variables:
+    IMAGE: debian-armhf-cross
+
+cross-arm64-system:
+  <<: *cross_system_build_job_definition
+  variables:
+    IMAGE: debian-arm64-cross
+
+cross-arm64-user:
+  <<: *cross_user_build_job_definition
+  variables:
+    IMAGE: debian-arm64-cross
+
+cross-mips-system:
+  <<: *cross_system_build_job_definition
+  variables:
+    IMAGE: debian-mips-cross
+
+cross-mips-user:
+  <<: *cross_user_build_job_definition
+  variables:
+    IMAGE: debian-mips-cross
+
+cross-mipsel-system:
+  <<: *cross_system_build_job_definition
+  variables:
+    IMAGE: debian-mipsel-cross
+
+cross-mipsel-user:
+  <<: *cross_user_build_job_definition
+  variables:
+    IMAGE: debian-mipsel-cross
+
+cross-mips64el-system:
+  <<: *cross_system_build_job_definition
+  variables:
+    IMAGE: debian-mips64el-cross
+
+cross-mips64el-user:
+  <<: *cross_user_build_job_definition
+  variables:
+    IMAGE: debian-mips64el-cross
+
+cross-ppc64el-system:
+  <<: *cross_system_build_job_definition
+  variables:
+    IMAGE: debian-ppc64el-cross
+
+cross-ppc64el-user:
+  <<: *cross_user_build_job_definition
+  variables:
+    IMAGE: debian-ppc64el-cross
+
+cross-s390x-system:
+  <<: *cross_system_build_job_definition
+  variables:
+    IMAGE: debian-s390x-cross
+
+cross-s390x-user:
+  <<: *cross_user_build_job_definition
+  variables:
+    IMAGE: debian-s390x-cross
+
+cross-win32-system:
+  <<: *cross_system_build_job_definition
+  variables:
+    IMAGE: debian-win32-cross
+
+cross-win64-system:
+  <<: *cross_system_build_job_definition
+  variables:
+    IMAGE: debian-win64-cross
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 362e5ee755..8f748237f8 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -18,6 +18,7 @@ include:
   - local: '/.gitlab-ci.d/edk2.yml'
   - local: '/.gitlab-ci.d/opensbi.yml'
   - local: '/.gitlab-ci.d/containers.yml'
+  - local: '/.gitlab-ci.d/crossbuilds.yml'
 
 .native_build_job_template: &native_build_job_definition
   stage: build
diff --git a/MAINTAINERS b/MAINTAINERS
index 0886eb3d2b..2731f7a594 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3067,6 +3067,7 @@ M: Alex Bennée <alex.bennee@linaro.org>
 R: Wainer dos Santos Moschetta <wainersm@redhat.com>
 S: Maintained
 F: .gitlab-ci.yml
+F: .gitlab-ci.d/crossbuilds.yml
 
 Guest Test Compilation Support
 M: Alex Bennée <alex.bennee@linaro.org>
-- 
2.18.1

[PATCH 10/11] configure: Allow automatic WHPX detection

[Thomas Huth]

The whpx variable is currently initialized to "no" which causes the WHPX
check to skip the detection unless the user specified --enable-whpx.
Since the detection code should be able to figure it out correctly, let's
initialized the variable to "" on MinGW-builds for proper auto-detection
instead.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 configure | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index 2acc4d1465..f44e428c91 100755
--- a/configure
+++ b/configure
@@ -809,6 +809,7 @@ case $targetos in
 MINGW32*)
   mingw32="yes"
   hax="yes"
+  whpx=""
   vhost_user="no"
   audio_possible_drivers="dsound sdl"
   if check_include dsound.h; then
-- 
2.18.1

[PATCH 11/11] dockerfiles/debian-win64-cross: Download WHPX MinGW headers

[Thomas Huth]

To compile-test the WHPX accelerator, we need to download these system
headers first (they are unfortunately not part of any released and
packaged MinGW toolchain yet).

Idea taken from another patch by Stefan Weil.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/docker/dockerfiles/debian-win64-cross.docker | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tests/docker/dockerfiles/debian-win64-cross.docker b/tests/docker/dockerfiles/debian-win64-cross.docker
index 2fc9cfcbc6..4cc4a3f365 100644
--- a/tests/docker/dockerfiles/debian-win64-cross.docker
+++ b/tests/docker/dockerfiles/debian-win64-cross.docker
@@ -32,7 +32,14 @@ RUN apt-get update && \
         mxe-$TARGET-w64-mingw32.shared-sdl2 \
         mxe-$TARGET-w64-mingw32.shared-sdl2-mixer \
         mxe-$TARGET-w64-mingw32.shared-sdl2-gfx \
-        mxe-$TARGET-w64-mingw32.shared-zlib
+        mxe-$TARGET-w64-mingw32.shared-zlib \
+        curl && \
+    curl -s -S -o /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/WinHvEmulation.h \
+        "https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvemulation.h?format=raw" && \
+    curl -s -S -o /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/WinHvPlatform.h \
+        "https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvplatform.h?format=raw" && \
+    curl -s -S -o /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/winhvplatformdefs.h \
+        "https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvplatformdefs.h?format=raw"
 
 # Specify the cross prefix for this image (see tests/docker/common.rc)
 ENV QEMU_CONFIGURE_OPTS --cross-prefix=x86_64-w64-mingw32.shared-
-- 
2.18.1

Re: [PATCH 00/11] Run cross-compilation build tests in the gitlab-CI

[Thomas Huth]

On 04/08/2020 19.00, Thomas Huth wrote:
> Now that we can use all our QEMU build containers in the gitlab-CI,
> we can also run the cross-compilation jobs there. Of course, some
> problems have to be fixed first, since apparently nobody was running
> "make check-build" for QEMU for 32-bit big endian targets or MinGW
> recently...
> 
> As a bonus, the last two patches also enable WHPX builds with our
> debian-win64-cross container, so that we can compile-test this accelerator
> code now, too.

I forgot to mention: A test run can be found here:

 https://gitlab.com/huth/qemu/-/pipelines/174203171

 Thomas

Re: [PATCH 02/11] target/riscv/vector_helper: Fix build on 32-bit big endian targets

[Richard Henderson]

On 8/4/20 10:00 AM, Thomas Huth wrote:
> The code currently fails to compile on 32-bit big endian targets:
> 
>  target/riscv/vector_helper.c: In function 'vext_clear':
>  target/riscv/vector_helper.c:154:16: error: cast to pointer from integer
>  of different size [-Werror=int-to-pointer-cast]
>          memset((void *)((uintptr_t)tail & ~(7ULL)), 0, part1);
>                 ^
>  target/riscv/vector_helper.c:155:16: error: cast to pointer from integer
>  of different size [-Werror=int-to-pointer-cast]
>          memset((void *)(((uintptr_t)tail + 8) & ~(7ULL)), 0, part2);
>                 ^
>  cc1: all warnings being treated as errors
> 
> We should not use "long long" (i.e. 64-bit) values here to avoid the
> problem. Switch to our QEMU_ALIGN_PTR_DOWN/UP macros instead.
> 
> Fixes: 751538d5da ("add vector stride load and store instructions")
> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  target/riscv/vector_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 06/11] tests/Makefile: Add $(EXESUF) to fp-test target

[Richard Henderson]

On 8/4/20 10:00 AM, Thomas Huth wrote:
> In case we ever want to compile this for Windows, we need the $(EXESUF)
> here.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/Makefile.include | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 07/11] Get rid of the libqemustub.a remainders

[Richard Henderson]

On 8/4/20 10:00 AM, Thomas Huth wrote:
> libqemustub.a has been removed in commit ebedb37c8d ("Makefile: Remove
> libqemustub.a"). Some remainders have been missed. Remove them now.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  Makefile                                | 2 +-
>  scripts/coverity-scan/run-coverity-scan | 3 ---
>  tests/test-util-sockets.c               | 3 ++-
>  3 files changed, 3 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 02/11] target/riscv/vector_helper: Fix build on 32-bit big endian targets

[Philippe Mathieu-Daudé]

On 8/4/20 7:00 PM, Thomas Huth wrote:
> The code currently fails to compile on 32-bit big endian targets:

s/target/host/ here and in subject?

> 
>  target/riscv/vector_helper.c: In function 'vext_clear':
>  target/riscv/vector_helper.c:154:16: error: cast to pointer from integer
>  of different size [-Werror=int-to-pointer-cast]
>          memset((void *)((uintptr_t)tail & ~(7ULL)), 0, part1);
>                 ^
>  target/riscv/vector_helper.c:155:16: error: cast to pointer from integer
>  of different size [-Werror=int-to-pointer-cast]
>          memset((void *)(((uintptr_t)tail + 8) & ~(7ULL)), 0, part2);
>                 ^
>  cc1: all warnings being treated as errors
> 
> We should not use "long long" (i.e. 64-bit) values here to avoid the
> problem. Switch to our QEMU_ALIGN_PTR_DOWN/UP macros instead.
> 
> Fixes: 751538d5da ("add vector stride load and store instructions")
> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  target/riscv/vector_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 39f44d1029..793af99067 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -151,8 +151,8 @@ static void vext_clear(void *tail, uint32_t cnt, uint32_t tot)
>      if (cnt % 8) {
>          part1 = 8 - (cnt % 8);
>          part2 = tot - cnt - part1;
> -        memset((void *)((uintptr_t)tail & ~(7ULL)), 0, part1);
> -        memset((void *)(((uintptr_t)tail + 8) & ~(7ULL)), 0, part2);
> +        memset(QEMU_ALIGN_PTR_DOWN(tail, 8), 0, part1);
> +        memset(QEMU_ALIGN_PTR_UP(tail, 8), 0, part2);
>      } else {
>          memset(tail, 0, part2);
>      }
> 

Re: [PATCH 10/11] configure: Allow automatic WHPX detection

[Philippe Mathieu-Daudé]

On 8/4/20 7:00 PM, Thomas Huth wrote:
> The whpx variable is currently initialized to "no" which causes the WHPX
> check to skip the detection unless the user specified --enable-whpx.
> Since the detection code should be able to figure it out correctly, let's
> initialized the variable to "" on MinGW-builds for proper auto-detection
> instead.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  configure | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/configure b/configure
> index 2acc4d1465..f44e428c91 100755
> --- a/configure
> +++ b/configure
> @@ -809,6 +809,7 @@ case $targetos in
>  MINGW32*)
>    mingw32="yes"
>    hax="yes"
> +  whpx=""
>    vhost_user="no"
>    audio_possible_drivers="dsound sdl"
>    if check_include dsound.h; then
> 

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 08/11] stubs/notify-event: Mark qemu_notify_event() stub as "weak"

[Richard Henderson]

On 8/4/20 10:00 AM, Thomas Huth wrote:
> Otherwise there is a linker error with MinGW while compiling the tests:
> 
>   LINK    tests/test-timed-average.exe
>  libqemuutil.a(main-loop.o): In function `qemu_notify_event':
>  /builds/huth/qemu/util/main-loop.c:139: multiple definition of
>   `qemu_notify_event'
>  tests/test-timed-average.o:/builds/huth/qemu/tests/../stubs/notify-event.c:5:
>   first defined here
>  collect2: error: ld returned 1 exit status
>  /builds/huth/qemu/rules.mak:124: recipe for target
>   'tests/test-timed-average.exe' failed
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  stubs/notify-event.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

That doesn't make sense.  Since the symbol is satisfied from main-loop.c, it
should not be pulled in from libqemuutil.a.

What's really happening here?


r~

Re: [PATCH 06/11] tests/Makefile: Add $(EXESUF) to fp-test target

[Philippe Mathieu-Daudé]

On 8/4/20 7:00 PM, Thomas Huth wrote:
> In case we ever want to compile this for Windows, we need the $(EXESUF)
> here.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/Makefile.include | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 2806e062d0..e2532e12e2 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -687,7 +687,7 @@ check-report.tap: $(patsubst %,check-report-qtest-%.tap, $(QTEST_TARGETS)) check
>  # generic Makefile expansions. Once we are cleanly passing all
>  # the tests we can simplify the make syntax.
>  
> -FP_TEST_BIN=$(BUILD_DIR)/tests/fp/fp-test
> +FP_TEST_BIN=$(BUILD_DIR)/tests/fp/fp-test$(EXESUF)
>  
>  # the build dir is created by configure
>  $(FP_TEST_BIN): config-host.h $(test-util-obj-y)
> 

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 11/11] dockerfiles/debian-win64-cross: Download WHPX MinGW headers

[Philippe Mathieu-Daudé]

On 8/4/20 7:00 PM, Thomas Huth wrote:
> To compile-test the WHPX accelerator, we need to download these system
> headers first (they are unfortunately not part of any released and
> packaged MinGW toolchain yet).
> 
> Idea taken from another patch by Stefan Weil.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/docker/dockerfiles/debian-win64-cross.docker | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/docker/dockerfiles/debian-win64-cross.docker b/tests/docker/dockerfiles/debian-win64-cross.docker
> index 2fc9cfcbc6..4cc4a3f365 100644
> --- a/tests/docker/dockerfiles/debian-win64-cross.docker
> +++ b/tests/docker/dockerfiles/debian-win64-cross.docker
> @@ -32,7 +32,14 @@ RUN apt-get update && \
>          mxe-$TARGET-w64-mingw32.shared-sdl2 \
>          mxe-$TARGET-w64-mingw32.shared-sdl2-mixer \
>          mxe-$TARGET-w64-mingw32.shared-sdl2-gfx \
> -        mxe-$TARGET-w64-mingw32.shared-zlib
> +        mxe-$TARGET-w64-mingw32.shared-zlib \
> +        curl && \
> +    curl -s -S -o /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/WinHvEmulation.h \
> +        "https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvemulation.h?format=raw" && \
> +    curl -s -S -o /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/WinHvPlatform.h \
> +        "https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvplatform.h?format=raw" && \
> +    curl -s -S -o /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/winhvplatformdefs.h \
> +        "https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvplatformdefs.h?format=raw"

Seems legally safer than my older approach =)
https://www.mail-archive.com/qemu-devel@nongnu.org/msg645794.html

>  
>  # Specify the cross prefix for this image (see tests/docker/common.rc)
>  ENV QEMU_CONFIGURE_OPTS --cross-prefix=x86_64-w64-mingw32.shared-
> 

Re: [PATCH 08/11] stubs/notify-event: Mark qemu_notify_event() stub as "weak"

[Thomas Huth]

On 04/08/2020 19.50, Richard Henderson wrote:
> On 8/4/20 10:00 AM, Thomas Huth wrote:
>> Otherwise there is a linker error with MinGW while compiling the tests:
>>
>>   LINK    tests/test-timed-average.exe
>>  libqemuutil.a(main-loop.o): In function `qemu_notify_event':
>>  /builds/huth/qemu/util/main-loop.c:139: multiple definition of
>>   `qemu_notify_event'
>>  tests/test-timed-average.o:/builds/huth/qemu/tests/../stubs/notify-event.c:5:
>>   first defined here
>>  collect2: error: ld returned 1 exit status
>>  /builds/huth/qemu/rules.mak:124: recipe for target
>>   'tests/test-timed-average.exe' failed
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  stubs/notify-event.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> That doesn't make sense.  Since the symbol is satisfied from main-loop.c, it
> should not be pulled in from libqemuutil.a.
> 
> What's really happening here?

Honestly, I don't have a clue. But since commit ebedb37c8d2aa4775, both
the code from util/ and from stubs/ are put into the same library,
libqemuutil.a, which is causing the trouble here, I guess.
Maybe the linker pulled in the code from the stub first, then some other
part used another function from  util/main-loop.c which caused the
linker to pull in main-loop.o, too, so that it finally found that there
is a clash? ... but that's just a plain guess, of course. Paolo (as
author of commit ebedb37c8d2), do you have an idea what might be going
on here?

 Thomas

Re: [PATCH 10/11] configure: Allow automatic WHPX detection

[Stefan Weil]

Am 04.08.20 um 19:00 schrieb Thomas Huth:

> The whpx variable is currently initialized to "no" which causes the WHPX
> check to skip the detection unless the user specified --enable-whpx.
> Since the detection code should be able to figure it out correctly, let's
> initialized the variable to "" on MinGW-builds for proper auto-detection
> instead.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  configure | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/configure b/configure
> index 2acc4d1465..f44e428c91 100755
> --- a/configure
> +++ b/configure
> @@ -809,6 +809,7 @@ case $targetos in
>  MINGW32*)
>    mingw32="yes"
>    hax="yes"
> +  whpx=""
>    vhost_user="no"
>    audio_possible_drivers="dsound sdl"
>    if check_include dsound.h; then


Reviewed-by: Stefan Weil <sw@weilnetz.de>

Re: [PATCH 11/11] dockerfiles/debian-win64-cross: Download WHPX MinGW headers

[Stefan Weil]

Am 04.08.20 um 19:00 schrieb Thomas Huth:

> To compile-test the WHPX accelerator, we need to download these system
> headers first (they are unfortunately not part of any released and
> packaged MinGW toolchain yet).
>
> Idea taken from another patch by Stefan Weil.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/docker/dockerfiles/debian-win64-cross.docker | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/tests/docker/dockerfiles/debian-win64-cross.docker b/tests/docker/dockerfiles/debian-win64-cross.docker
> index 2fc9cfcbc6..4cc4a3f365 100644
> --- a/tests/docker/dockerfiles/debian-win64-cross.docker
> +++ b/tests/docker/dockerfiles/debian-win64-cross.docker
> @@ -32,7 +32,14 @@ RUN apt-get update && \
>          mxe-$TARGET-w64-mingw32.shared-sdl2 \
>          mxe-$TARGET-w64-mingw32.shared-sdl2-mixer \
>          mxe-$TARGET-w64-mingw32.shared-sdl2-gfx \
> -        mxe-$TARGET-w64-mingw32.shared-zlib
> +        mxe-$TARGET-w64-mingw32.shared-zlib \
> +        curl && \
> +    curl -s -S -o /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/WinHvEmulation.h \
> +        "https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvemulation.h?format=raw" && \
> +    curl -s -S -o /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/WinHvPlatform.h \
> +        "https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvplatform.h?format=raw" && \
> +    curl -s -S -o /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/winhvplatformdefs.h \
> +        "https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvplatformdefs.h?format=raw"
>  
>  # Specify the cross prefix for this image (see tests/docker/common.rc)
>  ENV QEMU_CONFIGURE_OPTS --cross-prefix=x86_64-w64-mingw32.shared-


I expect a build failure: Mingw-w64 decided to use lower case filenames,
and those header files include each other.

We need both lower case filenames (for Mingw-w64) and camel case
filenames (for QEMU). That's why I used additional symlinks.

Regards

Stefan

Re: [PATCH 11/11] dockerfiles/debian-win64-cross: Download WHPX MinGW headers

[Thomas Huth]

On 04/08/2020 20.32, Stefan Weil wrote:
> Am 04.08.20 um 19:00 schrieb Thomas Huth:
> 
>> To compile-test the WHPX accelerator, we need to download these system
>> headers first (they are unfortunately not part of any released and
>> packaged MinGW toolchain yet).
>>
>> Idea taken from another patch by Stefan Weil.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  tests/docker/dockerfiles/debian-win64-cross.docker | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/docker/dockerfiles/debian-win64-cross.docker b/tests/docker/dockerfiles/debian-win64-cross.docker
>> index 2fc9cfcbc6..4cc4a3f365 100644
>> --- a/tests/docker/dockerfiles/debian-win64-cross.docker
>> +++ b/tests/docker/dockerfiles/debian-win64-cross.docker
>> @@ -32,7 +32,14 @@ RUN apt-get update && \
>>          mxe-$TARGET-w64-mingw32.shared-sdl2 \
>>          mxe-$TARGET-w64-mingw32.shared-sdl2-mixer \
>>          mxe-$TARGET-w64-mingw32.shared-sdl2-gfx \
>> -        mxe-$TARGET-w64-mingw32.shared-zlib
>> +        mxe-$TARGET-w64-mingw32.shared-zlib \
>> +        curl && \
>> +    curl -s -S -o /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/WinHvEmulation.h \
>> +        "https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvemulation.h?format=raw" && \
>> +    curl -s -S -o /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/WinHvPlatform.h \
>> +        "https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvplatform.h?format=raw" && \
>> +    curl -s -S -o /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/winhvplatformdefs.h \
>> +        "https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvplatformdefs.h?format=raw"
>>  
>>  # Specify the cross prefix for this image (see tests/docker/common.rc)
>>  ENV QEMU_CONFIGURE_OPTS --cross-prefix=x86_64-w64-mingw32.shared-
> 
> 
> I expect a build failure: Mingw-w64 decided to use lower case filenames,
> and those header files include each other.

The first two headers include the third header, that's why I've used
lowercase for the third header (which is apparently not included by QEMU
directly).
But yes, all that CamelCase vs. lower-case stuff is very annoying. I
think once there is a new MinGW release available in the major Linux
distros that ships with these headers, we should change the QEMU source
code to always use the lowercase headers, too.

 Thomas

Re: [PATCH 02/11] target/riscv/vector_helper: Fix build on 32-bit big endian targets

[Thomas Huth]

On 04/08/2020 19.46, Philippe Mathieu-Daudé wrote:
> On 8/4/20 7:00 PM, Thomas Huth wrote:
>> The code currently fails to compile on 32-bit big endian targets:
> 
> s/target/host/ here and in subject?

Yes, "host" is better, I'll change that.

 Thanks,
  Thomas

Re: [PATCH 11/11] dockerfiles/debian-win64-cross: Download WHPX MinGW headers

[Daniel P. Berrangé]

On Tue, Aug 04, 2020 at 08:32:18PM +0200, Stefan Weil wrote:
> Am 04.08.20 um 19:00 schrieb Thomas Huth:
> 
> > To compile-test the WHPX accelerator, we need to download these system
> > headers first (they are unfortunately not part of any released and
> > packaged MinGW toolchain yet).
> >
> > Idea taken from another patch by Stefan Weil.
> >
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >  tests/docker/dockerfiles/debian-win64-cross.docker | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/docker/dockerfiles/debian-win64-cross.docker b/tests/docker/dockerfiles/debian-win64-cross.docker
> > index 2fc9cfcbc6..4cc4a3f365 100644
> > --- a/tests/docker/dockerfiles/debian-win64-cross.docker
> > +++ b/tests/docker/dockerfiles/debian-win64-cross.docker
> > @@ -32,7 +32,14 @@ RUN apt-get update && \
> >          mxe-$TARGET-w64-mingw32.shared-sdl2 \
> >          mxe-$TARGET-w64-mingw32.shared-sdl2-mixer \
> >          mxe-$TARGET-w64-mingw32.shared-sdl2-gfx \
> > -        mxe-$TARGET-w64-mingw32.shared-zlib
> > +        mxe-$TARGET-w64-mingw32.shared-zlib \
> > +        curl && \
> > +    curl -s -S -o /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/WinHvEmulation.h \
> > +        "https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvemulation.h?format=raw" && \
> > +    curl -s -S -o /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/WinHvPlatform.h \
> > +        "https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvplatform.h?format=raw" && \
> > +    curl -s -S -o /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/winhvplatformdefs.h \
> > +        "https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvplatformdefs.h?format=raw"
> >  
> >  # Specify the cross prefix for this image (see tests/docker/common.rc)
> >  ENV QEMU_CONFIGURE_OPTS --cross-prefix=x86_64-w64-mingw32.shared-
> 
> 
> I expect a build failure: Mingw-w64 decided to use lower case filenames,
> and those header files include each other.
> 
> We need both lower case filenames (for Mingw-w64) and camel case
> filenames (for QEMU). That's why I used additional symlinks.

Hmm, this feels wrong. Either QEMU or Mingw-w64 is broken here.

Mingw-w64 needs to match whatever upper/lower case scheme that Windows
has used for these headers. QEMU also needs to match whatever Windows
used.

Mingw originally commited the files with mixed case, and then later
renamed then to lower case, but the commit doing the rename gives
zero information about why it did that.

The microsoft docs appear to show that camel case is required:

  https://docs.microsoft.com/en-us/virtualization/api/hypervisor-platform/hypervisor-platform

so I think mingw just needs fixing to revert that broken commit
the changed to lowercase.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 11/11] dockerfiles/debian-win64-cross: Download WHPX MinGW headers

[罗勇刚(Yonggang Luo)]

[-- Attachment #1: Type: text/plain, Size: 3664 bytes --]

Fixes in mingw seems not possible and would broken lots of things, the
better way may change all include of Windows API headers to lowercase,


On Wed, Aug 5, 2020 at 5:12 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Tue, Aug 04, 2020 at 08:32:18PM +0200, Stefan Weil wrote:
> > Am 04.08.20 um 19:00 schrieb Thomas Huth:
> >
> > > To compile-test the WHPX accelerator, we need to download these system
> > > headers first (they are unfortunately not part of any released and
> > > packaged MinGW toolchain yet).
> > >
> > > Idea taken from another patch by Stefan Weil.
> > >
> > > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > > ---
> > >  tests/docker/dockerfiles/debian-win64-cross.docker | 9 ++++++++-
> > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tests/docker/dockerfiles/debian-win64-cross.docker
> b/tests/docker/dockerfiles/debian-win64-cross.docker
> > > index 2fc9cfcbc6..4cc4a3f365 100644
> > > --- a/tests/docker/dockerfiles/debian-win64-cross.docker
> > > +++ b/tests/docker/dockerfiles/debian-win64-cross.docker
> > > @@ -32,7 +32,14 @@ RUN apt-get update && \
> > >          mxe-$TARGET-w64-mingw32.shared-sdl2 \
> > >          mxe-$TARGET-w64-mingw32.shared-sdl2-mixer \
> > >          mxe-$TARGET-w64-mingw32.shared-sdl2-gfx \
> > > -        mxe-$TARGET-w64-mingw32.shared-zlib
> > > +        mxe-$TARGET-w64-mingw32.shared-zlib \
> > > +        curl && \
> > > +    curl -s -S -o
> /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/WinHvEmulation.h \
> > > +        "
> https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvemulation.h?format=raw"
> && \
> > > +    curl -s -S -o
> /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/WinHvPlatform.h \
> > > +        "
> https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvplatform.h?format=raw"
> && \
> > > +    curl -s -S -o
> /usr/lib/mxe/usr/x86_64-w64-mingw32.shared/include/winhvplatformdefs.h \
> > > +        "
> https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/include/winhvplatformdefs.h?format=raw
> "
> > >
> > >  # Specify the cross prefix for this image (see tests/docker/common.rc)
> > >  ENV QEMU_CONFIGURE_OPTS --cross-prefix=x86_64-w64-mingw32.shared-
> >
> >
> > I expect a build failure: Mingw-w64 decided to use lower case filenames,
> > and those header files include each other.
> >
> > We need both lower case filenames (for Mingw-w64) and camel case
> > filenames (for QEMU). That's why I used additional symlinks.
>
> Hmm, this feels wrong. Either QEMU or Mingw-w64 is broken here.
>
> Mingw-w64 needs to match whatever upper/lower case scheme that Windows
> has used for these headers. QEMU also needs to match whatever Windows
> used.
>
> Mingw originally commited the files with mixed case, and then later
> renamed then to lower case, but the commit doing the rename gives
> zero information about why it did that.
>
> The microsoft docs appear to show that camel case is required:
>
>
> https://docs.microsoft.com/en-us/virtualization/api/hypervisor-platform/hypervisor-platform
>
> so I think mingw just needs fixing to revert that broken commit
> the changed to lowercase.
>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-
> https://www.instagram.com/dberrange :|
>
>
>

-- 
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

[-- Attachment #2: Type: text/html, Size: 5658 bytes --]

Re: [PATCH 08/11] stubs/notify-event: Mark qemu_notify_event() stub as "weak"

[Paolo Bonzini]

On 04/08/20 20:20, Thomas Huth wrote:
> On 04/08/2020 19.50, Richard Henderson wrote:
>> On 8/4/20 10:00 AM, Thomas Huth wrote:
>>> Otherwise there is a linker error with MinGW while compiling the tests:
>>>
>>>   LINK    tests/test-timed-average.exe
>>>  libqemuutil.a(main-loop.o): In function `qemu_notify_event':
>>>  /builds/huth/qemu/util/main-loop.c:139: multiple definition of
>>>   `qemu_notify_event'
>>>  tests/test-timed-average.o:/builds/huth/qemu/tests/../stubs/notify-event.c:5:
>>>   first defined here
>>>  collect2: error: ld returned 1 exit status
>>>  /builds/huth/qemu/rules.mak:124: recipe for target
>>>   'tests/test-timed-average.exe' failed
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>  stubs/notify-event.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> That doesn't make sense.  Since the symbol is satisfied from main-loop.c, it
>> should not be pulled in from libqemuutil.a.
>>
>> What's really happening here?
> 
> Honestly, I don't have a clue. But since commit ebedb37c8d2aa4775, both
> the code from util/ and from stubs/ are put into the same library,
> libqemuutil.a, which is causing the trouble here, I guess.
> Maybe the linker pulled in the code from the stub first, then some other
> part used another function from  util/main-loop.c which caused the
> linker to pull in main-loop.o, too, so that it finally found that there
> is a clash? ... but that's just a plain guess, of course. Paolo (as
> author of commit ebedb37c8d2), do you have an idea what might be going
> on here?

I think your analysis is right but is the stub needed at all, since te
linker can include util/main-loop.c instead?

Paolo

Re: [PATCH 06/11] tests/Makefile: Add $(EXESUF) to fp-test target

[Paolo Bonzini]

On 04/08/20 19:00, Thomas Huth wrote:
> In case we ever want to compile this for Windows, we need the $(EXESUF)
> here.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/Makefile.include | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 2806e062d0..e2532e12e2 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -687,7 +687,7 @@ check-report.tap: $(patsubst %,check-report-qtest-%.tap, $(QTEST_TARGETS)) check
>  # generic Makefile expansions. Once we are cleanly passing all
>  # the tests we can simplify the make syntax.
>  
> -FP_TEST_BIN=$(BUILD_DIR)/tests/fp/fp-test
> +FP_TEST_BIN=$(BUILD_DIR)/tests/fp/fp-test$(EXESUF)
>  
>  # the build dir is created by configure
>  $(FP_TEST_BIN): config-host.h $(test-util-obj-y)
> 

When converting it to Meson, fptest broke badly for me on Windows due to
namespace pollution (Windows headers use FLOAT128 and LITTLEENDIAN
symbols).  I would just leave out this patch since check-softfloat has
been converted to meson already.

Paolo