Message-ID: <b7f48e6b-2b48-4946-8ab6-fae7a3fec061@arm.com>
Date: Mon, 28 Jul 2025 10:15:25 +0100
From: Ben Horgan <ben.horgan@....com>
To: James Morse <james.morse@....com>, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org
Cc: Rob Herring <robh@...nel.org>, Rohit Mathew <rohit.mathew@....com>,
 Shanker Donthineni <sdonthineni@...dia.com>, Zeng Heng
 <zengheng4@...wei.com>, Lecopzer Chen <lecopzerc@...dia.com>,
 Carl Worth <carl@...amperecomputing.com>,
 shameerali.kolothum.thodi@...wei.com,
 D Scott Phillips OS <scott@...amperecomputing.com>, lcherian@...vell.com,
 bobo.shaobowang@...wei.com, tan.shaopeng@...itsu.com,
 baolin.wang@...ux.alibaba.com, Jamie Iles <quic_jiles@...cinc.com>,
 Xin Hao <xhao@...ux.alibaba.com>, peternewman@...gle.com,
 dfustini@...libre.com, amitsinght@...vell.com,
 David Hildenbrand <david@...hat.com>, Rex Nie <rex.nie@...uarmicro.com>,
 Dave Martin <dave.martin@....com>, Koba Ko <kobak@...dia.com>
Subject: Re: [RFC PATCH 21/36] arm_mpam: Merge supported features during
 mpam_enable() into mpam_class

Hi James,

