When a CPU comes online, it may bring a newly accessible MSC with
it. Only the default partid has its value reset by hardware, and
even then the MSC might not have been reset since its config was
previously dirtyied. e.g. Kexec.
Any in-use partid must have its configuration restored, or reset.
In-use partids may be held in caches and evicted later.
MSC are also reset when CPUs are taken offline to cover cases where
firmware doesn't reset the MSC over reboot using UEFI, or kexec
where there is no firmware involvement.
If the configuration for a RIS has not been touched since it was
brought online, it does not need resetting again.
To reset, write the maximum values for all discovered controls.
CC: Rohit Mathew <Rohit.Mathew@arm.com>
Signed-off-by: James Morse <james.morse@arm.com>
---
Changes since RFC:
* Last bitmap write will always be non-zero.
* Dropped READ_ONCE() - teh value can no longer change.
* Write 0 to proporitional stride, remove the bwa_fract variable.
* Removed nested srcu lock, the assert should cover it.
---
drivers/resctrl/mpam_devices.c | 117 ++++++++++++++++++++++++++++++++
drivers/resctrl/mpam_internal.h | 8 +++
2 files changed, 125 insertions(+)
diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index cd8e95fa5fd6..0353313cf284 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -7,6 +7,7 @@
#include <linux/atomic.h>
#include <linux/arm_mpam.h>
#include <linux/bitfield.h>
+#include <linux/bitmap.h>
#include <linux/cacheinfo.h>
#include <linux/cpu.h>
#include <linux/cpumask.h>
@@ -777,8 +778,110 @@ static int mpam_msc_hw_probe(struct mpam_msc *msc)
return 0;
}
+static void mpam_reset_msc_bitmap(struct mpam_msc *msc, u16 reg, u16 wd)
+{
+ u32 num_words, msb;
+ u32 bm = ~0;
+ int i;
+
+ lockdep_assert_held(&msc->part_sel_lock);
+
+ if (wd == 0)
+ return;
+
+ /*
+ * Write all ~0 to all but the last 32bit-word, which may
+ * have fewer bits...
+ */
+ num_words = DIV_ROUND_UP(wd, 32);
+ for (i = 0; i < num_words - 1; i++, reg += sizeof(bm))
+ __mpam_write_reg(msc, reg, bm);
+
+ /*
+ * ....and then the last (maybe) partial 32bit word. When wd is a
+ * multiple of 32, msb should be 31 to write a full 32bit word.
+ */
+ msb = (wd - 1) % 32;
+ bm = GENMASK(msb, 0);
+ __mpam_write_reg(msc, reg, bm);
+}
+
+static void mpam_reset_ris_partid(struct mpam_msc_ris *ris, u16 partid)
+{
+ struct mpam_msc *msc = ris->vmsc->msc;
+ struct mpam_props *rprops = &ris->props;
+
+ mpam_assert_srcu_read_lock_held();
+
+ mutex_lock(&msc->part_sel_lock);
+ __mpam_part_sel(ris->ris_idx, partid, msc);
+
+ if (mpam_has_feature(mpam_feat_cpor_part, rprops))
+ mpam_reset_msc_bitmap(msc, MPAMCFG_CPBM, rprops->cpbm_wd);
+
+ if (mpam_has_feature(mpam_feat_mbw_part, rprops))
+ mpam_reset_msc_bitmap(msc, MPAMCFG_MBW_PBM, rprops->mbw_pbm_bits);
+
+ if (mpam_has_feature(mpam_feat_mbw_min, rprops))
+ mpam_write_partsel_reg(msc, MBW_MIN, 0);
+
+ if (mpam_has_feature(mpam_feat_mbw_max, rprops))
+ mpam_write_partsel_reg(msc, MBW_MAX, MPAMCFG_MBW_MAX_MAX);
+
+ if (mpam_has_feature(mpam_feat_mbw_prop, rprops))
+ mpam_write_partsel_reg(msc, MBW_PROP, 0);
+ mutex_unlock(&msc->part_sel_lock);
+}
+
+static void mpam_reset_ris(struct mpam_msc_ris *ris)
+{
+ u16 partid, partid_max;
+
+ mpam_assert_srcu_read_lock_held();
+
+ if (ris->in_reset_state)
+ return;
+
+ spin_lock(&partid_max_lock);
+ partid_max = mpam_partid_max;
+ spin_unlock(&partid_max_lock);
+ for (partid = 0; partid < partid_max; partid++)
+ mpam_reset_ris_partid(ris, partid);
+}
+
+static void mpam_reset_msc(struct mpam_msc *msc, bool online)
+{
+ struct mpam_msc_ris *ris;
+
+ mpam_assert_srcu_read_lock_held();
+
+ list_for_each_entry_srcu(ris, &msc->ris, msc_list, srcu_read_lock_held(&mpam_srcu)) {
+ mpam_reset_ris(ris);
+
+ /*
+ * Set in_reset_state when coming online. The reset state
+ * for non-zero partid may be lost while the CPUs are offline.
+ */
+ ris->in_reset_state = online;
+ }
+}
+
static int mpam_cpu_online(unsigned int cpu)
{
+ int idx;
+ struct mpam_msc *msc;
+
+ idx = srcu_read_lock(&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;
+
+ if (atomic_fetch_inc(&msc->online_refs) == 0)
+ mpam_reset_msc(msc, true);
+ }
+ srcu_read_unlock(&mpam_srcu, idx);
+
return 0;
}
@@ -818,6 +921,20 @@ static int mpam_discovery_cpu_online(unsigned int cpu)
static int mpam_cpu_offline(unsigned int cpu)
{
+ int idx;
+ struct mpam_msc *msc;
+
+ idx = srcu_read_lock(&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;
+
+ if (atomic_dec_and_test(&msc->online_refs))
+ mpam_reset_msc(msc, false);
+ }
+ srcu_read_unlock(&mpam_srcu, idx);
+
return 0;
}
diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
index eace5ba871f3..6e047fbd3512 100644
--- a/drivers/resctrl/mpam_internal.h
+++ b/drivers/resctrl/mpam_internal.h
@@ -5,6 +5,7 @@
#define MPAM_INTERNAL_H
#include <linux/arm_mpam.h>
+#include <linux/atomic.h>
#include <linux/cpumask.h>
#include <linux/io.h>
#include <linux/llist.h>
@@ -45,6 +46,7 @@ struct mpam_msc {
struct pcc_mbox_chan *pcc_chan;
u32 nrdy_usec;
cpumask_t accessibility;
+ atomic_t online_refs;
/*
* probe_lock is only taken during discovery. After discovery these
@@ -223,6 +225,7 @@ struct mpam_msc_ris {
u8 ris_idx;
u64 idr;
struct mpam_props props;
+ bool in_reset_state;
cpumask_t affinity;
@@ -242,6 +245,11 @@ struct mpam_msc_ris {
extern struct srcu_struct mpam_srcu;
extern struct list_head mpam_classes;
+static inline void mpam_assert_srcu_read_lock_held(void)
+{
+ WARN_ON_ONCE(!srcu_read_lock_held((&mpam_srcu)));
+}
+
/* System wide partid/pmg values */
extern u16 mpam_partid_max;
extern u8 mpam_pmg_max;
--
2.39.5On Wed, 10 Sep 2025 20:42:55 +0000
James Morse <james.morse@arm.com> wrote:
> When a CPU comes online, it may bring a newly accessible MSC with
> it. Only the default partid has its value reset by hardware, and
> even then the MSC might not have been reset since its config was
> previously dirtyied. e.g. Kexec.
>
> Any in-use partid must have its configuration restored, or reset.
> In-use partids may be held in caches and evicted later.
>
> MSC are also reset when CPUs are taken offline to cover cases where
> firmware doesn't reset the MSC over reboot using UEFI, or kexec
> where there is no firmware involvement.
>
> If the configuration for a RIS has not been touched since it was
> brought online, it does not need resetting again.
>
> To reset, write the maximum values for all discovered controls.
>
> CC: Rohit Mathew <Rohit.Mathew@arm.com>
> Signed-off-by: James Morse <james.morse@arm.com>
Just one trivial passing comment from me.
Jonathan
> ---
> Changes since RFC:
> * Last bitmap write will always be non-zero.
> * Dropped READ_ONCE() - teh value can no longer change.
> * Write 0 to proporitional stride, remove the bwa_fract variable.
> * Removed nested srcu lock, the assert should cover it.
> ---
> drivers/resctrl/mpam_devices.c | 117 ++++++++++++++++++++++++++++++++
> drivers/resctrl/mpam_internal.h | 8 +++
> 2 files changed, 125 insertions(+)
>
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index cd8e95fa5fd6..0353313cf284 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
>
> @@ -818,6 +921,20 @@ static int mpam_discovery_cpu_online(unsigned int cpu)
>
> static int mpam_cpu_offline(unsigned int cpu)
> {
> + int idx;
> + struct mpam_msc *msc;
> +
> + idx = srcu_read_lock(&mpam_srcu);
Might be worth using
guard(srcu)(&mpam_srcu);
here but only real advantage it bring is in hiding the local idx variable
away.
> + 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;
> +
> + if (atomic_dec_and_test(&msc->online_refs))
> + mpam_reset_msc(msc, false);
> + }
> + srcu_read_unlock(&mpam_srcu, idx);
> +
> return 0;
> }
Hi Jonathan,
On 12/09/2025 12:55, Jonathan Cameron wrote:
> On Wed, 10 Sep 2025 20:42:55 +0000
> James Morse <james.morse@arm.com> wrote:
>
>> When a CPU comes online, it may bring a newly accessible MSC with
>> it. Only the default partid has its value reset by hardware, and
>> even then the MSC might not have been reset since its config was
>> previously dirtyied. e.g. Kexec.
>>
>> Any in-use partid must have its configuration restored, or reset.
>> In-use partids may be held in caches and evicted later.
>>
>> MSC are also reset when CPUs are taken offline to cover cases where
>> firmware doesn't reset the MSC over reboot using UEFI, or kexec
>> where there is no firmware involvement.
>>
>> If the configuration for a RIS has not been touched since it was
>> brought online, it does not need resetting again.
>>
>> To reset, write the maximum values for all discovered controls.
> Just one trivial passing comment from me.
>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
>> index cd8e95fa5fd6..0353313cf284 100644
>> --- a/drivers/resctrl/mpam_devices.c
>> +++ b/drivers/resctrl/mpam_devices.c
>> @@ -818,6 +921,20 @@ static int mpam_discovery_cpu_online(unsigned int cpu)
>>
>> static int mpam_cpu_offline(unsigned int cpu)
>> {
>> + int idx;
>> + struct mpam_msc *msc;
>> +
>> + idx = srcu_read_lock(&mpam_srcu);
> Might be worth using
> guard(srcu)(&mpam_srcu);
> here but only real advantage it bring is in hiding the local idx variable
> away.
Sure. I did that in a few other places, it looks like I either missed this, or thought it
got more complicated later...
Thanks,
James
Hi James,
On 9/10/25 21:42, James Morse wrote:
> When a CPU comes online, it may bring a newly accessible MSC with
> it. Only the default partid has its value reset by hardware, and
> even then the MSC might not have been reset since its config was
> previously dirtyied. e.g. Kexec.
>
> Any in-use partid must have its configuration restored, or reset.
> In-use partids may be held in caches and evicted later.
>
> MSC are also reset when CPUs are taken offline to cover cases where
> firmware doesn't reset the MSC over reboot using UEFI, or kexec
> where there is no firmware involvement.
>
> If the configuration for a RIS has not been touched since it was
> brought online, it does not need resetting again.
>
> To reset, write the maximum values for all discovered controls.
>
> CC: Rohit Mathew <Rohit.Mathew@arm.com>
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
> Changes since RFC:
> * Last bitmap write will always be non-zero.
> * Dropped READ_ONCE() - teh value can no longer change.
> * Write 0 to proporitional stride, remove the bwa_fract variable.
> * Removed nested srcu lock, the assert should cover it.
> ---
> drivers/resctrl/mpam_devices.c | 117 ++++++++++++++++++++++++++++++++
> drivers/resctrl/mpam_internal.h | 8 +++
> 2 files changed, 125 insertions(+)
>
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index cd8e95fa5fd6..0353313cf284 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -7,6 +7,7 @@
> #include <linux/atomic.h>
> #include <linux/arm_mpam.h>
> #include <linux/bitfield.h>
> +#include <linux/bitmap.h>
> #include <linux/cacheinfo.h>
> #include <linux/cpu.h>
> #include <linux/cpumask.h>
> @@ -777,8 +778,110 @@ static int mpam_msc_hw_probe(struct mpam_msc *msc)
> return 0;
> }
>
> +static void mpam_reset_msc_bitmap(struct mpam_msc *msc, u16 reg, u16 wd)
> +{
> + u32 num_words, msb;
> + u32 bm = ~0;
> + int i;
> +
> + lockdep_assert_held(&msc->part_sel_lock);
> +
> + if (wd == 0)
> + return;
> +
> + /*
> + * Write all ~0 to all but the last 32bit-word, which may
> + * have fewer bits...
> + */
> + num_words = DIV_ROUND_UP(wd, 32);
> + for (i = 0; i < num_words - 1; i++, reg += sizeof(bm))
> + __mpam_write_reg(msc, reg, bm);
> +
> + /*
> + * ....and then the last (maybe) partial 32bit word. When wd is a
> + * multiple of 32, msb should be 31 to write a full 32bit word.
> + */
> + msb = (wd - 1) % 32;
> + bm = GENMASK(msb, 0);
> + __mpam_write_reg(msc, reg, bm);
> +}
> +
> +static void mpam_reset_ris_partid(struct mpam_msc_ris *ris, u16 partid)
> +{
> + struct mpam_msc *msc = ris->vmsc->msc;
> + struct mpam_props *rprops = &ris->props;
> +
> + mpam_assert_srcu_read_lock_held();
> +
> + mutex_lock(&msc->part_sel_lock);
> + __mpam_part_sel(ris->ris_idx, partid, msc);
> +
> + if (mpam_has_feature(mpam_feat_cpor_part, rprops))
> + mpam_reset_msc_bitmap(msc, MPAMCFG_CPBM, rprops->cpbm_wd);
> +
> + if (mpam_has_feature(mpam_feat_mbw_part, rprops))
> + mpam_reset_msc_bitmap(msc, MPAMCFG_MBW_PBM, rprops->mbw_pbm_bits);
> +
> + if (mpam_has_feature(mpam_feat_mbw_min, rprops))
> + mpam_write_partsel_reg(msc, MBW_MIN, 0);
> +
> + if (mpam_has_feature(mpam_feat_mbw_max, rprops))
> + mpam_write_partsel_reg(msc, MBW_MAX, MPAMCFG_MBW_MAX_MAX);
> +
> + if (mpam_has_feature(mpam_feat_mbw_prop, rprops))
> + mpam_write_partsel_reg(msc, MBW_PROP, 0);
If mpam_feat_ccap_part is already in enum mpam_device_features then the
reset would belong here but I expect it is better just to introduce
mpam_feat_ccap_part later (patch 21). I also commented on this feature
introduction split on patch 13.
> + mutex_unlock(&msc->part_sel_lock);
> +}
> +
> +static void mpam_reset_ris(struct mpam_msc_ris *ris)
> +{
> + u16 partid, partid_max;
> +
> + mpam_assert_srcu_read_lock_held();
> +
> + if (ris->in_reset_state)
> + return;
> +
> + spin_lock(&partid_max_lock);
> + partid_max = mpam_partid_max;
> + spin_unlock(&partid_max_lock);
> + for (partid = 0; partid < partid_max; partid++)
> + mpam_reset_ris_partid(ris, partid);
> +}
> +
> +static void mpam_reset_msc(struct mpam_msc *msc, bool online)
> +{
> + struct mpam_msc_ris *ris;
> +
> + mpam_assert_srcu_read_lock_held();
Unneeded? Checked in list_for_each_entry_srcu().> +
> + list_for_each_entry_srcu(ris, &msc->ris, msc_list, srcu_read_lock_held(&mpam_srcu)) {
> + mpam_reset_ris(ris);
> +
> + /*
> + * Set in_reset_state when coming online. The reset state
> + * for non-zero partid may be lost while the CPUs are offline.
> + */
> + ris->in_reset_state = online;
> + }
> +}
> +
> static int mpam_cpu_online(unsigned int cpu)
> {
> + int idx;
> + struct mpam_msc *msc;
> +
> + idx = srcu_read_lock(&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;
> +
> + if (atomic_fetch_inc(&msc->online_refs) == 0)
> + mpam_reset_msc(msc, true);
> + }
> + srcu_read_unlock(&mpam_srcu, idx);
> +
> return 0;
> }
>
> @@ -818,6 +921,20 @@ static int mpam_discovery_cpu_online(unsigned int cpu)
>
> static int mpam_cpu_offline(unsigned int cpu)
> {
> + int idx;
> + struct mpam_msc *msc;
> +
> + idx = srcu_read_lock(&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;
> +
> + if (atomic_dec_and_test(&msc->online_refs))
> + mpam_reset_msc(msc, false);
> + }
> + srcu_read_unlock(&mpam_srcu, idx);
> +
> return 0;
> }
>
> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
> index eace5ba871f3..6e047fbd3512 100644
> --- a/drivers/resctrl/mpam_internal.h
> +++ b/drivers/resctrl/mpam_internal.h
> @@ -5,6 +5,7 @@
> #define MPAM_INTERNAL_H
>
> #include <linux/arm_mpam.h>
> +#include <linux/atomic.h>
> #include <linux/cpumask.h>
> #include <linux/io.h>
> #include <linux/llist.h>
> @@ -45,6 +46,7 @@ struct mpam_msc {
> struct pcc_mbox_chan *pcc_chan;
> u32 nrdy_usec;
> cpumask_t accessibility;
> + atomic_t online_refs;
>
> /*
> * probe_lock is only taken during discovery. After discovery these
> @@ -223,6 +225,7 @@ struct mpam_msc_ris {
> u8 ris_idx;
> u64 idr;
> struct mpam_props props;
> + bool in_reset_state;
>
> cpumask_t affinity;
>
> @@ -242,6 +245,11 @@ struct mpam_msc_ris {
> extern struct srcu_struct mpam_srcu;
> extern struct list_head mpam_classes;
>
> +static inline void mpam_assert_srcu_read_lock_held(void)
> +{
> + WARN_ON_ONCE(!srcu_read_lock_held((&mpam_srcu)));
> +}
> +
> /* System wide partid/pmg values */
> extern u16 mpam_partid_max;
> extern u8 mpam_pmg_max;
Thanks,
Ben
Hi Ben,
On 12/09/2025 12:25, Ben Horgan wrote:
> On 9/10/25 21:42, James Morse wrote:
>> When a CPU comes online, it may bring a newly accessible MSC with
>> it. Only the default partid has its value reset by hardware, and
>> even then the MSC might not have been reset since its config was
>> previously dirtyied. e.g. Kexec.
>>
>> Any in-use partid must have its configuration restored, or reset.
>> In-use partids may be held in caches and evicted later.
>>
>> MSC are also reset when CPUs are taken offline to cover cases where
>> firmware doesn't reset the MSC over reboot using UEFI, or kexec
>> where there is no firmware involvement.
>>
>> If the configuration for a RIS has not been touched since it was
>> brought online, it does not need resetting again.
>>
>> To reset, write the maximum values for all discovered controls.
>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
>> index cd8e95fa5fd6..0353313cf284 100644
>> --- a/drivers/resctrl/mpam_devices.c
>> +++ b/drivers/resctrl/mpam_devices.c
>> @@ -777,8 +778,110 @@ static int mpam_msc_hw_probe(struct mpam_msc *msc)
>> +static void mpam_reset_ris_partid(struct mpam_msc_ris *ris, u16 partid)
>> +{
>> + struct mpam_msc *msc = ris->vmsc->msc;
>> + struct mpam_props *rprops = &ris->props;
>> +
>> + mpam_assert_srcu_read_lock_held();
>> +
>> + mutex_lock(&msc->part_sel_lock);
>> + __mpam_part_sel(ris->ris_idx, partid, msc);
>> +
>> + if (mpam_has_feature(mpam_feat_cpor_part, rprops))
>> + mpam_reset_msc_bitmap(msc, MPAMCFG_CPBM, rprops->cpbm_wd);
>> +
>> + if (mpam_has_feature(mpam_feat_mbw_part, rprops))
>> + mpam_reset_msc_bitmap(msc, MPAMCFG_MBW_PBM, rprops->mbw_pbm_bits);
>> +
>> + if (mpam_has_feature(mpam_feat_mbw_min, rprops))
>> + mpam_write_partsel_reg(msc, MBW_MIN, 0);
>> +
>> + if (mpam_has_feature(mpam_feat_mbw_max, rprops))
>> + mpam_write_partsel_reg(msc, MBW_MAX, MPAMCFG_MBW_MAX_MAX);
>> +
>> + if (mpam_has_feature(mpam_feat_mbw_prop, rprops))
>> + mpam_write_partsel_reg(msc, MBW_PROP, 0);
> If mpam_feat_ccap_part is already in enum mpam_device_features then the
> reset would belong here but I expect it is better just to introduce
> mpam_feat_ccap_part later (patch 21). I also commented on this feature
> introduction split on patch 13.
Yup, this is some knock on cleanup from that change.
(I didn't understand what you meant the first time round!)
>> + mutex_unlock(&msc->part_sel_lock);
>> +}
>> +static void mpam_reset_msc(struct mpam_msc *msc, bool online)
>> +{
>> + struct mpam_msc_ris *ris;
>> +
>> + mpam_assert_srcu_read_lock_held();
> Unneeded? Checked in list_for_each_entry_srcu().> +
So it is! I'll rip those out. They were mostly for documentation anyway.
There will likely be a few others of these in the series...
>> + list_for_each_entry_srcu(ris, &msc->ris, msc_list, srcu_read_lock_held(&mpam_srcu)) {
>> + mpam_reset_ris(ris);
>> +
>> + /*
>> + * Set in_reset_state when coming online. The reset state
>> + * for non-zero partid may be lost while the CPUs are offline.
>> + */
>> + ris->in_reset_state = online;
>> + }
>> +}
>> +
Thanks,
James
On 9/12/25 12:25, Ben Horgan wrote:
> Hi James,
>
> On 9/10/25 21:42, James Morse wrote:
>> When a CPU comes online, it may bring a newly accessible MSC with
>> it. Only the default partid has its value reset by hardware, and
>> even then the MSC might not have been reset since its config was
>> previously dirtyied. e.g. Kexec.
>>
>> Any in-use partid must have its configuration restored, or reset.
>> In-use partids may be held in caches and evicted later.
>>
>> MSC are also reset when CPUs are taken offline to cover cases where
>> firmware doesn't reset the MSC over reboot using UEFI, or kexec
>> where there is no firmware involvement.
>>
>> If the configuration for a RIS has not been touched since it was
>> brought online, it does not need resetting again.
>>
>> To reset, write the maximum values for all discovered controls.
>>
>> CC: Rohit Mathew <Rohit.Mathew@arm.com>
>> Signed-off-by: James Morse <james.morse@arm.com>
>> ---
>> Changes since RFC:
>> * Last bitmap write will always be non-zero.
>> * Dropped READ_ONCE() - teh value can no longer change.
>> * Write 0 to proporitional stride, remove the bwa_fract variable.
>> * Removed nested srcu lock, the assert should cover it.
>> ---
>> drivers/resctrl/mpam_devices.c | 117 ++++++++++++++++++++++++++++++++
>> drivers/resctrl/mpam_internal.h | 8 +++
>> 2 files changed, 125 insertions(+)
>>
>> +
>> +static void mpam_reset_msc(struct mpam_msc *msc, bool online)
>> +{
>> + struct mpam_msc_ris *ris;
>> +
>> + mpam_assert_srcu_read_lock_held();
>
> Unneeded? Checked in list_for_each_entry_srcu().> +
If you do get rid of this then that leaves one use of the helper,
mpam_assert_srcu_read_lock_held(), and so the helper could go.
> Thanks,
>
> Ben
>
>
Thanks,
Ben
Hi Ben,
On 12/09/2025 15:52, Ben Horgan wrote:
> On 9/12/25 12:25, Ben Horgan wrote:
>> Hi James,
>>
>> On 9/10/25 21:42, James Morse wrote:
>>> When a CPU comes online, it may bring a newly accessible MSC with
>>> it. Only the default partid has its value reset by hardware, and
>>> even then the MSC might not have been reset since its config was
>>> previously dirtyied. e.g. Kexec.
>>>
>>> Any in-use partid must have its configuration restored, or reset.
>>> In-use partids may be held in caches and evicted later.
>>>
>>> MSC are also reset when CPUs are taken offline to cover cases where
>>> firmware doesn't reset the MSC over reboot using UEFI, or kexec
>>> where there is no firmware involvement.
>>>
>>> If the configuration for a RIS has not been touched since it was
>>> brought online, it does not need resetting again.
>>>
>>> To reset, write the maximum values for all discovered controls.
>>> +static void mpam_reset_msc(struct mpam_msc *msc, bool online)
>>> +{
>>> + struct mpam_msc_ris *ris;
>>> +
>>> + mpam_assert_srcu_read_lock_held();
>>
>> Unneeded? Checked in list_for_each_entry_srcu().> +
>
> If you do get rid of this then that leaves one use of the helper,
> mpam_assert_srcu_read_lock_held(), and so the helper could go.
By the end of the series, yes. But there are transiently a few more until then - they get
removed and replaced with comments when those functions get called by IPI as lockdep
expects the lock to be held by current, which isn't true if you IPI'd.
I'll drop the helper.
Thanks,
James
© 2016 - 2025 Red Hat, Inc.