Re: [PATCH for 10.1 11/13] tests/tcg: reduce the number of plugin tests combinations

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

Manos Pitsidianakis <manos.pitsidianakis@linaro.org> writes:

> On Thu, Jul 24, 2025 at 3:00 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Manos Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
>>
>> > On Thu, Jul 24, 2025 at 2:00 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>> >>
>> >> As our set of multiarch tests has grown the practice of running every
>> >> plugin with every test is becoming unsustainable. If we switch to
>> >> ensuring every test gets run with at least one plugin we can speed
>> >> things up.
>> >>
>> >> Some plugins do need to be run with specific tests (for example the
>> >> memory instrumentation test). We can handle this by manually adding
>> >> them to EXTRA_RUNS. We also need to wrap rules in a CONFIG_PLUGIN test
>> >> so we don't enable the runs when plugins are not enabled.
>> >>
>> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> >> ---
>> >>  tests/tcg/Makefile.target                     | 23 ++++++++++++++-----
>> >>  tests/tcg/multiarch/Makefile.target           |  8 +++++--
>> >>  .../multiarch/system/Makefile.softmmu-target  | 11 +++++----
>> >>  3 files changed, 30 insertions(+), 12 deletions(-)
>> >>
>> >> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
>> >> index a12b15637ea..18afd5be194 100644
>> >> --- a/tests/tcg/Makefile.target
>> >> +++ b/tests/tcg/Makefile.target
>> >> @@ -173,14 +173,25 @@ PLUGINS=$(filter-out $(DISABLE_PLUGINS), \
>> >>  # We need to ensure expand the run-plugin-TEST-with-PLUGIN
>> >>  # pre-requistes manually here as we can't use stems to handle it. We
>> >>  # only expand MULTIARCH_TESTS which are common on most of our targets
>> >> -# to avoid an exponential explosion as new tests are added. We also
>> >> -# add some special helpers the run-plugin- rules can use below.
>> >> +# and rotate the plugins so we don't grow too out of control as new
>> >> +# tests are added. Plugins that need to run with a specific test
>> >> +# should ensure they add their combination to EXTRA_RUNS.
>> >>
>> >>  ifneq ($(MULTIARCH_TESTS),)
>> >> -$(foreach p,$(PLUGINS), \
>> >> -       $(foreach t,$(MULTIARCH_TESTS),\
>> >> -               $(eval run-plugin-$(t)-with-$(p): $t $p) \
>> >> -               $(eval RUN_TESTS+=run-plugin-$(t)-with-$(p))))
>> >> +
>> >> +NUM_PLUGINS := $(words $(PLUGINS))
>> >> +NUM_TESTS := $(words $(MULTIARCH_TESTS))
>> >> +
>> >> +define mod_plus_one
>> >> +  $(shell $(PYTHON) -c "print( ($(1) % $(2)) + 1 )")
>> >> +endef
>> >> +
>> >> +$(foreach _idx, $(shell seq 1 $(NUM_TESTS)), \
>> >> +       $(eval _test := $(word $(_idx), $(MULTIARCH_TESTS))) \
>> >> +       $(eval _plugin := $(word $(call mod_plus_one, $(_idx), $(NUM_PLUGINS)), $(PLUGINS))) \
>> >> +       $(eval run-plugin-$(_test)-with-$(_plugin): $(_test) $(_plugin)) \
>> >> +       $(eval RUN_TESTS+=run-plugin-$(_test)-with-$(_plugin)))
>> >> +
>> >>  endif # MULTIARCH_TESTS
>> >>  endif # CONFIG_PLUGIN
>> >>
>> >> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
>> >> index bfdf7197a7b..38345ff8805 100644
>> >> --- a/tests/tcg/multiarch/Makefile.target
>> >> +++ b/tests/tcg/multiarch/Makefile.target
>> >> @@ -189,6 +189,10 @@ run-plugin-semiconsole-with-%:
>> >>  TESTS += semihosting semiconsole
>> >>  endif
>> >>
>> >> +test-plugin-mem-access: CFLAGS+=-pthread -O0
>> >> +test-plugin-mem-access: LDFLAGS+=-pthread -O0
>> >> +
>> >> +ifeq ($(CONFIG_PLUGIN),y)
>> >>  # Test plugin memory access instrumentation
>> >>  run-plugin-test-plugin-mem-access-with-libmem.so: \
>> >>         PLUGIN_ARGS=$(COMMA)print-accesses=true
>> >> @@ -197,8 +201,8 @@ run-plugin-test-plugin-mem-access-with-libmem.so: \
>> >>         $(SRC_PATH)/tests/tcg/multiarch/check-plugin-output.sh \
>> >>         $(QEMU) $<
>> >>
>> >> -test-plugin-mem-access: CFLAGS+=-pthread -O0
>> >> -test-plugin-mem-access: LDFLAGS+=-pthread -O0
>> >> +EXTRA_RUNS += run-plugin-test-plugin-mem-access-with-libmem.so
>> >> +endif
>> >>
>> >>  # Update TESTS
>> >>  TESTS += $(MULTIARCH_TESTS)
>> >> diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target
>> >> index 5acf2700812..4171b4e6aa0 100644
>> >> --- a/tests/tcg/multiarch/system/Makefile.softmmu-target
>> >> +++ b/tests/tcg/multiarch/system/Makefile.softmmu-target
>> >> @@ -71,8 +71,11 @@ endif
>> >>  MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt \
>> >>         run-gdbstub-untimely-packet run-gdbstub-registers
>> >>
>> >> +ifeq ($(CONFIG_PLUGIN),y)
>> >>  # Test plugin memory access instrumentation
>> >> -run-plugin-memory-with-libmem.so:              \
>> >> -       PLUGIN_ARGS=$(COMMA)region-summary=true
>> >> -run-plugin-memory-with-libmem.so:              \
>> >> -       CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out
>> >> +run-plugin-memory-with-libmem.so: memory libmem.so
>> >
>> > Hm why wasn't this needed before this change?
>> >
>> > I see the make `memory` target sets CHECK_UNALIGNED but where is
>> > libmem.so target coming from?
>> >
>> >> +run-plugin-memory-with-libmem.so: PLUGIN_ARGS=$(COMMA)region-summary=true
>> >> +run-plugin-memory-with-libmem.so: CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out
>> >> +
>> >> +EXTRA_RUNS += run-plugin-memory-with-libmem.so
>>
>> Because we add it to EXTRA_RUNS so we don't want that if plugins won't
>> work. We could just wrap EXTRA_RUNS but the rest of the recipe is moot
>> at that point anyway.
>
> I am asking about the line
>
>> +run-plugin-memory-with-libmem.so: memory libmem.so
>
> Which adds two make target prerequisites `memory` and `libmem.so`, I
> don't see where they came from, or maybe I am not understanding the
> makefile logic here correctly.

Originally we generated the pre-reqs in the:

  $(foreach p,$(PLUGINS), \
      $(foreach t,$(MULTIARCH_TESTS),\

loop. Now that doesn't cover every combination we need to add an
explicit prereq for the test case we know we will run.

>
>>
>> >> +endif
>> >> --
>> >> 2.47.2
>> >>
>> >>
>>
>> --
>> Alex Bennée
>> Virtualisation Tech Lead @ Linaro

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro