Re: [PATCH V2 5/8] mdev: introduce device specific ops

From: Jason Wang <jasowang@redhat.com>
To: Rob Miller <rob.miller@broadcom.com>,
	"Tian, Kevin" <kevin.tian@intel.com>
Cc: "christophe.de.dinechin@gmail.com"
	<christophe.de.dinechin@gmail.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"mst@redhat.com" <mst@redhat.com>,
	"airlied@linux.ie" <airlied@linux.ie>,
	"joonas.lahtinen@linux.intel.com"
	<joonas.lahtinen@linux.intel.com>,
	"heiko.carstens@de.ibm.com" <heiko.carstens@de.ibm.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"virtualization@lists.linux-foundation.org"
	<virtualization@lists.linux-foundation.org>,
	"kwankhede@nvidia.com" <kwankhede@nvidia.com>,
	"parav@mellanox.com" <parav@mellanox.com>,
	"linux-s390@vger.kernel.org" <linux-s390@vger.kernel.org>,
	"sebott@linux.ibm.com" <sebott@linux.ibm.com>,
	"lulu@redhat.com" <lulu@redhat.com>,
	"borntraeger@de.ibm.com" <borntraeger@de.ibm.com>,
	"pasic@linux.ibm.com" <pasic@linux.ibm.com>,
	"eperezma@redhat.com" <eperezma@redhat.com>,
	"haotian.wang@sifive.com" <haoti>
Subject: Re: [PATCH V2 5/8] mdev: introduce device specific ops
Date: Fri, 27 Sep 2019 16:04:25 +0800	[thread overview]
Message-ID: <c532d7ce-90be-6456-a630-96f56cb6409d@redhat.com> (raw)
In-Reply-To: <CAJPjb1+i7jb0aVfr=7CDuUBpeo_w3wAfTrnmuLVJ-h+qOV+1hw@mail.gmail.com>

On 2019/9/25 下午10:11, Rob Miller wrote:
>
> > >     mdev_set_class_id(mdev, MDEV_ID_VFIO);
> > > +   mdev_set_dev_ops(mdev, &intel_vfio_vgpu_dev_ops);
> >
> > This seems rather unrefined.  We're registering interdependent data in
> > separate calls.  All drivers need to make both of these calls.  I'm not
> > sure if this is a good idea, but what if we had:
> >
> > static const struct vfio_mdev_device_ops intel_vfio_vgpu_dev_ops = {
> >       .id                     = MDEV_ID_VFIO,
> >       .open                   = intel_vgpu_open,
> >       .release                = intel_vgpu_release,
> >         ...
> >
> > And the set function passed &intel_vfio_vgpu_dev_ops.id 
> <http://intel_vfio_vgpu_dev_ops.id> and the mdev
> > bus drivers used container_of to get to their callbacks?
>
> or just make it explicit? e.g.
>
> mdev_set_class(mdev, MDEV_ID_VFIO, &intel_vfio_vgpu_dev_ops);

Yes, this seems even better.

Will do this in V3.

Thanks

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization