Re: [PATCH v7 02/14] iommu/arm-smmu-v3: Start building a generic PASID layer

[Nicolin Chen <nicolinc@nvidia.com>]

On Wed, May 08, 2024 at 03:57:10PM -0300, Jason Gunthorpe wrote:
> @@ -611,10 +599,9 @@ void arm_smmu_sva_remove_dev_pasid(struct iommu_domain *domain,
>  	struct arm_smmu_bond *bond = NULL, *t;
>  	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
>  
> +	arm_smmu_remove_pasid(master, to_smmu_domain(domain), id);
> +
>  	mutex_lock(&sva_lock);
> -
> -	arm_smmu_clear_cd(master, id);
> -

Should the new arm_smmu_remove_pasid() be inside the sva_lock as
the arm_smmu_clear_cd() previously? This would also seem to match
with the arm_smmu_set_pasid() that's inside the sva_lock too.

With that, arm_smmu_remove_pasid() seems to be removed entirely
by a following change, though its function declare still exists
in arm-smmu-v3.h (I probably should find what following patch and
comment there..)

> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2412,6 +2412,10 @@ static void arm_smmu_install_ste_for_dev(struct arm_smmu_master *master,
>  	int i, j;
>  	struct arm_smmu_device *smmu = master->smmu;
>  
> +	master->cd_table.in_ste =
> +		FIELD_GET(STRTAB_STE_0_CFG, le64_to_cpu(target->data[0])) ==
> +		STRTAB_STE_0_CFG_S1_TRANS;
> +

> +int arm_smmu_set_pasid(struct arm_smmu_master *master,
> +		       struct arm_smmu_domain *smmu_domain, ioasid_t pasid,
> +		       const struct arm_smmu_cd *cd)
> +{
> +	struct arm_smmu_cd *cdptr;
> +
> +	/* The core code validates pasid */
> +
> +	if (!master->cd_table.in_ste)
> +		return -ENODEV;
> +
> +	cdptr = arm_smmu_alloc_cd_ptr(master, pasid);
> +	if (!cdptr)
> +		return -ENOMEM;

Though I might be missing some piece, the in_ste is set in the
arm_smmu_install_ste_for_dev() when STE.Config=S1_TRANS, in which
case cdptr is already allocated? If so, do we still need to call
arm_smmu_alloc_cd_ptr()? Or just arm_smmu_get_cd_ptr() instead?

Thanks
Nicolin