[RFC PATCH 0/3] tests/tcg/ppc64le: paddi tests

[RFC PATCH 0/3] tests/tcg/ppc64le: paddi tests

[matheus.ferst]

From: Matheus Ferst <matheus.ferst@eldorado.org.br>

Based-on: <20210413211129.457272-1-luis.pires@eldorado.org.br>

This series adds gcc-10 based images to enable the build of tests with Power10
instructions. Then two tests for paddi are added:
- The first one checks a weird behavior observed on POWER10 Functional Simulator
  1.1.0, where the 34-bit immediate is treated as a 32-bits one;
- The second one exercises the R=1 path of paddi, where CIA is used instead of RA.
  The test is failing with the current implementation because we use cpu_nip,
  which is not updated all the time. Luis already has the fix, it should be
  applied on the next version of his patch series.

The main reason to submit this patch as an RFC first is the docker part. I would
lie if I tell you that I understand half of what is going on there.
 - 'make docker-test-tcg' fails, but apparently on unrelated things;
 - 'make docker-run-test-tcg@debian-ppc64el-cross' passes, but it looks
   like the test is skipped?
 - 'make check-tcg' runs the test and passes (with the fix in place for the
   second).

Finally, get_maintainer.pl found no maintainers for
tests/tcg/ppc64{,le}/Makefile.target. Would it be Mr. Gibson?

Thanks,
Matheus K. Ferst

Matheus Ferst (3):
  tests/docker: gcc-10 based images for ppc64{,le} tests
  tests/tcg/ppc64le: load 33-bits constant with paddi
  tests/tcg/ppc64le: R=1 test for paddi

 tests/docker/Makefile.include                 |  4 +++
 .../debian-ppc64-test-cross.docker            | 13 ++++++++++
 .../debian-ppc64el-test-cross.docker          | 17 ++++++++++++
 tests/tcg/configure.sh                        | 12 ++++++---
 tests/tcg/ppc64/Makefile.target               |  6 +++++
 tests/tcg/ppc64le/Makefile.target             |  6 +++++
 tests/tcg/ppc64le/pla.c                       | 26 +++++++++++++++++++
 tests/tcg/ppc64le/pli_33bits.c                | 22 ++++++++++++++++
 8 files changed, 102 insertions(+), 4 deletions(-)
 create mode 100644 tests/docker/dockerfiles/debian-ppc64-test-cross.docker
 create mode 100644 tests/docker/dockerfiles/debian-ppc64el-test-cross.docker
 create mode 100644 tests/tcg/ppc64le/pla.c
 create mode 100644 tests/tcg/ppc64le/pli_33bits.c

-- 
2.25.1

[RFC PATCH 1/3] tests/docker: gcc-10 based images for ppc64{, le} tests

[matheus.ferst]

From: Matheus Ferst <matheus.ferst@eldorado.org.br>

A newer compiler is needed to build tests for Power10 instructions. As
done for arm64 on c729a99d2701, new '-test-cross' images are created for
ppc64 and ppc64le. As done on 936fda4d771f, a test for compiler support
is added to verify that the toolchain in use has '-mpower10'.

Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
 tests/docker/Makefile.include                   |  4 ++++
 .../dockerfiles/debian-ppc64-test-cross.docker  | 13 +++++++++++++
 .../debian-ppc64el-test-cross.docker            | 17 +++++++++++++++++
 tests/tcg/configure.sh                          | 12 ++++++++----
 4 files changed, 42 insertions(+), 4 deletions(-)
 create mode 100644 tests/docker/dockerfiles/debian-ppc64-test-cross.docker
 create mode 100644 tests/docker/dockerfiles/debian-ppc64el-test-cross.docker

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 9f464cb92c..1f8941d290 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -152,10 +152,14 @@ docker-image-debian-sparc64-cross: docker-image-debian10
 docker-image-debian-tricore-cross: docker-image-debian10
 docker-image-debian-all-test-cross: docker-image-debian10
 docker-image-debian-arm64-test-cross: docker-image-debian11
+docker-image-debian-ppc64-test-cross: docker-image-debian11
+docker-image-debian-ppc64el-test-cross: docker-image-debian11
 
 # These images may be good enough for building tests but not for test builds
 DOCKER_PARTIAL_IMAGES += debian-alpha-cross
 DOCKER_PARTIAL_IMAGES += debian-arm64-test-cross
