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

[Matheus K. Ferst]

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
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software Júnior
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>