Re: [PATCH v2 05/13] iommufd: Add IOMMUFD_OBJ_EVENTQ_VIRQ and IOMMUFD_CMD_VIRQ_ALLOC

On Wed, Dec 11, 2024 at 07:55:53AM +0000, Tian, Kevin wrote:

quoted

From: Nicolin Chen <redacted>
Sent: Wednesday, December 4, 2024 6:10 AM
+
+/* An iommufd_virq represents a vIOMMU interrupt in an eventq_virq
queue */
+struct iommufd_virq {
+	struct iommufd_eventq_virq *eventq_virq;
+	struct list_head node;
+	ssize_t irq_len;
+	void *irq_data;
+};

looks the only use of eventq_virq is in below:

quoted

+
+static inline int iommufd_eventq_virq_handler(struct iommufd_virq *virq)
+{
+	return iommufd_eventq_notify(&virq->eventq_virq->common,
&virq->node);
+}

If there is no other intended usages upon that field, it's simpler to
remove it and directly pass the pointer in when the handler is 
called. Anyway iommufd_viommu_report_irq() needs to find the
eventq first before calling it.

OK.

quoted

+/**
+ * struct iommu_virq_alloc - ioctl(IOMMU_VIRQ_ALLOC)
+ * @size: sizeof(struct iommu_virq_alloc)
+ * @flags: Must be 0
+ * @viommu: virtual IOMMU ID to associate the virtual IRQ with
+ * @type: Type of the virtual IRQ. Must be defined in enum
iommu_virq_type
+ * @out_virq_id: The ID of the new virtual IRQ
+ * @out_fault_fd: The fd of the new virtual IRQ. User space must close the
+ *                successfully returned fd after using it

s/out_fault_fd/out_virq_fd/

quoted

+ *
+ * Explicitly allocate a virtual IRQ handler for a vIOMMU. A vIOMMU can
have
+ * multiple FDs for different @type, but is confined to one FD per @type.
+ */

s/handler/interface/

quoted

+
+	eventq_virq->irq_wq = alloc_workqueue("viommu_irq/%d",
WQ_UNBOUND, 0,
+					      eventq_virq->common.obj.id);
+	if (!eventq_virq->irq_wq) {
+		rc = -ENOMEM;
+		goto out_put_fdno;
+	}

there is no use of this wq

Oops. Looks like I forgot to clean it up.

quoted

@@ -335,6 +335,8 @@ static const struct iommufd_ioctl_op

iommufd_ioctl_ops[] = {
 	IOCTL_OP(IOMMU_DESTROY, iommufd_destroy, struct
iommu_destroy, id),
 	IOCTL_OP(IOMMU_FAULT_QUEUE_ALLOC,
iommufd_eventq_iopf_alloc,
 		 struct iommu_fault_alloc, out_fault_fd),
+	IOCTL_OP(IOMMU_VIRQ_ALLOC, iommufd_eventq_virq_alloc,
+		 struct iommu_virq_alloc, out_virq_fd),

sort it in alphabetical order.

Ack.

Thanks
Nic