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 C1FA4126C02 for ; Sat, 9 Nov 2024 12:28:51 +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=1731155331; cv=none; b=mm88gqppBZkmtHP3xPT9ApzC0EI/0QMaQnEsizePHcPg1NiQJhY/24JtftfpcFihfHNw0qbg3eIHDVbmuBr+V94qh9t034aEQBMP1ixrLkOppEw1DfEC46oX6h7SdCF7tk/rglrG9iPDxH3n1PSSsakpxyNtzVyYR1ySjBJ47/I= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731155331; c=relaxed/simple; bh=EhIT4EPy2Z+pW/7+vA/P37NbId564gsaYC3btH4wjSM=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=JSYE46/bTj4P+pQi92/ziLv97+5HeDW04EfBFSO4w6uPTpv8GmRrEXBPlVQkinbyuqgSH2qxILFML/fH5lc7mnwUl4VQi6mx6eY7jpxirbVWTCpTeF4Rz/Ez8bYN1hLMVnQfBslVCQyG8r6TNR2GG7iHQ3S//a7l3wM9IEZrjkw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=F1QzTAU2; 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="F1QzTAU2" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 443C9C4CECF; Sat, 9 Nov 2024 12:28:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1731155331; bh=EhIT4EPy2Z+pW/7+vA/P37NbId564gsaYC3btH4wjSM=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=F1QzTAU2N1rXkHxHgoWxlNGUNLdyaS7EyVpYpheybSXdRc6/OYPzDynzgRq2AoRiX q5wapbv3pZDCbu74sbu50FnCuYfpPciEbtd8Ev+QA1c8d/YxLoCoAUoqqkzSuYsZvf Km9LRf/yqGWdgez6wTIX1Qy0YeBdQ2LFfvTKBacvX+80SWTEML1Mw88ah7VYPUmB7o JSjxlI66/XBS/yP8sN2mOO+ba11XYhQYwyCXXFIiJ1pGIrpTXipWwNFx4rokiN2LRs 11F0/ocge0EddmsbXadhvr41dK7TghDE3TteGcXxS6adxeUGK2nOFsRpdNK5a/Hq0j wO5m38sG9Dsxg== 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 1t9ka8-00BPuA-8i; Sat, 09 Nov 2024 12:28:49 +0000 Date: Sat, 09 Nov 2024 12:28:47 +0000 Message-ID: <86wmhczj40.wl-maz@kernel.org> From: Marc Zyngier To: Oliver Upton Cc: kvmarm@lists.linux.dev, Joey Gouly , Suzuki K Poulose , Zenghui Yu , Mingwei Zhang , Colton Lewis , Alexandru Elisei Subject: Re: [PATCH 13/15] KVM: arm64: Compute MDCR_EL2 at vcpu_load() In-Reply-To: <20241108222418.1677420-14-oliver.upton@linux.dev> References: <20241108222418.1677420-1-oliver.upton@linux.dev> <20241108222418.1677420-14-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/29.4 (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: oliver.upton@linux.dev, kvmarm@lists.linux.dev, joey.gouly@arm.com, suzuki.poulose@arm.com, yuzenghui@huawei.com, mizhang@google.com, coltonlewis@google.com, alexandru.elisei@arm.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false On Fri, 08 Nov 2024 22:24:17 +0000, Oliver Upton wrote: > > KVM has picked up several hacks to cope with vcpu->arch.mdcr_el2 needing > to be prepared before vcpu_load(), which is when it gets programmed > into hardware on VHE. > > Now that the flows for reprogramming MDCR_EL2 have been simplified, move > that computation to vcpu_load(). > > Signed-off-by: Oliver Upton > --- > arch/arm64/include/asm/kvm_host.h | 1 - > arch/arm64/kvm/arm.c | 2 -- > arch/arm64/kvm/debug.c | 28 +++++++--------------------- > 3 files changed, 7 insertions(+), 24 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 91341fb3405d..f8c7e37180ab 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -1341,7 +1341,6 @@ static inline bool kvm_system_needs_idmapped_vectors(void) > static inline void kvm_arch_sync_events(struct kvm *kvm) {} > > void kvm_init_host_debug_data(void); > -void kvm_arm_vcpu_init_debug(struct kvm_vcpu *vcpu); > void kvm_arm_setup_debug(struct kvm_vcpu *vcpu); > void kvm_arm_clear_debug(struct kvm_vcpu *vcpu); > void kvm_vcpu_load_debug(struct kvm_vcpu *vcpu); > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 44a6093b0d9e..ec581aeb41f9 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -804,8 +804,6 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu) > > kvm_init_mpidr_data(kvm); > > - kvm_arm_vcpu_init_debug(vcpu); > - > if (likely(irqchip_in_kernel(kvm))) { > /* > * Map the VGIC hardware resources before running a vcpu the > diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c > index 04a62660ca5b..9ac237c3ae0c 100644 > --- a/arch/arm64/kvm/debug.c > +++ b/arch/arm64/kvm/debug.c > @@ -91,20 +91,6 @@ static void kvm_arm_setup_mdcr_el2(struct kvm_vcpu *vcpu) > vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA; > } > > -/** > - * kvm_arm_vcpu_init_debug - setup vcpu debug traps > - * > - * @vcpu: the vcpu pointer > - * > - * Set vcpu initial mdcr_el2 value. > - */ > -void kvm_arm_vcpu_init_debug(struct kvm_vcpu *vcpu) > -{ > - preempt_disable(); > - kvm_arm_setup_mdcr_el2(vcpu); > - preempt_enable(); > -} > - > /** > * kvm_arm_setup_debug - set up debug related stuff > * > @@ -122,12 +108,10 @@ void kvm_arm_vcpu_init_debug(struct kvm_vcpu *vcpu) > > void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) > { > - unsigned long mdscr, orig_mdcr_el2 = vcpu->arch.mdcr_el2; > + unsigned long mdscr; > > trace_kvm_arm_setup_debug(vcpu, vcpu->guest_debug); > > - kvm_arm_setup_mdcr_el2(vcpu); > - > /* Check if we need to use the debug registers. */ > if (vcpu->guest_debug || kvm_vcpu_os_lock_enabled(vcpu)) { > /* Save guest debug state */ > @@ -197,10 +181,6 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) > vcpu_write_sys_reg(vcpu, mdscr, MDSCR_EL1); > } > } > - > - /* Write mdcr_el2 changes since vcpu_load on VHE systems */ > - if (has_vhe() && orig_mdcr_el2 != vcpu->arch.mdcr_el2) > - write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2); > } > > void kvm_arm_clear_debug(struct kvm_vcpu *vcpu) > @@ -265,6 +245,8 @@ void kvm_vcpu_load_debug(struct kvm_vcpu *vcpu) > else > vcpu->arch.debug_owner = VCPU_DEBUG_FREE; > } > + > + kvm_arm_setup_mdcr_el2(vcpu); > } > > void kvm_handle_debug_access(struct kvm_vcpu *vcpu) > @@ -274,6 +256,10 @@ void kvm_handle_debug_access(struct kvm_vcpu *vcpu) > > WARN_ON_ONCE(vcpu->arch.debug_owner == VCPU_DEBUG_GUEST_OWNED); > vcpu->arch.debug_owner = VCPU_DEBUG_GUEST_OWNED; > + kvm_arm_setup_mdcr_el2(vcpu); > + > + if (has_vhe()) > + write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2); OK, now I see this function does a bit more than just ownership. Oh well. But it now also appears that vcpu->arch.mdcr_el2 isn't part of the vcpu state. It really is host state that we manage for the sake of emulation. How about kicking it where it belongs? It would definitely help understanding the lifetime of this piece of state. Thanks, M. -- Without deviation from the norm, progress is not possible.