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

["Alex Bennée" <alex.bennee@linaro.org>]

Leandro Lupori <leandro.lupori@eldorado.org.br> writes:

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

Don't forget to add new files to MAINTAINERS by the way ;-)

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

So according to IRC this is because the ppc64-softmmu target can support
dynamically switching between BE/LE modes so there is only needs to be
one 64 bit ppc system binary.

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

There is another approach you can take which is to generate alternative
binaries from the same sources in the build. For example we build the
sha512 test with a couple of different compiler options and run with
slightly different QEMU_OPTS:

  sha512-vector: CFLAGS +=-mcpu=power10 -O3
  sha512-vector: sha512.c
          $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)

  run-sha512-vector: QEMU_OPTS+=-cpu POWER10
  run-plugin-sha512-vector-with-%: QEMU_OPTS+=-cpu POWER10

  PPC64LE_TESTS += sha512-vector

So you could do something similar for le versions of the tests.

I'm ambivalent to which makes the best approach. I only worry the
"pseudo target" approach might break something else down the line.
However as long as the ppc maintainers are happy with the tests you can
have my:

Acked-by: Alex Bennée <alex.bennee@linaro.org>

for the check-tcg plumbing changes.

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


-- 
Alex Bennée