Skip to contentSkip to conversation

KVM: arm64: TLBI fixes for the pgtable code

PATCH Superseded Will Deacon 3 participants 13 messages Mar 25, 2024 – Mar 27, 2024

Subsystems (2)
arm64 KVM arm64

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Will Deacon

author maintainer
#0 lore raw

cover KVM: arm64: TLBI fixes for the pgtable code

Hi folks,

While debugging something else, I noticed a few TLBI oddities in KVM's page-table code. These were found by inspection, so please have a good look as I may be mistaken.

Patches against -rc1.

Cheers,

Will

Cc Catalin Marinas <[email protected]>
Cc Gavin Shan <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Mostafa Saleh <[email protected]>
Cc Oliver Upton <[email protected]>
Cc Quentin Perret <[email protected]>
Cc Raghavendra Rao Ananta <[email protected]>
Cc Ryan Roberts <[email protected]>
Cc Shaoqin Huang <[email protected]>
Cc Suzuki K Poulose <[email protected]>
Cc Zenghui Yu <[email protected]>

--->8

Will Deacon (3):
KVM: arm64: Don't defer TLB invalidation when zapping table entries
KVM: arm64: Don't pass a TLBI level hint when zapping table entries
KVM: arm64: Use TLBI_TTL_UNKNOWN in __kvm_tlb_flush_vmid_range()

 arch/arm64/kvm/hyp/nvhe/tlb.c |  3 ++-
 arch/arm64/kvm/hyp/pgtable.c  | 12 ++++++++----
 arch/arm64/kvm/hyp/vhe/tlb.c  |  3 ++-
 3 files changed, 12 insertions(+), 6 deletions(-)
Signature · 1 line
2.44.0.396.g6e790dbe36-goog
author maintainer
#1 lore raw

PATCH 1/3 KVM: arm64: Don't defer TLB invalidation when zapping table entries

Commit 7657ea920c54 ("KVM: arm64: Use TLBI range-based instructions for unmap") introduced deferred TLB invalidation for the stage-2 page-table so that range-based invalidation can be used for the accumulated addresses. This works fine if the structure of the page-tables remains unchanged, but if entire tables are zapped and subsequently freed then we transiently leave the hardware page-table walker with a reference to freed memory thanks to the translation walk caches. For example, stage2_unmap_walker() will free page-table pages:

	if (childp)
		mm_ops->put_page(childp);

and issue the TLB invalidation later in kvm_pgtable_stage2_unmap():

	if (stage2_unmap_defer_tlb_flush(pgt))
		/* Perform the deferred TLB invalidations */
		kvm_tlb_flush_vmid_range(pgt->mmu, addr, size);

For now, take the conservative approach and invalidate the TLB eagerly when we clear a table entry.

Cc Raghavendra Rao Ananta <[email protected]>
Cc Shaoqin Huang <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Oliver Upton <[email protected]>
Signed-off-by Will Deacon <[email protected]>
---
 arch/arm64/kvm/hyp/pgtable.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
arch/arm64/kvm/hyp/pgtable.c +3 -1
index 3fae5830f8d2..de0b667ba296 100644
@@ -896,9 +896,11 @@ static void stage2_unmap_put_pte(const struct kvm_pgtable_visit_ctx *ctx,
 	if (kvm_pte_valid(ctx->old)) {
 		kvm_clear_pte(ctx->ptep);
 
-		if (!stage2_unmap_defer_tlb_flush(pgt))
+		if (!stage2_unmap_defer_tlb_flush(pgt) ||
+		    kvm_pte_table(ctx->old, ctx->level)) {
 			kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu,
 					ctx->addr, ctx->level);
+		}
 	}
 
 	mm_ops->put_page(ctx->ptep);
Signature · 1 line
2.44.0.396.g6e790dbe36-goog
#2 lore raw

Re: [PATCH 1/3] KVM: arm64: Don't defer TLB invalidation when zapping table entries

Hey Will,

Quoted 7 lines · Will Deacon

Commit 7657ea920c54 ("KVM: arm64: Use TLBI range-based instructions for unmap") introduced deferred TLB invalidation for the stage-2 page-table so that range-based invalidation can be used for the accumulated addresses. This works fine if the structure of the page-tables remains unchanged, but if entire tables are zapped and subsequently freed then we transiently leave the hardware page-table walker with a reference to freed memory thanks to the translation walk caches.

Yikes! Well spotted. This is rather unfortunate because the unmap path has been found to be a massive pain w/o aggregating invalidations. But sacrificing correctness in the name of performance... No thanks :)

Quoted 5 lines
arch/arm64/kvm/hyp/pgtable.c +0 -0
index 3fae5830f8d2..de0b667ba296 100644

arch/arm64/kvm/hyp/pgtable.c static void stage2_unmap_put_pte(const struct kvm_pgtable_visit_ctx *ctx, · line 903

Quoted 9 lines
arch/arm64/kvm/hyp/pgtable.c +3 -1
@@ -896,9 +896,11 @@ static void stage2_unmap_put_pte(const struct kvm_pgtable_visit_ctx *ctx,
 	if (kvm_pte_valid(ctx->old)) {
 		kvm_clear_pte(ctx->ptep);
 
-		if (!stage2_unmap_defer_tlb_flush(pgt))
+		if (!stage2_unmap_defer_tlb_flush(pgt) ||
+		    kvm_pte_table(ctx->old, ctx->level)) {
 			kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu,
 					ctx->addr, ctx->level);
+		}

I'm not sure this is correct, though. My understanding of TTL is that we're telling hardware where the *leaf* entry we're invalidating is found, however here we know that the addressed PTE is a table entry.

So maybe in the case of a table PTE this invalidation should TLBI_TTL_UNKNOWN.

arch/arm64/kvm/hyp/pgtable.c static void stage2_unmap_put_pte(const struct kvm_pgtable_visit_ctx *ctx, · line 906

Quoted 3 lines
arch/arm64/kvm/hyp/pgtable.c +0 -0
 	}
 
 	mm_ops->put_page(ctx->ptep);

At least for the 'normal' MMU where we use RCU, this could be changed to ->free_unlinked_table() which would defer the freeing of memory til after the invalidation completes. But that still hoses pKVM's stage-2 MMU freeing in-place.

Signature · 2 lines
Thanks,
Oliver
#3 lore raw

Re: [PATCH 1/3] KVM: arm64: Don't defer TLB invalidation when zapping table entries

Quoted 8 lines · Oliver Upton

Review

Quoted 3 lines
(quoted diff) +0 -0
 	}
 
 	mm_ops->put_page(ctx->ptep);

At least for the 'normal' MMU where we use RCU, this could be changed to ->free_unlinked_table() which would defer the freeing of memory til after the invalidation completes. But that still hoses pKVM's stage-2 MMU freeing in-place.

How about this (untested) diff? I _think_ it should address the invalidation issue while leaving the performance optimization in place for a 'normal' stage-2.

arch/arm64/kvm/hyp/pgtable.c +13 -8
index 3fae5830f8d2..896fdc0d157d 100644
@@ -872,14 +872,19 @@ static void stage2_make_pte(const struct kvm_pgtable_visit_ctx *ctx, kvm_pte_t n
 static bool stage2_unmap_defer_tlb_flush(struct kvm_pgtable *pgt)
 {
 	/*
-	 * If FEAT_TLBIRANGE is implemented, defer the individual
-	 * TLB invalidations until the entire walk is finished, and
-	 * then use the range-based TLBI instructions to do the
-	 * invalidations. Condition deferred TLB invalidation on the
-	 * system supporting FWB as the optimization is entirely
-	 * pointless when the unmap walker needs to perform CMOs.
+	 * It is possible to use FEAT_TLBIRANGE to do TLB invalidations at the
+	 * end of the walk if certain conditions are met:
+	 *
+	 *  - The stage-2 is for a 'normal' VM (i.e. managed in the kernel
+	 *    context). RCU provides sufficient guarantees to ensure that all
+	 *    hardware and software references on the stage-2 page tables are
+	 *    relinquished before freeing a table page.
+	 *
+	 *  - The system supports FEAT_FWB. Otherwise, KVM needs to do CMOs
+	 *    during the page table table walk.
 	 */
-	return system_supports_tlb_range() && stage2_has_fwb(pgt);
+	return !is_hyp_code() && system_supports_tlb_range() &&
+		stage2_has_fwb(pgt);
 }
 
 static void stage2_unmap_put_pte(const struct kvm_pgtable_visit_ctx *ctx,

@@ -1163,7 +1168,7 @@ static int stage2_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx,
 					       kvm_granule_size(ctx->level));
 
 	if (childp)
-		mm_ops->put_page(childp);
+		mm_ops->free_unlinked_table(childp, ctx->level);
 
 	return 0;
 }
author maintainer
#4 lore raw

Re: [PATCH 1/3] KVM: arm64: Don't defer TLB invalidation when zapping table entries

Quoted 46 lines · Oliver Upton
Quoted 8 lines · Oliver Upton

Review

Quoted 3 lines
(quoted diff) +0 -0
 	}
 
 	mm_ops->put_page(ctx->ptep);

At least for the 'normal' MMU where we use RCU, this could be changed to ->free_unlinked_table() which would defer the freeing of memory til after the invalidation completes. But that still hoses pKVM's stage-2 MMU freeing in-place.

How about this (untested) diff? I _think_ it should address the invalidation issue while leaving the performance optimization in place for a 'normal' stage-2.

arch/arm64/kvm/hyp/pgtable.c +12 -7
index 3fae5830f8d2..896fdc0d157d 100644
@@ -872,14 +872,19 @@ static void stage2_make_pte(const struct kvm_pgtable_visit_ctx *ctx, kvm_pte_t n
 static bool stage2_unmap_defer_tlb_flush(struct kvm_pgtable *pgt)
 {
 	/*
-	 * If FEAT_TLBIRANGE is implemented, defer the individual
-	 * TLB invalidations until the entire walk is finished, and
-	 * then use the range-based TLBI instructions to do the
-	 * invalidations. Condition deferred TLB invalidation on the
-	 * system supporting FWB as the optimization is entirely
-	 * pointless when the unmap walker needs to perform CMOs.
+	 * It is possible to use FEAT_TLBIRANGE to do TLB invalidations at the
+	 * end of the walk if certain conditions are met:
+	 *
+	 *  - The stage-2 is for a 'normal' VM (i.e. managed in the kernel
+	 *    context). RCU provides sufficient guarantees to ensure that all
+	 *    hardware and software references on the stage-2 page tables are
+	 *    relinquished before freeing a table page.
+	 *
+	 *  - The system supports FEAT_FWB. Otherwise, KVM needs to do CMOs
+	 *    during the page table table walk.
 	 */
-	return system_supports_tlb_range() && stage2_has_fwb(pgt);
+	return !is_hyp_code() && system_supports_tlb_range() &&
+		stage2_has_fwb(pgt);
 }
 
 static void stage2_unmap_put_pte(const struct kvm_pgtable_visit_ctx *ctx,

arch/arm64/kvm/hyp/pgtable.c static int stage2_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx, · line 1171

Quoted 5 lines
arch/arm64/kvm/hyp/pgtable.c +1 -1
@@ -1163,7 +1168,7 @@ static int stage2_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx,
 					       kvm_granule_size(ctx->level));
 
 	if (childp)
-		mm_ops->put_page(childp);
+		mm_ops->free_unlinked_table(childp, ctx->level);

Hmm, but doesn't the deferred TLBI still happen after the RCU critical section?

I also think I found another bug, so I'll send a v2 with an extra patch...

Will

#5 lore raw

Re: [PATCH 1/3] KVM: arm64: Don't defer TLB invalidation when zapping table entries

Quoted 9 lines · Will Deacon

arch/arm64/kvm/hyp/pgtable.c static int stage2_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx, · line 1171

Quoted 6 lines
arch/arm64/kvm/hyp/pgtable.c +1 -1
@@ -1163,7 +1168,7 @@ static int stage2_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx,
 					       kvm_granule_size(ctx->level));
 
 	if (childp)
-		mm_ops->put_page(childp);
+		mm_ops->free_unlinked_table(childp, ctx->level);

Hmm, but doesn't the deferred TLBI still happen after the RCU critical section?

Right... We'd need to change that too :) Although I suppose table invalidations are relatively infrequent when compared to leaf invalidations.

Quoted 1 line

I also think I found another bug, so I'll send a v2 with an extra patch...

And I'm sure there's plenty more to be found too, heh.

Signature · 2 lines
Thanks,
Oliver
author maintainer
#6 lore raw

