[PATCH v2 0/5] KVM: e500: map readonly host pages for read, and cleanup

[PATCH v2 0/5] KVM: e500: map readonly host pages for read, and cleanup

[Paolo Bonzini]

The new __kvm_faultin_pfn() function is upset by the fact that e500
KVM ignores host page permissions - __kvm_faultin requires a "writable"
outgoing argument, but e500 KVM is passing NULL.

While a simple fix would be possible that simply allows writable to
be NULL, it is quite ugly to have e500 KVM ignore completely the host
permissions and map readonly host pages as guest-writable.  A more
complete fix is present in the second to fourth patches (the first is
an independent bugfix, Cc'd to stable).

The last one removes the VMA-based attempts at building huge shadow TLB
entries, in favor of using a PTE lookup similar to what is done for x86.
This special casing of VM_PFNMAP does not work well with remap_pfn_range()
as it assumes that VM_PFNMAP areas are contiguous.  Note that the same
incorrect logic is there in ARM's get_vma_page_shift() and RISC-V's
kvm_riscv_gstage_ioremap().

Fortunately, for e500 most of the code is already there; it just has to
be changed to compute the range from find_linux_pte()'s output rather
than find_vma().  The new code works for both VM_PFNMAP and hugetlb
mappings, so the latter is removed.

If this does not work out I'll go for something like
https://lore.kernel.org/kvm/Z3wnsQQ67GBf1Vsb@google.com/, but
with the helper in arch/powerpc/kvm/e500_mmu_host.c.

Patches 2-5 were tested by the reporter, Christian Zigotzky.  Since
the difference with v1 is minimal, I am going to send it to Linus
today.

Thanks,

Paolo

v1->v2: do not bother checking again that a memslot exists, instead
	add a fix to restore irqs even if !ptep


Paolo Bonzini (5):
  KVM: e500: always restore irqs
  KVM: e500: use shadow TLB entry as witness for writability
  KVM: e500: track host-writability of pages
  KVM: e500: map readonly host pages for read
  KVM: e500: perform hugepage check after looking up the PFN

 arch/powerpc/kvm/e500.h          |   2 +
 arch/powerpc/kvm/e500_mmu_host.c | 199 +++++++++++++------------------
 2 files changed, 85 insertions(+), 116 deletions(-)

-- 
2.47.1

[PATCH 1/5] KVM: e500: always restore irqs

[Paolo Bonzini]

If find_linux_pte fails, IRQs will not be restored.  This is unlikely
to happen in practice since it would have been reported as hanging
hosts, but it should of course be fixed anyway.

Cc: stable@vger.kernel.org
Reported-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/powerpc/kvm/e500_mmu_host.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index e5a145b578a4..6824e8139801 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -479,7 +479,6 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 		if (pte_present(pte)) {
 			wimg = (pte_val(pte) >> PTE_WIMGE_SHIFT) &
 				MAS2_WIMGE_MASK;
-			local_irq_restore(flags);
 		} else {
 			local_irq_restore(flags);
 			pr_err_ratelimited("%s: pte not present: gfn %lx,pfn %lx\n",
@@ -488,8 +487,9 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 			goto out;
 		}
 	}
+	local_irq_restore(flags);
+
 	writable = kvmppc_e500_ref_setup(ref, gtlbe, pfn, wimg);
-
 	kvmppc_e500_setup_stlbe(&vcpu_e500->vcpu, gtlbe, tsize,
 				ref, gvaddr, stlbe);
 
-- 
2.47.1

[PATCH 2/5] KVM: e500: use shadow TLB entry as witness for writability

[Paolo Bonzini]

kvmppc_e500_ref_setup is returning whether the guest TLB entry is writable,
which is than passed to kvm_release_faultin_page.  This makes little sense
for two reasons: first, because the function sets up the private data for
the page and the return value feels like it has been bolted on the side;
second, because what really matters is whether the _shadow_ TLB entry is
writable.  If it is not writable, the page can be released as non-dirty.
Shift from using tlbe_is_writable(gtlbe) to doing the same check on
the shadow TLB entry.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/powerpc/kvm/e500_mmu_host.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index 6824e8139801..c266c02f120f 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -242,7 +242,7 @@ static inline int tlbe_is_writable(struct kvm_book3e_206_tlb_entry *tlbe)
 	return tlbe->mas7_3 & (MAS3_SW|MAS3_UW);
 }
 
