From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from aserp2120.oracle.com (aserp2120.oracle.com [141.146.126.78]) by mx.groups.io with SMTP id smtpd.web12.48495.1612246893703626089 for ; Mon, 01 Feb 2021 22:21:33 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@oracle.com header.s=corp-2020-01-29 header.b=bSs+nlTN; spf=pass (domain: oracle.com, ip: 141.146.126.78, mailfrom: ankur.a.arora@oracle.com) Received: from pps.filterd (aserp2120.oracle.com [127.0.0.1]) by aserp2120.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 1126F2eY039287; Tue, 2 Feb 2021 06:21:29 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=subject : to : cc : references : from : message-id : date : in-reply-to : content-type : content-transfer-encoding : mime-version; s=corp-2020-01-29; bh=PrdtmISTPjRLa5+6GCqJl928xnDSWF9PsZaqBby+xJU=; b=bSs+nlTN+4blso+m8dfjrPRpZsFaJ/FcTgVZ66pWw/DXYt/GTt07OXG3WrXdKU0GNa50 w+hk5oEA2uwhD1Y2Xh9nxpBqLjc+xe/ozQMUG8zLtWTDyqFF3eIRcuEtQd8APXGV/ce7 rWvQXIKKHlWfk+GbTKwodWrlyrk3o7PM+7zriX1sD5Mtw9aOtiJmO80odTC4aVS3qQrm 2oeAgDnxMKSXs1hOq+U+R9u1/zNmlTFioyVoSF+Iw11Fexpq2d/VvYDDq21cflc2bGtc bW8vrnTvZYObNGWdYzKGEAg4wvtDbG+aPZQDA0HG5B0TKuYzJ86Lb8lpJpQpkjt4zayz Zg== Received: from userp3020.oracle.com (userp3020.oracle.com [156.151.31.79]) by aserp2120.oracle.com with ESMTP id 36cydks0pr-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 02 Feb 2021 06:21:29 +0000 Received: from pps.filterd (userp3020.oracle.com [127.0.0.1]) by userp3020.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 1126G9LA037177; Tue, 2 Feb 2021 06:19:28 GMT Received: from nam12-dm6-obe.outbound.protection.outlook.com (mail-dm6nam12lp2173.outbound.protection.outlook.com [104.47.59.173]) by userp3020.oracle.com with ESMTP id 36dh7qx22n-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 02 Feb 2021 06:19:28 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=e/lDDWVS+XbtauDlr1+swHbMtSMLs6JcUvaXmOO8/6xRYbtbEgZ1tZdO6kRso4ZEeis3VzooxUvUzvKuCgch3xrZ6JWZTt7T/Lk4SHnLo0XGJxyf8Gdh0EFiR1VHfQjn+qe+X1gSemZxiI7BrhrohQuqzaheFYazPO9htijlbIx9ZNZdPviaJAOBS3PWpBWZ+vRYJQqABL/yfewxJG4lx1BEQhEBcg9liGi4/VjZzo8bFOLonSpiwYtEcAEPy1hZkff5AseJMZV9C3OuYTVhbKG3vOk1zmleGGb4Co+ySxT1Ul330CSqWhlF1VpFIqWpcAxgkTCgMocqT9hdpl+VCA== 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=PrdtmISTPjRLa5+6GCqJl928xnDSWF9PsZaqBby+xJU=; b=H2YQJGR8v9PtM1jWcehYA/8uvrzWhCYjQ9QRneGv/8JdYUncld4cJSU9hSp4PZqU/EXaLi6b5+DvlYyOcskhCr3Er1Vq1ASfFlx8wzaSmZ4tx2KU8vV0ekZ2c+pXq6J2x1A1DypaZisC25BwGDFJ1uXbqdl9KrSFLAcZ1HqMkhnLys1xqTqAzM3yo9Eae+QAQiyVKLAAzNhpk21PuYw5BPJCbo1sZ3k1GERvkUzvKDMbbIxWFYBakf4BVA5hLCfCirPpw3yUEy8P+++tVo28PrGQ+cTNcLgat2WvE/+3GlJYtpktWEltzh0fBSLspo9doYyf3eE1hGy8hpHzBWTBjQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oracle.com; dmarc=pass action=none header.from=oracle.com; dkim=pass header.d=oracle.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.onmicrosoft.com; s=selector2-oracle-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=PrdtmISTPjRLa5+6GCqJl928xnDSWF9PsZaqBby+xJU=; b=OaoQPJ0As0PgeDo865R7lBj8Ce/+dun+dt4NI6wHYDEG4+iDGPvfVAsgdiXgre+NidWkkZHcHAabOd+gibvg/rFg5058mKT73XQ1d9UKRUsKmi58daz28YYM2OcBLp2XvRNv1Vn/q1GWVrTQx0a/Lj4RMdk/NKObTUDEt0a8n40= Received: from SJ0PR10MB4605.namprd10.prod.outlook.com (2603:10b6:a03:2d9::24) by BYAPR10MB3511.namprd10.prod.outlook.com (2603:10b6:a03:129::25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3805.24; Tue, 2 Feb 2021 06:19:26 +0000 Received: from SJ0PR10MB4605.namprd10.prod.outlook.com ([fe80::b1e9:811d:aa8e:f62a]) by SJ0PR10MB4605.namprd10.prod.outlook.com ([fe80::b1e9:811d:aa8e:f62a%6]) with mapi id 15.20.3805.028; Tue, 2 Feb 2021 06:19:26 +0000 Subject: Re: [edk2-devel] [PATCH v6 1/9] OvmfPkg/CpuHotplugSmm: refactor hotplug logic To: Laszlo Ersek , devel@edk2.groups.io Cc: imammedo@redhat.com, boris.ostrovsky@oracle.com, Jordan Justen , Ard Biesheuvel , Aaron Young References: <20210129005950.467638-1-ankur.a.arora@oracle.com> <20210129005950.467638-2-ankur.a.arora@oracle.com> From: "Ankur Arora" Message-ID: <0b14dbc3-7d93-fcf1-61ca-ae377e6e0017@oracle.com> Date: Mon, 1 Feb 2021 22:19:24 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 In-Reply-To: X-Originating-IP: [70.36.60.91] X-ClientProxiedBy: MWHPR13CA0002.namprd13.prod.outlook.com (2603:10b6:300:16::12) To SJ0PR10MB4605.namprd10.prod.outlook.com (2603:10b6:a03:2d9::24) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [192.168.0.108] (70.36.60.91) by MWHPR13CA0002.namprd13.prod.outlook.com (2603:10b6:300:16::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3825.8 via Frontend Transport; Tue, 2 Feb 2021 06:19:25 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: c7ee403a-a104-4acf-cc97-08d8c7427d57 X-MS-TrafficTypeDiagnostic: BYAPR10MB3511: 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: jSpAbkQvcA6OIumPy0ND//wmlnPUoikDDybaXytLtSv+a9feFgOrqxk/YLvQ8KBeNGUdvPJiP8iZqQ7UnIscW2T6ra9pjNALlgO+3MVHHhaj27ExQt5anJ02jKg9+5b1AVEU/v+flzr+YTdz2PYhbW+BnqV/+YO+AHBFqQemVyLmWYyYBPvEC5EFga0dVWbCpb9F6J8QH2/WViupCkWZklkiox1TIIEiYyBdu9y19bU/7kmrethG8Gz3ao3chsnIwLutBuok1oLIdlf2rwSC8HzbrBfkV9b3qQfOfRybDG+gqayVuxG/KnXXK+Yqq2mNi3H9JXPUCKbGczkRwXyqG9LLeJHvJrlY5Lr3UgNUeL9QoaZYcAndv+wag7kyCtWzjHPkdHOBUEkZ1qzM6u2zSR7ctl/GcAd6C4lEfeCHR/azdshOGd8QSlpdmbOESKwFEmR7zLefzHk3qTo19D/8CxCxT7iqb4ddVgbvspzNtHDJIxpXBnZN8TsV8NK8Wxj9uPXtokzX/UcgLPrWNG4K8xyGzCThif2bMYG6lRLER1afVNr86vzyvJoTBQLdfpe55IeLOAHNu8fOhQy5KXeLfcn64TQqGADQatH9+gy8bOPZIQomIw6wYYJOCQ0DjVnTpFzSXNlifJB1fsYTBXhp3abZV1kzs/aX8KsmwKHuqqNDdkS6ySSkQmeJmPk5xqgD X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:SJ0PR10MB4605.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(396003)(346002)(136003)(39860400002)(366004)(376002)(186003)(16526019)(53546011)(26005)(107886003)(66476007)(31686004)(8676002)(66556008)(4326008)(86362001)(5660300002)(31696002)(316002)(36756003)(956004)(83380400001)(16576012)(54906003)(8936002)(2616005)(6486002)(966005)(66946007)(2906002)(478600001)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?bFNwVUJqeVMyVEc5YUpTV0J4T0twYWNBYWVveklDbXRyNTFTK0NjY21xbmhW?= =?utf-8?B?Q285Q0U1VnJDcDNIMlB2dWd3UGgwY1VDNk5MYVl3bUVtWjVzOHp4SzFaaHZx?= =?utf-8?B?WEZyL2ZnTzZBNldieUdCaGpCNlo3eDFlL1Boc3V5V2tvcUgzYlI5cXNRcU5W?= =?utf-8?B?eFdBTzZBMmlLSWU0VzRVYlpKMHRtNzNyK01rQTBqSHFwSkNDcXMzekYrTy9L?= =?utf-8?B?OExkemdpUjFiNGJKSDVDSnFNQmZBS1JvcVFqUlQ0QzhFZWxxOFRTWFc3YTVC?= =?utf-8?B?R05CY2FySnN1SzVCeWsxUFpPVm5oRkNOUHV0NnJ1aXpKT1dueXZiK1RQV0dE?= =?utf-8?B?TVZOaXNtMWlOeDRKcm1sWk50Mk9QWmRGMnA1ZGorcEVkNEZEbWdncHRMU1dD?= =?utf-8?B?dFYzMjhGY21rNzYwZDlvbjFRbzlFMUFXTkJsbkhhdFpTOUQxK2h5eTJyZVNE?= =?utf-8?B?aDhmbWgxYnhuODRjZy9BdWdNMUorcERHN3JHbkN0WkFZcWY5SEQ5WWVCT3lE?= =?utf-8?B?Z1JNY1dmUzNwb0M1ZjFBZlovRi9yRWlsSjZzOTVHbjNmU2JINzNzcnZ5Ulgz?= =?utf-8?B?bkVDZ216NVdDc3RwdEVuUTU5WkErTDNvZFhnUW9XQVZQeHRkdkhNd21QZHlC?= =?utf-8?B?TXNoQXBsb0dTZW11cXV1cU0xOUJDd3BQNDhpNjEzK0l4KzdYZzZtdTQ1d2Ji?= =?utf-8?B?R1hWV1dVK2xzZXlXRjFNc3lsNng5RjhPOWwvZG1vdkpIWGVISG03R3BRRS8v?= =?utf-8?B?cUJWL3cwT2VWSEdsL0ZjYTZibEFmZ09CZUttTVN1Tlp2Vm5YOGQvMkNxZUx0?= =?utf-8?B?TTV2Qk0wdC9VVTRDZ1o3UGZZV3IvQjcybEF2Z0J6b21YTUpjenhxYVBlZDhr?= =?utf-8?B?THJIMjFhalNLdEg0alNUZ0dvcU1vYXhJYmkwRDZGRHdrWnl6OW5ZVFFETWRV?= =?utf-8?B?U1VHMmFZdGlrVXY5M0FSQUFISDhVbitoallWaUhDODcxY2JOY0hhZ0x0aHl4?= =?utf-8?B?RzRtRG15OWhlY0hVZ25jNkhodkNGSllhcGYxT3lHUjI5M1VEV1dndTFCRlVy?= =?utf-8?B?YTNwZjlZUzNPaGthRW9FeTUzVWJoNTFERDV0cDJtMlk5YlA4VTMwZXZYZEhz?= =?utf-8?B?TUNua2xmclBYZW8zRThLYkp0VzN1OXRuQlV3WC9SbWZTSUpFK1VINC9qUWFF?= =?utf-8?B?MTREUnpxbXh0VWlrTVNZekxYWGZJL1I3RnIrTjE4TEVwRWdIVlNBeTI4dnZn?= =?utf-8?B?VFNoK1Y1bTlhR3BQOG03VG8yUHRlNkJDRnBaTDM0bFlvVDk0eGJSMElac245?= =?utf-8?B?dks1T3RNRkpBREp1NjV0bXlrRDhGSW0zNEZOb2VtZThMVXFoTnhnYkNkRTB2?= =?utf-8?B?MXVkVGdPbVBOZDE1NTNOVUlSN3ZNd0RDQUs0dHlvdUpLcitER2phODVTWVBN?= =?utf-8?Q?+H2jmn0A?= X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: c7ee403a-a104-4acf-cc97-08d8c7427d57 X-MS-Exchange-CrossTenant-AuthSource: SJ0PR10MB4605.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Feb 2021 06:19:26.0069 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: 2WPl83CYSzxdBNeJymyWaJsciiq8r8QORYSc+vduTQ6+sWP6fxlmPrI0xTkQTrOm2Qz7GRp3eAC7u6iU61KLHgGGAHjNp5n31zxHRrUldTk= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB3511 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9882 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxscore=0 suspectscore=0 spamscore=0 mlxlogscore=999 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2102020044 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9882 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 adultscore=0 priorityscore=1501 impostorscore=0 malwarescore=0 clxscore=1015 spamscore=0 lowpriorityscore=0 phishscore=0 mlxlogscore=999 mlxscore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2102020044 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 2021-01-29 5:15 p.m., Laszlo Ersek wrote: > On 01/29/21 01:59, Ankur Arora wrote: >> Refactor CpuHotplugMmi() to pull out the CPU hotplug logic into >> ProcessHotAddedCpus(). This is in preparation for supporting CPU >> hot-unplug. >> >> Cc: Laszlo Ersek >> Cc: Jordan Justen >> Cc: Ard Biesheuvel >> Cc: Igor Mammedov >> Cc: Boris Ostrovsky >> Cc: Aaron Young >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132 >> Signed-off-by: Ankur Arora >> --- >> >> Notes: >> > + if (EFI_ERROR(Status)) { >> > + goto Fatal; >> > } >> >> (13) Without having seen the rest of the patches, I think this error >> check should be nested under the same (PluggedCount > 0) condition; in >> other words, I think it only makes sense to check Status after we >> actually call ProcessHotAddedCpus(). >> >> Addresses all comments from v5, except for this one, since the (lack) of >> nesting makes more sense after patch 4, "OvmfPkg/CpuHotplugSmm: introduce >> UnplugCpus()". >> >> OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 214 ++++++++++++++++++++++--------------- >> 1 file changed, 129 insertions(+), 85 deletions(-) >> >> diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c >> index cfe698ed2b5e..05b1f8cb63a6 100644 >> --- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c >> +++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c >> @@ -62,6 +62,130 @@ STATIC UINT32 mPostSmmPenAddress; >> // >> STATIC EFI_HANDLE mDispatchHandle; >> >> +/** >> + Process CPUs that have been hot-added, per QemuCpuhpCollectApicIds(). >> + >> + For each such CPU, relocate the SMBASE, and report the CPU to PiSmmCpuDxeSmm >> + via EFI_SMM_CPU_SERVICE_PROTOCOL. If the supposedly hot-added CPU is already >> + known, skip it silently. >> + >> + @param[in] PluggedApicIds The APIC IDs of the CPUs that have been >> + hot-plugged. >> + >> + @param[in] PluggedCount The number of filled-in APIC IDs in >> + PluggedApicIds. >> + >> + @retval EFI_SUCCESS CPUs corresponding to all the APIC IDs are >> + populated. >> + >> + @retval EFI_OUT_OF_RESOURCES Out of APIC ID space in "mCpuHotPlugData". >> + >> + @return Error codes propagated from SmbaseRelocate() >> + and mMmCpuService->AddProcessor(). >> + >> +**/ >> +STATIC >> +EFI_STATUS >> +ProcessHotAddedCpus ( >> + IN APIC_ID *PluggedApicIds, >> + IN UINT32 PluggedCount >> + ) >> +{ >> + EFI_STATUS Status; >> + UINT32 PluggedIdx; >> + UINT32 NewSlot; >> + >> + // >> + // The Post-SMM Pen need not be reinstalled multiple times within a single >> + // root MMI handling. Even reinstalling once per root MMI is only prudence; >> + // in theory installing the pen in the driver's entry point function should >> + // suffice. >> + // >> + SmbaseReinstallPostSmmPen (mPostSmmPenAddress); >> + >> + PluggedIdx = 0; >> + NewSlot = 0; >> + while (PluggedIdx < PluggedCount) { >> + APIC_ID NewApicId; >> + UINT32 CheckSlot; >> + UINTN NewProcessorNumberByProtocol; >> + >> + NewApicId = PluggedApicIds[PluggedIdx]; >> + >> + // >> + // Check if the supposedly hot-added CPU is already known to us. >> + // >> + for (CheckSlot = 0; >> + CheckSlot < mCpuHotPlugData->ArrayLength; >> + CheckSlot++) { >> + if (mCpuHotPlugData->ApicId[CheckSlot] == NewApicId) { >> + break; >> + } >> + } >> + if (CheckSlot < mCpuHotPlugData->ArrayLength) { >> + DEBUG ((DEBUG_VERBOSE, "%a: APIC ID " FMT_APIC_ID " was hot-plugged " >> + "before; ignoring it\n", __FUNCTION__, NewApicId)); >> + PluggedIdx++; >> + continue; >> + } >> + >> + // >> + // Find the first empty slot in CPU_HOT_PLUG_DATA. >> + // >> + while (NewSlot < mCpuHotPlugData->ArrayLength && >> + mCpuHotPlugData->ApicId[NewSlot] != MAX_UINT64) { >> + NewSlot++; >> + } >> + if (NewSlot == mCpuHotPlugData->ArrayLength) { >> + DEBUG ((DEBUG_ERROR, "%a: no room for APIC ID " FMT_APIC_ID "\n", >> + __FUNCTION__, NewApicId)); >> + return EFI_OUT_OF_RESOURCES; >> + } >> + >> + // >> + // Store the APIC ID of the new processor to the slot. >> + // >> + mCpuHotPlugData->ApicId[NewSlot] = NewApicId; >> + >> + // >> + // Relocate the SMBASE of the new CPU. >> + // >> + Status = SmbaseRelocate (NewApicId, mCpuHotPlugData->SmBase[NewSlot], >> + mPostSmmPenAddress); >> + if (EFI_ERROR (Status)) { >> + goto RevokeNewSlot; >> + } >> + >> + // >> + // Add the new CPU with EFI_SMM_CPU_SERVICE_PROTOCOL. >> + // >> + Status = mMmCpuService->AddProcessor (mMmCpuService, NewApicId, >> + &NewProcessorNumberByProtocol); >> + if (EFI_ERROR (Status)) { >> + DEBUG ((DEBUG_ERROR, "%a: AddProcessor(" FMT_APIC_ID "): %r\n", >> + __FUNCTION__, NewApicId, Status)); >> + goto RevokeNewSlot; >> + } >> + >> + DEBUG ((DEBUG_INFO, "%a: hot-added APIC ID " FMT_APIC_ID ", SMBASE 0x%Lx, " >> + "EFI_SMM_CPU_SERVICE_PROTOCOL assigned number %Lu\n", __FUNCTION__, >> + NewApicId, (UINT64)mCpuHotPlugData->SmBase[NewSlot], >> + (UINT64)NewProcessorNumberByProtocol)); >> + >> + NewSlot++; >> + PluggedIdx++; >> + } >> + >> + // >> + // We've processed this batch of hot-added CPUs. >> + // >> + return EFI_SUCCESS; >> + >> +RevokeNewSlot: >> + mCpuHotPlugData->ApicId[NewSlot] = MAX_UINT64; >> + >> + return Status; >> +} >> >> /** >> CPU Hotplug MMI handler function. >> @@ -122,8 +246,6 @@ CpuHotplugMmi ( >> UINT8 ApmControl; >> UINT32 PluggedCount; >> UINT32 ToUnplugCount; >> - UINT32 PluggedIdx; >> - UINT32 NewSlot; >> >> // >> // Assert that we are entering this function due to our root MMI handler >> @@ -179,87 +301,12 @@ CpuHotplugMmi ( >> goto Fatal; >> } >> >> - // >> - // Process hot-added CPUs. >> - // >> - // The Post-SMM Pen need not be reinstalled multiple times within a single >> - // root MMI handling. Even reinstalling once per root MMI is only prudence; >> - // in theory installing the pen in the driver's entry point function should >> - // suffice. >> - // >> - SmbaseReinstallPostSmmPen (mPostSmmPenAddress); >> + if (PluggedCount > 0) { >> + Status = ProcessHotAddedCpus (mPluggedApicIds, PluggedCount); >> + } >> >> - PluggedIdx = 0; >> - NewSlot = 0; >> - while (PluggedIdx < PluggedCount) { >> - APIC_ID NewApicId; >> - UINT32 CheckSlot; >> - UINTN NewProcessorNumberByProtocol; >> - >> - NewApicId = mPluggedApicIds[PluggedIdx]; >> - >> - // >> - // Check if the supposedly hot-added CPU is already known to us. >> - // >> - for (CheckSlot = 0; >> - CheckSlot < mCpuHotPlugData->ArrayLength; >> - CheckSlot++) { >> - if (mCpuHotPlugData->ApicId[CheckSlot] == NewApicId) { >> - break; >> - } >> - } >> - if (CheckSlot < mCpuHotPlugData->ArrayLength) { >> - DEBUG ((DEBUG_VERBOSE, "%a: APIC ID " FMT_APIC_ID " was hot-plugged " >> - "before; ignoring it\n", __FUNCTION__, NewApicId)); >> - PluggedIdx++; >> - continue; >> - } >> - >> - // >> - // Find the first empty slot in CPU_HOT_PLUG_DATA. >> - // >> - while (NewSlot < mCpuHotPlugData->ArrayLength && >> - mCpuHotPlugData->ApicId[NewSlot] != MAX_UINT64) { >> - NewSlot++; >> - } >> - if (NewSlot == mCpuHotPlugData->ArrayLength) { >> - DEBUG ((DEBUG_ERROR, "%a: no room for APIC ID " FMT_APIC_ID "\n", >> - __FUNCTION__, NewApicId)); >> - goto Fatal; >> - } >> - >> - // >> - // Store the APIC ID of the new processor to the slot. >> - // >> - mCpuHotPlugData->ApicId[NewSlot] = NewApicId; >> - >> - // >> - // Relocate the SMBASE of the new CPU. >> - // >> - Status = SmbaseRelocate (NewApicId, mCpuHotPlugData->SmBase[NewSlot], >> - mPostSmmPenAddress); >> - if (EFI_ERROR (Status)) { >> - goto RevokeNewSlot; >> - } >> - >> - // >> - // Add the new CPU with EFI_SMM_CPU_SERVICE_PROTOCOL. >> - // >> - Status = mMmCpuService->AddProcessor (mMmCpuService, NewApicId, >> - &NewProcessorNumberByProtocol); >> - if (EFI_ERROR (Status)) { >> - DEBUG ((DEBUG_ERROR, "%a: AddProcessor(" FMT_APIC_ID "): %r\n", >> - __FUNCTION__, NewApicId, Status)); >> - goto RevokeNewSlot; >> - } >> - >> - DEBUG ((DEBUG_INFO, "%a: hot-added APIC ID " FMT_APIC_ID ", SMBASE 0x%Lx, " >> - "EFI_SMM_CPU_SERVICE_PROTOCOL assigned number %Lu\n", __FUNCTION__, >> - NewApicId, (UINT64)mCpuHotPlugData->SmBase[NewSlot], >> - (UINT64)NewProcessorNumberByProtocol)); >> - >> - NewSlot++; >> - PluggedIdx++; >> + if (EFI_ERROR(Status)) { > > (1) I understand why you skipped point (13) from the previous review, > but you missed point (14) as well -- space character missing after > "EFI_ERROR": > > https://edk2.groups.io/g/devel/message/70785 > > Anyway, in case v7 will not be necessary, I can fix this up myself. > > With the space character added: > > Reviewed-by: Laszlo Ersek Thanks. Will fix in v7 (not quite sure how this one escaped me; my eyes are too used to not having the additional space, but I did have a grep command that should have caught this one as well.) Thanks Ankur > > Thanks > Laszlo > > >> + goto Fatal; >> } >> >> // >> @@ -267,9 +314,6 @@ CpuHotplugMmi ( >> // >> return EFI_SUCCESS; >> >> -RevokeNewSlot: >> - mCpuHotPlugData->ApicId[NewSlot] = MAX_UINT64; >> - >> Fatal: >> ASSERT (FALSE); >> CpuDeadLoop (); >> >