* [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if only one processor supported @ 2016-11-04 8:18 Jeff Fan 2016-11-04 16:40 ` Laszlo Ersek 2016-11-09 7:21 ` Tian, Feng 0 siblings, 2 replies; 7+ messages in thread From: Jeff Fan @ 2016-11-04 8:18 UTC (permalink / raw) To: edk2-devel; +Cc: Feng Tian, Liming Gao, Michael Kinney If MaxLogicalProcessorNumber is only 1, we needn't to wake up APs at all and needn't to register callback functions. It could improve boot performance on single supported system. https://bugzilla.tianocore.org/show_bug.cgi?id=204 Cc: Feng Tian <feng.tian@intel.com> Cc: Liming Gao <liming.gao@intel.com> Cc: Michael Kinney <michael.d.kinney@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jeff Fan <jeff.fan@intel.com> --- UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 7 ++++++ UefiCpuPkg/Library/MpInitLib/MpLib.c | 39 +++++++++++++++++++-------------- UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 8 +++++++ 3 files changed, 37 insertions(+), 17 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c index b399f1c..eb36d6f 100644 --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c @@ -290,6 +290,13 @@ InitMpGlobalData ( SaveCpuMpData (CpuMpData); + if (CpuMpData->CpuCount == 1) { + // + // If only BSP exists, return + // + return; + } + // // Avoid APs access invalid buff data which allocated by BootServices, // so we will allocate reserved data for AP loop code. diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c index 56b870e..a0edc55 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -1143,6 +1143,7 @@ MpInitLibInitialize ( } else { MaxLogicalProcessorNumber = OldCpuMpData->CpuCount; } + ASSERT (MaxLogicalProcessorNumber != 0); AsmGetAddressMap (&AddressMap); ApResetVectorSize = AddressMap.RendezvousFunnelSize + sizeof (MP_CPU_EXCHANGE_INFO); @@ -1209,10 +1210,12 @@ MpInitLibInitialize ( MtrrGetAllMtrrs (&CpuMpData->MtrrTable); if (OldCpuMpData == NULL) { - // - // Wakeup all APs and calculate the processor count in system - // - CollectProcessorCount (CpuMpData); + if (MaxLogicalProcessorNumber > 1) { + // + // Wakeup all APs and calculate the processor count in system + // + CollectProcessorCount (CpuMpData); + } } else { // // APs have been wakeup before, just get the CPU Information @@ -1238,19 +1241,21 @@ MpInitLibInitialize ( sizeof (CPU_VOLATILE_REGISTERS) ); } - // - // Wakeup APs to do some AP initialize sync - // - WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData); - // - // Wait for all APs finished initialization - // - while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) { - CpuPause (); - } - CpuMpData->InitFlag = ApInitDone; - for (Index = 0; Index < CpuMpData->CpuCount; Index++) { - SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); + if (MaxLogicalProcessorNumber > 1) { + // + // Wakeup APs to do some AP initialize sync + // + WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData); + // + // Wait for all APs finished initialization + // + while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) { + CpuPause (); + } + CpuMpData->InitFlag = ApInitDone; + for (Index = 0; Index < CpuMpData->CpuCount; Index++) { + SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); + } } } diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c index e242d37..1f2fcb8 100644 --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c @@ -321,6 +321,14 @@ InitMpGlobalData ( EFI_STATUS Status; SaveCpuMpData (CpuMpData); + + if (CpuMpData->CpuCount == 1) { + // + // If only BSP exists, return + // + return; + } + // // Register an event for EndOfPei // -- 2.9.3.windows.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if only one processor supported 2016-11-04 8:18 [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if only one processor supported Jeff Fan @ 2016-11-04 16:40 ` Laszlo Ersek 2016-11-07 0:47 ` Fan, Jeff 2016-11-09 7:21 ` Tian, Feng 1 sibling, 1 reply; 7+ messages in thread From: Laszlo Ersek @ 2016-11-04 16:40 UTC (permalink / raw) To: Jeff Fan, edk2-devel; +Cc: Michael Kinney, Feng Tian, Liming Gao On 11/04/16 09:18, Jeff Fan wrote: > If MaxLogicalProcessorNumber is only 1, we needn't to wake up APs at all > and needn't to register callback functions. > > It could improve boot performance on single supported system. > > https://bugzilla.tianocore.org/show_bug.cgi?id=204 > > Cc: Feng Tian <feng.tian@intel.com> > Cc: Liming Gao <liming.gao@intel.com> > Cc: Michael Kinney <michael.d.kinney@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jeff Fan <jeff.fan@intel.com> > --- > UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 7 ++++++ > UefiCpuPkg/Library/MpInitLib/MpLib.c | 39 +++++++++++++++++++-------------- > UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 8 +++++++ > 3 files changed, 37 insertions(+), 17 deletions(-) The MP services protocol and PPI that depend on this library will remain available to callers, right? Thanks Laszlo > diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > index b399f1c..eb36d6f 100644 > --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > @@ -290,6 +290,13 @@ InitMpGlobalData ( > > SaveCpuMpData (CpuMpData); > > + if (CpuMpData->CpuCount == 1) { > + // > + // If only BSP exists, return > + // > + return; > + } > + > // > // Avoid APs access invalid buff data which allocated by BootServices, > // so we will allocate reserved data for AP loop code. > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index 56b870e..a0edc55 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -1143,6 +1143,7 @@ MpInitLibInitialize ( > } else { > MaxLogicalProcessorNumber = OldCpuMpData->CpuCount; > } > + ASSERT (MaxLogicalProcessorNumber != 0); > > AsmGetAddressMap (&AddressMap); > ApResetVectorSize = AddressMap.RendezvousFunnelSize + sizeof (MP_CPU_EXCHANGE_INFO); > @@ -1209,10 +1210,12 @@ MpInitLibInitialize ( > MtrrGetAllMtrrs (&CpuMpData->MtrrTable); > > if (OldCpuMpData == NULL) { > - // > - // Wakeup all APs and calculate the processor count in system > - // > - CollectProcessorCount (CpuMpData); > + if (MaxLogicalProcessorNumber > 1) { > + // > + // Wakeup all APs and calculate the processor count in system > + // > + CollectProcessorCount (CpuMpData); > + } > } else { > // > // APs have been wakeup before, just get the CPU Information > @@ -1238,19 +1241,21 @@ MpInitLibInitialize ( > sizeof (CPU_VOLATILE_REGISTERS) > ); > } > - // > - // Wakeup APs to do some AP initialize sync > - // > - WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData); > - // > - // Wait for all APs finished initialization > - // > - while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) { > - CpuPause (); > - } > - CpuMpData->InitFlag = ApInitDone; > - for (Index = 0; Index < CpuMpData->CpuCount; Index++) { > - SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); > + if (MaxLogicalProcessorNumber > 1) { > + // > + // Wakeup APs to do some AP initialize sync > + // > + WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData); > + // > + // Wait for all APs finished initialization > + // > + while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) { > + CpuPause (); > + } > + CpuMpData->InitFlag = ApInitDone; > + for (Index = 0; Index < CpuMpData->CpuCount; Index++) { > + SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); > + } > } > } > > diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c > index e242d37..1f2fcb8 100644 > --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c > @@ -321,6 +321,14 @@ InitMpGlobalData ( > EFI_STATUS Status; > > SaveCpuMpData (CpuMpData); > + > + if (CpuMpData->CpuCount == 1) { > + // > + // If only BSP exists, return > + // > + return; > + } > + > // > // Register an event for EndOfPei > // > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if only one processor supported 2016-11-04 16:40 ` Laszlo Ersek @ 2016-11-07 0:47 ` Fan, Jeff 2016-11-07 13:37 ` Laszlo Ersek 0 siblings, 1 reply; 7+ messages in thread From: Fan, Jeff @ 2016-11-07 0:47 UTC (permalink / raw) To: Laszlo Ersek, edk2-devel@ml01.01.org Cc: Kinney, Michael D, Tian, Feng, Gao, Liming Laszlo, Yes. MP PPI and Protocol still be installed even there is only one processor supported or found? Jeff -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek Sent: Saturday, November 05, 2016 12:40 AM To: Fan, Jeff; edk2-devel@ml01.01.org Cc: Kinney, Michael D; Tian, Feng; Gao, Liming Subject: Re: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if only one processor supported On 11/04/16 09:18, Jeff Fan wrote: > If MaxLogicalProcessorNumber is only 1, we needn't to wake up APs at > all and needn't to register callback functions. > > It could improve boot performance on single supported system. > > https://bugzilla.tianocore.org/show_bug.cgi?id=204 > > Cc: Feng Tian <feng.tian@intel.com> > Cc: Liming Gao <liming.gao@intel.com> > Cc: Michael Kinney <michael.d.kinney@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jeff Fan <jeff.fan@intel.com> > --- > UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 7 ++++++ > UefiCpuPkg/Library/MpInitLib/MpLib.c | 39 +++++++++++++++++++-------------- > UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 8 +++++++ > 3 files changed, 37 insertions(+), 17 deletions(-) The MP services protocol and PPI that depend on this library will remain available to callers, right? Thanks Laszlo > diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > index b399f1c..eb36d6f 100644 > --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > @@ -290,6 +290,13 @@ InitMpGlobalData ( > > SaveCpuMpData (CpuMpData); > > + if (CpuMpData->CpuCount == 1) { > + // > + // If only BSP exists, return > + // > + return; > + } > + > // > // Avoid APs access invalid buff data which allocated by BootServices, > // so we will allocate reserved data for AP loop code. > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c > b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index 56b870e..a0edc55 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -1143,6 +1143,7 @@ MpInitLibInitialize ( > } else { > MaxLogicalProcessorNumber = OldCpuMpData->CpuCount; > } > + ASSERT (MaxLogicalProcessorNumber != 0); > > AsmGetAddressMap (&AddressMap); > ApResetVectorSize = AddressMap.RendezvousFunnelSize + sizeof > (MP_CPU_EXCHANGE_INFO); @@ -1209,10 +1210,12 @@ MpInitLibInitialize ( > MtrrGetAllMtrrs (&CpuMpData->MtrrTable); > > if (OldCpuMpData == NULL) { > - // > - // Wakeup all APs and calculate the processor count in system > - // > - CollectProcessorCount (CpuMpData); > + if (MaxLogicalProcessorNumber > 1) { > + // > + // Wakeup all APs and calculate the processor count in system > + // > + CollectProcessorCount (CpuMpData); > + } > } else { > // > // APs have been wakeup before, just get the CPU Information @@ > -1238,19 +1241,21 @@ MpInitLibInitialize ( > sizeof (CPU_VOLATILE_REGISTERS) > ); > } > - // > - // Wakeup APs to do some AP initialize sync > - // > - WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData); > - // > - // Wait for all APs finished initialization > - // > - while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) { > - CpuPause (); > - } > - CpuMpData->InitFlag = ApInitDone; > - for (Index = 0; Index < CpuMpData->CpuCount; Index++) { > - SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); > + if (MaxLogicalProcessorNumber > 1) { > + // > + // Wakeup APs to do some AP initialize sync > + // > + WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData); > + // > + // Wait for all APs finished initialization > + // > + while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) { > + CpuPause (); > + } > + CpuMpData->InitFlag = ApInitDone; > + for (Index = 0; Index < CpuMpData->CpuCount; Index++) { > + SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); > + } > } > } > > diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c > b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c > index e242d37..1f2fcb8 100644 > --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c > @@ -321,6 +321,14 @@ InitMpGlobalData ( > EFI_STATUS Status; > > SaveCpuMpData (CpuMpData); > + > + if (CpuMpData->CpuCount == 1) { > + // > + // If only BSP exists, return > + // > + return; > + } > + > // > // Register an event for EndOfPei > // > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if only one processor supported 2016-11-07 0:47 ` Fan, Jeff @ 2016-11-07 13:37 ` Laszlo Ersek 2016-11-08 0:55 ` Fan, Jeff 0 siblings, 1 reply; 7+ messages in thread From: Laszlo Ersek @ 2016-11-07 13:37 UTC (permalink / raw) To: Fan, Jeff, edk2-devel@ml01.01.org Cc: Kinney, Michael D, Tian, Feng, Gao, Liming On 11/07/16 01:47, Fan, Jeff wrote: > Laszlo, > > Yes. MP PPI and Protocol still be installed even there is only one processor supported or found? I'm sorry -- are you confirming that the PPI and the protocol will exist, or are you asking if I would like them to exist? (I'm confused by the question mark at the end of your second sentence.) Thanks! Laszlo > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek > Sent: Saturday, November 05, 2016 12:40 AM > To: Fan, Jeff; edk2-devel@ml01.01.org > Cc: Kinney, Michael D; Tian, Feng; Gao, Liming > Subject: Re: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if only one processor supported > > On 11/04/16 09:18, Jeff Fan wrote: >> If MaxLogicalProcessorNumber is only 1, we needn't to wake up APs at >> all and needn't to register callback functions. >> >> It could improve boot performance on single supported system. >> >> https://bugzilla.tianocore.org/show_bug.cgi?id=204 >> >> Cc: Feng Tian <feng.tian@intel.com> >> Cc: Liming Gao <liming.gao@intel.com> >> Cc: Michael Kinney <michael.d.kinney@intel.com> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Jeff Fan <jeff.fan@intel.com> >> --- >> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 7 ++++++ >> UefiCpuPkg/Library/MpInitLib/MpLib.c | 39 +++++++++++++++++++-------------- >> UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 8 +++++++ >> 3 files changed, 37 insertions(+), 17 deletions(-) > > The MP services protocol and PPI that depend on this library will remain available to callers, right? > > Thanks > Laszlo > >> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >> b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >> index b399f1c..eb36d6f 100644 >> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >> @@ -290,6 +290,13 @@ InitMpGlobalData ( >> >> SaveCpuMpData (CpuMpData); >> >> + if (CpuMpData->CpuCount == 1) { >> + // >> + // If only BSP exists, return >> + // >> + return; >> + } >> + >> // >> // Avoid APs access invalid buff data which allocated by BootServices, >> // so we will allocate reserved data for AP loop code. >> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c >> b/UefiCpuPkg/Library/MpInitLib/MpLib.c >> index 56b870e..a0edc55 100644 >> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c >> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c >> @@ -1143,6 +1143,7 @@ MpInitLibInitialize ( >> } else { >> MaxLogicalProcessorNumber = OldCpuMpData->CpuCount; >> } >> + ASSERT (MaxLogicalProcessorNumber != 0); >> >> AsmGetAddressMap (&AddressMap); >> ApResetVectorSize = AddressMap.RendezvousFunnelSize + sizeof >> (MP_CPU_EXCHANGE_INFO); @@ -1209,10 +1210,12 @@ MpInitLibInitialize ( >> MtrrGetAllMtrrs (&CpuMpData->MtrrTable); >> >> if (OldCpuMpData == NULL) { >> - // >> - // Wakeup all APs and calculate the processor count in system >> - // >> - CollectProcessorCount (CpuMpData); >> + if (MaxLogicalProcessorNumber > 1) { >> + // >> + // Wakeup all APs and calculate the processor count in system >> + // >> + CollectProcessorCount (CpuMpData); >> + } >> } else { >> // >> // APs have been wakeup before, just get the CPU Information @@ >> -1238,19 +1241,21 @@ MpInitLibInitialize ( >> sizeof (CPU_VOLATILE_REGISTERS) >> ); >> } >> - // >> - // Wakeup APs to do some AP initialize sync >> - // >> - WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData); >> - // >> - // Wait for all APs finished initialization >> - // >> - while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) { >> - CpuPause (); >> - } >> - CpuMpData->InitFlag = ApInitDone; >> - for (Index = 0; Index < CpuMpData->CpuCount; Index++) { >> - SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); >> + if (MaxLogicalProcessorNumber > 1) { >> + // >> + // Wakeup APs to do some AP initialize sync >> + // >> + WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData); >> + // >> + // Wait for all APs finished initialization >> + // >> + while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) { >> + CpuPause (); >> + } >> + CpuMpData->InitFlag = ApInitDone; >> + for (Index = 0; Index < CpuMpData->CpuCount; Index++) { >> + SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); >> + } >> } >> } >> >> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >> b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >> index e242d37..1f2fcb8 100644 >> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >> @@ -321,6 +321,14 @@ InitMpGlobalData ( >> EFI_STATUS Status; >> >> SaveCpuMpData (CpuMpData); >> + >> + if (CpuMpData->CpuCount == 1) { >> + // >> + // If only BSP exists, return >> + // >> + return; >> + } >> + >> // >> // Register an event for EndOfPei >> // >> > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if only one processor supported 2016-11-07 13:37 ` Laszlo Ersek @ 2016-11-08 0:55 ` Fan, Jeff 2016-11-08 13:37 ` Laszlo Ersek 0 siblings, 1 reply; 7+ messages in thread From: Fan, Jeff @ 2016-11-08 0:55 UTC (permalink / raw) To: Laszlo Ersek, edk2-devel@ml01.01.org Cc: Kinney, Michael D, Tian, Feng, Gao, Liming Laszlo, Oh. Sorry for the question mark in my last sentence and confusing you. :-) it should be point. It's my typo. MP PPI and the protocol DO exist even there is only one processor in the system. Thanks! Jeff -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek Sent: Monday, November 07, 2016 9:37 PM To: Fan, Jeff; edk2-devel@ml01.01.org Cc: Kinney, Michael D; Tian, Feng; Gao, Liming Subject: Re: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if only one processor supported On 11/07/16 01:47, Fan, Jeff wrote: > Laszlo, > > Yes. MP PPI and Protocol still be installed even there is only one processor supported or found? I'm sorry -- are you confirming that the PPI and the protocol will exist, or are you asking if I would like them to exist? (I'm confused by the question mark at the end of your second sentence.) Thanks! Laszlo > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Laszlo Ersek > Sent: Saturday, November 05, 2016 12:40 AM > To: Fan, Jeff; edk2-devel@ml01.01.org > Cc: Kinney, Michael D; Tian, Feng; Gao, Liming > Subject: Re: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if > only one processor supported > > On 11/04/16 09:18, Jeff Fan wrote: >> If MaxLogicalProcessorNumber is only 1, we needn't to wake up APs at >> all and needn't to register callback functions. >> >> It could improve boot performance on single supported system. >> >> https://bugzilla.tianocore.org/show_bug.cgi?id=204 >> >> Cc: Feng Tian <feng.tian@intel.com> >> Cc: Liming Gao <liming.gao@intel.com> >> Cc: Michael Kinney <michael.d.kinney@intel.com> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Jeff Fan <jeff.fan@intel.com> >> --- >> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 7 ++++++ >> UefiCpuPkg/Library/MpInitLib/MpLib.c | 39 +++++++++++++++++++-------------- >> UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 8 +++++++ >> 3 files changed, 37 insertions(+), 17 deletions(-) > > The MP services protocol and PPI that depend on this library will remain available to callers, right? > > Thanks > Laszlo > >> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >> b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >> index b399f1c..eb36d6f 100644 >> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >> @@ -290,6 +290,13 @@ InitMpGlobalData ( >> >> SaveCpuMpData (CpuMpData); >> >> + if (CpuMpData->CpuCount == 1) { >> + // >> + // If only BSP exists, return >> + // >> + return; >> + } >> + >> // >> // Avoid APs access invalid buff data which allocated by BootServices, >> // so we will allocate reserved data for AP loop code. >> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c >> b/UefiCpuPkg/Library/MpInitLib/MpLib.c >> index 56b870e..a0edc55 100644 >> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c >> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c >> @@ -1143,6 +1143,7 @@ MpInitLibInitialize ( >> } else { >> MaxLogicalProcessorNumber = OldCpuMpData->CpuCount; >> } >> + ASSERT (MaxLogicalProcessorNumber != 0); >> >> AsmGetAddressMap (&AddressMap); >> ApResetVectorSize = AddressMap.RendezvousFunnelSize + sizeof >> (MP_CPU_EXCHANGE_INFO); @@ -1209,10 +1210,12 @@ MpInitLibInitialize ( >> MtrrGetAllMtrrs (&CpuMpData->MtrrTable); >> >> if (OldCpuMpData == NULL) { >> - // >> - // Wakeup all APs and calculate the processor count in system >> - // >> - CollectProcessorCount (CpuMpData); >> + if (MaxLogicalProcessorNumber > 1) { >> + // >> + // Wakeup all APs and calculate the processor count in system >> + // >> + CollectProcessorCount (CpuMpData); >> + } >> } else { >> // >> // APs have been wakeup before, just get the CPU Information @@ >> -1238,19 +1241,21 @@ MpInitLibInitialize ( >> sizeof (CPU_VOLATILE_REGISTERS) >> ); >> } >> - // >> - // Wakeup APs to do some AP initialize sync >> - // >> - WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData); >> - // >> - // Wait for all APs finished initialization >> - // >> - while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) { >> - CpuPause (); >> - } >> - CpuMpData->InitFlag = ApInitDone; >> - for (Index = 0; Index < CpuMpData->CpuCount; Index++) { >> - SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); >> + if (MaxLogicalProcessorNumber > 1) { >> + // >> + // Wakeup APs to do some AP initialize sync >> + // >> + WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData); >> + // >> + // Wait for all APs finished initialization >> + // >> + while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) { >> + CpuPause (); >> + } >> + CpuMpData->InitFlag = ApInitDone; >> + for (Index = 0; Index < CpuMpData->CpuCount; Index++) { >> + SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); >> + } >> } >> } >> >> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >> b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >> index e242d37..1f2fcb8 100644 >> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >> @@ -321,6 +321,14 @@ InitMpGlobalData ( >> EFI_STATUS Status; >> >> SaveCpuMpData (CpuMpData); >> + >> + if (CpuMpData->CpuCount == 1) { >> + // >> + // If only BSP exists, return >> + // >> + return; >> + } >> + >> // >> // Register an event for EndOfPei >> // >> > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if only one processor supported 2016-11-08 0:55 ` Fan, Jeff @ 2016-11-08 13:37 ` Laszlo Ersek 0 siblings, 0 replies; 7+ messages in thread From: Laszlo Ersek @ 2016-11-08 13:37 UTC (permalink / raw) To: Fan, Jeff, edk2-devel@ml01.01.org Cc: Kinney, Michael D, Tian, Feng, Gao, Liming On 11/08/16 01:55, Fan, Jeff wrote: > Laszlo, > > Oh. Sorry for the question mark in my last sentence and confusing you. :-) it should be point. It's my typo. > > MP PPI and the protocol DO exist even there is only one processor in the system. > > Thanks! > Jeff Thanks. With that guarantee, the proposed change looks sensible. I didn't review the patch, but I want to confirm that (with the above guarantee) I agree with its goal: Acked-by: Laszlo Ersek <lersek@redhat.com> Thanks Laszlo > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek > Sent: Monday, November 07, 2016 9:37 PM > To: Fan, Jeff; edk2-devel@ml01.01.org > Cc: Kinney, Michael D; Tian, Feng; Gao, Liming > Subject: Re: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if only one processor supported > > On 11/07/16 01:47, Fan, Jeff wrote: >> Laszlo, >> >> Yes. MP PPI and Protocol still be installed even there is only one processor supported or found? > > I'm sorry -- are you confirming that the PPI and the protocol will exist, or are you asking if I would like them to exist? > > (I'm confused by the question mark at the end of your second sentence.) > > Thanks! > Laszlo > >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of >> Laszlo Ersek >> Sent: Saturday, November 05, 2016 12:40 AM >> To: Fan, Jeff; edk2-devel@ml01.01.org >> Cc: Kinney, Michael D; Tian, Feng; Gao, Liming >> Subject: Re: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if >> only one processor supported >> >> On 11/04/16 09:18, Jeff Fan wrote: >>> If MaxLogicalProcessorNumber is only 1, we needn't to wake up APs at >>> all and needn't to register callback functions. >>> >>> It could improve boot performance on single supported system. >>> >>> https://bugzilla.tianocore.org/show_bug.cgi?id=204 >>> >>> Cc: Feng Tian <feng.tian@intel.com> >>> Cc: Liming Gao <liming.gao@intel.com> >>> Cc: Michael Kinney <michael.d.kinney@intel.com> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Jeff Fan <jeff.fan@intel.com> >>> --- >>> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 7 ++++++ >>> UefiCpuPkg/Library/MpInitLib/MpLib.c | 39 +++++++++++++++++++-------------- >>> UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 8 +++++++ >>> 3 files changed, 37 insertions(+), 17 deletions(-) >> >> The MP services protocol and PPI that depend on this library will remain available to callers, right? >> >> Thanks >> Laszlo >> >>> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >>> b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >>> index b399f1c..eb36d6f 100644 >>> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >>> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >>> @@ -290,6 +290,13 @@ InitMpGlobalData ( >>> >>> SaveCpuMpData (CpuMpData); >>> >>> + if (CpuMpData->CpuCount == 1) { >>> + // >>> + // If only BSP exists, return >>> + // >>> + return; >>> + } >>> + >>> // >>> // Avoid APs access invalid buff data which allocated by BootServices, >>> // so we will allocate reserved data for AP loop code. >>> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c >>> b/UefiCpuPkg/Library/MpInitLib/MpLib.c >>> index 56b870e..a0edc55 100644 >>> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c >>> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c >>> @@ -1143,6 +1143,7 @@ MpInitLibInitialize ( >>> } else { >>> MaxLogicalProcessorNumber = OldCpuMpData->CpuCount; >>> } >>> + ASSERT (MaxLogicalProcessorNumber != 0); >>> >>> AsmGetAddressMap (&AddressMap); >>> ApResetVectorSize = AddressMap.RendezvousFunnelSize + sizeof >>> (MP_CPU_EXCHANGE_INFO); @@ -1209,10 +1210,12 @@ MpInitLibInitialize ( >>> MtrrGetAllMtrrs (&CpuMpData->MtrrTable); >>> >>> if (OldCpuMpData == NULL) { >>> - // >>> - // Wakeup all APs and calculate the processor count in system >>> - // >>> - CollectProcessorCount (CpuMpData); >>> + if (MaxLogicalProcessorNumber > 1) { >>> + // >>> + // Wakeup all APs and calculate the processor count in system >>> + // >>> + CollectProcessorCount (CpuMpData); >>> + } >>> } else { >>> // >>> // APs have been wakeup before, just get the CPU Information @@ >>> -1238,19 +1241,21 @@ MpInitLibInitialize ( >>> sizeof (CPU_VOLATILE_REGISTERS) >>> ); >>> } >>> - // >>> - // Wakeup APs to do some AP initialize sync >>> - // >>> - WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData); >>> - // >>> - // Wait for all APs finished initialization >>> - // >>> - while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) { >>> - CpuPause (); >>> - } >>> - CpuMpData->InitFlag = ApInitDone; >>> - for (Index = 0; Index < CpuMpData->CpuCount; Index++) { >>> - SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); >>> + if (MaxLogicalProcessorNumber > 1) { >>> + // >>> + // Wakeup APs to do some AP initialize sync >>> + // >>> + WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData); >>> + // >>> + // Wait for all APs finished initialization >>> + // >>> + while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) { >>> + CpuPause (); >>> + } >>> + CpuMpData->InitFlag = ApInitDone; >>> + for (Index = 0; Index < CpuMpData->CpuCount; Index++) { >>> + SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); >>> + } >>> } >>> } >>> >>> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >>> b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >>> index e242d37..1f2fcb8 100644 >>> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >>> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >>> @@ -321,6 +321,14 @@ InitMpGlobalData ( >>> EFI_STATUS Status; >>> >>> SaveCpuMpData (CpuMpData); >>> + >>> + if (CpuMpData->CpuCount == 1) { >>> + // >>> + // If only BSP exists, return >>> + // >>> + return; >>> + } >>> + >>> // >>> // Register an event for EndOfPei >>> // >>> >> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel >> > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if only one processor supported 2016-11-04 8:18 [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if only one processor supported Jeff Fan 2016-11-04 16:40 ` Laszlo Ersek @ 2016-11-09 7:21 ` Tian, Feng 1 sibling, 0 replies; 7+ messages in thread From: Tian, Feng @ 2016-11-09 7:21 UTC (permalink / raw) To: Fan, Jeff, edk2-devel@ml01.01.org Cc: Gao, Liming, Kinney, Michael D, Tian, Feng Reviewed-by: Feng Tian <feng.tian@Intel.com> Thanks Feng -----Original Message----- From: Fan, Jeff Sent: Friday, November 4, 2016 4:18 PM To: edk2-devel@ml01.01.org Cc: Tian, Feng <feng.tian@intel.com>; Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com> Subject: [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if only one processor supported If MaxLogicalProcessorNumber is only 1, we needn't to wake up APs at all and needn't to register callback functions. It could improve boot performance on single supported system. https://bugzilla.tianocore.org/show_bug.cgi?id=204 Cc: Feng Tian <feng.tian@intel.com> Cc: Liming Gao <liming.gao@intel.com> Cc: Michael Kinney <michael.d.kinney@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jeff Fan <jeff.fan@intel.com> --- UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 7 ++++++ UefiCpuPkg/Library/MpInitLib/MpLib.c | 39 +++++++++++++++++++-------------- UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 8 +++++++ 3 files changed, 37 insertions(+), 17 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c index b399f1c..eb36d6f 100644 --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c @@ -290,6 +290,13 @@ InitMpGlobalData ( SaveCpuMpData (CpuMpData); + if (CpuMpData->CpuCount == 1) { + // + // If only BSP exists, return + // + return; + } + // // Avoid APs access invalid buff data which allocated by BootServices, // so we will allocate reserved data for AP loop code. diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c index 56b870e..a0edc55 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -1143,6 +1143,7 @@ MpInitLibInitialize ( } else { MaxLogicalProcessorNumber = OldCpuMpData->CpuCount; } + ASSERT (MaxLogicalProcessorNumber != 0); AsmGetAddressMap (&AddressMap); ApResetVectorSize = AddressMap.RendezvousFunnelSize + sizeof (MP_CPU_EXCHANGE_INFO); @@ -1209,10 +1210,12 @@ MpInitLibInitialize ( MtrrGetAllMtrrs (&CpuMpData->MtrrTable); if (OldCpuMpData == NULL) { - // - // Wakeup all APs and calculate the processor count in system - // - CollectProcessorCount (CpuMpData); + if (MaxLogicalProcessorNumber > 1) { + // + // Wakeup all APs and calculate the processor count in system + // + CollectProcessorCount (CpuMpData); + } } else { // // APs have been wakeup before, just get the CPU Information @@ -1238,19 +1241,21 @@ MpInitLibInitialize ( sizeof (CPU_VOLATILE_REGISTERS) ); } - // - // Wakeup APs to do some AP initialize sync - // - WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData); - // - // Wait for all APs finished initialization - // - while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) { - CpuPause (); - } - CpuMpData->InitFlag = ApInitDone; - for (Index = 0; Index < CpuMpData->CpuCount; Index++) { - SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); + if (MaxLogicalProcessorNumber > 1) { + // + // Wakeup APs to do some AP initialize sync + // + WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData); + // + // Wait for all APs finished initialization + // + while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) { + CpuPause (); + } + CpuMpData->InitFlag = ApInitDone; + for (Index = 0; Index < CpuMpData->CpuCount; Index++) { + SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); + } } } diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c index e242d37..1f2fcb8 100644 --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c @@ -321,6 +321,14 @@ InitMpGlobalData ( EFI_STATUS Status; SaveCpuMpData (CpuMpData); + + if (CpuMpData->CpuCount == 1) { + // + // If only BSP exists, return + // + return; + } + // // Register an event for EndOfPei // -- 2.9.3.windows.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-11-09 7:21 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-11-04 8:18 [PATCH] UefiCpuPkg/MpInitLib: Do not wakeup AP if only one processor supported Jeff Fan 2016-11-04 16:40 ` Laszlo Ersek 2016-11-07 0:47 ` Fan, Jeff 2016-11-07 13:37 ` Laszlo Ersek 2016-11-08 0:55 ` Fan, Jeff 2016-11-08 13:37 ` Laszlo Ersek 2016-11-09 7:21 ` Tian, Feng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox