Re: [RFC PATCH 5/6] KVM: arm64: Optimize the stage2 map path with TLBI range instructions
From: Oliver Upton <hidden>
Date: 2023-01-24 23:26:00
Also in:
kvm, kvmarm, lkml
Hi Raghavendra, My comment from the previous change also applies here: KVM: arm64: Use range-based TLBIs when collapsing hugepages On Mon, Jan 09, 2023 at 09:53:46PM +0000, Raghavendra Rao Ananta wrote:
Currently, when the map path of stage2 page-table coalesces a bunch of pages into a hugepage, KVM invalidates the entire VM's TLB entries. This would cause a perforamance penality for the guest whose pages have already been coalesced earlier as they would have to refill their TLB entries unnecessarily again.
It is also problematic that we do this on every single fault where we collapse a hugepage.
quoted hunk ↗ jump to hunk
Hence, if the system supports it, use __kvm_tlb_flush_range_vmid_ipa() to flush only the range of pages that have been combined into a hugepage, while leaving other TLB entries alone. Signed-off-by: Raghavendra Rao Ananta <redacted> --- arch/arm64/kvm/hyp/pgtable.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-)diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c index b11cf2c618a6c..099032bb01bce 100644 --- a/arch/arm64/kvm/hyp/pgtable.c +++ b/arch/arm64/kvm/hyp/pgtable.c@@ -686,6 +686,22 @@ static bool stage2_try_set_pte(const struct kvm_pgtable_visit_ctx *ctx, kvm_pte_ return cmpxchg(ctx->ptep, ctx->old, new) == ctx->old; } +static void kvm_table_pte_flush(struct kvm_s2_mmu *mmu, u64 addr, u32 level, u32 tlb_level)
Could you call this something like kvm_pgtable_flush_range() and take an address range as an argument? TLBIRANGE can be used outside the context of a table (i.e. a subset of PTEs).
quoted hunk ↗ jump to hunk
+{ + if (system_supports_tlb_range()) { + u64 end = addr + kvm_granule_size(level); + + kvm_call_hyp(__kvm_tlb_flush_range_vmid_ipa, mmu, addr, end, tlb_level); + } else { + /* + * Invalidate the whole stage-2, as we may have numerous leaf + * entries below us which would otherwise need invalidating + * individually. + */ + kvm_call_hyp(__kvm_tlb_flush_vmid, mmu); + } +} + /** * stage2_try_break_pte() - Invalidates a pte according to the * 'break-before-make' requirements of the@@ -693,6 +709,7 @@ static bool stage2_try_set_pte(const struct kvm_pgtable_visit_ctx *ctx, kvm_pte_ * * @ctx: context of the visited pte. * @mmu: stage-2 mmu + * @tlb_level: The level at which the leaf pages are expected (for FEAT_TTL hint)
Do we need the caller to provide the TTL hint? We already have ctx->level, and stage2_try_break_pte() also knows what the removed PTE contained (i.e. a table or a block/page).
quoted hunk ↗ jump to hunk
* Returns: true if the pte was successfully broken. *@@ -701,7 +718,7 @@ static bool stage2_try_set_pte(const struct kvm_pgtable_visit_ctx *ctx, kvm_pte_ * on the containing table page. */ static bool stage2_try_break_pte(const struct kvm_pgtable_visit_ctx *ctx, - struct kvm_s2_mmu *mmu) + struct kvm_s2_mmu *mmu, u32 tlb_level) { struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;@@ -722,7 +739,7 @@ static bool stage2_try_break_pte(const struct kvm_pgtable_visit_ctx *ctx, * value (if any). */ if (kvm_pte_table(ctx->old, ctx->level)) - kvm_call_hyp(__kvm_tlb_flush_vmid, mmu); + kvm_table_pte_flush(mmu, ctx->addr, ctx->level, tlb_level);
I don't think we should provide a TTL hint for a removed table. It is entirely possible for the unlinked table to contain a mix of blocks and pages, meaning there isn't a uniform table level for the whole range.
quoted hunk ↗ jump to hunk
else if (kvm_pte_valid(ctx->old)) kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu, ctx->addr, ctx->level);@@ -804,7 +821,7 @@ static int stage2_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx, if (!stage2_pte_needs_update(ctx->old, new)) return -EAGAIN; - if (!stage2_try_break_pte(ctx, data->mmu)) + if (!stage2_try_break_pte(ctx, data->mmu, ctx->level)) return -EAGAIN; /* Perform CMOs before installation of the guest stage-2 PTE */@@ -861,7 +878,11 @@ static int stage2_map_walk_leaf(const struct kvm_pgtable_visit_ctx *ctx, if (!childp) return -ENOMEM; - if (!stage2_try_break_pte(ctx, data->mmu)) { + /* + * As the table will be replaced with a block, one level down would + * be the current page entries held by the table. + */
This isn't necessarily true. Ignoring mixed block/pages for a moment, Collapsing a PUD entry into a block after dirty logging (where we mapped at PTE level) would imply a TTL of ctx->level + 2. But again, I think it is best to provide no hint in this case. -- Thanks, Oliver _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel