Re: [PATCH v2 4/4] KVM: VFIO: register kvm_device_ops dynamically

[Will Deacon <will.deacon@arm.com>]

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 */
> >  }
> >  
> > +static int kvm_vfio_create(struct kvm_device *dev, u32 type);
> > +
> > +static struct kvm_device_ops kvm_vfio_ops = {
> > +	.name = "kvm-vfio",
> > +	.create = kvm_vfio_create,
> > +	.destroy = kvm_vfio_destroy,
> > +	.set_attr = kvm_vfio_set_attr,
> > +	.has_attr = kvm_vfio_has_attr,
> > +};
> > +
> 
> Why move the struct?  We wouldn't need the prototype if it was left in
> place and it seems like the only change we're making to set it static.
> 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 == &kvm_vfio_ops)
                        return -EBUSY;

so you have a circular dependency, which I just resolved in the most obvious
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 ‘kvm_vfio_create’:
arch/arm64/kvm/../../../virt/kvm/vfio.c:256:20: error: ‘kvm_vfio_ops’ undeclared (first use in this function)
   if (tmp->ops == &kvm_vfio_ops)
                    ^
arch/arm64/kvm/../../../virt/kvm/vfio.c:256:20: note: each undeclared identifier is reported only once for each function it appears in