Re: [PATCH 01/13] KVM: arm64: Hide CNTHV_*_EL2 from userspace for nVHE guests

[Marc Zyngier <maz@kernel.org>]

On Tue, 30 Sep 2025 01:35:07 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> Hey,
> 
> On Mon, Sep 29, 2025 at 05:04:45PM +0100, Marc Zyngier wrote:
> > Although we correctly UNDEF any CNTHV_*_EL2 access from the guest
> > when E2H==0, we still expose these registers to userspace, which
> > is a bad idea.
> > 
> > Drop the ad-hoc UNDEF injection and switch to a .visibility()
> > callback which will also hide the register from userspace.
> > 
> > Fixes: 0e45981028550 ("KVM: arm64: timer: Don't adjust the EL2 virtual timer offset")
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/kvm/sys_regs.c | 26 +++++++++++++-------------
> >  1 file changed, 13 insertions(+), 13 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index ee8a7033c85bf..9f2f4e0b042e8 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -1594,16 +1594,6 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu,
> >  	return true;
> >  }
> >  
> > -static bool access_hv_timer(struct kvm_vcpu *vcpu,
> > -			    struct sys_reg_params *p,
> > -			    const struct sys_reg_desc *r)
> > -{
> > -	if (!vcpu_el2_e2h_is_set(vcpu))
> > -		return undef_access(vcpu, p, r);
> > -
> > -	return access_arch_timer(vcpu, p, r);
> > -}
> > -
> >  static s64 kvm_arm64_ftr_safe_value(u32 id, const struct arm64_ftr_bits *ftrp,
> >  				    s64 new, s64 cur)
> >  {
> > @@ -2831,6 +2821,16 @@ static unsigned int s1pie_el2_visibility(const struct kvm_vcpu *vcpu,
> >  	return __el2_visibility(vcpu, rd, s1pie_visibility);
> >  }
> >  
> > +static unsigned int cnthv_visibility(const struct kvm_vcpu *vcpu,
> > +				     const struct sys_reg_desc *rd)
> > +{
> > +	if (vcpu_has_nv(vcpu) &&
> > +	    !vcpu_has_feature(vcpu, KVM_ARM_VCPU_HAS_EL2_E2H0))
> > +		return 0;
> > +
> > +	return REG_HIDDEN;
> > +}
> 
> Hmm. We've already exposed these to userspace at this point, we just
> conveniently last the get-reg-list test to assert the accessibility of
> these (broken) exposures.
> 
> Given the amount of UAPI mishaps we've had with registers in the past I
> don't have much appetite for taking away something we already
> advertised.
> 
> What about making these RAZ/WI from userspace?

Honestly, I don't think we should bother.

The only VMM supporting NV is QEMU, and it explicitly isn't able to
select E2H0. I'm happy to Cc stable on this, but worrying about nVHE
save/restore at this stage seems like an overreaction -- I'm pretty
sure NV save/restore is generally broken in many more ways.

Thanks,

	M.

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