[kvm-unit-tests PATCH 0/7] A number of small arm/run fixes

[kvm-unit-tests PATCH 0/7] A number of small arm/run fixes

[Alex Bennée]

Following on from yesterdays comments I've re-spun the changes as
suggested. I've also added a very simple opt parsing for arm/run to
allow us to --force-tcg and --debug "more qemu cmdline".

As a bonus I've codified some notes for contributing into the README.

Alex Bennée (7):
  READ: add some CONTRIBUTING notes
  configure: emit HOST=$host to config.mak
  arm/run: set indentation defaults for emacs
  run/arm: introduce usingkvm var and use it
  arm/run: clean-up setting of accel options
  arm/run: introduce basic option parsing
  arm/run: add --debug option

 README    | 22 ++++++++++++++++++++++
 arm/run   | 46 +++++++++++++++++++++++++++++++++++++++++++---
 configure |  2 ++
 3 files changed, 67 insertions(+), 3 deletions(-)

-- 
2.4.5

[kvm-unit-tests PATCH 1/7] READ: add some CONTRIBUTING notes

[Alex Bennée]

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 README | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/README b/README
index e9869d1..4001456 100644
--- a/README
+++ b/README
@@ -25,3 +25,25 @@ Directory structure:
 ./<ARCH>:	the sources of the tests and the created objects/images
 
 See <ARCH>/README for architecture specific documentation.
+
+CONTRIBUTING:
+=============
+
+Style
+-----
+
+  - C: please use standard linux-with-tabs
+  - Shell: use TABs for indentation
+
+Patches
+-------
+
+Patches are welcome at the KVM mailing list <kvm@vger.kernel.org>.
+
+Please prefix messages with: [kvm-unit-tests PATCH]
+
+You can add the following to .git/config to do this automatically for you:
+
+[format]
+	subjectprefix = kvm-unit-tests PATCH
+
-- 
2.4.5

Re: [kvm-unit-tests PATCH 1/7] READ: add some CONTRIBUTING notes

[Andrew Jones]

$SUBJECT is missing the 'ME' in README, but no biggy...

On Fri, Jul 03, 2015 at 02:48:41PM +0100, Alex Bennée wrote:
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  README | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/README b/README
> index e9869d1..4001456 100644
> --- a/README
> +++ b/README
> @@ -25,3 +25,25 @@ Directory structure:
>  ./<ARCH>:	the sources of the tests and the created objects/images
>  
>  See <ARCH>/README for architecture specific documentation.
> +
> +CONTRIBUTING:
> +=============
> +
> +Style
> +-----
> +
> +  - C: please use standard linux-with-tabs
> +  - Shell: use TABs for indentation

This is (unfortunately) not globally true. We have plenty of
lib/ and x86/ code that don't follow this style. Maybe we should
bite the bullet and fix those someday. Or, add a line to this README
that says, 'use the style of the file for consistency'.

> +
> +Patches
> +-------
> +
> +Patches are welcome at the KVM mailing list <kvm@vger.kernel.org>.
> +
> +Please prefix messages with: [kvm-unit-tests PATCH]
> +
> +You can add the following to .git/config to do this automatically for you:
> +
> +[format]
> +	subjectprefix = kvm-unit-tests PATCH
> +

This is good to document.

Thanks,
drew

[kvm-unit-tests PATCH 2/7] configure: emit HOST=$host to config.mak

[Alex Bennée]

This is useful information for the run scripts to know, especially if
they want to drop to using TCG.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 configure | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure b/configure
index b2ad32a..078b70c 100755
--- a/configure
+++ b/configure
@@ -7,6 +7,7 @@ ld=ld
 objcopy=objcopy
 ar=ar
 arch=`uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/'`
+host=$arch
 cross_prefix=
 
 usage() {
@@ -122,6 +123,7 @@ ln -s $asm lib/asm
 cat <<EOF > config.mak
 PREFIX=$prefix
 KERNELDIR=$(readlink -f $kerneldir)
+HOST=$host
 ARCH=$arch
 ARCH_NAME=$arch_name
 PROCESSOR=$processor
-- 
2.4.5

Re: [kvm-unit-tests PATCH 2/7] configure: emit HOST=$host to config.mak

[Andrew Jones]

On Fri, Jul 03, 2015 at 02:48:42PM +0100, Alex Bennée wrote:
> This is useful information for the run scripts to know, especially if
> they want to drop to using TCG.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  configure | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/configure b/configure
> index b2ad32a..078b70c 100755
> --- a/configure
> +++ b/configure
> @@ -7,6 +7,7 @@ ld=ld
>  objcopy=objcopy
>  ar=ar
>  arch=`uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/'`
> +host=$arch
>  cross_prefix=
>  
>  usage() {
> @@ -122,6 +123,7 @@ ln -s $asm lib/asm
>  cat <<EOF > config.mak
>  PREFIX=$prefix
>  KERNELDIR=$(readlink -f $kerneldir)
> +HOST=$host
>  ARCH=$arch
>  ARCH_NAME=$arch_name
>  PROCESSOR=$processor
> -- 
> 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

Looks good to me.

[kvm-unit-tests PATCH 3/7] arm/run: set indentation defaults for emacs

[Alex Bennée]

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 arm/run | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arm/run b/arm/run
index 662a856..6b42a2e 100755
--- a/arm/run
+++ b/arm/run
@@ -1,4 +1,5 @@
 #!/bin/bash
+# -*- sh-basic-offset:8 indent-tabs-mode: t -*-
 
 if [ ! -f config.mak ]; then
 	echo run ./configure first. See ./configure -h
-- 
2.4.5

Re: [kvm-unit-tests PATCH 3/7] arm/run: set indentation defaults for emacs

[Andrew Jones]

On Fri, Jul 03, 2015 at 02:48:43PM +0100, Alex Bennée wrote:
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  arm/run | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arm/run b/arm/run
> index 662a856..6b42a2e 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -1,4 +1,5 @@
>  #!/bin/bash
> +# -*- sh-basic-offset:8 indent-tabs-mode: t -*-

This is kinda ugly, and I don't think we'll be editing this file
enough to warrant it. Just let your left-most fingers on your
left hand get some stretching in by reaching for that tab key :-)

>  
>  if [ ! -f config.mak ]; then
>  	echo run ./configure first. See ./configure -h
> -- 
> 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

[kvm-unit-tests PATCH 4/7] run/arm: introduce usingkvm var and use it

[Alex Bennée]

This makes it easier to force KVM off in later patches.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 arm/run | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arm/run b/arm/run
index 6b42a2e..493ce0d 100755
--- a/arm/run
+++ b/arm/run
@@ -8,6 +8,16 @@ fi
 source config.mak
 processor="$PROCESSOR"
 
+# Default to using KVM if available and on the right ARM host
+usingkvm=0
+if [ -c /dev/kvm ]; then
+	if [ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]; then
+		usingkvm=1
+	elif [ "$HOST" = "aarch64" ]; then
+		usingkvm=1
+	fi
+fi
+
 qemu="${QEMU:-qemu-system-$ARCH_NAME}"
 qpath=$(which $qemu 2>/dev/null)
 
@@ -39,7 +49,7 @@ chr_testdev='-device virtio-serial-device'
 chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd'
 
 # arm64 must use '-cpu host' with kvm
-if [ "$(arch)" = "aarch64" ] && [ "$ARCH" = "arm64" ] && [ -c /dev/kvm ]; then
+if [ $usingkvm = 1 ] && [ "$ARCH" = "arm64" ]; then
 	processor="host"
 fi
 
-- 
2.4.5

Re: [kvm-unit-tests PATCH 4/7] run/arm: introduce usingkvm var and use it

[Andrew Jones]

On Fri, Jul 03, 2015 at 02:48:44PM +0100, Alex Bennée wrote:
> This makes it easier to force KVM off in later patches.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  arm/run | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arm/run b/arm/run
> index 6b42a2e..493ce0d 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -8,6 +8,16 @@ fi
>  source config.mak
>  processor="$PROCESSOR"
>  
> +# Default to using KVM if available and on the right ARM host
> +usingkvm=0
> +if [ -c /dev/kvm ]; then
> +	if [ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]; then
> +		usingkvm=1
> +	elif [ "$HOST" = "aarch64" ]; then
> +		usingkvm=1
> +	fi
> +fi
> +
>  qemu="${QEMU:-qemu-system-$ARCH_NAME}"
>  qpath=$(which $qemu 2>/dev/null)
>  
> @@ -39,7 +49,7 @@ chr_testdev='-device virtio-serial-device'
>  chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd'
>  
>  # arm64 must use '-cpu host' with kvm
> -if [ "$(arch)" = "aarch64" ] && [ "$ARCH" = "arm64" ] && [ -c /dev/kvm ]; then
> +if [ $usingkvm = 1 ] && [ "$ARCH" = "arm64" ]; then
>  	processor="host"
>  fi

Looks good.

>  
> -- 
> 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

[kvm-unit-tests PATCH 5/7] arm/run: clean-up setting of accel options

[Alex Bennée]

It would be nice to use --no-kvm but that flags a warning on pure-TCG
builds. We echo the fact we are using TCG for the benefit of interactive
use.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 arm/run | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arm/run b/arm/run
index 493ce0d..a3a33b3 100755
--- a/arm/run
+++ b/arm/run
@@ -44,7 +44,14 @@ if $qemu $M -chardev testdev,id=id -initrd . 2>&1 \
 	exit 2
 fi
 
-M='-machine virt,accel=kvm:tcg'
+M='-machine virt'
+if [ $usingkvm = 1 ]; then
+	M+=",accel=kvm"
+else
+	echo "Running with TCG"
+	M+=',accel=tcg'
+fi
+
 chr_testdev='-device virtio-serial-device'
 chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd'
 
-- 
2.4.5

Re: [kvm-unit-tests PATCH 5/7] arm/run: clean-up setting of accel options

[Andrew Jones]

On Fri, Jul 03, 2015 at 02:48:45PM +0100, Alex Bennée wrote:
> It would be nice to use --no-kvm but that flags a warning on pure-TCG
> builds. We echo the fact we are using TCG for the benefit of interactive
> use.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  arm/run | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arm/run b/arm/run
> index 493ce0d..a3a33b3 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -44,7 +44,14 @@ if $qemu $M -chardev testdev,id=id -initrd . 2>&1 \
>  	exit 2
>  fi
>  
> -M='-machine virt,accel=kvm:tcg'
> +M='-machine virt'

This M='-machine virt' is now redundant with one about 10 lines above.

> +if [ $usingkvm = 1 ]; then
> +	M+=",accel=kvm"
> +else
> +	echo "Running with TCG"
> +	M+=',accel=tcg'
> +fi

This looks good.
> +
>  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

[kvm-unit-tests PATCH 6/7] arm/run: introduce basic option parsing

[Alex Bennée]

So far this simple option parsing loop allows us to --force-tcg even
when running on ARM hardware.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 arm/run | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arm/run b/arm/run
index a3a33b3..43d7508 100755
--- a/arm/run
+++ b/arm/run
@@ -18,6 +18,23 @@ if [ -c /dev/kvm ]; then
 	fi
 fi
 
+while :; do
+	case $1 in
+		--force-tcg)
+			usingkvm=0
+			shift
+			;;
+		--)
+			# End of all options.
+			shift
+			break
+			;;
+		*)
+			break
+			;;
+	esac
+done
+
 qemu="${QEMU:-qemu-system-$ARCH_NAME}"
 qpath=$(which $qemu 2>/dev/null)
 
