Ensure that etm_perf_add_symlink_sink() is only called for devices
that implement the alloc_buffer operation. This prevents invalid
symlink creation for dummy sinks that do not implement alloc_buffer.
Without this check, perf may attempt to use a dummy sink that lacks
alloc_buffer operationsu to initialise perf's ring buffer, leading
to runtime failures.
Fixes: 9d3ba0b6c0569 ("Coresight: Add coresight dummy driver")
Signed-off-by: Yuanfang Zhang <quic_yuanfang(a)quicinc.com>
---
drivers/hwtracing/coresight/coresight-core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index fa758cc21827552a5c97b6bdd05d22dec4994b22..fddf04c5ee46eb4d559416296f7e85ce6c5689fa 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1374,8 +1374,9 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
goto out_unlock;
}
- if (csdev->type == CORESIGHT_DEV_TYPE_SINK ||
- csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) {
+ if ((csdev->type == CORESIGHT_DEV_TYPE_SINK ||
+ csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) &&
+ sink_ops(csdev)->alloc_buffer) {
ret = etm_perf_add_symlink_sink(csdev);
if (ret) {
---
base-commit: 408c97c4a5e0b634dcd15bf8b8808b382e888164
change-id: 20250630-etm_perf_sink-ee0fcffc6820
Best regards,
--
Yuanfang Zhang <quic_yuanfang(a)quicinc.com>
DEN0154 states that hardware will be allowed to ignore writes to TRB*
registers while the trace buffer is enabled. Add an ISB to ensure that
it's disabled before clearing the other registers.
This is purely defensive because it's expected that arm_trbe_disable()
would be called before teardown which has the required ISB.
Fixes: a2b579c41fe9 ("coresight: trbe: Remove redundant disable call")
Signed-off-by: James Clark <james.clark(a)linaro.org>
---
drivers/hwtracing/coresight/coresight-trbe.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 8267dd1a2130..10f3fb401edf 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -257,6 +257,7 @@ static void trbe_drain_and_disable_local(struct trbe_cpudata *cpudata)
static void trbe_reset_local(struct trbe_cpudata *cpudata)
{
write_sysreg_s(0, SYS_TRBLIMITR_EL1);
+ isb();
trbe_drain_buffer();
write_sysreg_s(0, SYS_TRBPTR_EL1);
write_sysreg_s(0, SYS_TRBBASER_EL1);
---
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
change-id: 20250609-james-cs-trblimitr-isb-523f20d874d6
Best regards,
--
James Clark <james.clark(a)linaro.org>
This series refactors the way CPU IDs are retrieved from the device
tree.
Usually, there is a for loop that goes over every single CPU that can be
avoided. This also reduces the amount of NULL pointer checks in drivers.
I have abstracted away that loop and introduced a new function
(of_cpu_node_to_id) for this.
This patchset is a subset of [1], where I removed content and patches
relevant to hyper-threaded cores for DT. Based on the discussion, the
code refactor is still useful, hence this patchset.
[1] https://lore.kernel.org/all/20250512080715.82-1-alireza.sanaee@huawei.com/
Alireza Sanaee (5):
of: add infra for finding CPU id from phandle
arch_topology: update CPU map to use the new API
coresight: cti: Use of_cpu_phandle_to_id for grabbing CPU id
coresight: Use of_cpu_phandle_to_id for grabbing CPU id
perf/arm-dsu: refactor cpu id retrieval via new API
of_cpu_phandle_to_id
drivers/base/arch_topology.c | 12 ++++----
.../coresight/coresight-cti-platform.c | 15 ++--------
.../hwtracing/coresight/coresight-platform.c | 14 ++-------
drivers/of/cpu.c | 29 +++++++++++++++++++
drivers/perf/arm_dsu_pmu.c | 6 ++--
include/linux/of.h | 9 ++++++
6 files changed, 51 insertions(+), 34 deletions(-)
--
2.43.0
On Wed, Jul 02, 2025 at 12:05:10PM +0100, Yeoreum Yun wrote:
[...]
> > @@ -445,13 +445,37 @@ static int etm4_enable_trace_unit(struct etmv4_drvdata *drvdata)
> > etm4x_relaxed_write32(csa, TRCRSR_TA, TRCRSR);
> >
> > etm4x_allow_trace(drvdata);
> > +
> > + /*
> > + * According to software usage PKLXF in Arm ARM (ARM DDI 0487 L.a),
> > + * execute a Context synchronization event to guarantee the trace unit
> > + * will observe the new values of the System registers.
> > + */
> > + if (!csa->io_mem)
> > + isb();
> > +
>
> But, when write to SYS_TRFCR_EL1 in etm4x_allow_trace(), it already does
> isb(). Is it redundant?
Good point. It is not sufficient. As a system register writing in
kvm_tracing_set_el1_configuration(), this is why adds a ISB here.
Thanks,
Leo
Hi Levi,
On Wed, Jul 02, 2025 at 12:10:17PM +0100, Yeoreum Yun wrote:
[...]
> > @@ -579,6 +572,13 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
> >
> > if (!drvdata->paused)
> > rc = etm4_enable_trace_unit(drvdata);
> > +
> > + /*
> > + * As recommended by section 4.3.7 (Synchronization of register updates)
> > + * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
> > + * ISB instruction after programming the trace unit registers.
> > + */
> > + isb();
>
> But according to 4.3.7 ("Synchronization when using memory-mapped
> interface"), doesn't it need to dsb like:
>
> if (csa->iomem)
> dsb(sy);
> isb();
>
> Or am I missing something?
Section 4.3.7 suggests using a DSB barrier to ensure that writes have
completed in MMIO mode. It also mentions an alternative:
"If the memory is marked as Device-nGnRE or stronger, read back the
value of any register in the trace unit. This relies on the peripheral
coherence order defined in the Arm architecture."
In the etm4_{enable|disable}_trace_unit() functions, each time the
TRCPRGCTLR register is written, the driver polls bits in TRCSTATR.
This acts as synchronization using read-after-write (RAW), which is
exactly the approach suggested above.
This is why we don't need DSB() anymore.
Thanks,
Leo
Hi Levi,
On Wed, Jul 02, 2025 at 10:49:01AM +0100, Yeoreum Yun wrote:
> Hi Leo,
>
> > {
> > - return local_cmpxchg(&csdev->mode, CS_MODE_DISABLED, new_mode) ==
> > - CS_MODE_DISABLED;
> > + int curr = CS_MODE_DISABLED;
> > +
> > + return atomic_try_cmpxchg_acquire(&csdev->mode, &curr, new_mode);
> > }
>
> Just question. why is acquire symentic enough in here?
My understanding is that acquire semantics ensure ordering between
cmpxchg_acquire() and all memory accesses that follow it. However, it
does not guarantee that memory accesses appearing before the acquire
are ordered as well.
This is exactly what we want in the driver. We must ensure to first grab
an active device mode, then it is safe to proceed later operations (e.g.
set configurations in driver data and access registers).
> before this change, local_cmpxchg seems to use full_fenced.
Not really. Arm64 has atomic instruction for cmpxchg, it does not use
full_fenced. It should run into the path of arch_cmpxchg().
Thanks,
Leo
On 2025/6/20 19:53, Jonathan Cameron wrote:
> On Fri, 20 Jun 2025 15:54:10 +0800
> Junhao He <hejunhao3(a)huawei.com> wrote:
>
>> From: Yicong Yang <yangyicong(a)hisilicon.com>
>>
>> tmc_drvdata::reading is used to indicate whether a reading process
>> is performed through /dev/xyz.tmc. Document it.
>>
>> Reviewed-by: James Clark <james.clark(a)linaro.org>
>> Signed-off-by: Yicong Yang <yangyicong(a)hisilicon.com>
>> Signed-off-by: Junhao He <hejunhao3(a)huawei.com>
>> ---
>> drivers/hwtracing/coresight/coresight-tmc.h | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
>> index 6541a27a018e..3ca0d40c580d 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc.h
>> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
>> @@ -220,6 +220,7 @@ struct tmc_resrv_buf {
>> * @pid: Process ID of the process that owns the session that is using
>> * this component. For example this would be the pid of the Perf
>> * process.
>> + * @reading: buffer's in the reading through "/dev/xyz.tmc" entry
> Hi,
>
> Perhaps reword:
>
> "buffer is being read through "/dev/xyz.tmc" entry" or
> "buffer read in progress through "/dev/xyz.tmc" entry"
>
> I've not checked what this actually means - just looking at what you have here.
Will update this. Thanks
Best regards,
Junhao.
>> * @stop_on_flush: Stop on flush trigger user configuration.
>> * @buf: Snapshot of the trace data for ETF/ETB.
>> * @etr_buf: details of buffer used in TMC-ETR
> .
>