From: Andrew Jones <andrew.jones@linux.dev>
To: Gavin Shan <gshan@redhat.com>
Cc: kvmarm@lists.linux.dev, kvm@vger.kernel.org,
kvm-ppc@vger.kernel.org, linux-s390@vger.kernel.org,
lvivier@redhat.com, thuth@redhat.com, frankja@linux.ibm.com,
imbrenda@linux.ibm.com, david@redhat.com, pbonzini@redhat.com,
nrb@linux.ibm.com, shan.gavin@gmail.com
Subject: Re: [kvm-unit-tests PATCH v4] runtime: Allow to specify properties for accelerator
Date: Fri, 23 Jun 2023 15:38:07 +0000 [thread overview]
Message-ID: <20230623-4a8c1d21acfdfe304333173d@orel> (raw)
In-Reply-To: <20230623035750.312679-1-gshan@redhat.com>
On Fri, Jun 23, 2023 at 01:57:50PM +1000, Gavin Shan wrote:
> There are extra properties for accelerators to enable the specific
> features. For example, the dirty ring for KVM accelerator can be
> enabled by "-accel kvm,dirty-ring-sizee536". Unfortuntely, the
> extra properties for the accelerators aren't supported. It makes
> it's impossible to test the combination of KVM and dirty ring
> as the following error message indicates.
>
> # cd /home/gavin/sandbox/kvm-unit-tests/tests
> # QEMU=/home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \
> ACCEL=kvm,dirty-ring-sizee536 ./its-migration
> :
> BUILD_HEAD/ffb37e
> timeout -k 1s --foreground 90s \
> /home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \
> -nodefaults -machine virt -accel kvm,dirty-ring-sizee536 \
> -cpu cortex-a57 -device virtio-serial-device \
> -device virtconsole,chardev=ctd -chardev testdev,id=ctd \
> -device pci-testdev -display none -serial stdio \
> -kernel _NO_FILE_4Uhere_ -smp 160 -machine gic-version=3 \
> -append its-pending-migration # -initrd /tmp/tmp.gfDLa1EtWk
> :
> qemu-system-aarch64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0):
> Invalid argument
>
> Allow to specify extra properties for accelerators. With this, the
> "its-migration" can be tested for the combination of KVM and dirty
> ring. Rename get_qemu_accelerator() to set_qemu_accelerator() since
> no values are returned by printing at return.
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> Tested-by: Nico Boehr <nrb@linux.ibm.com>
> Acked-by: Nico Boehr <nrb@linux.ibm.com>
> ---
> v4: Rename get_qemu_accelerator() to set_qemu_accelerator() and
> don't break the fix included in commit c7d6c7f00e7c by setting
> $ACCEL to "tcg" before set_qemu_accelerator() is called, suggested
> by Drew.
> ---
> arm/run | 20 ++++++++------------
> powerpc/run | 5 ++---
> s390x/run | 5 ++---
> scripts/arch-run.bash | 23 ++++++++++++++---------
> x86/run | 5 ++---
> 5 files changed, 28 insertions(+), 30 deletions(-)
>
> diff --git a/arm/run b/arm/run
> index c6f25b8..956940f 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -10,24 +10,20 @@ if [ -z "$KUT_STANDALONE" ]; then
> fi
> processor="$PROCESSOR"
>
> -accel=$(get_qemu_accelerator) ||
> - exit $?
> +if [ "$QEMU" ] && [ -z "$ACCEL" ] &&
> + [ "$HOST" = "aarch64" ] && [ "$ARCH" = "arm" ] &&
> + [ "$(basename $QEMU)" = "qemu-system-arm" ]; then
> + ACCEL="tcg"
> +fi
>
> -if [ "$accel" = "kvm" ]; then
> +set_qemu_accelerator || exit $?
> +if [ "$ACCEL" = "kvm" ]; then
> QEMU_ARCH=$HOST
> fi
>
> qemu=$(search_qemu_binary) ||
> exit $?
>
> -if [ "$QEMU" ] && [ -z "$ACCEL" ] &&
> - [ "$HOST" = "aarch64" ] && [ "$ARCH" = "arm" ] &&
> - [ "$(basename $QEMU)" = "qemu-system-arm" ]; then
> - accel=tcg
> -fi
> -
> -ACCEL=$accel
> -
> if ! $qemu -machine '?' | grep -q 'ARM Virtual Machine'; then
> echo "$qemu doesn't support mach-virt ('-machine virt'). Exiting."
> exit 2
> @@ -72,7 +68,7 @@ if $qemu $M -device '?' | grep -q pci-testdev; then
> pci_testdev="-device pci-testdev"
> fi
>
> -A="-accel $ACCEL"
> +A="-accel $ACCEL$ACCEL_PROPS"
> command="$qemu -nodefaults $M $A -cpu $processor $chr_testdev $pci_testdev"
> command+=" -display none -serial stdio -kernel"
> command="$(migration_cmd) $(timeout_cmd) $command"
> diff --git a/powerpc/run b/powerpc/run
> index ee38e07..b353169 100755
> --- a/powerpc/run
> +++ b/powerpc/run
> @@ -9,8 +9,7 @@ if [ -z "$KUT_STANDALONE" ]; then
> source scripts/arch-run.bash
> fi
>
> -ACCEL=$(get_qemu_accelerator) ||
> - exit $?
> +set_qemu_accelerator || exit $?
>
> qemu=$(search_qemu_binary) ||
> exit $?
> @@ -21,7 +20,7 @@ if ! $qemu -machine '?' 2>&1 | grep 'pseries' > /dev/null; then
> fi
>
> M='-machine pseries'
> -M+=",accel=$ACCEL"
> +M+=",accel=$ACCEL$ACCEL_PROPS"
> command="$qemu -nodefaults $M -bios $FIRMWARE"
> command+=" -display none -serial stdio -kernel"
> command="$(migration_cmd) $(timeout_cmd) $command"
> diff --git a/s390x/run b/s390x/run
> index f1111db..dcbf3f0 100755
> --- a/s390x/run
> +++ b/s390x/run
> @@ -9,8 +9,7 @@ if [ -z "$KUT_STANDALONE" ]; then
> source scripts/arch-run.bash
> fi
>
> -ACCEL=$(get_qemu_accelerator) ||
> - exit $?
> +set_qemu_accelerator || exit $?
>
> qemu=$(search_qemu_binary) ||
> exit $?
> @@ -26,7 +25,7 @@ if [ "${1: -7}" = ".pv.bin" ] || [ "${TESTNAME: -3}" = "_PV" ] && [ "$MIGRATION"
> fi
>
> M='-machine s390-ccw-virtio'
> -M+=",accel=$ACCEL"
> +M+=",accel=$ACCEL$ACCEL_PROPS"
> command="$qemu -nodefaults -nographic $M"
> command+=" -chardev stdio,id=con0 -device sclpconsole,chardev=con0"
> command+=" -kernel"
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 51e4b97..2d28e0b 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -410,8 +410,11 @@ hvf_available ()
> ( [ "$HOST" = x86_64 ] && [ "$ARCH" = i386 ] )
> }
>
> -get_qemu_accelerator ()
> +set_qemu_accelerator ()
> {
> + ACCEL_PROPS=${ACCEL#"${ACCEL%%,*}"}
> + ACCEL=${ACCEL%%,*}
> +
> if [ "$ACCEL" = "kvm" ] && ! kvm_available; then
> echo "KVM is needed, but not available on this host" >&2
> return 2
> @@ -421,13 +424,15 @@ get_qemu_accelerator ()
> return 2
> fi
>
> - if [ "$ACCEL" ]; then
> - echo $ACCEL
> - elif kvm_available; then
> - echo kvm
> - elif hvf_available; then
> - echo hvf
> - else
> - echo tcg
> + if [ -z "$ACCEL" ]; then
> + if kvm_available; then
> + ACCEL="kvm"
> + elif hvf_available; then
> + ACCEL="hvf"
> + else
> + ACCEL="tcg"
> + fi
> fi
> +
> + return 0
> }
> diff --git a/x86/run b/x86/run
> index 4d53b72..a3d3e7d 100755
> --- a/x86/run
> +++ b/x86/run
> @@ -9,8 +9,7 @@ if [ -z "$KUT_STANDALONE" ]; then
> source scripts/arch-run.bash
> fi
>
> -ACCEL=$(get_qemu_accelerator) ||
> - exit $?
> +set_qemu_accelerator || exit $?
>
> qemu=$(search_qemu_binary) ||
> exit $?
> @@ -38,7 +37,7 @@ else
> fi
>
> command="${qemu} --no-reboot -nodefaults $pc_testdev -vnc none -serial stdio $pci_testdev"
> -command+=" -machine accel=$ACCEL"
> +command+=" -machine accel=$ACCEL$ACCEL_PROPS"
> if [ "${CONFIG_EFI}" != y ]; then
> command+=" -kernel"
> fi
> --
> 2.40.1
>
Applied to arm/queue
https://gitlab.com/jones-drew/kvm-unit-tests/-/commits/arm/queue
Thanks,
drew
prev parent reply other threads:[~2023-06-23 15:38 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-23 3:57 [kvm-unit-tests PATCH v4] runtime: Allow to specify properties for accelerator Gavin Shan
2023-06-23 7:38 ` Andrew Jones
2023-06-23 15:38 ` Andrew Jones [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230623-4a8c1d21acfdfe304333173d@orel \
--to=andrew.jones@linux.dev \
--cc=david@redhat.com \
--cc=frankja@linux.ibm.com \
--cc=gshan@redhat.com \
--cc=imbrenda@linux.ibm.com \
--cc=kvm-ppc@vger.kernel.org \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.linux.dev \
--cc=linux-s390@vger.kernel.org \
--cc=lvivier@redhat.com \
--cc=nrb@linux.ibm.com \
--cc=pbonzini@redhat.com \
--cc=shan.gavin@gmail.com \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).