linux-kernel - Re: [PATCH v3 1/2] KVM: arm64: Make ID_AA64MMFR1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aMSV_tLO-W4VKYRX@linux.dev>
Date: Fri, 12 Sep 2025 14:51:58 -0700
From: Oliver Upton <oliver.upton@...ux.dev>
To: Jinqian Yang <yangjinqian1@...wei.com>
Cc: yuzenghui@...wei.com, maz@...nel.org,
	linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.linux.dev,
	linux-kernel@...r.kernel.org, liuyonglong@...wei.com,
	wangzhou1@...ilicon.com
Subject: Re: [PATCH v3 1/2] KVM: arm64: Make ID_AA64MMFR1_EL1.{HCX, TWED}
 writable from userspace

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@...wei.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