Received-SPF: pass (google.com: domain of linux-kernel+bounces-52711-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Mon, 5 Feb 2024 14:25:50 +0100
From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
To: Ilpo =?utf-8?B?SsOkcnZpbmVu?= <ilpo.jarvinen@linux.intel.com>
CC: <fenghua.yu@intel.com>, Reinette Chatre <reinette.chatre@intel.com>,
	<shuah@kernel.org>, <linux-kselftest@vger.kernel.org>, LKML
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 5/5] selftests/resctrl: Add non-contiguous CBMs CAT
 test
Message-ID: <m76an3zrp375meemqdqv2tx577kwyjy6g64ain2wp2vvnyx4ex@zby3einmlszv>
References: <cover.1707130307.git.maciej.wieczor-retman@intel.com>
 <b341ecc8d1143932307708aff44ab90db3a91564.1707130307.git.maciej.wieczor-retman@intel.com>
 <81064977-28cd-10bf-6721-c5a0916dee85@linux.intel.com>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <81064977-28cd-10bf-6721-c5a0916dee85@linux.intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?fAOytU4HuRCu2uEMWa8teKisWYWsx+edIfVBedvjP4yqid8NUawqlamAsT?=
 =?iso-8859-1?Q?fDUPDZvA71F/mBlOhgaudKs66uAtFzQ86RJYE9fIJUwgjtnkl70E1FIIbD?=
 =?iso-8859-1?Q?LgYS4s6EQq81uKLUicPWPmG3PILj7tdb5amxSr1BBWDUWVmUQ3YYldQ1yd?=
 =?iso-8859-1?Q?KrIqWJi0il4Ddf2T3ajN+s6X8HcWNN3wtKDoqpVsFmR5xvVwELit5tyElZ?=
 =?iso-8859-1?Q?SnSsjGzEkp6IuEQCQzwinhlvW2jniLT5buGTfn10lMCJtKKn88J6RZRvIs?=
 =?iso-8859-1?Q?ll5tgDYbcpBwtW5cKS87RzsXe3OGCMcTt0GLAyx0h3KOESO5QjQ5Ofu3Us?=
 =?iso-8859-1?Q?Kltej760Mktc2NJiJwUQVuEb1iLALiUUdWBVlFtQ7URueXP8ItF/P6rFr/?=
 =?iso-8859-1?Q?kI6qMzKZosqShaYf0HmoUj3teuKUNcHjvp5umdKedqgMtjvFhXpfYbnUU9?=
 =?iso-8859-1?Q?a4045uqjytxGwdWHLOHQlgi2L63nmnvA2K9ztYiKbE84xzVjBCU68a5NAn?=
 =?iso-8859-1?Q?pI/1tJakG4+Y3x7eolebGK/NGk1+IAdvwL/klnOEYGNcO44X5n2RGO12l3?=
 =?iso-8859-1?Q?6QP1nMmr0V6jfXRj72jDlZHFtfKK78CSCdZ38ti6gXMWsjpIQH2YklmKW3?=
 =?iso-8859-1?Q?0K0CP1IpIGjL2j6wpWvOR0X7zDjZ6kxhZKn+y62rCkPXYDax5dHdCPBxDM?=
 =?iso-8859-1?Q?Qr2aqpD/v0QPpZ9vi3YBiD1G2G8vhC6AkDfkfS80M7qvcfICkfmRF3a5V2?=
 =?iso-8859-1?Q?OduN6Be/Bh5953gwT9OVujFhO3YHYl7swoH3tC+YvC/QkrimhzdRUjDwkg?=
 =?iso-8859-1?Q?s9Vbcbcwsrp0ZDQGZD2Dhm+0f6ixiZ+hb/uBtDGm1z9O2QhYL9d+gey4VB?=
 =?iso-8859-1?Q?bRacfACT2OuTry+AJfdhg2nG2zwgiA1VzDGABkn6HCoREeJKXr77fPOXna?=
 =?iso-8859-1?Q?B678VOHjHXEL+ZhRUHn7IVgQlzx6EPRAhT0dTPi/DAlMKfpanIq5FvReLc?=
 =?iso-8859-1?Q?equuNhwLSYpVhG+CIlzg6c4QJKYRdqD32hExnxdS3IfN4uMAjqImivJNr1?=
 =?iso-8859-1?Q?530eN96VC8fDr0tbpdkiRicYHYMt9CRwHdY56FYWW6ArP8HduQmKrk6u1K?=
 =?iso-8859-1?Q?1pMBMIK1kHRiaCrRUMQcA8L756Cc4Cf6MtWBqL+sGNDLVeD0cgGLxlaLCs?=
 =?iso-8859-1?Q?R1wePPpjAXl/9Fw0JMxpMiPYYawoRum7pPZW/6UIFH+VnoVvzKub1O4Nds?=
 =?iso-8859-1?Q?MCAgjXQ48Um3gOdOba5tQVBi9m7ERZ39Rf5W499vcM25Ig6dXB7d9JLCnO?=
 =?iso-8859-1?Q?blx0SxWIU2QeaYHQvtIudEQeWCybToKxLGJ3+4ii96GCY8GQDawtJJqtv5?=
 =?iso-8859-1?Q?irb2OgKTEO1U/5aHrjytoryvRDCNgtclJdrmFNWDPn5E7T+tBPzwp2N2al?=
 =?iso-8859-1?Q?nPLXSKElYV/+lK0oVLpOk3uBvgYVEP4dV7D/SVCLAm/4pc8AvwTB2v9Prt?=
 =?iso-8859-1?Q?61REeAF0hfVPqxvOprR7c0Gx9pDglHPoAUf0sBDuED234+mNVoLdLGEVEG?=
 =?iso-8859-1?Q?ovAr7t62uSiUs63h7ZPfC4aISmYAbWyYERGGKXVZU+8gCR4f4U+Td1fJIm?=
 =?iso-8859-1?Q?9pYaKFFV+EzXKZB4MxAIlCCD2Swp4cLzCDiZCJX3F4mlbBn0Yb+sU5I+yD?=
 =?iso-8859-1?Q?6FCP/cOJid8/DgVSNXc=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 198069ab-85c9-4b22-827a-08dc264dfd42
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6231.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Feb 2024 13:25:58.4361
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: EnEmFK6bbwQKD67n7Bre4EBs8Hla8QUxEhjpgQh3L1+QzR+0hi1sZhTtI1sSxHUbf0oida3n9YHQlhzFk6si/jp4YUGqbfcvKgl7uu5bdc8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB4944
X-OriginatorOrg: intel.com

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@intel.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@linux.intel.com>
>
>
>-- 
> i.


-- 
Kind regards
Maciej Wiecz?r-Retman