Fix various typos in comments and documentation.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Antoine Coeur <coeur@gmx.fr>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
Message-Id: <20200207010831.9046-78-philmd@redhat.com>
Fix few typos in comments and documentation.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Antoine Coeur <coeur@gmx.fr>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
Message-Id: <20200207010831.9046-77-philmd@redhat.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>
In commit 4eee0cc7cc ("UefiCpuPkg/PiSmmCpu: Enable 5 level paging when
CPU supports", 2019-07-12), the Page Directory Entry setting was regressed
(corrupted) when splitting a 2MB page to 512 4KB pages, in the
InitPaging() function.
Consider the following hunk, displayed with
$ git show --function-context --ignore-space-change 4eee0cc7cc
> //
> // If it is 2M page, check IsAddressSplit()
> //
> if (((*Pd & IA32_PG_PS) != 0) && IsAddressSplit (Address)) {
> //
> // Based on current page table, create 4KB page table for split area.
> //
> ASSERT (Address == (*Pd & PHYSICAL_ADDRESS_MASK));
>
> Pt = AllocatePageTableMemory (1);
> ASSERT (Pt != NULL);
>
> + *Pd = (UINTN) Pt | IA32_PG_RW | IA32_PG_P;
> +
> // Split it
> - for (PtIndex = 0; PtIndex < SIZE_4KB / sizeof(*Pt); PtIndex++) {
> - Pt[PtIndex] = Address + ((PtIndex << 12) | mAddressEncMask | PAGE_ATTRIBUTE_BITS);
> + for (PtIndex = 0; PtIndex < SIZE_4KB / sizeof(*Pt); PtIndex++, Pt++) {
> + *Pt = Address + ((PtIndex << 12) | mAddressEncMask | PAGE_ATTRIBUTE_BITS);
> } // end for PT
> *Pd = (UINT64)(UINTN)Pt | mAddressEncMask | PAGE_ATTRIBUTE_BITS;
> } // end if IsAddressSplit
> } // end for PD
First, the new assignment to the Page Directory Entry (*Pd) is
superfluous. That's because (a) we set (*Pd) after the Page Table Entry
loop anyway, and (b) here we do not attempt to access the memory starting
at "Address" (which is mapped by the original value of the Page Directory
Entry).
Second, appending "Pt++" to the incrementing expression of the PTE loop is
a bug. It causes "Pt" to point *right past* the just-allocated Page Table,
once we finish the loop. But the PDE assignment that immediately follows
the loop assumes that "Pt" still points to the *start* of the new Page
Table.
The result is that the originally mapped 2MB page disappears from the
processor's view. The PDE now points to a "Page Table" that is filled with
garbage. The random entries in that "Page Table" will cause some virtual
addresses in the original 2MB area to fault. Other virtual addresses in
the same range will no longer have a 1:1 physical mapping, but be
scattered over random physical page frames.
The second phase of the InitPaging() function ("Go through page table and
set several page table entries to absent or execute-disable") already
manipulates entries in wrong Page Tables, for such PDEs that got split in
the first phase.
This issue has been caught as follows:
- OVMF is started with 2001 MB of guest RAM.
- This places the main SMRAM window at 0x7C10_1000.
- The SMRAM management in the SMM Core links this SMRAM window into
"mSmmMemoryMap", with a FREE_PAGE_LIST record placed at the start of the
area.
- At "SMM Ready To Lock" time, PiSmmCpuDxeSmm calls InitPaging(). The
first phase (quoted above) decides to split the 2MB page at 0x7C00_0000
into 512 4KB pages, and corrupts the PDE. The new Page Table is
allocated at 0x7CE0_D000, but the PDE is set to 0x7CE0_E000 (plus
attributes 0x67).
- Due to the corrupted PDE, the second phase of InitPaging() already looks
up the PTE for Address=0x7C10_1000 in the wrong place. The second phase
goes on to mark bogus PTEs as "NX".
- PiSmmCpuDxeSmm calls SetMemMapAttributes(). Address 0x7C10_1000 is at
the base of the SMRAM window, therefore it happens to be listed in the
SMRAM map as an EfiConventionalMemory region. SetMemMapAttributes()
calls SmmSetMemoryAttributes() to mark the region as XP. However,
GetPageTableEntry() in ConvertMemoryPageAttributes() fails -- address
0x7C10_1000 is no longer mapped by anything! -- and so the attribute
setting fails with RETURN_UNSUPPORTED. This error goes unnoticed, as
SetMemMapAttributes() ignores the return value of
SmmSetMemoryAttributes().
- When SetMemMapAttributes() reaches another entry in the SMRAM map,
ConvertMemoryPageAttributes() decides it needs to split a 2MB page, and
calls SplitPage().
- SplitPage() calls AllocatePageTableMemory() for the new Page Table,
which takes us to InternalAllocMaxAddress() in the SMM Core.
- The SMM core attempts to read the FREE_PAGE_LIST record at 0x7C10_1000.
Because this virtual address is no longer mapped, the firmware crashes
in InternalAllocMaxAddress(), when accessing (Pages->NumberOfPages).
Remove the useless assignment to (*Pd) from before the loop. Revert the
loop incrementing and the PTE assignment to the known good version.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1789335
Fixes: 4eee0cc7cc
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.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>
Add code to set SMXE in CR4 in the SmxInitialize flow when SMX is enabled.
Signed-off-by: Jason Voelz <jason.voelz@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.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>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2434
Current code use below loops to enumerate the CPUs:
for (Index = mMaxNumberOfCpus; Index-- > 0;) {
it has no issue but not easy for the developers to read the code.
Update above code to below style,
for (Index = 0; Index < mMaxNumberOfCpus; Index++) {
It make the developers easy to read and consistent with other
similar cases in this driver.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
This reverts commit 123b720eeb.
The commit message for commit 123b720eeb is not correct.
Cc: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2388
Token is new introduced by MM MP Protocol. Current logic allocate Token
every time when need to use it. The logic caused SMI latency raised to
very high. Update logic to allocate Token buffer at driver's entry point.
Later use the token from the allocated token buffer. Only when all the
buffer have been used, then need to allocate new buffer.
Former change (9caaa79dd7) missed
PROCEDURE_TOKEN part, this change covers it.
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-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=2430
This commit will add the definitions for EDKII microcode patch HOB.
The intention of adding this HOB is to provide a scheme to store the below
information:
A. The base address and size of the microcode patches that are being
loaded (from flash) into memory;
B. The information of detected microcode patch for each processor within
the system.
The producer of the HOB will be the UefiCpuPkg/MpInitLib (where the load,
detect and apply of the microcode happen). The consumer of the HOB can be
modules that want to detect/apply the microcode patch by themselves again
later during the boot flow.
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=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>
The size for the array of mSmmMpSyncData->CpuData[] is 0 ~
mMaxNumberOfCpus -1. But current code may use
mSmmMpSyncData->CpuData[mMaxNumberOfCpus].
This patch fixed this issue.
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2268
In current implementation, when check whether APs called by StartUpAllAPs
or StartUpThisAp, it checks the Tokens value used by other APs. Also the AP
will update the Token value for itself if its task finished. In this
case, the potential race condition issues happens for the tokens.
Because of this, system may trig ASSERT during cycling test.
This change enhance the code logic, add new attributes for the token to
remove the reference for the tokens belongs to other APs.
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2388
Token is new introduced by MM MP Protocol. Current logic allocate Token
every time when need to use it. The logic caused SMI latency raised to
very high. Update logic to allocate Token buffer at driver's entry point.
Later use the token from the allocated token buffer. Only when all the
buffer have been used, then need to allocate new buffer.
Reviewed-by: Ray Ni <ray.ni@intel.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.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>
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>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2329
XD (ExecutionDisable) feature, when turned on, allows page table
entry BIT63 set to 1 indicating the memory pointed by the page table
is disallowed to execute.
DxeIpl::CreateIdentityMappingPageTables() enables the XD when CPU
supports it.
Later DxeCore modifies the page table to set the BIT63 to protect
the stack/heap to disallow code execution in stack/heap.
UefiCpuPkg/CpuCommonFeaturesLib enables/disables the XD feature
according to PcdCpuFeaturesSetting.
When XD is disabled, GP fault is generated immediately because some
page entries have BIT63 set.
To fix this issue, this patch removes the XD feature logic from
UefiCpuPkg/CpuCommonFeaturesLib so the XD feature is only taken
care of by DxeIpl.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
https://bugzilla.tianocore.org/show_bug.cgi?id=2315
Add YAML file to the package directory with the
configuration of the checks to perform during a
CI build.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.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>
https://bugzilla.tianocore.org/show_bug.cgi?id=2255
Update UefiCpuPkg.dsc to guarantee all libraries and
modules are always built. Add the following components.
* UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.inf
* UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf
ResetVector.inf is a binary INF, so no source builds are
triggered from adding this line. However, a build with
this component does verify the contents of the INF file.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.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>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1039
Current implementation not checks system mode before using
PAGE_TABLE_LIB_PAGING_CONTEXT.ContextData.X64 or
PAGE_TABLE_LIB_PAGING_CONTEXT.ContextData.Ia32. This patch check the
mode before using the correct one.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2150
v4:
The v3 posting didn't do what it promised to do, so do it now for real.
V3 changes:
change to mov instruction (non locking instuction) instead
of xchg to simplify design.
V2 changes:
Add xchg 16 bit instructions to handle sgdt and sidt base
63:48 bits and 47:32 bits.
Add comment to explain why xchg 64bit isnt being used
Split lock happens when a locking instruction is used on mis-aligned data
that crosses two cachelines. If close source platform enables Alignment
Check Exception(#AC), They can hit a double fault due to split lock being
in CpuExceptionHandlerLib.
sigt and sgdt saves 10 bytes to memory, 8 bytes is base and 2 bytes is limit.
The data is mis-aligned, can cross two cacheline, and a xchg
instruction(locking instuction) is being utilize.
Signed-off-by: John E Lofgren <john.e.lofgren@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Due to needs a tackling the deficiency of the AP API, it's necessary to
ensure that in non-blocking mode previous AP executed command is
finished before starting new one.
To remedy above:
1) execute AcquireSpinLock instead AcquireSpinLockOrFail - this will
ensure time "window" to eliminate potential race condition between
BSP and AP spinLock release in non-blocking mode.
This also will eliminate possibility to start executing new AP
function before last is finished.
2) remove returns EFI_STATUS - EFI_NOT_READY - in new scenario returned
status is not necessary to caller.
Signed-off-by: Damian Nikodem <damian.nikodem@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Krzysztof Rusocki <krzysztof.rusocki@intel.com>
PcdCpuSmmRestrictedMemoryAccess is introduced to replace
PcdCpuSmmStaticPageTable.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Today's behavior is to enable 5l paging when CPU supports it
(CPUID[7,0].ECX.BIT[16] is set).
The patch changes the behavior to enable 5l paging when two
conditions are both met:
1. CPU supports it;
2. The max physical address bits is bigger than 48.
Because 4-level paging can support to address physical address up to
2^48 - 1, there is no need to enable 5-level paging with max
physical address bits <= 48.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2153
Current logic will skip searching EFI_PEI_CORE_FV_LOCATION_PPI when the
PPI in PpiList having EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST flag,
but platform may pass single PPI in PpiList that should be supported.
Changed the logic to verify PpiList first before checking
EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST flag.
Test: Verified both single EFI_PEI_CORE_FV_LOCATION_PPI and multiple
PPIs in PpiList cases and both can boot with the PeiCore
specified by EFI_PEI_CORE_FV_LOCATION_PPI.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
There are three PCDs that may impact the behavior of each other in
SMM environment:
PcdCpuSmmProfileEnable
PcdHeapGuardPropertyMask in MdeModulePkg
PcdCpuSmmRestrictedMemoryAccess
The patch updates the comments in DEC file to document it.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Today's behavior is to always restrict access to non-SMRAM regardless
the value of PcdCpuSmmRestrictedMemoryAccess.
Because RAS components require to access all non-SMRAM memory, the
patch changes the code logic to honor PcdCpuSmmRestrictedMemoryAccess
so that only when the PCD is true, the restriction takes affect and
page table memory is also protected.
Because IA32 build doesn't reference this PCD, such restriction
always takes affect in IA32 build.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
The patch changes PiSmmCpu driver to consume PCD
PcdCpuSmmRestrictedMemoryAccess.
Because the behavior controlled by PcdCpuSmmStaticPageTable in
original code is not changed after switching to
PcdCpuSmmRestrictedMemoryAccess.
The functionality is not impacted by this patch.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
The patch adds a new X64 only PCD PcdCpuSmmRestrictedMemoryAccess.
The PCD indicates access to non-SMRAM memory is restricted to
reserved, runtime and ACPI NVS type after SmmReadyToLock.
MMIO access is always allowed regardless of the value of this PCD.
Loose of such restriction is only required by RAS components in X64
platforms.
The PCD value is considered as constantly TRUE in IA32 platforms.
When the PCD value is TRUE, page table is initialized to cover all
memory spaces and the memory occupied by page table is protected by
page table itself as read-only.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2136
SecPlatformMain is a platform hook function which let platform does
some update. Some platform may adjust SecCoreData->PeiTemporaryRamBase
which caused former saved AllSecPpiList variable invalid.
This patch update the logic to get AllSecPpiList after SecPlatformMain.
If SecPlatformMain() returns no platform-specific PPI list, then there
is nothing to merge, so we don't need "AllSecPpiList" at all.
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1909
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Amy Chan <amy.chan@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Signed-off-by: Donald Kuo <donald.kuo@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2040
Below code is current implementation:
if (MsrRegister[ProcessorNumber].Bits.Lock == 0) {
CPU_REGISTER_TABLE_WRITE_FIELD (
ProcessorNumber,
Msr,
MSR_IA32_FEATURE_CONTROL,
MSR_IA32_FEATURE_CONTROL_REGISTER,
Bits.Lock,
1
);
}
1. In first normal boot, the Bits.Lock is 0, 1 will be added
into the register table and then will set to the MSR.
2. Trig warm reboot, MSR value preserves. After normal boot phase,
the Bits.Lock is 1, so it will not be added into the register
table during the warm reboot phase.
3. Trig S3 then resume, the Bits.Lock change to 0 and Bits.Lock is
not added in register table, so it's still 0 after resume. This
is not an expect behavior. The expect value is the value should
always 1 after booting or resuming from S3.
The root cause for this issue is
1. driver bases on current value to insert the "set value action" to
the register table.
2. Some MSRs may reserve their value during warm reboot.
The solution for this issue is using new added macros for the MSRs which
preserve value during warm reboot.
Signed-off-by: Eric Dong <eric.dong@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=2040
Supports new logic which test current value before write new value.
Only write new value when current value not same as new value.
Signed-off-by: Eric Dong <eric.dong@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=2040
Supports new logic which test current value before write new value.
Only write new value when current value not same as new value.
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=2040
Add below new micros which test the current value before write the new
value. Only write new value when current value not same as new value.
CPU_REGISTER_TABLE_TEST_THEN_WRITE32
CPU_REGISTER_TABLE_TEST_THEN_WRITE64
CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD
Also add below API:
CpuRegisterTableTestThenWrite
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Reclaim may free page table pages that are required to handle current page
fault. This causes a page leak, and, after sufficent number of specific
page fault+reclaim pairs, we run out of reclaimable pages and hit:
ASSERT (MinAcc != (UINT64)-1);
To remedy, prevent pages essential to handling current page fault:
(1) from being considered as reclaim candidates (first reclaim phase)
(2) from being freed as part of "branch cleanup" (second reclaim phase)
Signed-off-by: Damian Nikodem <damian.nikodem@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Krzysztof Rusocki <krzysztof.rusocki@intel.com>
LeafFunction needs to be described in comments.
https://bugzilla.tianocore.org/show_bug.cgi?id=2052
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>
1. Update CPUStatus to CpuStatus in comments to align comments
with code.
2. Add "OUT" attribute for "ProcedureArguments" parameter in function
header.
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>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2060
Remove the useless ConsoleLogLock spinlock.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2060
Remove the useless ConsoleLogLock spinlock.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.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>
The patch moves all files under UefiCpuPkg/Include/Register/ to
MdePkg/Include/Register using following detailed approaches:
1. Move UefiCpuPkg/Include/Register/Amd/ to
MdePkg/Include/Register/Amd folder.
2. Move remaining in UefiCpuPkg/Include/Register/ to
MdePkg/Include/Register/Intel folder.
3. Create wrapper header files under UefiCpuPkg/Include/Register/
to include the accordingly files in MdePkg/Include/Register/Intel.
This is to avoid build break because code in other repos like
edk2-platform includes the file from UefiCpuPkg.
The wrapper header files will be removed after all consumers code
is updated.
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>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2008
MdeModulePkg/DxeIpl needs to get CPUID output for CPU
5-level paging capability detection.
In order to use the macros/structures defined in
UefiCpuPkg/Include/Register/Cpuid.h, the patch adds Intel/Cpuid.h
to MdePkg/Include/Register/ directory and updates Cpuid.h
in UefiCpuPkg to include the new one in MdePkg.
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Liming Gao <liming.gao@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
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>
Today's code defines macros like CR0_PG, CR0_WP, CR4_PSE, CR4_PAE
when checking whether individual bits are set in CR0 or CR4 register.
The patch changes the code to use IA32_CR0 and IA32_CR4 structure
defined in MdePkg/Include/Library/BaseLib.h so that the module
local macros can be removed.
There is no functionality impact to this change.
Cc: Jiewen Yao <jiewen.yao@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>
This debug message may be called by BSP and APs. It may
caused ASSERT when APs call this debug code.
In order to avoid system boot assert, Remove this debug
message.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
This debug message may be called by BSP and APs. It may
caused ASSERT when APs call this debug code.
In order to avoid system boot assert, Remove this debug
message.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
1.Remove "out" attribute for " Buffer" parameter in function header.
2.Add "out" attribute for " Token" parameter in function header.
3.Update ProcedureArgument to ProcedureArguments.
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>
The pointer Pml5Entry, returned from call to function
AllocatePageTableMemory, may be null.
So add check for it.
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: Ray Ni <ray.ni@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
For semaphore type register, it required all processors to do the
task at the same time.
Current logic begins BSP's task after all APs have finished their tasks.
This will caused set semaphore task hang if semaphore has package
level type.
This patch use new EDKII_PEI_MP_SERVICES2_PPI to start all processors at
the same time to fix the potential hang issue.
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>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1973
Produce new EDKII_PEI_MP_SERVICES2_PPI service.
It includes all APIs existed in EFI_PEI_MP_SERVICES_PPI
and add new API EDKII_PEI_MP_SERVICES_STARTUP_ALL_CPUS.
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=1973
Add definition for new EDKII_PEI_MP_SERVICES2_PPI.
It includes all APIs existed in EFI_PEI_MP_SERVICES_PPI
and add new API EDKII_PEI_MP_SERVICES_STARTUP_ALL_CPUS.
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=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>
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=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>
This reverts commit 30f6148546.
Commit 30f6148546 causes a build failure, when building for IA32:
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c: In function
> 'PerformRemainingTasks':
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c:1440:9: error:
> 'mCpuSmmStaticPageTable' undeclared (first use in this function)
> if (mCpuSmmStaticPageTable) {
"mCpuSmmStaticPageTable" is an X64-only variable. It is defined in
"X64/PageTbl.c", which is not linked into the IA32 binary. We must not
reference the variable in such code that is linked into both IA32 and X64
builds, such as "PiSmmCpuDxeSmm.c".
We have encountered the same challenge at least once in the past:
- https://bugzilla.tianocore.org/show_bug.cgi?id=1593
- commit 37f9fea5b8 ("UefiCpuPkg\CpuSmm: Save & restore CR2 on-demand
paging in SMM", 2019-04-04)
The right approach is to declare a new function in "PiSmmCpuDxeSmm.h", and
to provide two definitions for the function, one in "Ia32/PageTbl.c", and
another in "X64/PageTbl.c". The IA32 implementation should return a
constant value. The X64 implementation should return
"mCpuSmmStaticPageTable". (In the example named above, the functions were
SaveCr2() and RestoreCr2().)
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
[lersek@redhat.com: push revert immediately, due to build breakage that
would have been easy to catch before submitting the patch]
Commit c60d36b4d1
* UefiCpuPkg/SmmCpu: Block access-out only when static paging is used
updated page fault handler to treat SMM access-out as allowed
address when static paging is not used.
But that commit is not complete because the page table is still
updated in SetUefiMemMapAttributes() for non-SMRAM memory. When SMM
code accesses non-SMRAM memory, page fault is still generated.
This patch skips to update page table for non-SMRAM memory and
page table itself.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1972
Function in this library may be used by APs. Assert will be trig if AP
uses dynamic pcd.
This patch enhance the current code, remove the unnecessary usage of
dynamic PCD. This change try to avoid report this issue again later.
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>
Reviewed-by: Star Zeng <star.zeng@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1972
AP calls CollectProcessorData() to collect processor info.
CollectProcessorData function finally calls PcdGetSize function to
get DynamicPCD PcdCpuFeaturesSetting value. PcdGetSize will use
PeiServices table which caused below assert info:
Processor Info: Package: 1, MaxCore : 4, MaxThread: 1
Package: 0, Valid Core : 4
ASSERT [CpuFeaturesPei] c:\projects\jsl\jsl_v1193\Edk2\MdePkg\Library
\PeiServicesTablePointerLibIdt\PeiServicesTablePointer.c(48):
PeiServices != ((void *) 0)
This change uses saved global pcd size instead of calls PcdGetSize to
fix this issue.
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>
Reviewed-by: Star Zeng <star.zeng@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1968
The new VS2012 build failure is caused by 7a0df26.
xxx\registercpufeatureslib\dxeregistercpufeatureslib.c(258) :
warning C4701: potentially uninitialized local variable 'MpEvent' used
It is a false positive alarm.
MpEvent is assigned at line 238 and will be used at line 258, both
lines are controlled by "if (CpuFeaturesData->NumberOfCpus > 1)".
This patch initializes MpEvent to suppress incorrect compiler/analyzer
warnings.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1961
Enhance Ppin code to enable and unlock for TRUE State,
and disable and lock for FALSE State.
Note: enable and lock could not be set both.
According to SDM, once Enable_PPIN is set, attempt to write
1 to LockOut will cause #GP, and writing 1 to LockOut is
permitted only if Enable_PPIN is clear.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Cc: Kevin Li <kevin.y.li@intel.com>
Signed-off-by: Star Zeng <star.zeng@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=1937
Add MM Mp Protocol in PiSmmCpuDxeSmm driver.
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.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>
Fixes: 4eee0cc7c
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1946
The patch changes SMM environment to use 5 level paging when CPU
supports it.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
(cherry picked from commit 7365eb2c8c)
This reverts commit 7365eb2c8c.
Commit
7c5010c7f8 MdePkg/BaseLib.h: Update IA32_CR4 structure for 5-level paging
technically breaks the EDKII development process documented in
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process
and Maintainers.txt in EDKII repo root directory.
The voilation is commit 7c5010c7f8 doesn't have a Reviewed-by or
Acked-by from MdePkg maintainers.
In order to revert 7c5010c7f8, 7365eb2c8 needs to revert first otherwise
simply reverting 7c5010c7f8 will cause build break.
Signed-off-by: Ray Ni <ray.ni@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1968
We met assertion like below, it happens when there is only
one processor.
ASSERT_EFI_ERROR (Status = Not started)
ASSERT [CpuFeaturesDxe] X:\XXX\XXX\RegisterCpuFeaturesLib\
DxeRegisterCpuFeaturesLib.c(149): !EFI_ERROR (Status)
The code should not call StartupAllAPs when there is only one processor.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Cc: Kevin Li <kevin.y.li@intel.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1946
The patch changes SMM environment to use 5 level paging when CPU
supports it.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Per SDM, for IA-32e 4-KByte paging, there are four layers in the page
table structure:
1. PML4
2. Page-Directory-Pointer Table (PDPT)
3. Page-Directory (PD)
4. Page Table (PT)
The patch changes the local variable names and comments to use
"PML4", "PDPT", "PD", "PT" to better align to terms used in SDM.
There is no functionality impact for this change.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.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>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1642
This patch fixes typo in CPU_FEATURE_THREE_STRICK_COUNTER.
CPU_FEATURE_THREE_STRICK_COUNTER
->
CPU_FEATURE_THREE_STRIKE_COUNTER
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Cc: Kevin Li <kevin.y.li@intel.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@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>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1810
This patch covers two problems.
1. Current code gets CPUID_THERMAL_POWER_MANAGEMENT in
ClockModulationInitialize() and uses its ECMD bit for all processors.
But ClockModulationInitialize() is only executed by BSP, that means
the bit is just for BSP.
It may have no functionality issue as all processors may have same
bit value in a great possibility. But for good practice, the code
should get CPUID_THERMAL_POWER_MANAGEMENT in ClockModulationSupport
(executed by all processors), and then use them in
ClockModulationInitialize() for all processors.
We can see that Aesni.c (and others) have used this good practice.
2. Current code uses 3 CPU_REGISTER_TABLE_WRITE_FIELD for
MSR_IA32_CLOCK_MODULATION in ClockModulationInitialize(), they can
be reduced to 1 CPU_REGISTER_TABLE_WRITE64 by getting
MSR_IA32_CLOCK_MODULATION for all processors in
ClockModulationSupport() and then update fields for register table
write in ClockModulationInitialize().
We may argue that there may be more times of MSR_IA32_CLOCK_MODULATION
getting. But actually the times of MSR_IA32_CLOCK_MODULATION getting
could be also reduced.
The reason is in ProgramProcessorRegister() of CpuFeaturesInitialize.c,
AsmMsrBitFieldWrite64 (AsmReadMsr64 + AsmWriteMsr64) will be used for
CPU_REGISTER_TABLE_WRITE_FIELD, and AsmWriteMsr64 will be used for
CPU_REGISTER_TABLE_WRITE64.
The times of MSR accessing could be reduced with this patch.
Without the patch:
3 CPU_REGISTER_TABLE_WRITE_FIELD (in ClockModulationInitialize)
==> 3 AsmMsrBitFieldWrite64
==> 3 AsmReadMsr64 + 3 AsmWriteMsr64
With the patch:
1 AsmReadMsr64 (in ClockModulationSupport) +
1 CPU_REGISTER_TABLE_WRITE64 (in ClockModulationInitialize)
==> 1 AsmWriteMsr64
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Cc: Kevin Li <kevin.y.li@intel.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1829
There will be ASSERT if LMCE is supported as below.
DXE_ASSERT!: [CpuFeaturesDxe]
XXX\UefiCpuPkg\Library\CpuCommonFeaturesLib\MachineCheck.c (342):
ConfigData != ((void *) 0)
The code should get Config Data and FeatureControlGetConfigData
could be used.
This issue is there since the code was added at the commit below.
Revision: 3d6275c113
Date: 2017/8/4 8:46:41
UefiCpuPkg CpuCommonFeaturesLib: Enable LMCE feature.
The commits below are also related to move the code.
Revision: 0233871442
Date: 2017/9/1 10:12:38
UefiCpuPkg/Lmce.c Remove useless file.
Revision: 306a5bcc6b
Date: 2017/8/17 11:40:38
UefiCpuPkg/CpuCommonFeaturesLib: Merge machine check code to same file.
So, the code may not be tested at all on a platform
that supports LMCE.
BTW: A typo in LmceInitialize is also fixed.
The typo is introduced by the commit below.
Revision: d28daaddb3
Date: 2018/10/17 9:24:05
UefiCpuPkg/CpuCommonFeaturesLib: Register MSR base on scope Info.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Cc: Kevin Li <kevin.y.li@intel.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1808
In current code, the values of TopaEntryPtr->Uint64 for TopaTable
and the values of OutputBaseReg.Uint64 and OutputMaskPtrsReg.Uint64
to register table write for RTIT_OUTPUT_BASE and RTIT_OUTPUT_MASK_PTRS
are not been initialized in whole. For example, the reserved bits in
OutputBaseReg.Uint64 are random that will cause GP fault like below
when SetProcessorRegister (in CpuFeaturesInitialize.c) sets register
based on register table.
!!!! X64 Exception Type - 0D(#GP - General Protection)
CPU Apic ID - 00000000 !!!!
ExceptionData - 0000000000000000
RIP -0000000064D69576, CS -0000000000000038, RFLAGS -0000000000010246
RAX -000000006B9F1001, RCX -0000000000000560, RDX -0000000000000000
RBX -0000000064EECA18, RSP -000000006CB82BA0, RBP -0000000000000008
RSI -0000000080000000, RDI -0000000000000011
R8 -000000006B9493D0, R9 -0000000000000010, R10 -00000000000000FF
R11 -000000006CB82A50, R12 -0000000064D70F50, R13 -0000000066547050
R14 -0000000064E3E198, R15 -0000000000000000
DS -0000000000000030, ES -0000000000000030, FS -0000000000000030
GS -0000000000000030, SS -0000000000000030
CR0 -0000000080010013, CR2 -0000000000000000, CR3 -000000006C601000
CR4 -0000000000000628, CR8 -0000000000000000
DR0 -0000000000000000, DR1 -0000000000000000, DR2 -0000000000000000
DR3 -0000000000000000, DR6 -00000000FFFF0FF0, DR7 -0000000000000400
GDTR -000000006B8CCF18 0000000000000047, LDTR -0000000000000000
IDTR -000000006687E018 0000000000000FFF, TR -0000000000000000
FXSAVE_STATE -000000006CB82800
And current code gets MSR_IA32_RTIT_CTL, MSR_IA32_RTIT_OUTPUT_BASE and
MSR_IA32_RTIT_OUTPUT_MASK_PTRS in ProcTraceInitialize() and uses their
values for all processors. But ProcTraceInitialize() is only executed
by BSP, that means the values just for BSP. For good practice, the code
should get MSR_IA32_RTIT_CTL, MSR_IA32_RTIT_OUTPUT_BASE and
MSR_IA32_RTIT_OUTPUT_MASK_PTRS in ProcTraceSupport (executed by all
processors), and then use them in ProcTraceInitialize() for all
processors. This can also resolve the issue that the values of
OutputBaseReg.Uint64 and OutputMaskPtrsReg.Uint64 are not been
initialized in whole.
For TopaEntryPtr->Uint64, this patch updates code to initialize it
in whole explicitly by TopaEntryPtr->Uint64 = 0 before updating its
fields.
At the same time, this patch also eliminates the ProcTraceSupported
field in PROC_TRACE_PROCESSOR_DATA and the TopaMemArrayCount field in
PROC_TRACE_DATA.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Cc: Kevin Li <kevin.y.li@intel.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1809
Current code disables TraceEn at the end of ProcTraceInitialize(),
then there will be much memory allocated even when ProcTrace feature
is disabled.
This patch updates code to disable TraceEn and return at the beginning
of ProcTraceInitialize() when when ProcTrace feature is disabled.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Cc: Kevin Li <kevin.y.li@intel.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1679
The checking to CpuInfo->CpuIdVersionInfoEcx.Bits.AESNI is enough,
the checking to CPU generation could be removed, then the code
could be reused by more platforms.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1136
CVE: CVE-2018-12182
Customer met system hang-up during serial port loopback test in OS.
It is a corner case happened with one CPU core doing "out dx,al" and
another CPU core(s) doing "rep outs dx,byte ptr [rsi]".
Detailed code flow is as below.
1. Serial port loopback test in OS.
One CPU core: "out dx,al" -> Writing B2h, SMI will happen.
Another CPU core(s): "rep outs dx,byte ptr [rsi]".
2. SMI happens to enter SMM.
"out dx" (SMM_IO_TYPE_OUT_DX) is saved as I/O instruction type in
SMRAM save state for CPU doing "out dx,al".
"rep outs dx" (SMM_IO_TYPE_REP_OUTS) is saved as I/O instruction
type and rsi is save as I/O Memory Address in SMRAM save state for
CPU doing "rep outs dx, byte ptr [rsi]".
NOTE: I/O Memory Address (rsi) is a virtual address mapped by
OS/Virtual Machine.
3. Some SMM code calls EFI_SMM_CPU_PROTOCOL.ReadSaveState() with
EFI_SMM_SAVE_STATE_REGISTER_IO and parse data returned.
For example:
https://github.com/tianocore/edk2/blob/master/QuarkSocPkg/
QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNC/QNCSmmSw.c#L76
4. SmmReadSaveState() is executed to read save state for
EFI_SMM_SAVE_STATE_REGISTER_IO.
- The SmmReadSaveState() function in
"UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c" calls the
SmmCpuFeaturesReadSaveStateRegister() function, from the platform's
SmmCpuFeaturesLib instance.
- If that platform-specific function returns EFI_UNSUPPORTED, then
PiSmmCpuDxeSmm falls back to the common function
ReadSaveStateRegister(), defined in file
"UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c".
Current ReadSaveStateRegister() in
UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c is trying to copy data
from I/O Memory Address for EFI_SMM_SAVE_STATE_IO_TYPE_REP_PREFIX,
PF will happen as SMM page table does not know and cover this
OS/Virtual Machine virtual address.
Same case is for SmmCpuFeaturesReadSaveStateRegister() in platform-
specific SmmCpuFeaturesLib instance if it has similar implementation
to read save state for EFI_SMM_SAVE_STATE_REGISTER_IO with
EFI_SMM_SAVE_STATE_IO_TYPE_REP_PREFIX.
Same case is for "ins", 'outs' and 'rep ins'.
So to fix the problem, this patch updates the code to only support
IN/OUT, but not INS/OUTS/REP INS/REP OUTS for SmmReadSaveState().
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
These files have \r\n line endings, but a few lines use \r\r\n which
is not a valid line ending. These lines were causing problems for git
and other tools.
Signed-off-by: Joe Richey <joerichey@google.com>
Review-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1464
Currently Framework compatibility support is not needed and
PcdFrameworkCompatibilitySupport will be removed from edk2.
So remove the usage of this PCD firstly.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
The CpuMpPei module uses a services from the CpuLib class,
but the CpuLib class is missing from the INF file. This
update is required to use the new MpInitLibUp instance that
does not use the CpuLib class.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Antoine Coeur
Hao A Wu
Laszlo Ersek
Siyuan Fu
Jason Voelz
Eric Dong
Shenglei Zhang
Ray Ni
Michael D Kinney
Sean Brogan
Leif Lindholm
John E Lofgren
Damian Nikodem
Chasel Chiu
Donald Kuo
Star Zeng
Zhichao Gao
Joe Richey
Dandan Bi