[U-Boot] driver model is not smp safe

[Simon Glass]

Hi,

On 31 July 2015 at 07:42, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Andrew,
>
> On Fri, Jul 31, 2015 at 8:30 PM, Andrew Bradford
> <andrew at bradfordembedded.com> wrote:
>> Hi Bin,
>>
>> On 07/30 12:12, Bin Meng wrote:
>>> Hi Simon,
>>>
>>> When adding x86 multi-cpu initialization on a board with 4 cores, I found:
>>>
>>> => cpu list
>>>   0: cpu at 0               Genuine Intel(R) CPU         @ 1.58GHz
>>>   1: cpu at 1               Genuine Intel(R) CPU         @ 1.58GHz
>>>   2: cpu at 2               Genuine Intel(R) CPU         @ 1.58GHz
>>>   2: cpu at 3               Genuine Intel(R) CPU         @ 1.58GHz
>>>
>>> cpu at 2 and cpu at 3 have the same sequence number, which indicates they
>>> are running parallelly to get the same sequence number. The call chain
>>> on an ap is: mp_init_cpu() -> device_probe() -> uclass_resolve_seq().
>>> Apparently ap2 and ap3 are running at the same time to get the same
>>> number.
>>>
>>> Note so far all x86 boards that we have enabled x86 multi-cpu
>>> initialization on only have 2 cores, which will not expose such issue
>>> as there is no parallel execution among aps.
>>>
>>> What does this mean?
>>>
>>> -  Driver model is not smp safe. But given U-Boot is a single-threaded
>>> environment, I don't think we want to add such support to driver
>>> model.
>>>
>>> OR:
>>>
>>> - We are using driver model incorrectly on x86 mp initialization codes.
>>>
>>> What do you think?
>>
>> I'm not sure what to do about this (if anything) but I also see this on
>> an E3845 based board.  I don't think it has affected me in any way.
>>
>
> So far I only see the seq number is not correct. Booting Linux with MP
> table seems to be OK as Linux can start all 4 cores correctly.
>
>> Does this also affect non-x86 processors?
>>
>
> I believe currently the cpu uclass is only implemented on x86, so only
> x86 is affected for now.
>
>> => cpu list
>>   0: cpu at 0              Intel(R) Atom(TM) CPU  E3845  @ 1.91GHz
>>   2: cpu at 1              Intel(R) Atom(TM) CPU  E3845  @ 1.91GHz
>>   2: cpu at 2              Intel(R) Atom(TM) CPU  E3845  @ 1.91GHz
>>   1: cpu at 3              Intel(R) Atom(TM) CPU  E3845  @ 1.91GHz
>>

My intention with this was that the sequence numbering would be fixed
in the device tree and each CPU would discover its number in
find_cpu_by_apic_id(). However this only works if you add aliases for
each CPU.

In general I don't think we should be making driver model thread-safe.
But we could add code to sipi_vector.S or even ap_init() to avoid this
problem.

One option which should work is to add something like this to mp_init_cpu():

dev->req_seq = fdtdec_get_init(blob, dev->of_offset, "reg", -1);

Regards,
Simon