Hi Kevin, On Thu, 2 Mar 2023 09:47:00 +0000, "Tian, Kevin" <kevin.tian@xxxxxxxxx> wrote: > > From: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx> > > Sent: Thursday, March 2, 2023 9:00 AM > > > > static int idxd_enable_system_pasid(struct idxd_device *idxd) > > { > > - return -EOPNOTSUPP; > > + struct pci_dev *pdev = idxd->pdev; > > + struct device *dev = &pdev->dev; > > + struct iommu_domain *domain; > > + union gencfg_reg gencfg; > > + ioasid_t pasid; > > + int ret; > > + > > + domain = iommu_get_domain_for_dev(dev); > > + if (!domain || domain->type == IOMMU_DOMAIN_BLOCKED) > > + return -EPERM; > > what about UNMANAGED? will fix this by getting the dma domain. > > > + > > + pasid = iommu_sva_reserve_pasid(1, dev->iommu->max_pasids); > > + if (pasid == IOMMU_PASID_INVALID) > > + return -ENOSPC; > > as commented in last patch we can just pass a device pointer to a > general allocation interface. will do > > > + > > + ret = iommu_attach_device_pasid(domain, dev, pasid); > > + if (ret) { > > + dev_err(dev, "failed to attach device pasid %d, domain > > type %d", > > + pasid, domain->type); > > + iommu_sva_unreserve_pasid(pasid); > > + return ret; > > + } > > + > > + /* Since we set user privilege for kernel DMA, enable > > completion IRQ */ > > + gencfg.bits = ioread32(idxd->reg_base + IDXD_GENCFG_OFFSET); > > + gencfg.user_int_en = 1; > > + iowrite32(gencfg.bits, idxd->reg_base + IDXD_GENCFG_OFFSET); > > + idxd->pasid = pasid; > > Why does user privilege requires a completion interrupt? > > Or instead it's more due to doing kernel DMA itself then we certainly > don't want to poll in the kernel? yes, kernel wq does not support polling, therefore it needs interrupts. Without user_int_en bit set, there would be no interrupts if we use user privilege for kernel wq. Thanks, Jacob