public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Marvin H?user <Marvin.Haeuser@outlook.com>
To: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: Andrew Fish <afish@apple.com>
Subject: Re: [MdeModulePkg][PeiCore] I seemed to have crashed the PEI Core by grabbing memory from PeiTemporaryRamBase?
Date: Sat, 13 Aug 2016 01:46:38 +0000	[thread overview]
Message-ID: <VI1PR06MB17927D516797C927220B0F1D80100@VI1PR06MB1792.eurprd06.prod.outlook.com> (raw)
In-Reply-To: <7B465500-570A-4B78-B1F2-458C36E7DC08@apple.com>

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


  reply	other threads:[~2016-08-13  1:47 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-12 23:25 [MdeModulePkg][PeiCore] I seemed to have crashed the PEI Core by grabbing memory from PeiTemporaryRamBase? Andrew Fish
2016-08-13  1:46 ` Marvin H?user [this message]
2016-08-14  2:24   ` Marvin H?user
2016-08-15 15:54 ` Gao, Liming
2016-08-15 16:11   ` Andrew Fish
2016-08-16 16:49     ` Gao, Liming

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=VI1PR06MB17927D516797C927220B0F1D80100@VI1PR06MB1792.eurprd06.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox