Hi Maciej,
On 12/12/2023 6:52 AM, 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@intel.com
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 | 75 +++++++++++++++++++ tools/testing/selftests/resctrl/resctrl.h | 3 + .../testing/selftests/resctrl/resctrl_tests.c | 2 + tools/testing/selftests/resctrl/resctrlfs.c | 2 +- 4 files changed, 81 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c index 7dc7206b3b99..ecf553a89aae 100644 --- a/tools/testing/selftests/resctrl/cat_test.c +++ b/tools/testing/selftests/resctrl/cat_test.c @@ -292,6 +292,65 @@ 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;
- int level, bit_center, sparse_masks;
- char schemata[64];
- /* Check to compare sparse_masks content to cpuid output. */
"cpuid" -> "CPUID" (to note it is an instruction)
- sparse_masks = read_info_res_file(test->resource, "sparse_masks");
- if (sparse_masks < 0)
return sparse_masks;- level = get_cache_level(test->resource);
- if (level < 0)
return -EINVAL;- __cpuid_count(0x10, 4 - level, eax, ebx, ecx, edx);
Please do not invent relationships. Please replace the "4 - level" with specific index used that depends on particular cache. The cache level may not even be needed, just use the resource to determine the correct index.
- if (sparse_masks != ((ecx >> 3) & 1))
return -1;
Can a message be displayed to support the debugging this test failure?
- /* Write checks initialization. */
- ret = get_full_cbm(test->resource, &full_cache_mask);
- if (ret < 0)
return ret;
I assume this test failure relies on the error message from get_bit_mask() that is called via get_full_cbm()?
- 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)
return ret;
How will user know what failed? I am seeing this single test exercise a few scenarios and it is not obvious to me if the issue will be clear if this test, noncont_cat_run_test(), fails.
- /*
* 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;
- snprintf(schemata, sizeof(schemata), "%lx", noncont_mask);
- ret = write_schemata("", schemata, uparams->cpu, test->resource);
- if (ret && sparse_masks)
ksft_print_msg("Non-contiguous CBMs supported but write failed\n");- else if (ret && !sparse_masks)
ksft_print_msg("Non-contiguous CBMs not supported and write failed as expected\n");- else if (!ret && !sparse_masks)
ksft_print_msg("Non-contiguous CBMs not supported but write succeeded\n");
Can these messages be made more specific with a "write" changed to "write of non-contiguous CBM"
- return !ret == !sparse_masks;
Please return negative on error. Ilpo just did a big cleanup to address this.
+}
+static bool noncont_cat_feature_check(const struct resctrl_test *test) +{
- if (!resctrl_resource_exists(test->resource))
return false;- return resctrl_cache_feature_exists(test->resource, "sparse_masks");
+}
struct resctrl_test l3_cat_test = { .name = "L3_CAT", .group = "CAT", @@ -299,3 +358,19 @@ struct resctrl_test l3_cat_test = { .feature_check = test_resource_feature_check, .run_test = cat_run_test, };
+struct resctrl_test l3_noncont_cat_test = {
- .name = "L3_NONCONT_CAT",
- .group = "CAT",
- .resource = "L3",
- .feature_check = noncont_cat_feature_check,
- .run_test = noncont_cat_run_test,
+};
+struct resctrl_test l2_noncont_cat_test = {
- .name = "L2_NONCONT_CAT",
- .group = "CAT",
- .resource = "L2",
- .feature_check = noncont_cat_feature_check,
- .run_test = noncont_cat_run_test,
+}; diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index 74041a35d4ba..7b7a48d1fddd 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -165,6 +165,7 @@ unsigned int count_contiguous_bits(unsigned long val, unsigned int *start); int get_full_cbm(const char *cache_type, unsigned long *mask); int get_mask_no_shareable(const char *cache_type, unsigned long *mask); int get_cache_size(int cpu_no, const char *cache_type, unsigned long *cache_size); +int get_cache_level(const char *cache_type); int read_info_res_file(const char *resource, const char *filename); void ctrlc_handler(int signum, siginfo_t *info, void *ptr); int signal_handler_register(void); @@ -201,5 +202,7 @@ extern struct resctrl_test mbm_test; extern struct resctrl_test mba_test; extern struct resctrl_test cmt_test; extern struct resctrl_test l3_cat_test; +extern struct resctrl_test l3_noncont_cat_test; +extern struct resctrl_test l2_noncont_cat_test; #endif /* RESCTRL_H */ diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index 3044179ee6e9..f3dc1b9696e7 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -19,6 +19,8 @@ static struct resctrl_test *resctrl_tests[] = { &mba_test, &cmt_test, &l3_cat_test,
- &l3_noncont_cat_test,
- &l2_noncont_cat_test,
}; static int detect_vendor(void) diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index 8546421f0940..8bd30973fec3 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -100,7 +100,7 @@ int umount_resctrlfs(void)
- Return: cache level as integer or -1 if @cache_type is invalid.
*/ -static int get_cache_level(const char *cache_type) +int get_cache_level(const char *cache_type) { if (!strcmp(cache_type, "L3")) return 3;
Reinette