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 10:13:16AM -0800, Oliver Upton wrote:
> Hi Leo,
> 
> On Fri, Nov 06, 2026 at 04:12:37PM +0000, 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
> > pagetable walk when the callback function does nothing on that level.
> > 
> > Signed-off-by: Leonardo Bras <leo.bras@arm.com>
> 
> I'm still not crazy about using a flag per level... This is what I have
> stuffed on top locally. I've got a similar oneline fix for patch #3 to
> initialize the field with an offset.
> 
> Any opinions?
> 
> Thanks,
> Oliver
> 
> commit cad2070228c4305b5a83ce0a4cfa4aaa3dc0eb44
> Author: Oliver Upton <oupton@kernel.org>
> Date:   Fri Nov 6 10:03:14 2026 -0800
> 
>     squash! KVM: arm64: Introduce KVM_PGTABLE_WALK_SKIP_LEVEL* walk flags
>     
>     KVM: arm64: Add support for early termination of table walk recursion
>     
>     Add a new field to the walk flags that represents an offset from the
>     last level of the page table hierarchy at which recursion should
>     terminate.
> 
>     Signed-off-by: Leonardo Bras <leo.bras@arm.com>
>     [oliver: pass a numeric value instead of a flag per level]
>     Signed-off-by: Oliver Upton <oupton@kernel.org>
> 
> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> index 1d7a9125e522..18dcce3c506e 100644
> --- a/arch/arm64/include/asm/kvm_pgtable.h
> +++ b/arch/arm64/include/asm/kvm_pgtable.h
> @@ -326,10 +326,8 @@ typedef bool (*kvm_pgtable_force_pte_cb_t)(u64 addr, u64 end,
>   * @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
> + * @KVM_PGTABLE_WALK_SKIP_OFFSET:	Offset from KVM_PGTABLE_LAST_LEVEL at
> + * 					which the walk should stop descending
>   */
>  enum kvm_pgtable_walk_flags {
>  	KVM_PGTABLE_WALK_LEAF			= BIT(0),
> @@ -339,18 +337,9 @@ enum kvm_pgtable_walk_flags {
>  	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),
> +	KVM_PGTABLE_WALK_SKIP_OFFSET		= GENMASK(9, 7),
>  };
>  
> -#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;
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index 6aaf2696c99a..3008817da6c5 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -147,9 +147,12 @@ static bool kvm_pgtable_walk_continue(const struct kvm_pgtable_walker *walker,
>  
>  static bool kvm_pgtable_skip_level(s8 level, enum kvm_pgtable_walk_flags flags)
>  {
> -	u32 skip = FIELD_GET(KVM_PGTABLE_WALK_SKIP_LEVELS, flags);
> +	u8 offset = FIELD_GET(KVM_PGTABLE_WALK_SKIP_OFFSET, flags);
>  
> -	return unlikely(skip && level >= ffs(skip) - 1);
> +	BUILD_BUG_ON(!FIELD_FIT(KVM_PGTABLE_WALK_SKIP_OFFSET,
> +				KVM_PGTABLE_LAST_LEVEL - KVM_PGTABLE_FIRST_LEVEL));
> +
> +	return unlikely(level > KVM_PGTABLE_LAST_LEVEL - offset);
>  }
>  
>  static int __kvm_pgtable_walk(struct kvm_pgtable_walk_data *data,
> 

Hi Oliver,
Sure, I agree it looks much better in terms of coding: it uses a single 
flag and only 3 bits for all levels. Checking it is simpler as well, so I 
am pretty much willing to use it in v5.

The only part that bothers me is naming... skip_offset seems a bit generic 
to me:

	struct kvm_pgtable_walker walker = {
		.flags  = KVM_PGTABLE_WALK_LEAF |
			  FIELD_PREP(KVM_PGTABLE_WALK_SKIP_OFFSET, 1),
	}

What if we use, instead: 

KVM_PGTABLE_WALK_SKIP_LAST_N_LEVELS 

Then when using, we could have something like:
 
        struct kvm_pgtable_walker walker = {
                .flags  = KVM_PGTABLE_WALK_LEAF |
			  FIELD_PREP(KVM_PGTABLE_WALK_SKIP_LAST_N_LEVELS, 1),
	}

It becomes much longer, but I think it looks easier to understand it like 
this, if I did not already knew what it does. (Maybe we can think on 
something a bit shorter?)


What do you think?

Thanks!
Leo