From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH v2 09/23] efi: create efi_enabled() Date: Thu, 20 Aug 2015 09:18:17 -0600 Message-ID: <55D60BD9020000780009C3CE@prv-mh.provo.novell.com> References: <1437402558-7313-1-git-send-email-daniel.kiper@oracle.com> <1437402558-7313-10-git-send-email-daniel.kiper@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZSRbj-0001BP-Mf for xen-devel@lists.xenproject.org; Thu, 20 Aug 2015 15:18:23 +0000 In-Reply-To: <1437402558-7313-10-git-send-email-daniel.kiper@oracle.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Daniel Kiper Cc: Juergen Gross , grub-devel@gnu.org, wei.liu2@citrix.com, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, roy.franz@linaro.org, ning.sun@intel.com, david.vrabel@citrix.com, phcoder@gmail.com, xen-devel@lists.xenproject.org, qiaowei.ren@intel.com, keir@xen.org, richard.l.maliszewski@intel.com, gang.wei@intel.com, fu.wei@linaro.org List-Id: xen-devel@lists.xenproject.org >>> On 20.07.15 at 16:29, wrote: > --- a/xen/arch/x86/efi/stub.c > +++ b/xen/arch/x86/efi/stub.c > @@ -4,9 +4,14 @@ > #include > #include > > -#ifndef efi_enabled > -const bool_t efi_enabled = 0; > -#endif > +struct efi __read_mostly efi = { > + .flags = 0, /* Initialized later. */ > + .acpi = EFI_INVALID_TABLE_ADDR, > + .acpi20 = EFI_INVALID_TABLE_ADDR, > + .mps = EFI_INVALID_TABLE_ADDR, > + .smbios = EFI_INVALID_TABLE_ADDR, > + .smbios3 = EFI_INVALID_TABLE_ADDR > +}; How is this change related to the subject of the patch? > --- a/xen/arch/x86/xen.lds.S > +++ b/xen/arch/x86/xen.lds.S > @@ -191,8 +191,6 @@ SECTIONS > .pad : { > . = ALIGN(MB(16)); > } :text > -#else > - efi = .; > #endif Same here. > --- a/xen/common/efi/boot.c > +++ b/xen/common/efi/boot.c > @@ -717,6 +717,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE > *SystemTable) > char *option_str; > bool_t use_cfg_file; > > +#ifndef CONFIG_ARM /* Disabled until runtime services implemented. */ > + set_bit(EFI_PLATFORM, &efi.flags); > +#endif Just for this to work? I don't see the need for all the pointers in the stub case - why can't this be a separate variable? We don't need to follow Linux with where the flags actually live... > --- a/xen/common/efi/runtime.c > +++ b/xen/common/efi/runtime.c > @@ -10,14 +10,10 @@ DEFINE_XEN_GUEST_HANDLE(CHAR16); > > #ifndef COMPAT > > -#ifdef CONFIG_ARM /* Disabled until runtime services implemented */ > -const bool_t efi_enabled = 0; > -#else > +#ifndef CONFIG_ARM > # include > # include > # include > - > -const bool_t efi_enabled = 1; > #endif Afaict the proper replacement (at least at this point in the series) for this would be to statically initialize the flag set in this xen.efi case. > @@ -40,6 +42,16 @@ 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 *); > > +/* > + * Test whether the above EFI_* bits are enabled. > + * > + * Stolen from Linux Kernel. I don't think this second part of the comment makes a lot of sense for a minor piece of functionality like this. Jan