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

[James Morse]

Hi Jonathan,

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