[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] 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@xxxxxxxx> >> Sent: 2022年1月27日 17:00 >> >> On 27.01.2022 09:51, Wei Chen wrote: >>>> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of >> Wei >>>> Chen >>>> Sent: 2022年1月27日 16:45 >>>> >>>>> From: Jan Beulich <jbeulich@xxxxxxxx> >>>>> 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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |