CoreSight

coresight@lists.linaro.org

7 participants
2372 discussions

Re: [PATCH 1/8] perf cs-etm: Don't flush when packet_queue fills up

by James Clark

On 11/09/2024 09:14, Leo Yan wrote: > > > On 9/10/2024 9:28 PM, Leo Yan wrote: >> On 9/5/2024 11:50 AM, James Clark wrote: >> >> [...] >> >>> cs_etm__flush(), like cs_etm__sample() is an operation that generates a >>> sample and then swaps the current with the previous packet. Calling >>> flush after processing the queues results in two swaps which corrupts >>> the next sample. Therefore it wasn't appropriate to call flush here so >>> remove it. >> >> In the cs_etm__sample(), if the period is not overflow, it is not necessarily >> to generate instruction samples and copy back stack entries. This is why we >> want to call cs_etm__flush() to make sure the last packet can be recorded >> properly for instruction sample with back stacks. >> >> We also need to take account into the case for the end of the session - in >> this case we need to generate samples for the last packet for complete info. >> >> I am wandering should we remove the cs_etm__packet_swap() from cs_etm__sample()? > > Sorry for typo. I meant to remove the cs_etm__packet_swap() from cs_etm__flush(). > > Thanks, > Leo Turns out there was already cs_etm__end_block() for the end of the session, but it was only called for the timeless modes. I added it for timestamped mode too in V2. I also kept the existing flush() function for discontinuities. I changed my mind that the differences to cs_etm__sample() weren't relevant. So I think we still need to keep the swap in flush() because otherwise the next sample won't start from the right place.

9 months, 1 week

[PATCH V8 0/4] arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset

by Steve Clevenger

Changes in V8: - in arm-cs-trace-disasm.py, ensure map_pgoff is not converted to string. - Remove map_pgoff integer conversion in dso not found print message. Changes in V7: - In arm-cs-trace-disasm.py, fix print message core dump resulting from mixed type arithmetic. - Modify CS_ETM_TRACE_ON filter to filter zero start_addr. The CS_ETM_TRACE_ON message is changed to print only in verbose mode. - Removed verbose mode only notification for start_addr/stop_addr outside of dso address range. Changes in V6: - In arm-cs-trace-disasm.py, zero map_pgoff for kernel files. Add map_pgoff to start/end address for dso not found message. - Added "Reviewed-by" trailer for patches 1-3 previously reviewed by Leo Yan in V4 and V5. 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 | 17 ++++-- 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, 86 insertions(+), 10 deletions(-) -- 2.44.0

9 months, 2 weeks

Re: [PATCH V7 4/4] Adjust objdump start/end range per map pgoff parameter

by Steve Clevenger

On 9/9/2024 1:33 PM, Leo Yan wrote: > On 9/7/2024 12:20 AM, Steve Clevenger wrote: > > [...] > >>>>> diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/ >>>>> perf/scripts/python/arm-cs-trace-disasm.py >>>>> index 7aff02d84ffb..a867e0db02b8 100755 >>>>> --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py >>>>> +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py >>>>> @@ -187,6 +187,7 @@ def process_event(param_dict): >>>>> dso_start = get_optional(param_dict, "dso_map_start") >>>>> dso_end = get_optional(param_dict, "dso_map_end") >>>>> symbol = get_optional(param_dict, "symbol") >>>>> + map_pgoff = get_optional(param_dict, "map_pgoff") >>>> >>>> >>>> I am concerned the two sentences below are inconsistence: one uses >>>> 'start_addr + map_pgoff' and the other uses 'start_addr + int(map_pgoff)'. >>>> >>> >>> Valid point. It's working fine as is, but how is it even working? I look >>> at print_disam/read_disasm, and no type conversion is done in either >>> call. The dso_start parameter for these calls is actually dso_vm_start >>> which is the dso_start integer conversion. > > I agreed with you. After reading code, I have same conclusion that we don't > need to type conversion to int type. > >> Python thinks the map_pgoff object is already an integer. For example, >> map_pgoff = get_optional(param_dict, "map_pgoff") >> print("%d" % map_pgoff.isdigit()) >> >> AttributeError: 'int' object has no attribute 'isdigit' >> Fatal Python error: handler_call die: problem in Python trace event handler >> >> Converting map_pgoff to a string works in the print statement. >> >> print("%d" % str(map_pgoff).isdigit()) >> 1 >> >> I'm not sure, but it's possible get_optional() during some call had >> converted it to '[unknown]' which would cause a problem. >> >> I can explicitly force to integer. > > For backward compatibility, we can use below code: > > map_pgoff = get_optional(param_dict, "map_pgoff") > if (map_pgoff == '[unknown]'): > map_pgoff = 0 > > Then in the later flow, we should can always use "map_pgoff" as an int type. > > I struggled for James reported issue. > > The variables “map_pgoff,” “dso_start,” and “dso_end” are set together in the > Python engine. All of them should be of type int if the DSO is found, or all > should be ‘[unknown]’ if the DSO is missing. We have checked the types for > “dso_start” and “dso_end”, and if either is ‘[unknown]’ the flow will directly > bail out. Thus, in theory, “map_pgoff” will not cause trouble if it is an > ‘[unknown]’ string. > > One possibility is that James has applied your patches but has not built perf. > So, the field “map_pgoff” is not passed from the Python engine, which will > cause the reported error. I think the proposed above change can effectively > avoid the error. > > Thanks, > Leo I added that exact'[unknown]' check in last week. There's no need to explicitly convert to an integer although Python doesn't complain when about integer to integer conversions. It's probably just a no op. I'll commit the change later today. Steve

9 months, 2 weeks

Re: [PATCH V7 4/4] Adjust objdump start/end range per map pgoff parameter

by Steve Clevenger

On 9/6/2024 4:27 AM, Leo Yan wrote: > > > On 9/5/24 23:28, Steve Clevenger wrote: >> >> Extract map_pgoff parameter from the dictionary, and adjust start/end >> range passed to objdump based on the value. >> >> A zero start_addr is filtered to prevent output of dso address range >> check failures. This script repeatedly sees a zero value passed >> in for >> start_addr = cpu_data[str(cpu) + 'addr'] >> >> These zero values are not a new problem. The start_addr/stop_addr warning >> clutters the instruction trace output, hence this change. >> >> Signed-off-by: Steve Clevenger <scclevenger(a)os.amperecomputing.com> >> --- >> tools/perf/scripts/python/arm-cs-trace-disasm.py | 14 +++++++++----- >> 1 file changed, 9 insertions(+), 5 deletions(-) >> >> diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/ >> perf/scripts/python/arm-cs-trace-disasm.py >> index 7aff02d84ffb..a867e0db02b8 100755 >> --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py >> +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py >> @@ -187,6 +187,7 @@ def process_event(param_dict): >> dso_start = get_optional(param_dict, "dso_map_start") >> dso_end = get_optional(param_dict, "dso_map_end") >> symbol = get_optional(param_dict, "symbol") >> + map_pgoff = get_optional(param_dict, "map_pgoff") > > > I am concerned the two sentences below are inconsistence: one uses > 'start_addr + map_pgoff' and the other uses 'start_addr + int(map_pgoff)'. > Valid point. It's working fine as is, but how is it even working? I look at print_disam/read_disasm, and no type conversion is done in either call. The dso_start parameter for these calls is actually dso_vm_start which is the dso_start integer conversion. > Here about below code? > > map_pgoff_str = get_optional(param_dict, "map_pgoff") > if map_pgoff_str == "": > map_pgoff = 0 > else: > map_pgoff = int(map_pgoff_str) > > With above change, 'map_pgoff' is an int type. As a result, the changes > below can simply add 'map_pgoff'. I propose: map_pgoff_str = get_optional(param_dict, "map_pgoff" if (map_pgoff_str.isdigit()): map_pgoff = int(map_pgoff) else: map_pgoff = 0 The int() conversions in the print() statement can then be removed. Steve > With these changes, LGTM: > > Reviewed-by: Leo Yan <leo.yan(a)arm.com> > > >> cpu = sample["cpu"] >> ip = sample["ip"] >> @@ -243,9 +244,11 @@ def process_event(param_dict): >> # Record for previous sample packet >> cpu_data[str(cpu) + 'addr'] = addr >> >> - # Handle CS_ETM_TRACE_ON packet if start_addr=0 and stop_addr=4 >> - if (start_addr == 0 and stop_addr == 4): >> - print("CPU%d: CS_ETM_TRACE_ON packet is inserted" % cpu) >> + # Filter out zero start_address. Optionally identify >> CS_ETM_TRACE_ON packet >> + # if start_addr=0 and stop_addr=4. >> + if (start_addr == 0): >> + if ((stop_addr == 4) and (options.verbose == True)): >> + print("CPU%d: CS_ETM_TRACE_ON packet is >> inserted" % cpu) >> return >> >> if (start_addr < int(dso_start) or start_addr > int(dso_end)): >> @@ -262,13 +265,14 @@ def process_event(param_dict): >> # vm_start to zero. >> if (dso == "[kernel.kallsyms]" or dso_start == >> 0x400000): >> dso_vm_start = 0 >> + map_pgoff = 0 >> else: >> dso_vm_start = int(dso_start) >> >> dso_fname = get_dso_file_path(dso, dso_bid) >> if path.exists(dso_fname): >> - print_disam(dso_fname, dso_vm_start, >> start_addr, stop_addr) >> + print_disam(dso_fname, dso_vm_start, >> start_addr + map_pgoff, stop_addr + map_pgoff) >> else: >> - print("Failed to find dso %s for address range >> [ 0x%x .. 0x%x ]" % (dso, start_addr, stop_addr)) >> + print("Failed to find dso %s for address range >> [ 0x%x .. 0x%x ]" % (dso, start_addr + int(map_pgoff), stop_addr + >> int(map_pgoff))) > >> >> print_srccode(comm, param_dict, sample, symbol, dso) >> -- >> 2.44.0 >>

9 months, 2 weeks

[PATCH V7 0/4] arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset

by Steve Clevenger

Changes in V7: - In arm-cs-trace-disasm.py, fix print message core dump resulting from mixed type arithmetic. - Modify CS_ETM_TRACE_ON filter to filter zero start_addr. The CS_ETM_TRACE_ON message is changed to print only in verbose mode. - Removed verbose mode only notification for start_addr/stop_addr outside of dso address range. Changes in V6: - In arm-cs-trace-disasm.py, zero map_pgoff for kernel files. Add map_pgoff to start/end address for dso not found message. - Added "Reviewed-by" trailer for patches 1-3 previously reviewed by Leo Yan in V4 and V5. 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 | 14 +++-- 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, 83 insertions(+), 10 deletions(-) -- 2.44.0

9 months, 2 weeks

[PATCH V6 0/4] arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset

by Steve Clevenger

Changes in V6: - In arm-cs-trace-disasm.py, zero map_pgoff for kernel files. Add map_pgoff to start/end address for dso not found message. - Added "Reviewed-by" trailer for patches 1-3 previously reviewed by Leo Yan in V4 and V5. 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 | 12 ++-- 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, 82 insertions(+), 9 deletions(-) -- 2.44.0

9 months, 2 weeks

[PATCH 0/8] perf: cs-etm: Coresight decode and disassembly improvements

by James Clark

A set of changes that came out of the issues reported here [1]. * First 3 patches fix a decode bug in Perf and add support for new consistency checks in OpenCSD * The remaining ones make the disassembly script easier to test and use. This also involves adding a new Python binding to Perf to get a config value (perf_config_get()) [1]: https://lore.kernel.org/linux-arm-kernel/20240719092619.274730-1-gankulkarn… James Clark (8): perf cs-etm: Don't flush when packet_queue fills up perf cs-etm: Use new OpenCSD consistency checks perf cs-etm: Remove cs_etm__flush() perf scripting python: Add function to get a config value perf scripts python cs-etm: Update to use argparse perf scripts python cs-etm: Improve arguments perf scripts python cs-etm: Add start and stop arguments perf test: cs-etm: Test Coresight disassembly script .../perf/Documentation/perf-script-python.txt | 2 +- .../scripts/python/Perf-Trace-Util/Context.c | 11 ++ .../scripts/python/arm-cs-trace-disasm.py | 109 +++++++++++++----- .../tests/shell/test_arm_coresight_disasm.sh | 63 ++++++++++ tools/perf/util/config.c | 22 ++++ tools/perf/util/config.h | 1 + .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 7 +- tools/perf/util/cs-etm.c | 68 +---------- 8 files changed, 192 insertions(+), 91 deletions(-) create mode 100755 tools/perf/tests/shell/test_arm_coresight_disasm.sh -- 2.34.1

9 months, 2 weeks

[PATCH v6 00/17] coresight: Use per-sink trace ID maps for Perf sessions

by James Clark

This will allow sessions with more than CORESIGHT_TRACE_IDS_MAX ETMs as long as there are fewer than that many ETMs connected to each sink. Each sink owns its own trace ID map, and any Perf session connecting to that sink will allocate from it, even if the sink is currently in use by other users. This is similar to the existing behavior where the dynamic trace IDs are constant as long as there is any concurrent Perf session active. It's not completely optimal because slightly more IDs will be used than necessary, but the optimal solution involves tracking the PIDs of each session and allocating ID maps based on the session owner. This is difficult to do with the combination of per-thread and per-cpu modes and some scheduling issues. The complexity of this isn't likely to worth it because even with multiple users they'd just see a difference in the ordering of ID allocations rather than hitting any limits (unless the hardware does have too many ETMs connected to one sink). Per-thread mode works but only until there are any overlapping IDs, at which point Perf will error out. Both per-thread mode and sysfs mode are left to future changes, but both can be added on top of this initial implementation and only sysfs mode requires further driver changes. The HW_ID version field hasn't been bumped in order to not break Perf which already has an error condition for other values of that field. Instead a new minor version has been added which signifies that there are new fields but the old fields are backwards compatible. Changes since v5: * Hide queue number printout behind -v option * Style change in cs_etm__process_aux_output_hw_id() * Move new format enum to an earlier commit to reduce churn Changes since v4: * Fix compilation failure when TRACE_ID_DEBUG is set * Expand comment about not freeing individual trace IDs in free_event_data() Changes since v3: * Fix issue where trace IDs were overwritten by possibly invalid ones by Perf in unformatted mode. Now the HW_IDs are also used for unformatted mode unless the kernel didn't emit any. * Add a commit to check the OpenCSD version. * Add a commit to not save invalid IDs in the Perf header. * Replace cs_etm_queue's formatted and formatted_set members with a single enum which is easier to use. * Drop CORESIGHT_TRACE_ID_UNUSED_FLAG as it's no longer needed. * Add a commit to print the queue number in the raw dump. * Don't assert on the number of unformatted decoders if decoders == 0. Changes since v2: * Rebase on coresight-next 6.10-rc2 (b9b25c8496). * Fix double free of csdev if device registration fails. * Fix leak of coresight_trace_id_perf_start() if trace ID allocation fails. * Don't resend HW_ID for sink changes in per-thread mode. The existing CPU field on AUX records can be used to track this instead. * Tidy function doc for coresight_trace_id_release_all() * Drop first two commits now that they are in coresight-next * Add a commit to make the trace ID spinlock local to the map Changes since V1: * Rename coresight_device.perf_id_map to perf_sink_id_map. * Instead of outputting a HW_ID for each reachable ETM, output the sink ID and continue to output only the HW_ID once for each mapping. * Keep the first two Perf patches so that it applies cleanly on coresight-next, although they have been applied on perf-tools-next * Add new *_map() functions to the trace ID public API instead of modifying existing ones. * Collapse "coresight: Pass trace ID map into source enable" into "coresight: Use per-sink trace ID maps for Perf sessions" because the first commit relied on the default map being accessible which is no longer necessary due to the previous bullet point. James Clark (17): perf: cs-etm: Create decoders after both AUX and HW_ID search passes perf: cs-etm: Allocate queues for all CPUs perf: cs-etm: Move traceid_list to each queue perf: cs-etm: Create decoders based on the trace ID mappings perf: cs-etm: Only save valid trace IDs into files perf: cs-etm: Support version 0.1 of HW_ID packets perf: cs-etm: Print queue number in raw trace dump perf: cs-etm: Add runtime version check for OpenCSD 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 drivers/hwtracing/coresight/coresight-core.c | 37 +- drivers/hwtracing/coresight/coresight-dummy.c | 3 +- .../hwtracing/coresight/coresight-etm-perf.c | 43 +- .../hwtracing/coresight/coresight-etm-perf.h | 18 - .../coresight/coresight-etm3x-core.c | 9 +- .../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 +- .../hwtracing/coresight/coresight-tmc-etr.c | 5 +- drivers/hwtracing/coresight/coresight-tmc.h | 5 +- drivers/hwtracing/coresight/coresight-tpdm.c | 3 +- .../hwtracing/coresight/coresight-trace-id.c | 138 ++-- .../hwtracing/coresight/coresight-trace-id.h | 70 +- include/linux/coresight-pmu.h | 17 +- include/linux/coresight.h | 21 +- tools/build/feature/test-libopencsd.c | 4 +- tools/include/linux/coresight-pmu.h | 17 +- tools/perf/Makefile.config | 2 +- tools/perf/arch/arm/util/cs-etm.c | 11 +- .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 49 +- .../perf/util/cs-etm-decoder/cs-etm-decoder.h | 3 +- .../util/cs-etm-decoder/cs-etm-min-version.h | 13 + tools/perf/util/cs-etm.c | 629 +++++++++++------- tools/perf/util/cs-etm.h | 12 +- 25 files changed, 650 insertions(+), 475 deletions(-) create mode 100644 tools/perf/util/cs-etm-decoder/cs-etm-min-version.h -- 2.34.1

9 months, 2 weeks

Re: [GIT PULL] coresight: Updates for Linux v6.12

by Suzuki K Poulose

On 03/09/2024 11:41, Greg KH wrote: > On Tue, Sep 03, 2024 at 11:36:03AM +0100, Suzuki K Poulose wrote: >> Hi Greg, >> >> On 27/08/2024 20:26, Suzuki K Poulose wrote: >>> >>> Greg, >>> >>> Please find the updates for CoreSight/hwtracing subsystem for v6.12. >> >> Gentle ping, in case this was missed out. > > I just pulled this, right? Indeed you did, thanks ! Suzuki "beaten by Greg" ;-) > > thanks, > > greg "drowning in patches" k-h

9 months, 3 weeks

Re: [GIT PULL] coresight: Updates for Linux v6.12

by Suzuki K Poulose

On 03/09/2024 11:07, Greg KH wrote: > On Tue, Aug 27, 2024 at 08:26:44PM +0100, Suzuki K Poulose wrote: >> >> 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 >> > > Pulled and pushed out, thanks. > Thanks Greg ! Suzuki

9 months, 3 weeks

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight