Thanks for your review comments, Abner! Updated in version 2 patch-set. Regards, Nickle
> -----Original Message----- > From: Chang, Abner <abner.ch...@amd.com> > Sent: Monday, May 15, 2023 11:20 AM > To: Nickle Wang <nick...@nvidia.com>; devel@edk2.groups.io > Cc: Igor Kulchytskyy <ig...@ami.com> > Subject: RE: [edk2-redfish-client][PATCH 2/3] RedfishClientPkg: Add Redfish > Resource Addendum Library > > External email: Use caution opening links or attachments > > > [AMD Official Use Only - General] > > Hi Nickle, > I read though this patch and was confused by function naming and the function > description in the function header. I think we have to revise it and make the > description of functionality clear for user to follow. > > I apologize if we ever discussed this before however I don't have the > synchronous > mind now. > > 1. Protocol definitions > This protocol is provided by platform that would like to add an additional > Redfish > resource in "Oem" property or to add other platform specific property in the > name other than "Oem". > Both of two protocol interfaces give platform an chance to add "OEM" or > other > Redfish property, however the naming of function prototypes are not > consistent. > The naming of "OemCallback" and "ProvisioningCallback" are also not > consistent. > > How do you think the naming below, > > struct _EDKII_REDFISH_RESOURCE_ADDENDUM_PROTOCOL { > UINT64 Revision; > EDKII_REDFISH_RESOURCE_ADDENDUM_OEM > AddAddendumOemRedfishResource; ///< > EDKII_REDFISH_RESOURCE_ADDENDUM_OTHER > AddAddendumOtherRedfishResource; ///< > }; > Would above look simple and clear? Also, please add some comments to these > two functions if you are agree with this change. > > 2. Function naming in the library, > RedfishGetOemData -> RedfishGetAddendumOemData > RedfishGetAddendumData-> RedfishGetAddendumOtherData > > 3. The current description in function header looks to me not quite clear. > Please > elaborates some more functionality in the function header and parameters > > 4. Shall we have to update readme.md? > > Thanks > Abner > > > > -----Original Message----- > > From: Nickle Wang <nick...@nvidia.com> > > Sent: Friday, May 12, 2023 3:23 PM > > To: devel@edk2.groups.io > > Cc: Chang, Abner <abner.ch...@amd.com>; Igor Kulchytskyy > > <ig...@ami.com> > > Subject: [edk2-redfish-client][PATCH 2/3] RedfishClientPkg: Add > > Redfish Resource Addendum Library > > > > Caution: This message originated from an External Source. Use proper > > caution when opening attachments, clicking links, or responding. > > > > > > Implement RedfishAddendumLib to support Redfish Resource Addendum > > Protocol. > > Feature driver calls this library to get addendum data before > > providing data to Redfish service > > > > Signed-off-by: Nickle Wang <nick...@nvidia.com> > > Cc: Abner Chang <abner.ch...@amd.com> > > Cc: Igor Kulchytskyy <ig...@ami.com> > > --- > > RedfishClientPkg/RedfishClientLibs.dsc.inc | 4 +- > > RedfishClientPkg/RedfishClientPkg.dsc | 2 + > > .../RedfishAddendumLib/RedfishAddendumLib.inf | 40 +++ > > .../Include/Library/RedfishAddendumLib.h | 65 +++++ > > .../RedfishAddendumLib/RedfishAddendumLib.c | 262 > > ++++++++++++++++++ > > 5 files changed, 372 insertions(+), 1 deletion(-) create mode 100644 > > RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.inf > > create mode 100644 > > RedfishClientPkg/Include/Library/RedfishAddendumLib.h > > create mode 100644 > > RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.c > > > > diff --git a/RedfishClientPkg/RedfishClientLibs.dsc.inc > > b/RedfishClientPkg/RedfishClientLibs.dsc.inc > > index fe0c4b08..5ea38015 100644 > > --- a/RedfishClientPkg/RedfishClientLibs.dsc.inc > > +++ b/RedfishClientPkg/RedfishClientLibs.dsc.inc > > @@ -2,10 +2,11 @@ > > # Redfish DSC include file for [LibraryClasses*] section of all > > Architectures. > > # > > # This file can be included to the [LibraryClasses*] section(s) of a > > platform DSC file -# by using "!include RedfishPkg/RedfisLibs.dsc.inc" > > to specify the library instances > > +# by using "!include RedfishPkg/RedfishLibs.dsc.inc" to specify the > > +library > > instances > > # of EDKII network library classes. > > # > > # (C) Copyright 2021-2022 Hewlett Packard Enterprise Development > > LP<BR> > > +# Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights > > reserved. > > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > # > > @@ -37,3 +38,4 @@ > > > > EdkIIRedfishResourceConfigLib|RedfishClientPkg/Library/EdkIIRedfishRes > > EdkIIRedfishResourceConfigLib|ou > > rceConfigLib/EdkIIRedfishResourceConfigLib.inf > > > > RedfishEventLib|RedfishClientPkg/Library/RedfishEventLib/RedfishEventL > > RedfishEventLib|ib.i > > nf > > > > RedfishVersionLib|RedfishClientPkg/Library/RedfishVersionLib/RedfishVe > > RedfishVersionLib|rsi > > onLib.inf > > + > > RedfishAddendumLib|RedfishClientPkg/Library/RedfishAddendumLib/Redfi > > shAddendumLib.inf > > diff --git a/RedfishClientPkg/RedfishClientPkg.dsc > > b/RedfishClientPkg/RedfishClientPkg.dsc > > index 2b2149cc..d3b645b6 100644 > > --- a/RedfishClientPkg/RedfishClientPkg.dsc > > +++ b/RedfishClientPkg/RedfishClientPkg.dsc > > @@ -2,6 +2,7 @@ > > # Redfish Client Package > > # > > # (C) Copyright 2021 Hewlett-Packard Enterprise Development LP. > > +# Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights > > reserved. > > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > # > > @@ -49,5 +50,6 @@ > > > > > > RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilit > > yLib.i > > nf > > RedfishClientPkg/PrivateLibrary/RedfishLib/RedfishLib.inf > > + RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.inf > > > > !include RedfishClientPkg/RedfishClient.dsc.inc > > diff --git > > a/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.inf > > b/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.inf > > new file mode 100644 > > index 00000000..1ecfaa69 > > --- /dev/null > > +++ > > b/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.inf > > @@ -0,0 +1,40 @@ > > +## @file > > +# > > +# Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All > > +rights > > reserved. > > +# > > +# SPDX-License-Identifier: BSD-2-Clause-Patent # ## > > + > > +[Defines] > > + INF_VERSION = 0x00010006 > > + BASE_NAME = RedfishAddendumLib > > + FILE_GUID = 7B227D39-746D-4247-8291-27B0FA79A7AF > > + MODULE_TYPE = DXE_DRIVER > > + VERSION_STRING = 1.0 > > + LIBRARY_CLASS = RedfishAddendumLib| DXE_DRIVER > > UEFI_DRIVER > > + > > +# > > +# VALID_ARCHITECTURES = IA32 X64 EBC > > +# > > + > > +[Sources] > > + RedfishAddendumLib.c > > + > > +[Packages] > > + MdePkg/MdePkg.dec > > + MdeModulePkg/MdeModulePkg.dec > > + RedfishPkg/RedfishPkg.dec > > + RedfishClientPkg/RedfishClientPkg.dec > > + > > +[LibraryClasses] > > + BaseLib > > + DebugLib > > + UefiBootServicesTableLib > > + MemoryAllocationLib > > + UefiLib > > + JsonLib > > + > > +[Protocols] > > + gEdkIIRedfishResourceAddendumProtocolGuid ## CONSUMED ## > > + > > diff --git a/RedfishClientPkg/Include/Library/RedfishAddendumLib.h > > b/RedfishClientPkg/Include/Library/RedfishAddendumLib.h > > new file mode 100644 > > index 00000000..d77347fd > > --- /dev/null > > +++ b/RedfishClientPkg/Include/Library/RedfishAddendumLib.h > > @@ -0,0 +1,65 @@ > > +/** @file > > + This file defines the Redfish addendum library interface. > > + > > + Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All > > + rights > > reserved. > > + > > + SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > + > > +#ifndef REDFISH_ADDENDUM_LIB_H_ > > +#define REDFISH_ADDENDUM_LIB_H_ > > + > > +#include <Uefi.h> > > +#include <Library/JsonLib.h> > > +#include <Protocol/EdkIIRedfishResourceAddendumProtocol.h> > > + > > +/** > > + Provide redfish resource with addendum data in given schema. > > + > > + @param[in] Uri Uri of input resource. > > + @param[in] Schema Redfish schema string. > > + @param[in] Version Schema version string. > > + @param[in] JsonText Input resource in JSON format string. > > + @param[out] JsonWithAddendum The input resource with addendum > > value attached. > > + > > + @retval EFI_SUCCESS Addendum data is attached. > > + @retval EFI_NOT_FOUND No addendum protocol is found in system. > > + @retval EFI_UNSUPPORTED No addendum data is required in given > > schema. > > + @retval Others Some error happened. > > + > > +**/ > > +EFI_STATUS > > +RedfishGetAddendumData ( > > + IN EFI_STRING Uri, > > + IN CHAR8 *Schema, > > + IN CHAR8 *Version, > > + IN CHAR8 *JsonText, > > + OUT CHAR8 **JsonWithAddendum > > + ); > > + > > +/** > > + Provide redfish OEM resource with given schema information. > > + > > + @param[in] Uri Uri of input resource. > > + @param[in] Schema Redfish schema string. > > + @param[in] Version Schema version string. > > + @param[in] JsonText Input resource in JSON format string. > > + @param[out] JsonWithOem The input resource with OEM value > > attached. > > + > > + @retval EFI_SUCCESS OEM data is attached. > > + @retval EFI_NOT_FOUND No addendum protocol is found in system. > > + @retval EFI_UNSUPPORTED No OEM data is required in given schema. > > + @retval Others Some error happened. > > + > > +**/ > > +EFI_STATUS > > +RedfishGetOemData ( > > + IN EFI_STRING Uri, > > + IN CHAR8 *Schema, > > + IN CHAR8 *Version, > > + IN CHAR8 *JsonText, > > + OUT CHAR8 **JsonWithOem > > + ); > > + > > +#endif > > diff --git > > a/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.c > > b/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.c > > new file mode 100644 > > index 00000000..6aa6e3cc > > --- /dev/null > > +++ > > b/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.c > > @@ -0,0 +1,262 @@ > > +/** @file > > + Redfish addendum library helps Redfish application to get addendum > > +data > > and OEM > > + data from platform driver. > > + > > + Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All > > + rights > > reserved. > > + > > + SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > + > > +#include <Uefi.h> > > +#include <RedfishBase.h> > > + > > +#include <Library/UefiLib.h> > > +#include <Library/BaseLib.h> > > +#include <Library/DebugLib.h> > > +#include <Library/UefiBootServicesTableLib.h> > > +#include <Library/MemoryAllocationLib.h> #include > > +<Library/RedfishAddendumLib.h> > > + > > +/** > > + > > + Convert Unicode string to ASCII string. It's call responsibility to > > + release > > returned buffer. > > + > > + @param[in] UnicodeStr Unicode string to convert. > > + > > + @retval CHAR8 * ASCII string returned. > > + @retval NULL Errors occur. > > + > > +**/ > > +CHAR8 * > > +UnicodeToAscii ( > > + IN EFI_STRING UnicodeStr > > + ) > > +{ > > + CHAR8 *AsciiStr; > > + UINTN AsciiStrSize; > > + EFI_STATUS Status; > > + > > + if (IS_EMPTY_STRING (UnicodeStr)) { > > + return NULL; > > + } > > + > > + AsciiStrSize = StrLen (UnicodeStr) + 1; > > + AsciiStr = AllocatePool (AsciiStrSize); > > + if (AsciiStr == NULL) { > > + return NULL; > > + } > > + > > + Status = UnicodeStrToAsciiStrS (UnicodeStr, AsciiStr, > > + AsciiStrSize); if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_ERROR, "UnicodeStrToAsciiStrS failed: %r\n", Status)); > > + FreePool (AsciiStr); > > + return NULL; > > + } > > + > > + return AsciiStr; > > +} > > + > > +/** > > + Provide redfish resource with addendum data in given schema. > > + > > + @param[in] Uri Uri of input resource. > > + @param[in] Schema Redfish schema string. > > + @param[in] Version Schema version string. > > + @param[in] JsonText Input resource in JSON format string. > > + @param[out] JsonWithAddendum The input resource with addendum > > value attached. > > + It is the caller's responsibility to free > > this buffer. > > + > > + @retval EFI_SUCCESS Addendum data is attached. > > + @retval EFI_NOT_FOUND No addendum protocol is found in system. > > + @retval EFI_UNSUPPORTED No addendum data is required in given > > schema. > > + @retval Others Some error happened. > > + > > +**/ > > +EFI_STATUS > > +RedfishGetAddendumData ( > > + IN EFI_STRING Uri, > > + IN CHAR8 *Schema, > > + IN CHAR8 *Version, > > + IN CHAR8 *JsonText, > > + OUT CHAR8 **JsonWithAddendum > > + ) > > +{ > > + REDFISH_RESOURCE_SCHEMA_INFO SchemaInfo; > > + EDKII_JSON_VALUE JsonValue; > > + EFI_STATUS Status; > > + EFI_HANDLE *HandleBuffer; > > + UINTN NumberOfHandles; > > + UINTN Index; > > + EDKII_REDFISH_RESOURCE_ADDENDUM_PROTOCOL *Protocol; > > + > > + if (IS_EMPTY_STRING (Uri) || IS_EMPTY_STRING (Schema) || > > IS_EMPTY_STRING (Version) || IS_EMPTY_STRING (JsonText) || > > (JsonWithAddendum == NULL)) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + *JsonWithAddendum = NULL; > > + SchemaInfo.Uri = UnicodeToAscii (Uri); > > + SchemaInfo.Schema = Schema; > > + SchemaInfo.Version = Version; > > + NumberOfHandles = 0; > > + HandleBuffer = NULL; > > + > > + JsonValue = JsonLoadString (JsonText, 0, NULL); if ((JsonValue == > > + NULL) || !JsonValueIsObject (JsonValue)) { > > + return EFI_OUT_OF_RESOURCES; > > + } > > + > > + Status = gBS->LocateHandleBuffer ( > > + ByProtocol, > > + &gEdkIIRedfishResourceAddendumProtocolGuid, > > + NULL, > > + &NumberOfHandles, > > + &HandleBuffer > > + ); > > + if (EFI_ERROR (Status)) { > > + goto ON_RELEASE; > > + } > > + > > + for (Index = 0; Index < NumberOfHandles; Index++) { > > + Status = gBS->HandleProtocol ( > > + HandleBuffer[Index], > > + &gEdkIIRedfishResourceAddendumProtocolGuid, > > + (VOID **)&Protocol > > + ); > > + if (EFI_ERROR (Status)) { > > + continue; > > + } > > + > > + Status = Protocol->ProvisioningCallback (Protocol, &SchemaInfo, > > JsonValue); > > + if (!EFI_ERROR (Status)) { > > + *JsonWithAddendum = JsonDumpString (JsonValue, > > EDKII_JSON_COMPACT); > > + break; > > + } > > + } > > + > > +ON_RELEASE: > > + > > + if (HandleBuffer != NULL) { > > + FreePool (HandleBuffer); > > + } > > + > > + if (JsonValue != NULL) { > > + JsonValueFree (JsonValue); > > + } > > + > > + if (SchemaInfo.Uri != NULL) { > > + FreePool (SchemaInfo.Uri); > > + } > > + > > + return Status; > > +} > > + > > +/** > > + Provide redfish OEM resource with given schema information. > > + > > + @param[in] Uri Uri of input resource. > > + @param[in] Schema Redfish schema string. > > + @param[in] Version Schema version string. > > + @param[in] JsonText Input resource in JSON format string. > > + @param[out] JsonWithOem The input resource with OEM value > > attached. > > + > > + @retval EFI_SUCCESS OEM data is attached. > > + @retval EFI_NOT_FOUND No addendum protocol is found in system. > > + @retval EFI_UNSUPPORTED No OEM data is required in given schema. > > + @retval Others Some error happened. > > + > > +**/ > > +EFI_STATUS > > +RedfishGetOemData ( > > + IN EFI_STRING Uri, > > + IN CHAR8 *Schema, > > + IN CHAR8 *Version, > > + IN CHAR8 *JsonText, > > + OUT CHAR8 **JsonWithOem > > + ) > > +{ > > + REDFISH_RESOURCE_SCHEMA_INFO SchemaInfo; > > + EDKII_JSON_VALUE JsonValue; > > + EDKII_JSON_VALUE JsonValueOem; > > + EFI_STATUS Status; > > + EFI_HANDLE *HandleBuffer; > > + UINTN NumberOfHandles; > > + UINTN Index; > > + EDKII_REDFISH_RESOURCE_ADDENDUM_PROTOCOL *Protocol; > > + > > + if (IS_EMPTY_STRING (Uri) || IS_EMPTY_STRING (Schema) || > > IS_EMPTY_STRING (Version) || IS_EMPTY_STRING (JsonText) || > > (JsonWithOem == NULL)) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + *JsonWithOem = NULL; > > + SchemaInfo.Uri = UnicodeToAscii (Uri); > > + SchemaInfo.Schema = Schema; > > + SchemaInfo.Version = Version; > > + JsonValue = NULL; > > + JsonValueOem = NULL; > > + NumberOfHandles = 0; > > + HandleBuffer = NULL; > > + > > + JsonValue = JsonLoadString (JsonText, 0, NULL); if ((JsonValue == > > + NULL) || !JsonValueIsObject (JsonValue)) { > > + return EFI_OUT_OF_RESOURCES; > > + } > > + > > + JsonValueOem = JsonValueInitObject (); if ((JsonValueOem == NULL) > > + || !JsonValueIsObject (JsonValueOem)) { > > + Status = EFI_OUT_OF_RESOURCES; > > + goto ON_RELEASE; > > + } > > + > > + Status = gBS->LocateHandleBuffer ( > > + ByProtocol, > > + &gEdkIIRedfishResourceAddendumProtocolGuid, > > + NULL, > > + &NumberOfHandles, > > + &HandleBuffer > > + ); > > + if (EFI_ERROR (Status)) { > > + goto ON_RELEASE; > > + } > > + > > + for (Index = 0; Index < NumberOfHandles; Index++) { > > + Status = gBS->HandleProtocol ( > > + HandleBuffer[Index], > > + &gEdkIIRedfishResourceAddendumProtocolGuid, > > + (VOID **)&Protocol > > + ); > > + if (EFI_ERROR (Status)) { > > + continue; > > + } > > + > > + Status = Protocol->OemCallback (Protocol, &SchemaInfo, JsonValueOem); > > + if (!EFI_ERROR (Status)) { > > + Status = JsonObjectSetValue (JsonValue, "Oem", JsonValueOem); > > + if (!EFI_ERROR (Status)) { > > + *JsonWithOem = JsonDumpString (JsonValue, EDKII_JSON_COMPACT); > > + } > > + > > + break; > > + } > > + } > > + > > +ON_RELEASE: > > + > > + if (HandleBuffer != NULL) { > > + FreePool (HandleBuffer); > > + } > > + > > + if (JsonValue != NULL) { > > + JsonValueFree (JsonValue); > > + } > > + > > + if (JsonValueOem != NULL) { > > + JsonValueFree (JsonValueOem); > > + } > > + > > + if (SchemaInfo.Uri != NULL) { > > + FreePool (SchemaInfo.Uri); > > + } > > + > > + return Status; > > +} > > -- > > 2.17.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#104858): https://edk2.groups.io/g/devel/message/104858 Mute This Topic: https://groups.io/mt/98844835/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-