Andrew:
  PI spec has not defined such information. But, PPI implementation and PeiCore 
needs to align new heap and stack layout. The full PPI should include new heap 
base and new stack base. Current PPI has only one Base. Then, PPI 
implementation needs mach PeiCore implementation.

Thanks
Liming
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Andrew 
Fish
Sent: Tuesday, August 16, 2016 12:12 AM
To: Gao, Liming <liming....@intel.com>
Cc: edk2-devel <edk2-devel@lists.01.org>
Subject: Re: [edk2] [MdeModulePkg][PeiCore] I seemed to have crashed the PEI 
Core by grabbing memory from PeiTemporaryRamBase?


> On Aug 15, 2016, at 8:54 AM, Gao, Liming wrote:
>
> 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.
>

Limiing,

I don't see any info in the PPI definition or the PI spec that defines the heap 
and stack are copied separately? The PPI just passes the entire ranges? That is 
why I assumes in the PPI case the offsets should be relative to the big shift?

/**
This service of the EFI_PEI_TEMPORARY_RAM_SUPPORT_PPI that migrates temporary 
RAM into
permanent memory.

@param PeiServices Pointer to the PEI Services Table.
@param TemporaryMemoryBase Source Address in temporary memory from which the 
SEC or PEIM will copy the
Temporary RAM contents.
@param PermanentMemoryBase Destination Address in permanent memory into which 
the SEC or PEIM will copy the
Temporary RAM contents.
@param CopySize Amount of memory to migrate from temporary to permanent memory.

@retval EFI_SUCCESS The data was successfully returned.
@retval EFI_INVALID_PARAMETER PermanentMemoryBase + CopySize > 
TemporaryMemoryBase when
TemporaryMemoryBase > PermanentMemoryBase.

**/
typedef
EFI_STATUS
(EFIAPI * TEMPORARY_RAM_MIGRATION)(
IN CONST EFI_PEI_SERVICES **PeiServices,
IN EFI_PHYSICAL_ADDRESS TemporaryMemoryBase,
IN EFI_PHYSICAL_ADDRESS PermanentMemoryBase,
IN UINTN CopySize
);


Thanks,

Andrew Fish

> 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
> 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<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto: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