[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <878qigzsmq.wl-maz@kernel.org>
Date: Sun, 14 Sep 2025 21:27:57 +0100
From: Marc Zyngier <maz@...nel.org>
To: Oliver Upton <oliver.upton@...ux.dev>
Cc: Jinqian Yang <yangjinqian1@...wei.com>,
yuzenghui@...wei.com,
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
On Fri, 12 Sep 2025 22:51:58 +0100,
Oliver Upton <oliver.upton@...ux.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@...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.
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.
Powered by blists - more mailing lists