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

[Roy Franz]

On Mon, Mar 2, 2015 at 9:21 AM, Stefano Stabellini
<address@hidden> 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 <address@hidden>
>> ---
>>  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). */
>> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
>> index 74c01e3..cdd17cb 100644
>> --- a/xen/arch/x86/time.c
>> +++ b/xen/arch/x86/time.c
>> @@ -689,7 +689,7 @@ static unsigned long get_cmos_time(void)
>>      static bool_t __read_mostly cmos_rtc_probe;
>>      boolean_param("cmos-rtc-probe", cmos_rtc_probe);
>>
>> -    if ( efi_enabled )
>> +    if ( efi_platform )
>>      {
>>          res = efi_get_time();
>>          if ( res )
>> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
>> index ac6881e..8aafcfd 100644
>> --- a/xen/common/efi/boot.c
>> +++ b/xen/common/efi/boot.c
>> @@ -708,6 +708,11 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
>> *SystemTable)
>>      char *option_str;
>>      bool_t use_cfg_file;
>>
>> +#ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */
>> +    efi_platform = 1;
>> +    efi_loader = 1;
>> +#endif
>
> What exactly needs to be implemented on ARM?  I thought that EFI ARM
> support for Xen is pretty much complete.
>
EFI runtime services support has not been implemented for ARM.
I'll have to look to see how efi_platform and efi_loader relate to
that, I don't know off hand.

>
>>      efi_ih = ImageHandle;
>>      efi_bs = SystemTable->BootServices;
>>      efi_rs = SystemTable->RuntimeServices;
>> diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
>> index c840e08..b229c69 100644
>> --- a/xen/common/efi/runtime.c
>> +++ b/xen/common/efi/runtime.c
>> @@ -11,13 +11,14 @@ DEFINE_XEN_GUEST_HANDLE(CHAR16);
>>  #ifndef COMPAT
>>
>>  #ifdef CONFIG_ARM  /* Disabled until runtime services implemented */
>> -const bool_t efi_enabled = 0;
>> +const bool_t efi_platform = 0;
>>  #else
>>  # include <asm/i387.h>
>>  # include <asm/xstate.h>
>>  # include <public/platform.h>
>>
>> -const bool_t efi_enabled = 1;
>> +bool_t efi_platform = 0;
>> +bool_t efi_loader = 0;
>>  #endif
>>
>>  unsigned int __read_mostly efi_num_ct;
>> diff --git a/xen/drivers/acpi/osl.c b/xen/drivers/acpi/osl.c
>> index 93c983c..b066459 100644
>> --- a/xen/drivers/acpi/osl.c
>> +++ b/xen/drivers/acpi/osl.c
>> @@ -66,7 +66,7 @@ void __init acpi_os_vprintf(const char *fmt, va_list args)
>>
>>  acpi_physical_address __init acpi_os_get_root_pointer(void)
>>  {
>> -     if (efi_enabled) {
>> +     if (efi_platform) {
>>               if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
>>                       return efi.acpi20;
>>               else if (efi.acpi != EFI_INVALID_TABLE_ADDR)
>> diff --git a/xen/include/xen/efi.h b/xen/include/xen/efi.h
>> index 5e02724..54cd390 100644
>> --- 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;
>>
>>  #define EFI_INVALID_TABLE_ADDR (~0UL)
>>
>> --
>> 1.7.10.4
>>