From patchwork Mon Jan 14 22:08:39 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 1973911 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id B18B8DF2E1 for ; Mon, 14 Jan 2013 22:11:49 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1TusCy-0008P4-5w; Mon, 14 Jan 2013 22:08:44 +0000 Received: from mail-ie0-f181.google.com ([209.85.223.181]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1TusCu-0008Ok-G2 for linux-arm-kernel@lists.infradead.org; Mon, 14 Jan 2013 22:08:40 +0000 Received: by mail-ie0-f181.google.com with SMTP id 16so5938141iea.26 for ; Mon, 14 Jan 2013 14:08:39 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:x-originating-ip:in-reply-to:references:date :message-id:subject:from:to:cc:content-type:x-gm-message-state; bh=/Ptm8x6Pcn2+Jt/q/O1y+ewOoSkRhI+LVMi1MxOJzK0=; b=Ly1eM7LLtsMm2UcwtCrNJTE0RtxwOU2CnoigalwQ6Kh43IbV9lf44kan1eR7ve3uM5 hy/2ayHDfKYIzoc85MrSDy2wPYQVsLvLmqNesWK76c6tLJ5Kzqyb0YtNcSCA6oTaPTLm ZKe6p5pAkhaisIcL+lksYTyiD9CrTP3id094ULJ8XEk+3uvjJdUM0e2o1mg4Ib7F6+TQ 76W6DW6JfH9qMIkgI/R8cahhjsLfvQa7+1bVboZFM/G/3a+jyUvNTCL/XaGhVfeKgnMM iYZ+Q7UCF6wke4chKHW4IUjRSZH2G4zpNvV1gvTBJNWLB8uH0+C4xye4eb1fPRDg0z1g hNlw== MIME-Version: 1.0 Received: by 10.50.36.198 with SMTP id s6mr51831igj.23.1358201319116; Mon, 14 Jan 2013 14:08:39 -0800 (PST) Received: by 10.64.37.70 with HTTP; Mon, 14 Jan 2013 14:08:39 -0800 (PST) X-Originating-IP: [72.80.83.148] In-Reply-To: <50F445BC.6050507@arm.com> References: <20130108184259.46758.17939.stgit@ubuntu> <20130108184327.46758.70599.stgit@ubuntu> <20130114152101.GD18935@mudshark.cambridge.arm.com> <50F445BC.6050507@arm.com> Date: Mon, 14 Jan 2013 17:08:39 -0500 Message-ID: Subject: Re: [PATCH v5 3/4] ARM: KVM: arch_timers: Add timer world switch From: Christoffer Dall To: Marc Zyngier X-Gm-Message-State: ALoCoQnw5hTy6kjcHzP51LJ5xI8soOjoVCrvAIGyjQ284UEsbnluqo+cNghSGB9M/LRvXLotJdkV X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130114_170840_639501_72827DD3 X-CRM114-Status: GOOD ( 12.93 ) X-Spam-Score: -2.6 (--) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-2.6 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [209.85.223.181 listed in list.dnswl.org] -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Will Deacon , "linux-arm-kernel@lists.infradead.org" , "kvm@vger.kernel.org" , "kvmarm@lists.cs.columbia.edu" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On Mon, Jan 14, 2013 at 12:51 PM, Marc Zyngier wrote: > On 14/01/13 15:21, Will Deacon wrote: >> On Tue, Jan 08, 2013 at 06:43:27PM +0000, Christoffer Dall wrote: >>> From: Marc Zyngier >>> >>> Do the necessary save/restore dance for the timers in the world >>> switch code. In the process, allow the guest to read the physical >>> counter, which is useful for its own clock_event_device. >> >> [...] >> >>> @@ -476,6 +513,7 @@ vcpu .req r0 @ vcpu pointer always in r0 >>> * for the host. >>> * >>> * Assumes vcpu pointer in vcpu reg >>> + * Clobbers r2-r4 >>> */ >>> .macro restore_timer_state >>> @ Disallow physical timer access for the guest >>> @@ -484,6 +522,30 @@ vcpu .req r0 @ vcpu pointer always in r0 >>> orr r2, r2, #CNTHCTL_PL1PCTEN >>> bic r2, r2, #CNTHCTL_PL1PCEN >>> mcr p15, 4, r2, c14, c1, 0 @ CNTHCTL >>> + >>> +#ifdef CONFIG_KVM_ARM_TIMER >>> + ldr r4, [vcpu, #VCPU_KVM] >>> + ldr r2, [r4, #KVM_TIMER_ENABLED] >>> + cmp r2, #0 >>> + beq 1f >>> + >>> + ldr r2, [r4, #KVM_TIMER_CNTVOFF] >>> + ldr r3, [r4, #(KVM_TIMER_CNTVOFF + 4)] >>> + mcrr p15, 4, r2, r3, c14 @ CNTVOFF >>> + isb >>> + >>> + ldr r4, =VCPU_TIMER_CNTV_CVAL >>> + add vcpu, vcpu, r4 >>> + ldrd r2, r3, [vcpu] >>> + sub vcpu, vcpu, r4 >>> + mcrr p15, 3, r2, r3, c14 @ CNTV_CVAL >>> + >>> + ldr r2, [vcpu, #VCPU_TIMER_CNTV_CTL] >>> + and r2, r2, #3 >>> + mcr p15, 0, r2, c14, c3, 1 @ CNTV_CTL >>> + isb >> >> How many of these isbs are actually needed, given that we're going to make >> an exception return to the guest? The last one certainly looks redundant and >> I can't see the need for ordering CNTVOFF vs CNTV_CVAL. I can see an >> argument to putting one *before* CNTV_CTL, but you don't have one there! > > CNTVOFF directly influences whether or not CNTV_CVAL will trigger or > not. Maybe I'm just being paranoid and moving the isb after CNTV_CVAL is > enough. > > The last one is definitively superfluous. > can't we also get rid of the isb on the return path then? do you agree with this patch: --- Thanks, -Christoffer diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S index 57cfa84..7e6eedf 100644 --- a/arch/arm/kvm/interrupts_head.S +++ b/arch/arm/kvm/interrupts_head.S @@ -492,7 +492,6 @@ vcpu .req r0 @ vcpu pointer always in r0 str r2, [vcpu, #VCPU_TIMER_CNTV_CTL] bic r2, #1 @ Clear ENABLE mcr p15, 0, r2, c14, c3, 1 @ CNTV_CTL - isb mrrc p15, 3, r2, r3, c14 @ CNTV_CVAL ldr r4, =VCPU_TIMER_CNTV_CVAL @@ -532,18 +531,17 @@ vcpu .req r0 @ vcpu pointer always in r0 ldr r2, [r4, #KVM_TIMER_CNTVOFF] ldr r3, [r4, #(KVM_TIMER_CNTVOFF + 4)] mcrr p15, 4, r2, r3, c14 @ CNTVOFF - isb ldr r4, =VCPU_TIMER_CNTV_CVAL add vcpu, vcpu, r4 ldrd r2, r3, [vcpu] sub vcpu, vcpu, r4 mcrr p15, 3, r2, r3, c14 @ CNTV_CVAL + isb ldr r2, [vcpu, #VCPU_TIMER_CNTV_CTL] and r2, r2, #3 mcr p15, 0, r2, c14, c3, 1 @ CNTV_CTL - isb 1: #endif .endm