diff mbox series

selftests/resctrl: Add non-contiguous CBMs CAT test

Message ID 20231109112847.432687-1-maciej.wieczor-retman@intel.com (mailing list archive)
State New
Headers show
Series selftests/resctrl: Add non-contiguous CBMs CAT test | expand

Commit Message

Maciej Wieczor-Retman Nov. 9, 2023, 11:28 a.m. UTC
Non-contiguous CBM support for Intel CAT has been merged into the kernel
with Commit 0e3cd31f6e90 ("x86/resctrl: Enable non-contiguous CBMs in
Intel CAT") but there is no selftest that would validate if this feature
works correctly.

The selftest needs to verify if writing non-contiguous CBMs to the
schemata file behaves as expected in comparison to the information about
non-contiguous CBMs support.

Add tests for both L2 and L3 CAT to verify if the return values
generated by writing non-contiguous CBMs don't contradict the
reported non-contiguous support information.

Comparing the return value of write_schemata() with non-contiguous CBMs
support information can be simplified as a logical XOR operation. In
other words if non-contiguous CBMs are supported and if non-contiguous
write succeeds the test should succeed and if the write fails the test
should also fail. The opposite should happen if non-contiguous CBMs are
not supported.

Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>

---

This patch is based on a rework of resctrl selftests that's currently in
review [1]. The patch also implements a similiar functionality presented
in the bash script included in the cover letter to the original
non-contiguous CBMs in Intel CAT series [2].

[1] https://lore.kernel.org/all/20231024092634.7122-1-ilpo.jarvinen@linux.intel.com/
[2] https://lore.kernel.org/all/cover.1696934091.git.maciej.wieczor-retman@intel.com/

 tools/testing/selftests/resctrl/cat_test.c    | 97 +++++++++++++++++++
 tools/testing/selftests/resctrl/resctrl.h     |  2 +
 .../testing/selftests/resctrl/resctrl_tests.c |  2 +
 3 files changed, 101 insertions(+)

Comments

Ilpo Järvinen Nov. 17, 2023, 12:55 p.m. UTC | #1
On Thu, 9 Nov 2023, Maciej Wieczor-Retman wrote:

> Non-contiguous CBM support for Intel CAT has been merged into the kernel
> with Commit 0e3cd31f6e90 ("x86/resctrl: Enable non-contiguous CBMs in
> Intel CAT") but there is no selftest that would validate if this feature
> works correctly.
> 
> The selftest needs to verify if writing non-contiguous CBMs to the
> schemata file behaves as expected in comparison to the information about
> non-contiguous CBMs support.
> 
> Add tests for both L2 and L3 CAT to verify if the return values
> generated by writing non-contiguous CBMs don't contradict the
> reported non-contiguous support information.

"if ... don't" sounds weird to me. Perhaps the "if" could just be dropped 
from it.

> Comparing the return value of write_schemata() with non-contiguous CBMs
> support information can be simplified as a logical XOR operation. In
> other words if non-contiguous CBMs are supported and if non-contiguous
> write succeeds the test should succeed and if the write fails the test
> should also fail. The opposite should happen if non-contiguous CBMs are
> not supported.

To me this sounds a bit verbose given how basic thing it talks about 
(but maybe I'm too old already to have actually come across a few xor
tricks in the past :-)). I'd simplify it to (or simply drop it):

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>
> 
> ---
> 
> This patch is based on a rework of resctrl selftests that's currently in
> review [1]. The patch also implements a similiar functionality presented
> in the bash script included in the cover letter to the original
> non-contiguous CBMs in Intel CAT series [2].
> 
> [1] https://lore.kernel.org/all/20231024092634.7122-1-ilpo.jarvinen@linux.intel.com/
> [2] https://lore.kernel.org/all/cover.1696934091.git.maciej.wieczor-retman@intel.com/
> 
>  tools/testing/selftests/resctrl/cat_test.c    | 97 +++++++++++++++++++
>  tools/testing/selftests/resctrl/resctrl.h     |  2 +
>  .../testing/selftests/resctrl/resctrl_tests.c |  2 +
>  3 files changed, 101 insertions(+)
> 
> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
> index bc88eb891f35..6a01a5da30b4 100644
> --- a/tools/testing/selftests/resctrl/cat_test.c
> +++ b/tools/testing/selftests/resctrl/cat_test.c
> @@ -342,6 +342,87 @@ 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 res_path[PATH_MAX];
> +	char schemata[64];
> +	int bit_center;
> +	FILE *fp;
> +
> +	/* Check to compare sparse_masks content to cpuid output. */
> +	snprintf(res_path, sizeof(res_path), "%s/%s/%s", INFO_PATH,
> +		 test->resource, "sparse_masks");
> +
> +	fp = fopen(res_path, "r");
> +	if (!fp) {
> +		perror("# Error in opening file\n");
> +		return errno;
> +	}
> +
> +	if (fscanf(fp, "%u", &sparse_masks) <= 0) {
> +		perror("Could not get sparse_masks contents\n");
> +		fclose(fp);
> +		return -1;
> +	}
> +
> +	fclose(fp);

Add a function to do this conversion into resctrlfs.c.

> +
> +	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;

This would be same as (you need to make the func non-static though):
	level = get_cache_level(test->resource);
	if (level < 0)
		return -EINVAL;
	__cpuid_count(0x10, 4 - level, eax, ebx, ecx, edx);

> +	if (sparse_masks != ((ecx >> 3) & 1))
> +		return -1;
> +
> +	/* Write checks initialization. */
> +	ret = get_cbm_mask(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)
> +		return ret;
> +
> +	/*
> +	 * 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 = ~(full_cache_mask & (0xf << bit_center)) & full_cache_mask;

Why is the full_cache_mask & part needed here? It's not like the second 
and can grow bits outside of full_cache_mask even if that would overflow 
the full_cache_mask (it won't be testing hole then though but that 
problem happens also at the boundary condition one bit prior to 
overflowing the 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");

Newline.

> +	return !ret == !sparse_masks;
> +}
> +
> +static bool noncont_cat_feature_check(const struct resctrl_test *test)
> +{
> +	char res_path[PATH_MAX];
> +	struct stat statbuf;
> +
> +	snprintf(res_path, sizeof(res_path), "%s/%s/%s", INFO_PATH,
> +		 test->resource, "sparse_masks");
> +
> +	if (stat(res_path, &statbuf))
> +		return false;

This looks generic enough that validate_resctrl_feature_request() should 
be somehow adapted to cover also these cases. Perhaps it would be best to 
just split validate_resctrl_feature_request() into multiple functions.

> +	return test_resource_feature_check(test);
> +}
> +
>  struct resctrl_test l3_cat_test = {
>  	.name = "L3_CAT",
>  	.group = "CAT",
> @@ -357,3 +438,19 @@ struct resctrl_test l2_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 = "NONCONT_CAT",

> +struct resctrl_test l2_noncont_cat_test = {
> +	.name = "L2_NONCONT_CAT",
> +	.group = "NONCONT_CAT",

I think these should be grouped among "CAT" group because it well, tests 
CAT functionality. Why you think a separate group for them is needed?
Maciej Wieczor-Retman Nov. 20, 2023, 10:48 a.m. UTC | #2
Hi, thank you for reviewing my patch!

On 2023-11-17 at 14:55:54 +0200, Ilpo Järvinen wrote:
>On Thu, 9 Nov 2023, Maciej Wieczor-Retman wrote:
>
>> Non-contiguous CBM support for Intel CAT has been merged into the kernel
>> with Commit 0e3cd31f6e90 ("x86/resctrl: Enable non-contiguous CBMs in
>> Intel CAT") but there is no selftest that would validate if this feature
>> works correctly.
>> 
>> The selftest needs to verify if writing non-contiguous CBMs to the
>> schemata file behaves as expected in comparison to the information about
>> non-contiguous CBMs support.
>> 
>> Add tests for both L2 and L3 CAT to verify if the return values
>> generated by writing non-contiguous CBMs don't contradict the
>> reported non-contiguous support information.
>
>"if ... don't" sounds weird to me. Perhaps the "if" could just be dropped 
>from it.

Thanks, that does sound better.

>> Comparing the return value of write_schemata() with non-contiguous CBMs
>> support information can be simplified as a logical XOR operation. In
>> other words if non-contiguous CBMs are supported and if non-contiguous
>> write succeeds the test should succeed and if the write fails the test
>> should also fail. The opposite should happen if non-contiguous CBMs are
>> not supported.
>
>To me this sounds a bit verbose given how basic thing it talks about 
>(but maybe I'm too old already to have actually come across a few xor
>tricks in the past :-)). I'd simplify it to (or simply drop it):
>
>Use a logical XOR to confirm return value of write_schemata() and  
>non-contiguous CBMs support information match.

Your version seems sufficient indeed. I didn't want to leave that XOR in the
code without any explanation.

>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> 
>> ---
>> 
>> This patch is based on a rework of resctrl selftests that's currently in
>> review [1]. The patch also implements a similiar functionality presented
>> in the bash script included in the cover letter to the original
>> non-contiguous CBMs in Intel CAT series [2].
>> 
>> [1] https://lore.kernel.org/all/20231024092634.7122-1-ilpo.jarvinen@linux.intel.com/
>> [2] https://lore.kernel.org/all/cover.1696934091.git.maciej.wieczor-retman@intel.com/
>> 
>>  tools/testing/selftests/resctrl/cat_test.c    | 97 +++++++++++++++++++
>>  tools/testing/selftests/resctrl/resctrl.h     |  2 +
>>  .../testing/selftests/resctrl/resctrl_tests.c |  2 +
>>  3 files changed, 101 insertions(+)
>> 
>> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
>> index bc88eb891f35..6a01a5da30b4 100644
>> --- a/tools/testing/selftests/resctrl/cat_test.c
>> +++ b/tools/testing/selftests/resctrl/cat_test.c
>> @@ -342,6 +342,87 @@ 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 res_path[PATH_MAX];
>> +	char schemata[64];
>> +	int bit_center;
>> +	FILE *fp;
>> +
>> +	/* Check to compare sparse_masks content to cpuid output. */
>> +	snprintf(res_path, sizeof(res_path), "%s/%s/%s", INFO_PATH,
>> +		 test->resource, "sparse_masks");
>> +
>> +	fp = fopen(res_path, "r");
>> +	if (!fp) {
>> +		perror("# Error in opening file\n");
>> +		return errno;
>> +	}
>> +
>> +	if (fscanf(fp, "%u", &sparse_masks) <= 0) {
>> +		perror("Could not get sparse_masks contents\n");
>> +		fclose(fp);
>> +		return -1;
>> +	}
>> +
>> +	fclose(fp);
>
>Add a function to do this conversion into resctrlfs.c.

By conversion do you mean the above calls to fopen, fscanf and fclose? Or did
you mean the below __cpuid_count? Since you mention making get_cache_level
non-static I assume the first is the case but just wanted to confirm.

>> +
>> +	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;
>
>This would be same as (you need to make the func non-static though):
>	level = get_cache_level(test->resource);
>	if (level < 0)
>		return -EINVAL;
>	__cpuid_count(0x10, 4 - level, eax, ebx, ecx, edx);

Thanks, that does look tidier.

>> +	if (sparse_masks != ((ecx >> 3) & 1))
>> +		return -1;
>> +
>> +	/* Write checks initialization. */
>> +	ret = get_cbm_mask(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)
>> +		return ret;
>> +
>> +	/*
>> +	 * 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 = ~(full_cache_mask & (0xf << bit_center)) & full_cache_mask;
>
>Why is the full_cache_mask & part needed here? It's not like the second 
>and can grow bits outside of full_cache_mask even if that would overflow 
>the full_cache_mask (it won't be testing hole then though but that 
>problem happens also at the boundary condition one bit prior to 
>overflowing the mask).

Okay, I see what you mean. Thanks, I'll remove the first operation. While
testing it also occurred to me that the the 0xf wide hole is offset by two bits
to the left so I'll adjust that in the next version.

>> +	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");
>
>Newline.

Sure, will add.

>> +	return !ret == !sparse_masks;
>> +}
>> +
>> +static bool noncont_cat_feature_check(const struct resctrl_test *test)
>> +{
>> +	char res_path[PATH_MAX];
>> +	struct stat statbuf;
>> +
>> +	snprintf(res_path, sizeof(res_path), "%s/%s/%s", INFO_PATH,
>> +		 test->resource, "sparse_masks");
>> +
>> +	if (stat(res_path, &statbuf))
>> +		return false;
>
>This looks generic enough that validate_resctrl_feature_request() should 
>be somehow adapted to cover also these cases. Perhaps it would be best to 
>just split validate_resctrl_feature_request() into multiple functions.

As in conditionally call a function inside validate_resctrl_feature_request()
that would check for the existance of a particular file that would indicate if a
feature is supported or not?

Does implementing it as a new entry in resctrl_test struct that would hold the
desired filename seem reasonable? Or would it be better to pass it as a new
function argument (similiar to how "const char *feature" is used now)?

>> +	return test_resource_feature_check(test);
>> +}
>> +
>>  struct resctrl_test l3_cat_test = {
>>  	.name = "L3_CAT",
>>  	.group = "CAT",
>> @@ -357,3 +438,19 @@ struct resctrl_test l2_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 = "NONCONT_CAT",
>
>> +struct resctrl_test l2_noncont_cat_test = {
>> +	.name = "L2_NONCONT_CAT",
>> +	.group = "NONCONT_CAT",
>
>I think these should be grouped among "CAT" group because it well, tests 
>CAT functionality. Why you think a separate group for them is needed?

It was convenient for my test purposes and I guess I didn't rethink that part
later. So you're right, it fits better grouped with CAT, I'll change it.
Ilpo Järvinen Nov. 20, 2023, 11:07 a.m. UTC | #3
On Mon, 20 Nov 2023, Maciej Wieczór-Retman wrote:
> On 2023-11-17 at 14:55:54 +0200, Ilpo Järvinen wrote:
> >On Thu, 9 Nov 2023, Maciej Wieczor-Retman wrote:
> >
> >> Non-contiguous CBM support for Intel CAT has been merged into the kernel
> >> with Commit 0e3cd31f6e90 ("x86/resctrl: Enable non-contiguous CBMs in
> >> Intel CAT") but there is no selftest that would validate if this feature
> >> works correctly.
> >> 
> >> The selftest needs to verify if writing non-contiguous CBMs to the
> >> schemata file behaves as expected in comparison to the information about
> >> non-contiguous CBMs support.
> >> 
> >> Add tests for both L2 and L3 CAT to verify if the return values
> >> generated by writing non-contiguous CBMs don't contradict the
> >> reported non-contiguous support information.

> >> @@ -342,6 +342,87 @@ 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 res_path[PATH_MAX];
> >> +	char schemata[64];
> >> +	int bit_center;
> >> +	FILE *fp;
> >> +
> >> +	/* Check to compare sparse_masks content to cpuid output. */
> >> +	snprintf(res_path, sizeof(res_path), "%s/%s/%s", INFO_PATH,
> >> +		 test->resource, "sparse_masks");
> >> +
> >> +	fp = fopen(res_path, "r");
> >> +	if (!fp) {
> >> +		perror("# Error in opening file\n");
> >> +		return errno;
> >> +	}
> >> +
> >> +	if (fscanf(fp, "%u", &sparse_masks) <= 0) {
> >> +		perror("Could not get sparse_masks contents\n");
> >> +		fclose(fp);
> >> +		return -1;
> >> +	}
> >> +
> >> +	fclose(fp);
> >
> >Add a function to do this conversion into resctrlfs.c.
> 
> By conversion do you mean the above calls to fopen, fscanf and fclose? Or did
> you mean the below __cpuid_count? Since you mention making get_cache_level
> non-static I assume the first is the case but just wanted to confirm.

You convert the 0/1 read from a resctrl related file into (unsigned) int 
here. Create a helper function for that into resctrlfs.c that returns int 
(to be able to return also errors) and just call it from here with the 
feature string you're interested in, the helper should deal with the rest.

> >> +	return !ret == !sparse_masks;
> >> +}
> >> +
> >> +static bool noncont_cat_feature_check(const struct resctrl_test *test)
> >> +{
> >> +	char res_path[PATH_MAX];
> >> +	struct stat statbuf;
> >> +
> >> +	snprintf(res_path, sizeof(res_path), "%s/%s/%s", INFO_PATH,
> >> +		 test->resource, "sparse_masks");
> >> +
> >> +	if (stat(res_path, &statbuf))
> >> +		return false;
> >
> >This looks generic enough that validate_resctrl_feature_request() should 
> >be somehow adapted to cover also these cases. Perhaps it would be best to 
> >just split validate_resctrl_feature_request() into multiple functions.
> 
> As in conditionally call a function inside validate_resctrl_feature_request()
> that would check for the existance of a particular file that would indicate if a
> feature is supported or not?

I meant that validate_resctrl_feature_request() should probably be split 
into 2 or 3 functions, each taking different arguments and one them 
checks mon_features, another presence of sparse_masks file (any file on 
the level actually).

> Does implementing it as a new entry in resctrl_test struct that would hold the
> desired filename seem reasonable?

I'm not convinced it's useful to put it into the test structure. A simple 
function that calls into one or more of the functions provided 
in resctrlfs.c seems enough for me.

> Or would it be better to pass it as a new function argument (similiar to 
> how "const char *feature" is used now)?

I'd create a separate function in resctrlfs.c instead (IMO, a new function 
should be also done for those callers which currently use const 
char *feature).
diff mbox series

Patch

diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index bc88eb891f35..6a01a5da30b4 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -342,6 +342,87 @@  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 res_path[PATH_MAX];
+	char schemata[64];
+	int bit_center;
+	FILE *fp;
+
+	/* Check to compare sparse_masks content to cpuid output. */
+	snprintf(res_path, sizeof(res_path), "%s/%s/%s", INFO_PATH,
+		 test->resource, "sparse_masks");
+
+	fp = fopen(res_path, "r");
+	if (!fp) {
+		perror("# Error in opening file\n");
+		return errno;
+	}
+
+	if (fscanf(fp, "%u", &sparse_masks) <= 0) {
+		perror("Could not get sparse_masks contents\n");
+		fclose(fp);
+		return -1;
+	}
+
+	fclose(fp);
+
+	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))
+		return -1;
+
+	/* Write checks initialization. */
+	ret = get_cbm_mask(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)
+		return ret;
+
+	/*
+	 * 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 = ~(full_cache_mask & (0xf << bit_center)) & 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");
+	return !ret == !sparse_masks;
+}
+
+static bool noncont_cat_feature_check(const struct resctrl_test *test)
+{
+	char res_path[PATH_MAX];
+	struct stat statbuf;
+
+	snprintf(res_path, sizeof(res_path), "%s/%s/%s", INFO_PATH,
+		 test->resource, "sparse_masks");
+
+	if (stat(res_path, &statbuf))
+		return false;
+
+	return test_resource_feature_check(test);
+}
+
 struct resctrl_test l3_cat_test = {
 	.name = "L3_CAT",
 	.group = "CAT",
@@ -357,3 +438,19 @@  struct resctrl_test l2_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 = "NONCONT_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 = "NONCONT_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 fffeb442c173..51b8a6ff3a0d 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -184,5 +184,7 @@  extern struct resctrl_test mba_test;
 extern struct resctrl_test cmt_test;
 extern struct resctrl_test l3_cat_test;
 extern struct resctrl_test l2_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 9e254bca6c25..fdeef82feb4e 100644
--- a/tools/testing/selftests/resctrl/resctrl_tests.c
+++ b/tools/testing/selftests/resctrl/resctrl_tests.c
@@ -16,6 +16,8 @@  static struct resctrl_test *resctrl_tests[] = {
 	&cmt_test,
 	&l3_cat_test,
 	&l2_cat_test,
+	&l3_noncont_cat_test,
+	&l2_noncont_cat_test,
 };
 
 static int detect_vendor(void)