[v1,13/14] arm_mpam: Handle resource instances mapped to different controls
Commit Message
At the moment, configuring multiple resource instances (mapped to same
control) under a resource class is not supported. For instance, on MARVELL
SoC MPAMF_IDR_NS[RIS_MAX] (under LLC MSC) is 0x2, and there are three
different resource at index 0,1,2. These are enumerated in
TAD_CMN_MPAM_RIS_E:
0: MSC
1: LTG
2: DTG
LLC MSC resource at index 0 has the Priority partitioning features.
If MPAMCFG_PART_SEL_NS[RIS] is set to 0 (MSC), then MPAMF_IDR_NS[HAS_PRI_PART]
is set to 1, leaving HAS_CPOR_PART bit to 0. CPOR and PRI_PART are
mutually exclusive resources as far configuration is concerned.
With this change, multiple resource instances that maps to different
control, say LTG for CPOR, and MSC for PRI_PART is handled properly.
Signed-off-by: Amit Singh Tomar <amitsinght@marvell.com>
---
Changes since RFC:
* Trimmed down the commit message.
* Tried to handle the corner case as suggested by Jonathan
by calling the __resource_props_mismatch even when different
controls are enumerated at different RIS index.
---
drivers/platform/mpam/mpam_devices.c | 36 ++++++++++++++++++++++++----
1 file changed, 31 insertions(+), 5 deletions(-)
Comments
Hi Amit,
On Wed, Jan 17, 2024 at 6:15 AM Amit Singh Tomar <amitsinght@marvell.com> wrote:
>
> +/* Club different resource properties under a class that resctrl uses,
> + * for instance, L3 cache that supports both CPOR, and DSPRI need to have
> + * knowledge of both cpbm_wd and dspri_wd. This is needed when two controls
> + * are enumerated under differnt RIS Index.
> + */
> +static void mpam_enable_club_class_features(struct mpam_class *class,
> + struct mpam_msc_ris *ris)
It looks like "club" is used as a synonym to "class" here to evade the
bigger issue that mpam_classes are not defined correctly as DSPRI
resources should not be in the same mpam_class as the L3 CPOR and CSU
features.
This hardware makes it clear that the definition of mpam_class as all
resources in a (level x {memory,cache}) needs to be revised.
-Peter
Hi Peter,
Thanks for having a look.
-----Original Message-----
From: Peter Newman <peternewman@google.com>
Sent: Wednesday, January 17, 2024 11:33 PM
To: Amit Singh Tomar <amitsinght@marvell.com>
Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; fenghua.yu@intel.com; reinette.chatre@intel.com; james.morse@arm.com; George Cherian <gcherian@marvell.com>; robh@kernel.org; dfustini@baylibre.com; jonathan.cameron@huawei.com
Subject: [EXT] Re: [PATCH v1 13/14] arm_mpam: Handle resource instances mapped to different controls
External Email
----------------------------------------------------------------------
Hi Amit,
On Wed, Jan 17, 2024 at 6:15 AM Amit Singh Tomar <amitsinght@marvell.com> wrote:
>
> +/* Club different resource properties under a class that resctrl
> +uses,
> + * for instance, L3 cache that supports both CPOR, and DSPRI need to
> +have
> + * knowledge of both cpbm_wd and dspri_wd. This is needed when two
> +controls
> + * are enumerated under differnt RIS Index.
> + */
> +static void mpam_enable_club_class_features(struct mpam_class *class,
> + struct mpam_msc_ris *ris)
It looks like "club" is used as a synonym to "class" here to evade the bigger issue that mpam_classes are not defined correctly as DSPRI resources should not be in the same mpam_class as the L3 CPOR and CSU features.
This hardware makes it clear that the definition of mpam_class as all resources in a (level x {memory,cache}) needs to be revised.
On Marvell platform, DSPRI control register (MPAMCFG_PRI_NS), and Identification Register (MPAMF_PRI_IDR_NS) are implemented within the LLC MPAM block (the address range contains control and identification registers for CPOR, and DSPRI), and therefore we treat DSPRI as one of the L3 resource. However, suppose we approach it as totally different standalone resource type (PRI) other than Cache storage resource type (CPOR, and CCAP), and define a new MPAM class type for it, there is no standard way to discover this new resource type (PRI) from ACPI tables.
I'm concerned about accessing DSPRI related registers, if we are going to tide it to new MPAM class (as we discover whole L3 MPAM block using firmware tables, and tide it's resources to L3 MPAM class).
Thanks
-Amit
[CC Tony Luck]
Hi Amit,
On Fri, Jan 19, 2024 at 5:01 AM Amit Singh Tomar <amitsinght@marvell.com> wrote:
>
> Hi Peter,
>
> Thanks for having a look.
>
> -----Original Message-----
> From: Peter Newman <peternewman@google.com>
> Sent: Wednesday, January 17, 2024 11:33 PM
> To: Amit Singh Tomar <amitsinght@marvell.com>
> Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; fenghua.yu@intel.com; reinette.chatre@intel.com; james.morse@arm.com; George Cherian <gcherian@marvell.com>; robh@kernel.org; dfustini@baylibre.com; jonathan.cameron@huawei.com
> Subject: [EXT] Re: [PATCH v1 13/14] arm_mpam: Handle resource instances mapped to different controls
>
> External Email
>
> ----------------------------------------------------------------------
> Hi Amit,
>
> On Wed, Jan 17, 2024 at 6:15 AM Amit Singh Tomar <amitsinght@marvell.com> wrote:
>
> >
> > +/* Club different resource properties under a class that resctrl
> > +uses,
> > + * for instance, L3 cache that supports both CPOR, and DSPRI need to
> > +have
> > + * knowledge of both cpbm_wd and dspri_wd. This is needed when two
> > +controls
> > + * are enumerated under differnt RIS Index.
> > + */
> > +static void mpam_enable_club_class_features(struct mpam_class *class,
> > + struct mpam_msc_ris *ris)
>
> It looks like "club" is used as a synonym to "class" here to evade the bigger issue that mpam_classes are not defined correctly as DSPRI resources should not be in the same mpam_class as the L3 CPOR and CSU features.
>
> This hardware makes it clear that the definition of mpam_class as all resources in a (level x {memory,cache}) needs to be revised.
>
> On Marvell platform, DSPRI control register (MPAMCFG_PRI_NS), and Identification Register (MPAMF_PRI_IDR_NS) are implemented within the LLC MPAM block (the address range contains control and identification registers for CPOR, and DSPRI), and therefore we treat DSPRI as one of the L3 resource. However, suppose we approach it as totally different standalone resource type (PRI) other than Cache storage resource type (CPOR, and CCAP), and define a new MPAM class type for it, there is no standard way to discover this new resource type (PRI) from ACPI tables.
>
> I'm concerned about accessing DSPRI related registers, if we are going to tide it to new MPAM class (as we discover whole L3 MPAM block using firmware tables, and tide it's resources to L3 MPAM class).
This is becoming more of a discussion of MPAM (and resctrl) in
general, so I hope James can participate. Also I should point out that
when discussing MPAM, "resource" refers to a non-RIS MSC or a single
RIS-index on a RIS-enabled MSC, while the "mpam_class" structure in
the code is the counterpart to what RDT (and resctrl) call a resource.
From my reading of the code, the consequence of (RIS) resources being
in the same mpam_class is that they can be programmed uniformly
through a single schema line in the schemata file, so
__resource_props_mismatch() goes to work on eliminating any resources
(and extra control granularity) which are not programmed exactly the
same way. This function seems more geared toward big.LITTLE systems
where the cache controls on one cluster are dissimilar from those on
the peer cluster and would need to be normalized (James, can you
confirm?). But in this situation, it seems like a better idea to
present a separate schema for one cluster's controls from the others.
For example, "L2P" and "L2E", with non-overlapping domains.
In the case of an MSC implementing RIS, the controls are independent
by definition, so I can't see why the work done in
__resource_props_mismatch() would be applicable.
-Peter
@@ -1788,11 +1788,15 @@ __resource_props_mismatch(struct mpam_msc_ris *ris, struct mpam_class *class)
lockdep_assert_held(&mpam_list_lock); /* we modify class */
- /* Clear missing features */
- cprops->features &= rprops->features;
+ /* Clear missing features, we don't want to clear features,
+ * when different controls are implemented at different RIS
+ * index.
+ */
+ if (class->props.features == ris->props.features)
+ cprops->features &= rprops->features;
/* Clear incompatible features */
- if (cprops->cpbm_wd != rprops->cpbm_wd)
+ if (cprops->cpbm_wd != rprops->cpbm_wd && rprops->cpbm_wd)
mpam_clear_feature(mpam_feat_cpor_part, &cprops->features);
if (cprops->mbw_pbm_bits != rprops->mbw_pbm_bits)
mpam_clear_feature(mpam_feat_mbw_part, &cprops->features);
@@ -1802,14 +1806,14 @@ __resource_props_mismatch(struct mpam_msc_ris *ris, struct mpam_class *class)
cprops->bwa_wd = min(cprops->bwa_wd, rprops->bwa_wd);
/* For num properties, take the minimum */
- if (cprops->num_csu_mon != rprops->num_csu_mon)
+ if (cprops->num_csu_mon != rprops->num_csu_mon && rprops->num_csu_mon)
cprops->num_csu_mon = min(cprops->num_csu_mon, rprops->num_csu_mon);
if (cprops->num_mbwu_mon != rprops->num_mbwu_mon)
cprops->num_mbwu_mon = min(cprops->num_mbwu_mon, rprops->num_mbwu_mon);
if (cprops->intpri_wd != rprops->intpri_wd)
cprops->intpri_wd = min(cprops->intpri_wd, rprops->intpri_wd);
- if (cprops->dspri_wd != rprops->dspri_wd)
+ if (cprops->dspri_wd != rprops->dspri_wd && rprops->dspri_wd)
cprops->dspri_wd = min(cprops->dspri_wd, rprops->dspri_wd);
/* {int,ds}pri may not have differing 0-low behaviour */
@@ -1845,6 +1849,20 @@ static void mpam_enable_init_class_features(struct mpam_class *class)
class->props = ris->props;
}
+/* Club different resource properties under a class that resctrl uses,
+ * for instance, L3 cache that supports both CPOR, and DSPRI need to have
+ * knowledge of both cpbm_wd and dspri_wd. This is needed when two controls
+ * are enumerated under differnt RIS Index.
+ */
+static void mpam_enable_club_class_features(struct mpam_class *class,
+ struct mpam_msc_ris *ris)
+{
+ class->props.features |= ris->props.features;
+ class->props.cpbm_wd |= ris->props.cpbm_wd;
+ class->props.dspri_wd |= ris->props.dspri_wd;
+ class->props.num_csu_mon |= ris->props.num_csu_mon;
+}
+
/* Merge all the common resource features into class. */
static void mpam_enable_merge_features(void)
{
@@ -1860,6 +1878,14 @@ static void mpam_enable_merge_features(void)
list_for_each_entry(comp, &class->components, class_list) {
list_for_each_entry(ris, &comp->ris, comp_list) {
__resource_props_mismatch(ris, class);
+ /*
+ * There can be multiple resources under a class which is
+ * mapped to different controls, For instance L3 cache
+ * can have both CPOR and DSPRI implemented with different
+ * RIS.
+ */
+ if (class->props.features != ris->props.features)
+ mpam_enable_club_class_features(class, ris);
class->nrdy_usec = max(class->nrdy_usec,
ris->msc->nrdy_usec);