V3 changes include:
1. Add comments for some complicate logic.
2. Use global variable instead of internal function to return string for 
register type
   and dependence type.
3. Verify the solution with internal server platform(2 socket/56 Core/112 
thread), set
   MSRs costs time change from 2849ms to 87ms.


V2 changes include:
1. Include the change for CpuCommonFeaturesLib which used to set MSR base on 
its scope info.
2. Include the change for CpuS3DataDxe driver which also handle the AcpiCpuData 
data.
3. Update code base on feedback for V1 changes.

V1 changes include:
In a system which has multiple cores, current set register value task costs 
huge times.
After investigation, current set MSR task costs most of the times. Current 
logic uses SpinLock to let set MSR task as an single thread task for all cores. 
Because MSR has scope attribute which may cause GP fault if multiple APs set 
MSR at the same time, current logic use an easiest solution (use SpinLock) to 
avoid this issue, but it will cost huge times.

In order to fix this performance issue, new solution will set MSRs base on 
their scope attribute. After this, the SpinLock will not needed. Without 
SpinLock, new issue raised which is caused by MSR dependence. For example, MSR 
A depends on MSR B which means MSR A must been set after MSR B has been set. 
Also MSR B is package scope level and MSR A is thread scope level. If system 
has multiple threads, Thread 1 needs to set the thread level MSRs and thread 2 
needs to set thread and package level MSRs. Set MSRs task for thread 1 and 
thread 2 like below:

            Thread 1                 Thread 2
MSR B          N                        Y
MSR A          Y                        Y

If driver don't control execute MSR order, for thread 1, it will execute MSR A 
first, but at this time, MSR B not been executed yet by thread 2. system may 
trig exception at this time.

In order to fix the above issue, driver introduces semaphore logic to control 
the MSR execute sequence. For the above case, a semaphore will be add between 
MSR A and B for all threads. Semaphore has scope info for it. The possible 
scope value is core or package.
For each thread, when it meets a semaphore during it set registers, it will 1) 
release semaphore (+1) for each threads in this core or package(based on the 
scope info for this
semaphore) 2) acquire semaphore (-1) for all the threads in this core or 
package(based on the scope info for this semaphore). With these two steps, 
driver can control MSR sequence. Sample code logic like below:

  //
  // First increase semaphore count by 1 for processors in this package.
  //
  for (ProcessorIndex = 0; ProcessorIndex < PackageThreadsCount ; 
ProcessorIndex ++) {
    LibReleaseSemaphore ((UINT32 *) &SemaphorePtr[PackageOffset + 
ProcessorIndex]);
  }
  //
  // Second, check whether the count has reach the check number.
  //
  for (ProcessorIndex = 0; ProcessorIndex < ValidApCount; ProcessorIndex ++) {
    LibWaitForSemaphore (&SemaphorePtr[ApOffset]);
  }

Platform Requirement:
1. This change requires register MSR setting base on MSR scope info. If still 
register MSR
   for all threads, exception may raised.

Known limitation:
1. Current CpuFeatures driver supports DXE instance and PEI instance. But 
semaphore logic
   requires Aps execute in async mode which is not supported by PEI driver. So 
CpuFeature
   PEI instance not works after this change. We plan to support async mode for 
PEI in phase
   2 for this task.
2. Current execute MSR task code in duplicated in PiSmmCpuDxeSmm driver and 
   RegisterCpuFeaturesLib library because the schedule limitation. Will merge 
the code to 
   RegisterCpuFeaturesLib and export as an API in phase 2 for this task.

Cc: Ruiyu Ni <ruiyu...@intel.com>
Cc: Laszlo Ersek <ler...@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.d...@intel.com>


Eric Dong (6):
  UefiCpuPkg/Include/AcpiCpuData.h: Add Semaphore related Information.
  UefiCpuPkg/RegisterCpuFeaturesLib.h: Add new dependence types.
  UefiCpuPkg/RegisterCpuFeaturesLib: Add logic to support semaphore
    type.
  UefiCpuPkg/PiSmmCpuDxeSmm: Add logic to support semaphore type.
  UefiCpuPkg/CpuS3DataDxe: Keep old data if value already existed.
  UefiCpuPkg/CpuCommonFeaturesLib: Register MSR base on scope Info.

 UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c                |   2 +
 UefiCpuPkg/Include/AcpiCpuData.h                   |  59 ++-
 .../Include/Library/RegisterCpuFeaturesLib.h       |  21 +-
 UefiCpuPkg/Library/CpuCommonFeaturesLib/C1e.c      |   8 +
 UefiCpuPkg/Library/CpuCommonFeaturesLib/Eist.c     |  12 +
 .../Library/CpuCommonFeaturesLib/ExecuteDisable.c  |  10 +
 .../Library/CpuCommonFeaturesLib/FastStrings.c     |  12 +
 .../Library/CpuCommonFeaturesLib/FeatureControl.c  |  38 ++
 .../CpuCommonFeaturesLib/LimitCpuIdMaxval.c        |  14 +
 .../Library/CpuCommonFeaturesLib/MachineCheck.c    |  38 ++
 .../Library/CpuCommonFeaturesLib/MonitorMwait.c    |  15 +
 .../Library/CpuCommonFeaturesLib/PendingBreak.c    |  11 +
 UefiCpuPkg/Library/CpuCommonFeaturesLib/Ppin.c     |  11 +
 .../Library/CpuCommonFeaturesLib/ProcTrace.c       |  11 +
 UefiCpuPkg/Library/CpuCommonFeaturesLib/X2Apic.c   |  10 +
 .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 392 ++++++++++++++---
 .../DxeRegisterCpuFeaturesLib.c                    |  71 ++-
 .../DxeRegisterCpuFeaturesLib.inf                  |   2 +
 .../PeiRegisterCpuFeaturesLib.c                    |  55 ++-
 .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h   |  61 ++-
 .../RegisterCpuFeaturesLib.c                       | 484 ++++++++++++++++++---
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c                  | 385 ++++++++++------
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c              |   3 -
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h         |   3 +-
 24 files changed, 1447 insertions(+), 281 deletions(-)

-- 
2.15.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to