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)),
-- 
2.33.0

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.

We do, after all, expect some level of sanity from userspace of feature
dependencies as we do not enforce the dependency graph at the moment.
Hell, this could be useful for someone cross-migrating a nested VM from
a machine w/o FEAT_HCX to one that has it.


Thanks,
Oliver

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.