| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2a2f842f-ddfd-4bd0-b3a4-efcd1a82df73@gmail.com>
Date: Tue, 2 Dec 2025 02:41:52 -0800
From: Bo Gan <ganboing@...il.com>
To: Anup Patel <apatel@...tanamicro.com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>, Greg KH <gregkh@...uxfoundation.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Ian Rogers <irogers@...gle.com>
Cc: Mark Rutland <mark.rutland@....com>, devicetree@...r.kernel.org,
Alexandre Ghiti <alex@...ti.fr>, Atish Patra <atish.patra@...ux.dev>,
Peter Zijlstra <peterz@...radead.org>, Anup Patel <anup@...infault.org>,
Adrian Hunter <adrian.hunter@...el.com>, linux-kernel@...r.kernel.org,
Mayuresh Chitale <mchitale@...tanamicro.com>, Ingo Molnar
<mingo@...hat.com>, Jiri Olsa <jolsa@...nel.org>,
Mayuresh Chitale <mchitale@...il.com>, Namhyung Kim <namhyung@...nel.org>,
linux-riscv@...ts.infradead.org, Andrew Jones <ajones@...tanamicro.com>,
Liang Kan <kan.liang@...ux.intel.com>
Subject: Re: [PATCH v2 02/12] rvtrace: Initial implementation of driver
framework
Hi Anup,
See my comments below. I'll probably have another round of review later
so expect more comments coming.
On 11/1/25 08:42, Anup Patel wrote:
> 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
>
> Co-developed-by: Mayuresh Chitale <mchitale@...tanamicro.com>
> Signed-off-by: Mayuresh Chitale <mchitale@...tanamicro.com>
> Signed-off-by: Anup Patel <apatel@...tanamicro.com>
> ---
> 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
>
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 8e1ffa4358d5..3c0c3bd40a89 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -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/
> diff --git a/drivers/hwtracing/Kconfig b/drivers/hwtracing/Kconfig
> index 911ee977103c..daeb38fe332d 100644
> --- a/drivers/hwtracing/Kconfig
> +++ b/drivers/hwtracing/Kconfig
> @@ -7,4 +7,6 @@ source "drivers/hwtracing/intel_th/Kconfig"
>
> source "drivers/hwtracing/ptt/Kconfig"
>
> +source "drivers/hwtracing/rvtrace/Kconfig"
> +
> endmenu
> diff --git a/drivers/hwtracing/rvtrace/Kconfig b/drivers/hwtracing/rvtrace/Kconfig
> new file mode 100644
> index 000000000000..f8f6feea1953
> --- /dev/null
> +++ b/drivers/hwtracing/rvtrace/Kconfig
> @@ -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.
> diff --git a/drivers/hwtracing/rvtrace/Makefile b/drivers/hwtracing/rvtrace/Makefile
> new file mode 100644
> index 000000000000..988525a379cf
> --- /dev/null
> +++ b/drivers/hwtracing/rvtrace/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_RVTRACE) += rvtrace.o
> +rvtrace-y := rvtrace-core.o rvtrace-platform.o
> diff --git a/drivers/hwtracing/rvtrace/rvtrace-core.c b/drivers/hwtracing/rvtrace/rvtrace-core.c
> new file mode 100644
> index 000000000000..32fda1647b39
> --- /dev/null
> +++ b/drivers/hwtracing/rvtrace/rvtrace-core.c
> @@ -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)
> +{
> + 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.
> +
> + return (i < 0) ? -ETIMEDOUT : 0;
> +}
> +EXPORT_SYMBOL_GPL(rvtrace_poll_bit);
> +
> +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.
> +}
> +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;
> + }
> + }
> +
> + 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()?
> + }
> + }
> +
> + 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);
> diff --git a/drivers/hwtracing/rvtrace/rvtrace-platform.c b/drivers/hwtracing/rvtrace/rvtrace-platform.c
> new file mode 100644
> index 000000000000..6eb362880e5c
> --- /dev/null
> +++ b/drivers/hwtracing/rvtrace/rvtrace-platform.c
> @@ -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;
> + 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.
> + 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
> +
> + 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,
> +};
> diff --git a/include/linux/rvtrace.h b/include/linux/rvtrace.h
> new file mode 100644
> index 000000000000..e8e055ad6583
> --- /dev/null
> +++ b/include/linux/rvtrace.h
> @@ -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
Powered by blists - more mailing lists