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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.09.2021 07:01, Stefano Stabellini wrote:
> On Tue, 28 Sep 2021, Wei Chen wrote:
>>> -----Original Message-----
>>> From: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>>> Sent: 2021年9月28日 9:00
>>> To: Wei Chen <Wei.Chen@xxxxxxx>
>>> Cc: Jan Beulich <jbeulich@xxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
>>> julien@xxxxxxx; Bertrand Marquis <Bertrand.Marquis@xxxxxxx>; Stefano
>>> Stabellini <sstabellini@xxxxxxxxxx>
>>> 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@xxxxxxxxxxxxxxxxxxxx> On Behalf Of
>>> Wei
>>>>> Chen
>>>>> Sent: 2021年9月26日 18:25
>>>>> To: Jan Beulich <jbeulich@xxxxxxxx>
>>>>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; julien@xxxxxxx; Bertrand Marquis
>>>>> <Bertrand.Marquis@xxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>
>>>>> 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@xxxxxxxxxxxxxxxxxxxx> On Behalf
>>> Of
>>>>> Jan
>>>>>> Beulich
>>>>>> Sent: 2021年9月24日 18:49
>>>>>> To: Wei Chen <Wei.Chen@xxxxxxx>
>>>>>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; julien@xxxxxxx; Bertrand Marquis
>>>>>> <Bertrand.Marquis@xxxxxxx>; Stefano Stabellini
>>> <sstabellini@xxxxxxxxxx>
>>>>>> 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@xxxxxxxx>
>>>>>>>> Sent: 2021年9月24日 15:59
>>>>>>>>
>>>>>>>> On 24.09.2021 06:34, Wei Chen wrote:
>>>>>>>>>> From: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>>>>>>>>>> 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