Re: [RFC 07/11] coresight: sink: Add TRBE driver

From: Suzuki K Poulose
Date: Thu Nov 12 2020 - 05:13:25 EST


On 11/10/20 12:45 PM, 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.

Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
---
Documentation/trace/coresight/coresight-trbe.rst | 36 ++
arch/arm64/include/asm/sysreg.h | 2 +
drivers/hwtracing/coresight/Kconfig | 11 +
drivers/hwtracing/coresight/Makefile | 1 +
drivers/hwtracing/coresight/coresight-trbe.c | 766 +++++++++++++++++++++++
drivers/hwtracing/coresight/coresight-trbe.h | 525 ++++++++++++++++
6 files changed, 1341 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..4320a8b
--- /dev/null
+++ b/Documentation/trace/coresight/coresight-trbe.rst
@@ -0,0 +1,36 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==============================
+Trace Buffer Extension (TRBE).
+==============================
+
+ :Author: Anshuman Khandual <anshuman.khandual@xxxxxxx>
+ :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.
+
+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/
+ irq align dbm
+
+*Key file items are:-*
+ * ``irq``: TRBE maintenance interrupt number
+ * ``align``: TRBE write pointer alignment
+ * ``dbm``: TRBE updates memory with access and dirty flags
+
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 14cb156..61136f6 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 __SYS_BARRIER_INSN(CRm, op2, Rt) \
__emit_inst(0xd5000000 | sys_insn(0, 3, 3, (CRm), (op2)) | ((Rt) & 0x1f))
@@ -865,6 +866,7 @@
#define ID_AA64MMFR2_CNP_SHIFT 0
/* id_aa64dfr0 */
+#define ID_AA64DFR0_TRBE_SHIFT 44
#define ID_AA64DFR0_TRACE_FILT_SHIFT 40
#define ID_AA64DFR0_DOUBLELOCK_SHIFT 36
#define ID_AA64DFR0_PMSVER_SHIFT 32
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
index c119824..0f5e101 100644
--- a/drivers/hwtracing/coresight/Kconfig
+++ b/drivers/hwtracing/coresight/Kconfig
@@ -156,6 +156,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"
+ depends on ARM64
+ 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.
+
config CORESIGHT_CTI_INTEGRATION_REGS
bool "Access CTI CoreSight Integration Registers"
depends on CORESIGHT_CTI
diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
index f20e357..d608165 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -21,5 +21,6 @@ obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o
obj-$(CONFIG_CORESIGHT_CATU) += coresight-catu.o
obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o
+obj-$(CONFIG_CORESIGHT_TRBE) += coresight-trbe.o
coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \
coresight-cti-sysfs.o
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
new file mode 100644
index 0000000..48a8ec3
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -0,0 +1,766 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * This driver enables Trace Buffer Extension (TRBE) as a per-cpu coresight
+ * sink device could then pair with an appropriate per-cpu coresight source
+ * device (ETE) thus generating required trace data. Trace can be enabled
+ * via the perf framework.
+ *
+ * Copyright (C) 2020 ARM Ltd.
+ *
+ * Author: Anshuman Khandual <anshuman.khandual@xxxxxxx>
+ */
+#define DRVNAME "arm_trbe"
+
+#define pr_fmt(fmt) DRVNAME ": " fmt
+
+#include "coresight-trbe.h"
+
+#define PERF_IDX2OFF(idx, buf) ((idx) % ((buf)->nr_pages << PAGE_SHIFT))
+
+#define ETE_IGNORE_PACKET 0x70

Add a comment here, on what this means to the decoder.

+
+static const char trbe_name[] = "trbe";

Why not

#define DEVNAME "trbe"


+
+enum trbe_fault_action {
+ TRBE_FAULT_ACT_WRAP,
+ TRBE_FAULT_ACT_SPURIOUS,
+ TRBE_FAULT_ACT_FATAL,
+};
+
+struct trbe_perf {

Please rename this to trbe_buf. This will be used for sysfs mode as well.

+ unsigned long trbe_base;
+ unsigned long trbe_limit;
+ unsigned long trbe_write;
+ pid_t pid;

Why do we need this ? This seems unused and moreover, there cannot
be multiple tracers into TRBE. So, we don't need to share the sink
unlike the traditional ones.

+ int nr_pages;
+ void **pages;
+ bool snapshot;
+ struct trbe_cpudata *cpudata;
+};
+
+struct trbe_cpudata {
+ struct coresight_device *csdev;
+ bool trbe_dbm;

Why do we need this ?

+ u64 trbe_align;
+ int cpu;
+ enum cs_mode mode;
+ struct trbe_perf *perf;
+ struct trbe_drvdata *drvdata;
+};
+
+struct trbe_drvdata {
+ struct trbe_cpudata __percpu *cpudata;
+ struct perf_output_handle __percpu *handle;

Shouldn't this be :

struct perf_output_handle __percpu **handle ?

as we get a handle from the etm-perf and is not controlled by
the TRBE ?

+ struct hlist_node hotplug_node;
+ int irq;
+ cpumask_t supported_cpus;
+ enum cpuhp_state trbe_online;
+ struct platform_device *pdev;
+ struct clk *atclk;

We don't have any clocks for the TRBE instance. Please remove.

+};
+
+static int trbe_alloc_node(struct perf_event *event)
+{
+ if (event->cpu == -1)
+ return NUMA_NO_NODE;
+ return cpu_to_node(event->cpu);
+}
+
+static void trbe_disable_and_drain_local(void)
+{
+ write_sysreg_s(0, SYS_TRBLIMITR_EL1);
+ isb();
+ dsb(nsh);
+ asm(TSB_CSYNC);
+}
+
+static void trbe_reset_local(void)
+{
+ trbe_disable_and_drain_local();
+ write_sysreg_s(0, SYS_TRBPTR_EL1);
+ isb();
+
+ write_sysreg_s(0, SYS_TRBBASER_EL1);
+ isb();
+
+ write_sysreg_s(0, SYS_TRBSR_EL1);
+ isb();
+}
+
+static void trbe_pad_buf(struct perf_output_handle *handle, int len)
+{
+ struct trbe_perf *perf = etm_perf_sink_config(handle);
+ u64 head = PERF_IDX2OFF(handle->head, perf);
+
+ memset((void *) perf->trbe_base + head, ETE_IGNORE_PACKET, len);
+ if (!perf->snapshot)
+ perf_aux_output_skip(handle, len);
+}
+
+static unsigned long trbe_snapshot_offset(struct perf_output_handle *handle)
+{
+ struct trbe_perf *perf = etm_perf_sink_config(handle);
+ u64 head = PERF_IDX2OFF(handle->head, perf);
+ u64 limit = perf->nr_pages * PAGE_SIZE;
+

So we are using half of the buffer for snapshot mode to avoid a case where the
analyzer is unable to decode the trace in case of an overflow.

+ if (head < limit >> 1)
+ limit >>= 1;

Also this needs to be thought out. We may not need this restriction. The trace decoder
will be able to walk forward and then find a synchronization packet and then continue
the tracing from there. So, we could use the entire buffer for TRBE.


+
+ return limit;
+}
+
+static unsigned long trbe_normal_offset(struct perf_output_handle *handle)
+{
+ struct trbe_perf *perf = etm_perf_sink_config(handle);
+ struct trbe_cpudata *cpudata = perf->cpudata;
+ const u64 bufsize = perf->nr_pages * PAGE_SIZE;
+ u64 limit = bufsize;
+ u64 head, tail, wakeup;
+

Commentary please.

+ head = PERF_IDX2OFF(handle->head, perf);
+ if (!IS_ALIGNED(head, cpudata->trbe_align)) {
+ unsigned long delta = roundup(head, cpudata->trbe_align) - head;
+
+ delta = min(delta, handle->size);
+ trbe_pad_buf(handle, delta);
+ head = PERF_IDX2OFF(handle->head, perf);
+ }
+
+ if (!handle->size) {
+ perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
+ return 0;
+ }
+
+ tail = PERF_IDX2OFF(handle->head + handle->size, perf);
+ wakeup = PERF_IDX2OFF(handle->wakeup, perf);
+

+ if (head < tail)

comment

+ limit = round_down(tail, PAGE_SIZE);
+
+ if (handle->wakeup < (handle->head + handle->size) && head <= wakeup)
+ limit = min(limit, round_up(wakeup, PAGE_SIZE));

comment. Also do we need an alignement to PAGE_SIZE ?

+
+ if (limit > head)
+ return limit;
+
+ trbe_pad_buf(handle, handle->size);
+ perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
+ return 0;
+}
+
+static unsigned long get_trbe_limit(struct perf_output_handle *handle)
+{
+ struct trbe_perf *perf = etm_perf_sink_config(handle);
+ unsigned long offset;
+
+ if (perf->snapshot)
+ offset = trbe_snapshot_offset(handle);
+ else
+ offset = trbe_normal_offset(handle);
+ return perf->trbe_base + offset;
+}
+
+static void trbe_enable_hw(struct trbe_perf *perf)
+{
+ WARN_ON(perf->trbe_write < perf->trbe_base);
+ WARN_ON(perf->trbe_write >= perf->trbe_limit);
+ set_trbe_disabled();
+ clr_trbe_irq();
+ clr_trbe_wrap();
+ clr_trbe_abort();
+ clr_trbe_ec();
+ clr_trbe_bsc();
+ clr_trbe_fsc();

Please merge all of these field updates to single register update
unless mandated by the architecture.

+ set_trbe_virtual_mode();
+ set_trbe_fill_mode(TRBE_FILL_STOP);
+ set_trbe_trig_mode(TRBE_TRIGGER_IGNORE);

Same here ^^

+ isb();
+ set_trbe_base_pointer(perf->trbe_base);
+ set_trbe_limit_pointer(perf->trbe_limit);
+ set_trbe_write_pointer(perf->trbe_write);
+ isb();
+ dsb(ishst);
+ flush_tlb_all();

Why is this needed ?

+ set_trbe_running();
+ set_trbe_enabled();
+ asm(TSB_CSYNC);
+}
+
+static void *arm_trbe_alloc_buffer(struct coresight_device *csdev,
+ struct perf_event *event, void **pages,
+ int nr_pages, bool snapshot)
+{
+ struct trbe_perf *perf;
+ struct page **pglist;
+ int i;
+
+ if ((nr_pages < 2) || (snapshot && (nr_pages & 1)))

We may be able to remove the restriction on snapshot mode, see my comment
above.

+ return NULL;
+
+ perf = kzalloc_node(sizeof(*perf), GFP_KERNEL, trbe_alloc_node(event));
+ if (IS_ERR(perf))
+ return ERR_PTR(-ENOMEM);
+
+ pglist = kcalloc(nr_pages, sizeof(*pglist), GFP_KERNEL);
+ if (IS_ERR(pglist)) {
+ kfree(perf);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ for (i = 0; i < nr_pages; i++)
+ pglist[i] = virt_to_page(pages[i]);
+
+ perf->trbe_base = (unsigned long) vmap(pglist, nr_pages, VM_MAP, PAGE_KERNEL);
+ if (IS_ERR((void *) perf->trbe_base)) {
+ kfree(pglist);
+ kfree(perf);
+ return ERR_PTR(perf->trbe_base);
+ }
+ perf->trbe_limit = perf->trbe_base + nr_pages * PAGE_SIZE;
+ perf->trbe_write = perf->trbe_base;
+ perf->pid = task_pid_nr(event->owner);
+ perf->snapshot = snapshot;
+ perf->nr_pages = nr_pages;
+ perf->pages = pages;
+ kfree(pglist);
+ return perf;
+}
+
+void arm_trbe_free_buffer(void *config)
+{
+ struct trbe_perf *perf = config;
+
+ vunmap((void *) perf->trbe_base);
+ kfree(perf);
+}
+
+static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev,
+ struct perf_output_handle *handle,
+ void *config)
+{
+ struct trbe_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+ struct trbe_cpudata *cpudata = dev_get_drvdata(&csdev->dev);
+ struct trbe_perf *perf = config;
+ unsigned long size, offset;
+
+ WARN_ON(perf->cpudata != cpudata);
+ WARN_ON(cpudata->cpu != smp_processor_id());
+ WARN_ON(cpudata->mode != CS_MODE_PERF);
+ WARN_ON(cpudata->drvdata != drvdata);
+
+ offset = get_trbe_write_pointer() - get_trbe_base_pointer();
+ size = offset - PERF_IDX2OFF(handle->head, perf);
+ if (perf->snapshot)
+ handle->head += size;
+ return size;
+}
+
+static int arm_trbe_enable(struct coresight_device *csdev, u32 mode, void *data)
+{
+ struct trbe_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+ struct trbe_cpudata *cpudata = dev_get_drvdata(&csdev->dev);
+ struct perf_output_handle *handle = data;
+ struct trbe_perf *perf = etm_perf_sink_config(handle);
+
+ WARN_ON(cpudata->cpu != smp_processor_id());
+ WARN_ON(mode != CS_MODE_PERF);

Why WARN_ON ? Simply return -EINVAL ? Also you need a check to make sure
the mode is DISABLED (when you get to sysfs mode).

+ WARN_ON(cpudata->drvdata != drvdata);
+
+ *this_cpu_ptr(drvdata->handle) = *handle;

That is wrong. Storing a local copy of a global perf generic structure
is calling for trouble, assuming that the global structure doesn't change
beneath us. Please store handle ptr.

+ cpudata->perf = perf;
+ cpudata->mode = mode;
+ perf->cpudata = cpudata;
+ perf->trbe_write = perf->trbe_base + PERF_IDX2OFF(handle->head, perf);
+ perf->trbe_limit = get_trbe_limit(handle);
+ if (perf->trbe_limit == perf->trbe_base) {
+ trbe_disable_and_drain_local();
+ return 0;
+ }
+ trbe_enable_hw(perf);
+ return 0;
+}
+
+static int arm_trbe_disable(struct coresight_device *csdev)
+{
+ struct trbe_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+ struct trbe_cpudata *cpudata = dev_get_drvdata(&csdev->dev);
+ struct trbe_perf *perf = cpudata->perf;
+
+ WARN_ON(perf->cpudata != cpudata);
+ WARN_ON(cpudata->cpu != smp_processor_id());
+ WARN_ON(cpudata->mode != CS_MODE_PERF);
+ WARN_ON(cpudata->drvdata != drvdata);
+
+ trbe_disable_and_drain_local();
+ perf->cpudata = NULL;
+ cpudata->perf = NULL;
+ cpudata->mode = CS_MODE_DISABLED;
+ return 0;
+}
+
+static void trbe_handle_fatal(struct perf_output_handle *handle)
+{
+ perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
+ perf_aux_output_end(handle, 0);
+ trbe_disable_and_drain_local();
+}
+
+static void trbe_handle_spurious(struct perf_output_handle *handle)
+{
+ struct trbe_perf *perf = etm_perf_sink_config(handle);
+
+ perf->trbe_write = perf->trbe_base + PERF_IDX2OFF(handle->head, perf);
+ perf->trbe_limit = get_trbe_limit(handle);
+ if (perf->trbe_limit == perf->trbe_base) {
+ trbe_disable_and_drain_local();
+ return;
+ }
+ trbe_enable_hw(perf);
+}
+
+static void trbe_handle_overflow(struct perf_output_handle *handle)
+{
+ struct perf_event *event = handle->event;
+ struct trbe_perf *perf = etm_perf_sink_config(handle);
+ unsigned long offset, size;
+ struct etm_event_data *event_data;
+
+ offset = get_trbe_limit_pointer() - get_trbe_base_pointer();
+ size = offset - PERF_IDX2OFF(handle->head, perf);
+ if (perf->snapshot)
+ handle->head = offset;

Is this correct ? Or was this supposed to mean :
handle->head += offset;


+ perf_aux_output_end(handle, size);
+
+ event_data = perf_aux_output_begin(handle, event);
+ if (!event_data) {
+ event->hw.state |= PERF_HES_STOPPED;
+ trbe_disable_and_drain_local();
+ return;
+ }
+ perf->trbe_write = perf->trbe_base;
+ perf->trbe_limit = get_trbe_limit(handle);
+ if (perf->trbe_limit == perf->trbe_base) {
+ trbe_disable_and_drain_local();
+ return;
+ }
+ *this_cpu_ptr(perf->cpudata->drvdata->handle) = *handle;
+ trbe_enable_hw(perf);
+}
+
+static bool is_perf_trbe(struct perf_output_handle *handle)
+{
+ struct trbe_perf *perf = etm_perf_sink_config(handle);
+ struct trbe_cpudata *cpudata = perf->cpudata;
+ struct trbe_drvdata *drvdata = cpudata->drvdata;

Can you trust the cpudata ptr here as we are still verifying
if this was legitimate ?

+ int cpu = smp_processor_id();
+
+ WARN_ON(perf->trbe_base != get_trbe_base_pointer());
+ WARN_ON(perf->trbe_limit != get_trbe_limit_pointer());
+
+ if (cpudata->mode != CS_MODE_PERF)
+ return false;
+
+ if (cpudata->cpu != cpu)
+ return false;
+
+ if (!cpumask_test_cpu(cpu, &drvdata->supported_cpus))
+ return false;
+
+ return true;
+}
+
+static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *handle)
+{
+ enum trbe_ec ec = get_trbe_ec();
+ enum trbe_bsc bsc = get_trbe_bsc();
+
+ WARN_ON(is_trbe_running());
+ asm(TSB_CSYNC);
+ dsb(nsh);
+ isb();
+
+ if (is_trbe_trg() || is_trbe_abort())
+ return TRBE_FAULT_ACT_FATAL;
+
+ if ((ec == TRBE_EC_STAGE1_ABORT) || (ec == TRBE_EC_STAGE2_ABORT))
+ return TRBE_FAULT_ACT_FATAL;
+
+ if (is_trbe_wrap() && (ec == TRBE_EC_OTHERS) && (bsc == TRBE_BSC_FILLED)) {
+ if (get_trbe_write_pointer() == get_trbe_base_pointer())
+ return TRBE_FAULT_ACT_WRAP;
+ }
+ return TRBE_FAULT_ACT_SPURIOUS;
+}
+
+static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
+{
+ struct perf_output_handle *handle = dev;
+ enum trbe_fault_action act;
+
+ WARN_ON(!is_trbe_irq());
+ clr_trbe_irq();
+
+ if (!perf_get_aux(handle))
+ return IRQ_NONE;
+
+ if (!is_perf_trbe(handle))
+ return IRQ_NONE;
+
+ irq_work_run();
+
+ act = trbe_get_fault_act(handle);
+ switch (act) {
+ case TRBE_FAULT_ACT_WRAP:
+ trbe_handle_overflow(handle);
+ break;
+ case TRBE_FAULT_ACT_SPURIOUS:
+ trbe_handle_spurious(handle);
+ break;
+ case TRBE_FAULT_ACT_FATAL:
+ trbe_handle_fatal(handle);
+ break;
+ }
+ return IRQ_HANDLED;
+}
+


+static void arm_trbe_probe_coresight_cpu(void *info)
+{
+ struct trbe_cpudata *cpudata = info;
+ struct device *dev = &cpudata->drvdata->pdev->dev;
+ struct coresight_desc desc = { 0 };
+
+ if (WARN_ON(!cpudata))
+ goto cpu_clear;
+
+ if (!is_trbe_available()) {
+ pr_err("TRBE is not implemented on cpu %d\n", cpudata->cpu);
+ goto cpu_clear;
+ }
+
+ if (!is_trbe_programmable()) {
+ pr_err("TRBE is owned in higher exception level on cpu %d\n", cpudata->cpu);
+ goto cpu_clear;
+ }
+ desc.name = devm_kasprintf(dev, GFP_KERNEL, "%s%d", trbe_name, smp_processor_id());
+ if (IS_ERR(desc.name))
+ goto cpu_clear;
+
+ desc.type = CORESIGHT_DEV_TYPE_SINK;
+ desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM;

May be should add a new subtype to make this higher priority than the normal ETR.
Something like :

CORESIGHT_DEV_SUBTYPE_SINK_PERCPU_SYSMEM

+ desc.ops = &arm_trbe_cs_ops;
+ desc.pdata = dev_get_platdata(dev);
+ desc.groups = arm_trbe_groups;
+ desc.dev = dev;
+ cpudata->csdev = coresight_register(&desc);
+ if (IS_ERR(cpudata->csdev))
+ goto cpu_clear;
+
+ dev_set_drvdata(&cpudata->csdev->dev, cpudata);
+ cpudata->trbe_dbm = get_trbe_flag_update();
+ cpudata->trbe_align = 1ULL << get_trbe_address_align();
+ if (cpudata->trbe_align > SZ_2K) {
+ pr_err("Unsupported alignment on cpu %d\n", cpudata->cpu);
+ goto cpu_clear;
+ }
+ return;
+cpu_clear:
+ cpumask_clear_cpu(cpudata->cpu, &cpudata->drvdata->supported_cpus);
+}
+
+static int arm_trbe_probe_coresight(struct trbe_drvdata *drvdata)
+{
+ struct trbe_cpudata *cpudata;
+ int cpu;
+
+ drvdata->cpudata = alloc_percpu(typeof(*drvdata->cpudata));
+ if (IS_ERR(drvdata->cpudata))
+ return PTR_ERR(drvdata->cpudata);
+
+ for_each_cpu(cpu, &drvdata->supported_cpus) {
+ cpudata = per_cpu_ptr(drvdata->cpudata, cpu);
+ cpudata->cpu = cpu;
+ cpudata->drvdata = drvdata;
+ smp_call_function_single(cpu, arm_trbe_probe_coresight_cpu, cpudata, 1);

We could batch it and run it on all CPUs at the same time ? Also it would be better to
leave the per_cpu area filled by the CPU itself, to avoid racing.


+ }
+ return 0;
+}
+
+static void arm_trbe_remove_coresight_cpu(void *info)
+{
+ struct trbe_drvdata *drvdata = info;
+
+ disable_percpu_irq(drvdata->irq);
+}
+
+static int arm_trbe_remove_coresight(struct trbe_drvdata *drvdata)
+{
+ struct trbe_cpudata *cpudata;
+ int cpu;
+
+ for_each_cpu(cpu, &drvdata->supported_cpus) {
+ smp_call_function_single(cpu, arm_trbe_remove_coresight_cpu, drvdata, 1);
+ cpudata = per_cpu_ptr(drvdata->cpudata, cpu);
+ if (cpudata->csdev) {
+ coresight_unregister(cpudata->csdev);
+ cpudata->drvdata = NULL;
+ cpudata->csdev = NULL;
+ }

Please leave this to the CPU to do the part.

+ }
+ free_percpu(drvdata->cpudata);
+ return 0;
+}
+
+static int arm_trbe_cpu_startup(unsigned int cpu, struct hlist_node *node)
+{
+ struct trbe_drvdata *drvdata = hlist_entry_safe(node, struct trbe_drvdata, hotplug_node);
+ struct trbe_cpudata *cpudata;
+
+ if (cpumask_test_cpu(cpu, &drvdata->supported_cpus)) {
+ cpudata = per_cpu_ptr(drvdata->cpudata, cpu);
+ if (!cpudata->csdev) {
+ cpudata->drvdata = drvdata;
+ smp_call_function_single(cpu, arm_trbe_probe_coresight_cpu, cpudata, 1);

Why do we need smp_call here ? We are already on the CPU.

+ }
+ trbe_reset_local();
+ enable_percpu_irq(drvdata->irq, IRQ_TYPE_NONE);
+ }
+ return 0;
+}
+
+static int arm_trbe_cpu_teardown(unsigned int cpu, struct hlist_node *node)
+{
+ struct trbe_drvdata *drvdata = hlist_entry_safe(node, struct trbe_drvdata, hotplug_node);
+ struct trbe_cpudata *cpudata;
+
+ if (cpumask_test_cpu(cpu, &drvdata->supported_cpus)) {
+ cpudata = per_cpu_ptr(drvdata->cpudata, cpu);
+ if (cpudata->csdev) {
+ coresight_unregister(cpudata->csdev);
+ cpudata->drvdata = NULL;
+ cpudata->csdev = NULL;
+ }
+ disable_percpu_irq(drvdata->irq);
+ trbe_reset_local();
+ }
+ return 0;
+}
+
+static int arm_trbe_probe_cpuhp(struct trbe_drvdata *drvdata)
+{
+ enum cpuhp_state trbe_online;
+
+ trbe_online = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, DRVNAME,
+ arm_trbe_cpu_startup, arm_trbe_cpu_teardown);
+ if (trbe_online < 0)
+ return -EINVAL;
+
+ if (cpuhp_state_add_instance(trbe_online, &drvdata->hotplug_node))
+ return -EINVAL;
+
+ drvdata->trbe_online = trbe_online;
+ return 0;
+}
+
+static void arm_trbe_remove_cpuhp(struct trbe_drvdata *drvdata)
+{
+ cpuhp_remove_multi_state(drvdata->trbe_online);
+}
+
+static int arm_trbe_probe_irq(struct platform_device *pdev,
+ struct trbe_drvdata *drvdata)
+{
+ drvdata->irq = platform_get_irq(pdev, 0);
+ if (!drvdata->irq) {
+ pr_err("IRQ not found for the platform device\n");
+ return -ENXIO;
+ }
+
+ if (!irq_is_percpu(drvdata->irq)) {
+ pr_err("IRQ is not a PPI\n");
+ return -EINVAL;
+ }
+
+ if (irq_get_percpu_devid_partition(drvdata->irq, &drvdata->supported_cpus))
+ return -EINVAL;
+
+ drvdata->handle = alloc_percpu(typeof(*drvdata->handle));
+ if (!drvdata->handle)
+ return -ENOMEM;
+
+ if (request_percpu_irq(drvdata->irq, arm_trbe_irq_handler, DRVNAME, drvdata->handle)) {
+ free_percpu(drvdata->handle);
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static void arm_trbe_remove_irq(struct trbe_drvdata *drvdata)
+{
+ free_percpu_irq(drvdata->irq, drvdata->handle);
+ free_percpu(drvdata->handle);
+}
+
+static int arm_trbe_device_probe(struct platform_device *pdev)
+{
+ struct coresight_platform_data *pdata;
+ struct trbe_drvdata *drvdata;
+ struct device *dev = &pdev->dev;
+ int ret;
+
+ drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+ if (IS_ERR(drvdata))
+ return -ENOMEM;
+
+ pdata = coresight_get_platform_data(dev);
+ if (IS_ERR(pdata)) {
+ kfree(drvdata);
+ return -ENOMEM;
+ }


+
+ drvdata->atclk = devm_clk_get(dev, "atclk");
+ if (!IS_ERR(drvdata->atclk)) {
+ ret = clk_prepare_enable(drvdata->atclk);
+ if (ret)
+ return ret;
+ }

Please drop the clocks, we don't have any

+ dev_set_drvdata(dev, drvdata);
+ dev->platform_data = pdata;
+ drvdata->pdev = pdev;
+ ret = arm_trbe_probe_irq(pdev, drvdata);
+ if (ret)
+ goto irq_failed;
+
+ ret = arm_trbe_probe_coresight(drvdata);
+ if (ret)
+ goto probe_failed;
+
+ ret = arm_trbe_probe_cpuhp(drvdata);
+ if (ret)
+ goto cpuhp_failed;
+
+ return 0;
+cpuhp_failed:
+ arm_trbe_remove_coresight(drvdata);
+probe_failed:
+ arm_trbe_remove_irq(drvdata);
+irq_failed:
+ kfree(pdata);
+ kfree(drvdata);
+ return ret;
+}
+
+static int arm_trbe_device_remove(struct platform_device *pdev)
+{
+ struct coresight_platform_data *pdata = dev_get_platdata(&pdev->dev);
+ struct trbe_drvdata *drvdata = platform_get_drvdata(pdev);
+
+ arm_trbe_remove_coresight(drvdata);
+ arm_trbe_remove_cpuhp(drvdata);
+ arm_trbe_remove_irq(drvdata);
+ kfree(pdata);
+ kfree(drvdata);
+ return 0;
+}
+
+#ifdef CONFIG_PM
+static int arm_trbe_runtime_suspend(struct device *dev)
+{
+ struct trbe_drvdata *drvdata = dev_get_drvdata(dev);
+
+ if (drvdata && !IS_ERR(drvdata->atclk))
+ clk_disable_unprepare(drvdata->atclk);
+

Remove. We may need to save/restore the TRBE ptrs, depending on the
TRBE.

+ return 0;
+}
+
+static int arm_trbe_runtime_resume(struct device *dev)
+{
+ struct trbe_drvdata *drvdata = dev_get_drvdata(dev);
+
+ if (drvdata && !IS_ERR(drvdata->atclk))
+ clk_prepare_enable(drvdata->atclk);

Remove. See above.

+
+ return 0;
+}
+#endif
+
+static const struct dev_pm_ops arm_trbe_dev_pm_ops = {
+ SET_RUNTIME_PM_OPS(arm_trbe_runtime_suspend, arm_trbe_runtime_resume, NULL)
+};
+
+static const struct of_device_id arm_trbe_of_match[] = {
+ { .compatible = "arm,arm-trbe", .data = (void *)1 },
+ {},
+};

I think it is better to call this, we have too many acronyms ;-)

"arm,trace-buffer-extension"

+MODULE_DEVICE_TABLE(of, arm_trbe_of_match);

+
+static const struct platform_device_id arm_trbe_match[] = {
+ { "arm,trbe", 0},
+ { }
+};
+MODULE_DEVICE_TABLE(platform, arm_trbe_match);

Please remove. The ACPI part can be added when we get to it.

+
+static struct platform_driver arm_trbe_driver = {
+ .id_table = arm_trbe_match,
+ .driver = {
+ .name = DRVNAME,
+ .of_match_table = of_match_ptr(arm_trbe_of_match),
+ .pm = &arm_trbe_dev_pm_ops,
+ .suppress_bind_attrs = true,
+ },
+ .probe = arm_trbe_device_probe,
+ .remove = arm_trbe_device_remove,
+};
+builtin_platform_driver(arm_trbe_driver)

Please make this modular.


diff --git a/drivers/hwtracing/coresight/coresight-trbe.h b/drivers/hwtracing/coresight/coresight-trbe.h
new file mode 100644
index 0000000..82ffbfc
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-trbe.h
@@ -0,0 +1,525 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * This contains all required hardware related helper functions for
+ * Trace Buffer Extension (TRBE) driver in the coresight framework.
+ *
+ * Copyright (C) 2020 ARM Ltd.
+ *
+ * Author: Anshuman Khandual <anshuman.khandual@xxxxxxx>
+ */
+#include <linux/coresight.h>
+#include <linux/device.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/smp.h>
+
+#include "coresight-etm-perf.h"
+
+static inline bool is_trbe_available(void)
+{
+ u64 aa64dfr0 = read_sysreg_s(SYS_ID_AA64DFR0_EL1);
+ int trbe = cpuid_feature_extract_unsigned_field(aa64dfr0, ID_AA64DFR0_TRBE_SHIFT);
+
+ return trbe >= 0b0001;
+}
+
+static inline bool is_ete_available(void)
+{
+ u64 aa64dfr0 = read_sysreg_s(SYS_ID_AA64DFR0_EL1);
+ int tracever = cpuid_feature_extract_unsigned_field(aa64dfr0, ID_AA64DFR0_TRACEVER_SHIFT);
+
+ return (tracever != 0b0000);

Why is this needed ?

+}
+
+static inline bool is_trbe_enabled(void)
+{
+ u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+
+ return trblimitr & TRBLIMITR_ENABLE;
+}
+
+enum trbe_ec {
+ TRBE_EC_OTHERS = 0,
+ TRBE_EC_STAGE1_ABORT = 36,
+ TRBE_EC_STAGE2_ABORT = 37,
+};
+
+static const char *const trbe_ec_str[] = {
+ [TRBE_EC_OTHERS] = "Maintenance exception",
+ [TRBE_EC_STAGE1_ABORT] = "Stage-1 exception",
+ [TRBE_EC_STAGE2_ABORT] = "Stage-2 exception",
+};
+

Please remove the defintions that are not used by the driver.

+static inline enum trbe_ec get_trbe_ec(void)
+{
+ u64 trbsr = read_sysreg_s(SYS_TRBSR_EL1);
+
+ return (trbsr >> TRBSR_EC_SHIFT) & TRBSR_EC_MASK;
+}
+
+static inline void clr_trbe_ec(void)
+{
+ u64 trbsr = read_sysreg_s(SYS_TRBSR_EL1);
+
+ trbsr &= ~(TRBSR_EC_MASK << TRBSR_EC_SHIFT);
+ write_sysreg_s(trbsr, SYS_TRBSR_EL1);
+}
+
+enum trbe_bsc {
+ TRBE_BSC_NOT_STOPPED = 0,
+ TRBE_BSC_FILLED = 1,
+ TRBE_BSC_TRIGGERED = 2,
+};
+
+static const char *const trbe_bsc_str[] = {
+ [TRBE_BSC_NOT_STOPPED] = "TRBE collection not stopped",
+ [TRBE_BSC_FILLED] = "TRBE filled",
+ [TRBE_BSC_TRIGGERED] = "TRBE triggered",
+};
+
+static inline enum trbe_bsc get_trbe_bsc(void)
+{
+ u64 trbsr = read_sysreg_s(SYS_TRBSR_EL1);
+
+ return (trbsr >> TRBSR_BSC_SHIFT) & TRBSR_BSC_MASK;
+}
+
+static inline void clr_trbe_bsc(void)
+{
+ u64 trbsr = read_sysreg_s(SYS_TRBSR_EL1);
+
+ trbsr &= ~(TRBSR_BSC_MASK << TRBSR_BSC_SHIFT);
+ write_sysreg_s(trbsr, SYS_TRBSR_EL1);
+}
+
+enum trbe_fsc {
+ TRBE_FSC_ASF_LEVEL0 = 0,
+ TRBE_FSC_ASF_LEVEL1 = 1,
+ TRBE_FSC_ASF_LEVEL2 = 2,
+ TRBE_FSC_ASF_LEVEL3 = 3,
+ TRBE_FSC_TF_LEVEL0 = 4,
+ TRBE_FSC_TF_LEVEL1 = 5,
+ TRBE_FSC_TF_LEVEL2 = 6,
+ TRBE_FSC_TF_LEVEL3 = 7,
+ TRBE_FSC_AFF_LEVEL0 = 8,
+ TRBE_FSC_AFF_LEVEL1 = 9,
+ TRBE_FSC_AFF_LEVEL2 = 10,
+ TRBE_FSC_AFF_LEVEL3 = 11,
+ TRBE_FSC_PF_LEVEL0 = 12,
+ TRBE_FSC_PF_LEVEL1 = 13,
+ TRBE_FSC_PF_LEVEL2 = 14,
+ TRBE_FSC_PF_LEVEL3 = 15,
+ TRBE_FSC_SEA_WRITE = 16,
+ TRBE_FSC_ASEA_WRITE = 17,
+ TRBE_FSC_SEA_LEVEL0 = 20,
+ TRBE_FSC_SEA_LEVEL1 = 21,
+ TRBE_FSC_SEA_LEVEL2 = 22,
+ TRBE_FSC_SEA_LEVEL3 = 23,
+ TRBE_FSC_ALIGN_FAULT = 33,
+ TRBE_FSC_TLB_FAULT = 48,
+ TRBE_FSC_ATOMIC_FAULT = 49,
+};

Please remove ^^^

+
+static const char *const trbe_fsc_str[] = {
+ [TRBE_FSC_ASF_LEVEL0] = "Address size fault - level 0",
+ [TRBE_FSC_ASF_LEVEL1] = "Address size fault - level 1",
+ [TRBE_FSC_ASF_LEVEL2] = "Address size fault - level 2",
+ [TRBE_FSC_ASF_LEVEL3] = "Address size fault - level 3",
+ [TRBE_FSC_TF_LEVEL0] = "Translation fault - level 0",
+ [TRBE_FSC_TF_LEVEL1] = "Translation fault - level 1",
+ [TRBE_FSC_TF_LEVEL2] = "Translation fault - level 2",
+ [TRBE_FSC_TF_LEVEL3] = "Translation fault - level 3",
+ [TRBE_FSC_AFF_LEVEL0] = "Access flag fault - level 0",
+ [TRBE_FSC_AFF_LEVEL1] = "Access flag fault - level 1",
+ [TRBE_FSC_AFF_LEVEL2] = "Access flag fault - level 2",
+ [TRBE_FSC_AFF_LEVEL3] = "Access flag fault - level 3",
+ [TRBE_FSC_PF_LEVEL0] = "Permission fault - level 0",
+ [TRBE_FSC_PF_LEVEL1] = "Permission fault - level 1",
+ [TRBE_FSC_PF_LEVEL2] = "Permission fault - level 2",
+ [TRBE_FSC_PF_LEVEL3] = "Permission fault - level 3",
+ [TRBE_FSC_SEA_WRITE] = "Synchronous external abort on write",
+ [TRBE_FSC_ASEA_WRITE] = "Asynchronous external abort on write",
+ [TRBE_FSC_SEA_LEVEL0] = "Syncrhonous external abort on table walk - level 0",
+ [TRBE_FSC_SEA_LEVEL1] = "Syncrhonous external abort on table walk - level 1",
+ [TRBE_FSC_SEA_LEVEL2] = "Syncrhonous external abort on table walk - level 2",
+ [TRBE_FSC_SEA_LEVEL3] = "Syncrhonous external abort on table walk - level 3",
+ [TRBE_FSC_ALIGN_FAULT] = "Alignment fault",
+ [TRBE_FSC_TLB_FAULT] = "TLB conflict fault",
+ [TRBE_FSC_ATOMIC_FAULT] = "Atmoc fault",
+};


Please remove ^^^



+enum trbe_address_mode {
+ TRBE_ADDRESS_VIRTUAL,
+ TRBE_ADDRESS_PHYSICAL,
+};

#define please.

+
+static const char *const trbe_address_mode_str[] = {
+ [TRBE_ADDRESS_VIRTUAL] = "Address mode - virtual",
+ [TRBE_ADDRESS_PHYSICAL] = "Address mode - physical",
+};

Do we need this ? We always use virtual.

+
+static inline bool is_trbe_virtual_mode(void)
+{
+ u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+
+ return !(trblimitr & TRBLIMITR_NVM);
+}
+

Remove

+static inline bool is_trbe_physical_mode(void)
+{
+ u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+
+ return trblimitr & TRBLIMITR_NVM;
+}

Remove

+
+static inline void set_trbe_virtual_mode(void)
+{
+ u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+
+ trblimitr &= ~TRBLIMITR_NVM;
+ write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);
+}
+

