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

Tue Sep 19 17:37:12 UTC 2017

As I predicted ...

[root at carrizo ~]# xxd -l 1024 -s 0xC0000 
/sys/kernel/debug/dri/0/amdgpu_iova
000c0000: 55aa 74e9 a902 0000 0000 0000 0000 0000  U.t.............
000c0010: 0000 0000 0000 0000 4c02 0000 0000 4942  ........L.....IB
000c0020: 4daa 2e93 0000 0000 0000 0000 0000 0004  M...............
000c0030: 2037 3631 3239 3535 3230 0000 0000 0000   761295520......
000c0040: a102 0000 0000 0000 2802 0000 0000 0000  ........(.......
000c0050: 3037 2f32 372f 3136 2030 343a 3036 0000  07/27/16 04:06..
000c0060: 3600 0000 e9b4 0300 e9c3 0300 0000 f400  6...............
000c0070: 0013 0000 00d0 0100 1a16 20e1 0280 7e00  .......... ...~.
000c0080: a200 4402 1200 0000 0000 003c 400e 0207  ..D........<@...
000c0090: 3c01 1a00 0400 0000 eea0 ff06 0008 3040  <.............0@
000c00a0: 0e01 0000 0000 0000 1403 0000 0000 0000  ................
000c00b0: be7e 1100 b907 1ad6 502c 0000 0000 0000  .~......P,......
000c00c0: 0000 0000 1440 4143 0000 0000 1000 0000  ..... at AC........
000c00d0: 4200 0000 407c 0600 2000 2000 1200 0e00  B...@|.. . .....
000c00e0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
000c00f0: 0000 0000 3131 332d 4339 3534 3031 3031  ....113-C9540101
000c0100: 2d30 3036 0046 474c 2045 4c4c 4553 4d45  -006.FGL ELLESME
000c0110: 5245 0050 4349 5f45 5850 5245 5353 0047  RE.PCI_EXPRESS.G
000c0120: 4444 5235 000d 0a43 3935 3420 506f 6c61  DDR5...C954 Pola
000c0130: 7269 7331 3020 474c 2041 3120 4744 4452  ris10 GL A1 GDDR
000c0140: 3520 3235 364d 7833 3220 3847 4220 3330  5 256Mx32 8GB 30
000c0150: 3065 2f33 3030 6d20 2020 2020 2020 2020  0e/300m
000c0160: 2020 2020 2020 2020 2020 2020 2020 2020
000c0170: 2020 200d 0a00 0d0a 200d 0a00 2843 2920     ..... ...(C)
000c0180: 3139 3838 2d32 3031 302c 2041 6476 616e  1988-2010, Advan
000c0190: 6365 6420 4d69 6372 6f20 4465 7669 6365  ced Micro Device
000c01a0: 732c 2049 6e63 2e00 4154 4f4d 4249 4f53  s, Inc..ATOMBIOS
000c01b0: 424b 2d41 4d44 2056 4552 3031 352e 3035  BK-AMD VER015.05
000c01c0: 302e 3030 302e 3030 302e 3030 3730 3036  0.000.000.007006
000c01d0: 0043 3935 3430 3130 312e 3030 3600 3132  .C9540101.006.12
000c01e0: 3935 3838 3720 0033 3534 3731 3220 2000  95887 .354712  .
000c01f0: 2020 2020 2020 2020 0041 4d44 5f45 4c4c          .AMD_ELL
000c0200: 4553 4d45 5245 5f43 3935 3430 315f 5854  ESMERE_C95401_XT
000c0210: 5f41 315f 4744 355f 3847 425c 636f 6e66  _A1_GD5_8GB\conf
000c0220: 6967 2e68 0000 0090 2400 0101 4154 4f4d  ig.h....$...ATOM

I can dump arbitrary system memory with the iommu enabled in the bios 
via my carrizo "dev" in the debugfs entry.

So if the issue is arbitrary access is a no no (and I don't get why) 
then the entire patch is a NAK because clearly I can "abuse" it.

Tom

On 19/09/17 01:26 PM, Tom St Denis wrote:
> 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
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx