2021-07-26 14:40:03

by Christoph Hellwig

[permalink] [raw]
Subject: two small mdev fixups

Hi all,

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


2021-07-26 14:40:15

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 2/2] vfio/mdev: don't warn if ->request is not set

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 <[email protected]>
---
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

2021-07-26 17:13:06

by Cornelia Huck

[permalink] [raw]
Subject: Re: [PATCH 2/2] vfio/mdev: don't warn if ->request is not set

On Mon, Jul 26 2021, Christoph Hellwig <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]>

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

2021-07-26 23:08:50

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH 2/2] vfio/mdev: don't warn if ->request is not set

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 <[email protected]>
> ---
> drivers/vfio/mdev/mdev_core.c | 4 ----
> 1 file changed, 4 deletions(-)

Reviewed-by: Jason Gunthorpe <[email protected]>

2021-07-26 23:10:29

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH 2/2] vfio/mdev: don't warn if ->request is not set

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

2021-07-26 23:30:01

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH 2/2] vfio/mdev: don't warn if ->request is not set

On Mon, 26 Jul 2021 20:09:06 -0300
Jason Gunthorpe <[email protected]> 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

2021-07-27 06:05:30

by Cornelia Huck

[permalink] [raw]
Subject: Re: [PATCH 2/2] vfio/mdev: don't warn if ->request is not set

On Mon, Jul 26 2021, Alex Williamson <[email protected]> wrote:

> On Mon, 26 Jul 2021 20:09:06 -0300
> Jason Gunthorpe <[email protected]> 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.