KVM: Support PUD hugepage at stage 2

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 <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

---
 virt/kvm/arm/mmu.c | 49 ++++++++++++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 19 deletions(-)

virt/kvm/arm/mmu.c +1 -1

index 5eca48bdb1a6..59595207c5e1 100644
@@ -1475,7 +1475,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 			  unsigned long fault_status)
 {
 	int ret;
-	bool write_fault, exec_fault, writable, hugetlb = false, force_pte = false;
+	bool write_fault, exec_fault, writable, force_pte = false;
 	unsigned long mmu_seq;
 	gfn_t gfn = fault_ipa >> PAGE_SHIFT;
 	struct kvm *kvm = vcpu->kvm;

virt/kvm/arm/mmu.c +0 -1

 	write_fault = kvm_is_write_fault(vcpu);
 	exec_fault = kvm_vcpu_trap_is_iabt(vcpu);

@@ -1504,10 +1504,16 @@ 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) {

virt/kvm/arm/mmu.c +1 -0

+
 		/*
 		 * Pages belonging to memslots that don't have the same
 		 * alignment for userspace and IPA cannot be mapped using

@@ -1573,23 +1579,33 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	if (mmu_notifier_retry(kvm, mmu_seq))

virt/kvm/arm/mmu.c +12 -9

+	}
+
+	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 (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))

@@ -1602,16 +1618,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);
 		}

Quoted 28 lines · Punit Agrawal

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 <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

---
  virt/kvm/arm/mmu.c | 49 ++++++++++++++++++++++++++++------------------
  1 file changed, 30 insertions(+), 19 deletions(-)

virt/kvm/arm/mmu.c +1 -1

index 5eca48bdb1a6..59595207c5e1 100644
@@ -1475,7 +1475,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
  			  unsigned long fault_status)
  {
  	int ret;
-	bool write_fault, exec_fault, writable, hugetlb = false, force_pte = false;
+	bool write_fault, exec_fault, writable, force_pte = false;
  	unsigned long mmu_seq;
  	gfn_t gfn = fault_ipa >> PAGE_SHIFT;
  	struct kvm *kvm = vcpu->kvm;

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1487

Quoted 6 lines

virt/kvm/arm/mmu.c +1 -1

@@ -1484,7 +1484,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;

A small nit s/vma_pagesize/pagesize. Why call it VMA ? Its implicit.

Quoted 18 lines

virt/kvm/arm/mmu.c +6 -2

  	write_fault = kvm_is_write_fault(vcpu);
  	exec_fault = kvm_vcpu_trap_is_iabt(vcpu);

@@ -1504,10 +1504,16 @@ 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) {
-		hugetlb = true;
+	vma_pagesize = vma_kernel_pagesize(vma);
+	if (vma_pagesize == PMD_SIZE && !logging_active) {
  		gfn = (fault_ipa & PMD_MASK) >> PAGE_SHIFT;
  	} else {
+		/*
+		 * Fallback to PTE if it's not one of the Stage 2
+		 * supported hugepage sizes
+		 */

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1515

Quoted 1 line

virt/kvm/arm/mmu.c +1 -0

+		vma_pagesize = PAGE_SIZE;

This seems redundant and should be dropped. vma_kernel_pagesize() here either calls hugetlb_vm_op_pagesize (via hugetlb_vm_ops->pagesize) or simply returns PAGE_SIZE. The vm_ops path is taken if the QEMU VMA covering any given HVA is backed either by HugeTLB pages or simply normal pages. vma_pagesize would either has a value of PMD_SIZE (HugeTLB hstate based) or PAGE_SIZE. Hence if its not PMD_SIZE it must be PAGE_SIZE which should not be assigned again.

Quoted 9 lines

virt/kvm/arm/mmu.c +1 -0

+
  		/*
  		 * Pages belonging to memslots that don't have the same
  		 * alignment for userspace and IPA cannot be mapped using

@@ -1573,23 +1579,33 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
  	if (mmu_notifier_retry(kvm, mmu_seq))
  		goto out_unlock;
 

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1587

Quoted 8 lines

virt/kvm/arm/mmu.c +6 -2

-	if (!hugetlb && !force_pte)
-		hugetlb = transparent_hugepage_adjust(&pfn, &fault_ipa);
+	if (vma_pagesize == PAGE_SIZE && !force_pte) {
+		/*
+		 * Only PMD_SIZE transparent hugepages(THP) are
+		 * currently supported. This code will need to be
+		 * updated to support other THP sizes.
+		 */

This comment belongs to transparent_hugepage_adjust() but not here.

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1589

Quoted 2 lines

virt/kvm/arm/mmu.c +2 -0

+		if (transparent_hugepage_adjust(&pfn, &fault_ipa))
+			vma_pagesize = PMD_SIZE;

IIUC transparent_hugepage_adjust() is only getting called here. Instead of returning 'true' when it is able to detect a huge page backing and doing an adjustment there after, it should rather return THP size (PMD_SIZE) to accommodate probable multi size THP support in future .

Quoted 39 lines

virt/kvm/arm/mmu.c +12 -9

+	}
+
+	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 (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))

@@ -1602,16 +1618,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);
  		}
 

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1628

Quoted 10 lines

virt/kvm/arm/mmu.c +0 -4

-		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))

kvm_set_pfn_dirty, clean_dcache_guest_page, invalidate_icache_guest_page can all be safely moved before setting the page table entries either as PMD or PTE.

Hi Anshuman,

Quoted 31 lines · Anshuman Khandual

Quoted 28 lines · Punit Agrawal

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 <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

---
 virt/kvm/arm/mmu.c | 49 ++++++++++++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 19 deletions(-)

virt/kvm/arm/mmu.c +1 -1

index 5eca48bdb1a6..59595207c5e1 100644
@@ -1475,7 +1475,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 			  unsigned long fault_status)
 {
 	int ret;
-	bool write_fault, exec_fault, writable, hugetlb = false, force_pte = false;
+	bool write_fault, exec_fault, writable, force_pte = false;
 	unsigned long mmu_seq;
 	gfn_t gfn = fault_ipa >> PAGE_SHIFT;
 	struct kvm *kvm = vcpu->kvm;

Quoted 8 lines

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1487

Quoted 6 lines

virt/kvm/arm/mmu.c +1 -1

@@ -1484,7 +1484,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;

A small nit s/vma_pagesize/pagesize. Why call it VMA ? Its implicit.

May be we could call it mapsize. pagesize is confusing.

Quoted 16 lines

Quoted 16 lines

virt/kvm/arm/mmu.c +6 -2

 	write_fault = kvm_is_write_fault(vcpu);
 	exec_fault = kvm_vcpu_trap_is_iabt(vcpu);

@@ -1504,10 +1504,16 @@ 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) {
-		hugetlb = true;
+	vma_pagesize = vma_kernel_pagesize(vma);
+	if (vma_pagesize == PMD_SIZE && !logging_active) {
 		gfn = (fault_ipa & PMD_MASK) >> PAGE_SHIFT;
 	} else {
+		/*
+		 * Fallback to PTE if it's not one of the Stage 2
+		 * supported hugepage sizes
+		 */

Quoted 8 lines

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1515

Quoted 1 line

virt/kvm/arm/mmu.c +1 -0

+		vma_pagesize = PAGE_SIZE;

This seems redundant and should be dropped. vma_kernel_pagesize() here either calls hugetlb_vm_op_pagesize (via hugetlb_vm_ops->pagesize) or simply returns PAGE_SIZE. The vm_ops path is taken if the QEMU VMA covering any given HVA is backed either by HugeTLB pages or simply normal pages. vma_pagesize would either has a value of PMD_SIZE (HugeTLB hstate based) or PAGE_SIZE. Hence if its not PMD_SIZE it must be PAGE_SIZE which should not be assigned again.

We may want to force using the PTE mappings when logging_active (e.g, migration ?) to prevent keep tracking of huge pages. So the check is still valid.

Quoted 10 lines

Quoted 10 lines

virt/kvm/arm/mmu.c +1 -2

+
 		/*
 		 * Pages belonging to memslots that don't have the same
 		 * alignment for userspace and IPA cannot be mapped using

@@ -1573,23 +1579,33 @@ 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)
-		hugetlb = transparent_hugepage_adjust(&pfn, &fault_ipa);

Quoted 8 lines

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1587

Quoted 6 lines

virt/kvm/arm/mmu.c +6 -0

+	if (vma_pagesize == PAGE_SIZE && !force_pte) {
+		/*
+		 * Only PMD_SIZE transparent hugepages(THP) are
+		 * currently supported. This code will need to be
+		 * updated to support other THP sizes.
+		 */

This comment belongs to transparent_hugepage_adjust() but not here.

I think this is relevant here than in thp_adjust, unless we rename the function below to something generic, handle_hugepage_adjust().

Quoted 7 lines

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1589

Quoted 2 lines

virt/kvm/arm/mmu.c +2 -0

+		if (transparent_hugepage_adjust(&pfn, &fault_ipa))
+			vma_pagesize = PMD_SIZE;

IIUC transparent_hugepage_adjust() is only getting called here. Instead of returning 'true' when it is able to detect a huge page backing and doing an adjustment there after, it should rather return THP size (PMD_SIZE) to accommodate probable multi size THP support in future .

That makes sense.

Quoted 41 lines

Quoted 41 lines

virt/kvm/arm/mmu.c +12 -13

+	}
+
+	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 (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))

@@ -1602,16 +1618,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);

Quoted 8 lines

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,

Quoted 4 lines

virt/kvm/arm/mmu.c +0 -0

 		} else if (fault_status == FSC_PERM) {
 			/* Preserve execute if XN was already cleared */
 			if (stage2_is_exec(kvm, fault_ipa))
 

kvm_set_pfn_dirty, clean_dcache_guest_page, invalidate_icache_guest_page can all be safely moved before setting the page table entries either as PMD or PTE.

I think this is what we do currently. So I assume this is fine.

Quoted 43 lines · Suzuki K Poulose

Hi Anshuman,

Quoted 39 lines · Anshuman Khandual

Quoted 29 lines · Punit Agrawal

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 <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

---
  virt/kvm/arm/mmu.c | 49 ++++++++++++++++++++++++++++------------------
  1 file changed, 30 insertions(+), 19 deletions(-)

virt/kvm/arm/mmu.c +1 -1

index 5eca48bdb1a6..59595207c5e1 100644
@@ -1475,7 +1475,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
  			  unsigned long fault_status)
  {
  	int ret;
-	bool write_fault, exec_fault, writable, hugetlb = false, force_pte = false;
+	bool write_fault, exec_fault, writable, force_pte = false;
  	unsigned long mmu_seq;
  	gfn_t gfn = fault_ipa >> PAGE_SHIFT;
  	struct kvm *kvm = vcpu->kvm;

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1487

Quoted 5 lines

virt/kvm/arm/mmu.c +1 -1

@@ -1484,7 +1484,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;

A small nit s/vma_pagesize/pagesize. Why call it VMA ? Its implicit.

Quoted 2 lines

May be we could call it mapsize. pagesize is confusing.

I'm ok with mapsize. I see the vma_pagesize name coming from the fact that this is initially set to the return value from vma_kernel_pagesize.

I have not problems with either vma_pagesize or mapsize.

Quoted 43 lines · Suzuki K Poulose

Hi Anshuman,

Quoted 31 lines · Anshuman Khandual

Quoted 28 lines · Punit Agrawal

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 <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

---
  virt/kvm/arm/mmu.c | 49 ++++++++++++++++++++++++++++------------------
  1 file changed, 30 insertions(+), 19 deletions(-)

virt/kvm/arm/mmu.c +1 -1

index 5eca48bdb1a6..59595207c5e1 100644
@@ -1475,7 +1475,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
  			  unsigned long fault_status)
  {
  	int ret;
-	bool write_fault, exec_fault, writable, hugetlb = false, force_pte = false;
+	bool write_fault, exec_fault, writable, force_pte = false;
  	unsigned long mmu_seq;
  	gfn_t gfn = fault_ipa >> PAGE_SHIFT;
  	struct kvm *kvm = vcpu->kvm;

Quoted 8 lines

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1487

Quoted 6 lines

virt/kvm/arm/mmu.c +1 -1

@@ -1484,7 +1484,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;

A small nit s/vma_pagesize/pagesize. Why call it VMA ? Its implicit.

Quoted 2 lines

May be we could call it mapsize. pagesize is confusing.

I'm ok with mapsize. I see the vma_pagesize name coming from the fact that this is initially set to the return value from vma_kernel_pagesize.

I have not problems with either vma_pagesize or mapsize.

Quoted 25 lines

Quoted 16 lines

Quoted 16 lines

virt/kvm/arm/mmu.c +6 -2

  	write_fault = kvm_is_write_fault(vcpu);
  	exec_fault = kvm_vcpu_trap_is_iabt(vcpu);

@@ -1504,10 +1504,16 @@ 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) {
-		hugetlb = true;
+	vma_pagesize = vma_kernel_pagesize(vma);
+	if (vma_pagesize == PMD_SIZE && !logging_active) {
  		gfn = (fault_ipa & PMD_MASK) >> PAGE_SHIFT;
  	} else {
+		/*
+		 * Fallback to PTE if it's not one of the Stage 2
+		 * supported hugepage sizes
+		 */

Quoted 8 lines

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1515

Quoted 1 line

virt/kvm/arm/mmu.c +1 -0

+		vma_pagesize = PAGE_SIZE;

This seems redundant and should be dropped. vma_kernel_pagesize() here either calls hugetlb_vm_op_pagesize (via hugetlb_vm_ops->pagesize) or simply returns PAGE_SIZE. The vm_ops path is taken if the QEMU VMA covering any given HVA is backed either by HugeTLB pages or simply normal pages. vma_pagesize would either has a value of PMD_SIZE (HugeTLB hstate based) or PAGE_SIZE. Hence if its not PMD_SIZE it must be PAGE_SIZE which should not be assigned again.

Quoted 4 lines

We may want to force using the PTE mappings when logging_active (e.g, migration ?) to prevent keep tracking of huge pages. So the check is still valid.

Agreed, and let's not try additionally change the logic and flow with this patch.

Quoted 19 lines

Quoted 10 lines

Quoted 10 lines

virt/kvm/arm/mmu.c +1 -2

+
  		/*
  		 * Pages belonging to memslots that don't have the same
  		 * alignment for userspace and IPA cannot be mapped using

@@ -1573,23 +1579,33 @@ 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)
-		hugetlb = transparent_hugepage_adjust(&pfn, &fault_ipa);

Quoted 8 lines

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1587

Quoted 6 lines

virt/kvm/arm/mmu.c +6 -0

+	if (vma_pagesize == PAGE_SIZE && !force_pte) {
+		/*
+		 * Only PMD_SIZE transparent hugepages(THP) are
+		 * currently supported. This code will need to be
+		 * updated to support other THP sizes.
+		 */

This comment belongs to transparent_hugepage_adjust() but not here.

Quoted 3 lines

I think this is relevant here than in thp_adjust, unless we rename the function below to something generic, handle_hugepage_adjust().

Agreed.

Quoted 8 lines

Quoted 7 lines

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1589

Quoted 2 lines

virt/kvm/arm/mmu.c +2 -0

+		if (transparent_hugepage_adjust(&pfn, &fault_ipa))
+			vma_pagesize = PMD_SIZE;

IIUC transparent_hugepage_adjust() is only getting called here. Instead of returning 'true' when it is able to detect a huge page backing and doing an adjustment there after, it should rather return THP size (PMD_SIZE) to accommodate probable multi size THP support in future .

Quoted 2 lines

That makes sense.

That's fine.

Quoted 121 lines · Christoffer Dall

Quoted 43 lines · Suzuki K Poulose

Hi Anshuman,

Quoted 31 lines · Anshuman Khandual

Quoted 28 lines · Punit Agrawal

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 <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

---
 virt/kvm/arm/mmu.c | 49 ++++++++++++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 19 deletions(-)

virt/kvm/arm/mmu.c +1 -1

index 5eca48bdb1a6..59595207c5e1 100644
@@ -1475,7 +1475,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 			  unsigned long fault_status)
 {
 	int ret;
-	bool write_fault, exec_fault, writable, hugetlb = false, force_pte = false;
+	bool write_fault, exec_fault, writable, force_pte = false;
 	unsigned long mmu_seq;
 	gfn_t gfn = fault_ipa >> PAGE_SHIFT;
 	struct kvm *kvm = vcpu->kvm;

Quoted 8 lines

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1487

Quoted 6 lines

virt/kvm/arm/mmu.c +1 -1

@@ -1484,7 +1484,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;

A small nit s/vma_pagesize/pagesize. Why call it VMA ? Its implicit.

Quoted 2 lines

May be we could call it mapsize. pagesize is confusing.

I'm ok with mapsize. I see the vma_pagesize name coming from the fact that this is initially set to the return value from vma_kernel_pagesize.

I have not problems with either vma_pagesize or mapsize.

Quoted 25 lines

Quoted 16 lines

Quoted 16 lines

virt/kvm/arm/mmu.c +6 -2

 	write_fault = kvm_is_write_fault(vcpu);
 	exec_fault = kvm_vcpu_trap_is_iabt(vcpu);

@@ -1504,10 +1504,16 @@ 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) {
-		hugetlb = true;
+	vma_pagesize = vma_kernel_pagesize(vma);
+	if (vma_pagesize == PMD_SIZE && !logging_active) {
 		gfn = (fault_ipa & PMD_MASK) >> PAGE_SHIFT;
 	} else {
+		/*
+		 * Fallback to PTE if it's not one of the Stage 2
+		 * supported hugepage sizes
+		 */

Quoted 8 lines

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1515

Quoted 1 line

virt/kvm/arm/mmu.c +1 -0

+		vma_pagesize = PAGE_SIZE;

This seems redundant and should be dropped. vma_kernel_pagesize() here either calls hugetlb_vm_op_pagesize (via hugetlb_vm_ops->pagesize) or simply returns PAGE_SIZE. The vm_ops path is taken if the QEMU VMA covering any given HVA is backed either by HugeTLB pages or simply normal pages. vma_pagesize would either has a value of PMD_SIZE (HugeTLB hstate based) or PAGE_SIZE. Hence if its not PMD_SIZE it must be PAGE_SIZE which should not be assigned again.

Quoted 4 lines

We may want to force using the PTE mappings when logging_active (e.g, migration ?) to prevent keep tracking of huge pages. So the check is still valid.

Agreed, and let's not try additionally change the logic and flow with this patch.

Quoted 19 lines

Quoted 10 lines

Quoted 10 lines

virt/kvm/arm/mmu.c +1 -2

+
 		/*
 		 * Pages belonging to memslots that don't have the same
 		 * alignment for userspace and IPA cannot be mapped using

@@ -1573,23 +1579,33 @@ 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)
-		hugetlb = transparent_hugepage_adjust(&pfn, &fault_ipa);

Quoted 8 lines

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1587

Quoted 6 lines

virt/kvm/arm/mmu.c +6 -0

+	if (vma_pagesize == PAGE_SIZE && !force_pte) {
+		/*
+		 * Only PMD_SIZE transparent hugepages(THP) are
+		 * currently supported. This code will need to be
+		 * updated to support other THP sizes.
+		 */

This comment belongs to transparent_hugepage_adjust() but not here.

Quoted 3 lines

I think this is relevant here than in thp_adjust, unless we rename the function below to something generic, handle_hugepage_adjust().

Agreed.

Quoted 10 lines

Quoted 7 lines

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1589

Quoted 2 lines

virt/kvm/arm/mmu.c +2 -0

+		if (transparent_hugepage_adjust(&pfn, &fault_ipa))
+			vma_pagesize = PMD_SIZE;

IIUC transparent_hugepage_adjust() is only getting called here. Instead of returning 'true' when it is able to detect a huge page backing and doing an adjustment there after, it should rather return THP size (PMD_SIZE) to accommodate probable multi size THP support in future .

That makes sense.

Quoted 2 lines

That's fine.

Btw, after a further thought, since we don't have any THP support for anything other than PMD_SIZE, I am dropping the above suggestion. We need to make changes in our stage2 page table manipulation code anyway to support the new sizes. So this could be addressed when we get there, to keep the changes minimal and specific to the PUD huge page support.

Stage 2 fault handler marks a page as executable if it is handling an execution fault or if it was a permission fault in which case the executable bit needs to be preserved.

The logic to decide if the page should be marked executable is duplicated for PMD and PTE entries. To avoid creating another copy when support for PUD hugepages is introduced refactor the code to share the checks needed to mark a page table entry as executable.

Signed-off-by Punit Agrawal <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

---
 virt/kvm/arm/mmu.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

virt/kvm/arm/mmu.c +0 -0

index 59595207c5e1..6912529946fb 100644

Quoted 22 lines · Punit Agrawal

Stage 2 fault handler marks a page as executable if it is handling an execution fault or if it was a permission fault in which case the executable bit needs to be preserved.

The logic to decide if the page should be marked executable is duplicated for PMD and PTE entries. To avoid creating another copy when support for PUD hugepages is introduced refactor the code to share the checks needed to mark a page table entry as executable.

Signed-off-by Punit Agrawal <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

---
  virt/kvm/arm/mmu.c | 28 +++++++++++++++-------------
  1 file changed, 15 insertions(+), 13 deletions(-)

virt/kvm/arm/mmu.c +0 -0

index 59595207c5e1..6912529946fb 100644

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1479

Quoted 6 lines

virt/kvm/arm/mmu.c +2 -1

@@ -1475,7 +1475,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
  			  unsigned long fault_status)
  {
  	int ret;
-	bool write_fault, exec_fault, writable, force_pte = false;
+	bool write_fault, writable, force_pte = false;
+	bool exec_fault, needs_exec;

New line not required, still within 80 characters.

Quoted 7 lines

virt/kvm/arm/mmu.c +0 -0

  	unsigned long mmu_seq;
  	gfn_t gfn = fault_ipa >> PAGE_SHIFT;
  	struct kvm *kvm = vcpu->kvm;

@@ -1598,19 +1599,25 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
  	if (exec_fault)
  		invalidate_icache_guest_page(pfn, vma_pagesize);
 

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1604

Quoted 3 lines

virt/kvm/arm/mmu.c +3 -0

+	/*
+	 * If we took an execution fault we have made the
+	 * icache/dcache coherent above and should now let the s2

Coherent or invalidated with invalidate_icache_guest_page ?

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1609

Quoted 6 lines

virt/kvm/arm/mmu.c +5 -0

+	 * mapping be executable.
+	 *
+	 * Write faults (!exec_fault && FSC_PERM) are orthogonal to
+	 * execute permissions, and we preserve whatever we have.
+	 */

Otherwise looks good.

Quoted 34 lines · Anshuman Khandual

Quoted 22 lines · Punit Agrawal

Stage 2 fault handler marks a page as executable if it is handling an execution fault or if it was a permission fault in which case the executable bit needs to be preserved.

The logic to decide if the page should be marked executable is duplicated for PMD and PTE entries. To avoid creating another copy when support for PUD hugepages is introduced refactor the code to share the checks needed to mark a page table entry as executable.

Signed-off-by Punit Agrawal <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

---
 virt/kvm/arm/mmu.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

virt/kvm/arm/mmu.c +0 -0

index 59595207c5e1..6912529946fb 100644

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1479

Quoted 6 lines

virt/kvm/arm/mmu.c +2 -1

@@ -1475,7 +1475,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 			  unsigned long fault_status)
 {
 	int ret;
-	bool write_fault, exec_fault, writable, force_pte = false;
+	bool write_fault, writable, force_pte = false;
+	bool exec_fault, needs_exec;

New line not required, still within 80 characters.

Quoted 11 lines

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1603

Quoted 9 lines

virt/kvm/arm/mmu.c +3 -0

 	unsigned long mmu_seq;
 	gfn_t gfn = fault_ipa >> PAGE_SHIFT;
 	struct kvm *kvm = vcpu->kvm;

@@ -1598,19 +1599,25 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	if (exec_fault)
 		invalidate_icache_guest_page(pfn, vma_pagesize);
+	/*
+	 * If we took an execution fault we have made the
+	 * icache/dcache coherent above and should now let the s2

Coherent or invalidated with invalidate_icache_guest_page ?

We also do clean_dcache above if needed. So that makes sure the data is coherent. Am I missing something here ?

Quoted 17 lines · Punit Agrawal

Stage 2 fault handler marks a page as executable if it is handling an execution fault or if it was a permission fault in which case the executable bit needs to be preserved.

The logic to decide if the page should be marked executable is duplicated for PMD and PTE entries. To avoid creating another copy when support for PUD hugepages is introduced refactor the code to share the checks needed to mark a page table entry as executable.

Signed-off-by Punit Agrawal <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

---
 virt/kvm/arm/mmu.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

virt/kvm/arm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1479

Quoted 11 lines

virt/kvm/arm/mmu.c +2 -1

index 59595207c5e1..6912529946fb 100644
@@ -1475,7 +1475,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 			  unsigned long fault_status)
 {
 	int ret;
-	bool write_fault, exec_fault, writable, force_pte = false;
+	bool write_fault, writable, force_pte = false;
+	bool exec_fault, needs_exec;

New line not required, still within 80 characters.

Introduce helpers to abstract architectural handling of the conversion of pfn to page table entries and marking a PMD page table entry as a block entry.

The helpers are introduced in preparation for supporting PUD hugepages at stage 2 - which are supported on arm64 but do not exist on arm.

Quoted 3 lines · Punit Agrawal

Introduce helpers to abstract architectural handling of the conversion of pfn to page table entries and marking a PMD page table entry as a block entry.

Why is this necessary ? we would still need to access PMD, PUD as is without any conversion. IOW KVM knows the breakdown of the page table at various levels. Is this something required from generic KVM code ?

Quoted 4 lines

The helpers are introduced in preparation for supporting PUD hugepages at stage 2 - which are supported on arm64 but do not exist on arm.

Some of these patches (including the earlier two) are good on it's own. Do we have still mention in commit message about the incoming PUD enablement as the reason for these cleanup patches ?

Quoted 3 lines · Punit Agrawal

Introduce helpers to abstract architectural handling of the conversion of pfn to page table entries and marking a PMD page table entry as a block entry.

Why is this necessary ? we would still need to access PMD, PUD as is without any conversion. IOW KVM knows the breakdown of the page table at various levels. Is this something required from generic KVM code ?

Quoted 1 line

Quoted 2 lines

The helpers are introduced in preparation for supporting PUD hugepages at stage 2 - which are supported on arm64 but do not exist on arm.

Some of these patches (including the earlier two) are good on it's own. Do we have still mention in commit message about the incoming PUD enablement as the reason for these cleanup patches ?

In preparation for creating PUD hugepages at stage 2, add support for write protecting PUD hugepages when they are encountered. Write protecting guest tables is used to track dirty pages when migrating VMs.

Also, provide trivial implementations of required kvm_s2pud_* helpers to allow sharing of code with arm32.

Signed-off-by Punit Agrawal <[email protected]>

Reviewed-by Christoffer Dall <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Russell King <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

---
 arch/arm/include/asm/kvm_mmu.h   | 15 +++++++++++++++
 arch/arm64/include/asm/kvm_mmu.h | 10 ++++++++++
 virt/kvm/arm/mmu.c               | 11 +++++++----
 3 files changed, 32 insertions(+), 4 deletions(-)

arch/arm/include/asm/kvm_mmu.h +4 -0

index e6eff8bf5d7f..37bf85d39607 100644
@@ -87,6 +87,21 @@ void kvm_clear_hyp_idmap(void);
 
 #define kvm_pmd_mkhuge(pmd)	pmd_mkhuge(pmd)
 
+/*
+ * The following kvm_*pud*() functions are provided strictly to allow
+ * sharing code with arm64. They should never be called in practice.
+ */

arch/arm/include/asm/kvm_mmu.h +1 -0

+
 static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
 {
 	pte_val(pte) |= L_PTE_S2_RDWR;

arch/arm64/include/asm/kvm_mmu.h +10 -0

index 13d482710292..8da6d1b2a196 100644
@@ -251,6 +251,16 @@ static inline bool kvm_s2pmd_exec(pmd_t *pmdp)
 	return !(READ_ONCE(pmd_val(*pmdp)) & PMD_S2_XN);
 }
 
+static inline void kvm_set_s2pud_readonly(pud_t *pudp)
+{
+	kvm_set_s2pte_readonly((pte_t *)pudp);
+}
+
+static inline bool kvm_s2pud_readonly(pud_t *pudp)
+{
+	return kvm_s2pte_readonly((pte_t *)pudp);
+}
+
 #define hyp_pte_table_empty(ptep) kvm_page_empty(ptep)
 
 #ifdef __PAGETABLE_PMD_FOLDED

virt/kvm/arm/mmu.c +0 -0

index fb5325f7a1ac..1c669c3c1208 100644
@@ -1347,9 +1347,12 @@ static void  stage2_wp_puds(struct kvm *kvm, pgd_t *pgd,
 	do {
 		next = stage2_pud_addr_end(kvm, addr, end);

Quoted 33 lines · Punit Agrawal

In preparation for creating PUD hugepages at stage 2, add support for write protecting PUD hugepages when they are encountered. Write protecting guest tables is used to track dirty pages when migrating VMs.

Also, provide trivial implementations of required kvm_s2pud_* helpers to allow sharing of code with arm32.

Signed-off-by Punit Agrawal <[email protected]>

Reviewed-by Christoffer Dall <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Russell King <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

---
  arch/arm/include/asm/kvm_mmu.h   | 15 +++++++++++++++
  arch/arm64/include/asm/kvm_mmu.h | 10 ++++++++++
  virt/kvm/arm/mmu.c               | 11 +++++++----
  3 files changed, 32 insertions(+), 4 deletions(-)

arch/arm/include/asm/kvm_mmu.h +4 -0

index e6eff8bf5d7f..37bf85d39607 100644
@@ -87,6 +87,21 @@ void kvm_clear_hyp_idmap(void);
 
  #define kvm_pmd_mkhuge(pmd)	pmd_mkhuge(pmd)
 
+/*
+ * The following kvm_*pud*() functions are provided strictly to allow
+ * sharing code with arm64. They should never be called in practice.
+ */

arch/arm/include/asm/kvm_mmu.h void kvm_clear_hyp_idmap(void); · line 103

Quoted 10 lines

arch/arm/include/asm/kvm_mmu.h +10 -0

+static inline void kvm_set_s2pud_readonly(pud_t *pud)
+{
+	BUG();
+}
+
+static inline bool kvm_s2pud_readonly(pud_t *pud)
+{
+	BUG();
+	return false;
+}

As arm32 does not support direct manipulation of PUD entries.

Quoted 39 lines

arch/arm/include/asm/kvm_mmu.h +1 -0

+
  static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
  {
  	pte_val(pte) |= L_PTE_S2_RDWR;

arch/arm64/include/asm/kvm_mmu.h +10 -0

index 13d482710292..8da6d1b2a196 100644
@@ -251,6 +251,16 @@ static inline bool kvm_s2pmd_exec(pmd_t *pmdp)
  	return !(READ_ONCE(pmd_val(*pmdp)) & PMD_S2_XN);
  }
 
+static inline void kvm_set_s2pud_readonly(pud_t *pudp)
+{
+	kvm_set_s2pte_readonly((pte_t *)pudp);
+}
+
+static inline bool kvm_s2pud_readonly(pud_t *pudp)
+{
+	return kvm_s2pte_readonly((pte_t *)pudp);
+}
+
  #define hyp_pte_table_empty(ptep) kvm_page_empty(ptep)
 
  #ifdef __PAGETABLE_PMD_FOLDED

virt/kvm/arm/mmu.c +3 -3

index fb5325f7a1ac..1c669c3c1208 100644
@@ -1347,9 +1347,12 @@ static void  stage2_wp_puds(struct kvm *kvm, pgd_t *pgd,
  	do {
  		next = stage2_pud_addr_end(kvm, addr, end);
  		if (!stage2_pud_none(kvm, *pud)) {
-			/* TODO:PUD not supported, revisit later if supported */
-			BUG_ON(stage2_pud_huge(kvm, *pud));
-			stage2_wp_pmds(kvm, pud, addr, next);
+			if (stage2_pud_huge(kvm, *pud)) {
+				if (!kvm_s2pud_readonly(pud))
+					kvm_set_s2pud_readonly(pud);

virt/kvm/arm/mmu.c static void stage2_wp_puds(struct kvm *kvm, pgd_t *pgd, · line 1355

Quoted 3 lines

virt/kvm/arm/mmu.c +3 -0

+			} else {
+				stage2_wp_pmds(kvm, pud, addr, next);
+			}

As this series is enabling PUD related changes in multiple places, it is reasonable to enable PGD level support as well even if it might not be used much at the moment. I dont see much extra code to enable PGD, then why not ? Even just to make the HugeTLB support matrix complete.

In preparation for creating PUD hugepages at stage 2, add support for detecting execute permissions on PUD page table entries. Faults due to lack of execute permissions on page table entries is used to perform i-cache invalidation on first execute.

Provide trivial implementations of arm32 helpers to allow sharing of code.

Signed-off-by Punit Agrawal <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Russell King <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

---
 arch/arm/include/asm/kvm_mmu.h         |  6 +++
 arch/arm64/include/asm/kvm_mmu.h       |  5 +++
 arch/arm64/include/asm/pgtable-hwdef.h |  2 +
 virt/kvm/arm/mmu.c                     | 53 +++++++++++++++++++++++---
 4 files changed, 61 insertions(+), 5 deletions(-)

arch/arm/include/asm/kvm_mmu.h +0 -0

index 37bf85d39607..839a619873d3 100644

arch/arm/include/asm/kvm_mmu.h +3 -0

+	return false;
+}
+
 static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
 {
 	pte_val(pte) |= L_PTE_S2_RDWR;

arch/arm64/include/asm/kvm_mmu.h +5 -0

index 8da6d1b2a196..c755b37b3f92 100644
@@ -261,6 +261,11 @@ static inline bool kvm_s2pud_readonly(pud_t *pudp)
 	return kvm_s2pte_readonly((pte_t *)pudp);
 }
 
+static inline bool kvm_s2pud_exec(pud_t *pudp)
+{
+	return !(READ_ONCE(pud_val(*pudp)) & PUD_S2_XN);
+}
+
 #define hyp_pte_table_empty(ptep) kvm_page_empty(ptep)
 
 #ifdef __PAGETABLE_PMD_FOLDED

arch/arm64/include/asm/pgtable-hwdef.h +2 -0

index 1d7d8da2ef9b..336e24cddc87 100644
@@ -193,6 +193,8 @@
 #define PMD_S2_RDWR		(_AT(pmdval_t, 3) << 6)   /* HAP[2:1] */
 #define PMD_S2_XN		(_AT(pmdval_t, 2) << 53)  /* XN[1:0] */
 
+#define PUD_S2_XN		(_AT(pudval_t, 2) << 53)  /* XN[1:0] */
+
 /*
  * Memory Attribute override for Stage-2 (MemAttr[3:0])
  */

virt/kvm/arm/mmu.c +0 -0

index 1c669c3c1208..8e44dccd1b47 100644
@@ -1083,23 +1083,66 @@ static int stage2_set_pmd_huge(struct kvm *kvm, struct kvm_mmu_memory_cache
 	return 0;
 }

virt/kvm/arm/mmu.c +40 -4

 {
+	pud_t *pudp;
 	pmd_t *pmdp;
 	pte_t *ptep;
 
-	pmdp = stage2_get_pmd(kvm, NULL, addr);
+	*pudpp = NULL;
+	*pmdpp = NULL;
+	*ptepp = NULL;
+
+	pudp = stage2_get_pud(kvm, NULL, addr);
+	if (!pudp || stage2_pud_none(kvm, *pudp) || !stage2_pud_present(kvm, *pudp))
+		return false;
+
+	if (stage2_pud_huge(kvm, *pudp)) {
+		*pudpp = pudp;
+		return true;
+	}
+
+	pmdp = stage2_pmd_offset(kvm, pudp, addr);
 	if (!pmdp || pmd_none(*pmdp) || !pmd_present(*pmdp))
 		return false;
 
-	if (pmd_thp_or_huge(*pmdp))
-		return kvm_s2pmd_exec(pmdp);
+	if (pmd_thp_or_huge(*pmdp)) {
+		*pmdpp = pmdp;
+		return true;
+	}
 
 	ptep = pte_offset_kernel(pmdp, addr);
 	if (!ptep || pte_none(*ptep) || !pte_present(*ptep))
 		return false;
 
-	return kvm_s2pte_exec(ptep);
+	*ptepp = ptep;
+	return true;
+}
+
+static bool stage2_is_exec(struct kvm *kvm, phys_addr_t addr)
+{
+	pud_t *pudp;
+	pmd_t *pmdp;
+	pte_t *ptep;
+	bool found;
+
+	found = stage2_get_leaf_entry(kvm, addr, &pudp, &pmdp, &ptep);
+	if (!found)
+		return false;
+
+	if (pudp)
+		return kvm_s2pud_exec(pudp);
+	else if (pmdp)
+		return kvm_s2pmd_exec(pmdp);
+	else
+		return kvm_s2pte_exec(ptep);
 }
 
 static int stage2_set_pte(struct kvm *kvm, struct kvm_mmu_memory_cache *cache,

Signature · 1 line

2.19.1

Quoted 27 lines · Punit Agrawal

In preparation for creating PUD hugepages at stage 2, add support for detecting execute permissions on PUD page table entries. Faults due to lack of execute permissions on page table entries is used to perform i-cache invalidation on first execute.

Provide trivial implementations of arm32 helpers to allow sharing of code.

Signed-off-by Punit Agrawal <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Russell King <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

---
  arch/arm/include/asm/kvm_mmu.h         |  6 +++
  arch/arm64/include/asm/kvm_mmu.h       |  5 +++
  arch/arm64/include/asm/pgtable-hwdef.h |  2 +
  virt/kvm/arm/mmu.c                     | 53 +++++++++++++++++++++++---
  4 files changed, 61 insertions(+), 5 deletions(-)

arch/arm/include/asm/kvm_mmu.h +0 -0

index 37bf85d39607..839a619873d3 100644

arch/arm/include/asm/kvm_mmu.h static inline bool kvm_s2pud_readonly(pud_t *pud) · line 107

Quoted 6 lines

arch/arm/include/asm/kvm_mmu.h +3 -0

@@ -102,6 +102,12 @@ static inline bool kvm_s2pud_readonly(pud_t *pud)
  	return false;
  }
 
+static inline bool kvm_s2pud_exec(pud_t *pud)
+{
+	BUG();

nit: I think this should be WARN() now :)

Quoted 42 lines

arch/arm/include/asm/kvm_mmu.h +3 -0

+	return false;
+}
+
  static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
  {
  	pte_val(pte) |= L_PTE_S2_RDWR;

arch/arm64/include/asm/kvm_mmu.h +5 -0

index 8da6d1b2a196..c755b37b3f92 100644
@@ -261,6 +261,11 @@ static inline bool kvm_s2pud_readonly(pud_t *pudp)
  	return kvm_s2pte_readonly((pte_t *)pudp);
  }
 
+static inline bool kvm_s2pud_exec(pud_t *pudp)
+{
+	return !(READ_ONCE(pud_val(*pudp)) & PUD_S2_XN);
+}
+
  #define hyp_pte_table_empty(ptep) kvm_page_empty(ptep)
 
  #ifdef __PAGETABLE_PMD_FOLDED

arch/arm64/include/asm/pgtable-hwdef.h +2 -0

index 1d7d8da2ef9b..336e24cddc87 100644
@@ -193,6 +193,8 @@
  #define PMD_S2_RDWR		(_AT(pmdval_t, 3) << 6)   /* HAP[2:1] */
  #define PMD_S2_XN		(_AT(pmdval_t, 2) << 53)  /* XN[1:0] */
 
+#define PUD_S2_XN		(_AT(pudval_t, 2) << 53)  /* XN[1:0] */
+
  /*
   * Memory Attribute override for Stage-2 (MemAttr[3:0])
   */

virt/kvm/arm/mmu.c +0 -0

index 1c669c3c1208..8e44dccd1b47 100644
@@ -1083,23 +1083,66 @@ static int stage2_set_pmd_huge(struct kvm *kvm, struct kvm_mmu_memory_cache
  	return 0;
  }

virt/kvm/arm/mmu.c static int stage2_set_pmd_huge(struct kvm *kvm, struct kvm_mmu_memory_cache · line 1093

Quoted 10 lines

virt/kvm/arm/mmu.c +8 -1

 
-static bool stage2_is_exec(struct kvm *kvm, phys_addr_t addr)
+/*
+ * stage2_get_leaf_entry - walk the stage2 VM page tables and return
+ * true if a valid and present leaf-entry is found. A pointer to the
+ * leaf-entry is returned in the appropriate level variable - pudpp,
+ * pmdpp, ptepp.
+ */
+static bool stage2_get_leaf_entry(struct kvm *kvm, phys_addr_t addr,
+				  pud_t **pudpp, pmd_t **pmdpp, pte_t **ptepp)

Do we need this type madness or could this just return a u64 pointer (NULL if nothing is found) and pass that to kvm_s2pte_exec (because we know it's the same bit we need to check regardless of the pgtable level on both arm and arm64)?

Or do we consider that bad for some reason?

Quoted 90 lines · Christoffer Dall

Quoted 27 lines · Punit Agrawal

In preparation for creating PUD hugepages at stage 2, add support for detecting execute permissions on PUD page table entries. Faults due to lack of execute permissions on page table entries is used to perform i-cache invalidation on first execute.

Provide trivial implementations of arm32 helpers to allow sharing of code.

Signed-off-by Punit Agrawal <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Russell King <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

---
 arch/arm/include/asm/kvm_mmu.h         |  6 +++
 arch/arm64/include/asm/kvm_mmu.h       |  5 +++
 arch/arm64/include/asm/pgtable-hwdef.h |  2 +
 virt/kvm/arm/mmu.c                     | 53 +++++++++++++++++++++++---
 4 files changed, 61 insertions(+), 5 deletions(-)

arch/arm/include/asm/kvm_mmu.h +0 -0

index 37bf85d39607..839a619873d3 100644

arch/arm/include/asm/kvm_mmu.h static inline bool kvm_s2pud_readonly(pud_t *pud) · line 106

Quoted 5 lines

arch/arm/include/asm/kvm_mmu.h +3 -0

@@ -102,6 +102,12 @@ static inline bool kvm_s2pud_readonly(pud_t *pud)
 	return false;
 }
+static inline bool kvm_s2pud_exec(pud_t *pud)
+{
+	BUG();

nit: I think this should be WARN() now :)

Quoted 38 lines

arch/arm/include/asm/kvm_mmu.h +3 -0

+	return false;
+}
+
 static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
 {
 	pte_val(pte) |= L_PTE_S2_RDWR;

arch/arm64/include/asm/kvm_mmu.h +5 -0

index 8da6d1b2a196..c755b37b3f92 100644
@@ -261,6 +261,11 @@ static inline bool kvm_s2pud_readonly(pud_t *pudp)
 	return kvm_s2pte_readonly((pte_t *)pudp);
 }
+static inline bool kvm_s2pud_exec(pud_t *pudp)
+{
+	return !(READ_ONCE(pud_val(*pudp)) & PUD_S2_XN);
+}
+
 #define hyp_pte_table_empty(ptep) kvm_page_empty(ptep)
 #ifdef __PAGETABLE_PMD_FOLDED

arch/arm64/include/asm/pgtable-hwdef.h +2 -0

index 1d7d8da2ef9b..336e24cddc87 100644
@@ -193,6 +193,8 @@
 #define PMD_S2_RDWR		(_AT(pmdval_t, 3) << 6)   /* HAP[2:1] */
 #define PMD_S2_XN		(_AT(pmdval_t, 2) << 53)  /* XN[1:0] */
+#define PUD_S2_XN		(_AT(pudval_t, 2) << 53)  /* XN[1:0] */
+
 /*
  * Memory Attribute override for Stage-2 (MemAttr[3:0])
  */

virt/kvm/arm/mmu.c +0 -0

index 1c669c3c1208..8e44dccd1b47 100644
@@ -1083,23 +1083,66 @@ static int stage2_set_pmd_huge(struct kvm *kvm, struct kvm_mmu_memory_cache
 	return 0;

virt/kvm/arm/mmu.c static int stage2_set_pmd_huge(struct kvm *kvm, struct kvm_mmu_memory_cache · line 1092

Quoted 10 lines

virt/kvm/arm/mmu.c +8 -1

 }
-static bool stage2_is_exec(struct kvm *kvm, phys_addr_t addr)
+/*
+ * stage2_get_leaf_entry - walk the stage2 VM page tables and return
+ * true if a valid and present leaf-entry is found. A pointer to the
+ * leaf-entry is returned in the appropriate level variable - pudpp,
+ * pmdpp, ptepp.
+ */
+static bool stage2_get_leaf_entry(struct kvm *kvm, phys_addr_t addr,
+				  pud_t **pudpp, pmd_t **pmdpp, pte_t **ptepp)

Do we need this type madness or could this just return a u64 pointer (NULL if nothing is found) and pass that to kvm_s2pte_exec (because we know it's the same bit we need to check regardless of the pgtable level on both arm and arm64)?

Quoted 1 line

Or do we consider that bad for some reason?

Practically, yes the bit positions are same and thus we should be able to do this assuming that it is just a pte. When we get to independent stage2 pgtable implementation which treats all page table entries as a single type with a level information, we should be able to get rid of these. But since we have followed the Linux way of page-table manipulation where we have "level" specific accessors. The other option is open code the walking sequence from the pgd to the leaf entry everywhere.

I am fine with changing this code, if you like.

In preparation for creating PUD hugepages at stage 2, add support for detecting execute permissions on PUD page table entries. Faults due to lack of execute permissions on page table entries is used to perform i-cache invalidation on first execute.

Provide trivial implementations of arm32 helpers to allow sharing of code.

Signed-off-by Punit Agrawal <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Russell King <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

---
 arch/arm/include/asm/kvm_mmu.h         |  6 +++
 arch/arm64/include/asm/kvm_mmu.h       |  5 +++
 arch/arm64/include/asm/pgtable-hwdef.h |  2 +
 virt/kvm/arm/mmu.c                     | 53 +++++++++++++++++++++++---
 4 files changed, 61 insertions(+), 5 deletions(-)

arch/arm/include/asm/kvm_mmu.h +6 -0

index 37bf85d39607..839a619873d3 100644
@@ -102,6 +102,12 @@ static inline bool kvm_s2pud_readonly(pud_t *pud)
 	return false;
 }
 
+static inline bool kvm_s2pud_exec(pud_t *pud)
+{
+	BUG();
+	return false;
+}
+
 static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
 {
 	pte_val(pte) |= L_PTE_S2_RDWR;

arch/arm64/include/asm/kvm_mmu.h +5 -0

index 8da6d1b2a196..c755b37b3f92 100644
@@ -261,6 +261,11 @@ static inline bool kvm_s2pud_readonly(pud_t *pudp)
 	return kvm_s2pte_readonly((pte_t *)pudp);
 }
 
+static inline bool kvm_s2pud_exec(pud_t *pudp)
+{
+	return !(READ_ONCE(pud_val(*pudp)) & PUD_S2_XN);
+}
+
 #define hyp_pte_table_empty(ptep) kvm_page_empty(ptep)
 
 #ifdef __PAGETABLE_PMD_FOLDED

arch/arm64/include/asm/pgtable-hwdef.h +2 -0

index 1d7d8da2ef9b..336e24cddc87 100644
@@ -193,6 +193,8 @@
 #define PMD_S2_RDWR		(_AT(pmdval_t, 3) << 6)   /* HAP[2:1] */
 #define PMD_S2_XN		(_AT(pmdval_t, 2) << 53)  /* XN[1:0] */
 
+#define PUD_S2_XN		(_AT(pudval_t, 2) << 53)  /* XN[1:0] */
+
 /*
  * Memory Attribute override for Stage-2 (MemAttr[3:0])
  */

virt/kvm/arm/mmu.c +39 -5

index 1c669c3c1208..8e44dccd1b47 100644
@@ -1083,23 +1083,66 @@ static int stage2_set_pmd_huge(struct kvm *kvm, struct kvm_mmu_memory_cache
 	return 0;
 }
 
-static bool stage2_is_exec(struct kvm *kvm, phys_addr_t addr)
+/*
+ * stage2_get_leaf_entry - walk the stage2 VM page tables and return
+ * true if a valid and present leaf-entry is found. A pointer to the
+ * leaf-entry is returned in the appropriate level variable - pudpp,
+ * pmdpp, ptepp.
+ */
+static bool stage2_get_leaf_entry(struct kvm *kvm, phys_addr_t addr,
+				  pud_t **pudpp, pmd_t **pmdpp, pte_t **ptepp)
 {
+	pud_t *pudp;
 	pmd_t *pmdp;
 	pte_t *ptep;
 
-	pmdp = stage2_get_pmd(kvm, NULL, addr);
+	*pudpp = NULL;
+	*pmdpp = NULL;
+	*ptepp = NULL;
+
+	pudp = stage2_get_pud(kvm, NULL, addr);
+	if (!pudp || stage2_pud_none(kvm, *pudp) || !stage2_pud_present(kvm, *pudp))
+		return false;
+
+	if (stage2_pud_huge(kvm, *pudp)) {
+		*pudpp = pudp;
+		return true;
+	}
+
+	pmdp = stage2_pmd_offset(kvm, pudp, addr);
 	if (!pmdp || pmd_none(*pmdp) || !pmd_present(*pmdp))
 		return false;
 
-	if (pmd_thp_or_huge(*pmdp))
-		return kvm_s2pmd_exec(pmdp);
+	if (pmd_thp_or_huge(*pmdp)) {
+		*pmdpp = pmdp;
+		return true;
+	}
 
 	ptep = pte_offset_kernel(pmdp, addr);
 	if (!ptep || pte_none(*ptep) || !pte_present(*ptep))
 		return false;
 
-	return kvm_s2pte_exec(ptep);
+	*ptepp = ptep;
+	return true;
+}
+
+static bool stage2_is_exec(struct kvm *kvm, phys_addr_t addr)
+{
+	pud_t *pudp;
+	pmd_t *pmdp;
+	pte_t *ptep;
+	bool found;
+
+	found = stage2_get_leaf_entry(kvm, addr, &pudp, &pmdp, &ptep);

In preparation for creating larger hugepages at Stage 2, extend the access fault handling at Stage 2 to support PUD hugepages when encountered.

Provide trivial helpers for arm32 to allow sharing of code.

Signed-off-by Punit Agrawal <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Russell King <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

---
 arch/arm/include/asm/kvm_mmu.h   |  9 +++++++++
 arch/arm64/include/asm/kvm_mmu.h |  7 +++++++
 arch/arm64/include/asm/pgtable.h |  6 ++++++
 virt/kvm/arm/mmu.c               | 22 +++++++++++-----------
 4 files changed, 33 insertions(+), 11 deletions(-)

arch/arm/include/asm/kvm_mmu.h +3 -0

index 839a619873d3..fea5e723e3ac 100644
@@ -85,6 +85,9 @@ void kvm_clear_hyp_idmap(void);
 #define kvm_pfn_pte(pfn, prot)	pfn_pte(pfn, prot)
 #define kvm_pfn_pmd(pfn, prot)	pfn_pmd(pfn, prot)
 
+#define kvm_pud_pfn(pud)	({ BUG(); 0; })
+
+
 #define kvm_pmd_mkhuge(pmd)	pmd_mkhuge(pmd)
 
 /*

arch/arm/include/asm/kvm_mmu.h +3 -0

+	return pud;
+}
+
 static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
 {
 	pte_val(pte) |= L_PTE_S2_RDWR;

arch/arm64/include/asm/kvm_mmu.h +7 -0

index c755b37b3f92..612032bbb428 100644
@@ -187,6 +187,8 @@ void kvm_clear_hyp_idmap(void);
 #define kvm_pfn_pte(pfn, prot)		pfn_pte(pfn, prot)
 #define kvm_pfn_pmd(pfn, prot)		pfn_pmd(pfn, prot)
 
+#define kvm_pud_pfn(pud)		pud_pfn(pud)
+
 #define kvm_pmd_mkhuge(pmd)		pmd_mkhuge(pmd)
 
 static inline pte_t kvm_s2pte_mkwrite(pte_t pte)

@@ -266,6 +268,11 @@ static inline bool kvm_s2pud_exec(pud_t *pudp)
 	return !(READ_ONCE(pud_val(*pudp)) & PUD_S2_XN);
 }
 
+static inline pud_t kvm_s2pud_mkyoung(pud_t pud)
+{
+	return pud_mkyoung(pud);
+}
+
 #define hyp_pte_table_empty(ptep) kvm_page_empty(ptep)
 
 #ifdef __PAGETABLE_PMD_FOLDED

arch/arm64/include/asm/pgtable.h +6 -0

index 50b1ef8584c0..f51e2271e6a3 100644
@@ -314,6 +314,11 @@ static inline pte_t pud_pte(pud_t pud)
 	return __pte(pud_val(pud));
 }
 
+static inline pud_t pte_pud(pte_t pte)
+{
+	return __pud(pte_val(pte));
+}
+
 static inline pmd_t pud_pmd(pud_t pud)
 {
 	return __pmd(pud_val(pud));

@@ -381,6 +386,7 @@ static inline int pmd_protnone(pmd_t pmd)
 #define pfn_pmd(pfn,prot)	__pmd(__phys_to_pmd_val((phys_addr_t)(pfn) << PAGE_SHIFT) | pgprot_val(prot))
 #define mk_pmd(page,prot)	pfn_pmd(page_to_pfn(page),prot)
 
+#define pud_mkyoung(pud)	pte_pud(pte_mkyoung(pud_pte(pud)))
 #define pud_write(pud)		pte_write(pud_pte(pud))
 
 #define __pud_to_phys(pud)	__pte_to_phys(pud_pte(pud))

virt/kvm/arm/mmu.c +11 -11

index 8e44dccd1b47..bd749601195f 100644
@@ -1698,6 +1698,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
  */
 static void handle_access_fault(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa)
 {
+	pud_t *pud;
 	pmd_t *pmd;
 	pte_t *pte;
 	kvm_pfn_t pfn;

@@ -1707,24 +1708,23 @@ static void handle_access_fault(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa)
 
 	spin_lock(&vcpu->kvm->mmu_lock);
 
-	pmd = stage2_get_pmd(vcpu->kvm, NULL, fault_ipa);
-	if (!pmd || pmd_none(*pmd))	/* Nothing there */
+	if (!stage2_get_leaf_entry(vcpu->kvm, fault_ipa, &pud, &pmd, &pte))
 		goto out;
 
-	if (pmd_thp_or_huge(*pmd)) {	/* THP, HugeTLB */
+	if (pud) {		/* HugeTLB */
+		*pud = kvm_s2pud_mkyoung(*pud);
+		pfn = kvm_pud_pfn(*pud);
+		pfn_valid = true;
+	} else	if (pmd) {	/* THP, HugeTLB */
 		*pmd = pmd_mkyoung(*pmd);
 		pfn = pmd_pfn(*pmd);
 		pfn_valid = true;
-		goto out;
+	} else {
+		*pte = pte_mkyoung(*pte);	/* Just a page... */
+		pfn = pte_pfn(*pte);
+		pfn_valid = true;
 	}
 
-	pte = pte_offset_kernel(pmd, fault_ipa);
-	if (pte_none(*pte))		/* Nothing there either */
-		goto out;
-
-	*pte = pte_mkyoung(*pte);	/* Just a page... */
-	pfn = pte_pfn(*pte);
-	pfn_valid = true;
 out:
 	spin_unlock(&vcpu->kvm->mmu_lock);
 	if (pfn_valid)

Signature · 1 line

2.19.1

In preparation for creating larger hugepages at Stage 2, extend the access fault handling at Stage 2 to support PUD hugepages when encountered.

Provide trivial helpers for arm32 to allow sharing of code.

Signed-off-by Punit Agrawal <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Russell King <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

---
 arch/arm/include/asm/kvm_mmu.h   |  9 +++++++++
 arch/arm64/include/asm/kvm_mmu.h |  7 +++++++
 arch/arm64/include/asm/pgtable.h |  6 ++++++
 virt/kvm/arm/mmu.c               | 22 +++++++++++-----------
 4 files changed, 33 insertions(+), 11 deletions(-)

arch/arm/include/asm/kvm_mmu.h +9 -0

index 839a619873d3..fea5e723e3ac 100644
@@ -85,6 +85,9 @@ void kvm_clear_hyp_idmap(void);
 #define kvm_pfn_pte(pfn, prot)	pfn_pte(pfn, prot)
 #define kvm_pfn_pmd(pfn, prot)	pfn_pmd(pfn, prot)
 
+#define kvm_pud_pfn(pud)	({ BUG(); 0; })
+
+
 #define kvm_pmd_mkhuge(pmd)	pmd_mkhuge(pmd)
 
 /*

@@ -108,6 +111,12 @@ static inline bool kvm_s2pud_exec(pud_t *pud)
 	return false;
 }
 
+static inline pud_t kvm_s2pud_mkyoung(pud_t pud)
+{
+	BUG();
+	return pud;
+}
+
 static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
 {
 	pte_val(pte) |= L_PTE_S2_RDWR;

arch/arm64/include/asm/kvm_mmu.h +7 -0

index c755b37b3f92..612032bbb428 100644
@@ -187,6 +187,8 @@ void kvm_clear_hyp_idmap(void);
 #define kvm_pfn_pte(pfn, prot)		pfn_pte(pfn, prot)
 #define kvm_pfn_pmd(pfn, prot)		pfn_pmd(pfn, prot)
 
+#define kvm_pud_pfn(pud)		pud_pfn(pud)
+
 #define kvm_pmd_mkhuge(pmd)		pmd_mkhuge(pmd)
 
 static inline pte_t kvm_s2pte_mkwrite(pte_t pte)

@@ -266,6 +268,11 @@ static inline bool kvm_s2pud_exec(pud_t *pudp)
 	return !(READ_ONCE(pud_val(*pudp)) & PUD_S2_XN);
 }
 
+static inline pud_t kvm_s2pud_mkyoung(pud_t pud)
+{
+	return pud_mkyoung(pud);
+}
+
 #define hyp_pte_table_empty(ptep) kvm_page_empty(ptep)
 
 #ifdef __PAGETABLE_PMD_FOLDED

arch/arm64/include/asm/pgtable.h +0 -0

index 50b1ef8584c0..f51e2271e6a3 100644

arch/arm64/include/asm/pgtable.h +1 -0

 static inline pmd_t pud_pmd(pud_t pud)
 {
 	return __pmd(pud_val(pud));

@@ -381,6 +386,7 @@ static inline int pmd_protnone(pmd_t pmd)
 #define pfn_pmd(pfn,prot)	__pmd(__phys_to_pmd_val((phys_addr_t)(pfn) << PAGE_SHIFT) | pgprot_val(prot))
 #define mk_pmd(page,prot)	pfn_pmd(page_to_pfn(page),prot)
 
+#define pud_mkyoung(pud)	pte_pud(pte_mkyoung(pud_pte(pud)))
 #define pud_write(pud)		pte_write(pud_pte(pud))
 
 #define __pud_to_phys(pud)	__pte_to_phys(pud_pte(pud))

virt/kvm/arm/mmu.c +6 -3

index 8e44dccd1b47..bd749601195f 100644
@@ -1698,6 +1698,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
  */
 static void handle_access_fault(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa)
 {
+	pud_t *pud;
 	pmd_t *pmd;
 	pte_t *pte;
 	kvm_pfn_t pfn;

@@ -1707,24 +1708,23 @@ static void handle_access_fault(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa)
 
 	spin_lock(&vcpu->kvm->mmu_lock);
 
-	pmd = stage2_get_pmd(vcpu->kvm, NULL, fault_ipa);
-	if (!pmd || pmd_none(*pmd))	/* Nothing there */
+	if (!stage2_get_leaf_entry(vcpu->kvm, fault_ipa, &pud, &pmd, &pte))
 		goto out;
 
-	if (pmd_thp_or_huge(*pmd)) {	/* THP, HugeTLB */
+	if (pud) {		/* HugeTLB */
+		*pud = kvm_s2pud_mkyoung(*pud);
+		pfn = kvm_pud_pfn(*pud);
+		pfn_valid = true;

In preparation for creating larger hugepages at Stage 2, add support to the age handling notifiers for PUD hugepages when encountered.

Provide trivial helpers for arm32 to allow sharing code.

Signed-off-by Punit Agrawal <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Russell King <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

---
 arch/arm/include/asm/kvm_mmu.h   |  6 +++++
 arch/arm64/include/asm/kvm_mmu.h |  5 ++++
 arch/arm64/include/asm/pgtable.h |  1 +
 virt/kvm/arm/mmu.c               | 39 ++++++++++++++++----------------
 4 files changed, 32 insertions(+), 19 deletions(-)

arch/arm/include/asm/kvm_mmu.h +6 -0

index fea5e723e3ac..e62f0913ce7d 100644
@@ -117,6 +117,12 @@ static inline pud_t kvm_s2pud_mkyoung(pud_t pud)
 	return pud;
 }
 
+static inline bool kvm_s2pud_young(pud_t pud)
+{
+	BUG();
+	return false;
+}
+
 static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
 {
 	pte_val(pte) |= L_PTE_S2_RDWR;

arch/arm64/include/asm/kvm_mmu.h +5 -0

index 612032bbb428..9f941f70775c 100644
@@ -273,6 +273,11 @@ static inline pud_t kvm_s2pud_mkyoung(pud_t pud)
 	return pud_mkyoung(pud);
 }
 
+static inline bool kvm_s2pud_young(pud_t pud)
+{
+	return pud_young(pud);
+}
+
 #define hyp_pte_table_empty(ptep) kvm_page_empty(ptep)
 
 #ifdef __PAGETABLE_PMD_FOLDED

arch/arm64/include/asm/pgtable.h +1 -0

index f51e2271e6a3..bb0f3f17a7a9 100644
@@ -386,6 +386,7 @@ static inline int pmd_protnone(pmd_t pmd)
 #define pfn_pmd(pfn,prot)	__pmd(__phys_to_pmd_val((phys_addr_t)(pfn) << PAGE_SHIFT) | pgprot_val(prot))
 #define mk_pmd(page,prot)	pfn_pmd(page_to_pfn(page),prot)
 
+#define pud_young(pud)		pte_young(pud_pte(pud))
 #define pud_mkyoung(pud)	pte_pud(pte_mkyoung(pud_pte(pud)))
 #define pud_write(pud)		pte_write(pud_pte(pud))
 

virt/kvm/arm/mmu.c +19 -14

index bd749601195f..3893ea6a50bf 100644
@@ -1225,6 +1225,11 @@ static int stage2_pmdp_test_and_clear_young(pmd_t *pmd)
 	return stage2_ptep_test_and_clear_young((pte_t *)pmd);
 }
 
+static int stage2_pudp_test_and_clear_young(pud_t *pud)
+{
+	return stage2_ptep_test_and_clear_young((pte_t *)pud);
+}
+
 /**
  * kvm_phys_addr_ioremap - map a device range to guest IPA
  *

@@ -1932,42 +1937,38 @@ void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
 
 static int kvm_age_hva_handler(struct kvm *kvm, gpa_t gpa, u64 size, void *data)
 {
+	pud_t *pud;
 	pmd_t *pmd;
 	pte_t *pte;
 
-	WARN_ON(size != PAGE_SIZE && size != PMD_SIZE);
-	pmd = stage2_get_pmd(kvm, NULL, gpa);
-	if (!pmd || pmd_none(*pmd))	/* Nothing there */
+	WARN_ON(size != PAGE_SIZE && size != PMD_SIZE && size != PUD_SIZE);
+	if (!stage2_get_leaf_entry(kvm, gpa, &pud, &pmd, &pte))
 		return 0;
 
-	if (pmd_thp_or_huge(*pmd))	/* THP, HugeTLB */
+	if (pud)
+		return stage2_pudp_test_and_clear_young(pud);
+	else if (pmd)
 		return stage2_pmdp_test_and_clear_young(pmd);
-
-	pte = pte_offset_kernel(pmd, gpa);
-	if (pte_none(*pte))
-		return 0;
-
-	return stage2_ptep_test_and_clear_young(pte);
+	else
+		return stage2_ptep_test_and_clear_young(pte);
 }
 
 static int kvm_test_age_hva_handler(struct kvm *kvm, gpa_t gpa, u64 size, void *data)
 {
+	pud_t *pud;
 	pmd_t *pmd;
 	pte_t *pte;
 
-	WARN_ON(size != PAGE_SIZE && size != PMD_SIZE);
-	pmd = stage2_get_pmd(kvm, NULL, gpa);
-	if (!pmd || pmd_none(*pmd))	/* Nothing there */
+	WARN_ON(size != PAGE_SIZE && size != PMD_SIZE && size != PUD_SIZE);
+	if (!stage2_get_leaf_entry(kvm, gpa, &pud, &pmd, &pte))
 		return 0;
 
-	if (pmd_thp_or_huge(*pmd))		/* THP, HugeTLB */
+	if (pud)
+		return kvm_s2pud_young(*pud);
+	else if (pmd)
 		return pmd_young(*pmd);

KVM only supports PMD hugepages at stage 2. Now that the various page handling routines are updated, extend the stage 2 fault handling to map in PUD hugepages.

Addition of PUD hugepage support enables additional page sizes (e.g., 1G with 4K granule) which can be useful on cores that support mapping larger block sizes in the TLB entries.

Signed-off-by Punit Agrawal <[email protected]>

Reviewed-by Suzuki Poulose <[email protected]>

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Russell King <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

---
 arch/arm/include/asm/kvm_mmu.h         |  20 +++++
 arch/arm/include/asm/stage2_pgtable.h  |   5 ++
 arch/arm64/include/asm/kvm_mmu.h       |  16 ++++
 arch/arm64/include/asm/pgtable-hwdef.h |   2 +
 arch/arm64/include/asm/pgtable.h       |   2 +
 virt/kvm/arm/mmu.c                     | 104 +++++++++++++++++++++++--
 6 files changed, 143 insertions(+), 6 deletions(-)

arch/arm/include/asm/kvm_mmu.h +0 -0

index e62f0913ce7d..6336319a0d5b 100644

arch/arm/include/asm/kvm_mmu.h +9 -0

 #define kvm_pud_pfn(pud)	({ BUG(); 0; })
 
 
 #define kvm_pmd_mkhuge(pmd)	pmd_mkhuge(pmd)
+/* No support for pud hugepages */
+#define kvm_pud_mkhuge(pud)	( {BUG(); pud; })
 
 /*
  * The following kvm_*pud*() functions are provided strictly to allow

@@ -105,6 +108,23 @@ static inline bool kvm_s2pud_readonly(pud_t *pud)
 	return false;
 }
 
+static inline void kvm_set_pud(pud_t *pud, pud_t new_pud)
+{
+	BUG();
+}
+
+static inline pud_t kvm_s2pud_mkwrite(pud_t pud)
+{

arch/arm/include/asm/kvm_mmu.h +0 -0

 static inline bool kvm_s2pud_exec(pud_t *pud)
 {
 	BUG();

arch/arm/include/asm/stage2_pgtable.h +0 -0

index f6a7ea805232..f9017167a8d1 100644

arch/arm/include/asm/stage2_pgtable.h +0 -0

 #endif	/* __ARM_S2_PGTABLE_H_ */

arch/arm64/include/asm/kvm_mmu.h +10 -0

index 9f941f70775c..8af4b1befa42 100644
@@ -184,12 +184,16 @@ void kvm_clear_hyp_idmap(void);
 #define kvm_mk_pgd(pudp)					\
 	__pgd(__phys_to_pgd_val(__pa(pudp)) | PUD_TYPE_TABLE)
 
+#define kvm_set_pud(pudp, pud)		set_pud(pudp, pud)
+
 #define kvm_pfn_pte(pfn, prot)		pfn_pte(pfn, prot)
 #define kvm_pfn_pmd(pfn, prot)		pfn_pmd(pfn, prot)
+#define kvm_pfn_pud(pfn, prot)		pfn_pud(pfn, prot)
 
 #define kvm_pud_pfn(pud)		pud_pfn(pud)
 
 #define kvm_pmd_mkhuge(pmd)		pmd_mkhuge(pmd)
+#define kvm_pud_mkhuge(pud)		pud_mkhuge(pud)
 
 static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
 {

@@ -203,6 +207,12 @@ static inline pmd_t kvm_s2pmd_mkwrite(pmd_t pmd)
 	return pmd;
 }
 
+static inline pud_t kvm_s2pud_mkwrite(pud_t pud)
+{
+	pud_val(pud) |= PUD_S2_RDWR;
+	return pud;
+}
+
 static inline pte_t kvm_s2pte_mkexec(pte_t pte)
 {
 	pte_val(pte) &= ~PTE_S2_XN;

arch/arm64/include/asm/kvm_mmu.h +0 -0

 static inline void kvm_set_s2pte_readonly(pte_t *ptep)
 {
 	pteval_t old_pteval, pteval;

arch/arm64/include/asm/pgtable-hwdef.h +0 -0

index 336e24cddc87..6f1c187f1c86 100644

arch/arm64/include/asm/pgtable.h +0 -0

 #define __pud_to_phys(pud)	__pte_to_phys(pud_pte(pud))
 #define __phys_to_pud_val(phys)	__phys_to_pte_val(phys)
 #define pud_pfn(pud)		((__pud_to_phys(pud) & PUD_MASK) >> PAGE_SHIFT)

virt/kvm/arm/mmu.c +76 -2

index 3893ea6a50bf..2dcff38868d4 100644
@@ -115,6 +115,25 @@ static void stage2_dissolve_pmd(struct kvm *kvm, phys_addr_t addr, pmd_t *pmd)
 	put_page(virt_to_page(pmd));
 }
 
+/**
+ * stage2_dissolve_pud() - clear and flush huge PUD entry
+ * @kvm:	pointer to kvm structure.
+ * @addr:	IPA
+ * @pud:	pud pointer for IPA
+ *
+ * Function clears a PUD entry, flushes addr 1st and 2nd stage TLBs. Marks all
+ * pages in the range dirty.
+ */
+static void stage2_dissolve_pud(struct kvm *kvm, phys_addr_t addr, pud_t *pudp)
+{
+	if (!stage2_pud_huge(kvm, *pudp))
+		return;
+
+	stage2_pud_clear(kvm, pudp);
+	kvm_tlb_flush_vmid_ipa(kvm, addr);
+	put_page(virt_to_page(pudp));
+}
+
 static int mmu_topup_memory_cache(struct kvm_mmu_memory_cache *cache,
 				  int min, int max)
 {

@@ -1022,7 +1041,7 @@ static pmd_t *stage2_get_pmd(struct kvm *kvm, struct kvm_mmu_memory_cache *cache
 	pmd_t *pmd;
 
 	pud = stage2_get_pud(kvm, cache, addr);
-	if (!pud)
+	if (!pud || stage2_pud_huge(kvm, *pud))
 		return NULL;
 
 	if (stage2_pud_none(kvm, *pud)) {

@@ -1083,6 +1102,36 @@ static int stage2_set_pmd_huge(struct kvm *kvm, struct kvm_mmu_memory_cache
 	return 0;
 }
 
+static int stage2_set_pud_huge(struct kvm *kvm, struct kvm_mmu_memory_cache *cache,
+			       phys_addr_t addr, const pud_t *new_pudp)
+{
+	pud_t *pudp, old_pud;
+
+	pudp = stage2_get_pud(kvm, cache, addr);
+	VM_BUG_ON(!pudp);
+
+	old_pud = *pudp;
+
+	/*
+	 * A large number of vcpus faulting on the same stage 2 entry,
+	 * can lead to a refault due to the
+	 * stage2_pud_clear()/tlb_flush(). Skip updating the page
+	 * tables if there is no change.
+	 */
+	if (pud_val(old_pud) == pud_val(*new_pudp))
+		return 0;
+
+	if (stage2_pud_present(kvm, old_pud)) {
+		stage2_pud_clear(kvm, pudp);
+		kvm_tlb_flush_vmid_ipa(kvm, addr);
+	} else {
+		get_page(virt_to_page(pudp));
+	}
+
+	kvm_set_pud(pudp, *new_pudp);
+	return 0;
+}
+
 /*
  * stage2_get_leaf_entry - walk the stage2 VM page tables and return
  * true if a valid and present leaf-entry is found. A pointer to the

@@ -1149,6 +1198,7 @@ static int stage2_set_pte(struct kvm *kvm, struct kvm_mmu_memory_cache *cache,
 			  phys_addr_t addr, const pte_t *new_pte,
 			  unsigned long flags)
 {
+	pud_t *pud;
 	pmd_t *pmd;
 	pte_t *pte, old_pte;
 	bool iomap = flags & KVM_S2PTE_FLAG_IS_IOMAP;

@@ -1157,7 +1207,31 @@ static int stage2_set_pte(struct kvm *kvm, struct kvm_mmu_memory_cache *cache,
 	VM_BUG_ON(logging_active && !cache);
 
 	/* Create stage-2 page table mapping - Levels 0 and 1 */
-	pmd = stage2_get_pmd(kvm, cache, addr);
+	pud = stage2_get_pud(kvm, cache, addr);
+	if (!pud) {
+		/*
+		 * Ignore calls from kvm_set_spte_hva for unallocated
+		 * address ranges.
+		 */
+		return 0;
+	}
+
+	/*
+	 * While dirty page logging - dissolve huge PUD, then continue
+	 * on to allocate page.
+	 */
+	if (logging_active)
+		stage2_dissolve_pud(kvm, addr, pud);
+
+	if (stage2_pud_none(kvm, *pud)) {
+		if (!cache)
+			return 0; /* ignore calls from kvm_set_spte_hva */
+		pmd = mmu_memory_cache_alloc(cache);
+		stage2_pud_populate(kvm, pud, pmd);
+		get_page(virt_to_page(pud));
+	}
+
+	pmd = stage2_pmd_offset(kvm, pud, addr);
 	if (!pmd) {
 		/*
 		 * Ignore calls from kvm_set_spte_hva for unallocated

@@ -1557,12 +1631,19 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	}