From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Jones Subject: Re: [PATCH v7 27/27] KVM: arm64/sve: Document KVM API extensions for SVE Date: Fri, 5 Apr 2019 17:38:04 +0200 Message-ID: <20190405153804.qnw774e5bsudd2kn@kamzik.brq.redhat.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> <20190405130007.GB3567@e103592.cambridge.arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 4E15B4A36D for ; Fri, 5 Apr 2019 11:38:18 -0400 (EDT) Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id YOc8yIzOr3CY for ; Fri, 5 Apr 2019 11:38:16 -0400 (EDT) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 9C3C54A320 for ; Fri, 5 Apr 2019 11:38:16 -0400 (EDT) Content-Disposition: inline In-Reply-To: <20190405130007.GB3567@e103592.cambridge.arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu To: Dave Martin 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 List-Id: kvmarm@lists.cs.columbia.edu On Fri, Apr 05, 2019 at 02:00:07PM +0100, Dave Martin wrote: > 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/vi= rtual/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-d= ependent reasons > > > > > =A0EINVAL: =A0=A0other errors, such as bad size encoding for a = known 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 [12= 7:0] of > > > > > + the corresponding SVE Zn registers instead for vcpus that ha= ve 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 + 204= 7 : 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 ps= eudo-register > > > > > + > > > > > +Access to slices beyond the maximum vector length configured for= the > > > > > +vcpu (i.e., where 16 * slice >=3D max_vq (**)) will fail with EN= OENT. > > > > = > > > > 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 = fail > > > 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 en= abled. > > > > > +See KVM_ARM_VCPU_INIT for details. > > > > > + > > > > > +In addition, except for KVM_REG_ARM64_SVE_VLS, these registers a= re not > > > > > +accessible until the vcpu's SVE configuration has been finalized > > > > > +using KVM_ARM_VCPU_FINALIZE(KVM_ARM_VCPU_SVE). See KVM_ARM_VCPU= _INIT > > > > > +and KVM_ARM_VCPU_FINALIZE for more information about this proced= ure. > > > > > + > > > > > +KVM_REG_ARM64_SVE_VLS is a pseudo-register that allows the set o= f vector > > > > > +lengths supported by the vcpu to be discovered and configured by > > > > > +userspace. When transferred to or from user memory via KVM_GET_= ONE_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 gues= t on > > > > > +this vcpu, and determines which register slices are visible thro= ugh > > > > > +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 lengt= hs that > > > > > +the host supports. > > > > > + > > > > > +Userspace may subsequently modify it if desired until the vcpu's= SVE > > > > > +configuration is finalized using KVM_ARM_VCPU_FINALIZE(KVM_ARM_V= CPU_SVE). > > > > > + > > > > > +Apart from simply removing all vector lengths from the host set = that > > > > > +exceed some value, support for arbitrarily chosen sets of vector= lengths > > > > > +is hardware-dependent and may not be available. Attempting to c= onfigure > > > > > +an invalid set of vector lengths via KVM_SET_ONE_REG will fail w= ith > > > > > +EINVAL. > > > > > + > > > > > +After the vcpu's SVE configuration is finalized, further attempt= s to > > > > > +write this register will fail with EPERM. > > > > > + > > > > > = > > > > > MIPS registers are mapped using the lower 32 bits. The upper 16= of 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-d= ependent reasons > > > > > =A0EINVAL: =A0=A0other errors, such as bad size encoding for a = known register > > > > > = > > > > > This ioctl allows to receive the value of a single register impl= emented > > > > > @@ -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 bes= t 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 a= ccess > > > > > + 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 av= ailable > > > > > + for the vcpu. > > > > > + > > > > > + * After KVM_ARM_VCPU_FINALIZE(KVM_ARM_VCPU_SVE): > > > > > + > > > > > + - the KVM_REG_ARM64_SVE_VLS pseudo-register is immutable,= and 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' a= rray, which is then filled. > > > > > 'index' and 'flags' fields in 'struct kvm_cpuid_entry2' are curr= ently 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 shou= ldn't > > > > depend on the SVE cap. It should have it's own cap, or maybe it sho= uld > > > > just be an KVM_ARM_VCPU_SVE_FINALIZE ioctl instead, i.e. not genera= l. > > > = > > > 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 u= sed. > > > = > > > (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 alread= y 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 affect= ed feature, by > > > > > +means of a successful KVM_ARM_VCPU_INIT call with the appropriat= e flag set in > > > > > +features[]. > > > > > + > > > > > +For affected vcpu features, this is a mandatory step that must b= e performed > > > > > +before the vcpu is fully usable. > > > > > + > > > > > +Between KVM_ARM_VCPU_INIT and KVM_ARM_VCPU_FINALIZE, the feature= may be > > > > > +configured by use of ioctls such as KVM_SET_ONE_REG. The exact = configuration > > > > > +that should be performaned and how to do it are feature-dependen= t. > > > > > + > > > > > +Other calls that depend on a particular feature being finalized,= such as > > > > > +KVM_RUN, KVM_GET_REG_LIST, KVM_GET_ONE_REG and KVM_SET_ONE_REG, = will 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 = finalization > > > > > +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 subsys= tem > > > 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? Nope. Let's do that. EBADFD doesn't sound good, because we're using the FD without trouble before and even after we attempt these ioctls. I think EBADFD would indicate that no future ioctl (or read,write) should be expected to work. Thanks, drew 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=-9.0 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_NEOMUTT 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 478F9C4360F for ; Fri, 5 Apr 2019 15:38:27 +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 15CFB21726 for ; Fri, 5 Apr 2019 15:38:27 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="Q9frGtra" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 15CFB21726 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.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=skair/OA9CF1m+yBdwm54CT6mrXSTGMiAXjcyMiJeGc=; b=Q9frGtrarjfyai yVXADXwr7qLGpiAbkVAXDTU6/nruSPho2aRN17Fm8omggBCqonDxBDnW8lgxRcoiO2nWMsySaebp5 MxWZt7o7VAKz4F3IzCB2Mv3m1o+lCBp4l5P1PmROdmKA77uU3qYSdaOjP4ycyD4wElE96V0Y2T3Ih kzxccjv9ItdwFtc0uzNVzWF49NZr1DSN2qK6prbDXX8aO4mpfmF15jsIE0ZE3GbcuKNw9fgaR/BEY jUwwBijl44eCg3hm1fmzwbJSKyG6Do1SsNl0LNCIFwe4VcgqIY5gHeS2YwFqJoWlQkhh0zVykN2fe T4zFeJxVOOSdT3NiByOw==; 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 1hCQv6-0003gc-6b; Fri, 05 Apr 2019 15:38:20 +0000 Received: from mx1.redhat.com ([209.132.183.28]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1hCQv2-0003g1-7t for linux-arm-kernel@lists.infradead.org; Fri, 05 Apr 2019 15:38:18 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A545330BC679; Fri, 5 Apr 2019 15:38:15 +0000 (UTC) Received: from kamzik.brq.redhat.com (ovpn-204-32.brq.redhat.com [10.40.204.32]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9A46C5C205; Fri, 5 Apr 2019 15:38:12 +0000 (UTC) Date: Fri, 5 Apr 2019 17:38:04 +0200 From: Andrew Jones To: Dave Martin Subject: Re: [PATCH v7 27/27] KVM: arm64/sve: Document KVM API extensions for SVE Message-ID: <20190405153804.qnw774e5bsudd2kn@kamzik.brq.redhat.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> <20190405130007.GB3567@e103592.cambridge.arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190405130007.GB3567@e103592.cambridge.arm.com> User-Agent: NeoMutt/20180716 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.46]); Fri, 05 Apr 2019 15:38:16 +0000 (UTC) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190405_083816_533591_7C991725 X-CRM114-Status: GOOD ( 54.11 ) 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 02:00:07PM +0100, Dave Martin wrote: > 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/vi= rtual/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-d= ependent reasons > > > > > =A0EINVAL: =A0=A0other errors, such as bad size encoding for a = known 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 [12= 7:0] of > > > > > + the corresponding SVE Zn registers instead for vcpus that ha= ve 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 + 204= 7 : 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 ps= eudo-register > > > > > + > > > > > +Access to slices beyond the maximum vector length configured for= the > > > > > +vcpu (i.e., where 16 * slice >=3D max_vq (**)) will fail with EN= OENT. > > > > = > > > > 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 = fail > > > 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 en= abled. > > > > > +See KVM_ARM_VCPU_INIT for details. > > > > > + > > > > > +In addition, except for KVM_REG_ARM64_SVE_VLS, these registers a= re not > > > > > +accessible until the vcpu's SVE configuration has been finalized > > > > > +using KVM_ARM_VCPU_FINALIZE(KVM_ARM_VCPU_SVE). See KVM_ARM_VCPU= _INIT > > > > > +and KVM_ARM_VCPU_FINALIZE for more information about this proced= ure. > > > > > + > > > > > +KVM_REG_ARM64_SVE_VLS is a pseudo-register that allows the set o= f vector > > > > > +lengths supported by the vcpu to be discovered and configured by > > > > > +userspace. When transferred to or from user memory via KVM_GET_= ONE_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 gues= t on > > > > > +this vcpu, and determines which register slices are visible thro= ugh > > > > > +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 lengt= hs that > > > > > +the host supports. > > > > > + > > > > > +Userspace may subsequently modify it if desired until the vcpu's= SVE > > > > > +configuration is finalized using KVM_ARM_VCPU_FINALIZE(KVM_ARM_V= CPU_SVE). > > > > > + > > > > > +Apart from simply removing all vector lengths from the host set = that > > > > > +exceed some value, support for arbitrarily chosen sets of vector= lengths > > > > > +is hardware-dependent and may not be available. Attempting to c= onfigure > > > > > +an invalid set of vector lengths via KVM_SET_ONE_REG will fail w= ith > > > > > +EINVAL. > > > > > + > > > > > +After the vcpu's SVE configuration is finalized, further attempt= s to > > > > > +write this register will fail with EPERM. > > > > > + > > > > > = > > > > > MIPS registers are mapped using the lower 32 bits. The upper 16= of 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-d= ependent reasons > > > > > =A0EINVAL: =A0=A0other errors, such as bad size encoding for a = known register > > > > > = > > > > > This ioctl allows to receive the value of a single register impl= emented > > > > > @@ -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 bes= t 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 a= ccess > > > > > + 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 av= ailable > > > > > + for the vcpu. > > > > > + > > > > > + * After KVM_ARM_VCPU_FINALIZE(KVM_ARM_VCPU_SVE): > > > > > + > > > > > + - the KVM_REG_ARM64_SVE_VLS pseudo-register is immutable,= and 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' a= rray, which is then filled. > > > > > 'index' and 'flags' fields in 'struct kvm_cpuid_entry2' are curr= ently 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 shou= ldn't > > > > depend on the SVE cap. It should have it's own cap, or maybe it sho= uld > > > > just be an KVM_ARM_VCPU_SVE_FINALIZE ioctl instead, i.e. not genera= l. > > > = > > > 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 u= sed. > > > = > > > (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 alread= y 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 affect= ed feature, by > > > > > +means of a successful KVM_ARM_VCPU_INIT call with the appropriat= e flag set in > > > > > +features[]. > > > > > + > > > > > +For affected vcpu features, this is a mandatory step that must b= e performed > > > > > +before the vcpu is fully usable. > > > > > + > > > > > +Between KVM_ARM_VCPU_INIT and KVM_ARM_VCPU_FINALIZE, the feature= may be > > > > > +configured by use of ioctls such as KVM_SET_ONE_REG. The exact = configuration > > > > > +that should be performaned and how to do it are feature-dependen= t. > > > > > + > > > > > +Other calls that depend on a particular feature being finalized,= such as > > > > > +KVM_RUN, KVM_GET_REG_LIST, KVM_GET_ONE_REG and KVM_SET_ONE_REG, = will 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 = finalization > > > > > +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 subsys= tem > > > 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? Nope. Let's do that. EBADFD doesn't sound good, because we're using the FD without trouble before and even after we attempt these ioctls. I think EBADFD would indicate that no future ioctl (or read,write) should be expected to work. Thanks, drew _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel