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

[Shaoqin Huang <shahuang@redhat.com>]

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


-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv