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 DF9FEC47258 for ; Tue, 23 Jan 2024 17:34:43 +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=R3i9BjX33kuW8BksHtc07yaDS0LhFQo9OLPVFB1kkoo=; b=2h6VFTLHgVfsAX yfpDPBED4QfzlZ5hKCUhcwLsCpf0tFiwqXoAw+u8M4UmiK8h6cf7iWMyDK4cO3uqma+3XfjDVeZFT Dd83gJwn0MWxcYwn7eXRuevLxb8VE/c0i1S0ijb/IzQLMK15yJ/dHPsm41CjmmSQsuncHUiOZTKY3 r6atJVVvA//B3BTdlXtUJUGIJte5kXxgfknC+/YP8aqsZ/JOgI2pXxxBqIIJlOHnM8uKN+qsfdOTW 0P4e8Z+uIgrdX/7YHch0kcyFfIK8ltK8Dtr5lOxnqxwGh/LUjqUCFJB2GdaAC9OvkzzmGI21kv4on 1+sSzbTLY0PCcUrTRa6g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rSKfC-00HWkV-1h; Tue, 23 Jan 2024 17:34:18 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1rSKf8-00HWhY-2J for linux-arm-kernel@lists.infradead.org; Tue, 23 Jan 2024 17:34:16 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 0668F61E8E; Tue, 23 Jan 2024 17:33:41 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A8208C433C7; Tue, 23 Jan 2024 17:33:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1706031220; bh=/lCLPXXPYb4ngKV2FWBipNfw3VKm7+Egk53vy0zy5bs=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=dr2sAC1Q/cXrqtUnJ0EusnHqAYzONamk0A8xkatD87uhx5BIgRiLhIDC+orXn5FPC JpIFDefRYTYWv91WYbGv9FIsYJdJ8ERjglbQ7qPFmgYig/3YA/fju5SYQPxTsKlw3b T8AAmZuaBfRZiKSN7BapHumkqI7r7vbtBOGthDFws+5e08PAAKFMAoAzkzovW4eG+0 zSEbPiic9qx8pL1tQcXLOzGqOHegDAJ9PjY8QWmB64sFOAYPfG9bzOQr+RUzUtMxez kSHRSs5ZvvHsA3KohnqPgfzf4JYwfCDaybHmyUI55mw5FktPHyFeuKQm5LkpoW5VoV w/2z/AybwxkAw== 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 1rSKeY-00E4H0-AF; Tue, 23 Jan 2024 17:33:38 +0000 Date: Tue, 23 Jan 2024 17:33:37 +0000 Message-ID: <86il3k7y4u.wl-maz@kernel.org> From: Marc Zyngier To: Joey Gouly Cc: kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, James Morse , Suzuki K Poulose , Oliver Upton , Zenghui Yu , Catalin Marinas , Will Deacon , Mark Brown Subject: Re: [PATCH 03/25] KVM: arm64: nv: Add sanitising to VNCR-backed sysregs In-Reply-To: <20240123134857.GB1283334@e124191.cambridge.arm.com> References: <20240122201852.262057-1-maz@kernel.org> <20240122201852.262057-4-maz@kernel.org> <20240123134857.GB1283334@e124191.cambridge.arm.com> 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/29.1 (aarch64-unknown-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: joey.gouly@arm.com, kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, james.morse@arm.com, suzuki.poulose@arm.com, oliver.upton@linux.dev, yuzenghui@huawei.com, catalin.marinas@arm.com, will@kernel.org, broonie@kernel.org 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-20240123_093414_837072_E66D8B5B X-CRM114-Status: GOOD ( 40.04 ) 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 On Tue, 23 Jan 2024 13:48:57 +0000, Joey Gouly wrote: > > Hi, > > On Mon, Jan 22, 2024 at 08:18:30PM +0000, Marc Zyngier wrote: > > VNCR-backed "registers" are actually only memory. Which means that > > there is zero control over what the guest can write, and that it > > is the hypervisor's job to actually sanitise the content of the > > backing store. Yeah, this is fun. > > > > In order to preserve some form of sanity, add a repainting mechanism > > that makes use of a per-VM set of RES0/RES1 masks, one pair per VNCR > > register. These masks get applied on access to the backing store via > > __vcpu_sys_reg(), ensuring that the state that is consumed by KVM is > > correct. > > > > So far, nothing populates these masks, but stay tuned. > > > > Signed-off-by: Marc Zyngier > > --- > > arch/arm64/include/asm/kvm_host.h | 25 +++++++++++++++++++ > > arch/arm64/kvm/arm.c | 1 + > > arch/arm64/kvm/nested.c | 41 ++++++++++++++++++++++++++++++- > > 3 files changed, 66 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > > index c0cf9c5f5e8d..fe35c59214ad 100644 > > --- a/arch/arm64/include/asm/kvm_host.h > > +++ b/arch/arm64/include/asm/kvm_host.h > > @@ -238,6 +238,8 @@ static inline u16 kvm_mpidr_index(struct kvm_mpidr_data *data, u64 mpidr) > > return index; > > } > > > > +struct kvm_sysreg_masks; > > + > > struct kvm_arch { > > struct kvm_s2_mmu mmu; > > > > @@ -312,6 +314,9 @@ struct kvm_arch { > > #define KVM_ARM_ID_REG_NUM (IDREG_IDX(sys_reg(3, 0, 0, 7, 7)) + 1) > > u64 id_regs[KVM_ARM_ID_REG_NUM]; > > > > + /* Masks for VNCR-baked sysregs */ > > + struct kvm_sysreg_masks *sysreg_masks; > > + > > /* > > * For an untrusted host VM, 'pkvm.handle' is used to lookup > > * the associated pKVM instance in the hypervisor. > > @@ -474,6 +479,13 @@ enum vcpu_sysreg { > > NR_SYS_REGS /* Nothing after this line! */ > > }; > > > > +struct kvm_sysreg_masks { > > + struct { > > + u64 res0; > > + u64 res1; > > + } mask[NR_SYS_REGS - __VNCR_START__]; > > +}; > > + > > struct kvm_cpu_context { > > struct user_pt_regs regs; /* sp = sp_el0 */ > > > > @@ -868,7 +880,20 @@ static inline u64 *__ctxt_sys_reg(const struct kvm_cpu_context *ctxt, int r) > > > > #define ctxt_sys_reg(c,r) (*__ctxt_sys_reg(c,r)) > > > > +#if defined (__KVM_NVHE_HYPERVISOR__) > > #define __vcpu_sys_reg(v,r) (ctxt_sys_reg(&(v)->arch.ctxt, (r))) > > +#else > > +u64 kvm_vcpu_sanitise_vncr_reg(const struct kvm_vcpu *, enum vcpu_sysreg); > > +#define __vcpu_sys_reg(v,r) \ > > + (*({ \ > > + const struct kvm_cpu_context *ctxt = &(v)->arch.ctxt; \ > > + u64 *__r = __ctxt_sys_reg(ctxt, (r)); \ > > + if (unlikely(cpus_have_final_cap(ARM64_HAS_NESTED_VIRT) && \ > > + r >= __VNCR_START__ && ctxt->vncr_array)) \ > > + *__r = kvm_vcpu_sanitise_vncr_reg((v), (r)); \ > > + __r; \ > > + })) > > +#endif > > Can you not use vcpu_has_nv() here? I see that __ctxt_sys_reg() does a similar > check, but vcpu_has_nv() covers !__KVM_NVHE_HYPERVISOR__, ARM64_HAS_NESTED_VIRT > and KVM_ARM_VCPU_HAS_EL2 (which I guess is what the ctxt->vncr_array check is > doing?) I can see it's defined in kvm_nested.h, which includes kvm_host.h, so > maybe that's an issue. > > #define __vcpu_sys_reg(v,r) \ > (*({ \ > const struct kvm_cpu_context *ctxt = &(v)->arch.ctxt; \ > u64 *__r = __ctxt_sys_reg(ctxt, (r)); \ > if (unlikely(vcpu_has_nv(v) && r >= __VNCR_START__)) \ > *__r = kvm_vcpu_sanitise_vncr_reg((v), (r)); \ > __r; \ > })) > > And since vcpu_has_nv() already checks __KVM_NVHE_HYPERVISOR__, you don't need > to define __vcpu_sys_reg() twice. All good points. Now that we only cater for NV2, vncr_array not being NULL is a given, although we still need it in __ctxt_sys_reg() as we don't have the full-fat vcpu at this stage (and thus cannot check for flags). > > Also maybe move that derefence into the macro, like: *__r;, instead of being > after the first (. Surprisingly, this doesn't work: ./arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h:240:38: error: lvalue required as left operand of assignment 240 | __vcpu_sys_reg(vcpu, DBGVCR32_EL2) = read_sysreg(dbgvcr32_el2); There are plenty more. > I'm not sure about the ctxt->vncr_array check, so maybe that's still > important. In the absence of the flag, it is. And I'm actually tempted to standardise on checking for vncr_array in vcpu_has_nv() as a substitute for the flag. It is likely to be a bit cheaper and for the value to be needed down the line. I'll rework this shortly. 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