From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Jones Subject: Re: [PATCH] arm/run: don't enable KVM if system can't do it Date: Thu, 2 Jul 2015 15:30:02 +0200 Message-ID: <20150702133002.GB25987@hawk.localdomain> References: <1435835131-11437-1-git-send-email-alex.bennee@linaro.org> <20150702115152.GD4243@hawk.localdomain> <87r3oqsnq9.fsf@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org, mttcg@greensocs.com, mark.burton@greensocs.com, fred.konrad@greensocs.com To: Alex =?iso-8859-1?Q?Benn=E9e?= Return-path: Received: from mx1.redhat.com ([209.132.183.28]:46077 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752016AbbGBNaH (ORCPT ); Thu, 2 Jul 2015 09:30:07 -0400 Content-Disposition: inline In-Reply-To: <87r3oqsnq9.fsf@linaro.org> Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Jul 02, 2015 at 02:17:18PM +0100, Alex Benn=E9e wrote: >=20 > Andrew Jones writes: >=20 > > On Thu, Jul 02, 2015 at 12:05:31PM +0100, Alex Benn=E9e wrote: > >> As ARM (and no doubt other systems) can also run tests in pure TCG= mode > >> we might as well not bother enabling accel=3Dkvm if we aren't on a= real > >> ARM based system. This prevents us seeing ugly warning messages wh= en > >> testing TCG. > > > > First, > > YAY! We're getting contributions to kvm-unit-tests/arm! >=20 > :-) 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. >=20 > 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. >=20 > > > >>=20 > >> Signed-off-by: Alex Benn=E9e > >> --- > >> arm/run | 8 +++++++- > >> 1 file changed, 7 insertions(+), 1 deletion(-) > >>=20 > >> 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=3Did -initrd . = 2>&1 \ > >> exit 2 > >> fi > >> =20 > >> -M=3D'-machine virt,accel=3Dkvm:tcg' > >> +host=3D`uname -m | sed -e 's/arm.*/arm/'` > >> +if [ "${host}" =3D "arm" ] || [ "${host}" =3D "aarch64" ]; then > >> + M=3D'-machine virt,accel=3Dkvm:tcg' > >> +else > >> + M=3D'-machine virt,accel=3Dtcg' > >> +fi > > > > I think this is a good idea, although I had actually left that warn= ing > > 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 compl= ain, > > in order to make sure that the test output clearly showed that it h= ad > > not been running on kvm. Developing unit tests for tcg is also a go= od > > idea though, and there's really no reason not to share this framewo= rk. > > > > 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 do= wn. > > 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/' >=20 > Yeah makes sense. >=20 > > > > 2) we might as well do something like > > > > M=3D'-machine virt' > > if using-kvm > > M+=3D',accel=3Dkvm' > > else > > M+=3D',accel=3Dtcg' > > fi > > > > now, since we don't want to use the accel fallback feature anymo= re > > > > 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' t= ype > > 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. >=20 > Would it also be useful for other arches? Does run-tests.sh pass=20 Maybe someday, so we might as well add it there. As long as it allows current command lines to keep working as they have, then why not. > > > > 5) we use tabs for indentation in arm/run, and only bother with the > > variable's {}, if necessary >=20 > My shell quoting was rusty. I think $(host) was calling the host comm= and > for some reason. Yes, $(cmd) executes cmd. ${var} is correct, but only necessary if you'= re substituting a substring. For example X=3DFOO echo ${X}_BAR will echo FOO_BAR, but echo $X_BAR will echo whatever the variable X_BAR is. It's not necessary to use the {} in most cases though, space and some other characters, like /, automatically end the variable name. >=20 > > > > 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...). >=20 > /me ponders if he can just config git for that. You can. Add [format] subjectprefix =3D kvm-unit-tests PATCH to your kvm-unit-tests/.git/config. I just hadn't bothered until now... >=20 > I'll patch the readme ;-) Contributing code !AND! updating the readme! Double YAY! Thanks, drew