Received-SPF: pass (google.com: domain of linux-kernel+bounces-208679-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <0e153d27-fa1d-4c47-8d4e-a2004c991ffc@amd.com>
Date: Mon, 10 Jun 2024 12:51:25 -0500
User-Agent: Mozilla Thunderbird
Reply-To: babu.moger@amd.com
Subject: Re: [PATCH v2] selftests/resctrl: Fix noncont_cat_run_test for AMD
To: =?UTF-8?Q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Cc: fenghua.yu@intel.com, Reinette Chatre <reinette.chatre@intel.com>,
 shuah@kernel.org, LKML <linux-kernel@vger.kernel.org>,
 linux-kselftest@vger.kernel.org,
 =?UTF-8?Q?Maciej_Wiecz=C3=B3r-Retman?= <maciej.wieczor-retman@intel.com>,
 peternewman@google.com, eranian@google.com
References: <3a6c9dd9dc6bda6e2582db049bfe853cd836139f.1717622080.git.babu.moger@amd.com>
 <7679d70a0ea939db13ae9dac20de56644460d6df.1718035091.git.babu.moger@amd.com>
 <91052cec-071b-7cc7-5f85-4fa29ec2aea4@linux.intel.com>
Content-Language: en-US
From: "Moger, Babu" <babu.moger@amd.com>
In-Reply-To: <91052cec-071b-7cc7-5f85-4fa29ec2aea4@linux.intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?aTJaNXNzaE4zajFzblM1WkFza2RRckszSXdTU254VHJzT1RwUHF2aFNvc0Nr?=
 =?utf-8?B?bms5TkhKRmgwcXl2MmxsMnl5a2xnbVpveC9KSFlqMU9aN3Jud29CRWFoYm5s?=
 =?utf-8?B?NTlvd2NKUzZycUlRcG44eWZjRVh4RGpJRThTWko2SGdKSXpTbStacE9zb1Vp?=
 =?utf-8?B?WlBTOWlSYURjNzVnOUlDa1VlUjJ5QlRZWDhPb2JQc01MN2VUTDYwZVY3alNs?=
 =?utf-8?B?aVpwcncvMFBWVm1kM3pBcHhlckRGQzlvdUdhOW93UzVDZ25JN2pFeGcyU0xQ?=
 =?utf-8?B?UE9aZE9KNWlhOGNaK0NHODZGRHhjWktJa2x4ek5DSytTcjU0VXB5S0VBWWVr?=
 =?utf-8?B?dlNSWkFjRXJaZEJxeUY0VUMzRSthK1Q4NHlVSmE4bFJSM3M2Z3AvenhXRmZm?=
 =?utf-8?B?RXhSTGlwODdQRGJBQkVIZk4vODNHdjhicjd0dHhxTkcvMUc3Zk5OcUN1dWFa?=
 =?utf-8?B?ay95b0FBM1N2UmxPMUFXUmhSclRsSEU0KzZNOHpLeGNBMGc3S2pXWWZ2L2dR?=
 =?utf-8?B?bEl6WkV4VURVUEpVZnRpSGNXSTBhUm0rOFNVVU9LcHZKYkJPTmdzVk8xT05u?=
 =?utf-8?B?SFR6RXVPQldDL3NQU2lsVUZFK1dNMFZHcFlSRnQ1L1Q4ektUVHNDNUlNejRH?=
 =?utf-8?B?MWo2ZmIrSUVKLzJUMTFhN3p6S0IwRFRsNzNNdmRzTE5xRVQxWnIxdWIwRFF5?=
 =?utf-8?B?ZVJOSE94VzdRY1MvQzlFT2swSTFGVGtlVStuVjZJVGxTSFFzTlREUGpmT2tT?=
 =?utf-8?B?ZzkrRGxkNTBDUjJIaHA2VmpyTVhEMXZTZ2xaNnVUZi9NSGpZeWFtRUVaY0sz?=
 =?utf-8?B?MGVSbGM1M1VHeWR1OXdDS1FaUGlhNUdza0JRUThvT2dLK1d6RmgxNkFnbDdE?=
 =?utf-8?B?cVBxWXppajRIVm05Ulc4cUoydEpTSS9ObERBT09hR2RXN2hKZDd3T2pVOEVu?=
 =?utf-8?B?L2NDOG9acUhjTllOUEkrUVVFUGxjTzRBMFVWUVdpaGtHTUZURFlLZ2hJZzUz?=
 =?utf-8?B?ZkdBQ3hFMjNRdmEwbnZuWElIYWNhMVRWUlVBOXpGN3d0Tm1hZkVYS2RhWVkv?=
 =?utf-8?B?NldpNHUwSlBUZ2pHcjhscC9ra3pqUnZ0V3drckFRZ2xtb1QwUFJ3ZDYzaTNt?=
 =?utf-8?B?c2RMdkxNcU5aOEdHbUlhbW5pYzB5ckZKK3dYbVBFaVBkaHJLdVNTZEl1Q3F3?=
 =?utf-8?B?NThvaW9kNG5NSHU0R0JQOXdrL1N6RU5wQmhVNXJiMVU3djR4OVc4S2RvT3Vq?=
 =?utf-8?B?azBSNlB2VTI2elF3TkY0cVlTdDljaG1hTkFWMWI3S0htOFJoeUdhaUdZVnQ4?=
 =?utf-8?B?d3pZb1RrdnRzMExieHpFZ0Z5VWxoWVU1a0ZTVHZXWHU4aVZ0cDhqc200em1E?=
 =?utf-8?B?SlhkSDRIbjFPcnk3dit5STZRdmthMnRzSHBCTjViZkZ0aE9WSy9GNjVIOXhO?=
 =?utf-8?B?L1dFSFowNEs3TVRlcXFRR2R0TWRoR2FWdkFqUTJMVHFzOHcyU2xaY0xTaHRi?=
 =?utf-8?B?aGk4K3poZzdLbnpDOHBIWmFzbHd5TWErdWZVRE9CeStUUVB0Z2p0bk1Mblp4?=
 =?utf-8?B?R3VUNWh5Wk83NUh0S1lmWE1tM09mVWpXeFhzT3M4VE41ZXJEZlA1bzhwYW4x?=
 =?utf-8?B?d095YkZHc3dCQXZORFNkN0U4eXlQaCtOZVlKNGxiSHhudm1hSGU4UHhvTjNu?=
 =?utf-8?B?T1RJZW8veEc2QWtyanhmZDFsZ3ZRaWJqbnhOdGRaRzg1czdBWEhja2VhRTFk?=
 =?utf-8?B?dFRudWZIMXNOd2s5SUszZndTMTUwRmY0MHgwSm14WVpXMlo5SjdUNzEyczJh?=
 =?utf-8?B?RlU2ekQxSktZV1YyamZiaVg0WkxaQ0JxNkVBU1ZDZmFrV2g2UzJENTBDbWdo?=
 =?utf-8?B?ZW1jWGRMcS84aUt0SDFQaEhQNEZUb2tiTlFieU53Z1kvMHdHdXJOVUYrZDgy?=
 =?utf-8?B?YlhhL1lDTFA3NzlubUt2Nm1FRG8yVjBzM2tVa2Z2VnJCdnFlL1pnSDhpcStP?=
 =?utf-8?B?T1BuYzNsVWJYZlNnUW0xbkR2OThJOXphTzJnSWlCUUFRUHBYWEgyNzJJK3Rj?=
 =?utf-8?B?dUdNZTBQNW1HN1p3M0s3bGxMTWlpcFNsK0RWcDhmU2lLY2pRZis2SFg4dDBG?=
 =?utf-8?Q?AfSo=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 60953e25-e388-46c1-d378-08dc8975f41f
X-MS-Exchange-CrossTenant-AuthSource: MW3PR12MB4553.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Jun 2024 17:51:28.1146
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: r/hmDD4qf8WUsxcoxYASX7PFIRLBa8UNQ+5Jezj1YSSP+wwCBv57O2GnRUNp2Jbb
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR12MB5953

Hi

On 6/10/24 11:20, Ilpo Järvinen wrote:
> On Mon, 10 Jun 2024, Babu Moger wrote:
> 
>> The selftest noncont_cat_run_test fails on AMD with the warnings. Reason
> 
> noncont_cat_run_test()

I want to mention the test here. not function. How about this?

"The selftest non-contiguous CBM test fails on AMD with the warnings."

> 
> (In general, use () when refering to a function, same thing in the 
> shortlog).
> 
> "the warnings" sounds like I should know about what warning it fails with
> but there's no previous context which tells that information. I suggest 
> you either use "a warning" or quote the warning it fails with into the 
> commit message.
> 
>> is, AMD supports non contiguous CBM masks but does not report it via CPUID.
> 
> non-contiguous

Sure.

> 
>> Update noncont_cat_run_test to check for the vendor when verifying CPUID.
> 
> ()

Sure.

> 
>> Fixes: ae638551ab64 ("selftests/resctrl: Add non-contiguous CBMs CAT test")
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>> v2: Moved the non contiguous verification to a new function
>>     arch_supports_noncont_cat.
>>
>> v1:
>> This was part of the series
>> https://lore.kernel.org/lkml/cover.1708637563.git.babu.moger@amd.com/
>> Sending this as a separate fix per review comments.
>> ---
>>  tools/testing/selftests/resctrl/cat_test.c | 32 +++++++++++++++-------
>>  1 file changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
>> index d4dffc934bc3..742782438ca3 100644
>> --- a/tools/testing/selftests/resctrl/cat_test.c
>> +++ b/tools/testing/selftests/resctrl/cat_test.c
>> @@ -288,11 +288,30 @@ static int cat_run_test(const struct resctrl_test *test, const struct user_param
>>  	return ret;
>>  }
>>  
>> +static bool arch_supports_noncont_cat(const struct resctrl_test *test)
>> +{
>> +	unsigned int eax, ebx, ecx, edx;
>> +
>> +	/* AMD always supports non-contiguous CBM. */
>> +	if (get_vendor() == ARCH_AMD)
>> +		return true;
>> +
>> +	/* Intel support for non-contiguous CBM needs to be discovered. */
>> +	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 false;
>> +
>> +	return ((ecx >> 3) & 1);
>> +}
>> +
>>  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, sparse_masks;
>> +	unsigned int sparse_masks;
>>  	int bit_center, ret;
>>  	char schemata[64];
>>  
>> @@ -301,15 +320,8 @@ static int noncont_cat_run_test(const struct resctrl_test *test,
>>  	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");
>> +	if (arch_supports_noncont_cat(test) != sparse_masks) {
>> +		ksft_print_msg("Hardware and kernel differ on non-contiguous CBM support!\n");
>>  		return 1;
> 
> This looks better than the previous version, thanks.

Thanks.
Babu Moger