Re: [RFC PATCH 4/6] tests/tcg: add support for ppc64le softmmu tests

[Leandro Lupori]

On 24/03/2022 17:34, Alex Bennée wrote:
> Leandro Lupori <leandro.lupori@eldorado.org.br> writes:
>
> > Adding a new, "virtual" TCG test target, ppc64le-softmmu, seems to
> > be the cleanest way to support both BE and LE tests for
> > ppc64-softmmu.
> >
> > Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
> > ---
> >   tests/Makefile.include                    |  7 ++++---
> >   tests/tcg/configure.sh                    | 11 ++++++++++-
> >   tests/tcg/ppc64/Makefile.softmmu-target   |  2 ++
> >   tests/tcg/ppc64le/Makefile.softmmu-target |  7 +++++++
> >   4 files changed, 23 insertions(+), 4 deletions(-)
> >   create mode 100644 tests/tcg/ppc64le/Makefile.softmmu-target
> >
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index e7153c8e91..4001fedbc3 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -40,9 +40,10 @@ SPEED = quick
> >   TARGETS=$(patsubst libqemu-%.fa, %, $(filter libqemu-%.fa, $(ninja-targets)))
> >
> >   # Per guest TCG tests
> > -BUILD_TCG_TARGET_RULES=$(patsubst %,build-tcg-tests-%, $(TARGETS))
> > -CLEAN_TCG_TARGET_RULES=$(patsubst %,clean-tcg-tests-%, $(TARGETS))
> > -RUN_TCG_TARGET_RULES=$(patsubst %,run-tcg-tests-%, $(TARGETS))
> > +TCG_TARGETS=$(patsubst tests/tcg/config-%.mak, %, $(wildcard 
> > tests/tcg/config-*.mak))
> > +BUILD_TCG_TARGET_RULES=$(patsubst %,build-tcg-tests-%, $(TCG_TARGETS))
> > +CLEAN_TCG_TARGET_RULES=$(patsubst %,clean-tcg-tests-%, $(TCG_TARGETS))
> > +RUN_TCG_TARGET_RULES=$(patsubst %,run-tcg-tests-%, $(TCG_TARGETS))
>
> I'm not following what is going on here. Are we creating a new target
> type? Is this just to avoid duplication in tests/tcg subdirs?
Yes, together with the change in test/tcg/configure.sh, a new 
ppc64le-softmmu target is created, in the context of TCG tests only.
But it isn't just to avoid duplication in tests/tcg subdirs.

Without a ppc64le-softmmu target, the tcg tests' makefiles will only 
include tests/tcg/ppc64/Makefile.softmmu-target file. They won't try to 
include tests/tcg/ppc64le/Makefile.softmmu-target, because there is no 
ppc64le-softmmu target.

I've actually tried to do everything in 
tests/tcg/ppc64/Makefile.softmmu-target. But when it is included, 
everything is already setup to build for ppc64 (BE), such as CC, 
EXTRA_CFLAGS and other variables. So it seems that, to be able to also 
build and run the same tests for ppc64le, I would need to somehow change 
CC, EXTRA_CFLAGS, etc, to setup them for a ppc64le build and write 
another set of rules for the LE tests. Then I would also need to handle 
output file conflicts, to be able have both BE and LE binaries 
coexisting in the same ppc64-softmmu output directory.

So that's why I've added this new target, only for TCG tests, to avoid 
the issues above.

> >   # Probe for the Docker Builds needed for each build
> >   $(foreach PROBE_TARGET,$(TARGET_DIRS),                               \
> > diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
> > index ed4b5ccb1f..a4ac7a4e44 100755
> > --- a/tests/tcg/configure.sh
> > +++ b/tests/tcg/configure.sh
> > @@ -80,6 +80,10 @@ fi
> >   : ${cross_as_tricore="tricore-as"}
> >   : ${cross_ld_tricore="tricore-ld"}
> >
> > +# If target ppc64-softmmu is configured, also include the virtual test target
> > +# ppc64le-softmmu
> > +target_list=`echo $target_list | sed 's/ppc64-softmmu/& ppc64le-softmmu/'`
> > +
> >   for target in $target_list; do
> >     arch=${target%%-*}
> >
> > @@ -237,7 +241,12 @@ for target in $target_list; do
> >         ;;
> >       *-softmmu)
> >         echo "CONFIG_SOFTMMU=y" >> $config_target_mak
> > -      echo "QEMU=$PWD/qemu-system-$arch" >> $config_target_mak
> > +      if test $arch = "ppc64le"; then
> > +        sys_arch=ppc64
> > +      else
> > +        sys_arch=$arch
> > +      fi
> > +      echo "QEMU=$PWD/qemu-system-$sys_arch" >> $config_target_mak
> >         ;;
> >     esac
> >
> > diff --git a/tests/tcg/ppc64/Makefile.softmmu-target 
> > b/tests/tcg/ppc64/Makefile.softmmu-target
> > index 8f9925ca5a..511b6322df 100644
> > --- a/tests/tcg/ppc64/Makefile.softmmu-target
> > +++ b/tests/tcg/ppc64/Makefile.softmmu-target
> > @@ -2,6 +2,8 @@
> >   # PowerPC64 system tests
> >   #
> >
> > +BIG_ENDIAN ?= 1
> > +
> >   # For now, disable tests that are failing
> >   DISABLED_TESTS := memory
> >   DISABLED_EXTRA_RUNS := run-gdbstub-memory
> > diff --git a/tests/tcg/ppc64le/Makefile.softmmu-target 
> > b/tests/tcg/ppc64le/Makefile.softmmu-target
> > new file mode 100644
> > index 0000000000..d4162160ee
> > --- /dev/null
> > +++ b/tests/tcg/ppc64le/Makefile.softmmu-target
> > @@ -0,0 +1,7 @@
> > +#
> > +# PowerPC64 LE system tests
> > +#
> > +
> > +BIG_ENDIAN = 0
> > +
> > +include $(SRC_PATH)/tests/tcg/ppc64/Makefile.softmmu-target
>
>
> --
> Alex Bennée