| 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: <d6b4d9d5-f5bc-4a7d-a221-4451456fbbd3@arm.com>
Date: Thu, 6 Nov 2025 10:11:45 +0000
From: Ben Horgan <ben.horgan@....com>
To: Peter Newman <peternewman@...gle.com>,
"Shaopeng Tan (Fujitsu)" <tan.shaopeng@...itsu.com>
Cc: James Morse <james.morse@....com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
D Scott Phillips OS <scott@...amperecomputing.com>,
"carl@...amperecomputing.com" <carl@...amperecomputing.com>,
"lcherian@...vell.com" <lcherian@...vell.com>,
"bobo.shaobowang@...wei.com" <bobo.shaobowang@...wei.com>,
"baolin.wang@...ux.alibaba.com" <baolin.wang@...ux.alibaba.com>,
Jamie Iles <quic_jiles@...cinc.com>, Xin Hao <xhao@...ux.alibaba.com>,
"dfustini@...libre.com" <dfustini@...libre.com>,
"amitsinght@...vell.com" <amitsinght@...vell.com>,
David Hildenbrand <david@...hat.com>, Dave Martin <dave.martin@....com>,
Koba Ko <kobak@...dia.com>, Shanker Donthineni <sdonthineni@...dia.com>,
"fenghuay@...dia.com" <fenghuay@...dia.com>,
"baisheng.gao@...soc.com" <baisheng.gao@...soc.com>,
Jonathan Cameron <jonathan.cameron@...wei.com>, Rob Herring
<robh@...nel.org>, 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>,
Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Danilo Krummrich <dakr@...nel.org>, Jeremy Linton <jeremy.linton@....com>,
Gavin Shan <gshan@...hat.com>
Subject: Re: [PATCH v3 20/29] arm_mpam: Allow configuration to be applied and
restored during cpu online
Hi Shaopeng, Peter,
On 11/5/25 16:16, Peter Newman wrote:
> On Mon, Oct 27, 2025 at 9:48 AM Shaopeng Tan (Fujitsu)
> <tan.shaopeng@...itsu.com> wrote:
>>
>> Hello James,
>>
>>> When CPUs come online the MSC's original configuration should be restored.
>>>
>>> Add struct mpam_config to hold the configuration. This has a bitmap of
>>> features that were modified. Once the maximum partid is known, allocate a
>>> configuration array for each component, and reprogram each RIS configuration
>>> from this.
>>>
>>> CC: Dave Martin <Dave.Martin@....com>
>>> Signed-off-by: James Morse <james.morse@....com>
>>> Reviewed-by: Jonathan Cameron <jonathan.cameron@...wei.com>
>>> Reviewed-by: Ben Horgan <ben.horgan@....com>
>>> Tested-by: Fenghua Yu <fenghuay@...dia.com>
>>> ---
>>> Changes since v2:
>>> * Call mpam_init_reset_cfg() on alloated config as 0 is not longer correct.
>>> * init_garbage() on each config - the array has to be freed in one go, but
>>> otherwise this looks weird.
>>> * Use struct initialiser in mpam_init_reset_cfg(),
>>> * Moved int err definition.
>>> * Removed srcu lock taking based on squinting at the only caller.
>>> * Moved config reset to mpam_reset_component_cfg() for re-use in
>>> mpam_reset_component_locked(), previous memset() was not enough
>>> since zero
>>> no longer means reset.
>>>
>>> Changes since v1:
>>> * Switched entry_rcu to srcu versions.
>>>
>>> Changes since RFC:
>>> * Added a comment about the ordering around max_partid.
>>> * Allocate configurations after interrupts are registered to reduce churn.
>>> * Added mpam_assert_partid_sizes_fixed();
>>> * Make reset use an all-ones instead of zero config.
>>> ---
>>> drivers/resctrl/mpam_devices.c | 284
>>> +++++++++++++++++++++++++++++---
>>> drivers/resctrl/mpam_internal.h | 23 +++
>>> 2 files changed, 287 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
>>> index ab37ed1fb5de..e990ef67df5b 100644
>>> --- a/drivers/resctrl/mpam_devices.c
>>> +++ b/drivers/resctrl/mpam_devices.c
>>> @@ -118,6 +118,17 @@ static inline void init_garbage(struct mpam_garbage
>>> *garbage) {
>>> init_llist_node(&garbage->llist);
>>> }
>>> +
>>> +/*
>>> + * Once mpam is enabled, new requestors cannot further reduce the
>>> +available
>>> + * partid. Assert that the size is fixed, and new requestors will be
>>> +turned
>>> + * away.
>>> + */
>>> +static void mpam_assert_partid_sizes_fixed(void)
>>> +{
>>> + WARN_ON_ONCE(!partid_max_published);
>>> +}
>>> +
>>> static u32 __mpam_read_reg(struct mpam_msc *msc, u16 reg) {
>>> WARN_ON_ONCE(!cpumask_test_cpu(smp_processor_id(),
>>> &msc->accessibility)); @@ -366,12 +377,16 @@ static void
>>> mpam_class_destroy(struct mpam_class *class)
>>> add_to_garbage(class);
>>> }
>>>
>>> +static void __destroy_component_cfg(struct mpam_component *comp);
>>> +
>>> static void mpam_comp_destroy(struct mpam_component *comp) {
>>> struct mpam_class *class = comp->class;
>>>
>>> lockdep_assert_held(&mpam_list_lock);
>>>
>>> + __destroy_component_cfg(comp);
>>> +
>>> list_del_rcu(&comp->class_list);
>>> add_to_garbage(comp);
>>>
>>> @@ -812,48 +827,102 @@ static void mpam_reset_msc_bitmap(struct
>>> mpam_msc *msc, u16 reg, u16 wd)
>>> __mpam_write_reg(msc, reg, bm);
>>> }
>>>
>>> -static void mpam_reset_ris_partid(struct mpam_msc_ris *ris, u16 partid)
>>> +/* Called via IPI. Call while holding an SRCU reference */ static void
>>> +mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 partid,
>>> + struct mpam_config *cfg)
>>> {
>>> struct mpam_msc *msc = ris->vmsc->msc;
>>> struct mpam_props *rprops = &ris->props;
>>>
>>> - WARN_ON_ONCE(!srcu_read_lock_held((&mpam_srcu)));
>>> -
>>> 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_cpor_part, rprops) &&
>>> + mpam_has_feature(mpam_feat_cpor_part, cfg)) {
>>> + if (cfg->reset_cpbm)
>>> + mpam_reset_msc_bitmap(msc, MPAMCFG_CPBM,
>>> + rprops->cpbm_wd);
>>> + else
>>> + mpam_write_partsel_reg(msc, CPBM, cfg->cpbm);
>>> + }
>>>
>>> - 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_part, rprops) &&
>>> + mpam_has_feature(mpam_feat_mbw_part, cfg)) {
>>> + if (cfg->reset_mbw_pbm)
>>> + mpam_reset_msc_bitmap(msc,
>>> MPAMCFG_MBW_PBM,
>>> + rprops->mbw_pbm_bits);
>>> + else
>>> + mpam_write_partsel_reg(msc, MBW_PBM,
>>> cfg->mbw_pbm);
>>> + }
>>>
>>> - if (mpam_has_feature(mpam_feat_mbw_min, rprops))
>>> + if (mpam_has_feature(mpam_feat_mbw_min, rprops) &&
>>> + mpam_has_feature(mpam_feat_mbw_min, cfg))
>>> 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_max, rprops) &&
>>> + mpam_has_feature(mpam_feat_mbw_max, cfg))
>>> + mpam_write_partsel_reg(msc, MBW_MAX, cfg->mbw_max);
>>>
>>> mutex_unlock(&msc->part_sel_lock);
>>> }
>>>
>>> +struct reprogram_ris {
>>> + struct mpam_msc_ris *ris;
>>> + struct mpam_config *cfg;
>>> +};
>>> +
>>> +/* Call with MSC lock held */
>>> +static int mpam_reprogram_ris(void *_arg) {
>>> + u16 partid, partid_max;
>>> + struct reprogram_ris *arg = _arg;
>>> + struct mpam_msc_ris *ris = arg->ris;
>>> + struct mpam_config *cfg = arg->cfg;
>>> +
>>> + if (ris->in_reset_state)
>>> + return 0;
>>> +
>>> + spin_lock(&partid_max_lock);
>>> + partid_max = mpam_partid_max;
>>> + spin_unlock(&partid_max_lock);
>>> + for (partid = 0; partid <= partid_max + 1; partid++)
>>> + mpam_reprogram_ris_partid(ris, partid, cfg);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void mpam_init_reset_cfg(struct mpam_config *reset_cfg) {
>>> + *reset_cfg = (struct mpam_config) {
>>> + .cpbm = ~0,
>>> + .mbw_pbm = ~0,
>>> + .mbw_max = MPAMCFG_MBW_MAX_MAX,
>>
>> When rdtgroup_schemata_show() is called, the "cpbm" value is output to the schema file.
>> Since bitmap lengths are chip-dependent, I think we just need to reset the bitmap length portion.
>> Otherwise, 0xffffffff(u32) will be output from the schemata file.
>
> When I apply additional patches to add the mpam_resctrl.c stuff I
> notice this too:
>
> # grep L3 schemata
> L3:1=ffffffff
> # cat info/L3/shareable_bits
> ffff
>
> I noticed that new groups also get a too-long cbm as long as any other
> groups have a too-long cbm. Maybe this out-of-range value is bleeding
> into new groups in __init_one_rdt_domain() when it calls
> resctrl_arch_get_config() on all other groups.
>
> -Peter
mpam_init_reset_cfg() is used in places, one for ris reset and another
for component reset. In component reset these values persist and with
resctrl support added turn up in the schemata. More significantly, the
reset flags, reset_cpbm etc, are not reset to false and so the control
configuration doesn't take effect. I have an update for the component
case which I'll include when I repost this series for James.
Thanks,
Ben
Powered by blists - more mailing lists