From: Sean Christopherson <seanjc@google.com>
To: Vipin Sharma <vipinsh@google.com>
Cc: kvm@vger.kernel.org, kvmarm@lists.linux.dev,
kvm-riscv@lists.infradead.org, pbonzini@redhat.com,
borntraeger@linux.ibm.com, frankja@linux.ibm.com,
imbrenda@linux.ibm.com, anup@brainfault.org,
atish.patra@linux.dev, zhaotianrui@loongson.cn,
maobibo@loongson.cn, chenhuacai@kernel.org, maz@kernel.org,
oliver.upton@linux.dev, ajones@ventanamicro.com
Subject: Re: [PATCH v4 8/9] KVM: selftests: Add rule to generate default tests for KVM selftests runner
Date: Wed, 29 Jul 2026 12:07:35 -0700 [thread overview]
Message-ID: <ampPd9szwRoGKwds@google.com> (raw)
In-Reply-To: <20260331194202.1722082-9-vipinsh@google.com>
On Tue, Mar 31, 2026, Vipin Sharma wrote:
> Add 'tests_install' rule in the Makefile.kvm to auto generate default
> testcases for KVM selftests runner. Preserve the hierarchy of test
> executables for autogenerated files. Remove these testcases on
> invocation of 'make clean'.
>
> Autogeneration of default test files allows runner to execute default
> testcases easily. These default testcases don't need to be checked in as
> they are just executing the test without any command line options.
>
> Signed-off-by: Vipin Sharma <vipinsh@google.com>
> ---
> tools/testing/selftests/kvm/.gitignore | 1 +
> tools/testing/selftests/kvm/Makefile.kvm | 26 +++++++++++++++++++++++-
> 2 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> index 95af97b1ff9e..548d435bde2f 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -7,6 +7,7 @@
> !*.S
> !*.sh
> !*.test
> +default.test
> !.gitignore
> !config
> !settings
> diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
> index 6471fa214a9f..fb9439cb5f3d 100644
> --- a/tools/testing/selftests/kvm/Makefile.kvm
> +++ b/tools/testing/selftests/kvm/Makefile.kvm
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0-only
> include ../../../build/Build.include
>
> -all:
> +all: tests_install
>
> LIBKVM += lib/assert.c
> LIBKVM += lib/elf.c
> @@ -330,11 +330,15 @@ $(SPLIT_TEST_GEN_PROGS): $(OUTPUT)/%: $(OUTPUT)/%.o $(OUTPUT)/$(ARCH)/%.o
> $(SPLIT_TEST_GEN_OBJ): $(OUTPUT)/$(ARCH)/%.o: $(ARCH)/%.c
> $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
>
> +# Default testcases for KVM selftests runner will be generated in this directory.
> +DEFAULT_TESTCASES = testcases_default_gen
That's a cumbersome name. It was also written by Yoda :-) How about default_testcases?
> +
> EXTRA_CLEAN += $(GEN_HDRS) \
> $(LIBKVM_OBJS) \
> $(SPLIT_TEST_GEN_OBJ) \
> $(TEST_DEP_FILES) \
> $(TEST_GEN_OBJ) \
> + $(OUTPUT)/$(DEFAULT_TESTCASES) \
> cscope.*
>
> $(LIBKVM_C_OBJ): $(OUTPUT)/%.o: %.c $(GEN_HDRS)
> @@ -363,3 +367,23 @@ cscope:
> find . -name '*.c' \
> -exec realpath --relative-base=$(PWD) {} \;) | sort -u > cscope.files
> cscope -b
> +
> +# Generate runner testcases in DEFAULT_TESTCASES directory.
> +# $(OUTPUT) is either CWD or specified in the make command.
> +tests_install: list_progs = $(patsubst $(OUTPUT)/%,%,$(TEST_GEN_PROGS))
> +tests_install:
This is very much *not* an install. In fact, install is broken, because the
runner isn't added to TEST_FILES.
Another issue with install is that it flattens the directory structures, i.e.
drops the $ARCH/ subdirectories. That causes issues for the default.test testcases
due to them using partially qualified paths.
And coming back to this with fresh eyes, I don't love generating the testcases
for the "normal" build. It necessitates adding default.test to .gitignore,
effectively requires copying the runner to the output directory, and is pure
noise for folks that don't want to utilize the runner.
Rather than generate testcases for the default build, what if we make this 100%
opt-in, and take a hard dependency on install? That should obviate the need for
ignoring default.test, because if someone is silly enough to install testcases
in the source tree, they get to deal with the noise.
And we can commit to having the default testcases provide completely unqualified
paths, so that the Just Work with the flattened output. If someone wants to run
with an unflattened tree, then we should update the runner itself to handle
multiple paths, a la the actual PATH variable.
The other nice thing is that we can make the runner opt-in, without requiring the
user to run multiple make commands, by chaining testcases => install => all. If
we try to generate testcases as an optional step to the normal build, then we'll
end up being able to run install without having generated the testcases, which
is "fine", but probably not what we want for people that are trying to use the
runner.
The obvious downside is that it requires doing an install to get the default
testcases, but IMO that's an acceptable tradeoff.
> + $(foreach tc, $(TEST_PROGS), \
> + $(shell mkdir -p $(OUTPUT)/$(DEFAULT_TESTCASES)/$(patsubst %.sh,%,$(tc))))
> + $(foreach tc, $(TEST_PROGS), \
> + $(shell echo $(tc) > $(patsubst %.sh,$(OUTPUT)/$(DEFAULT_TESTCASES)/%/default.test,$(tc))))
> +
> + $(foreach tc, $(list_progs), \
> + $(shell mkdir -p $(OUTPUT)/$(DEFAULT_TESTCASES)/$(tc)))
> + $(foreach tc, $(list_progs), \
> + $(shell echo $(tc) > $(patsubst %,$(OUTPUT)/$(DEFAULT_TESTCASES)/%/default.test,$(tc))))
> +
> + @if [ ! -d $(OUTPUT)/runner ]; then \
This is very wrong, as this will fail to pick up any changes made to the runner.
All in all, this as fixup?
diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index 83aa2fe01bac..91d2b21d396f 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -8,7 +8,6 @@
!*.S
!*.sh
!*.test
-default.test
!.gitignore
!config
!settings
diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index c5821d495ea6..fc9dc41fdbeb 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -1,8 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
include ../../../build/Build.include
-all: tests_install
-
LIBKVM += lib/assert.c
LIBKVM += lib/elf.c
LIBKVM += lib/guest_modes.c
@@ -347,15 +345,13 @@ $(SPLIT_TEST_GEN_PROGS): $(OUTPUT)/%: $(OUTPUT)/%.o $(OUTPUT)/$(ARCH)/%.o
$(SPLIT_TEST_GEN_OBJ): $(OUTPUT)/$(ARCH)/%.o: $(ARCH)/%.c
$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
-# Default testcases for KVM selftests runner will be generated in this directory.
-DEFAULT_TESTCASES = testcases_default_gen
+TEST_FILES := $(selfdir)/kvm/runner
EXTRA_CLEAN += $(GEN_HDRS) \
$(LIBKVM_OBJS) \
$(SPLIT_TEST_GEN_OBJ) \
$(TEST_DEP_FILES) \
$(TEST_GEN_OBJ) \
- $(OUTPUT)/$(DEFAULT_TESTCASES) \
cscope.*
$(LIBKVM_C_OBJ): $(OUTPUT)/%.o: %.c $(GEN_HDRS)
@@ -386,21 +382,18 @@ cscope:
cscope -b
# Generate runner testcases in DEFAULT_TESTCASES directory.
-# $(OUTPUT) is either CWD or specified in the make command.
-tests_install: list_progs = $(patsubst $(OUTPUT)/%,%,$(TEST_GEN_PROGS))
-tests_install:
- $(foreach tc, $(TEST_PROGS), \
- $(shell mkdir -p $(OUTPUT)/$(DEFAULT_TESTCASES)/$(patsubst %.sh,%,$(tc))))
- $(foreach tc, $(TEST_PROGS), \
- $(shell echo $(tc) > $(patsubst %.sh,$(OUTPUT)/$(DEFAULT_TESTCASES)/%/default.test,$(tc))))
+DEFAULT_TESTCASES = default_testcases
+
+testcases: list_progs = $(notdir $(patsubst $(OUTPUT)/%,%,$(TEST_GEN_PROGS)))
+testcases: install
+ $(foreach tc, $(notdir $(TEST_PROGS)), \
+ $(shell mkdir -p $(INSTALL_PATH)/$(DEFAULT_TESTCASES)/$(notdir $(patsubst %.sh,%,$(tc)))))
+ $(foreach tc, $(notdir $(TEST_PROGS)), \
+ $(shell echo $(tc) > $(patsubst %.sh,$(INSTALL_PATH)/$(DEFAULT_TESTCASES)/%/default.test,$(tc))))
$(foreach tc, $(list_progs), \
- $(shell mkdir -p $(OUTPUT)/$(DEFAULT_TESTCASES)/$(tc)))
+ $(shell mkdir -p $(INSTALL_PATH)/$(DEFAULT_TESTCASES)/$(tc)))
$(foreach tc, $(list_progs), \
- $(shell echo $(tc) > $(patsubst %,$(OUTPUT)/$(DEFAULT_TESTCASES)/%/default.test,$(tc))))
-
- @if [ ! -d $(OUTPUT)/runner ]; then \
- cp -r $(selfdir)/kvm/runner $(OUTPUT); \
- fi
+ $(shell echo $(tc) > $(patsubst %,$(INSTALL_PATH)/$(DEFAULT_TESTCASES)/%/default.test,$(tc))))
@:
> + cp -r $(selfdir)/kvm/runner $(OUTPUT); \
> + fi
> +
> + @:
> --
> 2.53.0.1118.gaef5881109-goog
>
next prev parent reply other threads:[~2026-07-29 19:07 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-31 19:41 [PATCH v4 0/9] KVM: selftests: Create KVM selftests runner Vipin Sharma
2026-03-31 19:41 ` [PATCH v4 1/9] KVM: selftest: Create KVM selftest runner Vipin Sharma
2026-06-11 1:17 ` Ackerley Tng
2026-06-11 19:08 ` Sean Christopherson
2026-06-11 20:16 ` Ackerley Tng
2026-06-12 1:07 ` Sean Christopherson
2026-06-12 19:45 ` Ackerley Tng
2026-06-12 23:56 ` Sean Christopherson
2026-06-16 16:15 ` Ackerley Tng
2026-06-16 18:29 ` Vipin Sharma
2026-06-16 21:43 ` Vipin Sharma
2026-06-16 23:04 ` Ackerley Tng
2026-03-31 19:41 ` [PATCH v4 2/9] KVM: selftests: Provide executables path option to the " Vipin Sharma
2026-03-31 19:41 ` [PATCH v4 3/9] KVM: selftests: Add timeout option in selftests runner Vipin Sharma
2026-07-29 19:39 ` Sean Christopherson
2026-03-31 19:41 ` [PATCH v4 4/9] KVM: selftests: Add option to save selftest runner output to a directory Vipin Sharma
2026-03-31 19:41 ` [PATCH v4 5/9] KVM: selftests: Run tests concurrently in KVM selftests runner Vipin Sharma
2026-03-31 19:41 ` [PATCH v4 6/9] KVM: selftests: Add various print flags to KVM selftest runner Vipin Sharma
2026-07-29 19:14 ` Sean Christopherson
2026-03-31 19:42 ` [PATCH v4 7/9] KVM: selftests: Print sticky KVM selftests runner status at bottom Vipin Sharma
2026-03-31 19:42 ` [PATCH v4 8/9] KVM: selftests: Add rule to generate default tests for KVM selftests runner Vipin Sharma
2026-07-29 19:07 ` Sean Christopherson [this message]
2026-03-31 19:42 ` [PATCH v4 9/9] KVM: selftests: Provide README.rst " Vipin Sharma
2026-07-29 19:09 ` Sean Christopherson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ampPd9szwRoGKwds@google.com \
--to=seanjc@google.com \
--cc=ajones@ventanamicro.com \
--cc=anup@brainfault.org \
--cc=atish.patra@linux.dev \
--cc=borntraeger@linux.ibm.com \
--cc=chenhuacai@kernel.org \
--cc=frankja@linux.ibm.com \
--cc=imbrenda@linux.ibm.com \
--cc=kvm-riscv@lists.infradead.org \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.linux.dev \
--cc=maobibo@loongson.cn \
--cc=maz@kernel.org \
--cc=oliver.upton@linux.dev \
--cc=pbonzini@redhat.com \
--cc=vipinsh@google.com \
--cc=zhaotianrui@loongson.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox