[kvm-unit-tests PATCH v2 00/18] arm/arm64: Add kvmtool to the runner script

[kvm-unit-tests PATCH v2 00/18] arm/arm64: Add kvmtool to the runner script

[Alexandru Elisei]

Finally got fed up with manually running a test with kvmtool, so I've
decided to send v2 of the series [1] that adds kvmtool support to
run_tests.sh. The series has significantly more patches now, but that's
mostly because I split a large patch into several smaller ones (as per
Andre's suggestion), which I hope will make reviewing easier. Because of
this I removed two Reviewed-by tags from Drew and Thomas Huth - your review
is much appreciated!

To goal is to have an user 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.

The series has been rewritten since v1 [1]. This is a brief overview of the
major changes:

* Split into smaller patches.
* Document environment variables and --probe-maxsmp options.
* New unittest parameter, qemu_params, to replace extra_params going
forward (extra_params has been kept for compatibility)
* New unittest parameter, kvmtool_params, for kvmtool specific arguments
needed to run a test.
* New unittest parameter, disabled_if, to disable a test that cannot run
under kvmtool.

I would very much like more input regarding disabled_if. Allows all sorts
of combinations, like:

[ "$TARGET" = kvmtool ] && ([ -z "$CONFIG_EFI" ] || [ "$CONFIG_EFI" = n ])

and that's because it's evaluated as-is in a bash if statement - might have
security implications. I could have just added something like
supported_vmms, but I thought the current approach looks more flexible.
Although that might just be premature optimization.

There's only one limitation as far as I know - UEFI tests don't work. I
tried to run a .efi test with kvmtool manually, but kvmtool froze and I
didn't get any output. I am not familiar with EDK2, so I thought I can send
the this series and get feedback on it while I make time to figure out what
is going on - it might be something with kvm-unit-tests, EDK2, kvmtool, or
a combination of them. And I don't think UEFI support is very important at
the moment, no distro ships a EDK2 binary compiled for kvmtool so I don't
think there would be many users for it.

[1] https://lore.kernel.org/kvm/20210702163122.96110-1-alexandru.elisei@arm.com/

Please review,
Alex

Alexandru Elisei (18):
  run_tests: Document --probe-maxsmp argument
  Document environment variables
  scripts: Refuse to run the tests if not configured for qemu
  run_tests: Introduce unittest parameter 'qemu_params'
  scripts: Rename run_qemu_status -> run_test_status
  scripts: Merge the qemu parameter -smp into $qemu_opts
  scripts: Introduce kvmtool_opts
  scripts/runtime: Detect kvmtool failure in premature_failure()
  scripts/runtime: Skip test when kvmtool and $accel is not KVM
  scripts/arch-run: Add support for kvmtool
  arm/run: Add support for kvmtool
  scripts/runtime: Add default arguments for kvmtool
  run_tests: Do not probe for maximum number of VCPUs when using kvmtool
  run_tests: Add KVMTOOL environment variable for kvmtool binary path
  Add kvmtool_params to test specification
  scripts/mkstandalone: Export $TARGET
  unittest: Add disabled_if parameter and use it for kvmtool
  run_tests: Enable kvmtool

 arm/efi/run             |   8 ++
 arm/run                 | 164 +++++++++++++++++++++++++---------------
 arm/unittests.cfg       |  34 +++++++++
 docs/unittests.txt      |  43 +++++++++--
 powerpc/run             |   2 +-
 riscv/run               |   4 +-
 run_tests.sh            |  50 ++++++++----
 s390x/run               |   2 +-
 scripts/arch-run.bash   |  80 ++++++++++++++++++--
 scripts/common.bash     |  63 +++++++++------
 scripts/mkstandalone.sh |   9 +++
 scripts/runtime.bash    |  64 +++++++++++++---
 12 files changed, 399 insertions(+), 124 deletions(-)


base-commit: 0ed2cdf3c80ee803b9150898e687e77e4d6f5db2
-- 
2.34.1

[kvm-unit-tests PATCH v2 01/18] run_tests: Document --probe-maxsmp argument

[Alexandru Elisei]

Commit 5dd20ec76ea63 ("runtime: Update MAX_SMP probe") added the
--probe-maxmp argument, but the help message for run_tests.sh wasn't
updated. Document --probe-maxsmp.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 run_tests.sh | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/run_tests.sh b/run_tests.sh
index 152323ffc8a2..f30b6dbd131c 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -17,14 +17,15 @@ cat <<EOF
 
 Usage: $0 [-h] [-v] [-a] [-g group] [-j NUM-TASKS] [-t] [-l]
 
-    -h, --help      Output this help text
-    -v, --verbose   Enables verbose mode
-    -a, --all       Run all tests, including those flagged as 'nodefault'
-                    and those guarded by errata.
-    -g, --group     Only execute tests in the given group
-    -j, --parallel  Execute tests in parallel
-    -t, --tap13     Output test results in TAP format
-    -l, --list      Only output all tests list
+    -h, --help          Output this help text
+    -v, --verbose       Enables verbose mode
+    -a, --all           Run all tests, including those flagged as 'nodefault'
+                        and those guarded by errata.
+    -g, --group         Only execute tests in the given group
+    -j, --parallel      Execute tests in parallel
+    -t, --tap13         Output test results in TAP format
+    -l, --list          Only output all tests list
+        --probe-maxsmp  Update the maximum number of VCPUs supported by host
 
 Set the environment variable QEMU=/path/to/qemu-system-ARCH to
 specify the appropriate qemu binary for ARCH-run.
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 01/18] run_tests: Document --probe-maxsmp argument

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:42:59PM +0000, Alexandru Elisei wrote:
> Commit 5dd20ec76ea63 ("runtime: Update MAX_SMP probe") added the
> --probe-maxmp argument, but the help message for run_tests.sh wasn't
> updated. Document --probe-maxsmp.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  run_tests.sh | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/run_tests.sh b/run_tests.sh
> index 152323ffc8a2..f30b6dbd131c 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -17,14 +17,15 @@ cat <<EOF
>  
>  Usage: $0 [-h] [-v] [-a] [-g group] [-j NUM-TASKS] [-t] [-l]
>  
> -    -h, --help      Output this help text
> -    -v, --verbose   Enables verbose mode
> -    -a, --all       Run all tests, including those flagged as 'nodefault'
> -                    and those guarded by errata.
> -    -g, --group     Only execute tests in the given group
> -    -j, --parallel  Execute tests in parallel
> -    -t, --tap13     Output test results in TAP format
> -    -l, --list      Only output all tests list
> +    -h, --help          Output this help text
> +    -v, --verbose       Enables verbose mode
> +    -a, --all           Run all tests, including those flagged as 'nodefault'
> +                        and those guarded by errata.
> +    -g, --group         Only execute tests in the given group
> +    -j, --parallel      Execute tests in parallel
> +    -t, --tap13         Output test results in TAP format
> +    -l, --list          Only output all tests list
> +        --probe-maxsmp  Update the maximum number of VCPUs supported by host
>  
>  Set the environment variable QEMU=/path/to/qemu-system-ARCH to
>  specify the appropriate qemu binary for ARCH-run.
> -- 
> 2.47.1
> 
>

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

[kvm-unit-tests PATCH v2 02/18] Document environment variables

[Alexandru Elisei]

Document the environment variables that influence how a test is executed
by the run_tests.sh test runner.

Suggested-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 docs/unittests.txt |  5 ++++-
 run_tests.sh       | 12 +++++++++---
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/docs/unittests.txt b/docs/unittests.txt
index c4269f6230c8..dbc2c11e3b59 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -88,7 +88,8 @@ timeout
 -------
 timeout = <duration>
 
-Optional timeout in seconds, after which the test will be killed and fail.
+Optional timeout in seconds, after which the test will be killed and fail. Can
+be overwritten with the TIMEOUT=<duration> environment variable.
 
 check
 -----
@@ -99,3 +100,5 @@ can contain multiple files to check separated by a space, but each check
 parameter needs to be of the form <path>=<value>
 
 The path and value cannot contain space, =, or shell wildcard characters.
+
+Can be overwritten with the CHECK environment variable with the same syntax.
diff --git a/run_tests.sh b/run_tests.sh
index f30b6dbd131c..23d81b2caaa1 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -27,9 +27,15 @@ Usage: $0 [-h] [-v] [-a] [-g group] [-j NUM-TASKS] [-t] [-l]
     -l, --list          Only output all tests list
         --probe-maxsmp  Update the maximum number of VCPUs supported by host
 
-Set the environment variable QEMU=/path/to/qemu-system-ARCH to
-specify the appropriate qemu binary for ARCH-run.
-
+The following environment variables are used:
+
+    QEMU            Path to QEMU binary for ARCH-run
+    ACCEL           QEMU accelerator to use, e.g. 'kvm', 'hvf' or 'tcg'
+    ACCEL_PROPS     Extra argument to ACCEL
+    MACHINE         QEMU machine type
+    TIMEOUT         Timeout duration for the timeout(1) command
+    CHECK           Overwrites the 'check' unit test parameter (see
+                    docs/unittests.txt)
 EOF
 }
 
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 02/18] Document environment variables

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:43:00PM +0000, Alexandru Elisei wrote:
> Document the environment variables that influence how a test is executed
> by the run_tests.sh test runner.
> 
> Suggested-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  docs/unittests.txt |  5 ++++-
>  run_tests.sh       | 12 +++++++++---
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/unittests.txt b/docs/unittests.txt
> index c4269f6230c8..dbc2c11e3b59 100644
> --- a/docs/unittests.txt
> +++ b/docs/unittests.txt
> @@ -88,7 +88,8 @@ timeout
>  -------
>  timeout = <duration>
>  
> -Optional timeout in seconds, after which the test will be killed and fail.
> +Optional timeout in seconds, after which the test will be killed and fail. Can
> +be overwritten with the TIMEOUT=<duration> environment variable.
>  
>  check
>  -----
> @@ -99,3 +100,5 @@ can contain multiple files to check separated by a space, but each check
>  parameter needs to be of the form <path>=<value>
>  
>  The path and value cannot contain space, =, or shell wildcard characters.
> +
> +Can be overwritten with the CHECK environment variable with the same syntax.
> diff --git a/run_tests.sh b/run_tests.sh
> index f30b6dbd131c..23d81b2caaa1 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -27,9 +27,15 @@ Usage: $0 [-h] [-v] [-a] [-g group] [-j NUM-TASKS] [-t] [-l]
>      -l, --list          Only output all tests list
>          --probe-maxsmp  Update the maximum number of VCPUs supported by host
>  
> -Set the environment variable QEMU=/path/to/qemu-system-ARCH to
> -specify the appropriate qemu binary for ARCH-run.
> -
> +The following environment variables are used:
> +
> +    QEMU            Path to QEMU binary for ARCH-run
> +    ACCEL           QEMU accelerator to use, e.g. 'kvm', 'hvf' or 'tcg'
> +    ACCEL_PROPS     Extra argument to ACCEL
> +    MACHINE         QEMU machine type
> +    TIMEOUT         Timeout duration for the timeout(1) command
> +    CHECK           Overwrites the 'check' unit test parameter (see
> +                    docs/unittests.txt)
>  EOF
>  }
>  
> -- 
> 2.47.1
>

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

[kvm-unit-tests PATCH v2 03/18] scripts: Refuse to run the tests if not configured for qemu

[Alexandru Elisei]

Arm and arm64 support running the tests under kvmtool. Unsurprisingly,
kvmtool and qemu have a different command line syntax for configuring and
running a virtual machine.

On top of that, when kvm-unit-tests has been configured to run under
kvmtool (via ./configure --target=kvmtool), the early UART address changes,
and if then the tests are run with qemu, this warning is displayed:

WARNING: early print support may not work. Found uart at 0x9000000, but early base is 0x1000000.

At the moment, the only way to run a test under kvmtool is manually, as no
script has any knowledge of how to invoke kvmtool. Also, unless one looks
at the logs, it's not obvious that the test runner is using qemu to run the
tests, and not kvmtool.

To avoid any confusion for unsuspecting users, refuse to run a test via the
testing scripts when kvm-unit-tests has been configured for kvmtool.

There are four different ways to run a test using the test infrastructure:
with run_tests.sh, by invoking arm/run or arm/efi/run with the correct
parameters (only the arm directory is mentioned here because the tests can
be configured for kvmtool only on arm and arm64), and by creating
standalone tests. Add a check in each of these locations for the supported
virtual machine manager.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arm/efi/run             | 8 ++++++++
 arm/run                 | 9 +++++++++
 run_tests.sh            | 8 ++++++++
 scripts/mkstandalone.sh | 8 ++++++++
 4 files changed, 33 insertions(+)

diff --git a/arm/efi/run b/arm/efi/run
index 8f41fc02df31..916f4c4deef6 100755
--- a/arm/efi/run
+++ b/arm/efi/run
@@ -12,6 +12,14 @@ fi
 source config.mak
 source scripts/arch-run.bash
 
+case "$TARGET" in
+qemu)
+    ;;
+*)
+    echo "$0 does not support '$TARGET'"
+    exit 2
+esac
+
 if [ -f /usr/share/qemu-efi-aarch64/QEMU_EFI.fd ]; then
 	DEFAULT_UEFI=/usr/share/qemu-efi-aarch64/QEMU_EFI.fd
 elif [ -f /usr/share/edk2/aarch64/QEMU_EFI.silent.fd ]; then
diff --git a/arm/run b/arm/run
index efdd44ce86a7..6db32cf09c88 100755
--- a/arm/run
+++ b/arm/run
@@ -8,6 +8,15 @@ if [ -z "$KUT_STANDALONE" ]; then
 	source config.mak
 	source scripts/arch-run.bash
 fi
+
+case "$TARGET" in
+qemu)
+    ;;
+*)
+   echo "'$TARGET' not supported"
+   exit 3
+esac
+
 processor="$PROCESSOR"
 
 if [ "$QEMU" ] && [ -z "$ACCEL" ] &&
diff --git a/run_tests.sh b/run_tests.sh
index 23d81b2caaa1..61480d0c05ed 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -100,6 +100,14 @@ while [ $# -gt 0 ]; do
     shift
 done
 
+case "$TARGET" in
+qemu)
+    ;;
+*)
+    echo "$0 does not support '$TARGET'"
+    exit 2
+esac
+
 # RUNTIME_log_file will be configured later
 if [[ $tap_output == "no" ]]; then
     process_test_output() { cat >> $RUNTIME_log_file; }
diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 2318a85f0706..4de97056e641 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -7,6 +7,14 @@ fi
 source config.mak
 source scripts/common.bash
 
+case "$TARGET" in
+qemu)
+    ;;
+*)
+    echo "'$TARGET' not supported for standlone tests"
+    exit 2
+esac
+
 temp_file ()
 {
 	local var="$1"
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 03/18] scripts: Refuse to run the tests if not configured for qemu

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:43:01PM +0000, Alexandru Elisei wrote:
> Arm and arm64 support running the tests under kvmtool. Unsurprisingly,
> kvmtool and qemu have a different command line syntax for configuring and
> running a virtual machine.
> 
> On top of that, when kvm-unit-tests has been configured to run under
> kvmtool (via ./configure --target=kvmtool), the early UART address changes,
> and if then the tests are run with qemu, this warning is displayed:
> 
> WARNING: early print support may not work. Found uart at 0x9000000, but early base is 0x1000000.
> 
> At the moment, the only way to run a test under kvmtool is manually, as no
> script has any knowledge of how to invoke kvmtool. Also, unless one looks
> at the logs, it's not obvious that the test runner is using qemu to run the
> tests, and not kvmtool.
> 
> To avoid any confusion for unsuspecting users, refuse to run a test via the
> testing scripts when kvm-unit-tests has been configured for kvmtool.
> 
> There are four different ways to run a test using the test infrastructure:
> with run_tests.sh, by invoking arm/run or arm/efi/run with the correct
> parameters (only the arm directory is mentioned here because the tests can
> be configured for kvmtool only on arm and arm64), and by creating
> standalone tests. Add a check in each of these locations for the supported
> virtual machine manager.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  arm/efi/run             | 8 ++++++++
>  arm/run                 | 9 +++++++++
>  run_tests.sh            | 8 ++++++++
>  scripts/mkstandalone.sh | 8 ++++++++
>  4 files changed, 33 insertions(+)
> 
> diff --git a/arm/efi/run b/arm/efi/run
> index 8f41fc02df31..916f4c4deef6 100755
> --- a/arm/efi/run
> +++ b/arm/efi/run
> @@ -12,6 +12,14 @@ fi
>  source config.mak
>  source scripts/arch-run.bash
>  
> +case "$TARGET" in
> +qemu)
> +    ;;
> +*)
> +    echo "$0 does not support '$TARGET'"
> +    exit 2
> +esac
> +
>  if [ -f /usr/share/qemu-efi-aarch64/QEMU_EFI.fd ]; then
>  	DEFAULT_UEFI=/usr/share/qemu-efi-aarch64/QEMU_EFI.fd
>  elif [ -f /usr/share/edk2/aarch64/QEMU_EFI.silent.fd ]; then
> diff --git a/arm/run b/arm/run
> index efdd44ce86a7..6db32cf09c88 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -8,6 +8,15 @@ if [ -z "$KUT_STANDALONE" ]; then
>  	source config.mak
>  	source scripts/arch-run.bash
>  fi
> +
> +case "$TARGET" in
> +qemu)
> +    ;;
> +*)
> +   echo "'$TARGET' not supported"
> +   exit 3

I think we want exit code 2 here.

> +esac
> +
>  processor="$PROCESSOR"
>  
>  if [ "$QEMU" ] && [ -z "$ACCEL" ] &&
> diff --git a/run_tests.sh b/run_tests.sh
> index 23d81b2caaa1..61480d0c05ed 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -100,6 +100,14 @@ while [ $# -gt 0 ]; do
>      shift
>  done
>  
> +case "$TARGET" in
> +qemu)
> +    ;;
> +*)
> +    echo "$0 does not support '$TARGET'"
> +    exit 2
> +esac
> +
>  # RUNTIME_log_file will be configured later
>  if [[ $tap_output == "no" ]]; then
>      process_test_output() { cat >> $RUNTIME_log_file; }
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index 2318a85f0706..4de97056e641 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -7,6 +7,14 @@ fi
>  source config.mak
>  source scripts/common.bash
>  
> +case "$TARGET" in
> +qemu)
> +    ;;
> +*)
> +    echo "'$TARGET' not supported for standlone tests"
> +    exit 2
> +esac
> +
>  temp_file ()
>  {
>  	local var="$1"
> -- 
> 2.47.1
>

I think we could put the check in a function in scripts/arch-run.bash and
just use the same error message for all cases.

Thanks,
drew

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

Re: [kvm-unit-tests PATCH v2 03/18] scripts: Refuse to run the tests if not configured for qemu

[Alexandru Elisei]

Hi Drew,

On Tue, Jan 21, 2025 at 03:48:55PM +0100, Andrew Jones wrote:
> On Mon, Jan 20, 2025 at 04:43:01PM +0000, Alexandru Elisei wrote:
> > Arm and arm64 support running the tests under kvmtool. Unsurprisingly,
> > kvmtool and qemu have a different command line syntax for configuring and
> > running a virtual machine.
> > 
> > On top of that, when kvm-unit-tests has been configured to run under
> > kvmtool (via ./configure --target=kvmtool), the early UART address changes,
> > and if then the tests are run with qemu, this warning is displayed:
> > 
> > WARNING: early print support may not work. Found uart at 0x9000000, but early base is 0x1000000.
> > 
> > At the moment, the only way to run a test under kvmtool is manually, as no
> > script has any knowledge of how to invoke kvmtool. Also, unless one looks
> > at the logs, it's not obvious that the test runner is using qemu to run the
> > tests, and not kvmtool.
> > 
> > To avoid any confusion for unsuspecting users, refuse to run a test via the
> > testing scripts when kvm-unit-tests has been configured for kvmtool.
> > 
> > There are four different ways to run a test using the test infrastructure:
> > with run_tests.sh, by invoking arm/run or arm/efi/run with the correct
> > parameters (only the arm directory is mentioned here because the tests can
> > be configured for kvmtool only on arm and arm64), and by creating
> > standalone tests. Add a check in each of these locations for the supported
> > virtual machine manager.
> > 
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > ---
> >  arm/efi/run             | 8 ++++++++
> >  arm/run                 | 9 +++++++++
> >  run_tests.sh            | 8 ++++++++
> >  scripts/mkstandalone.sh | 8 ++++++++
> >  4 files changed, 33 insertions(+)
> > 
> > diff --git a/arm/efi/run b/arm/efi/run
> > index 8f41fc02df31..916f4c4deef6 100755
> > --- a/arm/efi/run
> > +++ b/arm/efi/run
> > @@ -12,6 +12,14 @@ fi
> >  source config.mak
> >  source scripts/arch-run.bash
> >  
> > +case "$TARGET" in
> > +qemu)
> > +    ;;
> > +*)
> > +    echo "$0 does not support '$TARGET'"
> > +    exit 2
> > +esac
> > +
> >  if [ -f /usr/share/qemu-efi-aarch64/QEMU_EFI.fd ]; then
> >  	DEFAULT_UEFI=/usr/share/qemu-efi-aarch64/QEMU_EFI.fd
> >  elif [ -f /usr/share/edk2/aarch64/QEMU_EFI.silent.fd ]; then
> > diff --git a/arm/run b/arm/run
> > index efdd44ce86a7..6db32cf09c88 100755
> > --- a/arm/run
> > +++ b/arm/run
> > @@ -8,6 +8,15 @@ if [ -z "$KUT_STANDALONE" ]; then
> >  	source config.mak
> >  	source scripts/arch-run.bash
> >  fi
> > +
> > +case "$TARGET" in
> > +qemu)
> > +    ;;
> > +*)
> > +   echo "'$TARGET' not supported"
> > +   exit 3
> 
> I think we want exit code 2 here.

Exit code 2 is already in use in arm/run. Now that I'm looking more closely
at it, exit code 2 is already in use in run_tests.sh, same for
mkstandalone.sh and arm/efi/run.

How about using 3 everywhere as the exit code?

Also, your idea (below) to use a function to test for supported $TARGETs is
a very good one, I'll do it in the next iteration.

Thanks,
Alex

> 
> > +esac
> > +
> >  processor="$PROCESSOR"
> >  
> >  if [ "$QEMU" ] && [ -z "$ACCEL" ] &&
> > diff --git a/run_tests.sh b/run_tests.sh
> > index 23d81b2caaa1..61480d0c05ed 100755
> > --- a/run_tests.sh
> > +++ b/run_tests.sh
> > @@ -100,6 +100,14 @@ while [ $# -gt 0 ]; do
> >      shift
> >  done
> >  
> > +case "$TARGET" in
> > +qemu)
> > +    ;;
> > +*)
> > +    echo "$0 does not support '$TARGET'"
> > +    exit 2
> > +esac
> > +
> >  # RUNTIME_log_file will be configured later
> >  if [[ $tap_output == "no" ]]; then
> >      process_test_output() { cat >> $RUNTIME_log_file; }
> > diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> > index 2318a85f0706..4de97056e641 100755
> > --- a/scripts/mkstandalone.sh
> > +++ b/scripts/mkstandalone.sh
> > @@ -7,6 +7,14 @@ fi
> >  source config.mak
> >  source scripts/common.bash
> >  
> > +case "$TARGET" in
> > +qemu)
> > +    ;;
> > +*)
> > +    echo "'$TARGET' not supported for standlone tests"
> > +    exit 2
> > +esac
> > +
> >  temp_file ()
> >  {
> >  	local var="$1"
> > -- 
> > 2.47.1
> >
> 
> I think we could put the check in a function in scripts/arch-run.bash and
> just use the same error message for all cases.
> 
> Thanks,
> drew
> 
> > 
> > -- 
> > kvm-riscv mailing list
> > kvm-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [kvm-unit-tests PATCH v2 03/18] scripts: Refuse to run the tests if not configured for qemu

[Andrew Jones]

On Tue, Jan 21, 2025 at 03:54:17PM +0000, Alexandru Elisei wrote:
> Hi Drew,
> 
> On Tue, Jan 21, 2025 at 03:48:55PM +0100, Andrew Jones wrote:
> > On Mon, Jan 20, 2025 at 04:43:01PM +0000, Alexandru Elisei wrote:
> > > Arm and arm64 support running the tests under kvmtool. Unsurprisingly,
> > > kvmtool and qemu have a different command line syntax for configuring and
> > > running a virtual machine.
> > > 
> > > On top of that, when kvm-unit-tests has been configured to run under
> > > kvmtool (via ./configure --target=kvmtool), the early UART address changes,
> > > and if then the tests are run with qemu, this warning is displayed:
> > > 
> > > WARNING: early print support may not work. Found uart at 0x9000000, but early base is 0x1000000.
> > > 
> > > At the moment, the only way to run a test under kvmtool is manually, as no
> > > script has any knowledge of how to invoke kvmtool. Also, unless one looks
> > > at the logs, it's not obvious that the test runner is using qemu to run the
> > > tests, and not kvmtool.
> > > 
> > > To avoid any confusion for unsuspecting users, refuse to run a test via the
> > > testing scripts when kvm-unit-tests has been configured for kvmtool.
> > > 
> > > There are four different ways to run a test using the test infrastructure:
> > > with run_tests.sh, by invoking arm/run or arm/efi/run with the correct
> > > parameters (only the arm directory is mentioned here because the tests can
> > > be configured for kvmtool only on arm and arm64), and by creating
> > > standalone tests. Add a check in each of these locations for the supported
> > > virtual machine manager.
> > > 
> > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > > ---
> > >  arm/efi/run             | 8 ++++++++
> > >  arm/run                 | 9 +++++++++
> > >  run_tests.sh            | 8 ++++++++
> > >  scripts/mkstandalone.sh | 8 ++++++++
> > >  4 files changed, 33 insertions(+)
> > > 
> > > diff --git a/arm/efi/run b/arm/efi/run
> > > index 8f41fc02df31..916f4c4deef6 100755
> > > --- a/arm/efi/run
> > > +++ b/arm/efi/run
> > > @@ -12,6 +12,14 @@ fi
> > >  source config.mak
> > >  source scripts/arch-run.bash
> > >  
> > > +case "$TARGET" in
> > > +qemu)
> > > +    ;;
> > > +*)
> > > +    echo "$0 does not support '$TARGET'"
> > > +    exit 2
> > > +esac
> > > +
> > >  if [ -f /usr/share/qemu-efi-aarch64/QEMU_EFI.fd ]; then
> > >  	DEFAULT_UEFI=/usr/share/qemu-efi-aarch64/QEMU_EFI.fd
> > >  elif [ -f /usr/share/edk2/aarch64/QEMU_EFI.silent.fd ]; then
> > > diff --git a/arm/run b/arm/run
> > > index efdd44ce86a7..6db32cf09c88 100755
> > > --- a/arm/run
> > > +++ b/arm/run
> > > @@ -8,6 +8,15 @@ if [ -z "$KUT_STANDALONE" ]; then
> > >  	source config.mak
> > >  	source scripts/arch-run.bash
> > >  fi
> > > +
> > > +case "$TARGET" in
> > > +qemu)
> > > +    ;;
> > > +*)
> > > +   echo "'$TARGET' not supported"
> > > +   exit 3
> > 
> > I think we want exit code 2 here.
> 
> Exit code 2 is already in use in arm/run. Now that I'm looking more closely
> at it, exit code 2 is already in use in run_tests.sh, same for
> mkstandalone.sh and arm/efi/run.
> 
> How about using 3 everywhere as the exit code?
>

In kvm-unit-tests, exit code 2 is what we use for "most likely a run
script failed" (see the comment above run_qemu() in
scripts/arch-run.bash). We don't try to create a new error code for each
type of error, but we do have the error message as well. So if there's a
higher level runner, which runs this runner, it only needs to learn that
2 is likely a script failure and that an error message will hopefully
point the way to the problem.

Thanks,
drew

Re: [kvm-unit-tests PATCH v2 03/18] scripts: Refuse to run the tests if not configured for qemu

[Alexandru Elisei]

Hi Drew,

On Tue, Jan 21, 2025 at 05:17:22PM +0100, Andrew Jones wrote:
> On Tue, Jan 21, 2025 at 03:54:17PM +0000, Alexandru Elisei wrote:
> > Hi Drew,
> > 
> > On Tue, Jan 21, 2025 at 03:48:55PM +0100, Andrew Jones wrote:
> > > On Mon, Jan 20, 2025 at 04:43:01PM +0000, Alexandru Elisei wrote:
> > > > Arm and arm64 support running the tests under kvmtool. Unsurprisingly,
> > > > kvmtool and qemu have a different command line syntax for configuring and
> > > > running a virtual machine.
> > > > 
> > > > On top of that, when kvm-unit-tests has been configured to run under
> > > > kvmtool (via ./configure --target=kvmtool), the early UART address changes,
> > > > and if then the tests are run with qemu, this warning is displayed:
> > > > 
> > > > WARNING: early print support may not work. Found uart at 0x9000000, but early base is 0x1000000.
> > > > 
> > > > At the moment, the only way to run a test under kvmtool is manually, as no
> > > > script has any knowledge of how to invoke kvmtool. Also, unless one looks
> > > > at the logs, it's not obvious that the test runner is using qemu to run the
> > > > tests, and not kvmtool.
> > > > 
> > > > To avoid any confusion for unsuspecting users, refuse to run a test via the
> > > > testing scripts when kvm-unit-tests has been configured for kvmtool.
> > > > 
> > > > There are four different ways to run a test using the test infrastructure:
> > > > with run_tests.sh, by invoking arm/run or arm/efi/run with the correct
> > > > parameters (only the arm directory is mentioned here because the tests can
> > > > be configured for kvmtool only on arm and arm64), and by creating
> > > > standalone tests. Add a check in each of these locations for the supported
> > > > virtual machine manager.
> > > > 
> > > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > > > ---
> > > >  arm/efi/run             | 8 ++++++++
> > > >  arm/run                 | 9 +++++++++
> > > >  run_tests.sh            | 8 ++++++++
> > > >  scripts/mkstandalone.sh | 8 ++++++++
> > > >  4 files changed, 33 insertions(+)
> > > > 
> > > > diff --git a/arm/efi/run b/arm/efi/run
> > > > index 8f41fc02df31..916f4c4deef6 100755
> > > > --- a/arm/efi/run
> > > > +++ b/arm/efi/run
> > > > @@ -12,6 +12,14 @@ fi
> > > >  source config.mak
> > > >  source scripts/arch-run.bash
> > > >  
> > > > +case "$TARGET" in
> > > > +qemu)
> > > > +    ;;
> > > > +*)
> > > > +    echo "$0 does not support '$TARGET'"
> > > > +    exit 2
> > > > +esac
> > > > +
> > > >  if [ -f /usr/share/qemu-efi-aarch64/QEMU_EFI.fd ]; then
> > > >  	DEFAULT_UEFI=/usr/share/qemu-efi-aarch64/QEMU_EFI.fd
> > > >  elif [ -f /usr/share/edk2/aarch64/QEMU_EFI.silent.fd ]; then
> > > > diff --git a/arm/run b/arm/run
> > > > index efdd44ce86a7..6db32cf09c88 100755
> > > > --- a/arm/run
> > > > +++ b/arm/run
> > > > @@ -8,6 +8,15 @@ if [ -z "$KUT_STANDALONE" ]; then
> > > >  	source config.mak
> > > >  	source scripts/arch-run.bash
> > > >  fi
> > > > +
> > > > +case "$TARGET" in
> > > > +qemu)
> > > > +    ;;
> > > > +*)
> > > > +   echo "'$TARGET' not supported"
> > > > +   exit 3
> > > 
> > > I think we want exit code 2 here.
> > 
> > Exit code 2 is already in use in arm/run. Now that I'm looking more closely
> > at it, exit code 2 is already in use in run_tests.sh, same for
> > mkstandalone.sh and arm/efi/run.
> > 
> > How about using 3 everywhere as the exit code?
> >
> 
> In kvm-unit-tests, exit code 2 is what we use for "most likely a run
> script failed" (see the comment above run_qemu() in
> scripts/arch-run.bash). We don't try to create a new error code for each
> type of error, but we do have the error message as well. So if there's a
> higher level runner, which runs this runner, it only needs to learn that
> 2 is likely a script failure and that an error message will hopefully
> point the way to the problem.
> 

I see, I missed the comment. Will change so it returns 2 everywhere.

Thanks,
Alex

Re: [kvm-unit-tests PATCH v2 03/18] scripts: Refuse to run the tests if not configured for qemu

[Alexandru Elisei]

Hi Drew,

On Tue, Jan 21, 2025 at 03:48:55PM +0100, Andrew Jones wrote:
> On Mon, Jan 20, 2025 at 04:43:01PM +0000, Alexandru Elisei wrote:
<snip>
> > ---
> >  arm/efi/run             | 8 ++++++++
> >  arm/run                 | 9 +++++++++
> >  run_tests.sh            | 8 ++++++++
> >  scripts/mkstandalone.sh | 8 ++++++++
> >  4 files changed, 33 insertions(+)
<snip>
> > +case "$TARGET" in
> > +qemu)
> > +    ;;
> > +*)
> > +    echo "'$TARGET' not supported for standlone tests"
> > +    exit 2
> > +esac
> 
> I think we could put the check in a function in scripts/arch-run.bash and
> just use the same error message for all cases.

Coming back to the series.

arm/efi/run and arm/run source scripts/arch-run.bash; run_tests.sh and
scripts/mkstandalone.sh don't source scripts/arch-run.bash. There doesn't
seem to be a common file that is sourced by all of them.

How about creating a new file in scripts (vmm.bash?) with only this
function?

Thanks,
Alex

> 
> Thanks,
> drew
> 
> > 
> > -- 
> > kvm-riscv mailing list
> > kvm-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [kvm-unit-tests PATCH v2 03/18] scripts: Refuse to run the tests if not configured for qemu

[Andrew Jones]

On Mon, Feb 10, 2025 at 10:41:53AM +0000, Alexandru Elisei wrote:
> Hi Drew,
> 
> On Tue, Jan 21, 2025 at 03:48:55PM +0100, Andrew Jones wrote:
> > On Mon, Jan 20, 2025 at 04:43:01PM +0000, Alexandru Elisei wrote:
> <snip>
> > > ---
> > >  arm/efi/run             | 8 ++++++++
> > >  arm/run                 | 9 +++++++++
> > >  run_tests.sh            | 8 ++++++++
> > >  scripts/mkstandalone.sh | 8 ++++++++
> > >  4 files changed, 33 insertions(+)
> <snip>
> > > +case "$TARGET" in
> > > +qemu)
> > > +    ;;
> > > +*)
> > > +    echo "'$TARGET' not supported for standlone tests"
> > > +    exit 2
> > > +esac
> > 
> > I think we could put the check in a function in scripts/arch-run.bash and
> > just use the same error message for all cases.
> 
> Coming back to the series.
> 
> arm/efi/run and arm/run source scripts/arch-run.bash; run_tests.sh and
> scripts/mkstandalone.sh don't source scripts/arch-run.bash. There doesn't
> seem to be a common file that is sourced by all of them.

scripts/mkstandalone.sh uses arch-run.bash, see generate_test().
run_tests.sh doesn't, but I'm not sure it needs to validate TARGET
since it can leave that to the lower-level scripts.

> 
> How about creating a new file in scripts (vmm.bash?) with only this
> function?

If we need a new file, then we can add one, but I'd try using
arch-run.bash or common.bash first.

Thanks,
drew

Re: [kvm-unit-tests PATCH v2 03/18] scripts: Refuse to run the tests if not configured for qemu

[Alexandru Elisei]

Hi Drew,

On Mon, Feb 10, 2025 at 02:56:25PM +0100, Andrew Jones wrote:
> On Mon, Feb 10, 2025 at 10:41:53AM +0000, Alexandru Elisei wrote:
> > Hi Drew,
> > 
> > On Tue, Jan 21, 2025 at 03:48:55PM +0100, Andrew Jones wrote:
> > > On Mon, Jan 20, 2025 at 04:43:01PM +0000, Alexandru Elisei wrote:
> > <snip>
> > > > ---
> > > >  arm/efi/run             | 8 ++++++++
> > > >  arm/run                 | 9 +++++++++
> > > >  run_tests.sh            | 8 ++++++++
> > > >  scripts/mkstandalone.sh | 8 ++++++++
> > > >  4 files changed, 33 insertions(+)
> > <snip>
> > > > +case "$TARGET" in
> > > > +qemu)
> > > > +    ;;
> > > > +*)
> > > > +    echo "'$TARGET' not supported for standlone tests"
> > > > +    exit 2
> > > > +esac
> > > 
> > > I think we could put the check in a function in scripts/arch-run.bash and
> > > just use the same error message for all cases.
> > 
> > Coming back to the series.
> > 
> > arm/efi/run and arm/run source scripts/arch-run.bash; run_tests.sh and
> > scripts/mkstandalone.sh don't source scripts/arch-run.bash. There doesn't
> > seem to be a common file that is sourced by all of them.
> 
> scripts/mkstandalone.sh uses arch-run.bash, see generate_test().

Are you referring to this bit:

generate_test ()
{
	<snip>
        (echo "#!/usr/bin/env bash"
         cat scripts/arch-run.bash "$TEST_DIR/run")

I think scripts/arch-run.bash would need to be sourced for any functions defined
there to be usable in mkstandalone.sh.

What I was thinking is something like this:

if ! vmm_supported $TARGET; then
	echo "$0 does not support '$TARGET'"
	exit 2
fi

Were you thinking of something else?

I think mkstandalone should error at the top level (when you do make
standalone), and not rely on the individual scripts to error if the VMM is
not supported. That's because I think creating the test files, booting a
machine and copying the files only to find out that kvm-unit-tests was
misconfigured is a pretty suboptimal experience.

> run_tests.sh doesn't, but I'm not sure it needs to validate TARGET
> since it can leave that to the lower-level scripts.

I put the check in arm/run, and removed it from run_tests.sh, and this is
what I got:

$ ./run_tests.sh selftest-setup
SKIP selftest-setup (./arm/run does not supported 'kvmtool')

which looks good to me.

> 
> > 
> > How about creating a new file in scripts (vmm.bash?) with only this
> > function?
> 
> If we need a new file, then we can add one, but I'd try using
> arch-run.bash or common.bash first.

common.bash seems to work (and the name fits), so I'll give that a go.

Thanks,
Alex

Re: [kvm-unit-tests PATCH v2 03/18] scripts: Refuse to run the tests if not configured for qemu

[Al Dunsmuir]

Hello Alexandru,

On Monday, February 10, 2025, 1:04:29 PM, you wrote:

> Hi Drew,

> On Mon, Feb 10, 2025 at 02:56:25PM +0100, Andrew Jones wrote:
>> On Mon, Feb 10, 2025 at 10:41:53AM +0000, Alexandru Elisei wrote:
>> > Hi Drew,
>> > 
>> > On Tue, Jan 21, 2025 at 03:48:55PM +0100, Andrew Jones wrote:
>> > > On Mon, Jan 20, 2025 at 04:43:01PM +0000, Alexandru Elisei wrote:
>> > <snip>
>> > > > ---
>> > > >  arm/efi/run             | 8 ++++++++
>> > > >  arm/run                 | 9 +++++++++
>> > > >  run_tests.sh            | 8 ++++++++
>> > > >  scripts/mkstandalone.sh | 8 ++++++++
>> > > >  4 files changed, 33 insertions(+)
>> > <snip>
>> > > > +case "$TARGET" in
>> > > > +qemu)
>> > > > +    ;;
>> > > > +*)
>> > > > +    echo "'$TARGET' not supported for standlone tests"
>> > > > +    exit 2
>> > > > +esac
>> > > 
>> > > I think we could put the check in a function in scripts/arch-run.bash and
>> > > just use the same error message for all cases.
>> > 
>> > Coming back to the series.
>> > 
>> > arm/efi/run and arm/run source scripts/arch-run.bash; run_tests.sh and
>> > scripts/mkstandalone.sh don't source scripts/arch-run.bash. There doesn't
>> > seem to be a common file that is sourced by all of them.
>> 
>> scripts/mkstandalone.sh uses arch-run.bash, see generate_test().

> Are you referring to this bit:

> generate_test ()
> {
>         <snip>
>         (echo "#!/usr/bin/env bash"
>          cat scripts/arch-run.bash "$TEST_DIR/run")

> I think scripts/arch-run.bash would need to be sourced for any functions defined
> there to be usable in mkstandalone.sh.

> What I was thinking is something like this:

> if ! vmm_supported $TARGET; then
>         echo "$0 does not support '$TARGET'"
>         exit 2
> fi

> Were you thinking of something else?

> I think mkstandalone should error at the top level (when you do make
> standalone), and not rely on the individual scripts to error if the VMM is
> not supported. That's because I think creating the test files, booting a
> machine and copying the files only to find out that kvm-unit-tests was
> misconfigured is a pretty suboptimal experience.

>> run_tests.sh doesn't, but I'm not sure it needs to validate TARGET
>> since it can leave that to the lower-level scripts.

> I put the check in arm/run, and removed it from run_tests.sh, and this is
> what I got:

> $ ./run_tests.sh selftest-setup
> SKIP selftest-setup (./arm/run does not supported 'kvmtool')

> which looks good to me.

Grammar nit:  This should be
SKIP selftest-setup (./arm/run does not support 'kvmtool')

Al

>> 
>> > 
>> > How about creating a new file in scripts (vmm.bash?) with only this
>> > function?
>> 
>> If we need a new file, then we can add one, but I'd try using
>> arch-run.bash or common.bash first.

> common.bash seems to work (and the name fits), so I'll give that a go.

> Thanks,
> Alex

[kvm-unit-tests PATCH v2 04/18] run_tests: Introduce unittest parameter 'qemu_params'

[Alexandru Elisei]

Tests for the arm and arm64 architectures can also be run with kvmtool, and
work is under way to have it supported by the run_tests.sh test runner. Not
suprisingly, kvmtool has a different syntax than qemu when configuring and
running a virtual machine.

Add a new unittest parameter, 'qemu_params', with the goal to add a similar
parameter for each virtual machine manager that run_tests.sh supports.

'qemu_params' and 'extra_params' are interchangeable, but it is expected
that going forward new tests will use 'qemu_params'. A test should have
only one of the two parameters.

While we're at it, rename the variable opts to qemu_opts to match the new
unit configuration name, and to make it easier to distinguish from the
kvmtool parameters when they'll be added.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 docs/unittests.txt   | 17 +++++++++-----
 scripts/common.bash  | 53 ++++++++++++++++++++++++++------------------
 scripts/runtime.bash | 10 ++++-----
 3 files changed, 47 insertions(+), 33 deletions(-)

diff --git a/docs/unittests.txt b/docs/unittests.txt
index dbc2c11e3b59..3e1a9e563016 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -24,9 +24,9 @@ param = value format.
 
 Available parameters
 ====================
-Note! Some parameters like smp and extra_params modify how a test is run,
-while others like arch and accel restrict the configurations in which the
-test is run.
+Note! Some parameters like smp and qemu_params/extra_params modify how a
+test is run, while others like arch and accel restrict the configurations
+in which the test is run.
 
 file
 ----
@@ -56,13 +56,18 @@ smp = <number>
 Optional, the number of processors created in the machine to run the test.
 Defaults to 1. $MAX_SMP can be used to specify the maximum supported.
 
-extra_params
-------------
+qemu_params
+-----------
 These are extra parameters supplied to the QEMU process. -append '...' can
 be used to pass arguments into the test case argv. Multiple parameters can
 be added, for example:
 
-extra_params = -m 256 -append 'smp=2'
+qemu_params = -m 256 -append 'smp=2'
+
+extra_params
+------------
+Alias for 'qemu_params', supported for compatibility purposes. Use
+'qemu_params' for new tests.
 
 groups
 ------
diff --git a/scripts/common.bash b/scripts/common.bash
index 3aa557c8c03d..a40c28121b6a 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -1,5 +1,28 @@
 source config.mak
 
+function parse_opts()
+{
+	local opts="$1"
+	local fd="$2"
+
+	while read -r -u $fd; do
+		#escape backslash newline, but not double backslash
+		if [[ $opts =~ [^\\]*(\\*)$'\n'$ ]]; then
+			if (( ${#BASH_REMATCH[1]} % 2 == 1 )); then
+				opts=${opts%\\$'\n'}
+			fi
+		fi
+		if [[ "$REPLY" =~ ^(.*)'"""'[:blank:]*$ ]]; then
+			opts+=${BASH_REMATCH[1]}
+			break
+		else
+			opts+=$REPLY$'\n'
+		fi
+	done
+
+	echo "$opts"
+}
+
 function for_each_unittest()
 {
 	local unittests="$1"
@@ -7,7 +30,7 @@ function for_each_unittest()
 	local testname
 	local smp
 	local kernel
-	local opts
+	local qemu_opts
 	local groups
 	local arch
 	local machine
@@ -22,12 +45,12 @@ function for_each_unittest()
 		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
 			rematch=${BASH_REMATCH[1]}
 			if [ -n "${testname}" ]; then
-				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
+				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout"
 			fi
 			testname=$rematch
 			smp=1
 			kernel=""
-			opts=""
+			qemu_opts=""
 			groups=""
 			arch=""
 			machine=""
@@ -38,24 +61,10 @@ function for_each_unittest()
 			kernel=$TEST_DIR/${BASH_REMATCH[1]}
 		elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
 			smp=${BASH_REMATCH[1]}
-		elif [[ $line =~ ^extra_params\ *=\ *'"""'(.*)$ ]]; then
-			opts=${BASH_REMATCH[1]}$'\n'
-			while read -r -u $fd; do
-				#escape backslash newline, but not double backslash
-				if [[ $opts =~ [^\\]*(\\*)$'\n'$ ]]; then
-					if (( ${#BASH_REMATCH[1]} % 2 == 1 )); then
-						opts=${opts%\\$'\n'}
-					fi
-				fi
-				if [[ "$REPLY" =~ ^(.*)'"""'[:blank:]*$ ]]; then
-					opts+=${BASH_REMATCH[1]}
-					break
-				else
-					opts+=$REPLY$'\n'
-				fi
-			done
-		elif [[ $line =~ ^extra_params\ *=\ *(.*)$ ]]; then
-			opts=${BASH_REMATCH[1]}
+		elif [[ $line =~ ^(extra_params|qemu_params)\ *=\ *'"""'(.*)$ ]]; then
+			qemu_opts=$(parse_opts ${BASH_REMATCH[2]}$'\n' $fd)
+		elif [[ $line =~ ^(extra_params|qemu_params)\ *=\ *(.*)$ ]]; then
+			qemu_opts=${BASH_REMATCH[2]}
 		elif [[ $line =~ ^groups\ *=\ *(.*)$ ]]; then
 			groups=${BASH_REMATCH[1]}
 		elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
@@ -71,7 +80,7 @@ function for_each_unittest()
 		fi
 	done
 	if [ -n "${testname}" ]; then
-		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
+		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout"
 	fi
 	exec {fd}<&-
 }
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 4b9c7d6b7c39..e5d661684ceb 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -34,7 +34,7 @@ premature_failure()
 get_cmdline()
 {
     local kernel=$1
-    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
+    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $qemu_opts"
 }
 
 skip_nodefault()
@@ -80,7 +80,7 @@ function run()
     local groups="$2"
     local smp="$3"
     local kernel="$4"
-    local opts="$5"
+    local qemu_opts="$5"
     local arch="$6"
     local machine="$7"
     local check="${CHECK:-$8}"
@@ -179,9 +179,9 @@ function run()
         echo $cmdline
     fi
 
-    # extra_params in the config file may contain backticks that need to be
-    # expanded, so use eval to start qemu.  Use "> >(foo)" instead of a pipe to
-    # preserve the exit status.
+    # qemu_params/extra_params in the config file may contain backticks that
+    # need to be expanded, so use eval to start qemu.  Use "> >(foo)" instead of
+    # a pipe to preserve the exit status.
     summary=$(eval "$cmdline" 2> >(RUNTIME_log_stderr $testname) \
                              > >(tee >(RUNTIME_log_stdout $testname $kernel) | extract_summary))
     ret=$?
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 04/18] run_tests: Introduce unittest parameter 'qemu_params'

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:43:02PM +0000, Alexandru Elisei wrote:
> Tests for the arm and arm64 architectures can also be run with kvmtool, and
> work is under way to have it supported by the run_tests.sh test runner. Not
> suprisingly, kvmtool has a different syntax than qemu when configuring and
> running a virtual machine.
> 
> Add a new unittest parameter, 'qemu_params', with the goal to add a similar
> parameter for each virtual machine manager that run_tests.sh supports.
> 
> 'qemu_params' and 'extra_params' are interchangeable, but it is expected
> that going forward new tests will use 'qemu_params'. A test should have
> only one of the two parameters.
> 
> While we're at it, rename the variable opts to qemu_opts to match the new
> unit configuration name, and to make it easier to distinguish from the
> kvmtool parameters when they'll be added.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  docs/unittests.txt   | 17 +++++++++-----
>  scripts/common.bash  | 53 ++++++++++++++++++++++++++------------------
>  scripts/runtime.bash | 10 ++++-----
>  3 files changed, 47 insertions(+), 33 deletions(-)
> 
> diff --git a/docs/unittests.txt b/docs/unittests.txt
> index dbc2c11e3b59..3e1a9e563016 100644
> --- a/docs/unittests.txt
> +++ b/docs/unittests.txt
> @@ -24,9 +24,9 @@ param = value format.
>  
>  Available parameters
>  ====================
> -Note! Some parameters like smp and extra_params modify how a test is run,
> -while others like arch and accel restrict the configurations in which the
> -test is run.
> +Note! Some parameters like smp and qemu_params/extra_params modify how a
> +test is run, while others like arch and accel restrict the configurations
> +in which the test is run.
>  
>  file
>  ----
> @@ -56,13 +56,18 @@ smp = <number>
>  Optional, the number of processors created in the machine to run the test.
>  Defaults to 1. $MAX_SMP can be used to specify the maximum supported.
>  
> -extra_params
> -------------
> +qemu_params
> +-----------
>  These are extra parameters supplied to the QEMU process. -append '...' can
>  be used to pass arguments into the test case argv. Multiple parameters can
>  be added, for example:
>  
> -extra_params = -m 256 -append 'smp=2'
> +qemu_params = -m 256 -append 'smp=2'
> +
> +extra_params
> +------------
> +Alias for 'qemu_params', supported for compatibility purposes. Use
> +'qemu_params' for new tests.
>  
>  groups
>  ------
> diff --git a/scripts/common.bash b/scripts/common.bash
> index 3aa557c8c03d..a40c28121b6a 100644
> --- a/scripts/common.bash
> +++ b/scripts/common.bash
> @@ -1,5 +1,28 @@
>  source config.mak
>  
> +function parse_opts()
> +{
> +	local opts="$1"
> +	local fd="$2"
> +
> +	while read -r -u $fd; do
> +		#escape backslash newline, but not double backslash
> +		if [[ $opts =~ [^\\]*(\\*)$'\n'$ ]]; then
> +			if (( ${#BASH_REMATCH[1]} % 2 == 1 )); then
> +				opts=${opts%\\$'\n'}
> +			fi
> +		fi
> +		if [[ "$REPLY" =~ ^(.*)'"""'[:blank:]*$ ]]; then
> +			opts+=${BASH_REMATCH[1]}
> +			break
> +		else
> +			opts+=$REPLY$'\n'
> +		fi
> +	done
> +
> +	echo "$opts"
> +}
> +
>  function for_each_unittest()
>  {
>  	local unittests="$1"
> @@ -7,7 +30,7 @@ function for_each_unittest()
>  	local testname
>  	local smp
>  	local kernel
> -	local opts
> +	local qemu_opts
>  	local groups
>  	local arch
>  	local machine
> @@ -22,12 +45,12 @@ function for_each_unittest()
>  		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
>  			rematch=${BASH_REMATCH[1]}
>  			if [ -n "${testname}" ]; then
> -				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
> +				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout"
>  			fi
>  			testname=$rematch
>  			smp=1
>  			kernel=""
> -			opts=""
> +			qemu_opts=""
>  			groups=""
>  			arch=""
>  			machine=""
> @@ -38,24 +61,10 @@ function for_each_unittest()
>  			kernel=$TEST_DIR/${BASH_REMATCH[1]}
>  		elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
>  			smp=${BASH_REMATCH[1]}
> -		elif [[ $line =~ ^extra_params\ *=\ *'"""'(.*)$ ]]; then
> -			opts=${BASH_REMATCH[1]}$'\n'
> -			while read -r -u $fd; do
> -				#escape backslash newline, but not double backslash
> -				if [[ $opts =~ [^\\]*(\\*)$'\n'$ ]]; then
> -					if (( ${#BASH_REMATCH[1]} % 2 == 1 )); then
> -						opts=${opts%\\$'\n'}
> -					fi
> -				fi
> -				if [[ "$REPLY" =~ ^(.*)'"""'[:blank:]*$ ]]; then
> -					opts+=${BASH_REMATCH[1]}
> -					break
> -				else
> -					opts+=$REPLY$'\n'
> -				fi
> -			done
> -		elif [[ $line =~ ^extra_params\ *=\ *(.*)$ ]]; then
> -			opts=${BASH_REMATCH[1]}
> +		elif [[ $line =~ ^(extra_params|qemu_params)\ *=\ *'"""'(.*)$ ]]; then
> +			qemu_opts=$(parse_opts ${BASH_REMATCH[2]}$'\n' $fd)
> +		elif [[ $line =~ ^(extra_params|qemu_params)\ *=\ *(.*)$ ]]; then
> +			qemu_opts=${BASH_REMATCH[2]}
>  		elif [[ $line =~ ^groups\ *=\ *(.*)$ ]]; then
>  			groups=${BASH_REMATCH[1]}
>  		elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
> @@ -71,7 +80,7 @@ function for_each_unittest()
>  		fi
>  	done
>  	if [ -n "${testname}" ]; then
> -		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
> +		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout"
>  	fi
>  	exec {fd}<&-
>  }
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 4b9c7d6b7c39..e5d661684ceb 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -34,7 +34,7 @@ premature_failure()
>  get_cmdline()
>  {
>      local kernel=$1
> -    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
> +    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $qemu_opts"
>  }
>  
>  skip_nodefault()
> @@ -80,7 +80,7 @@ function run()
>      local groups="$2"
>      local smp="$3"
>      local kernel="$4"
> -    local opts="$5"
> +    local qemu_opts="$5"
>      local arch="$6"
>      local machine="$7"
>      local check="${CHECK:-$8}"
> @@ -179,9 +179,9 @@ function run()
>          echo $cmdline
>      fi
>  
> -    # extra_params in the config file may contain backticks that need to be
> -    # expanded, so use eval to start qemu.  Use "> >(foo)" instead of a pipe to
> -    # preserve the exit status.
> +    # qemu_params/extra_params in the config file may contain backticks that
> +    # need to be expanded, so use eval to start qemu.  Use "> >(foo)" instead of
> +    # a pipe to preserve the exit status.
>      summary=$(eval "$cmdline" 2> >(RUNTIME_log_stderr $testname) \
>                               > >(tee >(RUNTIME_log_stdout $testname $kernel) | extract_summary))
>      ret=$?
> -- 
> 2.47.1
>

Hmm, I'll keep reading the series, but it seems like we should be choosing
generic names like 'extra_params' and 'opts' that we plan to use for both
QEMU and kvmtool since they both have the concepts of "options" and "extra
params".

Thanks,
drew

Re: [kvm-unit-tests PATCH v2 04/18] run_tests: Introduce unittest parameter 'qemu_params'

[Alexandru Elisei]

Hi Drew,

On Tue, Jan 21, 2025 at 04:46:24PM +0100, Andrew Jones wrote:
> On Mon, Jan 20, 2025 at 04:43:02PM +0000, Alexandru Elisei wrote:
> > Tests for the arm and arm64 architectures can also be run with kvmtool, and
> > work is under way to have it supported by the run_tests.sh test runner. Not
> > suprisingly, kvmtool has a different syntax than qemu when configuring and
> > running a virtual machine.
> > 
> > Add a new unittest parameter, 'qemu_params', with the goal to add a similar
> > parameter for each virtual machine manager that run_tests.sh supports.
> > 
> > 'qemu_params' and 'extra_params' are interchangeable, but it is expected
> > that going forward new tests will use 'qemu_params'. A test should have
> > only one of the two parameters.
> > 
> > While we're at it, rename the variable opts to qemu_opts to match the new
> > unit configuration name, and to make it easier to distinguish from the
> > kvmtool parameters when they'll be added.
> > 
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > ---
> >  docs/unittests.txt   | 17 +++++++++-----
> >  scripts/common.bash  | 53 ++++++++++++++++++++++++++------------------
> >  scripts/runtime.bash | 10 ++++-----
> >  3 files changed, 47 insertions(+), 33 deletions(-)
> > 
> > diff --git a/docs/unittests.txt b/docs/unittests.txt
> > index dbc2c11e3b59..3e1a9e563016 100644
> > --- a/docs/unittests.txt
> > +++ b/docs/unittests.txt
> > @@ -24,9 +24,9 @@ param = value format.
> >  
> >  Available parameters
> >  ====================
> > -Note! Some parameters like smp and extra_params modify how a test is run,
> > -while others like arch and accel restrict the configurations in which the
> > -test is run.
> > +Note! Some parameters like smp and qemu_params/extra_params modify how a
> > +test is run, while others like arch and accel restrict the configurations
> > +in which the test is run.
> >  
> >  file
> >  ----
> > @@ -56,13 +56,18 @@ smp = <number>
> >  Optional, the number of processors created in the machine to run the test.
> >  Defaults to 1. $MAX_SMP can be used to specify the maximum supported.
> >  
> > -extra_params
> > -------------
> > +qemu_params
> > +-----------
> >  These are extra parameters supplied to the QEMU process. -append '...' can
> >  be used to pass arguments into the test case argv. Multiple parameters can
> >  be added, for example:
> >  
> > -extra_params = -m 256 -append 'smp=2'
> > +qemu_params = -m 256 -append 'smp=2'
> > +
> > +extra_params
> > +------------
> > +Alias for 'qemu_params', supported for compatibility purposes. Use
> > +'qemu_params' for new tests.
> >  
> >  groups
> >  ------
> > diff --git a/scripts/common.bash b/scripts/common.bash
> > index 3aa557c8c03d..a40c28121b6a 100644
> > --- a/scripts/common.bash
> > +++ b/scripts/common.bash
> > @@ -1,5 +1,28 @@
> >  source config.mak
> >  
> > +function parse_opts()
> > +{
> > +	local opts="$1"
> > +	local fd="$2"
> > +
> > +	while read -r -u $fd; do
> > +		#escape backslash newline, but not double backslash
> > +		if [[ $opts =~ [^\\]*(\\*)$'\n'$ ]]; then
> > +			if (( ${#BASH_REMATCH[1]} % 2 == 1 )); then
> > +				opts=${opts%\\$'\n'}
> > +			fi
> > +		fi
> > +		if [[ "$REPLY" =~ ^(.*)'"""'[:blank:]*$ ]]; then
> > +			opts+=${BASH_REMATCH[1]}
> > +			break
> > +		else
> > +			opts+=$REPLY$'\n'
> > +		fi
> > +	done
> > +
> > +	echo "$opts"
> > +}
> > +
> >  function for_each_unittest()
> >  {
> >  	local unittests="$1"
> > @@ -7,7 +30,7 @@ function for_each_unittest()
> >  	local testname
> >  	local smp
> >  	local kernel
> > -	local opts
> > +	local qemu_opts
> >  	local groups
> >  	local arch
> >  	local machine
> > @@ -22,12 +45,12 @@ function for_each_unittest()
> >  		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
> >  			rematch=${BASH_REMATCH[1]}
> >  			if [ -n "${testname}" ]; then
> > -				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
> > +				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout"
> >  			fi
> >  			testname=$rematch
> >  			smp=1
> >  			kernel=""
> > -			opts=""
> > +			qemu_opts=""
> >  			groups=""
> >  			arch=""
> >  			machine=""
> > @@ -38,24 +61,10 @@ function for_each_unittest()
> >  			kernel=$TEST_DIR/${BASH_REMATCH[1]}
> >  		elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
> >  			smp=${BASH_REMATCH[1]}
> > -		elif [[ $line =~ ^extra_params\ *=\ *'"""'(.*)$ ]]; then
> > -			opts=${BASH_REMATCH[1]}$'\n'
> > -			while read -r -u $fd; do
> > -				#escape backslash newline, but not double backslash
> > -				if [[ $opts =~ [^\\]*(\\*)$'\n'$ ]]; then
> > -					if (( ${#BASH_REMATCH[1]} % 2 == 1 )); then
> > -						opts=${opts%\\$'\n'}
> > -					fi
> > -				fi
> > -				if [[ "$REPLY" =~ ^(.*)'"""'[:blank:]*$ ]]; then
> > -					opts+=${BASH_REMATCH[1]}
> > -					break
> > -				else
> > -					opts+=$REPLY$'\n'
> > -				fi
> > -			done
> > -		elif [[ $line =~ ^extra_params\ *=\ *(.*)$ ]]; then
> > -			opts=${BASH_REMATCH[1]}
> > +		elif [[ $line =~ ^(extra_params|qemu_params)\ *=\ *'"""'(.*)$ ]]; then
> > +			qemu_opts=$(parse_opts ${BASH_REMATCH[2]}$'\n' $fd)
> > +		elif [[ $line =~ ^(extra_params|qemu_params)\ *=\ *(.*)$ ]]; then
> > +			qemu_opts=${BASH_REMATCH[2]}
> >  		elif [[ $line =~ ^groups\ *=\ *(.*)$ ]]; then
> >  			groups=${BASH_REMATCH[1]}
> >  		elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
> > @@ -71,7 +80,7 @@ function for_each_unittest()
> >  		fi
> >  	done
> >  	if [ -n "${testname}" ]; then
> > -		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
> > +		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout"
> >  	fi
> >  	exec {fd}<&-
> >  }
> > diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> > index 4b9c7d6b7c39..e5d661684ceb 100644
> > --- a/scripts/runtime.bash
> > +++ b/scripts/runtime.bash
> > @@ -34,7 +34,7 @@ premature_failure()
> >  get_cmdline()
> >  {
> >      local kernel=$1
> > -    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
> > +    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $qemu_opts"
> >  }
> >  
> >  skip_nodefault()
> > @@ -80,7 +80,7 @@ function run()
> >      local groups="$2"
> >      local smp="$3"
> >      local kernel="$4"
> > -    local opts="$5"
> > +    local qemu_opts="$5"
> >      local arch="$6"
> >      local machine="$7"
> >      local check="${CHECK:-$8}"
> > @@ -179,9 +179,9 @@ function run()
> >          echo $cmdline
> >      fi
> >  
> > -    # extra_params in the config file may contain backticks that need to be
> > -    # expanded, so use eval to start qemu.  Use "> >(foo)" instead of a pipe to
> > -    # preserve the exit status.
> > +    # qemu_params/extra_params in the config file may contain backticks that
> > +    # need to be expanded, so use eval to start qemu.  Use "> >(foo)" instead of
> > +    # a pipe to preserve the exit status.
> >      summary=$(eval "$cmdline" 2> >(RUNTIME_log_stderr $testname) \
> >                               > >(tee >(RUNTIME_log_stdout $testname $kernel) | extract_summary))
> >      ret=$?
> > -- 
> > 2.47.1
> >
> 
> Hmm, I'll keep reading the series, but it seems like we should be choosing
> generic names like 'extra_params' and 'opts' that we plan to use for both
> QEMU and kvmtool since they both have the concepts of "options" and "extra
> params".

I'm afraid I don't follow you. 'qemu_params' was chosen because it uses
qemu-specific syntax. Same for 'kvmtool_params', introduced later in the
series. Are you referring to unittests.cfg or to something else?

Thanks,
Alex

Re: [kvm-unit-tests PATCH v2 04/18] run_tests: Introduce unittest parameter 'qemu_params'

[Andrew Jones]

On Wed, Feb 12, 2025 at 01:40:51PM +0000, Alexandru Elisei wrote:
...
> > > @@ -80,7 +80,7 @@ function run()
> > >      local groups="$2"
> > >      local smp="$3"
> > >      local kernel="$4"
> > > -    local opts="$5"
> > > +    local qemu_opts="$5"
> > >      local arch="$6"
> > >      local machine="$7"
> > >      local check="${CHECK:-$8}"
> > > @@ -179,9 +179,9 @@ function run()
> > >          echo $cmdline
> > >      fi
> > >  
> > > -    # extra_params in the config file may contain backticks that need to be
> > > -    # expanded, so use eval to start qemu.  Use "> >(foo)" instead of a pipe to
> > > -    # preserve the exit status.
> > > +    # qemu_params/extra_params in the config file may contain backticks that
> > > +    # need to be expanded, so use eval to start qemu.  Use "> >(foo)" instead of
> > > +    # a pipe to preserve the exit status.
> > >      summary=$(eval "$cmdline" 2> >(RUNTIME_log_stderr $testname) \
> > >                               > >(tee >(RUNTIME_log_stdout $testname $kernel) | extract_summary))
> > >      ret=$?
> > > -- 
> > > 2.47.1
> > >
> > 
> > Hmm, I'll keep reading the series, but it seems like we should be choosing
> > generic names like 'extra_params' and 'opts' that we plan to use for both
> > QEMU and kvmtool since they both have the concepts of "options" and "extra
> > params".
> 
> I'm afraid I don't follow you. 'qemu_params' was chosen because it uses
> qemu-specific syntax. Same for 'kvmtool_params', introduced later in the
> series. Are you referring to unittests.cfg or to something else?
>

I didn't like the renaming of opts to qemu_opts since both kvmtool and
qemu have "options", so it seems like we should be generalizing variable
names rather than making them more specific. I see later how there may
be a need for qemu_options, kvmtool_options, and unit test
cmdline_options in the unittests.cfg, though. However, here, it seems
like we could still use 'opts' for the variable and just use another
variable to determine if we parse qemu_options or kvmtool_options, since
there shouldn't be a need to parse both.

Thanks,
drew

[kvm-unit-tests PATCH v2 05/18] scripts: Rename run_qemu_status -> run_test_status

[Alexandru Elisei]

From: Alexandru Elisei <alexandru.elisei@gmail.com>

For the arm/arm64 architectures, kvm-unit-tests can also be run using the
kvmtool virtual machine manager. Rename run_qemu_status to run_test_status
to make it more generic, in preparation to add support for kvmtool.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arm/run               | 4 ++--
 powerpc/run           | 2 +-
 riscv/run             | 4 ++--
 s390x/run             | 2 +-
 scripts/arch-run.bash | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arm/run b/arm/run
index 6db32cf09c88..9b11feafffdd 100755
--- a/arm/run
+++ b/arm/run
@@ -85,9 +85,9 @@ command+=" -display none -serial stdio"
 command="$(migration_cmd) $(timeout_cmd) $command"
 
 if [ "$UEFI_SHELL_RUN" = "y" ]; then
-	ENVIRON_DEFAULT=n run_qemu_status $command "$@"
+	ENVIRON_DEFAULT=n run_test_status $command "$@"
 elif [ "$EFI_USE_ACPI" = "y" ]; then
-	run_qemu_status $command -kernel "$@"
+	run_test_status $command -kernel "$@"
 else
 	run_qemu $command -kernel "$@"
 fi
diff --git a/powerpc/run b/powerpc/run
index 27abf1ef6a4d..9b5fbc1197ed 100755
--- a/powerpc/run
+++ b/powerpc/run
@@ -63,4 +63,4 @@ command="$(migration_cmd) $(timeout_cmd) $command"
 # to fixup the fixup below by parsing the true exit code from the output.
 # The second fixup is also a FIXME, because once we add chr-testdev
 # support for powerpc, we won't need the second fixup.
-run_qemu_status $command "$@"
+run_test_status $command "$@"
diff --git a/riscv/run b/riscv/run
index 73f2bf54dc32..2a846d361a4d 100755
--- a/riscv/run
+++ b/riscv/run
@@ -34,8 +34,8 @@ command+=" $mach $acc $firmware -cpu $processor "
 command="$(migration_cmd) $(timeout_cmd) $command"
 
 if [ "$UEFI_SHELL_RUN" = "y" ]; then
-	ENVIRON_DEFAULT=n run_qemu_status $command "$@"
+	ENVIRON_DEFAULT=n run_test_status $command "$@"
 else
 	# We return the exit code via stdout, not via the QEMU return code
-	run_qemu_status $command -kernel "$@"
+	run_test_status $command -kernel "$@"
 fi
diff --git a/s390x/run b/s390x/run
index 34552c2747d4..9ecfaf983a3d 100755
--- a/s390x/run
+++ b/s390x/run
@@ -47,4 +47,4 @@ command+=" -kernel"
 command="$(panic_cmd) $(migration_cmd) $(timeout_cmd) $command"
 
 # We return the exit code via stdout, not via the QEMU return code
-run_qemu_status $command "$@"
+run_test_status $command "$@"
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 8643bab3b252..d6eaf0ee5f09 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -75,7 +75,7 @@ run_qemu ()
 	return $ret
 }
 
-run_qemu_status ()
+run_test_status ()
 {
 	local stdout ret
 
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 05/18] scripts: Rename run_qemu_status -> run_test_status

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:43:03PM +0000, Alexandru Elisei wrote:
> From: Alexandru Elisei <alexandru.elisei@gmail.com>
> 
> For the arm/arm64 architectures, kvm-unit-tests can also be run using the
> kvmtool virtual machine manager. Rename run_qemu_status to run_test_status
> to make it more generic, in preparation to add support for kvmtool.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  arm/run               | 4 ++--
>  powerpc/run           | 2 +-
>  riscv/run             | 4 ++--
>  s390x/run             | 2 +-
>  scripts/arch-run.bash | 2 +-
>  5 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arm/run b/arm/run
> index 6db32cf09c88..9b11feafffdd 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -85,9 +85,9 @@ command+=" -display none -serial stdio"
>  command="$(migration_cmd) $(timeout_cmd) $command"
>  
>  if [ "$UEFI_SHELL_RUN" = "y" ]; then
> -	ENVIRON_DEFAULT=n run_qemu_status $command "$@"
> +	ENVIRON_DEFAULT=n run_test_status $command "$@"
>  elif [ "$EFI_USE_ACPI" = "y" ]; then
> -	run_qemu_status $command -kernel "$@"
> +	run_test_status $command -kernel "$@"
>  else
>  	run_qemu $command -kernel "$@"
>  fi
> diff --git a/powerpc/run b/powerpc/run
> index 27abf1ef6a4d..9b5fbc1197ed 100755
> --- a/powerpc/run
> +++ b/powerpc/run
> @@ -63,4 +63,4 @@ command="$(migration_cmd) $(timeout_cmd) $command"
>  # to fixup the fixup below by parsing the true exit code from the output.
>  # The second fixup is also a FIXME, because once we add chr-testdev
>  # support for powerpc, we won't need the second fixup.
> -run_qemu_status $command "$@"
> +run_test_status $command "$@"
> diff --git a/riscv/run b/riscv/run
> index 73f2bf54dc32..2a846d361a4d 100755
> --- a/riscv/run
> +++ b/riscv/run
> @@ -34,8 +34,8 @@ command+=" $mach $acc $firmware -cpu $processor "
>  command="$(migration_cmd) $(timeout_cmd) $command"
>  
>  if [ "$UEFI_SHELL_RUN" = "y" ]; then
> -	ENVIRON_DEFAULT=n run_qemu_status $command "$@"
> +	ENVIRON_DEFAULT=n run_test_status $command "$@"
>  else
>  	# We return the exit code via stdout, not via the QEMU return code
> -	run_qemu_status $command -kernel "$@"
> +	run_test_status $command -kernel "$@"
>  fi
> diff --git a/s390x/run b/s390x/run
> index 34552c2747d4..9ecfaf983a3d 100755
> --- a/s390x/run
> +++ b/s390x/run
> @@ -47,4 +47,4 @@ command+=" -kernel"
>  command="$(panic_cmd) $(migration_cmd) $(timeout_cmd) $command"
>  
>  # We return the exit code via stdout, not via the QEMU return code
> -run_qemu_status $command "$@"
> +run_test_status $command "$@"
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 8643bab3b252..d6eaf0ee5f09 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -75,7 +75,7 @@ run_qemu ()
>  	return $ret
>  }
>  
> -run_qemu_status ()
> +run_test_status ()
>  {
>  	local stdout ret
>  
> -- 
> 2.47.1

Hmm, run_qemu_status() wraps run_qemu() so it seems appropriately named,
especially since the return value of run_qemu() has had QEMU-specific
return codes considered. It seems we should first decouple
run_qemu_status() from run_qemu() or to sanitize run_qemu() of anything
QEMU-specific and rename it to run_test() at the same time.

Thanks,
drew

[kvm-unit-tests PATCH v2 06/18] scripts: Merge the qemu parameter -smp into $qemu_opts

[Alexandru Elisei]

kvmtool has a different command line parameter to specify the number of
VCPUs (-c/--cpus). To make it easier to accommodate it, merge the qemu
specific parameter -smp into $qemu_opts when passing it to the
$RUNTIME_arch_run script.

This is safe to do because the $RUNTIME_arch_run script, on all
architectures, passes the parameters right back to run_qemu() et co, and
do not treat individual parameters separately.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 scripts/runtime.bash | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index e5d661684ceb..a89f2d10ab78 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -34,7 +34,8 @@ premature_failure()
 get_cmdline()
 {
     local kernel=$1
-    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $qemu_opts"
+
+    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel $qemu_opts"
 }
 
 skip_nodefault()
@@ -87,6 +88,8 @@ function run()
     local accel="$9"
     local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
 
+    qemu_opts="-smp $smp $qemu_opts"
+
     if [ "${CONFIG_EFI}" == "y" ]; then
         kernel=${kernel/%.flat/.efi}
     fi
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 06/18] scripts: Merge the qemu parameter -smp into $qemu_opts

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:43:04PM +0000, Alexandru Elisei wrote:
> kvmtool has a different command line parameter to specify the number of
> VCPUs (-c/--cpus). To make it easier to accommodate it, merge the qemu
> specific parameter -smp into $qemu_opts when passing it to the
> $RUNTIME_arch_run script.
> 
> This is safe to do because the $RUNTIME_arch_run script, on all
> architectures, passes the parameters right back to run_qemu() et co, and
> do not treat individual parameters separately.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  scripts/runtime.bash | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index e5d661684ceb..a89f2d10ab78 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -34,7 +34,8 @@ premature_failure()
>  get_cmdline()
>  {
>      local kernel=$1
> -    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $qemu_opts"
> +
> +    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel $qemu_opts"
>  }
>  
>  skip_nodefault()
> @@ -87,6 +88,8 @@ function run()
>      local accel="$9"
>      local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
>  
> +    qemu_opts="-smp $smp $qemu_opts"
> +
>      if [ "${CONFIG_EFI}" == "y" ]; then
>          kernel=${kernel/%.flat/.efi}
>      fi
> -- 
> 2.47.1
>

This seems fine, but I still think we want qemu_opts to be opts and we can
parameterize the option names. So we could have a table of option names
that we use in places like this

declare -A vmm_opts

vmm_opts[qemu,nr_cpus]='-smp'
vmm_opts[qemu,another_option]='foo'

vmm_opts[kvmtool,nr_cpus]='-c'
vmm_opts[kvmtool,another_option]='bar'

# $vmm gets set to whatever vmm we're using, e.g. vmm=qemu

opts="${vmm_opts[$vmm,nr_cpus]} $smp $opts"

Thanks,
drew

[kvm-unit-tests PATCH v2 07/18] scripts: Introduce kvmtool_opts

[Alexandru Elisei]

In preparation for supporting kvmtool, create and pass the variable
'kvmtool_opts' to the arch run script $RUNTIME_arch_run.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 scripts/common.bash  |  6 ++++--
 scripts/runtime.bash | 14 +++++++++++---
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/scripts/common.bash b/scripts/common.bash
index a40c28121b6a..1b5e0d667841 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -37,6 +37,7 @@ function for_each_unittest()
 	local check
 	local accel
 	local timeout
+	local kvmtool_opts
 	local rematch
 
 	exec {fd}<"$unittests"
@@ -45,7 +46,7 @@ function for_each_unittest()
 		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
 			rematch=${BASH_REMATCH[1]}
 			if [ -n "${testname}" ]; then
-				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout"
+				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts"
 			fi
 			testname=$rematch
 			smp=1
@@ -57,6 +58,7 @@ function for_each_unittest()
 			check=""
 			accel=""
 			timeout=""
+			kvmtool_opts=""
 		elif [[ $line =~ ^file\ *=\ *(.*)$ ]]; then
 			kernel=$TEST_DIR/${BASH_REMATCH[1]}
 		elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
@@ -80,7 +82,7 @@ function for_each_unittest()
 		fi
 	done
 	if [ -n "${testname}" ]; then
-		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout"
+		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts"
 	fi
 	exec {fd}<&-
 }
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index a89f2d10ab78..451b5585f010 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -35,7 +35,7 @@ get_cmdline()
 {
     local kernel=$1
 
-    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel $qemu_opts"
+    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel $opts"
 }
 
 skip_nodefault()
@@ -87,8 +87,16 @@ function run()
     local check="${CHECK:-$8}"
     local accel="$9"
     local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
-
-    qemu_opts="-smp $smp $qemu_opts"
+    local kvmtool_opts="${11}"
+
+    case "$TARGET" in
+    qemu)
+        opts="-smp $smp $qemu_opts"
+        ;;
+    kvmtool)
+        opts="--cpus $smp $kvmtool_opts"
+        ;;
+    esac
 
     if [ "${CONFIG_EFI}" == "y" ]; then
         kernel=${kernel/%.flat/.efi}
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 07/18] scripts: Introduce kvmtool_opts

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:43:05PM +0000, Alexandru Elisei wrote:
> In preparation for supporting kvmtool, create and pass the variable
> 'kvmtool_opts' to the arch run script $RUNTIME_arch_run.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  scripts/common.bash  |  6 ++++--
>  scripts/runtime.bash | 14 +++++++++++---
>  2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/common.bash b/scripts/common.bash
> index a40c28121b6a..1b5e0d667841 100644
> --- a/scripts/common.bash
> +++ b/scripts/common.bash
> @@ -37,6 +37,7 @@ function for_each_unittest()
>  	local check
>  	local accel
>  	local timeout
> +	local kvmtool_opts
>  	local rematch
>  
>  	exec {fd}<"$unittests"
> @@ -45,7 +46,7 @@ function for_each_unittest()
>  		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
>  			rematch=${BASH_REMATCH[1]}
>  			if [ -n "${testname}" ]; then
> -				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout"
> +				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts"

It looks odd to have both qemu_opts and kvmtool_opts at the same time.

>  			fi
>  			testname=$rematch
>  			smp=1
> @@ -57,6 +58,7 @@ function for_each_unittest()
>  			check=""
>  			accel=""
>  			timeout=""
> +			kvmtool_opts=""
>  		elif [[ $line =~ ^file\ *=\ *(.*)$ ]]; then
>  			kernel=$TEST_DIR/${BASH_REMATCH[1]}
>  		elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
> @@ -80,7 +82,7 @@ function for_each_unittest()
>  		fi
>  	done
>  	if [ -n "${testname}" ]; then
> -		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout"
> +		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts"
>  	fi
>  	exec {fd}<&-
>  }
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index a89f2d10ab78..451b5585f010 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -35,7 +35,7 @@ get_cmdline()
>  {
>      local kernel=$1
>  
> -    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel $qemu_opts"
> +    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel $RUNTIME_arch_run $kernel $opts"
>  }
>  
>  skip_nodefault()
> @@ -87,8 +87,16 @@ function run()
>      local check="${CHECK:-$8}"
>      local accel="$9"
>      local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
> -
> -    qemu_opts="-smp $smp $qemu_opts"
> +    local kvmtool_opts="${11}"
> +
> +    case "$TARGET" in
> +    qemu)
> +        opts="-smp $smp $qemu_opts"
> +        ;;
> +    kvmtool)
> +        opts="--cpus $smp $kvmtool_opts"
> +        ;;
> +    esac

This is similar to what I was proposing with the associative array, but
we'll only need to set a $vmm variable once with the array. If parsing
command lines is different between the vmms then we can even add
functions to the table

vmm_opts[qemu,func]=qemu_func
vmm_opts[kvmtool,func]=kvmtool_func

eval ${vmm_opts[$vmm,func]} ...

Thanks,
drew

>  
>      if [ "${CONFIG_EFI}" == "y" ]; then
>          kernel=${kernel/%.flat/.efi}
> -- 
> 2.47.1
> 
> 
> -- 
> kvm-riscv mailing list
> kvm-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kvm-riscv

[kvm-unit-tests PATCH v2 08/18] scripts/runtime: Detect kvmtool failure in premature_failure()

[Alexandru Elisei]

kvm-unit-tests assumes that if the VMM is able to get to where it tries to
load the kernel, then the VMM and the configuration parameters will also
work for running the test. All of this is done in premature_failure().

Teach premature_failure() about the kvmtool's error message when it fails
to load the dummy kernel.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 scripts/runtime.bash | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 451b5585f010..ee8a188b22ce 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -12,18 +12,27 @@ extract_summary()
     tail -5 | grep '^SUMMARY: ' | sed 's/^SUMMARY: /(/;s/'"$cr"'\{0,1\}$/)/'
 }
 
-# We assume that QEMU is going to work if it tried to load the kernel
+# We assume that the VMM is going to work if it tried to load the kernel
 premature_failure()
 {
     local log
 
     log="$(eval "$(get_cmdline _NO_FILE_4Uhere_)" 2>&1)"
 
-    echo "$log" | grep "_NO_FILE_4Uhere_" |
-        grep -q -e "[Cc]ould not \(load\|open\) kernel" \
-                -e "error loading" \
-                -e "failed to load" &&
-        return 1
+    case "$TARGET" in
+    qemu)
+
+        echo "$log" | grep "_NO_FILE_4Uhere_" |
+            grep -q -e "[Cc]ould not \(load\|open\) kernel" \
+                    -e "error loading" \
+                    -e "failed to load" &&
+            return 1
+        ;;
+    kvmtool)
+        echo "$log" | grep "Fatal: Unable to open kernel _NO_FILE_4Uhere_" &&
+            return 1
+        ;;
+    esac
 
     RUNTIME_log_stderr <<< "$log"
 
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 08/18] scripts/runtime: Detect kvmtool failure in premature_failure()

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:43:06PM +0000, Alexandru Elisei wrote:
> kvm-unit-tests assumes that if the VMM is able to get to where it tries to
> load the kernel, then the VMM and the configuration parameters will also
> work for running the test. All of this is done in premature_failure().
> 
> Teach premature_failure() about the kvmtool's error message when it fails
> to load the dummy kernel.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  scripts/runtime.bash | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 451b5585f010..ee8a188b22ce 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -12,18 +12,27 @@ extract_summary()
>      tail -5 | grep '^SUMMARY: ' | sed 's/^SUMMARY: /(/;s/'"$cr"'\{0,1\}$/)/'
>  }
>  
> -# We assume that QEMU is going to work if it tried to load the kernel
> +# We assume that the VMM is going to work if it tried to load the kernel
>  premature_failure()
>  {
>      local log
>  
>      log="$(eval "$(get_cmdline _NO_FILE_4Uhere_)" 2>&1)"
>  
> -    echo "$log" | grep "_NO_FILE_4Uhere_" |
> -        grep -q -e "[Cc]ould not \(load\|open\) kernel" \
> -                -e "error loading" \
> -                -e "failed to load" &&
> -        return 1
> +    case "$TARGET" in
> +    qemu)
> +

extra blank line here

> +        echo "$log" | grep "_NO_FILE_4Uhere_" |
> +            grep -q -e "[Cc]ould not \(load\|open\) kernel" \
> +                    -e "error loading" \
> +                    -e "failed to load" &&
> +            return 1
> +        ;;
> +    kvmtool)
> +        echo "$log" | grep "Fatal: Unable to open kernel _NO_FILE_4Uhere_" &&
> +            return 1
> +        ;;
> +    esac

This looks good, but could possibly become

 eval echo "$log" | ${vmm_opts[$TARGET,premature_failure]} && return 1

if we got the vmm_opts route.

Thanks,
drew

>  
>      RUNTIME_log_stderr <<< "$log"
>  
> -- 
> 2.47.1
> 
> 
> -- 
> kvm-riscv mailing list
> kvm-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kvm-riscv

[kvm-unit-tests PATCH v2 09/18] scripts/runtime: Skip test when kvmtool and $accel is not KVM

[Alexandru Elisei]

kvmtool, unlike qemu, cannot emulate a different architecture than the
host's, and as a result the only $accel parameter it can support is 'kvm'.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 scripts/runtime.bash | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index ee8a188b22ce..55d58eef9c7c 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -153,6 +153,11 @@ function run()
         accel="$ACCEL"
     fi
 
+    if [[ "$TARGET" = kvmtool ]] && [[ -n "$accel" ]] && [[ "$accel" != "kvm" ]]; then
+        print_result "SKIP" $testname "" "kvmtool does not support $accel"
+        return 2
+    fi
+
     # check a file for a particular value before running a test
     # the check line can contain multiple files to check separated by a space
     # but each check parameter needs to be of the form <path>=<value>
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 09/18] scripts/runtime: Skip test when kvmtool and $accel is not KVM

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:43:07PM +0000, Alexandru Elisei wrote:
> kvmtool, unlike qemu, cannot emulate a different architecture than the
> host's, and as a result the only $accel parameter it can support is 'kvm'.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  scripts/runtime.bash | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index ee8a188b22ce..55d58eef9c7c 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -153,6 +153,11 @@ function run()
>          accel="$ACCEL"
>      fi
>  
> +    if [[ "$TARGET" = kvmtool ]] && [[ -n "$accel" ]] && [[ "$accel" != "kvm" ]]; then
> +        print_result "SKIP" $testname "" "kvmtool does not support $accel"
> +        return 2
> +    fi
> +
>      # check a file for a particular value before running a test
>      # the check line can contain multiple files to check separated by a space
>      # but each check parameter needs to be of the form <path>=<value>
> -- 
> 2.47.1

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

[kvm-unit-tests PATCH v2 10/18] scripts/arch-run: Add support for kvmtool

[Alexandru Elisei]

Add two new functions, search_kvmtool_binary(), which, like the name
suggests, searches for the location of the kvmtool binary, and
run_kvmtool(), which runs a test with kvmtool as the VMM.

initrd_create() has also been modified to use the kvmtool syntax for
supplying an initrd, which is --initrd (two dashes instead of the single
dash that qemu uses).

arm/run does not know how to use these functions yet, but this will be
added in a subsequent patch.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 scripts/arch-run.bash | 94 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 81 insertions(+), 13 deletions(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index d6eaf0ee5f09..34f633cade01 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -75,16 +75,47 @@ run_qemu ()
 	return $ret
 }
 
+run_kvmtool ()
+{
+	local stdout errors ret sig
+
+	initrd_create || return $?
+
+	echo -n "$@"
+	[ "$ENVIRON_DEFAULT" = "yes" ] && echo -n " #"
+	echo " $INITRD"
+
+	# stdout to {stdout}, stderr to $errors and stderr
+	exec {stdout}>&1
+	"${@}" $INITRD </dev/null 2> >(tee /dev/stderr) > /dev/fd/$stdout
+	ret=$?
+	exec {stdout}>&-
+
+	return $ret
+}
+
 run_test_status ()
 {
-	local stdout ret
+	local stdout ret ret_success
 
 	exec {stdout}>&1
-	lines=$(run_qemu "$@" > >(tee /dev/fd/$stdout))
+
+	# For qemu, an exit status of 1 means that the test completed. For kvmtool,
+	# 0 means the same thing.
+	case "$TARGET" in
+	qemu)
+		ret_success=1
+		lines=$(run_qemu "$@" > >(tee /dev/fd/$stdout))
+		;;
+	kvmtool)
+		ret_success=0
+		lines=$(run_kvmtool "$@" > >(tee /dev/fd/$stdout))
+		;;
+	esac
 	ret=$?
 	exec {stdout}>&-
 
-	if [ $ret -eq 1 ]; then
+	if [ $ret -eq $ret_success ]; then
 		testret=$(grep '^EXIT: ' <<<"$lines" | head -n1 | sed 's/.*STATUS=\([0-9][0-9]*\).*/\1/')
 		if [ "$testret" ]; then
 			if [ $testret -eq 1 ]; then
@@ -422,6 +453,25 @@ search_qemu_binary ()
 	export PATH=$save_path
 }
 
+search_kvmtool_binary ()
+{
+	local kvmtoolcmd kvmtool
+
+	for kvmtoolcmd in lkvm vm lkvm-static; do
+		if $kvmtoolcmd --help 2>/dev/null| grep -q 'The most commonly used'; then
+			kvmtool="$kvmtoolcmd"
+			break
+		fi
+	done
+
+	if [ -z "$kvmtool" ]; then
+		echo "A kvmtool binary was not found." >&2
+		return 2
+	fi
+
+	command -v $kvmtool
+}
+
 initrd_cleanup ()
 {
 	rm -f $KVM_UNIT_TESTS_ENV
@@ -447,7 +497,18 @@ initrd_create ()
 	fi
 
 	unset INITRD
-	[ -f "$KVM_UNIT_TESTS_ENV" ] && INITRD="-initrd $KVM_UNIT_TESTS_ENV"
+	if [ ! -f "$KVM_UNIT_TESTS_ENV" ]; then
+		return 0
+	fi
+
+	case "$TARGET" in
+		qemu)
+			INITRD="-initrd $KVM_UNIT_TESTS_ENV"
+			;;
+		kvmtool)
+			INITRD="--initrd $KVM_UNIT_TESTS_ENV"
+			;;
+	esac
 
 	return 0
 }
@@ -471,18 +532,25 @@ env_params ()
 	local qemu have_qemu
 	local _ rest
 
-	qemu=$(search_qemu_binary) && have_qemu=1
+	env_add_params TARGET
+
+	# kvmtool's versioning has been broken since it was split from the kernel
+	# source.
+	if [ "$TARGET" = "qemu" ]; then
+		qemu=$(search_qemu_binary) && have_qemu=1
 
-	if [ "$have_qemu" ]; then
-		if [ -n "$ACCEL" ] || [ -n "$QEMU_ACCEL" ]; then
-			[ -n "$ACCEL" ] && QEMU_ACCEL=$ACCEL
+		if [ "$have_qemu" ]; then
+			if [ -n "$ACCEL" ] || [ -n "$QEMU_ACCEL" ]; then
+				[ -n "$ACCEL" ] && QEMU_ACCEL=$ACCEL
+			fi
+			QEMU_VERSION_STRING="$($qemu -h | head -1)"
+			# Shellcheck does not see QEMU_MAJOR|MINOR|MICRO are used
+			# shellcheck disable=SC2034
+			IFS='[ .]' read -r _ _ _ QEMU_MAJOR QEMU_MINOR QEMU_MICRO rest <<<"$QEMU_VERSION_STRING"
 		fi
-		QEMU_VERSION_STRING="$($qemu -h | head -1)"
-		# Shellcheck does not see QEMU_MAJOR|MINOR|MICRO are used
-		# shellcheck disable=SC2034
-		IFS='[ .]' read -r _ _ _ QEMU_MAJOR QEMU_MINOR QEMU_MICRO rest <<<"$QEMU_VERSION_STRING"
+
+		env_add_params QEMU_ACCEL QEMU_VERSION_STRING QEMU_MAJOR QEMU_MINOR QEMU_MICRO
 	fi
-	env_add_params QEMU_ACCEL QEMU_VERSION_STRING QEMU_MAJOR QEMU_MINOR QEMU_MICRO
 
 	KERNEL_VERSION_STRING=$(uname -r)
 	IFS=. read -r KERNEL_VERSION KERNEL_PATCHLEVEL rest <<<"$KERNEL_VERSION_STRING"
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 10/18] scripts/arch-run: Add support for kvmtool

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:43:08PM +0000, Alexandru Elisei wrote:
> Add two new functions, search_kvmtool_binary(), which, like the name
> suggests, searches for the location of the kvmtool binary, and
> run_kvmtool(), which runs a test with kvmtool as the VMM.
> 
> initrd_create() has also been modified to use the kvmtool syntax for
> supplying an initrd, which is --initrd (two dashes instead of the single
> dash that qemu uses).
> 
> arm/run does not know how to use these functions yet, but this will be
> added in a subsequent patch.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  scripts/arch-run.bash | 94 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 81 insertions(+), 13 deletions(-)
> 
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index d6eaf0ee5f09..34f633cade01 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -75,16 +75,47 @@ run_qemu ()
>  	return $ret
>  }
>  
> +run_kvmtool ()
> +{
> +	local stdout errors ret sig
> +
> +	initrd_create || return $?
> +
> +	echo -n "$@"
> +	[ "$ENVIRON_DEFAULT" = "yes" ] && echo -n " #"
> +	echo " $INITRD"
> +
> +	# stdout to {stdout}, stderr to $errors and stderr
> +	exec {stdout}>&1
> +	"${@}" $INITRD </dev/null 2> >(tee /dev/stderr) > /dev/fd/$stdout
> +	ret=$?
> +	exec {stdout}>&-
> +
> +	return $ret
> +}

It seems like run_qemu should mostly apply to kvmtool since kvmtool could
also terminate on a signal, like the timeout SIGALRM. Maybe we should try
to get them to use the same thing and name it run_test()? Or at least we
can factor out the common parts to avoid duplication.

> +
>  run_test_status ()
>  {
> -	local stdout ret
> +	local stdout ret ret_success
>  
>  	exec {stdout}>&1
> -	lines=$(run_qemu "$@" > >(tee /dev/fd/$stdout))
> +
> +	# For qemu, an exit status of 1 means that the test completed. For kvmtool,
> +	# 0 means the same thing.
> +	case "$TARGET" in
> +	qemu)
> +		ret_success=1
> +		lines=$(run_qemu "$@" > >(tee /dev/fd/$stdout))
> +		;;
> +	kvmtool)
> +		ret_success=0
> +		lines=$(run_kvmtool "$@" > >(tee /dev/fd/$stdout))
> +		;;
> +	esac
>  	ret=$?
>  	exec {stdout}>&-
>  
> -	if [ $ret -eq 1 ]; then
> +	if [ $ret -eq $ret_success ]; then
>  		testret=$(grep '^EXIT: ' <<<"$lines" | head -n1 | sed 's/.*STATUS=\([0-9][0-9]*\).*/\1/')
>  		if [ "$testret" ]; then
>  			if [ $testret -eq 1 ]; then
> @@ -422,6 +453,25 @@ search_qemu_binary ()
>  	export PATH=$save_path
>  }
>  
> +search_kvmtool_binary ()
> +{
> +	local kvmtoolcmd kvmtool
> +
> +	for kvmtoolcmd in lkvm vm lkvm-static; do
> +		if $kvmtoolcmd --help 2>/dev/null| grep -q 'The most commonly used'; then
> +			kvmtool="$kvmtoolcmd"
> +			break
> +		fi
> +	done
> +
> +	if [ -z "$kvmtool" ]; then
> +		echo "A kvmtool binary was not found." >&2

Do we want to support a KVMTOOL environment variable analogous to $QEMU?
If so we can also add the help text that search_qemu_binary() has here.

> +		return 2
> +	fi
> +
> +	command -v $kvmtool
> +}
> +
>  initrd_cleanup ()
>  {
>  	rm -f $KVM_UNIT_TESTS_ENV
> @@ -447,7 +497,18 @@ initrd_create ()
>  	fi
>  
>  	unset INITRD
> -	[ -f "$KVM_UNIT_TESTS_ENV" ] && INITRD="-initrd $KVM_UNIT_TESTS_ENV"
> +	if [ ! -f "$KVM_UNIT_TESTS_ENV" ]; then
> +		return 0
> +	fi
> +
> +	case "$TARGET" in
> +		qemu)
> +			INITRD="-initrd $KVM_UNIT_TESTS_ENV"
> +			;;
> +		kvmtool)
> +			INITRD="--initrd $KVM_UNIT_TESTS_ENV"
> +			;;
> +	esac

vmm_opts[qemu,initrd]='-initrd'
vmm_opts[kvmtool,initrd]='--initrd'

>  
>  	return 0
>  }
> @@ -471,18 +532,25 @@ env_params ()
>  	local qemu have_qemu
>  	local _ rest
>  
> -	qemu=$(search_qemu_binary) && have_qemu=1
> +	env_add_params TARGET
> +
> +	# kvmtool's versioning has been broken since it was split from the kernel
> +	# source.
> +	if [ "$TARGET" = "qemu" ]; then
> +		qemu=$(search_qemu_binary) && have_qemu=1
>  
> -	if [ "$have_qemu" ]; then
> -		if [ -n "$ACCEL" ] || [ -n "$QEMU_ACCEL" ]; then
> -			[ -n "$ACCEL" ] && QEMU_ACCEL=$ACCEL
> +		if [ "$have_qemu" ]; then
> +			if [ -n "$ACCEL" ] || [ -n "$QEMU_ACCEL" ]; then
> +				[ -n "$ACCEL" ] && QEMU_ACCEL=$ACCEL
> +			fi
> +			QEMU_VERSION_STRING="$($qemu -h | head -1)"
> +			# Shellcheck does not see QEMU_MAJOR|MINOR|MICRO are used
> +			# shellcheck disable=SC2034
> +			IFS='[ .]' read -r _ _ _ QEMU_MAJOR QEMU_MINOR QEMU_MICRO rest <<<"$QEMU_VERSION_STRING"
>  		fi
> -		QEMU_VERSION_STRING="$($qemu -h | head -1)"
> -		# Shellcheck does not see QEMU_MAJOR|MINOR|MICRO are used
> -		# shellcheck disable=SC2034
> -		IFS='[ .]' read -r _ _ _ QEMU_MAJOR QEMU_MINOR QEMU_MICRO rest <<<"$QEMU_VERSION_STRING"
> +
> +		env_add_params QEMU_ACCEL QEMU_VERSION_STRING QEMU_MAJOR QEMU_MINOR QEMU_MICRO
>  	fi
> -	env_add_params QEMU_ACCEL QEMU_VERSION_STRING QEMU_MAJOR QEMU_MINOR QEMU_MICRO
>  
>  	KERNEL_VERSION_STRING=$(uname -r)
>  	IFS=. read -r KERNEL_VERSION KERNEL_PATCHLEVEL rest <<<"$KERNEL_VERSION_STRING"
> -- 
> 2.47.1
>

Thanks,
drew

[kvm-unit-tests PATCH v2 11/18] arm/run: Add support for kvmtool

[Alexandru Elisei]

Teach the arm runner to use kvmtool when kvm-unit-tests has been configured
appropriately.

The test is ran using run_test_status() because kvmtool does not have a
testdev device to return the test exit code, so kvm-unit-tests must always
parse the "EXIT: STATUS" line for the exit code.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arm/run | 183 ++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 110 insertions(+), 73 deletions(-)

diff --git a/arm/run b/arm/run
index 9b11feafffdd..880d5afae86d 100755
--- a/arm/run
+++ b/arm/run
@@ -17,77 +17,114 @@ qemu)
    exit 3
 esac
 
-processor="$PROCESSOR"
+arch_run_qemu()
+{
+    processor="$PROCESSOR"
+
+    if [ "$QEMU" ] && [ -z "$ACCEL" ] &&
+       [ "$HOST" = "aarch64" ] && [ "$ARCH" = "arm" ] &&
+       [ "$(basename $QEMU)" = "qemu-system-arm" ]; then
+        ACCEL="tcg"
+    fi
+
+    set_qemu_accelerator || exit $?
+    if [ "$ACCEL" = "kvm" ]; then
+        QEMU_ARCH=$HOST
+    fi
+
+    qemu=$(search_qemu_binary) ||
+        exit $?
+
+    if ! $qemu -machine '?' | grep -q 'ARM Virtual Machine'; then
+        echo "$qemu doesn't support mach-virt ('-machine virt'). Exiting."
+        exit 2
+    fi
+
+    M='-machine virt'
+
+    if [ "$ACCEL" = "kvm" ]; then
+        if $qemu $M,\? | grep -q gic-version; then
+            M+=',gic-version=host'
+        fi
+    fi
+
+    if [ "$ACCEL" = "kvm" ] || [ "$ACCEL" = "hvf" ]; then
+        if [ "$HOST" = "aarch64" ] || [ "$HOST" = "arm" ]; then
+            processor="host"
+            if [ "$ARCH" = "arm" ] && [ "$HOST" = "aarch64" ]; then
+                processor+=",aarch64=off"
+            fi
+        fi
+    fi
+
+    if [ "$ARCH" = "arm" ]; then
+        M+=",highmem=off"
+    fi
+
+    if ! $qemu $M -device '?' | grep -q virtconsole; then
+        echo "$qemu doesn't support virtio-console for chr-testdev. Exiting."
+        exit 2
+    fi
+
+    if ! $qemu $M -chardev '?' | grep -q testdev; then
+        echo "$qemu doesn't support chr-testdev. Exiting."
+        exit 2
+    fi
+
+    if [ "$UEFI_SHELL_RUN" != "y" ] && [ "$EFI_USE_ACPI" != "y" ]; then
+        chr_testdev='-device virtio-serial-device'
+        chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd'
+    fi
+
+    pci_testdev=
+    if $qemu $M -device '?' | grep -q pci-testdev; then
+        pci_testdev="-device pci-testdev"
+    fi
+
+    A="-accel $ACCEL$ACCEL_PROPS"
+    command="$qemu -nodefaults $M $A -cpu $processor $chr_testdev $pci_testdev"
+    command+=" -display none -serial stdio"
+    command="$(migration_cmd) $(timeout_cmd) $command"
+
+    if [ "$UEFI_SHELL_RUN" = "y" ]; then
+        ENVIRON_DEFAULT=n run_test_status $command "$@"
+    elif [ "$EFI_USE_ACPI" = "y" ]; then
+        run_test_status $command -kernel "$@"
+    else
+        run_qemu $command -kernel "$@"
+    fi
+}
+
+arch_run_kvmtool()
+{
+    local command
+
+    kvmtool=$(search_kvmtool_binary) ||
+        exit $?
+
+    if [ "$ACCEL" ] && [ "$ACCEL" != "kvm" ]; then
+        echo "kvmtool does not support $ACCEL" >&2
+        exit 2
+    fi
+
+    if ! kvm_available; then
+        echo "KVM required by kvmtool but not available on the host" >&2
+        exit 2
+    fi
+
+    command="$(timeout_cmd) $kvmtool run"
+    if [ "$HOST" = "aarch64" ] && [ "$ARCH" = "arm" ]; then
+	    run_test_status $command --kernel "$@" --aarch32
+    else
+	    run_test_status $command --kernel "$@"
+    fi
+}
 
-if [ "$QEMU" ] && [ -z "$ACCEL" ] &&
-   [ "$HOST" = "aarch64" ] && [ "$ARCH" = "arm" ] &&
-   [ "$(basename $QEMU)" = "qemu-system-arm" ]; then
-	ACCEL="tcg"
-fi
-
-set_qemu_accelerator || exit $?
-if [ "$ACCEL" = "kvm" ]; then
-	QEMU_ARCH=$HOST
-fi
-
-qemu=$(search_qemu_binary) ||
-	exit $?
-
-if ! $qemu -machine '?' | grep -q 'ARM Virtual Machine'; then
-	echo "$qemu doesn't support mach-virt ('-machine virt'). Exiting."
-	exit 2
-fi
-
-M='-machine virt'
-
-if [ "$ACCEL" = "kvm" ]; then
-	if $qemu $M,\? | grep -q gic-version; then
-		M+=',gic-version=host'
-	fi
-fi
-
-if [ "$ACCEL" = "kvm" ] || [ "$ACCEL" = "hvf" ]; then
-	if [ "$HOST" = "aarch64" ] || [ "$HOST" = "arm" ]; then
-		processor="host"
-		if [ "$ARCH" = "arm" ] && [ "$HOST" = "aarch64" ]; then
-			processor+=",aarch64=off"
-		fi
-	fi
-fi
-
-if [ "$ARCH" = "arm" ]; then
-	M+=",highmem=off"
-fi
-
-if ! $qemu $M -device '?' | grep -q virtconsole; then
-	echo "$qemu doesn't support virtio-console for chr-testdev. Exiting."
-	exit 2
-fi
-
-if ! $qemu $M -chardev '?' | grep -q testdev; then
-	echo "$qemu doesn't support chr-testdev. Exiting."
-	exit 2
-fi
-
-if [ "$UEFI_SHELL_RUN" != "y" ] && [ "$EFI_USE_ACPI" != "y" ]; then
-	chr_testdev='-device virtio-serial-device'
-	chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd'
-fi
-
-pci_testdev=
-if $qemu $M -device '?' | grep -q pci-testdev; then
-	pci_testdev="-device pci-testdev"
-fi
-
-A="-accel $ACCEL$ACCEL_PROPS"
-command="$qemu -nodefaults $M $A -cpu $processor $chr_testdev $pci_testdev"
-command+=" -display none -serial stdio"
-command="$(migration_cmd) $(timeout_cmd) $command"
-
-if [ "$UEFI_SHELL_RUN" = "y" ]; then
-	ENVIRON_DEFAULT=n run_test_status $command "$@"
-elif [ "$EFI_USE_ACPI" = "y" ]; then
-	run_test_status $command -kernel "$@"
-else
-	run_qemu $command -kernel "$@"
-fi
+case "$TARGET" in
+qemu)
+    arch_run_qemu "$@"
+    ;;
+kvmtool)
+    arch_run_kvmtool "$@"
+    ;;
+esac
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 11/18] arm/run: Add support for kvmtool

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:43:09PM +0000, Alexandru Elisei wrote:
> Teach the arm runner to use kvmtool when kvm-unit-tests has been configured
> appropriately.
> 
> The test is ran using run_test_status() because kvmtool does not have a
> testdev device to return the test exit code, so kvm-unit-tests must always
> parse the "EXIT: STATUS" line for the exit code.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  arm/run | 183 ++++++++++++++++++++++++++++++++++----------------------
>  1 file changed, 110 insertions(+), 73 deletions(-)
> 
> diff --git a/arm/run b/arm/run
> index 9b11feafffdd..880d5afae86d 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -17,77 +17,114 @@ qemu)
>     exit 3
>  esac
>  
> -processor="$PROCESSOR"
> +arch_run_qemu()
> +{
> +    processor="$PROCESSOR"
> +
> +    if [ "$QEMU" ] && [ -z "$ACCEL" ] &&
> +       [ "$HOST" = "aarch64" ] && [ "$ARCH" = "arm" ] &&
> +       [ "$(basename $QEMU)" = "qemu-system-arm" ]; then
> +        ACCEL="tcg"
> +    fi
> +
> +    set_qemu_accelerator || exit $?
> +    if [ "$ACCEL" = "kvm" ]; then
> +        QEMU_ARCH=$HOST
> +    fi
> +
> +    qemu=$(search_qemu_binary) ||
> +        exit $?
> +
> +    if ! $qemu -machine '?' | grep -q 'ARM Virtual Machine'; then
> +        echo "$qemu doesn't support mach-virt ('-machine virt'). Exiting."
> +        exit 2
> +    fi
> +
> +    M='-machine virt'
> +
> +    if [ "$ACCEL" = "kvm" ]; then
> +        if $qemu $M,\? | grep -q gic-version; then
> +            M+=',gic-version=host'
> +        fi
> +    fi
> +
> +    if [ "$ACCEL" = "kvm" ] || [ "$ACCEL" = "hvf" ]; then
> +        if [ "$HOST" = "aarch64" ] || [ "$HOST" = "arm" ]; then
> +            processor="host"
> +            if [ "$ARCH" = "arm" ] && [ "$HOST" = "aarch64" ]; then
> +                processor+=",aarch64=off"
> +            fi
> +        fi
> +    fi
> +
> +    if [ "$ARCH" = "arm" ]; then
> +        M+=",highmem=off"
> +    fi
> +
> +    if ! $qemu $M -device '?' | grep -q virtconsole; then
> +        echo "$qemu doesn't support virtio-console for chr-testdev. Exiting."
> +        exit 2
> +    fi
> +
> +    if ! $qemu $M -chardev '?' | grep -q testdev; then
> +        echo "$qemu doesn't support chr-testdev. Exiting."
> +        exit 2
> +    fi
> +
> +    if [ "$UEFI_SHELL_RUN" != "y" ] && [ "$EFI_USE_ACPI" != "y" ]; then
> +        chr_testdev='-device virtio-serial-device'
> +        chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd'
> +    fi
> +
> +    pci_testdev=
> +    if $qemu $M -device '?' | grep -q pci-testdev; then
> +        pci_testdev="-device pci-testdev"
> +    fi
> +
> +    A="-accel $ACCEL$ACCEL_PROPS"
> +    command="$qemu -nodefaults $M $A -cpu $processor $chr_testdev $pci_testdev"
> +    command+=" -display none -serial stdio"
> +    command="$(migration_cmd) $(timeout_cmd) $command"
> +
> +    if [ "$UEFI_SHELL_RUN" = "y" ]; then
> +        ENVIRON_DEFAULT=n run_test_status $command "$@"
> +    elif [ "$EFI_USE_ACPI" = "y" ]; then
> +        run_test_status $command -kernel "$@"
> +    else
> +        run_qemu $command -kernel "$@"
> +    fi
> +}
> +
> +arch_run_kvmtool()
> +{
> +    local command
> +
> +    kvmtool=$(search_kvmtool_binary) ||
> +        exit $?
> +
> +    if [ "$ACCEL" ] && [ "$ACCEL" != "kvm" ]; then
> +        echo "kvmtool does not support $ACCEL" >&2
> +        exit 2
> +    fi
> +
> +    if ! kvm_available; then
> +        echo "KVM required by kvmtool but not available on the host" >&2
> +        exit 2
> +    fi
> +
> +    command="$(timeout_cmd) $kvmtool run"
> +    if [ "$HOST" = "aarch64" ] && [ "$ARCH" = "arm" ]; then
> +	    run_test_status $command --kernel "$@" --aarch32
> +    else
> +	    run_test_status $command --kernel "$@"
> +    fi
> +}
>  
> -if [ "$QEMU" ] && [ -z "$ACCEL" ] &&
> -   [ "$HOST" = "aarch64" ] && [ "$ARCH" = "arm" ] &&
> -   [ "$(basename $QEMU)" = "qemu-system-arm" ]; then
> -	ACCEL="tcg"
> -fi
> -
> -set_qemu_accelerator || exit $?
> -if [ "$ACCEL" = "kvm" ]; then
> -	QEMU_ARCH=$HOST
> -fi
> -
> -qemu=$(search_qemu_binary) ||
> -	exit $?
> -
> -if ! $qemu -machine '?' | grep -q 'ARM Virtual Machine'; then
> -	echo "$qemu doesn't support mach-virt ('-machine virt'). Exiting."
> -	exit 2
> -fi
> -
> -M='-machine virt'
> -
> -if [ "$ACCEL" = "kvm" ]; then
> -	if $qemu $M,\? | grep -q gic-version; then
> -		M+=',gic-version=host'
> -	fi
> -fi
> -
> -if [ "$ACCEL" = "kvm" ] || [ "$ACCEL" = "hvf" ]; then
> -	if [ "$HOST" = "aarch64" ] || [ "$HOST" = "arm" ]; then
> -		processor="host"
> -		if [ "$ARCH" = "arm" ] && [ "$HOST" = "aarch64" ]; then
> -			processor+=",aarch64=off"
> -		fi
> -	fi
> -fi
> -
> -if [ "$ARCH" = "arm" ]; then
> -	M+=",highmem=off"
> -fi
> -
> -if ! $qemu $M -device '?' | grep -q virtconsole; then
> -	echo "$qemu doesn't support virtio-console for chr-testdev. Exiting."
> -	exit 2
> -fi
> -
> -if ! $qemu $M -chardev '?' | grep -q testdev; then
> -	echo "$qemu doesn't support chr-testdev. Exiting."
> -	exit 2
> -fi
> -
> -if [ "$UEFI_SHELL_RUN" != "y" ] && [ "$EFI_USE_ACPI" != "y" ]; then
> -	chr_testdev='-device virtio-serial-device'
> -	chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd'
> -fi
> -
> -pci_testdev=
> -if $qemu $M -device '?' | grep -q pci-testdev; then
> -	pci_testdev="-device pci-testdev"
> -fi
> -
> -A="-accel $ACCEL$ACCEL_PROPS"
> -command="$qemu -nodefaults $M $A -cpu $processor $chr_testdev $pci_testdev"
> -command+=" -display none -serial stdio"
> -command="$(migration_cmd) $(timeout_cmd) $command"
> -
> -if [ "$UEFI_SHELL_RUN" = "y" ]; then
> -	ENVIRON_DEFAULT=n run_test_status $command "$@"
> -elif [ "$EFI_USE_ACPI" = "y" ]; then
> -	run_test_status $command -kernel "$@"
> -else
> -	run_qemu $command -kernel "$@"
> -fi
> +case "$TARGET" in
> +qemu)
> +    arch_run_qemu "$@"
> +    ;;
> +kvmtool)
> +    arch_run_kvmtool "$@"
> +    ;;
> +esac
> -- 
> 2.47.1
> 
>

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

[kvm-unit-tests PATCH v2 12/18] scripts/runtime: Add default arguments for kvmtool

[Alexandru Elisei]

kvmtool, unless told otherwise, will do its best to make sure that a kernel
successfully boots in a virtual machine. Among things like automatically
creating a rootfs, it also adds extra parameters to the kernel command
line. This is actively harmful to kvm-unit-tests, because some tests parse
the kernel command line and they will fail if they encounter the options
added by kvmtool.

Fortunately for us, kvmtool commit 5613ae26b998 ("Add --nodefaults command
line argument") addded the --nodefaults kvmtool parameter which disables
all the implicit virtual machine configuration that cannot be disabled by
using other parameters, like modifying the kernel command line. Always use
--nodefaults to allow a test to run.

kvmtool can be too verbose when running a virtual machine, and this is
controlled with parameters. Add those to the default kvmtool command line
to reduce this verbosity to a minimum.

Before:

$ vm run arm/selftest.flat --cpus 2 --mem 256 --params "setup smp=2 mem=256"
  Info: # lkvm run -k arm/selftest.flat -m 256 -c 2 --name guest-5035
Unknown subtest

EXIT: STATUS=127
  Warning: KVM compatibility warning.
	virtio-9p device was not detected.
	While you have requested a virtio-9p device, the guest kernel did not initialize it.
	Please make sure that the guest kernel was compiled with CONFIG_NET_9P_VIRTIO=y enabled in .config.
  Warning: KVM compatibility warning.
	virtio-net device was not detected.
	While you have requested a virtio-net device, the guest kernel did not initialize it.
	Please make sure that the guest kernel was compiled with CONFIG_VIRTIO_NET=y enabled in .config.
  Info: KVM session ended normally.

After:

$ vm run arm/selftest.flat --nodefaults --network mode=none --loglevel=warning --cpus 2 --mem 256 --params "setup smp=2 mem=256"
PASS: selftest: setup: smp: number of CPUs matches expectation
INFO: selftest: setup: smp: found 2 CPUs
PASS: selftest: setup: mem: memory size matches expectation
INFO: selftest: setup: mem: found 256 MB
SUMMARY: 2 tests

EXIT: STATUS=1

Note that KVMTOOL_DEFAULT_OPTS can be overwritten by an environment
variable with the same name, but it's not documented in the help string for
run_tests.sh. This has been done on purpose, since overwritting
KVMTOOL_DEFAULT_OPTS should only be necessary for debugging or development
purposes.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 scripts/runtime.bash | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 55d58eef9c7c..abfd1e67b2ef 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -2,6 +2,17 @@
 : "${MAX_SMP:=$(getconf _NPROCESSORS_ONLN)}"
 : "${TIMEOUT:=90s}"
 
+# The following parameters are enabled by default when running a test with
+# kvmtool:
+# --nodefaults: suppress VM configuration that cannot be disabled otherwise
+#               (like modifying the supplied kernel command line). Tests that
+#               use the command line will fail without this parameter.
+# --network mode=none: do not create a network device. kvmtool tries to help the
+#                user by automatically create one, and then prints a warning
+#                when the VM terminates if the device hasn't been initialized.
+# --loglevel=warning: reduce verbosity
+: "${KVMTOOL_DEFAULT_OPTS:="--nodefaults --network mode=none --loglevel=warning"}"
+
 PASS() { echo -ne "\e[32mPASS\e[0m"; }
 SKIP() { echo -ne "\e[33mSKIP\e[0m"; }
 FAIL() { echo -ne "\e[31mFAIL\e[0m"; }
@@ -103,7 +114,7 @@ function run()
         opts="-smp $smp $qemu_opts"
         ;;
     kvmtool)
-        opts="--cpus $smp $kvmtool_opts"
+        opts="$KVMTOOL_DEFAULT_OPTS --cpus $smp $kvmtool_opts"
         ;;
     esac
 
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 12/18] scripts/runtime: Add default arguments for kvmtool

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:43:10PM +0000, Alexandru Elisei wrote:
> kvmtool, unless told otherwise, will do its best to make sure that a kernel
> successfully boots in a virtual machine. Among things like automatically
> creating a rootfs, it also adds extra parameters to the kernel command
> line. This is actively harmful to kvm-unit-tests, because some tests parse
> the kernel command line and they will fail if they encounter the options
> added by kvmtool.
> 
> Fortunately for us, kvmtool commit 5613ae26b998 ("Add --nodefaults command
> line argument") addded the --nodefaults kvmtool parameter which disables

added

> all the implicit virtual machine configuration that cannot be disabled by
> using other parameters, like modifying the kernel command line. Always use
> --nodefaults to allow a test to run.
> 
> kvmtool can be too verbose when running a virtual machine, and this is
> controlled with parameters. Add those to the default kvmtool command line
> to reduce this verbosity to a minimum.
> 
> Before:
> 
> $ vm run arm/selftest.flat --cpus 2 --mem 256 --params "setup smp=2 mem=256"
>   Info: # lkvm run -k arm/selftest.flat -m 256 -c 2 --name guest-5035
> Unknown subtest
> 
> EXIT: STATUS=127
>   Warning: KVM compatibility warning.
> 	virtio-9p device was not detected.
> 	While you have requested a virtio-9p device, the guest kernel did not initialize it.
> 	Please make sure that the guest kernel was compiled with CONFIG_NET_9P_VIRTIO=y enabled in .config.
>   Warning: KVM compatibility warning.
> 	virtio-net device was not detected.
> 	While you have requested a virtio-net device, the guest kernel did not initialize it.
> 	Please make sure that the guest kernel was compiled with CONFIG_VIRTIO_NET=y enabled in .config.
>   Info: KVM session ended normally.
> 
> After:
> 
> $ vm run arm/selftest.flat --nodefaults --network mode=none --loglevel=warning --cpus 2 --mem 256 --params "setup smp=2 mem=256"

On riscv I've noticed that with --nodefaults if I don't add parameters
with --params then kvmtool segfaults. I have to add --params "" to
avoid it. Does that also happen on arm? Anyway, that's something we
should fix in kvmtool rather than workaround it here.

> PASS: selftest: setup: smp: number of CPUs matches expectation
> INFO: selftest: setup: smp: found 2 CPUs
> PASS: selftest: setup: mem: memory size matches expectation
> INFO: selftest: setup: mem: found 256 MB
> SUMMARY: 2 tests
> 
> EXIT: STATUS=1
> 
> Note that KVMTOOL_DEFAULT_OPTS can be overwritten by an environment
> variable with the same name, but it's not documented in the help string for
> run_tests.sh. This has been done on purpose, since overwritting
> KVMTOOL_DEFAULT_OPTS should only be necessary for debugging or development
> purposes.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  scripts/runtime.bash | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 55d58eef9c7c..abfd1e67b2ef 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -2,6 +2,17 @@
>  : "${MAX_SMP:=$(getconf _NPROCESSORS_ONLN)}"
>  : "${TIMEOUT:=90s}"
>  
> +# The following parameters are enabled by default when running a test with
> +# kvmtool:
> +# --nodefaults: suppress VM configuration that cannot be disabled otherwise
> +#               (like modifying the supplied kernel command line). Tests that
> +#               use the command line will fail without this parameter.
> +# --network mode=none: do not create a network device. kvmtool tries to help the
> +#                user by automatically create one, and then prints a warning
> +#                when the VM terminates if the device hasn't been initialized.
> +# --loglevel=warning: reduce verbosity
> +: "${KVMTOOL_DEFAULT_OPTS:="--nodefaults --network mode=none --loglevel=warning"}"
> +
>  PASS() { echo -ne "\e[32mPASS\e[0m"; }
>  SKIP() { echo -ne "\e[33mSKIP\e[0m"; }
>  FAIL() { echo -ne "\e[31mFAIL\e[0m"; }
> @@ -103,7 +114,7 @@ function run()
>          opts="-smp $smp $qemu_opts"
>          ;;
>      kvmtool)
> -        opts="--cpus $smp $kvmtool_opts"
> +        opts="$KVMTOOL_DEFAULT_OPTS --cpus $smp $kvmtool_opts"
>          ;;
>      esac
>  
> -- 
> 2.47.1
>

Otherwise,

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

Re: [kvm-unit-tests PATCH v2 12/18] scripts/runtime: Add default arguments for kvmtool

[Alexandru Elisei]

Hi,

On Thu, Jan 23, 2025 at 03:07:18PM +0100, Andrew Jones wrote:
> On Mon, Jan 20, 2025 at 04:43:10PM +0000, Alexandru Elisei wrote:
> > kvmtool, unless told otherwise, will do its best to make sure that a kernel
> > successfully boots in a virtual machine. Among things like automatically
> > creating a rootfs, it also adds extra parameters to the kernel command
> > line. This is actively harmful to kvm-unit-tests, because some tests parse
> > the kernel command line and they will fail if they encounter the options
> > added by kvmtool.
> > 
> > Fortunately for us, kvmtool commit 5613ae26b998 ("Add --nodefaults command
> > line argument") addded the --nodefaults kvmtool parameter which disables
> 
> added
> 
> > all the implicit virtual machine configuration that cannot be disabled by
> > using other parameters, like modifying the kernel command line. Always use
> > --nodefaults to allow a test to run.
> > 
> > kvmtool can be too verbose when running a virtual machine, and this is
> > controlled with parameters. Add those to the default kvmtool command line
> > to reduce this verbosity to a minimum.
> > 
> > Before:
> > 
> > $ vm run arm/selftest.flat --cpus 2 --mem 256 --params "setup smp=2 mem=256"
> >   Info: # lkvm run -k arm/selftest.flat -m 256 -c 2 --name guest-5035
> > Unknown subtest
> > 
> > EXIT: STATUS=127
> >   Warning: KVM compatibility warning.
> > 	virtio-9p device was not detected.
> > 	While you have requested a virtio-9p device, the guest kernel did not initialize it.
> > 	Please make sure that the guest kernel was compiled with CONFIG_NET_9P_VIRTIO=y enabled in .config.
> >   Warning: KVM compatibility warning.
> > 	virtio-net device was not detected.
> > 	While you have requested a virtio-net device, the guest kernel did not initialize it.
> > 	Please make sure that the guest kernel was compiled with CONFIG_VIRTIO_NET=y enabled in .config.
> >   Info: KVM session ended normally.
> > 
> > After:
> > 
> > $ vm run arm/selftest.flat --nodefaults --network mode=none --loglevel=warning --cpus 2 --mem 256 --params "setup smp=2 mem=256"
> 
> On riscv I've noticed that with --nodefaults if I don't add parameters
> with --params then kvmtool segfaults. I have to add --params "" to
> avoid it. Does that also happen on arm? Anyway, that's something we
> should fix in kvmtool rather than workaround it here.

This should fix it:

diff --git a/riscv/fdt.c b/riscv/fdt.c
index 85c8f95604f6..f6a702533258 100644
--- a/riscv/fdt.c
+++ b/riscv/fdt.c
@@ -256,9 +256,10 @@ static int setup_fdt(struct kvm *kvm)
                if (kvm->cfg.kernel_cmdline)
                        _FDT(fdt_property_string(fdt, "bootargs",
                                                 kvm->cfg.kernel_cmdline));
-       } else
+       } else if (kvm->cfg.real_cmdline) {
                _FDT(fdt_property_string(fdt, "bootargs",
                                         kvm->cfg.real_cmdline));
+       }

        _FDT(fdt_property_string(fdt, "stdout-path", "serial0"));


Looking at the timestamp on the commit, the patch that added --nodefaults
came before the patch that added riscv to kvmtool (by about a month). Just
in case you want to add a Fixes tag.

Thanks,
Alex

[kvm-unit-tests PATCH v2 13/18] run_tests: Do not probe for maximum number of VCPUs when using kvmtool

[Alexandru Elisei]

The --probe-maxsmp parameter updates MAX_SMP with the maximum number of
VCPUs that the host supports. Qemu will exit with an error when creating a
virtual machine if the number of VCPUs is exceeded.

kvmtool behaves differently: it will automatically limit the number of
VCPUs to the what KVM supports, which is exactly what --probe-maxsmp wants
to achieve. When doing --probe-maxsmp with kvmtool, print a message
explaining why it's redundant and don't do anything else.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 run_tests.sh | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/run_tests.sh b/run_tests.sh
index 61480d0c05ed..acaaadbb879b 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -89,7 +89,15 @@ while [ $# -gt 0 ]; do
             list_tests="yes"
             ;;
         --probe-maxsmp)
-            probe_maxsmp
+            case "$TARGET" in
+            qemu)
+                probe_maxsmp
+                ;;
+            kvmtool)
+                echo "kvmtool automatically limits the number of VCPUs to maximum supported"
+                echo "The 'smp' test parameter won't be modified"
+                ;;
+            esac
             ;;
         --)
             ;;
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 13/18] run_tests: Do not probe for maximum number of VCPUs when using kvmtool

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:43:11PM +0000, Alexandru Elisei wrote:
> The --probe-maxsmp parameter updates MAX_SMP with the maximum number of
> VCPUs that the host supports. Qemu will exit with an error when creating a
> virtual machine if the number of VCPUs is exceeded.
> 
> kvmtool behaves differently: it will automatically limit the number of
> VCPUs to the what KVM supports, which is exactly what --probe-maxsmp wants
> to achieve. When doing --probe-maxsmp with kvmtool, print a message
> explaining why it's redundant and don't do anything else.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  run_tests.sh | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/run_tests.sh b/run_tests.sh
> index 61480d0c05ed..acaaadbb879b 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -89,7 +89,15 @@ while [ $# -gt 0 ]; do
>              list_tests="yes"
>              ;;
>          --probe-maxsmp)
> -            probe_maxsmp
> +            case "$TARGET" in
> +            qemu)
> +                probe_maxsmp
> +                ;;
> +            kvmtool)
> +                echo "kvmtool automatically limits the number of VCPUs to maximum supported"
> +                echo "The 'smp' test parameter won't be modified"
> +                ;;
> +            esac
>              ;;
>          --)
>              ;;
> -- 
> 2.47.1
>

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

but I hope we can do something with the associative array idea to
avoid all the TARGET case statements that are getting scattered
around. Here, we could rename probe_maxsmp to qemu_probe_maxmp and
then add kvmtool_probe_maxsmp which does this output and each
to their respective vmm[$vmm,probe_maxsmp] member.

Thanks,
drew

[kvm-unit-tests PATCH v2 14/18] run_tests: Add KVMTOOL environment variable for kvmtool binary path

[Alexandru Elisei]

kvmtool is often used for prototyping new features, and a developer might
not want to install it system-wide. Add a KVMTOOL environment variable to
make it easier for tests to use a binary not in $PATH.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 run_tests.sh          | 1 +
 scripts/arch-run.bash | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/run_tests.sh b/run_tests.sh
index acaaadbb879b..d38954be9093 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -36,6 +36,7 @@ The following environment variables are used:
     TIMEOUT         Timeout duration for the timeout(1) command
     CHECK           Overwrites the 'check' unit test parameter (see
                     docs/unittests.txt)
+    KVMTOOL         Path to kvmtool binary for ARCH-run
 EOF
 }
 
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 34f633cade01..5d840b72f8cb 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -457,7 +457,7 @@ search_kvmtool_binary ()
 {
 	local kvmtoolcmd kvmtool
 
-	for kvmtoolcmd in lkvm vm lkvm-static; do
+	for kvmtoolcmd in ${KVMTOOL:-lkvm vm lkvm-static}; do
 		if $kvmtoolcmd --help 2>/dev/null| grep -q 'The most commonly used'; then
 			kvmtool="$kvmtoolcmd"
 			break
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 14/18] run_tests: Add KVMTOOL environment variable for kvmtool binary path

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:43:12PM +0000, Alexandru Elisei wrote:
> kvmtool is often used for prototyping new features, and a developer might
> not want to install it system-wide. Add a KVMTOOL environment variable to
> make it easier for tests to use a binary not in $PATH.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  run_tests.sh          | 1 +
>  scripts/arch-run.bash | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/run_tests.sh b/run_tests.sh
> index acaaadbb879b..d38954be9093 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -36,6 +36,7 @@ The following environment variables are used:
>      TIMEOUT         Timeout duration for the timeout(1) command
>      CHECK           Overwrites the 'check' unit test parameter (see
>                      docs/unittests.txt)
> +    KVMTOOL         Path to kvmtool binary for ARCH-run
>  EOF
>  }
>  
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 34f633cade01..5d840b72f8cb 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -457,7 +457,7 @@ search_kvmtool_binary ()
>  {
>  	local kvmtoolcmd kvmtool
>  
> -	for kvmtoolcmd in lkvm vm lkvm-static; do
> +	for kvmtoolcmd in ${KVMTOOL:-lkvm vm lkvm-static}; do
>  		if $kvmtoolcmd --help 2>/dev/null| grep -q 'The most commonly used'; then
>  			kvmtool="$kvmtoolcmd"
>  			break

Let's had the help text that search_qemu_binary() has with this patch
pointing out that a binary can be specified with KVMTOOL.

Thanks,
drew

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

[kvm-unit-tests PATCH v2 15/18] Add kvmtool_params to test specification

[Alexandru Elisei]

arm/arm64 supports running tests under kvmtool, but kvmtool's syntax for
running a virtual machine is different than qemu's. To run tests using the
automated test infrastructure, add a new test parameter, kvmtool_params.
The parameter serves the exact purpose as qemu_params/extra_params, but using
kvmtool's syntax.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arm/unittests.cfg   | 27 +++++++++++++++++++++++++++
 docs/unittests.txt  |  8 ++++++++
 scripts/common.bash |  4 ++++
 3 files changed, 39 insertions(+)

diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index 2bdad67d5693..974a5a9e4113 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -16,18 +16,21 @@
 file = selftest.flat
 smp = 2
 extra_params = -m 256 -append 'setup smp=2 mem=256'
+kvmtool_params = --mem 256 --params 'setup smp=2 mem=256'
 groups = selftest
 
 # Test vector setup and exception handling (kernel mode).
 [selftest-vectors-kernel]
 file = selftest.flat
 extra_params = -append 'vectors-kernel'
+kvmtool_params = --params 'vectors-kernel'
 groups = selftest
 
 # Test vector setup and exception handling (user mode).
 [selftest-vectors-user]
 file = selftest.flat
 extra_params = -append 'vectors-user'
+kvmtool_params = --params 'vectors-user'
 groups = selftest
 
 # Test SMP support
@@ -35,6 +38,7 @@ groups = selftest
 file = selftest.flat
 smp = $MAX_SMP
 extra_params = -append 'smp'
+kvmtool_params = --params 'smp'
 groups = selftest
 
 # Test PCI emulation
@@ -47,66 +51,77 @@ groups = pci
 file = pmu.flat
 groups = pmu
 extra_params = -append 'cycle-counter 0'
+kvmtool_params = --pmu --params 'cycle-counter 0'
 
 [pmu-event-introspection]
 file = pmu.flat
 groups = pmu
 arch = arm64
 extra_params = -append 'pmu-event-introspection'
+kvmtool_params = --pmu --params 'pmu-event-introspection'
 
 [pmu-event-counter-config]
 file = pmu.flat
 groups = pmu
 arch = arm64
 extra_params = -append 'pmu-event-counter-config'
+kvmtool_params = --pmu --params 'pmu-event-counter-config'
 
 [pmu-basic-event-count]
 file = pmu.flat
 groups = pmu
 arch = arm64
 extra_params = -append 'pmu-basic-event-count'
+kvmtool_params = --pmu --params 'pmu-basic-event-count'
 
 [pmu-mem-access]
 file = pmu.flat
 groups = pmu
 arch = arm64
 extra_params = -append 'pmu-mem-access'
+kvmtool_params = --pmu --params 'pmu-mem-access'
 
 [pmu-mem-access-reliability]
 file = pmu.flat
 groups = pmu
 arch = arm64
 extra_params = -append 'pmu-mem-access-reliability'
+kvmtool_params = --pmu --params 'pmu-mem-access-reliability'
 
 [pmu-sw-incr]
 file = pmu.flat
 groups = pmu
 arch = arm64
 extra_params = -append 'pmu-sw-incr'
+kvmtool_params = --pmu --params 'pmu-sw-incr'
 
 [pmu-chained-counters]
 file = pmu.flat
 groups = pmu
 arch = arm64
 extra_params = -append 'pmu-chained-counters'
+kvmtool_params = --pmu --params 'pmu-chained-counters'
 
 [pmu-chained-sw-incr]
 file = pmu.flat
 groups = pmu
 arch = arm64
 extra_params = -append 'pmu-chained-sw-incr'
+kvmtool_params = --pmu --params 'pmu-chained-sw-incr'
 
 [pmu-chain-promotion]
 file = pmu.flat
 groups = pmu
 arch = arm64
 extra_params = -append 'pmu-chain-promotion'
+kvmtool_params = --pmu --params 'pmu-chain-promotion'
 
 [pmu-overflow-interrupt]
 file = pmu.flat
 groups = pmu
 arch = arm64
 extra_params = -append 'pmu-overflow-interrupt'
+kvmtool_params = --pmu --params 'pmu-overflow-interrupt'
 
 # Test PMU support (TCG) with -icount IPC=1
 #[pmu-tcg-icount-1]
@@ -127,48 +142,56 @@ extra_params = -append 'pmu-overflow-interrupt'
 file = gic.flat
 smp = $((($MAX_SMP < 8)?$MAX_SMP:8))
 extra_params = -machine gic-version=2 -append 'ipi'
+kvmtool_params = --irqchip=gicv2 --params 'ipi'
 groups = gic
 
 [gicv2-mmio]
 file = gic.flat
 smp = $((($MAX_SMP < 8)?$MAX_SMP:8))
 extra_params = -machine gic-version=2 -append 'mmio'
+kvmtool_params = --irqchip=gicv2 --params 'mmio'
 groups = gic
 
 [gicv2-mmio-up]
 file = gic.flat
 smp = 1
 extra_params = -machine gic-version=2 -append 'mmio'
+kvmtool_params = --irqchip=gicv2 --params 'mmio'
 groups = gic
 
 [gicv2-mmio-3p]
 file = gic.flat
 smp = $((($MAX_SMP < 3)?$MAX_SMP:3))
 extra_params = -machine gic-version=2 -append 'mmio'
+kvmtool_params = --irqchip=gicv2 --params 'mmio'
 groups = gic
 
 [gicv3-ipi]
 file = gic.flat
 smp = $MAX_SMP
 extra_params = -machine gic-version=3 -append 'ipi'
+kvmtool_params = --irqchip=gicv3 --params 'ipi'
 groups = gic
 
 [gicv2-active]
 file = gic.flat
 smp = $((($MAX_SMP < 8)?$MAX_SMP:8))
 extra_params = -machine gic-version=2 -append 'active'
+kvmtool_params = --irqchip=gicv2 --params 'active'
 groups = gic
 
 [gicv3-active]
 file = gic.flat
 smp = $MAX_SMP
 extra_params = -machine gic-version=3 -append 'active'
+kvmtool_params = --irqchip=gicv3 --params 'active'
 groups = gic
 
 [its-introspection]
 file = gic.flat
 smp = $MAX_SMP
 extra_params = -machine gic-version=3 -append 'its-introspection'
+kvmtool_params = --irqchip=gicv3-its --params 'its-introspection'
 groups = its
 arch = arm64
 
@@ -176,6 +199,7 @@ arch = arm64
 file = gic.flat
 smp = $MAX_SMP
 extra_params = -machine gic-version=3 -append 'its-trigger'
+kvmtool_params = --irqchip=gicv3-its --params 'its-trigger'
 groups = its
 arch = arm64
 
@@ -232,6 +256,7 @@ groups = cache
 file = debug.flat
 arch = arm64
 extra_params = -append 'bp'
+kvmtool_params = --params 'bp'
 groups = debug
 
 [debug-bp-migration]
@@ -244,6 +269,7 @@ groups = debug migration
 file = debug.flat
 arch = arm64
 extra_params = -append 'wp'
+kvmtool_params = --params 'wp'
 groups = debug
 
 [debug-wp-migration]
@@ -256,6 +282,7 @@ groups = debug migration
 file = debug.flat
 arch = arm64
 extra_params = -append 'ss'
+kvmtool_params = --params 'ss'
 groups = debug
 
 [debug-sstep-migration]
diff --git a/docs/unittests.txt b/docs/unittests.txt
index 3e1a9e563016..ebb6994cab77 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -69,6 +69,14 @@ extra_params
 Alias for 'qemu_params', supported for compatibility purposes. Use
 'qemu_params' for new tests.
 
+kvmtool_params
+--------------
+Extra parameters supplied to the kvmtool process. Works similarly to
+qemu_params and extra_params, but uses kvmtool's syntax for command line
+arguments. The example for qemu_params, applied to kvmtool, would be:
+
+kvmtool_params = --mem 256 --params 'smp=2'
+
 groups
 ------
 groups = <group_name1> <group_name2> ...
diff --git a/scripts/common.bash b/scripts/common.bash
index 1b5e0d667841..f54ffbd7a87b 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -67,6 +67,10 @@ function for_each_unittest()
 			qemu_opts=$(parse_opts ${BASH_REMATCH[2]}$'\n' $fd)
 		elif [[ $line =~ ^(extra_params|qemu_params)\ *=\ *(.*)$ ]]; then
 			qemu_opts=${BASH_REMATCH[2]}
+		elif [[ $line =~ ^kvmtool_params\ *=\ *'"""'(.*)$ ]]; then
+			kvmtool_opts=$(parse_opts ${BASH_REMATCH[1]}$'\n' $fd)
+		elif [[ $line =~ ^kvmtool_params\ *=\ *(.*)$ ]]; then
+			kvmtool_opts=${BASH_REMATCH[1]}
 		elif [[ $line =~ ^groups\ *=\ *(.*)$ ]]; then
 			groups=${BASH_REMATCH[1]}
 		elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 15/18] Add kvmtool_params to test specification

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:43:13PM +0000, Alexandru Elisei wrote:
> arm/arm64 supports running tests under kvmtool, but kvmtool's syntax for
> running a virtual machine is different than qemu's. To run tests using the
> automated test infrastructure, add a new test parameter, kvmtool_params.
> The parameter serves the exact purpose as qemu_params/extra_params, but using
> kvmtool's syntax.

The need for qemu_params and kvmtool_params makes more sense to me now
that I see the use in unittests.cfg (I wonder if we can't rearrange this
series to help understand these things up front?). There's a lot of
duplication, though, with having two sets of params since the test-
specific inputs always have to be duplicated. To avoid the duplication
I think we can use extra_params for '-append' and '--params' by
parametrizing the option name for "params" (-append / --params) and then
create qemu_opts and kvmtool_opts for extra options like --pmu, --mem,
and irqchip.

Thanks,
drew

> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  arm/unittests.cfg   | 27 +++++++++++++++++++++++++++
>  docs/unittests.txt  |  8 ++++++++
>  scripts/common.bash |  4 ++++
>  3 files changed, 39 insertions(+)
> 
> diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> index 2bdad67d5693..974a5a9e4113 100644
> --- a/arm/unittests.cfg
> +++ b/arm/unittests.cfg
> @@ -16,18 +16,21 @@
>  file = selftest.flat
>  smp = 2
>  extra_params = -m 256 -append 'setup smp=2 mem=256'
> +kvmtool_params = --mem 256 --params 'setup smp=2 mem=256'
>  groups = selftest
>  
>  # Test vector setup and exception handling (kernel mode).
>  [selftest-vectors-kernel]
>  file = selftest.flat
>  extra_params = -append 'vectors-kernel'
> +kvmtool_params = --params 'vectors-kernel'
>  groups = selftest
>  
>  # Test vector setup and exception handling (user mode).
>  [selftest-vectors-user]
>  file = selftest.flat
>  extra_params = -append 'vectors-user'
> +kvmtool_params = --params 'vectors-user'
>  groups = selftest
>  
>  # Test SMP support
> @@ -35,6 +38,7 @@ groups = selftest
>  file = selftest.flat
>  smp = $MAX_SMP
>  extra_params = -append 'smp'
> +kvmtool_params = --params 'smp'
>  groups = selftest
>  
>  # Test PCI emulation
> @@ -47,66 +51,77 @@ groups = pci
>  file = pmu.flat
>  groups = pmu
>  extra_params = -append 'cycle-counter 0'
> +kvmtool_params = --pmu --params 'cycle-counter 0'
>  
>  [pmu-event-introspection]
>  file = pmu.flat
>  groups = pmu
>  arch = arm64
>  extra_params = -append 'pmu-event-introspection'
> +kvmtool_params = --pmu --params 'pmu-event-introspection'
>  
>  [pmu-event-counter-config]
>  file = pmu.flat
>  groups = pmu
>  arch = arm64
>  extra_params = -append 'pmu-event-counter-config'
> +kvmtool_params = --pmu --params 'pmu-event-counter-config'
>  
>  [pmu-basic-event-count]
>  file = pmu.flat
>  groups = pmu
>  arch = arm64
>  extra_params = -append 'pmu-basic-event-count'
> +kvmtool_params = --pmu --params 'pmu-basic-event-count'
>  
>  [pmu-mem-access]
>  file = pmu.flat
>  groups = pmu
>  arch = arm64
>  extra_params = -append 'pmu-mem-access'
> +kvmtool_params = --pmu --params 'pmu-mem-access'
>  
>  [pmu-mem-access-reliability]
>  file = pmu.flat
>  groups = pmu
>  arch = arm64
>  extra_params = -append 'pmu-mem-access-reliability'
> +kvmtool_params = --pmu --params 'pmu-mem-access-reliability'
>  
>  [pmu-sw-incr]
>  file = pmu.flat
>  groups = pmu
>  arch = arm64
>  extra_params = -append 'pmu-sw-incr'
> +kvmtool_params = --pmu --params 'pmu-sw-incr'
>  
>  [pmu-chained-counters]
>  file = pmu.flat
>  groups = pmu
>  arch = arm64
>  extra_params = -append 'pmu-chained-counters'
> +kvmtool_params = --pmu --params 'pmu-chained-counters'
>  
>  [pmu-chained-sw-incr]
>  file = pmu.flat
>  groups = pmu
>  arch = arm64
>  extra_params = -append 'pmu-chained-sw-incr'
> +kvmtool_params = --pmu --params 'pmu-chained-sw-incr'
>  
>  [pmu-chain-promotion]
>  file = pmu.flat
>  groups = pmu
>  arch = arm64
>  extra_params = -append 'pmu-chain-promotion'
> +kvmtool_params = --pmu --params 'pmu-chain-promotion'
>  
>  [pmu-overflow-interrupt]
>  file = pmu.flat
>  groups = pmu
>  arch = arm64
>  extra_params = -append 'pmu-overflow-interrupt'
> +kvmtool_params = --pmu --params 'pmu-overflow-interrupt'
>  
>  # Test PMU support (TCG) with -icount IPC=1
>  #[pmu-tcg-icount-1]
> @@ -127,48 +142,56 @@ extra_params = -append 'pmu-overflow-interrupt'
>  file = gic.flat
>  smp = $((($MAX_SMP < 8)?$MAX_SMP:8))
>  extra_params = -machine gic-version=2 -append 'ipi'
> +kvmtool_params = --irqchip=gicv2 --params 'ipi'
>  groups = gic
>  
>  [gicv2-mmio]
>  file = gic.flat
>  smp = $((($MAX_SMP < 8)?$MAX_SMP:8))
>  extra_params = -machine gic-version=2 -append 'mmio'
> +kvmtool_params = --irqchip=gicv2 --params 'mmio'
>  groups = gic
>  
>  [gicv2-mmio-up]
>  file = gic.flat
>  smp = 1
>  extra_params = -machine gic-version=2 -append 'mmio'
> +kvmtool_params = --irqchip=gicv2 --params 'mmio'
>  groups = gic
>  
>  [gicv2-mmio-3p]
>  file = gic.flat
>  smp = $((($MAX_SMP < 3)?$MAX_SMP:3))
>  extra_params = -machine gic-version=2 -append 'mmio'
> +kvmtool_params = --irqchip=gicv2 --params 'mmio'
>  groups = gic
>  
>  [gicv3-ipi]
>  file = gic.flat
>  smp = $MAX_SMP
>  extra_params = -machine gic-version=3 -append 'ipi'
> +kvmtool_params = --irqchip=gicv3 --params 'ipi'
>  groups = gic
>  
>  [gicv2-active]
>  file = gic.flat
>  smp = $((($MAX_SMP < 8)?$MAX_SMP:8))
>  extra_params = -machine gic-version=2 -append 'active'
> +kvmtool_params = --irqchip=gicv2 --params 'active'
>  groups = gic
>  
>  [gicv3-active]
>  file = gic.flat
>  smp = $MAX_SMP
>  extra_params = -machine gic-version=3 -append 'active'
> +kvmtool_params = --irqchip=gicv3 --params 'active'
>  groups = gic
>  
>  [its-introspection]
>  file = gic.flat
>  smp = $MAX_SMP
>  extra_params = -machine gic-version=3 -append 'its-introspection'
> +kvmtool_params = --irqchip=gicv3-its --params 'its-introspection'
>  groups = its
>  arch = arm64
>  
> @@ -176,6 +199,7 @@ arch = arm64
>  file = gic.flat
>  smp = $MAX_SMP
>  extra_params = -machine gic-version=3 -append 'its-trigger'
> +kvmtool_params = --irqchip=gicv3-its --params 'its-trigger'
>  groups = its
>  arch = arm64
>  
> @@ -232,6 +256,7 @@ groups = cache
>  file = debug.flat
>  arch = arm64
>  extra_params = -append 'bp'
> +kvmtool_params = --params 'bp'
>  groups = debug
>  
>  [debug-bp-migration]
> @@ -244,6 +269,7 @@ groups = debug migration
>  file = debug.flat
>  arch = arm64
>  extra_params = -append 'wp'
> +kvmtool_params = --params 'wp'
>  groups = debug
>  
>  [debug-wp-migration]
> @@ -256,6 +282,7 @@ groups = debug migration
>  file = debug.flat
>  arch = arm64
>  extra_params = -append 'ss'
> +kvmtool_params = --params 'ss'
>  groups = debug
>  
>  [debug-sstep-migration]
> diff --git a/docs/unittests.txt b/docs/unittests.txt
> index 3e1a9e563016..ebb6994cab77 100644
> --- a/docs/unittests.txt
> +++ b/docs/unittests.txt
> @@ -69,6 +69,14 @@ extra_params
>  Alias for 'qemu_params', supported for compatibility purposes. Use
>  'qemu_params' for new tests.
>  
> +kvmtool_params
> +--------------
> +Extra parameters supplied to the kvmtool process. Works similarly to
> +qemu_params and extra_params, but uses kvmtool's syntax for command line
> +arguments. The example for qemu_params, applied to kvmtool, would be:
> +
> +kvmtool_params = --mem 256 --params 'smp=2'
> +
>  groups
>  ------
>  groups = <group_name1> <group_name2> ...
> diff --git a/scripts/common.bash b/scripts/common.bash
> index 1b5e0d667841..f54ffbd7a87b 100644
> --- a/scripts/common.bash
> +++ b/scripts/common.bash
> @@ -67,6 +67,10 @@ function for_each_unittest()
>  			qemu_opts=$(parse_opts ${BASH_REMATCH[2]}$'\n' $fd)
>  		elif [[ $line =~ ^(extra_params|qemu_params)\ *=\ *(.*)$ ]]; then
>  			qemu_opts=${BASH_REMATCH[2]}
> +		elif [[ $line =~ ^kvmtool_params\ *=\ *'"""'(.*)$ ]]; then
> +			kvmtool_opts=$(parse_opts ${BASH_REMATCH[1]}$'\n' $fd)
> +		elif [[ $line =~ ^kvmtool_params\ *=\ *(.*)$ ]]; then
> +			kvmtool_opts=${BASH_REMATCH[1]}
>  		elif [[ $line =~ ^groups\ *=\ *(.*)$ ]]; then
>  			groups=${BASH_REMATCH[1]}
>  		elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
> -- 
> 2.47.1
> 
> 
> -- 
> kvm-riscv mailing list
> kvm-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [kvm-unit-tests PATCH v2 15/18] Add kvmtool_params to test specification

[Alexandru Elisei]

Hi Drew,

On Thu, Jan 23, 2025 at 04:53:29PM +0100, Andrew Jones wrote:
> On Mon, Jan 20, 2025 at 04:43:13PM +0000, Alexandru Elisei wrote:
> > arm/arm64 supports running tests under kvmtool, but kvmtool's syntax for
> > running a virtual machine is different than qemu's. To run tests using the
> > automated test infrastructure, add a new test parameter, kvmtool_params.
> > The parameter serves the exact purpose as qemu_params/extra_params, but using
> > kvmtool's syntax.
> 
> The need for qemu_params and kvmtool_params makes more sense to me now
> that I see the use in unittests.cfg (I wonder if we can't rearrange this
> series to help understand these things up front?). There's a lot of

Certainly, I'll move it closer to the beginning of the series.

> duplication, though, with having two sets of params since the test-
> specific inputs always have to be duplicated. To avoid the duplication
> I think we can use extra_params for '-append' and '--params' by
> parametrizing the option name for "params" (-append / --params) and then
> create qemu_opts and kvmtool_opts for extra options like --pmu, --mem,
> and irqchip.

How about something like this (I am using selftest-setup as an example, all the
other test definitions would be similarly modified):

diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index 2bdad67d5693..3009305ba2d3 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -15,7 +15,9 @@
 [selftest-setup]
 file = selftest.flat
 smp = 2
-extra_params = -m 256 -append 'setup smp=2 mem=256'
+test_args = setup smp=2 mem=256
+qemu_params = -m 256
+kvmtool_params = --mem 256
 groups = selftest

I was thinking about using 'test_args' instead of 'extra_params' to avoid any
confusion between the two, and to match how they are passed to a test
- they are in the argv main's argument.

Also, should I change the test definitions for all the other architectures?
It's not going to be possible for me to test all the changes.

Thanks,
Alex

> 
> Thanks,
> drew
> 
> > 
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > ---
> >  arm/unittests.cfg   | 27 +++++++++++++++++++++++++++
> >  docs/unittests.txt  |  8 ++++++++
> >  scripts/common.bash |  4 ++++
> >  3 files changed, 39 insertions(+)
> > 
> > diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> > index 2bdad67d5693..974a5a9e4113 100644
> > --- a/arm/unittests.cfg
> > +++ b/arm/unittests.cfg
> > @@ -16,18 +16,21 @@
> >  file = selftest.flat
> >  smp = 2
> >  extra_params = -m 256 -append 'setup smp=2 mem=256'
> > +kvmtool_params = --mem 256 --params 'setup smp=2 mem=256'
> >  groups = selftest
> >  
> >  # Test vector setup and exception handling (kernel mode).
> >  [selftest-vectors-kernel]
> >  file = selftest.flat
> >  extra_params = -append 'vectors-kernel'
> > +kvmtool_params = --params 'vectors-kernel'
> >  groups = selftest
> >  
> >  # Test vector setup and exception handling (user mode).
> >  [selftest-vectors-user]
> >  file = selftest.flat
> >  extra_params = -append 'vectors-user'
> > +kvmtool_params = --params 'vectors-user'
> >  groups = selftest
> >  
> >  # Test SMP support
> > @@ -35,6 +38,7 @@ groups = selftest
> >  file = selftest.flat
> >  smp = $MAX_SMP
> >  extra_params = -append 'smp'
> > +kvmtool_params = --params 'smp'
> >  groups = selftest
> >  
> >  # Test PCI emulation
> > @@ -47,66 +51,77 @@ groups = pci
> >  file = pmu.flat
> >  groups = pmu
> >  extra_params = -append 'cycle-counter 0'
> > +kvmtool_params = --pmu --params 'cycle-counter 0'
> >  
> >  [pmu-event-introspection]
> >  file = pmu.flat
> >  groups = pmu
> >  arch = arm64
> >  extra_params = -append 'pmu-event-introspection'
> > +kvmtool_params = --pmu --params 'pmu-event-introspection'
> >  
> >  [pmu-event-counter-config]
> >  file = pmu.flat
> >  groups = pmu
> >  arch = arm64
> >  extra_params = -append 'pmu-event-counter-config'
> > +kvmtool_params = --pmu --params 'pmu-event-counter-config'
> >  
> >  [pmu-basic-event-count]
> >  file = pmu.flat
> >  groups = pmu
> >  arch = arm64
> >  extra_params = -append 'pmu-basic-event-count'
> > +kvmtool_params = --pmu --params 'pmu-basic-event-count'
> >  
> >  [pmu-mem-access]
> >  file = pmu.flat
> >  groups = pmu
> >  arch = arm64
> >  extra_params = -append 'pmu-mem-access'
> > +kvmtool_params = --pmu --params 'pmu-mem-access'
> >  
> >  [pmu-mem-access-reliability]
> >  file = pmu.flat
> >  groups = pmu
> >  arch = arm64
> >  extra_params = -append 'pmu-mem-access-reliability'
> > +kvmtool_params = --pmu --params 'pmu-mem-access-reliability'
> >  
> >  [pmu-sw-incr]
> >  file = pmu.flat
> >  groups = pmu
> >  arch = arm64
> >  extra_params = -append 'pmu-sw-incr'
> > +kvmtool_params = --pmu --params 'pmu-sw-incr'
> >  
> >  [pmu-chained-counters]
> >  file = pmu.flat
> >  groups = pmu
> >  arch = arm64
> >  extra_params = -append 'pmu-chained-counters'
> > +kvmtool_params = --pmu --params 'pmu-chained-counters'
> >  
> >  [pmu-chained-sw-incr]
> >  file = pmu.flat
> >  groups = pmu
> >  arch = arm64
> >  extra_params = -append 'pmu-chained-sw-incr'
> > +kvmtool_params = --pmu --params 'pmu-chained-sw-incr'
> >  
> >  [pmu-chain-promotion]
> >  file = pmu.flat
> >  groups = pmu
> >  arch = arm64
> >  extra_params = -append 'pmu-chain-promotion'
> > +kvmtool_params = --pmu --params 'pmu-chain-promotion'
> >  
> >  [pmu-overflow-interrupt]
> >  file = pmu.flat
> >  groups = pmu
> >  arch = arm64
> >  extra_params = -append 'pmu-overflow-interrupt'
> > +kvmtool_params = --pmu --params 'pmu-overflow-interrupt'
> >  
> >  # Test PMU support (TCG) with -icount IPC=1
> >  #[pmu-tcg-icount-1]
> > @@ -127,48 +142,56 @@ extra_params = -append 'pmu-overflow-interrupt'
> >  file = gic.flat
> >  smp = $((($MAX_SMP < 8)?$MAX_SMP:8))
> >  extra_params = -machine gic-version=2 -append 'ipi'
> > +kvmtool_params = --irqchip=gicv2 --params 'ipi'
> >  groups = gic
> >  
> >  [gicv2-mmio]
> >  file = gic.flat
> >  smp = $((($MAX_SMP < 8)?$MAX_SMP:8))
> >  extra_params = -machine gic-version=2 -append 'mmio'
> > +kvmtool_params = --irqchip=gicv2 --params 'mmio'
> >  groups = gic
> >  
> >  [gicv2-mmio-up]
> >  file = gic.flat
> >  smp = 1
> >  extra_params = -machine gic-version=2 -append 'mmio'
> > +kvmtool_params = --irqchip=gicv2 --params 'mmio'
> >  groups = gic
> >  
> >  [gicv2-mmio-3p]
> >  file = gic.flat
> >  smp = $((($MAX_SMP < 3)?$MAX_SMP:3))
> >  extra_params = -machine gic-version=2 -append 'mmio'
> > +kvmtool_params = --irqchip=gicv2 --params 'mmio'
> >  groups = gic
> >  
> >  [gicv3-ipi]
> >  file = gic.flat
> >  smp = $MAX_SMP
> >  extra_params = -machine gic-version=3 -append 'ipi'
> > +kvmtool_params = --irqchip=gicv3 --params 'ipi'
> >  groups = gic
> >  
> >  [gicv2-active]
> >  file = gic.flat
> >  smp = $((($MAX_SMP < 8)?$MAX_SMP:8))
> >  extra_params = -machine gic-version=2 -append 'active'
> > +kvmtool_params = --irqchip=gicv2 --params 'active'
> >  groups = gic
> >  
> >  [gicv3-active]
> >  file = gic.flat
> >  smp = $MAX_SMP
> >  extra_params = -machine gic-version=3 -append 'active'
> > +kvmtool_params = --irqchip=gicv3 --params 'active'
> >  groups = gic
> >  
> >  [its-introspection]
> >  file = gic.flat
> >  smp = $MAX_SMP
> >  extra_params = -machine gic-version=3 -append 'its-introspection'
> > +kvmtool_params = --irqchip=gicv3-its --params 'its-introspection'
> >  groups = its
> >  arch = arm64
> >  
> > @@ -176,6 +199,7 @@ arch = arm64
> >  file = gic.flat
> >  smp = $MAX_SMP
> >  extra_params = -machine gic-version=3 -append 'its-trigger'
> > +kvmtool_params = --irqchip=gicv3-its --params 'its-trigger'
> >  groups = its
> >  arch = arm64
> >  
> > @@ -232,6 +256,7 @@ groups = cache
> >  file = debug.flat
> >  arch = arm64
> >  extra_params = -append 'bp'
> > +kvmtool_params = --params 'bp'
> >  groups = debug
> >  
> >  [debug-bp-migration]
> > @@ -244,6 +269,7 @@ groups = debug migration
> >  file = debug.flat
> >  arch = arm64
> >  extra_params = -append 'wp'
> > +kvmtool_params = --params 'wp'
> >  groups = debug
> >  
> >  [debug-wp-migration]
> > @@ -256,6 +282,7 @@ groups = debug migration
> >  file = debug.flat
> >  arch = arm64
> >  extra_params = -append 'ss'
> > +kvmtool_params = --params 'ss'
> >  groups = debug
> >  
> >  [debug-sstep-migration]
> > diff --git a/docs/unittests.txt b/docs/unittests.txt
> > index 3e1a9e563016..ebb6994cab77 100644
> > --- a/docs/unittests.txt
> > +++ b/docs/unittests.txt
> > @@ -69,6 +69,14 @@ extra_params
> >  Alias for 'qemu_params', supported for compatibility purposes. Use
> >  'qemu_params' for new tests.
> >  
> > +kvmtool_params
> > +--------------
> > +Extra parameters supplied to the kvmtool process. Works similarly to
> > +qemu_params and extra_params, but uses kvmtool's syntax for command line
> > +arguments. The example for qemu_params, applied to kvmtool, would be:
> > +
> > +kvmtool_params = --mem 256 --params 'smp=2'
> > +
> >  groups
> >  ------
> >  groups = <group_name1> <group_name2> ...
> > diff --git a/scripts/common.bash b/scripts/common.bash
> > index 1b5e0d667841..f54ffbd7a87b 100644
> > --- a/scripts/common.bash
> > +++ b/scripts/common.bash
> > @@ -67,6 +67,10 @@ function for_each_unittest()
> >  			qemu_opts=$(parse_opts ${BASH_REMATCH[2]}$'\n' $fd)
> >  		elif [[ $line =~ ^(extra_params|qemu_params)\ *=\ *(.*)$ ]]; then
> >  			qemu_opts=${BASH_REMATCH[2]}
> > +		elif [[ $line =~ ^kvmtool_params\ *=\ *'"""'(.*)$ ]]; then
> > +			kvmtool_opts=$(parse_opts ${BASH_REMATCH[1]}$'\n' $fd)
> > +		elif [[ $line =~ ^kvmtool_params\ *=\ *(.*)$ ]]; then
> > +			kvmtool_opts=${BASH_REMATCH[1]}
> >  		elif [[ $line =~ ^groups\ *=\ *(.*)$ ]]; then
> >  			groups=${BASH_REMATCH[1]}
> >  		elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
> > -- 
> > 2.47.1
> > 
> > 
> > -- 
> > kvm-riscv mailing list
> > kvm-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [kvm-unit-tests PATCH v2 15/18] Add kvmtool_params to test specification

[Andrew Jones]

On Tue, Feb 11, 2025 at 03:03:09PM +0000, Alexandru Elisei wrote:
> Hi Drew,
> 
> On Thu, Jan 23, 2025 at 04:53:29PM +0100, Andrew Jones wrote:
> > On Mon, Jan 20, 2025 at 04:43:13PM +0000, Alexandru Elisei wrote:
> > > arm/arm64 supports running tests under kvmtool, but kvmtool's syntax for
> > > running a virtual machine is different than qemu's. To run tests using the
> > > automated test infrastructure, add a new test parameter, kvmtool_params.
> > > The parameter serves the exact purpose as qemu_params/extra_params, but using
> > > kvmtool's syntax.
> > 
> > The need for qemu_params and kvmtool_params makes more sense to me now
> > that I see the use in unittests.cfg (I wonder if we can't rearrange this
> > series to help understand these things up front?). There's a lot of
> 
> Certainly, I'll move it closer to the beginning of the series.
> 
> > duplication, though, with having two sets of params since the test-
> > specific inputs always have to be duplicated. To avoid the duplication
> > I think we can use extra_params for '-append' and '--params' by
> > parametrizing the option name for "params" (-append / --params) and then
> > create qemu_opts and kvmtool_opts for extra options like --pmu, --mem,
> > and irqchip.
> 
> How about something like this (I am using selftest-setup as an example, all the
> other test definitions would be similarly modified):
> 
> diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> index 2bdad67d5693..3009305ba2d3 100644
> --- a/arm/unittests.cfg
> +++ b/arm/unittests.cfg
> @@ -15,7 +15,9 @@
>  [selftest-setup]
>  file = selftest.flat
>  smp = 2
> -extra_params = -m 256 -append 'setup smp=2 mem=256'
> +test_args = setup smp=2 mem=256
> +qemu_params = -m 256
> +kvmtool_params = --mem 256
>  groups = selftest
> 
> I was thinking about using 'test_args' instead of 'extra_params' to avoid any
> confusion between the two, and to match how they are passed to a test
> - they are in the argv main's argument.

Yes, this looks good and test_args is better than my suggestion in the
other mail of 'cmdline_options' since "cmdline" would be ambiguous with
the test's cmdline and the vmm's cmdline.

> 
> Also, should I change the test definitions for all the other architectures?
> It's not going to be possible for me to test all the changes.

We should be safe with an s/extra_params/qemu_params/ change for all
architectures and CI is pretty good, so we'd have good confidence
if it passes, but, I think we should keep extra_params as a qemu_params
alias anyway since it's possible that people have wrapped kvm-unit-tests
in test harnesses which generate unittests.cfg files.

Thanks,
drew

Re: [kvm-unit-tests PATCH v2 15/18] Add kvmtool_params to test specification

[Alexandru Elisei]

Hi Drew,

On Wed, Feb 12, 2025 at 04:56:42PM +0100, Andrew Jones wrote:
> On Tue, Feb 11, 2025 at 03:03:09PM +0000, Alexandru Elisei wrote:
> > Hi Drew,
> > 
> > On Thu, Jan 23, 2025 at 04:53:29PM +0100, Andrew Jones wrote:
> > > On Mon, Jan 20, 2025 at 04:43:13PM +0000, Alexandru Elisei wrote:
> > > > arm/arm64 supports running tests under kvmtool, but kvmtool's syntax for
> > > > running a virtual machine is different than qemu's. To run tests using the
> > > > automated test infrastructure, add a new test parameter, kvmtool_params.
> > > > The parameter serves the exact purpose as qemu_params/extra_params, but using
> > > > kvmtool's syntax.
> > > 
> > > The need for qemu_params and kvmtool_params makes more sense to me now
> > > that I see the use in unittests.cfg (I wonder if we can't rearrange this
> > > series to help understand these things up front?). There's a lot of
> > 
> > Certainly, I'll move it closer to the beginning of the series.
> > 
> > > duplication, though, with having two sets of params since the test-
> > > specific inputs always have to be duplicated. To avoid the duplication
> > > I think we can use extra_params for '-append' and '--params' by
> > > parametrizing the option name for "params" (-append / --params) and then
> > > create qemu_opts and kvmtool_opts for extra options like --pmu, --mem,
> > > and irqchip.
> > 
> > How about something like this (I am using selftest-setup as an example, all the
> > other test definitions would be similarly modified):
> > 
> > diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> > index 2bdad67d5693..3009305ba2d3 100644
> > --- a/arm/unittests.cfg
> > +++ b/arm/unittests.cfg
> > @@ -15,7 +15,9 @@
> >  [selftest-setup]
> >  file = selftest.flat
> >  smp = 2
> > -extra_params = -m 256 -append 'setup smp=2 mem=256'
> > +test_args = setup smp=2 mem=256
> > +qemu_params = -m 256
> > +kvmtool_params = --mem 256
> >  groups = selftest
> > 
> > I was thinking about using 'test_args' instead of 'extra_params' to avoid any
> > confusion between the two, and to match how they are passed to a test
> > - they are in the argv main's argument.
> 
> Yes, this looks good and test_args is better than my suggestion in the
> other mail of 'cmdline_options' since "cmdline" would be ambiguous with
> the test's cmdline and the vmm's cmdline.
> 
> > 
> > Also, should I change the test definitions for all the other architectures?
> > It's not going to be possible for me to test all the changes.
> 
> We should be safe with an s/extra_params/qemu_params/ change for all
> architectures and CI is pretty good, so we'd have good confidence
> if it passes, but, I think we should keep extra_params as a qemu_params
> alias anyway since it's possible that people have wrapped kvm-unit-tests
> in test harnesses which generate unittests.cfg files.

Sounds good, split extra_params into test_args and qemu_params in all
unittests.cfg files, and keep extra_params as an alias for qemu_params.

I was thinking that maybe I should send that as a separate patch, to make
sure it gets the visibility it deserves from the other maintainers, instead
of it being buried in a 18 patch series. What do you think?

Thanks,
Alex

Re: [kvm-unit-tests PATCH v2 15/18] Add kvmtool_params to test specification

[Andrew Jones]

On Wed, Feb 12, 2025 at 04:34:44PM +0000, Alexandru Elisei wrote:
> Hi Drew,
> 
> On Wed, Feb 12, 2025 at 04:56:42PM +0100, Andrew Jones wrote:
> > On Tue, Feb 11, 2025 at 03:03:09PM +0000, Alexandru Elisei wrote:
> > > Hi Drew,
> > > 
> > > On Thu, Jan 23, 2025 at 04:53:29PM +0100, Andrew Jones wrote:
> > > > On Mon, Jan 20, 2025 at 04:43:13PM +0000, Alexandru Elisei wrote:
> > > > > arm/arm64 supports running tests under kvmtool, but kvmtool's syntax for
> > > > > running a virtual machine is different than qemu's. To run tests using the
> > > > > automated test infrastructure, add a new test parameter, kvmtool_params.
> > > > > The parameter serves the exact purpose as qemu_params/extra_params, but using
> > > > > kvmtool's syntax.
> > > > 
> > > > The need for qemu_params and kvmtool_params makes more sense to me now
> > > > that I see the use in unittests.cfg (I wonder if we can't rearrange this
> > > > series to help understand these things up front?). There's a lot of
> > > 
> > > Certainly, I'll move it closer to the beginning of the series.
> > > 
> > > > duplication, though, with having two sets of params since the test-
> > > > specific inputs always have to be duplicated. To avoid the duplication
> > > > I think we can use extra_params for '-append' and '--params' by
> > > > parametrizing the option name for "params" (-append / --params) and then
> > > > create qemu_opts and kvmtool_opts for extra options like --pmu, --mem,
> > > > and irqchip.
> > > 
> > > How about something like this (I am using selftest-setup as an example, all the
> > > other test definitions would be similarly modified):
> > > 
> > > diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> > > index 2bdad67d5693..3009305ba2d3 100644
> > > --- a/arm/unittests.cfg
> > > +++ b/arm/unittests.cfg
> > > @@ -15,7 +15,9 @@
> > >  [selftest-setup]
> > >  file = selftest.flat
> > >  smp = 2
> > > -extra_params = -m 256 -append 'setup smp=2 mem=256'
> > > +test_args = setup smp=2 mem=256
> > > +qemu_params = -m 256
> > > +kvmtool_params = --mem 256
> > >  groups = selftest
> > > 
> > > I was thinking about using 'test_args' instead of 'extra_params' to avoid any
> > > confusion between the two, and to match how they are passed to a test
> > > - they are in the argv main's argument.
> > 
> > Yes, this looks good and test_args is better than my suggestion in the
> > other mail of 'cmdline_options' since "cmdline" would be ambiguous with
> > the test's cmdline and the vmm's cmdline.
> > 
> > > 
> > > Also, should I change the test definitions for all the other architectures?
> > > It's not going to be possible for me to test all the changes.
> > 
> > We should be safe with an s/extra_params/qemu_params/ change for all
> > architectures and CI is pretty good, so we'd have good confidence
> > if it passes, but, I think we should keep extra_params as a qemu_params
> > alias anyway since it's possible that people have wrapped kvm-unit-tests
> > in test harnesses which generate unittests.cfg files.
> 
> Sounds good, split extra_params into test_args and qemu_params in all
> unittests.cfg files, and keep extra_params as an alias for qemu_params.
> 
> I was thinking that maybe I should send that as a separate patch, to make
> sure it gets the visibility it deserves from the other maintainers, instead
> of it being buried in a 18 patch series. What do you think?

Sounds good.

Thanks,
drew

[kvm-unit-tests PATCH v2 16/18] scripts/mkstandalone: Export $TARGET

[Alexandru Elisei]

$TARGET is needed for the test runner to decide if it should use qemu or
kvmtool, so export it.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 scripts/mkstandalone.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 4de97056e641..10abb5e191b7 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -51,6 +51,7 @@ generate_test ()
 	config_export ARCH
 	config_export ARCH_NAME
 	config_export PROCESSOR
+	config_export TARGET
 
 	echo "echo BUILD_HEAD=$(cat build-head)"
 
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 16/18] scripts/mkstandalone: Export $TARGET

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:43:14PM +0000, Alexandru Elisei wrote:
> $TARGET is needed for the test runner to decide if it should use qemu or
> kvmtool, so export it.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  scripts/mkstandalone.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index 4de97056e641..10abb5e191b7 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -51,6 +51,7 @@ generate_test ()
>  	config_export ARCH
>  	config_export ARCH_NAME
>  	config_export PROCESSOR
> +	config_export TARGET
>  
>  	echo "echo BUILD_HEAD=$(cat build-head)"
>  
> -- 
> 2.47.1
>

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

[kvm-unit-tests PATCH v2 17/18] unittest: Add disabled_if parameter and use it for kvmtool

[Alexandru Elisei]

The pci-test is qemu specific. Other tests perform migration, which
isn't supported by kvmtool. In general, kvmtool is not as feature-rich
as qemu, so add a new unittest parameter, disabled_if, that causes a
test to be skipped if the condition evaluates to true.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arm/unittests.cfg    |  7 +++++++
 docs/unittests.txt   | 13 +++++++++++++
 scripts/common.bash  |  8 ++++++--
 scripts/runtime.bash |  6 ++++++
 4 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index 974a5a9e4113..9b1df5e02a58 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -44,6 +44,7 @@ groups = selftest
 # Test PCI emulation
 [pci-test]
 file = pci-test.flat
+disabled_if = [[ "$TARGET" != qemu ]]
 groups = pci
 
 # Test PMU support
@@ -208,6 +209,7 @@ file = gic.flat
 smp = $MAX_SMP
 extra_params = -machine gic-version=3 -append 'its-migration'
 groups = its migration
+disabled_if = [[ "$TARGET" != qemu ]]
 arch = arm64
 
 [its-pending-migration]
@@ -215,6 +217,7 @@ file = gic.flat
 smp = $MAX_SMP
 extra_params = -machine gic-version=3 -append 'its-pending-migration'
 groups = its migration
+disabled_if = [[ "$TARGET" != qemu ]]
 arch = arm64
 
 [its-migrate-unmapped-collection]
@@ -222,6 +225,7 @@ file = gic.flat
 smp = $MAX_SMP
 extra_params = -machine gic-version=3 -append 'its-migrate-unmapped-collection'
 groups = its migration
+disabled_if = [[ "$TARGET" != qemu ]]
 arch = arm64
 
 # Test PSCI emulation
@@ -263,6 +267,7 @@ groups = debug
 file = debug.flat
 arch = arm64
 extra_params = -append 'bp-migration'
+disabled_if = [[ "$TARGET" != qemu ]]
 groups = debug migration
 
 [debug-wp]
@@ -276,6 +281,7 @@ groups = debug
 file = debug.flat
 arch = arm64
 extra_params = -append 'wp-migration'
+disabled_if = [[ "$TARGET" != qemu ]]
 groups = debug migration
 
 [debug-sstep]
@@ -289,6 +295,7 @@ groups = debug
 file = debug.flat
 arch = arm64
 extra_params = -append 'ss-migration'
+disabled_if = [[ "$TARGET" != qemu ]]
 groups = debug migration
 
 # FPU/SIMD test
diff --git a/docs/unittests.txt b/docs/unittests.txt
index ebb6994cab77..58d1a29146a3 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -115,3 +115,16 @@ parameter needs to be of the form <path>=<value>
 The path and value cannot contain space, =, or shell wildcard characters.
 
 Can be overwritten with the CHECK environment variable with the same syntax.
+
+disabled_if
+------
+disabled_if = <condition>
+
+Do not run the test if <condition> is met. <condition> will be fed unmodified
+to a bash 'if' statement and follows the same syntax.
+
+This can be used to prevent running a test when kvm-unit-tests is configured a
+certain way. For example, it can be used to skip a qemu specific test when
+using another VMM and using UEFI:
+
+disabled_if = [[ "$TARGET" != qemu ]] && [[ "$CONFIG_EFI" = y ]]
diff --git a/scripts/common.bash b/scripts/common.bash
index f54ffbd7a87b..c0ea2eabeda6 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -38,6 +38,7 @@ function for_each_unittest()
 	local accel
 	local timeout
 	local kvmtool_opts
+	local disabled_cond
 	local rematch
 
 	exec {fd}<"$unittests"
@@ -46,7 +47,7 @@ function for_each_unittest()
 		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
 			rematch=${BASH_REMATCH[1]}
 			if [ -n "${testname}" ]; then
-				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts"
+				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts" "$disabled_cond"
 			fi
 			testname=$rematch
 			smp=1
@@ -59,6 +60,7 @@ function for_each_unittest()
 			accel=""
 			timeout=""
 			kvmtool_opts=""
+			disabled_cond=""
 		elif [[ $line =~ ^file\ *=\ *(.*)$ ]]; then
 			kernel=$TEST_DIR/${BASH_REMATCH[1]}
 		elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
@@ -79,6 +81,8 @@ function for_each_unittest()
 			machine=${BASH_REMATCH[1]}
 		elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
 			check=${BASH_REMATCH[1]}
+		elif [[ $line =~ ^disabled_if\ *=\ *(.*)$ ]]; then
+			disabled_cond=${BASH_REMATCH[1]}
 		elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
 			accel=${BASH_REMATCH[1]}
 		elif [[ $line =~ ^timeout\ *=\ *(.*)$ ]]; then
@@ -86,7 +90,7 @@ function for_each_unittest()
 		fi
 	done
 	if [ -n "${testname}" ]; then
-		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts"
+		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts" "$disabled_cond"
 	fi
 	exec {fd}<&-
 }
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index abfd1e67b2ef..002bd2744d6b 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -108,6 +108,7 @@ function run()
     local accel="$9"
     local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
     local kvmtool_opts="${11}"
+    local disabled_cond="${12}"
 
     case "$TARGET" in
     qemu)
@@ -186,6 +187,11 @@ function run()
         done
     fi
 
+    if [[ "$disabled_cond" ]] && (eval $disabled_cond); then
+		print_result "SKIP" $testname "" "disabled because: $disabled_cond"
+		return 2
+	fi
+
     log=$(premature_failure) && {
         skip=true
         if [ "${CONFIG_EFI}" == "y" ]; then
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 17/18] unittest: Add disabled_if parameter and use it for kvmtool

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:43:15PM +0000, Alexandru Elisei wrote:
> The pci-test is qemu specific. Other tests perform migration, which
> isn't supported by kvmtool. In general, kvmtool is not as feature-rich
> as qemu, so add a new unittest parameter, disabled_if, that causes a
> test to be skipped if the condition evaluates to true.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  arm/unittests.cfg    |  7 +++++++
>  docs/unittests.txt   | 13 +++++++++++++
>  scripts/common.bash  |  8 ++++++--
>  scripts/runtime.bash |  6 ++++++
>  4 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> index 974a5a9e4113..9b1df5e02a58 100644
> --- a/arm/unittests.cfg
> +++ b/arm/unittests.cfg
> @@ -44,6 +44,7 @@ groups = selftest
>  # Test PCI emulation
>  [pci-test]
>  file = pci-test.flat
> +disabled_if = [[ "$TARGET" != qemu ]]
>  groups = pci
>  
>  # Test PMU support
> @@ -208,6 +209,7 @@ file = gic.flat
>  smp = $MAX_SMP
>  extra_params = -machine gic-version=3 -append 'its-migration'
>  groups = its migration
> +disabled_if = [[ "$TARGET" != qemu ]]
>  arch = arm64
>  
>  [its-pending-migration]
> @@ -215,6 +217,7 @@ file = gic.flat
>  smp = $MAX_SMP
>  extra_params = -machine gic-version=3 -append 'its-pending-migration'
>  groups = its migration
> +disabled_if = [[ "$TARGET" != qemu ]]
>  arch = arm64
>  
>  [its-migrate-unmapped-collection]
> @@ -222,6 +225,7 @@ file = gic.flat
>  smp = $MAX_SMP
>  extra_params = -machine gic-version=3 -append 'its-migrate-unmapped-collection'
>  groups = its migration
> +disabled_if = [[ "$TARGET" != qemu ]]
>  arch = arm64
>  
>  # Test PSCI emulation
> @@ -263,6 +267,7 @@ groups = debug
>  file = debug.flat
>  arch = arm64
>  extra_params = -append 'bp-migration'
> +disabled_if = [[ "$TARGET" != qemu ]]
>  groups = debug migration
>  
>  [debug-wp]
> @@ -276,6 +281,7 @@ groups = debug
>  file = debug.flat
>  arch = arm64
>  extra_params = -append 'wp-migration'
> +disabled_if = [[ "$TARGET" != qemu ]]
>  groups = debug migration
>  
>  [debug-sstep]
> @@ -289,6 +295,7 @@ groups = debug
>  file = debug.flat
>  arch = arm64
>  extra_params = -append 'ss-migration'
> +disabled_if = [[ "$TARGET" != qemu ]]
>  groups = debug migration
>  
>  # FPU/SIMD test
> diff --git a/docs/unittests.txt b/docs/unittests.txt
> index ebb6994cab77..58d1a29146a3 100644
> --- a/docs/unittests.txt
> +++ b/docs/unittests.txt
> @@ -115,3 +115,16 @@ parameter needs to be of the form <path>=<value>
>  The path and value cannot contain space, =, or shell wildcard characters.
>  
>  Can be overwritten with the CHECK environment variable with the same syntax.
> +
> +disabled_if
> +------
> +disabled_if = <condition>
> +
> +Do not run the test if <condition> is met. <condition> will be fed unmodified
> +to a bash 'if' statement and follows the same syntax.
> +
> +This can be used to prevent running a test when kvm-unit-tests is configured a
> +certain way. For example, it can be used to skip a qemu specific test when
> +using another VMM and using UEFI:
> +
> +disabled_if = [[ "$TARGET" != qemu ]] && [[ "$CONFIG_EFI" = y ]]
> diff --git a/scripts/common.bash b/scripts/common.bash
> index f54ffbd7a87b..c0ea2eabeda6 100644
> --- a/scripts/common.bash
> +++ b/scripts/common.bash
> @@ -38,6 +38,7 @@ function for_each_unittest()
>  	local accel
>  	local timeout
>  	local kvmtool_opts
> +	local disabled_cond
>  	local rematch
>  
>  	exec {fd}<"$unittests"
> @@ -46,7 +47,7 @@ function for_each_unittest()
>  		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
>  			rematch=${BASH_REMATCH[1]}
>  			if [ -n "${testname}" ]; then
> -				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts"
> +				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts" "$disabled_cond"
>  			fi
>  			testname=$rematch
>  			smp=1
> @@ -59,6 +60,7 @@ function for_each_unittest()
>  			accel=""
>  			timeout=""
>  			kvmtool_opts=""
> +			disabled_cond=""
>  		elif [[ $line =~ ^file\ *=\ *(.*)$ ]]; then
>  			kernel=$TEST_DIR/${BASH_REMATCH[1]}
>  		elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
> @@ -79,6 +81,8 @@ function for_each_unittest()
>  			machine=${BASH_REMATCH[1]}
>  		elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
>  			check=${BASH_REMATCH[1]}
> +		elif [[ $line =~ ^disabled_if\ *=\ *(.*)$ ]]; then
> +			disabled_cond=${BASH_REMATCH[1]}
>  		elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
>  			accel=${BASH_REMATCH[1]}
>  		elif [[ $line =~ ^timeout\ *=\ *(.*)$ ]]; then
> @@ -86,7 +90,7 @@ function for_each_unittest()
>  		fi
>  	done
>  	if [ -n "${testname}" ]; then
> -		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts"
> +		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts" "$disabled_cond"
>  	fi
>  	exec {fd}<&-
>  }
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index abfd1e67b2ef..002bd2744d6b 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -108,6 +108,7 @@ function run()
>      local accel="$9"
>      local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
>      local kvmtool_opts="${11}"
> +    local disabled_cond="${12}"
>  
>      case "$TARGET" in
>      qemu)
> @@ -186,6 +187,11 @@ function run()
>          done
>      fi
>  
> +    if [[ "$disabled_cond" ]] && (eval $disabled_cond); then
> +		print_result "SKIP" $testname "" "disabled because: $disabled_cond"
> +		return 2
> +	fi
> +
>      log=$(premature_failure) && {
>          skip=true
>          if [ "${CONFIG_EFI}" == "y" ]; then
> -- 
> 2.47.1
>

I like disabled_if because I like the lambda-like thing it's doing, but I
wonder if it wouldn't be better to make TARGET a first class citizen by
adding a 'targets' unittest parameter which allows listing all targets the
test can run on, e.g.

[selftest-setup]
file = selftest.flat
smp = 2
extra_params = -m 256 -append 'setup smp=2 mem=256'
targets = qemu,kvmtool
groups = selftest

[pci-test]
file = pci-test.flat
targets = qemu
groups = pci

If targets isn't present then the default is only qemu.

Thanks,
drew

[kvm-unit-tests PATCH v2 18/18] run_tests: Enable kvmtool

[Alexandru Elisei]

Everything is in place to run the tests using kvmtool:

$ ./configure --target=kvmtool
$ make clean && make
$ KVMTOOL=<path/to/kvmtool> ./run_tests.sh

so enable it, and remove ERRATA_FORCE=y when configuring for kvmtool,
because the runner will generate and pass the correct environment to
kvmtool.

Missing is support for EFI tests. That's because distros don't ship a
EDK2 binary compiled for kvmtool, and on top of that kvm-unit-tests as
an EFI app hasn't been tested to work with kvmtool.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---

Should I also revert commit 35145f140442 ("arm/arm64: kvmtool: force all tests
to run") which introduced ERRATA_FORCE? I didn't do this now in case other
architectures use it/planning to use it.

 README.md               | 15 +++++++++++++++
 arm/run                 |  2 +-
 configure               |  1 -
 run_tests.sh            |  2 +-
 scripts/mkstandalone.sh |  2 +-
 5 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/README.md b/README.md
index be07dc28a094..5e7706f02553 100644
--- a/README.md
+++ b/README.md
@@ -65,6 +65,9 @@ or:
 
 to run them all.
 
+All tests can be run using QEMU. On arm and arm64, tests can also be run using
+kvmtool.
+
 By default the runner script searches for a suitable QEMU binary in the system.
 To select a specific QEMU binary though, specify the QEMU=path/to/binary
 environment variable:
@@ -80,10 +83,22 @@ For running tests that involve migration from one QEMU instance to another
 you also need to have the "ncat" binary (from the nmap.org project) installed,
 otherwise the related tests will be skipped.
 
+To run a test with kvmtool, please configure kvm-unit-tests accordingly first:
+
+   ./configure --arch=arm64 --target=kvmtool
+
+then run the test(s) like with QEMU above.
+
+To select a kvmtool binary, specify the KVMTOOL=path/to/binary environment
+variable. kvmtool supports only kvm as the accelerator.
+
 ## Running the tests with UEFI
 
 Check [x86/efi/README.md](./x86/efi/README.md).
 
+On arm and arm64, this is only supported with QEMU; kvmtool cannot run the
+tests under UEFI.
+
 # Tests configuration file
 
 The test case may need specific runtime configurations, for
diff --git a/arm/run b/arm/run
index 880d5afae86d..438a2617e564 100755
--- a/arm/run
+++ b/arm/run
@@ -10,7 +10,7 @@ if [ -z "$KUT_STANDALONE" ]; then
 fi
 
 case "$TARGET" in
-qemu)
+qemu | kvmtool)
     ;;
 *)
    echo "'$TARGET' not supported"
diff --git a/configure b/configure
index 86cf1da36467..17d3d931f2c0 100755
--- a/configure
+++ b/configure
@@ -299,7 +299,6 @@ elif [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then
         arm_uart_early_addr=0x09000000
     elif [ "$target" = "kvmtool" ]; then
         arm_uart_early_addr=0x1000000
-        errata_force=1
     else
         echo "--target must be one of 'qemu' or 'kvmtool'!"
         usage
diff --git a/run_tests.sh b/run_tests.sh
index d38954be9093..3921dcdcb344 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -110,7 +110,7 @@ while [ $# -gt 0 ]; do
 done
 
 case "$TARGET" in
-qemu)
+qemu | kvmtool)
     ;;
 *)
     echo "$0 does not support '$TARGET'"
diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 10abb5e191b7..16383b05adfa 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -8,7 +8,7 @@ source config.mak
 source scripts/common.bash
 
 case "$TARGET" in
-qemu)
+qemu | kvmtool)
     ;;
 *)
     echo "'$TARGET' not supported for standlone tests"
-- 
2.47.1

Re: [kvm-unit-tests PATCH v2 18/18] run_tests: Enable kvmtool

[Andrew Jones]

On Mon, Jan 20, 2025 at 04:43:16PM +0000, Alexandru Elisei wrote:
> Everything is in place to run the tests using kvmtool:
> 
> $ ./configure --target=kvmtool
> $ make clean && make
> $ KVMTOOL=<path/to/kvmtool> ./run_tests.sh
> 
> so enable it, and remove ERRATA_FORCE=y when configuring for kvmtool,
> because the runner will generate and pass the correct environment to
> kvmtool.
> 
> Missing is support for EFI tests. That's because distros don't ship a
> EDK2 binary compiled for kvmtool, and on top of that kvm-unit-tests as
> an EFI app hasn't been tested to work with kvmtool.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
> 
> Should I also revert commit 35145f140442 ("arm/arm64: kvmtool: force all tests
> to run") which introduced ERRATA_FORCE? I didn't do this now in case other
> architectures use it/planning to use it.

We can leave ERRATA_FORCE, I use from time to time for quick testing.

> 
>  README.md               | 15 +++++++++++++++
>  arm/run                 |  2 +-
>  configure               |  1 -
>  run_tests.sh            |  2 +-
>  scripts/mkstandalone.sh |  2 +-
>  5 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/README.md b/README.md
> index be07dc28a094..5e7706f02553 100644
> --- a/README.md
> +++ b/README.md
> @@ -65,6 +65,9 @@ or:
>  
>  to run them all.
>  
> +All tests can be run using QEMU. On arm and arm64, tests can also be run using
> +kvmtool.
> +
>  By default the runner script searches for a suitable QEMU binary in the system.
>  To select a specific QEMU binary though, specify the QEMU=path/to/binary
>  environment variable:
> @@ -80,10 +83,22 @@ For running tests that involve migration from one QEMU instance to another
>  you also need to have the "ncat" binary (from the nmap.org project) installed,
>  otherwise the related tests will be skipped.
>  
> +To run a test with kvmtool, please configure kvm-unit-tests accordingly first:
> +
> +   ./configure --arch=arm64 --target=kvmtool
> +
> +then run the test(s) like with QEMU above.
> +
> +To select a kvmtool binary, specify the KVMTOOL=path/to/binary environment
> +variable. kvmtool supports only kvm as the accelerator.
> +
>  ## Running the tests with UEFI
>  
>  Check [x86/efi/README.md](./x86/efi/README.md).
>  
> +On arm and arm64, this is only supported with QEMU; kvmtool cannot run the
> +tests under UEFI.
> +
>  # Tests configuration file
>  
>  The test case may need specific runtime configurations, for
> diff --git a/arm/run b/arm/run
> index 880d5afae86d..438a2617e564 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -10,7 +10,7 @@ if [ -z "$KUT_STANDALONE" ]; then
>  fi
>  
>  case "$TARGET" in
> -qemu)
> +qemu | kvmtool)
>      ;;
>  *)
>     echo "'$TARGET' not supported"
> diff --git a/configure b/configure
> index 86cf1da36467..17d3d931f2c0 100755
> --- a/configure
> +++ b/configure
> @@ -299,7 +299,6 @@ elif [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then
>          arm_uart_early_addr=0x09000000
>      elif [ "$target" = "kvmtool" ]; then
>          arm_uart_early_addr=0x1000000
> -        errata_force=1
>      else
>          echo "--target must be one of 'qemu' or 'kvmtool'!"
>          usage
> diff --git a/run_tests.sh b/run_tests.sh
> index d38954be9093..3921dcdcb344 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -110,7 +110,7 @@ while [ $# -gt 0 ]; do
>  done
>  
>  case "$TARGET" in
> -qemu)
> +qemu | kvmtool)
>      ;;
>  *)
>      echo "$0 does not support '$TARGET'"
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index 10abb5e191b7..16383b05adfa 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -8,7 +8,7 @@ source config.mak
>  source scripts/common.bash
>  
>  case "$TARGET" in
> -qemu)
> +qemu | kvmtool)
>      ;;
>  *)
>      echo "'$TARGET' not supported for standlone tests"
> -- 
> 2.47.1
>

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>