Changes since v1:
* Remove first 5 patches as they have been applied to perf/core
* Add Leo's kernel symbols suggestion and reviewed by tag
* Add reference to debuginfod as suggested by Arnaldo
Applies to perf/core f3c33cbd92
James Clark (1):
perf cs-etm: Add warnings for missing DSOs
tools/perf/util/cs-etm.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
--
2.23.0
On 30/07/2021 12:26, Anshuman Khandual wrote:
>
>
> On 7/28/21 7:22 PM, Suzuki K Poulose wrote:
>> Add the CPU Partnumbers for the new Arm designs.
>>
>> Cc: Catalin Marinas <catalin.marinas(a)arm.com>
>> Cc: Mark Rutland <mark.rutland(a)arm.com>
>> Cc: Will Deacon <will(a)kernel.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com>
>> ---
>> arch/arm64/include/asm/cputype.h | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
>> index 6231e1f0abe7..b71bd6c176c2 100644
>> --- a/arch/arm64/include/asm/cputype.h
>> +++ b/arch/arm64/include/asm/cputype.h
>> @@ -73,6 +73,8 @@
>> #define ARM_CPU_PART_CORTEX_A76 0xD0B
>> #define ARM_CPU_PART_NEOVERSE_N1 0xD0C
>> #define ARM_CPU_PART_CORTEX_A77 0xD0D
>> +#define ARM_CPU_PART_CORTEX_A710 0xD47
>> +#define ARM_CPU_PART_NEOVERSE_N2 0xD49
>>
>> #define APM_CPU_PART_POTENZA 0x000
>>
>> @@ -112,6 +114,8 @@
>> #define MIDR_CORTEX_A55 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A55)
>> #define MIDR_CORTEX_A76 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A76)
>> #define MIDR_NEOVERSE_N1 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_N1)
>> +#define MIDR_CORTEX_A710 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A710)
>> +#define MIDR_NEOVERSE_N2 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_N2)
>
> Should not the new ones be added after MIDR_CORTEX_A77 below to preserve the order ?
>
TBH. The order doesn't matter much as they are part numbers for
different CPUs, without any dependencies between them.
But for the sake of keeping the order, I could fix this.
Thanks
Suzuki
On 30/07/2021 12:02, Anshuman Khandual wrote:
>
>
> On 7/28/21 7:22 PM, Suzuki K Poulose wrote:
>> The TRBE hardware mandates a minimum alignment for the TRBPTR_EL1,
>> advertised via the TRBIDR_EL1. This is used by the driver to
>> align the buffer write head. This patch allows the driver to
>> choose a different alignment from that of the hardware, by
>> decoupling the alignment tracking. This will be useful for
>> working around errata.
>>
>> Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org>
>> Cc: Anshuman Khandual <anshuman.khandual(a)arm.com>
>> Cc: Mike Leach <mike.leach(a)linaro.org>
>> Cc: Leo Yan <leo.yan(a)linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com>
>> ---
>> drivers/hwtracing/coresight/coresight-trbe.c | 12 ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>> index 9735d514c5e1..9ea28813182b 100644
>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>> @@ -92,7 +92,8 @@ static unsigned long trbe_errata_cpucaps[TRBE_ERRATA_MAX] = {
>> /*
>> * struct trbe_cpudata: TRBE instance specific data
>> * @trbe_flag - TRBE dirty/access flag support
>> - * @tbre_align - Actual TRBE alignment required for TRBPTR_EL1.
>> + * @trbe_hw_align - Actual TRBE alignment required for TRBPTR_EL1.
>> + * @trbe_align - Software alignment used for the TRBPTR_EL1,
>> * @cpu - CPU this TRBE belongs to.
>> * @mode - Mode of current operation. (perf/disabled)
>> * @drvdata - TRBE specific drvdata
>> @@ -100,6 +101,7 @@ static unsigned long trbe_errata_cpucaps[TRBE_ERRATA_MAX] = {
>> */
>> struct trbe_cpudata {
>> bool trbe_flag;
>> + u64 trbe_hw_align;
>> u64 trbe_align;
>> int cpu;
>> enum cs_mode mode;
>> @@ -906,7 +908,7 @@ static ssize_t align_show(struct device *dev, struct device_attribute *attr, cha
>> {
>> struct trbe_cpudata *cpudata = dev_get_drvdata(dev);
>>
>> - return sprintf(buf, "%llx\n", cpudata->trbe_align);
>> + return sprintf(buf, "%llx\n", cpudata->trbe_hw_align);
>> }
>> static DEVICE_ATTR_RO(align);
>>
>> @@ -995,11 +997,13 @@ static void arm_trbe_probe_cpu(void *info)
>> }
>>
>> trbe_check_errata(cpudata);
>> - cpudata->trbe_align = 1ULL << get_trbe_address_align(trbidr);
>> - if (cpudata->trbe_align > SZ_2K) {
>> +
>> + cpudata->trbe_hw_align = 1ULL << get_trbe_address_align(trbidr);
>> + if (cpudata->trbe_hw_align > SZ_2K) {
>> pr_err("Unsupported alignment on cpu %d\n", cpu);
>> goto cpu_clear;
>> }
>> + cpudata->trbe_align = cpudata->trbe_hw_align;
>
> When it changes, it must be asserted that trbe_align would be a multiple
> of trbe_hw_align before existing from arm_trbe_probe_cpu().
We only set it to PAGE_SIZE, which is one of 4K, 16K, 64K all of which
are aligned to 2K or any of the smaller alignment supported by TRBE.
>
>> cpudata->trbe_flag = get_trbe_flag_update(trbidr);
>> cpudata->cpu = cpu;
>> cpudata->drvdata = drvdata;
>>
>
> Reviewed-by: Anshuman Khandual <anshuman.khandual(a)arm.com>
>
Thanks
Suzuki
On 30/07/2021 05:47, Anshuman Khandual wrote:
>
>
> On 7/23/21 6:16 PM, Suzuki K Poulose wrote:
>> We mark the buffer as TRUNCATED when there is no space left
>> in the buffer. But we do it at different points.
>> __trbe_normal_offset()
>> and also, at all the callers of the above function via
>> compute_trbe_buffer_limit(), when the limit == base (i.e
>> offset = 0 as returned by the __trbe_normal_offset()).
>>
>> So, given that the callers already mark the buffer as TRUNCATED
>> drop the caller inside the __trbe_normal_offset().
>>
>> This is in preparation to moving the handling of TRUNCATED
>> into a central place.
>>
>> Cc: Anshuman Khandual <anshuman.khandual(a)arm.com>
>> Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org>
>> Cc: Mike Leach <mike.leach(a)linaro.org>
>> Cc: Leo Yan <leo.yan(a)linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com>
>> ---
>> drivers/hwtracing/coresight/coresight-trbe.c | 7 +------
>> 1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>> index 446f080f8320..62e1a08f73ff 100644
>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>> @@ -253,13 +253,9 @@ static unsigned long __trbe_normal_offset(struct perf_output_handle *handle)
>> * trbe_base trbe_base + nr_pages
>> *
>> * Perf aux buffer does not have any space for the driver to write into.
>> - * Just communicate trace truncation event to the user space by marking
>> - * it with PERF_AUX_FLAG_TRUNCATED.
>> */
>> - if (!handle->size) {
>> - perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
>> + if (!handle->size)
>> return 0;
>> - }
>>
>> /* Compute the tail and wakeup indices now that we've aligned head */
>> tail = PERF_IDX2OFF(handle->head + handle->size, buf);
>> @@ -361,7 +357,6 @@ static unsigned long __trbe_normal_offset(struct perf_output_handle *handle)
>> return limit;
>>
>> trbe_pad_buf(handle, handle->size);
>> - perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
>> return 0;
>> }
>
> What about in trbe_handle_spurious() path which used to set the flag via
> compute_trbe_buffer_limit(), but would not any more after this change. I
> guess following additional change would be required to preserve the past
> behaviour.
>
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -685,6 +685,7 @@ static void trbe_handle_spurious(struct perf_output_handle *handle)
> buf->trbe_write = buf->trbe_base + PERF_IDX2OFF(handle->head, buf);
> if (buf->trbe_limit == buf->trbe_base) {
> trbe_drain_and_disable_local();
> + perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
> return;
> }
> trbe_enable_hw(buf);
>
Agreed, I will add this.
Thanks
Suzuki
On 30/07/2021 06:15, Anshuman Khandual wrote:
>
>
> On 7/23/21 6:16 PM, Suzuki K Poulose wrote:
>> On a spurious IRQ, right now we disable the TRBE and then re-enable
>> it back, resetting the "buffer" pointers(i.e BASE, LIMIT and more
>> importantly WRITE) to the original pointers from the AUX handle.
>> This implies that we overwrite any trace that was written so far,
>> (by overwriting TRBPTR) while we should have ignored the IRQ.
>
> The ideas was that a state (pointers) reset would improve the chances
> of not getting the spurious IRQ once again. This is assuming that some
> thing during this current state machine, had caused the spurious IRQ.
> Hence just restart it back from the beginning. Yes, it does lose some
> trace data but whats the real possibility of such spurious IRQs in the
> first place ?
>
>>
>> This patch cleans the behavior, by only stopping the TRBE if the
>> IRQ was indeed raised, as we can read the TRBSR without stopping
>> the TRBE (Only writes to the TRBSR requires the TRBE disabled).
>> And also, on detecting a spurious IRQ after examining the TRBSR,
>> we simply re-enable the TRBE without touching the other parameters.
>
> This makes sense. I was not sure if TRBSR could be safely read without
> actually stopping the TRBE.
>
>>
>> Cc: Anshuman Khandual <anshuman.khandual(a)arm.com>
>> Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org>
>> Cc: Mike Leach <mike.leach(a)linaro.org>
>> Cc: Leo Yan <leo.yan(a)linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com>
>> ---
>> drivers/hwtracing/coresight/coresight-trbe.c | 29 ++++++++++----------
>> 1 file changed, 15 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>> index 62e1a08f73ff..503bea0137ae 100644
>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>> @@ -679,15 +679,16 @@ static int arm_trbe_disable(struct coresight_device *csdev)
>>
>> static void trbe_handle_spurious(struct perf_output_handle *handle)
>> {
>> - struct trbe_buf *buf = etm_perf_sink_config(handle);
>> + u64 limitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
>>
>> - buf->trbe_limit = compute_trbe_buffer_limit(handle);
>> - buf->trbe_write = buf->trbe_base + PERF_IDX2OFF(handle->head, buf);
>> - if (buf->trbe_limit == buf->trbe_base) {
>> - trbe_drain_and_disable_local();
>> - return;
>> - }
>> - trbe_enable_hw(buf);
>> + /*
>> + * If the IRQ was spurious, simply re-enable the TRBE
>> + * back without modifiying the buffer parameters to
>
> Typo here ^^^^^^ s/modifiying/modifying
>
>> + * retain the trace collected so far.
>> + */
>> + limitr |= TRBLIMITR_ENABLE;
>> + write_sysreg_s(limitr, SYS_TRBLIMITR_EL1);
>> + isb();
>> }
>>
>> static void trbe_handle_overflow(struct perf_output_handle *handle)
>> @@ -760,12 +761,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
>> enum trbe_fault_action act;
>> u64 status;
>>
>> - /*
>> - * Ensure the trace is visible to the CPUs and
>> - * any external aborts have been resolved.
>> - */
>> - trbe_drain_and_disable_local();
>> -
>> + /* Reads to TRBSR_EL1 is fine when TRBE is active */
>> status = read_sysreg_s(SYS_TRBSR_EL1);
>> /*
>> * If the pending IRQ was handled by update_buffer callback
>> @@ -774,6 +770,11 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
>> if (!is_trbe_irq(status))
>
> Warn here that a non-related IRQ has been delivered to this handler ?
> But moving the trbe_drain_and_disable_local() later, enables it to
> return back immediately after detecting an unrelated IRQ.
Not really. There could be race with the update_buffer(), see the
comment right above that. When that happens, we have disabled the
TRBE in the update_buffer(). Either case, we have nothing to do.
>
>> return IRQ_NONE;
>>
>> + /*
>> + * Ensure the trace is visible to the CPUs and
>> + * any external aborts have been resolved.
>> + */
>> + trbe_drain_and_disable_local();
>> clr_trbe_irq();
>> isb();
>>
>>
>
> Actually there are two types of spurious interrupts here.
>
> 1. Non-TRBE spurious interrupt
>
> Fails is_trbe_irq() test and needs to be returned immediately from
> arm_trbe_irq_handler(), after an warning for the platform IRQ
> delivery wiring.
Not necessarily warrant a WARNING. See above.
>
> 2. TRBE spurious interrupt
>
> Clears is_trbe_irq() and get handled in trbe_handle_spurious(). I
> still think leaving this unchanged might be better as it reduces
> the chance of getting further spurious TRBE interrupts.
How does it reduce the chances of getting another spurious interrupt ?
If the TRBE gets a spurious IRQ, that we cannot decode, I would rather
leave it as NOP.
Suzuki
On 30/07/2021 05:26, Anshuman Khandual wrote:
>
>
> On 7/23/21 6:16 PM, Suzuki K Poulose wrote:
>> When the TRBE is stopped on truncating an event, we may not
>> set the FORMAT flag, even though the size of the record is 0.
>> Let us be consistent and not confuse the user. Always set the
>> format flag for TRBE generated records.
>>
>> Cc: Anshuman Khandual <anshuman.khandual(a)arm.com>
>> Cc: Leo Yan <leo.yan(a)linaro.org>
>> Cc: Mike Leach <mike.leach(a)linaro.org>
>> Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com>
>> ---
>> drivers/hwtracing/coresight/coresight-trbe.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>> index 176868496879..446f080f8320 100644
>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>> @@ -132,7 +132,8 @@ static void trbe_stop_and_truncate_event(struct perf_output_handle *handle)
>> * the update_buffer() to return a 0 size.
>> */
>> trbe_drain_and_disable_local();
>> - perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
>> + perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED |
>> + PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW);
>> *this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL;
>> }
>
> But why should not PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW also be set on
> zero sized records as well ? Otherwise there are two instances during
> TRBE buffer management, where PERF_AUX_FLAG_TRUNCATED is marked alone
> without PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW. Those could be changed as
> well.
All records (irrespective of the size) generated by the TRBE must
contain the "RAW" flag. Did I miss another instance where we don't
do this ?
Suzuki
>
On 30/07/2021 04:48, Anshuman Khandual wrote:
>
> On 7/23/21 6:16 PM, Suzuki K Poulose wrote:
>> The Trace Filtering support (FEAT_TRF) ensures that the ETM
>> can be prohibited from generating any trace for a given EL.
>> This is much stricter knob, than the TRCVICTLR exception level
>
> Could you please explain 'stricter' ? Are you suggesting that TRCVICTLR
> based exception filtering some times might not implement the filtering
> even if configured ?
>
Sure, the TRVICTLR only ensures that the ETM doesn't generate
any "branch" trace packets. But that doesn't prevent it from
generating the "Context" packets which may contain the kernel
addresses, if they are generated while in Kernel.
But, the FEAT_TRF strictly prevents the trace unit from generating
any packets while it is "prohibited". Thus it is a much better
control to prevent kernel address leaks via the trace.
>> masks. At the moment, we do a onetime enable trace at user and
>> kernel and leave it untouched for the kernel life time.
>>
>> This patch makes the switch dynamic, by honoring the filters
>> set by the user and enforcing them in the TRFCR controls.
>
> TRFCR actually helps in making the exception level filtering dynamic
> which was not possible earlier with TRCVICTLR.
>
>> We also rename the cpu_enable_tracing() appropriately to
>> cpu_detect_trace_filtering() and the drvdata member
>> trfc => trfcr to indicate the "value" of the TRFCR_EL1.
>
> Makes sense.
>
>>
>> Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org>
>> Cc: Al Grant <al.grant(a)arm.com>
>> Cc: Mike Leach <mike.leach(a)linaro.org>
>> Cc: Leo Yan <leo.yan(a)linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com>
>> ---
>> .../coresight/coresight-etm4x-core.c | 61 ++++++++++++++-----
>> drivers/hwtracing/coresight/coresight-etm4x.h | 5 +-
>> .../coresight/coresight-self-hosted-trace.h | 7 +++
>> 3 files changed, 55 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 3e548dac9b05..adba84b29455 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -237,6 +237,43 @@ struct etm4_enable_arg {
>> int rc;
>> };
>>
>> +/*
>> + * etm4x_prohibit_trace - Prohibit the CPU from tracing at all ELs.
>> + * When the CPU supports FEAT_TRF, we could move the ETM to a trace
>> + * prohibited state by filtering the Exception levels via TRFCR_EL1.
>> + */
>> +static void etm4x_prohibit_trace(struct etmv4_drvdata *drvdata)
>> +{
>> + if (drvdata->trfcr)
>> + cpu_prohibit_trace();
>
> Should it be as etm4x_allow_trace() instead, where drvdata->trfcr
> indicates the presence of FEAT_TRF - just to be clear ?
>
> /* If the CPU doesn't support FEAT_TRF, nothing to do */
> if (!drvdata->trfcr)
> return;
>
> cpu_prohibit_trace();
>
OK
>> +}
>> +
>> +/*
>> + * etm4x_allow_trace - Allow CPU tracing in the respective ELs,
>> + * as configured by the drvdata->config.mode for the current
>> + * session. Even though we have TRCVICTLR bits to filter the
>> + * trace in the ELs, it doesn't prevent the ETM from generating
>> + * a packet (e.g, TraceInfo) that might contain the addresses from
>> + * the excluded levels. Thus we use the additional controls provided
>> + * via the Trace Filtering controls (FEAT_TRF) to make sure no trace
>> + * is generated for the excluded ELs.
>> + */
>> +static void etm4x_allow_trace(struct etmv4_drvdata *drvdata)
>> +{
>> + u64 trfcr = drvdata->trfcr;
>> +
>> + /* If the CPU doesn't support FEAT_TRF, nothing to do */
>> + if (!trfcr)
>> + return;
>> +
>> + if (drvdata->config.mode & ETM_MODE_EXCL_KERN)
>> + trfcr &= ~TRFCR_ELx_ExTRE;
>> + if (drvdata->config.mode & ETM_MODE_EXCL_USER)
>> + trfcr &= ~TRFCR_ELx_E0TRE;
>> +
>> + write_trfcr(trfcr);
>> +}
>> +
>> #ifdef CONFIG_ETM4X_IMPDEF_FEATURE
>>
>> #define HISI_HIP08_AMBA_ID 0x000b6d01
>> @@ -441,6 +478,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
>> if (etm4x_is_ete(drvdata))
>> etm4x_relaxed_write32(csa, TRCRSR_TA, TRCRSR);
>>
>> + etm4x_allow_trace(drvdata);
>> /* Enable the trace unit */
>> etm4x_relaxed_write32(csa, 1, TRCPRGCTLR);
>>
>> @@ -719,7 +757,6 @@ static int etm4_enable(struct coresight_device *csdev,
>> static void etm4_disable_hw(void *info)
>> {
>> u32 control;
>> - u64 trfcr;
>> struct etmv4_drvdata *drvdata = info;
>> struct etmv4_config *config = &drvdata->config;
>> struct coresight_device *csdev = drvdata->csdev;
>> @@ -746,12 +783,7 @@ static void etm4_disable_hw(void *info)
>> * If the CPU supports v8.4 Trace filter Control,
>> * set the ETM to trace prohibited region.
>> */
>> - if (drvdata->trfc) {
>> - trfcr = read_sysreg_s(SYS_TRFCR_EL1);
>> - write_sysreg_s(trfcr & ~(TRFCR_ELx_ExTRE | TRFCR_ELx_E0TRE),
>> - SYS_TRFCR_EL1);
>> - isb();
>> - }
>> + etm4x_prohibit_trace(drvdata);
>> /*
>> * Make sure everything completes before disabling, as recommended
>> * by section 7.3.77 ("TRCVICTLR, ViewInst Main Control Register,
>> @@ -767,9 +799,6 @@ static void etm4_disable_hw(void *info)
>> if (coresight_timeout(csa, TRCSTATR, TRCSTATR_PMSTABLE_BIT, 1))
>> dev_err(etm_dev,
>> "timeout while waiting for PM stable Trace Status\n");
>> - if (drvdata->trfc)
>> - write_sysreg_s(trfcr, SYS_TRFCR_EL1);
>> -
>> /* read the status of the single shot comparators */
>> for (i = 0; i < drvdata->nr_ss_cmp; i++) {
>> config->ss_status[i] =
>> @@ -964,15 +993,15 @@ static bool etm4_init_csdev_access(struct etmv4_drvdata *drvdata,
>> return false;
>> }
>>
>> -static void cpu_enable_tracing(struct etmv4_drvdata *drvdata)
>> +static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata)
>> {
>> u64 dfr0 = read_sysreg(id_aa64dfr0_el1);
>> u64 trfcr;
>>
>> + drvdata->trfcr = 0;
>> if (!cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_TRACE_FILT_SHIFT))
>> return;
>>
>> - drvdata->trfc = true;
>> /*
>> * If the CPU supports v8.4 SelfHosted Tracing, enable
>> * tracing at the kernel EL and EL0, forcing to use the
>> @@ -986,7 +1015,7 @@ static void cpu_enable_tracing(struct etmv4_drvdata *drvdata)
>> if (is_kernel_in_hyp_mode())
>> trfcr |= TRFCR_EL2_CX;
>>
>> - write_trfcr(trfcr);
>> + drvdata->trfcr = trfcr;
>> }
>>
>> static void etm4_init_arch_data(void *info)
>> @@ -1177,7 +1206,7 @@ static void etm4_init_arch_data(void *info)
>> /* NUMCNTR, bits[30:28] number of counters available for tracing */
>> drvdata->nr_cntr = BMVAL(etmidr5, 28, 30);
>> etm4_cs_lock(drvdata, csa);
>> - cpu_enable_tracing(drvdata);
>> + cpu_detect_trace_filtering(drvdata);
>> }
>>
>> static inline u32 etm4_get_victlr_access_type(struct etmv4_config *config)
>> @@ -1673,7 +1702,7 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata)
>> int ret = 0;
>>
>> /* Save the TRFCR irrespective of whether the ETM is ON */
>> - if (drvdata->trfc)
>> + if (drvdata->trfcr)
>> drvdata->save_trfcr = read_trfcr();
>> /*
>> * Save and restore the ETM Trace registers only if
>> @@ -1782,7 +1811,7 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
>>
>> static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
>> {
>> - if (drvdata->trfc)
>> + if (drvdata->trfcr)
>> write_trfcr(drvdata->save_trfcr);
>> if (drvdata->state_needs_restore)
>> __etm4_cpu_restore(drvdata);
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
>> index 82cba16b73a6..724819592c2e 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
>> @@ -919,7 +919,8 @@ struct etmv4_save_state {
>> * @nooverflow: Indicate if overflow prevention is supported.
>> * @atbtrig: If the implementation can support ATB triggers
>> * @lpoverride: If the implementation can support low-power state over.
>> - * @trfc: If the implementation supports Arm v8.4 trace filter controls.
>> + * @trfcr: If the CPU supportfs FEAT_TRF, value of the TRFCR_ELx with
>
> Typo here. ^^^^^^ s/supportfs/supports
>
>> + * trace allowed at user and kernel ELs. Otherwise, 0.
>
> The sentence here does not make sense. Is not the exception level ELx and EL0
> can be filtered out independently ? Should this be something like ...
The value holds a superset of the possible "allowed" configurations.
We do this to avoid setting the TRFCR_CX everytime depending on the
kernel EL (only possible from EL2). So we initialize the field
with value of TRCR_ELx with all the ELs enabled. This can be filtered
later by the driver accordingly. This will also serve as marker
to check the availability of the feature.
Thanks
Suzuki
>
> "If the CPU supports FEAT_TRF, value of the TRFCR_ELx - indicating whether
> trace is allowed at user [and/or] kernel ELs. Otherwise, 0."
>
>> * @config: structure holding configuration parameters.
>> * @save_trfcr: Saved TRFCR_EL1 register during a CPU PM event.
>> * @save_state: State to be preserved across power loss
>> @@ -972,7 +973,7 @@ struct etmv4_drvdata {
>> bool nooverflow;
>> bool atbtrig;
>> bool lpoverride;
>> - bool trfc;
>> + u64 trfcr;
>> struct etmv4_config config;
>> u64 save_trfcr;
>> struct etmv4_save_state *save_state;
>> diff --git a/drivers/hwtracing/coresight/coresight-self-hosted-trace.h b/drivers/hwtracing/coresight/coresight-self-hosted-trace.h
>> index 53b35a28075e..586d26e0cba3 100644
>> --- a/drivers/hwtracing/coresight/coresight-self-hosted-trace.h
>> +++ b/drivers/hwtracing/coresight/coresight-self-hosted-trace.h
>> @@ -22,4 +22,11 @@ static inline void write_trfcr(u64 val)
>> isb();
>> }
>>
>> +static inline void cpu_prohibit_trace(void)
>> +{
>> + u64 trfcr = read_trfcr();
>> +
>> + /* Prohibit tracing at EL0 & the kernel EL */
>> + write_trfcr(trfcr & ~(TRFCR_ELx_ExTRE | TRFCR_ELx_E0TRE));
>> +}
>> #endif /* __CORESIGHT_SELF_HOSTED_TRACE_H */
>>
This patchset introduces initial concepts in CoreSight system
configuration management support. to allow more detailed and complex
programming to be applied to CoreSight systems during trace capture.
Configurations consist of 2 elements:-
1) Features - programming combinations for devices, applied to a class of
device on the system (all ETMv4), or individual devices.
2) Configurations - a set of programmed features used when the named
configuration is selected.
Features and configurations are declared as a data table, a set of register,
resource and parameter requirements. Features and configurations are loaded
into the system by the virtual cs_syscfg device. This then matches features
to any registered devices and loads the feature into them.
Individual device classes that support feature and configuration register
with cs_syscfg.
Once loaded a configuration can be enabled for a specific trace run.
Configurations are registered with the perf cs_etm event as entries in
cs_etm/events. These can be selected on the perf command line as follows:-
perf record -e cs_etm/<config_name>/ ...
This patch set has one pre-loaded configuration and feature.
A named "strobing" feature is provided for ETMv4.
A named "autofdo" configuration is provided. This configuration enables
strobing on any ETM in used.
Thus the command:
perf record -e cs_etm/autofdo/ ...
will trace the supplied application while enabling the "autofdo" configuation
on each ETM as it is enabled by perf. This in turn will enable strobing for
the ETM - with default parameters. Parameters can be adjusted using configfs.
The sink used in the trace run will be automatically selected.
A configuration can supply up to 15 of preset parameter values, which will
subsitute in parameter values for any feature used in the configuration.
Selection of preset values as follows
perf record -e cs_etm/autofdo,preset=1/ ...
(valid presets 1-N, where N is the number supplied in the configuration, not
exceeding 15. preset=0 is the same as not selecting a preset.)
Applies to & tested against coresight/next (5.13-rc6 base)
Changes since v8:
Patch 0003 - altered spinlock to use irq versions. Moved use of config enabled flag.
Patch 0005 - dropped in_enable flag. Use config enabled flag within spinlock guards.
Changes since v7:
Fixed kernel test robot issue - config with CORESIGHT=y & CONFIGFS_FS=m causes
build error. Altered CORESIGHT config to select CONFIGFS_FS.
Reported-by: kernel test robot <lkp(a)intel.com>
Replaced mutex use to protect loaded config lists in coresight devices with per
device spinlock to remove issue when disable called in interrupt context.
Reported-by: Branislav Rankov <branislav.rankov(a)arm.com>
Changes since v6:
Fixed kernel test robot issues-
Reported-by: kernel test robot <lkp(a)intel.com>
Changes since v5:
1) Fix code style issues from auto-build reports, as
Reported-by: kernel test robot <lkp(a)intel.com>
2) Update comments to get consistent docs for API functions.
3) remove unused #define from autofdo example.
4) fix perf code style issues from patch 4 (Mathieu)
5) fix configfs code style issues from patch 9. (Mathieu)
Changes since v4: (based on comments from Matthieu and Suzuki).
No large functional changes - primarily code improvements and naming schema.
1) Updated entire set to ensure a consistent naming scheme was used for
variables and struct members that refer to the key objects in the system.
Suffixes _desc used for all references to feature and configuraion descriptors,
suffix _csdev used for all references to load feature and configs in the csdev
instances. (Mathieu & Suzuki).
2) Dropped the 'configurations' sub dir in cs_etm perf directories as superfluous
with the configfs containing the same information. (Mathieu).
3) Simplified perf handling code (suzuki)
4) Multiple simplifications and improvements for code readability (Matthieu
and Suzuki)
Changes since v3: (Primarily based on comments from Matthieu)
1) Locking mechanisms simplified.
2) Removed the possibility to enable features independently from
configurations.Only configurations can be enabled now. Simplifies programming
logic.
3) Configuration now uses an activate->enable mechanism. This means that perf
will activate a selected configuration at the start of a session (during
setup_aux), and disable at the end of a session (around free_aux)
The active configuration and associated features will be programmed into the
CoreSight device instances when they are enabled. This locks the configuration
into the system while in use. Parameters cannot be altered while this is
in place. This mechanism will be extended in future for dynamic load / unload
of configurations to prevent removal while in use.
4) Removed the custom bus / driver as un-necessary. A single device is
registered to own perf fs elements and configfs.
5) Various other minor issues addressed.
Changes since v2:
1) Added documentation file.
2) Altered cs_syscfg driver to no longer be coresight_device based, and moved
to its own custom bus to remove it from the main coresight bus. (Mathieu)
3) Added configfs support to inspect and control loaded configurations and
features. Allows listing of preset values (Yabin Cui)
4) Dropped sysfs support for adjusting feature parameters on the per device
basis, in favour of a single point adjustment in configfs that is pushed to all
device instances.
5) Altered how the config and preset command line options are handled in perf
and the drivers. (Mathieu and Suzuki).
6) Fixes for various issues and technical points (Mathieu, Yabin)
Changes since v1:
1) Moved preloaded configurations and features out of individual drivers.
2) Added cs_syscfg driver to manage configurations and features. Individual
drivers register with cs_syscfg indicating support for config, and provide
matching information that the system uses to load features into the drivers.
This allows individual drivers to be updated on an as needed basis - and
removes the need to consider devices that cannot benefit from configuration -
static replicators, funnels, tpiu.
3) Added perf selection of configuarations.
4) Rebased onto the coresight module loading set.
To follow in future revisions / sets:-
a) load of additional config and features by loadable module.
b) load of additional config and features by configfs
c) enhanced resource management for ETMv4 and checking features have sufficient
resources to be enabled.
d) ECT and CTI support for configuration and features.
Mike Leach (10):
coresight: syscfg: Initial coresight system configuration
coresight: syscfg: Add registration and feature loading for cs devices
coresight: config: Add configuration and feature generic functions
coresight: etm-perf: update to handle configuration selection
coresight: syscfg: Add API to activate and enable configurations
coresight: etm-perf: Update to activate selected configuration
coresight: etm4x: Add complex configuration handlers to etmv4
coresight: config: Add preloaded configurations
coresight: syscfg: Add initial configfs support
Documentation: coresight: Add documentation for CoreSight config
.../trace/coresight/coresight-config.rst | 244 +++++
Documentation/trace/coresight/coresight.rst | 16 +
drivers/hwtracing/coresight/Kconfig | 1 +
drivers/hwtracing/coresight/Makefile | 7 +-
.../hwtracing/coresight/coresight-cfg-afdo.c | 153 ++++
.../coresight/coresight-cfg-preload.c | 31 +
.../coresight/coresight-cfg-preload.h | 13 +
.../hwtracing/coresight/coresight-config.c | 272 ++++++
.../hwtracing/coresight/coresight-config.h | 253 ++++++
drivers/hwtracing/coresight/coresight-core.c | 12 +-
.../hwtracing/coresight/coresight-etm-perf.c | 150 +++-
.../hwtracing/coresight/coresight-etm-perf.h | 12 +-
.../hwtracing/coresight/coresight-etm4x-cfg.c | 182 ++++
.../hwtracing/coresight/coresight-etm4x-cfg.h | 30 +
.../coresight/coresight-etm4x-core.c | 38 +-
.../coresight/coresight-etm4x-sysfs.c | 3 +
.../coresight/coresight-syscfg-configfs.c | 396 ++++++++
.../coresight/coresight-syscfg-configfs.h | 45 +
.../hwtracing/coresight/coresight-syscfg.c | 844 ++++++++++++++++++
.../hwtracing/coresight/coresight-syscfg.h | 81 ++
include/linux/coresight.h | 9 +
21 files changed, 2756 insertions(+), 36 deletions(-)
create mode 100644 Documentation/trace/coresight/coresight-config.rst
create mode 100644 drivers/hwtracing/coresight/coresight-cfg-afdo.c
create mode 100644 drivers/hwtracing/coresight/coresight-cfg-preload.c
create mode 100644 drivers/hwtracing/coresight/coresight-cfg-preload.h
create mode 100644 drivers/hwtracing/coresight/coresight-config.c
create mode 100644 drivers/hwtracing/coresight/coresight-config.h
create mode 100644 drivers/hwtracing/coresight/coresight-etm4x-cfg.c
create mode 100644 drivers/hwtracing/coresight/coresight-etm4x-cfg.h
create mode 100644 drivers/hwtracing/coresight/coresight-syscfg-configfs.c
create mode 100644 drivers/hwtracing/coresight/coresight-syscfg-configfs.h
create mode 100644 drivers/hwtracing/coresight/coresight-syscfg.c
create mode 100644 drivers/hwtracing/coresight/coresight-syscfg.h
--
2.17.1
This printf is the only occurrence outside of the tests. When using
OpenCSD with perf in TUI mode, printfs can corrupt the UI and make
perf's own error messages hard to see. The proper way to print would be
pr_warning() or ui__warning() which aren't available here.
I don't see an easy way of making this work, and I don't think this
printf is that useful either, so remove it.
Signed-off-by: James Clark <james.clark(a)arm.com>
---
decoder/source/trc_frame_deformatter.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/decoder/source/trc_frame_deformatter.cpp b/decoder/source/trc_frame_deformatter.cpp
index 4d46854..affc324 100644
--- a/decoder/source/trc_frame_deformatter.cpp
+++ b/decoder/source/trc_frame_deformatter.cpp
@@ -436,7 +436,6 @@ int TraceFmtDcdImpl::checkForResetFSyncPatterns()
if (num_fsyncs)
{
- printf("Frame deformatter: Found %d FSYNCS\n",num_fsyncs);
if ((num_fsyncs % 4) == 0)
{
// reset the upstream decoders
--
2.28.0