Re: [edk2-devel] [PATCH] Pkg-Module:UefiCpuPkg/MpLib

2024-05-23 Thread Ning Feng
hi, ray looks like after only changed the title, there created another groups, 
please check below link. thanks
[PATCH] Pkg-Module:UefiCpuPkg/MpLib:Do not assume BSP is #0 (groups.io) ( 
https://edk2.groups.io/g/devel/message/119161 )


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119170): https://edk2.groups.io/g/devel/message/119170
Mute This Topic: https://groups.io/mt/106256300/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH] Pkg-Module:UefiCpuPkg/MpLib

2024-05-23 Thread Ni, Ray
Ning,
The patch looks good to me.
But it seems you did not change the patch title to a more specific message.

[Ray.1] The subject should be more specific. E.g.: UefiCpuPkg/MpInitLib: Do not 
assume BSP is #0.




Thanks,
Ray

From: Feng, Ning 
Sent: Friday, May 24, 2024 2:02
To: devel@edk2.groups.io 
Cc: Feng, Ning ; Ni, Ray 
Subject: [PATCH] Pkg-Module:UefiCpuPkg/MpLib

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4778
MPInitlib have wrong expectation that BSP index should always be 0 in
MpInitLibInitialize(), SwitchBsp(),ApWakeupFunction().
That will cause the data mismatch, if the initial BSP is not 0.
Cc: Ray Ni 
Signed-off-by: Ning Feng 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 34 
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index d724456502..ae279c6ceb 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -114,6 +114,10 @@ FutureBSPProc (
   SaveVolatileRegisters (&DataInHob->APInfo.VolatileRegisters);

   AsmExchangeRole (&DataInHob->APInfo, &DataInHob->BSPInfo);

   RestoreVolatileRegisters (&DataInHob->APInfo.VolatileRegisters, FALSE);

+  //

+  // Restore VolatileReg saved in CpuMpData->CpuData

+  //

+  CopyMem (&DataInHob->CpuData[DataInHob->BspNumber].VolatileRegisters, 
&DataInHob->APInfo.VolatileRegisters, sizeof (CPU_VOLATILE_REGISTERS));

 }



 /**

@@ -761,11 +765,11 @@ ApWakeupFunction (
   BistData = (UINT32)ApStackData->Bist;



   //

-  // CpuMpData->CpuData[0].VolatileRegisters is initialized based on BSP 
environment,

+  // CpuMpData->CpuData[BspNumber].VolatileRegisters is initialized based 
on BSP environment,

   //   to initialize AP in InitConfig path.

-  // NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters 
points to a different IDT shared by all APs.

+  // NOTE: IDTR.BASE stored in 
CpuMpData->CpuData[BspNumber].VolatileRegisters points to a different IDT 
shared by all APs.

   //

-  RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, 
FALSE);

+  RestoreVolatileRegisters 
(&CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, FALSE);

   InitializeApData (CpuMpData, ProcessorNumber, BistData, ApTopOfStack);

   ApStartupSignalBuffer = 
CpuMpData->CpuData[ProcessorNumber].StartupApSignal;

 } else {

@@ -798,10 +802,10 @@ ApWakeupFunction (
 // 1. AP is re-enabled after it's disabled, in either PEI or DXE phase.

 // 2. AP is initialized in DXE phase.

 // In either case, use the volatile registers value derived from BSP.

-// NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters 
points to a

+// NOTE: IDTR.BASE stored in 
CpuMpData->CpuData[BspNumber].VolatileRegisters points to a

 //   different IDT shared by all APs.

 //

-RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, 
FALSE);

+RestoreVolatileRegisters 
(&CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, FALSE);

   } else {

 if (CpuMpData->ApLoopMode == ApInHltLoop) {

   //

@@ -927,7 +931,7 @@ DxeApEntryPoint (
 AsmWriteMsr64 (MSR_IA32_EFER, EferMsr.Uint64);

   }



-  RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, FALSE);

+  RestoreVolatileRegisters 
(&CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, FALSE);

   InterlockedIncrement ((UINT32 *)&CpuMpData->FinishedCount);

   PlaceAPInMwaitLoopOrRunLoop (

 CpuMpData->ApLoopMode,

@@ -2151,7 +2155,12 @@ MpInitLibInitialize (
   CpuMpData->BackupBufferSize = ApResetVectorSizeBelow1Mb;

   CpuMpData->WakeupBuffer = (UINTN)-1;

   CpuMpData->CpuCount = 1;

-  CpuMpData->BspNumber= 0;

+  if (MpHandOff == NULL) {

+CpuMpData->BspNumber = 0;

+  } else {

+CpuMpData->BspNumber = GetBspNumber (MpHandOff);

+  }

+

   CpuMpData->WaitEvent= NULL;

   CpuMpData->SwitchBspFlag= FALSE;

   CpuMpData->CpuData  = (CPU_AP_DATA *)(CpuMpData + 1);

@@ -2186,11 +2195,11 @@ MpInitLibInitialize (
   // Don't pass BSP's TR to APs to avoid AP init failure.

   //

   VolatileRegisters.Tr = 0;

-  CopyMem (&CpuMpData->CpuData[0].VolatileRegisters, &VolatileRegisters, 
sizeof (VolatileRegisters));

+  CopyMem (&CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, 
&VolatileRegisters, sizeof (VolatileRegisters));

   //

   // Set BSP basic information

   //

-  InitializeApData (CpuMpData, 0, 0, CpuMpData->Buffer + ApStackSize);

+  InitializeApData (CpuMpData, CpuMpData->BspNumber, 0, CpuMpData->Buffer + 
ApStackSize * (CpuMpData->BspNumber + 1));

   //

   // Save assembly code information

   //

@@ -2615,7 +2624,12 @@ SwitchBSPWorker (
   SaveVolatileRegisters (&CpuMpData->BSPInfo.VolatileRegisters);

   AsmExchangeRole (&CpuMpData->BSPInfo, &C

Re: [edk2-devel] [PATCH] Pkg-Module:UefiCpuPkg/MpLib

2024-05-23 Thread Ning Feng
Hi, ray,i have already update the patch as per request. run the uncrustify from 
local, fixed the coding style issue.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119141): https://edk2.groups.io/g/devel/message/119141
Mute This Topic: https://groups.io/mt/106256300/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH] Pkg-Module:UefiCpuPkg/MpLib

2024-05-23 Thread Ning Feng
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4778
MPInitlib have wrong expectation that BSP index should always be 0 in
MpInitLibInitialize(), SwitchBsp(),ApWakeupFunction().
That will cause the data mismatch, if the initial BSP is not 0.
Cc: Ray Ni 
Signed-off-by: Ning Feng 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 34 
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index d724456502..ae279c6ceb 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -114,6 +114,10 @@ FutureBSPProc (
   SaveVolatileRegisters (&DataInHob->APInfo.VolatileRegisters);
   AsmExchangeRole (&DataInHob->APInfo, &DataInHob->BSPInfo);
   RestoreVolatileRegisters (&DataInHob->APInfo.VolatileRegisters, FALSE);
+  //
+  // Restore VolatileReg saved in CpuMpData->CpuData
+  //
+  CopyMem (&DataInHob->CpuData[DataInHob->BspNumber].VolatileRegisters, 
&DataInHob->APInfo.VolatileRegisters, sizeof (CPU_VOLATILE_REGISTERS));
 }
 
 /**
@@ -761,11 +765,11 @@ ApWakeupFunction (
   BistData = (UINT32)ApStackData->Bist;
 
   //
-  // CpuMpData->CpuData[0].VolatileRegisters is initialized based on BSP 
environment,
+  // CpuMpData->CpuData[BspNumber].VolatileRegisters is initialized based 
on BSP environment,
   //   to initialize AP in InitConfig path.
-  // NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters 
points to a different IDT shared by all APs.
+  // NOTE: IDTR.BASE stored in 
CpuMpData->CpuData[BspNumber].VolatileRegisters points to a different IDT 
shared by all APs.
   //
-  RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, 
FALSE);
+  RestoreVolatileRegisters 
(&CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, FALSE);
   InitializeApData (CpuMpData, ProcessorNumber, BistData, ApTopOfStack);
   ApStartupSignalBuffer = 
CpuMpData->CpuData[ProcessorNumber].StartupApSignal;
 } else {
@@ -798,10 +802,10 @@ ApWakeupFunction (
 // 1. AP is re-enabled after it's disabled, in either PEI or DXE phase.
 // 2. AP is initialized in DXE phase.
 // In either case, use the volatile registers value derived from BSP.
-// NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters 
points to a
+// NOTE: IDTR.BASE stored in 
CpuMpData->CpuData[BspNumber].VolatileRegisters points to a
 //   different IDT shared by all APs.
 //
-RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, 
FALSE);
+RestoreVolatileRegisters 
(&CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, FALSE);
   } else {
 if (CpuMpData->ApLoopMode == ApInHltLoop) {
   //
@@ -927,7 +931,7 @@ DxeApEntryPoint (
 AsmWriteMsr64 (MSR_IA32_EFER, EferMsr.Uint64);
   }
 
-  RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, FALSE);
+  RestoreVolatileRegisters 
(&CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, FALSE);
   InterlockedIncrement ((UINT32 *)&CpuMpData->FinishedCount);
   PlaceAPInMwaitLoopOrRunLoop (
 CpuMpData->ApLoopMode,
@@ -2151,7 +2155,12 @@ MpInitLibInitialize (
   CpuMpData->BackupBufferSize = ApResetVectorSizeBelow1Mb;
   CpuMpData->WakeupBuffer = (UINTN)-1;
   CpuMpData->CpuCount = 1;
-  CpuMpData->BspNumber= 0;
+  if (MpHandOff == NULL) {
+CpuMpData->BspNumber = 0;
+  } else {
+CpuMpData->BspNumber = GetBspNumber (MpHandOff);
+  }
+
   CpuMpData->WaitEvent= NULL;
   CpuMpData->SwitchBspFlag= FALSE;
   CpuMpData->CpuData  = (CPU_AP_DATA *)(CpuMpData + 1);
@@ -2186,11 +2195,11 @@ MpInitLibInitialize (
   // Don't pass BSP's TR to APs to avoid AP init failure.
   //
   VolatileRegisters.Tr = 0;
-  CopyMem (&CpuMpData->CpuData[0].VolatileRegisters, &VolatileRegisters, 
sizeof (VolatileRegisters));
+  CopyMem (&CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, 
&VolatileRegisters, sizeof (VolatileRegisters));
   //
   // Set BSP basic information
   //
-  InitializeApData (CpuMpData, 0, 0, CpuMpData->Buffer + ApStackSize);
+  InitializeApData (CpuMpData, CpuMpData->BspNumber, 0, CpuMpData->Buffer + 
ApStackSize * (CpuMpData->BspNumber + 1));
   //
   // Save assembly code information
   //
@@ -2615,7 +2624,12 @@ SwitchBSPWorker (
   SaveVolatileRegisters (&CpuMpData->BSPInfo.VolatileRegisters);
   AsmExchangeRole (&CpuMpData->BSPInfo, &CpuMpData->APInfo);
   RestoreVolatileRegisters (&CpuMpData->BSPInfo.VolatileRegisters, FALSE);
-
+  //
+  // Restore VolatileRegs saved in CpuMpData->CpuData
+  // Don't pass BSP's TR to APs to avoid AP init failure.
+  //
+  CopyMem (&CpuMpData->CpuData[CpuMpData->NewBspNumber].VolatileRegisters, 
&CpuMpData->BSPInfo.VolatileRegisters, sizeof (CPU_VOLATILE_REGISTERS));
+  CpuMpData->CpuData[CpuMpData->NewBspNumber].VolatileRegisters.Tr = 0;
   //
   // Set the BSP bit o

[edk2-devel] [PATCH] Pkg-Module:UefiCpuPkg/MpLib

2024-05-23 Thread Ning Feng
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4778
MPInitlib have wrong expectation that bsp id should always be 0 in
MpInitLibInitialize(), SwitchBsp(),ApWakeupFunction().
That will cause the data mismatch, if the initial bsp is not 0.
Use CpuMpData->BspNumber insted of index 0 to avoid the issue.
Cc: Ray Ni 
Signed-off-by: Ning Feng 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 34 
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index d724456502..ae279c6ceb 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -114,6 +114,10 @@ FutureBSPProc (
   SaveVolatileRegisters (&DataInHob->APInfo.VolatileRegisters);
   AsmExchangeRole (&DataInHob->APInfo, &DataInHob->BSPInfo);
   RestoreVolatileRegisters (&DataInHob->APInfo.VolatileRegisters, FALSE);
+  //
+  // Restore VolatileReg saved in CpuMpData->CpuData
+  //
+  CopyMem (&DataInHob->CpuData[DataInHob->BspNumber].VolatileRegisters, 
&DataInHob->APInfo.VolatileRegisters, sizeof (CPU_VOLATILE_REGISTERS));
 }
 
 /**
@@ -761,11 +765,11 @@ ApWakeupFunction (
   BistData = (UINT32)ApStackData->Bist;
 
   //
-  // CpuMpData->CpuData[0].VolatileRegisters is initialized based on BSP 
environment,
+  // CpuMpData->CpuData[BspNumber].VolatileRegisters is initialized based 
on BSP environment,
   //   to initialize AP in InitConfig path.
-  // NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters 
points to a different IDT shared by all APs.
+  // NOTE: IDTR.BASE stored in 
CpuMpData->CpuData[BspNumber].VolatileRegisters points to a different IDT 
shared by all APs.
   //
-  RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, 
FALSE);
+  RestoreVolatileRegisters 
(&CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, FALSE);
   InitializeApData (CpuMpData, ProcessorNumber, BistData, ApTopOfStack);
   ApStartupSignalBuffer = 
CpuMpData->CpuData[ProcessorNumber].StartupApSignal;
 } else {
@@ -798,10 +802,10 @@ ApWakeupFunction (
 // 1. AP is re-enabled after it's disabled, in either PEI or DXE phase.
 // 2. AP is initialized in DXE phase.
 // In either case, use the volatile registers value derived from BSP.
-// NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters 
points to a
+// NOTE: IDTR.BASE stored in 
CpuMpData->CpuData[BspNumber].VolatileRegisters points to a
 //   different IDT shared by all APs.
 //
-RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, 
FALSE);
+RestoreVolatileRegisters 
(&CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, FALSE);
   } else {
 if (CpuMpData->ApLoopMode == ApInHltLoop) {
   //
@@ -927,7 +931,7 @@ DxeApEntryPoint (
 AsmWriteMsr64 (MSR_IA32_EFER, EferMsr.Uint64);
   }
 
-  RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, FALSE);
+  RestoreVolatileRegisters 
(&CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, FALSE);
   InterlockedIncrement ((UINT32 *)&CpuMpData->FinishedCount);
   PlaceAPInMwaitLoopOrRunLoop (
 CpuMpData->ApLoopMode,
@@ -2151,7 +2155,12 @@ MpInitLibInitialize (
   CpuMpData->BackupBufferSize = ApResetVectorSizeBelow1Mb;
   CpuMpData->WakeupBuffer = (UINTN)-1;
   CpuMpData->CpuCount = 1;
-  CpuMpData->BspNumber= 0;
+  if (MpHandOff == NULL) {
+CpuMpData->BspNumber = 0;
+  } else {
+CpuMpData->BspNumber = GetBspNumber (MpHandOff);
+  }
+
   CpuMpData->WaitEvent= NULL;
   CpuMpData->SwitchBspFlag= FALSE;
   CpuMpData->CpuData  = (CPU_AP_DATA *)(CpuMpData + 1);
@@ -2186,11 +2195,11 @@ MpInitLibInitialize (
   // Don't pass BSP's TR to APs to avoid AP init failure.
   //
   VolatileRegisters.Tr = 0;
-  CopyMem (&CpuMpData->CpuData[0].VolatileRegisters, &VolatileRegisters, 
sizeof (VolatileRegisters));
+  CopyMem (&CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, 
&VolatileRegisters, sizeof (VolatileRegisters));
   //
   // Set BSP basic information
   //
-  InitializeApData (CpuMpData, 0, 0, CpuMpData->Buffer + ApStackSize);
+  InitializeApData (CpuMpData, CpuMpData->BspNumber, 0, CpuMpData->Buffer + 
ApStackSize * (CpuMpData->BspNumber + 1));
   //
   // Save assembly code information
   //
@@ -2615,7 +2624,12 @@ SwitchBSPWorker (
   SaveVolatileRegisters (&CpuMpData->BSPInfo.VolatileRegisters);
   AsmExchangeRole (&CpuMpData->BSPInfo, &CpuMpData->APInfo);
   RestoreVolatileRegisters (&CpuMpData->BSPInfo.VolatileRegisters, FALSE);
-
+  //
+  // Restore VolatileRegs saved in CpuMpData->CpuData
+  // Don't pass BSP's TR to APs to avoid AP init failure.
+  //
+  CopyMem (&CpuMpData->CpuData[CpuMpData->NewBspNumber].VolatileRegisters, 
&CpuMpData->BSPInfo.VolatileRegisters, sizeof (CPU_VOLATILE_REGISTERS));
+  CpuMpData->CpuData[CpuMpData->NewBspNum

[edk2-devel] [PATCH] Pkg-Module:UefiCpuPkg/MpLib

2024-05-23 Thread Ning Feng
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4778
MPInitlib have wrong expectation that bsp id should always be 0 in
MpInitLibInitialize(), SwitchBsp(),ApWakeupFunction().
That will cause the data mismatch, if the initial bsp is not 0.
Use CpuMpData->BspNumber insted of index 0 to avoid the issue.
Cc: Ray Ni 
Signed-off-by: Ning Feng 
---
 ...-CodeQL-Update-from-2.16.1-to-2.17.3.patch |  82 
 0001-Pkg-Module-UefiCpuPkg-MpLib.patch| 120 ++
 UefiCpuPkg/Library/MpInitLib/MpLib.c  |  34 +++--
 3 files changed, 226 insertions(+), 10 deletions(-)
 create mode 100644 0001-CodeQL-Update-from-2.16.1-to-2.17.3.patch
 create mode 100644 0001-Pkg-Module-UefiCpuPkg-MpLib.patch

diff --git a/0001-CodeQL-Update-from-2.16.1-to-2.17.3.patch 
b/0001-CodeQL-Update-from-2.16.1-to-2.17.3.patch
new file mode 100644
index 00..2ae61e1a4b
--- /dev/null
+++ b/0001-CodeQL-Update-from-2.16.1-to-2.17.3.patch
@@ -0,0 +1,82 @@
+From 7142e648416ff5d3eac6c6d607874805f5de0ca8 Mon Sep 17 00:00:00 2001
+From: Michael Kubacki 
+Date: Fri, 17 May 2024 16:27:36 -0400
+Subject: [PATCH] CodeQL: Update from 2.16.1 to 2.17.3
+
+This fixes an issue where the CodeQL queries currently fetched in the
+pipeline are incompatible with the current executable used.
+
+Update to pick up functional and security fixes. See the following
+comparison for detailed differences:
+
+https://github.com/github/codeql-cli-binaries/compare/v2.16.1...v2.17.3
+
+Cc: Bob Feng 
+Cc: Joey Vagedes 
+Cc: Liming Gao 
+Cc: Michael D Kinney 
+Cc: Rebecca Cran 
+Cc: Sean Brogan 
+Cc: Yuwei Chen 
+Signed-off-by: Michael Kubacki 
+Reviewed-by: Michael D Kinney 
+---
+ BaseTools/Plugin/CodeQL/codeqlcli_ext_dep.yaml | 6 +++---
+ BaseTools/Plugin/CodeQL/codeqlcli_linux_ext_dep.yaml   | 6 +++---
+ BaseTools/Plugin/CodeQL/codeqlcli_windows_ext_dep.yaml | 6 +++---
+ 3 files changed, 9 insertions(+), 9 deletions(-)
+
+diff --git a/BaseTools/Plugin/CodeQL/codeqlcli_ext_dep.yaml 
b/BaseTools/Plugin/CodeQL/codeqlcli_ext_dep.yaml
+index 5ec56c6bf0..dbc9c2ba02 100644
+--- a/BaseTools/Plugin/CodeQL/codeqlcli_ext_dep.yaml
 b/BaseTools/Plugin/CodeQL/codeqlcli_ext_dep.yaml
+@@ -16,9 +16,9 @@
+   "scope": "codeql-ext-dep",
+   "type": "web",
+   "name": "codeql_cli",
+-  "source": 
"https://github.com/github/codeql-cli-binaries/releases/download/v2.16.1/codeql.zip";,
+-  "version": "2.16.1",
+-  "sha256": 
"86a98f6ebb8fd49efadf367f3275c438669fcb8426962c33415129aad8e093e6",
++  "source": 
"https://github.com/github/codeql-cli-binaries/releases/download/v2.17.3/codeql.zip";,
++  "version": "2.17.3",
++  "sha256": 
"e5ac1d87ab38e405c9af5db234a338b10dffabc98a648903f1664dd2a566dfd5",
+   "compression_type": "zip",
+   "internal_path": "/codeql/",
+   "flags": ["set_shell_var", ],
+diff --git a/BaseTools/Plugin/CodeQL/codeqlcli_linux_ext_dep.yaml 
b/BaseTools/Plugin/CodeQL/codeqlcli_linux_ext_dep.yaml
+index 5b4a919f1d..536322f2b3 100644
+--- a/BaseTools/Plugin/CodeQL/codeqlcli_linux_ext_dep.yaml
 b/BaseTools/Plugin/CodeQL/codeqlcli_linux_ext_dep.yaml
+@@ -14,9 +14,9 @@
+   "scope": "codeql-linux-ext-dep",
+   "type": "web",
+   "name": "codeql_linux_cli",
+-  "source": 
"https://github.com/github/codeql-cli-binaries/releases/download/v2.16.1/codeql-linux64.zip";,
+-  "version": "2.16.1",
+-  "sha256": 
"40dbb6c0c4064bd14601a02e60c61661fdc0271469f90eb91a2e7d51d4cbc171",
++  "source": 
"https://github.com/github/codeql-cli-binaries/releases/download/v2.17.3/codeql-linux64.zip";,
++  "version": "2.17.3",
++  "sha256": 
"9fba000c4b821534d354bc16821aa066fdb1304446226ea449870e64a8ad3c7a",
+   "compression_type": "zip",
+   "internal_path": "/codeql/",
+   "flags": ["set_shell_var", ],
+diff --git a/BaseTools/Plugin/CodeQL/codeqlcli_windows_ext_dep.yaml 
b/BaseTools/Plugin/CodeQL/codeqlcli_windows_ext_dep.yaml
+index c0c018c953..93a81ffd50 100644
+--- a/BaseTools/Plugin/CodeQL/codeqlcli_windows_ext_dep.yaml
 b/BaseTools/Plugin/CodeQL/codeqlcli_windows_ext_dep.yaml
+@@ -14,9 +14,9 @@
+   "scope": "codeql-windows-ext-dep",
+   "type": "web",
+   "name": "codeql_windows_cli",
+-  "source": 
"https://github.com/github/codeql-cli-binaries/releases/download/v2.16.1/codeql-win64.zip";,
+-  "version": "2.16.1",
+-  "sha256": 
"9ebe5ea8a7d0a77425428d50d49912319117fccee24ecb62f6219c12584f4f28",
++  "source": 
"https://github.com/github/codeql-cli-binaries/releases/download/v2.17.3/codeql-win64.zip";,
++  "version": "2.17.3",
++  "sha256": 
"4c6fbf2ea2eaf0f47bf0347eacf54c6b9d6bdf7acb6b63e17f9e6f2dd83b34e7",
+   "compression_type": "zip",
+   "internal_path": "/codeql/",
+   "flags": ["set_shell_var", ],
+-- 
+2.25.1
+
diff --git a/0001-Pkg-Module-UefiCpuPkg-MpLib.patch 
b/0001-Pkg-Module-UefiCpuPkg-MpLib.patch
new file mode 100644
index 00..1d769ce1ce
--- /dev/null
+++ b/0001-Pkg-Module-UefiCpuPkg-MpLib.patch
@@ -0,0 +1,120 @@
+From 3fbdb37f1ffe14917d38af0157562b23f5b3cdf0 Mon Sep 17 00:00:00 2001
+From: Ning Feng 
+Date: Thu, 23 May 202

Re: [edk2-devel] [PATCH] Pkg-Module:UefiCpuPkg/MpLib

2024-05-23 Thread Ni, Ray
Ning,
I tried to run uncrustify locally and still saw additional changes. It might 
mean the code style doesn't follow the uncrustify.
more comments embedded in the patch mail starting with [Ray.].

Thanks,
Ray


From: Feng, Ning 
Sent: Thursday, May 23, 2024 19:13
To: devel@edk2.groups.io 
Cc: Feng, Ning ; Ni, Ray ; Wu, Jiaxin 

Subject: [PATCH] Pkg-Module:UefiCpuPkg/MpLib
[Ray.1] The subject should be more specific. E.g.: UefiCpuPkg/MpInitLib: Do not 
assume BSP is #0



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

MPlib have wrong expectation that bsp id should always be 0 in
[Ray.1] MPlib should be MpInitLib. "bsp id" -> "BSP index".

MpInitLibInitialize(), SwitchBsp(),ApWakeupFunction().

That will caused the data mismatch, if the beginning bsp is not 0.
[Ray.2] "will caused" -> "will cause", "beginning bsp" -> "initial bsp".

Use CpuMpData->NewBspNumber insted of index 0 to avoid the issue.
[Ray.3] "NewBspNumber" is only used in SwitchBsp() but not other places. I 
would just delete this from
commit message.



+  // Restore VolatileReg saved in CpuMpData->CpuData
[Ray.4] "Restore VolatileRegisters saved in CpuMpData->CpuData".




   //

-  InitializeApData (CpuMpData, 0, 0, CpuMpData->Buffer + ApStackSize);

+  InitializeApData (CpuMpData, CpuMpData->BspNumber, 0, CpuMpData->Buffer + 
ApStackSize * (CpuMpData->BspNumber+1));
[Ray.5] "BspNumber+1" -> "BspNumber + 1". (spaces before and after "+")




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119133): https://edk2.groups.io/g/devel/message/119133
Mute This Topic: https://groups.io/mt/106256300/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH] Pkg-Module:UefiCpuPkg/MpLib

2024-05-22 Thread Ning Feng
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4778
MPlib have wrong expectation that bsp id should always be 0 in
MpInitLibInitialize(), SwitchBsp(),ApWakeupFunction().
That will caused the data mismatch, if the beginning bsp is not 0.
Use CpuMpData->NewBspNumber insted of index 0 to avoid the issue.
Cc: Ray Ni 
Cc: Jiaxin Wu 
Signed-off-by: Ning Feng 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 34 +++-
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index d724456502..3834f7236e 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -114,6 +114,10 @@ FutureBSPProc (
   SaveVolatileRegisters (&DataInHob->APInfo.VolatileRegisters);
   AsmExchangeRole (&DataInHob->APInfo, &DataInHob->BSPInfo);
   RestoreVolatileRegisters (&DataInHob->APInfo.VolatileRegisters, FALSE);
+  //
+  // Restore VolatileReg saved in CpuMpData->CpuData
+  //
+  CopyMem (&DataInHob->CpuData[DataInHob->BspNumber].VolatileRegisters, 
&DataInHob->APInfo.VolatileRegisters, sizeof(CPU_VOLATILE_REGISTERS));
 }

 /**
@@ -761,11 +765,11 @@ ApWakeupFunction (
   BistData = (UINT32)ApStackData->Bist;

   //
-  // CpuMpData->CpuData[0].VolatileRegisters is initialized based on BSP 
environment,
+  // CpuMpData->CpuData[BspNumber].VolatileRegisters is initialized based 
on BSP environment,
   //   to initialize AP in InitConfig path.
-  // NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters 
points to a different IDT shared by all APs.
+  // NOTE: IDTR.BASE stored in 
CpuMpData->CpuData[BspNumber].VolatileRegisters points to a different IDT 
shared by all APs.
   //
-  RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, 
FALSE);
+  RestoreVolatileRegisters 
(&CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, FALSE);
   InitializeApData (CpuMpData, ProcessorNumber, BistData, ApTopOfStack);
   ApStartupSignalBuffer = 
CpuMpData->CpuData[ProcessorNumber].StartupApSignal;
 } else {
@@ -798,10 +802,10 @@ ApWakeupFunction (
 // 1. AP is re-enabled after it's disabled, in either PEI or DXE phase.
 // 2. AP is initialized in DXE phase.
 // In either case, use the volatile registers value derived from BSP.
-// NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters 
points to a
+// NOTE: IDTR.BASE stored in 
CpuMpData->CpuData[BspNumber].VolatileRegisters points to a
 //   different IDT shared by all APs.
 //
-RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, 
FALSE);
+RestoreVolatileRegisters 
(&CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, FALSE);
   } else {
 if (CpuMpData->ApLoopMode == ApInHltLoop) {
   //
@@ -927,7 +931,7 @@ DxeApEntryPoint (
 AsmWriteMsr64 (MSR_IA32_EFER, EferMsr.Uint64);
   }

-  RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, FALSE);
+  RestoreVolatileRegisters 
(&CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, FALSE);
   InterlockedIncrement ((UINT32 *)&CpuMpData->FinishedCount);
   PlaceAPInMwaitLoopOrRunLoop (
 CpuMpData->ApLoopMode,
@@ -2151,7 +2155,11 @@ MpInitLibInitialize (
   CpuMpData->BackupBufferSize = ApResetVectorSizeBelow1Mb;
   CpuMpData->WakeupBuffer = (UINTN)-1;
   CpuMpData->CpuCount = 1;
-  CpuMpData->BspNumber= 0;
+  if (MpHandOff == NULL) {
+CpuMpData->BspNumber  = 0;
+  }else{
+CpuMpData->BspNumber  = GetBspNumber (MpHandOff);
+  }
   CpuMpData->WaitEvent= NULL;
   CpuMpData->SwitchBspFlag= FALSE;
   CpuMpData->CpuData  = (CPU_AP_DATA *)(CpuMpData + 1);
@@ -2186,11 +2194,11 @@ MpInitLibInitialize (
   // Don't pass BSP's TR to APs to avoid AP init failure.
   //
   VolatileRegisters.Tr = 0;
-  CopyMem (&CpuMpData->CpuData[0].VolatileRegisters, &VolatileRegisters, 
sizeof (VolatileRegisters));
+  CopyMem (&CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, 
&VolatileRegisters, sizeof (VolatileRegisters));
   //
   // Set BSP basic information
   //
-  InitializeApData (CpuMpData, 0, 0, CpuMpData->Buffer + ApStackSize);
+  InitializeApData (CpuMpData, CpuMpData->BspNumber, 0, CpuMpData->Buffer + 
ApStackSize * (CpuMpData->BspNumber+1));
   //
   // Save assembly code information
   //
@@ -2536,7 +2544,6 @@ SwitchBSPWorker (
   MSR_IA32_APIC_BASE_REGISTER  ApicBaseMsr;
   BOOLEAN  OldInterruptState;
   BOOLEAN  OldTimerInterruptState;
-
   //
   // Save and Disable Local APIC timer interrupt
   //
@@ -2615,7 +2622,12 @@ SwitchBSPWorker (
   SaveVolatileRegisters (&CpuMpData->BSPInfo.VolatileRegisters);
   AsmExchangeRole (&CpuMpData->BSPInfo, &CpuMpData->APInfo);
   RestoreVolatileRegisters (&CpuMpData->BSPInfo.VolatileRegisters, FALSE);
-
+  //
+  // Restore VolatileReg saved in CpuM