Re: [PATCH 04/10] driver core: Don't return EPROBE_DEFER to userspace during sysfs bind

Re: [PATCH 04/10] driver core: Don't return EPROBE_DEFER to userspace during sysfs bind

[Cornelia Huck]

On Tue, Jun 15 2021, Christoph Hellwig <hch@lst.de> wrote:

> EPROBE_DEFER is an internal kernel error code and it should not be leaked
> to userspace via the bind_store() sysfs. Userspace doesn't have this
> constant and cannot understand it.
>
> Further, it doesn't really make sense to have userspace trigger a deferred
> probe via bind_store(), which could eventually succeed, while
> simultaneously returning an error back.
>
> Resolve this by splitting driver_probe_device so that the version used
> by the sysfs binding that turns EPROBE_DEFER into -EAGAIN, while the one
> used for internally binding keeps the error code, and calls
> driver_deferred_probe_add where needed.  This also allows to nicely split
> out the defer_all_probes / probe_count checks so that they actually allow
> for full device_{block,unblock}_probing protection while not bothering
> the sysfs bind case.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/base/dd.c | 78 +++++++++++++++++++++++++----------------------
>  1 file changed, 42 insertions(+), 36 deletions(-)
>

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH 04/10] driver core: Don't return EPROBE_DEFER to userspace during sysfs bind

[Alex Williamson]

On Tue, 15 Jun 2021 15:35:13 +0200
Christoph Hellwig <hch@lst.de> wrote:

> @@ -547,10 +538,9 @@ static int call_driver_probe(struct device *dev, struct device_driver *drv)
>  
>  static int really_probe(struct device *dev, struct device_driver *drv)
>  {
> -	int local_trigger_count = atomic_read(&deferred_trigger_count);
>  	bool test_remove = IS_ENABLED(CONFIG_DEBUG_TEST_DRIVER_REMOVE) &&
>  			   !drv->suppress_bind_attrs;
> -	int ret = -EPROBE_DEFER, probe_ret = 0;
> +	int ret, probe_ret = 0;

nit, probe_ret initialization could be removed with this patch too.

>  
>  	if (defer_all_probes) {
>  		/*
> @@ -559,17 +549,13 @@ static int really_probe(struct device *dev, struct device_driver *drv)
>  		 * wait_for_device_probe() right after that to avoid any races.
>  		 */
>  		dev_dbg(dev, "Driver %s force probe deferral\n", drv->name);
> -		driver_deferred_probe_add(dev);
> -		return ret;
> +		return -EPROBE_DEFER;
>  	}
>  
>  	ret = device_links_check_suppliers(dev);
> -	if (ret == -EPROBE_DEFER)
> -		driver_deferred_probe_add_trigger(dev, local_trigger_count);
>  	if (ret)
>  		return ret;
>  
> -	atomic_inc(&probe_count);
>  	pr_debug("bus: '%s': %s: probing driver %s with device %s\n",
>  		 drv->bus->name, __func__, drv->name, dev_name(dev));
>  	if (!list_empty(&dev->devres_head)) {
> @@ -681,11 +667,7 @@ static int really_probe(struct device *dev, struct device_driver *drv)
>  		dev->pm_domain->dismiss(dev);
>  	pm_runtime_reinit(dev);
>  	dev_pm_set_driver_flags(dev, 0);
> -	if (probe_ret == -EPROBE_DEFER)
> -		driver_deferred_probe_add_trigger(dev, local_trigger_count);

This was the only possible uninitialized use case afaict.  Thanks,

Alex

Re: [PATCH 07/10] vfio/mdev: Allow the mdev_parent_ops to specify the device driver to bind

[Cornelia Huck]

On Tue, Jun 15 2021, Christoph Hellwig <hch@lst.de> wrote:

> From: Jason Gunthorpe <jgg@nvidia.com>
>
> This allows a mdev driver to opt out of using vfio_mdev.c, instead the
> driver will provide a 'struct mdev_driver' and register directly with the
> driver core.
>
> Much of mdev_parent_ops becomes unused in this mode:
> - create()/remove() are done via the mdev_driver probe()/remove()
> - mdev_attr_groups becomes mdev_driver driver.dev_groups
> - Wrapper function callbacks are replaced with the same ones from
>   struct vfio_device_ops
>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/vfio/mdev/mdev_core.c   | 30 ++++++++++++++++++++++--------
>  drivers/vfio/mdev/mdev_driver.c | 10 ++++++++++
>  include/linux/mdev.h            |  2 ++
>  3 files changed, 34 insertions(+), 8 deletions(-)
>

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH 07/10] vfio/mdev: Allow the mdev_parent_ops to specify the device driver to bind

[Greg Kroah-Hartman]

On Tue, Jun 15, 2021 at 03:35:16PM +0200, Christoph Hellwig wrote:
> From: Jason Gunthorpe <jgg@nvidia.com>
> 
> This allows a mdev driver to opt out of using vfio_mdev.c, instead the
> driver will provide a 'struct mdev_driver' and register directly with the
> driver core.
> 
> Much of mdev_parent_ops becomes unused in this mode:
> - create()/remove() are done via the mdev_driver probe()/remove()
> - mdev_attr_groups becomes mdev_driver driver.dev_groups
> - Wrapper function callbacks are replaced with the same ones from
>   struct vfio_device_ops
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Messy, but ok...

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 07/10] vfio/mdev: Allow the mdev_parent_ops to specify the device driver to bind

