From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mx.groups.io with SMTP id smtpd.web12.4709.1587556057934977178 for ; Wed, 22 Apr 2020 04:47:38 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: ray.ni@intel.com) IronPort-SDR: OF+rLGnlz6aygMFotpda8EilwQDju5tQ+FgZmnKId+Oi/S6MpfGG+lAWsKxd2PkP86FvHSHfu5 J5nez3D2n6kg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Apr 2020 04:47:37 -0700 IronPort-SDR: QaRJvw+UVLjqI7eFCdZn7A5Jiv2kG/8D2hQvrkNnp9w5CcpnwBMpRZR0hJ0xkVEmpSl2lpvje9 sLF+XRRcygjg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,414,1580803200"; d="scan'208";a="279989697" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga004.fm.intel.com with ESMTP; 22 Apr 2020 04:47:36 -0700 Received: from fmsmsx120.amr.corp.intel.com (10.18.124.208) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 22 Apr 2020 04:47:36 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx120.amr.corp.intel.com (10.18.124.208) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 22 Apr 2020 04:47:35 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.225]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.209]) with mapi id 14.03.0439.000; Wed, 22 Apr 2020 19:47:33 +0800 From: "Ni, Ray" To: "Dong, Eric" , "devel@edk2.groups.io" CC: Laszlo Ersek , "Kumar, Chandana C" Subject: Re: [PATCH v2 1/2] UefiCpuPkg/MpInitLib: Restore IDT context for APs. Thread-Topic: [PATCH v2 1/2] UefiCpuPkg/MpInitLib: Restore IDT context for APs. Thread-Index: AQHWGISYFrCXed+USUiWlLZGxVVERaiFAQLA Date: Wed, 22 Apr 2020 11:47:33 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C5087DB@SHSMSX104.ccr.corp.intel.com> References: <20200422090111.423-1-eric.dong@intel.com> In-Reply-To: <20200422090111.423-1-eric.dong@intel.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: ray.ni@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Eric, It's natural to use the volatile registers value derived from BSP in ApInit= Reconfig path. So I still prefer to use the code I suggested in the review comment to the = v1 patch. We can remove the below line that specially for ApInitReconfig path in MpIn= itLbInitialize(). https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/MpInitLib/= MpLib.c#L1783: CopyMem (&CpuMpData->CpuData[Index].VolatileRegisters, &VolatileRegis= ters, sizeof (CPU_VOLATILE_REGISTERS)); Thanks, Ray > -----Original Message----- > From: Dong, Eric > Sent: Wednesday, April 22, 2020 5:01 PM > To: devel@edk2.groups.io > Cc: Ni, Ray ; Laszlo Ersek ; Kumar, > Chandana C > Subject: [PATCH v2 1/2] UefiCpuPkg/MpInitLib: Restore IDT context for APs= . >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2683 >=20 > This patch fixes an assertion because AP can't find the CpuMpData. > When AP is waken up through Init-Sipi-Sipi, AP's IDT should > be restored to pre-allocated buffer so AP can get the CpuMpData > through the IDT base address. > Current code already has logic to handle this when CpuMpData-> > InitFlag is ApInitConfig but misses the logic > when CpuMpData->InitFlag is ApInitReconfig. > This patch fixes this gap. >=20 > Cc: Ray Ni > Cc: Laszlo Ersek > Cc: Chandana Kumar > Signed-off-by: Eric Dong > --- >=20 > V2: >=20 > 1. Enhance code comments. >=20 >=20 > UefiCpuPkg/Library/MpInitLib/MpLib.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) >=20 > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c > b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index 64a4c3546e..2e87aa1f06 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -692,6 +692,16 @@ ApWakeupFunction ( > // >=20 > RestoreVolatileRegisters (&CpuMpData- > >CpuData[ProcessorNumber].VolatileRegisters, TRUE); >=20 > } else { >=20 > + if (CpuMpData->InitFlag =3D=3D ApInitReconfig) { >=20 > + // >=20 > + // Initialize AP volatile registers in ApInitReconfig path. >=20 > + // ApInitReconfig happens when: >=20 > + // 1. AP is re-enabled after it's disabled, in either PEI or D= XE phase. >=20 > + // 2. AP is initialized in DXE phase. >=20 > + // >=20 > + RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegis= ters, > FALSE); >=20 > + } >=20 > + >=20 > // >=20 > // The CPU driver might not flush TLB for APs on spot after upda= ting >=20 > // page attributes. AP in mwait loop mode needs to take care of = it > when >=20 > -- > 2.23.0.windows.1