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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 42061C433F5 for ; Mon, 6 Dec 2021 18:49:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=mNM2Y0vKppo+92Uyezi9GGHSa1dzSWI+tKive7VHEOs=; b=vkossjNDL39Wfd 4fbbGOGokZNMlIYR4149qqQTi0pzL04OKOX3hHaheucnPemXU1zJNHvyrV9t7j8mYBuI/yZkN3AN7 LdiJoG286d9ia+FLbvo05d0HjijKpwzdFWYm0d1O9N+Fe+xnkrhs9fayeXMIN6TBe0TpG/6bgEpew AgFan791EEAd80/UIGzsuFj/7FCNCx4P1qjf1fxQ/ZYQ7XjBEKltytzIOXXvgsjnH4ZyLz9rRUoAV 5BAD9uhYN0o4C6T9DR4ke1eOUH6G9ejRHWN+6Hlru/8ApX8kO0ScimjpV7/pV20zz6TKaW/OTeUkx CqyMafpsAZyNWpYj0ASQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1muJ20-005OqU-M6; Mon, 06 Dec 2021 18:48:08 +0000 Received: from ams.source.kernel.org ([2604:1380:4601:e00::1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1muJ1w-005Opc-Hk for linux-arm-kernel@lists.infradead.org; Mon, 06 Dec 2021 18:48:06 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 458F4B811A3; Mon, 6 Dec 2021 18:48:03 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1CCF9C341C1; Mon, 6 Dec 2021 18:48:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1638816482; bh=6oJVcJI0NTvbhRn5nQbQbKPJF+DP2AeqgEIEpAEqYfo=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=H1q64ZLio0VI6XKQk8L0mLJWnQqiDX9W6f3w7c5ykk/qiTSJGDqtF2F+bVuCIcZOE dWd8PAXtBHzybXwe5hXd7h/vOl2Pt3Jyu65jEv1psOfkp9I26IfGaXLgcb9N1bLpD1 RCrm4O4UVwbwe+kPMsnrOdk4nBId6lr5jEPO0c5eiypSABad9xzEvcb9ry28/OUr3f br7X8T8wBHtgkck6TFvQwjXrxtjvtGbYlCuklMw5Wm3VRKzsOoPo1Ht6EL6/qSUJZ4 x1DMheJZeIWJliDLIPgh7MS7st+CpFtHPibw77woti4fRHcBpqA5tgx0h98EwxE43U PCzQ2Bp/l/18w== Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1muJ1s-00AHeO-6B; Mon, 06 Dec 2021 18:48:00 +0000 Date: Mon, 06 Dec 2021 18:47:59 +0000 Message-ID: <87tufl1sf4.wl-maz@kernel.org> From: Marc Zyngier To: Oliver Upton Cc: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, James Morse , Alexandru Elisei , Suzuki K Poulose , linux-arm-kernel@lists.infradead.org, Andrew Jones , Peter Shier , Ricardo Koller , Reiji Watanabe Subject: Re: [PATCH v3 3/6] KVM: arm64: Allow guest to set the OSLK bit In-Reply-To: References: <20211123210109.1605642-1-oupton@google.com> <20211123210109.1605642-4-oupton@google.com> <87sfvfmb8d.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/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: oupton@google.com, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, james.morse@arm.com, Alexandru.Elisei@arm.com, suzuki.poulose@arm.com, linux-arm-kernel@lists.infradead.org, drjones@redhat.com, pshier@google.com, ricarkol@google.com, reijiw@google.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211206_104804_903575_9BA60454 X-CRM114-Status: GOOD ( 35.48 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Oliver, On Mon, 06 Dec 2021 17:39:05 +0000, Oliver Upton wrote: > > On Mon, Nov 29, 2021 at 5:51 AM Marc Zyngier wrote: > > > > On Tue, 23 Nov 2021 21:01:06 +0000, > > Oliver Upton wrote: > > > > > > Allow writes to OSLAR and forward the OSLK bit to OSLSR. Do nothing with > > > the value for now. > > > > > > Reviewed-by: Reiji Watanabe > > > Signed-off-by: Oliver Upton > > > --- > > > arch/arm64/include/asm/sysreg.h | 6 ++++++ > > > arch/arm64/kvm/sys_regs.c | 33 ++++++++++++++++++++++++++------- > > > 2 files changed, 32 insertions(+), 7 deletions(-) > > > > > > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h > > > index 16b3f1a1d468..9fad61a82047 100644 > > > --- a/arch/arm64/include/asm/sysreg.h > > > +++ b/arch/arm64/include/asm/sysreg.h > > > @@ -129,7 +129,13 @@ > > > #define SYS_DBGWCRn_EL1(n) sys_reg(2, 0, 0, n, 7) > > > #define SYS_MDRAR_EL1 sys_reg(2, 0, 1, 0, 0) > > > #define SYS_OSLAR_EL1 sys_reg(2, 0, 1, 0, 4) > > > + > > > +#define SYS_OSLAR_OSLK BIT(0) > > > + > > > #define SYS_OSLSR_EL1 sys_reg(2, 0, 1, 1, 4) > > > + > > > +#define SYS_OSLSR_OSLK BIT(1) > > > + > > > #define SYS_OSDLR_EL1 sys_reg(2, 0, 1, 3, 4) > > > #define SYS_DBGPRCR_EL1 sys_reg(2, 0, 1, 4, 4) > > > #define SYS_DBGCLAIMSET_EL1 sys_reg(2, 0, 7, 8, 6) > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > > index 7bf350b3d9cd..5dbdb45d6d44 100644 > > > --- a/arch/arm64/kvm/sys_regs.c > > > +++ b/arch/arm64/kvm/sys_regs.c > > > @@ -44,6 +44,10 @@ > > > * 64bit interface. > > > */ > > > > > > +static int reg_from_user(u64 *val, const void __user *uaddr, u64 id); > > > +static int reg_to_user(void __user *uaddr, const u64 *val, u64 id); > > > +static u64 sys_reg_to_index(const struct sys_reg_desc *reg); > > > + > > > static bool read_from_write_only(struct kvm_vcpu *vcpu, > > > struct sys_reg_params *params, > > > const struct sys_reg_desc *r) > > > @@ -287,6 +291,24 @@ static bool trap_loregion(struct kvm_vcpu *vcpu, > > > return trap_raz_wi(vcpu, p, r); > > > } > > > > > > +static bool trap_oslar_el1(struct kvm_vcpu *vcpu, > > > + struct sys_reg_params *p, > > > + const struct sys_reg_desc *r) > > > +{ > > > + u64 oslsr; > > > + > > > + if (!p->is_write) > > > + return read_from_write_only(vcpu, p, r); > > > + > > > + /* Forward the OSLK bit to OSLSR */ > > > + oslsr = __vcpu_sys_reg(vcpu, OSLSR_EL1) & ~SYS_OSLSR_OSLK; > > > + if (p->regval & SYS_OSLAR_OSLK) > > > + oslsr |= SYS_OSLSR_OSLK; > > > + > > > + __vcpu_sys_reg(vcpu, OSLSR_EL1) = oslsr; > > > + return true; > > > +} > > > + > > > static bool trap_oslsr_el1(struct kvm_vcpu *vcpu, > > > struct sys_reg_params *p, > > > const struct sys_reg_desc *r) > > > @@ -309,9 +331,10 @@ static int set_oslsr_el1(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd, > > > if (err) > > > return err; > > > > > > - if (val != rd->val) > > > + if ((val & ~SYS_OSLSR_OSLK) != rd->val) > > > return -EINVAL; > > > > This looks odd. It means that once I have set the lock from userspace, > > I can't clear it? > > This does read weird, but I believe it makes sense still. rd->val is > the value of the register after warm reset, and does not store the > current value of the actual register. The true value is stashed in > kvm_cpu_context. Really, what I'm asserting here is that the only RW > bit is the OSLK bit. If any of the other bits are changed it should > return an error. Ah, the beauty of reading code in patches only. Of course, rd->val is only the reset value. And isn't called that, just to be confusing. Apologies for the noise. > I can either add a comment or make a macro for the expected register > value (or both) to make this more clear. A macro for the reset value would certainly be beneficial. But also, why not check the value against the current state and ignore the reset state altogether, since by the time you can poke at the vcpu, it has already been reset? It would certainly be more idiomatic. Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel