| Message ID | 20210923120236.3692135-21-wei.chen@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add device tree based NUMA support to Arm | expand |
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. > + ---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 >
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 > >
On 24.09.2021 06:34, Wei Chen wrote: >> From: Stefano Stabellini <sstabellini@kernel.org> >> Sent: 2021年9月24日 9:15 >> >> On Thu, 23 Sep 2021, Wei Chen wrote: >>> --- 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. But that's not true: At least on x86 EFI gets compiled depending on tool chain capabilities. Ultimately we may indeed want a user selectable option here, but until then I'm afraid having this option at all may be misleading on x86. Jan
Hi Jan, > -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 2021年9月24日 15:59 > To: Wei Chen <Wei.Chen@arm.com> > Cc: xen-devel@lists.xenproject.org; julien@xen.org; Bertrand Marquis > <Bertrand.Marquis@arm.com>; Stefano Stabellini <sstabellini@kernel.org> > Subject: Re: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for non- > EFI architecture > > On 24.09.2021 06:34, Wei Chen wrote: > >> From: Stefano Stabellini <sstabellini@kernel.org> > >> Sent: 2021年9月24日 9:15 > >> > >> On Thu, 23 Sep 2021, Wei Chen wrote: > >>> --- 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. > > But that's not true: At least on x86 EFI gets compiled depending on > tool chain capabilities. Ultimately we may indeed want a user > selectable option here, but until then I'm afraid having this option > at all may be misleading on x86. > I check the build scripts, yes, you're right. For x86, EFI is not a selectable option in Kconfig. I agree with you, we can't use Kconfig system to decide to enable EFI build for x86 or not. So how about we just use this EFI option for Arm only? Because on Arm, we do not have such toolchain dependency. > Jan
On 24.09.2021 12:31, Wei Chen wrote: >> From: Jan Beulich <jbeulich@suse.com> >> Sent: 2021年9月24日 15:59 >> >> On 24.09.2021 06:34, Wei Chen wrote: >>>> From: Stefano Stabellini <sstabellini@kernel.org> >>>> Sent: 2021年9月24日 9:15 >>>> >>>> On Thu, 23 Sep 2021, Wei Chen wrote: >>>>> --- 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. >> >> But that's not true: At least on x86 EFI gets compiled depending on >> tool chain capabilities. Ultimately we may indeed want a user >> selectable option here, but until then I'm afraid having this option >> at all may be misleading on x86. >> > > I check the build scripts, yes, you're right. For x86, EFI is not a > selectable option in Kconfig. I agree with you, we can't use Kconfig > system to decide to enable EFI build for x86 or not. > > So how about we just use this EFI option for Arm only? Because on Arm, > we do not have such toolchain dependency. To be honest - don't know. That's because I don't know what you want to use the option for subsequently. Jan
Hi Jan, > -----Original Message----- > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Jan > Beulich > Sent: 2021年9月24日 18:49 > To: Wei Chen <Wei.Chen@arm.com> > Cc: xen-devel@lists.xenproject.org; julien@xen.org; Bertrand Marquis > <Bertrand.Marquis@arm.com>; Stefano Stabellini <sstabellini@kernel.org> > Subject: Re: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for non- > EFI architecture > > On 24.09.2021 12:31, Wei Chen wrote: > >> From: Jan Beulich <jbeulich@suse.com> > >> Sent: 2021年9月24日 15:59 > >> > >> On 24.09.2021 06:34, Wei Chen wrote: > >>>> From: Stefano Stabellini <sstabellini@kernel.org> > >>>> Sent: 2021年9月24日 9:15 > >>>> > >>>> On Thu, 23 Sep 2021, Wei Chen wrote: > >>>>> --- 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. > >> > >> But that's not true: At least on x86 EFI gets compiled depending on > >> tool chain capabilities. Ultimately we may indeed want a user > >> selectable option here, but until then I'm afraid having this option > >> at all may be misleading on x86. > >> > > > > I check the build scripts, yes, you're right. For x86, EFI is not a > > selectable option in Kconfig. I agree with you, we can't use Kconfig > > system to decide to enable EFI build for x86 or not. > > > > So how about we just use this EFI option for Arm only? Because on Arm, > > we do not have such toolchain dependency. > > To be honest - don't know. That's because I don't know what you want > to use the option for subsequently. > In last version, I had introduced an arch-helper to stub EFI_BOOT in Arm's common code for Arm32. Because Arm32 doesn't support EFI. So Julien suggested me to introduce a CONFIG_EFI option for non-EFI supported architectures to stub in EFI layer. [1] https://lists.xenproject.org/archives/html/xen-devel/2021-08/msg00808.html > Jan >
Hi Julien, Stefano, > -----Original Message----- > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Wei > Chen > Sent: 2021年9月26日 18:25 > To: Jan Beulich <jbeulich@suse.com> > Cc: xen-devel@lists.xenproject.org; julien@xen.org; Bertrand Marquis > <Bertrand.Marquis@arm.com>; Stefano Stabellini <sstabellini@kernel.org> > Subject: RE: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for non- > EFI architecture > > Hi Jan, > > > -----Original Message----- > > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of > Jan > > Beulich > > Sent: 2021年9月24日 18:49 > > To: Wei Chen <Wei.Chen@arm.com> > > Cc: xen-devel@lists.xenproject.org; julien@xen.org; Bertrand Marquis > > <Bertrand.Marquis@arm.com>; Stefano Stabellini <sstabellini@kernel.org> > > Subject: Re: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for > non- > > EFI architecture > > > > On 24.09.2021 12:31, Wei Chen wrote: > > >> From: Jan Beulich <jbeulich@suse.com> > > >> Sent: 2021年9月24日 15:59 > > >> > > >> On 24.09.2021 06:34, Wei Chen wrote: > > >>>> From: Stefano Stabellini <sstabellini@kernel.org> > > >>>> Sent: 2021年9月24日 9:15 > > >>>> > > >>>> On Thu, 23 Sep 2021, Wei Chen wrote: > > >>>>> --- 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. > > >> > > >> But that's not true: At least on x86 EFI gets compiled depending on > > >> tool chain capabilities. Ultimately we may indeed want a user > > >> selectable option here, but until then I'm afraid having this option > > >> at all may be misleading on x86. > > >> > > > > > > I check the build scripts, yes, you're right. For x86, EFI is not a > > > selectable option in Kconfig. I agree with you, we can't use Kconfig > > > system to decide to enable EFI build for x86 or not. > > > > > > So how about we just use this EFI option for Arm only? Because on Arm, > > > we do not have such toolchain dependency. > > > > To be honest - don't know. That's because I don't know what you want > > to use the option for subsequently. > > > > In last version, I had introduced an arch-helper to stub EFI_BOOT > in Arm's common code for Arm32. Because Arm32 doesn't support EFI. > So Julien suggested me to introduce a CONFIG_EFI option for non-EFI > supported architectures to stub in EFI layer. > > [1] https://lists.xenproject.org/archives/html/xen-devel/2021- > 08/msg00808.html > As Jan' reminded, x86 doesn't depend on Kconfig to build EFI code. So, if we CONFIG_EFI to stub EFI API's for x86, we will encounter that toolchains enable EFI, but Kconfig disable EFI. Or Kconfig enable EFI but toolchain doesn't provide EFI build supports. And then x86 could not work well. If we use CONFIG_EFI for Arm only, that means CONFIG_EFI for x86 is off, this will also cause problem. So, can we still use previous arch_helpers to stub for Arm32? until x86 can use this selectable option? > > Jan > >
On Mon, 27 Sep 2021, Wei Chen wrote: > > -----Original Message----- > > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Wei > > Chen > > Sent: 2021年9月26日 18:25 > > To: Jan Beulich <jbeulich@suse.com> > > Cc: xen-devel@lists.xenproject.org; julien@xen.org; Bertrand Marquis > > <Bertrand.Marquis@arm.com>; Stefano Stabellini <sstabellini@kernel.org> > > Subject: RE: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for non- > > EFI architecture > > > > Hi Jan, > > > > > -----Original Message----- > > > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of > > Jan > > > Beulich > > > Sent: 2021年9月24日 18:49 > > > To: Wei Chen <Wei.Chen@arm.com> > > > Cc: xen-devel@lists.xenproject.org; julien@xen.org; Bertrand Marquis > > > <Bertrand.Marquis@arm.com>; Stefano Stabellini <sstabellini@kernel.org> > > > Subject: Re: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for > > non- > > > EFI architecture > > > > > > On 24.09.2021 12:31, Wei Chen wrote: > > > >> From: Jan Beulich <jbeulich@suse.com> > > > >> Sent: 2021年9月24日 15:59 > > > >> > > > >> On 24.09.2021 06:34, Wei Chen wrote: > > > >>>> From: Stefano Stabellini <sstabellini@kernel.org> > > > >>>> Sent: 2021年9月24日 9:15 > > > >>>> > > > >>>> On Thu, 23 Sep 2021, Wei Chen wrote: > > > >>>>> --- 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. > > > >> > > > >> But that's not true: At least on x86 EFI gets compiled depending on > > > >> tool chain capabilities. Ultimately we may indeed want a user > > > >> selectable option here, but until then I'm afraid having this option > > > >> at all may be misleading on x86. > > > >> > > > > > > > > I check the build scripts, yes, you're right. For x86, EFI is not a > > > > selectable option in Kconfig. I agree with you, we can't use Kconfig > > > > system to decide to enable EFI build for x86 or not. > > > > > > > > So how about we just use this EFI option for Arm only? Because on Arm, > > > > we do not have such toolchain dependency. > > > > > > To be honest - don't know. That's because I don't know what you want > > > to use the option for subsequently. > > > > > > > In last version, I had introduced an arch-helper to stub EFI_BOOT > > in Arm's common code for Arm32. Because Arm32 doesn't support EFI. > > So Julien suggested me to introduce a CONFIG_EFI option for non-EFI > > supported architectures to stub in EFI layer. > > > > [1] https://lists.xenproject.org/archives/html/xen-devel/2021- > > 08/msg00808.html > > > > As Jan' reminded, x86 doesn't depend on Kconfig to build EFI code. > So, if we CONFIG_EFI to stub EFI API's for x86, we will encounter > that toolchains enable EFI, but Kconfig disable EFI. Or Kconfig > enable EFI but toolchain doesn't provide EFI build supports. And > then x86 could not work well. > > If we use CONFIG_EFI for Arm only, that means CONFIG_EFI for x86 > is off, this will also cause problem. > > So, can we still use previous arch_helpers to stub for Arm32? > until x86 can use this selectable option? EFI doesn't have to be necessarily a user-visible option in Kconfig at this point. I think Julien was just asking to make the #ifdef based on a EFI-related config rather than just based CONFIG_ARM64. On x86 EFI is detected based on compiler support, setting XEN_BUILD_EFI in xen/arch/x86/Makefile. Let's say that we keep using the same name "XEN_BUILD_EFI" on ARM as well. On ARM32, XEN_BUILD_EFI should be always unset. On ARM64 XEN_BUILD_EFI should be always set. That's it, right? I'd argue that CONFIG_EFI or HAS_EFI are better names than XEN_BUILD_EFI, but that's OK anyway. So for instance you can make XEN_BUILD_EFI an invisible symbol in xen/arch/arm/Kconfig and select it only on ARM64.
Hi Stefano, > -----Original Message----- > From: Stefano Stabellini <sstabellini@kernel.org> > Sent: 2021年9月28日 9:00 > To: Wei Chen <Wei.Chen@arm.com> > Cc: Jan Beulich <jbeulich@suse.com>; xen-devel@lists.xenproject.org; > julien@xen.org; Bertrand Marquis <Bertrand.Marquis@arm.com>; Stefano > Stabellini <sstabellini@kernel.org> > Subject: RE: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for non- > EFI architecture > > On Mon, 27 Sep 2021, Wei Chen wrote: > > > -----Original Message----- > > > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of > Wei > > > Chen > > > Sent: 2021年9月26日 18:25 > > > To: Jan Beulich <jbeulich@suse.com> > > > Cc: xen-devel@lists.xenproject.org; julien@xen.org; Bertrand Marquis > > > <Bertrand.Marquis@arm.com>; Stefano Stabellini <sstabellini@kernel.org> > > > Subject: RE: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for > non- > > > EFI architecture > > > > > > Hi Jan, > > > > > > > -----Original Message----- > > > > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf > Of > > > Jan > > > > Beulich > > > > Sent: 2021年9月24日 18:49 > > > > To: Wei Chen <Wei.Chen@arm.com> > > > > Cc: xen-devel@lists.xenproject.org; julien@xen.org; Bertrand Marquis > > > > <Bertrand.Marquis@arm.com>; Stefano Stabellini > <sstabellini@kernel.org> > > > > Subject: Re: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for > > > non- > > > > EFI architecture > > > > > > > > On 24.09.2021 12:31, Wei Chen wrote: > > > > >> From: Jan Beulich <jbeulich@suse.com> > > > > >> Sent: 2021年9月24日 15:59 > > > > >> > > > > >> On 24.09.2021 06:34, Wei Chen wrote: > > > > >>>> From: Stefano Stabellini <sstabellini@kernel.org> > > > > >>>> Sent: 2021年9月24日 9:15 > > > > >>>> > > > > >>>> On Thu, 23 Sep 2021, Wei Chen wrote: > > > > >>>>> --- 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. > > > > >> > > > > >> But that's not true: At least on x86 EFI gets compiled depending > on > > > > >> tool chain capabilities. Ultimately we may indeed want a user > > > > >> selectable option here, but until then I'm afraid having this > option > > > > >> at all may be misleading on x86. > > > > >> > > > > > > > > > > I check the build scripts, yes, you're right. For x86, EFI is not > a > > > > > selectable option in Kconfig. I agree with you, we can't use > Kconfig > > > > > system to decide to enable EFI build for x86 or not. > > > > > > > > > > So how about we just use this EFI option for Arm only? Because on > Arm, > > > > > we do not have such toolchain dependency. > > > > > > > > To be honest - don't know. That's because I don't know what you want > > > > to use the option for subsequently. > > > > > > > > > > In last version, I had introduced an arch-helper to stub EFI_BOOT > > > in Arm's common code for Arm32. Because Arm32 doesn't support EFI. > > > So Julien suggested me to introduce a CONFIG_EFI option for non-EFI > > > supported architectures to stub in EFI layer. > > > > > > [1] https://lists.xenproject.org/archives/html/xen-devel/2021- > > > 08/msg00808.html > > > > > > > As Jan' reminded, x86 doesn't depend on Kconfig to build EFI code. > > So, if we CONFIG_EFI to stub EFI API's for x86, we will encounter > > that toolchains enable EFI, but Kconfig disable EFI. Or Kconfig > > enable EFI but toolchain doesn't provide EFI build supports. And > > then x86 could not work well. > > > > If we use CONFIG_EFI for Arm only, that means CONFIG_EFI for x86 > > is off, this will also cause problem. > > > > So, can we still use previous arch_helpers to stub for Arm32? > > until x86 can use this selectable option? > > EFI doesn't have to be necessarily a user-visible option in Kconfig at > this point. I think Julien was just asking to make the #ifdef based on > a EFI-related config rather than just based CONFIG_ARM64. > > On x86 EFI is detected based on compiler support, setting XEN_BUILD_EFI > in xen/arch/x86/Makefile. Let's say that we keep using the same name > "XEN_BUILD_EFI" on ARM as well. > > On ARM32, XEN_BUILD_EFI should be always unset. > > On ARM64 XEN_BUILD_EFI should be always set. > > That's it, right? I'd argue that CONFIG_EFI or HAS_EFI are better names > than XEN_BUILD_EFI, but that's OK anyway. So for instance you can make > XEN_BUILD_EFI an invisible symbol in xen/arch/arm/Kconfig and select it > only on ARM64. Thanks, this is a good approach. But if we place XEN_BUILD_EFI in Kconfig it will be transfer to CONFIG_XEN_BUILD_EFI. How about using another name in Kconfig like ARM_EFI, but use CONFIG_ARM_EFI in config.h to define XEN_BUILD_EFI?
On Tue, 28 Sep 2021, Wei Chen wrote: > > -----Original Message----- > > From: Stefano Stabellini <sstabellini@kernel.org> > > Sent: 2021年9月28日 9:00 > > To: Wei Chen <Wei.Chen@arm.com> > > Cc: Jan Beulich <jbeulich@suse.com>; xen-devel@lists.xenproject.org; > > julien@xen.org; Bertrand Marquis <Bertrand.Marquis@arm.com>; Stefano > > Stabellini <sstabellini@kernel.org> > > Subject: RE: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for non- > > EFI architecture > > > > On Mon, 27 Sep 2021, Wei Chen wrote: > > > > -----Original Message----- > > > > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of > > Wei > > > > Chen > > > > Sent: 2021年9月26日 18:25 > > > > To: Jan Beulich <jbeulich@suse.com> > > > > Cc: xen-devel@lists.xenproject.org; julien@xen.org; Bertrand Marquis > > > > <Bertrand.Marquis@arm.com>; Stefano Stabellini <sstabellini@kernel.org> > > > > Subject: RE: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for > > non- > > > > EFI architecture > > > > > > > > Hi Jan, > > > > > > > > > -----Original Message----- > > > > > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf > > Of > > > > Jan > > > > > Beulich > > > > > Sent: 2021年9月24日 18:49 > > > > > To: Wei Chen <Wei.Chen@arm.com> > > > > > Cc: xen-devel@lists.xenproject.org; julien@xen.org; Bertrand Marquis > > > > > <Bertrand.Marquis@arm.com>; Stefano Stabellini > > <sstabellini@kernel.org> > > > > > Subject: Re: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for > > > > non- > > > > > EFI architecture > > > > > > > > > > On 24.09.2021 12:31, Wei Chen wrote: > > > > > >> From: Jan Beulich <jbeulich@suse.com> > > > > > >> Sent: 2021年9月24日 15:59 > > > > > >> > > > > > >> On 24.09.2021 06:34, Wei Chen wrote: > > > > > >>>> From: Stefano Stabellini <sstabellini@kernel.org> > > > > > >>>> Sent: 2021年9月24日 9:15 > > > > > >>>> > > > > > >>>> On Thu, 23 Sep 2021, Wei Chen wrote: > > > > > >>>>> --- 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. > > > > > >> > > > > > >> But that's not true: At least on x86 EFI gets compiled depending > > on > > > > > >> tool chain capabilities. Ultimately we may indeed want a user > > > > > >> selectable option here, but until then I'm afraid having this > > option > > > > > >> at all may be misleading on x86. > > > > > >> > > > > > > > > > > > > I check the build scripts, yes, you're right. For x86, EFI is not > > a > > > > > > selectable option in Kconfig. I agree with you, we can't use > > Kconfig > > > > > > system to decide to enable EFI build for x86 or not. > > > > > > > > > > > > So how about we just use this EFI option for Arm only? Because on > > Arm, > > > > > > we do not have such toolchain dependency. > > > > > > > > > > To be honest - don't know. That's because I don't know what you want > > > > > to use the option for subsequently. > > > > > > > > > > > > > In last version, I had introduced an arch-helper to stub EFI_BOOT > > > > in Arm's common code for Arm32. Because Arm32 doesn't support EFI. > > > > So Julien suggested me to introduce a CONFIG_EFI option for non-EFI > > > > supported architectures to stub in EFI layer. > > > > > > > > [1] https://lists.xenproject.org/archives/html/xen-devel/2021- > > > > 08/msg00808.html > > > > > > > > > > As Jan' reminded, x86 doesn't depend on Kconfig to build EFI code. > > > So, if we CONFIG_EFI to stub EFI API's for x86, we will encounter > > > that toolchains enable EFI, but Kconfig disable EFI. Or Kconfig > > > enable EFI but toolchain doesn't provide EFI build supports. And > > > then x86 could not work well. > > > > > > If we use CONFIG_EFI for Arm only, that means CONFIG_EFI for x86 > > > is off, this will also cause problem. > > > > > > So, can we still use previous arch_helpers to stub for Arm32? > > > until x86 can use this selectable option? > > > > EFI doesn't have to be necessarily a user-visible option in Kconfig at > > this point. I think Julien was just asking to make the #ifdef based on > > a EFI-related config rather than just based CONFIG_ARM64. > > > > On x86 EFI is detected based on compiler support, setting XEN_BUILD_EFI > > in xen/arch/x86/Makefile. Let's say that we keep using the same name > > "XEN_BUILD_EFI" on ARM as well. > > > > On ARM32, XEN_BUILD_EFI should be always unset. > > > > On ARM64 XEN_BUILD_EFI should be always set. > > > > That's it, right? I'd argue that CONFIG_EFI or HAS_EFI are better names > > than XEN_BUILD_EFI, but that's OK anyway. So for instance you can make > > XEN_BUILD_EFI an invisible symbol in xen/arch/arm/Kconfig and select it > > only on ARM64. > > Thanks, this is a good approach. But if we place XEN_BUILD_EFI in Kconfig > it will be transfer to CONFIG_XEN_BUILD_EFI. How about using another name > in Kconfig like ARM_EFI, but use CONFIG_ARM_EFI in config.h to define > XEN_BUILD_EFI? I am OK with that. Another option is to rename XEN_BUILD_EFI to CONFIG_XEN_BUILD_EFI on x86. Either way is fine by me. Jan, do you havea preference?
On 28.09.2021 07:01, Stefano Stabellini wrote: > On Tue, 28 Sep 2021, Wei Chen wrote: >>> -----Original Message----- >>> From: Stefano Stabellini <sstabellini@kernel.org> >>> Sent: 2021年9月28日 9:00 >>> To: Wei Chen <Wei.Chen@arm.com> >>> Cc: Jan Beulich <jbeulich@suse.com>; xen-devel@lists.xenproject.org; >>> julien@xen.org; Bertrand Marquis <Bertrand.Marquis@arm.com>; Stefano >>> Stabellini <sstabellini@kernel.org> >>> Subject: RE: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for non- >>> EFI architecture >>> >>> On Mon, 27 Sep 2021, Wei Chen wrote: >>>>> -----Original Message----- >>>>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of >>> Wei >>>>> Chen >>>>> Sent: 2021年9月26日 18:25 >>>>> To: Jan Beulich <jbeulich@suse.com> >>>>> Cc: xen-devel@lists.xenproject.org; julien@xen.org; Bertrand Marquis >>>>> <Bertrand.Marquis@arm.com>; Stefano Stabellini <sstabellini@kernel.org> >>>>> Subject: RE: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for >>> non- >>>>> EFI architecture >>>>> >>>>> Hi Jan, >>>>> >>>>>> -----Original Message----- >>>>>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf >>> Of >>>>> Jan >>>>>> Beulich >>>>>> Sent: 2021年9月24日 18:49 >>>>>> To: Wei Chen <Wei.Chen@arm.com> >>>>>> Cc: xen-devel@lists.xenproject.org; julien@xen.org; Bertrand Marquis >>>>>> <Bertrand.Marquis@arm.com>; Stefano Stabellini >>> <sstabellini@kernel.org> >>>>>> Subject: Re: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for >>>>> non- >>>>>> EFI architecture >>>>>> >>>>>> On 24.09.2021 12:31, Wei Chen wrote: >>>>>>>> From: Jan Beulich <jbeulich@suse.com> >>>>>>>> Sent: 2021年9月24日 15:59 >>>>>>>> >>>>>>>> On 24.09.2021 06:34, Wei Chen wrote: >>>>>>>>>> From: Stefano Stabellini <sstabellini@kernel.org> >>>>>>>>>> Sent: 2021年9月24日 9:15 >>>>>>>>>> >>>>>>>>>> On Thu, 23 Sep 2021, Wei Chen wrote: >>>>>>>>>>> --- 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. >>>>>>>> >>>>>>>> But that's not true: At least on x86 EFI gets compiled depending >>> on >>>>>>>> tool chain capabilities. Ultimately we may indeed want a user >>>>>>>> selectable option here, but until then I'm afraid having this >>> option >>>>>>>> at all may be misleading on x86. >>>>>>>> >>>>>>> >>>>>>> I check the build scripts, yes, you're right. For x86, EFI is not >>> a >>>>>>> selectable option in Kconfig. I agree with you, we can't use >>> Kconfig >>>>>>> system to decide to enable EFI build for x86 or not. >>>>>>> >>>>>>> So how about we just use this EFI option for Arm only? Because on >>> Arm, >>>>>>> we do not have such toolchain dependency. >>>>>> >>>>>> To be honest - don't know. That's because I don't know what you want >>>>>> to use the option for subsequently. >>>>>> >>>>> >>>>> In last version, I had introduced an arch-helper to stub EFI_BOOT >>>>> in Arm's common code for Arm32. Because Arm32 doesn't support EFI. >>>>> So Julien suggested me to introduce a CONFIG_EFI option for non-EFI >>>>> supported architectures to stub in EFI layer. >>>>> >>>>> [1] https://lists.xenproject.org/archives/html/xen-devel/2021- >>>>> 08/msg00808.html >>>>> >>>> >>>> As Jan' reminded, x86 doesn't depend on Kconfig to build EFI code. >>>> So, if we CONFIG_EFI to stub EFI API's for x86, we will encounter >>>> that toolchains enable EFI, but Kconfig disable EFI. Or Kconfig >>>> enable EFI but toolchain doesn't provide EFI build supports. And >>>> then x86 could not work well. >>>> >>>> If we use CONFIG_EFI for Arm only, that means CONFIG_EFI for x86 >>>> is off, this will also cause problem. >>>> >>>> So, can we still use previous arch_helpers to stub for Arm32? >>>> until x86 can use this selectable option? >>> >>> EFI doesn't have to be necessarily a user-visible option in Kconfig at >>> this point. I think Julien was just asking to make the #ifdef based on >>> a EFI-related config rather than just based CONFIG_ARM64. >>> >>> On x86 EFI is detected based on compiler support, setting XEN_BUILD_EFI >>> in xen/arch/x86/Makefile. Let's say that we keep using the same name >>> "XEN_BUILD_EFI" on ARM as well. >>> >>> On ARM32, XEN_BUILD_EFI should be always unset. >>> >>> On ARM64 XEN_BUILD_EFI should be always set. >>> >>> That's it, right? I'd argue that CONFIG_EFI or HAS_EFI are better names >>> than XEN_BUILD_EFI, but that's OK anyway. So for instance you can make >>> XEN_BUILD_EFI an invisible symbol in xen/arch/arm/Kconfig and select it >>> only on ARM64. >> >> Thanks, this is a good approach. But if we place XEN_BUILD_EFI in Kconfig >> it will be transfer to CONFIG_XEN_BUILD_EFI. How about using another name >> in Kconfig like ARM_EFI, but use CONFIG_ARM_EFI in config.h to define >> XEN_BUILD_EFI? > > I am OK with that. Another option is to rename XEN_BUILD_EFI to > CONFIG_XEN_BUILD_EFI on x86. Either way is fine by me. Jan, do you havea > preference? Yes, I do: No new CONFIG_* settings please that don't originate from Kconfig. Hence I'm afraid this is a "no" to your suggestion. Mid-term we should try to get rid of the remaining CONFIG_* which get #define-d in e.g. asm/config.h. Jan
> -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 2021年9月28日 16:03 > To: Stefano Stabellini <sstabellini@kernel.org>; Wei Chen > <Wei.Chen@arm.com> > Cc: xen-devel@lists.xenproject.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 28.09.2021 07:01, Stefano Stabellini wrote: > > On Tue, 28 Sep 2021, Wei Chen wrote: > >>> -----Original Message----- > >>> From: Stefano Stabellini <sstabellini@kernel.org> > >>> Sent: 2021年9月28日 9:00 > >>> To: Wei Chen <Wei.Chen@arm.com> > >>> Cc: Jan Beulich <jbeulich@suse.com>; xen-devel@lists.xenproject.org; > >>> julien@xen.org; Bertrand Marquis <Bertrand.Marquis@arm.com>; Stefano > >>> Stabellini <sstabellini@kernel.org> > >>> Subject: RE: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for > non- > >>> EFI architecture > >>> > >>> On Mon, 27 Sep 2021, Wei Chen wrote: > >>>>> -----Original Message----- > >>>>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf > Of > >>> Wei > >>>>> Chen > >>>>> Sent: 2021年9月26日 18:25 > >>>>> To: Jan Beulich <jbeulich@suse.com> > >>>>> Cc: xen-devel@lists.xenproject.org; julien@xen.org; Bertrand Marquis > >>>>> <Bertrand.Marquis@arm.com>; Stefano Stabellini > <sstabellini@kernel.org> > >>>>> Subject: RE: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for > >>> non- > >>>>> EFI architecture > >>>>> > >>>>> Hi Jan, > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf > >>> Of > >>>>> Jan > >>>>>> Beulich > >>>>>> Sent: 2021年9月24日 18:49 > >>>>>> To: Wei Chen <Wei.Chen@arm.com> > >>>>>> Cc: xen-devel@lists.xenproject.org; julien@xen.org; Bertrand > Marquis > >>>>>> <Bertrand.Marquis@arm.com>; Stefano Stabellini > >>> <sstabellini@kernel.org> > >>>>>> Subject: Re: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API > for > >>>>> non- > >>>>>> EFI architecture > >>>>>> > >>>>>> On 24.09.2021 12:31, Wei Chen wrote: > >>>>>>>> From: Jan Beulich <jbeulich@suse.com> > >>>>>>>> Sent: 2021年9月24日 15:59 > >>>>>>>> > >>>>>>>> On 24.09.2021 06:34, Wei Chen wrote: > >>>>>>>>>> From: Stefano Stabellini <sstabellini@kernel.org> > >>>>>>>>>> Sent: 2021年9月24日 9:15 > >>>>>>>>>> > >>>>>>>>>> On Thu, 23 Sep 2021, Wei Chen wrote: > >>>>>>>>>>> --- 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. > >>>>>>>> > >>>>>>>> But that's not true: At least on x86 EFI gets compiled depending > >>> on > >>>>>>>> tool chain capabilities. Ultimately we may indeed want a user > >>>>>>>> selectable option here, but until then I'm afraid having this > >>> option > >>>>>>>> at all may be misleading on x86. > >>>>>>>> > >>>>>>> > >>>>>>> I check the build scripts, yes, you're right. For x86, EFI is not > >>> a > >>>>>>> selectable option in Kconfig. I agree with you, we can't use > >>> Kconfig > >>>>>>> system to decide to enable EFI build for x86 or not. > >>>>>>> > >>>>>>> So how about we just use this EFI option for Arm only? Because on > >>> Arm, > >>>>>>> we do not have such toolchain dependency. > >>>>>> > >>>>>> To be honest - don't know. That's because I don't know what you > want > >>>>>> to use the option for subsequently. > >>>>>> > >>>>> > >>>>> In last version, I had introduced an arch-helper to stub EFI_BOOT > >>>>> in Arm's common code for Arm32. Because Arm32 doesn't support EFI. > >>>>> So Julien suggested me to introduce a CONFIG_EFI option for non-EFI > >>>>> supported architectures to stub in EFI layer. > >>>>> > >>>>> [1] https://lists.xenproject.org/archives/html/xen-devel/2021- > >>>>> 08/msg00808.html > >>>>> > >>>> > >>>> As Jan' reminded, x86 doesn't depend on Kconfig to build EFI code. > >>>> So, if we CONFIG_EFI to stub EFI API's for x86, we will encounter > >>>> that toolchains enable EFI, but Kconfig disable EFI. Or Kconfig > >>>> enable EFI but toolchain doesn't provide EFI build supports. And > >>>> then x86 could not work well. > >>>> > >>>> If we use CONFIG_EFI for Arm only, that means CONFIG_EFI for x86 > >>>> is off, this will also cause problem. > >>>> > >>>> So, can we still use previous arch_helpers to stub for Arm32? > >>>> until x86 can use this selectable option? > >>> > >>> EFI doesn't have to be necessarily a user-visible option in Kconfig at > >>> this point. I think Julien was just asking to make the #ifdef based on > >>> a EFI-related config rather than just based CONFIG_ARM64. > >>> > >>> On x86 EFI is detected based on compiler support, setting > XEN_BUILD_EFI > >>> in xen/arch/x86/Makefile. Let's say that we keep using the same name > >>> "XEN_BUILD_EFI" on ARM as well. > >>> > >>> On ARM32, XEN_BUILD_EFI should be always unset. > >>> > >>> On ARM64 XEN_BUILD_EFI should be always set. > >>> > >>> That's it, right? I'd argue that CONFIG_EFI or HAS_EFI are better > names > >>> than XEN_BUILD_EFI, but that's OK anyway. So for instance you can make > >>> XEN_BUILD_EFI an invisible symbol in xen/arch/arm/Kconfig and select > it > >>> only on ARM64. > >> > >> Thanks, this is a good approach. But if we place XEN_BUILD_EFI in > Kconfig > >> it will be transfer to CONFIG_XEN_BUILD_EFI. How about using another > name > >> in Kconfig like ARM_EFI, but use CONFIG_ARM_EFI in config.h to define > >> XEN_BUILD_EFI? > > > > I am OK with that. Another option is to rename XEN_BUILD_EFI to > > CONFIG_XEN_BUILD_EFI on x86. Either way is fine by me. Jan, do you havea > > preference? > > Yes, I do: No new CONFIG_* settings please that don't originate from > Kconfig. Hence I'm afraid this is a "no" to your suggestion. > > Mid-term we should try to get rid of the remaining CONFIG_* which > get #define-d in e.g. asm/config.h. > I will do something like this: - introduce an ARM_EFI invisible symbol in kconfig, selected by ARM64 only - use CONFIG_ARM_EFI to define XEN_BUILD_EFI in config.h > Jan
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. > --- 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
Hi Jan, > -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 2022年1月25日 18:35 > To: Wei Chen <Wei.Chen@arm.com> > Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; xen- > devel@lists.xenproject.org; sstabellini@kernel.org; julien@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
Hi Jan, > -----Original Message----- > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Wei > Chen > Sent: 2022年1月27日 16:45 > To: Jan Beulich <jbeulich@suse.com> > Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; xen- > devel@lists.xenproject.org; sstabellini@kernel.org; julien@xen.org > Subject: RE: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for non- > EFI architecture > > Hi Jan, > > > -----Original Message----- > > From: Jan Beulich <jbeulich@suse.com> > > Sent: 2022年1月25日 18:35 > > To: Wei Chen <Wei.Chen@arm.com> > > Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; xen- > > devel@lists.xenproject.org; sstabellini@kernel.org; julien@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. > > I am sorry, I had missed this comment. In my latest changes, I have introduced a stub file for non-EFI architectures. The reason why we don't use a macro to stub the helpers in efi.h is that, some architectures have implemented stub helpers in their stub.c. If we define stub helpers in efi.h, this will cause function redefinition error. We need to fix this error for all architectures. And some helpers is not easy to implement as a inline function in efi.h. So we use stub file instead of stubing in efi.h > > Jan
On 27.01.2022 09:51, Wei Chen wrote: >> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Wei >> Chen >> Sent: 2022年1月27日 16:45 >> >>> From: Jan Beulich <jbeulich@suse.com> >>> Sent: 2022年1月25日 18:35 >>> >>> 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. > > I am sorry, I had missed this comment. In my latest changes, > I have introduced a stub file for non-EFI architectures. > The reason why we don't use a macro to stub the helpers > in efi.h is that, some architectures have implemented stub > helpers in their stub.c. If we define stub helpers in > efi.h, this will cause function redefinition error. We need > to fix this error for all architectures. And some helpers > is not easy to implement as a inline function in efi.h. > So we use stub file instead of stubing in efi.h But you realize we already have such a stub file on x86? Jan
Hi Jan, > -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 2022年1月27日 17:00 > To: Wei Chen <Wei.Chen@arm.com> > Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; xen- > devel@lists.xenproject.org; sstabellini@kernel.org; julien@xen.org > Subject: Re: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for non- > EFI architecture > > On 27.01.2022 09:51, Wei Chen wrote: > >> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of > Wei > >> Chen > >> Sent: 2022年1月27日 16:45 > >> > >>> From: Jan Beulich <jbeulich@suse.com> > >>> Sent: 2022年1月25日 18:35 > >>> > >>> 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. > > > > I am sorry, I had missed this comment. In my latest changes, > > I have introduced a stub file for non-EFI architectures. > > The reason why we don't use a macro to stub the helpers > > in efi.h is that, some architectures have implemented stub > > helpers in their stub.c. If we define stub helpers in > > efi.h, this will cause function redefinition error. We need > > to fix this error for all architectures. And some helpers > > is not easy to implement as a inline function in efi.h. > > So we use stub file instead of stubing in efi.h > > But you realize we already have such a stub file on x86? > Yes, we found the redefinition errors that are caused by x86 stub file and new macros in stub.h. We had tries to add: ifeq ($(XEN_BUILD_EFI),y) CFLAGS-y += -DXEN_BUILD_EFI XEN_CFLAGS += -DXEN_BUILD_EFI endif x86/Makefile to gate these new macros, but it seems that we may need to change EFI build logic for x86. It will cause more risks for me. So I want to introduce a similar stub.c in arch/arm. > Jan
On 27.01.2022 10:09, Wei Chen wrote: >> From: Jan Beulich <jbeulich@suse.com> >> Sent: 2022年1月27日 17:00 >> >> On 27.01.2022 09:51, Wei Chen wrote: >>>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of >> Wei >>>> Chen >>>> Sent: 2022年1月27日 16:45 >>>> >>>>> From: Jan Beulich <jbeulich@suse.com> >>>>> Sent: 2022年1月25日 18:35 >>>>> >>>>> 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. >>> >>> I am sorry, I had missed this comment. In my latest changes, >>> I have introduced a stub file for non-EFI architectures. >>> The reason why we don't use a macro to stub the helpers >>> in efi.h is that, some architectures have implemented stub >>> helpers in their stub.c. If we define stub helpers in >>> efi.h, this will cause function redefinition error. We need >>> to fix this error for all architectures. And some helpers >>> is not easy to implement as a inline function in efi.h. >>> So we use stub file instead of stubing in efi.h >> >> But you realize we already have such a stub file on x86? >> > > Yes, we found the redefinition errors that are caused by x86 stub file > and new macros in stub.h. We had tries to add: > ifeq ($(XEN_BUILD_EFI),y) > CFLAGS-y += -DXEN_BUILD_EFI > XEN_CFLAGS += -DXEN_BUILD_EFI > endif > x86/Makefile to gate these new macros, but it seems that we may need > to change EFI build logic for x86. It will cause more risks for me. > So I want to introduce a similar stub.c in arch/arm. While that's perhaps fine, ideally common bits would be common. Iirc already back at the time I was wondering why stub.c had to be x86- only. Jan
Hi Jan, > -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 2022年1月27日 17:17 > To: Wei Chen <Wei.Chen@arm.com> > Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; xen- > devel@lists.xenproject.org; sstabellini@kernel.org; julien@xen.org > Subject: Re: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for non- > EFI architecture > > On 27.01.2022 10:09, Wei Chen wrote: > >> From: Jan Beulich <jbeulich@suse.com> > >> Sent: 2022年1月27日 17:00 > >> > >> On 27.01.2022 09:51, Wei Chen wrote: > >>>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of > >> Wei > >>>> Chen > >>>> Sent: 2022年1月27日 16:45 > >>>> > >>>>> From: Jan Beulich <jbeulich@suse.com> > >>>>> Sent: 2022年1月25日 18:35 > >>>>> > >>>>> 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. > >>> > >>> I am sorry, I had missed this comment. In my latest changes, > >>> I have introduced a stub file for non-EFI architectures. > >>> The reason why we don't use a macro to stub the helpers > >>> in efi.h is that, some architectures have implemented stub > >>> helpers in their stub.c. If we define stub helpers in > >>> efi.h, this will cause function redefinition error. We need > >>> to fix this error for all architectures. And some helpers > >>> is not easy to implement as a inline function in efi.h. > >>> So we use stub file instead of stubing in efi.h > >> > >> But you realize we already have such a stub file on x86? > >> > > > > Yes, we found the redefinition errors that are caused by x86 stub file > > and new macros in stub.h. We had tries to add: > > ifeq ($(XEN_BUILD_EFI),y) > > CFLAGS-y += -DXEN_BUILD_EFI > > XEN_CFLAGS += -DXEN_BUILD_EFI > > endif > > x86/Makefile to gate these new macros, but it seems that we may need > > to change EFI build logic for x86. It will cause more risks for me. > > So I want to introduce a similar stub.c in arch/arm. > > While that's perhaps fine, ideally common bits would be common. Iirc > already back at the time I was wondering why stub.c had to be x86- > only. Some dummy functions in stub.c can be shared by arm or other architectures. But some functions like efi_multiboot2 are architecture dependent. > > Jan
On 27.01.2022 10:25, Wei Chen wrote: >> From: Jan Beulich <jbeulich@suse.com> >> Sent: 2022年1月27日 17:17 >> >> On 27.01.2022 10:09, Wei Chen wrote: >>>> From: Jan Beulich <jbeulich@suse.com> >>>> Sent: 2022年1月27日 17:00 >>>> >>>> But you realize we already have such a stub file on x86? >>>> >>> >>> Yes, we found the redefinition errors that are caused by x86 stub file >>> and new macros in stub.h. We had tries to add: >>> ifeq ($(XEN_BUILD_EFI),y) >>> CFLAGS-y += -DXEN_BUILD_EFI >>> XEN_CFLAGS += -DXEN_BUILD_EFI >>> endif >>> x86/Makefile to gate these new macros, but it seems that we may need >>> to change EFI build logic for x86. It will cause more risks for me. >>> So I want to introduce a similar stub.c in arch/arm. >> >> While that's perhaps fine, ideally common bits would be common. Iirc >> already back at the time I was wondering why stub.c had to be x86- >> only. > > Some dummy functions in stub.c can be shared by arm or other architectures. > But some functions like efi_multiboot2 are architecture dependent. Right - that's no different from the bulk of the non-stubbed EFI code. I don't really mind you making an Arm-specific stub file if there's not very much duplication, but it doesn't feel very good. In the end it's up to the Arm maintainers anyway. Jan
Hi, On 27/01/2022 09:27, Jan Beulich wrote: > On 27.01.2022 10:25, Wei Chen wrote: >>> From: Jan Beulich <jbeulich@suse.com> >>> Sent: 2022年1月27日 17:17 >>> >>> On 27.01.2022 10:09, Wei Chen wrote: >>>>> From: Jan Beulich <jbeulich@suse.com> >>>>> Sent: 2022年1月27日 17:00 >>>>> >>>>> But you realize we already have such a stub file on x86? >>>>> >>>> >>>> Yes, we found the redefinition errors that are caused by x86 stub file >>>> and new macros in stub.h. We had tries to add: >>>> ifeq ($(XEN_BUILD_EFI),y) >>>> CFLAGS-y += -DXEN_BUILD_EFI >>>> XEN_CFLAGS += -DXEN_BUILD_EFI >>>> endif >>>> x86/Makefile to gate these new macros, but it seems that we may need >>>> to change EFI build logic for x86. It will cause more risks for me. >>>> So I want to introduce a similar stub.c in arch/arm. >>> >>> While that's perhaps fine, ideally common bits would be common. Iirc >>> already back at the time I was wondering why stub.c had to be x86- >>> only. >> >> Some dummy functions in stub.c can be shared by arm or other architectures. >> But some functions like efi_multiboot2 are architecture dependent. > > Right - that's no different from the bulk of the non-stubbed EFI code. > I don't really mind you making an Arm-specific stub file if there's > not very much duplication, but it doesn't feel very good. In the end > it's up to the Arm maintainers anyway. If the stubs are mostly the same then they should be common rather than duplicated. > x86/Makefile to gate these new macros, but it seems that we may need > to change EFI build logic for x86. It will cause more risks for me. But that would be a build risk right? If so, it is quite easy to verify/catch it compare to runtime issue. Cheers, > > Jan >
Hi Julien, > -----Original Message----- > From: Julien Grall <julien@xen.org> > Sent: 2022年1月27日 18:01 > To: Jan Beulich <jbeulich@suse.com>; Wei Chen <Wei.Chen@arm.com> > Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; xen- > devel@lists.xenproject.org; sstabellini@kernel.org > Subject: Re: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for non- > EFI architecture > > Hi, > > On 27/01/2022 09:27, Jan Beulich wrote: > > On 27.01.2022 10:25, Wei Chen wrote: > >>> From: Jan Beulich <jbeulich@suse.com> > >>> Sent: 2022年1月27日 17:17 > >>> > >>> On 27.01.2022 10:09, Wei Chen wrote: > >>>>> From: Jan Beulich <jbeulich@suse.com> > >>>>> Sent: 2022年1月27日 17:00 > >>>>> > >>>>> But you realize we already have such a stub file on x86? > >>>>> > >>>> > >>>> Yes, we found the redefinition errors that are caused by x86 stub > file > >>>> and new macros in stub.h. We had tries to add: > >>>> ifeq ($(XEN_BUILD_EFI),y) > >>>> CFLAGS-y += -DXEN_BUILD_EFI > >>>> XEN_CFLAGS += -DXEN_BUILD_EFI > >>>> endif > >>>> x86/Makefile to gate these new macros, but it seems that we may need > >>>> to change EFI build logic for x86. It will cause more risks for me. > >>>> So I want to introduce a similar stub.c in arch/arm. > >>> > >>> While that's perhaps fine, ideally common bits would be common. Iirc > >>> already back at the time I was wondering why stub.c had to be x86- > >>> only. > >> > >> Some dummy functions in stub.c can be shared by arm or other > architectures. > >> But some functions like efi_multiboot2 are architecture dependent. > > > > Right - that's no different from the bulk of the non-stubbed EFI code. > > I don't really mind you making an Arm-specific stub file if there's > > not very much duplication, but it doesn't feel very good. In the end > > it's up to the Arm maintainers anyway. > > If the stubs are mostly the same then they should be common rather than > duplicated. > > > x86/Makefile to gate these new macros, but it seems that we may need > > to change EFI build logic for x86. It will cause more risks for me. > > But that would be a build risk right? If so, it is quite easy to > verify/catch it compare to runtime issue. > Risk may be not very accurate word. I mean, I was afraid that my changes to x86 EFI build would introduce some unforeseen problems to x86. Because I am not very familiar with that. As you and Jan think it's better, I will try to do it. > Cheers, > > > > > Jan > > > > -- > Julien Grall
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 + ---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__ */
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(-)