Re: [PATCH v3 08/17] verification/rvgen: Organise Kconfig entries for nested monitors
From: Gabriele Monaco
Date: Wed Jul 16 2025 - 03:40:40 EST
On Tue, 2025-07-15 at 16:48 +0200, Nam Cao wrote:
> On Tue, Jul 15, 2025 at 09:14:25AM +0200, Gabriele Monaco wrote:
> > The current behaviour of rvgen when running with the -a option is
> > to
> > append the necessary lines at the end of the configuration for
> > Kconfig,
> > Makefile and tracepoints.
> > This is not always the desired behaviour in case of nested
> > monitors:
> > while tracepoints are not affected by nesting and the Makefile's
> > only
> > requirement is that the parent monitor is built before its
> > children, in
> > the Kconfig it is better to have children defined right after their
> > parent, otherwise the result has wrong indentation:
> >
> > [*] foo_parent monitor
> > [*] foo_child1 monitor
> > [*] foo_child2 monitor
> > [*] bar_parent monitor
> > [*] bar_child1 monitor
> > [*] bar_child2 monitor
> > [*] foo_child3 monitor
> > [*] foo_child4 monitor
> >
> > Adapt rvgen to look for a different marker for nested monitors in
> > the
> > Kconfig file and append the line right after the last sibling,
> > instead
> > of the last monitor.
> > Also add the marker when creating a new parent monitor.
> >
> > Signed-off-by: Gabriele Monaco <gmonaco@xxxxxxxxxx>
> > ---
> > kernel/trace/rv/Kconfig | 5 +++++
> > tools/verification/rvgen/rvgen/container.py | 13 +++++++++++++
> > tools/verification/rvgen/rvgen/generator.py | 13 ++++++++-----
> > 3 files changed, 26 insertions(+), 5 deletions(-)
> >
> > diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
> > index c11bf7e61ebf0..26017378f79b8 100644
> > --- a/kernel/trace/rv/Kconfig
> > +++ b/kernel/trace/rv/Kconfig
> > @@ -43,6 +43,7 @@ config RV_PER_TASK_MONITORS
> >
> > source "kernel/trace/rv/monitors/wip/Kconfig"
> > source "kernel/trace/rv/monitors/wwnr/Kconfig"
> > +
> > source "kernel/trace/rv/monitors/sched/Kconfig"
> > source "kernel/trace/rv/monitors/tss/Kconfig"
> > source "kernel/trace/rv/monitors/sco/Kconfig"
> > @@ -50,9 +51,13 @@ source "kernel/trace/rv/monitors/snroc/Kconfig"
> > source "kernel/trace/rv/monitors/scpd/Kconfig"
> > source "kernel/trace/rv/monitors/snep/Kconfig"
> > source "kernel/trace/rv/monitors/sncid/Kconfig"
> > +# Add new sched monitors here
> > +
> > source "kernel/trace/rv/monitors/rtapp/Kconfig"
> > source "kernel/trace/rv/monitors/pagefault/Kconfig"
> > source "kernel/trace/rv/monitors/sleep/Kconfig"
> > +# Add new rtapp monitors here
> > +
> > # Add new monitors here
> >
> > config RV_REACTORS
> > diff --git a/tools/verification/rvgen/rvgen/container.py
> > b/tools/verification/rvgen/rvgen/container.py
> > index 47d8ab2ad3ec4..fee493f89fde6 100644
> > --- a/tools/verification/rvgen/rvgen/container.py
> > +++ b/tools/verification/rvgen/rvgen/container.py
> > @@ -20,3 +20,16 @@ class Container(generator.RVGenerator):
> > main_h = self.main_h
> > main_h = main_h.replace("%%MODEL_NAME%%", self.name)
> > return main_h
> > +
> > + def fill_kconfig_tooltip(self):
> > + """Override to produce a marker for this container in the
> > Kconfig"""
> > + container_marker = f"# Add new {self.name} monitors
> > here\n"
> > + if self.auto_patch:
> > + self._patch_file("Kconfig",
> > + "# Add new monitors here", "")
>
> Isn't this one excessive? I gave it a try, but I had double blank
> line:
>
> python3 tools/verification/rvgen -a container -n hello
>
> 61 source "kernel/trace/rv/monitors/sleep/Kconfig"
> 62 # Add new rtapp monitors here
> 63
> + 64
> + 65 source "kernel/trace/rv/monitors/hello/Kconfig"
> + 66 # Add new hello monitors here
> + 67
> 68 # Add new monitors here
> 69
> 70 config RV_REACTORS
>
Yeah indeed it is, thanks for finding out, it somehow slipped my
tests..
> > + result = super().fill_kconfig_tooltip()
> > + if self.auto_patch:
> > + self._patch_file("Kconfig",
> > + "# Add new monitors here",
> > container_marker)
> > + return result
> > + return result + container_marker
> > diff --git a/tools/verification/rvgen/rvgen/generator.py
> > b/tools/verification/rvgen/rvgen/generator.py
> > index 19d0078a38032..ac97c4505ff00 100644
> > --- a/tools/verification/rvgen/rvgen/generator.py
> > +++ b/tools/verification/rvgen/rvgen/generator.py
> > @@ -137,7 +137,8 @@ class RVGenerator:
> > kconfig = kconfig.replace("%%MONITOR_DEPS%%",
> > monitor_deps)
> > return kconfig
> >
> > - def __patch_file(self, file, marker, line):
> > + def _patch_file(self, file, marker, line):
> > + assert(self.auto_patch)
> > file_to_patch = os.path.join(self.rv_dir, file)
> > content = self._read_file(file_to_patch)
> > content = content.replace(marker, line + "\n" + marker)
> > @@ -146,7 +147,7 @@ class RVGenerator:
> > def fill_tracepoint_tooltip(self):
> > monitor_class_type = self.fill_monitor_class_type()
> > if self.auto_patch:
> > - self.__patch_file("rv_trace.h",
> > + self._patch_file("rv_trace.h",
> > "// Add new monitors based on
> > CONFIG_%s here" % monitor_class_type,
> > "#include <monitors/%s/%s_trace.h>" %
> > (self.name, self.name))
> > return " - Patching %s/rv_trace.h, double check the
> > result" % self.rv_dir
> > @@ -158,8 +159,10 @@ Add this line where other tracepoints are
> > included and %s is defined:
> >
> > def fill_kconfig_tooltip(self):
> > if self.auto_patch:
> > - self.__patch_file("Kconfig",
> > - "# Add new monitors here",
> > + # monitors with a container should stay together in
> > the Kconfig
> > + self._patch_file("Kconfig",
> > + "# Add new %smonitors here" %
> > + (self.parent + " " if self.parent
> > else ""),
>
> This one must be kept in sync with container_marker in
> Container.fill_kconfig_tooltip(). I think this is hard to maintain
> later
> on.
>
> How about we keep in centralized with a helper function, something
> like:
>
> def container_marker(container: str) -> str:
> return f"# Add new {container} monitors here\n"
>
Good point, will do.
Thanks,
Gabriele