From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.88; helo=mga01.intel.com; envelope-from=eric.dong@intel.com; receiver=edk2-devel@lists.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id E62FA210ED77D for ; Sun, 12 Aug 2018 18:55:39 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Aug 2018 18:55:39 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,231,1531810800"; d="scan'208";a="80765388" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga001.fm.intel.com with ESMTP; 12 Aug 2018 18:54:45 -0700 Received: from fmsmsx115.amr.corp.intel.com (10.18.116.19) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Sun, 12 Aug 2018 18:54:45 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by fmsmsx115.amr.corp.intel.com (10.18.116.19) with Microsoft SMTP Server (TLS) id 14.3.319.2; Sun, 12 Aug 2018 18:54:45 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.226]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.240]) with mapi id 14.03.0319.002; Mon, 13 Aug 2018 09:54:43 +0800 From: "Dong, Eric" To: Laszlo Ersek , "edk2-devel@lists.01.org" CC: "Ni, Ruiyu" Thread-Topic: [edk2] [Patch v3 1/5] UefiCpuPkg/PiSmmCpuDxeSmm: Use GDT/IDT saved in Smram. Thread-Index: AQHUMGFeiFJhgRoSsUW7WIzp01uhFKS4mfCAgARVf/A= Date: Mon, 13 Aug 2018 01:54:42 +0000 Message-ID: References: <20180810041909.12776-1-eric.dong@intel.com> <20180810041909.12776-2-eric.dong@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [Patch v3 1/5] UefiCpuPkg/PiSmmCpuDxeSmm: Use GDT/IDT saved in Smram. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 13 Aug 2018 01:55:40 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Laszlo, > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Laszlo Ersek > Sent: Friday, August 10, 2018 11:40 PM > To: Dong, Eric ; edk2-devel@lists.01.org > Cc: Ni, Ruiyu > Subject: Re: [edk2] [Patch v3 1/5] UefiCpuPkg/PiSmmCpuDxeSmm: Use > GDT/IDT saved in Smram. >=20 > On 08/10/18 06:19, Eric Dong wrote: > > Current implementation copies GDT/IDT at SmmReadyToLock point from > > ACPI NVS memory to Smram. Later at S3 resume phase, it restores memory > > saved in Smram to ACPI NVS. Driver can directly use GDT/IDT saved in > > Smram instead of restore the original ACPI NVS memory. > > > > This patch do this change. > > > > Test Done: > > Do the OS boot and S3 resume test. > > > > Cc: Laszlo Ersek > > Cc: Ruiyu Ni > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Eric Dong > > --- > > UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 11 ++++------- > > 1 file changed, 4 insertions(+), 7 deletions(-) > > > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > > b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > > index 0b8ef70359..764d8276d3 100644 > > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > > @@ -448,13 +448,6 @@ PrepareApStartupVector ( > > CopyMem ((VOID *) (UINTN) &mExchangeInfo->GdtrProfile, (VOID *) > (UINTN) mAcpiCpuData.GdtrProfile, sizeof (IA32_DESCRIPTOR)); > > CopyMem ((VOID *) (UINTN) &mExchangeInfo->IdtrProfile, (VOID *) > > (UINTN) mAcpiCpuData.IdtrProfile, sizeof (IA32_DESCRIPTOR)); > > > > - // > > - // Copy AP's GDT, IDT and Machine Check handler from SMRAM to ACPI > > NVS memory > > - // > > - CopyMem ((VOID *) mExchangeInfo->GdtrProfile.Base, mGdtForAp, > > mExchangeInfo->GdtrProfile.Limit + 1); > > - CopyMem ((VOID *) mExchangeInfo->IdtrProfile.Base, mIdtForAp, > > mExchangeInfo->IdtrProfile.Limit + 1); > > - CopyMem ((VOID *)(UINTN) > mAcpiCpuData.ApMachineCheckHandlerBase, > > mMachineCheckHandlerForAp, > mAcpiCpuData.ApMachineCheckHandlerSize); > > - > > mExchangeInfo->StackStart =3D (VOID *) (UINTN) > mAcpiCpuData.StackAddress; > > mExchangeInfo->StackSize =3D mAcpiCpuData.StackSize; > > mExchangeInfo->BufferStart =3D (UINT32) StartupVector; @@ -901,6 > > +894,10 @@ GetAcpiCpuData ( > > CopyMem (mGdtForAp, (VOID *)Gdtr->Base, Gdtr->Limit + 1); > > CopyMem (mIdtForAp, (VOID *)Idtr->Base, Idtr->Limit + 1); > > CopyMem (mMachineCheckHandlerForAp, (VOID > > *)(UINTN)mAcpiCpuData.ApMachineCheckHandlerBase, > > mAcpiCpuData.ApMachineCheckHandlerSize); > > + > > + Gdtr->Base =3D (UINTN)mGdtForAp; > > + Idtr->Base =3D (UINTN)mIdtForAp; > > + mAcpiCpuData.ApMachineCheckHandlerBase =3D > > + (EFI_PHYSICAL_ADDRESS)(UINTN)mMachineCheckHandlerForAp; > > } > > > > /** > > >=20 > This patch looks good to me (I'm ready to give R-b), but I think we can s= implify > the code further. Can we replace the mGdtForAp, mIdtForAp and > mMachineCheckHandlerForAp global variables, with GdtForAp, IdtForAp and > MachineCheckHandlerForAp local variables, used only in the > GetAcpiCpuData() function? > Agree to do this change in my next version change. Will send new patches af= ter finishing AllocateZeroPages related discussion. > If you prefer to do that as an incremental patch, that's fine too. >=20 > Thanks, > Laszlo > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel