public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Nickle Wang" <nicklew@nvidia.com>
To: "Chang, Abner" <Abner.Chang@amd.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Igor Kulchytskyy <igork@ami.com>, Nick Ramirez <nramirez@nvidia.com>
Subject: Re: [PATCH 2/3] RedfishPkg: Redfish discover driver improvement
Date: Wed, 29 Mar 2023 02:25:00 +0000	[thread overview]
Message-ID: <MW4PR12MB7031EB8A1EE535F22F35B3D2D9899@MW4PR12MB7031.namprd12.prod.outlook.com> (raw)
In-Reply-To: <BY5PR12MB3954F9C1B4BE60E49C78FC03EA899@BY5PR12MB3954.namprd12.prod.outlook.com>

Yes, let me handle this in version 2 patch. Thanks!

Regards,
Nickle

> -----Original Message-----
> From: Chang, Abner <Abner.Chang@amd.com>
> Sent: Wednesday, March 29, 2023 10:12 AM
> To: Nickle Wang <nicklew@nvidia.com>; devel@edk2.groups.io
> Cc: Igor Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@nvidia.com>
> Subject: RE: [PATCH 2/3] RedfishPkg: Redfish discover driver improvement
> 
> External email: Use caution opening links or attachments
> 
> 
> [AMD Official Use Only - General]
> 
> Hi Nickle,
> I found that we had defined the Host IP address format in
> Redfish\Incluode\IndustryStandard\RedfishHostInterface.h
> 
> #define REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN
> 0x00
> #define REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP4      0x01
> #define REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP6      0x02
> 
> Could you please help to update the references of HostIpAddressFormat to use
> above macros?
> Thanks
> Abner
> 
> > -----Original Message-----
> > From: Nickle Wang <nicklew@nvidia.com>
> > Sent: Tuesday, March 28, 2023 1:59 PM
> > To: devel@edk2.groups.io
> > Cc: Chang, Abner <Abner.Chang@amd.com>; Igor Kulchytskyy
> > <igork@ami.com>; Nick Ramirez <nramirez@nvidia.com>
> > Subject: [PATCH 2/3] RedfishPkg: Redfish discover driver improvement
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > Bug fix:
> > - function stack fault
> > - properly handle "SubnetAddrInfoIPv6" when there is no IPv6 support
> > - copy-n-paste error in RedfishGetHostInterfaceProtocolData()
> > - fix typo
> > Enhancement:
> > - Redfish discover driver now can configure host IP address based on
> >   the information from SMBIOS type 42 record. This saves the effort of
> >   configuring host IP address in setup menu.
> > - Performance improvement to driver binding process. Redfish discover
> >   driver will wait until all required drivers are ready and do driver
> >   binding start().
> > - Use CopyGuid() to copy GUID instead of intrinsic function.
> > - Error handling when SMBIOS data is corrupted.
> >
> > Signed-off-by: Nickle Wang <nicklew@nvidia.com>
> > Cc: Abner Chang <abner.chang@amd.com>
> > Cc: Igor Kulchytskyy <igork@ami.com>
> > Cc: Nick Ramirez <nramirez@nvidia.com>
> > ---
> >  .../RedfishDiscoverInternal.h                 |  20 ++-
> >  .../RedfishDiscoverDxe/RedfishDiscoverDxe.c   | 142 ++++++++++++------
> >  .../RedfishSmbiosHostInterface.c              |  13 +-
> >  3 files changed, 119 insertions(+), 56 deletions(-)
> >
> > diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> > b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> > index 04fa09e1cc..2704cd955d 100644
> > --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> > +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> > @@ -3,6 +3,7 @@
> >
> >    (C) Copyright 2021 Hewlett Packard Enterprise Development LP<BR>
> >    Copyright (c) 2022, AMD Incorporated. All rights reserved.
> > +  Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> >
> >    SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -102,7 +103,7 @@ typedef struct {
> >    UINT32                                         SubnetAddrInfoIPv6Number;   ///< IPv6
> > address info number.
> >    EFI_IP6_ADDRESS_INFO                           *SubnetAddrInfoIPv6;        ///< IPv6
> > address info.
> >    //
> > -  // Network interface protocol and REST EX infor.
> > +  // Network interface protocol and REST EX info.
> >    //
> >    UINT32                                         NetworkProtocolType;          ///< Network
> > protocol type. Refer to
> >
> > ///< NETWORK_INTERFACE_PROTOCOL_TYPE.
> > @@ -112,7 +113,7 @@ typedef struct {
> >    // EFI_REDFISH_DISCOVER_PROTOCOL instance installed
> >    // on this network interface.
> >    //
> > -  EFI_HANDLE                                     EfiRedfishDiscoverProtocolHandle; ///<
> > EFI_REDFISH_DISCOVER_PROTOTOCOL instance installed
> > +  EFI_HANDLE                                     EfiRedfishDiscoverProtocolHandle; ///<
> > EFI_REDFISH_DISCOVER_PROTOCOL instance installed
> >                                                                                     ///< on this network interface.
> >  } EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL;
> >
> > @@ -123,7 +124,7 @@ typedef struct {
> >    LIST_ENTRY              Entry;                      ///< Link list entry.
> >    EFI_HANDLE              OpenDriverAgentHandle;      ///< The agent to open
> > network protocol.
> >    EFI_HANDLE              OpenDriverControllerHandle; ///< The controller handle
> > to open network protocol.
> > -  EFI_HANDLE              RestExChildHandle;          ///< The child handle created
> > throught REST EX Service Protocol.
> > +  EFI_HANDLE              RestExChildHandle;          ///< The child handle created
> > through REST EX Service Protocol.
> >    EFI_HANDLE              RestExControllerHandle;     ///< The controller handle
> > which provide REST EX protocol.
> >    EFI_REST_EX_PROTOCOL    *RestExProtocolInterface;   ///< Pointer to
> > EFI_REST_EX_PROTOCOL.
> >    UINT32                  RestExId;                   ///< The identifier installed on REST EX
> > controller handle.
> > @@ -178,15 +179,18 @@ typedef struct {
> >    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> > *NetworkInterface; ///<
> > EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> >
> > ///< instance used to discover Redfish service.
> >    //
> > -  // Below for Host insterface discovery.
> > +  // Below for Host interface discovery.
> >    //
> >    BOOLEAN                                            HostIntfValidation; ///< Indicates whether
> > to validate Redfish Host interface.
> >    EFI_IP_ADDRESS                                     TargetIpAddress;    ///< Target IP
> > address reported in Redfish Host interface.
> > +  UINT8                                              HostAddrFormat;     ///< Unknown=00h,
> > Ipv4=01h, Ipv6=02h.
> > +  EFI_IP_ADDRESS                                     HostIpAddress;      ///< Host IP address
> > reported in Redfish Host interface.
> > +  EFI_IP_ADDRESS                                     HostSubnetMask;     ///< Host subnet
> > mask address reported in Redfish Host interface.
> >  } EFI_REDFISH_DISCOVERED_INTERNAL_INSTANCE;
> >
> >  /**
> > -  The function adds a new foudn Redfish service to internal list and
> > -  notify clinet.
> > +  The function adds a new found Redfish service to internal list and
> > + notify client.
> >
> >    It simply frees the packet.
> >
> > @@ -197,7 +201,7 @@ typedef struct {
> >    @param[in]  Os                    OS string.
> >    @param[in]  OsVer                 OS version string.
> >    @param[in]  Product               Product string.
> > -  @param[in]  ProductVer            Product verison string.
> > +  @param[in]  ProductVer            Product version string.
> >    @param[in]  UseHttps              Redfish service requires secured connection.
> >    @retval EFI_SUCCESS               Redfish service is added to list successfully.
> >
> > @@ -224,7 +228,7 @@ AddAndSignalNewRedfishService (
> >    @param[out] DeviceDescriptor Pointer to REDFISH_INTERFACE_DATA.
> >    @param[out] ProtocolData     Pointer to
> > REDFISH_OVER_IP_PROTOCOL_DATA.
> >
> > -  @retval EFI_SUCCESS    Get host interface succesfully.
> > +  @retval EFI_SUCCESS    Get host interface successfully.
> >    @retval Otherwise      Fail to tet host interface.
> >
> >  **/
> > diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > index 042d6d5fd5..ba42c8a5de 100644
> > --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > @@ -1,9 +1,10 @@
> >  /** @file
> >
> > -  The implementation of EFI Redfidh Discover Protocol.
> > +  The implementation of EFI Redfish Discover Protocol.
> >
> >    (C) Copyright 2021 Hewlett Packard Enterprise Development LP<BR>
> >    Copyright (c) 2022, AMD Incorporated. All rights reserved.
> > +  Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> >
> >    SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -270,10 +271,13 @@ Tcp6GetSubnetInfo (
> >
> >    if (IpModedata.AddressCount == 0) {
> >      DEBUG ((DEBUG_INFO, "%a: No IPv6 address configured.\n",
> > __FUNCTION__));
> > +    Instance->SubnetAddrInfoIPv6Number = 0;
> > +    return EFI_SUCCESS;
> >    }
> >
> >    if (Instance->SubnetAddrInfoIPv6 != NULL) {
> >      FreePool (Instance->SubnetAddrInfoIPv6);
> > +    Instance->SubnetAddrInfoIPv6 = NULL;
> >    }
> >
> >    Instance->SubnetAddrInfoIPv6 = AllocateZeroPool
> > (IpModedata.AddressCount * sizeof (EFI_IP6_ADDRESS_INFO)); @@ -447,7
> > +451,7 @@ NumberOfNetworkInterface (
> >
> >  /**
> >    This function checks the  IP version supported on this
> > -  netwoek interface.
> > +  network interface.
> >
> >    @param[in]    ThisNetworkInterface
> > EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> >
> > @@ -472,7 +476,7 @@ CheckIsIpVersion6 (
> >    @param[in]    Instance     EFI_REDFISH_DISCOVERED_INTERNAL_INSTANCE
> >
> >    @retval EFI_SUCCESS        Redfish service is discovered through SMBIOS
> > Host interface.
> > -  @retval Others             Fail to discover Redfish service throught SMBIOS host
> > interface
> > +  @retval Others             Fail to discover Redfish service through SMBIOS host
> > interface
> >
> >  **/
> >  EFI_STATUS
> > @@ -487,7 +491,7 @@ DiscoverRedfishHostInterface (
> >    CHAR16                         Ipv6Str[sizeof "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff" + 1];
> >    CHAR8                          RedfishServiceLocateStr[sizeof
> > "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff" + 1];
> >    UINTN                          StrSize;
> > -  UINTN                          MacCompareStstus;
> > +  UINTN                          MacCompareStatus;
> >    BOOLEAN                        IsHttps;
> >
> >    Data             = NULL;
> > @@ -503,24 +507,52 @@ DiscoverRedfishHostInterface (
> >    Status = RedfishGetHostInterfaceProtocolData (mSmbios,
> > &DeviceDescriptor, &Data); // Search for SMBIOS type 42h
> >    if (!EFI_ERROR (Status) && (Data != NULL) && (DeviceDescriptor != NULL)) {
> >      //
> > -    // Chceck if we can reach out Redfish service using this network interface.
> > -    // Check with MAC address using Device Descroptor Data Device Type 04
> > and Type 05.
> > +    // Check if we can reach out Redfish service using this network interface.
> > +    // Check with MAC address using Device Descriptor Data Device
> > + Type 04
> > and Type 05.
> >      // Those two types of Redfish host interface device has MAC information.
> >      //
> >      if (DeviceDescriptor->DeviceType ==
> > REDFISH_HOST_INTERFACE_DEVICE_TYPE_PCI_PCIE_V2) {
> > -      MacCompareStstus = CompareMem (&Instance->NetworkInterface-
> > >MacAddress, &DeviceDescriptor-
> > >DeviceDescriptor.PciPcieDeviceV2.MacAddress, 6);
> > +      MacCompareStatus = CompareMem
> > + (&Instance->NetworkInterface->MacAddress,
> > + &DeviceDescriptor->DeviceDescriptor.PciPcieDeviceV2.MacAddress, 6);
> >      } else if (DeviceDescriptor->DeviceType ==
> > REDFISH_HOST_INTERFACE_DEVICE_TYPE_USB_V2) {
> > -      MacCompareStstus = CompareMem (&Instance->NetworkInterface-
> > >MacAddress, &DeviceDescriptor-
> > >DeviceDescriptor.UsbDeviceV2.MacAddress, 6);
> > +      MacCompareStatus = CompareMem
> > + (&Instance->NetworkInterface->MacAddress,
> > + &DeviceDescriptor->DeviceDescriptor.UsbDeviceV2.MacAddress, 6);
> >      } else {
> >        return EFI_UNSUPPORTED;
> >      }
> >
> > -    if (MacCompareStstus != 0) {
> > +    if (MacCompareStatus != 0) {
> >        return EFI_UNSUPPORTED;
> >      }
> >
> > +    Instance->HostAddrFormat = Data->HostIpAddressFormat;
> > +    if (Data->HostIpAddressFormat == 1) {
> > +      IP4_COPY_ADDRESS ((VOID *)&Instance->HostIpAddress.v4, (VOID
> > *)Data->HostIpAddress);
> > +      IP4_COPY_ADDRESS ((VOID *)&Instance->HostSubnetMask.v4, (VOID
> > + *)Data->HostIpMask);
> > +
> > +      if (EFI_IP4_EQUAL (&Instance->HostIpAddress.v4, &mZeroIp4Addr)) {
> > +        DEBUG ((DEBUG_ERROR, "%a: invalid host IP address: zero
> > + address\n",
> > __FUNCTION__));
> > +        //
> > +        // Invalid IP address detected. Change address format to
> > + Unknown and
> > use system default address.
> > +        //
> > +        Instance->HostAddrFormat = 0x00;
> > +      }
> > +
> > +      if (!IP4_IS_VALID_NETMASK (EFI_IP4
> > + (Instance->HostSubnetMask.v4)))
> > {
> > +        DEBUG ((DEBUG_ERROR, "%a: invalid subnet mask address\n",
> > __FUNCTION__));
> > +        //
> > +        // Invalid subnet mast address detected. Change address
> > + format to
> > Unknown and use system default address.
> > +        //
> > +        Instance->HostAddrFormat = 0x00;
> > +      }
> > +    } else if (Data->HostIpAddressFormat == 2) {
> > +      IP6_COPY_ADDRESS ((VOID *)&Instance->HostIpAddress.v6, (VOID
> > *)Data->HostIpAddress);
> > +    }
> > +
> >      if (Data->RedfishServiceIpAddressFormat == 1) {
> >        IP4_COPY_ADDRESS ((VOID *)&Instance->TargetIpAddress.v4, (VOID
> > *)Data->RedfishServiceIpAddress);
> > +
> > +      if (EFI_IP4_EQUAL (&Instance->TargetIpAddress.v4, &mZeroIp4Addr)) {
> > +        DEBUG ((DEBUG_ERROR, "%a: invalid service IP address: zero
> > address\n", __FUNCTION__));
> > +      }
> >      } else {
> >        IP6_COPY_ADDRESS ((VOID *)&Instance->TargetIpAddress.v6, (VOID
> > *)Data->RedfishServiceIpAddress);
> >      }
> > @@ -530,7 +562,7 @@ DiscoverRedfishHostInterface (
> >        Status = EFI_UNSUPPORTED;
> >      } else {
> >        //
> > -      // Add this istance to list without detial information of Redfish
> > +      // Add this instance to list without detail information of
> > + Redfish
> >        // service.
> >        //
> >        IsHttps = FALSE;
> > @@ -614,7 +646,7 @@ DiscoverRedfishHostInterface (
> >    @param[in]  Os                    OS string.
> >    @param[in]  OsVer                 OS version string.
> >    @param[in]  Product               Product string.
> > -  @param[in]  ProductVer            Product verison string.
> > +  @param[in]  ProductVer            Product version string.
> >    @param[in]  UseHttps              Redfish service requires secured connection.
> >    @retval EFI_SUCCESS               Redfish service is added to list successfully.
> >
> > @@ -671,7 +703,7 @@ AddAndSignalNewRedfishService (
> >      do {
> >        if ((Char16Uuid == NULL) ||
> > (DiscoveredList->Instance->Information.Uuid
> > == NULL)) {
> >          //
> > -        // Check if this Redfish instance already found using IP addrress.
> > +        // Check if this Redfish instance already found using IP address.
> >          //
> >          if (!CheckIsIpVersion6 (NetworkInterface)) {
> >            if (CompareMem (
> > @@ -849,6 +881,10 @@ AddAndSignalNewRedfishService (
> >            Status = EFI_OUT_OF_RESOURCES;
> >            goto EXIT_FREE_CONFIG_DATA;
> >          }
> > +
> > +        if (Instance->HostAddrFormat == 2) {
> > +          IP6_COPY_ADDRESS (&RestExHttpConfigData-
> > >HttpConfigData.AccessPoint.IPv6Node->LocalAddress, &Instance-
> > >HostIpAddress.v6);
> > +        }
> >        } else {
> >          RestExHttpConfigData->HttpConfigData.AccessPoint.IPv4Node =
> > AllocateZeroPool (sizeof (EFI_HTTPv4_ACCESS_POINT));
> >          if (RestExHttpConfigData->HttpConfigData.AccessPoint.IPv4Node
> > ==
> > NULL) { @@ -856,7 +892,13 @@ AddAndSignalNewRedfishService (
> >            goto EXIT_FREE_CONFIG_DATA;
> >          }
> >
> > -        RestExHttpConfigData->HttpConfigData.AccessPoint.IPv4Node-
> > >UseDefaultAddress = TRUE;
> > +        if (Instance->HostAddrFormat == 1) {
> > +          RestExHttpConfigData->HttpConfigData.AccessPoint.IPv4Node-
> > >UseDefaultAddress = FALSE;
> > +          IP4_COPY_ADDRESS (&RestExHttpConfigData-
> > >HttpConfigData.AccessPoint.IPv4Node->LocalAddress, &Instance-
> > >HostIpAddress.v4);
> > +          IP4_COPY_ADDRESS (&RestExHttpConfigData-
> > >HttpConfigData.AccessPoint.IPv4Node->LocalSubnet, &Instance-
> > >HostSubnetMask.v4);
> > +        } else {
> > +          RestExHttpConfigData->HttpConfigData.AccessPoint.IPv4Node-
> > >UseDefaultAddress = TRUE;
> > +        }
> >        }
> >
> >        Status = RestEx->Configure (
> > @@ -955,7 +997,7 @@ NetworkInterfaceGetSubnetInfo (
> >                                                 Instance
> >                                                 );
> >      if (EFI_ERROR (Status)) {
> > -      DEBUG ((DEBUG_ERROR, "%a:Faile to get Subnet infomation.\n",
> > __FUNCTION__));
> > +      DEBUG ((DEBUG_ERROR, "%a:Failed to get Subnet infomation.\n",
> > + __FUNCTION__));
> >        return Status;
> >      } else {
> >        DEBUG ((DEBUG_INFO, "%a:MAC address: %s\n", __FUNCTION__,
> > Instance->StrMacAddr)); @@ -982,7 +1024,7 @@
> > NetworkInterfaceGetSubnetInfo (
> >          ThisSubnetAddrInfoIPv6++;
> >          for (IPv6InfoIndex = 0; IPv6InfoIndex < Instance-
> > >SubnetAddrInfoIPv6Number - 1; IPv6InfoIndex++) {
> >            //
> > -          // Build up addtional
> > EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL instances.
> > +          // Build up additional
> > EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL instances.
> >            //
> >            NewNetworkInterface =
> > (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> > *)AllocateZeroPool (sizeof
> > (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL));
> >            if (NewNetworkInterface != NULL) { @@ -1103,9 +1145,9 @@
> > RedfishServiceGetNetworkInterface (
> >  /**
> >    This function acquires Redfish services by discovering static Redfish setting
> >    according to Redfish Host Interface or through SSDP. Returns a list
> > of EFI
> > -  handles in EFI_REDFISH_DISCOVERED_LIST. Each of EFI handle has
> > cooresponding
> > -  EFI REST EX instance installed on it. Each REST EX isntance is a
> > child instance which
> > -  created through EFI REST EX serivce protoocl for communicating with
> > specific
> > +  handles in EFI_REDFISH_DISCOVERED_LIST. Each of EFI handle has
> > + corresponding  EFI REST EX instance installed on it. Each REST EX
> > + instance is a child instance which  created through EFI REST EX
> > + service protocol for communicating with specific
> >    Redfish service.
> >
> >    @param[in]    This                    EFI_REDFISH_DISCOVER_PROTOCOL instance.
> > @@ -1255,7 +1297,7 @@ RedfishServiceAbortAcquire (
> >    )
> >  {
> >    // This function is used to abort Redfish service discovery through
> > SSDP
> > -  // on the network interface. SSDP is optionally supprted by
> > EFI_REDFISH_DISCOVER_PROTOCOL,
> > +  // on the network interface. SSDP is optionally suppoted by
> > + EFI_REDFISH_DISCOVER_PROTOCOL,
> >    // we dont have implementation for SSDP now.
> >
> >    return EFI_UNSUPPORTED;
> > @@ -1425,7 +1467,7 @@ CreateRedfishDiscoverNetworkInterface (  }
> >
> >  /**
> > -  This function destory network interface
> > +  This function destroy network interface
> >
> >
> >    @param[in]  ThisNetworkInterface
> > EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL instance.
> > @@ -1433,7 +1475,7 @@ CreateRedfishDiscoverNetworkInterface (
> >    @retval EFI_STATUS
> >  **/
> >  EFI_STATUS
> > -DestroyRedfishNetwrokInterface (
> > +DestroyRedfishNetworkInterface (
> >    IN EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> > *ThisNetworkInterface
> >    )
> >  {
> > @@ -1467,11 +1509,13 @@ TestForRequiredProtocols (
> >    IN EFI_HANDLE                   ControllerHandle
> >    )
> >  {
> > -  UINT32      Id;
> > +  UINT32      *Id;
> >    UINTN       Index;
> >    EFI_STATUS  Status;
> > +  UINTN       ListCount;
> >
> > -  for (Index = 0; Index < (sizeof (gRequiredProtocol) / sizeof
> > (REDFISH_DISCOVER_REQUIRED_PROTOCOL)); Index++) {
> > +  ListCount = (sizeof (gRequiredProtocol) / sizeof
> > + (REDFISH_DISCOVER_REQUIRED_PROTOCOL));
> > +  for (Index = 0; Index < ListCount; Index++) {
> >      Status = gBS->OpenProtocol (
> >                      ControllerHandle,
> >
> > gRequiredProtocol[Index].RequiredServiceBindingProtocolGuid,
> > @@ -1490,8 +1534,10 @@ TestForRequiredProtocols (
> >                        EFI_OPEN_PROTOCOL_GET_PROTOCOL
> >                        );
> >        if (EFI_ERROR (Status)) {
> > -        DEBUG ((DEBUG_ERROR, "%a: %s is found on this controller handle.\n",
> > __FUNCTION__, gRequiredProtocol[Index].ProtocolName));
> > -        return EFI_SUCCESS;
> > +        if (Index == ListCount - 1) {
> > +          DEBUG ((DEBUG_ERROR, "%a: all required protocols are found
> > + on this
> > controller handle: %p.\n", __FUNCTION__, ControllerHandle));
> > +          return EFI_SUCCESS;
> > +        }
> >        }
> >      }
> >    }
> > @@ -1517,7 +1563,7 @@ BuildupNetworkInterface (
> >    IN EFI_HANDLE                   ControllerHandle
> >    )
> >  {
> > -  UINT32                                           Id;
> > +  UINT32                                           *Id;
> >    UINT32                                           Index;
> >    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> > *NetworkInterface;
> >    BOOLEAN                                          IsNew;
> > @@ -1578,13 +1624,11 @@ BuildupNetworkInterface (
> >          return Status;
> >        }
> >
> > -      NetworkInterface->NetworkProtocolType                       =
> > gRequiredProtocol[Index].ProtocolType;
> > -      NetworkInterface->OpenDriverAgentHandle                     = This-
> > >DriverBindingHandle;
> > -      NetworkInterface->OpenDriverControllerHandle                =
> > ControllerHandle;
> > -      NetworkInterface->NetworkInterfaceProtocolInfo.ProtocolGuid = \
> > -        *gRequiredProtocol[Index].RequiredProtocolGuid;
> > -      NetworkInterface->NetworkInterfaceProtocolInfo.ProtocolServiceGuid
> > = \
> > -        *gRequiredProtocol[Index].RequiredServiceBindingProtocolGuid;
> > +      NetworkInterface->NetworkProtocolType        =
> > gRequiredProtocol[Index].ProtocolType;
> > +      NetworkInterface->OpenDriverAgentHandle      = This-
> > >DriverBindingHandle;
> > +      NetworkInterface->OpenDriverControllerHandle = ControllerHandle;
> > +      CopyGuid (&NetworkInterface-
> > >NetworkInterfaceProtocolInfo.ProtocolGuid,
> > gRequiredProtocol[Index].RequiredProtocolGuid);
> > +      CopyGuid
> > + (&NetworkInterface->NetworkInterfaceProtocolInfo.ProtocolServiceGuid
> > + , gRequiredProtocol[Index].RequiredServiceBindingProtocolGuid);
> >        ProtocolDiscoverIdPtr        = &NetworkInterface-
> > >NetworkInterfaceProtocolInfo.ProtocolDiscoverId;
> >        OpenDriverAgentHandle        = NetworkInterface-
> > >OpenDriverAgentHandle;
> >        OpenDriverControllerHandle   = NetworkInterface-
> > >OpenDriverControllerHandle;
> > @@ -1598,7 +1642,7 @@ BuildupNetworkInterface (
> >
> >        gBS->RestoreTPL (OldTpl);
> >      } else {
> > -      // Record REST_EX instance. REST_EX is created when clinet asks for
> > Redfish service discovery.
> > +      // Record REST_EX instance. REST_EX is created when client asks
> > + for
> > Redfish service discovery.
> >        // Redfish Service Discover protocol will match REST EX to the
> > corresponding EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> >        // when discovery.
> >
> > @@ -1655,9 +1699,9 @@ BuildupNetworkInterface (
> >                        );
> >        if (!EFI_ERROR (Status)) {
> >          if ((gRequiredProtocol[Index].ProtocolType == ProtocolTypeRestEx)) {
> > -          // Install Redfish Discover Protocol when EFI REST EX protcol is
> > discovered.
> > +          // Install Redfish Discover Protocol when EFI REST EX
> > + protocol is
> > discovered.
> >            // This ensures EFI REST EX is ready while the consumer of
> > EFI_REDFISH_DISCOVER_PROTOCOL
> > -          // acquires Redfish serivce over network interface.
> > +          // acquires Redfish service over network interface.
> >
> >            if (!NewNetworkInterfaceInstalled) {
> >              NetworkInterface =
> > GetTargetNetworkInterfaceInternalByController
> > (ControllerHandle); @@ -1678,6 +1722,14 @@ BuildupNetworkInterface (
> >            if (EFI_ERROR (Status)) {
> >              DEBUG ((DEBUG_ERROR, "%a: Fail to install
> > EFI_REDFISH_DISCOVER_PROTOCOL\n", __FUNCTION__));
> >            }
> > +        } else {
> > +          DEBUG ((DEBUG_INFO, "%a: Not REST EX, continue with
> > + next\n",
> > __FUNCTION__));
> > +          Index++;
> > +          if (Index == (sizeof (gRequiredProtocol) / sizeof
> > (REDFISH_DISCOVER_REQUIRED_PROTOCOL))) {
> > +            break;
> > +          }
> > +
> > +          continue;
> >          }
> >        }
> >
> > @@ -1692,11 +1744,11 @@ BuildupNetworkInterface (
> >      }
> >    } while (Index < (sizeof (gRequiredProtocol) / sizeof
> > (REDFISH_DISCOVER_REQUIRED_PROTOCOL)));
> >
> > -  return EFI_UNSUPPORTED;
> > +  return EFI_DEVICE_ERROR;
> >  }
> >
> >  /**
> > -  Close the protocol opened for Redfish discovery. This function also
> > destories
> > +  Close the protocol opened for Redfish discovery. This function also
> > + destroy
> >    the network services.
> >
> >    @param[in]  ThisBindingProtocol     A pointer to the
> > EFI_DRIVER_BINDING_PROTOCOL instance.
> > @@ -1707,8 +1759,8 @@ BuildupNetworkInterface (
> >    @param[in]  DriverAgentHandle      Driver agent handle which used to open
> > protocol earlier.
> >    @param[in]  DriverControllerHandle Driver controller handle which
> > used to open protocol earlier.
> >
> > -  @retval EFI_SUCCESS                Prorocol is closed successfully.
> > -  @retval Others                     Prorocol is closed unsuccessfully.
> > +  @retval EFI_SUCCESS                Protocol is closed successfully.
> > +  @retval Others                     Protocol is closed unsuccessfully.
> >
> >  **/
> >  EFI_STATUS
> > @@ -1748,7 +1800,7 @@ CloseProtocolService (
> >                                     must support a protocol interface that supplies
> >                                     an I/O abstraction to the driver.
> >    @retval EFI_SUCCESS              One of required protocol is found.
> > -  @retval Others                   Faile to stop the services on network interface.
> > +  @retval Others                   Failed to stop the services on network interface.
> >  **/
> >  EFI_STATUS
> >  StopServiceOnNetworkInterface (
> > @@ -1792,14 +1844,14 @@ StopServiceOnNetworkInterface (
> >                         ThisNetworkInterface->OpenDriverControllerHandle
> >                         );
> >              if (!EFI_ERROR (Status)) {
> > -              Status = DestroyRedfishNetwrokInterface (ThisNetworkInterface);
> > +              Status = DestroyRedfishNetworkInterface
> > + (ThisNetworkInterface);
> >              }
> >
> >              gBS->RestoreTPL (OldTpl);
> >
> >              //
> >              // Disconnect EFI Redfish discover driver controller to notify the
> > -            // clinet which uses .EFI Redfish discover protocol.
> > +            // client which uses .EFI Redfish discover protocol.
> >              //
> >              if (DiscoverProtocolHandle != NULL) {
> >                gBS->DisconnectController (DiscoverProtocolHandle,
> > NULL, NULL); @@ -1945,7 +1997,7 @@
> RedfishDiscoverDriverBindingSupported (
> >    @retval EFI_SUCCESS              The device was started.
> >    @retval EFI_DEVICE_ERROR         The device could not be started due to a
> > device error.Currently not implemented.
> >    @retval EFI_OUT_OF_RESOURCES     The request could not be completed
> > due to a lack of resources.
> > -  @retval Others                   The driver failded to start the device.
> > +  @retval Others                   The driver failed to start the device.
> >
> >  **/
> >  EFI_STATUS
> > @@ -2030,7 +2082,7 @@ RedfishDiscoverEntryPoint (
> >    InitializeListHead (&mEfiRedfishDiscoverNetworkInterface);
> >    InitializeListHead (&mEfiRedfishDiscoverRestExInstance);
> >    //
> > -  // Install binding protoocl to obtain UDP and REST EX protocol.
> > +  // Install binding protocol to obtain UDP and REST EX protocol.
> >    //
> >    Status = EfiLibInstallDriverBindingComponentName2 (
> >               ImageHandle,
> > diff --git
> > a/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
> > b/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
> > index 88cec1f416..0d6edc7dc3 100644
> > --- a/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
> > +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
> > @@ -4,6 +4,7 @@
> >    Discover Redfish SMBIOS Host Interface.
> >
> >    (C) Copyright 2021 Hewlett Packard Enterprise Development LP<BR>
> > +  Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> >
> >    SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -22,7 +23,7 @@ SMBIOS_TABLE_TYPE42  *mType42Record;
> >    @param[out] DeviceDescriptor Pointer to REDFISH_INTERFACE_DATA.
> >    @param[out] ProtocolData     Pointer to
> > REDFISH_OVER_IP_PROTOCOL_DATA.
> >
> > -  @retval EFI_SUCCESS    Get host interface succesfully.
> > +  @retval EFI_SUCCESS    Get host interface successfully.
> >    @retval Otherwise      Fail to tet host interface.
> >
> >  **/
> > @@ -69,9 +70,15 @@ RedfishGetHostInterfaceProtocolData (
> >          //
> >          if ((*RecordTmp ==
> > REDFISH_HOST_INTERFACE_DEVICE_TYPE_PCI_PCIE_V2) || (*RecordTmp ==
> > REDFISH_HOST_INTERFACE_DEVICE_TYPE_USB_V2)) {
> >            if (*RecordTmp ==
> > REDFISH_HOST_INTERFACE_DEVICE_TYPE_PCI_PCIE_V2) {
> > -            ASSERT (SpecificDataLen == sizeof
> > (PCI_OR_PCIE_INTERFACE_DEVICE_DESCRIPTOR_V2) + 1);
> > +            if (SpecificDataLen != sizeof
> > (PCI_OR_PCIE_INTERFACE_DEVICE_DESCRIPTOR_V2) + 1) {
> > +              ASSERT (SpecificDataLen == sizeof
> > (PCI_OR_PCIE_INTERFACE_DEVICE_DESCRIPTOR_V2) + 1);
> > +              return EFI_VOLUME_CORRUPTED;
> > +            }
> >            } else {
> > -            ASSERT (SpecificDataLen > sizeof
> > (REDFISH_HOST_INTERFACE_DEVICE_TYPE_USB_V2) + 1);
> > +            if (SpecificDataLen != sizeof
> > (USB_INTERFACE_DEVICE_DESCRIPTOR_V2) + 1) {
> > +              ASSERT (SpecificDataLen == sizeof
> > (USB_INTERFACE_DEVICE_DESCRIPTOR_V2) + 1);
> > +              return EFI_VOLUME_CORRUPTED;
> > +            }
> >            }
> >
> >            *DeviceDescriptor = (REDFISH_INTERFACE_DATA *)RecordTmp;
> > --
> > 2.40.0.windows.1

      reply	other threads:[~2023-03-29  2:25 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-28  5:59 [PATCH 2/3] RedfishPkg: Redfish discover driver improvement Nickle Wang
2023-03-29  2:12 ` Chang, Abner
2023-03-29  2:25   ` Nickle Wang [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=MW4PR12MB7031EB8A1EE535F22F35B3D2D9899@MW4PR12MB7031.namprd12.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox