diff mbox series

[kvm-unit-tests] s390x: Support newer version of genprotimg

Message ID 20241205160011.100609-1-mhartmay@linux.ibm.com (mailing list archive)
State New
Headers show
Series [kvm-unit-tests] s390x: Support newer version of genprotimg | expand

Commit Message

Marc Hartmayer Dec. 5, 2024, 4 p.m. UTC 
Since s390-tools commit f4cf4ae6ebb1 ("rust: Add a new tool called 'pvimg'") the
genprotimg command checks if a given image/kernel is a s390x Linux kernel, and
it does no longer overwrite the output file by default. Disable the component
check, since a KUT test is being prepared, and use the '--overwrite' option to
overwrite the output.

Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
---
 s390x/Makefile | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)


base-commit: 0ed2cdf3c80ee803b9150898e687e77e4d6f5db2

Comments

Nico Boehr Dec. 9, 2024, 7:42 a.m. UTC | #1 
On Thu Dec 5, 2024 at 5:00 PM CET, Marc Hartmayer wrote:
> Since s390-tools commit f4cf4ae6ebb1 ("rust: Add a new tool called 'pvimg'") the
> genprotimg command checks if a given image/kernel is a s390x Linux kernel, and
> it does no longer overwrite the output file by default. Disable the component
> check, since a KUT test is being prepared, and use the '--overwrite' option to
> overwrite the output.
>
> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>

Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
Nico Boehr Dec. 9, 2024, 7:59 a.m. UTC | #2 
On Thu Dec 5, 2024 at 5:00 PM CET, Marc Hartmayer wrote:
[...]
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 23342bd64f44..3da3bebb6775 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -197,17 +197,26 @@ $(comm-key):
>  %.bin: %.elf
>  	$(OBJCOPY) -O binary  $< $@
>  
> +define test_genprotimg_opt
> +$(shell $(GENPROTIMG) --help | grep -q -- "$1" && echo yes || echo no)
> +endef
> +
> +GENPROTIMG_DEFAULT_ARGS := --no-verify
> +ifneq ($(HOST_KEY_DOCUMENT),)
>  # 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
> -ifneq ($(HOST_KEY_DOCUMENT),)
> -GENPROTIMG_HAS_COMM_KEY = $(shell $(GENPROTIMG) --help | grep -q -- --comm-key && echo yes)
> -ifeq ($(GENPROTIMG_HAS_COMM_KEY),yes)
> +ifeq ($(call test_genprotimg_opt,--comm-key),yes)
>  	GENPROTIMG_COMM_OPTION := --comm-key
>  else
>  	GENPROTIMG_COMM_OPTION := --x-comm-key
>  endif
> -else
> -GENPROTIMG_HAS_COMM_KEY =
> +# Newer version of the genprotimg command checks if the given image/kernel is a

After having my first cup of coffee, one question: at which version did this behaviour change?
Marc Hartmayer Dec. 9, 2024, 9:34 a.m. UTC | #3 
On Mon, Dec 09, 2024 at 08:59 AM +0100, "Nico Boehr" <nrb@linux.ibm.com> wrote:
> On Thu Dec 5, 2024 at 5:00 PM CET, Marc Hartmayer wrote:
> [...]
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index 23342bd64f44..3da3bebb6775 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -197,17 +197,26 @@ $(comm-key):
>>  %.bin: %.elf
>>  	$(OBJCOPY) -O binary  $< $@
>>  
>> +define test_genprotimg_opt
>> +$(shell $(GENPROTIMG) --help | grep -q -- "$1" && echo yes || echo no)
>> +endef
>> +
>> +GENPROTIMG_DEFAULT_ARGS := --no-verify
>> +ifneq ($(HOST_KEY_DOCUMENT),)
>>  # 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
>> -ifneq ($(HOST_KEY_DOCUMENT),)
>> -GENPROTIMG_HAS_COMM_KEY = $(shell $(GENPROTIMG) --help | grep -q -- --comm-key && echo yes)
>> -ifeq ($(GENPROTIMG_HAS_COMM_KEY),yes)
>> +ifeq ($(call test_genprotimg_opt,--comm-key),yes)
>>  	GENPROTIMG_COMM_OPTION := --comm-key
>>  else
>>  	GENPROTIMG_COMM_OPTION := --x-comm-key
>>  endif
>> -else
>> -GENPROTIMG_HAS_COMM_KEY =
>> +# Newer version of the genprotimg command checks if the given image/kernel is a
>
> After having my first cup of coffee, one question: at which version
> did this behaviour change?

