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

2015-03-27 Thread Jan Beulich 
 On 27.03.15 at 14:32, daniel.ki...@oracle.com wrote:
 On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote:
  On 30.01.15 at 18:54, daniel.ki...@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.

Jan


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

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

2015-03-27 Thread Daniel Kiper 
On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote:
  On 30.01.15 at 18:54, daniel.ki...@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.

efi_loader is used to differentiate between EFI native loader
and multiboot2 protocol.

 seem the right approach.

[...]

  --- a/xen/include/xen/efi.h
  +++ b/xen/include/xen/efi.h
  @@ -5,7 +5,11 @@
   #include xen/types.h
   #endif
 
  -extern const bool_t efi_enabled;
  +/* If true then Xen runs on EFI platform. */
  +extern bool_t efi_platform;
  +
  +/* If true then Xen was loaded by native EFI loader as PE application. */
  +extern bool_t efi_loader;

 I don't see why you're losing the constness.

Both of them could be set during runtime. Please look above
for more details.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

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

2015-03-27 Thread Andrew Cooper 

On 27/03/15 13:43, Jan Beulich wrote:

On 27.03.15 at 14:32, daniel.ki...@oracle.com wrote:

On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote:

On 30.01.15 at 18:54, daniel.ki...@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.


~Andrew

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

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

2015-03-27 Thread Jan Beulich 
 On 27.03.15 at 14:53, andrew.coop...@citrix.com wrote:
 On 27/03/15 13:43, Jan Beulich wrote:
 On 27.03.15 at 14:32, daniel.ki...@oracle.com wrote:
 On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote:
 On 30.01.15 at 18:54, daniel.ki...@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.

Jan


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

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

2015-03-27 Thread Lennart Sorensen 
On Fri, Mar 27, 2015 at 02:04:22PM +, Jan Beulich wrote:
  On 27.03.15 at 14:53, andrew.coop...@citrix.com wrote:
  On 27/03/15 13:43, Jan Beulich wrote:
  On 27.03.15 at 14:32, daniel.ki...@oracle.com wrote:
  On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote:
  On 30.01.15 at 18:54, daniel.ki...@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.

-- 
Len Sorensen

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

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

2015-03-03 Thread Jan Beulich 
 On 03.03.15 at 00:40, roy.fr...@linaro.org wrote:
 Reviewing the #ifndef CONFIG_ARM in EFI code, and the efi_enabled
 usage elsewhere,
 the remaining EFI tasks on ARM look like:
 * Support for SetVirtualAddressMap
 * Runtime service support - looks like just time function used by x86
 in get_cmos_time()

* Provide runtime service access for Dom0

Jan

 Not strictly EFI, but related:
 * Lookup of ACPI tables
 * Lookup of SMBIOS tables
 
 Roy




___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

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

2015-03-02 Thread Roy Franz 
On Mon, Mar 2, 2015 at 9:21 AM, Stefano Stabellini
stefano.stabell...@eu.citrix.com wrote:
 On Fri, 30 Jan 2015, Daniel Kiper wrote:
 We need more fine grained knowledge about EFI environment and check
 for EFI platform and EFI loader separately to properly support
 multiboot2 protocol. 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.

 Signed-off-by: Daniel Kiper daniel.ki...@oracle.com
 ---
  xen/arch/x86/dmi_scan.c|4 ++--
  xen/arch/x86/domain_page.c |2 +-
  xen/arch/x86/efi/stub.c|5 +++--
  xen/arch/x86/mpparse.c |4 ++--
  xen/arch/x86/setup.c   |8 
  xen/arch/x86/time.c|2 +-
  xen/common/efi/boot.c  |5 +
  xen/common/efi/runtime.c   |5 +++--
  xen/drivers/acpi/osl.c |2 +-
  xen/include/xen/efi.h  |6 +-
  10 files changed, 27 insertions(+), 16 deletions(-)

 diff --git a/xen/arch/x86/dmi_scan.c b/xen/arch/x86/dmi_scan.c
 index 500133a..63b976c 100644
 --- a/xen/arch/x86/dmi_scan.c
 +++ b/xen/arch/x86/dmi_scan.c
 @@ -150,7 +150,7 @@ int __init dmi_get_table(u32 *base, u32 *len)
   struct dmi_eps eps;
   char __iomem *p, *q;

 - if (efi_enabled) {
 + if (efi_platform) {
   if (!efi_dmi_size)
   return -1;
   *base = efi_dmi_address;
 @@ -516,7 +516,7 @@ static void __init dmi_decode(struct dmi_header *dm)

  void __init dmi_scan_machine(void)
  {
 - if ((!efi_enabled ? dmi_iterate(dmi_decode) :
 + if ((!efi_platform ? dmi_iterate(dmi_decode) :
   dmi_efi_iterate(dmi_decode)) == 0)
   dmi_check_system(dmi_blacklist);
   else
 diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c
 index 158a164..5d4564c 100644
 --- a/xen/arch/x86/domain_page.c
 +++ b/xen/arch/x86/domain_page.c
 @@ -45,7 +45,7 @@ static inline struct vcpu *mapcache_current_vcpu(void)
  sync_local_execstate();
  /* We must now be running on the idle page table. */
  ASSERT((cr3 = read_cr3()) == __pa(idle_pg_table) ||
 -   (efi_enabled  cr3 == efi_rs_page_table()));
 +   (efi_platform  cr3 == efi_rs_page_table()));
  }

  return v;
 diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c
 index b8f49f8..5060e6f 100644
 --- a/xen/arch/x86/efi/stub.c
 +++ b/xen/arch/x86/efi/stub.c
 @@ -3,8 +3,9 @@
  #include xen/init.h
  #include xen/lib.h

 -#ifndef efi_enabled
 -const bool_t efi_enabled = 0;
 +#ifndef efi_platform
 +bool_t efi_platform = 0;
 +bool_t efi_loader = 0;
  #endif

  void __init efi_init_memory(void) { }
 diff --git a/xen/arch/x86/mpparse.c b/xen/arch/x86/mpparse.c
 index a38e016..c4e3041 100644
 --- a/xen/arch/x86/mpparse.c
 +++ b/xen/arch/x86/mpparse.c
 @@ -540,7 +540,7 @@ static inline void __init 
 construct_default_ISA_mptable(int mpc_default_type)

  static __init void efi_unmap_mpf(void)
  {
 - if (efi_enabled)
 + if (efi_platform)
   __set_fixmap(FIX_EFI_MPF, 0, 0);
  }

 @@ -698,7 +698,7 @@ void __init find_smp_config (void)
  {
   unsigned int address;

 - if (efi_enabled) {
 + if (efi_platform) {
   efi_check_config();
   return;
   }
 diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
 index c27c49c..711fdb0 100644
 --- a/xen/arch/x86/setup.c
 +++ b/xen/arch/x86/setup.c
 @@ -431,7 +431,7 @@ static void __init parse_video_info(void)
  struct boot_video_info *bvi = bootsym(boot_vid_info);

  /* The EFI loader fills vga_console_info directly. */
 -if ( efi_enabled )
 +if ( efi_platform )
  return;

  if ( (bvi-orig_video_isVGA == 1)  (bvi-orig_video_mode == 3) )
 @@ -663,7 +663,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  if ( ((unsigned long)cpu0_stack  (STACK_SIZE-1)) != 0 )
  panic(Misaligned CPU0 stack.);

 -if ( efi_enabled )
 +if ( efi_loader )
  {
  set_pdx_range(xen_phys_start  PAGE_SHIFT,
(xen_phys_start + BOOTSTRAP_MAP_BASE)  PAGE_SHIFT);
 @@ -774,7 +774,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
   * we can relocate the dom0 kernel and other multiboot modules. Also, on
   * x86/64, we relocate Xen to higher memory.
   */
 -for ( i = 0; !efi_enabled  i  mbi-mods_count; i++ )
 +for ( i = 0; !efi_loader  i  mbi-mods_count; i++ )
  {
  if ( mod[i].mod_start  (PAGE_SIZE - 1) )
  panic(Bootloader didn't honor module alignment request.);
 @@ -962,7 +962,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)

  if ( !xen_phys_start )
  panic(Not enough memory to relocate Xen.);
 -reserve_e820_ram(boot_e820, efi_enabled ? mbi-mem_upper : 
 __pa(_start),
 +reserve_e820_ram(boot_e820, efi_loader ? mbi-mem_upper : 
 __pa(_start),

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

2015-03-02 Thread Roy Franz 
On Mon, Mar 2, 2015 at 10:43 AM, Roy Franz roy.fr...@linaro.org wrote:
 On Mon, Mar 2, 2015 at 9:21 AM, Stefano Stabellini
 stefano.stabell...@eu.citrix.com wrote:
 On Fri, 30 Jan 2015, Daniel Kiper wrote:
 We need more fine grained knowledge about EFI environment and check
 for EFI platform and EFI loader separately to properly support
 multiboot2 protocol. 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.

 Signed-off-by: Daniel Kiper daniel.ki...@oracle.com
 ---
  xen/arch/x86/dmi_scan.c|4 ++--
  xen/arch/x86/domain_page.c |2 +-
  xen/arch/x86/efi/stub.c|5 +++--
  xen/arch/x86/mpparse.c |4 ++--
  xen/arch/x86/setup.c   |8 
  xen/arch/x86/time.c|2 +-
  xen/common/efi/boot.c  |5 +
  xen/common/efi/runtime.c   |5 +++--
  xen/drivers/acpi/osl.c |2 +-
  xen/include/xen/efi.h  |6 +-
  10 files changed, 27 insertions(+), 16 deletions(-)

 diff --git a/xen/arch/x86/dmi_scan.c b/xen/arch/x86/dmi_scan.c
 index 500133a..63b976c 100644
 --- a/xen/arch/x86/dmi_scan.c
 +++ b/xen/arch/x86/dmi_scan.c
 @@ -150,7 +150,7 @@ int __init dmi_get_table(u32 *base, u32 *len)
   struct dmi_eps eps;
   char __iomem *p, *q;

 - if (efi_enabled) {
 + if (efi_platform) {
   if (!efi_dmi_size)
   return -1;
   *base = efi_dmi_address;
 @@ -516,7 +516,7 @@ static void __init dmi_decode(struct dmi_header *dm)

  void __init dmi_scan_machine(void)
  {
 - if ((!efi_enabled ? dmi_iterate(dmi_decode) :
 + if ((!efi_platform ? dmi_iterate(dmi_decode) :
   dmi_efi_iterate(dmi_decode)) == 0)
   dmi_check_system(dmi_blacklist);
   else
 diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c
 index 158a164..5d4564c 100644
 --- a/xen/arch/x86/domain_page.c
 +++ b/xen/arch/x86/domain_page.c
 @@ -45,7 +45,7 @@ static inline struct vcpu *mapcache_current_vcpu(void)
  sync_local_execstate();
  /* We must now be running on the idle page table. */
  ASSERT((cr3 = read_cr3()) == __pa(idle_pg_table) ||
 -   (efi_enabled  cr3 == efi_rs_page_table()));
 +   (efi_platform  cr3 == efi_rs_page_table()));
  }

  return v;
 diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c
 index b8f49f8..5060e6f 100644
 --- a/xen/arch/x86/efi/stub.c
 +++ b/xen/arch/x86/efi/stub.c
 @@ -3,8 +3,9 @@
  #include xen/init.h
  #include xen/lib.h

 -#ifndef efi_enabled
 -const bool_t efi_enabled = 0;
 +#ifndef efi_platform
 +bool_t efi_platform = 0;
 +bool_t efi_loader = 0;
  #endif

  void __init efi_init_memory(void) { }
 diff --git a/xen/arch/x86/mpparse.c b/xen/arch/x86/mpparse.c
 index a38e016..c4e3041 100644
 --- a/xen/arch/x86/mpparse.c
 +++ b/xen/arch/x86/mpparse.c
 @@ -540,7 +540,7 @@ static inline void __init 
 construct_default_ISA_mptable(int mpc_default_type)

  static __init void efi_unmap_mpf(void)
  {
 - if (efi_enabled)
 + if (efi_platform)
   __set_fixmap(FIX_EFI_MPF, 0, 0);
  }

 @@ -698,7 +698,7 @@ void __init find_smp_config (void)
  {
   unsigned int address;

 - if (efi_enabled) {
 + if (efi_platform) {
   efi_check_config();
   return;
   }
 diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
 index c27c49c..711fdb0 100644
 --- a/xen/arch/x86/setup.c
 +++ b/xen/arch/x86/setup.c
 @@ -431,7 +431,7 @@ static void __init parse_video_info(void)
  struct boot_video_info *bvi = bootsym(boot_vid_info);

  /* The EFI loader fills vga_console_info directly. */
 -if ( efi_enabled )
 +if ( efi_platform )
  return;

  if ( (bvi-orig_video_isVGA == 1)  (bvi-orig_video_mode == 3) )
 @@ -663,7 +663,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  if ( ((unsigned long)cpu0_stack  (STACK_SIZE-1)) != 0 )
  panic(Misaligned CPU0 stack.);

 -if ( efi_enabled )
 +if ( efi_loader )
  {
  set_pdx_range(xen_phys_start  PAGE_SHIFT,
(xen_phys_start + BOOTSTRAP_MAP_BASE)  PAGE_SHIFT);
 @@ -774,7 +774,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
   * we can relocate the dom0 kernel and other multiboot modules. Also, 
 on
   * x86/64, we relocate Xen to higher memory.
   */
 -for ( i = 0; !efi_enabled  i  mbi-mods_count; i++ )
 +for ( i = 0; !efi_loader  i  mbi-mods_count; i++ )
  {
  if ( mod[i].mod_start  (PAGE_SIZE - 1) )
  panic(Bootloader didn't honor module alignment request.);
 @@ -962,7 +962,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)

  if ( !xen_phys_start )
  panic(Not enough memory to relocate Xen.);
 -reserve_e820_ram(boot_e820, efi_enabled ? mbi-mem_upper : 
 __pa(_start),
 +

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

2015-03-02 Thread Stefano Stabellini 
On Fri, 30 Jan 2015, Daniel Kiper wrote:
 We need more fine grained knowledge about EFI environment and check
 for EFI platform and EFI loader separately to properly support
 multiboot2 protocol. 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.
 
 Signed-off-by: Daniel Kiper daniel.ki...@oracle.com
 ---
  xen/arch/x86/dmi_scan.c|4 ++--
  xen/arch/x86/domain_page.c |2 +-
  xen/arch/x86/efi/stub.c|5 +++--
  xen/arch/x86/mpparse.c |4 ++--
  xen/arch/x86/setup.c   |8 
  xen/arch/x86/time.c|2 +-
  xen/common/efi/boot.c  |5 +
  xen/common/efi/runtime.c   |5 +++--
  xen/drivers/acpi/osl.c |2 +-
  xen/include/xen/efi.h  |6 +-
  10 files changed, 27 insertions(+), 16 deletions(-)
 
 diff --git a/xen/arch/x86/dmi_scan.c b/xen/arch/x86/dmi_scan.c
 index 500133a..63b976c 100644
 --- a/xen/arch/x86/dmi_scan.c
 +++ b/xen/arch/x86/dmi_scan.c
 @@ -150,7 +150,7 @@ int __init dmi_get_table(u32 *base, u32 *len)
   struct dmi_eps eps;
   char __iomem *p, *q;
  
 - if (efi_enabled) {
 + if (efi_platform) {
   if (!efi_dmi_size)
   return -1;
   *base = efi_dmi_address;
 @@ -516,7 +516,7 @@ static void __init dmi_decode(struct dmi_header *dm)
  
  void __init dmi_scan_machine(void)
  {
 - if ((!efi_enabled ? dmi_iterate(dmi_decode) :
 + if ((!efi_platform ? dmi_iterate(dmi_decode) :
   dmi_efi_iterate(dmi_decode)) == 0)
   dmi_check_system(dmi_blacklist);
   else
 diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c
 index 158a164..5d4564c 100644
 --- a/xen/arch/x86/domain_page.c
 +++ b/xen/arch/x86/domain_page.c
 @@ -45,7 +45,7 @@ static inline struct vcpu *mapcache_current_vcpu(void)
  sync_local_execstate();
  /* We must now be running on the idle page table. */
  ASSERT((cr3 = read_cr3()) == __pa(idle_pg_table) ||
 -   (efi_enabled  cr3 == efi_rs_page_table()));
 +   (efi_platform  cr3 == efi_rs_page_table()));
  }
  
  return v;
 diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c
 index b8f49f8..5060e6f 100644
 --- a/xen/arch/x86/efi/stub.c
 +++ b/xen/arch/x86/efi/stub.c
 @@ -3,8 +3,9 @@
  #include xen/init.h
  #include xen/lib.h
  
 -#ifndef efi_enabled
 -const bool_t efi_enabled = 0;
 +#ifndef efi_platform
 +bool_t efi_platform = 0;
 +bool_t efi_loader = 0;
  #endif
  
  void __init efi_init_memory(void) { }
 diff --git a/xen/arch/x86/mpparse.c b/xen/arch/x86/mpparse.c
 index a38e016..c4e3041 100644
 --- a/xen/arch/x86/mpparse.c
 +++ b/xen/arch/x86/mpparse.c
 @@ -540,7 +540,7 @@ static inline void __init 
 construct_default_ISA_mptable(int mpc_default_type)
  
  static __init void efi_unmap_mpf(void)
  {
 - if (efi_enabled)
 + if (efi_platform)
   __set_fixmap(FIX_EFI_MPF, 0, 0);
  }
  
 @@ -698,7 +698,7 @@ void __init find_smp_config (void)
  {
   unsigned int address;
  
 - if (efi_enabled) {
 + if (efi_platform) {
   efi_check_config();
   return;
   }
 diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
 index c27c49c..711fdb0 100644
 --- a/xen/arch/x86/setup.c
 +++ b/xen/arch/x86/setup.c
 @@ -431,7 +431,7 @@ static void __init parse_video_info(void)
  struct boot_video_info *bvi = bootsym(boot_vid_info);
  
  /* The EFI loader fills vga_console_info directly. */
 -if ( efi_enabled )
 +if ( efi_platform )
  return;
  
  if ( (bvi-orig_video_isVGA == 1)  (bvi-orig_video_mode == 3) )
 @@ -663,7 +663,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  if ( ((unsigned long)cpu0_stack  (STACK_SIZE-1)) != 0 )
  panic(Misaligned CPU0 stack.);
  
 -if ( efi_enabled )
 +if ( efi_loader )
  {
  set_pdx_range(xen_phys_start  PAGE_SHIFT,
(xen_phys_start + BOOTSTRAP_MAP_BASE)  PAGE_SHIFT);
 @@ -774,7 +774,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
   * we can relocate the dom0 kernel and other multiboot modules. Also, on
   * x86/64, we relocate Xen to higher memory.
   */
 -for ( i = 0; !efi_enabled  i  mbi-mods_count; i++ )
 +for ( i = 0; !efi_loader  i  mbi-mods_count; i++ )
  {
  if ( mod[i].mod_start  (PAGE_SIZE - 1) )
  panic(Bootloader didn't honor module alignment request.);
 @@ -962,7 +962,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  
  if ( !xen_phys_start )
  panic(Not enough memory to relocate Xen.);
 -reserve_e820_ram(boot_e820, efi_enabled ? mbi-mem_upper : 
 __pa(_start),
 +reserve_e820_ram(boot_e820, efi_loader ? mbi-mem_upper : __pa(_start),
   __pa(_end));
  
  /* Late kexec reservation (dynamic start address).

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

2015-02-20 Thread Jan Beulich 
 On 30.01.15 at 18:54, daniel.ki...@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
seem the right approach.

 --- a/xen/arch/x86/setup.c
 +++ b/xen/arch/x86/setup.c
 @@ -431,7 +431,7 @@ static void __init parse_video_info(void)
  struct boot_video_info *bvi = bootsym(boot_vid_info);
  
  /* The EFI loader fills vga_console_info directly. */
 -if ( efi_enabled )
 +if ( efi_platform )

This looks wrong considering the comment.

 @@ -663,7 +663,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  if ( ((unsigned long)cpu0_stack  (STACK_SIZE-1)) != 0 )
  panic(Misaligned CPU0 stack.);
  
 -if ( efi_enabled )
 +if ( efi_loader )
  {
  set_pdx_range(xen_phys_start  PAGE_SHIFT,
(xen_phys_start + BOOTSTRAP_MAP_BASE)  PAGE_SHIFT);
 @@ -774,7 +774,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
   * we can relocate the dom0 kernel and other multiboot modules. Also, on
   * x86/64, we relocate Xen to higher memory.
   */
 -for ( i = 0; !efi_enabled  i  mbi-mods_count; i++ )
 +for ( i = 0; !efi_loader  i  mbi-mods_count; i++ )
  {
  if ( mod[i].mod_start  (PAGE_SIZE - 1) )
  panic(Bootloader didn't honor module alignment request.);
 @@ -962,7 +962,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  
  if ( !xen_phys_start )
  panic(Not enough memory to relocate Xen.);
 -reserve_e820_ram(boot_e820, efi_enabled ? mbi-mem_upper : 
 __pa(_start),
 +reserve_e820_ram(boot_e820, efi_loader ? mbi-mem_upper : __pa(_start),

For all of these it's really hard to tell whether they're right without
knowing which parts of the current EFI loading code you intend to
re-use. Perhaps switching these would better be done along with
adding the re-using code (or splitting out the not re-used parts)?

 --- a/xen/include/xen/efi.h
 +++ b/xen/include/xen/efi.h
 @@ -5,7 +5,11 @@
  #include xen/types.h
  #endif
  
 -extern const bool_t efi_enabled;
 +/* If true then Xen runs on EFI platform. */
 +extern bool_t efi_platform;
 +
 +/* If true then Xen was loaded by native EFI loader as PE application. */
 +extern bool_t efi_loader;

I don't see why you're losing the constness.

Jan


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel