xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH for-4.5 V6 00/14] arm64 EFI stub
@ 2014-09-24  5:02 Roy Franz
  2014-09-24  5:02 ` [PATCH for-4.5 V6 01/14] move x86 EFI boot/runtime code to common/efi Roy Franz
                   ` (14 more replies)
  0 siblings, 15 replies; 30+ messages in thread
From: Roy Franz @ 2014-09-24  5:02 UTC (permalink / raw)
  To: xen-devel, ian.campbell, stefano.stabellini, tim, jbeulich, keir
  Cc: Roy Franz, fu.wei

This patch series adds EFI boot support for arm64.  A PE/COFF header is created
in head.S, as there is no toolchain support for PE/COFF on arm64.  This also
has the advantage that the file is both an "Image" file and a PE/COFF
executable - the same binary can be loaded and run either way.  The EFI 'stub'
code is a shim layer that serves as the loader for the XEN kernel in the EFI
environment.  The stub loads the dom0 kernel and initrd if required, and adds
entries for them as well as for the EFI data structures into the device tree
passed to XEN.  Once the device tree is constructed, EFI boot services are
exited, and the stub transfers control to the normal XEN entry point.  The only
indication XEN has that it was loaded via the stub is that the device tree
contains EFI properties.  This is all very similar to the arm/arm64 Linux
kernel EFI stubs.

Open feedback:
* moving extern declaration of efi_l4_pgtable.  I have left this x86 specific
  declaration in the x86 specific efi-boot.h file, as I don't see a clearly
  better place to put it.

Changes since v5:
* FDT and ARM cache flushing patches removed, as those have been applied
  to master branch
* rebased to master that includes above changesets.
* Added new patch fixing bug in PCI rom scan code. 
  (separate patch since it is a bug fix)
* efi-boot.h files moved from global include directories to arch/*/efi
  directories, as that is the only place they are used.
* removed redundant newlines in blexit() calls
* removed uneccessary initializers.
* reverted string variable changes that where left over from other reverted
  changes.
* removed local memory map variables, no longer required.
* made symlinks from commone/efi relative
* simplify get_argv() call with NULL parameter
* rename efi_arch_memory to efi_arch_memory_setup
* fixed type of local 'i' variable in new arch functions.  reviewed
  other local variables.
* fixed wstrlen coding style
* various formatting fixes - init ordering, remove declration reordering,
  blank lines, etc.
* moved x86 externs to appropriate global includes.
* fixed XXX placeholder in comments
* simplified ucode file handling, ignore anything after filename, as options
  not supported
* Added Acked-by lines for patches fully acked, or where minor changes were
  requested and made.

Changes since v4:
* Move runtime.c/compat.c/efi.h from arch/x86/efi to common/efi
* Create symbolic links at build time from common/efi to arch/xxx/efi
* Moved all non-VGA video code back to boot.c
* Cleaned up __init in forward declarations, moved declarations to common file
* Fixed type of i in efi_arch_process_memory_map()
* Added early/late arch cfg file functions - arm64 needs early, x86 wants late.
* Added #ifdefs to disable runtime services code that is not yet implemented for
  ARM (code moved back from arch specific file.)
  Global variables are left, as this allows common code setting these in boot
  code to be left.
* just provide arch specific allocator for EFI memory map, not function
  returning the map.
* Renamed truncate_string() to split_string()
* Fixed up MBI static inilization comments in moved code.
* Added explanation of ARM config file ordering requirement to commit message
  and code.
* Added explanation of increase of NR_MEM_BANKS
* Variety of minor formating cleanups.
* Remove mem-reserve regions from DTB in addition to the memory banks.  The EFI
  memory map is the only memory description that is used.

Changes since v3:
* Add symbolic link from common/efi/boot.c to arch/x86/efi/boot.c.  This
  simplifies the build system changes and resolves the parallel build issues
  in v3.  x86 EFI code build is unchanged, and ARM EFI code uses the normal
  common build structure. (This symlink seems to have confused git's moved file
  detection for boot.c)
* Fix XEN to be Xen throughout
* Fix spacing and missing blank lines between functions
* Add pseudo-prototype of efi_xen_start is head.S, change prototype to take
  pointer instead of integer to avoid cast, add comment explaining saving of x0
* Use uintptr_t in casts from uint to pointer for device tree.
* Change alignment requirements to 4k in PE/COFF header and alignment check
  in code.
* Rebased to latest master branch.

Changes since v2:
* Major refactor to use common EFI entry point and factor out arch specific
  code, rather than factoring out the common code.
* Update entire libfdt to v1.4.0 to provide fdt_create_empty_tree()

Changes since v1:
* Added common/efi directory for shared EFI code, and arch/arm/efi for 
  arm-specfic code.  Global build hacking of -fshort-wchar removed.
  arm32, arm64, and x86 with/without EFI enabled toolchain all build.
  The x86 build previously autodetected whether the EFI version should
  be built or not based on toolchain support.  I couldn't get this working
  nicely with the common code, so for x86 I have the common code always
  build, and the EFI autodection works as normal for building the EFI
  version.
* Basic use of the EFI memory map instead of FDT based memory description.
  More work needed to resolve differences between FDT description of 
  a small number of large memory banks with reserved regions, and EFI's
  potentially long list of available regions, which can be long.
* More refactoring of common EFI code to not directly exit using blexit(),
  as this broke the pre-linking targets.  All shared code returns status,
  and it is up to the caller to exit and clean up.
* Reduced the number of patches.  Refactoring of x86 code first, then moving
  all code to efi-shared.c in one patch.
* Fixed formatting/tab issues in new files, added Emacs footer.
* Fixed efi_get_memory_map to return NULL map pointer on error in addition
  to failed status.
* Added comments in head.S regarding PE/COFF specification, and 1:1 
  mapping used by EFI code.
* Updated device tree bindings to use new multiboot bindings.  Since the stub
  is always built into XEN, we don't have to support older bindings.

Roy Franz (14):
  move x86 EFI boot/runtime code to common/efi
  Move x86 specific funtions/variables to arch header
  create arch functions to allocate memory for and process EFI memory
    map.
  Add architecture functions for pre/post ExitBootServices
  Add efi_arch_cfg_file_early/late() to handle arch specific cfg file
    fields
  Add efi_arch_handle_cmdline() for processing commandline
  Move x86 specific disk probing code
  Create arch functions for console and video init
  Add efi_arch_memory() for arch specific memory setup
  Add arch specific module handling to read_file()
  Add several misc. arch functions for EFI boot code.
  Add efi_arch_use_config_file() function to control use of config file
  Fix freeing of uninitialized pointer
  Add ARM EFI boot support

 .gitignore                             |   8 +
 xen/Makefile                           |   4 +
 xen/arch/arm/Makefile                  |   1 +
 xen/arch/arm/arm64/head.S              | 150 +++++-
 xen/arch/arm/efi/Makefile              |   4 +
 xen/arch/arm/efi/efi-boot.h            | 593 ++++++++++++++++++++++++
 xen/arch/arm/xen.lds.S                 |   1 +
 xen/arch/x86/efi/efi-boot.h            | 637 ++++++++++++++++++++++++++
 xen/{arch/x86 => common}/efi/boot.c    | 810 ++++++++-------------------------
 xen/{arch/x86 => common}/efi/compat.c  |   0
 xen/{arch/x86 => common}/efi/efi.h     |   2 -
 xen/{arch/x86 => common}/efi/runtime.c |  15 +-
 xen/include/asm-arm/arm64/efibind.h    | 216 +++++++++
 xen/include/asm-arm/efibind.h          |   2 +
 xen/include/asm-arm/setup.h            |   2 +-
 xen/include/asm-x86/processor.h        |   1 +
 xen/include/xen/kernel.h               |   2 +-
 17 files changed, 1818 insertions(+), 630 deletions(-)
 create mode 100644 xen/arch/arm/efi/Makefile
 create mode 100644 xen/arch/arm/efi/efi-boot.h
 create mode 100644 xen/arch/x86/efi/efi-boot.h
 rename xen/{arch/x86 => common}/efi/boot.c (59%)
 rename xen/{arch/x86 => common}/efi/compat.c (100%)
 rename xen/{arch/x86 => common}/efi/efi.h (95%)
 rename xen/{arch/x86 => common}/efi/runtime.c (98%)
 create mode 100644 xen/include/asm-arm/arm64/efibind.h
 create mode 100644 xen/include/asm-arm/efibind.h

-- 
2.1.0

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH for-4.5 V6 01/14] move x86 EFI boot/runtime code to common/efi
  2014-09-24  5:02 [PATCH for-4.5 V6 00/14] arm64 EFI stub Roy Franz
@ 2014-09-24  5:02 ` Roy Franz
  2014-09-24 15:50   ` Jan Beulich
  2014-09-24  5:03 ` [PATCH for-4.5 V6 02/14] Move x86 specific funtions/variables to arch header Roy Franz
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 30+ messages in thread
From: Roy Franz @ 2014-09-24  5:02 UTC (permalink / raw)
  To: xen-devel, ian.campbell, stefano.stabellini, tim, jbeulich, keir
  Cc: Roy Franz, fu.wei

This moves the EFI boot and runtime services code to the common/efi directory.
This code is symbolicly linked back into the arch/x86/efi directory where it is
built if a build-time check for PE/COFF support in the toolchain passes.  In
the PE/COFF supporting case, both the EFI executable and the normal Xen image
(with stubbed EFI functions) are built.  We can't use the normal common build
infrastructure since we are building two versions at the same time, with
different EFI related code in each.  No code changes, just file movement and
make updates.  The files are symbolicly linked at build time back toe the
original arch/x86/efi directory.  This is in preparation for adding ARM EFI
support where much of these files can be shared.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
 .gitignore                             | 4 ++++
 xen/Makefile                           | 4 ++++
 xen/{arch/x86 => common}/efi/boot.c    | 0
 xen/{arch/x86 => common}/efi/compat.c  | 0
 xen/{arch/x86 => common}/efi/efi.h     | 0
 xen/{arch/x86 => common}/efi/runtime.c | 0
 6 files changed, 8 insertions(+)
 rename xen/{arch/x86 => common}/efi/boot.c (100%)
 rename xen/{arch/x86 => common}/efi/compat.c (100%)
 rename xen/{arch/x86 => common}/efi/efi.h (100%)
 rename xen/{arch/x86 => common}/efi/runtime.c (100%)

diff --git a/.gitignore b/.gitignore
index 8e34b85..35e4147 100644
--- a/.gitignore
+++ b/.gitignore
@@ -254,6 +254,10 @@ xen/arch/x86/efi.lds
 xen/arch/x86/efi/check.efi
 xen/arch/x86/efi/disabled
 xen/arch/x86/efi/mkreloc
+xen/arch/x86/efi/boot.c
+xen/arch/x86/efi/runtime.c
+xen/arch/x86/efi/compat.c
+xen/arch/x86/efi/efi.h
 xen/ddb/*
 xen/include/headers.chk
 xen/include/asm
diff --git a/xen/Makefile b/xen/Makefile
index 0e58bce..d3c9bf2 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -100,6 +100,10 @@ $(TARGET): delete-unfresh-files
 	$(MAKE) -C tools
 	$(MAKE) -f $(BASEDIR)/Rules.mk include/xen/compile.h
 	[ -e include/asm ] || ln -sf asm-$(TARGET_ARCH) include/asm
+	[ -e arch/$(TARGET_ARCH)/efi ] && ln -nsf ../../../common/efi/boot.c arch/$(TARGET_ARCH)/efi/;\
+			ln -nsf ../../../common/efi/runtime.c arch/$(TARGET_ARCH)/efi/;\
+			ln -nsf ../../../common/efi/compat.c arch/$(TARGET_ARCH)/efi/;\
+			ln -nsf ../../../common/efi/efi.h arch/$(TARGET_ARCH)/efi/;
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C include
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) asm-offsets.s
 	$(MAKE) -f $(BASEDIR)/Rules.mk include/asm-$(TARGET_ARCH)/asm-offsets.h
diff --git a/xen/arch/x86/efi/boot.c b/xen/common/efi/boot.c
similarity index 100%
rename from xen/arch/x86/efi/boot.c
rename to xen/common/efi/boot.c
diff --git a/xen/arch/x86/efi/compat.c b/xen/common/efi/compat.c
similarity index 100%
rename from xen/arch/x86/efi/compat.c
rename to xen/common/efi/compat.c
diff --git a/xen/arch/x86/efi/efi.h b/xen/common/efi/efi.h
similarity index 100%
rename from xen/arch/x86/efi/efi.h
rename to xen/common/efi/efi.h
diff --git a/xen/arch/x86/efi/runtime.c b/xen/common/efi/runtime.c
similarity index 100%
rename from xen/arch/x86/efi/runtime.c
rename to xen/common/efi/runtime.c
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH for-4.5 V6 02/14] Move x86 specific funtions/variables to arch header
  2014-09-24  5:02 [PATCH for-4.5 V6 00/14] arm64 EFI stub Roy Franz
  2014-09-24  5:02 ` [PATCH for-4.5 V6 01/14] move x86 EFI boot/runtime code to common/efi Roy Franz
@ 2014-09-24  5:03 ` Roy Franz
  2014-09-24 15:56   ` Jan Beulich
  2014-09-24  5:03 ` [PATCH for-4.5 V6 03/14] create arch functions to allocate memory for and process EFI memory map Roy Franz
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 30+ messages in thread
From: Roy Franz @ 2014-09-24  5:03 UTC (permalink / raw)
  To: xen-devel, ian.campbell, stefano.stabellini, tim, jbeulich, keir
  Cc: Roy Franz, fu.wei

Move the global variables and functions that can be moved as-is
from the common boot.c file to the x86 implementation header file.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
 xen/arch/x86/efi/efi-boot.h     | 132 +++++++++++++++++++++++++++++++++++
 xen/common/efi/boot.c           | 148 ++++------------------------------------
 xen/include/asm-x86/processor.h |   1 +
 xen/include/xen/kernel.h        |   2 +-
 4 files changed, 149 insertions(+), 134 deletions(-)
 create mode 100644 xen/arch/x86/efi/efi-boot.h

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
new file mode 100644
index 0000000..23712b2
--- /dev/null
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -0,0 +1,132 @@
+/*
+ * Architecture specific implementation for EFI boot code.  This file
+ * is intended to be included by common/efi/boot.c _only_, and
+ * therefore can define arch specific global variables.
+ */
+#include <asm/e820.h>
+#include <asm/edd.h>
+#define __ASSEMBLY__ /* avoid pulling in ACPI stuff (conflicts with EFI) */
+#include <asm/fixmap.h>
+#undef __ASSEMBLY__
+#include <asm/msr.h>
+#include <asm/processor.h>
+
+static struct file __initdata ucode;
+static multiboot_info_t __initdata mbi = {
+    .flags = MBI_MODULES | MBI_LOADERNAME
+};
+static module_t __initdata mb_modules[3];
+
+static void __init edd_put_string(u8 *dst, size_t n, const char *src)
+{
+    while ( n-- && *src )
+       *dst++ = *src++;
+    if ( *src )
+       PrintErrMesg(L"Internal error populating EDD info",
+                    EFI_BUFFER_TOO_SMALL);
+    while ( n-- )
+       *dst++ = ' ';
+}
+#define edd_put_string(d, s) edd_put_string(d, ARRAY_SIZE(d), s)
+
+extern const intpte_t __page_tables_start[], __page_tables_end[];
+#define in_page_tables(v) ((intpte_t *)(v) >= __page_tables_start && \
+                           (intpte_t *)(v) < __page_tables_end)
+
+#define PE_BASE_RELOC_ABS      0
+#define PE_BASE_RELOC_HIGHLOW  3
+#define PE_BASE_RELOC_DIR64   10
+
+extern const struct pe_base_relocs {
+    u32 rva;
+    u32 size;
+    u16 entries[];
+} __base_relocs_start[], __base_relocs_end[];
+
+static void __init efi_arch_relocate_image(unsigned long delta)
+{
+    const struct pe_base_relocs *base_relocs;
+
+    for ( base_relocs = __base_relocs_start; base_relocs < __base_relocs_end; )
+    {
+        unsigned int i, n;
+
+        n = (base_relocs->size - sizeof(*base_relocs)) /
+            sizeof(*base_relocs->entries);
+        for ( i = 0; i < n; ++i )
+        {
+            unsigned long addr = xen_phys_start + base_relocs->rva +
+                                 (base_relocs->entries[i] & 0xfff);
+
+            switch ( base_relocs->entries[i] >> 12 )
+            {
+            case PE_BASE_RELOC_ABS:
+                break;
+            case PE_BASE_RELOC_HIGHLOW:
+                if ( delta )
+                {
+                    *(u32 *)addr += delta;
+                    if ( in_page_tables(addr) )
+                        *(u32 *)addr += xen_phys_start;
+                }
+                break;
+            case PE_BASE_RELOC_DIR64:
+                if ( delta )
+                {
+                    *(u64 *)addr += delta;
+                    if ( in_page_tables(addr) )
+                        *(intpte_t *)addr += xen_phys_start;
+                }
+                break;
+            default:
+                blexit(L"Unsupported relocation type");
+            }
+        }
+        base_relocs = (const void *)(base_relocs->entries + i + (i & 1));
+    }
+}
+
+extern const s32 __trampoline_rel_start[], __trampoline_rel_stop[];
+extern const s32 __trampoline_seg_start[], __trampoline_seg_stop[];
+
+static void __init relocate_trampoline(unsigned long phys)
+{
+    const s32 *trampoline_ptr;
+
+    trampoline_phys = phys;
+    /* Apply relocations to trampoline. */
+    for ( trampoline_ptr = __trampoline_rel_start;
+          trampoline_ptr < __trampoline_rel_stop;
+          ++trampoline_ptr )
+        *(u32 *)(*trampoline_ptr + (long)trampoline_ptr) += phys;
+    for ( trampoline_ptr = __trampoline_seg_start;
+          trampoline_ptr < __trampoline_seg_stop;
+          ++trampoline_ptr )
+        *(u16 *)(*trampoline_ptr + (long)trampoline_ptr) = phys >> 4;
+}
+
+static void __init place_string(u32 *addr, const char *s)
+{
+    static char *__initdata alloc = start;
+
+    if ( s && *s )
+    {
+        size_t len1 = strlen(s) + 1;
+        const char *old = (char *)(long)*addr;
+        size_t len2 = *addr ? strlen(old) + 1 : 0;
+
+        alloc -= len1 + len2;
+        /*
+         * Insert new string before already existing one. This is needed
+         * for options passed on the command line to override options from
+         * the configuration file.
+         */
+        memcpy(alloc, s, len1);
+        if ( *addr )
+        {
+            alloc[len1 - 1] = ' ';
+            memcpy(alloc + len1, old, len2);
+        }
+    }
+    *addr = (long)alloc;
+}
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 3bdc158..4d5e310 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -18,13 +18,6 @@
 #include <xen/string.h>
 #include <xen/stringify.h>
 #include <xen/vga.h>
-#include <asm/e820.h>
-#include <asm/edd.h>
-#define __ASSEMBLY__ /* avoid pulling in ACPI stuff (conflicts with EFI) */
-#include <asm/fixmap.h>
-#undef __ASSEMBLY__
-#include <asm/msr.h>
-#include <asm/processor.h>
 
 /* Using SetVirtualAddressMap() is incompatible with kexec: */
 #undef USE_SET_VIRTUAL_ADDRESS_MAP
@@ -41,9 +34,6 @@ typedef struct {
     EFI_SHIM_LOCK_VERIFY Verify;
 } EFI_SHIM_LOCK_PROTOCOL;
 
-extern char start[];
-extern u32 cpuid_ext_features;
-
 union string {
     CHAR16 *w;
     char *s;
@@ -58,6 +48,13 @@ struct file {
     };
 };
 
+static CHAR16 *FormatDec(UINT64 Val, CHAR16 *Buffer);
+static CHAR16 *FormatHex(UINT64 Val, UINTN Width, CHAR16 *Buffer);
+static void  DisplayUint(UINT64 Val, INTN Width);
+static CHAR16 *wstrcpy(CHAR16 *d, const CHAR16 *s);
+static void noreturn blexit(const CHAR16 *str);
+static void PrintErrMesg(const CHAR16 *mesg, EFI_STATUS ErrCode);
+
 static EFI_BOOT_SERVICES *__initdata efi_bs;
 static EFI_HANDLE __initdata efi_ih;
 
@@ -69,19 +66,18 @@ static UINT32 __initdata mdesc_ver;
 static struct file __initdata cfg;
 static struct file __initdata kernel;
 static struct file __initdata ramdisk;
-static struct file __initdata ucode;
 static struct file __initdata xsm;
-
-static multiboot_info_t __initdata mbi = {
-    .flags = MBI_MODULES | MBI_LOADERNAME
-};
-static module_t __initdata mb_modules[3];
-
 static CHAR16 __initdata newline[] = L"\r\n";
 
 #define PrintStr(s) StdOut->OutputString(StdOut, s)
 #define PrintErr(s) StdErr->OutputString(StdErr, s)
 
+/*
+ * Include architecture specific implementation here, which references the
+ * static globals defined above.
+ */
+#include "efi-boot.h"
+
 static CHAR16 *__init FormatDec(UINT64 Val, CHAR16 *Buffer)
 {
     if ( Val >= 10 )
@@ -255,32 +251,6 @@ static void __init PrintErrMesg(const CHAR16 *mesg, EFI_STATUS ErrCode)
     blexit(mesg);
 }
 
-static void __init place_string(u32 *addr, const char *s)
-{
-    static char *__initdata alloc = start;
-
-    if ( s && *s )
-    {
-        size_t len1 = strlen(s) + 1;
-        const char *old = (char *)(long)*addr;
-        size_t len2 = *addr ? strlen(old) + 1 : 0;
-
-        alloc -= len1 + len2;
-        /*
-         * Insert new string before already existing one. This is needed
-         * for options passed on the command line to override options from
-         * the configuration file.
-         */
-        memcpy(alloc, s, len1);
-        if ( *addr )
-        {
-            alloc[len1 - 1] = ' ';
-            memcpy(alloc + len1, old, len2);
-        }
-    }
-    *addr = (long)alloc;
-}
-
 static unsigned int __init get_argv(unsigned int argc, CHAR16 **argv,
                                     CHAR16 *cmdline, UINTN cmdsize)
 {
@@ -574,18 +544,6 @@ static void __init split_value(char *s)
     *s = 0;
 }
 
-static void __init edd_put_string(u8 *dst, size_t n, const char *src)
-{
-    while ( n-- && *src )
-       *dst++ = *src++;
-    if ( *src )
-       PrintErrMesg(L"Internal error populating EDD info",
-                    EFI_BUFFER_TOO_SMALL);
-    while ( n-- )
-       *dst++ = ' ';
-}
-#define edd_put_string(d, s) edd_put_string(d, ARRAY_SIZE(d), s)
-
 static void __init setup_efi_pci(void)
 {
     EFI_STATUS status;
@@ -687,82 +645,6 @@ static int __init set_color(u32 mask, int bpp, u8 *pos, u8 *sz)
    return max(*pos + *sz, bpp);
 }
 
-extern const intpte_t __page_tables_start[], __page_tables_end[];
-#define in_page_tables(v) ((intpte_t *)(v) >= __page_tables_start && \
-                           (intpte_t *)(v) < __page_tables_end)
-
-#define PE_BASE_RELOC_ABS      0
-#define PE_BASE_RELOC_HIGHLOW  3
-#define PE_BASE_RELOC_DIR64   10
-
-extern const struct pe_base_relocs {
-    u32 rva;
-    u32 size;
-    u16 entries[];
-} __base_relocs_start[], __base_relocs_end[];
-
-static void __init relocate_image(unsigned long delta)
-{
-    const struct pe_base_relocs *base_relocs;
-
-    for ( base_relocs = __base_relocs_start; base_relocs < __base_relocs_end; )
-    {
-        unsigned int i, n;
-
-        n = (base_relocs->size - sizeof(*base_relocs)) /
-            sizeof(*base_relocs->entries);
-        for ( i = 0; i < n; ++i )
-        {
-            unsigned long addr = xen_phys_start + base_relocs->rva +
-                                 (base_relocs->entries[i] & 0xfff);
-
-            switch ( base_relocs->entries[i] >> 12 )
-            {
-            case PE_BASE_RELOC_ABS:
-                break;
-            case PE_BASE_RELOC_HIGHLOW:
-                if ( delta )
-                {
-                    *(u32 *)addr += delta;
-                    if ( in_page_tables(addr) )
-                        *(u32 *)addr += xen_phys_start;
-                }
-                break;
-            case PE_BASE_RELOC_DIR64:
-                if ( delta )
-                {
-                    *(u64 *)addr += delta;
-                    if ( in_page_tables(addr) )
-                        *(intpte_t *)addr += xen_phys_start;
-                }
-                break;
-            default:
-                blexit(L"Unsupported relocation type");
-            }
-        }
-        base_relocs = (const void *)(base_relocs->entries + i + (i & 1));
-    }
-}
-
-extern const s32 __trampoline_rel_start[], __trampoline_rel_stop[];
-extern const s32 __trampoline_seg_start[], __trampoline_seg_stop[];
-
-static void __init relocate_trampoline(unsigned long phys)
-{
-    const s32 *trampoline_ptr;
-
-    trampoline_phys = phys;
-    /* Apply relocations to trampoline. */
-    for ( trampoline_ptr = __trampoline_rel_start;
-          trampoline_ptr < __trampoline_rel_stop;
-          ++trampoline_ptr )
-        *(u32 *)(*trampoline_ptr + (long)trampoline_ptr) += phys;
-    for ( trampoline_ptr = __trampoline_seg_start;
-          trampoline_ptr < __trampoline_seg_stop;
-          ++trampoline_ptr )
-        *(u16 *)(*trampoline_ptr + (long)trampoline_ptr) = phys >> 4;
-}
-
 void EFIAPI __init noreturn
 efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 {
@@ -879,7 +761,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     PrintStr(L"Xen " __stringify(XEN_VERSION) "." __stringify(XEN_SUBVERSION)
              XEN_EXTRAVERSION " (c/s " XEN_CHANGESET ") EFI loader\r\n");
 
-    relocate_image(0);
+    efi_arch_relocate_image(0);
 
     if ( StdOut->QueryMode(StdOut, StdOut->Mode->Mode,
                            &cols, &rows) == EFI_SUCCESS )
@@ -1460,7 +1342,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START;
     efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START;
 
-    relocate_image(__XEN_VIRT_START - xen_phys_start);
+    efi_arch_relocate_image(__XEN_VIRT_START - xen_phys_start);
     memcpy((void *)trampoline_phys, trampoline_start, cfg.size);
 
     /* Set system registers and transfer control. */
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 9e1f210..f98eaf5 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -199,6 +199,7 @@ extern void set_cpuid_faulting(bool_t enable);
 
 extern u64 host_pat;
 extern bool_t opt_cpu_info;
+extern u32 cpuid_ext_features;
 
 /* Maximum width of physical addresses supported by the hardware */
 extern unsigned int paddr_bits;
diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
index 2c6d448..548b64d 100644
--- a/xen/include/xen/kernel.h
+++ b/xen/include/xen/kernel.h
@@ -65,7 +65,7 @@
 	1;                                      \
 })
 
-extern char _start[], _end[];
+extern char _start[], _end[], start[];
 #define is_kernel(p) ({                         \
     char *__p = (char *)(unsigned long)(p);     \
     (__p >= _start) && (__p < _end);            \
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH for-4.5 V6 03/14] create arch functions to allocate memory for and process EFI memory map.
  2014-09-24  5:02 [PATCH for-4.5 V6 00/14] arm64 EFI stub Roy Franz
  2014-09-24  5:02 ` [PATCH for-4.5 V6 01/14] move x86 EFI boot/runtime code to common/efi Roy Franz
  2014-09-24  5:03 ` [PATCH for-4.5 V6 02/14] Move x86 specific funtions/variables to arch header Roy Franz