[Jason Gunthorpe]

On Tue, Jun 15, 2021 at 04:11:29PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 15, 2021 at 03:35:16PM +0200, Christoph Hellwig wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > 
> > This allows a mdev driver to opt out of using vfio_mdev.c, instead the
> > driver will provide a 'struct mdev_driver' and register directly with the
> > driver core.
> > 
> > Much of mdev_parent_ops becomes unused in this mode:
> > - create()/remove() are done via the mdev_driver probe()/remove()
> > - mdev_attr_groups becomes mdev_driver driver.dev_groups
> > - Wrapper function callbacks are replaced with the same ones from
> >   struct vfio_device_ops
> > 
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Messy, but ok...

Is there something you'd like to see changed, eg in later patches?
This whole work still has another approx 30 patches to go and much of
this ends up being erased once the drivers are all converted.

Jason

Re: [PATCH 07/10] vfio/mdev: Allow the mdev_parent_ops to specify the device driver to bind

[Greg Kroah-Hartman]

On Tue, Jun 15, 2021 at 09:00:40PM -0300, Jason Gunthorpe wrote:
> On Tue, Jun 15, 2021 at 04:11:29PM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Jun 15, 2021 at 03:35:16PM +0200, Christoph Hellwig wrote:
> > > From: Jason Gunthorpe <jgg@nvidia.com>
> > > 
> > > This allows a mdev driver to opt out of using vfio_mdev.c, instead the
> > > driver will provide a 'struct mdev_driver' and register directly with the
> > > driver core.
> > > 
> > > Much of mdev_parent_ops becomes unused in this mode:
> > > - create()/remove() are done via the mdev_driver probe()/remove()
> > > - mdev_attr_groups becomes mdev_driver driver.dev_groups
> > > - Wrapper function callbacks are replaced with the same ones from
> > >   struct vfio_device_ops
> > > 
> > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > 
> > Messy, but ok...
> 
> Is there something you'd like to see changed, eg in later patches?
> This whole work still has another approx 30 patches to go and much of
> this ends up being erased once the drivers are all converted.

If this mostly gets removed in the end, I'm happy.  Let's see how it
looks after all of that is done.  This is going forward in the right
way, so I do not object to this at all.

thanks,

greg k-h

Re: [PATCH 07/10] vfio/mdev: Allow the mdev_parent_ops to specify the device driver to bind

[Kirti Wankhede]

On 6/15/2021 7:05 PM, Christoph Hellwig wrote:
> From: Jason Gunthorpe <jgg@nvidia.com>
> 
> This allows a mdev driver to opt out of using vfio_mdev.c, instead the
> driver will provide a 'struct mdev_driver' and register directly with the
> driver core.
> 
> Much of mdev_parent_ops becomes unused in this mode:
> - create()/remove() are done via the mdev_driver probe()/remove()
> - mdev_attr_groups becomes mdev_driver driver.dev_groups
> - Wrapper function callbacks are replaced with the same ones from
>    struct vfio_device_ops
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/vfio/mdev/mdev_core.c   | 30 ++++++++++++++++++++++--------
>   drivers/vfio/mdev/mdev_driver.c | 10 ++++++++++
>   include/linux/mdev.h            |  2 ++
>   3 files changed, 34 insertions(+), 8 deletions(-)

