Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Message-ID: <89f48b0a-a6b6-2204-7d96-28ebbe96b18a@intel.com>
Date:   Mon, 25 Sep 2023 09:08:31 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH v2 4/9] platform/x86/intel/ifs: Gen2 Scan test support
Content-Language: en-US
To:     =?UTF-8?Q?Ilpo_J=c3=a4rvinen?= <ilpo.jarvinen@linux.intel.com>
CC:     Hans de Goede <hdegoede@redhat.com>, <markgross@kernel.org>,
        <tglx@linutronix.de>, <mingo@redhat.com>, <bp@alien8.de>,
        <dave.hansen@linux.intel.com>, <x86@kernel.org>, <hpa@zytor.com>,
        <rostedt@goodmis.org>, <ashok.raj@intel.com>,
        <tony.luck@intel.com>, LKML <linux-kernel@vger.kernel.org>,
        <platform-driver-x86@vger.kernel.org>, <patches@lists.linux.dev>,
        <ravi.v.shankar@intel.com>, <pengfei.xu@intel.com>
References: <20230913183348.1349409-1-jithu.joseph@intel.com>
 <20230922232606.1928026-1-jithu.joseph@intel.com>
 <20230922232606.1928026-5-jithu.joseph@intel.com>
 <c390bdaf-ab5c-bf1f-bd64-29e2827d01f@linux.intel.com>
From:   "Joseph, Jithu" <jithu.joseph@intel.com>
In-Reply-To: <c390bdaf-ab5c-bf1f-bd64-29e2827d01f@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VlZiN1ZWQlI3WlRSVE1nbE5DN24vTHZreFRuM2ptUnFkRjBybFppcGZ1V25C?=
 =?utf-8?B?TXdMTzl6b2JRL0p5RnhmMXJ2Q0o5RkVsRE5wRTlrMDBtZm5ReUhaVUdqQWhU?=
 =?utf-8?B?S0N3am1ONGx2L2hZNUFYaEZUVndadTAxQzdpQlVOSGZpNHlJaXA1VWxCS1RY?=
 =?utf-8?B?WDFUZTJXUEtoOE1mbjU5TkkxOWJtMkpHR0pqbXVycjdXdzVVSHdOcHlnVzlB?=
 =?utf-8?B?NEtiNzVkV05GU0RrMysraFRTUExidTlNL2hkVFE0UU9TcmJCNTQ3Vk9EMm1V?=
 =?utf-8?B?NnAwVFR3cHNYdnQ5Vmx4bWhXOEtkSTNoQUFXUmxySWxXT0tLZWpwSzBVU0to?=
 =?utf-8?B?VXRIT3dWK2FhcHVFb01meVlyZ3RUOVQ2N2tUNm50bXFUQWx5a01Vc1ZZWUcr?=
 =?utf-8?B?RzlJcU8wT1ovVXVQN3NKRTRjbWhjVXVadlRJVFYxblRUWE1IMCtOcldiYnl1?=
 =?utf-8?B?UXZxWmVZcUt4WHVyT2k0UWtsZ1BBSENkUTY2TXI1OTlOWVlCamtiZ1lQLzNk?=
 =?utf-8?B?QUhPb2NQTlBDMnBOYnY3Tk95SGtKTHB3Zkg4TC95bTBlbUY2WlNzdUhuZytT?=
 =?utf-8?B?T0tIREZDOFhNRVV3aU8yRW1RamdJTG0rR1pQYVZLektlYStFL0NueWI4b3lW?=
 =?utf-8?B?bXRaTWExaVU0UHY3NGErV3dEMzhycllra0NORkR0MDJQR1VJb1A0cHRtTzNC?=
 =?utf-8?B?bGQ5K3B1N2pVTmdkaUlZM0lZZERhcWp6Rmh5Vitic1lkN1pRZWg3Ym5xbXJ0?=
 =?utf-8?B?aitDZHEzWlhGZUFYNjZyUS9QOUlHb2l6ZkZEYXVZK05IczFXRFJWbXcrTzlh?=
 =?utf-8?B?S1Y1M2ZmY1BqaFZHcklMcVNURGk0K01KR3pwNWxJbzdTZ2xnc0R5YnFCcE92?=
 =?utf-8?B?Zmt0MjdZV1RPd1oxR1E2Y0dHMXFKd3RvWEg2RDlWeFdDWHl3bE5wbjM2a1N0?=
 =?utf-8?B?cVAvOFBHOGlMWi9FWmk3VUpzV0FvRStGS3oxL3B6b3UvellPbmRQdmFzWGx5?=
 =?utf-8?B?MXdyS2Nud0ZhVjJOdkJ4V3oyMERzUkprelV3bG8rK0lLOTdBKzV1ajNIOE1H?=
 =?utf-8?B?UXNXZ3B1Y0JiUXZpeVNLb3hkaEVPbHJMeEozQ2VIS3NxeitSQ1JwQzlSdmYz?=
 =?utf-8?B?cU1MeENaQjZUcGpybUtFMWZpQXp1S1RodVFrWHRsaC9rUUFXL1NUbWNNbXRQ?=
 =?utf-8?B?dVBGM2lSMHc5ZXIvbUZqMzJvQnI3WDk5RGErbEhZUkdDRFh3Q0NKVWdZa1BE?=
 =?utf-8?B?RUo2Z0YwQ3BwV1BITVhCd1VKR0ttTlBSRmNaMGV6U2F0K2FjdzdydHA1NHhh?=
 =?utf-8?B?THNkQ2NYQnEvejZ5TXZaQkhLWGVFMTA0ZkFQdjhlR0NoYlRWWTA1VDg3OTZJ?=
 =?utf-8?B?c2dET1J2cjdqTkJDWDZpT3dWbkJES0JBaGplSEkzdXdISmJxL0Fkb2MxOFEv?=
 =?utf-8?B?dU5CREZkMWxWd1loWUtucXBMNmo4dTV5aGJTUUZmWnhMQmQrVTI5UG9GRUl3?=
 =?utf-8?B?ZzBJL1VLS1BzaURMUGJ3MklMK2F6Wnc2amsyMXV4OWJZZ0dZZkcyRlNjOWxI?=
 =?utf-8?B?eE5SdGJZbG5nM0dReXREd2NsTU9zOUh0SC80aU5ycWJXTnZ2QmE3cm5JaVl1?=
 =?utf-8?B?ZUVLUUxaNS9sSTErb0lxYW1saFBwQlNvMCtvUVFlZDVOVjZPMUk1V1djYTdS?=
 =?utf-8?B?ejZTUWdCSDF4YS9JWXVRMmFxbmhhMFc0U1NoUTZod0c4QUhSS1ZHNmhld3lV?=
 =?utf-8?B?cFVpTWxGSFlMbFM5blFjbG1lWk4xOGlMQnlNRVhkaFR4eGtGaHB5SkY5dzB1?=
 =?utf-8?B?eEFvWExPRVRKWEc1SDJ2ZlRFVzRHNTRTVnlFbWl6R2pQaUt2Qm5NRCtZZlU0?=
 =?utf-8?B?ek1iWTA0YXBnZnh2am9lMlplaVZmZ3RFVHVmTk1hZy9tN2V1d2JFZ2cyQlh0?=
 =?utf-8?B?WVkwQ3JGdkh2ck52MnBMU1FSZ0tXYkRJVnN4aTd4WTVNTC9YSUU5WU1vcW9u?=
 =?utf-8?B?S0xBRE1vTzhXLzdjUW85SnR1SjBHbjFyMUE4SktoUzRXUWRJRS90MWRQYm52?=
 =?utf-8?B?MytRWmMvbXBlMGl3WTA0a0ROTWk4L1pZMjh0djM3cDgvMFRwQlNCbDZZdmMr?=
 =?utf-8?B?dzRqRkJjOEVGNmliOWIzbkdrSzZFYlZqMGhnaUduOGVKQ2Mrai9WR2d2YzVR?=
 =?utf-8?B?WWc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: f76bd38c-8c14-4f3d-f9bf-08dbbde1ac27
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3768.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Sep 2023 16:08:35.8499
 (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: xPHrcXdfVnKrf0+1xMS6AepKqZo9SkmHyOZfjR3kQYVFc1w7KxWasgVu0IE3BTtEWyI3R1CvZVwAp/TZ18BtdA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR11MB7789
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.3 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Mon, 25 Sep 2023 09:12:42 -0700 (PDT)


On 9/25/2023 8:39 AM, Ilpo Järvinen wrote:
> On Fri, 22 Sep 2023, Jithu Joseph wrote:
> 

...

>>  
>> -	activate.rsvd = 0;
>>  	activate.delay = IFS_THREAD_WAIT;
>>  	activate.sigmce = 0;
>> -	activate.start = 0;
>> -	activate.stop = ifsd->valid_chunks - 1;
>> +	to_start = 0;
>> +	to_stop = ifsd->valid_chunks - 1;
>> +
>> +	if (ifsd->generation) {
>> +		activate.gen2.start = to_start;
>> +		activate.gen2.stop = to_stop;
>> +	} else {
>> +		activate.gen0.start = to_start;
>> +		activate.gen0.stop = to_stop;
>> +	}
> 
> Is it okay to not do activate.gen0.rsvd = 0 anymore? If you know it is, it 
> would be nice to record that fact into the changelog so that it can be 
> found in the history.

I did test on a gen0 to check if there is a problem due to this (and it seemed fine).
I will make a note in changelog as you suggest

> 
>>  
>>  	timeout = jiffies + HZ / 2;
>>  	retries = MAX_IFS_RETRIES;
>>  
>> -	while (activate.start <= activate.stop) {
>> +	while (to_start <= to_stop) {
>>  		if (time_after(jiffies, timeout)) {
>>  			status.error_code = IFS_SW_TIMEOUT;
>>  			break;
>> @@ -196,13 +205,14 @@ static void ifs_test_core(int cpu, struct device *dev)
>>  
>>  		status.data = msrvals[1];
>>  
>> -		trace_ifs_status(cpu, activate, status);
>> +		trace_ifs_status(cpu, to_start, to_stop, status.data);
>>  
>>  		/* Some cases can be retried, give up for others */
>>  		if (!can_restart(status))
>>  			break;
>>  
>> -		if (status.chunk_num == activate.start) {
>> +		status_chunk = ifsd->generation ? status.gen2.chunk_num : status.gen0.chunk_num;
>> +		if (status_chunk == to_start) {
>>  			/* Check for forward progress */
>>  			if (--retries == 0) {
>>  				if (status.error_code == IFS_NO_ERROR)
>> @@ -211,7 +221,9 @@ static void ifs_test_core(int cpu, struct device *dev)
>>  			}
>>  		} else {
>>  			retries = MAX_IFS_RETRIES;
>> -			activate.start = status.chunk_num;
>> +			ifsd->generation ? (activate.gen2.start = status_chunk) :
>> +			(activate.gen0.start = status_chunk);
> 
> The alignment of the second line is still not correct but now I notice how 
> the left-hand side is hidden within those expressions. Just do a normal if 
> instead so that it is simpler to understand, please.

In v1 the second line was kept 1 space to the right of previous line.  In v2  I kept
them at the same indent, since your original comment was Misaliged.

I will change these two lines to
if (ifsd->generation)
	activate.gen2.start = status_chunk;
else
	activate.gen0.start = status_chunk


Jithu