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
>> mcrr p15, 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
