Re: [Xen-devel] [PATCH for-4.5 V7 14/14] Add ARM EFI boot support

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.09.14 at 03:42, <roy.franz@xxxxxxxxxx> wrote:
> @@ -1060,6 +1071,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
> *SystemTable)
>      for( ; ; ); /* not reached */
>  }
>  
> +#ifndef CONFIG_ARM /* TODO - runtime service support */
>  #ifndef USE_SET_VIRTUAL_ADDRESS_MAP
>  static __init void copy_mapping(unsigned long mfn, unsigned long end,
>                                  bool_t (*is_valid)(unsigned long smfn,
> @@ -1285,3 +1297,4 @@ void __init efi_init_memory(void)
>          efi_l4_pgtable[i] = idle_pg_table[i];
>  #endif
>  }
> +#endif

So you want from moving this code block out in its entirety back to
leaving it in its entirety, despite there being clearly x86-specific
pieces. That's a little sad...

> --- a/xen/common/efi/efi.h
> +++ b/xen/common/efi/efi.h
> @@ -28,7 +28,9 @@ extern EFI_RUNTIME_SERVICES *efi_rs;
>  extern UINTN efi_memmap_size, efi_mdesc_size;
>  extern void *efi_memmap;
>  
> +#ifndef CONFIG_ARM
>  extern l4_pgentry_t *efi_l4_pgtable;
> +#endif

#ifdef CONFIG_X86 please.

> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -4,17 +4,22 @@
>  #include <xen/guest_access.h>
>  #include <xen/irq.h>
>  #include <xen/time.h>
> -#include <asm/mc146818rtc.h>
> +
> +extern spinlock_t rtc_lock;

No extern declarations in .c files for things being defined in other .c
files please.

> @@ -24,7 +29,6 @@ unsigned int __read_mostly efi_fw_revision;
>  const CHAR16 *__read_mostly efi_fw_vendor;
>  
>  EFI_RUNTIME_SERVICES *__read_mostly efi_rs;
> -static DEFINE_SPINLOCK(efi_rs_lock);
>  
>  UINTN __read_mostly efi_memmap_size;
>  UINTN __read_mostly efi_mdesc_size;
> @@ -41,10 +45,11 @@ struct efi __read_mostly efi = {
>       .smbios = EFI_INVALID_TABLE_ADDR,
>  };
>  
> -l4_pgentry_t *__read_mostly efi_l4_pgtable;
> -
>  const struct efi_pci_rom *__read_mostly efi_pci_roms;
>  
> +#ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */
> +static DEFINE_SPINLOCK(efi_rs_lock);
> +l4_pgentry_t *__read_mostly efi_l4_pgtable;
>  unsigned long efi_rs_enter(void)

Blank lines missing.

> @@ -135,7 +140,9 @@ void efi_reset_system(bool_t warm)
>  }
>  
>  #endif
> +#endif

Please attach a /* CONFIG_ARM */ comment to the right one of
those two (at once making clear without having to consult the
actual file that the #ifndef below can't be folded with this one).

Jan

> +#ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */
>  int efi_get_info(uint32_t idx, union xenpf_efi_info *info)
>  {
>      unsigned int i, n;



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