From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56217) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SqgMV-0002ds-Np for qemu-devel@nongnu.org; Mon, 16 Jul 2012 04:09:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SqgMP-0004gS-Tq for qemu-devel@nongnu.org; Mon, 16 Jul 2012 04:08:59 -0400 Received: from cantor2.suse.de ([195.135.220.15]:39729 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SqgMP-0004g8-It for qemu-devel@nongnu.org; Mon, 16 Jul 2012 04:08:53 -0400 Mime-Version: 1.0 (Apple Message framework v1278) Content-Type: text/plain; charset=us-ascii From: Alexander Graf In-Reply-To: <87pq7wxje8.fsf@rustcorp.com.au> Date: Mon, 16 Jul 2012 10:08:46 +0200 Content-Transfer-Encoding: quoted-printable Message-Id: References: <878veoz5yv.fsf@rustcorp.com.au> <87pq7wxje8.fsf@rustcorp.com.au> Subject: Re: [Qemu-devel] [kvmarm] [PATCH] target-arm: kvm: use KVM_SET_SREGS to set target to Cortex A15 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Rusty Russell Cc: "peter.maydell@linaro.org" , "kvmarm@lists.cs.columbia.edu" , "qemu-devel@nongnu.org" On 16.07.2012, at 09:19, Rusty Russell wrote: > On Fri, 13 Jul 2012 12:06:26 +0200, Alexander Graf = wrote: >>> struct kvm_sregs { >>> + __u32 target; >>> + __u32 num_features; >>> + __u32 features[14]; >>> }; >>=20 >> Are you sure you want to use sregs? We did the mistake of reusing it >> on ppc, but that doesn't mean you need to repeat the same one :) >>=20 >> Basically sregs are an x86 specific struct for its segment register >> information. I'm quite sure that this is not what your use of them is >> here. >=20 > Since each arch is given a hook already, I just abused it. I'll = change > this to a fresh KVM_ARM_SET_TARGET ioctl. =20 >=20 >> 3) ENABLE_CAP >>=20 >> If you only need to enable a feature and care about backwards >> compatibility of the API (which you don't yet), this is a good one. = it >> basically allows you to enable new features in newer kernel versions >> which would otherwise break compatibility. You can also pass = arbitrary >> data to ENABLE_CAP to pass in additional information. >=20 > Hmm, it's not quite a clean fit: this bitmap is for guest features, = not > kvm ones. Which ones you can enable depends on the target CPU, at = least > in theory. Sure. You'd do (pseudo-code): kvm_ioctl(fd, KVM_ENABLE_CAP, KVM_CAP_ARM_CPU_TARGET, ARM_CPU_NEON | = ARM_CPU_VFP16 | ARM_CPU_V7); >=20 > eg. FP/NEON support and debug register support are (in theory) = optional > features for an implementation. There may be more in future, I guess. >=20 > And you really want to initialize this all at the same time; eg. the = cpu > identification registers need to be initialized depending on the > presence of various features. It's also possible that various = features > may be related, so you can't turn a single one off at a time. That argument does hold true. We are in that mess with ppc, where we = don't have a proper "init" ioctl. So you'd definitely be better off = defining an extensible init handshake now, so you can easily configure = the kernel side early on. Currently for PPC, we just try to either make things stateless or put = additional constraints on certain CAPs, like "may only be set before the = first vcpu run". Not as pretty as doing it cleanly. > Currently, it's a bit theoretical, since we don't have any guest > features, but it was suggested that we'll want them in future. Yes, hence I'm trying to get you guys to something where you won't be = stuck in 6 months from now with a stable ABI that totally doesn't fit = you :) Alex