On Tue, 19 Jun 2018 at 19:24, Leo Yan leo.yan@linaro.org wrote:
On Tue, Jun 19, 2018 at 10:08:57AM -0600, Mathieu Poirier wrote:
On Tue, 19 Jun 2018 at 07:37, Leo Yan leo.yan@linaro.org wrote:
Hi Mathieu,
On Mon, Jun 18, 2018 at 07:26:20PM -0600, Mathieu Poirier wrote:
On Mon, 18 Jun 2018 at 18:59, Leo Yan leo.yan@linaro.org wrote:
Hi all,
Just in case you have the same issue, when I use acme's branch perf/core with latest commit e238cf2e3d2e ("perf intel-pt: Fix packet decoding of CYC packets"), I can easily reproduce below failure with 'perf record' command.
If you have fixing for this, please let me know. Otherwise I will dig a bit for this issue (probably relying on 'git bisect').
Haven't see this, though my tree is based on mainline 4.18-rc1. Let me know what bisect gives you.
Finally I found this issue is caused by I removed 'nohlt' in kernel command line, so CPU Idle states are enabled.
After CPU Idle states are enabled, we cannot access the register /sys/bus/coresight/devices/f659c000.etm/mgmt/trcauthstatus when the CPU stays in the low power states.
Interesting.
So following this issue, I have two questions (sorry if you guys have discussed these questions before):
- The first one question is should we support Runtime PM (Or GenPD) in Coresight? So when we access some component (e.g. the register trcauthstatus) we need to ensure the corresponding power domain is powered on properly before we access it.
We rely heavily on runtime PM everywhere [1] in the code to make sure devices are accessed only when they're powered up.
TRCAUTHSTATUS is a "management" register [2] and as such should not need to have the core powered up [3], which is why it is accessed with macro coresight_etm4x_reg() rather than coresight_etm4x_cross_read(). That being said implementation can differ from the guidelines.
First thing to do is to try going from coresight_etm4x_reg() to coresight_etm4x_cross_read() here [4] and see if there is any changes.
Thanks for suggestions. I verified below change can fix 'synchronous external abort' error when access TRCAUTHSTATUS register.
Just want to confirm should we commit one patch for this special case?
Tricky question... What we do here depends on how the HW has been designed and if the FW does the right thing. In function etm4_enable_hw(), the powerup request bit gets flipped [1] to tell the architecture to keep powering the tracers. As the note explains on page 7-360 of [2], this bit should drive a line that is connected to the power controller. The FW in the power controller needs to recognised the line is held high and keep powering the unit.
So, if the line isn't there on the board of if the power controller doesn't do anything with the incoming line there is no point in doing a patch, the kernel will end up crashing at one point.
The best thing to do is go back to HiSilicon and ask them about the HW design and the FW. It is only at that time that we can safely enable CPUidle and apply your patch.
Mathieu
[1]. https://elixir.bootlin.com/linux/latest/source/drivers/hwtracing/coresight/c... [2]. "ARM Embedded Trace Macrocell Architecture Specifiation" (ID032614)
---8<---
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c @@ -2065,7 +2065,7 @@ static u32 etmv4_cross_read(const struct device *dev, u32 offset) coresight_etm4x_reg(trcpdcr, TRCPDCR); coresight_etm4x_reg(trcpdsr, TRCPDSR); coresight_etm4x_reg(trclsr, TRCLSR); -coresight_etm4x_reg(trcauthstatus, TRCAUTHSTATUS); +coresight_etm4x_cross_read(trcauthstatus, TRCAUTHSTATUS); coresight_etm4x_reg(trcdevid, TRCDEVID); coresight_etm4x_reg(trcdevtype, TRCDEVTYPE); coresight_etm4x_reg(trcpidr0, TRCPIDR0);
If that doesn't work you need to ask HiSilicon what power domain the register is in. From there get back to me and we'll talk things over.
[1]. https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/arm/juno-... [2]. See "ARM Embedded Trace Macrocell Architecture Specifiation" (ID032614), table 7-1 on page 7-311 for details. [3]. Same document as above, table 7-4 on page 7-314 [4]. https://elixir.bootlin.com/linux/latest/source/drivers/hwtracing/coresight/c...
- The second one question is another side topic: if we support CPU wide tracing, should we support suspend/resume flow for Coresight ETM? So we can save and restore context for CPU respectively.
When using Perf suspend/resume is already supported - tracers won't be kept on whey there is no more processes executing on a CPU (just before going to idle). Tracers will also be switched back on once the CPU has resumed, just before processes start being executed on it. Configuration of the trace unit is kept in memory. In sysFS mode I expected users to disable CPUidle, but that can be change if you have a use case.
[ + Chunyan ]
From my understanding, the most common use case is to use Coresight tracing to capture system hang case. So after system booting up, we can use sysFS to enable tracers and sinks (e.g. ETB), then the system is ongoing to run some workloads with Coresight tracing enabled, after the system hang we can read out Coresight trace data and analyze the execution flow before hang issue. For this case, usually the ARM CPU idle states are enabled so CPUs might frequently enter and exit low power states.
Thanks, Leo Yan