From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (NAM12-BN8-obe.outbound.protection.outlook.com [40.107.237.80]) by mx.groups.io with SMTP id smtpd.web10.20017.1596206869984524884 for ; Fri, 31 Jul 2020 07:47:50 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@amdcloud.onmicrosoft.com header.s=selector2-amdcloud-onmicrosoft-com header.b=v3zyPQ2N; spf=none, err=SPF record not found (domain: amd.com, ip: 40.107.237.80, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=AwfZhUGQT38fd4r6x1AIuozhEJ+tIJDeKnnymPA4EfBTc9K0s8q5b4+SxkgAaLZPC4TD0W5Ph6kNxgZgENH/iOpc/dE6Rz0QUMZddl296sITosPdCDvdy30uPhPyR1KKoQ6Zg3jjFGdsG67h4o98EI31MhP0cr/LblA3vKCHSc3pcgqpqgYlu1bdM4059EhxcrDGrAHAtHIZlV0Kt3VN4rFYabSaZakzuv2WwIfmaA/AphdEky1lq8+ILHPxhVsP+nd5cMqfPx9oBWkJYgk3R7Lv36BiTeEnNoJqvErCsU74knQlJNzqSGeRgPObGNsxCDAzuz4FXRj3AYuYvi9vWA== 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=iufxMmL3C4zCJ+vwuggaWGTEc+sRPY0vIjzfnDlulYA=; b=YmH8mUDxo0XF1rJYaNljgQ7ZJNbZ3CqJUtutDb6fc/tIrlQnRK0FD1fOSozGvA8KkS/Bx9rmw/wK1BVubNCPVBXQr/Q2YDiKoCPgMvSZNuND0f/bm0exhAr/ed465W3/CEQQrD86prp9YqVr/1hXgxNKCH2PupSGZqF7XJLmmanh+2Y92FaRaOPy3vG9G7NYi2ohmBhPZ92B4b2ynFhaZQ2QFzCvZEJOfjmNRATiduNrModv8eDHWuC0jxo1KtcQqu99X8pQDGhhl+0tWyqkR6ucbZFATl3P4tU6J1wMRNc2ktY1Li9jm6eFZtVYZIXSHS6t8O+l0E1ijutB3kMlPQ== 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=iufxMmL3C4zCJ+vwuggaWGTEc+sRPY0vIjzfnDlulYA=; b=v3zyPQ2NxHRTswqNkv7MQLbRe38Y1Bdi0f3C+R8NI/N6QlmhIaPw2e+DlvivtCcil+8LgTJsuHUZ/YFgohsLGxgXQWoRRrU1Ur99vfR3FO/g6YBK72ORYN3xI62gHnq3vaPVelexT0jYp0cvU0wocRzue2yBa2WCzdYwK3XvtCU= 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 DM5PR12MB2501.namprd12.prod.outlook.com (2603:10b6:4:b4::34) 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:47:48 +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:47:48 +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> <83386de9-69e2-3a0c-8b32-6a14176ba64d@amd.com> Message-ID: Date: Fri, 31 Jul 2020 09:47:46 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 In-Reply-To: <83386de9-69e2-3a0c-8b32-6a14176ba64d@amd.com> X-ClientProxiedBy: SN1PR12CA0104.namprd12.prod.outlook.com (2603:10b6:802:21::39) 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 SN1PR12CA0104.namprd12.prod.outlook.com (2603:10b6:802:21::39) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3239.18 via Frontend Transport; Fri, 31 Jul 2020 14:47:47 +0000 X-Originating-IP: [67.79.209.213] X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: 9fdaae02-6146-406f-9ef3-08d83560b157 X-MS-TrafficTypeDiagnostic: DM5PR12MB2501: 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: 2wUXGs8GujBhJcjUeqlWPN6z/MzmqAKpCVXV9eIdsIk/+QYNw4ds4ev9tUOv/sHKHeLOYWK5k8IgjI4ntW8NT1uGObfOeDZ4roEzlAZOZy6gvU32FAWwiW8p3gF+9rmBw9wV9ZqX8UGUq0nejpw8BAkujLTEyhYAYHfeUnYr/NV2PQqJwV53WQT9g0Ttq7oSY/rgMSfBV3SJikyJO1XPEnslXHFbmE4UeB3xzEFyIn082QrAlC9DGc20+ZLgJ7GlfKIq5O31oa5BcEoGz9YdEiqaoM7EBvBzg9dv+v/8ARKBBvkSR2Y6r2Eb8iRddZswOL6q3z2tI2xL4vuv6Ob/9HYJHVGMXnqTko6bisQauhP2IsRmJXDARmSe81D3QXjIrPWXPilT/+MbkpT+yvVOUqZpriCPpZ4E2N+d3Gn/V9ZPb8k/l+ib6KmSlkQNgSlM1wmoWmRyWXTitDM7jGk3fg== 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)(39860400002)(136003)(366004)(376002)(396003)(346002)(8936002)(956004)(2616005)(6506007)(53546011)(16526019)(186003)(26005)(19627235002)(2906002)(52116002)(31686004)(36756003)(83380400001)(8676002)(6512007)(54906003)(45080400002)(966005)(66946007)(5660300002)(316002)(31696002)(478600001)(66556008)(66476007)(86362001)(6486002)(4326008)(30864003)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: oiXUxPPdxcHD3oWbX5RDw77Ryo8+DOk//n0leGKXFbcIfwD87UtOcrwVQIi3XRUncGYrthaBcAhkD4uGKgTIR1u1dsn6iCBDl1pd2iRbIFCpFy1cXMQ0vq/6X5K5k4/zhNghdiJzo1MiBk6MKCouY37Pg9jhVaYqQIdrQDNg93eBbinqYsccfm0XZ6J8JCbYiZTKG2w3+cTuUj2/hLcgpkAzzkwU1uyzhs5pAt8efRGfSHKgX5oAzCvteMx5f4LR6SZWPEBtZuo0aZFWYIvLCjbCskRllCnS2em+XS9PsfpWXe9z7C0JaD3aV6+7k0HkYzDFeZLUx547AYXQF5PwTiZKxo5GwNzwDHaWCWTHGlMUgtzqz8y8NUtO3YLd0NwClB8dZLNn9MCcN8X9qQhNJxFHJxz5YJT4iedsJR42yTkF0XQlBYifN6oWo/uC7X6uG7SNV/Mx/5IssbVJGlbM7y785rCmCwVopghWGxi1KxX+1Gc+rwqNiwvCia9kNaL8Y5N7s+3AADig8wACrfEenE2cydy7eMOLda954qtMA7tmK8ZWaZdgJzre0XmdC3bXh4KBh/QytXN2+Q+ol6t/fS7l4V4LH353ok2DPyVcIYAIJjs6DC+yo3ZlPLZu/ZstciCMTEqDLBpjk/MQVkCsBw== X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 9fdaae02-6146-406f-9ef3-08d83560b157 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:47:48.3396 (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: ToD2jSqylExXkH5Kaay6YxQpL4+MxO6/zmofprk9IP2OGksIeoA78nY76sUApp0L3rTCtNWd8GVaHiSQwPZftQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR12MB2501 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 7/31/20 9:44 AM, Tom Lendacky wrote: > 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. Looking at the formatting, I'm not sure if Thunderbird messed up the diff. I'll send you another copy directly to you using git send-email just in case. Thanks, Tom > > 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 >>>