linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* two small mdev fixups
@ 2021-07-26 14:35 Christoph Hellwig
  2021-07-26 14:35 ` [PATCH 1/2] vfio/mdev: turn mdev_init into a subsys_initcall Christoph Hellwig
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Christoph Hellwig @ 2021-07-26 14:35 UTC (permalink / raw)
  To: Kirti Wankhede, Alex Williamson, Jason Gunthorpe
  Cc: Cornelia Huck, kvm, linux-kernel

Hi all,

two small mdev fixes - one to fix mdev for built-in drivers, and the other
one to remove a pointless warning.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/2] vfio/mdev: turn mdev_init into a subsys_initcall
  2021-07-26 14:35 two small mdev fixups Christoph Hellwig
@ 2021-07-26 14:35 ` Christoph Hellwig
  2021-07-26 17:00   ` Cornelia Huck
  2021-07-26 23:06   ` Jason Gunthorpe
  2021-07-26 14:35 ` [PATCH 2/2] vfio/mdev: don't warn if ->request is not set Christoph Hellwig
  2021-08-03 18:45 ` two small mdev fixups Alex Williamson
  2 siblings, 2 replies; 15+ messages in thread
From: Christoph Hellwig @ 2021-07-26 14:35 UTC (permalink / raw)
  To: Kirti Wankhede, Alex Williamson, Jason Gunthorpe
  Cc: Cornelia Huck, kvm, linux-kernel

Without this setups with buіlt-in mdev and mdev-drivers fail to
register like this:

[1.903149] Driver 'intel_vgpu_mdev' was unable to register with bus_type 'mdev' because the bus was not initialized.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/vfio/mdev/mdev_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index e4581ec093a6..b16606ebafa1 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -398,7 +398,7 @@ static void __exit mdev_exit(void)
 	mdev_bus_unregister();
 }
 
-module_init(mdev_init)
+subsys_initcall(mdev_init)
 module_exit(mdev_exit)
 
 MODULE_VERSION(DRIVER_VERSION);
-- 
2.30.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/2] vfio/mdev: don't warn if ->request is not set
  2021-07-26 14:35 two small mdev fixups Christoph Hellwig
  2021-07-26 14:35 ` [PATCH 1/2] vfio/mdev: turn mdev_init into a subsys_initcall Christoph Hellwig
@ 2021-07-26 14:35 ` Christoph Hellwig
  2021-07-26 17:07   ` Cornelia Huck
  2021-07-26 23:07   ` Jason Gunthorpe
  2021-08-03 18:45 ` two small mdev fixups Alex Williamson
  2 siblings, 2 replies; 15+ messages in thread
From: Christoph Hellwig @ 2021-07-26 14:35 UTC (permalink / raw)
  To: Kirti Wankhede, Alex Williamson, Jason Gunthorpe
  Cc: Cornelia Huck, kvm, linux-kernel

Only a single driver actually sets the ->request method, so don't print
a scary warning if it isn't.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/vfio/mdev/mdev_core.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index b16606ebafa1..b314101237fe 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -138,10 +138,6 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
 	if (!dev)
 		return -EINVAL;
 
-	/* Not mandatory, but its absence could be a problem */
-	if (!ops->request)
-		dev_info(dev, "Driver cannot be asked to release device\n");
-
 	mutex_lock(&parent_list_lock);
 
 	/* Check for duplicate */
-- 
2.30.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] vfio/mdev: turn mdev_init into a subsys_initcall
  2021-07-26 14:35 ` [PATCH 1/2] vfio/mdev: turn mdev_init into a subsys_initcall Christoph Hellwig
@ 2021-07-26 17:00   ` Cornelia Huck
  2021-07-26 23:06   ` Jason Gunthorpe
  1 sibling, 0 replies; 15+ messages in thread
From: Cornelia Huck @ 2021-07-26 17:00 UTC (permalink / raw)
  To: Christoph Hellwig, Kirti Wankhede, Alex Williamson, Jason Gunthorpe
  Cc: kvm, linux-kernel

On Mon, Jul 26 2021, Christoph Hellwig <hch@lst.de> wrote:

> Without this setups with buіlt-in mdev and mdev-drivers fail to
> register like this:
>
> [1.903149] Driver 'intel_vgpu_mdev' was unable to register with bus_type 'mdev' because the bus was not initialized.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/vfio/mdev/mdev_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] vfio/mdev: don't warn if ->request is not set
  2021-07-26 14:35 ` [PATCH 2/2] vfio/mdev: don't warn if ->request is not set Christoph Hellwig
@ 2021-07-26 17:07   ` Cornelia Huck
  2021-07-26 23:09     ` Jason Gunthorpe
  2021-07-26 23:07   ` Jason Gunthorpe
  1 sibling, 1 reply; 15+ messages in thread
From: Cornelia Huck @ 2021-07-26 17:07 UTC (permalink / raw)
  To: Christoph Hellwig, Kirti Wankhede, Alex Williamson, Jason Gunthorpe
  Cc: kvm, linux-kernel

On Mon, Jul 26 2021, Christoph Hellwig <hch@lst.de> wrote:

> Only a single driver actually sets the ->request method, so don't print
> a scary warning if it isn't.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/vfio/mdev/mdev_core.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index b16606ebafa1..b314101237fe 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -138,10 +138,6 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
>  	if (!dev)
>  		return -EINVAL;
>  
> -	/* Not mandatory, but its absence could be a problem */
> -	if (!ops->request)
> -		dev_info(dev, "Driver cannot be asked to release device\n");
> -
>  	mutex_lock(&parent_list_lock);
>  
>  	/* Check for duplicate */

We also log a warning if we would like to call ->request() but none was
provided, so I think that's fine.

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

But I wonder why nobody else implements this? Lack of surprise removal?


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] vfio/mdev: turn mdev_init into a subsys_initcall
  2021-07-26 14:35 ` [PATCH 1/2] vfio/mdev: turn mdev_init into a subsys_initcall Christoph Hellwig
  2021-07-26 17:00   ` Cornelia Huck
@ 2021-07-26 23:06   ` Jason Gunthorpe
  1 sibling, 0 replies; 15+ messages in thread
From: Jason Gunthorpe @ 2021-07-26 23:06 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Kirti Wankhede, Alex Williamson, Cornelia Huck, kvm, linux-kernel

On Mon, Jul 26, 2021 at 04:35:23PM +0200, Christoph Hellwig wrote:
> Without this setups with buіlt-in mdev and mdev-drivers fail to
> register like this:
> 
> [1.903149] Driver 'intel_vgpu_mdev' was unable to register with bus_type 'mdev' because the bus was not initialized.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/vfio/mdev/mdev_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] vfio/mdev: don't warn if ->request is not set
  2021-07-26 14:35 ` [PATCH 2/2] vfio/mdev: don't warn if ->request is not set Christoph Hellwig
  2021-07-26 17:07   ` Cornelia Huck
@ 2021-07-26 23:07   ` Jason Gunthorpe
  1 sibling, 0 replies; 15+ messages in thread
From: Jason Gunthorpe @ 2021-07-26 23:07 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Kirti Wankhede, Alex Williamson, Cornelia Huck, kvm, linux-kernel

On Mon, Jul 26, 2021 at 04:35:24PM +0200, Christoph Hellwig wrote:
> Only a single driver actually sets the ->request method, so don't print
> a scary warning if it isn't.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/vfio/mdev/mdev_core.c | 4 ----
>  1 file changed, 4 deletions(-)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] vfio/mdev: don't warn if ->request is not set
  2021-07-26 17:07   ` Cornelia Huck
@ 2021-07-26 23:09     ` Jason Gunthorpe
  2021-07-26 23:28       ` Alex Williamson
  0 siblings, 1 reply; 15+ messages in thread
From: Jason Gunthorpe @ 2021-07-26 23:09 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: Christoph Hellwig, Kirti Wankhede, Alex Williamson, kvm, linux-kernel

On Mon, Jul 26, 2021 at 07:07:04PM +0200, Cornelia Huck wrote:

> But I wonder why nobody else implements this? Lack of surprise removal?

The only implementation triggers an eventfd that seems to be the same
eventfd as the interrupt..

Do you know how this works in userspace? I'm surprised that the
interrupt eventfd can trigger an observation that the kernel driver
wants to be unplugged?

Jason 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] vfio/mdev: don't warn if ->request is not set
  2021-07-26 23:09     ` Jason Gunthorpe
