On 11.05.2022 03:46, Wei Chen wrote: > x86 is using compiler feature testing to decide EFI build > enable or not. When EFI build is disabled, x86 will use an > efi/stub.c file to replace efi/runtime.c for build objects. > Following this idea, we introduce a stub file for Arm, but > use CONFIG_ARM_EFI to decide EFI build enable or not. > > And the most functions in x86 EFI stub.c can be reused for > other architectures, like Arm. So we move them to common > and keep the x86 specific function in x86/efi/stub.c. > > To avoid the symbol link conflict error when linking common > stub files to x86/efi. We add a regular file check in efi > stub files' link script. Depends on this check we can bypass > the link behaviors for existed stub files in x86/efi. > > As there is no Arm specific EFI stub function for Arm in > current stage, Arm still can use the existed symbol link > method for EFI stub files.
Wouldn't it be better to mandate that every arch has its stub.c, and in the Arm one all you'd do (for now) is #include the common one? (But see also below.) > --- a/xen/arch/arm/Makefile > +++ b/xen/arch/arm/Makefile > @@ -1,6 +1,5 @@ > obj-$(CONFIG_ARM_32) += arm32/ > obj-$(CONFIG_ARM_64) += arm64/ > -obj-$(CONFIG_ARM_64) += efi/ > obj-$(CONFIG_ACPI) += acpi/ > obj-$(CONFIG_HAS_PCI) += pci/ > ifneq ($(CONFIG_NO_PLAT),y) > @@ -20,6 +19,7 @@ obj-y += domain.o > obj-y += domain_build.init.o > obj-y += domctl.o > obj-$(CONFIG_EARLY_PRINTK) += early_printk.o > +obj-y += efi/ > obj-y += gic.o > obj-y += gic-v2.o > obj-$(CONFIG_GICV3) += gic-v3.o > diff --git a/xen/arch/arm/efi/Makefile b/xen/arch/arm/efi/Makefile > index 4313c39066..dffe72e589 100644 > --- a/xen/arch/arm/efi/Makefile > +++ b/xen/arch/arm/efi/Makefile > @@ -1,4 +1,12 @@ > include $(srctree)/common/efi/efi-common.mk > > +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 > +endif I realize Stefano indicated he's happy with the Arm side, but I still wonder: What use is the stub on Arm32? Even further - once you have a config option (rather than x86'es build-time check plus x86'es dual- purposing of all object files), why do you need a stub in the first place? You ought to be able to deal with things via inline functions and macros, I would think. > --- a/xen/common/efi/efi-common.mk > +++ b/xen/common/efi/efi-common.mk > @@ -9,7 +9,8 @@ CFLAGS-y += -iquote $(srcdir) > # e.g.: It transforms "dir/foo/bar" into successively > # "dir foo bar", ".. .. ..", "../../.." > $(obj)/%.c: $(srctree)/common/efi/%.c FORCE > - $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, > ,$(obj))))/source/common/efi/$(<F) $@ > + $(Q)test -f $@ || \ > + ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, > ,$(obj))))/source/common/efi/$(<F) $@ Please can you indent the "ln" to match "test", such that it's easily visible (without paying attention to line continuation characters) that these two lines are a single command? Jan
