diff mbox

[Xen-devel,for-4.5,V6,02/14] Move x86 specific funtions/variables to arch header

Message ID 1411534992-27443-3-git-send-email-roy.franz@linaro.org
State New
Headers show

Commit Message

Roy Franz Sept. 24, 2014, 5:03 a.m. UTC
Move the global variables and functions that can be moved as-is
from the common boot.c file to the x86 implementation header file.

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

Comments

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

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

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

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

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

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

that should be moved.

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

Yes.

Jan
diff mbox

Patch

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