On 2/1/2021 7:34 AM, Cornelia Huck wrote:
> On Sat, 30 Jan 2021 11:51:41 -0500
> Steven Sistare <steven.sistare@xxxxxxxxxx> wrote:
>
>> On 1/29/2021 4:57 PM, Alex Williamson wrote:
>>> On Fri, 29 Jan 2021 08:54:10 -0800
>>> Steve Sistare <steven.sistare@xxxxxxxxxx> wrote:
>>>
>>>> Define a vfio_iommu_driver_ops notify callback, for sending events to
>>>> the driver. Drivers are not required to provide the callback, and
>>>> may ignore any events. The handling of events is driver specific.
>>>>
>>>> Define the CONTAINER_CLOSE event, called when the container's file
>>>> descriptor is closed. This event signifies that no further state changes
>>>> will occur via container ioctl's.
>>>>
>>>> Signed-off-by: Steve Sistare <steven.sistare@xxxxxxxxxx>
>>>> ---
>>>> drivers/vfio/vfio.c | 5 +++++
>>>> include/linux/vfio.h | 5 +++++
>>>> 2 files changed, 10 insertions(+)
>>>>
>>>> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
>>>> index 262ab0e..99458fc 100644
>>>> --- a/drivers/vfio/vfio.c
>>>> +++ b/drivers/vfio/vfio.c
>>>> @@ -1220,6 +1220,11 @@ static int vfio_fops_open(struct inode *inode, struct file *filep)
>>>> static int vfio_fops_release(struct inode *inode, struct file *filep)
>>>> {
>>>> struct vfio_container *container = filep->private_data;
>>>> + struct vfio_iommu_driver *driver = container->iommu_driver;
>>>> +
>>>> + if (driver && driver->ops->notify)
>>>> + driver->ops->notify(container->iommu_data,
>>>> + VFIO_DRIVER_NOTIFY_CONTAINER_CLOSE, NULL);
>>>>
>>>> filep->private_data = NULL;
>>>>
>>>> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
>>>> index 38d3c6a..9642579 100644
>>>> --- a/include/linux/vfio.h
>>>> +++ b/include/linux/vfio.h
>>>> @@ -57,6 +57,9 @@ extern int vfio_add_group_dev(struct device *dev,
>>>> extern void vfio_device_put(struct vfio_device *device);
>>>> extern void *vfio_device_data(struct vfio_device *device);
>>>>
>>>> +/* events for the backend driver notify callback */
>>>> +#define VFIO_DRIVER_NOTIFY_CONTAINER_CLOSE 1
>>>
>>> We should use an enum for type checking.
>>
>> Agreed.
>> I see you changed the value to 0. Do you want to reserve 0 for invalid-event?
>> (I know, this is internal and can be changed). Your call.
>
> I'm not sure what we would use an invalid-event event for... the type
> checking provided by the enum should be enough?
I should have described it as no-event or null-event. It can be useful when
initializing a struct member that stores an event, eg, last-event-received.
- Steve
>>>> +
>>>> /**
>>>> * struct vfio_iommu_driver_ops - VFIO IOMMU driver callbacks
>>>> */
>>>> @@ -90,6 +93,8 @@ struct vfio_iommu_driver_ops {
>>>> struct notifier_block *nb);
>>>> int (*dma_rw)(void *iommu_data, dma_addr_t user_iova,
>>>> void *data, size_t count, bool write);
>>>> + void (*notify)(void *iommu_data, unsigned int event,
>>>> + void *data);
>>>
>>> I'm not sure why we're pre-enabling this 3rd arg, do you have some
>>> short term usage in mind that we can't easily add on demand later?
>>> This is an internal API, not one we need to keep stable. Thanks,
>>
>> No short term need, just seems sensible for an event callback. I was mimic'ing the
>> signature of the callback for vfio_register_notifier. Your call.
>
> I'd drop *data for now, if we don't have a concrete use case.
>
>>
>> - Steve
>>
>>>
>>> Alex
>>>
>>>> };
>>>>
>>>> extern int vfio_register_iommu_driver(const struct vfio_iommu_driver_ops *ops);
>>>
>>
>
