diff mbox series

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

Message ID 20210420013308.813323-2-matheus.ferst@eldorado.org.br (mailing list archive)
State New
Headers show
Series tests/docker: tests/tcg/ppc64le: Newer toolchain to build tests for PowerISA v3.1 instructions | expand

Commit Message

Matheus K. Ferst April 20, 2021, 1:33 a.m. UTC
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, a new '-test-cross' image is 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                   |  2 ++
 .../debian-powerpc-test-cross.docker            | 17 +++++++++++++++++
 tests/tcg/configure.sh                          | 12 ++++++++----
 3 files changed, 27 insertions(+), 4 deletions(-)
 create mode 100644 tests/docker/dockerfiles/debian-powerpc-test-cross.docker

Comments

David Gibson April 20, 2021, 3:59 a.m. UTC | #1
On Mon, Apr 19, 2021 at 10:33:07PM -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, a new '-test-cross' image is 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                   |  2 ++
>  .../debian-powerpc-test-cross.docker            | 17 +++++++++++++++++
>  tests/tcg/configure.sh                          | 12 ++++++++----
>  3 files changed, 27 insertions(+), 4 deletions(-)
>  create mode 100644 tests/docker/dockerfiles/debian-powerpc-test-cross.docker
> 
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 9f464cb92c..f1dbcc639f 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -152,10 +152,12 @@ 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-power-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-power-test-cross

You add these "power" (as opposed to "powerpc" or "ppc64" or whatever)
images here, but they don't seem to be referenced anywhere else.

>  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-powerpc-test-cross.docker b/tests/docker/dockerfiles/debian-powerpc-test-cross.docker
> new file mode 100644
> index 0000000000..36b336f709
> --- /dev/null
> +++ b/tests/docker/dockerfiles/debian-powerpc-test-cross.docker
> @@ -0,0 +1,17 @@
> +#
> +# Docker powerpc/ppc64/ppc64le cross-compiler target
> +#
> +# This docker target builds on the debian Bullseye base image.
> +#
> +FROM qemu/debian11
> +
> +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-ppc64el-cross

I'm not really clear on what distinguishes a -cross.docker image and a
-test-cross.docker image.

> diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
> index fa1a4261a4..a0be066499 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-powerpc-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-powerpc-test-cross
> +      container_cross_cc=powerpc64le-linux-gnu-gcc-10

Doesn't this remove all uses of the debian-pp64-cross and
debian-ppc64el-cross images, in which case you could remove the
matching docker files and so forth too, no?

>        ;;
>      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 April 20, 2021, 12:20 p.m. UTC | #2
David Gibson <david@gibson.dropbear.id.au> writes:

> On Mon, Apr 19, 2021 at 10:33:07PM -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, a new '-test-cross' image is 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                   |  2 ++
>>  .../debian-powerpc-test-cross.docker            | 17 +++++++++++++++++
>>  tests/tcg/configure.sh                          | 12 ++++++++----
>>  3 files changed, 27 insertions(+), 4 deletions(-)
>>  create mode 100644 tests/docker/dockerfiles/debian-powerpc-test-cross.docker
>> 
>> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
>> index 9f464cb92c..f1dbcc639f 100644
>> --- a/tests/docker/Makefile.include
>> +++ b/tests/docker/Makefile.include
>> @@ -152,10 +152,12 @@ 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-power-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-power-test-cross
>
> You add these "power" (as opposed to "powerpc" or "ppc64" or whatever)
> images here, but they don't seem to be referenced anywhere else.
>
>>  DOCKER_PARTIAL_IMAGES += debian-hppa-cross
>>  DOCKER_PARTIAL_IMAGES += debian-m68k-cross debian-mips64-cross
>>  DOCKER_PARTIAL_IMAGES += debian-powerpc-cross debian-ppc64-cross

I was expecting the new debian-power-test-cross to replace both
debian-powerpc-cross and debian-ppc64-cross.

>> diff --git a/tests/docker/dockerfiles/debian-powerpc-test-cross.docker b/tests/docker/dockerfiles/debian-powerpc-test-cross.docker
>> new file mode 100644
>> index 0000000000..36b336f709
>> --- /dev/null
>> +++ b/tests/docker/dockerfiles/debian-powerpc-test-cross.docker
>> @@ -0,0 +1,17 @@
>> +#
>> +# Docker powerpc/ppc64/ppc64le cross-compiler target
>> +#
>> +# This docker target builds on the debian Bullseye base image.
>> +#
>> +FROM qemu/debian11
>> +
>> +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-ppc64el-cross
>
> I'm not really clear on what distinguishes a -cross.docker image and a
> -test-cross.docker image.

The normal cross image (debian-ppc64le-cross) can cross build QEMU. The
-test-cross images are just used for building check-tcg test cases.

>
>> diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
>> index fa1a4261a4..a0be066499 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-powerpc-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-powerpc-test-cross
>> +      container_cross_cc=powerpc64le-linux-gnu-gcc-10
>
> Doesn't this remove all uses of the debian-pp64-cross and

Yes this should be removed.

> debian-ppc64el-cross images, in which case you could remove the
> matching docker files and so forth too, no?

Well I assume you still want to check cross building of QEMU. Arguably
we could still use that for the test cases as well.

>
>>        ;;
>>      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 \
Philippe Mathieu-Daudé April 20, 2021, 12:35 p.m. UTC | #3
On 4/20/21 2:20 PM, Alex Bennée wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
>> On Mon, Apr 19, 2021 at 10:33:07PM -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, a new '-test-cross' image is 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                   |  2 ++
>>>  .../debian-powerpc-test-cross.docker            | 17 +++++++++++++++++
>>>  tests/tcg/configure.sh                          | 12 ++++++++----
>>>  3 files changed, 27 insertions(+), 4 deletions(-)
>>>  create mode 100644 tests/docker/dockerfiles/debian-powerpc-test-cross.docker
>>>
>>> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
>>> index 9f464cb92c..f1dbcc639f 100644
>>> --- a/tests/docker/Makefile.include
>>> +++ b/tests/docker/Makefile.include
>>> @@ -152,10 +152,12 @@ 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-power-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-power-test-cross
>>
>> You add these "power" (as opposed to "powerpc" or "ppc64" or whatever)
>> images here, but they don't seem to be referenced anywhere else.
>>
>>>  DOCKER_PARTIAL_IMAGES += debian-hppa-cross
>>>  DOCKER_PARTIAL_IMAGES += debian-m68k-cross debian-mips64-cross
>>>  DOCKER_PARTIAL_IMAGES += debian-powerpc-cross debian-ppc64-cross
> 
> I was expecting the new debian-power-test-cross to replace both
> debian-powerpc-cross and debian-ppc64-cross.

Maybe rename the images as:

DISTRIB-TARGET-crossbuild and DISTRIB-TARGET-crosstest-tcg?
(as it is not obvious for every contributor than 'cross' imply
emulation and not virtualization).
Matheus K. Ferst April 20, 2021, 2:31 p.m. UTC | #4
On 20/04/2021 09:20, Alex Bennée wrote:
> 
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
>> On Mon, Apr 19, 2021 at 10:33:07PM -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, a new '-test-cross' image is 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                   |  2 ++
>>>   .../debian-powerpc-test-cross.docker            | 17 +++++++++++++++++
>>>   tests/tcg/configure.sh                          | 12 ++++++++----
>>>   3 files changed, 27 insertions(+), 4 deletions(-)
>>>   create mode 100644 tests/docker/dockerfiles/debian-powerpc-test-cross.docker
>>>
>>> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
>>> index 9f464cb92c..f1dbcc639f 100644
>>> --- a/tests/docker/Makefile.include
>>> +++ b/tests/docker/Makefile.include
>>> @@ -152,10 +152,12 @@ 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-power-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-power-test-cross
>>
>> You add these "power" (as opposed to "powerpc" or "ppc64" or whatever)
>> images here, but they don't seem to be referenced anywhere else.
>>

It's a typo. Probably worked on my machine because a correctly named image
was created outside qemu build before. I'll nuke docker and test again.

>>>   DOCKER_PARTIAL_IMAGES += debian-hppa-cross
>>>   DOCKER_PARTIAL_IMAGES += debian-m68k-cross debian-mips64-cross
>>>   DOCKER_PARTIAL_IMAGES += debian-powerpc-cross debian-ppc64-cross
> 
> I was expecting the new debian-power-test-cross to replace both
> debian-powerpc-cross and debian-ppc64-cross.
> 

OK, I'll change for powerpc too.

>>> diff --git a/tests/docker/dockerfiles/debian-powerpc-test-cross.docker b/tests/docker/dockerfiles/debian-powerpc-test-cross.docker
>>> new file mode 100644
>>> index 0000000000..36b336f709
>>> --- /dev/null
>>> +++ b/tests/docker/dockerfiles/debian-powerpc-test-cross.docker
>>> @@ -0,0 +1,17 @@
>>> +#
>>> +# Docker powerpc/ppc64/ppc64le cross-compiler target
>>> +#
>>> +# This docker target builds on the debian Bullseye base image.
>>> +#
>>> +FROM qemu/debian11
>>> +
>>> +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-ppc64el-cross
>>
>> I'm not really clear on what distinguishes a -cross.docker image and a
>> -test-cross.docker image.
> 
> The normal cross image (debian-ppc64le-cross) can cross build QEMU. The
> -test-cross images are just used for building check-tcg test cases.
> 
>>
>>> diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
>>> index fa1a4261a4..a0be066499 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-powerpc-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-powerpc-test-cross
>>> +      container_cross_cc=powerpc64le-linux-gnu-gcc-10
>>
>> Doesn't this remove all uses of the debian-pp64-cross and
> 
> Yes this should be removed.
> 

