Re: [PATCH] UefiCpuPkg/MpInitLib: AP uses memory preceding IDT to store CpuMpData

["Ni, Ruiyu" <ruiyu.ni@Intel.com>]

On 6/27/2018 8:00 PM, Laszlo Ersek wrote:
> Roughly, I figured that we could set the pre-IDT pointer on the BSP,
> *before*  starting up the APs, to a mapping table. The mapping table
> would consist of two columns, the first containing the APIC ID (of each
> CPU), the second containing AP-specific pointer values. Then, after
> starting up the APs, each AP could locate its own row (based on APIC ID)
> in the table, and the context pointer that belongs to it.
> 
> But, this is likely not useful, as we want to expose just the same one
> pointer value to all APs together.
> 
>>> - or else, we should stick with the current idea, but use atomic
>>>     compare-and-swap operations, so that the original PEI services pointer
>>>     value be restored ultimately, at the least,
>> I like this idea. Will generate the V2 patch.

I finally give up on this direction.
The basic idea of this approach is there is a master AP and many other 
slave APs. The master AP is responsible to save CpuMpData to PeiServices 
pointer location, the restores PeiServices pointer back after all slave 
APs finish.
When slave AP doesn't hit timeout when executing AP procedure, 
everything should be good as long as we take good care of the 
synchronous stuff.
But when one slave AP hits timeout, the master AP is treated as hitting 
timeout as well from BSP's perspective.
So BSP resets the two APs (including master AP) which leads to no one 
restores back the PeiServices pointer.
It's true that in such case BSP can do the restore job. But that causes 
the code very unreadable.

>>
>>> - or else (possibly the simplest fix), allocate a separate IDT for each
>>>     AP, including the UINTN that precedes the (now AP-specific) IDT. This
>>>     means that the PEI services pointer*location*  would be separate for
>>>     each AP, and no contention would occur.
>> I think it's the most complicated fix:)
> It might take the most code, but I guess it would be the simplest to
> reason about. No data sharing --> no locking necessary, and no races
> possible.

I will go in this direction. Yes it may introduce more code but much 
more readable than the above solution. Easy to maintain.

> 
> Anyway, I saw your v2 (just the subject, and your note that we should
> ignore v2 for now). I'll stay tuned for v3.
> 
> Meta-question: some of your emails are apparently addressed to the list
> only, and not CC'd to people who commented earlier on the thread. Did
> you drop CCs deliberately, or is it some kind of mail agent glitch?

I am using Outlook for internal communication, ThunderBird for this 
mailing list.
Sometimes I may click [Reply List] button in that mail client. It will 
only send the mail to the mailing list without CC.
I agree [Reply All] is better:)

-- 
Thanks,
Ray