[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>> On 26.09.14 at 06:42, <roy.franz@xxxxxxxxxx> wrote:
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -18,9 +18,17 @@
>  #include <xen/string.h>
>  #include <xen/stringify.h>
>  #include <xen/vga.h>
> +#ifdef CONFIG_X86
> +/*
> + * Keep this arch-specific modified include in the common file, as moving
> + * it to the arch specific include file would obscure that special care is
> + * taken to include it with __ASSEMBLY__ defined.
> + */
>  #define __ASSEMBLY__ /* avoid pulling in ACPI stuff (conflicts with EFI) */
>  #include <asm/fixmap.h>
>  #undef __ASSEMBLY__
> +#endif
> +#include <xen/bitops.h>

This belongs ahead of the asm/ one(s). And note that they're all
sorted alphabetically (in order to reduce collisions on racing patches
by default inserting at the end).

> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -4,17 +4,23 @@
>  #include <xen/guest_access.h>
>  #include <xen/irq.h>
>  #include <xen/time.h>
> +#ifdef CONFIG_X86
>  #include <asm/mc146818rtc.h>
> +#endif

As said elsewhere - please introduce runtime.h to hold this on the
x86 side.

> @@ -24,7 +30,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);

Even at the expense of a few more temporary #ifdef-s - please
don't move this down, it got placed next to efi_rs for a reason.

> @@ -41,10 +46,12 @@ 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;

This again is a candidate for movement into runtime.h.

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.