Re: [edk2-devel] [staging/dynamictables-reorg PATCH v1 1/2] DynamicTablesPkg: Introduce an Arch Common Namespace

["Sunil V L" <sunilvl@ventanamicro.com>]

On Mon, Mar 04, 2024 at 04:46:16PM +0000, Sami Mujawar wrote:
> Introduce an Arch Common Namespace so that the common
> architectural objects can be defined under this namespace
> in the Configuration manager. Also rearrange the namespace
> IDs so that the Arch Common Namespace has a value of 0x1,
> the Arm Namespace ID has a value of 0x2, and the Custom/OEM
> namespace ID has a value of 0xF. Also introduce a helper
> macro to create configuration manager objects in the Arch
> Common Namespace.
> 
> The Arch Common Namespace shall contain objects like
> Serial Port, PCI Bus information etc. It must not contain
> Architecture specific components e.g. GICC which is Arm
> architecture specific component and therefore must be
> defined in the Arm Namespace.
> 
> Cc: Pierre Gondois <Pierre.Gondois@arm.com>
> Cc: Yeo Reum Yun <YeoReum.Yun@arm.com>
> Cc: Sunil V L <sunilvl@ventanamicro.com>
> Cc: AbdulLateef Attar <AbdulLateef.Attar@amd.com>
> Cc: Jeshua Smith <jeshuas@nvidia.com>
> Cc: Jeff Brasen <jbrasen@nvidia.com>
> Cc: Girish Mahadevan <gmahadevan@nvidia.com>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>  DynamicTablesPkg/Include/ConfigurationManagerObject.h | 26 ++++--
>  DynamicTablesPkg/Readme.md                            | 98 ++++++++++++++++++++
>  2 files changed, 117 insertions(+), 7 deletions(-)
> 
> diff --git a/DynamicTablesPkg/Include/ConfigurationManagerObject.h b/DynamicTablesPkg/Include/ConfigurationManagerObject.h
> index 74ad25d5d94a34da39e6c8dd909f9863e223d086..4255c82b42aefb52148a14a44c91328ec82f1550 100644
> --- a/DynamicTablesPkg/Include/ConfigurationManagerObject.h
> +++ b/DynamicTablesPkg/Include/ConfigurationManagerObject.h
> @@ -1,6 +1,6 @@
>  /** @file
>  
> -  Copyright (c) 2017 - 2022, ARM Limited. All rights reserved.
> +  Copyright (c) 2017 - 2024, Arm Limited. All rights reserved.
>  
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @@ -29,8 +29,9 @@ _______________________________________________________________________________
>  
>  Bits: [31:28] - Name Space ID
>                  0000 - Standard
> -                0001 - ARM
> -                1000 - Custom/OEM
> +                0001 - Arch Common
> +                0010 - ARM
> +                1111 - Custom/OEM
>                  All other values are reserved.
>  
>  Bits: [27:16] - Reserved.
> @@ -105,10 +106,11 @@ typedef UINT32 CM_OBJECT_ID;
>      for the Configuration Manager Objects.
>  */
>  typedef enum ObjectNameSpaceID {
> -  EObjNameSpaceStandard,      ///< Standard Objects Namespace
> -  EObjNameSpaceArm,           ///< ARM Objects Namespace
> -  EObjNameSpaceOem = 0x8,     ///< OEM Objects Namespace
> -  EObjNameSpaceMax
> +  EObjNameSpaceStandard,          ///< Standard Objects Namespace
> +  EObjNameSpaceArchCommon,        ///< Arch Common Objects Namespace
> +  EObjNameSpaceArm,               ///< ARM Objects Namespace
> +  EObjNameSpaceOem = 0xF,         ///< OEM Objects Namespace
> +  EObjNameSpaceMax,
>  } EOBJECT_NAMESPACE_ID;
>  
>  /** A descriptor for Configuration Manager Objects.
> @@ -182,6 +184,16 @@ typedef struct CmObjDescriptor {
>  #define CREATE_CM_ARM_OBJECT_ID(ObjectId) \
>            (CREATE_CM_OBJECT_ID (EObjNameSpaceArm, ObjectId))
>  
> +/** This macro returns a Configuration Manager Object ID
> +    in the Arch Common Object Namespace.
> +
> +  @param [in] ObjectId    The Object ID.
> +
> +  @retval Returns an Arch Common Configuration Manager Object ID.
> +**/
> +#define CREATE_CM_ARCH_COMMON_OBJECT_ID(ObjectId) \
> +          (CREATE_CM_OBJECT_ID (EObjNameSpaceArchCommon, ObjectId))
> +
>  /** This macro returns a Configuration Manager Object ID
>      in the OEM Object Namespace.
>  
> diff --git a/DynamicTablesPkg/Readme.md b/DynamicTablesPkg/Readme.md
> index c1cdc5e17326709e811a2f3fe530316e95dec7ee..ba4ae7155f13a77298ece74cc567bb3e9dfc303d 100644
> --- a/DynamicTablesPkg/Readme.md
> +++ b/DynamicTablesPkg/Readme.md
> @@ -402,3 +402,101 @@ Refer to the following presentation from *UEFI Plugfest Seattle 2018*:
>  
>  [Dynamic Tables Framework: A Step Towards Automatic Generation of Advanced Configuration and Power Interface (ACPI) & System Management BIOS (SMBIOS) Tables](http://www.uefi.org/sites/default/files/resources/Arm_Dynamic%20Tables%20Framework%20A%20Step%20Towards%20Automatic%20Generation%20of%20Advanced%20Configuration%20and%20Power%20Interface%20%28ACPI%29%20%26%20System%20Management%20BIOS%20%28SMBIOS%29%20Tables%20_0.pdf)
>  
> +## Configuration Manager Objects
> +
> +The CM_OBJECT_ID type is used to identify the Configuration Manager
> +    objects.
> +
> +## Description of Configuration Manager Object ID
> +
> +| 31 30 29 28 | 27 | 26 | 25 | 24 | 23 | 22 | 21 | 20 | 19 | 18 | 17 | 16 | 15 | 14 | 13 | 12 | 11 | 10 | 9 | 8 | 7 6 5 4 3 2 1 0 |
> +| ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: |
> +| `Name Space ID` | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | `Object ID` |
> +---------------------------------------------------------------------------------
> +
NIT: Can we minimize the width by using range instead of each bits? 31-28,
27-8 and 7-0 would be better even when it is rendered in the browser
IMO.

Otherwise LGTM.

Thanks!

Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116359): https://edk2.groups.io/g/devel/message/116359
Mute This Topic: https://groups.io/mt/104726065/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-