1. Patchew
  2. EDK2
  3. [edk2] [PATCH 0/3] UefiCpuPkg/PiSmmCpuDxeSmm: fix IA32 SmmStartup() regression on KVM
  4. View patch

[edk2] [PATCH 2/3] UefiCpuPkg/PiSmmCpuDxeSmm: remove unneeded DBs from IA32 SmmStartup()

Laszlo Ersek posted 3 patches 6 years, 9 months ago
[edk2] [PATCH 2/3] UefiCpuPkg/PiSmmCpuDxeSmm: remove unneeded DBs from IA32 SmmStartup()
Posted by Laszlo Ersek 6 years, 9 months ago 
The SmmStartup() executes in SMM, which is very similar to real mode. Add
"BITS 16" before it and "BITS 32" after it (just before the @32bit label).

Remove the manual 0x66 operand-size override prefixes, for selecting
32-bit operands -- the sizes of our operands trigger NASM to insert the
prefixes automatically in almost every spot. The one place where we have
to add it back manually is the LGDT instruction. (The 0x67 address-size
override prefix is also auto-generated.)

This patch causes NASM to generate byte-identical object code (determined
by disassembling both the pre-patch and post-patch versions, and comparing
the listings), except:

> @@ -158,7 +158,7 @@
>  00000142  6689D3            mov ebx,edx
>  00000145  66B800000000      mov eax,0x0
>  0000014B  0F22D8            mov cr3,eax
> -0000014E  67662E0F0155F6    o32 lgdt [cs:ebp-0xa]
> +0000014E  2E66670F0155F6    o32 lgdt [cs:ebp-0xa]
>  00000155  66B800000000      mov eax,0x0
>  0000015B  0F22E0            mov cr4,eax
>  0000015E  66B9800000C0      mov ecx,0xc0000080

The only difference is the prefix list order, it changes from:

- 0x67, 0x66, 0x2E

to

- 0x2E, 0x66, 0x67

(0x2E is "CS segment override").

Cc: Eric Dong <eric.dong@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=866
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
index 08534dba64b7..9231aa5b3ded 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
@@ -38,43 +38,42 @@ global ASM_PFX(gcSmmInitTemplate)
 
 ASM_PFX(gcSmiInitGdtr):
             DW      0
             DQ      0
 
 global ASM_PFX(SmmStartup)
+
+BITS 16
 ASM_PFX(SmmStartup):
-    DB      0x66
     mov     eax, 0x80000001             ; read capability
     cpuid
-    DB      0x66
     mov     ebx, edx                    ; rdmsr will change edx. keep it in ebx.
     DB      0x66, 0xb8                  ; mov eax, imm32
 ASM_PFX(gSmmCr3): DD 0
     mov     cr3, eax
-    DB      0x67, 0x66
-    lgdt    [cs:ebp + (ASM_PFX(gcSmiInitGdtr) - ASM_PFX(SmmStartup))]
+o32 lgdt    [cs:ebp + (ASM_PFX(gcSmiInitGdtr) - ASM_PFX(SmmStartup))]
     DB      0x66, 0xb8                  ; mov eax, imm32
 ASM_PFX(gSmmCr4): DD 0
     mov     cr4, eax
-    DB      0x66
     mov     ecx, 0xc0000080             ; IA32_EFER MSR
     rdmsr
-    DB      0x66
     test    ebx, BIT20                  ; check NXE capability
     jz      .1
     or      ah, BIT3                    ; set NXE bit
     wrmsr
 .1:
     DB      0x66, 0xb8                  ; mov eax, imm32
 ASM_PFX(gSmmCr0): DD 0
-    DB      0xbf, PROTECT_MODE_DS, 0    ; mov di, PROTECT_MODE_DS
+    mov     di, PROTECT_MODE_DS
     mov     cr0, eax
     DB      0x66, 0xea                  ; jmp far [ptr48]
 ASM_PFX(gSmmJmpAddr):
     DD      @32bit
     DW      PROTECT_MODE_CS
+
+BITS 32
 @32bit:
     mov     ds, edi
     mov     es, edi
     mov     fs, edi
     mov     gs, edi
     mov     ss, edi
