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 X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BF1F9C4360F for ; Thu, 4 Apr 2019 08:47:49 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 85D0220882 for ; Thu, 4 Apr 2019 08:47:49 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="quZL9KZH" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 85D0220882 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=T+UEbTo1oEjuc7vUf/XsEwnmVQ8pLg4q41IgVqGAGqo=; b=quZL9KZHnxMlwh PDBWl2aMLpy015ParAl2stRnvFMUX2XZ7tDxsB8pHCVm629LuoRZksGA2WPc8jdnLTy7lyr+8fVEH 2kWAWvyDX3yR6ixvF22CNvLt0MSRHeoVLvzpbu//n4wXqA3LpLiwdjZ04+AlTK1lRS2HO/FV2Rgxg x78coPV3bICyj74+VvBLuzR7AVEQpvpbJc82B0RbE/uSN+6vNmHBojxNca5C+IHCUyDsBqKJvkfx1 w2cRuXejcD7zqRlaZv0dWs0N1hIUr+BJGwu70qj+7Z2MFshB1WkzU3i/D/WSQS/nWzSH6rjKQ27IO giAHFszVqqMovYBvPYNw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hBy2C-00048F-Nw; Thu, 04 Apr 2019 08:47:44 +0000 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70] helo=foss.arm.com) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hBy29-00047j-8t for linux-arm-kernel@lists.infradead.org; Thu, 04 Apr 2019 08:47:42 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 9D7C680D; Thu, 4 Apr 2019 01:47:40 -0700 (PDT) Received: from e103592.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A5E693F557; Thu, 4 Apr 2019 01:47:38 -0700 (PDT) Date: Thu, 4 Apr 2019 09:47:36 +0100 From: Dave Martin To: Andrew Jones Subject: Re: [PATCH v7 12/27] KVM: arm64/sve: System register context switch and access support Message-ID: <20190404084735.GB3567@e103592.cambridge.arm.com> References: <1553864452-15080-1-git-send-email-Dave.Martin@arm.com> <1553864452-15080-13-git-send-email-Dave.Martin@arm.com> <20190403193943.nv7s36yf2nkzvzwo@kamzik.brq.redhat.com> <20190404080658.GX3567@e103592.cambridge.arm.com> <20190404083218.7mpl4hp4tbbcql7b@kamzik.brq.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190404083218.7mpl4hp4tbbcql7b@kamzik.brq.redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190404_014741_319893_C6F375E5 X-CRM114-Status: GOOD ( 25.20 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Okamoto Takayuki , Christoffer Dall , Ard Biesheuvel , Marc Zyngier , Catalin Marinas , Will Deacon , Zhang Lei , Julien Grall , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Apr 04, 2019 at 10:32:18AM +0200, Andrew Jones wrote: > On Thu, Apr 04, 2019 at 09:06:58AM +0100, Dave Martin wrote: > > On Wed, Apr 03, 2019 at 09:39:43PM +0200, Andrew Jones wrote: > > > On Fri, Mar 29, 2019 at 01:00:37PM +0000, Dave Martin wrote: [...] > > > > +static int get_id_aa64zfr0_el1(struct kvm_vcpu *vcpu, > > > > + const struct sys_reg_desc *rd, > > > > + const struct kvm_one_reg *reg, void __user *uaddr) > > > > +{ > > > > + u64 val; > > > > + > > > > + if (!vcpu_has_sve(vcpu)) > > > > + return -ENOENT; > > > > > > This shouldn't be necessary. The visibility check in > > > kvm_arm_sys_reg_get_reg already covers it. > > > > > > > + > > > > + val = guest_id_aa64zfr0_el1(vcpu); > > > > + return reg_to_user(uaddr, &val, reg->id); > > > > +} > > > > + > > > > +static int set_id_aa64zfr0_el1(struct kvm_vcpu *vcpu, > > > > + const struct sys_reg_desc *rd, > > > > + const struct kvm_one_reg *reg, void __user *uaddr) > > > > +{ > > > > + const u64 id = sys_reg_to_index(rd); > > > > + int err; > > > > + u64 val; > > > > + > > > > + if (!vcpu_has_sve(vcpu)) > > > > + return -ENOENT; > > > > > > Also not necessary. > > > > Hmm, true. Because the logic is a bit spread out I felt uneasy with > > simply deleting these checks, but if they fire, something has > > definitely gone wrong elsewhere. > > > > In its current form the code makes it look like it could be legitimate > > to get here with !vcpu_has_sve(vcpu), which is misleading. > > > > What if we demote these to WARN_ON()? This isn't a fast path. > > A WARN_ON sounds good to me. OK. Are you happy to give your Reviewed-by on the following? --8<-- >From 88aeb182ba787102e627e33728a08b70f467758c Mon Sep 17 00:00:00 2001 From: Dave Martin Date: Thu, 4 Apr 2019 09:41:46 +0100 Subject: [PATCH] KVM: arm64/sve: Demote redundant sysreg vcpu_has_sve() checks to WARNs Because of the logic in kvm_arm_sys_reg_{get,set}_reg() and sve_id_visibility(), we should never call {get,set}_id_aa64zfr0_el1() for a vcpu where !vcpu_has_sve(vcpu). To avoid the code giving the impression that it is valid for these functions to be called in this situation, and to help the compiler make the right optimisation decisions, this patch adds WARN_ON() for these cases. Given the way the logic is spread out, this seems preferable to dropping the checks altogether. Signed-off-by: Dave Martin --- arch/arm64/kvm/sys_regs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 09e9b06..7046c76 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1144,7 +1144,7 @@ static int get_id_aa64zfr0_el1(struct kvm_vcpu *vcpu, { u64 val; - if (!vcpu_has_sve(vcpu)) + if (WARN_ON(!vcpu_has_sve(vcpu))) return -ENOENT; val = guest_id_aa64zfr0_el1(vcpu); @@ -1159,7 +1159,7 @@ static int set_id_aa64zfr0_el1(struct kvm_vcpu *vcpu, int err; u64 val; - if (!vcpu_has_sve(vcpu)) + if (WARN_ON(!vcpu_has_sve(vcpu))) return -ENOENT; err = reg_from_user(&val, uaddr, id); -- 2.1.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel