[kvm-unit-tests PATCH 0/2] powerpc: Test SPR persistency during migration

[kvm-unit-tests PATCH 0/2] powerpc: Test SPR persistency during migration

[Thomas Huth]

Here's a KVM-unit-test to check the persistency of SPRs during migration.
First patch adds a helper script for doing migration tests in the
kvm-unit-test framework, and the second patch contains the SPR tester.

(Note: I've already posted the SPR tester once many months ago, but I did
not have the migration helper script ready at that point in time yet, so
I refrained from calling this a v2)

Thomas Huth (2):
  Add the possibility to do simple migration tests
  powerpc: Add Special Purpose Register persistency test

 powerpc/Makefile.common          |   3 +-
 powerpc/cstart64.S               |   2 +
 powerpc/run                      |   5 +
 powerpc/sprs.c                   | 303 +++++++++++++++++++++++++++++++++++++++
 powerpc/unittests.cfg            |   5 +
 scripts/qemu-migration-helper.sh |  51 +++++++
 scripts/runtime.bash             |   3 +
 7 files changed, 371 insertions(+), 1 deletion(-)
 create mode 100644 powerpc/sprs.c
 create mode 100755 scripts/qemu-migration-helper.sh

-- 
1.8.3.1

[kvm-unit-tests PATCH 1/2] Add the possibility to do simple migration tests

[Thomas Huth]

To be able to do simple migration tests with kvm-unit-tests, too,
add a helper script that does all the necessary work: Start two
instances of QEMU (source and destination) with QMP sockets for
sending commands to them, then trigger the migration from one
instance to the other and finally signal the end of the migration
to the guest by injecting an NMI.
This helper script is now used automatically for powerpc tests
if the test is put into the "migration" group in the unittests.cfg
file.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 powerpc/run                      |  5 ++++
 scripts/qemu-migration-helper.sh | 51 ++++++++++++++++++++++++++++++++++++++++
 scripts/runtime.bash             |  3 +++
 3 files changed, 59 insertions(+)
 create mode 100755 scripts/qemu-migration-helper.sh

diff --git a/powerpc/run b/powerpc/run
index 6269abb..126fed5 100755
--- a/powerpc/run
+++ b/powerpc/run
@@ -41,6 +41,11 @@ if ! $qemu -machine '?' 2>&1 | grep 'pseries' > /dev/null; then
 	exit 2
 fi
 
+if [ "$MIGRATION" = "1" ]; then
+	export QEMU_BIN="$qemu"
+	qemu=`dirname $0`/../scripts/qemu-migration-helper.sh
+fi
+
 M='-machine pseries'
 M+=",accel=$ACCEL"
 command="$qemu -nodefaults $M -bios $FIRMWARE"
diff --git a/scripts/qemu-migration-helper.sh b/scripts/qemu-migration-helper.sh
new file mode 100755
index 0000000..0cb7e4a
--- /dev/null
+++ b/scripts/qemu-migration-helper.sh
@@ -0,0 +1,51 @@
+#!/bin/sh
+
+# This script runs two instances of QEMU and then migrates the guest from one
+# instance to the other. The end of the migration is signalled to the guest by
+# injecting an NMI.
+
+if [ "x$QEMU_BIN" = "x" ]; then
+    echo "QEMU_BIN must be set to the QEMU binary"
+    exit 1
+fi
+
+if ! command -v nc >/dev/null 2>&1; then
+    echo "$0 needs nc (netcat)"
+    exit 1
+fi
+
+qmp_cmd()
+{
+    echo '{ "execute": "qmp_capabilities" }{ "execute":' "$2" '}' | nc -U $1
+}
+
+tempname=`mktemp`
+qmp1=${tempname}.qmp1
+qmp2=${tempname}.qmp2
+qmpout1=${tempname}.qmpout1
+qmpout2=${tempname}.qmpout2
+stdout1=${tempname}.stdout1
+stdout2=${tempname}.stdout2
+migsock=${tempname}.mig
+
+$QEMU_BIN $* -chardev socket,id=mon1,path=${qmp1},server,nowait \
+	-mon chardev=mon1,mode=control > ${stdout1} &
+
+$QEMU_BIN $* -chardev socket,id=mon2,path=${qmp2},server,nowait \
+	-mon chardev=mon2,mode=control -incoming unix:${migsock} > ${stdout2} &
+
+sleep 1
+
+qmp_cmd ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > ${qmpout1}
+while qmp_cmd ${qmp1} '"query-migrate"' | grep -q '"active"' ; do
+    sleep 1
+done
+qmp_cmd ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
+
+qmp_cmd ${qmp2} '"inject-nmi"'> ${qmpout2}
+
+wait
+
+cat ${stdout1} ${stdout2}
+
+rm -f ${tempname} ${qmpout1} ${qmpout2} ${stdout1} ${stdout2} ${migsock}
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 9c1bc3b..b383816 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -98,6 +98,9 @@ function run()
     }
 
     cmdline=$(get_cmdline $kernel)
+    if grep -qw "migration" <<<$groups ; then
+        cmdline="MIGRATION=1 $cmdline"
+    fi
     if [ "$verbose" = "yes" ]; then
         echo $cmdline
     fi
-- 
1.8.3.1

Re: [kvm-unit-tests PATCH 1/2] Add the possibility to do simple migration tests

[Cédric Le Goater]

On 03/09/2017 06:27 PM, Thomas Huth wrote:
> To be able to do simple migration tests with kvm-unit-tests, too,
> add a helper script that does all the necessary work: Start two
> instances of QEMU (source and destination) with QMP sockets for
> sending commands to them, then trigger the migration from one
> instance to the other and finally signal the end of the migration
> to the guest by injecting an NMI.
> This helper script is now used automatically for powerpc tests
> if the test is put into the "migration" group in the unittests.cfg
> file.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  powerpc/run                      |  5 ++++
>  scripts/qemu-migration-helper.sh | 51 ++++++++++++++++++++++++++++++++++++++++
>  scripts/runtime.bash             |  3 +++
>  3 files changed, 59 insertions(+)
>  create mode 100755 scripts/qemu-migration-helper.sh
> 
> diff --git a/powerpc/run b/powerpc/run
> index 6269abb..126fed5 100755
> --- a/powerpc/run
> +++ b/powerpc/run
> @@ -41,6 +41,11 @@ if ! $qemu -machine '?' 2>&1 | grep 'pseries' > /dev/null; then
>  	exit 2
>  fi
>  
> +if [ "$MIGRATION" = "1" ]; then
> +	export QEMU_BIN="$qemu"
> +	qemu=`dirname $0`/../scripts/qemu-migration-helper.sh
> +fi
> +
>  M='-machine pseries'
>  M+=",accel=$ACCEL"
>  command="$qemu -nodefaults $M -bios $FIRMWARE"
> diff --git a/scripts/qemu-migration-helper.sh b/scripts/qemu-migration-helper.sh
> new file mode 100755
> index 0000000..0cb7e4a
> --- /dev/null
> +++ b/scripts/qemu-migration-helper.sh
> @@ -0,0 +1,51 @@
> +#!/bin/sh
> +
> +# This script runs two instances of QEMU and then migrates the guest from one
> +# instance to the other. The end of the migration is signalled to the guest by
> +# injecting an NMI.
> +
> +if [ "x$QEMU_BIN" = "x" ]; then
> +    echo "QEMU_BIN must be set to the QEMU binary"
> +    exit 1
> +fi
> +
> +if ! command -v nc >/dev/null 2>&1; then
> +    echo "$0 needs nc (netcat)"
> +    exit 1
> +fi
> +
> +qmp_cmd()
> +{
> +    echo '{ "execute": "qmp_capabilities" }{ "execute":' "$2" '}' | nc -U $1
> +}
> +
> +tempname=`mktemp`
> +qmp1=${tempname}.qmp1
> +qmp2=${tempname}.qmp2
> +qmpout1=${tempname}.qmpout1
> +qmpout2=${tempname}.qmpout2
> +stdout1=${tempname}.stdout1
> +stdout2=${tempname}.stdout2
> +migsock=${tempname}.mig
> +
> +$QEMU_BIN $* -chardev socket,id=mon1,path=${qmp1},server,nowait \
> +	-mon chardev=mon1,mode=control > ${stdout1} &
> +
> +$QEMU_BIN $* -chardev socket,id=mon2,path=${qmp2},server,nowait \
> +	-mon chardev=mon2,mode=control -incoming unix:${migsock} > ${stdout2} &
> +
> +sleep 1
> +
> +qmp_cmd ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > ${qmpout1}
> +while qmp_cmd ${qmp1} '"query-migrate"' | grep -q '"active"' ; do
> +    sleep 1
> +done

I had to add an extra sleep here, else I was getting : 

    qemu-system-ppc64: Not a migration stream
    qemu-system-ppc64: load of migration failed: Invalid argument

May be the synchronization on completion of the migration needs 
to be improved.

C.

> +qmp_cmd ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
> +
> +qmp_cmd ${qmp2} '"inject-nmi"'> ${qmpout2}
> +
> +wait
> +
> +cat ${stdout1} ${stdout2}
> +
> +rm -f ${tempname} ${qmpout1} ${qmpout2} ${stdout1} ${stdout2} ${migsock}
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 9c1bc3b..b383816 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -98,6 +98,9 @@ function run()
>      }
>  
>      cmdline=$(get_cmdline $kernel)
> +    if grep -qw "migration" <<<$groups ; then
> +        cmdline="MIGRATION=1 $cmdline"
> +    fi
>      if [ "$verbose" = "yes" ]; then
>          echo $cmdline
>      fi
> 