2.36.0
(https://github.com/ibm-s390-linux/s390-tools/commit/0cd063e40d12d7ca5bc59a09b2ee4803653678bd)
Nico Boehr Dec. 9, 2024, 9:46 a.m. UTC | #4 
On Mon Dec 9, 2024 at 10:34 AM CET, Marc Hartmayer wrote:
> 2.36.0
> (https://github.com/ibm-s390-linux/s390-tools/commit/0cd063e40d12d7ca5bc59a09b2ee4803653678bd)

Thanks, pushed to devel for CI coverage, this should also unbreak PV tests.
Janosch Frank Dec. 9, 2024, 10:05 a.m. UTC | #5 
On 12/5/24 5:00 PM, Marc Hartmayer wrote:
> Since s390-tools commit f4cf4ae6ebb1 ("rust: Add a new tool called 'pvimg'") the
> genprotimg command checks if a given image/kernel is a s390x Linux kernel, and
> it does no longer overwrite the output file by default. Disable the component
> check, since a KUT test is being prepared, and use the '--overwrite' option to
> overwrite the output.
> 
> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>

I'm okish with this since there will be no changes for users that 
convert linux kernels AFAIK. So we're the odd one out anyway.

Just be prepared that I might not be as lenient next time. This is the 
second time that we have to introduce conditional arguments and the make 
file only gets messier every time.
diff mbox series

Patch

diff --git a/s390x/Makefile b/s390x/Makefile
index 23342bd64f44..3da3bebb6775 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -197,17 +197,26 @@  $(comm-key):
 %.bin: %.elf
 	$(OBJCOPY) -O binary  $< $@
 
+define test_genprotimg_opt
+$(shell $(GENPROTIMG) --help | grep -q -- "$1" && echo yes || echo no)
+endef
+
+GENPROTIMG_DEFAULT_ARGS := --no-verify
+ifneq ($(HOST_KEY_DOCUMENT),)
 # 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
-ifneq ($(HOST_KEY_DOCUMENT),)
-GENPROTIMG_HAS_COMM_KEY = $(shell $(GENPROTIMG) --help | grep -q -- --comm-key && echo yes)
-ifeq ($(GENPROTIMG_HAS_COMM_KEY),yes)
+ifeq ($(call test_genprotimg_opt,--comm-key),yes)
 	GENPROTIMG_COMM_OPTION := --comm-key
 else
 	GENPROTIMG_COMM_OPTION := --x-comm-key
 endif
-else
-GENPROTIMG_HAS_COMM_KEY =
+# Newer version of the genprotimg command checks if the given image/kernel is a
+# s390x Linux kernel and it does not overwrite the output file by default.
+# Disable the component check, since a KUT test is being prepared, and always
+# overwrite the output.
+ifeq ($(call test_genprotimg_opt,--overwrite),yes)
+	GENPROTIMG_DEFAULT_ARGS += --overwrite --no-component-check
+endif
 endif
 
 ifeq ($(CONFIG_DUMP),yes)
@@ -221,7 +230,7 @@  endif
 $(patsubst %.parmfile,%.pv.bin,$(wildcard s390x/*.parmfile)): %.pv.bin: %.parmfile
 %.pv.bin: %.bin $(HOST_KEY_DOCUMENT) $(comm-key)
 	$(eval parmfile_args = $(if $(filter %.parmfile,$^),--parmfile $(filter %.parmfile,$^),))
-	$(GENPROTIMG) --host-key-document $(HOST_KEY_DOCUMENT) --no-verify $(GENPROTIMG_COMM_OPTION) $(comm-key) --x-pcf $(GENPROTIMG_PCF) $(parmfile_args) --image $(filter %.bin,$^) -o $@
+	$(GENPROTIMG) $(GENPROTIMG_DEFAULT_ARGS) --host-key-document $(HOST_KEY_DOCUMENT) $(GENPROTIMG_COMM_OPTION) $(comm-key) --x-pcf $(GENPROTIMG_PCF) $(parmfile_args) --image $(filter %.bin,$^) -o $@
 
 $(snippet_asmlib): $$(patsubst %.o,%.S,$$@) $(asm-offsets)
 	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<