| Message ID | 20181001155443.23032-3-punit.agrawal@arm.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | KVM: Support PUD hugepage at stage 2 | expand |
On 10/01/2018 04:54 PM, Punit Agrawal wrote: > The code for operations such as marking the pfn as dirty, and > dcache/icache maintenance during stage 2 fault handling is duplicated > between normal pages and PMD hugepages. > > Instead of creating another copy of the operations when we introduce > PUD hugepages, let's share them across the different pagesizes. > > Signed-off-by: Punit Agrawal <punit.agrawal@arm.com> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: Christoffer Dall <christoffer.dall@arm.com> > Cc: Marc Zyngier <marc.zyngier@arm.com> > --- Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Hi Punit, On 01/10/18 16:54, Punit Agrawal wrote: > The code for operations such as marking the pfn as dirty, and > dcache/icache maintenance during stage 2 fault handling is duplicated > between normal pages and PMD hugepages. > > Instead of creating another copy of the operations when we introduce > PUD hugepages, let's share them across the different pagesizes. > > Signed-off-by: Punit Agrawal <punit.agrawal@arm.com> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: Christoffer Dall <christoffer.dall@arm.com> > Cc: Marc Zyngier <marc.zyngier@arm.com> > --- > virt/kvm/arm/mmu.c | 45 +++++++++++++++++++++++++++++---------------- > 1 file changed, 29 insertions(+), 16 deletions(-) > > diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c > index c23a1b323aad..5b76ee204000 100644 > --- a/virt/kvm/arm/mmu.c > +++ b/virt/kvm/arm/mmu.c > @@ -1490,7 +1490,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, > kvm_pfn_t pfn; > pgprot_t mem_type = PAGE_S2; > bool logging_active = memslot_is_logging(memslot); > - unsigned long flags = 0; > + unsigned long vma_pagesize, flags = 0; > > write_fault = kvm_is_write_fault(vcpu); > exec_fault = kvm_vcpu_trap_is_iabt(vcpu); > @@ -1510,10 +1510,17 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, > return -EFAULT; > } > > - if (vma_kernel_pagesize(vma) == PMD_SIZE && !logging_active) { > + vma_pagesize = vma_kernel_pagesize(vma); > + if (vma_pagesize == PMD_SIZE && !logging_active) { > hugetlb = true; > gfn = (fault_ipa & PMD_MASK) >> PAGE_SHIFT; > } else { > + /* > + * Fallback to PTE if it's not one of the Stage 2 > + * supported hugepage sizes > + */ > + vma_pagesize = PAGE_SIZE; > + > /* > * Pages belonging to memslots that don't have the same > * alignment for userspace and IPA cannot be mapped using > @@ -1579,23 +1586,34 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, > if (mmu_notifier_retry(kvm, mmu_seq)) > goto out_unlock; > > - if (!hugetlb && !force_pte) > + if (!hugetlb && !force_pte) { > + /* > + * Only PMD_SIZE transparent hugepages(THP) are > + * currently supported. This code will need to be > + * updated to support other THP sizes. > + */ > hugetlb = transparent_hugepage_adjust(&pfn, &fault_ipa); > + if (hugetlb) > + vma_pagesize = PMD_SIZE; > + } > + > + if (writable) > + kvm_set_pfn_dirty(pfn); > > - if (hugetlb) { > + if (fault_status != FSC_PERM) > + clean_dcache_guest_page(pfn, vma_pagesize); > + > + if (exec_fault) > + invalidate_icache_guest_page(pfn, vma_pagesize); > + > + if (hugetlb && vma_pagesize == PMD_SIZE) { Can you end-up in a situation where hugetlb==false and vma_pagesize == PMD_SIZE? If that's the case, then the above CMOs are not done on the same granularity as they were done before this patch. If that cannot happen, then the above condition can be simplified. Which one is it? Thanks, M.
Marc Zyngier <marc.zyngier@arm.com> writes: > Hi Punit, > > On 01/10/18 16:54, Punit Agrawal wrote: >> The code for operations such as marking the pfn as dirty, and >> dcache/icache maintenance during stage 2 fault handling is duplicated >> between normal pages and PMD hugepages. >> >> Instead of creating another copy of the operations when we introduce >> PUD hugepages, let's share them across the different pagesizes. >> >> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com> >> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> >> Cc: Christoffer Dall <christoffer.dall@arm.com> >> Cc: Marc Zyngier <marc.zyngier@arm.com> >> --- >> virt/kvm/arm/mmu.c | 45 +++++++++++++++++++++++++++++---------------- >> 1 file changed, 29 insertions(+), 16 deletions(-) >> >> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c >> index c23a1b323aad..5b76ee204000 100644 >> --- a/virt/kvm/arm/mmu.c >> +++ b/virt/kvm/arm/mmu.c >> @@ -1490,7 +1490,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, >> kvm_pfn_t pfn; >> pgprot_t mem_type = PAGE_S2; >> bool logging_active = memslot_is_logging(memslot); >> - unsigned long flags = 0; >> + unsigned long vma_pagesize, flags = 0; >> write_fault = kvm_is_write_fault(vcpu); >> exec_fault = kvm_vcpu_trap_is_iabt(vcpu); >> @@ -1510,10 +1510,17 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, >> return -EFAULT; >> } >> - if (vma_kernel_pagesize(vma) == PMD_SIZE && !logging_active) { >> + vma_pagesize = vma_kernel_pagesize(vma); >> + if (vma_pagesize == PMD_SIZE && !logging_active) { >> hugetlb = true; >> gfn = (fault_ipa & PMD_MASK) >> PAGE_SHIFT; >> } else { >> + /* >> + * Fallback to PTE if it's not one of the Stage 2 >> + * supported hugepage sizes >> + */ >> + vma_pagesize = PAGE_SIZE; >> + >> /* >> * Pages belonging to memslots that don't have the same >> * alignment for userspace and IPA cannot be mapped using >> @@ -1579,23 +1586,34 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, >> if (mmu_notifier_retry(kvm, mmu_seq)) >> goto out_unlock; >> - if (!hugetlb && !force_pte) >> + if (!hugetlb && !force_pte) { >> + /* >> + * Only PMD_SIZE transparent hugepages(THP) are >> + * currently supported. This code will need to be >> + * updated to support other THP sizes. >> + */ >> hugetlb = transparent_hugepage_adjust(&pfn, &fault_ipa); >> + if (hugetlb) >> + vma_pagesize = PMD_SIZE; >> + } >> + >> + if (writable) >> + kvm_set_pfn_dirty(pfn); >> - if (hugetlb) { >> + if (fault_status != FSC_PERM) >> + clean_dcache_guest_page(pfn, vma_pagesize); >> + >> + if (exec_fault) >> + invalidate_icache_guest_page(pfn, vma_pagesize); >> + >> + if (hugetlb && vma_pagesize == PMD_SIZE) { > > Can you end-up in a situation where hugetlb==false and vma_pagesize == > PMD_SIZE? If that's the case, then the above CMOs are not done on the > same granularity as they were done before this patch. If that cannot > happen, then the above condition can be simplified. > > Which one is it? hugetlb is a hangover from when we didn't have vma_pagesize. I think we can drop it and rely on the pagesize to control the size of mapping we put down. Let me give that a try. Thanks for taking a look. > > > Thanks, > > M.
diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c index c23a1b323aad..5b76ee204000 100644 --- a/virt/kvm/arm/mmu.c +++ b/virt/kvm/arm/mmu.c @@ -1490,7 +1490,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, kvm_pfn_t pfn; pgprot_t mem_type = PAGE_S2; bool logging_active = memslot_is_logging(memslot); - unsigned long flags = 0; + unsigned long vma_pagesize, flags = 0; write_fault = kvm_is_write_fault(vcpu); exec_fault = kvm_vcpu_trap_is_iabt(vcpu); @@ -1510,10 +1510,17 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, return -EFAULT; } - if (vma_kernel_pagesize(vma) == PMD_SIZE && !logging_active) { + vma_pagesize = vma_kernel_pagesize(vma); + if (vma_pagesize == PMD_SIZE && !logging_active) { hugetlb = true; gfn = (fault_ipa & PMD_MASK) >> PAGE_SHIFT; } else { + /* + * Fallback to PTE if it's not one of the Stage 2 + * supported hugepage sizes + */ + vma_pagesize = PAGE_SIZE; + /* * Pages belonging to memslots that don't have the same * alignment for userspace and IPA cannot be mapped using @@ -1579,23 +1586,34 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, if (mmu_notifier_retry(kvm, mmu_seq)) goto out_unlock; - if (!hugetlb && !force_pte) + if (!hugetlb && !force_pte) { + /* + * Only PMD_SIZE transparent hugepages(THP) are + * currently supported. This code will need to be + * updated to support other THP sizes. + */ hugetlb = transparent_hugepage_adjust(&pfn, &fault_ipa); + if (hugetlb) + vma_pagesize = PMD_SIZE; + } + + if (writable) + kvm_set_pfn_dirty(pfn); - if (hugetlb) { + if (fault_status != FSC_PERM) + clean_dcache_guest_page(pfn, vma_pagesize); + + if (exec_fault) + invalidate_icache_guest_page(pfn, vma_pagesize); + + if (hugetlb && vma_pagesize == PMD_SIZE) { pmd_t new_pmd = pfn_pmd(pfn, mem_type); new_pmd = pmd_mkhuge(new_pmd); - if (writable) { + if (writable) new_pmd = kvm_s2pmd_mkwrite(new_pmd); - kvm_set_pfn_dirty(pfn); - } - - if (fault_status != FSC_PERM) - clean_dcache_guest_page(pfn, PMD_SIZE); if (exec_fault) { new_pmd = kvm_s2pmd_mkexec(new_pmd); - invalidate_icache_guest_page(pfn, PMD_SIZE); } else if (fault_status == FSC_PERM) { /* Preserve execute if XN was already cleared */ if (stage2_is_exec(kvm, fault_ipa)) @@ -1608,16 +1626,11 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, if (writable) { new_pte = kvm_s2pte_mkwrite(new_pte); - kvm_set_pfn_dirty(pfn); mark_page_dirty(kvm, gfn); } - if (fault_status != FSC_PERM) - clean_dcache_guest_page(pfn, PAGE_SIZE); - if (exec_fault) { new_pte = kvm_s2pte_mkexec(new_pte); - invalidate_icache_guest_page(pfn, PAGE_SIZE); } else if (fault_status == FSC_PERM) { /* Preserve execute if XN was already cleared */ if (stage2_is_exec(kvm, fault_ipa))
The code for operations such as marking the pfn as dirty, and dcache/icache maintenance during stage 2 fault handling is duplicated between normal pages and PMD hugepages. Instead of creating another copy of the operations when we introduce PUD hugepages, let's share them across the different pagesizes. Signed-off-by: Punit Agrawal <punit.agrawal@arm.com> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> Cc: Christoffer Dall <christoffer.dall@arm.com> Cc: Marc Zyngier <marc.zyngier@arm.com> --- virt/kvm/arm/mmu.c | 45 +++++++++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 16 deletions(-)