From: "Ni, Ray" <ray.ni@intel.com>
To: Nhi Pham <nhiphambka@gmail.com>,
"devel@edk2.groups.io" <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: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation
Date: Mon, 11 Dec 2023 05:49:17 +0000 [thread overview]
Message-ID: <MN6PR11MB8244F5935BF898B015A90E7D8C8FA@MN6PR11MB8244.namprd11.prod.outlook.com> (raw)
In-Reply-To: <MN6PR11MB8244CE3F8F1D2275DD6135068C89A@MN6PR11MB8244.namprd11.prod.outlook.com>
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]
-=-=-=-=-=-=-=-=-=-=-=-
next prev parent reply other threads:[~2023-12-11 5:49 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MN6PR11MB8244F5935BF898B015A90E7D8C8FA@MN6PR11MB8244.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox