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

[Roy Franz <roy.franz@xxxxxxxxxx>]

On Mon, Mar 2, 2015 at 10:43 AM, Roy Franz <roy.franz@xxxxxxxxxx> wrote:
> On Mon, Mar 2, 2015 at 9:21 AM, Stefano Stabellini
> <stefano.stabellini@xxxxxxxxxxxxx> 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.kiper@xxxxxxxxxx>
>>> ---
>>>  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.
>
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()
Not strictly EFI, but related:
* Lookup of ACPI tables
* Lookup of SMBIOS tables

Roy

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