From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM04-CO1-obe.outbound.protection.outlook.com (NAM04-CO1-obe.outbound.protection.outlook.com [40.107.69.70]) by mx.groups.io with SMTP id smtpd.web10.288.1601656930748583539 for ; Fri, 02 Oct 2020 09:42:11 -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=Ni3/R+Oe; spf=none, err=SPF record not found (domain: amd.com, ip: 40.107.69.70, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=E+rMFpOlqZ0R7Czq8/Eam9rIkIvxtARTiHNM5nWkZ5DdckLLRl+QJ1932iFTr8dBKqZJIzonIz9ovO5rm4U56c82EJyjdYL1BoKVl69jRTkWlhdseFLdAS1OO/Dj0hW0W1KykA2h/HZP5lgpwYkSpCUEZO3ztE2tnunoLUfYajbUO9cqS5uvKqIZAq7tjRk6yQT/sby82rNq0/PjIUU34Dyjc0GRBwr4/1yxVwBFy3wUspakbiCAsPe1neCB5pu7FhmXEeiNl9hsi5TXDIlun8HtDxGy+O7yFpo4t5n9J52jxIWud3RGz+yFwDspEgaDqQbWo3FRKY7yFnOEmcZ4DQ== 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=ZRzZtvBduhwNgxeZcedE469ZwNoAhNtbbX3LfQlkWLE=; b=K2D2A9G82vdsesjjlNGpVDd4UNbBZ+vrfWWCnYp2QTmgRE9/wq/IaZuUBgibJXVa294+v7lrx6nst/jfRJ00vfb0JNQgyr9JpTwrFPLyiIiaKxGbxE9rWf54y69ScEJRprvS6W3MexFu/nRDnc7hbx70o4l6SxC6bTj/eLxwvKRD87Z68IWBgFyDGdY52H+RQVPklDE7vdejFPQr3W35K1aPkv1GEIYhaev27zDPpXC3H60aZNTFvZXm4zwA0rDCdQGByjF2kWFZICZLnNhuoxShQbQOe3lhw8Y0lg8xf2uJ4GK97GQTXSXP0lQ4FLJ9uR/ab5X3mdt496spY7osNA== 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=ZRzZtvBduhwNgxeZcedE469ZwNoAhNtbbX3LfQlkWLE=; b=Ni3/R+OeO0v9QmtchmmYLJ6V96noniBXFnTVagL3t2oYmCF75kWgilSMsUCy9f0CGmeKcR4lFKlZ49qZuZndqQxsgcmCCIt3g/pKT5tVL+sajpsD2DoJuxz+wkpyIBIb4Zn1qbtuM4VpWhDUgOXfxK15B268XN9yj4kuN77ELSE= 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 DM6PR12MB4217.namprd12.prod.outlook.com (2603:10b6:5:219::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3433.36; Fri, 2 Oct 2020 16:42:08 +0000 Received: from DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::4d88:9239:2419:7348]) by DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::4d88:9239:2419:7348%2]) with mapi id 15.20.3433.039; Fri, 2 Oct 2020 16:42:08 +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> <6f1b41f0-fa17-7cb5-8ee2-3fc66b58a25c@amd.com> <48451727-6cbc-8cd7-300a-d9e8c284dabf@redhat.com> From: "Lendacky, Thomas" Message-ID: <686df789-8a14-6730-f031-1ae6925d896c@amd.com> Date: Fri, 2 Oct 2020 11:42:06 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 In-Reply-To: <48451727-6cbc-8cd7-300a-d9e8c284dabf@redhat.com> X-Originating-IP: [67.79.209.213] X-ClientProxiedBy: SN2PR01CA0057.prod.exchangelabs.com (2603:10b6:800::25) 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 SN2PR01CA0057.prod.exchangelabs.com (2603:10b6:800::25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3433.35 via Frontend Transport; Fri, 2 Oct 2020 16:42:08 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: 7a7ca471-833b-4202-704b-08d866f21a84 X-MS-TrafficTypeDiagnostic: DM6PR12MB4217: 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: bFpn4A6eDoOPcaA8BREF5+cm0WBB+mfMVRuoLPlEbp0oYkb112UljeRSiRKJEbTezIYh1KblYEjFv4Cfa+kE5Y5DQ2ZHKLlkxoXmajnLsE5nd2Yy4NWrgK1yjTlRwARmRIILBAtWuT6mkWGz9+eUdOt2ttZm4t7FGDu4ze3L5KLtqVMTEA/HelZv/bH/H7rFZ3NmTwVhIbD4mhbnUR7waMPSHzWopI0WoOv9pBH4nNLlECtARANozm/MYi7uyVIQDasb9MkLkeNvnLWxC1w9fQT4r1EXkkbmbc+2oqt+dZKrJQ+OGL1l4OFRk7uBrOT2kwjxtIrhsL71qTfilKbwHaZVlm3ivTX2RukSmfpCxQulzgti5s1acWDvt4ItOs4K 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)(136003)(366004)(376002)(396003)(346002)(83380400001)(52116002)(2616005)(5660300002)(66946007)(4326008)(6506007)(53546011)(478600001)(316002)(8936002)(31696002)(31686004)(956004)(66556008)(54906003)(186003)(86362001)(66476007)(8676002)(26005)(16526019)(6486002)(2906002)(6512007)(36756003)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: sHElknXhWJVsXeZZNMwkQz/yTmt1APh2Z24XV1lpEbDTMyHpNOaj5bdR0wJNQm22Xb3Xes5CKgym8yCH35q6sQmiOpjogsXFCoD+hS2I2WGRis4BavM2qGgduig99bjbQ9K2QX0GCp6EXlJfOqidf5HWg276H7JhH7WSnkqY5XOOzuru8Qt3mBGxUS17IU6XNHr3HApcJUkmZaysUybZhyHuRCJw3wHQYHJPh/iyfHqpkPWZT2WT28bX1FN9t+a2mMWJ/KBU4rSgmVSUB2fdNyPztCMEfirsilP3cdmoAcaTCgb7ney+cO6nGXJ5DyOoHNdt2Y7PWAj6YpOj+u62AvgqleyrJAhMa/kEDMngSKJUyf9XOrqc2Amqz2qSo8iuuMM868tGe+0zUtShWpElJ6I0F1Y2FBUYnML4+HKx0Uf7qMUqmd4Gxge2E0cYv3gH7C9WKnsmeSWIQEzb72tv1DPupu68zecPYBett7cBdKm2npenC+rH+XMmUA1LzCznweOhkGU2jslUF1ztXUl2Cu7pnI3K1j23sctPshpjyxol60G7Epbq6QI2Ss+3KIj1xyQoSYBm3FFDWVWBF4CCVl8RyChSelEY1L8D03/OjgPkTQGjpyHtAePdNVq5ksPVT/9hVQ63DU8Xgo9D2tkoYQ== X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 7a7ca471-833b-4202-704b-08d866f21a84 X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1355.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Oct 2020 16:42:08.7670 (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: hPS+xaQ2zD4PrbeebXpIKmocaJdAO4PyGfyC9KbFOCUQIjQ3S1Hdvo4K51/7lPS04Zqrgz96ORFLjHjl+M9loQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4217 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable 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 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 >>>> --- >>>> =C2=A0 UefiCpuPkg/Library/MpInitLib/MpLib.c | 36 +++++++++----------- >>>> =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 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 Calculate the size of the reset vector. >>>> =C2=A0 @@ -1170,11 +1156,23 @@ GetApResetVectorSize ( >>>> =C2=A0 { >>>> =C2=A0=C2=A0=C2=A0 UINTN=C2=A0 Size; >>>> =C2=A0 -=C2=A0 Size =3D ALIGN_VALUE (AddressMap->RendezvousFunnelSize= + >>>> -=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 Addre= ssMap->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 sizeo= f (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_ALIGNME= NT); >>>> -=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 AddressM= ap->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 ad= d 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 area= . >>>> +=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 return Size; >>>> =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 want= s >>> to solve. >>> >>> - I was a bit surprised at first upon seeing the reordering of alignmen= t >>> 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). >=20 > Given that the stack grows down, one could plausibly claim that the > variant seen in this patch (=3D align at last) is *more* intuitive. >=20 > I'm OK with this version merged (with the whitespace fixed up). Any concern from the maintainers on this? Are you waiting on me for=20 anything? Just want to check... Thanks, Tom >=20 >> I'm surprised that PatchCheck.py didn't pick up on the >> spacing with PcdGet32. >=20 > Or maybe ECC should warn about it in CI?... >=20 > Thanks > Laszlo >=20