From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id 94B38AC1A2D for ; Wed, 1 May 2024 21:31:53 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=ddMSZyxXRAbaLdZFbWOuwrdhF4jpGmcwWPh7W+3D/c4=; c=relaxed/simple; d=groups.io; h=DKIM-Filter:Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20240206; t=1714599112; v=1; b=GBMuhVy2Drbho2LZFwlAcrr4LICnGvmfvPNgBjfjVcZ1mEMAlWhW5qHAml6DIXjTRJm7PWOh i52jKmv7nF+ErEoUPyu+LqCAjbk+4OAh2Eo1fpCr40YehvaMMOJ7JTBLk8bRDI1pINNrlYadY3J IyJTZN8xocTvWQ/NczMp3zXdGAN3ImrIma4IvmfP2qFtyCjdAteczZhzqGDKUclQsAi2hmO3di1 U6Uo+u7WF4HlDtVKS5nHqiNwLAShsK5ZSWEwtMZIPDE1fWIc4jr8sg4a26MkKwMR1qkHaw6960G P7z1POx20LIW5bpnbI0Oc8tuqtK4o72hHUSCCHO/03qUw== X-Received: by 127.0.0.2 with SMTP id TGigYY7687511xuuWQioIV4T; Wed, 01 May 2024 14:31:52 -0700 X-Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web10.9018.1714599111102460749 for ; Wed, 01 May 2024 14:31:51 -0700 X-Received: from [10.137.194.171] (unknown [131.107.159.43]) by linux.microsoft.com (Postfix) with ESMTPSA id A449B206B4D7; Wed, 1 May 2024 14:31:50 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com A449B206B4D7 Message-ID: <0d99c3c9-9dd0-494a-93b1-e9270914162b@linux.microsoft.com> Date: Wed, 1 May 2024 14:31:50 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation To: devel@edk2.groups.io, nhiphambka@gmail.com Cc: Ard Biesheuvel , Ray Ni , Sami Mujawar , Kun Qin References: <20231205134757.14692-1-nhiphambka@gmail.com> From: "Oliver Smith-Denny" In-Reply-To: <20231205134757.14692-1-nhiphambka@gmail.com> Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Wed, 01 May 2024 14:31:51 -0700 Resent-From: osde@linux.microsoft.com Reply-To: devel@edk2.groups.io,osde@linux.microsoft.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: rue6mgv4mpejKuBY2HNWYuZCx7686176AA= Content-Language: en-CA Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=GBMuhVy2; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=linux.microsoft.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io Hi folks, returning to this thread because I noticed that HOB creation still exists in StandaloneMmCore for ARM: https://github.com/tianocore/edk2/blob/5d4c5253e8bbc0baa8837fcd868925212df8= 5201/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. >=20 > [1] https://edk2.groups.io/g/devel/message/108333 >=20 > Cc: Ard Biesheuvel > Cc: Ray Ni > Cc: Sami Mujawar > Cc: Oliver Smith-Denny > Signed-off-by: Nhi Pham > --- > StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c | 171 += +++++-------------- > 1 file changed, 51 insertions(+), 120 deletions(-) >=20 > diff --git a/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLi= b.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 >=20 > - HOB Library implementation for Standalone MM Core. >=20 > + HOB Library implementation for Standalone MM modules. >=20 > =20 >=20 > Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
>=20 > Copyright (c) 2017 - 2018, ARM Limited. All rights reserved.
>=20 > @@ -250,48 +250,13 @@ GetBootModeHob ( > return HandOffHob->BootMode; >=20 > } >=20 > =20 >=20 > -VOID * >=20 > -CreateHob ( >=20 > - IN UINT16 HobType, >=20 > - IN UINT16 HobLength >=20 > - ) >=20 > -{ >=20 > - EFI_HOB_HANDOFF_INFO_TABLE *HandOffHob; >=20 > - EFI_HOB_GENERIC_HEADER *HobEnd; >=20 > - EFI_PHYSICAL_ADDRESS FreeMemory; >=20 > - VOID *Hob; >=20 > - >=20 > - HandOffHob =3D GetHobList (); >=20 > - >=20 > - HobLength =3D (UINT16)((HobLength + 0x7) & (~0x7)); >=20 > - >=20 > - FreeMemory =3D HandOffHob->EfiFreeMemoryTop - HandOffHob->EfiFreeMemor= yBottom; >=20 > - >=20 > - if (FreeMemory < HobLength) { >=20 > - return NULL; >=20 > - } >=20 > - >=20 > - Hob =3D (VOID *)(UINTN)HandOffH= ob->EfiEndOfHobList; >=20 > - ((EFI_HOB_GENERIC_HEADER *)Hob)->HobType =3D HobType; >=20 > - ((EFI_HOB_GENERIC_HEADER *)Hob)->HobLength =3D HobLength; >=20 > - ((EFI_HOB_GENERIC_HEADER *)Hob)->Reserved =3D 0; >=20 > - >=20 > - HobEnd =3D (EFI_HOB_GENERIC_HEADER *)((UINTN)Hob = + HobLength); >=20 > - HandOffHob->EfiEndOfHobList =3D (EFI_PHYSICAL_ADDRESS)(UINTN)HobEnd; >=20 > - >=20 > - HobEnd->HobType =3D EFI_HOB_TYPE_END_OF_HOB_LIST; >=20 > - HobEnd->HobLength =3D sizeof (EFI_HOB_GENERIC_HEADER); >=20 > - HobEnd->Reserved =3D 0; >=20 > - HobEnd++; >=20 > - HandOffHob->EfiFreeMemoryBottom =3D (EFI_PHYSICAL_ADDRESS)(UINTN)HobEn= d; >=20 > - >=20 > - return Hob; >=20 > -} >=20 > - >=20 > /** >=20 > Builds a HOB for a loaded PE32 module. >=20 > =20 >=20 > This function builds a HOB for a loaded PE32 module. >=20 > + It can only be invoked by Standalone MM Core. >=20 > + For Standalone MM drivers, it will ASSERT() since HOB is read only for= Standalone MM drivers. >=20 > + >=20 > If ModuleName is NULL, then ASSERT(). >=20 > If there is no additional space for HOB creation, then ASSERT(). >=20 > =20 >=20 > @@ -310,33 +275,19 @@ BuildModuleHob ( > IN EFI_PHYSICAL_ADDRESS EntryPoint >=20 > ) >=20 > { >=20 > - EFI_HOB_MEMORY_ALLOCATION_MODULE *Hob; >=20 > - >=20 > - ASSERT ( >=20 > - ((MemoryAllocationModule & (EFI_PAGE_SIZE - 1)) =3D=3D 0) && >=20 > - ((ModuleLength & (EFI_PAGE_SIZE - 1)) =3D=3D 0) >=20 > - ); >=20 > - >=20 > - Hob =3D CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEM= ORY_ALLOCATION_MODULE)); >=20 > - >=20 > - CopyGuid (&(Hob->MemoryAllocationHeader.Name), &gEfiHobMemoryAllocModu= leGuid); >=20 > - Hob->MemoryAllocationHeader.MemoryBaseAddress =3D MemoryAllocationModu= le; >=20 > - Hob->MemoryAllocationHeader.MemoryLength =3D ModuleLength; >=20 > - Hob->MemoryAllocationHeader.MemoryType =3D EfiBootServicesCode; >=20 > - >=20 > // >=20 > - // Zero the reserved space to match HOB spec >=20 > + // HOB is read only for Standalone MM drivers >=20 > // >=20 > - ZeroMem (Hob->MemoryAllocationHeader.Reserved, sizeof (Hob->MemoryAllo= cationHeader.Reserved)); >=20 > - >=20 > - CopyGuid (&Hob->ModuleName, ModuleName); >=20 > - Hob->EntryPoint =3D EntryPoint; >=20 > + ASSERT (FALSE); >=20 > } >=20 > =20 >=20 > /** >=20 > Builds a HOB that describes a chunk of system memory. >=20 > =20 >=20 > This function builds a HOB that describes a chunk of system memory. >=20 > + It can only be invoked by Standalone MM Core. >=20 > + For Standalone MM drivers, it will ASSERT() since HOB is read only for= Standalone MM drivers. >=20 > + >=20 > If there is no additional space for HOB creation, then ASSERT(). >=20 > =20 >=20 > @param ResourceType The type of resource described by this HO= B. >=20 > @@ -354,15 +305,10 @@ BuildResourceDescriptorHob ( > IN UINT64 NumberOfBytes >=20 > ) >=20 > { >=20 > - EFI_HOB_RESOURCE_DESCRIPTOR *Hob; >=20 > - >=20 > - Hob =3D CreateHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, sizeof (EFI_HOB_R= ESOURCE_DESCRIPTOR)); >=20 > - ASSERT (Hob !=3D NULL); >=20 > - >=20 > - Hob->ResourceType =3D ResourceType; >=20 > - Hob->ResourceAttribute =3D ResourceAttribute; >=20 > - Hob->PhysicalStart =3D PhysicalStart; >=20 > - Hob->ResourceLength =3D NumberOfBytes; >=20 > + // >=20 > + // HOB is read only for Standalone MM drivers >=20 > + // >=20 > + ASSERT (FALSE); >=20 > } >=20 > =20 >=20 > /** >=20 > @@ -371,6 +317,9 @@ BuildResourceDescriptorHob ( > This function builds a customized HOB tagged with a GUID for identifi= cation >=20 > and returns the start address of GUID HOB data so that caller can fil= l the customized data. >=20 > The HOB Header and Name field is already stripped. >=20 > + It can only be invoked by Standalone MM Core. >=20 > + For Standalone MM drivers, it will ASSERT() since HOB is read only for= Standalone MM drivers. >=20 > + >=20 > If Guid is NULL, then ASSERT(). >=20 > If there is no additional space for HOB creation, then ASSERT(). >=20 > If DataLength >=3D (0x10000 - sizeof (EFI_HOB_GUID_TYPE)), then ASSER= T(). >=20 > @@ -388,16 +337,11 @@ BuildGuidHob ( > IN UINTN DataLength >=20 > ) >=20 > { >=20 > - EFI_HOB_GUID_TYPE *Hob; >=20 > - >=20 > // >=20 > - // Make sure that data length is not too long. >=20 > + // HOB is read only for Standalone MM drivers >=20 > // >=20 > - ASSERT (DataLength <=3D (0xffff - sizeof (EFI_HOB_GUID_TYPE))); >=20 > - >=20 > - Hob =3D CreateHob (EFI_HOB_TYPE_GUID_EXTENSION, (UINT16)(sizeof (EFI_H= OB_GUID_TYPE) + DataLength)); >=20 > - CopyGuid (&Hob->Name, Guid); >=20 > - return Hob + 1; >=20 > + ASSERT (FALSE); >=20 > + return NULL; >=20 > } >=20 > =20 >=20 > /** >=20 > @@ -406,6 +350,9 @@ BuildGuidHob ( > This function builds a customized HOB tagged with a GUID for identifi= cation, >=20 > copies the input data to the HOB data field and returns the start add= ress of the GUID HOB data. >=20 > The HOB Header and Name field is already stripped. >=20 > + It can only be invoked by Standalone MM Core. >=20 > + For Standalone MM drivers, it will ASSERT() since HOB is read only for= Standalone MM drivers. >=20 > + >=20 > If Guid is NULL, then ASSERT(). >=20 > If Data is NULL and DataLength > 0, then ASSERT(). >=20 > If there is no additional space for HOB creation, then ASSERT(). >=20 > @@ -426,19 +373,20 @@ BuildGuidDataHob ( > IN UINTN DataLength >=20 > ) >=20 > { >=20 > - VOID *HobData; >=20 > - >=20 > - ASSERT (Data !=3D NULL || DataLength =3D=3D 0); >=20 > - >=20 > - HobData =3D BuildGuidHob (Guid, DataLength); >=20 > - >=20 > - return CopyMem (HobData, Data, DataLength); >=20 > + // >=20 > + // HOB is read only for Standalone MM drivers >=20 > + // >=20 > + ASSERT (FALSE); >=20 > + return NULL; >=20 > } >=20 > =20 >=20 > /** >=20 > Builds a Firmware Volume HOB. >=20 > =20 >=20 > This function builds a Firmware Volume HOB. >=20 > + It can only be invoked by Standalone MM Core. >=20 > + For Standalone MM drivers, it will ASSERT() since HOB is read only for= Standalone MM drivers. >=20 > + >=20 > If there is no additional space for HOB creation, then ASSERT(). >=20 > =20 >=20 > @param BaseAddress The base address of the Firmware Volume. >=20 > @@ -452,18 +400,19 @@ BuildFvHob ( > IN UINT64 Length >=20 > ) >=20 > { >=20 > - EFI_HOB_FIRMWARE_VOLUME *Hob; >=20 > - >=20 > - Hob =3D CreateHob (EFI_HOB_TYPE_FV, sizeof (EFI_HOB_FIRMWARE_VOLUME)); >=20 > - >=20 > - Hob->BaseAddress =3D BaseAddress; >=20 > - Hob->Length =3D Length; >=20 > + // >=20 > + // HOB is read only for Standalone MM drivers >=20 > + // >=20 > + ASSERT (FALSE); >=20 > } >=20 > =20 >=20 > /** >=20 > Builds a EFI_HOB_TYPE_FV2 HOB. >=20 > =20 >=20 > This function builds a EFI_HOB_TYPE_FV2 HOB. >=20 > + It can only be invoked by Standalone MM Core. >=20 > + For Standalone MM drivers, it will ASSERT() since HOB is read only for= Standalone MM drivers. >=20 > + >=20 > If there is no additional space for HOB creation, then ASSERT(). >=20 > =20 >=20 > @param BaseAddress The base address of the Firmware Volume. >=20 > @@ -481,20 +430,19 @@ BuildFv2Hob ( > IN CONST EFI_GUID *FileName >=20 > ) >=20 > { >=20 > - EFI_HOB_FIRMWARE_VOLUME2 *Hob; >=20 > - >=20 > - Hob =3D CreateHob (EFI_HOB_TYPE_FV2, sizeof (EFI_HOB_FIRMWARE_VOLUME2)= ); >=20 > - >=20 > - Hob->BaseAddress =3D BaseAddress; >=20 > - Hob->Length =3D Length; >=20 > - CopyGuid (&Hob->FvName, FvName); >=20 > - CopyGuid (&Hob->FileName, FileName); >=20 > + // >=20 > + // HOB is read only for Standalone MM drivers >=20 > + // >=20 > + ASSERT (FALSE); >=20 > } >=20 > =20 >=20 > /** >=20 > Builds a HOB for the CPU. >=20 > =20 >=20 > This function builds a HOB for the CPU. >=20 > + It can only be invoked by Standalone MM Core. >=20 > + For Standalone MM drivers, it will ASSERT() since HOB is read only for= Standalone MM drivers. >=20 > + >=20 > If there is no additional space for HOB creation, then ASSERT(). >=20 > =20 >=20 > @param SizeOfMemorySpace The maximum physical memory addressabilit= y of the processor. >=20 > @@ -508,23 +456,19 @@ BuildCpuHob ( > IN UINT8 SizeOfIoSpace >=20 > ) >=20 > { >=20 > - EFI_HOB_CPU *Hob; >=20 > - >=20 > - Hob =3D CreateHob (EFI_HOB_TYPE_CPU, sizeof (EFI_HOB_CPU)); >=20 > - >=20 > - Hob->SizeOfMemorySpace =3D SizeOfMemorySpace; >=20 > - Hob->SizeOfIoSpace =3D SizeOfIoSpace; >=20 > - >=20 > // >=20 > - // Zero the reserved space to match HOB spec >=20 > + // HOB is read only for Standalone MM drivers >=20 > // >=20 > - ZeroMem (Hob->Reserved, sizeof (Hob->Reserved)); >=20 > + ASSERT (FALSE); >=20 > } >=20 > =20 >=20 > /** >=20 > Builds a HOB for the memory allocation. >=20 > =20 >=20 > This function builds a HOB for the memory allocation. >=20 > + It can only be invoked by Standalone MM Core. >=20 > + For Standalone MM drivers, it will ASSERT() since HOB is read only for= Standalone MM drivers. >=20 > + >=20 > If there is no additional space for HOB creation, then ASSERT(). >=20 > =20 >=20 > @param BaseAddress The 64 bit physical address of the memory. >=20 > @@ -540,23 +484,10 @@ BuildMemoryAllocationHob ( > IN EFI_MEMORY_TYPE MemoryType >=20 > ) >=20 > { >=20 > - EFI_HOB_MEMORY_ALLOCATION *Hob; >=20 > - >=20 > - ASSERT ( >=20 > - ((BaseAddress & (EFI_PAGE_SIZE - 1)) =3D=3D 0) && >=20 > - ((Length & (EFI_PAGE_SIZE - 1)) =3D=3D 0) >=20 > - ); >=20 > - >=20 > - Hob =3D CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEM= ORY_ALLOCATION)); >=20 > - >=20 > - ZeroMem (&(Hob->AllocDescriptor.Name), sizeof (EFI_GUID)); >=20 > - Hob->AllocDescriptor.MemoryBaseAddress =3D BaseAddress; >=20 > - Hob->AllocDescriptor.MemoryLength =3D Length; >=20 > - Hob->AllocDescriptor.MemoryType =3D MemoryType; >=20 > // >=20 > - // Zero the reserved space to match HOB spec >=20 > + // HOB is read only for Standalone MM drivers >=20 > // >=20 > - ZeroMem (Hob->AllocDescriptor.Reserved, sizeof (Hob->AllocDescriptor.R= eserved)); >=20 > + ASSERT (FALSE); >=20 > } >=20 > =20 >=20 > /** >=20 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- 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] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-