Re: [PATCH v3 03/10] iommu/sva: Manage process address spaces

[Jean-Philippe Brucker <jean-philippe.brucker@arm.com>]

On 25/09/2018 04:15, Lu Baolu wrote:
>> +     /* If an io_mm already exists, use it */
>> +     spin_lock(&iommu_sva_lock);
>> +     idr_for_each_entry(&iommu_pasid_idr, io_mm, i) {
> 
> This might be problematic for vt-d (and other possible arch's which use
> PASID other than SVA). When vt-d iommu works in scalable mode, a PASID
> might be allocated for:
> 
> (1) SVA
> (2) Device Assignable Interface (might be a mdev or directly managed
>      within a device driver).
> (3) SVA in VM guest
> (4) Device Assignable Interface in VM guest
> 
> So we can't expect that an io_mm pointer was associated with each PASID.

Yes as discussed on the previous series, we'll need to move the PASID
allocator outside of iommu-sva at some point, and even outside of
drivers/iommu since device driver might want to use the PASID allocator
without an IOMMU.

To be usable by all consumers of PASIDs, that allocator will need the
same interface as IDR, so I don't think we have a problem here. I
haven't had time or need to write such allocator yet (and don't plan to
do it as part of this series), but I drafted an interface, that at least
fulfills the needs of SVA.

* Single system-wide PASID space
* Multiple consumers, each associating their own structure to PASIDs.
  Each consumer gets a token.
* Device drivers might want to use both SVA and private PASIDs for a
  device at the same time.
* In my opinion "pasid" isn't the right name, "ioasid" would be better
  but that's not important.

typedef unsigned int pasid_t;

/* Returns consumer token */
void *pasid_get_consumer();
void pasid_put_consumer(void *consumer);

/* Returns pasid or invalid (pasid_t)(-1) */
pasid_t pasid_alloc(void *consumer, pasid_t min, pasid_t max,
                    void *private);
void pasid_remove(pasid_t pasid);

/* Iterate over PASIDs for this consumer. Func returns non-zero to stop
iterating */
int pasid_for_each(void *consumer, void *iter_data,
		   int (*func)(void *iter_data, pasid_t pasid,
			       void *private));
/* Returns priv data or NULL */
void *pasid_find(void *consumer, pasid_t pasid);

Thanks,
Jean

> And this code might run into problem if the pasid is allocated for
> usages other than SVA.
> 
> Best regards,
> Lu Baolu