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 17/18] x86/efi: create new early memory allocator

2015-03-02 Thread Roy Franz
On Mon, Mar 2, 2015 at 9:23 AM, Jan Beulich jbeul...@suse.com wrote:
 On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote:
 --- a/xen/arch/x86/efi/efi-boot.h
 +++ b/xen/arch/x86/efi/efi-boot.h
 @@ -103,9 +103,35 @@ static void __init relocate_trampoline(unsigned long 
 phys)
  *(u16 *)(*trampoline_ptr + (long)trampoline_ptr) = phys  4;
  }

 +#define __MALLOC_SIZE65536
 +
 +static char __malloc_mem[__MALLOC_SIZE];
 +static char *__malloc_free = NULL;
 +
 +static void __init *__malloc(size_t size)

 Please do away with all these prefixing underscores.

 +{
 +void *ptr;
 +
 +/*
 + * Init __malloc_free on runtime. Static initialization
 + * will not work because it puts virtual address there.
 + */
 +if ( __malloc_free == NULL )
 +__malloc_free = __malloc_mem;
 +
 +ptr = __malloc_free;
 +
 +__malloc_free += size;
 +
 +if ( __malloc_free - __malloc_mem  sizeof(__malloc_mem) )
 +blexit(LOut of static memory\r\n);
 +
 +return ptr;
 +}

 You're ignoring alignment requirements here altogether.

 @@ -192,12 +218,7 @@ static void __init
 efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,

  static void *__init efi_arch_allocate_mmap_buffer(UINTN *map_size)
  {
 -place_string(mbi.mem_upper, NULL);
 -mbi.mem_upper -= *map_size;
 -mbi.mem_upper = -__alignof__(EFI_MEMORY_DESCRIPTOR);
 -if ( mbi.mem_upper  xen_phys_start )
 -return NULL;
 -return (void *)(long)mbi.mem_upper;
 +return __malloc(*map_size);
  }

 Which then even suggests that _if_ we go this route, this could be
 shared with ARM (and hence become common code again).

 Jan


We could do the same thing on ARM.  For ARM, 2 allocations are done: 1
for the FDT, and
this one for the EFI memory map.  Both of these are currently
allocated with EFI allocation
functions, so don't have fixed size limits.  If we go with the fixed
size pool, I don't think that 64k
will be enough for the ARM case, as FDTs can be 20-50k in size.

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 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),
 +reserve_e820_ram(boot_e820