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

["Gerd Hoffmann" <kraxel@redhat.com>]

  Hi,

> > +    ;
> > +    ; use 5-level paging with gigabyte pages
> > +    ;

> > +    ; level 5
> > +    mov     dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDP_ATTR
> > +    mov     dword[PT_ADDR (4)], edx
> > +
> > +    ; level 4
> > +    mov     dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + PAGE_PDP_ATTR
> > +    mov     dword[PT_ADDR (0x1004)], edx
> > +
> > +    ; level 3
> > +    mov     dword[PT_ADDR (0x2000)], (0 << 30) + PAGE_2M_PDE_ATTR
> > +    mov     dword[PT_ADDR (0x2004)], edx
> > +    mov     dword[PT_ADDR (0x2008)], (1 << 30) + PAGE_2M_PDE_ATTR
> > +    mov     dword[PT_ADDR (0x200c)], edx
> > +    mov     dword[PT_ADDR (0x2010)], (2 << 30) + PAGE_2M_PDE_ATTR
> > +    mov     dword[PT_ADDR (0x2014)], edx
> > +    mov     dword[PT_ADDR (0x2018)], (3 << 30) + PAGE_2M_PDE_ATTR
> > +    mov     dword[PT_ADDR (0x201c)], edx
> 
> (2) So this is my problem. I've found this hard to decipher. The
> original (4-level) counterpart is much better commented.
> 
> So you either consulted the documentation to invent this (but then
> didn't write up your thought process), or you just "winged it" (which
> may still be fine, but you need to explain the setup even more).
> 
> As far as I can tell, we're covering the first 4GB of address space with
> 4 pages of 1GB size each, and because of that, we need not go deeper
> than level 3.

Correct.

> That one sentence in a comment (which would have cost you 5 seconds)

Well, there is a comment saying it's 5-level paging with gigabyte pages
(quoted above).  I'll add more details.

> (3) While the macro PAGE_2M_PDE_ATTR might expand to just the right
> constant here, the "2M" in the name is extremely confusing. Please
> introduce a new macro for selecting 1GB pages.

I think rename "2M" to "LARGEPAGE" would be better here.  The page
directory bit essentially says "stop page table walk here, there are no
page tables/page directories below this".  When set in a 4-level page
directory you'll get a 2M page, when set in a 3-level page directory
you'll get a 1G page.

> > +    ; done
> > +    jmp     SetCr3
> 
> (4) Functionality question: is the 5-level branch compatible with SEV
> and/or TDX? Asking because:
> 
> - EDX is still from GetSevCBitMaskAbove31. I don't know what happens if
> SEV is enabled and such an EDX is used at level 5.

Not fully sure.  The 4-level paging code applies edx on all levels, so I
did the same for 5-level paging.

Tom?

> - jumping to the SetCr3 label from here omits the
> "SevClearPageEncMaskForGhcbPage" and "TdxPostBuildPageTables" pieces of
> logic.

Good catch.  That is a bug.

take care,
  Gerd



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