[edk2-devel] [PATCH 00/10] OvmfPkg/ResetVector: cleanup and add 5-level paging support.

[edk2-devel] [PATCH 00/10] OvmfPkg/ResetVector: cleanup and add 5-level paging support.

[Gerd Hoffmann]

So I ran with the suggestion by Laszlo to move the page table setup into
macros and untangle the non-CoCo / TDX / SEV code paths.  The first five
patches of the series are doing that (without functional changes).

Support for 5-level paging is added by the following five patches.  This
way it is indeed easier to understand.  Additional bonus is that the
patches can be splitted into smaller pieces and 5-level paging for the
three cases (non-CoCo / TDX / SEC) can be enabled independently.

The SEV patches (#9 + #10) are included here for completeness, but it is
probably a good idea to merge them only after 5-level paging support was
added to BaseMemEncryptSevLib.  This way we can turn on 5-level paging
support without breaking SEV.

Gerd Hoffmann (10):
  OvmfPkg/ResetVector: improve page table flag names
  OvmfPkg/ResetVector: add ClearOvmfPageTables macro
  OvmfPkg/ResetVector: add CreatePageTables4Level macro
  OvmfPkg/ResetVector: split TDX BSP workflow
  OvmfPkg/ResetVector: split SEV and non-CoCo workflows
  OvmfPkg/ResetVector: add 5-level paging support
  OvmfPkg/ResetVector: print post codes for 4/5 level paging
  OvmfPkg/ResetVector: wire up 5-level paging for TDX
  OvmfPkg/ResetVector: leave SEV VC handler installed longer
  OvmfPkg/ResetVector: wire up 5-level paging for SEV

 OvmfPkg/ResetVector/ResetVector.inf       |   1 +
 OvmfPkg/ResetVector/Ia32/AmdSev.asm       |  34 +--
 OvmfPkg/ResetVector/Ia32/IntelTdx.asm     |  17 +-
 OvmfPkg/ResetVector/Ia32/PageTables64.asm | 300 +++++++++++++++++-----
 OvmfPkg/ResetVector/Main.asm              |   4 +
 OvmfPkg/ResetVector/ResetVector.nasmb     |   1 +
 6 files changed, 264 insertions(+), 93 deletions(-)

-- 
2.43.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115802): https://edk2.groups.io/g/devel/message/115802
Mute This Topic: https://groups.io/mt/104506788/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 01/10] OvmfPkg/ResetVector: improve page table flag names

[Gerd Hoffmann]

Add comments, rename some of the PAGE_* flags and combined attributes.
Specifically use "LARGEPAGE" instead of "2M" because that bit is used
for both 2M and 1G large pages.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/ResetVector/Ia32/PageTables64.asm | 39 +++++++++++++----------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index 317cad430f29..6fec6f2beeea 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -10,6 +10,7 @@
 
 BITS    32
 
+; common for all levels
 %define PAGE_PRESENT            0x01
 %define PAGE_READ_WRITE         0x02
 %define PAGE_USER_SUPERVISOR    0x04
@@ -17,25 +18,29 @@ BITS    32
 %define PAGE_CACHE_DISABLE     0x010
 %define PAGE_ACCESSED          0x020
 %define PAGE_DIRTY             0x040
-%define PAGE_PAT               0x080
 %define PAGE_GLOBAL           0x0100
-%define PAGE_2M_MBO            0x080
-%define PAGE_2M_PAT          0x01000
+
+; page table entries (level 1)
+%define PAGE_PTE_PAT           0x080
+
+; page directory entries (level 2+)
+%define PAGE_PDE_LARGEPAGE     0x080
+%define PAGE_PDE_PAT         0x01000
 
 %define PAGE_4K_PDE_ATTR (PAGE_ACCESSED + \
                           PAGE_DIRTY + \
                           PAGE_READ_WRITE + \
                           PAGE_PRESENT)
 
-%define PAGE_2M_PDE_ATTR (PAGE_2M_MBO + \
-                          PAGE_ACCESSED + \
-                          PAGE_DIRTY + \
-                          PAGE_READ_WRITE + \
-                          PAGE_PRESENT)
+%define PAGE_PDE_LARGEPAGE_ATTR (PAGE_PDE_LARGEPAGE + \
+                                 PAGE_ACCESSED + \
+                                 PAGE_DIRTY + \
+                                 PAGE_READ_WRITE + \
+                                 PAGE_PRESENT)
 
-%define PAGE_PDP_ATTR (PAGE_ACCESSED + \
-                       PAGE_READ_WRITE + \
-                       PAGE_PRESENT)
+%define PAGE_PDE_DIRECTORY_ATTR (PAGE_ACCESSED + \
+                                 PAGE_READ_WRITE + \
+                                 PAGE_PRESENT)
 
 %define TDX_BSP         1
 %define TDX_AP          2
@@ -84,19 +89,19 @@ clearPageTablesMemoryLoop:
     ;
     ; Top level Page Directory Pointers (1 * 512GB entry)
     ;
-    mov     dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDP_ATTR
+    mov     dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDE_DIRECTORY_ATTR
     mov     dword[PT_ADDR (4)], edx
 
     ;
     ; Next level Page Directory Pointers (4 * 1GB entries => 4GB)
     ;
-    mov     dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + PAGE_PDP_ATTR
+    mov     dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + PAGE_PDE_DIRECTORY_ATTR
     mov     dword[PT_ADDR (0x1004)], edx
-    mov     dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + PAGE_PDP_ATTR
+    mov     dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + PAGE_PDE_DIRECTORY_ATTR
     mov     dword[PT_ADDR (0x100C)], edx
-    mov     dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + PAGE_PDP_ATTR
+    mov     dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + PAGE_PDE_DIRECTORY_ATTR
     mov     dword[PT_ADDR (0x1014)], edx
-    mov     dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + PAGE_PDP_ATTR
+    mov     dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + PAGE_PDE_DIRECTORY_ATTR
     mov     dword[PT_ADDR (0x101C)], edx
 
     ;
@@ -107,7 +112,7 @@ pageTableEntriesLoop:
     mov     eax, ecx
     dec     eax
     shl     eax, 21
-    add     eax, PAGE_2M_PDE_ATTR
+    add     eax, PAGE_PDE_LARGEPAGE_ATTR
     mov     [ecx * 8 + PT_ADDR (0x2000 - 8)], eax
     mov     [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
     loop    pageTableEntriesLoop
-- 
2.43.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115804): https://edk2.groups.io/g/devel/message/115804
Mute This Topic: https://groups.io/mt/104506790/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 02/10] OvmfPkg/ResetVector: add ClearOvmfPageTables macro

[Gerd Hoffmann]

Move code to clear the page tables to a nasm macro.
No functional change.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/ResetVector/Ia32/PageTables64.asm | 35 ++++++++++++-----------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index 6fec6f2beeea..378ba2feeb4f 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -45,6 +45,24 @@ BITS    32
 %define TDX_BSP         1
 %define TDX_AP          2
 
+;
+; For OVMF, build some initial page tables at
+; PcdOvmfSecPageTablesBase - (PcdOvmfSecPageTablesBase + 0x6000).
+;
+; This range should match with PcdOvmfSecPageTablesSize which is
+; declared in the FDF files.
+;
+; At the end of PEI, the pages tables will be rebuilt into a
+; more permanent location by DxeIpl.
+;
+%macro ClearOvmfPageTables 0
+    mov     ecx, 6 * 0x1000 / 4
+    xor     eax, eax
+.clearPageTablesMemoryLoop:
+    mov     dword[ecx * 4 + PT_ADDR (0) - 4], eax
+    loop    .clearPageTablesMemoryLoop
+%endmacro
+
 ;
 ; Modified:  EAX, EBX, ECX, EDX
 ;
@@ -69,22 +87,7 @@ SetCr3ForPageTables64:
     OneTimeCall   GetSevCBitMaskAbove31
 
 ClearOvmfPageTables:
-    ;
-    ; For OVMF, build some initial page tables at
-    ; PcdOvmfSecPageTablesBase - (PcdOvmfSecPageTablesBase + 0x6000).
-    ;
-    ; This range should match with PcdOvmfSecPageTablesSize which is
-    ; declared in the FDF files.
-    ;
-    ; At the end of PEI, the pages tables will be rebuilt into a
-    ; more permanent location by DxeIpl.
-    ;
-
-    mov     ecx, 6 * 0x1000 / 4
-    xor     eax, eax
-clearPageTablesMemoryLoop:
-    mov     dword[ecx * 4 + PT_ADDR (0) - 4], eax
-    loop    clearPageTablesMemoryLoop
+    ClearOvmfPageTables
 
     ;
     ; Top level Page Directory Pointers (1 * 512GB entry)
