| 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: <a6e787e3-f332-4445-9f7a-b2ec65daf019@gmail.com>
Date: Tue, 14 Oct 2025 01:59:22 -0700
From: Bo Gan <ganboing@...il.com>
To: Mayuresh Chitale <mchitale@...tanamicro.com>, 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 Mayuresh,
On 10/14/25 01:10, Mayuresh Chitale wrote:
> 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.
Sure, that also works.
>>
>> Bo
Bo
Powered by blists - more mailing lists