[PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[Christian König]

Completely agree with Felix here.

It makes system memory access slightly simpler, but I would say that you 
accidentally corrupt the GART table and that you accidentally corrupt 
the the AGP aperture is equally likely.

Regards,
Christian.

Am 28.08.2018 um 20:12 schrieb Felix Kuehling:
> The GPU always has access to the entire guest physical address space.
> You can just program arbitrary addresses into page table entries and set
> the system bit. That's how GART and GPUVM mappings work. They will not
> go through the AGP aperture. And there is no mechanism (other than an
> IOMMU) to protect system memory from GPU access.
>
> Regards,
>    Felix
>
>
> On 2018-08-28 07:41 AM, Christian König wrote:
>> Well that is indeed true, but we still have IOMMU between the GPU and
>> the system memory.
>>
>> So we should still catch issues when something goes terrible wrong.
>>
>> Additional to that only the system domain, e.g. kernel copies, page
>> table updates etc are allowed to use it.
>>
>> Regards,
>> Christian.
>>
>> Am 28.08.2018 um 09:06 schrieb Xiao, Jack:
>>> I mean it has risk to make GPU allowed to access to most system
>>> memory without explicit claiming, it's easier to mask problem.
>>>
>>> Regards,
>>> Jack
>>>
>>> -----Original Message-----
>>> From: Koenig, Christian
>>> Sent: Tuesday, August 28, 2018 2:46 PM
>>> To: Xiao, Jack <Jack.Xiao at amd.com>; Kuehling, Felix
>>> <Felix.Kuehling at amd.com>; Christian König
>>> <ckoenig.leichtzumerken at gmail.com>; amd-gfx at lists.freedesktop.org
>>> Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address
>>> space on GMC9
>>>
>>>> This series patches seems to make AGP aperture allowed to access any
>>>> system memory (16GB) bypass GPU VM protection.
>>> The system aperture should only be active in the system domain, or
>>> otherwise applications would have access to local memory as well.
>>>
>>> There are some bits in the VM registers to enable/disable that, but
>>> when we would have that setting incorrect we would see quite a bunch
>>> of other problems.
>>>
>>> Might still be a good idea to double check if all the bits are setup
>>> correctly.
>>>
>>> Regards,
>>> Christian.
>>>
>>> Am 28.08.2018 um 07:31 schrieb Xiao, Jack:
>>>> This series patches seems to make AGP aperture allowed to access any
>>>> system memory (16GB) bypass GPU VM protection.
>>>> If someone made a wrong logic requesting an illegal address which
>>>> occasionally was located inside AGP aperture, but without any VM
>>>> protection, the illegal address would be finally translated into a
>>>> system memory address; if GPU read/wrote such system memory address,
>>>> the system memory address might belong to kernel or any user
>>>> application, the r/w operation would result in any unpredictable issue.
>>>> The most important is that such kind of issue is so hard to be
>>>> addressed.
>>>> Is it worth doing this, but exposing risk?
>>>>
>>>> Regards,
>>>> Jack
>>>>
>>>> -----Original Message-----
>>>> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of
>>>> Felix Kuehling
>>>> Sent: Tuesday, August 28, 2018 3:03 AM
>>>> To: Christian König <ckoenig.leichtzumerken at gmail.com>;
>>>> amd-gfx at lists.freedesktop.org; Koenig, Christian
>>>> <Christian.Koenig at amd.com>
>>>> Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address
>>>> space on GMC9
>>>>
>>>> The point of this series seems to be to allow access to small system
>>>> memory BOs (one page) without a GART mapping. I'm guessing that
>>>> reduces pressure on the GART and removes the need for HDP and TLB
>>>> flushes. Why does Patch 10 only enable that on GFXv9? Is there less
>>>> benefit on older chips?
>>>>
>>>> Is this related to your recent changes to allow page tables in
>>>> system memory?
>>>>
>>>> See my replies to patch 6 and 8. Other than that, the series is
>>>> Acked-by: Felix Kuehling <Felix.Kuehling at amd.com>
>>>>
>>>> Regards,
>>>>      Felix
>>>>
>>>>
>>>> On 2018-08-27 12:53 PM, Christian König wrote:
>>>>> Only use the lower address space on GMC9 for the system domain.
>>>>> Otherwise we would need to sign extend GMC addresses.
>>>>>
>>>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>>>> ---
>>>>>     drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
>>>>>     1 file changed, 3 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> index e44b5191735d..d982956c8329 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> @@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
>>>>>         if (r)
>>>>>             return r;
>>>>>     -    /* Set the internal MC address mask
>>>>> -     * This is the max address of the GPU's
>>>>> -     * internal address space.
>>>>> +    /* Use only the lower range for the internal MC address mask.
>>>>> This is
>>>>> +     * the max address of the GPU's internal address space.
>>>>>          */
>>>>> -    adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>>>>> +    adev->gmc.mc_mask = 0x7fffffffffffULL;
>>>>>            /* set DMA mask + need_dma32 flags.
>>>>>          * PCIE - can handle 44-bits.
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx