[Skiboot] [PATCH] fix lock error when BT IRQ preempt BT timer

[Vasant Hegde]

On 1/11/21 3:26 PM, DD Aric li wrote:
> Hi Vasant：
> I wonder if every core will buffer infight_bt_msg in it own cache.
> If that true, can the lock ensure that the value of "infight_bt_msg" read
> by every thread is the latest?

I think lock will take care of it. We don't need volatile.

-Vasant


> 
> Vasant Hegde <hegdevasant at linux.vnet.ibm.com> 于2021年1月11日周一 下午2:12写道：
> 
>> On 1/11/21 8:39 AM, DD Aric li wrote:
>>> Vasant Hegde <hegdevasant at linux.vnet.ibm.com> 于2021年1月7日周四 下午7:36写道：
>>>
>>>> On 1/6/21 2:03 PM, lixg wrote:
>>>>> BT IRQ may preempt BT timer if BMC response host when bt msg timeout.
>>>>> When BT IRQ preempt BT timer, the infight_bt_msg did not protected by
>>>> bt.lock very well.
>>>>>
>>>>> And we will see the following log:
>>>>> [29006114.163785853,3] BT: seq 0x81 netfn 0x0a cmd 0x23: Timeout
>> sending
>>>> message
>>>>> [29006114.288029290,3] BT: seq 0x81 netfn 0x0b cmd 0x23: Timeout
>> sending
>>>> message
>>>>> [29006114.288917798,3] IPMI: Incorrect netfn 0x0b in response
>>>>>
>>>>> It may cause 'CPU Hardlock UP', 'memory refree', 'kernel crash' or
>>>> something else...
>>>>>
>>>>> Signed-off-by: lixg <867314078 at qq.com>
>>>>> ---
>>>>>     hw/bt.c | 9 +++++++--
>>>>>     1 file changed, 7 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/hw/bt.c b/hw/bt.c
>>>>> index cf967f89..24e6ef7f 100644
>>>>> --- a/hw/bt.c
>>>>> +++ b/hw/bt.c
>>>>> @@ -111,7 +111,7 @@ struct bt {
>>>>>     };
>>>>>
>>>>>     static struct bt bt;
>>>>> -static struct bt_msg *inflight_bt_msg; /* Holds in flight message */
>>>>> +static struct bt_msg * volatile inflight_bt_msg; /* Holds in flight
>>>> message */
>>>>
>>>> Why do we need volatile here?
>>>>
>>>
>>> There will be multiple thread write " inflight_bt_msg ", so we need to
>>> ensure that it is always the latest value.
>>
>> This is protected by locks. So we are good right?
>>
>> -Vasant
>>
>