Re: [PATCH v8 4/6] KVM: PPC: Book3S HV: Nested support in H_RPT_INVALIDATE

[Bharata B Rao <bharata@linux.ibm.com>]

On Tue, Jun 22, 2021 at 10:05:45AM +0530, Bharata B Rao wrote:
> On Mon, Jun 21, 2021 at 10:12:42AM -0700, Nathan Chancellor wrote:
> > I have not seen this reported yet so apologies if it has and there is a
> > fix I am missing:
> > 
> > arch/powerpc/kvm/book3s_hv_nested.c:1334:11: error: variable 'ap' is uninitialized when used here [-Werror,-Wuninitialized]
> >                                                            ap, start, end);
> >                                                            ^~
> > arch/powerpc/kvm/book3s_hv_nested.c:1276:25: note: initialize the variable 'ap' to silence this warning
> >         unsigned long psize, ap;
> >                                ^
> >                                 = 0
> 
> Thanks for catching this, this wasn't caught in my environment.
> 
> I will repost the series with proper initialization to ap.

Michael,

Here is the fix for this on top of powerpc/next. If it is easier
and cleaner to fold this into the original series and re-post
the whole series against any updated tree, let me know.


From 2e7198e28c0d1137f3230d4645e9cfddaccf4987 Mon Sep 17 00:00:00 2001
From: Bharata B Rao <bharata@linux.ibm.com>
Date: Tue, 22 Jun 2021 12:07:01 +0530
Subject: [PATCH 1/1] KVM: PPC: Book3S HV: Use proper ap value in
 H_RPT_INVALIDATE

The ap value that is used when performing range based partition
scoped invalidations for the nested guests wasn't initialized
correctly.

Fix this and while we are here, reorganize the routine that does
this invalidation for better readability.

Fixes: 0e67d866cb32 ("KVM: PPC: Book3S HV: Nested support in H_RPT_INVALIDATE")
Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
---
 arch/powerpc/kvm/book3s_hv_nested.c | 90 +++++++++++++----------------
 1 file changed, 40 insertions(+), 50 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index d78efb5f5bb3..3a06ac0b53e2 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -1222,27 +1222,6 @@ long kvmhv_do_nested_tlbie(struct kvm_vcpu *vcpu)
 	return H_SUCCESS;
 }
 
-static long do_tlb_invalidate_nested_tlb(struct kvm_vcpu *vcpu,
-					 unsigned long lpid,
-					 unsigned long page_size,
-					 unsigned long ap,
-					 unsigned long start,
-					 unsigned long end)
-{
-	unsigned long addr = start;
-	int ret;
-
-	do {
-		ret = kvmhv_emulate_tlbie_tlb_addr(vcpu, lpid, ap,
-						   get_epn(addr));
-		if (ret)
-			return ret;
-		addr += page_size;
-	} while (addr < end);
-
-	return ret;
-}
-
 static long do_tlb_invalidate_nested_all(struct kvm_vcpu *vcpu,
 					 unsigned long lpid, unsigned long ric)
 {
@@ -1263,6 +1242,42 @@ static long do_tlb_invalidate_nested_all(struct kvm_vcpu *vcpu,
  */
 static unsigned long tlb_range_flush_page_ceiling __read_mostly = 33;
 
+static long do_tlb_invalidate_nested_tlb(struct kvm_vcpu *vcpu,
+					 unsigned long lpid,
+					 unsigned long pg_sizes,
+					 unsigned long start,
+					 unsigned long end)
+{
+	int ret = H_P4;
+	unsigned long addr, nr_pages;
+	struct mmu_psize_def *def;
+	unsigned long psize, ap, page_size;
+	bool flush_lpid;
+
+	for (psize = 0; psize < MMU_PAGE_COUNT; psize++) {
+		def = &mmu_psize_defs[psize];
+		if (!(pg_sizes & def->h_rpt_pgsize))
+			continue;
+
+		nr_pages = (end - start) >> def->shift;
+		flush_lpid = nr_pages > tlb_range_flush_page_ceiling;
+		if (flush_lpid)
+			return do_tlb_invalidate_nested_all(vcpu, lpid,
+							RIC_FLUSH_TLB);
+		addr = start;
+		ap = mmu_get_ap(psize);
+		page_size = 1UL << def->shift;
+		do {
+			ret = kvmhv_emulate_tlbie_tlb_addr(vcpu, lpid, ap,
+						   get_epn(addr));
+			if (ret)
+				return H_P4;
+			addr += page_size;
+		} while (addr < end);
+	}
+	return ret;
+}
+
 /*
  * Performs partition-scoped invalidations for nested guests
  * as part of H_RPT_INVALIDATE hcall.
@@ -1271,10 +1286,6 @@ long do_h_rpt_invalidate_pat(struct kvm_vcpu *vcpu, unsigned long lpid,
 			     unsigned long type, unsigned long pg_sizes,
 			     unsigned long start, unsigned long end)
 {
-	struct kvm_nested_guest *gp;
-	long ret;
-	unsigned long psize, ap;
-
 	/*
 	 * If L2 lpid isn't valid, we need to return H_PARAMETER.
 	 *
@@ -1284,8 +1295,7 @@ long do_h_rpt_invalidate_pat(struct kvm_vcpu *vcpu, unsigned long lpid,
 	 * H_ENTER_NESTED call. Since we can't differentiate this case from
 	 * the invalid case, we ignore such flush requests and return success.
 	 */
-	gp = kvmhv_find_nested(vcpu->kvm, lpid);
-	if (!gp)
+	if (!kvmhv_find_nested(vcpu->kvm, lpid))
 		return H_SUCCESS;
 
 	/*
@@ -1313,29 +1323,9 @@ long do_h_rpt_invalidate_pat(struct kvm_vcpu *vcpu, unsigned long lpid,
 	if (start == 0 && end == -1)
 		return do_tlb_invalidate_nested_all(vcpu, lpid, RIC_FLUSH_TLB);
 
-	if (type & H_RPTI_TYPE_TLB) {
-		struct mmu_psize_def *def;
-		bool flush_lpid;
-		unsigned long nr_pages;
-
-		for (psize = 0; psize < MMU_PAGE_COUNT; psize++) {
-			def = &mmu_psize_defs[psize];
-			if (!(pg_sizes & def->h_rpt_pgsize))
-				continue;
-
-			nr_pages = (end - start) >> def->shift;
-			flush_lpid = nr_pages > tlb_range_flush_page_ceiling;
-			if (flush_lpid)
-				return do_tlb_invalidate_nested_all(vcpu, lpid,
-								RIC_FLUSH_TLB);
-
-			ret = do_tlb_invalidate_nested_tlb(vcpu, lpid,
-							   (1UL << def->shift),
-							   ap, start, end);
-			if (ret)
-				return H_P4;
-		}
-	}
+	if (type & H_RPTI_TYPE_TLB)
+		return do_tlb_invalidate_nested_tlb(vcpu, lpid, pg_sizes,
+						    start, end);
 	return H_SUCCESS;
 }
 
-- 
2.31.1