| 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: <11594604-a6f6-44cb-96f9-0c06e2cc9ae0@gmail.com>
Date: Wed, 8 Oct 2025 02:51:05 -0700
From: Bo Gan <ganboing@...il.com>
To: Mayuresh Chitale <mchitale@...tanamicro.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>, 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, 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 05/11] rvtrace: Add trace encoder driver
Hi Mayuresh,
I overlooked the ordering of your rvtrace_component_reset and registering
on rvtrace bus. Yes, you are doing proper reset, so please ignore my
previous msg. Sorry for the noise. Now back to figure out why my hacked
version doesn't work on my p550...
On a side note, it seems that there's something wrong with your email
client? I don't see your reply on the mailing-list:
https://lore.kernel.org/all/20251002060732.100213-1-apatel@ventanamicro.com/
And your reply mail appears not in plain text mode.
On 10/8/25 02:22, Mayuresh Chitale wrote:
>
>
> On Wed, Oct 8, 2025 at 2:19 PM Bo Gan <ganboing@...il.com <mailto:ganboing@...il.com>> wrote:
>
> On 10/7/25 00:09, Bo Gan wrote:
> > On 10/1/25 23:07, Anup Patel wrote:
> >> From: Mayuresh Chitale <mchitale@...tanamicro.com <mailto:mchitale@...tanamicro.com>>
> >>
> >> Add initial implementation of RISC-V trace encoder driver. The encoder
> >> is defined in the RISC-V Trace Control Interface specification.
> >>
> >> Co-developed-by: Anup Patel <apatel@...tanamicro.com <mailto:apatel@...tanamicro.com>>
> >> Signed-off-by: Anup Patel <apatel@...tanamicro.com <mailto:apatel@...tanamicro.com>>
> >> Signed-off-by: Mayuresh Chitale <mchitale@...tanamicro.com <mailto:mchitale@...tanamicro.com>>
> >> ---
> >> 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
> >>
> >> diff --git a/drivers/hwtracing/rvtrace/Kconfig b/drivers/hwtracing/rvtrace/Kconfig
> >> index f8f6feea1953..ba35c05f3f54 100644
> >> --- a/drivers/hwtracing/rvtrace/Kconfig
> >> +++ b/drivers/hwtracing/rvtrace/Kconfig
> >> @@ -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.
> >> diff --git a/drivers/hwtracing/rvtrace/Makefile b/drivers/hwtracing/rvtrace/Makefile
> >> index 988525a379cf..f320693a1fc5 100644
> >> --- a/drivers/hwtracing/rvtrace/Makefile
> >> +++ b/drivers/hwtracing/rvtrace/Makefile
> >> @@ -2,3 +2,4 @@
> >> obj-$(CONFIG_RVTRACE) += rvtrace.o
> >> rvtrace-y := rvtrace-core.o rvtrace-platform.o
> >> +obj-$(CONFIG_RVTRACE_ENCODER) += rvtrace-encoder.o
> >> diff --git a/drivers/hwtracing/rvtrace/rvtrace-encoder.c b/drivers/hwtracing/rvtrace/rvtrace-encoder.c
> >> new file mode 100644
> >> index 000000000000..45d1c5b12c51
> >> --- /dev/null
> >> +++ b/drivers/hwtracing/rvtrace/rvtrace-encoder.c
> >> @@ -0,0 +1,107 @@
> >> +// 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>
> >> +
> >> +#define RVTRACE_COMPONENT_CTRL_ITRACE_MASK 0x1
> >> +#define RVTRACE_COMPONENT_CTRL_ITRACE_SHIFT 2
> >> +#define RVTRACE_COMPONENT_CTRL_INSTMODE_MASK 0x7
> >> +#define RVTRACE_COMPONENT_CTRL_INSTMODE_SHIFT 4
> >> +
> >> +static int rvtrace_encoder_start(struct rvtrace_component *comp)
> >> +{
> >> + u32 val;
> >> +
> >> + val = rvtrace_read32(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET);
> >> + val |= BIT(RVTRACE_COMPONENT_CTRL_ITRACE_SHIFT);
> >> + rvtrace_write32(comp->pdata, val, RVTRACE_COMPONENT_CTRL_OFFSET);
> >> + return rvtrace_poll_bit(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET,
> >> + RVTRACE_COMPONENT_CTRL_ITRACE_SHIFT, 1,
> >> + comp->pdata->control_poll_timeout_usecs);
> >> +}
> >> +
> >> +static int rvtrace_encoder_stop(struct rvtrace_component *comp)
> >> +{
> >> + u32 val;
> >> +
> >> + val = rvtrace_read32(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET);
> >> + val &= ~BIT(RVTRACE_COMPONENT_CTRL_ITRACE_SHIFT);
> >> + rvtrace_write32(comp->pdata, val, RVTRACE_COMPONENT_CTRL_OFFSET);
> >> + return rvtrace_poll_bit(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET,
> >> + RVTRACE_COMPONENT_CTRL_ITRACE_SHIFT, 0,
> >> + comp->pdata->control_poll_timeout_usecs);
> >> +}
> >> +
> >> +static void rvtrace_encoder_setmode(struct rvtrace_component *comp, u32 mode)
> >> +{
> >> + u32 val;
> >> +
> >> + val = rvtrace_read32(comp->pdata, RVTRACE_COMPONENT_CTRL_OFFSET);
> >> + val |= (mode << RVTRACE_COMPONENT_CTRL_INSTMODE_SHIFT);
> >> + rvtrace_write32(comp->pdata, val, RVTRACE_COMPONENT_CTRL_OFFSET);
> >> +}
> >> +
> >> +static int rvtrace_encoder_probe(struct rvtrace_component *comp)
> >> +{
> >> + int ret;
> >> +
> >> + rvtrace_encoder_setmode(comp, 0x6);
> >> + ret = rvtrace_enable_component(comp);
> >> + if (ret)
> >> + return dev_err_probe(&comp->dev, ret, "failed to enable encoder.\n");
> >> +
> >> + return 0;
> >> +}
> > Trace components needs proper probing before use. Refer to Control Interface
> > Spec: https://github.com/riscv-non-isa/tg-nexus-trace/releases/download/1.0_Ratified/RISC-V-Trace-Control-Interface.pdf <https://github.com/riscv-non-isa/tg-nexus-trace/releases/download/1.0_Ratified/RISC-V-Trace-Control-Interface.pdf>
> > Chapter 5. This versioning applies to all components, e.g., sinks/funnels...
> > The driver should check the HW impl version with what it supports, and
> > rejects registering the component(s) if not supported. Chapter 5. has more
> > details.
> >
> Forgot to mention another thing: you also need to follow Control Interface
> Spec Chapter 11.2 Reset and Discovery to toggle tr??Active. Do not access
> any other registers or set any other bits in tr??Control before properly
> enable tr??Active. My userspace driver does the reset as:
> https://github.com/ganboing/riscv-trace-umd/blob/master/rvtrace/device.py#L57 <https://github.com/ganboing/riscv-trace-umd/blob/master/rvtrace/device.py#L57>
>
>
> Actually reset is done when a component is registered (rvtrace_register_component) and it is enabled when it gets probed (rvtrace_enable_component). I will incorporate the other changes in V2.
>
> Your HW IP might ignore tr??Active bit and the trace components are always
> clocked, and that's probably why you don't see any issue without proper
> resetting. However, on my p550 (EIC7700), I adopted your code to a v6.6
> tree, boot, and load it as module. The core immediately hangs, very likely
> due to it tries to set tr??Control bits without proper resetting. You
> should expect other IPs that implement clock gating and strictly follow
> the Spec. Thanks.
>
> >> +
> >> +static void rvtrace_encoder_remove(struct rvtrace_component *comp)
> >> +{
> >> + int ret;
> >> +
> >> + ret = rvtrace_disable_component(comp);
> >> + if (ret)
> >> + dev_err(&comp->dev, "failed to disable encoder.\n");
> >> +}
> >> +
> >> +static struct rvtrace_component_id rvtrace_encoder_ids[] = {
> >> + { .type = RVTRACE_COMPONENT_TYPE_ENCODER,
> >> + .version = rvtrace_component_mkversion(1, 0), },
> >> + {},
> >> +};
> >> +
> >> +static struct rvtrace_driver rvtrace_encoder_driver = {
> >> + .id_table = rvtrace_encoder_ids,
> >> + .start = rvtrace_encoder_start,
> >> + .stop = rvtrace_encoder_stop,
> >> + .probe = rvtrace_encoder_probe,
> >> + .remove = rvtrace_encoder_remove,
> >> + .driver = {
> >> + .name = "rvtrace-encoder",
> >> + },
> >> +};
> >> +
> >> +static int __init rvtrace_encoder_init(void)
> >> +{
> >> + return rvtrace_register_driver(&rvtrace_encoder_driver);
> >> +}
> >> +
> >> +static void __exit rvtrace_encoder_exit(void)
> >> +{
> >> + rvtrace_unregister_driver(&rvtrace_encoder_driver);
> >> +}
> >> +
> >> +module_init(rvtrace_encoder_init);
> >> +module_exit(rvtrace_encoder_exit);
> >> +
> >> +/* Module information */
> >> +MODULE_AUTHOR("Mayuresh Chitale <mchitale@...tanamicro.com <mailto:mchitale@...tanamicro.com>>");
> >> +MODULE_DESCRIPTION("RISC-V Trace Encoder Driver");
> >> +MODULE_LICENSE("GPL");
> >
> > Bo
>
> Bo
>
Bo
Powered by blists - more mailing lists