Hi folks, returning to this thread because I noticed that HOB
creation still exists in StandaloneMmCore for ARM:

https://github.com/tianocore/edk2/blob/5d4c5253e8bbc0baa8837fcd868925212df85201/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c

As far as I can tell, there is only this one file that creates 6
HOBs from StandaloneMmCore. Per our earlier discussion that led to
disabling HOB creation in StandaloneMm, I think that this falls into
the case where StandaloneMm is a HOB consumer phase, not a producer
phase and so it should not be creating these HOBs. On the x64 side,
all of the StandaloneMm HOB creation functions ASSERT with the
comment that StandaloneMm is a HOB consumer phase and should not
be creating HOBs.

On ARM this is more complicated, as all of this information would
seem to originate from TF-A and so we would need TF-A to produce
these HOBs to tell StandaloneMm the information it needs to
operate. Today TF-A already has to communicate this information, the
HOBs are just created in StandaloneMmCore instead of in TF-A.

Curious to get other folks thoughts here on this paradigm.

Thanks,
Oliver

On 12/5/2023 5:47 AM, Nhi Pham wrote:
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+tianoc...@kernel.org>
Cc: Ray Ni <ray...@intel.com>
Cc: Sami Mujawar <sami.muja...@arm.com>
Cc: Oliver Smith-Denny <o...@linux.microsoft.com>
Signed-off-by: Nhi Pham <nhipham...@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);

  }

  /**



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118482): https://edk2.groups.io/g/devel/message/118482
Mute This Topic: https://groups.io/mt/103018869/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to