diff mbox series

[kvm-unit-tests,v1] s390x: do not enable PV dump support by default

Message ID 20221018111528.173989-1-nrb@linux.ibm.com (mailing list archive)
State Superseded
Headers show
Series [kvm-unit-tests,v1] s390x: do not enable PV dump support by default | expand

Commit Message

Nico Boehr Oct. 18, 2022, 11:15 a.m. UTC
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 30 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>
---
 configure      | 11 +++++++++++
 s390x/Makefile |  9 ++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

Comments

Janosch Frank Oct. 19, 2022, 2:45 p.m. UTC | #1
On 10/18/22 13:15, Nico Boehr 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 30 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.
> 

The patch is fine, some nits below.

But I'm wondering when the core maintainers will start complaining about 
excessive argument usage. :)

> @@ -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..5b4aff5e57ef 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -173,6 +173,11 @@ $(comm-key):
>   %.bin: %.elf
>   	$(OBJCOPY) -O binary  $< $@
>   
> +GENPROTIMG_COMM_KEY =

This should have a comment:
Will only be filled when dump has been enabled

> +# allow PCKMO
> +genprotimg_pcf = 0x000000e0
> +
> +ifeq ($(CONFIG_DUMP),yes)

I'd appreciate indents here

>   # 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)
> @@ -182,9 +187,11 @@ else
>   	GENPROTIMG_COMM_KEY = --x-comm-key $(comm-key)
>   endif
>   
> -# use x-pcf to be compatible with old genprotimg versions
>   # allow dumping + PCKMO
>   genprotimg_pcf = 0x200000e0
> +endif
> +
> +# use x-pcf to be compatible with old genprotimg versions
>   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)
diff mbox series

Patch

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..5b4aff5e57ef 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -173,6 +173,11 @@  $(comm-key):
 %.bin: %.elf
 	$(OBJCOPY) -O binary  $< $@
 
+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)
@@ -182,9 +187,11 @@  else
 	GENPROTIMG_COMM_KEY = --x-comm-key $(comm-key)
 endif
 
-# use x-pcf to be compatible with old genprotimg versions
 # allow dumping + PCKMO
 genprotimg_pcf = 0x200000e0
+endif
+
+# use x-pcf to be compatible with old genprotimg versions
 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)