On Mon, Jun 12, 2023 at 03:07:08PM +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-size=65536". 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-size=65536 ./its-migration
> :
> BUILD_HEAD=2fffb37e
> timeout -k 1s --foreground 90s /home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \
> -nodefaults -machine virt -accel kvm,dirty-ring-size=65536 -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.
>
> Signed-off-by: Gavin Shan <gshan@xxxxxxxxxx>
> ---
> arm/run | 4 ++--
> scripts/arch-run.bash | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arm/run b/arm/run
> index c6f25b8..bbf80e0 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -35,13 +35,13 @@ fi
>
> M='-machine virt'
>
> -if [ "$ACCEL" = "kvm" ]; then
> +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 [[ "$ACCEL" =~ ^kvm.* ]] || [[ "$ACCEL" =~ ^hvf.* ]]; then
> if [ "$HOST" = "aarch64" ] || [ "$HOST" = "arm" ]; then
> processor="host"
> if [ "$ARCH" = "arm" ] && [ "$HOST" = "aarch64" ]; then
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 51e4b97..e20b965 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -412,11 +412,11 @@ hvf_available ()
>
> get_qemu_accelerator ()
> {
> - if [ "$ACCEL" = "kvm" ] && ! kvm_available; then
> + if [[ "$ACCEL" =~ ^kvm.* ]] && [[ ! kvm_available ]]; then
> echo "KVM is needed, but not available on this host" >&2
> return 2
> fi
> - if [ "$ACCEL" = "hvf" ] && ! hvf_available; then
> + if [[ "$ACCEL" =~ ^hvf.* ]] && [[ ! hvf_available ]]; then
> echo "HVF is needed, but not available on this host" >&2
> return 2
> fi
> --
> 2.23.0
>
Hi Gavin,
I'd prefer that when we want to match 'kvm', 'tcg', etc. that we split
on the first comma, rather than use a regular expression that allows
arbitrary characters to follow the pattern. Actually
get_qemu_accelerator() could do the splitting itself, providing two
variables, ACCEL (only kvm, tcg, etc.) and ACCEL_PROPS (which is
either null or has a leading comma). Then command lines just need
to use $ACCEL$ACCEL_PROPS. If we do that, then get_qemu_accelerator()
should also allow the user to pre-split, i.e.
ACCEL=kvm ACCEL_PROPS=dirty-ring-size=65536 arm/run ...
Finally, did you also test this with the accel property in the
unittests.cfg file run with run_tests.sh?
Thanks,
drew
