Re: [RFC PATCH 17/36] arm_mpam: Add cpuhp callbacks to probe MSC hardware

[Ben Horgan]

Hi James,

On 11/07/2025 19:36, James Morse 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.
>
> Later once MPAM is enabled, this cpuhp callback will be replaced by
> one that avoids the global list.
>
> Enabling a static key will also take the cpuhp lock, so can't be done
> from the cpuhp callback. Whenever a new MSC has been probed schedule
> work to test if all the MSCs have now been probed.
>
> CC: Lecopzer Chen <lecopzerc@xxxxxxxxxx>
> Signed-off-by: James Morse <james.morse@xxxxxxx>
> ---
>   drivers/platform/arm64/mpam/mpam_devices.c  | 149 +++++++++++++++++++-
>   drivers/platform/arm64/mpam/mpam_internal.h |   8 +-
>   2 files changed, 152 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/platform/arm64/mpam/mpam_devices.c b/drivers/platform/arm64/mpam/mpam_devices.c
> index 0d6d5180903b..89434ae3efa6 100644
> --- a/drivers/platform/arm64/mpam/mpam_devices.c
> +++ b/drivers/platform/arm64/mpam/mpam_devices.c
> @@ -4,6 +4,7 @@
>   #define pr_fmt(fmt) "%s:%s: " fmt, KBUILD_MODNAME, __func__
>   
>   #include <linux/acpi.h>
> +#include <linux/atomic.h>
>   #include <linux/arm_mpam.h>
>   #include <linux/cacheinfo.h>
>   #include <linux/cpu.h>
> @@ -21,6 +22,7 @@
>   #include <linux/slab.h>
>   #include <linux/spinlock.h>
>   #include <linux/types.h>
> +#include <linux/workqueue.h>
>   
>   #include <acpi/pcc.h>
>   
> @@ -39,6 +41,16 @@ struct srcu_struct mpam_srcu;
>   /* MPAM isn't available until all the MSC have been probed. */
>   static u32 mpam_num_msc;
>   
> +static int mpam_cpuhp_state;
> +static DEFINE_MUTEX(mpam_cpuhp_state_lock);
> +
> +/*
> + * mpam is enabled once all devices have been probed from CPU online callbacks,
> + * scheduled via this work_struct. If access to an MSC depends on a CPU that
> + * was not brought online at boot, this can happen surprisingly late.
> + */
> +static DECLARE_WORK(mpam_enable_work, &mpam_enable);
> +
>   /*
>    * An MSC is a physical container for controls and monitors, each identified by
>    * their RIS index. These share a base-address, interrupts and some MMIO
> @@ -78,6 +90,22 @@ LIST_HEAD(mpam_classes);
>   /* List of all objects that can be free()d after synchronise_srcu() */
>   static LLIST_HEAD(mpam_garbage);
>   
> +static u32 __mpam_read_reg(struct mpam_msc *msc, u16 reg)
> +{
> +	WARN_ON_ONCE(reg > msc->mapped_hwpage_sz);
> +	WARN_ON_ONCE(!cpumask_test_cpu(smp_processor_id(), &msc->accessibility));
> +
> +	return readl_relaxed(msc->mapped_hwpage + reg);
> +}
> +
> +static inline u32 _mpam_read_partsel_reg(struct mpam_msc *msc, u16 reg)
> +{
> +	lockdep_assert_held_once(&msc->part_sel_lock);
> +	return __mpam_read_reg(msc, reg);
> +}
> +
> +#define mpam_read_partsel_reg(msc, reg)        _mpam_read_partsel_reg(msc, MPAMF_##reg)
> +
>   #define init_garbage(x)	init_llist_node(&(x)->garbage.llist)
>   
>   static struct mpam_vmsc *
> @@ -513,9 +541,84 @@ int mpam_ris_create(struct mpam_msc *msc, u8 ris_idx,
>   	return err;
>   }
>   
> -static void mpam_discovery_complete(void)
> +static int mpam_msc_hw_probe(struct mpam_msc *msc)
>   {
> -	pr_err("Discovered all MSC\n");
> +	u64 idr;
> +	int err;
> +
> +	lockdep_assert_held(&msc->probe_lock);
> +
> +	mutex_lock(&msc->part_sel_lock);
> +	idr = mpam_read_partsel_reg(msc, AIDR);
> +	if ((idr & MPAMF_AIDR_ARCH_MAJOR_REV) != MPAM_ARCHITECTURE_V1) {
> +		pr_err_once("%s does not match MPAM architecture v1.0\n",
> +			    dev_name(&msc->pdev->dev));
The error message need only mention the major revision. You've added 
support for v1.1 and v1.0.> +		err = -EIO;
> +	} else {
> +		msc->probed = true;
> +		err = 0;
> +	}
> +	mutex_unlock(&msc->part_sel_lock);
> +
> +	return err;
> +}
[snip]
Thanks,

Ben