On Tue, Oct 07, 2014 at 02:40:27PM +0100, Marc Zyngier wrote: > 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. > Sounds reasonable, I'll try to work it into the next version of the patches. -Christoffer -- 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
