Thanks for your explanation, Abner.
From: Chang, Abner <Abner.Chang@amd.com>
Sent: Tuesday, February 27, 2024 8:40 AM
To: Nickle Wang <nicklew@nvidia.com>; devel@edk2.groups.io
Cc: Igor Kulchytskyy <igork@ami.com>
Subject: RE: [PATCH] RedfishPkg/RestJsonStructureDxe: Refine REST JSON C Structure DXE driver
|
External email: Use caution opening links or attachments
|
[AMD Official Use Only - General]
Hi Nickle,
As from spec viewpoint, REST JSON C Structure was not designed for Redfish only. Which was designed for any RESTful application that uses JSON as the representation of data model.
I was proposed having this driver under MdeModulePkg long time ago when this driver was introduced, but as I can remember they considered this driver is only used by Redfish back to the moment. That’s true, however this driver doesn’t require
any reference of Redfish actually.
Thanks
Abner
From: Nickle Wang <nicklew@nvidia.com>
Sent: Monday, February 26, 2024 9:50 PM
To: Chang, Abner <Abner.Chang@amd.com>;
devel@edk2.groups.io
Cc: Igor Kulchytskyy <igork@ami.com>
Subject: RE: [PATCH] RedfishPkg/RestJsonStructureDxe: Refine REST JSON C Structure DXE driver
|
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
|
RestJsonStructureDxe is a driver in RedfishPkg. May I know why we can not have dependency to RedfishPkg? Do we need to move it to MdeModulePkg for this goal (removing dependency to RedfishPkg)?
Regards,
Nickle
> -----Original Message-----
> From: abner.chang@amd.com <abner.chang@amd.com>
> Sent: Monday, February 26, 2024 10:44 AM
> To: devel@edk2.groups.io
> Cc: Nickle Wang <nicklew@nvidia.com>; Igor Kulchytskyy <igork@ami.com>
> Subject: [PATCH] RedfishPkg/RestJsonStructureDxe: Refine REST JSON C Structure
> DXE driver
>
> External email: Use caution opening links or attachments
>
>
> From: Abner Chang <abner.chang@amd.com>
>
> BZ #: 4711
> - Add mode debug messages.
> - This driver shouldn't have a dependency on Redfish package and
> the references of "Redfish" terminology.
> Remove the references of "Redfish" from this driver.
> - Fix the missing parameter of DEBUG macros used in this
> driver.
>
> Signed-off-by: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> ---
> .../RestJsonStructureDxe.inf | 3 +-
> .../RestJsonStructureInternal.h | 3 +-
> .../RestJsonStructureDxe.c | 96 ++++++++++++++++++-
> 3 files changed, 96 insertions(+), 6 deletions(-)
>
> diff --git a/RedfishPkg/RestJsonStructureDxe/RestJsonStructureDxe.inf
> b/RedfishPkg/RestJsonStructureDxe/RestJsonStructureDxe.inf
> index 61e6253d318..e74c9dfd38b 100644
> --- a/RedfishPkg/RestJsonStructureDxe/RestJsonStructureDxe.inf
> +++ b/RedfishPkg/RestJsonStructureDxe/RestJsonStructureDxe.inf
> @@ -2,6 +2,7 @@
> # Implementation of EFI REST JSON Structure Protocol.
> #
> # (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
> +# Copyright (C) 2024 Advanced Micro Devices, Inc. All rights
> +reserved.<BR>
> # SPDX-License-Identifier: BSD-2-Clause-Patent ##
>
> @@ -17,7 +18,6 @@
> [Packages]
> MdePkg/MdePkg.dec
> MdeModulePkg/MdeModulePkg.dec
> - RedfishPkg/RedfishPkg.dec
>
> [Sources]
> RestJsonStructureDxe.c
> @@ -26,6 +26,7 @@
> [LibraryClasses]
> BaseLib
> BaseMemoryLib
> + DebugLib
> MemoryAllocationLib
> UefiBootServicesTableLib
> UefiDriverEntryPoint
> diff --git a/RedfishPkg/RestJsonStructureDxe/RestJsonStructureInternal.h
> b/RedfishPkg/RestJsonStructureDxe/RestJsonStructureInternal.h
> index 8d7175125c1..04be5cc80b5 100644
> --- a/RedfishPkg/RestJsonStructureDxe/RestJsonStructureInternal.h
> +++ b/RedfishPkg/RestJsonStructureDxe/RestJsonStructureInternal.h
> @@ -3,6 +3,7 @@
> Protocol.
>
> (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
> + Copyright (C) 2024 Advanced Micro Devices, Inc. All rights
> + reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -25,7 +26,7 @@
> typedef struct _REST_JSON_STRUCTURE_INSTANCE {
> LIST_ENTRY NextRestJsonStructureInstance; ///< Next
> convertor instance
> UINTN NumberOfNameSpaceToConvert; ///< Number
> of resource type this convertor supports.
> - EFI_REST_JSON_RESOURCE_TYPE_IDENTIFIER *SupportedRsrcIndentifier;
> ///< The resource type linklist
> + EFI_REST_JSON_RESOURCE_TYPE_IDENTIFIER *SupportedRsrcIndentifier;
> ///< The supported resource type array.
> EFI_REST_JSON_STRUCTURE_TO_STRUCTURE JsonToStructure;
> ///< JSON to C structure function
> EFI_REST_JSON_STRUCTURE_TO_JSON StructureToJson; ///< C
> structure to JSON function
> EFI_REST_JSON_STRUCTURE_DESTORY_STRUCTURE DestroyStructure;
> ///< Destory C struture function.
> diff --git a/RedfishPkg/RestJsonStructureDxe/RestJsonStructureDxe.c
> b/RedfishPkg/RestJsonStructureDxe/RestJsonStructureDxe.c
> index 404866fb319..0da5132e5ae 100644
> --- a/RedfishPkg/RestJsonStructureDxe/RestJsonStructureDxe.c
> +++ b/RedfishPkg/RestJsonStructureDxe/RestJsonStructureDxe.c
> @@ -4,12 +4,14 @@
> Protocol.
>
> (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
> + Copyright (C) 2024 Advanced Micro Devices, Inc. All rights
> + reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
>
> #include <Uefi.h>
> +#include <Library/DebugLib.h>
> #include <Protocol/RestJsonStructure.h> #include "RestJsonStructureInternal.h"
>
> @@ -72,6 +74,8 @@ RestJsonStructureRegister (
> }
> }
>
> + DEBUG ((DEBUG_MANAGEABILITY, "%a: %d REST JSON-C interpreter(s) to
> + register for the name spaces.\n", __func__, NumberOfNS));
> +
> Instance =
> (REST_JSON_STRUCTURE_INSTANCE *)AllocateZeroPool (sizeof
> (REST_JSON_STRUCTURE_INSTANCE) + NumberOfNS * sizeof
> (EFI_REST_JSON_RESOURCE_TYPE_IDENTIFIER));
> if (Instance == NULL) {
> @@ -88,6 +92,10 @@ RestJsonStructureRegister (
> ThisSupportedInterp = JsonStructureSupported;
> for (Index = 0; Index < NumberOfNS; Index++) {
> CopyMem ((VOID *)CloneSupportedInterpId, (VOID *)&ThisSupportedInterp-
> >RestResourceInterp, sizeof (EFI_REST_JSON_RESOURCE_TYPE_IDENTIFIER));
> + DEBUG ((DEBUG_MANAGEABILITY, " Resource type : %a\n",
> ThisSupportedInterp->RestResourceInterp.NameSpace.ResourceTypeName));
> + DEBUG ((DEBUG_MANAGEABILITY, " Major version : %a\n",
> ThisSupportedInterp->RestResourceInterp.NameSpace.MajorVersion));
> + DEBUG ((DEBUG_MANAGEABILITY, " Minor version : %a\n",
> ThisSupportedInterp->RestResourceInterp.NameSpace.MinorVersion));
> + DEBUG ((DEBUG_MANAGEABILITY, " Errata version: %a\n\n",
> + ThisSupportedInterp->RestResourceInterp.NameSpace.ErrataVersion));
> ThisSupportedInterp = (EFI_REST_JSON_STRUCTURE_SUPPORTED
> *)ThisSupportedInterp->NextSupportedRsrcInterp.ForwardLink;
> CloneSupportedInterpId++;
> }
> @@ -125,6 +133,8 @@ InterpreterInstanceToStruct (
> EFI_STATUS Status;
> EFI_REST_JSON_RESOURCE_TYPE_IDENTIFIER *ThisSupportedRsrcTypeId;
>
> + DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry\n", __func__));
> +
> if ((This == NULL) ||
> (InterpreterInstance == NULL) ||
> (ResourceRaw == NULL) ||
> @@ -146,9 +156,23 @@ InterpreterInstanceToStruct (
> ResourceRaw,
> RestJSonHeader
> );
> + if (EFI_ERROR (Status)) {
> + if (Status == EFI_UNSUPPORTED) {
> + DEBUG ((
> + DEBUG_MANAGEABILITY,
> + "%a %a.%a.%a REST JSON to C structure interpreter has no capability to
> interpret the resource.\n",
> + InterpreterInstance->SupportedRsrcIndentifier-
> >NameSpace.ResourceTypeName,
> + InterpreterInstance->SupportedRsrcIndentifier-
> >NameSpace.MajorVersion,
> + InterpreterInstance->SupportedRsrcIndentifier-
> >NameSpace.MinorVersion,
> + InterpreterInstance->SupportedRsrcIndentifier-
> >NameSpace.ErrataVersion
> + ));
> + } else {
> + DEBUG ((DEBUG_MANAGEABILITY, "REST JsonToStructure returns failure -
> %r\n", Status));
> + }
> + }
> } else {
> //
> - // Check if the namesapce and version is supported by this interpreter.
> + // Check if the namespace and version is supported by this interpreter.
> //
> ThisSupportedRsrcTypeId = InterpreterInstance->SupportedRsrcIndentifier;
> for (Index = 0; Index < InterpreterInstance->NumberOfNameSpaceToConvert;
> Index++) { @@ -171,6 +195,11 @@ InterpreterInstanceToStruct (
> ResourceRaw,
> RestJSonHeader
> );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_MANAGEABILITY, "Don't check version of this resource
> type identifier JsonToStructure returns %r\n", Status));
> + DEBUG ((DEBUG_MANAGEABILITY, " Supported ResourceTypeName =
> %a\n", ThisSupportedRsrcTypeId->NameSpace.ResourceTypeName));
> + }
> +
> break;
> } else {
> //
> @@ -195,6 +224,14 @@ InterpreterInstanceToStruct (
> ResourceRaw,
> RestJSonHeader
> );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_MANAGEABILITY, "Check version of this resource type
> identifier JsonToStructure returns %r\n", Status));
> + DEBUG ((DEBUG_MANAGEABILITY, " Supported ResourceTypeName =
> %a\n", ThisSupportedRsrcTypeId->NameSpace.ResourceTypeName));
> + DEBUG ((DEBUG_MANAGEABILITY, " Supported MajorVersion =
> %a\n", ThisSupportedRsrcTypeId->NameSpace.MajorVersion));
> + DEBUG ((DEBUG_MANAGEABILITY, " Supported MinorVersion =
> %a\n", ThisSupportedRsrcTypeId->NameSpace.MinorVersion));
> + DEBUG ((DEBUG_MANAGEABILITY, " Supported ErrataVersion =
> %a\n", ThisSupportedRsrcTypeId->NameSpace.ErrataVersion));
> + }
> +
> break;
> }
> }
> @@ -232,6 +269,8 @@ InterpreterEfiStructToInstance (
> EFI_REST_JSON_RESOURCE_TYPE_IDENTIFIER *ThisSupportedRsrcTypeId;
> EFI_REST_JSON_RESOURCE_TYPE_IDENTIFIER *RsrcTypeIdentifier;
>
> + DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry\n", __func__));
> +
> if ((This == NULL) ||
> (InterpreterInstance == NULL) ||
> (RestJSonHeader == NULL) ||
> @@ -284,6 +323,14 @@ InterpreterEfiStructToInstance (
> RestJSonHeader,
> ResourceRaw
> );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_MANAGEABILITY, "StructureToJson returns %r\n",
> Status));
> + DEBUG ((DEBUG_MANAGEABILITY, " Supported ResourceTypeName =
> %a\n", ThisSupportedRsrcTypeId->NameSpace.ResourceTypeName));
> + DEBUG ((DEBUG_MANAGEABILITY, " Supported MajorVersion = %a\n",
> ThisSupportedRsrcTypeId->NameSpace.MajorVersion));
> + DEBUG ((DEBUG_MANAGEABILITY, " Supported MinorVersion = %a\n",
> ThisSupportedRsrcTypeId->NameSpace.MinorVersion));
> + DEBUG ((DEBUG_MANAGEABILITY, " Supported ErrataVersion = %a\n",
> ThisSupportedRsrcTypeId->NameSpace.ErrataVersion));
> + }
> +
> break;
> }
> }
> @@ -416,6 +463,35 @@ RestJsonStructureToStruct (
> return EFI_UNSUPPORTED;
> }
>
> + if (RsrcTypeIdentifier != NULL) {
> + DEBUG ((DEBUG_MANAGEABILITY, "%a: Looking for the REST JSON to C
> Structure converter:\n", __func__));
> + if (RsrcTypeIdentifier->NameSpace.ResourceTypeName != NULL) {
> + DEBUG ((DEBUG_MANAGEABILITY, " ResourceType: %a\n",
> RsrcTypeIdentifier->NameSpace.ResourceTypeName));
> + } else {
> + DEBUG ((DEBUG_MANAGEABILITY, " ResourceType: NULL"));
> + }
> +
> + if (RsrcTypeIdentifier->NameSpace.MajorVersion != NULL) {
> + DEBUG ((DEBUG_MANAGEABILITY, " MajorVersion: %a\n",
> RsrcTypeIdentifier->NameSpace.MajorVersion));
> + } else {
> + DEBUG ((DEBUG_MANAGEABILITY, " MajorVersion: NULL"));
> + }
> +
> + if (RsrcTypeIdentifier->NameSpace.MinorVersion != NULL) {
> + DEBUG ((DEBUG_MANAGEABILITY, " MinorVersion: %a\n",
> RsrcTypeIdentifier->NameSpace.MinorVersion));
> + } else {
> + DEBUG ((DEBUG_MANAGEABILITY, " MinorVersion: NULL"));
> + }
> +
> + if (RsrcTypeIdentifier->NameSpace.ErrataVersion != NULL) {
> + DEBUG ((DEBUG_MANAGEABILITY, " ErrataVersion: %a\n",
> RsrcTypeIdentifier->NameSpace.ErrataVersion));
> + } else {
> + DEBUG ((DEBUG_MANAGEABILITY, " ErrataVersion: NULL"));
> + }
> + } else {
> + DEBUG ((DEBUG_MANAGEABILITY, "%a: RsrcTypeIdentifier is given as
> + NULL, go through all of the REST JSON to C structure interpreters.\n",
> + __func__)); }
> +
> Status = EFI_SUCCESS;
> Instance = (REST_JSON_STRUCTURE_INSTANCE *)GetFirstNode
> (&mRestJsonStructureList);
> while (TRUE) {
> @@ -427,10 +503,12 @@ RestJsonStructureToStruct (
> JsonStructure
> );
> if (!EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_MANAGEABILITY, "%a: REST JSON to C structure is
> + interpreted successfully.\n", __func__));
> break;
> }
>
> if (IsNodeAtEnd (&mRestJsonStructureList, &Instance-
> >NextRestJsonStructureInstance)) {
> + DEBUG ((DEBUG_ERROR, "%a: No REST JSON to C structure interpreter
> + found.\n", __func__));
> Status = EFI_UNSUPPORTED;
> break;
> }
> @@ -483,6 +561,7 @@ RestJsonStructureDestroyStruct (
> }
>
> if (IsNodeAtEnd (&mRestJsonStructureList, &Instance-
> >NextRestJsonStructureInstance)) {
> + DEBUG ((DEBUG_ERROR, "%a: No REST JSON to C structure interpreter
> + found.\n", __func__));
> Status = EFI_UNSUPPORTED;
> break;
> }
> @@ -512,8 +591,9 @@ RestJsonStructureToJson (
> OUT CHAR8 **ResourceRaw
> )
> {
> - EFI_STATUS Status;
> - REST_JSON_STRUCTURE_INSTANCE *Instance;
> + EFI_STATUS Status;
> + REST_JSON_STRUCTURE_INSTANCE *Instance;
> + EFI_REST_JSON_RESOURCE_TYPE_IDENTIFIER *RsrcTypeIdentifier;
>
> if ((This == NULL) || (RestJSonHeader == NULL) || (ResourceRaw == NULL)) {
> return EFI_INVALID_PARAMETER;
> @@ -523,6 +603,13 @@ RestJsonStructureToJson (
> return EFI_UNSUPPORTED;
> }
>
> + RsrcTypeIdentifier = &RestJSonHeader->JsonRsrcIdentifier;
> + DEBUG ((DEBUG_MANAGEABILITY, "Looking for the REST C Structure to
> + JSON resource converter:\n")); DEBUG ((DEBUG_MANAGEABILITY, "
> + ResourceType : %a\n",
> + RsrcTypeIdentifier->NameSpace.ResourceTypeName));
> + DEBUG ((DEBUG_MANAGEABILITY, " MajorVersion : %a\n",
> + RsrcTypeIdentifier->NameSpace.MajorVersion));
> + DEBUG ((DEBUG_MANAGEABILITY, " MinorVersion : %a\n",
> + RsrcTypeIdentifier->NameSpace.MinorVersion));
> + DEBUG ((DEBUG_MANAGEABILITY, " ErrataVersion: %a\n",
> + RsrcTypeIdentifier->NameSpace.ErrataVersion));
> +
> Status = EFI_SUCCESS;
> Instance = (REST_JSON_STRUCTURE_INSTANCE *)GetFirstNode
> (&mRestJsonStructureList);
> while (TRUE) {
> @@ -537,6 +624,7 @@ RestJsonStructureToJson (
> }
>
> if (IsNodeAtEnd (&mRestJsonStructureList, &Instance-
> >NextRestJsonStructureInstance)) {
> + DEBUG ((DEBUG_ERROR, "%a: No REST C structure to JSON interpreter
> + found.\n", __func__));
> Status = EFI_UNSUPPORTED;
> break;
> }
> @@ -587,7 +675,7 @@ RestJsonStructureEntryPoint ( }
>
> /**
> - This is the unload handle for Redfish discover module.
> + This is the unload handle for REST JSON to C structure module.
>
> Disconnect the driver specified by ImageHandle from all the devices in the
> handle database.
> Uninstall all the protocols installed in the driver entry point.
> --
> 2.37.1.windows.1