Re: [PATCH v5 04/14] iommufd: Abstract an iommufd_eventq from iommufd_fault
From: Jason Gunthorpe <jgg@nvidia.com>
Date: 2025-01-10 17:26:40
Also in:
linux-doc, linux-iommu, linux-kselftest, linux-patches, lkml
Subsystem:
iommu subsystem, iommufd, the rest · Maintainers:
Joerg Roedel, Will Deacon, Jason Gunthorpe, Kevin Tian, Linus Torvalds
On Tue, Jan 07, 2025 at 09:10:07AM -0800, Nicolin Chen wrote:
quoted hunk ↗ jump to hunk
@@ -433,32 +434,35 @@ void iopt_remove_access(struct io_pagetable *iopt, u32 iopt_access_list_id); void iommufd_access_destroy_object(struct iommufd_object *obj); -/* - * An iommufd_fault object represents an interface to deliver I/O page faults - * to the user space. These objects are created/destroyed by the user space and - * associated with hardware page table objects during page-table allocation. - */ -struct iommufd_fault { +struct iommufd_eventq_ops { + ssize_t (*read)(struct iommufd_eventq *eventq, char __user *buf, + size_t count, loff_t *ppos); /* Mandatory op */ + ssize_t (*write)(struct iommufd_eventq *eventq, const char __user *buf, + size_t count, loff_t *ppos); /* Optional op */ +};
I think I recommend to avoid this, more like this:
diff --git a/drivers/iommu/iommufd/eventq.c b/drivers/iommu/iommufd/eventq.c
index b5be629f38eda7..73f5e8a6b17f54 100644
--- a/drivers/iommu/iommufd/eventq.c
+++ b/drivers/iommu/iommufd/eventq.c@@ -341,11 +341,6 @@ static ssize_t iommufd_fault_fops_write(struct iommufd_eventq *eventq, return done == 0 ? rc : done; } -static const struct iommufd_eventq_ops iommufd_fault_ops = { - .read = &iommufd_fault_fops_read, - .write = &iommufd_fault_fops_write, -}; - /* IOMMUFD_OBJ_VEVENTQ Functions */ void iommufd_veventq_abort(struct iommufd_object *obj)
@@ -409,31 +404,8 @@ static ssize_t iommufd_veventq_fops_read(struct iommufd_eventq *eventq, return done == 0 ? rc : done; } -static const struct iommufd_eventq_ops iommufd_veventq_ops = { - .read = &iommufd_veventq_fops_read, -}; - /* Common Event Queue Functions */ -static ssize_t iommufd_eventq_fops_read(struct file *filep, char __user *buf, - size_t count, loff_t *ppos) -{ - struct iommufd_eventq *eventq = filep->private_data; - - return eventq->ops->read(eventq, buf, count, ppos); -} - -static ssize_t iommufd_eventq_fops_write(struct file *filep, - const char __user *buf, size_t count, - loff_t *ppos) -{ - struct iommufd_eventq *eventq = filep->private_data; - - if (!eventq->ops->write) - return -EOPNOTSUPP; - return eventq->ops->write(eventq, buf, count, ppos); -} - static __poll_t iommufd_eventq_fops_poll(struct file *filep, struct poll_table_struct *wait) {
@@ -458,34 +430,31 @@ static int iommufd_eventq_fops_release(struct inode *inode, struct file *filep) return 0; } -static const struct file_operations iommufd_eventq_fops = { - .owner = THIS_MODULE, - .open = nonseekable_open, - .read = iommufd_eventq_fops_read, - .write = iommufd_eventq_fops_write, - .poll = iommufd_eventq_fops_poll, - .release = iommufd_eventq_fops_release, -}; +#define INIT_EVENTQ_FOPS(read_op, write_op) \ + (struct file_operations){ \ + .owner = THIS_MODULE, \ + .open = nonseekable_open, \ + .read = read_op, \ + .write = write_op, \ + .poll = iommufd_eventq_fops_poll, \ + .release = iommufd_eventq_fops_release, \ + } static int iommufd_eventq_init(struct iommufd_eventq *eventq, char *name, struct iommufd_ctx *ictx, - const struct iommufd_eventq_ops *ops) + const struct file_operations *fops) { struct file *filep; int fdno; - if (WARN_ON_ONCE(!ops || !ops->read)) - return -EINVAL; - mutex_init(&eventq->mutex); INIT_LIST_HEAD(&eventq->deliver); init_waitqueue_head(&eventq->wait_queue); - filep = anon_inode_getfile(name, &iommufd_eventq_fops, eventq, O_RDWR); + filep = anon_inode_getfile(name, fops, eventq, O_RDWR); if (IS_ERR(filep)) return PTR_ERR(filep); - eventq->ops = ops; eventq->ictx = ictx; iommufd_ctx_get(eventq->ictx); refcount_inc(&eventq->obj.users);
@@ -497,6 +466,9 @@ static int iommufd_eventq_init(struct iommufd_eventq *eventq, char *name, return fdno; } +static const struct file_operations iommufd_pgfault_fops = + INIT_EVENTQ_FOPS(iommufd_fault_fops_read, iommufd_fault_fops_write); + int iommufd_fault_alloc(struct iommufd_ucmd *ucmd) { struct iommu_fault_alloc *cmd = ucmd->cmd;
@@ -515,7 +487,7 @@ int iommufd_fault_alloc(struct iommufd_ucmd *ucmd) xa_init_flags(&fault->response, XA_FLAGS_ALLOC1); fdno = iommufd_eventq_init(&fault->common, "[iommufd-pgfault]", - ucmd->ictx, &iommufd_fault_ops); + ucmd->ictx, &iommufd_pgfault_fops); if (fdno < 0) { rc = fdno; goto out_abort;
@@ -541,6 +513,9 @@ int iommufd_fault_alloc(struct iommufd_ucmd *ucmd) return rc; } +static const struct file_operations iommufd_viommu_fops = + INIT_EVENTQ_FOPS(iommufd_veventq_fops_read, NULL); + int iommufd_veventq_alloc(struct iommufd_ucmd *ucmd) { struct iommu_veventq_alloc *cmd = ucmd->cmd;
@@ -580,7 +555,7 @@ int iommufd_veventq_alloc(struct iommufd_ucmd *ucmd) list_add_tail(&veventq->node, &viommu->veventqs); fdno = iommufd_eventq_init(&veventq->common, "[iommufd-viommu-event]", - ucmd->ictx, &iommufd_veventq_ops); + ucmd->ictx, &iommufd_viommu_fops); if (fdno < 0) { rc = fdno; goto out_abort;
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index 3c0374154a94d3..6c23d5b58901af 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h@@ -434,20 +434,11 @@ void iopt_remove_access(struct io_pagetable *iopt, u32 iopt_access_list_id); void iommufd_access_destroy_object(struct iommufd_object *obj); -struct iommufd_eventq_ops { - ssize_t (*read)(struct iommufd_eventq *eventq, char __user *buf, - size_t count, loff_t *ppos); /* Mandatory op */ - ssize_t (*write)(struct iommufd_eventq *eventq, const char __user *buf, - size_t count, loff_t *ppos); /* Optional op */ -}; - struct iommufd_eventq { struct iommufd_object obj; struct iommufd_ctx *ictx; struct file *filep; - const struct iommufd_eventq_ops *ops; - /* The lists of outstanding events protected by below mutex. */ struct mutex mutex; struct list_head deliver;