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 7BA2712EBCC; Fri, 14 Jun 2024 11:14:34 +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=1718363674; cv=none; b=Wq6cuPjHHBOEGvM8mPe8kk7gXhkEaihI8PD2cP1QTNM6b9cVpGLM8VNnzTxUYXRH+EEHTPle5UvJ5Zeqdgiu9f8NdNIIwAyTkZ84vpB3XcDLZCuq39023gmJDUREwIyxfrColGhNxvnXmnnz2kbz0Nl5kiSXvc62L7nCB4ftUbw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718363674; c=relaxed/simple; bh=lhlmxNTzACpYsFyN2wyOKSAxA0zRyk6hLY+uw6e/V1U=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=jQ0jw/+CrMLf+IF1zKqjW1y7DzxA6jSS+4xgFKDB43L3X2hI2KnYpFIlRMwgiWLdf7vnPe6G42fl9UKjHK2C9NfxuVeNktuFQYPvs/N45xrkuOQzhU8ahHtb67NHjnxFESojQA6t+ojdZXUGipRx1NGLcw4ebZRfNovAwU1Rn2Q= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=GRKlFEPu; 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="GRKlFEPu" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0677FC2BD10; Fri, 14 Jun 2024 11:14:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1718363674; bh=lhlmxNTzACpYsFyN2wyOKSAxA0zRyk6hLY+uw6e/V1U=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=GRKlFEPu082dCB2u64JN73iI+mUjbqdENos+zhhF/FO+WPiauqQ84mBPWoMq2ytzq gpCrcJcqzbQ+TxhbuHozw3TisyzqmX/KjGt6C0R0SA9VcazRGi11L28Lt9AM5N3ats qkbe/qPCwXqS7MidpYZJFnKP61ejouJ2I4IHVXSPCuRCsAA1BJqOdxRW6XJzZTy+Lx mk5PXI1yu/iWEL7z6pGWeO/r8IFz9KpPnjJfsrHySZhu7ExTl2oLDf377UEg89uPOn /CJo6CuXLlCYdg8PmBUgEJCBTapU42yoOphePJvkFIkOwS/TkGMgPcP9ttkfsmmHfI yPup4ilFHnVyA== Received: from ip-185-104-136-29.ptr.icomera.net ([185.104.136.29] helo=wait-a-minute.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 1sI4t5-003tna-OH; Fri, 14 Jun 2024 12:14:31 +0100 Date: Fri, 14 Jun 2024 12:14:30 +0100 Message-ID: <87a5jn3hex.wl-maz@kernel.org> From: Marc Zyngier To: Oliver Upton Cc: kvmarm@lists.linux.dev, James Morse , Suzuki K Poulose , Zenghui Yu , kvm@vger.kernel.org, Fuad Tabba Subject: Re: [PATCH v2 05/15] KVM: arm64: nv: Load guest hyp's ZCR into EL1 state In-Reply-To: <20240613201756.3258227-6-oliver.upton@linux.dev> References: <20240613201756.3258227-1-oliver.upton@linux.dev> <20240613201756.3258227-6-oliver.upton@linux.dev> 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 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) Precedence: bulk X-Mailing-List: kvm@vger.kernel.org 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.104.136.29 X-SA-Exim-Rcpt-To: oliver.upton@linux.dev, kvmarm@lists.linux.dev, james.morse@arm.com, suzuki.poulose@arm.com, yuzenghui@huawei.com, kvm@vger.kernel.org, tabba@google.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false On Thu, 13 Jun 2024 21:17:46 +0100, Oliver Upton wrote: > > Load the guest hypervisor's ZCR_EL2 into the corresponding EL1 register > when restoring SVE state, as ZCR_EL2 affects the VL in the hypervisor > context. > > Signed-off-by: Oliver Upton > --- > arch/arm64/include/asm/kvm_host.h | 4 ++++ > arch/arm64/kvm/hyp/include/hyp/switch.h | 3 ++- > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 8170c04fde91..e01e6de414f1 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -844,6 +844,10 @@ struct kvm_vcpu_arch { > > #define vcpu_sve_max_vq(vcpu) sve_vq_from_vl((vcpu)->arch.sve_max_vl) > > +#define vcpu_sve_zcr_el1(vcpu) \ > + (unlikely(is_hyp_ctxt(vcpu)) ? __vcpu_sys_reg(vcpu, ZCR_EL2) : \ > + __vcpu_sys_reg(vcpu, ZCR_EL1)) > + I have the feeling this abstracts the access at the wrong level. It's not that it gives the wrong result, but it hides the register and is only concerned with the *value*. In turn, it makes the helper unusable with the *write* side, as shown in patch 7. > #define vcpu_sve_state_size(vcpu) ({ \ > size_t __size_ret; \ > unsigned int __vcpu_vq; \ > diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h > index 5ecd2600d9df..71a93e336a0c 100644 > --- a/arch/arm64/kvm/hyp/include/hyp/switch.h > +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h > @@ -317,7 +317,8 @@ static inline void __hyp_sve_restore_guest(struct kvm_vcpu *vcpu) > sve_cond_update_zcr_vq(vcpu_sve_max_vq(vcpu) - 1, SYS_ZCR_EL2); > __sve_restore_state(vcpu_sve_pffr(vcpu), > &vcpu->arch.ctxt.fp_regs.fpsr); > - write_sysreg_el1(__vcpu_sys_reg(vcpu, ZCR_EL1), SYS_ZCR); > + > + write_sysreg_el1(vcpu_sve_zcr_el1(vcpu), SYS_ZCR); > } > > /* I would instead propose the following: diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index aeb1c567dfad..2c3eff0031eb 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -845,9 +845,8 @@ struct kvm_vcpu_arch { #define vcpu_sve_max_vq(vcpu) sve_vq_from_vl((vcpu)->arch.sve_max_vl) -#define vcpu_sve_zcr_el1(vcpu) \ - (unlikely(is_hyp_ctxt(vcpu)) ? __vcpu_sys_reg(vcpu, ZCR_EL2) : \ - __vcpu_sys_reg(vcpu, ZCR_EL1)) +#define vcpu_sve_zcr_elx(vcpu) \ + (unlikely(is_hyp_ctxt(vcpu)) ? ZCR_EL2 : ZCR_EL1) #define vcpu_sve_state_size(vcpu) ({ \ size_t __size_ret; \ diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c index bb2ef3166c63..947486a111e1 100644 --- a/arch/arm64/kvm/fpsimd.c +++ b/arch/arm64/kvm/fpsimd.c @@ -179,10 +179,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu) * If the vCPU is in the hyp context then ZCR_EL1 is * loaded with its vEL2 counterpart. */ - if (is_hyp_ctxt(vcpu)) - __vcpu_sys_reg(vcpu, ZCR_EL2) = zcr; - else - __vcpu_sys_reg(vcpu, ZCR_EL1) = zcr; + __vcpu_sys_reg(vcpu, vcpu_sve_zcr_elx(vcpu)) = zcr; /* * Restore the VL that was saved when bound to the CPU, diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h index 0a6935a18490..ad8dec0b450b 100644 --- a/arch/arm64/kvm/hyp/include/hyp/switch.h +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h @@ -330,7 +330,7 @@ static inline void __hyp_sve_restore_guest(struct kvm_vcpu *vcpu) if (vcpu_has_nv(vcpu) && !is_hyp_ctxt(vcpu)) sve_cond_update_zcr_vq(__vcpu_sys_reg(vcpu, ZCR_EL2), SYS_ZCR_EL2); - write_sysreg_el1(vcpu_sve_zcr_el1(vcpu), SYS_ZCR); + write_sysreg_el1(__vcpu_sys_reg(vcpu, vcpu_sve_zcr_elx(vcpu)), SYS_ZCR); } /* which makes the helper select the correct guest register for the context, and only that. In turn, the write side is much cleaner and symmetry is restored. Thanks, M. -- Without deviation from the norm, progress is not possible.