Reviewed-by: Kirti Wankhede <kwankhede@nvidia.com>

Re: [PATCH 05/10] driver core: Export device_driver_attach()

[Greg Kroah-Hartman]

On Tue, Jun 15, 2021 at 03:35:14PM +0200, Christoph Hellwig wrote:
> From: Jason Gunthorpe <jgg@nvidia.com>
> 
> This is intended as a replacement API for device_bind_driver(). It has at
> least the following benefits:
> 
> - Internal locking. Few of the users of device_bind_driver() follow the
>   locking rules
> 
> - Calls device driver probe() internally. Notably this means that devm
>   support for probe works correctly as probe() error will call
>   devres_release_all()
> 
> - struct device_driver -> dev_groups is supported
> 
> - Simplified calling convention, no need to manually call probe().
> 
> The general usage is for situations that already know what driver to bind
> and need to ensure the bind is synchronized with other logic. Call
> device_driver_attach() after device_add().
> 
> If probe() returns a failure then this will be preserved up through to the
> error return of device_driver_attach().
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> ---

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 09/10] vfio/mdpy: Convert to use vfio_register_group_dev()

[Greg Kroah-Hartman]

On Tue, Jun 15, 2021 at 03:35:18PM +0200, Christoph Hellwig wrote:
> From: Jason Gunthorpe <jgg@nvidia.com>
> 
> This is straightforward conversion, the mdev_state is actually serving as
> the vfio_device and we can replace all the mdev_get_drvdata()'s and the
> wonky dead code with a simple container_of().
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  samples/vfio-mdev/mdpy.c | 159 ++++++++++++++++++++++-----------------
>  1 file changed, 88 insertions(+), 71 deletions(-)

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 10/10] vfio/mbochs: Convert to use vfio_register_group_dev()

[Greg Kroah-Hartman]

On Tue, Jun 15, 2021 at 03:35:19PM +0200, Christoph Hellwig wrote:
> From: Jason Gunthorpe <jgg@nvidia.com>
> 
> This is straightforward conversion, the mdev_state is actually serving as
> the vfio_device and we can replace all the mdev_get_drvdata()'s and the
> wonky dead code with a simple container_of().
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  samples/vfio-mdev/mbochs.c | 163 +++++++++++++++++++++----------------
>  1 file changed, 91 insertions(+), 72 deletions(-)

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: Allow mdev drivers to directly create the vfio_device (v3)

[Alex Williamson]

On Tue, 15 Jun 2021 15:35:09 +0200
Christoph Hellwig <hch@lst.de> wrote:

> This is my alternative take on this series from Jason:
> 
> https://lore.kernel.org/dri-devel/87czsszi9i.fsf@redhat.com/T/
> 
> The mdev/vfio parts are exactly the same, but this solves the driver core
> changes for the direct probing without the in/out flag that Greg hated,
> which cause a little more work, but probably make the result better.
> 
> Original decription from Jason below:
> 
> The mdev bus's core part for managing the lifecycle of devices is mostly
> as one would expect for a driver core bus subsystem.
> 
> However instead of having a normal 'struct device_driver' and binding the
> actual mdev drivers through the standard driver core mechanisms it open
> codes this with the struct mdev_parent_ops and provides a single driver
> that shims between the VFIO core's struct vfio_device and the actual
> device driver.
> 
> Instead, allow mdev drivers implement an actual struct mdev_driver and
> directly call vfio_register_group_dev() in the probe() function for the
> mdev. Arrange to bind the created mdev_device to the mdev_driver that is
> provided by the end driver.
> 
> The actual execution flow doesn't change much, eg what was
> parent_ops->create is now device_driver->probe and it is called at almost
> the exact same time - except under the normal control of the driver core.
> 
> Ultimately converting all the drivers unlocks a fair number of additional
> VFIO simplifications and cleanups.

Looks like we need an update to
Documentation/driver-api/vfio-mediated-device.rst to go along with
this.

Also, if we're preserving compatibility with the "legacy"
mdev_parent_ops callbacks without deprecating them, does it really make
sense to convert every one of the sample drivers to this new direct
registration?  Thanks,

Alex

Re: Allow mdev drivers to directly create the vfio_device (v3)

[Jason Gunthorpe]

On Tue, Jun 15, 2021 at 01:35:49PM -0600, Alex Williamson wrote:
> On Tue, 15 Jun 2021 15:35:09 +0200
> Christoph Hellwig <hch@lst.de> wrote:
> 
> > This is my alternative take on this series from Jason:
> > 
> > https://lore.kernel.org/dri-devel/87czsszi9i.fsf@redhat.com/T/
> > 
> > The mdev/vfio parts are exactly the same, but this solves the driver core
> > changes for the direct probing without the in/out flag that Greg hated,
> > which cause a little more work, but probably make the result better.
> > 
> > Original decription from Jason below:
> > 
> > The mdev bus's core part for managing the lifecycle of devices is mostly
> > as one would expect for a driver core bus subsystem.
> > 
> > However instead of having a normal 'struct device_driver' and binding the
> > actual mdev drivers through the standard driver core mechanisms it open
> > codes this with the struct mdev_parent_ops and provides a single driver
> > that shims between the VFIO core's struct vfio_device and the actual
> > device driver.
> > 
> > Instead, allow mdev drivers implement an actual struct mdev_driver and
> > directly call vfio_register_group_dev() in the probe() function for the
> > mdev. Arrange to bind the created mdev_device to the mdev_driver that is
> > provided by the end driver.
> > 
> > The actual execution flow doesn't change much, eg what was
> > parent_ops->create is now device_driver->probe and it is called at almost
> > the exact same time - except under the normal control of the driver core.
> > 
> > Ultimately converting all the drivers unlocks a fair number of additional
> > VFIO simplifications and cleanups.
> 
> Looks like we need an update to
> Documentation/driver-api/vfio-mediated-device.rst to go along with
> this.

I have those updates in the patch that removes the old way, do you
want to move them forward to here?

> Also, if we're preserving compatibility with the "legacy"
> mdev_parent_ops callbacks without deprecating them,

I view this as breaking up the work into manageable steps and patch
series. This is already at 10 patches just to provide the
infrastructure. The next steps will be to move the driver conversions
ahead.

> does it really make sense to convert every one of the sample drivers
> to this new direct registration?  

Yes, the rest of the drivers will get converted eventually too. There
is no reason to hold things back. Depending on timelines we might be
able to get AP into this cycle too...

Jason

Re: Allow mdev drivers to directly create the vfio_device (v3)

[Jason Gunthorpe]

On Wed, Jun 16, 2021 at 05:13:13AM +0200, Christoph Hellwig wrote:
> On Tue, Jun 15, 2021 at 05:35:15PM -0300, Jason Gunthorpe wrote:
> > Yes, the rest of the drivers will get converted eventually too. There
> > is no reason to hold things back. Depending on timelines we might be
> > able to get AP into this cycle too...
> 
> And I have a WIP tree to get rid of the weird indirections in i915/gvt.
> Once I find some cycles to test that I'll also test the vfio interface
> conversion.  This will probably be for next cycle, though.

Wow, that is a major project cool

I was going to ping the gvt guys once we get this landed

Jason

Re: [PATCH 02/10] driver core: Better distinguish probe errors in really_probe

[Cornelia Huck]

On Tue, Jun 15 2021, Christoph Hellwig <hch@lst.de> wrote:

> really_probe tries to special case errors from ->probe, but due to all
> other initialization added to the function over time now a lot of
> internal errors hit that code path as well.  Untangle that by adding
> a new probe_err local variable and apply the special casing only to
> that.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/base/dd.c | 72 +++++++++++++++++++++++++++--------------------
>  1 file changed, 42 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 7477d3322b3a..fd83817240e6 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -513,12 +513,44 @@ static ssize_t state_synced_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(state_synced);
>  
> +
> +static int call_driver_probe(struct device *dev, struct device_driver *drv)
> +{
> +	int ret = 0;
> +
> +	if (dev->bus->probe)
> +		ret = dev->bus->probe(dev);
> +	else if (drv->probe)
> +		ret = drv->probe(dev);
> +
> +	switch (ret) {
> +	case 0:
> +		break;
> +	case -EPROBE_DEFER:
> +		/* Driver requested deferred probing */
> +		dev_dbg(dev, "Driver %s requests probe deferral\n", drv->name);
> +		break;
> +	case -ENODEV:
> +	case -ENXIO:
> +		pr_debug("%s: probe of %s rejects match %d\n",
> +			 drv->name, dev_name(dev), ret);
> +		break;
> +	default:
> +		/* driver matched but the probe failed */
> +		pr_warn("%s: probe of %s failed with error %d\n",
> +			drv->name, dev_name(dev), ret);

Convert these two pr_* to dev_* when touching the code anyway?

> +		break;
> +	}
> +
> +	return ret;
> +}

(...)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH 02/10] driver core: Better distinguish probe errors in really_probe

[Greg Kroah-Hartman]

On Tue, Jun 15, 2021 at 03:53:46PM +0200, Cornelia Huck wrote:
> On Tue, Jun 15 2021, Christoph Hellwig <hch@lst.de> wrote:
> 
> > really_probe tries to special case errors from ->probe, but due to all
> > other initialization added to the function over time now a lot of
> > internal errors hit that code path as well.  Untangle that by adding
> > a new probe_err local variable and apply the special casing only to
> > that.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  drivers/base/dd.c | 72 +++++++++++++++++++++++++++--------------------
> >  1 file changed, 42 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > index 7477d3322b3a..fd83817240e6 100644
> > --- a/drivers/base/dd.c
> > +++ b/drivers/base/dd.c
> > @@ -513,12 +513,44 @@ static ssize_t state_synced_show(struct device *dev,
> >  }
> >  static DEVICE_ATTR_RO(state_synced);
> >  
> > +
> > +static int call_driver_probe(struct device *dev, struct device_driver *drv)
> > +{
> > +	int ret = 0;
> > +
> > +	if (dev->bus->probe)
> > +		ret = dev->bus->probe(dev);
> > +	else if (drv->probe)
> > +		ret = drv->probe(dev);
> > +
> > +	switch (ret) {
> > +	case 0:
> > +		break;
> > +	case -EPROBE_DEFER:
> > +		/* Driver requested deferred probing */
> > +		dev_dbg(dev, "Driver %s requests probe deferral\n", drv->name);
> > +		break;
> > +	case -ENODEV:
> > +	case -ENXIO:
> > +		pr_debug("%s: probe of %s rejects match %d\n",
> > +			 drv->name, dev_name(dev), ret);
> > +		break;
> > +	default:
> > +		/* driver matched but the probe failed */
> > +		pr_warn("%s: probe of %s failed with error %d\n",
> > +			drv->name, dev_name(dev), ret);
> 
> Convert these two pr_* to dev_* when touching the code anyway?

It can wait, it's just moving code around for now.

thanks,

greg k-h

Re: [PATCH 02/10] driver core: Better distinguish probe errors in really_probe

[Greg Kroah-Hartman]

On Tue, Jun 15, 2021 at 03:35:11PM +0200, Christoph Hellwig wrote:
> really_probe tries to special case errors from ->probe, but due to all
> other initialization added to the function over time now a lot of
> internal errors hit that code path as well.  Untangle that by adding
> a new probe_err local variable and apply the special casing only to
> that.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 02/10] driver core: Better distinguish probe errors in really_probe

[Kirti Wankhede]

On 6/15/2021 7:05 PM, Christoph Hellwig wrote:
> really_probe tries to special case errors from ->probe, but due to all
> other initialization added to the function over time now a lot of
> internal errors hit that code path as well.  Untangle that by adding
> a new probe_err local variable and apply the special casing only to
> that.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>


Reviewed-by: Kirti Wankhede <kwankhede@nvidia.com>

Re: [PATCH 06/10] vfio/mdev: Remove CONFIG_VFIO_MDEV_DEVICE

[Greg Kroah-Hartman]

On Tue, Jun 15, 2021 at 03:35:15PM +0200, Christoph Hellwig wrote:
> From: Jason Gunthorpe <jgg@nvidia.com>
> 
> For some reason the vfio_mdev shim mdev_driver has its own module and
> kconfig. As the next patch requires access to it from mdev.ko merge the
> two modules together and remove VFIO_MDEV_DEVICE.
> 
> A later patch deletes this driver entirely.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  Documentation/s390/vfio-ap.rst   |  1 -
>  arch/s390/Kconfig                |  2 +-
>  drivers/gpu/drm/i915/Kconfig     |  2 +-
>  drivers/vfio/mdev/Kconfig        |  7 -------
>  drivers/vfio/mdev/Makefile       |  3 +--
>  drivers/vfio/mdev/mdev_core.c    | 16 ++++++++++++++--
>  drivers/vfio/mdev/mdev_private.h |  2 ++
>  drivers/vfio/mdev/vfio_mdev.c    | 24 +-----------------------
>  samples/Kconfig                  |  6 +++---
>  9 files changed, 23 insertions(+), 40 deletions(-)
> 

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 06/10] vfio/mdev: Remove CONFIG_VFIO_MDEV_DEVICE

[Kirti Wankhede]

On 6/15/2021 7:05 PM, Christoph Hellwig wrote:
> From: Jason Gunthorpe <jgg@nvidia.com>
> 
> For some reason the vfio_mdev shim mdev_driver has its own module and
> kconfig. As the next patch requires access to it from mdev.ko merge the
> two modules together and remove VFIO_MDEV_DEVICE.
> 
> A later patch deletes this driver entirely.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Reviewed-by: Kirti Wankhede <kwankhede@nvidia.com>

Re: [PATCH 08/10] vfio/mtty: Convert to use vfio_register_group_dev()

[Greg Kroah-Hartman]

On Tue, Jun 15, 2021 at 03:35:17PM +0200, Christoph Hellwig wrote:
> From: Jason Gunthorpe <jgg@nvidia.com>
> 
> This is straightforward conversion, the mdev_state is actually serving as
> the vfio_device and we can replace all the mdev_get_drvdata()'s and the
> wonky dead code with a simple container_of()
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  samples/vfio-mdev/mtty.c | 185 ++++++++++++++++++---------------------
>  1 file changed, 83 insertions(+), 102 deletions(-)


Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 08/10] vfio/mtty: Convert to use vfio_register_group_dev()

[Kirti Wankhede]

> -static int mtty_reset(struct mdev_device *mdev)
> +static int mtty_reset(struct mdev_state *mdev_stte)

Nit pick:
s/mdev_stte/mdev_state


>   
> +static const struct vfio_device_ops mtty_dev_ops = {
> +	.name = "vfio-mdev",

I think name should be different that 'vfio-mdev', probably
'vfio-mdev-mtty' or 'vfio-mtty'

Rest looks fine to me.

Reviewed-by: Kirti Wankhede <kwankhede@nvidia.com>

Re: [PATCH 07/10] vfio/mdev: Allow the mdev_parent_ops to specify the device driver to bind

[Cornelia Huck]

On Mon, Jun 14 2021, Christoph Hellwig <hch@lst.de> wrote:

> From: Jason Gunthorpe <jgg@nvidia.com>
>
> This allows a mdev driver to opt out of using vfio_mdev.c, instead the
> driver will provide a 'struct mdev_driver' and register directly with the
> driver core.
>
> Much of mdev_parent_ops becomes unused in this mode:
> - create()/remove() are done via the mdev_driver probe()/remove()
> - mdev_attr_groups becomes mdev_driver driver.dev_groups
> - Wrapper function callbacks are replaced with the same ones from
>   struct vfio_device_ops
>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/vfio/mdev/mdev_core.c   | 30 ++++++++++++++++++++++--------
>  drivers/vfio/mdev/mdev_driver.c | 10 ++++++++++
>  include/linux/mdev.h            |  2 ++
>  3 files changed, 34 insertions(+), 8 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>