[PATCH] drm/amd/amdgpu: Fix iova debugfs for non-iommu case

[Tom St Denis]

On 19/09/17 01:23 PM, Christian König wrote:
> Am 19.09.2017 um 19:20 schrieb Tom St Denis:
>> On 19/09/17 01:18 PM, Christian König wrote:
>>> Am 19.09.2017 um 19:14 schrieb Tom St Denis:
>>>> On 19/09/17 01:10 PM, Christian König wrote:
>>>>> As far as I know we don't need #ifdefs cause there are dummy 
>>>>> functions when IOMMU is not compiled in.
>>>>>
>>>>> But this patch is a NAK since it circumvents the /dev/mem 
>>>>> restriction when IOMMU is disabled and that is not something we can 
>>>>> easily allow.
>>>>
>>>> I raised this objection 24 hours ago and was told to go ahead with 
>>>> the read/write methods anyways.
>>>>
>>>> Short of making a list of mappings/allocations in the ttm layer I 
>>>> have no idea how we would track this in the non-iommu case which 
>>>> means the entire iova debugfs entry should have been NAK'ed in the 
>>>> first place.
>>>>
>>>> I'm open to suggestions.
>>>
>>> As long as IOMMU is enabled the entry is perfectly fine, cause only 
>>> memory mapped to the IOMMU is accessible to the GPU and so accessible 
>>> using the debugfs entry as well.
>>
>> On devices where there's no translation (e.g. Carrizo) does the iommu 
>> layer track mappings?  I'm wondering if on those I could seek outside 
>> of boundaries and read system memory anyways.
> 
> Why do you think there isn't any translation on CZ? I mean the whole ATC 
> thing uses the IOMMU (v2 in this case) on APUs.

On my Carrizo devel system (A12-9800) the GPU dma addresses are physical 
addresses.  I only first saw iommu with APUs enabled on Raven.

I'll see if I can read non-mapped pages via the iova file on my Carrizo.

>>> Only when IOMMU is disabled we can't go this way cause we can't know 
>>> which memory is mapped to the driver and which isn't (or could we 
>>> somehow track this?).
>>>
>>> I suggest to not create the file in the first place if IOMMU is 
>>> disabled.
>>
>> I could easily do this (basically check if the domain is not null at 
>> debugfs init time).
> 
> Yeah, please do this for now.
> 
> I mean it would be great to have this interface even with IOMMU 
> disabled, but of hand I can't think of a way to allow this.

Sent to the list already.  Tested with enabled/disabled.  Seems to work 
for me.

Tom