[PATCH for 6.0-rc2? v1 0/6] check-tcg for non-x86 hosts

[PATCH for 6.0-rc2? v1 0/6] check-tcg for non-x86 hosts

[Alex Bennée]

Hi,

The following are broadly just expanding our testing on arm64 systems
which I think is permissible on later release candidates but it's not a
major problem if they slip into the next release.

The latest arm64 CI box has access to a fair few cross compilers for
other arches thanks to Ubuntu and the fall back of Debian containers
covers a few more. There is an outstanding problem with the inline
assembler constraints for i386 16-bit code calls which I'm hoping
someone with more asm-foo than me can figure out the right runes for
it.

Please review (and test if you have access to !(arm64/x86) developer
boxes).

Alex Bennée (6):
  tests/tcg: update the defaults for x86 compilers
  tests/docker: don't set DOCKER_REGISTRY on non-x86_64
  tests/tcg: add concept of container_hosts
  tests/tcg/configure.sh: make sure we pick up x86_64 cross compilers
  tests/tcg/i386: expand .data sections for system tests
  tests/i386/test-i386: try and correct constrain 16 bit call (WIP)

 configure                       |  2 +-
 tests/tcg/i386/test-i386.c      |  6 +++---
 tests/docker/Makefile.include   |  5 ++++-
 tests/tcg/configure.sh          | 36 +++++++++++++++++++++++++++++----
 tests/tcg/i386/system/kernel.ld |  2 +-
 5 files changed, 41 insertions(+), 10 deletions(-)

-- 
2.20.1

[PATCH v1 1/6] tests/tcg: update the defaults for x86 compilers

[Alex Bennée]

You don't usually notice this is broken on developer system on x86 as
we use the normal host compiler. However on other systems the -pc was
extraneous. Also for 32 bit only i686 packages exist now so we should
use those when available.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/tcg/configure.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
index ce304f4933..af4aecf14e 100755
--- a/tests/tcg/configure.sh
+++ b/tests/tcg/configure.sh
@@ -52,7 +52,7 @@ fi
 : ${cross_cc_hexagon="hexagon-unknown-linux-musl-clang"}
 : ${cross_cc_cflags_hexagon="-mv67 -O2 -static"}
 : ${cross_cc_hppa="hppa-linux-gnu-gcc"}
-: ${cross_cc_i386="i386-pc-linux-gnu-gcc"}
+: ${cross_cc_i386="i686-linux-gnu-gcc"}
 : ${cross_cc_cflags_i386="-m32"}
 : ${cross_cc_m68k="m68k-linux-gnu-gcc"}
 : $(cross_cc_mips64el="mips64el-linux-gnuabi64-gcc")
@@ -69,7 +69,7 @@ fi
 : ${cross_cc_cflags_sparc="-m32 -mv8plus -mcpu=ultrasparc"}
 : ${cross_cc_sparc64="sparc64-linux-gnu-gcc"}
 : ${cross_cc_cflags_sparc64="-m64 -mcpu=ultrasparc"}
-: ${cross_cc_x86_64="x86_64-pc-linux-gnu-gcc"}
+: ${cross_cc_x86_64="x86_64-linux-gnu-gcc"}
 : ${cross_cc_cflags_x86_64="-m64"}
 
 for target in $target_list; do
-- 
2.20.1

Re: [PATCH v1 1/6] tests/tcg: update the defaults for x86 compilers

[Richard Henderson]

On 3/29/21 5:02 AM, Alex Bennée wrote:
> You don't usually notice this is broken on developer system on x86 as
> we use the normal host compiler. However on other systems the -pc was
> extraneous. Also for 32 bit only i686 packages exist now so we should
> use those when available.
> 
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> ---
>   tests/tcg/configure.sh | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

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

r~

[PATCH v1 2/6] tests/docker: don't set DOCKER_REGISTRY on non-x86_64

[Alex Bennée]