+static inline void set_trbe_physical_mode(void)
+{
+ u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+
+ trblimitr |= TRBLIMITR_NVM;
+ write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);
+}

Remove

+
+enum trbe_trig_mode {
+ TRBE_TRIGGER_STOP = 0,
+ TRBE_TRIGGER_IRQ = 1,
+ TRBE_TRIGGER_IGNORE = 3,
+};
+
+static const char *const trbe_trig_mode_str[] = {
+ [TRBE_TRIGGER_STOP] = "Trigger mode - stop",
+ [TRBE_TRIGGER_IRQ] = "Trigger mode - irq",
+ [TRBE_TRIGGER_IGNORE] = "Trigger mode - ignore",
+};
+
+static inline enum trbe_trig_mode get_trbe_trig_mode(void)
+{
+ u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+
+ return (trblimitr >> TRBLIMITR_TRIG_MODE_SHIFT) & TRBLIMITR_TRIG_MODE_MASK;
+}
+
+static inline void set_trbe_trig_mode(enum trbe_trig_mode mode)
+{
+ u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+
+ trblimitr &= ~(TRBLIMITR_TRIG_MODE_MASK << TRBLIMITR_TRIG_MODE_SHIFT);
+ trblimitr |= ((mode & TRBLIMITR_TRIG_MODE_MASK) << TRBLIMITR_TRIG_MODE_SHIFT);
+ write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);
+}
+
+enum trbe_fill_mode {
+ TRBE_FILL_STOP = 0,
+ TRBE_FILL_WRAP = 1,
+ TRBE_FILL_CIRCULAR = 3,
+};
+

Please use #define

+static const char *const trbe_fill_mode_str[] = {
+ [TRBE_FILL_STOP] = "Buffer mode - stop",
+ [TRBE_FILL_WRAP] = "Buffer mode - wrap",
+ [TRBE_FILL_CIRCULAR] = "Buffer mode - circular",
+};
+
+static inline enum trbe_fill_mode get_trbe_fill_mode(void)
+{
+ u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+
+ return (trblimitr >> TRBLIMITR_FILL_MODE_SHIFT) & TRBLIMITR_FILL_MODE_MASK;
+}
+
+static inline void set_trbe_fill_mode(enum trbe_fill_mode mode)
+{
+ u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+
+ trblimitr &= ~(TRBLIMITR_FILL_MODE_MASK << TRBLIMITR_FILL_MODE_SHIFT);
+ trblimitr |= ((mode & TRBLIMITR_FILL_MODE_MASK) << TRBLIMITR_FILL_MODE_SHIFT);
+ write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);
+}
+
+static inline void set_trbe_disabled(void)
+{
+ u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+
+ trblimitr &= ~TRBLIMITR_ENABLE;
+ write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);
+}
+
+static inline void set_trbe_enabled(void)
+{
+ u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+
+ trblimitr |= TRBLIMITR_ENABLE;
+ write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);
+}
+
+static inline bool get_trbe_flag_update(void)
+{
+ u64 trbidr = read_sysreg_s(SYS_TRBIDR_EL1);
+
+ return trbidr & TRBIDR_FLAG;
+}
+
+static inline bool is_trbe_programmable(void)
+{
+ u64 trbidr = read_sysreg_s(SYS_TRBIDR_EL1);
+
+ return !(trbidr & TRBIDR_PROG);
+}
+#
+enum trbe_buffer_align {
+ TRBE_BUFFER_BYTE,
+ TRBE_BUFFER_HALF_WORD,
+ TRBE_BUFFER_WORD,
+ TRBE_BUFFER_DOUBLE_WORD,
+ TRBE_BUFFER_16_BYTES,
+ TRBE_BUFFER_32_BYTES,
+ TRBE_BUFFER_64_BYTES,
+ TRBE_BUFFER_128_BYTES,
+ TRBE_BUFFER_256_BYTES,
+ TRBE_BUFFER_512_BYTES,
+ TRBE_BUFFER_1K_BYTES,
+ TRBE_BUFFER_2K_BYTES,
+};
+

Remove ^^

+static const char *const trbe_buffer_align_str[] = {
+ [TRBE_BUFFER_BYTE] = "Byte",
+ [TRBE_BUFFER_HALF_WORD] = "Half word",
+ [TRBE_BUFFER_WORD] = "Word",
+ [TRBE_BUFFER_DOUBLE_WORD] = "Double word",
+ [TRBE_BUFFER_16_BYTES] = "16 bytes",
+ [TRBE_BUFFER_32_BYTES] = "32 bytes",
+ [TRBE_BUFFER_64_BYTES] = "64 bytes",
+ [TRBE_BUFFER_128_BYTES] = "128 bytes",
+ [TRBE_BUFFER_256_BYTES] = "256 bytes",
+ [TRBE_BUFFER_512_BYTES] = "512 bytes",
+ [TRBE_BUFFER_1K_BYTES] = "1K bytes",
+ [TRBE_BUFFER_2K_BYTES] = "2K bytes",
+};

We don't need any of this. We could simply "<<" and get the
size.


+
+static inline enum trbe_buffer_align get_trbe_address_align(void)
+{
+ u64 trbidr = read_sysreg_s(SYS_TRBIDR_EL1);
+
+ return (trbidr >> TRBIDR_ALIGN_SHIFT) & TRBIDR_ALIGN_MASK;
+}
+
+static inline void assert_trbe_address_mode(unsigned long addr)
+{
+ bool virt_addr = virt_addr_valid(addr) || is_vmalloc_addr((void *)addr);
+ bool virt_mode = is_trbe_virtual_mode();
+
+ WARN_ON(addr && ((virt_addr && !virt_mode) || (!virt_addr && virt_mode)));
+}

I am not sure if this is really helpful. You have to trust the kernel vmalloc().

+
+static inline void assert_trbe_address_align(unsigned long addr)
+{
+ unsigned long nr_bytes = 1ULL << get_trbe_address_align();
+
+ WARN_ON(addr & (nr_bytes - 1));
+}
+
+static inline unsigned long get_trbe_write_pointer(void)
+{
+ u64 trbptr = read_sysreg_s(SYS_TRBPTR_EL1);
+ unsigned long addr = (trbptr >> TRBPTR_PTR_SHIFT) & TRBPTR_PTR_MASK;
+
+ assert_trbe_address_mode(addr);
+ assert_trbe_address_align(addr);
+ return addr;
+}
+
+static inline void set_trbe_write_pointer(unsigned long addr)
+{
+ WARN_ON(is_trbe_enabled());
+ assert_trbe_address_mode(addr);
+ assert_trbe_address_align(addr);
+ addr = (addr >> TRBPTR_PTR_SHIFT) & TRBPTR_PTR_MASK;
+ write_sysreg_s(addr, SYS_TRBPTR_EL1);
+}
+
+static inline unsigned long get_trbe_limit_pointer(void)
+{
+ u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+ unsigned long limit = (trblimitr >> TRBLIMITR_LIMIT_SHIFT) & TRBLIMITR_LIMIT_MASK;
+ unsigned long addr = limit << TRBLIMITR_LIMIT_SHIFT;
+
+ WARN_ON(addr & (PAGE_SIZE - 1));
+ assert_trbe_address_mode(addr);
+ assert_trbe_address_align(addr);
+ return addr;
+}
+
+static inline void set_trbe_limit_pointer(unsigned long addr)
+{
+ u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+
+ WARN_ON(is_trbe_enabled());
+ assert_trbe_address_mode(addr);
+ assert_trbe_address_align(addr);
+ WARN_ON(addr & ((1UL << TRBLIMITR_LIMIT_SHIFT) - 1));
+ WARN_ON(addr & (PAGE_SIZE - 1));
+ trblimitr &= ~(TRBLIMITR_LIMIT_MASK << TRBLIMITR_LIMIT_SHIFT);
+ trblimitr |= (addr & PAGE_MASK);
+ write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);
+}
+
+static inline unsigned long get_trbe_base_pointer(void)
+{
+ u64 trbbaser = read_sysreg_s(SYS_TRBBASER_EL1);
+ unsigned long addr = (trbbaser >> TRBBASER_BASE_SHIFT) & TRBBASER_BASE_MASK;
+
+ addr = addr << TRBBASER_BASE_SHIFT;
+ WARN_ON(addr & (PAGE_SIZE - 1));
+ assert_trbe_address_mode(addr);
+ assert_trbe_address_align(addr);
+ return addr;
+}
+
+static inline void set_trbe_base_pointer(unsigned long addr)
+{
+ WARN_ON(is_trbe_enabled());
+ assert_trbe_address_mode(addr);
+ assert_trbe_address_align(addr);
+ WARN_ON(addr & ((1UL << TRBBASER_BASE_SHIFT) - 1));
+ WARN_ON(addr & (PAGE_SIZE - 1));
+ write_sysreg_s(addr, SYS_TRBBASER_EL1);
+}


Suzuki