[kvm-unit-tests PATCH v2 4/5] configure: Add --qemu-cpu option
Alexandru Elisei
alexandru.elisei at arm.com
Sun Mar 23 04:16:19 PDT 2025
Hi Drew,
On Sat, Mar 22, 2025 at 12:26:59PM +0100, Andrew Jones wrote:
> On Fri, Mar 14, 2025 at 03:49:04PM +0000, Jean-Philippe Brucker wrote:
> > Add the --qemu-cpu option to let users set the CPU type to run on.
> > At the moment --processor allows to set both GCC -mcpu flag and QEMU
> > -cpu. On Arm we'd like to pass `-cpu max` to QEMU in order to enable all
> > the TCG features by default, and it could also be nice to let users
> > modify the CPU capabilities by setting extra -cpu options.
> > Since GCC -mcpu doesn't accept "max" or "host", separate the compiler
> > and QEMU arguments.
> >
> > `--processor` is now exclusively for compiler options, as indicated by
> > its documentation ("processor to compile for"). So use $QEMU_CPU on
> > RISC-V as well.
> >
> > Suggested-by: Andrew Jones <andrew.jones at linux.dev>
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe at linaro.org>
> > ---
> > scripts/mkstandalone.sh | 2 +-
> > arm/run | 17 +++++++++++------
> > riscv/run | 8 ++++----
> > configure | 7 +++++++
> > 4 files changed, 23 insertions(+), 11 deletions(-)
> >
> > diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> > index 2318a85f..6b5f725d 100755
> > --- a/scripts/mkstandalone.sh
> > +++ b/scripts/mkstandalone.sh
> > @@ -42,7 +42,7 @@ generate_test ()
> >
> > config_export ARCH
> > config_export ARCH_NAME
> > - config_export PROCESSOR
> > + config_export QEMU_CPU
> >
> > echo "echo BUILD_HEAD=$(cat build-head)"
> >
> > diff --git a/arm/run b/arm/run
> > index efdd44ce..561bafab 100755
> > --- a/arm/run
> > +++ b/arm/run
> > @@ -8,7 +8,7 @@ if [ -z "$KUT_STANDALONE" ]; then
> > source config.mak
> > source scripts/arch-run.bash
> > fi
> > -processor="$PROCESSOR"
> > +qemu_cpu="$QEMU_CPU"
> >
> > if [ "$QEMU" ] && [ -z "$ACCEL" ] &&
> > [ "$HOST" = "aarch64" ] && [ "$ARCH" = "arm" ] &&
> > @@ -37,12 +37,17 @@ if [ "$ACCEL" = "kvm" ]; then
> > fi
> > fi
> >
> > -if [ "$ACCEL" = "kvm" ] || [ "$ACCEL" = "hvf" ]; then
> > - if [ "$HOST" = "aarch64" ] || [ "$HOST" = "arm" ]; then
> > - processor="host"
> > +if [ -z "$qemu_cpu" ]; then
> > + if ( [ "$ACCEL" = "kvm" ] || [ "$ACCEL" = "hvf" ] ) &&
> > + ( [ "$HOST" = "aarch64" ] || [ "$HOST" = "arm" ] ); then
> > + qemu_cpu="host"
> > if [ "$ARCH" = "arm" ] && [ "$HOST" = "aarch64" ]; then
> > - processor+=",aarch64=off"
> > + qemu_cpu+=",aarch64=off"
> > fi
> > + elif [ "$ARCH" = "arm64" ]; then
> > + qemu_cpu="cortex-a57"
> > + else
> > + qemu_cpu="cortex-a15"
>
> configure could set this in config.mak as DEFAULT_PROCESSOR, avoiding the
> need to duplicate it here.
That was my first instinct too, having the default value in config.mak seemed
like the correct solution.
But the problem with this is that the default -cpu type depends on -accel (set
via unittests.cfg or as an environment variable), host and test architecture
combination. All of these variables are known only at runtime.
Let's say we have DEFAULT_QEMU_CPU=cortex-a57 in config.mak. If we keep the
above heuristic, arm/run will override it with host,aarch64=off. IMO, having it
in config.mak, but arm/run using it only under certain conditions is worse than
not having it at all. arm/run choosing the default value **all the time** is at
least consistent.
We could modify the help text for --qemu-cpu to say something like "If left
unset, the $ARCH/run script will choose a best value based on the host system
and test configuration."
Thanks,
Alex
>
> > fi
> > fi
> >
> > @@ -71,7 +76,7 @@ if $qemu $M -device '?' | grep -q pci-testdev; then
> > fi
> >
> > A="-accel $ACCEL$ACCEL_PROPS"
> > -command="$qemu -nodefaults $M $A -cpu $processor $chr_testdev $pci_testdev"
> > +command="$qemu -nodefaults $M $A -cpu $qemu_cpu $chr_testdev $pci_testdev"
> > command+=" -display none -serial stdio"
> > command="$(migration_cmd) $(timeout_cmd) $command"
> >
> > diff --git a/riscv/run b/riscv/run
> > index e2f5a922..02fcf0c0 100755
> > --- a/riscv/run
> > +++ b/riscv/run
> > @@ -11,12 +11,12 @@ fi
> >
> > # Allow user overrides of some config.mak variables
> > mach=$MACHINE_OVERRIDE
> > -processor=$PROCESSOR_OVERRIDE
> > +qemu_cpu=$QEMU_CPU_OVERRIDE
> > firmware=$FIRMWARE_OVERRIDE
> >
> > -[ "$PROCESSOR" = "$ARCH" ] && PROCESSOR="max"
> > +[ -z "$QEMU_CPU" ] && QEMU_CPU="max"
> > : "${mach:=virt}"
> > -: "${processor:=$PROCESSOR}"
> > +: "${qemu_cpu:=$QEMU_CPU}"
> > : "${firmware:=$FIRMWARE}"
> > [ "$firmware" ] && firmware="-bios $firmware"
> >
> > @@ -32,7 +32,7 @@ fi
> > mach="-machine $mach"
> >
> > command="$qemu -nodefaults -nographic -serial mon:stdio"
> > -command+=" $mach $acc $firmware -cpu $processor "
> > +command+=" $mach $acc $firmware -cpu $qemu_cpu "
> > command="$(migration_cmd) $(timeout_cmd) $command"
> >
> > if [ "$UEFI_SHELL_RUN" = "y" ]; then
> > diff --git a/configure b/configure
> > index 5306bad3..d25bd23e 100755
> > --- a/configure
> > +++ b/configure
> > @@ -52,6 +52,7 @@ page_size=
> > earlycon=
> > efi=
> > efi_direct=
> > +qemu_cpu=
> >
> > # Enable -Werror by default for git repositories only (i.e. developer builds)
> > if [ -e "$srcdir"/.git ]; then
> > @@ -69,6 +70,8 @@ usage() {
> > --arch=ARCH architecture to compile for ($arch). ARCH can be one of:
> > arm, arm64, i386, ppc64, riscv32, riscv64, s390x, x86_64
> > --processor=PROCESSOR processor to compile for ($processor)
> > + --qemu-cpu=CPU the CPU model to run on. The default depends on
> > + the configuration, usually it is "host" or "max".
> > --target=TARGET target platform that the tests will be running on (qemu or
> > kvmtool, default is qemu) (arm/arm64 only)
> > --cross-prefix=PREFIX cross compiler prefix
> > @@ -142,6 +145,9 @@ while [[ $optno -le $argc ]]; do
> > --processor)
> > processor="$arg"
> > ;;
> > + --qemu-cpu)
> > + qemu_cpu="$arg"
> > + ;;
> > --target)
> > target="$arg"
> > ;;
> > @@ -464,6 +470,7 @@ ARCH=$arch
> > ARCH_NAME=$arch_name
> > ARCH_LIBDIR=$arch_libdir
> > PROCESSOR=$processor
> > +QEMU_CPU=$qemu_cpu
> > CC=$cc
> > CFLAGS=$cflags
> > LD=$cross_prefix$ld
> > --
> > 2.48.1
> >
>
> With the Alex's and Eric's requested changes to the help text,
>
> Reviewed-by: Andrew Jones <andrew.jones at linux.dev>
More information about the kvm-riscv
mailing list