On Thu, Jul 02, 2015 at 02:17:18PM +0100, Alex Bennée wrote:
>
> Andrew Jones <drjones@xxxxxxxxxx> 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@xxxxxxxxxx>
> >> ---
> >> 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
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
>
> My shell quoting was rusty. I think $(host) was calling the host command
> for some reason.
Yes, $(cmd) executes cmd. ${var} is correct, but only necessary if you're
substituting a substring. For example
X=FOO
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.
>
> >
> > 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.
You can. Add
[format]
subjectprefix = kvm-unit-tests PATCH
to your kvm-unit-tests/.git/config. I just hadn't bothered until now...
>
> I'll patch the readme ;-)
Contributing code !AND! updating the readme! Double YAY!
Thanks,
drew
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
