Re: [PATCH 10/12] KVM: arm64: Implement HW access flag management in stage-1 SW PTW

[Oliver Upton <oupton@kernel.org>]

Hey,

On Mon, Nov 17, 2025 at 02:49:49PM +0000, Marc Zyngier wrote:
> On Wed, 12 Nov 2025 18:34:04 +0000,
> > @@ -490,6 +506,17 @@ static int walk_s1(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
> >  	if (check_output_size(baddr & GENMASK(52, va_bottom), wi))
> >  		goto addrsz;
> >  
> > +	if (wi->ha)
> > +		new_desc |= PTE_AF;
> 
> What initialised new_desc the first place? Shouldn't there be a
> 'new_desc = desc;' somewhere before that?

Lol, I had this right after the read succeeds but threw out that part of
the diff.

> > @@ -1489,13 +1528,17 @@ void __kvm_at_s1e2(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)
> >  	}
> >  
> >  	/* We failed the translation, let's replay it in slow motion */
> > -	if ((par & SYS_PAR_EL1_F) && !par_check_s1_perm_fault(par))
> > -		par = handle_at_slow(vcpu, op, vaddr);
> > +	if ((par & SYS_PAR_EL1_F) && !par_check_s1_perm_fault(par)) {
> > +		ret = handle_at_slow(vcpu, op, vaddr, &par);
> > +		if (ret)
> > +			return ret;
> > +	}
> >  
> >  	vcpu_write_sys_reg(vcpu, par, PAR_EL1);
> > +	return 0;
> >  }
> 
> There is a quite a bit of churn in this patch changing the signature
> of the __kvm_at_s*() functions (and whatever calls them to propagate
> the errors). It'd be worth pulling this refactor as a preliminary
> patch, and then focus on the functional change.

Fine by me, thanks for reviewing!

-- 
Thanks,
Oliver