From mboxrd@z Thu Jan 1 00:00:00 1970 From: Will Deacon Subject: Re: [PATCH v2 4/4] KVM: VFIO: register kvm_device_ops dynamically Date: Wed, 9 Jul 2014 17:47:32 +0100 Message-ID: <20140709164732.GV9485@arm.com> References: <1404225918-8903-1-git-send-email-will.deacon@arm.com> <1404225918-8903-4-git-send-email-will.deacon@arm.com> <1404922764.4256.181.camel@ul30vt.home> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: "kvm@vger.kernel.org" , "kvmarm@lists.cs.columbia.edu" , "cornelia.huck@de.ibm.com" , "agraf@suse.de" , "gleb@kernel.org" , "pbonzini@redhat.com" , Marc Zyngier , "christoffer.dall@linaro.org" To: Alex Williamson Return-path: Received: from cam-admin0.cambridge.arm.com ([217.140.96.50]:42789 "EHLO cam-admin0.cambridge.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750951AbaGIQrf (ORCPT ); Wed, 9 Jul 2014 12:47:35 -0400 Content-Disposition: inline In-Reply-To: <1404922764.4256.181.camel@ul30vt.home> Sender: kvm-owner@vger.kernel.org List-ID: Hi Alex, On Wed, Jul 09, 2014 at 05:19:24PM +0100, Alex Williamson wrote: > On Tue, 2014-07-01 at 15:45 +0100, Will Deacon wrote: > > diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c > > index ba1a93f935c7..bb11b36ee8a2 100644 > > --- a/virt/kvm/vfio.c > > +++ b/virt/kvm/vfio.c > > @@ -246,6 +246,16 @@ static void kvm_vfio_destroy(struct kvm_device= *dev) > > kfree(dev); /* alloc by kvm_ioctl_create_device, free by .destroy= */ > > } > > =20 > > +static int kvm_vfio_create(struct kvm_device *dev, u32 type); > > + > > +static struct kvm_device_ops kvm_vfio_ops =3D { > > + .name =3D "kvm-vfio", > > + .create =3D kvm_vfio_create, > > + .destroy =3D kvm_vfio_destroy, > > + .set_attr =3D kvm_vfio_set_attr, > > + .has_attr =3D kvm_vfio_has_attr, > > +}; > > + >=20 > Why move the struct? We wouldn't need the prototype if it was left i= n > place and it seems like the only change we're making to set it static= =2E > Functionally the change is fine, but the ordering was cleaner before > imho. Thanks, The problem is that kvm_vfio_create takes the address of kvm_vfio_ops: /* Only one VFIO "device" per VM */ list_for_each_entry(tmp, &dev->kvm->devices, vm_node) if (tmp->ops =3D=3D &kvm_vfio_ops) return -EBUSY; so you have a circular dependency, which I just resolved in the most ob= vious way to me. I'm happy to solve it a better way, if you have a preference= ? Will --->8 arch/arm64/kvm/../../../virt/kvm/vfio.c: In function =E2=80=98kvm_vfio_= create=E2=80=99: arch/arm64/kvm/../../../virt/kvm/vfio.c:256:20: error: =E2=80=98kvm_vfi= o_ops=E2=80=99 undeclared (first use in this function) if (tmp->ops =3D=3D &kvm_vfio_ops) ^ arch/arm64/kvm/../../../virt/kvm/vfio.c:256:20: note: each undeclared i= dentifier is reported only once for each function it appears in