| 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: <CAN37VV4jFyuEYw0cxU00FucgV=so5SfTon_1hdvbVuqX1QzYCQ@mail.gmail.com>
Date: Tue, 14 Oct 2025 13:40:59 +0530
From: Mayuresh Chitale <mchitale@...tanamicro.com>
To: Bo Gan <ganboing@...il.com>
Cc: 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>,
Alexandre Ghiti <alex@...ti.fr>, Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>, Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>, Liang Kan <kan.liang@...ux.intel.com>,
Mayuresh Chitale <mchitale@...il.com>, Anup Patel <anup@...infault.org>,
Atish Patra <atish.patra@...ux.dev>, Andrew Jones <ajones@...tanamicro.com>,
Sunil V L <sunilvl@...tanamicro.com>, linux-riscv@...ts.infradead.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 04/11] rvtrace: Add functions to start/stop tracing on a
component path
Hi Bo,
On Mon, Oct 13, 2025 at 10:23 AM Bo Gan <ganboing@...il.com> wrote:
>
> On 10/12/25 20:43, Anup Patel wrote:
> > On Wed, Oct 8, 2025 at 2:45 PM Bo Gan <ganboing@...il.com> wrote:
> >>
> >> On 10/1/25 23:07, Anup Patel wrote:
> >>> From: Mayuresh Chitale <mchitale@...tanamicro.com>
> >>>
> >>> 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.
> >>>
> >>> Co-developed-by: Anup Patel <apatel@...tanamicro.com>
> >>> Signed-off-by: Anup Patel <apatel@...tanamicro.com>
> >>> Signed-off-by: Mayuresh Chitale <mchitale@...tanamicro.com>
> >>> ---
> >>> drivers/hwtracing/rvtrace/rvtrace-core.c | 44 ++++++++++++++++++++++++
> >>> include/linux/rvtrace.h | 6 ++++
> >>> 2 files changed, 50 insertions(+)
> >>>
> >>> diff --git a/drivers/hwtracing/rvtrace/rvtrace-core.c b/drivers/hwtracing/rvtrace/rvtrace-core.c
> >>> index 7013d50ca569..109be40d4b24 100644
> >>> --- a/drivers/hwtracing/rvtrace/rvtrace-core.c
> >>> +++ b/drivers/hwtracing/rvtrace/rvtrace-core.c
> >>> @@ -614,6 +614,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)
> >>> diff --git a/include/linux/rvtrace.h b/include/linux/rvtrace.h
> >>> index f2174f463a69..e7bd335d388f 100644
> >>> --- a/include/linux/rvtrace.h
> >>> +++ b/include/linux/rvtrace.h
> >>> @@ -273,10 +273,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
> >>> @@ -285,6 +289,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,
> >>
> >> I'd suggest add another function (*quiesce) or something like that. Trace
> >> components have a tr??Empty bit that indicates trace has been all flushed
> >> out. Also along the path when you do rvtrace_path_stop, you need to ensure
> >> the source has stopped and quiescent before beginning to stop the sink.
> >> Otherwise you'll get partial or corrupted trace. In essence, follow Control
> >> Interface Spec 11.3 Enabling and Disabling. FYI: my userspace driver:
> >> https://github.com/ganboing/riscv-trace-umd/blob/master/rvtrace/funnel.py#L223
> >
> > It's better to add functions on a need basis rather than adding
> > it now without any potential user.
> >
> > Regards,
> > Anu
>
> Hi Anup, my previous comment also applies to your current use case where you
> have encoder->RAM sink directly connected together. Having a longer path,
> e.g., funnels in between makes it worse. The driver needs to poll the empty
> bit tr??Empty (bit 3) of the control register to check if trace has been
> completely flushed. Otherwise, you get a partial trace, possibly with last
> few messages missing or truncated. So, yes, there's really a need to do so.
I think this can also be implemented in the component's 'stop' callback.
>
> Bo
Powered by blists - more mailing lists