Re: [PATCH v3 1/2] KVM: arm64: Make ID_AA64MMFR1_EL1.{HCX, TWED} writable from userspace

[Marc Zyngier <maz@kernel.org>]

On Fri, 12 Sep 2025 22:51:58 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> Hi Jinqian,
> 
> On Thu, Sep 11, 2025 at 07:46:20PM +0800, Jinqian Yang wrote:
> > Allow userspace to downgrade {HCX, TWED} in ID_AA64MMFR1_EL1. Userspace can
> > only change the value from high to low.
> > 
> > Signed-off-by: Jinqian Yang <yangjinqian1@huawei.com>
> > ---
> >  arch/arm64/kvm/sys_regs.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 82ffb3b3b3cf..db49beb8804e 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -3002,8 +3002,6 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> >  				      ~(ID_AA64MMFR0_EL1_RES0 |
> >  					ID_AA64MMFR0_EL1_ASIDBITS)),
> >  	ID_WRITABLE(ID_AA64MMFR1_EL1, ~(ID_AA64MMFR1_EL1_RES0 |
> > -					ID_AA64MMFR1_EL1_HCX |
> > -					ID_AA64MMFR1_EL1_TWED |
> >  					ID_AA64MMFR1_EL1_XNX |
> >  					ID_AA64MMFR1_EL1_VH |
> >  					ID_AA64MMFR1_EL1_VMIDBits)),
> 
> I still have a bone to pick with Marc regarding the NV implications of
> this :) Attaching conversation below. Although for non-nested this LGTM.
> 
> On Tue, Sep 09, 2025 at 11:10:28AM +0100, Marc Zyngier wrote:
> > My concern here is the transitive implications of FEAT_HCX being
> > disabled: a quick look shows about 20 features that depend on
> > FEAT_HCX, and we don't really track this. I can probably generate the
> > dependency graph, but that's not going to be small. Or very useful.
> >
> > However, we should be able to let FEAT_HCX being disabled without
> > problem if the downgrading is limited to non-EL2 VMs. Same thing for
> > FEAT_VHE.
> >
> > What do you think?
> 
> So I'm a bit worried about making fields sometimes-writable, it creates
> a very confusing UAPI behavior. On top of that, our writable masks are
> currently static.
> 
> What if we treat the entire register as RES0 in this case? It seems to
> be consistent with all the underlying bits / features being NI. A
> mis-described VM isn't long for this world anyway (e.g. FEAT_SCTLR2 && !FEAT_HCX)
> and in that case I'd prefer an approach that keeps the KVM code as
> simple as possible.

I've pushed out a branch implementing this[1], though it hasn't had
much testing yet. I'll post it once I've convinced myself that this is
sane enough.

	M.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/el2-res0

-- 
Jazz isn't dead. It just smells funny.