Re: [PATCH v5 02/25] iommu/sva: Manage process address spaces

[Felix Kuehling <felix.kuehling@amd.com>]

Am 2020-04-20 um 8:40 a.m. schrieb Christian König:
> Am 20.04.20 um 13:55 schrieb Christoph Hellwig:
>> On Mon, Apr 20, 2020 at 01:44:56PM +0200, Christian König wrote:
>>> Am 20.04.20 um 10:10 schrieb Christoph Hellwig:
>>>> On Mon, Apr 20, 2020 at 09:42:13AM +0200, Jean-Philippe Brucker wrote:
>>>>> Right, I can see the appeal. I still like having a single mmu
>>>>> notifier per
>>>>> mm because it ensures we allocate a single PASID per mm (as
>>>>> required by
>>>>> x86). I suppose one alternative is to maintain a hashtable of
>>>>> mm->pasid,
>>>>> to avoid iterating over all bonds during allocation.
>>>> Given that the PASID is a pretty generic and important concept can
>>>> we just add it directly to the mm_struct and allocate it lazily once
>>>> we first need it?
>>> Well the problem is that the PASID might as well be device specific.
>>> E.g.
>>> some devices use 16bit PASIDs, some 15bit, some other only 12bit.
>>>
>>> So what could (at least in theory) happen is that you need to allocate
>>> different PASIDs for the same process because different devices need
>>> one.
>> This directly contradicts the statement from Jean-Philippe above that
>> x86 requires a single PASID per mm_struct.  If we may need different
>> PASIDs for different devices and can actually support this just
>> allocating one per [device, mm_struct] would make most sense of me, as
>> it doesn't couple otherwise disjoint state.
>
> Well I'm not an expert on this topic. Felix can probably tell you a
> bit more about that.
>
> Maybe it is sufficient to keep the allocated PASIDs as small as
> possible and return an appropriate error if a device can't deal with
> the allocated number.
>
> If a device can only deal with 12bit PASIDs and more than 2^12 try to
> use it there isn't much else we can do than returning an error anyway.

I'm probably missing some context. But let me try giving a useful reply.

The hardware allows you to have different PASIDs for each device
referring to the same address space. But I think it's OK for software to
choose not to do that. If Linux wants to manage one PASID namespace for
all devices, that's a reasonable choice IMO.

Different devices have different limits for the size of PASID they can
support. For example AMD GPUs support 16-bits but the IOMMU supports
less. So on APUs we use small PASIDs for contexts that want to use
IOMMUv2 to access memory, but bigger PASIDs for contexts that do not.

I choose the word "context" deliberately, because the amdgpu driver uses
PASIDs even when we're not using IOMMUv2, and we're using them to
identify GPU virtual address spaces. There can be more than one per
process. In practice you can have two, one for graphics (not SVM,
doesn't use IOMMUv2) and one for KFD compute (SVM, can use IOMMUv2 on APUs).

Because the IOMMUv2 supports only smaller PASIDs, we want to avoid
exhausting that space with PASID allocations that don't use the IOMMUv2.
So our PASID allocation function has a "size" parameter, and we try to
allocated a PASID as big as possible in order to leave more precious
smaller PASIDs for contexts that need them.

The bottom line is, when you allocate a PASID for a context, you want to
know how small it needs to be for all the devices that want to use it.
If you make it too big, some device will not be able to use it. If you
make it too small, you waste precious PASIDs that could be used for
other contexts that need them.

Regards,
  Felix