Re: [kvm-unit-tests RFC] s390x: Add Protected VM support

Marc Hartmayer
2020-05-07T13:15:03Z

On Wed, May 06, 2020 at 03:50 PM +0200, Andrew Jones <drjones@redhat.com> wrote:
> On Wed, May 06, 2020 at 02:46:36PM +0200, Marc Hartmayer wrote:
>> Add support for Protected Virtual Machine (PVM) tests. For starting a
>> PVM guest we must be able to generate a PVM image by using the
>> `genprotimg` tool from the s390-tools collection. This requires the
>> ability to pass a machine-specific host-key document, so the option
>> `--host-key-document` is added to the configure script.
>>
>> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
>> ---
>> @@ -16,6 +16,8 @@
>> # # a test. The check line can contain multiple files
>> # # to check separated by a space but each check
>> # # parameter needs to be of the form <path>=<value>
>> +# pv_support = 0|1 # Optionally specify whether a test supports the
>> +# # execution as a PV guest.
>
> Maybe pv_supported vs. pv_support?

pv_supported is better, thanks.

>>
>> exec {fd}<"$unittests"
>>
>> while read -r -u $fd line; do
>> if [[ "$line" =~ ^\[(.*)\]$ ]]; then
>> - "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"

This line was accidentally removed.

>> + if [ "${pv_support}" == 1 ]; then
>> + pv_cmd "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
>> + fi
>> +
>> testname=${BASH_REMATCH[1]}
>> smp=1
>> kernel=""
>> @@ -27,6 +48,7 @@ function for_each_unittest()
>> check=""
>> accel=""
>> timeout=""
>> + pv_support=""
>> elif [[ $line =~ ^file\ *=\ *(.*)$ ]]; then
>> kernel=$TEST_DIR/${BASH_REMATCH[1]}
>> elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
>> @@ -43,8 +65,14 @@ function for_each_unittest()
>> accel=${BASH_REMATCH[1]}
>> elif [[ $line =~ ^timeout\ *=\ *(.*)$ ]]; then
>> timeout=${BASH_REMATCH[1]}
>> + elif [[ $line =~ ^pv_support\ *=\ *(.*)$ ]]; then
>> + pv_support=${BASH_REMATCH[1]}
>> fi
>> done
>> +
>> "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
>> + if [ "${pv_support}" == 1 ]; then
>> + pv_cmd "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
>> + fi
>> exec {fd}<&-
>> }
>> --
>> 2.17.0
>>
>
> I don't think making the changes to scripts/common.bash will work for
> standalone tests.

With the fix above it works (tested on x86 and s390x).

$ make standalone
Written tests/selftest-setup.
Written tests/intercept.
Written tests/intercept_PV.
Written tests/emulator.
Written tests/emulator_PV.
Written tests/sieve.
Written tests/sieve_PV.
…

> Why not do this stuff in s390x/run instead?

I will try.

> Also,
> do you need the pv_support[ed] parameter? You could just do a
> [ -f "${kernel%.elf}.pv.img" ] to decide if you should run again
> with PV, right?

AFAIK, for the other test cases the kernel file is also not checked, but
this would be an option - thanks.

>
> Thanks,
> drew
>

Thanks for the feedback!

--
Kind regards / Beste Grüße
Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294