Re: [XEN PATCH v3 04/25] tools/firmware/hvmloader: rework Makefile

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.06.2022 18:04, Anthony PERARD wrote:
> Setup proper dependencies with libacpi so we don't need to run "make
> hvmloader" in the "all" target. ("build.o" new prerequisite isn't
> exactly proper but a side effect of building the $(DSDT_FILES) is to
> generate the "ssdt_*.h" needed by "build.o".)

Maybe leave a brief comment there?

> Make use if "-iquote" instead of a plain "-I".
> 
> For "roms.inc" target, use "$(SHELL)" instead of plain "sh". And use
> full path to "mkhex" instead of a relative one. Lastly, add "-f" flag
> to "mv", in case the target already exist.

Hmm - according to my understanding -f isn't needed just because the
file may already exist. It would be needed if a pre-existing file
isn't writable. (I don't mind the addition of the flag, but I think
what you say can end up misleading.)

Jan

> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
>  tools/firmware/hvmloader/Makefile | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/firmware/hvmloader/Makefile 
> b/tools/firmware/hvmloader/Makefile
> index b754220839..fc20932110 100644
> --- a/tools/firmware/hvmloader/Makefile
> +++ b/tools/firmware/hvmloader/Makefile
> @@ -60,8 +60,7 @@ ROMS += $(ROMBIOS_ROM) $(STDVGA_ROM) $(CIRRUSVGA_ROM)
>  endif
>  
>  .PHONY: all
> -all: acpi
> -     $(MAKE) hvmloader
> +all: hvmloader
>  
>  .PHONY: acpi
>  acpi:
> @@ -73,12 +72,15 @@ smbios.o: CFLAGS += 
> -D__SMBIOS_DATE__="\"$(SMBIOS_REL_DATE)\""
>  ACPI_PATH = ../../libacpi
>  DSDT_FILES = dsdt_anycpu.c dsdt_15cpu.c dsdt_anycpu_qemu_xen.c
>  ACPI_OBJS = $(patsubst %.c,%.o,$(DSDT_FILES)) build.o static_tables.o
> -$(ACPI_OBJS): CFLAGS += -I. -DLIBACPI_STDUTILS=\"$(CURDIR)/util.h\"
> +$(ACPI_OBJS): CFLAGS += -iquote . -DLIBACPI_STDUTILS=\"$(CURDIR)/util.h\"
>  CFLAGS += -I$(ACPI_PATH)
>  vpath build.c $(ACPI_PATH)
>  vpath static_tables.c $(ACPI_PATH)
>  OBJS += $(ACPI_OBJS)
>  
> +$(DSDT_FILES): acpi
> +build.o: $(DSDT_FILES)
> +
>  hvmloader: $(OBJS) hvmloader.lds
>       $(LD) $(LDFLAGS_DIRECT) -N -T hvmloader.lds -o $@ $(OBJS)
>  
> @@ -87,21 +89,21 @@ roms.inc: $(ROMS)
>  
>  ifneq ($(ROMBIOS_ROM),)
>       echo "#ifdef ROM_INCLUDE_ROMBIOS" >> $@.new
> -     sh ../../misc/mkhex rombios $(ROMBIOS_ROM) >> $@.new
> +     $(SHELL) $(XEN_ROOT)/tools/misc/mkhex rombios $(ROMBIOS_ROM) >> $@.new
>       echo "#endif" >> $@.new
>  endif
>  
>  ifneq ($(STDVGA_ROM),)
>       echo "#ifdef ROM_INCLUDE_VGABIOS" >> $@.new
> -     sh ../../misc/mkhex vgabios_stdvga $(STDVGA_ROM) >> $@.new
> +     $(SHELL) $(XEN_ROOT)/tools/misc/mkhex vgabios_stdvga $(STDVGA_ROM) >> 
> $@.new
>       echo "#endif" >> $@.new
>  endif
>  ifneq ($(CIRRUSVGA_ROM),)
>       echo "#ifdef ROM_INCLUDE_VGABIOS" >> $@.new
> -     sh ../../misc/mkhex vgabios_cirrusvga $(CIRRUSVGA_ROM) >> $@.new
> +     $(SHELL) $(XEN_ROOT)/tools/misc/mkhex vgabios_cirrusvga 
> $(CIRRUSVGA_ROM) >> $@.new
>       echo "#endif" >> $@.new
>  endif
> -     mv $@.new $@
> +     mv -f $@.new $@
>  
>  .PHONY: clean
>  clean: