RE: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for non-EFI architecture

From: Wei Chen <Wei.Chen@arm.com>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	"julien@xen.org" <julien@xen.org>,
	Bertrand Marquis <Bertrand.Marquis@arm.com>
Subject: RE: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for non-EFI architecture
Date: Fri, 24 Sep 2021 04:34:26 +0000	[thread overview]
Message-ID: <PAXPR08MB68640027443F267495804A529EA49@PAXPR08MB6864.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <alpine.DEB.2.21.2109231811420.17979@sstabellini-ThinkPad-T480s>

Hi Stefano,

> -----Original Message-----
> From: Stefano Stabellini <sstabellini@kernel.org>
> Sent: 2021年9月24日 9:15
> To: Wei Chen <Wei.Chen@arm.com>
> Cc: xen-devel@lists.xenproject.org; sstabellini@kernel.org; julien@xen.org;
> Bertrand Marquis <Bertrand.Marquis@arm.com>
> Subject: Re: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for non-
> EFI architecture
> 
> On Thu, 23 Sep 2021, Wei Chen wrote:
> > Some architectures do not support EFI, but EFI API will be used
> > in some common features. Instead of spreading #ifdef ARCH, we
> > introduce this Kconfig option to give Xen the ability of stubing
> > EFI API for non-EFI supported architectures.
> >
> > Signed-off-by: Wei Chen <wei.chen@arm.com>
> > ---
> >  xen/arch/arm/Kconfig  |  1 +
> >  xen/arch/arm/Makefile |  2 +-
> >  xen/arch/x86/Kconfig  |  1 +
> >  xen/common/Kconfig    | 11 +++++++++++
> >  xen/include/xen/efi.h |  4 ++++
> >  5 files changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index ecfa6822e4..865ad83a89 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -6,6 +6,7 @@ config ARM_64
> >  	def_bool y
> >  	depends on !ARM_32
> >  	select 64BIT
> > +	select EFI
> >  	select HAS_FAST_MULTIPLY
> >
> >  config ARM
> > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> > index 3d3b97b5b4..ae4efbf76e 100644
> > --- a/xen/arch/arm/Makefile
> > +++ b/xen/arch/arm/Makefile
> > @@ -1,6 +1,6 @@
> >  obj-$(CONFIG_ARM_32) += arm32/
> >  obj-$(CONFIG_ARM_64) += arm64/
> > -obj-$(CONFIG_ARM_64) += efi/
> > +obj-$(CONFIG_EFI) += efi/
> >  obj-$(CONFIG_ACPI) += acpi/
> >  ifneq ($(CONFIG_NO_PLAT),y)
> >  obj-y += platforms/
> > diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> > index 28d13b9705..b9ed187f6b 100644
> > --- a/xen/arch/x86/Kconfig
> > +++ b/xen/arch/x86/Kconfig
> > @@ -10,6 +10,7 @@ config X86
> >  	select ALTERNATIVE_CALL
> >  	select ARCH_SUPPORTS_INT128
> >  	select CORE_PARKING
> > +	select EFI
> >  	select HAS_ALTERNATIVE
> >  	select HAS_COMPAT
> >  	select HAS_CPUFREQ
> > diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> > index 9ebb1c239b..f998746a1a 100644
> > --- a/xen/common/Kconfig
> > +++ b/xen/common/Kconfig
> > @@ -11,6 +11,16 @@ config COMPAT
> >  config CORE_PARKING
> >  	bool
> >
> > +config EFI
> > +	bool
> 
> Without the title the option is not user-selectable (or de-selectable).
> So the help message below can never be seen.
> 
> Either add a title, e.g.:
> 
> bool "EFI support"
> 
> Or fully make the option a silent option by removing the help text.
> 
> 

OK, in current Xen code, EFI is unconditionally compiled. Before
we change related code, I prefer to remove the help text.

> 
> > +	---help---
> > +      This option provides support for runtime services provided
> > +      by UEFI firmware (such as non-volatile variables, realtime
> > +      clock, and platform reset). A UEFI stub is also provided to
> > +      allow the kernel to be booted as an EFI application. This
> > +      is only useful for kernels that may run on systems that have
> > +      UEFI firmware.
> > +
> >  config GRANT_TABLE
> >  	bool "Grant table support" if EXPERT
> >  	default y
> > @@ -196,6 +206,7 @@ config KEXEC
> >
> >  config EFI_SET_VIRTUAL_ADDRESS_MAP
> >      bool "EFI: call SetVirtualAddressMap()" if EXPERT
> > +    depends on EFI
> >      ---help---
> >        Call EFI SetVirtualAddressMap() runtime service to setup memory
> map for
> >        further runtime services. According to UEFI spec, it isn't
> strictly
> > diff --git a/xen/include/xen/efi.h b/xen/include/xen/efi.h
> > index 94a7e547f9..661a48286a 100644
> > --- a/xen/include/xen/efi.h
> > +++ b/xen/include/xen/efi.h
> > @@ -25,6 +25,8 @@ extern struct efi efi;
> >
> >  #ifndef __ASSEMBLY__
> >
> > +#ifdef CONFIG_EFI
> > +
> >  union xenpf_efi_info;
> >  union compat_pf_efi_info;
> >
> > @@ -45,6 +47,8 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *);
> >  int efi_compat_get_info(uint32_t idx, union compat_pf_efi_info *);
> >  int efi_compat_runtime_call(struct compat_pf_efi_runtime_call *);
> >
> > +#endif /* CONFIG_EFI*/
> > +
> >  #endif /* !__ASSEMBLY__ */
> >
> >  #endif /* __XEN_EFI_H__ */
> > --
> > 2.25.1
> >