From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id 82900740034 for ; Fri, 24 May 2024 03:03:16 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=mNg4V0f9wChHulQK5pCHFO1TRcR8C//f+fGUReEL3SI=; c=relaxed/simple; d=groups.io; h=From:To:Subject:Thread-Topic:Thread-Index:Date:Message-ID:References:In-Reply-To:Accept-Language:msip_labels:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type; s=20240206; t=1716519795; v=1; b=cYaqcea86hItIHMRF4pQTPZ0UHOfIXcjR2nZIgiIQo/EXHCNcQVtWm9HhlZOJH0FTJFS+ji/ nIvVe5PmXyP8n6QwRHeVM5IWl5hPy/4hvRIqv+vt0trW67A8zXMwv+XoH6dKy845T6Kxz/OnnrJ O1vs2KS4S93Ga1BCQuh9JntUQyuQ7fm4bPCxIzBy3CTiCFteGcQ42FFMznzXI1ODzdQYZsMqE/l moVTy0a5xWgymxvt9fMUYwcuKJQI7YOEr+cgCyTAOmMHrAXca7Bi/A11NjJEeJNg84D1rYcy5ob 9nAGlLzpFhVIW/k1+rI02cl/ocalCZqYzN2Tjf2Q9ziJQ== X-Received: by 127.0.0.2 with SMTP id Nl0TYY7687511xmE1YCn9a01; Thu, 23 May 2024 20:03:15 -0700 X-Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.8]) by mx.groups.io with SMTP id smtpd.web11.7649.1716519793939823292 for ; Thu, 23 May 2024 20:03:14 -0700 X-CSE-ConnectionGUID: WPjcn3vpSkq8phxemE+GjQ== X-CSE-MsgGUID: iWIBPnjkSDW2zRc322u0QA== X-IronPort-AV: E=McAfee;i="6600,9927,11081"; a="30411901" X-IronPort-AV: E=Sophos;i="6.08,184,1712646000"; d="scan'208,217";a="30411901" X-Received: from fmviesa006.fm.intel.com ([10.60.135.146]) by fmvoesa102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 May 2024 20:03:13 -0700 X-CSE-ConnectionGUID: hVOs8TAqRT+nDm7ixy4F7w== X-CSE-MsgGUID: q1EKQ7WcSQ6lQFneQZKFNQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.08,184,1712646000"; d="scan'208,217";a="33895244" X-Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by fmviesa006.fm.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 23 May 2024 20:03:13 -0700 X-Received: from fmsmsx601.amr.corp.intel.com (10.18.126.81) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.39; Thu, 23 May 2024 20:03:13 -0700 X-Received: from FMSEDG603.ED.cps.intel.com (10.1.192.133) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.39 via Frontend Transport; Thu, 23 May 2024 20:03:13 -0700 X-Received: from NAM11-DM6-obe.outbound.protection.outlook.com (104.47.57.168) by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Thu, 23 May 2024 20:03:12 -0700 X-Received: from MN6PR11MB8244.namprd11.prod.outlook.com (2603:10b6:208:470::14) by SA2PR11MB5100.namprd11.prod.outlook.com (2603:10b6:806:119::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7611.19; Fri, 24 May 2024 03:03:10 +0000 X-Received: from MN6PR11MB8244.namprd11.prod.outlook.com ([fe80::41a4:c775:32e6:76a8]) by MN6PR11MB8244.namprd11.prod.outlook.com ([fe80::41a4:c775:32e6:76a8%4]) with mapi id 15.20.7587.035; Fri, 24 May 2024 03:03:10 +0000 From: "Ni, Ray" To: "Feng, Ning" , "devel@edk2.groups.io" Subject: Re: [edk2-devel] [PATCH] Pkg-Module:UefiCpuPkg/MpLib:Do not assume BSP is #0. Thread-Topic: [PATCH] Pkg-Module:UefiCpuPkg/MpLib:Do not assume BSP is #0. Thread-Index: AQHarSC0GO2jdw5qG0qboSaRnupb37GlsrBG Date: Fri, 24 May 2024 03:03:10 +0000 Message-ID: References: <20240523231605.1652-1-ning.feng@intel.com> In-Reply-To: <20240523231605.1652-1-ning.feng@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: x-ms-publictraffictype: Email x-ms-traffictypediagnostic: MN6PR11MB8244:EE_|SA2PR11MB5100:EE_ x-ms-office365-filtering-correlation-id: 6ca67620-379a-42ce-0cb3-08dc7b9e0b4b x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam-message-info: =?us-ascii?Q?+ZfMknOkafUrnS/XUybj19sz6q0tOP9JjhqbgTsKJMjv+C6oriZk93q3AV6N?= =?us-ascii?Q?TyTWPZCeY3vnkC8S1QBjgTHjraMRTwOvtfDtJb3XxnnLGpDsIp8aIRVkq+S6?= =?us-ascii?Q?TvSAuQO2ZpEU5gjcQ/VF5fsn3ZMXq55fF7zXm+u8YCgPuDXe+FeuSjT3MWlM?= =?us-ascii?Q?FX0+C4Uw8KgHWou4KajTwM+vg5+hlybWE39SMrHkogro+gA8PpkbZmjtFYGX?= =?us-ascii?Q?rCI54/9/x/vMAd9ZLCaC8rtsKIMoFZwxb3Q/IO+rEuIr8M1AIHPJO7uG+ILk?= =?us-ascii?Q?TYoFNIYBJNOJTifj5kwkNakxX4VZ0q+ttucLzrGuJkRzlp0r9lEISUaJb+wE?= =?us-ascii?Q?k89UldHYm9SG8KGQYRiMmcx+u0pWTzG0A7NVJDSSRQE7rRLYm2BwCtceXq1y?= =?us-ascii?Q?nfX5FjpeQnSx0a9iqPqrwafraExxmcZR2Z+E6MEuCBSnnL2+tj490RIw677i?= =?us-ascii?Q?++DyREba1NTZFyjAtH1vql2E1hxAGGJGiExkDEwaw10fXpZGqWGvzjQMOkLV?= =?us-ascii?Q?tQ4viwr/LUD7EUmiRo98Qdk7owo+JY08IXU9z4B7ncSZ3wJWZYHKbuH0xsAc?= =?us-ascii?Q?6ogQUCx0OIGSIyChMZNuRRIkQnMqYpq0j4dnyEwGE8PW+tRfJIIqU4Y+C5nP?= =?us-ascii?Q?kRQXxo2bwKPaB1cqSXvt+NIJWQI3QYgfW+kz7R4haPvK58KSTEYamZCQk+ga?= =?us-ascii?Q?c/0ITaKeAifWSWqqxYGqa3b2OfUkhJPT4HBxVtncMFi0z7KGb0X5qMJWUY9B?= =?us-ascii?Q?fGYk+Nmg0Hv3eJkuUQZJ3T9F8Xo88ddz3eKdRgd4IpzLQAGIKH8DbnY4tm1R?= =?us-ascii?Q?P0cALbay99lspeuN2jWE+dkOLTqFJg76DcY4R7cHdn5AMp/MCIj+Saqb97uv?= =?us-ascii?Q?z9ocMnxJpFix4/SmxsqYeyM3JVg6uJIv970PcXCrGRco3v3Q0A76a48oRIvf?= =?us-ascii?Q?yOxUAWdsBfrxnvkqpjRsTUwce5Nr4az1Bi5GXQOqWEm5i5nK4GZSe+f+aT+/?= =?us-ascii?Q?dz55Kkih7fqfwFw54LI3ehiWsr7AfLWL8D/JBBULFsSR6Dm6H6P64onJ9tZc?= =?us-ascii?Q?mYn1IqGrvn5VZvkP+s/58qAZpzeF29pCDu3vNFLl1bAdroa0qrM0FWODm6Ub?= =?us-ascii?Q?kMydlxYumiHT6VyGnmuYogLz3Tt5AzLdm9XbafgmFE4rkP3CnG4IhsW1PAUv?= =?us-ascii?Q?PPQU6yuhaDhl3NXPYvTX+umz2ETWwBPkkAQjcQJY9UrngYDPlYYNI1Zerdqh?= =?us-ascii?Q?mrT4mT3VzjdzPaJBkomTZd1+eY5GFO8riOymW9lP9Q=3D=3D?= x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?OOPO6plgq6pRuk1oyIXD00YhE0sz9fAsT2owxwTlHt5oq7/FEDOZjFmI5lKd?= =?us-ascii?Q?OpZOnwcOUWY/+qNiUDhCinKM1F8rQKcrpl1eiNhpIWs1MRMLKDsmh8i3XqRO?= =?us-ascii?Q?NXs8IvllQWD7hF7wnSn+d1wfbHGHYgugMw7GuXXjFjqgPKcsO283yaTmsOdU?= =?us-ascii?Q?MbePv2NZ8TtPzGwd2s9kog5NgOTvMk9J+QG8EfK1ua2hF+b3PNtA//E3oLop?= =?us-ascii?Q?85HqSkITOSRABCLo94AIRH7oiNQJugNTlYDwwg/Cc8DRxMkCjme/6zpEOTDa?= =?us-ascii?Q?Ba4wyZThLSGHmz1b0wQ/eEKESxSoIh2Ds+LJx+RHgGxMbZxp4jCzV5cAkn7Q?= =?us-ascii?Q?/tLjVfATbsF3DxMbjPN3IuFBCTguGOn9wjLYAA78qh+/mcMvb44Hn1x/Q516?= =?us-ascii?Q?afBH2sgvG8JyqFL7AaFH0Y1XMJ162hR1WNMCb4/tteHVr9/wIbcIeAtM6igE?= =?us-ascii?Q?nXKd/I8HDGxskQB5XmMXhrBP4yUUvCQL5B2gq+QD8R00KjyKq2Piaztr8Oyh?= =?us-ascii?Q?aLG1bqmsJMn+JAXGDpheiujPXnuyKfZJfceeK1/AYf1iqKzEUgR20wDfF2K7?= =?us-ascii?Q?VimvKJFK1oDiLbHvUMBcdtm/ysFbY778FKu86DK93hYdqxeshGCmA3yWJVuE?= =?us-ascii?Q?3h9Ht0i5F+kHQCVdN1COskpy2Qf6LNeNgXndbMBZ6mG+28KM+BNAnQpe6ZiW?= =?us-ascii?Q?86Q6LNzAUWAWk8zieaHG+m1l85XbkwO/M2x0tg/gcRDIdazyXeazXw2Fgr4N?= =?us-ascii?Q?Sn5MFuCMjxzIq5pYQjz5/YDqL55kgafYAoBQOUOndCYdIkCz212D0u7MCiID?= =?us-ascii?Q?2TlNgeaoW9mL3+WMvIiwHKssmZTbsULUYa9xznAI/GQgusv411Mt9aYkiz0M?= =?us-ascii?Q?v3KE0bc7sElETG/3Scc455MYZrhMtGfKtVu0LmJLx4nUNR6aJtNCDM3jOZ8g?= =?us-ascii?Q?f4iopErE8KWxW6MK/YJ/G7Ml1GZD6mMzyDhhUbUJBzcXZLinQYim+Xm8GiI+?= =?us-ascii?Q?Pc8209RX1muRHzxIp1gkQ9VBFUq2toFTxVP0xNNAvryCjVuVVHtVR9nxg+GC?= =?us-ascii?Q?eworQQcy61pGceMWT4V/j1nH0rGDY2iTt9tfpHpkThaKCZV+Gn8Sqw1pe3Oq?= =?us-ascii?Q?be/Hy+SL1Ooa8Wn6F1MX8RgbnxrRRUge5XUhOVK3AyXfXM3dXd/Exm2WL5DN?= =?us-ascii?Q?JYXKWk4NYxwo6/tgrp0Y0aVqF8vyMUdn4wG0FLtlabAQ1qELBByzGxwc2Z+3?= =?us-ascii?Q?QuWaoZk4TMPfHMZXRe6NQKN6bqPJ2c92YwXAWzz/Ucd0MGMCckfpyook5Szy?= =?us-ascii?Q?+/+tSub4iY+3FmuGkPM75OiKGSU2Gkly4nk5Tjd3VngDSwE+6KyvVBhyRzzH?= =?us-ascii?Q?ZWvBRw6k4LbjwQym+adIIAkURmzGXpoZiQKBqaA1w8LximUDS4WVIbp1TJ3l?= =?us-ascii?Q?UXj8S7D4w8Zv9pqHNbfiDKRL3EhNle4rTv/2SOOW2zfvtINP+BNJN/Dt6F/u?= =?us-ascii?Q?S1x4BmW9WqUfZmazFObDz3hPC+y9ATPRmZqPldUXZuI4BLsF3BB3qlbdg1hS?= =?us-ascii?Q?HIFwUSBTfnVrMngrx0g=3D?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MN6PR11MB8244.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 6ca67620-379a-42ce-0cb3-08dc7b9e0b4b X-MS-Exchange-CrossTenant-originalarrivaltime: 24 May 2024 03:03:10.3907 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: Y/fgYM5uIdMnCI2rc+33KkLJxHF/ofXfue+AjPDpSzh6kZo24wbSJM/AV6Xcsbf9ZZQHsA2JRDKAWwTVeAtimg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB5100 X-OriginatorOrg: intel.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Thu, 23 May 2024 20:03:14 -0700 Resent-From: ray.ni@intel.com Reply-To: devel@edk2.groups.io,ray.ni@intel.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: LrG6KucKgZ7xs9YhIPbGn1RHx7686176AA= Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_MN6PR11MB82442612AB473C619874D1D48CF52MN6PR11MB8244namp_" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=cYaqcea8; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=intel.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io --_000_MN6PR11MB82442612AB473C619874D1D48CF52MN6PR11MB8244namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Ning, I missed one minor issue with your patch. Can you check if the following GetBspNumber() call can be removed? if (FirstMpHandOff =3D=3D NULL) { ... } else { ... CpuMpData->CpuCount =3D MaxLogicalProcessorNumber; CpuMpData->BspNumber =3D GetBspNumber (FirstMpHandOff); CpuInfoInHob =3D (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoIn= Hob; BTW, please remove "Pkg-Module:" from the subject. Thanks, Ray ________________________________ From: Feng, Ning Sent: Friday, May 24, 2024 7:16 To: devel@edk2.groups.io Cc: Feng, Ning ; Ni, Ray Subject: [PATCH] Pkg-Module:UefiCpuPkg/MpLib:Do not assume BSP is #0. REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D4778 MPInitlib have wrong expectation that BSP index should always be 0 in MpInitLibInitialize(), SwitchBsp(),ApWakeupFunction(). That will cause the data mismatch, if the initial BSP is not 0. Cc: Ray Ni Signed-off-by: Ning Feng --- UefiCpuPkg/Library/MpInitLib/MpLib.c | 34 ++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpIn= itLib/MpLib.c index d724456502..ae279c6ceb 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -114,6 +114,10 @@ FutureBSPProc ( SaveVolatileRegisters (&DataInHob->APInfo.VolatileRegisters); AsmExchangeRole (&DataInHob->APInfo, &DataInHob->BSPInfo); RestoreVolatileRegisters (&DataInHob->APInfo.VolatileRegisters, FALSE); + // + // Restore VolatileReg saved in CpuMpData->CpuData + // + CopyMem (&DataInHob->CpuData[DataInHob->BspNumber].VolatileRegisters, &D= ataInHob->APInfo.VolatileRegisters, sizeof (CPU_VOLATILE_REGISTERS)); } /** @@ -761,11 +765,11 @@ ApWakeupFunction ( BistData =3D (UINT32)ApStackData->Bist; // - // CpuMpData->CpuData[0].VolatileRegisters is initialized based on B= SP environment, + // CpuMpData->CpuData[BspNumber].VolatileRegisters is initialized ba= sed on BSP environment, // to initialize AP in InitConfig path. - // NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters= points to a different IDT shared by all APs. + // NOTE: IDTR.BASE stored in CpuMpData->CpuData[BspNumber].VolatileR= egisters points to a different IDT shared by all APs. // - RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, = FALSE); + RestoreVolatileRegisters (&CpuMpData->CpuData[CpuMpData->BspNumber].= VolatileRegisters, FALSE); InitializeApData (CpuMpData, ProcessorNumber, BistData, ApTopOfStack= ); ApStartupSignalBuffer =3D CpuMpData->CpuData[ProcessorNumber].Startu= pApSignal; } else { @@ -798,10 +802,10 @@ ApWakeupFunction ( // 1. AP is re-enabled after it's disabled, in either PEI or DXE p= hase. // 2. AP is initialized in DXE phase. // In either case, use the volatile registers value derived from B= SP. - // NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegiste= rs points to a + // NOTE: IDTR.BASE stored in CpuMpData->CpuData[BspNumber].Volatil= eRegisters points to a // different IDT shared by all APs. // - RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters= , FALSE); + RestoreVolatileRegisters (&CpuMpData->CpuData[CpuMpData->BspNumber= ].VolatileRegisters, FALSE); } else { if (CpuMpData->ApLoopMode =3D=3D ApInHltLoop) { // @@ -927,7 +931,7 @@ DxeApEntryPoint ( AsmWriteMsr64 (MSR_IA32_EFER, EferMsr.Uint64); } - RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, FALS= E); + RestoreVolatileRegisters (&CpuMpData->CpuData[CpuMpData->BspNumber].Vola= tileRegisters, FALSE); InterlockedIncrement ((UINT32 *)&CpuMpData->FinishedCount); PlaceAPInMwaitLoopOrRunLoop ( CpuMpData->ApLoopMode, @@ -2151,7 +2155,12 @@ MpInitLibInitialize ( CpuMpData->BackupBufferSize =3D ApResetVectorSizeBelow1Mb; CpuMpData->WakeupBuffer =3D (UINTN)-1; CpuMpData->CpuCount =3D 1; - CpuMpData->BspNumber =3D 0; + if (MpHandOff =3D=3D NULL) { + CpuMpData->BspNumber =3D 0; + } else { + CpuMpData->BspNumber =3D GetBspNumber (MpHandOff); + } + CpuMpData->WaitEvent =3D NULL; CpuMpData->SwitchBspFlag =3D FALSE; CpuMpData->CpuData =3D (CPU_AP_DATA *)(CpuMpData + 1); @@ -2186,11 +2195,11 @@ MpInitLibInitialize ( // Don't pass BSP's TR to APs to avoid AP init failure. // VolatileRegisters.Tr =3D 0; - CopyMem (&CpuMpData->CpuData[0].VolatileRegisters, &VolatileRegisters, s= izeof (VolatileRegisters)); + CopyMem (&CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, &V= olatileRegisters, sizeof (VolatileRegisters)); // // Set BSP basic information // - InitializeApData (CpuMpData, 0, 0, CpuMpData->Buffer + ApStackSize); + InitializeApData (CpuMpData, CpuMpData->BspNumber, 0, CpuMpData->Buffer = + ApStackSize * (CpuMpData->BspNumber + 1)); // // Save assembly code information // @@ -2615,7 +2624,12 @@ SwitchBSPWorker ( SaveVolatileRegisters (&CpuMpData->BSPInfo.VolatileRegisters); AsmExchangeRole (&CpuMpData->BSPInfo, &CpuMpData->APInfo); RestoreVolatileRegisters (&CpuMpData->BSPInfo.VolatileRegisters, FALSE); - + // + // Restore VolatileRegs saved in CpuMpData->CpuData + // Don't pass BSP's TR to APs to avoid AP init failure. + // + CopyMem (&CpuMpData->CpuData[CpuMpData->NewBspNumber].VolatileRegisters,= &CpuMpData->BSPInfo.VolatileRegisters, sizeof (CPU_VOLATILE_REGISTERS)); + CpuMpData->CpuData[CpuMpData->NewBspNumber].VolatileRegisters.Tr =3D 0; // // Set the BSP bit of MSR_IA32_APIC_BASE on new BSP // -- 2.25.1 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119181): https://edk2.groups.io/g/devel/message/119181 Mute This Topic: https://groups.io/mt/106263733/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --_000_MN6PR11MB82442612AB473C619874D1D48CF52MN6PR11MB8244namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable
Ning,
I missed one minor issue with your patch.

Can you check if the following GetBspNumber() call can be removed?

  if (FirstMpHandOff =3D=3D NULL) {
...
  } else {
...
    CpuMpData->CpuCount  =3D MaxLogicalProcessorNumber;
    CpuMpData->BspNumber =3D GetBspNumber (FirstMpHandOff);
    CpuInfoInHob         =3D (CPU_INFO_IN_HOB= *)(UINTN)CpuMpData->CpuInfoInHob;


BTW, please remove "Pkg-Module:" from the subject.

Thanks,
Ray
From: Feng, Ning <ning.f= eng@intel.com>
Sent: Friday, May 24, 2024 7:16
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Feng, Ning <ning.feng@intel.com>; Ni, Ray <ray.ni@intel= .com>
Subject: [PATCH] Pkg-Module:UefiCpuPkg/MpLib:Do not assume BSP is #0= .
 
REF:https://bugzilla.tianocore.org/show_bug.cgi?id= =3D4778
MPInitlib have wrong expectation that BSP index should always be 0 in
MpInitLibInitialize(), SwitchBsp(),ApWakeupFunction().
That will cause the data mismatch, if the initial BSP is not 0.
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Ning Feng <ning.feng@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 34 ++++++++++++++++++++-------= -
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpIn= itLib/MpLib.c
index d724456502..ae279c6ceb 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -114,6 +114,10 @@ FutureBSPProc (
   SaveVolatileRegisters (&DataInHob->APInfo.VolatileRegis= ters);

   AsmExchangeRole (&DataInHob->APInfo, &DataInHob->= ;BSPInfo);

   RestoreVolatileRegisters (&DataInHob->APInfo.VolatileRe= gisters, FALSE);

+  //

+  // Restore VolatileReg saved in CpuMpData->CpuData

+  //

+  CopyMem (&DataInHob->CpuData[DataInHob->BspNumber].Volati= leRegisters, &DataInHob->APInfo.VolatileRegisters, sizeof (CPU_VOLAT= ILE_REGISTERS));

 }

 

 /**

@@ -761,11 +765,11 @@ ApWakeupFunction (
       BistData     =3D (= UINT32)ApStackData->Bist;

 

       //

-      // CpuMpData->CpuData[0].VolatileRegiste= rs is initialized based on BSP environment,

+      // CpuMpData->CpuData[BspNumber].Volatil= eRegisters is initialized based on BSP environment,

       //   to initialize AP in Ini= tConfig path.

-      // NOTE: IDTR.BASE stored in CpuMpData->= CpuData[0].VolatileRegisters points to a different IDT shared by all APs.
+      // NOTE: IDTR.BASE stored in CpuMpData->= CpuData[BspNumber].VolatileRegisters points to a different IDT shared by al= l APs.

       //

-      RestoreVolatileRegisters (&CpuMpData-&g= t;CpuData[0].VolatileRegisters, FALSE);

+      RestoreVolatileRegisters (&CpuMpData-&g= t;CpuData[CpuMpData->BspNumber].VolatileRegisters, FALSE);

       InitializeApData (CpuMpData, Processor= Number, BistData, ApTopOfStack);

       ApStartupSignalBuffer =3D CpuMpData-&g= t;CpuData[ProcessorNumber].StartupApSignal;

     } else {

@@ -798,10 +802,10 @@ ApWakeupFunction (
         // 1. AP is re-enabled aft= er it's disabled, in either PEI or DXE phase.

         // 2. AP is initialized in= DXE phase.

         // In either case, use the= volatile registers value derived from BSP.

-        // NOTE: IDTR.BASE stored in Cp= uMpData->CpuData[0].VolatileRegisters points to a

+        // NOTE: IDTR.BASE stored in Cp= uMpData->CpuData[BspNumber].VolatileRegisters points to a

         //   different I= DT shared by all APs.

         //

-        RestoreVolatileRegisters (&= CpuMpData->CpuData[0].VolatileRegisters, FALSE);

+        RestoreVolatileRegisters (&= CpuMpData->CpuData[CpuMpData->BspNumber].VolatileRegisters, FALSE);
       } else {

         if (CpuMpData->ApLoopMo= de =3D=3D ApInHltLoop) {

           //

@@ -927,7 +931,7 @@ DxeApEntryPoint (
     AsmWriteMsr64 (MSR_IA32_EFER, EferMsr.Uint64);

   }

 

-  RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileReg= isters, FALSE);

+  RestoreVolatileRegisters (&CpuMpData->CpuData[CpuMpData->= BspNumber].VolatileRegisters, FALSE);

   InterlockedIncrement ((UINT32 *)&CpuMpData->FinishedCou= nt);

   PlaceAPInMwaitLoopOrRunLoop (

     CpuMpData->ApLoopMode,

@@ -2151,7 +2155,12 @@ MpInitLibInitialize (
   CpuMpData->BackupBufferSize =3D ApResetVectorSizeBelow1Mb;<= br>
   CpuMpData->WakeupBuffer     =3D (UINTN)= -1;

   CpuMpData->CpuCount      &nbs= p;  =3D 1;

-  CpuMpData->BspNumber        = =3D 0;

+  if (MpHandOff =3D=3D NULL) {

+    CpuMpData->BspNumber =3D 0;

+  } else {

+    CpuMpData->BspNumber =3D GetBspNumber (MpHandOff);
+  }

+

   CpuMpData->WaitEvent      &nb= sp; =3D NULL;

   CpuMpData->SwitchBspFlag    =3D FALSE;

   CpuMpData->CpuData       = ;   =3D (CPU_AP_DATA *)(CpuMpData + 1);

@@ -2186,11 +2195,11 @@ MpInitLibInitialize (
   // Don't pass BSP's TR to APs to avoid AP init failure.

   //

   VolatileRegisters.Tr =3D 0;

-  CopyMem (&CpuMpData->CpuData[0].VolatileRegisters, &Vola= tileRegisters, sizeof (VolatileRegisters));

+  CopyMem (&CpuMpData->CpuData[CpuMpData->BspNumber].Volati= leRegisters, &VolatileRegisters, sizeof (VolatileRegisters));

   //

   // Set BSP basic information

   //

-  InitializeApData (CpuMpData, 0, 0, CpuMpData->Buffer + ApStackSi= ze);

+  InitializeApData (CpuMpData, CpuMpData->BspNumber, 0, CpuMpData-= >Buffer + ApStackSize * (CpuMpData->BspNumber + 1));

   //

   // Save assembly code information

   //

@@ -2615,7 +2624,12 @@ SwitchBSPWorker (
   SaveVolatileRegisters (&CpuMpData->BSPInfo.VolatileRegi= sters);

   AsmExchangeRole (&CpuMpData->BSPInfo, &CpuMpData-&g= t;APInfo);

   RestoreVolatileRegisters (&CpuMpData->BSPInfo.VolatileR= egisters, FALSE);

-

+  //

+  // Restore VolatileRegs saved in CpuMpData->CpuData

+  // Don't pass BSP's TR to APs to avoid AP init failure.

+  //

+  CopyMem (&CpuMpData->CpuData[CpuMpData->NewBspNumber].Vol= atileRegisters, &CpuMpData->BSPInfo.VolatileRegisters, sizeof (CPU_V= OLATILE_REGISTERS));

+  CpuMpData->CpuData[CpuMpData->NewBspNumber].VolatileRegisters= .Tr =3D 0;

   //

   // Set the BSP bit of MSR_IA32_APIC_BASE on new BSP

   //

--
2.25.1

_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#119181) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--_000_MN6PR11MB82442612AB473C619874D1D48CF52MN6PR11MB8244namp_--