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 C0885EE57CD for ; Sun, 10 Sep 2023 11:46:38 +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=ZQvjigo7rez1kz61Lgiezfpav2KBniexVsRiXdTRo34=; b=aETLsQCyD24I6t +JQCQ6r8PC5ZWmkJMyuIAbljJhQTrTbhgstw9dlpE64Tfbr+JbP4ovjQnvIje7X/NUzCwvyi5vbR1 N7+VrGlyAxCu0FdRKagY8GiD6fqdsfMLFNYpV2Q7/CYz+NiJtcvLZco7P3FkdMaVEkAhMlQo4XeqV IfV40ae/fYcYvMOJ9HgMxAKmyx1Y/Q+wI0AfqvQZ9n7ycproFHh56eTNNaA6EksmlT0AABeCHANle r2P8UcArgqN4tLu7toIXNXQhj6dH+9a65l9xxb9seIvqskxglZhOTUeQWQYdrVg/K0/rs4FUA/kmR du15W8GziB3VlSIbj4vA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qfItJ-00GZJk-0R; Sun, 10 Sep 2023 11:46:13 +0000 Received: from sin.source.kernel.org ([2604:1380:40e1:4800::1]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qfItE-00GZJN-0q for linux-arm-kernel@lists.infradead.org; Sun, 10 Sep 2023 11:46:11 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by sin.source.kernel.org (Postfix) with ESMTPS id D3282CE09AA; Sun, 10 Sep 2023 11:46:00 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CCE92C433C8; Sun, 10 Sep 2023 11:45:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694346359; bh=1fPgt6NqjDOo0BmZtv5QdNaLWSnnSlPoyiIVdfmQyi0=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=QDDaexTUFDKdmVvCWhEPitc7stGlgZIlrhwXVBJ5Mqj3fyZCEiErhXvUZ7Ixv6SWR 2ivotYuf1mPueOcrNd+CDiOZIw3PfMA/thrgMr3pGeleB+3DFUB6uye/THSw260Y0f NtPpSYV7n5+n8KHXBYGxqvwLrmsMFxs7NL/yIkrGOJkuC3FpCj0FMq6irdpMrXDBuP oqzTefuCrGVz54cgYtcgkLThrkafS53HzlDe7xFb65cfCDXsh4tDixjdEf5rE6Jjam nXKyYAaqZCh92qpq55WTnXpmYK18FUrRwNmihbCvlqDy7XzCD5fBqepy5PRiwrk0gy d189TXbKxNN/g== 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 1qfIt2-00Bipc-3b; Sun, 10 Sep 2023 12:45:56 +0100 Date: Sun, 10 Sep 2023 12:45:55 +0100 Message-ID: <86bkeadzf0.wl-maz@kernel.org> From: Marc Zyngier To: Ganapatrao Kulkarni Cc: linux-kernel@vger.kernel.org, kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, Christoffer.Dall@arm.com, eauger@redhat.com, miguel.luis@oracle.com, darren@os.amperecomputing.com, scott@os.amperecomputing.com Subject: Re: [PATCH v2 2/2] KVM: arm64: timers: Save restore CVAL of a ptimer across guest entry and exits In-Reply-To: <20230904114218.590304-3-gankulkarni@os.amperecomputing.com> References: <20230904114218.590304-1-gankulkarni@os.amperecomputing.com> <20230904114218.590304-3-gankulkarni@os.amperecomputing.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/28.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: gankulkarni@os.amperecomputing.com, linux-kernel@vger.kernel.org, kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, Christoffer.Dall@arm.com, eauger@redhat.com, miguel.luis@oracle.com, darren@os.amperecomputing.com, scott@os.amperecomputing.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-20230910_044608_687764_A7D818E6 X-CRM114-Status: GOOD ( 36.59 ) 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 Mon, 04 Sep 2023 12:42:18 +0100, Ganapatrao Kulkarni wrote: > > As per FEAT_ECV, when HCR_EL2.{E2H, TGE} == {1, 1}, Enhanced Counter > Virtualization functionality is disabled and CNTPOFF_EL2 value is treated > as zero. On VHE host, E2H and TGE are set, hence it is required > to save Guest's CVAL to memory and increment it by CNTPOFF_EL2 at > guest exit to avoid false physical timer interrupts and also > restore back the CVAL with saved value before the guest entry. > > Signed-off-by: Ganapatrao Kulkarni > --- > arch/arm64/kvm/arch_timer.c | 60 ++++++++++++++++++++++++++++++--- > arch/arm64/kvm/hyp/vhe/switch.c | 13 +++++++ > include/kvm/arm_arch_timer.h | 1 + > 3 files changed, 69 insertions(+), 5 deletions(-) > > diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c > index 98b0e8ac02ae..9fe3fa6ed98a 100644 > --- a/arch/arm64/kvm/arch_timer.c > +++ b/arch/arm64/kvm/arch_timer.c > @@ -514,6 +514,59 @@ static void set_cntpoff(u64 cntpoff) > write_sysreg_s(cntpoff, SYS_CNTPOFF_EL2); > } > > +static void ptimer_cval_save(struct arch_timer_context *ctx, u64 offset) > +{ > + unsigned long flags; > + u64 cval; > + > + local_irq_save(flags); > + cval = read_sysreg_el0(SYS_CNTP_CVAL); > + timer_set_cval(ctx, cval); > + cval += offset; > + write_sysreg_el0(cval, SYS_CNTP_CVAL); > + isb(); > + local_irq_restore(flags); > +} > + > +static void ptimer_cval_restore(struct arch_timer_context *ctx, u64 offset) > +{ > + unsigned long flags; > + u64 cval; > + > + local_irq_save(flags); > + cval = timer_get_cval(ctx); > + write_sysreg_el0(cval, SYS_CNTP_CVAL); > + isb(); > + local_irq_restore(flags); > +} > + > +void kvm_ptimer_cval_save_restore(struct kvm_vcpu *vcpu, bool save) > +{ > + struct timer_map map; > + struct arch_timer_cpu *timer; > + struct arch_timer_context *ctxp; > + u64 offset; > + > + get_timer_map(vcpu, &map); > + ctxp = map.direct_ptimer; > + > + if (unlikely(ctxp == NULL)) > + return; > + > + offset = timer_get_offset(ctxp); > + if (!offset) > + return; > + > + timer = vcpu_timer(ctxp->vcpu); > + if (!timer->enabled || !ctxp->loaded) > + return; > + > + if (save) > + ptimer_cval_save(ctxp, offset); > + else > + ptimer_cval_restore(ctxp, offset); > +} > + I really don't want to see any of this code in the arch_timer backend. There is nothing here that executes in the context of the world-switch until this, and I think this is the wrong level of abstraction. You end-up doing things that make no sense in the expected execution context (timer not loaded?, not enabled?, disabling interrupts?), and this makes the whole thing pointlessly complex. My take on this problem is still the same (vaguely amended compared to the previous version). If this doesn't work for you, please explain what is wrong with it. But it has the shape of what I really want to see. Thanks, M. >From 2516a1410c0d45a7d3ba0523847be339bfcb1e50 Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Tue, 22 Aug 2023 13:18:10 +0100 Subject: [PATCH] KVM: arm64: timers: Correctly handle TGE flip with CNTPOFF_EL2 Contrary to common belief, HCR_EL2.TGE has a direct and immediate effect on the way the EL0 physical counter is offset. Flipping TGE from 1 to 0 while at EL2 immediately changes the way the counter compared to the CVAL limit. This means that we cannot directly save/restore the guest's view of CVAL, but that we instead must treat it as if CNTPOFF didn't exist. Only in the world switch, once we figure out that we do have CNTPOFF, can we must the offset back and forth depending on the polarity of TGE. Fixes: 2b4825a86940 ("KVM: arm64: timers: Use CNTPOFF_EL2 to offset the physical timer") Reported-by: Ganapatrao Kulkarni Signed-off-by: Marc Zyngier --- arch/arm64/kvm/arch_timer.c | 13 +++------- arch/arm64/kvm/hyp/vhe/switch.c | 45 +++++++++++++++++++++++++++++++++ include/kvm/arm_arch_timer.h | 7 +++++ 3 files changed, 55 insertions(+), 10 deletions(-) diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c index 6dcdae4d38cb..a1e24228aaaa 100644 --- a/arch/arm64/kvm/arch_timer.c +++ b/arch/arm64/kvm/arch_timer.c @@ -55,11 +55,6 @@ static struct irq_ops arch_timer_irq_ops = { .get_input_level = kvm_arch_timer_get_input_level, }; -static bool has_cntpoff(void) -{ - return (has_vhe() && cpus_have_final_cap(ARM64_HAS_ECV_CNTPOFF)); -} - static int nr_timers(struct kvm_vcpu *vcpu) { if (!vcpu_has_nv(vcpu)) @@ -180,7 +175,7 @@ u64 kvm_phys_timer_read(void) return timecounter->cc->read(timecounter->cc); } -static void get_timer_map(struct kvm_vcpu *vcpu, struct timer_map *map) +void get_timer_map(struct kvm_vcpu *vcpu, struct timer_map *map) { if (vcpu_has_nv(vcpu)) { if (is_hyp_ctxt(vcpu)) { @@ -548,8 +543,7 @@ static void timer_save_state(struct arch_timer_context *ctx) timer_set_ctl(ctx, read_sysreg_el0(SYS_CNTP_CTL)); cval = read_sysreg_el0(SYS_CNTP_CVAL); - if (!has_cntpoff()) - cval -= timer_get_offset(ctx); + cval -= timer_get_offset(ctx); timer_set_cval(ctx, cval); @@ -636,8 +630,7 @@ static void timer_restore_state(struct arch_timer_context *ctx) cval = timer_get_cval(ctx); offset = timer_get_offset(ctx); set_cntpoff(offset); - if (!has_cntpoff()) - cval += offset; + cval += offset; write_sysreg_el0(cval, SYS_CNTP_CVAL); isb(); write_sysreg_el0(timer_get_ctl(ctx), SYS_CNTP_CTL); diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c index 6537f58b1a8c..5dcbde57f466 100644 --- a/arch/arm64/kvm/hyp/vhe/switch.c +++ b/arch/arm64/kvm/hyp/vhe/switch.c @@ -39,6 +39,26 @@ static void __activate_traps(struct kvm_vcpu *vcpu) ___activate_traps(vcpu); + if (has_cntpoff()) { + struct timer_map map; + + get_timer_map(vcpu, &map); + + /* + * We're entrering the guest. Reload the correct + * values from memory now that TGE is clear. + */ + if (map.direct_ptimer == vcpu_ptimer(vcpu)) + val = __vcpu_sys_reg(vcpu, CNTP_CVAL_EL0); + if (map.direct_ptimer == vcpu_hptimer(vcpu)) + val = __vcpu_sys_reg(vcpu, CNTHP_CVAL_EL2); + + if (map.direct_ptimer) { + write_sysreg_s(val, SYS_CNTP_CVAL_EL0); + isb(); + } + } + val = read_sysreg(cpacr_el1); val |= CPACR_ELx_TTA; val &= ~(CPACR_EL1_ZEN_EL0EN | CPACR_EL1_ZEN_EL1EN | @@ -77,6 +97,31 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu) write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2); + if (has_cntpoff()) { + struct timer_map map; + u64 val, offset; + + get_timer_map(vcpu, &map); + + /* + * We're exiting the guest. Save the latest CVAL value + * to memory and apply the offset now that TGE is set. + */ + val = read_sysreg_s(SYS_CNTP_CVAL_EL0); + if (map.direct_ptimer == vcpu_ptimer(vcpu)) + __vcpu_sys_reg(vcpu, CNTP_CVAL_EL0) = val; + if (map.direct_ptimer == vcpu_hptimer(vcpu)) + __vcpu_sys_reg(vcpu, CNTHP_CVAL_EL2) = val; + + offset = read_sysreg_s(SYS_CNTPOFF_EL2); + + if (map.direct_ptimer && offset) { + offset = read_sysreg_s(SYS_CNTPOFF_EL2); + write_sysreg_s(val + offset, SYS_CNTP_CVAL_EL0); + isb(); + } + } + /* * ARM errata 1165522 and 1530923 require the actual execution of the * above before we can switch to the EL2/EL0 translation regime used by diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h index bb3cb005873e..e748bc957d83 100644 --- a/include/kvm/arm_arch_timer.h +++ b/include/kvm/arm_arch_timer.h @@ -82,6 +82,8 @@ struct timer_map { struct arch_timer_context *emul_ptimer; }; +void get_timer_map(struct kvm_vcpu *vcpu, struct timer_map *map); + struct arch_timer_cpu { struct arch_timer_context timers[NR_KVM_TIMERS]; @@ -145,4 +147,9 @@ u64 timer_get_cval(struct arch_timer_context *ctxt); void kvm_timer_cpu_up(void); void kvm_timer_cpu_down(void); +static inline bool has_cntpoff(void) +{ + return (has_vhe() && cpus_have_final_cap(ARM64_HAS_ECV_CNTPOFF)); +} + #endif -- 2.34.1 -- 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