Re: [PATCH v4 12/16] iommu/arm-smmu-v3: Add a global static IDENTITY domain

[Jason Gunthorpe <jgg@nvidia.com>]

On Mon, Jan 29, 2024 at 06:11:48PM +0000, Shameerali Kolothum Thodi wrote:

> > @@ -3056,6 +3089,7 @@ static void arm_smmu_remove_dev_pasid(struct
> > device *dev, ioasid_t pasid)
> >  }
> > 
> >  static struct iommu_ops arm_smmu_ops = {
> > +	.identity_domain	= &arm_smmu_identity_domain,
> 
> This seems to create a problem when we have set the identity domain and
> try to enable sva for the device. Since there is no smmu_domain for this case 
> and there is no specific domain type checking in iommu_sva_bind_device() path,
> it eventually crashes(hangs in my test) in,

Yeah, that is a longstanding issue in the SVA implementation, it only
works if the RID is set to a S1 paging domain.

I cleaned it up here so that the SVA series was cleaer:

https://lore.kernel.org/linux-iommu/1-v4-e7091cdd9e8d+43b1-smmuv3_newapi_p2_jgg@nvidia.com/

> iommu_sva_bind_device()
>    ...
>       arm_smmu_sva_set_dev_pasid()
>         __arm_smmu_sva_bind()
>            arm_smmu_mmu_notifier_get(smmu_domain, ..)  --> never exit the mmu notifier list loop.
> 
> I think we should check for the domain type in iommu_sva_bind_device() or later
> before trying to use smmu_domain.  At present(ie, without this series) it returns error
> while we are trying to write the CD. But that looks too late as well.

Oh wow, is that how it worked? OK, I figured it was just broken but if
there was some error code that happened indirectly then lets've move
the above patch ahead of this one.

Thanks,
Jason