@ 2021-07-26 23:28       ` Alex Williamson
  2021-07-27  6:04         ` Cornelia Huck
  0 siblings, 1 reply; 15+ messages in thread
From: Alex Williamson @ 2021-07-26 23:28 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Cornelia Huck, Christoph Hellwig, Kirti Wankhede, kvm, linux-kernel

On Mon, 26 Jul 2021 20:09:06 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Mon, Jul 26, 2021 at 07:07:04PM +0200, Cornelia Huck wrote:
> 
> > But I wonder why nobody else implements this? Lack of surprise removal?  
> 
> The only implementation triggers an eventfd that seems to be the same
> eventfd as the interrupt..
> 
> Do you know how this works in userspace? I'm surprised that the
> interrupt eventfd can trigger an observation that the kernel driver
> wants to be unplugged?

I think we're talking about ccw, but I see QEMU registering separate
eventfds for each of the 3 IRQ indexes and the mdev driver specifically
triggering the req_trigger...?  Thanks,

Alex


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] vfio/mdev: don't warn if ->request is not set
  2021-07-26 23:28       ` Alex Williamson
@ 2021-07-27  6:04         ` Cornelia Huck
  2021-07-27 17:32           ` Jason Gunthorpe
  0 siblings, 1 reply; 15+ messages in thread
From: Cornelia Huck @ 2021-07-27  6:04 UTC (permalink / raw)
  To: Alex Williamson, Jason Gunthorpe
  Cc: Christoph Hellwig, Kirti Wankhede, kvm, linux-kernel

On Mon, Jul 26 2021, Alex Williamson <alex.williamson@redhat.com> wrote:

> On Mon, 26 Jul 2021 20:09:06 -0300
> Jason Gunthorpe <jgg@nvidia.com> wrote:
>
>> On Mon, Jul 26, 2021 at 07:07:04PM +0200, Cornelia Huck wrote:
>> 
>> > But I wonder why nobody else implements this? Lack of surprise removal?  
>> 
>> The only implementation triggers an eventfd that seems to be the same
>> eventfd as the interrupt..
>> 
>> Do you know how this works in userspace? I'm surprised that the
>> interrupt eventfd can trigger an observation that the kernel driver
>> wants to be unplugged?
>
> I think we're talking about ccw, but I see QEMU registering separate
> eventfds for each of the 3 IRQ indexes and the mdev driver specifically
> triggering the req_trigger...?  Thanks,
>
> Alex

Exactly, ccw has a trigger for normal I/O interrupts, CRW (machine
checks), and this one.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] vfio/mdev: don't warn if ->request is not set
  2021-07-27  6:04         ` Cornelia Huck
@ 2021-07-27 17:32           ` Jason Gunthorpe
  2021-07-27 18:53             ` Alex Williamson
  0 siblings, 1 reply; 15+ messages in thread
From: Jason Gunthorpe @ 2021-07-27 17:32 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: Alex Williamson, Christoph Hellwig, Kirti Wankhede, kvm, linux-kernel

On Tue, Jul 27, 2021 at 08:04:16AM +0200, Cornelia Huck wrote:
> On Mon, Jul 26 2021, Alex Williamson <alex.williamson@redhat.com> wrote:
> 
> > On Mon, 26 Jul 2021 20:09:06 -0300
> > Jason Gunthorpe <jgg@nvidia.com> wrote:
> >
> >> On Mon, Jul 26, 2021 at 07:07:04PM +0200, Cornelia Huck wrote:
> >> 
> >> > But I wonder why nobody else implements this? Lack of surprise removal?  
> >> 
> >> The only implementation triggers an eventfd that seems to be the same
> >> eventfd as the interrupt..
> >> 
> >> Do you know how this works in userspace? I'm surprised that the
> >> interrupt eventfd can trigger an observation that the kernel driver
> >> wants to be unplugged?
> >
> > I think we're talking about ccw, but I see QEMU registering separate
> > eventfds for each of the 3 IRQ indexes and the mdev driver specifically
> > triggering the req_trigger...?  Thanks,
> >
> > Alex
> 
> Exactly, ccw has a trigger for normal I/O interrupts, CRW (machine
> checks), and this one.

If it is a dedicated eventfd for 'device being removed' why is it in
the CCW implementation and not core code?

Is PCI doing the same?

Jason 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] vfio/mdev: don't warn if ->request is not set
  2021-07-27 17:32           ` Jason Gunthorpe
@ 2021-07-27 18:53             ` Alex Williamson
  2021-07-27 19:03               ` Jason Gunthorpe
  0 siblings, 1 reply; 15+ messages in thread
From: Alex Williamson @ 2021-07-27 18:53 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Cornelia Huck, Christoph Hellwig, Kirti Wankhede, kvm, linux-kernel

On Tue, 27 Jul 2021 14:32:09 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Tue, Jul 27, 2021 at 08:04:16AM +0200, Cornelia Huck wrote:
> > On Mon, Jul 26 2021, Alex Williamson <alex.williamson@redhat.com> wrote:
> >   
> > > On Mon, 26 Jul 2021 20:09:06 -0300
> > > Jason Gunthorpe <jgg@nvidia.com> wrote:
> > >  
> > >> On Mon, Jul 26, 2021 at 07:07:04PM +0200, Cornelia Huck wrote:
> > >>   
> > >> > But I wonder why nobody else implements this? Lack of surprise removal?    
> > >> 
> > >> The only implementation triggers an eventfd that seems to be the same
> > >> eventfd as the interrupt..
> > >> 
> > >> Do you know how this works in userspace? I'm surprised that the
> > >> interrupt eventfd can trigger an observation that the kernel driver
> > >> wants to be unplugged?  
> > >
> > > I think we're talking about ccw, but I see QEMU registering separate
> > > eventfds for each of the 3 IRQ indexes and the mdev driver specifically
> > > triggering the req_trigger...?  Thanks,
> > >
> > > Alex  
> > 
> > Exactly, ccw has a trigger for normal I/O interrupts, CRW (machine
> > checks), and this one.  
> 
> If it is a dedicated eventfd for 'device being removed' why is it in
> the CCW implementation and not core code?

The CCW implementation (likewise the vfio-pci implementation) owns the
IRQ index address space and the decision to make this a signal to
userspace rather than perhaps some handling a device might be able to
do internally.  For instance an alternate vfio-pci implementation might
zap all mmaps, block all r/w access, and turn this into a surprise
removal.  Another implementation might be more aggressive to sending
SIGKILL to the user process.  This was the thought behind why vfio-core
triggers the driver request callback with a counter, leaving the policy
to the driver.

> Is PCI doing the same?

Yes, that's where this handling originated.  Thanks,

Alex


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] vfio/mdev: don't warn if ->request is not set
  2021-07-27 18:53             ` Alex Williamson
@ 2021-07-27 19:03               ` Jason Gunthorpe
  2021-07-27 19:25                 ` Alex Williamson
  0 siblings, 1 reply; 15+ messages in thread
From: Jason Gunthorpe @ 2021-07-27 19:03 UTC (permalink / raw)
  To: Alex Williamson
  Cc: Cornelia Huck, Christoph Hellwig, Kirti Wankhede, kvm, linux-kernel

