Re: [Xen-devel] [RFC PATCH v2 04/16] firmware/makefile: install BIOS and ACPI blob ...

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Mon, 2015-10-26 at 16:03 +0000, Anthony PERARD wrote:
> ... into the firmware directory, along with hvmloader.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
> Â.gitignoreÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ|ÂÂ1 +
> Âtools/firmware/MakefileÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ| 15 +++++++++++++++
> Âtools/firmware/hvmloader/acpi/Makefile |ÂÂ2 +-
> Â3 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/.gitignore b/.gitignore
> index 9ead7c4..7c7bb56 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -117,6 +117,7 @@ tools/firmware/hvmloader/acpi/mk_dsdt
> Âtools/firmware/hvmloader/acpi/dsdt*.c
> Âtools/firmware/hvmloader/acpi/dsdt_*.asl
> Âtools/firmware/hvmloader/acpi/ssdt_*.h
> +tools/firmware/hvmloader/acpi/dsdt_anycpu_qemu_xen.aml
> Âtools/firmware/hvmloader/hvmloader
> Âtools/firmware/hvmloader/roms.h
> Âtools/firmware/hvmloader/roms.inc
> diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile
> index 6cc86ce..9c63991 100644
> --- a/tools/firmware/Makefile
> +++ b/tools/firmware/Makefile
> @@ -19,6 +19,10 @@ SUBDIRS-y += hvmloader
> Â
> ÂLD32BIT-$(CONFIG_FreeBSD) := LD32BIT_FLAG=-melf_i386_fbsd
> Â
> +SEABIOS_ROM := seabios-dir/out/bios.bin
> +OVMF_ROM := ovmf-dir/ovmf.bin
> +ACPI_TABLE_QEMU_PC_I440FX = hvmloader/acpi/dsdt_anycpu_qemu_xen.aml
> +
> Âovmf-dir:
> Â     GIT=$(GIT) $(XEN_ROOT)/scripts/git-checkout.sh
> $(OVMF_UPSTREAM_URL) $(OVMF_UPSTREAM_REVISION) ovmf-dir
> Â     cp ovmf-makefile ovmf-dir/Makefile;
> @@ -45,6 +49,17 @@ endif
> Âinstall: all
> Â     [ -d $(INST_DIR) ] || $(INSTALL_DIR) $(INST_DIR)
> Â     [ ! -e $(TARGET) ] || $(INSTALL_DATA) $(TARGET) $(INST_DIR)
> +ifeq ($(CONFIG_SEABIOS),y)
> +ifeq ($(SEABIOS_PATH),)
> +     [ ! -e $(SEABIOS_ROM) ] || $(INSTALL_DATA) $(SEABIOS_ROM) 
> $(INST_DIR)/seabios.bin

If $(SEABIOS_ROM) doesn't exist surely that indicates something has gone
wrong prior to this?

The -e $(TARGET) you are (presumably) copying is similarly strange,
although maybe that is to do with it not being inside an ifeq like the
CONFIG_SEABIOS which protects your new one.

IOW I think you can drop the -e check from both, the ifeq is clearer and
less prone to carrying on in the face of other errors AFAICT.

> +endif
> +endif
> +ifeq ($(CONFIG_OVMF),y)
> +ifeq ($(OVMF_PATH),)
> +     [ ! -e $(OVMF_ROM) ] || $(INSTALL_DATA) $(OVMF_ROM)
> $(INST_DIR)/ovmf.bin
> +endif
> +endif
> +>    > [ ! -e $(ACPI_TABLE_QEMU_PC_I440FX) ] || $(INSTALL_DATA) 
> $(ACPI_TABLE_QEMU_PC_I440FX) $(INST_DIR)
> Â
> Â.PHONY: clean
> Âclean: subdirs-clean
> diff --git a/tools/firmware/hvmloader/acpi/Makefile
> b/tools/firmware/hvmloader/acpi/Makefile
> index d3e882a..3d8dd21 100644
> --- a/tools/firmware/hvmloader/acpi/Makefile
> +++ b/tools/firmware/hvmloader/acpi/Makefile
> @@ -46,7 +46,7 @@ $(filter dsdt_%.c,$(C_SRC)): %.c: iasl %.asl
> Â     iasl -vs -p $* -tc $*.asl
> Â     sed -e 's/AmlCode/$*/g' $*.hex >$@
> Â     echo "int $*_len=sizeof($*);" >>$@
> -     rm -f $*.aml $*.hex
> +     rm -f $*.hex

Is $*.aml an incidental output of iasl? The iasl manpage is not terribly
informative on the issue, but it says that -tc will create C output in a
.hex fix. I suppose the .aml is implied.
    ./mk_dsdt --debug=$(debug) --maxcpu $*ÂÂ>> $@

I think (but please check, there may be some weird subtlety with this sort
of rule) that it would be correct to change the rule here from:
    $(filter dsdt_%.c,$(C_SRC)): %.c: iasl %.asl
into:
    $(filter dsdt_%.c,$(C_SRC)): %.c %.aml: iasl %.asl

(my actual preference would be to invoke iasl twice, once for asl->aml and
then again for aml->hex, but that doesn't seem possible).

Oh, maybe the need for .hex file will go away later in this series and so
you can adjust this to only talk about the .aml? In which case after
mentioning that in the commit message you could ignore all the above and
fix up the rules properly at that point

Do you not need to also add this newly left lying around file to
.gitignore.

Ian.

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