Andrew:
  In permanent memory, PeiCore places heap base as stack top. Heap is above 
stack. There is no hole between them. SEC needs to follow this layout and 
migrate the temporary memory to permanent memory. It should copy TemporaryRam 
HEAP and STACK range separately. HEAP range is specified by PeiTemporaryRamBase 
and PeiTemporaryRamSize, and STACK range is specified by StackBase and 
StackSize. The grabbed memory is not migrated, because PeiCore doesn't know it. 
But, EmulatorPkg Sec SecTemporaryRamSupport() migrates the whole temporary 
memory together. The grabbed memory is also migrated and wrongly regarded as 
heap data. So, the fix is to update SecTemporaryRamSupport() implementation in 
SEC. 

Thanks
Liming
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Andrew 
Fish
Sent: Saturday, August 13, 2016 7:25 AM
To: edk2-devel <edk2-devel@lists.01.org>
Subject: [edk2] [MdeModulePkg][PeiCore] I seemed to have crashed the PEI Core 
by grabbing memory from PeiTemporaryRamBase?

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




_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to