Re: [PATCH RFC 2/3] usb, kcov: collect coverage from hub_event

2019-10-17 Thread Greg Kroah-Hartman
On Thu, Oct 17, 2019 at 09:06:56PM +0200, Andrey Konovalov wrote:
> On Thu, Oct 17, 2019 at 8:19 PM Greg Kroah-Hartman
>  wrote:
> >
> > On Thu, Oct 17, 2019 at 07:44:14PM +0200, Andrey Konovalov wrote:
> > > This patch adds kcov_remote_start/kcov_remote_stop annotations to the
> > > hub_event function, which is responsible for processing events on USB
> > > buses, in particular events that happen during USB device enumeration.
> > > Each USB bus gets a unique id, which can be used to attach a kcov device
> > > to a particular USB bus for coverage collection.
> > >
> > > Signed-off-by: Andrey Konovalov 
> > > ---
> > >  drivers/usb/core/hub.c| 4 
> > >  include/linux/kcov.h  | 1 +
> > >  include/uapi/linux/kcov.h | 7 +++
> > >  3 files changed, 12 insertions(+)
> > >
> > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > > index 236313f41f4a..03a40e41b099 100644
> > > --- a/drivers/usb/core/hub.c
> > > +++ b/drivers/usb/core/hub.c
> > > @@ -5374,6 +5374,8 @@ static void hub_event(struct work_struct *work)
> > >   hub_dev = hub->intfdev;
> > >   intf = to_usb_interface(hub_dev);
> > >
> > > + kcov_remote_start(kcov_remote_handle_usb(hdev->bus->busnum));
> > > +
> > >   dev_dbg(hub_dev, "state %d ports %d chg %04x evt %04x\n",
> > >   hdev->state, hdev->maxchild,
> > >   /* NOTE: expects max 15 ports... */
> > > @@ -5480,6 +5482,8 @@ static void hub_event(struct work_struct *work)
> > >   /* Balance the stuff in kick_hub_wq() and allow autosuspend */
> > >   usb_autopm_put_interface(intf);
> > >   kref_put(>kref, hub_release);
> > > +
> > > + kcov_remote_stop();
> > >  }
> > >
> > >  static const struct usb_device_id hub_id_table[] = {
> > > diff --git a/include/linux/kcov.h b/include/linux/kcov.h
> > > index 702672d98d35..38a47e0b67c2 100644
> > > --- a/include/linux/kcov.h
> > > +++ b/include/linux/kcov.h
> > > @@ -30,6 +30,7 @@ void kcov_task_exit(struct task_struct *t);
> > >  /*
> > >   * Reserved handle ranges:
> > >   *  -  : common handles
> > > + * 0001 - 0001 : USB subsystem handles
> >
> > So how many bits are you going to have for any in-kernel tasks?  Aren't
> > you going to run out quickly?
> 
> With these patches we only collect coverage from hub_event threads,
> and we need one ID per USB bus, the number of which is quite limited.
> But then we might want to collect coverage from other parts of the USB
> subsystem, so we might need more IDs. I don't expect the number of
> different subsystem from which we want to collect coverage to be
> large, so the idea here is to use 2 bytes of an ID to denote the
> subsystem, and the other 6 to denote different coverage collection
> sections within it.
> 
> But overall, which encoding scheme to use here is a good question.
> Ideas are welcome.
> 
> > >   */
> > >  void kcov_remote_start(u64 handle);
> > >  void kcov_remote_stop(void);
> > > diff --git a/include/uapi/linux/kcov.h b/include/uapi/linux/kcov.h
> > > index 46f78f716ca9..45c9ae59cebc 100644
> > > --- a/include/uapi/linux/kcov.h
> > > +++ b/include/uapi/linux/kcov.h
> > > @@ -43,4 +43,11 @@ enum {
> > >  #define KCOV_CMP_SIZE(n)((n) << 1)
> > >  #define KCOV_CMP_MASK   KCOV_CMP_SIZE(3)
> > >
> > > +#define KCOV_REMOTE_HANDLE_USB  0x0001ull
> > > +
> > > +static inline __u64 kcov_remote_handle_usb(unsigned int bus)
> > > +{
> > > + return KCOV_REMOTE_HANDLE_USB + (__u64)bus;
> > > +}
> >
> > Why is this function in a uapi .h file?  What userspace code would call
> > this?
> 
> A userspace process that wants to collect coverage from USB bus # N
> needs to pass kcov_remote_handle_usb(N) into KCOV_REMOTE_ENABLE ioctl.

Ugh, ok.  Then you should make "unsigned int bus" a __u64 so that this
actually will work on all kernels properly.

thanks,

greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH RFC 3/3] vhost, kcov: collect coverage from vhost_worker

2019-10-17 Thread Greg Kroah-Hartman
On Thu, Oct 17, 2019 at 09:00:18PM +0200, Andrey Konovalov wrote:
> On Thu, Oct 17, 2019 at 8:18 PM Greg Kroah-Hartman
>  wrote:
> >
> > On Thu, Oct 17, 2019 at 07:44:15PM +0200, Andrey Konovalov wrote:
> > > This patch adds kcov_remote_start/kcov_remote_stop annotations to the
> > > vhost_worker function, which is responsible for processing vhost works.
> > > Since vhost_worker is spawned when a vhost device instance is created,
> > > the common kcov handle is used for kcov_remote_start/stop annotations.
> > >
> > > Signed-off-by: Andrey Konovalov 
> > > ---
> > >  drivers/vhost/vhost.c | 15 +++
> > >  drivers/vhost/vhost.h |  3 +++
> > >  2 files changed, 18 insertions(+)
> > >
> > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > index 36ca2cf419bf..71a349f6b352 100644
> > > --- a/drivers/vhost/vhost.c
> > > +++ b/drivers/vhost/vhost.c
> > > @@ -357,7 +357,13 @@ static int vhost_worker(void *data)
> > >   llist_for_each_entry_safe(work, work_next, node, node) {
> > >   clear_bit(VHOST_WORK_QUEUED, >flags);
> > >   __set_current_state(TASK_RUNNING);
> > > +#ifdef CONFIG_KCOV
> > > + kcov_remote_start(dev->kcov_handle);
> > > +#endif
> >
> > Shouldn't you hide these #ifdefs in a .h file?  This is not a "normal"
> > kernel coding style at all.
> 
> Well, if it's acceptable to add a kcov_handle field into vhost_dev
> even when CONFIG_KCOV is not enabled, then we can get rid of those
> #ifdefs.

It should be, it's not a big deal and there's not a ton of those
structures around that one more field is going to hurt anything...

thanks,

greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH RFC 2/3] usb, kcov: collect coverage from hub_event

2019-10-17 Thread Greg Kroah-Hartman
On Thu, Oct 17, 2019 at 07:44:14PM +0200, Andrey Konovalov wrote:
> This patch adds kcov_remote_start/kcov_remote_stop annotations to the
> hub_event function, which is responsible for processing events on USB
> buses, in particular events that happen during USB device enumeration.
> Each USB bus gets a unique id, which can be used to attach a kcov device
> to a particular USB bus for coverage collection.
> 
> Signed-off-by: Andrey Konovalov 
> ---
>  drivers/usb/core/hub.c| 4 
>  include/linux/kcov.h  | 1 +
>  include/uapi/linux/kcov.h | 7 +++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 236313f41f4a..03a40e41b099 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -5374,6 +5374,8 @@ static void hub_event(struct work_struct *work)
>   hub_dev = hub->intfdev;
>   intf = to_usb_interface(hub_dev);
>  
> + kcov_remote_start(kcov_remote_handle_usb(hdev->bus->busnum));
> +
>   dev_dbg(hub_dev, "state %d ports %d chg %04x evt %04x\n",
>   hdev->state, hdev->maxchild,
>   /* NOTE: expects max 15 ports... */
> @@ -5480,6 +5482,8 @@ static void hub_event(struct work_struct *work)
>   /* Balance the stuff in kick_hub_wq() and allow autosuspend */
>   usb_autopm_put_interface(intf);
>   kref_put(>kref, hub_release);
> +
> + kcov_remote_stop();
>  }
>  
>  static const struct usb_device_id hub_id_table[] = {
> diff --git a/include/linux/kcov.h b/include/linux/kcov.h
> index 702672d98d35..38a47e0b67c2 100644
> --- a/include/linux/kcov.h
> +++ b/include/linux/kcov.h
> @@ -30,6 +30,7 @@ void kcov_task_exit(struct task_struct *t);
>  /*
>   * Reserved handle ranges:
>   *  -  : common handles
> + * 0001 - 0001 : USB subsystem handles

So how many bits are you going to have for any in-kernel tasks?  Aren't
you going to run out quickly?


>   */
>  void kcov_remote_start(u64 handle);
>  void kcov_remote_stop(void);
> diff --git a/include/uapi/linux/kcov.h b/include/uapi/linux/kcov.h
> index 46f78f716ca9..45c9ae59cebc 100644
> --- a/include/uapi/linux/kcov.h
> +++ b/include/uapi/linux/kcov.h
> @@ -43,4 +43,11 @@ enum {
>  #define KCOV_CMP_SIZE(n)((n) << 1)
>  #define KCOV_CMP_MASK   KCOV_CMP_SIZE(3)
>  
> +#define KCOV_REMOTE_HANDLE_USB  0x0001ull
> +
> +static inline __u64 kcov_remote_handle_usb(unsigned int bus)
> +{
> + return KCOV_REMOTE_HANDLE_USB + (__u64)bus;
> +}

Why is this function in a uapi .h file?  What userspace code would call
this?

thanks,

greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH RFC 3/3] vhost, kcov: collect coverage from vhost_worker

2019-10-17 Thread Greg Kroah-Hartman
On Thu, Oct 17, 2019 at 07:44:15PM +0200, Andrey Konovalov wrote:
> This patch adds kcov_remote_start/kcov_remote_stop annotations to the
> vhost_worker function, which is responsible for processing vhost works.
> Since vhost_worker is spawned when a vhost device instance is created,
> the common kcov handle is used for kcov_remote_start/stop annotations.
> 
> Signed-off-by: Andrey Konovalov 
> ---
>  drivers/vhost/vhost.c | 15 +++
>  drivers/vhost/vhost.h |  3 +++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 36ca2cf419bf..71a349f6b352 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -357,7 +357,13 @@ static int vhost_worker(void *data)
>   llist_for_each_entry_safe(work, work_next, node, node) {
>   clear_bit(VHOST_WORK_QUEUED, >flags);
>   __set_current_state(TASK_RUNNING);
> +#ifdef CONFIG_KCOV
> + kcov_remote_start(dev->kcov_handle);
> +#endif

Shouldn't you hide these #ifdefs in a .h file?  This is not a "normal"
kernel coding style at all.

>   work->fn(work);
> +#ifdef CONFIG_KCOV
> + kcov_remote_stop();
> +#endif
>   if (need_resched())
>   schedule();
>   }
> @@ -546,6 +552,9 @@ long vhost_dev_set_owner(struct vhost_dev *dev)
>  
>   /* No owner, become one */
>   dev->mm = get_task_mm(current);
> +#ifdef CONFIG_KCOV
> + dev->kcov_handle = current->kcov_handle;
> +#endif
>   worker = kthread_create(vhost_worker, dev, "vhost-%d", current->pid);
>   if (IS_ERR(worker)) {
>   err = PTR_ERR(worker);
> @@ -571,6 +580,9 @@ long vhost_dev_set_owner(struct vhost_dev *dev)
>   if (dev->mm)
>   mmput(dev->mm);
>   dev->mm = NULL;
> +#ifdef CONFIG_KCOV
> + dev->kcov_handle = 0;
> +#endif
>  err_mm:
>   return err;
>  }
> @@ -682,6 +694,9 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
>   if (dev->worker) {
>   kthread_stop(dev->worker);
>   dev->worker = NULL;
> +#ifdef CONFIG_KCOV
> + dev->kcov_handle = 0;
> +#endif
>   }
>   if (dev->mm)
>   mmput(dev->mm);
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index e9ed2722b633..010ca1ebcbd5 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -173,6 +173,9 @@ struct vhost_dev {
>   int iov_limit;
>   int weight;
>   int byte_weight;
> +#ifdef CONFIG_KCOV
> + u64 kcov_handle;
> +#endif

Why is this a #ifdef at all here?

thanks,

greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH V4 4/6] mdev: introduce virtio device and its device ops

2019-10-17 Thread Alex Williamson
On Thu, 17 Oct 2019 18:48:34 +0800
Jason Wang  wrote:

> This patch implements basic support for mdev driver that supports
> virtio transport for kernel virtio driver.
> 
> Signed-off-by: Jason Wang 
> ---
>  drivers/vfio/mdev/mdev_core.c |  12 +++
>  include/linux/mdev.h  |   4 +
>  include/linux/virtio_mdev.h   | 151 ++
>  3 files changed, 167 insertions(+)
>  create mode 100644 include/linux/virtio_mdev.h
> 
> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index d0f3113c8071..5834f6b7c7a5 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -57,6 +57,18 @@ void mdev_set_vfio_ops(struct mdev_device *mdev,
>  }
>  EXPORT_SYMBOL(mdev_set_vfio_ops);
>  
> +/* Specify the virtio device ops for the mdev device, this
> + * must be called during create() callback for virtio mdev device.
> + */
> +void mdev_set_virtio_ops(struct mdev_device *mdev,
> +  const struct virtio_mdev_device_ops *virtio_ops)
> +{
> + BUG_ON(mdev->class_id);

Nit, this one is a BUG_ON, but the vfio one is a WARN_ON.  Thanks,

Alex

> + mdev->class_id = MDEV_CLASS_ID_VIRTIO;
> + mdev->device_ops = virtio_ops;
> +}
> +EXPORT_SYMBOL(mdev_set_virtio_ops);
> +
>  const void *mdev_get_dev_ops(struct mdev_device *mdev)
>  {
>   return mdev->device_ops;
> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
> index 3d29e09e20c9..13e045e09d3b 100644
> --- a/include/linux/mdev.h
> +++ b/include/linux/mdev.h
> @@ -17,6 +17,7 @@
>  
>  struct mdev_device;
>  struct vfio_mdev_device_ops;
> +struct virtio_mdev_device_ops;
>  
>  /*
>   * Called by the parent device driver to set the device which represents
> @@ -111,6 +112,8 @@ void mdev_set_drvdata(struct mdev_device *mdev, void 
> *data);
>  const guid_t *mdev_uuid(struct mdev_device *mdev);
>  void mdev_set_vfio_ops(struct mdev_device *mdev,
>  const struct vfio_mdev_device_ops *vfio_ops);
> +void mdev_set_virtio_ops(struct mdev_device *mdev,
> + const struct virtio_mdev_device_ops *virtio_ops);
>  const void *mdev_get_dev_ops(struct mdev_device *mdev);
>  
>  extern struct bus_type mdev_bus_type;
> @@ -127,6 +130,7 @@ struct mdev_device *mdev_from_dev(struct device *dev);
>  
>  enum {
>   MDEV_CLASS_ID_VFIO = 1,
> + MDEV_CLASS_ID_VIRTIO = 2,
>   /* New entries must be added here */
>  };
>  
> diff --git a/include/linux/virtio_mdev.h b/include/linux/virtio_mdev.h
> new file mode 100644
> index ..b965b50f9b24
> --- /dev/null
> +++ b/include/linux/virtio_mdev.h
> @@ -0,0 +1,151 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Virtio mediated device driver
> + *
> + * Copyright 2019, Red Hat Corp.
> + * Author: Jason Wang 
> + */
> +#ifndef _LINUX_VIRTIO_MDEV_H
> +#define _LINUX_VIRTIO_MDEV_H
> +
> +#include 
> +#include 
> +#include 
> +
> +#define VIRTIO_MDEV_DEVICE_API_STRING"virtio-mdev"
> +#define VIRTIO_MDEV_F_VERSION_1 0x1
> +
> +struct virtio_mdev_callback {
> + irqreturn_t (*callback)(void *data);
> + void *private;
> +};
> +
> +/**
> + * struct vfio_mdev_device_ops - Structure to be registered for each
> + * mdev device to register the device to virtio-mdev module.
> + *
> + * @set_vq_address:  Set the address of virtqueue
> + *   @mdev: mediated device
> + *   @idx: virtqueue index
> + *   @desc_area: address of desc area
> + *   @driver_area: address of driver area
> + *   @device_area: address of device area
> + *   Returns integer: success (0) or error (< 0)
> + * @set_vq_num:  Set the size of virtqueue
> + *   @mdev: mediated device
> + *   @idx: virtqueue index
> + *   @num: the size of virtqueue
> + * @kick_vq: Kick the virtqueue
> + *   @mdev: mediated device
> + *   @idx: virtqueue index
> + * @set_vq_cb:   Set the interrupt callback function for
> + *   a virtqueue
> + *   @mdev: mediated device
> + *   @idx: virtqueue index
> + *   @cb: virtio-mdev interrupt callback structure
> + * @set_vq_ready:Set ready status for a virtqueue
> + *   @mdev: mediated device
> + *   @idx: virtqueue index
> + *   @ready: ready (true) not ready(false)
> + * @get_vq_ready:Get ready status for a virtqueue
> + *   @mdev: mediated device
> + *   @idx: virtqueue index
> + *   Returns boolean: ready (true) or not (false)
> + * @set_vq_state:Set the state for a 

Re: [PATCH V4 3/6] mdev: introduce device specific ops

2019-10-17 Thread Alex Williamson
On Thu, 17 Oct 2019 17:07:55 +0200
Cornelia Huck  wrote:

> On Thu, 17 Oct 2019 18:48:33 +0800
> Jason Wang  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 
> > ---
> >  .../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,

Alex
 
> >  However, the mdev_parent_ops structure is not required in the function call
> >  that a driver should use to unregister itself with the mdev core driver::
> >
> 
> (...)
> 
> > diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> > index 3a9c52d71b4e..d0f3113c8071 100644
> > --- a/drivers/vfio/mdev/mdev_core.c
> > +++ b/drivers/vfio/mdev/mdev_core.c
> > @@ -45,15 +45,23 @@ void mdev_set_drvdata(struct mdev_device *mdev, void 
> > *data)
> >  }
> >  EXPORT_SYMBOL(mdev_set_drvdata);
> >  
> > -/* Specify the class for the mdev device, this must be called during
> 

Re: [PATCH V4 3/6] mdev: introduce device specific ops

2019-10-17 Thread Cornelia Huck
On Thu, 17 Oct 2019 18:48:33 +0800
Jason Wang  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 
> ---
>  .../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.)

>  However, the mdev_parent_ops structure is not required in the function call
>  that a driver should use to unregister itself with the mdev core driver::
>  

(...)

> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index 3a9c52d71b4e..d0f3113c8071 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -45,15 +45,23 @@ void mdev_set_drvdata(struct mdev_device *mdev, void 
> *data)
>  }
>  EXPORT_SYMBOL(mdev_set_drvdata);
>  
> -/* Specify the class for the mdev device, this must be called during
> - * create() callback.
> +/* Specify the VFIO device ops for the mdev device, this
> + * must be called during create() callback for VFIO mdev device.
>   */

/*
 * Specify the mdev device to be a VFIO mdev device, and set the
 * VFIO devices ops for it. This must be called from the create()
 * callback for VFIO mdev devices.
 */

?

> -void mdev_set_class(struct mdev_device *mdev, u16 id)
> +void mdev_set_vfio_ops(struct mdev_device *mdev,
> +const struct vfio_mdev_device_ops *vfio_ops)
>  {
>   WARN_ON(mdev->class_id);
> - 

[PATCH 4/5] drm/qxl: use DEFINE_DRM_GEM_FOPS()

2019-10-17 Thread Gerd Hoffmann
We have no qxl-specific fops any more.

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_drv.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 65464630ac98..1d601f57a6ba 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -150,15 +150,7 @@ qxl_pci_remove(struct pci_dev *pdev)
drm_dev_put(dev);
 }
 
-static const struct file_operations qxl_fops = {
-   .owner = THIS_MODULE,
-   .open = drm_open,
-   .release = drm_release,
-   .unlocked_ioctl = drm_ioctl,
-   .poll = drm_poll,
-   .read = drm_read,
-   .mmap = drm_gem_mmap,
-};
+DEFINE_DRM_GEM_FOPS(qxl_fops);
 
 static int qxl_drm_freeze(struct drm_device *dev)
 {
-- 
2.18.1

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


[PATCH 5/5] drm/qxl: allocate small objects top-down

2019-10-17 Thread Gerd Hoffmann
qxl uses small buffer objects for qxl commands.
Allocate them top-down to reduce fragmentation.

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_object.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 927ab917b834..ad336c98a0cf 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -54,9 +54,14 @@ bool qxl_ttm_bo_is_qxl_bo(struct ttm_buffer_object *bo)
 void qxl_ttm_placement_from_domain(struct qxl_bo *qbo, u32 domain, bool pinned)
 {
u32 c = 0;
-   u32 pflag = pinned ? TTM_PL_FLAG_NO_EVICT : 0;
+   u32 pflag = 0;
unsigned int i;
 
+   if (pinned)
+   pflag |= TTM_PL_FLAG_NO_EVICT;
+   if (qbo->tbo.base.size <= PAGE_SIZE)
+   pflag |= TTM_PL_FLAG_TOPDOWN;
+
qbo->placement.placement = qbo->placements;
qbo->placement.busy_placement = qbo->placements;
if (domain == QXL_GEM_DOMAIN_VRAM)
-- 
2.18.1

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


[PATCH 1/5] drm/qxl: drop qxl_ttm_fault

2019-10-17 Thread Gerd Hoffmann
Not sure what this hook is supposed to do.  vmf->vma->vm_private_data
should never be NULL, so the extra check in qxl_ttm_fault should have no
effect.

Drop it.

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_ttm.c | 27 +--
 1 file changed, 1 insertion(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index cbc6c2ba8630..dba925589e17 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -48,24 +48,8 @@ static struct qxl_device *qxl_get_qdev(struct ttm_bo_device 
*bdev)
return qdev;
 }
 
-static struct vm_operations_struct qxl_ttm_vm_ops;
-static const struct vm_operations_struct *ttm_vm_ops;
-
-static vm_fault_t qxl_ttm_fault(struct vm_fault *vmf)
-{
-   struct ttm_buffer_object *bo;
-   vm_fault_t ret;
-
-   bo = (struct ttm_buffer_object *)vmf->vma->vm_private_data;
-   if (bo == NULL)
-   return VM_FAULT_NOPAGE;
-   ret = ttm_vm_ops->fault(vmf);
-   return ret;
-}
-
 int qxl_mmap(struct file *filp, struct vm_area_struct *vma)
 {
-   int r;
struct drm_file *file_priv = filp->private_data;
struct qxl_device *qdev = file_priv->minor->dev->dev_private;
 
@@ -77,16 +61,7 @@ int qxl_mmap(struct file *filp, struct vm_area_struct *vma)
DRM_DEBUG_DRIVER("filp->private_data = 0x%p, vma->vm_pgoff = %lx\n",
  filp->private_data, vma->vm_pgoff);
 
-   r = ttm_bo_mmap(filp, vma, >mman.bdev);
-   if (unlikely(r != 0))
-   return r;
-   if (unlikely(ttm_vm_ops == NULL)) {
-   ttm_vm_ops = vma->vm_ops;
-   qxl_ttm_vm_ops = *ttm_vm_ops;
-   qxl_ttm_vm_ops.fault = _ttm_fault;
-   }
-   vma->vm_ops = _ttm_vm_ops;
-   return 0;
+   return ttm_bo_mmap(filp, vma, >mman.bdev);
 }
 
 static int qxl_invalidate_caches(struct ttm_bo_device *bdev, uint32_t flags)
-- 
2.18.1

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


[PATCH 2/5] drm/qxl: switch qxl to _gem_object_funcs.mmap

2019-10-17 Thread Gerd Hoffmann
Wire up the new drm_gem_ttm_mmap() helper function.
Use generic drm_gem_mmap() and remove qxl_mmap().

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_drv.h|  1 -
 drivers/gpu/drm/qxl/qxl_drv.c|  2 +-
 drivers/gpu/drm/qxl/qxl_object.c |  1 +
 drivers/gpu/drm/qxl/qxl_ttm.c| 16 
 4 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index d4051409ce64..a5cb3864d686 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -355,7 +355,6 @@ int qxl_mode_dumb_mmap(struct drm_file *filp,
 /* qxl ttm */
 int qxl_ttm_init(struct qxl_device *qdev);
 void qxl_ttm_fini(struct qxl_device *qdev);
-int qxl_mmap(struct file *filp, struct vm_area_struct *vma);
 
 /* qxl image */
 
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 483b4c57554a..65464630ac98 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -157,7 +157,7 @@ static const struct file_operations qxl_fops = {
.unlocked_ioctl = drm_ioctl,
.poll = drm_poll,
.read = drm_read,
-   .mmap = qxl_mmap,
+   .mmap = drm_gem_mmap,
 };
 
 static int qxl_drm_freeze(struct drm_device *dev)
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index c013c516f561..927ab917b834 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -86,6 +86,7 @@ static const struct drm_gem_object_funcs qxl_object_funcs = {
.get_sg_table = qxl_gem_prime_get_sg_table,
.vmap = qxl_gem_prime_vmap,
.vunmap = qxl_gem_prime_vunmap,
+   .mmap = drm_gem_ttm_mmap,
.print_info = drm_gem_ttm_print_info,
 };
 
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index dba925589e17..629ac8e77a21 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -48,22 +48,6 @@ static struct qxl_device *qxl_get_qdev(struct ttm_bo_device 
*bdev)
return qdev;
 }
 
-int qxl_mmap(struct file *filp, struct vm_area_struct *vma)
-{
-   struct drm_file *file_priv = filp->private_data;
-   struct qxl_device *qdev = file_priv->minor->dev->dev_private;
-
-   if (qdev == NULL) {
-   DRM_ERROR(
-"filp->private_data->minor->dev->dev_private == NULL\n");
-   return -EINVAL;
-   }
-   DRM_DEBUG_DRIVER("filp->private_data = 0x%p, vma->vm_pgoff = %lx\n",
- filp->private_data, vma->vm_pgoff);
-
-   return ttm_bo_mmap(filp, vma, >mman.bdev);
-}
-
 static int qxl_invalidate_caches(struct ttm_bo_device *bdev, uint32_t flags)
 {
return 0;
-- 
2.18.1

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


[PATCH 3/5] drm/qxl: drop verify_access

2019-10-17 Thread Gerd Hoffmann
Not needed any more.

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_ttm.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 629ac8e77a21..54cc5a5b607e 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -110,14 +110,6 @@ static void qxl_evict_flags(struct ttm_buffer_object *bo,
*placement = qbo->placement;
 }
 
-static int qxl_verify_access(struct ttm_buffer_object *bo, struct file *filp)
-{
-   struct qxl_bo *qbo = to_qxl_bo(bo);
-
-   return drm_vma_node_verify_access(>tbo.base.vma_node,
- filp->private_data);
-}
-
 static int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
  struct ttm_mem_reg *mem)
 {
@@ -269,7 +261,6 @@ static struct ttm_bo_driver qxl_bo_driver = {
.eviction_valuable = ttm_bo_eviction_valuable,
.evict_flags = _evict_flags,
.move = _bo_move,
-   .verify_access = _verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
.move_notify = _bo_move_notify,
-- 
2.18.1

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


[PATCH net 1/2] vsock/virtio: send a credit update when buffer size is changed

2019-10-17 Thread Stefano Garzarella
When the user application set a new buffer size value, we should
update the remote peer about this change, since it uses this
information to calculate the credit available.

Signed-off-by: Stefano Garzarella 
---
 net/vmw_vsock/virtio_transport_common.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/vmw_vsock/virtio_transport_common.c 
b/net/vmw_vsock/virtio_transport_common.c
index a666ef8fc54e..db127a69f5c3 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -458,6 +458,9 @@ void virtio_transport_set_buffer_size(struct vsock_sock 
*vsk, u64 val)
vvs->buf_size_max = val;
vvs->buf_size = val;
vvs->buf_alloc = val;
+
+   virtio_transport_send_credit_update(vsk, VIRTIO_VSOCK_TYPE_STREAM,
+   NULL);
 }
 EXPORT_SYMBOL_GPL(virtio_transport_set_buffer_size);
 
-- 
2.21.0

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


[PATCH net 2/2] vsock/virtio: discard packets if credit is not respected

2019-10-17 Thread Stefano Garzarella
If the remote peer doesn't respect the credit information
(buf_alloc, fwd_cnt), sending more data than it can send,
we should drop the packets to prevent a malicious peer
from using all of our memory.

This is patch follows the VIRTIO spec: "VIRTIO_VSOCK_OP_RW data
packets MUST only be transmitted when the peer has sufficient
free buffer space for the payload"

Signed-off-by: Stefano Garzarella 
---
 net/vmw_vsock/virtio_transport_common.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c 
b/net/vmw_vsock/virtio_transport_common.c
index db127a69f5c3..481f7f8a1655 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -204,10 +204,14 @@ static int virtio_transport_send_pkt_info(struct 
vsock_sock *vsk,
return virtio_transport_get_ops()->send_pkt(pkt);
 }
 
-static void virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
+static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
struct virtio_vsock_pkt *pkt)
 {
+   if (vvs->rx_bytes + pkt->len > vvs->buf_alloc)
+   return false;
+
vvs->rx_bytes += pkt->len;
+   return true;
 }
 
 static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs,
@@ -879,14 +883,18 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
  struct virtio_vsock_pkt *pkt)
 {
struct virtio_vsock_sock *vvs = vsk->trans;
-   bool free_pkt = false;
+   bool can_enqueue, free_pkt = false;
 
pkt->len = le32_to_cpu(pkt->hdr.len);
pkt->off = 0;
 
spin_lock_bh(>rx_lock);
 
-   virtio_transport_inc_rx_pkt(vvs, pkt);
+   can_enqueue = virtio_transport_inc_rx_pkt(vvs, pkt);
+   if (!can_enqueue) {
+   free_pkt = true;
+   goto out;
+   }
 
/* Try to copy small packets into the buffer of last packet queued,
 * to avoid wasting memory queueing the entire buffer with a small
-- 
2.21.0

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


[PATCH net 0/2] vsock/virtio: make the credit mechanism more robust

2019-10-17 Thread Stefano Garzarella
This series makes the credit mechanism implemented in the
virtio-vsock devices more robust.
Patch 1 sends an update to the remote peer when the buf_alloc
change.
Patch 2 prevents a malicious peer (especially the guest) can
consume all the memory of the other peer, discarding packets
when the credit available is not respected.

Stefano Garzarella (2):
  vsock/virtio: send a credit update when buffer size is changed
  vsock/virtio: discard packets if credit is not respected

 net/vmw_vsock/virtio_transport_common.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

-- 
2.21.0

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


Re: [PATCH -next] drm/qxl: Fix randbuild error

2019-10-17 Thread Gerd Hoffmann
On Tue, Oct 08, 2019 at 10:40:54AM +0800, YueHaibing wrote:
> If DEM_QXL is y and DRM_TTM_HELPER is m, building fails:
> 
> drivers/gpu/drm/qxl/qxl_object.o: undefined reference to 
> `drm_gem_ttm_print_info'
> 
> Select DRM_TTM_HELPER to fix this.

Queued up for drm-misc-next.

thanks,
  Gerd

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


[PATCH V4 6/6] docs: sample driver to demonstrate how to implement virtio-mdev framework

2019-10-17 Thread Jason Wang
This sample driver creates mdev device that simulate virtio net device
over virtio mdev transport. The device is implemented through vringh
and workqueue. A device specific dma ops is to make sure HVA is used
directly as the IOVA. This should be sufficient for kernel virtio
driver to work.

Only 'virtio' type is supported right now. I plan to add 'vhost' type
on top which requires some virtual IOMMU implemented in this sample
driver.

Signed-off-by: Jason Wang 
---
 MAINTAINERS|   1 +
 samples/Kconfig|   7 +
 samples/vfio-mdev/Makefile |   1 +
 samples/vfio-mdev/mvnet.c  | 691 +
 4 files changed, 700 insertions(+)
 create mode 100644 samples/vfio-mdev/mvnet.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 3d196a023b5e..cb51351cd5c9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17254,6 +17254,7 @@ F:  include/linux/virtio*.h
 F: include/uapi/linux/virtio_*.h
 F: drivers/crypto/virtio/
 F: mm/balloon_compaction.c
+F: samples/vfio-mdev/mvnet.c
 
 VIRTIO BLOCK AND SCSI DRIVERS
 M: "Michael S. Tsirkin" 
diff --git a/samples/Kconfig b/samples/Kconfig
index c8dacb4dda80..a1a1ca2c00b7 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -131,6 +131,13 @@ config SAMPLE_VFIO_MDEV_MDPY
  mediated device.  It is a simple framebuffer and supports
  the region display interface (VFIO_GFX_PLANE_TYPE_REGION).
 
+config SAMPLE_VIRTIO_MDEV_NET
+tristate "Build virtio mdev net example mediated device sample code -- 
loadable modules only"
+   depends on VIRTIO_MDEV_DEVICE && VHOST_RING && m
+   help
+ Build a networking sample device for use as a virtio
+ mediated device.
+
 config SAMPLE_VFIO_MDEV_MDPY_FB
tristate "Build VFIO mdpy example guest fbdev driver -- loadable module 
only"
depends on FB && m
diff --git a/samples/vfio-mdev/Makefile b/samples/vfio-mdev/Makefile
index 10d179c4fdeb..f34af90ed0a0 100644
--- a/samples/vfio-mdev/Makefile
+++ b/samples/vfio-mdev/Makefile
@@ -3,3 +3,4 @@ obj-$(CONFIG_SAMPLE_VFIO_MDEV_MTTY) += mtty.o
 obj-$(CONFIG_SAMPLE_VFIO_MDEV_MDPY) += mdpy.o
 obj-$(CONFIG_SAMPLE_VFIO_MDEV_MDPY_FB) += mdpy-fb.o
 obj-$(CONFIG_SAMPLE_VFIO_MDEV_MBOCHS) += mbochs.o
+obj-$(CONFIG_SAMPLE_VIRTIO_MDEV_NET) += mvnet.o
diff --git a/samples/vfio-mdev/mvnet.c b/samples/vfio-mdev/mvnet.c
new file mode 100644
index ..19e823a449cd
--- /dev/null
+++ b/samples/vfio-mdev/mvnet.c
@@ -0,0 +1,691 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Mediated virtual virtio-net device driver.
+ *
+ * Copyright (c) 2019, Red Hat Inc. All rights reserved.
+ * Author: Jason Wang 
+ *
+ * Sample driver that creates mdev device that simulates ethernet loopback
+ * device.
+ *
+ * Usage:
+ *
+ * # modprobe virtio_mdev
+ * # modprobe mvnet
+ * # cd /sys/devices/virtual/mvnet/mvnet/mdev_supported_types/mvnet-virtio
+ * # echo "83b8f4f2-509f-382f-3c1e-e6bfe0fa1001" > ./create
+ * # cd devices/83b8f4f2-509f-382f-3c1e-e6bfe0fa1001
+ * # ls -d virtio0
+ * virtio0
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define VERSION_STRING  "0.1"
+#define DRIVER_AUTHOR   "Red Hat Corporation"
+
+#define MVNET_CLASS_NAME "mvnet"
+#define MVNET_NAME   "mvnet"
+
+/*
+ * Global Structures
+ */
+
+static struct mvnet_dev {
+   struct class*vd_class;
+   struct idr  vd_idr;
+   struct device   dev;
+} mvnet_dev;
+
+struct mvnet_virtqueue {
+   struct vringh vring;
+   struct vringh_kiov iov;
+   unsigned short head;
+   bool ready;
+   u64 desc_addr;
+   u64 device_addr;
+   u64 driver_addr;
+   u32 num;
+   void *private;
+   irqreturn_t (*cb)(void *data);
+};
+
+#define MVNET_QUEUE_ALIGN PAGE_SIZE
+#define MVNET_QUEUE_MAX 256
+#define MVNET_DEVICE_ID 0x1
+#define MVNET_VENDOR_ID 0
+
+u64 mvnet_features = (1ULL << VIRTIO_F_ANY_LAYOUT) |
+(1ULL << VIRTIO_F_VERSION_1) |
+(1ULL << VIRTIO_F_IOMMU_PLATFORM);
+
+/* State of each mdev device */
+struct mvnet_state {
+   struct mvnet_virtqueue vqs[2];
+   struct work_struct work;
+   spinlock_t lock;
+   struct mdev_device *mdev;
+   struct virtio_net_config config;
+   void *buffer;
+   u32 status;
+   u32 generation;
+   u64 features;
+   struct list_head next;
+};
+
+static struct mutex mdev_list_lock;
+static struct list_head mdev_devices_list;
+
+static void mvnet_queue_ready(struct mvnet_state *mvnet, unsigned int idx)
+{
+   struct mvnet_virtqueue *vq = >vqs[idx];
+   int ret;
+
+   ret = vringh_init_kern(>vring, mvnet_features, MVNET_QUEUE_MAX,
+  false, (struct vring_desc *)vq->desc_addr,
+  (struct vring_avail *)vq->driver_addr,
+  

[PATCH V4 5/6] virtio: introduce a mdev based transport

2019-10-17 Thread Jason Wang
This patch introduces a new mdev transport for virtio. This is used to
use kernel virtio driver to drive the mediated device that is capable
of populating virtqueue directly.

A new virtio-mdev driver will be registered to the mdev bus, when a
new virtio-mdev device is probed, it will register the device with
mdev based config ops. This means it is a software transport between
mdev driver and mdev device. The transport was implemented through
device specific ops which is a part of mdev_parent_ops now.

Signed-off-by: Jason Wang 
---
 drivers/virtio/Kconfig   |   7 +
 drivers/virtio/Makefile  |   1 +
 drivers/virtio/virtio_mdev.c | 409 +++
 3 files changed, 417 insertions(+)
 create mode 100644 drivers/virtio/virtio_mdev.c

diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index 078615cf2afc..8d18722ab572 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -43,6 +43,13 @@ config VIRTIO_PCI_LEGACY
 
  If unsure, say Y.
 
+config VIRTIO_MDEV_DEVICE
+   tristate "VIRTIO driver for Mediated devices"
+   depends on VFIO_MDEV && VIRTIO
+   default n
+   help
+ VIRTIO based driver for Mediated devices.
+
 config VIRTIO_PMEM
tristate "Support for virtio pmem driver"
depends on VIRTIO
diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
index 3a2b5c5dcf46..ebc7fa15ae82 100644
--- a/drivers/virtio/Makefile
+++ b/drivers/virtio/Makefile
@@ -6,3 +6,4 @@ virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
 virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o
 obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
 obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o
+obj-$(CONFIG_VIRTIO_MDEV_DEVICE) += virtio_mdev.o
diff --git a/drivers/virtio/virtio_mdev.c b/drivers/virtio/virtio_mdev.c
new file mode 100644
index ..24c0df5ffe77
--- /dev/null
+++ b/drivers/virtio/virtio_mdev.c
@@ -0,0 +1,409 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * VIRTIO based driver for Mediated device
+ *
+ * Copyright (c) 2019, Red Hat. All rights reserved.
+ * Author: Jason Wang 
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DRIVER_VERSION  "0.1"
+#define DRIVER_AUTHOR   "Red Hat Corporation"
+#define DRIVER_DESC "VIRTIO based driver for Mediated device"
+
+#define to_virtio_mdev_device(dev) \
+   container_of(dev, struct virtio_mdev_device, vdev)
+
+struct virtio_mdev_device {
+   struct virtio_device vdev;
+   struct mdev_device *mdev;
+   unsigned long version;
+
+   /* The lock to protect virtqueue list */
+   spinlock_t lock;
+   /* List of virtio_mdev_vq_info */
+   struct list_head virtqueues;
+};
+
+struct virtio_mdev_vq_info {
+   /* the actual virtqueue */
+   struct virtqueue *vq;
+
+   /* the list node for the virtqueues list */
+   struct list_head node;
+};
+
+static struct mdev_device *vm_get_mdev(struct virtio_device *vdev)
+{
+   struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
+   struct mdev_device *mdev = vm_dev->mdev;
+
+   return mdev;
+}
+
+static void virtio_mdev_get(struct virtio_device *vdev, unsigned offset,
+   void *buf, unsigned len)
+{
+   struct mdev_device *mdev = vm_get_mdev(vdev);
+   const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev);
+
+   ops->get_config(mdev, offset, buf, len);
+}
+
+static void virtio_mdev_set(struct virtio_device *vdev, unsigned offset,
+   const void *buf, unsigned len)
+{
+   struct mdev_device *mdev = vm_get_mdev(vdev);
+   const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev);
+
+   ops->set_config(mdev, offset, buf, len);
+}
+
+static u32 virtio_mdev_generation(struct virtio_device *vdev)
+{
+   struct mdev_device *mdev = vm_get_mdev(vdev);
+   const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev);
+
+   return ops->get_generation(mdev);
+}
+
+static u8 virtio_mdev_get_status(struct virtio_device *vdev)
+{
+   struct mdev_device *mdev = vm_get_mdev(vdev);
+   const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev);
+
+   return ops->get_status(mdev);
+}
+
+static void virtio_mdev_set_status(struct virtio_device *vdev, u8 status)
+{
+   struct mdev_device *mdev = vm_get_mdev(vdev);
+   const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev);
+
+   return ops->set_status(mdev, status);
+}
+
+static void virtio_mdev_reset(struct virtio_device *vdev)
+{
+   struct mdev_device *mdev = vm_get_mdev(vdev);
+   const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev);
+
+   return ops->set_status(mdev, 0);
+}
+
+static bool virtio_mdev_notify(struct virtqueue *vq)
+{
+   struct mdev_device *mdev = vm_get_mdev(vq->vdev);
+   const struct virtio_mdev_device_ops *ops = 

[PATCH V4 4/6] mdev: introduce virtio device and its device ops

2019-10-17 Thread Jason Wang
This patch implements basic support for mdev driver that supports
virtio transport for kernel virtio driver.

Signed-off-by: Jason Wang 
---
 drivers/vfio/mdev/mdev_core.c |  12 +++
 include/linux/mdev.h  |   4 +
 include/linux/virtio_mdev.h   | 151 ++
 3 files changed, 167 insertions(+)
 create mode 100644 include/linux/virtio_mdev.h

diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index d0f3113c8071..5834f6b7c7a5 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -57,6 +57,18 @@ void mdev_set_vfio_ops(struct mdev_device *mdev,
 }
 EXPORT_SYMBOL(mdev_set_vfio_ops);
 
+/* Specify the virtio device ops for the mdev device, this
+ * must be called during create() callback for virtio mdev device.
+ */
+void mdev_set_virtio_ops(struct mdev_device *mdev,
+const struct virtio_mdev_device_ops *virtio_ops)
+{
+   BUG_ON(mdev->class_id);
+   mdev->class_id = MDEV_CLASS_ID_VIRTIO;
+   mdev->device_ops = virtio_ops;
+}
+EXPORT_SYMBOL(mdev_set_virtio_ops);
+
 const void *mdev_get_dev_ops(struct mdev_device *mdev)
 {
return mdev->device_ops;
diff --git a/include/linux/mdev.h b/include/linux/mdev.h
index 3d29e09e20c9..13e045e09d3b 100644
--- a/include/linux/mdev.h
+++ b/include/linux/mdev.h
@@ -17,6 +17,7 @@
 
 struct mdev_device;
 struct vfio_mdev_device_ops;
+struct virtio_mdev_device_ops;
 
 /*
  * Called by the parent device driver to set the device which represents
@@ -111,6 +112,8 @@ void mdev_set_drvdata(struct mdev_device *mdev, void *data);
 const guid_t *mdev_uuid(struct mdev_device *mdev);
 void mdev_set_vfio_ops(struct mdev_device *mdev,
   const struct vfio_mdev_device_ops *vfio_ops);
+void mdev_set_virtio_ops(struct mdev_device *mdev,
+ const struct virtio_mdev_device_ops *virtio_ops);
 const void *mdev_get_dev_ops(struct mdev_device *mdev);
 
 extern struct bus_type mdev_bus_type;
@@ -127,6 +130,7 @@ struct mdev_device *mdev_from_dev(struct device *dev);
 
 enum {
MDEV_CLASS_ID_VFIO = 1,
+   MDEV_CLASS_ID_VIRTIO = 2,
/* New entries must be added here */
 };
 
diff --git a/include/linux/virtio_mdev.h b/include/linux/virtio_mdev.h
new file mode 100644
index ..b965b50f9b24
--- /dev/null
+++ b/include/linux/virtio_mdev.h
@@ -0,0 +1,151 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Virtio mediated device driver
+ *
+ * Copyright 2019, Red Hat Corp.
+ * Author: Jason Wang 
+ */
+#ifndef _LINUX_VIRTIO_MDEV_H
+#define _LINUX_VIRTIO_MDEV_H
+
+#include 
+#include 
+#include 
+
+#define VIRTIO_MDEV_DEVICE_API_STRING  "virtio-mdev"
+#define VIRTIO_MDEV_F_VERSION_1 0x1
+
+struct virtio_mdev_callback {
+   irqreturn_t (*callback)(void *data);
+   void *private;
+};
+
+/**
+ * struct vfio_mdev_device_ops - Structure to be registered for each
+ * mdev device to register the device to virtio-mdev module.
+ *
+ * @set_vq_address:Set the address of virtqueue
+ * @mdev: mediated device
+ * @idx: virtqueue index
+ * @desc_area: address of desc area
+ * @driver_area: address of driver area
+ * @device_area: address of device area
+ * Returns integer: success (0) or error (< 0)
+ * @set_vq_num:Set the size of virtqueue
+ * @mdev: mediated device
+ * @idx: virtqueue index
+ * @num: the size of virtqueue
+ * @kick_vq:   Kick the virtqueue
+ * @mdev: mediated device
+ * @idx: virtqueue index
+ * @set_vq_cb: Set the interrupt callback function for
+ * a virtqueue
+ * @mdev: mediated device
+ * @idx: virtqueue index
+ * @cb: virtio-mdev interrupt callback structure
+ * @set_vq_ready:  Set ready status for a virtqueue
+ * @mdev: mediated device
+ * @idx: virtqueue index
+ * @ready: ready (true) not ready(false)
+ * @get_vq_ready:  Get ready status for a virtqueue
+ * @mdev: mediated device
+ * @idx: virtqueue index
+ * Returns boolean: ready (true) or not (false)
+ * @set_vq_state:  Set the state for a virtqueue
+ * @mdev: mediated device
+ * @idx: virtqueue index
+ * @state: virtqueue state (last_avail_idx)
+ * Returns integer: success (0) or error (< 0)
+ * @get_vq_state:  Get the state for a 

[PATCH V4 3/6] mdev: introduce device specific ops

2019-10-17 Thread Jason Wang
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 
---
 .../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()
+callback. E.g for vfio-mdev device it needs to be done through:
+
+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
+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
+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);
-
 However, the mdev_parent_ops structure is not required in the function call
 that a driver should use to unregister itself with the mdev core driver::
 
diff --git a/MAINTAINERS b/MAINTAINERS
index 8824f61cd2c0..3d196a023b5e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17127,6 +17127,7 @@ S:  Maintained
 F: Documentation/driver-api/vfio-mediated-device.rst
 F: drivers/vfio/mdev/
 F: include/linux/mdev.h
+F: include/linux/vfio_mdev.h
 F: samples/vfio-mdev/
 
 VFIO PLATFORM DRIVER
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 6420f0dbd31b..306f934c7857 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -42,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -643,6 +644,8 @@ static void kvmgt_put_vfio_device(void *vgpu)
vfio_device_put(((struct intel_vgpu *)vgpu)->vdev.vfio_device);
 }
 
+static const struct vfio_mdev_device_ops intel_vfio_vgpu_dev_ops;
+
 static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
 {
struct intel_vgpu *vgpu = NULL;
@@ -678,7 +681,7 @@ static int intel_vgpu_create(struct kobject *kobj, struct 
mdev_device *mdev)
 dev_name(mdev_dev(mdev)));
ret = 0;
 
-   mdev_set_class(mdev, MDEV_CLASS_ID_VFIO);
+   mdev_set_vfio_ops(mdev, _vfio_vgpu_dev_ops);
 out:
return ret;
 }
@@ -1599,20 +1602,21 @@ static const struct attribute_group 
*intel_vgpu_groups[] = {
NULL,
 };
 
-static struct mdev_parent_ops intel_vgpu_ops = {
-   .mdev_attr_groups   = intel_vgpu_groups,
-   .create = intel_vgpu_create,
-   .remove = intel_vgpu_remove,
-
+static const struct vfio_mdev_device_ops 

[PATCH V4 2/6] modpost: add support for mdev class id

2019-10-17 Thread Jason Wang
Add support to parse mdev class id table.

Signed-off-by: Jason Wang 
---
 drivers/vfio/mdev/vfio_mdev.c |  2 ++
 scripts/mod/devicetable-offsets.c |  3 +++
 scripts/mod/file2alias.c  | 10 ++
 3 files changed, 15 insertions(+)

diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
index 7b24ee9cb8dd..cb701cd646f0 100644
--- a/drivers/vfio/mdev/vfio_mdev.c
+++ b/drivers/vfio/mdev/vfio_mdev.c
@@ -125,6 +125,8 @@ static const struct mdev_class_id id_table[] = {
{ 0 },
 };
 
+MODULE_DEVICE_TABLE(mdev, id_table);
+
 static struct mdev_driver vfio_mdev_driver = {
.name   = "vfio_mdev",
.probe  = vfio_mdev_probe,
diff --git a/scripts/mod/devicetable-offsets.c 
b/scripts/mod/devicetable-offsets.c
index 054405b90ba4..6cbb1062488a 100644
--- a/scripts/mod/devicetable-offsets.c
+++ b/scripts/mod/devicetable-offsets.c
@@ -231,5 +231,8 @@ int main(void)
DEVID(wmi_device_id);
DEVID_FIELD(wmi_device_id, guid_string);
 
+   DEVID(mdev_class_id);
+   DEVID_FIELD(mdev_class_id, id);
+
return 0;
 }
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index c91eba751804..d365dfe7c718 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -1335,6 +1335,15 @@ static int do_wmi_entry(const char *filename, void 
*symval, char *alias)
return 1;
 }
 
+/* looks like: "mdev:cN" */
+static int do_mdev_entry(const char *filename, void *symval, char *alias)
+{
+   DEF_FIELD(symval, mdev_class_id, id);
+
+   sprintf(alias, "mdev:c%02X", id);
+   return 1;
+}
+
 /* Does namelen bytes of name exactly match the symbol? */
 static bool sym_is(const char *name, unsigned namelen, const char *symbol)
 {
@@ -1407,6 +1416,7 @@ static const struct devtable devtable[] = {
{"typec", SIZE_typec_device_id, do_typec_entry},
{"tee", SIZE_tee_client_device_id, do_tee_entry},
{"wmi", SIZE_wmi_device_id, do_wmi_entry},
+   {"mdev", SIZE_mdev_class_id, do_mdev_entry},
 };
 
 /* Create MODULE_ALIAS() statements.
-- 
2.19.1

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


[PATCH V4 1/6] mdev: class id support

2019-10-17 Thread Jason Wang
Mdev bus only supports vfio driver right now, so it doesn't implement
match method. But in the future, we may add drivers other than vfio,
the first driver could be virtio-mdev. This means we need to add
device class id support in bus match method to pair the mdev device
and mdev driver correctly.

So this patch adds id_table to mdev_driver and class_id for mdev
device with the match method for mdev bus.

Signed-off-by: Jason Wang 
---
 .../driver-api/vfio-mediated-device.rst   |  7 +-
 drivers/gpu/drm/i915/gvt/kvmgt.c  |  1 +
 drivers/s390/cio/vfio_ccw_ops.c   |  1 +
 drivers/s390/crypto/vfio_ap_ops.c |  1 +
 drivers/vfio/mdev/mdev_core.c | 18 +++
 drivers/vfio/mdev/mdev_driver.c   | 22 +++
 drivers/vfio/mdev/mdev_private.h  |  1 +
 drivers/vfio/mdev/vfio_mdev.c |  6 +
 include/linux/mdev.h  |  8 +++
 include/linux/mod_devicetable.h   |  8 +++
 samples/vfio-mdev/mbochs.c|  1 +
 samples/vfio-mdev/mdpy.c  |  1 +
 samples/vfio-mdev/mtty.c  |  1 +
 13 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/Documentation/driver-api/vfio-mediated-device.rst 
b/Documentation/driver-api/vfio-mediated-device.rst
index 25eb7d5b834b..f9a78d75a67a 100644
--- a/Documentation/driver-api/vfio-mediated-device.rst
+++ b/Documentation/driver-api/vfio-mediated-device.rst
@@ -102,12 +102,14 @@ structure to represent a mediated device's driver::
   * @probe: called when new device created
   * @remove: called when device removed
   * @driver: device driver structure
+  * @id_table: the ids serviced by this driver
   */
  struct mdev_driver {
 const char *name;
 int  (*probe)  (struct device *dev);
 void (*remove) (struct device *dev);
 struct device_driverdriver;
+const struct mdev_class_id *id_table;
  };
 
 A mediated bus driver for mdev should use this structure in the function calls
@@ -165,12 +167,15 @@ 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);
+
 However, the mdev_parent_ops structure is not required in the function call
 that a driver should use to unregister itself with the mdev core driver::
 
extern void mdev_unregister_device(struct device *dev);
 
-
 Mediated Device Management Interface Through sysfs
 ==
 
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 343d79c1cb7e..6420f0dbd31b 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -678,6 +678,7 @@ static int intel_vgpu_create(struct kobject *kobj, struct 
mdev_device *mdev)
 dev_name(mdev_dev(mdev)));
ret = 0;
 
+   mdev_set_class(mdev, MDEV_CLASS_ID_VFIO);
 out:
return ret;
 }
diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
index f0d71ab77c50..cf2c013ae32f 100644
--- a/drivers/s390/cio/vfio_ccw_ops.c
+++ b/drivers/s390/cio/vfio_ccw_ops.c
@@ -129,6 +129,7 @@ static int vfio_ccw_mdev_create(struct kobject *kobj, 
struct mdev_device *mdev)
   private->sch->schid.ssid,
   private->sch->schid.sch_no);
 
+   mdev_set_class(mdev, MDEV_CLASS_ID_VFIO);
return 0;
 }
 
diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
b/drivers/s390/crypto/vfio_ap_ops.c
index 5c0f53c6dde7..07c31070afeb 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -343,6 +343,7 @@ static int vfio_ap_mdev_create(struct kobject *kobj, struct 
mdev_device *mdev)
list_add(_mdev->node, _dev->mdev_list);
mutex_unlock(_dev->lock);
 
+   mdev_set_class(mdev, MDEV_CLASS_ID_VFIO);
return 0;
 }
 
diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index b558d4cfd082..3a9c52d71b4e 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -45,6 +45,16 @@ void mdev_set_drvdata(struct mdev_device *mdev, void *data)
 }
 EXPORT_SYMBOL(mdev_set_drvdata);
 
+/* Specify the class for the mdev device, this must be called during
+ * create() callback.
+ */
+void mdev_set_class(struct mdev_device *mdev, u16 id)
+{
+   WARN_ON(mdev->class_id);
+   mdev->class_id = id;
+}
+EXPORT_SYMBOL(mdev_set_class);
+
 struct device *mdev_dev(struct mdev_device *mdev)
 {
return >dev;
@@ -135,6 +145,7 @@ static int mdev_device_remove_cb(struct device *dev, void 
*data)
  * mdev_register_device : Register a device
  * @dev: device structure representing 

[PATCH V4 0/6] mdev based hardware virtio offloading support

2019-10-17 Thread Jason Wang
Hi all:

There are hardwares that can do virtio datapath offloading while
having its own control path. This path tries to implement a mdev based
unified API to support using kernel virtio driver to drive those
devices. This is done by introducing a new mdev transport for virtio
(virtio_mdev) and register itself as a new kind of mdev driver. Then
it provides a unified way for kernel virtio driver to talk with mdev
device implementation.

Though the series only contains kernel driver support, the goal is to
make the transport generic enough to support userspace drivers. This
means vhost-mdev[1] could be built on top as well by resuing the
transport.

A sample driver is also implemented which simulate a virito-net
loopback ethernet device on top of vringh + workqueue. This could be
used as a reference implementation for real hardware driver.

Also a real ICF VF driver was also posted here[2] which is a good
reference for vendors who is interested in their own virtio datapath
offloading product.

Consider mdev framework only support VFIO device and driver right now,
this series also extend it to support other types. This is done
through introducing class id to the device and pairing it with
id_talbe claimed by the driver. On top, this seris also decouple
device specific parents ops out of the common ones.

Pktgen test was done with virito-net + mvnet loop back device.

Please review.

[1] https://lkml.org/lkml/2019/9/26/15
[2] https://lkml.org/lkml/2019/10/15/1226

Changes from V3:

- document that class id (device ops) must be specified in create()
- add WARN() when trying to set class_id when it has already set
- add WARN() when class_id is not specified in create() and correctly
  return an error in this case
- correct the prototype of mdev_set_class() in the doc
- add documention of mdev_set_class()
- remove the unnecessary "class_id_fail" label when class id is not
  specified in create()
- convert id_table in vfio_mdev to const
- move mdev_set_class and its friends after mdev_uuid()
- suqash the patch of bus uevent into patch of introducing class id
- tweak the words in the docs per Cornelia suggestion
- tie class_id and device ops through class specific initialization
  routine like mdev_set_vfio_ops()
- typos fixes in the docs of virtio-mdev callbacks
- document the usage of virtqueues in struct virtio_mdev_device
- remove the useless vqs array in struct virtio_mdev_device
- rename MDEV_ID_XXX to MDEV_CLASS_ID_XXX

Changes from V2:

- fail when class_id is not specified
- drop the vringh patch
- match the doc to the code
- tweak the commit log
- move device_ops from parent to mdev device
- remove the unused MDEV_ID_VHOST

Changes from V1:

- move virtio_mdev.c to drivers/virtio
- store class_id in mdev_device instead of mdev_parent
- store device_ops in mdev_device instead of mdev_parent
- reorder the patch, vringh fix comes first
- really silent compiling warnings
- really switch to use u16 for class_id
- uevent and modpost support for mdev class_id
- vraious tweaks per comments from Parav

Changes from RFC-V2:

- silent compile warnings on some specific configuration
- use u16 instead u8 for class id
- reseve MDEV_ID_VHOST for future vhost-mdev work
- introduce "virtio" type for mvnet and make "vhost" type for future
  work
- add entries in MAINTAINER
- tweak and typos fixes in commit log

Changes from RFC-V1:

- rename device id to class id
- add docs for class id and device specific ops (device_ops)
- split device_ops into seperate headers
- drop the mdev_set_dma_ops()
- use device_ops to implement the transport API, then it's not a part
  of UAPI any more
- use GFP_ATOMIC in mvnet sample device and other tweaks
- set_vring_base/get_vring_base support for mvnet device

Jason Wang (6):
  mdev: class id support
  modpost: add support for mdev class id
  mdev: introduce device specific ops
  mdev: introduce virtio device and its device ops
  virtio: introduce a mdev based transport
  docs: sample driver to demonstrate how to implement virtio-mdev
framework

 .../driver-api/vfio-mediated-device.rst   |  22 +-
 MAINTAINERS   |   2 +
 drivers/gpu/drm/i915/gvt/kvmgt.c  |  17 +-
 drivers/s390/cio/vfio_ccw_ops.c   |  17 +-
 drivers/s390/crypto/vfio_ap_ops.c |  13 +-
 drivers/vfio/mdev/mdev_core.c |  38 +
 drivers/vfio/mdev/mdev_driver.c   |  22 +
 drivers/vfio/mdev/mdev_private.h  |   2 +
 drivers/vfio/mdev/vfio_mdev.c |  45 +-
 drivers/virtio/Kconfig|   7 +
 drivers/virtio/Makefile   |   1 +
 drivers/virtio/virtio_mdev.c  | 409 +++
 include/linux/mdev.h  |  55 +-
 include/linux/mod_devicetable.h   |   8 +
 include/linux/vfio_mdev.h |  52 ++
 include/linux/virtio_mdev.h   | 151 
 samples/Kconfig   |   7 +
 

Re: [PATCH V3 0/7] mdev based hardware virtio offloading support

2019-10-17 Thread Stefan Hajnoczi
On Thu, Oct 17, 2019 at 09:42:53AM +0800, Jason Wang wrote:
> 
> On 2019/10/15 下午10:37, Stefan Hajnoczi wrote:
> > On Tue, Oct 15, 2019 at 11:37:17AM +0800, Jason Wang wrote:
> > > On 2019/10/15 上午1:49, Stefan Hajnoczi wrote:
> > > > On Fri, Oct 11, 2019 at 04:15:50PM +0800, Jason Wang wrote:
> > > > > There are hardware that can do virtio datapath offloading while having
> > > > > its own control path. This path tries to implement a mdev based
> > > > > unified API to support using kernel virtio driver to drive those
> > > > > devices. This is done by introducing a new mdev transport for virtio
> > > > > (virtio_mdev) and register itself as a new kind of mdev driver. Then
> > > > > it provides a unified way for kernel virtio driver to talk with mdev
> > > > > device implementation.
> > > > > 
> > > > > Though the series only contains kernel driver support, the goal is to
> > > > > make the transport generic enough to support userspace drivers. This
> > > > > means vhost-mdev[1] could be built on top as well by resuing the
> > > > > transport.
> > > > > 
> > > > > A sample driver is also implemented which simulate a virito-net
> > > > > loopback ethernet device on top of vringh + workqueue. This could be
> > > > > used as a reference implementation for real hardware driver.
> > > > > 
> > > > > Consider mdev framework only support VFIO device and driver right now,
> > > > > this series also extend it to support other types. This is done
> > > > > through introducing class id to the device and pairing it with
> > > > > id_talbe claimed by the driver. On top, this seris also decouple
> > > > > device specific parents ops out of the common ones.
> > > > I was curious so I took a quick look and posted comments.
> > > > 
> > > > I guess this driver runs inside the guest since it registers virtio
> > > > devices?
> > > 
> > > It could run in either guest or host. But the main focus is to run in the
> > > host then we can use virtio drivers in containers.
> > > 
> > > 
> > > > If this is used with physical PCI devices that support datapath
> > > > offloading then how are physical devices presented to the guest without
> > > > SR-IOV?
> > > 
> > > We will do control path meditation through vhost-mdev[1] and 
> > > vhost-vfio[2].
> > > Then we will present a full virtio compatible ethernet device for guest.
> > > 
> > > SR-IOV is not a must, any mdev device that implements the API defined in
> > > patch 5 can be used by this framework.
> > What I'm trying to understand is: if you want to present a virtio-pci
> > device to the guest (e.g. using vhost-mdev or vhost-vfio), then how is
> > that related to this patch series?
> 
> 
> This series introduce some infrastructure that would be used by vhost-mdev:
> 
> 1) allow new type of mdev devices/drivers other than vfio (through class_id
> and device ops)
> 
> 2) a set of virtio specific callbacks that will be used by both vhost-mdev
> and virtio-mdev defined in patch 5
> 
> Then vhost-mdev can be implemented on top: a new mdev class id but reuse the
> callback defined in 2. Through this way the parent can provides a single set
> of callbacks (device ops) for both kernel virtio driver (through
> virtio-mdev) or userspace virtio driver (through vhost-mdev).

Okay, thanks for explaining!

Stefan


signature.asc
Description: PGP signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH V3 4/7] mdev: introduce device specific ops

2019-10-17 Thread Jason Wang


On 2019/10/17 下午4:45, Cornelia Huck wrote:

On Thu, 17 Oct 2019 16:30:43 +0800
Jason Wang  wrote:


On 2019/10/17 上午12:53, Alex Williamson wrote:

Yet another suggestion: have the class id derive from the function
you use to set up the ops.

void mdev_set_vfio_ops(struct mdev_device *mdev, const struct
vfio_mdev_ops *vfio_ops) {
mdev->device_ops = vfio_ops;
mdev->class_id = MDEV_ID_VFIO;
}

void mdev_set_virtio_ops(struct mdev_device *mdev, const struct
virtio_mdev_ops *virtio_ops) {
mdev->device_ops = virtio_ops;
mdev->class_id = MDEV_ID_VIRTIO;
}

void mdev_set_vhost_ops(struct mdev_device *mdev, const struct
virtio_mdev_ops *virtio_ops) {
mdev->device_ops = virtio_ops;
mdev->class_id = MDEV_ID_VHOST;
}

void mdev_set_vendor_ops(struct mdev_device *mdev) /* no ops */ {
mdev->class_id = MDEV_ID_VENDOR;
}

One further step towards making this hard to use incorrectly might be
to return an error if class_id is already set.  Thanks,

Alex


I will add a BUG_ON() when class_id has already set.

Probably better a WARN_ON()?



Right.

Thanks

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

Re: [PATCH V3 4/7] mdev: introduce device specific ops

2019-10-17 Thread Cornelia Huck
On Thu, 17 Oct 2019 16:30:43 +0800
Jason Wang  wrote:

> On 2019/10/17 上午12:53, Alex Williamson wrote:

> >>> Yet another suggestion: have the class id derive from the function
> >>> you use to set up the ops.
> >>>
> >>> void mdev_set_vfio_ops(struct mdev_device *mdev, const struct
> >>> vfio_mdev_ops *vfio_ops) {
> >>>   mdev->device_ops = vfio_ops;
> >>>   mdev->class_id = MDEV_ID_VFIO;
> >>> }
> >>>
> >>> void mdev_set_virtio_ops(struct mdev_device *mdev, const struct
> >>> virtio_mdev_ops *virtio_ops) {
> >>>   mdev->device_ops = virtio_ops;
> >>>   mdev->class_id = MDEV_ID_VIRTIO;
> >>> }
> >>>
> >>> void mdev_set_vhost_ops(struct mdev_device *mdev, const struct
> >>> virtio_mdev_ops *virtio_ops) {
> >>>   mdev->device_ops = virtio_ops;
> >>>   mdev->class_id = MDEV_ID_VHOST;
> >>> }
> >>>
> >>> void mdev_set_vendor_ops(struct mdev_device *mdev) /* no ops */ {
> >>>   mdev->class_id = MDEV_ID_VENDOR;
> >>> }  
> > One further step towards making this hard to use incorrectly might be
> > to return an error if class_id is already set.  Thanks,
> >
> > Alex  
> 
> 
> I will add a BUG_ON() when class_id has already set.

