Re: [PATCH v1.1 2/3] x86/build: Don't convert boot/{cmdline,head}.bin back to .S

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Thu, Apr 14, 2022 at 05:27:39PM +0100, Andrew Cooper wrote:
> There's no point wasting time converting binaries back to asm source.  Just
> use .incbin directly.  Explain in head.S what these binaries are.
> 
> Also, align the blobs.  While there's very little static data in the blobs,
> they should have at least 4 byte alignment.  There was previously no guarantee
> that cmdline_parse_early was aligned, and there is no longer an implicit
> 4-byte alignment between cmdline_parse_early and reloc caused by the use of
> .long.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
> index a5dd094836f6..0670e03b72e0 100644
> --- a/xen/arch/x86/boot/Makefile
> +++ b/xen/arch/x86/boot/Makefile
> @@ -10,7 +10,10 @@ head-srcs := $(addprefix $(obj)/, $(head-srcs))
>  ifdef building_out_of_srctree
>  $(obj)/head.o: CFLAGS-y += -iquote $(obj)

With this patch, we don't the "-iquote" option above, it was only useful
for both "#include" been removed.

>  endif
> -$(obj)/head.o: $(head-srcs)
> +# For .incbin - add $(obj) to the include path and add the dependencies
> +# manually as they're not included in .d
> +$(obj)/head.o: AFLAGS-y += -Wa$(comma)-I$(obj)
> +$(obj)/head.o: $(head-srcs:.S=.bin)

The manual dependencies are needed because `make` needs to know what
other target are needed before building "head.o". The .d files wouldn't
exist on a first build. I don't think a comment about that isn't really
necessary, but if there's one it should be about telling `make` to build
cmdline.bin and head.bin first.

Otherwise, the patch looks good.

Thanks,

-- 
Anthony PERARD