linux-kernel - Re: [PATCH v8 00/22] perf arm-spe: Refactor decoding & dumping flow

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 11 Nov 2020 16:20:26 +0000
From:   André Przywara <andre.przywara@....com>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Leo Yan <leo.yan@...aro.org>
Cc:     Dave Martin <Dave.Martin@....com>,
        James Clark <james.clark@....com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Al Grant <Al.Grant@....com>, Wei Li <liwei391@...wei.com>,
        John Garry <john.garry@...wei.com>,
        Will Deacon <will@...nel.org>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8 00/22] perf arm-spe: Refactor decoding & dumping flow

On 11/11/2020 16:15, Arnaldo Carvalho de Melo wrote:
> Em Wed, Nov 11, 2020 at 01:10:51PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Wed, Nov 11, 2020 at 03:11:27PM +0800, Leo Yan escreveu:
>>> This is patch set v8 for refactoring Arm SPE trace decoding and dumping.
>>>
>>> This version addresses Andre's comment to pass parameter '&buf_len' at
>>> the last call arm_spe_pkt_snprintf() in the function arm_spe_pkt_desc().
>>>
>>> This patch set is cleanly applied on the top of perf/core branch
>>> with commit 644bf4b0f7ac ("perf jevents: Add test for arch std events").
>>>
>>> I retested this patch set on Hisilicon D06 platform with commands
>>> "perf report -D" and "perf script", compared the decoding results
>>> between with this patch set and without this patch set, "diff" tool
>>> shows the result as expected.
>>
>> With the patches I applied I'm getting:
>>
>> util/arm-spe-decoder/arm-spe-pkt-decoder.c: In function 'arm_spe_pkt_desc':
>> util/arm-spe-decoder/arm-spe-pkt-decoder.c:410:3: error: left shift count >= width of type [-Werror]
>>    case 1: ns = !!(packet->payload & NS_FLAG);
>>    ^
>> util/arm-spe-decoder/arm-spe-pkt-decoder.c:411:4: error: left shift count >= width of type [-Werror]
>>     el = (packet->payload & EL_FLAG) >> 61;
>>     ^
>> util/arm-spe-decoder/arm-spe-pkt-decoder.c:411:4: error: left shift count >= width of type [-Werror]
>> util/arm-spe-decoder/arm-spe-pkt-decoder.c:416:3: error: left shift count >= width of type [-Werror]
>>    case 3: ns = !!(packet->payload & NS_FLAG);
>>    ^
>>   CC       /tmp/build/perf/util/arm-spe-decoder/arm-spe-decoder.o
>>  
>>
>> On:
>>
>>   16    11.70 android-ndk:r12b-arm          : FAIL arm-linux-androideabi-gcc (GCC) 4.9.x 20150123 (prerelease)
>>   17    11.32 android-ndk:r15c-arm          : FAIL arm-linux-androideabi-gcc (GCC) 4.9.x 20150123 (prerelease)
>>
>> That were building ok before, builds still under way, perhaps its just
>> on these old systems...
> 
> [acme@...e perf]$ git bisect good
> cc6fa07fb1458cca3741919774eb050976471000 is the first bad commit
> commit cc6fa07fb1458cca3741919774eb050976471000
> Author: Leo Yan <leo.yan@...aro.org>
> Date:   Wed Nov 11 15:11:28 2020 +0800
> 
>     perf arm-spe: Include bitops.h for BIT() macro
> 
>     Include header linux/bitops.h, directly use its BIT() macro and remove
>     the self defined macros.
> 
>     Signed-off-by: Leo Yan <leo.yan@...aro.org>
>     Reviewed-by: Andre Przywara <andre.przywara@....com>
>     Link: https://lore.kernel.org/r/20201111071149.815-2-leo.yan@linaro.org
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
> 
>  tools/perf/util/arm-spe-decoder/arm-spe-decoder.c     | 5 +----
>  tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c | 3 +--
>  2 files changed, 2 insertions(+), 6 deletions(-)


Ah, thanks! I think I mentioned the missing usage of BIT_ULL() in an
earlier review, and thought this was fixed. Possibly this gets fixed in
a later patch in this series, and is a temporary regression?

How do you want to handle this? Shall Leo resend, amending this patch
(and merging 06 and 07 on the way ;-)?

Cheers,
Andre