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.81]) by mx.groups.io with SMTP id smtpd.web11.16921.1600954218986817283 for ; Thu, 24 Sep 2020 06:30:19 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@amdcloud.onmicrosoft.com header.s=selector2-amdcloud-onmicrosoft-com header.b=AwL56qIz; spf=none, err=SPF record not found (domain: amd.com, ip: 40.107.237.81, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VhSwgP9yODd5TCThWFRLmLBE6jd+tvyKl9OBE6d4pxZWuAkabUuceewxZ0vU0bjhdu/tV4n3ea4CT9DFDctevARAMo3orAHtl65bctwMFSDWKzPlKliotn9eeGXvg5DsgBA3mlVAxPRRO+6VdFqrWi/8gOfhZbSrSEE9pOTBNjRvW/d7jysG/I+yrjtn+XxQBx8Huue7AqhS6nmbujODqcxAXnKwI64ZwvWb2bw3Qa7jq37eEQjncvkMcqBWjvYbPeXia1pMKWTZbjG4cwBM4Thasw+YxnEsNr8eSbxocLBJe6xjqc2w0U9yU78qPzi+EdlnDgRd+Lr1QJuUpuzqGA== 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=6desbGB/w53/gJCEYXeIReGrsjuT+sC1ggiwT+j/xd4=; b=etNcqhoZ+Bvmgzz5HuPPgwPZxJAvfuRA2Hi7SWxxzeToWGvqH4JIC3q9+PxMK8gPFYBe47dpt/c+2S2Vjsc0Y3/Hgb4KWAjMtPAUDbB3epvmpTwotc44YpXK9Wj15O4jdQRn1Th1eqkK56DFzg2qeVn7WJM+zymyxy0X24S4A2zRGfgl97o8lni8y9PI+eWCRYG8dzbyIIWPNCWwYbcMfxFBt8csbadiXzK77a/mNqiKDivLMEUPpRb/jgtSghaGjAPlIFBadvrRKW9+z/PtPOnAPOzs7N8rskQXZ7ALNZH5E9NwdNvTu4oFahCt6ZJdv/N7TZVdlhe6tygha0n9wA== 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=6desbGB/w53/gJCEYXeIReGrsjuT+sC1ggiwT+j/xd4=; b=AwL56qIzQ9eqpU93H3TEeSQS9UvVV1ycgpcv2hpvb41Tz4AaLjfyHfh779GhxPqwyws5TnuKNp1GMMFkiQzY2BX/++t2omIdvjtiy6QgVx0QvQC6CEGT19UnueP4hBaEvRtOrWakQMdr2wm/5kFJ6Ntor4myCx7d4ooEDfAvyaI= Authentication-Results: amd.com; dkim=none (message not signed) header.d=none;amd.com; dmarc=none action=none header.from=amd.com; Received: from DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) by DM5PR12MB1162.namprd12.prod.outlook.com (2603:10b6:3:72::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3412.22; Thu, 24 Sep 2020 13:30:16 +0000 Received: from DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::299a:8ed2:23fc:6346]) by DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::299a:8ed2:23fc:6346%3]) with mapi id 15.20.3391.024; Thu, 24 Sep 2020 13:30:16 +0000 Subject: Re: [PATCH v2 1/1] UefiCpuPkg/MpInitLib: Reduce reset vector memory pressure To: Laszlo Ersek , devel@edk2.groups.io Cc: Eric Dong , Ray Ni , Rahul Kumar , Brijesh Singh , Garrett Kirkendall References: <21345cdbc906519558202b3851257ca07b9239ba.1600884239.git.thomas.lendacky@amd.com> From: "Lendacky, Thomas" Message-ID: <6f1b41f0-fa17-7cb5-8ee2-3fc66b58a25c@amd.com> Date: Thu, 24 Sep 2020 08:30:14 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 In-Reply-To: X-ClientProxiedBy: SN6PR01CA0033.prod.exchangelabs.com (2603:10b6:805:b6::46) 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 SN6PR01CA0033.prod.exchangelabs.com (2603:10b6:805:b6::46) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3412.22 via Frontend Transport; Thu, 24 Sep 2020 13:30:15 +0000 X-Originating-IP: [67.79.209.213] X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: 0eb5cdcb-7af1-41ad-b2e6-08d8608df965 X-MS-TrafficTypeDiagnostic: DM5PR12MB1162: 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: aPdIrkc5wgAc4kElIZLuo3I66qDGt3BPnE6/aRWOl7i6n8NHRYZR0T3a/rV5TBDeW+1OoRSR9bW1nNxlTgvn7AW0OFsVJhBL55EjUz7p8ou/GP23myfaQXd5gBUXaLMBv7HgIsyBvuVs2YI/6fcx05aOYMdhL3xG2sbYXgdgMcyW52sYYQjq3mn6ZfKg0gdyhtWncIbtyuvwMX2pWQF0stJAyKxeTCXs6AIDzoHTbdabRDFtmyb5cNM0foUSK6wYcT2qDWgcmjFS1KiWUbSOf6asNsALhYm+oyyPy0BR8Wj9zi8r51iViDmYe5+hv1SOR8JwuBDZ+sDFqncOGxyu7VzBcULjTsRUsh7a753YMEaggynrUCz2xzN8UUXqxHw6 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)(39860400002)(376002)(396003)(136003)(346002)(366004)(4326008)(5660300002)(83380400001)(53546011)(6486002)(26005)(6506007)(52116002)(8936002)(8676002)(86362001)(31686004)(31696002)(66476007)(66556008)(54906003)(2906002)(66946007)(2616005)(186003)(16526019)(316002)(36756003)(6512007)(956004)(478600001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: LD/9DTlTL06OoapYF3pDWccBrNCXxlFlDSTdjBXasLKMmKiqPNvwROJUSJGgrFmxkCzhn109MxpAQns4D/FWWfDxA+kp3odbIZKoTBc5th1txp4aLwvQOdRGRtOilHVDWY3KcdrzPQwTgO0j9uNlc8qMNcFv6lPekqFO7GA8MZZW5wGwuEmwQZxAab2TwY0AFZoTqe/eDVZhTIAhWt0eciO0V/tZD7xvfcFbURnCnQQrO8JrrbyFFoZmlhHEG1mjCM/EzPHkN/hmSGmvoNor3S1+pzh9e87AcYNufcjnLtHZLKMM8Hhgaf7bcCYCvABSFw5bhJt46n0na4mGljxSIrZHrAenriGHZQwcYHN5qONZmLhQzRc/y1QpNBHS89QE/S3Ykzv1pSbbE7NTlITEkNJI27x4AD2LgrMNht29TLAi9XloIJWdRO8L2wuvk99Ioa7nfmX81YQvVNDIkADJDxXHsLKJA7/gEevJyXVntnqsMhQ6C+1Be85yxNpUjrx2WArppSZOx5CVHuMRWxo8D4MOeavQebROUGOVgmwPBR/KOB7uBD+VxsUOOm5jwoZEVkMdKVixsrR/okPOOibXmHSa1RQwi5rzj5y09Q/1WTk0OOQjx47N3M7tGlA8XwyK8UgMFhJmwDTDKTEC0z50dw== X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 0eb5cdcb-7af1-41ad-b2e6-08d8608df965 X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1355.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Sep 2020 13:30:16.6720 (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: c6su9Ne1Pp7zRDjXQi3i5eTfXNF3642IA10Kkwu/WRBPo6y1TEsUq5IdCZWcwRgEXaYYLFgHjLfIl0wiqyPMfA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR12MB1162 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 9/24/20 1:22 AM, Laszlo Ersek wrote: > On 09/23/20 20:04, Tom Lendacky wrote: >> From: Tom Lendacky >> >> The AP reset vector stack allocation is only required if running as an >> SEV-ES guest. Since the reset vector allocation is below 1MB in memory, >> eliminate the requirement for bare-metal systems and non SEV-ES guests >> to allocate the extra stack area, which can be large if the >> PcdCpuMaxLogicalProcessorNumber value is large, and also remove the >> CPU_STACK_ALIGNMENT alignment. >> >> Fixes: 7b7508ad784d ("UefiCpuPkg: Allow AP booting under SEV-ES") >> Cc: Garrett Kirkendall >> Signed-off-by: Tom Lendacky >> --- >> UefiCpuPkg/Library/MpInitLib/MpLib.c | 36 +++++++++----------- >> 1 file changed, 17 insertions(+), 19 deletions(-) >> >> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c >> index 07426274f639..a9708c6479d2 100644 >> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c >> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c >> @@ -1141,20 +1141,6 @@ RestoreWakeupBuffer( >> ); >> } >> >> -/** >> - Calculate the size of the reset stack. >> - >> - @return Total amount of memory required for stacks >> -**/ >> -STATIC >> -UINTN >> -GetApResetStackSize ( >> - VOID >> - ) >> -{ >> - return AP_RESET_STACK_SIZE * PcdGet32(PcdCpuMaxLogicalProcessorNumber); >> -} >> - >> /** >> Calculate the size of the reset vector. >> >> @@ -1170,11 +1156,23 @@ GetApResetVectorSize ( >> { >> UINTN Size; >> >> - Size = ALIGN_VALUE (AddressMap->RendezvousFunnelSize + >> - AddressMap->SwitchToRealSize + >> - sizeof (MP_CPU_EXCHANGE_INFO), >> - CPU_STACK_ALIGNMENT); >> - Size += GetApResetStackSize (); >> + Size = AddressMap->RendezvousFunnelSize + >> + 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; >> } >> > > - I don't remember if it's required that the APIC ID space be densely > populated. For example, if we have a topology with 7 possible (= > maximum) logical CPUs, I'm unsure if a spec forbids any of those CPUs > from having APIC ID 7. That could cause a problem in > MpInitLibSevEsAPReset(), I assume. > > Anyway: that's a different topic. This patch looks OK to me because it > only spells out the existent assumption wrt. APIC IDs vs. > PcdCpuMaxLogicalProcessorNumber, plus it does solve the problem it wants > to solve. > > - I was a bit surprised at first upon seeing the reordering of alignment > vs. addition. But AP_RESET_STACK_SIZE is a whole multiple of > CPU_STACK_ALIGNMENT. So adding AP_RESET_STACK_SIZE to Size n times as > first step, does not change the congruence class of Size modulo > CPU_STACK_ALIGNMENT. Therefore ALIGN_VALUE() will increment Size by the > same value, regardless of whether it's done before or after the > AP_RESET_STACK_SIZE additions. Ah, yes, I did change that order. I could submit one more version that puts the ALIGN_VALUE back to the original position and fix the PcdGet32 space if that is desired (but, as you determined, it doesn't change the resulting value). I'm surprised that PatchCheck.py didn't pick up on the spacing with PcdGet32. Thanks, Tom > > - We should insert a space character after "PcdGet32", before merging > this patch. I think Ray or Eric could do this without a repost. > > Reviewed-by: Laszlo Ersek > > Thanks > Laszlo >