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

[Xiao, Jack]

Can you explain what the benefits can be gained from AGP aperture enablement? Otherwise, it would increase our maintenance workload.

Regards,
Jack

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken at gmail.com> 
Sent: Wednesday, August 29, 2018 4:47 PM
To: Kuehling, Felix <Felix.Kuehling at amd.com>; Koenig, Christian <Christian.Koenig at amd.com>; Xiao, Jack <Jack.Xiao at amd.com>; amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

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