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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAK9=C2Vpd8EajVXdnwMjSjSFkt4OFkn8OT4XHr-Nfs6ipjynpg@mail.gmail.com>
Date: Thu, 30 Oct 2025 14:07:01 +0530
From: Anup Patel <apatel@...tanamicro.com>
To: Bo Gan <ganboing@...il.com>
Cc: 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>, 
	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 02/11] rvtrace: Initial implementation of driver framework

On Sat, Oct 11, 2025 at 6:33 AM Bo Gan <ganboing@...il.com> wrote:
>
> On 10/1/25 23:07, 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     | 484 +++++++++++++++++++
> >   drivers/hwtracing/rvtrace/rvtrace-platform.c | 174 +++++++
> >   include/linux/rvtrace.h                      | 272 +++++++++++
> >   7 files changed, 953 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 b5749cf67044..466a55580f60 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..52ea931745fc
> > --- /dev/null
> > +++ b/drivers/hwtracing/rvtrace/rvtrace-core.c
> > @@ -0,0 +1,484 @@
> > +// 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)
> > +{
> > +     const struct rvtrace_component_id *id;
> > +
> > +     for (id = ids; id->version && id->type; id++) {
> > +             if (comp->id.type == id->type &&
> > +                 comp->id.version == id->version)
> > +                     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);
> > +     }
> > +
> > +     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);
> > +}
> > +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;
> > +             }
> > +     }
> > +
> > +     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..a110ff1f2f08
> > --- /dev/null
> > +++ b/drivers/hwtracing/rvtrace/rvtrace-platform.c
> > @@ -0,0 +1,174 @@
> > +// 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);
> > +             if (!rep_node) {
> > +                     ret = -ENODEV;
> > +                     of_node_put(ep_node);
> > +                     break;
> > +             }
> > +             rdev_node = of_graph_get_port_parent(rep_node);
> > +
> > +             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);
>
> Don't you need the fwnode_handle_get the dest_fwnode to hold the reference
> to the sink like coresight does?
>
> https://elixir.bootlin.com/linux/v6.17.1/source/drivers/hwtracing/coresight/coresight-platform.c#L243

Okay, I will update.

>
> > +             conn->dest_comp = rvtrace_find_by_fwnode(conn->dest_fwnode);
> > +             if (!conn->dest_comp) {
> > +                     ret = -EPROBE_DEFER;
> > +                     of_node_put(ep_node);
>
> Missing a break; here.
>
> I finally figured out the hang issue on my p550. It's caused by this missing
> "break;". It caused the "ep_node" to get of_node_put twice. Please check.

Good catch. Thanks for reporting.

>
> > +             }
> > +
> > +             pdata->outconns[i] = conn;
> > +             i++;
> > +     }
> > +
> > +done:
> > +     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), "cpu", 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);
> > +
> > +     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..04eb03e62601
> > --- /dev/null
> > +++ b/include/linux/rvtrace.h
> > @@ -0,0 +1,272 @@
> > +/* 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
> > +
> > +/* 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);
> > +}
> > +
> > +#endif
> I suggest stress-testing it by creating some negative cases such as having
> dangling endpoint links in DT, e.g., RAM sinks disabled, but source encoder
> enabled, and check if the driver can properly handle it. Also because there
> can be 3 modules rvtrace/rvtrace-encoder/rvtrace-ramsink (and probably more
> for funnel later), check different module load orders and other edge cases
> such as only loading rvtrace+encoder and see if the driver can safely handle
> it without crash. In addition, try to load/unload the driver in a loop and
> see if resources are free'ed.

Testing infrastructure will develop over-time and there are a lot of topology
configurations to cover.

The current goal is to add a reasonably flexible rvtrace framework which
can be extended and hardened over time.

Regards,
Anup

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