[RFC PATCH 16/17] verification/rvgen: Add support for per-obj monitors

Gabriele Monaco posted 17 patches 1 month, 3 weeks ago
There is a newer version of this series
[RFC PATCH 16/17] verification/rvgen: Add support for per-obj monitors
Posted by Gabriele Monaco 1 month, 3 weeks ago
The special per-object monitor type was just introduced in RV, this
requires the user to define some functions and type specific to the
object.

Adapt rvgen to add stub definitions for the monitor_target type, the
da_get_id() function and other modifications required to create
per-object monitors.

Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 tools/verification/rvgen/rvgen/dot2k.py     | 20 ++++++++++++++++++++
 tools/verification/rvgen/rvgen/generator.py |  2 +-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/tools/verification/rvgen/rvgen/dot2k.py b/tools/verification/rvgen/rvgen/dot2k.py
index 1f6ad11117ac..951bdb893592 100644
--- a/tools/verification/rvgen/rvgen/dot2k.py
+++ b/tools/verification/rvgen/rvgen/dot2k.py
@@ -23,10 +23,14 @@ class dot2k(Monitor, Dot2c):
         self.monitor_class = extra_params["monitor_class"]
 
     def fill_monitor_type(self) -> str:
+        if self.monitor_type == "per_obj":
+            return self.monitor_type.upper() + """
+typedef /* XXX: define the target type */ *monitor_target;"""
         return self.monitor_type.upper()
 
     def fill_tracepoint_handlers_skel(self) -> str:
         buff = []
+        buff += self.fill_per_obj_definitions()
         buff += self.fill_hybrid_definitions()
         for event in self.events:
             buff.append("static void handle_%s(void *data, /* XXX: fill header */)" % event)
@@ -41,6 +45,9 @@ class dot2k(Monitor, Dot2c):
             if self.monitor_type == "per_task":
                 buff.append("\tstruct task_struct *p = /* XXX: how do I get p? */;");
                 buff.append("\tda_%s(p, %s%s);" % (handle, event, self.enum_suffix));
+            elif self.monitor_type == "per_obj":
+                buff.append("\tmonitor_target t = /* XXX: how do I get t? */;");
+                buff.append("\tda_%s(t, %s%s);" % (handle, event, self.enum_suffix));
             else:
                 buff.append("\tda_%s(%s%s);" % (handle, event, self.enum_suffix));
             buff.append("}")
@@ -130,6 +137,19 @@ class dot2k(Monitor, Dot2c):
         """Stub, not valid for deterministic automata"""
         return []
 
+    def fill_per_obj_definitions(self) -> list:
+        if self.monitor_type == "per_obj":
+            return ["""
+/*
+ * da_get_id - Get the id from a target
+ */
+static inline da_id_type da_get_id(monitor_target target)
+{
+	return /* XXX: define how to get an id from the target */;
+}
+"""]
+        return []
+
     def fill_main_c(self) -> str:
         main_c = super().fill_main_c()
 
diff --git a/tools/verification/rvgen/rvgen/generator.py b/tools/verification/rvgen/rvgen/generator.py
index b80af3fd6701..5eac12e110dc 100644
--- a/tools/verification/rvgen/rvgen/generator.py
+++ b/tools/verification/rvgen/rvgen/generator.py
@@ -243,7 +243,7 @@ obj-$(CONFIG_RV_MON_%s) += monitors/%s/%s.o
 
 
 class Monitor(RVGenerator):
-    monitor_types = { "global" : 1, "per_cpu" : 2, "per_task" : 3 }
+    monitor_types = { "global" : 1, "per_cpu" : 2, "per_task" : 3, "per_obj" : 4 }
 
     def __init__(self, extra_params={}):
         super().__init__(extra_params)
-- 
2.50.1
Re: [RFC PATCH 16/17] verification/rvgen: Add support for per-obj monitors
Posted by Nam Cao 1 month ago
On Thu, Aug 14, 2025 at 05:08:08PM +0200, Gabriele Monaco wrote:
> +    def fill_per_obj_definitions(self) -> list:
> +        if self.monitor_type == "per_obj":
> +            return ["""
> +/*
> + * da_get_id - Get the id from a target
> + */
> +static inline da_id_type da_get_id(monitor_target target)
> +{
> +	return /* XXX: define how to get an id from the target */;
> +}
> +"""]
> +        return []
> +

I know this is the existing style that we have. But I think this is not
something we should keep. How about something like:

import textwrap

def fill_per_obj_definitions(self) -> list:
    if self.monitor_type == "per_obj":
        return [textwrap.dedent("""
            /*
             * da_get_id - Get the id from a target
             */
            static inline da_id_type da_get_id(monitor_target target)
            {
            	return /* XXX: define how to get an id from the target */;
            }
            """)]
    return []
Re: [RFC PATCH 16/17] verification/rvgen: Add support for per-obj monitors
Posted by Gabriele Monaco 1 month ago

On Thu, 2025-09-04 at 10:20 +0200, Nam Cao wrote:
> On Thu, Aug 14, 2025 at 05:08:08PM +0200, Gabriele Monaco wrote:
> > +    def fill_per_obj_definitions(self) -> list:
> > +        if self.monitor_type == "per_obj":
> > +            return ["""
> > +/*
> > + * da_get_id - Get the id from a target
> > + */
> > +static inline da_id_type da_get_id(monitor_target target)
> > +{
> > +	return /* XXX: define how to get an id from the target */;
> > +}
> > +"""]
> > +        return []
> > +
> 
> I know this is the existing style that we have. But I think this is
> not something we should keep. How about something like:
> 
> import textwrap
> 
> def fill_per_obj_definitions(self) -> list:
>     if self.monitor_type == "per_obj":
>         return [textwrap.dedent("""
>             /*
>              * da_get_id - Get the id from a target
>              */
>             static inline da_id_type da_get_id(monitor_target target)
>             {
>             	return /* XXX: define how to get an id from the
> target */;
>             }
>             """)]
>     return []

Mmh, I see what you mean.
I wasn't aware of this textwrap, but what immediately comes to mind is
that we'd end up mixing spaces (python indentation) and tabs (C
indentation).
While textwrap may handle the case, it doesn't look too readable to me.

I'd say we can (slowly) transition to using textwrap here but mandate
having \t for tabs to make them explicit (didn't test).

What do you think?

Thanks,
Gabriele
Re: [RFC PATCH 16/17] verification/rvgen: Add support for per-obj monitors
Posted by Nam Cao 1 month ago
On Thu, Sep 04, 2025 at 10:39:31AM +0200, Gabriele Monaco wrote:
> Mmh, I see what you mean.
> I wasn't aware of this textwrap, but what immediately comes to mind is
> that we'd end up mixing spaces (python indentation) and tabs (C
> indentation).
>
> While textwrap may handle the case, it doesn't look too readable to me.
> 
> I'd say we can (slowly) transition to using textwrap here but mandate
> having \t for tabs to make them explicit (didn't test).
> 
> What do you think?

Right, I didn't consider the mixing indentation problem.

Using \t isn't much better, it is harder to see how the generated code will
look.

One idea is using something like clang-format
(Documentation/dev-tools/clang-format.rst). It doesn't have to be done in
this series of course, I'm fine leaving this be for now.

Nam