[PATCH v1 00/11] testing and plugin pre-PR (configure, gitlab, plugins)

[PATCH v1 00/11] testing and plugin pre-PR (configure, gitlab, plugins)

[Alex Bennée]

Hi,

This is mostly a collection of updates for testing plus a few minor
plugin tweaks that didn't seem worth stuffing in a separate series. I
think most of this is ready so I'll likely cut a PR from it next week.

The following still need review:

 - tests/tcg: move some multiarch files and make conditional
 - configure: don't override the selected host test compiler if defined

Alex Bennée (5):
  configure: don't override the selected host test compiler if defined
  tests/tcg/sha1: remove endian include
  tests/tcg: move some multiarch files and make conditional
  tests/docker: promote debian-riscv64-cross to a full image
  accel/tcg: re-factor plugin_inject_cb so we can assert insn_idx is
    valid

Daniel P. Berrangé (2):
  gitlab: skip the check-patch job on the upstream repo
  gitlab: fix passing of TEST_TARGETS env to cirrus

John Snow (1):
  python: Update for pylint 2.10

Lukas Jünger (1):
  plugins/: Add missing functions to symbol list

Richard Henderson (1):
  gitlab: Add cross-riscv64-system, cross-riscv64-user

Thomas Huth (1):
  travis.yml: Remove the "Release tarball" job

 configure                                     |   7 +-
 include/tcg/tcg.h                             |   6 -
 accel/tcg/plugin-gen.c                        | 157 ++++++++++--------
 .../tcg/multiarch/{ => libs}/float_helpers.c  |   2 +-
 tests/tcg/multiarch/{ => linux}/linux-test.c  |   0
 tests/tcg/multiarch/sha1.c                    |   1 -
 .gitlab-ci.d/cirrus.yml                       |   2 +-
 .gitlab-ci.d/cirrus/build.yml                 |   1 +
 .gitlab-ci.d/container-cross.yml              |   3 +-
 .gitlab-ci.d/crossbuilds.yml                  |  19 +++
 .gitlab-ci.d/static_checks.yml                |   2 +-
 .travis.yml                                   |  23 ---
 plugins/qemu-plugins.symbols                  |   3 +
 python/qemu/machine/machine.py                |   9 +-
 python/setup.cfg                              |   1 +
 tests/docker/Makefile.include                 |   2 -
 .../dockerfiles/debian-riscv64-cross.docker   |  46 ++++-
 tests/tcg/multiarch/Makefile.target           |  15 +-
 tests/tcg/x86_64/Makefile.target              |   4 +
 19 files changed, 182 insertions(+), 121 deletions(-)
 rename tests/tcg/multiarch/{ => libs}/float_helpers.c (99%)
 rename tests/tcg/multiarch/{ => linux}/linux-test.c (100%)

-- 
2.30.2

[PATCH v1 01/11] configure: don't override the selected host test compiler if defined

[Alex Bennée]

There are not many cases you would want to do this but one is if you
want to use a test friendly compiler like gcc instead of a system
compiler like clang. Either way we should honour the users choice if
they have made it.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Warner Losh <imp@bsdimp.com>
---
 configure | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 1043ccce4f..049c50a933 100755
--- a/configure
+++ b/configure
@@ -1686,8 +1686,11 @@ case "$cpu" in
     # No special flags required for other host CPUs
 esac
 
-eval "cross_cc_${cpu}=\$cc"
-cross_cc_vars="$cross_cc_vars cross_cc_${cpu}"
+if eval test -z "\${cross_cc_$cpu}"; then
+    eval "cross_cc_${cpu}=\$cc"
+    cross_cc_vars="$cross_cc_vars cross_cc_${cpu}"
+fi
+
 QEMU_CFLAGS="$CPU_CFLAGS $QEMU_CFLAGS"
 
 # For user-mode emulation the host arch has to be one we explicitly
-- 
2.30.2

[PATCH v1 02/11] tests/tcg/sha1: remove endian include

[Alex Bennée]

This doesn't exist in BSD world and doesn't seem to be needed by
either.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Warner Losh <imp@bsdimp.com>
---
 tests/tcg/multiarch/sha1.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/tcg/multiarch/sha1.c b/tests/tcg/multiarch/sha1.c
index 87bfbcdf52..0081bd7657 100644
--- a/tests/tcg/multiarch/sha1.c
+++ b/tests/tcg/multiarch/sha1.c
@@ -43,7 +43,6 @@ void SHA1Init(SHA1_CTX* context);
 void SHA1Update(SHA1_CTX* context, const unsigned char* data, uint32_t len);
 void SHA1Final(unsigned char digest[20], SHA1_CTX* context);
 /* ================ end of sha1.h ================ */
-#include <endian.h>
 
 #define rol(value, bits) (((value) << (bits)) | ((value) >> (32 - (bits))))
 
-- 
2.30.2

[PATCH v1 03/11] tests/tcg: move some multiarch files and make conditional

[Alex Bennée]

We had some messy code to filter out stuff we can't build. Lets junk
that and simplify the logic by pushing some stuff into subdirs. In
particular we move:

  float_helpers into libs - not a standalone test
  linux-test into linux - so we only build on Linux hosts

This allows for at least some of the tests to be nominally usable
by *BSD user builds.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Warner Losh <imp@bsdimp.com>
---
 tests/tcg/multiarch/{ => libs}/float_helpers.c |  2 +-
 tests/tcg/multiarch/{ => linux}/linux-test.c   |  0
 tests/tcg/multiarch/Makefile.target            | 15 ++++++++++-----
 tests/tcg/x86_64/Makefile.target               |  4 ++++
 4 files changed, 15 insertions(+), 6 deletions(-)
 rename tests/tcg/multiarch/{ => libs}/float_helpers.c (99%)
 rename tests/tcg/multiarch/{ => linux}/linux-test.c (100%)

