"Matheus K. Ferst" <matheus.fe...@eldorado.org.br> writes:
> On 20/04/2021 09:20, Alex Bennée wrote:
>> David Gibson <da...@gibson.dropbear.id.au> writes:
>>
>>> On Mon, Apr 19, 2021 at 10:33:07PM -0300, matheus.fe...@eldorado.org.br
>>> wrote:
>>>> From: Matheus Ferst <matheus.fe...@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.fe...@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?
--
Alex Bennée
