Re: [kvm-unit-tests PATCH v1 1/3] s390x/Makefile: snippets: Add separate target for the ELF snippets

Nicholas Piggin
2024-06-05T01:21:09Z

On Tue Jun 4, 2024 at 9:59 PM AEST, Marc Hartmayer wrote:
> It's unusual to create multiple files in one target rule, and it's even more
> unusual to create an ELF file with a `.gbin` file extension first, and then
> overwrite it in the next step. It might even lead to errors as the input file
> path is also used as the output file path - but this depends on the objcopy
> implementation. Therefore, create an extra target for the ELF files and list it
> as a prerequisite for the *.gbin targets.

I had some pain trying to figure out another ("pretty printing") patch
that changed some s390x/Makefile because of this. As far as I can tell
it looks good.

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

>
> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
> ---
> s390x/Makefile | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 23342bd64f44..784818b2883e 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -153,14 +153,18 @@ $(SNIPPET_DIR)/asm/%.o: $(SNIPPET_DIR)/asm/%.S $(asm-offsets)
> $(SNIPPET_DIR)/c/%.o: $(SNIPPET_DIR)/c/%.c $(asm-offsets)
> $(CC) $(CFLAGS) -c -nostdlib -o $@ $<
>
> -$(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o $(SNIPPET_DIR)/asm/flat.lds
> +$(SNIPPET_DIR)/asm/%.elf: $(SNIPPET_DIR)/asm/%.o $(SNIPPET_DIR)/asm/flat.lds
> $(CC) $(LDFLAGS) -o $@ -T $(SNIPPET_DIR)/asm/flat.lds $<
> - $(OBJCOPY) -O binary -j ".rodata" -j ".lowcore" -j ".text" -j ".data" -j ".bss" --set-section-flags .bss=alloc,load,contents $@ $@
> +
> +$(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.elf
> + $(OBJCOPY) -O binary -j ".rodata" -j ".lowcore" -j ".text" -j ".data" -j ".bss" --set-section-flags .bss=alloc,load,contents $< $@
> truncate -s '%4096' $@
>
> -$(SNIPPET_DIR)/c/%.gbin: $(SNIPPET_DIR)/c/%.o $(snippet_lib) $(FLATLIBS) $(SNIPPET_DIR)/c/flat.lds
> +$(SNIPPET_DIR)/c/%.elf: $(SNIPPET_DIR)/c/%.o $(snippet_lib) $(FLATLIBS) $(SNIPPET_DIR)/c/flat.lds
> $(CC) $(LDFLAGS) -o $@ -T $(SNIPPET_DIR)/c/flat.lds $< $(snippet_lib) $(FLATLIBS)
> - $(OBJCOPY) -O binary -j ".rodata" -j ".lowcore" -j ".text" -j ".data" -j ".bss" --set-section-flags .bss=alloc,load,contents $@ $@
> +
> +$(SNIPPET_DIR)/c/%.gbin: $(SNIPPET_DIR)/c/%.elf
> + $(OBJCOPY) -O binary -j ".rodata" -j ".lowcore" -j ".text" -j ".data" -j ".bss" --set-section-flags .bss=alloc,load,contents $< $@
> truncate -s '%4096' $@
>
> %.hdr: %.gbin $(HOST_KEY_DOCUMENT)