* [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