@ 2014-09-24  5:03 ` Roy Franz
  2014-09-24 15:57   ` Jan Beulich
  2014-09-24  5:03 ` [PATCH for-4.5 V6 04/14] Add architecture functions for pre/post ExitBootServices Roy Franz
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 30+ messages in thread
From: Roy Franz @ 2014-09-24  5:03 UTC (permalink / raw)
  To: xen-devel, ian.campbell, stefano.stabellini, tim, jbeulich, keir
  Cc: Roy Franz, fu.wei

The memory used to store the EFI memory map is allocated in an architecture
specific way, and the processing of the memory map itself uses x86 specific
data structures. This patch adds architecture specific funtions so each
architecture can provide its own implementation.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
 xen/arch/x86/efi/efi-boot.h | 71 +++++++++++++++++++++++++++++++++++++++++++++
 xen/common/efi/boot.c       | 62 ++++-----------------------------------
 2 files changed, 77 insertions(+), 56 deletions(-)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 23712b2..d355115 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -130,3 +130,74 @@ static void __init place_string(u32 *addr, const char *s)
     }
     *addr = (long)alloc;
 }
+
+static void __init efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
+                                               void *map,
+                                               UINTN map_size,
+                                               UINTN desc_size,
+                                               UINT32 desc_ver)
+{
+    struct e820entry *e;
+    unsigned int i;
+
+    /* Populate E820 table and check trampoline area availability. */
+    e = e820map - 1;
+    for ( i = 0; i < map_size; i += desc_size )
+    {
+        EFI_MEMORY_DESCRIPTOR *desc = map + i;
+        u64 len = desc->NumberOfPages << EFI_PAGE_SHIFT;
+        u32 type;
+
+        switch ( desc->Type )
+        {
+        default:
+            type = E820_RESERVED;
+            break;
+        case EfiConventionalMemory:
+        case EfiBootServicesCode:
+        case EfiBootServicesData:
+            if ( !trampoline_phys && desc->PhysicalStart + len <= 0x100000 &&
+                 len >= cfg.size && desc->PhysicalStart + len > cfg.addr )
+                cfg.addr = (desc->PhysicalStart + len - cfg.size) & PAGE_MASK;
+            /* fall through */
+        case EfiLoaderCode:
+        case EfiLoaderData:
+            if ( desc->Attribute & EFI_MEMORY_WB )
+                type = E820_RAM;
+            else
+        case EfiUnusableMemory:
+                type = E820_UNUSABLE;
+            break;
+        case EfiACPIReclaimMemory:
+            type = E820_ACPI;
+            break;
+        case EfiACPIMemoryNVS:
+            type = E820_NVS;
+            break;
+        }
+        if ( e820nr && type == e->type &&
+             desc->PhysicalStart == e->addr + e->size )
+            e->size += len;
+        else if ( !len || e820nr >= E820MAX )
+            continue;
+        else
+        {
+            ++e;
+            e->addr = desc->PhysicalStart;
+            e->size = len;
+            e->type = type;
+            ++e820nr;
+        }
+    }
+
+}
+
+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;
+}
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 4d5e310..191a463 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -664,7 +664,6 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *mode_info;
     EFI_FILE_HANDLE dir_handle;
     union string section = { NULL }, name;
-    struct e820entry *e;
     u64 efer;
     bool_t base_video = 0;
 
@@ -966,8 +965,6 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     mbi.boot_loader_name = (long)"EFI";
     mbi.mods_addr = (long)mb_modules;
 
-    place_string(&mbi.mem_upper, NULL);
-
     /* Collect EDD info. */
     BUILD_BUG_ON(offsetof(struct edd_info, edd_device_params) != EDDEXTSIZE);
     BUILD_BUG_ON(sizeof(struct edd_device_params) != EDDPARMSIZE);
@@ -1264,65 +1261,18 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 
     efi_bs->GetMemoryMap(&efi_memmap_size, NULL, &map_key,
                          &efi_mdesc_size, &mdesc_ver);
-    mbi.mem_upper -= efi_memmap_size;
-    mbi.mem_upper &= -__alignof__(EFI_MEMORY_DESCRIPTOR);
-    if ( mbi.mem_upper < xen_phys_start )
-        blexit(L"Out of static memory");
-    efi_memmap = (void *)(long)mbi.mem_upper;
+    efi_memmap = efi_arch_allocate_mmap_buffer(efi_memmap_size);
+    if ( !efi_memmap )
+        blexit(L"ERROR Unable to allocate memory for EFI memory map");
+
     status = efi_bs->GetMemoryMap(&efi_memmap_size, efi_memmap, &map_key,
                                   &efi_mdesc_size, &mdesc_ver);
     if ( EFI_ERROR(status) )
         PrintErrMesg(L"Cannot obtain memory map", status);
 
-    /* Populate E820 table and check trampoline area availability. */
-    e = e820map - 1;
-    for ( i = 0; i < efi_memmap_size; i += efi_mdesc_size )
-    {
-        EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;
-        u64 len = desc->NumberOfPages << EFI_PAGE_SHIFT;
-        u32 type;
+    efi_arch_process_memory_map(SystemTable, efi_memmap, efi_memmap_size,
+                                efi_mdesc_size, mdesc_ver);
 
-        switch ( desc->Type )
-        {
-        default:
-            type = E820_RESERVED;
-            break;
-        case EfiConventionalMemory:
-        case EfiBootServicesCode:
-        case EfiBootServicesData:
-            if ( !trampoline_phys && desc->PhysicalStart + len <= 0x100000 &&
-                 len >= cfg.size && desc->PhysicalStart + len > cfg.addr )
-                cfg.addr = (desc->PhysicalStart + len - cfg.size) & PAGE_MASK;
-            /* fall through */
-        case EfiLoaderCode:
-        case EfiLoaderData:
-            if ( desc->Attribute & EFI_MEMORY_WB )
-                type = E820_RAM;
-            else
-        case EfiUnusableMemory:
-                type = E820_UNUSABLE;
-            break;
-        case EfiACPIReclaimMemory:
-            type = E820_ACPI;
-            break;
-        case EfiACPIMemoryNVS:
-            type = E820_NVS;
-            break;
-        }
-        if ( e820nr && type == e->type &&
-             desc->PhysicalStart == e->addr + e->size )
-            e->size += len;
-        else if ( !len || e820nr >= E820MAX )
-            continue;
-        else
-        {
-            ++e;
-            e->addr = desc->PhysicalStart;
-            e->size = len;
-            e->type = type;
-            ++e820nr;
-        }
-    }
     if ( !trampoline_phys )
     {
         if ( !cfg.addr )
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH for-4.5 V6 04/14] Add architecture functions for pre/post ExitBootServices
  2014-09-24  5:02 [PATCH for-4.5 V6 00/14] arm64 EFI stub Roy Franz
                   ` (2 preceding siblings ...)
  2014-09-24  5:03 ` [PATCH for-4.5 V6 03/14] create arch functions to allocate memory for and process EFI memory map Roy Franz
@ 2014-09-24  5:03 ` Roy Franz
  2014-09-24  5:03 ` [PATCH for-4.5 V6 05/14] Add efi_arch_cfg_file_early/late() to handle arch specific cfg file fields Roy Franz
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 30+ messages in thread
From: Roy Franz @ 2014-09-24  5:03 UTC (permalink / raw)
  To: xen-devel, ian.campbell, stefano.stabellini, tim, jbeulich, keir
  Cc: Roy Franz, fu.wei

The UEFI ExitBootServices function is invoked to transition the
system to the 'runtime' mode of operation, and is done right before
transitioning from the EFI loader code into Xen proper. x86 does some
arch specific memory management (trampoline) before exit boot services,
and the code that transitions from the EFI application state to Xen
is architecture specific.  This patch adds two functions, one pre
and one post ExitBootServices to allow each architecture to
to handle these cases in a customized manner.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/efi/efi-boot.h | 50 +++++++++++++++++++++++++++++++++++++++++++++
 xen/common/efi/boot.c       | 42 ++-----------------------------------
 2 files changed, 52 insertions(+), 40 deletions(-)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index d355115..07b0509 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -201,3 +201,53 @@ static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size)
         return NULL;
     return (void *)(long)mbi.mem_upper;
 }
+
+static void __init efi_arch_pre_exit_boot(void)
+{
+    if ( !trampoline_phys )
+    {
+        if ( !cfg.addr )
+            blexit(L"No memory for trampoline");
+        relocate_trampoline(cfg.addr);
+    }
+}
+
+static void __init noreturn efi_arch_post_exit_boot(void)
+{
+    u64 efer;
+
+    efi_arch_relocate_image(__XEN_VIRT_START - xen_phys_start);
+    memcpy((void *)trampoline_phys, trampoline_start, cfg.size);
+
+    /* Set system registers and transfer control. */
+    asm volatile("pushq $0\n\tpopfq");
+    rdmsrl(MSR_EFER, efer);
+    efer |= EFER_SCE;
+    if ( cpuid_ext_features & (1 << (X86_FEATURE_NX & 0x1f)) )
+        efer |= EFER_NX;
+    wrmsrl(MSR_EFER, efer);
+    write_cr0(X86_CR0_PE | X86_CR0_MP | X86_CR0_ET | X86_CR0_NE | X86_CR0_WP |
+              X86_CR0_AM | X86_CR0_PG);
+    asm volatile ( "mov    %[cr4], %%cr4\n\t"
+                   "mov    %[cr3], %%cr3\n\t"
+                   "movabs $__start_xen, %[rip]\n\t"
+                   "lgdt   gdt_descr(%%rip)\n\t"
+                   "mov    stack_start(%%rip), %%rsp\n\t"
+                   "mov    %[ds], %%ss\n\t"
+                   "mov    %[ds], %%ds\n\t"
+                   "mov    %[ds], %%es\n\t"
+                   "mov    %[ds], %%fs\n\t"
+                   "mov    %[ds], %%gs\n\t"
+                   "movl   %[cs], 8(%%rsp)\n\t"
+                   "mov    %[rip], (%%rsp)\n\t"
+                   "lretq  %[stkoff]-16"
+                   : [rip] "=&r" (efer/* any dead 64-bit variable */)
+                   : [cr3] "r" (idle_pg_table),
+                     [cr4] "r" (mmu_cr4_features),
+                     [cs] "ir" (__HYPERVISOR_CS),
+                     [ds] "r" (__HYPERVISOR_DS),
+                     [stkoff] "i" (STACK_SIZE - sizeof(struct cpu_info)),
+                     "D" (&mbi)
+                   : "memory" );
+    for( ; ; ); /* not reached */
+}
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 191a463..dea0954 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -664,7 +664,6 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *mode_info;
     EFI_FILE_HANDLE dir_handle;
     union string section = { NULL }, name;
-    u64 efer;
     bool_t base_video = 0;
 
     efi_ih = ImageHandle;
@@ -1273,12 +1272,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     efi_arch_process_memory_map(SystemTable, efi_memmap, efi_memmap_size,
                                 efi_mdesc_size, mdesc_ver);
 
-    if ( !trampoline_phys )
-    {
-        if ( !cfg.addr )
-            blexit(L"No memory for trampoline");
-        relocate_trampoline(cfg.addr);
-    }
+    efi_arch_pre_exit_boot();
 
     status = efi_bs->ExitBootServices(ImageHandle, map_key);
     if ( EFI_ERROR(status) )
@@ -1292,39 +1286,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START;
     efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START;
 
-    efi_arch_relocate_image(__XEN_VIRT_START - xen_phys_start);
-    memcpy((void *)trampoline_phys, trampoline_start, cfg.size);
-
-    /* Set system registers and transfer control. */
-    asm volatile("pushq $0\n\tpopfq");
-    rdmsrl(MSR_EFER, efer);
-    efer |= EFER_SCE;
-    if ( cpuid_ext_features & (1 << (X86_FEATURE_NX & 0x1f)) )
-        efer |= EFER_NX;
-    wrmsrl(MSR_EFER, efer);
-    write_cr0(X86_CR0_PE | X86_CR0_MP | X86_CR0_ET | X86_CR0_NE | X86_CR0_WP |
-              X86_CR0_AM | X86_CR0_PG);
-    asm volatile ( "mov    %[cr4], %%cr4\n\t"
-                   "mov    %[cr3], %%cr3\n\t"
-                   "movabs $__start_xen, %[rip]\n\t"
-                   "lgdt   gdt_descr(%%rip)\n\t"
-                   "mov    stack_start(%%rip), %%rsp\n\t"
-                   "mov    %[ds], %%ss\n\t"
-                   "mov    %[ds], %%ds\n\t"
-                   "mov    %[ds], %%es\n\t"
-                   "mov    %[ds], %%fs\n\t"
-                   "mov    %[ds], %%gs\n\t"
-                   "movl   %[cs], 8(%%rsp)\n\t"
-                   "mov    %[rip], (%%rsp)\n\t"
-                   "lretq  %[stkoff]-16"
-                   : [rip] "=&r" (efer/* any dead 64-bit variable */)
-                   : [cr3] "r" (idle_pg_table),
-                     [cr4] "r" (mmu_cr4_features),
-                     [cs] "ir" (__HYPERVISOR_CS),
-                     [ds] "r" (__HYPERVISOR_DS),
-                     [stkoff] "i" (STACK_SIZE - sizeof(struct cpu_info)),
-                     "D" (&mbi)
-                   : "memory" );
+    efi_arch_post_exit_boot();
     for( ; ; ); /* not reached */
 }
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH for-4.5 V6 05/14] Add efi_arch_cfg_file_early/late() to handle arch specific cfg file fields
  2014-09-24  5:02 [PATCH for-4.5 V6 00/14] arm64 EFI stub Roy Franz
                   ` (3 preceding siblings ...)
  2014-09-24  5:03 ` [PATCH for-4.5 V6 04/14] Add architecture functions for pre/post ExitBootServices Roy Franz
@ 2014-09-24  5:03 ` Roy Franz
  2014-09-24  5:03 ` [PATCH for-4.5 V6 06/14] Add efi_arch_handle_cmdline() for processing commandline Roy Franz
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 30+ messages in thread
From: Roy Franz @ 2014-09-24  5:03 UTC (permalink / raw)
  To: xen-devel, ian.campbell, stefano.stabellini, tim, jbeulich, keir
  Cc: Roy Franz, fu.wei

Different architectures have some different configuration file
fields that need to be handled.  In particular, x86 has ucode
and ARM has device tree files to be loaded.  These arch specific
functions is used to allow each architecture to implement these
features in arch specific code.  Early/late versions are provided,
as ARM needs to process the DTB entry first, and x86 wants to process
the ucode entry last as it is the smallest allocation.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/efi/efi-boot.h | 20 ++++++++++++++++++++
 xen/common/efi/boot.c       | 21 ++++++++++-----------
 2 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 07b0509..049e5f3 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -251,3 +251,23 @@ static void __init noreturn efi_arch_post_exit_boot(void)
                    : "memory" );
     for( ; ; ); /* not reached */
 }
+
+static void __init efi_arch_cfg_file_early(EFI_FILE_HANDLE dir_handle, char *section)
+{
+}
+
+static void __init efi_arch_cfg_file_late(EFI_FILE_HANDLE dir_handle, char *section)
+{
+    union string name;
+
+    name.s = get_value(&cfg, section, "ucode");
+    if ( !name.s )
+        name.s = get_value(&cfg, "global", "ucode");
+    if ( name.s )
+    {
+        microcode_set_module(mbi.mods_count);
+        split_value(name.s);
+        read_file(dir_handle, s2w(&name), &ucode);
+        efi_bs->FreePool(name.w);
+    }
+}
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index dea0954..0b096a6 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -54,6 +54,12 @@ static void  DisplayUint(UINT64 Val, INTN Width);
 static CHAR16 *wstrcpy(CHAR16 *d, const CHAR16 *s);
 static void noreturn blexit(const CHAR16 *str);
 static void PrintErrMesg(const CHAR16 *mesg, EFI_STATUS ErrCode);
+static char *get_value(const struct file *cfg, const char *section,
+                              const char *item);
+static void  split_value(char *s);
+static CHAR16 *s2w(union string *str);
+static bool_t  read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
+                               struct file *file);
 
 static EFI_BOOT_SERVICES *__initdata efi_bs;
 static EFI_HANDLE __initdata efi_ih;
@@ -842,6 +848,9 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     }
     if ( !name.s )
         blexit(L"No Dom0 kernel image specified.");
+
+    efi_arch_cfg_file_early(dir_handle, section.s);
+
     split_value(name.s);
     read_file(dir_handle, s2w(&name), &kernel);
     efi_bs->FreePool(name.w);
@@ -859,17 +868,6 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
         efi_bs->FreePool(name.w);
     }
 
-    name.s = get_value(&cfg, section.s, "ucode");
-    if ( !name.s )
-        name.s = get_value(&cfg, "global", "ucode");
-    if ( name.s )
-    {
-        microcode_set_module(mbi.mods_count);
-        split_value(name.s);
-        read_file(dir_handle, s2w(&name), &ucode);
-        efi_bs->FreePool(name.w);
-    }
-
     name.s = get_value(&cfg, section.s, "xsm");
     if ( name.s )
     {
@@ -908,6 +906,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
                 cols = rows = depth = 0;
         }
     }
+    efi_arch_cfg_file_late(dir_handle, section.s);
 
     efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
     cfg.addr = 0;
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH for-4.5 V6 06/14] Add efi_arch_handle_cmdline() for processing commandline
  2014-09-24  5:02 [PATCH for-4.5 V6 00/14] arm64 EFI stub Roy Franz
                   ` (4 preceding siblings ...)
  2014-09-24  5:03 ` [PATCH for-4.5 V6 05/14] Add efi_arch_cfg_file_early/late() to handle arch specific cfg file fields Roy Franz
@ 2014-09-24  5:03 ` Roy Franz
  2014-09-24  5:03 ` [PATCH for-4.5 V6 07/14] Move x86 specific disk probing code Roy Franz
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 30+ messages in thread
From: Roy Franz @ 2014-09-24  5:03 UTC (permalink / raw)
  To: xen-devel, ian.campbell, stefano.stabellini, tim, jbeulich, keir
  Cc: Roy Franz, fu.wei

Add arch function for processing the Xen commandline and
updating internal structures.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/efi/efi-boot.h | 34 ++++++++++++++++++++++++++++++++++
 xen/common/efi/boot.c       | 37 +++++++++----------------------------
 2 files changed, 43 insertions(+), 28 deletions(-)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 049e5f3..633a498 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -271,3 +271,37 @@ static void __init efi_arch_cfg_file_late(EFI_FILE_HANDLE dir_handle, char *sect
         efi_bs->FreePool(name.w);
     }
 }
+
+static void __init efi_arch_handle_cmdline(CHAR16 *image_name,
+                                           CHAR16 *cmdline_options,
+                                           char *cfgfile_options)
+{
+    union string name;
+
+    if ( cmdline_options )
+    {
+        name.w = cmdline_options;
+        w2s(&name);
+        place_string(&mbi.cmdline, name.s);
+    }
+    if ( cfgfile_options )
+        place_string(&mbi.cmdline, cfgfile_options);
+    /* Insert image name last, as it gets prefixed to the other options. */
+    if ( image_name )
+    {
+        name.w = image_name;
+        w2s(&name);
+    }
+    else
+        name.s = "xen";
+    place_string(&mbi.cmdline, name.s);
+
+    if ( mbi.cmdline )
+        mbi.flags |= MBI_CMDLINE;
+    /*
+     * These must not be initialized statically, since the value must
+     * not get relocated when processing base relocations later.
+     */
+    mbi.boot_loader_name = (long)"EFI";
+    mbi.mods_addr = (long)mb_modules;
+}
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 0b096a6..639280b 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -58,6 +58,7 @@ static char *get_value(const struct file *cfg, const char *section,
                               const char *item);
 static void  split_value(char *s);
 static CHAR16 *s2w(union string *str);
+static char *w2s(const union string *str);
 static bool_t  read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
                                struct file *file);
 
@@ -258,7 +259,8 @@ static void __init PrintErrMesg(const CHAR16 *mesg, EFI_STATUS ErrCode)
 }
 
 static unsigned int __init get_argv(unsigned int argc, CHAR16 **argv,
-                                    CHAR16 *cmdline, UINTN cmdsize)
+                                    CHAR16 *cmdline, UINTN cmdsize,
+                                    CHAR16 **options)
 {
     CHAR16 *ptr = (CHAR16 *)(argv + argc + 1), *prev = NULL;
     bool_t prev_sep = TRUE;
@@ -284,10 +286,8 @@ static unsigned int __init get_argv(unsigned int argc, CHAR16 **argv,
                 ++argc;
             else if ( prev && wstrcmp(prev, L"--") == 0 )
             {
-                union string rest = { .w = cmdline };
-
-                --argv;
-                place_string(&mbi.cmdline, w2s(&rest));
+                if ( options )
+                    *options = cmdline;
                 break;
             }
             else
@@ -662,7 +662,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     EFI_LOADED_IMAGE *loaded_image;
     EFI_STATUS status;
     unsigned int i, argc;
-    CHAR16 **argv, *file_name, *cfg_file_name = NULL;
+    CHAR16 **argv, *file_name, *cfg_file_name = NULL, *options = NULL;
     UINTN cols, rows, depth, size, map_key, info_size, gop_mode = ~0;
     EFI_HANDLE *handles = NULL;
     EFI_SHIM_LOCK_PROTOCOL *shim_lock;
@@ -700,14 +700,14 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     dir_handle = get_parent_handle(loaded_image, &file_name);
 
     argc = get_argv(0, NULL, loaded_image->LoadOptions,
-                    loaded_image->LoadOptionsSize);
+                    loaded_image->LoadOptionsSize, NULL);
     if ( argc > 0 &&
          efi_bs->AllocatePool(EfiLoaderData,
                               (argc + 1) * sizeof(*argv) +
                                   loaded_image->LoadOptionsSize,
                               (void **)&argv) == EFI_SUCCESS )
         get_argv(argc, argv, loaded_image->LoadOptions,
-                 loaded_image->LoadOptionsSize);
+                 loaded_image->LoadOptionsSize, &options);
     else
         argc = 0;
     for ( i = 1; i < argc; ++i )
@@ -877,17 +877,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     }
 
     name.s = get_value(&cfg, section.s, "options");
-    if ( name.s )
-        place_string(&mbi.cmdline, name.s);
-    /* Insert image name last, as it gets prefixed to the other options. */
-    if ( argc )
-    {
-        name.w = *argv;
-        w2s(&name);
-    }
-    else
-        name.s = "xen";
-    place_string(&mbi.cmdline, name.s);
+    efi_arch_handle_cmdline(argc ? *argv : NULL, options, name.s);
 
     cols = rows = depth = 0;
     if ( !base_video )
@@ -954,15 +944,6 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
         }
     }
 
-    if ( mbi.cmdline )
-        mbi.flags |= MBI_CMDLINE;
-    /*
-     * These must not be initialized statically, since the value must
-     * not get relocated when processing base relocations below.
-     */
-    mbi.boot_loader_name = (long)"EFI";
-    mbi.mods_addr = (long)mb_modules;
-
     /* Collect EDD info. */
     BUILD_BUG_ON(offsetof(struct edd_info, edd_device_params) != EDDEXTSIZE);
     BUILD_BUG_ON(sizeof(struct edd_device_params) != EDDPARMSIZE);
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH for-4.5 V6 07/14] Move x86 specific disk probing code
  2014-09-24  5:02 [PATCH for-4.5 V6 00/14] arm64 EFI stub Roy Franz
                   ` (5 preceding siblings ...)
  2014-09-24  5:03 ` [PATCH for-4.5 V6 06/14] Add efi_arch_handle_cmdline() for processing commandline Roy Franz
@ 2014-09-24  5:03 ` Roy Franz
  2014-09-24  5:03 ` [PATCH for-4.5 V6 08/14] Create arch functions for console and video init Roy Franz
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 30+ messages in thread
From: Roy Franz @ 2014-09-24  5:03 UTC (permalink / raw)
  To: xen-devel, ian.campbell, stefano.stabellini, tim, jbeulich, keir
  Cc: Roy Franz, fu.wei

Move x86 specific disk (EDD) probing to arch specific file.  This code is x86
only and relates to legacy BIOS handling of disk drives.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/efi/efi-boot.h | 152 ++++++++++++++++++++++++++++++++++++++++++++
 xen/common/efi/boot.c       | 144 +----------------------------------------
 2 files changed, 153 insertions(+), 143 deletions(-)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 633a498..d7e2faf 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -305,3 +305,155 @@ static void __init efi_arch_handle_cmdline(CHAR16 *image_name,
     mbi.boot_loader_name = (long)"EFI";
     mbi.mods_addr = (long)mb_modules;
 }
+
+static void __init efi_arch_edd(void)
+{
+    static EFI_GUID __initdata bio_guid = BLOCK_IO_PROTOCOL;
+    static EFI_GUID __initdata devp_guid = DEVICE_PATH_PROTOCOL;
+    EFI_HANDLE *handles = NULL;
+    unsigned int i;
+    UINTN size;
+    EFI_STATUS status;
+
+    /* Collect EDD info. */
+    BUILD_BUG_ON(offsetof(struct edd_info, edd_device_params) != EDDEXTSIZE);
+    BUILD_BUG_ON(sizeof(struct edd_device_params) != EDDPARMSIZE);
+    size = 0;
+    status = efi_bs->LocateHandle(ByProtocol, &bio_guid, NULL, &size, NULL);
+    if ( status == EFI_BUFFER_TOO_SMALL )
+        status = efi_bs->AllocatePool(EfiLoaderData, size, (void **)&handles);
+    if ( !EFI_ERROR(status) )
+        status = efi_bs->LocateHandle(ByProtocol, &bio_guid, NULL, &size,
+                                      handles);
+    if ( EFI_ERROR(status) )
+        size = 0;
+    for ( i = 0; i < size / sizeof(*handles); ++i )
+    {
+        EFI_BLOCK_IO *bio;
+        EFI_DEV_PATH_PTR devp;
+        struct edd_info *info = boot_edd_info + boot_edd_info_nr;
+        struct edd_device_params *params = &info->edd_device_params;
+        enum { root, acpi, pci, ctrlr } state = root;
+
+        status = efi_bs->HandleProtocol(handles[i], &bio_guid, (void **)&bio);
+        if ( EFI_ERROR(status) ||
+             bio->Media->RemovableMedia ||
+             bio->Media->LogicalPartition )
+            continue;
+        if ( boot_edd_info_nr < EDD_INFO_MAX )
+        {
+            info->device = 0x80 + boot_edd_info_nr; /* fake */
+            info->version = 0x11;
+            params->length = offsetof(struct edd_device_params, dpte_ptr);
+            params->number_of_sectors = bio->Media->LastBlock + 1;
+            params->bytes_per_sector = bio->Media->BlockSize;
+            params->dpte_ptr = ~0;
+        }
+        ++boot_edd_info_nr;
+        status = efi_bs->HandleProtocol(handles[i], &devp_guid,
+                                        (void **)&devp);
+        if ( EFI_ERROR(status) )
+            continue;
+        for ( ; !IsDevicePathEnd(devp.DevPath);
+              devp.DevPath = NextDevicePathNode(devp.DevPath) )
+        {
+            switch ( DevicePathType(devp.DevPath) )
+            {
+                const u8 *p;
+
+            case ACPI_DEVICE_PATH:
+                if ( state != root || boot_edd_info_nr > EDD_INFO_MAX )
+                    break;
+                switch ( DevicePathSubType(devp.DevPath) )
+                {
+                case ACPI_DP:
+                    if ( devp.Acpi->HID != EISA_PNP_ID(0xA03) &&
+                         devp.Acpi->HID != EISA_PNP_ID(0xA08) )
+                        break;
+                    params->interface_path.pci.bus = devp.Acpi->UID;
+                    state = acpi;
+                    break;
+                case EXPANDED_ACPI_DP:
+                    /* XXX */
+                    break;
+                }
+                break;
+            case HARDWARE_DEVICE_PATH:
+                if ( state != acpi ||
+                     DevicePathSubType(devp.DevPath) != HW_PCI_DP ||
+                     boot_edd_info_nr > EDD_INFO_MAX )
+                    break;
+                state = pci;
+                edd_put_string(params->host_bus_type, "PCI");
+                params->interface_path.pci.slot = devp.Pci->Device;
+                params->interface_path.pci.function = devp.Pci->Function;
+                break;
+            case MESSAGING_DEVICE_PATH:
+                if ( state != pci || boot_edd_info_nr > EDD_INFO_MAX )
+                    break;
+                state = ctrlr;
+                switch ( DevicePathSubType(devp.DevPath) )
+                {
+                case MSG_ATAPI_DP:
+                    edd_put_string(params->interface_type, "ATAPI");
+                    params->interface_path.pci.channel =
+                        devp.Atapi->PrimarySecondary;
+                    params->device_path.atapi.device = devp.Atapi->SlaveMaster;
+                    params->device_path.atapi.lun = devp.Atapi->Lun;
+                    break;
+                case MSG_SCSI_DP:
+                    edd_put_string(params->interface_type, "SCSI");
+                    params->device_path.scsi.id = devp.Scsi->Pun;
+                    params->device_path.scsi.lun = devp.Scsi->Lun;
+                    break;
+                case MSG_FIBRECHANNEL_DP:
+                    edd_put_string(params->interface_type, "FIBRE");
+                    params->device_path.fibre.wwid = devp.FibreChannel->WWN;
+                    params->device_path.fibre.lun = devp.FibreChannel->Lun;
+                    break;
+                case MSG_1394_DP:
+                    edd_put_string(params->interface_type, "1394");
+                    params->device_path.i1394.eui = devp.F1394->Guid;
+                    break;
+                case MSG_USB_DP:
+                case MSG_USB_CLASS_DP:
+                    edd_put_string(params->interface_type, "USB");
+                    break;
+                case MSG_I2O_DP:
+                    edd_put_string(params->interface_type, "I2O");
+                    params->device_path.i2o.identity_tag = devp.I2O->Tid;
+                    break;
+                default:
+                    continue;
+                }
+                info->version = 0x30;
+                params->length = sizeof(struct edd_device_params);
+                params->key = 0xbedd;
+                params->device_path_info_length =
+                    sizeof(struct edd_device_params) -
+                    offsetof(struct edd_device_params, key);
+                for ( p = (const u8 *)&params->key; p < &params->checksum; ++p )
+                    params->checksum -= *p;
+                break;
+            case MEDIA_DEVICE_PATH:
+                if ( DevicePathSubType(devp.DevPath) == MEDIA_HARDDRIVE_DP &&
+                     devp.HardDrive->MBRType == MBR_TYPE_PCAT &&
+                     boot_mbr_signature_nr < EDD_MBR_SIG_MAX )
+                {
+                    struct mbr_signature *sig = boot_mbr_signature +
+                                                boot_mbr_signature_nr;
+
+                    sig->device = 0x80 + boot_edd_info_nr; /* fake */
+                    memcpy(&sig->signature, devp.HardDrive->Signature,
+                           sizeof(sig->signature));
+                    ++boot_mbr_signature_nr;
+                }
+                break;
+            }
+        }
+    }
+    if ( handles )
+        efi_bs->FreePool(handles);
+    if ( boot_edd_info_nr > EDD_INFO_MAX )
+        boot_edd_info_nr = EDD_INFO_MAX;
+}
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 639280b..4566cf0 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -656,8 +656,6 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 {
     static EFI_GUID __initdata loaded_image_guid = LOADED_IMAGE_PROTOCOL;
     static EFI_GUID __initdata gop_guid = EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID;
-    static EFI_GUID __initdata bio_guid = BLOCK_IO_PROTOCOL;
-    static EFI_GUID __initdata devp_guid = DEVICE_PATH_PROTOCOL;
     static EFI_GUID __initdata shim_lock_guid = SHIM_LOCK_PROTOCOL_GUID;
     EFI_LOADED_IMAGE *loaded_image;
     EFI_STATUS status;
@@ -944,147 +942,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
         }
     }
 
-    /* Collect EDD info. */
-    BUILD_BUG_ON(offsetof(struct edd_info, edd_device_params) != EDDEXTSIZE);
-    BUILD_BUG_ON(sizeof(struct edd_device_params) != EDDPARMSIZE);
-    size = 0;
-    status = efi_bs->LocateHandle(ByProtocol, &bio_guid, NULL, &size, NULL);
-    if ( status == EFI_BUFFER_TOO_SMALL )
-        status = efi_bs->AllocatePool(EfiLoaderData, size, (void **)&handles);
-    if ( !EFI_ERROR(status) )
-        status = efi_bs->LocateHandle(ByProtocol, &bio_guid, NULL, &size,
-                                      handles);
-    if ( EFI_ERROR(status) )
-        size = 0;
-    for ( i = 0; i < size / sizeof(*handles); ++i )
-    {
-        EFI_BLOCK_IO *bio;
-        EFI_DEV_PATH_PTR devp;
-        struct edd_info *info = boot_edd_info + boot_edd_info_nr;
-        struct edd_device_params *params = &info->edd_device_params;
-        enum { root, acpi, pci, ctrlr } state = root;
-
-        status = efi_bs->HandleProtocol(handles[i], &bio_guid, (void **)&bio);
-        if ( EFI_ERROR(status) ||
-             bio->Media->RemovableMedia ||
-             bio->Media->LogicalPartition )
-            continue;
-        if ( boot_edd_info_nr < EDD_INFO_MAX )
-        {
-            info->device = 0x80 + boot_edd_info_nr; /* fake */
-            info->version = 0x11;
-            params->length = offsetof(struct edd_device_params, dpte_ptr);
-            params->number_of_sectors = bio->Media->LastBlock + 1;
-            params->bytes_per_sector = bio->Media->BlockSize;
-            params->dpte_ptr = ~0;
-        }
-        ++boot_edd_info_nr;
-        status = efi_bs->HandleProtocol(handles[i], &devp_guid,
-                                        (void **)&devp);
-        if ( EFI_ERROR(status) )
-            continue;
-        for ( ; !IsDevicePathEnd(devp.DevPath);
-              devp.DevPath = NextDevicePathNode(devp.DevPath) )
-        {
-            switch ( DevicePathType(devp.DevPath) )
-            {
-                const u8 *p;
-
-            case ACPI_DEVICE_PATH:
-                if ( state != root || boot_edd_info_nr > EDD_INFO_MAX )
-                    break;
-                switch ( DevicePathSubType(devp.DevPath) )
-                {
-                case ACPI_DP:
-                    if ( devp.Acpi->HID != EISA_PNP_ID(0xA03) &&
-                         devp.Acpi->HID != EISA_PNP_ID(0xA08) )
-                        break;
-                    params->interface_path.pci.bus = devp.Acpi->UID;
-                    state = acpi;
-                    break;
-                case EXPANDED_ACPI_DP:
-                    /* XXX */
-                    break;
-                }
-                break;
-            case HARDWARE_DEVICE_PATH:
-                if ( state != acpi ||
-                     DevicePathSubType(devp.DevPath) != HW_PCI_DP ||
-                     boot_edd_info_nr > EDD_INFO_MAX )
-                    break;
-                state = pci;
-                edd_put_string(params->host_bus_type, "PCI");
-                params->interface_path.pci.slot = devp.Pci->Device;
-                params->interface_path.pci.function = devp.Pci->Function;
-                break;
-            case MESSAGING_DEVICE_PATH:
-                if ( state != pci || boot_edd_info_nr > EDD_INFO_MAX )
-                    break;
-                state = ctrlr;
-                switch ( DevicePathSubType(devp.DevPath) )
-                {
-                case MSG_ATAPI_DP:
-                    edd_put_string(params->interface_type, "ATAPI");
-                    params->interface_path.pci.channel =
-                        devp.Atapi->PrimarySecondary;
-                    params->device_path.atapi.device = devp.Atapi->SlaveMaster;
-                    params->device_path.atapi.lun = devp.Atapi->Lun;
-                    break;
-                case MSG_SCSI_DP:
-                    edd_put_string(params->interface_type, "SCSI");
-                    params->device_path.scsi.id = devp.Scsi->Pun;
-                    params->device_path.scsi.lun = devp.Scsi->Lun;
-                    break;
-                case MSG_FIBRECHANNEL_DP:
-                    edd_put_string(params->interface_type, "FIBRE");
-                    params->device_path.fibre.wwid = devp.FibreChannel->WWN;
-                    params->device_path.fibre.lun = devp.FibreChannel->Lun;
-                    break;
-                case MSG_1394_DP:
-                    edd_put_string(params->interface_type, "1394");
-                    params->device_path.i1394.eui = devp.F1394->Guid;
-                    break;
-                case MSG_USB_DP:
-                case MSG_USB_CLASS_DP:
-                    edd_put_string(params->interface_type, "USB");
-                    break;
-                case MSG_I2O_DP:
-                    edd_put_string(params->interface_type, "I2O");
-                    params->device_path.i2o.identity_tag = devp.I2O->Tid;
-                    break;
-                default:
-                    continue;
-                }
-                info->version = 0x30;
-                params->length = sizeof(struct edd_device_params);
-                params->key = 0xbedd;
-                params->device_path_info_length =
-                    sizeof(struct edd_device_params) -
-                    offsetof(struct edd_device_params, key);
-                for ( p = (const u8 *)&params->key; p < &params->checksum; ++p )
-                    params->checksum -= *p;
-                break;
-            case MEDIA_DEVICE_PATH:
-                if ( DevicePathSubType(devp.DevPath) == MEDIA_HARDDRIVE_DP &&
-                     devp.HardDrive->MBRType == MBR_TYPE_PCAT &&
-                     boot_mbr_signature_nr < EDD_MBR_SIG_MAX )
-                {
-                    struct mbr_signature *sig = boot_mbr_signature +
-                                                boot_mbr_signature_nr;
-
-                    sig->device = 0x80 + boot_edd_info_nr; /* fake */
-                    memcpy(&sig->signature, devp.HardDrive->Signature,
-                           sizeof(sig->signature));
-                    ++boot_mbr_signature_nr;
-                }
-                break;
-            }
-        }
-    }
-    if ( handles )
-        efi_bs->FreePool(handles);
-    if ( boot_edd_info_nr > EDD_INFO_MAX )
-        boot_edd_info_nr = EDD_INFO_MAX;
+    efi_arch_edd();
 
     /* XXX Collect EDID info. */
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH for-4.5 V6 08/14] Create arch functions for console and video init
  2014-09-24  5:02 [PATCH for-4.5 V6 00/14] arm64 EFI stub Roy Franz
                   ` (6 preceding siblings ...)
  2014-09-24  5:03 ` [PATCH for-4.5 V6 07/14] Move x86 specific disk probing code Roy Franz
@ 2014-09-24  5:03 ` Roy Franz
  2014-09-24  5:03 ` [PATCH for-4.5 V6 09/14] Add efi_arch_memory() for arch specific memory setup Roy Franz
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 30+ messages in thread
From: Roy Franz @ 2014-09-24  5:03 UTC (permalink / raw)
  To: xen-devel, ian.campbell, stefano.stabellini, tim, jbeulich, keir
  Cc: Roy Franz, fu.wei

Add arch functions for text console and graphics initialization, and move VGA
specific code to x86 architecture file.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/efi/efi-boot.h | 75 +++++++++++++++++++++++++++++++++++++++++++++
 xen/common/efi/boot.c       | 69 ++---------------------------------------
 2 files changed, 78 insertions(+), 66 deletions(-)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index d7e2faf..7f8a603 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -457,3 +457,78 @@ static void __init efi_arch_edd(void)
     if ( boot_edd_info_nr > EDD_INFO_MAX )
         boot_edd_info_nr = EDD_INFO_MAX;
 }
+
+static void __init efi_arch_console_init(UINTN cols, UINTN rows)
+{
+    vga_console_info.video_type = XEN_VGATYPE_TEXT_MODE_3;
+    vga_console_info.u.text_mode_3.columns = cols;
+    vga_console_info.u.text_mode_3.rows = rows;
+    vga_console_info.u.text_mode_3.font_height = 16;
+}
+
+static void __init efi_arch_video_init(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop,
+                                       UINTN info_size,
+                                       EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *mode_info)
+{
+    int bpp = 0;
+
+    switch ( mode_info->PixelFormat )
+    {
+    case PixelRedGreenBlueReserved8BitPerColor:
+        vga_console_info.u.vesa_lfb.red_pos = 0;
+        vga_console_info.u.vesa_lfb.red_size = 8;
+        vga_console_info.u.vesa_lfb.green_pos = 8;
+        vga_console_info.u.vesa_lfb.green_size = 8;
+        vga_console_info.u.vesa_lfb.blue_pos = 16;
+        vga_console_info.u.vesa_lfb.blue_size = 8;
+        vga_console_info.u.vesa_lfb.rsvd_pos = 24;
+        vga_console_info.u.vesa_lfb.rsvd_size = 8;
+        bpp = 32;
+        break;
+    case PixelBlueGreenRedReserved8BitPerColor:
+        vga_console_info.u.vesa_lfb.red_pos = 16;
+        vga_console_info.u.vesa_lfb.red_size = 8;
+        vga_console_info.u.vesa_lfb.green_pos = 8;
+        vga_console_info.u.vesa_lfb.green_size = 8;
+        vga_console_info.u.vesa_lfb.blue_pos = 0;
+        vga_console_info.u.vesa_lfb.blue_size = 8;
+        vga_console_info.u.vesa_lfb.rsvd_pos = 24;
+        vga_console_info.u.vesa_lfb.rsvd_size = 8;
+        bpp = 32;
+        break;
+    case PixelBitMask:
+        bpp = set_color(mode_info->PixelInformation.RedMask, bpp,
+                        &vga_console_info.u.vesa_lfb.red_pos,
+                        &vga_console_info.u.vesa_lfb.red_size);
+        bpp = set_color(mode_info->PixelInformation.GreenMask, bpp,
+                        &vga_console_info.u.vesa_lfb.green_pos,
+                        &vga_console_info.u.vesa_lfb.green_size);
+        bpp = set_color(mode_info->PixelInformation.BlueMask, bpp,
+                        &vga_console_info.u.vesa_lfb.blue_pos,
+                        &vga_console_info.u.vesa_lfb.blue_size);
+        bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp,
+                        &vga_console_info.u.vesa_lfb.rsvd_pos,
+                        &vga_console_info.u.vesa_lfb.rsvd_size);
+        if ( bpp > 0 )
+            break;
+        /* fall through */
+    default:
+        PrintErr(L"Current graphics mode is unsupported!\r\n");
+        bpp  = 0;
+        break;
+    }
+    if ( bpp > 0 )
+    {
+        vga_console_info.video_type = XEN_VGATYPE_EFI_LFB;
+        vga_console_info.u.vesa_lfb.gbl_caps = 2; /* possibly non-VGA */
+        vga_console_info.u.vesa_lfb.width =
+            mode_info->HorizontalResolution;
+        vga_console_info.u.vesa_lfb.height = mode_info->VerticalResolution;
+        vga_console_info.u.vesa_lfb.bits_per_pixel = bpp;
+        vga_console_info.u.vesa_lfb.bytes_per_line =
+            (mode_info->PixelsPerScanLine * bpp + 7) >> 3;
+        vga_console_info.u.vesa_lfb.lfb_base = gop->Mode->FrameBufferBase;
+        vga_console_info.u.vesa_lfb.lfb_size =
+            (gop->Mode->FrameBufferSize + 0xffff) >> 16;
+    }
+}
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 4566cf0..3893311 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -61,6 +61,7 @@ static CHAR16 *s2w(union string *str);
 static char *w2s(const union string *str);
 static bool_t  read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
                                struct file *file);
+static int set_color(u32 mask, int bpp, u8 *pos, u8 *sz);
 
 static EFI_BOOT_SERVICES *__initdata efi_bs;
 static EFI_HANDLE __initdata efi_ih;
@@ -767,12 +768,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 
     if ( StdOut->QueryMode(StdOut, StdOut->Mode->Mode,
                            &cols, &rows) == EFI_SUCCESS )
-    {
-        vga_console_info.video_type = XEN_VGATYPE_TEXT_MODE_3;
-        vga_console_info.u.text_mode_3.columns = cols;
-        vga_console_info.u.text_mode_3.rows = rows;
-        vga_console_info.u.text_mode_3.font_height = 16;
-    }
+        efi_arch_console_init(cols, rows);
 
     size = 0;
     status = efi_bs->LocateHandle(ByProtocol, &gop_guid, NULL, &size, NULL);
@@ -1026,7 +1022,6 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 
     if ( gop )
     {
-        int bpp = 0;
 
         /* Set graphics mode. */
         if ( gop_mode < gop->Mode->MaxMode && gop_mode != gop->Mode->Mode )
@@ -1035,65 +1030,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
         /* Get graphics and frame buffer info. */
         status = gop->QueryMode(gop, gop->Mode->Mode, &info_size, &mode_info);
         if ( !EFI_ERROR(status) )
-            switch ( mode_info->PixelFormat )
-            {
-            case PixelRedGreenBlueReserved8BitPerColor:
-                vga_console_info.u.vesa_lfb.red_pos = 0;
-                vga_console_info.u.vesa_lfb.red_size = 8;
-                vga_console_info.u.vesa_lfb.green_pos = 8;
-                vga_console_info.u.vesa_lfb.green_size = 8;
-                vga_console_info.u.vesa_lfb.blue_pos = 16;
-                vga_console_info.u.vesa_lfb.blue_size = 8;
-                vga_console_info.u.vesa_lfb.rsvd_pos = 24;
-                vga_console_info.u.vesa_lfb.rsvd_size = 8;
-                bpp = 32;
-                break;
-            case PixelBlueGreenRedReserved8BitPerColor:
-                vga_console_info.u.vesa_lfb.red_pos = 16;
-                vga_console_info.u.vesa_lfb.red_size = 8;
-                vga_console_info.u.vesa_lfb.green_pos = 8;
-                vga_console_info.u.vesa_lfb.green_size = 8;
-                vga_console_info.u.vesa_lfb.blue_pos = 0;
-                vga_console_info.u.vesa_lfb.blue_size = 8;
-                vga_console_info.u.vesa_lfb.rsvd_pos = 24;
-                vga_console_info.u.vesa_lfb.rsvd_size = 8;
-                bpp = 32;
-                break;
-            case PixelBitMask:
-                bpp = set_color(mode_info->PixelInformation.RedMask, bpp,
-                                &vga_console_info.u.vesa_lfb.red_pos,
-                                &vga_console_info.u.vesa_lfb.red_size);
-                bpp = set_color(mode_info->PixelInformation.GreenMask, bpp,
-                                &vga_console_info.u.vesa_lfb.green_pos,
-                                &vga_console_info.u.vesa_lfb.green_size);
-                bpp = set_color(mode_info->PixelInformation.BlueMask, bpp,
-                                &vga_console_info.u.vesa_lfb.blue_pos,
-                                &vga_console_info.u.vesa_lfb.blue_size);
-                bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp,
-                                &vga_console_info.u.vesa_lfb.rsvd_pos,
-                                &vga_console_info.u.vesa_lfb.rsvd_size);
-                if ( bpp > 0 )
-                    break;
-                /* fall through */
-            default:
-                PrintErr(L"Current graphics mode is unsupported!\r\n");
-                status = EFI_UNSUPPORTED;
-                break;
-            }
-        if ( !EFI_ERROR(status) )
-        {
-            vga_console_info.video_type = XEN_VGATYPE_EFI_LFB;
-            vga_console_info.u.vesa_lfb.gbl_caps = 2; /* possibly non-VGA */
-            vga_console_info.u.vesa_lfb.width =
-                mode_info->HorizontalResolution;
-            vga_console_info.u.vesa_lfb.height = mode_info->VerticalResolution;
-            vga_console_info.u.vesa_lfb.bits_per_pixel = bpp;
-            vga_console_info.u.vesa_lfb.bytes_per_line =
-                (mode_info->PixelsPerScanLine * bpp + 7) >> 3;
-            vga_console_info.u.vesa_lfb.lfb_base = gop->Mode->FrameBufferBase;
-            vga_console_info.u.vesa_lfb.lfb_size =
-                (gop->Mode->FrameBufferSize + 0xffff) >> 16;
-        }
+            efi_arch_video_init(gop, info_size, mode_info);
     }
 
     efi_bs->GetMemoryMap(&efi_memmap_size, NULL, &map_key,
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH for-4.5 V6 09/14] Add efi_arch_memory() for arch specific memory setup
  2014-09-24  5:02 [PATCH for-4.5 V6 00/14] arm64 EFI stub Roy Franz
                   ` (7 preceding siblings ...)
  2014-09-24  5:03 ` [PATCH for-4.5 V6 08/14] Create arch functions for console and video init Roy Franz
@ 2014-09-24  5:03 ` Roy Franz
  2014-09-24  5:03 ` [PATCH for-4.5 V6 10/14] Add arch specific module handling to read_file() Roy Franz
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 30+ messages in thread
From: Roy Franz @ 2014-09-24  5:03 UTC (permalink / raw)
  To: xen-devel, ian.campbell, stefano.stabellini, tim, jbeulich, keir
  Cc: Roy Franz, fu.wei

This patch adds efi_arch_memory() to allow each architecture a hook
to use for do memory setup.  x86 uses this for trampoline memory setup
and some pagetable setup.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/efi/efi-boot.h | 35 +++++++++++++++++++++++++++++++++++
 xen/common/efi/boot.c       | 29 +----------------------------
 2 files changed, 36 insertions(+), 28 deletions(-)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 7f8a603..32fc950 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -532,3 +532,38 @@ static void __init efi_arch_video_init(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop,
             (gop->Mode->FrameBufferSize + 0xffff) >> 16;
     }
 }
+
+static void __init efi_arch_memory_setup(void)
+{
+    unsigned int i;
+    EFI_STATUS status;
+
+    /* Allocate space for trampoline (in first Mb). */
+    cfg.addr = 0x100000;
+    cfg.size = trampoline_end - trampoline_start;
+    status = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
+                                   PFN_UP(cfg.size), &cfg.addr);
+    if ( status == EFI_SUCCESS )
+        relocate_trampoline(cfg.addr);
+    else
+    {
+        cfg.addr = 0;
+        PrintStr(L"Trampoline space cannot be allocated; will try fallback.\r\n");
+    }
+
+    /* Initialise L2 identity-map and boot-map page table entries (16MB). */
+    for ( i = 0; i < 8; ++i )
+    {
+        unsigned int slot = (xen_phys_start >> L2_PAGETABLE_SHIFT) + i;
+        paddr_t addr = slot << L2_PAGETABLE_SHIFT;
+
+        l2_identmap[slot] = l2e_from_paddr(addr, PAGE_HYPERVISOR|_PAGE_PSE);
+        slot &= L2_PAGETABLE_ENTRIES - 1;
+        l2_bootmap[slot] = l2e_from_paddr(addr, __PAGE_HYPERVISOR|_PAGE_PSE);
+    }
+    /* Initialise L3 boot-map page directory entries. */
+    l3_bootmap[l3_table_offset(xen_phys_start)] =
+        l3e_from_paddr((UINTN)l2_bootmap, __PAGE_HYPERVISOR);
+    l3_bootmap[l3_table_offset(xen_phys_start + (8 << L2_PAGETABLE_SHIFT) - 1)] =
+        l3e_from_paddr((UINTN)l2_bootmap, __PAGE_HYPERVISOR);
+}
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 3893311..596c16f 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -991,34 +991,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
         PrintStr(newline);
     }
 
-    /* Allocate space for trampoline (in first Mb). */
-    cfg.addr = 0x100000;
-    cfg.size = trampoline_end - trampoline_start;
-    status = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
-                                   PFN_UP(cfg.size), &cfg.addr);
-    if ( status == EFI_SUCCESS )
-        relocate_trampoline(cfg.addr);
-    else
-    {
-        cfg.addr = 0;
-        PrintStr(L"Trampoline space cannot be allocated; will try fallback.\r\n");
-    }
-
-    /* Initialise L2 identity-map and boot-map page table entries (16MB). */
-    for ( i = 0; i < 8; ++i )
-    {
-        unsigned int slot = (xen_phys_start >> L2_PAGETABLE_SHIFT) + i;
-        paddr_t addr = slot << L2_PAGETABLE_SHIFT;
-
-        l2_identmap[slot] = l2e_from_paddr(addr, PAGE_HYPERVISOR|_PAGE_PSE);
-        slot &= L2_PAGETABLE_ENTRIES - 1;
-        l2_bootmap[slot] = l2e_from_paddr(addr, __PAGE_HYPERVISOR|_PAGE_PSE);
-    }
-    /* Initialise L3 boot-map page directory entries. */
-    l3_bootmap[l3_table_offset(xen_phys_start)] =
-        l3e_from_paddr((UINTN)l2_bootmap, __PAGE_HYPERVISOR);
-    l3_bootmap[l3_table_offset(xen_phys_start + (8 << L2_PAGETABLE_SHIFT) - 1)] =
-        l3e_from_paddr((UINTN)l2_bootmap, __PAGE_HYPERVISOR);
+    efi_arch_memory_setup();
 
     if ( gop )
     {
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH for-4.5 V6 10/14] Add arch specific module handling to read_file()
  2014-09-24  5:02 [PATCH for-4.5 V6 00/14] arm64 EFI stub Roy Franz
                   ` (8 preceding siblings ...)
  2014-09-24  5:03 ` [PATCH for-4.5 V6 09/14] Add efi_arch_memory() for arch specific memory setup Roy Franz
@ 2014-09-24  5:03 ` Roy Franz
  2014-09-24 16:02   ` Jan Beulich
  2014-09-24  5:03 ` [PATCH for-4.5 V6 11/14] Add several misc. arch functions for EFI boot code Roy Franz
                   ` (4 subsequent siblings)
  14 siblings, 1 reply; 30+ messages in thread
From: Roy Franz @ 2014-09-24  5:03 UTC (permalink / raw)
  To: xen-devel, ian.campbell, stefano.stabellini, tim, jbeulich, keir
  Cc: Roy Franz, fu.wei

Each architecture tracks modules differently internally,
so add efi_arch_handle_module() routine to enable the common
code to invoke the proper handling of modules as the are loaded.
ucode module handling is changed to not process remainder of string
after filename as options, since ucode module does not take options.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
 xen/arch/x86/efi/efi-boot.h | 31 ++++++++++++++++--
 xen/common/efi/boot.c       | 77 ++++++++++++++++++++++++++++-----------------
 2 files changed, 78 insertions(+), 30 deletions(-)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 32fc950..ee43cc4 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -266,8 +266,8 @@ static void __init efi_arch_cfg_file_late(EFI_FILE_HANDLE dir_handle, char *sect
     if ( name.s )
     {
         microcode_set_module(mbi.mods_count);
-        split_value(name.s);
-        read_file(dir_handle, s2w(&name), &ucode);
+        split_string(name.s);
+        read_file(dir_handle, s2w(&name), &ucode, NULL);
         efi_bs->FreePool(name.w);
     }
 }
@@ -567,3 +567,30 @@ static void __init efi_arch_memory_setup(void)
     l3_bootmap[l3_table_offset(xen_phys_start + (8 << L2_PAGETABLE_SHIFT) - 1)] =
         l3e_from_paddr((UINTN)l2_bootmap, __PAGE_HYPERVISOR);
 }
+
+static void __init efi_arch_handle_module(struct file *file, const CHAR16 *name,
+                                          char *options)
+{
+    union string local_name;
+    void *ptr;
+
+    /*
+     * Make a copy, as conversion is destructive, and caller still wants
+     * wide string available after this call returns.
+     */
+    if ( efi_bs->AllocatePool(EfiLoaderData, (wstrlen(name) + 1) * sizeof(*name),
+                              &ptr) != EFI_SUCCESS )
+        blexit(L"ERROR Unable to allocate string buffer");
+
+    local_name.w = ptr;
+    wstrcpy(local_name.w, name);
+    w2s(&local_name);
+
+    place_string(&mb_modules[mbi.mods_count].string, options);
+    place_string(&mb_modules[mbi.mods_count].string, "");
+    place_string(&mb_modules[mbi.mods_count].string, local_name.s);
+    mb_modules[mbi.mods_count].mod_start = file->addr >> PAGE_SHIFT;
+    mb_modules[mbi.mods_count].mod_end = file->size;
+    ++mbi.mods_count;
+    efi_bs->FreePool(ptr);
+}
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 596c16f..4e579c6 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -56,11 +56,12 @@ static void noreturn blexit(const CHAR16 *str);
 static void PrintErrMesg(const CHAR16 *mesg, EFI_STATUS ErrCode);
 static char *get_value(const struct file *cfg, const char *section,
                               const char *item);
-static void  split_value(char *s);
+static char *split_string(char *s);
 static CHAR16 *s2w(union string *str);
 static char *w2s(const union string *str);
-static bool_t  read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
-                               struct file *file);
+static bool_t read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
+                        struct file *file, char *options);
+static size_t wstrlen(const CHAR16 * s);
 static int set_color(u32 mask, int bpp, u8 *pos, u8 *sz);
 
 static EFI_BOOT_SERVICES *__initdata efi_bs;
@@ -117,6 +118,15 @@ static void __init DisplayUint(UINT64 Val, INTN Width)
     PrintStr(PrintString);
 }
 
+static size_t __init wstrlen(const CHAR16 *s)
+{
+    const CHAR16 *sc;
+
+    for ( sc = s; *sc != L'\0'; ++sc )
+        /* nothing */;
+    return sc - s;
+}
+
 static CHAR16 *__init wstrcpy(CHAR16 *d, const CHAR16 *s)
 {
     CHAR16 *r = d;
@@ -406,9 +416,25 @@ static CHAR16 *__init point_tail(CHAR16 *fn)
             break;
         }
 }
+/*
+ * Truncate string at first space, and return pointer
+ * to remainder of string, if any/ NULL returned if
+ * no remainder after space.
+ */
+static char * __init split_string(char *s)
+{
+    while ( *s && !isspace(*s) )
+        ++s;
+    if ( *s )
+    {
+        *s = 0;
+        return s + 1;
+    }
+    return NULL;
+}
 
 static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
-                               struct file *file)
+                               struct file *file, char *options)
 {
     EFI_FILE_HANDLE FileHandle = NULL;
     UINT64 size;
@@ -449,6 +475,7 @@ static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
     }
     else
     {
+        file->size = size;
         if ( file != &cfg )
         {
             PrintStr(name);
@@ -457,12 +484,9 @@ static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
             PrintStr(L"-");
             DisplayUint(file->addr + size, 2 * sizeof(file->addr));
             PrintStr(newline);
-            mb_modules[mbi.mods_count].mod_start = file->addr >> PAGE_SHIFT;
-            mb_modules[mbi.mods_count].mod_end = size;
-            ++mbi.mods_count;
+            efi_arch_handle_module(file, name, options);
         }
 
-        file->size = size;
         ret = FileHandle->Read(FileHandle, &file->size, file->ptr);
         if ( !EFI_ERROR(ret) && file->size != size )
             ret = EFI_ABORTED;
@@ -533,7 +557,13 @@ static char *__init get_value(const struct file *cfg, const char *section,
             break;
         default:
             if ( match && strncmp(ptr, item, ilen) == 0 && ptr[ilen] == '=' )
-                return ptr + ilen + 1;
+            {
+                ptr += ilen + 1;
+                /* strip off any leading spaces */
+                while ( *ptr && isspace(*ptr) )
+                    ptr++;
+                return ptr;
+            }
             break;
         }
         ptr += strlen(ptr);
@@ -541,16 +571,6 @@ static char *__init get_value(const struct file *cfg, const char *section,
     return NULL;
 }
 
-static void __init split_value(char *s)
-{
-    while ( *s && isspace(*s) )
-        ++s;
-    place_string(&mb_modules[mbi.mods_count].string, s);
-    while ( *s && !isspace(*s) )
-        ++s;
-    *s = 0;
-}
-
 static void __init setup_efi_pci(void)
 {
     EFI_STATUS status;
@@ -670,6 +690,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     EFI_FILE_HANDLE dir_handle;
     union string section = { NULL }, name;
     bool_t base_video = 0;
+    char *option_str;
 
     efi_ih = ImageHandle;
     efi_bs = SystemTable->BootServices;
@@ -801,7 +822,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
         while ( (tail = point_tail(file_name)) != NULL )
         {
             wstrcpy(tail, L".cfg");
-            if ( read_file(dir_handle, file_name, &cfg) )
+            if ( read_file(dir_handle, file_name, &cfg, NULL) )
                 break;
             *tail = 0;
         }
@@ -811,7 +832,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
         PrintStr(file_name);
         PrintStr(L"'\r\n");
     }
-    else if ( !read_file(dir_handle, cfg_file_name, &cfg) )
+    else if ( !read_file(dir_handle, cfg_file_name, &cfg, NULL) )
         blexit(L"Configuration file not found.");
     pre_parse(&cfg);
 
@@ -830,7 +851,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
             break;
         efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
         cfg.addr = 0;
-        if ( !read_file(dir_handle, s2w(&name), &cfg) )
+        if ( !read_file(dir_handle, s2w(&name), &cfg, NULL) )
         {
             PrintStr(L"Chained configuration file '");
             PrintStr(name.w);
@@ -845,8 +866,8 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 
     efi_arch_cfg_file_early(dir_handle, section.s);
 
-    split_value(name.s);
-    read_file(dir_handle, s2w(&name), &kernel);
+    option_str = split_string(name.s);
+    read_file(dir_handle, s2w(&name), &kernel, option_str);
     efi_bs->FreePool(name.w);
 
     if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
@@ -857,16 +878,16 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     name.s = get_value(&cfg, section.s, "ramdisk");
     if ( name.s )
     {
-        split_value(name.s);
-        read_file(dir_handle, s2w(&name), &ramdisk);
+        option_str = split_string(name.s);
+        read_file(dir_handle, s2w(&name), &ramdisk, option_str);
         efi_bs->FreePool(name.w);
     }
 
     name.s = get_value(&cfg, section.s, "xsm");
     if ( name.s )
     {
-        split_value(name.s);
-        read_file(dir_handle, s2w(&name), &xsm);
+        option_str = split_string(name.s);
+        read_file(dir_handle, s2w(&name), &xsm, option_str);
         efi_bs->FreePool(name.w);
     }
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH for-4.5 V6 11/14] Add several misc. arch functions for EFI boot code.
  2014-09-24  5:02 [PATCH for-4.5 V6 00/14] arm64 EFI stub Roy Franz
                   ` (9 preceding siblings ...)
  2014-09-24  5:03 ` [PATCH for-4.5 V6 10/14] Add arch specific module handling to read_file() Roy Franz
@ 2014-09-24  5:03 ` Roy Franz
  2014-09-24  5:03 ` [PATCH for-4.5 V6 12/14] Add efi_arch_use_config_file() function to control use of config file Roy Franz
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 30+ messages in thread
From: Roy Franz @ 2014-09-24  5:03 UTC (permalink / raw)
  To: xen-devel, ian.campbell, stefano.stabellini, tim, jbeulich, keir
  Cc: Roy Franz, fu.wei

Add efi_arch_blexit() for arch specific cleanup on error exit,
efi_arch_load_addr_check() to do the arch specific verifications
of where the UEFI firmware loaded Xen, and efi_arch_cpu() for
probing CPU features.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/efi/efi-boot.h | 25 +++++++++++++++++++++++++
 xen/common/efi/boot.c       | 18 ++++--------------
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index ee43cc4..e2c9e87 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -594,3 +594,28 @@ static void __init efi_arch_handle_module(struct file *file, const CHAR16 *name,
     ++mbi.mods_count;
     efi_bs->FreePool(ptr);
 }
+
+static void __init efi_arch_cpu(void)
+{
+    if ( cpuid_eax(0x80000000) > 0x80000000 )
+    {
+        cpuid_ext_features = cpuid_edx(0x80000001);
+        boot_cpu_data.x86_capability[1] = cpuid_ext_features;
+    }
+}
+
+static void __init efi_arch_blexit(void)
+{
+    if ( ucode.addr )
+        efi_bs->FreePages(ucode.addr, PFN_UP(ucode.size));
+}
+
+static void __init efi_arch_load_addr_check(EFI_LOADED_IMAGE *loaded_image)
+{
+    xen_phys_start = (UINTN)loaded_image->ImageBase;
+    if ( (xen_phys_start + loaded_image->ImageSize - 1) >> 32 )
+        blexit(L"Xen must be loaded below 4Gb.");
+    if ( xen_phys_start & ((1 << L2_PAGETABLE_SHIFT) - 1) )
+        blexit(L"Xen must be loaded at a 2Mb boundary.");
+    trampoline_xen_phys_start = xen_phys_start;
+}
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 4e579c6..4352b48 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -209,11 +209,11 @@ static void __init noreturn blexit(const CHAR16 *str)
         efi_bs->FreePages(kernel.addr, PFN_UP(kernel.size));
     if ( ramdisk.addr )
         efi_bs->FreePages(ramdisk.addr, PFN_UP(ramdisk.size));
-    if ( ucode.addr )
-        efi_bs->FreePages(ucode.addr, PFN_UP(ucode.size));
     if ( xsm.addr )
         efi_bs->FreePages(xsm.addr, PFN_UP(xsm.size));
 
+    efi_arch_blexit();
+
     efi_bs->Exit(efi_ih, EFI_SUCCESS, 0, NULL);
     unreachable(); /* not reached */
 }
@@ -709,12 +709,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     if ( status != EFI_SUCCESS )
         PrintErrMesg(L"No Loaded Image Protocol", status);
 
-    xen_phys_start = (UINTN)loaded_image->ImageBase;
-    if ( (xen_phys_start + loaded_image->ImageSize - 1) >> 32 )
-        blexit(L"Xen must be loaded below 4Gb.");
-    if ( xen_phys_start & ((1 << L2_PAGETABLE_SHIFT) - 1) )
-        blexit(L"Xen must be loaded at a 2Mb boundary.");
-    trampoline_xen_phys_start = xen_phys_start;
+    efi_arch_load_addr_check(loaded_image);
 
     /* Get the file system interface. */
     dir_handle = get_parent_handle(loaded_image, &file_name);
@@ -962,12 +957,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     efi_arch_edd();
 
     /* XXX Collect EDID info. */
-
-    if ( cpuid_eax(0x80000000) > 0x80000000 )
-    {
-        cpuid_ext_features = cpuid_edx(0x80000001);
-        boot_cpu_data.x86_capability[1] = cpuid_ext_features;
-    }
+    efi_arch_cpu();
 
     /* Obtain basic table pointers. */
     for ( i = 0; i < efi_num_ct; ++i )
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH for-4.5 V6 12/14] Add efi_arch_use_config_file() function to control use of config file
  2014-09-24  5:02 [PATCH for-4.5 V6 00/14] arm64 EFI stub Roy Franz
                   ` (10 preceding siblings ...)
  2014-09-24  5:03 ` [PATCH for-4.5 V6 11/14] Add several misc. arch functions for EFI boot code Roy Franz
@ 2014-09-24  5:03 ` Roy Franz
  2014-09-24  5:03 ` [PATCH for-4.5 V6 13/14] Fix freeing of uninitialized pointer Roy Franz
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 30+ messages in thread
From: Roy Franz @ 2014-09-24  5:03 UTC (permalink / raw)
  To: xen-devel, ian.campbell, stefano.stabellini, tim, jbeulich, keir
  Cc: Roy Franz, fu.wei

The x86 EFI build of Xen always uses a configuration file to load modules, but
the ARM version can either use a config file to specify the modules, or be
loaded by GRUB in which case GRUB loads the modules and adds them to the DTB
that is passed to Xen.  Add the efi_arch_use_config_file() to indicate if a
configuration file is required.  For x86, this will always be true.  ARM will
examine the DTB passed via EFI configuration table (if any), and if it contains
module information will use that that not use the configuration file at all.
Add Emacs footer to efi-boot.h and boot.c

Signed-off-by: Roy Franz <roy.franz@linaro.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/efi/efi-boot.h |  14 ++++
 xen/common/efi/boot.c       | 179 +++++++++++++++++++++++---------------------
 2 files changed, 107 insertions(+), 86 deletions(-)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index e2c9e87..72037a5 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -619,3 +619,17 @@ static void __init efi_arch_load_addr_check(EFI_LOADED_IMAGE *loaded_image)
         blexit(L"Xen must be loaded at a 2Mb boundary.");
     trampoline_xen_phys_start = xen_phys_start;
 }
+
+static bool_t __init efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)
+{
+    return 1; /* x86 always uses a config file */
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 4352b48..d5c9355 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -687,7 +687,6 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     EFI_SHIM_LOCK_PROTOCOL *shim_lock;
     EFI_GRAPHICS_OUTPUT_PROTOCOL *gop = NULL;
     EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *mode_info;
-    EFI_FILE_HANDLE dir_handle;
     union string section = { NULL }, name;
     bool_t base_video = 0;
     char *option_str;
@@ -711,9 +710,6 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 
     efi_arch_load_addr_check(loaded_image);
 
-    /* Get the file system interface. */
-    dir_handle = get_parent_handle(loaded_image, &file_name);
-
     argc = get_argv(0, NULL, loaded_image->LoadOptions,
                     loaded_image->LoadOptionsSize, NULL);
     if ( argc > 0 &&
@@ -809,109 +805,120 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     if ( EFI_ERROR(status) )
         gop = NULL;
 
-    /* Read and parse the config file. */
-    if ( !cfg_file_name )
+    cols = rows = depth = 0;
+    if ( efi_arch_use_config_file(SystemTable) )
     {
-        CHAR16 *tail;
+        EFI_FILE_HANDLE dir_handle;
 
-        while ( (tail = point_tail(file_name)) != NULL )
+        /* Get the file system interface. */
+        dir_handle = get_parent_handle(loaded_image, &file_name);
+
+        /* Read and parse the config file. */
+        if ( !cfg_file_name )
         {
-            wstrcpy(tail, L".cfg");
-            if ( read_file(dir_handle, file_name, &cfg, NULL) )
-                break;
-            *tail = 0;
+            CHAR16 *tail;
+
+            while ( (tail = point_tail(file_name)) != NULL )
+            {
+                wstrcpy(tail, L".cfg");
+                if ( read_file(dir_handle, file_name, &cfg, NULL) )
+                    break;
+                *tail = 0;
+            }
+            if ( !tail )
+                blexit(L"No configuration file found.");
+            PrintStr(L"Using configuration file '");
+            PrintStr(file_name);
+            PrintStr(L"'\r\n");
         }
-        if ( !tail )
-            blexit(L"No configuration file found.");
-        PrintStr(L"Using configuration file '");
-        PrintStr(file_name);
-        PrintStr(L"'\r\n");
-    }
-    else if ( !read_file(dir_handle, cfg_file_name, &cfg, NULL) )
-        blexit(L"Configuration file not found.");
-    pre_parse(&cfg);
+        else if ( !read_file(dir_handle, cfg_file_name, &cfg, NULL) )
+            blexit(L"Configuration file not found.");
+        pre_parse(&cfg);
 
-    if ( section.w )
-        w2s(&section);
-    else
-        section.s = get_value(&cfg, "global", "default");
+        if ( section.w )
+            w2s(&section);
+        else
+            section.s = get_value(&cfg, "global", "default");
 
-    for ( ; ; )
-    {
-        name.s = get_value(&cfg, section.s, "kernel");
-        if ( name.s )
-            break;
-        name.s = get_value(&cfg, "global", "chain");
-        if ( !name.s )
-            break;
-        efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
-        cfg.addr = 0;
-        if ( !read_file(dir_handle, s2w(&name), &cfg, NULL) )
+        for ( ; ; )
         {
-            PrintStr(L"Chained configuration file '");
-            PrintStr(name.w);
+            name.s = get_value(&cfg, section.s, "kernel");
+            if ( name.s )
+                break;
+            name.s = get_value(&cfg, "global", "chain");
+            if ( !name.s )
+                break;
+            efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
+            cfg.addr = 0;
+            if ( !read_file(dir_handle, s2w(&name), &cfg, NULL) )
+            {
+                PrintStr(L"Chained configuration file '");
+                PrintStr(name.w);
+                efi_bs->FreePool(name.w);
+                blexit(L"'not found.");
+            }
+            pre_parse(&cfg);
             efi_bs->FreePool(name.w);
-            blexit(L"'not found.");
         }
-        pre_parse(&cfg);
-        efi_bs->FreePool(name.w);
-    }
-    if ( !name.s )
-        blexit(L"No Dom0 kernel image specified.");
-
-    efi_arch_cfg_file_early(dir_handle, section.s);
 
-    option_str = split_string(name.s);
-    read_file(dir_handle, s2w(&name), &kernel, option_str);
-    efi_bs->FreePool(name.w);
+        if ( !name.s )
+            blexit(L"No Dom0 kernel image specified.");
 
-    if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
-                    (void **)&shim_lock)) &&
-         (status = shim_lock->Verify(kernel.ptr, kernel.size)) != EFI_SUCCESS )
-        PrintErrMesg(L"Dom0 kernel image could not be verified", status);
+        efi_arch_cfg_file_early(dir_handle, section.s);
 
-    name.s = get_value(&cfg, section.s, "ramdisk");
-    if ( name.s )
-    {
         option_str = split_string(name.s);
-        read_file(dir_handle, s2w(&name), &ramdisk, option_str);
+        read_file(dir_handle, s2w(&name), &kernel, option_str);
         efi_bs->FreePool(name.w);
-    }
 
-    name.s = get_value(&cfg, section.s, "xsm");
-    if ( name.s )
-    {
-        option_str = split_string(name.s);
-        read_file(dir_handle, s2w(&name), &xsm, option_str);
-        efi_bs->FreePool(name.w);
-    }
+        if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
+                        (void **)&shim_lock)) &&
+             (status = shim_lock->Verify(kernel.ptr, kernel.size)) != EFI_SUCCESS )
+            PrintErrMesg(L"Dom0 kernel image could not be verified", status);
 
-    name.s = get_value(&cfg, section.s, "options");
-    efi_arch_handle_cmdline(argc ? *argv : NULL, options, name.s);
+        name.s = get_value(&cfg, section.s, "ramdisk");
+        if ( name.s )
+        {
+            option_str = split_string(name.s);
+            read_file(dir_handle, s2w(&name), &ramdisk, option_str);
+            efi_bs->FreePool(name.w);
+        }
 
-    cols = rows = depth = 0;
-    if ( !base_video )
-    {
-        name.cs = get_value(&cfg, section.s, "video");
-        if ( !name.cs )
-            name.cs = get_value(&cfg, "global", "video");
-        if ( name.cs && !strncmp(name.cs, "gfx-", 4) )
+        name.s = get_value(&cfg, section.s, "xsm");
+        if ( name.s )
         {
-            cols = simple_strtoul(name.cs + 4, &name.cs, 10);
-            if ( *name.cs == 'x' )
-                rows = simple_strtoul(name.cs + 1, &name.cs, 10);
-            if ( *name.cs == 'x' )
-                depth = simple_strtoul(name.cs + 1, &name.cs, 10);
-            if ( *name.cs )
-                cols = rows = depth = 0;
+            option_str = split_string(name.s);
+            read_file(dir_handle, s2w(&name), &xsm, option_str);
+            efi_bs->FreePool(name.w);
+        }
+
+        name.s = get_value(&cfg, section.s, "options");
+        efi_arch_handle_cmdline(argc ? *argv : NULL, options, name.s);
+
+        if ( !base_video )
+        {
+            name.cs = get_value(&cfg, section.s, "video");
+            if ( !name.cs )
+                name.cs = get_value(&cfg, "global", "video");
+            if ( name.cs && !strncmp(name.cs, "gfx-", 4) )
+            {
+                cols = simple_strtoul(name.cs + 4, &name.cs, 10);
+                if ( *name.cs == 'x' )
+                    rows = simple_strtoul(name.cs + 1, &name.cs, 10);
+                if ( *name.cs == 'x' )
+                    depth = simple_strtoul(name.cs + 1, &name.cs, 10);
+                if ( *name.cs )
+                    cols = rows = depth = 0;
+            }
         }
-    }
-    efi_arch_cfg_file_late(dir_handle, section.s);
 
-    efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
-    cfg.addr = 0;
+        efi_arch_cfg_file_late(dir_handle, section.s);
 
-    dir_handle->Close(dir_handle);
+        efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
+        cfg.addr = 0;
+
+        dir_handle->Close(dir_handle);
+
+    }
 
     if ( gop && !base_video )
     {
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH for-4.5 V6 13/14] Fix freeing of uninitialized pointer
  2014-09-24  5:02 [PATCH for-4.5 V6 00/14] arm64 EFI stub Roy Franz
                   ` (11 preceding siblings ...)
  2014-09-24  5:03 ` [PATCH for-4.5 V6 12/14] Add efi_arch_use_config_file() function to control use of config file Roy Franz
@ 2014-09-24  5:03 ` Roy Franz
  2014-09-24  9:07   ` Jan Beulich
  2014-09-24  5:03 ` [PATCH for-4.5 V6 14/14] Add ARM EFI boot support Roy Franz
  2014-09-24  8:55 ` [PATCH for-4.5 V6 00/14] arm64 EFI stub Jan Beulich
  14 siblings, 1 reply; 30+ messages in thread
From: Roy Franz @ 2014-09-24  5:03 UTC (permalink / raw)
  To: xen-devel, ian.campbell, stefano.stabellini, tim, jbeulich, keir
  Cc: Roy Franz, fu.wei

The only valid response from the LocateHandle() call is EFI_BUFFER_TOO_SMALL,
so exit if we get anything else.  We pass a 0 size/NULL pointer buffer, so the
only other returns we will get is an error.  Return right away as there is
nothing to do.  Also return if there is an error allocating the buffer, as the
previous code path also allowed for an undefined pointer to be freed.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
 xen/common/efi/boot.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index d5c9355..54db5c9 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -582,9 +582,15 @@ static void __init setup_efi_pci(void)
     status = efi_bs->LocateHandle(ByProtocol, &pci_guid, NULL, &size, NULL);
     if ( status == EFI_BUFFER_TOO_SMALL )
         status = efi_bs->AllocatePool(EfiLoaderData, size, (void **)&handles);
+    else
+        return;
+
     if ( !EFI_ERROR(status) )
         status = efi_bs->LocateHandle(ByProtocol, &pci_guid, NULL, &size,
                                       handles);
+    else
+        return;
+
     if ( EFI_ERROR(status) )
         size = 0;
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH for-4.5 V6 14/14] Add ARM EFI boot support
  2014-09-24  5:02 [PATCH for-4.5 V6 00/14] arm64 EFI stub Roy Franz
                   ` (12 preceding siblings ...)
  2014-09-24  5:03 ` [PATCH for-4.5 V6 13/14] Fix freeing of uninitialized pointer Roy Franz
@ 2014-09-24  5:03 ` Roy Franz
  2014-09-24 16:11   ` Julien Grall
  2014-09-24  8:55 ` [PATCH for-4.5 V6 00/14] arm64 EFI stub Jan Beulich
  14 siblings, 1 reply; 30+ messages in thread
From: Roy Franz @ 2014-09-24  5:03 UTC (permalink / raw)
  To: xen-devel, ian.campbell, stefano.stabellini, tim, jbeulich, keir
  Cc: Roy Franz, fu.wei

This patch adds EFI boot support for ARM based on the previous refactoring of
the x86 EFI boot code.  All ARM specific code is in the ARM efi-boot.h header
file, with the main EFI entry point common/efi/boot.c.  The PE/COFF header is
open-coded in head.S, which allows us to have a single binary be both an EFI
executable and a normal arm64 IMAGE file. There is currently no PE/COFF
toolchain support for arm64, so it is not possible to create the PE/COFF header
in the same manner as on x86.  This also simplifies the build as compared to
x86, as we always build the same executable, whereas x86 builds 2.  An ARM
version of efi-bind.h is added, which is based on the x86_64 version with the
x86 specific portions removed.  The Makefile in common/efi is different for x86
and ARM, as for ARM we always build in EFI support.
NR_MEM_BANKS is increased, as memory regions are now added from the UEFI memory map,
rather than memory banks from a DTB.  The UEFI memory map may be fragmented so a larger
number of regions will be used.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
 .gitignore                          |   4 +
 xen/arch/arm/Makefile               |   1 +
 xen/arch/arm/arm64/head.S           | 150 ++++++++-
 xen/arch/arm/efi/Makefile           |   4 +
 xen/arch/arm/efi/efi-boot.h         | 593 ++++++++++++++++++++++++++++++++++++
 xen/arch/arm/xen.lds.S              |   1 +
 xen/arch/x86/efi/efi-boot.h         |   2 +
 xen/common/efi/boot.c               |  10 +-
 xen/common/efi/efi.h                |   2 -
 xen/common/efi/runtime.c            |  15 +-
 xen/include/asm-arm/arm64/efibind.h | 216 +++++++++++++
 xen/include/asm-arm/efibind.h       |   2 +
 xen/include/asm-arm/setup.h         |   2 +-
 13 files changed, 990 insertions(+), 12 deletions(-)
 create mode 100644 xen/arch/arm/efi/Makefile
 create mode 100644 xen/arch/arm/efi/efi-boot.h
 create mode 100644 xen/include/asm-arm/arm64/efibind.h
 create mode 100644 xen/include/asm-arm/efibind.h

diff --git a/.gitignore b/.gitignore
index 35e4147..0ab2f62 100644
--- a/.gitignore
+++ b/.gitignore
@@ -258,6 +258,10 @@ xen/arch/x86/efi/boot.c
 xen/arch/x86/efi/runtime.c
 xen/arch/x86/efi/compat.c
 xen/arch/x86/efi/efi.h
+xen/arch/arm/efi/boot.c
+xen/arch/arm/efi/runtime.c
+xen/arch/arm/efi/compat.c
+xen/arch/arm/efi/efi.h
 xen/ddb/*
 xen/include/headers.chk
 xen/include/asm
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index e557cac..f330302 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -1,6 +1,7 @@
 subdir-$(arm32) += arm32
 subdir-$(arm64) += arm64
 subdir-y += platforms
+subdir-$(arm64) += efi
 
 obj-$(EARLY_PRINTK) += early_printk.o
 obj-y += cpu.o
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index d22af1c..7650abe 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -24,6 +24,8 @@
 #include <asm/page.h>
 #include <asm/asm_defns.h>
 #include <asm/early_printk.h>
+#include <efi/efierr.h>
+#include <asm/arm64/efibind.h>
 
 #define PT_PT     0xf7f /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=1 P=1 */
 #define PT_MEM    0xf7d /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=0 P=1 */
@@ -104,8 +106,14 @@ GLOBAL(start)
         /*
          * DO NOT MODIFY. Image header expected by Linux boot-loaders.
          */
-        b       real_start           /* branch to kernel start, magic */
-        .long   0                    /* reserved */
+efi_head:
+        /*
+         * This add instruction has no meaningful effect except that
+         * its opcode forms the magic "MZ" signature of a PE/COFF file
+         * that is required for UEFI applications.
+         */
+        add     x13, x18, #0x16
+        b       real_start           /* branch to kernel start */
         .quad   0                    /* Image load offset from start of RAM */
         .quad   0                    /* reserved */
         .quad   0                    /* reserved */
@@ -116,8 +124,113 @@ GLOBAL(start)
         .byte   0x52
         .byte   0x4d
         .byte   0x64
-        .word   0                    /* reserved */
+        .long   pe_header - efi_head        /* Offset to the PE header. */
+
+        /*
+         * Add the PE/COFF header to the file.  The address of this header
+         * is at offset 0x3c in the file, and is part of Linux "Image"
+         * header.  The arm64 Linux Image format is designed to support
+         * being both an 'Image' format binary and a PE/COFF binary.
+         * The PE/COFF format is defined by Microsoft, and is available
+         * from: http://msdn.microsoft.com/en-us/gg463119.aspx
+         * Version 8.3 adds support for arm64 and UEFI usage.
+         */
+
+        .align  3
+pe_header:
+        .ascii  "PE"
+        .short  0
+coff_header:
+        .short  0xaa64                          /* AArch64 */
+        .short  2                               /* nr_sections */
+        .long   0                               /* TimeDateStamp */
+        .long   0                               /* PointerToSymbolTable */
+        .long   1                               /* NumberOfSymbols */
+        .short  section_table - optional_header /* SizeOfOptionalHeader */
+        .short  0x206                           /* Characteristics. */
+                                                /* IMAGE_FILE_DEBUG_STRIPPED | */
+                                                /* IMAGE_FILE_EXECUTABLE_IMAGE | */
+                                                /* IMAGE_FILE_LINE_NUMS_STRIPPED */
+optional_header:
+        .short  0x20b                           /* PE32+ format */
+        .byte   0x02                            /* MajorLinkerVersion */
+        .byte   0x14                            /* MinorLinkerVersion */
+        .long   _end - real_start               /* SizeOfCode */
+        .long   0                               /* SizeOfInitializedData */
+        .long   0                               /* SizeOfUninitializedData */
+        .long   efi_start - efi_head            /* AddressOfEntryPoint */
+        .long   real_start - efi_head           /* BaseOfCode */
+
+extra_header_fields:
+        .quad   0                               /* ImageBase */
+        .long   0x1000                          /* SectionAlignment (4 KByte) */
+        .long   0x8                             /* FileAlignment */
+        .short  0                               /* MajorOperatingSystemVersion */
+        .short  0                               /* MinorOperatingSystemVersion */
+        .short  0                               /* MajorImageVersion */
+        .short  0                               /* MinorImageVersion */
+        .short  0                               /* MajorSubsystemVersion */
+        .short  0                               /* MinorSubsystemVersion */
+        .long   0                               /* Win32VersionValue */
+
+        .long   _end - efi_head                 /* SizeOfImage */
+
+        /* Everything before the kernel image is considered part of the header */
+        .long   real_start - efi_head           /* SizeOfHeaders */
+        .long   0                               /* CheckSum */
+        .short  0xa                             /* Subsystem (EFI application) */
+        .short  0                               /* DllCharacteristics */
+        .quad   0                               /* SizeOfStackReserve */
+        .quad   0                               /* SizeOfStackCommit */
+        .quad   0                               /* SizeOfHeapReserve */
+        .quad   0                               /* SizeOfHeapCommit */
+        .long   0                               /* LoaderFlags */
+        .long   0x6                             /* NumberOfRvaAndSizes */
+
+        .quad   0                               /* ExportTable */
+        .quad   0                               /* ImportTable */
+        .quad   0                               /* ResourceTable */
+        .quad   0                               /* ExceptionTable */
+        .quad   0                               /* CertificationTable */
+        .quad   0                               /* BaseRelocationTable */
+
+        /* Section table */
+section_table:
 
+        /*
+         * The EFI application loader requires a relocation section
+         * because EFI applications must be relocatable.  This is a
+         * dummy section as far as we are concerned.
+         */
+        .ascii  ".reloc"
+        .byte   0
+        .byte   0                               /* end of 0 padding of section name */
+        .long   0
+        .long   0
+        .long   0                               /* SizeOfRawData */
+        .long   0                               /* PointerToRawData */
+        .long   0                               /* PointerToRelocations */
+        .long   0                               /* PointerToLineNumbers */
+        .short  0                               /* NumberOfRelocations */
+        .short  0                               /* NumberOfLineNumbers */
+        .long   0x42100040                      /* Characteristics (section flags) */
+
+
+        .ascii  ".text"
+        .byte   0
+        .byte   0
+        .byte   0                               /* end of 0 padding of section name */
+        .long   _end - real_start               /* VirtualSize */
+        .long   real_start - efi_head           /* VirtualAddress */
+        .long   __init_end_efi - real_start     /* SizeOfRawData */
+        .long   real_start - efi_head           /* PointerToRawData */
+
+        .long   0                /* PointerToRelocations (0 for executables) */
+        .long   0                /* PointerToLineNumbers (0 for executables) */
+        .short  0                /* NumberOfRelocations  (0 for executables) */
+        .short  0                /* NumberOfLineNumbers  (0 for executables) */
+        .long   0xe0500020       /* Characteristics (section flags) */
+        .align  5
 real_start:
         msr   DAIFSet, 0xf           /* Disable all interrupts */
 
@@ -621,6 +734,37 @@ putn:   ret
 ENTRY(lookup_processor_type)
         mov  x0, #0
         ret
+/*
+ *  Function to transition from EFI loader in C, to Xen entry point.
+ *  void noreturn efi_xen_start(void *fdt_ptr);
+ */
+ENTRY(efi_xen_start)
+        /*
+         * Turn off cache and MMU as Xen expects. EFI enables them, but also
+         * mandates a 1:1 (unity) VA->PA mapping, so we can turn off the
+         * MMU while executing EFI code before entering Xen.
+         * The EFI loader calls this to start Xen.
+         * Preserve x0 (fdf pointer) across call to __flush_dcache_all,
+         * restore for entry into Xen.
+         */
+        mov   x20, x0
+        bl    __flush_dcache_all
+        ic    ialluis
+
+        /* Turn off Dcache and MMU */
+        mrs   x0, sctlr_el2
+        bic   x0, x0, #1 << 0        /* clear SCTLR.M */
+        bic   x0, x0, #1 << 2        /* clear SCTLR.C */
+        msr   sctlr_el2, x0
+        isb
+
+        /* Jump to Xen entry point */
+        mov   x0, x20
+        mov   x1, xzr
+        mov   x2, xzr
+        mov   x3, xzr
+        b     real_start
+ENDPROC(efi_xen_start)
 
 /*
  * Local variables:
diff --git a/xen/arch/arm/efi/Makefile b/xen/arch/arm/efi/Makefile
new file mode 100644
index 0000000..e8a93e3
--- /dev/null
+++ b/xen/arch/arm/efi/Makefile
@@ -0,0 +1,4 @@
+CFLAGS += -fshort-wchar
+
+obj-y +=  boot.init.o runtime.o
+
diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
new file mode 100644
index 0000000..f01a143
--- /dev/null
+++ b/xen/arch/arm/efi/efi-boot.h
@@ -0,0 +1,593 @@
+/*
+ * Architecture specific implementation for EFI boot code.  This file
+ * is intended to be included by common/efi/boot.c _only_, and
+ * therefore can define arch specific global variables.
+ */
+#include <xen/libfdt/libfdt.h>
+#include <asm/setup.h>
+
+void noreturn efi_xen_start(void *fdt_ptr);
+
+#define DEVICE_TREE_GUID \
+{0xb1b621d5, 0xf19c, 0x41a5, {0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0}}
+
+static struct file __initdata dtbfile;
+static void __initdata *fdt;
+static void __initdata *memmap;
+
+static int __init setup_chosen_node(void *fdt, int *addr_cells, int *size_cells)
+{
+    int node;
+    const struct fdt_property *prop;
+    int len;
+    uint32_t val;
+
+    if ( !fdt || !addr_cells || !size_cells )
+        return -1;
+
+    /* locate chosen node, which is where we add Xen module info. */
+    node = fdt_subnode_offset(fdt, 0, "chosen");
+    if ( node < 0 )
+    {
+        node = fdt_add_subnode(fdt, 0, "chosen");
+        if ( node < 0 )
+            return node;
+    }
+
+    /* Get or set #address-cells and #size-cells */
+    prop = fdt_get_property(fdt, node, "#address-cells", &len);
+    if ( !prop )
+    {
+        val = cpu_to_fdt32(2);
+        if ( fdt_setprop(fdt, node, "#address-cells", &val, sizeof(val)) )
+            return -1;
+        *addr_cells = 2;
+    }
+    else
+        *addr_cells = fdt32_to_cpu(*((uint32_t *)prop->data));
+
+    prop = fdt_get_property(fdt, node, "#size-cells", &len);
+    if ( !prop )
+    {
+        val = cpu_to_fdt32(2);
+        if ( fdt_setprop(fdt, node, "#size-cells", &val, sizeof(val)) )
+            return -1;
+        *size_cells = 2;
+    }
+    else
+        *size_cells = fdt32_to_cpu(*((uint32_t *)prop->data));
+
+    /*
+     * Make sure ranges is empty if it exists, otherwise create empty ranges
+     * property.
+     */
+    prop = fdt_get_property(fdt, node, "ranges", &len);
+    if ( !prop )
+    {
+        val = cpu_to_fdt32(0);
+        if ( fdt_setprop(fdt, node, "ranges", &val, 0) )
+            return -1;
+    }
+    else if ( fdt32_to_cpu(prop->len) )
+            return -1;  /* Non-empty ranges property */
+    return node;
+}
+
+/*
+ * Set a single 'reg' property taking into account the
+ * configured addr and size cell sizes.
+ */
+static int __init fdt_set_reg(void *fdt, int node, int addr_cells,
+                              int size_cells, uint64_t addr, uint64_t len)
+{
+    uint8_t data[16]; /* at most 2 64 bit words */
+    void *p = data;
+
+    /* Make sure that the values provided can be represented in
+     * the reg property.
+     */
+    if ( addr_cells == 1 && (addr >> 32) )
+        return -1;
+    if ( size_cells == 1 && (len >> 32) )
+        return -1;
+
+    if ( addr_cells == 1 )
+    {
+        *(uint32_t *)p = cpu_to_fdt32(addr);
+        p += sizeof(uint32_t);
+    }
+    else if ( addr_cells == 2 )
+    {
+        *(uint64_t *)p = cpu_to_fdt64(addr);
+        p += sizeof(uint64_t);
+    }
+    else
+        return -1;
+
+    if ( size_cells == 1 )
+    {
+        *(uint32_t *)p = cpu_to_fdt32(len);
+        p += sizeof(uint32_t);
+    }
+    else if ( size_cells == 2 )
+    {
+        *(uint64_t *)p = cpu_to_fdt64(len);
+        p += sizeof(uint64_t);
+    }
+    else
+        return -1;
+
+    return(fdt_setprop(fdt, node, "reg", data, p - (void *)data));
+}
+
+static void __init *lookup_fdt_config_table(EFI_SYSTEM_TABLE *sys_table)
+{
+    const EFI_GUID fdt_guid = DEVICE_TREE_GUID;
+    EFI_CONFIGURATION_TABLE *tables;
+    void *fdt = NULL;
+    int i;
+
+    tables = sys_table->ConfigurationTable;
+    for ( i = 0; i < sys_table->NumberOfTableEntries; i++ )
+    {
+        if ( match_guid(&tables[i].VendorGuid, &fdt_guid) )
+        {
+            fdt = tables[i].VendorTable;
+            break;
+        }
+    }
+    return fdt;
+}
+
+static EFI_STATUS __init efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR *map,
+                                                UINTN mmap_size,
+                                                UINTN desc_size)
+{
+    int Index;
+    int i = 0;
+    EFI_MEMORY_DESCRIPTOR *desc_ptr = map;
+
+    for ( Index = 0; Index < (mmap_size / desc_size); Index++ )
+    {
+        if ( desc_ptr->Type == EfiConventionalMemory
+             || desc_ptr->Type == EfiBootServicesCode
+             || desc_ptr->Type == EfiBootServicesData )
+        {
+            bootinfo.mem.bank[i].start = desc_ptr->PhysicalStart;
+            bootinfo.mem.bank[i].size = desc_ptr->NumberOfPages * EFI_PAGE_SIZE;
+            if ( ++i >= NR_MEM_BANKS )
+            {
+                PrintStr(L"Warning: All ");
+                DisplayUint(NR_MEM_BANKS, -1);
+                PrintStr(L" bootinfo mem banks exhausted.\r\n");
+                break;
+            }
+        }
+        desc_ptr = NextMemoryDescriptor(desc_ptr, desc_size);
+    }
+
+    bootinfo.mem.nr_banks = i;
+    return EFI_SUCCESS;
+}
+
+/*
+ * Add the FDT nodes for the standard EFI information, which consist
+ * of the System table address, the address of the final EFI memory map,
+ * and memory map information.
+ */
+EFI_STATUS __init fdt_add_uefi_nodes(EFI_SYSTEM_TABLE *sys_table,
+                                            void *fdt,
+                                            EFI_MEMORY_DESCRIPTOR *memory_map,
+                                            UINTN map_size,
+                                            UINTN desc_size,
+                                            UINT32 desc_ver)
+{
+    int node;
+    int status;
+    u32 fdt_val32;
+    u64 fdt_val64;
+    int prev;
+    int num_rsv;
+
+    /*
+     * Delete any memory nodes present.  The EFI memory map is the only
+     * memory description provided to Xen.
+     */
+    prev = 0;
+    for (;;)
+    {
+        const char *type;
+        int len;
+
+        node = fdt_next_node(fdt, prev, NULL);
+        if ( node < 0 )
+            break;
+
+        type = fdt_getprop(fdt, node, "device_type", &len);
+        if ( type && strncmp(type, "memory", len) == 0 )
+        {
+            fdt_del_node(fdt, node);
+            continue;
+        }
+
+        prev = node;
+    }
+
+   /*
+    * Delete all memory reserve map entries. When booting via UEFI,
+    * kernel will use the UEFI memory map to find reserved regions.
+    */
+   num_rsv = fdt_num_mem_rsv(fdt);
+   while ( num_rsv-- > 0 )
+       fdt_del_mem_rsv(fdt, num_rsv);
+
+    /* Add FDT entries for EFI runtime services in chosen node. */
+    node = fdt_subnode_offset(fdt, 0, "chosen");
+    if ( node < 0 )
+    {
+        node = fdt_add_subnode(fdt, 0, "chosen");
+        if ( node < 0 )
+        {
+            status = node; /* node is error code when negative */
+            goto fdt_set_fail;
+        }
+    }
+
+    fdt_val64 = cpu_to_fdt64((u64)(uintptr_t)sys_table);
+    status = fdt_setprop(fdt, node, "linux,uefi-system-table",
+                         &fdt_val64, sizeof(fdt_val64));
+    if ( status )
+        goto fdt_set_fail;
+
+    fdt_val64 = cpu_to_fdt64((u64)(uintptr_t)memory_map);
+    status = fdt_setprop(fdt, node, "linux,uefi-mmap-start",
+                         &fdt_val64,  sizeof(fdt_val64));
+    if ( status )
+        goto fdt_set_fail;
+
+    fdt_val32 = cpu_to_fdt32(map_size);
+    status = fdt_setprop(fdt, node, "linux,uefi-mmap-size",
+                         &fdt_val32,  sizeof(fdt_val32));
+    if ( status )
+        goto fdt_set_fail;
+
+    fdt_val32 = cpu_to_fdt32(desc_size);
+    status = fdt_setprop(fdt, node, "linux,uefi-mmap-desc-size",
+                         &fdt_val32, sizeof(fdt_val32));
+    if ( status )
+        goto fdt_set_fail;
+
+    fdt_val32 = cpu_to_fdt32(desc_ver);
+    status = fdt_setprop(fdt, node, "linux,uefi-mmap-desc-ver",
+                         &fdt_val32, sizeof(fdt_val32));
+    if ( status )
+        goto fdt_set_fail;
+
+    return EFI_SUCCESS;
+
+fdt_set_fail:
+    if ( status == -FDT_ERR_NOSPACE )
+        return EFI_BUFFER_TOO_SMALL;
+
+    return EFI_LOAD_ERROR;
+}
+
+/*
+ * Allocates new memory for a larger FDT, and frees existing memory if
+ * struct file size is non-zero.  Updates file struct with new memory
+ * address/size for later freeing.  If fdtfile.ptr is NULL, an empty FDT
+ * is created.
+ */
+static void __init *fdt_increase_size(struct file *fdtfile, int add_size)
+{
+    EFI_STATUS status;
+    EFI_PHYSICAL_ADDRESS fdt_addr;
+    int fdt_size;
+    int pages;
+    void *new_fdt;
+
+    if ( fdtfile->ptr )
+        fdt_size = fdt_totalsize(fdtfile->ptr);
+    else
+        fdt_size = 0;
+
+    pages = PFN_UP(fdt_size + add_size);
+    status = efi_bs->AllocatePages(AllocateAnyPages, EfiLoaderData,
+                                   pages, &fdt_addr);
+
+    if ( status != EFI_SUCCESS )
+        return NULL;
+
+    new_fdt = (void *)fdt_addr;
+
+    if ( fdt_size )
+    {
+        if ( fdt_open_into(dtbfile.ptr, new_fdt, pages * EFI_PAGE_SIZE) )
+            return NULL;
+    }
+    else
+    {
+        /*
+         * Create an empty FDT if not provided one, which is the expected case
+         * when booted from the UEFI shell on an ACPI only system.  We will use
+         * the FDT to pass the EFI information to Xen, as well as nodes for
+         * any modules the stub loads.  The ACPI tables are part of the UEFI
+         * system table that is passed in the FDT.
+         */
+        if ( fdt_create_empty_tree(new_fdt, pages * EFI_PAGE_SIZE) )
+            return NULL;
+    }
+
+    /*
+     * Now that we have the new FDT allocated and copied, free the
+     * original and update the struct file so that the error handling
+     * code will free it.  If the original FDT came from a configuration
+     * table, we don't own that memory and can't free it.
+     */
+    if ( dtbfile.size )
+        efi_bs->FreePages(dtbfile.addr, PFN_UP(dtbfile.size));
+
+    /* Update 'file' info for new memory so we clean it up on error exits */
+    dtbfile.addr = fdt_addr;
+    dtbfile.size = pages * EFI_PAGE_SIZE;
+    return new_fdt;
+}
+
+static void __init efi_arch_relocate_image(unsigned long delta)
+{
+}
+
+static void __init efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
+                                               void *map,
+                                               UINTN map_size,
+                                               UINTN desc_size,
+                                               UINT32 desc_ver)
+{
+    EFI_STATUS status;
+
+    status = efi_process_memory_map_bootinfo(map, map_size, desc_size);
+    if ( EFI_ERROR(status) )
+        blexit(L"ERROR processing EFI memory map");
+
+    status = fdt_add_uefi_nodes(SystemTable, fdt, map, map_size, desc_size,
+                                desc_ver);
+    if ( EFI_ERROR(status) )
+        PrintErrMesg(L"ERROR updating FDT\r\n", status);
+}
+
+static void __init efi_arch_pre_exit_boot(void)
+{
+}
+
+static void __init efi_arch_post_exit_boot(void)
+{
+    efi_xen_start(fdt);
+}
+
+static void __init efi_arch_cfg_file_early(EFI_FILE_HANDLE dir_handle, char *section)
+{
+    union string name;
+
+    /*
+     * The DTB must be processed before any other entries in the configuration
+     * file, as the DTB is updated as modules are loaded.
+     */
+    name.s = get_value(&cfg, section, "dtb");
+    if ( name.s )
+    {
+        split_string(name.s);
+        read_file(dir_handle, s2w(&name), &dtbfile, NULL);
+            blexit(NULL);
+        efi_bs->FreePool(name.w);
+    }
+    fdt = fdt_increase_size(&dtbfile, cfg.size + EFI_PAGE_SIZE);
+    if ( !fdt )
+        blexit(L"Unable to create new FDT");
+}
+
+static void __init efi_arch_cfg_file_late(EFI_FILE_HANDLE dir_handle, char *section)
+{
+}
+
+static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size)
+{
+    void *ptr;
+    EFI_STATUS status;
+
+    status = efi_bs->AllocatePool(EfiLoaderData, map_size + EFI_PAGE_SIZE, &ptr);
+    if ( status != EFI_SUCCESS )
+        return NULL;
+    return ptr;
+}
+
+static void __init efi_arch_edd(void)
+{
+}
+
+static void __init efi_arch_memory_setup(void)
+{
+}
+
+static void __init efi_arch_handle_cmdline(CHAR16 *image_name,
+                                           CHAR16 *cmdline_options,
+                                           char *cfgfile_options)
+{
+    union string name;
+    char *buf;
+    EFI_STATUS status;
+    int prop_len;
+    int chosen;
+
+    /* locate chosen node, which is where we add Xen module info. */
+    chosen = fdt_subnode_offset(fdt, 0, "chosen");
+    if ( chosen < 0 )
+        blexit(L"ERROR unable to find chosen node");
+
+    status = efi_bs->AllocatePool(EfiBootServicesData, EFI_PAGE_SIZE, (void **)&buf);
+    if ( EFI_ERROR(status) )
+        PrintErrMesg(L"ERROR allocating memory.\r\n", status);
+
+    if ( image_name )
+    {
+        name.w = image_name;
+        w2s(&name);
+    }
+    else
+        name.s = "xen";
+
+    prop_len = 0;
+    prop_len += snprintf(buf + prop_len,
+                           EFI_PAGE_SIZE - prop_len, "%s", name.s);
+    if ( prop_len >= EFI_PAGE_SIZE )
+        blexit(L"FDT string overflow");
+
+    if ( cfgfile_options )
+    {
+        prop_len += snprintf(buf + prop_len,
+                               EFI_PAGE_SIZE - prop_len, " %s", cfgfile_options);
+        if ( prop_len >= EFI_PAGE_SIZE )
+            blexit(L"FDT string overflow");
+    }
+
+    if ( cmdline_options )
+    {
+        name.w = cmdline_options;
+        w2s(&name);
+    }
+    else
+        name.s = NULL;
+
+    if ( name.s )
+    {
+        prop_len += snprintf(buf + prop_len,
+                               EFI_PAGE_SIZE - prop_len, " %s", name.s);
+        if ( prop_len >= EFI_PAGE_SIZE )
+            blexit(L"FDT string overflow");
+    }
+
+    if ( fdt_setprop_string(fdt, chosen, "xen,xen-bootargs", buf) < 0 )
+        blexit(L"unable to set xen,xen-bootargs property.");
+
+    efi_bs->FreePool(buf);
+}
+
+static void __init efi_arch_handle_module(struct file *file, const CHAR16 *name,
+                                          char *options)
+{
+    int node;
+    int chosen;
+    int addr_len, size_len;
+
+    if ( file == &dtbfile )
+        return;
+    chosen = setup_chosen_node(fdt, &addr_len, &size_len);
+    if ( chosen < 0 )
+        blexit(L"Unable to setup chosen node");
+
+    if ( file == &ramdisk )
+    {
+        char ramdisk_compat[] = "multiboot,ramdisk\0multiboot,module";
+        node = fdt_add_subnode(fdt, chosen, "ramdisk");
+        if ( node < 0 )
+            blexit(L"Error adding ramdisk FDT node.");
+        if ( fdt_setprop(fdt, node, "compatible", ramdisk_compat,
+                         sizeof(ramdisk_compat)) < 0 )
+            blexit(L"unable to set compatible property.");
+        if ( fdt_set_reg(fdt, node, addr_len, size_len, ramdisk.addr,
+                    ramdisk.size) < 0 )
+            blexit(L"unable to set reg property.");
+    }
+    else if ( file == &xsm )
+    {
+        char xsm_compat[] = "xen,xsm-policy\0multiboot,module";
+        node = fdt_add_subnode(fdt, chosen, "xsm");
+        if ( node < 0 )
+            blexit(L"Error adding xsm FDT node.");
+        if ( fdt_setprop(fdt, node, "compatible", xsm_compat,
+                         sizeof(xsm_compat)) < 0 )
+            blexit(L"unable to set compatible property.");
+        if ( fdt_set_reg(fdt, node, addr_len, size_len, xsm.addr,
+                    xsm.size) < 0 )
+            blexit(L"unable to set reg property.");
+    }
+    else if ( file == &kernel )
+    {
+        char kernel_compat[] = "multiboot,kernel\0multiboot,module";
+        node = fdt_add_subnode(fdt, chosen, "kernel");
+        if ( node < 0 )
+            blexit(L"Error adding dom0 FDT node.");
+        if ( fdt_setprop(fdt, node, "compatible", kernel_compat,
+                         sizeof(kernel_compat)) < 0 )
+            blexit(L"unable to set compatible property.");
+        if ( options && fdt_setprop_string(fdt, node, "bootargs", options) < 0 )
+            blexit(L"unable to set bootargs property.");
+        if ( fdt_set_reg(fdt, node, addr_len, size_len, kernel.addr,
+                         kernel.size) < 0 )
+            blexit(L"unable to set reg property.");
+    }
+    else
+        blexit(L"Unknown module type");
+}
+
+static void __init efi_arch_cpu(void)
+{
+}
+
+static void __init efi_arch_blexit(void)
+{
+    if ( dtbfile.addr && dtbfile.size )
+        efi_bs->FreePages(dtbfile.addr, PFN_UP(dtbfile.size));
+    if ( memmap )
+        efi_bs->FreePool(memmap);
+}
+
+static void __init efi_arch_load_addr_check(EFI_LOADED_IMAGE *loaded_image)
+{
+    if ( (unsigned long)loaded_image->ImageBase & ((1 << 12) - 1) )
+        blexit(L"Xen must be loaded at a 4 KByte boundary.");
+}
+
+static bool_t __init efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)
+{
+    /*
+     * For arm, we may get a device tree from GRUB (or other bootloader)
+     * that contains modules that have already been loaded into memory.  In
+     * this case, we do not use a configuration file, and rely on the
+     * bootloader to have loaded all required modules and appropriate
+     * options.
+     */
+
+    fdt = lookup_fdt_config_table(SystemTable);
+    dtbfile.ptr = fdt;
+    dtbfile.size = 0;  /* Config table memory can't be freed, so set size to 0 */
+    if ( !fdt || fdt_node_offset_by_compatible(fdt, 0, "multiboot,module") < 0 )
+    {
+        /*
+         * We either have no FDT, or one without modules, so we must have a
+         * Xen EFI configuration file to specify modules.  (dom0 required)
+         */
+        return 1;
+    }
+    PrintStr(L"Using modules provided by bootloader in FDT\r\n");
+    /* We have modules already defined in fdt, just add space. */
+    fdt = fdt_increase_size(&dtbfile, EFI_PAGE_SIZE);
+    return 0;
+}
+
+static void __init efi_arch_console_init(UINTN cols, UINTN rows)
+{
+}
+
+static void __init efi_arch_video_init(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop,
+                                       UINTN info_size,
+                                       EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *mode_info)
+{
+}
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 079e085..d8b0cfe 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -135,6 +135,7 @@ SECTIONS
        *(.xsm_initcall.init)
        __xsm_initcall_end = .;
   } :text
+  __init_end_efi = .;
   . = ALIGN(STACK_SIZE);
   __init_end = .;
 
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 72037a5..d41ecc6 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -17,6 +17,8 @@ static multiboot_info_t __initdata mbi = {
 };
 static module_t __initdata mb_modules[3];
 
+extern l4_pgentry_t *efi_l4_pgtable;
+
 static void __init edd_put_string(u8 *dst, size_t n, const char *src)
 {
     while ( n-- && *src )
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 54db5c9..d4c27ed 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -18,6 +18,7 @@
 #include <xen/string.h>
 #include <xen/stringify.h>
 #include <xen/vga.h>
+#include <xen/bitops.h>
 
 /* Using SetVirtualAddressMap() is incompatible with kexec: */
 #undef USE_SET_VIRTUAL_ADDRESS_MAP
@@ -63,6 +64,7 @@ static bool_t read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
                         struct file *file, char *options);
 static size_t wstrlen(const CHAR16 * s);
 static int set_color(u32 mask, int bpp, u8 *pos, u8 *sz);
+static bool_t match_guid(const EFI_GUID *guid1, const EFI_GUID *guid2);
 
 static EFI_BOOT_SERVICES *__initdata efi_bs;
 static EFI_HANDLE __initdata efi_ih;
@@ -118,7 +120,7 @@ static void __init DisplayUint(UINT64 Val, INTN Width)
     PrintStr(PrintString);
 }
 
-static size_t __init wstrlen(const CHAR16 *s)
+static size_t __init __maybe_unused wstrlen(const CHAR16 *s)
 {
     const CHAR16 *sc;
 
@@ -663,7 +665,7 @@ static void __init setup_efi_pci(void)
     efi_bs->FreePool(handles);
 }
 
-static int __init set_color(u32 mask, int bpp, u8 *pos, u8 *sz)
+static int __init __maybe_unused set_color(u32 mask, int bpp, u8 *pos, u8 *sz)
 {
    if ( bpp < 0 )
        return bpp;
@@ -990,8 +992,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 	       efi.smbios = (long)efi_ct[i].VendorTable;
     }
 
+#ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */
     if (efi.smbios != EFI_INVALID_TABLE_ADDR)
         dmi_efi_get_table((void *)(long)efi.smbios);
+#endif
 
     /* Collect PCI ROM contents. */
     setup_efi_pci();
@@ -1062,6 +1066,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,
@@ -1287,3 +1292,4 @@ void __init efi_init_memory(void)
         efi_l4_pgtable[i] = idle_pg_table[i];
 #endif
 }
+#endif
diff --git a/xen/common/efi/efi.h b/xen/common/efi/efi.h
index a80d5f1..09b0b0d 100644
--- a/xen/common/efi/efi.h
+++ b/xen/common/efi/efi.h
@@ -28,8 +28,6 @@ extern EFI_RUNTIME_SERVICES *efi_rs;
 extern UINTN efi_memmap_size, efi_mdesc_size;
 extern void *efi_memmap;
 
-extern l4_pgentry_t *efi_l4_pgtable;
-
 extern const struct efi_pci_rom *efi_pci_roms;
 
 extern UINT64 efi_boot_max_var_store_size, efi_boot_remain_var_store_size,
diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
index 166852d..ac9deae 100644
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -4,17 +4,21 @@
 #include <xen/guest_access.h>
 #include <xen/irq.h>
 #include <xen/time.h>
-#include <asm/mc146818rtc.h>
+extern spinlock_t rtc_lock;
 
 DEFINE_XEN_GUEST_HANDLE(CHAR16);
 
 #ifndef COMPAT
 
+#ifdef CONFIG_ARM  /* Disabled until runtime services implemented */
+const bool_t efi_enabled = 0;
+#else
 # include <asm/i387.h>
 # include <asm/xstate.h>
 # include <public/platform.h>
 
 const bool_t efi_enabled = 1;
+#endif
 
 unsigned int __read_mostly efi_num_ct;
 EFI_CONFIGURATION_TABLE *__read_mostly efi_ct;
@@ -24,7 +28,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 +44,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)
 {
     static const u16 fcw = FCW_DEFAULT;
@@ -135,7 +139,9 @@ void efi_reset_system(bool_t warm)
 }
 
 #endif
+#endif
 
+#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;
@@ -545,3 +551,4 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op)
 
     return rc;
 }
+#endif
diff --git a/xen/include/asm-arm/arm64/efibind.h b/xen/include/asm-arm/arm64/efibind.h
new file mode 100644
index 0000000..2b0bf40
--- /dev/null
+++ b/xen/include/asm-arm/arm64/efibind.h
@@ -0,0 +1,216 @@
+/*++
+
+Copyright (c) 1998  Intel Corporation
+
+Module Name:
+
+    efefind.h
+
+Abstract:
+
+    EFI to compile bindings
+
+
+
+
+Revision History
+
+--*/
+
+#ifndef __GNUC__
+#pragma pack()
+#endif
+
+#define EFIERR(a)           (0x8000000000000000 | a)
+#define EFI_ERROR_MASK      0x8000000000000000
+#define EFIERR_OEM(a)       (0xc000000000000000 | a)
+
+#define BAD_POINTER         0xFBFBFBFBFBFBFBFB
+#define MAX_ADDRESS         0xFFFFFFFFFFFFFFFF
+
+#define EFI_STUB_ERROR      MAX_ADDRESS
+
+#ifndef __ASSEMBLY__
+//
+// Basic int types of various widths
+//
+
+#if !defined(__STDC_VERSION__) || (__STDC_VERSION__ < 199901L )
+
+    // No ANSI C 1999/2000 stdint.h integer width declarations
+
+    #if defined(__GNUC__)
+        typedef unsigned long long  uint64_t __attribute__((aligned (8)));
+        typedef long long           int64_t __attribute__((aligned (8)));
+        typedef unsigned int        uint32_t;
+        typedef int                 int32_t;
+        typedef unsigned short      uint16_t;
+        typedef short               int16_t;
+        typedef unsigned char       uint8_t;
+        typedef char                int8_t;
+    #elif defined(UNIX_LP64)
+
+        /*  Use LP64 programming model from C_FLAGS for integer width declarations */
+
+       typedef unsigned long       uint64_t;
+       typedef long                int64_t;
+       typedef unsigned int        uint32_t;
+       typedef int                 int32_t;
+       typedef unsigned short      uint16_t;
+       typedef short               int16_t;
+       typedef unsigned char       uint8_t;
+       typedef char                int8_t;
+    #else
+
+       /*  Assume P64 programming model from C_FLAGS for integer width declarations */
+
+       typedef unsigned long long  uint64_t __attribute__((aligned (8)));
+       typedef long long           int64_t __attribute__((aligned (8)));
+       typedef unsigned int        uint32_t;
+       typedef int                 int32_t;
+       typedef unsigned short      uint16_t;
+       typedef short               int16_t;
+       typedef unsigned char       uint8_t;
+       typedef char                int8_t;
+    #endif
+#endif
+
+//
+// Basic EFI types of various widths
+//
+
+#ifndef __WCHAR_TYPE__
+# define __WCHAR_TYPE__ short
+#endif
+
+typedef uint64_t   UINT64;
+typedef int64_t    INT64;
+
+#ifndef _BASETSD_H_
+    typedef uint32_t   UINT32;
+    typedef int32_t    INT32;
+#endif
+
+typedef uint16_t   UINT16;
+typedef int16_t    INT16;
+typedef uint8_t    UINT8;
+typedef int8_t     INT8;
+typedef __WCHAR_TYPE__ WCHAR;
+
+#undef VOID
+#define VOID    void
+
+
+typedef int64_t    INTN;
+typedef uint64_t   UINTN;
+
+#define POST_CODE(_Data)
+
+
+#define BREAKPOINT()        while (TRUE);    // Make it hang on Bios[Dbg]32
+
+//
+// Pointers must be aligned to these address to function
+//
+
+#define MIN_ALIGNMENT_SIZE  4
+
+#define ALIGN_VARIABLE(Value ,Adjustment) \
+            (UINTN)Adjustment = 0; \
+            if((UINTN)Value % MIN_ALIGNMENT_SIZE) \
+                (UINTN)Adjustment = MIN_ALIGNMENT_SIZE - ((UINTN)Value % MIN_ALIGNMENT_SIZE); \
+            Value = (UINTN)Value + (UINTN)Adjustment
+
+
+//
+// Define macros to build data structure signatures from characters.
+//
+
+#define EFI_SIGNATURE_16(A,B)             ((A) | (B<<8))
+#define EFI_SIGNATURE_32(A,B,C,D)         (EFI_SIGNATURE_16(A,B)     | (EFI_SIGNATURE_16(C,D)     << 16))
+#define EFI_SIGNATURE_64(A,B,C,D,E,F,G,H) (EFI_SIGNATURE_32(A,B,C,D) | ((UINT64)(EFI_SIGNATURE_32(E,F,G,H)) << 32))
+
+#define EXPORTAPI
+
+
+//
+// EFIAPI - prototype calling convention for EFI function pointers
+// BOOTSERVICE - prototype for implementation of a boot service interface
+// RUNTIMESERVICE - prototype for implementation of a runtime service interface
+// RUNTIMEFUNCTION - prototype for implementation of a runtime function that is not a service
+// RUNTIME_CODE - pragma macro for declaring runtime code
+//
+
+#ifndef EFIAPI                  // Forces EFI calling conventions reguardless of compiler options
+        #define EFIAPI          // Substitute expresion to force C calling convention
+#endif
+
+#define BOOTSERVICE
+//#define RUNTIMESERVICE(proto,a)    alloc_text("rtcode",a); proto a
+//#define RUNTIMEFUNCTION(proto,a)   alloc_text("rtcode",a); proto a
+#define RUNTIMESERVICE
+#define RUNTIMEFUNCTION
+
+
+#define RUNTIME_CODE(a)         alloc_text("rtcode", a)
+#define BEGIN_RUNTIME_DATA()    data_seg("rtdata")
+#define END_RUNTIME_DATA()      data_seg("")
+
+#define VOLATILE    volatile
+
+#define MEMORY_FENCE()
+
+
+//
+// When build similiar to FW, then link everything together as
+// one big module.
+//
+
+#define EFI_DRIVER_ENTRY_POINT(InitFunction)    \
+    UINTN                                       \
+    InitializeDriver (                          \
+        VOID    *ImageHandle,                   \
+        VOID    *SystemTable                    \
+        )                                       \
+    {                                           \
+        return InitFunction(ImageHandle,        \
+                SystemTable);                   \
+    }                                           \
+                                                \
+    EFI_STATUS efi_main(                        \
+        EFI_HANDLE image,                       \
+        EFI_SYSTEM_TABLE *systab                \
+        ) __attribute__((weak,                  \
+                alias ("InitializeDriver")));
+
+#define LOAD_INTERNAL_DRIVER(_if, type, name, entry)    \
+        (_if)->LoadInternal(type, name, entry)
+
+
+//
+// Some compilers don't support the forward reference construct:
+//  typedef struct XXXXX
+//
+// The following macro provide a workaround for such cases.
+//
+#ifdef NO_INTERFACE_DECL
+#define INTERFACE_DECL(x)
+#else
+#ifdef __GNUC__
+#define INTERFACE_DECL(x) struct x
+#else
+#define INTERFACE_DECL(x) typedef struct x
+#endif
+#endif
+
+#endif
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-arm/efibind.h b/xen/include/asm-arm/efibind.h
new file mode 100644
index 0000000..09dca7a
--- /dev/null
+++ b/xen/include/asm-arm/efibind.h
@@ -0,0 +1,2 @@
+#include <xen/types.h>
+#include <asm/arm64/efibind.h>
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index 36e5704..ba5a67d 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -3,7 +3,7 @@
 
 #include <public/version.h>
 
-#define NR_MEM_BANKS 8
+#define NR_MEM_BANKS 64
 
 #define MAX_MODULES 5 /* Current maximum useful modules */
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH for-4.5 V6 00/14] arm64 EFI stub
  2014-09-24  5:02 [PATCH for-4.5 V6 00/14] arm64 EFI stub Roy Franz
                   ` (13 preceding siblings ...)
  2014-09-24  5:03 ` [PATCH for-4.5 V6 14/14] Add ARM EFI boot support Roy Franz
@ 2014-09-24  8:55 ` Jan Beulich
  2014-09-24 19:37   ` Roy Franz
  14 siblings, 1 reply; 30+ messages in thread
From: Jan Beulich @ 2014-09-24  8:55 UTC (permalink / raw)
  To: Roy Franz; +Cc: keir, ian.campbell, tim, xen-devel, stefano.stabellini, fu.wei

>>> On 24.09.14 at 07:02, <roy.franz@linaro.org> wrote:
> Open feedback:
> * moving extern declaration of efi_l4_pgtable.  I have left this x86 specific
>   declaration in the x86 specific efi-boot.h file, as I don't see a clearly
>   better place to put it.

Since runtime.c (hopefully) doesn't include efi-boot.h, this is certainly
not a viable option - files defining variables should include the headers
declaring them.

Jan

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH for-4.5 V6 13/14] Fix freeing of uninitialized pointer
  2014-09-24  5:03 ` [PATCH for-4.5 V6 13/14] Fix freeing of uninitialized pointer Roy Franz
@ 2014-09-24  9:07   ` Jan Beulich
  2014-09-24 19:22     ` Roy Franz
  0 siblings, 1 reply; 30+ messages in thread
From: Jan Beulich @ 2014-09-24  9:07 UTC (permalink / raw)
  To: Roy Franz; +Cc: keir, ian.campbell, tim, xen-devel, stefano.stabellini, fu.wei

>>> On 24.09.14 at 07:03, <roy.franz@linaro.org> wrote:
> The only valid response from the LocateHandle() call is EFI_BUFFER_TOO_SMALL,
> so exit if we get anything else.  We pass a 0 size/NULL pointer buffer, so the
> only other returns we will get is an error.  Return right away as there is
> nothing to do.  Also return if there is an error allocating the buffer, as the
> previous code path also allowed for an undefined pointer to be freed.
> 
> Signed-off-by: Roy Franz <roy.franz@linaro.org>

Thanks, but I restructured the patch (see below). Additionally such
bug fixes would better be placed at the start of a series to ease
backporting.

Jan

x86/EFI: fix freeing of uninitialized pointer

The only valid response from the LocateHandle() call is EFI_BUFFER_TOO_SMALL,
so exit if we get anything else.  We pass a 0 size/NULL pointer buffer, so the
only other returns we will get is an error.  Return right away as there is
nothing to do.  Also return if there is an error allocating the buffer, as the
previous code path also allowed for an undefined pointer to be freed.

Signed-off-by: Roy Franz <roy.franz@linaro.org>

Re-structure the change.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/efi/boot.c
+++ b/xen/arch/x86/efi/boot.c
@@ -595,11 +595,12 @@ static void __init setup_efi_pci(void)
     struct efi_pci_rom *last = NULL;
 
     status = efi_bs->LocateHandle(ByProtocol, &pci_guid, NULL, &size, NULL);
-    if ( status == EFI_BUFFER_TOO_SMALL )
-        status = efi_bs->AllocatePool(EfiLoaderData, size, (void **)&handles);
-    if ( !EFI_ERROR(status) )
-        status = efi_bs->LocateHandle(ByProtocol, &pci_guid, NULL, &size,
-                                      handles);
+    if ( status != EFI_BUFFER_TOO_SMALL )
+        return;
+    status = efi_bs->AllocatePool(EfiLoaderData, size, (void **)&handles);
+    if ( EFI_ERROR(status) )
+        return;
+    status = efi_bs->LocateHandle(ByProtocol, &pci_guid, NULL, &size, handles);
     if ( EFI_ERROR(status) )
         size = 0;

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH for-4.5 V6 01/14] move x86 EFI boot/runtime code to common/efi
  2014-09-24  5:02 ` [PATCH for-4.5 V6 01/14] move x86 EFI boot/runtime code to common/efi Roy Franz
@ 2014-09-24 15:50   ` Jan Beulich
  2014-09-24 19:41     ` Roy Franz
  0 siblings, 1 reply; 30+ messages in thread
From: Jan Beulich @ 2014-09-24 15:50 UTC (permalink / raw)
  To: Roy Franz; +Cc: keir, ian.campbell, tim, xen-devel, stefano.stabellini, fu.wei

>>> On 24.09.14 at 07:02, <roy.franz@linaro.org> wrote:
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -100,6 +100,10 @@ $(TARGET): delete-unfresh-files
>  	$(MAKE) -C tools
>  	$(MAKE) -f $(BASEDIR)/Rules.mk include/xen/compile.h
>  	[ -e include/asm ] || ln -sf asm-$(TARGET_ARCH) include/asm
> +	[ -e arch/$(TARGET_ARCH)/efi ] && ln -nsf ../../../common/efi/boot.c arch/$(TARGET_ARCH)/efi/;\
> +			ln -nsf ../../../common/efi/runtime.c arch/$(TARGET_ARCH)/efi/;\
> +			ln -nsf ../../../common/efi/compat.c arch/$(TARGET_ARCH)/efi/;\
> +			ln -nsf ../../../common/efi/efi.h arch/$(TARGET_ARCH)/efi/;

I don't think this does what you want: The && applies only to the first
ln invocation, the others will be done unconditionally. And this would
be better done with "for f in boot.c runtime.c ..." anyway, at which
point the issue magically disappears.

Furthermore (I'm sorry for not paying attention to the before) you
shouldn't use && in make rules without some extra care: If the left
side evaluates to false, the whole compound command (and hence
the make rule) will fail. Either use || or invoke /bin/true as last
operation.

Jan

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH for-4.5 V6 02/14] Move x86 specific funtions/variables to arch header
  2014-09-24  5:03 ` [PATCH for-4.5 V6 02/14] Move x86 specific funtions/variables to arch header Roy Franz
@ 2014-09-24 15:56   ` Jan Beulich
  2014-09-24 19:45     ` Roy Franz
  0 siblings, 1 reply; 30+ messages in thread
From: Jan Beulich @ 2014-09-24 15:56 UTC (permalink / raw)
  To: Roy Franz; +Cc: keir, ian.campbell, tim, xen-devel, stefano.stabellini, fu.wei

>>> On 24.09.14 at 07:03, <roy.franz@linaro.org> wrote:
> --- /dev/null
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -0,0 +1,132 @@
> +/*
> + * Architecture specific implementation for EFI boot code.  This file
> + * is intended to be included by common/efi/boot.c _only_, and
> + * therefore can define arch specific global variables.
> + */
> +#include <asm/e820.h>
> +#include <asm/edd.h>
> +#define __ASSEMBLY__ /* avoid pulling in ACPI stuff (conflicts with EFI) */
> +#include <asm/fixmap.h>
> +#undef __ASSEMBLY__

Just noticed this: Moving this here makes things even more fragile
than they are now: If asm/fixmap.h happens to be included by any
other header processed prior to reaching this point, the whole effect
will be gone. Please keep this in the original place (with an #ifdef
and comment as to why).

With that adjusted
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH for-4.5 V6 03/14] create arch functions to allocate memory for and process EFI memory map.
  2014-09-24  5:03 ` [PATCH for-4.5 V6 03/14] create arch functions to allocate memory for and process EFI memory map Roy Franz
@ 2014-09-24 15:57   ` Jan Beulich
  0 siblings, 0 replies; 30+ messages in thread
From: Jan Beulich @ 2014-09-24 15:57 UTC (permalink / raw)
  To: Roy Franz; +Cc: keir, ian.campbell, tim, xen-devel, stefano.stabellini, fu.wei

>>> On 24.09.14 at 07:03, <roy.franz@linaro.org> wrote:
> The memory used to store the EFI memory map is allocated in an architecture
> specific way, and the processing of the memory map itself uses x86 specific
> data structures. This patch adds architecture specific funtions so each
> architecture can provide its own implementation.
> 
> Signed-off-by: Roy Franz <roy.franz@linaro.org>

Acked-by: Jan Beulich <jbeulich@suse.com>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH for-4.5 V6 10/14] Add arch specific module handling to read_file()
  2014-09-24  5:03 ` [PATCH for-4.5 V6 10/14] Add arch specific module handling to read_file() Roy Franz
@ 2014-09-24 16:02   ` Jan Beulich
  0 siblings, 0 replies; 30+ messages in thread
From: Jan Beulich @ 2014-09-24 16:02 UTC (permalink / raw)
  To: Roy Franz; +Cc: keir, ian.campbell, tim, xen-devel, stefano.stabellini, fu.wei

>>> On 24.09.14 at 07:03, <roy.franz@linaro.org> wrote:
> +static void __init efi_arch_handle_module(struct file *file, const CHAR16 *name,
> +                                          char *options)
> +{
> +    union string local_name;
> +    void *ptr;
> +
> +    /*
> +     * Make a copy, as conversion is destructive, and caller still wants
> +     * wide string available after this call returns.
> +     */
> +    if ( efi_bs->AllocatePool(EfiLoaderData, (wstrlen(name) + 1) * sizeof(*name),
> +                              &ptr) != EFI_SUCCESS )
> +        blexit(L"ERROR Unable to allocate string buffer");

As I keep seeing this: Please drop these "ERROR " prefixes from
blexit() messages - their text already makes clear that this is an
error (and even more so together with the module returning to
whatever component invoked it).

> +
> +    local_name.w = ptr;
> +    wstrcpy(local_name.w, name);
> +    w2s(&local_name);
> +
> +    place_string(&mb_modules[mbi.mods_count].string, options);
> +    place_string(&mb_modules[mbi.mods_count].string, "");

What is this good for btw? I don't see anything similar being done in
the original code.

Jan

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH for-4.5 V6 14/14] Add ARM EFI boot support
  2014-09-24  5:03 ` [PATCH for-4.5 V6 14/14] Add ARM EFI boot support Roy Franz
@ 2014-09-24 16:11   ` Julien Grall
  2014-09-24 19:53     ` Roy Franz
  0 siblings, 1 reply; 30+ messages in thread
From: Julien Grall @ 2014-09-24 16:11 UTC (permalink / raw)
  To: Roy Franz, xen-devel, ian.campbell, stefano.stabellini, tim,
	jbeulich, keir
  Cc: fu.wei

Hi Roy,

On 09/24/2014 06:03 AM, Roy Franz wrote:
> +static int __init fdt_set_reg(void *fdt, int node, int addr_cells,
> +                              int size_cells, uint64_t addr, uint64_t len)
> +{
> +    uint8_t data[16]; /* at most 2 64 bit words */
> +    void *p = data;
> +
> +    /* Make sure that the values provided can be represented in
> +     * the reg property.
> +     */
> +    if ( addr_cells == 1 && (addr >> 32) )
> +        return -1;
> +    if ( size_cells == 1 && (len >> 32) )
> +        return -1;
> +
> +    if ( addr_cells == 1 )
> +    {
> +        *(uint32_t *)p = cpu_to_fdt32(addr);
> +        p += sizeof(uint32_t);
> +    }
> +    else if ( addr_cells == 2 )
> +    {
> +        *(uint64_t *)p = cpu_to_fdt64(addr);
> +        p += sizeof(uint64_t);
> +    }
> +    else
> +        return -1;
> +
> +    if ( size_cells == 1 )
> +    {
> +        *(uint32_t *)p = cpu_to_fdt32(len);
> +        p += sizeof(uint32_t);
> +    }
> +    else if ( size_cells == 2 )
> +    {
> +        *(uint64_t *)p = cpu_to_fdt64(len);
> +        p += sizeof(uint64_t);
> +    }
> +    else
> +        return -1;

You could give a look to dt_set_cell (xen/common/device_tree.c). I think
it will fit to your usage and avoid open coding size in this function.

> diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
> index 166852d..ac9deae 100644
> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -4,17 +4,21 @@
>  #include <xen/guest_access.h>
>  #include <xen/irq.h>
>  #include <xen/time.h>
> -#include <asm/mc146818rtc.h>

NIT: Missing a blank line here.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH for-4.5 V6 13/14] Fix freeing of uninitialized pointer
  2014-09-24  9:07   ` Jan Beulich
@ 2014-09-24 19:22     ` Roy Franz
  2014-09-25  8:13       ` Jan Beulich
  0 siblings, 1 reply; 30+ messages in thread
From: Roy Franz @ 2014-09-24 19:22 UTC (permalink / raw)
  To: Jan Beulich
  Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei

On Wed, Sep 24, 2014 at 2:07 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 24.09.14 at 07:03, <roy.franz@linaro.org> wrote:
>> The only valid response from the LocateHandle() call is EFI_BUFFER_TOO_SMALL,
>> so exit if we get anything else.  We pass a 0 size/NULL pointer buffer, so the
>> only other returns we will get is an error.  Return right away as there is
>> nothing to do.  Also return if there is an error allocating the buffer, as the
>> previous code path also allowed for an undefined pointer to be freed.
>>
>> Signed-off-by: Roy Franz <roy.franz@linaro.org>
>
> Thanks, but I restructured the patch (see below). Additionally such
> bug fixes would better be placed at the start of a series to ease
> backporting.
>
> Jan
>
> x86/EFI: fix freeing of uninitialized pointer
>
> The only valid response from the LocateHandle() call is EFI_BUFFER_TOO_SMALL,
> so exit if we get anything else.  We pass a 0 size/NULL pointer buffer, so the
> only other returns we will get is an error.  Return right away as there is
> nothing to do.  Also return if there is an error allocating the buffer, as the
> previous code path also allowed for an undefined pointer to be freed.
>
> Signed-off-by: Roy Franz <roy.franz@linaro.org>
>
> Re-structure the change.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/efi/boot.c
> +++ b/xen/arch/x86/efi/boot.c
> @@ -595,11 +595,12 @@ static void __init setup_efi_pci(void)
>      struct efi_pci_rom *last = NULL;
>
>      status = efi_bs->LocateHandle(ByProtocol, &pci_guid, NULL, &size, NULL);
> -    if ( status == EFI_BUFFER_TOO_SMALL )
> -        status = efi_bs->AllocatePool(EfiLoaderData, size, (void **)&handles);
> -    if ( !EFI_ERROR(status) )
> -        status = efi_bs->LocateHandle(ByProtocol, &pci_guid, NULL, &size,
> -                                      handles);
> +    if ( status != EFI_BUFFER_TOO_SMALL )
> +        return;
> +    status = efi_bs->AllocatePool(EfiLoaderData, size, (void **)&handles);
> +    if ( EFI_ERROR(status) )
> +        return;
> +    status = efi_bs->LocateHandle(ByProtocol, &pci_guid, NULL, &size, handles);
>      if ( EFI_ERROR(status) )
>          size = 0;
>
>
>
>

OK, I'll use your version, and move it to the start of the patch series.

Roy

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH for-4.5 V6 00/14] arm64 EFI stub
  2014-09-24  8:55 ` [PATCH for-4.5 V6 00/14] arm64 EFI stub Jan Beulich
@ 2014-09-24 19:37   ` Roy Franz
  0 siblings, 0 replies; 30+ messages in thread
From: Roy Franz @ 2014-09-24 19:37 UTC (permalink / raw)
  To: Jan Beulich
  Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei

On Wed, Sep 24, 2014 at 1:55 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 24.09.14 at 07:02, <roy.franz@linaro.org> wrote:
>> Open feedback:
>> * moving extern declaration of efi_l4_pgtable.  I have left this x86 specific
>>   declaration in the x86 specific efi-boot.h file, as I don't see a clearly
>>   better place to put it.
>
> Since runtime.c (hopefully) doesn't include efi-boot.h, this is certainly
> not a viable option - files defining variables should include the headers
> declaring them.
>
> Jan
>

I'll move it and add a #ifdef

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH for-4.5 V6 01/14] move x86 EFI boot/runtime code to common/efi
  2014-09-24 15:50   ` Jan Beulich
@ 2014-09-24 19:41     ` Roy Franz
  0 siblings, 0 replies; 30+ messages in thread
From: Roy Franz @ 2014-09-24 19:41 UTC (permalink / raw)
  To: Jan Beulich
  Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei

On Wed, Sep 24, 2014 at 8:50 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 24.09.14 at 07:02, <roy.franz@linaro.org> wrote:
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -100,6 +100,10 @@ $(TARGET): delete-unfresh-files
>>       $(MAKE) -C tools
>>       $(MAKE) -f $(BASEDIR)/Rules.mk include/xen/compile.h
>>       [ -e include/asm ] || ln -sf asm-$(TARGET_ARCH) include/asm
>> +     [ -e arch/$(TARGET_ARCH)/efi ] && ln -nsf ../../../common/efi/boot.c arch/$(TARGET_ARCH)/efi/;\
>> +                     ln -nsf ../../../common/efi/runtime.c arch/$(TARGET_ARCH)/efi/;\
>> +                     ln -nsf ../../../common/efi/compat.c arch/$(TARGET_ARCH)/efi/;\
>> +                     ln -nsf ../../../common/efi/efi.h arch/$(TARGET_ARCH)/efi/;
>
> I don't think this does what you want: The && applies only to the first
> ln invocation, the others will be done unconditionally. And this would
> be better done with "for f in boot.c runtime.c ..." anyway, at which
> point the issue magically disappears.
>
> Furthermore (I'm sorry for not paying attention to the before) you
> shouldn't use && in make rules without some extra care: If the left
> side evaluates to false, the whole compound command (and hence
> the make rule) will fail. Either use || or invoke /bin/true as last
> operation.
>
> Jan
>

I had overlooked that effect of the failure -  I'll redo this as a for loop.

thanks,
Roy

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH for-4.5 V6 02/14] Move x86 specific funtions/variables to arch header
  2014-09-24 15:56   ` Jan Beulich
@ 2014-09-24 19:45     ` Roy Franz
  2014-09-25  8:12       ` Jan Beulich
  0 siblings, 1 reply; 30+ messages in thread
From: Roy Franz @ 2014-09-24 19:45 UTC (permalink / raw)
  To: Jan Beulich
  Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei

On Wed, Sep 24, 2014 at 8:56 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 24.09.14 at 07:03, <roy.franz@linaro.org> wrote:
>> --- /dev/null
>> +++ b/xen/arch/x86/efi/efi-boot.h
>> @@ -0,0 +1,132 @@
>> +/*
>> + * Architecture specific implementation for EFI boot code.  This file
>> + * is intended to be included by common/efi/boot.c _only_, and
>> + * therefore can define arch specific global variables.
>> + */
>> +#include <asm/e820.h>
>> +#include <asm/edd.h>
>> +#define __ASSEMBLY__ /* avoid pulling in ACPI stuff (conflicts with EFI) */
>> +#include <asm/fixmap.h>
>> +#undef __ASSEMBLY__
>
> Just noticed this: Moving this here makes things even more fragile
> than they are now: If asm/fixmap.h happens to be included by any
> other header processed prior to reaching this point, the whole effect
> will be gone. Please keep this in the original place (with an #ifdef
> and comment as to why).
>
> With that adjusted
> Acked-by: Jan Beulich <jbeulich@suse.com>
>
> Jan
>

OK, I'll move that.  Just to clarify, it's just the

>> +#define __ASSEMBLY__ /* avoid pulling in ACPI stuff (conflicts with EFI) */
>> +#include <asm/fixmap.h>
>> +#undef __ASSEMBLY__

that should be moved.

Roy

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH for-4.5 V6 14/14] Add ARM EFI boot support
  2014-09-24 16:11   ` Julien Grall
@ 2014-09-24 19:53     ` Roy Franz
  0 siblings, 0 replies; 30+ messages in thread
From: Roy Franz @ 2014-09-24 19:53 UTC (permalink / raw)
  To: Julien Grall
  Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini,
	Jan Beulich, Fu Wei

On Wed, Sep 24, 2014 at 9:11 AM, Julien Grall <julien.grall@linaro.org> wrote:
> Hi Roy,
>
> On 09/24/2014 06:03 AM, Roy Franz wrote:
>> +static int __init fdt_set_reg(void *fdt, int node, int addr_cells,
>> +                              int size_cells, uint64_t addr, uint64_t len)
>> +{
>> +    uint8_t data[16]; /* at most 2 64 bit words */
>> +    void *p = data;
>> +
>> +    /* Make sure that the values provided can be represented in
>> +     * the reg property.
>> +     */
>> +    if ( addr_cells == 1 && (addr >> 32) )
>> +        return -1;
>> +    if ( size_cells == 1 && (len >> 32) )
>> +        return -1;
>> +
>> +    if ( addr_cells == 1 )
>> +    {
>> +        *(uint32_t *)p = cpu_to_fdt32(addr);
>> +        p += sizeof(uint32_t);
>> +    }
>> +    else if ( addr_cells == 2 )
>> +    {
>> +        *(uint64_t *)p = cpu_to_fdt64(addr);
>> +        p += sizeof(uint64_t);
>> +    }
>> +    else
>> +        return -1;
>> +
>> +    if ( size_cells == 1 )
>> +    {
>> +        *(uint32_t *)p = cpu_to_fdt32(len);
>> +        p += sizeof(uint32_t);
>> +    }
>> +    else if ( size_cells == 2 )
>> +    {
>> +        *(uint64_t *)p = cpu_to_fdt64(len);
>> +        p += sizeof(uint64_t);
>> +    }
>> +    else
>> +        return -1;
>
> You could give a look to dt_set_cell (xen/common/device_tree.c). I think
> it will fit to your usage and avoid open coding size in this function.

I think this will work - I'll take a closer look to confirm.  This
would clean this code
up a fair bit.

>
>> diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
>> index 166852d..ac9deae 100644
>> --- a/xen/common/efi/runtime.c
>> +++ b/xen/common/efi/runtime.c
>> @@ -4,17 +4,21 @@
>>  #include <xen/guest_access.h>
>>  #include <xen/irq.h>
>>  #include <xen/time.h>
>> -#include <asm/mc146818rtc.h>
>
> NIT: Missing a blank line here.

I'll fix this.

Thanks,
Roy

>
> Regards,
>
> --
> Julien Grall

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH for-4.5 V6 02/14] Move x86 specific funtions/variables to arch header
  2014-09-24 19:45     ` Roy Franz
@ 2014-09-25  8:12       ` Jan Beulich
  0 siblings, 0 replies; 30+ messages in thread
From: Jan Beulich @ 2014-09-25  8:12 UTC (permalink / raw)
  To: Roy Franz; +Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei

>>> On 24.09.14 at 21:45, <roy.franz@linaro.org> wrote:
> On Wed, Sep 24, 2014 at 8:56 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 24.09.14 at 07:03, <roy.franz@linaro.org> wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/x86/efi/efi-boot.h
>>> @@ -0,0 +1,132 @@
>>> +/*
>>> + * Architecture specific implementation for EFI boot code.  This file
>>> + * is intended to be included by common/efi/boot.c _only_, and
>>> + * therefore can define arch specific global variables.
>>> + */
>>> +#include <asm/e820.h>
>>> +#include <asm/edd.h>
>>> +#define __ASSEMBLY__ /* avoid pulling in ACPI stuff (conflicts with EFI) */
>>> +#include <asm/fixmap.h>
>>> +#undef __ASSEMBLY__
>>
>> Just noticed this: Moving this here makes things even more fragile
>> than they are now: If asm/fixmap.h happens to be included by any
>> other header processed prior to reaching this point, the whole effect
>> will be gone. Please keep this in the original place (with an #ifdef
>> and comment as to why).
>>
>> With that adjusted
>> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> OK, I'll move that.  Just to clarify, it's just the
> 
>>> +#define __ASSEMBLY__ /* avoid pulling in ACPI stuff (conflicts with EFI) */
>>> +#include <asm/fixmap.h>
>>> +#undef __ASSEMBLY__
> 
> that should be moved.

Yes.

Jan

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH for-4.5 V6 13/14] Fix freeing of uninitialized pointer
  2014-09-24 19:22     ` Roy Franz
@ 2014-09-25  8:13       ` Jan Beulich
  2014-09-25 15:46         ` Roy Franz
  0 siblings, 1 reply; 30+ messages in thread
From: Jan Beulich @ 2014-09-25  8:13 UTC (permalink / raw)
  To: Roy Franz; +Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei

>>> On 24.09.14 at 21:22, <roy.franz@linaro.org> wrote:
> On Wed, Sep 24, 2014 at 2:07 AM, Jan Beulich <JBeulich@suse.com> wrote:
>> --- a/xen/arch/x86/efi/boot.c
>> +++ b/xen/arch/x86/efi/boot.c
>> @@ -595,11 +595,12 @@ static void __init setup_efi_pci(void)
>>      struct efi_pci_rom *last = NULL;
>>
>>      status = efi_bs->LocateHandle(ByProtocol, &pci_guid, NULL, &size, NULL);
>> -    if ( status == EFI_BUFFER_TOO_SMALL )
>> -        status = efi_bs->AllocatePool(EfiLoaderData, size, (void **)&handles);
>> -    if ( !EFI_ERROR(status) )
>> -        status = efi_bs->LocateHandle(ByProtocol, &pci_guid, NULL, &size,
>> -                                      handles);
>> +    if ( status != EFI_BUFFER_TOO_SMALL )
>> +        return;
>> +    status = efi_bs->AllocatePool(EfiLoaderData, size, (void **)&handles);
>> +    if ( EFI_ERROR(status) )
>> +        return;
>> +    status = efi_bs->LocateHandle(ByProtocol, &pci_guid, NULL, &size, handles);
>>      if ( EFI_ERROR(status) )
>>          size = 0;
>>
> OK, I'll use your version, and move it to the start of the patch series.

Did you overlook that I committed it already?

Jan

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH for-4.5 V6 13/14] Fix freeing of uninitialized pointer
  2014-09-25  8:13       ` Jan Beulich
@ 2014-09-25 15:46         ` Roy Franz
  0 siblings, 0 replies; 30+ messages in thread
From: Roy Franz @ 2014-09-25 15:46 UTC (permalink / raw)
  To: Jan Beulich
  Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei

On Thu, Sep 25, 2014 at 1:13 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 24.09.14 at 21:22, <roy.franz@linaro.org> wrote:
>> On Wed, Sep 24, 2014 at 2:07 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>> --- a/xen/arch/x86/efi/boot.c
>>> +++ b/xen/arch/x86/efi/boot.c
>>> @@ -595,11 +595,12 @@ static void __init setup_efi_pci(void)
>>>      struct efi_pci_rom *last = NULL;
>>>
>>>      status = efi_bs->LocateHandle(ByProtocol, &pci_guid, NULL, &size, NULL);
>>> -    if ( status == EFI_BUFFER_TOO_SMALL )
>>> -        status = efi_bs->AllocatePool(EfiLoaderData, size, (void **)&handles);
>>> -    if ( !EFI_ERROR(status) )
>>> -        status = efi_bs->LocateHandle(ByProtocol, &pci_guid, NULL, &size,
>>> -                                      handles);
>>> +    if ( status != EFI_BUFFER_TOO_SMALL )
>>> +        return;
>>> +    status = efi_bs->AllocatePool(EfiLoaderData, size, (void **)&handles);
>>> +    if ( EFI_ERROR(status) )
>>> +        return;
>>> +    status = efi_bs->LocateHandle(ByProtocol, &pci_guid, NULL, &size, handles);
>>>      if ( EFI_ERROR(status) )
>>>          size = 0;
>>>
>> OK, I'll use your version, and move it to the start of the patch series.
>
> Did you overlook that I committed it already?
>
> Jan
>
I sure did..

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2014-09-25 15:46 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-24  5:02 [PATCH for-4.5 V6 00/14] arm64 EFI stub Roy Franz
2014-09-24  5:02 ` [PATCH for-4.5 V6 01/14] move x86 EFI boot/runtime code to common/efi Roy Franz
2014-09-24 15:50   ` Jan Beulich
2014-09-24 19:41     ` Roy Franz
2014-09-24  5:03 ` [PATCH for-4.5 V6 02/14] Move x86 specific funtions/variables to arch header Roy Franz
2014-09-24 15:56   ` Jan Beulich
2014-09-24 19:45     ` Roy Franz
2014-09-25  8:12       ` Jan Beulich
2014-09-24  5:03 ` [PATCH for-4.5 V6 03/14] create arch functions to allocate memory for and process EFI memory map Roy Franz
2014-09-24 15:57   ` Jan Beulich
2014-09-24  5:03 ` [PATCH for-4.5 V6 04/14] Add architecture functions for pre/post ExitBootServices Roy Franz
2014-09-24  5:03 ` [PATCH for-4.5 V6 05/14] Add efi_arch_cfg_file_early/late() to handle arch specific cfg file fields Roy Franz
2014-09-24  5:03 ` [PATCH for-4.5 V6 06/14] Add efi_arch_handle_cmdline() for processing commandline Roy Franz
2014-09-24  5:03 ` [PATCH for-4.5 V6 07/14] Move x86 specific disk probing code Roy Franz
2014-09-24  5:03 ` [PATCH for-4.5 V6 08/14] Create arch functions for console and video init Roy Franz
2014-09-24  5:03 ` [PATCH for-4.5 V6 09/14] Add efi_arch_memory() for arch specific memory setup Roy Franz
2014-09-24  5:03 ` [PATCH for-4.5 V6 10/14] Add arch specific module handling to read_file() Roy Franz
2014-09-24 16:02   ` Jan Beulich
2014-09-24  5:03 ` [PATCH for-4.5 V6 11/14] Add several misc. arch functions for EFI boot code Roy Franz
2014-09-24  5:03 ` [PATCH for-4.5 V6 12/14] Add efi_arch_use_config_file() function to control use of config file Roy Franz
2014-09-24  5:03 ` [PATCH for-4.5 V6 13/14] Fix freeing of uninitialized pointer Roy Franz
2014-09-24  9:07   ` Jan Beulich
2014-09-24 19:22     ` Roy Franz
2014-09-25  8:13       ` Jan Beulich
2014-09-25 15:46         ` Roy Franz
2014-09-24  5:03 ` [PATCH for-4.5 V6 14/14] Add ARM EFI boot support Roy Franz
2014-09-24 16:11   ` Julien Grall
2014-09-24 19:53     ` Roy Franz
2014-09-24  8:55 ` [PATCH for-4.5 V6 00/14] arm64 EFI stub Jan Beulich
2014-09-24 19:37   ` Roy Franz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).