* [kvm-unit-tests PATCH v2 0/1] s390x: do not enable PV dump support by default @ 2022-10-19 14:53 Nico Boehr 2022-10-19 14:53 ` [kvm-unit-tests PATCH v2 1/1] " Nico Boehr 2022-10-19 15:19 ` [kvm-unit-tests PATCH v2 0/1] " Claudio Imbrenda 0 siblings, 2 replies; 9+ messages in thread From: Nico Boehr @ 2022-10-19 14:53 UTC (permalink / raw) To: kvm; +Cc: frankja, imbrenda, thuth v1->v2: --- * add indent to CONFIG_DUMP if in Makefile (thanks Janosch) * add comment (thanks Janosch) 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. Nico Boehr (1): s390x: do not enable PV dump support by default configure | 11 +++++++++++ s390x/Makefile | 26 +++++++++++++++++--------- 2 files changed, 28 insertions(+), 9 deletions(-) -- 2.36.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [kvm-unit-tests PATCH v2 1/1] s390x: do not enable PV dump support by default 2022-10-19 14:53 [kvm-unit-tests PATCH v2 0/1] s390x: do not enable PV dump support by default Nico Boehr @ 2022-10-19 14:53 ` Nico Boehr 2022-10-19 15:13 ` Claudio Imbrenda 2022-10-20 6:56 ` Janosch Frank 2022-10-19 15:19 ` [kvm-unit-tests PATCH v2 0/1] " Claudio Imbrenda 1 sibling, 2 replies; 9+ messages in thread From: Nico Boehr @ 2022-10-19 14:53 UTC (permalink / raw) To: kvm; +Cc: frankja, imbrenda, thuth 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> --- 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) -- 2.36.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests PATCH v2 1/1] s390x: do not enable PV dump support by default 2022-10-19 14:53 ` [kvm-unit-tests PATCH v2 1/1] " Nico Boehr @ 2022-10-19 15:13 ` Claudio Imbrenda 2022-10-20 6:56 ` Janosch Frank 1 sibling, 0 replies; 9+ messages in thread From: Claudio Imbrenda @ 2022-10-19 15:13 UTC (permalink / raw) To: Nico Boehr; +Cc: kvm, frankja, thuth 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) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests PATCH v2 1/1] s390x: do not enable PV dump support by default 2022-10-19 14:53 ` [kvm-unit-tests PATCH v2 1/1] " Nico Boehr 2022-10-19 15:13 ` Claudio Imbrenda @ 2022-10-20 6:56 ` Janosch Frank 1 sibling, 0 replies; 9+ messages in thread From: Janosch Frank @ 2022-10-20 6:56 UTC (permalink / raw) To: Nico Boehr, kvm; +Cc: imbrenda, thuth On 10/19/22 16:53, 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 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. > Reviewed-by: Janosch Frank <frankja@linux.ibm.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests PATCH v2 0/1] s390x: do not enable PV dump support by default 2022-10-19 14:53 [kvm-unit-tests PATCH v2 0/1] s390x: do not enable PV dump support by default Nico Boehr 2022-10-19 14:53 ` [kvm-unit-tests PATCH v2 1/1] " Nico Boehr @ 2022-10-19 15:19 ` Claudio Imbrenda 2022-10-19 15:24 ` Nico Boehr 2022-10-19 15:47 ` Janosch Frank 1 sibling, 2 replies; 9+ messages in thread From: Claudio Imbrenda @ 2022-10-19 15:19 UTC (permalink / raw) To: Nico Boehr; +Cc: kvm, frankja, thuth On Wed, 19 Oct 2022 16:53:19 +0200 Nico Boehr <nrb@linux.ibm.com> wrote: > v1->v2: > --- > * add indent to CONFIG_DUMP if in Makefile (thanks Janosch) > * add comment (thanks Janosch) > > 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. maybe for the long term we could try to fix the stub generated by genprotimg to check the plaintext flags and the available features and refuse to try to start if the required features are missing. ideally providing a custom message when generating the image, to be shown if the required features are missing. e.g. for kvm unit test, the custom message could be something like SKIP: $TEST_NAME: Missing hardware features once that is in place, we could revert this patch > > Nico Boehr (1): > s390x: do not enable PV dump support by default > > configure | 11 +++++++++++ > s390x/Makefile | 26 +++++++++++++++++--------- > 2 files changed, 28 insertions(+), 9 deletions(-) > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests PATCH v2 0/1] s390x: do not enable PV dump support by default 2022-10-19 15:19 ` [kvm-unit-tests PATCH v2 0/1] " Claudio Imbrenda @ 2022-10-19 15:24 ` Nico Boehr 2022-10-19 16:03 ` Claudio Imbrenda 2022-10-19 15:47 ` Janosch Frank 1 sibling, 1 reply; 9+ messages in thread From: Nico Boehr @ 2022-10-19 15:24 UTC (permalink / raw) To: Claudio Imbrenda; +Cc: kvm, frankja, thuth Quoting Claudio Imbrenda (2022-10-19 17:19:43) > On Wed, 19 Oct 2022 16:53:19 +0200 > Nico Boehr <nrb@linux.ibm.com> wrote: > > > v1->v2: > > --- > > * add indent to CONFIG_DUMP if in Makefile (thanks Janosch) > > * add comment (thanks Janosch) > > > > 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. > > maybe for the long term we could try to fix the stub generated by > genprotimg to check the plaintext flags and the available features and > refuse to try to start if the required features are missing. > > ideally providing a custom message when generating the image, to be > shown if the required features are missing. e.g. for kvm unit test, the > custom message could be something like > SKIP: $TEST_NAME: Missing hardware features > > once that is in place, we could revert this patch But that would mean that on machines which don't support dumping, PV tests will never run, will they? So we need some way of specifing at compile time whether you want dump support or not. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests PATCH v2 0/1] s390x: do not enable PV dump support by default 2022-10-19 15:24 ` Nico Boehr @ 2022-10-19 16:03 ` Claudio Imbrenda 0 siblings, 0 replies; 9+ messages in thread From: Claudio Imbrenda @ 2022-10-19 16:03 UTC (permalink / raw) To: Nico Boehr; +Cc: kvm, frankja, thuth On Wed, 19 Oct 2022 17:24:16 +0200 Nico Boehr <nrb@linux.ibm.com> wrote: > Quoting Claudio Imbrenda (2022-10-19 17:19:43) > > On Wed, 19 Oct 2022 16:53:19 +0200 > > Nico Boehr <nrb@linux.ibm.com> wrote: > > > > > v1->v2: > > > --- > > > * add indent to CONFIG_DUMP if in Makefile (thanks Janosch) > > > * add comment (thanks Janosch) > > > > > > 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. > > > > maybe for the long term we could try to fix the stub generated by > > genprotimg to check the plaintext flags and the available features and > > refuse to try to start if the required features are missing. > > > > ideally providing a custom message when generating the image, to be > > shown if the required features are missing. e.g. for kvm unit test, the > > custom message could be something like > > SKIP: $TEST_NAME: Missing hardware features > > > > once that is in place, we could revert this patch > > But that would mean that on machines which don't support dumping, PV tests will never run, will they? no, the check would be done at run time, so the test would only be skipped on machines that don't support dumping (or whatever other feature) but again, this is a long term idea, for now we'll take your patch since it solves the problem :) > > So we need some way of specifing at compile time whether you want dump support or not. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests PATCH v2 0/1] s390x: do not enable PV dump support by default 2022-10-19 15:19 ` [kvm-unit-tests PATCH v2 0/1] " Claudio Imbrenda 2022-10-19 15:24 ` Nico Boehr @ 2022-10-19 15:47 ` Janosch Frank 2022-10-19 16:17 ` Claudio Imbrenda 1 sibling, 1 reply; 9+ messages in thread From: Janosch Frank @ 2022-10-19 15:47 UTC (permalink / raw) To: Claudio Imbrenda, Nico Boehr; +Cc: kvm, thuth On 10/19/22 17:19, Claudio Imbrenda wrote: > On Wed, 19 Oct 2022 16:53:19 +0200 > Nico Boehr <nrb@linux.ibm.com> wrote: > >> v1->v2: >> --- >> * add indent to CONFIG_DUMP if in Makefile (thanks Janosch) >> * add comment (thanks Janosch) >> >> 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. > > maybe for the long term we could try to fix the stub generated by > genprotimg to check the plaintext flags and the available features and > refuse to try to start if the required features are missing. That's not possible on multiple levels: * Unsecure G2 does not have stfle 158 * Dump is a host feature so I'm unsure if it would even be indicated in the guest > > ideally providing a custom message when generating the image, to be > shown if the required features are missing. e.g. for kvm unit test, the > custom message could be something like > SKIP: $TEST_NAME: Missing hardware features > > once that is in place, we could revert this patch Also the host that's using genprotimg might not be PV enabled or even s390x so checking on image generation is no option either. > >> >> Nico Boehr (1): >> s390x: do not enable PV dump support by default >> >> configure | 11 +++++++++++ >> s390x/Makefile | 26 +++++++++++++++++--------- >> 2 files changed, 28 insertions(+), 9 deletions(-) >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests PATCH v2 0/1] s390x: do not enable PV dump support by default 2022-10-19 15:47 ` Janosch Frank @ 2022-10-19 16:17 ` Claudio Imbrenda 0 siblings, 0 replies; 9+ messages in thread From: Claudio Imbrenda @ 2022-10-19 16:17 UTC (permalink / raw) To: Janosch Frank; +Cc: Nico Boehr, kvm, thuth On Wed, 19 Oct 2022 17:47:36 +0200 Janosch Frank <frankja@linux.ibm.com> wrote: > On 10/19/22 17:19, Claudio Imbrenda wrote: > > On Wed, 19 Oct 2022 16:53:19 +0200 > > Nico Boehr <nrb@linux.ibm.com> wrote: > > > >> v1->v2: > >> --- > >> * add indent to CONFIG_DUMP if in Makefile (thanks Janosch) > >> * add comment (thanks Janosch) > >> > >> 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. > > > > maybe for the long term we could try to fix the stub generated by > > genprotimg to check the plaintext flags and the available features and > > refuse to try to start if the required features are missing. > > That's not possible on multiple levels: > * Unsecure G2 does not have stfle 158 > * Dump is a host feature so I'm unsure if it would even be indicated in > the guest fair enough. maybe those are problems that can be somehow fixed in the (far) future. for example when loading the blob with diag 308 subcode 8, the host could reject it with an appropriate error code (since it would fail to boot anyway) not something we have to worry about now, though > > > > > ideally providing a custom message when generating the image, to be > > shown if the required features are missing. e.g. for kvm unit test, the > > custom message could be something like > > SKIP: $TEST_NAME: Missing hardware features > > > > once that is in place, we could revert this patch > > Also the host that's using genprotimg might not be PV enabled or even > s390x so checking on image generation is no option either. checking at image generation time is exactly what I did not want. > > > > >> > >> Nico Boehr (1): > >> s390x: do not enable PV dump support by default > >> > >> configure | 11 +++++++++++ > >> s390x/Makefile | 26 +++++++++++++++++--------- > >> 2 files changed, 28 insertions(+), 9 deletions(-) > >> > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-10-20 6:56 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-19 14:53 [kvm-unit-tests PATCH v2 0/1] s390x: do not enable PV dump support by default Nico Boehr 2022-10-19 14:53 ` [kvm-unit-tests PATCH v2 1/1] " Nico Boehr 2022-10-19 15:13 ` Claudio Imbrenda 2022-10-20 6:56 ` Janosch Frank 2022-10-19 15:19 ` [kvm-unit-tests PATCH v2 0/1] " Claudio Imbrenda 2022-10-19 15:24 ` Nico Boehr 2022-10-19 16:03 ` Claudio Imbrenda 2022-10-19 15:47 ` Janosch Frank 2022-10-19 16:17 ` Claudio Imbrenda
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.