Re: [PATCH v2 29/29] arm_mpam: Add kunit tests for props_mismatch()

James Morse
2025-10-10T16:54:11Z

Hi Jonathan,

On 12/09/2025 14:41, Jonathan Cameron wrote:
> On Wed, 10 Sep 2025 20:43:09 +0000
> James Morse <james.morse@arm.com> 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@arm.com>

> Nice in general though I didn't go through the test expected results etc.

> A few comments inline.
>
> Thanks and looking forward to seeing this go in.

>> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
>> index 7cbcafe8294a..6119e4573187 100644
>> --- a/drivers/resctrl/mpam_internal.h
>> +++ b/drivers/resctrl/mpam_internal.h
>> @@ -189,7 +195,7 @@ struct mpam_props {
>> u16 dspri_wd;
>> u16 num_csu_mon;
>> u16 num_mbwu_mon;
>> -};
>> +} PACKED_FOR_KUNIT;
>
> Add a comment on 'why'.

| * Kunit tests use memset() to set up feature combinations that should be
| * removed, and will false-positive if the compiler introduces padding that
| * isn't cleared during sanitisation.

>> diff --git a/drivers/resctrl/test_mpam_devices.c b/drivers/resctrl/test_mpam_devices.c
>> index 3e7058f7601c..4eca8590c691 100644
>> --- a/drivers/resctrl/test_mpam_devices.c
>> +++ b/drivers/resctrl/test_mpam_devices.c
>> @@ -4,6 +4,325 @@
>>
>> #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 struct list_head fake_classes_list;
>> +static struct mpam_class fake_class = { 0 };
>> +static struct mpam_component fake_comp1 = { 0 };
>> +static struct mpam_component fake_comp2 = { 0 };
>> +static struct mpam_vmsc fake_vmsc1 = { 0 };
>> +static struct mpam_vmsc fake_vmsc2 = { 0 };
>> +static struct mpam_msc fake_msc1 = { 0 };
>> +static struct mpam_msc fake_msc2 = { 0 };
>> +static struct mpam_msc_ris fake_ris1 = { 0 };
>> +static struct mpam_msc_ris fake_ris2 = { 0 };
>> +static struct platform_device fake_pdev = { 0 };
>> +
>> +static void test_mpam_enable_merge_features(struct kunit *test)
>> +{
>> +#define RESET_FAKE_HIEARCHY() do { \
>> + INIT_LIST_HEAD(&fake_classes_list); \
>> + \
>> + memset(&fake_class, 0, sizeof(fake_class)); \

> Maybe just use a function? Seems to be changing stuff that is
> global mostly anyway so seems like it won't need large numbers
> of parameters or anything like that.

Sure - it only became global in v3.

Thanks,

James