Re: [PATCH 11/22] KVM: arm64: Use a struct for stage-1 walk context

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

On Tue, Jun 23, 2026 at 11:41:50AM -0700, Oliver Upton wrote:
> Consolidate the current walk step in a struct such that helpers can be
> spun off from the core implementation.
> 
> Signed-off-by: Oliver Upton <oupton@kernel.org>
> ---
>  arch/arm64/kvm/at.c | 94 ++++++++++++++++++++++++---------------------
>  1 file changed, 51 insertions(+), 43 deletions(-)
> 
> diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
> index 597e9cddfc7e..816d23e7752d 100644
> --- a/arch/arm64/kvm/at.c
> +++ b/arch/arm64/kvm/at.c
> @@ -429,6 +429,14 @@ static int setup_s1_walk(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
>  	return -EFAULT;
>  }
>  
> +struct s1_walk_step {
> +	u64			desc;
> +	u64			desc_ipa;
> +	u64			desc_pa;
> +	struct kvm_s2_trans	s2_trans;
> +	int			level;
> +};
> +
>  static int kvm_read_s1_desc(struct kvm_vcpu *vcpu, u64 pa, u64 *desc,
>  			    struct s1_walk_info *wi)
>  {
> @@ -468,12 +476,12 @@ static void compute_s1_permissions(struct kvm_vcpu *vcpu,
>  static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
>  		   struct s1_walk_result *wr, struct kvm_walk_access *access)
>  {
> -	u64 va_top, va_bottom, baddr, desc, new_desc, ipa, va;
> -	struct kvm_s2_trans s2_trans = {};
> -	int level, stride, ret;
> +	u64 va_top, va_bottom, baddr, new_desc, va;
> +	struct s1_walk_step ws = {};
> +	int stride, ret;
>  
>  	va = access->ia;
> -	level = wi->sl;
> +	ws.level = wi->sl;
>  	stride = wi->pgshift - 3;
>  	baddr = wi->baddr;
>  
> @@ -482,15 +490,15 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
>  	while (1) {
>  		u64 index;
>  
> -		va_bottom = (3 - level) * stride + wi->pgshift;
> +		va_bottom = (3 - ws.level) * stride + wi->pgshift;
>  		index = (va & GENMASK_ULL(va_top, va_bottom)) >> (va_bottom - 3);
>  
> -		ipa = baddr | index;
> +		ws.desc_ipa = ws.desc_pa = baddr | index;
>  
>  		if (wi->s2) {
>  			struct kvm_walk_access s2_access = {
>  				.type	= WALK_ACCESS_S1PTW,
> -				.ia	= ipa,
> +				.ia	= ws.desc_ipa,

I only see desc_ipa being used here, and at this point, desc_ipa is the 
same as desc_pa, and we are copying it's value to this new struct.

Maybe we should postpone it's introduction to when it's actually needed?

All the rest seems just converting walk_s1() to use the new struct instead 
of the previously existing variables. So it should be ok.

If there is a good reason for desc_ipa being needed here, or if it can be 
removed on the next version, then FWIW:

Reviewed-by: Leonardo Bras <leo.bras@arm.com>

Thanks!
Leo