PATCH 2/3 KVM: arm64: Don't pass a TLBI level hint when zapping table entries

In reply to Will Deacon

The TLBI level hints are for leaf entries only, so take care not to pass them incorrectly after clearing a table entry.

Cc Gavin Shan <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Quentin Perret <[email protected]>
Fixes 82bb02445de5 ("KVM: arm64: Implement kvm_pgtable_hyp_unmap() at EL2")
Fixes 6d9d2115c480 ("KVM: arm64: Add support for stage-2 map()/unmap() in generic page-table")
Signed-off-by Will Deacon <[email protected]>
---
 arch/arm64/kvm/hyp/pgtable.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
arch/arm64/kvm/hyp/pgtable.c +7 -5
index de0b667ba296..a40dafc43bb6 100644
@@ -528,7 +528,7 @@ static int hyp_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx,
 
 		kvm_clear_pte(ctx->ptep);
 		dsb(ishst);
-		__tlbi_level(vae2is, __TLBI_VADDR(ctx->addr, 0), ctx->level);
+		__tlbi_level(vae2is, __TLBI_VADDR(ctx->addr, 0), TLBI_TTL_UNKNOWN);
 	} else {
 		if (ctx->end - ctx->addr < granule)
 			return -EINVAL;

@@ -896,10 +896,12 @@ static void stage2_unmap_put_pte(const struct kvm_pgtable_visit_ctx *ctx,
 	if (kvm_pte_valid(ctx->old)) {
 		kvm_clear_pte(ctx->ptep);
 
-		if (!stage2_unmap_defer_tlb_flush(pgt) ||
-		    kvm_pte_table(ctx->old, ctx->level)) {
-			kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu,
-					ctx->addr, ctx->level);
+		if (kvm_pte_table(ctx->old, ctx->level)) {
+			kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu, ctx->addr,
+				     TLBI_TTL_UNKNOWN);
+		} else if (!stage2_unmap_defer_tlb_flush(pgt)) {
+			kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu, ctx->addr,
+				     ctx->level);
 		}
 	}
Signature · 1 line
2.44.0.396.g6e790dbe36-goog
#7 lore raw

Re: [PATCH 2/3] KVM: arm64: Don't pass a TLBI level hint when zapping table entries

Quoted 27 lines · Will Deacon

The TLBI level hints are for leaf entries only, so take care not to pass them incorrectly after clearing a table entry.

Cc Gavin Shan <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Quentin Perret <[email protected]>
Fixes 82bb02445de5 ("KVM: arm64: Implement kvm_pgtable_hyp_unmap() at EL2")
Fixes 6d9d2115c480 ("KVM: arm64: Add support for stage-2 map()/unmap() in generic page-table")
Signed-off-by Will Deacon <[email protected]>
---
 arch/arm64/kvm/hyp/pgtable.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
arch/arm64/kvm/hyp/pgtable.c +1 -1
index de0b667ba296..a40dafc43bb6 100644
@@ -528,7 +528,7 @@ static int hyp_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx,
 
 		kvm_clear_pte(ctx->ptep);
 		dsb(ishst);
-		__tlbi_level(vae2is, __TLBI_VADDR(ctx->addr, 0), ctx->level);
+		__tlbi_level(vae2is, __TLBI_VADDR(ctx->addr, 0), TLBI_TTL_UNKNOWN);
 	} else {
 		if (ctx->end - ctx->addr < granule)
 			return -EINVAL;

arch/arm64/kvm/hyp/pgtable.c static void stage2_unmap_put_pte(const struct kvm_pgtable_visit_ctx *ctx, · line 901

Quoted 10 lines
arch/arm64/kvm/hyp/pgtable.c +3 -4
@@ -896,10 +896,12 @@ static void stage2_unmap_put_pte(const struct kvm_pgtable_visit_ctx *ctx,
 	if (kvm_pte_valid(ctx->old)) {
 		kvm_clear_pte(ctx->ptep);
 
-		if (!stage2_unmap_defer_tlb_flush(pgt) ||
-		    kvm_pte_table(ctx->old, ctx->level)) {
-			kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu,
-					ctx->addr, ctx->level);
+		if (kvm_pte_table(ctx->old, ctx->level)) {
+			kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu, ctx->addr,
+				     TLBI_TTL_UNKNOWN);

Ah, here it is! Can you add this invalidation to patch #1? Otherwise the fix will intermediately introduce another bug, AFAICT.

Rest of the diff looks good.

Signature · 2 lines
Thanks,
Oliver
author maintainer
#8 lore raw

Re: [PATCH 2/3] KVM: arm64: Don't pass a TLBI level hint when zapping table entries

Quoted 31 lines · Oliver Upton
Quoted 30 lines · Will Deacon

The TLBI level hints are for leaf entries only, so take care not to pass them incorrectly after clearing a table entry.

Cc Gavin Shan <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Quentin Perret <[email protected]>
Fixes 82bb02445de5 ("KVM: arm64: Implement kvm_pgtable_hyp_unmap() at EL2")
Fixes 6d9d2115c480 ("KVM: arm64: Add support for stage-2 map()/unmap() in generic page-table")
Signed-off-by Will Deacon <[email protected]>
---
 arch/arm64/kvm/hyp/pgtable.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
arch/arm64/kvm/hyp/pgtable.c +1 -1
index de0b667ba296..a40dafc43bb6 100644
@@ -528,7 +528,7 @@ static int hyp_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx,
 
 		kvm_clear_pte(ctx->ptep);
 		dsb(ishst);
-		__tlbi_level(vae2is, __TLBI_VADDR(ctx->addr, 0), ctx->level);
+		__tlbi_level(vae2is, __TLBI_VADDR(ctx->addr, 0), TLBI_TTL_UNKNOWN);
 	} else {
 		if (ctx->end - ctx->addr < granule)
 			return -EINVAL;

@@ -896,10 +896,12 @@ static void stage2_unmap_put_pte(const struct kvm_pgtable_visit_ctx *ctx,
 	if (kvm_pte_valid(ctx->old)) {
 		kvm_clear_pte(ctx->ptep);
Quoted 10 lines

arch/arm64/kvm/hyp/pgtable.c static void stage2_unmap_put_pte(const struct kvm_pgtable_visit_ctx *ctx, · line 901

Quoted 7 lines
arch/arm64/kvm/hyp/pgtable.c +3 -4
-		if (!stage2_unmap_defer_tlb_flush(pgt) ||
-		    kvm_pte_table(ctx->old, ctx->level)) {
-			kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu,
-					ctx->addr, ctx->level);
+		if (kvm_pte_table(ctx->old, ctx->level)) {
+			kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu, ctx->addr,
+				     TLBI_TTL_UNKNOWN);

Ah, here it is! Can you add this invalidation to patch #1? Otherwise the fix will intermediately introduce another bug, AFAICT.

Heh, well I'd argue that bug already exists in the case that TLB invalidation isn't deferred, so that's why I kept them separate.

But happy to fold in if you prefer. Main thing is to get it fixed!

Will

#9 lore raw

Re: [PATCH 2/3] KVM: arm64: Don't pass a TLBI level hint when zapping table entries

Quoted 43 lines · Will Deacon
Quoted 31 lines · Oliver Upton
Quoted 30 lines · Will Deacon

The TLBI level hints are for leaf entries only, so take care not to pass them incorrectly after clearing a table entry.

Cc Gavin Shan <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Quentin Perret <[email protected]>
Fixes 82bb02445de5 ("KVM: arm64: Implement kvm_pgtable_hyp_unmap() at EL2")
Fixes 6d9d2115c480 ("KVM: arm64: Add support for stage-2 map()/unmap() in generic page-table")
Signed-off-by Will Deacon <[email protected]>
---
 arch/arm64/kvm/hyp/pgtable.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
arch/arm64/kvm/hyp/pgtable.c +1 -1
index de0b667ba296..a40dafc43bb6 100644
@@ -528,7 +528,7 @@ static int hyp_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx,
 
 		kvm_clear_pte(ctx->ptep);
 		dsb(ishst);
-		__tlbi_level(vae2is, __TLBI_VADDR(ctx->addr, 0), ctx->level);
+		__tlbi_level(vae2is, __TLBI_VADDR(ctx->addr, 0), TLBI_TTL_UNKNOWN);
 	} else {
 		if (ctx->end - ctx->addr < granule)
 			return -EINVAL;

@@ -896,10 +896,12 @@ static void stage2_unmap_put_pte(const struct kvm_pgtable_visit_ctx *ctx,
 	if (kvm_pte_valid(ctx->old)) {
 		kvm_clear_pte(ctx->ptep);
Quoted 10 lines

arch/arm64/kvm/hyp/pgtable.c static void stage2_unmap_put_pte(const struct kvm_pgtable_visit_ctx *ctx, · line 901

Quoted 7 lines
arch/arm64/kvm/hyp/pgtable.c +3 -4
-		if (!stage2_unmap_defer_tlb_flush(pgt) ||
-		    kvm_pte_table(ctx->old, ctx->level)) {
-			kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu,
-					ctx->addr, ctx->level);
+		if (kvm_pte_table(ctx->old, ctx->level)) {
+			kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu, ctx->addr,
+				     TLBI_TTL_UNKNOWN);

Ah, here it is! Can you add this invalidation to patch #1? Otherwise the fix will intermediately introduce another bug, AFAICT.

Quoted 2 lines

Heh, well I'd argue that bug already exists in the case that TLB invalidation isn't deferred, so that's why I kept them separate.

Ah, that's fine, when I was looking at this earlier I thought this patch was getting blamed on LPA2, although patch #1 needs to go a bit further back.

I'm fine with this as is, but maybe add a blurb to the changelog hinting that the level hint is BS but will be fixed in a future change.

Signature · 2 lines
Thanks,
Oliver
author maintainer
#10 lore raw

PATCH 3/3 KVM: arm64: Use TLBI_TTL_UNKNOWN in __kvm_tlb_flush_vmid_range()

In reply to Will Deacon

Commit c910f2b65518 ("arm64/mm: Update tlb invalidation routines for FEAT_LPA2") updated the __tlbi_level() macro to take the target level as an argument, with TLBI_TTL_UNKNOWN (rather than 0) indicating that the caller cannot provide level information. Unfortunately, the two implementations of __kvm_tlb_flush_vmid_range() were not updated and so now ask for an level 0 invalidation if FEAT_LPA2 is implemented.

Fix the problem by passing TLBI_TTL_UNKNOWN instead of 0 as the level argument to __flush_s2_tlb_range_op() in __kvm_tlb_flush_vmid_range().

Cc Ryan Roberts <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Oliver Upton <[email protected]>
Cc Marc Zyngier <[email protected]>
Fixes c910f2b65518 ("arm64/mm: Update tlb invalidation routines for FEAT_LPA2")
Signed-off-by Will Deacon <[email protected]>
---
 arch/arm64/kvm/hyp/nvhe/tlb.c | 3 ++-
 arch/arm64/kvm/hyp/vhe/tlb.c  | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)
arch/arm64/kvm/hyp/nvhe/tlb.c +2 -1
index a60fb13e2192..2fc68da4036d 100644
@@ -154,7 +154,8 @@ void __kvm_tlb_flush_vmid_range(struct kvm_s2_mmu *mmu,
 	/* Switch to requested VMID */
 	__tlb_switch_to_guest(mmu, &cxt, false);
 
-	__flush_s2_tlb_range_op(ipas2e1is, start, pages, stride, 0);
+	__flush_s2_tlb_range_op(ipas2e1is, start, pages, stride,
+				TLBI_TTL_UNKNOWN);
 
 	dsb(ish);
 	__tlbi(vmalle1is);
arch/arm64/kvm/hyp/vhe/tlb.c +2 -1
index b32e2940df7d..1a60b95381e8 100644
@@ -171,7 +171,8 @@ void __kvm_tlb_flush_vmid_range(struct kvm_s2_mmu *mmu,
 	/* Switch to requested VMID */
 	__tlb_switch_to_guest(mmu, &cxt);
 
-	__flush_s2_tlb_range_op(ipas2e1is, start, pages, stride, 0);
+	__flush_s2_tlb_range_op(ipas2e1is, start, pages, stride,
+				TLBI_TTL_UNKNOWN);
 
 	dsb(ish);
 	__tlbi(vmalle1is);
Signature · 1 line
2.44.0.396.g6e790dbe36-goog
Reviewed-by on v1 3/3
#11 lore raw

Re: [PATCH 3/3] KVM: arm64: Use TLBI_TTL_UNKNOWN in __kvm_tlb_flush_vmid_range()

Quoted 6 lines · Will Deacon

Commit c910f2b65518 ("arm64/mm: Update tlb invalidation routines for FEAT_LPA2") updated the __tlbi_level() macro to take the target level as an argument, with TLBI_TTL_UNKNOWN (rather than 0) indicating that the caller cannot provide level information. Unfortunately, the two implementations of __kvm_tlb_flush_vmid_range() were not updated and so now ask for an level 0 invalidation if FEAT_LPA2 is implemented.

Ouch, sorry about this! I remember rebasing my change onto the KVM tlbi range changes and having a few conflicts. Obviously I didn't do a good enough job of reviewing the result and missed this new user.

Quoted 10 lines

Fix the problem by passing TLBI_TTL_UNKNOWN instead of 0 as the level argument to __flush_s2_tlb_range_op() in __kvm_tlb_flush_vmid_range().

Cc Ryan Roberts <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Oliver Upton <[email protected]>
Cc Marc Zyngier <[email protected]>
Fixes c910f2b65518 ("arm64/mm: Update tlb invalidation routines for FEAT_LPA2")
Signed-off-by Will Deacon <[email protected]>
Reviewed-by Ryan Roberts <[email protected]>
Quoted 33 lines
---
 arch/arm64/kvm/hyp/nvhe/tlb.c | 3 ++-
 arch/arm64/kvm/hyp/vhe/tlb.c  | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)
arch/arm64/kvm/hyp/nvhe/tlb.c +2 -1
index a60fb13e2192..2fc68da4036d 100644
@@ -154,7 +154,8 @@ void __kvm_tlb_flush_vmid_range(struct kvm_s2_mmu *mmu,
 	/* Switch to requested VMID */
 	__tlb_switch_to_guest(mmu, &cxt, false);
 
-	__flush_s2_tlb_range_op(ipas2e1is, start, pages, stride, 0);
+	__flush_s2_tlb_range_op(ipas2e1is, start, pages, stride,
+				TLBI_TTL_UNKNOWN);
 
 	dsb(ish);
 	__tlbi(vmalle1is);
arch/arm64/kvm/hyp/vhe/tlb.c +2 -1
index b32e2940df7d..1a60b95381e8 100644
@@ -171,7 +171,8 @@ void __kvm_tlb_flush_vmid_range(struct kvm_s2_mmu *mmu,
 	/* Switch to requested VMID */
 	__tlb_switch_to_guest(mmu, &cxt);
 
-	__flush_s2_tlb_range_op(ipas2e1is, start, pages, stride, 0);
+	__flush_s2_tlb_range_op(ipas2e1is, start, pages, stride,
+				TLBI_TTL_UNKNOWN);
 
 	dsb(ish);
 	__tlbi(vmalle1is);
author maintainer
#12 lore raw

Re: [PATCH 3/3] KVM: arm64: Use TLBI_TTL_UNKNOWN in __kvm_tlb_flush_vmid_range()

Quoted 11 lines · Ryan Roberts
Quoted 6 lines · Will Deacon

Commit c910f2b65518 ("arm64/mm: Update tlb invalidation routines for FEAT_LPA2") updated the __tlbi_level() macro to take the target level as an argument, with TLBI_TTL_UNKNOWN (rather than 0) indicating that the caller cannot provide level information. Unfortunately, the two implementations of __kvm_tlb_flush_vmid_range() were not updated and so now ask for an level 0 invalidation if FEAT_LPA2 is implemented.

Ouch, sorry about this! I remember rebasing my change onto the KVM tlbi range changes and having a few conflicts. Obviously I didn't do a good enough job of reviewing the result and missed this new user.

No problem, it's easily done. It's also not your fault, as we shouldn't have been using the reserved encoding of 0 to mean "no hint" in the first place!

Quoted 8 lines
Quoted 6 lines
Cc Ryan Roberts <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Oliver Upton <[email protected]>
Cc Marc Zyngier <[email protected]>
Fixes c910f2b65518 ("arm64/mm: Update tlb invalidation routines for FEAT_LPA2")
Signed-off-by Will Deacon <[email protected]>
Reviewed-by Ryan Roberts <[email protected]>

Thanks.

Will