Re: [PATCH V3 11/14] coresight: sink: Add TRBE driver

Anshuman Khandual
2021-02-08T04:21:21Z

On 2/5/21 11:23 PM, Mathieu Poirier wrote:
> On Wed, Jan 27, 2021 at 02:25:35PM +0530, Anshuman Khandual wrote:
>> Trace Buffer Extension (TRBE) implements a trace buffer per CPU which is
>> accessible via the system registers. The TRBE supports different addressing
>> modes including CPU virtual address and buffer modes including the circular
>> buffer mode. The TRBE buffer is addressed by a base pointer (TRBBASER_EL1),
>> an write pointer (TRBPTR_EL1) and a limit pointer (TRBLIMITR_EL1). But the
>> access to the trace buffer could be prohibited by a higher exception level
>> (EL3 or EL2), indicated by TRBIDR_EL1.P. The TRBE can also generate a CPU
>> private interrupt (PPI) on address translation errors and when the buffer
>> is full. Overall implementation here is inspired from the Arm SPE driver.
>>
>
> I got this message when applying the patch:
>
> Applying: coresight: sink: Add TRBE driver
> .git/rebase-apply/patch:76: new blank line at EOF.
> +
> warning: 1 line adds whitespace errors.

It could be the additional blank line at the end of documentation file
i.e Documentation/trace/coresight/coresight-trbe.rst, will drop it.

>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>> Changes in V3:
>>
>> - Added new DT bindings document TRBE.yaml
>> - Changed TRBLIMITR_TRIG_MODE_SHIFT from 2 to 3
>> - Dropped isb() from trbe_reset_local()
>> - Dropped gap between (void *) and buf->trbe_base
>> - Changed 'int' to 'unsigned int' in is_trbe_available()
>> - Dropped unused function set_trbe_running(), set_trbe_virtual_mode(),
>> set_trbe_enabled() and set_trbe_limit_pointer()
>> - Changed get_trbe_flag_update(), is_trbe_programmable() and
>> get_trbe_address_align() to accept TRBIDR value
>> - Changed is_trbe_running(), is_trbe_abort(), is_trbe_wrap(), is_trbe_trg(),
>> is_trbe_irq(), get_trbe_bsc() and get_trbe_ec() to accept TRBSR value
>> - Dropped snapshot mode condition in arm_trbe_alloc_buffer()
>> - Exit arm_trbe_init() when arm64_kernel_unmapped_at_el0() is enabled
>> - Compute trbe_limit before trbe_write to get the updated handle
>> - Added trbe_stop_and_truncate_event()
>> - Dropped trbe_handle_fatal()
>>
>> Documentation/trace/coresight/coresight-trbe.rst | 39 +
>> arch/arm64/include/asm/sysreg.h | 1 +
>> drivers/hwtracing/coresight/Kconfig | 11 +
>> drivers/hwtracing/coresight/Makefile | 1 +
>> drivers/hwtracing/coresight/coresight-trbe.c | 1023 ++++++++++++++++++++++
>> drivers/hwtracing/coresight/coresight-trbe.h | 160 ++++
>> 6 files changed, 1235 insertions(+)
>> create mode 100644 Documentation/trace/coresight/coresight-trbe.rst
>> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.c
>> create mode 100644 drivers/hwtracing/coresight/coresight-trbe.h
>>
>> diff --git a/Documentation/trace/coresight/coresight-trbe.rst b/Documentation/trace/coresight/coresight-trbe.rst
>> new file mode 100644
>> index 0000000..1cbb819
>> --- /dev/null
>> +++ b/Documentation/trace/coresight/coresight-trbe.rst
>> @@ -0,0 +1,39 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +==============================
>> +Trace Buffer Extension (TRBE).
>> +==============================
>> +
>> + :Author: Anshuman Khandual <anshuman.khandual@arm.com>
>> + :Date: November 2020
>> +
>> +Hardware Description
>> +--------------------
>> +
>> +Trace Buffer Extension (TRBE) is a percpu hardware which captures in system
>> +memory, CPU traces generated from a corresponding percpu tracing unit. This
>> +gets plugged in as a coresight sink device because the corresponding trace
>> +genarators (ETE), are plugged in as source device.
>> +
>> +The TRBE is not compliant to CoreSight architecture specifications, but is
>> +driven via the CoreSight driver framework to support the ETE (which is
>> +CoreSight compliant) integration.
>> +
>> +Sysfs files and directories
>> +---------------------------
>> +
>> +The TRBE devices appear on the existing coresight bus alongside the other
>> +coresight devices::
>> +
>> + >$ ls /sys/bus/coresight/devices
>> + trbe0 trbe1 trbe2 trbe3
>> +
>> +The ``trbe<N>`` named TRBEs are associated with a CPU.::
>> +
>> + >$ ls /sys/bus/coresight/devices/trbe0/
>> + align dbm
>> +
>> +*Key file items are:-*
>> + * ``align``: TRBE write pointer alignment
>> + * ``dbm``: TRBE updates memory with access and dirty flags
>> +
>
> Please add documentation for these, the same way it was done for all the other CS
> components [1].
>
> [1]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing
> (sysfs-bus-coresight-device-xyz)

Sure, will add the following new sysfs doc file in this regard.
Marked the KernelVersion as 5.12, will change if required.

new file mode 100644
index 0000000..5cb090f
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-trbe
@@ -0,0 +1,14 @@
+What: /sys/bus/coresight/devices/trbe<cpu>/align
+Date: Feb 2021
+KernelVersion: 5.12
+Contact: Anshuman Khandual <anshuman.khandual@arm.com>
+Description: (Read) Shows the TRBE write pointer alignment. This value
+ is fetched from the TRBIDR register.
+
+What: /sys/bus/coresight/devices/trbe<cpu>/dbm
+Date: Feb 2021
+KernelVersion: 5.12
+Contact: Anshuman Khandual <anshuman.khandual@arm.com>
+Description: (Read) Shows if TRBE updates in the memory are with access
+ and dirty flag updates as well. This value is fetched from
+ the TRBIDR register.

>
>> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
>> index 85ae4db..9e2e9b7 100644
>> --- a/arch/arm64/include/asm/sysreg.h
>> +++ b/arch/arm64/include/asm/sysreg.h
>> @@ -97,6 +97,7 @@
>> #define SET_PSTATE_UAO(x) __emit_inst(0xd500401f | PSTATE_UAO | ((!!x) << PSTATE_Imm_shift))
>> #define SET_PSTATE_SSBS(x) __emit_inst(0xd500401f | PSTATE_SSBS | ((!!x) << PSTATE_Imm_shift))
>> #define SET_PSTATE_TCO(x) __emit_inst(0xd500401f | PSTATE_TCO | ((!!x) << PSTATE_Imm_shift))
>> +#define TSB_CSYNC __emit_inst(0xd503225f)
>>
>> #define set_pstate_pan(x) asm volatile(SET_PSTATE_PAN(x))
>> #define set_pstate_uao(x) asm volatile(SET_PSTATE_UAO(x))
>> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
>> index f154ae7..aa657ab 100644
>> --- a/drivers/hwtracing/coresight/Kconfig
>> +++ b/drivers/hwtracing/coresight/Kconfig
>> @@ -164,6 +164,17 @@ config CORESIGHT_CTI
>> To compile this driver as a module, choose M here: the
>> module will be called coresight-cti.
>>
>> +config CORESIGHT_TRBE
>> + bool "Trace Buffer Extension (TRBE) driver"
>
> Shouldn't that be "tristate"?

Yes, will change.

>
>> + depends on ARM64
>
> I would make it dependent on ETMv4 as well since it can only be used by that
> component.

Sure, will also add CORESIGHT_SOURCE_ETM4X as dependency.

>
>> + help
>> + This driver provides support for percpu Trace Buffer Extension (TRBE).
>> + TRBE always needs to be used along with it's corresponding percpu ETE
>> + component. ETE generates trace data which is then captured with TRBE.
>> + Unlike traditional sink devices, TRBE is a CPU feature accessible via
>> + system registers. But it's explicit dependency with trace unit (ETE)
>> + requires it to be plugged in as a coresight sink device.
>
> Please add:
>
> "To compile this driver as a module, choose M here: the
> module will be called coresight-trbe."

Added.

>
> I'm out of time for today, I will continue on Monday.

Sure.