Re: [U-Boot] [PATCH 5/6] ARM: extend non-secure switch to also go into HYP mode

2013-05-31 Thread Andre Przywara

On 05/31/2013 07:43 AM, Christoffer Dall wrote:

On Mon, May 06, 2013 at 03:17:49PM +0200, Andre Przywara wrote:

For the KVM and XEN hypervisors to be usable, we need to enter the
kernel in HYP mode. Now that we already are in non-secure state,
HYP mode switching is within short reach.

While doing the non-secure switch, we have to enable the HVC
instruction and setup the HYP mode HVBAR (while still secure).

The actual switch is done by dropping back from a HYP mode handler
without actually leaving HYP mode, so we introduce a new handler
routine in the exception vector table.

In the assembly switching routine - which we rename to hyp_gic_switch
on the way - we save and restore the banked LR and SP registers
around the hypercall to do the actual HYP mode switch.

The C routine first checks whether we are in HYP mode already and
also whether the virtualization extensions are available. It also
checks whether the HYP mode switch was finally successful.
The bootm command part only adds and adjusts some error reporting.

Signed-off-by: Andre Przywara andre.przyw...@linaro.org
---
  arch/arm/cpu/armv7/start.S   | 34 +++---
  arch/arm/include/asm/armv7.h |  4 ++--
  arch/arm/lib/bootm.c | 12 +---
  arch/arm/lib/virt-v7.c   | 22 +++---
  4 files changed, 49 insertions(+), 23 deletions(-)

diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
index 02234c7..921e9d9 100644
--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -41,7 +41,7 @@ _start: b reset
ldr pc, _software_interrupt
ldr pc, _prefetch_abort
ldr pc, _data_abort
-   ldr pc, _not_used
+   ldr pc, _hyp_trap
ldr pc, _irq
ldr pc, _fiq
  #ifdef CONFIG_SPL_BUILD
@@ -49,7 +49,7 @@ _undefined_instruction: .word _undefined_instruction
  _software_interrupt:  .word _software_interrupt
  _prefetch_abort:  .word _prefetch_abort
  _data_abort:  .word _data_abort
-_not_used: .word _not_used
+_hyp_trap: .word _hyp_trap
  _irq: .word _irq
  _fiq: .word _fiq
  _pad: .word 0x12345678 /* now 16*4=64 */
@@ -58,7 +58,7 @@ _undefined_instruction: .word undefined_instruction
  _software_interrupt:  .word software_interrupt
  _prefetch_abort:  .word prefetch_abort
  _data_abort:  .word data_abort
-_not_used: .word not_used
+_hyp_trap: .word hyp_trap
  _irq: .word irq
  _fiq: .word fiq
  _pad: .word 0x12345678 /* now 16*4=64 */
@@ -513,12 +513,18 @@ software_interrupt:
mrc p15, 0, r1, c1, c1, 0   @ read SCR
bic r1, r1, #0x07f
orr r1, r1, #0x31   @ enable NS, AW, FW
+   mrc p15, 0, r0, c0, c1, 1   @ check for Virt ext
+   and r0, r0, #0xf000
+   cmp r0, #0x1000


you can just do ands r0, r0 ,#0xf000 and change the orreq below to orrne


Can I? But I want to make sure that Virt ext is of version 1 only, 
anything beyond that I cannot reliably support, right? Or is there a 
guarantee that this is backwards compatible?



+   orreq   r1, r1, #0x100  @ allow HVC instruction

mrc p15, 0, r0, c12, c0, 0  @ save secure copy of VBAR
mcr p15, 0, r1, c1, c1, 0   @ write SCR, switch to non-sec
isb
mcr p15, 0, r0, c12, c0, 0  @ write non-secure copy of VBAR

+   mcreq   p15, 4, r0, c12, c0, 0  @ write HYP mode HVBAR
+


nit: s/HYP mode//


movspc, lr

.align  5
@@ -534,10 +540,9 @@ data_abort:
bl  do_data_abort

.align  5
-not_used:
-   get_bad_stack
-   bad_save_user_regs
-   bl  do_not_used
+hyp_trap:
+   .byte 0x00, 0xe3, 0x0e, 0xe1@ mrs lr, elr_hyp


do we really need to support this on assemblers that old?


What do you mean with old? I can check again, but I think it didn't work 
with my self-compiled binutils 2.23. Or do I need a directive to enable 
this?



+   mov pc, lr

  #ifdef CONFIG_USE_IRQ

@@ -574,21 +579,21 @@ fiq:
  #endif /* CONFIG_SPL_BUILD */

  #ifdef CONFIG_ARMV7_VIRT
-/* Routine to initialize GIC CPU interface and switch to nonsecure state.
- * Will be executed directly by secondary CPUs after coming out of
+/* Routine to initialize GIC CPU interface, switch to nonsecure and to HYP
+ * mode. Will be executed directly by secondary CPUs after coming out of


So now this routine does three different things in different context at
once, why?


Not three. At most two. But actually it does only one thing: switch a 
single core to HYP mode. Only the entry and exit points are different.
I just see that I could make the smp_pen a separate function, calling 
the switch function. This would also solve this LR abuse thing.

Will try this.


   * WFI, or can be called 

Re: [U-Boot] [PATCH 5/6] ARM: extend non-secure switch to also go into HYP mode

2013-05-31 Thread Christoffer Dall
On Fri, May 31, 2013 at 11:34:38AM +0200, Andre Przywara wrote:
 On 05/31/2013 07:43 AM, Christoffer Dall wrote:
 On Mon, May 06, 2013 at 03:17:49PM +0200, Andre Przywara wrote:
 For the KVM and XEN hypervisors to be usable, we need to enter the
 kernel in HYP mode. Now that we already are in non-secure state,
 HYP mode switching is within short reach.
 
 While doing the non-secure switch, we have to enable the HVC
 instruction and setup the HYP mode HVBAR (while still secure).
 
 The actual switch is done by dropping back from a HYP mode handler
 without actually leaving HYP mode, so we introduce a new handler
 routine in the exception vector table.
 
 In the assembly switching routine - which we rename to hyp_gic_switch
 on the way - we save and restore the banked LR and SP registers
 around the hypercall to do the actual HYP mode switch.
 
 The C routine first checks whether we are in HYP mode already and
 also whether the virtualization extensions are available. It also
 checks whether the HYP mode switch was finally successful.
 The bootm command part only adds and adjusts some error reporting.
 
 Signed-off-by: Andre Przywara andre.przyw...@linaro.org
 ---
   arch/arm/cpu/armv7/start.S   | 34 +++---
   arch/arm/include/asm/armv7.h |  4 ++--
   arch/arm/lib/bootm.c | 12 +---
   arch/arm/lib/virt-v7.c   | 22 +++---
   4 files changed, 49 insertions(+), 23 deletions(-)
 
 diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
 index 02234c7..921e9d9 100644
 --- a/arch/arm/cpu/armv7/start.S
 +++ b/arch/arm/cpu/armv7/start.S
 @@ -41,7 +41,7 @@ _start: b reset
 ldr pc, _software_interrupt
 ldr pc, _prefetch_abort
 ldr pc, _data_abort
 -   ldr pc, _not_used
 +   ldr pc, _hyp_trap
 ldr pc, _irq
 ldr pc, _fiq
   #ifdef CONFIG_SPL_BUILD
 @@ -49,7 +49,7 @@ _undefined_instruction: .word _undefined_instruction
   _software_interrupt:  .word _software_interrupt
   _prefetch_abort:  .word _prefetch_abort
   _data_abort:  .word _data_abort
 -_not_used: .word _not_used
 +_hyp_trap: .word _hyp_trap
   _irq: .word _irq
   _fiq: .word _fiq
   _pad: .word 0x12345678 /* now 16*4=64 */
 @@ -58,7 +58,7 @@ _undefined_instruction: .word undefined_instruction
   _software_interrupt:  .word software_interrupt
   _prefetch_abort:  .word prefetch_abort
   _data_abort:  .word data_abort
 -_not_used: .word not_used
 +_hyp_trap: .word hyp_trap
   _irq: .word irq
   _fiq: .word fiq
   _pad: .word 0x12345678 /* now 16*4=64 */
 @@ -513,12 +513,18 @@ software_interrupt:
 mrc p15, 0, r1, c1, c1, 0   @ read SCR
 bic r1, r1, #0x07f
 orr r1, r1, #0x31   @ enable NS, AW, FW
 +   mrc p15, 0, r0, c0, c1, 1   @ check for Virt ext
 +   and r0, r0, #0xf000
 +   cmp r0, #0x1000
 
 you can just do ands r0, r0 ,#0xf000 and change the orreq below to orrne
 
 Can I? But I want to make sure that Virt ext is of version 1 only,
 anything beyond that I cannot reliably support, right? Or is there a
 guarantee that this is backwards compatible?

