On Fri, May 21, 2021 at 05:19:36PM -0700, Dave Jiang wrote:

Add common helper code to setup IMS once the MSI domain has been
setup by the device driver. The main helper function is
mdev_ims_set_msix_trigger() that is called by the VFIO ioctl
VFIO_DEVICE_SET_IRQS. The function deals with the setup and
teardown of emulated and IMS backed eventfd that gets exported
to the guest kernel via VFIO as MSIX vectors.

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/vfio/mdev/Kconfig     |   12 ++
 drivers/vfio/mdev/Makefile    |    3 
 drivers/vfio/mdev/mdev_irqs.c |  318 +++++++++++++++++++++++++++++++++++++++++
 include/linux/mdev.h          |   51 +++++++
 4 files changed, 384 insertions(+)
 create mode 100644 drivers/vfio/mdev/mdev_irqs.c

IMS is not mdev specific, do not entangle it with mdev code. This
should be generic VFIO stuff.

+static int mdev_msix_set_vector_signal(struct mdev_irq *mdev_irq, int vector, int fd)
+{
+	int rc, irq;
+	struct mdev_device *mdev = irq_to_mdev(mdev_irq);
+	struct mdev_irq_entry *entry;
+	struct device *dev = &mdev->dev;
+	struct eventfd_ctx *trigger;
+	char *name;
+	bool pasid_en;
+	u32 auxval;
+
+	if (vector < 0 || vector >= mdev_irq->num)
+		return -EINVAL;
+
+	entry = &mdev_irq->irq_entries[vector];
+
+	if (entry->ims)
+		irq = dev_msi_irq_vector(dev, entry->ims_id);
+	else
+		irq = 0;
+
+	pasid_en = mdev_irq->pasid != INVALID_IOASID ? true : false;
+
+	/* IMS and invalid pasid is not a valid configuration */
+	if (entry->ims && !pasid_en)
+		return -EINVAL;
+
+	if (entry->trigger) {
+		if (irq) {
+			irq_bypass_unregister_producer(&entry->producer);
+			free_irq(irq, entry->trigger);
+			if (pasid_en) {
+				auxval = ims_ctrl_pasid_aux(0, false);
+				irq_set_auxdata(irq, IMS_AUXDATA_CONTROL_WORD, auxval);
+			}
+		}
+		kfree(entry->name);
+		eventfd_ctx_put(entry->trigger);
+		entry->trigger = NULL;
+	}
+
+	if (fd < 0)
+		return 0;
+
+	name = kasprintf(GFP_KERNEL, "vfio-mdev-irq[%d](%s)", vector, dev_name(dev));
+	if (!name)
+		return -ENOMEM;
+
+	trigger = eventfd_ctx_fdget(fd);
+	if (IS_ERR(trigger)) {
+		kfree(name);
+		return PTR_ERR(trigger);
+	}
+
+	entry->name = name;
+	entry->trigger = trigger;
+
+	if (!irq)
+		return 0;
+
+	if (pasid_en) {
+		auxval = ims_ctrl_pasid_aux(mdev_irq->pasid, true);
+		rc = irq_set_auxdata(irq, IMS_AUXDATA_CONTROL_WORD, auxval);
+		if (rc < 0)
+			goto err;

Why is anything to do with PASID here? Something has gone wrong with
the layers I suspect..

Oh yes. drivers/irqchip/irq-ims-msi.c is dxd specific and shouldn't be
pretending to be common code.

The protocol to stuff the pasid and other stuff into the auxdata is
also compeltely idxd specific and is just a hacky way to communicate
from this code to the IDXD irq-chip.

So this doesn't belong here either. Pass in the auxdata from the idxd
code and I'd rename the irq-ims-msi to irq-ims-idxd

+static int mdev_msix_enable(struct mdev_irq *mdev_irq, int nvec)
+{
+	struct mdev_device *mdev = irq_to_mdev(mdev_irq);
+	struct device *dev;
+	int rc;
+
+	if (nvec != mdev_irq->num)
+		return -EINVAL;
+
+	if (mdev_irq->ims_num) {
+		dev = &mdev->dev;
+		rc = msi_domain_alloc_irqs(dev_get_msi_domain(dev), dev, mdev_irq->ims_num);

Huh? The PCI device should be the only device touching IRQ stuff. I'm
nervous to see you mix in the mdev struct device into this function.

Isn't the msi_domain just idxd->ims_domain?

Jason