Hi Ben,

On 24/07/2025 16:08, Ben Horgan wrote:

On 7/11/25 19:36, James Morse wrote:

quoted

Expand the probing support with the control and monitor types
we can use with resctrl.

quoted

diff --git a/drivers/platform/arm64/mpam/mpam_devices.c b/drivers/platform/arm64/mpam/

mpam_devices.c
index 8646fb85ad09..61911831ab39 100644

--- a/drivers/platform/arm64/mpam/mpam_devices.c
+++ b/drivers/platform/arm64/mpam/mpam_devices.c

quoted

@@ -645,6 +659,137 @@ static struct mpam_msc_ris *mpam_get_or_create_ris(struct mpam_msc

*msc,
      return found;
  }
  +/*
+ * IHI009A.a has this nugget: "If a monitor does not support automatic behaviour
+ * of NRDY, software can use this bit for any purpose" - so hardware might not
+ * implement this - but it isn't RES0.
+ *
+ * Try and see what values stick in this bit. If we can write either value,
+ * its probably not implemented by hardware.
+ */
+#define mpam_ris_hw_probe_hw_nrdy(_ris, _mon_reg, _result)            \
+do {                                        \
+    u32 now;                                \
+    u64 mon_sel;                                \
+    bool can_set, can_clear;                        \
+    struct mpam_msc *_msc = _ris->vmsc->msc;                \
+                                        \
+    if (WARN_ON_ONCE(!mpam_mon_sel_inner_lock(_msc))) {            \
+        _result = false;                        \
+        break;                                \
+    }                                    \
+    mon_sel = FIELD_PREP(MSMON_CFG_MON_SEL_MON_SEL, 0) |            \
+          FIELD_PREP(MSMON_CFG_MON_SEL_RIS, _ris->ris_idx);        \
+    mpam_write_monsel_reg(_msc, CFG_MON_SEL, mon_sel);            \
+                                        \
+    mpam_write_monsel_reg(_msc, _mon_reg, MSMON___NRDY);            \
+    now = mpam_read_monsel_reg(_msc, _mon_reg);                \
+    can_set = now & MSMON___NRDY;                        \
+                                        \
+    mpam_write_monsel_reg(_msc, _mon_reg, 0);                \
+    now = mpam_read_monsel_reg(_msc, _mon_reg);                \
+    can_clear = !(now & MSMON___NRDY);                    \
+    mpam_mon_sel_inner_unlock(_msc);                    \
+                                        \
+    _result = (!can_set || !can_clear);                    \
+} while (0)

It is a bit surprising that something that looks like a function modifies a boolean passed
by value.

Consider continuing the pattern you have above:
#define mpam_ris_hw_probe_hw_nrdy(_ris, _mon_reg, _result)
_mpam_ris_hw_probe_hw_nrdy(_ris, MSMON##_mon_reg, _result)

with signature:
void _mpam_ris_hw_probe_hw_nrdy(struct mpam_msc *msc, u16 reg, bool *hw_managed);

and using the _mpam functions from the new _mpam_ris_hw_probe_hw_nrdy().

With that, it may as well return the result.
Done.


Thanks,

James