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?
>
> >> +
> >> /**
> >> * 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);
> >
>