-static inline bool kvmppc_e500_ref_setup(struct tlbe_ref *ref,
+static inline void kvmppc_e500_ref_setup(struct tlbe_ref *ref,
 					 struct kvm_book3e_206_tlb_entry *gtlbe,
 					 kvm_pfn_t pfn, unsigned int wimg)
 {
@@ -251,8 +251,6 @@ static inline bool kvmppc_e500_ref_setup(struct tlbe_ref *ref,
 
 	/* Use guest supplied MAS2_G and MAS2_E */
 	ref->flags |= (gtlbe->mas2 & MAS2_ATTRIB_MASK) | wimg;
-
-	return tlbe_is_writable(gtlbe);
 }
 
 static inline void kvmppc_e500_ref_release(struct tlbe_ref *ref)
@@ -489,9 +487,10 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 	}
 	local_irq_restore(flags);
 
-	writable = kvmppc_e500_ref_setup(ref, gtlbe, pfn, wimg);
+	kvmppc_e500_ref_setup(ref, gtlbe, pfn, wimg);
 	kvmppc_e500_setup_stlbe(&vcpu_e500->vcpu, gtlbe, tsize,
 				ref, gvaddr, stlbe);
+	writable = tlbe_is_writable(stlbe);
 
 	/* Clear i-cache for new pages */
 	kvmppc_mmu_flush_icache(pfn);
-- 
2.47.1

[PATCH 3/5] KVM: e500: track host-writability of pages

[Paolo Bonzini]

Add the possibility of marking a page so that the UW and SW bits are
force-cleared.  This is stored in the private info so that it persists
across multiple calls to kvmppc_e500_setup_stlbe.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/powerpc/kvm/e500.h          |  2 ++
 arch/powerpc/kvm/e500_mmu_host.c | 15 +++++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h
index 6d0d329cbb35..f9acf866c709 100644
--- a/arch/powerpc/kvm/e500.h
+++ b/arch/powerpc/kvm/e500.h
@@ -34,6 +34,8 @@ enum vcpu_ftr {
 #define E500_TLB_BITMAP		(1 << 30)
 /* TLB1 entry is mapped by host TLB0 */
 #define E500_TLB_TLB0		(1 << 29)
+/* entry is writable on the host */
+#define E500_TLB_WRITABLE	(1 << 28)
 /* bits [6-5] MAS2_X1 and MAS2_X0 and [4-0] bits for WIMGE */
 #define E500_TLB_MAS2_ATTR	(0x7f)
 
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index c266c02f120f..b1be39639d4a 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -45,11 +45,14 @@ static inline unsigned int tlb1_max_shadow_size(void)
 	return host_tlb_params[1].entries - tlbcam_index - 1;
 }
 
-static inline u32 e500_shadow_mas3_attrib(u32 mas3, int usermode)
+static inline u32 e500_shadow_mas3_attrib(u32 mas3, bool writable, int usermode)
 {
 	/* Mask off reserved bits. */
 	mas3 &= MAS3_ATTRIB_MASK;
 
+	if (!writable)
+		mas3 &= ~(MAS3_UW|MAS3_SW);
+
 #ifndef CONFIG_KVM_BOOKE_HV
 	if (!usermode) {
 		/* Guest is in supervisor mode,
@@ -244,10 +247,13 @@ static inline int tlbe_is_writable(struct kvm_book3e_206_tlb_entry *tlbe)
 
 static inline void kvmppc_e500_ref_setup(struct tlbe_ref *ref,
 					 struct kvm_book3e_206_tlb_entry *gtlbe,
-					 kvm_pfn_t pfn, unsigned int wimg)
+					 kvm_pfn_t pfn, unsigned int wimg,
+					 bool writable)
 {
 	ref->pfn = pfn;
 	ref->flags = E500_TLB_VALID;
+	if (writable)
+		ref->flags |= E500_TLB_WRITABLE;
 
 	/* Use guest supplied MAS2_G and MAS2_E */
 	ref->flags |= (gtlbe->mas2 & MAS2_ATTRIB_MASK) | wimg;
@@ -303,6 +309,7 @@ static void kvmppc_e500_setup_stlbe(
 {
 	kvm_pfn_t pfn = ref->pfn;
 	u32 pr = vcpu->arch.shared->msr & MSR_PR;
+	bool writable = !!(ref->flags & E500_TLB_WRITABLE);
 
 	BUG_ON(!(ref->flags & E500_TLB_VALID));
 
@@ -310,7 +317,7 @@ static void kvmppc_e500_setup_stlbe(
 	stlbe->mas1 = MAS1_TSIZE(tsize) | get_tlb_sts(gtlbe) | MAS1_VALID;
 	stlbe->mas2 = (gvaddr & MAS2_EPN) | (ref->flags & E500_TLB_MAS2_ATTR);
 	stlbe->mas7_3 = ((u64)pfn << PAGE_SHIFT) |
-			e500_shadow_mas3_attrib(gtlbe->mas7_3, pr);
+			e500_shadow_mas3_attrib(gtlbe->mas7_3, writable, pr);
 }
 
 static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
@@ -487,7 +494,7 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 	}
 	local_irq_restore(flags);
 
-	kvmppc_e500_ref_setup(ref, gtlbe, pfn, wimg);
+	kvmppc_e500_ref_setup(ref, gtlbe, pfn, wimg, true);
 	kvmppc_e500_setup_stlbe(&vcpu_e500->vcpu, gtlbe, tsize,
 				ref, gvaddr, stlbe);
 	writable = tlbe_is_writable(stlbe);
-- 
2.47.1

[PATCH 4/5] KVM: e500: map readonly host pages for read

[Paolo Bonzini]

The new __kvm_faultin_pfn() function is upset by the fact that e500 KVM
ignores host page permissions - __kvm_faultin requires a "writable"
outgoing argument, but e500 KVM is nonchalantly passing NULL.

If the host page permissions do not include writability, the shadow
TLB entry is forcibly mapped read-only.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/powerpc/kvm/e500_mmu_host.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index b1be39639d4a..b38679e5821b 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -374,6 +374,7 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 			unsigned long slot_start, slot_end;
 
 			pfnmap = 1;
+			writable = vma->vm_flags & VM_WRITE;
 
 			start = vma->vm_pgoff;
 			end = start +
@@ -449,7 +450,7 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 
 	if (likely(!pfnmap)) {
 		tsize_pages = 1UL << (tsize + 10 - PAGE_SHIFT);
-		pfn = __kvm_faultin_pfn(slot, gfn, FOLL_WRITE, NULL, &page);
+		pfn = __kvm_faultin_pfn(slot, gfn, FOLL_WRITE, &writable, &page);
 		if (is_error_noslot_pfn(pfn)) {
 			if (printk_ratelimit())
 				pr_err("%s: real page not found for gfn %lx\n",
@@ -494,7 +495,7 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 	}
 	local_irq_restore(flags);
 
-	kvmppc_e500_ref_setup(ref, gtlbe, pfn, wimg, true);
+	kvmppc_e500_ref_setup(ref, gtlbe, pfn, wimg, writable);
 	kvmppc_e500_setup_stlbe(&vcpu_e500->vcpu, gtlbe, tsize,
 				ref, gvaddr, stlbe);
 	writable = tlbe_is_writable(stlbe);
-- 
2.47.1

[PATCH 5/5] KVM: e500: perform hugepage check after looking up the PFN

[Paolo Bonzini]

e500 KVM tries to bypass __kvm_faultin_pfn() in order to map VM_PFNMAP
VMAs as huge pages.  This is a Bad Idea because VM_PFNMAP VMAs could
become noncontiguous as a result of callsto remap_pfn_range().

Instead, use the already existing host PTE lookup to retrieve a
valid host-side mapping level after __kvm_faultin_pfn() has
returned.  Then find the largest size that will satisfy the
guest's request while staying within a single host PTE.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/powerpc/kvm/e500_mmu_host.c | 178 ++++++++++++-------------------
 1 file changed, 69 insertions(+), 109 deletions(-)

diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index b38679e5821b..06caf8bbbe2b 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -326,15 +326,14 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 	struct tlbe_ref *ref)
 {
 	struct kvm_memory_slot *slot;
-	unsigned long pfn = 0; /* silence GCC warning */
+	unsigned int psize;
+	unsigned long pfn;
 	struct page *page = NULL;
 	unsigned long hva;
-	int pfnmap = 0;
 	int tsize = BOOK3E_PAGESZ_4K;
 	int ret = 0;
 	unsigned long mmu_seq;
 	struct kvm *kvm = vcpu_e500->vcpu.kvm;
-	unsigned long tsize_pages = 0;
 	pte_t *ptep;
 	unsigned int wimg = 0;
 	pgd_t *pgdir;
@@ -356,111 +355,12 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 	slot = gfn_to_memslot(vcpu_e500->vcpu.kvm, gfn);
 	hva = gfn_to_hva_memslot(slot, gfn);
 
-	if (tlbsel == 1) {
-		struct vm_area_struct *vma;
-		mmap_read_lock(kvm->mm);
-
-		vma = find_vma(kvm->mm, hva);
-		if (vma && hva >= vma->vm_start &&
-		    (vma->vm_flags & VM_PFNMAP)) {
-			/*
-			 * This VMA is a physically contiguous region (e.g.
-			 * /dev/mem) that bypasses normal Linux page
-			 * management.  Find the overlap between the
-			 * vma and the memslot.
-			 */
-
-			unsigned long start, end;
-			unsigned long slot_start, slot_end;
-
-			pfnmap = 1;
-			writable = vma->vm_flags & VM_WRITE;
-
-			start = vma->vm_pgoff;
-			end = start +
-			      vma_pages(vma);
-
-			pfn = start + ((hva - vma->vm_start) >> PAGE_SHIFT);
-
-			slot_start = pfn - (gfn - slot->base_gfn);
-			slot_end = slot_start + slot->npages;
-
-			if (start < slot_start)
-				start = slot_start;
-			if (end > slot_end)
-				end = slot_end;
-
-			tsize = (gtlbe->mas1 & MAS1_TSIZE_MASK) >>
-				MAS1_TSIZE_SHIFT;
-
-			/*
-			 * e500 doesn't implement the lowest tsize bit,
-			 * or 1K pages.
-			 */
-			tsize = max(BOOK3E_PAGESZ_4K, tsize & ~1);
-
-			/*
-			 * Now find the largest tsize (up to what the guest
-			 * requested) that will cover gfn, stay within the
-			 * range, and for which gfn and pfn are mutually
-			 * aligned.
-			 */
-
-			for (; tsize > BOOK3E_PAGESZ_4K; tsize -= 2) {
-				unsigned long gfn_start, gfn_end;
-				tsize_pages = 1UL << (tsize - 2);
-
-				gfn_start = gfn & ~(tsize_pages - 1);
-				gfn_end = gfn_start + tsize_pages;
-
-				if (gfn_start + pfn - gfn < start)
-					continue;
-				if (gfn_end + pfn - gfn > end)
-					continue;
-				if ((gfn & (tsize_pages - 1)) !=
-				    (pfn & (tsize_pages - 1)))
-					continue;
-
-				gvaddr &= ~((tsize_pages << PAGE_SHIFT) - 1);
-				pfn &= ~(tsize_pages - 1);
-				break;
-			}
-		} else if (vma && hva >= vma->vm_start &&
-			   is_vm_hugetlb_page(vma)) {
-			unsigned long psize = vma_kernel_pagesize(vma);
-
-			tsize = (gtlbe->mas1 & MAS1_TSIZE_MASK) >>
-				MAS1_TSIZE_SHIFT;
-
-			/*
-			 * Take the largest page size that satisfies both host
-			 * and guest mapping
-			 */
-			tsize = min(__ilog2(psize) - 10, tsize);
-
-			/*
-			 * e500 doesn't implement the lowest tsize bit,
-			 * or 1K pages.
-			 */
-			tsize = max(BOOK3E_PAGESZ_4K, tsize & ~1);
-		}
-
-		mmap_read_unlock(kvm->mm);
-	}
-
-	if (likely(!pfnmap)) {
-		tsize_pages = 1UL << (tsize + 10 - PAGE_SHIFT);
-		pfn = __kvm_faultin_pfn(slot, gfn, FOLL_WRITE, &writable, &page);
-		if (is_error_noslot_pfn(pfn)) {
-			if (printk_ratelimit())
-				pr_err("%s: real page not found for gfn %lx\n",
-				       __func__, (long)gfn);
-			return -EINVAL;
-		}
-
-		/* Align guest and physical address to page map boundaries */
-		pfn &= ~(tsize_pages - 1);
-		gvaddr &= ~((tsize_pages << PAGE_SHIFT) - 1);
+	pfn = __kvm_faultin_pfn(slot, gfn, FOLL_WRITE, &writable, &page);
+	if (is_error_noslot_pfn(pfn)) {
+		if (printk_ratelimit())
+			pr_err("%s: real page not found for gfn %lx\n",
+			       __func__, (long)gfn);
+		return -EINVAL;
 	}
 
 	spin_lock(&kvm->mmu_lock);
@@ -478,7 +378,7 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 	 * can't run hence pfn won't change.
 	 */
 	local_irq_save(flags);
-	ptep = find_linux_pte(pgdir, hva, NULL, NULL);
+	ptep = find_linux_pte(pgdir, hva, NULL, &psize);
 	if (ptep) {
 		pte_t pte = READ_ONCE(*ptep);
 
@@ -495,6 +395,66 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 	}
 	local_irq_restore(flags);
 
+	if (psize && tlbsel == 1) {
+		unsigned long psize_pages, tsize_pages;
+		unsigned long start, end;
+		unsigned long slot_start, slot_end;
+
+		psize_pages = 1UL << (psize - PAGE_SHIFT);
+		start = pfn & ~(psize_pages - 1);
+		end = start + psize_pages;
+
+		slot_start = pfn - (gfn - slot->base_gfn);
+		slot_end = slot_start + slot->npages;
+
+		if (start < slot_start)
+			start = slot_start;
+		if (end > slot_end)
+			end = slot_end;
+
+		tsize = (gtlbe->mas1 & MAS1_TSIZE_MASK) >>
+			MAS1_TSIZE_SHIFT;
+
+		/*
+		 * Any page size that doesn't satisfy the host mapping
+		 * will fail the start and end tests.
+		 */
+		tsize = min(psize - PAGE_SHIFT + BOOK3E_PAGESZ_4K, tsize);
+
+		/*
+		 * e500 doesn't implement the lowest tsize bit,
+		 * or 1K pages.
+		 */
+		tsize = max(BOOK3E_PAGESZ_4K, tsize & ~1);
+
+		/*
+		 * Now find the largest tsize (up to what the guest
+		 * requested) that will cover gfn, stay within the
+		 * range, and for which gfn and pfn are mutually
+		 * aligned.
+		 */
+
+		for (; tsize > BOOK3E_PAGESZ_4K; tsize -= 2) {
+			unsigned long gfn_start, gfn_end;
+			tsize_pages = 1UL << (tsize - 2);
+
+			gfn_start = gfn & ~(tsize_pages - 1);
+			gfn_end = gfn_start + tsize_pages;
+
+			if (gfn_start + pfn - gfn < start)
+				continue;
+			if (gfn_end + pfn - gfn > end)
+				continue;
+			if ((gfn & (tsize_pages - 1)) !=
+			    (pfn & (tsize_pages - 1)))
+				continue;
+
+			gvaddr &= ~((tsize_pages << PAGE_SHIFT) - 1);
+			pfn &= ~(tsize_pages - 1);
+			break;
+		}
+	}
+
 	kvmppc_e500_ref_setup(ref, gtlbe, pfn, wimg, writable);
 	kvmppc_e500_setup_stlbe(&vcpu_e500->vcpu, gtlbe, tsize,
 				ref, gvaddr, stlbe);
-- 
2.47.1

Re: [PATCH 1/5] KVM: e500: always restore irqs

[Nicholas Piggin]

On Sun Jan 12, 2025 at 7:55 PM AEST, Paolo Bonzini wrote:
> If find_linux_pte fails, IRQs will not be restored.  This is unlikely
> to happen in practice since it would have been reported as hanging
> hosts, but it should of course be fixed anyway.
>
> Cc: stable@vger.kernel.org
> Reported-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

> ---
>  arch/powerpc/kvm/e500_mmu_host.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
> index e5a145b578a4..6824e8139801 100644
> --- a/arch/powerpc/kvm/e500_mmu_host.c
> +++ b/arch/powerpc/kvm/e500_mmu_host.c
> @@ -479,7 +479,6 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
>  		if (pte_present(pte)) {
>  			wimg = (pte_val(pte) >> PTE_WIMGE_SHIFT) &
>  				MAS2_WIMGE_MASK;
> -			local_irq_restore(flags);
>  		} else {
>  			local_irq_restore(flags);
>  			pr_err_ratelimited("%s: pte not present: gfn %lx,pfn %lx\n",
> @@ -488,8 +487,9 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
>  			goto out;
>  		}
>  	}
> +	local_irq_restore(flags);
> +
>  	writable = kvmppc_e500_ref_setup(ref, gtlbe, pfn, wimg);
> -
>  	kvmppc_e500_setup_stlbe(&vcpu_e500->vcpu, gtlbe, tsize,
>  				ref, gvaddr, stlbe);
>  

[Kernel 6.12.17] [PowerPC e5500] KVM HV compilation error

[Christian Zigotzky]

Hi All,

The stable long-term kernel 6.12.17 cannot compile with KVM HV support for e5500 PowerPC machines anymore.

Bug report: https://github.com/chzigotzky/kernels/issues/6

Kernel config: https://github.com/chzigotzky/kernels/blob/6_12/configs/x5000_defconfig

Error messages:

arch/powerpc/kvm/e500_mmu_host.c: In function 'kvmppc_e500_shadow_map':
arch/powerpc/kvm/e500_mmu_host.c:447:9: error: implicit declaration of function '__kvm_faultin_pfn' [-Werror=implicit-function-declaration]
   pfn = __kvm_faultin_pfn(slot, gfn, FOLL_WRITE, NULL, &page);
         ^~~~~~~~~~~~~~~~~
  CC      kernel/notifier.o
arch/powerpc/kvm/e500_mmu_host.c:500:2: error: implicit declaration of function 'kvm_release_faultin_page'; did you mean 'kvm_read_guest_page'? [-Werror=implicit-function-declaration]
  kvm_release_faultin_page(kvm, page, !!ret, writable);

After that, I compiled it without KVM HV support.

Kernel config: https://github.com/chzigotzky/kernels/blob/6_12/configs/e5500_defconfig

Please check the error messages.

Thanks,
Christian

Re: [Kernel 6.12.17] [PowerPC e5500] KVM HV compilation error

[Greg KH]

On Wed, Mar 05, 2025 at 03:14:13PM +0100, Christian Zigotzky wrote:
> Hi All,
> 
> The stable long-term kernel 6.12.17 cannot compile with KVM HV support for e5500 PowerPC machines anymore.
> 
> Bug report: https://github.com/chzigotzky/kernels/issues/6
> 
> Kernel config: https://github.com/chzigotzky/kernels/blob/6_12/configs/x5000_defconfig
> 
> Error messages:
> 
> arch/powerpc/kvm/e500_mmu_host.c: In function 'kvmppc_e500_shadow_map':
> arch/powerpc/kvm/e500_mmu_host.c:447:9: error: implicit declaration of function '__kvm_faultin_pfn' [-Werror=implicit-function-declaration]
>    pfn = __kvm_faultin_pfn(slot, gfn, FOLL_WRITE, NULL, &page);
>          ^~~~~~~~~~~~~~~~~
>   CC      kernel/notifier.o
> arch/powerpc/kvm/e500_mmu_host.c:500:2: error: implicit declaration of function 'kvm_release_faultin_page'; did you mean 'kvm_read_guest_page'? [-Werror=implicit-function-declaration]
>   kvm_release_faultin_page(kvm, page, !!ret, writable);
> 
> After that, I compiled it without KVM HV support.
> 
> Kernel config: https://github.com/chzigotzky/kernels/blob/6_12/configs/e5500_defconfig
> 
> Please check the error messages.

Odd, what commit caused this problem?  Any hint as to what commit is
missing to fix it?

thanks,

greg k-h

Re: [Kernel 6.12.17] [PowerPC e5500] KVM HV compilation error

[Sean Christopherson]

On Wed, Mar 05, 2025, Greg KH wrote:
> On Wed, Mar 05, 2025 at 03:14:13PM +0100, Christian Zigotzky wrote:
> > Hi All,
> > 
> > The stable long-term kernel 6.12.17 cannot compile with KVM HV support for e5500 PowerPC machines anymore.
> > 
> > Bug report: https://github.com/chzigotzky/kernels/issues/6
> > 
> > Kernel config: https://github.com/chzigotzky/kernels/blob/6_12/configs/x5000_defconfig
> > 
> > Error messages:
> > 
> > arch/powerpc/kvm/e500_mmu_host.c: In function 'kvmppc_e500_shadow_map':
> > arch/powerpc/kvm/e500_mmu_host.c:447:9: error: implicit declaration of function '__kvm_faultin_pfn' [-Werror=implicit-function-declaration]
> >    pfn = __kvm_faultin_pfn(slot, gfn, FOLL_WRITE, NULL, &page);
> >          ^~~~~~~~~~~~~~~~~
> >   CC      kernel/notifier.o
> > arch/powerpc/kvm/e500_mmu_host.c:500:2: error: implicit declaration of function 'kvm_release_faultin_page'; did you mean 'kvm_read_guest_page'? [-Werror=implicit-function-declaration]
> >   kvm_release_faultin_page(kvm, page, !!ret, writable);
> > 
> > After that, I compiled it without KVM HV support.
> > 
> > Kernel config: https://github.com/chzigotzky/kernels/blob/6_12/configs/e5500_defconfig
> > 
> > Please check the error messages.
> 
> Odd, what commit caused this problem?  Any hint as to what commit is
> missing to fix it?

833f69be62ac.  It most definitely should be reverted.  The "dependency" for commit
87ecfdbc699c ("KVM: e500: always restore irqs") is a superficial code conflict.

Oof.  The same buggy patch was queue/proposed for all stable trees from 5.4 onward,
but it look like it only landed in 6.1, 6.6, and 6.12.  I'll send reverts.

commit 833f69be62ac366b5c23b4a6434389e470dd5c7f
Author:     Sean Christopherson <seanjc@google.com>
AuthorDate: Thu Oct 10 11:23:56 2024 -0700
Commit:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CommitDate: Mon Feb 17 10:04:56 2025 +0100

    KVM: PPC: e500: Use __kvm_faultin_pfn() to handle page faults
    
    [ Upstream commit 419cfb983ca93e75e905794521afefcfa07988bb ]
    
    Convert PPC e500 to use __kvm_faultin_pfn()+kvm_release_faultin_page(),
    and continue the inexorable march towards the demise of
    kvm_pfn_to_refcounted_page().
    
    Signed-off-by: Sean Christopherson <seanjc@google.com>
    Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
    Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
    Message-ID: <20241010182427.1434605-55-seanjc@google.com>
    Stable-dep-of: 87ecfdbc699c ("KVM: e500: always restore irqs")
    Signed-off-by: Sasha Levin <sashal@kernel.org>

Re: [Kernel 6.12.17] [PowerPC e5500] KVM HV compilation error

[Paolo Bonzini]

On Wed, Mar 5, 2025 at 3:19 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> On Wed, Mar 05, 2025 at 03:14:13PM +0100, Christian Zigotzky wrote:
> > Hi All,
> >
> > The stable long-term kernel 6.12.17 cannot compile with KVM HV support for e5500 PowerPC machines anymore.
> >
> > Bug report: https://github.com/chzigotzky/kernels/issues/6
> >
> > Kernel config: https://github.com/chzigotzky/kernels/blob/6_12/configs/x5000_defconfig
> >
> > Error messages:
> >
> > arch/powerpc/kvm/e500_mmu_host.c: In function 'kvmppc_e500_shadow_map':
> > arch/powerpc/kvm/e500_mmu_host.c:447:9: error: implicit declaration of function '__kvm_faultin_pfn' [-Werror=implicit-function-declaration]
> >    pfn = __kvm_faultin_pfn(slot, gfn, FOLL_WRITE, NULL, &page);
> >          ^~~~~~~~~~~~~~~~~
> >   CC      kernel/notifier.o
> > arch/powerpc/kvm/e500_mmu_host.c:500:2: error: implicit declaration of function 'kvm_release_faultin_page'; did you mean 'kvm_read_guest_page'? [-Werror=implicit-function-declaration]
> >   kvm_release_faultin_page(kvm, page, !!ret, writable);
> >
> > After that, I compiled it without KVM HV support.
> >
> > Kernel config: https://github.com/chzigotzky/kernels/blob/6_12/configs/e5500_defconfig
> >
> > Please check the error messages.
>
> Odd, what commit caused this problem?

48fe216d7db6b651972c1c1d8e3180cd699971b0

> Any hint as to what commit is missing to fix it?

A big-ass 90 patch series. __kvm_faultin_pfn and
kvm_release_faultin_page were introduced in 6.13, as part of a big
revamp of how KVM does page faults on all architectures.

Just revert all this crap and apply the version that I've just sent
(https://lore.kernel.org/stable/20250305144938.212918-1-pbonzini@redhat.com/):

commit 48fe216d7db6b651972c1c1d8e3180cd699971b0
    KVM: e500: always restore irqs

commit 833f69be62ac366b5c23b4a6434389e470dd5c7f
    KVM: PPC: e500: Use __kvm_faultin_pfn() to handle page faults
    Message-ID: <20241010182427.1434605-55-seanjc@google.com>
    Stable-dep-of: 87ecfdbc699c ("KVM: e500: always restore irqs")

commit f2623aec7fdc2675667042c85f87502c9139c098
    KVM: PPC: e500: Mark "struct page" pfn accessed before dropping mmu_lock
    Message-ID: <20241010182427.1434605-54-seanjc@google.com>
    Stable-dep-of: 87ecfdbc699c ("KVM: e500: always restore irqs")

commit dec857329fb9a66a5bce4f9db14c97ef64725a32
    KVM: PPC: e500: Mark "struct page" dirty in kvmppc_e500_shadow_map()
    Message-ID: <20241010182427.1434605-53-seanjc@google.com>
    Stable-dep-of: 87ecfdbc699c ("KVM: e500: always restore irqs")

And this, ladies and gentlemen, is why I always include the apparently
silly Message-ID trailer. Don't you just love how someone, whether
script or human, cherry picked patches 53-55 without even wondering
what was in the 52 before. I'm not sure if it'd be worse for it to be
a human or a script... because if it's a script, surely the same level
of sophistication could have been put into figuring out whether the
thing even COMPILES.

Sasha, this wins the prize for most ridiculous automatic backport
ever. Please stop playing maintainer if you can't be bothered to read
the commit messages for random stuff that you apply.

Paolo

Re: [Kernel 6.12.17] [PowerPC e5500] KVM HV compilation error

[Greg KH]

On Wed, Mar 05, 2025 at 03:54:06PM +0100, Paolo Bonzini wrote:
> On Wed, Mar 5, 2025 at 3:19 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Wed, Mar 05, 2025 at 03:14:13PM +0100, Christian Zigotzky wrote:
> > > Hi All,
> > >
> > > The stable long-term kernel 6.12.17 cannot compile with KVM HV support for e5500 PowerPC machines anymore.
> > >
> > > Bug report: https://github.com/chzigotzky/kernels/issues/6
> > >
> > > Kernel config: https://github.com/chzigotzky/kernels/blob/6_12/configs/x5000_defconfig
> > >
> > > Error messages:
> > >
> > > arch/powerpc/kvm/e500_mmu_host.c: In function 'kvmppc_e500_shadow_map':
> > > arch/powerpc/kvm/e500_mmu_host.c:447:9: error: implicit declaration of function '__kvm_faultin_pfn' [-Werror=implicit-function-declaration]
> > >    pfn = __kvm_faultin_pfn(slot, gfn, FOLL_WRITE, NULL, &page);
> > >          ^~~~~~~~~~~~~~~~~
> > >   CC      kernel/notifier.o
> > > arch/powerpc/kvm/e500_mmu_host.c:500:2: error: implicit declaration of function 'kvm_release_faultin_page'; did you mean 'kvm_read_guest_page'? [-Werror=implicit-function-declaration]
> > >   kvm_release_faultin_page(kvm, page, !!ret, writable);
> > >
> > > After that, I compiled it without KVM HV support.
> > >
> > > Kernel config: https://github.com/chzigotzky/kernels/blob/6_12/configs/e5500_defconfig
> > >
> > > Please check the error messages.
> >
> > Odd, what commit caused this problem?
> 
> 48fe216d7db6b651972c1c1d8e3180cd699971b0
> 
> > Any hint as to what commit is missing to fix it?
> 
> A big-ass 90 patch series. __kvm_faultin_pfn and
> kvm_release_faultin_page were introduced in 6.13, as part of a big
> revamp of how KVM does page faults on all architectures.
> 
> Just revert all this crap and apply the version that I've just sent
> (https://lore.kernel.org/stable/20250305144938.212918-1-pbonzini@redhat.com/):
> 
> commit 48fe216d7db6b651972c1c1d8e3180cd699971b0
>     KVM: e500: always restore irqs
> 
> commit 833f69be62ac366b5c23b4a6434389e470dd5c7f
>     KVM: PPC: e500: Use __kvm_faultin_pfn() to handle page faults
>     Message-ID: <20241010182427.1434605-55-seanjc@google.com>
>     Stable-dep-of: 87ecfdbc699c ("KVM: e500: always restore irqs")
> 
> commit f2623aec7fdc2675667042c85f87502c9139c098
>     KVM: PPC: e500: Mark "struct page" pfn accessed before dropping mmu_lock
>     Message-ID: <20241010182427.1434605-54-seanjc@google.com>
>     Stable-dep-of: 87ecfdbc699c ("KVM: e500: always restore irqs")
> 
> commit dec857329fb9a66a5bce4f9db14c97ef64725a32
>     KVM: PPC: e500: Mark "struct page" dirty in kvmppc_e500_shadow_map()
>     Message-ID: <20241010182427.1434605-53-seanjc@google.com>
>     Stable-dep-of: 87ecfdbc699c ("KVM: e500: always restore irqs")
> 
> And this, ladies and gentlemen, is why I always include the apparently
> silly Message-ID trailer. Don't you just love how someone, whether
> script or human, cherry picked patches 53-55 without even wondering
> what was in the 52 before. I'm not sure if it'd be worse for it to be
> a human or a script... because if it's a script, surely the same level
> of sophistication could have been put into figuring out whether the
> thing even COMPILES.
> 
> Sasha, this wins the prize for most ridiculous automatic backport
> ever. Please stop playing maintainer if you can't be bothered to read
> the commit messages for random stuff that you apply.

Sasha, I thought we weren't taking any kvm patches that were not
explicitly tagged for stable?  The filter list says that...

Anyway, let me go revert these, thanks for the report.

thanks,

greg k-h

Re: [Kernel 6.12.17] [PowerPC e5500] KVM HV compilation error

[Greg KH]

On Wed, Mar 05, 2025 at 06:53:19AM -0800, Sean Christopherson wrote:
> On Wed, Mar 05, 2025, Greg KH wrote:
> > On Wed, Mar 05, 2025 at 03:14:13PM +0100, Christian Zigotzky wrote:
> > > Hi All,
> > > 
> > > The stable long-term kernel 6.12.17 cannot compile with KVM HV support for e5500 PowerPC machines anymore.
> > > 
> > > Bug report: https://github.com/chzigotzky/kernels/issues/6
> > > 
> > > Kernel config: https://github.com/chzigotzky/kernels/blob/6_12/configs/x5000_defconfig
> > > 
> > > Error messages:
> > > 
> > > arch/powerpc/kvm/e500_mmu_host.c: In function 'kvmppc_e500_shadow_map':
> > > arch/powerpc/kvm/e500_mmu_host.c:447:9: error: implicit declaration of function '__kvm_faultin_pfn' [-Werror=implicit-function-declaration]
> > >    pfn = __kvm_faultin_pfn(slot, gfn, FOLL_WRITE, NULL, &page);
> > >          ^~~~~~~~~~~~~~~~~
> > >   CC      kernel/notifier.o
> > > arch/powerpc/kvm/e500_mmu_host.c:500:2: error: implicit declaration of function 'kvm_release_faultin_page'; did you mean 'kvm_read_guest_page'? [-Werror=implicit-function-declaration]
> > >   kvm_release_faultin_page(kvm, page, !!ret, writable);
> > > 
> > > After that, I compiled it without KVM HV support.
> > > 
> > > Kernel config: https://github.com/chzigotzky/kernels/blob/6_12/configs/e5500_defconfig
> > > 
> > > Please check the error messages.
> > 
> > Odd, what commit caused this problem?  Any hint as to what commit is
> > missing to fix it?
> 
> 833f69be62ac.  It most definitely should be reverted.  The "dependency" for commit
> 87ecfdbc699c ("KVM: e500: always restore irqs") is a superficial code conflict.
> 
> Oof.  The same buggy patch was queue/proposed for all stable trees from 5.4 onward,
> but it look like it only landed in 6.1, 6.6, and 6.12.  I'll send reverts.
> 
> commit 833f69be62ac366b5c23b4a6434389e470dd5c7f
> Author:     Sean Christopherson <seanjc@google.com>
> AuthorDate: Thu Oct 10 11:23:56 2024 -0700
> Commit:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> CommitDate: Mon Feb 17 10:04:56 2025 +0100
> 
>     KVM: PPC: e500: Use __kvm_faultin_pfn() to handle page faults
>     
>     [ Upstream commit 419cfb983ca93e75e905794521afefcfa07988bb ]
>     
>     Convert PPC e500 to use __kvm_faultin_pfn()+kvm_release_faultin_page(),
>     and continue the inexorable march towards the demise of
>     kvm_pfn_to_refcounted_page().
>     
>     Signed-off-by: Sean Christopherson <seanjc@google.com>
>     Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>     Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>     Message-ID: <20241010182427.1434605-55-seanjc@google.com>
>     Stable-dep-of: 87ecfdbc699c ("KVM: e500: always restore irqs")
>     Signed-off-by: Sasha Levin <sashal@kernel.org>

All now reverted, sorry about that.

greg k-h