All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@nvidia.com>
To: Matthew Rosato <mjrosato@linux.ibm.com>
Cc: jjherne@linux.ibm.com, akrowiak@linux.ibm.com,
	kvm@vger.kernel.org, hch@infradead.org,
	linux-s390@vger.kernel.org, intel-gfx@lists.freedesktop.org,
	cohuck@redhat.com, linux-kernel@vger.kernel.org,
	pasic@linux.ibm.com, borntraeger@linux.ibm.com,
	intel-gvt-dev@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/1] vfio: remove VFIO_GROUP_NOTIFY_SET_KVM
Date: Wed, 18 May 2022 12:12:47 -0300	[thread overview]
Message-ID: <20220518151247.GI1343366@nvidia.com> (raw)
In-Reply-To: <2e51b388-48d0-4689-07f4-65f607dbce59@linux.ibm.com>

On Wed, May 18, 2022 at 10:37:48AM -0400, Matthew Rosato wrote:
> On 5/17/22 2:08 PM, Matthew Rosato wrote:
> > Rather than relying on a notifier for associating the KVM with
> > the group, let's assume that the association has already been
> > made prior to device_open.  The first time a device is opened
> > associate the group KVM with the device.
> > 
> > Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> 
> ...
> 
> > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> > index cfcff7764403..c5d421eda275 100644
> > +++ b/drivers/vfio/vfio.c
> > @@ -10,6 +10,7 @@
> >    * Author: Tom Lyon, pugs@cisco.com
> >    */
> > +#include "linux/kvm_host.h"
> >   #include <linux/cdev.h>
> >   #include <linux/compat.h>
> >   #include <linux/device.h>
> > @@ -1083,6 +1084,13 @@ static struct file *vfio_device_open(struct vfio_device *device)
> >   	mutex_lock(&device->dev_set->lock);
> >   	device->open_count++;
> > +	down_write(&device->group->group_rwsem);
> > +	if (device->open_count == 1 && device->group->kvm) {
> > +		device->kvm = device->group->kvm;
> > +		kvm_get_kvm(device->kvm);
> 
> Did some more compile testing, since vfio has no hard kvm dependency,
> kvm_get_kvm and kvm_put_kvm are an issue if KVM is a module while vfio is
> built-in...

Ugh, my other plan was to have the driver itself capture the kvm, ie
we lock the group_rwsem to keep the group->kvm valid and then pass the
kvm to open_device in some way, then the driver can kvm_get_kvm() it

Alternatively, I don't know why kvm_get_kvm() is an exported symbol
when it is just calling refcount_inc() - inlining it would be an
improvement I think.

Jason

WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe <jgg@nvidia.com>
To: Matthew Rosato <mjrosato@linux.ibm.com>
Cc: alex.williamson@redhat.com, cohuck@redhat.com,
	borntraeger@linux.ibm.com, jjherne@linux.ibm.com,
	akrowiak@linux.ibm.com, pasic@linux.ibm.com,
	zhenyuw@linux.intel.com, zhi.a.wang@intel.com, hch@infradead.org,
	intel-gfx@lists.freedesktop.org,
	intel-gvt-dev@lists.freedesktop.org, linux-s390@vger.kernel.org,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] vfio: remove VFIO_GROUP_NOTIFY_SET_KVM
Date: Wed, 18 May 2022 12:12:47 -0300	[thread overview]
Message-ID: <20220518151247.GI1343366@nvidia.com> (raw)
In-Reply-To: <2e51b388-48d0-4689-07f4-65f607dbce59@linux.ibm.com>

On Wed, May 18, 2022 at 10:37:48AM -0400, Matthew Rosato wrote:
> On 5/17/22 2:08 PM, Matthew Rosato wrote:
> > Rather than relying on a notifier for associating the KVM with
> > the group, let's assume that the association has already been
> > made prior to device_open.  The first time a device is opened
> > associate the group KVM with the device.
> > 
> > Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> 
> ...
> 
> > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> > index cfcff7764403..c5d421eda275 100644
> > +++ b/drivers/vfio/vfio.c
> > @@ -10,6 +10,7 @@
> >    * Author: Tom Lyon, pugs@cisco.com
> >    */
> > +#include "linux/kvm_host.h"
> >   #include <linux/cdev.h>
> >   #include <linux/compat.h>
> >   #include <linux/device.h>
> > @@ -1083,6 +1084,13 @@ static struct file *vfio_device_open(struct vfio_device *device)
> >   	mutex_lock(&device->dev_set->lock);
> >   	device->open_count++;
> > +	down_write(&device->group->group_rwsem);
> > +	if (device->open_count == 1 && device->group->kvm) {
> > +		device->kvm = device->group->kvm;
> > +		kvm_get_kvm(device->kvm);
> 
> Did some more compile testing, since vfio has no hard kvm dependency,
> kvm_get_kvm and kvm_put_kvm are an issue if KVM is a module while vfio is
> built-in...

Ugh, my other plan was to have the driver itself capture the kvm, ie
we lock the group_rwsem to keep the group->kvm valid and then pass the
kvm to open_device in some way, then the driver can kvm_get_kvm() it

Alternatively, I don't know why kvm_get_kvm() is an exported symbol
when it is just calling refcount_inc() - inlining it would be an
improvement I think.

Jason

  reply	other threads:[~2022-05-18 15:12 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-17 18:08 [PATCH 0/1] vfio: remove VFIO_GROUP_NOTIFY_SET_KVM Matthew Rosato
2022-05-17 18:08 ` [PATCH 1/1] " Matthew Rosato
2022-05-17 18:56   ` [Intel-gfx] " Jason Gunthorpe
2022-05-17 18:56     ` Jason Gunthorpe
2022-05-18  7:39     ` [Intel-gfx] " Christoph Hellwig
2022-05-18  7:39       ` Christoph Hellwig
2022-05-18 11:58       ` [Intel-gfx] " Jason Gunthorpe
2022-05-18 11:58         ` Jason Gunthorpe
2022-05-18  7:47   ` [Intel-gfx] " Christoph Hellwig
2022-05-18  7:47     ` Christoph Hellwig
2022-05-18 14:37   ` Matthew Rosato
2022-05-18 15:12     ` Jason Gunthorpe [this message]
2022-05-18 15:12       ` Jason Gunthorpe
2022-05-18 15:33       ` Matthew Rosato
2022-05-18 15:38         ` [Intel-gfx] " Jason Gunthorpe
2022-05-18 15:38           ` Jason Gunthorpe
2022-05-17 21:07 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with [1/1] " Patchwork

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20220518151247.GI1343366@nvidia.com \
    --to=jgg@nvidia.com \
    --cc=akrowiak@linux.ibm.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=cohuck@redhat.com \
    --cc=hch@infradead.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-gvt-dev@lists.freedesktop.org \
    --cc=jjherne@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=mjrosato@linux.ibm.com \
    --cc=pasic@linux.ibm.com \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.