From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM02-SN1-obe.outbound.protection.outlook.com (NAM02-SN1-obe.outbound.protection.outlook.com [40.107.96.46]) by mx.groups.io with SMTP id smtpd.web11.8754.1620999220174449048 for ; Fri, 14 May 2021 06:33:40 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@amd.com header.s=selector1 header.b=I6e/ICvH; spf=permerror, err=parse error for token &{10 18 %{i}._ip.%{h}._ehlo.%{d}._spf.vali.email}: invalid domain name (domain: amd.com, ip: 40.107.96.46, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=QEtu13/626BWnwKacJCW1YOySWuWYMhqiW7KmToDnu93Zbj7bDqtYVSzZmr5Lfdso2L+v/RRKZYmEQNSL5i31+uivI5I+U0RGmRHisfimYXiytoATn/PPeBnF2TtvfZfEIKiZ3rK7xkO5ts68ZR9fmHPcOtRYiv/CJ2WnmwtNZQyZ2e7rWc2QDEJRAR0xIQOZtuXGNQOJgLJQScVcvkT50NlKD3ebGfNi0uj8yC7VQqAnfjlrBcNzPlFgag5W4dXjhmSTEvYiu+hRilQsRC4hiXKpc3G5GgUmxWD3tRc41rQMdwSpn7iEsL+ncfSdQvLIyrcl7LkgkHuepTkXgSzPQ== 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=NfkKG4il0aB3ixD6zcCoAZZc/pjFzG1gHbKGpnO92rw=; b=j50R6CcEcKU5WN2Kqa5dgadjXlcPuoDPscjF8VXJY+7yVIv14hxphDU0wy//MeFVkRJUcvb1ahOrHM8xfi1LhIEoEGzmZy+lSu9WnR+pTFlHW/xECFs2pzyR47FDMxePzTUdySTOYhX7zDRjYSXW/uBfsQjZ/CHjSM88FH/8xJt5GafyJOfF3HOEEM5+phT0Ly+nx2u3AeuZSfLWzILqPhHNsuuwREhtTu03zNNwh2CmhMKOiP0ZzV1nnw/Fh8o72r5cCyoWeWcLh2+S+kwwOzScyMiwCujisK/yoK78Mss15NxPl6DZdW0z7JEMnpLimD2zcNNqwwRfwK+Zx4KiTA== 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=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=NfkKG4il0aB3ixD6zcCoAZZc/pjFzG1gHbKGpnO92rw=; b=I6e/ICvHUeCE03TcCzVfeC7kQ7l1c6KfXansbXIhIbo867Fk3W6bw/am2sDTpA9oR8XiHtzStxf7gaym/hgJFDFfRf3P1bI9LFZX+JUSKlXbY/8RgL1dYsX4lICNXCCH12Y7YqCIdu7rZA1iYRwHAc9A1+Gy8+a7mitOmXIgIuU= 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 DM6PR12MB4516.namprd12.prod.outlook.com (2603:10b6:5:2ac::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4129.25; Fri, 14 May 2021 13:33:38 +0000 Received: from DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::b914:4704:ad6f:aba9]) by DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::b914:4704:ad6f:aba9%12]) with mapi id 15.20.4129.025; Fri, 14 May 2021 13:33:38 +0000 Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg/MpInitLib: Allocate a separate SEV-ES AP reset stack area To: Laszlo Ersek , devel@edk2.groups.io Cc: Brijesh Singh , Eric Dong , Ray Ni , Rahul Kumar References: <1162d1f4fe09048aaafba6f6ea3046bebd34fc2b.1620766224.git.thomas.lendacky@amd.com> <0ac5ccbf-98ce-5288-e6d4-d692b4855272@redhat.com> From: "Lendacky, Thomas" Message-ID: <5d7133d0-d933-a5d3-af0c-e21275d78176@amd.com> Date: Fri, 14 May 2021 08:33:36 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 In-Reply-To: <0ac5ccbf-98ce-5288-e6d4-d692b4855272@redhat.com> X-Originating-IP: [67.79.209.213] X-ClientProxiedBy: SN6PR01CA0030.prod.exchangelabs.com (2603:10b6:805:b6::43) 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 SN6PR01CA0030.prod.exchangelabs.com (2603:10b6:805:b6::43) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4108.25 via Frontend Transport; Fri, 14 May 2021 13:33:37 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 10b699af-731d-488d-7583-08d916dce17b X-MS-TrafficTypeDiagnostic: DM6PR12MB4516: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: TTR5SWgYUNbe6vJC8Hj2hYLtrCt/PF5nFwEjjJNTMPFRvZu7nEa/tXEBJyx3onLnIhCXkiQ43Cp2E4VLdk+26n0adEL8og/iGS481xAjDk4xRZA5Sk11Dmc8J9W7zpiZdgKcRX032UKAplLFMA9CEIUCvtW8D3ayhEM9oVcGxEqju6L8IjT+FxTqmmk/PoODlYh+R0mncPErYYM8vmd/phXwtfNnnQo+0AgZ4q4jSCJ8Q7l0dcJA2SN3E96LeiKumYxLbEfj2hIiq2s03f0aDkTOY0IQPg4CHOeHEalpIItXEhGtpSOuBtYsi4682fsClUXGDg3MTD0fKsry4Pv1pXpdZ8rYq+iFhhyUqP1TTfg5II4OSujEBMeG0ahcIryadKkiLXo7KlFn7XIleJ4VOe4X8nn9t8Gtugs36O5h+H8pHh55q1vXSO9yDeAEWn65SimMVT4hNzB5sQbTny9iuFtbVcihkmsmRs5E6Qy6euz1gTHCOl00ek15IitKsLlMza4kZavMVtHJKwslwsJ3Zin9QH+IWetnYaWaHE08ZbHgq1MXbAJceDJ7MJFttaf+DFxRaICjvv3pfipQ/Rtgx7OB6C9F/egQ9quC5ou0U7ZwCSg3Ky3zT403UHai3XNW6SSZFcUaHt1Z6UJ1PolAXBWf6Pl/95OxlCu695v6VaXKOgZUSfy9GyoF+SLTGOESWzCBpAQ2blr3FfxOE3klpqocPp/bZBWye7nNbs0Mx/+LAqX4vt5gKxYyHDRcbHM7 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;SFS:(6029001)(4636009)(346002)(39840400004)(366004)(396003)(136003)(376002)(4326008)(53546011)(6506007)(31686004)(66476007)(66556008)(54906003)(66946007)(86362001)(186003)(31696002)(45080400002)(83380400001)(26005)(316002)(2906002)(38100700002)(8936002)(956004)(2616005)(966005)(6512007)(478600001)(6486002)(8676002)(36756003)(16526019)(5660300002)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?YS9yeTJ4VklKQjE0Szdnbnp0eDgvUmFXcWlFNlhzdUxTNmFucHQ3aVgxeFMx?= =?utf-8?B?T3crditmc0xnd0crcTRKOFM1eTczQWJHdVk5QVZHY0w1OHdTVHVseDB3QmlG?= =?utf-8?B?dUV6aE1iMFFJSlhLQjVtcURxV3czUWZWejcwNll0aG84VmJ4Z1lZcmRaNnNV?= =?utf-8?B?cHlOTk45Z3dYMmFsYWI5YXRRNnRMb004RFpIaVg1NXpYSnJZV0IwanJETWd6?= =?utf-8?B?NUhkUjh3eksvTWFicVo2ano4b2t2N3p5ZUJDRXVKdUJMdGdJYjRzNVNRTTFv?= =?utf-8?B?UHJKRW9sTXEvMFN2Wk9jczVBbGpyZWw5ZFRNRXVGTlc1SzI3bENJSE1UY1hC?= =?utf-8?B?bWZVL3FBZzZ4T3VwRmo3UkZlZlRiOE9UbTRrSHJlVFRKN1YzdWlmc0Q0Kzdz?= =?utf-8?B?SHFSZ2ZaQmE5TUhtdGtGOFN6NTJwOW9pS1RZVDZycFBxejdPWmhZT0tNSW1k?= =?utf-8?B?ZFVjOU1zMEIwWDBwYU5aWHNURStiVmw3cWxMQzdyaTYwWm5ZVzh5R3dnME1p?= =?utf-8?B?eWpJcnZoYnE1Ynlzak5TMXNwYzJYNmFjWFBNUTExZmx4V3BVMWYraHpuejY5?= =?utf-8?B?bUVrajJLMDhlTXJJRVBiVEVFR0lRaTR2TkUzUEZUTHdvclNtQnhNckt3TXhv?= =?utf-8?B?K3VVelhlYkJpb2tDdFVQbXF2MzlsRER6bGdDd05lT2FiVGRKY1o4bm1NVjZn?= =?utf-8?B?V0VwMU14c0dVaHBhclN6ZW9aUXJLb0FnM01Fa0MwZlh5L1pmaC8xRE1vYVBs?= =?utf-8?B?Z01DNzZ6bzJUdk0vbmFhVkhTWFR1SmR5Ym1qZzRrRFdMOHVobUlVTC9DK0d3?= =?utf-8?B?ZkFCVm1KeGd3UldNMHZtakN4YzRVcThzc04wemY3SFVuWWR4bTJOUVBubjFn?= =?utf-8?B?bWJSU3Y0VjcxNmxPL1EzdU5SUFBXMUpMblBGcStiU1NmTjJCN1dzTnRha2N2?= =?utf-8?B?dDJsOFFKZjE5ZFIzMzBvSTkraTlCYmNnMFdFSENMNG5XOTdlTERKaDZZNEZT?= =?utf-8?B?OGNBeXZwZmdXYzBkODJ3ZC9LZmRNdzhWVkk5azQ3ekFOcm0wcHdjVGV0di9K?= =?utf-8?B?b1Nja0tYVENkSkwyR1h1QjlIRXRkWUNib0Vnc2FWdFVsK0VxNkJESnU1TzJV?= =?utf-8?B?VXA2Z1VVTzlOUnV2TENkT29vclJWSGZwaDBPNUxIT3VNNXFBSE5sL08zR0gz?= =?utf-8?B?SHNlNzRZei9lelJEb1UrTHlXUy8za3pSOVY5bEdNVkY1WEl3MlZyU1p0dm1l?= =?utf-8?B?aUxVZzNCVDZxcHJkc1Y0NVhmVVMvTk11UGlzMCs2d3JNcmtjdFF0RzJuVjhV?= =?utf-8?B?ZjdyTlJRTnlzaVllb0RXeHh3WEVXQmd3RnplZnIzcDFMems3UjE0clJhL0J4?= =?utf-8?B?U2RtRUlpTFNyZFl4dTBNaUEyYVZKSGNMZ2JDUHBRdnNMTkZLOC9sK1k4UUp5?= =?utf-8?B?THVyTFA3SWVSQmdSRzBhL05wUmVhOGNIY29DYXpkOGRjQUd5Vm9PQ3k5NEdD?= =?utf-8?B?a2ZOQ09tcFNaaXNjWWRCVkllTkVFeFBQcGpSRDNEVHpQRWZOSVEyYmV3Mmg1?= =?utf-8?B?Z1FzU29HOThzN3Q3cnVnK29SSzAwSzZJUmluTFpvWFM2c2pSY1dDMWNoNHl4?= =?utf-8?B?N3g4V2lLSGFzOC9jMTZUWE9xM0RMWnR0a0J5L01BTmxqN1J1L1Ava2hDby80?= =?utf-8?B?ekQ4aU1VdjJrVFRVTFNCQ0FlSmM5akZsT2RrV3FnRTVZYjlUL2RZY0FZUjBF?= =?utf-8?Q?uqIScOQ9/6SLIEM6cdv5lWx7zHXeeABQPBxPeag?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 10b699af-731d-488d-7583-08d916dce17b X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1355.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 May 2021 13:33:38.4175 (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: pk48BuR3oc5rDlrKAhh+yytVOUGKkFvxLy5lTuSLf0iFjtHVzmL/Jrwsz9cVjEM81aDB7kQ/nxNmG8f6iQefSQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4516 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 5/14/21 4:14 AM, Laszlo Ersek wrote: > On 05/11/21 22:50, Lendacky, Thomas wrote: >> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3324&data=04%7C01%7Cthomas.lendacky%40amd.com%7C7c28b41e27cc4b9a8b9508d916b8a955%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637565804655837525%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Ixw2PdtFLryJs9KHplJ8bvomtaqjBJF8KuDdWO5ERdw%3D&reserved=0 >> >> The SEV-ES stacks currently share a page with the reset code and data. >> Separate the SEV-ES stacks from the reset vector code and data to avoid >> possible stack overflows from overwriting the code and/or data. >> >> When SEV-ES is enabled, invoke the GetWakeupBuffer() routine a second time >> to allocate a new area, below the reset vector and data. >> >> Both the PEI and DXE versions of GetWakeupBuffer() are changed to track >> the previous reset buffer allocation in order to ensure that the new >> buffer allocation is below the previous allocation. >> >> Fixes: 7b7508ad784d16a5208c8d12dff43aef6df0835b >> Cc: Eric Dong >> Cc: Ray Ni >> Cc: Laszlo Ersek >> Cc: Rahul Kumar >> Signed-off-by: Tom Lendacky >> --- >> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 12 ++++- >> UefiCpuPkg/Library/MpInitLib/MpLib.c | 48 +++++++++++++------- >> UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 14 ++++-- >> 3 files changed, 54 insertions(+), 20 deletions(-) >> >> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >> index 7839c249760e..fdfa0755d37a 100644 >> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >> @@ -29,6 +29,11 @@ VOID *mReservedApLoopFunc = NULL; >> UINTN mReservedTopOfApStack; >> volatile UINT32 mNumberToFinish = 0; >> >> +// >> +// Begin wakeup buffer allocation below 0x88000 >> +// >> +STATIC EFI_PHYSICAL_ADDRESS mWakeupBuffer = 0x88000; > > (1) Please rename this to mSevEsDxeWakeupBuffer. The code is correct > as-is, but regarding code editors that can jump to tags, it helps if we > eliminate duplicate identifiers. Ok, will do here and in the PEI file. > >> + >> /** >> Enable Debug Agent to support source debugging on AP function. >> >> @@ -102,7 +107,7 @@ GetWakeupBuffer ( >> // LagacyBios driver depends on CPU Arch protocol which guarantees below >> // allocation runs earlier than LegacyBios driver. >> // >> - StartAddress = 0x88000; >> + StartAddress = mWakeupBuffer; >> Status = gBS->AllocatePages ( >> AllocateMaxAddress, >> MemoryType, >> @@ -112,6 +117,11 @@ GetWakeupBuffer ( >> ASSERT_EFI_ERROR (Status); >> if (EFI_ERROR (Status)) { >> StartAddress = (EFI_PHYSICAL_ADDRESS) -1; >> + } else { >> + // >> + // Next wakeup buffer allocation must be below this allocation >> + // >> + mWakeupBuffer = StartAddress; >> } >> >> DEBUG ((DEBUG_INFO, "WakeupBufferStart = %x, WakeupBufferSize = %x\n", > > (2) Please make these changes conditional on "PcdSevEsIsEnabled". If the > PCD is false, the behavior of the function shouldn't change at all -- > not just the caller-observable behavior, but the internal behavior either. > > For the SEV-ES disabled case, it's much easier to see that the change is > regression-free if we don't just rely on GetWakeupBuffer() not being > called for a second time, but explicitly make the patch so that it does > nothing here if the PCD is false. > > Something like: > > if (PcdGetBool (PcdSevEsIsEnabled)) { > StartAddress = mSevEsDxeWakeupBuffer; > } else { > StartAddress = 0x88000; > } > > ... > > if (EFI_ERROR (Status)) { > StartAddress = (EFI_PHYSICAL_ADDRESS) -1; > } else if PcdGetBool (PcdSevEsIsEnabled) { > mSevEsDxeWakeupBuffer = StartAddress; > } > Will do. > >> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c >> index dc2a54aa31e8..a76dae437606 100644 >> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c >> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c >> @@ -1164,20 +1164,6 @@ GetApResetVectorSize ( >> AddressMap->SwitchToRealSize + >> sizeof (MP_CPU_EXCHANGE_INFO); >> >> - // >> - // The AP reset stack is only used by SEV-ES guests. Do not add to the >> - // allocation if SEV-ES is not enabled. >> - // >> - if (PcdGetBool (PcdSevEsIsEnabled)) { >> - // >> - // Stack location is based on APIC ID, so use the total number of >> - // processors for calculating the total stack area. >> - // >> - Size += AP_RESET_STACK_SIZE * PcdGet32 (PcdCpuMaxLogicalProcessorNumber); >> - >> - Size = ALIGN_VALUE (Size, CPU_STACK_ALIGNMENT); >> - } >> - >> return Size; >> } >> > > This change is clearly regression-free in case the PCD is false. OK. > >> @@ -1207,9 +1193,39 @@ AllocateResetVector ( >> CpuMpData->AddressMap.ModeTransitionOffset >> ); >> // >> - // The reset stack starts at the end of the buffer. >> + // The AP reset stack is only used by SEV-ES guests. Do not allocate it >> + // if SEV-ES is not enabled. >> // >> - CpuMpData->SevEsAPResetStackStart = CpuMpData->WakeupBuffer + ApResetVectorSize; >> + if (PcdGetBool (PcdSevEsIsEnabled)) { >> + UINTN ApResetStackSize; > > (3) While I very much like inner-block-scoped variables, and use them > heavily in ArmVirtPkg and OvmfPkg, unfortunately they are not tolerated > in the rest of edk2. Please move this variable to the top of the > function, and if necessary, put "SevEs" in its name. Will do. > >> + >> + // >> + // Stack location is based on ProcessorNumber, so use the total number >> + // of processors for calculating the total stack area. >> + // >> + ApResetStackSize = AP_RESET_STACK_SIZE * >> + PcdGet32 (PcdCpuMaxLogicalProcessorNumber); > > (4) A bit more idiomatic way to break this up: > > ApResetStackSize = (AP_RESET_STACK_SIZE * > PcdGet32 (PcdCpuMaxLogicalProcessorNumber)); > > (indented similarly to the controlling expressions of "if", "while" ...) Ok. > >> + >> + // >> + // Invoke GetWakeupBuffer a second time to allocate the stack area >> + // below 1MB. The returned buffer will be page aligned and sized and >> + // below the previously allocated buffer. >> + // >> + CpuMpData->SevEsAPResetStackStart = GetWakeupBuffer (ApResetStackSize); >> + >> + // >> + // Check to be sure that the "allocate below" behavior hasn't changed. >> + // This will also catch a failed allocation, as "-1" is returned on >> + // failure. >> + // >> + if (CpuMpData->SevEsAPResetStackStart >= CpuMpData->WakeupBuffer) { >> + DEBUG ((DEBUG_ERROR, >> + "SEV-ES AP reset stack is not below wakeup buffer\n")); > > (5) Indentation: > > DEBUG (( > DEBUG_ERROR, > "SEV-ES AP reset stack is not below wakeup buffer\n" > )); > > (The condensed style you used is superior IMO, and I encourage it in > ArmVirtPkg and OvmfPkg, but it's not universally accepted in the rest of > edk2.) > Heh, I was trying to figure this one out and seeing multiple forms. I'll update it. > >> + >> + ASSERT (FALSE); >> + CpuDeadLoop (); >> + } >> + } >> } >> BackupAndPrepareWakeupBuffer (CpuMpData); >> } >> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >> index 3989bd6a7a9f..4d09e89b4128 100644 >> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >> @@ -11,6 +11,8 @@ >> #include >> #include >> >> +STATIC UINT64 mWakeupBuffer = BASE_1MB; >> + >> /** >> S3 SMM Init Done notification function. >> > > (6) Please rename this to mSevEsPeiWakeupBuffer. (Same argument as in (1).) Yup. > >> @@ -220,11 +222,11 @@ GetWakeupBuffer ( >> // Need memory under 1MB to be collected here >> // >> WakeupBufferEnd = Hob.ResourceDescriptor->PhysicalStart + Hob.ResourceDescriptor->ResourceLength; >> - if (WakeupBufferEnd > BASE_1MB) { >> + if (WakeupBufferEnd > mWakeupBuffer) { >> // >> - // Wakeup buffer should be under 1MB >> + // Wakeup buffer should be under 1MB and under the previous one >> // >> - WakeupBufferEnd = BASE_1MB; >> + WakeupBufferEnd = mWakeupBuffer; >> } >> while (WakeupBufferEnd > WakeupBufferSize) { >> // > > (7) Can we use a WakeupBufferLimit helper variable here, and set it like > "StartAddress" under (2)? Will do. > >> @@ -244,6 +246,12 @@ GetWakeupBuffer ( >> } >> DEBUG ((DEBUG_INFO, "WakeupBufferStart = %x, WakeupBufferSize = %x\n", >> WakeupBufferStart, WakeupBufferSize)); >> + >> + // >> + // Next wakeup buffer allocation must be below this allocation >> + // >> + mWakeupBuffer = WakeupBufferStart; >> + >> return (UINTN)WakeupBufferStart; >> } >> } >> > > (8) And IMO I think we should adjust mSevEsPeiWakeupBuffer here > conditionally on the PCD as well. Will do. Thanks, Tom > > Thanks! > Laszlo >