-- 
2.4.5

Re: [kvm-unit-tests PATCH 6/7] arm/run: introduce basic option parsing

[Andrew Jones]

On Fri, Jul 03, 2015 at 02:48:46PM +0100, Alex Bennée wrote:
> So far this simple option parsing loop allows us to --force-tcg even
> when running on ARM hardware.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  arm/run | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/arm/run b/arm/run
> index a3a33b3..43d7508 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -18,6 +18,23 @@ if [ -c /dev/kvm ]; then
>  	fi
>  fi
>  
> +while :; do
> +	case $1 in
> +		--force-tcg)
> +			usingkvm=0
> +			shift
> +			;;
> +		--)
> +			# End of all options.
> +			shift
> +			break
> +			;;
> +		*)
> +			break
> +			;;
> +	esac
> +done
> +
>  qemu="${QEMU:-qemu-system-$ARCH_NAME}"
>  qpath=$(which $qemu 2>/dev/null)

I don't think we need this. If we eventually do, then I think it should
be in a common location sharable by all architectures, and also
run_tests.sh should be taught how to pass options through. For this use
though, Paolo's suggestion should be good enough. That is to do

arm/run arm/some-test.flat -machine accel=tcg

IIUC, since this -machine option will come later on the command line
than the one in arm/run, then it will override whatever we've already
done there. It doesn't look like we need to specify virt again though,
so it must add to what we have, and only override the earlier specified
option with what we specify later. The qemu command line is a
complicated animal...

Thanks,
drew

[kvm-unit-tests PATCH 7/7] arm/run: add --debug option

[Alex Bennée]

This allows you to pass debug options through to the QEMU command line.
e.g.:

  ./arm/run --debug "-name debug-threads=on" -- arm/vos-spinlock-test.flat -smp 4

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 arm/run | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arm/run b/arm/run
index 43d7508..871e35e 100755
--- a/arm/run
+++ b/arm/run
@@ -18,10 +18,15 @@ if [ -c /dev/kvm ]; then
 	fi
 fi
 
+debugopts=""
 while :; do
 	case $1 in
 		--force-tcg)
 			usingkvm=0
+			;;
+		--debug)
+			shift
+			debugopts=$1
 			shift
 			;;
 		--)
@@ -78,7 +83,7 @@ if [ $usingkvm = 1 ] && [ "$ARCH" = "arm64" ]; then
 fi
 
 command="$qemu $M -cpu $processor $chr_testdev"
-command+=" -display none -serial stdio -kernel"
+command+=" -display none -serial stdio $debugopts -kernel"
 
 echo $command "$@"
 $command "$@"
-- 
2.4.5

Re: [kvm-unit-tests PATCH 7/7] arm/run: add --debug option

[Andrew Jones]

On Fri, Jul 03, 2015 at 02:48:47PM +0100, Alex Bennée wrote:
> This allows you to pass debug options through to the QEMU command line.
> e.g.:
> 
>   ./arm/run --debug "-name debug-threads=on" -- arm/vos-spinlock-test.flat -smp 4

doesn't
./arm/run arm/vos-spinlock-test.flat -smp 4 -name debug-threads=on
do the same thing?

> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  arm/run | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arm/run b/arm/run
> index 43d7508..871e35e 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -18,10 +18,15 @@ if [ -c /dev/kvm ]; then
>  	fi
>  fi
>  
> +debugopts=""
>  while :; do
>  	case $1 in
>  		--force-tcg)
>  			usingkvm=0
> +			;;
> +		--debug)
> +			shift
> +			debugopts=$1
>  			shift
>  			;;
>  		--)
> @@ -78,7 +83,7 @@ if [ $usingkvm = 1 ] && [ "$ARCH" = "arm64" ]; then
>  fi
>  
>  command="$qemu $M -cpu $processor $chr_testdev"
> -command+=" -display none -serial stdio -kernel"
> +command+=" -display none -serial stdio $debugopts -kernel"
>  
>  echo $command "$@"
>  $command "$@"
> -- 
> 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

Re: [kvm-unit-tests PATCH 7/7] arm/run: add --debug option

[Alex Bennée]

Andrew Jones <drjones@redhat.com> writes:

> On Fri, Jul 03, 2015 at 02:48:47PM +0100, Alex Bennée wrote:
>> This allows you to pass debug options through to the QEMU command line.
>> e.g.:
>> 
>>   ./arm/run --debug "-name debug-threads=on" -- arm/vos-spinlock-test.flat -smp 4
>
> doesn't
> ./arm/run arm/vos-spinlock-test.flat -smp 4 -name debug-threads=on
> do the same thing?

You are right of course, I forgot you needed an explict -append for the
args to the test anyway.

>
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  arm/run | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arm/run b/arm/run
>> index 43d7508..871e35e 100755
>> --- a/arm/run
>> +++ b/arm/run
>> @@ -18,10 +18,15 @@ if [ -c /dev/kvm ]; then
>>  	fi
>>  fi
>>  
>> +debugopts=""
>>  while :; do
>>  	case $1 in
>>  		--force-tcg)
>>  			usingkvm=0
>> +			;;
>> +		--debug)
>> +			shift
>> +			debugopts=$1
>>  			shift
>>  			;;
>>  		--)
>> @@ -78,7 +83,7 @@ if [ $usingkvm = 1 ] && [ "$ARCH" = "arm64" ]; then
>>  fi
>>  
>>  command="$qemu $M -cpu $processor $chr_testdev"
>> -command+=" -display none -serial stdio -kernel"
>> +command+=" -display none -serial stdio $debugopts -kernel"
>>  
>>  echo $command "$@"
>>  $command "$@"
>> -- 
>> 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