Hi James,
On 8/22/25 16:30, James Morse wrote:
> MPAM supports more features than are going to be exposed to resctrl.
> For partid other than 0, the reset values of these controls isn't
> known.
>
> Discover the rest of the features so they can be reset to avoid any
> side effects when resctrl is in use.
>
> PARTID narrowing allows MSC/RIS to support less configuration space than
> is usable. If this feature is found on a class of device we are likely
> to use, then reduce the partid_max to make it usable. This allows us
> to map a PARTID to itself.
>
> CC: Rohit Mathew <Rohit.Mathew@xxxxxxx>
> CC: Zeng Heng <zengheng4@xxxxxxxxxx>
> CC: Dave Martin <Dave.Martin@xxxxxxx>
> Signed-off-by: James Morse <james.morse@xxxxxxx>
> ---
> drivers/resctrl/mpam_devices.c | 175 ++++++++++++++++++++++++++++++++
> drivers/resctrl/mpam_internal.h | 16 ++-
> 2 files changed, 189 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 8f6df2406c22..aedd743d6827 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -213,6 +213,15 @@ static void __mpam_part_sel(u8 ris_idx, u16 partid, struct mpam_msc *msc)
> __mpam_part_sel_raw(partsel, msc);
> }
>
> +static void __mpam_intpart_sel(u8 ris_idx, u16 intpartid, struct mpam_msc *msc)
> +{
> + u32 partsel = FIELD_PREP(MPAMCFG_PART_SEL_RIS, ris_idx) |
> + FIELD_PREP(MPAMCFG_PART_SEL_PARTID_SEL, intpartid) |
> + MPAMCFG_PART_SEL_INTERNAL;
> +
> + __mpam_part_sel_raw(partsel, msc);
> +}
> +
> int mpam_register_requestor(u16 partid_max, u8 pmg_max)
> {
> int err = 0;
> @@ -743,10 +752,35 @@ static void mpam_ris_hw_probe(struct mpam_msc_ris *ris)
> int err;
> struct mpam_msc *msc = ris->vmsc->msc;
> struct mpam_props *props = &ris->props;
> + struct mpam_class *class = ris->vmsc->comp->class;
>
> lockdep_assert_held(&msc->probe_lock);
> lockdep_assert_held(&msc->part_sel_lock);
>
> + /* Cache Capacity Partitioning */
> + if (FIELD_GET(MPAMF_IDR_HAS_CCAP_PART, ris->idr)) {
> + u32 ccap_features = mpam_read_partsel_reg(msc, CCAP_IDR);
> +
> + props->cmax_wd = FIELD_GET(MPAMF_CCAP_IDR_CMAX_WD, ccap_features);
> + if (props->cmax_wd &&
> + FIELD_GET(MPAMF_CCAP_IDR_HAS_CMAX_SOFTLIM, ccap_features))
> + mpam_set_feature(mpam_feat_cmax_softlim, props);
> +
> + if (props->cmax_wd &&
> + !FIELD_GET(MPAMF_CCAP_IDR_NO_CMAX, ccap_features))
> + mpam_set_feature(mpam_feat_cmax_cmax, props);
> +
> + if (props->cmax_wd &&
> + FIELD_GET(MPAMF_CCAP_IDR_HAS_CMIN, ccap_features))
> + mpam_set_feature(mpam_feat_cmax_cmin, props);
> +
> + props->cassoc_wd = FIELD_GET(MPAMF_CCAP_IDR_CASSOC_WD, ccap_features);
> +
> + if (props->cassoc_wd &&
> + FIELD_GET(MPAMF_CCAP_IDR_HAS_CASSOC, ccap_features))
> + mpam_set_feature(mpam_feat_cmax_cassoc, props);
> + }
> +
> /* Cache Portion partitioning */
> if (FIELD_GET(MPAMF_IDR_HAS_CPOR_PART, ris->idr)) {
> u32 cpor_features = mpam_read_partsel_reg(msc, CPOR_IDR);
> @@ -769,6 +803,31 @@ static void mpam_ris_hw_probe(struct mpam_msc_ris *ris)
> props->bwa_wd = FIELD_GET(MPAMF_MBW_IDR_BWA_WD, mbw_features);
> if (props->bwa_wd && FIELD_GET(MPAMF_MBW_IDR_HAS_MAX, mbw_features))
> mpam_set_feature(mpam_feat_mbw_max, props);
> +
> + if (props->bwa_wd && FIELD_GET(MPAMF_MBW_IDR_HAS_MIN, mbw_features))
> + mpam_set_feature(mpam_feat_mbw_min, props);
> +
> + if (props->bwa_wd && FIELD_GET(MPAMF_MBW_IDR_HAS_PROP, mbw_features))
> + mpam_set_feature(mpam_feat_mbw_prop, props);
> + }
> +
> + /* Priority partitioning */
> + if (FIELD_GET(MPAMF_IDR_HAS_PRI_PART, ris->idr)) {
> + u32 pri_features = mpam_read_partsel_reg(msc, PRI_IDR);
> +
> + props->intpri_wd = FIELD_GET(MPAMF_PRI_IDR_INTPRI_WD, pri_features);
> + if (props->intpri_wd && FIELD_GET(MPAMF_PRI_IDR_HAS_INTPRI, pri_features)) {
> + mpam_set_feature(mpam_feat_intpri_part, props);
> + if (FIELD_GET(MPAMF_PRI_IDR_INTPRI_0_IS_LOW, pri_features))
> + mpam_set_feature(mpam_feat_intpri_part_0_low, props);
> + }
> +
> + props->dspri_wd = FIELD_GET(MPAMF_PRI_IDR_DSPRI_WD, pri_features);
> + if (props->dspri_wd && FIELD_GET(MPAMF_PRI_IDR_HAS_DSPRI, pri_features)) {
> + mpam_set_feature(mpam_feat_dspri_part, props);
> + if (FIELD_GET(MPAMF_PRI_IDR_DSPRI_0_IS_LOW, pri_features))
> + mpam_set_feature(mpam_feat_dspri_part_0_low, props);
> + }
> }
>
> /* Performance Monitoring */
> @@ -832,6 +891,21 @@ static void mpam_ris_hw_probe(struct mpam_msc_ris *ris)
> */
> }
> }
> +
> + /*
> + * RIS with PARTID narrowing don't have enough storage for one
> + * configuration per PARTID. If these are in a class we could use,
> + * reduce the supported partid_max to match the number of intpartid.
> + * If the class is unknown, just ignore it.
> + */
> + if (FIELD_GET(MPAMF_IDR_HAS_PARTID_NRW, ris->idr) &&
> + class->type != MPAM_CLASS_UNKNOWN) {
> + u32 nrwidr = mpam_read_partsel_reg(msc, PARTID_NRW_IDR);
> + u16 partid_max = FIELD_GET(MPAMF_PARTID_NRW_IDR_INTPARTID_MAX, nrwidr);
> +
> + mpam_set_feature(mpam_feat_partid_nrw, props);
> + msc->partid_max = min(msc->partid_max, partid_max);
> + }
> }
>
> static int mpam_msc_hw_probe(struct mpam_msc *msc)
> @@ -929,13 +1003,29 @@ static void mpam_reset_msc_bitmap(struct mpam_msc *msc, u16 reg, u16 wd)
> static void mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 partid,
> struct mpam_config *cfg)
> {
> + u32 pri_val = 0;
> + u16 cmax = MPAMCFG_CMAX_CMAX;
> u16 bwa_fract = MPAMCFG_MBW_MAX_MAX;
> struct mpam_msc *msc = ris->vmsc->msc;
> struct mpam_props *rprops = &ris->props;
> + u16 dspri = GENMASK(rprops->dspri_wd, 0);
> + u16 intpri = GENMASK(rprops->intpri_wd, 0);
>
> mutex_lock(&msc->part_sel_lock);
> __mpam_part_sel(ris->ris_idx, partid, msc);
>
> + if (mpam_has_feature(mpam_feat_partid_nrw, rprops)) {
> + /* Update the intpartid mapping */
> + mpam_write_partsel_reg(msc, INTPARTID,
> + MPAMCFG_INTPARTID_INTERNAL | partid);
> +
> + /*
> + * Then switch to the 'internal' partid to update the
> + * configuration.
> + */
> + __mpam_intpart_sel(ris->ris_idx, partid, msc);
> + }
> +
> if (mpam_has_feature(mpam_feat_cpor_part, rprops)) {
> if (mpam_has_feature(mpam_feat_cpor_part, cfg))
> mpam_write_partsel_reg(msc, CPBM, cfg->cpbm);
> @@ -964,6 +1054,29 @@ static void mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 partid,
>
> if (mpam_has_feature(mpam_feat_mbw_prop, rprops))
> mpam_write_partsel_reg(msc, MBW_PROP, bwa_fract);
> +
> + if (mpam_has_feature(mpam_feat_cmax_cmax, rprops))
> + mpam_write_partsel_reg(msc, CMAX, cmax);
> +
> + if (mpam_has_feature(mpam_feat_cmax_cmin, rprops))
> + mpam_write_partsel_reg(msc, CMIN, 0);
Missing reset for cmax_cassoc. I wonder if it makes sense to have
separate enums for partitioning features, which require reset, and the rest.
> +
> + if (mpam_has_feature(mpam_feat_intpri_part, rprops) ||
> + mpam_has_feature(mpam_feat_dspri_part, rprops)) {
> + /* aces high? */
> + if (!mpam_has_feature(mpam_feat_intpri_part_0_low, rprops))
> + intpri = 0;
> + if (!mpam_has_feature(mpam_feat_dspri_part_0_low, rprops))
> + dspri = 0;
> +
> + if (mpam_has_feature(mpam_feat_intpri_part, rprops))
> + pri_val |= FIELD_PREP(MPAMCFG_PRI_INTPRI, intpri);
> + if (mpam_has_feature(mpam_feat_dspri_part, rprops))
> + pri_val |= FIELD_PREP(MPAMCFG_PRI_DSPRI, dspri);
> +
> + mpam_write_partsel_reg(msc, PRI, pri_val);
> + }
> +
> mutex_unlock(&msc->part_sel_lock);
> }
>
> @@ -1529,6 +1642,16 @@ static bool mpam_has_bwa_wd_feature(struct mpam_props *props)
> return false;
> }
>
> +/* Any of these features mean the CMAX_WD field is valid. */
> +static bool mpam_has_cmax_wd_feature(struct mpam_props *props)
> +{
> + if (mpam_has_feature(mpam_feat_cmax_cmax, props))
> + return true;
> + if (mpam_has_feature(mpam_feat_cmax_cmin, props))
> + return true;
> + return false;
> +}
> +
> #define MISMATCHED_HELPER(parent, child, helper, field, alias) \
> helper(parent) && \
> ((helper(child) && (parent)->field != (child)->field) || \
> @@ -1583,6 +1706,23 @@ static void __props_mismatch(struct mpam_props *parent,
> parent->bwa_wd = min(parent->bwa_wd, child->bwa_wd);
> }
>
> + if (alias && !mpam_has_cmax_wd_feature(parent) && mpam_has_cmax_wd_feature(child)) {
> + parent->cmax_wd = child->cmax_wd;
> + } else if (MISMATCHED_HELPER(parent, child, mpam_has_cmax_wd_feature,
> + cmax_wd, alias)) {
> + pr_debug("%s took the min cmax_wd\n", __func__);
> + parent->cmax_wd = min(parent->cmax_wd, child->cmax_wd);
> + }
> +
> + if (CAN_MERGE_FEAT(parent, child, mpam_feat_cmax_cassoc, alias)) {
> + parent->cassoc_wd = child->cassoc_wd;
> + } else if (MISMATCHED_FEAT(parent, child, mpam_feat_cmax_cassoc,
> + cassoc_wd, alias)) {
> + pr_debug("%s cleared cassoc_wd\n", __func__);
> + mpam_clear_feature(mpam_feat_cmax_cassoc, &parent->features);
> + parent->cassoc_wd = 0;
> + }
> +
> /* For num properties, take the minimum */
> if (CAN_MERGE_FEAT(parent, child, mpam_feat_msmon_csu, alias)) {
> parent->num_csu_mon = child->num_csu_mon;
> @@ -1600,6 +1740,41 @@ static void __props_mismatch(struct mpam_props *parent,
> parent->num_mbwu_mon = min(parent->num_mbwu_mon, child->num_mbwu_mon);
> }
>
> + if (CAN_MERGE_FEAT(parent, child, mpam_feat_intpri_part, alias)) {
> + parent->intpri_wd = child->intpri_wd;
> + } else if (MISMATCHED_FEAT(parent, child, mpam_feat_intpri_part,
> + intpri_wd, alias)) {
> + pr_debug("%s took the min intpri_wd\n", __func__);
> + parent->intpri_wd = min(parent->intpri_wd, child->intpri_wd);
> + }
> +
> + if (CAN_MERGE_FEAT(parent, child, mpam_feat_dspri_part, alias)) {
> + parent->dspri_wd = child->dspri_wd;
> + } else if (MISMATCHED_FEAT(parent, child, mpam_feat_dspri_part,
> + dspri_wd, alias)) {
> + pr_debug("%s took the min dspri_wd\n", __func__);
> + parent->dspri_wd = min(parent->dspri_wd, child->dspri_wd);
> + }
> +
> + /* TODO: alias support for these two */
> + /* {int,ds}pri may not have differing 0-low behaviour */
> + if (mpam_has_feature(mpam_feat_intpri_part, parent) &&
> + (!mpam_has_feature(mpam_feat_intpri_part, child) ||
> + mpam_has_feature(mpam_feat_intpri_part_0_low, parent) !=
> + mpam_has_feature(mpam_feat_intpri_part_0_low, child))) {
> + pr_debug("%s cleared intpri_part\n", __func__);
> + mpam_clear_feature(mpam_feat_intpri_part, &parent->features);
> + mpam_clear_feature(mpam_feat_intpri_part_0_low, &parent->features);
> + }
> + if (mpam_has_feature(mpam_feat_dspri_part, parent) &&
> + (!mpam_has_feature(mpam_feat_dspri_part, child) ||
> + mpam_has_feature(mpam_feat_dspri_part_0_low, parent) !=
> + mpam_has_feature(mpam_feat_dspri_part_0_low, child))) {
> + pr_debug("%s cleared dspri_part\n", __func__);
> + mpam_clear_feature(mpam_feat_dspri_part, &parent->features);
> + mpam_clear_feature(mpam_feat_dspri_part_0_low, &parent->features);
> + }
> +
> if (alias) {
> /* Merge features for aliased resources */
> parent->features |= child->features;
> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
> index 70cba9f22746..23445aedbabd 100644
> --- a/drivers/resctrl/mpam_internal.h
> +++ b/drivers/resctrl/mpam_internal.h
> @@ -157,16 +157,23 @@ static inline void mpam_mon_sel_lock_held(struct mpam_msc *msc)
> * When we compact the supported features, we don't care what they are.
> * Storing them as a bitmap makes life easy.
> */
> -typedef u16 mpam_features_t;
> +typedef u32 mpam_features_t;
>
> /* Bits for mpam_features_t */
> enum mpam_device_features {
> - mpam_feat_ccap_part = 0,
> + mpam_feat_cmax_softlim,
> + mpam_feat_cmax_cmax,
> + mpam_feat_cmax_cmin,
> + mpam_feat_cmax_cassoc,
> mpam_feat_cpor_part,
> mpam_feat_mbw_part,
> mpam_feat_mbw_min,
> mpam_feat_mbw_max,
> mpam_feat_mbw_prop,
> + mpam_feat_intpri_part,
> + mpam_feat_intpri_part_0_low,
> + mpam_feat_dspri_part,
> + mpam_feat_dspri_part_0_low,
> mpam_feat_msmon,
> mpam_feat_msmon_csu,
> mpam_feat_msmon_csu_capture,
> @@ -176,6 +183,7 @@ enum mpam_device_features {
> mpam_feat_msmon_mbwu_rwbw,
> mpam_feat_msmon_mbwu_hw_nrdy,
> mpam_feat_msmon_capt,
> + mpam_feat_partid_nrw,
> MPAM_FEATURE_LAST,
> };
> static_assert(BITS_PER_TYPE(mpam_features_t) >= MPAM_FEATURE_LAST);
> @@ -187,6 +195,10 @@ struct mpam_props {
> u16 cpbm_wd;
> u16 mbw_pbm_bits;
> u16 bwa_wd;
> + u16 cmax_wd;
> + u16 cassoc_wd;
> + u16 intpri_wd;
> + u16 dspri_wd;
> u16 num_csu_mon;
> u16 num_mbwu_mon;
> };
Thanks,
Ben
