From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4C993C83F11 for ; Sat, 26 Aug 2023 10:52:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232029AbjHZKv5 (ORCPT ); Sat, 26 Aug 2023 06:51:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50666 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232492AbjHZKv2 (ORCPT ); Sat, 26 Aug 2023 06:51:28 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 13B5B2121 for ; Sat, 26 Aug 2023 03:51:20 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id A61A760C3F for ; Sat, 26 Aug 2023 10:51:19 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 092A9C433C7; Sat, 26 Aug 2023 10:51:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1693047079; bh=P71fNEsmNmi49zzw500omXgcWHcNUIdoP9+jGwHPSqA=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=HDmV/4maL6U2JYHEsHR3WIRw62Y6M4IxKBBmC2HnG/Det88Do+zccdytuXHqk7Hvc YMa8BBrjunxEi8Rtky45H13sN1lIjp8wO4X4uNWgLqo2v5VSlm0E73IToQjSG6/Sy/ 9SVpks5gr9gy64+HQ5Lad2H+Rb6ORwNWHJto3/nnYTEQtnQhpeZY43FBfhPSOW4Xu5 7QcNifJBG/nJze2BEzaTMvn3i9ToDwHZFpKs/xJi3wSGXyDo8n7du6+p92on0Ql5/P +bsadfPVzhmSQ7KQKr+r5TKeFq5fArXzC8s+X80r96RfBOoObF5pQ9fmxsgxJmnFrZ do3IFJX3BAy/g== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1qZqsu-008Gvp-Ej; Sat, 26 Aug 2023 11:51:16 +0100 Date: Sat, 26 Aug 2023 11:51:16 +0100 Message-ID: <86a5uef55n.wl-maz@kernel.org> From: Marc Zyngier To: Jing Zhang Cc: KVM , KVMARM , ARMLinux , Oliver Upton , Will Deacon , Paolo Bonzini , James Morse , Alexandru Elisei , Suzuki K Poulose , Fuad Tabba , Reiji Watanabe , Raghavendra Rao Ananta , Suraj Jitindar Singh , Cornelia Huck , Shaoqin Huang Subject: Re: [PATCH v9 05/11] KVM: arm64: Enable writable for ID_AA64DFR0_EL1 and ID_DFR0_EL1 In-Reply-To: References: <20230821212243.491660-1-jingzhangos@google.com> <20230821212243.491660-6-jingzhangos@google.com> <878ra3pndw.wl-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: jingzhangos@google.com, kvm@vger.kernel.org, kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, oliver.upton@linux.dev, will@kernel.org, pbonzini@redhat.com, james.morse@arm.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com, tabba@google.com, reijiw@google.com, rananta@google.com, surajjs@amazon.com, cohuck@redhat.com, shahuang@redhat.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Tue, 22 Aug 2023 19:35:12 +0100, Jing Zhang wrote: >=20 > Hi Marc, >=20 > On Tue, Aug 22, 2023 at 12:06=E2=80=AFAM Marc Zyngier wr= ote: > > > > On Mon, 21 Aug 2023 22:22:37 +0100, > > Jing Zhang wrote: > > > > > > All valid fields in ID_AA64DFR0_EL1 and ID_DFR0_EL1 are writable > > > from userspace with this change. > > > RES0 fields and those fields hidden by KVM are not writable. > > > > > > Signed-off-by: Jing Zhang > > > --- > > > arch/arm64/kvm/sys_regs.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > > index afade7186675..20fc38bad4e8 100644 > > > --- a/arch/arm64/kvm/sys_regs.c > > > +++ b/arch/arm64/kvm/sys_regs.c > > > @@ -1931,6 +1931,8 @@ static bool access_spsr(struct kvm_vcpu *vcpu, > > > return true; > > > } > > > > > > +#define ID_AA64DFR0_EL1_RES0_MASK (GENMASK(59, 56) | GENMASK(27, 24)= | GENMASK(19, 16)) > > > + > > > /* > > > * Architected system registers. > > > * Important: Must be sorted ascending by Op0, Op1, CRn, CRm, Op2 > > > @@ -2006,7 +2008,7 @@ static const struct sys_reg_desc sys_reg_descs[= ] =3D { > > > .set_user =3D set_id_dfr0_el1, > > > .visibility =3D aa32_id_visibility, > > > .reset =3D read_sanitised_id_dfr0_el1, > > > - .val =3D ID_DFR0_EL1_PerfMon_MASK, }, > > > + .val =3D GENMASK(31, 0), }, > > > > Can you *please* look at the register and realise that we *don't* > > support writing to the whole of the low 32 bits? What does it mean to > > allow selecting the M-profile debug? Or the memory-mapped trace? > > > > You are advertising a lot of crap to userspace, and that's definitely > > not on. > > > > > ID_HIDDEN(ID_AFR0_EL1), > > > AA32_ID_SANITISED(ID_MMFR0_EL1), > > > AA32_ID_SANITISED(ID_MMFR1_EL1), > > > @@ -2055,7 +2057,7 @@ static const struct sys_reg_desc sys_reg_descs[= ] =3D { > > > .get_user =3D get_id_reg, > > > .set_user =3D set_id_aa64dfr0_el1, > > > .reset =3D read_sanitised_id_aa64dfr0_el1, > > > - .val =3D ID_AA64DFR0_EL1_PMUVer_MASK, }, > > > + .val =3D ~(ID_AA64DFR0_EL1_PMSVer_MASK | ID_AA64DFR0_EL1_RES0= _MASK), }, > > > > And it is the same thing here. Where is the handling code to deal with > > variable breakpoint numbers? Oh wait, there is none. Really, the only > > thing we support writing to are the PMU and Debug versions. And > > nothing else. > > > > What does it mean for userspace? Either the write will be denied > > despite being advertised a writable field (remember the first patch of > > the series???), or we'll blindly accept the write and further ignore > > the requested values. Do you really think any of this is acceptable? > > > > This is the *9th* version of this series, and we're still battling > > over some extremely basic userspace issues... I don't think we can > > merge this series as is stands. >=20 > I removed sanity checks across multiple fields after the discussion > here: https://lore.kernel.org/all/ZKRC80hb4hXwW8WK@thinky-boi/ > I might have misunderstood the discussion. I thought we'd let VMM do > more complete sanity checks. The problem is that you don't even have a statement about how this is supposed to be handled. What are the rules? How can the VMM author *know*? That's my real issue with this series: at no point do we state when an ID register can be written, what are the rules that must be followed, where is the split in responsibility between KVM and the VMM, and what is the expected behaviour when the VMM exposes something that is completely outlandish to the guest (such as the M-profile debug). Do you see the issue? We can always fix the code. But once we've exposed that to userspace, there is no going back. And I really want everybody's buy-in on that front, including the VMM people. Thanks, M. --=20 Without deviation from the norm, progress is not possible.