Re: [Xen-devel] [PATCH v4 10/19] efi: move efi struct initialization to xen/common/lib.c

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Wed, Aug 17, 2016 at 09:56:39AM -0600, Jan Beulich wrote:
> >>> On 06.08.16 at 01:04, <daniel.kiper@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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

Here it is (I list just first email from thread in a given month):
  https://lists.xen.org/archives/html/xen-devel/2016-04/msg02186.html
  https://lists.xen.org/archives/html/xen-devel/2016-05/msg02659.html
  https://lists.xen.org/archives/html/xen-devel/2016-06/msg00124.html
  https://lists.xen.org/archives/html/xen-devel/2016-07/msg00530.html

> 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.

I see two solutions for this issue:
  - define efi struct members conditionally; this requires also
    some #ifs sprinkled over Xen code (not very nice) or other
    substantial changes,
  - replace efi.flags with efi_flags and leave existing code as is.

What is your choice?

Personally I prefer existing patch (maybe with minimal changes
suggested by you).

Daniel

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