-- 
2.43.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115803): https://edk2.groups.io/g/devel/message/115803
Mute This Topic: https://groups.io/mt/104506789/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 02/10] OvmfPkg/ResetVector: add ClearOvmfPageTables macro

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> Move code to clear the page tables to a nasm macro.
> No functional change.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 35 ++++++++++++-----------
>  1 file changed, 19 insertions(+), 16 deletions(-)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 6fec6f2beeea..378ba2feeb4f 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -45,6 +45,24 @@ BITS    32
>  %define TDX_BSP         1
>  %define TDX_AP          2
>  
> +;
> +; For OVMF, build some initial page tables at
> +; PcdOvmfSecPageTablesBase - (PcdOvmfSecPageTablesBase + 0x6000).
> +;
> +; This range should match with PcdOvmfSecPageTablesSize which is
> +; declared in the FDF files.
> +;
> +; At the end of PEI, the pages tables will be rebuilt into a
> +; more permanent location by DxeIpl.
> +;
> +%macro ClearOvmfPageTables 0
> +    mov     ecx, 6 * 0x1000 / 4
> +    xor     eax, eax
> +.clearPageTablesMemoryLoop:
> +    mov     dword[ecx * 4 + PT_ADDR (0) - 4], eax
> +    loop    .clearPageTablesMemoryLoop
> +%endmacro
> +
>  ;
>  ; Modified:  EAX, EBX, ECX, EDX
>  ;

Ah, this made me read up on local labels:

https://www.nasm.us/xdoc/2.16.01/html/nasmdoc3.html#section-3.9

Should we rather call the label

  ..@clearPageTablesMemoryLoop

?

According to the documentation, that seems to be the safest / most
robust label type to be used inside macros.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

> @@ -69,22 +87,7 @@ SetCr3ForPageTables64:
>      OneTimeCall   GetSevCBitMaskAbove31
>  
>  ClearOvmfPageTables:
> -    ;
> -    ; For OVMF, build some initial page tables at
> -    ; PcdOvmfSecPageTablesBase - (PcdOvmfSecPageTablesBase + 0x6000).
> -    ;
> -    ; This range should match with PcdOvmfSecPageTablesSize which is
> -    ; declared in the FDF files.
> -    ;
> -    ; At the end of PEI, the pages tables will be rebuilt into a
> -    ; more permanent location by DxeIpl.
> -    ;
> -
> -    mov     ecx, 6 * 0x1000 / 4
> -    xor     eax, eax
> -clearPageTablesMemoryLoop:
> -    mov     dword[ecx * 4 + PT_ADDR (0) - 4], eax
> -    loop    clearPageTablesMemoryLoop
> +    ClearOvmfPageTables
>  
>      ;
>      ; Top level Page Directory Pointers (1 * 512GB entry)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116080): https://edk2.groups.io/g/devel/message/116080
Mute This Topic: https://groups.io/mt/104506789/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 02/10] OvmfPkg/ResetVector: add ClearOvmfPageTables macro

[Gerd Hoffmann]

On Wed, Feb 28, 2024 at 05:09:32AM +0100, Laszlo Ersek wrote:
> On 2/22/24 12:54, Gerd Hoffmann wrote:
> > Move code to clear the page tables to a nasm macro.
> > No functional change.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 35 ++++++++++++-----------
> >  1 file changed, 19 insertions(+), 16 deletions(-)
> > 
> > diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> > index 6fec6f2beeea..378ba2feeb4f 100644
> > --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> > +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> > @@ -45,6 +45,24 @@ BITS    32
> >  %define TDX_BSP         1
> >  %define TDX_AP          2
> >  
> > +;
> > +; For OVMF, build some initial page tables at
> > +; PcdOvmfSecPageTablesBase - (PcdOvmfSecPageTablesBase + 0x6000).
> > +;
> > +; This range should match with PcdOvmfSecPageTablesSize which is
> > +; declared in the FDF files.
> > +;
> > +; At the end of PEI, the pages tables will be rebuilt into a
> > +; more permanent location by DxeIpl.
> > +;
> > +%macro ClearOvmfPageTables 0
> > +    mov     ecx, 6 * 0x1000 / 4
> > +    xor     eax, eax
> > +.clearPageTablesMemoryLoop:
> > +    mov     dword[ecx * 4 + PT_ADDR (0) - 4], eax
> > +    loop    .clearPageTablesMemoryLoop
> > +%endmacro
> > +
> >  ;
> >  ; Modified:  EAX, EBX, ECX, EDX
> >  ;
> 
> Ah, this made me read up on local labels:
> 
> https://www.nasm.us/xdoc/2.16.01/html/nasmdoc3.html#section-3.9
> 
> Should we rather call the label
> 
>   ..@clearPageTablesMemoryLoop
> 
> ?

I've tried that and something (which I don't remember) didn't work as
expected.  Given that each branch which uses that macro will have a jump
label anyway (so the local label is expanded to something like
TdxBspInit.clearPageTablesMemoryLoop) I've figured this is good enough

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116094): https://edk2.groups.io/g/devel/message/116094
Mute This Topic: https://groups.io/mt/104506789/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 02/10] OvmfPkg/ResetVector: add ClearOvmfPageTables macro

[Laszlo Ersek]

On 2/28/24 09:22, Gerd Hoffmann wrote:
> On Wed, Feb 28, 2024 at 05:09:32AM +0100, Laszlo Ersek wrote:
>> On 2/22/24 12:54, Gerd Hoffmann wrote:
>>> Move code to clear the page tables to a nasm macro.
>>> No functional change.
>>>
>>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>>> ---
>>>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 35 ++++++++++++-----------
>>>  1 file changed, 19 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>>> index 6fec6f2beeea..378ba2feeb4f 100644
>>> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>>> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>>> @@ -45,6 +45,24 @@ BITS    32
>>>  %define TDX_BSP         1
>>>  %define TDX_AP          2
>>>  
>>> +;
>>> +; For OVMF, build some initial page tables at
>>> +; PcdOvmfSecPageTablesBase - (PcdOvmfSecPageTablesBase + 0x6000).
>>> +;
>>> +; This range should match with PcdOvmfSecPageTablesSize which is
>>> +; declared in the FDF files.
>>> +;
>>> +; At the end of PEI, the pages tables will be rebuilt into a
>>> +; more permanent location by DxeIpl.
>>> +;
>>> +%macro ClearOvmfPageTables 0
>>> +    mov     ecx, 6 * 0x1000 / 4
>>> +    xor     eax, eax
>>> +.clearPageTablesMemoryLoop:
>>> +    mov     dword[ecx * 4 + PT_ADDR (0) - 4], eax
>>> +    loop    .clearPageTablesMemoryLoop
>>> +%endmacro
>>> +
>>>  ;
>>>  ; Modified:  EAX, EBX, ECX, EDX
>>>  ;
>>
>> Ah, this made me read up on local labels:
>>
>> https://www.nasm.us/xdoc/2.16.01/html/nasmdoc3.html#section-3.9
>>
>> Should we rather call the label
>>
>>   ..@clearPageTablesMemoryLoop
>>
>> ?
> 
> I've tried that and something (which I don't remember) didn't work as
> expected.  Given that each branch which uses that macro will have a jump
> label anyway (so the local label is expanded to something like
> TdxBspInit.clearPageTablesMemoryLoop) I've figured this is good enough

Sure, if you've tried it already, then the current method is fine!

Thanks!
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116135): https://edk2.groups.io/g/devel/message/116135
Mute This Topic: https://groups.io/mt/104506789/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 03/10] OvmfPkg/ResetVector: add CreatePageTables4Level macro

[Gerd Hoffmann]

Move code to create 4-level page tables to a nasm macro.
No functional change.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/ResetVector/Ia32/PageTables64.asm | 70 +++++++++++++----------
 1 file changed, 39 insertions(+), 31 deletions(-)

diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index 378ba2feeb4f..14cc2c33aa3d 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -63,6 +63,44 @@ BITS    32
     loop    .clearPageTablesMemoryLoop
 %endmacro
 
+;
+; Create page tables for 4-level paging
+;
+; Argument: upper 32 bits of the page table entries
+;
+%macro CreatePageTables4Level 1
+    ;
+    ; Top level Page Directory Pointers (1 * 512GB entry)
+    ;
+    mov     dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDE_DIRECTORY_ATTR
+    mov     dword[PT_ADDR (4)], %1
+
+    ;
+    ; Next level Page Directory Pointers (4 * 1GB entries => 4GB)
+    ;
+    mov     dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + PAGE_PDE_DIRECTORY_ATTR
+    mov     dword[PT_ADDR (0x1004)], %1
+    mov     dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + PAGE_PDE_DIRECTORY_ATTR
+    mov     dword[PT_ADDR (0x100C)], %1
+    mov     dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + PAGE_PDE_DIRECTORY_ATTR
+    mov     dword[PT_ADDR (0x1014)], %1
+    mov     dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + PAGE_PDE_DIRECTORY_ATTR
+    mov     dword[PT_ADDR (0x101C)], %1
+
+    ;
+    ; Page Table Entries (2048 * 2MB entries => 4GB)
+    ;
+    mov     ecx, 0x800
+.pageTableEntriesLoop4Level:
+    mov     eax, ecx
+    dec     eax
+    shl     eax, 21
+    add     eax, PAGE_PDE_LARGEPAGE_ATTR
+    mov     dword[ecx * 8 + PT_ADDR (0x2000 - 8)], eax
+    mov     dword[(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], %1
+    loop    .pageTableEntriesLoop4Level
+%endmacro
+
 ;
 ; Modified:  EAX, EBX, ECX, EDX
 ;
@@ -88,37 +126,7 @@ SetCr3ForPageTables64:
 
 ClearOvmfPageTables:
     ClearOvmfPageTables
-
-    ;
-    ; Top level Page Directory Pointers (1 * 512GB entry)
-    ;
-    mov     dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDE_DIRECTORY_ATTR
-    mov     dword[PT_ADDR (4)], edx
-
-    ;
-    ; Next level Page Directory Pointers (4 * 1GB entries => 4GB)
-    ;
-    mov     dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + PAGE_PDE_DIRECTORY_ATTR
-    mov     dword[PT_ADDR (0x1004)], edx
-    mov     dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + PAGE_PDE_DIRECTORY_ATTR
-    mov     dword[PT_ADDR (0x100C)], edx
-    mov     dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + PAGE_PDE_DIRECTORY_ATTR
-    mov     dword[PT_ADDR (0x1014)], edx
-    mov     dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + PAGE_PDE_DIRECTORY_ATTR
-    mov     dword[PT_ADDR (0x101C)], edx
-
-    ;
-    ; Page Table Entries (2048 * 2MB entries => 4GB)
-    ;
-    mov     ecx, 0x800
-pageTableEntriesLoop:
-    mov     eax, ecx
-    dec     eax
-    shl     eax, 21
-    add     eax, PAGE_PDE_LARGEPAGE_ATTR
-    mov     [ecx * 8 + PT_ADDR (0x2000 - 8)], eax
-    mov     [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
-    loop    pageTableEntriesLoop
+    CreatePageTables4Level edx
 
     ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
     OneTimeCall   SevClearPageEncMaskForGhcbPage
-- 
2.43.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115805): https://edk2.groups.io/g/devel/message/115805
Mute This Topic: https://groups.io/mt/104506791/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 03/10] OvmfPkg/ResetVector: add CreatePageTables4Level macro

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> Move code to create 4-level page tables to a nasm macro.
> No functional change.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 70 +++++++++++++----------
>  1 file changed, 39 insertions(+), 31 deletions(-)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 378ba2feeb4f..14cc2c33aa3d 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -63,6 +63,44 @@ BITS    32
>      loop    .clearPageTablesMemoryLoop
>  %endmacro
>  
> +;
> +; Create page tables for 4-level paging
> +;
> +; Argument: upper 32 bits of the page table entries
> +;
> +%macro CreatePageTables4Level 1
> +    ;
> +    ; Top level Page Directory Pointers (1 * 512GB entry)
> +    ;
> +    mov     dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDE_DIRECTORY_ATTR
> +    mov     dword[PT_ADDR (4)], %1
> +
> +    ;
> +    ; Next level Page Directory Pointers (4 * 1GB entries => 4GB)
> +    ;
> +    mov     dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + PAGE_PDE_DIRECTORY_ATTR
> +    mov     dword[PT_ADDR (0x1004)], %1
> +    mov     dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + PAGE_PDE_DIRECTORY_ATTR
> +    mov     dword[PT_ADDR (0x100C)], %1
> +    mov     dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + PAGE_PDE_DIRECTORY_ATTR
> +    mov     dword[PT_ADDR (0x1014)], %1
> +    mov     dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + PAGE_PDE_DIRECTORY_ATTR
> +    mov     dword[PT_ADDR (0x101C)], %1
> +
> +    ;
> +    ; Page Table Entries (2048 * 2MB entries => 4GB)
> +    ;
> +    mov     ecx, 0x800
> +.pageTableEntriesLoop4Level:
> +    mov     eax, ecx
> +    dec     eax
> +    shl     eax, 21
> +    add     eax, PAGE_PDE_LARGEPAGE_ATTR
> +    mov     dword[ecx * 8 + PT_ADDR (0x2000 - 8)], eax
> +    mov     dword[(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], %1
> +    loop    .pageTableEntriesLoop4Level
> +%endmacro
> +
>  ;
>  ; Modified:  EAX, EBX, ECX, EDX
>  ;
> @@ -88,37 +126,7 @@ SetCr3ForPageTables64:
>  
>  ClearOvmfPageTables:
>      ClearOvmfPageTables
> -
> -    ;
> -    ; Top level Page Directory Pointers (1 * 512GB entry)
> -    ;
> -    mov     dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDE_DIRECTORY_ATTR
> -    mov     dword[PT_ADDR (4)], edx
> -
> -    ;
> -    ; Next level Page Directory Pointers (4 * 1GB entries => 4GB)
> -    ;
> -    mov     dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + PAGE_PDE_DIRECTORY_ATTR
> -    mov     dword[PT_ADDR (0x1004)], edx
> -    mov     dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + PAGE_PDE_DIRECTORY_ATTR
> -    mov     dword[PT_ADDR (0x100C)], edx
> -    mov     dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + PAGE_PDE_DIRECTORY_ATTR
> -    mov     dword[PT_ADDR (0x1014)], edx
> -    mov     dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + PAGE_PDE_DIRECTORY_ATTR
> -    mov     dword[PT_ADDR (0x101C)], edx
> -
> -    ;
> -    ; Page Table Entries (2048 * 2MB entries => 4GB)
> -    ;
> -    mov     ecx, 0x800
> -pageTableEntriesLoop:
> -    mov     eax, ecx
> -    dec     eax
> -    shl     eax, 21
> -    add     eax, PAGE_PDE_LARGEPAGE_ATTR
> -    mov     [ecx * 8 + PT_ADDR (0x2000 - 8)], eax
> -    mov     [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
> -    loop    pageTableEntriesLoop
> +    CreatePageTables4Level edx
>  
>      ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
>      OneTimeCall   SevClearPageEncMaskForGhcbPage

Nice.

"--color-moved=zebra" is really useful for viewing this patch.

Same comment on the ".pageTableEntriesLoop4Level" label name as under
patch#2: can we use "..@pageTableEntriesLoop4Level"?

Reviewed-by: Laszlo Ersek <lersek@redhat.com>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116081): https://edk2.groups.io/g/devel/message/116081
Mute This Topic: https://groups.io/mt/104506791/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 04/10] OvmfPkg/ResetVector: split TDX BSP workflow

[Gerd Hoffmann]

Create a separate control flow for TDX BSP.

TdxPostBuildPageTables will now only be called when running in TDX
mode, so the TDX check in that function is not needed any more.

No functional change.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/ResetVector/Ia32/IntelTdx.asm     |  4 ----
 OvmfPkg/ResetVector/Ia32/PageTables64.asm | 15 ++++++++++-----
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
index 06794baef81d..c6b86019dfb9 100644
--- a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
+++ b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
@@ -197,11 +197,7 @@ NotTdx:
 ; Set byte[TDX_WORK_AREA_PGTBL_READY] to 1
 ;
 TdxPostBuildPageTables:
-    cmp     byte[WORK_AREA_GUEST_TYPE], VM_GUEST_TDX
-    jne     ExitTdxPostBuildPageTables
     mov     byte[TDX_WORK_AREA_PGTBL_READY], 1
-
-ExitTdxPostBuildPageTables:
     OneTimeCallRet TdxPostBuildPageTables
 
 ;
diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index 14cc2c33aa3d..166e80293c89 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -112,7 +112,7 @@ SetCr3ForPageTables64:
     ; is set.
     OneTimeCall   CheckTdxFeaturesBeforeBuildPagetables
     cmp       eax, TDX_BSP
-    je        ClearOvmfPageTables
+    je        TdxBspInit
     cmp       eax, TDX_AP
     je        SetCr3
 
@@ -124,16 +124,21 @@ SetCr3ForPageTables64:
     ; the page table build below.
     OneTimeCall   GetSevCBitMaskAbove31
 
-ClearOvmfPageTables:
     ClearOvmfPageTables
     CreatePageTables4Level edx
 
     ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
     OneTimeCall   SevClearPageEncMaskForGhcbPage
+    jmp SetCr3
 
-    ; TDX will do some PostBuildPages task, such as setting
-    ; byte[TDX_WORK_AREA_PGTBL_READY].
-    OneTimeCall   TdxPostBuildPageTables
+TdxBspInit:
+    ;
+    ; TDX BSP workflow
+    ;
+    ClearOvmfPageTables
+    CreatePageTables4Level 0
+    OneTimeCall TdxPostBuildPageTables
+    jmp SetCr3
 
 SetCr3:
     ;
-- 
2.43.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115809): https://edk2.groups.io/g/devel/message/115809
Mute This Topic: https://groups.io/mt/104506797/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 04/10] OvmfPkg/ResetVector: split TDX BSP workflow

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> Create a separate control flow for TDX BSP.
> 
> TdxPostBuildPageTables will now only be called when running in TDX
> mode, so the TDX check in that function is not needed any more.
> 
> No functional change.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/ResetVector/Ia32/IntelTdx.asm     |  4 ----
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 15 ++++++++++-----
>  2 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
> index 06794baef81d..c6b86019dfb9 100644
> --- a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
> +++ b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
> @@ -197,11 +197,7 @@ NotTdx:
>  ; Set byte[TDX_WORK_AREA_PGTBL_READY] to 1
>  ;
>  TdxPostBuildPageTables:
> -    cmp     byte[WORK_AREA_GUEST_TYPE], VM_GUEST_TDX
> -    jne     ExitTdxPostBuildPageTables
>      mov     byte[TDX_WORK_AREA_PGTBL_READY], 1
> -
> -ExitTdxPostBuildPageTables:
>      OneTimeCallRet TdxPostBuildPageTables
>  
>  ;
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 14cc2c33aa3d..166e80293c89 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -112,7 +112,7 @@ SetCr3ForPageTables64:
>      ; is set.
>      OneTimeCall   CheckTdxFeaturesBeforeBuildPagetables
>      cmp       eax, TDX_BSP
> -    je        ClearOvmfPageTables
> +    je        TdxBspInit
>      cmp       eax, TDX_AP
>      je        SetCr3
>  
> @@ -124,16 +124,21 @@ SetCr3ForPageTables64:
>      ; the page table build below.
>      OneTimeCall   GetSevCBitMaskAbove31
>  
> -ClearOvmfPageTables:
>      ClearOvmfPageTables
>      CreatePageTables4Level edx
>  
>      ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
>      OneTimeCall   SevClearPageEncMaskForGhcbPage
> +    jmp SetCr3
>  
> -    ; TDX will do some PostBuildPages task, such as setting
> -    ; byte[TDX_WORK_AREA_PGTBL_READY].
> -    OneTimeCall   TdxPostBuildPageTables
> +TdxBspInit:
> +    ;
> +    ; TDX BSP workflow
> +    ;
> +    ClearOvmfPageTables
> +    CreatePageTables4Level 0
> +    OneTimeCall TdxPostBuildPageTables
> +    jmp SetCr3
>  
>  SetCr3:
>      ;

Reviewed-by: Laszlo Ersek <lersek@redhat.com>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116082): https://edk2.groups.io/g/devel/message/116082
Mute This Topic: https://groups.io/mt/104506797/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 05/10] OvmfPkg/ResetVector: split SEV and non-CoCo workflows

