On Thu, Jun 13, 2013 at 01:01:12PM +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 our new secure exception vector table.
>
> In the assembly switching routine 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@xxxxxxxxxx>
> ---
> arch/arm/cpu/armv7/nonsec_virt.S | 31 ++++++++++++++++++++++++++++---
> arch/arm/include/asm/armv7.h | 7 +++++--
> arch/arm/lib/bootm.c | 14 ++++++++++----
> arch/arm/lib/virt-v7.c | 27 ++++++++++++++++++++++-----
> 4 files changed, 65 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S
> index 919f6e9..950da6f 100644
> --- a/arch/arm/cpu/armv7/nonsec_virt.S
> +++ b/arch/arm/cpu/armv7/nonsec_virt.S
> @@ -1,5 +1,5 @@
> /*
> - * code for switching cores into non-secure state
> + * code for switching cores into non-secure state and into HYP mode
> *
> * Copyright (c) 2013 Andre Przywara <andre.przywara@xxxxxxxxxx>
> *
> @@ -26,14 +26,14 @@
> #include <asm/gic.h>
> #include <asm/armv7.h>
>
> -/* the vector table for secure state */
> +/* the vector table for secure state and HYP mode */
> _secure_vectors:
> .word 0 /* reset */
> .word 0 /* undef */
> adr pc, _secure_monitor
> .word 0
> .word 0
> - .word 0
> + adr pc, _hyp_trap
> .word 0
> .word 0
> .word 0 /* pad */
> @@ -50,10 +50,23 @@ _secure_monitor:
> bic r1, r1, #0x4e @ clear IRQ, FIQ, EA, nET bits
> orr r1, r1, #0x31 @ enable NS, AW, FW bits
>
> + mrc p15, 0, r0, c0, c1, 1 @ read ID_PFR1
> + and r0, r0, #CPUID_ARM_VIRT_MASK @ mask virtualization bits
> + cmp r0, #(1 << CPUID_ARM_VIRT_SHIFT)
> + orreq r1, r1, #0x100 @ allow HVC instruction
> +
> mcr p15, 0, r1, c1, c1, 0 @ write SCR (with NS bit set)
>
> + mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value
Why not just do load the address of _secure_vectors directly? I think it
makes it more clear what happens.
> + mcreq p15, 4, r0, c12, c0, 0 @ write HVBAR
> +
> movs pc, lr @ return to non-secure SVC
>
> +_hyp_trap:
> + .byte 0x00, 0xe3, 0x0e, 0xe1 @ mrs lr, elr_hyp
Again, why not just add the necessary .arch_extension or assembler
directive in the makefile and use the instructions directly.
The only reason I would see for performing this obscurity would be to
support really old compilers, which I doubt we fill for future boards?
> + mov pc, lr @ do no switch modes, but
> + @ return to caller
> +
> /*
> * Secondary CPUs start here and call the code for the core specific parts
> * of the non-secure and HYP mode transition. The GIC distributor specific
> @@ -69,6 +82,7 @@ _smp_pen:
> mcr p15, 0, r1, c12, c0, 0 @ set VBAR
>
> bl _nonsec_init
> + bl _hyp_init
>
> ldr r1, [r3, #0x0c] @ read GICD acknowledge
> str r1, [r3, #0x10] @ write GICD EOI
> @@ -145,3 +159,14 @@ _nonsec_init:
> str r1, [r2] @ allow private interrupts
>
> bx lr
> +
> +.globl _hyp_init
> +_hyp_init:
nit: the naming here is a little misleading for someone not knowing
what's going on. You're not really initializing the mode, but switching
to it:
_switch_to_hyp: ???
> + mov r2, lr
> + mov r3, sp @ save SVC copy of LR and SP
> + isb
I don't think this isb is necessary.
> + .byte 0x70, 0x00, 0x40, 0xe1 @ hvc #0
again, this is really funky. If it's really necessary, can we please
have a define somewhere so you can just do HVC(0) instead?
> + mov sp, r3
> + mov lr, r2 @ fix HYP mode banked LR and SP
nit: s/fix HYP mode/restore S-SVC mode/
> +
> + bx lr
> diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
> index 04545b9..8c3a85e 100644
> --- a/arch/arm/include/asm/armv7.h
> +++ b/arch/arm/include/asm/armv7.h
> @@ -89,15 +89,18 @@ void v7_outer_cache_inval_range(u32 start, u32 end);
>
> #ifdef CONFIG_ARMV7_VIRT
>
> -#define HYP_ERR_NO_SEC_EXT 2
> +#define HYP_ERR_ALREADY_HYP_MODE 1
> +#define HYP_ERR_NO_VIRT_EXT 2
> #define HYP_ERR_NO_GIC_ADDRESS 3
> #define HYP_ERR_GIC_ADDRESS_ABOVE_4GB 4
> +#define HYP_ERR_NOT_HYP_MODE 5
see, this is just messy. I really don't think you need to propogate
error values like this.
>
> -int armv7_switch_nonsec(void);
> +int armv7_switch_hyp(void);
>
> /* defined in cpu/armv7/nonsec_virt.S */
> void _nonsec_init(void);
> void _smp_pen(void);
> +void _hyp_init(void);
> #endif /* CONFIG_ARMV7_VIRT */
>
> #endif /* ! __ASSEMBLY__ */
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index 8251a89..7edd84d 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -227,12 +227,15 @@ static void boot_prep_linux(bootm_headers_t *images)
> hang();
> }
> #ifdef CONFIG_ARMV7_VIRT
> - switch (armv7_switch_nonsec()) {
> + switch (armv7_switch_hyp()) {
> case 0:
> - debug("entered non-secure state\n");
> + debug("entered HYP mode\n");
> break;
> - case HYP_ERR_NO_SEC_EXT:
> - printf("HYP mode: Security extensions not implemented.\n");
> + case HYP_ERR_ALREADY_HYP_MODE:
> + debug("CPU already in HYP mode\n");
> + break;
> + case HYP_ERR_NO_VIRT_EXT:
> + printf("HYP mode: Virtualization extensions not implemented.\n");
> break;
> case HYP_ERR_NO_GIC_ADDRESS:
> printf("HYP mode: could not determine GIC address.\n");
> @@ -240,6 +243,9 @@ static void boot_prep_linux(bootm_headers_t *images)
> case HYP_ERR_GIC_ADDRESS_ABOVE_4GB:
> printf("HYP mode: PERIPHBASE is above 4 GB, cannot access this.\n");
> break;
> + case HYP_ERR_NOT_HYP_MODE:
> + printf("HYP mode: switch not successful.\n");
> + break;
> }
> #endif
> }
> diff --git a/arch/arm/lib/virt-v7.c b/arch/arm/lib/virt-v7.c
> index 6946e4d..1e206b9 100644
> --- a/arch/arm/lib/virt-v7.c
> +++ b/arch/arm/lib/virt-v7.c
> @@ -3,6 +3,7 @@
> * Andre Przywara, Linaro
> *
> * Routines to transition 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
> @@ -29,6 +30,14 @@
> #include <asm/gic.h>
> #include <asm/io.h>
>
> +static unsigned int read_cpsr(void)
> +{
> + unsigned int reg;
> +
> + asm volatile ("mrs %0, cpsr\n" : "=r" (reg));
> + return reg;
> +}
> +
> static unsigned int read_id_pfr1(void)
> {
> unsigned int reg;
> @@ -110,16 +119,20 @@ static void kick_secondary_cpus(char *gicdptr)
> writel(1U << 24, &gicdptr[GICD_SGIR]);
> }
>
> -int armv7_switch_nonsec(void)
> +int armv7_switch_hyp(void)
> {
> unsigned int reg, ret;
> char *gicdptr;
> unsigned itlinesnr, i;
>
> - /* check whether the CPU supports the security extensions */
> + /* check whether we are in HYP mode already */
> + if ((read_cpsr() & 0x1f) == 0x1a)
> + return HYP_ERR_ALREADY_HYP_MODE;
> +
> + /* check whether the CPU supports the virtualization extensions */
> reg = read_id_pfr1();
> - if ((reg & 0xF0) == 0)
> - return HYP_ERR_NO_SEC_EXT;
> + if ((reg & CPUID_ARM_VIRT_MASK) != 1 << CPUID_ARM_VIRT_SHIFT)
> + return HYP_ERR_NO_VIRT_EXT;
>
> set_generic_timer_frequency();
>
> @@ -147,8 +160,12 @@ int armv7_switch_nonsec(void)
>
> kick_secondary_cpus(gicdptr);
>
> - /* call the non-sec switching code on this CPU also */
> + /* call the HYP switching code on this CPU also */
> _nonsec_init();
> + _hyp_init();
> +
> + if ((read_cpsr() & 0x1F) != 0x1a)
> + return HYP_ERR_NOT_HYP_MODE;
>
> return 0;
> }
> --
> 1.7.12.1
>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
