Received-SPF: pass (google.com: domain of linux-kernel+bounces-99314-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <1cacbd08-1131-4be4-b318-58c05afda2de@intel.com>
Date: Mon, 11 Mar 2024 10:39:10 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] selftests: x86: skip the tests if prerequisites aren't
 fulfilled
To: Muhammad Usama Anjum <usama.anjum@collabora.com>, Shuah Khan
	<shuah@kernel.org>, Dave Hansen <dave.hansen@linux.intel.com>, "Kirill A.
 Shutemov" <kirill.shutemov@linux.intel.com>, "Peter Zijlstra (Intel)"
	<peterz@infradead.org>, Weihong Zhang <weihong.zhang@intel.com>, Binbin Wu
	<binbin.wu@linux.intel.com>, angquan yu <angquan21@gmail.com>
CC: <kernel@collabora.com>, <linux-kselftest@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
References: <20240307183730.2858264-1-usama.anjum@collabora.com>
 <dc8d122a-22b7-4d17-abd9-66262af0b058@intel.com>
 <c3362840-365e-40cb-80fe-895aa2d979ec@collabora.com>
Content-Language: en-US
From: "Chang S. Bae" <chang.seok.bae@intel.com>
In-Reply-To: <c3362840-365e-40cb-80fe-895aa2d979ec@collabora.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
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?bUM3LysvNjdxYVRkY08wa2NoS2ZUaTV3S3p4M3lhZm1RT3FmNDh0SlJENHZ1?=
 =?utf-8?B?ZXlrR3VKREk3YndIWFk5MHFPUG1UeGMrL0dYS2NWejdxdWxHS1J5N1cyb01w?=
 =?utf-8?B?QVpCeWQyUUIrL21nd3BWNE1ldTJjOEtBSkUzNUpPcWNzeWNlbml2eFNleE5h?=
 =?utf-8?B?eC9tZHFieU5DYmVWeXNsOFRmRlBjR3BQcGtWNTg2Rmg4MERtVnpWeHZ1U1VG?=
 =?utf-8?B?QUp0ZWVRWHVLT3pKc1BOUTd3UURCYkNzSUU4SFh5ejVNMldrRnJlYXZMeElk?=
 =?utf-8?B?YUpBQ1FUZEhCMjI0RFNVTlUyaS9BamJLS012VDhjdDlnNEQ5K3d6SFFYcW85?=
 =?utf-8?B?QzE3d1dQNDB0UStIRmpYZHZqT2c0eEl5RHBGRGdCUHE2L3RTUHRtNmREa1Vn?=
 =?utf-8?B?QnhjUll3ZHNqSUZ1Mk95ZWU4YjJKb3NkdGFFY1B3blFvcjdQQlRxanoxN1Vy?=
 =?utf-8?B?MHh1bm4vOU9FYVB3ZWhmUDZYeHpCajZCWjhnRFhhZCtMOEJ3c2gzSUhFNkFv?=
 =?utf-8?B?U0dLMVFBenVZU3crWVVlMTZrVXpHWUMzcktkV0h4Y2RhVTk2amNaU2lzaWRZ?=
 =?utf-8?B?VmJvci9Dd2Z3NmdtbXJTOXBLOW9OM21Tb3Z1UTZEUGpNUHAwRWhObGs0YWhT?=
 =?utf-8?B?NGhaenpSaVJhWDdKYXBnYjNrMEdBTFJMZkJ2UGIzS2I5M3p2T2R0QXBkQ1BO?=
 =?utf-8?B?Y29UN1gwZG85V1pZTENrVDg5RkliRWxzVXJRTHdyTmxrMUxmTjNPSDNudFBT?=
 =?utf-8?B?N0ZEZEFUTTVtcGNKZG45YTJhakgrcHVmRzhGelVUMk5xN2xGWUttMFpCc1Y0?=
 =?utf-8?B?ZEh5NFpWVEZWZHpsNFpvVk1USlZlVGNkeHVhRzhsS3RuZDR0ME53SG4ybm9k?=
 =?utf-8?B?QzM2QU85bnJFSzY1TTFmU01ONUxHclR1ckpYRHN5UlZPdCtJM2g0Y0NHeTNE?=
 =?utf-8?B?REZmUlBoUyt6U2J6bFRUMS91Unp0RXlJMFYybkRZVjczT3paMGN3N3phRFQ2?=
 =?utf-8?B?OEg4eW53UENyZXZOTUlUS0lidlJEMVkwc0xkN0g1b1l3VEF4akUyRWxkWkU5?=
 =?utf-8?B?Q3BHNFg4djZ0N1E1L2FYOU9ibGJjU0ROMFU2ZmpYamliQ3VvbjNwc3dGSzJs?=
 =?utf-8?B?T1JFekRBNHJFdHJRRHNseHBZaEpjVms0WEFHQ29jWE9EM2F6cWZpKzZhZ3oy?=
 =?utf-8?B?U1JXcXhYd1crTlVncElzQmdMYUwrcEREcURKYk9ROTRtcWF2OTlmUDdlUTJs?=
 =?utf-8?B?eFN3d09aWVRpbkh4TjlWNEVVdmNKdTVod3hHUStmN3pGMU0zdldDR3JuY0lZ?=
 =?utf-8?B?ODN4S291elp3TC9oWUJsenZTd0JSYzhMR1Y5aWRrVzJoN3pFd0RLZHA5ckF4?=
 =?utf-8?B?SitYQ3dEVkRzOUtsNFVuM1F4Z1JPNTVhaDVEaWxreXZrM0d1Wm4yM2tqNFlS?=
 =?utf-8?B?MEFEcDZnMUVVZkV2WnQvZHFud0pOZXM5RVF2MnFQMmM3QzBiQTBQeDVBcXNV?=
 =?utf-8?B?V3VWRXNaRXlzREtiZ2V6UG5IcmZBQVFjVnFFZzFWei81SkE0NUIxOXV6N3Rx?=
 =?utf-8?B?amdFZGZ1bFZZWUd1NlJOa3FVQmtsQUxqSWNYajdrc1IvdXVyRzYzU0t1VmVk?=
 =?utf-8?B?WkkzV3dnanI2c01ZdXBzSTBUYVBKUWtDdmdCUjJsWVBXL0Y3TFFpajM4YjZR?=
 =?utf-8?B?SzNCODNmcm5BbU1kT3hZYks3bnlEcEVjeTIrMWVYS0UrOUQ4NjdFRnRqNDA1?=
 =?utf-8?B?VlA2bWFseW5HcWFNMkdpZ1lHNS9CcWZlckQzT1JhKzlFc0RKSHpxdmNNbSsv?=
 =?utf-8?B?eGZaUVNpQmJzaXV0aXNMazA0NGN2N2FqYUU4ZisvcVlGZ014NDZqVzU0dG12?=
 =?utf-8?B?L3pKUmlyaVRUbTkvckJZMnVqWlp2TUpBSmxpOFVwQ2ZCUU9CaXp4cEtPVXVt?=
 =?utf-8?B?aUkxRXg1ZnBLS2hwWXZZNXpGWmlSalRaMjVOQ2ZTTDJUcmtWT3p3NnEyN0F2?=
 =?utf-8?B?MDdGeGdkTnAzc0ttaUt6R3pjaXl6SDBUMGx5MENWV0xrM1NXK2hXcy8zM0Z3?=
 =?utf-8?B?ZVBtSStHaU41OXhPT2tWTDl4WEdUWVMrZ0RHRkRSYzl1Ti96cGNuYTUvellK?=
 =?utf-8?B?b01aSjZMamlJZUk5cDJRUHAwY3hxd2FCQW5hY0ZnNTNCcmJvL0MyaERzbjBs?=
 =?utf-8?B?aVE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 2a565294-4764-4ae9-769d-08dc41f22a74
X-MS-Exchange-CrossTenant-AuthSource: MW4PR11MB7164.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Mar 2024 17:39:13.0991
 (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: jNHJ8fPkVzlqtRwiC9Lv+8hHBcWJ9m62nRwaWg1G7NdODtUfsG0ZydYgvYKp4rZEGwq6zPc7cOzdJ64I3H+P6e0NjmmN5zMD5RtBGLt3WBE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR11MB4745
X-OriginatorOrg: intel.com

On 3/11/2024 10:02 AM, Muhammad Usama Anjum wrote:
> On 3/9/24 6:06 AM, Chang S. Bae wrote:
>> On 3/7/2024 10:37 AM, Muhammad Usama Anjum wrote:
>>
>>> -static void check_cpuid_xtiledata(void)
>>> +static int check_cpuid_xtiledata(void)
>>>    {
>>>        uint32_t eax, ebx, ecx, edx;
>>>    @@ -153,12 +160,16 @@ static void check_cpuid_xtiledata(void)
>>>         * eax: XTILEDATA state component size
>>>         * ebx: XTILEDATA state component offset in user buffer
>>>         */
>>> -    if (!eax || !ebx)
>>> -        fatal_error("xstate cpuid: invalid tile data size/offset: %d/%d",
>>> -                eax, ebx);
>>> +    if (!eax || !ebx) {
>>> +        ksft_print_msg("xstate cpuid: invalid tile data size/offset:
>>> %d/%d\n",
>>> +                   eax, ebx);
>>> +        return -1;
>>> +    }
>>>          xtiledata.size          = eax;
>>>        xtiledata.xbuf_offset = ebx;
>>> +
>>> +    return 0;
>>>    }
>>
>> I don't think it is okay to silently skip the test here. If the feature is
>> available, the tile data size and offset should not be zero.
> We are logging that data size/offset are invalid if either eax or ebx are
> invalid and then we are skipping. Not sure what you are asking me to change.

You intention seems to skip the test when AMX is not available. But this 
function should only be invoked when AMX is actually available, not as 
part of the feature availability check. Therefore, I think this change 
is not relevant. Also, if we encounter invalid TILEDATA CPUID, it should 
be a reason to *fail* the test, rather than calling out a skip, right?

Thanks,
Chang