* [kvm-unit-tests RFC] s390x: Add Protected VM support
@ 2020-05-06 12:46 ` Marc Hartmayer
0 siblings, 0 replies; 10+ messages in thread
From: Marc Hartmayer @ 2020-05-06 12:46 UTC (permalink / raw)
To: kvm
Cc: Thomas Huth, David Hildenbrand, Janosch Frank, Cornelia Huck,
Andrew Jones, Paolo Bonzini, Christian Borntraeger, linux-s390
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>
---
.gitignore | 1 +
configure | 8 ++++++++
s390x/Makefile | 16 +++++++++++++---
s390x/unittests.cfg | 20 ++++++++++++++++++++
scripts/common.bash | 30 +++++++++++++++++++++++++++++-
5 files changed, 71 insertions(+), 4 deletions(-)
diff --git a/.gitignore b/.gitignore
index 784cb2ddbcb8..1fa5c0c0ea76 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,6 +4,7 @@
*.o
*.flat
*.elf
+*.img
.pc
patches
.stgit-*
diff --git a/configure b/configure
index 5d2cd90cd180..29191f4b0994 100755
--- a/configure
+++ b/configure
@@ -18,6 +18,7 @@ u32_long=
vmm="qemu"
errata_force=0
erratatxt="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)
EOF
exit 1
}
@@ -91,6 +94,9 @@ while [[ "$1" = -* ]]; do
--erratatxt)
erratatxt="$arg"
;;
+ --host-key-document)
+ host_key_document="$arg"
+ ;;
--help)
usage
;;
@@ -207,6 +213,8 @@ PRETTY_PRINT_STACKS=$pretty_print_stacks
ENVIRON_DEFAULT=$environ_default
ERRATATXT=$erratatxt
U32_LONG_FMT=$u32_long
+GENPROTIMG=genprotimg
+HOST_KEY_DOCUMENT=$host_key_document
EOF
cat <<EOF > lib/config.h
diff --git a/s390x/Makefile b/s390x/Makefile
index ddb4b48ecbf9..a57655dcce10 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -17,12 +17,19 @@ tests += $(TEST_DIR)/stsi.elf
tests += $(TEST_DIR)/skrf.elf
tests += $(TEST_DIR)/smp.elf
tests += $(TEST_DIR)/sclp.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_img = $(patsubst %.elf,%.pv.img,$(tests))
+else
+tests_pv_img =
+endif
+
+all: directories test_cases test_cases_binary test_cases_pv
test_cases: $(tests)
test_cases_binary: $(tests_binary)
+test_cases_pv: $(tests_pv_img)
CFLAGS += -std=gnu99
CFLAGS += -ffreestanding
@@ -68,8 +75,11 @@ FLATLIBS = $(libcflat)
%.bin: %.elf
$(OBJCOPY) -O binary $< $@
+%.pv.img: %.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
+ $(RM) $(TEST_DIR)/*.{o,elf,bin,img} $(TEST_DIR)/.*.d lib/s390x/.*.d
generated-files = $(asm-offsets)
$(tests:.elf=.o) $(cstart.o) $(cflatobjs): $(generated-files)
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index b307329354f6..6beaca45fb20 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -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.
##############################################################################
[selftest-setup]
@@ -25,62 +27,80 @@ extra_params = -append 'test 123'
[intercept]
file = intercept.elf
+pv_support = 1
[emulator]
file = emulator.elf
+pv_support = 1
[sieve]
file = sieve.elf
groups = selftest
# can take fairly long when KVM is nested inside z/VM
timeout = 600
+pv_support = 1
[sthyi]
file = sthyi.elf
+pv_support = 1
[skey]
file = skey.elf
+pv_support = 1
[diag10]
file = diag10.elf
+pv_support = 1
[diag308]
file = diag308.elf
+pv_support = 1
[pfmf]
file = pfmf.elf
+pv_support = 1
[cmm]
file = cmm.elf
+pv_support = 1
[vector]
file = vector.elf
+pv_support = 1
[gs]
file = gs.elf
+pv_support = 1
[iep]
file = iep.elf
+pv_support = 1
[cpumodel]
file = cpumodel.elf
+pv_support = 1
[diag288]
file = diag288.elf
extra_params=-device diag288,id=watchdog0 --watchdog-action inject-nmi
+pv_support = 1
[stsi]
file = stsi.elf
extra_params=-name kvm-unit-test --uuid 0fb84a86-727c-11ea-bc55-0242ac130003 -smp 1,maxcpus=8
+pv_support = 1
[smp]
file = smp.elf
smp = 2
+pv_support = 1
[sclp-1g]
file = sclp.elf
extra_params = -m 1G
+pv_support = 1
[sclp-3g]
file = sclp.elf
extra_params = -m 3G
+pv_support = 1
diff --git a/scripts/common.bash b/scripts/common.bash
index 9a6ebbd7f287..971d0661287c 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -1,3 +1,20 @@
+function pv_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.img
+ # do not run the PV test cases by default
+ "$cmd" "${testname}_PV" "$groups pv nodefault" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+}
function for_each_unittest()
{
@@ -12,12 +29,16 @@ function for_each_unittest()
local check
local accel
local timeout
+ local pv_support
exec {fd}<"$unittests"
while read -r -u $fd line; do
if [[ "$line" =~ ^\[(.*)\]$ ]]; then
- "$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
+
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [kvm-unit-tests RFC] s390x: Add Protected VM support
@ 2020-05-06 12:46 ` Marc Hartmayer
0 siblings, 0 replies; 10+ messages in thread
From: Marc Hartmayer @ 2020-05-06 12:46 UTC (permalink / raw)
To: kvm
Cc: Thomas Huth, David Hildenbrand, Janosch Frank, Cornelia Huck,
Andrew Jones, Paolo Bonzini, Christian Borntraeger, linux-s390
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>
---
.gitignore | 1 +
configure | 8 ++++++++
s390x/Makefile | 16 +++++++++++++---
s390x/unittests.cfg | 20 ++++++++++++++++++++
scripts/common.bash | 30 +++++++++++++++++++++++++++++-
5 files changed, 71 insertions(+), 4 deletions(-)
diff --git a/.gitignore b/.gitignore
index 784cb2ddbcb8..1fa5c0c0ea76 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,6 +4,7 @@
*.o
*.flat
*.elf
+*.img
.pc
patches
.stgit-*
diff --git a/configure b/configure
index 5d2cd90cd180..29191f4b0994 100755
--- a/configure
+++ b/configure
@@ -18,6 +18,7 @@ u32_long=
vmm="qemu"
errata_force=0
erratatxt="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)
EOF
exit 1
}
@@ -91,6 +94,9 @@ while [[ "$1" = -* ]]; do
--erratatxt)
erratatxt="$arg"
;;
+ --host-key-document)
+ host_key_document="$arg"
+ ;;
--help)
usage
;;
@@ -207,6 +213,8 @@ PRETTY_PRINT_STACKS=$pretty_print_stacks
ENVIRON_DEFAULT=$environ_default
ERRATATXT=$erratatxt
U32_LONG_FMT=$u32_long
+GENPROTIMG=genprotimg
+HOST_KEY_DOCUMENT=$host_key_document
EOF
cat <<EOF > lib/config.h
diff --git a/s390x/Makefile b/s390x/Makefile
index ddb4b48ecbf9..a57655dcce10 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -17,12 +17,19 @@ tests += $(TEST_DIR)/stsi.elf
tests += $(TEST_DIR)/skrf.elf
tests += $(TEST_DIR)/smp.elf
tests += $(TEST_DIR)/sclp.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_img = $(patsubst %.elf,%.pv.img,$(tests))
+else
+tests_pv_img =
+endif
+
+all: directories test_cases test_cases_binary test_cases_pv
test_cases: $(tests)
test_cases_binary: $(tests_binary)
+test_cases_pv: $(tests_pv_img)
CFLAGS += -std=gnu99
CFLAGS += -ffreestanding
@@ -68,8 +75,11 @@ FLATLIBS = $(libcflat)
%.bin: %.elf
$(OBJCOPY) -O binary $< $@
+%.pv.img: %.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
+ $(RM) $(TEST_DIR)/*.{o,elf,bin,img} $(TEST_DIR)/.*.d lib/s390x/.*.d
generated-files = $(asm-offsets)
$(tests:.elf=.o) $(cstart.o) $(cflatobjs): $(generated-files)
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index b307329354f6..6beaca45fb20 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -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.
##############################################################################
[selftest-setup]
@@ -25,62 +27,80 @@ extra_params = -append 'test 123'
[intercept]
file = intercept.elf
+pv_support = 1
[emulator]
file = emulator.elf
+pv_support = 1
[sieve]
file = sieve.elf
groups = selftest
# can take fairly long when KVM is nested inside z/VM
timeout = 600
+pv_support = 1
[sthyi]
file = sthyi.elf
+pv_support = 1
[skey]
file = skey.elf
+pv_support = 1
[diag10]
file = diag10.elf
+pv_support = 1
[diag308]
file = diag308.elf
+pv_support = 1
[pfmf]
file = pfmf.elf
+pv_support = 1
[cmm]
file = cmm.elf
+pv_support = 1
[vector]
file = vector.elf
+pv_support = 1
[gs]
file = gs.elf
+pv_support = 1
[iep]
file = iep.elf
+pv_support = 1
[cpumodel]
file = cpumodel.elf
+pv_support = 1
[diag288]
file = diag288.elf
extra_params=-device diag288,id=watchdog0 --watchdog-action inject-nmi
+pv_support = 1
[stsi]
file = stsi.elf
extra_params=-name kvm-unit-test --uuid 0fb84a86-727c-11ea-bc55-0242ac130003 -smp 1,maxcpus=8
+pv_support = 1
[smp]
file = smp.elf
smp = 2
+pv_support = 1
[sclp-1g]
file = sclp.elf
extra_params = -m 1G
+pv_support = 1
[sclp-3g]
file = sclp.elf
extra_params = -m 3G
+pv_support = 1
diff --git a/scripts/common.bash b/scripts/common.bash
index 9a6ebbd7f287..971d0661287c 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -1,3 +1,20 @@
+function pv_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.img
+ # do not run the PV test cases by default
+ "$cmd" "${testname}_PV" "$groups pv nodefault" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+}
function for_each_unittest()
{
@@ -12,12 +29,16 @@ function for_each_unittest()
local check
local accel
local timeout
+ local pv_support
exec {fd}<"$unittests"
while read -r -u $fd line; do
if [[ "$line" =~ ^\[(.*)\]$ ]]; then
- "$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
+
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests RFC] s390x: Add Protected VM support
2020-05-06 12:46 ` Marc Hartmayer
(?)
@ 2020-05-06 13:50 ` Andrew Jones
2020-05-07 13:14 ` Marc Hartmayer
2020-05-07 13:40 ` Marc Hartmayer
-1 siblings, 2 replies; 10+ messages in thread
From: Andrew Jones @ 2020-05-06 13:50 UTC (permalink / raw)
To: Marc Hartmayer
Cc: kvm, Thomas Huth, David Hildenbrand, Janosch Frank,
Cornelia Huck, Paolo Bonzini, Christian Borntraeger, linux-s390
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>
> ---
> .gitignore | 1 +
> configure | 8 ++++++++
> s390x/Makefile | 16 +++++++++++++---
> s390x/unittests.cfg | 20 ++++++++++++++++++++
> scripts/common.bash | 30 +++++++++++++++++++++++++++++-
> 5 files changed, 71 insertions(+), 4 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index 784cb2ddbcb8..1fa5c0c0ea76 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -4,6 +4,7 @@
> *.o
> *.flat
> *.elf
> +*.img
> .pc
> patches
> .stgit-*
> diff --git a/configure b/configure
> index 5d2cd90cd180..29191f4b0994 100755
> --- a/configure
> +++ b/configure
> @@ -18,6 +18,7 @@ u32_long=
> vmm="qemu"
> errata_force=0
> erratatxt="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)
> EOF
> exit 1
> }
> @@ -91,6 +94,9 @@ while [[ "$1" = -* ]]; do
> --erratatxt)
> erratatxt="$arg"
> ;;
> + --host-key-document)
> + host_key_document="$arg"
> + ;;
> --help)
> usage
> ;;
> @@ -207,6 +213,8 @@ PRETTY_PRINT_STACKS=$pretty_print_stacks
> ENVIRON_DEFAULT=$environ_default
> ERRATATXT=$erratatxt
> U32_LONG_FMT=$u32_long
> +GENPROTIMG=genprotimg
> +HOST_KEY_DOCUMENT=$host_key_document
> EOF
>
> cat <<EOF > lib/config.h
> diff --git a/s390x/Makefile b/s390x/Makefile
> index ddb4b48ecbf9..a57655dcce10 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -17,12 +17,19 @@ tests += $(TEST_DIR)/stsi.elf
> tests += $(TEST_DIR)/skrf.elf
> tests += $(TEST_DIR)/smp.elf
> tests += $(TEST_DIR)/sclp.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_img = $(patsubst %.elf,%.pv.img,$(tests))
> +else
> +tests_pv_img =
> +endif
> +
> +all: directories test_cases test_cases_binary test_cases_pv
>
> test_cases: $(tests)
> test_cases_binary: $(tests_binary)
> +test_cases_pv: $(tests_pv_img)
>
> CFLAGS += -std=gnu99
> CFLAGS += -ffreestanding
> @@ -68,8 +75,11 @@ FLATLIBS = $(libcflat)
> %.bin: %.elf
> $(OBJCOPY) -O binary $< $@
>
> +%.pv.img: %.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
> + $(RM) $(TEST_DIR)/*.{o,elf,bin,img} $(TEST_DIR)/.*.d lib/s390x/.*.d
>
> generated-files = $(asm-offsets)
> $(tests:.elf=.o) $(cstart.o) $(cflatobjs): $(generated-files)
> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index b307329354f6..6beaca45fb20 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -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?
> ##############################################################################
>
> [selftest-setup]
> @@ -25,62 +27,80 @@ extra_params = -append 'test 123'
>
> [intercept]
> file = intercept.elf
> +pv_support = 1
>
> [emulator]
> file = emulator.elf
> +pv_support = 1
>
> [sieve]
> file = sieve.elf
> groups = selftest
> # can take fairly long when KVM is nested inside z/VM
> timeout = 600
> +pv_support = 1
>
> [sthyi]
> file = sthyi.elf
> +pv_support = 1
>
> [skey]
> file = skey.elf
> +pv_support = 1
>
> [diag10]
> file = diag10.elf
> +pv_support = 1
>
> [diag308]
> file = diag308.elf
> +pv_support = 1
>
> [pfmf]
> file = pfmf.elf
> +pv_support = 1
>
> [cmm]
> file = cmm.elf
> +pv_support = 1
>
> [vector]
> file = vector.elf
> +pv_support = 1
>
> [gs]
> file = gs.elf
> +pv_support = 1
>
> [iep]
> file = iep.elf
> +pv_support = 1
>
> [cpumodel]
> file = cpumodel.elf
> +pv_support = 1
>
> [diag288]
> file = diag288.elf
> extra_params=-device diag288,id=watchdog0 --watchdog-action inject-nmi
> +pv_support = 1
>
> [stsi]
> file = stsi.elf
> extra_params=-name kvm-unit-test --uuid 0fb84a86-727c-11ea-bc55-0242ac130003 -smp 1,maxcpus=8
> +pv_support = 1
>
> [smp]
> file = smp.elf
> smp = 2
> +pv_support = 1
>
> [sclp-1g]
> file = sclp.elf
> extra_params = -m 1G
> +pv_support = 1
>
> [sclp-3g]
> file = sclp.elf
> extra_params = -m 3G
> +pv_support = 1
> diff --git a/scripts/common.bash b/scripts/common.bash
> index 9a6ebbd7f287..971d0661287c 100644
> --- a/scripts/common.bash
> +++ b/scripts/common.bash
> @@ -1,3 +1,20 @@
> +function pv_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.img
> + # do not run the PV test cases by default
> + "$cmd" "${testname}_PV" "$groups pv nodefault" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> +}
>
> function for_each_unittest()
> {
> @@ -12,12 +29,16 @@ function for_each_unittest()
> local check
> local accel
> local timeout
> + local pv_support
>
> exec {fd}<"$unittests"
>
> while read -r -u $fd line; do
> if [[ "$line" =~ ^\[(.*)\]$ ]]; then
> - "$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
> +
> 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. Why not do this stuff in s390x/run instead? 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?
Thanks,
drew
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests RFC] s390x: Add Protected VM support
2020-05-06 12:46 ` Marc Hartmayer
(?)
(?)
@ 2020-05-06 14:03 ` Janosch Frank
2020-05-06 14:05 ` David Hildenbrand
2020-05-07 12:30 ` Marc Hartmayer
-1 siblings, 2 replies; 10+ messages in thread
From: Janosch Frank @ 2020-05-06 14:03 UTC (permalink / raw)
To: Marc Hartmayer, kvm
Cc: Thomas Huth, David Hildenbrand, Cornelia Huck, Andrew Jones,
Paolo Bonzini, Christian Borntraeger, linux-s390
[-- Attachment #1.1: Type: text/plain, Size: 4563 bytes --]
On 5/6/20 2:46 PM, 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>
> ---
> .gitignore | 1 +
> configure | 8 ++++++++
> s390x/Makefile | 16 +++++++++++++---
> s390x/unittests.cfg | 20 ++++++++++++++++++++
> scripts/common.bash | 30 +++++++++++++++++++++++++++++-
> 5 files changed, 71 insertions(+), 4 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index 784cb2ddbcb8..1fa5c0c0ea76 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -4,6 +4,7 @@
> *.o
> *.flat
> *.elf
> +*.img
> .pc
> patches
> .stgit-*
> diff --git a/configure b/configure
> index 5d2cd90cd180..29191f4b0994 100755
> --- a/configure
> +++ b/configure
> @@ -18,6 +18,7 @@ u32_long=
> vmm="qemu"
> errata_force=0
> erratatxt="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)
> EOF
> exit 1
> }
> @@ -91,6 +94,9 @@ while [[ "$1" = -* ]]; do
> --erratatxt)
> erratatxt="$arg"
> ;;
> + --host-key-document)
> + host_key_document="$arg"
> + ;;
> --help)
> usage
> ;;
> @@ -207,6 +213,8 @@ PRETTY_PRINT_STACKS=$pretty_print_stacks
> ENVIRON_DEFAULT=$environ_default
> ERRATATXT=$erratatxt
> U32_LONG_FMT=$u32_long
> +GENPROTIMG=genprotimg
> +HOST_KEY_DOCUMENT=$host_key_document
> EOF
>
> cat <<EOF > lib/config.h
> diff --git a/s390x/Makefile b/s390x/Makefile
> index ddb4b48ecbf9..a57655dcce10 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -17,12 +17,19 @@ tests += $(TEST_DIR)/stsi.elf
> tests += $(TEST_DIR)/skrf.elf
> tests += $(TEST_DIR)/smp.elf
> tests += $(TEST_DIR)/sclp.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_img = $(patsubst %.elf,%.pv.img,$(tests))
> +else
> +tests_pv_img =
> +endif
> +
> +all: directories test_cases test_cases_binary test_cases_pv
>
> test_cases: $(tests)
> test_cases_binary: $(tests_binary)
> +test_cases_pv: $(tests_pv_img)
>
> CFLAGS += -std=gnu99
> CFLAGS += -ffreestanding
> @@ -68,8 +75,11 @@ FLATLIBS = $(libcflat)
> %.bin: %.elf
> $(OBJCOPY) -O binary $< $@
>
> +%.pv.img: %.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
> + $(RM) $(TEST_DIR)/*.{o,elf,bin,img} $(TEST_DIR)/.*.d lib/s390x/.*.d
>
> generated-files = $(asm-offsets)
> $(tests:.elf=.o) $(cstart.o) $(cflatobjs): $(generated-files)
> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index b307329354f6..6beaca45fb20 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -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.
> ##############################################################################
>
> [selftest-setup]
> @@ -25,62 +27,80 @@ extra_params = -append 'test 123'
>
> [intercept]
> file = intercept.elf
> +pv_support = 1
So, let's do this discussion once more:
Why would we need a opt-in for something which works on all our current
tests? I'd much rather have a opt-out or just a bail-out when running
the test like I already implemented for the storage key related tests...
I don't see any benefit for this right now other than forcing me to add
another line to this file that was not needed before..
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests RFC] s390x: Add Protected VM support
2020-05-06 14:03 ` Janosch Frank
@ 2020-05-06 14:05 ` David Hildenbrand
2020-05-06 14:26 ` Janosch Frank
2020-05-07 12:30 ` Marc Hartmayer
1 sibling, 1 reply; 10+ messages in thread
From: David Hildenbrand @ 2020-05-06 14:05 UTC (permalink / raw)
To: Janosch Frank, Marc Hartmayer, kvm
Cc: Thomas Huth, Cornelia Huck, Andrew Jones, Paolo Bonzini,
Christian Borntraeger, linux-s390
On 06.05.20 16:03, Janosch Frank wrote:
> On 5/6/20 2:46 PM, 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>
>> ---
>> .gitignore | 1 +
>> configure | 8 ++++++++
>> s390x/Makefile | 16 +++++++++++++---
>> s390x/unittests.cfg | 20 ++++++++++++++++++++
>> scripts/common.bash | 30 +++++++++++++++++++++++++++++-
>> 5 files changed, 71 insertions(+), 4 deletions(-)
>>
>> diff --git a/.gitignore b/.gitignore
>> index 784cb2ddbcb8..1fa5c0c0ea76 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -4,6 +4,7 @@
>> *.o
>> *.flat
>> *.elf
>> +*.img
>> .pc
>> patches
>> .stgit-*
>> diff --git a/configure b/configure
>> index 5d2cd90cd180..29191f4b0994 100755
>> --- a/configure
>> +++ b/configure
>> @@ -18,6 +18,7 @@ u32_long=
>> vmm="qemu"
>> errata_force=0
>> erratatxt="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)
>> EOF
>> exit 1
>> }
>> @@ -91,6 +94,9 @@ while [[ "$1" = -* ]]; do
>> --erratatxt)
>> erratatxt="$arg"
>> ;;
>> + --host-key-document)
>> + host_key_document="$arg"
>> + ;;
>> --help)
>> usage
>> ;;
>> @@ -207,6 +213,8 @@ PRETTY_PRINT_STACKS=$pretty_print_stacks
>> ENVIRON_DEFAULT=$environ_default
>> ERRATATXT=$erratatxt
>> U32_LONG_FMT=$u32_long
>> +GENPROTIMG=genprotimg
>> +HOST_KEY_DOCUMENT=$host_key_document
>> EOF
>>
>> cat <<EOF > lib/config.h
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index ddb4b48ecbf9..a57655dcce10 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -17,12 +17,19 @@ tests += $(TEST_DIR)/stsi.elf
>> tests += $(TEST_DIR)/skrf.elf
>> tests += $(TEST_DIR)/smp.elf
>> tests += $(TEST_DIR)/sclp.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_img = $(patsubst %.elf,%.pv.img,$(tests))
>> +else
>> +tests_pv_img =
>> +endif
>> +
>> +all: directories test_cases test_cases_binary test_cases_pv
>>
>> test_cases: $(tests)
>> test_cases_binary: $(tests_binary)
>> +test_cases_pv: $(tests_pv_img)
>>
>> CFLAGS += -std=gnu99
>> CFLAGS += -ffreestanding
>> @@ -68,8 +75,11 @@ FLATLIBS = $(libcflat)
>> %.bin: %.elf
>> $(OBJCOPY) -O binary $< $@
>>
>> +%.pv.img: %.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
>> + $(RM) $(TEST_DIR)/*.{o,elf,bin,img} $(TEST_DIR)/.*.d lib/s390x/.*.d
>>
>> generated-files = $(asm-offsets)
>> $(tests:.elf=.o) $(cstart.o) $(cflatobjs): $(generated-files)
>> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
>> index b307329354f6..6beaca45fb20 100644
>> --- a/s390x/unittests.cfg
>> +++ b/s390x/unittests.cfg
>> @@ -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.
>> ##############################################################################
>>
>> [selftest-setup]
>> @@ -25,62 +27,80 @@ extra_params = -append 'test 123'
>>
>> [intercept]
>> file = intercept.elf
>> +pv_support = 1
>
> So, let's do this discussion once more:
> Why would we need a opt-in for something which works on all our current
> tests? I'd much rather have a opt-out or just a bail-out when running
> the test like I already implemented for the storage key related tests...
>
> I don't see any benefit for this right now other than forcing me to add
> another line to this file that was not needed before..
>
Exactly my thought. I would assume that most tests that properly test
for feature availability should just work?
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests RFC] s390x: Add Protected VM support
2020-05-06 14:05 ` David Hildenbrand
@ 2020-05-06 14:26 ` Janosch Frank
0 siblings, 0 replies; 10+ messages in thread
From: Janosch Frank @ 2020-05-06 14:26 UTC (permalink / raw)
To: David Hildenbrand, Marc Hartmayer, kvm
Cc: Thomas Huth, Cornelia Huck, Andrew Jones, Paolo Bonzini,
Christian Borntraeger, linux-s390
[-- Attachment #1.1: Type: text/plain, Size: 5192 bytes --]
On 5/6/20 4:05 PM, David Hildenbrand wrote:
> On 06.05.20 16:03, Janosch Frank wrote:
>> On 5/6/20 2:46 PM, 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>
>>> ---
>>> .gitignore | 1 +
>>> configure | 8 ++++++++
>>> s390x/Makefile | 16 +++++++++++++---
>>> s390x/unittests.cfg | 20 ++++++++++++++++++++
>>> scripts/common.bash | 30 +++++++++++++++++++++++++++++-
>>> 5 files changed, 71 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/.gitignore b/.gitignore
>>> index 784cb2ddbcb8..1fa5c0c0ea76 100644
>>> --- a/.gitignore
>>> +++ b/.gitignore
>>> @@ -4,6 +4,7 @@
>>> *.o
>>> *.flat
>>> *.elf
>>> +*.img
>>> .pc
>>> patches
>>> .stgit-*
>>> diff --git a/configure b/configure
>>> index 5d2cd90cd180..29191f4b0994 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -18,6 +18,7 @@ u32_long=
>>> vmm="qemu"
>>> errata_force=0
>>> erratatxt="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)
>>> EOF
>>> exit 1
>>> }
>>> @@ -91,6 +94,9 @@ while [[ "$1" = -* ]]; do
>>> --erratatxt)
>>> erratatxt="$arg"
>>> ;;
>>> + --host-key-document)
>>> + host_key_document="$arg"
>>> + ;;
>>> --help)
>>> usage
>>> ;;
>>> @@ -207,6 +213,8 @@ PRETTY_PRINT_STACKS=$pretty_print_stacks
>>> ENVIRON_DEFAULT=$environ_default
>>> ERRATATXT=$erratatxt
>>> U32_LONG_FMT=$u32_long
>>> +GENPROTIMG=genprotimg
>>> +HOST_KEY_DOCUMENT=$host_key_document
>>> EOF
>>>
>>> cat <<EOF > lib/config.h
>>> diff --git a/s390x/Makefile b/s390x/Makefile
>>> index ddb4b48ecbf9..a57655dcce10 100644
>>> --- a/s390x/Makefile
>>> +++ b/s390x/Makefile
>>> @@ -17,12 +17,19 @@ tests += $(TEST_DIR)/stsi.elf
>>> tests += $(TEST_DIR)/skrf.elf
>>> tests += $(TEST_DIR)/smp.elf
>>> tests += $(TEST_DIR)/sclp.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_img = $(patsubst %.elf,%.pv.img,$(tests))
>>> +else
>>> +tests_pv_img =
>>> +endif
>>> +
>>> +all: directories test_cases test_cases_binary test_cases_pv
>>>
>>> test_cases: $(tests)
>>> test_cases_binary: $(tests_binary)
>>> +test_cases_pv: $(tests_pv_img)
>>>
>>> CFLAGS += -std=gnu99
>>> CFLAGS += -ffreestanding
>>> @@ -68,8 +75,11 @@ FLATLIBS = $(libcflat)
>>> %.bin: %.elf
>>> $(OBJCOPY) -O binary $< $@
>>>
>>> +%.pv.img: %.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
>>> + $(RM) $(TEST_DIR)/*.{o,elf,bin,img} $(TEST_DIR)/.*.d lib/s390x/.*.d
>>>
>>> generated-files = $(asm-offsets)
>>> $(tests:.elf=.o) $(cstart.o) $(cflatobjs): $(generated-files)
>>> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
>>> index b307329354f6..6beaca45fb20 100644
>>> --- a/s390x/unittests.cfg
>>> +++ b/s390x/unittests.cfg
>>> @@ -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.
>>> ##############################################################################
>>>
>>> [selftest-setup]
>>> @@ -25,62 +27,80 @@ extra_params = -append 'test 123'
>>>
>>> [intercept]
>>> file = intercept.elf
>>> +pv_support = 1
>>
>> So, let's do this discussion once more:
>> Why would we need a opt-in for something which works on all our current
>> tests? I'd much rather have a opt-out or just a bail-out when running
>> the test like I already implemented for the storage key related tests...
>>
>> I don't see any benefit for this right now other than forcing me to add
>> another line to this file that was not needed before..
>>
>
> Exactly my thought. I would assume that most tests that properly test
> for feature availability should just work?
>
Yes
At the beginning of firmware development it was sometimes necessary to
blacklist some tests, but now all of them run or bail-out.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests RFC] s390x: Add Protected VM support
2020-05-06 14:03 ` Janosch Frank
2020-05-06 14:05 ` David Hildenbrand
@ 2020-05-07 12:30 ` Marc Hartmayer
2020-05-07 12:34 ` Christian Borntraeger
1 sibling, 1 reply; 10+ messages in thread
From: Marc Hartmayer @ 2020-05-07 12:30 UTC (permalink / raw)
To: Janosch Frank, Marc Hartmayer, kvm
Cc: Thomas Huth, David Hildenbrand, Cornelia Huck, Andrew Jones,
Paolo Bonzini, Christian Borntraeger, linux-s390
On Wed, May 06, 2020 at 04:03 PM +0200, Janosch Frank <frankja@linux.ibm.com> wrote:
> On 5/6/20 2:46 PM, 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>
[…snip…]
>> [intercept]
>> file = intercept.elf
>> +pv_support = 1
>
> So, let's do this discussion once more:
> Why would we need a opt-in for something which works on all our current
> tests? I'd much rather have a opt-out or just a bail-out when running
> the test like I already implemented for the storage key related
> tests...
>
> I don't see any benefit for this right now other than forcing me to add
> another line to this file that was not needed before..
>
Okay. So shall I add an option ’pv_not_supported’? Or simply assume that
the actual test cases will handle it?
--
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests RFC] s390x: Add Protected VM support
2020-05-07 12:30 ` Marc Hartmayer
@ 2020-05-07 12:34 ` Christian Borntraeger
0 siblings, 0 replies; 10+ messages in thread
From: Christian Borntraeger @ 2020-05-07 12:34 UTC (permalink / raw)
To: Marc Hartmayer, Janosch Frank, kvm
Cc: Thomas Huth, David Hildenbrand, Cornelia Huck, Andrew Jones,
Paolo Bonzini, linux-s390
On 07.05.20 14:30, Marc Hartmayer wrote:
> On Wed, May 06, 2020 at 04:03 PM +0200, Janosch Frank <frankja@linux.ibm.com> wrote:
>> On 5/6/20 2:46 PM, 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>
>
> […snip…]
>
>>> [intercept]
>>> file = intercept.elf
>>> +pv_support = 1
>>
>> So, let's do this discussion once more:
>> Why would we need a opt-in for something which works on all our current
>> tests? I'd much rather have a opt-out or just a bail-out when running
>> the test like I already implemented for the storage key related
>> tests...
>>
>> I don't see any benefit for this right now other than forcing me to add
>> another line to this file that was not needed before..
>>
>
> Okay. So shall I add an option ’pv_not_supported’? Or simply assume that
> the actual test cases will handle it?
I would suggest to fix the testcases (e.g. do an early exit when we are running
secure and we know it does not work)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests RFC] s390x: Add Protected VM support
2020-05-06 13:50 ` Andrew Jones
@ 2020-05-07 13:14 ` Marc Hartmayer
2020-05-07 13:40 ` Marc Hartmayer
1 sibling, 0 replies; 10+ messages in thread
From: Marc Hartmayer @ 2020-05-07 13:14 UTC (permalink / raw)
To: Andrew Jones, Marc Hartmayer
Cc: kvm, Thomas Huth, David Hildenbrand, Janosch Frank,
Cornelia Huck, Paolo Bonzini, Christian Borntraeger, linux-s390
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests RFC] s390x: Add Protected VM support
2020-05-06 13:50 ` Andrew Jones
2020-05-07 13:14 ` Marc Hartmayer
@ 2020-05-07 13:40 ` Marc Hartmayer
1 sibling, 0 replies; 10+ messages in thread
From: Marc Hartmayer @ 2020-05-07 13:40 UTC (permalink / raw)
To: Andrew Jones, Marc Hartmayer
Cc: kvm, Thomas Huth, David Hildenbrand, Janosch Frank,
Cornelia Huck, Paolo Bonzini, Christian Borntraeger, linux-s390
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>
>> ---
[…snip…]
> +
>> "$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. Why not do this stuff in s390x/run instead?
Okay, I’ve looked into the code, and the reason for this approach is
that I want to treat the PVM and the “normal” test case as two separate
test cases, but using the same test configuration. I don’t see how I can
achieve this by editing s390x/run and for the standalone case.
Maybe this approach is already broken and I should simply add the PVM
testcases as extra test cases to the unittest.cfg - but this would
result in duplicated code in the configuration file.
> 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?
>
> Thanks,
> drew
>
--
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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-05-07 13:40 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-06 12:46 [kvm-unit-tests RFC] s390x: Add Protected VM support Marc Hartmayer
2020-05-06 12:46 ` Marc Hartmayer
2020-05-06 13:50 ` Andrew Jones
2020-05-07 13:14 ` Marc Hartmayer
2020-05-07 13:40 ` Marc Hartmayer
2020-05-06 14:03 ` Janosch Frank
2020-05-06 14:05 ` David Hildenbrand
2020-05-06 14:26 ` Janosch Frank
2020-05-07 12:30 ` Marc Hartmayer
2020-05-07 12:34 ` Christian Borntraeger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.