Re: [XEN PATCH] build: Fix x86 build without EFI

[Julien Grall <julien@xxxxxxx>]

Hi Anthony,

On 16/08/2022 11:30, Anthony PERARD wrote:
> We can't have a source file with the same name that exist in both the
> common code and in the arch specific code for efi/. This can lead to
> comfusion in make and it can pick up the wrong source file. This issue
Typo: s/comfusion/confusion/

> lead to a failure to build a pv-shim for x86 out-of-tree, as this is
> one example of an x86 build using the efi/stub.c.
>
> The issue is that in out-of-tree, make might find x86/efi/stub.c via
> VPATH, but as the target needs to be rebuilt due to FORCE, make
> actually avoid changing the source tree and rebuilt the target with
> VPATH ignored, so $@ lead to the build tree where "stub.c" dosen't
> exist yet so a link is made to "common/stub.c".
>
> Rework the new common/stub.c file to have a different name than the
> already existing one. And build both *stub.c as two different objects.
> This mean we have to move some efi_compat_* aliases which are probably
> useless for Arm.
>
> Avoid using $(EFIOBJ-y) as an alias for $(clean-files), add
> common_stub.c directly to $(clean-files).
>
> Fixes: 7f96859b0d00 ("xen: reuse x86 EFI stub functions for Arm")
> Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
>
> For the cflag addition in non-ARM_EFI, I was tempted to apply it to
> the whole directory instead of just stub.o. (Even if there's only a
> single object). I think that would be enough to overwrite the
> -fshort-wchar from efi-common.mk, but I forgot what cflags come after
> that. But adding it to just one object mean that it's added at the
> last possible moment.
> ---
>   xen/arch/arm/efi/Makefile                | 8 ++------
>   xen/arch/x86/efi/Makefile                | 2 +-
>   xen/common/efi/efi-common.mk             | 4 ++--
>   xen/arch/x86/efi/stub.c                  | 7 -------
>   xen/common/efi/{stub.c => common_stub.c} | 6 ++++++
I haven't looked at the rest of the patch. However, I think you also 
want to update .gitignore to excluse arch/*/efi/common_stub.c.
Also, I am thinking to drop my patch [1] which update .gitignore as this 
will become moot with this change. Let me know what you think.
Cheers,

[1] 20220812191930.34494-1-julien@xxxxxxx

--
Julien Grall