Re: [PATCH RESEND 03/10] iommu/amd: Initial SVA support for AMD IOMMU

[Jason Gunthorpe <jgg@ziepe.ca>]

On Tue, Sep 05, 2023 at 08:09:39PM +0530, Vasant Hegde wrote:
> >>   - set_dev_pasid() will check the compatibility and bind device/pasid only if
> >> its compatibility. In AMD case we will check against protection domain. Ex:
> >>  If we have two devices (devA and devB) in two different protection domain then:
> >>    set_dev_pasid(sva_domain, devA, pasidX) - SUCCESS
> >>    set_dev_pasid(sva_domain, devB, pasidX) - Compatibility check fail
> 
> Here I expect it to fails as devB is in different protection domain. From
> invalidation point of view, devA and devB are not compatible. Hence I think it
> should fail binding.

No, that isn't the best thing to do - you could do this, but it will
be more inefficient.

The domain should represent the ioptes not the target of invalidation.

> >>   - During invalidation, we will retrieve SVA protection domain using mmu
> >> notifier. Use device protection domain which was tracked in this SVA domain for
> >> invalidation.
> > 
> > The iommu_domain/protection_domain must NOT be 1:1 with a device. The
> > driver must maintain a list of devices attached to the domain, and the
> > per-device-attachment parameters like PASID/cache tags/etc.
> 
> We want to track SVA protection domain to device protection domain link. So that
> invalidation becomes straight.
> If we track dev/PASID then I am not sure how we can solve duplicate invalidation
> issue that we have today (i. e. if we have two devices within same protection
> domain and if we track dev/pasid combinatin, then will call invalidation twice).

Look at what Michael is doing for SMMUv3, they have a per-smmu
instance cache tag and a per-device ATC invalidation they need to
issue. They keep a sorted list of device attachments and simply do one
invalidation per-smmu instance and one invalidation per entry.

It is pretty simple logic.

> > PAGING domains need to support PASID+PRI, in today's language that
> > means UNMANAGED domains.
> > 
> > We have many use cases for PRI support with generic PAGING domains.
> 
> What is that usecase? That's what I am trying to understand it better.

iommufd will require this for generic vSVA in all paths that don't use
nesting.

We have use cases to share a KVM page table with PRI.

Google apparently has some usecase since they are fixing it in ARM.

Besides that, it is the IOMMU API, drivers have to implement it, you
don't get to pick and choose.

> > SVA is a special case of a PAGING domain where there is no map/unmap/invalidate
> > API and the IO page table comes from a mm_struct.
> > 
> > Start by making UNMANAGED (aka PAGING) domains work with PASID + PRI
> > and then SVA is a very small incremental step.
> 
> If there is a real use case then why not? We can do that. But for now we want
> the proper SVA to go first.

You are not going to get SVA without also properly doing all
infrastructure to enable paging and unmanaged - I won't support
another shortcut hackjob for SVA that needs unwinding like ARM has.

PASID and PRI must not be tightly linked to SVA.
      
Jason