On 29/08/2023 05:44, Steve Clevenger wrote:
>
> Ampere has been using the following command sequence to generate a 'perf
> record' kernel instruction trace on an AmpereOne AARCH64 based systems
> using a 6.3.0 Fedora distribution with CoreSight related patches.
>
>
> [root@sut01sys-b212 kernel]# uname -a
> Linux sut01sys-b212.scc-lab.amperecomputing.com 6.3.0-coresight-enabled+
> #9 SMP PREEMPT_DYNAMIC Fri Jun 30 12:54:14 PDT 2023 aarch64 aarch64
> aarch64 GNU/Linux
> [root@sut01sys-b212 kernel]#
> [root@sut01sys-b212 kernel]# timeout 45s perf record --kcore -o kcore -e
> cs_etm/@tmc_etr63/k --per-thread taskset --cpu-list 15 dd if=/dev/zero
> of=/dev/null
> [ perf record: Woken up 41 times to write data ]
> [ perf record: Captured and wrote 144.056 MB kcore ]
> [root@sut01sys-b212 kernel]#
> [root@sut01sys-b212 kernel]# ls -l ~/linux/vmlinux
> -rwxr-xr-x. 1 root root 390426912 Jun 30 12:54 /home/stevec/linux/vmlinux*
> [root@sut01sys-b212 kernel]#
> [root@sut01sys-b212 kernel]# timeout 45s perf script --input
> ./kcore/data -s ../../scripts/arm-cs-trace-disasm.py -F
> cpu,event,ip,addr,sym – -d objdump -k ~/linux/vmlinux > ./perf.itrace
> [root@sut01sys-b212 kernel]#
> [root@sut01sys-b212 kernel]# ls -l perf.itrace
> -rw-r--r--. 1 root root 485834744 Jul 17 14:18 perf.itrace
>
>
> The executive summary is the code sections in vmlinux do not match the
> actual code executing on the target due to self-modifying code
> (alternate code sequences) in the kernel image. These sequences are
> typically applied over nop place-holder instructions. This was verified
> by comparing a location in the ./drivers/char/mem.c read_zero objdump
> instruction stream to the memory resident instructions captured with the
> TRACE32 ETM instruction trace feature. This particular instruction trace
> example shows a disconnect. It displays a ‘b ffff800008ab8a70
> <read_zero+0x168>’ instruction at address 0xffff800008ab89e8 which did
> not branch. The actual instruction present at 0xffff800008ab89e8 is a ‘nop’.
> .
> .
> ffff800008ab89a8: 1400000d b ffff800008ab89dc
> <read_zero+0xd4>
> dd 8774/8774 [0015] 0.000000000 read_zero+0xa0
> ffff800008ab89dc <read_zero+0xd4>:
> ffff800008ab89dc: 9248f840 and x0, x2,
> #0xff7fffffffffffff
> ffff800008ab89e0: aa1403e1 mov x1, x20
> ffff800008ab89e4: 9418b6fb bl ffff8000090e65d0
> <__arch_clear_user>
> dd 8774/8774 [0015] 0.000000000 read_zero+0xdc
> ffff8000090e65d0 <__arch_clear_user>:
> ffff8000090e65d0: d503245f bti c
> ffff8000090e65d4: 8b010002 add x2, x0, x1
> ffff8000090e65d8: f1002021 subs x1, x1, #0x8
> ffff8000090e65dc: 54000104 b.mi ffff8000090e65fc
> <__arch_clear_user+0x2c> // b.first
> ffff8000090e65e0: f800081f sttr xzr, [x0]
> ffff8000090e65e4: 91002000 add x0, x0, #0x8
> ffff8000090e65e8: f1002021 subs x1, x1, #0x8
> ffff8000090e65ec: 54ffffa8 b.hi ffff8000090e65e0
> <__arch_clear_user+0x10> // b.pmore
> dd 8774/8774 [0015] 0.000000000
> __arch_clear_user+0x1c
> ...vec/linux/arch/arm64/lib/clear_user.S 31 b.hi 1b
> ffff8000090e65e0 <__arch_clear_user+0x10>:
> ffff8000090e65e0: f800081f sttr xzr, [x0]
> ffff8000090e65e4: 91002000 add x0, x0, #0x8
> ffff8000090e65e8: f1002021 subs x1, x1, #0x8
> ffff8000090e65ec: 54ffffa8 b.hi ffff8000090e65e0
> <__arch_clear_user+0x10> // b.pmore
> .
> .
> ffff8000090e65f0: f81f885f sttr xzr, [x2, #-8]
> ffff8000090e65f4: d2800000 mov x0, #0x0
> // #0
> ffff8000090e65f8: d65f03c0 ret
> dd 8774/8774 [0015] 0.000000000
> __arch_clear_user+0x28
> ...vec/linux/arch/arm64/lib/clear_user.S 34 ret
> ffff800008ab89e8 <read_zero+0xe0>:
> ffff800008ab89e8: 14000022 b ffff800008ab8a70
> <read_zero+0x168>
> ffff800008ab89ec: d503201f nop
> dd 8774/8774 [0015] 0.000000000 read_zero+0xe4
> /home/stevec/linux/drivers/char/mem.c 521
> left = clear_user(buf + cleared, chunk);
> ffff800008ab8a18 <read_zero+0x110>:
> ffff800008ab8a18: 8b14035a add x26, x26, x20
> ffff800008ab8a1c: b5000360 cbnz x0,
> ffff800008ab8a88 <read_zero+0x180>
> ffff800008ab8a20: f9400320 ldr x0, [x25]
> ffff800008ab8a24: cb140273 sub x19, x19, x20
> .
> .
>
> Ostensibly, the ‘perf record –kcore’ option produces a representative
> image of the kernel. But this option does not produce suitable output to
> generate ARM64 kernel instruction trace. perf doesn’t disassemble ARM64,
> so the arm-cs-trace-disasm.py python script is used with the native
> objdump utility to provide ARM64 disassembly from CoreSight trace
> capture. objdump itself requires an ELF/DWARF file image + symbols to
> generate the symbolic (+ line information for mixed source) disassembly.
> The linux vmlinux image + symbols file is typically used for this
> purpose. The kallsyms file is not formatted for objdump use. As an
> experiment, I patched the executable code sections in a local copy of
> vmlinux with the corresponding executable code segments extracted from
> the kcore image using an Ampere internal ELF patch utility.
>
> This patch utility leverages the (MIT Licensed) ELFIO open source
> library API. These were the commands.
>
>
> [root@sut01sys-b212 kernel]# timeout 30s perf record --kcore -o kcore -e
> cs_etm/@tmc_etr63/k --per-thread taskset --cpu-list 15 dd if=/dev/zero
> of=/dev/null
> [ perf record: Woken up 25 times to write data ]
> [ perf record: Captured and wrote 88.053 MB kcore ]
> [root@sut01sys-b212 kernel]#
> [root@sut01sys-b212 kernel]# ls -l ~/linux/vmlinux
> -rwxr-xr-x. 1 root root 390426912 Jun 30 12:54 /home/stevec/linux/vmlinux*
> [root@sut01sys-b212 kernel]# cp ~/linux/vmlinux .
> [root@sut01sys-b212 kernel]# patch-elf --help
>
> patch-elf overlays the kernel image from a local copy of
> '/proc/kcore' to the corresponding (by address) ELF sections
> in a local copy of the vmlinux ELF file.
> A local '/proc/kcore' is created by:
> 'perf report --kcore -e cs_etm/@tmc_etr1/k ...'
> The local (patched) vmlinux copy is used by:
> 'perf script -s arm-cs-trace-disasm.py ...'
> See the CoreSight Hardware-Assisted Trace Application Note for
> use of the 'perf report' and 'perf script' commands.
>
> Usage: patch-elf <--verbose> kcore_file vmlinux_file
> [root@sut01sys-b212 kernel]#
> [root@sut01sys-b212 kernel]# patch-elf kcore/kcore_dir/kcore ./vmlinux
> ELF File kcore Properties
> ELF file class: ELF64
> ELF file encoding: Little endian
> Machine: ARM AArch64
> Type: Core file
> Number of segments: 3
> Number of sections: 0
>
> ELF File vmlinux Properties
> ELF file class: ELF64
> ELF file encoding: Little endian
> Machine: ARM AArch64
> Type: Shared object file
> Number of segments: 3
> Number of sections: 43
>
> Patching section[ 2] ffff800008010000 17997936 bytes
>
> Patching section[15] ffff800009a31000 20480 bytes
>
> Patching section[16] ffff800009a40000 612372 bytes
>
> Patching section[17] ffff800009ad5818 24752 bytes
>
> [root@sut01sys-b212 kernel]# ls -l ./vmlinux
> -rwxr-xr-x. 1 root root 390426908 Jul 19 11:14 ./vmlinux*
> [root@sut01sys-b212 kernel]#
> [root@sut01sys-b212 kernel]# timeout 45s perf script --input
> ./kcore/data -s ../../scripts/arm-cs-trace-disasm.py -F
> cpu,event,ip,addr,sym – -d objdump -k ./vmlinux > ./perf.itrace
> [root@sut01sys-b212 kernel]#
> [root@sut01sys-b212 kernel]# ls -l perf.itrace
> -rw-r--r--. 1 root root 32142060 Jul 19 11:18 perf.itrace
>
> Here is the representative kernel instruction trace using a patched vmlinux.
>
> .
> .
> ffff800008ab89a8: 1400000d b ffff800008ab89dc
> <read_zero+0xd4>
> dd 8774/8774 [0015] 0.000000000 read_zero+0xa0
> ffff800008ab89dc <read_zero+0xd4>:
> ffff800008ab89dc: 9248f840 and x0, x2,
> #0xff7fffffffffffff
> ffff800008ab89e0: aa1403e1 mov x1, x20
> ffff800008ab89e4: 9418b6fb bl ffff8000090e65d0
> <__arch_clear_user>
> dd 8774/8774 [0015] 0.000000000 read_zero+0xdc
> ffff8000090e65d0 <__arch_clear_user>:
> ffff8000090e65d0: d503245f bti c
> ffff8000090e65d4: 8b010002 add x2, x0, x1
> ffff8000090e65d8: f1002021 subs x1, x1, #0x8
> ffff8000090e65dc: 54000104 b.mi ffff8000090e65fc
> <__arch_clear_user+0x2c> // b.first
> ffff8000090e65e0: f800081f sttr xzr, [x0]
> ffff8000090e65e4: 91002000 add x0, x0, #0x8
> ffff8000090e65e8: f1002021 subs x1, x1, #0x8
> ffff8000090e65ec: 54ffffa8 b.hi ffff8000090e65e0
> <__arch_clear_user+0x10> // b.pmore
> dd 8774/8774 [0015] 0.000000000
> __arch_clear_user+0x1c
> ffff8000090e65e0 <__arch_clear_user+0x10>:
> ffff8000090e65e0: f800081f sttr xzr, [x0]
> ffff8000090e65e4: 91002000 add x0, x0, #0x8
> ffff8000090e65e8: f1002021 subs x1, x1, #0x8
> ffff8000090e65ec: 54ffffa8 b.hi ffff8000090e65e0
> <__arch_clear_user+0x10> // b.pmore
> .
> .
> ffff8000090e65f0: f81f885f sttr xzr, [x2, #-8]
> ffff8000090e65f4: d2800000 mov x0, #0x0
> // #0
> ffff8000090e65f8: d65f03c0 ret
> dd 8774/8774 [0015] 0.000000000
> __arch_clear_user+0x28
> ...vec/linux/arch/arm64/lib/clear_user.S 34 ret
> ffff800008ab89e8 <read_zero+0xe0>:
> ffff800008ab89e8: d503201f nop
> ffff800008ab89ec: 1400000b b ffff800008ab8a18
> <read_zero+0x110>
> dd 8774/8774 [0015] 0.000000000 read_zero+0xe4
> /home/stevec/linux/drivers/char/mem.c 521
> left = clear_user(buf + cleared, chunk);
> ffff800008ab8a18 <read_zero+0x110>:
> ffff800008ab8a18: 8b14035a add x26, x26, x20
> ffff800008ab8a1c: b5000360 cbnz x0,
> ffff800008ab8a88 <read_zero+0x180>
> ffff800008ab8a20: f9400320 ldr x0, [x25]
> ffff800008ab8a24: cb140273 sub x19, x19, x20
> .
> .
> .
>
> This begs the question what perf enhancements could be added to make
> ARM64 kernel instruction trace easier to use? The process I’ve followed
> is cumbersome, but could be done behind the scenes by perf. The caveat
> is it requires a vmlinux which might not be available to an end user.
> Here are 2 options.
>
> 1. 'perf report -kcore'could use the process I’ve used here
> transparently in the background. The plus side is the objdump feature of
> mixed disassembly is available based on the current vmlinux.
>
Hi Steve,
What you're saying makes sense to me. I think #1 sounds best, I'm not
sure of the use-case where you wanted to make actionable decisions from
the trace but wouldn't have vmlinux available? Maybe an end-user could
be missing it, but I can only imagine use cases where you are actively
building and developing the kernel.
> 2. 'perf report -kcore' generates an ELF + symbols file based on
> kallsyms (and/or System.map). No vmlinux patching, so intermixed source
> and disassembly wouldn’t be available. It’s a reasonable alternative
> without relying on vmlinux.
>
> It makes sense performance-wise to use an ARM64 disassembler directly
> through perf. perf-script use of the arm-cs-trace-disasm.py python
> script can be slow. I’m unfamiliar with the Intel implementation, but
> perf-annotate uses objdump. Unfortunately, I can't seem to get annotate
> to work for me. A patch operation is still be required if vmlinux is used.
>
We have had other reports about arm-cs-trace-disasm.py being slow. I had
a plan to make it work with disassembly directly from Perf as that is
available in other modes. For example the annotate mode in perf already
works on Arm. What error are you getting with it exactly? It might
require installation of the aarch64 objdump tool if you are running on
x86, or provide the --objdump option?
> I suspect the CoreSight/perf communities are aware of these issues.
> Is there any ongoing work not known to the outside world?
>
> Thanks and regards,
> Steve C.
On 28/08/2023 17:30, Will Deacon wrote:
> On Mon, Aug 28, 2023 at 08:11:41AM +0530, Anshuman Khandual wrote:
>>
>>
>> On 8/18/23 23:34, Will Deacon wrote:
>>> On Thu, 17 Aug 2023 11:24:01 +0530, Anshuman Khandual wrote:
>>>> This series enables detection of ACPI based TRBE devices via a stand alone
>>>> purpose built representative platform device. But as a pre-requisite this
>>>> changes coresight_platform_data structure assignment for the TRBE device.
>>>>
>>>> This series is based on v6.5-rc5 kernel, is also dependent on the following
>>>> EDK2 changes posted earlier by Sami.
>>>>
>>>> [...]
>>>
>>> Applied to will (for-next/perf), thanks!
>>>
>>> [1/4] arm_pmu: acpi: Refactor arm_spe_acpi_register_device()
>>> https://git.kernel.org/will/c/81e5ee471609
>>> [2/4] arm_pmu: acpi: Add a representative platform device for TRBE
>>> https://git.kernel.org/will/c/1aa3d0274a4a
>>
>> It seems like the above two changes are still going in for 6.6-rc1 ? I could
>> see these in arm64/for-next/core and latest linux-next next-20230825.
>
> Yes, as I said, I only dropped the coresight bits.
Ok, then I can pick up coresight changes via my tree and push it to Greg.
Suzuki
>
> Will
On 28/08/2023 22:35, Will Deacon wrote:
> On Sun, Aug 27, 2023 at 11:11:16PM +0100, Suzuki K Poulose wrote:
>> On 21/08/2023 12:28, Will Deacon wrote:
>>> Hmm, the rationale behind your change to make the pdata allocation
>>> per-device in ("coresight: trbe: Allocate platform data per device")
>>> confuses me: with Anshuman's change to allocate the pdata using
>>> devm_kzalloc(), there shouldn't be any connections for the coresight
>>> core to trip over, should there?
>>
>> Anshuman's patch is working around the problem of "TRBE platform
>> device with ACPI doesn't have a valid companion device" - this is a problem
>> for the acpi_get_coresight_platform_data(). The work
>> around is to move the "allocation" from coresight_get_platform_data()
>> to the driver (given we don't need anything else from the ACPI except
>> the IRQ). That doesn't change *how* it is allocated.
>> Also please note that, the TRBE driver creates a TRBE coresight_device
>> per-CPU and the platform data is shared by all of these devices, which
>> the coresight core driver doesn't cope with. The other option is to
>> move the releasing of these platform-data to the individual drivers,
>> which is quite an invasive change. Or, make the core driver tolerate
>> a NULL platform data, which is also again invasive. So the merged fix
>> is correct and is still valid after this patch.
>
> Ah, ok, so the problem with TRBE isn't anything to do with its connections,
> but simply because the pdata is shared?
Correct, thats an issue outside of the ACPI support. With ACPI, the
coresight_get_platform_data() can't cope with the TRBE with missing
companion device, that is fixed by Anshuman's patch in this series.
Also, please could you confirm the plan forward for merging this
(next version of course) ?
Cheers
Suzuki
>
> Will
This series makes ETM TRCCCCTRL based 'cc_threshold' user configurable via
the perf event attribute. But first, this implements an errata work around
affecting ETM TRCIDR3.CCITMIN value on certain cpus, overriding the field.
This series applies on v6.5-rc7.
Cc: Catalin Marinas <catalin.marinas(a)arm.com>
Cc: Will Deacon <will(a)kernel.org>
Cc: Suzuki K Poulose <suzuki.poulose(a)arm.com>
Cc: Mike Leach <mike.leach(a)linaro.org>
Cc: James Clark <james.clark(a)arm.com>
Cc: Leo Yan <leo.yan(a)linaro.org>
Cc: Jonathan Corbet <corbet(a)lwn.net>
Cc: linux-doc(a)vger.kernel.org
Cc: coresight(a)lists.linaro.org
Cc: linux-arm-kernel(a)lists.infradead.org
Cc: linux-kernel(a)vger.kernel.org
Changes in V5:
- Replaced 'where as' with single word 'whereas'
- Reworked 'cc_threshold' fallback to ETM_CYC_THRESHOLD_DEFAULT
Changes in V4:
https://lore.kernel.org/all/20230818112051.594986-1-anshuman.khandual@arm.c…
- Fixed a typo s/rangess/ranges,
- Renamed etm4_work_around_wrong_ccitmin() as etm4_core_reads_wrong_ccitmin()
- Moved drvdata->ccitmin value check for 256 inside etm4_core_reads_wrong_ccitmin()
- Moved the comment inside etm4_core_reads_wrong_ccitmin()
Changes in V3:
https://lore.kernel.org/all/20230811034600.944386-1-anshuman.khandual@arm.c…
- Added errata work around affecting TRCIDR3.CCITMIN
- Split the document update into a separate patch
Changes in V2:
https://lore.kernel.org/all/20230808074533.380537-1-anshuman.khandual@arm.c…
- s/treshhold/threshold
Changes in V1:
https://lore.kernel.org/all/20230804044720.1478900-1-anshuman.khandual@arm.…
Anshuman Khandual (3):
coresight: etm: Override TRCIDR3.CCITMIN on errata affected cpus
coresight: etm: Make cycle count threshold user configurable
Documentation: coresight: Add cc_threshold tunable
Documentation/arch/arm64/silicon-errata.rst | 10 +++++
Documentation/trace/coresight/coresight.rst | 4 ++
.../hwtracing/coresight/coresight-etm-perf.c | 2 +
.../coresight/coresight-etm4x-core.c | 45 ++++++++++++++++++-
4 files changed, 59 insertions(+), 2 deletions(-)
--
2.25.1
On 8/18/23 23:34, Will Deacon wrote:
> On Thu, 17 Aug 2023 11:24:01 +0530, Anshuman Khandual wrote:
>> This series enables detection of ACPI based TRBE devices via a stand alone
>> purpose built representative platform device. But as a pre-requisite this
>> changes coresight_platform_data structure assignment for the TRBE device.
>>
>> This series is based on v6.5-rc5 kernel, is also dependent on the following
>> EDK2 changes posted earlier by Sami.
>>
>> [...]
>
> Applied to will (for-next/perf), thanks!
>
> [1/4] arm_pmu: acpi: Refactor arm_spe_acpi_register_device()
> https://git.kernel.org/will/c/81e5ee471609
> [2/4] arm_pmu: acpi: Add a representative platform device for TRBE
> https://git.kernel.org/will/c/1aa3d0274a4a
It seems like the above two changes are still going in for 6.6-rc1 ? I could
see these in arm64/for-next/core and latest linux-next next-20230825.
> [3/4] coresight: trbe: Add a representative coresight_platform_data for TRBE
> https://git.kernel.org/will/c/e926b8e9eb40
> [4/4] coresight: trbe: Enable ACPI based TRBE devices
> https://git.kernel.org/will/c/0fb93c5ede13
>
> Cheers,
On 21/08/2023 12:28, Will Deacon wrote:
> On Sat, Aug 19, 2023 at 08:36:28AM +0100, Suzuki K Poulose wrote:
>> On 18/08/2023 19:04, Will Deacon wrote:
>>> On Thu, 17 Aug 2023 11:24:01 +0530, Anshuman Khandual wrote:
>>>> This series enables detection of ACPI based TRBE devices via a stand alone
>>>> purpose built representative platform device. But as a pre-requisite this
>>>> changes coresight_platform_data structure assignment for the TRBE device.
>>>>
>>>> This series is based on v6.5-rc5 kernel, is also dependent on the following
>>>> EDK2 changes posted earlier by Sami.
>>>>
>>>> [...]
>>>
>>> Applied to will (for-next/perf), thanks!
>>>
>>> [1/4] arm_pmu: acpi: Refactor arm_spe_acpi_register_device()
>>> https://git.kernel.org/will/c/81e5ee471609
>>> [2/4] arm_pmu: acpi: Add a representative platform device for TRBE
>>> https://git.kernel.org/will/c/1aa3d0274a4a
>>> [3/4] coresight: trbe: Add a representative coresight_platform_data for TRBE
>>> https://git.kernel.org/will/c/e926b8e9eb40
>>
>> This will conflict with what I have (already) sent to Greg for
>> coresight/next. Please let me know how you would like handle it
>
> Hmm, the rationale behind your change to make the pdata allocation
> per-device in ("coresight: trbe: Allocate platform data per device")
> confuses me: with Anshuman's change to allocate the pdata using
> devm_kzalloc(), there shouldn't be any connections for the coresight
> core to trip over, should there?
Anshuman's patch is working around the problem of "TRBE platform
device with ACPI doesn't have a valid companion device" - this is a
problem for the acpi_get_coresight_platform_data(). The work
around is to move the "allocation" from coresight_get_platform_data()
to the driver (given we don't need anything else from the ACPI except
the IRQ). That doesn't change *how* it is allocated.
Also please note that, the TRBE driver creates a TRBE coresight_device
per-CPU and the platform data is shared by all of these devices, which
the coresight core driver doesn't cope with. The other option is to
move the releasing of these platform-data to the individual drivers,
which is quite an invasive change. Or, make the core driver tolerate
a NULL platform data, which is also again invasive. So the merged fix
is correct and is still valid after this patch.
>
> It would've been nice to know about the conflict earlier, but since I
> think you're away this week and we're likely to hit the merge window
> next week, I'm going to drop the coresight patches for now.
Apologies, I was expecting to queue the changes via coresight tree,
given how it was affecting the tree and was awaiting your Ack. However
I didn't confirm it on the list, which is my mistake.
The other problem was reported and the fix eventually had to
conflict with Anshuman's series, which he was made aware of.
Given, your Ack was missing I hoping that Anshuman could respin
the series with your Ack on top of the fix and eventually queue
that via my tree.
Suzuki
>
> Will
Hi Yabin
On 26/08/2023 01:30, Yabin Cui wrote:
> Hi Suzuki,
>
> When I saw the crash, the kernel log stopped immediately before "devarch
> = read_etm4x_sysreg_const_offset(TRCDEVARCH);".
> And then the device rebooted. The reason for the reboot is watchdog crash.
Isn't there any way to collect information from your hypervisor ?
>
> If EL2 doesn't allow accessing the TRC* registers via system instruction
> interface, why can't I find any description in the ARM manual?
Please look at the description for HDFGRTR_EL2/HDFGWTR_EL2 for more
information on this.
e.g., HDFGRTR_EL2:TRCID controls read access to TRCDEVARCH, which is
where you hit the crash.
>
> And from my understanding, ETE is still powered on even when the cpu is
> idle. Can it not be powered? There is extra clock enabling work
> when using the iomem interface, I will try if it matters.
As I mentioned, this is very unlikely. Most likely, your hypervisor is
at fault.
Suzuki
>
> Thanks,
> Yabin
>
>
> On Fri, Aug 18, 2023 at 1:47 AM Suzuki K Poulose <suzuki.poulose(a)arm.com
> <mailto:suzuki.poulose@arm.com>> wrote:
>
> Hi Yabin
>
> On 17/08/2023 22:06, Yabin Cui wrote:
> > Hi coresight driver maintainers,
> >
> > I am from the Android toolchain team. I am trying to use ETE
> (Embedded
> > Trace Extension). But on my device, the system crashes when the ETM
> > driver is trying to read TRCDEVARCH via sysreg interface.
>
> When you say crashes, what exactly is reported ? Does the kernel
> get an exception ? Or system hangs completely ?
>
> I suspect :
>
> 1) You are running some hyp at EL2, which doesn't allow
> accessing the TRC* registers and injects a Illegal instruction ?
> 2) If not the above, the ETE is not powered (very unlikely)
>
>
> Suzuki
>
>
> > In
> >
> https://github.com/torvalds/linux/blob/master/drivers/hwtracing/coresight/c… <https://github.com/torvalds/linux/blob/master/drivers/hwtracing/coresight/c…> <https://github.com/torvalds/linux/blob/master/drivers/hwtracing/coresight/c… <https://github.com/torvalds/linux/blob/master/drivers/hwtracing/coresight/c…>> :
> >
> > static bool etm4_init_sysreg_access(struct etmv4_drvdata *drvdata,
> > struct csdev_access *csa)
> > {
> > u32 devarch;
> >
> > if (!cpu_supports_sysreg_trace()) <- this returns true
> > return false;
> >
> > /*
> > * ETMs implementing sysreg access must implement TRCDEVARCH.
> > */
> > devarch = read_etm4x_sysreg_const_offset(TRCDEVARCH); <- never
> see the
> > message logged immediately after this line, so should abort here.
> > ...
> > }
> >
> > Do you have any suggestions on how I can debug the problem? Is
> there any
> > other place making sysreg interface to ETE not accessible? Or can
> the
> > ETE sysreg interface not be implemented?
> > I tried the memory mapped register interface of ETE, it works
> fine. The
> > only change I need is to bypass the devarch version check
> > in etm4_init_iomem_access, in in
> >
> https://github.com/torvalds/linux/blob/master/drivers/hwtracing/coresight/c… <https://github.com/torvalds/linux/blob/master/drivers/hwtracing/coresight/c…> <https://github.com/torvalds/linux/blob/master/drivers/hwtracing/coresight/c… <https://github.com/torvalds/linux/blob/master/drivers/hwtracing/coresight/c…>>
> > I wonder if we can allow iomem access for ETE if the sysreg access
> > doesn't work on some devices.
> >
> > Thanks,
> > Yabin
>
On 26/08/2023 00:39, Yabin Cui wrote:
> Because the non-secure access can be enabled later on some devices.
How can this be enabled ? Why not enable it before probing the ETR ?
How can a user know if this has been done or not ? It is asking for
trouble to continue without this.
Suzuki
>
> Signed-off-by: Yabin Cui <yabinc(a)google.com>
> ---
> drivers/hwtracing/coresight/coresight-tmc-core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
> index c106d142e632..5ebfd12b627b 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
> @@ -370,7 +370,7 @@ static int tmc_etr_setup_caps(struct device *parent, u32 devid, void *dev_caps)
> struct tmc_drvdata *drvdata = dev_get_drvdata(parent);
>
> if (!tmc_etr_has_non_secure_access(drvdata))
> - return -EACCES;
> + dev_warn(parent, "TMC ETR doesn't have non-secure access\n");
>
> /* Set the unadvertised capabilities */
> tmc_etr_init_caps(drvdata, (u32)(unsigned long)dev_caps);
Hi Steve
On 26/08/2023 01:14, Steve Clevenger wrote:
>
> Unfortunately, I tested with the original patch not [PATCH V2]. I've
> remedied this. My results below:
>
> [root@sut01sys-b212 linux]# cat
> /sys/devices/system/cpu/cpu123/ARMHC501\:23/tmc_etr35/buf_modes_available
> auto flat catu
> [root@sut01sys-b212 linux]# cat
> /sys/devices/system/cpu/cpu123/ARMHC501\:23/tmc_etr35/buf_mode_preferred
> auto
> [root@sut01sys-b212 linux]# echo "catu" >
> /sys/devices/system/cpu/cpu123/ARMHC501\:23/tmc_etr35/buf_mode_preferred
> [root@sut01sys-b212 linux]# cat
> /sys/devices/system/cpu/cpu123/ARMHC501\:23/tmc_etr35/buf_mode_preferred
> catu
>
> As with the V1 patch, auto defaults to catu.
>
> I expected to see tmc-sg (former default) as an available mode, but do
> not. As I recall, the buffer mode defaulted to ETR scatter-gather prior
> to this patch. Must this capability now be explicitly advertised? I've
> seen this done as "arm,scatter-gather" in device trees, but not used by
> Ampere. Perhaps someone can enlighten me.
Yes, you must add that property to the TMC-ETR node (for both DT and
ACPI). In the past, almost all of the TMC-ETRs (except Juno board)
locked up the system while using the SG mode (due to the interconnect
issues, something to do with the transaction). Thus, we decided to
add a property explicitly enabling this for a given platform.
When you mentioned, it was using TMC-ETR SG mode, how did you verify
this ? Please be aware that the table allocation code etc are shared
by both TMC-SG and CATU.
Kind regards
Suzuki
>
> Steve C.
>
> On 8/23/2023 4:10 PM, Steve Clevenger wrote:
>>
>> Here's some quick feedback. My system shows two modes available; auto catu
>>
>> etr_buf_mode_current is writable. I expected to see tmc-sg (former
>> default) listed in etr_buf_modes_available but it doesn't show up.
>>
>> Note that both the auto and catu etr_buf_mode_current settings default
>> to catu. My understanding is auto should revert to the default behavior.
>> On my system the default was tmc-sg.
>>
>> More later.
>>
>> [root@sut01sys-b212 kernel]# cat
>> /sys/devices/system/cpu/cpu20/ARMHC501\:60/tmc_etr96/etr_buf_modes_available
>>
>> auto catu
>> [root@sut01sys-b212 kernel]# cat
>> /sys/devices/system/cpu/cpu20/ARMHC501\:60/tmc_etr96/etr_buf_mode_current
>> catu
>> [root@sut01sys-b212 kernel]# echo "catu" >
>> /sys/devices/system/cpu/cpu20/ARMHC501\:60/tmc_etr96/etr_buf_mode_current
>> [root@sut01sys-b212 kernel]# cat
>> /sys/devices/system/cpu/cpu20/ARMHC501\:60/tmc_etr96/etr_buf_mode_current
>> catu
>>
>> Steve C.
>>
>>
>> On 8/21/2023 12:40 PM, Steve Clevenger wrote:
>>>
>>> Hi Suzuki,
>>>
>>> I may be able to test it this week. You've already pointed me at the
>>> patch thread(s). The main holdup is I need to merge the 6.6 pending
>>> platform work in order to use the Ampere ACPI. I couldn't get these
>>> patches to apply directly to 6.4 last I tried.
>>>
>>> Steve C.
>>>
>>> On 8/18/2023 2:39 AM, Suzuki K Poulose wrote:
>>>> Cc: Steve
>>>>
>>>> Steve,
>>>>
>>>> Are you able to test this with CATU ?
>>>>
>>>>
>>>> On 18/08/2023 09:21, Anshuman Khandual wrote:
>>>>> Currently TMC-ETR automatically selects the buffer mode from all
>>>>> available
>>>>> methods in the following sequentially fallback manner - also in that
>>>>> order.
>>>>>
>>>>> 1. FLAT mode with or without IOMMU
>>>>> 2. TMC-ETR-SG (scatter gather) mode when available
>>>>> 3. CATU mode when available
>>>>>
>>>>> But this order might not be ideal for all situations. For example if
>>>>> there
>>>>> is a CATU connected to ETR, it may be better to use TMC-ETR scatter
>>>>> gather
>>>>> method, rather than CATU. But hard coding such order changes will prevent
>>>>> us from testing or using a particular mode. This change provides
>>>>> following
>>>>> new sysfs tunables for the user to control TMC-ETR buffer mode
>>>>> explicitly,
>>>>> if required. This adds following new sysfs files for buffer mode
>>>>> selection
>>>>> purpose explicitly in the user space.
>>>>>
>>>>> /sys/bus/coresight/devices/tmc_etr<N>/buf_modes_available
>>>>> /sys/bus/coresight/devices/tmc_etr<N>/buf_mode_preferred
>>>>>
>>>>> $ cat buf_modes_available
>>>>> auto flat tmc-sg catu ------------------> Supported TMC-ETR buffer
>>>>> modes
>>>>>
>>>>> $ echo catu > buf_mode_preferred -------> Explicit buffer mode request
>>>>>
>>>>> But explicit user request has to be within supported ETR buffer modes
>>>>> only.
>>>>> These sysfs interface files are exclussive to ETR, and hence these are
>>>>> not
>>>>> available for other TMC devices such as ETB or ETF etc.
>>>>>
>>>>> A new auto' mode (i.e ETR_MODE_AUTO) has been added to help fallback
>>>>> to the
>>>>> existing default behaviour, when user provided preferred buffer mode
>>>>> fails.
>>>>> ETR_MODE_FLAT and ETR_MODE_AUTO are always available as preferred modes.
>>>>>
>>>>> Cc: Suzuki K Poulose <suzuki.poulose(a)arm.com>
>>>>> Cc: Mike Leach <mike.leach(a)linaro.org>
>>>>> Cc: James Clark <james.clark(a)arm.com>
>>>>> Cc: Leo Yan <leo.yan(a)linaro.org>
>>>>> Cc: Alexander Shishkin <alexander.shishkin(a)linux.intel.com>
>>>>> Cc: coresight(a)lists.linaro.org
>>>>> Cc: linux-arm-kernel(a)lists.infradead.org
>>>>> Cc: linux-kernel(a)vger.kernel.org
>>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual(a)arm.com>
>>>>> ---
>>>>> This applies on v6.5-rc6
>>>>>
>>>>> Changes in V2:
>>>>>
>>>>> - Renamed sysfs file etr_buf_modes_available as buf_modes_available
>>>>> - Renamed sysfs file buf_mode_current as buf_mode_preferred
>>>>> - Renamed etr_supports_flat_mode() as etr_can_use_flat_mode()
>>>>> - Renamed coresight_tmc_groups[] as coresight_etf_groups[]
>>>>> - Reused coresight_tmc_group[] for trigger_cntr and buffer_size
>>>>> - Fallback trying ETR_MODE_AUTO when user preferred mode fails
>>>>> - Moved ETR sysfs details into coresight-tmc-etr.c
>>>>> - Dropped etr_can_use_flat_mode() check while offering ETR_MODE_FLAT
>>>>> in sysfs
>>>>> - Moved struct etr_buf_hw inside coresight-tmc-etr.c
>>>>> - Moved get_etr_buf_hw() and etr_can_use_flat_mode() inside
>>>>> coresight-tmc-etr.c
>>>>> - Updated month in
>>>>> Documentation/ABI/testing/sysfs-bus-coresight-devices-tmc
>>>>>
>>>>> Changes in V1:
>>>>>
>>>>> https://lore.kernel.org/all/20230728084837.276551-1-anshuman.khandual@arm.c…
>>>>>
>>>>> .../testing/sysfs-bus-coresight-devices-tmc | 16 +++
>>>>> .../hwtracing/coresight/coresight-tmc-core.c | 15 ++-
>>>>> .../hwtracing/coresight/coresight-tmc-etr.c | 111 ++++++++++++++++--
>>>>> drivers/hwtracing/coresight/coresight-tmc.h | 3 +
>>>>> 4 files changed, 131 insertions(+), 14 deletions(-)
>>>>
>>>>
>>>> Looks good to me.
>>>>
>>>> Suzuki
>>>>
>>>>
On 8/19/23 02:41, Randy Dunlap wrote:
> Hi--
>
> On 8/18/23 04:20, Anshuman Khandual wrote:
>> This work arounds errata 1490853 on Cortex-A76, and Neoverse-N1, errata
>> 1491015 on Cortex-A77, errata 1502854 on Cortex-X1, and errata 1619801 on
>> Neoverse-V1, based affected cpus, where software read for TRCIDR3.CCITMIN
>> field in ETM gets an wrong value.
>>
>> If software uses the value returned by the TRCIDR3.CCITMIN register field,
>> then it will limit the range which could be used for programming the ETM.
>> In reality, the ETM could be programmed with a much smaller value than what
>> is indicated by the TRCIDR3.CCITMIN field and still function correctly.
>>
>> If software reads the TRCIDR3.CCITMIN register field, corresponding to the
>> instruction trace counting minimum threshold, observe the value 0x100 or a
>> minimum cycle count threshold of 256. The correct value should be 0x4 or a
>> minimum cycle count threshold of 4.
>>
>> This work arounds the problem via storing 4 in drvdata->ccitmin on affected
>> systems where the TRCIDR3.CCITMIN has been 256, thus preserving cycle count
>> threshold granularity.
>>
>> These errata information has been updated in arch/arm64/silicon-errata.rst,
>> but without their corresponding configs because these have been implemented
>> directly in the driver.
>>
>> Cc: Catalin Marinas <catalin.marinas(a)arm.com>
>> Cc: Will Deacon <will(a)kernel.org>
>> Cc: Suzuki K Poulose <suzuki.poulose(a)arm.com>
>> Cc: Mike Leach <mike.leach(a)linaro.org>
>> Cc: James Clark <james.clark(a)arm.com>
>> Cc: Jonathan Corbet <corbet(a)lwn.net>
>> Cc: linux-doc(a)vger.kernel.org
>> Cc: coresight(a)lists.linaro.org
>> Cc: linux-arm-kernel(a)lists.infradead.org
>> Cc: linux-kernel(a)vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual(a)arm.com>
>> ---
>> Documentation/arch/arm64/silicon-errata.rst | 10 ++++++
>> .../coresight/coresight-etm4x-core.c | 36 +++++++++++++++++++
>> 2 files changed, 46 insertions(+)
>>
>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 7e307022303a..591fab73ee79 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -1131,6 +1131,39 @@ static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata)
>> drvdata->trfcr = trfcr;
>> }
>>
>> +/*
>> + * The following errata on applicable cpu ranges, affect the CCITMIN filed
>> + * in TCRIDR3 register. Software read for the field returns 0x100 limiting
>> + * the cycle threshold granularity, where as the right value should have
>
> whereas
>
>> + * been 0x4, which is well supported in the hardware.
>> + */
>> +static struct midr_range etm_wrong_ccitmin_cpus[] = {
>> + /* Erratum #1490853 - Cortex-A76 */
>> + MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 4, 0),
>> + /* Erratum #1490853 - Neoverse-N1 */
>> + MIDR_RANGE(MIDR_NEOVERSE_N1, 0, 0, 4, 0),
>> + /* Erratum #1491015 - Cortex-A77 */
>> + MIDR_RANGE(MIDR_CORTEX_A77, 0, 0, 1, 0),
>> + /* Erratum #1502854 - Cortex-X1 */
>> + MIDR_REV(MIDR_CORTEX_X1, 0, 0),
>> + /* Erratum #1619801 - Neoverse-V1 */
>> + MIDR_REV(MIDR_NEOVERSE_V1, 0, 0),
>> + {},
>> +};
>> +
>> +static bool etm4_core_reads_wrong_ccitmin(struct etmv4_drvdata *drvdata)
>> +{
>> + /*
>> + * Erratum affected cpus will read 256 as the minimum
>> + * instruction trace cycle counting threshold where as
>
> whereas
Right, 'whereas' is a single word indeed. I will change these as required.