Re: [PATCH 4/4] dmaengine/idxd: Re-enable kernel workqueue under DMA API

[Baolu Lu <baolu.lu@linux.intel.com>]

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.

Best regards,
baolu