[PATCH v2] drm/amdgpu: fix the wrong logic checking when secure buffer is created (v2)

[Christian König]

Am 18.02.20 um 10:44 schrieb Huang Rui:
> On Tue, Feb 18, 2020 at 05:35:35PM +0800, Huang Rui wrote:
>> On Tue, Feb 18, 2020 at 05:15:33PM +0800, Koenig, Christian wrote:
>>> Am 18.02.20 um 09:55 schrieb Huang Rui:
>>>> On Tue, Feb 18, 2020 at 04:50:01PM +0800, Koenig, Christian wrote:
>>>>> Am 18.02.20 um 09:30 schrieb Huang Rui:
>>>>>> While the current amdgpu doesn't support TMZ, it will return the error if user
>>>>>> mode would like to allocate secure buffer.
>>>>>>
>>>>>> Fixes: 17d907c drm/amdgpu: implement TMZ accessor (v3)
>>>>>>
>>>>>> v2: we didn't need this checking anymore.
>>>>> You misunderstood me.
>>>>>
>>>> ok...
>>>>
>>>>>> Signed-off-by: Huang Rui <ray.huang at amd.com>
>>>>>> ---
>>>>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 5 -----
>>>>>>     1 file changed, 5 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>>> index b51a060..5cbc63a 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>>> @@ -235,11 +235,6 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>>>>>>     	if (args->in.domains & ~AMDGPU_GEM_DOMAIN_MASK)
>>>>>>     		return -EINVAL;
>>>>>>     
>>>>>> -	if (amdgpu_is_tmz(adev) && (flags & AMDGPU_GEM_CREATE_ENCRYPTED)) {
>>>>>> -		DRM_ERROR("Cannot allocate secure buffer since TMZ is disabled\n");
>>>>> Only the error message should be removed, but we still need to test the
>>>>> flag.
>>>>>
>>>>> E.g. in general we should never log that userspace sends invalid parameters.
>>>>>
>>>> Why? It probably can help userspace to understand what wrong input it is.
>>> That's correct, but it also allows userspace to spam the logs with those
>>> messages.
>>>
>>> While the kernel tools are rather clever and filter out repeated
>>> messages this is still considered a bad idea because it might hide more
>>> serious error messages.
>>>
>>> What we could do is either make this debug severity or use something
>>> DRM_NOTE_ONCE() to print it only once.
>> I see, make sense. There is no DRM_ERROR_ONCE() and I think this case is
> How about about DRM_ERROR_RATELIMITED()?

That would work, but I think DRM_NOTE_ONCE() is more appropriate since 
this is not an error caused inside the kernel.

Regards,
Christian.

>
> Thanks,
> Ray