From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.100; helo=mga07.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (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 2C0B021165247 for ; Thu, 11 Oct 2018 00:37:19 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Oct 2018 00:37:19 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,367,1534834800"; d="scan'208";a="264756162" Received: from ray-dev.ccr.corp.intel.com (HELO [10.239.9.11]) ([10.239.9.11]) by orsmga005.jf.intel.com with ESMTP; 11 Oct 2018 00:36:57 -0700 To: Eric Dong , edk2-devel@lists.01.org Cc: Laszlo Ersek , Jian J Wang References: <20181011070553.6996-1-eric.dong@intel.com> From: "Ni, Ruiyu" Message-ID: <9d1eca44-83fe-8d83-e981-7b1f81fe4934@Intel.com> Date: Thu, 11 Oct 2018 15:38:06 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20181011070553.6996-1-eric.dong@intel.com> Subject: Re: [Patch v5] UefiCpuPkg/S3Resume2Pei: disable paging before creating new page table. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Oct 2018 07:37:20 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 10/11/2018 3:05 PM, Eric Dong wrote: > V5: > 1. Add ASSERT to indicate this assumption that environment is 32 bit mode. > 2. Add description in INF about this driver's expected result > in different environment. > > V4: > Only disable paging when it is enabled. > > V3 changes: > No need to change inf file. > > V2 changes: > Only disable paging in 32 bit mode, no matter it is enable or not. > > V1 changes: > PEI Stack Guard needs to enable paging. This might cause #GP if code > trying to write CR3 register with PML4 page table while the processor > is enabled with PAE paging. > > Simply disabling paging before updating CR3 can solve this conflict. > > It's an regression caused by change: 0a0d5296e448fc350de1594c49b9c0deff7fad60 > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1232 > > Cc: Ruiyu Ni > Cc: Laszlo Ersek > Cc: Jian J Wang > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong > --- > UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 17 +++++++++++++++++ > UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf | 10 ++++++++++ > 2 files changed, 27 insertions(+) > > diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c > index f164c1713b..8415ab1583 100644 > --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c > +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c > @@ -964,6 +964,7 @@ S3RestoreConfig2 ( > VOID *GuidHob; > BOOLEAN Build4GPageTableOnly; > BOOLEAN InterruptStatus; > + IA32_CR0 CR0Reg; > > TempAcpiS3Context = 0; > TempEfiBootScriptExecutorVariable = 0; > @@ -1045,6 +1046,13 @@ S3RestoreConfig2 ( > // > GuidHob = GetFirstGuidHob (&gEfiAcpiVariableGuid); > if (GuidHob != NULL) { > + // > + // Below SwitchStack/AsmEnablePaging64 function has > + // assumption that it's in 32 bits mode now. > + // Add ASSERT code to indicate this assumption. > + // > + ASSERT(sizeof (UINTN) == sizeof (UINT32)); > + > Status = PeiServicesLocatePpi ( > &gPeiSmmAccessPpiGuid, > 0, > @@ -1105,6 +1113,15 @@ S3RestoreConfig2 ( > // > SetInterruptState (InterruptStatus); > > + CR0Reg.UintN = AsmReadCr0 (); > + if (CR0Reg.Bits.PG != 0) { > + // > + // We're in 32-bit mode, with paging enabled. We can't set CR3 to > + // the 64-bit page tables without first disabling paging. > + // > + CR0Reg.Bits.PG = 0; > + AsmWriteCr0 (CR0Reg.UintN); > + } > AsmWriteCr3 ((UINTN)SmmS3ResumeState->SmmS3Cr3); > > // > diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf > index 6ce1bf944c..1d0740526f 100644 > --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf > +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf > @@ -33,6 +33,16 @@ > # VALID_ARCHITECTURES = IA32 X64 > # > > +# > +# This module is not always workable in IA32 and X64 mode. It has below result: > +# when it works with SMM mode: > +# =============================== > +# SMM:used SMM:unused > +# PEI:IA32 works works > +# PEI:X64 fails works > +# =============================== > +# > + > [Sources] > S3Resume.c > > Can you change CR0Reg to Cr0? With this change, Reviewed-by: Ruiyu Ni -- Thanks, Ray