| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAL_JsqK5HZdyq_6rQibtmDE_H=gy+3aeCSCioA5MHR5+Gh_drQ@mail.gmail.com>
Date: Wed, 27 Aug 2025 11:08:10 -0500
From: Rob Herring <robh@...nel.org>
To: James Morse <james.morse@....com>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-acpi@...r.kernel.org, devicetree@...r.kernel.org,
shameerali.kolothum.thodi@...wei.com,
D Scott Phillips OS <scott@...amperecomputing.com>, carl@...amperecomputing.com,
lcherian@...vell.com, bobo.shaobowang@...wei.com, tan.shaopeng@...itsu.com,
baolin.wang@...ux.alibaba.com, Jamie Iles <quic_jiles@...cinc.com>,
Xin Hao <xhao@...ux.alibaba.com>, peternewman@...gle.com, dfustini@...libre.com,
amitsinght@...vell.com, David Hildenbrand <david@...hat.com>,
Rex Nie <rex.nie@...uarmicro.com>, Dave Martin <dave.martin@....com>,
Koba Ko <kobak@...dia.com>, Shanker Donthineni <sdonthineni@...dia.com>, fenghuay@...dia.com,
baisheng.gao@...soc.com, Jonathan Cameron <jonathan.cameron@...wei.com>,
Rohit Mathew <rohit.mathew@....com>, Rafael Wysocki <rafael@...nel.org>, Len Brown <lenb@...nel.org>,
Lorenzo Pieralisi <lpieralisi@...nel.org>, Hanjun Guo <guohanjun@...wei.com>,
Sudeep Holla <sudeep.holla@....com>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Danilo Krummrich <dakr@...nel.org>, Lecopzer Chen <lecopzerc@...dia.com>
Subject: Re: [PATCH 14/33] arm_mpam: Add cpuhp callbacks to probe MSC hardware
On Fri, Aug 22, 2025 at 10:32 AM James Morse <james.morse@....com> 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@...dia.com>
> Signed-off-by: James Morse <james.morse@....com>
> ---
> drivers/resctrl/mpam_devices.c | 144 +++++++++++++++++++++++++++++++-
> drivers/resctrl/mpam_internal.h | 8 +-
> 2 files changed, 147 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 5baf2a8786fb..9d6516f98acf 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/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));
These either make __mpam_read_reg uninlined or add 2 checks to every
register read. Neither seems very good.
> +
> + 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);
Similar thing here.
> + 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 *
> @@ -511,9 +539,84 @@ int mpam_ris_create(struct mpam_msc *msc, u8 ris_idx,
> return err;
> }
>
> -static void mpam_discovery_complete(void)
It is annoying to review things which disappear in later patches...
> +static int mpam_msc_hw_probe(struct mpam_msc *msc)
> +{
> + u64 idr;
> + int err;
> +
> + lockdep_assert_held(&msc->probe_lock);
> +
> + mutex_lock(&msc->part_sel_lock);
> + idr = mpam_read_partsel_reg(msc, AIDR);
I don't think AIDR access depends on PART_SEL.
> + if ((idr & MPAMF_AIDR_ARCH_MAJOR_REV) != MPAM_ARCHITECTURE_V1) {
> + pr_err_once("%s does not match MPAM architecture v1.x\n",
> + dev_name(&msc->pdev->dev));
> + err = -EIO;
> + } else {
> + msc->probed = true;
> + err = 0;
> + }
> + mutex_unlock(&msc->part_sel_lock);
> +
> + return err;
> +}
> +
> +static int mpam_cpu_online(unsigned int cpu)
> {
> - pr_err("Discovered all MSC\n");
> + return 0;
> +}
> +
> +/* 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;
> +
> + mutex_lock(&mpam_list_lock);
> + list_for_each_entry(msc, &mpam_all_msc, glbl_list) {
> + 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; // mpam_broken
> + }
> + mutex_unlock(&mpam_list_lock);
> +
> + if (new_device_probed && !err)
> + schedule_work(&mpam_enable_work);
> +
> + return err;
> +}
> +
> +static int mpam_cpu_offline(unsigned int cpu)
> +{
> + return 0;
> +}
> +
> +static void mpam_register_cpuhp_callbacks(int (*online)(unsigned int online),
> + int (*offline)(unsigned int offline))
> +{
> + mutex_lock(&mpam_cpuhp_state_lock);
> + if (mpam_cpuhp_state) {
> + cpuhp_remove_state(mpam_cpuhp_state);
> + mpam_cpuhp_state = 0;
> + }
> +
> + mpam_cpuhp_state = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "mpam:online",
> + online, offline);
> + if (mpam_cpuhp_state <= 0) {
> + pr_err("Failed to register cpuhp callbacks");
> + mpam_cpuhp_state = 0;
> + }
> + mutex_unlock(&mpam_cpuhp_state_lock);
> }
>
> static int mpam_dt_count_msc(void)
> @@ -772,7 +875,7 @@ static int mpam_msc_drv_probe(struct platform_device *pdev)
> }
>
> if (!err && fw_num_msc == mpam_num_msc)
> - mpam_discovery_complete();
> + mpam_register_cpuhp_callbacks(&mpam_discovery_cpu_online, NULL);
>
> if (err && msc)
> mpam_msc_drv_remove(pdev);
> @@ -795,6 +898,41 @@ static struct platform_driver mpam_msc_driver = {
> .remove = mpam_msc_drv_remove,
> };
>
> +static void mpam_enable_once(void)
> +{
> + mpam_register_cpuhp_callbacks(mpam_cpu_online, mpam_cpu_offline);
> +
> + pr_info("MPAM enabled\n");
> +}
> +
> +/*
> + * Enable mpam once all devices have been probed.
> + * Scheduled by mpam_discovery_cpu_online() once all devices have been created.
> + * Also scheduled when new devices are probed when new CPUs come online.
> + */
> +void mpam_enable(struct work_struct *work)
> +{
> + static atomic_t once;
> + struct mpam_msc *msc;
> + bool all_devices_probed = true;
> +
> + /* Have we probed all the hw devices? */
> + mutex_lock(&mpam_list_lock);
> + list_for_each_entry(msc, &mpam_all_msc, glbl_list) {
> + mutex_lock(&msc->probe_lock);
> + if (!msc->probed)
> + all_devices_probed = false;
> + mutex_unlock(&msc->probe_lock);
> +
> + if (!all_devices_probed)
> + break;
> + }
> + mutex_unlock(&mpam_list_lock);
> +
> + if (all_devices_probed && !atomic_fetch_inc(&once))
> + mpam_enable_once();
> +}
> +
> /*
> * MSC that are hidden under caches are not created as platform devices
> * as there is no cache driver. Caches are also special-cased in
> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
> index 6e0982a1a9ac..a98cca08a2ef 100644
> --- a/drivers/resctrl/mpam_internal.h
> +++ b/drivers/resctrl/mpam_internal.h
> @@ -49,6 +49,7 @@ struct mpam_msc {
> * properties become read-only and the lists are protected by SRCU.
> */
> struct mutex probe_lock;
> + bool probed;
> unsigned long ris_idxs[128 / BITS_PER_LONG];
> u32 ris_max;
>
> @@ -59,14 +60,14 @@ struct mpam_msc {
> * part_sel_lock protects access to the MSC hardware registers that are
> * affected by MPAMCFG_PART_SEL. (including the ID registers that vary
> * by RIS).
> - * If needed, take msc->lock first.
> + * If needed, take msc->probe_lock first.
Humm. I think this belongs in patch 10.
> */
> struct mutex part_sel_lock;
>
> /*
> * mon_sel_lock protects access to the MSC hardware registers that are
> * affeted by MPAMCFG_MON_SEL.
> - * If needed, take msc->lock first.
> + * If needed, take msc->probe_lock first.
> */
> struct mutex outer_mon_sel_lock;
> raw_spinlock_t inner_mon_sel_lock;
> @@ -147,6 +148,9 @@ struct mpam_msc_ris {
> extern struct srcu_struct mpam_srcu;
> extern struct list_head mpam_classes;
>
> +/* Scheduled work callback to enable mpam once all MSC have been probed */
> +void mpam_enable(struct work_struct *work);
> +
> int mpam_get_cpumask_from_cache_id(unsigned long cache_id, u32 cache_level,
> cpumask_t *affinity);
>
> --
> 2.20.1
>
Powered by blists - more mailing lists