On 3/14/25 13:03, Richard Henderson wrote:
On 3/14/25 11:36, Pierrick Bouvier wrote:
On 3/14/25 11:13, Richard Henderson wrote:
On 3/13/25 14:00, Pierrick Bouvier wrote:
diff --git a/include/hw/intc/armv7m_nvic.h b/include/hw/intc/armv7m_nvic.h
index 89fe8aedaa..7b9964fe7e 100644
--- a/include/hw/intc/armv7m_nvic.h
+++ b/include/hw/intc/armv7m_nvic.h
@@ -189,21 +189,7 @@ int armv7m_nvic_raw_execution_priority(NVICState *s);
* @secure: the security state to test
* This corresponds to the pseudocode IsReqExecPriNeg().
*/
-#ifndef CONFIG_USER_ONLY
bool armv7m_nvic_neg_prio_requested(NVICState *s, bool secure);
-#else
-static inline bool armv7m_nvic_neg_prio_requested(NVICState *s, bool secure)
-{
- return false;
-}
-#endif
-#ifndef CONFIG_USER_ONLY
bool armv7m_nvic_can_take_pending_exception(NVICState *s);
-#else
-static inline bool armv7m_nvic_can_take_pending_exception(NVICState *s)
-{
- return true;
-}
-#endif
#endif
I'm a bit worried we might have regression when doing things this way.
I expect link errors to diagnose errors.
In this case, yes.
More generally, for system vs user, it might be sufficient (even though I would
prefer to
be blindly prudent on this), but it might not protect all cases, with subtle
configs or
features enabled/disabled.
With a runtime check, we could identify the missing calls "feature_enabled()".
In this
case, we would link, but could end up call_feature in some cases, when it
should be hidden
behind a "_enabled()" check.
if (feature_enabled()) {
call_feature();
}
I'm not quite sure what you're arguing for here.
A build-time error is vastly preferable to a run-time error.
Even though this specific patch is safe (code calling those functions
should be under system anyway, so it should not be linked in a user
binary), I just wonder if we should not add explicit checks for this,
for other kind of replacement we'll have to do.
If it's a lesser used configuration or feature, a run-time error could lay
dormant for
years before a user encounters it.
Sure, but wouldn't it better to have an explicit assert, instead of
observing a random behaviour?
I'm just worried we end up calling something we should not (user vs
system, or any other ifdef CONFIG/TARGET that might be hidden
somewhere), and silently return a wrong value, which would not be
covered by our test suite.
r~
