From patchwork Wed Aug 19 17:49:58 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 52560 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-la0-f72.google.com (mail-la0-f72.google.com [209.85.215.72]) by patches.linaro.org (Postfix) with ESMTPS id 06B232156D for ; Wed, 19 Aug 2015 17:51:07 +0000 (UTC) Received: by lagz9 with SMTP id z9sf3843621lag.3 for ; Wed, 19 Aug 2015 10:51:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:date:from:to:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent :precedence:list-id:list-unsubscribe:list-archive:list-post :list-help:list-subscribe:cc:content-type:content-transfer-encoding :sender:errors-to:x-original-sender :x-original-authentication-results:mailing-list; bh=e9XKpwkUd/MWJo3Vepk8yonI6DcxMf3Lo7s756qBgFU=; b=IQIOXDG0oGvHYApwCCkplj3vyNVSD7WsRXBvElqQaeBu6DWUkFn7a4aA4nnCxE2KRN oUr6zcy8Pa/J43Fpu/zB7t9Vw4tMZtEk1W0Q/EyJ1hqEJigExWviIvDHIGMmbTegPB8F z+8V3p0xk+QFqJg0d7d1aYmc6Bd08syCmIKRMgLyaaRv/J3Rs8nFB9FPf3QHSSZb8vZT dxU9fFoVs/UtDMWif8qt3i6V4h9JltY4X3KHrh7SyGNm1L29iaETPuwe2Kcxr4hbfURv 7ACdvuLB7nPQxzc5gtlVb7qlKPcXOXWnD+td8qk3RTLMk2ovpQ6NEKWJFc3mXLOJ7ugn i43A== X-Gm-Message-State: ALoCoQnRYw4nQZ9vfbRsZPyGgMrzPNJ/XGuOfXbpa/iF7grzHjSCxQW/zB5Tb3WK0PTi1D2zwQT5 X-Received: by 10.112.50.10 with SMTP id y10mr3590264lbn.10.1440006665783; Wed, 19 Aug 2015 10:51:05 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.21.163 with SMTP id w3ls62032lae.21.gmail; Wed, 19 Aug 2015 10:51:05 -0700 (PDT) X-Received: by 10.112.89.230 with SMTP id br6mr10228625lbb.15.1440006665378; Wed, 19 Aug 2015 10:51:05 -0700 (PDT) Received: from mail-la0-f48.google.com (mail-la0-f48.google.com. [209.85.215.48]) by mx.google.com with ESMTPS id z10si1263777lae.0.2015.08.19.10.51.05 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 19 Aug 2015 10:51:05 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.48 as permitted sender) client-ip=209.85.215.48; Received: by laba3 with SMTP id a3so7870070lab.1 for ; Wed, 19 Aug 2015 10:51:05 -0700 (PDT) X-Received: by 10.112.168.100 with SMTP id zv4mr5437929lbb.117.1440006665178; Wed, 19 Aug 2015 10:51:05 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.162.200 with SMTP id yc8csp523118lbb; Wed, 19 Aug 2015 10:51:03 -0700 (PDT) X-Received: by 10.70.38.35 with SMTP id d3mr16686855pdk.33.1440006663840; Wed, 19 Aug 2015 10:51:03 -0700 (PDT) Received: from bombadil.infradead.org (bombadil.infradead.org. [2001:1868:205::9]) by mx.google.com with ESMTPS id ca5si2520760pbd.19.2015.08.19.10.51.03 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 19 Aug 2015 10:51:03 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) client-ip=2001:1868:205::9; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZS7UE-0001p0-OI; Wed, 19 Aug 2015 17:49:18 +0000 Received: from mail-la0-f51.google.com ([209.85.215.51]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZS7UC-0001nP-3B for linux-arm-kernel@lists.infradead.org; Wed, 19 Aug 2015 17:49:16 +0000 Received: by lahi9 with SMTP id i9so7772924lah.2 for ; Wed, 19 Aug 2015 10:48:53 -0700 (PDT) X-Received: by 10.153.8.137 with SMTP id dk9mr12644385lad.57.1440006533155; Wed, 19 Aug 2015 10:48:53 -0700 (PDT) Received: from localhost (0187900153.0.fullrate.dk. [2.110.55.193]) by smtp.gmail.com with ESMTPSA id o6sm409795lah.20.2015.08.19.10.48.52 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Wed, 19 Aug 2015 10:48:52 -0700 (PDT) Date: Wed, 19 Aug 2015 19:49:58 +0200 From: Christoffer Dall To: Marc Zyngier Subject: Re: [PATCH v4 1/2] arm64: KVM: Optimize arm64 skip 30-50% vfp/simd save/restore on exits Message-ID: <20150819174958.GA11518@cbox> References: <1437082178-11039-1-git-send-email-m.smarduch@samsung.com> <1437082178-11039-2-git-send-email-m.smarduch@samsung.com> <55C235B9.3000800@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <55C235B9.3000800@arm.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150819_104916_329050_28F68A33 X-CRM114-Status: GOOD ( 24.82 ) X-Spam-Score: -2.6 (--) X-Spam-Report: SpamAssassin version 3.4.0 on bombadil.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.215.51 listed in list.dnswl.org] -0.0 RCVD_IN_MSPIKE_H3 RBL: Good reputation (+3) [209.85.215.51 listed in wl.mailspike.net] -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.0 RCVD_IN_MSPIKE_WL Mailspike good senders X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: , List-Help: , List-Subscribe: , Cc: "linux-arm-kernel@lists.infradead.org" , "kvmarm@lists.cs.columbia.edu" , "kvm@vger.kernel.org" , Mario Smarduch Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: christoffer.dall@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.48 as permitted sender) smtp.mailfrom=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 Hi Mario, On Wed, Aug 05, 2015 at 05:11:37PM +0100, Marc Zyngier wrote: > On 16/07/15 22:29, Mario Smarduch wrote: > > This patch only saves and restores FP/SIMD registers on Guest access. To do > > this cptr_el2 FP/SIMD trap is set on Guest entry and later checked on exit. > > lmbench, hackbench show significant improvements, for 30-50% exits FP/SIMD > > context is not saved/restored > > > > Signed-off-by: Mario Smarduch > > So this patch seems to break 32bit guests on arm64. I've had a look, > squashed a few bugs that I dangerously overlooked during the review, but > it still doesn't work (it doesn't crash anymore, but I get random > illegal VFP instructions in 32bit guests). > > I'd be glad if someone could eyeball the following patch and tell me > what's going wrong. If we don't find the root cause quickly enough, I'll > have to drop the series from -next, and that'd be a real shame. > > Thanks, > > M. > > commit 5777dc55fbc170426a85e00c26002dd5a795cfa5 > Author: Marc Zyngier > Date: Wed Aug 5 16:53:01 2015 +0100 > > KVM: arm64: NOTAFIX: Prevent crash when 32bit guest uses VFP > > Since we switch FPSIMD in a lazy way, access to FPEXC32_EL2 > must be guarded by skip_fpsimd_state. Otherwise, all hell > break loose. > > Also, FPEXC32_EL2 must be restored when we trap to EL2 to > enable floating point. > > Note that while it prevents the host from catching fire, the > guest still doesn't work properly, and I don't understand why just > yet. > > Not-really-signed-off-by: Marc Zyngier > > diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S > index c8e0c70..b53ec5d 100644 > --- a/arch/arm64/kvm/hyp.S > +++ b/arch/arm64/kvm/hyp.S > @@ -431,10 +431,12 @@ > add x3, x2, #CPU_SYSREG_OFFSET(DACR32_EL2) > mrs x4, dacr32_el2 > mrs x5, ifsr32_el2 > - mrs x6, fpexc32_el2 > stp x4, x5, [x3] > - str x6, [x3, #16] > > + skip_fpsimd_state x8, 3f > + mrs x6, fpexc32_el2 > + str x6, [x3, #16] > +3: > skip_debug_state x8, 2f > mrs x7, dbgvcr32_el2 > str x7, [x3, #24] > @@ -461,10 +463,8 @@ > > add x3, x2, #CPU_SYSREG_OFFSET(DACR32_EL2) > ldp x4, x5, [x3] > - ldr x6, [x3, #16] > msr dacr32_el2, x4 > msr ifsr32_el2, x5 > - msr fpexc32_el2, x6 > > skip_debug_state x8, 2f > ldr x7, [x3, #24] > @@ -669,12 +669,14 @@ __restore_debug: > ret > > __save_fpsimd: > + skip_fpsimd_state x3, 1f > save_fpsimd > - ret > +1: ret > > __restore_fpsimd: > + skip_fpsimd_state x3, 1f > restore_fpsimd > - ret > +1: ret > > switch_to_guest_fpsimd: > push x4, lr > @@ -682,6 +684,7 @@ switch_to_guest_fpsimd: > mrs x2, cptr_el2 > bic x2, x2, #CPTR_EL2_TFP > msr cptr_el2, x2 > + isb > > mrs x0, tpidr_el2 > > @@ -692,6 +695,10 @@ switch_to_guest_fpsimd: > add x2, x0, #VCPU_CONTEXT > bl __restore_fpsimd > > + skip_32bit_state x3, 1f > + ldr x4, [x2, #CPU_SYSREG_OFFSET(FPEXC32_EL2)] > + msr fpexc32_el2, x4 > +1: > pop x4, lr > pop x2, x3 > pop x0, x1 > @@ -754,9 +761,7 @@ __kvm_vcpu_return: > add x2, x0, #VCPU_CONTEXT > > save_guest_regs > - skip_fpsimd_state x3, 1f > bl __save_fpsimd > -1: > bl __save_sysregs > > skip_debug_state x3, 1f > @@ -777,9 +782,7 @@ __kvm_vcpu_return: > kern_hyp_va x2 > > bl __restore_sysregs > - skip_fpsimd_state x3, 1f > bl __restore_fpsimd > -1: > /* Clear FPSIMD and Trace trapping */ > msr cptr_el2, xzr > > Marc and I have hunted down the issue at KVM Forum and we believe we've found the issue. Please have a look at the following follow-up patch to Marc's patch above: Thanks, -Christoffer diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S index 8b2a73b4..842e727 100644 --- a/arch/arm64/kvm/hyp.S +++ b/arch/arm64/kvm/hyp.S @@ -769,11 +769,26 @@ .macro activate_traps ldr x2, [x0, #VCPU_HCR_EL2] + + /* + * We are about to set CPTR_EL2.TFP to trap all floating point + * register accesses to EL2, however, the ARM ARM clearly states that + * traps are only taken to EL2 if the operation would not otherwise + * trap to EL1. Therefore, always make sure that for 32-bit guests, + * we set FPEXC.EN to prevent traps to EL1, when setting the TFP bit. + */ + tbnz x2, #HCR_RW_SHIFT, 99f // open code skip_32bit_state + mov x3, #(1 << 30) + msr fpexc32_el2, x3 + isb +99: + msr hcr_el2, x2 mov x2, #CPTR_EL2_TTA orr x2, x2, #CPTR_EL2_TFP msr cptr_el2, x2 + mov x2, #(1 << 15) // Trap CP15 Cr=15 msr hstr_el2, x2