Re: [edk2-devel] [PATCH] UefiPayloadPkg: Always split page table entry to 4K if it covers stack.

["Ni, Ray" <ray.ni@intel.com>]

Gerd,
We saw page fault in the following situation:
* a 2M page entry (with present bit set) points to some memory [p, p+2M)
* Firmware code wants to mark [p, p+4k) as read-only
* Firstly [p, p+2M) is split to 512 page-entries with each pointing to 4K memory (with present bit set still)
* Secondly, the R/W bit in first page entry is cleared

The code is in UefiCpuPkg/CpuDxe/CpuPageTable.c:

    //
    // Split 2M to 4K
    //
    ASSERT (SplitAttribute == Page4K);
    if (SplitAttribute == Page4K) {
      NewPageEntry = AllocatePagesFunc (1);
      DEBUG ((DEBUG_VERBOSE, "Split - 0x%x\n", NewPageEntry));
      if (NewPageEntry == NULL) {
        return RETURN_OUT_OF_RESOURCES;
      }

      BaseAddress = *PageEntry & ~AddressEncMask & PAGING_2M_ADDRESS_MASK_64;
      for (Index = 0; Index < SIZE_4KB / sizeof (UINT64); Index++) {
        NewPageEntry[Index] = (BaseAddress + SIZE_4KB * Index) | AddressEncMask | ((*PageEntry) & PAGE_PROGATE_BITS);
      }

      (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask | ((*PageEntry) & PAGE_ATTRIBUTE_BITS);

Page fault exception happens just after the above assignment.
We observed that the instruction causing the exception is accessing the stack and stack is within [p, p+2M) range.

To be frank, we are still trying to understand whether a CR3 flush or INVLPG should be performed immediately after the above assignment.

Before that's fully understood, we think the page table split for stack does no harm to the functionality and code complexity. That's why we choose this fix first.

I am not quite sure how Linux handles such case?

Thanks,
Ray

> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Tuesday, May 31, 2022 3:45 PM
> To: devel@edk2.groups.io; Liu, Zhiguang <zhiguang.liu@intel.com>
> Cc: Dong, Guo <guo.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Maurice Ma <maurice.ma@intel.com>; You, Benjamin
> <benjamin.you@intel.com>; Rhodes, Sean <sean@starlabs.systems>
> Subject: Re: [edk2-devel] [PATCH] UefiPayloadPkg: Always split page table entry to 4K if it covers stack.
> 
> On Tue, May 31, 2022 at 01:39:37PM +0800, Zhiguang Liu wrote:
> > There is a concern case that stack and a proteced DXE memory range is in
> > the same 2M Page Table entry, and somehow CPU doesn't flash the page
> > table entry cache for stack, and causes Page fault when using stack.
> 
> Can you clarify the "somehow" please?  Are we discussing a workaround
> for a cpu bug here?  If not this sounds like a tlbflush instruction is
> missing somewhere ...
> 
> take care,
>   Gerd