+DOCKER_PARTIAL_IMAGES += debian-ppc64-test-cross
+DOCKER_PARTIAL_IMAGES += debian-ppc64el-test-cross
 DOCKER_PARTIAL_IMAGES += debian-hppa-cross
 DOCKER_PARTIAL_IMAGES += debian-m68k-cross debian-mips64-cross
 DOCKER_PARTIAL_IMAGES += debian-powerpc-cross debian-ppc64-cross
diff --git a/tests/docker/dockerfiles/debian-ppc64-test-cross.docker b/tests/docker/dockerfiles/debian-ppc64-test-cross.docker
new file mode 100644
index 0000000000..66abfdeb47
--- /dev/null
+++ b/tests/docker/dockerfiles/debian-ppc64-test-cross.docker
@@ -0,0 +1,13 @@
+#
+# Docker ppc64 cross-compiler target (tests only)
+#
+# This docker target builds on the debian Bullseye base image.
+#
+FROM qemu/debian11
+
+# Add the foreign architecture we want and install dependencies
+RUN dpkg --add-architecture ppc64
+RUN apt update && \
+    DEBIAN_FRONTEND=noninteractive eatmydata \
+        apt install -y --no-install-recommends \
+        libc6-dev-ppc64-cross gcc-10-powerpc64-linux-gnu
diff --git a/tests/docker/dockerfiles/debian-ppc64el-test-cross.docker b/tests/docker/dockerfiles/debian-ppc64el-test-cross.docker
new file mode 100644
index 0000000000..7582508467
--- /dev/null
+++ b/tests/docker/dockerfiles/debian-ppc64el-test-cross.docker
@@ -0,0 +1,17 @@
+#
+# Docker ppc64el cross-compiler target (tests only)
+#
+# This docker target builds on the debian Bullseye base image.
+#
+FROM qemu/debian11
+
+# Add the foreign architecture we want and install dependencies
+RUN dpkg --add-architecture ppc64el
+RUN apt update && \
+    DEBIAN_FRONTEND=noninteractive eatmydata \
+        apt install -y --no-install-recommends \
+        crossbuild-essential-ppc64el gcc-10-powerpc64le-linux-gnu
+
+# Specify the cross prefix for this image (see tests/docker/common.rc)
+#ENV QEMU_CONFIGURE_OPTS --cross-prefix=powerpc64le-linux-gnu-
+#ENV DEF_TARGET_LIST ppc64-softmmu,ppc64-linux-user,ppc64abi32-linux-user
diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
index fa1a4261a4..5f5db91a01 100755
--- a/tests/tcg/configure.sh
+++ b/tests/tcg/configure.sh
@@ -170,13 +170,13 @@ for target in $target_list; do
       ;;
     ppc64-*)
       container_hosts=x86_64
-      container_image=debian-ppc64-cross
-      container_cross_cc=powerpc64-linux-gnu-gcc
+      container_image=debian-ppc64-test-cross
+      container_cross_cc=powerpc64-linux-gnu-gcc-10
       ;;
     ppc64le-*)
       container_hosts=x86_64
-      container_image=debian-ppc64el-cross
-      container_cross_cc=powerpc64le-linux-gnu-gcc
+      container_image=debian-ppc64el-test-cross
+      container_cross_cc=powerpc64le-linux-gnu-gcc-10
       ;;
     riscv64-*)
       container_hosts=x86_64
@@ -280,6 +280,10 @@ for target in $target_list; do
                -mpower8-vector -o $TMPE $TMPC; then
                 echo "CROSS_CC_HAS_POWER8_VECTOR=y" >> $config_target_mak
             fi
+            if do_compiler "$target_compiler" $target_compiler_cflags \
+               -mpower10 -o $TMPE $TMPC; then
+                echo "CROSS_CC_HAS_POWER10=y" >> $config_target_mak
+            fi
         ;;
         i386-linux-user)
             if do_compiler "$target_compiler" $target_compiler_cflags \
-- 
2.25.1

[RFC PATCH 2/3] tests/tcg/ppc64le: load 33-bits constant with paddi

[matheus.ferst]

From: Matheus Ferst <matheus.ferst@eldorado.org.br>

This test checks that we can correctly load a 33-bit constant and its
two's complement. At least until version 1.1-0, POWER10 Functional
Simulation fails this test, processing the immediate as if it were
32-bits instead of 34, so it's probably something to keep an eye on.

Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
 tests/tcg/ppc64/Makefile.target   |  5 +++++
 tests/tcg/ppc64le/Makefile.target |  5 +++++
 tests/tcg/ppc64le/pli_33bits.c    | 22 ++++++++++++++++++++++
 3 files changed, 32 insertions(+)
 create mode 100644 tests/tcg/ppc64le/pli_33bits.c

diff --git a/tests/tcg/ppc64/Makefile.target b/tests/tcg/ppc64/Makefile.target
index 0c6a4585fc..6eccd2c06f 100644
--- a/tests/tcg/ppc64/Makefile.target
+++ b/tests/tcg/ppc64/Makefile.target
@@ -10,4 +10,9 @@ PPC64_TESTS=bcdsub
 endif
 bcdsub: CFLAGS += -mpower8-vector
 
+ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_POWER10),)
+PPC64LE_TESTS += pli_33bits
+endif
+pli_33bits: CFLAGS += -mpower10
+
 TESTS += $(PPC64_TESTS)
diff --git a/tests/tcg/ppc64le/Makefile.target b/tests/tcg/ppc64le/Makefile.target
index 1acfcff94a..2003eab2df 100644
--- a/tests/tcg/ppc64le/Makefile.target
+++ b/tests/tcg/ppc64le/Makefile.target
@@ -9,4 +9,9 @@ PPC64LE_TESTS=bcdsub
 endif
 bcdsub: CFLAGS += -mpower8-vector
 
+ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_POWER10),)
+PPC64LE_TESTS += pli_33bits
+endif
+pli_33bits: CFLAGS += -mpower10
+
 TESTS += $(PPC64LE_TESTS)
diff --git a/tests/tcg/ppc64le/pli_33bits.c b/tests/tcg/ppc64le/pli_33bits.c
new file mode 100644
index 0000000000..848cbce165
--- /dev/null
+++ b/tests/tcg/ppc64le/pli_33bits.c
@@ -0,0 +1,22 @@
+#include <assert.h>
+#include <unistd.h>
+#include <signal.h>
+
+int main(void)
+{
+    long int var;
+    struct sigaction action;
+
+    action.sa_handler = _exit;
+    sigaction(SIGABRT, &action, NULL);
+
+    asm(" pli %0,0x1FFFFFFFF\n"
+        : "=r"(var));
+    assert(var == 0x1FFFFFFFF);
+
+    asm(" pli %0,-0x1FFFFFFFF\n"
+       : "=r"(var));
+    assert(var == -0x1FFFFFFFF);
+
+    return 0;
+}
-- 
2.25.1

[RFC PATCH 3/3] tests/tcg/ppc64le: R=1 test for paddi

[matheus.ferst]

From: Matheus Ferst <matheus.ferst@eldorado.org.br>

This test exercise the R=1 path of paddi implementation using the
extended mnemonic "pla".

Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
 tests/tcg/ppc64/Makefile.target   |  3 ++-
 tests/tcg/ppc64le/Makefile.target |  3 ++-
 tests/tcg/ppc64le/pla.c           | 26 ++++++++++++++++++++++++++
 3 files changed, 30 insertions(+), 2 deletions(-)
 create mode 100644 tests/tcg/ppc64le/pla.c

diff --git a/tests/tcg/ppc64/Makefile.target b/tests/tcg/ppc64/Makefile.target
index 6eccd2c06f..a6cd7a21b2 100644
--- a/tests/tcg/ppc64/Makefile.target
+++ b/tests/tcg/ppc64/Makefile.target
@@ -11,8 +11,9 @@ endif
 bcdsub: CFLAGS += -mpower8-vector
 
 ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_POWER10),)
-PPC64LE_TESTS += pli_33bits
+PPC64LE_TESTS += pli_33bits pla
 endif
 pli_33bits: CFLAGS += -mpower10
+pla: CFLAGS += -mpower10
 
 TESTS += $(PPC64_TESTS)
diff --git a/tests/tcg/ppc64le/Makefile.target b/tests/tcg/ppc64le/Makefile.target
index 2003eab2df..db92b2ec99 100644
--- a/tests/tcg/ppc64le/Makefile.target
+++ b/tests/tcg/ppc64le/Makefile.target
@@ -10,8 +10,9 @@ endif
 bcdsub: CFLAGS += -mpower8-vector
 
 ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_POWER10),)
-PPC64LE_TESTS += pli_33bits
+PPC64LE_TESTS += pli_33bits pla
 endif
 pli_33bits: CFLAGS += -mpower10
+pla: CFLAGS += -mpower10
 
 TESTS += $(PPC64LE_TESTS)
diff --git a/tests/tcg/ppc64le/pla.c b/tests/tcg/ppc64le/pla.c
new file mode 100644
index 0000000000..4826579216
--- /dev/null
+++ b/tests/tcg/ppc64le/pla.c
@@ -0,0 +1,26 @@
+#include <assert.h>
+#include <unistd.h>
+#include <signal.h>
+
+int main(void)
+{
+    long unsigned int label, addr;
+    struct sigaction action;
+
+    action.sa_handler = _exit;
+    sigaction(SIGABRT, &action, NULL);
+
+    asm("insn:\n"
+        " lis    %0, insn@highest\n"
+        " addi   %0, %0, insn@higher\n"
+        " rldicr %0, %0, 32, 31\n"
+        " oris   %0, %0, insn@h\n"
+        " ori    %0, %0, insn@l\n"
+        " pla    %1, %2\n"
+        : "=r" (label), "=r" (addr)
+        : "i" (-5 * 4)); /* number of instruction between label and pla */
+    assert(addr == label);
+
+    return 0;
+}
+
-- 
2.25.1

Re: [RFC PATCH 0/3] tests/tcg/ppc64le: paddi tests

[David Gibson]

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

On Thu, Apr 15, 2021 at 06:41:35PM -0300, matheus.ferst@eldorado.org.br wrote:
> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
> 
> Based-on: <20210413211129.457272-1-luis.pires@eldorado.org.br>

First things first: it's unclear to me if this is testing stuff that's
already merged, or it's speculative tests for the in-progress prefixed
instruction stuff.  i.e. If these tests are applied right now, will
they pass?

> This series adds gcc-10 based images to enable the build of tests with Power10
> instructions. Then two tests for paddi are added:
> - The first one checks a weird behavior observed on POWER10 Functional Simulator
>   1.1.0, where the 34-bit immediate is treated as a 32-bits one;
> - The second one exercises the R=1 path of paddi, where CIA is used instead of RA.
>   The test is failing with the current implementation because we use cpu_nip,
>   which is not updated all the time. Luis already has the fix, it should be
>   applied on the next version of his patch series.
> 
> The main reason to submit this patch as an RFC first is the docker part. I would
> lie if I tell you that I understand half of what is going on there.
>  - 'make docker-test-tcg' fails, but apparently on unrelated things;
>  - 'make docker-run-test-tcg@debian-ppc64el-cross' passes, but it looks
>    like the test is skipped?
>  - 'make check-tcg' runs the test and passes (with the fix in place for the
>    second).

What sort of host was that on?  Unfortunately 'make check-tcg' has
been broken on a POWER host for some time, and I've never had time to
look into it.

> 
> Finally, get_maintainer.pl found no maintainers for
> tests/tcg/ppc64{,le}/Makefile.target. Would it be Mr. Gibson?

Uh... sorta?  I also don't know much about what's going on here, but
I'm probably maintainer by default.


> 
> Thanks,
> Matheus K. Ferst
> 
> Matheus Ferst (3):
>   tests/docker: gcc-10 based images for ppc64{,le} tests
>   tests/tcg/ppc64le: load 33-bits constant with paddi
>   tests/tcg/ppc64le: R=1 test for paddi
> 
>  tests/docker/Makefile.include                 |  4 +++
>  .../debian-ppc64-test-cross.docker            | 13 ++++++++++
>  .../debian-ppc64el-test-cross.docker          | 17 ++++++++++++
>  tests/tcg/configure.sh                        | 12 ++++++---
>  tests/tcg/ppc64/Makefile.target               |  6 +++++
>  tests/tcg/ppc64le/Makefile.target             |  6 +++++
>  tests/tcg/ppc64le/pla.c                       | 26 +++++++++++++++++++
>  tests/tcg/ppc64le/pli_33bits.c                | 22 ++++++++++++++++
>  8 files changed, 102 insertions(+), 4 deletions(-)
>  create mode 100644 tests/docker/dockerfiles/debian-ppc64-test-cross.docker
>  create mode 100644 tests/docker/dockerfiles/debian-ppc64el-test-cross.docker
>  create mode 100644 tests/tcg/ppc64le/pla.c
>  create mode 100644 tests/tcg/ppc64le/pli_33bits.c
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC PATCH 1/3] tests/docker: gcc-10 based images for ppc64{, le} tests

[David Gibson]

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

On Thu, Apr 15, 2021 at 06:41:36PM -0300, matheus.ferst@eldorado.org.br wrote:
> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
> 
> A newer compiler is needed to build tests for Power10 instructions. As
> done for arm64 on c729a99d2701, new '-test-cross' images are created for
> ppc64 and ppc64le. As done on 936fda4d771f, a test for compiler support
> is added to verify that the toolchain in use has '-mpower10'.
> 
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---
>  tests/docker/Makefile.include                   |  4 ++++
>  .../dockerfiles/debian-ppc64-test-cross.docker  | 13 +++++++++++++
>  .../debian-ppc64el-test-cross.docker            | 17 +++++++++++++++++
>  tests/tcg/configure.sh                          | 12 ++++++++----
>  4 files changed, 42 insertions(+), 4 deletions(-)
>  create mode 100644 tests/docker/dockerfiles/debian-ppc64-test-cross.docker
>  create mode 100644 tests/docker/dockerfiles/debian-ppc64el-test-cross.docker
> 
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 9f464cb92c..1f8941d290 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -152,10 +152,14 @@ docker-image-debian-sparc64-cross: docker-image-debian10
>  docker-image-debian-tricore-cross: docker-image-debian10
>  docker-image-debian-all-test-cross: docker-image-debian10
>  docker-image-debian-arm64-test-cross: docker-image-debian11
> +docker-image-debian-ppc64-test-cross: docker-image-debian11
> +docker-image-debian-ppc64el-test-cross: docker-image-debian11
>  
>  # These images may be good enough for building tests but not for test builds
>  DOCKER_PARTIAL_IMAGES += debian-alpha-cross
>  DOCKER_PARTIAL_IMAGES += debian-arm64-test-cross
> +DOCKER_PARTIAL_IMAGES += debian-ppc64-test-cross
> +DOCKER_PARTIAL_IMAGES += debian-ppc64el-test-cross
>  DOCKER_PARTIAL_IMAGES += debian-hppa-cross
>  DOCKER_PARTIAL_IMAGES += debian-m68k-cross debian-mips64-cross
>  DOCKER_PARTIAL_IMAGES += debian-powerpc-cross debian-ppc64-cross

Why are you adding new images, rather than updating the existing
debian-powerpc-cross image?  I don't think you should need separate
ppc64 and ppc64el images, a single image with a gcc that can target
both should suffice.  (Also, it's typically ppc64le, not ppc64el,
which, yes, is different from what the mips and arm people do for no
particularly good reason).

> diff --git a/tests/docker/dockerfiles/debian-ppc64-test-cross.docker b/tests/docker/dockerfiles/debian-ppc64-test-cross.docker
> new file mode 100644
> index 0000000000..66abfdeb47
> --- /dev/null
> +++ b/tests/docker/dockerfiles/debian-ppc64-test-cross.docker
> @@ -0,0 +1,13 @@
> +#
> +# Docker ppc64 cross-compiler target (tests only)
> +#
> +# This docker target builds on the debian Bullseye base image.
> +#
> +FROM qemu/debian11
> +
> +# Add the foreign architecture we want and install dependencies
> +RUN dpkg --add-architecture ppc64
> +RUN apt update && \
> +    DEBIAN_FRONTEND=noninteractive eatmydata \
> +        apt install -y --no-install-recommends \
> +        libc6-dev-ppc64-cross gcc-10-powerpc64-linux-gnu
> diff --git a/tests/docker/dockerfiles/debian-ppc64el-test-cross.docker b/tests/docker/dockerfiles/debian-ppc64el-test-cross.docker
> new file mode 100644
> index 0000000000..7582508467
> --- /dev/null
> +++ b/tests/docker/dockerfiles/debian-ppc64el-test-cross.docker
> @@ -0,0 +1,17 @@
> +#
> +# Docker ppc64el cross-compiler target (tests only)
> +#
> +# This docker target builds on the debian Bullseye base image.
> +#
> +FROM qemu/debian11
> +
> +# Add the foreign architecture we want and install dependencies
> +RUN dpkg --add-architecture ppc64el
> +RUN apt update && \
> +    DEBIAN_FRONTEND=noninteractive eatmydata \
> +        apt install -y --no-install-recommends \
> +        crossbuild-essential-ppc64el gcc-10-powerpc64le-linux-gnu
> +
> +# Specify the cross prefix for this image (see tests/docker/common.rc)
> +#ENV QEMU_CONFIGURE_OPTS --cross-prefix=powerpc64le-linux-gnu-
> +#ENV DEF_TARGET_LIST ppc64-softmmu,ppc64-linux-user,ppc64abi32-linux-user
> diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
> index fa1a4261a4..5f5db91a01 100755
> --- a/tests/tcg/configure.sh
> +++ b/tests/tcg/configure.sh
> @@ -170,13 +170,13 @@ for target in $target_list; do
>        ;;
>      ppc64-*)
>        container_hosts=x86_64
> -      container_image=debian-ppc64-cross
> -      container_cross_cc=powerpc64-linux-gnu-gcc
> +      container_image=debian-ppc64-test-cross
> +      container_cross_cc=powerpc64-linux-gnu-gcc-10
>        ;;
>      ppc64le-*)
>        container_hosts=x86_64
> -      container_image=debian-ppc64el-cross
> -      container_cross_cc=powerpc64le-linux-gnu-gcc
> +      container_image=debian-ppc64el-test-cross
> +      container_cross_cc=powerpc64le-linux-gnu-gcc-10
>        ;;
>      riscv64-*)
>        container_hosts=x86_64
> @@ -280,6 +280,10 @@ for target in $target_list; do
>                 -mpower8-vector -o $TMPE $TMPC; then
>                  echo "CROSS_CC_HAS_POWER8_VECTOR=y" >> $config_target_mak
>              fi
> +            if do_compiler "$target_compiler" $target_compiler_cflags \
> +               -mpower10 -o $TMPE $TMPC; then
> +                echo "CROSS_CC_HAS_POWER10=y" >> $config_target_mak
> +            fi
>          ;;
>          i386-linux-user)
>              if do_compiler "$target_compiler" $target_compiler_cflags \

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC PATCH 1/3] tests/docker: gcc-10 based images for ppc64{,le} tests

[Alex Bennée]

David Gibson <david@gibson.dropbear.id.au> writes:

> On Thu, Apr 15, 2021 at 06:41:36PM -0300, matheus.ferst@eldorado.org.br wrote:
>> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
>> 
>> A newer compiler is needed to build tests for Power10 instructions. As
>> done for arm64 on c729a99d2701, new '-test-cross' images are created for
>> ppc64 and ppc64le. As done on 936fda4d771f, a test for compiler support
>> is added to verify that the toolchain in use has '-mpower10'.
>> 
>> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
>> ---
>>  tests/docker/Makefile.include                   |  4 ++++
>>  .../dockerfiles/debian-ppc64-test-cross.docker  | 13 +++++++++++++
>>  .../debian-ppc64el-test-cross.docker            | 17 +++++++++++++++++
>>  tests/tcg/configure.sh                          | 12 ++++++++----
>>  4 files changed, 42 insertions(+), 4 deletions(-)
>>  create mode 100644 tests/docker/dockerfiles/debian-ppc64-test-cross.docker
>>  create mode 100644 tests/docker/dockerfiles/debian-ppc64el-test-cross.docker
>> 
>> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
>> index 9f464cb92c..1f8941d290 100644
>> --- a/tests/docker/Makefile.include
>> +++ b/tests/docker/Makefile.include
>> @@ -152,10 +152,14 @@ docker-image-debian-sparc64-cross: docker-image-debian10
>>  docker-image-debian-tricore-cross: docker-image-debian10
>>  docker-image-debian-all-test-cross: docker-image-debian10
>>  docker-image-debian-arm64-test-cross: docker-image-debian11
>> +docker-image-debian-ppc64-test-cross: docker-image-debian11
>> +docker-image-debian-ppc64el-test-cross: docker-image-debian11
>>  
>>  # These images may be good enough for building tests but not for test builds
>>  DOCKER_PARTIAL_IMAGES += debian-alpha-cross
>>  DOCKER_PARTIAL_IMAGES += debian-arm64-test-cross
>> +DOCKER_PARTIAL_IMAGES += debian-ppc64-test-cross
>> +DOCKER_PARTIAL_IMAGES += debian-ppc64el-test-cross
>>  DOCKER_PARTIAL_IMAGES += debian-hppa-cross
>>  DOCKER_PARTIAL_IMAGES += debian-m68k-cross debian-mips64-cross
>>  DOCKER_PARTIAL_IMAGES += debian-powerpc-cross debian-ppc64-cross
>
> Why are you adding new images, rather than updating the existing
> debian-powerpc-cross image?

Right - we could consider renaming the image to debian-power-test-cross
just to make it clear it's just for building tests.

The final image would essentially be:

RUN apt update && \
    DEBIAN_FRONTEND=noninteractive eatmydata \
    apt install -y --no-install-recommends \
        gcc-powerpc-linux-gnu \
        libc6-dev-powerpc-cross \
        gcc-10-powerpc64-linux-gnu \
        libc6-dev-ppc64-cross \
        gcc-10-powerpc64le-linux-gnu \
        libc6-dev-ppc64le-cross 

