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 A6AB5C4360F for ; Fri, 5 Apr 2019 13:00:21 +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 75E8D21855 for ; Fri, 5 Apr 2019 13:00:21 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="QAY9/Ds5" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 75E8D21855 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=RCcQMqJ3N4dnqAXwf6t4KXAFXCny1WC2CENN2qopPq0=; b=QAY9/Ds5Kf+2D4 86HT0L/DqsKca8rgXdihZGel3hbkt4Wi8dRNfGBj3bFcQe2XfxjMILkuDHyL/jLiWAdGiSMUxNc9w 3/8HhjUOda1VleYR7wVHcYoK7wWaI1PsX716wGN2TEOvtjvkJ/oefJ3oUzVvlMYoNdr8L6Jvw7ZW6 re+vuJ/79xIQsl564BkUGOxtZY/TxAdIyW5qUTt4gUYCLsboaKH6TPmxi6XaoIvZl+Tqoc9l4Do6Q UVbsGbRlnp0sneM/QXd8eFzD2+ssWGV2qtLfvehu9dwZ2LEk1SNZ7TYgBGBsWSDQaW7fxbDfutcrc gvykP3X0qACVYSWVeeXg==; 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 1hCOS8-0008J6-LA; Fri, 05 Apr 2019 13:00:16 +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 1hCOS4-0008Ia-RK for linux-arm-kernel@lists.infradead.org; Fri, 05 Apr 2019 13:00:14 +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 CA4701688; Fri, 5 Apr 2019 06:00:11 -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 D8B193F557; Fri, 5 Apr 2019 06:00:09 -0700 (PDT) Date: Fri, 5 Apr 2019 14:00:07 +0100 From: Dave Martin To: Andrew Jones Subject: Re: [PATCH v7 27/27] KVM: arm64/sve: Document KVM API extensions for SVE Message-ID: <20190405130007.GB3567@e103592.cambridge.arm.com> References: <1553864452-15080-1-git-send-email-Dave.Martin@arm.com> <1553864452-15080-28-git-send-email-Dave.Martin@arm.com> <20190404210921.7xz6o56wietd77li@kamzik.brq.redhat.com> <20190405093617.GO3567@e103592.cambridge.arm.com> <20190405103937.quhaxxwkjjdxab6z@kamzik.brq.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190405103937.quhaxxwkjjdxab6z@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-20190405_060012_901774_CE158FE5 X-CRM114-Status: GOOD ( 52.65 ) 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="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Apr 05, 2019 at 12:39:37PM +0200, Andrew Jones wrote: > On Fri, Apr 05, 2019 at 10:36:17AM +0100, Dave Martin wrote: > > On Thu, Apr 04, 2019 at 11:09:21PM +0200, Andrew Jones wrote: > > > On Fri, Mar 29, 2019 at 01:00:52PM +0000, Dave Martin wrote: > > > > This patch adds sections to the KVM API documentation describing > > > > the extensions for supporting the Scalable Vector Extension (SVE) > > > > in guests. > > > > = > > > > Signed-off-by: Dave Martin > > > > = > > > > --- > > > > = > > > > Changes since v5: > > > > = > > > > * Document KVM_ARM_VCPU_FINALIZE and its interactions with SVE. > > > > --- > > > > Documentation/virtual/kvm/api.txt | 132 ++++++++++++++++++++++++++= +++++++++++- > > > > 1 file changed, 129 insertions(+), 3 deletions(-) > > > > = > > > > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virt= ual/kvm/api.txt > > > > index cd920dd..68509de 100644 > > > > --- a/Documentation/virtual/kvm/api.txt > > > > +++ b/Documentation/virtual/kvm/api.txt > > > > @@ -1873,6 +1873,7 @@ Parameters: struct kvm_one_reg (in) > > > > Returns: 0 on success, negative value on failure > > > > Errors: > > > > =A0ENOENT: =A0=A0no such register > > > > + =A0EPERM: =A0=A0=A0register access forbidden for architecture-dep= endent reasons > > > > =A0EINVAL: =A0=A0other errors, such as bad size encoding for a kn= own register > > > > = > > > > struct kvm_one_reg { > > > > @@ -2127,13 +2128,20 @@ Specifically: > > > > 0x6030 0000 0010 004c SPSR_UND 64 spsr[KVM_SPSR_UND] > > > > 0x6030 0000 0010 004e SPSR_IRQ 64 spsr[KVM_SPSR_IRQ] > > > > 0x6060 0000 0010 0050 SPSR_FIQ 64 spsr[KVM_SPSR_FIQ] > > > > - 0x6040 0000 0010 0054 V0 128 fp_regs.vregs[0] > > > > - 0x6040 0000 0010 0058 V1 128 fp_regs.vregs[1] > > > > + 0x6040 0000 0010 0054 V0 128 fp_regs.vregs[0] (*) > > > > + 0x6040 0000 0010 0058 V1 128 fp_regs.vregs[1] (*) > > > > ... > > > > - 0x6040 0000 0010 00d0 V31 128 fp_regs.vregs[31] > > > > + 0x6040 0000 0010 00d0 V31 128 fp_regs.vregs[31] (*) > > > > 0x6020 0000 0010 00d4 FPSR 32 fp_regs.fpsr > > > > 0x6020 0000 0010 00d5 FPCR 32 fp_regs.fpcr > > > > = > > > > +(*) These encodings are not accepted for SVE-enabled vcpus. See > > > > + KVM_ARM_VCPU_INIT. > > > > + > > > > + The equivalent register content can be accessed via bits [127:= 0] of > > > > + the corresponding SVE Zn registers instead for vcpus that have= SVE > > > > + enabled (see below). > > > > + > > > > arm64 CCSIDR registers are demultiplexed by CSSELR value: > > > > 0x6020 0000 0011 00 > > > > = > > > > @@ -2143,6 +2151,61 @@ arm64 system registers have the following id= bit patterns: > > > > arm64 firmware pseudo-registers have the following bit pattern: > > > > 0x6030 0000 0014 > > > > = > > > > +arm64 SVE registers have the following bit patterns: > > > > + 0x6080 0000 0015 00 Zn bits[2048*slice + 2047 = : 2048*slice] > > > > + 0x6050 0000 0015 04 Pn bits[256*slice + 255 : = 256*slice] > > > > + 0x6050 0000 0015 060 FFR bits[256*slice + 255 := 256*slice] > > > > + 0x6060 0000 0015 ffff KVM_REG_ARM64_SVE_VLS pseu= do-register > > > > + > > > > +Access to slices beyond the maximum vector length configured for t= he > > > > +vcpu (i.e., where 16 * slice >=3D max_vq (**)) will fail with ENOE= NT. > > > = > > > I've been doing pretty well keeping track of the 16's, 128's, VL's and > > > VQ's, but this example lost me. Also, should it be >=3D or just > ? > > = > > It should be >=3D. It's analogous to not being allowed to derefence an > > array index that is >=3D the size of the array. > > = > > Also, the 16 here is not the number of bytes per quadword (as it often > > does with all things SVE), but the number of quadwords per 2048-bit > > KVM register slice. > > = > > To make matters worse (**) resembles some weird C syntax. > > = > > Maybe this would be less confusing written as > > = > > Access to register IDs where 2048 * slice >=3D 128 * max_vq will fa= il > > with ENOENT. max_vq is the vcpu's maximum supported vector length > > in 128-bit quadwords: see (**) below. > > = > > Does that help at all? > = > Yes. Thanks. OK, I'll do that. > = > > = > > > = > > > > + > > > > +These registers are only accessible on vcpus for which SVE is enab= led. > > > > +See KVM_ARM_VCPU_INIT for details. > > > > + > > > > +In addition, except for KVM_REG_ARM64_SVE_VLS, these registers are= not > > > > +accessible until the vcpu's SVE configuration has been finalized > > > > +using KVM_ARM_VCPU_FINALIZE(KVM_ARM_VCPU_SVE). See KVM_ARM_VCPU_I= NIT > > > > +and KVM_ARM_VCPU_FINALIZE for more information about this procedur= e. > > > > + > > > > +KVM_REG_ARM64_SVE_VLS is a pseudo-register that allows the set of = vector > > > > +lengths supported by the vcpu to be discovered and configured by > > > > +userspace. When transferred to or from user memory via KVM_GET_ON= E_REG > > > > +or KVM_SET_ONE_REG, the value of this register is of type __u64[8]= , and > > > > +encodes the set of vector lengths as follows: > > > > + > > > > +__u64 vector_lengths[8]; > > > > + > > > > +if (vq >=3D SVE_VQ_MIN && vq <=3D SVE_VQ_MAX && > > > > + ((vector_lengths[(vq - 1) / 64] >> ((vq - 1) % 64)) & 1)) > > > > + /* Vector length vq * 16 bytes supported */ > > > > +else > > > > + /* Vector length vq * 16 bytes not supported */ > > > > + > > > > +(**) The maximum value vq for which the above condition is true is > > > > +max_vq. This is the maximum vector length available to the guest = on > > > > +this vcpu, and determines which register slices are visible through > > > > +this ioctl interface. > > > > + > > > > +(See Documentation/arm64/sve.txt for an explanation of the "vq" > > > > +nomenclature.) > > > > + > > > > +KVM_REG_ARM64_SVE_VLS is only accessible after KVM_ARM_VCPU_INIT. > > > > +KVM_ARM_VCPU_INIT initialises it to the best set of vector lengths= that > > > > +the host supports. > > > > + > > > > +Userspace may subsequently modify it if desired until the vcpu's S= VE > > > > +configuration is finalized using KVM_ARM_VCPU_FINALIZE(KVM_ARM_VCP= U_SVE). > > > > + > > > > +Apart from simply removing all vector lengths from the host set th= at > > > > +exceed some value, support for arbitrarily chosen sets of vector l= engths > > > > +is hardware-dependent and may not be available. Attempting to con= figure > > > > +an invalid set of vector lengths via KVM_SET_ONE_REG will fail with > > > > +EINVAL. > > > > + > > > > +After the vcpu's SVE configuration is finalized, further attempts = to > > > > +write this register will fail with EPERM. > > > > + > > > > = > > > > MIPS registers are mapped using the lower 32 bits. The upper 16 o= f that is > > > > the register group type: > > > > @@ -2197,6 +2260,7 @@ Parameters: struct kvm_one_reg (in and out) > > > > Returns: 0 on success, negative value on failure > > > > Errors: > > > > =A0ENOENT: =A0=A0no such register > > > > + =A0EPERM: =A0=A0=A0register access forbidden for architecture-dep= endent reasons > > > > =A0EINVAL: =A0=A0other errors, such as bad size encoding for a kn= own register > > > > = > > > > This ioctl allows to receive the value of a single register implem= ented > > > > @@ -2690,6 +2754,33 @@ Possible features: > > > > - KVM_ARM_VCPU_PMU_V3: Emulate PMUv3 for the CPU. > > > > Depends on KVM_CAP_ARM_PMU_V3. > > > > = > > > > + - KVM_ARM_VCPU_SVE: Enables SVE for the CPU (arm64 only). > > > > + Depends on KVM_CAP_ARM_SVE. > > > > + Requires KVM_ARM_VCPU_FINALIZE(KVM_ARM_VCPU_SVE): > > > > + > > > > + * After KVM_ARM_VCPU_INIT: > > > > + > > > > + - KVM_REG_ARM64_SVE_VLS may be read using KVM_GET_ONE_REG: = the > > > > + initial value of this pseudo-register indicates the best = set of > > > > + vector lengths possible for a vcpu on this host. > > > > + > > > > + * Before KVM_ARM_VCPU_FINALIZE(KVM_ARM_VCPU_SVE): > > > > + > > > > + - KVM_RUN and KVM_GET_REG_LIST are not available; > > > > + > > > > + - KVM_GET_ONE_REG and KVM_SET_ONE_REG cannot be used to acc= ess > > > > + the scalable archietctural SVE registers > > > > + KVM_REG_ARM64_SVE_ZREG(), KVM_REG_ARM64_SVE_PREG() or > > > > + KVM_REG_ARM64_SVE_FFR; > > > > + > > > > + - KVM_REG_ARM64_SVE_VLS may optionally be written using > > > > + KVM_SET_ONE_REG, to modify the set of vector lengths avai= lable > > > > + for the vcpu. > > > > + > > > > + * After KVM_ARM_VCPU_FINALIZE(KVM_ARM_VCPU_SVE): > > > > + > > > > + - the KVM_REG_ARM64_SVE_VLS pseudo-register is immutable, a= nd can > > > > + no longer be written using KVM_SET_ONE_REG. > > > > = > > > > 4.83 KVM_ARM_PREFERRED_TARGET > > > > = > > > > @@ -3904,6 +3995,41 @@ number of valid entries in the 'entries' arr= ay, which is then filled. > > > > 'index' and 'flags' fields in 'struct kvm_cpuid_entry2' are curren= tly reserved, > > > > userspace should not expect to get any particular value there. > > > > = > > > > +4.119 KVM_ARM_VCPU_FINALIZE > > > > + > > > > +Capability: KVM_CAP_ARM_SVE > > > = > > > The KVM_ARM_VCPU_FINALIZE vcpu ioctl isn't SVE specific, so it should= n't > > > depend on the SVE cap. It should have it's own cap, or maybe it should > > > just be an KVM_ARM_VCPU_SVE_FINALIZE ioctl instead, i.e. not general. > > = > > This one's a bit annoying. This would ideally read > > = > > Capability: KVM_CAP_ARM_SVE, or any other capability that advertises the > > availability of a feature that requires KVM_ARM_VCPU_FINALIZE to be use= d. > > = > > (which sounds vague). > > = > > We could add a specific cap for KVM_ARM_VCPU_FINALIZE, but that seemed > > overkill. Or document KVM_ARM_VCPU_FINALIZE as unconditionally > > supported, but make the individual feature values cap-dependent. This > > works because the symptom of missing support is the same (EINVAL) > > ragardless of whether it is the specific feature or > > KVM_ARM_VCPU_FINALIZE that is unsupported. > > = > > Thoughts? > > = > = > I like that unconditionally supported idea. OK, I'll see how to write this up in the documentation. > > > > +Architectures: arm, arm64 > > > > +Type: vcpu ioctl > > > > +Parameters: int feature (in) > > > = > > > This was called 'what' in the code. > > = > > Indeed it is. I wanted to avoid the implication that this paramter > > exactly maps onto the KVM_ARM_VCPU_INIT features. But "what" seemed > > a bit too vague for the documentation. > > = > > Mind you, if lseek() can have a "whence" parameter, perhaps "what" is > > also acceptable. > > = > > OTOH I don't mind changing the name in the code to "feature" if you > > think that's preferable. > > = > > Thoughts? > = > I prefer them to be the same, and I think both are fine. OK. I'll go with "feature". > > > > +Returns: 0 on success, -1 on error > > > > +Errors: > > > > + EPERM: feature not enabled, needs configuration, or already = finalized > > > > + EINVAL: unknown feature > > > > + > > > > +Recognised values for feature: > > > > + arm64 KVM_ARM_VCPU_SVE > > > > + > > > > +Finalizes the configuration of the specified vcpu feature. > > > > + > > > > +The vcpu must already have been initialised, enabling the affected= feature, by > > > > +means of a successful KVM_ARM_VCPU_INIT call with the appropriate = flag set in > > > > +features[]. > > > > + > > > > +For affected vcpu features, this is a mandatory step that must be = performed > > > > +before the vcpu is fully usable. > > > > + > > > > +Between KVM_ARM_VCPU_INIT and KVM_ARM_VCPU_FINALIZE, the feature m= ay be > > > > +configured by use of ioctls such as KVM_SET_ONE_REG. The exact co= nfiguration > > > > +that should be performaned and how to do it are feature-dependent. > > > > + > > > > +Other calls that depend on a particular feature being finalized, s= uch as > > > > +KVM_RUN, KVM_GET_REG_LIST, KVM_GET_ONE_REG and KVM_SET_ONE_REG, wi= ll fail with > > > > +-EPERM unless the feature has already been finalized by means of a > > > > +KVM_ARM_VCPU_FINALIZE call. > > > > + > > > > +See KVM_ARM_VCPU_INIT for details of vcpu features that require fi= nalization > > > > +using this ioctl. > > > > + > > > > 5. The kvm_run structure > > > > ------------------------ > > > > > > > = > > > I'm still not sure about EPERM vs. ENOEXEC. > > = > > There is no need to distinguish the two: this is just a generic "vcpu in > > wrong state for this to work" error. I can't think of another subsystem > > that uses ENOEXEC for this meaning, but it's established in KVM. > > = > > If you can't see a reason why we would need these to be distinct > > errors (?) then I'm happy for this to be ENOEXEC for all cases. > > = > = > I do see some value in keeping them distinct. I think it's just the use > of EPERM specifically that bothers me, but I don't have that strong of > an opinion against its use either. So I'll just shut up :) In an earlier incarnation I had EBADFD, which does kind of mean the right thing. If there's not a clear way forward, I may leave this all as-is for now (but I'll remove the explicit documentation for KVM_{GET,SET}_ONE_REG error codes to give us more flexibility about this in the future, as discussed). Any objection? Cheers ---Dave _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel