Re: [PATCH 6/7] ARM: KVM: switch to a dual-step HYP init code
On Fri, Apr 05, 2013 at 10:08:04AM +0100, Marc Zyngier wrote: On 04/04/13 23:10, Geoff Levand wrote: Hi, On Tue, 2013-04-02 at 14:25 +0100, Marc Zyngier wrote: + @ Jump to the trampoline page + ldr r2, =#PAGE_MASK + adr r3, target + bic r3, r3, r2 + ldr r2, =#TRAMPOLINE_VA + add r3, r3, r2 + mov pc, r3 I guess you need 'ldr r2, =PAGE_MASK'. arch/arm/kvm/init.S:114: Error: bad expression -- `ldr r2,=#(~((112)-1))' arch/arm/kvm/init.S:117: Error: bad expression -- `ldr r2,=#0x' Oddly enough, this code compiles perfectly fine on my box. What's your compiler/binutils versions? The standard format for this is: ldr rd, =value without a '#' and has been that way for as long as I remember binutils accepting that format. It's entirely possible that later binutils has decided to be a bit more flexible by allowing the '#' in there, but that's something which will be incompatible with older versions. Best loose the '#' in there. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] ARM: KVM: switch to a dual-step HYP init code
On 18/04/13 16:54, Russell King - ARM Linux wrote: On Fri, Apr 05, 2013 at 10:08:04AM +0100, Marc Zyngier wrote: On 04/04/13 23:10, Geoff Levand wrote: Hi, On Tue, 2013-04-02 at 14:25 +0100, Marc Zyngier wrote: + @ Jump to the trampoline page + ldr r2, =#PAGE_MASK + adr r3, target + bic r3, r3, r2 + ldr r2, =#TRAMPOLINE_VA + add r3, r3, r2 + mov pc, r3 I guess you need 'ldr r2, =PAGE_MASK'. arch/arm/kvm/init.S:114: Error: bad expression -- `ldr r2,=#(~((112)-1))' arch/arm/kvm/init.S:117: Error: bad expression -- `ldr r2,=#0x' Oddly enough, this code compiles perfectly fine on my box. What's your compiler/binutils versions? The standard format for this is: ldr rd, =value without a '#' and has been that way for as long as I remember binutils accepting that format. It's entirely possible that later binutils has decided to be a bit more flexible by allowing the '#' in there, but that's something which will be incompatible with older versions. Best loose the '#' in there. Indeed. I've fixed the code in a later version of the patch. Thanks, M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] ARM: KVM: switch to a dual-step HYP init code
On 04/04/13 23:10, Geoff Levand wrote: Hi, On Tue, 2013-04-02 at 14:25 +0100, Marc Zyngier wrote: +@ Jump to the trampoline page +ldr r2, =#PAGE_MASK +adr r3, target +bic r3, r3, r2 +ldr r2, =#TRAMPOLINE_VA +add r3, r3, r2 +mov pc, r3 I guess you need 'ldr r2, =PAGE_MASK'. arch/arm/kvm/init.S:114: Error: bad expression -- `ldr r2,=#(~((112)-1))' arch/arm/kvm/init.S:117: Error: bad expression -- `ldr r2,=#0x' Oddly enough, this code compiles perfectly fine on my box. What's your compiler/binutils versions? Thanks, M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] ARM: KVM: switch to a dual-step HYP init code
Hi Marc, On Fri, 2013-04-05 at 10:08 +0100, Marc Zyngier wrote: On 04/04/13 23:10, Geoff Levand wrote: On Tue, 2013-04-02 at 14:25 +0100, Marc Zyngier wrote: + @ Jump to the trampoline page + ldr r2, =#PAGE_MASK + adr r3, target + bic r3, r3, r2 + ldr r2, =#TRAMPOLINE_VA + add r3, r3, r2 + mov pc, r3 I guess you need 'ldr r2, =PAGE_MASK'. arch/arm/kvm/init.S:114: Error: bad expression -- `ldr r2,=#(~((112)-1))' arch/arm/kvm/init.S:117: Error: bad expression -- `ldr r2,=#0x' Oddly enough, this code compiles perfectly fine on my box. What's your compiler/binutils versions? I thought # was for an immediate value in instructions. This is an assembler pseudo-instruction, and based on the ARM manual here I would think your coding should fail: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0473i/Chdcegci.html I use the current arm-linux-gnueabihf cross toolchain packages from Ubuntu 12.04: arm-linux-gnueabihf-gcc-4.6 (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3 GNU assembler (GNU Binutils for Ubuntu) 2.22 -Geoff -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] ARM: KVM: switch to a dual-step HYP init code
On 05/04/13 17:46, Geoff Levand wrote: Hi Geoff, On Fri, 2013-04-05 at 10:08 +0100, Marc Zyngier wrote: On 04/04/13 23:10, Geoff Levand wrote: On Tue, 2013-04-02 at 14:25 +0100, Marc Zyngier wrote: + @ Jump to the trampoline page + ldr r2, =#PAGE_MASK + adr r3, target + bic r3, r3, r2 + ldr r2, =#TRAMPOLINE_VA + add r3, r3, r2 + mov pc, r3 I guess you need 'ldr r2, =PAGE_MASK'. arch/arm/kvm/init.S:114: Error: bad expression -- `ldr r2,=#(~((112)-1))' arch/arm/kvm/init.S:117: Error: bad expression -- `ldr r2,=#0x' Oddly enough, this code compiles perfectly fine on my box. What's your compiler/binutils versions? I thought # was for an immediate value in instructions. This is an assembler pseudo-instruction, and based on the ARM manual here I would think your coding should fail: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0473i/Chdcegci.html I don't dispute the validity of your remark. I just find odd that my compiler doesn't barf on this as it probably should. I use the current arm-linux-gnueabihf cross toolchain packages from Ubuntu 12.04: arm-linux-gnueabihf-gcc-4.6 (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3 GNU assembler (GNU Binutils for Ubuntu) 2.22 I'm using this: arm-linux-gnueabihf-gcc (crosstool-NG linaro-1.13.1-4.7-2012.12-20121214 - Linaro GCC 2012.12) 4.7.3 20121205 (prerelease) GNU assembler (crosstool-NG linaro-1.13.1-4.7-2012.12-20121214 - Linaro GCC 2012.12) 2.22 Looks like there's a regression somewhere. Thanks, M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] ARM: KVM: switch to a dual-step HYP init code
On 04/04/13 00:15, Christoffer Dall wrote: On Wed, Apr 03, 2013 at 11:38:30AM +0100, Marc Zyngier wrote: On 03/04/13 11:07, Will Deacon wrote: On Tue, Apr 02, 2013 at 02:25:14PM +0100, Marc Zyngier wrote: Our HYP init code suffers from two major design issues: - it cannot support CPU hotplug, as we tear down the idmap very early - it cannot perform a TLB invalidation when switching from init to runtime mappings, as pages are manipulated from PL1 exclusively [...] diff --git a/arch/arm/kvm/init.S b/arch/arm/kvm/init.S index 35a463f..b2c6967 100644 --- a/arch/arm/kvm/init.S +++ b/arch/arm/kvm/init.S @@ -21,6 +21,7 @@ #include asm/asm-offsets.h #include asm/kvm_asm.h #include asm/kvm_arm.h +#include asm/kvm_mmu.h / * Hypervisor initialization @@ -47,6 +48,9 @@ __kvm_hyp_init: W(b). __do_hyp_init: + cmp r2, #0 @ We have a SP? + bne phase2 @ Yes, second stage init + @ Set the HTTBR to point to the hypervisor PGD pointer passed mcrrp15, 4, r0, r1, c2 @@ -96,14 +100,35 @@ __do_hyp_init: orr r0, r0, r1 isb mcr p15, 4, r0, c1, c0, 0 @ HSCR - isb - @ Set stack pointer and return to the kernel + eret + +phase2: + @ Set stack pointer mov sp, r2 @ Set HVBAR to point to the HYP vectors mcr p15, 4, r3, c12, c0, 0 @ HVBAR + @ Jump to the trampoline page + ldr r2, =#PAGE_MASK Shifting right by PAGE_SHIFT can avoid the load. Not really. We're masking out the top bits of target and adding them to the trampoline base address, so shifting doesn't help. But, as you suggested offline, BFI can come to the rescue and make that code totally fun and unreadable. How about (untested): ldr r2, =#TRAMPOLINE_VA adr r3, target bfi r2, r3, #0, #PAGE_SHIFT mov pc, r2 I really like it! :) What kind of drugs are you on? Ok, I actually like it too. Implemented, tested, works. + adr r3, target + bic r3, r3, r2 + ldr r2, =#TRAMPOLINE_VA + add r3, r3, r2 + mov pc, r3 + + nop insert dead chicken and voodoo chant here ... You know I'll never sleep no more ... Seriously, what kind of drugs are you guys on? Someone did comment last year about the quality of the water in Cambridge. He may have been right. But in this occurrence, it's only a mild case of Frank Zappatis (Zomby Woof variety). M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] ARM: KVM: switch to a dual-step HYP init code
On 04/04/13 00:15, Christoffer Dall wrote: On Tue, Apr 02, 2013 at 02:25:14PM +0100, Marc Zyngier wrote: Our HYP init code suffers from two major design issues: - it cannot support CPU hotplug, as we tear down the idmap very early - it cannot perform a TLB invalidation when switching from init to runtime mappings, as pages are manipulated from PL1 exclusively The hotplug problem mandates that we keep two sets of page tables (boot and runtime). The TLB problem mandates that we're able to transition from one PGD to another while in HYP, invalidating the TLBs in the process. To be able to do this, we need to share a page between the two page tables. A page that will have the same VA in both configurations. All we need is a VA that has the following properties: - This VA can't be used to represent a kernel mapping. - This VA will not conflict with the physical address of the kernel text The vectors page seems to satisfy this requirement: - The kernel never maps anything else there - The kernel text being copied at the beginning of the physical memory, it is unlikely to use the last 64kB (I doubt we'll ever support KVM on a system with something like 4MB of RAM, but patches are very welcome). Let's call this VA the trampoline VA. Now, we map our init page at 3 locations: - idmap in the boot pgd - trampoline VA in the boot pgd - trampoline VA in the runtime pgd The init scenario is now the following: - We jump in HYP with four parameters: boot HYP pgd, runtime HYP pgd, runtime stack, runtime vectors - Enable the MMU with the boot pgd - Jump to a target into the trampoline page (remember, this is the same physical page!) - Now switch to the runtime pgd (same VA, and still the same physical page!) - Invalidate TLBs - Set stack and vectors - Profit! (or eret, if you only care about the code). So I'm going to do my usual commenting routine. Was it an idea to insert this commit text (which I really liked by the way!) into init.S where the current comment is a little lacking giving the massive complexity this is turning into, madness? Will do. [...] diff --git a/arch/arm/kvm/init.S b/arch/arm/kvm/init.S index 35a463f..b2c6967 100644 --- a/arch/arm/kvm/init.S +++ b/arch/arm/kvm/init.S @@ -21,6 +21,7 @@ #include asm/asm-offsets.h #include asm/kvm_asm.h #include asm/kvm_arm.h +#include asm/kvm_mmu.h / * Hypervisor initialization @@ -47,6 +48,9 @@ __kvm_hyp_init: W(b). __do_hyp_init: + cmp r2, #0 @ We have a SP? + bne phase2 @ Yes, second stage init + @ Set the HTTBR to point to the hypervisor PGD pointer passed mcrrp15, 4, r0, r1, c2 @@ -96,14 +100,35 @@ __do_hyp_init: orr r0, r0, r1 isb mcr p15, 4, r0, c1, c0, 0 @ HSCR - isb - @ Set stack pointer and return to the kernel + eret Could you add some comment here to indicate we're done with phase1, it seems like this eret should not go unnoticed by casual readers (ok, they shouldn't read this code casually, but anyway..., it will make me sleep better) Sure, no problem. M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] ARM: KVM: switch to a dual-step HYP init code
Hi, On Tue, 2013-04-02 at 14:25 +0100, Marc Zyngier wrote: + @ Jump to the trampoline page + ldr r2, =#PAGE_MASK + adr r3, target + bic r3, r3, r2 + ldr r2, =#TRAMPOLINE_VA + add r3, r3, r2 + mov pc, r3 I guess you need 'ldr r2, =PAGE_MASK'. arch/arm/kvm/init.S:114: Error: bad expression -- `ldr r2,=#(~((112)-1))' arch/arm/kvm/init.S:117: Error: bad expression -- `ldr r2,=#0x' -Geoff -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] ARM: KVM: switch to a dual-step HYP init code
On Tue, Apr 02, 2013 at 02:25:14PM +0100, Marc Zyngier wrote: Our HYP init code suffers from two major design issues: - it cannot support CPU hotplug, as we tear down the idmap very early - it cannot perform a TLB invalidation when switching from init to runtime mappings, as pages are manipulated from PL1 exclusively [...] diff --git a/arch/arm/kvm/init.S b/arch/arm/kvm/init.S index 35a463f..b2c6967 100644 --- a/arch/arm/kvm/init.S +++ b/arch/arm/kvm/init.S @@ -21,6 +21,7 @@ #include asm/asm-offsets.h #include asm/kvm_asm.h #include asm/kvm_arm.h +#include asm/kvm_mmu.h / * Hypervisor initialization @@ -47,6 +48,9 @@ __kvm_hyp_init: W(b). __do_hyp_init: + cmp r2, #0 @ We have a SP? + bne phase2 @ Yes, second stage init + @ Set the HTTBR to point to the hypervisor PGD pointer passed mcrrp15, 4, r0, r1, c2 @@ -96,14 +100,35 @@ __do_hyp_init: orr r0, r0, r1 isb mcr p15, 4, r0, c1, c0, 0 @ HSCR - isb - @ Set stack pointer and return to the kernel + eret + +phase2: + @ Set stack pointer mov sp, r2 @ Set HVBAR to point to the HYP vectors mcr p15, 4, r3, c12, c0, 0 @ HVBAR + @ Jump to the trampoline page + ldr r2, =#PAGE_MASK Shifting right by PAGE_SHIFT can avoid the load. + adr r3, target + bic r3, r3, r2 + ldr r2, =#TRAMPOLINE_VA + add r3, r3, r2 + mov pc, r3 + + nop insert dead chicken and voodoo chant here + +target: @ We're now in the trampoline code, switch page tables + mcrrp15, 4, r0, r1, c2 + isb + + @ Invalidate the old TLBs + mcr p15, 4, r0, c8, c7, 0 @ TLBIALLH + dsb + isb You don't actually need this isb (there's an eret next!). eret .ltorg Will -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] ARM: KVM: switch to a dual-step HYP init code
On 03/04/13 11:07, Will Deacon wrote: On Tue, Apr 02, 2013 at 02:25:14PM +0100, Marc Zyngier wrote: Our HYP init code suffers from two major design issues: - it cannot support CPU hotplug, as we tear down the idmap very early - it cannot perform a TLB invalidation when switching from init to runtime mappings, as pages are manipulated from PL1 exclusively [...] diff --git a/arch/arm/kvm/init.S b/arch/arm/kvm/init.S index 35a463f..b2c6967 100644 --- a/arch/arm/kvm/init.S +++ b/arch/arm/kvm/init.S @@ -21,6 +21,7 @@ #include asm/asm-offsets.h #include asm/kvm_asm.h #include asm/kvm_arm.h +#include asm/kvm_mmu.h / * Hypervisor initialization @@ -47,6 +48,9 @@ __kvm_hyp_init: W(b). __do_hyp_init: +cmp r2, #0 @ We have a SP? +bne phase2 @ Yes, second stage init + @ Set the HTTBR to point to the hypervisor PGD pointer passed mcrrp15, 4, r0, r1, c2 @@ -96,14 +100,35 @@ __do_hyp_init: orr r0, r0, r1 isb mcr p15, 4, r0, c1, c0, 0 @ HSCR -isb -@ Set stack pointer and return to the kernel +eret + +phase2: +@ Set stack pointer mov sp, r2 @ Set HVBAR to point to the HYP vectors mcr p15, 4, r3, c12, c0, 0 @ HVBAR +@ Jump to the trampoline page +ldr r2, =#PAGE_MASK Shifting right by PAGE_SHIFT can avoid the load. Not really. We're masking out the top bits of target and adding them to the trampoline base address, so shifting doesn't help. But, as you suggested offline, BFI can come to the rescue and make that code totally fun and unreadable. How about (untested): ldr r2, =#TRAMPOLINE_VA adr r3, target bfi r2, r3, #0, #PAGE_SHIFT mov pc, r2 I really like it! :) +adr r3, target +bic r3, r3, r2 +ldr r2, =#TRAMPOLINE_VA +add r3, r3, r2 +mov pc, r3 + +nop insert dead chicken and voodoo chant here ... You know I'll never sleep no more ... + +target: @ We're now in the trampoline code, switch page tables +mcrrp15, 4, r0, r1, c2 +isb + +@ Invalidate the old TLBs +mcr p15, 4, r0, c8, c7, 0 @ TLBIALLH +dsb +isb You don't actually need this isb (there's an eret next!). Good point. I'll remove it in V2. Thanks for reviewing, M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] ARM: KVM: switch to a dual-step HYP init code
On Tue, Apr 02, 2013 at 02:25:14PM +0100, Marc Zyngier wrote:
Our HYP init code suffers from two major design issues:
- it cannot support CPU hotplug, as we tear down the idmap very early
- it cannot perform a TLB invalidation when switching from init to
runtime mappings, as pages are manipulated from PL1 exclusively
The hotplug problem mandates that we keep two sets of page tables
(boot and runtime). The TLB problem mandates that we're able to
transition from one PGD to another while in HYP, invalidating the TLBs
in the process.
To be able to do this, we need to share a page between the two page
tables. A page that will have the same VA in both configurations. All we
need is a VA that has the following properties:
- This VA can't be used to represent a kernel mapping.
- This VA will not conflict with the physical address of the kernel text
The vectors page seems to satisfy this requirement:
- The kernel never maps anything else there
- The kernel text being copied at the beginning of the physical memory,
it is unlikely to use the last 64kB (I doubt we'll ever support KVM
on a system with something like 4MB of RAM, but patches are very
welcome).
Let's call this VA the trampoline VA.
Now, we map our init page at 3 locations:
- idmap in the boot pgd
- trampoline VA in the boot pgd
- trampoline VA in the runtime pgd
The init scenario is now the following:
- We jump in HYP with four parameters: boot HYP pgd, runtime HYP pgd,
runtime stack, runtime vectors
- Enable the MMU with the boot pgd
- Jump to a target into the trampoline page (remember, this is the same
physical page!)
- Now switch to the runtime pgd (same VA, and still the same physical
page!)
- Invalidate TLBs
- Set stack and vectors
- Profit! (or eret, if you only care about the code).
So I'm going to do my usual commenting routine. Was it an idea to insert
this commit text (which I really liked by the way!) into init.S where
the current comment is a little lacking giving the massive complexity
this is turning into, madness?
Note that we keep the boot mapping permanently (it is not strictly an
idmap anymore) to allow for CPU hotplug in later patches.
Signed-off-by: Marc Zyngier marc.zyng...@arm.com
---
arch/arm/include/asm/kvm_host.h | 18 ---
arch/arm/include/asm/kvm_mmu.h | 21 ++--
arch/arm/kvm/arm.c | 9 ++
arch/arm/kvm/init.S | 29 +++--
arch/arm/kvm/mmu.c | 71
++---
5 files changed, 101 insertions(+), 47 deletions(-)
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index a7a0bb5..3556684 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -190,22 +190,32 @@ int kvm_arm_coproc_set_reg(struct kvm_vcpu *vcpu, const
struct kvm_one_reg *);
int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
int exception_index);
-static inline void __cpu_init_hyp_mode(unsigned long long pgd_ptr,
+static inline void __cpu_init_hyp_mode(unsigned long long boot_pgd_ptr,
+unsigned long long pgd_ptr,
unsigned long hyp_stack_ptr,
unsigned long vector_ptr)
{
unsigned long pgd_low, pgd_high;
- pgd_low = (pgd_ptr ((1ULL 32) - 1));
- pgd_high = (pgd_ptr 32ULL);
+ pgd_low = (boot_pgd_ptr ((1ULL 32) - 1));
+ pgd_high = (boot_pgd_ptr 32ULL);
/*
* Call initialization code, and switch to the full blown
* HYP code. The init code doesn't need to preserve these registers as
- * r1-r3 and r12 are already callee save according to the AAPCS.
+ * r1-r3 and r12 are already callee saved according to the AAPCS.
* Note that we slightly misuse the prototype by casing the pgd_low to
* a void *.
+ *
+ * We don't have enough registers to perform the full init in one go.
+ * Install the boot PGD first, and then install the runtime PGD,
+ * stack pointer and vectors.
*/
+ kvm_call_hyp((void *)pgd_low, pgd_high, 0, 0);
+
+ pgd_low = (pgd_ptr ((1ULL 32) - 1));
+ pgd_high = (pgd_ptr 32ULL);
+
kvm_call_hyp((void *)pgd_low, pgd_high, hyp_stack_ptr, vector_ptr);
}
diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
index 92eb20d..3567a49 100644
--- a/arch/arm/include/asm/kvm_mmu.h
+++ b/arch/arm/include/asm/kvm_mmu.h
@@ -19,17 +19,29 @@
#ifndef __ARM_KVM_MMU_H__
#define __ARM_KVM_MMU_H__
-#include asm/cacheflush.h
-#include asm/pgalloc.h
+#include asm/memory.h
+#include asm/page.h
/*
* We directly use the kernel VA for the HYP, as we can directly share
* the mapping (HTTBR covers TTBR1).
*/
-#define HYP_PAGE_OFFSET_MASK (~0UL)
+#define HYP_PAGE_OFFSET_MASK UL(~0)
#define HYP_PAGE_OFFSET
Re: [PATCH 6/7] ARM: KVM: switch to a dual-step HYP init code
On Wed, Apr 03, 2013 at 11:38:30AM +0100, Marc Zyngier wrote: On 03/04/13 11:07, Will Deacon wrote: On Tue, Apr 02, 2013 at 02:25:14PM +0100, Marc Zyngier wrote: Our HYP init code suffers from two major design issues: - it cannot support CPU hotplug, as we tear down the idmap very early - it cannot perform a TLB invalidation when switching from init to runtime mappings, as pages are manipulated from PL1 exclusively [...] diff --git a/arch/arm/kvm/init.S b/arch/arm/kvm/init.S index 35a463f..b2c6967 100644 --- a/arch/arm/kvm/init.S +++ b/arch/arm/kvm/init.S @@ -21,6 +21,7 @@ #include asm/asm-offsets.h #include asm/kvm_asm.h #include asm/kvm_arm.h +#include asm/kvm_mmu.h / * Hypervisor initialization @@ -47,6 +48,9 @@ __kvm_hyp_init: W(b). __do_hyp_init: + cmp r2, #0 @ We have a SP? + bne phase2 @ Yes, second stage init + @ Set the HTTBR to point to the hypervisor PGD pointer passed mcrrp15, 4, r0, r1, c2 @@ -96,14 +100,35 @@ __do_hyp_init: orr r0, r0, r1 isb mcr p15, 4, r0, c1, c0, 0 @ HSCR - isb - @ Set stack pointer and return to the kernel + eret + +phase2: + @ Set stack pointer mov sp, r2 @ Set HVBAR to point to the HYP vectors mcr p15, 4, r3, c12, c0, 0 @ HVBAR + @ Jump to the trampoline page + ldr r2, =#PAGE_MASK Shifting right by PAGE_SHIFT can avoid the load. Not really. We're masking out the top bits of target and adding them to the trampoline base address, so shifting doesn't help. But, as you suggested offline, BFI can come to the rescue and make that code totally fun and unreadable. How about (untested): ldr r2, =#TRAMPOLINE_VA adr r3, target bfi r2, r3, #0, #PAGE_SHIFT mov pc, r2 I really like it! :) What kind of drugs are you on? Ok, I actually like it too. + adr r3, target + bic r3, r3, r2 + ldr r2, =#TRAMPOLINE_VA + add r3, r3, r2 + mov pc, r3 + + nop insert dead chicken and voodoo chant here ... You know I'll never sleep no more ... Seriously, what kind of drugs are you guys on? -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
