Re: [Xen-devel] [PATCH v3 10/16] efi: create efi_enabled()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.05.16 at 19:15, <daniel.kiper@xxxxxxxxxx> wrote:
> On Wed, May 25, 2016 at 01:20:23AM -0600, Jan Beulich wrote:
>> >>> On 15.04.16 at 14:33, <daniel.kiper@xxxxxxxxxx> wrote:
>> > --- a/xen/arch/x86/efi/stub.c
>> > +++ b/xen/arch/x86/efi/stub.c
>> > @@ -4,11 +4,8 @@
>> >  #include <xen/lib.h>
>> >  #include <asm/page.h>
>> >
>> > -#ifndef efi_enabled
>> > -const bool_t efi_enabled = 0;
>> > -#endif
>> > -
>> >  struct efi __read_mostly efi = {
>> > +  .flags   = 0, /* Initialized later. */
>>
>> This is pointless to add - the field will get zero-initialized anyway.
> 
> Sure thing. However, I think that we should be clear here that
> there is no default value for .flags (well, it is 0). Though if
> you wish I can remove that.

As you say, the initial value for flags is zero, with or without your
addition. Hence the addition is pointless.

>> > --- a/xen/include/xen/efi.h
>> > +++ b/xen/include/xen/efi.h
>> > @@ -2,15 +2,17 @@
>> >  #define __XEN_EFI_H__
>> >
>> >  #ifndef __ASSEMBLY__
>> > +#include <xen/bitops.h>
>> >  #include <xen/types.h>
>> >  #endif
>> >
>> > -extern const bool_t efi_enabled;
>> > -
>> >  #define EFI_INVALID_TABLE_ADDR (~0UL)
>> >
>> > +#define EFI_PLATFORM      0
>>
>> So what does "platform" mean? Did you consider using the more fine
> 
> It means "EFI platform". It differentiates from "legacy BIOS platform".

Well, that's what was clear from the beginning. The question however
was (taken together with the second one) what it means functionality
wise. The later addition makes clear it doesn't mean "loaded directly
from EFI". But looking at the various flags Linux has here, what
functionality does it imply? Does it e.g. mean runtime services are to
be used? If so, the flag would need to be cleared when their use if
being suppressed.

>> grained set of flags Linux uses nowadays? That would also eliminate
> 
> I wish to use just basic idea. However, I am not going to copy all
> stuff from Linux. We do not need that.

We don't need all of it, sure. But some more fine grained
identification of what functionality is available / to be used
would surely benefit us as a whole and your patch series in
particular.

Jan


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