> I don't think you should need separate
> ppc64 and ppc64el images, a single image with a gcc that can target
> both should suffice.  (Also, it's typically ppc64le, not ppc64el,
> which, yes, is different from what the mips and arm people do for no
> particularly good reason).
>
>> diff --git a/tests/docker/dockerfiles/debian-ppc64-test-cross.docker b/tests/docker/dockerfiles/debian-ppc64-test-cross.docker
>> new file mode 100644
>> index 0000000000..66abfdeb47
>> --- /dev/null
>> +++ b/tests/docker/dockerfiles/debian-ppc64-test-cross.docker
>> @@ -0,0 +1,13 @@
>> +#
>> +# Docker ppc64 cross-compiler target (tests only)
>> +#
>> +# This docker target builds on the debian Bullseye base image.
>> +#
>> +FROM qemu/debian11
>> +
>> +# Add the foreign architecture we want and install dependencies
>> +RUN dpkg --add-architecture ppc64

Adding a foreign architecture is only really required for getting cross
libraries for more complex things like QEMU, for tcg tests directly
including the libc is enough.
>> +
>> +# Add the foreign architecture we want and install dependencies
>> +RUN dpkg --add-architecture ppc64el
>> +RUN apt update && \
>> +    DEBIAN_FRONTEND=noninteractive eatmydata \
>> +        apt install -y --no-install-recommends \
>> +        crossbuild-essential-ppc64el gcc-10-powerpc64le-linux-gnu

include the libc instead of crossbuild which brings in a lot more than
we need for tests.

>> +
>> +# Specify the cross prefix for this image (see tests/docker/common.rc)
>> +#ENV QEMU_CONFIGURE_OPTS --cross-prefix=powerpc64le-linux-gnu-
>> +#ENV DEF_TARGET_LIST ppc64-softmmu,ppc64-linux-user,ppc64abi32-linux-user
>> diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
>> index fa1a4261a4..5f5db91a01 100755
>> --- a/tests/tcg/configure.sh
>> +++ b/tests/tcg/configure.sh
>> @@ -170,13 +170,13 @@ for target in $target_list; do
>>        ;;
>>      ppc64-*)
>>        container_hosts=x86_64
>> -      container_image=debian-ppc64-cross
>> -      container_cross_cc=powerpc64-linux-gnu-gcc
>> +      container_image=debian-ppc64-test-cross
>> +      container_cross_cc=powerpc64-linux-gnu-gcc-10
>>        ;;
>>      ppc64le-*)
>>        container_hosts=x86_64
>> -      container_image=debian-ppc64el-cross
>> -      container_cross_cc=powerpc64le-linux-gnu-gcc
>> +      container_image=debian-ppc64el-test-cross
>> +      container_cross_cc=powerpc64le-linux-gnu-gcc-10
>>        ;;

I don't know if it's possible to nest the cases but if it's not too
ugly bringing all the cases into one place and just differentiating the
container_cross_cc would be nice.

>>      riscv64-*)
>>        container_hosts=x86_64
>> @@ -280,6 +280,10 @@ for target in $target_list; do
>>                 -mpower8-vector -o $TMPE $TMPC; then
>>                  echo "CROSS_CC_HAS_POWER8_VECTOR=y" >> $config_target_mak
>>              fi
>> +            if do_compiler "$target_compiler" $target_compiler_cflags \
>> +               -mpower10 -o $TMPE $TMPC; then
>> +                echo "CROSS_CC_HAS_POWER10=y" >> $config_target_mak
>> +            fi
>>          ;;
>>          i386-linux-user)
>>              if do_compiler "$target_compiler" $target_compiler_cflags \


-- 
Alex Bennée

Re: [RFC PATCH 0/3] tests/tcg/ppc64le: paddi tests

[Matheus K. Ferst]

On 16/04/2021 00:52, David Gibson wrote:
> On Thu, Apr 15, 2021 at 06:41:35PM -0300, matheus.ferst@eldorado.org.br wrote:
>> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
>>
>> Based-on: <20210413211129.457272-1-luis.pires@eldorado.org.br>
> 
> First things first: it's unclear to me if this is testing stuff that's
> already merged, or it's speculative tests for the in-progress prefixed
> instruction stuff.  i.e. If these tests are applied right now, will
> they pass?

GCC-10 images can be used to test already merged Power10 instructions, 
such as brh/brw/brd, but I haven't writen tests for them (yet?). Both 
tests are targeting paddi, whose implementation is in-progress, so 
applying them now will fail. Maybe I should split the series? Patch 1 
for now, and Patch 2 and 3 when paddi are merged?

>> This series adds gcc-10 based images to enable the build of tests with Power10
>> instructions. Then two tests for paddi are added:
>> - The first one checks a weird behavior observed on POWER10 Functional Simulator
>>    1.1.0, where the 34-bit immediate is treated as a 32-bits one;
>> - The second one exercises the R=1 path of paddi, where CIA is used instead of RA.
>>    The test is failing with the current implementation because we use cpu_nip,
>>    which is not updated all the time. Luis already has the fix, it should be
>>    applied on the next version of his patch series.
>>
>> The main reason to submit this patch as an RFC first is the docker part. I would
>> lie if I tell you that I understand half of what is going on there.
>>   - 'make docker-test-tcg' fails, but apparently on unrelated things;
>>   - 'make docker-run-test-tcg@debian-ppc64el-cross' passes, but it looks
>>     like the test is skipped?
>>   - 'make check-tcg' runs the test and passes (with the fix in place for the
>>     second).
> 
> What sort of host was that on?  Unfortunately 'make check-tcg' has
> been broken on a POWER host for some time, and I've never had time to
> look into it.
> 

I'm testing on amd64, but I can also try on ppc64le.

>>
>> Finally, get_maintainer.pl found no maintainers for
>> tests/tcg/ppc64{,le}/Makefile.target. Would it be Mr. Gibson?
> 
> Uh... sorta?  I also don't know much about what's going on here, but
> I'm probably maintainer by default.
> 

So, should I update MAINTAINERS in this series?

-- 
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software Júnior
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>

Re: [RFC PATCH 0/3] tests/tcg/ppc64le: paddi tests

[David Gibson]

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

On Fri, Apr 16, 2021 at 11:13:48AM -0300, Matheus K. Ferst wrote:
> On 16/04/2021 00:52, David Gibson wrote:
> > On Thu, Apr 15, 2021 at 06:41:35PM -0300, matheus.ferst@eldorado.org.br wrote:
> > > From: Matheus Ferst <matheus.ferst@eldorado.org.br>
> > > 
> > > Based-on: <20210413211129.457272-1-luis.pires@eldorado.org.br>
> > 
> > First things first: it's unclear to me if this is testing stuff that's
> > already merged, or it's speculative tests for the in-progress prefixed
> > instruction stuff.  i.e. If these tests are applied right now, will
> > they pass?
> 
> GCC-10 images can be used to test already merged Power10 instructions, such
> as brh/brw/brd, but I haven't writen tests for them (yet?). Both tests are
> targeting paddi, whose implementation is in-progress, so applying them now
> will fail. Maybe I should split the series? Patch 1 for now, and Patch 2 and
> 3 when paddi are merged?

That sounds reasonable, as long as patch 1 does *something* visible
now (e.g. running existing tests with the new compiler).

> 
> > > This series adds gcc-10 based images to enable the build of tests with Power10
> > > instructions. Then two tests for paddi are added:
> > > - The first one checks a weird behavior observed on POWER10 Functional Simulator
> > >    1.1.0, where the 34-bit immediate is treated as a 32-bits one;
> > > - The second one exercises the R=1 path of paddi, where CIA is used instead of RA.
> > >    The test is failing with the current implementation because we use cpu_nip,
> > >    which is not updated all the time. Luis already has the fix, it should be
> > >    applied on the next version of his patch series.
> > > 
> > > The main reason to submit this patch as an RFC first is the docker part. I would
> > > lie if I tell you that I understand half of what is going on there.
> > >   - 'make docker-test-tcg' fails, but apparently on unrelated things;
> > >   - 'make docker-run-test-tcg@debian-ppc64el-cross' passes, but it looks
> > >     like the test is skipped?
> > >   - 'make check-tcg' runs the test and passes (with the fix in place for the
> > >     second).
> > 
> > What sort of host was that on?  Unfortunately 'make check-tcg' has
> > been broken on a POWER host for some time, and I've never had time to
> > look into it.
> > 
> 
> I'm testing on amd64, but I can also try on ppc64le.
> 
> > > 
> > > Finally, get_maintainer.pl found no maintainers for
> > > tests/tcg/ppc64{,le}/Makefile.target. Would it be Mr. Gibson?
> > 
> > Uh... sorta?  I also don't know much about what's going on here, but
> > I'm probably maintainer by default.
> > 
> 
> So, should I update MAINTAINERS in this series?
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]