BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008
Set the SEV-ES reset stack address for an AP based on the processor number
instead of the APIC ID in case the APIC IDs are not zero-based and densely
packed/enumerated. This will ensure an AP reset stack address does not get
set outside of the AP reset stack memory allocation.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Acked-by: Ray Ni <ray.ni@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <24866de07d2a954dec71df70972f1851273020d8.1604685192.git.thomas.lendacky@amd.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008
The QemuFlashPtrWrite() flash services runtime uses the GHCB and VmgExit()
directly to perform the flash write when running as an SEV-ES guest. If an
interrupt arrives between VmgInit() and VmgExit(), the Dr7 read in the
interrupt handler will generate a #VC, which can overwrite information in
the GHCB that QemuFlashPtrWrite() has set. This has been seen with the
timer interrupt firing and the CpuExceptionHandlerLib library code,
UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/
Xcode5ExceptionHandlerAsm.nasm and
ExceptionHandlerAsm.nasm
reading the Dr7 register while QemuFlashPtrWrite() is using the GHCB. In
general, it is necessary to protect the GHCB whenever it is used, not just
in QemuFlashPtrWrite().
Disable interrupts around the usage of the GHCB by modifying the VmgInit()
and VmgDone() interfaces:
- VmgInit() will take an extra parameter that is a pointer to a BOOLEAN
that will hold the interrupt state at the time of invocation. VmgInit()
will get and save this interrupt state before updating the GHCB.
- VmgDone() will take an extra parameter that is used to indicate whether
interrupts are to be (re)enabled. Before exiting, VmgDone() will enable
interrupts if that is requested.
Fixes: 437eb3f7a8
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Acked-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <c326a4fd78253f784b42eb317589176cf7d8592a.1604685192.git.thomas.lendacky@amd.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008
All fields that are set in the GHCB should have their associated bit in
the GHCB ValidBitmap field set. Add support to set the bits for the
software exit information fields when performing a VMGEXIT (SwExitCode,
SwExitInfo1, SwExitInfo2).
Fixes: 20da7ca42a
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Acked-by: Ray Ni <ray.ni@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <6e11dd7e161bddeacc3fb4817467cef24510c31c.1604685192.git.thomas.lendacky@amd.com>
The AP reset vector stack allocation is only required if running as an
SEV-ES guest. Since the reset vector allocation is below 1MB in memory,
eliminate the requirement for bare-metal systems and non SEV-ES guests
to allocate the extra stack area, which can be large if the
PcdCpuMaxLogicalProcessorNumber value is large, and also remove the
CPU_STACK_ALIGNMENT alignment.
Fixes: 7b7508ad78 ("UefiCpuPkg: Allow AP booting under SEV-ES")
Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <21345cdbc906519558202b3851257ca07b9239ba.1600884239.git.thomas.lendacky@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
[lersek@redhat.com: supply missing space character after "PcdGet32"]
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2901
The DoDecrement variable in ApWakeupFunction () wasn't always being
initialized. Update the code to always fully initialize it.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <76a9f18992475b915e5f8457704676067210cacf.1597935198.git.thomas.lendacky@amd.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Tested-by: Liming Gao <liming.gao@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
Before UEFI transfers control to the OS, it must park the AP. This is
done using the AsmRelocateApLoop function to transition into 32-bit
non-paging mode. For an SEV-ES guest, a few additional things must be
done:
- AsmRelocateApLoop must be updated to support SEV-ES. This means
performing a VMGEXIT AP Reset Hold instead of an MWAIT or HLT loop.
- Since the AP must transition to real mode, a small routine is copied
to the WakeupBuffer area. Since the WakeupBuffer will be used by
the AP during OS booting, it must be placed in reserved memory.
Additionally, the AP stack must be located where it can be accessed
in real mode.
- Once the AP is in real mode it will transfer control to the
destination specified by the OS in the SEV-ES AP Jump Table. The
SEV-ES AP Jump Table address is saved by the hypervisor for the OS
using the GHCB VMGEXIT AP Jump Table exit code.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
Typically, an AP is booted using the INIT-SIPI-SIPI sequence. This
sequence is intercepted by the hypervisor, which sets the AP's registers
to the values requested by the sequence. At that point, the hypervisor can
start the AP, which will then begin execution at the appropriate location.
Under SEV-ES, AP booting presents some challenges since the hypervisor is
not allowed to alter the AP's register state. In this situation, we have
to distinguish between the AP's first boot and AP's subsequent boots.
First boot:
Once the AP's register state has been defined (which is before the guest
is first booted) it cannot be altered. Should the hypervisor attempt to
alter the register state, the change would be detected by the hardware
and the VMRUN instruction would fail. Given this, the first boot for the
AP is required to begin execution with this initial register state, which
is typically the reset vector. This prevents the BSP from directing the
AP startup location through the INIT-SIPI-SIPI sequence.
To work around this, the firmware will provide a build time reserved area
that can be used as the initial IP value. The hypervisor can extract this
location value by checking for the SEV-ES reset block GUID that must be
located 48-bytes from the end of the firmware. The format of the SEV-ES
reset block area is:
0x00 - 0x01 - SEV-ES Reset IP
0x02 - 0x03 - SEV-ES Reset CS Segment Base[31:16]
0x04 - 0x05 - Size of the SEV-ES reset block
0x06 - 0x15 - SEV-ES Reset Block GUID
(00f771de-1a7e-4fcb-890e-68c77e2fb44e)
The total size is 22 bytes. Any expansion to this block must be done
by adding new values before existing values.
The hypervisor will use the IP and CS values obtained from the SEV-ES
reset block to set as the AP's initial values. The CS Segment Base
represents the upper 16 bits of the CS segment base and must be left
shifted by 16 bits to form the complete CS segment base value.
Before booting the AP for the first time, the BSP must initialize the
SEV-ES reset area. This consists of programming a FAR JMP instruction
to the contents of a memory location that is also located in the SEV-ES
reset area. The BSP must program the IP and CS values for the FAR JMP
based on values drived from the INIT-SIPI-SIPI sequence.
Subsequent boots:
Again, the hypervisor cannot alter the AP register state, so a method is
required to take the AP out of halt state and redirect it to the desired
IP location. If it is determined that the AP is running in an SEV-ES
guest, then instead of calling CpuSleep(), a VMGEXIT is issued with the
AP Reset Hold exit code (0x80000004). The hypervisor will put the AP in
a halt state, waiting for an INIT-SIPI-SIPI sequence. Once the sequence
is recognized, the hypervisor will resume the AP. At this point the AP
must transition from the current 64-bit long mode down to 16-bit real
mode and begin executing at the derived location from the INIT-SIPI-SIPI
sequence.
Another change is around the area of obtaining the (x2)APIC ID during AP
startup. During AP startup, the AP can't take a #VC exception before the
AP has established a stack. However, the AP stack is set by using the
(x2)APIC ID, which is obtained through CPUID instructions. A CPUID
instruction will cause a #VC, so a different method must be used. The
GHCB protocol supports a method to obtain CPUID information from the
hypervisor through the GHCB MSR. This method does not require a stack,
so it is used to obtain the necessary CPUID information to determine the
(x2)APIC ID.
The new 16-bit protected mode GDT entry is used in order to transition
from 64-bit long mode down to 16-bit real mode.
A new assembler routine is created that takes the AP from 64-bit long mode
to 16-bit real mode. This is located under 1MB in memory and transitions
from 64-bit long mode to 32-bit compatibility mode to 16-bit protected
mode and finally 16-bit real mode.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
When starting APs in an SMP configuration, the AP needs to know if it is
running as an SEV-ES guest in order to assign a GHCB page.
Add a field to the CPU_MP_DATA structure that will indicate if SEV-ES is
enabled. This new field is set during MP library initialization with the
PCD value PcdSevEsIsEnabled. This flag can then be used to determine if
SEV-ES is enabled.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Refactor StandardSignatureIsAuthenticAMD into BaseUefiCpuLib from
separate copies in BaseXApicLib, BaseXApicX2ApicLib, and MpInitLib.
This allows for future use of StandarSignatureIsAuthinticAMD without
creating more instances in other modules.
This function allows IA32/X64 code to determine if it is running on an
AMD brand processor.
UefiCpuLib is already included directly or indirectly in all modified
modules. Complete move is made in this change.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Message-Id: <20200622131825.1352-4-Garrett.Kirkendall@amd.com>
Intel SDM introduces 6-levels for describing the CPU topology:
* Package
* Module
* Tile
* Die
* Core
* Thread
A PI spec ECR was submitted to enhance CPU_MP PPI/Protocol to
support returning such information through GetProcessorInfo().
An accordingly change was implemented and pushed to edk2-staging.
Now the PI spec has been published.
The patch is cherry-picked from edk2-staging to edk2.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2678
This patch fixes a file permission issue introduced by accident.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Leo Duran <leo.duran@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Eric Dong <eric.dong@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2683
In PEI phase, AP already been waked up through ApInitConfig,
so it can directly wake up it through change wakup buffer
instead of use ApInitReconfig flag. It can save some time.
Change code to only use ApInitReconfig flag in DXE phase
which must need to update the wake up buffer.
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2683
This patch fixes an assertion because AP can't find the CpuMpData.
When AP is waken up through Init-Sipi-Sipi, AP's IDT should
be restored to pre-allocated buffer so AP can get the CpuMpData
through the IDT base address.
Current code already has logic to handle this when CpuMpData->
InitFlag is ApInitConfig but misses the logic
when CpuMpData->InitFlag is ApInitReconfig.
This patch fixes this gap.
Reviewed-by: Ray Ni <ray.ni@intel.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2632
Both PEI and DXE instances of the MpInitLib are using PcdLib APIs, but
none of them list the dependency of the PcdLib in INF & header files.
This commit will explicitly add such dependency in .H and .INF files.
Test done:
Library level build pass for VS2015x86 tool chain
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2627
The commit will introduce a static PCD to specify the periodic interval
for checking the AP status when MP services StartupAllAPs() and
StartupThisAP() are being executed in a non-blocking manner. Or in other
words, specifies the interval for callback function CheckApsStatus().
The purpose is to provide the platform owners with the ability to choose
the proper interval value to trigger CheckApsStatus() according to:
A) The number of processors in the system;
B) How MP services (StartupAllAPs & StartupThisAP) being used.
Setting the PCD to a small value means the AP status check callback will
be triggered more frequently, it can benefit the performance for the case
when the BSP uses WaitForEvent() or uses CheckEvent() in a loop to wait
for AP(s) to complete the task, especially when the task can be finished
considerably fast on AP(s).
An example is within function CpuFeaturesInitialize() under
UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c,
where BSP will perform the same task with APs and requires all the
processors to finish the task before BSP proceeds to its next task.
Setting the PCD to a big value, on the other hand, can reduce the impact
on BSP by the time being consumed in CheckApsStatus(), especially when the
number of processors is huge so that the time consumed in CheckApsStatus()
is not negligible.
The type of the PCD is UINT32, which means the maximum possible interval
value can be set to:
4,294,967,295 microseconds = 4,295 seconds = 71.58 minutes = 1.19 hours
which should be sufficient for usage.
For least impact, the default value of the new PCD will be the same with
the current interval value. It will be set to 100,000 microseconds, which
is 100 milliseconds.
Unitest done:
A) OS boot successfully;
B) Use debug message to confirm the 'TriggerTime' parameter for the
'SetTimer' service is the same before & after this patch.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Brian J. Johnson <brian.johnson@hpe.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2556
This patch uses CPUID signature check to skip reading the PlatformId MSR,
which is not implemented on AMD processors.
The PlatformId is used for loading microcode patches, which is also not
supported and AMD-based platforms. To mitigate the PlatformId dependency,
PcdCpuMicrocodePatchAddress and PcdCpuMicrodePatchRegionSize must be set
to 0 (default value), in order to bypass microcode loading code paths.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Leo Duran <leo.duran@amd.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Commit c7c964b and dd01704 add header file for FIT table and update
MpInitLib to support FIT based microcode shadow operation. There are
comments that FIT is Intel specific specification instead of industry
standard, which should not be placed in EDK2 MdePkg and UefiCpuPkg.
So this patch adds a platform PPI for the microcode shadow logic, and
remove the FIT related code from EDK2.
The FIT based microcode shadow support will be implemented as a new
platform PEIM in IntelSiliconPkg in edk2-platforms.
This patch doesn't provide a DXE version shadow microcode protocol,
a platform which only uses DxeMpInitLib instance only supports PCD
based microcode shadowing.
A detailed design doc can be found here:
https://edk2.groups.io/g/devel/files/Designs/2020/0214/Support%20
the%202nd%20Microcode%20FV%20Flash%20Region.pdf
TEST: Tested on FIT enabled platform.
BZ: https://tianocore.acgmultimedia.com/show_bug.cgi?id=2449
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Siyuan Fu <siyuan.fu@intel.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2465
Commit 89164babec:
UefiCpuPkg/MpInitLib: don't shadow the microcode patch twice.
attempted to use 'MicrocodePatchRegionSize' and 'MicrocodePatchAddress'
fields to avoid loading the microcode patches data into memory again in
the DXE phase.
However, the CPU_MP_DATA structure has members with type 'UINTN' or
pointer before the microcode patch related fields. This may cause issues
when PEI and DXE are of different archs (e.g. PEI - IA32, DXE - x64),
since the microcode patch related fields will have different offsets in
the CPU_MP_DATA structure.
Commit 88bd066166:
UefiCpuPkg/MpInitLib: Relocate microcode patch fields in CPU_MP_DATA
tried to resolve the above-mentioned issue by relocating the fields
'MicrocodePatchRegionSize' and 'MicrocodePatchAddress' before members with
different size between different archs. But it failed to take the case of
pre-built binaries (e.g. FSP) into consideration.
Binaries can be built when the code base had a different version of the
CPU_MP_DATA structure definition. This may cause issues when accessing
these microcode patch related fields, since their offsets are different
(between PEI phase in the binaries and DXE phase in current code
implementation).
This commit will use the newly introduced EDKII microcode patch HOB
instead for the DXE phase to get the information of the loaded microcode
patches data done in the PEI phase. And the 'MicrocodePatchRegionSize' and
'MicrocodePatchAddress' fields in CPU_MP_DATA will not be used to pass
information between phases.
For pre-built binaries, they can be classified into 3 types with regard to
the time when they are being built:
A. Before commit 89164babec
(In other words, 'MicrocodePatchRegionSize' and 'MicrocodePatchAddress'
were not being used to skip microcode load in DXE)
For this case, the EDKII microcode patch HOB will not be produced. This
commit will load the microcode patches data again in DXE. Such behavior is
the same with the code base back then.
B. After commit 89164babec, before commit e1ed55738e
(In other words, 'MicrocodePatchRegionSize' and 'MicrocodePatchAddress'
being used to skip microcode load in DXE, but failed to work properly
between differnt archs.)
For this case, the EDKII microcode patch HOB will not be produced as well.
This commit will also load the microcode patches data again in DXE.
But since commit 89164babec failed to keep the detection and application
of microcode patches working properly in DXE after skipping the load, we
fall back to the origin behavior (that is to load the microcode patches
data again in DXE).
C. After commit e1ed55738e
(In other words, EDKII microcode patch HOB will be produced.)
For this case, it will have the same behavior with the BIOS built from
the current source codes.
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
This reverts commit 88bd066166.
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2465
Commit 88bd066166 relocates the 'MicrocodePatchAddress' and
'MicrocodePatchRegionSize' fields in structure CPU_MP_DATA to ensure that
they can be properly passed between different architectures.
However, such change is not backward compatible with the scenario like
pre-existing binaries such as FSP. These binaries are built when the code
base has a different version of the CPU_MP_DATA structure definition. This
may cause issues when accessing the 'MicrocodePatchAddress' and
'MicrocodePatchRegionSize' fields, since their offsets are different
(between PEI phase in the FSP binaries and DXE phase in current code
implementation).
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2498
Commit fd30b00707 updated the logic in function MicrocodeDetect() that
will directly use the CPUID and PlatformID information from the 'CpuData'
field in the CPU_MP_DATA structure, instead of collecting these
information for each processor via AsmCpuid() and AsmReadMsr64() calls
respectively.
At that moment, this approach worked fine for APs. Since:
a) When the APs are waken up for the 1st time (1st MpInitLibInitialize()
entry at PEI phase), the function InitializeApData() will be called for
each AP and the CPUID and PlatformID information will be collected.
b) During the 2nd entry of MpInitLibInitialize() at DXE phase, when the
APs are waken up again, the function InitializeApData() will not be
called, which means the CPUID and PlatformID information will not be
collected. However, the below logics in MicrocodeDetect() function:
CurrentRevision = GetCurrentMicrocodeSignature ();
IsBspCallIn = (ProcessorNumber == (UINTN)CpuMpData->BspNumber) ? TRUE : FALSE;
if (CurrentRevision != 0 && !IsBspCallIn) {
//
// Skip loading microcode if it has been loaded successfully
//
return;
}
will ensure that the microcode detection and application will be
skipped due to the fact that such process has already been done in the
PEI phase.
But after commit 396e791059, which removes the above skip loading logic,
the CPUID and PlatformID information on APs will be used upon the 2nd
entry of the MpInitLibInitialize(). But since the CPUID and PlatformID
information has not been collected, it will bring issue to the microcode
detection process.
This commit will update the logic in MicrocodeDetect() back to always
collecting the CPUID and PlatformID information explicitly.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2474
Previous commit d786a17232:
UefiCpuPkg/MpInitLib: Reduce the size when loading microcode patches
Removed the below assignments for the 'InitFlag' field of CPU_MP_DATA
structure in function MpInitLibInitialize() when APs are waken up to do
some initialize sync:
CpuMpData->InitFlag = ApInitReconfig;
...
CpuMpData->InitFlag = ApInitDone;
The above commit mistakenly assumed the 'InitFlag' field will have a value
of 'ApInitDone' when the APs have been successfully waken up before. And
since there is no explicit comparision for the 'InitFlag' field with the
'ApInitReconfig' value. The commit removed those assignments.
However, under some cases (e.g. when variable OldCpuMpData is not NULL,
which means function CollectProcessorCount() will not be called), removing
the above assignments will left the 'InitFlag' field being uninitialized
with a value of 0, which is a invalid value for the type of 'InitFlag'
(AP_INIT_STATE).
It may potentially cause the WakeUpAP() function to run some unnecessary
codes when the APs have been successfully waken up before:
if (CpuMpData->WakeUpByInitSipiSipi ||
CpuMpData->InitFlag != ApInitDone) {
ResetVectorRequired = TRUE;
AllocateResetVector (CpuMpData);
FillExchangeInfoData (CpuMpData);
SaveLocalApicTimerSetting (CpuMpData);
}
This commit will address the above-mentioned issue.
Test done:
* OS boot on a real platform with multi processors
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
The existing MpInitLib will shadow the microcode update patches from
flash to memory and this is done by searching microcode region specified
by PCD PcdCpuMicrocodePatchAddress and PcdCpuMicrocodePatchRegionSize.
This brings a limition to platform FW that all the microcode patches must
be placed in one continuous flash space.
This patch shadows microcode update according to FIT microcode entries if
it's present, otherwise it will fallback to original logic (by PCD).
A new featured PCD gUefiCpuPkgTokenSpaceGuid.PcdCpuShadowMicrocodeByFit
is added for enabling/disabling this support.
TEST: Tested on FIT enabled platform.
BZ: https://tianocore.acgmultimedia.com/show_bug.cgi?id=2449
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Siyuan Fu <siyuan.fu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
This patch updates the microcode loader to always perform a microcode
detect and load on both BSP and AP processor. This is to fix a potential
microcode revision mismatch issue in below situation:
1. Assume there are two microcode co-exists in flash: one production
version and one debug version microcode.
2. FIT loads production microcode to BSP and all AP.
3. UefiCpuPkg loader loads debug microcode to BSP, and skip the loading
on AP.
As a result, different microcode patches are loaded to BSP and AP, and
trigger microcode mismatch error during OS boot.
BZ link: https://bugzilla.tianocore.org/show_bug.cgi?id=2431
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Siyuan Fu <siyuan.fu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
This patch removes the unnecessary alignment check on microcode patch
TotalSize introduced by commit d786a172. The TotalSize has already been
checked with 1K alignment and MAX_ADDRESS in previous code as below:
if ( (UINTN)MicrocodeEntryPoint > (MAX_ADDRESS - TotalSize) ||
((UINTN)MicrocodeEntryPoint + TotalSize) > MicrocodeEnd ||
(DataSize & 0x3) != 0 ||
(TotalSize & (SIZE_1KB - 1)) != 0 ||
TotalSize < DataSize
) {
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Siyuan Fu <siyuan.fu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Previous commits have introduced below fields in structure CPU_AP_DATA:
UINT32 ProcessorSignature;
UINT8 PlatformId;
UINT64 MicrocodeEntryAddr;
which store the information of:
A. CPUID
B. Platform ID
C. Detected microcode patch entry address (including the microcode patch
header)
for each processor within system.
Therefore, the below fields in structure CPU_MP_DATA:
UINT32 ProcessorSignature;
UINT32 ProcessorFlags;
UINT64 MicrocodeDataAddress;
UINT32 MicrocodeRevision;
which store the BSP's information of:
A. CPUID
B. Platform ID
C. The address and revision of detected microcode patch
are redundant and can be removed.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
The below 2 microcode patch related fields in structure CPU_MP_DATA:
UINT64 MicrocodePatchAddress;
UINT64 MicrocodePatchRegionSize;
They will be passed from PEI phase and be reused DXE phase.
Previously, these 2 fields were placed after some fields with type
'UINTN', this will lead to different field offset in different
architecture for them.
This commit will move them before the fields with different size in
different architecture to ensure they can be properly used in DXE phase.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2430
This commit will update the MpInitLib to:
A. Collect the base address and size information after microcode patches
being loaded into memory;
B. Collect the detected microcode patch for each processor within system;
C. Based on the collected information, produce the EDKII microcode patch
HOB.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2429
This commit will attempt to reduce the copy size when loading the
microcode patches data from flash into memory.
Such optimization is done by a pre-process of the microcode patch headers
(on flash). A microcode patch will be loaded into memory only when the
below 3 criteria are met:
A. With a microcode patch header (which means the data is not padding data
between microcode patches);
B. The 'ProcessorSignature' & 'ProcessorFlags' fields in the header match
at least one processor within system;
C. If the Extended Signature Table exists in a microcode patch, the
'ProcessorSignature' & 'ProcessorFlag' fields in the table entries
match at least one processor within system.
Criterion B and C will require all the processors to be woken up once to
collect their CPUID and Platform ID information. Hence, this commit will
move the copy, detect and apply of microcode patch on BSP and APs after
all the processors have been woken up.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2429
This commit will collect the CPUID and Platform ID information for each
processor within system. They will be stored in the CPU_AP_DATA structure.
These information will be used in the next commit to decide whether a
microcode patch will be loaded into memory.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
In MpLib.c, remove the white space on a new line.
In PageTbl.c and PiSmmCpuDxeSmm.h, update the comment style.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
MpInitLib sets X2ApicEnable in two places.
1. CollectProcessorCount()
This function is called when MpInitLibInitialize() hasn't been
called before.
It sets X2ApicEnable and later in the same function it configures
all CPUs to operate in X2 APIC mode.
2. MpInitLibInitialize()
The X2ApicEnable setting happens when this function is called in
second time. But after that setting, no code consumes that flag.
With the above analysis and with the purpose of simplifying the code,
the X2ApicEnable in #1 is changed to local variable and the #2 can be
changed to remove the setting of X2ApicEnable.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Today's logic sets X2ApicEnable flag in each AP's initialization
path when InitFlag == ApInitConfig.
Since all CPUs update the same global data, a spin-lock is used
to avoid modifications from multiple CPUs happen at the same time.
The spin-lock causes two problems:
1. Potential performance downgrade.
2. Undefined behavior when improper timer lib is used.
For example we saw certain platforms used AcpiTimerLib from
PcAtChipsetPkg and that library depends on retrieving PeiServices
from idtr. But in fact AP's (idtr - 4) doesn't point to
PeiServices.
The patch simplifies the code to let BSP set the X2ApicEnable flag so
the spin-lock acquisition from AP is not needed any more.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
- If a platform boots such that the boot CPU count is smaller than
PcdCpuMaxLogicalProcessorNumber, then the platform cannot use the "fast
AP detection" logic added in commit 6e1987f19a. (Which has been
documented as a subset of use case (2) in the previous patch.)
Said logic depends on the boot CPU count being equal to
PcdCpuMaxLogicalProcessorNumber. If the equality does not hold, the
platform either has to wait too long, or risk missing APs due to an
early timeout.
- The platform may not be able to use the variant added in commit
0594ec417c either. (Which has been documented as use case (1) in the
previous patch.)
See commit 861218740d. When OVMF runs on QEMU/KVM, APs may check in
with the BSP in arbitrary order, plus the individual AP may take
arbitrarily long to check-in. If "NumApsExecuting" falls to zero
mid-enumeration, APs will be missed.
Allow platforms to specify the exact boot CPU count, independently of
PcdCpuMaxLogicalProcessorNumber. In this mode, the BSP waits for all APs
to check-in regardless of timeout. If at least one AP fails to check-in,
then the AP enumeration hangs forever. That is the desired behavior when
the exact boot CPU count is known in advance. (A hung boot is better than
an AP checking-in after timeout, and executing code from released
storage.)
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1515
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Before adding another AP enumeration mode, clarify the documentation on
the current logic. No functional changes.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1515
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2008
MpInitLib is the library that's responsible to wake up APs to provide
MP PPI and Protocol services.
The patch synchronizes BSP's CR4.LA57 to each AP's CR4.LA57.
Without this change, AP may enter to GP fault when BSP's 5-level page
table is set to AP during AP wakes up.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1982
MpInitLibInitialize in MpLib.c will be invoked on both PEI and DXE
CPU code, MicrocodeDetect would be performed twice and copy
Microcode from flash to memory twice as well, which consider as
duplicate work to lead longer boot time.
This patch just use microcode memory copied in PEI phase if exist.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1973
Add new MpInitLibStartupAllCPUs API uses to start all processors
at the same time.
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1934
0x0 MicrocodeBegin MicrocodeEntry MicrocodeEnd 0xffffffff
|--------------|---------------|---------------|---------------|
valid TotalSize
TotalSize is only valid between 0 and (MicrocodeEnd - MicrocodeEntry).
So add '(UINTN)MicrocodeEntryPoint > (MAX_ADDRESS - TotalSize)' before
'((UINTN)MicrocodeEntryPoint + TotalSize) > MicrocodeEnd' to make sure
((UINTN)MicrocodeEntryPoint + TotalSize) wouldn't overflow.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
The patch fixes the bug that the memory under 1MB is modified by
firmware in S3 boot.
Root cause is a racing condition in MpInitLib:
1. BSP: WakeUpByInitSipiSipi is set by NotifyOnS3SmmInitDonePpi()
2. BSP: WakeUpAP() wakes all APs to run certain procedure.
2.1. AllocateResetVector() uses <1MB memory for wake up vector.
2.1. FillExchangeInfoData() resets NumApsExecuting to 0.
2.2. WaitApWakeup() waits AP to clear WAKEUP_AP_SIGNAL.
3. AP: ApWakeupFunction() clears WAKEUP_AP_SIGNAL to inform BSP.
5. BSP: FreeResetVector() restores the <1MB memory
4. AP: ApWakeupFunction() calls the certain procedure.
4.1. NumApsExecuting is decreased.
#4.1 happens after the 1MB memory is restored so the result is
memory below 1MB is changed by #4.1
It happens only when the AP executes procedure a bit longer.
AP returns back to ApWakeupFunction() from procedure after
BSP restores the <1MB memory.
Since NumApsExecuting is only used when InitFlag == ApInitConfig
for counting the processor count.
The patch moves the NumApsExecuting decrease to the path when
InitFlag == ApInitConfig.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Cc: Nandagopal Sathyanarayanan <nandagopal.sathyanarayanan@intel.com>
NumApsExecuting is only used when InitFlag == ApInitConfig for
counting the processor count.
The patch changes Ia32 version of waking up vector assembly code
to align to x64 version of waking up vector assembly code.
After the change both versions of waking up vector increase
NumApsExecuting when InitFlag == ApInitConfig.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
V2 changes:
Update the commit message and comments in the code.
When waking vector buffer allocated by CpuDxe is tested by MemTest86
in MP mode, an error is reported because the same range of memory is
modified by both CpuDxe driver and MemTest86.
The waking vector buffer is not expected to be tested by MemTest86 if
it is allocated out because MemTest86 only tests free memory. But
current CpuDxe driver "borrows" buffer instead of allocate buffer for
waking vector buffer (through allocate & free to get the buffer
pointer, backup the buffer data before using it and restore it after
using). With this implementation, if the buffer borrowed is not used
by any other drivers, MemTest86 tool will treat it as free memory
and test it.
In order to fix the above issue, CpuDxe changes to allocate the
buffer below 1M instead of borrowing it. But directly allocating
memory below 1MB causes LegacyBios driver fails to start. LegacyBios
driver allocates memory range from
"0xA0000 - PcdEbdaReservedMemorySize" to 0xA0000 as Ebda Reserved
Memory. The minimum value for "0xA0000 - PcdEbdaReservedMemorySize"
is 0x88000. If LegacyBios driver allocate this range failed, it
asserts.
LegacyBios also reserves range from 0x60000 to
"0x60000 + PcdOpromReservedMemorySize", it will be used as Oprom
Reserve Memory. The maximum value for "0x60000 +
PcdOpromReservedMemorySize" is 0x88000. LegacyBios driver tries to
allocate these range page(4K size) by page. It just reports warning
message if some pages are already allocated by others.
Base on above investigation, one page in range 0x60000 ~ 0x88000 can
be used as the waking vector buffer.
LegacyBios driver only reports warning when page allocation in range
[0x60000, 0x88000) fails. This library is consumed by CpuDxe driver
to produce CPU Arch protocol. LagacyBios driver depends on CPU Arch
protocol which guarantees below allocation runs earlier than
LegacyBios driver.
Cc: Ray Ni <ray.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1020
Should make sure the TotalSize of Microcode is aligned with 4 bytes
before calling CalculateSum32 function.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1020
The Microcode region indicated by MicrocodePatchAddress PCD may contain
more than one Microcode entry. We should save InCompleteCheckSum32 value
for each payload. Move the logic for calculate InCompleteCheckSum32 from
the outsize of the do-while loop to the inside.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1020
The following Microcode payload format is define in SDM spec.
Payload: |MicrocodeHeader|MicrocodeBinary|ExtendedHeader|ExtendedTable|.
When we verify the CheckSum32 with ExtendedTable, we should use the fields
of ExtendedTable to replace corresponding fields in MicrocodeHeader,
and then calculate the CheckSum32 with MicrocodeHeader+MicrocodeBinary.
This patch already verified on ICL platform.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Zhang Chao B <chao.b.zhang@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
In some special cases, after BSP sends Init-sipi-sipi signal
AP needs more time to start the Ap procedure. In this case
BSP may think AP has finished its task but in fact AP hasn't began
yet.
Rollback former change to keep the status which only be used
when AP really finished task.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Index is initialized to MAX_UINT16 as default failure value, which
is what the ASSERT is supposed to test for. The ASSERT condition
however can never return FALSE for INT16 != int, as due to
Integer Promotion[1], Index is converted to int, which can never
result in -1.
Furthermore, Index is used as a for loop index variable inbetween its
initialization and the ASSERT, so the value is unconditionally
overwritten too.
Fix the ASSERT check to compare Index to its upper boundary, which it
will be equal to if the loop was not broken out of on success.
[1] ISO/IEC 9899:2011, 6.5.9.4
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
The conflict issues are introduced by Stack Guard feature enabled for
PEI.
The first is CR0 which should be restored after CR3 and CR4.
Another is TR which should not be passed from BSP to AP during init
phase.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: "Ware, Ryan R" <ryan.r.ware@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Tom Lendacky
Kirkendall, Garrett
Ray Ni
Leo Duran
Dong, Eric
Hao A Wu
GuoMinJ
Siyuan Fu
Shenglei Zhang
Laszlo Ersek
Zhichao Gao
Michael D Kinney
Chen A Chen
Marvin H?user
Jian J Wang