On Wed, 10 Sep 2025 20:42:50 +0000
James Morse <james.morse@xxxxxxx> wrote:
> Because an MSC can only by accessed from the CPUs in its cpu-affinity
> set we need to be running on one of those CPUs to probe the MSC
> hardware.
>
> Do this work in the cpuhp callback. Probing the hardware will only
> happen before MPAM is enabled, walk all the MSCs and probe those we can
> reach that haven't already been probed as each CPU's online call is made.
>
> This adds the low-level MSC register accessors.
>
> Once all MSCs reported by the firmware have been probed from a CPU in
> their respective cpu-affinity set, the probe-time cpuhp callbacks are
> replaced. The replacement callbacks will ultimately need to handle
> save/restore of the runtime MSC state across power transitions, but for
> now there is nothing to do in them: so do nothing.
>
> The architecture's context switch code will be enabled by a static-key,
> this can be set by mpam_enable(), but must be done from process context,
> not a cpuhp callback because both take the cpuhp lock.
> Whenever a new MSC has been probed, the mpam_enable() work is scheduled
> to test if all the MSCs have been probed. If probing fails, mpam_disable()
> is scheduled to unregister the cpuhp callbacks and free memory.
>
> CC: Lecopzer Chen <lecopzerc@xxxxxxxxxx>
> Signed-off-by: James Morse <james.morse@xxxxxxx>
Trivial suggestion inline. Either way
Reviewed-by: Jonathan Cameron <jonathan.cameron@xxxxxxxxxx>
> +
> +/* Before mpam is enabled, try to probe new MSC */
> +static int mpam_discovery_cpu_online(unsigned int cpu)
> +{
> + int err = 0;
> + struct mpam_msc *msc;
> + bool new_device_probed = false;
> +
> + guard(srcu)(&mpam_srcu);
> + list_for_each_entry_srcu(msc, &mpam_all_msc, all_msc_list,
> + srcu_read_lock_held(&mpam_srcu)) {
> + if (!cpumask_test_cpu(cpu, &msc->accessibility))
> + continue;
> +
> + mutex_lock(&msc->probe_lock);
> + if (!msc->probed)
> + err = mpam_msc_hw_probe(msc);
> + mutex_unlock(&msc->probe_lock);
> +
> + if (!err)
> + new_device_probed = true;
> + else
> + break;
Unless this going to get more complex why not
if (err)
break;
new_device_probed = true;
> + }
> +
> + if (new_device_probed && !err)
> + schedule_work(&mpam_enable_work);
> + if (err) {
> + mpam_disable_reason = "error during probing";
> + schedule_work(&mpam_broken_work);
> + }
> +
> + return err;
> +}
> +static void mpam_enable_once(void)
> +{
> + mpam_register_cpuhp_callbacks(mpam_cpu_online, mpam_cpu_offline);
> +
> + pr_info("MPAM enabled\n");
Feels too noisy given it should be easy enough to tell. pr_dbg() perhaps.
> +}