On 7/11/25 19:36, James Morse wrote:
> To make a decision about whether to expose an mpam class as
> a resctrl resource we need to know its overall supported
> features and properties.
> 
> Once we've probed all the resources, we can walk the tree
> and produced overall values by merging the bitmaps. This
nit: s/produced/produce/
> eliminates features that are only supported by some MSC
> that make up a component or class.
> 
> If bitmap properties are mismatched within a component we
> cannot support the mismatched feature.
> 
> Care has to be taken as vMSC may hold mismatched RIS.
> 
> Signed-off-by: James Morse <james.morse@....com>
> ---
>   drivers/platform/arm64/mpam/mpam_devices.c  | 215 ++++++++++++++++++++
>   drivers/platform/arm64/mpam/mpam_internal.h |   8 +
>   2 files changed, 223 insertions(+)
> 
> diff --git a/drivers/platform/arm64/mpam/mpam_devices.c b/drivers/platform/arm64/mpam/mpam_devices.c
> index 61911831ab39..7b042a35405a 100644
> --- a/drivers/platform/arm64/mpam/mpam_devices.c
> +++ b/drivers/platform/arm64/mpam/mpam_devices.c
> @@ -1186,8 +1186,223 @@ static struct platform_driver mpam_msc_driver = {
>   	.remove = mpam_msc_drv_remove,
>   };
>   
> +/* Any of these features mean the BWA_WD field is valid. */
> +static bool mpam_has_bwa_wd_feature(struct mpam_props *props)
> +{
> +	if (mpam_has_feature(mpam_feat_mbw_min, props))
> +		return true;
> +	if (mpam_has_feature(mpam_feat_mbw_max, props))
> +		return true;
> +	if (mpam_has_feature(mpam_feat_mbw_prop, props))
> +		return true;
> +	return false;
> +}
> +
> +#define MISMATCHED_HELPER(parent, child, helper, field, alias)		\
> +	helper(parent) &&						\
> +	((helper(child) && (parent)->field != (child)->field) ||	\
> +	 (!helper(child) && !(alias)))
> +
> +#define MISMATCHED_FEAT(parent, child, feat, field, alias)		     \
> +	mpam_has_feature((feat), (parent)) &&				     \
> +	((mpam_has_feature((feat), (child)) && (parent)->field != (child)->field) || \
> +	 (!mpam_has_feature((feat), (child)) && !(alias)))
> +
> +#define CAN_MERGE_FEAT(parent, child, feat, alias)			\
> +	(alias) && !mpam_has_feature((feat), (parent)) &&		\
> +	mpam_has_feature((feat), (child))
> +
> +/*
> + * Combime two props fields.
nit: s/combime/combine/
> + * If this is for controls that alias the same resource, it is safe to just
> + * copy the values over. If two aliasing controls implement the same scheme
> + * a safe value must be picked.
> + * For non-aliasing controls, these control different resources, and the
> + * resulting safe value must be compatible with both. When merging values in
> + * the tree, all the aliasing resources must be handled first.
> + * On mismatch, parent is modified.
> + */
> +static void __props_mismatch(struct mpam_props *parent,
> +			     struct mpam_props *child, bool alias)
> +{
> +	if (CAN_MERGE_FEAT(parent, child, mpam_feat_cpor_part, alias)) {
> +		parent->cpbm_wd = child->cpbm_wd;
> +	} else if (MISMATCHED_FEAT(parent, child, mpam_feat_cpor_part,
> +				   cpbm_wd, alias)) {
> +		pr_debug("%s cleared cpor_part\n", __func__);
> +		mpam_clear_feature(mpam_feat_cpor_part, &parent->features);
> +		parent->cpbm_wd = 0;
> +	}
> +
> +	if (CAN_MERGE_FEAT(parent, child, mpam_feat_mbw_part, alias)) {
> +		parent->mbw_pbm_bits = child->mbw_pbm_bits;
> +	} else if (MISMATCHED_FEAT(parent, child, mpam_feat_mbw_part,
> +				   mbw_pbm_bits, alias)) {
> +		pr_debug("%s cleared mbw_part\n", __func__);
> +		mpam_clear_feature(mpam_feat_mbw_part, &parent->features);
> +		parent->mbw_pbm_bits = 0;
> +	}
> +
> +	/* bwa_wd is a count of bits, fewer bits means less precision */
> +	if (alias && !mpam_has_bwa_wd_feature(parent) && mpam_has_bwa_wd_feature(child)) {
> +		parent->bwa_wd = child->bwa_wd;
> +	} else if (MISMATCHED_HELPER(parent, child, mpam_has_bwa_wd_feature,
> +				     bwa_wd, alias)) {
> +		pr_debug("%s took the min bwa_wd\n", __func__);
> +		parent->bwa_wd = min(parent->bwa_wd, child->bwa_wd);
> +	}
> +
> +	/* 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;
> +	} else if (MISMATCHED_FEAT(parent, child, mpam_feat_msmon_csu,
> +				   num_csu_mon, alias)) {
> +		pr_debug("%s took the min num_csu_mon\n", __func__);
> +		parent->num_csu_mon = min(parent->num_csu_mon, child->num_csu_mon);
> +	}
> +
> +	if (CAN_MERGE_FEAT(parent, child, mpam_feat_msmon_mbwu, alias)) {
> +		parent->num_mbwu_mon = child->num_mbwu_mon;
> +	} else if (MISMATCHED_FEAT(parent, child, mpam_feat_msmon_mbwu,
> +				   num_mbwu_mon, alias)) {
> +		pr_debug("%s took the min num_mbwu_mon\n", __func__);
> +		parent->num_mbwu_mon = min(parent->num_mbwu_mon, child->num_mbwu_mon);
> +	}
> +
> +	if (alias) {
> +		/* Merge features for aliased resources */
> +		parent->features |= child->features;
> +	} else {
> +		/* Clear missing features for non aliasing */
> +		parent->features &= child->features;
> +	}
> +}
> +
> +/*
> + * If a vmsc doesn't match class feature/configuration, do the right thing(tm).
> + * For 'num' properties we can just take the minimum.
> + * For properties where the mismatched unused bits would make a difference, we
> + * nobble the class feature, as we can't configure all the resources.
> + * e.g. The L3 cache is composed of two resources with 13 and 17 portion
> + * bitmaps respectively.
> + */
> +static void
> +__class_props_mismatch(struct mpam_class *class, struct mpam_vmsc *vmsc)
> +{
> +	struct mpam_props *cprops = &class->props;
> +	struct mpam_props *vprops = &vmsc->props;
> +
> +	lockdep_assert_held(&mpam_list_lock); /* we modify class */
> +
> +	pr_debug("%s: Merging features for class:0x%lx &= vmsc:0x%lx\n",
> +		 dev_name(&vmsc->msc->pdev->dev),
> +		 (long)cprops->features, (long)vprops->features);
> +
> +	/* Take the safe value for any common features */
> +	__props_mismatch(cprops, vprops, false);
> +}
> +
> +static void
> +__vmsc_props_mismatch(struct mpam_vmsc *vmsc, struct mpam_msc_ris *ris)
> +{
> +	struct mpam_props *rprops = &ris->props;
> +	struct mpam_props *vprops = &vmsc->props;
> +
> +	lockdep_assert_held(&mpam_list_lock); /* we modify vmsc */
> +
> +	pr_debug("%s: Merging features for vmsc:0x%lx |= ris:0x%lx\n",
> +		 dev_name(&vmsc->msc->pdev->dev),
> +		 (long)vprops->features, (long)rprops->features);
> +
> +	/*
> +	 * Merge mismatched features - Copy any features that aren't common,
> +	 * but take the safe value for any common features.
> +	 */
> +	__props_mismatch(vprops, rprops, true);
> +}
> +
> +/*
> + * Copy the first component's first vMSC's properties and features to the
> + * class. __class_props_mismatch() will remove conflicts.
> + * It is not possible to have a class with no components, or a component with
> + * no resources. The vMSC properties have already been built.
> + */
> +static void mpam_enable_init_class_features(struct mpam_class *class)
> +{
> +	struct mpam_vmsc *vmsc;
> +	struct mpam_component *comp;
> +
> +	comp = list_first_entry_or_null(&class->components,
> +					struct mpam_component, class_list);
> +	if (WARN_ON(!comp))
> +		return;
> +
> +	vmsc = list_first_entry_or_null(&comp->vmsc,
> +					struct mpam_vmsc, comp_list);
> +	if (WARN_ON(!vmsc))
> +		return;
> +
> +	class->props = vmsc->props;
> +}
> +
> +static void mpam_enable_merge_vmsc_features(struct mpam_component *comp)
> +{
> +	struct mpam_vmsc *vmsc;
> +	struct mpam_msc_ris *ris;
> +	struct mpam_class *class = comp->class;
> +
> +	list_for_each_entry(vmsc, &comp->vmsc, comp_list) {
> +		list_for_each_entry(ris, &vmsc->ris, vmsc_list) {
> +			__vmsc_props_mismatch(vmsc, ris);
> +			class->nrdy_usec = max(class->nrdy_usec,
> +					       vmsc->msc->nrdy_usec);
> +		}
> +	}
> +}
> +
> +static void mpam_enable_merge_class_features(struct mpam_component *comp)
> +{
> +	struct mpam_vmsc *vmsc;
> +	struct mpam_class *class = comp->class;
> +
> +	list_for_each_entry(vmsc, &comp->vmsc, comp_list)
> +		__class_props_mismatch(class, vmsc);
> +}
> +
> +/*
> + * Merge all the common resource features into class.
> + * vmsc features are bitwise-or'd together, this must be done first.
> + * Next the class features are the bitwise-and of all the vmsc features.
> + * Other features are the min/max as appropriate.
> + *
> + * To avoid walking the whole tree twice, the class->nrdy_usec property is
> + * updated when working with the vmsc as it is a max(), and doesn't need
> + * initialising first.
> + */
> +static void mpam_enable_merge_features(struct list_head *all_classes_list)
> +{
> +	struct mpam_class *class;
> +	struct mpam_component *comp;
> +
> +	lockdep_assert_held(&mpam_list_lock);
> +
> +	list_for_each_entry(class, all_classes_list, classes_list) {
> +		list_for_each_entry(comp, &class->components, class_list)
> +			mpam_enable_merge_vmsc_features(comp);
> +
> +		mpam_enable_init_class_features(class);
> +
> +		list_for_each_entry(comp, &class->components, class_list)
> +			mpam_enable_merge_class_features(comp);
> +	}
> +}
> +
>   static void mpam_enable_once(void)
>   {
> +	mutex_lock(&mpam_list_lock);
> +	mpam_enable_merge_features(&mpam_classes);
> +	mutex_unlock(&mpam_list_lock);
> +
>   	mutex_lock(&mpam_cpuhp_state_lock);
>   	cpuhp_remove_state(mpam_cpuhp_state);
>   	mpam_cpuhp_state = 0;
> diff --git a/drivers/platform/arm64/mpam/mpam_internal.h b/drivers/platform/arm64/mpam/mpam_internal.h
> index ae6fd1f62cc4..be56234b84b4 100644
> --- a/drivers/platform/arm64/mpam/mpam_internal.h
> +++ b/drivers/platform/arm64/mpam/mpam_internal.h
> @@ -185,12 +185,20 @@ static inline void mpam_set_feature(enum mpam_device_features feat,
>   	props->features |= (1 << feat);
>   }
>   
> +static inline void mpam_clear_feature(enum mpam_device_features feat,
> +				      mpam_features_t *supported)
> +{
> +	*supported &= ~(1 << feat);
> +}
> +
>   struct mpam_class {
>   	/* mpam_components in this class */
>   	struct list_head	components;
>   
>   	cpumask_t		affinity;
>   
> +	struct mpam_props	props;
> +	u32			nrdy_usec;
>   	u8			level;
>   	enum mpam_class_types	type;
>   


Thanks,

Ben

Powered by blists - more mailing lists