Re: [kvm-unit-tests PATCH 1/2] Add the possibility to do simple migration tests

[Thomas Huth]

On 10.03.2017 09:30, Cédric Le Goater wrote:
> On 03/09/2017 06:27 PM, Thomas Huth wrote:
>> To be able to do simple migration tests with kvm-unit-tests, too,
>> add a helper script that does all the necessary work: Start two
>> instances of QEMU (source and destination) with QMP sockets for
>> sending commands to them, then trigger the migration from one
>> instance to the other and finally signal the end of the migration
>> to the guest by injecting an NMI.
>> This helper script is now used automatically for powerpc tests
>> if the test is put into the "migration" group in the unittests.cfg
>> file.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  powerpc/run                      |  5 ++++
>>  scripts/qemu-migration-helper.sh | 51 ++++++++++++++++++++++++++++++++++++++++
>>  scripts/runtime.bash             |  3 +++
>>  3 files changed, 59 insertions(+)
>>  create mode 100755 scripts/qemu-migration-helper.sh
>>
>> diff --git a/powerpc/run b/powerpc/run
>> index 6269abb..126fed5 100755
>> --- a/powerpc/run
>> +++ b/powerpc/run
>> @@ -41,6 +41,11 @@ if ! $qemu -machine '?' 2>&1 | grep 'pseries' > /dev/null; then
>>  	exit 2
>>  fi
>>  
>> +if [ "$MIGRATION" = "1" ]; then
>> +	export QEMU_BIN="$qemu"
>> +	qemu=`dirname $0`/../scripts/qemu-migration-helper.sh
>> +fi
>> +
>>  M='-machine pseries'
>>  M+=",accel=$ACCEL"
>>  command="$qemu -nodefaults $M -bios $FIRMWARE"
>> diff --git a/scripts/qemu-migration-helper.sh b/scripts/qemu-migration-helper.sh
>> new file mode 100755
>> index 0000000..0cb7e4a
>> --- /dev/null
>> +++ b/scripts/qemu-migration-helper.sh
>> @@ -0,0 +1,51 @@
>> +#!/bin/sh
>> +
>> +# This script runs two instances of QEMU and then migrates the guest from one
>> +# instance to the other. The end of the migration is signalled to the guest by
>> +# injecting an NMI.
>> +
>> +if [ "x$QEMU_BIN" = "x" ]; then
>> +    echo "QEMU_BIN must be set to the QEMU binary"
>> +    exit 1
>> +fi
>> +
>> +if ! command -v nc >/dev/null 2>&1; then
>> +    echo "$0 needs nc (netcat)"
>> +    exit 1
>> +fi
>> +
>> +qmp_cmd()
>> +{
>> +    echo '{ "execute": "qmp_capabilities" }{ "execute":' "$2" '}' | nc -U $1
>> +}
>> +
>> +tempname=`mktemp`
>> +qmp1=${tempname}.qmp1
>> +qmp2=${tempname}.qmp2
>> +qmpout1=${tempname}.qmpout1
>> +qmpout2=${tempname}.qmpout2
>> +stdout1=${tempname}.stdout1
>> +stdout2=${tempname}.stdout2
>> +migsock=${tempname}.mig
>> +
>> +$QEMU_BIN $* -chardev socket,id=mon1,path=${qmp1},server,nowait \
>> +	-mon chardev=mon1,mode=control > ${stdout1} &
>> +
>> +$QEMU_BIN $* -chardev socket,id=mon2,path=${qmp2},server,nowait \
>> +	-mon chardev=mon2,mode=control -incoming unix:${migsock} > ${stdout2} &
>> +
>> +sleep 1
>> +
>> +qmp_cmd ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > ${qmpout1}
>> +while qmp_cmd ${qmp1} '"query-migrate"' | grep -q '"active"' ; do
>> +    sleep 1
>> +done
> 
> I had to add an extra sleep here, else I was getting : 
> 
>     qemu-system-ppc64: Not a migration stream
>     qemu-system-ppc64: load of migration failed: Invalid argument
> 
> May be the synchronization on completion of the migration needs 
> to be improved.

Right, there seems to be another state called "setup" which could be
reported by query-migrate. So you likely got that state here and the
while loop terminated too early. I'll fix that and send a new version
(after waiting for some more review feedback)...

Thanks for your testing!

 Thomas

Re: [kvm-unit-tests PATCH 1/2] Add the possibility to do simple migration tests

[Andrew Jones]

On Thu, Mar 09, 2017 at 06:27:06PM +0100, Thomas Huth wrote:
> To be able to do simple migration tests with kvm-unit-tests, too,
> add a helper script that does all the necessary work: Start two
> instances of QEMU (source and destination) with QMP sockets for
> sending commands to them, then trigger the migration from one
> instance to the other and finally signal the end of the migration
> to the guest by injecting an NMI.
> This helper script is now used automatically for powerpc tests
> if the test is put into the "migration" group in the unittests.cfg
> file.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  powerpc/run                      |  5 ++++
>  scripts/qemu-migration-helper.sh | 51 ++++++++++++++++++++++++++++++++++++++++
>  scripts/runtime.bash             |  3 +++
>  3 files changed, 59 insertions(+)
>  create mode 100755 scripts/qemu-migration-helper.sh
> 
> diff --git a/powerpc/run b/powerpc/run
> index 6269abb..126fed5 100755
> --- a/powerpc/run
> +++ b/powerpc/run
> @@ -41,6 +41,11 @@ if ! $qemu -machine '?' 2>&1 | grep 'pseries' > /dev/null; then
>  	exit 2
>  fi
>  
> +if [ "$MIGRATION" = "1" ]; then

I'd prefer we use "yes" like our other bash booleans.

> +	export QEMU_BIN="$qemu"
> +	qemu=`dirname $0`/../scripts/qemu-migration-helper.sh

Can't we just use a relative pathname, i.e. no need for dirname?

> +fi
> +
>  M='-machine pseries'
>  M+=",accel=$ACCEL"
>  command="$qemu -nodefaults $M -bios $FIRMWARE"
> diff --git a/scripts/qemu-migration-helper.sh b/scripts/qemu-migration-helper.sh
> new file mode 100755
> index 0000000..0cb7e4a
> --- /dev/null
> +++ b/scripts/qemu-migration-helper.sh
> @@ -0,0 +1,51 @@
> +#!/bin/sh

Might as well use bash, as we require it for all the other scripts.

> +

nit: prefer no blank line between the interpreter and comment block

> +# This script runs two instances of QEMU and then migrates the guest from one
> +# instance to the other. The end of the migration is signalled to the guest by
> +# injecting an NMI.
> +
> +if [ "x$QEMU_BIN" = "x" ]; then

