On 10/26/17 07:59, Eric Dong wrote: > For some special platforms (such as Ovmf), it is possible > that, some APs start up *and finish* before the remaining > APs not start up *at all*. In this case, the enhance
(1) I think the word "not" should be removed from the above line, so that we get: "some APs start up *and finish* before the remaining APs [] start up *at all*". (2) For the subject line: can we add "AP counting" or "AP collection" somehow? For example: UefiCpuPkg/MpInitLib: Keep compatible with former AP counting solution. I don't insist, but I think these changes would improve the commit message. They can be implemented before pushing. Either way, Reviewed-by: Laszlo Ersek <[email protected]> Thank you for the quick update, Eric and Jeff! Laszlo > solution by changes 0594ec41 not works as expected. > > This change remove check CpuMpData->CpuCount logic to let old > solution still workable if platform owner still set a long > time for PcdCpuApInitTimeOutInMicroSeconds. It's platform > owner's response to decide which solution to use. > > Cc: Ruiyu Ni <[email protected]> > Cc: Laszlo Ersek <[email protected]> > Cc: Jeff Fan <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong <[email protected]> > --- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c > b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index 48f930b..18060fd 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -936,15 +936,20 @@ WakeUpAP ( > } > if (CpuMpData->InitFlag == ApInitConfig) { > // > - // Wait for one potential AP waken up in one specified period > + // Here support two methods to collect AP count through adjust > + // PcdCpuApInitTimeOutInMicroSeconds values. > // > - if (CpuMpData->CpuCount == 0) { > - TimedWaitForApFinish ( > - CpuMpData, > - PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1, > - PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds) > - ); > - } > + // one way is set a value to just let the first AP to start the > + // initialization, then through the later while loop to wait all Aps > + // finsh the initialization. > + // The other way is set a value to let all APs finished the > initialzation. > + // In this case, the later while loop is useless. > + // > + TimedWaitForApFinish ( > + CpuMpData, > + PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1, > + PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds) > + ); > > while (CpuMpData->MpCpuExchangeInfo->NumApsExecuting != 0) { > CpuPause(); > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