In the ARMv7 specifications it mentions either 1 or 0 as the possible
values of this field, and doesn't say anything about reserved values
iirc, but maybe that shouldn't be taken that literally.  Just keep the
code as it is then.

 
 +   orreq   r1, r1, #0x100  @ allow HVC instruction
 
 mrc p15, 0, r0, c12, c0, 0  @ save secure copy of VBAR
 mcr p15, 0, r1, c1, c1, 0   @ write SCR, switch to non-sec
 isb
 mcr p15, 0, r0, c12, c0, 0  @ write non-secure copy of VBAR
 
 +   mcreq   p15, 4, r0, c12, c0, 0  @ write HYP mode HVBAR
 +
 
 nit: s/HYP mode//
 
 movspc, lr
 
 .align  5
 @@ -534,10 +540,9 @@ data_abort:
 bl  do_data_abort
 
 .align  5
 -not_used:
 -   get_bad_stack
 -   bad_save_user_regs
 -   bl  do_not_used
 +hyp_trap:
 +   .byte 0x00, 0xe3, 0x0e, 0xe1@ mrs lr, elr_hyp
 
 do we really need to support this on assemblers that old?
 
 What do you mean with old? I can check again, but I think it didn't
 work with my self-compiled binutils 2.23. Or do I need a directive
 to enable this?
 

You need a directive, see the Makefile in arch/arm/kvm/Makefile and the
corresponding source files in the kernel.

 +   mov pc, lr
 
   #ifdef CONFIG_USE_IRQ
 
 @@ -574,21 +579,21 @@ fiq:
   #endif /* CONFIG_SPL_BUILD */
 
   #ifdef CONFIG_ARMV7_VIRT
 -/* Routine to initialize GIC CPU interface and switch to nonsecure state.
 - * Will be executed directly by secondary CPUs after coming out of
 +/* Routine to initialize GIC CPU interface, switch to nonsecure and to HYP
 + * mode. Will be executed directly by secondary CPUs after coming out of
 

Re: [U-Boot] [PATCH 5/6] ARM: extend non-secure switch to also go into HYP mode

2013-05-30 Thread Christoffer Dall
On Mon, May 06, 2013 at 03:17:49PM +0200, Andre Przywara wrote:
 For the KVM and XEN hypervisors to be usable, we need to enter the
 kernel in HYP mode. Now that we already are in non-secure state,
 HYP mode switching is within short reach.
 
 While doing the non-secure switch, we have to enable the HVC
 instruction and setup the HYP mode HVBAR (while still secure).
 
 The actual switch is done by dropping back from a HYP mode handler
 without actually leaving HYP mode, so we introduce a new handler
 routine in the exception vector table.
 
 In the assembly switching routine - which we rename to hyp_gic_switch
 on the way - we save and restore the banked LR and SP registers
 around the hypercall to do the actual HYP mode switch.
 
 The C routine first checks whether we are in HYP mode already and
 also whether the virtualization extensions are available. It also
 checks whether the HYP mode switch was finally successful.
 The bootm command part only adds and adjusts some error reporting.
 
 Signed-off-by: Andre Przywara andre.przyw...@linaro.org
 ---
  arch/arm/cpu/armv7/start.S   | 34 +++---
  arch/arm/include/asm/armv7.h |  4 ++--
  arch/arm/lib/bootm.c | 12 +---
  arch/arm/lib/virt-v7.c   | 22 +++---
  4 files changed, 49 insertions(+), 23 deletions(-)
 
 diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
 index 02234c7..921e9d9 100644
 --- a/arch/arm/cpu/armv7/start.S
 +++ b/arch/arm/cpu/armv7/start.S
 @@ -41,7 +41,7 @@ _start: b   reset
   ldr pc, _software_interrupt
   ldr pc, _prefetch_abort
   ldr pc, _data_abort
 - ldr pc, _not_used
 + ldr pc, _hyp_trap
   ldr pc, _irq
   ldr pc, _fiq
  #ifdef CONFIG_SPL_BUILD
 @@ -49,7 +49,7 @@ _undefined_instruction: .word _undefined_instruction
  _software_interrupt: .word _software_interrupt
  _prefetch_abort: .word _prefetch_abort
  _data_abort: .word _data_abort
 -_not_used:   .word _not_used
 +_hyp_trap:   .word _hyp_trap
  _irq:.word _irq
  _fiq:.word _fiq
  _pad:.word 0x12345678 /* now 16*4=64 */
 @@ -58,7 +58,7 @@ _undefined_instruction: .word undefined_instruction
  _software_interrupt: .word software_interrupt
  _prefetch_abort: .word prefetch_abort
  _data_abort: .word data_abort
 -_not_used:   .word not_used
 +_hyp_trap:   .word hyp_trap
  _irq:.word irq
  _fiq:.word fiq
  _pad:.word 0x12345678 /* now 16*4=64 */
 @@ -513,12 +513,18 @@ software_interrupt:
   mrc p15, 0, r1, c1, c1, 0   @ read SCR
   bic r1, r1, #0x07f
   orr r1, r1, #0x31   @ enable NS, AW, FW
 + mrc p15, 0, r0, c0, c1, 1   @ check for Virt ext
 + and r0, r0, #0xf000
 + cmp r0, #0x1000

you can just do ands r0, r0 ,#0xf000 and change the orreq below to orrne

 + orreq   r1, r1, #0x100  @ allow HVC instruction
  
   mrc p15, 0, r0, c12, c0, 0  @ save secure copy of VBAR
   mcr p15, 0, r1, c1, c1, 0   @ write SCR, switch to non-sec
   isb
   mcr p15, 0, r0, c12, c0, 0  @ write non-secure copy of VBAR
  
 + mcreq   p15, 4, r0, c12, c0, 0  @ write HYP mode HVBAR
 +

nit: s/HYP mode//

   movspc, lr
  
   .align  5
 @@ -534,10 +540,9 @@ data_abort:
   bl  do_data_abort
  
   .align  5
 -not_used:
 - get_bad_stack
 - bad_save_user_regs
 - bl  do_not_used
 +hyp_trap:
 + .byte 0x00, 0xe3, 0x0e, 0xe1@ mrs lr, elr_hyp

do we really need to support this on assemblers that old?

 + mov pc, lr
  
  #ifdef CONFIG_USE_IRQ
  
 @@ -574,21 +579,21 @@ fiq:
  #endif /* CONFIG_SPL_BUILD */
  
  #ifdef CONFIG_ARMV7_VIRT
 -/* Routine to initialize GIC CPU interface and switch to nonsecure state.
 - * Will be executed directly by secondary CPUs after coming out of
 +/* Routine to initialize GIC CPU interface, switch to nonsecure and to HYP
 + * mode. Will be executed directly by secondary CPUs after coming out of

So now this routine does three different things in different context at
once, why?

   * WFI, or can be called directly by C code for CPU 0.
   * Those two paths mandate to not use any stack and to only use registers
   * r0-r3 to comply with both the C ABI and the requirement of SMP startup
   * code.
   */
 -.globl _nonsec_gic_switch
 +.globl _hyp_gic_switch
  .globl _smp_pen
  _smp_pen:
   mrs r0, cpsr
   orr r0, r0, #0xc0
   msr cpsr, r0@ disable interrupts
   mov lr, #0  @ clear LR to mark secondary
 -_nonsec_gic_switch:
 +_hyp_gic_switch:
   mrc p15, 4, r2, c15, c0, 0  @ r2 = PERIPHBASE
   add r3, r2, #0x1000 @ GIC dist 

Re: [U-Boot] [PATCH 5/6] ARM: extend non-secure switch to also go into HYP mode

2013-05-09 Thread Tom Rini
On Mon, May 06, 2013 at 03:17:49PM +0200, Andre Przywara wrote:

[snip]
 - printf(HYP mode: Security extensions not implemented.\n);
 + printf(HYP mode: Virtualization extensions not 
 implemented.\n);

When we don't need printf-modifiers, using puts is preferred.

And I leave review of the implementation to others that know the area,
but nothing jumps out at me.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot