[PATCH kvmtool 00/10] Run kvm-unit-tests with --kernel

From: Alexandru Elisei <alexandru.elisei@arm.com>
To: will@kernel.org, julien.thierry.kdev@gmail.com, kvm@vger.kernel.org
Cc: christoffer.dall@arm.com, vivek.gautam@arm.com
Subject: [PATCH kvmtool 00/10] Run kvm-unit-tests with --kernel
Date: Thu, 23 Sep 2021 15:44:55 +0100	[thread overview]
Message-ID: <20210923144505.60776-1-alexandru.elisei@arm.com> (raw)

What prompted this series (which I really hoped will turn out smaller than
it did) is my attempt to add support for kvmtool to kvm-unit-tests
automated test runner [1]. When working through the review comments for
that series, I realized that kvmtool must be able to load an initrd when
running a test to get all the features that tests rely on.

kvm-unit-tests uses the initrd, which is expected to be a text file in the
format key=value, to pass parameters to a test. The initrd is by default
generated by the runner script, but the location of a custom initrd file
can also be set using the environment variable KVM_UNIT_TEST_ENV (many
thanks to Andrew Jones for explaining that). Contained in the automatically
generated initrd is information about the presence of certain commits in
the host kernel.  These commits are important because they fix serious bugs
in KVM, and running tests which are designed to exercise the fix on systems
where it isn't present can cause the host kernel to crash. kvm-unit-tests
calls these bug fixing commits erratas, and their presence is signalled by
an entry ERRATA_<commit_id>=y in the initrd.

Using --kernel to run a test is not possible for two reasons:

1. Commit fd0a05bd27dd ("arm64: Obtain text offset from kernel image") made
it such that the kernel load offset is read from the binary file even if
the kernel header is not detected. kvmtool will try to load a test at a
random address read from the text section of the binary, which most of the
time is well above the upper limit for the VM's RAM in a normal VM
configuration.

2. kvm-unit-tests uses the kernel command line as the source for various
test parameters. kvmtool modifies the kernel command line that the user
specified to try to make it as painless as possible to boot a kernel, but
for a test this means that parsing the kernel command line for the required
parameters fails because of those unexpected, and in this case, unwanted
additions.

Currently, running any kvm-unit-tests test with kvmtool can only be done
with --firmware, which does not touch the command line, but it has the
downside of not being able to load an initrd. I decided to add a new
kvmtool command line option, --nodefaults, which disables all the automated
stuff that kvmtool does without being explicitly told so by the user (which
includes modifying the kernel command line).

I believe a --nodefaults option has merit even outside enabling support for
automating kvm-unit-tests runs. There are legitimate reasons for a user to
remove all the parameters that kvmtool adds to the kernel command line
(like testing or trying to understand how something works), or the
generated rootfs filesystem (for example, the initrd with the rootfs is
included in the kernel). I think that giving the user as much control as
possible is very useful for a program like kvmtool, which lends itself very
well to quick testing and prototyping.

The --nocompat option was added because compat warnings, in certain
situations, can be more confusing than useful on arm64, which has virtio in
defconfig. Of course, this is under the assumption that a user who removes
virtio from the kernel config knows what he's doing, but the compat
warnings are still shown by default just in case. Also, with the --firmware
option, the assumption that every guest is a Linux kernel and has working
virtio is not really true any more.

A quick summary of the patches:

* Patches #1 through #4 are for making kvmtool's command line more
  informative when options are ignored because --kernel and --firmware are
  handled differently.

* Patch #5 removes kvm->cfg.image_count, which is really the same
  thing as kvm->nr_disks. I found this very confusing when trying to
  understand the function kvm_cmd_run_init(), but the patch is not
  necessary for what this series is trying to achieve.

* Patch #6 is a preparatory patch and #7 adds the --nodefaults option.

* Patch #8 adds the --nocompat option.

* Patch #9 and #10 is my attempt at making kvmtool slightly more lenient
  when loading something other than a kernel with --kernel. Patch #9 is
  squarely aimed at loading kvm-unit-tests with --kernel (which was possible
  before kernel header parsing, but not anymore). Patch #10 is my attempt
  at hiding a warning which is harmless in the context of loading a
  kvm-unit-tests test.

[1] https://lists.cs.columbia.edu/pipermail/kvmarm/2021-July/047747.html

Alexandru Elisei (10):
  builtin-run: Treat specifying both --kernel and --firmware as an error
  builtin-run: Warn when ignoring initrd because --firmware was
    specified
  builtin-run: Do not attempt to find vmlinux if --firmware
  builtin-run: Abstract argument validation into a separate function
  Use kvm->nr_disks instead of kvm->cfg.image_count
  builtin-run: Move kernel command line generation to a separate
    function
  Add --nodefaults command line argument
  Add --nocompat option to disable compat warnings
  arm64: Use the default offset when the kernel image magic is not found
  arm64: Be more permissive when parsing the kernel header

 arm/aarch64/kvm.c        |  18 ++---
 arm/fdt.c                |   3 +-
 builtin-run.c            | 144 ++++++++++++++++++++++++---------------
 disk/core.c              |  18 ++---
 guest_compat.c           |   1 +
 include/kvm/kvm-config.h |   3 +-
 mips/kvm.c               |   3 +-
 7 files changed, 114 insertions(+), 76 deletions(-)

-- 
2.31.1