Skip to contentSkip to conversation

s390x: Add Protected VM support

PATCH In review Marc Hartmayer 5 participants 19 messages Aug 18, 2020 – Aug 21, 2020

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover s390x: Add Protected VM support

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.

For everybody's convenience there is a branch:
https://gitlab.com/mhartmay/kvm-unit-tests/-/tree/pv_v1

Changelog:
RFC v2 -> v1:
+ Rebased
+ patch 1:
- add r-b from Andrew
+ patch 2:
- add explicit dependency on config.mak (Andrew)
- add comment about the order of sourcing (Andrew)
+ patch 3:
- drop dummy function (Andrew)
- add arch_cmd hook function (Andrew)
+ patch 4:
- rephrased the documentation of the configure option (Conny)
- Skip test case if a PVM image wasn't built or the host-key document wasn't set (Conny)
- Run PV tests by default

RFC v1 -> RFC v2:
+ Remove pv_support option (Janosch, David)
+ Add some preliminary patches:
- move "testname guard"
- add support for architecture dependent functions
+ Add support for specifying a parmline file for the PV image generation. This is necessary for the selftest because the kernel cmdline set on the QEMU command line is ignored for PV guests

Marc Hartmayer (4):
common.bash: run cmd only if a test case was found
scripts: add support for architecture dependent functions run_tests/mkstandalone: add arch_cmd hook
s390x: add Protected VM support

 README.md               |  3 ++-
 configure               |  9 +++++++++
 run_tests.sh            |  3 ---
 s390x/Makefile          | 17 +++++++++++++++--
 s390x/selftest.parmfile |  1 +
 s390x/unittests.cfg     |  1 +
 scripts/common.bash     | 21 +++++++++++++++++++--
 scripts/mkstandalone.sh |  4 ----
 scripts/s390x/func.bash | 35 +++++++++++++++++++++++++++++++++++
 9 files changed, 82 insertions(+), 12 deletions(-)
 create mode 100644 s390x/selftest.parmfile
 create mode 100644 scripts/s390x/func.bash
Signature · 1 line
2.25.4
#1 lore raw

PATCH 1/4 common.bash: run `cmd` only if a test case was found

It's only useful to run cmd in for_each_unittest if a test case was found. This change allows us to remove the guards from the functions run_task and mkstandalone.

---
 run_tests.sh            | 3 ---
 scripts/common.bash     | 8 ++++++--
 scripts/mkstandalone.sh | 4 ----
 3 files changed, 6 insertions(+), 9 deletions(-)
run_tests.sh +0 -3
index 01e36dcfa06e..24aba9cc3a98 100755
@@ -126,9 +126,6 @@ RUNTIME_log_stdout () {
 function run_task()
 {
 	local testname="$1"
-	if [ -z "$testname" ]; then
-		return
-	fi
 
 	while (( $(jobs | wc -l) == $unittest_run_queues )); do
 		# wait for any background test to finish
scripts/common.bash +6 -2
index 9a6ebbd7f287..96655c9ffd1f 100644
@@ -17,7 +17,9 @@ function for_each_unittest()
 
 	while read -r -u $fd line; do
 		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
-			"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+			if [ -n "${testname}" ]; then
+				"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+			fi
 			testname=${BASH_REMATCH[1]}
 			smp=1
 			kernel=""
@@ -45,6 +47,8 @@ function for_each_unittest()
 			timeout=${BASH_REMATCH[1]}
 		fi
 	done
-	"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+	if [ -n "${testname}" ]; then
+		"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+	fi
 	exec {fd}<&-
 }
scripts/mkstandalone.sh +0 -4
index 9d506cc95072..cefdec30cb33 100755
@@ -83,10 +83,6 @@ function mkstandalone()
 {
 	local testname="$1"
 
-	if [ -z "$testname" ]; then
-		return
-	fi
-
 	if [ -n "$one_testname" ] && [ "$testname" != "$one_testname" ]; then
 		return
 	fi
Signature · 1 line
2.25.4
Reviewed-by on v1 1/4
#2 lore raw

Re: [kvm-unit-tests PATCH 1/4] common.bash: run `cmd` only if a test case was found

Quoted 11 lines · On Tue, 18 Aug 2020 15:04:21 +0200 Marc Hartmayer <[email protected]>…

It's only useful to run cmd in for_each_unittest if a test case was found. This change allows us to remove the guards from the functions run_task and mkstandalone.

---
 run_tests.sh            | 3 ---
 scripts/common.bash     | 8 ++++++--
 scripts/mkstandalone.sh | 4 ----
 3 files changed, 6 insertions(+), 9 deletions(-)
Reviewed-by on v1 1/4
#3 lore raw

Re: [kvm-unit-tests PATCH 1/4] common.bash: run `cmd` only if a test case was found

In reply to Marc Hartmayer
Quoted 57 lines · Marc Hartmayer

It's only useful to run cmd in for_each_unittest if a test case was found. This change allows us to remove the guards from the functions run_task and mkstandalone.

---
 run_tests.sh            | 3 ---
 scripts/common.bash     | 8 ++++++--
 scripts/mkstandalone.sh | 4 ----
 3 files changed, 6 insertions(+), 9 deletions(-)
run_tests.sh +0 -3
index 01e36dcfa06e..24aba9cc3a98 100755
@@ -126,9 +126,6 @@ RUNTIME_log_stdout () {
 function run_task()
 {
 	local testname="$1"
-	if [ -z "$testname" ]; then
-		return
-	fi
 
 	while (( $(jobs | wc -l) == $unittest_run_queues )); do
 		# wait for any background test to finish
scripts/common.bash +6 -2
index 9a6ebbd7f287..96655c9ffd1f 100644
@@ -17,7 +17,9 @@ function for_each_unittest()
 
 	while read -r -u $fd line; do
 		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
-			"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+			if [ -n "${testname}" ]; then
+				"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+			fi
 			testname=${BASH_REMATCH[1]}
 			smp=1
 			kernel=""
@@ -45,6 +47,8 @@ function for_each_unittest()
 			timeout=${BASH_REMATCH[1]}
 		fi
 	done
-	"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+	if [ -n "${testname}" ]; then
+		"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+	fi
 	exec {fd}<&-
 }
scripts/mkstandalone.sh +0 -0
index 9d506cc95072..cefdec30cb33 100755

scripts/mkstandalone.sh function mkstandalone() · line 88

Quoted 10 lines
scripts/mkstandalone.sh +0 -4
@@ -83,10 +83,6 @@ function mkstandalone()
 {
 	local testname="$1"
 
-	if [ -z "$testname" ]; then
-		return
-	fi
-
 	if [ -n "$one_testname" ] && [ "$testname" != "$one_testname" ]; then
 		return
 	fi
Signature · 3 lines
Thanks,

David / dhildenb
#4 lore raw

PATCH 2/4 scripts: add support for architecture dependent functions

In reply to Marc Hartmayer

This is necessary to keep architecture dependent code separate from common code.

---
 README.md           | 3 ++-
 scripts/common.bash | 8 ++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)
README.md +2 -1
index 48be206c6db1..24d4bdaaee0d 100644
@@ -134,7 +134,8 @@ all unit tests.
 ## Directory structure
 
     .:                  configure script, top-level Makefile, and run_tests.sh
-    ./scripts:          helper scripts for building and running tests
+    ./scripts:          general architecture neutral helper scripts for building and running tests
+    ./scripts/<ARCH>:   architecture dependent helper scripts for building and running tests
     ./lib:              general architecture neutral services for the tests
     ./lib/<ARCH>:       architecture dependent services for the tests
     ./<ARCH>:           the sources of the tests and the created objects/images
scripts/common.bash +8 -0
index 96655c9ffd1f..c7acdf14a835 100644
@@ -1,3 +1,4 @@
+source config.mak
 
 function for_each_unittest()
 {
@@ -52,3 +53,10 @@ function for_each_unittest()
 	fi
 	exec {fd}<&-
 }
+
+# The current file has to be the only file sourcing the arch helper
+# file
+ARCH_FUNC=scripts/${ARCH}/func.bash
+if [ -f "${ARCH_FUNC}" ]; then
+	source "${ARCH_FUNC}"
+fi
Signature · 1 line
2.25.4
Reviewed-by on v1 2/4
#5 lore raw

Re: [kvm-unit-tests PATCH 2/4] scripts: add support for architecture dependent functions

Quoted 46 lines · Marc Hartmayer

This is necessary to keep architecture dependent code separate from common code.

---
 README.md           | 3 ++-
 scripts/common.bash | 8 ++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)
README.md +2 -1
index 48be206c6db1..24d4bdaaee0d 100644
@@ -134,7 +134,8 @@ all unit tests.
 ## Directory structure
 
     .:                  configure script, top-level Makefile, and run_tests.sh
-    ./scripts:          helper scripts for building and running tests
+    ./scripts:          general architecture neutral helper scripts for building and running tests
+    ./scripts/<ARCH>:   architecture dependent helper scripts for building and running tests
     ./lib:              general architecture neutral services for the tests
     ./lib/<ARCH>:       architecture dependent services for the tests
     ./<ARCH>:           the sources of the tests and the created objects/images
scripts/common.bash +8 -0
index 96655c9ffd1f..c7acdf14a835 100644
@@ -1,3 +1,4 @@
+source config.mak
 
 function for_each_unittest()
 {
@@ -52,3 +53,10 @@ function for_each_unittest()
 	fi
 	exec {fd}<&-
 }
+
+# The current file has to be the only file sourcing the arch helper
+# file
+ARCH_FUNC=scripts/${ARCH}/func.bash
+if [ -f "${ARCH_FUNC}" ]; then
+	source "${ARCH_FUNC}"
+fi
Signature · 1 line
2.25.4
Reviewed-by on v1 2/4
#6 lore raw

Re: [kvm-unit-tests PATCH 2/4] scripts: add support for architecture dependent functions

In reply to Marc Hartmayer
Quoted 9 lines · On Tue, 18 Aug 2020 15:04:22 +0200 Marc Hartmayer <[email protected]>…

This is necessary to keep architecture dependent code separate from common code.

---
 README.md           | 3 ++-
 scripts/common.bash | 8 ++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)
Reviewed-by on v1 2/4
#7 lore raw

Re: [kvm-unit-tests PATCH 2/4] scripts: add support for architecture dependent functions

In reply to Marc Hartmayer
Quoted 34 lines · Marc Hartmayer

This is necessary to keep architecture dependent code separate from common code.

---
 README.md           | 3 ++-
 scripts/common.bash | 8 ++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)
README.md +2 -1
index 48be206c6db1..24d4bdaaee0d 100644
@@ -134,7 +134,8 @@ all unit tests.
 ## Directory structure
 
     .:                  configure script, top-level Makefile, and run_tests.sh
-    ./scripts:          helper scripts for building and running tests
+    ./scripts:          general architecture neutral helper scripts for building and running tests
+    ./scripts/<ARCH>:   architecture dependent helper scripts for building and running tests
     ./lib:              general architecture neutral services for the tests
     ./lib/<ARCH>:       architecture dependent services for the tests
     ./<ARCH>:           the sources of the tests and the created objects/images
scripts/common.bash +1 -0
index 96655c9ffd1f..c7acdf14a835 100644
@@ -1,3 +1,4 @@
+source config.mak
 
 function for_each_unittest()
 {

scripts/common.bash function for_each_unittest() · line 62

Quoted 10 lines
scripts/common.bash +7 -0
@@ -52,3 +53,10 @@ function for_each_unittest()
 	fi
 	exec {fd}<&-
 }
+
+# The current file has to be the only file sourcing the arch helper
+# file
+ARCH_FUNC=scripts/${ARCH}/func.bash
+if [ -f "${ARCH_FUNC}" ]; then
+	source "${ARCH_FUNC}"
+fi
Signature · 3 lines
Thanks,

David / dhildenb
#8 lore raw

PATCH 3/4 run_tests/mkstandalone: add arch_cmd hook

In reply to Marc Hartmayer

This allows us, for example, to auto generate a new test case based on an existing test case.

---
 scripts/common.bash | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
scripts/common.bash +7 -2
index c7acdf14a835..a6044b7c6c35 100644
@@ -19,7 +19,7 @@ function for_each_unittest()
 	while read -r -u $fd line; do
 		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
 			if [ -n "${testname}" ]; then
-				"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
 			fi
 			testname=${BASH_REMATCH[1]}
 			smp=1
@@ -49,11 +49,16 @@ function for_each_unittest()
 		fi
 	done
 	if [ -n "${testname}" ]; then
-		"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
 	fi
 	exec {fd}<&-
 }
 
+function arch_cmd()
+{
+	[ "${ARCH_CMD}" ] && echo "${ARCH_CMD}"
+}
+
 # The current file has to be the only file sourcing the arch helper
 # file
 ARCH_FUNC=scripts/${ARCH}/func.bash
Signature · 1 line
2.25.4
Reviewed-by on v1 3/4
#9 lore raw

Re: [kvm-unit-tests PATCH 3/4] run_tests/mkstandalone: add arch_cmd hook

Quoted 42 lines · Marc Hartmayer

This allows us, for example, to auto generate a new test case based on an existing test case.

---
 scripts/common.bash | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
scripts/common.bash +7 -2
index c7acdf14a835..a6044b7c6c35 100644
@@ -19,7 +19,7 @@ function for_each_unittest()
 	while read -r -u $fd line; do
 		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
 			if [ -n "${testname}" ]; then
-				"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
 			fi
 			testname=${BASH_REMATCH[1]}
 			smp=1
@@ -49,11 +49,16 @@ function for_each_unittest()
 		fi
 	done
 	if [ -n "${testname}" ]; then
-		"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
 	fi
 	exec {fd}<&-
 }
 
+function arch_cmd()
+{
+	[ "${ARCH_CMD}" ] && echo "${ARCH_CMD}"
+}
+
 # The current file has to be the only file sourcing the arch helper
 # file
 ARCH_FUNC=scripts/${ARCH}/func.bash
Signature · 1 line
2.25.4
Reviewed-by on v1 3/4
#10 lore raw

Re: [kvm-unit-tests PATCH 3/4] run_tests/mkstandalone: add arch_cmd hook

In reply to Marc Hartmayer
Quoted 7 lines · On Tue, 18 Aug 2020 15:04:23 +0200 Marc Hartmayer <[email protected]>…

This allows us, for example, to auto generate a new test case based on an existing test case.

---
 scripts/common.bash | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
Reviewed-by on v1 3/4
#11 lore raw

Re: [kvm-unit-tests PATCH 3/4] run_tests/mkstandalone: add arch_cmd hook

In reply to Marc Hartmayer
Quoted 2 lines · Marc Hartmayer

This allows us, for example, to auto generate a new test case based on an existing test case.

An example would have been nice :)

Quoted 28 lines
---
 scripts/common.bash | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
scripts/common.bash +2 -2
index c7acdf14a835..a6044b7c6c35 100644
@@ -19,7 +19,7 @@ function for_each_unittest()
 	while read -r -u $fd line; do
 		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
 			if [ -n "${testname}" ]; then
-				"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
 			fi
 			testname=${BASH_REMATCH[1]}
 			smp=1
@@ -49,11 +49,16 @@ function for_each_unittest()
 		fi
 	done
 	if [ -n "${testname}" ]; then
-		"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
 	fi
 	exec {fd}<&-

scripts/common.bash function for_each_unittest() · line 64

Quoted 10 lines
scripts/common.bash +5 -0
 }
 
+function arch_cmd()
+{
+	[ "${ARCH_CMD}" ] && echo "${ARCH_CMD}"
+}
+
 # The current file has to be the only file sourcing the arch helper
 # file
 ARCH_FUNC=scripts/${ARCH}/func.bash
Signature · 3 lines
Thanks,

David / dhildenb
#12 lore raw

PATCH 4/4 s390x: add Protected VM support

In reply to Marc Hartmayer

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.

---
 configure               |  9 +++++++++
 s390x/Makefile          | 17 +++++++++++++++--
 s390x/selftest.parmfile |  1 +
 s390x/unittests.cfg     |  1 +
 scripts/s390x/func.bash | 35 +++++++++++++++++++++++++++++++++++
 5 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100644 s390x/selftest.parmfile
 create mode 100644 scripts/s390x/func.bash
configure +9 -0
index f9d030fd2f03..0e64af58b3c1 100755
@@ -18,6 +18,7 @@ u32_long=
 vmm="qemu"
 errata_force=0
 erratatxt="$srcdir/errata.txt"
+host_key_document=
 
 usage() {
     cat <<-EOF
@@ -40,6 +41,9 @@ 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
+	                           Specify the machine-specific host-key document for creating
+	                           a PVM image with 'genprotimg' (s390x only)
 EOF
     exit 1
 }
@@ -92,6 +96,9 @@ while [[ "$1" = -* ]]; do
 	    erratatxt=
 	    [ "$arg" ] && erratatxt=$(eval realpath "$arg")
 	    ;;
+	--host-key-document)
+	    host_key_document="$arg"
+	    ;;
 	--help)
 	    usage
 	    ;;
@@ -205,6 +212,8 @@ PRETTY_PRINT_STACKS=$pretty_print_stacks
 ENVIRON_DEFAULT=$environ_default
 ERRATATXT=$erratatxt
 U32_LONG_FMT=$u32_long
+GENPROTIMG=${GENPROTIMG-genprotimg}
+HOST_KEY_DOCUMENT=$host_key_document
 EOF
 
 cat <<EOF > lib/config.h
s390x/Makefile +15 -2
index 0f54bf43bfb7..cd4e270952ec 100644
@@ -18,12 +18,19 @@ tests += $(TEST_DIR)/skrf.elf
 tests += $(TEST_DIR)/smp.elf
 tests += $(TEST_DIR)/sclp.elf
 tests += $(TEST_DIR)/css.elf
-tests_binary = $(patsubst %.elf,%.bin,$(tests))
 
-all: directories test_cases test_cases_binary
+tests_binary = $(patsubst %.elf,%.bin,$(tests))
+ifneq ($(HOST_KEY_DOCUMENT),)
+tests_pv_binary = $(patsubst %.bin,%.pv.bin,$(tests_binary))
+else
+tests_pv_binary =
+endif
+
+all: directories test_cases test_cases_binary test_cases_pv
 
 test_cases: $(tests)
 test_cases_binary: $(tests_binary)
+test_cases_pv: $(tests_pv_binary)
 
 CFLAGS += -std=gnu99
 CFLAGS += -ffreestanding
@@ -72,6 +79,12 @@ FLATLIBS = $(libcflat)
 %.bin: %.elf
 	$(OBJCOPY) -O binary  $< $@
 
+%selftest.pv.bin: %selftest.bin $(HOST_KEY_DOCUMENT) $(patsubst %.pv.bin,%.parmfile,$@)
+	$(GENPROTIMG) --host-key-document $(HOST_KEY_DOCUMENT) --parmfile $(patsubst %.pv.bin,%.parmfile,$@) --no-verify --image $< -o $@
+
+%.pv.bin: %.bin $(HOST_KEY_DOCUMENT)
+	$(GENPROTIMG) --host-key-document $(HOST_KEY_DOCUMENT) --no-verify --image $< -o $@
+
 arch_clean: asm_offsets_clean
 	$(RM) $(TEST_DIR)/*.{o,elf,bin} $(TEST_DIR)/.*.d lib/s390x/.*.d
 
s390x/selftest.parmfile new file +1 -0
new file mode 100644
index 000000000000..5613931aa5c6
@@ -0,0 +1 @@
+test 123
\ No newline at end of file
s390x/unittests.cfg +1 -0
index 0f156afbe741..12f6fb613995 100644
@@ -21,6 +21,7 @@
 [selftest-setup]
 file = selftest.elf
 groups = selftest
+# please keep the kernel cmdline in sync with $(TEST_DIR)/selftest.parmfile
 extra_params = -append 'test 123'
 
 [intercept]
scripts/s390x/func.bash new file +35 -0
new file mode 100644
index 000000000000..b2d59d0d6f25
@@ -0,0 +1,35 @@
+# The file scripts/common.bash has to be the only file sourcing this
+# arch helper file
+source config.mak
+
+ARCH_CMD=arch_cmd_s390x
+
+function arch_cmd_s390x()
+{
+	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}
+
+	# run the normal test case
+	"$cmd" "${testname}" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+
+	# run PV test case
+	kernel=${kernel%.elf}.pv.bin
+	if [ ! -f "${kernel}" ]; then
+		if [ -z "${HOST_KEY_DOCUMENT}" ]; then
+			print_result 'SKIP' $testname '(no host-key document specified)'
+			return 2
+		fi
+
+		print_result 'SKIP' $testname '(PVM image was not created)'
+		return 2
+	fi
+	"$cmd" "${testname}_PV" "$groups pv" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+}
Signature · 1 line
2.25.4
Reviewed-by on v1 4/4
#13 lore raw

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

Quoted 7 lines · Marc Hartmayer

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.

I like that solution a lot, it's easy to add the parmline, the host key document is configured once and the tests are run as default if they have been created. It's everything that I wished for.

Quoted 152 lines
---
 configure               |  9 +++++++++
 s390x/Makefile          | 17 +++++++++++++++--
 s390x/selftest.parmfile |  1 +
 s390x/unittests.cfg     |  1 +
 scripts/s390x/func.bash | 35 +++++++++++++++++++++++++++++++++++
 5 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100644 s390x/selftest.parmfile
 create mode 100644 scripts/s390x/func.bash
configure +9 -0
index f9d030fd2f03..0e64af58b3c1 100755
@@ -18,6 +18,7 @@ u32_long=
 vmm="qemu"
 errata_force=0
 erratatxt="$srcdir/errata.txt"
+host_key_document=
 
 usage() {
     cat <<-EOF
@@ -40,6 +41,9 @@ 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
+	                           Specify the machine-specific host-key document for creating
+	                           a PVM image with 'genprotimg' (s390x only)
 EOF
     exit 1
 }
@@ -92,6 +96,9 @@ while [[ "$1" = -* ]]; do
 	    erratatxt=
 	    [ "$arg" ] && erratatxt=$(eval realpath "$arg")
 	    ;;
+	--host-key-document)
+	    host_key_document="$arg"
+	    ;;
 	--help)
 	    usage
 	    ;;
@@ -205,6 +212,8 @@ PRETTY_PRINT_STACKS=$pretty_print_stacks
 ENVIRON_DEFAULT=$environ_default
 ERRATATXT=$erratatxt
 U32_LONG_FMT=$u32_long
+GENPROTIMG=${GENPROTIMG-genprotimg}
+HOST_KEY_DOCUMENT=$host_key_document
 EOF
 
 cat <<EOF > lib/config.h
s390x/Makefile +15 -2
index 0f54bf43bfb7..cd4e270952ec 100644
@@ -18,12 +18,19 @@ tests += $(TEST_DIR)/skrf.elf
 tests += $(TEST_DIR)/smp.elf
 tests += $(TEST_DIR)/sclp.elf
 tests += $(TEST_DIR)/css.elf
-tests_binary = $(patsubst %.elf,%.bin,$(tests))
 
-all: directories test_cases test_cases_binary
+tests_binary = $(patsubst %.elf,%.bin,$(tests))
+ifneq ($(HOST_KEY_DOCUMENT),)
+tests_pv_binary = $(patsubst %.bin,%.pv.bin,$(tests_binary))
+else
+tests_pv_binary =
+endif
+
+all: directories test_cases test_cases_binary test_cases_pv
 
 test_cases: $(tests)
 test_cases_binary: $(tests_binary)
+test_cases_pv: $(tests_pv_binary)
 
 CFLAGS += -std=gnu99
 CFLAGS += -ffreestanding
@@ -72,6 +79,12 @@ FLATLIBS = $(libcflat)
 %.bin: %.elf
 	$(OBJCOPY) -O binary  $< $@
 
+%selftest.pv.bin: %selftest.bin $(HOST_KEY_DOCUMENT) $(patsubst %.pv.bin,%.parmfile,$@)
+	$(GENPROTIMG) --host-key-document $(HOST_KEY_DOCUMENT) --parmfile $(patsubst %.pv.bin,%.parmfile,$@) --no-verify --image $< -o $@
+
+%.pv.bin: %.bin $(HOST_KEY_DOCUMENT)
+	$(GENPROTIMG) --host-key-document $(HOST_KEY_DOCUMENT) --no-verify --image $< -o $@
+
 arch_clean: asm_offsets_clean
 	$(RM) $(TEST_DIR)/*.{o,elf,bin} $(TEST_DIR)/.*.d lib/s390x/.*.d
 
s390x/selftest.parmfile new file +1 -0
new file mode 100644
index 000000000000..5613931aa5c6
@@ -0,0 +1 @@
+test 123
\ No newline at end of file
s390x/unittests.cfg +1 -0
index 0f156afbe741..12f6fb613995 100644
@@ -21,6 +21,7 @@
 [selftest-setup]
 file = selftest.elf
 groups = selftest
+# please keep the kernel cmdline in sync with $(TEST_DIR)/selftest.parmfile
 extra_params = -append 'test 123'
 
 [intercept]
scripts/s390x/func.bash new file +35 -0
new file mode 100644
index 000000000000..b2d59d0d6f25
@@ -0,0 +1,35 @@
+# The file scripts/common.bash has to be the only file sourcing this
+# arch helper file
+source config.mak
+
+ARCH_CMD=arch_cmd_s390x
+
+function arch_cmd_s390x()
+{
+	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}
+
+	# run the normal test case
+	"$cmd" "${testname}" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+
+	# run PV test case
+	kernel=${kernel%.elf}.pv.bin
+	if [ ! -f "${kernel}" ]; then
+		if [ -z "${HOST_KEY_DOCUMENT}" ]; then
+			print_result 'SKIP' $testname '(no host-key document specified)'
+			return 2
+		fi
+
+		print_result 'SKIP' $testname '(PVM image was not created)'
+		return 2
+	fi
+	"$cmd" "${testname}_PV" "$groups pv" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+}
#14 lore raw

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

In reply to Marc Hartmayer
Quoted 16 lines · On Tue, 18 Aug 2020 15:04:24 +0200 Marc Hartmayer <[email protected]>…

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.

---
 configure               |  9 +++++++++
 s390x/Makefile          | 17 +++++++++++++++--
 s390x/selftest.parmfile |  1 +
 s390x/unittests.cfg     |  1 +
 scripts/s390x/func.bash | 35 +++++++++++++++++++++++++++++++++++
 5 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100644 s390x/selftest.parmfile
 create mode 100644 scripts/s390x/func.bash

(...)

Quoted 15 lines
scripts/s390x/func.bash new file +15 -0
+function arch_cmd_s390x()
+{
+	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}
+
+	# run the normal test case
+	"$cmd" "${testname}" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"

scripts/s390x/func.bash line 31

Quoted 10 lines
scripts/s390x/func.bash new file +10 -0
+
+	# run PV test case
+	kernel=${kernel%.elf}.pv.bin
+	if [ ! -f "${kernel}" ]; then
+		if [ -z "${HOST_KEY_DOCUMENT}" ]; then
+			print_result 'SKIP' $testname '(no host-key document specified)'
+			return 2
+		fi
+
+		print_result 'SKIP' $testname '(PVM image was not created)'

When can that happen? Don't we already fail earlier if we specified a host key document, but genprotimg does not work?

Quoted 4 lines
scripts/s390x/func.bash new file +4 -0
+		return 2
+	fi
+	"$cmd" "${testname}_PV" "$groups pv" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+}
#15 lore raw

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

Quoted 42 lines · Cornelia Huck
Quoted 16 lines · On Tue, 18 Aug 2020 15:04:24 +0200 Marc Hartmayer <[email protected]>…

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.

---
 configure               |  9 +++++++++
 s390x/Makefile          | 17 +++++++++++++++--
 s390x/selftest.parmfile |  1 +
 s390x/unittests.cfg     |  1 +
 scripts/s390x/func.bash | 35 +++++++++++++++++++++++++++++++++++
 5 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100644 s390x/selftest.parmfile
 create mode 100644 scripts/s390x/func.bash

(...)

Quoted 20 lines
scripts/s390x/func.bash new file +20 -0
+function arch_cmd_s390x()
+{
+	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}
+
+	# run the normal test case
+	"$cmd" "${testname}" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+
+	# run PV test case
+	kernel=${kernel%.elf}.pv.bin
+	if [ ! -f "${kernel}" ]; then
+		if [ -z "${HOST_KEY_DOCUMENT}" ]; then
Quoted 8 lines

scripts/s390x/func.bash line 31

Quoted 5 lines
scripts/s390x/func.bash new file +5 -0
+			print_result 'SKIP' $testname '(no host-key document specified)'
+			return 2
+		fi
+
+		print_result 'SKIP' $testname '(PVM image was not created)'

When can that happen? Don't we already fail earlier if we specified a host key document, but genprotimg does not work?

./configure
make -j
./configure --host-key-document=…
./run_tests.sh

A contrived example, but…

Quoted 6 lines
Quoted 5 lines
scripts/s390x/func.bash new file +4 -0
+		return 2
+	fi
+	"$cmd" "${testname}_PV" "$groups pv" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+}
Signature · 8 lines
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
#16 lore raw

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

In reply to Marc Hartmayer
Quoted 110 lines · Marc Hartmayer

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.

---
 configure               |  9 +++++++++
 s390x/Makefile          | 17 +++++++++++++++--
 s390x/selftest.parmfile |  1 +
 s390x/unittests.cfg     |  1 +
 scripts/s390x/func.bash | 35 +++++++++++++++++++++++++++++++++++
 5 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100644 s390x/selftest.parmfile
 create mode 100644 scripts/s390x/func.bash
configure +9 -0
index f9d030fd2f03..0e64af58b3c1 100755
@@ -18,6 +18,7 @@ u32_long=
 vmm="qemu"
 errata_force=0
 erratatxt="$srcdir/errata.txt"
+host_key_document=
 
 usage() {
     cat <<-EOF
@@ -40,6 +41,9 @@ 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
+	                           Specify the machine-specific host-key document for creating
+	                           a PVM image with 'genprotimg' (s390x only)
 EOF
     exit 1
 }
@@ -92,6 +96,9 @@ while [[ "$1" = -* ]]; do
 	    erratatxt=
 	    [ "$arg" ] && erratatxt=$(eval realpath "$arg")
 	    ;;
+	--host-key-document)
+	    host_key_document="$arg"
+	    ;;
 	--help)
 	    usage
 	    ;;
@@ -205,6 +212,8 @@ PRETTY_PRINT_STACKS=$pretty_print_stacks
 ENVIRON_DEFAULT=$environ_default
 ERRATATXT=$erratatxt
 U32_LONG_FMT=$u32_long
+GENPROTIMG=${GENPROTIMG-genprotimg}
+HOST_KEY_DOCUMENT=$host_key_document
 EOF
 
 cat <<EOF > lib/config.h
s390x/Makefile +15 -2
index 0f54bf43bfb7..cd4e270952ec 100644
@@ -18,12 +18,19 @@ tests += $(TEST_DIR)/skrf.elf
 tests += $(TEST_DIR)/smp.elf
 tests += $(TEST_DIR)/sclp.elf
 tests += $(TEST_DIR)/css.elf
-tests_binary = $(patsubst %.elf,%.bin,$(tests))
 
-all: directories test_cases test_cases_binary
+tests_binary = $(patsubst %.elf,%.bin,$(tests))
+ifneq ($(HOST_KEY_DOCUMENT),)
+tests_pv_binary = $(patsubst %.bin,%.pv.bin,$(tests_binary))
+else
+tests_pv_binary =
+endif
+
+all: directories test_cases test_cases_binary test_cases_pv
 
 test_cases: $(tests)
 test_cases_binary: $(tests_binary)
+test_cases_pv: $(tests_pv_binary)
 
 CFLAGS += -std=gnu99
 CFLAGS += -ffreestanding
@@ -72,6 +79,12 @@ FLATLIBS = $(libcflat)
 %.bin: %.elf
 	$(OBJCOPY) -O binary  $< $@
 
+%selftest.pv.bin: %selftest.bin $(HOST_KEY_DOCUMENT) $(patsubst %.pv.bin,%.parmfile,$@)
+	$(GENPROTIMG) --host-key-document $(HOST_KEY_DOCUMENT) --parmfile $(patsubst %.pv.bin,%.parmfile,$@) --no-verify --image $< -o $@
+
+%.pv.bin: %.bin $(HOST_KEY_DOCUMENT)
+	$(GENPROTIMG) --host-key-document $(HOST_KEY_DOCUMENT) --no-verify --image $< -o $@
+
 arch_clean: asm_offsets_clean
 	$(RM) $(TEST_DIR)/*.{o,elf,bin} $(TEST_DIR)/.*.d lib/s390x/.*.d
 
s390x/selftest.parmfile new file +1 -0
new file mode 100644
index 000000000000..5613931aa5c6
@@ -0,0 +1 @@
+test 123
\ No newline at end of file
s390x/unittests.cfg +0 -0
index 0f156afbe741..12f6fb613995 100644

s390x/unittests.cfg line 25

Quoted 5 lines
s390x/unittests.cfg +1 -0
@@ -21,6 +21,7 @@
 [selftest-setup]
 file = selftest.elf
 groups = selftest
+# please keep the kernel cmdline in sync with $(TEST_DIR)/selftest.parmfile
 extra_params = -append 'test 123'

This is ugly, can't we somehow create the parmfiles dynamically?

Quoted 20 lines
s390x/unittests.cfg +0 -0
 [intercept]
scripts/s390x/func.bash new file +12 -0
new file mode 100644
index 000000000000..b2d59d0d6f25
@@ -0,0 +1,35 @@
+# The file scripts/common.bash has to be the only file sourcing this
+# arch helper file
+source config.mak
+
+ARCH_CMD=arch_cmd_s390x
+
+function arch_cmd_s390x()
+{
+	local cmd=$1
+	local testname=$2
+	local groups=$3
+	local smp=$4

scripts/s390x/func.bash line 22

Quoted 10 lines
scripts/s390x/func.bash new file +10 -0
+	local kernel=$5
+	local opts=$6
+	local arch=$7
+	local check=$8
+	local accel=$9
+	local timeout=${10}
+
+	# run the normal test case
+	"$cmd" "${testname}" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+

Why ${testname} instead of $testname ?

scripts/s390x/func.bash line 34

Quoted 12 lines
scripts/s390x/func.bash new file +12 -0
+	# run PV test case
+	kernel=${kernel%.elf}.pv.bin
+	if [ ! -f "${kernel}" ]; then
+		if [ -z "${HOST_KEY_DOCUMENT}" ]; then
+			print_result 'SKIP' $testname '(no host-key document specified)'
+			return 2
+		fi
+
+		print_result 'SKIP' $testname '(PVM image was not created)'
+		return 2
+	fi
+	"$cmd" "${testname}_PV" "$groups pv" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"

dito

Quoted 2 lines
scripts/s390x/func.bash new file +1 -0
+}
Signature · 3 lines
Thanks,

David / dhildenb
#17 lore raw

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

Quoted 110 lines · David Hildenbrand
Quoted 109 lines · Marc Hartmayer

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.

---
 configure               |  9 +++++++++
 s390x/Makefile          | 17 +++++++++++++++--
 s390x/selftest.parmfile |  1 +
 s390x/unittests.cfg     |  1 +
 scripts/s390x/func.bash | 35 +++++++++++++++++++++++++++++++++++
 5 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100644 s390x/selftest.parmfile
 create mode 100644 scripts/s390x/func.bash
configure +9 -0
index f9d030fd2f03..0e64af58b3c1 100755
@@ -18,6 +18,7 @@ u32_long=
 vmm="qemu"
 errata_force=0
 erratatxt="$srcdir/errata.txt"
+host_key_document=
 
 usage() {
     cat <<-EOF
@@ -40,6 +41,9 @@ 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
+	                           Specify the machine-specific host-key document for creating
+	                           a PVM image with 'genprotimg' (s390x only)
 EOF
     exit 1
 }
@@ -92,6 +96,9 @@ while [[ "$1" = -* ]]; do
 	    erratatxt=
 	    [ "$arg" ] && erratatxt=$(eval realpath "$arg")
 	    ;;
+	--host-key-document)
+	    host_key_document="$arg"
+	    ;;
 	--help)
 	    usage
 	    ;;
@@ -205,6 +212,8 @@ PRETTY_PRINT_STACKS=$pretty_print_stacks
 ENVIRON_DEFAULT=$environ_default
 ERRATATXT=$erratatxt
 U32_LONG_FMT=$u32_long
+GENPROTIMG=${GENPROTIMG-genprotimg}
+HOST_KEY_DOCUMENT=$host_key_document
 EOF
 
 cat <<EOF > lib/config.h
s390x/Makefile +15 -2
index 0f54bf43bfb7..cd4e270952ec 100644
@@ -18,12 +18,19 @@ tests += $(TEST_DIR)/skrf.elf
 tests += $(TEST_DIR)/smp.elf
 tests += $(TEST_DIR)/sclp.elf
 tests += $(TEST_DIR)/css.elf
-tests_binary = $(patsubst %.elf,%.bin,$(tests))
 
-all: directories test_cases test_cases_binary
+tests_binary = $(patsubst %.elf,%.bin,$(tests))
+ifneq ($(HOST_KEY_DOCUMENT),)
+tests_pv_binary = $(patsubst %.bin,%.pv.bin,$(tests_binary))
+else
+tests_pv_binary =
+endif
+
+all: directories test_cases test_cases_binary test_cases_pv
 
 test_cases: $(tests)
 test_cases_binary: $(tests_binary)
+test_cases_pv: $(tests_pv_binary)
 
 CFLAGS += -std=gnu99
 CFLAGS += -ffreestanding
@@ -72,6 +79,12 @@ FLATLIBS = $(libcflat)
 %.bin: %.elf
 	$(OBJCOPY) -O binary  $< $@
 
+%selftest.pv.bin: %selftest.bin $(HOST_KEY_DOCUMENT) $(patsubst %.pv.bin,%.parmfile,$@)
+	$(GENPROTIMG) --host-key-document $(HOST_KEY_DOCUMENT) --parmfile $(patsubst %.pv.bin,%.parmfile,$@) --no-verify --image $< -o $@
+
+%.pv.bin: %.bin $(HOST_KEY_DOCUMENT)
+	$(GENPROTIMG) --host-key-document $(HOST_KEY_DOCUMENT) --no-verify --image $< -o $@
+
 arch_clean: asm_offsets_clean
 	$(RM) $(TEST_DIR)/*.{o,elf,bin} $(TEST_DIR)/.*.d lib/s390x/.*.d
 
s390x/selftest.parmfile new file +1 -0
new file mode 100644
index 000000000000..5613931aa5c6
@@ -0,0 +1 @@
+test 123
\ No newline at end of file
s390x/unittests.cfg +0 -0
index 0f156afbe741..12f6fb613995 100644
Quoted 8 lines

s390x/unittests.cfg line 25

Quoted 6 lines
s390x/unittests.cfg +1 -0
@@ -21,6 +21,7 @@
 [selftest-setup]
 file = selftest.elf
 groups = selftest
+# please keep the kernel cmdline in sync with $(TEST_DIR)/selftest.parmfile
 extra_params = -append 'test 123'

This is ugly, can't we somehow create the parmfiles dynamically?

An alternative would be to parse the unittests.cfg (and the extra_params) in the Makefile. Not sure it's worth the effort.

Quoted 25 lines
Quoted 25 lines
s390x/unittests.cfg +0 -0
 [intercept]
scripts/s390x/func.bash new file +16 -0
new file mode 100644
index 000000000000..b2d59d0d6f25
@@ -0,0 +1,35 @@
+# The file scripts/common.bash has to be the only file sourcing this
+# arch helper file
+source config.mak
+
+ARCH_CMD=arch_cmd_s390x
+
+function arch_cmd_s390x()
+{
+	local cmd=$1
+	local testname=$2
+	local groups=$3
+	local smp=$4
+	local kernel=$5
+	local opts=$6
+	local arch=$7
+	local check=$8
Quoted 8 lines

scripts/s390x/func.bash line 22

Quoted 6 lines
scripts/s390x/func.bash new file +6 -0
+	local accel=$9
+	local timeout=${10}
+
+	# run the normal test case
+	"$cmd" "${testname}" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+

Why ${testname} instead of $testname ?

Probably to be in common with "${testname}_PV" :) I can change it here.

Quoted 15 lines

scripts/s390x/func.bash line 34

Quoted 13 lines
scripts/s390x/func.bash new file +12 -0
+	# run PV test case
+	kernel=${kernel%.elf}.pv.bin
+	if [ ! -f "${kernel}" ]; then
+		if [ -z "${HOST_KEY_DOCUMENT}" ]; then
+			print_result 'SKIP' $testname '(no host-key document specified)'
+			return 2
+		fi
+
+		print_result 'SKIP' $testname '(PVM image was not created)'
+		return 2
+	fi
+	"$cmd" "${testname}_PV" "$groups pv" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"

dito

$testname_PV would refer to a non-existent variable.

Thanks for the feedback!

Quoted 10 lines
Quoted 3 lines
scripts/s390x/func.bash new file +1 -0
+}
Signature · 3 lines
Thanks,

David / dhildenb
Signature · 8 lines
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
#18 lore raw

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

In reply to Marc Hartmayer
Quoted 140 lines · Marc Hartmayer

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.

---
 configure               |  9 +++++++++
 s390x/Makefile          | 17 +++++++++++++++--
 s390x/selftest.parmfile |  1 +
 s390x/unittests.cfg     |  1 +
 scripts/s390x/func.bash | 35 +++++++++++++++++++++++++++++++++++
 5 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100644 s390x/selftest.parmfile
 create mode 100644 scripts/s390x/func.bash
configure +9 -0
index f9d030fd2f03..0e64af58b3c1 100755
@@ -18,6 +18,7 @@ u32_long=
 vmm="qemu"
 errata_force=0
 erratatxt="$srcdir/errata.txt"
+host_key_document=
 
 usage() {
     cat <<-EOF
@@ -40,6 +41,9 @@ 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
+	                           Specify the machine-specific host-key document for creating
+	                           a PVM image with 'genprotimg' (s390x only)
 EOF
     exit 1
 }
@@ -92,6 +96,9 @@ while [[ "$1" = -* ]]; do
 	    erratatxt=
 	    [ "$arg" ] && erratatxt=$(eval realpath "$arg")
 	    ;;
+	--host-key-document)
+	    host_key_document="$arg"
+	    ;;
 	--help)
 	    usage
 	    ;;
@@ -205,6 +212,8 @@ PRETTY_PRINT_STACKS=$pretty_print_stacks
 ENVIRON_DEFAULT=$environ_default
 ERRATATXT=$erratatxt
 U32_LONG_FMT=$u32_long
+GENPROTIMG=${GENPROTIMG-genprotimg}
+HOST_KEY_DOCUMENT=$host_key_document
 EOF
 
 cat <<EOF > lib/config.h
s390x/Makefile +15 -2
index 0f54bf43bfb7..cd4e270952ec 100644
@@ -18,12 +18,19 @@ tests += $(TEST_DIR)/skrf.elf
 tests += $(TEST_DIR)/smp.elf
 tests += $(TEST_DIR)/sclp.elf
 tests += $(TEST_DIR)/css.elf
-tests_binary = $(patsubst %.elf,%.bin,$(tests))
 
-all: directories test_cases test_cases_binary
+tests_binary = $(patsubst %.elf,%.bin,$(tests))
+ifneq ($(HOST_KEY_DOCUMENT),)
+tests_pv_binary = $(patsubst %.bin,%.pv.bin,$(tests_binary))
+else
+tests_pv_binary =
+endif
+
+all: directories test_cases test_cases_binary test_cases_pv
 
 test_cases: $(tests)
 test_cases_binary: $(tests_binary)
+test_cases_pv: $(tests_pv_binary)
 
 CFLAGS += -std=gnu99
 CFLAGS += -ffreestanding
@@ -72,6 +79,12 @@ FLATLIBS = $(libcflat)
 %.bin: %.elf
 	$(OBJCOPY) -O binary  $< $@
 
+%selftest.pv.bin: %selftest.bin $(HOST_KEY_DOCUMENT) $(patsubst %.pv.bin,%.parmfile,$@)
+	$(GENPROTIMG) --host-key-document $(HOST_KEY_DOCUMENT) --parmfile $(patsubst %.pv.bin,%.parmfile,$@) --no-verify --image $< -o $@
+
+%.pv.bin: %.bin $(HOST_KEY_DOCUMENT)
+	$(GENPROTIMG) --host-key-document $(HOST_KEY_DOCUMENT) --no-verify --image $< -o $@
+
 arch_clean: asm_offsets_clean
 	$(RM) $(TEST_DIR)/*.{o,elf,bin} $(TEST_DIR)/.*.d lib/s390x/.*.d
 
s390x/selftest.parmfile new file +1 -0
new file mode 100644
index 000000000000..5613931aa5c6
@@ -0,0 +1 @@
+test 123
\ No newline at end of file
s390x/unittests.cfg +1 -0
index 0f156afbe741..12f6fb613995 100644
@@ -21,6 +21,7 @@
 [selftest-setup]
 file = selftest.elf
 groups = selftest
+# please keep the kernel cmdline in sync with $(TEST_DIR)/selftest.parmfile
 extra_params = -append 'test 123'
 
 [intercept]
scripts/s390x/func.bash new file +17 -0
new file mode 100644
index 000000000000..b2d59d0d6f25
@@ -0,0 +1,35 @@
+# The file scripts/common.bash has to be the only file sourcing this
+# arch helper file
+source config.mak
+
+ARCH_CMD=arch_cmd_s390x
+
+function arch_cmd_s390x()
+{
+	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

scripts/s390x/func.bash line 27

Quoted 10 lines
scripts/s390x/func.bash new file +10 -0
+	local timeout=${10}
+
+	# run the normal test case
+	"$cmd" "${testname}" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+
+	# run PV test case
+	kernel=${kernel%.elf}.pv.bin
+	if [ ! -f "${kernel}" ]; then
+		if [ -z "${HOST_KEY_DOCUMENT}" ]; then
+			print_result 'SKIP' $testname '(no host-key document specified)'

One argument for ‘print_result‘ is missing and $testname has to be ${testname}_PV.

scripts/s390x/func.bash line 31

Quoted 4 lines
scripts/s390x/func.bash new file +4 -0
+			return 2
+		fi
+
+		print_result 'SKIP' $testname '(PVM image was not created)'

…and here as well. I’ll fix it.

Quoted 7 lines
scripts/s390x/func.bash new file +4 -0
+		return 2
+	fi
+	"$cmd" "${testname}_PV" "$groups pv" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+}
Signature · 1 line
2.25.4
Signature · 8 lines
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