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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.03.15 at 15:09, <lsorense@xxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, Mar 27, 2015 at 02:04:22PM +0000, Jan Beulich wrote:
>> >>> On 27.03.15 at 14:53, <andrew.cooper3@xxxxxxxxxx> wrote:
>> > On 27/03/15 13:43, Jan Beulich wrote:
>> >>>>> On 27.03.15 at 14:32, <daniel.kiper@xxxxxxxxxx> wrote:
>> >>> On Fri, Feb 20, 2015 at 04:17:35PM +0000, Jan Beulich wrote:
>> >>>>>>> On 30.01.15 at 18:54, <daniel.kiper@xxxxxxxxxx> 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.

Jan


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