netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Cornelia Huck <cohuck@redhat.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>,
	kvm@vger.kernel.org, linux-s390@vger.kernel.org,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org,
	intel-gvt-dev@lists.freedesktop.org, kwankhede@nvidia.com,
	mst@redhat.com, tiwei.bie@intel.com,
	virtualization@lists.linux-foundation.org,
	netdev@vger.kernel.org, maxime.coquelin@redhat.com,
	cunming.liang@intel.com, zhihong.wang@intel.com,
	rob.miller@broadcom.com, xiao.w.wang@intel.com,
	haotian.wang@sifive.com, zhenyuw@linux.intel.com,
	zhi.a.wang@intel.com, jani.nikula@linux.intel.com,
	joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com,
	airlied@linux.ie, daniel@ffwll.ch, farman@linux.ibm.com,
	pasic@linux.ibm.com, sebott@linux.ibm.com, oberpar@linux.ibm.com,
	heiko.carstens@de.ibm.com, gor@linux.ibm.com,
	borntraeger@de.ibm.com, akrowiak@linux.ibm.com,
	freude@linux.ibm.com, lingshan.zhu@intel.com, idos@mellanox.com,
	eperezma@redhat.com, lulu@redhat.com, parav@mellanox.com,
	christophe.de.dinechin@gmail.com, kevin.tian@intel.com,
	stefanha@redhat.com
Subject: Re: [PATCH V4 3/6] mdev: introduce device specific ops
Date: Fri, 18 Oct 2019 09:44:07 +0200	[thread overview]
Message-ID: <20191018094407.1a91e843.cohuck@redhat.com> (raw)
In-Reply-To: <20191017115310.0481cc52@x1.home>

On Thu, 17 Oct 2019 11:53:10 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Thu, 17 Oct 2019 17:07:55 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
> 
> > On Thu, 17 Oct 2019 18:48:33 +0800
> > Jason Wang <jasowang@redhat.com> wrote:
> >   
> > > Currently, except for the create and remove, the rest of
> > > mdev_parent_ops is designed for vfio-mdev driver only and may not help
> > > for kernel mdev driver. With the help of class id, this patch
> > > introduces device specific callbacks inside mdev_device
> > > structure. This allows different set of callback to be used by
> > > vfio-mdev and virtio-mdev.
> > > 
> > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > > ---
> > >  .../driver-api/vfio-mediated-device.rst       | 25 +++++----
> > >  MAINTAINERS                                   |  1 +
> > >  drivers/gpu/drm/i915/gvt/kvmgt.c              | 18 ++++---
> > >  drivers/s390/cio/vfio_ccw_ops.c               | 18 ++++---
> > >  drivers/s390/crypto/vfio_ap_ops.c             | 14 +++--
> > >  drivers/vfio/mdev/mdev_core.c                 | 18 +++++--
> > >  drivers/vfio/mdev/mdev_private.h              |  1 +
> > >  drivers/vfio/mdev/vfio_mdev.c                 | 37 ++++++-------
> > >  include/linux/mdev.h                          | 45 ++++------------
> > >  include/linux/vfio_mdev.h                     | 52 +++++++++++++++++++
> > >  samples/vfio-mdev/mbochs.c                    | 20 ++++---
> > >  samples/vfio-mdev/mdpy.c                      | 20 ++++---
> > >  samples/vfio-mdev/mtty.c                      | 18 ++++---
> > >  13 files changed, 184 insertions(+), 103 deletions(-)
> > >  create mode 100644 include/linux/vfio_mdev.h
> > > 
> > > diff --git a/Documentation/driver-api/vfio-mediated-device.rst b/Documentation/driver-api/vfio-mediated-device.rst
> > > index f9a78d75a67a..0cca84d19603 100644
> > > --- a/Documentation/driver-api/vfio-mediated-device.rst
> > > +++ b/Documentation/driver-api/vfio-mediated-device.rst
> > > @@ -152,11 +152,22 @@ callbacks per mdev parent device, per mdev type, or any other categorization.
> > >  Vendor drivers are expected to be fully asynchronous in this respect or
> > >  provide their own internal resource protection.)
> > >  
> > > -The callbacks in the mdev_parent_ops structure are as follows:
> > > -
> > > -* open: open callback of mediated device
> > > -* close: close callback of mediated device
> > > -* ioctl: ioctl callback of mediated device
> > > +As multiple types of mediated devices may be supported, the device
> > > +must set up the class id and the device specific callbacks in create()    
> > 
> > s/in create()/in the create()/
> >   
> > > +callback. E.g for vfio-mdev device it needs to be done through:    
> > 
> > "Each class provides a helper function to do so; e.g. for vfio-mdev
> > devices, the function to be called is:"
> > 
> > ?
> >   
> > > +
> > > +    int mdev_set_vfio_ops(struct mdev_device *mdev,
> > > +                          const struct vfio_mdev_ops *vfio_ops);
> > > +
> > > +The class id (set to MDEV_CLASS_ID_VFIO) is used to match a device    
> > 
> > "(set by this helper function to MDEV_CLASS_ID_VFIO)" ?
> >   
> > > +with an mdev driver via its id table. The device specific callbacks
> > > +(specified in *ops) are obtainable via mdev_get_dev_ops() (for use by    
> > 
> > "(specified in *vfio_ops by the caller)" ?
> >   
> > > +the mdev bus driver). A vfio-mdev device (class id MDEV_CLASS_ID_VFIO)
> > > +uses the following device-specific ops:
> > > +
> > > +* open: open callback of vfio mediated device
> > > +* close: close callback of vfio mediated device
> > > +* ioctl: ioctl callback of vfio mediated device
> > >  * read : read emulation callback
> > >  * write: write emulation callback
> > >  * mmap: mmap emulation callback
> > > @@ -167,10 +178,6 @@ register itself with the mdev core driver::
> > >  	extern int  mdev_register_device(struct device *dev,
> > >  	                                 const struct mdev_parent_ops *ops);
> > >  
> > > -It is also required to specify the class_id in create() callback through::
> > > -
> > > -	int mdev_set_class(struct mdev_device *mdev, u16 id);
> > > -    
> > 
> > I'm wondering if this patch set should start out with introducing
> > helper functions already (i.e. don't introduce mdev_set_class(), but
> > start out with mdev_set_class_vfio() which will gain the *vfio_ops
> > argument in this patch.)  
> 
> Yes, it would be cleaner, but is it really worth the churn?  Correct me
> if I'm wrong, but I think we get to the same point after this patch and
> aside from the function name itself, the difference is really just that
> the class_id is briefly exposed to the parent driver, right?  Thanks,

