Re: [kvm-unit-tests PATCH v3 06/16] scripts: Refuse to run the tests if not configured for qemu

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi Drew,

On Wed, May 07, 2025 at 06:10:08PM +0200, Andrew Jones wrote:
> On Wed, May 07, 2025 at 04:12:46PM +0100, Alexandru Elisei wrote:
> > Arm and arm64 support running the tests under kvmtool. kvmtool has a
> > different command line syntax for configuring and running a virtual
> > machine, and the automated scripts know only how to use qemu.
> > 
> > One issue with that is even though the tests have been configured for
> > kvmtool (with ./configure --target=kvmtool), the scripts will use qemu to
> > run the tests, and without looking at the logs there is no indication that
> > the tests haven't been run with kvmtool, as configured.
> > 
> > Another issue is that kvmtool uses a different address for the UART and
> > when running the tests with qemu via the scripts, this warning is
> > displayed:
> > 
> > WARNING: early print support may not work. Found uart at 0x9000000, but early base is 0x1000000.
> > 
> > which might trip up an unsuspected user.
> > 
> > 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.
> > 
> > run_tests.sh ends up execuing either arm/run or arm/efi/run, so add a check
> 
> executing

Ack.

> 
> > to these two scripts for the test target, and refuse to run the test if
> > kvm-unit-tests has been configured for kvmtool.
> > 
> > mkstandalone.sh also executes arm/run or arm/efi run, but the usual use
> > case for standalone tests is to compile them on one machine, and then to
> > run them on a different machine. This two step process can be time
> > consuming, so save the user time (and frustration!) and add a check
> > directly to mkstandalone.sh.
> > 
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > ---
> >  arm/efi/run             |  3 +++
> >  arm/run                 |  4 ++++
> >  scripts/mkstandalone.sh |  3 +++
> >  scripts/vmm.bash        | 14 ++++++++++++++
> >  4 files changed, 24 insertions(+)
> >  create mode 100644 scripts/vmm.bash
> > 
> > diff --git a/arm/efi/run b/arm/efi/run
> > index 8f41fc02df31..53d71297cc52 100755
> > --- a/arm/efi/run
> > +++ b/arm/efi/run
> > @@ -11,6 +11,9 @@ if [ ! -f config.mak ]; then
> >  fi
> >  source config.mak
> >  source scripts/arch-run.bash
> > +source scripts/vmm.bash
> > +
> > +check_vmm_supported
> >  
> >  if [ -f /usr/share/qemu-efi-aarch64/QEMU_EFI.fd ]; then
> >  	DEFAULT_UEFI=/usr/share/qemu-efi-aarch64/QEMU_EFI.fd
> > diff --git a/arm/run b/arm/run
> > index ef58558231b7..56562ed1628f 100755
> > --- a/arm/run
> > +++ b/arm/run
> > @@ -7,7 +7,11 @@ if [ -z "$KUT_STANDALONE" ]; then
> >  	fi
> >  	source config.mak
> >  	source scripts/arch-run.bash
> > +	source scripts/vmm.bash
> >  fi
> > +
> > +check_vmm_supported
> > +
> >  qemu_cpu="$TARGET_CPU"
> >  
> >  if [ "$QEMU" ] && [ -z "$ACCEL" ] &&
> > diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> > index c4ba81f18935..4f666cefe076 100755
> > --- a/scripts/mkstandalone.sh
> > +++ b/scripts/mkstandalone.sh
> > @@ -6,6 +6,9 @@ if [ ! -f config.mak ]; then
> >  fi
> >  source config.mak
> >  source scripts/common.bash
> > +source scripts/vmm.bash
> > +
> > +check_vmm_supported
> >  
> >  temp_file ()
> >  {
> > diff --git a/scripts/vmm.bash b/scripts/vmm.bash
> > new file mode 100644
> > index 000000000000..39325858c6b3
> > --- /dev/null
> > +++ b/scripts/vmm.bash
> > @@ -0,0 +1,14 @@
> > +source config.mak
> > +
> > +function check_vmm_supported()
> > +{
> > +	case "$TARGET" in
> > +	qemu)
> > +		return 0
> > +		;;
> > +	*)
> > +		echo "$0 does not support target '$TARGET'"
> > +		exit 2
> > +		;;
> > +	esac
> > +}
> 
> Hmm. We now have configure saying one thing for arm/arm64 and this
> function saying another. Assuming this is just temporary and will
> be resolved in the next patches, then that's probably OK, though.

This is resolved in the last patch. Wanted to add this in case someone doesn't
apply the entire series and they end up with partial kvmtool support.

And thanks for having a look so fast!

Alex

> 
> Thanks,
> drew

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