[RFC 2/4] KVM: arm64: Support dirty page tracking for PUD hugepages

Christoffer Dall [off-list ref] writes:

On Wed, Jan 10, 2018 at 07:07:27PM +0000, Punit Agrawal wrote:

quoted

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 code to compile on arm32.

Signed-off-by: Punit Agrawal <redacted>
Cc: Christoffer Dall <redacted>
Cc: Marc Zyngier <redacted>
---
 arch/arm/include/asm/kvm_mmu.h   |  9 +++++++++
 arch/arm64/include/asm/kvm_mmu.h | 10 ++++++++++
 virt/kvm/arm/mmu.c               |  9 ++++++---
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
index fa6f2174276b..3fbe919b9181 100644
--- a/arch/arm/include/asm/kvm_mmu.h
+++ b/arch/arm/include/asm/kvm_mmu.h

@@ -103,6 +103,15 @@ static inline bool kvm_s2pmd_readonly(pmd_t *pmd)
 	return (pmd_val(*pmd) & L_PMD_S2_RDWR) == L_PMD_S2_RDONLY;
 }
 
+static inline void kvm_set_s2pud_readonly(pud_t *pud)
+{
+}
+
+static inline bool kvm_s2pud_readonly(pud_t *pud)
+{
+	return true;

why true?  Shouldn't this return the pgd's readonly value, strictly
speaking, or if we rely on this never being called, have  VM_BUG_ON() ?

It returns true as it prevents a call to kvm_set_s2pud_readonly() but
both of the above functions should never be called on ARM due to
stage2_pud_huge() returning 0.

I'll add a VM_BUG_ON(pud) to indicate that these functions should never
be called and...

In any case, a comment explaining why we unconditionally return true
would be nice.

... add a comment to explain what's going on.

quoted

+}
+
 static inline bool kvm_page_empty(void *ptr)
 {
 	struct page *ptr_page = virt_to_page(ptr);

diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 672c8684d5c2..dbfd18e08cfb 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h

@@ -201,6 +201,16 @@ static inline bool kvm_s2pmd_readonly(pmd_t *pmd)
 	return kvm_s2pte_readonly((pte_t *)pmd);
 }
 
+static inline void kvm_set_s2pud_readonly(pud_t *pud)
+{
+	kvm_set_s2pte_readonly((pte_t *)pud);
+}
+
+static inline bool kvm_s2pud_readonly(pud_t *pud)
+{
+	return kvm_s2pte_readonly((pte_t *)pud);
+}
+
 static inline bool kvm_page_empty(void *ptr)
 {
 	struct page *ptr_page = virt_to_page(ptr);

diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 9dea96380339..02eefda5d71e 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c

@@ -1155,9 +1155,12 @@ static void  stage2_wp_puds(pgd_t *pgd, phys_addr_t addr, phys_addr_t end)
 	do {
 		next = stage2_pud_addr_end(addr, end);
 		if (!stage2_pud_none(*pud)) {
-			/* TODO:PUD not supported, revisit later if supported */
-			BUG_ON(stage2_pud_huge(*pud));
-			stage2_wp_pmds(pud, addr, next);
+			if (stage2_pud_huge(*pud)) {
+				if (!kvm_s2pud_readonly(pud))
+					kvm_set_s2pud_readonly(pud);
+			} else {
+				stage2_wp_pmds(pud, addr, next);
+			}
 		}
 	} while (pud++, addr = next, addr != end);
 }

-- 
2.15.1

Otherwise:

Reviewed-by: Christoffer Dall <redacted>

Thanks for taking a look.

Punit