Hi Ben,
On 29/08/2025 18:11, Ben Horgan wrote:
> 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.
>> 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 @@
>> +static void test_mpam_enable_merge_features(struct kunit *test)
>> +{
[...]
>> + /* 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/
Thanks,
>> + * 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.
Copy and paste!
>> + 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.
Fixed as:
/*
* RIS with different control properties need to be sanitised so the
* class has the common set of properties.
*/
>> + 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);
Thanks,
James