[ -z "$QEMU" ] ? (I don't think we need the ugly x-stuff with bash)

We use $QEMU everywhere else, any reason to avoid it here (using
QEMU_BIN)?

> +    echo "QEMU_BIN must be set to the QEMU binary"
> +    exit 1
> +fi
> +
> +if ! command -v nc >/dev/null 2>&1; then

'command' is nice (I wasn't aware of it when I used 'which' elsewhere)

> +    echo "$0 needs nc (netcat)"
> +    exit 1
> +fi
> +
> +qmp_cmd()
> +{
> +    echo '{ "execute": "qmp_capabilities" }{ "execute":' "$2" '}' | nc -U $1
> +}
> +
> +tempname=`mktemp`
> +qmp1=${tempname}.qmp1
> +qmp2=${tempname}.qmp2
> +qmpout1=${tempname}.qmpout1
> +qmpout2=${tempname}.qmpout2
> +stdout1=${tempname}.stdout1
> +stdout2=${tempname}.stdout2
> +migsock=${tempname}.mig

I think all the above should invoke mktemp themselves, otherwise this
script is a bit unsafe. If the suffixes are useful for debug purposes
then you can use mktemp --suffix.

> +
> +$QEMU_BIN $* -chardev socket,id=mon1,path=${qmp1},server,nowait \
> +	-mon chardev=mon1,mode=control > ${stdout1} &
> +
> +$QEMU_BIN $* -chardev socket,id=mon2,path=${qmp2},server,nowait \
> +	-mon chardev=mon2,mode=control -incoming unix:${migsock} > ${stdout2} &
> +
> +sleep 1
> +
> +qmp_cmd ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > ${qmpout1}
> +while qmp_cmd ${qmp1} '"query-migrate"' | grep -q '"active"' ; do
> +    sleep 1

nit: should use tab like above

> +done
> +qmp_cmd ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
> +
> +qmp_cmd ${qmp2} '"inject-nmi"'> ${qmpout2}
> +
> +wait
> +
> +cat ${stdout1} ${stdout2}
> +
> +rm -f ${tempname} ${qmpout1} ${qmpout2} ${stdout1} ${stdout2} ${migsock}

How about doing this in an EXIT trap handler to make sure the files get
removed?

> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 9c1bc3b..b383816 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -98,6 +98,9 @@ function run()
>      }
>  
>      cmdline=$(get_cmdline $kernel)
> +    if grep -qw "migration" <<<$groups ; then
> +        cmdline="MIGRATION=1 $cmdline"
> +    fi
>      if [ "$verbose" = "yes" ]; then
>          echo $cmdline
>      fi
> -- 
> 1.8.3.1
>

Thanks,
drew

Re: [kvm-unit-tests PATCH 1/2] Add the possibility to do simple migration tests

[Thomas Huth]

On 10.03.2017 16:04, Andrew Jones wrote:
> On Thu, Mar 09, 2017 at 06:27:06PM +0100, Thomas Huth wrote:
>> To be able to do simple migration tests with kvm-unit-tests, too,
>> add a helper script that does all the necessary work: Start two
>> instances of QEMU (source and destination) with QMP sockets for
>> sending commands to them, then trigger the migration from one
>> instance to the other and finally signal the end of the migration
>> to the guest by injecting an NMI.
>> This helper script is now used automatically for powerpc tests
>> if the test is put into the "migration" group in the unittests.cfg
>> file.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  powerpc/run                      |  5 ++++
>>  scripts/qemu-migration-helper.sh | 51 ++++++++++++++++++++++++++++++++++++++++
>>  scripts/runtime.bash             |  3 +++
>>  3 files changed, 59 insertions(+)
>>  create mode 100755 scripts/qemu-migration-helper.sh
>>
>> diff --git a/powerpc/run b/powerpc/run
>> index 6269abb..126fed5 100755
>> --- a/powerpc/run
>> +++ b/powerpc/run
>> @@ -41,6 +41,11 @@ if ! $qemu -machine '?' 2>&1 | grep 'pseries' > /dev/null; then
>>  	exit 2
>>  fi
>>  
>> +if [ "$MIGRATION" = "1" ]; then
> 
> I'd prefer we use "yes" like our other bash booleans.

Sure. I'll change it.

>> +	export QEMU_BIN="$qemu"
>> +	qemu=`dirname $0`/../scripts/qemu-migration-helper.sh
> 
> Can't we just use a relative pathname, i.e. no need for dirname?

I thought that dirname trick would be nice to be able to call the "run"
script from another $PWD, too, but apparently the other scripts like
scripts/arch-run.bash are also called there directly. So I'll change to
that style here, too.

>> +fi
>> +
>>  M='-machine pseries'
>>  M+=",accel=$ACCEL"
>>  command="$qemu -nodefaults $M -bios $FIRMWARE"
>> diff --git a/scripts/qemu-migration-helper.sh b/scripts/qemu-migration-helper.sh
>> new file mode 100755
>> index 0000000..0cb7e4a
>> --- /dev/null
>> +++ b/scripts/qemu-migration-helper.sh
>> @@ -0,0 +1,51 @@
>> +#!/bin/sh
> 
> Might as well use bash, as we require it for all the other scripts.

OK.

>> +
> 
> nit: prefer no blank line between the interpreter and comment block
> 
>> +# This script runs two instances of QEMU and then migrates the guest from one
>> +# instance to the other. The end of the migration is signalled to the guest by
>> +# injecting an NMI.
>> +
>> +if [ "x$QEMU_BIN" = "x" ]; then
> 
> [ -z "$QEMU" ] ? (I don't think we need the ugly x-stuff with bash)
> 
> We use $QEMU everywhere else, any reason to avoid it here (using
> QEMU_BIN)?

I did not want to interfere with the user supplied $QEMU in the "run"
script. But thinking about this again, I likely don't need an env
variable here at all and could also simply pass it as the first
parameter to the script. I'll try to do that instead...

>> +    echo "QEMU_BIN must be set to the QEMU binary"
>> +    exit 1
>> +fi
>> +
>> +if ! command -v nc >/dev/null 2>&1; then
> 
> 'command' is nice (I wasn't aware of it when I used 'which' elsewhere)

Yes, I've been told that this is more portable (i.e. POSIX compliant)
than using "which".

>> +    echo "$0 needs nc (netcat)"
>> +    exit 1
>> +fi
>> +
>> +qmp_cmd()
>> +{
>> +    echo '{ "execute": "qmp_capabilities" }{ "execute":' "$2" '}' | nc -U $1
>> +}
>> +
>> +tempname=`mktemp`
>> +qmp1=${tempname}.qmp1
>> +qmp2=${tempname}.qmp2
>> +qmpout1=${tempname}.qmpout1
>> +qmpout2=${tempname}.qmpout2
>> +stdout1=${tempname}.stdout1
>> +stdout2=${tempname}.stdout2
>> +migsock=${tempname}.mig
> 
> I think all the above should invoke mktemp themselves, otherwise this
> script is a bit unsafe. If the suffixes are useful for debug purposes
> then you can use mktemp --suffix.

OK, I'll rework this section.

>> +
>> +$QEMU_BIN $* -chardev socket,id=mon1,path=${qmp1},server,nowait \
>> +	-mon chardev=mon1,mode=control > ${stdout1} &
>> +
>> +$QEMU_BIN $* -chardev socket,id=mon2,path=${qmp2},server,nowait \
>> +	-mon chardev=mon2,mode=control -incoming unix:${migsock} > ${stdout2} &
>> +
>> +sleep 1
>> +
>> +qmp_cmd ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > ${qmpout1}
>> +while qmp_cmd ${qmp1} '"query-migrate"' | grep -q '"active"' ; do
>> +    sleep 1
> 
> nit: should use tab like above

I guess I stared at runtime.bash for too long 8-) ... that one is only
using spaces for indentation.

>> +done
>> +qmp_cmd ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
>> +
>> +qmp_cmd ${qmp2} '"inject-nmi"'> ${qmpout2}
>> +
>> +wait
>> +
>> +cat ${stdout1} ${stdout2}
>> +
>> +rm -f ${tempname} ${qmpout1} ${qmpout2} ${stdout1} ${stdout2} ${migsock}
> 
> How about doing this in an EXIT trap handler to make sure the files get
> removed?

Good idea, I'll add that.

Thanks for the review!

 Thomas

Re: [kvm-unit-tests PATCH 1/2] Add the possibility to do simple migration tests

[David Matlack]

On Thu, Mar 9, 2017 at 9:27 AM, Thomas Huth <thuth@redhat.com> wrote:
> To be able to do simple migration tests with kvm-unit-tests, too,
> add a helper script that does all the necessary work: Start two
> instances of QEMU (source and destination) with QMP sockets for
> sending commands to them, then trigger the migration from one
> instance to the other and finally signal the end of the migration
> to the guest by injecting an NMI.
> This helper script is now used automatically for powerpc tests
> if the test is put into the "migration" group in the unittests.cfg
> file.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  powerpc/run                      |  5 ++++
>  scripts/qemu-migration-helper.sh | 51 ++++++++++++++++++++++++++++++++++++++++
>  scripts/runtime.bash             |  3 +++
>  3 files changed, 59 insertions(+)
>  create mode 100755 scripts/qemu-migration-helper.sh
>
> diff --git a/powerpc/run b/powerpc/run
> index 6269abb..126fed5 100755
> --- a/powerpc/run
> +++ b/powerpc/run
> @@ -41,6 +41,11 @@ if ! $qemu -machine '?' 2>&1 | grep 'pseries' > /dev/null; then
>         exit 2
>  fi
>
> +if [ "$MIGRATION" = "1" ]; then
> +       export QEMU_BIN="$qemu"
> +       qemu=`dirname $0`/../scripts/qemu-migration-helper.sh
> +fi
> +
>  M='-machine pseries'
>  M+=",accel=$ACCEL"
>  command="$qemu -nodefaults $M -bios $FIRMWARE"
> diff --git a/scripts/qemu-migration-helper.sh b/scripts/qemu-migration-helper.sh
> new file mode 100755
> index 0000000..0cb7e4a
> --- /dev/null
> +++ b/scripts/qemu-migration-helper.sh
> @@ -0,0 +1,51 @@
> +#!/bin/sh
> +
> +# This script runs two instances of QEMU and then migrates the guest from one
> +# instance to the other. The end of the migration is signalled to the guest by
> +# injecting an NMI.
> +
> +if [ "x$QEMU_BIN" = "x" ]; then
> +    echo "QEMU_BIN must be set to the QEMU binary"
> +    exit 1
> +fi
> +
> +if ! command -v nc >/dev/null 2>&1; then
> +    echo "$0 needs nc (netcat)"
> +    exit 1
> +fi
> +
> +qmp_cmd()
> +{
> +    echo '{ "execute": "qmp_capabilities" }{ "execute":' "$2" '}' | nc -U $1
> +}
> +
> +tempname=`mktemp`
> +qmp1=${tempname}.qmp1
> +qmp2=${tempname}.qmp2
> +qmpout1=${tempname}.qmpout1
> +qmpout2=${tempname}.qmpout2
> +stdout1=${tempname}.stdout1
> +stdout2=${tempname}.stdout2
> +migsock=${tempname}.mig
> +
> +$QEMU_BIN $* -chardev socket,id=mon1,path=${qmp1},server,nowait \
> +       -mon chardev=mon1,mode=control > ${stdout1} &
> +
> +$QEMU_BIN $* -chardev socket,id=mon2,path=${qmp2},server,nowait \
> +       -mon chardev=mon2,mode=control -incoming unix:${migsock} > ${stdout2} &
> +
> +sleep 1

Suggest monitoring the QEMU serial port for a specific token to know
when to initiate the migration. e.g. the guest can print
"MIGRATE_ME_NOW" when it's ready. This would avoid initiating a
migration too early.

> +
> +qmp_cmd ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > ${qmpout1}
> +while qmp_cmd ${qmp1} '"query-migrate"' | grep -q '"active"' ; do
> +    sleep 1
> +done
> +qmp_cmd ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
> +
> +qmp_cmd ${qmp2} '"inject-nmi"'> ${qmpout2}

Does "inject-nmi" work on other architectures?

An alternative approach is to add a new Qemu testdev register that
counts the number of times a VM has been migrated. The guest can
monitor this to know when the migration is complete.

There might be a way to initiate the migration (or at least a
save/restore cycle) from the testdev as well.

> +
> +wait
> +
> +cat ${stdout1} ${stdout2}
> +
> +rm -f ${tempname} ${qmpout1} ${qmpout2} ${stdout1} ${stdout2} ${migsock}
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 9c1bc3b..b383816 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -98,6 +98,9 @@ function run()
>      }
>
>      cmdline=$(get_cmdline $kernel)
> +    if grep -qw "migration" <<<$groups ; then
> +        cmdline="MIGRATION=1 $cmdline"
> +    fi
>      if [ "$verbose" = "yes" ]; then
>          echo $cmdline
>      fi
> --
> 1.8.3.1
>

Re: [kvm-unit-tests PATCH 1/2] Add the possibility to do simple migration tests

[Thomas Huth]

On 11.03.2017 02:42, David Matlack wrote:
> On Thu, Mar 9, 2017 at 9:27 AM, Thomas Huth <thuth@redhat.com> wrote:
>> To be able to do simple migration tests with kvm-unit-tests, too,
>> add a helper script that does all the necessary work: Start two
>> instances of QEMU (source and destination) with QMP sockets for
>> sending commands to them, then trigger the migration from one
>> instance to the other and finally signal the end of the migration
>> to the guest by injecting an NMI.
>> This helper script is now used automatically for powerpc tests
>> if the test is put into the "migration" group in the unittests.cfg
>> file.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  powerpc/run                      |  5 ++++
>>  scripts/qemu-migration-helper.sh | 51 ++++++++++++++++++++++++++++++++++++++++
>>  scripts/runtime.bash             |  3 +++
>>  3 files changed, 59 insertions(+)
>>  create mode 100755 scripts/qemu-migration-helper.sh
>>
>> diff --git a/powerpc/run b/powerpc/run
>> index 6269abb..126fed5 100755
>> --- a/powerpc/run
>> +++ b/powerpc/run
>> @@ -41,6 +41,11 @@ if ! $qemu -machine '?' 2>&1 | grep 'pseries' > /dev/null; then
>>         exit 2
>>  fi
>>
>> +if [ "$MIGRATION" = "1" ]; then
>> +       export QEMU_BIN="$qemu"
>> +       qemu=`dirname $0`/../scripts/qemu-migration-helper.sh
>> +fi
>> +
>>  M='-machine pseries'
>>  M+=",accel=$ACCEL"
>>  command="$qemu -nodefaults $M -bios $FIRMWARE"
>> diff --git a/scripts/qemu-migration-helper.sh b/scripts/qemu-migration-helper.sh
>> new file mode 100755
>> index 0000000..0cb7e4a
>> --- /dev/null
>> +++ b/scripts/qemu-migration-helper.sh
>> @@ -0,0 +1,51 @@
>> +#!/bin/sh
>> +
>> +# This script runs two instances of QEMU and then migrates the guest from one
>> +# instance to the other. The end of the migration is signalled to the guest by
>> +# injecting an NMI.
>> +
>> +if [ "x$QEMU_BIN" = "x" ]; then
>> +    echo "QEMU_BIN must be set to the QEMU binary"
>> +    exit 1
>> +fi
>> +
>> +if ! command -v nc >/dev/null 2>&1; then
>> +    echo "$0 needs nc (netcat)"
>> +    exit 1
>> +fi
>> +
>> +qmp_cmd()
>> +{
>> +    echo '{ "execute": "qmp_capabilities" }{ "execute":' "$2" '}' | nc -U $1
>> +}
>> +
>> +tempname=`mktemp`
>> +qmp1=${tempname}.qmp1
>> +qmp2=${tempname}.qmp2
>> +qmpout1=${tempname}.qmpout1
>> +qmpout2=${tempname}.qmpout2
>> +stdout1=${tempname}.stdout1
>> +stdout2=${tempname}.stdout2
>> +migsock=${tempname}.mig
>> +
>> +$QEMU_BIN $* -chardev socket,id=mon1,path=${qmp1},server,nowait \
>> +       -mon chardev=mon1,mode=control > ${stdout1} &
>> +
>> +$QEMU_BIN $* -chardev socket,id=mon2,path=${qmp2},server,nowait \
>> +       -mon chardev=mon2,mode=control -incoming unix:${migsock} > ${stdout2} &
>> +
>> +sleep 1
> 
> Suggest monitoring the QEMU serial port for a specific token to know
> when to initiate the migration. e.g. the guest can print
> "MIGRATE_ME_NOW" when it's ready. This would avoid initiating a
> migration too early.

Yes, that's a good idea, I'll add a check for something like this.

>> +
>> +qmp_cmd ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > ${qmpout1}
>> +while qmp_cmd ${qmp1} '"query-migrate"' | grep -q '"active"' ; do
>> +    sleep 1
>> +done
>> +qmp_cmd ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
>> +
>> +qmp_cmd ${qmp2} '"inject-nmi"'> ${qmpout2}
> 
> Does "inject-nmi" work on other architectures?

It should work at least on x86. Not sure about ARM though.

> An alternative approach is to add a new Qemu testdev register that
> counts the number of times a VM has been migrated. The guest can
> monitor this to know when the migration is complete.

AFAIK the testdev only works on x86, so that's not an option for powerpc
here.

 Thomas

[kvm-unit-tests PATCH 2/2] powerpc: Add Special Purpose Register persistency test

[Thomas Huth]

This test has two purposes: First, check whether the hypervisor can be
destabilized by writing random values into the SPRs of the PowerPC CPU
(this indeed revealed a bug last year, see CVE-2016-3044).
Second, this test can be used to check whether the SPRs are synchronized
properly between the KVM host CPU and QEMU, e.g. when migrating the VM
from one QEMU instance to another.
The test first fills the various SPRs with some non-zero value, then reads
the values back into a first array. It then either sleeps a short period
of time (for testing without migration, in the hope that we're rescheduled
on another host CPU), or it waits for a key or NMI (with the '-w' option)
so that it is possible to migrate the VM before continuing. The test then
finally reads the values from the SPRs back into another array and then
compares them with the initial values.
Currently the test only supports the SPRs from the PowerISA v2.01
(PowerPC 970) and PowerISA v2.07 specification (i.e. POWER8 CPUs),
but other versions should be pretty easy to add later.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 powerpc/Makefile.common |   3 +-
 powerpc/cstart64.S      |   2 +
 powerpc/sprs.c          | 303 ++++++++++++++++++++++++++++++++++++++++++++++++
 powerpc/unittests.cfg   |   5 +
 4 files changed, 312 insertions(+), 1 deletion(-)
 create mode 100644 powerpc/sprs.c

diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 37f8caa..92809a5 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -9,7 +9,8 @@ tests-common = \
 	$(TEST_DIR)/spapr_hcall.elf \
 	$(TEST_DIR)/rtas.elf \
 	$(TEST_DIR)/emulator.elf \
-	$(TEST_DIR)/tm.elf
+	$(TEST_DIR)/tm.elf \
+	$(TEST_DIR)/sprs.elf
 
 tests-all = $(tests-common) $(tests)
 all: $(TEST_DIR)/boot_rom.bin $(tests-all)
diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
index 2204e3b..ec673b3 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S
@@ -247,6 +247,8 @@ call_handler:
 	.globl __start_interrupts
 __start_interrupts:
 
+VECTOR(0x100)
+VECTOR(0x200)
 VECTOR(0x300)
 VECTOR(0x400)
 VECTOR(0x500)
diff --git a/powerpc/sprs.c b/powerpc/sprs.c
new file mode 100644
index 0000000..7792db4
--- /dev/null
+++ b/powerpc/sprs.c
@@ -0,0 +1,303 @@
+/*
+ * Test Special Purpose Registers
+ *
+ * Copyright 2017  Thomas Huth, Red Hat Inc.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ *
+ * The basic idea of this test is to check whether the contents of the Special
+ * Purpose Registers (SPRs) are preserved correctly during migration. So we
+ * fill in the SPRs with a well-known value, read the values back (since not
+ * all bits might be retained in the SPRs), then wait for a key or NMI (if the
+ * '-w' option has been specified) so that the user has a chance to migrate the
+ * VM. Alternatively, the test can also simply sleep a little bit with the
+ * H_CEDE hypercall, in the hope that we'll get scheduled to another host CPU
+ * and thus register contents might have changed, too (in case of bugs).
+ * Finally, we read back the values from the SPRs and compare them with the
+ * values before the migration. Mismatches are reported as test failures.
+ * Note that we do not test all SPRs since some of the registers change their
+ * content automatically, and some are only accessible with hypervisor privi-
+ * ledges or have bad side effects, so we have to omit those registers.
+ */
+#include <libcflat.h>
+#include <util.h>
+#include <alloc.h>
+#include <asm/handlers.h>
+#include <asm/hcall.h>
+#include <asm/processor.h>
+
+#define mfspr(nr) ({ \
+	uint64_t ret; \
+	asm volatile("mfspr %0,%1" : "=r"(ret) : "i"(nr)); \
+	ret; \
+})
+
+#define mtspr(nr, val) \
+	asm volatile("mtspr %0,%1" : : "i"(nr), "r"(val))
+
+uint64_t before[1024], after[1024];
+
+volatile int nmi_occurred;
+
+static void nmi_handler(struct pt_regs *regs __unused, void *opaque __unused)
+{
+	nmi_occurred = 1;
+}
+
+static int h_get_term_char(uint64_t termno)
+{
+	register uint64_t r3 asm("r3") = 0x54; /* H_GET_TERM_CHAR */
+	register uint64_t r4 asm("r4") = termno;
+	register uint64_t r5 asm("r5");
+
+	asm volatile (" sc 1 "	: "+r"(r3), "+r"(r4), "=r"(r5)
+				: "r"(r3),  "r"(r4));
+
+	return r3 == H_SUCCESS && r4 > 0 ? r5 >> 48 : 0;
+}
+
+/* Common SPRs for all PowerPC CPUs */
+static void set_sprs_common(uint64_t val)
+{
+	mtspr(9, val);		/* CTR */
+	// mtspr(273, val);	/* SPRG1 */  /* Used by our exception handler */
+	mtspr(274, val);	/* SPRG2 */
+	mtspr(275, val);	/* SPRG3 */
+}
+
+/* SPRs from PowerPC Operating Environment Architecture, Book III, Vers. 2.01 */
+static void set_sprs_book3s_201(uint64_t val)
+{
+	mtspr(18, val);		/* DSISR */
+	mtspr(19, val);		/* DAR */
+	mtspr(152, val);	/* CTRL */
+	mtspr(256, val);	/* VRSAVE */
+	mtspr(786, val);	/* MMCRA */
+	mtspr(795, val);	/* MMCR0 */
+	mtspr(798, val);	/* MMCR1 */
+}
+
+/* SPRs from PowerISA 2.07 Book III-S */
+static void set_sprs_book3s_207(uint64_t val)
+{
+	mtspr(3, val);		/* DSCR */
+	mtspr(13, val);		/* AMR */
+	mtspr(17, val);		/* DSCR */
+	mtspr(18, val);		/* DSISR */
+	mtspr(19, val);		/* DAR */
+	mtspr(29, val);		/* AMR */
+	mtspr(61, val);		/* IAMR */
+	// mtspr(152, val);	/* CTRL */  /* TODO: Needs a fix in KVM */
+	mtspr(153, val);	/* FSCR */
+	mtspr(157, val);	/* UAMOR */
+	mtspr(159, val);	/* PSPB */
+	mtspr(256, val);	/* VRSAVE */
+	// mtspr(272, val);	/* SPRG0 */ /* Used by our exception handler */
+	mtspr(769, val);	/* MMCR2 */
+	mtspr(770, val);	/* MMCRA */
+	mtspr(771, val);	/* PMC1 */
+	mtspr(772, val);	/* PMC2 */
+	mtspr(773, val);	/* PMC3 */
+	mtspr(774, val);	/* PMC4 */
+	mtspr(775, val);	/* PMC5 */
+	mtspr(776, val);	/* PMC6 */
+	mtspr(779, (val & 0xfffffffffbab3fffULL) | 0xfa0b2070);	/* MMCR0 */
+	mtspr(784, val);	/* SIER */
+	mtspr(785, val);	/* MMCR2 */
+	mtspr(786, val);	/* MMCRA */
+	mtspr(787, val);	/* PMC1 */
+	mtspr(788, val);	/* PMC2 */
+	mtspr(789, val);	/* PMC3 */
+	mtspr(790, val);	/* PMC4 */
+	mtspr(791, val);	/* PMC5 */
+	mtspr(792, val);	/* PMC6 */
+	mtspr(795, (val & 0xfffffffffbab3fffULL) | 0xfa0b2070);	/* MMCR0 */
+	mtspr(796, val);	/* SIAR */
+	mtspr(797, val);	/* SDAR */
+	mtspr(798, val);	/* MMCR1 */
+	mtspr(800, val);	/* BESCRS */
+	mtspr(801, val);	/* BESCCRSU */
+	mtspr(802, val);	/* BESCRR */
+	mtspr(803, val);	/* BESCRRU */
+	mtspr(804, val);	/* EBBHR */
+	mtspr(805, val);	/* EBBRR */
+	mtspr(806, val);	/* BESCR */
+	mtspr(815, val);	/* TAR */
+}
+
+static void set_sprs(uint64_t val)
+{
+	uint32_t pvr = mfspr(287);	/* Processor Version Register */
+
+	set_sprs_common(val);
+
+	switch (pvr >> 16) {
+	case 0x39:			/* PPC970 */
+	case 0x3C:			/* PPC970FX */
+	case 0x44:			/* PPC970MP */
+		set_sprs_book3s_201(val);
+		break;
+	case 0x4b:			/* POWER8E */
+	case 0x4c:			/* POWER8NVL */
+	case 0x4d:			/* POWER8 */
+		set_sprs_book3s_207(val);
+		break;
+	default:
+		puts("Warning: Unknown processor version!\n");
+	}
+}
+
+static void get_sprs_common(uint64_t *v)
+{
+	v[9] = mfspr(9);	/* CTR */
+	// v[273] = mfspr(273);	/* SPRG1 */ /* Used by our exception handler */
+	v[274] = mfspr(274);	/* SPRG2 */
+	v[275] = mfspr(275);	/* SPRG3 */
+}
+
+static void get_sprs_book3s_201(uint64_t *v)
+{
+	v[18] = mfspr(18);	/* DSISR */
+	v[19] = mfspr(19);	/* DAR */
+	v[136] = mfspr(136);	/* CTRL */
+	v[256] = mfspr(256);	/* VRSAVE */
+	v[786] = mfspr(786);	/* MMCRA */
+	v[795] = mfspr(795);	/* MMCR0 */
+	v[798] = mfspr(798);	/* MMCR1 */
+}
+
+static void get_sprs_book3s_207(uint64_t *v)
+{
+	v[3] = mfspr(3);	/* DSCR */
+	v[13] = mfspr(13);	/* AMR */
+	v[17] = mfspr(17);	/* DSCR */
+	v[18] = mfspr(18);	/* DSISR */
+	v[19] = mfspr(19);	/* DAR */
+	v[29] = mfspr(29);	/* AMR */
+	v[61] = mfspr(61);	/* IAMR */
+	//v[136] = mfspr(136);	/* CTRL */  /* TODO: Needs a fix in KVM */
+	v[153] = mfspr(153);	/* FSCR */
+	v[157] = mfspr(157);	/* UAMOR */
+	v[159] = mfspr(159);	/* PSPB */
+	v[256] = mfspr(256);	/* VRSAVE */
+	v[259] = mfspr(259);	/* SPRG3 (read only) */
+	// v[272] = mfspr(272);	/* SPRG0 */  /* Used by our exception handler */
+	v[769] = mfspr(769);	/* MMCR2 */
+	v[770] = mfspr(770);	/* MMCRA */
+	v[771] = mfspr(771);	/* PMC1 */
+	v[772] = mfspr(772);	/* PMC2 */
+	v[773] = mfspr(773);	/* PMC3 */
+	v[774] = mfspr(774);	/* PMC4 */
+	v[775] = mfspr(775);	/* PMC5 */
+	v[776] = mfspr(776);	/* PMC6 */
+	v[779] = mfspr(779);	/* MMCR0 */
+	v[780] = mfspr(780);	/* SIAR (read only) */
+	v[781] = mfspr(781);	/* SDAR (read only) */
+	v[782] = mfspr(782);	/* MMCR1 (read only) */
+	v[784] = mfspr(784);	/* SIER */
+	v[785] = mfspr(785);	/* MMCR2 */
+	v[786] = mfspr(786);	/* MMCRA */
+	v[787] = mfspr(787);	/* PMC1 */
+	v[788] = mfspr(788);	/* PMC2 */
+	v[789] = mfspr(789);	/* PMC3 */
+	v[790] = mfspr(790);	/* PMC4 */
+	v[791] = mfspr(791);	/* PMC5 */
+	v[792] = mfspr(792);	/* PMC6 */
+	v[795] = mfspr(795);	/* MMCR0 */
+	v[796] = mfspr(796);	/* SIAR */
+	v[797] = mfspr(797);	/* SDAR */
+	v[798] = mfspr(798);	/* MMCR1 */
+	v[800] = mfspr(800);	/* BESCRS */
+	v[801] = mfspr(801);	/* BESCCRSU */
+	v[802] = mfspr(802);	/* BESCRR */
+	v[803] = mfspr(803);	/* BESCRRU */
+	v[804] = mfspr(804);	/* EBBHR */
+	v[805] = mfspr(805);	/* EBBRR */
+	v[806] = mfspr(806);	/* BESCR */
+	v[815] = mfspr(815);	/* TAR */
+}
+
+static void get_sprs(uint64_t *v)
+{
+	uint32_t pvr = mfspr(287);	/* Processor Version Register */
+
+	get_sprs_common(v);
+
+	switch (pvr >> 16) {
+	case 0x39:			/* PPC970 */
+	case 0x3C:			/* PPC970FX */
+	case 0x44:			/* PPC970MP */
+		get_sprs_book3s_201(v);
+		break;
+	case 0x4b:			/* POWER8E */
+	case 0x4c:			/* POWER8NVL */
+	case 0x4d:			/* POWER8 */
+		get_sprs_book3s_207(v);
+		break;
+	}
+}
+
+int main(int argc, char **argv)
+{
+	int i;
+	bool waitkey = false;
+	uint64_t pat = 0xcafefacec0debabeULL;
+	const uint64_t patterns[] = {
+		0xcafefacec0debabeULL, ~0xcafefacec0debabeULL,
+		0xAAAA5555AAAA5555ULL, 0x5555AAAA5555AAAAULL,
+		0x1234567890ABCDEFULL, 0xFEDCBA0987654321ULL,
+		-1ULL,
+	};
+
+	for (i = 1; i < argc; i++) {
+		if (!strcmp(argv[i], "-w")) {
+			waitkey = true;
+		} else if (!strcmp(argv[i], "-p")) {
+			i += 1;
+			if (i >= argc || *argv[i] < '0'
+			    || *argv[i] >= '0' + ARRAY_SIZE(patterns))
+				report_abort("Error: bad value for -p");
+			pat ^= patterns[*argv[i] - '0'];
+		} else if (!strcmp(argv[i], "-t")) {
+			/* Randomize with timebase register */
+			asm volatile("mftb %0" : "=r"(i));
+			pat ^= i;
+			asm volatile("mftb %0" : "=r"(i));
+			pat ^= ~(uint64_t)i << 32;
+		} else {
+			report_abort("Warning: Unsupported argument: %s",
+			             argv[i]);
+		}
+	}
+
+	printf("Settings SPRs to 0x%lx...\n", pat);
+	set_sprs(pat);
+
+	memset(before, 0, sizeof(before));
+	memset(after, 0, sizeof(after));
+
+	get_sprs(before);
+
+	if (waitkey) {
+		handle_exception(0x100, &nmi_handler, NULL);
+		puts("Now migrate the VM, then press a key or send NMI...\n");
+		while (!nmi_occurred && h_get_term_char(0) == 0)
+			asm volatile(" nop " ::: "memory");
+	} else {
+		puts("Sleeping...\n");
+		handle_exception(0x900, &dec_except_handler, NULL);
+		asm volatile("mtdec %0" : : "r" (0x3FFFFFFF));
+		hcall(H_CEDE);
+	}
+
+	get_sprs(after);
+
+	puts("Checking SPRs...\n");
+	for (i = 0; i < 1024; i++) {
+		if (before[i] != 0 || after[i] != 0)
+			report("SPR %d:\t0x%016lx <==> 0x%016lx",
+				before[i] == after[i], i, before[i], after[i]);
+	}
+
+	return report_summary();
+}
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index 20dbde6..fb6b70e 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -59,3 +59,8 @@ file = tm.elf
 smp = 2,threads=2
 extra_params = -append "h_cede_tm"
 groups = nodefault,h_cede_tm
+
+[sprs]
+file = sprs.elf
+extra_params = -append '-w'
+groups = migration
-- 
1.8.3.1

Re: [kvm-unit-tests PATCH 2/2] powerpc: Add Special Purpose Register persistency test

[Cédric Le Goater]

On 03/09/2017 06:27 PM, Thomas Huth wrote:
> This test has two purposes: First, check whether the hypervisor can be
> destabilized by writing random values into the SPRs of the PowerPC CPU
> (this indeed revealed a bug last year, see CVE-2016-3044).
> Second, this test can be used to check whether the SPRs are synchronized
> properly between the KVM host CPU and QEMU, e.g. when migrating the VM
> from one QEMU instance to another.
> The test first fills the various SPRs with some non-zero value, then reads
> the values back into a first array. It then either sleeps a short period
> of time (for testing without migration, in the hope that we're rescheduled
> on another host CPU), or it waits for a key or NMI (with the '-w' option)
> so that it is possible to migrate the VM before continuing. The test then
> finally reads the values from the SPRs back into another array and then
> compares them with the initial values.
> Currently the test only supports the SPRs from the PowerISA v2.01
> (PowerPC 970) and PowerISA v2.07 specification (i.e. POWER8 CPUs),
> but other versions should be pretty easy to add later.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

It looks good to me. I gave it a try and it worked fine with some
extra tuning in the migration script.  

LDFLAGS needs fix for binutils 2.28 but that is another issue.

Tested-by: Cédric Le Goater <clg@kaod.org>

C.

> ---
>  powerpc/Makefile.common |   3 +-
>  powerpc/cstart64.S      |   2 +
>  powerpc/sprs.c          | 303 ++++++++++++++++++++++++++++++++++++++++++++++++
>  powerpc/unittests.cfg   |   5 +
>  4 files changed, 312 insertions(+), 1 deletion(-)
>  create mode 100644 powerpc/sprs.c
> 
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 37f8caa..92809a5 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -9,7 +9,8 @@ tests-common = \
>  	$(TEST_DIR)/spapr_hcall.elf \
>  	$(TEST_DIR)/rtas.elf \
>  	$(TEST_DIR)/emulator.elf \
> -	$(TEST_DIR)/tm.elf
> +	$(TEST_DIR)/tm.elf \
> +	$(TEST_DIR)/sprs.elf
>  
>  tests-all = $(tests-common) $(tests)
>  all: $(TEST_DIR)/boot_rom.bin $(tests-all)
> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
> index 2204e3b..ec673b3 100644
> --- a/powerpc/cstart64.S
> +++ b/powerpc/cstart64.S
> @@ -247,6 +247,8 @@ call_handler:
>  	.globl __start_interrupts
>  __start_interrupts:
>  
> +VECTOR(0x100)
> +VECTOR(0x200)
>  VECTOR(0x300)
>  VECTOR(0x400)
>  VECTOR(0x500)
> diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> new file mode 100644
> index 0000000..7792db4
> --- /dev/null
> +++ b/powerpc/sprs.c
> @@ -0,0 +1,303 @@
> +/*
> + * Test Special Purpose Registers
> + *
> + * Copyright 2017  Thomas Huth, Red Hat Inc.
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + *
> + * The basic idea of this test is to check whether the contents of the Special
> + * Purpose Registers (SPRs) are preserved correctly during migration. So we
> + * fill in the SPRs with a well-known value, read the values back (since not
> + * all bits might be retained in the SPRs), then wait for a key or NMI (if the
> + * '-w' option has been specified) so that the user has a chance to migrate the
> + * VM. Alternatively, the test can also simply sleep a little bit with the
> + * H_CEDE hypercall, in the hope that we'll get scheduled to another host CPU
> + * and thus register contents might have changed, too (in case of bugs).
> + * Finally, we read back the values from the SPRs and compare them with the
> + * values before the migration. Mismatches are reported as test failures.
> + * Note that we do not test all SPRs since some of the registers change their
> + * content automatically, and some are only accessible with hypervisor privi-
> + * ledges or have bad side effects, so we have to omit those registers.
> + */
> +#include <libcflat.h>
> +#include <util.h>
> +#include <alloc.h>
> +#include <asm/handlers.h>
> +#include <asm/hcall.h>
> +#include <asm/processor.h>
> +
> +#define mfspr(nr) ({ \
> +	uint64_t ret; \
> +	asm volatile("mfspr %0,%1" : "=r"(ret) : "i"(nr)); \
> +	ret; \
> +})
> +
> +#define mtspr(nr, val) \
> +	asm volatile("mtspr %0,%1" : : "i"(nr), "r"(val))
> +
> +uint64_t before[1024], after[1024];
> +
> +volatile int nmi_occurred;
> +
> +static void nmi_handler(struct pt_regs *regs __unused, void *opaque __unused)
> +{
> +	nmi_occurred = 1;
> +}
> +
> +static int h_get_term_char(uint64_t termno)
> +{
> +	register uint64_t r3 asm("r3") = 0x54; /* H_GET_TERM_CHAR */
> +	register uint64_t r4 asm("r4") = termno;
> +	register uint64_t r5 asm("r5");
> +
> +	asm volatile (" sc 1 "	: "+r"(r3), "+r"(r4), "=r"(r5)
> +				: "r"(r3),  "r"(r4));
> +
> +	return r3 == H_SUCCESS && r4 > 0 ? r5 >> 48 : 0;
> +}
> +
> +/* Common SPRs for all PowerPC CPUs */
> +static void set_sprs_common(uint64_t val)
> +{
> +	mtspr(9, val);		/* CTR */
> +	// mtspr(273, val);	/* SPRG1 */  /* Used by our exception handler */
> +	mtspr(274, val);	/* SPRG2 */
> +	mtspr(275, val);	/* SPRG3 */
> +}
> +
> +/* SPRs from PowerPC Operating Environment Architecture, Book III, Vers. 2.01 */
> +static void set_sprs_book3s_201(uint64_t val)
> +{
> +	mtspr(18, val);		/* DSISR */
> +	mtspr(19, val);		/* DAR */
> +	mtspr(152, val);	/* CTRL */
> +	mtspr(256, val);	/* VRSAVE */
> +	mtspr(786, val);	/* MMCRA */
> +	mtspr(795, val);	/* MMCR0 */
> +	mtspr(798, val);	/* MMCR1 */
> +}
> +
> +/* SPRs from PowerISA 2.07 Book III-S */
> +static void set_sprs_book3s_207(uint64_t val)
> +{
> +	mtspr(3, val);		/* DSCR */
> +	mtspr(13, val);		/* AMR */
> +	mtspr(17, val);		/* DSCR */
> +	mtspr(18, val);		/* DSISR */
> +	mtspr(19, val);		/* DAR */
> +	mtspr(29, val);		/* AMR */
> +	mtspr(61, val);		/* IAMR */
> +	// mtspr(152, val);	/* CTRL */  /* TODO: Needs a fix in KVM */
> +	mtspr(153, val);	/* FSCR */
> +	mtspr(157, val);	/* UAMOR */
> +	mtspr(159, val);	/* PSPB */
> +	mtspr(256, val);	/* VRSAVE */
> +	// mtspr(272, val);	/* SPRG0 */ /* Used by our exception handler */
> +	mtspr(769, val);	/* MMCR2 */
> +	mtspr(770, val);	/* MMCRA */
> +	mtspr(771, val);	/* PMC1 */
> +	mtspr(772, val);	/* PMC2 */
> +	mtspr(773, val);	/* PMC3 */
> +	mtspr(774, val);	/* PMC4 */
> +	mtspr(775, val);	/* PMC5 */
> +	mtspr(776, val);	/* PMC6 */
> +	mtspr(779, (val & 0xfffffffffbab3fffULL) | 0xfa0b2070);	/* MMCR0 */
> +	mtspr(784, val);	/* SIER */
> +	mtspr(785, val);	/* MMCR2 */
> +	mtspr(786, val);	/* MMCRA */
> +	mtspr(787, val);	/* PMC1 */
> +	mtspr(788, val);	/* PMC2 */
> +	mtspr(789, val);	/* PMC3 */
> +	mtspr(790, val);	/* PMC4 */
> +	mtspr(791, val);	/* PMC5 */
> +	mtspr(792, val);	/* PMC6 */
> +	mtspr(795, (val & 0xfffffffffbab3fffULL) | 0xfa0b2070);	/* MMCR0 */
> +	mtspr(796, val);	/* SIAR */
> +	mtspr(797, val);	/* SDAR */
> +	mtspr(798, val);	/* MMCR1 */
> +	mtspr(800, val);	/* BESCRS */
> +	mtspr(801, val);	/* BESCCRSU */
> +	mtspr(802, val);	/* BESCRR */
> +	mtspr(803, val);	/* BESCRRU */
> +	mtspr(804, val);	/* EBBHR */
> +	mtspr(805, val);	/* EBBRR */
> +	mtspr(806, val);	/* BESCR */
> +	mtspr(815, val);	/* TAR */
> +}
> +
> +static void set_sprs(uint64_t val)
> +{
> +	uint32_t pvr = mfspr(287);	/* Processor Version Register */
> +
> +	set_sprs_common(val);
> +
> +	switch (pvr >> 16) {
> +	case 0x39:			/* PPC970 */
> +	case 0x3C:			/* PPC970FX */
> +	case 0x44:			/* PPC970MP */
> +		set_sprs_book3s_201(val);
> +		break;
> +	case 0x4b:			/* POWER8E */
> +	case 0x4c:			/* POWER8NVL */
> +	case 0x4d:			/* POWER8 */
> +		set_sprs_book3s_207(val);
> +		break;
> +	default:
> +		puts("Warning: Unknown processor version!\n");
> +	}
> +}
> +
> +static void get_sprs_common(uint64_t *v)
> +{
> +	v[9] = mfspr(9);	/* CTR */
> +	// v[273] = mfspr(273);	/* SPRG1 */ /* Used by our exception handler */
> +	v[274] = mfspr(274);	/* SPRG2 */
> +	v[275] = mfspr(275);	/* SPRG3 */
> +}
> +
> +static void get_sprs_book3s_201(uint64_t *v)
> +{
> +	v[18] = mfspr(18);	/* DSISR */
> +	v[19] = mfspr(19);	/* DAR */
> +	v[136] = mfspr(136);	/* CTRL */
> +	v[256] = mfspr(256);	/* VRSAVE */
> +	v[786] = mfspr(786);	/* MMCRA */
> +	v[795] = mfspr(795);	/* MMCR0 */
> +	v[798] = mfspr(798);	/* MMCR1 */
> +}
> +
> +static void get_sprs_book3s_207(uint64_t *v)
> +{
> +	v[3] = mfspr(3);	/* DSCR */
> +	v[13] = mfspr(13);	/* AMR */
> +	v[17] = mfspr(17);	/* DSCR */
> +	v[18] = mfspr(18);	/* DSISR */
> +	v[19] = mfspr(19);	/* DAR */
> +	v[29] = mfspr(29);	/* AMR */
> +	v[61] = mfspr(61);	/* IAMR */
> +	//v[136] = mfspr(136);	/* CTRL */  /* TODO: Needs a fix in KVM */
> +	v[153] = mfspr(153);	/* FSCR */
> +	v[157] = mfspr(157);	/* UAMOR */
> +	v[159] = mfspr(159);	/* PSPB */
> +	v[256] = mfspr(256);	/* VRSAVE */
> +	v[259] = mfspr(259);	/* SPRG3 (read only) */
> +	// v[272] = mfspr(272);	/* SPRG0 */  /* Used by our exception handler */
> +	v[769] = mfspr(769);	/* MMCR2 */
> +	v[770] = mfspr(770);	/* MMCRA */
> +	v[771] = mfspr(771);	/* PMC1 */
> +	v[772] = mfspr(772);	/* PMC2 */
> +	v[773] = mfspr(773);	/* PMC3 */
> +	v[774] = mfspr(774);	/* PMC4 */
> +	v[775] = mfspr(775);	/* PMC5 */
> +	v[776] = mfspr(776);	/* PMC6 */
> +	v[779] = mfspr(779);	/* MMCR0 */
> +	v[780] = mfspr(780);	/* SIAR (read only) */
> +	v[781] = mfspr(781);	/* SDAR (read only) */
> +	v[782] = mfspr(782);	/* MMCR1 (read only) */
> +	v[784] = mfspr(784);	/* SIER */
> +	v[785] = mfspr(785);	/* MMCR2 */
> +	v[786] = mfspr(786);	/* MMCRA */
> +	v[787] = mfspr(787);	/* PMC1 */
> +	v[788] = mfspr(788);	/* PMC2 */
> +	v[789] = mfspr(789);	/* PMC3 */
> +	v[790] = mfspr(790);	/* PMC4 */
> +	v[791] = mfspr(791);	/* PMC5 */
> +	v[792] = mfspr(792);	/* PMC6 */
> +	v[795] = mfspr(795);	/* MMCR0 */
> +	v[796] = mfspr(796);	/* SIAR */
> +	v[797] = mfspr(797);	/* SDAR */
> +	v[798] = mfspr(798);	/* MMCR1 */
> +	v[800] = mfspr(800);	/* BESCRS */
> +	v[801] = mfspr(801);	/* BESCCRSU */
> +	v[802] = mfspr(802);	/* BESCRR */
> +	v[803] = mfspr(803);	/* BESCRRU */
> +	v[804] = mfspr(804);	/* EBBHR */
> +	v[805] = mfspr(805);	/* EBBRR */
> +	v[806] = mfspr(806);	/* BESCR */
> +	v[815] = mfspr(815);	/* TAR */
> +}
> +
> +static void get_sprs(uint64_t *v)
> +{
> +	uint32_t pvr = mfspr(287);	/* Processor Version Register */
> +
> +	get_sprs_common(v);
> +
> +	switch (pvr >> 16) {
> +	case 0x39:			/* PPC970 */
> +	case 0x3C:			/* PPC970FX */
> +	case 0x44:			/* PPC970MP */
> +		get_sprs_book3s_201(v);
> +		break;
> +	case 0x4b:			/* POWER8E */
> +	case 0x4c:			/* POWER8NVL */
> +	case 0x4d:			/* POWER8 */
> +		get_sprs_book3s_207(v);
> +		break;
> +	}
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	int i;
> +	bool waitkey = false;
> +	uint64_t pat = 0xcafefacec0debabeULL;
> +	const uint64_t patterns[] = {
> +		0xcafefacec0debabeULL, ~0xcafefacec0debabeULL,
> +		0xAAAA5555AAAA5555ULL, 0x5555AAAA5555AAAAULL,
> +		0x1234567890ABCDEFULL, 0xFEDCBA0987654321ULL,
> +		-1ULL,
> +	};
> +
> +	for (i = 1; i < argc; i++) {
> +		if (!strcmp(argv[i], "-w")) {
> +			waitkey = true;
> +		} else if (!strcmp(argv[i], "-p")) {
> +			i += 1;
> +			if (i >= argc || *argv[i] < '0'
> +			    || *argv[i] >= '0' + ARRAY_SIZE(patterns))
> +				report_abort("Error: bad value for -p");
> +			pat ^= patterns[*argv[i] - '0'];
> +		} else if (!strcmp(argv[i], "-t")) {
> +			/* Randomize with timebase register */
> +			asm volatile("mftb %0" : "=r"(i));
> +			pat ^= i;
> +			asm volatile("mftb %0" : "=r"(i));
> +			pat ^= ~(uint64_t)i << 32;
> +		} else {
> +			report_abort("Warning: Unsupported argument: %s",
> +			             argv[i]);
> +		}
> +	}
> +
> +	printf("Settings SPRs to 0x%lx...\n", pat);
> +	set_sprs(pat);
> +
> +	memset(before, 0, sizeof(before));
> +	memset(after, 0, sizeof(after));
> +
> +	get_sprs(before);
> +
> +	if (waitkey) {
> +		handle_exception(0x100, &nmi_handler, NULL);
> +		puts("Now migrate the VM, then press a key or send NMI...\n");
> +		while (!nmi_occurred && h_get_term_char(0) == 0)
> +			asm volatile(" nop " ::: "memory");
> +	} else {
> +		puts("Sleeping...\n");
> +		handle_exception(0x900, &dec_except_handler, NULL);
> +		asm volatile("mtdec %0" : : "r" (0x3FFFFFFF));
> +		hcall(H_CEDE);
> +	}
> +
> +	get_sprs(after);
> +
> +	puts("Checking SPRs...\n");
> +	for (i = 0; i < 1024; i++) {
> +		if (before[i] != 0 || after[i] != 0)
> +			report("SPR %d:\t0x%016lx <==> 0x%016lx",
> +				before[i] == after[i], i, before[i], after[i]);
> +	}
> +
> +	return report_summary();
> +}
> diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
> index 20dbde6..fb6b70e 100644
> --- a/powerpc/unittests.cfg
> +++ b/powerpc/unittests.cfg
> @@ -59,3 +59,8 @@ file = tm.elf
>  smp = 2,threads=2
>  extra_params = -append "h_cede_tm"
>  groups = nodefault,h_cede_tm
> +
> +[sprs]
> +file = sprs.elf
> +extra_params = -append '-w'
> +groups = migration
> 

