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,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 2DB0BC4360F for ; Thu, 4 Apr 2019 08:07:13 +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 F00C520855 for ; Thu, 4 Apr 2019 08:07:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="HovnVsNQ" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F00C520855 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=Sb5yOVza0mcTQ7J7M73N8hcGVgBmRI8GkPwhO+EtJ2s=; b=HovnVsNQYP/wiC GztFaKD4FsXBQ+E3aEMr10MJ5D2fz2Bt6yDImjH2vgw2tS7+APX41WeiR5E18ql8poAMErkMigJtt 27Lp2o1hBbqSZZ0kjBi0lKjfUL+wBpKtPA8WCCSZKLa7RUU+7CtMxn8qbItkkFWgKXrWNUVXoLLSJ BJ9gdkaF71vH52GJvmay3EpSaQFmaO8iO6xqqTNUtOjeF1NN+ZSzYwgdYY1Of2kV9ZMlPLmqHGcAD hPErWBIWtaobZ2ZgQ9ZFd+jB7/YLmPLq4K2HHbEJCPxTGPZIVumt1+2aHxEAMpg+cC9f9AyFoTAwl FUSv5Y+n2CDV3U0vrh6Q==; 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 1hBxOt-0001Gp-Uc; Thu, 04 Apr 2019 08:07:07 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hBxOq-0001GV-6L for linux-arm-kernel@lists.infradead.org; Thu, 04 Apr 2019 08:07:05 +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 6111680D; Thu, 4 Apr 2019 01:07:03 -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 693F03F68F; Thu, 4 Apr 2019 01:07:01 -0700 (PDT) Date: Thu, 4 Apr 2019 09:06:58 +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: <20190404080658.GX3567@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> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190403193943.nv7s36yf2nkzvzwo@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_010704_246957_DC4F306D X-CRM114-Status: GOOD ( 37.89 ) 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 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: > > This patch adds the necessary support for context switching ZCR_EL1 > > for each vcpu. > > > > ZCR_EL1 is trapped alongside the FPSIMD/SVE registers, so it makes > > sense for it to be handled as part of the guest FPSIMD/SVE context > > for context switch purposes instead of handling it as a general > > system register. This means that it can be switched in lazily at > > the appropriate time. No effort is made to track host context for > > this register, since SVE requires VHE: thus the hosts's value for > > this register lives permanently in ZCR_EL2 and does not alias the > > guest's value at any time. > > > > The Hyp switch and fpsimd context handling code is extended > > appropriately. > > > > Accessors are added in sys_regs.c to expose the SVE system > > registers and ID register fields. Because these need to be > > conditionally visible based on the guest configuration, they are > > implemented separately for now rather than by use of the generic > > system register helpers. This may be abstracted better later on > > when/if there are more features requiring this model. > > > > ID_AA64ZFR0_EL1 is RO-RAZ for MRS/MSR when SVE is disabled for the > > guest, but for compatibility with non-SVE aware KVM implementations > > the register should not be enumerated at all for KVM_GET_REG_LIST > > in this case. For consistency we also reject ioctl access to the > > register. This ensures that a non-SVE-enabled guest looks the same > > to userspace, irrespective of whether the kernel KVM implementation > > supports SVE. > > > > Signed-off-by: Dave Martin > > Reviewed-by: Julien Thierry > > Tested-by: zhang.lei > > > > --- > > > > Changes since v5: > > > > * Port to the renamed visibility() framework. > > > > * Swap visiblity() helpers so that they appear by the relevant accessor > > functions. > > > > * [Julien Grall] With the visibility() checks, {get,set}_zcr_el1() > > degenerate to doing exactly what the common code does, so drop them. > > > > The ID_AA64ZFR0_EL1 handlers are still needed to provide contitional > > RAZ behaviour. This could be moved to the common code too, but since > > this is a one-off case I don't do this for now. We can address this > > later if other regs need to follow the same pattern. > > > > * [Julien Thierry] Reset ZCR_EL1 to a fixed value using reset_val > > instead of using relying on reset_unknown() honouring set bits in val > > as RES0. > > > > Most of the bits in ZCR_EL1 are RES0 anyway, and many implementations > > of SVE will support larger vectors than 128 bits, so 0 seems as good > > a value as any to expose guests that forget to initialise this > > register properly. > > --- [...] > > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c > > index 3563fe6..9d46066 100644 > > --- a/arch/arm64/kvm/hyp/switch.c > > +++ b/arch/arm64/kvm/hyp/switch.c [...] > > +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. [...] > > /* > > * cpufeature ID register user accessors > > * > > @@ -1346,7 +1418,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { > > ID_SANITISED(ID_AA64PFR1_EL1), > > ID_UNALLOCATED(4,2), > > ID_UNALLOCATED(4,3), > > - ID_UNALLOCATED(4,4), > > + { SYS_DESC(SYS_ID_AA64ZFR0_EL1), access_id_aa64zfr0_el1, .get_user = get_id_aa64zfr0_el1, .set_user = set_id_aa64zfr0_el1, .visibility = sve_id_visibility }, > > ID_UNALLOCATED(4,5), > > ID_UNALLOCATED(4,6), > > ID_UNALLOCATED(4,7), > > @@ -1383,6 +1455,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { > > > > { SYS_DESC(SYS_SCTLR_EL1), access_vm_reg, reset_val, SCTLR_EL1, 0x00C50078 }, > > { SYS_DESC(SYS_CPACR_EL1), NULL, reset_val, CPACR_EL1, 0 }, > > + { SYS_DESC(SYS_ZCR_EL1), NULL, reset_val, ZCR_EL1, 0, .visibility = sve_visibility }, > > { SYS_DESC(SYS_TTBR0_EL1), access_vm_reg, reset_unknown, TTBR0_EL1 }, > > { SYS_DESC(SYS_TTBR1_EL1), access_vm_reg, reset_unknown, TTBR1_EL1 }, > > { SYS_DESC(SYS_TCR_EL1), access_vm_reg, reset_val, TCR_EL1, 0 }, > > Mixing designated and non-designated initializers isn't particularly nice, > but I guess it's valid, and the simple alternatives are probably worse. Agreed. We could add designators everywhere or more wrapper macros, but I couldn't see any option that didn't have drawbacks. Since this is a rare case today, I preferred to keep it as explicit in the source as possible. Cheers ---Dave _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel