Skip to contentSkip to conversation

Linux RISC-V trace framework and drivers

PATCH In review Anup Patel 6 participants 29 messages Nov 1, 2025 – Feb 9

Subsystems (4)
lore
Share & tools
lore

Anup Patel

author
#0 lore raw

v2 cover Linux RISC-V trace framework and drivers

This series adds initial support for RISC-V trace framework and drivers. The RISC-V trace v1.0 specification is already ratified and can be found at:
https://github.com/riscv-non-isa/e-trace-encap/releases/tag/v1.0.0-ratified
https://github.com/riscv-non-isa/tg-nexus-trace/releases/tag/1.0_Ratified

The RISC-V trace framework and drivers are designed to be agnostic to the underlying trace protocol hence both RISC-V E-trace and RISC-V N-trace should work fine. The discovery of trace protocl parameters are left to user-space trace decoder.

In ther future, there will be subsequent series adding:
1) Sysfs support
2) ACPI support
3) More trace drivers (such as funnel, ATB, etc)
4) Support for upcoming self-hosted trace specification
5) ... and more ...

These patches can also be found in the riscv_trace_support_v2 branch at:
https://github.com/avpatel/linux.git

To test the patches, we need QEMU virt machine with RISC-V trace support which can be found in rv-etrace branch at:
https://gitlab.com/danielhb/qemu.git

To capture rvtrace data using perf on QEMU virt machine do the following:
1) Launch QEMU virt machine

   $ qemu-system-riscv64 -nographic -M virt -smp 2 -bios fw_dynamic.bin \
     -kernel Image -append "root=/dev/vda rw console=ttyS0 earlycon=sbi" \
     -drive file=/path/to/rootfs.img,id=disk1,if=none,format=raw \
     -device virtio-blk-device,drive=disk1
2) Run perf record to capture rvtrace data
   $ perf record --all-cpus -e rvtrace/event=0x1/ <command>
3) The step2 would create a perf.data file which has the rvtrace data.
   Now run perf report -D and look for PERF_RECORD_AUXTRACE event
   section(s) which point(s) to the actual rvtrace data offset.

Changes since v1:
- Rebased on Linux-6.18-rc3
- Addressed Rob's comments in dt-bindings added by PATCH1
- Get reference of conn->dest_fwnode and add missing break in rvtrace_of_parse_outconns() of rvtrace-platform drivers added by PATCH2
- Added new inline function rvtrace_comp_is_empty() in PATCH2 and used it in rvtrace_encoder_stop() added by PATCH5
- Fixed trRamWPLow usage in PATCH7
- Determine RAM sink buffer size based on component implementation ID and reduce default RAM sink buffer size to 1MB
- Add new PATCH8 to enable DMA_RESTRICTED_POOL in RISC-V defconfig so that implementations with RAM sink address restrictions can be handled.

Anup Patel (6):
dt-bindings: Add RISC-V trace component bindings
rvtrace: Initial implementation of driver framework
rvtrace: Add functions to create/destroy a trace component path
rvtrace: Add function to copy into perf AUX buffer
riscv: Enable DMA_RESTRICTED_POOL in defconfig
MAINTAINERS: Add entry for RISC-V trace framework and drivers

Mayuresh Chitale (6):
rvtrace: Add functions to start/stop tracing on a component path
rvtrace: Add trace encoder driver
rvtrace: Add trace ramsink driver
rvtrace: Add perf driver for tracing using perf tool perf tools: Add RISC-V trace PMU record capabilities perf tools: Initial support for RISC-V trace decoder

 .../bindings/riscv/riscv,trace-component.yaml | 112 +++
 MAINTAINERS                                   |  11 +
 arch/riscv/configs/defconfig                  |   1 +
 drivers/Makefile                              |   1 +
 drivers/hwtracing/Kconfig                     |   2 +
 drivers/hwtracing/rvtrace/Kconfig             |  33 +
 drivers/hwtracing/rvtrace/Makefile            |   6 +
 drivers/hwtracing/rvtrace/rvtrace-core.c      | 798 ++++++++++++++++++
 drivers/hwtracing/rvtrace/rvtrace-encoder.c   | 107 +++
 drivers/hwtracing/rvtrace/rvtrace-perf.c      | 343 ++++++++
 drivers/hwtracing/rvtrace/rvtrace-platform.c  | 192 +++++
 drivers/hwtracing/rvtrace/rvtrace-ramsink.c   | 262 ++++++
 include/linux/rvtrace.h                       | 349 ++++++++
 tools/perf/arch/riscv/util/Build              |   1 +
 tools/perf/arch/riscv/util/auxtrace.c         | 218 +++++
 tools/perf/util/Build                         |   1 +
 tools/perf/util/auxtrace.c                    |   4 +
 tools/perf/util/auxtrace.h                    |   1 +
 tools/perf/util/rvtrace-decoder.c             |  91 ++
 tools/perf/util/rvtrace.h                     |  20 +
 20 files changed, 2553 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml
 create mode 100644 drivers/hwtracing/rvtrace/Kconfig
 create mode 100644 drivers/hwtracing/rvtrace/Makefile
 create mode 100644 drivers/hwtracing/rvtrace/rvtrace-core.c
 create mode 100644 drivers/hwtracing/rvtrace/rvtrace-encoder.c
 create mode 100644 drivers/hwtracing/rvtrace/rvtrace-perf.c
 create mode 100644 drivers/hwtracing/rvtrace/rvtrace-platform.c
 create mode 100644 drivers/hwtracing/rvtrace/rvtrace-ramsink.c
 create mode 100644 include/linux/rvtrace.h
 create mode 100644 tools/perf/arch/riscv/util/auxtrace.c
 create mode 100644 tools/perf/util/rvtrace-decoder.c
 create mode 100644 tools/perf/util/rvtrace.h
Signature · 1 line
2.43.0
author
#1 lore raw

PATCH v2 1/12 dt-bindings: Add RISC-V trace component bindings

Add device tree bindings for the memory mapped RISC-V trace components which support both the RISC-V efficient trace (E-trace) protocol and the RISC-V Nexus-based trace (N-trace) protocol.

The RISC-V trace components are defined by the RISC-V trace control interface specification.

---
 .../bindings/riscv/riscv,trace-component.yaml | 112 ++++++++++++++++++
 1 file changed, 112 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml
Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml new file +112 -0
new file mode 100644
index 000000000000..7979af3d4174
@@ -0,0 +1,112 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/riscv/riscv,trace-component.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V Trace Component
+
+maintainers:
+  - Anup Patel <[email protected]>
+
+description:
+  The RISC-V trace control interface specification standard memory mapped
+  components (or devices) which support both the RISC-V efficient trace
+  (E-trace) protocol and the RISC-V Nexus-based trace (N-trace) protocol.
+  The RISC-V trace components have implementation specific directed acyclic
+  graph style interdependency where output of one component serves as input
+  to another component and certain components (such as funnel) can take inputs
+  from multiple components. The type and version of a RISC-V trace component
+  can be discovered from it's IMPL memory mapped register hence component
+  specific compatible strings are not needed.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - qemu,trace-component
+      - const: riscv,trace-component
+
+  reg:
+    maxItems: 1
+
+  cpus:
+    maxItems: 1
+    description:
+      phandle to the cpu to which the RISC-V trace component is bound.
+
+  in-ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+    patternProperties:
+      '^port(@[0-7])?$':
+        description: Input connections from RISC-V trace component
+        $ref: /schemas/graph.yaml#/properties/port
+
+  out-ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+    patternProperties:
+      '^port(@[0-7])?$':
+        description: Output connections from RISC-V trace component
+        $ref: /schemas/graph.yaml#/properties/port
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    // Example 1 (Per-hart encoder and ramsink components):
+
+    encoder@c000000 {
+      compatible = "qemu,trace-component", "riscv,trace-component";
+      reg = <0xc000000 0x1000>;
+      cpus = <&CPU0>;
+      out-ports {
+        port {
+          CPU0_ENCODER_OUTPUT: endpoint {
+            remote-endpoint = <&CPU0_RAMSINK_INPUT>;
+          };
+        };
+      };
+    };
+
+    ramsink@c001000 {
+      compatible = "qemu,trace-component", "riscv,trace-component";
+      reg = <0xc001000 0x1000>;
+      cpus = <&CPU0>;
+      in-ports {
+        port {
+          CPU0_RAMSINK_INPUT: endpoint {
+          };
+        };
+      };
+    };
+
+    encoder@c002000 {
+      compatible = "qemu,trace-component", "riscv,trace-component";
+      reg = <0xc002000 0x1000>;
+      cpus = <&CPU1>;
+      out-ports {
+        port {
+          CPU1_ENCODER_OUTPUT: endpoint {
+            remote-endpoint = <&CPU1_RAMSINK_INPUT>;
+          };
+        };
+      };
+    };
+
+    ramsink@c003000 {
+      compatible = "qemu,trace-component", "riscv,trace-component";
+      reg = <0xc003000 0x1000>;
+      cpus = <&CPU1>;
+      in-ports {
+        port {
+          CPU1_RAMSINK_INPUT: endpoint {
+          };
+        };
+      };
+    };
+
+...
Signature · 1 line
2.43.0
maintainer
#2 lore raw

Re: [PATCH v2 01/12] dt-bindings: Add RISC-V trace component bindings

Quoted 37 lines · Anup Patel

Add device tree bindings for the memory mapped RISC-V trace components which support both the RISC-V efficient trace (E-trace) protocol and the RISC-V Nexus-based trace (N-trace) protocol.

The RISC-V trace components are defined by the RISC-V trace control interface specification.

---
 .../bindings/riscv/riscv,trace-component.yaml | 112 ++++++++++++++++++
 1 file changed, 112 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml
Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml new file +18 -0
new file mode 100644
index 000000000000..7979af3d4174
@@ -0,0 +1,112 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/riscv/riscv,trace-component.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V Trace Component
+
+maintainers:
+  - Anup Patel <[email protected]>
+
+description:
+  The RISC-V trace control interface specification standard memory mapped
+  components (or devices) which support both the RISC-V efficient trace
+  (E-trace) protocol and the RISC-V Nexus-based trace (N-trace) protocol.
+  The RISC-V trace components have implementation specific directed acyclic
+  graph style interdependency where output of one component serves as input
+  to another component and certain components (such as funnel) can take inputs

Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml line 28

Quoted 10 lines
Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml new file +10 -0
+  from multiple components. The type and version of a RISC-V trace component
+  can be discovered from it's IMPL memory mapped register hence component
+  specific compatible strings are not needed.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - qemu,trace-component
+      - const: riscv,trace-component

'component' seems a bit redundant.

Quoted 16 lines
Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml new file +16 -0
+
+  reg:
+    maxItems: 1
+
+  cpus:
+    maxItems: 1
+    description:
+      phandle to the cpu to which the RISC-V trace component is bound.
+
+  in-ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+    patternProperties:
+      '^port(@[0-7])?$':
+        description: Input connections from RISC-V trace component
+        $ref: /schemas/graph.yaml#/properties/port
+

Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml line 54

Quoted 10 lines
Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml new file +10 -0
+  out-ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+    patternProperties:
+      '^port(@[0-7])?$':
+        description: Output connections from RISC-V trace component
+        $ref: /schemas/graph.yaml#/properties/port
+
+required:
+  - compatible
+  - reg

Is no in or out ports valid? If not, you need:

anyOf:
- required: [ in-ports ]
- required: [ out-ports ]

Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml line 62

Quoted 8 lines
Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml new file +8 -0
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    // Example 1 (Per-hart encoder and ramsink components):
+
+    encoder@c000000 {

trace@...?

Node names should be generic and a given compatible should only have 1 possible node name. (Yes, we failed to do this on Arm coresight stuff.)

Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml line 65

Quoted 3 lines
Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml new file +3 -0
+      compatible = "qemu,trace-component", "riscv,trace-component";
+      reg = <0xc000000 0x1000>;
+      cpus = <&CPU0>;

blank line between properties and child nodes.

Quoted 50 lines
Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml new file +47 -0
+      out-ports {
+        port {
+          CPU0_ENCODER_OUTPUT: endpoint {
+            remote-endpoint = <&CPU0_RAMSINK_INPUT>;
+          };
+        };
+      };
+    };
+
+    ramsink@c001000 {
+      compatible = "qemu,trace-component", "riscv,trace-component";
+      reg = <0xc001000 0x1000>;
+      cpus = <&CPU0>;
+      in-ports {
+        port {
+          CPU0_RAMSINK_INPUT: endpoint {
+          };
+        };
+      };
+    };
+
+    encoder@c002000 {
+      compatible = "qemu,trace-component", "riscv,trace-component";
+      reg = <0xc002000 0x1000>;
+      cpus = <&CPU1>;
+      out-ports {
+        port {
+          CPU1_ENCODER_OUTPUT: endpoint {
+            remote-endpoint = <&CPU1_RAMSINK_INPUT>;
+          };
+        };
+      };
+    };
+
+    ramsink@c003000 {
+      compatible = "qemu,trace-component", "riscv,trace-component";
+      reg = <0xc003000 0x1000>;
+      cpus = <&CPU1>;
+      in-ports {
+        port {
+          CPU1_RAMSINK_INPUT: endpoint {
+          };
+        };
+      };
+    };
+
+...
Signature · 1 line
2.43.0
author
#3 lore raw

Re: [PATCH v2 01/12] dt-bindings: Add RISC-V trace component bindings

Quoted 43 lines · Rob Herring
Quoted 41 lines · Anup Patel

Add device tree bindings for the memory mapped RISC-V trace components which support both the RISC-V efficient trace (E-trace) protocol and the RISC-V Nexus-based trace (N-trace) protocol.

The RISC-V trace components are defined by the RISC-V trace control interface specification.

---
 .../bindings/riscv/riscv,trace-component.yaml | 112 ++++++++++++++++++
 1 file changed, 112 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml
Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml new file +22 -0
new file mode 100644
index 000000000000..7979af3d4174
@@ -0,0 +1,112 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/riscv/riscv,trace-component.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V Trace Component
+
+maintainers:
+  - Anup Patel <[email protected]>
+
+description:
+  The RISC-V trace control interface specification standard memory mapped
+  components (or devices) which support both the RISC-V efficient trace
+  (E-trace) protocol and the RISC-V Nexus-based trace (N-trace) protocol.
+  The RISC-V trace components have implementation specific directed acyclic
+  graph style interdependency where output of one component serves as input
+  to another component and certain components (such as funnel) can take inputs
+  from multiple components. The type and version of a RISC-V trace component
+  can be discovered from it's IMPL memory mapped register hence component
+  specific compatible strings are not needed.
+
Quoted 8 lines

Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml line 28

Quoted 6 lines
Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml new file +6 -0
+properties:
+  compatible:
+    items:
+      - enum:
+          - qemu,trace-component
+      - const: riscv,trace-component

'component' seems a bit redundant.

The RISC-V trace control specification refers to MMIO trace devices as 'components' hence we should keep the 'component' name in the compatible string.

Quoted 30 lines
Quoted 17 lines
Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml new file +16 -0
+
+  reg:
+    maxItems: 1
+
+  cpus:
+    maxItems: 1
+    description:
+      phandle to the cpu to which the RISC-V trace component is bound.
+
+  in-ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+    patternProperties:
+      '^port(@[0-7])?$':
+        description: Input connections from RISC-V trace component
+        $ref: /schemas/graph.yaml#/properties/port
+

Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml line 54

Quoted 10 lines
Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml new file +10 -0
+  out-ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+    patternProperties:
+      '^port(@[0-7])?$':
+        description: Output connections from RISC-V trace component
+        $ref: /schemas/graph.yaml#/properties/port
+
+required:
+  - compatible
+  - reg

Is no in or out ports valid? If not, you need:

Quoted 3 lines

anyOf:
- required: [ in-ports ]
- required: [ out-ports ]

Trace component with no in or out port is invalid so I will include the above "anyOf" snippet in the next revision.

Quoted 11 lines

Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml line 62

Quoted 9 lines
Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml new file +8 -0
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    // Example 1 (Per-hart encoder and ramsink components):
+
+    encoder@c000000 {

trace@...?

Okay, I will update.

Quoted 9 lines

Node names should be generic and a given compatible should only have 1 possible node name. (Yes, we failed to do this on Arm coresight stuff.)

Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml line 65

Quoted 3 lines
Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml new file +3 -0
+      compatible = "qemu,trace-component", "riscv,trace-component";
+      reg = <0xc000000 0x1000>;
+      cpus = <&CPU0>;

blank line between properties and child nodes.

Okay, I will update.

Quoted 43 lines
Quoted 43 lines
Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml new file +42 -0
+      out-ports {
+        port {
+          CPU0_ENCODER_OUTPUT: endpoint {
+            remote-endpoint = <&CPU0_RAMSINK_INPUT>;
+          };
+        };
+      };
+    };
+
+    ramsink@c001000 {
+      compatible = "qemu,trace-component", "riscv,trace-component";
+      reg = <0xc001000 0x1000>;
+      cpus = <&CPU0>;
+      in-ports {
+        port {
+          CPU0_RAMSINK_INPUT: endpoint {
+          };
+        };
+      };
+    };
+
+    encoder@c002000 {
+      compatible = "qemu,trace-component", "riscv,trace-component";
+      reg = <0xc002000 0x1000>;
+      cpus = <&CPU1>;
+      out-ports {
+        port {
+          CPU1_ENCODER_OUTPUT: endpoint {
+            remote-endpoint = <&CPU1_RAMSINK_INPUT>;
+          };
+        };
+      };
+    };
+
+    ramsink@c003000 {
+      compatible = "qemu,trace-component", "riscv,trace-component";
+      reg = <0xc003000 0x1000>;
+      cpus = <&CPU1>;
+      in-ports {
+        port {
+          CPU1_RAMSINK_INPUT: endpoint {
+          };
Quoted 8 lines
Quoted 8 lines
Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml new file +5 -1
+        };
+      };
+    };
+
+...
--

2.43.0

Regards,
Anup

author
#4 lore raw

PATCH v2 2/12 rvtrace: Initial implementation of driver framework

In reply to Anup Patel

The RISC-V Trace Control Interface Specification [1] defines a standard way of implementing RISC-V trace related modular components irrespective to underlying trace format (E-trace or N-trace). These RISC-V trace components are organized in a graph-like topology where each RISC-V hart has its own RISC-V trace encoder component.

Implement a basic driver framework for RISC-V trace where RISC-V trace components are instantiated by a common platform driver and a separate RISC-V trace driver for each type of RISC-V trace component.

[1] https://github.com/riscv-non-isa/tg-nexus-trace/releases/download/1.0_Ratified/RISC-V-Trace-Control-Interface.pdf

---
 drivers/Makefile                             |   1 +
 drivers/hwtracing/Kconfig                    |   2 +
 drivers/hwtracing/rvtrace/Kconfig            |  16 +
 drivers/hwtracing/rvtrace/Makefile           |   4 +
 drivers/hwtracing/rvtrace/rvtrace-core.c     | 501 +++++++++++++++++++
 drivers/hwtracing/rvtrace/rvtrace-platform.c | 192 +++++++
 include/linux/rvtrace.h                      | 280 +++++++++++
 7 files changed, 996 insertions(+)
 create mode 100644 drivers/hwtracing/rvtrace/Kconfig
 create mode 100644 drivers/hwtracing/rvtrace/Makefile
 create mode 100644 drivers/hwtracing/rvtrace/rvtrace-core.c
 create mode 100644 drivers/hwtracing/rvtrace/rvtrace-platform.c
 create mode 100644 include/linux/rvtrace.h
drivers/Makefile +1 -0
index 8e1ffa4358d5..3c0c3bd40a89 100644
@@ -178,6 +178,7 @@ obj-$(CONFIG_CORESIGHT)		+= hwtracing/coresight/
 obj-y				+= hwtracing/intel_th/
 obj-$(CONFIG_STM)		+= hwtracing/stm/
 obj-$(CONFIG_HISI_PTT)		+= hwtracing/ptt/
+obj-$(CONFIG_RVTRACE)		+= hwtracing/rvtrace/
 obj-y				+= android/
 obj-$(CONFIG_NVMEM)		+= nvmem/
 obj-$(CONFIG_FPGA)		+= fpga/
drivers/hwtracing/Kconfig +2 -0
index 911ee977103c..daeb38fe332d 100644
@@ -7,4 +7,6 @@ source "drivers/hwtracing/intel_th/Kconfig"
 
 source "drivers/hwtracing/ptt/Kconfig"
 
+source "drivers/hwtracing/rvtrace/Kconfig"
+
 endmenu
drivers/hwtracing/rvtrace/Kconfig new file +16 -0
new file mode 100644
index 000000000000..f8f6feea1953
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+menuconfig RVTRACE
+	tristate "RISC-V Trace Support"
+	depends on RISCV
+	depends on OF
+	default RISCV
+	help
+	  This framework provides a kernel interface for the RISC-V trace
+	  drivers (including both e-trace and n-trace). It's intended to
+	  build a topological view of the RISC-V trace components and
+	  configure the right series of components when trace is enabled
+	  on a CPU.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called rvtrace.
drivers/hwtracing/rvtrace/Makefile new file +4 -0
new file mode 100644
index 000000000000..988525a379cf
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_RVTRACE) += rvtrace.o
+rvtrace-y := rvtrace-core.o rvtrace-platform.o
drivers/hwtracing/rvtrace/rvtrace-core.c new file +501 -0

501 lines not on this page · raw message

drivers/hwtracing/rvtrace/rvtrace-platform.c new file +192 -0

192 lines not on this page · raw message

include/linux/rvtrace.h new file +280 -0

280 lines not on this page · raw message

Signature · 1 line
2.43.0
Reviewed-by on v2 2/12
#5 lore raw

Re: [PATCH v2 02/12] rvtrace: Initial implementation of driver framework

Quoted 24 lines · Anup Patel

The RISC-V Trace Control Interface Specification [1] defines a standard way of implementing RISC-V trace related modular components irrespective to underlying trace format (E-trace or N-trace). These RISC-V trace components are organized in a graph-like topology where each RISC-V hart has its own RISC-V trace encoder component.

Implement a basic driver framework for RISC-V trace where RISC-V trace components are instantiated by a common platform driver and a separate RISC-V trace driver for each type of RISC-V trace component.

[1] https://github.com/riscv-non-isa/tg-nexus-trace/releases/download/1.0_Ratified/RISC-V-Trace-Control-Interface.pdf

---
  drivers/Makefile                             |   1 +
  drivers/hwtracing/Kconfig                    |   2 +
  drivers/hwtracing/rvtrace/Kconfig            |  16 +
  drivers/hwtracing/rvtrace/Makefile           |   4 +
  drivers/hwtracing/rvtrace/rvtrace-core.c     | 501 +++++++++++++++++++
  drivers/hwtracing/rvtrace/rvtrace-platform.c | 192 +++++++
  include/linux/rvtrace.h                      | 280 +++++++++++
  7 files changed, 996 insertions(+)
Quoted 5 lines

create mode 100644 drivers/hwtracing/rvtrace/Kconfig create mode 100644 drivers/hwtracing/rvtrace/Makefile create mode 100644 drivers/hwtracing/rvtrace/rvtrace-core.c create mode 100644 drivers/hwtracing/rvtrace/rvtrace-platform.c create mode 100644 include/linux/rvtrace.h

Glad to see this series and look forward to supporting 'perf kvm'.

Thanks,
Nutty

#6 lore raw

Re: [PATCH v2 02/12] rvtrace: Initial implementation of driver framework

In reply to Anup Patel

Hi Anup,

See my comments below. I'll probably have another round of review later so expect more comments coming.

Quoted 263 lines · Anup Patel

The RISC-V Trace Control Interface Specification [1] defines a standard way of implementing RISC-V trace related modular components irrespective to underlying trace format (E-trace or N-trace). These RISC-V trace components are organized in a graph-like topology where each RISC-V hart has its own RISC-V trace encoder component.

Implement a basic driver framework for RISC-V trace where RISC-V trace components are instantiated by a common platform driver and a separate RISC-V trace driver for each type of RISC-V trace component.

[1] https://github.com/riscv-non-isa/tg-nexus-trace/releases/download/1.0_Ratified/RISC-V-Trace-Control-Interface.pdf

---
  drivers/Makefile                             |   1 +
  drivers/hwtracing/Kconfig                    |   2 +
  drivers/hwtracing/rvtrace/Kconfig            |  16 +
  drivers/hwtracing/rvtrace/Makefile           |   4 +
  drivers/hwtracing/rvtrace/rvtrace-core.c     | 501 +++++++++++++++++++
  drivers/hwtracing/rvtrace/rvtrace-platform.c | 192 +++++++
  include/linux/rvtrace.h                      | 280 +++++++++++
  7 files changed, 996 insertions(+)

create mode 100644 drivers/hwtracing/rvtrace/Kconfig create mode 100644 drivers/hwtracing/rvtrace/Makefile create mode 100644 drivers/hwtracing/rvtrace/rvtrace-core.c create mode 100644 drivers/hwtracing/rvtrace/rvtrace-platform.c create mode 100644 include/linux/rvtrace.h

drivers/Makefile +1 -0
index 8e1ffa4358d5..3c0c3bd40a89 100644
@@ -178,6 +178,7 @@ obj-$(CONFIG_CORESIGHT)		+= hwtracing/coresight/
  obj-y				+= hwtracing/intel_th/
  obj-$(CONFIG_STM)		+= hwtracing/stm/
  obj-$(CONFIG_HISI_PTT)		+= hwtracing/ptt/
+obj-$(CONFIG_RVTRACE)		+= hwtracing/rvtrace/
  obj-y				+= android/
  obj-$(CONFIG_NVMEM)		+= nvmem/
  obj-$(CONFIG_FPGA)		+= fpga/
drivers/hwtracing/Kconfig +2 -0
index 911ee977103c..daeb38fe332d 100644
@@ -7,4 +7,6 @@ source "drivers/hwtracing/intel_th/Kconfig"
 
  source "drivers/hwtracing/ptt/Kconfig"
 
+source "drivers/hwtracing/rvtrace/Kconfig"
+
  endmenu
