Re: [Patch] UefiCpuPkg/S3Resume2Pei: disable paging before creating new page table.

["Yao, Jiewen" <jiewen.yao@intel.com>]

Hey
I do not think we need add if (sizeof (UINTN) == sizeof (UINT32))

This piece of code assume PEI is 32 bit.
The following code AsmEnablePaging64() does not work for X64.

Thank you
Yao Jiewen

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Eric Dong
> Sent: Wednesday, October 10, 2018 3:44 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [edk2] [Patch] UefiCpuPkg/S3Resume2Pei: disable paging before
> creating new page table.
> 
> 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
> 
> Change-Id: I99bfdba5daa48a95a4c4ef97eeca1af086558957
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by:Eric Dong <eric.dong@intel.com>
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> index f164c1713b..c059c42db5 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;
> @@ -1105,6 +1106,17 @@ S3RestoreConfig2 (
>        //
>        SetInterruptState (InterruptStatus);
> 
> +      if (sizeof (UINTN) == sizeof (UINT32)) {
> +        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);
> 
>        //
> --
> 2.15.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel