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 7D213C433EF for ; Mon, 22 Nov 2021 15:59:00 +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=Q78Tp/fY3NmK/NVVSpct6kcIzmQdK7v6S3bf2XhjPjQ=; b=IoIzGXSBFnheN/ SN7rh6c4KDDLn87SWDgYW8Mj3lbIzfU4qvIoJOmAzwjCiuQ+L2clCXoAsnIZgi14/qQ+i7D9T+Iaj OyrVshb5Tk9UTtnk551J57NpsaIMAwmCcCnnWcAl8TA97QnJ2/8ZcoSApWG/FPSk2gPeBRJUTrqXQ SodjTs9LduHt5W5rCc6pZQ5ZGNFk2afQ1SzDWvNhX50i22ntAsOy+Hsa57AnOA23bnz8oSTNtdMT4 CYrimpIXelYaSiJC1vX2gEU+NwjFASWSa4wM1iOCyLdJAbXMbu2c9JEJJw0Ntg8o83KYkLpDKaEqF y22C595JOFDeT7bv+8rg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mpBhS-00H6PR-Dr; Mon, 22 Nov 2021 15:57:46 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mpBhO-00H6Op-B3 for linux-arm-kernel@lists.infradead.org; Mon, 22 Nov 2021 15:57:43 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 0498D60D07; Mon, 22 Nov 2021 15:57:42 +0000 (UTC) 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 1mpBhL-0075am-OG; Mon, 22 Nov 2021 15:57:39 +0000 Date: Mon, 22 Nov 2021 15:57:32 +0000 Message-ID: <871r38dvyr.wl-maz@kernel.org> From: Marc Zyngier To: Zenghui Yu Cc: , , , James Morse , Suzuki K Poulose , Alexandru Elisei , Quentin Perret , Will Deacon , , Subject: Re: [PATCH v2 2/5] KVM: arm64: Get rid of host SVE tracking/saving In-Reply-To: <5ab3836f-2b39-2ff5-3286-8258addd01e4@huawei.com> References: <20211028111640.3663631-1-maz@kernel.org> <20211028111640.3663631-3-maz@kernel.org> <5ab3836f-2b39-2ff5-3286-8258addd01e4@huawei.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/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: yuzenghui@huawei.com, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, james.morse@arm.com, suzuki.poulose@arm.com, alexandru.elisei@arm.com, qperret@google.com, will@kernel.org, broonie@kernel.org, kernel-team@android.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-20211122_075742_441603_207A0519 X-CRM114-Status: GOOD ( 35.11 ) 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 Zenghui, On Wed, 10 Nov 2021 13:19:23 +0000, Zenghui Yu wrote: > > Hi Marc, > > On 2021/10/28 19:16, Marc Zyngier wrote: > > The SVE host tracking in KVM is pretty involved. It relies on a > > set of flags tracking the ownership of the SVE register, as well > > as that of the EL0 access. > > > > It is also pretty scary: __hyp_sve_save_host() computes > > a thread_struct pointer and obtains a sve_state which gets directly > > accessed without further ado, even on nVHE. How can this even work? > > > > The answer to that is that it doesn't, and that this is mostly dead > > code. Closer examination shows that on executing a syscall, userspace > > loses its SVE state entirely. This is part of the ABI. Another > > thing to notice is that although the kernel provides helpers such as > > kernel_neon_begin()/end(), they only deal with the FP/NEON state, > > and not SVE. > > > > Given that you can only execute a guest as the result of a syscall, > > and that the kernel cannot use SVE by itself, it becomes pretty > > obvious that there is never any host SVE state to save, and that > > this code is only there to increase confusion. > > > > Get rid of the TIF_SVE tracking and host save infrastructure altogether. > > > > Signed-off-by: Marc Zyngier > > diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c > > index 5621020b28de..38ca332c10fe 100644 > > --- a/arch/arm64/kvm/fpsimd.c > > +++ b/arch/arm64/kvm/fpsimd.c > > @@ -73,15 +73,11 @@ int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu) > > void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu) > > { > > BUG_ON(!current->mm); > > + BUG_ON(test_thread_flag(TIF_SVE)); > > - vcpu->arch.flags &= ~(KVM_ARM64_FP_ENABLED | > > - KVM_ARM64_HOST_SVE_IN_USE | > > - KVM_ARM64_HOST_SVE_ENABLED); > > + vcpu->arch.flags &= ~KVM_ARM64_FP_ENABLED; > > vcpu->arch.flags |= KVM_ARM64_FP_HOST; > > - if (test_thread_flag(TIF_SVE)) > > - vcpu->arch.flags |= KVM_ARM64_HOST_SVE_IN_USE; > > The comment about TIF_SVE on top of kvm_arch_vcpu_load_fp() becomes > obsolete now. Maybe worth removing it? > > | * > | * TIF_SVE is backed up here, since it may get clobbered with guest state. > | * This flag is restored by kvm_arch_vcpu_put_fp(vcpu). Indeed. Now gone. > > > diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h > > index a0e78a6027be..722dfde7f1aa 100644 > > --- a/arch/arm64/kvm/hyp/include/hyp/switch.h > > +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h > > @@ -207,16 +207,6 @@ static inline bool __populate_fault_info(struct kvm_vcpu *vcpu) > > return __get_fault_info(esr, &vcpu->arch.fault); > > } > > -static inline void __hyp_sve_save_host(struct kvm_vcpu *vcpu) > > -{ > > - struct thread_struct *thread; > > - > > - thread = container_of(vcpu->arch.host_fpsimd_state, struct thread_struct, > > - uw.fpsimd_state); > > - > > - __sve_save_state(sve_pffr(thread), &vcpu->arch.host_fpsimd_state->fpsr); > > -} > > Nit: This removes the only user of __sve_save_state() helper. Should we > still keep it in fpsimd.S? I was in two minds about that, as I'd like to eventually be able to use SVE for protected guests, where the hypervisor itself has to be in charge of the FP/SVE save-restore. But that's probably several months away, and I can always revert a deletion patch if I need to, so let's get rid of it now. Thanks for the suggestions. 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