Re: [PATCH 1/6] UefiCpuPkg/MpInitLib: split wake up buffer into two parts

[Laszlo Ersek <lersek@redhat.com>]

Hello Jian,

On 01/15/18 09:54, Jian J Wang wrote:
> If PcdDxeNxMemoryProtectionPolicy is set to enable protection for memory
> of EfiBootServicesCode, EfiConventionalMemory, the BIOS will hang at a page
> fault exception during MP initialization.
> 
> The root cause is that the AP wake up buffer, which is below 1MB and used
> to hold both AP init code and data, is type of EfiConventionalMemory (not
> really allocated because of potential conflict with legacy code), and is
> marked as non-executable. During the transition from real address mode
> to long mode, the AP init code has to enable paging which will then cause
> itself a page fault exception because it's just running in non-executable
> memory.
> 
> The solution is splitting AP wake up buffer into two part: lower part is
> still below 1MB and shared with legacy system, higher part is really
> allocated memory of BootServicesCode type. The init code in the memory
> below 1MB will not enable paging but just switch to protected mode and
> jump to higher memory, in which the init code will enable paging and
> switch to long mode.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c        | 34 ++++++++++
>  UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc    |  5 ++
>  UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 32 +++++-----
>  UefiCpuPkg/Library/MpInitLib/MpLib.c           | 45 +++++++++++++
>  UefiCpuPkg/Library/MpInitLib/MpLib.h           | 22 +++++++
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c        | 23 +++++++
>  UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc     |  5 +-
>  UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm  | 87 ++++++++++++++++----------
>  8 files changed, 204 insertions(+), 49 deletions(-)

This patch breaks OVMF on KVM. The symptom is that the guest crashes and reboots as follows (infinite reboot loop):

> Loading PEIM at 0x0007FEB0000 EntryPoint=0x0007FEB5C96 CpuMpPei.efi
> AP Loop Mode is 1
> WakeupBufferStart = 9F000, WakeupBufferSize = 1000
> -- crash & reboot here --
> SecCoreStartupWithStack(0xFFFCC000, 0x820000)

Here's the bisection log:

git bisect start
# bad: [06c1f423e17fe5ddef824d688d21c83730238ba6] BeagleBoardPkg: reroute Firmware Vendor Pcd to MdeModulePkg
git bisect bad 06c1f423e17fe5ddef824d688d21c83730238ba6
# good: [018432f0ce1b42541977f61f9c7607257a4bf43a] MdeModulePkg/Ip4Dxe: Add an independent timer for reconfig checking
git bisect good 018432f0ce1b42541977f61f9c7607257a4bf43a
# bad: [8ab0bd2397c9d3922e0c7dbb1aa6f7e08799079f] MdePkg/DMAR: Add the definition for DMA_CTRL_PLATFORM_OPT_IN_FLAG bit
git bisect bad 8ab0bd2397c9d3922e0c7dbb1aa6f7e08799079f
# good: [24a105a7d8b4b8312743cf265f869dc049b7ff92] BaseTools: Disable warning varargs in XCODE5 align to CLANG38
git bisect good 24a105a7d8b4b8312743cf265f869dc049b7ff92
# good: [b2725f57c7a1e6feeb176f1563a4f1a8c2eb6c6f] IntelSiliconPkg IntelVTdPmrPei: Get high top by host address width
git bisect good b2725f57c7a1e6feeb176f1563a4f1a8c2eb6c6f
# good: [4f10654e04601fe67a750c9b5a4242efd4141569] UefiCpuPkg/CpuDxe: fix SetMemoryAttributes issue in 32-bit mode
git bisect good 4f10654e04601fe67a750c9b5a4242efd4141569
# bad: [fbe2c4b9be98a5c2b9c1f6976f51e2456467e752] UefiCpuPkg/CpuDxe: clear NX attr for page directory
git bisect bad fbe2c4b9be98a5c2b9c1f6976f51e2456467e752
# bad: [fceafda5185af0445d83f8c819b65417b981c485] UefiCpuPkg/CpuExceptionHandlerLib: alloc code memory for exception handlers
git bisect bad fceafda5185af0445d83f8c819b65417b981c485
# bad: [f32bfe6d061420a15bac6083063d227c567e6388] UefiCpuPkg/MpInitLib: split wake up buffer into two parts
git bisect bad f32bfe6d061420a15bac6083063d227c567e6388
# first bad commit: [f32bfe6d061420a15bac6083063d227c567e6388] UefiCpuPkg/MpInitLib: split wake up buffer into two parts

Thanks
Laszlo