Re: [edk2-devel] [PATCH] UefiCpuPkg RegisterCpuFeaturesLib: Fix an ASSERTION issue

2019-07-10 Thread Dong, Eric 
Reviewed-by: Eric Dong 

> -Original Message-
> From: Zeng, Star
> Sent: Wednesday, July 10, 2019 7:43 PM
> To: devel@edk2.groups.io
> Cc: Zeng, Star ; Laszlo Ersek ;
> Dong, Eric ; Ni, Ray ; Kumar,
> Chandana C ; Li, Kevin Y
> 
> Subject: [PATCH] UefiCpuPkg RegisterCpuFeaturesLib: Fix an ASSERTION
> issue
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1968
> 
> We met assertion like below, it happens when there is only one processor.
> 
> ASSERT_EFI_ERROR (Status = Not started)
> ASSERT [CpuFeaturesDxe] X:\XXX\XXX\RegisterCpuFeaturesLib\
>   DxeRegisterCpuFeaturesLib.c(149): !EFI_ERROR (Status)
> 
> The code should not call StartupAllAPs when there is only one processor.
> 
> Cc: Laszlo Ersek 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Chandana Kumar 
> Cc: Kevin Li 
> Signed-off-by: Star Zeng 
> ---
>  .../CpuFeaturesInitialize.c   | 10 +++--
>  .../DxeRegisterCpuFeaturesLib.c   | 43 +++
>  .../PeiRegisterCpuFeaturesLib.c   | 11 +++--
>  3 files changed, 37 insertions(+), 27 deletions(-)
> 
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> index aff7ad600cad..1746f4f07f7b 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> @@ -1071,10 +1071,12 @@ CpuFeaturesDetect (
> 
>CpuInitDataInitialize ();
> 
> -  //
> -  // Wakeup all APs for data collection.
> -  //
> -  StartupAPsWorker (CollectProcessorData, NULL);
> +  if (CpuFeaturesData->NumberOfCpus > 1) {
> +//
> +// Wakeup all APs for data collection.
> +//
> +StartupAPsWorker (CollectProcessorData, NULL);  }
> 
>//
>// Collect data on BSP
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
> index 9c78a2d993c4..ffd99046a6cd 100644
> ---
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
> +++
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLi
> +++ b.c
> @@ -229,31 +229,36 @@ CpuFeaturesInitialize (
>OldBspNumber = GetProcessorIndex (CpuFeaturesData);
>CpuFeaturesData->BspNumber = OldBspNumber;
> 
> -  Status = gBS->CreateEvent (
> -  EVT_NOTIFY_WAIT,
> -  TPL_CALLBACK,
> -  EfiEventEmptyFunction,
> -  NULL,
> -  
> -  );
> -  ASSERT_EFI_ERROR (Status);
> +  if (CpuFeaturesData->NumberOfCpus > 1) {
> +Status = gBS->CreateEvent (
> +EVT_NOTIFY_WAIT,
> +TPL_CALLBACK,
> +EfiEventEmptyFunction,
> +NULL,
> +
> +);
> +ASSERT_EFI_ERROR (Status);
> +
> +//
> +// Wakeup all APs for programming.
> +//
> +StartupAPsWorker (SetProcessorRegister, MpEvent);  }
> 
> -  //
> -  // Wakeup all APs for programming.
> -  //
> -  StartupAPsWorker (SetProcessorRegister, MpEvent);
>//
>// Programming BSP
>//
>SetProcessorRegister (CpuFeaturesData);
> 
> -  //
> -  // Wait all processors to finish the task.
> -  //
> -  do {
> -Status = gBS->CheckEvent (MpEvent);
> -  } while (Status == EFI_NOT_READY);
> -  ASSERT_EFI_ERROR (Status);
> +  if (CpuFeaturesData->NumberOfCpus > 1) {
> +//
> +// Wait all processors to finish the task.
> +//
> +do {
> +  Status = gBS->CheckEvent (MpEvent);
> +} while (Status == EFI_NOT_READY);
> +ASSERT_EFI_ERROR (Status);
> +  }
> 
>//
>// Switch to new BSP if required
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c
> index 2b1553f9b84b..8ad5a40e5a44 100644
> ---
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c
> +++
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLi
> +++ b.c
> @@ -273,10 +273,13 @@ CpuFeaturesInitialize (
>// DXE type instance.
>//
> 
> -  //
> -  // Wakeup all APs for programming.
> -  //
> -  StartupAPsWorker (SetProcessorRegister, NULL);
> +  if (CpuFeaturesData->NumberOfCpus > 1) {
> +//
> +// Wakeup all APs for programming.
> +//
> +StartupAPsWorker (SetProcessorRegister, NULL);  }
> +
>//
>// Programming BSP
>//
> --
> 2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#43546): https://edk2.groups.io/g/devel/message/43546
Mute This Topic: https://groups.io/mt/32415229/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] UefiCpuPkg RegisterCpuFeaturesLib: Fix an ASSERTION issue

2019-07-10 Thread Laszlo Ersek 
On 07/10/19 13:42, Star Zeng wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1968
> 
> We met assertion like below, it happens when there is only
> one processor.
> 
> ASSERT_EFI_ERROR (Status = Not started)
> ASSERT [CpuFeaturesDxe] X:\XXX\XXX\RegisterCpuFeaturesLib\
>   DxeRegisterCpuFeaturesLib.c(149): !EFI_ERROR (Status)
> 
> The code should not call StartupAllAPs when there is only one processor.
> 
> Cc: Laszlo Ersek 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Chandana Kumar 
> Cc: Kevin Li 
> Signed-off-by: Star Zeng 
> ---
>  .../CpuFeaturesInitialize.c   | 10 +++--
>  .../DxeRegisterCpuFeaturesLib.c   | 43 +++
>  .../PeiRegisterCpuFeaturesLib.c   | 11 +++--
>  3 files changed, 37 insertions(+), 27 deletions(-)

I'll skip this patch.

Thanks!
Laszlo

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#43485): https://edk2.groups.io/g/devel/message/43485
Mute This Topic: https://groups.io/mt/32415229/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH] UefiCpuPkg RegisterCpuFeaturesLib: Fix an ASSERTION issue

2019-07-10 Thread Zeng, Star 
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1968

We met assertion like below, it happens when there is only
one processor.

ASSERT_EFI_ERROR (Status = Not started)
ASSERT [CpuFeaturesDxe] X:\XXX\XXX\RegisterCpuFeaturesLib\
  DxeRegisterCpuFeaturesLib.c(149): !EFI_ERROR (Status)

The code should not call StartupAllAPs when there is only one processor.

Cc: Laszlo Ersek 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Chandana Kumar 
Cc: Kevin Li 
Signed-off-by: Star Zeng 
---
 .../CpuFeaturesInitialize.c   | 10 +++--
 .../DxeRegisterCpuFeaturesLib.c   | 43 +++
 .../PeiRegisterCpuFeaturesLib.c   | 11 +++--
 3 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c 
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index aff7ad600cad..1746f4f07f7b 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -1071,10 +1071,12 @@ CpuFeaturesDetect (
 
   CpuInitDataInitialize ();
 
-  //
-  // Wakeup all APs for data collection.
-  //
-  StartupAPsWorker (CollectProcessorData, NULL);
+  if (CpuFeaturesData->NumberOfCpus > 1) {
+//
+// Wakeup all APs for data collection.
+//
+StartupAPsWorker (CollectProcessorData, NULL);
+  }
 
   //
   // Collect data on BSP
diff --git 
a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c 
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
index 9c78a2d993c4..ffd99046a6cd 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
@@ -229,31 +229,36 @@ CpuFeaturesInitialize (
   OldBspNumber = GetProcessorIndex (CpuFeaturesData);
   CpuFeaturesData->BspNumber = OldBspNumber;
 
-  Status = gBS->CreateEvent (
-  EVT_NOTIFY_WAIT,
-  TPL_CALLBACK,
-  EfiEventEmptyFunction,
-  NULL,
-  
-  );
-  ASSERT_EFI_ERROR (Status);
+  if (CpuFeaturesData->NumberOfCpus > 1) {
+Status = gBS->CreateEvent (
+EVT_NOTIFY_WAIT,
+TPL_CALLBACK,
+EfiEventEmptyFunction,
+NULL,
+
+);
+ASSERT_EFI_ERROR (Status);
+
+//
+// Wakeup all APs for programming.
+//
+StartupAPsWorker (SetProcessorRegister, MpEvent);
+  }
 
-  //
-  // Wakeup all APs for programming.
-  //
-  StartupAPsWorker (SetProcessorRegister, MpEvent);
   //
   // Programming BSP
   //
   SetProcessorRegister (CpuFeaturesData);
 
-  //
-  // Wait all processors to finish the task.
-  //
-  do {
-Status = gBS->CheckEvent (MpEvent);
-  } while (Status == EFI_NOT_READY);
-  ASSERT_EFI_ERROR (Status);
+  if (CpuFeaturesData->NumberOfCpus > 1) {
+//
+// Wait all processors to finish the task.
+//
+do {
+  Status = gBS->CheckEvent (MpEvent);
+} while (Status == EFI_NOT_READY);
+ASSERT_EFI_ERROR (Status);
+  }
 
   //
   // Switch to new BSP if required
diff --git 
a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c 
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c
index 2b1553f9b84b..8ad5a40e5a44 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c
@@ -273,10 +273,13 @@ CpuFeaturesInitialize (
   // DXE type instance.
   //
 
-  //
-  // Wakeup all APs for programming.
-  //
-  StartupAPsWorker (SetProcessorRegister, NULL);
+  if (CpuFeaturesData->NumberOfCpus > 1) {
+//
+// Wakeup all APs for programming.
+//
+StartupAPsWorker (SetProcessorRegister, NULL);
+  }
+
   //
   // Programming BSP
   //
-- 
2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#43476): https://edk2.groups.io/g/devel/message/43476
Mute This Topic: https://groups.io/mt/32415229/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-