-- 
2.14.1.3.gb7cf6e02401b


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 2/3] UefiCpuPkg/PiSmmCpuDxeSmm: remove unneeded DBs from IA32 SmmStartup()
Posted by Ni, Ruiyu 6 years, 9 months ago 
On 1/30/2018 11:33 PM, Laszlo Ersek wrote:
> The SmmStartup() executes in SMM, which is very similar to real mode. Add
> "BITS 16" before it and "BITS 32" after it (just before the @32bit label).
> 
> Remove the manual 0x66 operand-size override prefixes, for selecting
> 32-bit operands -- the sizes of our operands trigger NASM to insert the
> prefixes automatically in almost every spot. The one place where we have
> to add it back manually is the LGDT instruction. (The 0x67 address-size
> override prefix is also auto-generated.)
> 
> This patch causes NASM to generate byte-identical object code (determined
> by disassembling both the pre-patch and post-patch versions, and comparing
> the listings), except:
> 
>> @@ -158,7 +158,7 @@
>>   00000142  6689D3            mov ebx,edx
>>   00000145  66B800000000      mov eax,0x0
>>   0000014B  0F22D8            mov cr3,eax
>> -0000014E  67662E0F0155F6    o32 lgdt [cs:ebp-0xa]
>> +0000014E  2E66670F0155F6    o32 lgdt [cs:ebp-0xa]
>>   00000155  66B800000000      mov eax,0x0
>>   0000015B  0F22E0            mov cr4,eax
>>   0000015E  66B9800000C0      mov ecx,0xc0000080
> 
> The only difference is the prefix list order, it changes from:
> 
> - 0x67, 0x66, 0x2E
> 
> to
> 
> - 0x2E, 0x66, 0x67
> 
> (0x2E is "CS segment override").
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=866
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>   UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm | 13 ++++++-------
>   1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
> index 08534dba64b7..9231aa5b3ded 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
> @@ -38,43 +38,42 @@ global ASM_PFX(gcSmmInitTemplate)
>   
>   ASM_PFX(gcSmiInitGdtr):
>               DW      0
>               DQ      0
>   
>   global ASM_PFX(SmmStartup)
> +
> +BITS 16
>   ASM_PFX(SmmStartup):
> -    DB      0x66
>       mov     eax, 0x80000001             ; read capability
>       cpuid
> -    DB      0x66
>       mov     ebx, edx                    ; rdmsr will change edx. keep it in ebx.
>       DB      0x66, 0xb8                  ; mov eax, imm32
>   ASM_PFX(gSmmCr3): DD 0
>       mov     cr3, eax
> -    DB      0x67, 0x66
> -    lgdt    [cs:ebp + (ASM_PFX(gcSmiInitGdtr) - ASM_PFX(SmmStartup))]
> +o32 lgdt    [cs:ebp + (ASM_PFX(gcSmiInitGdtr) - ASM_PFX(SmmStartup))]
>       DB      0x66, 0xb8                  ; mov eax, imm32
>   ASM_PFX(gSmmCr4): DD 0
>       mov     cr4, eax
> -    DB      0x66
>       mov     ecx, 0xc0000080             ; IA32_EFER MSR
>       rdmsr
> -    DB      0x66
>       test    ebx, BIT20                  ; check NXE capability
>       jz      .1
>       or      ah, BIT3                    ; set NXE bit
>       wrmsr
>   .1:
>       DB      0x66, 0xb8                  ; mov eax, imm32
>   ASM_PFX(gSmmCr0): DD 0
> -    DB      0xbf, PROTECT_MODE_DS, 0    ; mov di, PROTECT_MODE_DS
> +    mov     di, PROTECT_MODE_DS
>       mov     cr0, eax
>       DB      0x66, 0xea                  ; jmp far [ptr48]
>   ASM_PFX(gSmmJmpAddr):
>       DD      @32bit
>       DW      PROTECT_MODE_CS
> +
> +BITS 32
>   @32bit:
>       mov     ds, edi
>       mov     es, edi
>       mov     fs, edi
>       mov     gs, edi
>       mov     ss, edi
> 
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

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

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.