Currently our gitlab registry is x86_64 only so attempting to pull an
image from it on something else will end in tears.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/docker/Makefile.include | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 7cab761bf5..9f464cb92c 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -16,7 +16,10 @@ DOCKER_IMAGES := $(sort $(notdir $(basename $(wildcard $(DOCKER_FILES_DIR)/*.doc
 DOCKER_TARGETS := $(patsubst %,docker-image-%,$(DOCKER_IMAGES))
 # Use a global constant ccache directory to speed up repetitive builds
 DOCKER_CCACHE_DIR := $$HOME/.cache/qemu-docker-ccache
-DOCKER_REGISTRY := $(if $(REGISTRY),$(REGISTRY),registry.gitlab.com/qemu-project/qemu)
+ifeq ($(HOST_ARCH),x86_64)
+DOCKER_DEFAULT_REGISTRY := registry.gitlab.com/qemu-project/qemu
+endif
+DOCKER_REGISTRY := $(if $(REGISTRY),$(REGISTRY),$(DOCKER_DEFAULT_REGISTRY))
 
 DOCKER_TESTS := $(notdir $(shell \
 	find $(SRC_PATH)/tests/docker/ -name 'test-*' -type f))
-- 
2.20.1

Re: [PATCH v1 2/6] tests/docker: don't set DOCKER_REGISTRY on non-x86_64

[Thomas Huth]

On 29/03/2021 13.02, Alex Bennée wrote:
> Currently our gitlab registry is x86_64 only so attempting to pull an
> image from it on something else will end in tears.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   tests/docker/Makefile.include | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 7cab761bf5..9f464cb92c 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -16,7 +16,10 @@ DOCKER_IMAGES := $(sort $(notdir $(basename $(wildcard $(DOCKER_FILES_DIR)/*.doc
>   DOCKER_TARGETS := $(patsubst %,docker-image-%,$(DOCKER_IMAGES))
>   # Use a global constant ccache directory to speed up repetitive builds
>   DOCKER_CCACHE_DIR := $$HOME/.cache/qemu-docker-ccache
> -DOCKER_REGISTRY := $(if $(REGISTRY),$(REGISTRY),registry.gitlab.com/qemu-project/qemu)
> +ifeq ($(HOST_ARCH),x86_64)
> +DOCKER_DEFAULT_REGISTRY := registry.gitlab.com/qemu-project/qemu
> +endif
> +DOCKER_REGISTRY := $(if $(REGISTRY),$(REGISTRY),$(DOCKER_DEFAULT_REGISTRY))

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v1 2/6] tests/docker: don't set DOCKER_REGISTRY on non-x86_64

[Willian Rampazzo]

On Mon, Mar 29, 2021 at 8:03 AM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Currently our gitlab registry is x86_64 only so attempting to pull an
> image from it on something else will end in tears.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/docker/Makefile.include | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH v1 2/6] tests/docker: don't set DOCKER_REGISTRY on non-x86_64

[Richard Henderson]

On 3/29/21 5:02 AM, Alex Bennée wrote:
> Currently our gitlab registry is x86_64 only so attempting to pull an
> image from it on something else will end in tears.
> 
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> ---
>   tests/docker/Makefile.include | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)

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

r~

Re: [PATCH v1 2/6] tests/docker: don't set DOCKER_REGISTRY on non-x86_64

[Philippe Mathieu-Daudé]

Hi Alex, Willian,

On 3/29/21 1:02 PM, Alex Bennée wrote:
> Currently our gitlab registry is x86_64 only so attempting to pull an
> image from it on something else will end in tears.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/docker/Makefile.include | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 7cab761bf5..9f464cb92c 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -16,7 +16,10 @@ DOCKER_IMAGES := $(sort $(notdir $(basename $(wildcard $(DOCKER_FILES_DIR)/*.doc
>  DOCKER_TARGETS := $(patsubst %,docker-image-%,$(DOCKER_IMAGES))
>  # Use a global constant ccache directory to speed up repetitive builds
>  DOCKER_CCACHE_DIR := $$HOME/.cache/qemu-docker-ccache
> -DOCKER_REGISTRY := $(if $(REGISTRY),$(REGISTRY),registry.gitlab.com/qemu-project/qemu)
> +ifeq ($(HOST_ARCH),x86_64)
> +DOCKER_DEFAULT_REGISTRY := registry.gitlab.com/qemu-project/qemu
> +endif
> +DOCKER_REGISTRY := $(if $(REGISTRY),$(REGISTRY),$(DOCKER_DEFAULT_REGISTRY))

I remember we chatted about Docker multiarch on IRC.

It doesn't look going into the right direction. IMO the problem
you are having is because we push an amd64 docker image into the now
multiarch namespace, so it is obviously failing on all non-amd64 hosts.

I think the correct way is to remove the images stored in the multiarch
namespace (they are amd64, not multiarch); and store the amd64 images
under the amd64 namespace.

IOW prepend amd64/ in our amd64 specific images.

Then you could push a aarch64 image using the arm64v8/ prefix namespace
and the buildsys machinery will work with the registry.

FWIW I tested it with mips64le/ prefix on the mips64le runner.

Willian, can you have a try with ppc64le/ (eventually s390x/) prefix
for similar hosts?

Related info:
https://github.com/docker-library/official-images#architectures-other-than-amd64
https://github.com/docker-library/official-images#multiple-architectures

Regards,

Phil.

Re: [PATCH v1 2/6] tests/docker: don't set DOCKER_REGISTRY on non-x86_64

[Alex Bennée]

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

> Hi Alex, Willian,
>
> On 3/29/21 1:02 PM, Alex Bennée wrote:
>> Currently our gitlab registry is x86_64 only so attempting to pull an
>> image from it on something else will end in tears.
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  tests/docker/Makefile.include | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
>> index 7cab761bf5..9f464cb92c 100644
>> --- a/tests/docker/Makefile.include
>> +++ b/tests/docker/Makefile.include
>> @@ -16,7 +16,10 @@ DOCKER_IMAGES := $(sort $(notdir $(basename $(wildcard $(DOCKER_FILES_DIR)/*.doc
>>  DOCKER_TARGETS := $(patsubst %,docker-image-%,$(DOCKER_IMAGES))
>>  # Use a global constant ccache directory to speed up repetitive builds
>>  DOCKER_CCACHE_DIR := $$HOME/.cache/qemu-docker-ccache
>> -DOCKER_REGISTRY := $(if $(REGISTRY),$(REGISTRY),registry.gitlab.com/qemu-project/qemu)
>> +ifeq ($(HOST_ARCH),x86_64)
>> +DOCKER_DEFAULT_REGISTRY := registry.gitlab.com/qemu-project/qemu
>> +endif
>> +DOCKER_REGISTRY := $(if $(REGISTRY),$(REGISTRY),$(DOCKER_DEFAULT_REGISTRY))
>
> I remember we chatted about Docker multiarch on IRC.
>
> It doesn't look going into the right direction. IMO the problem
> you are having is because we push an amd64 docker image into the now
> multiarch namespace, so it is obviously failing on all non-amd64 hosts.
>
> I think the correct way is to remove the images stored in the multiarch
> namespace (they are amd64, not multiarch); and store the amd64 images
> under the amd64 namespace.
>
> IOW prepend amd64/ in our amd64 specific images.

I agree we should fix this properly but I don't think it's worth the
churn on the run up to release. ATM we don't really test building on
other architectures (qemu or tests) except manually. This step makes it
a little less painful. Once we actually have the non-x86 runners going
we can then start working out how to get them to push properly to the
registry and revert this change.

>
> Then you could push a aarch64 image using the arm64v8/ prefix namespace
> and the buildsys machinery will work with the registry.
>
> FWIW I tested it with mips64le/ prefix on the mips64le runner.
>
> Willian, can you have a try with ppc64le/ (eventually s390x/) prefix
> for similar hosts?
>
> Related info:
> https://github.com/docker-library/official-images#architectures-other-than-amd64
> https://github.com/docker-library/official-images#multiple-architectures
>
> Regards,
>
> Phil.


-- 
Alex Bennée

[PATCH v1 3/6] tests/tcg: add concept of container_hosts

[Alex Bennée]

While docker is nominally multarch these days it doesn't mean our
distros actually package all cross compilers for all architectures.
The upcoming Debian bullseye release will improve things further. At
least for now we can get things like the 32 bit ARM compiler on it's
64 bit cousin.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 configure              |  2 +-
 tests/tcg/configure.sh | 27 +++++++++++++++++++++++++--
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 535e6a9269..7df7750a14 100755
--- a/configure
+++ b/configure
@@ -6299,7 +6299,7 @@ done
 (for i in $cross_cc_vars; do
   export $i
 done
-export target_list source_path use_containers
+export target_list source_path use_containers ARCH
 $source_path/tests/tcg/configure.sh)
 
 # temporary config to build submodules
diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
index af4aecf14e..87a9f24b20 100755
--- a/tests/tcg/configure.sh
+++ b/tests/tcg/configure.sh
@@ -108,79 +108,98 @@ for target in $target_list; do
   case $target in
     aarch64-*)
       # We don't have any bigendian build tools so we only use this for AArch64
+      container_hosts="x86_64 aarch64"
       container_image=debian-arm64-test-cross
       container_cross_cc=aarch64-linux-gnu-gcc-10
       ;;
     alpha-*)
+      container_hosts=x86_64
       container_image=debian-alpha-cross
       container_cross_cc=alpha-linux-gnu-gcc
       ;;
     arm-*)
       # We don't have any bigendian build tools so we only use this for ARM
+      container_hosts="x86_64 aarch64"
       container_image=debian-armhf-cross
       container_cross_cc=arm-linux-gnueabihf-gcc
       ;;
     cris-*)
+      container_hosts=x86_64
       container_image=fedora-cris-cross
       container_cross_cc=cris-linux-gnu-gcc
       ;;
     hppa-*)
+      container_hosts=x86_64
       container_image=debian-hppa-cross
       container_cross_cc=hppa-linux-gnu-gcc
       ;;
     i386-*)
+      container_hosts=x86_64
       container_image=fedora-i386-cross
       container_cross_cc=gcc
       ;;
     m68k-*)
+      container_hosts=x86_64
       container_image=debian-m68k-cross
       container_cross_cc=m68k-linux-gnu-gcc
       ;;
     mips64el-*)
+      container_hosts=x86_64
       container_image=debian-mips64el-cross
       container_cross_cc=mips64el-linux-gnuabi64-gcc
       ;;
     mips64-*)
+      container_hosts=x86_64
       container_image=debian-mips64-cross
       container_cross_cc=mips64-linux-gnuabi64-gcc
       ;;
     mipsel-*)
+      container_hosts=x86_64
       container_image=debian-mipsel-cross
       container_cross_cc=mipsel-linux-gnu-gcc
       ;;
     mips-*)
+      container_hosts=x86_64
       container_image=debian-mips-cross
       container_cross_cc=mips-linux-gnu-gcc
       ;;
     ppc-*|ppc64abi32-*)
+      container_hosts=x86_64
       container_image=debian-powerpc-cross
       container_cross_cc=powerpc-linux-gnu-gcc
       ;;
     ppc64-*)
+      container_hosts=x86_64
       container_image=debian-ppc64-cross
       container_cross_cc=powerpc64-linux-gnu-gcc
       ;;
     ppc64le-*)
+      container_hosts=x86_64
       container_image=debian-ppc64el-cross
       container_cross_cc=powerpc64le-linux-gnu-gcc
       ;;
     riscv64-*)
+      container_hosts=x86_64
       container_image=debian-riscv64-cross
       container_cross_cc=riscv64-linux-gnu-gcc
       ;;
     s390x-*)
+      container_hosts=x86_64
       container_image=debian-s390x-cross
       container_cross_cc=s390x-linux-gnu-gcc
       ;;
     sh4-*)
+      container_hosts=x86_64
       container_image=debian-sh4-cross
       container_cross_cc=sh4-linux-gnu-gcc
       ;;
     sparc64-*)
+      container_hosts=x86_64
       container_image=debian-sparc64-cross
       container_cross_cc=sparc64-linux-gnu-gcc
       ;;
     xtensa*-softmmu)
+      container_hosts=x86_64
       container_image=debian-xtensa-cross
 
       # default to the dc232b cpu
@@ -265,7 +284,11 @@ for target in $target_list; do
   done
 
   if test $got_cross_cc = no && test "$container" != no && test -n "$container_image"; then
-    echo "DOCKER_IMAGE=$container_image" >> $config_target_mak
-    echo "DOCKER_CROSS_CC_GUEST=$container_cross_cc" >> $config_target_mak
+      for host in $container_hosts; do
+          if test "$host" = "$ARCH"; then
+              echo "DOCKER_IMAGE=$container_image" >> $config_target_mak
+              echo "DOCKER_CROSS_CC_GUEST=$container_cross_cc" >> $config_target_mak
+          fi
+      done
   fi
 done
-- 
2.20.1

[PATCH v1 4/6] tests/tcg/configure.sh: make sure we pick up x86_64 cross compilers

[Alex Bennée]

While it's hard to find cross compilers packaged for arches other than
x86_64 the same cannot be said for the x86_64 compiler which is
available on Debians i386, arm64 and ppc64el release architectures.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/tcg/configure.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
index 87a9f24b20..90fd81f506 100755
--- a/tests/tcg/configure.sh
+++ b/tests/tcg/configure.sh
@@ -198,6 +198,11 @@ for target in $target_list; do
       container_image=debian-sparc64-cross
       container_cross_cc=sparc64-linux-gnu-gcc
       ;;
+    x86_64-*)
+      container_hosts="aarch64 ppc64el x86_64"
+      container_image=debian-amd64-cross
+      container_cross_cc=x86_64-linux-gnu-gcc
+      ;;
     xtensa*-softmmu)
       container_hosts=x86_64
       container_image=debian-xtensa-cross
-- 
2.20.1

[PATCH v1 5/6] tests/tcg/i386: expand .data sections for system tests

[Alex Bennée]

Newer compilers might end up putting some data in .data.rel.local
which was getting skipped resulting in hilarious confusion on some
tests. Fix that.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/tcg/i386/system/kernel.ld | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/tcg/i386/system/kernel.ld b/tests/tcg/i386/system/kernel.ld
index 92de525e93..27ea5bbe04 100644
--- a/tests/tcg/i386/system/kernel.ld
+++ b/tests/tcg/i386/system/kernel.ld
@@ -12,7 +12,7 @@ SECTIONS {
 	}
 
 	.data : {
-		*(.data)
+		*(.data*)
 		__load_en = .;
 	}
 
-- 
2.20.1

Re: [PATCH v1 5/6] tests/tcg/i386: expand .data sections for system tests

[Thomas Huth]

On 29/03/2021 13.03, Alex Bennée wrote:
> Newer compilers might end up putting some data in .data.rel.local
> which was getting skipped resulting in hilarious confusion on some
> tests. Fix that.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   tests/tcg/i386/system/kernel.ld | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/tcg/i386/system/kernel.ld b/tests/tcg/i386/system/kernel.ld
> index 92de525e93..27ea5bbe04 100644
> --- a/tests/tcg/i386/system/kernel.ld
> +++ b/tests/tcg/i386/system/kernel.ld
> @@ -12,7 +12,7 @@ SECTIONS {
>   	}
>   
>   	.data : {
> -		*(.data)
> +		*(.data*)
>   		__load_en = .;
>   	}
>   
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v1 5/6] tests/tcg/i386: expand .data sections for system tests

[Richard Henderson]

On 3/29/21 5:03 AM, Alex Bennée wrote:
> Newer compilers might end up putting some data in .data.rel.local
> which was getting skipped resulting in hilarious confusion on some
> tests. Fix that.
> 
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> ---
>   tests/tcg/i386/system/kernel.ld | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

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

r~

[PATCH v1 6/6] tests/i386/test-i386: try and correct constrain 16 bit call (WIP)

[Alex Bennée]

Newer compilers complain about non-matching constraints:

  test-i386.c:1393:5: error: asm operand 2 probably doesn’t match constraints [-Werror]
   1393 |     asm volatile ("lcall %1, %2"
        |     ^~~

It seems the plain "i" immediate constraint is a little too lax here.
What we are actually dealing with is a 32 bit offset into a particular
segment. I think that is "Ts" (Address operand without segment
register).

[AJB: however this just seems to push it a bit further down the road:

test-i386.c:1393: Error: operand type mismatch for `lcall'

Inline ASM constraints are hard :-/]

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/tcg/i386/test-i386.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/tcg/i386/test-i386.c b/tests/tcg/i386/test-i386.c
index 18d5609665..6d8aeccfb5 100644
--- a/tests/tcg/i386/test-i386.c
+++ b/tests/tcg/i386/test-i386.c
@@ -1392,15 +1392,15 @@ void test_code16(void)
     /* call the first function */
     asm volatile ("lcall %1, %2"
                   : "=a" (res)
-                  : "i" (MK_SEL(1)), "i" (&code16_func1): "memory", "cc");
+                  : "i" (MK_SEL(1)), "Ts" (&code16_func1): "memory", "cc");
     printf("func1() = 0x%08x\n", res);
     asm volatile ("lcall %2, %3"
                   : "=a" (res), "=c" (res2)
-                  : "i" (MK_SEL(1)), "i" (&code16_func2): "memory", "cc");
+                  : "i" (MK_SEL(1)), "Ts" (&code16_func2): "memory", "cc");
     printf("func2() = 0x%08x spdec=%d\n", res, res2);
     asm volatile ("lcall %1, %2"
                   : "=a" (res)
-                  : "i" (MK_SEL(1)), "i" (&code16_func3): "memory", "cc");
+                  : "i" (MK_SEL(1)), "Ts" (&code16_func3): "memory", "cc");
     printf("func3() = 0x%08x\n", res);
 }
 #endif
-- 
2.20.1

Re: [PATCH v1 6/6] tests/i386/test-i386: try and correct constrain 16 bit call (WIP)

[Richard Henderson]

On 3/29/21 5:03 AM, Alex Bennée wrote:
> Newer compilers complain about non-matching constraints:
> 
>    test-i386.c:1393:5: error: asm operand 2 probably doesn’t match constraints [-Werror]
>     1393 |     asm volatile ("lcall %1, %2"
>          |     ^~~

The problem is that the newer compiler is defaulting to -fpie, so &code16_func1 
is not a constant.

Need to add -fno-pie to compile this.  Which of course adds the confusion of 
old compilers that don't support that.  We have CFLAGS_NOPIE in configure, but 
that doesn't directly help cross-compile.


r~

Re: [PATCH v1 6/6] tests/i386/test-i386: try and correct constrain 16 bit call (WIP)

[Alex Bennée]

Hmm actually the fedora-i386-cross image is:

  gcc (GCC) 10.2.1 20201125 (Red Hat 10.2.1-9)

with CROSS_CC_GUEST_CFLAGS=-m32 so I wonder what the difference is
between that and:

  i686-linux-gnu-gcc -m32

i686-linux-gnu-gcc --version
i686-linux-gnu-gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

They are both pretty new.

On Tue, 30 Mar 2021 at 16:58, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 3/29/21 5:03 AM, Alex Bennée wrote:
> > Newer compilers complain about non-matching constraints:
> >
> >    test-i386.c:1393:5: error: asm operand 2 probably doesn’t match constraints [-Werror]
> >     1393 |     asm volatile ("lcall %1, %2"
> >          |     ^~~
>
> The problem is that the newer compiler is defaulting to -fpie, so &code16_func1
> is not a constant.
>
> Need to add -fno-pie to compile this.  Which of course adds the confusion of
> old compilers that don't support that.  We have CFLAGS_NOPIE in configure, but
> that doesn't directly help cross-compile.
>
>
> r~



-- 
Alex Bennée
KVM/QEMU Hacker for Linaro

Re: [PATCH v1 6/6] tests/i386/test-i386: try and correct constrain 16 bit call (WIP)

[Richard Henderson]

On 3/30/21 12:20 PM, Alex Bennée wrote:
> Hmm actually the fedora-i386-cross image is:
> 
>    gcc (GCC) 10.2.1 20201125 (Red Hat 10.2.1-9)
> 
> with CROSS_CC_GUEST_CFLAGS=-m32 so I wonder what the difference is
> between that and:
> 
>    i686-linux-gnu-gcc -m32
> 
> i686-linux-gnu-gcc --version
> i686-linux-gnu-gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
> Copyright (C) 2019 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 
> They are both pretty new.

The difference, I'm sure, is a local distro patch enabling -fpie by default.

I'm hoping that we can just use

--- a/tests/tcg/i386/Makefile.target
+++ b/tests/tcg/i386/Makefile.target
@@ -10,6 +10,9 @@ ALL_X86_TESTS=$(I386_SRCS:.c=)
  SKIP_I386_TESTS=test-i386-ssse3
  X86_64_TESTS:=$(filter test-i386-ssse3, $(ALL_X86_TESTS))

+# test-i386 has non-pic calls into 16-bit mode
+test-i386: CFLAGS += -fno-pie
+
  test-i386-sse-exceptions: CFLAGS += -msse4.1 -mfpmath=sse
  run-test-i386-sse-exceptions: QEMU_OPTS += -cpu max
  run-plugin-test-i386-sse-exceptions-%: QEMU_OPTS += -cpu max

Checking with gitlab to see if our oldest host compiler accepts it.


r~

Re: [PATCH v1 6/6] tests/i386/test-i386: try and correct constrain 16 bit call (WIP)

[Peter Maydell]

On Tue, 30 Mar 2021 at 21:27, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 3/30/21 12:20 PM, Alex Bennée wrote:
> > Hmm actually the fedora-i386-cross image is:
> >
> >    gcc (GCC) 10.2.1 20201125 (Red Hat 10.2.1-9)
> >
> > with CROSS_CC_GUEST_CFLAGS=-m32 so I wonder what the difference is
> > between that and:
> >
> >    i686-linux-gnu-gcc -m32
> >
> > i686-linux-gnu-gcc --version
> > i686-linux-gnu-gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
> > Copyright (C) 2019 Free Software Foundation, Inc.
> > This is free software; see the source for copying conditions.  There is NO
> > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> >
> > They are both pretty new.
>
> The difference, I'm sure, is a local distro patch enabling -fpie by default.
>
> I'm hoping that we can just use
>
> --- a/tests/tcg/i386/Makefile.target
> +++ b/tests/tcg/i386/Makefile.target
> @@ -10,6 +10,9 @@ ALL_X86_TESTS=$(I386_SRCS:.c=)
>   SKIP_I386_TESTS=test-i386-ssse3
>   X86_64_TESTS:=$(filter test-i386-ssse3, $(ALL_X86_TESTS))
>
> +# test-i386 has non-pic calls into 16-bit mode
> +test-i386: CFLAGS += -fno-pie
> +
>   test-i386-sse-exceptions: CFLAGS += -msse4.1 -mfpmath=sse
>   run-test-i386-sse-exceptions: QEMU_OPTS += -cpu max
>   run-plugin-test-i386-sse-exceptions-%: QEMU_OPTS += -cpu max

configure will set CFLAGS_NOPIE in config-host.mak, but I don't know
if you have access to that here. (It does suggest that there are cases
where -fno-pie isn't available, though I guess they might be non-x86.)

-- PMM

Re: [PATCH v1 6/6] tests/i386/test-i386: try and correct constrain 16 bit call (WIP)

[Richard Henderson]

On 3/30/21 2:46 PM, Peter Maydell wrote:
> On Tue, 30 Mar 2021 at 21:27, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> On 3/30/21 12:20 PM, Alex Bennée wrote:
>>> Hmm actually the fedora-i386-cross image is:
>>>
>>>     gcc (GCC) 10.2.1 20201125 (Red Hat 10.2.1-9)
>>>
>>> with CROSS_CC_GUEST_CFLAGS=-m32 so I wonder what the difference is
>>> between that and:
>>>
>>>     i686-linux-gnu-gcc -m32
>>>
>>> i686-linux-gnu-gcc --version
>>> i686-linux-gnu-gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
>>> Copyright (C) 2019 Free Software Foundation, Inc.
>>> This is free software; see the source for copying conditions.  There is NO
>>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>>>
>>> They are both pretty new.
>>
>> The difference, I'm sure, is a local distro patch enabling -fpie by default.
>>
>> I'm hoping that we can just use
>>
>> --- a/tests/tcg/i386/Makefile.target
>> +++ b/tests/tcg/i386/Makefile.target
>> @@ -10,6 +10,9 @@ ALL_X86_TESTS=$(I386_SRCS:.c=)
>>    SKIP_I386_TESTS=test-i386-ssse3
>>    X86_64_TESTS:=$(filter test-i386-ssse3, $(ALL_X86_TESTS))
>>
>> +# test-i386 has non-pic calls into 16-bit mode
>> +test-i386: CFLAGS += -fno-pie
>> +
>>    test-i386-sse-exceptions: CFLAGS += -msse4.1 -mfpmath=sse
>>    run-test-i386-sse-exceptions: QEMU_OPTS += -cpu max
>>    run-plugin-test-i386-sse-exceptions-%: QEMU_OPTS += -cpu max
> 
> configure will set CFLAGS_NOPIE in config-host.mak, but I don't know
> if you have access to that here. (It does suggest that there are cases
> where -fno-pie isn't available, though I guess they might be non-x86.)

I think it's merely old compilers.  We do have to be worried about old host 
compilers here.  I'm hoping that gitlab will find out if our oldest docker 
build will find out one way or another.


r~

Re: [PATCH for 6.0-rc2? v1 0/6] check-tcg for non-x86 hosts

[no-reply]

Patchew URL: https://patchew.org/QEMU/20210329110303.15235-1-alex.bennee@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210329110303.15235-1-alex.bennee@linaro.org
Subject: [PATCH for 6.0-rc2? v1 0/6] check-tcg for non-x86 hosts

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210329110303.15235-1-alex.bennee@linaro.org -> patchew/20210329110303.15235-1-alex.bennee@linaro.org
Switched to a new branch 'test'
80c8848 tests/i386/test-i386: try and correct constrain 16 bit call (WIP)
4288d31 tests/tcg/i386: expand .data sections for system tests
0f797f0 tests/tcg/configure.sh: make sure we pick up x86_64 cross compilers
2049387 tests/tcg: add concept of container_hosts
24cf690 tests/docker: don't set DOCKER_REGISTRY on non-x86_64
399a62b tests/tcg: update the defaults for x86 compilers

=== OUTPUT BEGIN ===
1/6 Checking commit 399a62bdaede (tests/tcg: update the defaults for x86 compilers)
2/6 Checking commit 24cf690e3b06 (tests/docker: don't set DOCKER_REGISTRY on non-x86_64)
3/6 Checking commit 204938792670 (tests/tcg: add concept of container_hosts)
WARNING: line over 80 characters
#145: FILE: tests/tcg/configure.sh:290:
+              echo "DOCKER_CROSS_CC_GUEST=$container_cross_cc" >> $config_target_mak

total: 0 errors, 1 warnings, 119 lines checked

Patch 3/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/6 Checking commit 0f797f01c762 (tests/tcg/configure.sh: make sure we pick up x86_64 cross compilers)
5/6 Checking commit 4288d31858fd (tests/tcg/i386: expand .data sections for system tests)
6/6 Checking commit 80c884804e4a (tests/i386/test-i386: try and correct constrain 16 bit call (WIP))
ERROR: spaces required around that ':' (ctx:VxW)
#39: FILE: tests/tcg/i386/test-i386.c:1395:
+                  : "i" (MK_SEL(1)), "Ts" (&code16_func1): "memory", "cc");
                                                          ^

ERROR: spaces required around that ':' (ctx:VxW)
#44: FILE: tests/tcg/i386/test-i386.c:1399:
+                  : "i" (MK_SEL(1)), "Ts" (&code16_func2): "memory", "cc");
                                                          ^

ERROR: spaces required around that ':' (ctx:VxW)
#49: FILE: tests/tcg/i386/test-i386.c:1403:
+                  : "i" (MK_SEL(1)), "Ts" (&code16_func3): "memory", "cc");
                                                          ^

total: 3 errors, 0 warnings, 18 lines checked

Patch 6/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210329110303.15235-1-alex.bennee@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com