diff --git a/tests/tcg/multiarch/float_helpers.c b/tests/tcg/multiarch/libs/float_helpers.c
similarity index 99%
rename from tests/tcg/multiarch/float_helpers.c
rename to tests/tcg/multiarch/libs/float_helpers.c
index bc530e5732..4e68d2b659 100644
--- a/tests/tcg/multiarch/float_helpers.c
+++ b/tests/tcg/multiarch/libs/float_helpers.c
@@ -22,7 +22,7 @@
 #include <float.h>
 #include <fenv.h>
 
-#include "float_helpers.h"
+#include "../float_helpers.h"
 
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 
diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux/linux-test.c
similarity index 100%
rename from tests/tcg/multiarch/linux-test.c
rename to tests/tcg/multiarch/linux/linux-test.c
diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
index 85a6fb7a2e..3763df2019 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -8,18 +8,23 @@
 MULTIARCH_SRC=$(SRC_PATH)/tests/tcg/multiarch
 
 # Set search path for all sources
-VPATH 		+= $(MULTIARCH_SRC)
-MULTIARCH_SRCS   =$(notdir $(wildcard $(MULTIARCH_SRC)/*.c))
-MULTIARCH_TESTS  =$(filter-out float_helpers, $(MULTIARCH_SRCS:.c=))
+VPATH 	       += $(MULTIARCH_SRC)
+MULTIARCH_SRCS =  $(notdir $(wildcard $(MULTIARCH_SRC)/*.c))
+ifneq ($(CONFIG_LINUX),)
+VPATH 	       += $(MULTIARCH_SRC)/linux
+MULTIARCH_SRCS += $(notdir $(wildcard $(MULTIARCH_SRC)/linux/*.c))
+endif
+MULTIARCH_TESTS = $(MULTIARCH_SRCS:.c=)
 
+$(info SRCS=${MULTIARCH_SRCS} and ${MULTIARCH_TESTS})
 #
 # The following are any additional rules needed to build things
 #
 
 
 float_%: LDFLAGS+=-lm
-float_%: float_%.c float_helpers.c
-	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< $(MULTIARCH_SRC)/float_helpers.c -o $@ $(LDFLAGS)
+float_%: float_%.c libs/float_helpers.c
+	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< $(MULTIARCH_SRC)/libs/float_helpers.c -o $@ $(LDFLAGS)
 
 run-float_%: float_%
 	$(call run-test,$<, $(QEMU) $(QEMU_OPTS) $<,"$< on $(TARGET_NAME)")
diff --git a/tests/tcg/x86_64/Makefile.target b/tests/tcg/x86_64/Makefile.target
index 2151ea6302..d7a7385583 100644
--- a/tests/tcg/x86_64/Makefile.target
+++ b/tests/tcg/x86_64/Makefile.target
@@ -8,8 +8,12 @@
 
 include $(SRC_PATH)/tests/tcg/i386/Makefile.target
 
+ifneq ($(CONFIG_LINUX),)
 X86_64_TESTS += vsyscall
 TESTS=$(MULTIARCH_TESTS) $(X86_64_TESTS) test-x86_64
+else
+TESTS=$(MULTIARCH_TESTS)
+endif
 QEMU_OPTS += -cpu max
 
 test-x86_64: LDFLAGS+=-lm -lc
-- 
2.30.2

[PATCH v1 04/11] tests/docker: promote debian-riscv64-cross to a full image

[Alex Bennée]

To be able to cross build QEMU itself we need to include a few more
libraries. These are only available in Debian's unstable ports repo
for now so we need to base the riscv64 image on sid with the the
minimal libs needed to build QEMU (glib/pixman).

The result works but is not as clean as using build-dep to bring in
more dependencies. However sid is by definition a shifting pile of
sand and by keeping the list of libs minimal we reduce the chance of
having an image we can't build. It's good enough for a basic cross
build testing of TCG.

Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20210914185830.1378771-2-richard.henderson@linaro.org>
[AJB: tweak allow_failure]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v2
  - allow_failure: true
---
 .gitlab-ci.d/container-cross.yml              |  3 +-
 tests/docker/Makefile.include                 |  2 -
 .../dockerfiles/debian-riscv64-cross.docker   | 46 +++++++++++++++++--
 3 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/.gitlab-ci.d/container-cross.yml b/.gitlab-ci.d/container-cross.yml
index 0fcebe363a..a3b5b90552 100644
--- a/.gitlab-ci.d/container-cross.yml
+++ b/.gitlab-ci.d/container-cross.yml
@@ -134,7 +134,8 @@ ppc64el-debian-cross-container:
 riscv64-debian-cross-container:
   extends: .container_job_template
   stage: containers-layer2
-  needs: ['amd64-debian10-container']
+  # as we are currently based on 'sid/unstable' we may break so...
+  allow_failure: true
   variables:
     NAME: debian-riscv64-cross
 
diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index ff5d732889..3b03763186 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -141,7 +141,6 @@ docker-image-debian-mips64-cross: docker-image-debian10
 docker-image-debian-mips64el-cross: docker-image-debian10
 docker-image-debian-mipsel-cross: docker-image-debian10
 docker-image-debian-ppc64el-cross: docker-image-debian10
-docker-image-debian-riscv64-cross: docker-image-debian10
 docker-image-debian-s390x-cross: docker-image-debian10
 docker-image-debian-sh4-cross: docker-image-debian10
 docker-image-debian-sparc64-cross: docker-image-debian10
@@ -180,7 +179,6 @@ DOCKER_PARTIAL_IMAGES += debian-arm64-test-cross
 DOCKER_PARTIAL_IMAGES += debian-powerpc-test-cross
 DOCKER_PARTIAL_IMAGES += debian-hppa-cross
 DOCKER_PARTIAL_IMAGES += debian-m68k-cross debian-mips64-cross
-DOCKER_PARTIAL_IMAGES += debian-riscv64-cross
 DOCKER_PARTIAL_IMAGES += debian-sh4-cross debian-sparc64-cross
 DOCKER_PARTIAL_IMAGES += debian-tricore-cross
 DOCKER_PARTIAL_IMAGES += debian-xtensa-cross
diff --git a/tests/docker/dockerfiles/debian-riscv64-cross.docker b/tests/docker/dockerfiles/debian-riscv64-cross.docker
index 2bbff19772..594d97982c 100644
--- a/tests/docker/dockerfiles/debian-riscv64-cross.docker
+++ b/tests/docker/dockerfiles/debian-riscv64-cross.docker
@@ -1,12 +1,48 @@
 #
-# Docker cross-compiler target
+# Docker cross-compiler target for riscv64
 #
-# This docker target builds on the debian Buster base image.
+# Currently the only distro that gets close to cross compiling riscv64
+# images is Debian Sid (with unofficial ports). As this is a moving
+# target we keep the library list minimal and are aiming to migrate
+# from this hack as soon as we are able.
 #
-FROM qemu/debian10
+FROM docker.io/library/debian:sid-slim
+
+# Add ports
+RUN apt update && \
+    DEBIAN_FRONTEND=noninteractive apt install -yy eatmydata && \
+    DEBIAN_FRONTEND=noninteractive eatmydata apt update -yy && \
+    DEBIAN_FRONTEND=noninteractive eatmydata apt upgrade -yy
+
+# Install common build utilities
+RUN DEBIAN_FRONTEND=noninteractive eatmydata apt install -yy \
+    bc \
+    build-essential \
+    ca-certificates \
+    debian-ports-archive-keyring \
+    dpkg-dev \
+    gettext \
+    git \
+    ninja-build \
+    pkg-config \
+    python3
+
+# Add ports and riscv64 architecture
+RUN echo "deb http://ftp.ports.debian.org/debian-ports/ sid main" >> /etc/apt/sources.list
+RUN dpkg --add-architecture riscv64
+
+# Duplicate deb line as deb-src
+RUN cat /etc/apt/sources.list | sed "s/^deb\ /deb-src /" >> /etc/apt/sources.list
 
 RUN apt update && \
     DEBIAN_FRONTEND=noninteractive eatmydata \
     apt install -y --no-install-recommends \
-        gcc-riscv64-linux-gnu \
-        libc6-dev-riscv64-cross
+         gcc-riscv64-linux-gnu \
+         libc6-dev-riscv64-cross \
+         libffi-dev:riscv64 \
+         libglib2.0-dev:riscv64 \
+         libpixman-1-dev:riscv64
+
+# Specify the cross prefix for this image (see tests/docker/common.rc)
+ENV QEMU_CONFIGURE_OPTS --cross-prefix=riscv64-linux-gnu-
+ENV DEF_TARGET_LIST riscv64-softmmu,riscv64-linux-user
-- 
2.30.2

[PATCH v1 05/11] gitlab: Add cross-riscv64-system, cross-riscv64-user

[Alex Bennée]

From: Richard Henderson <richard.henderson@linaro.org>

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20210914185830.1378771-3-richard.henderson@linaro.org>
[AJB: add allow_failure]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>

---
v2
 - allow_failure: true
---
 .gitlab-ci.d/crossbuilds.yml | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index f10168db2e..17d6cb3e45 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -124,6 +124,25 @@ cross-ppc64el-user:
   variables:
     IMAGE: debian-ppc64el-cross
 
+# The riscv64 cross-builds currently use a 'sid' container to get
+# compilers and libraries. Until something more stable is found we
+# allow_failure so as not to block CI.
+cross-riscv64-system:
+  extends: .cross_system_build_job
+  allow_failure: true
+  needs:
+    job: riscv64-debian-cross-container
+  variables:
+    IMAGE: debian-riscv64-cross
+
+cross-riscv64-user:
+  extends: .cross_user_build_job
+  allow_failure: true
+  needs:
+    job: riscv64-debian-cross-container
+  variables:
+    IMAGE: debian-riscv64-cross
+
 cross-s390x-system:
   extends: .cross_system_build_job
   needs:
-- 
2.30.2

[PATCH v1 06/11] python: Update for pylint 2.10

[Alex Bennée]

From: John Snow <jsnow@redhat.com>

A few new annoyances. Of note is the new warning for an unspecified
encoding when opening a text file, which actually does indicate a
potentially real problem; see
https://www.python.org/dev/peps/pep-0597/#motivation

Use LC_CTYPE to determine an encoding to use for interpreting QEMU's
terminal output. Note that Python states: "language code and encoding
may be None if their values cannot be determined" -- use a platform
default as a backup.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210916040955.628560-2-jsnow@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 python/qemu/machine/machine.py | 9 ++++++++-
 python/setup.cfg               | 1 +
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index a7081b1845..51b6e79a13 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -19,6 +19,7 @@
 
 import errno
 from itertools import chain
+import locale
 import logging
 import os
 import shutil
@@ -290,8 +291,14 @@ def get_pid(self) -> Optional[int]:
         return self._subp.pid
 
     def _load_io_log(self) -> None:
+        # Assume that the output encoding of QEMU's terminal output
+        # is defined by our locale. If indeterminate, use a platform default.
+        _, encoding = locale.getlocale()
+        if encoding is None:
+            encoding = locale.getpreferredencoding(do_setlocale=False)
         if self._qemu_log_path is not None:
-            with open(self._qemu_log_path, "r") as iolog:
+            with open(self._qemu_log_path, "r",
+                      encoding=encoding) as iolog:
                 self._iolog = iolog.read()
 
     @property
diff --git a/python/setup.cfg b/python/setup.cfg
index 83909c1c97..0f0cab098f 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -104,6 +104,7 @@ good-names=i,
 [pylint.similarities]
 # Ignore imports when computing similarities.
 ignore-imports=yes
+ignore-signatures=yes
 
 # Minimum lines number of a similarity.
 # TODO: Remove after we opt in to Pylint 2.8.3. See commit msg.
-- 
2.30.2

[PATCH v1 07/11] travis.yml: Remove the "Release tarball" job

[Alex Bennée]

From: Thomas Huth <thuth@redhat.com>

This is a leftover from the days when we were using Travis excessively,
but since x86 jobs are not really usable there anymore, this job has
likely never been used since many months. Let's simply remove it now.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20210917094826.466047-1-thuth@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 .travis.yml | 23 -----------------------
 1 file changed, 23 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 0faddf7b4e..41010ebe6b 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -305,26 +305,3 @@ jobs:
         - CONFIG="--disable-containers --disable-tcg --enable-kvm
                   --disable-tools --host-cc=clang --cxx=clang++"
         - UNRELIABLE=true
-
-    # Release builds
-    # The make-release script expect a QEMU version, so our tag must start with a 'v'.
-    # This is the case when release candidate tags are created.
-    - name: "Release tarball"
-      if: tag IS present AND tag =~ /^v\d+\.\d+(\.\d+)?(-\S*)?$/
-      env:
-        # We want to build from the release tarball
-        - BUILD_DIR="release/build/dir" SRC_DIR="../../.."
-        - BASE_CONFIG="--prefix=$PWD/dist"
-        - CONFIG="--target-list=x86_64-softmmu,aarch64-softmmu,armeb-linux-user,ppc-linux-user"
-        - TEST_CMD="make install -j${JOBS}"
-        - QEMU_VERSION="${TRAVIS_TAG:1}"
-        - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-default"
-      script:
-        - make -C ${SRC_DIR} qemu-${QEMU_VERSION}.tar.bz2
-        - ls -l ${SRC_DIR}/qemu-${QEMU_VERSION}.tar.bz2
-        - tar -xf ${SRC_DIR}/qemu-${QEMU_VERSION}.tar.bz2 && cd qemu-${QEMU_VERSION}
-        - mkdir -p release-build && cd release-build
-        - ../configure ${BASE_CONFIG} ${CONFIG} || { cat config.log meson-logs/meson-log.txt && exit 1; }
-        - make install
-  allow_failures:
-    - env: UNRELIABLE=true
-- 
2.30.2

[PATCH v1 08/11] gitlab: skip the check-patch job on the upstream repo

[Alex Bennée]

From: Daniel P. Berrangé <berrange@redhat.com>

The check-patch job is intended to be used by contributors or
subsystem maintainers to see if there are style mistakes. The
false positive rate is too high to be used in a gating scenario
so should not run it on the upstream repo ever.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20210915125452.1704899-2-berrange@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 .gitlab-ci.d/static_checks.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.gitlab-ci.d/static_checks.yml b/.gitlab-ci.d/static_checks.yml
index 96dbd9e310..902843f8b3 100644
--- a/.gitlab-ci.d/static_checks.yml
+++ b/.gitlab-ci.d/static_checks.yml
@@ -8,7 +8,7 @@ check-patch:
   variables:
     GIT_DEPTH: 1000
   rules:
-    - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH'
+    - if: '$CI_PROJECT_NAMESPACE == "qemu-project"'
       when: never
     - when: on_success
       allow_failure: true
-- 
2.30.2

[PATCH v1 09/11] gitlab: fix passing of TEST_TARGETS env to cirrus

[Alex Bennée]

From: Daniel P. Berrangé <berrange@redhat.com>

A typo meant the substitution would not work, and the placeholder in the
target file didn't even exist.

The result was that tests were never run on the FreeBSD and macOS jobs,
only a basic build.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Acked-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20210915125452.1704899-3-berrange@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 .gitlab-ci.d/cirrus.yml       | 2 +-
 .gitlab-ci.d/cirrus/build.yml | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/.gitlab-ci.d/cirrus.yml b/.gitlab-ci.d/cirrus.yml
index 675db69622..e7b25e7427 100644
--- a/.gitlab-ci.d/cirrus.yml
+++ b/.gitlab-ci.d/cirrus.yml
@@ -35,7 +35,7 @@
           -e "s|[@]PIP3@|$PIP3|g"
           -e "s|[@]PYPI_PKGS@|$PYPI_PKGS|g"
           -e "s|[@]CONFIGURE_ARGS@|$CONFIGURE_ARGS|g"
-          -e "s|[@]TEST_TARGETSS@|$TEST_TARGETSS|g"
+          -e "s|[@]TEST_TARGETS@|$TEST_TARGETS|g"
       <.gitlab-ci.d/cirrus/build.yml >.gitlab-ci.d/cirrus/$NAME.yml
     - cat .gitlab-ci.d/cirrus/$NAME.yml
     - cirrus-run -v --show-build-log always .gitlab-ci.d/cirrus/$NAME.yml
diff --git a/.gitlab-ci.d/cirrus/build.yml b/.gitlab-ci.d/cirrus/build.yml
index 857bdc5536..c555f5d36e 100644
--- a/.gitlab-ci.d/cirrus/build.yml
+++ b/.gitlab-ci.d/cirrus/build.yml
@@ -13,6 +13,7 @@ env:
   PYTHON: "@PYTHON@"
   MAKE: "@MAKE@"
   CONFIGURE_ARGS: "@CONFIGURE_ARGS@"
+  TEST_TARGETS: "@TEST_TARGETS@"
 
 build_task:
   install_script:
-- 
2.30.2

[PATCH v1 10/11] plugins/: Add missing functions to symbol list

[Alex Bennée]

From: Lukas Jünger <lukas.junger@greensocs.com>

Some functions of the plugin API were missing in
the symbol list. However, they are all used by
the contributed example plugins. QEMU fails to
load the plugin if the function symbol is not
exported.

Signed-off-by: Lukas Jünger <lukas.junger@greensocs.com>
Message-Id: <20210905140939.638928-2-lukas.junger@greensocs.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 plugins/qemu-plugins.symbols | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
index 67b309ea2a..4834756ba3 100644
--- a/plugins/qemu-plugins.symbols
+++ b/plugins/qemu-plugins.symbols
@@ -1,11 +1,14 @@
 {
   qemu_plugin_bool_parse;
   qemu_plugin_get_hwaddr;
+  qemu_plugin_hwaddr_device_name;
   qemu_plugin_hwaddr_is_io;
+  qemu_plugin_hwaddr_phys_addr;
   qemu_plugin_insn_data;
   qemu_plugin_insn_disas;
   qemu_plugin_insn_haddr;
   qemu_plugin_insn_size;
+  qemu_plugin_insn_symbol;
   qemu_plugin_insn_vaddr;
   qemu_plugin_mem_is_big_endian;
   qemu_plugin_mem_is_sign_extended;
-- 
2.30.2

[PATCH v1 11/11] accel/tcg: re-factor plugin_inject_cb so we can assert insn_idx is valid

[Alex Bennée]

Coverity doesn't know enough about how we have arranged our plugin TCG
ops to know we will always have incremented insn_idx before injecting
the callback. Let us assert it for the benefit of Coverity and protect
ourselves from accidentally breaking the assumption and triggering
harder to grok errors deeper in the code if we attempt a negative
indexed array lookup.

However to get to this point we re-factor the code and remove the
second hand instruction boundary detection in favour of scanning the
full set of ops and using the existing INDEX_op_insn_start to cleanly
detect when the instruction has started. As we no longer need the
plugin specific list of ops we delete that.

My initial benchmarks shows no discernible impact of dropping the
plugin specific ops list.

Fixes: Coverity 1459509
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Cc: Peter Maydell <peter.maydell@linaro.org>

---
v2/v3
  - re-factor and drop the plugin_op/link approach
---
 include/tcg/tcg.h      |   6 --
 accel/tcg/plugin-gen.c | 157 ++++++++++++++++++++++-------------------
 2 files changed, 85 insertions(+), 78 deletions(-)

diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index 44ccd86f3e..a7f6475348 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -492,9 +492,6 @@ typedef struct TCGOp {
 
     /* Next and previous opcodes.  */
     QTAILQ_ENTRY(TCGOp) link;
-#ifdef CONFIG_PLUGIN
-    QSIMPLEQ_ENTRY(TCGOp) plugin_link;
-#endif
 
     /* Arguments for the opcode.  */
     TCGArg args[MAX_OPC_PARAM];
@@ -604,9 +601,6 @@ struct TCGContext {
 
     /* descriptor of the instruction being translated */
     struct qemu_plugin_insn *plugin_insn;
-
-    /* list to quickly access the injected ops */
-    QSIMPLEQ_HEAD(, TCGOp) plugin_ops;
 #endif
 
     GHashTable *const_table[TCG_TYPE_COUNT];
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 88e25c6df9..f145b815c0 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -163,11 +163,7 @@ static void gen_empty_mem_helper(void)
 static void gen_plugin_cb_start(enum plugin_gen_from from,
                                 enum plugin_gen_cb type, unsigned wr)
 {
-    TCGOp *op;
-
     tcg_gen_plugin_cb_start(from, type, wr);
-    op = tcg_last_op();
-    QSIMPLEQ_INSERT_TAIL(&tcg_ctx->plugin_ops, op, plugin_link);
 }
 
 static void gen_wrapped(enum plugin_gen_from from,
@@ -707,62 +703,6 @@ static void plugin_gen_disable_mem_helper(const struct qemu_plugin_tb *ptb,
     inject_mem_disable_helper(insn, begin_op);
 }
 
-static void plugin_inject_cb(const struct qemu_plugin_tb *ptb, TCGOp *begin_op,
-                             int insn_idx)
-{
-    enum plugin_gen_from from = begin_op->args[0];
-    enum plugin_gen_cb type = begin_op->args[1];
-
-    switch (from) {
-    case PLUGIN_GEN_FROM_TB:
-        switch (type) {
-        case PLUGIN_GEN_CB_UDATA:
-            plugin_gen_tb_udata(ptb, begin_op);
-            return;
-        case PLUGIN_GEN_CB_INLINE:
-            plugin_gen_tb_inline(ptb, begin_op);
-            return;
-        default:
-            g_assert_not_reached();
-        }
-    case PLUGIN_GEN_FROM_INSN:
-        switch (type) {
-        case PLUGIN_GEN_CB_UDATA:
-            plugin_gen_insn_udata(ptb, begin_op, insn_idx);
-            return;
-        case PLUGIN_GEN_CB_INLINE:
-            plugin_gen_insn_inline(ptb, begin_op, insn_idx);
-            return;
-        case PLUGIN_GEN_ENABLE_MEM_HELPER:
-            plugin_gen_enable_mem_helper(ptb, begin_op, insn_idx);
-            return;
-        default:
-            g_assert_not_reached();
-        }
-    case PLUGIN_GEN_FROM_MEM:
-        switch (type) {
-        case PLUGIN_GEN_CB_MEM:
-            plugin_gen_mem_regular(ptb, begin_op, insn_idx);
-            return;
-        case PLUGIN_GEN_CB_INLINE:
-            plugin_gen_mem_inline(ptb, begin_op, insn_idx);
-            return;
-        default:
-            g_assert_not_reached();
-        }
-    case PLUGIN_GEN_AFTER_INSN:
-        switch (type) {
-        case PLUGIN_GEN_DISABLE_MEM_HELPER:
-            plugin_gen_disable_mem_helper(ptb, begin_op, insn_idx);
-            return;
-        default:
-            g_assert_not_reached();
-        }
-    default:
-        g_assert_not_reached();
-    }
-}
-
 /* #define DEBUG_PLUGIN_GEN_OPS */
 static void pr_ops(void)
 {
@@ -820,21 +760,95 @@ static void pr_ops(void)
 static void plugin_gen_inject(const struct qemu_plugin_tb *plugin_tb)
 {
     TCGOp *op;
-    int insn_idx;
+    int insn_idx = -1;
 
     pr_ops();
-    insn_idx = -1;
-    QSIMPLEQ_FOREACH(op, &tcg_ctx->plugin_ops, plugin_link) {
-        enum plugin_gen_from from = op->args[0];
-        enum plugin_gen_cb type = op->args[1];
-
-        tcg_debug_assert(op->opc == INDEX_op_plugin_cb_start);
-        /* ENABLE_MEM_HELPER is the first callback of an instruction */
-        if (from == PLUGIN_GEN_FROM_INSN &&
-            type == PLUGIN_GEN_ENABLE_MEM_HELPER) {
+
+    QTAILQ_FOREACH(op, &tcg_ctx->ops, link) {
+        switch (op->opc) {
+        case INDEX_op_insn_start:
             insn_idx++;
+            break;
+        case INDEX_op_plugin_cb_start:
+        {
+            enum plugin_gen_from from = op->args[0];
+            enum plugin_gen_cb type = op->args[1];
+
+            switch (from) {
+            case PLUGIN_GEN_FROM_TB:
+            {
+                g_assert(insn_idx == -1);
+
+                switch (type) {
+                case PLUGIN_GEN_CB_UDATA:
+                    plugin_gen_tb_udata(plugin_tb, op);
+                    break;
+                case PLUGIN_GEN_CB_INLINE:
+                    plugin_gen_tb_inline(plugin_tb, op);
+                    break;
+                default:
+                    g_assert_not_reached();
+                }
+                break;
+            }
+            case PLUGIN_GEN_FROM_INSN:
+            {
+                g_assert(insn_idx >= 0);
+
+                switch (type) {
+                case PLUGIN_GEN_CB_UDATA:
+                    plugin_gen_insn_udata(plugin_tb, op, insn_idx);
+                    break;
+                case PLUGIN_GEN_CB_INLINE:
+                    plugin_gen_insn_inline(plugin_tb, op, insn_idx);
+                    break;
+                case PLUGIN_GEN_ENABLE_MEM_HELPER:
+                    plugin_gen_enable_mem_helper(plugin_tb, op, insn_idx);
+                    break;
+                default:
+                    g_assert_not_reached();
+                }
+                break;
+            }
+            case PLUGIN_GEN_FROM_MEM:
+            {
+                g_assert(insn_idx >= 0);
+
+                switch (type) {
+                case PLUGIN_GEN_CB_MEM:
+                    plugin_gen_mem_regular(plugin_tb, op, insn_idx);
+                    break;
+                case PLUGIN_GEN_CB_INLINE:
+                    plugin_gen_mem_inline(plugin_tb, op, insn_idx);
+                    break;
+                default:
+                    g_assert_not_reached();
+                }
+
+                break;
+            }
+            case PLUGIN_GEN_AFTER_INSN:
+            {
+                g_assert(insn_idx >= 0);
+
+                switch (type) {
+                case PLUGIN_GEN_DISABLE_MEM_HELPER:
+                    plugin_gen_disable_mem_helper(plugin_tb, op, insn_idx);
+                    break;
+                default:
+                    g_assert_not_reached();
+                }
+                break;
+            }
+            default:
+                g_assert_not_reached();
+            }
+            break;
+        }
+        default:
+            /* plugins don't care about any other ops */
+            break;
         }
-        plugin_inject_cb(plugin_tb, op, insn_idx);
     }
     pr_ops();
 }
@@ -847,7 +861,6 @@ bool plugin_gen_tb_start(CPUState *cpu, const TranslationBlock *tb, bool mem_onl
     if (test_bit(QEMU_PLUGIN_EV_VCPU_TB_TRANS, cpu->plugin_mask)) {
         ret = true;
 
-        QSIMPLEQ_INIT(&tcg_ctx->plugin_ops);
         ptb->vaddr = tb->pc;
         ptb->vaddr2 = -1;
         get_page_addr_code_hostp(cpu->env_ptr, tb->pc, &ptb->haddr1);
-- 
2.30.2

Re: [PATCH v1 06/11] python: Update for pylint 2.10

[John Snow]

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

Hi, I sent a PR for this (and an additional fix that is needed as of
yesterday):

https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg04477.html

Please drop this patch and prefer that series.

--js

On Fri, Sep 17, 2021 at 12:23 PM Alex Bennée <alex.bennee@linaro.org> wrote:

> From: John Snow <jsnow@redhat.com>
>
> A few new annoyances. Of note is the new warning for an unspecified
> encoding when opening a text file, which actually does indicate a
> potentially real problem; see
> https://www.python.org/dev/peps/pep-0597/#motivation
>
> Use LC_CTYPE to determine an encoding to use for interpreting QEMU's
> terminal output. Note that Python states: "language code and encoding
> may be None if their values cannot be determined" -- use a platform
> default as a backup.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Message-Id: <20210916040955.628560-2-jsnow@redhat.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Tested-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  python/qemu/machine/machine.py | 9 ++++++++-
>  python/setup.cfg               | 1 +
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/python/qemu/machine/machine.py
> b/python/qemu/machine/machine.py
> index a7081b1845..51b6e79a13 100644
> --- a/python/qemu/machine/machine.py
> +++ b/python/qemu/machine/machine.py
> @@ -19,6 +19,7 @@
>
>  import errno
>  from itertools import chain
> +import locale
>  import logging
>  import os
>  import shutil
> @@ -290,8 +291,14 @@ def get_pid(self) -> Optional[int]:
>          return self._subp.pid
>
>      def _load_io_log(self) -> None:
> +        # Assume that the output encoding of QEMU's terminal output
> +        # is defined by our locale. If indeterminate, use a platform
> default.
> +        _, encoding = locale.getlocale()
> +        if encoding is None:
> +            encoding = locale.getpreferredencoding(do_setlocale=False)
>          if self._qemu_log_path is not None:
> -            with open(self._qemu_log_path, "r") as iolog:
> +            with open(self._qemu_log_path, "r",
> +                      encoding=encoding) as iolog:
>                  self._iolog = iolog.read()
>
>      @property
> diff --git a/python/setup.cfg b/python/setup.cfg
> index 83909c1c97..0f0cab098f 100644
> --- a/python/setup.cfg
> +++ b/python/setup.cfg
> @@ -104,6 +104,7 @@ good-names=i,
>  [pylint.similarities]
>  # Ignore imports when computing similarities.
>  ignore-imports=yes
> +ignore-signatures=yes
>
>  # Minimum lines number of a similarity.
>  # TODO: Remove after we opt in to Pylint 2.8.3. See commit msg.
> --
> 2.30.2
>
>

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

Re: [PATCH v1 01/11] configure: don't override the selected host test compiler if defined

[Warner Losh]

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

On Fri, Sep 17, 2021 at 10:23 AM Alex Bennée <alex.bennee@linaro.org> wrote:

> There are not many cases you would want to do this but one is if you
> want to use a test friendly compiler like gcc instead of a system
> compiler like clang. Either way we should honour the users choice if
> they have made it.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Warner Losh <imp@bsdimp.com>
> ---
>  configure | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>


> diff --git a/configure b/configure
> index 1043ccce4f..049c50a933 100755
> --- a/configure
> +++ b/configure
> @@ -1686,8 +1686,11 @@ case "$cpu" in
>      # No special flags required for other host CPUs
>  esac
>
> -eval "cross_cc_${cpu}=\$cc"
> -cross_cc_vars="$cross_cc_vars cross_cc_${cpu}"
> +if eval test -z "\${cross_cc_$cpu}"; then
> +    eval "cross_cc_${cpu}=\$cc"
> +    cross_cc_vars="$cross_cc_vars cross_cc_${cpu}"
> +fi
> +
>  QEMU_CFLAGS="$CPU_CFLAGS $QEMU_CFLAGS"
>
>  # For user-mode emulation the host arch has to be one we explicitly
> --
> 2.30.2
>
>

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

Re: [PATCH v1 03/11] tests/tcg: move some multiarch files and make conditional

[Warner Losh]

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

On Fri, Sep 17, 2021 at 10:23 AM Alex Bennée <alex.bennee@linaro.org> wrote:

> We had some messy code to filter out stuff we can't build. Lets junk
> that and simplify the logic by pushing some stuff into subdirs. In
> particular we move:
>
>   float_helpers into libs - not a standalone test
>   linux-test into linux - so we only build on Linux hosts
>
> This allows for at least some of the tests to be nominally usable
> by *BSD user builds.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Warner Losh <imp@bsdimp.com>
> ---
>  tests/tcg/multiarch/{ => libs}/float_helpers.c |  2 +-
>  tests/tcg/multiarch/{ => linux}/linux-test.c   |  0
>  tests/tcg/multiarch/Makefile.target            | 15 ++++++++++-----
>  tests/tcg/x86_64/Makefile.target               |  4 ++++
>  4 files changed, 15 insertions(+), 6 deletions(-)
>  rename tests/tcg/multiarch/{ => libs}/float_helpers.c (99%)
>  rename tests/tcg/multiarch/{ => linux}/linux-test.c (100%)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>

I've also tested it as well.


> diff --git a/tests/tcg/multiarch/float_helpers.c
> b/tests/tcg/multiarch/libs/float_helpers.c
> similarity index 99%
> rename from tests/tcg/multiarch/float_helpers.c
> rename to tests/tcg/multiarch/libs/float_helpers.c
> index bc530e5732..4e68d2b659 100644
> --- a/tests/tcg/multiarch/float_helpers.c
> +++ b/tests/tcg/multiarch/libs/float_helpers.c
> @@ -22,7 +22,7 @@
>  #include <float.h>
>  #include <fenv.h>
>
> -#include "float_helpers.h"
> +#include "../float_helpers.h"
>
>  #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>
> diff --git a/tests/tcg/multiarch/linux-test.c
> b/tests/tcg/multiarch/linux/linux-test.c
> similarity index 100%
> rename from tests/tcg/multiarch/linux-test.c
> rename to tests/tcg/multiarch/linux/linux-test.c
> diff --git a/tests/tcg/multiarch/Makefile.target
> b/tests/tcg/multiarch/Makefile.target
> index 85a6fb7a2e..3763df2019 100644
> --- a/tests/tcg/multiarch/Makefile.target
> +++ b/tests/tcg/multiarch/Makefile.target
> @@ -8,18 +8,23 @@
>  MULTIARCH_SRC=$(SRC_PATH)/tests/tcg/multiarch
>
>  # Set search path for all sources
> -VPATH          += $(MULTIARCH_SRC)
> -MULTIARCH_SRCS   =$(notdir $(wildcard $(MULTIARCH_SRC)/*.c))
> -MULTIARCH_TESTS  =$(filter-out float_helpers, $(MULTIARCH_SRCS:.c=))
> +VPATH         += $(MULTIARCH_SRC)
> +MULTIARCH_SRCS =  $(notdir $(wildcard $(MULTIARCH_SRC)/*.c))
> +ifneq ($(CONFIG_LINUX),)
> +VPATH         += $(MULTIARCH_SRC)/linux
> +MULTIARCH_SRCS += $(notdir $(wildcard $(MULTIARCH_SRC)/linux/*.c))
> +endif
> +MULTIARCH_TESTS = $(MULTIARCH_SRCS:.c=)
>
> +$(info SRCS=${MULTIARCH_SRCS} and ${MULTIARCH_TESTS})
>  #
>  # The following are any additional rules needed to build things
>  #
>
>
>  float_%: LDFLAGS+=-lm
> -float_%: float_%.c float_helpers.c
> -       $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $<
> $(MULTIARCH_SRC)/float_helpers.c -o $@ $(LDFLAGS)
> +float_%: float_%.c libs/float_helpers.c
> +       $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $<
> $(MULTIARCH_SRC)/libs/float_helpers.c -o $@ $(LDFLAGS)
>
>  run-float_%: float_%
>         $(call run-test,$<, $(QEMU) $(QEMU_OPTS) $<,"$< on $(TARGET_NAME)")
> diff --git a/tests/tcg/x86_64/Makefile.target
> b/tests/tcg/x86_64/Makefile.target
> index 2151ea6302..d7a7385583 100644
> --- a/tests/tcg/x86_64/Makefile.target
> +++ b/tests/tcg/x86_64/Makefile.target
> @@ -8,8 +8,12 @@
>
>  include $(SRC_PATH)/tests/tcg/i386/Makefile.target
>
> +ifneq ($(CONFIG_LINUX),)
>  X86_64_TESTS += vsyscall
>  TESTS=$(MULTIARCH_TESTS) $(X86_64_TESTS) test-x86_64
> +else
> +TESTS=$(MULTIARCH_TESTS)
> +endif
>  QEMU_OPTS += -cpu max
>
>  test-x86_64: LDFLAGS+=-lm -lc
> --
> 2.30.2
>
>

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

Re: [PATCH v1 05/11] gitlab: Add cross-riscv64-system, cross-riscv64-user

[Philippe Mathieu-Daudé]

On 9/17/21 6:23 PM, Alex Bennée wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> Message-Id: <20210914185830.1378771-3-richard.henderson@linaro.org>
> [AJB: add allow_failure]
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

This was a Nacked-by... but I'm find with this version,
so OK for the R-b tag.

> Reviewed-by: Willian Rampazzo <willianr@redhat.com>
> 
> ---
> v2
>  - allow_failure: true
> ---
>  .gitlab-ci.d/crossbuilds.yml | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
> index f10168db2e..17d6cb3e45 100644
> --- a/.gitlab-ci.d/crossbuilds.yml
> +++ b/.gitlab-ci.d/crossbuilds.yml
> @@ -124,6 +124,25 @@ cross-ppc64el-user:
>    variables:
>      IMAGE: debian-ppc64el-cross
>  
> +# The riscv64 cross-builds currently use a 'sid' container to get
> +# compilers and libraries. Until something more stable is found we
> +# allow_failure so as not to block CI.
> +cross-riscv64-system:
> +  extends: .cross_system_build_job
> +  allow_failure: true
> +  needs:
> +    job: riscv64-debian-cross-container
> +  variables:
> +    IMAGE: debian-riscv64-cross
> +
> +cross-riscv64-user:
> +  extends: .cross_user_build_job
> +  allow_failure: true
> +  needs:
> +    job: riscv64-debian-cross-container
> +  variables:
> +    IMAGE: debian-riscv64-cross
> +
>  cross-s390x-system:
>    extends: .cross_system_build_job
>    needs:
> 

Re: [PATCH v1 03/11] tests/tcg: move some multiarch files and make conditional

[Philippe Mathieu-Daudé]

On 9/17/21 18:23, Alex Bennée wrote:
> We had some messy code to filter out stuff we can't build. Lets junk
> that and simplify the logic by pushing some stuff into subdirs. In
> particular we move:
> 
>   float_helpers into libs - not a standalone test
>   linux-test into linux - so we only build on Linux hosts
> 
> This allows for at least some of the tests to be nominally usable
> by *BSD user builds.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Warner Losh <imp@bsdimp.com>
> ---
>  tests/tcg/multiarch/{ => libs}/float_helpers.c |  2 +-
>  tests/tcg/multiarch/{ => linux}/linux-test.c   |  0
>  tests/tcg/multiarch/Makefile.target            | 15 ++++++++++-----
>  tests/tcg/x86_64/Makefile.target               |  4 ++++
>  4 files changed, 15 insertions(+), 6 deletions(-)
>  rename tests/tcg/multiarch/{ => libs}/float_helpers.c (99%)
>  rename tests/tcg/multiarch/{ => linux}/linux-test.c (100%)

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

Re: [PATCH v1 01/11] configure: don't override the selected host test compiler if defined

[Ed Maste]

On Fri, 17 Sept 2021 at 12:27, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> There are not many cases you would want to do this but one is if you
> want to use a test friendly compiler like gcc instead of a system
> compiler like clang. Either way we should honour the users choice if
> they have made it.

A little bit of a tangent, but I'm curious about this comment - what
aspects of GCC make it more test friendly? (I help maintain LLVM tools
on FreeBSD, and would like to make sure they provide as good an
experience as possible.)

Re: [PATCH v1 01/11] configure: don't override the selected host test compiler if defined

[Alex Bennée]

Ed Maste <emaste@freebsd.org> writes:

> On Fri, 17 Sept 2021 at 12:27, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> There are not many cases you would want to do this but one is if you
>> want to use a test friendly compiler like gcc instead of a system
>> compiler like clang. Either way we should honour the users choice if
>> they have made it.
>
> A little bit of a tangent, but I'm curious about this comment - what
> aspects of GCC make it more test friendly? (I help maintain LLVM tools
> on FreeBSD, and would like to make sure they provide as good an
> experience as possible.)

Some of the inline assembler in the old tests trips up LLVM. There is a
chance the inline assembly is a bit bogus with it's constraints and GCC
is just a bit more lax and accepting.

I'm totally fine with merging something that restores support for clang
for building tests it just needs the appropriate fixups to the tests. We
already use clang for the hexagon builds after all. Once that is done we
can drop the stuff in cf22f936f2 (tests/tcg: don't allow clang as a
cross compiler).

-- 
Alex Bennée