From c6be6dab9c4bdf135bc02b61ecc304d5511c3588 Mon Sep 17 00:00:00 2001
From: Ray Ni <ray.ni@intel.com>
Date: Thu, 28 Jan 2021 13:03:54 +0800
Subject: UefiCpuPkg/MpInitLib: Don't increase CpuCount in ApWakeupFunction

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3179

When BSP first time wakes all APs, each AP atomically increases
CpuMpData->CpuCount and CpuMpData->FinishedCount.

Each AP atomically increases CpuMpData->NumApsExecuting
in early assembly code and decreases it before it enters to HLT or
MWAIT state.

Putting them together, the 3 variables are changed in the following order:
1. NumApsExecuting++ // in assembly
2. CpuCpunt++
4. FinishedCount++
3. NumApsExecuting-- // in C

BSP waits for a certain timeout and then polls NumApsExecuting
until it drops to zero. It assumes all APs are waken up concurrently
and NumApsExecuting only drops to zero when all APs have checked in.

Then it additionally waits for FinishedCount == CpuCount - 1. (FinishedCount doesn't include BSP while CpuCount includes BSP.)

There is no need to additionally wait for
FinishedCount == CpuCount - 1 because when NumApsExecuting == 0,
the number of increament of FinishedCount and CpuCount should equal.

This patch simplifies the code to remove "CpuCount++" in
ApWakeupFunction() and
assigns FinishedCount + 1 to CpuCount after WakeUpAP().

Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

(limited to 'UefiCpuPkg/Library')

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 8b1f7f84ba..2568986d8c 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1,7 +1,7 @@
 /** @file
   CPU MP Initialize Library common functions.
 
-  Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>
   Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -485,14 +485,12 @@ CollectProcessorCount (
   CpuMpData->InitFlag = ApInitConfig;
   WakeUpAP (CpuMpData, TRUE, 0, NULL, NULL, TRUE);
   CpuMpData->InitFlag = ApInitDone;
-  ASSERT (CpuMpData->CpuCount <= PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
   //
-  // Wait for all APs finished the initialization
+  // When InitFlag == ApInitConfig, WakeUpAP () guarantees all APs are checked in.
+  // FinishedCount is the number of check-in APs.
   //
-  while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) {
-    CpuPause ();
-  }
-
+  CpuMpData->CpuCount = CpuMpData->FinishedCount + 1;
+  ASSERT (CpuMpData->CpuCount <= PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
 
   //
   // Enable x2APIC mode if
@@ -751,10 +749,6 @@ ApWakeupFunction (
   CurrentApicMode = GetApicMode ();
   while (TRUE) {
     if (CpuMpData->InitFlag == ApInitConfig) {
-      //
-      // Add CPU number
-      //
-      InterlockedIncrement ((UINT32 *) &CpuMpData->CpuCount);
       ProcessorNumber = ApIndex;
       //
       // This is first time AP wakeup, get BIST information from AP stack
-- 
cgit v1.2.3