Hi Catalin. I hope my comments below make sense. > On Sun, 2011-05-08 at 15:00 +0100, Christoffer Dall wrote: >> --- /dev/null >> +++ b/arch/arm/kvm/arm_interrupts.S > ... >> +__do_hyp_init: >> + ? ? ? @ Copy the Hyp stack pointer >> + ? ? ? mov ? ? sp, r0 >> + ? ? ? mov ? ? lr, r1 >> + >> + ? ? ? @ Set the HTTBR to be the same as the TTBR1 holding the kernel >> + ? ? ? @ level-1 page table >> + ? ? ? mrrc ? ?p15, 1, r0, r1, c2 >> + ? ? ? mcrr ? ?p15, 4, r0, r1, c2 >> + >> + ? ? ? @ Set the HTCR to the same shareability and cacheability settings as the >> + ? ? ? @ non-secure TTBCR and with T0SZ == 0. >> + ? ? ? mrc ? ? p15, 4, r0, c2, c0, 2 >> + ? ? ? ldr ? ? r12, =HTCR_MASK >> + ? ? ? bic ? ? r0, r0, r12 >> + ? ? ? mrc ? ? p15, 0, r1, c2, c0, 2 >> + ? ? ? and ? ? r1, r1, #(HTCR_MASK & ~TTBCR_T0SZ) >> + ? ? ? orr ? ? r0, r0, r1 >> + ? ? ? mcr ? ? p15, 4, r0, c2, c0, 2 > > If you set HTCR.T0SZ to 0, you have a different value than TTBCR.T1SZ. > With a 3:1 memory split for Linux, TTBR1 (with LPAE) no longer uses 3 > levels of page tables but 2 and TTBR1 points to level 2 tables only. > Similarly, with 2:2 split TTBR1 skips the first 2 level 1 page table > entries. But here you copy TTBR1 to HTCR directly but changing T0SZ. > Nice catch. I was thinking that even though the range was smaller, the indexing into the first table would have to be the same, but I didn't correlate that with the starting level. > Anyway, I would not recommend changing the kernel pgd directly as per > your init code but allocate a new one via identity_mapping_add. You than > use this pgd to set HTTBR and T0SZ to 0 (as you need the full 32-bit > address space for the identity mapping). > > Once the Hyp code starts and jumps to the virtual address (that the > kernel is also using), you can change HTTBR to TTBR1 and HTCR.T0SZ to > TTBCR.T1SZ. After that you can remove the original pgd via > identity_mapping_del. I am not sure I completely understand how to do this, but I also don't like to modify the kernel pgd directly. Let me see if I get this right. 1. I allocate a pgd using get_free_pages (because pgd_alloc interacts with an mm_struct). 2. I pass this empty pgd to identity_mapping_add along with the physical address of the hyp-init code. 3. I map the init code at the kernel virtual address in the new pgd somehow (using vmap_page_range?) 4. I pass the physical address of this pgd to the hyp-init code to be placed in HTTBR and I pass the kernel virtual address of the init code (+offset). 5. The init code runs and enables the MMU using the new PGD and thereby identity mapping 6. The init code jumps to the kernel virtual address mapped in (3) (+offset) 7. The init code (now running at kernel virtual address mapped in init pgd and kernel pgd) changes the HTTBR to TTBR1 and HTCR.T0SZ to TTBCR.T1SZ. But, is it not incorrect to set the HTCR.T0SZ to TTBCR.T1SZ, since T1SZ determines the address from upper bound and down, while T0SZ from lower bound and up - thus excluding incorrect address ranges? While modifying the kernel pgd doesn't feel nice, what is really the problem with the approach in terms of bugs, maintainability, etc.? > > But you need to pass an extra argument to your Hyp initialisation code > for HTTBR. Here I'm more in favour of setting up a structure holding > parameters like HTTBR, SP etc. and only pass a pointer to the Hyp code. > Well, is this not more complicated than that (or am I missing something?). That struct would have to be allocated in the same section as the init code or the address of the struct would also need an identity mapping and we would pass the physical address of the struct to the init code. If we're limited to 4 values, would it not be simpler to just keep them in r0-r3? The only advantage I see with the struct is to clarify which parameters are actually passed, but with careful names and commenting I think that can be met by passing the values in registers... -Christoffer
