On 5/28/2021 5:21 AM, Jason Gunthorpe wrote:

On Thu, May 27, 2021 at 06:49:59PM -0700, Dave Jiang wrote:

quoted

quoted

quoted

+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.

As we talked about in the other thread. We have a single IMS domain per
device. The domain is set to the mdev 'struct device' and we allocate the
vectors to each mdev 'struct device' so we can manage those IMS vectors
specifically for that mdev.

That is not the point, I'm asking if you should be calling
dev_set_msi_domain(mdev) at all

I'm not familiar with the standard way of doing this. Should I not set 
the domain to the mdev 'struct device' because I can have multiple mdev 
using the same domain? With the domain set, I am able to retrieve it and 
call the msi_domain_alloc_irqs() in common code. Alternatively we can 
pass in the domain during init and not rely on dev->msi_domain.

Jason