| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250110172637.GF396083@nvidia.com>
Date: Fri, 10 Jan 2025 13:26:37 -0400
From: Jason Gunthorpe <jgg@...dia.com>
To: Nicolin Chen <nicolinc@...dia.com>
Cc: kevin.tian@...el.com, corbet@....net, will@...nel.org, joro@...tes.org,
suravee.suthikulpanit@....com, robin.murphy@....com,
dwmw2@...radead.org, baolu.lu@...ux.intel.com, shuah@...nel.org,
linux-kernel@...r.kernel.org, iommu@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org,
linux-kselftest@...r.kernel.org, linux-doc@...r.kernel.org,
eric.auger@...hat.com, jean-philippe@...aro.org, mdf@...nel.org,
mshavit@...gle.com, shameerali.kolothum.thodi@...wei.com,
smostafa@...gle.com, ddutile@...hat.com, yi.l.liu@...el.com,
patches@...ts.linux.dev
Subject: Re: [PATCH v5 04/14] iommufd: Abstract an iommufd_eventq from
iommufd_fault
On Tue, Jan 07, 2025 at 09:10:07AM -0800, Nicolin Chen wrote:
> @@ -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;
Powered by blists - more mailing lists