Re: [PATCH v2 17/21] DynamicTablesPkg: AML code generation for a Method returning a NS

["Sami Mujawar" <sami.mujawar@arm.com>]

Hi Pierre,

I think this v2 patch has introduced an issue (which was not there in
v1), see my comment marked inline as [SAMI].

With that fixed,

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar


On 07/10/2021 04:32 PM, Pierre.Gondois@arm.com wrote:
> From: Pierre Gondois <Pierre.Gondois@arm.com>
>
> Add AmlCodeGenMethodRetNameString() to generate AML code to create
> a Method returning a NameString (NS).
>
> AmlCodeGenMethodRetNameString (
>    "MET0", "_CRS", 1, TRUE, 3, ParentNode, NewObjectNode
>    );
> is equivalent of the following ASL code:
> Method(MET0, 1, Serialized, 3) {
>    Return (_CRS)
> }
>
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
>   .../Include/Library/AmlLib/AmlLib.h           |  53 +++++++++
>   .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 107 ++++++++++++++++++
>   2 files changed, 160 insertions(+)
>
> diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> index 6824cf3a6c82..7740aac24470 100644
> --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> @@ -619,6 +619,59 @@ AmlCodeGenScope (
>     OUT       AML_OBJECT_NODE_HANDLE  * NewObjectNode   OPTIONAL
>     );
>
> +/** AML code generation for a method returning a NameString.
> +
> +  AmlCodeGenMethodRetNameString (
> +    "MET0", "_CRS", 1, TRUE, 3, ParentNode, NewObjectNode
> +    );
> +  is equivalent of the following ASL code:
> +    Method(MET0, 1, Serialized, 3) {
> +      Return (_CRS)
> +    }
> +
> +  The ASL parameters "ReturnType" and "ParameterTypes" are not asked
> +  in this function. They are optional parameters in ASL.
> +
> +  @ingroup CodeGenApis
> +
> +  @param [in]  MethodNameString     The new Method's name.
> +                                    Must be a NULL-terminated ASL NameString
> +                                    e.g.: "MET0", "_SB.MET0", etc.
> +                                    The input string is copied.
> +  @param [in]  ReturnedNameString   The name of the object returned by the
> +                                    method. Optional parameter, can be:
> +                                     - NULL (ignored).
> +                                     - A NULL-terminated ASL NameString.
> +                                       e.g.: "MET0", "_SB.MET0", etc.
> +                                       The input string is copied.
> +  @param [in]  NumArgs              Number of arguments.
> +                                    Must be 0 <= NumArgs <= 6.
> +  @param [in]  IsSerialized         TRUE is equivalent to Serialized.
> +                                    FALSE is equivalent to NotSerialized.
> +                                    Default is NotSerialized in ASL spec.
> +  @param [in]  SyncLevel            Synchronization level for the method.
> +                                    Must be 0 <= SyncLevel <= 15.
> +                                    Default is 0 in ASL.
> +  @param [in]  ParentNode           If provided, set ParentNode as the parent
> +                                    of the node created.
> +  @param [out] NewObjectNode        If success, contains the created node.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
> +**/
> +EFI_STATUS
> +EFIAPI
> +AmlCodeGenMethodRetNameString (
> +  IN  CONST CHAR8                   * MethodNameString,
> +  IN  CONST CHAR8                   * ReturnedNameString,  OPTIONAL
> +  IN        UINT8                     NumArgs,
> +  IN        BOOLEAN                   IsSerialized,
> +  IN        UINT8                     SyncLevel,
> +  IN        AML_NODE_HANDLE           ParentNode,          OPTIONAL
> +  OUT       AML_OBJECT_NODE_HANDLE  * NewObjectNode        OPTIONAL
> +  );
> +
>   // DEPRECATED APIS
>   #ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>
> diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> index 93d4ba79e937..67474094b975 100644
> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> @@ -1335,3 +1335,110 @@ exit_handler:
>     }
>     return Status;
>   }
> +
> +/** AML code generation for a method returning a NameString.
> +
> +  AmlCodeGenMethodRetNameString (
> +    "MET0", "_CRS", 1, TRUE, 3, ParentNode, NewObjectNode
> +    );
> +  is equivalent of the following ASL code:
> +    Method(MET0, 1, Serialized, 3) {
> +      Return (_CRS)
> +    }
> +
> +  The ASL parameters "ReturnType" and "ParameterTypes" are not asked
> +  in this function. They are optional parameters in ASL.
> +
> +  @param [in]  MethodNameString     The new Method's name.
> +                                    Must be a NULL-terminated ASL NameString
> +                                    e.g.: "MET0", "_SB.MET0", etc.
> +                                    The input string is copied.
> +  @param [in]  ReturnedNameString   The name of the object returned by the
> +                                    method. Optional parameter, can be:
> +                                     - NULL (ignored).
> +                                     - A NULL-terminated ASL NameString.
> +                                       e.g.: "MET0", "_SB.MET0", etc.
> +                                       The input string is copied.
> +  @param [in]  NumArgs              Number of arguments.
> +                                    Must be 0 <= NumArgs <= 6.
> +  @param [in]  IsSerialized         TRUE is equivalent to Serialized.
> +                                    FALSE is equivalent to NotSerialized.
> +                                    Default is NotSerialized in ASL spec.
> +  @param [in]  SyncLevel            Synchronization level for the method.
> +                                    Must be 0 <= SyncLevel <= 15.
> +                                    Default is 0 in ASL.
> +  @param [in]  ParentNode           If provided, set ParentNode as the parent
> +                                    of the node created.
> +  @param [out] NewObjectNode        If success, contains the created node.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
> +**/
> +EFI_STATUS
> +EFIAPI
> +AmlCodeGenMethodRetNameString (
> +  IN  CONST CHAR8                   * MethodNameString,
> +  IN  CONST CHAR8                   * ReturnedNameString,  OPTIONAL
> +  IN        UINT8                     NumArgs,
> +  IN        BOOLEAN                   IsSerialized,
> +  IN        UINT8                     SyncLevel,
> +  IN        AML_NODE_HANDLE           ParentNode,          OPTIONAL
> +  OUT       AML_OBJECT_NODE_HANDLE  * NewObjectNode        OPTIONAL
> +  )
> +{
> +  EFI_STATUS                Status;
> +  AML_OBJECT_NODE_HANDLE    MethodNode;
> +
> +  if ((MethodNameString == NULL)  ||
> +      ((ParentNode == NULL) && (NewObjectNode == NULL))) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // Create a Method named MethodNameString.
> +  Status = AmlCodeGenMethod (
> +             MethodNameString,
> +             NumArgs,
> +             IsSerialized,
> +             SyncLevel,
> +             NULL,
> +             &MethodNode
> +             );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +    return Status;
> +  }
> +
> +  // Return ReturnedNameString if provided.
> +  if (ReturnedNameString != NULL) {
> +    Status = AmlCodeGenReturnNameString (
> +               ReturnedNameString,
> +               (AML_NODE_HANDLE)MethodNode,
> +               NULL
> +               );
> +    if (EFI_ERROR (Status)) {
> +      ASSERT (0);
> +      MethodNode = NULL;
[SAMI] Setting MethodNode to NULL will result in the MethodNode not
being deleted in case of an error.
> +      goto error_handler;
> +    }
> +  }
> +
> +  Status = LinkNode (
> +             MethodNode,
> +             ParentNode,
> +             NewObjectNode
> +             );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +    goto error_handler;
> +  }
> +
> +  return Status;
> +
> +error_handler:
> +  if (MethodNode != NULL) {
> +    AmlDeleteTree ((AML_NODE_HANDLE)MethodNode);
> +  }
> +  return Status;
> +}

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.