| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ceb64a9b-0ffe-4193-942e-5ddf71478e5d@arm.com>
Date: Fri, 29 Aug 2025 18:11:32 +0100
From: Ben Horgan <ben.horgan@....com>
To: James Morse <james.morse@....com>, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-acpi@...r.kernel.org,
devicetree@...r.kernel.org
Cc: shameerali.kolothum.thodi@...wei.com,
D Scott Phillips OS <scott@...amperecomputing.com>,
carl@...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>,
Shanker Donthineni <sdonthineni@...dia.com>, fenghuay@...dia.com,
baisheng.gao@...soc.com, Jonathan Cameron <jonathan.cameron@...wei.com>,
Rob Herring <robh@...nel.org>, Rohit Mathew <rohit.mathew@....com>,
Rafael Wysocki <rafael@...nel.org>, Len Brown <lenb@...nel.org>,
Lorenzo Pieralisi <lpieralisi@...nel.org>, Hanjun Guo
<guohanjun@...wei.com>, Sudeep Holla <sudeep.holla@....com>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Danilo Krummrich <dakr@...nel.org>
Subject: Re: [PATCH 33/33] arm_mpam: Add kunit tests for props_mismatch()
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 <james.morse@....com>
> ---
> 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 <kunit/test.h>
>
> +/*
> + * 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
Powered by blists - more mailing lists