From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (NAM12-MW2-obe.outbound.protection.outlook.com [40.107.244.78]) by mx.groups.io with SMTP id smtpd.web09.14877.1603119734888066461 for ; Mon, 19 Oct 2020 08:02:15 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@amdcloud.onmicrosoft.com header.s=selector2-amdcloud-onmicrosoft-com header.b=FYn0KpNv; spf=none, err=SPF record not found (domain: amd.com, ip: 40.107.244.78, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=iSATBFBIGdRDMy9KMquEjubAQnytkF2k2whdBaayi8D2vpMLrC/Fe/rWjT9s7QNtoEoV7ap+5pMUVSRgi4rS7Ly2g/4LwsfmcU5aEBsfy6uFfLnEk80sO0tFMD5ou7cyVIPQ7ntmKDoKpdYUJNcXM9F6QXFtjPSmw50gBqlIKfpUSpFMBw14l7RnYPJGMdb9+jonqsVHm/Z3n2IV7XXyZn4hlxOGOFH1Okh3Jzd+8G5lSGBKmWggWLsA+PrKnHDHFycrUChSOoLzYif3D0HwSbPD3ij1IfgYZaFotQiuOHEtImy07DmmnvqdvgTKwGtpjioTNH0Y4Arjp3+QqJwlgw== 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=rJVk4idsSbtGsGZzaftByuVpDItfxnooZcZ/TrZUkAE=; b=DY4IGjvuCxYr4YJbu28oL0ztOw9zsd/Yi3f7HytJ3us4jzpGPq7MSh8LA3o1DWi6uv+otNiunB/NcS+phE7sITJiUhF8qaqe/TG2Zt9ZexHdg6kIvwYtgRkl0VYmlE18JasWchecpdA7OdRcvhWcArYaZEo3ZpF7xovShdQqdjuG5IGJsBw+opBZkT7yhvxz7+9fOBBS19LMo6hJ8TB9qfpOSlvVGFMMC3MlWl19vg6deqsr6aiEZWUcVh3Wyq6DWVA6yPfctWR4+EAEqdJKeg4qj25F6aE1R8sYlxde66eBzFrM2WaMe8BsAzS1lssDDSF77wp/tBC8MvsBoT4eDw== 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=rJVk4idsSbtGsGZzaftByuVpDItfxnooZcZ/TrZUkAE=; b=FYn0KpNvE8y+/PEzx9VybLwBVnZs4rvO9a/L8gV+MnJ+YcLF7AFLiGKg7P30ABdh2KpSl2Z4xluH98g4sE7GNdXxwRH93HrnKZ4ICN0yXo7g+BzdKq0Afwbjq4Mj5jqt+X4K9FBj0gOgyRZiFoOAVfyob/AqyPGQMxZWPkkDVmo= 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 DM6PR12MB4828.namprd12.prod.outlook.com (2603:10b6:5:1f8::26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3477.20; Mon, 19 Oct 2020 15:02:13 +0000 Received: from DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::e442:c052:8a2c:5fba]) by DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::e442:c052:8a2c:5fba%6]) with mapi id 15.20.3477.028; Mon, 19 Oct 2020 15:02:13 +0000 Subject: Re: [PATCH v2 1/1] UefiCpuPkg/MpInitLib: Reduce reset vector memory pressure From: "Lendacky, Thomas" To: devel@edk2.groups.io, Eric Dong , Ray Ni CC: Laszlo Ersek , Rahul Kumar , Brijesh Singh , Garrett Kirkendall References: <21345cdbc906519558202b3851257ca07b9239ba.1600884239.git.thomas.lendacky@amd.com> <6f1b41f0-fa17-7cb5-8ee2-3fc66b58a25c@amd.com> <48451727-6cbc-8cd7-300a-d9e8c284dabf@redhat.com> <686df789-8a14-6730-f031-1ae6925d896c@amd.com> Message-ID: <0794f733-ca18-3b6e-b82f-ac62fa94a216@amd.com> Date: Mon, 19 Oct 2020 10:02:11 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 In-Reply-To: <686df789-8a14-6730-f031-1ae6925d896c@amd.com> X-Originating-IP: [165.204.77.1] X-ClientProxiedBy: SN6PR16CA0050.namprd16.prod.outlook.com (2603:10b6:805:ca::27) 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 [10.236.30.118] (165.204.77.1) by SN6PR16CA0050.namprd16.prod.outlook.com (2603:10b6:805:ca::27) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3477.23 via Frontend Transport; Mon, 19 Oct 2020 15:02:12 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: 6fa2810e-5dbd-4dba-f775-08d8743ff5b4 X-MS-TrafficTypeDiagnostic: DM6PR12MB4828: 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: IzyoF8OoBnvGBl383jdY82SiMYOOmGwqHk9keeJH2hogdr0y96Kaj0B3k96n9ZbeIUa0vXpanvhBiNsEcYVstWKuNfHB+kps7za0HT4mpL3p2EGVn1FI8Ud5zutkxs1S+Ar08rFbJR+Fh/E0wp7vm4/SYktCdAPYr023E8aII4GrQ9KWq4/MoG1EEvgwHMjCP8bIDT6NnWvFjE0KDD7GBtqxN6yPHS44PEIgAzPpvgsGPPKi1IrQN7MjnHQlYUG+EMeKOW0uIalaEdcypYeTd/Qq2J9JiYj0QHafSYG2GgaE13+3k1h2PnpR2wTuP9I+lBzCQixwdS2VJ01AMvhh4pjJSL1ZyWKHXYL842UvP67LbP6TGJW0JLROCI3P9Yvw 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)(136003)(346002)(376002)(366004)(396003)(39860400002)(8676002)(186003)(16526019)(26005)(86362001)(110136005)(16576012)(316002)(5660300002)(6486002)(36756003)(54906003)(66556008)(66476007)(52116002)(2906002)(53546011)(478600001)(66946007)(83380400001)(31696002)(31686004)(4326008)(8936002)(956004)(2616005)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: 5pxaEDcPBEd09dsSVabs6nUwQWbezwWVsC+qJ+G9Wb27OwXBW7wMRWK4WOT0mopdIJSN3O/uHPCEOdcyf6EY0XQCUREyHMPuglM9JmADRDlG1SnnZb7S0XMsymSGJ0uXNpW+FLdarlnrQt164xs4+R0WBJKuaBDObrmgtD9FZMPmnTABf10pPGh8ZR8DfSMp3te9qD3PEloIMhDuTOAOEIZ+EZ0fa1ihWqyNmNGsrvzEm55zCXM6W7fC4JWZ+Q0yZm7ZhNUlIhxRLPVKnwlmHOlEXIs7+ae4jbJv+7CGYx2iFo5QBQkCCOUquHcKymfPj+6PuEyPJvCHvotn1BIPc27X8+MSNQArRb9sRZDugef+FtAd4KOldrqebiBYuoL8G2TbyO2S4syHV+965m0tvEczaqpBPcEsVAYXlIxyjnOTyINMl45DDTCNl/D0mq13mQaKRIhEfprpvv2AHqBO009rU4sYUkTBLDMxZIoTPbO8csjN5sFuY6uX/DzC9jqCOEUdFEe0YRNA1JyQenmLVwfHfPFIEibJWR944srY8h6crXluBsEXu/9jz7GWzLnGCKKy92cZom++f2Jt0NWhRC4UBYdYdYOhr4EXcrh3Avzw2Zt6o3B9xGHGI7iE/ex4i1BtyNshx28hEjcXlBdtfQ== X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 6fa2810e-5dbd-4dba-f775-08d8743ff5b4 X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1355.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Oct 2020 15:02:13.2222 (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: uT1ixy6T8o91hoboo6nStvkwDl6LzFA2emw+IaWrDMNG25awTNd02wHmUsNm5lrQiLpnzebyszfq9qWPZv+q/Q== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4828 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 10/2/20 11:42 AM, Tom Lendacky wrote: > On 9/24/20 10:03 AM, Laszlo Ersek wrote: >> On 09/24/20 15:30, Tom Lendacky wrote: >>> 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 a= n >>>>> SEV-ES guest. Since the reset vector allocation is below 1MB in memor= y, >>>>> eliminate the requirement for bare-metal systems and non SEV-ES guest= s >>>>> 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 >>>>> --- >>>>> =C2=A0=C2=A0 UefiCpuPkg/Library/MpInitLib/MpLib.c | 36 +++++++++-----= ------ >>>>> =C2=A0=C2=A0 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( >>>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ); >>>>> =C2=A0=C2=A0 } >>>>> =C2=A0=C2=A0 -/** >>>>> -=C2=A0 Calculate the size of the reset stack. >>>>> - >>>>> -=C2=A0 @return=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Total amount of memory required = for stacks >>>>> -**/ >>>>> -STATIC >>>>> -UINTN >>>>> -GetApResetStackSize ( >>>>> -=C2=A0 VOID >>>>> -=C2=A0 ) >>>>> -{ >>>>> -=C2=A0 return AP_RESET_STACK_SIZE * >>>>> PcdGet32(PcdCpuMaxLogicalProcessorNumber); >>>>> -} >>>>> - >>>>> =C2=A0=C2=A0 /** >>>>> =C2=A0=C2=A0=C2=A0=C2=A0 Calculate the size of the reset vector. >>>>> =C2=A0=C2=A0 @@ -1170,11 +1156,23 @@ GetApResetVectorSize ( >>>>> =C2=A0=C2=A0 { >>>>> =C2=A0=C2=A0=C2=A0=C2=A0 UINTN=C2=A0 Size; >>>>> =C2=A0=C2=A0 -=C2=A0 Size =3D ALIGN_VALUE (AddressMap->RendezvousFunn= elSize + >>>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Ad= dressMap->SwitchToRealSize + >>>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 si= zeof (MP_CPU_EXCHANGE_INFO), >>>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 CPU_STACK_ALIG= NMENT); >>>>> -=C2=A0 Size +=3D GetApResetStackSize (); >>>>> +=C2=A0 Size =3D AddressMap->RendezvousFunnelSize + >>>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Address= Map->SwitchToRealSize + >>>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 sizeof = (MP_CPU_EXCHANGE_INFO); >>>>> + >>>>> +=C2=A0 // >>>>> +=C2=A0 // The AP reset stack is only used by SEV-ES guests. Do not a= dd to >>>>> the >>>>> +=C2=A0 // allocation if SEV-ES is not enabled. >>>>> +=C2=A0 // >>>>> +=C2=A0 if (PcdGetBool (PcdSevEsIsEnabled)) { >>>>> +=C2=A0=C2=A0=C2=A0 // >>>>> +=C2=A0=C2=A0=C2=A0 // Stack location is based on APIC ID, so use the= total number of >>>>> +=C2=A0=C2=A0=C2=A0 // processors for calculating the total stack are= a. >>>>> +=C2=A0=C2=A0=C2=A0 // >>>>> +=C2=A0=C2=A0=C2=A0 Size +=3D AP_RESET_STACK_SIZE * >>>>> PcdGet32(PcdCpuMaxLogicalProcessorNumber); >>>>> + >>>>> +=C2=A0=C2=A0=C2=A0 Size =3D ALIGN_VALUE (Size, CPU_STACK_ALIGNMENT); >>>>> +=C2=A0 } >>>>> =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 return Size; >>>>> =C2=A0=C2=A0 } >>>>> >>>> >>>> - 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 (=3D >>>> 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 wan= ts >>>> to solve. >>>> >>>> - I was a bit surprised at first upon seeing the reordering of alignme= nt >>>> 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 th= e >>>> 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). >> >> Given that the stack grows down, one could plausibly claim that the >> variant seen in this patch (=3D align at last) is *more* intuitive. >> >> I'm OK with this version merged (with the whitespace fixed up). >=20 > Any concern from the maintainers on this? Are you waiting on me for > anything? Just want to check... Another ping to see what the status of this patch is... Thanks, Tom >=20 > Thanks, > Tom >=20 >> >>> I'm surprised that PatchCheck.py didn't pick up on the >>> spacing with PcdGet32. >> >> Or maybe ECC should warn about it in CI?... >> >> Thanks >> Laszlo >>