Re: [Nouveau] [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function

From: Felix Kuehling <felix.kuehling@amd.com>
To: "Thomas Zimmermann" <tzimmermann@suse.de>,
	"Christian König" <christian.koenig@amd.com>,
	alexander.deucher@amd.com, airlied@linux.ie, daniel@ffwll.ch,
	bskeggs@redhat.com, ray.huang@amd.com,
	linux-graphics-maintainer@vmware.com, sroland@vmware.com,
	zackr@vmware.com, shashank.sharma@amd.com, sam@ravnborg.org,
	emil.velikov@collabora.com, nirmoy.das@amd.com
Cc: nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	amd-gfx@lists.freedesktop.org
Subject: Re: [Nouveau] [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function
Date: Wed, 14 Apr 2021 04:37:26 -0400	[thread overview]
Message-ID: <f2e3ae0d-083c-c919-74bd-8fedbfad7718@amd.com> (raw)
In-Reply-To: <63c90815-c9d8-68df-daa1-c165215a6b7c@suse.de>

Am 2021-04-14 um 3:44 a.m. schrieb Thomas Zimmermann:
> Hi
>
> Am 07.04.21 um 21:49 schrieb Felix Kuehling:
>> On 2021-04-07 3:34 p.m., Felix Kuehling wrote:
>>> On 2021-04-07 7:25 a.m., Christian König wrote:
>>>>>>>> +    /*
>>>>>>>> +     * Don't verify access for KFD BOs. They 
> don't have a GEM
>>>>>>>> +     * object associated with them.
>>>>>>>> +     */
>>>>>>>> +    if (bo->kfd_bo)
>>>>>>>> +        goto out;
>>>>>>> Who does the access verification now?
>>>>>> This is somewhat confusing.
>>>>>>
>>>>>> I took this check as-is, including the comment, from amdgpu's
>>>>>> verify_access function. The verify_access function was called by
>>>>>> ttm_bo_mmap. It returned 0 and ttm_bo_mmap did the mapping.
>>>>> This is probably a left-over from when we mapped BOs using /dev/kfd. 
> We
>>>>> changed this to use /dev/dri/renderD* a long time ago to fix CPU
>>>>> mapping
>>>>> invalidations on memory evictions. I think we can let GEM do the
>>>>> access
>>>>> check.
>>>>
>>>> Ok, good to know.
>>>>
>>>> Thomas can you remove the extra handling in a separate prerequisite
>>>> patch?
>>>>
>>>> If anybody then bisects to this patch we at least know what to do
>>>> to get it working again.
>>>
>>> FWIW, I ran KFDTest test with this shortcut removed on current
>>> amd-staging-drm-next + my HMM patch series, and it didn't seem to
>>> cause any issues.
>>
>> Wait, I celebrated too soon. I was running the wrong kernel. I do see
>> some failures where access is being denied. I need to do more debugging 
>
>> to figure out what's causing that.
>
> Any news here? I saw the patch at [1], which removes the kfd_bo test.
> Can I assume that the series addresses the issue?

Yes, that series fixes the problem. I need to pester more people to
review it.

Regards,
  Felix

>
> Best regards
> Thomas
>
> [1] https://patchwork.freedesktop.org/patch/427516/?series=88822&rev=1
>
>>
>> Regards,
>>    Felix
>>
>>
>>>
>>> Regards,
>>>   Felix
>>>
>>>
>>>>
>>>> Regards,
>>>> Christian. 
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau