From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 091F2C83F17 for ; Mon, 28 Jul 2025 09:18:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=mxVpK/LCpI6jDy5FBBoMfKQKH9OF2AKVHBOgSun6w8Q=; b=Enjo+lN5thLQdvEt7i+Q3Pbri3 +ee6m9dM0UbCpHHe76T3ppKl0vlH5XWEnvnE0TDo87XjjTObFpn8aSNiO8kylQ4beoQ1F/7I+hFRR fq3KjVnHV45HeyUiFSbBzo3KElPL9uIqK5EROMEruEj+VqpfgiyzDc5Aky6RR0+A/9jwiU1X7Vdjo JV2GkrjjeqrEX5s5ysj974jBh+cH/Og+VOqp56RBHoDdS5qufXHiutrqD/4Us2hFNISB5cOteZy72 O0IqWXASPs5rn0yN0TRXocgIo82tz1HL4Rhs7bpt4MrTIjQz/iUsKymg2cr3OcdR/HtuKNmCZtIKy Zu7fweSg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1ugJzk-0000000E5Jg-383W; Mon, 28 Jul 2025 09:18:08 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1ugJxE-0000000E590-19Rb for linux-arm-kernel@lists.infradead.org; Mon, 28 Jul 2025 09:15:33 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 497171596; Mon, 28 Jul 2025 02:15:23 -0700 (PDT) Received: from [10.1.196.46] (e134344.arm.com [10.1.196.46]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7C75A3F66E; Mon, 28 Jul 2025 02:15:27 -0700 (PDT) Message-ID: Date: Mon, 28 Jul 2025 10:15:25 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH 21/36] arm_mpam: Merge supported features during mpam_enable() into mpam_class To: James Morse , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Cc: Rob Herring , Rohit Mathew , Shanker Donthineni , Zeng Heng , Lecopzer Chen , Carl Worth , shameerali.kolothum.thodi@huawei.com, D Scott Phillips OS , lcherian@marvell.com, bobo.shaobowang@huawei.com, tan.shaopeng@fujitsu.com, baolin.wang@linux.alibaba.com, Jamie Iles , Xin Hao , peternewman@google.com, dfustini@baylibre.com, amitsinght@marvell.com, David Hildenbrand , Rex Nie , Dave Martin , Koba Ko References: <20250711183648.30766-1-james.morse@arm.com> <20250711183648.30766-22-james.morse@arm.com> Content-Language: en-US From: Ben Horgan In-Reply-To: <20250711183648.30766-22-james.morse@arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250728_021532_401296_A53B7FA3 X-CRM114-Status: GOOD ( 36.44 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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 > --- > 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