From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51607) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agUQP-0002ve-BX for qemu-devel@nongnu.org; Thu, 17 Mar 2016 05:41:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1agUQO-0004zX-4p for qemu-devel@nongnu.org; Thu, 17 Mar 2016 05:41:01 -0400 Date: Thu, 17 Mar 2016 17:40:48 +0800 From: Peter Xu Message-ID: <20160317094048.GH23495@pxdev.xzpeter.org> References: <1457422582-28799-1-git-send-email-peterx@redhat.com> <1457422582-28799-4-git-send-email-peterx@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v4 3/3] arm: implement query-gic-capabilities List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Wei Huang , Andrew Jones , Markus Armbruster , Michael Roth , QEMU Developers , qemu-arm , Andrea Bolognani On Wed, Mar 16, 2016 at 10:30:36AM +0000, Peter Maydell wrote: > On 8 March 2016 at 07:36, Peter Xu wrote: > > For emulated GIC capabilities, currently only gicv2 is supported. We > > need to add gicv3 in when emulated gicv3 ready. For KVM accelerated ARM > > VM, we detect the capability bits using ioctls. > > > > When probing the KVM capabilities, we cannot leverage existing helper > > functions like kvm_create_device() since QEMU might be using TCG while > > probing (actually this is the case for libvirt probing). So, one > > temporary VM is created to do the probing. > > > > Signed-off-by: Peter Xu > > --- > > target-arm/machine.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 93 insertions(+), 1 deletion(-) > > > > diff --git a/target-arm/machine.c b/target-arm/machine.c > > index 813909e..8f52f74 100644 > > --- a/target-arm/machine.c > > +++ b/target-arm/machine.c > > @@ -1,3 +1,5 @@ > > +#include > > This will break compilation on non-Linux hosts; you can't include > linux headers like this. > > (This is all in the wrong file anyway, machine.c is for migration.) Okay. Will move all kvm lines into kvm*.c. > > > +#include > > #include "qemu/osdep.h" > > #include "hw/hw.h" > > #include "hw/boards.h" > > @@ -347,7 +349,97 @@ const char *gicv3_class_name(void) > > exit(1); > > } > > > > +static GICCapability *gic_cap_new(int version) > > +{ > > + GICCapability *cap = g_new0(GICCapability, 1); > > + cap->version = version; > > + /* by default, support none */ > > + cap->emulated = false; > > + cap->kernel = false; > > + return cap; > > +} > > + > > +static GICCapabilityList *gic_cap_list_add(GICCapabilityList *head, > > + GICCapability *cap) > > +{ > > + GICCapabilityList *item = g_new0(GICCapabilityList, 1); > > + item->value = cap; > > + item->next = head; > > + return item; > > +} > > + > > +#ifdef CONFIG_KVM > > +/* Test whether KVM support specific device. */ > > +static inline int kvm_support_device(int vmfd, uint64_t type) > > +{ > > + struct kvm_create_device create_dev = { > > + .type = type, > > + .fd = -1, > > + .flags = KVM_CREATE_DEVICE_TEST, > > + }; > > + return ioctl(vmfd, KVM_CREATE_DEVICE, &create_dev); > > +} > > +#endif > > This is not ARM specific so it should go in kvm-all.c. Will do. > > > + > > GICCapabilityList *qmp_query_gic_capabilities(Error **errp) > > { > > - return NULL; > > + GICCapabilityList *head = NULL; > > + GICCapability *v2 = gic_cap_new(2), *v3 = gic_cap_new(3); > > + > > + v2->emulated = true; > > + /* FIXME: we'd change to true after we get emulated GICv3. */ > > + v3->emulated = false; > > + > > +#ifdef CONFIG_KVM > > KVM specific code should be factored out and live in one of > the target-arm/kvm*.c files. Ok, will fix. > > > + { > > + int kvm_fd = -1; > > + int vmfd = -1; > > + /* > > + * HACK: here we create one temporary VM, do the probing, > > + * then release it properly. > > + */ > > + kvm_fd = qemu_open("/dev/kvm", O_RDWR); > > + if (kvm_fd == -1) { > > + /* KVM may not enabled on host, which is fine. */ > > + goto out; > > + } > > + > > + do { > > + /* For ARM, VM type could only be zero now. */ > > + vmfd = ioctl(kvm_fd, KVM_CREATE_VM, 0); > > + } while (vmfd == -EINTR); > > + > > + if (vmfd < 0) { > > + goto kvm_fd_close; > > + } > > Rather than open-coding this you might as well use > kvm_arm_creat_scratch_host_vcpu() (you don't need the vcpu fd but > it's pretty harmless to create it.) Thanks for the func pointer! That's what I was looking for. > > > > + > > + if (ioctl(kvm_fd, KVM_CHECK_EXTENSION, > > + KVM_CAP_DEVICE_CTRL) <= 0) { > > + /* older version of KVM possibly */ > > + goto kvm_vmfd_close; > > + } > > Do this in kvm_support_device() [mostly just to parallel how > kvm_create_device() does it.] Will fix. Thanks! -- peterx