* [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation @ 2023-12-05 13:47 Nhi Pham 2023-12-09 1:27 ` Ni, Ray 2024-05-01 21:31 ` Oliver Smith-Denny 0 siblings, 2 replies; 8+ messages in thread From: Nhi Pham @ 2023-12-05 13:47 UTC (permalink / raw) To: devel; +Cc: Nhi Pham, Ard Biesheuvel, Ray Ni, Sami Mujawar, Oliver Smith-Denny 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 (#112136): https://edk2.groups.io/g/devel/message/112136 Mute This Topic: https://groups.io/mt/103018869/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation 2023-12-05 13:47 [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation Nhi Pham @ 2023-12-09 1:27 ` Ni, Ray 2023-12-11 5:49 ` Ni, Ray 2024-05-01 21:31 ` Oliver Smith-Denny 1 sibling, 1 reply; 8+ messages in thread From: Ni, Ray @ 2023-12-09 1:27 UTC (permalink / raw) To: Nhi Pham, devel@edk2.groups.io Cc: Ard Biesheuvel, Sami Mujawar, Oliver Smith-Denny 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] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation 2023-12-09 1:27 ` Ni, Ray @ 2023-12-11 5:49 ` Ni, Ray 0 siblings, 0 replies; 8+ messages in thread From: Ni, Ray @ 2023-12-11 5:49 UTC (permalink / raw) To: Nhi Pham, devel@edk2.groups.io Cc: Ard Biesheuvel, Sami Mujawar, Oliver Smith-Denny Merged at bb13a4adab Thanks, Ray > -----Original Message----- > From: Ni, Ray > Sent: Saturday, December 9, 2023 9:27 AM > To: Nhi Pham <nhiphambka@gmail.com>; devel@edk2.groups.io > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar > <sami.mujawar@arm.com>; Oliver Smith-Denny <osde@linux.microsoft.com> > Subject: RE: [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: > Remove HOB creation > > 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 (#112268): https://edk2.groups.io/g/devel/message/112268 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] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation 2023-12-05 13:47 [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation Nhi Pham 2023-12-09 1:27 ` Ni, Ray @ 2024-05-01 21:31 ` Oliver Smith-Denny 2024-05-02 14:52 ` Sami Mujawar 2024-05-03 2:28 ` Nhi Pham via groups.io 1 sibling, 2 replies; 8+ messages in thread From: Oliver Smith-Denny @ 2024-05-01 21:31 UTC (permalink / raw) To: devel, nhiphambka; +Cc: Ard Biesheuvel, Ray Ni, Sami Mujawar, Kun Qin 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+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); > > } > > > > /** > -=-=-=-=-=-=-=-=-=-=-=- 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/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation 2024-05-01 21:31 ` Oliver Smith-Denny @ 2024-05-02 14:52 ` Sami Mujawar 2024-05-02 17:02 ` Oliver Smith-Denny 2024-05-03 2:28 ` Nhi Pham via groups.io 1 sibling, 1 reply; 8+ messages in thread From: Sami Mujawar @ 2024-05-02 14:52 UTC (permalink / raw) To: Oliver Smith-Denny, devel@edk2.groups.io, nhiphambka@gmail.com, Yeo Reum Yun Cc: Ard Biesheuvel, Ray Ni, Kun Qin Hi Oliver, We are working on a solution to remove the HOB creation logic from StandaloneMm for Arm, and this involves implementing the Firmware handoff specification (https://github.com/FirmwareHandoff/firmware_handoff/releases/download/v0.9/firmware_handoff.pdf). As you rightly mentioned this also requires changes in TF-A, and this work is in progress. Levi (Yeo) is currently working on this feature and will post the patches to the mailing list once we have the necessary components ready. Regards, Sami Mujawar On 01/05/2024, 22:32, "Oliver Smith-Denny" <osde@linux.microsoft.com <mailto:osde@linux.microsoft.com>> wrote: 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 <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 <https://edk2.groups.io/g/devel/message/108333> > > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org <mailto:ardb+tianocore@kernel.org>> > Cc: Ray Ni <ray.ni@intel.com <mailto:ray.ni@intel.com>> > Cc: Sami Mujawar <sami.mujawar@arm.com <mailto:sami.mujawar@arm.com>> > Cc: Oliver Smith-Denny <osde@linux.microsoft.com <mailto:osde@linux.microsoft.com>> > Signed-off-by: Nhi Pham <nhiphambka@gmail.com <mailto: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); > > } > > > > /** > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118519): https://edk2.groups.io/g/devel/message/118519 Mute This Topic: https://groups.io/mt/103018869/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation 2024-05-02 14:52 ` Sami Mujawar @ 2024-05-02 17:02 ` Oliver Smith-Denny 0 siblings, 0 replies; 8+ messages in thread From: Oliver Smith-Denny @ 2024-05-02 17:02 UTC (permalink / raw) To: Sami Mujawar, devel@edk2.groups.io, nhiphambka@gmail.com, Yeo Reum Yun Cc: Ard Biesheuvel, Ray Ni, Kun Qin Hi Sami, Great! It's always a good day when the thing you are looking at is already being worked on :). Looking forward to seeing the patches. Thanks, Oliver On 5/2/2024 7:52 AM, Sami Mujawar wrote: > Hi Oliver, > > We are working on a solution to remove the HOB creation logic from StandaloneMm for Arm, and this involves implementing the Firmware handoff specification (https://github.com/FirmwareHandoff/firmware_handoff/releases/download/v0.9/firmware_handoff.pdf). > > As you rightly mentioned this also requires changes in TF-A, and this work is in progress. > > Levi (Yeo) is currently working on this feature and will post the patches to the mailing list once we have the necessary components ready. > > Regards, > > Sami Mujawar > > On 01/05/2024, 22:32, "Oliver Smith-Denny" <osde@linux.microsoft.com <mailto:osde@linux.microsoft.com>> wrote: > > > 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 <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 <https://edk2.groups.io/g/devel/message/108333> >> >> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org <mailto:ardb+tianocore@kernel.org>> >> Cc: Ray Ni <ray.ni@intel.com <mailto:ray.ni@intel.com>> >> Cc: Sami Mujawar <sami.mujawar@arm.com <mailto:sami.mujawar@arm.com>> >> Cc: Oliver Smith-Denny <osde@linux.microsoft.com <mailto:osde@linux.microsoft.com>> >> Signed-off-by: Nhi Pham <nhiphambka@gmail.com <mailto: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); >> >> } >> >> >> >> /** >> > > > > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118534): https://edk2.groups.io/g/devel/message/118534 Mute This Topic: https://groups.io/mt/103018869/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation 2024-05-01 21:31 ` Oliver Smith-Denny 2024-05-02 14:52 ` Sami Mujawar @ 2024-05-03 2:28 ` Nhi Pham via groups.io 2024-05-03 13:56 ` Oliver Smith-Denny 1 sibling, 1 reply; 8+ messages in thread From: Nhi Pham via groups.io @ 2024-05-03 2:28 UTC (permalink / raw) To: devel, osde, nhiphambka; +Cc: Ard Biesheuvel, Ray Ni, Sami Mujawar, Kun Qin On 5/2/2024 4:31 AM, Oliver Smith-Denny via groups.io wrote: > 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. The HOB creation in StandaloneMmCoreEntryPoint is exclusively consumed by the MM_CORE_STANDALONE module (StandaloneMmPkg/Core/StandaloneMmCore.inf), not a MM_STANDALONE module. Per my understanding earlier, the MM_CORE_STANDALONE is a producer and other MM_STANDALONE modules are consumers. So we removed the HOB creation in the StandaloneMmHobLib which is consumed by MM_STANDALONE modules. Also, we still retain the HOB creation in the StandaloneMmPkg/Library/StandaloneMmCoreHobLib library instance. Regards, Nhi > > 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+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); >> >> } >> >> >> /** >> > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118552): https://edk2.groups.io/g/devel/message/118552 Mute This Topic: https://groups.io/mt/103018869/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation 2024-05-03 2:28 ` Nhi Pham via groups.io @ 2024-05-03 13:56 ` Oliver Smith-Denny 0 siblings, 0 replies; 8+ messages in thread From: Oliver Smith-Denny @ 2024-05-03 13:56 UTC (permalink / raw) To: devel, nhi, nhiphambka; +Cc: Ard Biesheuvel, Ray Ni, Sami Mujawar, Kun Qin On 5/2/2024 7:28 PM, Nhi Pham via groups.io wrote: > On 5/2/2024 4:31 AM, Oliver Smith-Denny via groups.io wrote: >> 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. > > The HOB creation in StandaloneMmCoreEntryPoint is exclusively consumed > by the MM_CORE_STANDALONE module > (StandaloneMmPkg/Core/StandaloneMmCore.inf), not a MM_STANDALONE module. > Per my understanding earlier, the MM_CORE_STANDALONE is a producer and > other MM_STANDALONE modules are consumers. So we removed the HOB > creation in the StandaloneMmHobLib which is consumed by MM_STANDALONE > modules. > > Also, we still retain the HOB creation in the > StandaloneMmPkg/Library/StandaloneMmCoreHobLib library instance. > The PI spec is light on details on HOBs in Standalone MM, but it is clear that there are HOB producer phases and HOB consumer phases. Pre-Standalone MM, the HOB producer phase was PEI and the HOB consumer phase was DXE/SMM. In the Standalone MM world, it still follows that a boot phase is either a HOB consumer or a HOB producer. Currently, Standalone MM is both, regardless that it is the core that is producing the HOBs. I think that Levi's work to remove HOB creation in Standalone MM and move it to TF-A is the right move here. Thanks, Oliver -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118556): https://edk2.groups.io/g/devel/message/118556 Mute This Topic: https://groups.io/mt/103018869/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-05-03 13:56 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-12-05 13:47 [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation Nhi Pham 2023-12-09 1:27 ` Ni, Ray 2023-12-11 5:49 ` Ni, Ray 2024-05-01 21:31 ` Oliver Smith-Denny 2024-05-02 14:52 ` Sami Mujawar 2024-05-02 17:02 ` Oliver Smith-Denny 2024-05-03 2:28 ` Nhi Pham via groups.io 2024-05-03 13:56 ` Oliver Smith-Denny
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox