[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


  • To: Wei Chen <Wei.Chen@xxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 27 Jan 2022 10:16:38 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=QgMJcaqGP+p1yHBFYVmlw1ktTEcpgcZXxS6jIjG8JEU=; b=PonTF3eUBq6YHGCiX2Q6Q0VTcJCuOlHtXvMhWWMmiMlS2QYhiaGAryksidjluYLdYqdkByZc7Ak3EiYRJSjZO5/4FxgGvHP7Qbednf8MAZtu3S8RmQRFJ7OlZmSjyogbX7+MTup5Lj5xBTfySOhvd6oV6H0BoMYnE+1s9tHLO2TBHI5d93PxDmbi2j3l2TGMq8640K4W3RQtmDltXONlYRgaj4m/PN2LI7U/6kxhN56BXpfKcp+7cyHjuzQVmGN7EB7M+gJRgBifTg96yX0CSmuV4CXgEuKmwT5jeA+4+RS13XV7mJNykPTtWYbZJ+C05olF1oqa36PUHZEV9R/DDQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=oSddCkKhbMns9zWmqaSrcPUFrin5r57xTA+Uk9VJRHuhqaGhhRuybpKdz1rXAaW0jgzYRutq8drUOFhykjsFCbZG5y7ae4rcc/tWC/s5Su/QaTtXC6jYaqEJ8djuMkgQpR/sDN3jMNc98Gg0p2WyHd2lRnKL+tNAjMROVzBiRiVKmjEHH3rSrkwRQCX+1swZkAdX6PNN/RxxzRuvjBTn74PEO+WDw8HJ2NgmI3wu3XSBKJ1d4oLw5a7VWPdjPi+LlamBSF6VFV4tLEuIa32Ili2HbAYKHKubBkAwH1MylFVS7aUUvfTNyLvdSf+CZhs3WBoKkGr4r3umLTJna98c4Q==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "sstabellini@xxxxxxxxxx" <sstabellini@xxxxxxxxxx>, "julien@xxxxxxx" <julien@xxxxxxx>
  • Delivery-date: Thu, 27 Jan 2022 09:16:59 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.