summaryrefslogtreecommitdiffstats
path: root/UefiCpuPkg/PiSmmCpuDxeSmm
Commit message (Collapse)AuthorAgeFilesLines
* UefiCpuPkg/PiSmmCpuDxeSmm: Remove useless code in ResetTokens.Dong, Eric2020-04-131-20/+0
| | | | | | | | | | | | | | | | | REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2388 After remove Used parameter, below code in ResetTokens can also be removed: 1. The RunningApCount parameter will be reset in GetFreeToken. 2. The ReleaseSpinLock should be called in ReleaseToken function, Code in this function seems like a later fix if ReleaseToken not Release it. We should remove code here and fix the real issue if existed. Signed-off-by: Eric Dong <eric.dong@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Star Zeng <star.zeng@intel.com> Cc: Laszlo Ersek <lersek@redhat.com>
* UefiCpuPkg/PiSmmCpuDxeSmm: Remove Used parameter.Dong, Eric2020-04-132-6/+5
| | | | | | | | | | | | | | REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2388 After patch "UefiCpuPkg/PiSmmCpuDxeSmm: Improve the performance of GetFreeToken()" which adds new parameter FirstFreeToken, it's not need to use Uses parameter. This patch used to remove this parameter. Signed-off-by: Eric Dong <eric.dong@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Star Zeng <star.zeng@intel.com> Cc: Laszlo Ersek <lersek@redhat.com>
* UefiCpuPkg/PiSmmCpuDxeSmm: Improve the performance of GetFreeToken()Ray Ni2020-04-132-47/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | | Today's GetFreeToken() runs at the algorithm complexity of O(n) where n is the size of the token list. The change introduces a new global variable FirstFreeToken and it always points to the first free token. So the algorithm complexity of GetFreeToken() decreases from O(n) to O(1). The improvement matters when some SMI code uses StartupThisAP() service for each of the AP such that the algorithm complexity becomes O(n) * O(m) where m is the AP count. As next steps, 1. PROCEDURE_TOKEN.Used field can be optimized out because all tokens before FirstFreeToken should have "Used" set while all after FirstFreeToken should have "Used" cleared. 2. ResetTokens() can be optimized to only reset tokens before FirstFreeToken. v2: add missing line in InitializeDataForMmMp. v3: update copyright year to 2020. Signed-off-by: Ray Ni <ray.ni@intel.com> Reviewed-by: Eric Dong <eric.dong@intel.com> Reviewed-by: Star Zeng <star.zeng@intel.com>
* UefiCpuPkg/PiSmmCpuDxeSmm: fix S3 Resume for CPU hotplugLaszlo Ersek2020-03-041-2/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The "ACPI_CPU_DATA.NumberOfCpus" field is specified as follows, in "UefiCpuPkg/Include/AcpiCpuData.h" (rewrapped for this commit message): // // The number of CPUs. If a platform does not support hot plug CPUs, // then this is the number of CPUs detected when the platform is booted, // regardless of being enabled or disabled. If a platform does support // hot plug CPUs, then this is the maximum number of CPUs that the // platform supports. // The InitializeCpuBeforeRebase() and InitializeCpuAfterRebase() functions in "UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c" try to restore CPU configuration on the S3 Resume path for *all* CPUs accounted for in "ACPI_CPU_DATA.NumberOfCpus". This is wrong, as with CPU hotplug, not all of the possible CPUs may be present at the time of S3 Suspend / Resume. The symptom is an infinite wait. Instead, the "mNumberOfCpus" variable should be used, which is properly maintained through the EFI_SMM_CPU_SERVICE_PROTOCOL implementation (see SmmAddProcessor(), SmmRemoveProcessor(), SmmCpuUpdate() in "UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c"). When CPU hotplug is disabled, "mNumberOfCpus" is constant, and equals "ACPI_CPU_DATA.NumberOfCpus" at all times. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Eric Dong <eric.dong@intel.com> Cc: Igor Mammedov <imammedo@redhat.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Michael Kinney <michael.d.kinney@intel.com> Cc: Philippe Mathieu-Daudé <philmd@redhat.com> Cc: Ray Ni <ray.ni@intel.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1512 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200226221156.29589-3-lersek@redhat.com> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Eric Dong <eric.dong@intel.com> Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> [lersek@redhat.com: shut up UINTN->UINT32 warning from Windows VS2019 PR]
* UefiCpuPkg/PiSmm: Fix various typosAntoine Coeur2020-02-107-16/+16
| | | | | | | | | | | | | | 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>
* UefiCpuPkg/PiSmmCpuDxeSmm: fix 2M->4K page splitting regression for PDEsLaszlo Ersek2020-01-171-4/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In commit 4eee0cc7cc0d ("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 4eee0cc7cc0db > // > // 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: 4eee0cc7cc0db74489b99c19eba056b53eda6358 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com> Reviewed-by: Ray Ni <ray.ni@intel.com>
* UefiCpuPkg/PiSmmCpuDxeSmm: Add missed comments for parameter.Eric Dong2020-01-101-0/+2
| | | | | | | | | | | This issue caused by below change: SHA-1: b948a496150f4ae4f656c0f0ab672608723c80e6 * UefiCpuPkg/PiSmmCpuDxeSmm: Pre-allocate PROCEDURE_TOKEN buffer REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2388 Reviewed-by: Ray Ni <ray.ni@intel.com> Acked-by: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Eric Dong <eric.dong@intel.com>
* UefiCpuPkg/PiSmmCpuDxeSmm: improve the coding styleEric Dong2020-01-081-9/+9
| | | | | | | | | | | | | | | | | 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>
* Revert "UefiCpuPkg/PiSmmCpuDxeSmm: Fix buffer overflow issue."Eric Dong2020-01-081-8/+8
| | | | | | | | | | This reverts commit 123b720eeb371e0a31eb727bcf59255b584e355f. The commit message for commit 123b720eeb37 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>
* UefiCpuPkg/PiSmmCpuDxeSmm: Pre-allocate PROCEDURE_TOKEN bufferEric Dong2020-01-022-87/+109
| | | | | | | | | | | | | | | | | 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 (9caaa79dd7e078ebb4012dde3b3d3a5d451df609) 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>
* UefiCpuPkg/PiSmmCpuDxeSmm: Fix buffer overflow issue.Eric Dong2019-12-241-8/+8
| | | | | | | | | | | | 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>
* UefiCpuPkg/PiSmmCpuDxeSmm: Remove dependence between APsEric Dong2019-12-242-85/+45
| | | | | | | | | | | | | | | | | 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>
* UefiCpuPkg/PiSmmCpuDxeSmm: Avoid allocate Token every timeEric Dong2019-12-063-4/+81
| | | | | | | | | | | | | | 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>
* UefiCpuPkg: Update the coding stylesShenglei Zhang2019-12-043-3/+3
| | | | | | | | | | | | 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>
* UefiCpuPkg: Fix potential spinLock issue in SmmStartupThisApDamian Nikodem2019-09-201-7/+2
| | | | | | | | | | | | | | | | | | | | | | 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>
* UefiCpuPkg/PiSmmCpu: Enable 5L paging only when phy addr line > 48Ray Ni2019-09-132-22/+39
| | | | | | | | | | | | | | | | | | | 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>
* UefiCpuPkg/PiSmmCpu: Restrict access per PcdCpuSmmRestrictedMemoryAccessRay Ni2019-09-044-8/+49
| | | | | | | | | | | | | | | | | | 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>
* UefiCpuPkg/PiSmmCpu: Use new PCD PcdCpuSmmRestrictedMemoryAccessRay Ni2019-09-042-22/+34
| | | | | | | | | | | | | | | 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>
* UefiCpuPkg/PiSmmCpuDxeSmm: Supports test then write new value logic.Dong, Eric2019-08-211-0/+29
| | | | | | | | | | | 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>
* UefiCpuPkg/PiSmmCpuDxeSmm: Combine CR read/write action.Dong, Eric2019-08-211-42/+62
| | | | | | Signed-off-by: Eric Dong <eric.dong@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
* UefiCpuPkg/PiSmmCpuDxeSmm: don't free page table pages that are required to ↵Damian Nikodem2019-08-211-40/+59
| | | | | | | | | | | | | | | | | | | | | handle current page fault 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>
* UefiCpuPkg/PiSmmCpuDxeSmm: Fix coding styleShenglei Zhang2019-08-132-6/+6
| | | | | | | | | | | | | 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>
* UefiCpuPkg/PiSmmCpuDxeSmm: Clean up useless ConsoleLogLock spinlock.Eric Dong2019-08-091-2/+0
| | | | | | | | | | 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>
* UefiCpuPkg: Update code to include register definitions from MdePkgNi, Ray2019-08-092-4/+2
| | | | | | | 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>
* UefiCpuPkg/PiSmmCpuDxeSmm: Remove debug message.Eric Dong2019-08-061-21/+1
| | | | | | | | | | | | 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>
* UefiCpuPkg/PiSmmCpuDxeSmm: Make code consistent with commentsshenglei2019-08-062-3/+3
| | | | | | | | | | | | 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>
* UefiCpuPkg/PiSmmCpuDxeSmm: Add check for pointer Pml5EntryShenglei Zhang2019-08-051-0/+1
| | | | | | | | | | | | 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>
* UefiCpuPkg/PiSmmCpuDxeSmm: Keep function comment and declaration adjacentShenglei Zhang2019-08-021-1/+0
| | | | | | | | | 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: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Eric Dong <eric.dong@intel.com>
* Revert "UefiCpuPkg/PiSmmCpu: Allow SMM access-out when static paging is OFF"Laszlo Ersek2019-07-261-15/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit 30f6148546c7092650ab4886fc6d95d5065c3188. Commit 30f6148546c7 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 37f9fea5b88d ("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]
* UefiCpuPkg/PiSmmCpu: Allow SMM access-out when static paging is OFFNi, Ray2019-07-261-6/+15
| | | | | | | | | | | | | | | | | | | | | | Commit c60d36b4d1ee1f69b7cca897d3621dfa951895c2 * 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>
* UefiCpuPkg/PiSmmCpuDxeSmm: Enable MM MP ProtocolEric Dong2019-07-166-25/+1391
| | | | | | | | | | | | 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>
* UefiCpuPkg/PiSmmCpu: ReclaimPages: fix incorrect operator bindingRay Ni2019-07-121-1/+1
| | | | | | | | | 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>
* UefiCpuPkg/PiSmmCpu: Enable 5 level paging when CPU supportsRay Ni2019-07-125-300/+561
| | | | | | | | | | | | | 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 7365eb2c8cf1d7112330d09918c0c67e8d0b827a)
* Revert "UefiCpuPkg/PiSmmCpu: Enable 5 level paging when CPU supports"Ray Ni2019-07-125-561/+300
| | | | | | | | | | | | | | | | | This reverts commit 7365eb2c8cf1d7112330d09918c0c67e8d0b827a. 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>
* UefiCpuPkg/PiSmmCpu: Enable 5 level paging when CPU supportsRay Ni2019-07-105-300/+561
| | | | | | | | | | | | 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>
* UefiCpuPkg/PiSmmCpu: Change variable names and comments to follow SDMRay Ni2019-07-101-57/+57
| | | | | | | | | | | | | | | | | | | 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>
* UefiCpuPkg PiSmmCpuDxeSmm: Only support IN/OUT IO save state read ↵Star Zeng2019-05-141-8/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | (CVE-2018-12182) 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>
* UefiCpuPkg: Replace BSD License with BSD+Patent LicenseMichael D Kinney2019-04-0935-245/+35
| | | | | | | | | | | | | | | | | | | | | https://bugzilla.tianocore.org/show_bug.cgi?id=1373 Replace BSD 2-Clause License with BSD+Patent License. This change is based on the following emails: https://lists.01.org/pipermail/edk2-devel/2019-February/036260.html https://lists.01.org/pipermail/edk2-devel/2018-October/030385.html RFCs with detailed process for the license change: V3: https://lists.01.org/pipermail/edk2-devel/2019-March/038116.html V2: https://lists.01.org/pipermail/edk2-devel/2019-March/037669.html V1: https://lists.01.org/pipermail/edk2-devel/2019-March/037500.html Contributed-under: TianoCore Contribution Agreement 1.1 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>
* UefiCpuPkg\CpuSmm: Save & restore CR2 on-demand paging in SMMVanguput, Narendra K2019-04-044-3/+84
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1593 For every SMI occurrence, save and restore CR2 register only when SMM on-demand paging support is enabled in 64 bit operation mode. This is not a bug but to have better improvement of code. Patch5 is updated with separate functions for Save and Restore of CR2 based on review feedback. Patch6 - Removed Global Cr2 instead used function parameter. Patch7 - Removed checking Cr2 with 0 as per feedback. Patch8 and 9 - Aligned with EDK2 Coding style. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Vanguput Narendra K <narendra.k.vanguput@intel.com> Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Yao Jiewen <jiewen.yao@intel.com> Reviewed-by: Eric Dong <eric.dong@intel.com> Reviewed-by: Star Zeng <star.zeng@intel.com> Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* UefiCpuPkg/PiSmmCpu: Add Shadow Stack Support for X86 SMM.Jiewen Yao2019-02-2817-44/+781
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1521 We scan the SMM code with ROPgadget. http://shell-storm.org/project/ROPgadget/ https://github.com/JonathanSalwan/ROPgadget/tree/master This tool reports the gadget in SMM driver. This patch enabled CET ShadowStack for X86 SMM. If CET is supported, SMM will enable CET ShadowStack. SMM CET will save the OS CET context at SmmEntry and restore OS CET context at SmmExit. Test: 1) test Intel internal platform (x64 only, CET enabled/disabled) Boot test: CET supported or not supported CPU on CET supported platform CET enabled/disabled PcdCpuSmmCetEnable enabled/disabled Single core/Multiple core PcdCpuSmmStackGuard enabled/disabled PcdCpuSmmProfileEnable enabled/disabled PcdCpuSmmStaticPageTable enabled/disabled CET exception test: #CF generated with PcdCpuSmmStackGuard enabled/disabled. Other exception test: #PF for normal stack overflow #PF for NX protection #PF for RO protection CET env test: Launch SMM in CET enabled/disabled environment (DXE) - no impact to DXE The test case can be found at https://github.com/jyao1/SecurityEx/tree/master/ControlFlowPkg 2) test ovmf (both IA32 and X64 SMM, CET disabled only) test OvmfIa32/Ovmf3264, with -D SMM_REQUIRE. qemu-system-x86_64.exe -machine q35,smm=on -smp 4 -serial file:serial.log -drive if=pflash,format=raw,unit=0,file=OVMF_CODE.fd,readonly=on -drive if=pflash,format=raw,unit=1,file=OVMF_VARS.fd QEMU emulator version 3.1.0 (v3.1.0-11736-g7a30e7adb0-dirty) 3) not tested IA32 CET enabled platform Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Yao Jiewen <jiewen.yao@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* UefiCpuPkg: Merge StuffRsb.inc files into one in UefiCpuPkg/IncludeHao Wu2019-01-026-114/+4
| | | | | | | | | | | | | | | | | | | | | | | | REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1091 Previously, when compiling NASM source files, BaseTools did not support including files outside of the NASM source file directory. As a result, we duplicated multiple copies of "StuffRsb.inc" files in UefiCpuPkg. Those INC files contain the common logic to stuff the Return Stack Buffer and are identical. After the fix of BZ 1085: https://bugzilla.tianocore.org/show_bug.cgi?id=1085 The above support was introduced. Thus, this commit will merge all the StuffRsb.inc files in UefiCpuPkg into one file. The merged file will be named 'StuffRsbNasm.inc' and be placed under folder UefiCpuPkg/Include/. Cc: Ruiyu Ni <ruiyu.ni@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu <hao.a.wu@intel.com> Reviewed-by: Eric Dong <eric.dong@intel.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
* UefiCpuPkg/PiSmmCpuDxeSmm: Update to consume SpeculationBarrierHao Wu2018-12-251-3/+3
| | | | | | | | | | | | | | | | | REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1417 Since BaseLib API AsmLfence() is a x86 arch specific API and should be avoided using in generic codes, this commit replaces the usage of AsmLfence() with arch-generic API SpeculationBarrier(). Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Liming Gao <liming.gao@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu <hao.a.wu@intel.com> Reviewed-by: Eric Dong <eric.dong@intel.com>
* UefiCpuPkg/PiSmmCpuDxeSmm: Separate semaphore container.Eric Dong2018-11-111-9/+20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In current implementation, core and package level sync uses same semaphores. Sharing the semaphore may cause wrong execution order. For example: 1. Feature A has CPU_FEATURE_CORE_BEFORE dependency with Feature B. 2. Feature C has CPU_FEATURE_PACKAGE_AFTER dependency with Feature B. The expected feature initialization order is A B C: A ---- (Core Depends) ----> B ---- (Package Depends) ----> C For a CPU has 1 package, 2 cores and 4 threads. The feature initialization order may like below: Thread#1 Thread#2 Thread#3 Thread#4 [A.Init] [A.Init] [A.Init] Release(S1, S2) Release(S1, S2) Release(S3, S4) Wait(S1) * 2 Wait(S2) * 2 <------------------------------- Core sync [B.Init] [B.Init] Release (S1,S2,S3,S4) Wait (S1) * 4 <----------------------------------------------------- Package sync Wait(S4 * 2) <- Core sync [B.Init] In above case, for thread#4, when it syncs in core level, Wait(S4) * 2 isn't blocked and [B.Init] runs. But [A.Init] hasn't run in thread#3. It's wrong! Thread#4 should execute [B.Init] after thread#3 executes [A.Init] because B core level depends on A. The reason of the wrong execution order is that S4 is released in thread#1 by calling Release (S1, S2, S3, S4) and in thread #4 by calling Release (S3, S4). To fix this issue, core level sync and package level sync should use separate semaphores. In above example, the S4 released in Release (S1, S2, S3, S4) should not be the same semaphore as that in Release (S3, S4). Related BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1311 Cc: Laszlo Ersek <lersek@redhat.com> 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> Acked-by: Laszlo Ersek <lersek@redhat.com>
* UefiCpuPkg/SmmCpu: Block access-out only when static paging is usedJiewen Yao2018-11-081-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | When static paging is disabled, page table for below 4GB is created and page table for above 4GB is created dynamically in page fault handler. Today's implementation only allow SMM access-out to below types of memory address no matter static paging is enabled or not: 1. Reserved, run time and ACPI NVS type 2. MMIO But certain platform feature like RAS may need to access other types of memory from SMM. Today's code blocks these platforms. This patch simplifies the policy to only block when static paging is used so that the static paging can be disabled in these platforms to meet their SMM access-out need. Setting PcdCpuSmmStaticPageTable to FALSE can disable the static paging. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jiewen Yao <jiewen.yao@intel.com> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Eric Dong <eric.dong@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com> Acked-by: Laszlo Ersek <lersek@redhat.com>
* UefiCpuPkg/PiSmmCpuDxeSmm: Fix ASSERT for success.Marvin H?user2018-10-301-2/+1
| | | | | | | | | | | | | | | | | | | | | 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>
* UefiCpuPkg/PiSmmCpuDxeSmm: Fix build failure for VS2012 and GCC49.Eric Dong2018-10-261-0/+1
| | | | | | | | | | | | Code initialized in function can't be correctly detected by build tool. Add code to clearly initialize the local variable before use it. Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Dandan Bi <dandan.bi@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>
* UefiCpuPkg/PiSmmCpuDxeSmm: Clean up useless code.Eric Dong2018-10-262-25/+1
| | | | | | | | | | | Remove useless code after change 93324390. Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Dandan Bi <dandan.bi@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>
* UefiCpuPkg/PiSmmCpuDxeSmm: Remove white space at line end.Eric Dong2018-10-261-2/+2
| | | | | | | | | | | Remove extra white space at the end of line. Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Dandan Bi <dandan.bi@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>
* UefiCpuPkg/PiSmmCpuDxeSmm: Add logic to support semaphore type.Eric Dong2018-10-223-143/+276
| | | | | | | | | | | | | | | | | | | | | | | | | | V4 changes: 1. Serial console log for different threads when program register table. 2. Check the AcpiCpuData before use it to avoid potential ASSERT. V3 changes: 1. Use global variable instead of internal function to return string for register type and dependence type. 2. Add comments for some complicated logic. V1 changes: Because this driver needs to set MSRs saved in normal boot phase, sync semaphore logic from RegisterCpuFeaturesLib code which used for normal boot phase. Detail see below change for RegisterCpuFeaturesLib: UefiCpuPkg/RegisterCpuFeaturesLib: Add logic to support semaphore type. Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.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> Acked-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* UefiCpuPkg/PiSmmCpuDxeSmm: [CVE-2017-5753] Fix bounds check bypassHao Wu2018-09-301-0/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1194 Speculative execution is used by processor to avoid having to wait for data to arrive from memory, or for previous operations to finish, the processor may speculate as to what will be executed. If the speculation is incorrect, the speculatively executed instructions might leave hints such as which memory locations have been brought into cache. Malicious actors can use the bounds check bypass method (code gadgets with controlled external inputs) to infer data values that have been used in speculative operations to reveal secrets which should not otherwise be accessed. It is possible for SMI handler(s) to call EFI_SMM_CPU_PROTOCOL service ReadSaveState() and use the content in the 'CommBuffer' (controlled external inputs) as the 'CpuIndex'. So this commit will insert AsmLfence API to mitigate the bounds check bypass issue within SmmReadSaveState(). For SmmReadSaveState(): The 'CpuIndex' will be passed into function ReadSaveStateRegister(). And then in to ReadSaveStateRegisterByIndex(). With the call: ReadSaveStateRegisterByIndex ( CpuIndex, SMM_SAVE_STATE_REGISTER_IOMISC_INDEX, sizeof(IoMisc.Uint32), &IoMisc.Uint32 ); The 'IoMisc' can be a cross boundary access during speculative execution. Later, 'IoMisc' is used as the index to access buffers 'mSmmCpuIoWidth' and 'mSmmCpuIoType'. One can observe which part of the content within those buffers was brought into cache to possibly reveal the value of 'IoMisc'. Hence, this commit adds a AsmLfence() after the check of 'CpuIndex' within function SmmReadSaveState() to prevent the speculative execution. A more detailed explanation of the purpose of commit is under the 'Bounds check bypass mitigation' section of the below link: https://software.intel.com/security-software-guidance/insights/host-firmware-speculative-execution-side-channel-mitigation And the document at: https://software.intel.com/security-software-guidance/api-app/sites/default/files/337879-analyzing-potential-bounds-Check-bypass-vulnerabilities.pdf Cc: Michael D Kinney <michael.d.kinney@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu <hao.a.wu@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com> Reviewed-by: Eric Dong <eric.dong@intel.com> Acked-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-03 07:33:17 +0000