[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: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Wei Chen <Wei.Chen@xxxxxxx>
  • Date: Thu, 27 Jan 2022 09:25:00 +0000
  • Accept-language: en-US
  • 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=Eda+o/eaVRL+ux6gK4p5TJDZR21Rzh0PM3SZM2Rf6hM=; b=ZWcu85iMKs3ZRexolBKXz9n8WSUHusObwJCENW6Iy6CGyUsjBAC8CBCfPeMIeYfk6eBU4jyta61rjCqK9kJU8xuWpea9nxH30QNhEF2nrOZaUseBGz2LpoJTbka+QZBbXRdJfrMIS4grZufengv+biv9bmOxsyrm1cfqtOiWbGtnLlf2FRPHmpsMI0WtyvBCfv0zwu1r5596/gIh0d4v7lUlug81wj60kRa9tpyGYVsZINXtdL7omcKbJ0LMgMwAZPbI567hlz/rplo9yNoLzZ5/BGd147nDUAcwCjGCl9X35LHby0jpIAyCXa65o0YjMYbJDSgUyDBDWHQEnXzzJQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nYoO41eCQQexXx2axcDfqGYecRq+qLpw7yDlt+wAXpfeuU1gxhGPLe6AUcdyxM0FItmzL1BLaHJavd+qmZnDTQWsk2jdhmuTsj+cX1mHWYDG4XXv3CGpJ2LLItXc6S0ynI7noYDAxpSkv8dgW1KIg3iO3NVsEP49kRzwwoF6A5SrKXQXU9qhgz6AX1HIuq51yU+mXJxwF5SRXGutCVVsJVqiKv0Oq4ep9NGDCrMEsJ0GR2cc27Pjmtr78anHqGhmI/ByUL+Da3gLRxTVD99elNCfvwxiqp1Afau4Th7BdwAIAp2yAH0KMkZr4N+aGv88pBRftP6GeakF7gs6wEAHYA==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.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:25:23 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHXsHMtR+l18+MmpE6rl/RjSXTBgqx0TXyAgAME0ECAAAH2oIAAA3YAgAAA4bCAAAOtAIAAAa7A
  • Thread-topic: [PATCH 20/37] xen: introduce CONFIG_EFI to stub API for non-EFI architecture

Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 2022年1月27日 17:17
> To: Wei Chen <Wei.Chen@xxxxxxx>
> Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>; xen-
> devel@xxxxxxxxxxxxxxxxxxxx; sstabellini@xxxxxxxxxx; julien@xxxxxxx
> 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@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.

Some dummy functions in stub.c can be shared by arm or other architectures.
But some functions like efi_multiboot2 are architecture dependent.

> 
> Jan


 


Rackspace

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