[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CALPaoChLKRQqjZO+O92WQ=MsWjV+q=hVE8=BXCOdkta6ZEXNMQ@mail.gmail.com>
Date: Wed, 5 Nov 2025 17:16:24 +0100
From: Peter Newman <peternewman@...gle.com>
To: "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>,
Ben Horgan <ben.horgan@....com>
Subject: Re: [PATCH v3 20/29] arm_mpam: Allow configuration to be applied and
restored during cpu online
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
Powered by blists - more mailing lists