Yes, it does not really matter much. If there's no other reason to
rework things, I'd just keep this as it is now.

  reply	other threads:[~2019-10-18  7:44 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-17 10:48 [PATCH V4 0/6] mdev based hardware virtio offloading support Jason Wang
2019-10-17 10:48 ` [PATCH V4 1/6] mdev: class id support Jason Wang
2019-10-20 23:25   ` Parav Pandit
2019-10-17 10:48 ` [PATCH V4 2/6] modpost: add support for mdev class id Jason Wang
2019-10-20 23:25   ` Parav Pandit
2019-10-17 10:48 ` [PATCH V4 3/6] mdev: introduce device specific ops Jason Wang
2019-10-17 15:07   ` Cornelia Huck
2019-10-17 17:53     ` Alex Williamson
2019-10-18  7:44       ` Cornelia Huck [this message]
2019-10-18  7:34     ` Jason Wang
2019-10-20 23:41   ` Parav Pandit
2019-10-21  6:02     ` Jason Wang
2019-10-17 10:48 ` [PATCH V4 4/6] mdev: introduce virtio device and its device ops Jason Wang
2019-10-17 17:53   ` Alex Williamson
2019-10-18  7:35     ` Jason Wang
2019-10-18  9:46   ` Cornelia Huck
2019-10-18 10:55     ` Jason Wang
2019-10-18 13:30       ` Cornelia Huck
2019-10-21  5:36         ` Jason Wang
2019-10-18  9:46   ` Tiwei Bie
2019-10-18 10:57     ` Jason Wang
2019-10-17 10:48 ` [PATCH V4 5/6] virtio: introduce a mdev based transport Jason Wang
2019-10-18 14:20   ` Cornelia Huck
2019-10-21  5:59     ` Jason Wang
2019-10-21  9:36       ` Cornelia Huck
2019-10-21 10:13         ` Jason Wang
2019-10-17 10:48 ` [PATCH V4 6/6] docs: sample driver to demonstrate how to implement virtio-mdev framework Jason Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191018094407.1a91e843.cohuck@redhat.com \
    --to=cohuck@redhat.com \
    --cc=airlied@linux.ie \
    --cc=akrowiak@linux.ibm.com \
    --cc=alex.williamson@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=christophe.de.dinechin@gmail.com \
    --cc=cunming.liang@intel.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=eperezma@redhat.com \
    --cc=farman@linux.ibm.com \
    --cc=freude@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=haotian.wang@sifive.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=idos@mellanox.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-gvt-dev@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=jasowang@redhat.com \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=kevin.tian@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=kwankhede@nvidia.com \
    --cc=lingshan.zhu@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=lulu@redhat.com \
    --cc=maxime.coquelin@redhat.com \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=oberpar@linux.ibm.com \
    --cc=parav@mellanox.com \
    --cc=pasic@linux.ibm.com \
    --cc=rob.miller@broadcom.com \
    --cc=rodrigo.vivi@intel.com \
    --cc=sebott@linux.ibm.com \
    --cc=stefanha@redhat.com \
    --cc=tiwei.bie@intel.com \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=xiao.w.wang@intel.com \
    --cc=zhenyuw@linux.intel.com \
    --cc=zhi.a.wang@intel.com \
    --cc=zhihong.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).