Probably better a WARN_ON()?
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH V3 4/7] mdev: introduce device specific ops

2019-10-17 Thread Jason Wang


On 2019/10/17 上午12:53, Alex Williamson wrote:

On Wed, 16 Oct 2019 15:31:25 +
Parav Pandit  wrote:


-Original Message-
From: Cornelia Huck 
Sent: Wednesday, October 16, 2019 3:53 AM
To: Parav Pandit 
Cc: Alex Williamson ; Jason Wang
; k...@vger.kernel.org; linux-s...@vger.kernel.org;
linux-ker...@vger.kernel.org; dri-de...@lists.freedesktop.org; intel-
g...@lists.freedesktop.org; intel-gvt-...@lists.freedesktop.org;
kwankh...@nvidia.com; m...@redhat.com; tiwei@intel.com;
virtualization@lists.linux-foundation.org; net...@vger.kernel.org;
maxime.coque...@redhat.com; cunming.li...@intel.com;
zhihong.w...@intel.com; rob.mil...@broadcom.com; xiao.w.w...@intel.com;
haotian.w...@sifive.com; zhen...@linux.intel.com; zhi.a.w...@intel.com;
jani.nik...@linux.intel.com; joonas.lahti...@linux.intel.com;
rodrigo.v...@intel.com; airl...@linux.ie; dan...@ffwll.ch;
far...@linux.ibm.com; pa...@linux.ibm.com; seb...@linux.ibm.com;
ober...@linux.ibm.com; heiko.carst...@de.ibm.com; g...@linux.ibm.com;
borntrae...@de.ibm.com; akrow...@linux.ibm.com; fre...@linux.ibm.com;
lingshan@intel.com; Ido Shamay ;
epere...@redhat.com; l...@redhat.com; christophe.de.dinec...@gmail.com;
kevin.t...@intel.com
Subject: Re: [PATCH V3 4/7] mdev: introduce device specific ops

On Wed, 16 Oct 2019 05:50:08 +
Parav Pandit  wrote:
   

Hi Alex,
  

-Original Message-
From: Alex Williamson 
Sent: Tuesday, October 15, 2019 12:27 PM
To: Jason Wang 
Cc: Cornelia Huck ; k...@vger.kernel.org; linux-
s...@vger.kernel.org; linux-ker...@vger.kernel.org; dri-
de...@lists.freedesktop.org; intel-...@lists.freedesktop.org;
intel-gvt- d...@lists.freedesktop.org; kwankh...@nvidia.com;
m...@redhat.com; tiwei@intel.com;
virtualization@lists.linux-foundation.org;
net...@vger.kernel.org; maxime.coque...@redhat.com;
cunming.li...@intel.com; zhihong.w...@intel.com;
rob.mil...@broadcom.com; xiao.w.w...@intel.com;
haotian.w...@sifive.com; zhen...@linux.intel.com;
zhi.a.w...@intel.com; jani.nik...@linux.intel.com;
joonas.lahti...@linux.intel.com; rodrigo.v...@intel.com;
airl...@linux.ie; dan...@ffwll.ch; far...@linux.ibm.com;
pa...@linux.ibm.com; seb...@linux.ibm.com; ober...@linux.ibm.com;
heiko.carst...@de.ibm.com; g...@linux.ibm.com;
borntrae...@de.ibm.com; akrow...@linux.ibm.com;
fre...@linux.ibm.com; lingshan@intel.com; Ido Shamay
; epere...@redhat.com; l...@redhat.com; Parav
Pandit ; christophe.de.dinec...@gmail.com;
kevin.t...@intel.com
Subject: Re: [PATCH V3 4/7] mdev: introduce device specific ops

On Tue, 15 Oct 2019 20:17:01 +0800
Jason Wang  wrote:
  

On 2019/10/15 下午6:41, Cornelia Huck wrote:

On Fri, 11 Oct 2019 16:15:54 +0800 Jason Wang
 wrote:
   

@@ -167,9 +176,10 @@ 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 through::
+It is also required to specify the class_id and device
+specific ops

through::

-   extern int mdev_set_class(struct device *dev, u16 id);
+   extern int mdev_set_class(struct device *dev, u16 id,
+ const void *ops);

Apologies if that has already been discussed, but do we want a
1:1 relationship between id and ops, or can different devices
with the same id register different ops?


I think we have a N:1 mapping between id and ops, e.g we want both
virtio-mdev and vhost-mdev use a single set of device ops.

The contents of the ops structure is essentially defined by the id,
which is why I was leaning towards them being defined together.
They are effectively interlocked, the id defines which mdev "endpoint"
driver is loaded and that driver requires mdev_get_dev_ops() to
return the structure required by the driver.  I wish there was a way
we could incorporate type checking here.  We toyed with the idea of
having the class in the same structure as the ops, but I think this
approach was chosen for simplicity.  We could still do something like:

int mdev_set_class_struct(struct device *dev, const struct
mdev_class_struct *class);

struct mdev_class_struct {
u16 id;
union {
struct vfio_mdev_ops vfio_ops;
struct virtio_mdev_ops virtio_ops;
};
};

Maybe even:

struct vfio_mdev_ops *mdev_get_vfio_ops(struct mdev_device *mdev) {
BUG_ON(mdev->class.id != MDEV_ID_VFIO);
return >class.vfio_ops;
}

The match callback would of course just use the mdev->class.id value.
Functionally equivalent, but maybe better type characteristics.
Thanks,

Alex

We have 3 use cases of mdev.
1. current mdev binding to vfio_mdev
2. mdev binding to virtio
3. mdev binding to mlx5_core without dev_ops

Also
(a) a given parent may serve multiple types of classes in future.
(b) number of classes may not likely explode, they will be handful of
them. (vfio_mdev, virtio)

So, instead of making copies of this dev_ops pointer in each mdev, it 

Re: [PATCH V3 1/7] mdev: class id support

2019-10-17 Thread Jason Wang


On 2019/10/16 下午12:57, Parav Pandit wrote:

+static struct mdev_class_id id_table[] = {

static const


+   { MDEV_ID_VFIO },

I guess you don't need extra braces for each entry.
Since this enum represents MDEV class id, it better to name it as 
MDEV_CLASS_ID_VFIO.
(Similar to  PCI_VENDOR_ID, PCI_DEVICE_ID)..



Gcc start to complain like:

warning: missing braces around initializer [-Wmissing-braces]
 static const struct mdev_class_id id_table[] = {
    ^
  MDEV_ID_VFIO, 0,
  {   } {
 };
 }

So I will keep this part untouched.

Thanks



+   { 0 },
+};

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