Re: [kvm-unit-tests RFC v2 4/4] s390x: add Protected VM support

[Marc Hartmayer <mhartmay@linux.ibm.com>]

On Thu, Aug 13, 2020 at 04:22 PM +0200, Cornelia Huck <cohuck@redhat.com> wrote:
> On Thu, 13 Aug 2020 15:08:51 +0200
> Marc Hartmayer <mhartmay@linux.ibm.com> wrote:
>
>> On Thu, Aug 13, 2020 at 01:56 PM +0200, Cornelia Huck <cohuck@redhat.com> wrote:
>> > On Wed, 12 Aug 2020 11:27:05 +0200
>> > Marc Hartmayer <mhartmay@linux.ibm.com> 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>
>> >> ---
>> >>  configure               |  8 ++++++++
>> >>  s390x/Makefile          | 17 +++++++++++++++--
>> >>  s390x/selftest.parmfile |  1 +
>> >>  s390x/unittests.cfg     |  1 +
>> >>  scripts/s390x/func.bash | 18 ++++++++++++++++++
>> >>  5 files changed, 43 insertions(+), 2 deletions(-)
>> >>  create mode 100644 s390x/selftest.parmfile
>> >>  create mode 100644 scripts/s390x/func.bash
>> >> 
>> >> diff --git a/configure b/configure
>> >> index f9d030fd2f03..aa528af72534 100755
>> >> --- a/configure
>> >> +++ b/configure
>> >> @@ -18,6 +18,7 @@ u32_long=
>> >>  vmm="qemu"
>> >>  errata_force=0
>> >>  erratatxt="$srcdir/errata.txt"
>> >> +host_key_document=
>> >>  
>> >>  usage() {
>> >>      cat <<-EOF
>> >> @@ -40,6 +41,8 @@ usage() {
>> >>  	                           no environ is provided by the user (enabled by default)
>> >>  	    --erratatxt=FILE       specify a file to use instead of errata.txt. Use
>> >>  	                           '--erratatxt=' to ensure no file is used.
>> >> +	    --host-key-document=HOST_KEY_DOCUMENT
>> >> +	                           host-key-document to use (s390x only)  
>> >
>> > Maybe a bit more verbose? If I see only this option, I have no idea
>> > what it is used for and where to get it.  
>> 
>> “Specifies the machine-specific host-key document required to create a
>> PVM image using the `genprotimg` tool from the s390-tools collection
>> (s390x only)”
>> 
>> Better?
>
> "specify the machine-specific host-key document for creating a PVM
> image with 'genprotimg' (s390x only)"
>
> I think you can figure out where to get genprotimg if you actually know
> that you want it ;)
>
> (...)
>
>> >> diff --git a/scripts/s390x/func.bash b/scripts/s390x/func.bash
>> >> new file mode 100644
>> >> index 000000000000..5c682cb47f73
>> >> --- /dev/null
>> >> +++ b/scripts/s390x/func.bash
>> >> @@ -0,0 +1,18 @@
>> >> +# Run Protected VM test
>> >> +function arch_cmd()
>> >> +{
>> >> +	local cmd=$1
>> >> +	local testname=$2
>> >> +	local groups=$3
>> >> +	local smp=$4
>> >> +	local kernel=$5
>> >> +	local opts=$6
>> >> +	local arch=$7
>> >> +	local check=$8
>> >> +	local accel=$9
>> >> +	local timeout=${10}
>> >> +
>> >> +	kernel=${kernel%.elf}.pv.bin
>> >> +	# do not run PV test cases by default
>> >> +	"$cmd" "${testname}_PV" "$groups pv nodefault" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"  
>> >
>> > If we don't run this test, can we maybe print some informative message
>> > like "PV tests not run; specify --host-key-document to enable" or so?
>> > (At whichever point that makes the most sense.)  
>> 
>> Currently, the output looks like this:
>> 
>> $ ./run_tests.sh    
>> PASS selftest-setup (14 tests)
>> SKIP selftest-setup_PV (test marked as manual run only)
>> PASS intercept (20 tests)
>> SKIP intercept_PV (test marked as manual run only)
>> …
>> 
>> And if you’re trying to run the PV tests without specifying the host-key
>> document it results in:
>> 
>> $ ./run_tests.sh -a
>> PASS selftest-setup (14 tests)
>> FAIL selftest-setup_PV 
>> PASS intercept (20 tests)
>> FAIL intercept_PV 
>> …
>> 
>> But if you like I can return a hint that the PVM image was not
>> generated. Should the PV test case then be skipped?
>
> Yes, I was expecting something like
>
> SKIP selftest-setup_PV (no host-key document specified)
> SKIP intercept_PV (no host-key document specified)
>
> so that you get a hint what you may want to set up.

Okay, I’ll add this and remove the marker that PV tests aren’t executed
by default. Because when someone builds the PV images, he will most
likely want to run the PV test cases as well.

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