Hi Christoffer, On 06/10/14 21:30, Christoffer Dall wrote: > This patch adds the necessary support for all host kernel PGSIZE and > VA_SPACE configuration options for both EL2 and the Stage-2 page tables. > > However, for 40bit and 42bit PARange systems, the architecture mandates > that VTCR_EL2.SL0 is maximum 1, resulting in fewer levels of stage-2 > pagge tables than levels of host kernel page tables. At the same time, > systems with a PARange > 42bit, we limit the IPA range by always setting > VTCR_EL2.T0SZ to 24. > > To solve the situation with different levels of page tables for Stage-2 > translation than the host kernel page tables, we allocate a dummy PGD > with pointers to our actual inital level Stage-2 page table, in order > for us to reuse the kernel pgtable manipulation primitives. Reproducing > all these in KVM does not look pretty and unnecessarily complicates the > 32-bit side. > > Systems with a PARange < 40bits are not yet supported. > > [ I have reworked this patch from its original form submitted by > Jungseok to take the architecture constraints into consideration. > There were too many changes from the original patch for me to > preserve the authorship. Thanks to Catalin Marinas for his help in > figuring out a good solution to this challenge. I have also fixed > various bugs and missing error code handling from the original > patch. - Christoffer ] > > Cc: Marc Zyngier <marc.zyngier@xxxxxxx> > Cc: Catalin Marinas <catalin.marinas@xxxxxxx> > Signed-off-by: Jungseok Lee <jungseoklee85@xxxxxxxxx> > Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx> On top of Catalin's review, I have the following comments: [...] > diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > index bb06f76..3b3e18f 100644 > --- a/arch/arm/kvm/mmu.c > +++ b/arch/arm/kvm/mmu.c > @@ -42,7 +42,7 @@ static unsigned long hyp_idmap_start; > static unsigned long hyp_idmap_end; > static phys_addr_t hyp_idmap_vector; > > -#define pgd_order get_order(PTRS_PER_PGD * sizeof(pgd_t)) > +#define hyp_pgd_order get_order(PTRS_PER_PGD * sizeof(pgd_t)) > > #define kvm_pmd_huge(_x) (pmd_huge(_x) || pmd_trans_huge(_x)) > > @@ -158,7 +158,7 @@ static void unmap_pmds(struct kvm *kvm, pud_t *pud, > } > } while (pmd++, addr = next, addr != end); > > - if (kvm_pmd_table_empty(start_pmd)) > + if (kvm_pmd_table_empty(start_pmd) && (!kvm || KVM_PREALLOC_LEVEL < 2)) This really feels clunky. Can we fold the additional tests inside kvm_pmd_table_empty(), taking kvm as an additional parameter? > clear_pud_entry(kvm, pud, start_addr); > } > > @@ -182,7 +182,7 @@ static void unmap_puds(struct kvm *kvm, pgd_t *pgd, > } > } while (pud++, addr = next, addr != end); > > - if (kvm_pud_table_empty(start_pud)) > + if (kvm_pud_table_empty(start_pud) && (!kvm || KVM_PREALLOC_LEVEL < 1)) Same here. 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html