Re: [edk2] [PATCH v1 10/18] StandaloneMmPkg/HobLib: Add AARCH64 Specific HOB Library for management mode.

[Supreeth Venkatesh]

My response inline.

-Original Message-
From: Achin Gupta
Sent: Wednesday, April 25, 2018 9:50 AM
To: Supreeth Venkatesh 
Cc: edk2-devel@lists.01.org; michael.d.kin...@intel.com; liming@intel.com; 
jiewen@intel.com; leif.lindh...@linaro.org; ard.biesheu...@linaro.org; nd 

Subject: Re: [PATCH v1 10/18] StandaloneMmPkg/HobLib: Add AARCH64 Specific HOB 
Library for management mode.

Hi Supreeth,

On Fri, Apr 06, 2018 at 03:42:15PM +0100, Supreeth Venkatesh wrote:
> The Standalone MM environment is initialized during the SEC phase on
> ARM Standard Platforms. The MM Core driver implements an entry point
> module which is architecture specific and runs prior to the generic
> core driver code. The former creates a Hob list that the latter
> consumes. This happens in the same phase.
>
> This patch implements a Hob library that can be used by the entry
> point module to produce a Hob list and by the core driver code to consume it.

References to DXE core need to be removed and the copyright years needs to be 
updated.

I think it is worth getting this hoblib reviewed by the ArmPkg maintainers.
[Supreeth] I have modified it as Jiewen mentioned as it is StandaloneMmPkg 
specific. However, More the merrier.

Acked-by: Achin Gupta 

>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Achin Gupta 
> Signed-off-by: Supreeth Venkatesh 
> ---
>  StandaloneMmPkg/Library/HobLib/Arm/HobLib.c | 697 
> 
>  StandaloneMmPkg/Library/HobLib/HobLib.inf   |  45 ++
>  2 files changed, 742 insertions(+)
>  create mode 100644 StandaloneMmPkg/Library/HobLib/Arm/HobLib.c
>  create mode 100644 StandaloneMmPkg/Library/HobLib/HobLib.inf
>
> diff --git a/StandaloneMmPkg/Library/HobLib/Arm/HobLib.c
> b/StandaloneMmPkg/Library/HobLib/Arm/HobLib.c
> new file mode 100644
> index 00..62abf47f95
> --- /dev/null
> +++ b/StandaloneMmPkg/Library/HobLib/Arm/HobLib.c
> @@ -0,0 +1,697 @@
> +/** @file
> +  HOB Library implementation for DxeCore driver.
> +
> +Copyright (c) 2006 - 2014, Intel Corporation. All rights
> +reserved. Copyright (c) 2017, ARM Limited. All rights
> +reserved.
> +
> +This program and the accompanying materials are licensed and made
> +available under the terms and conditions of the BSD License which
> +accompanies this distribution.  The full text of the license may be
> +found at http://opensource.org/licenses/bsd-license.php.
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +//
> +// Cache copy of HobList pointer.
> +//
> +VOID *gHobList = NULL;
> +
> +/**
> +  Returns the pointer to the HOB list.
> +
> +  This function returns the pointer to first HOB in the list.
> +  For PEI phase, the PEI service GetHobList() can be used to retrieve
> + the pointer  to the HOB list.  For the DXE phase, the HOB list
> + pointer can be retrieved through  the EFI System Table by looking up theHOB 
> list GUID in the System Configuration Table.
> +  Since the System Configuration Table does not exist that the time
> + the DXE Core is  launched, the DXE Core uses a global variable from
> + the DXE Core Entry Point Library  to manage the pointer to the HOB list.
> +
> +  If the pointer to the HOB list is NULL, then ASSERT().
> +
> +  @return The pointer to the HOB list.
> +
> +**/
> +VOID *
> +EFIAPI
> +GetHobList (
> +  VOID
> +  )
> +{
> +  ASSERT (gHobList != NULL);
> +  return gHobList;
> +}
> +
> +/**
> +  Returns the next instance of a HOB type from the starting HOB.
> +
> +  This function searches the first instance of a HOB type from the starting 
> HOB pointer.
> +  If there does not exist such HOB type from the starting HOB pointer, it 
> will return NULL.
> +  In contrast with macro GET_NEXT_HOB(), this function does not skip
> + the starting HOB pointer
> +  unconditionally: it returns HobStart back if HobStart itself meets
> + the requirement;  caller is required to use GET_NEXT_HOB() if it wishes to 
> skip current HobStart.
> +
> +  If HobStart is NULL, then ASSERT().
> +
> +  @param  Type  The HOB type to return.
> +  @param  HobStart  The starting HOB pointer to search from.
> +
> +  @return The next instance of a HOB type from the starting HOB.
> +
> +**/
> +VOID *
> +EFIAPI
> +GetNextHob (
> +  IN UINT16 Type,
> +  IN CONST VOID *HobStart
> +  )
> +{
> +  EFI_PEI_HOB_POINTERS  Hob;
> +
> +  ASSERT (HobStart != NULL);
> +
> +  Hob.Raw = (UINT8 *) HobStart;
> +  //
> +  // Parse the HOB list until end of list or matching type is found.
> +  //
> +  while (!END_OF_HOB_LIST (Hob)) {
> +if (Hob.Header->HobType == Type) {
> +  return Hob.Raw;
> +}
> +Hob.Raw = GET_NEXT_HOB (Hob);
> +  }
> +  

Re: [edk2] [PATCH v1 10/18] StandaloneMmPkg/HobLib: Add AARCH64 Specific HOB Library for management mode.

[Supreeth Venkatesh]

My response inline.

-Original Message-
From: Yao, Jiewen 
Sent: Thursday, April 26, 2018 8:04 AM
To: Achin Gupta ; Supreeth Venkatesh 

Cc: edk2-devel@lists.01.org; Kinney, Michael D ; 
Gao, Liming ; leif.lindh...@linaro.org; 
ard.biesheu...@linaro.org; nd 
Subject: RE: [PATCH v1 10/18] StandaloneMmPkg/HobLib: Add AARCH64 Specific HOB 
Library for management mode.

Maybe we can use same layout as MmMemLib.

It seems only HobConstructor() is Arm specific. Other functions are quite 
generic.

[Supreeth] Ok. I made the changes. Please check version 2.

Thank you

> -Original Message-
> From: Achin Gupta [mailto:achin.gu...@arm.com]
> Sent: Wednesday, April 25, 2018 7:50 AM
> To: Supreeth Venkatesh 
> Cc: edk2-devel@lists.01.org; Kinney, Michael D
> ; Gao, Liming ; Yao,
> Jiewen ; leif.lindh...@linaro.org;
> ard.biesheu...@linaro.org; n...@arm.com
> Subject: Re: [PATCH v1 10/18] StandaloneMmPkg/HobLib: Add AARCH64
> Specific HOB Library for management mode.
>
> Hi Supreeth,
>
> On Fri, Apr 06, 2018 at 03:42:15PM +0100, Supreeth Venkatesh wrote:
> > The Standalone MM environment is initialized during the SEC phase on
> > ARM Standard Platforms. The MM Core driver implements an entry point
> > module which is architecture specific and runs prior to the generic
> > core driver code. The former creates a Hob list that the latter
> > consumes. This happens in the same phase.
> >
> > This patch implements a Hob library that can be used by the entry
> > point module to produce a Hob list and by the core driver code to consume 
> > it.
>
> References to DXE core need to be removed and the copyright years
> needs to be updated.
>
> I think it is worth getting this hoblib reviewed by the ArmPkg maintainers.
>
> Acked-by: Achin Gupta 
>
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Achin Gupta 
> > Signed-off-by: Supreeth Venkatesh 
> > ---
> >  StandaloneMmPkg/Library/HobLib/Arm/HobLib.c | 697
> 
> >  StandaloneMmPkg/Library/HobLib/HobLib.inf   |  45 ++
> >  2 files changed, 742 insertions(+)
> >  create mode 100644 StandaloneMmPkg/Library/HobLib/Arm/HobLib.c
> >  create mode 100644 StandaloneMmPkg/Library/HobLib/HobLib.inf
> >
> > diff --git a/StandaloneMmPkg/Library/HobLib/Arm/HobLib.c
> b/StandaloneMmPkg/Library/HobLib/Arm/HobLib.c
> > new file mode 100644
> > index 00..62abf47f95
> > --- /dev/null
> > +++ b/StandaloneMmPkg/Library/HobLib/Arm/HobLib.c
> > @@ -0,0 +1,697 @@
> > +/** @file
> > +  HOB Library implementation for DxeCore driver.
> > +
> > +Copyright (c) 2006 - 2014, Intel Corporation. All rights
> > +reserved. Copyright (c) 2017, ARM Limited. All rights
> > +reserved.
> > +
> > +This program and the accompanying materials are licensed and made
> > +available under the terms and conditions of the BSD
> License
> > +which accompanies this distribution.  The full text of the license
> > +may be
> found at
> > +http://opensource.org/licenses/bsd-license.php.
> > +
> > +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +
> > +**/
> > +
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +
> > +//
> > +// Cache copy of HobList pointer.
> > +//
> > +VOID *gHobList = NULL;
> > +
> > +/**
> > +  Returns the pointer to the HOB list.
> > +
> > +  This function returns the pointer to first HOB in the list.
> > +  For PEI phase, the PEI service GetHobList() can be used to
> > + retrieve the
> pointer
> > +  to the HOB list.  For the DXE phase, the HOB list pointer can be
> > + retrieved
> through
> > +  the EFI System Table by looking up theHOB list GUID in the System
> Configuration Table.
> > +  Since the System Configuration Table does not exist that the time
> > + the DXE
> Core is
> > +  launched, the DXE Core uses a global variable from the DXE Core
> > + Entry
> Point Library
> > +  to manage the pointer to the HOB list.
> > +
> > +  If the pointer to the HOB list is NULL, then ASSERT().
> > +
> > +  @return The pointer to the HOB list.
> > +
> > +**/
> > +VOID *
> > +EFIAPI
> > +GetHobList (
> > +  VOID
> > +  )
> > +{
> > +  ASSERT (gHobList != NULL);
> > +  return gHobList;
> > +}
> > +
> > +/**
> > +  Returns the next instance of a HOB type from the starting HOB.
> > +
> > +  This function searches the first instance of a HOB type from the
> > + starting
> HOB pointer.
> > +  If there does not exist such HOB type from the starting HOB
> > + pointer, it will
> return NULL.
> > +  In contrast with macro GET_NEXT_HOB(), this function does not
> > + skip the
> starting HOB 

Re: [edk2] [PATCH v1 10/18] StandaloneMmPkg/HobLib: Add AARCH64 Specific HOB Library for management mode.

[Yao, Jiewen]

Maybe we can use same layout as MmMemLib.

It seems only HobConstructor() is Arm specific. Other functions are quite 
generic.

Thank you

> -Original Message-
> From: Achin Gupta [mailto:achin.gu...@arm.com]
> Sent: Wednesday, April 25, 2018 7:50 AM
> To: Supreeth Venkatesh 
> Cc: edk2-devel@lists.01.org; Kinney, Michael D ;
> Gao, Liming ; Yao, Jiewen ;
> leif.lindh...@linaro.org; ard.biesheu...@linaro.org; n...@arm.com
> Subject: Re: [PATCH v1 10/18] StandaloneMmPkg/HobLib: Add AARCH64
> Specific HOB Library for management mode.
> 
> Hi Supreeth,
> 
> On Fri, Apr 06, 2018 at 03:42:15PM +0100, Supreeth Venkatesh wrote:
> > The Standalone MM environment is initialized during the SEC phase on ARM
> > Standard Platforms. The MM Core driver implements an entry point module
> > which is architecture specific and runs prior to the generic core driver
> > code. The former creates a Hob list that the latter consumes. This
> > happens in the same phase.
> >
> > This patch implements a Hob library that can be used by the entry point
> > module to produce a Hob list and by the core driver code to consume it.
> 
> References to DXE core need to be removed and the copyright years needs to be
> updated.
> 
> I think it is worth getting this hoblib reviewed by the ArmPkg maintainers.
> 
> Acked-by: Achin Gupta 
> 
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Achin Gupta 
> > Signed-off-by: Supreeth Venkatesh 
> > ---
> >  StandaloneMmPkg/Library/HobLib/Arm/HobLib.c | 697
> 
> >  StandaloneMmPkg/Library/HobLib/HobLib.inf   |  45 ++
> >  2 files changed, 742 insertions(+)
> >  create mode 100644 StandaloneMmPkg/Library/HobLib/Arm/HobLib.c
> >  create mode 100644 StandaloneMmPkg/Library/HobLib/HobLib.inf
> >
> > diff --git a/StandaloneMmPkg/Library/HobLib/Arm/HobLib.c
> b/StandaloneMmPkg/Library/HobLib/Arm/HobLib.c
> > new file mode 100644
> > index 00..62abf47f95
> > --- /dev/null
> > +++ b/StandaloneMmPkg/Library/HobLib/Arm/HobLib.c
> > @@ -0,0 +1,697 @@
> > +/** @file
> > +  HOB Library implementation for DxeCore driver.
> > +
> > +Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
> > +Copyright (c) 2017, ARM Limited. All rights reserved.
> > +
> > +This program and the accompanying materials
> > +are licensed and made available under the terms and conditions of the BSD
> License
> > +which accompanies this distribution.  The full text of the license may be
> found at
> > +http://opensource.org/licenses/bsd-license.php.
> > +
> > +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +
> > +**/
> > +
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +
> > +//
> > +// Cache copy of HobList pointer.
> > +//
> > +VOID *gHobList = NULL;
> > +
> > +/**
> > +  Returns the pointer to the HOB list.
> > +
> > +  This function returns the pointer to first HOB in the list.
> > +  For PEI phase, the PEI service GetHobList() can be used to retrieve the
> pointer
> > +  to the HOB list.  For the DXE phase, the HOB list pointer can be 
> > retrieved
> through
> > +  the EFI System Table by looking up theHOB list GUID in the System
> Configuration Table.
> > +  Since the System Configuration Table does not exist that the time the DXE
> Core is
> > +  launched, the DXE Core uses a global variable from the DXE Core Entry
> Point Library
> > +  to manage the pointer to the HOB list.
> > +
> > +  If the pointer to the HOB list is NULL, then ASSERT().
> > +
> > +  @return The pointer to the HOB list.
> > +
> > +**/
> > +VOID *
> > +EFIAPI
> > +GetHobList (
> > +  VOID
> > +  )
> > +{
> > +  ASSERT (gHobList != NULL);
> > +  return gHobList;
> > +}
> > +
> > +/**
> > +  Returns the next instance of a HOB type from the starting HOB.
> > +
> > +  This function searches the first instance of a HOB type from the starting
> HOB pointer.
> > +  If there does not exist such HOB type from the starting HOB pointer, it 
> > will
> return NULL.
> > +  In contrast with macro GET_NEXT_HOB(), this function does not skip the
> starting HOB pointer
> > +  unconditionally: it returns HobStart back if HobStart itself meets the
> requirement;
> > +  caller is required to use GET_NEXT_HOB() if it wishes to skip current
> HobStart.
> > +
> > +  If HobStart is NULL, then ASSERT().
> > +
> > +  @param  Type  The HOB type to return.
> > +  @param  HobStart  The starting HOB pointer to search from.
> > +
> > +  @return The next instance of a HOB type from the starting HOB.
> > +
> > +**/
> > +VOID *
> > +EFIAPI
> > +GetNextHob (
> > +  IN UINT16 Type,
> > +  IN CONST VOID *HobStart
> > +  )
> > +{
> > +  

Re: [edk2] [PATCH v1 10/18] StandaloneMmPkg/HobLib: Add AARCH64 Specific HOB Library for management mode.

[Achin Gupta]

Hi Supreeth,

On Fri, Apr 06, 2018 at 03:42:15PM +0100, Supreeth Venkatesh wrote:
> The Standalone MM environment is initialized during the SEC phase on ARM
> Standard Platforms. The MM Core driver implements an entry point module
> which is architecture specific and runs prior to the generic core driver
> code. The former creates a Hob list that the latter consumes. This
> happens in the same phase.
> 
> This patch implements a Hob library that can be used by the entry point
> module to produce a Hob list and by the core driver code to consume it.

References to DXE core need to be removed and the copyright years needs to be
updated.

I think it is worth getting this hoblib reviewed by the ArmPkg maintainers. 

Acked-by: Achin Gupta 

> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Achin Gupta 
> Signed-off-by: Supreeth Venkatesh 
> ---
>  StandaloneMmPkg/Library/HobLib/Arm/HobLib.c | 697 
> 
>  StandaloneMmPkg/Library/HobLib/HobLib.inf   |  45 ++
>  2 files changed, 742 insertions(+)
>  create mode 100644 StandaloneMmPkg/Library/HobLib/Arm/HobLib.c
>  create mode 100644 StandaloneMmPkg/Library/HobLib/HobLib.inf
> 
> diff --git a/StandaloneMmPkg/Library/HobLib/Arm/HobLib.c 
> b/StandaloneMmPkg/Library/HobLib/Arm/HobLib.c
> new file mode 100644
> index 00..62abf47f95
> --- /dev/null
> +++ b/StandaloneMmPkg/Library/HobLib/Arm/HobLib.c
> @@ -0,0 +1,697 @@
> +/** @file
> +  HOB Library implementation for DxeCore driver.
> +
> +Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
> +Copyright (c) 2017, ARM Limited. All rights reserved.
> +
> +This program and the accompanying materials
> +are licensed and made available under the terms and conditions of the BSD 
> License
> +which accompanies this distribution.  The full text of the license may be 
> found at
> +http://opensource.org/licenses/bsd-license.php.
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +//
> +// Cache copy of HobList pointer.
> +//
> +VOID *gHobList = NULL;
> +
> +/**
> +  Returns the pointer to the HOB list.
> +
> +  This function returns the pointer to first HOB in the list.
> +  For PEI phase, the PEI service GetHobList() can be used to retrieve the 
> pointer
> +  to the HOB list.  For the DXE phase, the HOB list pointer can be retrieved 
> through
> +  the EFI System Table by looking up theHOB list GUID in the System 
> Configuration Table.
> +  Since the System Configuration Table does not exist that the time the DXE 
> Core is
> +  launched, the DXE Core uses a global variable from the DXE Core Entry 
> Point Library
> +  to manage the pointer to the HOB list.
> +
> +  If the pointer to the HOB list is NULL, then ASSERT().
> +
> +  @return The pointer to the HOB list.
> +
> +**/
> +VOID *
> +EFIAPI
> +GetHobList (
> +  VOID
> +  )
> +{
> +  ASSERT (gHobList != NULL);
> +  return gHobList;
> +}
> +
> +/**
> +  Returns the next instance of a HOB type from the starting HOB.
> +
> +  This function searches the first instance of a HOB type from the starting 
> HOB pointer.
> +  If there does not exist such HOB type from the starting HOB pointer, it 
> will return NULL.
> +  In contrast with macro GET_NEXT_HOB(), this function does not skip the 
> starting HOB pointer
> +  unconditionally: it returns HobStart back if HobStart itself meets the 
> requirement;
> +  caller is required to use GET_NEXT_HOB() if it wishes to skip current 
> HobStart.
> +
> +  If HobStart is NULL, then ASSERT().
> +
> +  @param  Type  The HOB type to return.
> +  @param  HobStart  The starting HOB pointer to search from.
> +
> +  @return The next instance of a HOB type from the starting HOB.
> +
> +**/
> +VOID *
> +EFIAPI
> +GetNextHob (
> +  IN UINT16 Type,
> +  IN CONST VOID *HobStart
> +  )
> +{
> +  EFI_PEI_HOB_POINTERS  Hob;
> +
> +  ASSERT (HobStart != NULL);
> +
> +  Hob.Raw = (UINT8 *) HobStart;
> +  //
> +  // Parse the HOB list until end of list or matching type is found.
> +  //
> +  while (!END_OF_HOB_LIST (Hob)) {
> +if (Hob.Header->HobType == Type) {
> +  return Hob.Raw;
> +}
> +Hob.Raw = GET_NEXT_HOB (Hob);
> +  }
> +  return NULL;
> +}
> +
> +/**
> +  Returns the first instance of a HOB type among the whole HOB list.
> +
> +  This function searches the first instance of a HOB type among the whole 
> HOB list.
> +  If there does not exist such HOB type in the HOB list, it will return NULL.
> +
> +  If the pointer to the HOB list is NULL, then ASSERT().
> +
> +  @param  Type  The HOB type to return.
> +
> +  @return The next instance of a HOB type from the starting HOB.
> +
> +**/
> +VOID *
> +EFIAPI
> +GetFirstHob (
> +  IN UINT16