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 8230494144C for ; Thu, 2 May 2024 17:02:33 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=95miXKqJ21JR9g7g0FifsiEKTQJvB+QT6b3fWGKrNBs=; 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=1714669352; v=1; b=1c9X/ML0d5TQP9wegHWAR7kVyPiVm1cTRR7dIjnjOZlRgRP7g4+nFbiaTwRUqEeFiwXLrO/k PnyjP72HPEU/YPeXvPFYEFhc6Ttywsp2mSuKxoWH6TsXBsgSxWGjaj/Ic2X5YUrLwfUOfM57SK0 26et+7FekltaeVqhGG4PgP0l6LfbEFtGQ1u7OHU7SpEdEmN0ihi0tVq/hozuPBM248lZl/vhAfw UnbudcG9mLE99ychPdATtgL+3Xx5hWcn77GbBLP+BX9I0nF8/KCiPjdazsXkML4lJTgKb9YGSaW pWjj0HAIvLKnHVqwYECfO3sO4yx2Kb4F6XL5s37CINRFw== X-Received: by 127.0.0.2 with SMTP id dKfbYY7687511xhG0dKXgbCD; Thu, 02 May 2024 10:02:32 -0700 X-Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web11.139.1714669351093271750 for ; Thu, 02 May 2024 10:02:31 -0700 X-Received: from [10.137.194.171] (unknown [131.107.159.43]) by linux.microsoft.com (Postfix) with ESMTPSA id 9547520B2C80; Thu, 2 May 2024 10:02:30 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 9547520B2C80 Message-ID: <3c6acbc8-8507-46e9-9055-ffc30ad4ad2d@linux.microsoft.com> Date: Thu, 2 May 2024 10:02:30 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation To: Sami Mujawar , "devel@edk2.groups.io" , "nhiphambka@gmail.com" , Yeo Reum Yun Cc: Ard Biesheuvel , Ray Ni , Kun Qin References: <20231205134757.14692-1-nhiphambka@gmail.com> <0d99c3c9-9dd0-494a-93b1-e9270914162b@linux.microsoft.com> <81C8FF71-D500-480D-A0A1-EC57039B1083@arm.com> From: "Oliver Smith-Denny" In-Reply-To: <81C8FF71-D500-480D-A0A1-EC57039B1083@arm.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: Thu, 02 May 2024 10:02:31 -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: OyqLjYiFmVotpT7pZyFiAT02x7686176AA= 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="1c9X/ML0"; 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 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, >=20 > We are working on a solution to remove the HOB creation logic from Standa= loneMm for Arm, and this involves implementing the Firmware handoff specifi= cation (https://github.com/FirmwareHandoff/firmware_handoff/releases/downlo= ad/v0.9/firmware_handoff.pdf). >=20 > As you rightly mentioned this also requires changes in TF-A, and this wor= k is in progress. >=20 > Levi (Yeo) is currently working on this feature and will post the patches= to the mailing list once we have the necessary components ready. >=20 > Regards, >=20 > Sami Mujawar >=20 > =EF=BB=BFOn 01/05/2024, 22:32, "Oliver Smith-Denny" > wrote: >=20 >=20 > Hi folks, returning to this thread because I noticed that HOB > creation still exists in StandaloneMmCore for ARM: >=20 >=20 > https://github.com/tianocore/edk2/blob/5d4c5253e8bbc0baa8837fcd868925212d= f85201/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList= .c >=20 >=20 > 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. >=20 >=20 > 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. >=20 >=20 > Curious to get other folks thoughts here on this paradigm. >=20 >=20 > Thanks, > Oliver >=20 >=20 > 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 > >> 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(-) >> >> diff --git a/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobL= ib.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.
>> >> Copyright (c) 2017 - 2018, ARM Limited. All rights reserved.
>> >> @@ -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 =3D GetHobList (); >> >> - >> >> - HobLength =3D (UINT16)((HobLength + 0x7) & (~0x7)); >> >> - >> >> - FreeMemory =3D HandOffHob->EfiFreeMemoryTop - HandOffHob->EfiFreeMemor= yBottom; >> >> - >> >> - if (FreeMemory < HobLength) { >> >> - return NULL; >> >> - } >> >> - >> >> - Hob =3D (VOID *)(UINTN)HandOffHob->EfiEndOfHobList; >> >> - ((EFI_HOB_GENERIC_HEADER *)Hob)->HobType =3D HobType; >> >> - ((EFI_HOB_GENERIC_HEADER *)Hob)->HobLength =3D HobLength; >> >> - ((EFI_HOB_GENERIC_HEADER *)Hob)->Reserved =3D 0; >> >> - >> >> - HobEnd =3D (EFI_HOB_GENERIC_HEADER *)((UINTN)Hob + HobLength); >> >> - HandOffHob->EfiEndOfHobList =3D (EFI_PHYSICAL_ADDRESS)(UINTN)HobEnd; >> >> - >> >> - HobEnd->HobType =3D EFI_HOB_TYPE_END_OF_HOB_LIST; >> >> - HobEnd->HobLength =3D sizeof (EFI_HOB_GENERIC_HEADER); >> >> - HobEnd->Reserved =3D 0; >> >> - HobEnd++; >> >> - HandOffHob->EfiFreeMemoryBottom =3D (EFI_PHYSICAL_ADDRESS)(UINTN)HobEn= d; >> >> - >> >> - 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)) =3D=3D 0) && >> >> - ((ModuleLength & (EFI_PAGE_SIZE - 1)) =3D=3D 0) >> >> - ); >> >> - >> >> - Hob =3D CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEM= ORY_ALLOCATION_MODULE)); >> >> - >> >> - CopyGuid (&(Hob->MemoryAllocationHeader.Name), &gEfiHobMemoryAllocModu= leGuid); >> >> - Hob->MemoryAllocationHeader.MemoryBaseAddress =3D MemoryAllocationModu= le; >> >> - Hob->MemoryAllocationHeader.MemoryLength =3D ModuleLength; >> >> - Hob->MemoryAllocationHeader.MemoryType =3D EfiBootServicesCode; >> >> - >> >> // >> >> - // Zero the reserved space to match HOB spec >> >> + // HOB is read only for Standalone MM drivers >> >> // >> >> - ZeroMem (Hob->MemoryAllocationHeader.Reserved, sizeof (Hob->MemoryAllo= cationHeader.Reserved)); >> >> - >> >> - CopyGuid (&Hob->ModuleName, ModuleName); >> >> - Hob->EntryPoint =3D 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 =3D CreateHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, sizeof (EFI_HOB_R= ESOURCE_DESCRIPTOR)); >> >> - ASSERT (Hob !=3D NULL); >> >> - >> >> - Hob->ResourceType =3D ResourceType; >> >> - Hob->ResourceAttribute =3D ResourceAttribute; >> >> - Hob->PhysicalStart =3D PhysicalStart; >> >> - Hob->ResourceLength =3D 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 identificat= ion >> >> and returns the start address of GUID HOB data so that caller can fill t= he 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 >=3D (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 <=3D (0xffff - sizeof (EFI_HOB_GUID_TYPE))); >> >> - >> >> - Hob =3D CreateHob (EFI_HOB_TYPE_GUID_EXTENSION, (UINT16)(sizeof (EFI_H= OB_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 identificat= ion, >> >> copies the input data to the HOB data field and returns the start addres= s 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 !=3D NULL || DataLength =3D=3D 0); >> >> - >> >> - HobData =3D 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 =3D CreateHob (EFI_HOB_TYPE_FV, sizeof (EFI_HOB_FIRMWARE_VOLUME)); >> >> - >> >> - Hob->BaseAddress =3D BaseAddress; >> >> - Hob->Length =3D 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 =3D CreateHob (EFI_HOB_TYPE_FV2, sizeof (EFI_HOB_FIRMWARE_VOLUME2)= ); >> >> - >> >> - Hob->BaseAddress =3D BaseAddress; >> >> - Hob->Length =3D 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 t= he processor. >> >> @@ -508,23 +456,19 @@ BuildCpuHob ( >> IN UINT8 SizeOfIoSpace >> >> ) >> >> { >> >> - EFI_HOB_CPU *Hob; >> >> - >> >> - Hob =3D CreateHob (EFI_HOB_TYPE_CPU, sizeof (EFI_HOB_CPU)); >> >> - >> >> - Hob->SizeOfMemorySpace =3D SizeOfMemorySpace; >> >> - Hob->SizeOfIoSpace =3D 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)) =3D=3D 0) && >> >> - ((Length & (EFI_PAGE_SIZE - 1)) =3D=3D 0) >> >> - ); >> >> - >> >> - Hob =3D CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEM= ORY_ALLOCATION)); >> >> - >> >> - ZeroMem (&(Hob->AllocDescriptor.Name), sizeof (EFI_GUID)); >> >> - Hob->AllocDescriptor.MemoryBaseAddress =3D BaseAddress; >> >> - Hob->AllocDescriptor.MemoryLength =3D Length; >> >> - Hob->AllocDescriptor.MemoryType =3D MemoryType; >> >> // >> >> - // Zero the reserved space to match HOB spec >> >> + // HOB is read only for Standalone MM drivers >> >> // >> >> - ZeroMem (Hob->AllocDescriptor.Reserved, sizeof (Hob->AllocDescriptor.R= eserved)); >> >> + ASSERT (FALSE); >> >> } >> >> >> >> /** >> >=20 >=20 >=20 > IMPORTANT NOTICE: The contents of this email and any attachments are conf= idential and may also be privileged. If you are not the intended recipient,= please notify the sender immediately and do not disclose the contents to a= ny other person, use it for any purpose, or store or copy the information i= n any medium. Thank you. -=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 (#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] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-