Re: [kvm-unit-tests PATCH 2/2] powerpc: Add Special Purpose Register persistency test

[Cédric Le Goater]

On 03/10/2017 09:48 AM, Cédric Le Goater wrote:
> On 03/09/2017 06:27 PM, Thomas Huth wrote:
>> This test has two purposes: First, check whether the hypervisor can be
>> destabilized by writing random values into the SPRs of the PowerPC CPU
>> (this indeed revealed a bug last year, see CVE-2016-3044).
>> Second, this test can be used to check whether the SPRs are synchronized
>> properly between the KVM host CPU and QEMU, e.g. when migrating the VM
>> from one QEMU instance to another.
>> The test first fills the various SPRs with some non-zero value, then reads
>> the values back into a first array. It then either sleeps a short period
>> of time (for testing without migration, in the hope that we're rescheduled
>> on another host CPU), or it waits for a key or NMI (with the '-w' option)
>> so that it is possible to migrate the VM before continuing. The test then
>> finally reads the values from the SPRs back into another array and then
>> compares them with the initial values.
>> Currently the test only supports the SPRs from the PowerISA v2.01
>> (PowerPC 970) and PowerISA v2.07 specification (i.e. POWER8 CPUs),
>> but other versions should be pretty easy to add later.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> 
> It looks good to me. I gave it a try and it worked fine with some
> extra tuning in the migration script.  
> 
> LDFLAGS needs fix for binutils 2.28 but that is another issue.

An here's the "fix" for it. I haven't studied deeply the question 
though.  

Thanks,

C.

>From 820e2d614d63bf1cd911d13396b44a5ff8fb7bdc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= <clg@kaod.org>
Date: Fri, 10 Mar 2017 08:54:26 +0100
Subject: [PATCH] powerpc: add -N to LDFLAGS
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ld 2.28 complains with :

ld -EL -nostdlib -pie -o powerpc/selftest.elf \
	-T powerpc/flat.lds --build-id=none \
	powerpc/selftest.o powerpc/cstart64.o powerpc/reloc64.o lib/libcflat.a lib/libfdt/libfdt.a powerpc/selftest.aux.o
ld: powerpc/selftest.elf: Not enough room for program headers, try linking with -N
ld: final link failed: Bad value

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 powerpc/Makefile.common | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 92809a5468ea..6c62ac85311a 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -43,7 +43,7 @@ cflatobjs += lib/powerpc/smp.o
 
 FLATLIBS = $(libcflat) $(LIBFDT_archive)
 %.elf: CFLAGS += $(arch_CFLAGS)
-%.elf: LDFLAGS = $(arch_LDFLAGS) -nostdlib -pie
+%.elf: LDFLAGS = $(arch_LDFLAGS) -nostdlib -pie -N
 %.elf: %.o $(FLATLIBS) powerpc/flat.lds $(cstart.o) $(reloc.o)
 	$(CC) $(CFLAGS) -c -o $(@:.elf=.aux.o) lib/auxinfo.c -DPROGNAME=\"$@\"
 	$(LD) $(LDFLAGS) -o $@ \
-- 
2.7.4

Re: [kvm-unit-tests PATCH 2/2] powerpc: Add Special Purpose Register persistency test

[Thomas Huth]

On 10.03.2017 09:53, Cédric Le Goater wrote:
> On 03/10/2017 09:48 AM, Cédric Le Goater wrote:
>> On 03/09/2017 06:27 PM, Thomas Huth wrote:
>>> This test has two purposes: First, check whether the hypervisor can be
>>> destabilized by writing random values into the SPRs of the PowerPC CPU
>>> (this indeed revealed a bug last year, see CVE-2016-3044).
>>> Second, this test can be used to check whether the SPRs are synchronized
>>> properly between the KVM host CPU and QEMU, e.g. when migrating the VM
>>> from one QEMU instance to another.
>>> The test first fills the various SPRs with some non-zero value, then reads
>>> the values back into a first array. It then either sleeps a short period
>>> of time (for testing without migration, in the hope that we're rescheduled
>>> on another host CPU), or it waits for a key or NMI (with the '-w' option)
>>> so that it is possible to migrate the VM before continuing. The test then
>>> finally reads the values from the SPRs back into another array and then
>>> compares them with the initial values.
>>> Currently the test only supports the SPRs from the PowerISA v2.01
>>> (PowerPC 970) and PowerISA v2.07 specification (i.e. POWER8 CPUs),
>>> but other versions should be pretty easy to add later.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>
>> It looks good to me. I gave it a try and it worked fine with some
>> extra tuning in the migration script.  
>>
>> LDFLAGS needs fix for binutils 2.28 but that is another issue.
> 
> An here's the "fix" for it. I haven't studied deeply the question 
> though.  

I think this could be an explanation for this issue:
https://lists.gnu.org/archive/html/bug-gnu-utils/2002-08/msg00242.html

Could you please check whether it works with "-n" instead of "-N", too?
If so, I think that would be the better option.

 Thanks,
  Thomas

> From 820e2d614d63bf1cd911d13396b44a5ff8fb7bdc Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= <clg@kaod.org>
> Date: Fri, 10 Mar 2017 08:54:26 +0100
> Subject: [PATCH] powerpc: add -N to LDFLAGS
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> ld 2.28 complains with :
> 
> ld -EL -nostdlib -pie -o powerpc/selftest.elf \
> 	-T powerpc/flat.lds --build-id=none \
> 	powerpc/selftest.o powerpc/cstart64.o powerpc/reloc64.o lib/libcflat.a lib/libfdt/libfdt.a powerpc/selftest.aux.o
> ld: powerpc/selftest.elf: Not enough room for program headers, try linking with -N
> ld: final link failed: Bad value
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  powerpc/Makefile.common | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 92809a5468ea..6c62ac85311a 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -43,7 +43,7 @@ cflatobjs += lib/powerpc/smp.o
>  
>  FLATLIBS = $(libcflat) $(LIBFDT_archive)
>  %.elf: CFLAGS += $(arch_CFLAGS)
> -%.elf: LDFLAGS = $(arch_LDFLAGS) -nostdlib -pie
> +%.elf: LDFLAGS = $(arch_LDFLAGS) -nostdlib -pie -N
>  %.elf: %.o $(FLATLIBS) powerpc/flat.lds $(cstart.o) $(reloc.o)
>  	$(CC) $(CFLAGS) -c -o $(@:.elf=.aux.o) lib/auxinfo.c -DPROGNAME=\"$@\"
>  	$(LD) $(LDFLAGS) -o $@ \
> 

Re: [kvm-unit-tests PATCH 2/2] powerpc: Add Special Purpose Register persistency test

[Cédric Le Goater]

On 03/10/2017 10:38 AM, Thomas Huth wrote:
> On 10.03.2017 09:53, Cédric Le Goater wrote:
>> On 03/10/2017 09:48 AM, Cédric Le Goater wrote:
>>> On 03/09/2017 06:27 PM, Thomas Huth wrote:
>>>> This test has two purposes: First, check whether the hypervisor can be
>>>> destabilized by writing random values into the SPRs of the PowerPC CPU
>>>> (this indeed revealed a bug last year, see CVE-2016-3044).
>>>> Second, this test can be used to check whether the SPRs are synchronized
>>>> properly between the KVM host CPU and QEMU, e.g. when migrating the VM
>>>> from one QEMU instance to another.
>>>> The test first fills the various SPRs with some non-zero value, then reads
>>>> the values back into a first array. It then either sleeps a short period
>>>> of time (for testing without migration, in the hope that we're rescheduled
>>>> on another host CPU), or it waits for a key or NMI (with the '-w' option)
>>>> so that it is possible to migrate the VM before continuing. The test then
>>>> finally reads the values from the SPRs back into another array and then
>>>> compares them with the initial values.
>>>> Currently the test only supports the SPRs from the PowerISA v2.01
>>>> (PowerPC 970) and PowerISA v2.07 specification (i.e. POWER8 CPUs),
>>>> but other versions should be pretty easy to add later.
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>
>>> It looks good to me. I gave it a try and it worked fine with some
>>> extra tuning in the migration script.  
>>>
>>> LDFLAGS needs fix for binutils 2.28 but that is another issue.
>>
>> An here's the "fix" for it. I haven't studied deeply the question 
>> though.  
> 
> I think this could be an explanation for this issue:
> https://lists.gnu.org/archive/html/bug-gnu-utils/2002-08/msg00242.html
> 
> Could you please check whether it works with "-n" instead of "-N", too?
> If so, I think that would be the better option.


It works. I will send the patch

Thanks,

C.