drivers/hwtracing/rvtrace/Kconfig new file +16 -0
new file mode 100644
index 000000000000..f8f6feea1953
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+menuconfig RVTRACE
+	tristate "RISC-V Trace Support"
+	depends on RISCV
+	depends on OF
+	default RISCV
+	help
+	  This framework provides a kernel interface for the RISC-V trace
+	  drivers (including both e-trace and n-trace). It's intended to
+	  build a topological view of the RISC-V trace components and
+	  configure the right series of components when trace is enabled
+	  on a CPU.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called rvtrace.
drivers/hwtracing/rvtrace/Makefile new file +4 -0
new file mode 100644
index 000000000000..988525a379cf
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_RVTRACE) += rvtrace.o
+rvtrace-y := rvtrace-core.o rvtrace-platform.o
drivers/hwtracing/rvtrace/rvtrace-core.c new file +172 -0
new file mode 100644
index 000000000000..32fda1647b39
@@ -0,0 +1,501 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 Ventana Micro Systems Inc.
+ */
+
+#include <linux/cpumask.h>
+#include <linux/delay.h>
+#include <linux/export.h>
+#include <linux/idr.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/percpu.h>
+#include <linux/rvtrace.h>
+
+/* Mutex to serialize component registration/unregistration */
+static DEFINE_MUTEX(rvtrace_mutex);
+
+/* Per-CPU encoder instances */
+static DEFINE_PER_CPU(struct rvtrace_component *, rvtrace_cpu_encoder);
+
+/* Component type based id generator */
+struct rvtrace_type_idx {
+	/* Lock to protect the type ID generator */
+	struct mutex lock;
+	struct idr idr;
+};
+
+/* Array of component type based id generator */
+static struct rvtrace_type_idx rvtrace_type_idx_array[RVTRACE_COMPONENT_TYPE_MAX];
+
+static int rvtrace_alloc_type_idx(struct rvtrace_component *comp)
+{
+	struct rvtrace_type_idx *rvidx = &rvtrace_type_idx_array[comp->id.type];
+	int idx;
+
+	mutex_lock(&rvidx->lock);
+	idx = idr_alloc(&rvidx->idr, comp, 0, 0, GFP_KERNEL);
+	mutex_unlock(&rvidx->lock);
+	if (idx < 0)
+		return idx;
+
+	comp->type_idx = idx;
+	return 0;
+}
+
+static void rvtrace_free_type_idx(struct rvtrace_component *comp)
+{
+	struct rvtrace_type_idx *rvidx = &rvtrace_type_idx_array[comp->id.type];
+
+	mutex_lock(&rvidx->lock);
+	idr_remove(&rvidx->idr, comp->type_idx);
+	mutex_unlock(&rvidx->lock);
+}
+
+static void __init rvtrace_init_type_idx(void)
+{
+	struct rvtrace_type_idx *rvidx;
+	int i;
+
+	for (i = 0; i < RVTRACE_COMPONENT_TYPE_MAX; i++) {
+		rvidx = &rvtrace_type_idx_array[i];
+		mutex_init(&rvidx->lock);
+		idr_init(&rvidx->idr);
+	}
+}
+
+const struct rvtrace_component_id *rvtrace_match_id(struct rvtrace_component *comp,
+						    const struct rvtrace_component_id *ids)
+{
+	u32 comp_maj, comp_min, id_maj, id_min;
+	const struct rvtrace_component_id *id;
+
+	for (id = ids; id->version && id->type; id++) {
+		if (comp->id.type != id->type)
+			return NULL;
+
+		id_maj = rvtrace_component_version_major(id->version);
+		id_min = rvtrace_component_version_minor(id->version);
+		comp_maj = rvtrace_component_version_major(comp->id.version);
+		comp_min = rvtrace_component_version_minor(comp->id.version);
+		if (comp_maj > id_maj)
+			continue;
+
+		/* Refer to Ch. 5 'Versioning of components of the Trace Control spec. */
+		if (comp_maj < id_maj)
+			dev_warn(&comp->dev, "Older component with major version %d\n", comp_maj);
+		if (comp_min == 15)
+			dev_warn(&comp->dev, "Experimental component\n");
+		else if (comp_min > id_min)
+			dev_warn(&comp->dev, "Newer component with minor version %d\n", comp_min);
+
+		return id;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(rvtrace_match_id);
+
+static int rvtrace_match_device(struct device *dev, const struct device_driver *drv)
+{
+	const struct rvtrace_driver *rtdrv = to_rvtrace_driver(drv);
+	struct rvtrace_component *comp = to_rvtrace_component(dev);
+
+	return rvtrace_match_id(comp, rtdrv->id_table) ? 1 : 0;
+}
+
+static int rvtrace_probe(struct device *dev)
+{
+	const struct rvtrace_driver *rtdrv = to_rvtrace_driver(dev->driver);
+	struct rvtrace_component *comp = to_rvtrace_component(dev);
+	int ret = -ENODEV;
+
+	if (!rtdrv->probe)
+		return ret;
+
+	ret = rtdrv->probe(comp);
+	if (!ret)
+		comp->ready = true;
+
+	return ret;
+}
+
+static void rvtrace_remove(struct device *dev)
+{
+	const struct rvtrace_driver *rtdrv = to_rvtrace_driver(dev->driver);
+	struct rvtrace_component *comp = to_rvtrace_component(dev);
+
+	comp->ready = false;
+	if (rtdrv->remove)
+		rtdrv->remove(comp);
+}
+
+const struct bus_type rvtrace_bustype = {
+	.name	= "rvtrace",
+	.match	= rvtrace_match_device,
+	.probe	= rvtrace_probe,
+	.remove	= rvtrace_remove,
+};
+
+struct rvtrace_fwnode_match_data {
+	struct fwnode_handle *fwnode;
+	struct rvtrace_component *match;
+};
+
+static int rvtrace_match_fwnode(struct device *dev, void *data)
+{
+	struct rvtrace_component *comp = to_rvtrace_component(dev);
+	struct rvtrace_fwnode_match_data *d = data;
+
+	if (device_match_fwnode(&comp->dev, d->fwnode)) {
+		d->match = comp;
+		return 1;
+	}
+
+	return 0;
+}
+
+struct rvtrace_component *rvtrace_find_by_fwnode(struct fwnode_handle *fwnode)
+{
+	struct rvtrace_fwnode_match_data d = { .fwnode = fwnode, .match = NULL };
+	int ret;
+
+	ret = bus_for_each_dev(&rvtrace_bustype, NULL, &d, rvtrace_match_fwnode);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	return d.match;
+}
+EXPORT_SYMBOL_GPL(rvtrace_find_by_fwnode);
+
+int rvtrace_poll_bit(struct rvtrace_platform_data *pdata, int offset,
+		     int bit, int bitval, int timeout)

drivers/hwtracing/rvtrace/rvtrace-core.c line 182

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-core.c new file +10 -0
+{
+	int i = 10;
+	u32 val;
+
+	while (i--) {
+		val = rvtrace_read32(pdata, offset);
+		if (((val >> bit) & 0x1) == bitval)
+			break;
+		udelay(timeout);
+	}

I suggest we use delay(1) and let i = timeout. With i = 10, we basically are using "timeout" as the polling interval, and 10* polling_interval as the actual timeout value. It's not clear to the user and may cause some confusion. delay(1) also helps to poll faster.

Quoted 5 lines
drivers/hwtracing/rvtrace/rvtrace-core.c new file +5 -0
+
+	return (i < 0) ? -ETIMEDOUT : 0;
+}
+EXPORT_SYMBOL_GPL(rvtrace_poll_bit);
+

drivers/hwtracing/rvtrace/rvtrace-core.c line 197

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-core.c new file +10 -0
+int rvtrace_enable_component(struct rvtrace_component *comp)
+{
+	u32 val;
+
+	val = rvtrace_read32(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET);
+	val |= BIT(RVTRACE_COMPONENT_CTRL_ENABLE_SHIFT);
+	rvtrace_write32(comp->pdata, val, RVTRACE_COMPONENT_CTRL_OFFSET);
+	return rvtrace_poll_bit(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET,
+				RVTRACE_COMPONENT_CTRL_ENABLE_SHIFT, 1,
+				comp->pdata->control_poll_timeout_usecs);

Suggest adding 2 more helper functions such as:

rvtrace_set_poll_bit()
rvtrace_unset_poll_bit()

It'll simplify several use cases that need to set/unset and poll for that bit to get set/unset.

Quoted 140 lines
drivers/hwtracing/rvtrace/rvtrace-core.c new file +140 -0
+}
+EXPORT_SYMBOL_GPL(rvtrace_enable_component);
+
+int rvtrace_disable_component(struct rvtrace_component *comp)
+{
+	u32 val;
+
+	val = rvtrace_read32(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET);
+	val &= ~BIT(RVTRACE_COMPONENT_CTRL_ENABLE_SHIFT);
+	rvtrace_write32(comp->pdata, val, RVTRACE_COMPONENT_CTRL_OFFSET);
+	return rvtrace_poll_bit(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET,
+				RVTRACE_COMPONENT_CTRL_ENABLE_SHIFT, 0,
+				comp->pdata->control_poll_timeout_usecs);
+}
+EXPORT_SYMBOL_GPL(rvtrace_disable_component);
+
+struct rvtrace_component *rvtrace_cpu_source(unsigned int cpu)
+{
+	if (!cpu_present(cpu))
+		return NULL;
+
+	return per_cpu(rvtrace_cpu_encoder, cpu);
+}
+EXPORT_SYMBOL_GPL(rvtrace_cpu_source);
+
+static int rvtrace_cleanup_inconn(struct device *dev, void *data)
+{
+	struct rvtrace_component *comp = to_rvtrace_component(dev);
+	struct rvtrace_platform_data *pdata = comp->pdata;
+	struct rvtrace_connection *conn = data;
+	int i;
+
+	if (device_match_fwnode(&comp->dev, conn->dest_fwnode)) {
+		for (i = 0; i < pdata->nr_inconns; i++) {
+			if (pdata->inconns[i] != conn)
+				continue;
+			pdata->inconns[i] = NULL;
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+static void rvtrace_cleanup_inconns_from_outconns(struct rvtrace_component *comp)
+{
+	struct rvtrace_platform_data *pdata = comp->pdata;
+	struct rvtrace_connection *conn;
+	int i;
+
+	lockdep_assert_held(&rvtrace_mutex);
+
+	for (i = 0; i < pdata->nr_outconns; i++) {
+		conn = pdata->outconns[i];
+		bus_for_each_dev(&rvtrace_bustype, NULL, conn, rvtrace_cleanup_inconn);
+	}
+}
+
+static int rvtrace_setup_inconn(struct device *dev, void *data)
+{
+	struct rvtrace_component *comp = to_rvtrace_component(dev);
+	struct rvtrace_platform_data *pdata = comp->pdata;
+	struct rvtrace_connection *conn = data;
+	int i;
+
+	if (device_match_fwnode(&comp->dev, conn->dest_fwnode)) {
+		for (i = 0; i < pdata->nr_inconns; i++) {
+			if (pdata->inconns[i])
+				continue;
+			pdata->inconns[i] = conn;
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+static int rvtrace_setup_inconns_from_outconns(struct rvtrace_component *comp)
+{
+	struct rvtrace_platform_data *pdata = comp->pdata;
+	struct rvtrace_connection *conn;
+	int i, ret;
+
+	lockdep_assert_held(&rvtrace_mutex);
+
+	for (i = 0; i < pdata->nr_outconns; i++) {
+		conn = pdata->outconns[i];
+		ret = bus_for_each_dev(&rvtrace_bustype, NULL, conn, rvtrace_setup_inconn);
+		if (ret < 0) {
+			rvtrace_cleanup_inconns_from_outconns(comp);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static void rvtrace_component_release(struct device *dev)
+{
+	struct rvtrace_component *comp = to_rvtrace_component(dev);
+
+	fwnode_handle_put(comp->dev.fwnode);
+	rvtrace_free_type_idx(comp);
+	kfree(comp);
+}
+
+static int rvtrace_component_reset(struct rvtrace_platform_data *pdata)
+{
+	int ret;
+
+	rvtrace_write32(pdata, 0, RVTRACE_COMPONENT_CTRL_OFFSET);
+	ret = rvtrace_poll_bit(pdata, RVTRACE_COMPONENT_CTRL_OFFSET,
+			       RVTRACE_COMPONENT_CTRL_ACTIVE_SHIFT, 0,
+			       pdata->control_poll_timeout_usecs);
+	if (ret)
+		return ret;
+
+	rvtrace_write32(pdata, RVTRACE_COMPONENT_CTRL_ACTIVE_MASK,
+			RVTRACE_COMPONENT_CTRL_OFFSET);
+	return rvtrace_poll_bit(pdata, RVTRACE_COMPONENT_CTRL_OFFSET,
+				RVTRACE_COMPONENT_CTRL_ACTIVE_SHIFT, 1,
+				pdata->control_poll_timeout_usecs);
+}
+
+struct rvtrace_component *rvtrace_register_component(struct rvtrace_platform_data *pdata)
+{
+	struct rvtrace_connection *conn;
+	struct rvtrace_component *comp;
+	u32 impl, type, major, minor;
+	int i, ret = 0;
+
+	if (!pdata || !pdata->dev) {
+		ret = -EINVAL;
+		goto err_out;
+	}
+
+	for (i = 0; i < pdata->nr_inconns; i++) {
+		if (pdata->inconns[i]) {
+			ret = -EINVAL;
+			goto err_out;

drivers/hwtracing/rvtrace/rvtrace-core.c line 347

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-core.c new file +10 -0
+		}
+	}
+
+	for (i = 0; i < pdata->nr_outconns; i++) {
+		conn = pdata->outconns[i];
+		if (!conn || conn->src_port < 0 || conn->src_comp ||
+		    !device_match_fwnode(pdata->dev, conn->src_fwnode) ||
+		    conn->dest_port < 0 || !conn->dest_fwnode || !conn->dest_comp) {
+			ret = -EINVAL;
+			goto err_out;

I feel like this complex condition should be simplified. If tested false, it indicates a logic error in the code, rather than a failure to validate some external input, such as the topology in the device-tree, as the para- meters are all controlled by the caller. They are not simply de-serialized from device-tree or something. Perhaps use BUG_ON on some of them to do assert()?

Quoted 198 lines
drivers/hwtracing/rvtrace/rvtrace-core.c new file +154 -0
+		}
+	}
+
+	ret = rvtrace_component_reset(pdata);
+	if (ret)
+		goto err_out;
+
+	impl = rvtrace_read32(pdata, RVTRACE_COMPONENT_IMPL_OFFSET);
+	type = (impl >> RVTRACE_COMPONENT_IMPL_TYPE_SHIFT) &
+		RVTRACE_COMPONENT_IMPL_TYPE_MASK;
+	major = (impl >> RVTRACE_COMPONENT_IMPL_VERMAJOR_SHIFT) &
+		RVTRACE_COMPONENT_IMPL_VERMAJOR_MASK;
+	minor = (impl >> RVTRACE_COMPONENT_IMPL_VERMINOR_SHIFT) &
+		RVTRACE_COMPONENT_IMPL_VERMINOR_MASK;
+
+	if (pdata->bound_cpu >= 0 && !cpu_present(pdata->bound_cpu)) {
+		ret = -EINVAL;
+		goto err_out;
+	}
+	if (type == RVTRACE_COMPONENT_TYPE_ENCODER && pdata->bound_cpu < 0) {
+		ret = -EINVAL;
+		goto err_out;
+	}
+
+	comp = kzalloc(sizeof(*comp), GFP_KERNEL);
+	if (!comp) {
+		ret = -ENOMEM;
+		goto err_out;
+	}
+	comp->pdata = pdata;
+	comp->id.type = type;
+	comp->id.version = rvtrace_component_mkversion(major, minor);
+	ret = rvtrace_alloc_type_idx(comp);
+	if (ret) {
+		kfree(comp);
+		goto err_out;
+	}
+
+	comp->dev.parent = pdata->dev;
+	comp->dev.coherent_dma_mask = pdata->dev->coherent_dma_mask;
+	comp->dev.release = rvtrace_component_release;
+	comp->dev.bus = &rvtrace_bustype;
+	comp->dev.fwnode = fwnode_handle_get(dev_fwnode(pdata->dev));
+	switch (comp->id.type) {
+	case RVTRACE_COMPONENT_TYPE_ENCODER:
+		dev_set_name(&comp->dev, "encoder-%d", comp->type_idx);
+		break;
+	case RVTRACE_COMPONENT_TYPE_FUNNEL:
+		dev_set_name(&comp->dev, "funnel-%d", comp->type_idx);
+		break;
+	case RVTRACE_COMPONENT_TYPE_RAMSINK:
+		dev_set_name(&comp->dev, "ramsink-%d", comp->type_idx);
+		break;
+	case RVTRACE_COMPONENT_TYPE_PIBSINK:
+		dev_set_name(&comp->dev, "pibsink-%d", comp->type_idx);
+		break;
+	case RVTRACE_COMPONENT_TYPE_ATBBRIDGE:
+		dev_set_name(&comp->dev, "atbbridge-%d", comp->type_idx);
+		break;
+	default:
+		dev_set_name(&comp->dev, "type%d-%d", comp->id.type, comp->type_idx);
+		break;
+	}
+
+	mutex_lock(&rvtrace_mutex);
+
+	ret = device_register(&comp->dev);
+	if (ret) {
+		put_device(&comp->dev);
+		goto err_out_unlock;
+	}
+
+	for (i = 0; i < pdata->nr_outconns; i++) {
+		conn = pdata->outconns[i];
+		conn->src_comp = comp;
+	}
+
+	ret = rvtrace_setup_inconns_from_outconns(comp);
+	if (ret < 0) {
+		device_unregister(&comp->dev);
+		goto err_out_unlock;
+	}
+
+	if (comp->id.type == RVTRACE_COMPONENT_TYPE_ENCODER) {
+		rvtrace_get_component(comp);
+		per_cpu(rvtrace_cpu_encoder, comp->pdata->bound_cpu) = comp;
+	}
+
+	mutex_unlock(&rvtrace_mutex);
+
+	return comp;
+
+err_out_unlock:
+	mutex_unlock(&rvtrace_mutex);
+err_out:
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(rvtrace_register_component);
+
+void rvtrace_unregister_component(struct rvtrace_component *comp)
+{
+	struct rvtrace_component *c;
+
+	mutex_lock(&rvtrace_mutex);
+
+	if (comp->id.type == RVTRACE_COMPONENT_TYPE_ENCODER) {
+		c = per_cpu(rvtrace_cpu_encoder, comp->pdata->bound_cpu);
+		per_cpu(rvtrace_cpu_encoder, comp->pdata->bound_cpu) = NULL;
+		rvtrace_put_component(c);
+	}
+
+	rvtrace_cleanup_inconns_from_outconns(comp);
+	device_unregister(&comp->dev);
+
+	mutex_unlock(&rvtrace_mutex);
+}
+EXPORT_SYMBOL_GPL(rvtrace_unregister_component);
+
+int __rvtrace_register_driver(struct module *owner, struct rvtrace_driver *rtdrv)
+{
+	rtdrv->driver.owner = owner;
+	rtdrv->driver.bus = &rvtrace_bustype;
+
+	return driver_register(&rtdrv->driver);
+}
+EXPORT_SYMBOL_GPL(__rvtrace_register_driver);
+
+static int __init rvtrace_init(void)
+{
+	int ret;
+
+	rvtrace_init_type_idx();
+
+	ret = bus_register(&rvtrace_bustype);
+	if (ret)
+		return ret;
+
+	ret = platform_driver_register(&rvtrace_platform_driver);
+	if (ret) {
+		bus_unregister(&rvtrace_bustype);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void __exit rvtrace_exit(void)
+{
+	platform_driver_unregister(&rvtrace_platform_driver);
+	bus_unregister(&rvtrace_bustype);
+}
+
+module_init(rvtrace_init);
+module_exit(rvtrace_exit);
drivers/hwtracing/rvtrace/rvtrace-platform.c new file +38 -0
new file mode 100644
index 000000000000..6eb362880e5c
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 Ventana Micro Systems Inc.
+ */
+
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_graph.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/rvtrace.h>
+#include <linux/types.h>
+
+static int rvtrace_of_parse_outconns(struct rvtrace_platform_data *pdata)
+{
+	struct device_node *parent, *ep_node, *rep_node, *rdev_node;
+	struct rvtrace_connection *conn;
+	struct of_endpoint ep, rep;
+	int ret = 0, i = 0;
+
+	parent = of_get_child_by_name(dev_of_node(pdata->dev), "out-ports");
+	if (!parent)
+		return 0;
+
+	pdata->nr_outconns = of_graph_get_endpoint_count(parent);
+	pdata->outconns = devm_kcalloc(pdata->dev, pdata->nr_outconns,
+				       sizeof(*pdata->outconns), GFP_KERNEL);
+	if (!pdata->outconns) {
+		ret = -ENOMEM;
+		goto done;
+	}
+
+	for_each_endpoint_of_node(parent, ep_node) {
+		conn = devm_kzalloc(pdata->dev, sizeof(*conn), GFP_KERNEL);
+		if (!conn) {
+			of_node_put(ep_node);
+			ret = -ENOMEM;

drivers/hwtracing/rvtrace/rvtrace-platform.c line 48

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-platform.c new file +10 -0
+			break;
+		}
+
+		ret = of_graph_parse_endpoint(ep_node, &ep);
+		if (ret) {
+			of_node_put(ep_node);
+			break;
+		}
+
+		rep_node = of_graph_get_remote_endpoint(ep_node);

There's no put() of the rep_node. I printed the refcount of rep_node, and confirmed that it's always increasing, and never decrements even after the rvtrace.ko gets unloaded. Needs to be fixed.

drivers/hwtracing/rvtrace/rvtrace-platform.c line 54

Quoted 6 lines
drivers/hwtracing/rvtrace/rvtrace-platform.c new file +6 -0
+		if (!rep_node) {
+			ret = -ENODEV;
+			of_node_put(ep_node);
+			break;
+		}
+		rdev_node = of_graph_get_port_parent(rep_node);

Same for rdev_node.

---

Bo

Quoted 424 lines
drivers/hwtracing/rvtrace/rvtrace-platform.c new file +138 -0
+
+		ret = of_graph_parse_endpoint(rep_node, &rep);
+		if (ret) {
+			of_node_put(ep_node);
+			break;
+		}
+
+		conn->src_port = ep.port;
+		conn->src_fwnode = dev_fwnode(pdata->dev);
+		/* The 'src_comp' is set by rvtrace_register_component() */
+		conn->src_comp = NULL;
+		conn->dest_port = rep.port;
+		conn->dest_fwnode = of_fwnode_handle(rdev_node);
+		fwnode_handle_get(conn->dest_fwnode);
+		conn->dest_comp = rvtrace_find_by_fwnode(conn->dest_fwnode);
+		if (!conn->dest_comp) {
+			ret = -EPROBE_DEFER;
+			of_node_put(ep_node);
+			break;
+		}
+
+		pdata->outconns[i] = conn;
+		i++;
+	}
+
+done:
+	if (ret) {
+		for (i = 0; i < pdata->nr_outconns; i++) {
+			conn = pdata->outconns[i];
+			if (conn && conn->dest_fwnode)
+				fwnode_handle_put(conn->dest_fwnode);
+		}
+	}
+	of_node_put(parent);
+	return ret;
+}
+
+static int rvtrace_of_parse_inconns(struct rvtrace_platform_data *pdata)
+{
+	struct device_node *parent;
+	int ret = 0;
+
+	parent = of_get_child_by_name(dev_of_node(pdata->dev), "in-ports");
+	if (!parent)
+		return 0;
+
+	pdata->nr_inconns = of_graph_get_endpoint_count(parent);
+	pdata->inconns = devm_kcalloc(pdata->dev, pdata->nr_inconns,
+				      sizeof(*pdata->inconns), GFP_KERNEL);
+	if (!pdata->inconns)
+		ret = -ENOMEM;
+
+	of_node_put(parent);
+	return ret;
+}
+
+static int rvtrace_platform_probe(struct platform_device *pdev)
+{
+	struct rvtrace_platform_data *pdata;
+	struct device *dev = &pdev->dev;
+	struct rvtrace_component *comp;
+	struct device_node *node;
+	struct resource *res;
+	int ret;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+	pdata->dev = dev;
+	pdata->impid = RVTRACE_COMPONENT_IMPID_UNKNOWN;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -EINVAL;
+
+	pdata->io_mem = true;
+	pdata->base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	if (!pdata->base)
+		return dev_err_probe(dev, -ENOMEM, "failed to ioremap %pR\n", res);
+
+	pdata->bound_cpu = -1;
+	node = of_parse_phandle(dev_of_node(dev), "cpus", 0);
+	if (node) {
+		ret = of_cpu_node_to_id(node);
+		of_node_put(node);
+		if (ret < 0)
+			return dev_err_probe(dev, ret, "failed to get CPU id for %pOF\n", node);
+		pdata->bound_cpu = ret;
+	}
+
+	/* Default control poll timeout */
+	pdata->control_poll_timeout_usecs = 10;
+
+	ret = rvtrace_of_parse_outconns(pdata);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to parse output connections\n");
+
+	ret = rvtrace_of_parse_inconns(pdata);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to parse input connections\n");
+
+	comp = rvtrace_register_component(pdata);
+	if (IS_ERR(comp))
+		return PTR_ERR(comp);
+
+	platform_set_drvdata(pdev, comp);
+	return 0;
+}
+
+static void rvtrace_platform_remove(struct platform_device *pdev)
+{
+	struct rvtrace_component *comp = platform_get_drvdata(pdev);
+	struct rvtrace_platform_data *pdata = comp->pdata;
+	struct rvtrace_connection *conn;
+	int i;
+
+	for (i = 0; i < pdata->nr_outconns; i++) {
+		conn = pdata->outconns[i];
+		if (conn && conn->dest_fwnode)
+			fwnode_handle_put(conn->dest_fwnode);
+	}
+
+	rvtrace_unregister_component(comp);
+}
+
+static const struct of_device_id rvtrace_platform_match[] = {
+	{ .compatible = "riscv,trace-component" },
+	{}
+};
+
+struct platform_driver rvtrace_platform_driver = {
+	.driver = {
+		.name		= "rvtrace",
+		.of_match_table	= rvtrace_platform_match,
+	},
+	.probe = rvtrace_platform_probe,
+	.remove = rvtrace_platform_remove,
+};
include/linux/rvtrace.h new file +280 -0
new file mode 100644
index 000000000000..e8e055ad6583
@@ -0,0 +1,280 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2025 Ventana Micro Systems Inc.
+ */
+
+#ifndef __LINUX_RVTRACE_H__
+#define __LINUX_RVTRACE_H__
+
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/types.h>
+
+/* Control register common across all RISC-V trace components */
+#define RVTRACE_COMPONENT_CTRL_OFFSET		0x000
+#define RVTRACE_COMPONENT_CTRL_ACTIVE_MASK	0x1
+#define RVTRACE_COMPONENT_CTRL_ACTIVE_SHIFT	0
+#define RVTRACE_COMPONENT_CTRL_ENABLE_MASK	0x1
+#define RVTRACE_COMPONENT_CTRL_ENABLE_SHIFT	1
+#define RVTRACE_COMPONENT_CTRL_EMPTY_SHIFT	3
+
+/* Implementation register common across all RISC-V trace components */
+#define RVTRACE_COMPONENT_IMPL_OFFSET		0x004
+#define RVTRACE_COMPONENT_IMPL_VERMAJOR_MASK	0xf
+#define RVTRACE_COMPONENT_IMPL_VERMAJOR_SHIFT	0
+#define RVTRACE_COMPONENT_IMPL_VERMINOR_MASK	0xf
+#define RVTRACE_COMPONENT_IMPL_VERMINOR_SHIFT	4
+#define RVTRACE_COMPONENT_IMPL_TYPE_MASK	0xf
+#define RVTRACE_COMPONENT_IMPL_TYPE_SHIFT	8
+
+/* Possible component types defined by the RISC-V Trace Control Interface */
+enum rvtrace_component_type {
+	RVTRACE_COMPONENT_TYPE_RESV0,
+	RVTRACE_COMPONENT_TYPE_ENCODER, /* 0x1 */
+	RVTRACE_COMPONENT_TYPE_RESV2,
+	RVTRACE_COMPONENT_TYPE_RESV3,
+	RVTRACE_COMPONENT_TYPE_RESV4,
+	RVTRACE_COMPONENT_TYPE_RESV5,
+	RVTRACE_COMPONENT_TYPE_RESV6,
+	RVTRACE_COMPONENT_TYPE_RESV7,
+	RVTRACE_COMPONENT_TYPE_FUNNEL, /* 0x8 */
+	RVTRACE_COMPONENT_TYPE_RAMSINK, /* 0x9 */
+	RVTRACE_COMPONENT_TYPE_PIBSINK, /* 0xA */
+	RVTRACE_COMPONENT_TYPE_RESV11,
+	RVTRACE_COMPONENT_TYPE_RESV12,
+	RVTRACE_COMPONENT_TYPE_RESV13,
+	RVTRACE_COMPONENT_TYPE_ATBBRIDGE, /* 0xE */
+	RVTRACE_COMPONENT_TYPE_RESV15,
+	RVTRACE_COMPONENT_TYPE_MAX
+};
+
+/* Encoding/decoding macros for RISC-V trace component version */
+#define rvtrace_component_version_major(__version)	\
+	(((__version) >> 16) & 0xffff)
+#define rvtrace_component_version_minor(__version)	\
+	((__version) & 0xffff)
+#define rvtrace_component_mkversion(__major, __minor)	\
+	((((__major) & 0xffff) << 16) |	((__minor) & 0xffff))
+
+/*
+ * Possible component implementation IDs discovered from DT or ACPI
+ * shared across the RISC-V trace drivers to infer trace parameters,
+ * quirks, and work-arounds. These component implementation IDs are
+ * internal to Linux and must not be exposed to user-space.
+ *
+ * The component implementation ID should be named as follows:
+ *    RVTRACE_COMPONENT_IMPID_<vendor>_<part>
+ */
+enum rvtrace_component_impid {
+	RVTRACE_COMPONENT_IMPID_UNKNOWN,
+	RVTRACE_COMPONENT_IMPID_MAX
+};
+
+/**
+ * struct rvtrace_connection - Representation of a physical connection between
+ * two RISC-V trace components.
+ * @src_port:    A connection's source port number.
+ * @src_fwnode:  Source component's fwnode handle..
+ * @src_comp:    Source component's pointer.
+ * @dest_port:   A connection's destination port number.
+ * @dest_fwnode: Destination component's fwnode handle.
+ * @dest_comp:   Destination component's pointer.
+ */
+struct rvtrace_connection {
+	int src_port;
+	struct fwnode_handle *src_fwnode;
+	int dest_port;
+	struct fwnode_handle *dest_fwnode;
+	struct rvtrace_component *src_comp;
+	struct rvtrace_component *dest_comp;
+};
+
+/**
+ * struct rvtrace_platform_data - Platform-level data for a RISC-V trace component
+ * discovered from DT or ACPI.
+ * @dev:         Parent device.
+ * @impid:       Component implementation ID
+ * @io_mem:      Flag showing whether component registers are memory mapped.
+ * @base:        If io_mem == true then base address of the memory mapped registers.
+ * @read:        If io_mem == false then read register from the given "offset".
+ * @write:       If io_mem == false then write register to the given "offset".
+ * @bound_cpu:   CPU to which the component is bound. This should be -1 if
+ *               the component is not bound to any CPU. For encoder component
+ *               type this must not be -1.
+ * @nr_inconns:  Number of input connections.
+ * @inconns:     Array of pointers to input connections.
+ * @nr_outconns: Number of output connections.
+ * @outconns:    Array of pointers to output connections.
+ */
+struct rvtrace_platform_data {
+	struct device *dev;
+
+	enum rvtrace_component_impid impid;
+
+	bool io_mem;
+	union {
+		void __iomem *base;
+		struct {
+			u32 (*read)(struct rvtrace_platform_data *pdata,
+				    u32 offset, bool relaxed);
+			void (*write)(struct rvtrace_platform_data *pdata,
+				      u32 val, u32 offset, bool relaxed);
+		};
+	};
+
+	int bound_cpu;
+
+	/* Delay in microseconds when polling control register bits */
+	int control_poll_timeout_usecs;
+
+	/*
+	 * Platform driver must only populate empty pointer array without
+	 * any actual input connections.
+	 */
+	unsigned int nr_inconns;
+	struct rvtrace_connection **inconns;
+
+	/*
+	 * Platform driver must fully populate pointer array with individual
+	 * array elements pointing to actual output connections. The src_comp
+	 * of each output connection is automatically updated at the time of
+	 * registering component.
+	 */
+	unsigned int nr_outconns;
+	struct rvtrace_connection **outconns;
+};
+
+static inline u32 rvtrace_read32(struct rvtrace_platform_data *pdata, u32 offset)
+{
+	if (likely(pdata->io_mem))
+		return readl(pdata->base + offset);
+
+	return pdata->read(pdata, offset, false);
+}
+
+static inline u32 rvtrace_relaxed_read32(struct rvtrace_platform_data *pdata, u32 offset)
+{
+	if (likely(pdata->io_mem))
+		return readl_relaxed(pdata->base + offset);
+
+	return pdata->read(pdata, offset, true);
+}
+
+static inline void rvtrace_write32(struct rvtrace_platform_data *pdata, u32 val, u32 offset)
+{
+	if (likely(pdata->io_mem))
+		writel(val, pdata->base + offset);
+	else
+		pdata->write(pdata, val, offset, false);
+}
+
+static inline void rvtrace_relaxed_write32(struct rvtrace_platform_data *pdata,
+					   u32 val, u32 offset)
+{
+	if (likely(pdata->io_mem))
+		writel_relaxed(val, pdata->base + offset);
+	else
+		pdata->write(pdata, val, offset, true);
+}
+
+static inline bool rvtrace_is_source(struct rvtrace_platform_data *pdata)
+{
+	return !pdata->nr_inconns ? true : false;
+}
+
+static inline bool rvtrace_is_sink(struct rvtrace_platform_data *pdata)
+{
+	return !pdata->nr_outconns ? true : false;
+}
+
+/**
+ * struct rvtrace_component_id - Details to identify or match a RISC-V trace component
+ * @type:      Type of the component
+ * @version:   Version of the component
+ * @data:      Data pointer for driver use
+ */
+struct rvtrace_component_id {
+	enum rvtrace_component_type type;
+	u32 version;
+	void *data;
+};
+
+/**
+ * struct rvtrace_component - Representation of a RISC-V trace component
+ * pdata:    Pointer to underlying platform data
+ * id:       Details to match the component
+ * type_idx: Unique number based on component type
+ * dev:      Device instance
+ * ready:    Flag showing whether RISC-V trace driver was probed successfully
+ */
+struct rvtrace_component {
+	struct rvtrace_platform_data *pdata;
+	struct rvtrace_component_id id;
+	u32 type_idx;
+	struct device dev;
+	bool ready;
+};
+
+#define to_rvtrace_component(__dev)	container_of_const(__dev, struct rvtrace_component, dev)
+
+static inline void rvtrace_get_component(struct rvtrace_component *comp)
+{
+	get_device(&comp->dev);
+}
+
+static inline void rvtrace_put_component(struct rvtrace_component *comp)
+{
+	put_device(&comp->dev);
+}
+
+const struct rvtrace_component_id *rvtrace_match_id(struct rvtrace_component *comp,
+						    const struct rvtrace_component_id *ids);
+struct rvtrace_component *rvtrace_find_by_fwnode(struct fwnode_handle *fwnode);
+
+int rvtrace_poll_bit(struct rvtrace_platform_data *pdata, int offset,
+		     int bit, int bitval, int timeout);
+int rvtrace_enable_component(struct rvtrace_component *comp);
+int rvtrace_disable_component(struct rvtrace_component *comp);
+
+struct rvtrace_component *rvtrace_cpu_source(unsigned int cpu);
+
+struct rvtrace_component *rvtrace_register_component(struct rvtrace_platform_data *pdata);
+void rvtrace_unregister_component(struct rvtrace_component *comp);
+
+/**
+ * struct rvtrace_driver - Representation of a RISC-V trace driver
+ * id_table: Table to match components handled by the driver
+ * probe:    Driver probe() function
+ * remove:   Driver remove() function
+ * driver:   Device driver instance
+ */
+struct rvtrace_driver {
+	const struct rvtrace_component_id *id_table;
+	int			(*probe)(struct rvtrace_component *comp);
+	void			(*remove)(struct rvtrace_component *comp);
+	struct device_driver	driver;
+};
+
+#define to_rvtrace_driver(__drv)   \
+	((__drv) ? container_of_const((__drv), struct rvtrace_driver, driver) : NULL)
+
+extern struct platform_driver rvtrace_platform_driver;
+
+int __rvtrace_register_driver(struct module *owner, struct rvtrace_driver *rtdrv);
+#define rvtrace_register_driver(driver) __rvtrace_register_driver(THIS_MODULE, driver)
+static inline void rvtrace_unregister_driver(struct rvtrace_driver *rtdrv)
+{
+	if (rtdrv)
+		driver_unregister(&rtdrv->driver);
+}
+
+static inline int rvtrace_comp_is_empty(struct rvtrace_component *comp)
+{
+	return rvtrace_poll_bit(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET,
+				RVTRACE_COMPONENT_CTRL_EMPTY_SHIFT, 1,
+				comp->pdata->control_poll_timeout_usecs);
+}
+
+#endif
author
#7 lore raw

PATCH v2 3/12 rvtrace: Add functions to create/destroy a trace component path

In reply to Anup Patel

Trace needs to be configured on a chain of trace components which are connected to each other. These chain of components is also referred to as trace component path. Add functions to create/destroy a trace component path which will be later used by RISC-V trace perf support.

---
 drivers/hwtracing/rvtrace/rvtrace-core.c | 223 +++++++++++++++++++++++
 include/linux/rvtrace.h                  |  43 ++++-
 2 files changed, 264 insertions(+), 2 deletions(-)
drivers/hwtracing/rvtrace/rvtrace-core.c +223 -0
index 32fda1647b39..7006a4469d62 100644
@@ -211,6 +211,53 @@ int rvtrace_disable_component(struct rvtrace_component *comp)
 }
 EXPORT_SYMBOL_GPL(rvtrace_disable_component);
 
+static int __rvtrace_walk_output_components(struct rvtrace_component *comp,
+					    bool *stop, void *priv,
+					    int (*fn)(struct rvtrace_component *comp, bool *stop,
+						      struct rvtrace_connection *stop_conn,
+						      void *priv))
+{
+	struct rvtrace_connection *conn, *stop_conn = NULL;
+	struct rvtrace_platform_data *pdata = comp->pdata;
+	int i, ret;
+
+	for (i = 0; i < pdata->nr_outconns; i++) {
+		conn = pdata->outconns[i];
+		ret = __rvtrace_walk_output_components(conn->dest_comp, stop, priv, fn);
+		if (ret)
+			return ret;
+		if (*stop) {
+			stop_conn = conn;
+			break;
+		}
+	}
+
+	ret = fn(comp, stop, stop_conn, priv);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+int rvtrace_walk_output_components(struct rvtrace_component *comp, void *priv,
+				   int (*fn)(struct rvtrace_component *comp, bool *stop,
+					     struct rvtrace_connection *stop_conn,
+					     void *priv))
+{
+	bool stop = false;
+	int ret;
+
+	if (!comp || !fn)
+		return -EINVAL;
+
+	mutex_lock(&rvtrace_mutex);
+	ret = __rvtrace_walk_output_components(comp, &stop, priv, fn);
+	mutex_unlock(&rvtrace_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rvtrace_walk_output_components);
+
 struct rvtrace_component *rvtrace_cpu_source(unsigned int cpu)
 {
 	if (!cpu_present(cpu))
@@ -463,6 +510,182 @@ void rvtrace_unregister_component(struct rvtrace_component *comp)
 }
 EXPORT_SYMBOL_GPL(rvtrace_unregister_component);
 
+struct rvtrace_path_node {
+	struct list_head		head;
+	struct rvtrace_component	*comp;
+	struct rvtrace_connection	*conn;
+};
+
+struct rvtrace_component *rvtrace_path_source(struct rvtrace_path *path)
+{
+	struct rvtrace_path_node *node;
+
+	node = list_first_entry(&path->comp_list, struct rvtrace_path_node, head);
+	return node->comp;
+}
+EXPORT_SYMBOL_GPL(rvtrace_path_source);
+
+struct rvtrace_component *rvtrace_path_sink(struct rvtrace_path *path)
+{
+	struct rvtrace_path_node *node;
+
+	node = list_last_entry(&path->comp_list, struct rvtrace_path_node, head);
+	return node->comp;
+}
+EXPORT_SYMBOL_GPL(rvtrace_path_sink);
+
+static int rvtrace_assign_trace_id(struct rvtrace_path *path)
+{
+	const struct rvtrace_driver *rtdrv;
+	struct rvtrace_component *comp;
+	struct rvtrace_path_node *node;
+	int trace_id;
+
+	list_for_each_entry(node, &path->comp_list, head) {
+		comp = node->comp;
+		rtdrv = to_rvtrace_driver(comp->dev.driver);
+
+		if (!rtdrv->get_trace_id)
+			continue;
+
+		trace_id = rtdrv->get_trace_id(comp, path->mode);
+		if (trace_id > 0) {
+			path->trace_id = trace_id;
+			return 0;
+		} else if (trace_id < 0) {
+			return trace_id;
+		}
+	}
+
+	return 0;
+}
+
+static void rvtrace_unassign_trace_id(struct rvtrace_path *path)
+{
+	const struct rvtrace_driver *rtdrv;
+	struct rvtrace_component *comp;
+	struct rvtrace_path_node *node;
+
+	list_for_each_entry(node, &path->comp_list, head) {
+		comp = node->comp;
+		rtdrv = to_rvtrace_driver(comp->dev.driver);
+
+		if (!rtdrv->put_trace_id)
+			continue;
+
+		rtdrv->put_trace_id(comp, path->mode, path->trace_id);
+	}
+}
+
+static bool rvtrace_path_ready(struct rvtrace_path *path)
+{
+	struct rvtrace_path_node *node;
+
+	list_for_each_entry(node, &path->comp_list, head) {
+		if (!node->comp->ready)
+			return false;
+	}
+
+	return true;
+}
+
+struct build_path_walk_priv {
+	struct rvtrace_path		*path;
+	struct rvtrace_component	*sink;
+};
+
+static int build_path_walk_fn(struct rvtrace_component *comp, bool *stop,
+			      struct rvtrace_connection *stop_conn,
+			      void *priv)
+{
+	struct build_path_walk_priv *ppriv = priv;
+	struct rvtrace_path *path = ppriv->path;
+	struct rvtrace_path_node *node;
+
+	if ((!ppriv->sink && rvtrace_is_sink(comp->pdata)) ||
+	    (ppriv->sink && ppriv->sink == comp))
+		*stop = true;
+
+	if (*stop) {
+		node = kzalloc(sizeof(*node), GFP_KERNEL);
+		if (!path)
+			return -ENOMEM;
+		INIT_LIST_HEAD(&node->head);
+		rvtrace_get_component(comp);
+		node->comp = comp;
+		node->conn = stop_conn;
+		list_add(&node->head, &path->comp_list);
+	}
+
+	return 0;
+}
+
+static void rvtrace_release_path_nodes(struct rvtrace_path *path)
+{
+	struct rvtrace_path_node *node, *node1;
+
+	list_for_each_entry_safe(node, node1, &path->comp_list, head) {
+		list_del(&node->head);
+		rvtrace_put_component(node->comp);
+		kfree(node);
+	}
+}
+
+struct rvtrace_path *rvtrace_create_path(struct rvtrace_component *source,
+					 struct rvtrace_component *sink,
+					 enum rvtrace_component_mode mode)
+{
+	struct build_path_walk_priv priv;
+	struct rvtrace_path *path;
+	int ret = 0;
+
+	if (!source || mode >= RVTRACE_COMPONENT_MODE_MAX) {
+		ret = -EINVAL;
+		goto err_out;
+	}
+
+	path = kzalloc(sizeof(*path), GFP_KERNEL);
+	if (!path) {
+		ret = -ENOMEM;
+		goto err_out;
+	}
+	INIT_LIST_HEAD(&path->comp_list);
+	path->mode = mode;
+	path->trace_id = RVTRACE_INVALID_TRACE_ID;
+
+	priv.path = path;
+	priv.sink = sink;
+	ret = rvtrace_walk_output_components(source, &priv, build_path_walk_fn);
+	if (ret < 0)
+		goto err_release_path_nodes;
+
+	if (!rvtrace_path_ready(path)) {
+		ret = -EOPNOTSUPP;
+		goto err_release_path_nodes;
+	}
+
+	ret = rvtrace_assign_trace_id(path);
+	if (ret < 0)
+		goto err_release_path_nodes;
+
+	return path;
+
+err_release_path_nodes:
+	rvtrace_release_path_nodes(path);
+	kfree(path);
+err_out:
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(rvtrace_create_path);
+
+void rvtrace_destroy_path(struct rvtrace_path *path)
+{
+	rvtrace_unassign_trace_id(path);
+	rvtrace_release_path_nodes(path);
+	kfree(path);
+}
+EXPORT_SYMBOL_GPL(rvtrace_destroy_path);
+
 int __rvtrace_register_driver(struct module *owner, struct rvtrace_driver *rtdrv)
 {
 	rtdrv->driver.owner = owner;
include/linux/rvtrace.h +41 -2

75 lines not on this page · raw message

Signature · 1 line
2.43.0
author
#8 lore raw

PATCH v2 4/12 rvtrace: Add functions to start/stop tracing on a component path

In reply to Anup Patel

From: Mayuresh Chitale <[email protected]>

The perf driver framework needs to be able to start / stop all components in a trace component path during its operation. Add rvtrace_path_start() and rvtrace_path_stop() functions for this purpose.

---
 drivers/hwtracing/rvtrace/rvtrace-core.c | 44 ++++++++++++++++++++++++
 include/linux/rvtrace.h                  |  6 ++++
 2 files changed, 50 insertions(+)
drivers/hwtracing/rvtrace/rvtrace-core.c +44 -0
index 7006a4469d62..35bd77d6880a 100644
@@ -631,6 +631,50 @@ static void rvtrace_release_path_nodes(struct rvtrace_path *path)
 	}
 }
 
+int rvtrace_path_start(struct rvtrace_path *path)
+{
+	const struct rvtrace_driver *rtdrv;
+	struct rvtrace_component *comp;
+	struct rvtrace_path_node *node;
+	int ret;
+
+	list_for_each_entry(node, &path->comp_list, head) {
+		comp = node->comp;
+		rtdrv = to_rvtrace_driver(comp->dev.driver);
+		if (!rtdrv->start)
+			continue;
+
+		ret = rtdrv->start(comp);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rvtrace_path_start);
+
+int rvtrace_path_stop(struct rvtrace_path *path)
+{
+	const struct rvtrace_driver *rtdrv;
+	struct rvtrace_component *comp;
+	struct rvtrace_path_node *node;
+	int ret;
+
+	list_for_each_entry(node, &path->comp_list, head) {
+		comp = node->comp;
+		rtdrv = to_rvtrace_driver(comp->dev.driver);
+		if (!rtdrv->stop)
+			continue;
+
+		ret = rtdrv->stop(comp);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rvtrace_path_stop);
+
 struct rvtrace_path *rvtrace_create_path(struct rvtrace_component *source,
 					 struct rvtrace_component *sink,
 					 enum rvtrace_component_mode mode)
include/linux/rvtrace.h +6 -0
index 4b31ab96e721..e2842e8728d4 100644
@@ -274,10 +274,14 @@ struct rvtrace_path *rvtrace_create_path(struct rvtrace_component *source,
 					 struct rvtrace_component *sink,
 					 enum rvtrace_component_mode mode);
 void rvtrace_destroy_path(struct rvtrace_path *path);
+int rvtrace_path_start(struct rvtrace_path *path);
+int rvtrace_path_stop(struct rvtrace_path *path);
 
 /**
  * struct rvtrace_driver - Representation of a RISC-V trace driver
  * id_table: Table to match components handled by the driver
+ * start:        Callback to start tracing
+ * stop:         Callback to stop tracing
  * probe:        Driver probe() function
  * remove:       Driver remove() function
  * get_trace_id: Get/allocate a trace ID
@@ -286,6 +290,8 @@ void rvtrace_destroy_path(struct rvtrace_path *path);
  */
 struct rvtrace_driver {
 	const struct rvtrace_component_id *id_table;
+	int			(*start)(struct rvtrace_component *comp);
+	int			(*stop)(struct rvtrace_component *comp);
 	int			(*probe)(struct rvtrace_component *comp);
 	void			(*remove)(struct rvtrace_component *comp);
 	int			(*get_trace_id)(struct rvtrace_component *comp,
Signature · 1 line
2.43.0
author
#9 lore raw

PATCH v2 5/12 rvtrace: Add trace encoder driver

In reply to Anup Patel

From: Mayuresh Chitale <[email protected]>

Add initial implementation of RISC-V trace encoder driver. The encoder is defined in the RISC-V Trace Control Interface specification.

---
 drivers/hwtracing/rvtrace/Kconfig           |   7 ++
 drivers/hwtracing/rvtrace/Makefile          |   1 +
 drivers/hwtracing/rvtrace/rvtrace-encoder.c | 107 ++++++++++++++++++++
 3 files changed, 115 insertions(+)
 create mode 100644 drivers/hwtracing/rvtrace/rvtrace-encoder.c
drivers/hwtracing/rvtrace/Kconfig +7 -0
index f8f6feea1953..ba35c05f3f54 100644
@@ -14,3 +14,10 @@ menuconfig RVTRACE
 
 	  To compile this driver as a module, choose M here: the module
 	  will be called rvtrace.
+
+config RVTRACE_ENCODER
+	tristate "RISC-V Trace Encoder driver"
+	depends on RVTRACE
+	default y
+	help
+	  This driver provides support for RISC-V Trace Encoder component.
drivers/hwtracing/rvtrace/Makefile +1 -0
index 988525a379cf..f320693a1fc5 100644
@@ -2,3 +2,4 @@
 
 obj-$(CONFIG_RVTRACE) += rvtrace.o
 rvtrace-y := rvtrace-core.o rvtrace-platform.o
+obj-$(CONFIG_RVTRACE_ENCODER) += rvtrace-encoder.o
drivers/hwtracing/rvtrace/rvtrace-encoder.c new file +107 -0

107 lines not on this page · raw message

Signature · 1 line
2.43.0
author
#10 lore raw

PATCH v2 6/12 rvtrace: Add function to copy into perf AUX buffer

In reply to Anup Patel

The RISC-V trace ramsink will need a mechanism to copy trace data into the perf AUX buffer. Add rvtrace_path_copyto_auxbuf() function and corresponding trace driver callback copyto_auxbuf() for this purpose.

---
 drivers/hwtracing/rvtrace/rvtrace-core.c | 22 ++++++++++++++++++++++
 include/linux/rvtrace.h                  | 21 +++++++++++++++++++++
 2 files changed, 43 insertions(+)
drivers/hwtracing/rvtrace/rvtrace-core.c +22 -0
index 35bd77d6880a..b955e5f3b048 100644
@@ -675,6 +675,28 @@ int rvtrace_path_stop(struct rvtrace_path *path)
 }
 EXPORT_SYMBOL_GPL(rvtrace_path_stop);
 
+int rvtrace_path_copyto_auxbuf(struct rvtrace_path *path,
+			       struct rvtrace_perf_auxbuf *buf,
+			       size_t *bytes_copied)
+{
+	const struct rvtrace_driver *rtdrv;
+	struct rvtrace_component *comp;
+	struct rvtrace_path_node *node;
+
+	list_for_each_entry(node, &path->comp_list, head) {
+		comp = node->comp;
+		rtdrv = to_rvtrace_driver(comp->dev.driver);
+		if (!rtdrv->copyto_auxbuf)
+			continue;
+
+		*bytes_copied = rtdrv->copyto_auxbuf(comp, buf);
+		return 0;
+	}
+
+	return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL_GPL(rvtrace_path_copyto_auxbuf);
+
 struct rvtrace_path *rvtrace_create_path(struct rvtrace_component *source,
 					 struct rvtrace_component *sink,
 					 enum rvtrace_component_mode mode)
include/linux/rvtrace.h +21 -0
index e2842e8728d4..cecf6c153ca6 100644
@@ -277,9 +277,28 @@ void rvtrace_destroy_path(struct rvtrace_path *path);
 int rvtrace_path_start(struct rvtrace_path *path);
 int rvtrace_path_stop(struct rvtrace_path *path);
 
+/**
+ * struct rvtrace_perf_auxbuf - Representation of the perf AUX buffer
+ * @length:   size of the AUX buffer
+ * @nr_pages: number of pages of the AUX buffer
+ * @base:     start address of AUX buffer
+ * @pos:      position in the AUX buffer to commit traced data
+ */
+struct rvtrace_perf_auxbuf {
+	size_t length;
+	int nr_pages;
+	void *base;
+	long pos;
+};
+
+int rvtrace_path_copyto_auxbuf(struct rvtrace_path *path,
+			       struct rvtrace_perf_auxbuf *buf,
+			       size_t *bytes_copied);
+
 /**
  * struct rvtrace_driver - Representation of a RISC-V trace driver
  * id_table: Table to match components handled by the driver
+ * copyto_auxbuf:Callback to copy data into perf AUX buffer
  * start:        Callback to start tracing
  * stop:         Callback to stop tracing
  * probe:        Driver probe() function
@@ -290,6 +309,8 @@ int rvtrace_path_stop(struct rvtrace_path *path);
  */
 struct rvtrace_driver {
 	const struct rvtrace_component_id *id_table;
+	size_t			(*copyto_auxbuf)(struct rvtrace_component *comp,
+						 struct rvtrace_perf_auxbuf *buf);
 	int			(*start)(struct rvtrace_component *comp);
 	int			(*stop)(struct rvtrace_component *comp);
 	int			(*probe)(struct rvtrace_component *comp);
Signature · 1 line
2.43.0
author
#11 lore raw

PATCH v2 7/12 rvtrace: Add trace ramsink driver

In reply to Anup Patel

From: Mayuresh Chitale <[email protected]>

Add initial implementation of RISC-V trace ramsink driver. The ramsink is defined in the RISC-V Trace Control Interface specification.

---
 drivers/hwtracing/rvtrace/Kconfig           |   9 +
 drivers/hwtracing/rvtrace/Makefile          |   1 +
 drivers/hwtracing/rvtrace/rvtrace-ramsink.c | 262 ++++++++++++++++++++
 3 files changed, 272 insertions(+)
 create mode 100644 drivers/hwtracing/rvtrace/rvtrace-ramsink.c
drivers/hwtracing/rvtrace/Kconfig +9 -0
index ba35c05f3f54..0577f9acb858 100644
@@ -21,3 +21,12 @@ config RVTRACE_ENCODER
 	default y
 	help
 	  This driver provides support for RISC-V Trace Encoder component.
+
+config RVTRACE_RAMSINK
+	tristate "RISC-V Trace Ramsink driver"
+	depends on RVTRACE
+	select DMA_SHARED_BUFFER
+	default y
+	help
+	  This driver provides support for Risc-V E-Trace Ramsink
+	  component.
drivers/hwtracing/rvtrace/Makefile +1 -0
index f320693a1fc5..122e575da9fb 100644
@@ -3,3 +3,4 @@
 obj-$(CONFIG_RVTRACE) += rvtrace.o
 rvtrace-y := rvtrace-core.o rvtrace-platform.o
 obj-$(CONFIG_RVTRACE_ENCODER) += rvtrace-encoder.o
+obj-$(CONFIG_RVTRACE_RAMSINK) += rvtrace-ramsink.o
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +262 -0

262 lines not on this page · raw message

Signature · 1 line
2.43.0
#12 lore raw

Re: [PATCH v2 07/12] rvtrace: Add trace ramsink driver

Hi Anup,

My comments below:

Quoted 56 lines · Anup Patel

From: Mayuresh Chitale <[email protected]>

Add initial implementation of RISC-V trace ramsink driver. The ramsink is defined in the RISC-V Trace Control Interface specification.

---
  drivers/hwtracing/rvtrace/Kconfig           |   9 +
  drivers/hwtracing/rvtrace/Makefile          |   1 +
  drivers/hwtracing/rvtrace/rvtrace-ramsink.c | 262 ++++++++++++++++++++
  3 files changed, 272 insertions(+)

create mode 100644 drivers/hwtracing/rvtrace/rvtrace-ramsink.c

drivers/hwtracing/rvtrace/Kconfig +9 -0
index ba35c05f3f54..0577f9acb858 100644
@@ -21,3 +21,12 @@ config RVTRACE_ENCODER
  	default y
  	help
  	  This driver provides support for RISC-V Trace Encoder component.
+
+config RVTRACE_RAMSINK
+	tristate "RISC-V Trace Ramsink driver"
+	depends on RVTRACE
+	select DMA_SHARED_BUFFER
+	default y
+	help
+	  This driver provides support for Risc-V E-Trace Ramsink
+	  component.
drivers/hwtracing/rvtrace/Makefile +1 -0
index f320693a1fc5..122e575da9fb 100644
@@ -3,3 +3,4 @@
  obj-$(CONFIG_RVTRACE) += rvtrace.o
  rvtrace-y := rvtrace-core.o rvtrace-platform.o
  obj-$(CONFIG_RVTRACE_ENCODER) += rvtrace-encoder.o
+obj-$(CONFIG_RVTRACE_RAMSINK) += rvtrace-ramsink.o
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +9 -0
new file mode 100644
index 000000000000..676344c9387c
@@ -0,0 +1,262 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 Ventana Micro Systems Inc.
+ */
+
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/dma-mapping.h>

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 19

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+#include <linux/rvtrace.h>
+#include <linux/types.h>
+#include <linux/sizes.h>
+
+#define RVTRACE_RAMSINK_STARTLOW_OFF		0x010
+#define RVTRACE_RAMSINK_STARTHIGH_OFF		0x014
+#define RVTRACE_RAMSINK_LIMITLOW_OFF		0x018
+#define RVTRACE_RAMSINK_LIMITHIGH_OFF		0x01c
+#define RVTRACE_RAMSINK_WPLOW_OFF		0x020
+#define   RVTRACE_RAMSINK_WPLOW_WRAP		0x1

nit: wrong spacing

Quoted 15 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +15 -0
+#define RVTRACE_RAMSINK_WPHIGH_OFF		0x024
+#define RVTRACE_RAMSINK_RPLOW_OFF		0x028
+#define RVTRACE_RAMSINK_RPHIGH_OFF		0x02c
+
+struct rvtrace_ramsink_priv {
+	size_t size;
+	void *va;
+	dma_addr_t start;
+	dma_addr_t end;
+	/* WP from prev iteration */
+	dma_addr_t prev_head;
+};
+
+struct trace_buf {
+	void *base;

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 44

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+	size_t size;
+	long cur;
+	size_t len;
+};
+
+static int rvtrace_ramsink_stop(struct rvtrace_component *comp)
+{
+	return rvtrace_comp_is_empty(comp);
+}
+

We should first set trRamEnable to 0, poll it, then poll trRamEmpty. It's essentially the same as encoder, so perhaps move it to core.c. Here we're just checking whether it's empty, which is wrong. We're also doing similar things in encoder.c Should fix that as well.

Quoted 30 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +30 -0
+static void tbuf_to_pbuf_copy(struct trace_buf *src, struct trace_buf *dst)
+{
+	int bytes_dst, bytes_src, bytes;
+	void *dst_addr, *src_addr;
+
+	while (src->size) {
+		src_addr = src->base + src->cur;
+		dst_addr = dst->base + dst->cur;
+
+		if (dst->len - dst->cur < src->size)
+			bytes_dst = dst->len - dst->cur;
+		else
+			bytes_dst = src->size;
+		if (src->len - src->cur < src->size)
+			bytes_src = src->len - src->cur;
+		else
+			bytes_src = src->size;
+		bytes = bytes_dst < bytes_src ? bytes_dst : bytes_src;
+		memcpy(dst_addr, src_addr, bytes);
+		dst->cur = (dst->cur + bytes) % dst->len;
+		src->cur = (src->cur + bytes) % src->len;
+		src->size -= bytes;
+	}
+}
+
+static size_t rvtrace_ramsink_copyto_auxbuf(struct rvtrace_component *comp,
+					    struct rvtrace_perf_auxbuf *buf)
+{
+	struct rvtrace_ramsink_priv *priv = dev_get_drvdata(&comp->dev);
+	struct trace_buf src, dst;

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 84

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+	u32 wp_low, wp_high;
+	u64 buf_cur_head;
+	size_t size;
+
+	wp_low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_WPLOW_OFF);
+	wp_high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_WPHIGH_OFF);
+	buf_cur_head = (u64)(wp_high) << 32 | (wp_low & ~RVTRACE_RAMSINK_WPLOW_WRAP);
+
+	if (buf_cur_head == priv->prev_head)
+		return 0;

cur_head == prev_head could mean that we've wrapped around and stopped at the exact position. Thus, we need to check for the wrapped around bit. If set, then this case can be merged into buf_cur_head <= priv->prev_head below.

For cur_head > prev_head case, we also need to check for wrap-around. If wrapped-around, we actually filled the entire buffer, not just cur - prev We also should consider the possibility that the buffer has been wrapped around multiple times. In all cases, maintaining a prev_head seems to complicate things, so why don't we just reset the WP to priv->start each time we start the ramsink component and get rid of prev_head? This way, we'll always have cur >= start, and depending on wrapped bit, we either get the whole buffer, or cur - start.

Quoted 33 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +33 -0
+
+	dst.base = buf->base;
+	dst.len = buf->length;
+	dst.cur = buf->pos;
+
+	src.base = priv->va;
+	src.len = priv->end - priv->start;
+	if (buf_cur_head > priv->prev_head) {
+		src.size = buf_cur_head - priv->prev_head;
+	} else {
+		src.size = priv->end - priv->prev_head;
+		src.size += buf_cur_head - priv->start;
+	}
+
+	src.cur = buf_cur_head - priv->start;
+	size = src.size;
+	tbuf_to_pbuf_copy(&src, &dst);
+	buf->pos = dst.cur;
+	priv->prev_head = buf_cur_head;
+
+	return size;
+}
+
+static int rvtrace_ramsink_setup_buf(struct rvtrace_component *comp)
+{
+	struct rvtrace_ramsink_priv *priv = dev_get_drvdata(&comp->dev);
+	u64 start_min, limit_max, end;
+	u32 low, high;
+
+	/* Probe min and max values for start and limit registers */
+	rvtrace_write32(comp->pdata, 0, RVTRACE_RAMSINK_STARTLOW_OFF);
+	rvtrace_write32(comp->pdata, 0, RVTRACE_RAMSINK_STARTHIGH_OFF);
+	low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_STARTLOW_OFF);

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 127

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+	high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_STARTHIGH_OFF);
+	start_min = (u64)(high) << 32 | low;
+
+	rvtrace_write32(comp->pdata, 0xffffffff, RVTRACE_RAMSINK_LIMITLOW_OFF);
+	rvtrace_write32(comp->pdata, 0xffffffff, RVTRACE_RAMSINK_LIMITHIGH_OFF);
+	low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITLOW_OFF);
+	high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITHIGH_OFF);
+	limit_max = (u64)(high) << 32 | low;
+
+	if (priv->end < start_min) {

Where was priv->end initialized before this check?

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 133

Quoted 6 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +6 -0
+		dev_err(&comp->dev, "DMA memory not addressable by device\n");
+		return -EINVAL;
+	}
+
+	/* Setup ram sink start addresses */
+	if (priv->start < start_min) {

No checks for start_min >= priv->end?

Quoted 12 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +12 -0
+		dev_warn(&comp->dev, "Ramsink start address updated from %llx to %llx\n",
+			 priv->start, start_min);
+		priv->va += start_min - priv->start;
+		priv->start = start_min;
+	}
+
+	priv->prev_head = priv->start;
+	priv->end = priv->start + priv->size;
+	rvtrace_write32(comp->pdata, lower_32_bits(priv->start), RVTRACE_RAMSINK_STARTLOW_OFF);
+	rvtrace_write32(comp->pdata, upper_32_bits(priv->start), RVTRACE_RAMSINK_STARTHIGH_OFF);
+	rvtrace_write32(comp->pdata, lower_32_bits(priv->start), RVTRACE_RAMSINK_WPLOW_OFF);
+	rvtrace_write32(comp->pdata, upper_32_bits(priv->start), RVTRACE_RAMSINK_WPHIGH_OFF);

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 155

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+	/* Setup ram sink limit addresses */
+	if (priv->end > limit_max) {
+		dev_warn(&comp->dev, "Ramsink limit address updated from %llx to %llx\n", priv->end,
+			 limit_max);
+		priv->end = limit_max;
+		priv->size = priv->end - priv->start;
+	}
+
+	/* Limit address needs to be set to end - 4 to avoid overflow */
+	end = priv->end - 4;

Should not hard-code 4, instead, do write32(0xfffffff, RamLimit), then ffs(RamLimit) to probe for the the write width, aka, "A" value in Table
47. Typical Trace RAM Sink Configuration in the Spec.

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 165

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+	rvtrace_write32(comp->pdata, lower_32_bits(end), RVTRACE_RAMSINK_LIMITLOW_OFF);
+	rvtrace_write32(comp->pdata, upper_32_bits(end), RVTRACE_RAMSINK_LIMITHIGH_OFF);
+	low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITLOW_OFF);
+	high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITHIGH_OFF);
+	end = (u64)(high) << 32 | low;
+	if (end != (priv->end - 4)) {
+		dev_warn(&comp->dev, "Ramsink limit address updated from %llx to %llx\n", priv->end,
+			 end);
+		priv->end = end;
+		priv->size = priv->end - priv->start;

Actually the RamLimit read is not the real limit. It's Limit - "A" (the write width).

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 97

Quoted 4 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +4 -0
+	}
+
+	return 0;
+}

I think overall we should simplify this setup_buf function. For RamStart alone, there're alignment + min_addr requirement, which is very unlikely that the lowest valid address happens to be within the range that dma- allocated in the caller (when you need to adjust priv->start). Thus, to do it properly, we first need to detect the min_addr + alignment *before* dma-alloc, and pass such requirements into dma-alloc, then depending on what dma-alloc returns, detect the "M" value by interrogating RamLimit. I'm not sure if dma-alloc can be passed a hint address that whatever returned must be higher, and whether it can enforce random alignment, so may be we can just program RamStart/Limit based on what dma-alloc gives and error out if value is not retained. We do however need to probe for write width ("A" value). It's usually 4 or 8, but it does vary per-impl, and remember, the total buffer size is still 1MB, not 1MB - 4 or -8.

Quoted 37 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +37 -0
+
+static int rvtrace_ramsink_setup(struct rvtrace_component *comp)
+{
+	struct device *pdev = comp->pdata->dev;
+	struct rvtrace_ramsink_priv *priv;
+
+	priv = devm_kzalloc(&comp->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	dev_set_drvdata(&comp->dev, priv);
+
+	/* Derive RAM sink memory size based on component implementation ID */
+	switch (comp->pdata->impid) {
+	default:
+		priv->size = SZ_1M;
+		break;
+	}
+
+	priv->va = dma_alloc_coherent(pdev, priv->size, &priv->start, GFP_KERNEL);
+	if (!priv->va)
+		return -ENOMEM;
+
+	return rvtrace_ramsink_setup_buf(comp);
+}
+
+static void rvtrace_ramsink_cleanup(struct rvtrace_component *comp)
+{
+	struct rvtrace_ramsink_priv *priv = dev_get_drvdata(&comp->dev);
+
+	dma_free_coherent(&comp->dev, priv->size, priv->va, priv->start);
+}
+
+static int rvtrace_ramsink_probe(struct rvtrace_component *comp)
+{
+	int ret;
+

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 216

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+	ret = rvtrace_ramsink_setup(comp);
+	if (ret)
+		return dev_err_probe(&comp->dev, ret, "failed to setup ramsink.\n");
+
+	ret = rvtrace_enable_component(comp);
+	if (ret)
+		return dev_err_probe(&comp->dev, ret, "failed to enable ramsink.\n");
+
+	return ret;
+}

In probe, we should detect and set it to SMEM mode. The default is SRAM mode.

Quoted 11 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +11 -0
+
+static void rvtrace_ramsink_remove(struct rvtrace_component *comp)
+{
+	int ret;
+
+	ret = rvtrace_disable_component(comp);
+	if (ret)
+		dev_err(&comp->dev, "failed to disable ramsink.\n");
+
+	rvtrace_ramsink_cleanup(comp);
+}

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 237

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+
+static struct rvtrace_component_id rvtrace_ramsink_ids[] = {
+	{ .type = RVTRACE_COMPONENT_TYPE_RAMSINK,
+	  .version = rvtrace_component_mkversion(1, 0), },
+	{},
+};
+
+static struct rvtrace_driver rvtrace_ramsink_driver = {
+	.id_table = rvtrace_ramsink_ids,
+	.copyto_auxbuf = rvtrace_ramsink_copyto_auxbuf,

Should have a .start function like the encoder (setting trRamEnable), and reset the WP to priv->start, as suggested above.

Quoted 15 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +15 -0
+	.stop = rvtrace_ramsink_stop,
+	.probe = rvtrace_ramsink_probe,
+	.remove = rvtrace_ramsink_remove,
+	.driver = {
+		.name = "rvtrace-ramsink",
+	},
+};
+
+static int __init rvtrace_ramsink_init(void)
+{
+	return rvtrace_register_driver(&rvtrace_ramsink_driver);
+}
+
+static void __exit rvtrace_ramsink_exit(void)
+{

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 262

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+	rvtrace_unregister_driver(&rvtrace_ramsink_driver);
+}
+
+module_init(rvtrace_ramsink_init);
+module_exit(rvtrace_ramsink_exit);
+
+/* Module information */
+MODULE_AUTHOR("Mayuresh Chitale <[email protected]>");
+MODULE_DESCRIPTION("RISC-V Trace Ramsink Driver");
+MODULE_LICENSE("GPL");

Bo

#13 lore raw

Re: [PATCH v2 07/12] rvtrace: Add trace ramsink driver

Hi Bo,

Quoted 97 lines · Bo Gan

Hi Anup,

My comments below:

Quoted 56 lines · Anup Patel

From: Mayuresh Chitale <[email protected]>

Add initial implementation of RISC-V trace ramsink driver. The ramsink is defined in the RISC-V Trace Control Interface specification.

---
  drivers/hwtracing/rvtrace/Kconfig           |   9 +
  drivers/hwtracing/rvtrace/Makefile          |   1 +
  drivers/hwtracing/rvtrace/rvtrace-ramsink.c | 262 ++++++++++++++++++++
  3 files changed, 272 insertions(+)

create mode 100644 drivers/hwtracing/rvtrace/rvtrace-ramsink.c

drivers/hwtracing/rvtrace/Kconfig +9 -0
index ba35c05f3f54..0577f9acb858 100644
@@ -21,3 +21,12 @@ config RVTRACE_ENCODER
      default y
      help
        This driver provides support for RISC-V Trace Encoder component.
+
+config RVTRACE_RAMSINK
+     tristate "RISC-V Trace Ramsink driver"
+     depends on RVTRACE
+     select DMA_SHARED_BUFFER
+     default y
+     help
+       This driver provides support for Risc-V E-Trace Ramsink
+       component.
drivers/hwtracing/rvtrace/Makefile +1 -0
index f320693a1fc5..122e575da9fb 100644
@@ -3,3 +3,4 @@
  obj-$(CONFIG_RVTRACE) += rvtrace.o
  rvtrace-y := rvtrace-core.o rvtrace-platform.o
  obj-$(CONFIG_RVTRACE_ENCODER) += rvtrace-encoder.o
+obj-$(CONFIG_RVTRACE_RAMSINK) += rvtrace-ramsink.o
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +9 -0
new file mode 100644
index 000000000000..676344c9387c
@@ -0,0 +1,262 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 Ventana Micro Systems Inc.
+ */
+
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/dma-mapping.h>

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 19

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+#include <linux/rvtrace.h>
+#include <linux/types.h>
+#include <linux/sizes.h>
+
+#define RVTRACE_RAMSINK_STARTLOW_OFF         0x010
+#define RVTRACE_RAMSINK_STARTHIGH_OFF                0x014
+#define RVTRACE_RAMSINK_LIMITLOW_OFF         0x018
+#define RVTRACE_RAMSINK_LIMITHIGH_OFF                0x01c
+#define RVTRACE_RAMSINK_WPLOW_OFF            0x020
+#define   RVTRACE_RAMSINK_WPLOW_WRAP         0x1

nit: wrong spacing

Quoted 22 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +22 -0
+#define RVTRACE_RAMSINK_WPHIGH_OFF           0x024
+#define RVTRACE_RAMSINK_RPLOW_OFF            0x028
+#define RVTRACE_RAMSINK_RPHIGH_OFF           0x02c
+
+struct rvtrace_ramsink_priv {
+     size_t size;
+     void *va;
+     dma_addr_t start;
+     dma_addr_t end;
+     /* WP from prev iteration */
+     dma_addr_t prev_head;
+};
+
+struct trace_buf {
+     void *base;
+     size_t size;
+     long cur;
+     size_t len;
+};
+
+static int rvtrace_ramsink_stop(struct rvtrace_component *comp)
+{
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 44

Quoted 3 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +3 -0
+     return rvtrace_comp_is_empty(comp);
+}
+

We should first set trRamEnable to 0, poll it, then poll trRamEmpty. It's essentially the same as encoder, so perhaps move it to core.c. Here we're just checking whether it's empty, which is wrong. We're also doing similar things in encoder.c Should fix that as well.

This code is called to just stop all components in a path which means:
- for encoders the driver clears the trTeInstTracing bit and then polls for empty bit to be cleared
- for ramsinks the driver just polls for the empty bit to be cleared More on this below.

Quoted 38 lines
Quoted 38 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +37 -0
+static void tbuf_to_pbuf_copy(struct trace_buf *src, struct trace_buf *dst)
+{
+     int bytes_dst, bytes_src, bytes;
+     void *dst_addr, *src_addr;
+
+     while (src->size) {
+             src_addr = src->base + src->cur;
+             dst_addr = dst->base + dst->cur;
+
+             if (dst->len - dst->cur < src->size)
+                     bytes_dst = dst->len - dst->cur;
+             else
+                     bytes_dst = src->size;
+             if (src->len - src->cur < src->size)
+                     bytes_src = src->len - src->cur;
+             else
+                     bytes_src = src->size;
+             bytes = bytes_dst < bytes_src ? bytes_dst : bytes_src;
+             memcpy(dst_addr, src_addr, bytes);
+             dst->cur = (dst->cur + bytes) % dst->len;
+             src->cur = (src->cur + bytes) % src->len;
+             src->size -= bytes;
+     }
+}
+
+static size_t rvtrace_ramsink_copyto_auxbuf(struct rvtrace_component *comp,
+                                         struct rvtrace_perf_auxbuf *buf)
+{
+     struct rvtrace_ramsink_priv *priv = dev_get_drvdata(&comp->dev);
+     struct trace_buf src, dst;
+     u32 wp_low, wp_high;
+     u64 buf_cur_head;
+     size_t size;
+
+     wp_low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_WPLOW_OFF);
+     wp_high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_WPHIGH_OFF);
+     buf_cur_head = (u64)(wp_high) << 32 | (wp_low & ~RVTRACE_RAMSINK_WPLOW_WRAP);
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 84

Quoted 3 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +3 -0
+
+     if (buf_cur_head == priv->prev_head)
+             return 0;

cur_head == prev_head could mean that we've wrapped around and stopped at the exact position. Thus, we need to check for the wrapped around bit. If set, then this case can be merged into buf_cur_head <= priv->prev_head below.

cur_head is the current value in the wp registers whereas prev_head is the value in wp registers from the previous iteration which is saved explicitly by software. So prev_head can't change dynamically.

Quoted 9 lines

For cur_head > prev_head case, we also need to check for wrap-around. If wrapped-around, we actually filled the entire buffer, not just cur - prev We also should consider the possibility that the buffer has been wrapped around multiple times. In all cases, maintaining a prev_head seems to complicate things, so why don't we just reset the WP to priv->start each time we start the ramsink component and get rid of prev_head? This way, we'll always have cur >= start, and depending on wrapped bit, we either get the whole buffer, or cur - start.

Currently the software never clears trRamWrap bit because as per the spec:
- It is only set to 0 if trRamWPLow is written
- After trace is enabled and active , the trace tool should NOT write any of trRamStart/Limit/WP?? registers.
Let me test this change to disable all the components (instead of just stopping trace) every time we want to copy data to perf buffer. This should let us use the trRamWrap bit as expected. Multiple wraparounds can't be avoided unless StopOnWrap was set. I will update the ramsink driver to set StopOnWrap based on the implementation id.

Quoted 39 lines
Quoted 39 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +37 -0
+
+     dst.base = buf->base;
+     dst.len = buf->length;
+     dst.cur = buf->pos;
+
+     src.base = priv->va;
+     src.len = priv->end - priv->start;
+     if (buf_cur_head > priv->prev_head) {
+             src.size = buf_cur_head - priv->prev_head;
+     } else {
+             src.size = priv->end - priv->prev_head;
+             src.size += buf_cur_head - priv->start;
+     }
+
+     src.cur = buf_cur_head - priv->start;
+     size = src.size;
+     tbuf_to_pbuf_copy(&src, &dst);
+     buf->pos = dst.cur;
+     priv->prev_head = buf_cur_head;
+
+     return size;
+}
+
+static int rvtrace_ramsink_setup_buf(struct rvtrace_component *comp)
+{
+     struct rvtrace_ramsink_priv *priv = dev_get_drvdata(&comp->dev);
+     u64 start_min, limit_max, end;
+     u32 low, high;
+
+     /* Probe min and max values for start and limit registers */
+     rvtrace_write32(comp->pdata, 0, RVTRACE_RAMSINK_STARTLOW_OFF);
+     rvtrace_write32(comp->pdata, 0, RVTRACE_RAMSINK_STARTHIGH_OFF);
+     low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_STARTLOW_OFF);
+     high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_STARTHIGH_OFF);
+     start_min = (u64)(high) << 32 | low;
+
+     rvtrace_write32(comp->pdata, 0xffffffff, RVTRACE_RAMSINK_LIMITLOW_OFF);
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 127

Quoted 6 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +6 -0
+     rvtrace_write32(comp->pdata, 0xffffffff, RVTRACE_RAMSINK_LIMITHIGH_OFF);
+     low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITLOW_OFF);
+     high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITHIGH_OFF);
+     limit_max = (u64)(high) << 32 | low;
+
+     if (priv->end < start_min) {

Where was priv->end initialized before this check?

I will move it before the check.

Quoted 28 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 133

Quoted 7 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +6 -0
+             dev_err(&comp->dev, "DMA memory not addressable by device\n");
+             return -EINVAL;
+     }
+
+     /* Setup ram sink start addresses */
+     if (priv->start < start_min) {

No checks for start_min >= priv->end?

Quoted 18 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +18 -0
+             dev_warn(&comp->dev, "Ramsink start address updated from %llx to %llx\n",
+                      priv->start, start_min);
+             priv->va += start_min - priv->start;
+             priv->start = start_min;
+     }
+
+     priv->prev_head = priv->start;
+     priv->end = priv->start + priv->size;
+     rvtrace_write32(comp->pdata, lower_32_bits(priv->start), RVTRACE_RAMSINK_STARTLOW_OFF);
+     rvtrace_write32(comp->pdata, upper_32_bits(priv->start), RVTRACE_RAMSINK_STARTHIGH_OFF);
+     rvtrace_write32(comp->pdata, lower_32_bits(priv->start), RVTRACE_RAMSINK_WPLOW_OFF);
+     rvtrace_write32(comp->pdata, upper_32_bits(priv->start), RVTRACE_RAMSINK_WPHIGH_OFF);
+     /* Setup ram sink limit addresses */
+     if (priv->end > limit_max) {
+             dev_warn(&comp->dev, "Ramsink limit address updated from %llx to %llx\n", priv->end,
+                      limit_max);
+             priv->end = limit_max;
+             priv->size = priv->end - priv->start;
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 155

Quoted 4 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +4 -0
+     }
+
+     /* Limit address needs to be set to end - 4 to avoid overflow */
+     end = priv->end - 4;

Should not hard-code 4, instead, do write32(0xfffffff, RamLimit), then ffs(RamLimit) to probe for the the write width, aka, "A" value in Table
47. Typical Trace RAM Sink Configuration in the Spec.

I dont think 'A' can be probed that way because as per section 7.1:
"Not every value may be settable in trRamStart/Limit registers. Value written may be trimmed (for example aligned on a particular 2^N boundary)". So writing 0xffffffff to limit register will reveal the boundary alignment but not 'A'. I think 'A' needs to be specified by an implementation with default as 4. I will change it accordingly.

Quoted 25 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 165

Quoted 11 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+     rvtrace_write32(comp->pdata, lower_32_bits(end), RVTRACE_RAMSINK_LIMITLOW_OFF);
+     rvtrace_write32(comp->pdata, upper_32_bits(end), RVTRACE_RAMSINK_LIMITHIGH_OFF);
+     low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITLOW_OFF);
+     high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITHIGH_OFF);
+     end = (u64)(high) << 32 | low;
+     if (end != (priv->end - 4)) {
+             dev_warn(&comp->dev, "Ramsink limit address updated from %llx to %llx\n", priv->end,
+                      end);
+             priv->end = end;
+             priv->size = priv->end - priv->start;

Actually the RamLimit read is not the real limit. It's Limit - "A" (the write width).

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 97

Quoted 4 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +4 -0
+     }
+
+     return 0;
+}

I think overall we should simplify this setup_buf function. For RamStart alone, there're alignment + min_addr requirement, which is very unlikely that the lowest valid address happens to be within the range that dma- allocated in the caller (when you need to adjust priv->start). Thus, to do it properly, we first need to detect the min_addr + alignment *before*

Quoted 8 lines

dma-alloc, and pass such requirements into dma-alloc, then depending on what dma-alloc returns, detect the "M" value by interrogating RamLimit. I'm not sure if dma-alloc can be passed a hint address that whatever returned must be higher, and whether it can enforce random alignment, so may be we can just program RamStart/Limit based on what dma-alloc gives and error out if value is not retained. We do however need to probe for write width ("A" value). It's usually 4 or 8, but it does vary per-impl, and remember, the total buffer size is still 1MB, not 1MB - 4 or -8.

I think the issue is that the physical memory region returned by dma_alloc* might not be addressable by the hw and so the start / end addresses are already trimmed in the code accordingly but I agree this can be handled differently. For e.g
- probe max RamLimit value .
- set the dma_mask to the probed value
- dma_alloc*
- further address checks

Quoted 43 lines
Quoted 43 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +42 -0
+
+static int rvtrace_ramsink_setup(struct rvtrace_component *comp)
+{
+     struct device *pdev = comp->pdata->dev;
+     struct rvtrace_ramsink_priv *priv;
+
+     priv = devm_kzalloc(&comp->dev, sizeof(*priv), GFP_KERNEL);
+     if (!priv)
+             return -ENOMEM;
+
+     dev_set_drvdata(&comp->dev, priv);
+
+     /* Derive RAM sink memory size based on component implementation ID */
+     switch (comp->pdata->impid) {
+     default:
+             priv->size = SZ_1M;
+             break;
+     }
+
+     priv->va = dma_alloc_coherent(pdev, priv->size, &priv->start, GFP_KERNEL);
+     if (!priv->va)
+             return -ENOMEM;
+
+     return rvtrace_ramsink_setup_buf(comp);
+}
+
+static void rvtrace_ramsink_cleanup(struct rvtrace_component *comp)
+{
+     struct rvtrace_ramsink_priv *priv = dev_get_drvdata(&comp->dev);
+
+     dma_free_coherent(&comp->dev, priv->size, priv->va, priv->start);
+}
+
+static int rvtrace_ramsink_probe(struct rvtrace_component *comp)
+{
+     int ret;
+
+     ret = rvtrace_ramsink_setup(comp);
+     if (ret)
+             return dev_err_probe(&comp->dev, ret, "failed to setup ramsink.\n");
+
+     ret = rvtrace_enable_component(comp);
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 216

Quoted 5 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +5 -0
+     if (ret)
+             return dev_err_probe(&comp->dev, ret, "failed to enable ramsink.\n");
+
+     return ret;
+}

In probe, we should detect and set it to SMEM mode. The default is SRAM mode.

Actually the default is not set but I agree this needs to be handled differently. We can make this an implementation defined parameter and then cross check with trRamImpl register during probe.

Quoted 17 lines
Quoted 17 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +16 -0
+
+static void rvtrace_ramsink_remove(struct rvtrace_component *comp)
+{
+     int ret;
+
+     ret = rvtrace_disable_component(comp);
+     if (ret)
+             dev_err(&comp->dev, "failed to disable ramsink.\n");
+
+     rvtrace_ramsink_cleanup(comp);
+}
+
+static struct rvtrace_component_id rvtrace_ramsink_ids[] = {
+     { .type = RVTRACE_COMPONENT_TYPE_RAMSINK,
+       .version = rvtrace_component_mkversion(1, 0), },
+     {},
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 237

Quoted 5 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +5 -0
+};
+
+static struct rvtrace_driver rvtrace_ramsink_driver = {
+     .id_table = rvtrace_ramsink_ids,
+     .copyto_auxbuf = rvtrace_ramsink_copyto_auxbuf,

Should have a .start function like the encoder (setting trRamEnable), and reset the WP to priv->start, as suggested above.

The active and enable bits for all components are handled via generic rvtrace_enable_component/rvtrace_disable_component functions. When all components are disabled before copying data to perf then WP would be reset to priv->start as a part of copy_to_auxbuf callback.

Quoted 29 lines
Quoted 16 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +15 -0
+     .stop = rvtrace_ramsink_stop,
+     .probe = rvtrace_ramsink_probe,
+     .remove = rvtrace_ramsink_remove,
+     .driver = {
+             .name = "rvtrace-ramsink",
+     },
+};
+
+static int __init rvtrace_ramsink_init(void)
+{
+     return rvtrace_register_driver(&rvtrace_ramsink_driver);
+}
+
+static void __exit rvtrace_ramsink_exit(void)
+{

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 262

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+     rvtrace_unregister_driver(&rvtrace_ramsink_driver);
+}
+
+module_init(rvtrace_ramsink_init);
+module_exit(rvtrace_ramsink_exit);
+
+/* Module information */
+MODULE_AUTHOR("Mayuresh Chitale <[email protected]>");
+MODULE_DESCRIPTION("RISC-V Trace Ramsink Driver");
+MODULE_LICENSE("GPL");

Bo

#14 lore raw

Re: [PATCH v2 07/12] rvtrace: Add trace ramsink driver

Hi Mayuresh,

Thanks for your reply. My comments below.

Quoted 108 lines · Mayuresh Chitale

Hi Bo,

Quoted 97 lines · Bo Gan

Hi Anup,

My comments below:

Quoted 56 lines · Anup Patel

From: Mayuresh Chitale <[email protected]>

Add initial implementation of RISC-V trace ramsink driver. The ramsink is defined in the RISC-V Trace Control Interface specification.

---
   drivers/hwtracing/rvtrace/Kconfig           |   9 +
   drivers/hwtracing/rvtrace/Makefile          |   1 +
   drivers/hwtracing/rvtrace/rvtrace-ramsink.c | 262 ++++++++++++++++++++
   3 files changed, 272 insertions(+)

create mode 100644 drivers/hwtracing/rvtrace/rvtrace-ramsink.c

drivers/hwtracing/rvtrace/Kconfig +9 -0
index ba35c05f3f54..0577f9acb858 100644
@@ -21,3 +21,12 @@ config RVTRACE_ENCODER
       default y
       help
         This driver provides support for RISC-V Trace Encoder component.
+
+config RVTRACE_RAMSINK
+     tristate "RISC-V Trace Ramsink driver"
+     depends on RVTRACE
+     select DMA_SHARED_BUFFER
+     default y
+     help
+       This driver provides support for Risc-V E-Trace Ramsink
+       component.
drivers/hwtracing/rvtrace/Makefile +1 -0
index f320693a1fc5..122e575da9fb 100644
@@ -3,3 +3,4 @@
   obj-$(CONFIG_RVTRACE) += rvtrace.o
   rvtrace-y := rvtrace-core.o rvtrace-platform.o
   obj-$(CONFIG_RVTRACE_ENCODER) += rvtrace-encoder.o
+obj-$(CONFIG_RVTRACE_RAMSINK) += rvtrace-ramsink.o
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +9 -0
new file mode 100644
index 000000000000..676344c9387c
@@ -0,0 +1,262 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 Ventana Micro Systems Inc.
+ */
+
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/dma-mapping.h>

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 19

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+#include <linux/rvtrace.h>
+#include <linux/types.h>
+#include <linux/sizes.h>
+
+#define RVTRACE_RAMSINK_STARTLOW_OFF         0x010
+#define RVTRACE_RAMSINK_STARTHIGH_OFF                0x014
+#define RVTRACE_RAMSINK_LIMITLOW_OFF         0x018
+#define RVTRACE_RAMSINK_LIMITHIGH_OFF                0x01c
+#define RVTRACE_RAMSINK_WPLOW_OFF            0x020
+#define   RVTRACE_RAMSINK_WPLOW_WRAP         0x1

nit: wrong spacing

Quoted 22 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +22 -0
+#define RVTRACE_RAMSINK_WPHIGH_OFF           0x024
+#define RVTRACE_RAMSINK_RPLOW_OFF            0x028
+#define RVTRACE_RAMSINK_RPHIGH_OFF           0x02c
+
+struct rvtrace_ramsink_priv {
+     size_t size;
+     void *va;
+     dma_addr_t start;
+     dma_addr_t end;
+     /* WP from prev iteration */
+     dma_addr_t prev_head;
+};
+
+struct trace_buf {
+     void *base;
+     size_t size;
+     long cur;
+     size_t len;
+};
+
+static int rvtrace_ramsink_stop(struct rvtrace_component *comp)
+{
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 44

Quoted 3 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +3 -0
+     return rvtrace_comp_is_empty(comp);
+}
+

We should first set trRamEnable to 0, poll it, then poll trRamEmpty. It's essentially the same as encoder, so perhaps move it to core.c. Here we're just checking whether it's empty, which is wrong. We're also doing similar things in encoder.c Should fix that as well.

Quoted 5 lines

This code is called to just stop all components in a path which means:
- for encoders the driver clears the trTeInstTracing bit and then polls for empty bit to be cleared
- for ramsinks the driver just polls for the empty bit to be cleared More on this below.

Can we rename it to something like rvtrace_comp_poll_empty(...) for clarity? This name kind of confused me into thinking it's just a check.

Quoted 59 lines
Quoted 38 lines
Quoted 38 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +37 -0
+static void tbuf_to_pbuf_copy(struct trace_buf *src, struct trace_buf *dst)
+{
+     int bytes_dst, bytes_src, bytes;
+     void *dst_addr, *src_addr;
+
+     while (src->size) {
+             src_addr = src->base + src->cur;
+             dst_addr = dst->base + dst->cur;
+
+             if (dst->len - dst->cur < src->size)
+                     bytes_dst = dst->len - dst->cur;
+             else
+                     bytes_dst = src->size;
+             if (src->len - src->cur < src->size)
+                     bytes_src = src->len - src->cur;
+             else
+                     bytes_src = src->size;
+             bytes = bytes_dst < bytes_src ? bytes_dst : bytes_src;
+             memcpy(dst_addr, src_addr, bytes);
+             dst->cur = (dst->cur + bytes) % dst->len;
+             src->cur = (src->cur + bytes) % src->len;
+             src->size -= bytes;
+     }
+}
+
+static size_t rvtrace_ramsink_copyto_auxbuf(struct rvtrace_component *comp,
+                                         struct rvtrace_perf_auxbuf *buf)
+{
+     struct rvtrace_ramsink_priv *priv = dev_get_drvdata(&comp->dev);
+     struct trace_buf src, dst;
+     u32 wp_low, wp_high;
+     u64 buf_cur_head;
+     size_t size;
+
+     wp_low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_WPLOW_OFF);
+     wp_high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_WPHIGH_OFF);
+     buf_cur_head = (u64)(wp_high) << 32 | (wp_low & ~RVTRACE_RAMSINK_WPLOW_WRAP);
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 84

Quoted 3 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +3 -0
+
+     if (buf_cur_head == priv->prev_head)
+             return 0;

cur_head == prev_head could mean that we've wrapped around and stopped at the exact position. Thus, we need to check for the wrapped around bit. If set, then this case can be merged into buf_cur_head <= priv->prev_head below.

cur_head is the current value in the wp registers whereas prev_head is the value in wp registers from the previous iteration which is saved explicitly by software. So prev_head can't change dynamically.

Quoted 9 lines

For cur_head > prev_head case, we also need to check for wrap-around. If wrapped-around, we actually filled the entire buffer, not just cur - prev We also should consider the possibility that the buffer has been wrapped around multiple times. In all cases, maintaining a prev_head seems to complicate things, so why don't we just reset the WP to priv->start each time we start the ramsink component and get rid of prev_head? This way, we'll always have cur >= start, and depending on wrapped bit, we either get the whole buffer, or cur - start.

Quoted 4 lines

Currently the software never clears trRamWrap bit because as per the spec:
- It is only set to 0 if trRamWPLow is written
- After trace is enabled and active , the trace tool should NOT write any of trRamStart/Limit/WP?? registers.

I think you missed my point. I'm not suggesting that we should change the register values when trace is active. I'm saying that there's no need to maintain a prev_head. For every trace session, we can just start fresh, and reset WP to Start before trace starts. This way we don't need to deal with the complex conditions of cur_head > prev_head, because you'll also need to deal with wrap around, and the if condition will be complex. The trace can wrap around and over-write old traces anyway, so I don't see a value of keeping track of prev_head.

Quoted 89 lines

Let me test this change to disable all the components (instead of just stopping trace) every time we want to copy data to perf buffer. This should let us use the trRamWrap bit as expected. Multiple wraparounds can't be avoided unless StopOnWrap was set. I will update the ramsink driver to set StopOnWrap based on the implementation id.

Quoted 39 lines
Quoted 39 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +37 -0
+
+     dst.base = buf->base;
+     dst.len = buf->length;
+     dst.cur = buf->pos;
+
+     src.base = priv->va;
+     src.len = priv->end - priv->start;
+     if (buf_cur_head > priv->prev_head) {
+             src.size = buf_cur_head - priv->prev_head;
+     } else {
+             src.size = priv->end - priv->prev_head;
+             src.size += buf_cur_head - priv->start;
+     }
+
+     src.cur = buf_cur_head - priv->start;
+     size = src.size;
+     tbuf_to_pbuf_copy(&src, &dst);
+     buf->pos = dst.cur;
+     priv->prev_head = buf_cur_head;
+
+     return size;
+}
+
+static int rvtrace_ramsink_setup_buf(struct rvtrace_component *comp)
+{
+     struct rvtrace_ramsink_priv *priv = dev_get_drvdata(&comp->dev);
+     u64 start_min, limit_max, end;
+     u32 low, high;
+
+     /* Probe min and max values for start and limit registers */
+     rvtrace_write32(comp->pdata, 0, RVTRACE_RAMSINK_STARTLOW_OFF);
+     rvtrace_write32(comp->pdata, 0, RVTRACE_RAMSINK_STARTHIGH_OFF);
+     low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_STARTLOW_OFF);
+     high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_STARTHIGH_OFF);
+     start_min = (u64)(high) << 32 | low;
+
+     rvtrace_write32(comp->pdata, 0xffffffff, RVTRACE_RAMSINK_LIMITLOW_OFF);
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 127

Quoted 6 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +6 -0
+     rvtrace_write32(comp->pdata, 0xffffffff, RVTRACE_RAMSINK_LIMITHIGH_OFF);
+     low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITLOW_OFF);
+     high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITHIGH_OFF);
+     limit_max = (u64)(high) << 32 | low;
+
+     if (priv->end < start_min) {

Where was priv->end initialized before this check?

I will move it before the check.

Quoted 28 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 133

Quoted 7 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +6 -0
+             dev_err(&comp->dev, "DMA memory not addressable by device\n");
+             return -EINVAL;
+     }
+
+     /* Setup ram sink start addresses */
+     if (priv->start < start_min) {

No checks for start_min >= priv->end?

Quoted 18 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +18 -0
+             dev_warn(&comp->dev, "Ramsink start address updated from %llx to %llx\n",
+                      priv->start, start_min);
+             priv->va += start_min - priv->start;
+             priv->start = start_min;
+     }
+
+     priv->prev_head = priv->start;
+     priv->end = priv->start + priv->size;
+     rvtrace_write32(comp->pdata, lower_32_bits(priv->start), RVTRACE_RAMSINK_STARTLOW_OFF);
+     rvtrace_write32(comp->pdata, upper_32_bits(priv->start), RVTRACE_RAMSINK_STARTHIGH_OFF);
+     rvtrace_write32(comp->pdata, lower_32_bits(priv->start), RVTRACE_RAMSINK_WPLOW_OFF);
+     rvtrace_write32(comp->pdata, upper_32_bits(priv->start), RVTRACE_RAMSINK_WPHIGH_OFF);
+     /* Setup ram sink limit addresses */
+     if (priv->end > limit_max) {
+             dev_warn(&comp->dev, "Ramsink limit address updated from %llx to %llx\n", priv->end,
+                      limit_max);
+             priv->end = limit_max;
+             priv->size = priv->end - priv->start;
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 155

Quoted 4 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +4 -0
+     }
+
+     /* Limit address needs to be set to end - 4 to avoid overflow */
+     end = priv->end - 4;

Should not hard-code 4, instead, do write32(0xfffffff, RamLimit), then ffs(RamLimit) to probe for the the write width, aka, "A" value in Table
47. Typical Trace RAM Sink Configuration in the Spec.

Quoted 7 lines

I dont think 'A' can be probed that way because as per section 7.1:
"Not every value may be settable in trRamStart/Limit registers. Value written may be trimmed (for example aligned on a particular 2^N boundary)". So writing 0xffffffff to limit register will reveal the boundary alignment but not 'A'. I think 'A' needs to be specified by an implementation with default as 4. I will change it accordingly.

Quoting from spec:

"Value A means alignment which depends on memory access width. If we
have memory access width of 32-bits, A=4 and value of trRamLimit register should be 0x…FC. Some implementations may impose bigger alignment of trace data (to allow more efficient transfer rates) for SMEM mode. For SRAM mode A must be 4 as access to trace via trRamData is always 32-bits wide "

the 4 byte of A value is only for SRAM mode, not SMEM mode. My inter- pretation of the spec is that this can be detected by writing -1 to Limit, and detect M, A as follows.

0x11110000011111000

          ^    ^
          |    |
      M value  A value

A = 1 << (__builtin_ffs(v) - 1)
M = 1 << __builtin_ctz(v + A - 1)

I'll start a thread on the DTPM mailing list to confirm this. BTW, my P550 N-trace (pre-ratified) ramsink as A=8.

Quoted 94 lines
Quoted 25 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 165

Quoted 11 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+     rvtrace_write32(comp->pdata, lower_32_bits(end), RVTRACE_RAMSINK_LIMITLOW_OFF);
+     rvtrace_write32(comp->pdata, upper_32_bits(end), RVTRACE_RAMSINK_LIMITHIGH_OFF);
+     low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITLOW_OFF);
+     high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITHIGH_OFF);
+     end = (u64)(high) << 32 | low;
+     if (end != (priv->end - 4)) {
+             dev_warn(&comp->dev, "Ramsink limit address updated from %llx to %llx\n", priv->end,
+                      end);
+             priv->end = end;
+             priv->size = priv->end - priv->start;

Actually the RamLimit read is not the real limit. It's Limit - "A" (the write width).

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 97

Quoted 4 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +4 -0
+     }
+
+     return 0;
+}

I think overall we should simplify this setup_buf function. For RamStart alone, there're alignment + min_addr requirement, which is very unlikely that the lowest valid address happens to be within the range that dma- allocated in the caller (when you need to adjust priv->start). Thus, to do it properly, we first need to detect the min_addr + alignment *before*

Quoted 8 lines

dma-alloc, and pass such requirements into dma-alloc, then depending on what dma-alloc returns, detect the "M" value by interrogating RamLimit. I'm not sure if dma-alloc can be passed a hint address that whatever returned must be higher, and whether it can enforce random alignment, so may be we can just program RamStart/Limit based on what dma-alloc gives and error out if value is not retained. We do however need to probe for write width ("A" value). It's usually 4 or 8, but it does vary per-impl, and remember, the total buffer size is still 1MB, not 1MB - 4 or -8.

I think the issue is that the physical memory region returned by dma_alloc* might not be addressable by the hw and so the start / end addresses are already trimmed in the code accordingly but I agree this can be handled differently. For e.g
- probe max RamLimit value .
- set the dma_mask to the probed value
- dma_alloc*
- further address checks

Quoted 43 lines
Quoted 43 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +42 -0
+
+static int rvtrace_ramsink_setup(struct rvtrace_component *comp)
+{
+     struct device *pdev = comp->pdata->dev;
+     struct rvtrace_ramsink_priv *priv;
+
+     priv = devm_kzalloc(&comp->dev, sizeof(*priv), GFP_KERNEL);
+     if (!priv)
+             return -ENOMEM;
+
+     dev_set_drvdata(&comp->dev, priv);
+
+     /* Derive RAM sink memory size based on component implementation ID */
+     switch (comp->pdata->impid) {
+     default:
+             priv->size = SZ_1M;
+             break;
+     }
+
+     priv->va = dma_alloc_coherent(pdev, priv->size, &priv->start, GFP_KERNEL);
+     if (!priv->va)
+             return -ENOMEM;
+
+     return rvtrace_ramsink_setup_buf(comp);
+}
+
+static void rvtrace_ramsink_cleanup(struct rvtrace_component *comp)
+{
+     struct rvtrace_ramsink_priv *priv = dev_get_drvdata(&comp->dev);
+
+     dma_free_coherent(&comp->dev, priv->size, priv->va, priv->start);
+}
+
+static int rvtrace_ramsink_probe(struct rvtrace_component *comp)
+{
+     int ret;
+
+     ret = rvtrace_ramsink_setup(comp);
+     if (ret)
+             return dev_err_probe(&comp->dev, ret, "failed to setup ramsink.\n");
+
+     ret = rvtrace_enable_component(comp);
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 216

Quoted 5 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +5 -0
+     if (ret)
+             return dev_err_probe(&comp->dev, ret, "failed to enable ramsink.\n");
+
+     return ret;
+}

In probe, we should detect and set it to SMEM mode. The default is SRAM mode.

Quoted 3 lines

Actually the default is not set but I agree this needs to be handled differently. We can make this an implementation defined parameter and then cross check with trRamImpl register during probe.

You are right. The reset value of trRamMode is Undef. Thus, we do need to test and set it to SMEM mode. (It's a WARL field)

Quoted 25 lines
Quoted 17 lines
Quoted 17 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +16 -0
+
+static void rvtrace_ramsink_remove(struct rvtrace_component *comp)
+{
+     int ret;
+
+     ret = rvtrace_disable_component(comp);
+     if (ret)
+             dev_err(&comp->dev, "failed to disable ramsink.\n");
+
+     rvtrace_ramsink_cleanup(comp);
+}
+
+static struct rvtrace_component_id rvtrace_ramsink_ids[] = {
+     { .type = RVTRACE_COMPONENT_TYPE_RAMSINK,
+       .version = rvtrace_component_mkversion(1, 0), },
+     {},
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 237

Quoted 5 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +5 -0
+};
+
+static struct rvtrace_driver rvtrace_ramsink_driver = {
+     .id_table = rvtrace_ramsink_ids,
+     .copyto_auxbuf = rvtrace_ramsink_copyto_auxbuf,

Should have a .start function like the encoder (setting trRamEnable), and reset the WP to priv->start, as suggested above.

Quoted 4 lines

The active and enable bits for all components are handled via generic rvtrace_enable_component/rvtrace_disable_component functions. When all components are disabled before copying data to perf then WP would be reset to priv->start as a part of copy_to_auxbuf callback.

Disagree. You can't rely on enable/disable_component for ramsink. It's the same as encoder, you need proper start/stop. If the RamEnable bit is not cleared, trace may never gets flushed, and you'll never get RamEmpty. Also you can't access WP without clearing RamEnable.

I'd also think it's more appropriate to reset WP in start callback. As said earlier, new traces can overwrite already captured ones anyway, so why not reset WP on each start. Future enhancements we can support directly reading out the trace buffer when the encoder/sink is inactive, so better just leave the WP where it was after stopping.

Bo

Quoted 29 lines
Quoted 29 lines
Quoted 16 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +15 -0
+     .stop = rvtrace_ramsink_stop,
+     .probe = rvtrace_ramsink_probe,
+     .remove = rvtrace_ramsink_remove,
+     .driver = {
+             .name = "rvtrace-ramsink",
+     },
+};
+
+static int __init rvtrace_ramsink_init(void)
+{
+     return rvtrace_register_driver(&rvtrace_ramsink_driver);
+}
+
+static void __exit rvtrace_ramsink_exit(void)
+{

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 262

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+     rvtrace_unregister_driver(&rvtrace_ramsink_driver);
+}
+
+module_init(rvtrace_ramsink_init);
+module_exit(rvtrace_ramsink_exit);
+
+/* Module information */
+MODULE_AUTHOR("Mayuresh Chitale <[email protected]>");
+MODULE_DESCRIPTION("RISC-V Trace Ramsink Driver");
+MODULE_LICENSE("GPL");

Bo

#15 lore raw

Re: [PATCH v2 07/12] rvtrace: Add trace ramsink driver

Hi Bo,

Thanks,
Mayuresh.

Quoted 114 lines · Bo Gan

Hi Mayuresh,

Thanks for your reply. My comments below.

Quoted 108 lines · Mayuresh Chitale

Hi Bo,

Quoted 105 lines · Bo Gan

Hi Anup,

My comments below:

Quoted 56 lines · Anup Patel

From: Mayuresh Chitale <[email protected]>

Add initial implementation of RISC-V trace ramsink driver. The ramsink is defined in the RISC-V Trace Control Interface specification.

---
   drivers/hwtracing/rvtrace/Kconfig           |   9 +
   drivers/hwtracing/rvtrace/Makefile          |   1 +
   drivers/hwtracing/rvtrace/rvtrace-ramsink.c | 262 ++++++++++++++++++++
   3 files changed, 272 insertions(+)

create mode 100644 drivers/hwtracing/rvtrace/rvtrace-ramsink.c

drivers/hwtracing/rvtrace/Kconfig +9 -0
index ba35c05f3f54..0577f9acb858 100644
@@ -21,3 +21,12 @@ config RVTRACE_ENCODER
       default y
       help
         This driver provides support for RISC-V Trace Encoder component.
+
+config RVTRACE_RAMSINK
+     tristate "RISC-V Trace Ramsink driver"
+     depends on RVTRACE
+     select DMA_SHARED_BUFFER
+     default y
+     help
+       This driver provides support for Risc-V E-Trace Ramsink
+       component.
drivers/hwtracing/rvtrace/Makefile +1 -0
index f320693a1fc5..122e575da9fb 100644
@@ -3,3 +3,4 @@
   obj-$(CONFIG_RVTRACE) += rvtrace.o
   rvtrace-y := rvtrace-core.o rvtrace-platform.o
   obj-$(CONFIG_RVTRACE_ENCODER) += rvtrace-encoder.o
+obj-$(CONFIG_RVTRACE_RAMSINK) += rvtrace-ramsink.o
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +9 -0
new file mode 100644
index 000000000000..676344c9387c
@@ -0,0 +1,262 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 Ventana Micro Systems Inc.
+ */
+
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/dma-mapping.h>

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 19

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+#include <linux/rvtrace.h>
+#include <linux/types.h>
+#include <linux/sizes.h>
+
+#define RVTRACE_RAMSINK_STARTLOW_OFF         0x010
+#define RVTRACE_RAMSINK_STARTHIGH_OFF                0x014
+#define RVTRACE_RAMSINK_LIMITLOW_OFF         0x018
+#define RVTRACE_RAMSINK_LIMITHIGH_OFF                0x01c
+#define RVTRACE_RAMSINK_WPLOW_OFF            0x020
+#define   RVTRACE_RAMSINK_WPLOW_WRAP         0x1

nit: wrong spacing

Quoted 15 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +15 -0
+#define RVTRACE_RAMSINK_WPHIGH_OFF           0x024
+#define RVTRACE_RAMSINK_RPLOW_OFF            0x028
+#define RVTRACE_RAMSINK_RPHIGH_OFF           0x02c
+
+struct rvtrace_ramsink_priv {
+     size_t size;
+     void *va;
+     dma_addr_t start;
+     dma_addr_t end;
+     /* WP from prev iteration */
+     dma_addr_t prev_head;
+};
+
+struct trace_buf {
+     void *base;

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 44

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+     size_t size;
+     long cur;
+     size_t len;
+};
+
+static int rvtrace_ramsink_stop(struct rvtrace_component *comp)
+{
+     return rvtrace_comp_is_empty(comp);
+}
+

We should first set trRamEnable to 0, poll it, then poll trRamEmpty. It's essentially the same as encoder, so perhaps move it to core.c. Here we're just checking whether it's empty, which is wrong. We're also doing similar things in encoder.c Should fix that as well.

Quoted 8 lines
Quoted 5 lines

This code is called to just stop all components in a path which means:
- for encoders the driver clears the trTeInstTracing bit and then polls for empty bit to be cleared
- for ramsinks the driver just polls for the empty bit to be cleared More on this below.

Can we rename it to something like rvtrace_comp_poll_empty(...) for clarity? This name kind of confused me into thinking it's just a check.

Ok.

Quoted 65 lines
Quoted 60 lines
Quoted 39 lines
Quoted 39 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +37 -0
+static void tbuf_to_pbuf_copy(struct trace_buf *src, struct trace_buf *dst)
+{
+     int bytes_dst, bytes_src, bytes;
+     void *dst_addr, *src_addr;
+
+     while (src->size) {
+             src_addr = src->base + src->cur;
+             dst_addr = dst->base + dst->cur;
+
+             if (dst->len - dst->cur < src->size)
+                     bytes_dst = dst->len - dst->cur;
+             else
+                     bytes_dst = src->size;
+             if (src->len - src->cur < src->size)
+                     bytes_src = src->len - src->cur;
+             else
+                     bytes_src = src->size;
+             bytes = bytes_dst < bytes_src ? bytes_dst : bytes_src;
+             memcpy(dst_addr, src_addr, bytes);
+             dst->cur = (dst->cur + bytes) % dst->len;
+             src->cur = (src->cur + bytes) % src->len;
+             src->size -= bytes;
+     }
+}
+
+static size_t rvtrace_ramsink_copyto_auxbuf(struct rvtrace_component *comp,
+                                         struct rvtrace_perf_auxbuf *buf)
+{
+     struct rvtrace_ramsink_priv *priv = dev_get_drvdata(&comp->dev);
+     struct trace_buf src, dst;
+     u32 wp_low, wp_high;
+     u64 buf_cur_head;
+     size_t size;
+
+     wp_low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_WPLOW_OFF);
+     wp_high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_WPHIGH_OFF);
+     buf_cur_head = (u64)(wp_high) << 32 | (wp_low & ~RVTRACE_RAMSINK_WPLOW_WRAP);
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 84

Quoted 3 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +3 -0
+
+     if (buf_cur_head == priv->prev_head)
+             return 0;

cur_head == prev_head could mean that we've wrapped around and stopped at the exact position. Thus, we need to check for the wrapped around bit. If set, then this case can be merged into buf_cur_head <= priv->prev_head below.

cur_head is the current value in the wp registers whereas prev_head is the value in wp registers from the previous iteration which is saved explicitly by software. So prev_head can't change dynamically.

Quoted 9 lines

For cur_head > prev_head case, we also need to check for wrap-around. If wrapped-around, we actually filled the entire buffer, not just cur - prev We also should consider the possibility that the buffer has been wrapped around multiple times. In all cases, maintaining a prev_head seems to complicate things, so why don't we just reset the WP to priv->start each time we start the ramsink component and get rid of prev_head? This way, we'll always have cur >= start, and depending on wrapped bit, we either get the whole buffer, or cur - start.

Quoted 4 lines

Currently the software never clears trRamWrap bit because as per the spec:
- It is only set to 0 if trRamWPLow is written
- After trace is enabled and active , the trace tool should NOT write any of trRamStart/Limit/WP?? registers.

Quoted 9 lines

I think you missed my point. I'm not suggesting that we should change the register values when trace is active. I'm saying that there's no need to maintain a prev_head. For every trace session, we can just start fresh, and reset WP to Start before trace starts. This way we don't need to deal with the complex conditions of cur_head > prev_head, because you'll also need to deal with wrap around, and the if condition will be complex. The trace can wrap around and over-write old traces anyway, so I don't see a value of keeping track of prev_head.

We can't write the 'Start' value into WP if trRamEnable is set (please see my previous comment). In any case in v3, I am changing how the path gets stopped. After that we should be able to reset WP.

Quoted 106 lines
Quoted 89 lines

Let me test this change to disable all the components (instead of just stopping trace) every time we want to copy data to perf buffer. This should let us use the trRamWrap bit as expected. Multiple wraparounds can't be avoided unless StopOnWrap was set. I will update the ramsink driver to set StopOnWrap based on the implementation id.

Quoted 39 lines
Quoted 39 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +37 -0
+
+     dst.base = buf->base;
+     dst.len = buf->length;
+     dst.cur = buf->pos;
+
+     src.base = priv->va;
+     src.len = priv->end - priv->start;
+     if (buf_cur_head > priv->prev_head) {
+             src.size = buf_cur_head - priv->prev_head;
+     } else {
+             src.size = priv->end - priv->prev_head;
+             src.size += buf_cur_head - priv->start;
+     }
+
+     src.cur = buf_cur_head - priv->start;
+     size = src.size;
+     tbuf_to_pbuf_copy(&src, &dst);
+     buf->pos = dst.cur;
+     priv->prev_head = buf_cur_head;
+
+     return size;
+}
+
+static int rvtrace_ramsink_setup_buf(struct rvtrace_component *comp)
+{
+     struct rvtrace_ramsink_priv *priv = dev_get_drvdata(&comp->dev);
+     u64 start_min, limit_max, end;
+     u32 low, high;
+
+     /* Probe min and max values for start and limit registers */
+     rvtrace_write32(comp->pdata, 0, RVTRACE_RAMSINK_STARTLOW_OFF);
+     rvtrace_write32(comp->pdata, 0, RVTRACE_RAMSINK_STARTHIGH_OFF);
+     low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_STARTLOW_OFF);
+     high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_STARTHIGH_OFF);
+     start_min = (u64)(high) << 32 | low;
+
+     rvtrace_write32(comp->pdata, 0xffffffff, RVTRACE_RAMSINK_LIMITLOW_OFF);
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 127

Quoted 6 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +6 -0
+     rvtrace_write32(comp->pdata, 0xffffffff, RVTRACE_RAMSINK_LIMITHIGH_OFF);
+     low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITLOW_OFF);
+     high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITHIGH_OFF);
+     limit_max = (u64)(high) << 32 | low;
+
+     if (priv->end < start_min) {

Where was priv->end initialized before this check?

I will move it before the check.

Quoted 28 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 133

Quoted 7 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +6 -0
+             dev_err(&comp->dev, "DMA memory not addressable by device\n");
+             return -EINVAL;
+     }
+
+     /* Setup ram sink start addresses */
+     if (priv->start < start_min) {

No checks for start_min >= priv->end?

Quoted 18 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +18 -0
+             dev_warn(&comp->dev, "Ramsink start address updated from %llx to %llx\n",
+                      priv->start, start_min);
+             priv->va += start_min - priv->start;
+             priv->start = start_min;
+     }
+
+     priv->prev_head = priv->start;
+     priv->end = priv->start + priv->size;
+     rvtrace_write32(comp->pdata, lower_32_bits(priv->start), RVTRACE_RAMSINK_STARTLOW_OFF);
+     rvtrace_write32(comp->pdata, upper_32_bits(priv->start), RVTRACE_RAMSINK_STARTHIGH_OFF);
+     rvtrace_write32(comp->pdata, lower_32_bits(priv->start), RVTRACE_RAMSINK_WPLOW_OFF);
+     rvtrace_write32(comp->pdata, upper_32_bits(priv->start), RVTRACE_RAMSINK_WPHIGH_OFF);
+     /* Setup ram sink limit addresses */
+     if (priv->end > limit_max) {
+             dev_warn(&comp->dev, "Ramsink limit address updated from %llx to %llx\n", priv->end,
+                      limit_max);
+             priv->end = limit_max;
+             priv->size = priv->end - priv->start;
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 155

Quoted 4 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +4 -0
+     }
+
+     /* Limit address needs to be set to end - 4 to avoid overflow */
+     end = priv->end - 4;

Should not hard-code 4, instead, do write32(0xfffffff, RamLimit), then ffs(RamLimit) to probe for the the write width, aka, "A" value in Table
47. Typical Trace RAM Sink Configuration in the Spec.

Quoted 7 lines

I dont think 'A' can be probed that way because as per section 7.1:
"Not every value may be settable in trRamStart/Limit registers. Value written may be trimmed (for example aligned on a particular 2^N boundary)". So writing 0xffffffff to limit register will reveal the boundary alignment but not 'A'. I think 'A' needs to be specified by an implementation with default as 4. I will change it accordingly.

Quoting from spec:

"Value A means alignment which depends on memory access width. If we
have memory access width of 32-bits, A=4 and value of trRamLimit register should be 0x…FC. Some implementations may impose bigger alignment of trace data (to allow more efficient transfer rates) for SMEM mode. For SRAM mode A must be 4 as access to trace via trRamData is always 32-bits wide "

Quoted 3 lines

the 4 byte of A value is only for SRAM mode, not SMEM mode. My inter- pretation of the spec is that this can be detected by writing -1 to Limit, and detect M, A as follows.

No. The text above doesn't talk about probing the value of 'A', just that 'A' is implementation specific.

Quoted 10 lines

0x11110000011111000

           ^    ^
           |    |
       M value  A value

A = 1 << (__builtin_ffs(v) - 1)
M = 1 << __builtin_ctz(v + A - 1)

I'll start a thread on the DTPM mailing list to confirm this.

OK.

BTW,

Quoted 1 line

my P550 N-trace (pre-ratified) ramsink as A=8.

You can set it by defining a new id for P550 N-trace in the rvtrace_component_impid enum. Please refer rvtrace.h for more details.

Quoted 132 lines
Quoted 95 lines
Quoted 26 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 165

Quoted 12 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+     rvtrace_write32(comp->pdata, lower_32_bits(end), RVTRACE_RAMSINK_LIMITLOW_OFF);
+     rvtrace_write32(comp->pdata, upper_32_bits(end), RVTRACE_RAMSINK_LIMITHIGH_OFF);
+     low = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITLOW_OFF);
+     high = rvtrace_read32(comp->pdata, RVTRACE_RAMSINK_LIMITHIGH_OFF);
+     end = (u64)(high) << 32 | low;
+     if (end != (priv->end - 4)) {
+             dev_warn(&comp->dev, "Ramsink limit address updated from %llx to %llx\n", priv->end,
+                      end);
+             priv->end = end;
+             priv->size = priv->end - priv->start;

Actually the RamLimit read is not the real limit. It's Limit - "A" (the write width).

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 97

Quoted 4 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +4 -0
+     }
+
+     return 0;
+}

I think overall we should simplify this setup_buf function. For RamStart alone, there're alignment + min_addr requirement, which is very unlikely that the lowest valid address happens to be within the range that dma- allocated in the caller (when you need to adjust priv->start). Thus, to do it properly, we first need to detect the min_addr + alignment *before*

Quoted 8 lines

dma-alloc, and pass such requirements into dma-alloc, then depending on what dma-alloc returns, detect the "M" value by interrogating RamLimit. I'm not sure if dma-alloc can be passed a hint address that whatever returned must be higher, and whether it can enforce random alignment, so may be we can just program RamStart/Limit based on what dma-alloc gives and error out if value is not retained. We do however need to probe for write width ("A" value). It's usually 4 or 8, but it does vary per-impl, and remember, the total buffer size is still 1MB, not 1MB - 4 or -8.

I think the issue is that the physical memory region returned by dma_alloc* might not be addressable by the hw and so the start / end addresses are already trimmed in the code accordingly but I agree this can be handled differently. For e.g
- probe max RamLimit value .
- set the dma_mask to the probed value
- dma_alloc*
- further address checks

Quoted 43 lines
Quoted 43 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +42 -0
+
+static int rvtrace_ramsink_setup(struct rvtrace_component *comp)
+{
+     struct device *pdev = comp->pdata->dev;
+     struct rvtrace_ramsink_priv *priv;
+
+     priv = devm_kzalloc(&comp->dev, sizeof(*priv), GFP_KERNEL);
+     if (!priv)
+             return -ENOMEM;
+
+     dev_set_drvdata(&comp->dev, priv);
+
+     /* Derive RAM sink memory size based on component implementation ID */
+     switch (comp->pdata->impid) {
+     default:
+             priv->size = SZ_1M;
+             break;
+     }
+
+     priv->va = dma_alloc_coherent(pdev, priv->size, &priv->start, GFP_KERNEL);
+     if (!priv->va)
+             return -ENOMEM;
+
+     return rvtrace_ramsink_setup_buf(comp);
+}
+
+static void rvtrace_ramsink_cleanup(struct rvtrace_component *comp)
+{
+     struct rvtrace_ramsink_priv *priv = dev_get_drvdata(&comp->dev);
+
+     dma_free_coherent(&comp->dev, priv->size, priv->va, priv->start);
+}
+
+static int rvtrace_ramsink_probe(struct rvtrace_component *comp)
+{
+     int ret;
+
+     ret = rvtrace_ramsink_setup(comp);
+     if (ret)
+             return dev_err_probe(&comp->dev, ret, "failed to setup ramsink.\n");
+
+     ret = rvtrace_enable_component(comp);
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 216

Quoted 5 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +5 -0
+     if (ret)
+             return dev_err_probe(&comp->dev, ret, "failed to enable ramsink.\n");
+
+     return ret;
+}

In probe, we should detect and set it to SMEM mode. The default is SRAM mode.

Quoted 3 lines

Actually the default is not set but I agree this needs to be handled differently. We can make this an implementation defined parameter and then cross check with trRamImpl register during probe.

You are right. The reset value of trRamMode is Undef. Thus, we do need to test and set it to SMEM mode. (It's a WARL field)

Quoted 25 lines
Quoted 17 lines
Quoted 17 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +16 -0
+
+static void rvtrace_ramsink_remove(struct rvtrace_component *comp)
+{
+     int ret;
+
+     ret = rvtrace_disable_component(comp);
+     if (ret)
+             dev_err(&comp->dev, "failed to disable ramsink.\n");
+
+     rvtrace_ramsink_cleanup(comp);
+}
+
+static struct rvtrace_component_id rvtrace_ramsink_ids[] = {
+     { .type = RVTRACE_COMPONENT_TYPE_RAMSINK,
+       .version = rvtrace_component_mkversion(1, 0), },
+     {},
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 237

Quoted 5 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +5 -0
+};
+
+static struct rvtrace_driver rvtrace_ramsink_driver = {
+     .id_table = rvtrace_ramsink_ids,
+     .copyto_auxbuf = rvtrace_ramsink_copyto_auxbuf,

Should have a .start function like the encoder (setting trRamEnable), and reset the WP to priv->start, as suggested above.

Quoted 4 lines

The active and enable bits for all components are handled via generic rvtrace_enable_component/rvtrace_disable_component functions. When all components are disabled before copying data to perf then WP would be reset to priv->start as a part of copy_to_auxbuf callback.

Quoted 8 lines

Disagree. You can't rely on enable/disable_component for ramsink. It's the same as encoder, you need proper start/stop. If the RamEnable bit is not cleared, trace may never gets flushed, and you'll never get RamEmpty. Also you can't access WP without clearing RamEnable.

I'd also think it's more appropriate to reset WP in start callback. As said earlier, new traces can overwrite already captured ones anyway, so why not reset WP on each start.

Please see the comment above.

Quoted 36 lines

Future enhancements we can support
directly reading out the trace buffer when the encoder/sink is inactive, so better just leave the WP where it was after stopping.

Bo

Quoted 29 lines
Quoted 29 lines
Quoted 16 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +15 -0
+     .stop = rvtrace_ramsink_stop,
+     .probe = rvtrace_ramsink_probe,
+     .remove = rvtrace_ramsink_remove,
+     .driver = {
+             .name = "rvtrace-ramsink",
+     },
+};
+
+static int __init rvtrace_ramsink_init(void)
+{
+     return rvtrace_register_driver(&rvtrace_ramsink_driver);
+}
+
+static void __exit rvtrace_ramsink_exit(void)
+{

drivers/hwtracing/rvtrace/rvtrace-ramsink.c line 262

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-ramsink.c new file +10 -0
+     rvtrace_unregister_driver(&rvtrace_ramsink_driver);
+}
+
+module_init(rvtrace_ramsink_init);
+module_exit(rvtrace_ramsink_exit);
+
+/* Module information */
+MODULE_AUTHOR("Mayuresh Chitale <[email protected]>");
+MODULE_DESCRIPTION("RISC-V Trace Ramsink Driver");
+MODULE_LICENSE("GPL");

Bo

author
#16 lore raw

PATCH v2 8/12 riscv: Enable DMA_RESTRICTED_POOL in defconfig

In reply to Anup Patel

The RISC-V ramsink trace component may have implementation specific restrictions such that the component can only write trace data in particular parts of DRAM.

Enable DMA_RESTRICTED_POOL in the defconfig so that dma_alloc_*() and dma_free_*() APIs work for devices with DMA address restrictions.

---
 arch/riscv/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)
arch/riscv/configs/defconfig +1 -0
index d2c1ea2a866c..4e870b8f2220 100644
@@ -305,6 +305,7 @@ CONFIG_SECURITY_APPARMOR=y
 CONFIG_DEFAULT_SECURITY_DAC=y
 CONFIG_CRYPTO_USER_API_HASH=y
 CONFIG_CRYPTO_DEV_VIRTIO=y
+CONFIG_DMA_RESTRICTED_POOL=y
 CONFIG_PRINTK_TIME=y
 CONFIG_DEBUG_KERNEL=y
 CONFIG_DEBUG_FS=y
Signature · 1 line
2.43.0
author
#17 lore raw

PATCH v2 9/12 rvtrace: Add perf driver for tracing using perf tool

In reply to Anup Patel

From: Mayuresh Chitale <[email protected]>

Add perf driver for RISC-V tracing similar to ARM Coresight and Hisilicon PTT drivers. The driver adds 'rvtrace' event descriptor which can be used by the perf tool to record the RISC-V trace data.

---
 drivers/hwtracing/rvtrace/Kconfig        |   1 +
 drivers/hwtracing/rvtrace/Makefile       |   2 +-
 drivers/hwtracing/rvtrace/rvtrace-core.c |   8 +
 drivers/hwtracing/rvtrace/rvtrace-perf.c | 343 +++++++++++++++++++++++
 include/linux/rvtrace.h                  |   3 +
 5 files changed, 356 insertions(+), 1 deletion(-)
 create mode 100644 drivers/hwtracing/rvtrace/rvtrace-perf.c
drivers/hwtracing/rvtrace/Kconfig +1 -0
index 0577f9acb858..ba11acf1117d 100644
@@ -4,6 +4,7 @@ menuconfig RVTRACE
 	tristate "RISC-V Trace Support"
 	depends on RISCV
 	depends on OF
+	select PERF_EVENTS
 	default RISCV
 	help
 	  This framework provides a kernel interface for the RISC-V trace
drivers/hwtracing/rvtrace/Makefile +1 -1
index 122e575da9fb..07403f4d94e3 100644
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_RVTRACE) += rvtrace.o
-rvtrace-y := rvtrace-core.o rvtrace-platform.o
+rvtrace-y := rvtrace-core.o rvtrace-platform.o rvtrace-perf.o
 obj-$(CONFIG_RVTRACE_ENCODER) += rvtrace-encoder.o
 obj-$(CONFIG_RVTRACE_RAMSINK) += rvtrace-ramsink.o
drivers/hwtracing/rvtrace/rvtrace-core.c +8 -0
index b955e5f3b048..bbe39aaf930d 100644
@@ -777,11 +777,19 @@ static int __init rvtrace_init(void)
 		return ret;
 	}
 
+	ret = rvtrace_perf_init();
+	if (ret) {
+		platform_driver_unregister(&rvtrace_platform_driver);
+		bus_unregister(&rvtrace_bustype);
+		return ret;
+	}
+
 	return 0;
 }
 
 static void __exit rvtrace_exit(void)
 {
+	rvtrace_perf_exit();
 	platform_driver_unregister(&rvtrace_platform_driver);
 	bus_unregister(&rvtrace_bustype);
 }
drivers/hwtracing/rvtrace/rvtrace-perf.c new file +343 -0

343 lines not on this page · raw message

include/linux/rvtrace.h +3 -0
index cecf6c153ca6..d5f782ac132f 100644
@@ -343,4 +343,7 @@ static inline int rvtrace_comp_is_empty(struct rvtrace_component *comp)
 				comp->pdata->control_poll_timeout_usecs);
 }
 
+int rvtrace_perf_init(void);
+void rvtrace_perf_exit(void);
+
 #endif
Signature · 1 line
2.43.0
#18 lore raw

Re: [PATCH v2 09/12] rvtrace: Add perf driver for tracing using perf tool

Hi Anup,

Quoted 86 lines · Anup Patel

From: Mayuresh Chitale <[email protected]>

Add perf driver for RISC-V tracing similar to ARM Coresight and Hisilicon PTT drivers. The driver adds 'rvtrace' event descriptor which can be used by the perf tool to record the RISC-V trace data.

---
 drivers/hwtracing/rvtrace/Kconfig        |   1 +
 drivers/hwtracing/rvtrace/Makefile       |   2 +-
 drivers/hwtracing/rvtrace/rvtrace-core.c |   8 +
 drivers/hwtracing/rvtrace/rvtrace-perf.c | 343 +++++++++++++++++++++++
 include/linux/rvtrace.h                  |   3 +
 5 files changed, 356 insertions(+), 1 deletion(-)
 create mode 100644 drivers/hwtracing/rvtrace/rvtrace-perf.c
drivers/hwtracing/rvtrace/Kconfig +1 -0
index 0577f9acb858..ba11acf1117d 100644
@@ -4,6 +4,7 @@ menuconfig RVTRACE
        tristate "RISC-V Trace Support"
        depends on RISCV
        depends on OF
+       select PERF_EVENTS
        default RISCV
        help
          This framework provides a kernel interface for the RISC-V trace
drivers/hwtracing/rvtrace/Makefile +1 -1
index 122e575da9fb..07403f4d94e3 100644
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_RVTRACE) += rvtrace.o
-rvtrace-y := rvtrace-core.o rvtrace-platform.o
+rvtrace-y := rvtrace-core.o rvtrace-platform.o rvtrace-perf.o
 obj-$(CONFIG_RVTRACE_ENCODER) += rvtrace-encoder.o
 obj-$(CONFIG_RVTRACE_RAMSINK) += rvtrace-ramsink.o
drivers/hwtracing/rvtrace/rvtrace-core.c +8 -0
index b955e5f3b048..bbe39aaf930d 100644
@@ -777,11 +777,19 @@ static int __init rvtrace_init(void)
                return ret;
        }
 
+       ret = rvtrace_perf_init();
+       if (ret) {
+               platform_driver_unregister(&rvtrace_platform_driver);
+               bus_unregister(&rvtrace_bustype);
+               return ret;
+       }
+
        return 0;
 }
 
 static void __exit rvtrace_exit(void)
 {
+       rvtrace_perf_exit();
        platform_driver_unregister(&rvtrace_platform_driver);
        bus_unregister(&rvtrace_bustype);
 }
drivers/hwtracing/rvtrace/rvtrace-perf.c new file +13 -0
new file mode 100644
index 000000000000..2d3039f8b681
@@ -0,0 +1,343 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright(C) 2025 Ventanamicro Limited. All rights reserved.
+ * Author: Mayuresh Chitale <[email protected]>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/cpumask.h>
+#include <linux/device.h>
+#include <linux/list.h>
+#include <linux/mm.h>
+#include <linux/init.h>
+#include <linux/perf_event.h>

drivers/hwtracing/rvtrace/rvtrace-perf.c line 23

Quoted 10 lines
drivers/hwtracing/rvtrace/rvtrace-perf.c new file +10 -0
+#include <linux/vmalloc.h>
+#include <linux/percpu-defs.h>
+#include <linux/slab.h>
+#include <linux/stringhash.h>
+#include <linux/types.h>
+#include <linux/workqueue.h>
+#include <linux/rvtrace.h>
+
+#define RVTRACE_PMU_NAME "rvtrace"
+#define RVTRACE_BUF_LEN (4 * 1024 * 1024)

Currently, it seems "RVTRACE_BUF_LEN" is not used. Should we remove it?

Best regards,
Eric Lin

Quoted 340 lines
drivers/hwtracing/rvtrace/rvtrace-perf.c new file +320 -0
+
+static struct pmu rvtrace_pmu;
+static DEFINE_SPINLOCK(perf_buf_lock);
+
+/**
+ * struct rvtrace_event_data - RISC-V trace specific perf event data
+ * @work:              Handle to free allocated memory outside IRQ context.
+ * @mask:              Hold the CPU(s) this event was set for.
+ * @aux_hwid_done:     Whether a CPU has emitted the TraceID packet or not.
+ * @path:              An array of path, each slot for one CPU.
+ * @buf:               Aux buffer / pages allocated by perf framework.
+ */
+struct rvtrace_event_data {
+       struct work_struct work;
+       cpumask_t mask;
+       cpumask_t aux_hwid_done;
+       struct rvtrace_path * __percpu *path;
+       struct rvtrace_perf_auxbuf buf;
+};
+
+struct rvtrace_ctxt {
+       struct perf_output_handle handle;
+       struct rvtrace_event_data *event_data;
+};
+
+static DEFINE_PER_CPU(struct rvtrace_ctxt, rvtrace_ctxt);
+
+static void *alloc_event_data(int cpu)
+{
+       struct rvtrace_event_data *event_data;
+       cpumask_t *mask;
+
+       event_data = kzalloc(sizeof(*event_data), GFP_KERNEL);
+       if (!event_data)
+               return NULL;
+
+       /* Update mask as per selected CPUs */
+       mask = &event_data->mask;
+       if (cpu != -1)
+               cpumask_set_cpu(cpu, mask);
+       else
+               cpumask_copy(mask, cpu_present_mask);
+
+       event_data->path = alloc_percpu(struct rvtrace_path *);
+       return event_data;
+}
+
+static void rvtrace_free_aux(void *data)
+{
+       struct rvtrace_event_data *event_data = data;
+
+       schedule_work(&event_data->work);
+}
+
+static struct rvtrace_path **rvtrace_event_cpu_path_ptr(struct rvtrace_event_data *data,
+                                                       int cpu)
+{
+       return per_cpu_ptr(data->path, cpu);
+}
+
+static void free_event_data(struct work_struct *work)
+{
+       struct rvtrace_event_data *event_data;
+       struct rvtrace_path *path;
+       cpumask_t *mask;
+       int cpu;
+
+       event_data = container_of(work, struct rvtrace_event_data, work);
+       mask = &event_data->mask;
+       for_each_cpu(cpu, mask) {
+               path = *rvtrace_event_cpu_path_ptr(event_data, cpu);
+               rvtrace_destroy_path(path);
+       }
+       free_percpu(event_data->path);
+       kfree(event_data);
+}
+
+static void *rvtrace_setup_aux(struct perf_event *event, void **pages,
+                              int nr_pages, bool overwrite)
+{
+       struct rvtrace_event_data *event_data = NULL;
+       struct page **pagelist;
+       int cpu = event->cpu, i;
+       cpumask_t *mask;
+
+       event_data = alloc_event_data(cpu);
+       if (!event_data)
+               return NULL;
+
+       INIT_WORK(&event_data->work, free_event_data);
+       mask = &event_data->mask;
+       /*
+        * Create the path for each CPU in the mask. In case of any failure skip the CPU
+        */
+       for_each_cpu(cpu, mask) {
+               struct rvtrace_component *src;
+               struct rvtrace_path *path;
+
+               src = rvtrace_cpu_source(cpu);
+               if (!src)
+                       continue;
+
+               path = rvtrace_create_path(src, NULL, RVTRACE_COMPONENT_MODE_PERF);
+               if (!path)
+                       continue;
+
+               *rvtrace_event_cpu_path_ptr(event_data, cpu) = path;
+       }
+
+       /* If we don't have any CPUs ready for tracing, abort */
+       cpu = cpumask_first(&event_data->mask);
+       if (cpu >= nr_cpu_ids)
+               goto err;
+
+       pagelist = kcalloc(nr_pages, sizeof(*pagelist), GFP_KERNEL);
+       if (!pagelist)
+               goto err;
+
+       for (i = 0; i < nr_pages; i++)
+               pagelist[i] = virt_to_page(pages[i]);
+
+       event_data->buf.base = vmap(pagelist, nr_pages, VM_MAP, PAGE_KERNEL);
+       if (!event_data->buf.base) {
+               kfree(pagelist);
+               goto err;
+       }
+
+       event_data->buf.nr_pages = nr_pages;
+       event_data->buf.length = nr_pages * PAGE_SIZE;
+       event_data->buf.pos = 0;
+       return event_data;
+err:
+       rvtrace_free_aux(event_data);
+       return NULL;
+}
+
+static void rvtrace_event_read(struct perf_event *event)
+{
+}
+
+static void rvtrace_event_destroy(struct perf_event *event)
+{
+}
+
+static int rvtrace_event_init(struct perf_event *event)
+{
+       if (event->attr.type != rvtrace_pmu.type)
+               return -EINVAL;
+
+       event->destroy = rvtrace_event_destroy;
+       return 0;
+}
+
+static void rvtrace_event_start(struct perf_event *event, int flags)
+{
+       struct rvtrace_ctxt *ctxt = this_cpu_ptr(&rvtrace_ctxt);
+       struct perf_output_handle *handle = &ctxt->handle;
+       struct rvtrace_event_data *event_data;
+       int cpu = smp_processor_id();
+       struct rvtrace_path *path;
+
+       if (WARN_ON(ctxt->event_data))
+               goto fail;
+
+       /*
+        * Deal with the ring buffer API and get a handle on the
+        * session's information.
+        */
+       event_data = perf_aux_output_begin(handle, event);
+       if (!event_data)
+               goto fail;
+
+       if (!cpumask_test_cpu(cpu, &event_data->mask))
+               goto out;
+
+       event_data->buf.pos = handle->head % event_data->buf.length;
+       path = *rvtrace_event_cpu_path_ptr(event_data, cpu);
+       if (!path) {
+               pr_err("Error. Path not found\n");
+               return;
+       }
+
+       if (rvtrace_path_start(path)) {
+               pr_err("Error. Tracing not started\n");
+               return;
+       }
+
+       /*
+        * output cpu / trace ID in perf record, once for the lifetime
+        * of the event.
+        */
+       if (!cpumask_test_cpu(cpu, &event_data->aux_hwid_done)) {
+               cpumask_set_cpu(cpu, &event_data->aux_hwid_done);
+               perf_report_aux_output_id(event, cpu);
+       }
+
+out:
+       /* Tell the perf core the event is alive */
+       event->hw.state = 0;
+       ctxt->event_data = event_data;
+       return;
+fail:
+       event->hw.state = PERF_HES_STOPPED;
+}
+
+static void rvtrace_event_stop(struct perf_event *event, int mode)
+{
+       struct rvtrace_ctxt *ctxt = this_cpu_ptr(&rvtrace_ctxt);
+       struct perf_output_handle *handle = &ctxt->handle;
+       struct rvtrace_event_data *event_data;
+       int ret, cpu = smp_processor_id();
+       struct rvtrace_path *path;
+       size_t size;
+
+       if (event->hw.state == PERF_HES_STOPPED)
+               return;
+
+       if (handle->event &&
+           WARN_ON(perf_get_aux(handle) != ctxt->event_data))
+               return;
+
+       event_data = ctxt->event_data;
+       ctxt->event_data = NULL;
+
+       if (WARN_ON(!event_data))
+               return;
+
+       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;
+       }
+
+       /* stop tracing */
+       path = *rvtrace_event_cpu_path_ptr(event_data, cpu);
+       if (!path) {
+               pr_err("Error. Path not found\n");
+               return;
+       }
+
+       if (rvtrace_path_stop(path)) {
+               pr_err("Error. Tracing not stopped\n");
+               return;
+       }
+
+       event->hw.state = PERF_HES_STOPPED;
+       if (handle->event && (mode & PERF_EF_UPDATE)) {
+               if (WARN_ON_ONCE(handle->event != event))
+                       return;
+               spin_lock(&perf_buf_lock);
+               ret = rvtrace_path_copyto_auxbuf(path, &event_data->buf, &size);
+               spin_unlock(&perf_buf_lock);
+               WARN_ON_ONCE(ret);
+               if (READ_ONCE(handle->event))
+                       perf_aux_output_end(handle, size);
+               else
+                       WARN_ON(size);
+       }
+}
+
+static int rvtrace_event_add(struct perf_event *event, int mode)
+{
+       struct hw_perf_event *hwc = &event->hw;
+       int ret = 0;
+
+       if (mode & PERF_EF_START) {
+               rvtrace_event_start(event, 0);
+               if (hwc->state & PERF_HES_STOPPED)
+                       ret = -EINVAL;
+       } else {
+               hwc->state = PERF_HES_STOPPED;
+       }
+
+       return ret;
+}
+
+static void rvtrace_event_del(struct perf_event *event, int mode)
+{
+       rvtrace_event_stop(event, PERF_EF_UPDATE);
+}
+
+PMU_FORMAT_ATTR(event, "config:0-0");
+
+static struct attribute *rvtrace_pmu_formats_attr[] = {
+       &format_attr_event.attr,
+       NULL,
+};
+
+static struct attribute_group rvtrace_pmu_format_group = {
+       .name = "format",
+       .attrs = rvtrace_pmu_formats_attr,
+};
+
+static const struct attribute_group *rvtrace_pmu_attr_groups[] = {
+       &rvtrace_pmu_format_group,
+       NULL,
+};
+
+int __init rvtrace_perf_init(void)
+{
+       rvtrace_pmu.capabilities        = (PERF_PMU_CAP_EXCLUSIVE | PERF_PMU_CAP_ITRACE);
+       rvtrace_pmu.attr_groups         = rvtrace_pmu_attr_groups;
+       rvtrace_pmu.task_ctx_nr         = perf_sw_context;
+       rvtrace_pmu.read                = rvtrace_event_read;
+       rvtrace_pmu.event_init          = rvtrace_event_init;
+       rvtrace_pmu.setup_aux           = rvtrace_setup_aux;
+       rvtrace_pmu.free_aux            = rvtrace_free_aux;
+       rvtrace_pmu.start               = rvtrace_event_start;
+       rvtrace_pmu.stop                = rvtrace_event_stop;
+       rvtrace_pmu.add                 = rvtrace_event_add;
+       rvtrace_pmu.del                 = rvtrace_event_del;
+       rvtrace_pmu.module              = THIS_MODULE;
+
+       return perf_pmu_register(&rvtrace_pmu, RVTRACE_PMU_NAME, -1);
+}
+
+void __exit rvtrace_perf_exit(void)
+{
+       perf_pmu_unregister(&rvtrace_pmu);
+}
include/linux/rvtrace.h +3 -1
index cecf6c153ca6..d5f782ac132f 100644
@@ -343,4 +343,7 @@ static inline int rvtrace_comp_is_empty(struct rvtrace_component *comp)
                                comp->pdata->control_poll_timeout_usecs);
 }
 
+int rvtrace_perf_init(void);
+void rvtrace_perf_exit(void);
+
 #endif
--

2.43.0

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

#19 lore raw

Re: [PATCH v2 09/12] rvtrace: Add perf driver for tracing using perf tool

Hi Eric,

Quoted 94 lines · Eric Lin

Hi Anup,

Quoted 90 lines · Anup Patel

From: Mayuresh Chitale <[email protected]>

Add perf driver for RISC-V tracing similar to ARM Coresight and Hisilicon PTT drivers. The driver adds 'rvtrace' event descriptor which can be used by the perf tool to record the RISC-V trace data.

---
 drivers/hwtracing/rvtrace/Kconfig        |   1 +
 drivers/hwtracing/rvtrace/Makefile       |   2 +-
 drivers/hwtracing/rvtrace/rvtrace-core.c |   8 +
 drivers/hwtracing/rvtrace/rvtrace-perf.c | 343 +++++++++++++++++++++++
 include/linux/rvtrace.h                  |   3 +
 5 files changed, 356 insertions(+), 1 deletion(-)
 create mode 100644 drivers/hwtracing/rvtrace/rvtrace-perf.c
drivers/hwtracing/rvtrace/Kconfig +1 -0
index 0577f9acb858..ba11acf1117d 100644
@@ -4,6 +4,7 @@ menuconfig RVTRACE
        tristate "RISC-V Trace Support"
        depends on RISCV
        depends on OF
+       select PERF_EVENTS
        default RISCV
        help
          This framework provides a kernel interface for the RISC-V trace
drivers/hwtracing/rvtrace/Makefile +1 -1
index 122e575da9fb..07403f4d94e3 100644
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_RVTRACE) += rvtrace.o
-rvtrace-y := rvtrace-core.o rvtrace-platform.o
+rvtrace-y := rvtrace-core.o rvtrace-platform.o rvtrace-perf.o
 obj-$(CONFIG_RVTRACE_ENCODER) += rvtrace-encoder.o
 obj-$(CONFIG_RVTRACE_RAMSINK) += rvtrace-ramsink.o
drivers/hwtracing/rvtrace/rvtrace-core.c +8 -0
index b955e5f3b048..bbe39aaf930d 100644
@@ -777,11 +777,19 @@ static int __init rvtrace_init(void)
                return ret;
        }
 
+       ret = rvtrace_perf_init();
+       if (ret) {
+               platform_driver_unregister(&rvtrace_platform_driver);
+               bus_unregister(&rvtrace_bustype);
+               return ret;
+       }
+
        return 0;
 }
 
 static void __exit rvtrace_exit(void)
 {
+       rvtrace_perf_exit();
        platform_driver_unregister(&rvtrace_platform_driver);
        bus_unregister(&rvtrace_bustype);
 }
drivers/hwtracing/rvtrace/rvtrace-perf.c new file +17 -0
new file mode 100644
index 000000000000..2d3039f8b681
@@ -0,0 +1,343 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright(C) 2025 Ventanamicro Limited. All rights reserved.
+ * Author: Mayuresh Chitale <[email protected]>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/cpumask.h>
+#include <linux/device.h>
+#include <linux/list.h>
+#include <linux/mm.h>
+#include <linux/init.h>
+#include <linux/perf_event.h>
+#include <linux/vmalloc.h>
+#include <linux/percpu-defs.h>
+#include <linux/slab.h>
+#include <linux/stringhash.h>
Quoted 8 lines

drivers/hwtracing/rvtrace/rvtrace-perf.c line 23

Quoted 6 lines
drivers/hwtracing/rvtrace/rvtrace-perf.c new file +6 -0
+#include <linux/types.h>
+#include <linux/workqueue.h>
+#include <linux/rvtrace.h>
+
+#define RVTRACE_PMU_NAME "rvtrace"
+#define RVTRACE_BUF_LEN (4 * 1024 * 1024)

Currently, it seems "RVTRACE_BUF_LEN" is not used. Should we remove it?

Yes, it can be removed.

Quoted 343 lines

Best regards,
Eric Lin

Quoted 340 lines
drivers/hwtracing/rvtrace/rvtrace-perf.c new file +320 -0
+
+static struct pmu rvtrace_pmu;
+static DEFINE_SPINLOCK(perf_buf_lock);
+
+/**
+ * struct rvtrace_event_data - RISC-V trace specific perf event data
+ * @work:              Handle to free allocated memory outside IRQ context.
+ * @mask:              Hold the CPU(s) this event was set for.
+ * @aux_hwid_done:     Whether a CPU has emitted the TraceID packet or not.
+ * @path:              An array of path, each slot for one CPU.
+ * @buf:               Aux buffer / pages allocated by perf framework.
+ */
+struct rvtrace_event_data {
+       struct work_struct work;
+       cpumask_t mask;
+       cpumask_t aux_hwid_done;
+       struct rvtrace_path * __percpu *path;
+       struct rvtrace_perf_auxbuf buf;
+};
+
+struct rvtrace_ctxt {
+       struct perf_output_handle handle;
+       struct rvtrace_event_data *event_data;
+};
+
+static DEFINE_PER_CPU(struct rvtrace_ctxt, rvtrace_ctxt);
+
+static void *alloc_event_data(int cpu)
+{
+       struct rvtrace_event_data *event_data;
+       cpumask_t *mask;
+
+       event_data = kzalloc(sizeof(*event_data), GFP_KERNEL);
+       if (!event_data)
+               return NULL;
+
+       /* Update mask as per selected CPUs */
+       mask = &event_data->mask;
+       if (cpu != -1)
+               cpumask_set_cpu(cpu, mask);
+       else
+               cpumask_copy(mask, cpu_present_mask);
+
+       event_data->path = alloc_percpu(struct rvtrace_path *);
+       return event_data;
+}
+
+static void rvtrace_free_aux(void *data)
+{
+       struct rvtrace_event_data *event_data = data;
+
+       schedule_work(&event_data->work);
+}
+
+static struct rvtrace_path **rvtrace_event_cpu_path_ptr(struct rvtrace_event_data *data,
+                                                       int cpu)
+{
+       return per_cpu_ptr(data->path, cpu);
+}
+
+static void free_event_data(struct work_struct *work)
+{
+       struct rvtrace_event_data *event_data;
+       struct rvtrace_path *path;
+       cpumask_t *mask;
+       int cpu;
+
+       event_data = container_of(work, struct rvtrace_event_data, work);
+       mask = &event_data->mask;
+       for_each_cpu(cpu, mask) {
+               path = *rvtrace_event_cpu_path_ptr(event_data, cpu);
+               rvtrace_destroy_path(path);
+       }
+       free_percpu(event_data->path);
+       kfree(event_data);
+}
+
+static void *rvtrace_setup_aux(struct perf_event *event, void **pages,
+                              int nr_pages, bool overwrite)
+{
+       struct rvtrace_event_data *event_data = NULL;
+       struct page **pagelist;
+       int cpu = event->cpu, i;
+       cpumask_t *mask;
+
+       event_data = alloc_event_data(cpu);
+       if (!event_data)
+               return NULL;
+
+       INIT_WORK(&event_data->work, free_event_data);
+       mask = &event_data->mask;
+       /*
+        * Create the path for each CPU in the mask. In case of any failure skip the CPU
+        */
+       for_each_cpu(cpu, mask) {
+               struct rvtrace_component *src;
+               struct rvtrace_path *path;
+
+               src = rvtrace_cpu_source(cpu);
+               if (!src)
+                       continue;
+
+               path = rvtrace_create_path(src, NULL, RVTRACE_COMPONENT_MODE_PERF);
+               if (!path)
+                       continue;
+
+               *rvtrace_event_cpu_path_ptr(event_data, cpu) = path;
+       }
+
+       /* If we don't have any CPUs ready for tracing, abort */
+       cpu = cpumask_first(&event_data->mask);
+       if (cpu >= nr_cpu_ids)
+               goto err;
+
+       pagelist = kcalloc(nr_pages, sizeof(*pagelist), GFP_KERNEL);
+       if (!pagelist)
+               goto err;
+
+       for (i = 0; i < nr_pages; i++)
+               pagelist[i] = virt_to_page(pages[i]);
+
+       event_data->buf.base = vmap(pagelist, nr_pages, VM_MAP, PAGE_KERNEL);
+       if (!event_data->buf.base) {
+               kfree(pagelist);
+               goto err;
+       }
+
+       event_data->buf.nr_pages = nr_pages;
+       event_data->buf.length = nr_pages * PAGE_SIZE;
+       event_data->buf.pos = 0;
+       return event_data;
+err:
+       rvtrace_free_aux(event_data);
+       return NULL;
+}
+
+static void rvtrace_event_read(struct perf_event *event)
+{
+}
+
+static void rvtrace_event_destroy(struct perf_event *event)
+{
+}
+
+static int rvtrace_event_init(struct perf_event *event)
+{
+       if (event->attr.type != rvtrace_pmu.type)
+               return -EINVAL;
+
+       event->destroy = rvtrace_event_destroy;
+       return 0;
+}
+
+static void rvtrace_event_start(struct perf_event *event, int flags)
+{
+       struct rvtrace_ctxt *ctxt = this_cpu_ptr(&rvtrace_ctxt);
+       struct perf_output_handle *handle = &ctxt->handle;
+       struct rvtrace_event_data *event_data;
+       int cpu = smp_processor_id();
+       struct rvtrace_path *path;
+
+       if (WARN_ON(ctxt->event_data))
+               goto fail;
+
+       /*
+        * Deal with the ring buffer API and get a handle on the
+        * session's information.
+        */
+       event_data = perf_aux_output_begin(handle, event);
+       if (!event_data)
+               goto fail;
+
+       if (!cpumask_test_cpu(cpu, &event_data->mask))
+               goto out;
+
+       event_data->buf.pos = handle->head % event_data->buf.length;
+       path = *rvtrace_event_cpu_path_ptr(event_data, cpu);
+       if (!path) {
+               pr_err("Error. Path not found\n");
+               return;
+       }
+
+       if (rvtrace_path_start(path)) {
+               pr_err("Error. Tracing not started\n");
+               return;
+       }
+
+       /*
+        * output cpu / trace ID in perf record, once for the lifetime
+        * of the event.
+        */
+       if (!cpumask_test_cpu(cpu, &event_data->aux_hwid_done)) {
+               cpumask_set_cpu(cpu, &event_data->aux_hwid_done);
+               perf_report_aux_output_id(event, cpu);
+       }
+
+out:
+       /* Tell the perf core the event is alive */
+       event->hw.state = 0;
+       ctxt->event_data = event_data;
+       return;
+fail:
+       event->hw.state = PERF_HES_STOPPED;
+}
+
+static void rvtrace_event_stop(struct perf_event *event, int mode)
+{
+       struct rvtrace_ctxt *ctxt = this_cpu_ptr(&rvtrace_ctxt);
+       struct perf_output_handle *handle = &ctxt->handle;
+       struct rvtrace_event_data *event_data;
+       int ret, cpu = smp_processor_id();
+       struct rvtrace_path *path;
+       size_t size;
+
+       if (event->hw.state == PERF_HES_STOPPED)
+               return;
+
+       if (handle->event &&
+           WARN_ON(perf_get_aux(handle) != ctxt->event_data))
+               return;
+
+       event_data = ctxt->event_data;
+       ctxt->event_data = NULL;
+
+       if (WARN_ON(!event_data))
+               return;
+
+       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;
+       }
+
+       /* stop tracing */
+       path = *rvtrace_event_cpu_path_ptr(event_data, cpu);
+       if (!path) {
+               pr_err("Error. Path not found\n");
+               return;
+       }
+
+       if (rvtrace_path_stop(path)) {
+               pr_err("Error. Tracing not stopped\n");
+               return;
+       }
+
+       event->hw.state = PERF_HES_STOPPED;
+       if (handle->event && (mode & PERF_EF_UPDATE)) {
+               if (WARN_ON_ONCE(handle->event != event))
+                       return;
+               spin_lock(&perf_buf_lock);
+               ret = rvtrace_path_copyto_auxbuf(path, &event_data->buf, &size);
+               spin_unlock(&perf_buf_lock);
+               WARN_ON_ONCE(ret);
+               if (READ_ONCE(handle->event))
+                       perf_aux_output_end(handle, size);
+               else
+                       WARN_ON(size);
+       }
+}
+
+static int rvtrace_event_add(struct perf_event *event, int mode)
+{
+       struct hw_perf_event *hwc = &event->hw;
+       int ret = 0;
+
+       if (mode & PERF_EF_START) {
+               rvtrace_event_start(event, 0);
+               if (hwc->state & PERF_HES_STOPPED)
+                       ret = -EINVAL;
+       } else {
+               hwc->state = PERF_HES_STOPPED;
+       }
+
+       return ret;
+}
+
+static void rvtrace_event_del(struct perf_event *event, int mode)
+{
+       rvtrace_event_stop(event, PERF_EF_UPDATE);
+}
+
+PMU_FORMAT_ATTR(event, "config:0-0");
+
+static struct attribute *rvtrace_pmu_formats_attr[] = {
+       &format_attr_event.attr,
+       NULL,
+};
+
+static struct attribute_group rvtrace_pmu_format_group = {
+       .name = "format",
+       .attrs = rvtrace_pmu_formats_attr,
+};
+
+static const struct attribute_group *rvtrace_pmu_attr_groups[] = {
+       &rvtrace_pmu_format_group,
+       NULL,
+};
+
+int __init rvtrace_perf_init(void)
+{
+       rvtrace_pmu.capabilities        = (PERF_PMU_CAP_EXCLUSIVE | PERF_PMU_CAP_ITRACE);
+       rvtrace_pmu.attr_groups         = rvtrace_pmu_attr_groups;
+       rvtrace_pmu.task_ctx_nr         = perf_sw_context;
+       rvtrace_pmu.read                = rvtrace_event_read;
+       rvtrace_pmu.event_init          = rvtrace_event_init;
+       rvtrace_pmu.setup_aux           = rvtrace_setup_aux;
+       rvtrace_pmu.free_aux            = rvtrace_free_aux;
+       rvtrace_pmu.start               = rvtrace_event_start;
+       rvtrace_pmu.stop                = rvtrace_event_stop;
+       rvtrace_pmu.add                 = rvtrace_event_add;
+       rvtrace_pmu.del                 = rvtrace_event_del;
+       rvtrace_pmu.module              = THIS_MODULE;
+
+       return perf_pmu_register(&rvtrace_pmu, RVTRACE_PMU_NAME, -1);
+}
+
+void __exit rvtrace_perf_exit(void)
+{
+       perf_pmu_unregister(&rvtrace_pmu);
+}
include/linux/rvtrace.h +3 -1
index cecf6c153ca6..d5f782ac132f 100644
@@ -343,4 +343,7 @@ static inline int rvtrace_comp_is_empty(struct rvtrace_component *comp)
                                comp->pdata->control_poll_timeout_usecs);
 }
 
+int rvtrace_perf_init(void);
+void rvtrace_perf_exit(void);
+
 #endif
--

2.43.0

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

author
#20 lore raw

PATCH v2 10/12 perf tools: Add RISC-V trace PMU record capabilities

In reply to Anup Patel

From: Mayuresh Chitale <[email protected]>

Introduce the required auxiliary API functions allowing the perf core to interact with RISC-V trace perf driver.

---
 tools/perf/arch/riscv/util/Build      |   1 +
 tools/perf/arch/riscv/util/auxtrace.c | 218 ++++++++++++++++++++++++++
 tools/perf/util/auxtrace.c            |   1 +
 tools/perf/util/auxtrace.h            |   1 +
 tools/perf/util/rvtrace.h             |  18 +++
 5 files changed, 239 insertions(+)
 create mode 100644 tools/perf/arch/riscv/util/auxtrace.c
 create mode 100644 tools/perf/util/rvtrace.h
tools/perf/arch/riscv/util/Build +1 -0
index 58a672246024..2ba1fb98a0ad 100644
@@ -3,3 +3,4 @@ perf-util-y += header.o
 
 perf-util-$(CONFIG_LIBTRACEEVENT) += kvm-stat.o
 perf-util-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
+perf-util-$(CONFIG_AUXTRACE) += auxtrace.o
tools/perf/arch/riscv/util/auxtrace.c new file +218 -0

218 lines not on this page · raw message

tools/perf/util/auxtrace.c +1 -0
index 1539c1dc823c..c905563e0d8a 100644
@@ -1394,6 +1394,7 @@ int perf_event__process_auxtrace_info(struct perf_session *session,
 	case PERF_AUXTRACE_VPA_DTL:
 		err = powerpc_vpadtl_process_auxtrace_info(event, session);
 		break;
+	case PERF_AUXTRACE_RISCV_TRACE:
 	case PERF_AUXTRACE_UNKNOWN:
 	default:
 		return -EINVAL;
tools/perf/util/auxtrace.h +1 -0
index e0a5b39fed12..1c89a281a06c 100644
@@ -51,6 +51,7 @@ enum auxtrace_type {
 	PERF_AUXTRACE_S390_CPUMSF,
 	PERF_AUXTRACE_HISI_PTT,
 	PERF_AUXTRACE_VPA_DTL,
+	PERF_AUXTRACE_RISCV_TRACE,
 };
 
 enum itrace_period_type {
tools/perf/util/rvtrace.h new file +18 -0
new file mode 100644
index 000000000000..93c041db8660
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright(C) 2015 Linaro Limited. All rights reserved.
+ * Author: Mathieu Poirier <[email protected]>
+ */
+
+#ifndef INCLUDE__UTIL_PERF_RVTRACE_H__
+#define INCLUDE__UTIL_PERF_RVTRACE_H__
+
+#include "debug.h"
+#include "auxtrace.h"
+#include "util/event.h"
+#include "util/session.h"
+#include <linux/bits.h>
+
+#define RVTRACE_AUXTRACE_PRIV_SIZE	sizeof(u64)
+
+#endif
Signature · 1 line
2.43.0
Reviewed-by on v2 10/12
#21 lore raw

Re: [PATCH v2 10/12] perf tools: Add RISC-V trace PMU record capabilities

Quoted 36 lines · Anup Patel

From: Mayuresh Chitale <[email protected]>

Introduce the required auxiliary API functions allowing the perf core to interact with RISC-V trace perf driver.

---
  tools/perf/arch/riscv/util/Build      |   1 +
  tools/perf/arch/riscv/util/auxtrace.c | 218 ++++++++++++++++++++++++++
  tools/perf/util/auxtrace.c            |   1 +
  tools/perf/util/auxtrace.h            |   1 +
  tools/perf/util/rvtrace.h             |  18 +++
  5 files changed, 239 insertions(+)

create mode 100644 tools/perf/arch/riscv/util/auxtrace.c create mode 100644 tools/perf/util/rvtrace.h

tools/perf/arch/riscv/util/Build +1 -0
index 58a672246024..2ba1fb98a0ad 100644
@@ -3,3 +3,4 @@ perf-util-y += header.o
 
  perf-util-$(CONFIG_LIBTRACEEVENT) += kvm-stat.o
  perf-util-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
+perf-util-$(CONFIG_AUXTRACE) += auxtrace.o
tools/perf/arch/riscv/util/auxtrace.c new file +3 -0
new file mode 100644
index 000000000000..adc86bd38998
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Risc-V E-Trace support

tools/perf/arch/riscv/util/auxtrace.c line 13

Quoted 10 lines
tools/perf/arch/riscv/util/auxtrace.c new file +10 -0
+ */
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/bitops.h>
+#include <linux/log2.h>
+#include <linux/zalloc.h>
+#include <time.h>
+
+#include <internal/lib.h> // page_size

Seems the above comment 'page_size' is redundant.

Quoted 136 lines
tools/perf/arch/riscv/util/auxtrace.c new file +136 -0
+#include "../../../util/auxtrace.h"
+#include "../../../util/cpumap.h"
+#include "../../../util/debug.h"
+#include "../../../util/event.h"
+#include "../../../util/evlist.h"
+#include "../../../util/evsel.h"
+#include "../../../util/rvtrace.h"
+#include "../../../util/pmu.h"
+#include "../../../util/record.h"
+#include "../../../util/session.h"
+#include "../../../util/tsc.h"
+
+#define RVTRACE_PMU_NAME "rvtrace"
+#define KiB(x) ((x) * 1024)
+#define MiB(x) ((x) * 1024 * 1024)
+
+struct rvtrace_recording {
+	struct auxtrace_record	itr;
+	struct perf_pmu *rvtrace_pmu;
+	struct evlist *evlist;
+};
+
+static size_t rvtrace_info_priv_size(struct auxtrace_record *itr __maybe_unused,
+				     struct evlist *evlist __maybe_unused)
+{
+	return RVTRACE_AUXTRACE_PRIV_SIZE;
+}
+
+static int rvtrace_info_fill(struct auxtrace_record *itr, struct perf_session *session,
+			     struct perf_record_auxtrace_info *auxtrace_info, size_t priv_size)
+{
+	struct rvtrace_recording *ptr = container_of(itr, struct rvtrace_recording, itr);
+	struct perf_pmu *rvtrace_pmu = ptr->rvtrace_pmu;
+
+	if (priv_size != RVTRACE_AUXTRACE_PRIV_SIZE)
+		return -EINVAL;
+
+	if (!session->evlist->core.nr_mmaps)
+		return -EINVAL;
+
+	auxtrace_info->type = PERF_AUXTRACE_RISCV_TRACE;
+	auxtrace_info->priv[0] = rvtrace_pmu->type;
+
+	return 0;
+}
+
+static int rvtrace_set_auxtrace_mmap_page(struct record_opts *opts)
+{
+	bool privileged = perf_event_paranoid_check(-1);
+
+	if (!opts->full_auxtrace)
+		return 0;
+
+	if (opts->full_auxtrace && !opts->auxtrace_mmap_pages) {
+		if (privileged) {
+			opts->auxtrace_mmap_pages = MiB(16) / page_size;
+		} else {
+			opts->auxtrace_mmap_pages = KiB(128) / page_size;
+			if (opts->mmap_pages == UINT_MAX)
+				opts->mmap_pages = KiB(256) / page_size;
+		}
+	}
+
+	/* Validate auxtrace_mmap_pages */
+	if (opts->auxtrace_mmap_pages) {
+		size_t sz = opts->auxtrace_mmap_pages * (size_t)page_size;
+		size_t min_sz = KiB(8);
+
+		if (sz < min_sz || !is_power_of_2(sz)) {
+			pr_err("Invalid mmap size : must be at least %zuKiB and a power of 2\n",
+			       min_sz / 1024);
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static int rvtrace_recording_options(struct auxtrace_record *itr, struct evlist *evlist,
+				     struct record_opts *opts)
+{
+	struct rvtrace_recording *ptr = container_of(itr, struct rvtrace_recording, itr);
+	struct perf_pmu *rvtrace_pmu = ptr->rvtrace_pmu;
+	struct evsel *evsel, *rvtrace_evsel = NULL;
+	struct evsel *tracking_evsel;
+	int err;
+
+	ptr->evlist = evlist;
+	evlist__for_each_entry(evlist, evsel) {
+		if (evsel->core.attr.type == rvtrace_pmu->type) {
+			if (rvtrace_evsel) {
+				pr_err("There may be only one " RVTRACE_PMU_NAME "x event\n");
+				return -EINVAL;
+			}
+			evsel->core.attr.freq = 0;
+			evsel->core.attr.sample_period = 1;
+			evsel->needs_auxtrace_mmap = true;
+			rvtrace_evsel = evsel;
+			opts->full_auxtrace = true;
+		}
+	}
+
+	err = rvtrace_set_auxtrace_mmap_page(opts);
+	if (err)
+		return err;
+	/*
+	 * To obtain the auxtrace buffer file descriptor, the auxtrace event
+	 * must come first.
+	 */
+	evlist__to_front(evlist, rvtrace_evsel);
+	evsel__set_sample_bit(rvtrace_evsel, TIME);
+
+	/* Add dummy event to keep tracking */
+	err = parse_event(evlist, "dummy:u");
+	if (err)
+		return err;
+
+	tracking_evsel = evlist__last(evlist);
+	evlist__set_tracking_event(evlist, tracking_evsel);
+
+	tracking_evsel->core.attr.freq = 0;
+	tracking_evsel->core.attr.sample_period = 1;
+	evsel__set_sample_bit(tracking_evsel, TIME);
+
+	return 0;
+}
+
+static u64 rvtrace_reference(struct auxtrace_record *itr __maybe_unused)
+{
+	return rdtsc();
+}
+
+static void rvtrace_recording_free(struct auxtrace_record *itr)
+{
+	struct rvtrace_recording *ptr =
+			container_of(itr, struct rvtrace_recording, itr);

tools/perf/arch/riscv/util/auxtrace.c line 159

Quoted 10 lines
tools/perf/arch/riscv/util/auxtrace.c new file +10 -0
+
+	free(ptr);
+}
+
+static struct auxtrace_record *rvtrace_recording_init(int *err, struct perf_pmu *rvtrace_pmu)
+{
+	struct rvtrace_recording *ptr;
+
+	if (!rvtrace_pmu) {
+		*err = -ENODEV;

Nit: How about checking if 'err' is NULL since it's passed from other module ?

Otherwise,

Thanks,
Nutty

author
#22 lore raw

Re: [PATCH v2 10/12] perf tools: Add RISC-V trace PMU record capabilities

Quoted 41 lines · Nutty.Liu
Quoted 39 lines · Anup Patel

From: Mayuresh Chitale <[email protected]>

Introduce the required auxiliary API functions allowing the perf core to interact with RISC-V trace perf driver.

---
  tools/perf/arch/riscv/util/Build      |   1 +
  tools/perf/arch/riscv/util/auxtrace.c | 218 ++++++++++++++++++++++++++
  tools/perf/util/auxtrace.c            |   1 +
  tools/perf/util/auxtrace.h            |   1 +
  tools/perf/util/rvtrace.h             |  18 +++
  5 files changed, 239 insertions(+)

create mode 100644 tools/perf/arch/riscv/util/auxtrace.c create mode 100644 tools/perf/util/rvtrace.h

tools/perf/arch/riscv/util/Build +1 -0
index 58a672246024..2ba1fb98a0ad 100644
@@ -3,3 +3,4 @@ perf-util-y += header.o
 
  perf-util-$(CONFIG_LIBTRACEEVENT) += kvm-stat.o
  perf-util-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
+perf-util-$(CONFIG_AUXTRACE) += auxtrace.o
tools/perf/arch/riscv/util/auxtrace.c new file +6 -0
new file mode 100644
index 000000000000..adc86bd38998
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Risc-V E-Trace support
+ */
+
+#include <linux/kernel.h>
Quoted 8 lines

tools/perf/arch/riscv/util/auxtrace.c line 13

Quoted 7 lines
tools/perf/arch/riscv/util/auxtrace.c new file +7 -0
+#include <linux/types.h>
+#include <linux/bitops.h>
+#include <linux/log2.h>
+#include <linux/zalloc.h>
+#include <time.h>
+
+#include <internal/lib.h> // page_size

Seems the above comment 'page_size' is redundant.

Okay, I will update.

Quoted 141 lines
Quoted 141 lines
tools/perf/arch/riscv/util/auxtrace.c new file +140 -0
+#include "../../../util/auxtrace.h"
+#include "../../../util/cpumap.h"
+#include "../../../util/debug.h"
+#include "../../../util/event.h"
+#include "../../../util/evlist.h"
+#include "../../../util/evsel.h"
+#include "../../../util/rvtrace.h"
+#include "../../../util/pmu.h"
+#include "../../../util/record.h"
+#include "../../../util/session.h"
+#include "../../../util/tsc.h"
+
+#define RVTRACE_PMU_NAME "rvtrace"
+#define KiB(x) ((x) * 1024)
+#define MiB(x) ((x) * 1024 * 1024)
+
+struct rvtrace_recording {
+     struct auxtrace_record  itr;
+     struct perf_pmu *rvtrace_pmu;
+     struct evlist *evlist;
+};
+
+static size_t rvtrace_info_priv_size(struct auxtrace_record *itr __maybe_unused,
+                                  struct evlist *evlist __maybe_unused)
+{
+     return RVTRACE_AUXTRACE_PRIV_SIZE;
+}
+
+static int rvtrace_info_fill(struct auxtrace_record *itr, struct perf_session *session,
+                          struct perf_record_auxtrace_info *auxtrace_info, size_t priv_size)
+{
+     struct rvtrace_recording *ptr = container_of(itr, struct rvtrace_recording, itr);
+     struct perf_pmu *rvtrace_pmu = ptr->rvtrace_pmu;
+
+     if (priv_size != RVTRACE_AUXTRACE_PRIV_SIZE)
+             return -EINVAL;
+
+     if (!session->evlist->core.nr_mmaps)
+             return -EINVAL;
+
+     auxtrace_info->type = PERF_AUXTRACE_RISCV_TRACE;
+     auxtrace_info->priv[0] = rvtrace_pmu->type;
+
+     return 0;
+}
+
+static int rvtrace_set_auxtrace_mmap_page(struct record_opts *opts)
+{
+     bool privileged = perf_event_paranoid_check(-1);
+
+     if (!opts->full_auxtrace)
+             return 0;
+
+     if (opts->full_auxtrace && !opts->auxtrace_mmap_pages) {
+             if (privileged) {
+                     opts->auxtrace_mmap_pages = MiB(16) / page_size;
+             } else {
+                     opts->auxtrace_mmap_pages = KiB(128) / page_size;
+                     if (opts->mmap_pages == UINT_MAX)
+                             opts->mmap_pages = KiB(256) / page_size;
+             }
+     }
+
+     /* Validate auxtrace_mmap_pages */
+     if (opts->auxtrace_mmap_pages) {
+             size_t sz = opts->auxtrace_mmap_pages * (size_t)page_size;
+             size_t min_sz = KiB(8);
+
+             if (sz < min_sz || !is_power_of_2(sz)) {
+                     pr_err("Invalid mmap size : must be at least %zuKiB and a power of 2\n",
+                            min_sz / 1024);
+                     return -EINVAL;
+             }
+     }
+
+     return 0;
+}
+
+static int rvtrace_recording_options(struct auxtrace_record *itr, struct evlist *evlist,
+                                  struct record_opts *opts)
+{
+     struct rvtrace_recording *ptr = container_of(itr, struct rvtrace_recording, itr);
+     struct perf_pmu *rvtrace_pmu = ptr->rvtrace_pmu;
+     struct evsel *evsel, *rvtrace_evsel = NULL;
+     struct evsel *tracking_evsel;
+     int err;
+
+     ptr->evlist = evlist;
+     evlist__for_each_entry(evlist, evsel) {
+             if (evsel->core.attr.type == rvtrace_pmu->type) {
+                     if (rvtrace_evsel) {
+                             pr_err("There may be only one " RVTRACE_PMU_NAME "x event\n");
+                             return -EINVAL;
+                     }
+                     evsel->core.attr.freq = 0;
+                     evsel->core.attr.sample_period = 1;
+                     evsel->needs_auxtrace_mmap = true;
+                     rvtrace_evsel = evsel;
+                     opts->full_auxtrace = true;
+             }
+     }
+
+     err = rvtrace_set_auxtrace_mmap_page(opts);
+     if (err)
+             return err;
+     /*
+      * To obtain the auxtrace buffer file descriptor, the auxtrace event
+      * must come first.
+      */
+     evlist__to_front(evlist, rvtrace_evsel);
+     evsel__set_sample_bit(rvtrace_evsel, TIME);
+
+     /* Add dummy event to keep tracking */
+     err = parse_event(evlist, "dummy:u");
+     if (err)
+             return err;
+
+     tracking_evsel = evlist__last(evlist);
+     evlist__set_tracking_event(evlist, tracking_evsel);
+
+     tracking_evsel->core.attr.freq = 0;
+     tracking_evsel->core.attr.sample_period = 1;
+     evsel__set_sample_bit(tracking_evsel, TIME);
+
+     return 0;
+}
+
+static u64 rvtrace_reference(struct auxtrace_record *itr __maybe_unused)
+{
+     return rdtsc();
+}
+
+static void rvtrace_recording_free(struct auxtrace_record *itr)
+{
+     struct rvtrace_recording *ptr =
+                     container_of(itr, struct rvtrace_recording, itr);
+
+     free(ptr);
+}
+
Quoted 8 lines

tools/perf/arch/riscv/util/auxtrace.c line 159

Quoted 6 lines
tools/perf/arch/riscv/util/auxtrace.c new file +6 -0
+static struct auxtrace_record *rvtrace_recording_init(int *err, struct perf_pmu *rvtrace_pmu)
+{
+     struct rvtrace_recording *ptr;
+
+     if (!rvtrace_pmu) {
+             *err = -ENODEV;

Nit: How about checking if 'err' is NULL since it's passed from other module ?

Other archs (ARM and x86) also don't check "err" anywhere so to be consistent we don't check "err" here.

Quoted 6 lines

Otherwise,

Thanks,
Nutty

Regards,
Anup

#23 lore raw

Re: [PATCH v2 10/12] perf tools: Add RISC-V trace PMU record capabilities

In reply to Anup Patel

Hi Anup,

Quoted 225 lines · Anup Patel

From: Mayuresh Chitale <[email protected]>

Introduce the required auxiliary API functions allowing the perf core to interact with RISC-V trace perf driver.

---
 tools/perf/arch/riscv/util/Build      |   1 +
 tools/perf/arch/riscv/util/auxtrace.c | 218 ++++++++++++++++++++++++++
 tools/perf/util/auxtrace.c            |   1 +
 tools/perf/util/auxtrace.h            |   1 +
 tools/perf/util/rvtrace.h             |  18 +++
 5 files changed, 239 insertions(+)
 create mode 100644 tools/perf/arch/riscv/util/auxtrace.c
 create mode 100644 tools/perf/util/rvtrace.h
tools/perf/arch/riscv/util/Build +1 -0
index 58a672246024..2ba1fb98a0ad 100644
@@ -3,3 +3,4 @@ perf-util-y += header.o
 
 perf-util-$(CONFIG_LIBTRACEEVENT) += kvm-stat.o
 perf-util-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
+perf-util-$(CONFIG_AUXTRACE) += auxtrace.o
tools/perf/arch/riscv/util/auxtrace.c new file +191 -0
new file mode 100644
index 000000000000..adc86bd38998
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Risc-V E-Trace support
+ */
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/bitops.h>
+#include <linux/log2.h>
+#include <linux/zalloc.h>
+#include <time.h>
+
+#include <internal/lib.h> // page_size
+#include "../../../util/auxtrace.h"
+#include "../../../util/cpumap.h"
+#include "../../../util/debug.h"
+#include "../../../util/event.h"
+#include "../../../util/evlist.h"
+#include "../../../util/evsel.h"
+#include "../../../util/rvtrace.h"
+#include "../../../util/pmu.h"
+#include "../../../util/record.h"
+#include "../../../util/session.h"
+#include "../../../util/tsc.h"
+
+#define RVTRACE_PMU_NAME "rvtrace"
+#define KiB(x) ((x) * 1024)
+#define MiB(x) ((x) * 1024 * 1024)
+
+struct rvtrace_recording {
+       struct auxtrace_record  itr;
+       struct perf_pmu *rvtrace_pmu;
+       struct evlist *evlist;
+};
+
+static size_t rvtrace_info_priv_size(struct auxtrace_record *itr __maybe_unused,
+                                    struct evlist *evlist __maybe_unused)
+{
+       return RVTRACE_AUXTRACE_PRIV_SIZE;
+}
+
+static int rvtrace_info_fill(struct auxtrace_record *itr, struct perf_session *session,
+                            struct perf_record_auxtrace_info *auxtrace_info, size_t priv_size)
+{
+       struct rvtrace_recording *ptr = container_of(itr, struct rvtrace_recording, itr);
+       struct perf_pmu *rvtrace_pmu = ptr->rvtrace_pmu;
+
+       if (priv_size != RVTRACE_AUXTRACE_PRIV_SIZE)
+               return -EINVAL;
+
+       if (!session->evlist->core.nr_mmaps)
+               return -EINVAL;
+
+       auxtrace_info->type = PERF_AUXTRACE_RISCV_TRACE;
+       auxtrace_info->priv[0] = rvtrace_pmu->type;
+
+       return 0;
+}
+
+static int rvtrace_set_auxtrace_mmap_page(struct record_opts *opts)
+{
+       bool privileged = perf_event_paranoid_check(-1);
+
+       if (!opts->full_auxtrace)
+               return 0;
+
+       if (opts->full_auxtrace && !opts->auxtrace_mmap_pages) {
+               if (privileged) {
+                       opts->auxtrace_mmap_pages = MiB(16) / page_size;
+               } else {
+                       opts->auxtrace_mmap_pages = KiB(128) / page_size;
+                       if (opts->mmap_pages == UINT_MAX)
+                               opts->mmap_pages = KiB(256) / page_size;
+               }
+       }
+
+       /* Validate auxtrace_mmap_pages */
+       if (opts->auxtrace_mmap_pages) {
+               size_t sz = opts->auxtrace_mmap_pages * (size_t)page_size;
+               size_t min_sz = KiB(8);
+
+               if (sz < min_sz || !is_power_of_2(sz)) {
+                       pr_err("Invalid mmap size : must be at least %zuKiB and a power of 2\n",
+                              min_sz / 1024);
+                       return -EINVAL;
+               }
+       }
+
+       return 0;
+}
+
+static int rvtrace_recording_options(struct auxtrace_record *itr, struct evlist *evlist,
+                                    struct record_opts *opts)
+{
+       struct rvtrace_recording *ptr = container_of(itr, struct rvtrace_recording, itr);
+       struct perf_pmu *rvtrace_pmu = ptr->rvtrace_pmu;
+       struct evsel *evsel, *rvtrace_evsel = NULL;
+       struct evsel *tracking_evsel;
+       int err;
+
+       ptr->evlist = evlist;
+       evlist__for_each_entry(evlist, evsel) {
+               if (evsel->core.attr.type == rvtrace_pmu->type) {
+                       if (rvtrace_evsel) {
+                               pr_err("There may be only one " RVTRACE_PMU_NAME "x event\n");
+                               return -EINVAL;
+                       }
+                       evsel->core.attr.freq = 0;
+                       evsel->core.attr.sample_period = 1;
+                       evsel->needs_auxtrace_mmap = true;
+                       rvtrace_evsel = evsel;
+                       opts->full_auxtrace = true;
+               }
+       }
+
+       err = rvtrace_set_auxtrace_mmap_page(opts);
+       if (err)
+               return err;
+       /*
+        * To obtain the auxtrace buffer file descriptor, the auxtrace event
+        * must come first.
+        */
+       evlist__to_front(evlist, rvtrace_evsel);
+       evsel__set_sample_bit(rvtrace_evsel, TIME);
+
+       /* Add dummy event to keep tracking */
+       err = parse_event(evlist, "dummy:u");
+       if (err)
+               return err;
+
+       tracking_evsel = evlist__last(evlist);
+       evlist__set_tracking_event(evlist, tracking_evsel);
+
+       tracking_evsel->core.attr.freq = 0;
+       tracking_evsel->core.attr.sample_period = 1;
+       evsel__set_sample_bit(tracking_evsel, TIME);
+
+       return 0;
+}
+
+static u64 rvtrace_reference(struct auxtrace_record *itr __maybe_unused)
+{
+       return rdtsc();
+}
+
+static void rvtrace_recording_free(struct auxtrace_record *itr)
+{
+       struct rvtrace_recording *ptr =
+                       container_of(itr, struct rvtrace_recording, itr);
+
+       free(ptr);
+}
+
+static struct auxtrace_record *rvtrace_recording_init(int *err, struct perf_pmu *rvtrace_pmu)
+{
+       struct rvtrace_recording *ptr;
+
+       if (!rvtrace_pmu) {
+               *err = -ENODEV;
+               return NULL;
+       }
+
+       ptr = zalloc(sizeof(*ptr));
+       if (!ptr) {
+               *err = -ENOMEM;
+               return NULL;
+       }
+
+       ptr->rvtrace_pmu = rvtrace_pmu;
+       ptr->itr.recording_options = rvtrace_recording_options;
+       ptr->itr.info_priv_size = rvtrace_info_priv_size;
+       ptr->itr.info_fill = rvtrace_info_fill;
+       ptr->itr.free = rvtrace_recording_free;
+       ptr->itr.reference = rvtrace_reference;
+       ptr->itr.read_finish = auxtrace_record__read_finish;
+       ptr->itr.alignment = 0;
+
+       *err = 0;
+       return &ptr->itr;
+}
+
+static struct perf_pmu *find_pmu_for_event(struct perf_pmu **pmus,
+                                          int pmu_nr, struct evsel *evsel)
+{
+       int i;
+
+       if (!pmus)
+               return NULL;
+
+       for (i = 0; i < pmu_nr; i++) {
+               if (evsel->core.attr.type == pmus[i]->type)

tools/perf/arch/riscv/util/auxtrace.c line 201

Quoted 10 lines
tools/perf/arch/riscv/util/auxtrace.c new file +10 -0
+                       return pmus[i];
+       }
+
+       return NULL;
+}
+
+struct auxtrace_record *auxtrace_record__init(struct evlist *evlist, int *err)
+{
+       struct perf_pmu *rvtrace_pmu = NULL;
+       struct perf_pmu *found_etm = NULL;
                                        ^^^^^^^^^

Nit: it seems "found_etm" implies ARM CoreSight ETM. Since this is for RISC-V, should we rename this to "found_rvtrace" or simply "found_pmu" to avoid confusion? Thanks

Regards,
Eric Lin

Quoted 73 lines
tools/perf/arch/riscv/util/auxtrace.c new file +17 -0
+       struct evsel *evsel;
+
+       if (!evlist)
+               return NULL;
+
+       rvtrace_pmu = perf_pmus__find(RVTRACE_PMU_NAME);
+       evlist__for_each_entry(evlist, evsel) {
+               if (rvtrace_pmu && !found_etm)
+                       found_etm = find_pmu_for_event(&rvtrace_pmu, 1, evsel);
+       }
+
+       if (found_etm)
+               return rvtrace_recording_init(err, rvtrace_pmu);
+
+       *err = 0;
+       return NULL;
+}
tools/perf/util/auxtrace.c +1 -0
index 1539c1dc823c..c905563e0d8a 100644
@@ -1394,6 +1394,7 @@ int perf_event__process_auxtrace_info(struct perf_session *session,
        case PERF_AUXTRACE_VPA_DTL:
                err = powerpc_vpadtl_process_auxtrace_info(event, session);
                break;
+       case PERF_AUXTRACE_RISCV_TRACE:
        case PERF_AUXTRACE_UNKNOWN:
        default:
                return -EINVAL;
tools/perf/util/auxtrace.h +1 -0
index e0a5b39fed12..1c89a281a06c 100644
@@ -51,6 +51,7 @@ enum auxtrace_type {
        PERF_AUXTRACE_S390_CPUMSF,
        PERF_AUXTRACE_HISI_PTT,
        PERF_AUXTRACE_VPA_DTL,
+       PERF_AUXTRACE_RISCV_TRACE,
 };
 
 enum itrace_period_type {
tools/perf/util/rvtrace.h new file +18 -1
new file mode 100644
index 000000000000..93c041db8660
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright(C) 2015 Linaro Limited. All rights reserved.
+ * Author: Mathieu Poirier <[email protected]>
+ */
+
+#ifndef INCLUDE__UTIL_PERF_RVTRACE_H__
+#define INCLUDE__UTIL_PERF_RVTRACE_H__
+
+#include "debug.h"
+#include "auxtrace.h"
+#include "util/event.h"
+#include "util/session.h"
+#include <linux/bits.h>
+
+#define RVTRACE_AUXTRACE_PRIV_SIZE     sizeof(u64)
+
+#endif
--

2.43.0

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

#24 lore raw

Re: [PATCH v2 10/12] perf tools: Add RISC-V trace PMU record capabilities

Hi Eric,

Quoted 240 lines · Eric Lin

Hi Anup,

Quoted 225 lines · Anup Patel

From: Mayuresh Chitale <[email protected]>

Introduce the required auxiliary API functions allowing the perf core to interact with RISC-V trace perf driver.

---
 tools/perf/arch/riscv/util/Build      |   1 +
 tools/perf/arch/riscv/util/auxtrace.c | 218 ++++++++++++++++++++++++++
 tools/perf/util/auxtrace.c            |   1 +
 tools/perf/util/auxtrace.h            |   1 +
 tools/perf/util/rvtrace.h             |  18 +++
 5 files changed, 239 insertions(+)
 create mode 100644 tools/perf/arch/riscv/util/auxtrace.c
 create mode 100644 tools/perf/util/rvtrace.h
tools/perf/arch/riscv/util/Build +1 -0
index 58a672246024..2ba1fb98a0ad 100644
@@ -3,3 +3,4 @@ perf-util-y += header.o
 
 perf-util-$(CONFIG_LIBTRACEEVENT) += kvm-stat.o
 perf-util-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
+perf-util-$(CONFIG_AUXTRACE) += auxtrace.o
tools/perf/arch/riscv/util/auxtrace.c new file +191 -0
new file mode 100644
index 000000000000..adc86bd38998
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Risc-V E-Trace support
+ */
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/bitops.h>
+#include <linux/log2.h>
+#include <linux/zalloc.h>
+#include <time.h>
+
+#include <internal/lib.h> // page_size
+#include "../../../util/auxtrace.h"
+#include "../../../util/cpumap.h"
+#include "../../../util/debug.h"
+#include "../../../util/event.h"
+#include "../../../util/evlist.h"
+#include "../../../util/evsel.h"
+#include "../../../util/rvtrace.h"
+#include "../../../util/pmu.h"
+#include "../../../util/record.h"
+#include "../../../util/session.h"
+#include "../../../util/tsc.h"
+
+#define RVTRACE_PMU_NAME "rvtrace"
+#define KiB(x) ((x) * 1024)
+#define MiB(x) ((x) * 1024 * 1024)
+
+struct rvtrace_recording {
+       struct auxtrace_record  itr;
+       struct perf_pmu *rvtrace_pmu;
+       struct evlist *evlist;
+};
+
+static size_t rvtrace_info_priv_size(struct auxtrace_record *itr __maybe_unused,
+                                    struct evlist *evlist __maybe_unused)
+{
+       return RVTRACE_AUXTRACE_PRIV_SIZE;
+}
+
+static int rvtrace_info_fill(struct auxtrace_record *itr, struct perf_session *session,
+                            struct perf_record_auxtrace_info *auxtrace_info, size_t priv_size)
+{
+       struct rvtrace_recording *ptr = container_of(itr, struct rvtrace_recording, itr);
+       struct perf_pmu *rvtrace_pmu = ptr->rvtrace_pmu;
+
+       if (priv_size != RVTRACE_AUXTRACE_PRIV_SIZE)
+               return -EINVAL;
+
+       if (!session->evlist->core.nr_mmaps)
+               return -EINVAL;
+
+       auxtrace_info->type = PERF_AUXTRACE_RISCV_TRACE;
+       auxtrace_info->priv[0] = rvtrace_pmu->type;
+
+       return 0;
+}
+
+static int rvtrace_set_auxtrace_mmap_page(struct record_opts *opts)
+{
+       bool privileged = perf_event_paranoid_check(-1);
+
+       if (!opts->full_auxtrace)
+               return 0;
+
+       if (opts->full_auxtrace && !opts->auxtrace_mmap_pages) {
+               if (privileged) {
+                       opts->auxtrace_mmap_pages = MiB(16) / page_size;
+               } else {
+                       opts->auxtrace_mmap_pages = KiB(128) / page_size;
+                       if (opts->mmap_pages == UINT_MAX)
+                               opts->mmap_pages = KiB(256) / page_size;
+               }
+       }
+
+       /* Validate auxtrace_mmap_pages */
+       if (opts->auxtrace_mmap_pages) {
+               size_t sz = opts->auxtrace_mmap_pages * (size_t)page_size;
+               size_t min_sz = KiB(8);
+
+               if (sz < min_sz || !is_power_of_2(sz)) {
+                       pr_err("Invalid mmap size : must be at least %zuKiB and a power of 2\n",
+                              min_sz / 1024);
+                       return -EINVAL;
+               }
+       }
+
+       return 0;
+}
+
+static int rvtrace_recording_options(struct auxtrace_record *itr, struct evlist *evlist,
+                                    struct record_opts *opts)
+{
+       struct rvtrace_recording *ptr = container_of(itr, struct rvtrace_recording, itr);
+       struct perf_pmu *rvtrace_pmu = ptr->rvtrace_pmu;
+       struct evsel *evsel, *rvtrace_evsel = NULL;
+       struct evsel *tracking_evsel;
+       int err;
+
+       ptr->evlist = evlist;
+       evlist__for_each_entry(evlist, evsel) {
+               if (evsel->core.attr.type == rvtrace_pmu->type) {
+                       if (rvtrace_evsel) {
+                               pr_err("There may be only one " RVTRACE_PMU_NAME "x event\n");
+                               return -EINVAL;
+                       }
+                       evsel->core.attr.freq = 0;
+                       evsel->core.attr.sample_period = 1;
+                       evsel->needs_auxtrace_mmap = true;
+                       rvtrace_evsel = evsel;
+                       opts->full_auxtrace = true;
+               }
+       }
+
+       err = rvtrace_set_auxtrace_mmap_page(opts);
+       if (err)
+               return err;
+       /*
+        * To obtain the auxtrace buffer file descriptor, the auxtrace event
+        * must come first.
+        */
+       evlist__to_front(evlist, rvtrace_evsel);
+       evsel__set_sample_bit(rvtrace_evsel, TIME);
+
+       /* Add dummy event to keep tracking */
+       err = parse_event(evlist, "dummy:u");
+       if (err)
+               return err;
+
+       tracking_evsel = evlist__last(evlist);
+       evlist__set_tracking_event(evlist, tracking_evsel);
+
+       tracking_evsel->core.attr.freq = 0;
+       tracking_evsel->core.attr.sample_period = 1;
+       evsel__set_sample_bit(tracking_evsel, TIME);
+
+       return 0;
+}
+
+static u64 rvtrace_reference(struct auxtrace_record *itr __maybe_unused)
+{
+       return rdtsc();
+}
+
+static void rvtrace_recording_free(struct auxtrace_record *itr)
+{
+       struct rvtrace_recording *ptr =
+                       container_of(itr, struct rvtrace_recording, itr);
+
+       free(ptr);
+}
+
+static struct auxtrace_record *rvtrace_recording_init(int *err, struct perf_pmu *rvtrace_pmu)
+{
+       struct rvtrace_recording *ptr;
+
+       if (!rvtrace_pmu) {
+               *err = -ENODEV;
+               return NULL;
+       }
+
+       ptr = zalloc(sizeof(*ptr));
+       if (!ptr) {
+               *err = -ENOMEM;
+               return NULL;
+       }
+
+       ptr->rvtrace_pmu = rvtrace_pmu;
+       ptr->itr.recording_options = rvtrace_recording_options;
+       ptr->itr.info_priv_size = rvtrace_info_priv_size;
+       ptr->itr.info_fill = rvtrace_info_fill;
+       ptr->itr.free = rvtrace_recording_free;
+       ptr->itr.reference = rvtrace_reference;
+       ptr->itr.read_finish = auxtrace_record__read_finish;
+       ptr->itr.alignment = 0;
+
+       *err = 0;
+       return &ptr->itr;
+}
+
+static struct perf_pmu *find_pmu_for_event(struct perf_pmu **pmus,
+                                          int pmu_nr, struct evsel *evsel)
+{
+       int i;
+
+       if (!pmus)
+               return NULL;
+
+       for (i = 0; i < pmu_nr; i++) {
+               if (evsel->core.attr.type == pmus[i]->type)

tools/perf/arch/riscv/util/auxtrace.c line 201

Quoted 10 lines
tools/perf/arch/riscv/util/auxtrace.c new file +10 -0
+                       return pmus[i];
+       }
+
+       return NULL;
+}
+
+struct auxtrace_record *auxtrace_record__init(struct evlist *evlist, int *err)
+{
+       struct perf_pmu *rvtrace_pmu = NULL;
+       struct perf_pmu *found_etm = NULL;
                                        ^^^^^^^^^
Quoted 3 lines

Nit: it seems "found_etm" implies ARM CoreSight ETM. Since this is for RISC-V, should we rename this to "found_rvtrace" or simply "found_pmu" to avoid confusion? Thanks

Yes, this should be renamed as found_pmu.

Quoted 77 lines

Regards,
Eric Lin

Quoted 73 lines
tools/perf/arch/riscv/util/auxtrace.c new file +17 -0
+       struct evsel *evsel;
+
+       if (!evlist)
+               return NULL;
+
+       rvtrace_pmu = perf_pmus__find(RVTRACE_PMU_NAME);
+       evlist__for_each_entry(evlist, evsel) {
+               if (rvtrace_pmu && !found_etm)
+                       found_etm = find_pmu_for_event(&rvtrace_pmu, 1, evsel);
+       }
+
+       if (found_etm)
+               return rvtrace_recording_init(err, rvtrace_pmu);
+
+       *err = 0;
+       return NULL;
+}
tools/perf/util/auxtrace.c +1 -0
index 1539c1dc823c..c905563e0d8a 100644
@@ -1394,6 +1394,7 @@ int perf_event__process_auxtrace_info(struct perf_session *session,
        case PERF_AUXTRACE_VPA_DTL:
                err = powerpc_vpadtl_process_auxtrace_info(event, session);
                break;
+       case PERF_AUXTRACE_RISCV_TRACE:
        case PERF_AUXTRACE_UNKNOWN:
        default:
                return -EINVAL;
tools/perf/util/auxtrace.h +1 -0
index e0a5b39fed12..1c89a281a06c 100644
@@ -51,6 +51,7 @@ enum auxtrace_type {
        PERF_AUXTRACE_S390_CPUMSF,
        PERF_AUXTRACE_HISI_PTT,
        PERF_AUXTRACE_VPA_DTL,
+       PERF_AUXTRACE_RISCV_TRACE,
 };
 
 enum itrace_period_type {
tools/perf/util/rvtrace.h new file +18 -1
new file mode 100644
index 000000000000..93c041db8660
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright(C) 2015 Linaro Limited. All rights reserved.
+ * Author: Mathieu Poirier <[email protected]>
+ */
+
+#ifndef INCLUDE__UTIL_PERF_RVTRACE_H__
+#define INCLUDE__UTIL_PERF_RVTRACE_H__
+
+#include "debug.h"
+#include "auxtrace.h"
+#include "util/event.h"
+#include "util/session.h"
+#include <linux/bits.h>
+
+#define RVTRACE_AUXTRACE_PRIV_SIZE     sizeof(u64)
+
+#endif
--

2.43.0

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

author
#25 lore raw

PATCH v2 11/12 perf tools: Initial support for RISC-V trace decoder

In reply to Anup Patel

From: Mayuresh Chitale <[email protected]>

Add bare bones support for RISC-V trace decoder so that the data received from the hardware by the RISC-V trace perf driver can be written to the perf record output file.

---
 tools/perf/util/Build             |  1 +
 tools/perf/util/auxtrace.c        |  3 +
 tools/perf/util/rvtrace-decoder.c | 91 +++++++++++++++++++++++++++++++
 tools/perf/util/rvtrace.h         |  2 +
 4 files changed, 97 insertions(+)
 create mode 100644 tools/perf/util/rvtrace-decoder.c
tools/perf/util/Build +1 -0
index 4be313cd115a..f736cea51fd8 100644
@@ -145,6 +145,7 @@ perf-util-$(CONFIG_AUXTRACE) += cs-etm.o
 perf-util-$(CONFIG_AUXTRACE) += cs-etm-decoder/
 endif
 perf-util-$(CONFIG_AUXTRACE) += cs-etm-base.o
+perf-util-$(CONFIG_AUXTRACE) += rvtrace-decoder.o
 
 perf-util-y += parse-branch-options.o
 perf-util-y += dump-insn.o
tools/perf/util/auxtrace.c +3 -0
index c905563e0d8a..299991d5d305 100644
@@ -54,6 +54,7 @@
 #include "arm-spe.h"
 #include "hisi-ptt.h"
 #include "s390-cpumsf.h"
+#include "rvtrace.h"
 #include "util/mmap.h"
 #include "powerpc-vpadtl.h"
 
@@ -1395,6 +1396,8 @@ int perf_event__process_auxtrace_info(struct perf_session *session,
 		err = powerpc_vpadtl_process_auxtrace_info(event, session);
 		break;
 	case PERF_AUXTRACE_RISCV_TRACE:
+		err = rvtrace__process_auxtrace_info(event, session);
+		break;
 	case PERF_AUXTRACE_UNKNOWN:
 	default:
 		return -EINVAL;
tools/perf/util/rvtrace-decoder.c new file +91 -0

91 lines not on this page · raw message

tools/perf/util/rvtrace.h +2 -0
index 93c041db8660..fdf2e5866c85 100644
@@ -15,4 +15,6 @@
 
 #define RVTRACE_AUXTRACE_PRIV_SIZE	sizeof(u64)
 
+int rvtrace__process_auxtrace_info(union perf_event *event, struct perf_session *session);
+struct auxtrace_record *rvtrace_record_init(int *err);
 #endif
Signature · 1 line
2.43.0
#26 lore raw

Re: [PATCH v2 11/12] perf tools: Initial support for RISC-V trace decoder

Hi Anup,

Quoted 153 lines · Anup Patel

From: Mayuresh Chitale <[email protected]>

Add bare bones support for RISC-V trace decoder so that the data received from the hardware by the RISC-V trace perf driver can be written to the perf record output file.

---
 tools/perf/util/Build             |  1 +
 tools/perf/util/auxtrace.c        |  3 +
 tools/perf/util/rvtrace-decoder.c | 91 +++++++++++++++++++++++++++++++
 tools/perf/util/rvtrace.h         |  2 +
 4 files changed, 97 insertions(+)
 create mode 100644 tools/perf/util/rvtrace-decoder.c
tools/perf/util/Build +1 -0
index 4be313cd115a..f736cea51fd8 100644
@@ -145,6 +145,7 @@ perf-util-$(CONFIG_AUXTRACE) += cs-etm.o
 perf-util-$(CONFIG_AUXTRACE) += cs-etm-decoder/
 endif
 perf-util-$(CONFIG_AUXTRACE) += cs-etm-base.o
+perf-util-$(CONFIG_AUXTRACE) += rvtrace-decoder.o
 
 perf-util-y += parse-branch-options.o
 perf-util-y += dump-insn.o
tools/perf/util/auxtrace.c +3 -0
index c905563e0d8a..299991d5d305 100644
@@ -54,6 +54,7 @@
 #include "arm-spe.h"
 #include "hisi-ptt.h"
 #include "s390-cpumsf.h"
+#include "rvtrace.h"
 #include "util/mmap.h"
 #include "powerpc-vpadtl.h"
 
@@ -1395,6 +1396,8 @@ int perf_event__process_auxtrace_info(struct perf_session *session,
                err = powerpc_vpadtl_process_auxtrace_info(event, session);
                break;
        case PERF_AUXTRACE_RISCV_TRACE:
+               err = rvtrace__process_auxtrace_info(event, session);
+               break;
        case PERF_AUXTRACE_UNKNOWN:
        default:
                return -EINVAL;
tools/perf/util/rvtrace-decoder.c new file +91 -0
new file mode 100644
index 000000000000..58db5ca62c1a
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * RISC-V trace Decoder
+ */
+
+#include <errno.h>
+#include <inttypes.h>
+#include "evlist.h"
+#include <internal/lib.h>
+#include "rvtrace.h"
+
+struct rvtrace_decoder {
+       struct auxtrace auxtrace;
+       u32 auxtrace_type;
+       struct perf_session *session;
+       struct machine *machine;
+       u32 pmu_type;
+};
+
+static int rvtrace_process_event(struct perf_session *session __maybe_unused,
+                                union perf_event *event __maybe_unused,
+                                struct perf_sample *sample __maybe_unused,
+                                const struct perf_tool *tool __maybe_unused)
+{
+       return 0;
+}
+
+static int rvtrace_process_auxtrace_event(struct perf_session *session __maybe_unused,
+                                         union perf_event *event __maybe_unused,
+                                         const struct perf_tool *tool __maybe_unused)
+{
+       return 0;
+}
+
+static int rvtrace_flush(struct perf_session *session __maybe_unused,
+                        const struct perf_tool *tool __maybe_unused)
+{
+       return 0;
+}
+
+static void rvtrace_free_events(struct perf_session *session __maybe_unused)
+{
+}
+
+static void rvtrace_free(struct perf_session *session)
+{
+       struct rvtrace_decoder *ptr = container_of(session->auxtrace, struct rvtrace_decoder,
+                                           auxtrace);
+
+       session->auxtrace = NULL;
+       free(ptr);
+}
+
+static bool rvtrace_evsel_is_auxtrace(struct perf_session *session,
+                                     struct evsel *evsel)
+{
+       struct rvtrace_decoder *ptr = container_of(session->auxtrace,
+                                                  struct rvtrace_decoder, auxtrace);
+
+       return evsel->core.attr.type == ptr->pmu_type;
+}
+
+int rvtrace__process_auxtrace_info(union perf_event *event,
+                                  struct perf_session *session)
+{
+       struct perf_record_auxtrace_info *auxtrace_info = &event->auxtrace_info;
+       struct rvtrace_decoder *ptr;
+
+       if (auxtrace_info->header.size < RVTRACE_AUXTRACE_PRIV_SIZE +
+           sizeof(struct perf_record_auxtrace_info))
+               return -EINVAL;
+
+       ptr = zalloc(sizeof(*ptr));
+       if (!ptr)
+               return -ENOMEM;
+
+       ptr->session = session;
+       ptr->machine = &session->machines.host;
+       ptr->auxtrace_type = auxtrace_info->type;
+       ptr->pmu_type = auxtrace_info->priv[0];
+
+       ptr->auxtrace.process_event = rvtrace_process_event;
+       ptr->auxtrace.process_auxtrace_event = rvtrace_process_auxtrace_event;
+       ptr->auxtrace.flush_events = rvtrace_flush;
+       ptr->auxtrace.free_events = rvtrace_free_events;
+       ptr->auxtrace.free = rvtrace_free;
+       ptr->auxtrace.evsel_is_auxtrace = rvtrace_evsel_is_auxtrace;
+       session->auxtrace = &ptr->auxtrace;
+
+       return 0;
+}
tools/perf/util/rvtrace.h +0 -0
index 93c041db8660..fdf2e5866c85 100644

tools/perf/util/rvtrace.h line 19

Quoted 5 lines
tools/perf/util/rvtrace.h +2 -0
@@ -15,4 +15,6 @@
 
 #define RVTRACE_AUXTRACE_PRIV_SIZE     sizeof(u64)
 
+int rvtrace__process_auxtrace_info(union perf_event *event, struct perf_session *session);
+struct auxtrace_record *rvtrace_record_init(int *err);

It looks like the implementation of rvtrace_recording_init in the auxtrace.c file is static (private), and the arguments don't match this declaration.
Should we remove "static" from auxtrace.c and update this prototype
to: "struct auxtrace_record *rvtrace_recording_init(int *err, struct perf_pmu *rvtrace_pmu);"?

Or should we simply remove this declaration from the header? Thanks.

Regards,
Eric Lin,

Quoted 9 lines
tools/perf/util/rvtrace.h +0 -1
 #endif
--

2.43.0

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

#27 lore raw

Re: [PATCH v2 11/12] perf tools: Initial support for RISC-V trace decoder

Hi Eric,

Quoted 170 lines · Eric Lin

Hi Anup,

Quoted 153 lines · Anup Patel

From: Mayuresh Chitale <[email protected]>

Add bare bones support for RISC-V trace decoder so that the data received from the hardware by the RISC-V trace perf driver can be written to the perf record output file.

---
 tools/perf/util/Build             |  1 +
 tools/perf/util/auxtrace.c        |  3 +
 tools/perf/util/rvtrace-decoder.c | 91 +++++++++++++++++++++++++++++++
 tools/perf/util/rvtrace.h         |  2 +
 4 files changed, 97 insertions(+)
 create mode 100644 tools/perf/util/rvtrace-decoder.c
tools/perf/util/Build +1 -0
index 4be313cd115a..f736cea51fd8 100644
@@ -145,6 +145,7 @@ perf-util-$(CONFIG_AUXTRACE) += cs-etm.o
 perf-util-$(CONFIG_AUXTRACE) += cs-etm-decoder/
 endif
 perf-util-$(CONFIG_AUXTRACE) += cs-etm-base.o
+perf-util-$(CONFIG_AUXTRACE) += rvtrace-decoder.o
 
 perf-util-y += parse-branch-options.o
 perf-util-y += dump-insn.o
tools/perf/util/auxtrace.c +3 -0
index c905563e0d8a..299991d5d305 100644
@@ -54,6 +54,7 @@
 #include "arm-spe.h"
 #include "hisi-ptt.h"
 #include "s390-cpumsf.h"
+#include "rvtrace.h"
 #include "util/mmap.h"
 #include "powerpc-vpadtl.h"
 
@@ -1395,6 +1396,8 @@ int perf_event__process_auxtrace_info(struct perf_session *session,
                err = powerpc_vpadtl_process_auxtrace_info(event, session);
                break;
        case PERF_AUXTRACE_RISCV_TRACE:
+               err = rvtrace__process_auxtrace_info(event, session);
+               break;
        case PERF_AUXTRACE_UNKNOWN:
        default:
                return -EINVAL;
tools/perf/util/rvtrace-decoder.c new file +91 -0
new file mode 100644
index 000000000000..58db5ca62c1a
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * RISC-V trace Decoder
+ */
+
+#include <errno.h>
+#include <inttypes.h>
+#include "evlist.h"
+#include <internal/lib.h>
+#include "rvtrace.h"
+
+struct rvtrace_decoder {
+       struct auxtrace auxtrace;
+       u32 auxtrace_type;
+       struct perf_session *session;
+       struct machine *machine;
+       u32 pmu_type;
+};
+
+static int rvtrace_process_event(struct perf_session *session __maybe_unused,
+                                union perf_event *event __maybe_unused,
+                                struct perf_sample *sample __maybe_unused,
+                                const struct perf_tool *tool __maybe_unused)
+{
+       return 0;
+}
+
+static int rvtrace_process_auxtrace_event(struct perf_session *session __maybe_unused,
+                                         union perf_event *event __maybe_unused,
+                                         const struct perf_tool *tool __maybe_unused)
+{
+       return 0;
+}
+
+static int rvtrace_flush(struct perf_session *session __maybe_unused,
+                        const struct perf_tool *tool __maybe_unused)
+{
+       return 0;
+}
+
+static void rvtrace_free_events(struct perf_session *session __maybe_unused)
+{
+}
+
+static void rvtrace_free(struct perf_session *session)
+{
+       struct rvtrace_decoder *ptr = container_of(session->auxtrace, struct rvtrace_decoder,
+                                           auxtrace);
+
+       session->auxtrace = NULL;
+       free(ptr);
+}
+
+static bool rvtrace_evsel_is_auxtrace(struct perf_session *session,
+                                     struct evsel *evsel)
+{
+       struct rvtrace_decoder *ptr = container_of(session->auxtrace,
+                                                  struct rvtrace_decoder, auxtrace);
+
+       return evsel->core.attr.type == ptr->pmu_type;
+}
+
+int rvtrace__process_auxtrace_info(union perf_event *event,
+                                  struct perf_session *session)
+{
+       struct perf_record_auxtrace_info *auxtrace_info = &event->auxtrace_info;
+       struct rvtrace_decoder *ptr;
+
+       if (auxtrace_info->header.size < RVTRACE_AUXTRACE_PRIV_SIZE +
+           sizeof(struct perf_record_auxtrace_info))
+               return -EINVAL;
+
+       ptr = zalloc(sizeof(*ptr));
+       if (!ptr)
+               return -ENOMEM;
+
+       ptr->session = session;
+       ptr->machine = &session->machines.host;
+       ptr->auxtrace_type = auxtrace_info->type;
+       ptr->pmu_type = auxtrace_info->priv[0];
+
+       ptr->auxtrace.process_event = rvtrace_process_event;
+       ptr->auxtrace.process_auxtrace_event = rvtrace_process_auxtrace_event;
+       ptr->auxtrace.flush_events = rvtrace_flush;
+       ptr->auxtrace.free_events = rvtrace_free_events;
+       ptr->auxtrace.free = rvtrace_free;
+       ptr->auxtrace.evsel_is_auxtrace = rvtrace_evsel_is_auxtrace;
+       session->auxtrace = &ptr->auxtrace;
+
+       return 0;
+}
tools/perf/util/rvtrace.h +0 -0
index 93c041db8660..fdf2e5866c85 100644

tools/perf/util/rvtrace.h line 19

Quoted 5 lines
tools/perf/util/rvtrace.h +2 -0
@@ -15,4 +15,6 @@
 
 #define RVTRACE_AUXTRACE_PRIV_SIZE     sizeof(u64)
 
+int rvtrace__process_auxtrace_info(union perf_event *event, struct perf_session *session);
+struct auxtrace_record *rvtrace_record_init(int *err);

It looks like the implementation of rvtrace_recording_init in the auxtrace.c file is static (private), and the arguments don't match this declaration.
Should we remove "static" from auxtrace.c and update this prototype
to: "struct auxtrace_record *rvtrace_recording_init(int *err, struct perf_pmu *rvtrace_pmu);"?

Quoted 1 line

Or should we simply remove this declaration from the header? Thanks.

The declaration should be removed from the header.

Quoted 13 lines

Regards,
Eric Lin,

Quoted 9 lines
tools/perf/util/rvtrace.h +0 -1
 #endif
--

2.43.0

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

author
#28 lore raw

PATCH v2 12/12 MAINTAINERS: Add entry for RISC-V trace framework and drivers

In reply to Anup Patel

Add Mayuresh and myself as maintainers for RISC-V trace framework and drivers.

---
 MAINTAINERS | 11 +++++++++++
 1 file changed, 11 insertions(+)
MAINTAINERS +11 -0
index 29e94a94aa0a..45718190cf44 100644
@@ -22228,6 +22228,17 @@ F:	include/dt-bindings/power/thead,th1520-power.h
 F:	include/dt-bindings/reset/thead,th1520-reset.h
 F:	include/linux/firmware/thead/thead,th1520-aon.h
 
+RISC-V TRACE FRAMEWORK AND DRIVERS
+M:	Mayuresh Chitale <[email protected]>
+M:	Anup Patel <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	Documentation/devicetree/bindings/riscv/riscv,trace-component.yaml
+F:	drivers/hwtracing/rvtrace/*
+F:	include/linux/rvtrace*
+F:	tools/perf/arch/riscv/util/auxtrace.c
+F:	tools/perf/util/rvtrace*
+
 RNBD BLOCK DRIVERS
 M:	Md. Haris Iqbal <[email protected]>
 M:	Jack Wang <[email protected]>
Signature · 1 line
2.43.0