Re: [PATCH] KVM: arm64: nv: Check RW permissions for insn abort due to S1PTW

[Marc Zyngier <maz@kernel.org>]

On Wed, 24 Jun 2026 10:39:43 +0100,
Oliver Upton <oupton@kernel.org> wrote:
> 
> On Wed, Jun 24, 2026 at 09:41:26AM +0100, Marc Zyngier wrote:
> > > diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> > > index 5bf3d7e1d92c..d5c61e0027c8 100644
> > > --- a/arch/arm64/include/asm/kvm_emulate.h
> > > +++ b/arch/arm64/include/asm/kvm_emulate.h
> > > @@ -479,21 +479,12 @@ static __always_inline int kvm_vcpu_sys_get_rt(struct kvm_vcpu *vcpu)
> > >  
> > >  static inline bool kvm_is_write_fault(struct kvm_vcpu *vcpu)
> > >  {
> > > -	if (kvm_vcpu_abt_iss1tw(vcpu)) {
> > > -		/*
> > > -		 * Only a permission fault on a S1PTW should be
> > > -		 * considered as a write. Otherwise, page tables baked
> > > -		 * in a read-only memslot will result in an exception
> > > -		 * being delivered in the guest.
> > > -		 *
> > > -		 * The drawback is that we end-up faulting twice if the
> > > -		 * guest is using any of HW AF/DB: a translation fault
> > > -		 * to map the page containing the PT (read only at
> > > -		 * first), then a permission fault to allow the flags
> > > -		 * to be set.
> > > -		 */
> > > -		return kvm_vcpu_trap_is_permission_fault(vcpu);
> > > -	}
> > > +	/*
> > > +	 * The architecture sucks; assume that the S1PTW fetched for write if
> > > +	 * HA is enabled at stage-1.
> > > +	 */
> > > +	if (kvm_vcpu_abt_iss1tw(vcpu))
> > > +		return effective_tcr_ha(vcpu);
> > 
> > OK, so you're trading the implicit state machine (translation fault ->
> > RO, permission fault -> RW) for a direct TCR.HA lookup, because the
> > only reason you'd get a S1PTW fault for write is if you were updating
> > the descriptor. This is cute, and I wish I had thought of that, as it
> > saves us a round trip.
> > 
> > But why can't that happen with HD? Surely this results in a similar
> > fault, and I think we should also evaluate that bit.
> 
> The effective value of both HAFT and HD is 0 if TCR_ELx.HA is 0. So
> evaluating HA covers them all :)

Indeed it does! Maybe worth capturing in a comment, because it isn't
completely obvious (we have R_SNVTX for HAFT, but you need to dive
into the TCR_EL1 register description to read the blurb about the
effective value of HD).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.