[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <m76an3zrp375meemqdqv2tx577kwyjy6g64ain2wp2vvnyx4ex@zby3einmlszv>
Date: Mon, 5 Feb 2024 14:25:50 +0100
From: Maciej Wieczor-Retman <maciej.wieczor-retman@...el.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
CC: <fenghua.yu@...el.com>, Reinette Chatre <reinette.chatre@...el.com>,
<shuah@...nel.org>, <linux-kselftest@...r.kernel.org>, LKML
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 5/5] selftests/resctrl: Add non-contiguous CBMs CAT
test
On 2024-02-05 at 15:11:22 +0200, Ilpo Järvinen wrote:
>On Mon, 5 Feb 2024, Maciej Wieczor-Retman wrote:
>
>> Add tests for both L2 and L3 CAT to verify the return values
>> generated by writing non-contiguous CBMs don't contradict the
>> reported non-contiguous support information.
>>
>> Use a logical XOR to confirm return value of write_schemata() and
>> non-contiguous CBMs support information match.
>>
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@...el.com>
>> ---
>> Changelog v4:
>> - Return failure instead of error on check of cpuid against sparse_masks
>> and on contiguous write_schemata fail. (Reinette)
>>
>> Changelog v3:
>> - Roll back __cpuid_count part. (Reinette)
>> - Update function name to read sparse_masks file.
>> - Roll back get_cache_level() changes.
>> - Add ksft_print_msg() to contiguous schemata write error handling
>> (Reinette).
>>
>> Changelog v2:
>> - Redo the patch message. (Ilpo)
>> - Tidy up __cpuid_count calls. (Ilpo)
>> - Remove redundant AND in noncont_mask calculations (Ilpo)
>> - Fix bit_center offset.
>> - Add newline before function return. (Ilpo)
>> - Group non-contiguous tests with CAT tests. (Ilpo)
>> - Use a helper for reading sparse_masks file. (Ilpo)
>> - Make get_cache_level() available in other source files. (Ilpo)
>>
>> tools/testing/selftests/resctrl/cat_test.c | 81 +++++++++++++++++++
>> tools/testing/selftests/resctrl/resctrl.h | 2 +
>> .../testing/selftests/resctrl/resctrl_tests.c | 2 +
>> 3 files changed, 85 insertions(+)
>>
>> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
>> index 39fc9303b8e8..20eb978e624b 100644
>> --- a/tools/testing/selftests/resctrl/cat_test.c
>> +++ b/tools/testing/selftests/resctrl/cat_test.c
>> @@ -294,6 +294,71 @@ static int cat_run_test(const struct resctrl_test *test, const struct user_param
>> return ret;
>> }
>>
>> +static int noncont_cat_run_test(const struct resctrl_test *test,
>> + const struct user_params *uparams)
>> +{
>> + unsigned long full_cache_mask, cont_mask, noncont_mask;
>> + unsigned int eax, ebx, ecx, edx, ret, sparse_masks;
>> + char schemata[64];
>> + int bit_center;
>> +
>> + /* Check to compare sparse_masks content to CPUID output. */
>> + ret = resource_info_unsigned_get(test->resource, "sparse_masks", &sparse_masks);
>> + if (ret)
>> + return ret;
>> +
>> + if (!strcmp(test->resource, "L3"))
>> + __cpuid_count(0x10, 1, eax, ebx, ecx, edx);
>> + else if (!strcmp(test->resource, "L2"))
>> + __cpuid_count(0x10, 2, eax, ebx, ecx, edx);
>> + else
>> + return -EINVAL;
>> +
>> + if (sparse_masks != ((ecx >> 3) & 1)) {
>> + ksft_print_msg("CPUID output doesn't match 'sparse_masks' file content!\n");
>> + return 1;
>> + }
>> +
>> + /* Write checks initialization. */
>> + ret = get_full_cbm(test->resource, &full_cache_mask);
>> + if (ret < 0)
>> + return ret;
>> + bit_center = count_bits(full_cache_mask) / 2;
>> + cont_mask = full_cache_mask >> bit_center;
>> +
>> + /* Contiguous mask write check. */
>> + snprintf(schemata, sizeof(schemata), "%lx", cont_mask);
>> + ret = write_schemata("", schemata, uparams->cpu, test->resource);
>> + if (ret) {
>> + ksft_print_msg("Write of contiguous CBM failed\n");
>> + return 1;
>> + }
>> +
>> + /*
>> + * Non-contiguous mask write check. CBM has a 0xf hole approximately in the middle.
>> + * Output is compared with support information to catch any edge case errors.
>> + */
>> + noncont_mask = ~(0xf << (bit_center - 2)) & full_cache_mask;
>
>To be on the safe side, I think the types could be made to match here
>with 0xfUL to avoid sizeof(int) vs sizeof(unsigned long) related bit
>drops in the & (although it feel somewhat theoretical given the bitmask
>sizes we are currently seeing).
Sure, I'll add that for the next version. Thanks!
>
>Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
>
>
>--
> i.
--
Kind regards
Maciej Wieczór-Retman
Powered by blists - more mailing lists