Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation

["Ni, Ray" <ray.ni@intel.com>]

Nhi,
Thanks for the follow up!

Reviewed-by: Ray Ni <ray.ni@intel.com>

Thanks,
Ray
> -----Original Message-----
> From: Nhi Pham <nhiphambka@gmail.com>
> Sent: Tuesday, December 5, 2023 9:48 PM
> To: devel@edk2.groups.io
> Cc: Nhi Pham <nhiphambka@gmail.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Ni, Ray <ray.ni@intel.com>; Sami Mujawar
> <sami.mujawar@arm.com>; Oliver Smith-Denny <osde@linux.microsoft.com>
> Subject: [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove
> HOB creation
> 
> According to the discussion in "StandaloneMmPkg: Fix HOB space and
> heap space conflicted issue" [1], Standalone MM modules should be HOB
> consumers where HOB is read-only. Therefore, this patch removes the
> supported functions for HOB creation in the StandaloneMmHobLib.
> 
> [1] https://edk2.groups.io/g/devel/message/108333
> 
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Oliver Smith-Denny <osde@linux.microsoft.com>
> Signed-off-by: Nhi Pham <nhiphambka@gmail.com>
> ---
>  StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c |
> 171 ++++++--------------
>  1 file changed, 51 insertions(+), 120 deletions(-)
> 
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
> b/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
> index ee61bdd227d0..bef66d167494 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
> +++
> b/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
> @@ -1,5 +1,5 @@
>  /** @file
> 
> -  HOB Library implementation for Standalone MM Core.
> 
> +  HOB Library implementation for Standalone MM modules.
> 
> 
> 
>  Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
> 
>  Copyright (c) 2017 - 2018, ARM Limited. All rights reserved.<BR>
> 
> @@ -250,48 +250,13 @@ GetBootModeHob (
>    return HandOffHob->BootMode;
> 
>  }
> 
> 
> 
> -VOID *
> 
> -CreateHob (
> 
> -  IN  UINT16  HobType,
> 
> -  IN  UINT16  HobLength
> 
> -  )
> 
> -{
> 
> -  EFI_HOB_HANDOFF_INFO_TABLE  *HandOffHob;
> 
> -  EFI_HOB_GENERIC_HEADER      *HobEnd;
> 
> -  EFI_PHYSICAL_ADDRESS        FreeMemory;
> 
> -  VOID                        *Hob;
> 
> -
> 
> -  HandOffHob = GetHobList ();
> 
> -
> 
> -  HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
> 
> -
> 
> -  FreeMemory = HandOffHob->EfiFreeMemoryTop - HandOffHob-
> >EfiFreeMemoryBottom;
> 
> -
> 
> -  if (FreeMemory < HobLength) {
> 
> -    return NULL;
> 
> -  }
> 
> -
> 
> -  Hob                                        = (VOID *)(UINTN)HandOffHob->EfiEndOfHobList;
> 
> -  ((EFI_HOB_GENERIC_HEADER *)Hob)->HobType   = HobType;
> 
> -  ((EFI_HOB_GENERIC_HEADER *)Hob)->HobLength = HobLength;
> 
> -  ((EFI_HOB_GENERIC_HEADER *)Hob)->Reserved  = 0;
> 
> -
> 
> -  HobEnd                      = (EFI_HOB_GENERIC_HEADER *)((UINTN)Hob +
> HobLength);
> 
> -  HandOffHob->EfiEndOfHobList = (EFI_PHYSICAL_ADDRESS)(UINTN)HobEnd;
> 
> -
> 
> -  HobEnd->HobType   = EFI_HOB_TYPE_END_OF_HOB_LIST;
> 
> -  HobEnd->HobLength = sizeof (EFI_HOB_GENERIC_HEADER);
> 
> -  HobEnd->Reserved  = 0;
> 
> -  HobEnd++;
> 
> -  HandOffHob->EfiFreeMemoryBottom =
> (EFI_PHYSICAL_ADDRESS)(UINTN)HobEnd;
> 
> -
> 
> -  return Hob;
> 
> -}
> 
> -
> 
>  /**
> 
>    Builds a HOB for a loaded PE32 module.
> 
> 
> 
>    This function builds a HOB for a loaded PE32 module.
> 
> +  It can only be invoked by Standalone MM Core.
> 
> +  For Standalone MM drivers, it will ASSERT() since HOB is read only for
> Standalone MM drivers.
> 
> +
> 
>    If ModuleName is NULL, then ASSERT().
> 
>    If there is no additional space for HOB creation, then ASSERT().
> 
> 
> 
> @@ -310,33 +275,19 @@ BuildModuleHob (
>    IN EFI_PHYSICAL_ADDRESS  EntryPoint
> 
>    )
> 
>  {
> 
> -  EFI_HOB_MEMORY_ALLOCATION_MODULE  *Hob;
> 
> -
> 
> -  ASSERT (
> 
> -    ((MemoryAllocationModule & (EFI_PAGE_SIZE - 1)) == 0) &&
> 
> -    ((ModuleLength & (EFI_PAGE_SIZE - 1)) == 0)
> 
> -    );
> 
> -
> 
> -  Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof
> (EFI_HOB_MEMORY_ALLOCATION_MODULE));
> 
> -
> 
> -  CopyGuid (&(Hob->MemoryAllocationHeader.Name),
> &gEfiHobMemoryAllocModuleGuid);
> 
> -  Hob->MemoryAllocationHeader.MemoryBaseAddress =
> MemoryAllocationModule;
> 
> -  Hob->MemoryAllocationHeader.MemoryLength      = ModuleLength;
> 
> -  Hob->MemoryAllocationHeader.MemoryType        = EfiBootServicesCode;
> 
> -
> 
>    //
> 
> -  // Zero the reserved space to match HOB spec
> 
> +  // HOB is read only for Standalone MM drivers
> 
>    //
> 
> -  ZeroMem (Hob->MemoryAllocationHeader.Reserved, sizeof (Hob-
> >MemoryAllocationHeader.Reserved));
> 
> -
> 
> -  CopyGuid (&Hob->ModuleName, ModuleName);
> 
> -  Hob->EntryPoint = EntryPoint;
> 
> +  ASSERT (FALSE);
> 
>  }
> 
> 
> 
>  /**
> 
>    Builds a HOB that describes a chunk of system memory.
> 
> 
> 
>    This function builds a HOB that describes a chunk of system memory.
> 
> +  It can only be invoked by Standalone MM Core.
> 
> +  For Standalone MM drivers, it will ASSERT() since HOB is read only for
> Standalone MM drivers.
> 
> +
> 
>    If there is no additional space for HOB creation, then ASSERT().
> 
> 
> 
>    @param  ResourceType        The type of resource described by this HOB.
> 
> @@ -354,15 +305,10 @@ BuildResourceDescriptorHob (
>    IN UINT64                       NumberOfBytes
> 
>    )
> 
>  {
> 
> -  EFI_HOB_RESOURCE_DESCRIPTOR  *Hob;
> 
> -
> 
> -  Hob = CreateHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, sizeof
> (EFI_HOB_RESOURCE_DESCRIPTOR));
> 
> -  ASSERT (Hob != NULL);
> 
> -
> 
> -  Hob->ResourceType      = ResourceType;
> 
> -  Hob->ResourceAttribute = ResourceAttribute;
> 
> -  Hob->PhysicalStart     = PhysicalStart;
> 
> -  Hob->ResourceLength    = NumberOfBytes;
> 
> +  //
> 
> +  // HOB is read only for Standalone MM drivers
> 
> +  //
> 
> +  ASSERT (FALSE);
> 
>  }
> 
> 
> 
>  /**
> 
> @@ -371,6 +317,9 @@ BuildResourceDescriptorHob (
>    This function builds a customized HOB tagged with a GUID for identification
> 
>    and returns the start address of GUID HOB data so that caller can fill the
> customized data.
> 
>    The HOB Header and Name field is already stripped.
> 
> +  It can only be invoked by Standalone MM Core.
> 
> +  For Standalone MM drivers, it will ASSERT() since HOB is read only for
> Standalone MM drivers.
> 
> +
> 
>    If Guid is NULL, then ASSERT().
> 
>    If there is no additional space for HOB creation, then ASSERT().
> 
>    If DataLength >= (0x10000 - sizeof (EFI_HOB_GUID_TYPE)), then ASSERT().
> 
> @@ -388,16 +337,11 @@ BuildGuidHob (
>    IN UINTN           DataLength
> 
>    )
> 
>  {
> 
> -  EFI_HOB_GUID_TYPE  *Hob;
> 
> -
> 
>    //
> 
> -  // Make sure that data length is not too long.
> 
> +  // HOB is read only for Standalone MM drivers
> 
>    //
> 
> -  ASSERT (DataLength <= (0xffff - sizeof (EFI_HOB_GUID_TYPE)));
> 
> -
> 
> -  Hob = CreateHob (EFI_HOB_TYPE_GUID_EXTENSION, (UINT16)(sizeof
> (EFI_HOB_GUID_TYPE) + DataLength));
> 
> -  CopyGuid (&Hob->Name, Guid);
> 
> -  return Hob + 1;
> 
> +  ASSERT (FALSE);
> 
> +  return NULL;
> 
>  }
> 
> 
> 
>  /**
> 
> @@ -406,6 +350,9 @@ BuildGuidHob (
>    This function builds a customized HOB tagged with a GUID for identification,
> 
>    copies the input data to the HOB data field and returns the start address of
> the GUID HOB data.
> 
>    The HOB Header and Name field is already stripped.
> 
> +  It can only be invoked by Standalone MM Core.
> 
> +  For Standalone MM drivers, it will ASSERT() since HOB is read only for
> Standalone MM drivers.
> 
> +
> 
>    If Guid is NULL, then ASSERT().
> 
>    If Data is NULL and DataLength > 0, then ASSERT().
> 
>    If there is no additional space for HOB creation, then ASSERT().
> 
> @@ -426,19 +373,20 @@ BuildGuidDataHob (
>    IN UINTN           DataLength
> 
>    )
> 
>  {
> 
> -  VOID  *HobData;
> 
> -
> 
> -  ASSERT (Data != NULL || DataLength == 0);
> 
> -
> 
> -  HobData = BuildGuidHob (Guid, DataLength);
> 
> -
> 
> -  return CopyMem (HobData, Data, DataLength);
> 
> +  //
> 
> +  // HOB is read only for Standalone MM drivers
> 
> +  //
> 
> +  ASSERT (FALSE);
> 
> +  return NULL;
> 
>  }
> 
> 
> 
>  /**
> 
>    Builds a Firmware Volume HOB.
> 
> 
> 
>    This function builds a Firmware Volume HOB.
> 
> +  It can only be invoked by Standalone MM Core.
> 
> +  For Standalone MM drivers, it will ASSERT() since HOB is read only for
> Standalone MM drivers.
> 
> +
> 
>    If there is no additional space for HOB creation, then ASSERT().
> 
> 
> 
>    @param  BaseAddress   The base address of the Firmware Volume.
> 
> @@ -452,18 +400,19 @@ BuildFvHob (
>    IN UINT64                Length
> 
>    )
> 
>  {
> 
> -  EFI_HOB_FIRMWARE_VOLUME  *Hob;
> 
> -
> 
> -  Hob = CreateHob (EFI_HOB_TYPE_FV, sizeof
> (EFI_HOB_FIRMWARE_VOLUME));
> 
> -
> 
> -  Hob->BaseAddress = BaseAddress;
> 
> -  Hob->Length      = Length;
> 
> +  //
> 
> +  // HOB is read only for Standalone MM drivers
> 
> +  //
> 
> +  ASSERT (FALSE);
> 
>  }
> 
> 
> 
>  /**
> 
>    Builds a EFI_HOB_TYPE_FV2 HOB.
> 
> 
> 
>    This function builds a EFI_HOB_TYPE_FV2 HOB.
> 
> +  It can only be invoked by Standalone MM Core.
> 
> +  For Standalone MM drivers, it will ASSERT() since HOB is read only for
> Standalone MM drivers.
> 
> +
> 
>    If there is no additional space for HOB creation, then ASSERT().
> 
> 
> 
>    @param  BaseAddress   The base address of the Firmware Volume.
> 
> @@ -481,20 +430,19 @@ BuildFv2Hob (
>    IN CONST    EFI_GUID              *FileName
> 
>    )
> 
>  {
> 
> -  EFI_HOB_FIRMWARE_VOLUME2  *Hob;
> 
> -
> 
> -  Hob = CreateHob (EFI_HOB_TYPE_FV2, sizeof
> (EFI_HOB_FIRMWARE_VOLUME2));
> 
> -
> 
> -  Hob->BaseAddress = BaseAddress;
> 
> -  Hob->Length      = Length;
> 
> -  CopyGuid (&Hob->FvName, FvName);
> 
> -  CopyGuid (&Hob->FileName, FileName);
> 
> +  //
> 
> +  // HOB is read only for Standalone MM drivers
> 
> +  //
> 
> +  ASSERT (FALSE);
> 
>  }
> 
> 
> 
>  /**
> 
>    Builds a HOB for the CPU.
> 
> 
> 
>    This function builds a HOB for the CPU.
> 
> +  It can only be invoked by Standalone MM Core.
> 
> +  For Standalone MM drivers, it will ASSERT() since HOB is read only for
> Standalone MM drivers.
> 
> +
> 
>    If there is no additional space for HOB creation, then ASSERT().
> 
> 
> 
>    @param  SizeOfMemorySpace   The maximum physical memory
> addressability of the processor.
> 
> @@ -508,23 +456,19 @@ BuildCpuHob (
>    IN UINT8  SizeOfIoSpace
> 
>    )
> 
>  {
> 
> -  EFI_HOB_CPU  *Hob;
> 
> -
> 
> -  Hob = CreateHob (EFI_HOB_TYPE_CPU, sizeof (EFI_HOB_CPU));
> 
> -
> 
> -  Hob->SizeOfMemorySpace = SizeOfMemorySpace;
> 
> -  Hob->SizeOfIoSpace     = SizeOfIoSpace;
> 
> -
> 
>    //
> 
> -  // Zero the reserved space to match HOB spec
> 
> +  // HOB is read only for Standalone MM drivers
> 
>    //
> 
> -  ZeroMem (Hob->Reserved, sizeof (Hob->Reserved));
> 
> +  ASSERT (FALSE);
> 
>  }
> 
> 
> 
>  /**
> 
>    Builds a HOB for the memory allocation.
> 
> 
> 
>    This function builds a HOB for the memory allocation.
> 
> +  It can only be invoked by Standalone MM Core.
> 
> +  For Standalone MM drivers, it will ASSERT() since HOB is read only for
> Standalone MM drivers.
> 
> +
> 
>    If there is no additional space for HOB creation, then ASSERT().
> 
> 
> 
>    @param  BaseAddress   The 64 bit physical address of the memory.
> 
> @@ -540,23 +484,10 @@ BuildMemoryAllocationHob (
>    IN EFI_MEMORY_TYPE       MemoryType
> 
>    )
> 
>  {
> 
> -  EFI_HOB_MEMORY_ALLOCATION  *Hob;
> 
> -
> 
> -  ASSERT (
> 
> -    ((BaseAddress & (EFI_PAGE_SIZE - 1)) == 0) &&
> 
> -    ((Length & (EFI_PAGE_SIZE - 1)) == 0)
> 
> -    );
> 
> -
> 
> -  Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof
> (EFI_HOB_MEMORY_ALLOCATION));
> 
> -
> 
> -  ZeroMem (&(Hob->AllocDescriptor.Name), sizeof (EFI_GUID));
> 
> -  Hob->AllocDescriptor.MemoryBaseAddress = BaseAddress;
> 
> -  Hob->AllocDescriptor.MemoryLength      = Length;
> 
> -  Hob->AllocDescriptor.MemoryType        = MemoryType;
> 
>    //
> 
> -  // Zero the reserved space to match HOB spec
> 
> +  // HOB is read only for Standalone MM drivers
> 
>    //
> 
> -  ZeroMem (Hob->AllocDescriptor.Reserved, sizeof (Hob-
> >AllocDescriptor.Reserved));
> 
> +  ASSERT (FALSE);
> 
>  }
> 
> 
> 
>  /**
> 
> --
> 2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112248): https://edk2.groups.io/g/devel/message/112248
Mute This Topic: https://groups.io/mt/103018869/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-