| Message ID | 1411674174-30672-2-git-send-email-christoffer.dall@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Sep 26, 2014, at 4:42 AM, Christoffer Dall wrote: Hi, Christoffer > 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. This is a very similar idea suggested by Steve Capper to deal with swapper table when I worked on 4 level solution. I believe that dummy PGD approach can handle all combinations described in ARMv8 architecture. > 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 ] No doubt. This is yours :) I really thank you for picking up the patch and writing this graceful solution. At first glance, it looks good except some unnecessary newlines. I will leave comments after reading the patches carefully. Best Regards Jungseok Lee
Hi Christoffer, On Thu, Sep 25, 2014 at 08:42:53PM +0100, Christoffer Dall wrote: > diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c > index 7796051..048f37f 100644 > --- a/arch/arm/kvm/arm.c > +++ b/arch/arm/kvm/arm.c > @@ -409,7 +409,7 @@ static void update_vttbr(struct kvm *kvm) > kvm_next_vmid++; > > /* update vttbr to be used with the new vmid */ > - pgd_phys = virt_to_phys(kvm->arch.pgd); > + pgd_phys = kvm_get_hwpgd(kvm); > BUG_ON(pgd_phys & ~VTTBR_BADDR_MASK); > vmid = ((u64)(kvm->arch.vmid) << VTTBR_VMID_SHIFT) & VTTBR_VMID_MASK; > kvm->arch.vttbr = pgd_phys | vmid; > diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > index bb06f76..4532f5f 100644 > --- a/arch/arm/kvm/mmu.c > +++ b/arch/arm/kvm/mmu.c > @@ -33,6 +33,7 @@ > > extern char __hyp_idmap_text_start[], __hyp_idmap_text_end[]; > > +static int kvm_prealloc_levels; Do you have plans to determine this dynamically? In this patch, I can only see it being assigned to KVM_PREALLOC_LEVELS. I guess you could simply use the macro. > @@ -521,6 +554,7 @@ int create_hyp_io_mappings(void *from, void *to, phys_addr_t phys_addr) > */ > int kvm_alloc_stage2_pgd(struct kvm *kvm) > { > + int ret; > pgd_t *pgd; > > if (kvm->arch.pgd != NULL) { > @@ -533,9 +567,17 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) > return -ENOMEM; > > memset(pgd, 0, PTRS_PER_S2_PGD * sizeof(pgd_t)); I don't think you need to allocate a full page here (not shown here in the diff context) but it may not be trivial since you use get_page() to keep track of when page tables could be freed (do you do this for the pgd as well?). > + > + ret = kvm_prealloc_hwpgd(kvm, pgd); > + if (ret) > + goto out; > + > kvm_clean_pgd(pgd); > kvm->arch.pgd = pgd; > - > + ret = 0; > +out: > + if (ret) > + free_pages((unsigned long)pgd, S2_PGD_ORDER); > return 0; > } Does this always return 0, even in case of error? Does it look better as: ret = kvm_prealloc_hwpgd(kvm, pgd); if (ret) goto out; kvm_clean_pgd(pgd); kvm->arch.pgd = pgd; return 0; out: free_pages((unsigned long)pgd, S2_PGD_ORDER); return ret; > @@ -572,19 +614,36 @@ void kvm_free_stage2_pgd(struct kvm *kvm) > return; > > unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE); > + kvm_free_hwpgd(kvm); > free_pages((unsigned long)kvm->arch.pgd, S2_PGD_ORDER); > kvm->arch.pgd = NULL; > } > > -static pmd_t *stage2_get_pmd(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, > +static pud_t *stage2_get_pud(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, > phys_addr_t addr) > { > pgd_t *pgd; > pud_t *pud; > - pmd_t *pmd; > > pgd = kvm->arch.pgd + pgd_index(addr); > - pud = pud_offset(pgd, addr); > + if (pgd_none(*pgd)) { > + if (!cache) > + return NULL; > + pud = mmu_memory_cache_alloc(cache); > + pgd_populate(NULL, pgd, pud); > + get_page(virt_to_page(pgd)); > + } > + > + return pud_offset(pgd, addr); > +} This function looks correct, though we would not expect pgd_none() to ever be true as we pre-populate it. You could but a WARN_ON or something until we actually have hardware 4-levels stage 2 (though I don't see a need yet). > +static pmd_t *stage2_get_pmd(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, > + phys_addr_t addr) > +{ > + pud_t *pud; > + pmd_t *pmd; > + > + pud = stage2_get_pud(kvm, cache, addr); > if (pud_none(*pud)) { > if (!cache) > return NULL; > @@ -630,7 +689,7 @@ static int stage2_set_pte(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, > pmd_t *pmd; > pte_t *pte, old_pte; > > - /* Create stage-2 page table mapping - Level 1 */ > + /* Create stage-2 page table mapping - Levels 0 and 1 */ > pmd = stage2_get_pmd(kvm, cache, addr); > if (!pmd) { > /* > @@ -688,7 +747,8 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa, > for (addr = guest_ipa; addr < end; addr += PAGE_SIZE) { > pte_t pte = pfn_pte(pfn, PAGE_S2_DEVICE); > > - ret = mmu_topup_memory_cache(&cache, 2, 2); > + ret = mmu_topup_memory_cache(&cache, KVM_MMU_CACHE_MIN_PAGES, > + KVM_MMU_CACHE_MIN_PAGES); BTW, why do you need to preallocate the pages in KVM's own cache? Would GFP_ATOMIC not work within the kvm->mmu_lock region? > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index fd4e81a..0f3e0a9 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -200,7 +200,6 @@ config ARM64_VA_BITS_42 > > config ARM64_VA_BITS_48 > bool "48-bit" > - depends on BROKEN I'm ok with removing BROKEN here but I think at the same time we need to disable the SMMU driver which has similar issues with (not) supporting 4-levels page tables for stage 2 (stage 1 should be fine). > diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h > index a030d16..313c6f9 100644 > --- a/arch/arm64/include/asm/kvm_mmu.h > +++ b/arch/arm64/include/asm/kvm_mmu.h [...] > @@ -118,13 +128,123 @@ static inline bool kvm_page_empty(void *ptr) > } > > #define kvm_pte_table_empty(ptep) kvm_page_empty(ptep) > -#ifndef CONFIG_ARM64_64K_PAGES > -#define kvm_pmd_table_empty(pmdp) kvm_page_empty(pmdp) > -#else > +#if CONFIG_ARM64_PGTABLE_LEVELS == 2 > #define kvm_pmd_table_empty(pmdp) (0) > -#endif > #define kvm_pud_table_empty(pudp) (0) > +#elif CONFIG_ARM64_PGTABLE_LEVELS == 3 > +#define kvm_pmd_table_empty(pmdp) kvm_page_empty(pmdp) > +#define kvm_pud_table_empty(pudp) (0) > +#elif CONFIG_ARM64_PGTABLE_LEVELS == 4 > +#define kvm_pmd_table_empty(pmdp) kvm_page_empty(pmdp) > +#define kvm_pud_table_empty(pudp) kvm_page_empty(pudp) > +#endif Not sure whether it's clearer as (up to you): #ifdef __PAGETABLE_PMD_FOLDED #define kvm_pmd_table_empty(pmdp) (0) #else #define kvm_pmd_table_empty(pmdp) kvm_page_empty(pmdp) #endif #ifdef __PAGETABLE_PUD_FOLDED #define kvm_pud_table_empty(pudp) (0) #else #define kvm_pud_table_empty(pudp) kvm_page_empty(pudp) #endif > + > +/** > + * kvm_prealloc_hwpgd - allocate inital table for VTTBR > + * @kvm: The KVM struct pointer for the VM. > + * @pgd: The kernel pseudo pgd "hwpgd" doesn't sound like the right name since it's actually a fake pgd that it is allocating. Maybe "swpgd"? > + * > + * When the kernel uses more levels of page tables than the guest, we allocate > + * a fake PGD and pre-populate it to point to the next-level page table, which > + * will be the real initial page table pointed to by the VTTBR. > + * > + * When KVM_PREALLOC_PMD is defined, we allocate a single page for the PMD and > + * the kernel will use folded pud. When KVM_PREALLOC_PUD is defined, we > + * allocate 2 consecutive PUD pages. > + */ > +#if defined(CONFIG_ARM64_64K_PAGES) && CONFIG_ARM64_PGTABLE_LEVELS == 3 > +#define KVM_PREALLOC_LEVELS 2 > +#define PTRS_PER_S2_PGD 1 > +#define S2_PGD_ORDER get_order(PTRS_PER_S2_PGD * sizeof(pgd_t)) > + > + > +static inline int kvm_prealloc_hwpgd(struct kvm *kvm, pgd_t *pgd) > +{ > + pud_t *pud; > + pmd_t *pmd; > + > + pud = pud_offset(pgd, 0); > + pmd = (pmd_t *)__get_free_pages(GFP_KERNEL, 0); > + > + if (!pmd) > + return -ENOMEM; > + memset(pmd, 0, PAGE_SIZE); You could use __GFP_ZERO. > + pud_populate(NULL, pud, pmd); > + get_page(virt_to_page(pud)); > + > + return 0; > +} > > +static inline void kvm_free_hwpgd(struct kvm *kvm) > +{ > + pgd_t *pgd = kvm->arch.pgd; > + pud_t *pud = pud_offset(pgd, 0); > + pmd_t *pmd = pmd_offset(pud, 0); > + free_pages((unsigned long)pmd, 0); > + put_page(virt_to_page(pud)); > +} > + > +static inline phys_addr_t kvm_get_hwpgd(struct kvm *kvm) > +{ > + pgd_t *pgd = kvm->arch.pgd; > + pud_t *pud = pud_offset(pgd, 0); > + pmd_t *pmd = pmd_offset(pud, 0); > + return virt_to_phys(pmd); > + > +} > +#elif defined(CONFIG_ARM64_4K_PAGES) && CONFIG_ARM64_PGTABLE_LEVELS == 4 > +#define KVM_PREALLOC_LEVELS 1 > +#define PTRS_PER_S2_PGD 2 > +#define S2_PGD_ORDER get_order(PTRS_PER_S2_PGD * sizeof(pgd_t)) > + > +static inline int kvm_prealloc_hwpgd(struct kvm *kvm, pgd_t *pgd) > +{ > + pud_t *pud; > + > + pud = (pud_t *)__get_free_pages(GFP_KERNEL, 1); > + if (!pud) > + return -ENOMEM; > + memset(pud, 0, 2 * PAGE_SIZE); > + pgd_populate(NULL, pgd, pud); > + pgd_populate(NULL, pgd + 1, pud + PTRS_PER_PUD); > + get_page(virt_to_page(pgd)); > + get_page(virt_to_page(pgd)); > + > + return 0; > +} > + > +static inline void kvm_free_hwpgd(struct kvm *kvm) > +{ > + pgd_t *pgd = kvm->arch.pgd; > + pud_t *pud = pud_offset(pgd, 0); > + free_pages((unsigned long)pud, 1); > + put_page(virt_to_page(pgd)); > + put_page(virt_to_page(pgd)); > +} > + > +static inline phys_addr_t kvm_get_hwpgd(struct kvm *kvm) > +{ > + pgd_t *pgd = kvm->arch.pgd; > + pud_t *pud = pud_offset(pgd, 0); > + return virt_to_phys(pud); > +} > +#else > +#define KVM_PREALLOC_LEVELS 0 > +#define PTRS_PER_S2_PGD (1 << (KVM_PHYS_SHIFT - PGDIR_SHIFT)) > +#define S2_PGD_ORDER get_order(PTRS_PER_S2_PGD * sizeof(pgd_t)) > + > +static inline int kvm_prealloc_hwpgd(struct kvm *kvm, pgd_t *pgd) > +{ > + return 0; > +} > + > +static inline void kvm_free_hwpgd(struct kvm *kvm) { } > + > +static inline phys_addr_t kvm_get_hwpgd(struct kvm *kvm) > +{ > + return virt_to_phys(kvm->arch.pgd); > +} > +#endif I think all the above could be squashed into a single set of function implementations with the right macros defined before. For the KVM levels, you could use something like (we exchanged emails in private on this topic and how I got to these macros but they need better checking): #define KVM_PGTABLE_LEVELS ((KVM_PHYS_SHIFT - PAGE_SHIFT - 5) / (PAGE_SHIFT - 3) + 1) #define KVM_PREALLOC_LEVELS (CONFIG_ARM64_PGTABLE_LEVELS - KVM_PGTABLE_LEVELS) #if PGDIR_SHIFT > KVM_PHYS_SHIFT #define PTRS_PER_S2_PGD (1) #else #define PTRS_PER_S2_PGD (1 << (KVM_PHYS_SHIFT - PGDIR_SHIFT)) #endif -- Catalin
Hi Catalin, On Tue, Sep 30, 2014 at 01:39:47PM +0100, Catalin Marinas wrote: > Hi Christoffer, > > On Thu, Sep 25, 2014 at 08:42:53PM +0100, Christoffer Dall wrote: > > diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c > > index 7796051..048f37f 100644 > > --- a/arch/arm/kvm/arm.c > > +++ b/arch/arm/kvm/arm.c > > @@ -409,7 +409,7 @@ static void update_vttbr(struct kvm *kvm) > > kvm_next_vmid++; > > > > /* update vttbr to be used with the new vmid */ > > - pgd_phys = virt_to_phys(kvm->arch.pgd); > > + pgd_phys = kvm_get_hwpgd(kvm); > > BUG_ON(pgd_phys & ~VTTBR_BADDR_MASK); > > vmid = ((u64)(kvm->arch.vmid) << VTTBR_VMID_SHIFT) & VTTBR_VMID_MASK; > > kvm->arch.vttbr = pgd_phys | vmid; > > diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > > index bb06f76..4532f5f 100644 > > --- a/arch/arm/kvm/mmu.c > > +++ b/arch/arm/kvm/mmu.c > > @@ -33,6 +33,7 @@ > > > > extern char __hyp_idmap_text_start[], __hyp_idmap_text_end[]; > > > > +static int kvm_prealloc_levels; > > Do you have plans to determine this dynamically? In this patch, I can > only see it being assigned to KVM_PREALLOC_LEVELS. I guess you could > simply use the macro. > Yes, I could. Not really sure why I did this. > > @@ -521,6 +554,7 @@ int create_hyp_io_mappings(void *from, void *to, phys_addr_t phys_addr) > > */ > > int kvm_alloc_stage2_pgd(struct kvm *kvm) > > { > > + int ret; > > pgd_t *pgd; > > > > if (kvm->arch.pgd != NULL) { > > @@ -533,9 +567,17 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) > > return -ENOMEM; > > > > memset(pgd, 0, PTRS_PER_S2_PGD * sizeof(pgd_t)); > > I don't think you need to allocate a full page here (not shown here in > the diff context) but it may not be trivial since you use get_page() to > keep track of when page tables could be freed (do you do this for the > pgd as well?). > The code becomes a bit weird, because we'd have to check if this is really just the fake pseudo-pgds and we can just use kmalloc(), but if it's not, we have to use __get_free_pages() to adhere to the architecture's alignment requirements for the VTTBR physical address. I'll try to fix this in the next version and we can see how it looks like. As far as the get_page() counting goes, since we always fully propogate this first level pseudo table, we never mess with the page counts except for in kvm_prealloc_hwpgd() and kvm_free_hwpgd() so we can just take all the get/put_page stuff out of those and we should be in the clear. > > + > > + ret = kvm_prealloc_hwpgd(kvm, pgd); > > + if (ret) > > + goto out; > > + > > kvm_clean_pgd(pgd); > > kvm->arch.pgd = pgd; > > - > > + ret = 0; > > +out: > > + if (ret) > > + free_pages((unsigned long)pgd, S2_PGD_ORDER); > > return 0; > > } > > Does this always return 0, even in case of error? Me being braindead, sorry. > Does it look better > as: > > ret = kvm_prealloc_hwpgd(kvm, pgd); > if (ret) > goto out; > > kvm_clean_pgd(pgd); > kvm->arch.pgd = pgd; > > return 0; > > out: > free_pages((unsigned long)pgd, S2_PGD_ORDER); > return ret; > probably looks better, I'd rename out to out_err, but yes. > > @@ -572,19 +614,36 @@ void kvm_free_stage2_pgd(struct kvm *kvm) > > return; > > > > unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE); > > + kvm_free_hwpgd(kvm); > > free_pages((unsigned long)kvm->arch.pgd, S2_PGD_ORDER); > > kvm->arch.pgd = NULL; > > } > > > > -static pmd_t *stage2_get_pmd(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, > > +static pud_t *stage2_get_pud(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, > > phys_addr_t addr) > > { > > pgd_t *pgd; > > pud_t *pud; > > - pmd_t *pmd; > > > > pgd = kvm->arch.pgd + pgd_index(addr); > > - pud = pud_offset(pgd, addr); > > + if (pgd_none(*pgd)) { > > + if (!cache) > > + return NULL; > > + pud = mmu_memory_cache_alloc(cache); > > + pgd_populate(NULL, pgd, pud); > > + get_page(virt_to_page(pgd)); > > + } > > + > > + return pud_offset(pgd, addr); > > +} > > This function looks correct, though we would not expect pgd_none() to > ever be true as we pre-populate it. > You could but a WARN_ON or something > until we actually have hardware 4-levels stage 2 (though I don't see a > need yet). > pgd_none will never be true, because on both aarch64 and arm we fold the pud into the pgd (and include pgtable-nopud.h) if we have less than 4 levels, and if we do have 4 levels, then we have preallocated and prepopulated the pgd. If I got this right, I agree, and we should add the WARN_ON or VM_BUG_ON or something. > > +static pmd_t *stage2_get_pmd(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, > > + phys_addr_t addr) > > +{ > > + pud_t *pud; > > + pmd_t *pmd; > > + > > + pud = stage2_get_pud(kvm, cache, addr); > > if (pud_none(*pud)) { > > if (!cache) > > return NULL; > > @@ -630,7 +689,7 @@ static int stage2_set_pte(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, > > pmd_t *pmd; > > pte_t *pte, old_pte; > > > > - /* Create stage-2 page table mapping - Level 1 */ > > + /* Create stage-2 page table mapping - Levels 0 and 1 */ > > pmd = stage2_get_pmd(kvm, cache, addr); > > if (!pmd) { > > /* > > @@ -688,7 +747,8 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa, > > for (addr = guest_ipa; addr < end; addr += PAGE_SIZE) { > > pte_t pte = pfn_pte(pfn, PAGE_S2_DEVICE); > > > > - ret = mmu_topup_memory_cache(&cache, 2, 2); > > + ret = mmu_topup_memory_cache(&cache, KVM_MMU_CACHE_MIN_PAGES, > > + KVM_MMU_CACHE_MIN_PAGES); > > BTW, why do you need to preallocate the pages in KVM's own cache? Would > GFP_ATOMIC not work within the kvm->mmu_lock region? > It would, but it doesn't really seem like the sort of thing you would want to tap into the GFP_ATOMIC pool for, would it? A bunch of VMs could seriously trash your host by asking for a lot of address space. I think that was the rationale for doing things this way on x86 and it made sense to me... > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > index fd4e81a..0f3e0a9 100644 > > --- a/arch/arm64/Kconfig > > +++ b/arch/arm64/Kconfig > > @@ -200,7 +200,6 @@ config ARM64_VA_BITS_42 > > > > config ARM64_VA_BITS_48 > > bool "48-bit" > > - depends on BROKEN > > I'm ok with removing BROKEN here but I think at the same time we need to > disable the SMMU driver which has similar issues with (not) supporting > 4-levels page tables for stage 2 (stage 1 should be fine). Should that be in separate patches, or is it fine to just include it here? > > > diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h > > index a030d16..313c6f9 100644 > > --- a/arch/arm64/include/asm/kvm_mmu.h > > +++ b/arch/arm64/include/asm/kvm_mmu.h > [...] > > @@ -118,13 +128,123 @@ static inline bool kvm_page_empty(void *ptr) > > } > > > > #define kvm_pte_table_empty(ptep) kvm_page_empty(ptep) > > -#ifndef CONFIG_ARM64_64K_PAGES > > -#define kvm_pmd_table_empty(pmdp) kvm_page_empty(pmdp) > > -#else > > +#if CONFIG_ARM64_PGTABLE_LEVELS == 2 > > #define kvm_pmd_table_empty(pmdp) (0) > > -#endif > > #define kvm_pud_table_empty(pudp) (0) > > +#elif CONFIG_ARM64_PGTABLE_LEVELS == 3 > > +#define kvm_pmd_table_empty(pmdp) kvm_page_empty(pmdp) > > +#define kvm_pud_table_empty(pudp) (0) > > +#elif CONFIG_ARM64_PGTABLE_LEVELS == 4 > > +#define kvm_pmd_table_empty(pmdp) kvm_page_empty(pmdp) > > +#define kvm_pud_table_empty(pudp) kvm_page_empty(pudp) > > +#endif > > Not sure whether it's clearer as (up to you): > > #ifdef __PAGETABLE_PMD_FOLDED > #define kvm_pmd_table_empty(pmdp) (0) > #else > #define kvm_pmd_table_empty(pmdp) kvm_page_empty(pmdp) > #endif > > #ifdef __PAGETABLE_PUD_FOLDED > #define kvm_pud_table_empty(pudp) (0) > #else > #define kvm_pud_table_empty(pudp) kvm_page_empty(pudp) > #endif > > I think it probably is more clear. > > + > > +/** > > + * kvm_prealloc_hwpgd - allocate inital table for VTTBR > > + * @kvm: The KVM struct pointer for the VM. > > + * @pgd: The kernel pseudo pgd > > "hwpgd" doesn't sound like the right name since it's actually a fake pgd > that it is allocating. Maybe "swpgd"? > The fake pgd was already allocated in kvm_alloc_stage2_pgd(), now we're allocating the first-level page table that is going to be walked by the MMU for Stage-2 translations (the address of which will go into the VTTBR), so that's the reasoning for calling it the hwpgd. Does that make sense? > > + * > > + * When the kernel uses more levels of page tables than the guest, we allocate > > + * a fake PGD and pre-populate it to point to the next-level page table, which > > + * will be the real initial page table pointed to by the VTTBR. > > + * > > + * When KVM_PREALLOC_PMD is defined, we allocate a single page for the PMD and > > + * the kernel will use folded pud. When KVM_PREALLOC_PUD is defined, we > > + * allocate 2 consecutive PUD pages. > > + */ > > +#if defined(CONFIG_ARM64_64K_PAGES) && CONFIG_ARM64_PGTABLE_LEVELS == 3 > > +#define KVM_PREALLOC_LEVELS 2 > > +#define PTRS_PER_S2_PGD 1 > > +#define S2_PGD_ORDER get_order(PTRS_PER_S2_PGD * sizeof(pgd_t)) > > + > > + > > +static inline int kvm_prealloc_hwpgd(struct kvm *kvm, pgd_t *pgd) > > +{ > > + pud_t *pud; > > + pmd_t *pmd; > > + > > + pud = pud_offset(pgd, 0); > > + pmd = (pmd_t *)__get_free_pages(GFP_KERNEL, 0); > > + > > + if (!pmd) > > + return -ENOMEM; > > + memset(pmd, 0, PAGE_SIZE); > > You could use __GFP_ZERO. > yes, Ard also commented on that, we should do that. > > + pud_populate(NULL, pud, pmd); > > + get_page(virt_to_page(pud)); > > + > > + return 0; > > +} > > > > +static inline void kvm_free_hwpgd(struct kvm *kvm) > > +{ > > + pgd_t *pgd = kvm->arch.pgd; > > + pud_t *pud = pud_offset(pgd, 0); > > + pmd_t *pmd = pmd_offset(pud, 0); > > + free_pages((unsigned long)pmd, 0); > > + put_page(virt_to_page(pud)); > > +} > > + > > +static inline phys_addr_t kvm_get_hwpgd(struct kvm *kvm) > > +{ > > + pgd_t *pgd = kvm->arch.pgd; > > + pud_t *pud = pud_offset(pgd, 0); > > + pmd_t *pmd = pmd_offset(pud, 0); > > + return virt_to_phys(pmd); > > + > > +} > > +#elif defined(CONFIG_ARM64_4K_PAGES) && CONFIG_ARM64_PGTABLE_LEVELS == 4 > > +#define KVM_PREALLOC_LEVELS 1 > > +#define PTRS_PER_S2_PGD 2 > > +#define S2_PGD_ORDER get_order(PTRS_PER_S2_PGD * sizeof(pgd_t)) > > + > > +static inline int kvm_prealloc_hwpgd(struct kvm *kvm, pgd_t *pgd) > > +{ > > + pud_t *pud; > > + > > + pud = (pud_t *)__get_free_pages(GFP_KERNEL, 1); > > + if (!pud) > > + return -ENOMEM; > > + memset(pud, 0, 2 * PAGE_SIZE); > > + pgd_populate(NULL, pgd, pud); > > + pgd_populate(NULL, pgd + 1, pud + PTRS_PER_PUD); > > + get_page(virt_to_page(pgd)); > > + get_page(virt_to_page(pgd)); > > + > > + return 0; > > +} > > + > > +static inline void kvm_free_hwpgd(struct kvm *kvm) > > +{ > > + pgd_t *pgd = kvm->arch.pgd; > > + pud_t *pud = pud_offset(pgd, 0); > > + free_pages((unsigned long)pud, 1); > > + put_page(virt_to_page(pgd)); > > + put_page(virt_to_page(pgd)); > > +} > > + > > +static inline phys_addr_t kvm_get_hwpgd(struct kvm *kvm) > > +{ > > + pgd_t *pgd = kvm->arch.pgd; > > + pud_t *pud = pud_offset(pgd, 0); > > + return virt_to_phys(pud); > > +} > > +#else > > +#define KVM_PREALLOC_LEVELS 0 > > +#define PTRS_PER_S2_PGD (1 << (KVM_PHYS_SHIFT - PGDIR_SHIFT)) > > +#define S2_PGD_ORDER get_order(PTRS_PER_S2_PGD * sizeof(pgd_t)) > > + > > +static inline int kvm_prealloc_hwpgd(struct kvm *kvm, pgd_t *pgd) > > +{ > > + return 0; > > +} > > + > > +static inline void kvm_free_hwpgd(struct kvm *kvm) { } > > + > > +static inline phys_addr_t kvm_get_hwpgd(struct kvm *kvm) > > +{ > > + return virt_to_phys(kvm->arch.pgd); > > +} > > +#endif > > I think all the above could be squashed into a single set of function > implementations with the right macros defined before. > > For the KVM levels, you could use something like (we exchanged emails in > private on this topic and how I got to these macros but they need better > checking): > > #define KVM_PGTABLE_LEVELS ((KVM_PHYS_SHIFT - PAGE_SHIFT - 5) / (PAGE_SHIFT - 3) + 1) > #define KVM_PREALLOC_LEVELS (CONFIG_ARM64_PGTABLE_LEVELS - KVM_PGTABLE_LEVELS) > #if PGDIR_SHIFT > KVM_PHYS_SHIFT > #define PTRS_PER_S2_PGD (1) > #else > #define PTRS_PER_S2_PGD (1 << (KVM_PHYS_SHIFT - PGDIR_SHIFT)) > #endif > I'll revisit this, but the amount of time I needed to spend making sure I understand this sort of indicated to me that it was simpler to just spell it out. What do you think? Do you strongly prefer the simple macro definitions? Thanks for the review! -Christoffer
On Mon, Oct 06, 2014 at 02:41:18PM +0100, Christoffer Dall wrote: > On Tue, Sep 30, 2014 at 01:39:47PM +0100, Catalin Marinas wrote: > > On Thu, Sep 25, 2014 at 08:42:53PM +0100, Christoffer Dall wrote: > > > @@ -572,19 +614,36 @@ void kvm_free_stage2_pgd(struct kvm *kvm) > > > return; > > > > > > unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE); > > > + kvm_free_hwpgd(kvm); > > > free_pages((unsigned long)kvm->arch.pgd, S2_PGD_ORDER); > > > kvm->arch.pgd = NULL; > > > } > > > > > > -static pmd_t *stage2_get_pmd(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, > > > +static pud_t *stage2_get_pud(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, > > > phys_addr_t addr) > > > { > > > pgd_t *pgd; > > > pud_t *pud; > > > - pmd_t *pmd; > > > > > > pgd = kvm->arch.pgd + pgd_index(addr); > > > - pud = pud_offset(pgd, addr); > > > + if (pgd_none(*pgd)) { > > > + if (!cache) > > > + return NULL; > > > + pud = mmu_memory_cache_alloc(cache); > > > + pgd_populate(NULL, pgd, pud); > > > + get_page(virt_to_page(pgd)); > > > + } > > > + > > > + return pud_offset(pgd, addr); > > > +} > > > > This function looks correct, though we would not expect pgd_none() to > > ever be true as we pre-populate it. > > You could but a WARN_ON or something > > until we actually have hardware 4-levels stage 2 (though I don't see a > > need yet). > > pgd_none will never be true, because on both aarch64 and arm we fold the > pud into the pgd (and include pgtable-nopud.h) if we have less than 4 > levels, and if we do have 4 levels, then we have preallocated and > prepopulated the pgd. If I got this right, I agree, and we should add > the WARN_ON or VM_BUG_ON or something. Yes. > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > > index fd4e81a..0f3e0a9 100644 > > > --- a/arch/arm64/Kconfig > > > +++ b/arch/arm64/Kconfig > > > @@ -200,7 +200,6 @@ config ARM64_VA_BITS_42 > > > > > > config ARM64_VA_BITS_48 > > > bool "48-bit" > > > - depends on BROKEN > > > > I'm ok with removing BROKEN here but I think at the same time we need to > > disable the SMMU driver which has similar issues with (not) supporting > > 4-levels page tables for stage 2 (stage 1 should be fine). > > Should that be in separate patches, or is it fine to just include it > here? I think a separate patch replacing BROKEN with !ARM_SMMU. But we'll need to test the kernel a bit more to make sure there isn't anything else that's broken before merging it. > > > + > > > +/** > > > + * kvm_prealloc_hwpgd - allocate inital table for VTTBR > > > + * @kvm: The KVM struct pointer for the VM. > > > + * @pgd: The kernel pseudo pgd > > > > "hwpgd" doesn't sound like the right name since it's actually a fake pgd > > that it is allocating. Maybe "swpgd"? > > The fake pgd was already allocated in kvm_alloc_stage2_pgd(), now we're > allocating the first-level page table that is going to be walked by the > MMU for Stage-2 translations (the address of which will go into the > VTTBR), so that's the reasoning for calling it the hwpgd. OK, it makes sense now. > > > +#define KVM_PREALLOC_LEVELS 0 > > > +#define PTRS_PER_S2_PGD (1 << (KVM_PHYS_SHIFT - PGDIR_SHIFT)) > > > +#define S2_PGD_ORDER get_order(PTRS_PER_S2_PGD * sizeof(pgd_t)) > > > + > > > +static inline int kvm_prealloc_hwpgd(struct kvm *kvm, pgd_t *pgd) > > > +{ > > > + return 0; > > > +} > > > + > > > +static inline void kvm_free_hwpgd(struct kvm *kvm) { } > > > + > > > +static inline phys_addr_t kvm_get_hwpgd(struct kvm *kvm) > > > +{ > > > + return virt_to_phys(kvm->arch.pgd); > > > +} > > > +#endif > > > > I think all the above could be squashed into a single set of function > > implementations with the right macros defined before. > > > > For the KVM levels, you could use something like (we exchanged emails in > > private on this topic and how I got to these macros but they need better > > checking): > > > > #define KVM_PGTABLE_LEVELS ((KVM_PHYS_SHIFT - PAGE_SHIFT - 5) / (PAGE_SHIFT - 3) + 1) > > #define KVM_PREALLOC_LEVELS (CONFIG_ARM64_PGTABLE_LEVELS - KVM_PGTABLE_LEVELS) > > #if PGDIR_SHIFT > KVM_PHYS_SHIFT > > #define PTRS_PER_S2_PGD (1) > > #else > > #define PTRS_PER_S2_PGD (1 << (KVM_PHYS_SHIFT - PGDIR_SHIFT)) > > #endif > > I'll revisit this, but the amount of time I needed to spend making sure > I understand this sort of indicated to me that it was simpler to just > spell it out. What do you think? Do you strongly prefer the simple > macro definitions? I don't have a strong preference but I was looking to reduce the #ifdef complexity when we make the next change to the host page table (16K pages is coming as well with 3 or 4 levels). -- Catalin
On Mon, Oct 06, 2014 at 04:54:46PM +0100, Catalin Marinas wrote: > On Mon, Oct 06, 2014 at 02:41:18PM +0100, Christoffer Dall wrote: > > On Tue, Sep 30, 2014 at 01:39:47PM +0100, Catalin Marinas wrote: > > > On Thu, Sep 25, 2014 at 08:42:53PM +0100, Christoffer Dall wrote: > > > > @@ -572,19 +614,36 @@ void kvm_free_stage2_pgd(struct kvm *kvm) > > > > return; > > > > > > > > unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE); > > > > + kvm_free_hwpgd(kvm); > > > > free_pages((unsigned long)kvm->arch.pgd, S2_PGD_ORDER); > > > > kvm->arch.pgd = NULL; > > > > } > > > > > > > > -static pmd_t *stage2_get_pmd(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, > > > > +static pud_t *stage2_get_pud(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, > > > > phys_addr_t addr) > > > > { > > > > pgd_t *pgd; > > > > pud_t *pud; > > > > - pmd_t *pmd; > > > > > > > > pgd = kvm->arch.pgd + pgd_index(addr); > > > > - pud = pud_offset(pgd, addr); > > > > + if (pgd_none(*pgd)) { > > > > + if (!cache) > > > > + return NULL; > > > > + pud = mmu_memory_cache_alloc(cache); > > > > + pgd_populate(NULL, pgd, pud); > > > > + get_page(virt_to_page(pgd)); > > > > + } > > > > + > > > > + return pud_offset(pgd, addr); > > > > +} > > > > > > This function looks correct, though we would not expect pgd_none() to > > > ever be true as we pre-populate it. > > > You could but a WARN_ON or something > > > until we actually have hardware 4-levels stage 2 (though I don't see a > > > need yet). > > > > pgd_none will never be true, because on both aarch64 and arm we fold the > > pud into the pgd (and include pgtable-nopud.h) if we have less than 4 > > levels, and if we do have 4 levels, then we have preallocated and > > prepopulated the pgd. If I got this right, I agree, and we should add > > the WARN_ON or VM_BUG_ON or something. > > Yes. > > > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > > > index fd4e81a..0f3e0a9 100644 > > > > --- a/arch/arm64/Kconfig > > > > +++ b/arch/arm64/Kconfig > > > > @@ -200,7 +200,6 @@ config ARM64_VA_BITS_42 > > > > > > > > config ARM64_VA_BITS_48 > > > > bool "48-bit" > > > > - depends on BROKEN > > > > > > I'm ok with removing BROKEN here but I think at the same time we need to > > > disable the SMMU driver which has similar issues with (not) supporting > > > 4-levels page tables for stage 2 (stage 1 should be fine). > > > > Should that be in separate patches, or is it fine to just include it > > here? > > I think a separate patch replacing BROKEN with !ARM_SMMU. But we'll > need to test the kernel a bit more to make sure there isn't anything > else that's broken before merging it. > > > > > + > > > > +/** > > > > + * kvm_prealloc_hwpgd - allocate inital table for VTTBR > > > > + * @kvm: The KVM struct pointer for the VM. > > > > + * @pgd: The kernel pseudo pgd > > > > > > "hwpgd" doesn't sound like the right name since it's actually a fake pgd > > > that it is allocating. Maybe "swpgd"? > > > > The fake pgd was already allocated in kvm_alloc_stage2_pgd(), now we're > > allocating the first-level page table that is going to be walked by the > > MMU for Stage-2 translations (the address of which will go into the > > VTTBR), so that's the reasoning for calling it the hwpgd. > > OK, it makes sense now. > > > > > +#define KVM_PREALLOC_LEVELS 0 > > > > +#define PTRS_PER_S2_PGD (1 << (KVM_PHYS_SHIFT - PGDIR_SHIFT)) > > > > +#define S2_PGD_ORDER get_order(PTRS_PER_S2_PGD * sizeof(pgd_t)) > > > > + > > > > +static inline int kvm_prealloc_hwpgd(struct kvm *kvm, pgd_t *pgd) > > > > +{ > > > > + return 0; > > > > +} > > > > + > > > > +static inline void kvm_free_hwpgd(struct kvm *kvm) { } > > > > + > > > > +static inline phys_addr_t kvm_get_hwpgd(struct kvm *kvm) > > > > +{ > > > > + return virt_to_phys(kvm->arch.pgd); > > > > +} > > > > +#endif > > > > > > I think all the above could be squashed into a single set of function > > > implementations with the right macros defined before. > > > > > > For the KVM levels, you could use something like (we exchanged emails in > > > private on this topic and how I got to these macros but they need better > > > checking): > > > > > > #define KVM_PGTABLE_LEVELS ((KVM_PHYS_SHIFT - PAGE_SHIFT - 5) / (PAGE_SHIFT - 3) + 1) > > > #define KVM_PREALLOC_LEVELS (CONFIG_ARM64_PGTABLE_LEVELS - KVM_PGTABLE_LEVELS) > > > #if PGDIR_SHIFT > KVM_PHYS_SHIFT > > > #define PTRS_PER_S2_PGD (1) > > > #else > > > #define PTRS_PER_S2_PGD (1 << (KVM_PHYS_SHIFT - PGDIR_SHIFT)) > > > #endif > > > > I'll revisit this, but the amount of time I needed to spend making sure > > I understand this sort of indicated to me that it was simpler to just > > spell it out. What do you think? Do you strongly prefer the simple > > macro definitions? > > I don't have a strong preference but I was looking to reduce the #ifdef > complexity when we make the next change to the host page table (16K > pages is coming as well with 3 or 4 levels). > I looked a bit at this tonight, and I still have some reservations: The KVM_PREALLOC_LEVELS macro doesn't seem to yield the right result, or at least not the result we have now, because it will give us 1 in both the 64K and 4K situation. I've renamed the symbol to KVM_PREALLOC_LEVEL in the new version to be slightly more clear. In the case where PGDIR_SHIFT > KVM_PHYS_SHIFT, we should have PTRS_PER_S2_PGD == 2, and this now has a real consequence when we're no longer allocating on a page-basis for the (potentially) fake PGD. Finally, I'm a bit worried about introducing a bunch of macro logic which is really hard to intuitively understand. Maybe I'm over emphasizing this issue, but I fint the KVM_PGTABLE_LEVELS definition pretty hard to explain. Anyway, I'm open to revisit this in v2. Thanks, -Christoffer
diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h index 3f688b4..0ffd2a8 100644 --- a/arch/arm/include/asm/kvm_mmu.h +++ b/arch/arm/include/asm/kvm_mmu.h @@ -37,6 +37,11 @@ */ #define TRAMPOLINE_VA UL(CONFIG_VECTORS_BASE) +/* + * KVM_MMU_CACHE_MIN_PAGES is the number of stage2 page table translation levels. + */ +#define KVM_MMU_CACHE_MIN_PAGES 2 + #ifndef __ASSEMBLY__ #include <asm/cacheflush.h> @@ -83,6 +88,11 @@ static inline void kvm_clean_pgd(pgd_t *pgd) clean_dcache_area(pgd, PTRS_PER_S2_PGD * sizeof(pgd_t)); } +static inline void kvm_clean_pmd(pmd_t *pmd) +{ + clean_dcache_area(pmd, PTRS_PER_PMD * sizeof(pmd_t)); +} + static inline void kvm_clean_pmd_entry(pmd_t *pmd) { clean_pmd_entry(pmd); @@ -127,6 +137,19 @@ static inline bool kvm_page_empty(void *ptr) #define kvm_pmd_table_empty(pmdp) kvm_page_empty(pmdp) #define kvm_pud_table_empty(pudp) (0) +#define KVM_PREALLOC_LEVELS 0 + +static inline int kvm_prealloc_hwpgd(struct kvm *kvm, pgd_t *pgd) +{ + return 0; +} + +static inline void kvm_free_hwpgd(struct kvm *kvm) { } + +static inline phys_addr_t kvm_get_hwpgd(struct kvm *kvm) +{ + return virt_to_phys(kvm->arch.pgd); +} struct kvm; diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c index 7796051..048f37f 100644 --- a/arch/arm/kvm/arm.c +++ b/arch/arm/kvm/arm.c @@ -409,7 +409,7 @@ static void update_vttbr(struct kvm *kvm) kvm_next_vmid++; /* update vttbr to be used with the new vmid */ - pgd_phys = virt_to_phys(kvm->arch.pgd); + pgd_phys = kvm_get_hwpgd(kvm); BUG_ON(pgd_phys & ~VTTBR_BADDR_MASK); vmid = ((u64)(kvm->arch.vmid) << VTTBR_VMID_SHIFT) & VTTBR_VMID_MASK; kvm->arch.vttbr = pgd_phys | vmid; diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c index bb06f76..4532f5f 100644 --- a/arch/arm/kvm/mmu.c +++ b/arch/arm/kvm/mmu.c @@ -33,6 +33,7 @@ extern char __hyp_idmap_text_start[], __hyp_idmap_text_end[]; +static int kvm_prealloc_levels; static pgd_t *boot_hyp_pgd; static pgd_t *hyp_pgd; static DEFINE_MUTEX(kvm_hyp_pgd_mutex); @@ -42,7 +43,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 +159,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_levels < 2)) clear_pud_entry(kvm, pud, start_addr); } @@ -182,7 +183,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_levels < 1)) clear_pgd_entry(kvm, pgd, start_addr); } @@ -306,7 +307,7 @@ void free_boot_hyp_pgd(void) if (boot_hyp_pgd) { unmap_range(NULL, boot_hyp_pgd, hyp_idmap_start, PAGE_SIZE); unmap_range(NULL, boot_hyp_pgd, TRAMPOLINE_VA, PAGE_SIZE); - free_pages((unsigned long)boot_hyp_pgd, pgd_order); + free_pages((unsigned long)boot_hyp_pgd, hyp_pgd_order); boot_hyp_pgd = NULL; } @@ -343,7 +344,7 @@ void free_hyp_pgds(void) for (addr = VMALLOC_START; is_vmalloc_addr((void*)addr); addr += PGDIR_SIZE) unmap_range(NULL, hyp_pgd, KERN_TO_HYP(addr), PGDIR_SIZE); - free_pages((unsigned long)hyp_pgd, pgd_order); + free_pages((unsigned long)hyp_pgd, hyp_pgd_order); hyp_pgd = NULL; } @@ -401,13 +402,46 @@ static int create_hyp_pmd_mappings(pud_t *pud, unsigned long start, return 0; } +static int create_hyp_pud_mappings(pgd_t *pgd, unsigned long start, + unsigned long end, unsigned long pfn, + pgprot_t prot) +{ + pud_t *pud; + pmd_t *pmd; + unsigned long addr, next; + int ret; + + addr = start; + do { + pud = pud_offset(pgd, addr); + + if (pud_none_or_clear_bad(pud)) { + pmd = pmd_alloc_one(NULL, addr); + if (!pmd) { + kvm_err("Cannot allocate Hyp pmd\n"); + return -ENOMEM; + } + pud_populate(NULL, pud, pmd); + get_page(virt_to_page(pud)); + kvm_flush_dcache_to_poc(pud, sizeof(*pud)); + } + + next = pud_addr_end(addr, end); + ret = create_hyp_pmd_mappings(pud, addr, next, pfn, prot); + if (ret) + return ret; + pfn += (next - addr) >> PAGE_SHIFT; + } while (addr = next, addr != end); + + return 0; +} + static int __create_hyp_mappings(pgd_t *pgdp, unsigned long start, unsigned long end, unsigned long pfn, pgprot_t prot) { pgd_t *pgd; pud_t *pud; - pmd_t *pmd; unsigned long addr, next; int err = 0; @@ -416,22 +450,21 @@ static int __create_hyp_mappings(pgd_t *pgdp, end = PAGE_ALIGN(end); do { pgd = pgdp + pgd_index(addr); - pud = pud_offset(pgd, addr); - if (pud_none_or_clear_bad(pud)) { - pmd = pmd_alloc_one(NULL, addr); - if (!pmd) { - kvm_err("Cannot allocate Hyp pmd\n"); + if (pgd_none(*pgd)) { + pud = pud_alloc_one(NULL, addr); + if (!pud) { + kvm_err("Cannot allocate Hyp pud\n"); err = -ENOMEM; goto out; } - pud_populate(NULL, pud, pmd); - get_page(virt_to_page(pud)); - kvm_flush_dcache_to_poc(pud, sizeof(*pud)); + pgd_populate(NULL, pgd, pud); + get_page(virt_to_page(pgd)); + kvm_flush_dcache_to_poc(pgd, sizeof(*pgd)); } next = pgd_addr_end(addr, end); - err = create_hyp_pmd_mappings(pud, addr, next, pfn, prot); + err = create_hyp_pud_mappings(pgd, addr, next, pfn, prot); if (err) goto out; pfn += (next - addr) >> PAGE_SHIFT; @@ -521,6 +554,7 @@ int create_hyp_io_mappings(void *from, void *to, phys_addr_t phys_addr) */ int kvm_alloc_stage2_pgd(struct kvm *kvm) { + int ret; pgd_t *pgd; if (kvm->arch.pgd != NULL) { @@ -533,9 +567,17 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) return -ENOMEM; memset(pgd, 0, PTRS_PER_S2_PGD * sizeof(pgd_t)); + + ret = kvm_prealloc_hwpgd(kvm, pgd); + if (ret) + goto out; + kvm_clean_pgd(pgd); kvm->arch.pgd = pgd; - + ret = 0; +out: + if (ret) + free_pages((unsigned long)pgd, S2_PGD_ORDER); return 0; } @@ -572,19 +614,36 @@ void kvm_free_stage2_pgd(struct kvm *kvm) return; unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE); + kvm_free_hwpgd(kvm); free_pages((unsigned long)kvm->arch.pgd, S2_PGD_ORDER); kvm->arch.pgd = NULL; } -static pmd_t *stage2_get_pmd(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, +static pud_t *stage2_get_pud(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, phys_addr_t addr) { pgd_t *pgd; pud_t *pud; - pmd_t *pmd; pgd = kvm->arch.pgd + pgd_index(addr); - pud = pud_offset(pgd, addr); + if (pgd_none(*pgd)) { + if (!cache) + return NULL; + pud = mmu_memory_cache_alloc(cache); + pgd_populate(NULL, pgd, pud); + get_page(virt_to_page(pgd)); + } + + return pud_offset(pgd, addr); +} + +static pmd_t *stage2_get_pmd(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, + phys_addr_t addr) +{ + pud_t *pud; + pmd_t *pmd; + + pud = stage2_get_pud(kvm, cache, addr); if (pud_none(*pud)) { if (!cache) return NULL; @@ -630,7 +689,7 @@ static int stage2_set_pte(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, pmd_t *pmd; pte_t *pte, old_pte; - /* Create stage-2 page table mapping - Level 1 */ + /* Create stage-2 page table mapping - Levels 0 and 1 */ pmd = stage2_get_pmd(kvm, cache, addr); if (!pmd) { /* @@ -688,7 +747,8 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa, for (addr = guest_ipa; addr < end; addr += PAGE_SIZE) { pte_t pte = pfn_pte(pfn, PAGE_S2_DEVICE); - ret = mmu_topup_memory_cache(&cache, 2, 2); + ret = mmu_topup_memory_cache(&cache, KVM_MMU_CACHE_MIN_PAGES, + KVM_MMU_CACHE_MIN_PAGES); if (ret) goto out; spin_lock(&kvm->mmu_lock); @@ -797,7 +857,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, up_read(¤t->mm->mmap_sem); /* We need minimum second+third level pages */ - ret = mmu_topup_memory_cache(memcache, 2, KVM_NR_MEM_OBJS); + ret = mmu_topup_memory_cache(memcache, KVM_MMU_CACHE_MIN_PAGES, + KVM_NR_MEM_OBJS); if (ret) return ret; @@ -1070,8 +1131,8 @@ int kvm_mmu_init(void) (unsigned long)phys_base); } - hyp_pgd = (pgd_t *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, pgd_order); - boot_hyp_pgd = (pgd_t *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, pgd_order); + hyp_pgd = (pgd_t *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, hyp_pgd_order); + boot_hyp_pgd = (pgd_t *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, hyp_pgd_order); if (!hyp_pgd || !boot_hyp_pgd) { kvm_err("Hyp mode PGD not allocated\n"); @@ -1113,6 +1174,8 @@ int kvm_mmu_init(void) goto out; } + kvm_prealloc_levels = KVM_PREALLOC_LEVELS; + return 0; out: free_hyp_pgds(); diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index fd4e81a..0f3e0a9 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -200,7 +200,6 @@ config ARM64_VA_BITS_42 config ARM64_VA_BITS_48 bool "48-bit" - depends on BROKEN endchoice diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h index a030d16..313c6f9 100644 --- a/arch/arm64/include/asm/kvm_mmu.h +++ b/arch/arm64/include/asm/kvm_mmu.h @@ -41,6 +41,18 @@ */ #define TRAMPOLINE_VA (HYP_PAGE_OFFSET_MASK & PAGE_MASK) +/* + * KVM_MMU_CACHE_MIN_PAGES is the number of stage2 page table translation + * levels in addition to the PGD and potentially the PUD which are + * pre-allocated (we pre-allocate the fake PGD and the PUD when the Stage-2 + * tables use one level of tables less than the kernel. + */ +#ifdef CONFIG_ARM64_64K_PAGES +#define KVM_MMU_CACHE_MIN_PAGES 1 +#else +#define KVM_MMU_CACHE_MIN_PAGES 2 +#endif + #ifdef __ASSEMBLY__ /* @@ -53,6 +65,7 @@ #else +#include <asm/pgalloc.h> #include <asm/cachetype.h> #include <asm/cacheflush.h> @@ -65,10 +78,6 @@ #define KVM_PHYS_SIZE (1UL << KVM_PHYS_SHIFT) #define KVM_PHYS_MASK (KVM_PHYS_SIZE - 1UL) -/* Make sure we get the right size, and thus the right alignment */ -#define PTRS_PER_S2_PGD (1 << (KVM_PHYS_SHIFT - PGDIR_SHIFT)) -#define S2_PGD_ORDER get_order(PTRS_PER_S2_PGD * sizeof(pgd_t)) - int create_hyp_mappings(void *from, void *to); int create_hyp_io_mappings(void *from, void *to, phys_addr_t); void free_boot_hyp_pgd(void); @@ -93,6 +102,7 @@ void kvm_clear_hyp_idmap(void); #define kvm_set_pmd(pmdp, pmd) set_pmd(pmdp, pmd) static inline void kvm_clean_pgd(pgd_t *pgd) {} +static inline void kvm_clean_pmd(pmd_t *pmd) {} static inline void kvm_clean_pmd_entry(pmd_t *pmd) {} static inline void kvm_clean_pte(pte_t *pte) {} static inline void kvm_clean_pte_entry(pte_t *pte) {} @@ -118,13 +128,123 @@ static inline bool kvm_page_empty(void *ptr) } #define kvm_pte_table_empty(ptep) kvm_page_empty(ptep) -#ifndef CONFIG_ARM64_64K_PAGES -#define kvm_pmd_table_empty(pmdp) kvm_page_empty(pmdp) -#else +#if CONFIG_ARM64_PGTABLE_LEVELS == 2 #define kvm_pmd_table_empty(pmdp) (0) -#endif #define kvm_pud_table_empty(pudp) (0) +#elif CONFIG_ARM64_PGTABLE_LEVELS == 3 +#define kvm_pmd_table_empty(pmdp) kvm_page_empty(pmdp) +#define kvm_pud_table_empty(pudp) (0) +#elif CONFIG_ARM64_PGTABLE_LEVELS == 4 +#define kvm_pmd_table_empty(pmdp) kvm_page_empty(pmdp) +#define kvm_pud_table_empty(pudp) kvm_page_empty(pudp) +#endif + +/** + * kvm_prealloc_hwpgd - allocate inital table for VTTBR + * @kvm: The KVM struct pointer for the VM. + * @pgd: The kernel pseudo pgd + * + * When the kernel uses more levels of page tables than the guest, we allocate + * a fake PGD and pre-populate it to point to the next-level page table, which + * will be the real initial page table pointed to by the VTTBR. + * + * When KVM_PREALLOC_PMD is defined, we allocate a single page for the PMD and + * the kernel will use folded pud. When KVM_PREALLOC_PUD is defined, we + * allocate 2 consecutive PUD pages. + */ +#if defined(CONFIG_ARM64_64K_PAGES) && CONFIG_ARM64_PGTABLE_LEVELS == 3 +#define KVM_PREALLOC_LEVELS 2 +#define PTRS_PER_S2_PGD 1 +#define S2_PGD_ORDER get_order(PTRS_PER_S2_PGD * sizeof(pgd_t)) + + +static inline int kvm_prealloc_hwpgd(struct kvm *kvm, pgd_t *pgd) +{ + pud_t *pud; + pmd_t *pmd; + + pud = pud_offset(pgd, 0); + pmd = (pmd_t *)__get_free_pages(GFP_KERNEL, 0); + + if (!pmd) + return -ENOMEM; + memset(pmd, 0, PAGE_SIZE); + pud_populate(NULL, pud, pmd); + get_page(virt_to_page(pud)); + + return 0; +} +static inline void kvm_free_hwpgd(struct kvm *kvm) +{ + pgd_t *pgd = kvm->arch.pgd; + pud_t *pud = pud_offset(pgd, 0); + pmd_t *pmd = pmd_offset(pud, 0); + free_pages((unsigned long)pmd, 0); + put_page(virt_to_page(pud)); +} + +static inline phys_addr_t kvm_get_hwpgd(struct kvm *kvm) +{ + pgd_t *pgd = kvm->arch.pgd; + pud_t *pud = pud_offset(pgd, 0); + pmd_t *pmd = pmd_offset(pud, 0); + return virt_to_phys(pmd); + +} +#elif defined(CONFIG_ARM64_4K_PAGES) && CONFIG_ARM64_PGTABLE_LEVELS == 4 +#define KVM_PREALLOC_LEVELS 1 +#define PTRS_PER_S2_PGD 2 +#define S2_PGD_ORDER get_order(PTRS_PER_S2_PGD * sizeof(pgd_t)) + +static inline int kvm_prealloc_hwpgd(struct kvm *kvm, pgd_t *pgd) +{ + pud_t *pud; + + pud = (pud_t *)__get_free_pages(GFP_KERNEL, 1); + if (!pud) + return -ENOMEM; + memset(pud, 0, 2 * PAGE_SIZE); + pgd_populate(NULL, pgd, pud); + pgd_populate(NULL, pgd + 1, pud + PTRS_PER_PUD); + get_page(virt_to_page(pgd)); + get_page(virt_to_page(pgd)); + + return 0; +} + +static inline void kvm_free_hwpgd(struct kvm *kvm) +{ + pgd_t *pgd = kvm->arch.pgd; + pud_t *pud = pud_offset(pgd, 0); + free_pages((unsigned long)pud, 1); + put_page(virt_to_page(pgd)); + put_page(virt_to_page(pgd)); +} + +static inline phys_addr_t kvm_get_hwpgd(struct kvm *kvm) +{ + pgd_t *pgd = kvm->arch.pgd; + pud_t *pud = pud_offset(pgd, 0); + return virt_to_phys(pud); +} +#else +#define KVM_PREALLOC_LEVELS 0 +#define PTRS_PER_S2_PGD (1 << (KVM_PHYS_SHIFT - PGDIR_SHIFT)) +#define S2_PGD_ORDER get_order(PTRS_PER_S2_PGD * sizeof(pgd_t)) + +static inline int kvm_prealloc_hwpgd(struct kvm *kvm, pgd_t *pgd) +{ + return 0; +} + +static inline void kvm_free_hwpgd(struct kvm *kvm) { } + +static inline phys_addr_t kvm_get_hwpgd(struct kvm *kvm) +{ + return virt_to_phys(kvm->arch.pgd); +} +#endif struct kvm;