* [PATCH V5 01/15] move x86 EFI boot/runtime code to common/efi
2014-09-18 22:49 [PATCH V5 00/15] arm64 EFI stub Roy Franz
@ 2014-09-18 22:49 ` Roy Franz
2014-09-19 8:49 ` Jan Beulich
2014-09-18 22:49 ` [PATCH V5 02/15] Move x86 specific funtions/variables to arch header Roy Franz
` (13 subsequent siblings)
14 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-18 22:49 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 | 1 +
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, 5 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 6d725aa..1aa4a5b 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..f182df2 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -100,6 +100,7 @@ $(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 -sf $(BASEDIR)/common/efi/* 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] 63+ messages in thread* Re: [PATCH V5 01/15] move x86 EFI boot/runtime code to common/efi
2014-09-18 22:49 ` [PATCH V5 01/15] move x86 EFI boot/runtime code to common/efi Roy Franz
@ 2014-09-19 8:49 ` Jan Beulich
2014-09-22 10:51 ` Ian Campbell
0 siblings, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2014-09-19 8:49 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, ian.campbell, tim, xen-devel, stefano.stabellini, fu.wei
>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -100,6 +100,7 @@ $(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 -sf $(BASEDIR)/common/efi/* arch/$(TARGET_ARCH)/efi/
I guess this would better be
ln -sf ../../../common/efi/* arch/$(TARGET_ARCH)/efi/
- there's no reason to prevent the tree from being moved.
Also I hope the recurring link creation upon incremental rebuilds
isn't going to cause any kind of problems.
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH V5 01/15] move x86 EFI boot/runtime code to common/efi
2014-09-19 8:49 ` Jan Beulich
@ 2014-09-22 10:51 ` Ian Campbell
0 siblings, 0 replies; 63+ messages in thread
From: Ian Campbell @ 2014-09-22 10:51 UTC (permalink / raw)
To: Jan Beulich; +Cc: keir, tim, xen-devel, Roy Franz, stefano.stabellini, fu.wei
On Fri, 2014-09-19 at 09:49 +0100, Jan Beulich wrote:
> >>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -100,6 +100,7 @@ $(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 -sf $(BASEDIR)/common/efi/* arch/$(TARGET_ARCH)/efi/
>
> I guess this would better be
>
> ln -sf ../../../common/efi/* arch/$(TARGET_ARCH)/efi/
>
> - there's no reason to prevent the tree from being moved.
>
> Also I hope the recurring link creation upon incremental rebuilds
> isn't going to cause any kind of problems.
FWIW my finger macros usually use ln -nfs to avoid issues of this type
of thing, although maybe that's only for directory targets, since the -n
stops it creating ever deeper links within the original target.
Ian.
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH V5 02/15] Move x86 specific funtions/variables to arch header
2014-09-18 22:49 [PATCH V5 00/15] arm64 EFI stub Roy Franz
2014-09-18 22:49 ` [PATCH V5 01/15] move x86 EFI boot/runtime code to common/efi Roy Franz
@ 2014-09-18 22:49 ` Roy Franz
2014-09-19 8:37 ` Jan Beulich
2014-09-22 12:54 ` Jan Beulich
2014-09-18 22:49 ` [PATCH V5 03/15] create arch functions to allocate memory for and process EFI memory map Roy Franz
` (12 subsequent siblings)
14 siblings, 2 replies; 63+ messages in thread
From: Roy Franz @ 2014-09-18 22:49 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/common/efi/boot.c | 148 +++++------------------------------------
xen/include/asm-x86/efi-boot.h | 135 +++++++++++++++++++++++++++++++++++++
2 files changed, 150 insertions(+), 133 deletions(-)
create mode 100644 xen/include/asm-x86/efi-boot.h
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 3bdc158..1a6cd7c 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 <asm/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/efi-boot.h b/xen/include/asm-x86/efi-boot.h
new file mode 100644
index 0000000..f250941
--- /dev/null
+++ b/xen/include/asm-x86/efi-boot.h
@@ -0,0 +1,135 @@
+/*
+ * Architecture specific implementation for EFI boot code. This file
+ * is intended to be included by XXX _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];
+
+extern char start[];
+extern u32 cpuid_ext_features;
+
+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;
+}
--
2.1.0
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH V5 02/15] Move x86 specific funtions/variables to arch header
2014-09-18 22:49 ` [PATCH V5 02/15] Move x86 specific funtions/variables to arch header Roy Franz
@ 2014-09-19 8:37 ` Jan Beulich
2014-09-22 10:52 ` Ian Campbell
2014-09-22 12:54 ` Jan Beulich
1 sibling, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2014-09-19 8:37 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, ian.campbell, tim, xen-devel, stefano.stabellini, fu.wei
>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
> @@ -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));
> - }
> -}
Hmm, so you're still moving the relocation processing code - why? I
don't recall you having said you're sure you'll not need this on ARM.
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH V5 02/15] Move x86 specific funtions/variables to arch header
2014-09-19 8:37 ` Jan Beulich
@ 2014-09-22 10:52 ` Ian Campbell
2014-09-22 10:56 ` Jan Beulich
0 siblings, 1 reply; 63+ messages in thread
From: Ian Campbell @ 2014-09-22 10:52 UTC (permalink / raw)
To: Jan Beulich; +Cc: keir, tim, xen-devel, Roy Franz, stefano.stabellini, fu.wei
On Fri, 2014-09-19 at 09:37 +0100, Jan Beulich wrote:
> >>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
> > @@ -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));
> > - }
> > -}
>
> Hmm, so you're still moving the relocation processing code - why? I
> don't recall you having said you're sure you'll not need this on ARM.
ARM relocates itself to the top of memory during bringup already. I
don't think we need to do it here as well/instead.
Ian.
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH V5 02/15] Move x86 specific funtions/variables to arch header
2014-09-22 10:52 ` Ian Campbell
@ 2014-09-22 10:56 ` Jan Beulich
2014-09-22 11:09 ` Ian Campbell
0 siblings, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2014-09-22 10:56 UTC (permalink / raw)
To: Ian Campbell; +Cc: keir, tim, xen-devel, Roy Franz, stefano.stabellini, fu.wei
>>> On 22.09.14 at 12:52, <Ian.Campbell@citrix.com> wrote:
> On Fri, 2014-09-19 at 09:37 +0100, Jan Beulich wrote:
>> >>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
>> > @@ -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));
>> > - }
>> > -}
>>
>> Hmm, so you're still moving the relocation processing code - why? I
>> don't recall you having said you're sure you'll not need this on ARM.
>
> ARM relocates itself to the top of memory during bringup already. I
> don't think we need to do it here as well/instead.
There are two relocations (on x86) actually: One (always used) is
to move the hypervisor image to high physical memory. The other
(needed under UEFI only) is to relocate the hypervisor from where
the UEFI loader put it (running in physical mode, virtual and
physical addresses are the same) to its linked virtual address. Are
you saying that ARM isn't in need of this?
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH V5 02/15] Move x86 specific funtions/variables to arch header
2014-09-22 10:56 ` Jan Beulich
@ 2014-09-22 11:09 ` Ian Campbell
2014-09-22 11:31 ` Jan Beulich
0 siblings, 1 reply; 63+ messages in thread
From: Ian Campbell @ 2014-09-22 11:09 UTC (permalink / raw)
To: Jan Beulich; +Cc: keir, tim, xen-devel, Roy Franz, stefano.stabellini, fu.wei
On Mon, 2014-09-22 at 11:56 +0100, Jan Beulich wrote:
> >>> On 22.09.14 at 12:52, <Ian.Campbell@citrix.com> wrote:
> > On Fri, 2014-09-19 at 09:37 +0100, Jan Beulich wrote:
> >> Hmm, so you're still moving the relocation processing code - why? I
> >> don't recall you having said you're sure you'll not need this on ARM.
> >
> > ARM relocates itself to the top of memory during bringup already. I
> > don't think we need to do it here as well/instead.
>
> There are two relocations (on x86) actually: One (always used) is
> to move the hypervisor image to high physical memory. The other
> (needed under UEFI only) is to relocate the hypervisor from where
> the UEFI loader put it (running in physical mode, virtual and
> physical addresses are the same) to its linked virtual address. Are
> you saying that ARM isn't in need of this?
Correct, the main ARM entry point (which the UEFI stub calls) takes care
of enabling paging, including handling the move from (potentially
differing) physical to virtual addresses.
Ian.
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH V5 02/15] Move x86 specific funtions/variables to arch header
2014-09-22 11:09 ` Ian Campbell
@ 2014-09-22 11:31 ` Jan Beulich
0 siblings, 0 replies; 63+ messages in thread
From: Jan Beulich @ 2014-09-22 11:31 UTC (permalink / raw)
To: Ian Campbell, Roy Franz; +Cc: keir, tim, stefano.stabellini, fu.wei, xen-devel
>>> On 22.09.14 at 13:09, <Ian.Campbell@citrix.com> wrote:
> On Mon, 2014-09-22 at 11:56 +0100, Jan Beulich wrote:
>> >>> On 22.09.14 at 12:52, <Ian.Campbell@citrix.com> wrote:
>> > On Fri, 2014-09-19 at 09:37 +0100, Jan Beulich wrote:
>> >> Hmm, so you're still moving the relocation processing code - why? I
>> >> don't recall you having said you're sure you'll not need this on ARM.
>> >
>> > ARM relocates itself to the top of memory during bringup already. I
>> > don't think we need to do it here as well/instead.
>>
>> There are two relocations (on x86) actually: One (always used) is
>> to move the hypervisor image to high physical memory. The other
>> (needed under UEFI only) is to relocate the hypervisor from where
>> the UEFI loader put it (running in physical mode, virtual and
>> physical addresses are the same) to its linked virtual address. Are
>> you saying that ARM isn't in need of this?
>
> Correct, the main ARM entry point (which the UEFI stub calls) takes care
> of enabling paging, including handling the move from (potentially
> differing) physical to virtual addresses.
Ah, right - this really makes the need for doing the extra relocation
step x86-64-specific. So Roy, I withdraw my objection then to moving
these pieces into arch-specific code.
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH V5 02/15] Move x86 specific funtions/variables to arch header
2014-09-18 22:49 ` [PATCH V5 02/15] Move x86 specific funtions/variables to arch header Roy Franz
2014-09-19 8:37 ` Jan Beulich
@ 2014-09-22 12:54 ` Jan Beulich
2014-09-23 2:08 ` Roy Franz
1 sibling, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2014-09-22 12:54 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, ian.campbell, tim, xen-devel, stefano.stabellini, fu.wei
>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
> --- /dev/null
> +++ b/xen/include/asm-x86/efi-boot.h
> @@ -0,0 +1,135 @@
> +/*
> + * Architecture specific implementation for EFI boot code. This file
> + * is intended to be included by XXX _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];
> +
> +extern char start[];
> +extern u32 cpuid_ext_features;
I don't think these (and other extern-s) are valid to be put here.
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH V5 02/15] Move x86 specific funtions/variables to arch header
2014-09-22 12:54 ` Jan Beulich
@ 2014-09-23 2:08 ` Roy Franz
2014-09-23 12:24 ` Jan Beulich
0 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-23 2:08 UTC (permalink / raw)
To: Jan Beulich
Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei
On Mon, Sep 22, 2014 at 5:54 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
>> --- /dev/null
>> +++ b/xen/include/asm-x86/efi-boot.h
>> @@ -0,0 +1,135 @@
>> +/*
>> + * Architecture specific implementation for EFI boot code. This file
>> + * is intended to be included by XXX _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];
>> +
>> +extern char start[];
>> +extern u32 cpuid_ext_features;
>
> I don't think these (and other extern-s) are valid to be put here.
>
> Jan
>
Why not, and where should they go??
cpuid_ext_features is x86 architecture specific, and start[] is only
used by the x86 code by the place_string() allocator. The extern
declaration is in the file in which the variables are referenced.
extern l4_pgentry_t *efi_l4_pgtable;
maybe should be moved back to boot.c for now, but this is an x86
specific structure that is only referenced there due to the
runtime code being #ifdef'ed out.
Roy
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH V5 02/15] Move x86 specific funtions/variables to arch header
2014-09-23 2:08 ` Roy Franz
@ 2014-09-23 12:24 ` Jan Beulich
2014-09-24 2:35 ` Roy Franz
0 siblings, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2014-09-23 12:24 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei
>>> On 23.09.14 at 04:08, <roy.franz@linaro.org> wrote:
> On Mon, Sep 22, 2014 at 5:54 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
>>> --- /dev/null
>>> +++ b/xen/include/asm-x86/efi-boot.h
>>> @@ -0,0 +1,135 @@
>>> +/*
>>> + * Architecture specific implementation for EFI boot code. This file
>>> + * is intended to be included by XXX _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];
>>> +
>>> +extern char start[];
>>> +extern u32 cpuid_ext_features;
>>
>> I don't think these (and other extern-s) are valid to be put here.
>
> Why not, and where should they go??
>
> cpuid_ext_features is x86 architecture specific, and start[] is only
> used by the x86 code by the place_string() allocator. The extern
> declaration is in the file in which the variables are referenced.
I'm sorry, I only now realized they're both currently being declared
in boot.c, so moving them here is kind of okay. Nevertheless it
would seem better to find them a more appropriate home: start[]
could likely go alongside _start[] in xen/kernel.h, and
cpuid_ext_features would seem to fit into either
asm-x86/cpufeature.h or asm-x86/processor.h.
> extern l4_pgentry_t *efi_l4_pgtable;
> maybe should be moved back to boot.c for now, but this is an x86
> specific structure that is only referenced there due to the
> runtime code being #ifdef'ed out.
It's currently being declared in xen/arch/x86/efi/efi.h, and I
can't really see why this isn't sufficient.
Speaking of which - putting the new header under asm-x86/ instead
of in x86/efi/ seems bogus with the changed symlinking model too.
The header should be as private as possible. Perhaps what you put
there could even go into said efi.h (possibly inside a suitable #if)?
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH V5 02/15] Move x86 specific funtions/variables to arch header
2014-09-23 12:24 ` Jan Beulich
@ 2014-09-24 2:35 ` Roy Franz
2014-09-24 8:12 ` Jan Beulich
0 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-24 2:35 UTC (permalink / raw)
To: Jan Beulich
Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei
On Tue, Sep 23, 2014 at 5:24 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 23.09.14 at 04:08, <roy.franz@linaro.org> wrote:
>> On Mon, Sep 22, 2014 at 5:54 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
>>>> --- /dev/null
>>>> +++ b/xen/include/asm-x86/efi-boot.h
>>>> @@ -0,0 +1,135 @@
>>>> +/*
>>>> + * Architecture specific implementation for EFI boot code. This file
>>>> + * is intended to be included by XXX _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];
>>>> +
>>>> +extern char start[];
>>>> +extern u32 cpuid_ext_features;
>>>
>>> I don't think these (and other extern-s) are valid to be put here.
>>
>> Why not, and where should they go??
>>
>> cpuid_ext_features is x86 architecture specific, and start[] is only
>> used by the x86 code by the place_string() allocator. The extern
>> declaration is in the file in which the variables are referenced.
>
> I'm sorry, I only now realized they're both currently being declared
> in boot.c, so moving them here is kind of okay. Nevertheless it
> would seem better to find them a more appropriate home: start[]
> could likely go alongside _start[] in xen/kernel.h, and
> cpuid_ext_features would seem to fit into either
> asm-x86/cpufeature.h or asm-x86/processor.h.
I moved start to xen/kernel.h, and cpuid_ext_features to asm-x86/processor.h
alongside some other externs. cpufeature.h didn't have any externs, so putting
it with other externs seemed a better fit.
>
>> extern l4_pgentry_t *efi_l4_pgtable;
>> maybe should be moved back to boot.c for now, but this is an x86
>> specific structure that is only referenced there due to the
>> runtime code being #ifdef'ed out.
>
> It's currently being declared in xen/arch/x86/efi/efi.h, and I
> can't really see why this isn't sufficient.
xen/arch/x86/efi/efi.h is a link to common/efi/efi.h, so it is a shared
header. I could put it there with #ifdef, or leave it the the architecture
specific header file.
>
> Speaking of which - putting the new header under asm-x86/ instead
> of in x86/efi/ seems bogus with the changed symlinking model too.
> The header should be as private as possible. Perhaps what you put
> there could even go into said efi.h (possibly inside a suitable #if)?
Yup, putting the efi-boot.h headers in arch/XX/efi is better, so I
have moved them.
Since the efi-boot.h is a 'special' include file - ie it defines
global variables,
I think having it directly included where it is intended is a good thing.
This would also require a lot of forward declarations for functions
and global variables referenced by this implementation header but defined
in efi-boot.c. This is all avoided right now by including efi-boot.h
a bit lower
down.
>
> Jan
>
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH V5 02/15] Move x86 specific funtions/variables to arch header
2014-09-24 2:35 ` Roy Franz
@ 2014-09-24 8:12 ` Jan Beulich
0 siblings, 0 replies; 63+ messages in thread
From: Jan Beulich @ 2014-09-24 8:12 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei
>>> On 24.09.14 at 04:35, <roy.franz@linaro.org> wrote:
> On Tue, Sep 23, 2014 at 5:24 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 23.09.14 at 04:08, <roy.franz@linaro.org> wrote:
>>> extern l4_pgentry_t *efi_l4_pgtable;
>>> maybe should be moved back to boot.c for now, but this is an x86
>>> specific structure that is only referenced there due to the
>>> runtime code being #ifdef'ed out.
>>
>> It's currently being declared in xen/arch/x86/efi/efi.h, and I
>> can't really see why this isn't sufficient.
>
> xen/arch/x86/efi/efi.h is a link to common/efi/efi.h, so it is a shared
> header. I could put it there with #ifdef, or leave it the the architecture
> specific header file.
Following the as-private-as-possible model going the #ifdef route
seems better to me even if resulting in slightly more ugly code.
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH V5 03/15] create arch functions to allocate memory for and process EFI memory map.
2014-09-18 22:49 [PATCH V5 00/15] arm64 EFI stub Roy Franz
2014-09-18 22:49 ` [PATCH V5 01/15] move x86 EFI boot/runtime code to common/efi Roy Franz
2014-09-18 22:49 ` [PATCH V5 02/15] Move x86 specific funtions/variables to arch header Roy Franz
@ 2014-09-18 22:49 ` Roy Franz
2014-09-19 8:47 ` Jan Beulich
2014-09-18 22:49 ` [PATCH V5 04/15] Add architecture functions for pre/post ExitBootServices Roy Franz
` (11 subsequent siblings)
14 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-18 22:49 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/common/efi/boot.c | 79 ++++++++----------------------------------
xen/include/asm-x86/efi-boot.h | 76 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 91 insertions(+), 64 deletions(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 1a6cd7c..307740b 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -61,8 +61,6 @@ static EFI_HANDLE __initdata efi_ih;
static SIMPLE_TEXT_OUTPUT_INTERFACE *__initdata StdOut;
static SIMPLE_TEXT_OUTPUT_INTERFACE *__initdata StdErr;
-static UINT32 __initdata mdesc_ver;
-
static struct file __initdata cfg;
static struct file __initdata kernel;
static struct file __initdata ramdisk;
@@ -657,16 +655,18 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
EFI_STATUS status;
unsigned int i, argc;
CHAR16 **argv, *file_name, *cfg_file_name = NULL;
- UINTN cols, rows, depth, size, map_key, info_size, gop_mode = ~0;
+ UINTN cols, rows, depth, size, info_size, gop_mode = ~0;
EFI_HANDLE *handles = NULL;
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;
- struct e820entry *e;
u64 efer;
bool_t base_video = 0;
+ UINT32 mmap_desc_ver = 0;
+ UINTN mmap_size, mmap_desc_size, mmap_key = 0;
+ void *mmap;
efi_ih = ImageHandle;
efi_bs = SystemTable->BootServices;
@@ -966,8 +966,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);
@@ -1262,67 +1260,20 @@ 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;
- status = efi_bs->GetMemoryMap(&efi_memmap_size, efi_memmap, &map_key,
- &efi_mdesc_size, &mdesc_ver);
+ efi_bs->GetMemoryMap(&mmap_size, NULL, &mmap_key,
+ &mmap_desc_size, &mmap_desc_ver);
+ mmap = efi_arch_allocate_mmap_buffer(mmap_size);
+ if ( !mmap )
+ blexit(L"ERROR Unable to allocate memory for EFI memory map\r\n");
+
+ status = efi_bs->GetMemoryMap(&mmap_size, mmap, &mmap_key,
+ &mmap_desc_size, &mmap_desc_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, mmap, mmap_size,
+ mmap_desc_size, mmap_desc_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 )
@@ -1330,7 +1281,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
relocate_trampoline(cfg.addr);
}
- status = efi_bs->ExitBootServices(ImageHandle, map_key);
+ status = efi_bs->ExitBootServices(ImageHandle, mmap_key);
if ( EFI_ERROR(status) )
PrintErrMesg(L"Cannot exit boot services", status);
diff --git a/xen/include/asm-x86/efi-boot.h b/xen/include/asm-x86/efi-boot.h
index f250941..8832b21 100644
--- a/xen/include/asm-x86/efi-boot.h
+++ b/xen/include/asm-x86/efi-boot.h
@@ -133,3 +133,79 @@ 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;
+
+ /* Set global EFI memory map variables */
+ efi_memmap_size = map_size;
+ efi_mdesc_size = desc_size;
+ efi_memmap = map;
+
+ /* 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;
+}
--
2.1.0
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH V5 03/15] create arch functions to allocate memory for and process EFI memory map.
2014-09-18 22:49 ` [PATCH V5 03/15] create arch functions to allocate memory for and process EFI memory map Roy Franz
@ 2014-09-19 8:47 ` Jan Beulich
2014-09-23 1:14 ` Roy Franz
0 siblings, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2014-09-19 8:47 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, ian.campbell, tim, xen-devel, stefano.stabellini, fu.wei
>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
> @@ -657,16 +655,18 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
> EFI_STATUS status;
> unsigned int i, argc;
> CHAR16 **argv, *file_name, *cfg_file_name = NULL;
> - UINTN cols, rows, depth, size, map_key, info_size, gop_mode = ~0;
> + UINTN cols, rows, depth, size, info_size, gop_mode = ~0;
> EFI_HANDLE *handles = NULL;
> 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;
> - struct e820entry *e;
> u64 efer;
> bool_t base_video = 0;
> + UINT32 mmap_desc_ver = 0;
> + UINTN mmap_size, mmap_desc_size, mmap_key = 0;
Are these initializers really needed?
Also I previously commented about mdesc_ver having been static
rather than automatic for a reason, yet you still don't retain that.
Similarly for efi_memmap_size, efi_mdesc_size, and efi_memmap:
With runtime.c moved and (presumably later in the series) also
built for ARM, there's no point in moving the setting of these into
efi_arch_process_memory_map().
> @@ -1262,67 +1260,20 @@ 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;
> - status = efi_bs->GetMemoryMap(&efi_memmap_size, efi_memmap, &map_key,
> - &efi_mdesc_size, &mdesc_ver);
> + efi_bs->GetMemoryMap(&mmap_size, NULL, &mmap_key,
> + &mmap_desc_size, &mmap_desc_ver);
> + mmap = efi_arch_allocate_mmap_buffer(mmap_size);
> + if ( !mmap )
> + blexit(L"ERROR Unable to allocate memory for EFI memory map\r\n");
blexit() appends a newline itself.
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH V5 03/15] create arch functions to allocate memory for and process EFI memory map.
2014-09-19 8:47 ` Jan Beulich
@ 2014-09-23 1:14 ` Roy Franz
0 siblings, 0 replies; 63+ messages in thread
From: Roy Franz @ 2014-09-23 1:14 UTC (permalink / raw)
To: Jan Beulich
Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei
On Fri, Sep 19, 2014 at 1:47 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
>> @@ -657,16 +655,18 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
>> EFI_STATUS status;
>> unsigned int i, argc;
>> CHAR16 **argv, *file_name, *cfg_file_name = NULL;
>> - UINTN cols, rows, depth, size, map_key, info_size, gop_mode = ~0;
>> + UINTN cols, rows, depth, size, info_size, gop_mode = ~0;
>> EFI_HANDLE *handles = NULL;
>> 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;
>> - struct e820entry *e;
>> u64 efer;
>> bool_t base_video = 0;
>> + UINT32 mmap_desc_ver = 0;
>> + UINTN mmap_size, mmap_desc_size, mmap_key = 0;
>
> Are these initializers really needed?
The base_video is, some of the others aren't in the current form. I will
audit the initialized local variables here.
>
> Also I previously commented about mdesc_ver having been static
> rather than automatic for a reason, yet you still don't retain that.
Sorry, I had missed that bit in the virtual mapping issues. I'll return this
to being static.
>
> Similarly for efi_memmap_size, efi_mdesc_size, and efi_memmap:
> With runtime.c moved and (presumably later in the series) also
> built for ARM, there's no point in moving the setting of these into
> efi_arch_process_memory_map().
Yes, I missed undoing this as part of pulling the in the global
runtime variables. These should
all be able to be reverted.
>
>> @@ -1262,67 +1260,20 @@ 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;
>> - status = efi_bs->GetMemoryMap(&efi_memmap_size, efi_memmap, &map_key,
>> - &efi_mdesc_size, &mdesc_ver);
>> + efi_bs->GetMemoryMap(&mmap_size, NULL, &mmap_key,
>> + &mmap_desc_size, &mmap_desc_ver);
>> + mmap = efi_arch_allocate_mmap_buffer(mmap_size);
>> + if ( !mmap )
>> + blexit(L"ERROR Unable to allocate memory for EFI memory map\r\n");
>
> blexit() appends a newline itself.
I'll fix this and review the other blexit() calls.
>
> Jan
>
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH V5 04/15] Add architecture functions for pre/post ExitBootServices
2014-09-18 22:49 [PATCH V5 00/15] arm64 EFI stub Roy Franz
` (2 preceding siblings ...)
2014-09-18 22:49 ` [PATCH V5 03/15] create arch functions to allocate memory for and process EFI memory map Roy Franz
@ 2014-09-18 22:49 ` Roy Franz
2014-09-22 12:11 ` Jan Beulich
2014-09-18 22:49 ` [PATCH V5 05/15] Add efi_arch_cfg_file_early/late() to handle arch specific cfg file fields Roy Franz
` (10 subsequent siblings)
14 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-18 22:49 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>
---
xen/common/efi/boot.c | 42 ++---------------------------------
xen/include/asm-x86/efi-boot.h | 50 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+), 40 deletions(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 307740b..f4662b4 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -662,7 +662,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;
UINT32 mmap_desc_ver = 0;
UINTN mmap_size, mmap_desc_size, mmap_key = 0;
@@ -1274,12 +1273,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
efi_arch_process_memory_map(SystemTable, mmap, mmap_size,
mmap_desc_size, mmap_desc_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, mmap_key);
if ( EFI_ERROR(status) )
@@ -1293,39 +1287,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 */
}
diff --git a/xen/include/asm-x86/efi-boot.h b/xen/include/asm-x86/efi-boot.h
index 8832b21..7aba173 100644
--- a/xen/include/asm-x86/efi-boot.h
+++ b/xen/include/asm-x86/efi-boot.h
@@ -209,3 +209,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 */
+}
--
2.1.0
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH V5 04/15] Add architecture functions for pre/post ExitBootServices
2014-09-18 22:49 ` [PATCH V5 04/15] Add architecture functions for pre/post ExitBootServices Roy Franz
@ 2014-09-22 12:11 ` Jan Beulich
0 siblings, 0 replies; 63+ messages in thread
From: Jan Beulich @ 2014-09-22 12:11 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, ian.campbell, tim, xen-devel, stefano.stabellini, fu.wei
>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
> 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>
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH V5 05/15] Add efi_arch_cfg_file_early/late() to handle arch specific cfg file fields
2014-09-18 22:49 [PATCH V5 00/15] arm64 EFI stub Roy Franz
` (3 preceding siblings ...)
2014-09-18 22:49 ` [PATCH V5 04/15] Add architecture functions for pre/post ExitBootServices Roy Franz
@ 2014-09-18 22:49 ` Roy Franz
2014-09-22 12:13 ` Jan Beulich
2014-09-18 22:49 ` [PATCH V5 06/15] Add efi_arch_handle_cmdline() for processing commandline Roy Franz
` (9 subsequent siblings)
14 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-18 22:49 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>
---
xen/common/efi/boot.c | 21 ++++++++++-----------
xen/include/asm-x86/efi-boot.h | 20 ++++++++++++++++++++
2 files changed, 30 insertions(+), 11 deletions(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index f4662b4..58e3cbc 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;
@@ -843,6 +849,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);
@@ -860,17 +869,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 )
{
@@ -909,6 +907,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;
diff --git a/xen/include/asm-x86/efi-boot.h b/xen/include/asm-x86/efi-boot.h
index 7aba173..93498bb 100644
--- a/xen/include/asm-x86/efi-boot.h
+++ b/xen/include/asm-x86/efi-boot.h
@@ -259,3 +259,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);
+ }
+}
--
2.1.0
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH V5 05/15] Add efi_arch_cfg_file_early/late() to handle arch specific cfg file fields
2014-09-18 22:49 ` [PATCH V5 05/15] Add efi_arch_cfg_file_early/late() to handle arch specific cfg file fields Roy Franz
@ 2014-09-22 12:13 ` Jan Beulich
0 siblings, 0 replies; 63+ messages in thread
From: Jan Beulich @ 2014-09-22 12:13 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, ian.campbell, tim, xen-devel, stefano.stabellini, fu.wei
>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
> 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>
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH V5 06/15] Add efi_arch_handle_cmdline() for processing commandline
2014-09-18 22:49 [PATCH V5 00/15] arm64 EFI stub Roy Franz
` (4 preceding siblings ...)
2014-09-18 22:49 ` [PATCH V5 05/15] Add efi_arch_cfg_file_early/late() to handle arch specific cfg file fields Roy Franz
@ 2014-09-18 22:49 ` Roy Franz
2014-09-22 12:17 ` Jan Beulich
2014-09-18 22:49 ` [PATCH V5 07/15] Move x86 specific disk probing code Roy Franz
` (8 subsequent siblings)
14 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-18 22:49 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>
---
xen/common/efi/boot.c | 37 +++++++++----------------------------
xen/include/asm-x86/efi-boot.h | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+), 28 deletions(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 58e3cbc..2f8dade 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);
@@ -256,7 +257,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;
@@ -282,10 +284,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
@@ -660,7 +660,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, info_size, gop_mode = ~0;
EFI_HANDLE *handles = NULL;
EFI_SHIM_LOCK_PROTOCOL *shim_lock;
@@ -701,14 +701,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, &options);
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 )
@@ -878,17 +878,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 )
@@ -955,15 +945,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);
diff --git a/xen/include/asm-x86/efi-boot.h b/xen/include/asm-x86/efi-boot.h
index 93498bb..6e9376a 100644
--- a/xen/include/asm-x86/efi-boot.h
+++ b/xen/include/asm-x86/efi-boot.h
@@ -279,3 +279,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;
+}
--
2.1.0
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH V5 06/15] Add efi_arch_handle_cmdline() for processing commandline
2014-09-18 22:49 ` [PATCH V5 06/15] Add efi_arch_handle_cmdline() for processing commandline Roy Franz
@ 2014-09-22 12:17 ` Jan Beulich
2014-09-23 1:40 ` Roy Franz
0 siblings, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2014-09-22 12:17 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, ian.campbell, tim, xen-devel, stefano.stabellini, fu.wei
>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
> Add arch function for processing the Xen commandline and
> updating internal structures.
>
> Signed-off-by: Roy Franz <roy.franz@linaro.org>
With two small requests for adjustments below
Acked-by: Jan Beulich <jbeulich@suse.com>
> @@ -256,7 +257,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)
I think this would better be named "dom0_options" or "dom0_opts" or
some such (also in the caller).
> @@ -701,14 +701,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, &options);
I don't see why you don't pass NULL here, the more that you add
a respective check to get_argv().
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH V5 06/15] Add efi_arch_handle_cmdline() for processing commandline
2014-09-22 12:17 ` Jan Beulich
@ 2014-09-23 1:40 ` Roy Franz
2014-09-23 12:25 ` Jan Beulich
0 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-23 1:40 UTC (permalink / raw)
To: Jan Beulich
Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei
On Mon, Sep 22, 2014 at 5:17 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
>> Add arch function for processing the Xen commandline and
>> updating internal structures.
>>
>> Signed-off-by: Roy Franz <roy.franz@linaro.org>
>
> With two small requests for adjustments below
> Acked-by: Jan Beulich <jbeulich@suse.com>
>
>> @@ -256,7 +257,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)
>
> I think this would better be named "dom0_options" or "dom0_opts" or
> some such (also in the caller).
I think xen_options would be better, or else I'm really confused :)
This string is combined with the Xen options from the
config file into mbi.cmdline. Dom0 arguments come from the "kernel"
line in the configuration tile.
>
>> @@ -701,14 +701,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, &options);
>
> I don't see why you don't pass NULL here, the more that you add
> a respective check to get_argv().
I don't either - I will change this.
>
> Jan
>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH V5 06/15] Add efi_arch_handle_cmdline() for processing commandline
2014-09-23 1:40 ` Roy Franz
@ 2014-09-23 12:25 ` Jan Beulich
2014-09-24 0:09 ` Roy Franz
0 siblings, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2014-09-23 12:25 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei
>>> On 23.09.14 at 03:40, <roy.franz@linaro.org> wrote:
> On Mon, Sep 22, 2014 at 5:17 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
>>> @@ -256,7 +257,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)
>>
>> I think this would better be named "dom0_options" or "dom0_opts" or
>> some such (also in the caller).
>
> I think xen_options would be better, or else I'm really confused :)
> This string is combined with the Xen options from the
> config file into mbi.cmdline. Dom0 arguments come from the "kernel"
> line in the configuration tile.
Indeed - I mixed things up: This is the first "--" separator, the Dom0
options come after the second. In which case staying with just
"options" is quite fine.
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH V5 06/15] Add efi_arch_handle_cmdline() for processing commandline
2014-09-23 12:25 ` Jan Beulich
@ 2014-09-24 0:09 ` Roy Franz
2014-09-24 8:13 ` Jan Beulich
0 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-24 0:09 UTC (permalink / raw)
To: Jan Beulich
Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei
On Tue, Sep 23, 2014 at 5:25 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 23.09.14 at 03:40, <roy.franz@linaro.org> wrote:
>> On Mon, Sep 22, 2014 at 5:17 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
>>>> @@ -256,7 +257,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)
>>>
>>> I think this would better be named "dom0_options" or "dom0_opts" or
>>> some such (also in the caller).
>>
>> I think xen_options would be better, or else I'm really confused :)
>> This string is combined with the Xen options from the
>> config file into mbi.cmdline. Dom0 arguments come from the "kernel"
>> line in the configuration tile.
>
> Indeed - I mixed things up: This is the first "--" separator, the Dom0
> options come after the second. In which case staying with just
> "options" is quite fine.
>
> Jan
>
Is this second "--" handled by XEN itself? I don't see any handling
of that in the EFI boot code. The only thing that goes
into the multiboot .string field for the dom0 kernel is from the config file.
Roy
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH V5 06/15] Add efi_arch_handle_cmdline() for processing commandline
2014-09-24 0:09 ` Roy Franz
@ 2014-09-24 8:13 ` Jan Beulich
2014-09-24 9:33 ` Ian Campbell
0 siblings, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2014-09-24 8:13 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei
>>> On 24.09.14 at 02:09, <roy.franz@linaro.org> wrote:
> On Tue, Sep 23, 2014 at 5:25 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 23.09.14 at 03:40, <roy.franz@linaro.org> wrote:
>>> On Mon, Sep 22, 2014 at 5:17 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
>>>>> @@ -256,7 +257,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)
>>>>
>>>> I think this would better be named "dom0_options" or "dom0_opts" or
>>>> some such (also in the caller).
>>>
>>> I think xen_options would be better, or else I'm really confused :)
>>> This string is combined with the Xen options from the
>>> config file into mbi.cmdline. Dom0 arguments come from the "kernel"
>>> line in the configuration tile.
>>
>> Indeed - I mixed things up: This is the first "--" separator, the Dom0
>> options come after the second. In which case staying with just
>> "options" is quite fine.
>
> Is this second "--" handled by XEN itself? I don't see any handling
> of that in the EFI boot code. The only thing that goes
> into the multiboot .string field for the dom0 kernel is from the config
> file.
Yes, that's being handled in __start_xen() (search for "kextra" if
you want to see where exactly).
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH V5 06/15] Add efi_arch_handle_cmdline() for processing commandline
2014-09-24 8:13 ` Jan Beulich
@ 2014-09-24 9:33 ` Ian Campbell
2014-09-24 11:34 ` Jan Beulich
0 siblings, 1 reply; 63+ messages in thread
From: Ian Campbell @ 2014-09-24 9:33 UTC (permalink / raw)
To: Jan Beulich; +Cc: keir, tim, xen-devel, Roy Franz, Stefano Stabellini, Fu Wei
On Wed, 2014-09-24 at 09:13 +0100, Jan Beulich wrote:
> >>> On 24.09.14 at 02:09, <roy.franz@linaro.org> wrote:
> > On Tue, Sep 23, 2014 at 5:25 AM, Jan Beulich <JBeulich@suse.com> wrote:
> >>>>> On 23.09.14 at 03:40, <roy.franz@linaro.org> wrote:
> >>> On Mon, Sep 22, 2014 at 5:17 AM, Jan Beulich <JBeulich@suse.com> wrote:
> >>>>>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
> >>>>> @@ -256,7 +257,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)
> >>>>
> >>>> I think this would better be named "dom0_options" or "dom0_opts" or
> >>>> some such (also in the caller).
> >>>
> >>> I think xen_options would be better, or else I'm really confused :)
> >>> This string is combined with the Xen options from the
> >>> config file into mbi.cmdline. Dom0 arguments come from the "kernel"
> >>> line in the configuration tile.
> >>
> >> Indeed - I mixed things up: This is the first "--" separator, the Dom0
> >> options come after the second. In which case staying with just
> >> "options" is quite fine.
> >
> > Is this second "--" handled by XEN itself? I don't see any handling
> > of that in the EFI boot code. The only thing that goes
> > into the multiboot .string field for the dom0 kernel is from the config
> > file.
>
> Yes, that's being handled in __start_xen() (search for "kextra" if
> you want to see where exactly).
I'd never heard of this behaviour before, and it is (currently at least)
x86 specific.
I don't know if this is relevant to the discusion here though, probably
not?
Ian.
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH V5 06/15] Add efi_arch_handle_cmdline() for processing commandline
2014-09-24 9:33 ` Ian Campbell
@ 2014-09-24 11:34 ` Jan Beulich
0 siblings, 0 replies; 63+ messages in thread
From: Jan Beulich @ 2014-09-24 11:34 UTC (permalink / raw)
To: Ian Campbell; +Cc: keir, tim, xen-devel, Roy Franz, Stefano Stabellini, Fu Wei
>>> On 24.09.14 at 11:33, <Ian.Campbell@citrix.com> wrote:
> On Wed, 2014-09-24 at 09:13 +0100, Jan Beulich wrote:
>> >>> On 24.09.14 at 02:09, <roy.franz@linaro.org> wrote:
>> > Is this second "--" handled by XEN itself? I don't see any handling
>> > of that in the EFI boot code. The only thing that goes
>> > into the multiboot .string field for the dom0 kernel is from the config
>> > file.
>>
>> Yes, that's being handled in __start_xen() (search for "kextra" if
>> you want to see where exactly).
>
> I'd never heard of this behaviour before, and it is (currently at least)
> x86 specific.
Yeah, we needed this a long while ago for the case where the GrUB
GUI shows only a single line for entering extra options, which then
all get passed to the hypervisor making it impossible to add extra
Dom0 kernel options this same way.
> I don't know if this is relevant to the discusion here though, probably
> not?
Definitely not. I just didn't want to leave Roy's question unanswered.
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH V5 07/15] Move x86 specific disk probing code
2014-09-18 22:49 [PATCH V5 00/15] arm64 EFI stub Roy Franz
` (5 preceding siblings ...)
2014-09-18 22:49 ` [PATCH V5 06/15] Add efi_arch_handle_cmdline() for processing commandline Roy Franz
@ 2014-09-18 22:49 ` Roy Franz
2014-09-22 12:20 ` Jan Beulich
2014-09-18 22:49 ` [PATCH V5 08/15] Create arch functions for console and video init Roy Franz
` (7 subsequent siblings)
14 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-18 22:49 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>
---
xen/common/efi/boot.c | 144 +-------------------------------------
xen/include/asm-x86/efi-boot.h | 152 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 153 insertions(+), 143 deletions(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 2f8dade..2914225 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -654,8 +654,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;
@@ -945,147 +943,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 *)¶ms->key; p < ¶ms->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. */
diff --git a/xen/include/asm-x86/efi-boot.h b/xen/include/asm-x86/efi-boot.h
index 6e9376a..17b5840 100644
--- a/xen/include/asm-x86/efi-boot.h
+++ b/xen/include/asm-x86/efi-boot.h
@@ -313,3 +313,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;
+ 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 *)¶ms->key; p < ¶ms->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;
+}
--
2.1.0
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH V5 07/15] Move x86 specific disk probing code
2014-09-18 22:49 ` [PATCH V5 07/15] Move x86 specific disk probing code Roy Franz
@ 2014-09-22 12:20 ` Jan Beulich
2014-09-23 1:44 ` Roy Franz
0 siblings, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2014-09-22 12:20 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, ian.campbell, tim, xen-devel, stefano.stabellini, fu.wei
>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
> 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>
as soon as ...
> +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;
> + int i;
... this becomes "unsigned int". Didn't you say you'd go through the
whole series and correct these?
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH V5 07/15] Move x86 specific disk probing code
2014-09-22 12:20 ` Jan Beulich
@ 2014-09-23 1:44 ` Roy Franz
0 siblings, 0 replies; 63+ messages in thread
From: Roy Franz @ 2014-09-23 1:44 UTC (permalink / raw)
To: Jan Beulich
Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei
On Mon, Sep 22, 2014 at 5:20 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 19.09.14 at 00:49, <roy.franz@linaro.org> wrote:
>> 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>
> as soon as ...
>
>> +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;
>> + int i;
>
> ... this becomes "unsigned int". Didn't you say you'd go through the
> whole series and correct these?
>
> Jan
>
My review all comment was regarding the blank line between variable
definitions and statements in functions. (looks like I missed one of
those too.)
I will fix this and review types of all local variables created in the
arch specific functions.
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH V5 08/15] Create arch functions for console and video init
2014-09-18 22:49 [PATCH V5 00/15] arm64 EFI stub Roy Franz
` (6 preceding siblings ...)
2014-09-18 22:49 ` [PATCH V5 07/15] Move x86 specific disk probing code Roy Franz
@ 2014-09-18 22:49 ` Roy Franz
2014-09-22 12:21 ` Jan Beulich
2014-09-18 22:50 ` [PATCH V5 09/15] Add efi_arch_memory() for arch specific memory setup Roy Franz
` (6 subsequent siblings)
14 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-18 22:49 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>
---
xen/common/efi/boot.c | 69 ++------------------------------------
xen/include/asm-x86/efi-boot.h | 75 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+), 66 deletions(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 2914225..1917052 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;
@@ -768,12 +769,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);
@@ -1027,7 +1023,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 )
@@ -1036,65 +1031,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(&mmap_size, NULL, &mmap_key,
diff --git a/xen/include/asm-x86/efi-boot.h b/xen/include/asm-x86/efi-boot.h
index 17b5840..7df04bb 100644
--- a/xen/include/asm-x86/efi-boot.h
+++ b/xen/include/asm-x86/efi-boot.h
@@ -465,3 +465,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;
+ }
+}
--
2.1.0
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH V5 09/15] Add efi_arch_memory() for arch specific memory setup
2014-09-18 22:49 [PATCH V5 00/15] arm64 EFI stub Roy Franz
` (7 preceding siblings ...)
2014-09-18 22:49 ` [PATCH V5 08/15] Create arch functions for console and video init Roy Franz
@ 2014-09-18 22:50 ` Roy Franz
2014-09-22 12:24 ` Jan Beulich
2014-09-18 22:50 ` [PATCH V5 10/15] Add arch specific module handling to read_file() Roy Franz
` (5 subsequent siblings)
14 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-18 22:50 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>
---
xen/common/efi/boot.c | 29 +----------------------------
xen/include/asm-x86/efi-boot.h | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 28 deletions(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 1917052..38177bf 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -992,34 +992,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();
if ( gop )
{
diff --git a/xen/include/asm-x86/efi-boot.h b/xen/include/asm-x86/efi-boot.h
index 7df04bb..2902970 100644
--- a/xen/include/asm-x86/efi-boot.h
+++ b/xen/include/asm-x86/efi-boot.h
@@ -540,3 +540,38 @@ static void __init efi_arch_video_init(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop,
(gop->Mode->FrameBufferSize + 0xffff) >> 16;
}
}
+
+static void __init efi_arch_memory(void)
+{
+ 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);
+}
--
2.1.0
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH V5 09/15] Add efi_arch_memory() for arch specific memory setup
2014-09-18 22:50 ` [PATCH V5 09/15] Add efi_arch_memory() for arch specific memory setup Roy Franz
@ 2014-09-22 12:24 ` Jan Beulich
2014-09-23 1:45 ` Roy Franz
0 siblings, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2014-09-22 12:24 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, ian.campbell, tim, xen-devel, stefano.stabellini, fu.wei
>>> On 19.09.14 at 00:50, <roy.franz@linaro.org> wrote:
> 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>
Again
Acked-by: Jan Beulich <jbeulich@suse.com>
provided ...
> +static void __init efi_arch_memory(void)
> +{
> + int i;
... this gets converted to unsigned int.
Also maybe name the function efi_arch_memory_setup()?
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH V5 09/15] Add efi_arch_memory() for arch specific memory setup
2014-09-22 12:24 ` Jan Beulich
@ 2014-09-23 1:45 ` Roy Franz
0 siblings, 0 replies; 63+ messages in thread
From: Roy Franz @ 2014-09-23 1:45 UTC (permalink / raw)
To: Jan Beulich
Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei
On Mon, Sep 22, 2014 at 5:24 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 19.09.14 at 00:50, <roy.franz@linaro.org> wrote:
>> 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>
>
> Again
> Acked-by: Jan Beulich <jbeulich@suse.com>
> provided ...
>
>> +static void __init efi_arch_memory(void)
>> +{
>> + int i;
>
> ... this gets converted to unsigned int.
>
> Also maybe name the function efi_arch_memory_setup()?
>
> Jan
>
Sure.
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH V5 10/15] Add arch specific module handling to read_file()
2014-09-18 22:49 [PATCH V5 00/15] arm64 EFI stub Roy Franz
` (8 preceding siblings ...)
2014-09-18 22:50 ` [PATCH V5 09/15] Add efi_arch_memory() for arch specific memory setup Roy Franz
@ 2014-09-18 22:50 ` Roy Franz
2014-09-22 12:44 ` Jan Beulich
2014-09-18 22:50 ` [PATCH V5 11/15] Add several misc. arch functions for EFI boot code Roy Franz
` (4 subsequent siblings)
14 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-18 22:50 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.
Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
xen/common/efi/boot.c | 102 ++++++++++++++++++++++++-----------------
xen/include/asm-x86/efi-boot.h | 32 ++++++++++++-
2 files changed, 91 insertions(+), 43 deletions(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 38177bf..4acab40 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -56,12 +56,13 @@ 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 size_t wstrlen(const CHAR16 * s);
static int set_color(u32 mask, int bpp, u8 *pos, u8 *sz);
+static bool_t read_file(EFI_FILE_HANDLE dir_handle, struct file *file,
+ CHAR16 *filename, char *options);
static EFI_BOOT_SERVICES *__initdata efi_bs;
static EFI_HANDLE __initdata efi_ih;
@@ -115,6 +116,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;
@@ -404,18 +414,33 @@ static CHAR16 *__init point_tail(CHAR16 *fn)
break;
}
}
+/*
+ * Truncate string at first space, and return pointer
+ * to remainder of string.
+ */
+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)
+static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, struct file *file,
+ CHAR16 *filename, char *options)
{
EFI_FILE_HANDLE FileHandle = NULL;
UINT64 size;
EFI_STATUS ret;
CHAR16 *what = NULL;
- if ( !name )
+ if ( !filename )
PrintErrMesg(L"No filename", EFI_OUT_OF_RESOURCES);
- ret = dir_handle->Open(dir_handle, &FileHandle, name,
+ ret = dir_handle->Open(dir_handle, &FileHandle, filename,
EFI_FILE_MODE_READ, 0);
if ( file == &cfg && ret == EFI_NOT_FOUND )
return 0;
@@ -447,20 +472,18 @@ static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
}
else
{
+ file->size = size;
if ( file != &cfg )
{
- PrintStr(name);
+ PrintStr(filename);
PrintStr(L": ");
DisplayUint(file->addr, 2 * sizeof(file->addr));
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, filename, options);
}
- file->size = size;
ret = FileHandle->Read(FileHandle, &file->size, file->ptr);
if ( !EFI_ERROR(ret) && file->size != size )
ret = EFI_ABORTED;
@@ -475,7 +498,7 @@ static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
{
PrintErr(what);
PrintErr(L" failed for ");
- PrintErrMesg(name, ret);
+ PrintErrMesg(filename, ret);
}
return 1;
@@ -531,7 +554,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);
@@ -539,16 +568,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;
@@ -659,18 +678,19 @@ 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, *options = NULL;
+ CHAR16 **argv, *options = NULL;
UINTN cols, rows, depth, size, info_size, gop_mode = ~0;
EFI_HANDLE *handles = NULL;
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;
+ union string section = { NULL }, name, file_name, cfg_file_name = {NULL};
bool_t base_video = 0;
UINT32 mmap_desc_ver = 0;
UINTN mmap_size, mmap_desc_size, mmap_key = 0;
void *mmap;
+ char *option_str;
efi_ih = ImageHandle;
efi_bs = SystemTable->BootServices;
@@ -697,7 +717,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
trampoline_xen_phys_start = xen_phys_start;
/* Get the file system interface. */
- dir_handle = get_parent_handle(loaded_image, &file_name);
+ dir_handle = get_parent_handle(loaded_image, &file_name.w);
argc = get_argv(0, NULL, loaded_image->LoadOptions,
loaded_image->LoadOptionsSize, &options);
@@ -721,9 +741,9 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
if ( wstrcmp(ptr + 1, L"basevideo") == 0 )
base_video = 1;
else if ( wstrncmp(ptr + 1, L"cfg=", 4) == 0 )
- cfg_file_name = ptr + 5;
+ cfg_file_name.w = ptr + 5;
else if ( i + 1 < argc && wstrcmp(ptr + 1, L"cfg") == 0 )
- cfg_file_name = argv[++i];
+ cfg_file_name.w = argv[++i];
else if ( wstrcmp(ptr + 1, L"help") == 0 ||
(ptr[1] == L'?' && !ptr[2]) )
{
@@ -795,24 +815,24 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
gop = NULL;
/* Read and parse the config file. */
- if ( !cfg_file_name )
+ if ( !cfg_file_name.w )
{
CHAR16 *tail;
- while ( (tail = point_tail(file_name)) != NULL )
+ while ( (tail = point_tail(file_name.w)) != NULL )
{
wstrcpy(tail, L".cfg");
- if ( read_file(dir_handle, file_name, &cfg) )
+ if ( read_file(dir_handle, &cfg, file_name.w, NULL) )
break;
*tail = 0;
}
if ( !tail )
blexit(L"No configuration file found.");
PrintStr(L"Using configuration file '");
- PrintStr(file_name);
+ PrintStr(file_name.w);
PrintStr(L"'\r\n");
}
- else if ( !read_file(dir_handle, cfg_file_name, &cfg) )
+ else if ( !read_file(dir_handle, &cfg, cfg_file_name.w, NULL) )
blexit(L"Configuration file not found.");
pre_parse(&cfg);
@@ -831,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, &cfg, s2w(&name), NULL) )
{
PrintStr(L"Chained configuration file '");
PrintStr(name.w);
@@ -846,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, &kernel, s2w(&name), option_str);
efi_bs->FreePool(name.w);
if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
@@ -858,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, &ramdisk, s2w(&name), 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, &xsm, s2w(&name), option_str);
efi_bs->FreePool(name.w);
}
diff --git a/xen/include/asm-x86/efi-boot.h b/xen/include/asm-x86/efi-boot.h
index 2902970..3902b6c 100644
--- a/xen/include/asm-x86/efi-boot.h
+++ b/xen/include/asm-x86/efi-boot.h
@@ -267,6 +267,7 @@ static void __init efi_arch_cfg_file_early(EFI_FILE_HANDLE dir_handle, char *sec
static void __init efi_arch_cfg_file_late(EFI_FILE_HANDLE dir_handle, char *section)
{
union string name;
+ char *options;
name.s = get_value(&cfg, section, "ucode");
if ( !name.s )
@@ -274,8 +275,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);
+ options = split_string(name.s);
+ read_file(dir_handle, &ucode, s2w(&name), options);
efi_bs->FreePool(name.w);
}
}
@@ -575,3 +576,30 @@ static void __init efi_arch_memory(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\r\n");
+
+ 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);
+}
--
2.1.0
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH V5 10/15] Add arch specific module handling to read_file()
2014-09-18 22:50 ` [PATCH V5 10/15] Add arch specific module handling to read_file() Roy Franz
@ 2014-09-22 12:44 ` Jan Beulich
2014-09-23 1:57 ` Roy Franz
0 siblings, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2014-09-22 12:44 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, ian.campbell, tim, xen-devel, stefano.stabellini, fu.wei
>>> On 19.09.14 at 00:50, <roy.franz@linaro.org> wrote:
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -56,12 +56,13 @@ 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 size_t wstrlen(const CHAR16 * s);
> static int set_color(u32 mask, int bpp, u8 *pos, u8 *sz);
> +static bool_t read_file(EFI_FILE_HANDLE dir_handle, struct file *file,
> + CHAR16 *filename, char *options);
Please don't needlessly move this declaration down.
> @@ -115,6 +116,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;
> +}
Coding style (many issues).
> @@ -404,18 +414,33 @@ static CHAR16 *__init point_tail(CHAR16 *fn)
> break;
> }
> }
> +/*
> + * Truncate string at first space, and return pointer
> + * to remainder of string.
... (if any).
> + */
> +static char * __init split_string(char *s)
> +{
> + while ( *s && !isspace(*s) )
> + ++s;
> + if ( *s )
> + {
> + *s = 0;
> + return(s + 1);
No parentheses here please.
> + }
> + return NULL;
> +}
>
> -static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
> - struct file *file)
> +static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, struct file *file,
> + CHAR16 *filename, char *options)
Is the renaming from "name" to "filename" really necessary/useful?
And the flipping of the order of pre-existing parameters?
> @@ -659,18 +678,19 @@ 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, *options = NULL;
> + CHAR16 **argv, *options = NULL;
> UINTN cols, rows, depth, size, info_size, gop_mode = ~0;
> EFI_HANDLE *handles = NULL;
> 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;
> + union string section = { NULL }, name, file_name, cfg_file_name = {NULL};
Your addition should be consistent with existing code (blanks around
NULL). But - why are you changing the types of the two variables in
the first place? Afaics all references to them now are using the .w
member access, suggesting that this is just a remnant of your earlier
version changes.
> @@ -274,8 +275,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);
> + options = split_string(name.s);
> + read_file(dir_handle, &ucode, s2w(&name), options);
Perhaps rather NULL instead of options?
> @@ -575,3 +576,30 @@ static void __init efi_arch_memory(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\r\n");
Iirc I said this before, but just in case: No explicit newline on strings
passed to blexit() please.
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH V5 10/15] Add arch specific module handling to read_file()
2014-09-22 12:44 ` Jan Beulich
@ 2014-09-23 1:57 ` Roy Franz
2014-09-23 12:28 ` Jan Beulich
2014-09-24 1:23 ` Roy Franz
0 siblings, 2 replies; 63+ messages in thread
From: Roy Franz @ 2014-09-23 1:57 UTC (permalink / raw)
To: Jan Beulich
Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei
On Mon, Sep 22, 2014 at 5:44 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 19.09.14 at 00:50, <roy.franz@linaro.org> wrote:
>> --- a/xen/common/efi/boot.c
>> +++ b/xen/common/efi/boot.c
>> @@ -56,12 +56,13 @@ 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 size_t wstrlen(const CHAR16 * s);
>> static int set_color(u32 mask, int bpp, u8 *pos, u8 *sz);
>> +static bool_t read_file(EFI_FILE_HANDLE dir_handle, struct file *file,
>> + CHAR16 *filename, char *options);
>
> Please don't needlessly move this declaration down.
>
I'll fix this.
>> @@ -115,6 +116,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;
>> +}
>
> Coding style (many issues).
static size_t __init wstrlen(const CHAR16 *s)
{
const CHAR16 *sc;
for ( sc = s; *sc != L'\0'; ++sc )
;
return sc - s;
}
Is the above OK? Not sure what else to change here...
>
>> @@ -404,18 +414,33 @@ static CHAR16 *__init point_tail(CHAR16 *fn)
>> break;
>> }
>> }
>> +/*
>> + * Truncate string at first space, and return pointer
>> + * to remainder of string.
>
> ... (if any).
>
Sure.
>> + */
>> +static char * __init split_string(char *s)
>> +{
>> + while ( *s && !isspace(*s) )
>> + ++s;
>> + if ( *s )
>> + {
>> + *s = 0;
>> + return(s + 1);
>
> No parentheses here please.
sure
>
>> + }
>> + return NULL;
>> +}
>>
>> -static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
>> - struct file *file)
>> +static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, struct file *file,
>> + CHAR16 *filename, char *options)
>
> Is the renaming from "name" to "filename" really necessary/useful?
> And the flipping of the order of pre-existing parameters?
I'll fix this.
>
>> @@ -659,18 +678,19 @@ 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, *options = NULL;
>> + CHAR16 **argv, *options = NULL;
>> UINTN cols, rows, depth, size, info_size, gop_mode = ~0;
>> EFI_HANDLE *handles = NULL;
>> 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;
>> + union string section = { NULL }, name, file_name, cfg_file_name = {NULL};
>
> Your addition should be consistent with existing code (blanks around
> NULL). But - why are you changing the types of the two variables in
> the first place? Afaics all references to them now are using the .w
> member access, suggesting that this is just a remnant of your earlier
> version changes.
>
Yes, this is leftover type changes from a previous version. Both
these should be able to to be
reverted to simple CHAR16 *
>> @@ -274,8 +275,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);
>> + options = split_string(name.s);
>> + read_file(dir_handle, &ucode, s2w(&name), options);
>
> Perhaps rather NULL instead of options?
Yup.
>
>> @@ -575,3 +576,30 @@ static void __init efi_arch_memory(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\r\n");
>
> Iirc I said this before, but just in case: No explicit newline on strings
> passed to blexit() please.
Yes.
>
> Jan
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH V5 10/15] Add arch specific module handling to read_file()
2014-09-23 1:57 ` Roy Franz
@ 2014-09-23 12:28 ` Jan Beulich
2014-09-23 12:41 ` Ian Campbell
2014-09-24 1:23 ` Roy Franz
1 sibling, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2014-09-23 12:28 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei
>>> On 23.09.14 at 03:57, <roy.franz@linaro.org> wrote:
> On Mon, Sep 22, 2014 at 5:44 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 19.09.14 at 00:50, <roy.franz@linaro.org> wrote:
>>> @@ -115,6 +116,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;
>>> +}
>>
>> Coding style (many issues).
>
> static size_t __init wstrlen(const CHAR16 *s)
> {
> const CHAR16 *sc;
>
> for ( sc = s; *sc != L'\0'; ++sc )
> ;
> return sc - s;
> }
>
> Is the above OK? Not sure what else to change here...
Almost - the lone ; is still indented one level (4 spaces) too deep.
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH V5 10/15] Add arch specific module handling to read_file()
2014-09-23 12:28 ` Jan Beulich
@ 2014-09-23 12:41 ` Ian Campbell
2014-09-23 12:55 ` Jan Beulich
0 siblings, 1 reply; 63+ messages in thread
From: Ian Campbell @ 2014-09-23 12:41 UTC (permalink / raw)
To: Jan Beulich; +Cc: keir, tim, xen-devel, Roy Franz, Stefano Stabellini, Fu Wei
On Tue, 2014-09-23 at 13:28 +0100, Jan Beulich wrote:
> >>> On 23.09.14 at 03:57, <roy.franz@linaro.org> wrote:
> > On Mon, Sep 22, 2014 at 5:44 AM, Jan Beulich <JBeulich@suse.com> wrote:
> >>>>> On 19.09.14 at 00:50, <roy.franz@linaro.org> wrote:
> >>> @@ -115,6 +116,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;
> >>> +}
> >>
> >> Coding style (many issues).
> >
> > static size_t __init wstrlen(const CHAR16 *s)
> > {
> > const CHAR16 *sc;
> >
> > for ( sc = s; *sc != L'\0'; ++sc )
> > ;
> > return sc - s;
> > }
> >
> > Is the above OK? Not sure what else to change here...
>
> Almost - the lone ; is still indented one level (4 spaces) too deep.
Would you be inclined to keep the /* nothing */ too as a signal to the
reader that it is deliberate?
Ian
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH V5 10/15] Add arch specific module handling to read_file()
2014-09-23 12:41 ` Ian Campbell
@ 2014-09-23 12:55 ` Jan Beulich
0 siblings, 0 replies; 63+ messages in thread
From: Jan Beulich @ 2014-09-23 12:55 UTC (permalink / raw)
To: Ian Campbell; +Cc: keir, tim, xen-devel, Roy Franz, Stefano Stabellini, Fu Wei
>>> On 23.09.14 at 14:41, <Ian.Campbell@citrix.com> wrote:
> On Tue, 2014-09-23 at 13:28 +0100, Jan Beulich wrote:
>> >>> On 23.09.14 at 03:57, <roy.franz@linaro.org> wrote:
>> > On Mon, Sep 22, 2014 at 5:44 AM, Jan Beulich <JBeulich@suse.com> wrote:
>> >>>>> On 19.09.14 at 00:50, <roy.franz@linaro.org> wrote:
>> >>> @@ -115,6 +116,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;
>> >>> +}
>> >>
>> >> Coding style (many issues).
>> >
>> > static size_t __init wstrlen(const CHAR16 *s)
>> > {
>> > const CHAR16 *sc;
>> >
>> > for ( sc = s; *sc != L'\0'; ++sc )
>> > ;
>> > return sc - s;
>> > }
>> >
>> > Is the above OK? Not sure what else to change here...
>>
>> Almost - the lone ; is still indented one level (4 spaces) too deep.
>
> Would you be inclined to keep the /* nothing */ too as a signal to the
> reader that it is deliberate?
Yes - I didn't pay attention to it having got dropped with the
re-formatting.
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH V5 10/15] Add arch specific module handling to read_file()
2014-09-23 1:57 ` Roy Franz
2014-09-23 12:28 ` Jan Beulich
@ 2014-09-24 1:23 ` Roy Franz
2014-09-24 4:43 ` Roy Franz
2014-09-24 8:18 ` Jan Beulich
1 sibling, 2 replies; 63+ messages in thread
From: Roy Franz @ 2014-09-24 1:23 UTC (permalink / raw)
To: Jan Beulich
Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei
On Mon, Sep 22, 2014 at 6:57 PM, Roy Franz <roy.franz@linaro.org> wrote:
> On Mon, Sep 22, 2014 at 5:44 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 19.09.14 at 00:50, <roy.franz@linaro.org> wrote:
>>> --- a/xen/common/efi/boot.c
>>> +++ b/xen/common/efi/boot.c
>>> @@ -56,12 +56,13 @@ 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 size_t wstrlen(const CHAR16 * s);
>>> static int set_color(u32 mask, int bpp, u8 *pos, u8 *sz);
>>> +static bool_t read_file(EFI_FILE_HANDLE dir_handle, struct file *file,
>>> + CHAR16 *filename, char *options);
>>
>> Please don't needlessly move this declaration down.
>>
> I'll fix this.
>
>>> @@ -115,6 +116,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;
>>> +}
>>
>> Coding style (many issues).
>
> static size_t __init wstrlen(const CHAR16 *s)
> {
> const CHAR16 *sc;
>
> for ( sc = s; *sc != L'\0'; ++sc )
> ;
> return sc - s;
> }
>
> Is the above OK? Not sure what else to change here...
>
>>
>>> @@ -404,18 +414,33 @@ static CHAR16 *__init point_tail(CHAR16 *fn)
>>> break;
>>> }
>>> }
>>> +/*
>>> + * Truncate string at first space, and return pointer
>>> + * to remainder of string.
>>
>> ... (if any).
>>
>
> Sure.
>
>
>>> + */
>>> +static char * __init split_string(char *s)
>>> +{
>>> + while ( *s && !isspace(*s) )
>>> + ++s;
>>> + if ( *s )
>>> + {
>>> + *s = 0;
>>> + return(s + 1);
>>
>> No parentheses here please.
>
> sure
>
>
>>
>>> + }
>>> + return NULL;
>>> +}
>>>
>>> -static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
>>> - struct file *file)
>>> +static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, struct file *file,
>>> + CHAR16 *filename, char *options)
>>
>> Is the renaming from "name" to "filename" really necessary/useful?
>> And the flipping of the order of pre-existing parameters?
>
> I'll fix this.
>
>>
>>> @@ -659,18 +678,19 @@ 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, *options = NULL;
>>> + CHAR16 **argv, *options = NULL;
>>> UINTN cols, rows, depth, size, info_size, gop_mode = ~0;
>>> EFI_HANDLE *handles = NULL;
>>> 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;
>>> + union string section = { NULL }, name, file_name, cfg_file_name = {NULL};
>>
>> Your addition should be consistent with existing code (blanks around
>> NULL). But - why are you changing the types of the two variables in
>> the first place? Afaics all references to them now are using the .w
>> member access, suggesting that this is just a remnant of your earlier
>> version changes.
>>
>
> Yes, this is leftover type changes from a previous version. Both
> these should be able to to be
> reverted to simple CHAR16 *
>>> @@ -274,8 +275,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);
>>> + options = split_string(name.s);
>>> + read_file(dir_handle, &ucode, s2w(&name), options);
>>
>> Perhaps rather NULL instead of options?
>
> Yup.
>
OK, I looked at this a little more, and the original code would take
anything after the ucode filename
and put it into the mbi.string field for the ucode module. All the
modules where treated the same in this
regard - they could all have options.
If ucode never has options, I can remove this, but this would be a
change in behavior.
>>
>>> @@ -575,3 +576,30 @@ static void __init efi_arch_memory(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\r\n");
>>
>> Iirc I said this before, but just in case: No explicit newline on strings
>> passed to blexit() please.
> Yes.
>>
>> Jan
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH V5 10/15] Add arch specific module handling to read_file()
2014-09-24 1:23 ` Roy Franz
@ 2014-09-24 4:43 ` Roy Franz
2014-09-24 8:18 ` Jan Beulich
1 sibling, 0 replies; 63+ messages in thread
From: Roy Franz @ 2014-09-24 4:43 UTC (permalink / raw)
To: Jan Beulich
Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei
On Tue, Sep 23, 2014 at 6:23 PM, Roy Franz <roy.franz@linaro.org> wrote:
> On Mon, Sep 22, 2014 at 6:57 PM, Roy Franz <roy.franz@linaro.org> wrote:
>> On Mon, Sep 22, 2014 at 5:44 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 19.09.14 at 00:50, <roy.franz@linaro.org> wrote:
>>>> --- a/xen/common/efi/boot.c
>>>> +++ b/xen/common/efi/boot.c
>>>> @@ -56,12 +56,13 @@ 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 size_t wstrlen(const CHAR16 * s);
>>>> static int set_color(u32 mask, int bpp, u8 *pos, u8 *sz);
>>>> +static bool_t read_file(EFI_FILE_HANDLE dir_handle, struct file *file,
>>>> + CHAR16 *filename, char *options);
>>>
>>> Please don't needlessly move this declaration down.
>>>
>> I'll fix this.
>>
>>>> @@ -115,6 +116,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;
>>>> +}
>>>
>>> Coding style (many issues).
>>
>> static size_t __init wstrlen(const CHAR16 *s)
>> {
>> const CHAR16 *sc;
>>
>> for ( sc = s; *sc != L'\0'; ++sc )
>> ;
>> return sc - s;
>> }
>>
>> Is the above OK? Not sure what else to change here...
>>
>>>
>>>> @@ -404,18 +414,33 @@ static CHAR16 *__init point_tail(CHAR16 *fn)
>>>> break;
>>>> }
>>>> }
>>>> +/*
>>>> + * Truncate string at first space, and return pointer
>>>> + * to remainder of string.
>>>
>>> ... (if any).
>>>
>>
>> Sure.
>>
>>
>>>> + */
>>>> +static char * __init split_string(char *s)
>>>> +{
>>>> + while ( *s && !isspace(*s) )
>>>> + ++s;
>>>> + if ( *s )
>>>> + {
>>>> + *s = 0;
>>>> + return(s + 1);
>>>
>>> No parentheses here please.
>>
>> sure
>>
>>
>>>
>>>> + }
>>>> + return NULL;
>>>> +}
>>>>
>>>> -static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
>>>> - struct file *file)
>>>> +static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, struct file *file,
>>>> + CHAR16 *filename, char *options)
>>>
>>> Is the renaming from "name" to "filename" really necessary/useful?
>>> And the flipping of the order of pre-existing parameters?
>>
>> I'll fix this.
>>
>>>
>>>> @@ -659,18 +678,19 @@ 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, *options = NULL;
>>>> + CHAR16 **argv, *options = NULL;
>>>> UINTN cols, rows, depth, size, info_size, gop_mode = ~0;
>>>> EFI_HANDLE *handles = NULL;
>>>> 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;
>>>> + union string section = { NULL }, name, file_name, cfg_file_name = {NULL};
>>>
>>> Your addition should be consistent with existing code (blanks around
>>> NULL). But - why are you changing the types of the two variables in
>>> the first place? Afaics all references to them now are using the .w
>>> member access, suggesting that this is just a remnant of your earlier
>>> version changes.
>>>
>>
>> Yes, this is leftover type changes from a previous version. Both
>> these should be able to to be
>> reverted to simple CHAR16 *
>>>> @@ -274,8 +275,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);
>>>> + options = split_string(name.s);
>>>> + read_file(dir_handle, &ucode, s2w(&name), options);
>>>
>>> Perhaps rather NULL instead of options?
>>
>> Yup.
>>
> OK, I looked at this a little more, and the original code would take
> anything after the ucode filename
> and put it into the mbi.string field for the ucode module. All the
> modules where treated the same in this
> regard - they could all have options.
>
> If ucode never has options, I can remove this, but this would be a
> change in behavior.
The EFI config file documentation specifies just the filename, so I
have simplified this
in the next version.
>
>
>>>
>>>> @@ -575,3 +576,30 @@ static void __init efi_arch_memory(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\r\n");
>>>
>>> Iirc I said this before, but just in case: No explicit newline on strings
>>> passed to blexit() please.
>> Yes.
>>>
>>> Jan
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH V5 10/15] Add arch specific module handling to read_file()
2014-09-24 1:23 ` Roy Franz
2014-09-24 4:43 ` Roy Franz
@ 2014-09-24 8:18 ` Jan Beulich
1 sibling, 0 replies; 63+ messages in thread
From: Jan Beulich @ 2014-09-24 8:18 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei
>>> On 24.09.14 at 03:23, <roy.franz@linaro.org> wrote:
> On Mon, Sep 22, 2014 at 6:57 PM, Roy Franz <roy.franz@linaro.org> wrote:
>> On Mon, Sep 22, 2014 at 5:44 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 19.09.14 at 00:50, <roy.franz@linaro.org> wrote:
>>>> @@ -274,8 +275,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);
>>>> + options = split_string(name.s);
>>>> + read_file(dir_handle, &ucode, s2w(&name), options);
>>>
>>> Perhaps rather NULL instead of options?
>>
>> Yup.
>>
> OK, I looked at this a little more, and the original code would take
> anything after the ucode filename
> and put it into the mbi.string field for the ucode module. All the
> modules where treated the same in this
> regard - they could all have options.
>
> If ucode never has options, I can remove this, but this would be a
> change in behavior.
This was done that way just to avoid extra conditionals. The resulting
mbi.string doesn't get consumed anywhere (just like for initrd and
xsm, albeit in the latter case one could at least envision a potential
future use).
Jan
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH V5 11/15] Add several misc. arch functions for EFI boot code.
2014-09-18 22:49 [PATCH V5 00/15] arm64 EFI stub Roy Franz
` (9 preceding siblings ...)
2014-09-18 22:50 ` [PATCH V5 10/15] Add arch specific module handling to read_file() Roy Franz
@ 2014-09-18 22:50 ` Roy Franz
2014-09-22 12:45 ` Jan Beulich
2014-09-18 22:50 ` [PATCH V5 12/15] Add efi_arch_use_config_file() function to control use of config file Roy Franz
` (3 subsequent siblings)
14 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-18 22:50 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>
---
xen/common/efi/boot.c | 18 ++++--------------
xen/include/asm-x86/efi-boot.h | 25 +++++++++++++++++++++++++
2 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 4acab40..f4ef308 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -207,11 +207,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.w);
@@ -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 )
diff --git a/xen/include/asm-x86/efi-boot.h b/xen/include/asm-x86/efi-boot.h
index 3902b6c..2a3a2b6 100644
--- a/xen/include/asm-x86/efi-boot.h
+++ b/xen/include/asm-x86/efi-boot.h
@@ -603,3 +603,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;
+}
--
2.1.0
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH V5 12/15] Add efi_arch_use_config_file() function to control use of config file
2014-09-18 22:49 [PATCH V5 00/15] arm64 EFI stub Roy Franz
` (10 preceding siblings ...)
2014-09-18 22:50 ` [PATCH V5 11/15] Add several misc. arch functions for EFI boot code Roy Franz
@ 2014-09-18 22:50 ` Roy Franz
2014-09-22 12:48 ` Jan Beulich
2014-09-18 22:50 ` [PATCH V5 13/15] add arm64 cache flushing code from linux v3.16 Roy Franz
` (2 subsequent siblings)
14 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-18 22:50 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>
---
xen/common/efi/boot.c | 178 +++++++++++++++++++++--------------------
xen/include/asm-x86/efi-boot.h | 14 ++++
2 files changed, 106 insertions(+), 86 deletions(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index f4ef308..006a73c 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -684,7 +684,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, file_name, cfg_file_name = {NULL};
bool_t base_video = 0;
UINT32 mmap_desc_ver = 0;
@@ -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.w);
-
argc = get_argv(0, NULL, loaded_image->LoadOptions,
loaded_image->LoadOptionsSize, &options);
if ( argc > 0 &&
@@ -809,109 +805,119 @@ 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.w )
+ if ( efi_arch_use_config_file(SystemTable) )
{
- CHAR16 *tail;
+ EFI_FILE_HANDLE dir_handle;
+ /* Get the file system interface. */
+ dir_handle = get_parent_handle(loaded_image, &file_name.w);
- while ( (tail = point_tail(file_name.w)) != NULL )
+ /* Read and parse the config file. */
+ if ( !cfg_file_name.w )
{
- wstrcpy(tail, L".cfg");
- if ( read_file(dir_handle, &cfg, file_name.w, NULL) )
- break;
- *tail = 0;
+ CHAR16 *tail;
+
+ while ( (tail = point_tail(file_name.w)) != NULL )
+ {
+ wstrcpy(tail, L".cfg");
+ if ( read_file(dir_handle, &cfg, file_name.w, NULL) )
+ break;
+ *tail = 0;
+ }
+ if ( !tail )
+ blexit(L"No configuration file found.");
+ PrintStr(L"Using configuration file '");
+ PrintStr(file_name.w);
+ PrintStr(L"'\r\n");
}
- if ( !tail )
- blexit(L"No configuration file found.");
- PrintStr(L"Using configuration file '");
- PrintStr(file_name.w);
- PrintStr(L"'\r\n");
- }
- else if ( !read_file(dir_handle, &cfg, cfg_file_name.w, NULL) )
- blexit(L"Configuration file not found.");
- pre_parse(&cfg);
+ else if ( !read_file(dir_handle, &cfg, cfg_file_name.w, NULL) )
+ blexit(L"Configuration file not found.");
+ pre_parse(&cfg);
- if ( section.w )
- w2s(§ion);
- else
- section.s = get_value(&cfg, "global", "default");
+ if ( section.w )
+ w2s(§ion);
+ 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, &cfg, s2w(&name), 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, &cfg, s2w(&name), 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, &kernel, s2w(&name), 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, &ramdisk, s2w(&name), option_str);
+ read_file(dir_handle, &kernel, s2w(&name), 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, &xsm, s2w(&name), 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, &ramdisk, s2w(&name), 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, &xsm, s2w(&name), option_str);
+ efi_bs->FreePool(name.w);
}
- }
- efi_arch_cfg_file_late(dir_handle, section.s);
- efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
- cfg.addr = 0;
+ name.s = get_value(&cfg, section.s, "options");
+ efi_arch_handle_cmdline(argc ? *argv : NULL, options, name.s);
+
+ 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) )
+ {
+ 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;
+ }
+ }
- dir_handle->Close(dir_handle);
+ efi_arch_cfg_file_late(dir_handle, section.s);
+
+ efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
+ cfg.addr = 0;
+
+ dir_handle->Close(dir_handle);
+
+ }
if ( gop && !base_video )
{
diff --git a/xen/include/asm-x86/efi-boot.h b/xen/include/asm-x86/efi-boot.h
index 2a3a2b6..3ff593d 100644
--- a/xen/include/asm-x86/efi-boot.h
+++ b/xen/include/asm-x86/efi-boot.h
@@ -628,3 +628,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 __init bool_t 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:
+ */
--
2.1.0
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH V5 12/15] Add efi_arch_use_config_file() function to control use of config file
2014-09-18 22:50 ` [PATCH V5 12/15] Add efi_arch_use_config_file() function to control use of config file Roy Franz
@ 2014-09-22 12:48 ` Jan Beulich
2014-09-23 1:59 ` Roy Franz
0 siblings, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2014-09-22 12:48 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, ian.campbell, tim, xen-devel, stefano.stabellini, fu.wei
>>> On 19.09.14 at 00:50, <roy.franz@linaro.org> wrote:
> 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>
with two formatting change requests:
> @@ -809,109 +805,119 @@ 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.w )
> + if ( efi_arch_use_config_file(SystemTable) )
> {
> - CHAR16 *tail;
> + EFI_FILE_HANDLE dir_handle;
> + /* Get the file system interface. */
> + dir_handle = get_parent_handle(loaded_image, &file_name.w);
Blank line between declarations and statements please.
> --- a/xen/include/asm-x86/efi-boot.h
> +++ b/xen/include/asm-x86/efi-boot.h
> @@ -628,3 +628,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 __init bool_t efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)
static <type> <attributes> <name>
please (i.e. __init and bool_t ought to switch places).
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH V5 12/15] Add efi_arch_use_config_file() function to control use of config file
2014-09-22 12:48 ` Jan Beulich
@ 2014-09-23 1:59 ` Roy Franz
0 siblings, 0 replies; 63+ messages in thread
From: Roy Franz @ 2014-09-23 1:59 UTC (permalink / raw)
To: Jan Beulich
Cc: keir, Ian Campbell, tim, xen-devel, Stefano Stabellini, Fu Wei
On Mon, Sep 22, 2014 at 5:48 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 19.09.14 at 00:50, <roy.franz@linaro.org> wrote:
>> 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>
> with two formatting change requests:
>
>> @@ -809,109 +805,119 @@ 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.w )
>> + if ( efi_arch_use_config_file(SystemTable) )
>> {
>> - CHAR16 *tail;
>> + EFI_FILE_HANDLE dir_handle;
>> + /* Get the file system interface. */
>> + dir_handle = get_parent_handle(loaded_image, &file_name.w);
>
> Blank line between declarations and statements please.
I reviewed all the functions - forgot to check all the other blocks....
>
>> --- a/xen/include/asm-x86/efi-boot.h
>> +++ b/xen/include/asm-x86/efi-boot.h
>> @@ -628,3 +628,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 __init bool_t efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)
>
> static <type> <attributes> <name>
>
> please (i.e. __init and bool_t ought to switch places).
>
I'll fix this..
Thanks,
Roy
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH V5 13/15] add arm64 cache flushing code from linux v3.16
2014-09-18 22:49 [PATCH V5 00/15] arm64 EFI stub Roy Franz
` (11 preceding siblings ...)
2014-09-18 22:50 ` [PATCH V5 12/15] Add efi_arch_use_config_file() function to control use of config file Roy Franz
@ 2014-09-18 22:50 ` Roy Franz
2014-09-22 10:54 ` Ian Campbell
2014-09-18 22:50 ` [PATCH V5 14/15] Update libfdt to v1.4.0 Roy Franz
2014-09-18 22:50 ` [PATCH V5 15/15] Add ARM EFI boot support Roy Franz
14 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-18 22:50 UTC (permalink / raw)
To: xen-devel, ian.campbell, stefano.stabellini, tim, jbeulich, keir
Cc: Roy Franz, fu.wei
__flush_dcache_all added from arch/arm64/mm/cache.S, with helper macros from
arch/arm64/include/asm/assembler.h, from v3.16. The cache flushing is required
when transitioning from EFI code that runs with cache enable to Xen startup
code which expects the cache to be disabled.
Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
xen/arch/arm/arm64/Makefile | 1 +
xen/arch/arm/arm64/cache.S | 100 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 101 insertions(+)
create mode 100644 xen/arch/arm/arm64/cache.S
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index d2d5875..c7243f5 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -7,5 +7,6 @@ obj-y += domain.o
obj-y += vfp.o
obj-y += smpboot.o
obj-y += domctl.o
+obj-y += cache.o
obj-$(EARLY_PRINTK) += debug.o
diff --git a/xen/arch/arm/arm64/cache.S b/xen/arch/arm/arm64/cache.S
new file mode 100644
index 0000000..fb6dff1
--- /dev/null
+++ b/xen/arch/arm/arm64/cache.S
@@ -0,0 +1,100 @@
+/*
+ * Cache maintenance
+ *
+ * Copyright (C) 2001 Deep Blue Solutions Ltd.
+ * Copyright (C) 2012 ARM Ltd.
+ * Copyright (C) 1996-2000 Russell King
+ * Copyright (C) 2012 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * Enable and disable interrupts.
+ */
+ .macro disable_irq
+ msr daifset, #2
+ .endm
+
+ .macro enable_irq
+ msr daifclr, #2
+ .endm
+
+/*
+ * Save/disable and restore interrupts.
+ */
+ .macro save_and_disable_irqs, olddaif
+ mrs \olddaif, daif
+ disable_irq
+ .endm
+
+ .macro restore_irqs, olddaif
+ msr daif, \olddaif
+ .endm
+
+/*
+ * __flush_dcache_all()
+ *
+ * Flush the whole D-cache.
+ *
+ * Corrupted registers: x0-x7, x9-x11
+ */
+ ENTRY(__flush_dcache_all)
+__flush_dcache_all:
+ dmb sy // ensure ordering with previous memory accesses
+ mrs x0, clidr_el1 // read clidr
+ and x3, x0, #0x7000000 // extract loc from clidr
+ lsr x3, x3, #23 // left align loc bit field
+ cbz x3, finished // if loc is 0, then no need to clean
+ mov x10, #0 // start clean at cache level 0
+loop1:
+ add x2, x10, x10, lsr #1 // work out 3x current cache level
+ lsr x1, x0, x2 // extract cache type bits from clidr
+ and x1, x1, #7 // mask of the bits for current cache only
+ cmp x1, #2 // see what cache we have at this level
+ b.lt skip // skip if no cache, or just i-cache
+ save_and_disable_irqs x9 // make CSSELR and CCSIDR access atomic
+ msr csselr_el1, x10 // select current cache level in csselr
+ isb // isb to sych the new cssr&csidr
+ mrs x1, ccsidr_el1 // read the new ccsidr
+ restore_irqs x9
+ and x2, x1, #7 // extract the length of the cache lines
+ add x2, x2, #4 // add 4 (line length offset)
+ mov x4, #0x3ff
+ and x4, x4, x1, lsr #3 // find maximum number on the way size
+ clz w5, w4 // find bit position of way size increment
+ mov x7, #0x7fff
+ and x7, x7, x1, lsr #13 // extract max number of the index size
+loop2:
+ mov x9, x4 // create working copy of max way size
+loop3:
+ lsl x6, x9, x5
+ orr x11, x10, x6 // factor way and cache number into x11
+ lsl x6, x7, x2
+ orr x11, x11, x6 // factor index number into x11
+ dc cisw, x11 // clean & invalidate by set/way
+ subs x9, x9, #1 // decrement the way
+ b.ge loop3
+ subs x7, x7, #1 // decrement the index
+ b.ge loop2
+skip:
+ add x10, x10, #2 // increment cache number
+ cmp x3, x10
+ b.gt loop1
+finished:
+ mov x10, #0 // swith back to cache level 0
+ msr csselr_el1, x10 // select current cache level in csselr
+ dsb sy
+ isb
+ ret
+ENDPROC(__flush_dcache_all)
--
2.1.0
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH V5 13/15] add arm64 cache flushing code from linux v3.16
2014-09-18 22:50 ` [PATCH V5 13/15] add arm64 cache flushing code from linux v3.16 Roy Franz
@ 2014-09-22 10:54 ` Ian Campbell
2014-09-22 23:42 ` Roy Franz
0 siblings, 1 reply; 63+ messages in thread
From: Ian Campbell @ 2014-09-22 10:54 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, tim, xen-devel, stefano.stabellini, jbeulich, fu.wei
On Thu, 2014-09-18 at 15:50 -0700, Roy Franz wrote:
> __flush_dcache_all added from arch/arm64/mm/cache.S, with helper macros from
> arch/arm64/include/asm/assembler.h, from v3.16. The cache flushing is required
> when transitioning from EFI code that runs with cache enable to Xen startup
> code which expects the cache to be disabled.
>
> Signed-off-by: Roy Franz <roy.franz@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
[...]
> + ENTRY(__flush_dcache_all)
If you need to repost please unindent this (if there's no need to repost
then I can do it on commit).
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH V5 13/15] add arm64 cache flushing code from linux v3.16
2014-09-22 10:54 ` Ian Campbell
@ 2014-09-22 23:42 ` Roy Franz
2014-09-23 7:42 ` Ian Campbell
0 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-22 23:42 UTC (permalink / raw)
To: Ian Campbell
Cc: keir, tim, xen-devel, Stefano Stabellini, Jan Beulich, Fu Wei
On Mon, Sep 22, 2014 at 3:54 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Thu, 2014-09-18 at 15:50 -0700, Roy Franz wrote:
>> __flush_dcache_all added from arch/arm64/mm/cache.S, with helper macros from
>> arch/arm64/include/asm/assembler.h, from v3.16. The cache flushing is required
>> when transitioning from EFI code that runs with cache enable to Xen startup
>> code which expects the cache to be disabled.
>>
>> Signed-off-by: Roy Franz <roy.franz@linaro.org>
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> [...]
>
>> + ENTRY(__flush_dcache_all)
>
> If you need to repost please unindent this (if there's no need to repost
> then I can do it on commit).
>
>
I'll be reposting a new series to address other feedback, so I'll include this.
Roy
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH V5 13/15] add arm64 cache flushing code from linux v3.16
2014-09-22 23:42 ` Roy Franz
@ 2014-09-23 7:42 ` Ian Campbell
0 siblings, 0 replies; 63+ messages in thread
From: Ian Campbell @ 2014-09-23 7:42 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, tim, xen-devel, Stefano Stabellini, Jan Beulich, Fu Wei
On Mon, 2014-09-22 at 16:42 -0700, Roy Franz wrote:
> On Mon, Sep 22, 2014 at 3:54 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Thu, 2014-09-18 at 15:50 -0700, Roy Franz wrote:
> >> __flush_dcache_all added from arch/arm64/mm/cache.S, with helper macros from
> >> arch/arm64/include/asm/assembler.h, from v3.16. The cache flushing is required
> >> when transitioning from EFI code that runs with cache enable to Xen startup
> >> code which expects the cache to be disabled.
> >>
> >> Signed-off-by: Roy Franz <roy.franz@linaro.org>
> >
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > [...]
> >
> >> + ENTRY(__flush_dcache_all)
> >
> > If you need to repost please unindent this (if there's no need to repost
> > then I can do it on commit).
> >
> >
> I'll be reposting a new series to address other feedback, so I'll include this.
No need, I applied this one already (and fixed it up)
Ian.
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH V5 14/15] Update libfdt to v1.4.0
2014-09-18 22:49 [PATCH V5 00/15] arm64 EFI stub Roy Franz
` (12 preceding siblings ...)
2014-09-18 22:50 ` [PATCH V5 13/15] add arm64 cache flushing code from linux v3.16 Roy Franz
@ 2014-09-18 22:50 ` Roy Franz
2014-09-22 11:20 ` Ian Campbell
2014-09-18 22:50 ` [PATCH V5 15/15] Add ARM EFI boot support Roy Franz
14 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-18 22:50 UTC (permalink / raw)
To: xen-devel, ian.campbell, stefano.stabellini, tim, jbeulich, keir
Cc: Roy Franz, fu.wei
Update libfdt to v1.4.0 of libfdt taken from git://git.jdl.com/software/dtc.git
Xen changes to libfdt_env.h carried over from existing libfdt (v1.3.0)
This update provides the fdt_create_empty_tree() function used by the ARM
EFI boot code.
Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
xen/common/libfdt/Makefile.libfdt | 4 +-
xen/common/libfdt/fdt.c | 30 +++-
xen/common/libfdt/fdt_empty_tree.c | 84 ++++++++++
xen/common/libfdt/fdt_ro.c | 7 +-
xen/common/libfdt/fdt_rw.c | 31 +++-
xen/common/libfdt/fdt_sw.c | 4 +-
xen/common/libfdt/fdt_wip.c | 2 +-
xen/common/libfdt/version.lds | 6 +
xen/include/xen/libfdt/fdt.h | 93 ++++++++---
xen/include/xen/libfdt/libfdt.h | 315 +++++++++++++++++++++++++++++++++---
xen/include/xen/libfdt/libfdt_env.h | 4 +
11 files changed, 529 insertions(+), 51 deletions(-)
create mode 100644 xen/common/libfdt/fdt_empty_tree.c
diff --git a/xen/common/libfdt/Makefile.libfdt b/xen/common/libfdt/Makefile.libfdt
index d55a6f8..91126c0 100644
--- a/xen/common/libfdt/Makefile.libfdt
+++ b/xen/common/libfdt/Makefile.libfdt
@@ -4,7 +4,7 @@
# be easily embeddable into other systems of Makefiles.
#
LIBFDT_soname = libfdt.$(SHAREDLIB_EXT).1
-LIBFDT_INCLUDES = fdt.h libfdt.h
+LIBFDT_INCLUDES = fdt.h libfdt.h libfdt_env.h
LIBFDT_VERSION = version.lds
-LIBFDT_SRCS = fdt.c fdt_ro.c fdt_wip.c fdt_sw.c fdt_rw.c fdt_strerror.c
+LIBFDT_SRCS = fdt.c fdt_ro.c fdt_wip.c fdt_sw.c fdt_rw.c fdt_strerror.c fdt_empty_tree.c
LIBFDT_OBJS = $(LIBFDT_SRCS:%.c=%.o)
diff --git a/xen/common/libfdt/fdt.c b/xen/common/libfdt/fdt.c
index e56833a..2ce6a44 100644
--- a/xen/common/libfdt/fdt.c
+++ b/xen/common/libfdt/fdt.c
@@ -92,7 +92,7 @@ const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len)
uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
{
- const uint32_t *tagp, *lenp;
+ const fdt32_t *tagp, *lenp;
uint32_t tag;
int offset = startoffset;
const char *p;
@@ -198,6 +198,34 @@ int fdt_next_node(const void *fdt, int offset, int *depth)
return offset;
}
+int fdt_first_subnode(const void *fdt, int offset)
+{
+ int depth = 0;
+
+ offset = fdt_next_node(fdt, offset, &depth);
+ if (offset < 0 || depth != 1)
+ return -FDT_ERR_NOTFOUND;
+
+ return offset;
+}
+
+int fdt_next_subnode(const void *fdt, int offset)
+{
+ int depth = 1;
+
+ /*
+ * With respect to the parent, the depth of the next subnode will be
+ * the same as the last.
+ */
+ do {
+ offset = fdt_next_node(fdt, offset, &depth);
+ if (offset < 0 || depth < 1)
+ return -FDT_ERR_NOTFOUND;
+ } while (depth > 1);
+
+ return offset;
+}
+
const char *_fdt_find_string(const char *strtab, int tabsize, const char *s)
{
int len = strlen(s) + 1;
diff --git a/xen/common/libfdt/fdt_empty_tree.c b/xen/common/libfdt/fdt_empty_tree.c
new file mode 100644
index 0000000..f72d13b
--- /dev/null
+++ b/xen/common/libfdt/fdt_empty_tree.c
@@ -0,0 +1,84 @@
+/*
+ * libfdt - Flat Device Tree manipulation
+ * Copyright (C) 2012 David Gibson, IBM Corporation.
+ *
+ * libfdt is dual licensed: you can use it either under the terms of
+ * the GPL, or the BSD license, at your option.
+ *
+ * a) This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this library; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ *
+ * Alternatively,
+ *
+ * b) Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer.
+ * 2. Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials
+ * provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+ * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
+ * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#include "libfdt_env.h"
+
+#include <fdt.h>
+#include <libfdt.h>
+
+#include "libfdt_internal.h"
+
+int fdt_create_empty_tree(void *buf, int bufsize)
+{
+ int err;
+
+ err = fdt_create(buf, bufsize);
+ if (err)
+ return err;
+
+ err = fdt_finish_reservemap(buf);
+ if (err)
+ return err;
+
+ err = fdt_begin_node(buf, "");
+ if (err)
+ return err;
+
+ err = fdt_end_node(buf);
+ if (err)
+ return err;
+
+ err = fdt_finish(buf);
+ if (err)
+ return err;
+
+ return fdt_open_into(buf, buf, bufsize);
+}
+
diff --git a/xen/common/libfdt/fdt_ro.c b/xen/common/libfdt/fdt_ro.c
index 02b6d68..50007f6 100644
--- a/xen/common/libfdt/fdt_ro.c
+++ b/xen/common/libfdt/fdt_ro.c
@@ -322,7 +322,7 @@ const void *fdt_getprop(const void *fdt, int nodeoffset,
uint32_t fdt_get_phandle(const void *fdt, int nodeoffset)
{
- const uint32_t *php;
+ const fdt32_t *php;
int len;
/* FIXME: This is a bit sub-optimal, since we potentially scan
@@ -515,8 +515,7 @@ int fdt_node_offset_by_phandle(const void *fdt, uint32_t phandle)
return offset; /* error from fdt_next_node() */
}
-static int _fdt_stringlist_contains(const char *strlist, int listlen,
- const char *str)
+int fdt_stringlist_contains(const char *strlist, int listlen, const char *str)
{
int len = strlen(str);
const char *p;
@@ -542,7 +541,7 @@ int fdt_node_check_compatible(const void *fdt, int nodeoffset,
prop = fdt_getprop(fdt, nodeoffset, "compatible", &len);
if (!prop)
return len;
- if (_fdt_stringlist_contains(prop, len, compatible))
+ if (fdt_stringlist_contains(prop, len, compatible))
return 0;
else
return 1;
diff --git a/xen/common/libfdt/fdt_rw.c b/xen/common/libfdt/fdt_rw.c
index 994037b..fdba618 100644
--- a/xen/common/libfdt/fdt_rw.c
+++ b/xen/common/libfdt/fdt_rw.c
@@ -289,6 +289,33 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
return 0;
}
+int fdt_appendprop(void *fdt, int nodeoffset, const char *name,
+ const void *val, int len)
+{
+ struct fdt_property *prop;
+ int err, oldlen, newlen;
+
+ FDT_RW_CHECK_HEADER(fdt);
+
+ prop = fdt_get_property_w(fdt, nodeoffset, name, &oldlen);
+ if (prop) {
+ newlen = len + oldlen;
+ err = _fdt_splice_struct(fdt, prop->data,
+ FDT_TAGALIGN(oldlen),
+ FDT_TAGALIGN(newlen));
+ if (err)
+ return err;
+ prop->len = cpu_to_fdt32(newlen);
+ memcpy(prop->data + oldlen, val, len);
+ } else {
+ err = _fdt_add_property(fdt, nodeoffset, name, len, &prop);
+ if (err)
+ return err;
+ memcpy(prop->data, val, len);
+ }
+ return 0;
+}
+
int fdt_delprop(void *fdt, int nodeoffset, const char *name)
{
struct fdt_property *prop;
@@ -312,7 +339,7 @@ int fdt_add_subnode_namelen(void *fdt, int parentoffset,
int nodelen;
int err;
uint32_t tag;
- uint32_t *endtag;
+ fdt32_t *endtag;
FDT_RW_CHECK_HEADER(fdt);
@@ -339,7 +366,7 @@ int fdt_add_subnode_namelen(void *fdt, int parentoffset,
nh->tag = cpu_to_fdt32(FDT_BEGIN_NODE);
memset(nh->name, 0, FDT_TAGALIGN(namelen+1));
memcpy(nh->name, name, namelen);
- endtag = (uint32_t *)((char *)nh + nodelen - FDT_TAGSIZE);
+ endtag = (fdt32_t *)((char *)nh + nodelen - FDT_TAGSIZE);
*endtag = cpu_to_fdt32(FDT_END_NODE);
return offset;
diff --git a/xen/common/libfdt/fdt_sw.c b/xen/common/libfdt/fdt_sw.c
index 55ebebf..f422754 100644
--- a/xen/common/libfdt/fdt_sw.c
+++ b/xen/common/libfdt/fdt_sw.c
@@ -153,7 +153,7 @@ int fdt_begin_node(void *fdt, const char *name)
int fdt_end_node(void *fdt)
{
- uint32_t *en;
+ fdt32_t *en;
FDT_SW_CHECK_HEADER(fdt);
@@ -213,7 +213,7 @@ int fdt_property(void *fdt, const char *name, const void *val, int len)
int fdt_finish(void *fdt)
{
char *p = (char *)fdt;
- uint32_t *end;
+ fdt32_t *end;
int oldstroffset, newstroffset;
uint32_t tag;
int offset, nextoffset;
diff --git a/xen/common/libfdt/fdt_wip.c b/xen/common/libfdt/fdt_wip.c
index 6025fa1..c5bbb68 100644
--- a/xen/common/libfdt/fdt_wip.c
+++ b/xen/common/libfdt/fdt_wip.c
@@ -74,7 +74,7 @@ int fdt_setprop_inplace(void *fdt, int nodeoffset, const char *name,
static void _fdt_nop_region(void *start, int len)
{
- uint32_t *p;
+ fdt32_t *p;
for (p = start; (char *)p < ((char *)start + len); p++)
*p = cpu_to_fdt32(FDT_NOP);
diff --git a/xen/common/libfdt/version.lds b/xen/common/libfdt/version.lds
index 3c3994e..80b322b 100644
--- a/xen/common/libfdt/version.lds
+++ b/xen/common/libfdt/version.lds
@@ -48,6 +48,12 @@ LIBFDT_1.2 {
fdt_strerror;
fdt_offset_ptr;
fdt_next_tag;
+ fdt_appendprop;
+ fdt_create_empty_tree;
+ fdt_first_property_offset;
+ fdt_get_property_by_offset;
+ fdt_getprop_by_offset;
+ fdt_next_property_offset;
local:
*;
diff --git a/xen/include/xen/libfdt/fdt.h b/xen/include/xen/libfdt/fdt.h
index 48ccfd9..526aedb 100644
--- a/xen/include/xen/libfdt/fdt.h
+++ b/xen/include/xen/libfdt/fdt.h
@@ -1,48 +1,99 @@
#ifndef _FDT_H
#define _FDT_H
+/*
+ * libfdt - Flat Device Tree manipulation
+ * Copyright (C) 2006 David Gibson, IBM Corporation.
+ * Copyright 2012 Kim Phillips, Freescale Semiconductor.
+ *
+ * libfdt is dual licensed: you can use it either under the terms of
+ * the GPL, or the BSD license, at your option.
+ *
+ * a) This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this library; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ *
+ * Alternatively,
+ *
+ * b) Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer.
+ * 2. Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials
+ * provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+ * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
+ * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
#ifndef __ASSEMBLY__
struct fdt_header {
- uint32_t magic; /* magic word FDT_MAGIC */
- uint32_t totalsize; /* total size of DT block */
- uint32_t off_dt_struct; /* offset to structure */
- uint32_t off_dt_strings; /* offset to strings */
- uint32_t off_mem_rsvmap; /* offset to memory reserve map */
- uint32_t version; /* format version */
- uint32_t last_comp_version; /* last compatible version */
+ fdt32_t magic; /* magic word FDT_MAGIC */
+ fdt32_t totalsize; /* total size of DT block */
+ fdt32_t off_dt_struct; /* offset to structure */
+ fdt32_t off_dt_strings; /* offset to strings */
+ fdt32_t off_mem_rsvmap; /* offset to memory reserve map */
+ fdt32_t version; /* format version */
+ fdt32_t last_comp_version; /* last compatible version */
/* version 2 fields below */
- uint32_t boot_cpuid_phys; /* Which physical CPU id we're
+ fdt32_t boot_cpuid_phys; /* Which physical CPU id we're
booting on */
/* version 3 fields below */
- uint32_t size_dt_strings; /* size of the strings block */
+ fdt32_t size_dt_strings; /* size of the strings block */
/* version 17 fields below */
- uint32_t size_dt_struct; /* size of the structure block */
+ fdt32_t size_dt_struct; /* size of the structure block */
};
struct fdt_reserve_entry {
- uint64_t address;
- uint64_t size;
+ fdt64_t address;
+ fdt64_t size;
};
struct fdt_node_header {
- uint32_t tag;
+ fdt32_t tag;
char name[0];
};
struct fdt_property {
- uint32_t tag;
- uint32_t len;
- uint32_t nameoff;
+ fdt32_t tag;
+ fdt32_t len;
+ fdt32_t nameoff;
char data[0];
};
#endif /* !__ASSEMBLY */
#define FDT_MAGIC 0xd00dfeed /* 4: version, 4: total size */
-#define FDT_TAGSIZE sizeof(uint32_t)
+#define FDT_TAGSIZE sizeof(fdt32_t)
#define FDT_BEGIN_NODE 0x1 /* Start node: full name */
#define FDT_END_NODE 0x2 /* End node */
@@ -51,10 +102,10 @@ struct fdt_property {
#define FDT_NOP 0x4 /* nop */
#define FDT_END 0x9
-#define FDT_V1_SIZE (7*sizeof(uint32_t))
-#define FDT_V2_SIZE (FDT_V1_SIZE + sizeof(uint32_t))
-#define FDT_V3_SIZE (FDT_V2_SIZE + sizeof(uint32_t))
+#define FDT_V1_SIZE (7*sizeof(fdt32_t))
+#define FDT_V2_SIZE (FDT_V1_SIZE + sizeof(fdt32_t))
+#define FDT_V3_SIZE (FDT_V2_SIZE + sizeof(fdt32_t))
#define FDT_V16_SIZE FDT_V3_SIZE
-#define FDT_V17_SIZE (FDT_V16_SIZE + sizeof(uint32_t))
+#define FDT_V17_SIZE (FDT_V16_SIZE + sizeof(fdt32_t))
#endif /* _FDT_H */
diff --git a/xen/include/xen/libfdt/libfdt.h b/xen/include/xen/libfdt/libfdt.h
index 6086047..37349f1 100644
--- a/xen/include/xen/libfdt/libfdt.h
+++ b/xen/include/xen/libfdt/libfdt.h
@@ -136,6 +136,28 @@ uint32_t fdt_next_tag(const void *fdt, int offset, int *nextoffset);
int fdt_next_node(const void *fdt, int offset, int *depth);
+/**
+ * fdt_first_subnode() - get offset of first direct subnode
+ *
+ * @fdt: FDT blob
+ * @offset: Offset of node to check
+ * @return offset of first subnode, or -FDT_ERR_NOTFOUND if there is none
+ */
+int fdt_first_subnode(const void *fdt, int offset);
+
+/**
+ * fdt_next_subnode() - get offset of next direct subnode
+ *
+ * After first calling fdt_first_subnode(), call this function repeatedly to
+ * get direct subnodes of a parent node.
+ *
+ * @fdt: FDT blob
+ * @offset: Offset of previous subnode
+ * @return offset of next subnode, or -FDT_ERR_NOTFOUND if there are no more
+ * subnodes
+ */
+int fdt_next_subnode(const void *fdt, int offset);
+
/**********************************************************************/
/* General functions */
/**********************************************************************/
@@ -582,7 +604,7 @@ const char *fdt_get_alias_namelen(const void *fdt,
* value of the property named 'name' in the node /aliases.
*
* returns:
- * a pointer to the expansion of the alias named 'name', of it exists
+ * a pointer to the expansion of the alias named 'name', if it exists
* NULL, if the given alias or the /aliases node does not exist
*/
const char *fdt_get_alias(const void *fdt, const char *name);
@@ -816,6 +838,20 @@ int fdt_node_check_compatible(const void *fdt, int nodeoffset,
int fdt_node_offset_by_compatible(const void *fdt, int startoffset,
const char *compatible);
+/**
+ * fdt_stringlist_contains - check a string list property for a string
+ * @strlist: Property containing a list of strings to check
+ * @listlen: Length of property
+ * @str: String to search for
+ *
+ * This is a utility function provided for convenience. The list contains
+ * one or more strings, each terminated by \0, as is found in a device tree
+ * "compatible" property.
+ *
+ * @return: 1 if the string is found in the list, 0 not found, or invalid list
+ */
+int fdt_stringlist_contains(const char *strlist, int listlen, const char *str);
+
/**********************************************************************/
/* Write-in-place functions */
/**********************************************************************/
@@ -852,17 +888,17 @@ int fdt_setprop_inplace(void *fdt, int nodeoffset, const char *name,
const void *val, int len);
/**
- * fdt_setprop_inplace_cell - change the value of a single-cell property
+ * fdt_setprop_inplace_u32 - change the value of a 32-bit integer property
* @fdt: pointer to the device tree blob
* @nodeoffset: offset of the node whose property to change
* @name: name of the property to change
- * @val: cell (32-bit integer) value to replace the property with
+ * @val: 32-bit integer value to replace the property with
*
- * fdt_setprop_inplace_cell() replaces the value of a given property
- * with the 32-bit integer cell value in val, converting val to
- * big-endian if necessary. This function cannot change the size of a
- * property, and so will only work if the property already exists and
- * has length 4.
+ * fdt_setprop_inplace_u32() replaces the value of a given property
+ * with the 32-bit integer value in val, converting val to big-endian
+ * if necessary. This function cannot change the size of a property,
+ * and so will only work if the property already exists and has length
+ * 4.
*
* This function will alter only the bytes in the blob which contain
* the given property value, and will not alter or move any other part
@@ -871,7 +907,42 @@ int fdt_setprop_inplace(void *fdt, int nodeoffset, const char *name,
* returns:
* 0, on success
* -FDT_ERR_NOSPACE, if the property's length is not equal to 4
- * -FDT_ERR_NOTFOUND, node does not have the named property
+ * -FDT_ERR_NOTFOUND, node does not have the named property
+ * -FDT_ERR_BADOFFSET, nodeoffset did not point to FDT_BEGIN_NODE tag
+ * -FDT_ERR_BADMAGIC,
+ * -FDT_ERR_BADVERSION,
+ * -FDT_ERR_BADSTATE,
+ * -FDT_ERR_BADSTRUCTURE,
+ * -FDT_ERR_TRUNCATED, standard meanings
+ */
+static inline int fdt_setprop_inplace_u32(void *fdt, int nodeoffset,
+ const char *name, uint32_t val)
+{
+ fdt32_t tmp = cpu_to_fdt32(val);
+ return fdt_setprop_inplace(fdt, nodeoffset, name, &tmp, sizeof(tmp));
+}
+
+/**
+ * fdt_setprop_inplace_u64 - change the value of a 64-bit integer property
+ * @fdt: pointer to the device tree blob
+ * @nodeoffset: offset of the node whose property to change
+ * @name: name of the property to change
+ * @val: 64-bit integer value to replace the property with
+ *
+ * fdt_setprop_inplace_u64() replaces the value of a given property
+ * with the 64-bit integer value in val, converting val to big-endian
+ * if necessary. This function cannot change the size of a property,
+ * and so will only work if the property already exists and has length
+ * 8.
+ *
+ * This function will alter only the bytes in the blob which contain
+ * the given property value, and will not alter or move any other part
+ * of the tree.
+ *
+ * returns:
+ * 0, on success
+ * -FDT_ERR_NOSPACE, if the property's length is not equal to 8
+ * -FDT_ERR_NOTFOUND, node does not have the named property
* -FDT_ERR_BADOFFSET, nodeoffset did not point to FDT_BEGIN_NODE tag
* -FDT_ERR_BADMAGIC,
* -FDT_ERR_BADVERSION,
@@ -879,11 +950,22 @@ int fdt_setprop_inplace(void *fdt, int nodeoffset, const char *name,
* -FDT_ERR_BADSTRUCTURE,
* -FDT_ERR_TRUNCATED, standard meanings
*/
+static inline int fdt_setprop_inplace_u64(void *fdt, int nodeoffset,
+ const char *name, uint64_t val)
+{
+ fdt64_t tmp = cpu_to_fdt64(val);
+ return fdt_setprop_inplace(fdt, nodeoffset, name, &tmp, sizeof(tmp));
+}
+
+/**
+ * fdt_setprop_inplace_cell - change the value of a single-cell property
+ *
+ * This is an alternative name for fdt_setprop_inplace_u32()
+ */
static inline int fdt_setprop_inplace_cell(void *fdt, int nodeoffset,
const char *name, uint32_t val)
{
- val = cpu_to_fdt32(val);
- return fdt_setprop_inplace(fdt, nodeoffset, name, &val, sizeof(val));
+ return fdt_setprop_inplace_u32(fdt, nodeoffset, name, val);
}
/**
@@ -945,10 +1027,19 @@ int fdt_add_reservemap_entry(void *fdt, uint64_t addr, uint64_t size);
int fdt_finish_reservemap(void *fdt);
int fdt_begin_node(void *fdt, const char *name);
int fdt_property(void *fdt, const char *name, const void *val, int len);
+static inline int fdt_property_u32(void *fdt, const char *name, uint32_t val)
+{
+ fdt32_t tmp = cpu_to_fdt32(val);
+ return fdt_property(fdt, name, &tmp, sizeof(tmp));
+}
+static inline int fdt_property_u64(void *fdt, const char *name, uint64_t val)
+{
+ fdt64_t tmp = cpu_to_fdt64(val);
+ return fdt_property(fdt, name, &tmp, sizeof(tmp));
+}
static inline int fdt_property_cell(void *fdt, const char *name, uint32_t val)
{
- val = cpu_to_fdt32(val);
- return fdt_property(fdt, name, &val, sizeof(val));
+ return fdt_property_u32(fdt, name, val);
}
#define fdt_property_string(fdt, name, str) \
fdt_property(fdt, name, str, strlen(str)+1)
@@ -959,6 +1050,7 @@ int fdt_finish(void *fdt);
/* Read-write functions */
/**********************************************************************/
+int fdt_create_empty_tree(void *buf, int bufsize);
int fdt_open_into(const void *fdt, void *buf, int bufsize);
int fdt_pack(void *fdt);
@@ -1068,14 +1160,14 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
const void *val, int len);
/**
- * fdt_setprop_cell - set a property to a single cell value
+ * fdt_setprop_u32 - set a property to a 32-bit integer
* @fdt: pointer to the device tree blob
* @nodeoffset: offset of the node whose property to change
* @name: name of the property to change
* @val: 32-bit integer value for the property (native endian)
*
- * fdt_setprop_cell() sets the value of the named property in the
- * given node to the given cell value (converting to big-endian if
+ * fdt_setprop_u32() sets the value of the named property in the given
+ * node to the given 32-bit integer value (converting to big-endian if
* necessary), or creates a new property with that value if it does
* not already exist.
*
@@ -1095,11 +1187,57 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
* -FDT_ERR_BADLAYOUT,
* -FDT_ERR_TRUNCATED, standard meanings
*/
+static inline int fdt_setprop_u32(void *fdt, int nodeoffset, const char *name,
+ uint32_t val)
+{
+ fdt32_t tmp = cpu_to_fdt32(val);
+ return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
+}
+
+/**
+ * fdt_setprop_u64 - set a property to a 64-bit integer
+ * @fdt: pointer to the device tree blob
+ * @nodeoffset: offset of the node whose property to change
+ * @name: name of the property to change
+ * @val: 64-bit integer value for the property (native endian)
+ *
+ * fdt_setprop_u64() sets the value of the named property in the given
+ * node to the given 64-bit integer value (converting to big-endian if
+ * necessary), or creates a new property with that value if it does
+ * not already exist.
+ *
+ * This function may insert or delete data from the blob, and will
+ * therefore change the offsets of some existing nodes.
+ *
+ * returns:
+ * 0, on success
+ * -FDT_ERR_NOSPACE, there is insufficient free space in the blob to
+ * contain the new property value
+ * -FDT_ERR_BADOFFSET, nodeoffset did not point to FDT_BEGIN_NODE tag
+ * -FDT_ERR_BADLAYOUT,
+ * -FDT_ERR_BADMAGIC,
+ * -FDT_ERR_BADVERSION,
+ * -FDT_ERR_BADSTATE,
+ * -FDT_ERR_BADSTRUCTURE,
+ * -FDT_ERR_BADLAYOUT,
+ * -FDT_ERR_TRUNCATED, standard meanings
+ */
+static inline int fdt_setprop_u64(void *fdt, int nodeoffset, const char *name,
+ uint64_t val)
+{
+ fdt64_t tmp = cpu_to_fdt64(val);
+ return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
+}
+
+/**
+ * fdt_setprop_cell - set a property to a single cell value
+ *
+ * This is an alternative name for fdt_setprop_u32()
+ */
static inline int fdt_setprop_cell(void *fdt, int nodeoffset, const char *name,
uint32_t val)
{
- val = cpu_to_fdt32(val);
- return fdt_setprop(fdt, nodeoffset, name, &val, sizeof(val));
+ return fdt_setprop_u32(fdt, nodeoffset, name, val);
}
/**
@@ -1134,6 +1272,147 @@ static inline int fdt_setprop_cell(void *fdt, int nodeoffset, const char *name,
fdt_setprop((fdt), (nodeoffset), (name), (str), strlen(str)+1)
/**
+ * fdt_appendprop - append to or create a property
+ * @fdt: pointer to the device tree blob
+ * @nodeoffset: offset of the node whose property to change
+ * @name: name of the property to append to
+ * @val: pointer to data to append to the property value
+ * @len: length of the data to append to the property value
+ *
+ * fdt_appendprop() appends the value to the named property in the
+ * given node, creating the property if it does not already exist.
+ *
+ * This function may insert data into the blob, and will therefore
+ * change the offsets of some existing nodes.
+ *
+ * returns:
+ * 0, on success
+ * -FDT_ERR_NOSPACE, there is insufficient free space in the blob to
+ * contain the new property value
+ * -FDT_ERR_BADOFFSET, nodeoffset did not point to FDT_BEGIN_NODE tag
+ * -FDT_ERR_BADLAYOUT,
+ * -FDT_ERR_BADMAGIC,
+ * -FDT_ERR_BADVERSION,
+ * -FDT_ERR_BADSTATE,
+ * -FDT_ERR_BADSTRUCTURE,
+ * -FDT_ERR_BADLAYOUT,
+ * -FDT_ERR_TRUNCATED, standard meanings
+ */
+int fdt_appendprop(void *fdt, int nodeoffset, const char *name,
+ const void *val, int len);
+
+/**
+ * fdt_appendprop_u32 - append a 32-bit integer value to a property
+ * @fdt: pointer to the device tree blob
+ * @nodeoffset: offset of the node whose property to change
+ * @name: name of the property to change
+ * @val: 32-bit integer value to append to the property (native endian)
+ *
+ * fdt_appendprop_u32() appends the given 32-bit integer value
+ * (converting to big-endian if necessary) to the value of the named
+ * property in the given node, or creates a new property with that
+ * value if it does not already exist.
+ *
+ * This function may insert data into the blob, and will therefore
+ * change the offsets of some existing nodes.
+ *
+ * returns:
+ * 0, on success
+ * -FDT_ERR_NOSPACE, there is insufficient free space in the blob to
+ * contain the new property value
+ * -FDT_ERR_BADOFFSET, nodeoffset did not point to FDT_BEGIN_NODE tag
+ * -FDT_ERR_BADLAYOUT,
+ * -FDT_ERR_BADMAGIC,
+ * -FDT_ERR_BADVERSION,
+ * -FDT_ERR_BADSTATE,
+ * -FDT_ERR_BADSTRUCTURE,
+ * -FDT_ERR_BADLAYOUT,
+ * -FDT_ERR_TRUNCATED, standard meanings
+ */
+static inline int fdt_appendprop_u32(void *fdt, int nodeoffset,
+ const char *name, uint32_t val)
+{
+ fdt32_t tmp = cpu_to_fdt32(val);
+ return fdt_appendprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
+}
+
+/**
+ * fdt_appendprop_u64 - append a 64-bit integer value to a property
+ * @fdt: pointer to the device tree blob
+ * @nodeoffset: offset of the node whose property to change
+ * @name: name of the property to change
+ * @val: 64-bit integer value to append to the property (native endian)
+ *
+ * fdt_appendprop_u64() appends the given 64-bit integer value
+ * (converting to big-endian if necessary) to the value of the named
+ * property in the given node, or creates a new property with that
+ * value if it does not already exist.
+ *
+ * This function may insert data into the blob, and will therefore
+ * change the offsets of some existing nodes.
+ *
+ * returns:
+ * 0, on success
+ * -FDT_ERR_NOSPACE, there is insufficient free space in the blob to
+ * contain the new property value
+ * -FDT_ERR_BADOFFSET, nodeoffset did not point to FDT_BEGIN_NODE tag
+ * -FDT_ERR_BADLAYOUT,
+ * -FDT_ERR_BADMAGIC,
+ * -FDT_ERR_BADVERSION,
+ * -FDT_ERR_BADSTATE,
+ * -FDT_ERR_BADSTRUCTURE,
+ * -FDT_ERR_BADLAYOUT,
+ * -FDT_ERR_TRUNCATED, standard meanings
+ */
+static inline int fdt_appendprop_u64(void *fdt, int nodeoffset,
+ const char *name, uint64_t val)
+{
+ fdt64_t tmp = cpu_to_fdt64(val);
+ return fdt_appendprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
+}
+
+/**
+ * fdt_appendprop_cell - append a single cell value to a property
+ *
+ * This is an alternative name for fdt_appendprop_u32()
+ */
+static inline int fdt_appendprop_cell(void *fdt, int nodeoffset,
+ const char *name, uint32_t val)
+{
+ return fdt_appendprop_u32(fdt, nodeoffset, name, val);
+}
+
+/**
+ * fdt_appendprop_string - append a string to a property
+ * @fdt: pointer to the device tree blob
+ * @nodeoffset: offset of the node whose property to change
+ * @name: name of the property to change
+ * @str: string value to append to the property
+ *
+ * fdt_appendprop_string() appends the given string to the value of
+ * the named property in the given node, or creates a new property
+ * with that value if it does not already exist.
+ *
+ * This function may insert data into the blob, and will therefore
+ * change the offsets of some existing nodes.
+ *
+ * returns:
+ * 0, on success
+ * -FDT_ERR_NOSPACE, there is insufficient free space in the blob to
+ * contain the new property value
+ * -FDT_ERR_BADOFFSET, nodeoffset did not point to FDT_BEGIN_NODE tag
+ * -FDT_ERR_BADLAYOUT,
+ * -FDT_ERR_BADMAGIC,
+ * -FDT_ERR_BADVERSION,
+ * -FDT_ERR_BADSTATE,
+ * -FDT_ERR_BADSTRUCTURE,
+ * -FDT_ERR_BADLAYOUT,
+ * -FDT_ERR_TRUNCATED, standard meanings
+ */
+#define fdt_appendprop_string(fdt, nodeoffset, name, str) \
+ fdt_appendprop((fdt), (nodeoffset), (name), (str), strlen(str)+1)
+
+/**
* fdt_delprop - delete a property
* @fdt: pointer to the device tree blob
* @nodeoffset: offset of the node whose property to nop
diff --git a/xen/include/xen/libfdt/libfdt_env.h b/xen/include/xen/libfdt/libfdt_env.h
index 2f1b03c..89a794b 100644
--- a/xen/include/xen/libfdt/libfdt_env.h
+++ b/xen/include/xen/libfdt/libfdt_env.h
@@ -6,6 +6,10 @@
#include <xen/string.h>
#include <asm/byteorder.h>
+typedef uint16_t fdt16_t;
+typedef uint32_t fdt32_t;
+typedef uint64_t fdt64_t;
+
#define fdt16_to_cpu(x) be16_to_cpu(x)
#define cpu_to_fdt16(x) cpu_to_be16(x)
#define fdt32_to_cpu(x) be32_to_cpu(x)
--
2.1.0
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH V5 14/15] Update libfdt to v1.4.0
2014-09-18 22:50 ` [PATCH V5 14/15] Update libfdt to v1.4.0 Roy Franz
@ 2014-09-22 11:20 ` Ian Campbell
2014-09-22 23:57 ` Roy Franz
0 siblings, 1 reply; 63+ messages in thread
From: Ian Campbell @ 2014-09-22 11:20 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, tim, xen-devel, stefano.stabellini, jbeulich, fu.wei
On Thu, 2014-09-18 at 15:50 -0700, Roy Franz wrote:
> Update libfdt to v1.4.0 of libfdt taken from git://git.jdl.com/software/dtc.git
> Xen changes to libfdt_env.h carried over from existing libfdt (v1.3.0)
> This update provides the fdt_create_empty_tree() function used by the ARM
> EFI boot code.
>
> Signed-off-by: Roy Franz <roy.franz@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
And I now remember that I planned to apply ths and the previous cache
flush one independently. So I have now done so.
For the cache flush I dropped the indent on ENTRY as discussed but I
also dropped the __flush_dcache_all label since ENTRY adds that already.
I hope that's ok.
Ian.
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH V5 14/15] Update libfdt to v1.4.0
2014-09-22 11:20 ` Ian Campbell
@ 2014-09-22 23:57 ` Roy Franz
2014-09-23 7:43 ` Ian Campbell
0 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-22 23:57 UTC (permalink / raw)
To: Ian Campbell
Cc: keir, tim, xen-devel, Stefano Stabellini, Jan Beulich, Fu Wei
On Mon, Sep 22, 2014 at 4:20 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Thu, 2014-09-18 at 15:50 -0700, Roy Franz wrote:
>> Update libfdt to v1.4.0 of libfdt taken from git://git.jdl.com/software/dtc.git
>> Xen changes to libfdt_env.h carried over from existing libfdt (v1.3.0)
>> This update provides the fdt_create_empty_tree() function used by the ARM
>> EFI boot code.
>>
>> Signed-off-by: Roy Franz <roy.franz@linaro.org>
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> And I now remember that I planned to apply ths and the previous cache
> flush one independently. So I have now done so.
>
> For the cache flush I dropped the indent on ENTRY as discussed but I
> also dropped the __flush_dcache_all label since ENTRY adds that already.
> I hope that's ok.
>
> Ian.
>
Sure - I'll leave them out of my next series.
I don't see them on the master branch yet though - is there an ARM
branch they go to first?
Thanks,
Roy
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH V5 14/15] Update libfdt to v1.4.0
2014-09-22 23:57 ` Roy Franz
@ 2014-09-23 7:43 ` Ian Campbell
0 siblings, 0 replies; 63+ messages in thread
From: Ian Campbell @ 2014-09-23 7:43 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, tim, xen-devel, Stefano Stabellini, Jan Beulich, Fu Wei
On Mon, 2014-09-22 at 16:57 -0700, Roy Franz wrote:
> On Mon, Sep 22, 2014 at 4:20 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Thu, 2014-09-18 at 15:50 -0700, Roy Franz wrote:
> >> Update libfdt to v1.4.0 of libfdt taken from git://git.jdl.com/software/dtc.git
> >> Xen changes to libfdt_env.h carried over from existing libfdt (v1.3.0)
> >> This update provides the fdt_create_empty_tree() function used by the ARM
> >> EFI boot code.
> >>
> >> Signed-off-by: Roy Franz <roy.franz@linaro.org>
> >
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> >
> > And I now remember that I planned to apply ths and the previous cache
> > flush one independently. So I have now done so.
> >
> > For the cache flush I dropped the indent on ENTRY as discussed but I
> > also dropped the __flush_dcache_all label since ENTRY adds that already.
> > I hope that's ok.
> >
> > Ian.
> >
>
> Sure - I'll leave them out of my next series.
> I don't see them on the master branch yet though - is there an ARM
> branch they go to first?
Things are committed to the "staging" branch which is then propagated to
"master" automatically when the tests pass (usually in a day or so,
unless there are failures).
Ian.
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH V5 15/15] Add ARM EFI boot support
2014-09-18 22:49 [PATCH V5 00/15] arm64 EFI stub Roy Franz
` (13 preceding siblings ...)
2014-09-18 22:50 ` [PATCH V5 14/15] Update libfdt to v1.4.0 Roy Franz
@ 2014-09-18 22:50 ` Roy Franz
2014-09-22 11:20 ` Ian Campbell
14 siblings, 1 reply; 63+ messages in thread
From: Roy Franz @ 2014-09-18 22:50 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/xen.lds.S | 1 +
xen/common/efi/boot.c | 12 +-
xen/common/efi/efi.h | 2 -
xen/common/efi/runtime.c | 15 +-
xen/include/asm-arm/arm64/efibind.h | 216 +++++++++++++
xen/include/asm-arm/efi-boot.h | 593 ++++++++++++++++++++++++++++++++++++
xen/include/asm-arm/efibind.h | 2 +
xen/include/asm-arm/setup.h | 2 +-
xen/include/asm-x86/efi-boot.h | 1 +
13 files changed, 990 insertions(+), 13 deletions(-)
create mode 100644 xen/arch/arm/efi/Makefile
create mode 100644 xen/include/asm-arm/arm64/efibind.h
create mode 100644 xen/include/asm-arm/efi-boot.h
create mode 100644 xen/include/asm-arm/efibind.h
diff --git a/.gitignore b/.gitignore
index 1aa4a5b..f8854aa 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 c13206f..be51685 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 43b5e72..158c102 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 */
@@ -617,6 +730,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/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/common/efi/boot.c b/xen/common/efi/boot.c
index 006a73c..0e0c32d 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 size_t wstrlen(const CHAR16 * s);
static int set_color(u32 mask, int bpp, u8 *pos, u8 *sz);
static bool_t read_file(EFI_FILE_HANDLE dir_handle, struct file *file,
CHAR16 *filename, char *options);
+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;
@@ -116,7 +118,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;
@@ -654,7 +656,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;
@@ -679,7 +681,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
EFI_STATUS status;
unsigned int i, argc;
CHAR16 **argv, *options = NULL;
- UINTN cols, rows, depth, size, info_size, gop_mode = ~0;
+ UINTN cols, rows, depth = 0, size, info_size, gop_mode = ~0;
EFI_HANDLE *handles = NULL;
EFI_SHIM_LOCK_PROTOCOL *shim_lock;
EFI_GRAPHICS_OUTPUT_PROTOCOL *gop = NULL;
@@ -983,8 +985,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();
@@ -1055,6 +1059,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,
@@ -1280,3 +1285,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/efi-boot.h b/xen/include/asm-arm/efi-boot.h
new file mode 100644
index 0000000..4a1877a
--- /dev/null
+++ b/xen/include/asm-arm/efi-boot.h
@@ -0,0 +1,593 @@
+/*
+ * Architecture specific implementation for EFI boot code. This file
+ * is intended to be included by XXX _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\r\n");
+
+ 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, &dtbfile, s2w(&name), 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\r\n");
+}
+
+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(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\r\n");
+
+ 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\r\n");
+
+ 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\r\n");
+}
+
+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 __init bool_t 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/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 */
diff --git a/xen/include/asm-x86/efi-boot.h b/xen/include/asm-x86/efi-boot.h
index 3ff593d..cb79956 100644
--- a/xen/include/asm-x86/efi-boot.h
+++ b/xen/include/asm-x86/efi-boot.h
@@ -19,6 +19,7 @@ static module_t __initdata mb_modules[3];
extern char start[];
extern u32 cpuid_ext_features;
+extern l4_pgentry_t *efi_l4_pgtable;
static void __init edd_put_string(u8 *dst, size_t n, const char *src)
{
--
2.1.0
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH V5 15/15] Add ARM EFI boot support
2014-09-18 22:50 ` [PATCH V5 15/15] Add ARM EFI boot support Roy Franz
@ 2014-09-22 11:20 ` Ian Campbell
2014-09-22 23:50 ` Roy Franz
0 siblings, 1 reply; 63+ messages in thread
From: Ian Campbell @ 2014-09-22 11:20 UTC (permalink / raw)
To: Roy Franz; +Cc: keir, tim, xen-devel, stefano.stabellini, jbeulich, fu.wei
On Thu, 2014-09-18 at 15:50 -0700, Roy Franz wrote:
> 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;
FWIW l4_pgentry_t is an x86 specific type and to add to the confusion
ARM counts the other way (so L4==root,L1==leaf on x86 and
L0==root,L3==leaf on ARM...)
But I presume handling that correctly is being put off until RTS
support, which is fine by me.
> diff --git a/xen/include/asm-arm/efi-boot.h b/xen/include/asm-arm/efi-boot.h
> new file mode 100644
> index 0000000..4a1877a
> --- /dev/null
> +++ b/xen/include/asm-arm/efi-boot.h
> @@ -0,0 +1,593 @@
> +/*
> + * Architecture specific implementation for EFI boot code. This file
> + * is intended to be included by XXX _only_, and therefore can define
I guess you meant to come back to the XXX here (I didn't check if the
x86 version had something similar).
If XXX is another header then it wouldn't be unreasonable to have an
#ifndef check for that header's guard variable and a #error.
Ian.
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH V5 15/15] Add ARM EFI boot support
2014-09-22 11:20 ` Ian Campbell
@ 2014-09-22 23:50 ` Roy Franz
0 siblings, 0 replies; 63+ messages in thread
From: Roy Franz @ 2014-09-22 23:50 UTC (permalink / raw)
To: Ian Campbell
Cc: keir, tim, xen-devel, Stefano Stabellini, Jan Beulich, Fu Wei
On Mon, Sep 22, 2014 at 4:20 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Thu, 2014-09-18 at 15:50 -0700, Roy Franz wrote:
>
>> 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;
>
> FWIW l4_pgentry_t is an x86 specific type and to add to the confusion
> ARM counts the other way (so L4==root,L1==leaf on x86 and
> L0==root,L3==leaf on ARM...)
>
> But I presume handling that correctly is being put off until RTS
> support, which is fine by me.
Yes, this extern was moved to the x86 specific header file, and the
equivalent is not
needed for ARM until runtime services are implemented.
>
>> diff --git a/xen/include/asm-arm/efi-boot.h b/xen/include/asm-arm/efi-boot.h
>> new file mode 100644
>> index 0000000..4a1877a
>> --- /dev/null
>> +++ b/xen/include/asm-arm/efi-boot.h
>> @@ -0,0 +1,593 @@
>> +/*
>> + * Architecture specific implementation for EFI boot code. This file
>> + * is intended to be included by XXX _only_, and therefore can define
>
> I guess you meant to come back to the XXX here (I didn't check if the
> x86 version had something similar).
>
> If XXX is another header then it wouldn't be unreasonable to have an
> #ifndef check for that header's guard variable and a #error.
>
> Ian.
>
oops :) That xxx needs to be fixed up - it should be xen/common/efi/boot.c.
Roy
^ permalink raw reply [flat|nested] 63+ messages in thread