[Gerd Hoffmann]

Use separate control flows for SEV and non-CoCo cases.

SevClearPageEncMaskForGhcbPage and GetSevCBitMaskAbove31 will now only
be called when running in SEV mode, so the SEV check in these functions
is not needed any more.

No functional change.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/ResetVector/Ia32/AmdSev.asm       | 16 ++--------------
 OvmfPkg/ResetVector/Ia32/PageTables64.asm | 17 ++++++++++++++---
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
index 043c88a7abbe..ed94f1dc668f 100644
--- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
+++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
@@ -152,12 +152,8 @@ SevEsUnexpectedRespTerminate:
 
 %ifdef ARCH_X64
 
-; If SEV-ES is enabled then initialize and make the GHCB page shared
+; initialize and make the GHCB page shared
 SevClearPageEncMaskForGhcbPage:
-    ; Check if SEV is enabled
-    cmp       byte[WORK_AREA_GUEST_TYPE], 1
-    jnz       SevClearPageEncMaskForGhcbPageExit
-
     ; Check if SEV-ES is enabled
     mov       ecx, 1
     bt        [SEV_ES_WORK_AREA_STATUS_MSR], ecx
@@ -195,20 +191,12 @@ pageTableEntries4kLoop:
 SevClearPageEncMaskForGhcbPageExit:
     OneTimeCallRet SevClearPageEncMaskForGhcbPage
 
-; Check if SEV is enabled, and get the C-bit mask above 31.
+; Get the C-bit mask above 31.
 ; Modified: EDX
 ;
 ; The value is returned in the EDX
 GetSevCBitMaskAbove31:
-    xor       edx, edx
-
-    ; Check if SEV is enabled
-    cmp       byte[WORK_AREA_GUEST_TYPE], 1
-    jnz       GetSevCBitMaskAbove31Exit
-
     mov       edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
-
-GetSevCBitMaskAbove31Exit:
     OneTimeCallRet GetSevCBitMaskAbove31
 
 %endif
diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index 166e80293c89..84a7b4efc019 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -118,15 +118,26 @@ SetCr3ForPageTables64:
 
     ; Check whether the SEV is active and populate the SevEsWorkArea
     OneTimeCall   CheckSevFeatures
+    cmp       byte[WORK_AREA_GUEST_TYPE], 1
+    jz        SevInit
 
+    ;
+    ; normal (non-CoCo) workflow
+    ;
+    ClearOvmfPageTables
+    CreatePageTables4Level 0
+    jmp SetCr3
+
+SevInit:
+    ;
+    ; SEV workflow
+    ;
+    ClearOvmfPageTables
     ; If SEV is enabled, the C-bit position is always above 31.
     ; The mask will be saved in the EDX and applied during the
     ; the page table build below.
     OneTimeCall   GetSevCBitMaskAbove31
-
-    ClearOvmfPageTables
     CreatePageTables4Level edx
-
     ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
     OneTimeCall   SevClearPageEncMaskForGhcbPage
     jmp SetCr3
-- 
2.43.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115807): https://edk2.groups.io/g/devel/message/115807
Mute This Topic: https://groups.io/mt/104506794/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 05/10] OvmfPkg/ResetVector: split SEV and non-CoCo workflows

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> Use separate control flows for SEV and non-CoCo cases.
> 
> SevClearPageEncMaskForGhcbPage and GetSevCBitMaskAbove31 will now only
> be called when running in SEV mode, so the SEV check in these functions
> is not needed any more.
> 
> No functional change.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/ResetVector/Ia32/AmdSev.asm       | 16 ++--------------
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 17 ++++++++++++++---
>  2 files changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> index 043c88a7abbe..ed94f1dc668f 100644
> --- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> +++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> @@ -152,12 +152,8 @@ SevEsUnexpectedRespTerminate:
>  
>  %ifdef ARCH_X64
>  
> -; If SEV-ES is enabled then initialize and make the GHCB page shared
> +; initialize and make the GHCB page shared

(1) This comment update is unjustified, I suggest reverting it.

(The SEV check is indeed superfluous below, but you -- correctly -- keep
the SEV-ES check, and the comment here is about SEV-ES, not SEV. Because
the check stays, the comment should stay too.)

>  SevClearPageEncMaskForGhcbPage:
> -    ; Check if SEV is enabled
> -    cmp       byte[WORK_AREA_GUEST_TYPE], 1
> -    jnz       SevClearPageEncMaskForGhcbPageExit
> -
>      ; Check if SEV-ES is enabled
>      mov       ecx, 1
>      bt        [SEV_ES_WORK_AREA_STATUS_MSR], ecx
> @@ -195,20 +191,12 @@ pageTableEntries4kLoop:
>  SevClearPageEncMaskForGhcbPageExit:
>      OneTimeCallRet SevClearPageEncMaskForGhcbPage
>  
> -; Check if SEV is enabled, and get the C-bit mask above 31.
> +; Get the C-bit mask above 31.
>  ; Modified: EDX
>  ;
>  ; The value is returned in the EDX
>  GetSevCBitMaskAbove31:
> -    xor       edx, edx
> -
> -    ; Check if SEV is enabled
> -    cmp       byte[WORK_AREA_GUEST_TYPE], 1
> -    jnz       GetSevCBitMaskAbove31Exit
> -
>      mov       edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
> -
> -GetSevCBitMaskAbove31Exit:
>      OneTimeCallRet GetSevCBitMaskAbove31
>  
>  %endif
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 166e80293c89..84a7b4efc019 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -118,15 +118,26 @@ SetCr3ForPageTables64:
>  
>      ; Check whether the SEV is active and populate the SevEsWorkArea
>      OneTimeCall   CheckSevFeatures
> +    cmp       byte[WORK_AREA_GUEST_TYPE], 1
> +    jz        SevInit
>  
> +    ;
> +    ; normal (non-CoCo) workflow
> +    ;
> +    ClearOvmfPageTables
> +    CreatePageTables4Level 0
> +    jmp SetCr3
> +
> +SevInit:
> +    ;
> +    ; SEV workflow
> +    ;
> +    ClearOvmfPageTables
>      ; If SEV is enabled, the C-bit position is always above 31.
>      ; The mask will be saved in the EDX and applied during the
>      ; the page table build below.
>      OneTimeCall   GetSevCBitMaskAbove31
> -
> -    ClearOvmfPageTables
>      CreatePageTables4Level edx
> -
>      ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
>      OneTimeCall   SevClearPageEncMaskForGhcbPage
>      jmp SetCr3

Nice.

The patch also sneakily reorders ClearOvmfPageTables against
GetSevCBitMaskAbove31 -- but that's an improvement: this way we no
longer depend on ClearOvmfPageTables not modifying EDX; instead, EDX
directly passes from GetSevCBitMaskAbove31 to CreatePageTables4Level.

With (1) undone:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116084): https://edk2.groups.io/g/devel/message/116084
Mute This Topic: https://groups.io/mt/104506794/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 06/10] OvmfPkg/ResetVector: add 5-level paging support

[Gerd Hoffmann]

Add macros to check for 5-level paging and gigabyte page support.
Enable 5-level paging for the non-confidential-computing case.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/ResetVector/ResetVector.inf       |   1 +
 OvmfPkg/ResetVector/Ia32/PageTables64.asm | 105 ++++++++++++++++++++++
 OvmfPkg/ResetVector/ResetVector.nasmb     |   1 +
 3 files changed, 107 insertions(+)

diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/ResetVector.inf
index a4154ca90c28..65f71b05a02e 100644
--- a/OvmfPkg/ResetVector/ResetVector.inf
+++ b/OvmfPkg/ResetVector/ResetVector.inf
@@ -64,3 +64,4 @@ [FixedPcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableSize
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable
diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index 84a7b4efc019..825589f31193 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -101,6 +101,97 @@ BITS    32
     loop    .pageTableEntriesLoop4Level
 %endmacro
 
+;
+; Check whenever 5-level paging can ca used
+;
+; Argument: jump label for 4-level paging
+;
+%macro Check5LevelPaging 1
+    ; check for cpuid leaf 0x07
+    mov     eax, 0x00
+    cpuid
+    cmp     eax, 0x07
+    jb      %1
+
+    ; check for la57 (aka 5-level paging)
+    mov     eax, 0x07
+    mov     ecx, 0x00
+    cpuid
+    bt      ecx, 16
+    jnc     %1
+
+    ; check for cpuid leaf 0x80000001
+    mov     eax, 0x80000000
+    cpuid
+    cmp     eax, 0x80000001
+    jb      %1
+
+    ; check for 1g pages
+    mov     eax, 0x80000001
+    cpuid
+    bt      edx, 26
+    jnc     %1
+%endmacro
+
+;
+; Create page tables for 5-level paging with gigabyte pages
+;
+; Argument: upper 32 bits of the page table entries
+;
+; We have 6 pages available for the early page tables,
+; we use four of them:
+;    PT_ADDR(0)      - level 5 directory
+;    PT_ADDR(0x1000) - level 4 directory
+;    PT_ADDR(0x2000) - level 2 directory (0 -> 1GB)
+;    PT_ADDR(0x3000) - level 3 directory
+;
+; The level 2 directory for the first gigabyte has the same
+; physical address in both 4-level and 5-level paging mode,
+; SevClearPageEncMaskForGhcbPage depends on this.
+;
+; The 1 GB -> 4 GB range is mapped using 1G pages in the
+; level 3 directory.
+;
+%macro CreatePageTables5Level 1
+    ; level 5
+    mov     dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDE_DIRECTORY_ATTR
+    mov     dword[PT_ADDR (4)], %1
+
+    ; level 4
+    mov     dword[PT_ADDR (0x1000)], PT_ADDR (0x3000) + PAGE_PDE_DIRECTORY_ATTR
+    mov     dword[PT_ADDR (0x1004)], %1
+
+    ; level 3 (1x -> level 2, 3x 1GB)
+    mov     dword[PT_ADDR (0x3000)], PT_ADDR (0x2000) + PAGE_PDE_DIRECTORY_ATTR
+    mov     dword[PT_ADDR (0x3004)], %1
+    mov     dword[PT_ADDR (0x3008)], (1 << 30) + PAGE_PDE_LARGEPAGE_ATTR
+    mov     dword[PT_ADDR (0x300c)], %1
+    mov     dword[PT_ADDR (0x3010)], (2 << 30) + PAGE_PDE_LARGEPAGE_ATTR
+    mov     dword[PT_ADDR (0x3014)], %1
+    mov     dword[PT_ADDR (0x3018)], (3 << 30) + PAGE_PDE_LARGEPAGE_ATTR
+    mov     dword[PT_ADDR (0x301c)], %1
+
+    ;
+    ; level 2 (512 * 2MB entries => 1GB)
+    ;
+    mov     ecx, 0x200
+.pageTableEntriesLoop5Level:
+    mov     eax, ecx
+    dec     eax
+    shl     eax, 21
+    add     eax, PAGE_PDE_LARGEPAGE_ATTR
+    mov     dword[ecx * 8 + PT_ADDR (0x2000 - 8)], eax
+    mov     dword[(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], %1
+    loop    .pageTableEntriesLoop5Level
+%endmacro
+
+%macro Enable5LevelPaging 0
+    ; set la57 bit in cr4
+    mov     eax, cr4
+    bts     eax, 12
+    mov     cr4, eax
+%endmacro
+
 ;
 ; Modified:  EAX, EBX, ECX, EDX
 ;
@@ -125,6 +216,12 @@ SetCr3ForPageTables64:
     ; normal (non-CoCo) workflow
     ;
     ClearOvmfPageTables
+%if PG_5_LEVEL
+    Check5LevelPaging Paging4Level
+    CreatePageTables5Level 0
+    jmp SetCr3La57
+Paging4Level:
+%endif
     CreatePageTables4Level 0
     jmp SetCr3
 
@@ -151,6 +248,14 @@ TdxBspInit:
     OneTimeCall TdxPostBuildPageTables
     jmp SetCr3
 
+    ;
+    ; common workflow
+    ;
+%if PG_5_LEVEL
+SetCr3La57:
+    Enable5LevelPaging
+%endif
+
 SetCr3:
     ;
     ; Set CR3 now that the paging structures are available
diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
index 366a70fb9992..2bd80149e58b 100644
--- a/OvmfPkg/ResetVector/ResetVector.nasmb
+++ b/OvmfPkg/ResetVector/ResetVector.nasmb
@@ -53,6 +53,7 @@
 
 %define WORK_AREA_GUEST_TYPE          (FixedPcdGet32 (PcdOvmfWorkAreaBase))
 %define PT_ADDR(Offset)               (FixedPcdGet32 (PcdOvmfSecPageTablesBase) + (Offset))
+%define PG_5_LEVEL                    (FixedPcdGetBool (PcdUse5LevelPageTable))
 
 %define GHCB_PT_ADDR                  (FixedPcdGet32 (PcdOvmfSecGhcbPageTableBase))
 %define GHCB_BASE                     (FixedPcdGet32 (PcdOvmfSecGhcbBase))
-- 
2.43.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115806): https://edk2.groups.io/g/devel/message/115806
Mute This Topic: https://groups.io/mt/104506793/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 06/10] OvmfPkg/ResetVector: add 5-level paging support

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> Add macros to check for 5-level paging and gigabyte page support.
> Enable 5-level paging for the non-confidential-computing case.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/ResetVector/ResetVector.inf       |   1 +
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 105 ++++++++++++++++++++++
>  OvmfPkg/ResetVector/ResetVector.nasmb     |   1 +
>  3 files changed, 107 insertions(+)
> 
> diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/ResetVector.inf
> index a4154ca90c28..65f71b05a02e 100644
> --- a/OvmfPkg/ResetVector/ResetVector.inf
> +++ b/OvmfPkg/ResetVector/ResetVector.inf
> @@ -64,3 +64,4 @@ [FixedPcd]
>    gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableSize
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 84a7b4efc019..825589f31193 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -101,6 +101,97 @@ BITS    32
>      loop    .pageTableEntriesLoop4Level
>  %endmacro
>  
> +;
> +; Check whenever 5-level paging can ca used

(1) typo in comment

> +;
> +; Argument: jump label for 4-level paging
> +;
> +%macro Check5LevelPaging 1
> +    ; check for cpuid leaf 0x07
> +    mov     eax, 0x00
> +    cpuid
> +    cmp     eax, 0x07
> +    jb      %1
> +
> +    ; check for la57 (aka 5-level paging)
> +    mov     eax, 0x07
> +    mov     ecx, 0x00
> +    cpuid
> +    bt      ecx, 16
> +    jnc     %1
> +
> +    ; check for cpuid leaf 0x80000001
> +    mov     eax, 0x80000000
> +    cpuid
> +    cmp     eax, 0x80000001
> +    jb      %1
> +
> +    ; check for 1g pages
> +    mov     eax, 0x80000001
> +    cpuid
> +    bt      edx, 26
> +    jnc     %1
> +%endmacro
> +
> +;
> +; Create page tables for 5-level paging with gigabyte pages
> +;
> +; Argument: upper 32 bits of the page table entries
> +;
> +; We have 6 pages available for the early page tables,
> +; we use four of them:
> +;    PT_ADDR(0)      - level 5 directory
> +;    PT_ADDR(0x1000) - level 4 directory
> +;    PT_ADDR(0x2000) - level 2 directory (0 -> 1GB)
> +;    PT_ADDR(0x3000) - level 3 directory
> +;
> +; The level 2 directory for the first gigabyte has the same
> +; physical address in both 4-level and 5-level paging mode,
> +; SevClearPageEncMaskForGhcbPage depends on this.
> +;
> +; The 1 GB -> 4 GB range is mapped using 1G pages in the
> +; level 3 directory.
> +;
> +%macro CreatePageTables5Level 1
> +    ; level 5
> +    mov     dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDE_DIRECTORY_ATTR
> +    mov     dword[PT_ADDR (4)], %1
> +
> +    ; level 4
> +    mov     dword[PT_ADDR (0x1000)], PT_ADDR (0x3000) + PAGE_PDE_DIRECTORY_ATTR
> +    mov     dword[PT_ADDR (0x1004)], %1
> +
> +    ; level 3 (1x -> level 2, 3x 1GB)
> +    mov     dword[PT_ADDR (0x3000)], PT_ADDR (0x2000) + PAGE_PDE_DIRECTORY_ATTR
> +    mov     dword[PT_ADDR (0x3004)], %1
> +    mov     dword[PT_ADDR (0x3008)], (1 << 30) + PAGE_PDE_LARGEPAGE_ATTR
> +    mov     dword[PT_ADDR (0x300c)], %1
> +    mov     dword[PT_ADDR (0x3010)], (2 << 30) + PAGE_PDE_LARGEPAGE_ATTR
> +    mov     dword[PT_ADDR (0x3014)], %1
> +    mov     dword[PT_ADDR (0x3018)], (3 << 30) + PAGE_PDE_LARGEPAGE_ATTR
> +    mov     dword[PT_ADDR (0x301c)], %1
> +
> +    ;
> +    ; level 2 (512 * 2MB entries => 1GB)
> +    ;
> +    mov     ecx, 0x200
> +.pageTableEntriesLoop5Level:

(2) suggest "..@pageTableEntriesLoop5Level" as label name

> +    mov     eax, ecx
> +    dec     eax
> +    shl     eax, 21
> +    add     eax, PAGE_PDE_LARGEPAGE_ATTR
> +    mov     dword[ecx * 8 + PT_ADDR (0x2000 - 8)], eax
> +    mov     dword[(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], %1
> +    loop    .pageTableEntriesLoop5Level
> +%endmacro
> +
> +%macro Enable5LevelPaging 0
> +    ; set la57 bit in cr4
> +    mov     eax, cr4
> +    bts     eax, 12
> +    mov     cr4, eax
> +%endmacro
> +
>  ;
>  ; Modified:  EAX, EBX, ECX, EDX
>  ;
> @@ -125,6 +216,12 @@ SetCr3ForPageTables64:
>      ; normal (non-CoCo) workflow
>      ;
>      ClearOvmfPageTables
> +%if PG_5_LEVEL
> +    Check5LevelPaging Paging4Level
> +    CreatePageTables5Level 0
> +    jmp SetCr3La57
> +Paging4Level:
> +%endif
>      CreatePageTables4Level 0
>      jmp SetCr3
>  
> @@ -151,6 +248,14 @@ TdxBspInit:
>      OneTimeCall TdxPostBuildPageTables
>      jmp SetCr3
>  
> +    ;
> +    ; common workflow
> +    ;
> +%if PG_5_LEVEL
> +SetCr3La57:
> +    Enable5LevelPaging
> +%endif
> +
>  SetCr3:
>      ;
>      ; Set CR3 now that the paging structures are available

(3) I don't like SetCr3La57.

It saves minimal code duplication, but makes the control flow much
harder to follow.

(3.1) From this patch, we have one (unconditional) jump to SetCr3La57. I
suggest simply inlining that, like this:

%if PG_5_LEVEL
    Check5LevelPaging Paging4Level
    CreatePageTables5Level 0
    Enable5LevelPaging                 <-------- inline
    jmp SetCr3                         <-------- here
Paging4Level:
%endif

This adds one extra line before the unconditional jump, but removes 8
lines at the destination, *plus* it removes a quirky jump / reused code
path.

(3.2) From patch #8 ("OvmfPkg/ResetVector: wire up 5-level paging for
TDX"), we have two jumps to SetCr3La57; one conditional and one
unconditional.

- The unconditional jump can be replaced with inlining (invoke the
Enable5LevelPaging macro, then jump to SetCr3).

- The conditional one can be reworked like this:

%if PG_5_LEVEL
    cmp       eax, TDX_AP_5_LEVEL
    jne       CheckForSev
    Enable5LevelPaging
    jmp       SetCr3
CheckForSev:
%endif

This way, the conditional jump is *local*; way easier to follow. The
"CheckForSev" label much resembles the "Paging4Level" label above.

The patch looks fine otherwise.

Thanks!
Laszlo


> diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
> index 366a70fb9992..2bd80149e58b 100644
> --- a/OvmfPkg/ResetVector/ResetVector.nasmb
> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb
> @@ -53,6 +53,7 @@
>  
>  %define WORK_AREA_GUEST_TYPE          (FixedPcdGet32 (PcdOvmfWorkAreaBase))
>  %define PT_ADDR(Offset)               (FixedPcdGet32 (PcdOvmfSecPageTablesBase) + (Offset))
> +%define PG_5_LEVEL                    (FixedPcdGetBool (PcdUse5LevelPageTable))
>  
>  %define GHCB_PT_ADDR                  (FixedPcdGet32 (PcdOvmfSecGhcbPageTableBase))
>  %define GHCB_BASE                     (FixedPcdGet32 (PcdOvmfSecGhcbBase))



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116085): https://edk2.groups.io/g/devel/message/116085
Mute This Topic: https://groups.io/mt/104506793/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 07/10] OvmfPkg/ResetVector: print post codes for 4/5 level paging

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/ResetVector/Ia32/PageTables64.asm | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index 825589f31193..d736db028277 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -69,6 +69,10 @@ BITS    32
 ; Argument: upper 32 bits of the page table entries
 ;
 %macro CreatePageTables4Level 1
+
+    ; indicate 4-level paging
+    debugShowPostCode 0x41
+
     ;
     ; Top level Page Directory Pointers (1 * 512GB entry)
     ;
@@ -153,6 +157,10 @@ BITS    32
 ; level 3 directory.
 ;
 %macro CreatePageTables5Level 1
+
+    ; indicate 5-level paging
+    debugShowPostCode 0x51
+
     ; level 5
     mov     dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDE_DIRECTORY_ATTR
     mov     dword[PT_ADDR (4)], %1
-- 
2.43.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115811): https://edk2.groups.io/g/devel/message/115811
Mute This Topic: https://groups.io/mt/104506801/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 07/10] OvmfPkg/ResetVector: print post codes for 4/5 level paging

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 825589f31193..d736db028277 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -69,6 +69,10 @@ BITS    32
>  ; Argument: upper 32 bits of the page table entries
>  ;
>  %macro CreatePageTables4Level 1
> +
> +    ; indicate 4-level paging
> +    debugShowPostCode 0x41
> +
>      ;
>      ; Top level Page Directory Pointers (1 * 512GB entry)
>      ;
> @@ -153,6 +157,10 @@ BITS    32
>  ; level 3 directory.
>  ;
>  %macro CreatePageTables5Level 1
> +
> +    ; indicate 5-level paging
> +    debugShowPostCode 0x51
> +
>      ; level 5
>      mov     dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDE_DIRECTORY_ATTR
>      mov     dword[PT_ADDR (4)], %1

Reviewed-by: Laszlo Ersek <lersek@redhat.com>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116086): https://edk2.groups.io/g/devel/message/116086
Mute This Topic: https://groups.io/mt/104506801/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 08/10] OvmfPkg/ResetVector: wire up 5-level paging for TDX

[Gerd Hoffmann]

BSP workflow is quite simliar to the non-coco case.

TDX_WORK_AREA_PGTBL_READY is used to record the paging mode:
  1 == 4-level paging
  2 == 5-level paging

APs will look at TDX_WORK_AREA_PGTBL_READY to figure whenever
they should enable 5-level paging or not.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/ResetVector/Ia32/IntelTdx.asm     | 13 ++++++++++++-
 OvmfPkg/ResetVector/Ia32/PageTables64.asm | 12 ++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
index c6b86019dfb9..7d775591a05b 100644
--- a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
+++ b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
@@ -179,7 +179,7 @@ InitTdx:
 ;
 ; Modified:  EAX, EDX
 ;
-; 0-NonTdx, 1-TdxBsp, 2-TdxAps
+; 0-NonTdx, 1-TdxBsp, 2-TdxAps, 3-TdxAps5Level
 ;
 CheckTdxFeaturesBeforeBuildPagetables:
     xor     eax, eax
@@ -200,6 +200,17 @@ TdxPostBuildPageTables:
     mov     byte[TDX_WORK_AREA_PGTBL_READY], 1
     OneTimeCallRet TdxPostBuildPageTables
 
