On 2019-08-24 2:59 p.m., Christian König wrote:
> Am 24.08.19 um 18:24 schrieb Kuehling, Felix:
>> On 2019-08-24 7:13, Christian König wrote:
>>> Am 23.08.19 um 23:33 schrieb Kuehling, Felix:
>>>> From: Oak Zeng <Oak.Zeng@xxxxxxx>
>>>>
>>>> Set snooped PTE flag according to mapping flag. Write request to a
>>>> page with snooped bit set, will send out invalidate probe request
>>>> to TCC of the remote GPU where the vram page resides.
>>>>
>>>> Change-Id: I799f68ec7a5a1abf32075f5ef31051641a0b3736
>>>> Signed-off-by: Oak Zeng <Oak.Zeng@xxxxxxx>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> index 9aafcda6c488..8a7c4ec69ae8 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> @@ -604,6 +604,9 @@ static uint64_t gmc_v9_0_get_vm_pte_flags(struct
>>>> amdgpu_device *adev,
>>>> if (flags & AMDGPU_VM_PAGE_PRT)
>>>> pte_flag |= AMDGPU_PTE_PRT;
>>>> + if (flags & AMDGPU_VM_PAGE_INVALIDATE_PROBE)
>>>> + pte_flag |= AMDGPU_PTE_SNOOPED;
>>>> +
>>> That is still a NAK without further checks. We need to make absolutely
>>> sure that we don't set this when PCIe routing is in use.
>> The only place where AMDGPU_VM_... flags are accepted from user mode
>> seems to be amdgpu_gem_va_ioctl. It has an explicit set of valid_flags
>> it accepts. The new INVALIDATE_PROBE flag is not part of it. That means
>> user mode will not be able to set it directly. If we added it to the
>> set of valid_flags in amdgpu_gem_va_ioctl, we'd need to add appropriate
>> checks at the same time.
>>
>> KFD does not expose AMDGPU_VM_... flags directly to user mode. It only
>> sets the INVALIDATE_PROBE flag in kernel mode for mappings in the same
>> XGMI hive on Arturus (in patch 4).
>>
>> If there is something I'm missing, please point it out. But AFAICT the
>> checking that is currently done should satisfy your requirements.
>
> The hardware behavior depends on the placement of the buffer, so at
> bare minimum we need to check if it's pointing to PCIe or local (check
> the system bit).
>
> But even if it's local what is the behavior for local memory? E.g. not
> accessed through XGMI?
>
> As far as I can see what we need to check here is that this is a
> remote access over XGMI and then (and only then!) we are allowed to
> set the snoop bit on the PTE.
My point is, the only place where this bit can get set right now is in
kernel mode in amdgpu_amdkfd_gpuvm.c. See patch 4 of my series. That
code already has all the right conditions to make sure the
INVALIDATE_PROBE bit is only set in the correct circumstances (remote
XGMI mappings in the same hive):
> + switch (adev->asic_type) {
> + case CHIP_ARCTURUS:
> + if (mem->alloc_flags & ALLOC_MEM_FLAGS_VRAM) {
> + if (bo_adev == adev) {
> + ...
> + } else {
> + ...
> + if (amdgpu_xgmi_same_hive(adev, bo_adev))
> + mapping_flags |=
> + AMDGPU_VM_PAGE_INVALIDATE_PROBE;
> + }
> + } else {
I think you're asking me to add an extra check for the same conditions
in the GMC code? Like GMC doesn't trust amdgpu_amdkfd. It seems
completely redundant and a bit paranoid to me. gmc_v9_0_get_vm_pte_flags
doesn't even have all the information it needs to make that determination.
Regards,
Felix
>
> Regards,
> Christian.
>
>>
>> Regards,
>> Felix
>>
>>> Christian.
>>>
>>>> return pte_flag;
>>>> }
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
