Re: [PATCH v2 4/7] platform/x86/intel/ifs: Implement Array BIST test

[Joseph, Jithu]

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