Hi Anthony,
> On 17 Aug 2022, at 10:15, Anthony PERARD <anthony.per...@citrix.com> 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
> 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, by renaming the existing one. We will take
> example of efi/boot.c and have the common stub.c include a per-arch
> stub.h. This at least avoid the need to expose to Arm both alias
> efi_compat_get_info and efi_compat_runtime_call.
>
> Avoid using $(EFIOBJ-y) as an alias for $(clean-files), add
> "stub.c" directly to $(clean-files).
>
> Also update .gitignore as this was also missing from the original
> patch.
>
> Fixes: 7f96859b0d00 ("xen: reuse x86 EFI stub functions for Arm")
> Reported-by: Andrew Cooper <andrew.coop...@citrix.com>
> Signed-off-by: Anthony PERARD <anthony.per...@citrix.com>
I do not really like the empty header but I have no better solution so:
Reviewed-by: Bertrand Marquis <bertrand.marq...@arm.com>
Also I did some compilation runs and it works.
Cheers
Bertrand
> ---
>
> Notes:
> v2:
> - instead of renaming common/efi/stub.c to common_stub.c; we rename
> arch/*/efi/stub.c to stub.h and include it from common/stub.c
> - update .gitignore
>
> CC: Jan Beulich <jbeul...@suse.com>
> CC: Wei Chen <wei.c...@arm.com>
>
> xen/arch/arm/efi/Makefile | 4 ----
> xen/common/efi/efi-common.mk | 4 ++--
> xen/arch/arm/efi/stub.h | 4 ++++
> xen/arch/x86/efi/{stub.c => stub.h} | 5 ++++-
> xen/common/efi/stub.c | 5 +++++
> .gitignore | 1 +
> 6 files changed, 16 insertions(+), 7 deletions(-)
> create mode 100644 xen/arch/arm/efi/stub.h
> rename xen/arch/x86/efi/{stub.c => stub.h} (93%)
>
> diff --git a/xen/arch/arm/efi/Makefile b/xen/arch/arm/efi/Makefile
> index bd954a3b2d..ff1bcd6c50 100644
> --- a/xen/arch/arm/efi/Makefile
> +++ b/xen/arch/arm/efi/Makefile
> @@ -4,10 +4,6 @@ ifeq ($(CONFIG_ARM_EFI),y)
> obj-y += $(EFIOBJ-y)
> obj-$(CONFIG_ACPI) += efi-dom0.init.o
> else
> -# Add stub.o to EFIOBJ-y to re-use the clean-files in
> -# efi-common.mk. Otherwise the link of stub.c in arm/efi
> -# will not be cleaned in "make clean".
> -EFIOBJ-y += stub.o
> obj-y += stub.o
>
> $(obj)/stub.o: CFLAGS-y += -fno-short-wchar
> diff --git a/xen/common/efi/efi-common.mk b/xen/common/efi/efi-common.mk
> index ec2c34f198..950f564575 100644
> --- a/xen/common/efi/efi-common.mk
> +++ b/xen/common/efi/efi-common.mk
> @@ -9,9 +9,9 @@ CFLAGS-y += -iquote $(srcdir)
> # e.g.: It transforms "dir/foo/bar" into successively
> # "dir foo bar", ".. .. ..", "../../.."
> $(obj)/%.c: $(srctree)/common/efi/%.c FORCE
> - $(Q)test -f $@ || \
> - ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /,
> ,$(obj))))/source/common/efi/$(<F) $@
> + $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /,
> ,$(obj))))/source/common/efi/$(<F) $@
>
> clean-files += $(patsubst %.o, %.c, $(EFIOBJ-y:.init.o=.o) $(EFIOBJ-))
> +clean-files += stub.c
>
> .PRECIOUS: $(obj)/%.c
> diff --git a/xen/arch/arm/efi/stub.h b/xen/arch/arm/efi/stub.h
> new file mode 100644
> index 0000000000..b0a9b03e59
> --- /dev/null
> +++ b/xen/arch/arm/efi/stub.h
> @@ -0,0 +1,4 @@
> +/*
> + * Architecture specific implementation for EFI stub code. This file
> + * is intended to be included by common/efi/stub.c _only_.
> + */
> diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.h
> similarity index 93%
> rename from xen/arch/x86/efi/stub.c
> rename to xen/arch/x86/efi/stub.h
> index f2365bc041..9d2845b833 100644
> --- a/xen/arch/x86/efi/stub.c
> +++ b/xen/arch/x86/efi/stub.h
> @@ -1,3 +1,7 @@
> +/*
> + * Architecture specific implementation for EFI stub code. This file
> + * is intended to be included by common/efi/stub.c _only_.
> + */
> #include <xen/efi.h>
> #include <xen/init.h>
> #include <asm/asm_defns.h>
> @@ -8,7 +12,6 @@
> #include <efi/eficon.h>
> #include <efi/efidevp.h>
> #include <efi/efiapi.h>
> -#include "../../../common/efi/stub.c"
>
> /*
> * Here we are in EFI stub. EFI calls are not supported due to lack
> diff --git a/xen/common/efi/stub.c b/xen/common/efi/stub.c
> index 15694632c2..854efd9c99 100644
> --- a/xen/common/efi/stub.c
> +++ b/xen/common/efi/stub.c
> @@ -30,3 +30,8 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op)
> {
> return -ENOSYS;
> }
> +
> +/*
> + * Include architecture specific implementation here.
> + */
> +#include "stub.h"
> diff --git a/.gitignore b/.gitignore
> index ed7bd8bdc7..3a91e79672 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -311,6 +311,7 @@ xen/arch/*/efi/ebmalloc.c
> xen/arch/*/efi/efi.h
> xen/arch/*/efi/pe.c
> xen/arch/*/efi/runtime.c
> +xen/arch/*/efi/stub.c
> xen/arch/*/include/asm/asm-offsets.h
> xen/common/config_data.S
> xen/common/config.gz
> --
> Anthony PERARD
>
