Re: [PATCH 0/6] Make set_dev_pasid op supportting domain replacement

[Yi Liu <yi.l.liu@xxxxxxxxx>]

On 2024/7/12 02:37, Jason Gunthorpe wrote:
> On Fri, Jun 28, 2024 at 01:55:32AM -0700, Yi Liu wrote:
> > This splits the preparation works of the iommu and the Intel iommu driver
> > out from the iommufd pasid attach/replace series. [1]
> >
> > To support domain replacement, the definition of the set_dev_pasid op
> > needs to be enhanced. Meanwhile, the existing set_dev_pasid callbacks
> > should be extended as well to suit the new definition.
> >
> > pasid attach/replace is mandatory on Intel VT-d given the PASID table
> > locates in the physical address space hence must be managed by the kernel,
> > both for supporting vSVA and coming SIOV. But it's optional on ARM/AMD
> > which allow configuring the PASID/CD table either in host physical address
> > space or nested on top of an GPA address space. This series only extends
> > the Intel iommu driver as the minimal requirement.
>
> Sicne this will be pushed to the next cyle that will have my ARM code
> the smmuv3 will need to be updated too. It is already prepped to
> support replace, just add this please:

thanks. So your related series has made the internal helpers to support
domain replacement in set_dev_pasid path. The below diff just passes the
old_domain across the helpers. Is it? Just to double confirm with you. :)

> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> index ead83d67421f10..44434978a218ae 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> @@ -350,7 +351,8 @@ void arm_smmu_sva_notifier_synchronize(void)
>   }
>   
>   static int arm_smmu_sva_set_dev_pasid(struct iommu_domain *domain,
> -				      struct device *dev, ioasid_t id)
> +				      struct device *dev, ioasid_t id,
> +				      struct iommu_domain *old_domain)
>   {
>   	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>   	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> @@ -367,7 +369,7 @@ static int arm_smmu_sva_set_dev_pasid(struct iommu_domain *domain,
>   	 */
>   	arm_smmu_make_sva_cd(&target, master, domain->mm, smmu_domain->asid,
>   			     smmu_domain->btm_invalidation);
> -	ret = arm_smmu_set_pasid(master, smmu_domain, id, &target);
> +	ret = arm_smmu_set_pasid(master, smmu_domain, id, &target, old_domain);
>   
>   	mmput(domain->mm);
>   	return ret;
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 238968b1709936..140aac5cd4ef57 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2943,7 +2943,8 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
>   }
>   
>   static int arm_smmu_s1_set_dev_pasid(struct iommu_domain *domain,
> -				      struct device *dev, ioasid_t id)
> +				      struct device *dev, ioasid_t id,
> +				      struct iommu_domain *old_domain)
>   {
>   	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>   	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> @@ -2969,7 +2970,7 @@ static int arm_smmu_s1_set_dev_pasid(struct iommu_domain *domain,
>   	 */
>   	arm_smmu_make_s1_cd(&target_cd, master, smmu_domain);
>   	return arm_smmu_set_pasid(master, to_smmu_domain(domain), id,
> -				  &target_cd);
> +				  &target_cd, old_domain);
>   }
>   
>   static void arm_smmu_update_ste(struct arm_smmu_master *master,
> @@ -2999,7 +3000,7 @@ static void arm_smmu_update_ste(struct arm_smmu_master *master,
>   
>   int arm_smmu_set_pasid(struct arm_smmu_master *master,
>   		       struct arm_smmu_domain *smmu_domain, ioasid_t pasid,
> -		       struct arm_smmu_cd *cd)
> +		       struct arm_smmu_cd *cd, struct iommu_domain *old_domain)
>   {
>   	struct iommu_domain *sid_domain = iommu_get_domain_for_dev(master->dev);
>   	struct arm_smmu_attach_state state = {
> @@ -3009,6 +3010,7 @@ int arm_smmu_set_pasid(struct arm_smmu_master *master,
>   		 * already attached, no need to set old_domain.
>   		 */
>   		.ssid = pasid,
> +		.old_domain = old_domain,
>   	};
>   	struct arm_smmu_cd *cdptr;
>   	int ret;
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> index bcf9ea9d929f5f..447a3cdf1c4e1c 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -828,7 +828,7 @@ void arm_smmu_write_cd_entry(struct arm_smmu_master *master, int ssid,
>   
>   int arm_smmu_set_pasid(struct arm_smmu_master *master,
>   		       struct arm_smmu_domain *smmu_domain, ioasid_t pasid,
> -		       struct arm_smmu_cd *cd);
> +		       struct arm_smmu_cd *cd, struct iommu_domain *old_domain);
>   
>   int arm_smmu_domain_alloc_id(struct arm_smmu_device *smmu,
>   			     struct arm_smmu_domain *smmu_domain);

--
Regards,
Yi Liu