public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io, thomas.lendacky@amd.com,
	Gerd Hoffmann <kraxel@redhat.com>
Cc: Erdem Aktas <erdemaktas@google.com>,
	Oliver Steffen <osteffen@redhat.com>,
	Jiewen Yao <jiewen.yao@intel.com>,
	Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Min Xu <min.m.xu@intel.com>, Michael Roth <michael.roth@amd.com>,
	Liming Gao <gaoliming@byosoft.com.cn>
Subject: Re: [edk2-devel] [PATCH v2 4/5] OvmfPkg/ResetVector: add 5-level paging support
Date: Fri, 2 Feb 2024 00:31:53 +0100	[thread overview]
Message-ID: <9b289ce3-bfe4-2654-7669-7cf04692fe2a@redhat.com> (raw)
In-Reply-To: <d64b418d-32f8-44df-96a7-9a9b1319bb0f@amd.com>

On 2/1/24 16:44, Lendacky, Thomas via groups.io wrote:
> On 1/30/24 06:32, Gerd Hoffmann wrote:
>> Compile the OVMF ResetVector with 5-level paging support in case
>> PcdUse5LevelPageTable is TRUE.
>>
>> When enabled the ResetVector will check at runtime whenever support for
>> 5-level paging and gigabyte pages is available.  In case both features
>> are supported it will run OVMF in 5-level paging mode, otherwise
>> fallback to 4-level paging.
>>
>> Gigabyte pages are required to make sure we can fit the page tables into
>> the available space.  We have six pages available, with gigabyte pages
>> we need three of them, with 2M pages we would need seven.
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>>   OvmfPkg/ResetVector/ResetVector.inf       |  1 +
>>   OvmfPkg/ResetVector/Ia32/PageTables64.asm | 77 +++++++++++++++++++++++
>>   OvmfPkg/ResetVector/ResetVector.nasmb     |  1 +
>>   3 files changed, 79 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 6fec6f2beeea..cf64c88b6cda 100644
>> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>> @@ -86,6 +86,82 @@ clearPageTablesMemoryLoop:
>>       mov     dword[ecx * 4 + PT_ADDR (0) - 4], eax
>>       loop    clearPageTablesMemoryLoop
>>   +%if PG_5_LEVEL
>> +
>> +    ; save GetSevCBitMaskAbove31 result (cpuid changes edx)
>> +    mov     edi, edx
>> +
>> +    ; check for cpuid leaf 0x07
>> +    mov     eax, 0x00
>> +    cpuid
> 
> Because of these CPUID instructions, this won't work for SEV-ES /
> SEV-SNP. To use these we'll need to have a (special 32-bit) #VC handler
> in place. Currently that is done in only in
> OvmfPkg/ResetVector/Ia32/AmdSev.asm for the CheckSevFeatures function,
> where the #VC handler is established at the beginning of the function,
> but it is removed when leaving the function.
> 
> The SEV support in general needs looking into in order to support
> 5-level paging. At the time the SEV support was developed, there wasn't
> a page table library and so there is some 4-level page table
> manipulation support in the BaseMemEncryptSevLib that really needs to be
> converted to use the page table library.
> 
> I don't have an objection to the series, as long as
> PcdUse5LevelPageTable is not set to TRUE by default for the Ovmf packages.

Well, I do have a slight objection:

> 
>> +    cmp     eax, 0x07
>> +    jb      Paging4Lvl
>> +
>> +    ; check for la57 (aka 5-level paging)
>> +    mov     eax, 0x07
>> +    mov     ecx, 0x00
>> +    cpuid
>> +    bt      ecx, 16
>> +    jnc     Paging4Lvl
>> +
>> +    ; check for cpuid leaf 0x80000001
>> +    mov     eax, 0x80000000
>> +    cpuid
>> +    cmp     eax, 0x80000001
>> +    jb      Paging4Lvl
>> +
>> +    ; check for 1g pages
>> +    mov     eax, 0x80000001
>> +    cpuid
>> +    bt      edx, 26
>> +    jnc     Paging4Lvl
>> +
>> +    ;
>> +    ; Use 5-level paging with gigabyte pages.
>> +    ;
>> +    ; We have 6 pages available for the early page tables,
>> +    ; due to the use of gigabyte pages we need three pages
>> +    ; and everything fits in.
>> +    ;
>> +    debugShowPostCode 0x51      ; 5-level paging
>> +
>> +    ; restore GetSevCBitMaskAbove31 result
>> +    mov     edx, edi
>> +
>> +    ; level 5
>> +    mov     dword[PT_ADDR (0)], PT_ADDR (0x1000) +
>> PAGE_PDE_DIRECTORY_ATTR
>> +    mov     dword[PT_ADDR (4)], edx
>> +
>> +    ; level 4
>> +    mov     dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) +
>> PAGE_PDE_DIRECTORY_ATTR
>> +    mov     dword[PT_ADDR (0x1004)], edx
>> +
>> +    ; level 3 (four 1GB pages for the lowest 4G)
>> +    mov     dword[PT_ADDR (0x2000)], (0 << 30) + PAGE_PDE_LARGEPAGE_ATTR
>> +    mov     dword[PT_ADDR (0x2004)], edx
>> +    mov     dword[PT_ADDR (0x2008)], (1 << 30) + PAGE_PDE_LARGEPAGE_ATTR
>> +    mov     dword[PT_ADDR (0x200c)], edx
>> +    mov     dword[PT_ADDR (0x2010)], (2 << 30) + PAGE_PDE_LARGEPAGE_ATTR
>> +    mov     dword[PT_ADDR (0x2014)], edx
>> +    mov     dword[PT_ADDR (0x2018)], (3 << 30) + PAGE_PDE_LARGEPAGE_ATTR
>> +    mov     dword[PT_ADDR (0x201c)], edx
>> +
>> +    ; set la57 bit in cr4
>> +    mov     eax, cr4
>> +    bts     eax, 12
>> +    mov     cr4, eax
>> +
>> +    ; done
>> +    jmp     PageTablesReady

