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

[Marvin H?user <Marvin.Haeuser@outlook.com>]

Sorry. For some reason, I didn't get to read the paragraph about TemporaryRamSupportPpi and shomehow skipped to the PS.
I suppose my hint is not related to the crash then, though I hope it was still helpful in some way, as it seems to assume that the PPI List is in the temporary heap nevertheless.

Regards,
Marvin.

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Marvin H?user
> Sent: Saturday, August 13, 2016 3:47 AM
> To: edk2-devel@lists.01.org
> Cc: Andrew Fish <afish@apple.com>
> Subject: Re: [edk2] [MdeModulePkg][PeiCore] I seemed to have crashed the
> PEI Core by grabbing memory from PeiTemporaryRamBase?
> 
> Hello Andrew,
> 
> Unfortunately I cannot test anything right now and I don't have a whole lot of
> knowledge in this area, though I might have a hint for you.
> 
> While PpiList is equal to the original TempRam base, the TempRam based
> passed to PEI is equal to the original TempRam base + the size of the PpiList,
> hence PpiList is smaller than the base address passed to PEI. The PpiList is
> then installed via the PeiServicesInstallPpi () function:
> 
> call:
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/
> PeiMain/PeiMain.c#L386
> implementation:
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/
> Ppi/Ppi.c#L183
> 
> The list is then added to PpiData.PpiListPtrs.
> 
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/
> Ppi/Ppi.c#L229
> 
> I am not sure at which point of time you are experiencing the crash, but after
> permanent memory is available, ConvertPpiPointers () is called, which then
> calls ConverSinglePpiPointer () for old heap, old stack and old hole (I'm afraid
> I do not know what TempRam Hole is and if it is related).
> 
> ConvertPpiPointers () call:
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/
> PeiMain/PeiMain.c#L237
> Old Heap ConverSinglePpiPointer () call:
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/
> Ppi/Ppi.c#L127
> 
> The call for the old heap conversion passes the TempRam base, which has
> been incremented as we know, and thus the comparison to TempBottom will
> fail, as TempBottom is PeiTemporaryRamBase, which is larger than PpiList,
> which is one of the items in PpiListPtrs, which is the object of the conversion.
> 
> comparison to TempBottom:
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/
> Ppi/Ppi.c#L60
> 
> As the pointer to the PpiList passed by SecCore is probably not converted as
> detailed above, I assume something post-mem attempts to access this
> former PpiList by the old temporary RAM address and that somehow causes
> trouble; I assume the SEC PpiList being part of the PEI memory is an
> assumption made by the person who wrote this code. I'm not sure about
> why it crashes, as I do not know the entire PEI control flow, though I hope
> this can help you in some way.
> 
> Please excuse me if I have made a mistake in understanding the referenced
> code and wasted your time.
> 
> Regards,
> Marvin.
> 
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Andrew Fish
> > Sent: Saturday, August 13, 2016 1: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#L1
> > 07
> >
> > #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