Done.

>> debian-ppc64el-cross images, in which case you could remove the
>> matching docker files and so forth too, no?
> 
> Well I assume you still want to check cross building of QEMU. Arguably
> we could still use that for the test cases as well.
>

But then we have the old gcc that can't handle the new instructions, which
is why I started the series :)

>>
>>>         ;;
>>>       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 \
> 
> 

Oh, about nesting "ppc64-*" and "ppc64le-*" in tests/tcg/configure.sh:

 >>> +# 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.

We could do something like:
 > ppc64*-|ppc64le-*)
 >     container_hosts=x86_64
 >     container_image=debian-powerpc-test-cross
 >     container_cross_cc=${target%%-*}-linux-gnu-gcc-10
 >     container_cross_cc=powerpc${container_cross_cc#ppc}
But I think it's kind of ugly to repeat container_cross_cc line. If we do
that, we can bring ppc-* too, but another line would be needed to handle
ppc64abi32-*...
Matheus K. Ferst April 20, 2021, 3:06 p.m. UTC | #5
On 20/04/2021 09:35, Philippe Mathieu-Daudé wrote:
> On 4/20/21 2:20 PM, Alex Bennée wrote:
>> David Gibson <david@gibson.dropbear.id.au> writes:
>>> On Mon, Apr 19, 2021 at 10:33:07PM -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, a new '-test-cross' image is 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                   |  2 ++
>>>>   .../debian-powerpc-test-cross.docker            | 17 +++++++++++++++++
>>>>   tests/tcg/configure.sh                          | 12 ++++++++----
>>>>   3 files changed, 27 insertions(+), 4 deletions(-)
>>>>   create mode 100644 tests/docker/dockerfiles/debian-powerpc-test-cross.docker
>>>>
>>>> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
>>>> index 9f464cb92c..f1dbcc639f 100644
>>>> --- a/tests/docker/Makefile.include
>>>> +++ b/tests/docker/Makefile.include
>>>> @@ -152,10 +152,12 @@ 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-power-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-power-test-cross
>>>
>>> You add these "power" (as opposed to "powerpc" or "ppc64" or whatever)
>>> images here, but they don't seem to be referenced anywhere else.
>>>
>>>>   DOCKER_PARTIAL_IMAGES += debian-hppa-cross
>>>>   DOCKER_PARTIAL_IMAGES += debian-m68k-cross debian-mips64-cross
>>>>   DOCKER_PARTIAL_IMAGES += debian-powerpc-cross debian-ppc64-cross
>>
>> I was expecting the new debian-power-test-cross to replace both
>> debian-powerpc-cross and debian-ppc64-cross.
> 
> Maybe rename the images as:
> 
> DISTRIB-TARGET-crossbuild and DISTRIB-TARGET-crosstest-tcg?
> (as it is not obvious for every contributor than 'cross' imply
> emulation and not virtualization).
> 

If I grep'ed it correctly, in a separate patch we'd mv:
debian-alpha-cross{,test-tcg}.docker
debian-arm64-{test-cross,crosstest-tcg}.docker
debian-hppa-cross{,test-tcg}.docker
debian-m68k-cross{,test-tcg}.docker
debian-mips64-cross{,test-tcg}.docker
debian-powerpc-cross{,test-tcg}.docker
debian-riscv64-cross{,test-tcg}.docker
debian-sh4-cross{,test-tcg}.docker
debian-sparc64-cross{,test-tcg}.docker
debian-tricore-cross{,test-tcg}.docker
debian-xtensa-cross{,test-tcg}.docker
debian-amd64-cross{,build}.docker
debian-arm64-cross{,build}.docker
debian-armel-cross{,build}.docker
debian-armhf-cross{,build}.docker
debian-mips64el-cross{,build}.docker
debian-mips-cross{,build}.docker
debian-mipsel-cross{,build}.docker
debian-ppc64{el-cross,le-crossbuild}.docker
debian-s390x-cross{,build}.docker
Alex Bennée April 20, 2021, 4:53 p.m. UTC | #6
"Matheus K. Ferst" <matheus.ferst@eldorado.org.br> writes:

> On 20/04/2021 09:20, Alex Bennée wrote:
>> David Gibson <david@gibson.dropbear.id.au> writes:
>> 
>>> On Mon, Apr 19, 2021 at 10:33:07PM -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, a new '-test-cross' image is 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                   |  2 ++
>>>>   .../debian-powerpc-test-cross.docker            | 17 +++++++++++++++++
>>>>   tests/tcg/configure.sh                          | 12 ++++++++----
>>>>   3 files changed, 27 insertions(+), 4 deletions(-)
>>>>   create mode 100644 tests/docker/dockerfiles/debian-powerpc-test-cross.docker
>>>>
>>>> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
>>>> index 9f464cb92c..f1dbcc639f 100644
>>>> --- a/tests/docker/Makefile.include
>>>> +++ b/tests/docker/Makefile.include
>>>> @@ -152,10 +152,12 @@ 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-power-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-power-test-cross
>>>
>>> You add these "power" (as opposed to "powerpc" or "ppc64" or whatever)
>>> images here, but they don't seem to be referenced anywhere else.
>>>
>
> It's a typo. Probably worked on my machine because a correctly named image
> was created outside qemu build before. I'll nuke docker and test again.
>
>>>>   DOCKER_PARTIAL_IMAGES += debian-hppa-cross
>>>>   DOCKER_PARTIAL_IMAGES += debian-m68k-cross debian-mips64-cross
>>>>   DOCKER_PARTIAL_IMAGES += debian-powerpc-cross debian-ppc64-cross
>> I was expecting the new debian-power-test-cross to replace both
>> debian-powerpc-cross and debian-ppc64-cross.
>> 
>
> OK, I'll change for powerpc too.
>
>>>> diff --git a/tests/docker/dockerfiles/debian-powerpc-test-cross.docker b/tests/docker/dockerfiles/debian-powerpc-test-cross.docker
>>>> new file mode 100644
>>>> index 0000000000..36b336f709
>>>> --- /dev/null
>>>> +++ b/tests/docker/dockerfiles/debian-powerpc-test-cross.docker
>>>> @@ -0,0 +1,17 @@
>>>> +#
>>>> +# Docker powerpc/ppc64/ppc64le cross-compiler target
>>>> +#
>>>> +# This docker target builds on the debian Bullseye base image.
>>>> +#
>>>> +FROM qemu/debian11
>>>> +
>>>> +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-ppc64el-cross
>>>
>>> I'm not really clear on what distinguishes a -cross.docker image and a
>>> -test-cross.docker image.
>> The normal cross image (debian-ppc64le-cross) can cross build QEMU.
>> The
>> -test-cross images are just used for building check-tcg test cases.
>> 
>>>
>>>> diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
>>>> index fa1a4261a4..a0be066499 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-powerpc-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-powerpc-test-cross
>>>> +      container_cross_cc=powerpc64le-linux-gnu-gcc-10
>>>
>>> Doesn't this remove all uses of the debian-pp64-cross and
>> Yes this should be removed.
>> 
>
> Done.
>
>>> debian-ppc64el-cross images, in which case you could remove the
>>> matching docker files and so forth too, no?
>> Well I assume you still want to check cross building of QEMU.
>> Arguably
>> we could still use that for the test cases as well.
>>
>
> But then we have the old gcc that can't handle the new instructions, which
> is why I started the series :)

Oh right - yes switch to the test-cross... ;-)

>
>>>
>>>>         ;;
>>>>       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 \
>> 
>
> Oh, about nesting "ppc64-*" and "ppc64le-*" in tests/tcg/configure.sh:
>
>>>> +# 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.
>
> We could do something like:
>> ppc64*-|ppc64le-*)
>>     container_hosts=x86_64
>>     container_image=debian-powerpc-test-cross
>>     container_cross_cc=${target%%-*}-linux-gnu-gcc-10
>>     container_cross_cc=powerpc${container_cross_cc#ppc}
> But I think it's kind of ugly to repeat container_cross_cc line. If we do
> that, we can bring ppc-* too, but another line would be needed to handle
> ppc64abi32-*...

Lets not over complicate it - but the ppc64/ppc64le in the same
container looks fair enough doesn't it?
diff mbox series

Patch

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 9f464cb92c..f1dbcc639f 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -152,10 +152,12 @@  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-power-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-power-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-powerpc-test-cross.docker b/tests/docker/dockerfiles/debian-powerpc-test-cross.docker
new file mode 100644
index 0000000000..36b336f709
--- /dev/null
+++ b/tests/docker/dockerfiles/debian-powerpc-test-cross.docker
@@ -0,0 +1,17 @@ 
+#
+# Docker powerpc/ppc64/ppc64le cross-compiler target
+#
+# This docker target builds on the debian Bullseye base image.
+#
+FROM qemu/debian11
+
+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-ppc64el-cross
+
diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
index fa1a4261a4..a0be066499 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-powerpc-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-powerpc-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 \