Re: [PATCH v5 12/14] iommu/amd: Initial SVA support for AMD IOMMU

[Jason Gunthorpe <jgg@ziepe.ca>]

On Thu, Feb 08, 2024 at 11:53:58PM +0530, Vasant Hegde wrote:
> >> We are doing 1 SVA domain : 1 PASID : N devices model. That means we need to
> >> check all these things while attaching *first* PASID to device. (That's what we
> >> had discussed sometime back when I had all these things in feature_enable(SVA)
> >> path).
> > 
> > I thought I said you 1 PASID is not technically correct, but you could
> > get away it it for a short term. You should be planning to support N
> > PASIDs because that is what the API defines.
> 
> I am describing API in SVA context only. In that flow once we allocate a SVA
> domain for a PASID, then all devices for that PASID is attached to
> same domain.

It is OK to take a shortcut here if it helps, but that is not the API
model. Domains, SVA or not, do not have PASIDs. The only place a PASID
comes is via the set_dev_pasid() argument and any domain should be
attachable to any combination of PASIDs.

So the only place a PASID should be stored is in the elements of the
invalidation tracking list of the domain. When you get that all
completed it should all be uniform and a limitation like this should
go away. But this is fine to hold over for a followup that completes
the PASID support.

> > You just trivially check these conditions at the top of set_dev_pasid, look at
> > what I did for ARM:
> > 
> > 	if (smmu_domain->smmu != master->smmu || pasid == IOMMU_NO_PASID)
> > 		return -EINVAL;
> > 
> > 	if (!master->cd_table.in_ste &&
> > 	    sid_domain->type != IOMMU_DOMAIN_IDENTITY &&
> > 	    sid_domain->type != IOMMU_DOMAIN_BLOCKED)
> > 		return -EINVAL;
> > 
> > For AMD "cd_table in_ste" means that the DTE points to a GCR3 table
> > already. This is trivially tracked in the DTE programming in set_dte
> > because you know if the dte is being configured with a GCR3 or not.
> > 
> > The other cases represent the situations where we know how to upgrade a DTE
> > for those domains to one with a GCR3. Everything else is blocked.
> 
> I got something similar. I have reworked set/remove pasid path completely.
> I will try to post it soon.

Nice!

Jason