On Tue, Jul 27, 2021 at 12:53:09PM -0600, Alex Williamson wrote:
> On Tue, 27 Jul 2021 14:32:09 -0300
> Jason Gunthorpe <jgg@nvidia.com> wrote:
> 
> > On Tue, Jul 27, 2021 at 08:04:16AM +0200, Cornelia Huck wrote:
> > > On Mon, Jul 26 2021, Alex Williamson <alex.williamson@redhat.com> wrote:
> > >   
> > > > On Mon, 26 Jul 2021 20:09:06 -0300
> > > > Jason Gunthorpe <jgg@nvidia.com> wrote:
> > > >  
> > > >> On Mon, Jul 26, 2021 at 07:07:04PM +0200, Cornelia Huck wrote:
> > > >>   
> > > >> > But I wonder why nobody else implements this? Lack of surprise removal?    
> > > >> 
> > > >> The only implementation triggers an eventfd that seems to be the same
> > > >> eventfd as the interrupt..
> > > >> 
> > > >> Do you know how this works in userspace? I'm surprised that the
> > > >> interrupt eventfd can trigger an observation that the kernel driver
> > > >> wants to be unplugged?  
> > > >
> > > > I think we're talking about ccw, but I see QEMU registering separate
> > > > eventfds for each of the 3 IRQ indexes and the mdev driver specifically
> > > > triggering the req_trigger...?  Thanks,
> > > >
> > > > Alex  
> > > 
> > > Exactly, ccw has a trigger for normal I/O interrupts, CRW (machine
> > > checks), and this one.  
> > 
> > If it is a dedicated eventfd for 'device being removed' why is it in
> > the CCW implementation and not core code?
> 
> The CCW implementation (likewise the vfio-pci implementation) owns
> the IRQ index address space and the decision to make this a signal
> to userspace rather than perhaps some handling a device might be
> able to do internally. 

The core code holds the vfio_device_get() so long as the FD is
open. There is no way to pass the wait_for_completion without
userspace closing the FD, so there isn't really much choice for the
drivers to do beyond signal to userpace to close the FD??

> For instance an alternate vfio-pci implementation might zap all
> mmaps, block all r/w access, and turn this into a surprise removal.

This is nice, but wouldn't close the FD, so needs core changes
anyhow..

> Another implementation might be more aggressive to sending SIGKILL
> to the user process.

We don't try to revoke FDs from the kernel, it is racy, dangerous and
unreliable.

> This was the thought behind why vfio-core triggers the driver
> request callback with a counter, leaving the policy to the driver.

IMHO subsystem policy does not belong in drivers. Down that road lies
a mess for userspace.

Jason

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] vfio/mdev: don't warn if ->request is not set
  2021-07-27 19:03               ` Jason Gunthorpe
@ 2021-07-27 19:25                 ` Alex Williamson
  0 siblings, 0 replies; 15+ messages in thread
From: Alex Williamson @ 2021-07-27 19:25 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Cornelia Huck, Christoph Hellwig, Kirti Wankhede, kvm, linux-kernel

On Tue, 27 Jul 2021 16:03:17 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Tue, Jul 27, 2021 at 12:53:09PM -0600, Alex Williamson wrote:
> > On Tue, 27 Jul 2021 14:32:09 -0300
> > Jason Gunthorpe <jgg@nvidia.com> wrote:
> >   
> > > On Tue, Jul 27, 2021 at 08:04:16AM +0200, Cornelia Huck wrote:  
> > > > On Mon, Jul 26 2021, Alex Williamson <alex.williamson@redhat.com> wrote:
> > > >     
> > > > > On Mon, 26 Jul 2021 20:09:06 -0300
> > > > > Jason Gunthorpe <jgg@nvidia.com> wrote:
> > > > >    
> > > > >> On Mon, Jul 26, 2021 at 07:07:04PM +0200, Cornelia Huck wrote:
> > > > >>     
> > > > >> > But I wonder why nobody else implements this? Lack of surprise removal?      
> > > > >> 
> > > > >> The only implementation triggers an eventfd that seems to be the same
> > > > >> eventfd as the interrupt..
> > > > >> 
> > > > >> Do you know how this works in userspace? I'm surprised that the
> > > > >> interrupt eventfd can trigger an observation that the kernel driver
> > > > >> wants to be unplugged?    
> > > > >
> > > > > I think we're talking about ccw, but I see QEMU registering separate
> > > > > eventfds for each of the 3 IRQ indexes and the mdev driver specifically
> > > > > triggering the req_trigger...?  Thanks,
> > > > >
> > > > > Alex    
> > > > 
> > > > Exactly, ccw has a trigger for normal I/O interrupts, CRW (machine
> > > > checks), and this one.    
> > > 
> > > If it is a dedicated eventfd for 'device being removed' why is it in
> > > the CCW implementation and not core code?  
> > 
> > The CCW implementation (likewise the vfio-pci implementation) owns
> > the IRQ index address space and the decision to make this a signal
> > to userspace rather than perhaps some handling a device might be
> > able to do internally.   
> 
> The core code holds the vfio_device_get() so long as the FD is
> open. There is no way to pass the wait_for_completion without
> userspace closing the FD, so there isn't really much choice for the
> drivers to do beyond signal to userpace to close the FD??
> 
> > For instance an alternate vfio-pci implementation might zap all
> > mmaps, block all r/w access, and turn this into a surprise removal.  
> 
> This is nice, but wouldn't close the FD, so needs core changes
> anyhow..

Right, the core would need to be able to handle an FD disconnected from
the device, obviously some core changes would be required.

> > Another implementation might be more aggressive to sending SIGKILL
> > to the user process.  
> 
> We don't try to revoke FDs from the kernel, it is racy, dangerous and
> unreliable.

I'm not sure how trying to kill the process using an open file becomes
a revoke...  In fact, the surprise hotplug might just be able to zap
mmaps and wait for userspace to generate a SIGBUS.

> > This was the thought behind why vfio-core triggers the driver
> > request callback with a counter, leaving the policy to the driver.  
> 
> IMHO subsystem policy does not belong in drivers. Down that road lies
> a mess for userspace.

I think my argument was that to this point it's been driver policy, not
subsystem policy.  The subsystem policy is to block until the device is
released, it's the driver policy whether it has a means to implement
something to expedite that.  Thanks,

Alex


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: two small mdev fixups
  2021-07-26 14:35 two small mdev fixups Christoph Hellwig
  2021-07-26 14:35 ` [PATCH 1/2] vfio/mdev: turn mdev_init into a subsys_initcall Christoph Hellwig
  2021-07-26 14:35 ` [PATCH 2/2] vfio/mdev: don't warn if ->request is not set Christoph Hellwig
@ 2021-08-03 18:45 ` Alex Williamson
  2 siblings, 0 replies; 15+ messages in thread
From: Alex Williamson @ 2021-08-03 18:45 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Kirti Wankhede, Jason Gunthorpe, Cornelia Huck, kvm, linux-kernel

On Mon, 26 Jul 2021 16:35:22 +0200
Christoph Hellwig <hch@lst.de> wrote:

> Hi all,
> 
> two small mdev fixes - one to fix mdev for built-in drivers, and the other
> one to remove a pointless warning.

Applied to vfio next branch for v5.15 with Connie and Jason's R-b.
Thanks,

Alex


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-08-03 18:45 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-26 14:35 two small mdev fixups Christoph Hellwig
2021-07-26 14:35 ` [PATCH 1/2] vfio/mdev: turn mdev_init into a subsys_initcall Christoph Hellwig
2021-07-26 17:00   ` Cornelia Huck
2021-07-26 23:06   ` Jason Gunthorpe
2021-07-26 14:35 ` [PATCH 2/2] vfio/mdev: don't warn if ->request is not set Christoph Hellwig
2021-07-26 17:07   ` Cornelia Huck
2021-07-26 23:09     ` Jason Gunthorpe
2021-07-26 23:28       ` Alex Williamson
2021-07-27  6:04         ` Cornelia Huck
2021-07-27 17:32           ` Jason Gunthorpe
2021-07-27 18:53             ` Alex Williamson
2021-07-27 19:03               ` Jason Gunthorpe
2021-07-27 19:25                 ` Alex Williamson
2021-07-26 23:07   ` Jason Gunthorpe
2021-08-03 18:45 ` two small mdev fixups Alex Williamson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).