Re: [Xen-devel] [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader

["Lennart Sorensen" <lsorense@csclub.uwaterloo.ca>]

On Fri, Mar 27, 2015 at 02:19:44PM +0000, Jan Beulich wrote:
> >>> On 27.03.15 at 15:09, <lsorense@csclub.uwaterloo.ca> wrote:
> > On Fri, Mar 27, 2015 at 02:04:22PM +0000, Jan Beulich wrote:
> >> >>> On 27.03.15 at 14:53, <andrew.cooper3@citrix.com> wrote:
> >> > On 27/03/15 13:43, Jan Beulich wrote:
> >> >>>>> On 27.03.15 at 14:32, <daniel.kiper@oracle.com> wrote:
> >> >>> On Fri, Feb 20, 2015 at 04:17:35PM +0000, Jan Beulich wrote:
> >> >>>>>>> On 30.01.15 at 18:54, <daniel.kiper@oracle.com> wrote:
> >> >>>>> We need more fine grained knowledge about EFI environment and check
> >> >>>>> for EFI platform and EFI loader separately to properly support
> >> >>>>> multiboot2 protocol.
> >> >>>> ... because of ... (i.e. I can't see from the description what the
> >> >>>> separation is good for). Looking at the comments you placed
> >> >>>> aside the variables doesn't help me either.
> >> >>>>
> >> >>>>> In general Xen loaded by this protocol uses
> >> >>>>> memory mappings and loaded modules in simliar way to Xen loaded
> >> >>>>> by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform
> >> >>>>> and efi_loader.
> >> >>>> And if I'm guessing things right, then introducing efi_loader but
> >> >>>> leaving efi_enabled alone (only converting where needed) would
> >> >>> efi_enabled is not fortunate name for new usage. Currently it means
> >> >>> that Xen binary have or does not have EFI support build in. However,
> >> >>> if we build in multiboot2 protocol into xen.gz then it means that
> >> >>> it can ran on legacy BIOS or EFI platform. So, I think that we
> >> >>> should rename efi_enabled to efi_platform because it will mean
> >> >>> that Xen runs on EFI platform or not.
> >> >> I disagree here.
> >> >>
> >> >>> efi_loader is used to differentiate between EFI native loader
> >> >>> and multiboot2 protocol.
> >> >> And I agree here.
> >> > 
> >> > I suppose "built with efi support" is known because of CONFIG_EFI or 
> >> > not, and doesn't need a variable.
> >> > 
> >> > However, "booted legacy" vs "booted EFI" does need distinguishing at 
> >> > runtime, as either is possible.
> >> 
> >> Right, but that's what efi_enabled is supposed to express after
> >> the change; there's no need to express "built with EFI support".
> >> It just so happens that right now, without all these changes,
> >> built-with-EFI-support == runs-on-EFI.
> > 
> > Then how about 'efi_booted' as the variable name.
> 
> Why should we rename a variable that's perfectly suitable for the
> purposes we have? Even more so that we don't just want to
> express that we were booted from EFI, but also that we're running
> in a respective environment, including using tables coming from
> EFI and using runtime services (unless specifically disabled). If
> anything we could follow Linux and make efi_enabled a bit mask.

OK, so it isn't just to tell that you booted from EFI.

-- 
Len Sorensen