Message-ID: <e2cd922a-0a5e-c1ef-3fe7-2347f3b062d1@intel.com>
Date:   Wed, 15 Feb 2023 09:44:39 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.2
Subject: Re: [PATCH v2 4/7] platform/x86/intel/ifs: Implement Array BIST test
Content-Language: en-US
To:     Dave Hansen <dave.hansen@intel.com>, <hdegoede@redhat.com>,
        <markgross@kernel.org>
CC:     <tglx@linutronix.de>, <mingo@redhat.com>, <bp@alien8.de>,
        <dave.hansen@linux.intel.com>, <x86@kernel.org>, <hpa@zytor.com>,
        <gregkh@linuxfoundation.org>, <rostedt@goodmis.org>,
        <ashok.raj@intel.com>, <tony.luck@intel.com>,
        <linux-kernel@vger.kernel.org>,
        <platform-driver-x86@vger.kernel.org>, <patches@lists.linux.dev>,
        <ravi.v.shankar@intel.com>, <thiago.macieira@intel.com>,
        <athenas.jimenez.gonzalez@intel.com>, <sohil.mehta@intel.com>
References: <20230131234302.3997223-1-jithu.joseph@intel.com>
 <20230214234426.344960-1-jithu.joseph@intel.com>
 <20230214234426.344960-5-jithu.joseph@intel.com>
 <a24c65f8-978d-8968-7874-6b83e14b01ba@intel.com>
From:   "Joseph, Jithu" <jithu.joseph@intel.com>
In-Reply-To: <a24c65f8-978d-8968-7874-6b83e14b01ba@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dWFybG9mVDB0SHZsVVRZdlU5dGZEMHpFSVdaZFBxK2FGQ21DZFhyNTJJalpa?=
 =?utf-8?B?Mi9tTE5rUFM4NmdkWTVXOTlMMkE5QVZ5RVJsK1I1TC9pVzgvYkh4cVl4YjBr?=
 =?utf-8?B?ZnlYK1ZFT3NaRWdaYVprUGZZVk5ybHovWDR5QW9BdVB4LzJ1MnV5RmtnOVlE?=
 =?utf-8?B?OFhYS0Y2eUhCWkdva1ZoeGlmVGVScEpMMmQ0dEZ3ak5LNnFlU2ovWXF4eWlB?=
 =?utf-8?B?YVhvUUNqOEw2OWFJZ1RGbWlJdmgzRkZYSU9TOHdYVlhHaVpLR092WnhEb2pa?=
 =?utf-8?B?anJQcFl3TDR3bmp1N1Erdnk3bHVQN2xPVTVuSm9hUlNvbEtwTjRhcjVnSG50?=
 =?utf-8?B?MGlZZG5TWlo3K0VsUEp0U0lselZJV1lDT3lvaFcxdGVLWXhsaEd2Q0F6SW9V?=
 =?utf-8?B?cUl5WU1LaXFlRUZ4S3N4dTUzMUFScjlmeWs1cHUrZTdlVHR5enM3L0lSdXpl?=
 =?utf-8?B?a0owMm1LbitCaU40UysvdjN3Z0tMWUtMb0c4TzJNc0wzcGRHTkVxWnFlT21B?=
 =?utf-8?B?Sm5WOGxjcjdpMUVuYXBSOStHUm41VXArSXVsSkN6KzU2R0ZTSHVjQkVGd2xl?=
 =?utf-8?B?VENJbXVMNHk3Rm5mdHRYdVVmOVNuOTE0ZE9hTkQxTkcwSkNqNHc5UzhYNTBS?=
 =?utf-8?B?ZWpSazhIckNHa2RTWXl1bFRtbG1vMFNNc24zS2ZkcnI0NUl4elhiZFhEWEJn?=
 =?utf-8?B?M3BKcjlqYlVCaDB4aDdrVTgwbUNKRXVjN3pIcWhDVGdXenlSdmswQkFWblVS?=
 =?utf-8?B?RHM3VXhCRktRZ0Ztd1lUQ08zcU1BbGJSYytBRE01cFA1c2l0QkhzNUZ0cTJE?=
 =?utf-8?B?YkllZkhjWmtZMk9zdVAxVEVJRk1wa2Nac1NkVURiVUwzZzFEWXdYMHdBUlBz?=
 =?utf-8?B?V21mTGdkcTVXS2NDUUQ4OEFFTnZJY0IybmE0cDZUQ0VidEl3U3hwSGNqNnVH?=
 =?utf-8?B?dWpnNUdFazZGNExORFZDaFArdlVuSG1WY0xHejZPdlJYQjlZeURtTXE2YzNO?=
 =?utf-8?B?U3NDcEkySUY0WU9WM2wrWDgwTk1BMGhBcE5xSWtzTDYyaEprbFM1UHYrSWFZ?=
 =?utf-8?B?YUpyQTExa2NQTlRzbXIwUlQ1ZnBSd2F4ZWFDYXJ0clNRNzlZNUh5M0pyWHVB?=
 =?utf-8?B?djM2N0Q4SkZCWE5nMldvcEdpenROR3FyU1VHYmdVOThKclFrREFHaSt0ZERh?=
 =?utf-8?B?QlFMRkoxQVNINDZ4ODZOTFoyVDgwWkZON3dSQUo0dnRRZ2VFQzZmOHJpT2lp?=
 =?utf-8?B?WThBN25lL2V0c25qSUF4cWU2YUdCMFFubk9zOFEzUWpORTY4d1lKMTBqQStP?=
 =?utf-8?B?SlZqK1E5dXhneVNPWGJzNE4yLzVDQmVBN1hWNlRxSnBFTGxEVHZYS3Q2c3E4?=
 =?utf-8?B?OElDUFFKZ1RXalFXZDBUY3BIbWVmdW52SmxyMWNVTGZpOTRTQTd2RHROdTFU?=
 =?utf-8?B?YXd5eDlZSlBTSnFkNW16MlZrOFpiemZvSTNScW9HY0c1RGdVYmdXS3dMbVZL?=
 =?utf-8?B?THZabzRpR0NNbDAxTGUyK2ppbGdzN3V4UmF3RG5oZTJXcUhNZjMrcGpmdHYx?=
 =?utf-8?B?NUxaYXBkYjZuRzBRVE45VXlRQ0k3SEFveitzbEVkYVhxclNnM3RSazZYT044?=
 =?utf-8?B?dEVkbDZoMitFcXZYS3FPaHBRNCsxUVVoa3FBVHZHL291NStRd2xkeEU5NU52?=
 =?utf-8?B?Z0paNGt6a1FkQzlWNFV3UnZLRU05SjVmS3QreGZ1WlpZVGpua3lrdmlkOGM2?=
 =?utf-8?B?ZGNwYnd4Tk9DZzlIdEt2MTFyVFhZcy9jSEJlK0N6SzNadkRZVHlwMFFiVllR?=
 =?utf-8?B?YjAreDdFdzJ0Uk41aEF5ZlU0V0FUbmlXRXUyb2RwaTBsa3R1ZkphZHBiY25i?=
 =?utf-8?B?UEw2d0VmV2hTMFovRFg5c3dRWGYvY0NoazluQUpwSE9EbHdLRVZRUnovL2Ur?=
 =?utf-8?B?Q2xPbVdXTGxTRGFGNE1wZjluNlZucVYyTFZUSHBKNWh1Nm4vYWNQZXZPdGZ5?=
 =?utf-8?B?eGVkenRUcXFQejg0SEVjQzFrOWlXZjRmRHNuOWVKVDNLNVhreEZQNEpqQWJD?=
 =?utf-8?B?aUxkcUhmcU13VTluVnZqalRvb2l3bnpoVWZlb0o1ekRLV1JxZDNJZmJkTU9I?=
 =?utf-8?B?aW01eUcvNkxIeHhmSnpLNEVqUHZCMHphdkkzU25UNEsxL3pMajlEeGpmY0Jl?=
 =?utf-8?B?blE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e7c14fa2-1024-40d9-17dc-08db0f7c512a
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3768.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Feb 2023 17:44:41.7318
 (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: jofk9IfRPInN9mKYuJ7YtjYHOir/wrDZCHAiQICpc9lqRCanM5wmvdRRS8U36aDv/NVuAGwzO/kd1foAFMhBnA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB5888
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 2/15/2023 8:58 AM, Dave Hansen wrote:
> On 2/14/23 15:44, Jithu Joseph wrote:
...
> 
>> +static void ifs_array_test_core(int cpu, struct device *dev)
>> +{
>> +	union ifs_array activate, status = {0};
> 
> So, 'status' here is initialized to 0.  But, 'activate'... hmmm
> 
> Here's 1 of the 4 fields getting initialized:
> 
>> +	activate.array_bitmask = ~0U;
>> +	timeout = jiffies + HZ / 2;
>> +
>> +	do {
>> +		if (time_after(jiffies, timeout)) {
>> +			timed_out = true;
>> +			break;
>> +		}
>> +
>> +		msrvals[0] = activate.data;
> 
> and then the *WHOLE* union is read here.  What *is* the uninitialized
> member behavior of a bitfield?  I actually haven't the foggiest idea
> since I never use them.  Is there some subtly C voodoo that initializes
> the other 3 fields?

Thanks for pointing the mistake Dave. I see the bug w.r.t not initializing activate to
zero.

Thanks Dave for the proposed patch . Let me get back after taking a detailed look

Jithu