[U-Boot] [PATCH 5/6] ARM: extend non-secure switch to also go into HYP mode
Andre Przywara
andre.przywara at linaro.org
Fri May 31 11:34:38 CEST 2013
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.przywara at 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//
>
>> movs pc, 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 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 i/f offset
>> mvn r1, #0
>> @@ -628,6 +633,13 @@ _nonsec_gic_switch:
>> add r2, r2, #0x1000 @ GIC dist i/f offset
>> str r1, [r2] @ allow private interrupts
>>
>> + mov r2, lr
>> + mov r1, sp
>> + .byte 0x70, 0x00, 0x40, 0xe1 @ hvc #0
>> + isb
>
> again, I'm doubtful this isb is necessary when you just did an exception
> return.
Good point. Exception returns should do this automatically, right? Is
this an official fact or just a feeling?
...
>> diff --git a/arch/arm/lib/virt-v7.c b/arch/arm/lib/virt-v7.c
>> index 0248010..3883463 100644
>> --- a/arch/arm/lib/virt-v7.c
>> +++ b/arch/arm/lib/virt-v7.c
>> @@ -3,6 +3,7 @@
>> * Andre Przywara, Linaro
>> *
>> * routines to push ARMv7 processors from secure into non-secure state
>> + * and from non-secure SVC into HYP mode
>> * needed to enable ARMv7 virtualization for current hypervisors
>> *
>> * See file CREDITS for list of people who contributed to this
>> @@ -43,16 +44,20 @@ static inline unsigned int read_cpsr(void)
>> return reg;
>> }
>>
>> -int armv7_switch_nonsec(void)
>> +int armv7_switch_hyp(void)
>> {
>> unsigned int reg;
>> volatile unsigned int *gicdptr;
>> unsigned itlinesnr, i;
>> unsigned int *sysflags;
>>
>> - /* check whether the CPU supports the security extensions */
>> + /* check whether we are in HYP mode already */
>> + if ((read_cpsr() & 0x1F) == 0x1a)
>> + return 1;
>> +
>> + /* check whether the CPU supports the virtualization extensions */
>> asm("mrc p15, 0, %0, c0, c1, 1\n" : "=r"(reg));
>> - if ((reg & 0xF0) == 0)
>> + if ((reg & 0xF000) != 0x1000)
>> return 2;
>>
>> /* the timer frequency for the generic timer needs to be
>> @@ -73,8 +78,8 @@ int armv7_switch_nonsec(void)
>> */
>>
>> /* check whether we are an Cortex-A15 or A7.
>> - * The actual non-secure switch should work with all CPUs supporting
>> - * the security extension, but we need the GIC address,
>> + * The actual HYP switch should work with all CPUs supporting
>> + * the virtualization extension, but we need the GIC address,
>> * which we know only for sure for those two CPUs.
>> */
>> asm("mrc p15, 0, %0, c0, c0, 0\n" : "=r"(reg));
>> @@ -113,8 +118,11 @@ int armv7_switch_nonsec(void)
>> sysflags[0] = (uintptr_t)_smp_pen;
>> gicdptr[GICD_SGIR / 4] = 1U << 24;
>>
>> - /* call the non-sec switching code on this CPU also */
>> - _nonsec_gic_switch();
>> + /* call the HYP switching code on this CPU also */
>> + _hyp_gic_switch();
>> +
>> + if ((read_cpsr() & 0x1F) != 0x1a)
>> + return 5;
>
> this is really a fatal crash right? We probably don't want to try and
> proceed with boot at this point.
Well not really. If we reach this point without being in HYP mode, that
just hasn't worked - for various reasons. Since this new bootm
functionality is unconditional, I felt we should better not crash.
Actually Linux will run fine, just without KVM (for certains meanings of
"fine" that is - as a KVM developer ;-)
Regards,
Andre.
>
>>
>> return 0;
>> }
>> --
>> 1.7.12.1
>>
More information about the U-Boot
mailing list