From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D613D3DB89 for ; Tue, 21 May 2024 22:52:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1716331945; cv=none; b=R35EVE+W9naE6xWmvZ7qZN+G8vk2NC7kiob8Ae3uslSTiDQvSO2NLdvS7zytf+gbGodS854ParLjdWKKk3ssdVmm373/wRi5FcI6PpiAPL4/fuQph5L0eWO9Oa46FkP4UVtoUPUwCthskqbkgGYpoUDzBb2855uJr1C88mNTQ/g= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1716331945; c=relaxed/simple; bh=v0Vv5zq7d4TRDM+lN818VW2kqCrPhvgiGE11Ah2/8hk=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=Ql+RBXTURN0OpsCudS2Z94z1qzdX2pBqZkqjsxepOjrIZw3yXGgH18M7Jw0SYLnOV0VlrniGKcnUfll/jpTNtttgU7RpSPyuVfzV1LEl6NCjVJpnUPyfPSLwgHMHzqXGH4oTXDvp+E8JAqnkxfG2sgUrmiTTu2rAQNWYTLbVJ0I= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=hdM/qZec; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="hdM/qZec" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3C106C2BD11; Tue, 21 May 2024 22:52:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1716331945; bh=v0Vv5zq7d4TRDM+lN818VW2kqCrPhvgiGE11Ah2/8hk=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=hdM/qZecuqhnFgS4Sw8ZspzBCv+n9hsir9UrJ4ZYIX484qgO8f/Q5DUn8Qy+nO6oL 8EypYSUwhtmLwcWkWZs/a8IwKHpxyClXbWDmX3YKgNr1M/S3UpjI9/+/BojQy49St+ gWt0JPlb5XXOjPCtAtjQF1pvyyOq6CjjwzzTbchZCE3yfpYVKtvaPy4xP7gEjHBuWF VCpIDCd3bb9Q4+nGgFyKwU+JXkF3YIHiGwTUxTNkOs5wBBJ7Gtmjc39eAbwk4ZOJD4 VZRcQMCKqHY2cWtmowg6jn6s4tziR8a/GtLmSwWk5DqBiv/UBu1HNrSWsdTfnyX4Mc X1c73jErm4Ksg== 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 1s9YLG-00Eoi6-L2; Tue, 21 May 2024 23:52:22 +0100 Date: Tue, 21 May 2024 23:52:21 +0100 Message-ID: <86o78yn5ga.wl-maz@kernel.org> From: Marc Zyngier To: Fuad Tabba Cc: kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, will@kernel.org, qperret@google.com, seanjc@google.com, alexandru.elisei@arm.com, catalin.marinas@arm.com, philmd@linaro.org, james.morse@arm.com, suzuki.poulose@arm.com, oliver.upton@linux.dev, mark.rutland@arm.com, broonie@kernel.org, joey.gouly@arm.com, rananta@google.com, yuzenghui@huawei.com Subject: Re: [PATCH v2 6/7] KVM: arm64: Eagerly restore host fpsimd/sve state in pKVM In-Reply-To: <20240521163720.3812851-7-tabba@google.com> References: <20240521163720.3812851-1-tabba@google.com> <20240521163720.3812851-7-tabba@google.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.2 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) Precedence: bulk X-Mailing-List: kvmarm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: tabba@google.com, kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, will@kernel.org, qperret@google.com, seanjc@google.com, alexandru.elisei@arm.com, catalin.marinas@arm.com, philmd@linaro.org, james.morse@arm.com, suzuki.poulose@arm.com, oliver.upton@linux.dev, mark.rutland@arm.com, broonie@kernel.org, joey.gouly@arm.com, rananta@google.com, yuzenghui@huawei.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false On Tue, 21 May 2024 17:37:19 +0100, Fuad Tabba wrote: > > When running in protected mode we don't want to leak protected > guest state to the host, including whether a guest has used > fpsimd/sve. Therefore, eagerly restore the host state on guest > exit when running in protected mode, which happens only if the > guest has used fpsimd/sve. > > As a future optimisation, we could go back to lazy restoring > state at the host after exiting non-protected guests. No. This sort of things is way too invasive and would require mapping the VHE host data at EL2. If we need to optimise this crap, then we apply the same techniques as we use for guests. If that's good enough for the guests, that's good enough for the host. > > Signed-off-by: Fuad Tabba > --- > arch/arm64/kvm/hyp/include/hyp/switch.h | 13 +++++- > arch/arm64/kvm/hyp/nvhe/hyp-main.c | 53 +++++++++++++++++++++++-- > arch/arm64/kvm/hyp/nvhe/pkvm.c | 2 + > arch/arm64/kvm/hyp/nvhe/switch.c | 16 +++++++- > 4 files changed, 79 insertions(+), 5 deletions(-) > > diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h > index 1897b73e635c..2fa29bfec0b6 100644 > --- a/arch/arm64/kvm/hyp/include/hyp/switch.h > +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h > @@ -320,6 +320,16 @@ static inline void __hyp_sve_restore_guest(struct kvm_vcpu *vcpu) > write_sysreg_el1(__vcpu_sys_reg(vcpu, ZCR_EL1), SYS_ZCR); > } > > +static inline void __hyp_sve_save_host(void) > +{ > + struct user_sve_state *sve_state = *host_data_ptr(sve_state); > + > + sve_state->zcr_el1 = read_sysreg_el1(SYS_ZCR); > + sve_cond_update_zcr_vq(ZCR_ELx_LEN_MASK, SYS_ZCR_EL2); > + __sve_save_state(sve_state->sve_regs + sve_ffr_offset(kvm_host_sve_max_vl), > + &sve_state->fpsr); > +} > + > static void kvm_hyp_save_fpsimd_host(struct kvm_vcpu *vcpu); > > /* > @@ -356,7 +366,8 @@ static bool kvm_hyp_handle_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code) > > /* First disable enough traps to allow us to update the registers */ > reg = CPACR_EL1_FPEN_EL0EN | CPACR_EL1_FPEN_EL1EN; > - if (sve_guest) > + if (sve_guest || > + (is_protected_kvm_enabled() && system_supports_sve())) This looks really ugly. Why can't we just compute sve_guest to take these conditions into account? > reg |= CPACR_EL1_ZEN_EL0EN | CPACR_EL1_ZEN_EL1EN; > cpacr_clear_set(0, reg); > isb(); > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c > index b07d44484f42..f79f0f7b2759 100644 > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c > @@ -23,20 +23,66 @@ DEFINE_PER_CPU(struct kvm_nvhe_init_params, kvm_init_params); > > void __kvm_hyp_host_forward_smc(struct kvm_cpu_context *host_ctxt); > > +static void __hyp_sve_save_guest(struct kvm_vcpu *vcpu) > +{ > + __vcpu_sys_reg(vcpu, ZCR_EL1) = read_sysreg_el1(SYS_ZCR); > + sve_cond_update_zcr_vq(vcpu_sve_max_vq(vcpu) - 1, SYS_ZCR_EL2); > + __sve_save_state(vcpu_sve_pffr(vcpu), &vcpu->arch.ctxt.fp_regs.fpsr); > + sve_cond_update_zcr_vq(ZCR_ELx_LEN_MASK, SYS_ZCR_EL2); > +} > + > +static void __hyp_sve_restore_host(void) > +{ > + struct user_sve_state *sve_state = *host_data_ptr(sve_state); > + > + sve_cond_update_zcr_vq(ZCR_ELx_LEN_MASK, SYS_ZCR_EL2); > + __sve_restore_state(sve_state->sve_regs + sve_ffr_offset(kvm_host_sve_max_vl), This is what I was worried about in a previous patch. kvm_host_sve_max_vl represents the max VL across all CPUs. if CPU0 supports 128bit SVE and CPU1 256bit, the value is 256. but on CPU0, ZCR_ELx_LEN_MASK will results in using 128bit accesses, and the offset will be wrong. I can't convince myself that anything really goes wrong as long as we're consistent between save and restore, but that's at best ugly and needs extensive documenting. On top of that, a conditional update of ZCR_EL2 with ZCR_ELx_LEN_MASK is unlikely to be beneficial, since nobody implements 2k vectors. A direct write followed by an ISB is likely to be better. > + &sve_state->fpsr); > + write_sysreg_el1(sve_state->zcr_el1, SYS_ZCR); > +} > + > +static void fpsimd_sve_flush(void) > +{ > + *host_data_ptr(fp_owner) = FP_STATE_HOST_OWNED; > +} > + > +static void fpsimd_sve_sync(struct kvm_vcpu *vcpu) > +{ > + if (!guest_owns_fp_regs()) > + return; > + > + cpacr_clear_set(0, (CPACR_EL1_ZEN_EL1EN | CPACR_EL1_ZEN_EL0EN | > + CPACR_EL1_FPEN_EL1EN | CPACR_EL1_FPEN_EL0EN)); > + isb(); > + > + if (vcpu_has_sve(vcpu)) > + __hyp_sve_save_guest(vcpu); > + else > + __fpsimd_save_state(&vcpu->arch.ctxt.fp_regs); > + > + if (system_supports_sve()) > + __hyp_sve_restore_host(); > + else > + __fpsimd_restore_state(*host_data_ptr(fpsimd_state)); > + > + *host_data_ptr(fp_owner) = FP_STATE_HOST_OWNED; > +} > + > static void flush_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu) > { > struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu; > > + fpsimd_sve_flush(); > + > hyp_vcpu->vcpu.arch.ctxt = host_vcpu->arch.ctxt; > > hyp_vcpu->vcpu.arch.sve_state = kern_hyp_va(host_vcpu->arch.sve_state); > - hyp_vcpu->vcpu.arch.sve_max_vl = host_vcpu->arch.sve_max_vl; > + hyp_vcpu->vcpu.arch.sve_max_vl = min(host_vcpu->arch.sve_max_vl, kvm_host_sve_max_vl); > > hyp_vcpu->vcpu.arch.hw_mmu = host_vcpu->arch.hw_mmu; > > hyp_vcpu->vcpu.arch.hcr_el2 = host_vcpu->arch.hcr_el2; > hyp_vcpu->vcpu.arch.mdcr_el2 = host_vcpu->arch.mdcr_el2; > - hyp_vcpu->vcpu.arch.cptr_el2 = host_vcpu->arch.cptr_el2; > > hyp_vcpu->vcpu.arch.iflags = host_vcpu->arch.iflags; > > @@ -54,10 +100,11 @@ static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu) > struct vgic_v3_cpu_if *host_cpu_if = &host_vcpu->arch.vgic_cpu.vgic_v3; > unsigned int i; > > + fpsimd_sve_sync(&hyp_vcpu->vcpu); > + > host_vcpu->arch.ctxt = hyp_vcpu->vcpu.arch.ctxt; > > host_vcpu->arch.hcr_el2 = hyp_vcpu->vcpu.arch.hcr_el2; > - host_vcpu->arch.cptr_el2 = hyp_vcpu->vcpu.arch.cptr_el2; > > host_vcpu->arch.fault = hyp_vcpu->vcpu.arch.fault; > > diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c > index 25e9a94f6d76..feb27b4ce459 100644 > --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c > +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c > @@ -588,6 +588,8 @@ int __pkvm_init_vcpu(pkvm_handle_t handle, struct kvm_vcpu *host_vcpu, > if (ret) > unmap_donated_memory(hyp_vcpu, sizeof(*hyp_vcpu)); > > + hyp_vcpu->vcpu.arch.cptr_el2 = kvm_get_reset_cptr_el2(&hyp_vcpu->vcpu); > + Eventually, we need to rename this to cpacr_el2 and make sure we consistently use the VHE format everywhere. I'm starting to be worried that we mix things badly. > return ret; > } > > diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c > index 019f863922fa..5bf437682b94 100644 > --- a/arch/arm64/kvm/hyp/nvhe/switch.c > +++ b/arch/arm64/kvm/hyp/nvhe/switch.c > @@ -184,7 +184,21 @@ static bool kvm_handle_pvm_sys64(struct kvm_vcpu *vcpu, u64 *exit_code) > > static void kvm_hyp_save_fpsimd_host(struct kvm_vcpu *vcpu) > { > - __fpsimd_save_state(*host_data_ptr(fpsimd_state)); > + /* > + * Non-protected kvm relies on the host restoring its sve state. > + * Protected kvm restores the host's sve state as not to reveal that > + * fpsimd was used by a guest nor leak upper sve bits. > + */ > + if (unlikely(is_protected_kvm_enabled() && system_supports_sve())) { > + __hyp_sve_save_host(); > + > + /* Re-enable SVE traps if not supported for the guest vcpu. */ > + if (!vcpu_has_sve(vcpu)) > + cpacr_clear_set(CPACR_EL1_ZEN_EL0EN | CPACR_EL1_ZEN_EL1EN, 0); > + nit: spurious new line Thanks, M. -- Without deviation from the norm, progress is not possible. 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 1B178C25B74 for ; Tue, 21 May 2024 22:52:47 +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=D44ALJmdM1IhGIxhB9DT4g80BdHCGJ1ajYYZg41WCGA=; b=uXkUfjsYW5nTz2 eDHRhKvfRv0f3GwREPUe+KElii7I3r4tL9C4FMIYmm8SZSL4nPsnBSYUXHhdp4PHboo5gaiwc+z6r iUL3G4XsQACkt81+pj+CbiK2YbST9wqAavJ3AhOqZJNGVqDhS/YJg70YgPTbB3eM5N1H8EbMkATHt hD6guj9HssHW6vQ/Myq11pDF52PU8WsWc+/EtsC9uVE/SzT7/QjNOQ6DxWZlo4LEkStMx+Ly5m22+ kI+Pks5nh+7JqXQivfsl3K7cF1rflX53bY+aJucqyMZm+l4jGxqeRp5cNcDU6y0R/HkBlC/9XyHPG cuRbNEncL0ZHZLRMwtKg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1s9YLS-00000001IC9-3dvu; Tue, 21 May 2024 22:52:34 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1s9YLP-00000001IBI-0l8y for linux-arm-kernel@lists.infradead.org; Tue, 21 May 2024 22:52:32 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id BA2E862614; Tue, 21 May 2024 22:52:25 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3C106C2BD11; Tue, 21 May 2024 22:52:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1716331945; bh=v0Vv5zq7d4TRDM+lN818VW2kqCrPhvgiGE11Ah2/8hk=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=hdM/qZecuqhnFgS4Sw8ZspzBCv+n9hsir9UrJ4ZYIX484qgO8f/Q5DUn8Qy+nO6oL 8EypYSUwhtmLwcWkWZs/a8IwKHpxyClXbWDmX3YKgNr1M/S3UpjI9/+/BojQy49St+ gWt0JPlb5XXOjPCtAtjQF1pvyyOq6CjjwzzTbchZCE3yfpYVKtvaPy4xP7gEjHBuWF VCpIDCd3bb9Q4+nGgFyKwU+JXkF3YIHiGwTUxTNkOs5wBBJ7Gtmjc39eAbwk4ZOJD4 VZRcQMCKqHY2cWtmowg6jn6s4tziR8a/GtLmSwWk5DqBiv/UBu1HNrSWsdTfnyX4Mc X1c73jErm4Ksg== 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 1s9YLG-00Eoi6-L2; Tue, 21 May 2024 23:52:22 +0100 Date: Tue, 21 May 2024 23:52:21 +0100 Message-ID: <86o78yn5ga.wl-maz@kernel.org> From: Marc Zyngier To: Fuad Tabba Cc: kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, will@kernel.org, qperret@google.com, seanjc@google.com, alexandru.elisei@arm.com, catalin.marinas@arm.com, philmd@linaro.org, james.morse@arm.com, suzuki.poulose@arm.com, oliver.upton@linux.dev, mark.rutland@arm.com, broonie@kernel.org, joey.gouly@arm.com, rananta@google.com, yuzenghui@huawei.com Subject: Re: [PATCH v2 6/7] KVM: arm64: Eagerly restore host fpsimd/sve state in pKVM In-Reply-To: <20240521163720.3812851-7-tabba@google.com> References: <20240521163720.3812851-1-tabba@google.com> <20240521163720.3812851-7-tabba@google.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.2 (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: tabba@google.com, kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, will@kernel.org, qperret@google.com, seanjc@google.com, alexandru.elisei@arm.com, catalin.marinas@arm.com, philmd@linaro.org, james.morse@arm.com, suzuki.poulose@arm.com, oliver.upton@linux.dev, mark.rutland@arm.com, broonie@kernel.org, joey.gouly@arm.com, rananta@google.com, yuzenghui@huawei.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-20240521_155231_348531_5012226A X-CRM114-Status: GOOD ( 36.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 On Tue, 21 May 2024 17:37:19 +0100, Fuad Tabba wrote: > > When running in protected mode we don't want to leak protected > guest state to the host, including whether a guest has used > fpsimd/sve. Therefore, eagerly restore the host state on guest > exit when running in protected mode, which happens only if the > guest has used fpsimd/sve. > > As a future optimisation, we could go back to lazy restoring > state at the host after exiting non-protected guests. No. This sort of things is way too invasive and would require mapping the VHE host data at EL2. If we need to optimise this crap, then we apply the same techniques as we use for guests. If that's good enough for the guests, that's good enough for the host. > > Signed-off-by: Fuad Tabba > --- > arch/arm64/kvm/hyp/include/hyp/switch.h | 13 +++++- > arch/arm64/kvm/hyp/nvhe/hyp-main.c | 53 +++++++++++++++++++++++-- > arch/arm64/kvm/hyp/nvhe/pkvm.c | 2 + > arch/arm64/kvm/hyp/nvhe/switch.c | 16 +++++++- > 4 files changed, 79 insertions(+), 5 deletions(-) > > diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h > index 1897b73e635c..2fa29bfec0b6 100644 > --- a/arch/arm64/kvm/hyp/include/hyp/switch.h > +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h > @@ -320,6 +320,16 @@ static inline void __hyp_sve_restore_guest(struct kvm_vcpu *vcpu) > write_sysreg_el1(__vcpu_sys_reg(vcpu, ZCR_EL1), SYS_ZCR); > } > > +static inline void __hyp_sve_save_host(void) > +{ > + struct user_sve_state *sve_state = *host_data_ptr(sve_state); > + > + sve_state->zcr_el1 = read_sysreg_el1(SYS_ZCR); > + sve_cond_update_zcr_vq(ZCR_ELx_LEN_MASK, SYS_ZCR_EL2); > + __sve_save_state(sve_state->sve_regs + sve_ffr_offset(kvm_host_sve_max_vl), > + &sve_state->fpsr); > +} > + > static void kvm_hyp_save_fpsimd_host(struct kvm_vcpu *vcpu); > > /* > @@ -356,7 +366,8 @@ static bool kvm_hyp_handle_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code) > > /* First disable enough traps to allow us to update the registers */ > reg = CPACR_EL1_FPEN_EL0EN | CPACR_EL1_FPEN_EL1EN; > - if (sve_guest) > + if (sve_guest || > + (is_protected_kvm_enabled() && system_supports_sve())) This looks really ugly. Why can't we just compute sve_guest to take these conditions into account? > reg |= CPACR_EL1_ZEN_EL0EN | CPACR_EL1_ZEN_EL1EN; > cpacr_clear_set(0, reg); > isb(); > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c > index b07d44484f42..f79f0f7b2759 100644 > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c > @@ -23,20 +23,66 @@ DEFINE_PER_CPU(struct kvm_nvhe_init_params, kvm_init_params); > > void __kvm_hyp_host_forward_smc(struct kvm_cpu_context *host_ctxt); > > +static void __hyp_sve_save_guest(struct kvm_vcpu *vcpu) > +{ > + __vcpu_sys_reg(vcpu, ZCR_EL1) = read_sysreg_el1(SYS_ZCR); > + sve_cond_update_zcr_vq(vcpu_sve_max_vq(vcpu) - 1, SYS_ZCR_EL2); > + __sve_save_state(vcpu_sve_pffr(vcpu), &vcpu->arch.ctxt.fp_regs.fpsr); > + sve_cond_update_zcr_vq(ZCR_ELx_LEN_MASK, SYS_ZCR_EL2); > +} > + > +static void __hyp_sve_restore_host(void) > +{ > + struct user_sve_state *sve_state = *host_data_ptr(sve_state); > + > + sve_cond_update_zcr_vq(ZCR_ELx_LEN_MASK, SYS_ZCR_EL2); > + __sve_restore_state(sve_state->sve_regs + sve_ffr_offset(kvm_host_sve_max_vl), This is what I was worried about in a previous patch. kvm_host_sve_max_vl represents the max VL across all CPUs. if CPU0 supports 128bit SVE and CPU1 256bit, the value is 256. but on CPU0, ZCR_ELx_LEN_MASK will results in using 128bit accesses, and the offset will be wrong. I can't convince myself that anything really goes wrong as long as we're consistent between save and restore, but that's at best ugly and needs extensive documenting. On top of that, a conditional update of ZCR_EL2 with ZCR_ELx_LEN_MASK is unlikely to be beneficial, since nobody implements 2k vectors. A direct write followed by an ISB is likely to be better. > + &sve_state->fpsr); > + write_sysreg_el1(sve_state->zcr_el1, SYS_ZCR); > +} > + > +static void fpsimd_sve_flush(void) > +{ > + *host_data_ptr(fp_owner) = FP_STATE_HOST_OWNED; > +} > + > +static void fpsimd_sve_sync(struct kvm_vcpu *vcpu) > +{ > + if (!guest_owns_fp_regs()) > + return; > + > + cpacr_clear_set(0, (CPACR_EL1_ZEN_EL1EN | CPACR_EL1_ZEN_EL0EN | > + CPACR_EL1_FPEN_EL1EN | CPACR_EL1_FPEN_EL0EN)); > + isb(); > + > + if (vcpu_has_sve(vcpu)) > + __hyp_sve_save_guest(vcpu); > + else > + __fpsimd_save_state(&vcpu->arch.ctxt.fp_regs); > + > + if (system_supports_sve()) > + __hyp_sve_restore_host(); > + else > + __fpsimd_restore_state(*host_data_ptr(fpsimd_state)); > + > + *host_data_ptr(fp_owner) = FP_STATE_HOST_OWNED; > +} > + > static void flush_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu) > { > struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu; > > + fpsimd_sve_flush(); > + > hyp_vcpu->vcpu.arch.ctxt = host_vcpu->arch.ctxt; > > hyp_vcpu->vcpu.arch.sve_state = kern_hyp_va(host_vcpu->arch.sve_state); > - hyp_vcpu->vcpu.arch.sve_max_vl = host_vcpu->arch.sve_max_vl; > + hyp_vcpu->vcpu.arch.sve_max_vl = min(host_vcpu->arch.sve_max_vl, kvm_host_sve_max_vl); > > hyp_vcpu->vcpu.arch.hw_mmu = host_vcpu->arch.hw_mmu; > > hyp_vcpu->vcpu.arch.hcr_el2 = host_vcpu->arch.hcr_el2; > hyp_vcpu->vcpu.arch.mdcr_el2 = host_vcpu->arch.mdcr_el2; > - hyp_vcpu->vcpu.arch.cptr_el2 = host_vcpu->arch.cptr_el2; > > hyp_vcpu->vcpu.arch.iflags = host_vcpu->arch.iflags; > > @@ -54,10 +100,11 @@ static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu) > struct vgic_v3_cpu_if *host_cpu_if = &host_vcpu->arch.vgic_cpu.vgic_v3; > unsigned int i; > > + fpsimd_sve_sync(&hyp_vcpu->vcpu); > + > host_vcpu->arch.ctxt = hyp_vcpu->vcpu.arch.ctxt; > > host_vcpu->arch.hcr_el2 = hyp_vcpu->vcpu.arch.hcr_el2; > - host_vcpu->arch.cptr_el2 = hyp_vcpu->vcpu.arch.cptr_el2; > > host_vcpu->arch.fault = hyp_vcpu->vcpu.arch.fault; > > diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c > index 25e9a94f6d76..feb27b4ce459 100644 > --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c > +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c > @@ -588,6 +588,8 @@ int __pkvm_init_vcpu(pkvm_handle_t handle, struct kvm_vcpu *host_vcpu, > if (ret) > unmap_donated_memory(hyp_vcpu, sizeof(*hyp_vcpu)); > > + hyp_vcpu->vcpu.arch.cptr_el2 = kvm_get_reset_cptr_el2(&hyp_vcpu->vcpu); > + Eventually, we need to rename this to cpacr_el2 and make sure we consistently use the VHE format everywhere. I'm starting to be worried that we mix things badly. > return ret; > } > > diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c > index 019f863922fa..5bf437682b94 100644 > --- a/arch/arm64/kvm/hyp/nvhe/switch.c > +++ b/arch/arm64/kvm/hyp/nvhe/switch.c > @@ -184,7 +184,21 @@ static bool kvm_handle_pvm_sys64(struct kvm_vcpu *vcpu, u64 *exit_code) > > static void kvm_hyp_save_fpsimd_host(struct kvm_vcpu *vcpu) > { > - __fpsimd_save_state(*host_data_ptr(fpsimd_state)); > + /* > + * Non-protected kvm relies on the host restoring its sve state. > + * Protected kvm restores the host's sve state as not to reveal that > + * fpsimd was used by a guest nor leak upper sve bits. > + */ > + if (unlikely(is_protected_kvm_enabled() && system_supports_sve())) { > + __hyp_sve_save_host(); > + > + /* Re-enable SVE traps if not supported for the guest vcpu. */ > + if (!vcpu_has_sve(vcpu)) > + cpacr_clear_set(CPACR_EL1_ZEN_EL0EN | CPACR_EL1_ZEN_EL1EN, 0); > + nit: spurious new line 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