Re: [PATCH v6 4/9] rv: Fix ha_invariant_passed_ns silent bypass of invariant check

[Gabriele Monaco <gmonaco@redhat.com>]

On Fri, 2026-08-21 at 00:45 +0800, wen.yang@linux.dev wrote:
> From: Wen Yang <wen.yang@linux.dev>
> 
> When env_store is U64_MAX (its initial sentinel value),
> ha_invariant_passed_ns() returns 0 immediately without initializing
> env_store to the current clock.  Subsequent calls to
> ha_check_invariant_ns() then find env_store still at U64_MAX, causing
> the elapsed comparison to wrap and always report the invariant as
> satisfied, silently masking any violations.
> 
> Fix by calling ha_reset_clk_ns() to establish the guard on the first
> invocation instead of returning early.  Apply the same fix to
> ha_invariant_passed_jiffy().
> 
> This is a stopgap: once the RV framework reworks the per-env clock
> guard, this first-invocation reset should be subsumed.
> 
> Signed-off-by: Wen Yang <wen.yang@linux.dev>
> ---

I would like to solve this a bit differently. I have this patch hanging around
to reset() when the monitor starts, which makes the whole invalid thing
obsolete.

I was planning on sending it together with other changes when I have time to
test them, but it seems the patch works fine on this series as well.

I reverted your fix, applied that and added the ha_reset_env() which you were
not defining:

diff --git a/kernel/trace/rv/monitors/tlob/tlob.c
b/kernel/trace/rv/monitors/tlob/tlob.c
index 18150cbf57a5..934eea34021a 100644
--- a/kernel/trace/rv/monitors/tlob/tlob.c
+++ b/kernel/trace/rv/monitors/tlob/tlob.c
@@ -175,6 +175,12 @@ static u64 ha_get_env(struct ha_monitor *ha_mon, enum
envs_tlob env,
        return ENV_INVALID_VALUE;
 }
 
+static void ha_reset_env(struct ha_monitor *ha_mon, enum envs_tlob env, u64
time_ns)
+{
+       if (env == clk_elapsed_tlob)
+               ha_reset_clk_ns(ha_mon, env, time_ns);
+}
+
 /*
  * Invariant: clk_elapsed < BUDGET_NS in running/waiting/sleeping.  "stopped"
  * is exempt: the parked period must not be measured against the old window's

---

Applying the appended patch too and running the selftests seems to work fine on
my setup.

If you want, you can add this patch to your series for now as it doesn't have
dependencies, I can sort the rest out.

Thanks,
Gabriele

---

From 29bdf58fc8f108d63b06051b415b97a45b27a976 Mon Sep 17 00:00:00 2001
From: Gabriele Monaco <gmonaco@redhat.com>
Date: Mon, 25 May 2026 10:23:25 +0200
Subject: [PATCH] rv: Force environment reset action on HA monitor start

Currently the monitor variables with a storage (e.g. clocks) are
initialised as invalid, then the first explicit reset() sets them as
valid for constraint. This only adds extra complexity to handle the
invalid case in constraints check.

Apply a reset() by default when starting monitors instead of resetting
to an invalid state. If monitors have no stored variable (hence no reset
function) add a macro HA_NO_RESET to stub it, this is all transparently
handled by rvgen.

The reset() action on ns-granularity clocks needs the (possibly cached)
current time, read it directly there. This might cause a double call to
ktime_get_ns() within the same da_handle_start_run_event() but will
occur only the first time and is harmless.

Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 include/rv/ha_monitor.h                 | 8 +++++++-
 kernel/trace/rv/monitors/opid/opid.h    | 1 +
 tools/verification/rvgen/rvgen/dot2c.py | 2 ++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/rv/ha_monitor.h b/include/rv/ha_monitor.h
index c2e200234e67..4fc5dadb4240 100644
--- a/include/rv/ha_monitor.h
+++ b/include/rv/ha_monitor.h
@@ -167,19 +167,25 @@ static void ha_monitor_destroy(void)
 
 /* Should be supplied by the monitor */
 static u64 ha_get_env(struct ha_monitor *ha_mon, enum envs env, u64 time_ns);
+static void ha_reset_env(struct ha_monitor *ha_mon, enum envs env, u64 time_ns);
 static bool ha_verify_constraint(struct ha_monitor *ha_mon,
 				 enum states curr_state,
 				 enum events event,
 				 enum states next_state,
 				 u64 time_ns);
+#ifdef HA_NO_RESET
+static void ha_reset_env(struct ha_monitor *ha_mon, enum envs env, u64 time_ns) { }
+#endif
 
 /*
  * ha_monitor_reset_all_stored - reset all environment variables in the monitor
  */
 static inline void ha_monitor_reset_all_stored(struct ha_monitor *ha_mon)
 {
+	u64 time_ns = ha_get_ns();
+
 	for (int i = 0; i < ENV_MAX_STORED; i++)
-		WRITE_ONCE(ha_mon->env_store[i], ENV_INVALID_VALUE);
+		ha_reset_env(ha_mon, i, time_ns);
 }
 
 /*
diff --git a/kernel/trace/rv/monitors/opid/opid.h b/kernel/trace/rv/monitors/opid/opid.h
index fb0aa4c28aa6..f85b1959c2fb 100644
--- a/kernel/trace/rv/monitors/opid/opid.h
+++ b/kernel/trace/rv/monitors/opid/opid.h
@@ -28,6 +28,7 @@ enum envs_opid {
 };
 
 _Static_assert(env_max_stored_opid <= MAX_HA_ENV_LEN, "Not enough slots");
+#define HA_NO_RESET
 
 struct automaton_opid {
 	char *state_names[state_max_opid];
diff --git a/tools/verification/rvgen/rvgen/dot2c.py b/tools/verification/rvgen/rvgen/dot2c.py
index 22938ce1bf6c..1532e6b6e199 100644
--- a/tools/verification/rvgen/rvgen/dot2c.py
+++ b/tools/verification/rvgen/rvgen/dot2c.py
@@ -90,6 +90,8 @@ class Dot2c(Automata):
                         ' "Not enough slots");')
             if {"ns", "us", "ms", "s"}.intersection(self.env_types.values()):
                 buff.append("#define HA_CLK_NS")
+            if len(self.env_stored) == 0:
+                buff.append("#define HA_NO_RESET")
             buff.append("")
         return buff
 
-- 
2.55.0