Re: [Xen-devel] [PATCH v1 12/20] acpi/hvmloader: Link ACPI object files directly

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.07.16 at 21:05, <boris.ostrovsky@xxxxxxxxxx> wrote:
> @@ -95,7 +96,15 @@ all: subdirs-all
>  ovmf.o rombios.o seabios.o hvmloader.o: roms.inc
>  smbios.o: CFLAGS += -D__SMBIOS_DATE__="\"$(SMBIOS_REL_DATE)\""
>  
> -hvmloader: $(OBJS) acpi/acpi.a
> +ACPI_PATH = $(XEN_ROOT)/tools/firmware/hvmloader/acpi

I think I did say so already on the RFC series: I'd much prefer if we
could stick to relative references, so trees have a better chances of
being movable.

> +ACPI_FILES = dsdt_anycpu.c dsdt_15cpu.c dsdt_anycpu_qemu_xen.c build.c 
> static_tables.c
> +ACPI_OBJS = $(patsubst %.c,%.o,$(ACPI_FILES))
> +$(ACPI_OBJS): CFLAGS += -I$(ACPI_PATH) -I.

What is the -I. needed for? The ACPI files shouldn't be referencing
other hvmloader files, and generated .h files should be included
using "" instead of <>.

> +vpath build.c $(ACPI_PATH)/
> +vpath static_tables.c $(ACPI_PATH)/

I don't think the trailing slashes are needed here.

> --- a/tools/firmware/hvmloader/acpi/Makefile
> +++ b/tools/firmware/hvmloader/acpi/Makefile
> @@ -17,36 +17,41 @@
>  XEN_ROOT = $(CURDIR)/../../../..
>  include $(XEN_ROOT)/tools/firmware/Rules.mk
>  
> -C_SRC = build.c dsdt_anycpu.c dsdt_15cpu.c static_tables.c 
> dsdt_anycpu_qemu_xen.c
> -OBJS  = $(patsubst %.c,%.o,$(C_SRC))
> +MK_DSDT = $(ACPI_BUILD_DIR)/mk_dsdt
>  
> -CFLAGS += $(CFLAGS_xeninclude)
> +# Sources to be generated
> +C_SRC = $(ACPI_BUILD_DIR)/dsdt_anycpu.c $(ACPI_BUILD_DIR)/dsdt_15cpu.c 
> $(ACPI_BUILD_DIR)/dsdt_anycpu_qemu_xen.c
> +H_SRC = $(ACPI_BUILD_DIR)/ssdt_s3.h $(ACPI_BUILD_DIR)/ssdt_s4.h 
> $(ACPI_BUILD_DIR)/ssdt_pm.h $(ACPI_BUILD_DIR)/ssdt_tpm.h

Use $(addprefix ) to limit line length?

>  vpath iasl $(PATH)
> -all: acpi.a
> +all: $(C_SRC) $(H_SRC)
>  
> -ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h: %.h: %.asl iasl
> +$(H_SRC): $(ACPI_BUILD_DIR)/%.h: %.asl iasl
> +     cd $(ACPI_BUILD_DIR)
>       iasl -vs -p $* -tc $<
>       sed -e 's/AmlCode/$*/g' $*.hex >$@
>       rm -f $*.hex $*.aml
> +     cd $(CURDIR)

cd in a make rule feel wrong. Could you prefix most of the $* with
$(ACPI_BUILD_DIR)/ instead?

> @@ -56,14 +61,8 @@ iasl:
>       @echo 
>       @exit 1
>  
> -build.o: ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h
> -
> -acpi.a: $(OBJS)
> -     $(AR) rc $@ $(OBJS)
> -
>  clean:
> -     rm -rf *.a *.o $(IASL_VER) $(IASL_VER).tar.gz $(DEPS)
> -     rm -rf ssdt_*.h dsdt*.c *~ *.aml *.hex mk_dsdt dsdt_*.asl
> +     rm -fr $(C_SRC) $(H_SRC) $(MK_DSDT) $(patsubst %.c,%.asl,$(C_SRC))

It seems to me that this would better done in from the consumer's
clean rule.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel