public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Jeff Fan <jeff.fan@intel.com>
To: edk2-devel@lists.01.org
Cc: Feng Tian <feng.tian@intel.com>,
	Michael D Kinney <michael.d.kinney@intel.com>
Subject: [PATCH 5/6] UefiCpuPkg/MpInitLib: Program AP stack in fixed address
Date: Mon, 14 Nov 2016 11:47:00 +0800	[thread overview]
Message-ID: <20161114034701.5996-6-jeff.fan@intel.com> (raw)
In-Reply-To: <20161114034701.5996-1-jeff.fan@intel.com>

Currently, MpInitLib will program AP stack in dynamic address. Each processor
will calculate its stack address by adding stack size based on the last stack
address. That means AP may have the different stack address everytime it is
wakeup by INIT-SIPI-SIPI.

When all APs have wakeup to execute AP task, each each has been assigned one
stack address. Once the timeout happened on some of APs, BSP will send INIT-
SIPI-SIPI to wake up APs. We need to re-assign stack for APs. Based on the
current implementation, we might assign one stack address used by other APs.
It will cause the unexpected stack overlapped issue.

This fix changed the stack assignment policy. We will record the stack address
assigned to AP at first time AP wakeup. When AP failed on AP task, BSP could
reassigned the same stack for it.

Getting initial APIC ID in assembly code could help AP to get saved its stack
address.

Cc: Feng Tian <feng.tian@intel.com>
Cc: Michael D 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/Ia32/MpFuncs.nasm | 65 +++++++++++++++++++-----
 UefiCpuPkg/Library/MpInitLib/MpLib.c           | 12 +++--
 UefiCpuPkg/Library/MpInitLib/MpLib.h           |  1 +
 UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm  | 68 +++++++++++++++++++++-----
 4 files changed, 119 insertions(+), 27 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index 8f6f0bf..4bfa084 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -114,7 +114,12 @@ Flat32Start:                                   ; protected mode entry point
     mov         cr0, eax
 
 SkipEnableExecuteDisable:
+    mov        edi, esi
+    add        edi, InitFlagLocation
+    cmp        dword [edi], 1       ; 1 == ApInitConfig
+    jnz        GetApicId
 
+    ; AP init
     mov        edi, esi
     add        edi, LockLocation
     mov        eax, NotVacantFlag
@@ -124,27 +129,65 @@ TestLock:
     cmp        eax, NotVacantFlag
     jz         TestLock
 
-    mov        edi, esi
-    add        edi, NumApsExecutingLocation
-    inc        dword [edi]
-    mov        ebx, [edi]
+    mov        ecx, esi
+    add        ecx, NumApsExecutingLocation
+    inc        dword [ecx]
+    mov        ebx, [ecx]
+
+Releaselock:
+    mov        eax, VacantFlag
+    xchg       [edi], eax
 
-ProgramStack:
     mov        edi, esi
     add        edi, StackSizeLocation
     mov        eax, [edi]
+    mov        ecx, ebx
+    inc        ecx
+    mul        ecx                               ; EAX = StackSize * (CpuNumber + 1)
     mov        edi, esi
     add        edi, StackStartAddressLocation
     add        eax, [edi]
     mov        esp, eax
-    mov        [edi], eax
+    jmp        CProcedureInvoke
+
+GetApicId:
+    mov        eax, 0
+    cpuid
+    cmp        eax, 0bh
+    jnb        X2Apic
+    ; Processor is not x2APIC capable, so get 8-bit APIC ID
+    mov        eax, 1
+    cpuid
+    shr        ebx, 24
+    mov        edx, ebx
+    jmp        GetProcessorNumber
+
+X2Apic:
+    ; Processor is x2APIC capable, so get 32-bit x2APIC ID
+    mov        eax, 0bh
+    xor        ecx, ecx
+    cpuid                   
+    ; edx save x2APIC ID
+    
+GetProcessorNumber:
+    ;
+    ; Get processor number for this AP
+    ; Note that BSP may become an AP due to SwitchBsp()
+    ;
+    xor         ebx, ebx
+    lea         eax, [esi + CpuInfoLocation]
+    mov         edi, [eax]
 
-Releaselock:
-    mov        eax, VacantFlag
-    mov        edi, esi
-    add        edi, LockLocation
-    xchg       [edi], eax
+GetNextProcNumber:
+    cmp         [edi], edx                       ; APIC ID match?
+    jz          ProgramStack
+    add         edi, 16
+    inc         ebx
+    jmp         GetNextProcNumber    
 
+ProgramStack:
+    mov         esp, [edi + 12]
+   
 CProcedureInvoke:
     push       ebp               ; push BIST data at top of AP stack
     xor        ebp, ebp          ; clear ebp for call stack trace
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index da814c6..748c8e7 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -433,7 +433,8 @@ VOID
 InitializeApData (
   IN OUT CPU_MP_DATA      *CpuMpData,
   IN     UINTN            ProcessorNumber,
-  IN     UINT32           BistData
+  IN     UINT32           BistData,
+  IN     UINTN            ApTopOfStack
   )
 {
   CPU_INFO_IN_HOB          *CpuInfoInHob;
@@ -442,6 +443,7 @@ InitializeApData (
   CpuInfoInHob[ProcessorNumber].InitialApicId = GetInitialApicId ();
   CpuInfoInHob[ProcessorNumber].ApicId        = GetApicId ();
   CpuInfoInHob[ProcessorNumber].Health        = BistData;
+  CpuInfoInHob[ProcessorNumber].ApTopOfStack  = (UINT32) ApTopOfStack;
 
   CpuMpData->CpuData[ProcessorNumber].Waiting    = FALSE;
   CpuMpData->CpuData[ProcessorNumber].CpuHealthy = (BistData == 0) ? TRUE : FALSE;
@@ -479,6 +481,7 @@ ApWakeupFunction (
   UINT32                     BistData;
   volatile UINT32            *ApStartupSignalBuffer;
   CPU_INFO_IN_HOB            *CpuInfoInHob;
+  UINTN                      ApTopOfStack;
 
   //
   // AP finished assembly code and begin to execute C code
@@ -497,7 +500,8 @@ ApWakeupFunction (
       //
       // This is first time AP wakeup, get BIST information from AP stack
       //
-      BistData = *(UINT32 *) (CpuMpData->Buffer + ProcessorNumber * CpuMpData->CpuApStackSize - sizeof (UINTN));
+      ApTopOfStack  = CpuMpData->Buffer + (ProcessorNumber + 1) * CpuMpData->CpuApStackSize;
+      BistData = *(UINT32 *) (ApTopOfStack - sizeof (UINTN));
       //
       // Do some AP initialize sync
       //
@@ -506,7 +510,7 @@ ApWakeupFunction (
       // Sync BSP's Control registers to APs
       //
       RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, FALSE);
-      InitializeApData (CpuMpData, ProcessorNumber, BistData);
+      InitializeApData (CpuMpData, ProcessorNumber, BistData, ApTopOfStack);
       ApStartupSignalBuffer = CpuMpData->CpuData[ProcessorNumber].StartupApSignal;
     } else {
       //
@@ -1195,7 +1199,7 @@ MpInitLibInitialize (
   //
   // Set BSP basic information
   //
-  InitializeApData (CpuMpData, 0, 0);
+  InitializeApData (CpuMpData, 0, 0, CpuMpData->Buffer);
   //
   // Save assembly code information
   //
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index a58c855..f81c819 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -133,6 +133,7 @@ typedef struct {
   UINT32                         InitialApicId;
   UINT32                         ApicId;
   UINT32                         Health;
+  UINT32                         ApTopOfStack;
 } CPU_INFO_IN_HOB;
 
 //
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 090e9fa..bfc3ff1 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -120,6 +120,12 @@ LongModeStart:
     mov        ss,  ax
 
     mov        esi, ebx
+    lea        edi, [esi + InitFlagLocation]
+    cmp        qword [edi], 1       ; ApInitConfig
+    jnz        GetApicId
+
+    ; AP init
+    mov        esi, ebx
     mov        edi, esi
     add        edi, LockLocation
     mov        rax, NotVacantFlag
@@ -129,26 +135,64 @@ TestLock:
     cmp        rax, NotVacantFlag
     jz         TestLock
 
-    mov        edi, esi
-    add        edi, NumApsExecutingLocation
-    inc        dword [edi]
-    mov        ebx, [edi]
+    lea        ecx, [esi + InitFlagLocation]
+    inc        dword [ecx]
+    mov        ebx, [ecx]
 
-ProgramStack:
+Releaselock:
+    mov        rax, VacantFlag
+    xchg       qword [edi], rax
+    ; program stack
     mov        edi, esi
     add        edi, StackSizeLocation
-    mov        rax, qword [edi]
+    mov        eax, dword [edi]
+    mov        ecx, ebx
+    inc        ecx
+    mul        ecx                               ; EAX = StackSize * (CpuNumber + 1)
     mov        edi, esi
     add        edi, StackStartAddressLocation
     add        rax, qword [edi]
     mov        rsp, rax
-    mov        qword [edi], rax
+    jmp        CProcedureInvoke
+
+GetApicId:
+    mov        eax, 0
+    cpuid
+    cmp        eax, 0bh
+    jnb        X2Apic
+    ; Processor is not x2APIC capable, so get 8-bit APIC ID
+    mov        eax, 1
+    cpuid
+    shr        ebx, 24
+    mov        edx, ebx
+    jmp        GetProcessorNumber
+
+X2Apic:
+    ; Processor is x2APIC capable, so get 32-bit x2APIC ID
+    mov        eax, 0bh
+    xor        ecx, ecx
+    cpuid                   
+    ; edx save x2APIC ID
+    
+GetProcessorNumber:
+    ;
+    ; Get processor number for this AP
+    ; Note that BSP may become an AP due to SwitchBsp()
+    ;
+    xor         ebx, ebx
+    lea         eax, [esi + CpuInfoLocation]
+    mov         edi, [eax]
 
-Releaselock:
-    mov        rax, VacantFlag
-    mov        edi, esi
-    add        edi, LockLocation
-    xchg       qword [edi], rax
+GetNextProcNumber:
+    cmp         dword [edi], edx                      ; APIC ID match?
+    jz          ProgramStack
+    add         edi, 16
+    inc         ebx
+    jmp         GetNextProcNumber    
+
+ProgramStack:
+    xor         rsp, rsp
+    mov         esp, dword [edi + 12]
 
 CProcedureInvoke:
     push       rbp               ; Push BIST data at top of AP stack
-- 
2.9.3.windows.2



  parent reply	other threads:[~2016-11-14  3:47 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-14  3:46 [PATCH 0/6] UefiCpuPkg/MpInitLib: Program AP stack in fixed address Jeff Fan
2016-11-14  3:46 ` [PATCH 1/6] UefiCpuPkg/MpInitLib: Fixed offset error on Cr3Location Jeff Fan
2016-11-14  3:46 ` [PATCH 2/6] UefiCpuPkg/MpInitLib: Force sending INIT-SIPI-SIPI to reset APs Jeff Fan
2016-11-14  3:46 ` [PATCH 3/6] UefiCpuPkg/MpInitLib: Remove CPU information from CPU_AP_DATA Jeff Fan
2016-11-14  3:46 ` [PATCH 4/6] UefiCpuPkg/MpInitLib: Add InitFlag and CpuInfo in MP_CPU_EXCHANGE_INFO Jeff Fan
2016-11-14  3:47 ` Jeff Fan [this message]
2016-11-14  3:47 ` [PATCH 6/6] UefiCpuPkg/MpInitLib: Update AP information when BSP switched Jeff Fan
2016-11-16  8:00 ` [PATCH 0/6] UefiCpuPkg/MpInitLib: Program AP stack in fixed address Tian, Feng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161114034701.5996-6-jeff.fan@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox