Re: [kvm-unit-tests PATCH v2 1/1] s390x: do not enable PV dump support by default

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Wed, 19 Oct 2022 16:53:20 +0200
Nico Boehr <nrb@linux.ibm.com> wrote:

> Currently, dump support is always enabled by setting the respective
> plaintext control flag (PCF). Unfortunately, older machines without
> support for PV dump will not start the guest when this PCF is set. This
> will result in an error message like this:
> 
> qemu-system-s390x: KVM PV command 2 (KVM_PV_SET_SEC_PARMS) failed: header rc 106 rrc 0 IOCTL rc: -22
> 
> Hence, by default, disable dump support to preserve compatibility with
> older machines. Users can enable dumping support by passing
> --enable-dump to the configure script.
> 
> Fixes: 3043685825d9 ("s390x: create persistent comm-key")
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  configure      | 11 +++++++++++
>  s390x/Makefile | 26 +++++++++++++++++---------
>  2 files changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/configure b/configure
> index 5b7daac3c6e8..b81f20942c9c 100755
> --- a/configure
> +++ b/configure
> @@ -28,6 +28,7 @@ errata_force=0
>  erratatxt="$srcdir/errata.txt"
>  host_key_document=
>  gen_se_header=
> +enable_dump=no
>  page_size=
>  earlycon=
>  efi=
> @@ -67,6 +68,9 @@ usage() {
>  	    --gen-se-header=GEN_SE_HEADER
>  	                           Provide an executable to generate a PV header
>  	                           requires --host-key-document. (s390x-snippets only)
> +	    --[enable|disable]-dump
> +	                           Allow PV guests to be dumped. Requires at least z16.
> +	                           (s390x only)
>  	    --page-size=PAGE_SIZE
>  	                           Specify the page size (translation granule) (4k, 16k or
>  	                           64k, default is 64k, arm64 only)
> @@ -146,6 +150,12 @@ while [[ "$1" = -* ]]; do
>  	--gen-se-header)
>  	    gen_se_header="$arg"
>  	    ;;
> +	--enable-dump)
> +	    enable_dump=yes
> +	    ;;
> +	--disable-dump)
> +	    enable_dump=no
> +	    ;;
>  	--page-size)
>  	    page_size="$arg"
>  	    ;;
> @@ -387,6 +397,7 @@ U32_LONG_FMT=$u32_long
>  WA_DIVIDE=$wa_divide
>  GENPROTIMG=${GENPROTIMG-genprotimg}
>  HOST_KEY_DOCUMENT=$host_key_document
> +CONFIG_DUMP=$enable_dump
>  CONFIG_EFI=$efi
>  CONFIG_WERROR=$werror
>  GEN_SE_HEADER=$gen_se_header
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 649486f2d4a0..271b6803a1c5 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -173,18 +173,26 @@ $(comm-key):
>  %.bin: %.elf
>  	$(OBJCOPY) -O binary  $< $@
>  
> -# The genprotimg arguments for the cck changed over time so we need to
> -# figure out which argument to use in order to set the cck
> -GENPROTIMG_HAS_COMM_KEY = $(shell $(GENPROTIMG) --help | grep -q -- --comm-key && echo yes)
> -ifeq ($(GENPROTIMG_HAS_COMM_KEY),yes)
> -	GENPROTIMG_COMM_KEY = --comm-key $(comm-key)
> -else
> -	GENPROTIMG_COMM_KEY = --x-comm-key $(comm-key)
> +# Will only be filled when dump has been enabled
> +GENPROTIMG_COMM_KEY =
> +# allow PCKMO
> +genprotimg_pcf = 0x000000e0
> +
> +ifeq ($(CONFIG_DUMP),yes)
> +	# The genprotimg arguments for the cck changed over time so we need to
> +	# figure out which argument to use in order to set the cck
> +	GENPROTIMG_HAS_COMM_KEY = $(shell $(GENPROTIMG) --help | grep -q -- --comm-key && echo yes)
> +	ifeq ($(GENPROTIMG_HAS_COMM_KEY),yes)
> +		GENPROTIMG_COMM_KEY = --comm-key $(comm-key)
> +	else
> +		GENPROTIMG_COMM_KEY = --x-comm-key $(comm-key)
> +	endif
> +
> +	# allow dumping + PCKMO
> +	genprotimg_pcf = 0x200000e0
>  endif
>  
>  # use x-pcf to be compatible with old genprotimg versions
> -# allow dumping + PCKMO
> -genprotimg_pcf = 0x200000e0
>  genprotimg_args = --host-key-document $(HOST_KEY_DOCUMENT) --no-verify $(GENPROTIMG_COMM_KEY) --x-pcf $(genprotimg_pcf)
>  
>  %selftest.pv.bin: %selftest.bin $(HOST_KEY_DOCUMENT) $(patsubst %.pv.bin,%.parmfile,$@) $(comm-key)