linux-kernel - Re: [PATCH 07/19] coresight: trbe: Refactor AUX flag setting

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]

Message-ID: <27417fd5-902e-4474-8d60-356a94ac9590@linaro.org>
Date: Fri, 12 Dec 2025 16:50:52 +0200
From: James Clark <james.clark@...aro.org>
To: Leo Yan <leo.yan@....com>
Cc: coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
 Suzuki K Poulose <suzuki.poulose@....com>, Mike Leach
 <mike.leach@...aro.org>, Anshuman Khandual <anshuman.khandual@....com>,
 Yeoreum Yun <yeoreum.yun@....com>, Will Deacon <will@...nel.org>,
 Mark Rutland <mark.rutland@....com>, Tamas Petz <tamas.petz@....com>,
 Tamas Zsoldos <tamas.zsoldos@....com>,
 Arnaldo Carvalho de Melo <acme@...nel.org>,
 Namhyung Kim <namhyung@...nel.org>, Jiri Olsa <jolsa@...nel.org>,
 Ian Rogers <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>
Subject: Re: [PATCH 07/19] coresight: trbe: Refactor AUX flag setting



On 10/12/2025 17:43, Leo Yan wrote:
> On Tue, Dec 09, 2025 at 01:37:39PM +0000, James Clark wrote:
> 
> [...]
> 
>>> +	if (!is_trbe_running(trbsr))
>>> +		perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
>>> +
>>
>> Do we need the complexity of COLLISION (changed to PARTIAL in a later
>> commit) and TRUNCATED at the same time?
> 
>>  From my understanding, TRUNCATED would be when Perf is too slow and we want
>> to disable the event for it to catch up, and PARTIAL is when there was some
>> buffer error so technically we don't need the event to be disabled, but Perf
>> still needs to be notified if we want to start only resetting the decoder
>> when a flag is set rather than on all aux records?
> 
> The main difference between the TRUNCATED flag and the COLLISION/PARTIAL
> flags is that the TRUNCATED flag implicitly asks the event core layer to
> disable the PMU event.  In contrast, the COLLISION and PARTIAL flags are
> only stored in AUX record and no additional action in the event core
> layer.
> 
> Setting the TRUNCATED flag would be a over-killer if the buffer has
> free space, as it would run the expensive sequence of disabling both ETE
> and TRBE and then requiring userspace to re-enable them.
> 
>> There is no happy path where there is a buffer error, so I'm not sure why we
>> need the complexity of an extra case? There's no harm in disabling the event
>> on a buffer error. Also it's hard to see what the exact scenario for PARTIAL
>> is, because we set truncated anyway for fatal statuses. The comment in the
>> later commit for the PARTIAL change just says "trace was stopped", but not
>> why.
> 
> The trace can be stopped on Fill mode (stop on wrap) or Stop on trigger, so
> it is not (only) about buffer error.  In these cases, even though the trace
> unit is stopped, the buffer may still have space available, tracing can be
> directly re-enabled in interrupt handler, thus the COLLISION/PARTIAL flags
> are better choices.

I suppose this is the bit that I don't understand. If there is space 
left then why is the stop hit at all?

For the other cases, I agree that it's overkill, but my point is that 
doesn't matter because we don't expect it to be hit in normal cases 
anyway. So it's worth the simplification of not doing anything different 
than TRUNCATED.

> 
> Only when the buffer is actually used out (i.e., handle->size == 0), it is
> appropriate to set TRUNCATED flag, since at that point we must wait for
> userspace to read out the data.
> 
> Thanks,
> Leo