Re: [RFC PATCH 09/10] arm64/efi: enable minimal UEFI Runtime Services for big endian

2014-07-23 Thread Mark Rutland 
Hi Ard,

This is certainly a neat feature, and I definitely want to be able to
boot BE kernels via UEFI.

However, I'm wary of calling EFI in a physical (i.e. idmap with dcaches
off) context. I'm not sure anyone else does that, and I'm not sure
whether that's going to work (both because of the cache maintenance
requirements and the expectations of a given UEFI implementation w.r.t.
memory cacheability).

I'd hoped we'd be able to use a LE EL0 context to call the runtime
services in, but I'm not sure that's possible by the spec :(

As I understand it, we shouldn't need these runtime services to simply
boot a BE kernel.

On Mon, Jul 21, 2014 at 04:16:24PM +0100, Ard Biesheuvel wrote:
 This enables the UEFI Runtime Services needed to manipulate the
 non-volatile variable store when running under a BE kernel.
 
 Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
 ---
  arch/arm64/include/asm/efi.h   |   2 +
  arch/arm64/kernel/efi-be-call.S|  55 
  arch/arm64/kernel/efi-be-runtime.c | 104 
 +
  arch/arm64/kernel/efi.c|  15 ++
  4 files changed, 176 insertions(+)
  create mode 100644 arch/arm64/kernel/efi-be-call.S
  create mode 100644 arch/arm64/kernel/efi-be-runtime.c
 
 diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
 index a34fd3b12e2b..44e642b6ab61 100644
 --- a/arch/arm64/include/asm/efi.h
 +++ b/arch/arm64/include/asm/efi.h
 @@ -44,4 +44,6 @@ extern void efi_idmap_init(void);
  
  #define efi_call_early(f, ...) sys_table_arg-boottime-f(__VA_ARGS__)
  
 +extern void efi_be_runtime_setup(void);
 +
  #endif /* _ASM_EFI_H */
 diff --git a/arch/arm64/kernel/efi-be-call.S b/arch/arm64/kernel/efi-be-call.S
 new file mode 100644
 index ..24c92a4c352b
 --- /dev/null
 +++ b/arch/arm64/kernel/efi-be-call.S
 @@ -0,0 +1,55 @@
 +
 +#include linux/linkage.h
 +
 + .text
 + .align  3
 +ENTRY(efi_be_phys_call)
 + /*
 +  * Entered at physical address with 1:1 mapping enabled.
 +  */
 + stp x29, x30, [sp, #-96]!
 + mov x29, sp
 + str x27, [sp, #16]
 +
 + ldr x8, =efi_be_phys_call   // virt address of this function
 + adr x9, efi_be_phys_call// phys address of this function
 + sub x9, x8, x9  // calculate virt to phys offset in x9
 +
 + /* preserve all inputs */
 + stp x0, x1, [sp, #32]
 + stp x2, x3, [sp, #48]
 + stp x4, x5, [sp, #64]
 + stp x6, x7, [sp, #80]
 +
 + /* get phys address of stack */
 + sub sp, sp, x9
 +
 + /* switch to LE, disable MMU and D-cache but leave I-cache enabled */
 + mrs x27, sctlr_el1
 + bic x8, x27, #1  2// clear SCTLR.C
 + msr sctlr_el1, x8
 +
 + bl  flush_cache_all

What is the cache flush for?

The only thing that flush_cache_all can do is empty the local
architected caches, and it can only do that when said caches are
disabled. Any other use is unsafe; we have no guarantee that the cache
is empty (or even clean), and we have no guarantee that prior writes
have made it to the PoC.

Even when the caches are disabled, flush_cache_all can only guarantee
that the local architected caches are empty. There is no guarantee that
the dirty data made it to the PoC.

 +
 + /* restore inputs but rotated by 1 register */
 + ldp x7, x0, [sp, #32]
 + ldp x1, x2, [sp, #48]
 + ldp x3, x4, [sp, #64]
 + ldp x5, x6, [sp, #80]
 +
 + bic x8, x27, #1  2// clear SCTLR.C
 + bic x8, x8, #1  0 // clear SCTLR.M
 + bic x8, x8, #1  25// clear SCTLR.EE
 + msr sctlr_el1, x8
 + isb
 +
 + blr x7

Is it safe to call EFI functions with the D-cache disabled?

Do the functions not care about the memory attributes for their own sue
(e.g. for exclusives)? 

Do they not care about IO? If IO to/from storage for variables is
cache-coherent EFI and the device won't have a coherent view of memory.

 +
 + /* switch back to BE and reenable MMU and D-cache */
 + msr sctlr_el1, x27
 +

Missing ISB?

 + mov sp, x29
 + ldr x27, [sp, #16]
 + ldp x29, x30, [sp], #96
 + ret
 +ENDPROC(efi_be_phys_call)
 diff --git a/arch/arm64/kernel/efi-be-runtime.c 
 b/arch/arm64/kernel/efi-be-runtime.c
 new file mode 100644
 index ..62e6c441b77b
 --- /dev/null
 +++ b/arch/arm64/kernel/efi-be-runtime.c
 @@ -0,0 +1,104 @@
 +
 +#include linux/efi.h
 +#include linux/spinlock.h
 +#include asm/efi.h
 +#include asm/neon.h
 +#include asm/tlbflush.h
 +
 +static efi_runtime_services_t *runtime;
 +static efi_status_t (*efi_be_call)(phys_addr_t func, ...);
 +
 +static DEFINE_SPINLOCK(efi_be_rt_lock);
 +
 +static unsigned long efi_be_call_pre(void)
 +{
 + unsigned long flags;
 +
 + kernel_neon_begin();
 + spin_lock_irqsave(efi_be_rt_lock, flags);

At this point we might still have DA_F unmasked, and I don't

Re: [RFC PATCH 09/10] arm64/efi: enable minimal UEFI Runtime Services for big endian

2014-07-23 Thread Ard Biesheuvel 
On 23 July 2014 11:34, Mark Rutland mark.rutl...@arm.com wrote:
 Hi Ard,

 This is certainly a neat feature, and I definitely want to be able to
 boot BE kernels via UEFI.


Good!

 However, I'm wary of calling EFI in a physical (i.e. idmap with dcaches
 off) context. I'm not sure anyone else does that, and I'm not sure
 whether that's going to work (both because of the cache maintenance
 requirements and the expectations of a given UEFI implementation w.r.t.
 memory cacheability).


I have developed an alternate version in the mean time that switches
to a LE idmap (so with D-cache enabled), but this is an imperfect
solution as well, as (like in the MMU off case), the vector base
virtual address cannot be resolved when the EE bit is cleared (as
TTBR1 points to a BE page table) so any exception taken locks the
machine hard. I am not sure if this can be solved in any way other
than changing exception levels. Or install an alternate vector table
for the duration of the runtime services call that flips the EE bit
back, restores VBAR to its original address, and jumps into it. None
of this is very sexy, though ...

 I'd hoped we'd be able to use a LE EL0 context to call the runtime
 services in, but I'm not sure that's possible by the spec :(


Nope, they should be called at the exception level UEFI was started in
(as Leif tells me)

 As I understand it, we shouldn't need these runtime services to simply
 boot a BE kernel.


Well, the significance of the variable store related Runtime Services
is that they are used by an installer (through efibootmgr) to program
the kernel command line. Hence the choice for just these services in
the minimal implementation.

-- 
Ard.


 On Mon, Jul 21, 2014 at 04:16:24PM +0100, Ard Biesheuvel wrote:
 This enables the UEFI Runtime Services needed to manipulate the
 non-volatile variable store when running under a BE kernel.

 Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
 ---
  arch/arm64/include/asm/efi.h   |   2 +
  arch/arm64/kernel/efi-be-call.S|  55 
  arch/arm64/kernel/efi-be-runtime.c | 104 
 +
  arch/arm64/kernel/efi.c|  15 ++
  4 files changed, 176 insertions(+)
  create mode 100644 arch/arm64/kernel/efi-be-call.S
  create mode 100644 arch/arm64/kernel/efi-be-runtime.c

 diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
 index a34fd3b12e2b..44e642b6ab61 100644
 --- a/arch/arm64/include/asm/efi.h
 +++ b/arch/arm64/include/asm/efi.h
 @@ -44,4 +44,6 @@ extern void efi_idmap_init(void);

  #define efi_call_early(f, ...) sys_table_arg-boottime-f(__VA_ARGS__)

 +extern void efi_be_runtime_setup(void);
 +
  #endif /* _ASM_EFI_H */
 diff --git a/arch/arm64/kernel/efi-be-call.S 
 b/arch/arm64/kernel/efi-be-call.S
 new file mode 100644
 index ..24c92a4c352b
 --- /dev/null
 +++ b/arch/arm64/kernel/efi-be-call.S
 @@ -0,0 +1,55 @@
 +
 +#include linux/linkage.h
 +
 + .text
 + .align  3
 +ENTRY(efi_be_phys_call)
 + /*
 +  * Entered at physical address with 1:1 mapping enabled.
 +  */
 + stp x29, x30, [sp, #-96]!
 + mov x29, sp
 + str x27, [sp, #16]
 +
 + ldr x8, =efi_be_phys_call   // virt address of this function
 + adr x9, efi_be_phys_call// phys address of this function
 + sub x9, x8, x9  // calculate virt to phys offset in x9
 +
 + /* preserve all inputs */
 + stp x0, x1, [sp, #32]
 + stp x2, x3, [sp, #48]
 + stp x4, x5, [sp, #64]
 + stp x6, x7, [sp, #80]
 +
 + /* get phys address of stack */
 + sub sp, sp, x9
 +
 + /* switch to LE, disable MMU and D-cache but leave I-cache enabled */
 + mrs x27, sctlr_el1
 + bic x8, x27, #1  2// clear SCTLR.C
 + msr sctlr_el1, x8
 +
 + bl  flush_cache_all

 What is the cache flush for?

 The only thing that flush_cache_all can do is empty the local
 architected caches, and it can only do that when said caches are
 disabled. Any other use is unsafe; we have no guarantee that the cache
 is empty (or even clean), and we have no guarantee that prior writes
 have made it to the PoC.

 Even when the caches are disabled, flush_cache_all can only guarantee
 that the local architected caches are empty. There is no guarantee that
 the dirty data made it to the PoC.

 +
 + /* restore inputs but rotated by 1 register */
 + ldp x7, x0, [sp, #32]
 + ldp x1, x2, [sp, #48]
 + ldp x3, x4, [sp, #64]
 + ldp x5, x6, [sp, #80]
 +
 + bic x8, x27, #1  2// clear SCTLR.C
 + bic x8, x8, #1  0 // clear SCTLR.M
 + bic x8, x8, #1  25// clear SCTLR.EE
 + msr sctlr_el1, x8
 + isb
 +
 + blr x7

 Is it safe to call EFI functions with the D-cache disabled?

 Do the functions not care about the memory attributes for their own sue
 (e.g. for exclusives)?

 Do they not care

Re: [RFC PATCH 09/10] arm64/efi: enable minimal UEFI Runtime Services for big endian

2014-07-23 Thread Ard Biesheuvel 
On 07/23/2014 12:59 PM, Ard Biesheuvel wrote:
 On 23 July 2014 11:34, Mark Rutland mark.rutl...@arm.com wrote:
 Hi Ard,

 This is certainly a neat feature, and I definitely want to be able to
 boot BE kernels via UEFI.

 
 Good!
 
 However, I'm wary of calling EFI in a physical (i.e. idmap with dcaches
 off) context. I'm not sure anyone else does that, and I'm not sure
 whether that's going to work (both because of the cache maintenance
 requirements and the expectations of a given UEFI implementation w.r.t.
 memory cacheability).

 
 I have developed an alternate version in the mean time that switches
 to a LE idmap (so with D-cache enabled), but this is an imperfect
 solution as well, as (like in the MMU off case), the vector base
 virtual address cannot be resolved when the EE bit is cleared (as
 TTBR1 points to a BE page table) so any exception taken locks the
 machine hard. I am not sure if this can be solved in any way other
 than changing exception levels. Or install an alternate vector table
 for the duration of the runtime services call that flips the EE bit
 back, restores VBAR to its original address, and jumps into it. None
 of this is very sexy, though ...
 
 I'd hoped we'd be able to use a LE EL0 context to call the runtime
 services in, but I'm not sure that's possible by the spec :(

 
 Nope, they should be called at the exception level UEFI was started in
 (as Leif tells me)
 
 As I understand it, we shouldn't need these runtime services to simply
 boot a BE kernel.

 
 Well, the significance of the variable store related Runtime Services
 is that they are used by an installer (through efibootmgr) to program
 the kernel command line. Hence the choice for just these services in
 the minimal implementation.
 

The below patch is an alternate approach with a LE id mapping in
efi_pg_dir. (Patch that sets it up omitted).

This dodges all the concerns related to caching, hopefully, as the LE id
mapping and the BE id mapping in idmap_pg_dir should agree on the memory
attributes of all common mappings.

This also addresses the FIQ and exception concerns, although I fully
realise that this is likely too controversial. Suggestions for less
controversial approaches are highly appreciated. As said, booting a BE
kernel is useful by itself, but without being able to use efibootmgr it
is a bit crippled.

-- 
Ard.


diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index a34fd3b12e2b..2eeae5ae55b2 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -44,4 +44,6 @@ extern void efi_idmap_init(void);

 #define efi_call_early(f, ...) sys_table_arg-boottime-f(__VA_ARGS__)

+extern int efi_be_runtime_setup(void);
+
 #endif /* _ASM_EFI_H */
diff --git a/arch/arm64/kernel/efi-be-call.S
b/arch/arm64/kernel/efi-be-call.S
new file mode 100644
index ..8da53a225fab
--- /dev/null
+++ b/arch/arm64/kernel/efi-be-call.S
@@ -0,0 +1,129 @@
+
+#include linux/linkage.h
+
+   .macro  flush_tlb_all
+   dsb ishst
+   tlbivmalle1is
+   dsb ish
+   isb
+   .endm
+
+   .text
+   /*
+* Alternate vector table so we can trap exceptions while in LE mode
+* and make the world sane again before letting the kernel handle the
+* exception as usual. Clobbers x30.
+*/
+   .align  12
+.Lvectors:
+   .irpc   i, 0123456789abcdef
+   .align  7
+   /* switch back to BE and temporarily disable MMU */
+   mrs x30, sctlr_el1
+   bic x30, x30, #1  0   // clear SCTLR.M
+   orr x30, x30, #1  25  // set SCTLR.EE
+   msr sctlr_el1, x30
+   isb
+
+   /* needed as TLBs are permitted to cache the EE bit */
+   flush_tlb_all
+
+   /* re-install BE idmap */
+   adrpx30, idmap_pg_dir
+   msr ttbr0_el1, x30
+   mrs x30, sctlr_el1
+   orr x30, x30, #1  0   // set SCTLR.M
+   msr sctlr_el1, x30  // re-enable MMU
+   isb
+
+   /*
+* Use the virtual and physical addresses of 'vectors' to restore the
+* virtual offset of sp.
+*/
+   adrpx30, vectors
+   add x30, x30, #:lo12:vectors
+   sub sp, sp, x30
+   ldr x30, =vectors
+   add sp, sp, x30
+
+   /* reinstall vector table */
+   msr vbar_el1, x30   // restore VBAR to 'vectors'
+   isb
+
+   add x30, x30, #(0x\i * 0x80) // jump to real vector
+   ret
+   .endr
+
+ENTRY(efi_be_phys_call)
+   /*
+* Entered at physical address with 1:1 mapping enabled and interrupts
+* disabled.
+*/
+   stp x29, x30, [sp, #-48]!
+   mov x29, sp
+   stp x25, x26, [sp, #16]
+   stp x27, x28, [sp, #32]
+
+   ldr x8, =efi_be_phys_call   // virt address of this function
+   adr x9, efi_be_phys_call// phys address of this function
+   sub x9, x8, x9  // calculate virt to

[RFC PATCH 09/10] arm64/efi: enable minimal UEFI Runtime Services for big endian

2014-07-21 Thread Ard Biesheuvel 
This enables the UEFI Runtime Services needed to manipulate the
non-volatile variable store when running under a BE kernel.

Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
---
 arch/arm64/include/asm/efi.h   |   2 +
 arch/arm64/kernel/efi-be-call.S|  55 
 arch/arm64/kernel/efi-be-runtime.c | 104 +
 arch/arm64/kernel/efi.c|  15 ++
 4 files changed, 176 insertions(+)
 create mode 100644 arch/arm64/kernel/efi-be-call.S
 create mode 100644 arch/arm64/kernel/efi-be-runtime.c

diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index a34fd3b12e2b..44e642b6ab61 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -44,4 +44,6 @@ extern void efi_idmap_init(void);
 
 #define efi_call_early(f, ...) sys_table_arg-boottime-f(__VA_ARGS__)
 
+extern void efi_be_runtime_setup(void);
+
 #endif /* _ASM_EFI_H */
diff --git a/arch/arm64/kernel/efi-be-call.S b/arch/arm64/kernel/efi-be-call.S
new file mode 100644
index ..24c92a4c352b
--- /dev/null
+++ b/arch/arm64/kernel/efi-be-call.S
@@ -0,0 +1,55 @@
+
+#include linux/linkage.h
+
+   .text
+   .align  3
+ENTRY(efi_be_phys_call)
+   /*
+* Entered at physical address with 1:1 mapping enabled.
+*/
+   stp x29, x30, [sp, #-96]!
+   mov x29, sp
+   str x27, [sp, #16]
+
+   ldr x8, =efi_be_phys_call   // virt address of this function
+   adr x9, efi_be_phys_call// phys address of this function
+   sub x9, x8, x9  // calculate virt to phys offset in x9
+
+   /* preserve all inputs */
+   stp x0, x1, [sp, #32]
+   stp x2, x3, [sp, #48]
+   stp x4, x5, [sp, #64]
+   stp x6, x7, [sp, #80]
+
+   /* get phys address of stack */
+   sub sp, sp, x9
+
+   /* switch to LE, disable MMU and D-cache but leave I-cache enabled */
+   mrs x27, sctlr_el1
+   bic x8, x27, #1  2// clear SCTLR.C
+   msr sctlr_el1, x8
+
+   bl  flush_cache_all
+
+   /* restore inputs but rotated by 1 register */
+   ldp x7, x0, [sp, #32]
+   ldp x1, x2, [sp, #48]
+   ldp x3, x4, [sp, #64]
+   ldp x5, x6, [sp, #80]
+
+   bic x8, x27, #1  2// clear SCTLR.C
+   bic x8, x8, #1  0 // clear SCTLR.M
+   bic x8, x8, #1  25// clear SCTLR.EE
+   msr sctlr_el1, x8
+   isb
+
+   blr x7
+
+   /* switch back to BE and reenable MMU and D-cache */
+   msr sctlr_el1, x27
+
+   mov sp, x29
+   ldr x27, [sp, #16]
+   ldp x29, x30, [sp], #96
+   ret
+ENDPROC(efi_be_phys_call)
diff --git a/arch/arm64/kernel/efi-be-runtime.c 
b/arch/arm64/kernel/efi-be-runtime.c
new file mode 100644
index ..62e6c441b77b
--- /dev/null
+++ b/arch/arm64/kernel/efi-be-runtime.c
@@ -0,0 +1,104 @@
+
+#include linux/efi.h
+#include linux/spinlock.h
+#include asm/efi.h
+#include asm/neon.h
+#include asm/tlbflush.h
+
+static efi_runtime_services_t *runtime;
+static efi_status_t (*efi_be_call)(phys_addr_t func, ...);
+
+static DEFINE_SPINLOCK(efi_be_rt_lock);
+
+static unsigned long efi_be_call_pre(void)
+{
+   unsigned long flags;
+
+   kernel_neon_begin();
+   spin_lock_irqsave(efi_be_rt_lock, flags);
+   cpu_switch_mm(idmap_pg_dir, init_mm);
+   flush_tlb_all();
+   return flags;
+}
+
+static void efi_be_call_post(unsigned long flags)
+{
+   cpu_switch_mm(current, current-active_mm);
+   flush_tlb_all();
+   spin_unlock_irqrestore(efi_be_rt_lock, flags);
+   kernel_neon_end();
+}
+
+static efi_status_t efi_be_get_variable(efi_char16_t *name,
+   efi_guid_t *vendor,
+   u32 *attr,
+   unsigned long *data_size,
+   void *data)
+{
+   unsigned long flags;
+   efi_status_t status;
+
+   *data_size = cpu_to_le64(*data_size);
+   flags = efi_be_call_pre();
+   status = efi_be_call(le64_to_cpu(runtime-get_variable),
+virt_to_phys(name), virt_to_phys(vendor),
+virt_to_phys(attr), virt_to_phys(data_size),
+virt_to_phys(data));
+   efi_be_call_post(flags);
+   *attr = le32_to_cpu(*attr);
+   *data_size = le64_to_cpu(*data_size);
+   return status;
+}
+
+static efi_status_t efi_be_get_next_variable(unsigned long *name_size,
+efi_char16_t *name,
+efi_guid_t *vendor)
+{
+   unsigned long flags;
+   efi_status_t status;
+
+   *name_size = cpu_to_le64(*name_size);
+   flags = efi_be_call_pre();
+   status = efi_be_call(le64_to_cpu(runtime-get_next_variable),
+