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

Zeng, Star posted 1 patch 1 week ago
Failed in applying to current master (apply log)
.../CpuFeaturesInitialize.c                   | 10 +++--
.../DxeRegisterCpuFeaturesLib.c               | 43 +++++++++++--------
.../PeiRegisterCpuFeaturesLib.c               | 11 +++--
3 files changed, 37 insertions(+), 27 deletions(-)

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

Posted by Zeng, Star 1 week ago
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 <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Cc: Kevin Li <kevin.y.li@intel.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 .../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,
-                  &MpEvent
-                  );
-  ASSERT_EFI_ERROR (Status);
+  if (CpuFeaturesData->NumberOfCpus > 1) {
+    Status = gBS->CreateEvent (
+                    EVT_NOTIFY_WAIT,
+                    TPL_CALLBACK,
+                    EfiEventEmptyFunction,
+                    NULL,
+                    &MpEvent
+                    );
+    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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Dong, Eric 1 week ago
Reviewed-by: Eric Dong <eric.dong@intel.com>

> -----Original Message-----
> From: Zeng, Star
> Sent: Wednesday, July 10, 2019 7:43 PM
> To: devel@edk2.groups.io
> Cc: Zeng, Star <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Chandana C <chandana.c.kumar@intel.com>; Li, Kevin Y
> <kevin.y.li@intel.com>
> 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 <lersek@redhat.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Chandana Kumar <chandana.c.kumar@intel.com>
> Cc: Kevin Li <kevin.y.li@intel.com>
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  .../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,
> -                  &MpEvent
> -                  );
> -  ASSERT_EFI_ERROR (Status);
> +  if (CpuFeaturesData->NumberOfCpus > 1) {
> +    Status = gBS->CreateEvent (
> +                    EVT_NOTIFY_WAIT,
> +                    TPL_CALLBACK,
> +                    EfiEventEmptyFunction,
> +                    NULL,
> +                    &MpEvent
> +                    );
> +    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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Laszlo Ersek 1 week ago
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 <lersek@redhat.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Chandana Kumar <chandana.c.kumar@intel.com>
> Cc: Kevin Li <kevin.y.li@intel.com>
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  .../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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-