From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Martin Subject: Re: [PATCH v7 23/27] KVM: arm64/sve: Add pseudo-register for the guest's vector lengths Date: Fri, 5 Apr 2019 10:36:10 +0100 Message-ID: <20190405093610.GN3567@e103592.cambridge.arm.com> References: <1553864452-15080-1-git-send-email-Dave.Martin@arm.com> <1553864452-15080-24-git-send-email-Dave.Martin@arm.com> <20190404203109.lrt5czzlqpritecd@kamzik.brq.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 122E14A480 for ; Fri, 5 Apr 2019 05:36:17 -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 ScAvy4vgH8ZK for ; Fri, 5 Apr 2019 05:36:15 -0400 (EDT) Received: from foss.arm.com (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by mm01.cs.columbia.edu (Postfix) with ESMTP id B4DC94A426 for ; Fri, 5 Apr 2019 05:36:15 -0400 (EDT) Content-Disposition: inline In-Reply-To: <20190404203109.lrt5czzlqpritecd@kamzik.brq.redhat.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: Andrew Jones 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 Thu, Apr 04, 2019 at 10:31:09PM +0200, Andrew Jones wrote: > On Fri, Mar 29, 2019 at 01:00:48PM +0000, Dave Martin wrote: > > This patch adds a new pseudo-register KVM_REG_ARM64_SVE_VLS to > > allow userspace to set and query the set of vector lengths visible > > to the guest. > > > > In the future, multiple register slices per SVE register may be > > visible through the ioctl interface. Once the set of slices has > > been determined we would not be able to allow the vector length set > > to be changed any more, in order to avoid userspace seeing > > inconsistent sets of registers. For this reason, this patch adds > > support for explicit finalization of the SVE configuration via the > > KVM_ARM_VCPU_FINALIZE ioctl. > > > > Finalization is the proper place to allocate the SVE register state > > storage in vcpu->arch.sve_state, so this patch adds that as > > appropriate. The data is freed via kvm_arch_vcpu_uninit(), which > > was previously a no-op on arm64. > > > > To simplify the logic for determining what vector lengths can be > > supported, some code is added to KVM init to work this out, in the > > kvm_arm_init_arch_resources() hook. > > > > The KVM_REG_ARM64_SVE_VLS pseudo-register is not exposed yet. > > Subsequent patches will allow SVE to be turned on for guest vcpus, > > making it visible. > > > > Signed-off-by: Dave Martin > > Reviewed-by: Julien Thierry > > Tested-by: zhang.lei [...] > > diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c [...] > > +/* > > + * Finalize vcpu's maximum SVE vector length, allocating > > + * vcpu->arch.sve_state as necessary. > > + */ > > +static int kvm_vcpu_finalize_sve(struct kvm_vcpu *vcpu) > > +{ > > + void *buf; > > + unsigned int vl; > > + > > + vl = vcpu->arch.sve_max_vl; > > + > > + /* > > + * Resposibility for these properties is shared between > > + * kvm_arm_init_arch_resources(), kvm_vcpu_enable_sve() and > > + * set_sve_vls(). Double-check here just to be sure: > > + */ > > + if (WARN_ON(!sve_vl_valid(vl) || vl > sve_max_virtualisable_vl || > > + vl > SVE_VL_ARCH_MAX)) > > + return -EIO; > > + > > + buf = kzalloc(SVE_SIG_REGS_SIZE(sve_vq_from_vl(vl)), GFP_KERNEL); > > + if (!buf) > > + return -ENOMEM; > > + > > + vcpu->arch.sve_state = buf; > > + vcpu->arch.flags |= KVM_ARM64_VCPU_SVE_FINALIZED; > > + return 0; > > +} > > + > > +int kvm_arm_vcpu_finalize(struct kvm_vcpu *vcpu, int what) > > +{ > > + switch (what) { > > + case KVM_ARM_VCPU_SVE: > > + if (!vcpu_has_sve(vcpu)) > > + return -EINVAL; > > + > > + if (kvm_arm_vcpu_sve_finalized(vcpu)) > > + return -EPERM; > > + > > + return kvm_vcpu_finalize_sve(vcpu); > > In the next patch I see that we already have KVM_ARM64_GUEST_HAS_SVE > set in vcpu->arch.flags at this point. So if this kvm_vcpu_finalize_sve() > call fails, shouldn't we unset KVM_ARM64_GUEST_HAS_SVE and anything > else used to indicate the vcpu has sve? If this fails, either userspace did the wrong thing, or there was some fatal error (like the ENOMEM case). Either way, the vcpu is not runnable and userspace is expected to bail out. Further, KVM_ARM_VCPU_INIT fixes the set of features requested by userspace: we shouldn't change the set of features under userspace's feet and try to limp on somehow. We have no means for userspace to find out that some features went away in the meantime... So, what would you be trying to achieve with this change? [...] Cheers ---Dave