diff mbox

[Xen-devel,RFC,02/19] Create efi-shared.[ch], and move string functions

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

Commit Message

Roy Franz June 28, 2014, 1:25 a.m. UTC 
Create the files for EFI code that will be shared with the ARM stub,
and move string functions and some global variables.

TODO: proper sharing of file (with proper -fshort-char compile option)

Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
 xen/arch/x86/Makefile         |   4 +-
 xen/arch/x86/efi/Makefile     |   2 +-
 xen/arch/x86/efi/boot.c       | 127 +++---------------------------------------
 xen/arch/x86/efi/efi-shared.c | 122 ++++++++++++++++++++++++++++++++++++++++
 xen/include/efi/efi-shared.h  |  39 +++++++++++++
 5 files changed, 171 insertions(+), 123 deletions(-)
 create mode 100644 xen/arch/x86/efi/efi-shared.c
 create mode 100644 xen/include/efi/efi-shared.h

Comments

Jan Beulich June 30, 2014, 11:15 a.m. UTC | #1 
>>> On 28.06.14 at 03:25, <roy.franz@linaro.org> wrote:
> Create the files for EFI code that will be shared with the ARM stub,
> and move string functions and some global variables.
> 
> TODO: proper sharing of file (with proper -fshort-char compile option)

Already at this point of the series I wonder what the value of
posting it is if even the basic question of placement within the tree
hasn't been sorted out yet. What are reviewers expected to review
(considering that code should just be getting moved, not modified)?

Jan
Ian Campbell July 2, 2014, 11:49 a.m. UTC | #2 
On Mon, 2014-06-30 at 12:15 +0100, Jan Beulich wrote:
> >>> On 28.06.14 at 03:25, <roy.franz@linaro.org> wrote:
> > Create the files for EFI code that will be shared with the ARM stub,
> > and move string functions and some global variables.
> > 
> > TODO: proper sharing of file (with proper -fshort-char compile option)
> 
> Already at this point of the series I wonder what the value of
> posting it is if even the basic question of placement within the tree
> hasn't been sorted out yet. What are reviewers expected to review
> (considering that code should just be getting moved, not modified)?

I think feedback on the correct placement is useful feedback at this
stage. I think you gave advise in the 00 mail though (xen/common/efi)
which makes sense to me too.

Ian.
Ian Campbell July 2, 2014, 11:55 a.m. UTC | #3 
On Mon, 2014-06-30 at 12:15 +0100, Jan Beulich wrote:
> >>> On 28.06.14 at 03:25, <roy.franz@linaro.org> wrote:
> > Create the files for EFI code that will be shared with the ARM stub,
> > and move string functions and some global variables.
> > 
> > TODO: proper sharing of file (with proper -fshort-char compile option)
> 
> Already at this point of the series I wonder what the value of
> posting it is if even the basic question of placement within the tree
> hasn't been sorted out yet. What are reviewers expected to review
> (considering that code should just be getting moved, not modified)?

FWIW I found that reading the remainder of the series with an implicit
s,xen/arch/x86/efi/efi-shared.c,xen/common/efi/blah.c in my head was
worthwhile -- there's some useful refactoring in here as well as the
code motion into efi-shared^Wblah.c.

Ian.
Roy Franz July 9, 2014, 6:31 p.m. UTC | #4 
On Wed, Jul 2, 2014 at 4:55 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Mon, 2014-06-30 at 12:15 +0100, Jan Beulich wrote:
>> >>> On 28.06.14 at 03:25, <roy.franz@linaro.org> wrote:
>> > Create the files for EFI code that will be shared with the ARM stub,
>> > and move string functions and some global variables.
>> >
>> > TODO: proper sharing of file (with proper -fshort-char compile option)
>>
>> Already at this point of the series I wonder what the value of
>> posting it is if even the basic question of placement within the tree
>> hasn't been sorted out yet. What are reviewers expected to review
>> (considering that code should just be getting moved, not modified)?
>
> FWIW I found that reading the remainder of the series with an implicit
> s,xen/arch/x86/efi/efi-shared.c,xen/common/efi/blah.c in my head was
> worthwhile -- there's some useful refactoring in here as well as the
> code motion into efi-shared^Wblah.c.
>
> Ian.
>
>


I'll move the shared C code to xen/common/efi/ in the next series.
For building with
the -fshort-wchar I'll likely need a subdirectory even if it has only
a single file for now.
diff mbox

Patch

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 6c90b1b..7504780 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -87,13 +87,13 @@  prelink-efi_lto.o: $(ALL_OBJS) efi/runtime.o efi/compat.o
 prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
 	$(LD) $(LDFLAGS) -r -o $@ $^
 
-prelink-efi.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink-efi_lto.o efi/boot.init.o
+prelink-efi.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink-efi_lto.o efi/boot.init.o efi/efi-shared.o
 	$(guard) $(LD) $(LDFLAGS) -r -o $@ $^
 else
 prelink.o: $(ALL_OBJS)
 	$(LD) $(LDFLAGS) -r -o $@ $^
 
-prelink-efi.o: $(ALL_OBJS) efi/boot.init.o efi/runtime.o efi/compat.o
+prelink-efi.o: $(ALL_OBJS) efi/boot.init.o efi/runtime.o efi/compat.o efi/efi-shared.o
 	$(guard) $(LD) $(LDFLAGS) -r -o $@ $(filter-out %/efi/built_in.o,$^)
 endif
 
diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
index 1daa7ac..9c198d2 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -9,6 +9,6 @@  efi := $(if $(efi),$(shell $(CC) $(filter-out $(CFLAGS-y) .%.d,$(CFLAGS)) -c che
 efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi check.o 2>disabled && echo y))
 efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call create,boot.init.o); $(call create,runtime.o)))
 
-extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o
+extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o efi-shared.o
 
 stub.o: $(extra-y)
diff --git a/xen/arch/x86/efi/boot.c b/xen/arch/x86/efi/boot.c
index 2b515f2..2b6bea3 100644
--- a/xen/arch/x86/efi/boot.c
+++ b/xen/arch/x86/efi/boot.c
@@ -1,6 +1,7 @@ 
 #include "efi.h"
 #include <efi/efiprot.h>
 #include <efi/efipciio.h>
+#include <efi/efi-shared.h>
 #include <public/xen.h>
 #include <xen/compile.h>
 #include <xen/ctype.h>
@@ -44,25 +45,16 @@  typedef struct {
 extern char start[];
 extern u32 cpuid_ext_features;
 
-union string {
-    CHAR16 *w;
-    char *s;
-    const char *cs;
-};
 
-struct file {
-    UINTN size;
-    union {
-        EFI_PHYSICAL_ADDRESS addr;
-        void *ptr;
-    };
-};
+/* Variables supplied/used by shared EFI code. */
+extern CHAR16 __initdata newline[];
+extern EFI_BOOT_SERVICES *__initdata efi_bs;
+extern SIMPLE_TEXT_OUTPUT_INTERFACE *__initdata StdOut;
+extern SIMPLE_TEXT_OUTPUT_INTERFACE *__initdata StdErr;
+
 
-static EFI_BOOT_SERVICES *__initdata efi_bs;
 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;
 
@@ -77,111 +69,6 @@  static multiboot_info_t __initdata mbi = {
 };
 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)
-
-static CHAR16 *__init FormatDec(UINT64 Val, CHAR16 *Buffer)
-{
-    if ( Val >= 10 )
-        Buffer = FormatDec(Val / 10, Buffer);
-    *Buffer = (CHAR16)(L'0' + Val % 10);
-    return Buffer + 1;
-}
-
-static CHAR16 *__init FormatHex(UINT64 Val, UINTN Width, CHAR16 *Buffer)
-{
-    if ( Width > 1 || Val >= 0x10 )
-        Buffer = FormatHex(Val >> 4, Width ? Width - 1 : 0, Buffer);
-    *Buffer = (CHAR16)((Val &= 0xf) < 10 ? L'0' + Val : L'a' + Val - 10);
-    return Buffer + 1;
-}
-
-static void __init DisplayUint(UINT64 Val, INTN Width)
-{
-    CHAR16 PrintString[32], *end;
-
-    if (Width < 0)
-        end = FormatDec(Val, PrintString);
-    else
-    {
-        PrintStr(L"0x");
-        end = FormatHex(Val, Width, PrintString);
-    }
-    *end = 0;
-    PrintStr(PrintString);
-}
-
-static CHAR16 *__init wstrcpy(CHAR16 *d, const CHAR16 *s)
-{
-    CHAR16 *r = d;
-
-    while ( (*d++ = *s++) != 0 )
-        ;
-    return r;
-}
-
-static int __init wstrcmp(const CHAR16 *s1, const CHAR16 *s2)
-{
-    while ( *s1 && *s1 == *s2 )
-    {
-        ++s1;
-        ++s2;
-    }
-    return *s1 - *s2;
-}
-
-static int __init wstrncmp(const CHAR16 *s1, const CHAR16 *s2, UINTN n)
-{
-    while ( n && *s1 && *s1 == *s2 )
-    {
-        --n;
-        ++s1;
-        ++s2;
-    }
-    return n ? *s1 - *s2 : 0;
-}
-
-static CHAR16 *__init s2w(union string *str)
-{
-    const char *s = str->s;
-    CHAR16 *w;
-    void *ptr;
-
-    if ( efi_bs->AllocatePool(EfiLoaderData, (strlen(s) + 1) * sizeof(*w),
-                              &ptr) != EFI_SUCCESS )
-        return NULL;
-
-    w = str->w = ptr;
-    do {
-        *w = *s++;
-    } while ( *w++ );
-
-    return str->w;
-}
-
-static char *__init w2s(const union string *str)
-{
-    const CHAR16 *w = str->w;
-    char *s = str->s;
-
-    do {
-        if ( *w > 0x007f )
-            return NULL;
-        *s = *w++;
-    } while ( *s++ );
-
-    return str->s;
-}
-
-static bool_t __init match_guid(const EFI_GUID *guid1, const EFI_GUID *guid2)
-{
-    return guid1->Data1 == guid2->Data1 &&
-           guid1->Data2 == guid2->Data2 &&
-           guid1->Data3 == guid2->Data3 &&
-           !memcmp(guid1->Data4, guid2->Data4, sizeof(guid1->Data4));
-}
 
 static void __init noreturn blexit(const CHAR16 *str)
 {
diff --git a/xen/arch/x86/efi/efi-shared.c b/xen/arch/x86/efi/efi-shared.c
new file mode 100644
index 0000000..b9c563a
--- /dev/null
+++ b/xen/arch/x86/efi/efi-shared.c
@@ -0,0 +1,122 @@ 
+/* EFI code shared between architectures. */
+
+#include "efi.h"
+#include <efi/efiprot.h>
+#include <efi/efipciio.h>
+#include <efi/efi-shared.h>
+#include <public/xen.h>
+#include <efi/efi-shared.h>
+#include <xen/compile.h>
+#include <xen/ctype.h>
+#include <xen/init.h>
+#include <asm/processor.h>
+
+
+SIMPLE_TEXT_OUTPUT_INTERFACE *__initdata StdOut;
+SIMPLE_TEXT_OUTPUT_INTERFACE *__initdata StdErr;
+EFI_BOOT_SERVICES *__initdata efi_bs;
+
+
+CHAR16 __initdata newline[] = L"\r\n";
+
+CHAR16 *__init FormatDec(UINT64 Val, CHAR16 *Buffer)
+{
+    if ( Val >= 10 )
+        Buffer = FormatDec(Val / 10, Buffer);
+    *Buffer = (CHAR16)(L'0' + Val % 10);
+    return Buffer + 1;
+}
+
+CHAR16 *__init FormatHex(UINT64 Val, UINTN Width, CHAR16 *Buffer)
+{
+    if ( Width > 1 || Val >= 0x10 )
+        Buffer = FormatHex(Val >> 4, Width ? Width - 1 : 0, Buffer);
+    *Buffer = (CHAR16)((Val &= 0xf) < 10 ? L'0' + Val : L'a' + Val - 10);
+    return Buffer + 1;
+}
+
+
+void __init DisplayUint(UINT64 Val, INTN Width)
+{
+    CHAR16 PrintString[32], *end;
+
+    if (Width < 0)
+        end = FormatDec(Val, PrintString);
+    else
+    {
+        PrintStr(L"0x");
+        end = FormatHex(Val, Width, PrintString);
+    }
+    *end = 0;
+    PrintStr(PrintString);
+}
+
+CHAR16 *__init wstrcpy(CHAR16 *d, const CHAR16 *s)
+{
+    CHAR16 *r = d;
+
+    while ( (*d++ = *s++) != 0 )
+        ;
+    return r;
+}
+
+int __init wstrcmp(const CHAR16 *s1, const CHAR16 *s2)
+{
+    while ( *s1 && *s1 == *s2 )
+    {
+        ++s1;
+        ++s2;
+    }
+    return *s1 - *s2;
+}
+
+int __init wstrncmp(const CHAR16 *s1, const CHAR16 *s2, UINTN n)
+{
+    while ( n && *s1 && *s1 == *s2 )
+    {
+        --n;
+        ++s1;
+        ++s2;
+    }
+    return n ? *s1 - *s2 : 0;
+}
+
+CHAR16 *__init s2w(union string *str)
+{
+    const char *s = str->s;
+    CHAR16 *w;
+    void *ptr;
+
+    if ( efi_bs->AllocatePool(EfiLoaderData, (strlen(s) + 1) * sizeof(*w),
+                              &ptr) != EFI_SUCCESS )
+        return NULL;
+
+    w = str->w = ptr;
+    do {
+        *w = *s++;
+    } while ( *w++ );
+
+    return str->w;
+}
+
+char *__init w2s(const union string *str)
+{
+    const CHAR16 *w = str->w;
+    char *s = str->s;
+
+    do {
+        if ( *w > 0x007f )
+            return NULL;
+        *s = *w++;
+    } while ( *s++ );
+
+    return str->s;
+}
+
+bool_t __init match_guid(const EFI_GUID *guid1, const EFI_GUID *guid2)
+{
+    return guid1->Data1 == guid2->Data1 &&
+           guid1->Data2 == guid2->Data2 &&
+           guid1->Data3 == guid2->Data3 &&
+           !memcmp(guid1->Data4, guid2->Data4, sizeof(guid1->Data4));
+}
diff --git a/xen/include/efi/efi-shared.h b/xen/include/efi/efi-shared.h
new file mode 100644
index 0000000..b4c7ac5
--- /dev/null
+++ b/xen/include/efi/efi-shared.h
@@ -0,0 +1,39 @@ 
+#ifndef __EFI_SHARED_H__
+#define __EFI_SHARED_H__
+
+#include <efi/efidef.h>
+#include <xen/init.h>
+
+
+union string {
+    CHAR16 *w;
+    char *s;
+    const char *cs;
+};
+
+struct file {
+    UINTN size;
+    union {
+        EFI_PHYSICAL_ADDRESS addr;
+        void *ptr;
+    };
+};
+
+
+#define PrintStr(s) StdOut->OutputString(StdOut, s)
+#define PrintErr(s) StdErr->OutputString(StdErr, s)
+
+
+
+CHAR16 * FormatDec(UINT64 Val, CHAR16 *Buffer);
+CHAR16 * FormatHex(UINT64 Val, UINTN Width, CHAR16 *Buffer);
+
+void __init DisplayUint(UINT64 Val, INTN Width);
+CHAR16 *__init wstrcpy(CHAR16 *d, const CHAR16 *s);
+int __init wstrcmp(const CHAR16 *s1, const CHAR16 *s2);
+int __init wstrncmp(const CHAR16 *s1, const CHAR16 *s2, UINTN n);
+CHAR16 *__init s2w(union string *str);
+char *__init w2s(const union string *str);
+bool_t __init match_guid(const EFI_GUID *guid1, const EFI_GUID *guid2);
+
+#endif