Re: [kvm-unit-tests PATCH v3 00/16] arm/arm64: Add kvmtool to the runner script

Shaoqin Huang
2025-05-19T08:57:10Z

Hi Alexandru,

For this series, I've tested it, everything works good.

On 5/7/25 11:12 PM, Alexandru Elisei wrote:
> v2 can be found here [1].
>
> To goal is to allow the user to do:
>
> $ ./configure --target=kvmtool
> $ make clean && make
> $ ./run_tests.sh
>
> to run all the tests automatically with kvmtool.
>
> Reasons to use kvmtool:
>
> * kvmtool is smaller and a lot easier to hack than qemu, which means
> developers may prefer it when adding or prototyping new features to KVM.
> Being able to run all the tests reliably and automatically is very useful
> in the development process.
>
> * kvmtool is faster to run the tests (a couple of times faster on
> my rockpro64), making for a quick turnaround. But do keep in mind that not
> all tests work on kvmtool because of missing features compared to qemu.
>
> * kvmtool does things differently than qemu: different memory layout,
> different uart, PMU emulation is disabled by default, etc. This makes it a
> good testing vehicule for kvm-unit-tests itself.
>
> Changes in v3
> -------------
>
> Lots of changes following the excellent feedback I got. A bird's eye view:
>
> * Split extra_params into qemu_params and test_args: qemu_params for qemu
> arguments and test_args for the test's main() function.
>
> Now that I'm putting the cover letter together I'm considering that maybe
> having qemu_params, kvmtool_params and test_params (instead of test_args)
> might be a better naming scheme.
>
> * TARGET is now exported unconditionally. Unfortunately a side effect of
> this is that checking out these series and running the tests will end up
> with an error because the scripts now expect TARGET to be defined in
> config.mak.
>
> If it's unacceptable, I can drop this and handle everything in vmm.bash by
> converting direct accesses to vmm_opts with functions defined in vmm.bash
> (vmm_opts[$TARGET:parse_premature_failure] becomes
> vmm_parse_premature_failure(), for example).
>
> * Introduced scripts/vmm.bash to keep the vmm stuff contained. As a
> consequence there's very little $TARGET stuff in scripts/runtime.bash (only
> for premature_failure(), and no more 'case' statements anywhere) and
> instead scripts/common.bash passes the correct arguments directly to
> runtime.bash::run().
>
> Unfortunately, because of all the changes, I decided not to keep some of
> the Reviewed-by tags. That's not to say that the effort is not appreciated,
> on the contrary, these changes are a direct result of the review; I dropped
> the tags because I was worried they might not apply to the current content
> of the patches.
>
> If no major changes are needed following this round of review, for the next
> iteration I'm planning to send the first two patches (extra_params renamed
> to qemu_params and the new test_args) separately, to make sure it gets the
> review it deserves from the rest of the architectures.
>
> Still haven't managed to get EDK2 to work with kvmtool, so I've decided to
> explicitely disabled UEFI tests in the last patch ("scripts: Enable
> kvmtool") - this is new.
>
> I would also like to point out that despite Drew's comment I kept the
> 'disabled_if' test definition because I think using 'targets', with the
> default value of 'qemu', will probably lead to most, if not all, of the new
> tests which will be added never being run or tested with kvmtool. More
> details in patch #15 ("scripts: Add 'disabled_if' test definition parameter
> for kvmtool to use").
>
> [1] https://lore.kernel.org/kvm/20250120164316.31473-1-alexandru.elisei@arm.com/
>
> Alexandru Elisei (16):
> scripts: unittests.cfg: Rename 'extra_params' to 'qemu_params'
> scripts: Add 'test_args' test definition parameter
> configure: Export TARGET unconditionally
> run_tests.sh: Document --probe-maxsmp argument
> scripts: Document environment variables
> scripts: Refuse to run the tests if not configured for qemu
> scripts: Use an associative array for qemu argument names
> scripts: Add 'kvmtool_params' to test definition
> scripts: Add support for kvmtool
> scripts: Add default arguments for kvmtool
> scripts: Add KVMTOOL environment variable for kvmtool binary path
> scripts: Detect kvmtool failure in premature_failure()
> scripts: Do not probe for maximum number of VCPUs when using kvmtool
> scripts/mkstandalone: Export $TARGET
> scripts: Add 'disabled_if' test definition parameter for kvmtool to
> use
> scripts: Enable kvmtool
>
> README.md | 18 ++++-
> arm/efi/run | 8 ++
> arm/run | 161 +++++++++++++++++++++++--------------
> arm/unittests.cfg | 125 ++++++++++++++++++++---------
> configure | 37 ++++++---
> docs/unittests.txt | 54 +++++++++++--
> powerpc/run | 4 +-
> powerpc/unittests.cfg | 21 ++---
> riscv/run | 4 +-
> riscv/unittests.cfg | 2 +-
> run_tests.sh | 35 ++++++---
> s390x/run | 2 +-
> s390x/unittests.cfg | 53 +++++++------
> scripts/arch-run.bash | 113 ++++++++++----------------
> scripts/common.bash | 71 +++++++++++------
> scripts/mkstandalone.sh | 4 +
> scripts/runtime.bash | 51 +++++-------
> scripts/vmm.bash | 170 ++++++++++++++++++++++++++++++++++++++++
> x86/run | 4 +-
> x86/unittests.cfg | 164 +++++++++++++++++++++-----------------
> 20 files changed, 730 insertions(+), 371 deletions(-)
> create mode 100644 scripts/vmm.bash
>
>
> base-commit: 08db0f5cfbca16b36f200b7bc54a78fa4941bcce

--
Shaoqin