Re: [PATCH RESEND v4 2/3] KVM: arm64: Introduce KVM_PGTABLE_WALK_SKIP_LEVEL* walk flags

[Leonardo Bras <leo.bras@arm.com>]

On Fri, Nov 06, 2026 at 05:02:31PM +0000, Suzuki K Poulose wrote:
> On 06/11/2026 16:12, Leonardo Bras wrote:
> > Add new walking flags KVM_PGTABLE_WALK_SKIP_LEVEL{0-3}.
> > When used, they make the kvm_pgtable_walk() skip the given level, as well
> > as levels higher than that.
> > 
> > Being able to skip levels substancially decreases the runtime of a
> 
> minor nit: substantially

fixed!

> 
> > pagetable walk when the callback function does nothing on that level.
> > 
> > Signed-off-by: Leonardo Bras <leo.bras@arm.com>
> > ---
> >   arch/arm64/include/asm/kvm_pgtable.h | 14 ++++++++++++++
> >   arch/arm64/kvm/hyp/pgtable.c         | 16 +++++++++++++---
> >   2 files changed, 27 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> > index 41a8687938eb..5f37ceef8d63 100644
> > --- a/arch/arm64/include/asm/kvm_pgtable.h
> > +++ b/arch/arm64/include/asm/kvm_pgtable.h
> > @@ -311,31 +311,45 @@ typedef bool (*kvm_pgtable_force_pte_cb_t)(u64 addr, u64 end,
> >    * @KVM_PGTABLE_WALK_SHARED:		Indicates the page-tables may be shared
> >    *					with other software walkers.
> >    * @KVM_PGTABLE_WALK_IGNORE_EAGAIN:	Don't terminate the walk early if
> >    *					the walker returns -EAGAIN.
> >    * @KVM_PGTABLE_WALK_SKIP_BBM_TLBI:	Visit and update table entries
> >    *					without Break-before-make's
> >    *					TLB invalidation.
> >    * @KVM_PGTABLE_WALK_SKIP_CMO:		Visit and update table entries
> >    *					without Cache maintenance
> >    *					operations required.
> > + * @KVM_PGTABLE_WALK_SKIP_LEVEL0:	Skip visiting level 0+ entries
> > + * @KVM_PGTABLE_WALK_SKIP_LEVEL1:	Skip visiting level 1+ entries
> > + * @KVM_PGTABLE_WALK_SKIP_LEVEL2:	Skip visiting level 2+ entries
> > + * @KVM_PGTABLE_WALK_SKIP_LEVEL3:	Skip visiting level 3 entries
> >    */
> >   enum kvm_pgtable_walk_flags {
> >   	KVM_PGTABLE_WALK_LEAF			= BIT(0),
> >   	KVM_PGTABLE_WALK_TABLE_PRE		= BIT(1),
> >   	KVM_PGTABLE_WALK_TABLE_POST		= BIT(2),
> >   	KVM_PGTABLE_WALK_SHARED			= BIT(3),
> >   	KVM_PGTABLE_WALK_IGNORE_EAGAIN		= BIT(4),
> >   	KVM_PGTABLE_WALK_SKIP_BBM_TLBI		= BIT(5),
> >   	KVM_PGTABLE_WALK_SKIP_CMO		= BIT(6),
> > +	/* SKIP_LEVEL* flags should not be split nor reordered */
> > +	KVM_PGTABLE_WALK_SKIP_LEVEL0		= BIT(7),
> > +	KVM_PGTABLE_WALK_SKIP_LEVEL1		= BIT(8),
> > +	KVM_PGTABLE_WALK_SKIP_LEVEL2		= BIT(9),
> > +	KVM_PGTABLE_WALK_SKIP_LEVEL3		= BIT(10),
> >   };
> > +#define KVM_PGTABLE_WALK_SKIP_LEVELS 	(KVM_PGTABLE_WALK_SKIP_LEVEL0 | \
> > +					 KVM_PGTABLE_WALK_SKIP_LEVEL1 | \
> > +					 KVM_PGTABLE_WALK_SKIP_LEVEL2 | \
> > +					 KVM_PGTABLE_WALK_SKIP_LEVEL3 )
> > +
> >   struct kvm_pgtable_visit_ctx {
> >   	kvm_pte_t				*ptep;
> >   	kvm_pte_t				old;
> >   	void					*arg;
> >   	struct kvm_pgtable_mm_ops		*mm_ops;
> >   	u64					start;
> >   	u64					addr;
> >   	u64					end;
> >   	s8					level;
> >   	enum kvm_pgtable_walk_flags		flags;
> > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> > index 7a51f78b6446..80744478c810 100644
> > --- a/arch/arm64/kvm/hyp/pgtable.c
> > +++ b/arch/arm64/kvm/hyp/pgtable.c
> > @@ -137,20 +137,27 @@ static bool kvm_pgtable_walk_continue(const struct kvm_pgtable_walker *walker,
> >   	 * Ignore the return code altogether for walkers outside a fault handler
> >   	 * (e.g. write protecting a range of memory) and chug along with the
> >   	 * page table walk.
> >   	 */
> >   	if (r == -EAGAIN)
> >   		return walker->flags & KVM_PGTABLE_WALK_IGNORE_EAGAIN;
> >   	return !r;
> >   }
> > +static bool kvm_pgtable_skip_level(s8 level, enum kvm_pgtable_walk_flags flags)
> > +{
> > +	u32 skip = FIELD_GET(KVM_PGTABLE_WALK_SKIP_LEVELS, flags);
> > +
> > +	return unlikely(skip && level >= ffs(skip) - 1);
> > +}
> > +
> >   static int __kvm_pgtable_walk(struct kvm_pgtable_walk_data *data,
> >   			      struct kvm_pgtable_mm_ops *mm_ops, kvm_pteref_t pgtable, s8 level);
> >   static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
> >   				      struct kvm_pgtable_mm_ops *mm_ops,
> >   				      kvm_pteref_t pteref, s8 level)
> >   {
> >   	enum kvm_pgtable_walk_flags flags = data->walker->flags;
> >   	kvm_pte_t *ptep = kvm_dereference_pteref(data->walker, pteref);
> >   	struct kvm_pgtable_visit_ctx ctx = {
> > @@ -185,35 +192,35 @@ static inline int __kvm_pgtable_visit(struct kvm_pgtable_walk_data *data,
> >   	 * into a newly installed or replaced table.
> >   	 */
> >   	if (reload) {
> >   		ctx.old = READ_ONCE(*ptep);
> >   		table = kvm_pte_table(ctx.old, level);
> >   	}
> >   	if (!kvm_pgtable_walk_continue(data->walker, ret))
> >   		return ret;
> > -	if (!table) {
> > +	if (!table || kvm_pgtable_skip_level(level + 1, ctx.flags)) {
> >   		data->addr = ALIGN_DOWN(data->addr, kvm_granule_size(level));
> >   		data->addr += kvm_granule_size(level);
> >   		goto out;
> >   	}
> >   	childp = (kvm_pteref_t)kvm_pte_follow(ctx.old, mm_ops);
> >   	ret = __kvm_pgtable_walk(data, mm_ops, childp, level + 1);
> >   	if (!kvm_pgtable_walk_continue(data->walker, ret))
> >   		return ret;
> > -	if (ctx.flags & KVM_PGTABLE_WALK_TABLE_POST)
> > +out:
> > +	if (table && ctx.flags & KVM_PGTABLE_WALK_TABLE_POST)
> >   		ret = kvm_pgtable_visitor_cb(data, &ctx, KVM_PGTABLE_WALK_TABLE_POST);
> > -out:
> >   	if (kvm_pgtable_walk_continue(data->walker, ret))
> >   		return 0;
> >   	return ret;
> >   }
> >   static int __kvm_pgtable_walk(struct kvm_pgtable_walk_data *data,
> >   			      struct kvm_pgtable_mm_ops *mm_ops, kvm_pteref_t pgtable, s8 level)
> >   {
> >   	u32 idx;
> > @@ -242,20 +249,23 @@ static int _kvm_pgtable_walk(struct kvm_pgtable *pgt, struct kvm_pgtable_walk_da
> >   	u32 idx;
> >   	int ret = 0;
> >   	u64 limit = BIT(pgt->ia_bits);
> >   	if (data->addr > limit || data->end > limit)
> >   		return -ERANGE;
> >   	if (!pgt->pgd)
> >   		return -EINVAL;
> > +	if (kvm_pgtable_skip_level(pgt->start_level, data->walker->flags))
> > +		return 0;
> > +
> 
> Should we rather return an error here ? A walk skipping the start level,
> is that any meaningful ?
> 

My rationale on not returning error is that it may be ok for some routine 
to call a walker on a pagetable range just to be sure, even if it actually 
does nothing.

I take that this happens based on the split walker being called on
kvm_arch_mmu_enable_log_dirty_pt_masked() multiple times in the same range, 
even though it's already split, for instance.

But I am fine on changing it to return an error here if it's important in 
some scenario.

Thanks!
Leo