linux-kernel - Re: Warning: check cp_fw_version and update it to realize GRBM requires 1-cycle delay in cp firmware

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <9e4d867b-cd30-eb2d-406b-d6a462bdb04c@amd.com>
Date:   Mon, 6 Jan 2020 15:07:57 +0100
From:   Christian König <christian.koenig@....com>
To:     Michel Dänzer <michel@...nzer.net>,
        Alex Deucher <alexdeucher@...il.com>,
        Paul Menzel <pmenzel+amd-gfx@...gen.mpg.de>
Cc:     David Airlie <airlied@...ux.ie>,
        LKML <linux-kernel@...r.kernel.org>,
        amd-gfx list <amd-gfx@...ts.freedesktop.org>,
        Chang Zhu <Changfeng.Zhu@....com>,
        Alex Deucher <alexander.deucher@....com>,
        Christian König <christian.koenig@....com>
Subject: Re: Warning: check cp_fw_version and update it to realize GRBM
 requires 1-cycle delay in cp firmware

Am 06.01.20 um 12:29 schrieb Michel Dänzer:
> On 2019-12-26 5:53 p.m., Alex Deucher wrote:
>> On Thu, Dec 26, 2019 at 5:11 AM Paul Menzel
>>>> [   13.446975] [drm] Warning: check cp_fw_version and update it to realize                          GRBM requires 1-cycle delay in cp firmware
>>> Chang, it looks like you added that warning in commit 11c6108934.
>>>
>>>> drm/amdgpu: add warning for GRBM 1-cycle delay issue in gfx9
>>>>
>>>> It needs to add warning to update firmware in gfx9
>>>> in case that firmware is too old to have function to
>>>> realize dummy read in cp firmware.
>>> Unfortunately, it looks like you did not even check how the warning is
>>> formatted (needless spaces), so I guess this was totally untested. Also,
>>> what is that warning about, and what is the user supposed to do? I am
>>> unable to find `cp_fw_version` in the source code at all.
>>>
>> The code looks fine.  Not sure why it's rendering funny in your log.
>>                 DRM_WARN_ONCE("Warning: check cp_fw_version and update
>> it to realize \
>>                               GRBM requires 1-cycle delay in cp firmware\n");
> Looks like the leading spaces after the backslash are included in the
> string. Something like this should be better:
>
>          DRM_WARN_ONCE("Warning: check cp_fw_version and update "
>                        "GRBM requires 1-cycle delay in cp firmware\n");

Warning and error messages in the kernel should be on a single line if 
possible to allow for searching the kernel code for the string of the 
message.

I suggest to just shorten the message. Especially it is irrelevant why 
the CP firmware is to old, that is debugging info at best. Additional to 
that the "Warning:" is superfluous.

Something like DRM_WARN_ONCE("CP firmware version to old, please 
update!") should perfectly do it.

Regards,
Christian.

>
> (or maybe the intention was to put the second sentence on a new line?)