Re: [PATCH] arm/run: don't enable KVM if system can't do it

["Alex Bennée" <alex.bennee@linaro.org>]

Andrew Jones <drjones@redhat.com> writes:

> On Thu, Jul 02, 2015 at 12:05:31PM +0100, Alex Bennée wrote:
>> As ARM (and no doubt other systems) can also run tests in pure TCG mode
>> we might as well not bother enabling accel=kvm if we aren't on a real
>> ARM based system. This prevents us seeing ugly warning messages when
>> testing TCG.
>
> First,
> YAY! We're getting contributions to kvm-unit-tests/arm!

:-) well so far I've been noodling about looking at it for KVM Guest
Debug testing. I've a hideous branch on github that attempts to test
exercise the debug register trapping code. However that falls down as I
really need to find an easy way of attaching GDB to the qemu-gdb stub
while the test is running.

However with the TCG multi-thread work coming up I certainly see the
need to exercise QEMU in a way that the internal TCG test code might
have trouble with.

>
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  arm/run | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arm/run b/arm/run
>> index 662a856..2bdb4be 100755
>> --- a/arm/run
>> +++ b/arm/run
>> @@ -33,7 +33,13 @@ if $qemu $M -chardev testdev,id=id -initrd . 2>&1 \
>>  	exit 2
>>  fi
>>  
>> -M='-machine virt,accel=kvm:tcg'
>> +host=`uname -m | sed -e 's/arm.*/arm/'`
>> +if [ "${host}" = "arm" ] || [ "${host}" = "aarch64" ]; then
>> +    M='-machine virt,accel=kvm:tcg'
>> +else
>> +    M='-machine virt,accel=tcg'
>> +fi
>
> I think this is a good idea, although I had actually left that warning
> on purpose. Originally, the plan was for these unit tests to be kvm
> specific. If they could be developed with the aid of tcg, and even used
> to test tcg, then fine, but running them on tcg should always complain,
> in order to make sure that the test output clearly showed that it had
> not been running on kvm. Developing unit tests for tcg is also a good
> idea though, and there's really no reason not to share this framework.
>
> So, for this patch I'd prefer we do a few things differently;
>
> 1) we should be able to integrate this new condition with the
>    "arm64 must use '-cpu host' with kvm" condition that is lower down.
>    And, let's just make this $HOST variable one that ./configure
>    prepares, allowing that arm64 condition to s/$(arch)/$HOST/ and
>    avoiding the need to duplicate the sed -e 's/arm.*/arm/'

Yeah makes sense.

>
> 2) we might as well do something like
>
>    M='-machine virt'
>    if using-kvm
>      M+=',accel=kvm'
>    else
>      M+=',accel=tcg'
>    fi
>
>    now, since we don't want to use the accel fallback feature anymore
>
> 3) outputting which one we're using might still be nice, otherwise
>    one must inspect the qemu command line in the logs to find out
>
> 4) I recently mentioned[*] it might be nice to add a '-force-tcg' type
>    of arm/run command line option, allowing tcg to be used even if
>    it's possible to use kvm. Adding that at the same time would be
>    nice.

Would it also be useful for other arches? Does run-tests.sh pass 
>
> 5) we use tabs for indentation in arm/run, and only bother with the
>    variable's {}, if necessary

My shell quoting was rusty. I think $(host) was calling the host command
for some reason.

>
> 6) we should post patches with [kvm-unit-tests PATCH] to avoid
>    confusion with other kvm postings. (I screwed that up on my
>    last two postings...).

/me ponders if he can just config git for that.

I'll patch the readme ;-)

>
> Thanks!
> drew
>
> [*] https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg07514.html
>
>> +
>>  chr_testdev='-device virtio-serial-device'
>>  chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd'
>>  
>> -- 
>> 2.4.5
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Alex Bennée