[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 3 Mar 2023 13:52:09 -0800
From: Jacob Pan <jacob.jun.pan@...ux.intel.com>
To: Baolu Lu <baolu.lu@...ux.intel.com>
Cc: LKML <linux-kernel@...r.kernel.org>, iommu@...ts.linux.dev,
Jason Gunthorpe <jgg@...dia.com>,
Joerg Roedel <joro@...tes.org>, dmaengine@...r.kernel.org,
vkoul@...nel.org, Robin Murphy <robin.murphy@....com>,
Will Deacon <will@...nel.org>,
David Woodhouse <dwmw2@...radead.org>,
Raj Ashok <ashok.raj@...el.com>,
"Tian, Kevin" <kevin.tian@...el.com>, Yi Liu <yi.l.liu@...el.com>,
"Yu, Fenghua" <fenghua.yu@...el.com>,
Dave Jiang <dave.jiang@...el.com>,
Tony Luck <tony.luck@...el.com>,
"Zanussi, Tom" <tom.zanussi@...el.com>,
jacob.jun.pan@...ux.intel.com
Subject: Re: [PATCH 4/4] dmaengine/idxd: Re-enable kernel workqueue under
DMA API
Hi Baolu,
On Fri, 3 Mar 2023 09:19:48 +0800, Baolu Lu <baolu.lu@...ux.intel.com>
wrote:
> On 3/2/23 8:59 AM, Jacob Pan wrote:
> > diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> > index f30eef701970..dadc908318aa 100644
> > --- a/drivers/dma/idxd/init.c
> > +++ b/drivers/dma/idxd/init.c
> > @@ -501,14 +501,52 @@ static struct idxd_device *idxd_alloc(struct
> > pci_dev *pdev, struct idxd_driver_d
> > 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;
>
> The idxd driver has claimed the DMA ownership of this device. Unless the
> idxd driver itself attached another domain, iommu_get_domain_for_dev()
> should never return a blocking domain.
>
> "domain == NULL" happens when CONFIG_IOMMU_API is not set.
>
> Furthermore, iommu_get_domain_for_dev() doesn't hold any refcount from
> the domain, so in theory it's not safe here because it possibly causes
> use-after-release case.
>
> I would say iommu_get_dma_domain() or something similar is more suitable
> for use here. It directly returns the device's default domain and the
> iommu core guarantees that default domain will always valid during the
> life cycle of any device driver.
>
will do, same as Jason's comments.
Thanks,
Jacob
Powered by blists - more mailing lists