Re: [PATCH] MdeModulePkg PeiCore: Not assume PpiDescriptor and Ppi in same range

["Gao, Liming" <liming.gao@intel.com>]

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, June 28, 2018 4:56 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Huang, Qing <qing.huang@intel.com>
> Subject: [PATCH] MdeModulePkg PeiCore: Not assume PpiDescriptor and Ppi in same range
> 
> Current code assumes PpiDescriptor and Ppi are in same range
> (heap/stack/hole).
> 
> This patch removes the assumption.
> 
> Descriptor needs to be converted first. It is also handled by this patch.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Qing Huang <qing.huang@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  MdeModulePkg/Core/Pei/Ppi/Ppi.c | 204 ++++++++++++++++++++++------------------
>  1 file changed, 115 insertions(+), 89 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Pei/Ppi/Ppi.c b/MdeModulePkg/Core/Pei/Ppi/Ppi.c
> index 139cfeda0575..d8ba2dd42b59 100644
> --- a/MdeModulePkg/Core/Pei/Ppi/Ppi.c
> +++ b/MdeModulePkg/Core/Pei/Ppi/Ppi.c
> @@ -38,9 +38,9 @@ InitializePpiServices (
> 
>  /**
> 
> -  Migrate Single PPI Pointer from the temporary memory to PEI installed memory.
> +  Migrate Pointer from the temporary memory to PEI installed memory.
> 
> -  @param PpiPointer      Pointer to Ppi
> +  @param Pointer         Pointer to the Pointer needs to be converted.
>    @param TempBottom      Base of old temporary memory
>    @param TempTop         Top of old temporary memory
>    @param Offset          Offset of new memory to old temporary memory.
> @@ -48,64 +48,138 @@ InitializePpiServices (
> 
>  **/
>  VOID
> -ConvertSinglePpiPointer (
> -  IN PEI_PPI_LIST_POINTERS *PpiPointer,
> +ConvertPointer (
> +  IN OUT VOID              **Pointer,
>    IN UINTN                 TempBottom,
>    IN UINTN                 TempTop,
>    IN UINTN                 Offset,
>    IN BOOLEAN               OffsetPositive
>    )
>  {
> -  if (((UINTN)PpiPointer->Raw < TempTop) &&
> -      ((UINTN)PpiPointer->Raw >= TempBottom)) {
> -    //
> -    // Convert the pointer to the PPI descriptor from the old TempRam
> -    // to the relocated physical memory.
> -    //
> +  if (((UINTN) *Pointer < TempTop) &&
> +    ((UINTN) *Pointer >= TempBottom)) {
>      if (OffsetPositive) {
> -      PpiPointer->Raw = (VOID *) ((UINTN)PpiPointer->Raw + Offset);
> +      *Pointer = (VOID *) ((UINTN) *Pointer + Offset);
>      } else {
> -      PpiPointer->Raw = (VOID *) ((UINTN)PpiPointer->Raw - Offset);
> +      *Pointer = (VOID *) ((UINTN) *Pointer - Offset);
>      }
> +  }
> +}
> 
> -    //
> -    // Only when the PEIM descriptor is in the old TempRam should it be necessary
> -    // to try to convert the pointers in the PEIM descriptor
> -    //
> +/**
> 
> -    if (((UINTN)PpiPointer->Ppi->Guid < TempTop) &&
> -        ((UINTN)PpiPointer->Ppi->Guid >= TempBottom)) {
> -      //
> -      // Convert the pointer to the GUID in the PPI or NOTIFY descriptor
> -      // from the old TempRam to the relocated physical memory.
> -      //
> -      if (OffsetPositive) {
> -        PpiPointer->Ppi->Guid = (VOID *) ((UINTN)PpiPointer->Ppi->Guid + Offset);
> -      } else {
> -        PpiPointer->Ppi->Guid = (VOID *) ((UINTN)PpiPointer->Ppi->Guid - Offset);
> -      }
> -    }
> +  Migrate Pointer in ranges of the temporary memory to PEI installed memory.
> +
> +  @param SecCoreData     Points to a data structure containing SEC to PEI handoff data, such as the size
> +                         and location of temporary RAM, the stack location and the BFV location.
> +  @param PrivateData     Pointer to PeiCore's private data structure.
> +  @param Pointer         Pointer to the Pointer needs to be converted.
> +
> +**/
> +VOID
> +ConvertPointerInRanges (
> +  IN CONST EFI_SEC_PEI_HAND_OFF  *SecCoreData,
> +  IN PEI_CORE_INSTANCE           *PrivateData,
> +  IN OUT VOID                    **Pointer
> +  )
> +{
> +  UINT8                 IndexHole;
> 
> +  if (PrivateData->MemoryPages.Size != 0) {
>      //
> -    // Convert the pointer to the PPI interface structure in the PPI descriptor
> -    // from the old TempRam to the relocated physical memory.
> +    // Convert PPI pointer in old memory pages
> +    // It needs to be done before Convert PPI pointer in old Heap
>      //
> -    if ((UINTN)PpiPointer->Ppi->Ppi < TempTop &&
> -        (UINTN)PpiPointer->Ppi->Ppi >= TempBottom) {
> -      if (OffsetPositive) {
> -        PpiPointer->Ppi->Ppi = (VOID *) ((UINTN)PpiPointer->Ppi->Ppi + Offset);
> -      } else {
> -        PpiPointer->Ppi->Ppi = (VOID *) ((UINTN)PpiPointer->Ppi->Ppi - Offset);
> -      }
> +    ConvertPointer (
> +      Pointer,
> +      (UINTN)PrivateData->MemoryPages.Base,
> +      (UINTN)PrivateData->MemoryPages.Base + PrivateData->MemoryPages.Size,
> +      PrivateData->MemoryPages.Offset,
> +      PrivateData->MemoryPages.OffsetPositive
> +      );
> +  }
> +
> +  //
> +  // Convert PPI pointer in old Heap
> +  //
> +  ConvertPointer (
> +    Pointer,
> +    (UINTN)SecCoreData->PeiTemporaryRamBase,
> +    (UINTN)SecCoreData->PeiTemporaryRamBase + SecCoreData->PeiTemporaryRamSize,
> +    PrivateData->HeapOffset,
> +    PrivateData->HeapOffsetPositive
> +    );
> +
> +  //
> +  // Convert PPI pointer in old Stack
> +  //
> +  ConvertPointer (
> +    Pointer,
> +    (UINTN)SecCoreData->StackBase,
> +    (UINTN)SecCoreData->StackBase + SecCoreData->StackSize,
> +    PrivateData->StackOffset,
> +    PrivateData->StackOffsetPositive
> +    );
> +
> +  //
> +  // Convert PPI pointer in old TempRam Hole
> +  //
> +  for (IndexHole = 0; IndexHole < HOLE_MAX_NUMBER; IndexHole ++) {
> +    if (PrivateData->HoleData[IndexHole].Size == 0) {
> +      continue;
>      }
> +
> +    ConvertPointer (
> +      Pointer,
> +      (UINTN)PrivateData->HoleData[IndexHole].Base,
> +      (UINTN)PrivateData->HoleData[IndexHole].Base + PrivateData->HoleData[IndexHole].Size,
> +      PrivateData->HoleData[IndexHole].Offset,
> +      PrivateData->HoleData[IndexHole].OffsetPositive
> +      );
>    }
>  }
> 
>  /**
> 
> +  Migrate Single PPI Pointer from the temporary memory to PEI installed memory.
> +
> +  @param SecCoreData     Points to a data structure containing SEC to PEI handoff data, such as the size
> +                         and location of temporary RAM, the stack location and the BFV location.
> +  @param PrivateData     Pointer to PeiCore's private data structure.
> +  @param PpiPointer      Pointer to Ppi
> +
> +**/
> +VOID
> +ConvertSinglePpiPointer (
> +  IN CONST EFI_SEC_PEI_HAND_OFF  *SecCoreData,
> +  IN PEI_CORE_INSTANCE           *PrivateData,
> +  IN PEI_PPI_LIST_POINTERS       *PpiPointer
> +  )
> +{
> +  //
> +  // 1. Convert the pointer to the PPI descriptor from the old TempRam
> +  //    to the relocated physical memory.
> +  // It (for the pointer to the PPI descriptor) needs to be done before 2 (for
> +  // the pointer to the GUID) and 3 (for the pointer to the PPI interface structure).
> +  //
> +  ConvertPointerInRanges (SecCoreData, PrivateData, &PpiPointer->Raw);
> +  //
> +  // 2. Convert the pointer to the GUID in the PPI or NOTIFY descriptor
> +  //    from the old TempRam to the relocated physical memory.
> +  //
> +  ConvertPointerInRanges (SecCoreData, PrivateData, (VOID **) &PpiPointer->Ppi->Guid);
> +  //
> +  // 3. Convert the pointer to the PPI interface structure in the PPI descriptor
> +  //    from the old TempRam to the relocated physical memory.
> +  //
> +  ConvertPointerInRanges (SecCoreData, PrivateData, (VOID **) &PpiPointer->Ppi->Ppi);
> +}
> +
> +/**
> +
>    Migrate PPI Pointers from the temporary memory to PEI installed memory.
> 
> -  @param SecCoreData     Points to a data structure containing SEC to PEI handoff data, such as the size
> +  @param SecCoreData     Points to a data structure containing SEC to PEI handoff data, such as the size
>                           and location of temporary RAM, the stack location and the BFV location.
>    @param PrivateData     Pointer to PeiCore's private data structure.
> 
> @@ -117,62 +191,14 @@ ConvertPpiPointers (
>    )
>  {
>    UINT8                 Index;
> -  UINT8                 IndexHole;
> 
>    for (Index = 0; Index < PcdGet32 (PcdPeiCoreMaxPpiSupported); Index++) {
>      if (Index < PrivateData->PpiData.PpiListEnd || Index > PrivateData->PpiData.NotifyListEnd) {
> -      if (PrivateData->MemoryPages.Size != 0) {
> -        //
> -        // Convert PPI pointer in old memory pages
> -        // It needs to be done before Convert PPI pointer in old Heap
> -        //
> -        ConvertSinglePpiPointer (
> -          &PrivateData->PpiData.PpiListPtrs[Index],
> -          (UINTN)PrivateData->MemoryPages.Base,
> -          (UINTN)PrivateData->MemoryPages.Base + PrivateData->MemoryPages.Size,
> -          PrivateData->MemoryPages.Offset,
> -          PrivateData->MemoryPages.OffsetPositive
> -          );
> -      }
> -
> -      //
> -      // Convert PPI pointer in old Heap
> -      //
>        ConvertSinglePpiPointer (
> -        &PrivateData->PpiData.PpiListPtrs[Index],
> -        (UINTN)SecCoreData->PeiTemporaryRamBase,
> -        (UINTN)SecCoreData->PeiTemporaryRamBase + SecCoreData->PeiTemporaryRamSize,
> -        PrivateData->HeapOffset,
> -        PrivateData->HeapOffsetPositive
> -        );
> -
> -      //
> -      // Convert PPI pointer in old Stack
> -      //
> -      ConvertSinglePpiPointer (
> -        &PrivateData->PpiData.PpiListPtrs[Index],
> -        (UINTN)SecCoreData->StackBase,
> -        (UINTN)SecCoreData->StackBase + SecCoreData->StackSize,
> -        PrivateData->StackOffset,
> -        PrivateData->StackOffsetPositive
> +        SecCoreData,
> +        PrivateData,
> +        &PrivateData->PpiData.PpiListPtrs[Index]
>          );
> -
> -      //
> -      // Convert PPI pointer in old TempRam Hole
> -      //
> -      for (IndexHole = 0; IndexHole < HOLE_MAX_NUMBER; IndexHole ++) {
> -        if (PrivateData->HoleData[IndexHole].Size == 0) {
> -          continue;
> -        }
> -
> -        ConvertSinglePpiPointer (
> -          &PrivateData->PpiData.PpiListPtrs[Index],
> -          (UINTN)PrivateData->HoleData[IndexHole].Base,
> -          (UINTN)PrivateData->HoleData[IndexHole].Base + PrivateData->HoleData[IndexHole].Size,
> -          PrivateData->HoleData[IndexHole].Offset,
> -          PrivateData->HoleData[IndexHole].OffsetPositive
> -          );
> -      }
>      }
>    }
>  }
> --
> 2.7.0.windows.1