On Fri, May 28 2021 at 13:39, Jason Gunthorpe wrote: > 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. The general expectation is that the MSI irqdomain is retrievable from struct device for any device which supports MSI. Thanks, tglx