[MdeModulePkg][PeiCore] I seemed to have crashed the PEI Core by grabbing memory from PeiTemporaryRamBase?

[Andrew Fish <afish@apple.com>]

I grabbed some memory between SEC and the PEI Core by adjusting SecCoreData-> PeiTemporaryRamBase and SecCoreData-> PeiTemporaryRamSize.

When looking at the code I don't really understand the logic of the algorithm? So maybe I'm doing something wrong. 

This adjustment does not seem right to me?
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c#L768
      //
      // Heap Offset
      //
      BaseOfNewHeap = TopOfNewStack;
      if (BaseOfNewHeap >= (UINTN)SecCoreData->PeiTemporaryRamBase) {
        Private->HeapOffsetPositive = TRUE;
        Private->HeapOffset = (UINTN)(BaseOfNewHeap - (UINTN)SecCoreData->PeiTemporaryRamBase);
      } else {
        Private->HeapOffsetPositive = FALSE;
        Private->HeapOffset = (UINTN)((UINTN)SecCoreData->PeiTemporaryRamBase - BaseOfNewHeap);
      }


The above code seems to be making a very strange adjustment. I noticed the adjustment in my failing case was off by 0xC0 which is the amount of memory I carved out prior to entering the PEI Core. 

https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c#L796

      //
      // Temporary Ram Support PPI is provided by platform, it will copy 
      // temporary memory to permenent memory and do stack switching.
      // After invoking Temporary Ram Support PPI, the following code's 
      // stack is in permanent memory.
      //
      TemporaryRamSupportPpi->TemporaryRamMigration (
                                PeiServices,
                                TemporaryRamBase,
                                (EFI_PHYSICAL_ADDRESS)(UINTN)(TopOfNewStack - TemporaryStackSize),
                                TemporaryRamSize
                                );


And this is also a case in which the stack got bigger. But it seems to me the shift if really defined by TemporaryRamBase, TopOfNewStack, and TemporaryStackSize in this case. 

The failure I hit was OldCoreData->Fv pointer was shifted so when the PPI was called the system crashed. Is this a bug in the gEfiTemporaryRamSupportPpiGuid path?

If I changed the HeadOffset algorithm my crash went away? Private->HeapOffset = ((UINTN)TopOfNewStack - TemporaryStackSize) - TemporaryRamBase;

Thanks,

Andrew Fish

PS My failure case was the EmulatorPkg. I've not had a chance to verify this failure in the open source yet, but I'm guessing reversing this #if will make it happen.


https://github.com/tianocore/edk2/blob/master/EmulatorPkg/Sec/Sec.c#L107

#if 0
  // Tell the PEI Core to not use our buffer in temp RAM
  SecPpiList = (EFI_PEI_PPI_DESCRIPTOR *)SecCoreData->PeiTemporaryRamBase;
  SecCoreData->PeiTemporaryRamBase = (VOID *)((UINTN)SecCoreData->PeiTemporaryRamBase + SecReseveredMemorySize);
  SecCoreData->PeiTemporaryRamSize -= SecReseveredMemorySize;
#else
  {
    //
    // When I subtrack from SecCoreData->PeiTemporaryRamBase PEI Core crashes? Either there is a bug
    // or I don't understand temp RAM correctly?
    //
    EFI_PEI_PPI_DESCRIPTOR    PpiArray[10];

    SecPpiList = &PpiArray[0];
    ASSERT (sizeof (PpiArray) >= SecReseveredMemorySize);
  }
#endif