Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation

2024-05-03 Thread Oliver Smith-Denny 

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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation

2024-05-02 Thread Nhi Pham via groups.io 

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 
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/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.

  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 = 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

Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation

2024-05-02 Thread Oliver Smith-Denny 

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" mailto:o...@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
 



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 mailto:ardb+tianoc...@kernel.org>>
Cc: Ray Ni mailto:ray...@intel.com>>
Cc: Sami Mujawar mailto:sami.muja...@arm.com>>
Cc: Oliver Smith-Denny mailto:o...@linux.microsoft.com>>
Signed-off-by: Nhi Pham mailto:nhipham...@gmail.com>>
---
StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c | 171 
++--
1 file changed, 51 insertions(+), 120 deletions(-)

diff --git a/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c 
b/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
index ee61bdd227d0..bef66d167494 100644
--- a/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
+++ b/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
@@ -1,5 +1,5 @@
/** @file

- HOB Library implementation for Standalone MM Core.

+ HOB Library implementation for Standalone MM modules.



Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.

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 = 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().

Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation

2024-05-02 Thread Sami Mujawar 
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" mailto:o...@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
 



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  >
> Cc: Ray Ni mailto:ray...@intel.com>>
> Cc: Sami Mujawar mailto:sami.muja...@arm.com>>
> Cc: Oliver Smith-Denny  >
> Signed-off-by: Nhi Pham mailto:nhipham...@gmail.com>>
> ---
> StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c | 171 
> ++--
> 1 file changed, 51 insertions(+), 120 deletions(-)
>
> diff --git a/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c 
> b/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
> index ee61bdd227d0..bef66d167494 100644
> --- a/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
> +++ b/StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
> @@ -1,5 +1,5 @@
> /** @file
>
> - HOB Library implementation for Standalone MM Core.
>
> + HOB Library implementation for Standalone MM modules.
>
>
>
> Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
>
> 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 = 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

Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation

2024-05-01 Thread Oliver Smith-Denny 

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 
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/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.

  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_ADDRESSFreeMemory;

-  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), 
);

-  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 (>ModuleName, ModuleName);

-  Hob->EntryPoint = EntryPoint;

+  ASSERT (FALSE);

  }

  


  /**

Builds

Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation

2023-12-10 Thread Ni, Ray 
Merged at bb13a4adab

Thanks,
Ray
> -Original Message-
> From: Ni, Ray
> Sent: Saturday, December 9, 2023 9:27 AM
> To: Nhi Pham ; devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Sami Mujawar
> ; Oliver Smith-Denny 
> Subject: RE: [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib:
> Remove HOB creation
> 
> Nhi,
> Thanks for the follow up!
> 
> Reviewed-by: Ray Ni 
> 
> Thanks,
> Ray
> > -Original Message-
> > From: Nhi Pham 
> > Sent: Tuesday, December 5, 2023 9:48 PM
> > To: devel@edk2.groups.io
> > Cc: Nhi Pham ; Ard Biesheuvel
> > ; Ni, Ray ; Sami Mujawar
> > ; Oliver Smith-Denny
> 
> > 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 
> > 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/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.
> >
> >  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_ADDRESSFreeMemory;
> >
> > -  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),
> > );
> >
> > -  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,

Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation

2023-12-08 Thread Ni, Ray 
Nhi,
Thanks for the follow up!

Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Nhi Pham 
> Sent: Tuesday, December 5, 2023 9:48 PM
> To: devel@edk2.groups.io
> Cc: Nhi Pham ; Ard Biesheuvel
> ; Ni, Ray ; Sami Mujawar
> ; Oliver Smith-Denny 
> 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 
> 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/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.
> 
>  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_ADDRESSFreeMemory;
> 
> -  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),
> );
> 
> -  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 (>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  ResourceTypeThe type of resource described by this HOB.
> 
>

[edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmHobLib: Remove HOB creation

2023-12-06 Thread Nhi Pham 
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/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.
 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_ADDRESSFreeMemory;
-  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), 
);
-  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 (>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  ResourceTypeThe 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,