+%if PG_5_LEVEL
+
+;
+; Set byte[TDX_WORK_AREA_PGTBL_READY] to 2
+;
+TdxPostBuildPageTables5Level:
+    mov     byte[TDX_WORK_AREA_PGTBL_READY], 2
+    OneTimeCallRet TdxPostBuildPageTables5Level
+
+%endif
+
 ;
 ; Check if TDX is enabled
 ;
diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index d736db028277..ada3dc0ffbe0 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -44,6 +44,7 @@ BITS    32
 
 %define TDX_BSP         1
 %define TDX_AP          2
+%define TDX_AP_5_LEVEL  3
 
 ;
 ; For OVMF, build some initial page tables at
@@ -214,6 +215,10 @@ SetCr3ForPageTables64:
     je        TdxBspInit
     cmp       eax, TDX_AP
     je        SetCr3
+%if PG_5_LEVEL
+    cmp       eax, TDX_AP_5_LEVEL
+    je        SetCr3La57
+%endif
 
     ; Check whether the SEV is active and populate the SevEsWorkArea
     OneTimeCall   CheckSevFeatures
@@ -252,6 +257,13 @@ TdxBspInit:
     ; TDX BSP workflow
     ;
     ClearOvmfPageTables
+%if PG_5_LEVEL
+    Check5LevelPaging Tdx4Level
+    CreatePageTables5Level 0
+    OneTimeCall TdxPostBuildPageTables5Level
+    jmp SetCr3La57
+Tdx4Level:
+%endif
     CreatePageTables4Level 0
     OneTimeCall TdxPostBuildPageTables
     jmp SetCr3
-- 
2.43.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115808): https://edk2.groups.io/g/devel/message/115808
Mute This Topic: https://groups.io/mt/104506795/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 08/10] OvmfPkg/ResetVector: wire up 5-level paging for TDX

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> BSP workflow is quite simliar to the non-coco case.
> 
> TDX_WORK_AREA_PGTBL_READY is used to record the paging mode:
>   1 == 4-level paging
>   2 == 5-level paging
> 
> APs will look at TDX_WORK_AREA_PGTBL_READY to figure whenever
> they should enable 5-level paging or not.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/ResetVector/Ia32/IntelTdx.asm     | 13 ++++++++++++-
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 12 ++++++++++++
>  2 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
> index c6b86019dfb9..7d775591a05b 100644
> --- a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
> +++ b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
> @@ -179,7 +179,7 @@ InitTdx:
>  ;
>  ; Modified:  EAX, EDX
>  ;
> -; 0-NonTdx, 1-TdxBsp, 2-TdxAps
> +; 0-NonTdx, 1-TdxBsp, 2-TdxAps, 3-TdxAps5Level
>  ;
>  CheckTdxFeaturesBeforeBuildPagetables:
>      xor     eax, eax
> @@ -200,6 +200,17 @@ TdxPostBuildPageTables:
>      mov     byte[TDX_WORK_AREA_PGTBL_READY], 1
>      OneTimeCallRet TdxPostBuildPageTables
>  
> +%if PG_5_LEVEL
> +
> +;
> +; Set byte[TDX_WORK_AREA_PGTBL_READY] to 2
> +;
> +TdxPostBuildPageTables5Level:
> +    mov     byte[TDX_WORK_AREA_PGTBL_READY], 2
> +    OneTimeCallRet TdxPostBuildPageTables5Level
> +
> +%endif
> +
>  ;
>  ; Check if TDX is enabled
>  ;
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index d736db028277..ada3dc0ffbe0 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -44,6 +44,7 @@ BITS    32
>  
>  %define TDX_BSP         1
>  %define TDX_AP          2
> +%define TDX_AP_5_LEVEL  3
>  
>  ;
>  ; For OVMF, build some initial page tables at
> @@ -214,6 +215,10 @@ SetCr3ForPageTables64:
>      je        TdxBspInit
>      cmp       eax, TDX_AP
>      je        SetCr3
> +%if PG_5_LEVEL
> +    cmp       eax, TDX_AP_5_LEVEL
> +    je        SetCr3La57
> +%endif
>  
>      ; Check whether the SEV is active and populate the SevEsWorkArea
>      OneTimeCall   CheckSevFeatures
> @@ -252,6 +257,13 @@ TdxBspInit:
>      ; TDX BSP workflow
>      ;
>      ClearOvmfPageTables
> +%if PG_5_LEVEL
> +    Check5LevelPaging Tdx4Level
> +    CreatePageTables5Level 0
> +    OneTimeCall TdxPostBuildPageTables5Level
> +    jmp SetCr3La57
> +Tdx4Level:
> +%endif
>      CreatePageTables4Level 0
>      OneTimeCall TdxPostBuildPageTables
>      jmp SetCr3

With SetCr3La57 eliminated as I request under patch #6, this patch looks
fine to me.

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116087): https://edk2.groups.io/g/devel/message/116087
Mute This Topic: https://groups.io/mt/104506795/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 09/10] OvmfPkg/ResetVector: leave SEV VC handler installed longer

[Gerd Hoffmann]

When running in SEV mode keep the VC handler installed.
Add a function to uninstall it later.

This allows using the cpuid instruction in SetCr3ForPageTables64,
which is needed to check for la57 & 1G page support.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/ResetVector/Ia32/AmdSev.asm       | 12 ++++++++++--
 OvmfPkg/ResetVector/Ia32/PageTables64.asm |  1 +
 OvmfPkg/ResetVector/Main.asm              |  4 ++++
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
index ed94f1dc668f..9063ce1080d3 100644
--- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
+++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
@@ -320,9 +320,9 @@ NoSevEsVcHlt:
 NoSevPass:
     xor       eax, eax
 
-SevExit:
     ;
-    ; Clear exception handlers and stack
+    ; When NOT running in SEV mode: clear exception handlers and stack here.
+    ; Otherwise: SevClearVcHandlerAndStack must be called later.
     ;
     push      eax
     mov       eax, ADDR_OF(IdtrClear)
@@ -330,8 +330,16 @@ SevExit:
     pop       eax
     mov       esp, 0
 
+SevExit:
     OneTimeCallRet CheckSevFeatures
 
+SevClearVcHandlerAndStack:
+    ; Clear exception handlers and stack
+    mov       eax, ADDR_OF(IdtrClear)
+    lidt      [cs:eax]
+    mov       esp, 0
+    OneTimeCallRet SevClearVcHandlerAndStack
+
 ; Start of #VC exception handling routines
 ;
 
diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index ada3dc0ffbe0..6e2063430802 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -250,6 +250,7 @@ SevInit:
     CreatePageTables4Level edx
     ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
     OneTimeCall   SevClearPageEncMaskForGhcbPage
+    OneTimeCall   SevClearVcHandlerAndStack
     jmp SetCr3
 
 TdxBspInit:
diff --git a/OvmfPkg/ResetVector/Main.asm b/OvmfPkg/ResetVector/Main.asm
index 46cfa87c4c0a..88b25db3bc9e 100644
--- a/OvmfPkg/ResetVector/Main.asm
+++ b/OvmfPkg/ResetVector/Main.asm
@@ -80,7 +80,11 @@ SearchBfv:
     ; Set the OVMF/SEV work area as appropriate.
     ;
     OneTimeCall CheckSevFeatures
+    cmp         byte[WORK_AREA_GUEST_TYPE], 1
+    jnz         NoSevIa32
+    OneTimeCall SevClearVcHandlerAndStack
 
+NoSevIa32:
     ;
     ; Restore initial EAX value into the EAX register
     ;
-- 
2.43.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115810): https://edk2.groups.io/g/devel/message/115810
Mute This Topic: https://groups.io/mt/104506799/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 09/10] OvmfPkg/ResetVector: leave SEV VC handler installed longer

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> When running in SEV mode keep the VC handler installed.
> Add a function to uninstall it later.
> 
> This allows using the cpuid instruction in SetCr3ForPageTables64,
> which is needed to check for la57 & 1G page support.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/ResetVector/Ia32/AmdSev.asm       | 12 ++++++++++--
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm |  1 +
>  OvmfPkg/ResetVector/Main.asm              |  4 ++++
>  3 files changed, 15 insertions(+), 2 deletions(-)

I'll leave this to Tom.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116089): https://edk2.groups.io/g/devel/message/116089
Mute This Topic: https://groups.io/mt/104506799/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 09/10] OvmfPkg/ResetVector: leave SEV VC handler installed longer

[Lendacky, Thomas via groups.io]

On 2/22/24 05:54, Gerd Hoffmann wrote:
> When running in SEV mode keep the VC handler installed.
> Add a function to uninstall it later.
> 
> This allows using the cpuid instruction in SetCr3ForPageTables64,
> which is needed to check for la57 & 1G page support.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Looks good, just one minor comment below.

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>   OvmfPkg/ResetVector/Ia32/AmdSev.asm       | 12 ++++++++++--
>   OvmfPkg/ResetVector/Ia32/PageTables64.asm |  1 +
>   OvmfPkg/ResetVector/Main.asm              |  4 ++++
>   3 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> index ed94f1dc668f..9063ce1080d3 100644
> --- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> +++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> @@ -320,9 +320,9 @@ NoSevEsVcHlt:
>   NoSevPass:
>       xor       eax, eax
>   
> -SevExit:
>       ;
> -    ; Clear exception handlers and stack
> +    ; When NOT running in SEV mode: clear exception handlers and stack here.
> +    ; Otherwise: SevClearVcHandlerAndStack must be called later.
>       ;
>       push      eax
>       mov       eax, ADDR_OF(IdtrClear)
> @@ -330,8 +330,16 @@ SevExit:
>       pop       eax
>       mov       esp, 0
>   
> +SevExit:
>       OneTimeCallRet CheckSevFeatures
>   
> +SevClearVcHandlerAndStack:
> +    ; Clear exception handlers and stack
> +    mov       eax, ADDR_OF(IdtrClear)
> +    lidt      [cs:eax]
> +    mov       esp, 0
> +    OneTimeCallRet SevClearVcHandlerAndStack
> +
>   ; Start of #VC exception handling routines
>   ;
>   
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index ada3dc0ffbe0..6e2063430802 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -250,6 +250,7 @@ SevInit:
>       CreatePageTables4Level edx
>       ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
>       OneTimeCall   SevClearPageEncMaskForGhcbPage
> +    OneTimeCall   SevClearVcHandlerAndStack
>       jmp SetCr3
>   
>   TdxBspInit:
> diff --git a/OvmfPkg/ResetVector/Main.asm b/OvmfPkg/ResetVector/Main.asm
> index 46cfa87c4c0a..88b25db3bc9e 100644
> --- a/OvmfPkg/ResetVector/Main.asm
> +++ b/OvmfPkg/ResetVector/Main.asm
> @@ -80,7 +80,11 @@ SearchBfv:
>       ; Set the OVMF/SEV work area as appropriate.
>       ;
>       OneTimeCall CheckSevFeatures
> +    cmp         byte[WORK_AREA_GUEST_TYPE], 1
> +    jnz         NoSevIa32
> +    OneTimeCall SevClearVcHandlerAndStack

I think it is safe to invoke SevClearVcHandlerAndStack no matter what if 
you want to avoid the cmp and jnz instructions.

Thanks,
Tom

>   
> +NoSevIa32:
>       ;
>       ; Restore initial EAX value into the EAX register
>       ;


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116176): https://edk2.groups.io/g/devel/message/116176
Mute This Topic: https://groups.io/mt/104506799/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 10/10] OvmfPkg/ResetVector: wire up 5-level paging for SEV

[Gerd Hoffmann]

Removes the GetSevCBitMaskAbove31 OneTimeCall because we need that twice
(for 4-level and 5-level paging).  Open code the single instruction left
in that function instead.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/ResetVector/Ia32/AmdSev.asm       |  8 --------
 OvmfPkg/ResetVector/Ia32/PageTables64.asm | 14 +++++++++++++-
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
index 9063ce1080d3..d1e5e8dfae71 100644
--- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
+++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
@@ -191,14 +191,6 @@ pageTableEntries4kLoop:
 SevClearPageEncMaskForGhcbPageExit:
     OneTimeCallRet SevClearPageEncMaskForGhcbPage
 
-; Get the C-bit mask above 31.
-; Modified: EDX
-;
-; The value is returned in the EDX
-GetSevCBitMaskAbove31:
-    mov       edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
-    OneTimeCallRet GetSevCBitMaskAbove31
-
 %endif
 
 ; Check if Secure Encrypted Virtualization (SEV) features are enabled.
diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index 6e2063430802..55664fa64f62 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -243,11 +243,23 @@ SevInit:
     ; SEV workflow
     ;
     ClearOvmfPageTables
+%if PG_5_LEVEL
+    Check5LevelPaging Sev4Level
     ; If SEV is enabled, the C-bit position is always above 31.
     ; The mask will be saved in the EDX and applied during the
     ; the page table build below.
-    OneTimeCall   GetSevCBitMaskAbove31
+    mov edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
+    CreatePageTables5Level edx
+    Enable5LevelPaging
+    jmp SevCommon
+Sev4Level:
+%endif
+    ; If SEV is enabled, the C-bit position is always above 31.
+    ; The mask will be saved in the EDX and applied during the
+    ; the page table build below.
+    mov edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
     CreatePageTables4Level edx
+SevCommon:
     ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
     OneTimeCall   SevClearPageEncMaskForGhcbPage
     OneTimeCall   SevClearVcHandlerAndStack
-- 
2.43.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115812): https://edk2.groups.io/g/devel/message/115812
Mute This Topic: https://groups.io/mt/104506802/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 10/10] OvmfPkg/ResetVector: wire up 5-level paging for SEV

[Laszlo Ersek]

On 2/22/24 12:54, Gerd Hoffmann wrote:
> Removes the GetSevCBitMaskAbove31 OneTimeCall because we need that twice
> (for 4-level and 5-level paging).  Open code the single instruction left
> in that function instead.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/ResetVector/Ia32/AmdSev.asm       |  8 --------
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 14 +++++++++++++-
>  2 files changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> index 9063ce1080d3..d1e5e8dfae71 100644
> --- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> +++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> @@ -191,14 +191,6 @@ pageTableEntries4kLoop:
>  SevClearPageEncMaskForGhcbPageExit:
>      OneTimeCallRet SevClearPageEncMaskForGhcbPage
>  
> -; Get the C-bit mask above 31.
> -; Modified: EDX
> -;
> -; The value is returned in the EDX
> -GetSevCBitMaskAbove31:
> -    mov       edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
> -    OneTimeCallRet GetSevCBitMaskAbove31
> -
>  %endif
>  
>  ; Check if Secure Encrypted Virtualization (SEV) features are enabled.
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 6e2063430802..55664fa64f62 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -243,11 +243,23 @@ SevInit:
>      ; SEV workflow
>      ;
>      ClearOvmfPageTables
> +%if PG_5_LEVEL
> +    Check5LevelPaging Sev4Level
>      ; If SEV is enabled, the C-bit position is always above 31.
>      ; The mask will be saved in the EDX and applied during the
>      ; the page table build below.
> -    OneTimeCall   GetSevCBitMaskAbove31
> +    mov edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
> +    CreatePageTables5Level edx
> +    Enable5LevelPaging
> +    jmp SevCommon
> +Sev4Level:
> +%endif
> +    ; If SEV is enabled, the C-bit position is always above 31.
> +    ; The mask will be saved in the EDX and applied during the
> +    ; the page table build below.
> +    mov edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
>      CreatePageTables4Level edx
> +SevCommon:
>      ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
>      OneTimeCall   SevClearPageEncMaskForGhcbPage
>      OneTimeCall   SevClearVcHandlerAndStack

This patch looks fine to me (especially the SevCommon label), but can
you please reimplement GetSevCBitMaskAbove31 as a macro, rather than
open-coding the

  mov edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]

instruction?

Thanks!
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116088): https://edk2.groups.io/g/devel/message/116088
Mute This Topic: https://groups.io/mt/104506802/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-