From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM11-DM6-obe.outbound.protection.outlook.com (NAM11-DM6-obe.outbound.protection.outlook.com [40.107.223.42]) by mx.groups.io with SMTP id smtpd.web10.19954.1596206697465133205 for ; Fri, 31 Jul 2020 07:44:57 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@amdcloud.onmicrosoft.com header.s=selector2-amdcloud-onmicrosoft-com header.b=EfSNcpw8; spf=none, err=SPF record not found (domain: amd.com, ip: 40.107.223.42, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=E09rL4sLVB7yL5Xg6EJEe12Pyr1X5mvGkyugPxIOUU0lr6TS3nJOndKy3iPgTqn6PMxRbVaEE2Qtn2gVcAHGtDpo01HuZd16kRiLCdF2bzpovx0HodBZNIpFuXy9Dw/nbJHnrrDj9LP1EKEL4MyG/yPit5YjhJ/P/DZQ8ciDqKIDv94aGvtv1lp8wA3/li3+RPzgg1S+UxoE3J61eG49U9SCYonuWC+BBJtxLLWg8+cYSyUZjB5XrOUeM4WrDlyGCTwxaPo+1SyzjL4U19MHdHqJ0XG1iyW+B4YGMG+5xInd2NLoH7eS90e1M8GChpYAdfc7PFdrAxxFLHkC94NGBQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=CZtVzDaRVDVDdBtJXnmx1pHKCCtX0OOuuOUl9Mbyf0U=; b=WwF9HqgyUtqp16Sn0dkhZeCb3ZNHcRgUzvoFqOYdu8yQm6Wgg0GKeKDcPhUjo+VeFUfNRVDCp+RZ5TdFoW6Jq3utCaYwAFVf+Ms0HfX1+BTzISYbc7DJvIA+n/LwE9QIlaD4XMaDgW5IruPAhf/kEaQv0L43KDcNIkLmjlT1kr1B4Wu+FEWi7nQbpkHCt6tLk0GwI2G2pbpR7feQrB0UpD/nvjzDVWM/ECARAEpdfwa4tQ4+5lVT1nOQZ6B7q5E3g+dceRDCX9WFKTPmjh6ZO/UX6ZAm0uIgEWgT42ONnb88J2WwcQa4elAFZgYokwnU7YDWOIqlQyufcVgypmxxFQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amdcloud.onmicrosoft.com; s=selector2-amdcloud-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=CZtVzDaRVDVDdBtJXnmx1pHKCCtX0OOuuOUl9Mbyf0U=; b=EfSNcpw8mY5FTUdN7dLIGLPoaWm5rYy33BVebDtpy7cGyMMcfMoDG2vnCqFzHFqX27Daht2TGntCgqQl1phGLPB/6dsL1YH2rVxYeRSrblBuA/ym8PmyiIFbffDtUj7yrpSfJeDavDOWohey9IiLgtp9IOC0mEIKYwHfdPehLAU= Authentication-Results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=amd.com; Received: from DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) by DM6PR12MB3372.namprd12.prod.outlook.com (2603:10b6:5:11b::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3239.17; Fri, 31 Jul 2020 14:44:55 +0000 Received: from DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::25ec:e6ba:197c:4eb0]) by DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::25ec:e6ba:197c:4eb0%8]) with mapi id 15.20.3239.020; Fri, 31 Jul 2020 14:44:55 +0000 Subject: Re: [PATCH v13 45/46] UefiCpuPkg/MpInitLib: Prepare SEV-ES guest APs for OS use From: "Lendacky, Thomas" To: Laszlo Ersek , devel@edk2.groups.io Cc: Brijesh Singh , Ard Biesheuvel , Eric Dong , Jordan Justen , Liming Gao , Michael D Kinney , Ray Ni References: <9d41b17caa823ab3f39f08464785afd0fd03578f.1596134638.git.thomas.lendacky@amd.com> <7be9efaf-3907-c29e-cfb4-52950104841f@redhat.com> <46286340-cc66-990f-a337-d807363d112e@amd.com> Message-ID: <83386de9-69e2-3a0c-8b32-6a14176ba64d@amd.com> Date: Fri, 31 Jul 2020 09:44:53 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 In-Reply-To: <46286340-cc66-990f-a337-d807363d112e@amd.com> X-ClientProxiedBy: SN4PR0501CA0073.namprd05.prod.outlook.com (2603:10b6:803:22::11) To DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) Return-Path: thomas.lendacky@amd.com MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from office-linux.texastahm.com (67.79.209.213) by SN4PR0501CA0073.namprd05.prod.outlook.com (2603:10b6:803:22::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3239.9 via Frontend Transport; Fri, 31 Jul 2020 14:44:54 +0000 X-Originating-IP: [67.79.209.213] X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: fc278c2f-3055-4355-6dac-08d835604a6c X-MS-TrafficTypeDiagnostic: DM6PR12MB3372: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:9508; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: aP518ezoiXkAALvzHar1+R/2C85L3uu5pFjrjpQ8w+rZsEZ4ElqiA6LWkaXesFKdHblGrsfSgWj+dH1sESIN79EeV4qNfwtxRy4rwXZTiALkcINmNd59suBSg75hZdiCjUD1qcOpPsj+5EGFWEhdNMiok7mu71xNLgix3bQ4bsNmAdQGbVHxXWOv3NwElIhq4QGb81O1tmRQDd0w4ZH+817Gp6Vc7QsjT32OqLcL0a8CFw25eiqULo6mDKIzKEXRq3/6O3o9KWXA7HnPa8cInlakwSUirLBXZqLMCGXwYNPVTqHqHi6Fz+17cC3GilWrR/rtv5rx+jpZWKqeVvJRvuGlvQ+KuqawY0rA0rX+qO256bUEoQW1SYY/Z+lOHFzgYUlLW7DGGhHz+08x6a8T4STKpfTnZi9SjQFDh3IPfW3bLQzmTdI6cSwXxyC/R7FqXAN0+Dh67hvcZLskJ4QHUQ== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM5PR12MB1355.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFTY:;SFS:(6029001)(4636009)(366004)(136003)(346002)(39860400002)(376002)(396003)(5660300002)(6512007)(86362001)(31696002)(66556008)(4326008)(31686004)(66946007)(19627235002)(83380400001)(66476007)(52116002)(956004)(966005)(2616005)(6486002)(2906002)(478600001)(45080400002)(8676002)(316002)(8936002)(30864003)(26005)(36756003)(54906003)(6506007)(16526019)(53546011)(186003)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: RQe41f5jLl+7sTXQOxt2t2Qnl+26nd/tWJ4E+43K7BVENGswKbz637Nm+RFdLEzYZnElhPk1hg32c/nu+7sk2kpbfXIXcTGj/m7Hf3O0B8Uk4Q0q4N+xRD0WMwBnbsy+gI2MzRpSzIz7zdbAtaTmuLl80UJ/jeO4v2ro2oVeiwN36iG3MNeYfMvB6Ll/21s8W2hz1rN1Shnil5khinD3PofE1k4Oyi4ehfnsR35JDxnPuV0cD0q7J7I3rEdfWfzljXe4KITjcB53TOp20F5bL3y5lgkMBF3lqz6TZ1KHZzCVoKpkyeW4vYOVzvrX9d+eZzsuZPG+A3OXoQd5RkIMRTSScBUva6BzADyOF0FYyPKeDthVkQndqNo9Fe0qplox5zh3yi6COHxyCkXUbK0TMvCEDNP+7wf7izmiEZcb83ReLyisUvuMG/EKp6mVwnfqsVjWvrUBJEQQrsSf+eHXURcZdPHuv6U8zkqmXCIYRwtQdp7Q1PWkfFpIDY3w9iwkQRRaHdNJ9BqQbXJP3XgUbDO56WDt1BQtroF5WnL8QdWqwTbSWq9AucxdYE3b57rSKU4aBsaLBx5Wm49TDl9ZZcIWv96TFywacU1ZFrFIiECX8fqhNbXLbtRVwa43DulNGzV03ghPfJvpqFUDIj6DGg== X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: fc278c2f-3055-4355-6dac-08d835604a6c X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1355.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jul 2020 14:44:55.8855 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: Br7FtrK0EAapa+Qjo0MTwYnN74IbsWDEikKJjKuffXaDTZV9NWTVxVFX0pm3Zh9vZwyYpSrlHHsk4gu5393oVQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB3372 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 7/31/20 8:36 AM, Tom Lendacky wrote: > On 7/31/20 7:43 AM, Laszlo Ersek wrote: >> Hi Tom, > > Hi Laszlo, Hi Laszlo, Can you try this incremental patch to see if it fixes the issue you're seeing? If it does, I'll merge it into patch #45 and send out a v14. Thanks, Tom diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c index 7165bcf3124a..2c00d72ddefe 100644 --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c @@ -365,9 +365,9 @@ RelocateApLoop ( MwaitSupport, CpuMpData->ApTargetCState, CpuMpData->PmCodeSegment, - CpuMpData->Pm16CodeSegment, StackStart - ProcessorNumber * AP_SAFE_STACK_SIZE, (UINTN) &mNumberToFinish, + CpuMpData->Pm16CodeSegment, CpuMpData->SevEsAPBuffer, CpuMpData->WakeupBuffer ); diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm index 309d53bf3b37..7e81d24aa60f 100644 --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm @@ -226,7 +226,10 @@ SwitchToRealProcStart: SwitchToRealProcEnd: ;------------------------------------------------------------------------------------- -; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish); +; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment, SevEsAPJumpTable, WakeupBuffer); +; +; The last three parameters (Pm16CodeSegment, SevEsAPJumpTable and WakeupBuffer) are +; specific to SEV-ES support and are not applicable on IA32. ;------------------------------------------------------------------------------------- global ASM_PFX(AsmRelocateApLoop) ASM_PFX(AsmRelocateApLoop): diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h index 267aa5201c50..02652eaae126 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h @@ -350,9 +350,9 @@ VOID IN BOOLEAN MwaitSupport, IN UINTN ApTargetCState, IN UINTN PmCodeSegment, - IN UINTN Pm16CodeSegment, IN UINTN TopOfApStack, IN UINTN NumberToFinish, + IN UINTN Pm16CodeSegment, IN UINTN SevEsAPJumpTable, IN UINTN WakeupBuffer ); diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm index 3b8ec477b8b3..5d30f35b201c 100644 --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm @@ -491,13 +491,13 @@ PM16Mode: SwitchToRealProcEnd: ;------------------------------------------------------------------------------------- -; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, Pm16CodeSegment, TopOfApStack, CountTofinish, SevEsAPJumpTable, WakeupBuffer); +; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment, SevEsAPJumpTable, WakeupBuffer); ;------------------------------------------------------------------------------------- global ASM_PFX(AsmRelocateApLoop) ASM_PFX(AsmRelocateApLoop): AsmRelocateApLoopStart: BITS 64 - cmp qword [rsp + 56], 0 + cmp qword [rsp + 56], 0 ; SevEsAPJumpTable je NoSevEs ; @@ -539,16 +539,17 @@ BITS 64 NoSevEs: cli ; Disable interrupt before switching to 32-bit mode - mov rax, [rsp + 48] ; CountTofinish + mov rax, [rsp + 40] ; CountTofinish lock dec dword [rax] ; (*CountTofinish)-- + mov r10, [rsp + 48] ; Pm16CodeSegment mov rax, [rsp + 56] ; SevEsAPJumpTable mov rbx, [rsp + 64] ; WakeupBuffer - mov rsp, [rsp + 40] ; TopOfApStack + mov rsp, r9 ; TopOfApStack push rax ; Save SevEsAPJumpTable push rbx ; Save WakeupBuffer - push r9 ; Save Pm16CodeSegment + push r10 ; Save Pm16CodeSegment push rcx ; Save MwaitSupport push rdx ; Save ApTargetCState > >> >> On 07/30/20 20:43, Tom Lendacky wrote: >>> From: Tom Lendacky >>> >>> BZ: >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&data=02%7C01%7Cthomas.lendacky%40amd.com%7Cb8c77cf296c949d2bbd808d8354f542b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637317962138028351&sdata=HISHZmLjOc%2FfgVrBm8MlNeDAk453NJ64%2B51bETZj4rk%3D&reserved=0 >>> >>> >>> Before UEFI transfers control to the OS, it must park the AP. This is >>> done using the AsmRelocateApLoop function to transition into 32-bit >>> non-paging mode. For an SEV-ES guest, a few additional things must be >>> done: >>> - AsmRelocateApLoop must be updated to support SEV-ES. This means >>> performing a VMGEXIT AP Reset Hold instead of an MWAIT or HLT loop. >>> - Since the AP must transition to real mode, a small routine is copied >>> to the WakeupBuffer area. Since the WakeupBuffer will be used by >>> the AP during OS booting, it must be placed in reserved memory. >>> Additionally, the AP stack must be located where it can be accessed >>> in real mode. >>> - Once the AP is in real mode it will transfer control to the >>> destination specified by the OS in the SEV-ES AP Jump Table. The >>> SEV-ES AP Jump Table address is saved by the hypervisor for the OS >>> using the GHCB VMGEXIT AP Jump Table exit code. >>> >>> Cc: Eric Dong >>> Cc: Ray Ni >>> Cc: Laszlo Ersek >>> Reviewed-by: Eric Dong >>> Signed-off-by: Tom Lendacky >>> --- >>> UefiCpuPkg/Library/MpInitLib/MpLib.h | 8 +- >>> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 54 +++++++- >>> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 131 ++++++++++++++++-- >>> 3 files changed, 175 insertions(+), 18 deletions(-) >> >> Now that this series is almost ready to merge, I've done a bit of >> regression-testing. >> >> Unfortunately, this patch breaks booting with IA32 OVMF. >> >> More precisely, it breaks the IA32 version of DxeMpInitLib. > > Yeah, that's not good. I will look into this based on your input below. > What's strange is that my system doesn't hang and successfully boots all > APs (up to 64 is what I've tested with). > > But, yes, both call sites should be the same and I will make that change. > >> >> The symptom is that just when the OS would be launched, the >> multiprocessor guest hangs. This is how the log terminates: >> >>> FSOpen: Open >>> '\370ac550dcaa48b88f1ca75ad903b0e7\4.16.7-100.fc26.i686\linux' >>> Success >>> [Security] 3rd party image[0] can be loaded after EndOfDxe: >>> PciRoot(0x0)/Pci(0x2,0x1)/Pci(0x0,0x0)/Scsi(0x0,0x0)/HD(1,GPT,D9F1FBA5-E5D3-440A-B6A7-87B593E4FAB1,0x800,0x100000)/\370ac550dcaa48b88f1ca75ad903b0e7\4.16.7-100.fc26.i686\linux. >>> >>> InstallProtocolInterface: [EfiLoadedImageProtocol] 853A03A8 >>> Loading driver at 0x00083E72000 EntryPoint=0x00083E76680 >>> InstallProtocolInterface: [EfiLoadedImageDevicePathProtocol] 853A0510 >>> ProtectUefiImageCommon - 0x853A03A8 >>> - 0x0000000083E72000 - 0x0000000000E75000 >>> FSOpen: Open >>> '370ac550dcaa48b88f1ca75ad903b0e7\4.16.7-100.fc26.i686\initrd' >>> Success >>> PixelBlueGreenRedReserved8BitPerColor >>> ConvertPages: range 400000 - 1274FFF covers multiple entries >>> SmmInstallProtocolInterface: [EdkiiSmmExitBootServicesProtocol] 0 >>> CpuDxe: 5-Level Paging = 0 >>> [HANG] >> >> Meanwhile some guest CPUs are pegged. >> >> Normally, when this series is not applied, the next log entry is (in >> place of [HANG]): >> >>> MpInitChangeApLoopCallback() done! >> >> I've identified this patch by bisection, after applying the series on >> current master (137c2c6eff67, "Revert "BaseTools/PatchCheck.py: Add >> LicenseCheck"", 2020-07-31). >> >> Here's the bisection log: >> >>> git bisect start >>> # good: [137c2c6eff67f4750d77e8e40af6683c412d3ed0] Revert >>> "BaseTools/PatchCheck.py: Add LicenseCheck" >>> git bisect good 137c2c6eff67f4750d77e8e40af6683c412d3ed0 >>> # bad: [d3f7971f4f70c9f39170b42af837e58e59435ad3] Maintainers.txt: Add >>> reviewers for the OvmfPkg SEV-related files >>> git bisect bad d3f7971f4f70c9f39170b42af837e58e59435ad3 >>> # good: [9551e3fc61ba0c0ddf8e79b425a22aa7dd61cb8b] OvmfPkg/VmgExitLib: >>> Add support for RDTSCP NAE events >>> git bisect good 9551e3fc61ba0c0ddf8e79b425a22aa7dd61cb8b >>> # good: [10acf16b38522d8a1b538b3aa432daaa72c0e97b] OvmfPkg: Reserve a >>> page in memory for the SEV-ES usage >>> git bisect good 10acf16b38522d8a1b538b3aa432daaa72c0e97b >>> # good: [ccb4267e76b6474657c41bef7e76a980930c22ea] UefiCpuPkg: Add a >>> 16-bit protected mode code segment descriptor >>> git bisect good ccb4267e76b6474657c41bef7e76a980930c22ea >>> # good: [94e238ae37505cfb081f3b9b4632067e4a113cf9] OvmfPkg: Use the >>> SEV-ES work area for the SEV-ES AP reset vector >>> git bisect good 94e238ae37505cfb081f3b9b4632067e4a113cf9 >>> # bad: [16c21b9d10b032d66d4105dd4693fd9dc6e6ec18] UefiCpuPkg/MpInitLib: >>> Prepare SEV-ES guest APs for OS use >>> git bisect bad 16c21b9d10b032d66d4105dd4693fd9dc6e6ec18 >>> # good: [49855596e383ab2aa6410fa060e22d4817d8e64e] OvmfPkg: Move the >>> GHCB allocations into reserved memory >>> git bisect good 49855596e383ab2aa6410fa060e22d4817d8e64e >>> # first bad commit: [16c21b9d10b032d66d4105dd4693fd9dc6e6ec18] >>> UefiCpuPkg/MpInitLib: Prepare SEV-ES guest APs for OS use >> >> So clearly we should be looking for an IA32-specific change, or >> IA32-specific *omission*, in this patch, that could cause the problem. >> >> The bug is the following: >> >> On 07/30/20 20:43, Tom Lendacky wrote: >>> >>> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h >>> b/UefiCpuPkg/Library/MpInitLib/MpLib.h >>> index b1a9d99cb3eb..267aa5201c50 100644 >>> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h >>> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h >>> @@ -349,8 +350,11 @@ VOID >>> IN BOOLEAN MwaitSupport, >>> IN UINTN ApTargetCState, >>> IN UINTN PmCodeSegment, >>> + IN UINTN Pm16CodeSegment, >>> IN UINTN TopOfApStack, >>> - IN UINTN NumberToFinish >>> + IN UINTN NumberToFinish, >>> + IN UINTN SevEsAPJumpTable, >>> + IN UINTN WakeupBuffer >>> ); >>> >>> /** >> >> (1) This hunk modifies the parameter list of functions pointed-to by >> ASM_RELOCATE_AP_LOOP. >> >>> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >>> b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >>> index 9115ff9e3e30..7165bcf3124a 100644 >>> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >>> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >>> @@ -330,17 +350,26 @@ RelocateApLoop ( >>> BOOLEAN MwaitSupport; >>> ASM_RELOCATE_AP_LOOP AsmRelocateApLoopFunc; >>> UINTN ProcessorNumber; >>> + UINTN StackStart; >>> >>> MpInitLibWhoAmI (&ProcessorNumber); >>> CpuMpData = GetCpuMpData (); >>> MwaitSupport = IsMwaitSupport (); >>> + if (CpuMpData->SevEsIsEnabled) { >>> + StackStart = CpuMpData->SevEsAPResetStackStart; >>> + } else { >>> + StackStart = mReservedTopOfApStack; >>> + } >>> AsmRelocateApLoopFunc = (ASM_RELOCATE_AP_LOOP) (UINTN) >>> mReservedApLoopFunc; >>> AsmRelocateApLoopFunc ( >>> MwaitSupport, >>> CpuMpData->ApTargetCState, >>> CpuMpData->PmCodeSegment, >>> - mReservedTopOfApStack - ProcessorNumber * AP_SAFE_STACK_SIZE, >>> - (UINTN) &mNumberToFinish >>> + CpuMpData->Pm16CodeSegment, >>> + StackStart - ProcessorNumber * AP_SAFE_STACK_SIZE, >>> + (UINTN) &mNumberToFinish, >>> + CpuMpData->SevEsAPBuffer, >>> + CpuMpData->WakeupBuffer >>> ); >>> // >>> // It should never reach here >> >> (2) This hunk modifies the call site, in accordance with the prototype >> change at (1). >> >>> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm >>> b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm >>> index 6956b408d004..3b8ec477b8b3 100644 >>> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm >>> +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm >>> @@ -465,6 +465,10 @@ BITS 16 >> >>> ; - IP for Real Mode (two bytes) >>> ; - CS for Real Mode (two bytes) >>> ; >>> + ; This label is also used with AsmRelocateApLoop. During MP >>> finalization, >>> + ; the code from PM16Mode to SwitchToRealProcEnd is copied to the >>> start of >>> + ; the WakeupBuffer, allowing a parked AP to be booted by an OS. >>> + ; >>> PM16Mode: >>> mov eax, cr0 ; Read CR0 >>> btr eax, 0 ; Set PE=0 >>> @@ -487,32 +491,95 @@ PM16Mode: >>> SwitchToRealProcEnd: >>> >>> >>> ;------------------------------------------------------------------------------------- >>> >>> -; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, >>> TopOfApStack, CountTofinish); >>> +; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, >>> Pm16CodeSegment, TopOfApStack, CountTofinish, SevEsAPJumpTable, >>> WakeupBuffer); >>> >>> ;------------------------------------------------------------------------------------- >>> >>> global ASM_PFX(AsmRelocateApLoop) >>> ASM_PFX(AsmRelocateApLoop): >>> AsmRelocateApLoopStart: >>> BITS 64 >> >> (3) Unfortunately, the patch only adapts the X64 implementation of the >> AsmRelocateApLoopStart() function to the new prototype; the IA32 >> implementation no longer matches the call site. >> >> (I'm not sure if the intent was for the IA32 version to simply ignore >> the new parameters, but even in that case, the "Pm16CodeSegment" >> parameter is inserted in the middle of the parameter list, likely >> offsetting the rest.) >> >> The problem is foreshadowed even by hunk (2). Namely, in hunk (2), the >> >> s/mReservedTopOfApStack/StackStart/ >> >> replacement is *more difficult* to verify than necessary -- exactly >> because "CpuMpData->Pm16CodeSegment" is inserted *before* it. > > I can do one of two things here and just put the 3 new parameters at the > end of the function call rather than keeping the code segment parameters > together or update the IA32 call site. Let me see which looks best. But > I'll likely update the IA32 call site no matter what with at least > comments about the parameters that aren't used, either way. > > Thanks, > Tom > >> >> Thanks >> Laszlo >>