Greg,
Please find the updates for CoreSight/hwtracing subsystem for v6.12.
Kindly pull
Suzuki
The following changes since commit 47ac09b91befbb6a235ab620c32af719f8208399:
Linux 6.11-rc4 (2024-08-18 13:17:27 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git tags/coresight-next-v6.12
for you to fetch changes up to 988d40a4d4e7d671305bea501562a5d1a1d479fa:
coresight: Make trace ID map spinlock local to the map (2024-08-20 15:02:38 +0100)
----------------------------------------------------------------
coresight: updates for Linux v6.12
CoreSight/hwtracing subsystem updates targeting Linux v6.12:
- Miscellaneous fixes and cleanups
- TraceID allocation per sink, allowing system with > 110 cores for
perf tracing.
Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com>
----------------------------------------------------------------
James Clark (9):
coresight: Remove unused ETM Perf stubs
coresight: Clarify comments around the PID of the sink owner
coresight: Move struct coresight_trace_id_map to common header
coresight: Expose map arguments in trace ID API
coresight: Make CPU id map a property of a trace ID map
coresight: Use per-sink trace ID maps for Perf sessions
coresight: Remove pending trace ID release mechanism
coresight: Emit sink ID in the HW_ID packets
coresight: Make trace ID map spinlock local to the map
Javier Carrasco (1):
coresight: cti: use device_* to iterate over device child nodes
Jie Gan (2):
Coresight: Set correct cs_mode for TPDM to fix disable issue
Coresight: Set correct cs_mode for dummy source to fix disable issue
Suzuki K Poulose (1):
coresight: tmc: sg: Do not leak sg_table
drivers/hwtracing/coresight/coresight-core.c | 37 ++++--
.../hwtracing/coresight/coresight-cti-platform.c | 10 +-
drivers/hwtracing/coresight/coresight-dummy.c | 7 +-
drivers/hwtracing/coresight/coresight-etm-perf.c | 43 ++++---
drivers/hwtracing/coresight/coresight-etm-perf.h | 18 ---
drivers/hwtracing/coresight/coresight-etm3x-core.c | 9 +-
drivers/hwtracing/coresight/coresight-etm4x-core.c | 9 +-
drivers/hwtracing/coresight/coresight-priv.h | 1 +
drivers/hwtracing/coresight/coresight-stm.c | 3 +-
drivers/hwtracing/coresight/coresight-sysfs.c | 3 +-
drivers/hwtracing/coresight/coresight-tmc-etr.c | 7 +-
drivers/hwtracing/coresight/coresight-tmc.h | 5 +-
drivers/hwtracing/coresight/coresight-tpdm.c | 9 +-
drivers/hwtracing/coresight/coresight-trace-id.c | 138 +++++++++------------
drivers/hwtracing/coresight/coresight-trace-id.h | 70 +++++------
include/linux/coresight-pmu.h | 17 ++-
include/linux/coresight.h | 21 +++-
17 files changed, 219 insertions(+), 188 deletions(-)
On 8/30/2024 12:20 AM, Leo Yan wrote:
> On 8/29/24 18:39, Steve Clevenger wrote:>
>> On 8/29/2024 12:55 AM, Leo Yan wrote:
>>> On 8/29/2024 12:17 AM, Steve Clevenger wrote:
>>>>
>>>> Changes in V5:
>>>> - In symbol-elf.c, branch to exit_close label if open file.
>>>> - In trace_event_python.c, correct indentation. set_sym_in_dict
>>>> call parameter "map_pgoff" renamed as "addr_map_pgoff" to
>>>> match local naming.
>>>
>>> For the series:
>>>
>>> Reviewed-by: Leo Yan <leo.yan(a)arm.com>
>>>
>>> Hi Steve,
>>>
>>> Later when you respin a new version patches, it is better to amend
>>> the review
>>> and ACK tags you have received. (Just remind, b4 is your friend for
>>> helping do
>>> these things).
>>>
>>> Thanks,
>>> Leo
>>
>> Got it. Thanks, Leo. By the way, I noticed when I rebased to
>> perf-tools-next the kernel version was 6.11-rc3. Is there any chance
>> this patch gets into 6.11?
>
> Linus' master branch now is 6.11-rc5, it is not far away from the 6.11.
>
> I think we still have much chance to get it done before it. Added
> maintainers
> in case any missing.
>
> Thanks,
> Leo
Hi Leo,
That's great. Unfortuantely, I need to submit a V6 for patch 4/4, which
is the arm-cs-trace-disasm.py script. Kernel instruction trace requires
zeroing map_pgoff. perf makes most offset decisions, but this one is in
the script. It's a one liner.
I know you're a b4 fan, but I've never used it. If I'm able to make
progress with it I'll use it to resubmit. Otherwise more of the same.
Steve
Steve
On 23/08/2024 10:57 am, Ganapatrao Kulkarni wrote:
>
> Hi James/Mike,
>
> On 23-08-2024 02:33 pm, James Clark wrote:
>>
>>
>> On 19/08/2024 11:59 am, Mike Leach wrote:
>>> Hi,
>>>
>>> A new branch of OpenCSD is available - ocsd-consistency-checks-1.5.4-rc1
>>>
>>> Testing I managed to do confirms the N atom on unconditional branches
>>> appear to work. I do not have a test case for the range
>>> discontinuities.
>>>
>>> The checks are enabled using operation flags on decoder creation. See
>>> the docs for details.
>>>
>>> Mike
>>>
>>
>> Hi Mike,
>>
>> I tested the new OpenCSD and I don't see the error anymore in the
>> disassembly script. I'm not sure if we need to go any further and add
>> the backwards check, it looks like just a later symptom and the checks
>> that you've added already prevent it.
>>
>> If you release a new version I can send the perf patch. I was going to
>> use these flags if that looks right to you? As far as I know that's the
>> set that can be always on and won't fail on bad hardware?
>>
>> I also assumed that ETM4_OPFLG_PKTDEC_AA64_OPCODE_CHK can be given even
>> for etmv3 and it's just a nop?
>>
>> diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
>> b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
>> index e917985bbbe6..90967fd807e6 100644
>> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
>> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
>> @@ -685,9 +685,14 @@ cs_etm_decoder__create_etm_decoder(struct
>> cs_etm_decoder_params *d_params,
>> return 0;
>>
>> if (d_params->operation == CS_ETM_OPERATION_DECODE) {
>> + int decode_flags = OCSD_CREATE_FLG_FULL_DECODER;
>> +#ifdef OCSD_OPFLG_N_UNCOND_DIR_BR_CHK
>> + decode_flags |= OCSD_OPFLG_N_UNCOND_DIR_BR_CHK |
>> OCSD_OPFLG_CHK_RANGE_CONTINUE |
>> + ETM4_OPFLG_PKTDEC_AA64_OPCODE_CHK;
>> +#endif
>> if (ocsd_dt_create_decoder(decoder->dcd_tree,
>> decoder->decoder_name,
>> - OCSD_CREATE_FLG_FULL_DECODER,
>> + decode_flags,
>> trace_config, &csid))
>> return -1;
>>
>
> I tried Mike's branch with above James's patch and still the segfault is
> happening to us.
>
Looks like the Perf bug is only on the timestamped decode path, you can
force timeless as a workaround. Timestamps aren't used by the
disassembly script anyway:
--itrace=Zb
Full command:
perf script -i ./kcore -s python:tools/perf/scripts/python/arm-cs-\
trace-disasm.py --itrace=Zb -- -k ./kcore/kcore_dir/kcore
You can also disable timestamps when recording then you don't need the
itrace option. This will save you a lot of data anyway.
But I'm still working on the proper fix.
On 8/29/2024 12:55 AM, Leo Yan wrote:
> On 8/29/2024 12:17 AM, Steve Clevenger wrote:
>>
>> Changes in V5:
>> - In symbol-elf.c, branch to exit_close label if open file.
>> - In trace_event_python.c, correct indentation. set_sym_in_dict
>> call parameter "map_pgoff" renamed as "addr_map_pgoff" to
>> match local naming.
>
> For the series:
>
> Reviewed-by: Leo Yan <leo.yan(a)arm.com>
>
> Hi Steve,
>
> Later when you respin a new version patches, it is better to amend the review
> and ACK tags you have received. (Just remind, b4 is your friend for helping do
> these things).
>
> Thanks,
> Leo
Got it. Thanks, Leo. By the way, I noticed when I rebased to
perf-tools-next the kernel version was 6.11-rc3. Is there any chance
this patch gets into 6.11?
Steve
OpenCSD version 1.5.4 is now released.
This update contains a number of optional consistency checks that the
client can enable in the library to detect incorrect memory images or
corrupt trace being supplied to the decoder by the client.
Mike
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
On 07/08/2024 5:48 pm, Leo Yan wrote:
> Hi all,
>
> On 8/7/2024 3:53 PM, James Clark wrote:
>
> A minor suggestion: if the discussion is too long, please delete the
> irrelevant message ;)
>
> [...]
>
>>> --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
>>> +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
>>> @@ -257,6 +257,11 @@ def process_event(param_dict):
>>> print("Stop address 0x%x is out of range [ 0x%x .. 0x%x
>>> ] for dso %s" % (stop_addr, int(dso_start), int(dso_end), dso))
>>> return
>>>
>>> + if (stop_addr < start_addr):
>>> + if (options.verbose == True):
>>> + print("Packet Dropped, Discontinuity detected
>>> [stop_add:0x%x start_addr:0x%x ] for dso %s" % (stop_addr, start_addr,
>>> dso))
>>> + return
>>> +
>>
>> I suppose my only concern with this is that it hides real errors and
>> Perf shouldn't be outputting samples that go backwards. Considering that
>> fixing this in OpenCSD and Perf has a much wider benefit I think that
>> should be the ultimate goal. I'm putting this on my todo list for now
>> (including Steve's merging idea).
>
> In the perf's util/cs-etm.c file, it handles DISCONTINUITY with:
>
> case CS_ETM_DISCONTINUITY:
> /*
> * The trace is discontinuous, if the previous packet is
> * instruction packet, set flag PERF_IP_FLAG_TRACE_END
> * for previous packet.
> */
> if (prev_packet->sample_type == CS_ETM_RANGE)
> prev_packet->flags |= PERF_IP_FLAG_BRANCH |
> PERF_IP_FLAG_TRACE_END;
>
> I am wandering if OpenCSD has passed the correct info so Perf decoder can
> detect the discontinuity. If yes, then the flag 'PERF_IP_FLAG_TRACE_END' will
> be set (it is a general flag in branch sample), then we can consider use it in
> the python script to handle discontinuous data.
No OpenCSD isn't passing the correct info here. Higher up in the thread
I suggested an OpenCSD patch that makes it detect the error earlier and
fixes the issue. It also needs to output a discontinuity when the
address goes backwards. So two fixes and then the script works without
modifications.
>
>>
>> But in the mean time what about having a force option?
>>
>>> + if (stop_addr < start_addr):
>>> + if (options.verbose == True or not options.force):
>>> + print("Packet Dropped, Discontinuity detected
>>> [stop_add:0x%x start_addr:0x%x ] for dso %s" % (stop_addr, start_addr,
>>> dso))
>>> + if (not options.force):
>>> + return
>
> If the stop address is less than the start address, it must be something
> wrong. In this case, we can report a warning for discontinuity and directly
> return (also need to save the `addr` into global variable for next parsing).
>
> I prefer to not add force option for this case - eventually, this will consume
> much time for reporting this kind of failure and need to root causing it. A
> better way is we just print out the reasoning in the log and continue to dump.
But in this case we've identified all the known issues that would cause
the script to fail and we can fix them in Perf and OpenCSD. There may
not even be any more issues that will cause the script to fail in the
future so there's no point in softening the error IMO. That will only
hide future issues (of which there may be none) and make root causing
harder when it hits some other tool.
Changes in V5:
- In symbol-elf.c, branch to exit_close label if open file.
- In trace_event_python.c, correct indentation. set_sym_in_dict
call parameter "map_pgoff" renamed as "addr_map_pgoff" to
match local naming.
Changes in V4:
- In trace-event-python.c, fixed perf-tools-next merge problem.
Changes in V3:
- Rebased to linux-perf-tools branch.
- Squash symbol-elf.c and symbol.h into same commit.
- In map.c, merge dso__is_pie() call into existing if statement.
- In arm-cs-trace-disasm.py, remove debug artifacts.
Changes in V2:
- In dso__is_pie() (symbol-elf.c), Decrease indentation, add null pointer
checks per Leo Yan review.
- Updated mailing list distribution
Steve Clevenger (4):
Add dso__is_pie call to identify ELF PIE
Force MAPPING_TYPE__IDENTIY for PIE
Add map pgoff to python dictionary based on MAPPING_TYPE
Adjust objdump start/end range per map pgoff parameter
.../scripts/python/arm-cs-trace-disasm.py | 9 ++-
tools/perf/util/map.c | 4 +-
.../scripting-engines/trace-event-python.c | 13 +++-
tools/perf/util/symbol-elf.c | 61 +++++++++++++++++++
tools/perf/util/symbol.h | 1 +
5 files changed, 80 insertions(+), 8 deletions(-)
--
2.25.1