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

["Ni, Ruiyu" <ruiyu.ni@Intel.com>]

On 10/9/2018 2:01 PM, Eric Dong wrote:
> V3 changes:
> No need to change inf file. Also update commit message to include regression info.
> 
> 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 <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>
> ---
>   UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> index f164c1713b..53ed76c6e6 100644
> --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> @@ -1105,6 +1105,14 @@ S3RestoreConfig2 (
>         //
>         SetInterruptState (InterruptStatus);
>   
> +      if (sizeof(UINTN) == sizeof(UINT32)) {
> +        //
> +        // Paging maybe enabled. If current mode is 32 bit mode and code try to
> +        // enable 64 bit mode page table, it will cause GP fault.
> +        // To avoid conflict configuration, disable paging first anyway.
> +        //
> +        AsmWriteCr0 (AsmReadCr0 () & (~BIT31));
> +      }
>         AsmWriteCr3 ((UINTN)SmmS3ResumeState->SmmS3Cr3);
>   
>         //
> 
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

-- 
Thanks,
Ray