CoreSight September 2021

coresight@lists.linaro.org

12 participants
48 discussions

[PATCH v4 0/5] coresight: TRBE and Self-Hosted trace fixes

by Suzuki K Poulose

Here is the updated patches from the v3 of the series [0], parts of which have been queued to coresight/next. This series now applies on the coresight/next branch. Changes since v3: - Split the spurious IRQ handling patch to : a) coresight: trbe: irq handler: Do not disable TRBE if no action is needed b) coresight: trbe: Fix handling of spurious interrupts - Added a helper to mark the ring buffer when there is WRAP event and added a comment to explain. [0] https://lkml.kernel.org/r/20210914102641.1852544-1-suzuki.poulose@arm.com Suzuki K Poulose (5): coresight: trbe: irq handler: Do not disable TRBE if no action is needed coresight: trbe: Fix handling of spurious interrupts coresight: trbe: Do not truncate buffer on IRQ coresight: trbe: End the AUX handle on truncation coresight: trbe: Prohibit trace before disabling TRBE .../coresight/coresight-self-hosted-trace.h | 4 +- drivers/hwtracing/coresight/coresight-trbe.c | 96 ++++++++++++------- 2 files changed, 64 insertions(+), 36 deletions(-) -- 2.24.1

2 years, 7 months

Re: [PATCH v2 2/2] arm64: dts: qcom: sm8250: Add Coresight support

by Suzuki K Poulose

On 23/09/2021 10:24, Tao Zhang wrote: > On Tue, Sep 21, 2021 at 05:35:37PM +0100, Suzuki K Poulose wrote: >> Hi Tao >> >> Are there no sinks at all on this platform ? I had this question on the >> previous series. How is CoreSight useful on this platform otherwise ? >> >> On 13/09/2021 07:40, Tao Zhang wrote: > ETF/ETR are the sinks on this target. And I have added the ETF to this > device tree file. Since the ETR needs SMMU support on this target and > SMMU has not been supported for now. I will add the ETR to device tree > later if the SMMU is ready for this platform. Thanks. That is fine. Btw, these sort of additional information could be added to the cover letter to give a better picture of what you are trying to do and why. >>> Add the basic coresight components found on Qualcomm SM8250 Soc. The >>> basic coresight components include ETF, ETMs,STM and the related >>> funnels. >>> >>> Signed-off-by: Tao Zhang <quic_taozha(a)quicinc.com> >>> --- >>> arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 442 ++++++++++++++++++++++- >>> 1 file changed, 438 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts >>> index 8ac96f8e79d4..9c8f87d80afc 100644 >>> --- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts >>> +++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts >>> @@ -222,11 +222,445 @@ >>> regulator-max-microvolt = <1800000>; >>> regulator-always-on; >>> }; >>> -}; >>> -&adsp { >>> - status = "okay"; >>> - firmware-name = "qcom/sm8250/adsp.mbn"; >> >> Unrelated change ? Please keep it separate from the CoreSight changes. >> >> Suzuki > I combined this change and ETM pid change into one seies because the ETM > pid change validation needs ETM support. If there is no ETM > configuration in the device tree, ETM pid change can not be verified. > Do you think it would be better to separate them? Do I need to resubmit > to separate them into two separate patches? No, I am asking about the lines removed above. i.e, -&adsp { - status = "okay"; - firmware-name = "qcom/sm8250/adsp.mbn"; It doesn't seem to be added back in the patch. So that means, the DT lost those lines, without any mention of that in the description of the patch. Moreover, the lines do not look like they were anything to do with CoreSight. This is why I mentioned they look like "unrelated". Suzuki

2 years, 7 months

Re: [PATCH v2 0/2] Add Coresight support for RB5 board

by Suzuki K Poulose

On 23/09/2021 10:16, Tao Zhang wrote: > On Tue, Sep 21, 2021 at 05:31:34PM +0100, Suzuki K Poulose wrote: >> Hi Tao >> >> On 13/09/2021 07:40, Tao Zhang wrote: >>> This series adds Coresight support for SM8250 Soc on RB5 board. >>> It is composed of two elements. >>> a) Add ETM PID for Kryo-5XX. >>> b) Add coresight support to DTS for RB5. >>> >>> This series applies to coresight/next >>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git >>> >> >> Please could you mention what has changed since the previous version >> in the cover letter ? >> >> Kind regards >> Suzuki >> > The version 2 of the series add more comments "Cortex-A77" for ETM pid. > Do I need to rewrite the cover letter and then resubmit it for review? No need to resubmit the series for reveiwing the patch. Ultimately patches are what matters. But it is a good practise to keep the changelog in the cover letter for a revision of a series. That helps the reviewers to understand what has changed and we all are able to spend our time efficiently. So, please follow that if possible. Thanks Suzuki

2 years, 7 months

[PATCH v3 00/10] coresight: TRBE and Self-Hosted trace fixes

by Suzuki K Poulose

This series fixes the following issues with the TRBE and Self-Hosted trace for CoreSight. The Self-hosted trace filter control registers are now save-restored across CPU PM event. And more importantly the Trace Filtering is now used to control per ETM session (rather than allowing the trace throughout the life time of the system). i.e, ETM configuration of the given run is used to enforce trace filtering (TRFCR) along with the Trace Exclusion controls in TRCVICTLR. For the TRBE, we were using the TRUNCATED flag in the AUX buffer on every IRQ to indicate that we may have lost a few bytes of trace. But this causes the event to be disabled until the userspace re-enables it back, even when there is space left in the ring buffer. To make things worse, we were restarting the AUX handle, which would soon be disabled, potentially creating 0 sized records (without truncation), which the perf tool tends to ignore. This might cause the event to be disabled permanently. Also, sometimes we leave the buffer TRUNCATED, but delay the closing of the handle to event schedule out, which could cause significant black out in the trace capture. This was reported by Tamas Zsoldos. This series removes the use of TRUNCATED flag for every IRQ. Instead, uses the COLLISION flag to indicate that there was a WRAP event in the trace collection. A patch to report the COLLISIONs has been queued for the perf tool. The TRUNCATED is only used if we really run out of space in the buffer. And also we make sure the "handle" is closed immediately on TRUNCATED case, which triggers the userspace to take action. The core perf layer has been hardened to handle this case where a "handle" is closed out. Finally, we make sure that the CPU trace is prohibited, when the TRBE is left disabled. The ETE/ETM driver will program the Trace Filtering appropriately since we do this dynamically now with the first half of the series. Applies on coresigh/next. The tree is also available here: https://gitlab.arm.com/linux-arm/linux-skp/-/tree/coresight/trbe-self-hoste… Changes since v2 [1]: - Address comments from Anshuman - Perf tool patch queued, to report COLLISIONs in a session. Changes since v1 [0]: - Moved TRFCR related accessors to a new header file - Following a discussion, dropped the TRUNCATED flag from the TRBE IRQ handler on WRAP. Instead mark COLLISION. - Added new patches to harden the ETM perf layer to handle an error in the sink driver. - Fix TRBE spurious IRQ handling - Cleanup TRBE driver to make the "TRUNCATE" cases managed at a central place. [0] https://lkml.kernel.org/r/20210712113830.2803257-1-suzuki.poulose@arm.com [1] https://lkml.kernel.org/r/20210723124611.3828908-1-suzuki.poulose@arm.com Suzuki K Poulose (10): coresight: etm4x: Save restore TRFCR_EL1 coresight: etm4x: Use Trace Filtering controls dynamically coresight: etm-pmu: Ensure the AUX handle is valid coresight: trbe: Ensure the format flag is set always coresight: trbe: Drop duplicate TRUNCATE flags coresight: trbe: Fix handling of spurious interrupts coresight: trbe: Do not truncate buffer on IRQ coresight: trbe: Unify the enabling sequence coresight: trbe: End the AUX handle on truncation coresight: trbe: Prohibit trace before disabling TRBE .../hwtracing/coresight/coresight-etm-perf.c | 27 ++++- .../coresight/coresight-etm4x-core.c | 100 ++++++++++++---- drivers/hwtracing/coresight/coresight-etm4x.h | 9 +- .../coresight/coresight-self-hosted-trace.h | 34 ++++++ drivers/hwtracing/coresight/coresight-trbe.c | 111 ++++++++++-------- 5 files changed, 200 insertions(+), 81 deletions(-) create mode 100644 drivers/hwtracing/coresight/coresight-self-hosted-trace.h -- 2.24.1

2 years, 8 months

Re: [PATCH v3 05/10] coresight: trbe: Drop duplicate TRUNCATE flags

by Mathieu Poirier

On Wed, Sep 15, 2021 at 10:56:17AM +0530, Anshuman Khandual wrote: > > > On 9/14/21 3:56 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> > > Reviewed-by: Anshuman Khandual <anshuman.khandual(a)arm.com> > > A small nit though. > > > --- > > Change since v2: > > - Added the flag for trbe_handle_spurious() - Anshuman > > --- > > drivers/hwtracing/coresight/coresight-trbe.c | 8 ++------ > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c > > index 81bf183a73a1..5297b11f26b7 100644 > > --- a/drivers/hwtracing/coresight/coresight-trbe.c > > +++ b/drivers/hwtracing/coresight/coresight-trbe.c > > @@ -252,13 +252,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. > > Should not this comment be moved, where PERF_AUX_FLAG_TRUNCATED > gets marked after this change ? I moved the comment and applied this patch. > > > */ > > - 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); > > @@ -360,7 +356,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; > > } > > > > @@ -688,6 +683,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); > >

2 years, 8 months

[PATCH v2 0/1] Don't immediately close events that are run on invalid CPU/sink combos

by James Clark

Changes since v1: * Replace goto with return after failure to maintain original failure behaviour * Add Mathieu and Suzuki's reviewed-by tags * Repeated testing James Clark (1): coresight: Don't immediately close events that are run on invalid CPU/sink combos .../hwtracing/coresight/coresight-etm-perf.c | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) -- 2.28.0

2 years, 8 months

[PATCH] coresight: Don't immediately close events that are run on invalid CPU/sink combos

by James Clark

When a traced process runs on a CPU that can't reach the selected sink, the event will be stopped with PERF_HES_STOPPED. This means that even if the process migrates to a valid CPU, tracing will not resume. This can be reproduced (on N1SDP) by using taskset to start the process on CPU 0, and then switching it to CPU 2 (ETF 1 is only reachable from CPU 2): taskset --cpu-list 0 ./perf record -e cs_etm/@tmc_etf1/ --per-thread -- taskset --cpu-list 2 ls This produces a single 0 length AUX record, and then no more trace: 0x3c8 [0x30]: PERF_RECORD_AUX offset: 0 size: 0 flags: 0x1 [T] After the fix, the same command produces normal AUX records. The perf self test "89: Check Arm CoreSight trace data recording and synthesized samples" no longer fails intermittently. This was because the taskset in the test is after the fork, so there is a period where the task is scheduled on a random CPU rather than forced to a valid one. Specifically selecting an invalid CPU will still result in a failure to open the event because it will never produce trace: ./perf record -C 2 -e cs_etm/@tmc_etf0/ failed to mmap with 12 (Cannot allocate memory) The only scenario that has changed is if the CPU mask has a valid CPU sink combo in it. Testing ======= * Coresight self test passes consistently: ./perf test Coresight * CPU wide mode still produces trace: ./perf record -e cs_etm// -a * Invalid -C options still fail to open: ./perf record -C 2,3 -e cs_etm/@tmc_etf0/ failed to mmap with 12 (Cannot allocate memory) * Migrating a task to a valid sink/CPU now produces trace: taskset --cpu-list 0 ./perf record -e cs_etm/@tmc_etf1/ --per-thread -- taskset --cpu-list 2 ls * If the task remains on an invalid CPU, no trace is emitted: taskset --cpu-list 0 ./perf record -e cs_etm/@tmc_etf1/ --per-thread -- ls Signed-off-by: James Clark <james.clark(a)arm.com> --- .../hwtracing/coresight/coresight-etm-perf.c | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 8ebd728d3a80..79346f0f0e0b 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -452,9 +452,14 @@ static void etm_event_start(struct perf_event *event, int flags) * sink from this ETM. We can't do much in this case if * the sink was specified or hinted to the driver. For * now, simply don't record anything on this ETM. + * + * As such we pretend that everything is fine, and let + * it continue without actually tracing. The event could + * continue tracing when it moves to a CPU where it is + * reachable to a sink. */ if (!cpumask_test_cpu(cpu, &event_data->mask)) - goto fail_end_stop; + goto out; path = etm_event_cpu_path(event_data, cpu); /* We need a sink, no need to continue without one */ @@ -466,16 +471,15 @@ static void etm_event_start(struct perf_event *event, int flags) if (coresight_enable_path(path, CS_MODE_PERF, handle)) goto fail_end_stop; - /* Tell the perf core the event is alive */ - event->hw.state = 0; - /* Finally enable the tracer */ if (source_ops(csdev)->enable(csdev, event, CS_MODE_PERF)) goto fail_disable_path; +out: + /* Tell the perf core the event is alive */ + event->hw.state = 0; /* Save the event_data for this ETM */ ctxt->event_data = event_data; -out: return; fail_disable_path: @@ -517,6 +521,19 @@ static void etm_event_stop(struct perf_event *event, int mode) if (WARN_ON(!event_data)) return; + /* + * Check if this ETM was allowed to trace, as decided at + * etm_setup_aux(). If it wasn't allowed to trace, then + * nothing needs to be torn down other than outputting a + * zero sized record. + */ + if (handle->event && (mode & PERF_EF_UPDATE) && + !cpumask_test_cpu(cpu, &event_data->mask)) { + event->hw.state = PERF_HES_STOPPED; + perf_aux_output_end(handle, 0); + return; + } + if (!csdev) return; -- 2.28.0

2 years, 8 months

Re: [PATCH v2 11/17] arm64: errata: Add workaround for TSB flush failures

by Suzuki K Poulose

Hi Anshuman On 22/09/2021 08:39, Anshuman Khandual wrote: > > > On 9/21/21 7:11 PM, Suzuki K Poulose wrote: >> Arm Neoverse-N2 (#2067961) and Cortex-A710 (#2054223) suffers >> from errata, where a TSB (trace synchronization barrier) >> fails to flush the trace data completely, when executed from >> a trace prohibited region. In Linux we always execute it >> after we have moved the PE to trace prohibited region. So, >> we can apply the workaround everytime a TSB is executed. > > s/everytime/every time Ack > >> >> The work around is to issue two TSB consecutively. >> >> NOTE: This errata is defined as LOCAL_CPU_ERRATUM, implying >> that a late CPU could be blocked from booting if it is the >> first CPU that requires the workaround. This is because we >> do not allow setting a cpu_hwcaps after the SMP boot. The >> other alternative is to use "this_cpu_has_cap()" instead >> of the faster system wide check, which may be a bit of an >> overhead, given we may have to do this in nvhe KVM host >> before a guest entry. >> >> Cc: Will Deacon <will(a)kernel.org> >> Cc: Catalin Marinas <catalin.marinas(a)arm.com> >> Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org> >> Cc: Mike Leach <mike.leach(a)linaro.org> >> Cc: Mark Rutland <mark.rutland(a)arm.com> >> Cc: Anshuman Khandual <anshuman.khandual(a)arm.com> >> Cc: Marc Zyngier <maz(a)kernel.org> >> Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com> >> --- ... >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >> index eac4030322df..0764774e12bb 100644 >> --- a/arch/arm64/Kconfig >> +++ b/arch/arm64/Kconfig >> @@ -705,6 +705,37 @@ config ARM64_ERRATUM_2139208 >> >> If unsure, say Y. >> >> +config ARM64_WORKAROUND_TSB_FLUSH_FAILURE >> + bool >> + >> +config ARM64_ERRATUM_2054223 >> + bool "Cortex-A710: 2054223: workaround TSB instruction failing to flush trace" >> + default y >> + help >> + Enable workaround for ARM Cortex-A710 erratum 2054223 >> + >> + Affected cores may fail to flush the trace data on a TSB instruction, when >> + the PE is in trace prohibited state. This will cause losing a few bytes >> + of the trace cached. >> + >> + Workaround is to issue two TSB consecutively on affected cores. >> + >> + If unsure, say Y. >> + >> +config ARM64_ERRATUM_2067961 >> + bool "Neoverse-N2: 2067961: workaround TSB instruction failing to flush trace" >> + default y >> + help >> + Enable workaround for ARM Neoverse-N2 erratum 2067961 >> + >> + Affected cores may fail to flush the trace data on a TSB instruction, when >> + the PE is in trace prohibited state. This will cause losing a few bytes >> + of the trace cached. >> + >> + Workaround is to issue two TSB consecutively on affected cores. > > Like I had mentioned in the previous patch, these descriptions here could > be just factored out inside ARM64_WORKAROUND_TSB_FLUSH_FAILURE instead. Please see my response there. > >> + >> + If unsure, say Y. >> + >> config CAVIUM_ERRATUM_22375 >> bool "Cavium erratum 22375, 24313" >> default y >> diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h >> index 451e11e5fd23..1c5a00598458 100644 >> --- a/arch/arm64/include/asm/barrier.h >> +++ b/arch/arm64/include/asm/barrier.h >> @@ -23,7 +23,7 @@ >> #define dsb(opt) asm volatile("dsb " #opt : : : "memory") >> >> #define psb_csync() asm volatile("hint #17" : : : "memory") >> -#define tsb_csync() asm volatile("hint #18" : : : "memory") >> +#define __tsb_csync() asm volatile("hint #18" : : : "memory") >> #define csdb() asm volatile("hint #20" : : : "memory") >> >> #ifdef CONFIG_ARM64_PSEUDO_NMI >> @@ -46,6 +46,20 @@ >> #define dma_rmb() dmb(oshld) >> #define dma_wmb() dmb(oshst) >> >> + >> +#define tsb_csync() \ >> + do { \ >> + /* \ >> + * CPUs affected by Arm Erratum 2054223 or 2067961 needs \ >> + * another TSB to ensure the trace is flushed. The barriers \ >> + * don't have to be strictly back to back, as long as the \ >> + * CPU is in trace prohibited state. \ >> + */ \ >> + if (cpus_have_final_cap(ARM64_WORKAROUND_TSB_FLUSH_FAILURE)) \ >> + __tsb_csync(); \ >> + __tsb_csync(); \ >> + } while (0) >> + >> /* >> * Generate a mask for array_index__nospec() that is ~0UL when 0 <= idx < sz >> * and 0 otherwise. >> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c >> index ccd757373f36..bdbeac75ead6 100644 >> --- a/arch/arm64/kernel/cpu_errata.c >> +++ b/arch/arm64/kernel/cpu_errata.c >> @@ -352,6 +352,18 @@ static const struct midr_range trbe_overwrite_fill_mode_cpus[] = { >> }; >> #endif /* CONFIG_ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE */ >> >> +#ifdef CONFIG_ARM64_WORKAROUND_TSB_FLUSH_FAILURE >> +static const struct midr_range tsb_flush_fail_cpus[] = { >> +#ifdef CONFIG_ARM64_ERRATUM_2067961 >> + MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N2), >> +#endif >> +#ifdef CONFIG_ARM64_ERRATUM_2054223 >> + MIDR_ALL_VERSIONS(MIDR_CORTEX_A710), >> +#endif >> + {}, >> +}; >> +#endif /* CONFIG_ARM64_WORKAROUND_TSB_FLUSH_FAILURE */ >> + >> const struct arm64_cpu_capabilities arm64_errata[] = { >> #ifdef CONFIG_ARM64_WORKAROUND_CLEAN_CACHE >> { >> @@ -558,6 +570,13 @@ const struct arm64_cpu_capabilities arm64_errata[] = { >> .type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE, >> CAP_MIDR_RANGE_LIST(trbe_overwrite_fill_mode_cpus), >> }, >> +#endif >> +#ifdef CONFIG_ARM64_WORKAROUND_TSB_FLUSH_FAILRE >> + { >> + .desc = "ARM erratum 2067961 or 2054223", >> + .capability = ARM64_WORKAROUND_TSB_FLUSH_FAILURE, >> + ERRATA_MIDR_RANGE_LIST(tsb_flush_fail_cpus), >> + }, >> #endif >> { >> } >> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps >> index 1ccb92165bd8..2102e15af43d 100644 >> --- a/arch/arm64/tools/cpucaps >> +++ b/arch/arm64/tools/cpucaps >> @@ -54,6 +54,7 @@ WORKAROUND_1463225 >> WORKAROUND_1508412 >> WORKAROUND_1542419 >> WORKAROUND_TRBE_OVERWRITE_FILL_MODE >> +WORKAROUND_TSB_FLUSH_FAILURE >> WORKAROUND_CAVIUM_23154 >> WORKAROUND_CAVIUM_27456 >> WORKAROUND_CAVIUM_30115 >> > > This adds all the required bits of these erratas in a single patch, > where as the previous work around had split all the required pieces > into multiple patches. Could we instead follow the same standard in > both the places ? We could do this for this particular erratum as the work around is within the arm64 kernel code, unlike the other ones - where the TRBE driver needs a change. So, there is a kind of dependency for the other two, which we don't in this particular case. i.e, TRBE driver needs a cpucap number to implement the work around -> The arm64 kernel must define one, which we cant advertise yet until we have a TRBE work around. Thus, they follow a 3 step model. - Define CPUCAP erratum - TRBE driver work around - Finally advertise to the user. I don't think this one needs that. Suzuki >

2 years, 8 months

Re: [PATCH v2 14/17] coresight: trbe: Make sure we have enough space

by Suzuki K Poulose

On 22/09/2021 10:58, Anshuman Khandual wrote: > > > On 9/21/21 7:11 PM, Suzuki K Poulose wrote: >> The TRBE driver makes sure that there is enough space for a meaningful >> run, otherwise pads the given space and restarts the offset calculation >> once. But there is no guarantee that we may find space or hit "no space". > > So what happens currently when it neither finds the required minimum buffer > space for a meaningful run nor does it hit the "no space" scenario ? It tries once today and assumes that it will either hit : - No space OR - Enough space which is reasonable, given the minimum space needed is a few bytes. But this may no longer be true with other erratum workaround. > >> Make sure that we repeat the step until, either : >> - We have the minimum space >> OR >> - There is NO space at all. >> >> 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 | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c >> index 3373f4e2183b..02f9e00e2091 100644 >> --- a/drivers/hwtracing/coresight/coresight-trbe.c >> +++ b/drivers/hwtracing/coresight/coresight-trbe.c >> @@ -451,10 +451,14 @@ static unsigned long trbe_normal_offset(struct perf_output_handle *handle) >> * If the head is too close to the limit and we don't >> * have space for a meaningful run, we rather pad it >> * and start fresh. >> + * >> + * We might have to do this more than once to make sure >> + * we have enough required space. > > OR no space at all, as explained in the commit message. > Hence this comment needs an update. > >> */ >> - if (limit && ((limit - head) < trbe_min_trace_buf_size(handle))) { >> + while (limit && ((limit - head) < trbe_min_trace_buf_size(handle))) { >> trbe_pad_buf(handle, limit - head); >> limit = __trbe_normal_offset(handle); >> + head = PERF_IDX2OFF(handle->head, buf); > > Should the loop be bound with a retry limit as well ? No. We will eventually hit No-space as we keep on padding the buffer. Suzuki

2 years, 8 months

Re: [PATCH v2 10/17] arm64: Enable workaround for TRBE overwrite in FILL mode

by Suzuki K Poulose

On 22/09/2021 08:23, Anshuman Khandual wrote: > > > On 9/21/21 7:11 PM, Suzuki K Poulose wrote: >> Now that we have the work around implmented in the TRBE >> driver, add the Kconfig entries and document the errata. >> >> Cc: Mark Rutland <mark.rutland(a)arm.com> >> Cc: Will Deacon <will(a)kernel.org> >> Cc: Catalin Marinas <catalin.marinas(a)arm.com> >> 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> >> --- >> Documentation/arm64/silicon-errata.rst | 4 +++ >> arch/arm64/Kconfig | 39 ++++++++++++++++++++++++++ >> 2 files changed, 43 insertions(+) >> >> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst >> index d410a47ffa57..2f99229d993c 100644 >> --- a/Documentation/arm64/silicon-errata.rst >> +++ b/Documentation/arm64/silicon-errata.rst >> @@ -92,12 +92,16 @@ stable kernels. >> +----------------+-----------------+-----------------+-----------------------------+ >> | ARM | Cortex-A77 | #1508412 | ARM64_ERRATUM_1508412 | >> +----------------+-----------------+-----------------+-----------------------------+ >> +| ARM | Cortex-A710 | #2119858 | ARM64_ERRATUM_2119858 | >> ++----------------+-----------------+-----------------+-----------------------------+ >> | ARM | Neoverse-N1 | #1188873,1418040| ARM64_ERRATUM_1418040 | >> +----------------+-----------------+-----------------+-----------------------------+ >> | ARM | Neoverse-N1 | #1349291 | N/A | >> +----------------+-----------------+-----------------+-----------------------------+ >> | ARM | Neoverse-N1 | #1542419 | ARM64_ERRATUM_1542419 | >> +----------------+-----------------+-----------------+-----------------------------+ >> +| ARM | Neoverse-N2 | #2139208 | ARM64_ERRATUM_2139208 | >> ++----------------+-----------------+-----------------+-----------------------------+ >> | ARM | MMU-500 | #841119,826419 | N/A | >> +----------------+-----------------+-----------------+-----------------------------+ >> +----------------+-----------------+-----------------+-----------------------------+ >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >> index 077f2ec4eeb2..eac4030322df 100644 >> --- a/arch/arm64/Kconfig >> +++ b/arch/arm64/Kconfig >> @@ -666,6 +666,45 @@ config ARM64_ERRATUM_1508412 >> >> If unsure, say Y. >> >> +config ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE >> + bool >> + >> +config ARM64_ERRATUM_2119858 >> + bool "Cortex-A710: 2119858: workaround TRBE overwriting trace data in FILL mode" >> + default y >> + depends on CORESIGHT_TRBE >> + select ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE >> + help >> + This option adds the workaround for ARM Cortex-A710 erratum 2119858. >> + >> + Affected Cortex-A710 cores could overwrite upto 3 cache lines of trace >> + data at the base of the buffer (ponited by TRBASER_EL1) in FILL mode in >> + the event of a WRAP event. >> + >> + Work around the issue by always making sure we move the TRBPTR_EL1 by >> + 256bytes before enabling the buffer and filling the first 256bytes of >> + the buffer with ETM ignore packets upon disabling. >> + >> + If unsure, say Y. >> + >> +config ARM64_ERRATUM_2139208 >> + bool "Neoverse-N2: 2139208: workaround TRBE overwriting trace data in FILL mode" >> + default y >> + depends on CORESIGHT_TRBE >> + select ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE >> + help >> + This option adds the workaround for ARM Neoverse-N2 erratum 2139208. >> + >> + Affected Neoverse-N2 cores could overwrite upto 3 cache lines of trace >> + data at the base of the buffer (ponited by TRBASER_EL1) in FILL mode in > > s/ponited/pointed > >> + the event of a WRAP event. >> + >> + Work around the issue by always making sure we move the TRBPTR_EL1 by >> + 256bytes before enabling the buffer and filling the first 256bytes of >> + the buffer with ETM ignore packets upon disabling. >> + >> + If unsure, say Y. >> + >> config CAVIUM_ERRATUM_22375 >> bool "Cavium erratum 22375, 24313" >> default y >> > > The real errata problem description for both these erratums are exactly > the same. Rather a more generalized description should be included for > the ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE, which abstracts out the > problem and a corresponding solution that is implemented in the driver. > This should also help us reduce current redundancy. > The issue is what a user wants to see. A user who wants to configure the kernel specifically for a given CPU (think embedded systems), they would want to hand pick the errata for the particular CPU. So, moving the help text to an implicitly selected Kconfig symbol. I would rather keep this as it is to keep it user friendly. This doesn't affect the code size anyways. The other option is to remove all the CPU specific Kconfig symbols and update the "title" to reflect both the CPU/erratum numbers. Kind regards Suzuki

2 years, 8 months

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight September 2021