Hi Mike
Thanks for the suggestion.
Previously, I just want to *ADD* without touch old logic. I will use your way 
to do it.

-----Original Message-----
From: Kinney, Michael D 
Sent: Thursday, November 26, 2015 2:01 AM
To: Yao, Jiewen; edk2-de...@ml01.01.org; Kinney, Michael D
Cc: Fan, Jeff
Subject: RE: [patch 2/2] UefiCpuPkg/PiSmmCpu: Always set WP in CR0.

Jiewen,

For IA32 assembly, can we combine into a single OR instruction that sets both 
page enable and WP?
For X64, does it make sense to use single OR instruction instead of 2 BTS 
instructions as well?

With those updates,

Reviewed-by: Michael Kinney <michael.d.kin...@intel.com>

Thanks,

Mike

> -----Original Message-----
> From: Yao, Jiewen
> Sent: Wednesday, November 25, 2015 4:35 AM
> To: edk2-de...@ml01.01.org
> Cc: Yao, Jiewen <jiewen....@intel.com>; Fan, Jeff <jeff....@intel.com>; 
> Kinney, Michael D <michael.d.kin...@intel.com>
> Subject: [patch 2/2] UefiCpuPkg/PiSmmCpu: Always set WP in CR0.
> 
> So that we can use write-protection for code later.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: "Yao, Jiewen" <jiewen....@intel.com>
> Cc: "Fan, Jeff" <jeff....@intel.com>
> Cc: "Kinney, Michael D" <michael.d.kin...@intel.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S   | 1 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm | 1 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S    | 1 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm  | 1 +
>  4 files changed, 4 insertions(+)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S
> index 7e1787c..8e64ce8 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S
> @@ -142,6 +142,7 @@ L13:
> 
>      movl    %cr0, %ebx
>      orl     $0x080000000, %ebx             # enable paging
> +    orl     $0x000010000, %ebx             # set WP
>      movl    %ebx, %cr0
>      leal    DSC_OFFSET(%edi),%ebx
>      movw    DSC_DS(%ebx),%ax
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm
> index e6af344..a955785 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm
> @@ -148,6 +148,7 @@ gSmiCr3     DD      ?
> 
>      mov     ebx, cr0
>      or      ebx, 080000000h             ; enable paging
> +    or      ebx, 000010000h             ; set WP
>      mov     cr0, ebx
>      lea     ebx, [edi + DSC_OFFSET]
>      mov     ax, [ebx + DSC_DS]
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S
> index 1d40819..8050a00 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S
> @@ -163,6 +163,7 @@ NxeDone:
>      wrmsr
>      movq    %cr0, %rbx
>      btsl    $31, %ebx
> +    btsl    $16, %ebx                   # set WP
>      movq    %rbx, %cr0
>      retf
>  LongMode:                               # long mode (64-bit code) starts here
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm
> index 6e1d3f1..db170d6 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm
> @@ -159,6 +159,7 @@ Base:
>      wrmsr
>      mov     rbx, cr0
>      bts     ebx, 31
> +    bts     ebx, 16                    ; set WP
>      mov     cr0, rbx
>      retf
>  @LongMode:                              ; long mode (64-bit code) starts here
> --
> 1.9.5.msysgit.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to