Hi Drew,
On 6/12/23 6:27 PM, Andrew Jones wrote:
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
--
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 for your quick comments. I think we can be smart to split
$ACCEL in get_qemu_accelarator() into $ACCEL and $ACCEL_PROPS if you
agree. It's not free to maintain another user visible property for
$ACCEL_PROPS.
I forgot to run "./runtest.sh" to make sure nothing is broken. I will
do this before v2 is posted. Note that v2 will be posted shortly :)
Thanks,
Gavin
