On Fri, May 28, 2021 at 09:37:56AM -0700, Dave Jiang wrote: > > On 5/28/2021 5:21 AM, Jason Gunthorpe wrote: > > On Thu, May 27, 2021 at 06:49:59PM -0700, Dave Jiang wrote: > > > > > +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_ Honestly, I don't know. I would prefer Thomas confirm what is the correct way to use the msi_domain as IDXD is going to be the reference everyone copies. Jason