Note this jump here...

>> +
>> +Paging4Lvl:
>> +    debugShowPostCode 0x41      ; 4-level paging
>> +
>> +    ; restore GetSevCBitMaskAbove31 result
>> +    mov     edx, edi
>> +
>> +%endif ; PG_5_LEVEL
>> +
>>       ;
>>       ; Top level Page Directory Pointers (1 * 512GB entry)
>>       ;
>> @@ -117,6 +193,7 @@ pageTableEntriesLoop:
>>       mov     [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
>>       loop    pageTableEntriesLoop
>>   +PageTablesReady:
>>       ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
>>       OneTimeCall   SevClearPageEncMaskForGhcbPage

Landing here.

I requested this; see point (4) at
<https://edk2.groups.io/g/devel/message/114745>.

But knowing (now!) that the neighborhood (= the 5 level paging setup)
isn't compatible with / reachable under SEV-ES in the first place, this
jump only seems wishful thinking.

The best I could propose: jump again to SetCr3 (like in v1), but add a
comment that it's not a mistake, but intentional (because the stuff
doesn't work on SEV-ES anyway).

Thanks
Laszlo


>>   diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb
>> b/OvmfPkg/ResetVector/ResetVector.nasmb
>> index 5832aaa8abf7..16b3eee57671 100644
>> --- a/OvmfPkg/ResetVector/ResetVector.nasmb
>> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb
>> @@ -49,6 +49,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 (#114982): https://edk2.groups.io/g/devel/message/114982
Mute This Topic: https://groups.io/mt/104052208/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



  parent reply	other threads:[~2024-02-01 23:32 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-30 12:31 [edk2-devel] [PATCH v2 0/5] OvmfPkg: Add support for 5-level paging Gerd Hoffmann
2024-01-30 12:32 ` [edk2-devel] [PATCH v2 1/5] MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert Gerd Hoffmann
2024-01-30 12:32 ` [edk2-devel] [PATCH v2 2/5] MdeModulePkg/DxeIplPeim: rename variable Gerd Hoffmann
2024-01-30 18:58   ` Laszlo Ersek
2024-01-30 12:32 ` [edk2-devel] [PATCH v2 3/5] OvmfPkg/ResetVector: improve page table flag names Gerd Hoffmann
2024-01-30 19:04   ` Laszlo Ersek
2024-01-30 19:46   ` Pedro Falcato
2024-01-30 22:28     ` Laszlo Ersek
2024-01-31  8:14       ` Gerd Hoffmann
2024-01-31 11:22         ` Laszlo Ersek
2024-01-31 17:50           ` Pedro Falcato
2024-01-30 12:32 ` [edk2-devel] [PATCH v2 4/5] OvmfPkg/ResetVector: add 5-level paging support Gerd Hoffmann
2024-01-30 19:13   ` Laszlo Ersek
2024-02-01 15:44   ` Lendacky, Thomas via groups.io
2024-02-01 16:33     ` Gerd Hoffmann
2024-02-01 23:31     ` Laszlo Ersek [this message]
2024-01-30 12:32 ` [edk2-devel] [PATCH v2 5/5] OvmfPkg/PlatformInitLib: " Gerd Hoffmann
2024-01-30 19:15 ` [edk2-devel] [PATCH v2 0/5] OvmfPkg: Add support for 5-level paging Laszlo Ersek
2024-02-01 16:01   ` Lendacky, Thomas via groups.io
2024-01-31  6:19 ` Min Xu
2024-01-31  8:24   ` Gerd Hoffmann
2024-02-01  5:44     ` Min Xu
2024-02-01  8:45       ` Gerd Hoffmann
2024-02-01 14:14       ` Gerd Hoffmann
2024-02-02  8:30         ` Min Xu
2024-02-02  8:44           ` Gerd Hoffmann
2024-02-02 19:28             ` Lendacky, Thomas via groups.io
2024-02-12 15:13               ` Gerd Hoffmann
2024-01-31 12:02 ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9b289ce3-bfe4-2654-7669-7cf04692fe2a@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox