On Wed, Aug 19, 2020 at 12:42 PM +0200, David Hildenbrand <david@xxxxxxxxxx> wrote:
> On 18.08.20 15:04, 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@xxxxxxxxxxxxx>
>> ---
>> 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
>>
>> diff --git a/configure b/configure
>> index f9d030fd2f03..0e64af58b3c1 100755
>> --- a/configure
>> +++ b/configure
>> @@ -18,6 +18,7 @@ u32_long=
>> vmm="qemu"
>> errata_force=0
>> erratatxt="$srcdir/errata.txt"
>> +host_key_document=
>>
>> usage() {
>> cat <<-EOF
>> @@ -40,6 +41,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
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index 0f54bf43bfb7..cd4e270952ec 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -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
>>
>> diff --git a/s390x/selftest.parmfile b/s390x/selftest.parmfile
>> new file mode 100644
>> index 000000000000..5613931aa5c6
>> --- /dev/null
>> +++ b/s390x/selftest.parmfile
>> @@ -0,0 +1 @@
>> +test 123
>> \ No newline at end of file
>> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
>> index 0f156afbe741..12f6fb613995 100644
>> --- a/s390x/unittests.cfg
>> +++ b/s390x/unittests.cfg
>> @@ -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.
>
>>
>> [intercept]
>> diff --git a/scripts/s390x/func.bash b/scripts/s390x/func.bash
>> new file mode 100644
>> index 000000000000..b2d59d0d6f25
>> --- /dev/null
>> +++ b/scripts/s390x/func.bash
>> @@ -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"
>> +
>
> Why ${testname} instead of $testname ?
Probably to be in common with "${testname}_PV" :) I can change it here.
>
>> + # 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!
>
>> +}
>>
>
>
> --
> Thanks,
>
> David / dhildenb
>
--
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
