|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] Is: Linux-EFI code to call Xen's EFI hypercalls [RFC PATCH comments] Was:Re: Xen 4.4 development update
On Thu, Aug 15, 2013 at 04:50:47PM -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Aug 15, 2013 at 04:24:20PM -0400, Eric Shelton wrote:
> > On Thu, Aug 15, 2013 at 10:12 AM, Konrad Rzeszutek Wilk
> > <konrad.wilk@xxxxxxxxxx> wrote:
> > > On Thu, Aug 15, 2013 at 01:32:12AM -0400, Eric Shelton wrote:
> > >
> > > First of, thank you for taking a stab at it and redoing it.
> >
> > Sure. The first releases of the patch were posted during a hectic
> > time in the release cycle. Three months later, I wanted to make sure
> > this had not gotten lost in the noise, and instead got wrapped up
> > while it was still fairly fresh for me.
> >
> > > Some general comments:
> > > - the #ifdef CONFIG_XEN is generaly frowend upon. If you need them they
> > > should
> > > be in header files. The exception is the CONFIG_X86 and
> > > CONFIG_X86_64. Now said that there are other instances of code where
> > > it is sprinkled with #ifdef CONFIG_ACPI .. #ifdef CONFIG_PCI, and so
> > > on. It would have been nice if all of that got moved to a header file
> > > but in reality that could make it just more confusing. Enough about
> > > that - what I want to say is that doing #idef CONFIG_XEN is something
> > > we have been trying the utmost to not put in generic code. The
> > > reasoning is that instead of using the API (so for example if we have
> > > an generic layer and then there are platform related drivers - we
> > > want to implement the stuff in the platform drivers - not add
> > > side-channels for a specific platform).
> >
> > OK. Hopefully the reorganization I suggest below will clear out most
> > or all of this.
> >
> > > - Is there any way to move the bulk of the hypercalls in the
> > > efi_runtime services. Meaning alter the efi_runtime services
> > > to do hypercalls instead of EFI calls? That way I would think most
> > > of the EFI general logic would be untouched? This is going back to
> > > the first comment - you would want to leave as much generic code
> > > untouched as possible. And implement the bulk of the code in the
> > > platform specific code.
> >
> > This sounds similar to Matthew Garrett's suggestion "to do this by
> > replacing efi_call_* I'm not quite sure how I would "alter the
> > efi_runtime services" to accomplish this - or at least not in some way
> > that is not worse than what is being done for things like
> > *_set_variable(). If you can more concretely show me how this might
> > be coded for one of the runtime service functions, I would be happy to
> > replicate that across the patch.
>
> Ha! I am just hand-waving right now :-)
>
> What I had in mind was that this:
>
> efi_phys.systab = (efi_system_table_t
> *)boot_params.efi_info.efi_systab;
>
> Is done, and we could implement some stub functions in the efi_systab
> that would convert the Microsoft stack passing to normal Linux
> and then do the hypercalls.
>
> It would be a bit of this:
>
> virt_efi_get_time -> calls efi_call_virt2 (efi_stub_32|64.S) ->
> assembler code Linux to EFI stacks, and calls in
> efi.systab->runtime->get_time
>
> The get_time would be our own little blob of code that alters the
> parameters from EFI stack to Linux and makes the hypercall (so probably
> in C). Then when the hypercall is done, it changes the stack back to EFI
> and returns.
>
> In other words we would implement an EFI runtime code that actually
> would do hypercalls.
>
> But from your analysis that does not solve the whole problem. Those
> efi_init* variants in some cases are unneccessary. Which brings another
> question - if we do barell throught them, what is the worst that will
> happen? Can the values returned be the same?
Right after I sent the email I thought about another option. Which is
the one I think Matthew was referring to. That is make efi_call*
function be more of a function pointer and the default one (compiled) is
the baremetal version. When Xen boots it over-writes it with its
specific. There is also some lookup to figure out which of the set_time,
get_time, etc calls are being called. This is all hand-waving and the
patch below has not even been compile tested.
From 197339fe9e4c95abe5b8948cf2bc3119c0ec87b5 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Date: Fri, 16 Aug 2013 10:06:52 -0400
Subject: [PATCH] efi/xen: Use a function pointer for baremetal and Xen
specific
efi calls.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
---
arch/x86/include/asm/efi.h | 43 +++++++++++++++++++++++++++++++------
arch/x86/platform/efi/efi_64.c | 11 ++++++++++
arch/x86/platform/efi/efi_stub_64.S | 28 ++++++++++++------------
arch/x86/xen/efi.c | 40 ++++++++++++++++++++++++++++++++++
arch/x86/xen/setup.c | 9 ++++++++
arch/x86/xen/xen-ops.h | 13 +++++++++++
6 files changed, 123 insertions(+), 21 deletions(-)
create mode 100644 arch/x86/xen/efi.c
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 0062a01..f234570 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -41,16 +41,45 @@ extern unsigned long asmlinkage efi_call_phys(void *, ...);
#define EFI_LOADER_SIGNATURE "EL64"
-extern u64 efi_call0(void *fp);
-extern u64 efi_call1(void *fp, u64 arg1);
-extern u64 efi_call2(void *fp, u64 arg1, u64 arg2);
-extern u64 efi_call3(void *fp, u64 arg1, u64 arg2, u64 arg3);
-extern u64 efi_call4(void *fp, u64 arg1, u64 arg2, u64 arg3, u64 arg4);
-extern u64 efi_call5(void *fp, u64 arg1, u64 arg2, u64 arg3,
+extern u64 native_efi_call0(void *fp);
+extern u64 native_efi_call1(void *fp, u64 arg1);
+extern u64 native_efi_call2(void *fp, u64 arg1, u64 arg2);
+extern u64 native_efi_call3(void *fp, u64 arg1, u64 arg2, u64 arg3);
+extern u64 native_efi_call4(void *fp, u64 arg1, u64 arg2, u64 arg3, u64 arg4);
+extern u64 native_efi_call5(void *fp, u64 arg1, u64 arg2, u64 arg3,
u64 arg4, u64 arg5);
-extern u64 efi_call6(void *fp, u64 arg1, u64 arg2, u64 arg3,
+extern u64 native_efi_call6(void *fp, u64 arg1, u64 arg2, u64 arg3,
u64 arg4, u64 arg5, u64 arg6);
+#ifdef CONFIG_PARAVIRT
+extern u64 (*platform_efi_call0)(void *fp);
+extern u64 (*platform_efi_call1)(void *fp, u64 arg1);
+extern u64 (*platform_efi_call2)(void *fp, u64 arg1, u64 arg2);
+extern u64 (*platform_efi_call3)(void *fp, u64 arg1, u64 arg2, u64 arg3);
+extern u64 (*platform_efi_call4)(void *fp, u64 arg1, u64 arg2, u64 arg3, u64
arg4);
+extern u64 (*platform_efi_call5)(void *fp, u64 arg1, u64 arg2, u64 arg3,
+ u64 arg4, u64 arg5);
+extern u64 (*platform_efi_call6)(void *fp, u64 arg1, u64 arg2, u64 arg3,
+ u64 arg4, u64 arg5, u64 arg6);
+
+#define efi_call0(f) platform_efi_call0(f)
+#define efi_call1(f, a1) platform_efi_call1(f, a1)
+#define efi_call2(f, a1, a2) platform_efi_call2(f, a1, a2)
+#define efi_call3(f, a1, a2, a3) platform_efi_call3(f, a1, a2, a3)
+#define efi_call4(f, a1, a2, a3, a4) platform_efi_call4(f, a1, a2, a3, a4)
+#define efi_call5(f, a1, a2, a3, a4, a5) platform_efi_call5(f, a1, a2,
a3, a4, a5)
+#define efi_call6(f, a1, a2, a3, a4, a5, a6) platform_efi_call6(f, a1, a2,
a3, a4, a5, a6)
+#else
+#define efi_call0(f) native_efi_call0(f)
+#define efi_call1(f, a1) native_efi_call1(f, a1)
+#define efi_call2(f, a1, a2) native_efi_call2(f, a1, a2)
+#define efi_call3(f, a1, a2, a3) native_efi_call3(f, a1, a2, a3)
+#define efi_call4(f, a1, a2, a3, a4) native_efi_call4(f, a1, a2, a3, a4)
+#define efi_call5(f, a1, a2, a3, a4, a5) native_efi_call5(f, a1, a2, a3,
a4, a5)
+#define efi_call6(f, a1, a2, a3, a4, a5, a6) native_efi_call6(f, a1, a2, a3,
a4, a5, a6)
+
+#endif
+
#define efi_call_phys0(f) \
efi_call0((f))
#define efi_call_phys1(f, a1) \
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index 39a0e7f..afa4354 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -113,3 +113,14 @@ void __iomem *__init efi_ioremap(unsigned long phys_addr,
unsigned long size,
return (void __iomem *)__va(phys_addr);
}
+#ifdef CONFIG_PARAVIRT
+u64 (*platform_efi_call0)(void *fp) = native_efi_call0;
+u64 (*platform_efi_call1)(void *fp, u64 arg1) = native_efi_call1;
+u64 (*platform_efi_call2)(void *fp, u64 arg1, u64 arg2) = native_efi_call2;
+u64 (*platform_efi_call3)(void *fp, u64 arg1, u64 arg2, u64 arg3) =
native_efi_call3;
+u64 (*platform_efi_call4)(void *fp, u64 arg1, u64 arg2, u64 arg3, u64 arg4) =
native_efi_call4;
+u64 (*platform_efi_call5)(void *fp, u64 arg1, u64 arg2, u64 arg3,
+ u64 arg4, u64 arg5) = native_efi_call5;
+u64 (*platform_efi_call6)(void *fp, u64 arg1, u64 arg2, u64 arg3,
+ u64 arg4, u64 arg5, u64 arg6) = native_efi_call6;
+#endif
diff --git a/arch/x86/platform/efi/efi_stub_64.S
b/arch/x86/platform/efi/efi_stub_64.S
index 4c07cca..60e993c 100644
--- a/arch/x86/platform/efi/efi_stub_64.S
+++ b/arch/x86/platform/efi/efi_stub_64.S
@@ -34,16 +34,16 @@
mov %rsi, %cr0; \
mov (%rsp), %rsp
-ENTRY(efi_call0)
+ENTRY(native_efi_call0)
SAVE_XMM
subq $32, %rsp
call *%rdi
addq $32, %rsp
RESTORE_XMM
ret
-ENDPROC(efi_call0)
+ENDPROC(native_efi_call0)
-ENTRY(efi_call1)
+ENTRY(native_efi_call1)
SAVE_XMM
subq $32, %rsp
mov %rsi, %rcx
@@ -51,9 +51,9 @@ ENTRY(efi_call1)
addq $32, %rsp
RESTORE_XMM
ret
-ENDPROC(efi_call1)
+ENDPROC(native_efi_call1)
-ENTRY(efi_call2)
+ENTRY(native_efi_call2)
SAVE_XMM
subq $32, %rsp
mov %rsi, %rcx
@@ -61,9 +61,9 @@ ENTRY(efi_call2)
addq $32, %rsp
RESTORE_XMM
ret
-ENDPROC(efi_call2)
+ENDPROC(native_efi_call2)
-ENTRY(efi_call3)
+ENTRY(native_efi_call3)
SAVE_XMM
subq $32, %rsp
mov %rcx, %r8
@@ -72,9 +72,9 @@ ENTRY(efi_call3)
addq $32, %rsp
RESTORE_XMM
ret
-ENDPROC(efi_call3)
+ENDPROC(native_efi_call3)
-ENTRY(efi_call4)
+ENTRY(native_efi_call4)
SAVE_XMM
subq $32, %rsp
mov %r8, %r9
@@ -84,9 +84,9 @@ ENTRY(efi_call4)
addq $32, %rsp
RESTORE_XMM
ret
-ENDPROC(efi_call4)
+ENDPROC(native_efi_call4)
-ENTRY(efi_call5)
+ENTRY(native_efi_call5)
SAVE_XMM
subq $48, %rsp
mov %r9, 32(%rsp)
@@ -97,9 +97,9 @@ ENTRY(efi_call5)
addq $48, %rsp
RESTORE_XMM
ret
-ENDPROC(efi_call5)
+ENDPROC(native_efi_call5)
-ENTRY(efi_call6)
+ENTRY(native_efi_call6)
SAVE_XMM
mov (%rsp), %rax
mov 8(%rax), %rax
@@ -113,4 +113,4 @@ ENTRY(efi_call6)
addq $48, %rsp
RESTORE_XMM
ret
-ENDPROC(efi_call6)
+ENDPROC(native_efi_call6)
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
new file mode 100644
index 0000000..f09f829
--- /dev/null
+++ b/arch/x86/xen/efi.c
@@ -0,0 +1,40 @@
+
+#include <asm/xen/hypercall.h>
+
+#include <xen/xen.h>
+#include <linux/efi.h>
+#include <asm/efi.h>
+#include <xen/interface/physdev.h>
+#include "xen-ops.h"
+
+efi_runtime_services_t xen_efi_fnc = {
+ .get_time = xen_get_time;
+ .set_time = xen_set_time;
+ /* and so on */
+};
+u64 xen_efi_call0(void *fp)
+{
+ char *func_idx;
+ u64 (*fnc)(void);
+
+ /*
+ * Look up which of the functions it is in the platform
+ * code, and find the same function in the Xen platform.
+ */
+ func_idx = &efi.systab->runtime - fp;
+ BUG_ON(func_idx == 0); /* Can't be the header! */
+ fnc = (char *)xen_efi_fnc + func_idx; /* This probably won't compile. */
+
+ BUG_ON(!fnc);
+ fnc();
+}
+/* and so on.
+u64 xen_efi_call1(void *fp, u64 arg1);
+u64 xen_efi_call2(void *fp, u64 arg1, u64 arg2);
+u64 xen_efi_call3(void *fp, u64 arg1, u64 arg2, u64 arg3);
+u64 xen_efi_call4(void *fp, u64 arg1, u64 arg2, u64 arg3, u64 arg4);
+u64 xen_efi_call5(void *fp, u64 arg1, u64 arg2, u64 arg3,
+ u64 arg4, u64 arg5);
+u64 xen_efi_call6(void *fp, u64 arg1, u64 arg2, u64 arg3,
+ u64 arg4, u64 arg5, u64 arg6);
+*/
diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index 056d11f..cc820d2 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -563,4 +563,13 @@ void __init xen_arch_setup(void)
#ifdef CONFIG_NUMA
numa_off = 1;
#endif
+#ifdef CONFIG_EFI
+ platform_efi_call0 = xen_efi_call0;
+ platform_efi_call1 = xen_efi_call1;
+ platform_efi_call2 = xen_efi_call2;
+ platform_efi_call3 = xen_efi_call3;
+ platform_efi_call4 = xen_efi_call4;
+ platform_efi_call5 = xen_efi_call5;
+ platform_efi_call6 = xen_efi_call6;
+#endif
}
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 86782c5..d680558 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -123,4 +123,17 @@ void xen_adjust_exception_frame(void);
extern int xen_panic_handler_init(void);
+#ifdef CONFIG_EFI
+extern u64 xen_efi_call0(void *fp);
+extern u64 xen_efi_call1(void *fp, u64 arg1);
+extern u64 xen_efi_call2(void *fp, u64 arg1, u64 arg2);
+extern u64 xen_efi_call3(void *fp, u64 arg1, u64 arg2, u64 arg3);
+extern u64 xen_efi_call4(void *fp, u64 arg1, u64 arg2, u64 arg3, u64 arg4);
+extern u64 xen_efi_call5(void *fp, u64 arg1, u64 arg2, u64 arg3,
+ u64 arg4, u64 arg5);
+extern u64 xen_efi_call6(void *fp, u64 arg1, u64 arg2, u64 arg3,
+ u64 arg4, u64 arg5, u64 arg6);
+
+
+#endif
#endif /* XEN_OPS_H */
--
1.7.11.7
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |