RE: [PATCH v3 1/7] iommu: Introduce a replace API for device pasid

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> Sent: Friday, June 28, 2024 5:06 PM
> 
> @@ -3289,7 +3290,20 @@ static int __iommu_set_group_pasid(struct
> iommu_domain *domain,
> 
>  		if (device == last_gdev)
>  			break;
> -		ops->remove_dev_pasid(device->dev, pasid, domain);
> +		/* If no old domain, undo the succeeded devices/pasid */
> +		if (!old) {
> +			ops->remove_dev_pasid(device->dev, pasid, domain);
> +			continue;
> +		}
> +
> +		/*
> +		 * Rollback the succeeded devices/pasid to the old domain.
> +		 * And it is a driver bug to fail attaching with a previously
> +		 * good domain.
> +		 */
> +		if (WARN_ON(old->ops->set_dev_pasid(old, device->dev,
> +						    pasid, domain)))
> +			ops->remove_dev_pasid(device->dev, pasid, domain);

I wonder whether @remove_dev_pasid() can be replaced by having
blocking_domain support @set_dev_pasid?

> +int iommu_replace_device_pasid(struct iommu_domain *domain,
> +			       struct device *dev, ioasid_t pasid)
> +{
> +	/* Caller must be a probed driver on dev */
> +	struct iommu_group *group = dev->iommu_group;
> +	void *curr;
> +	int ret;
> +
> +	if (!domain->ops->set_dev_pasid)
> +		return -EOPNOTSUPP;
> +
> +	if (!group)
> +		return -ENODEV;
> +
> +	if (!dev_has_iommu(dev) || dev_iommu_ops(dev) != domain-
> >owner ||
> +	    pasid == IOMMU_NO_PASID)
> +		return -EINVAL;
> +
> +	mutex_lock(&group->mutex);
> +	/*
> +	 * The recorded domain is inconsistent with the domain pasid is
> +	 * actually attached until pasid is attached to the new domain.
> +	 * This has race condition with the paths that do not hold
> +	 * group->mutex. E.g. the Page Request forwarding.
> +	 */

so?

> +	curr = xa_store(&group->pasid_array, pasid, domain, GFP_KERNEL);
> +	if (!curr) {
> +		xa_erase(&group->pasid_array, pasid);
> +		ret = -EINVAL;
> +		goto out_unlock;
> +	}
> +
> +	ret = xa_err(curr);
> +	if (ret)
> +		goto out_unlock;
> +
> +	if (curr == domain)
> +		goto out_unlock;
> +
> +	ret = __iommu_set_group_pasid(domain, group, pasid, curr);
> +	if (ret)
> +		WARN_ON(domain != xa_store(&group->pasid_array, pasid,
> +					   curr, GFP_KERNEL));

above can follow Jason's suggestion to iommu_group_replace_domain ()
in Baolu's series, i.e. doing a xa_reserve() first.





[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux