Hi Jan, > -----Original Message----- > From: Jan Beulich <jbeul...@suse.com> > Sent: 2022年1月25日 18:35 > To: Wei Chen <wei.c...@arm.com> > Cc: Bertrand Marquis <bertrand.marq...@arm.com>; xen- > de...@lists.xenproject.org; sstabell...@kernel.org; jul...@xen.org > Subject: Re: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for non- > EFI architecture > > On 23.09.2021 14:02, Wei Chen wrote: > > --- a/xen/common/Kconfig > > +++ b/xen/common/Kconfig > > @@ -11,6 +11,16 @@ config COMPAT > > config CORE_PARKING > > bool > > > > +config EFI > > + bool > > + ---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. > > The way enabling of (full) EFI support works on x86, I consider it > wrong / misleading to put the option in common code. At the very least > the help text would need to call out the extra dependencies. Plus the > help text of course then needs to be generic (i.e. applicable to both > Arm and x86). That's notwithstanding the fact that without a prompt > the help text won't ever be seen while configuring Xen. > > Also (nit): Indentation. And please don't use ---help--- anymore in > new code. >
I have used CONFIG_ARM_EFI to replace this common EFI config in my latest version. This Kconfig option has been removed. And thanks, I will not use --help-- anymore. > > --- 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*/ > > I can see that in the later patch, when introducing inline stubs, > you would need conditionals here, but I don't think you need them > right here (or you may want to introduce the stubs right away). > > Also (nit): Missing blank in the comment. > > Jan
