From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 5A1BB3376BA; Fri, 29 Aug 2025 17:11:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756487502; cv=none; b=UrglLBiWN55NaF8lUBfAdSEIgw3jlhjJabzWfnmPhj0lAigas4yNjgp9rQ+Q/LGQlHn2CcjjFBAnR9JwSIR+nYP3znXHCf6mHQQvsoV+EHskAsgx9/RPWmxRMDf14rTjpzSRwucYqGMOdiOKcdiaGOaZIhW1Ysb30P2Xik3Kzdo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756487502; c=relaxed/simple; bh=zyNQZ1IgidMZltd0cYpijjDck/USqsb2pcBAvuctArw=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=icZMPIngz744eVQQoK+cFgp1s5aJX0s//dwtjS9bPlxEgoS9nPhiOiAFIX1ztwDTjtPlCC063DgOPCyhJbBSCDv/GJmUrDby2Szip9OYdjgWS7uuVOnROFxiKUPoyMVIuey3XuFmDK51aKwdqdCTMauzWgU9U20kEJzuYFRIR30= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com 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 36AE219F0; Fri, 29 Aug 2025 10:11:31 -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 4B0823F738; Fri, 29 Aug 2025 10:11:34 -0700 (PDT) Message-ID: Date: Fri, 29 Aug 2025 18:11:32 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 33/33] arm_mpam: Add kunit tests for props_mismatch() To: James Morse , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-acpi@vger.kernel.org, devicetree@vger.kernel.org Cc: shameerali.kolothum.thodi@huawei.com, D Scott Phillips OS , carl@os.amperecomputing.com, 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 , Shanker Donthineni , fenghuay@nvidia.com, baisheng.gao@unisoc.com, Jonathan Cameron , Rob Herring , Rohit Mathew , Rafael Wysocki , Len Brown , Lorenzo Pieralisi , Hanjun Guo , Sudeep Holla , Krzysztof Kozlowski , Conor Dooley , Catalin Marinas , Will Deacon , Greg Kroah-Hartman , Danilo Krummrich References: <20250822153048.2287-1-james.morse@arm.com> <20250822153048.2287-34-james.morse@arm.com> From: Ben Horgan Content-Language: en-US In-Reply-To: <20250822153048.2287-34-james.morse@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Hi James, The tests seem reasonable. Just some comments on the comments. On 8/22/25 16:30, James Morse wrote: > When features are mismatched between MSC the way features are combined > to the class determines whether resctrl can support this SoC. > > Add some tests to illustrate the sort of thing that is expected to > work, and those that must be removed. > > Signed-off-by: James Morse > --- > drivers/resctrl/mpam_internal.h | 8 +- > drivers/resctrl/test_mpam_devices.c | 322 ++++++++++++++++++++++++++++ > 2 files changed, 329 insertions(+), 1 deletion(-) > > diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h > index bbf0306abc82..6e973be095f8 100644 > --- a/drivers/resctrl/mpam_internal.h > +++ b/drivers/resctrl/mpam_internal.h > @@ -18,6 +18,12 @@ > > DECLARE_STATIC_KEY_FALSE(mpam_enabled); > > +#ifdef CONFIG_MPAM_KUNIT_TEST > +#define PACKED_FOR_KUNIT __packed > +#else > +#define PACKED_FOR_KUNIT > +#endif > + > static inline bool mpam_is_enabled(void) > { > return static_branch_likely(&mpam_enabled); > @@ -209,7 +215,7 @@ struct mpam_props { > u16 dspri_wd; > u16 num_csu_mon; > u16 num_mbwu_mon; > -}; > +} PACKED_FOR_KUNIT; > > #define mpam_has_feature(_feat, x) ((1 << (_feat)) & (x)->features) > > diff --git a/drivers/resctrl/test_mpam_devices.c b/drivers/resctrl/test_mpam_devices.c > index 8e9d6c88171c..ef39696e7ff8 100644 > --- a/drivers/resctrl/test_mpam_devices.c > +++ b/drivers/resctrl/test_mpam_devices.c > @@ -4,6 +4,326 @@ > > #include > > +/* > + * This test catches fields that aren't being sanitised - but can't tell you > + * which one... > + */ > +static void test__props_mismatch(struct kunit *test) > +{ > + struct mpam_props parent = { 0 }; > + struct mpam_props child; > + > + memset(&child, 0xff, sizeof(child)); > + __props_mismatch(&parent, &child, false); > + > + memset(&child, 0, sizeof(child)); > + KUNIT_EXPECT_EQ(test, memcmp(&parent, &child, sizeof(child)), 0); > + > + memset(&child, 0xff, sizeof(child)); > + __props_mismatch(&parent, &child, true); > + > + KUNIT_EXPECT_EQ(test, memcmp(&parent, &child, sizeof(child)), 0); > +} > + > +static void test_mpam_enable_merge_features(struct kunit *test) > +{ > + /* o/` How deep is your stack? o/` */ > + struct list_head fake_classes_list; > + struct mpam_class fake_class = { 0 }; > + struct mpam_component fake_comp1 = { 0 }; > + struct mpam_component fake_comp2 = { 0 }; > + struct mpam_vmsc fake_vmsc1 = { 0 }; > + struct mpam_vmsc fake_vmsc2 = { 0 }; > + struct mpam_msc fake_msc1 = { 0 }; > + struct mpam_msc fake_msc2 = { 0 }; > + struct mpam_msc_ris fake_ris1 = { 0 }; > + struct mpam_msc_ris fake_ris2 = { 0 }; > + struct platform_device fake_pdev = { 0 }; > + > +#define RESET_FAKE_HIEARCHY() do { \ > + INIT_LIST_HEAD(&fake_classes_list); \ > + \ > + memset(&fake_class, 0, sizeof(fake_class)); \ > + fake_class.level = 3; \ > + fake_class.type = MPAM_CLASS_CACHE; \ > + INIT_LIST_HEAD_RCU(&fake_class.components); \ > + INIT_LIST_HEAD(&fake_class.classes_list); \ > + \ > + memset(&fake_comp1, 0, sizeof(fake_comp1)); \ > + memset(&fake_comp2, 0, sizeof(fake_comp2)); \ > + fake_comp1.comp_id = 1; \ > + fake_comp2.comp_id = 2; \ > + INIT_LIST_HEAD(&fake_comp1.vmsc); \ > + INIT_LIST_HEAD(&fake_comp1.class_list); \ > + INIT_LIST_HEAD(&fake_comp2.vmsc); \ > + INIT_LIST_HEAD(&fake_comp2.class_list); \ > + \ > + memset(&fake_vmsc1, 0, sizeof(fake_vmsc1)); \ > + memset(&fake_vmsc2, 0, sizeof(fake_vmsc2)); \ > + INIT_LIST_HEAD(&fake_vmsc1.ris); \ > + INIT_LIST_HEAD(&fake_vmsc1.comp_list); \ > + fake_vmsc1.msc = &fake_msc1; \ > + INIT_LIST_HEAD(&fake_vmsc2.ris); \ > + INIT_LIST_HEAD(&fake_vmsc2.comp_list); \ > + fake_vmsc2.msc = &fake_msc2; \ > + \ > + memset(&fake_ris1, 0, sizeof(fake_ris1)); \ > + memset(&fake_ris2, 0, sizeof(fake_ris2)); \ > + fake_ris1.ris_idx = 1; \ > + INIT_LIST_HEAD(&fake_ris1.msc_list); \ > + fake_ris2.ris_idx = 2; \ > + INIT_LIST_HEAD(&fake_ris2.msc_list); \ > + \ > + fake_msc1.pdev = &fake_pdev; \ > + fake_msc2.pdev = &fake_pdev; \ > + \ > + list_add(&fake_class.classes_list, &fake_classes_list); \ > +} while (0) > + > + RESET_FAKE_HIEARCHY(); > + > + mutex_lock(&mpam_list_lock); > + > + /* One Class+Comp, two RIS in one vMSC with common features */ > + fake_comp1.class = &fake_class; > + list_add(&fake_comp1.class_list, &fake_class.components); > + fake_comp2.class = NULL; > + fake_vmsc1.comp = &fake_comp1; > + list_add(&fake_vmsc1.comp_list, &fake_comp1.vmsc); > + fake_vmsc2.comp = NULL; > + fake_ris1.vmsc = &fake_vmsc1; > + list_add(&fake_ris1.vmsc_list, &fake_vmsc1.ris); > + fake_ris2.vmsc = &fake_vmsc1; > + list_add(&fake_ris2.vmsc_list, &fake_vmsc1.ris); > + > + mpam_set_feature(mpam_feat_cpor_part, &fake_ris1.props); > + mpam_set_feature(mpam_feat_cpor_part, &fake_ris2.props); > + fake_ris1.props.cpbm_wd = 4; > + fake_ris2.props.cpbm_wd = 4; > + > + mpam_enable_merge_features(&fake_classes_list); > + > + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_cpor_part, &fake_class.props)); > + KUNIT_EXPECT_EQ(test, fake_class.props.cpbm_wd, 4); > + > + RESET_FAKE_HIEARCHY(); > + > + /* One Class+Comp, two RIS in one vMSC with non-overlapping features */ > + fake_comp1.class = &fake_class; > + list_add(&fake_comp1.class_list, &fake_class.components); > + fake_comp2.class = NULL; > + fake_vmsc1.comp = &fake_comp1; > + list_add(&fake_vmsc1.comp_list, &fake_comp1.vmsc); > + fake_vmsc2.comp = NULL; > + fake_ris1.vmsc = &fake_vmsc1; > + list_add(&fake_ris1.vmsc_list, &fake_vmsc1.ris); > + fake_ris2.vmsc = &fake_vmsc1; > + list_add(&fake_ris2.vmsc_list, &fake_vmsc1.ris); > + > + mpam_set_feature(mpam_feat_cpor_part, &fake_ris1.props); > + mpam_set_feature(mpam_feat_cmax_cmin, &fake_ris2.props); > + fake_ris1.props.cpbm_wd = 4; > + fake_ris2.props.cmax_wd = 4; > + > + mpam_enable_merge_features(&fake_classes_list); > + > + /* Multiple RIS within one MSC controlling the same resource can be mismatched */ > + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_cpor_part, &fake_class.props)); > + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_cmax_cmin, &fake_class.props)); > + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_cmax_cmin, &fake_vmsc1.props)); > + KUNIT_EXPECT_EQ(test, fake_class.props.cpbm_wd, 4); > + KUNIT_EXPECT_EQ(test, fake_vmsc1.props.cmax_wd, 4); > + KUNIT_EXPECT_EQ(test, fake_class.props.cmax_wd, 4); > + > + RESET_FAKE_HIEARCHY(); > + > + /* One Class+Comp, two MSC with overlapping features */ > + fake_comp1.class = &fake_class; > + list_add(&fake_comp1.class_list, &fake_class.components); > + fake_comp2.class = NULL; > + fake_vmsc1.comp = &fake_comp1; > + list_add(&fake_vmsc1.comp_list, &fake_comp1.vmsc); > + fake_vmsc2.comp = &fake_comp1; > + list_add(&fake_vmsc2.comp_list, &fake_comp1.vmsc); > + fake_ris1.vmsc = &fake_vmsc1; > + list_add(&fake_ris1.vmsc_list, &fake_vmsc1.ris); > + fake_ris2.vmsc = &fake_vmsc2; > + list_add(&fake_ris2.vmsc_list, &fake_vmsc2.ris); > + > + mpam_set_feature(mpam_feat_cpor_part, &fake_ris1.props); > + mpam_set_feature(mpam_feat_cpor_part, &fake_ris2.props); > + fake_ris1.props.cpbm_wd = 4; > + fake_ris2.props.cpbm_wd = 4; > + > + mpam_enable_merge_features(&fake_classes_list); > + > + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_cpor_part, &fake_class.props)); > + KUNIT_EXPECT_EQ(test, fake_class.props.cpbm_wd, 4); > + > + RESET_FAKE_HIEARCHY(); > + > + /* One Class+Comp, two MSC with non-overlapping features */ > + fake_comp1.class = &fake_class; > + list_add(&fake_comp1.class_list, &fake_class.components); > + fake_comp2.class = NULL; > + fake_vmsc1.comp = &fake_comp1; > + list_add(&fake_vmsc1.comp_list, &fake_comp1.vmsc); > + fake_vmsc2.comp = &fake_comp1; > + list_add(&fake_vmsc2.comp_list, &fake_comp1.vmsc); > + fake_ris1.vmsc = &fake_vmsc1; > + list_add(&fake_ris1.vmsc_list, &fake_vmsc1.ris); > + fake_ris2.vmsc = &fake_vmsc2; > + list_add(&fake_ris2.vmsc_list, &fake_vmsc2.ris); > + > + mpam_set_feature(mpam_feat_cpor_part, &fake_ris1.props); > + mpam_set_feature(mpam_feat_cmax_cmin, &fake_ris2.props); > + fake_ris1.props.cpbm_wd = 4; > + fake_ris2.props.cmax_wd = 4; > + > + mpam_enable_merge_features(&fake_classes_list); > + > + /* > + * Multiple RIS in different MSC can't the same resource, mismatched s/can't the same/can't control the same/ > + * features can not be supported. > + */ > + KUNIT_EXPECT_FALSE(test, mpam_has_feature(mpam_feat_cpor_part, &fake_class.props)); > + KUNIT_EXPECT_FALSE(test, mpam_has_feature(mpam_feat_cmax_cmin, &fake_class.props)); > + KUNIT_EXPECT_EQ(test, fake_class.props.cpbm_wd, 0); > + KUNIT_EXPECT_EQ(test, fake_class.props.cmax_wd, 0); > + > + RESET_FAKE_HIEARCHY(); > + > + /* One Class+Comp, two MSC with incompatible overlapping features */ > + fake_comp1.class = &fake_class; > + list_add(&fake_comp1.class_list, &fake_class.components); > + fake_comp2.class = NULL; > + fake_vmsc1.comp = &fake_comp1; > + list_add(&fake_vmsc1.comp_list, &fake_comp1.vmsc); > + fake_vmsc2.comp = &fake_comp1; > + list_add(&fake_vmsc2.comp_list, &fake_comp1.vmsc); > + fake_ris1.vmsc = &fake_vmsc1; > + list_add(&fake_ris1.vmsc_list, &fake_vmsc1.ris); > + fake_ris2.vmsc = &fake_vmsc2; > + list_add(&fake_ris2.vmsc_list, &fake_vmsc2.ris); > + > + mpam_set_feature(mpam_feat_cpor_part, &fake_ris1.props); > + mpam_set_feature(mpam_feat_cpor_part, &fake_ris2.props); > + mpam_set_feature(mpam_feat_mbw_part, &fake_ris1.props); > + mpam_set_feature(mpam_feat_mbw_part, &fake_ris2.props); > + fake_ris1.props.cpbm_wd = 5; > + fake_ris2.props.cpbm_wd = 3; > + fake_ris1.props.mbw_pbm_bits = 5; > + fake_ris2.props.mbw_pbm_bits = 3; > + > + mpam_enable_merge_features(&fake_classes_list); > + > + /* > + * Multiple RIS in different MSC can't the same resource, mismatched > + * features can not be supported. > + */ Missing the word "control" again. > + KUNIT_EXPECT_FALSE(test, mpam_has_feature(mpam_feat_cpor_part, &fake_class.props)); > + KUNIT_EXPECT_FALSE(test, mpam_has_feature(mpam_feat_mbw_part, &fake_class.props)); > + KUNIT_EXPECT_EQ(test, fake_class.props.cpbm_wd, 0); > + KUNIT_EXPECT_EQ(test, fake_class.props.mbw_pbm_bits, 0); > + > + RESET_FAKE_HIEARCHY(); > + > + /* One Class+Comp, two MSC with overlapping features that need tweaking */ > + fake_comp1.class = &fake_class; > + list_add(&fake_comp1.class_list, &fake_class.components); > + fake_comp2.class = NULL; > + fake_vmsc1.comp = &fake_comp1; > + list_add(&fake_vmsc1.comp_list, &fake_comp1.vmsc); > + fake_vmsc2.comp = &fake_comp1; > + list_add(&fake_vmsc2.comp_list, &fake_comp1.vmsc); > + fake_ris1.vmsc = &fake_vmsc1; > + list_add(&fake_ris1.vmsc_list, &fake_vmsc1.ris); > + fake_ris2.vmsc = &fake_vmsc2; > + list_add(&fake_ris2.vmsc_list, &fake_vmsc2.ris); > + > + mpam_set_feature(mpam_feat_mbw_min, &fake_ris1.props); > + mpam_set_feature(mpam_feat_mbw_min, &fake_ris2.props); > + mpam_set_feature(mpam_feat_cmax_cmax, &fake_ris1.props); > + mpam_set_feature(mpam_feat_cmax_cmax, &fake_ris2.props); > + fake_ris1.props.bwa_wd = 5; > + fake_ris2.props.bwa_wd = 3; > + fake_ris1.props.cmax_wd = 5; > + fake_ris2.props.cmax_wd = 3; > + > + mpam_enable_merge_features(&fake_classes_list); > + > + /* > + * Multiple RIS in different MSC can't the same resource, mismatched > + * features can not be supported. > + */ Comment is for a different case. > + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_mbw_min, &fake_class.props)); > + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_cmax_cmax, &fake_class.props)); > + KUNIT_EXPECT_EQ(test, fake_class.props.bwa_wd, 3); > + KUNIT_EXPECT_EQ(test, fake_class.props.cmax_wd, 3); > + > + RESET_FAKE_HIEARCHY(); > + > + /* One Class Two Comp with overlapping features */ > + fake_comp1.class = &fake_class; > + list_add(&fake_comp1.class_list, &fake_class.components); > + fake_comp2.class = &fake_class; > + list_add(&fake_comp2.class_list, &fake_class.components); > + fake_vmsc1.comp = &fake_comp1; > + list_add(&fake_vmsc1.comp_list, &fake_comp1.vmsc); > + fake_vmsc2.comp = &fake_comp2; > + list_add(&fake_vmsc2.comp_list, &fake_comp2.vmsc); > + fake_ris1.vmsc = &fake_vmsc1; > + list_add(&fake_ris1.vmsc_list, &fake_vmsc1.ris); > + fake_ris2.vmsc = &fake_vmsc2; > + list_add(&fake_ris2.vmsc_list, &fake_vmsc2.ris); > + > + mpam_set_feature(mpam_feat_cpor_part, &fake_ris1.props); > + mpam_set_feature(mpam_feat_cpor_part, &fake_ris2.props); > + fake_ris1.props.cpbm_wd = 4; > + fake_ris2.props.cpbm_wd = 4; > + > + mpam_enable_merge_features(&fake_classes_list); > + > + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_cpor_part, &fake_class.props)); > + KUNIT_EXPECT_EQ(test, fake_class.props.cpbm_wd, 4); > + > + RESET_FAKE_HIEARCHY(); > + > + /* One Class Two Comp with non-overlapping features */ > + fake_comp1.class = &fake_class; > + list_add(&fake_comp1.class_list, &fake_class.components); > + fake_comp2.class = &fake_class; > + list_add(&fake_comp2.class_list, &fake_class.components); > + fake_vmsc1.comp = &fake_comp1; > + list_add(&fake_vmsc1.comp_list, &fake_comp1.vmsc); > + fake_vmsc2.comp = &fake_comp2; > + list_add(&fake_vmsc2.comp_list, &fake_comp2.vmsc); > + fake_ris1.vmsc = &fake_vmsc1; > + list_add(&fake_ris1.vmsc_list, &fake_vmsc1.ris); > + fake_ris2.vmsc = &fake_vmsc2; > + list_add(&fake_ris2.vmsc_list, &fake_vmsc2.ris); > + > + mpam_set_feature(mpam_feat_cpor_part, &fake_ris1.props); > + mpam_set_feature(mpam_feat_cmax_cmin, &fake_ris2.props); > + fake_ris1.props.cpbm_wd = 4; > + fake_ris2.props.cmax_wd = 4; > + > + mpam_enable_merge_features(&fake_classes_list); > + > + /* > + * Multiple components can't control the same resource, mismatched features can > + * not be supported. > + */ > + KUNIT_EXPECT_FALSE(test, mpam_has_feature(mpam_feat_cpor_part, &fake_class.props)); > + KUNIT_EXPECT_FALSE(test, mpam_has_feature(mpam_feat_cmax_cmin, &fake_class.props)); > + KUNIT_EXPECT_EQ(test, fake_class.props.cpbm_wd, 0); > + KUNIT_EXPECT_EQ(test, fake_class.props.cmax_wd, 0); > + > + mutex_unlock(&mpam_list_lock); > + > +#undef RESET_FAKE_HIEARCHY > +} > + > static void test_mpam_reset_msc_bitmap(struct kunit *test) > { > char *buf = kunit_kzalloc(test, SZ_16K, GFP_KERNEL); > @@ -57,6 +377,8 @@ static void test_mpam_reset_msc_bitmap(struct kunit *test) > > static struct kunit_case mpam_devices_test_cases[] = { > KUNIT_CASE(test_mpam_reset_msc_bitmap), > + KUNIT_CASE(test_mpam_enable_merge_features), > + KUNIT_CASE(test__props_mismatch), > {} > }; > Thanks, Ben