All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@suse.com>
To: Daniel Kiper <daniel.kiper@oracle.com>
Cc: Juergen Gross <JGross@suse.com>,
	sstabellini@kernel.org, andrew.cooper3@citrix.com,
	cardoe@cardoe.com, pgnet.dev@gmail.com, ning.sun@intel.com,
	david.vrabel@citrix.com, xen-devel@lists.xenproject.org,
	qiaowei.ren@intel.com, richard.l.maliszewski@intel.com,
	gang.wei@intel.com, fu.wei@linaro.org
Subject: Re: [PATCH v4 10/19] efi: move efi struct initialization to xen/common/lib.c
Date: Wed, 17 Aug 2016 09:56:39 -0600	[thread overview]
Message-ID: <57B4A5570200007800106C4C@prv-mh.provo.novell.com> (raw)
In-Reply-To: <1470438282-4226-11-git-send-email-daniel.kiper@oracle.com>

>>> On 06.08.16 at 01:04, <daniel.kiper@oracle.com> wrote:
> A subsequent patch adds efi struct flags member which is used
> during runtime to differentiate between legacy BIOS and EFI
> platforms and multiboot2 and EFI native loader. So, efi symbol
> have to proper representation in ELF and PE Xen image. Hence,
> move efi struct initialization to xen/common/lib.c and remove
> efi symbol from ld script.
> 
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> ---
> v4 - suggestions/fixes:
>    - move efi struct initialization to xen/common/lib.c
>      and drop one from xen/arch/x86/efi/stub.c
>      (suggested by Jan Beulich),

I recall I didn't like where you placed it last time round. I've just tried
to locate the old thread, but going back a whole year in the list archives
I was not able to find a mail with the title containing "move efi". Hence I
can only say what I think now, without reference to earlier remarks:
The struct currently isn't overly large, but I still don't see why non-EFI
builds need to include it instead of just the flags variable you mean to
introduce subsequently. And it's even less obvious what use it is on
platforms not even supporting EFI, i.e. ARM32.

> --- a/xen/common/lib.c
> +++ b/xen/common/lib.c
> @@ -1,4 +1,4 @@
> -
> +#include <xen/efi.h>
>  #include <xen/ctype.h>
>  #include <xen/lib.h>
>  #include <xen/types.h>

At least the visible section here is nicely sorted alphabetically, and I
don't think xen/efi.h absolutely needs to go first.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

  reply	other threads:[~2016-08-17 15:56 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-05 23:04 [PATCH v4 00/19] x86: multiboot2 protocol support Daniel Kiper
2016-08-05 23:04 ` [PATCH v4 01/19] x86: allow EFI reboot method neither on EFI platforms Daniel Kiper
2016-08-09 12:08   ` Jan Beulich
2016-08-05 23:04 ` [PATCH v4 02/19] x86/boot: remove multiboot1_header_end from symbol table Daniel Kiper
2016-08-09 13:24   ` Andrew Cooper
2016-08-09 13:52     ` Jan Beulich
2016-08-09 14:09       ` Andrew Cooper
2016-08-09 14:30         ` Jan Beulich
2016-08-05 23:04 ` [PATCH v4 03/19] x86/boot: create *.lnk files with linker script Daniel Kiper
2016-08-11 13:40   ` Jan Beulich
2016-08-05 23:04 ` [PATCH v4 04/19] x86/boot/reloc: reduce assembly usage as much as possible Daniel Kiper
2016-08-11 13:56   ` Jan Beulich
2016-08-05 23:04 ` [PATCH v4 05/19] x86/boot: call reloc() using stdcall calling convention Daniel Kiper
2016-08-11 13:59   ` Jan Beulich
2016-08-05 23:04 ` [PATCH v4 06/19] x86/boot/reloc: create generic alloc and copy functions Daniel Kiper
2016-08-11 14:12   ` Jan Beulich
2016-08-11 14:17     ` Jan Beulich
2016-08-18  8:53       ` Daniel Kiper
2016-08-18  9:41         ` Jan Beulich
2016-08-18 12:18           ` Daniel Kiper
2016-08-18 13:21             ` Jan Beulich
2016-08-05 23:04 ` [PATCH v4 07/19] x86/boot: use %ecx instead of %eax Daniel Kiper
2016-08-05 23:04 ` [PATCH v4 08/19] x86/boot/reloc: Rename some variables and rearrange code a bit Daniel Kiper
2016-08-11 14:16   ` Jan Beulich
2016-08-05 23:04 ` [PATCH v4 09/19] x86: add multiboot2 protocol support Daniel Kiper
2016-08-17 15:39   ` Jan Beulich
2016-08-18  9:23     ` Daniel Kiper
2016-08-18  9:43       ` Jan Beulich
2016-08-18 11:41         ` Daniel Kiper
2016-08-18 13:19           ` Jan Beulich
2016-08-05 23:04 ` [PATCH v4 10/19] efi: move efi struct initialization to xen/common/lib.c Daniel Kiper
2016-08-17 15:56   ` Jan Beulich [this message]
2016-08-18 10:17     ` Daniel Kiper
2016-08-18 11:17       ` Jan Beulich
2016-08-05 23:04 ` [PATCH v4 11/19] efi: create efi_enabled() Daniel Kiper
2016-08-17 16:08   ` Jan Beulich
2016-08-05 23:04 ` [PATCH v4 12/19] efi: introduce EFI_RS to ease control on runtime services usage Daniel Kiper
2016-08-17 16:12   ` Jan Beulich
2016-08-18 10:30     ` Daniel Kiper
2016-08-18 11:18       ` Jan Beulich
2016-08-18 11:49         ` Daniel Kiper
2016-08-05 23:04 ` [PATCH v4 13/19] efi: EFI_RS bit in efi.flags must be controlled by efi=[no-]rs command line argument Daniel Kiper
2016-08-05 23:04 ` [PATCH v4 14/19] efi: build xen.gz with EFI code Daniel Kiper
2016-08-19  9:24   ` Jan Beulich
2016-08-19 10:09     ` Daniel Kiper
2016-08-05 23:04 ` [PATCH v4 15/19] x86/efi: create new early memory allocator Daniel Kiper
2016-08-05 23:04 ` [PATCH v4 16/19] x86: add multiboot2 protocol support for EFI platforms Daniel Kiper
2016-08-05 23:04 ` [PATCH v4 17/19] x86/boot: implement early command line parser in C Daniel Kiper
2016-08-05 23:04 ` [PATCH v4 18/19] x86: make Xen early boot code relocatable Daniel Kiper
2016-08-05 23:04 ` [PATCH v4 19/19] x86: add multiboot2 protocol support for relocatable images Daniel Kiper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57B4A5570200007800106C4C@prv-mh.provo.novell.com \
    --to=jbeulich@suse.com \
    --cc=JGross@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=cardoe@cardoe.com \
    --cc=daniel.kiper@oracle.com \
    --cc=david.vrabel@citrix.com \
    --cc=fu.wei@linaro.org \
    --cc=gang.wei@intel.com \
    --cc=ning.sun@intel.com \
    --cc=pgnet.dev@gmail.com \
    --cc=qiaowei.ren@intel.com \
    --cc=richard.l.maliszewski@intel.com \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.