* [PATCH 2/2] RedfishPkg: Redfish discover driver improvement
@ 2023-02-01 3:05 Nickle Wang
2023-02-04 4:25 ` Chang, Abner
[not found] ` <1740847CF215CCF8.27199@groups.io>
0 siblings, 2 replies; 3+ messages in thread
From: Nickle Wang @ 2023-02-01 3:05 UTC (permalink / raw)
To: devel; +Cc: Abner Chang, Igor Kulchytskyy, Nick Ramirez
Bug fix:
- function stack fault
- properly handle "SubnetAddrInfoIPv6" when there is no IPv6 support
- copy-n-paste error in RedfishGetHostInterfaceProtocolData()
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.
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 | 4 ++
.../RedfishDiscoverDxe/RedfishDiscoverDxe.c | 52 +++++++++++++++----
.../RedfishSmbiosHostInterface.c | 2 +-
3 files changed, 47 insertions(+), 11 deletions(-)
diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
index 04fa09e1cc..3e1bfcdc4d 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
@@ -182,6 +183,9 @@ typedef struct {
//
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;
/**
diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
index 042d6d5fd5..e72ab71de8 100644
--- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
+++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
@@ -4,6 +4,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
@@ -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));
@@ -519,6 +523,14 @@ DiscoverRedfishHostInterface (
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);
+ } 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);
} else {
@@ -849,6 +861,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 +872,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 (
@@ -1467,11 +1489,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 +1514,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 +1543,7 @@ BuildupNetworkInterface (
IN EFI_HANDLE ControllerHandle
)
{
- UINT32 Id;
+ UINT32 *Id;
UINT32 Index;
EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *NetworkInterface;
BOOLEAN IsNew;
@@ -1581,10 +1607,8 @@ BuildupNetworkInterface (
NetworkInterface->NetworkProtocolType = gRequiredProtocol[Index].ProtocolType;
NetworkInterface->OpenDriverAgentHandle = This->DriverBindingHandle;
NetworkInterface->OpenDriverControllerHandle = ControllerHandle;
- NetworkInterface->NetworkInterfaceProtocolInfo.ProtocolGuid = \
- *gRequiredProtocol[Index].RequiredProtocolGuid;
- NetworkInterface->NetworkInterfaceProtocolInfo.ProtocolServiceGuid = \
- *gRequiredProtocol[Index].RequiredServiceBindingProtocolGuid;
+ CopyGuid (&NetworkInterface->NetworkInterfaceProtocolInfo.ProtocolGuid, gRequiredProtocol[Index].RequiredProtocolGuid);
+ CopyGuid (&NetworkInterface->NetworkInterfaceProtocolInfo.ProtocolServiceGuid, gRequiredProtocol[Index].RequiredServiceBindingProtocolGuid);
ProtocolDiscoverIdPtr = &NetworkInterface->NetworkInterfaceProtocolInfo.ProtocolDiscoverId;
OpenDriverAgentHandle = NetworkInterface->OpenDriverAgentHandle;
OpenDriverControllerHandle = NetworkInterface->OpenDriverControllerHandle;
@@ -1678,6 +1702,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;
}
}
diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c b/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
index 88cec1f416..b25a222a88 100644
--- a/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
+++ b/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
@@ -71,7 +71,7 @@ RedfishGetHostInterfaceProtocolData (
if (*RecordTmp == REDFISH_HOST_INTERFACE_DEVICE_TYPE_PCI_PCIE_V2) {
ASSERT (SpecificDataLen == sizeof (PCI_OR_PCIE_INTERFACE_DEVICE_DESCRIPTOR_V2) + 1);
} else {
- ASSERT (SpecificDataLen > sizeof (REDFISH_HOST_INTERFACE_DEVICE_TYPE_USB_V2) + 1);
+ ASSERT (SpecificDataLen == sizeof (USB_INTERFACE_DEVICE_DESCRIPTOR_V2) + 1);
}
*DeviceDescriptor = (REDFISH_INTERFACE_DATA *)RecordTmp;
--
2.39.1.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 2/2] RedfishPkg: Redfish discover driver improvement
2023-02-01 3:05 [PATCH 2/2] RedfishPkg: Redfish discover driver improvement Nickle Wang
@ 2023-02-04 4:25 ` Chang, Abner
[not found] ` <1740847CF215CCF8.27199@groups.io>
1 sibling, 0 replies; 3+ messages in thread
From: Chang, Abner @ 2023-02-04 4:25 UTC (permalink / raw)
To: Nickle Wang, devel@edk2.groups.io; +Cc: Igor Kulchytskyy, Nick Ramirez
[AMD Official Use Only - General]
Thanks for catching these bugs.
Other comments below in line.
> -----Original Message-----
> From: Nickle Wang <nicklew@nvidia.com>
> Sent: Wednesday, February 1, 2023 11:05 AM
> 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/2] 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()
> 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.
>
> 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 | 4 ++
> .../RedfishDiscoverDxe/RedfishDiscoverDxe.c | 52 +++++++++++++++----
> .../RedfishSmbiosHostInterface.c | 2 +-
> 3 files changed, 47 insertions(+), 11 deletions(-)
>
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> index 04fa09e1cc..3e1bfcdc4d 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
>
> @@ -182,6 +183,9 @@ typedef struct {
> //
> 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;
>
> /**
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 042d6d5fd5..e72ab71de8 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -4,6 +4,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
>
> @@ -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)); @@ -519,6 +523,14 @@
> DiscoverRedfishHostInterface (
> 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);
> + } 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);
> } else {
> @@ -849,6 +861,10 @@ AddAndSignalNewRedfishService (
> Status = EFI_OUT_OF_RESOURCES;
> goto EXIT_FREE_CONFIG_DATA;
> }
> +
> + if (Instance->HostAddrFormat == 2) {
I am thinking if we have to return error if Instance->HostAddrFormat is not equal to 2 when LocalAddressIsIPv6 is TRUE?
> + 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 +872,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 (
> @@ -1467,11 +1489,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 +1514,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) {
I am not quite understand this change. Could you please elaborate more about this change?
> + DEBUG ((DEBUG_ERROR, "%a: all required protocols are found on this
> controller handle: %p.\n", __FUNCTION__, ControllerHandle));
> + return EFI_SUCCESS;
> + }
> }
> }
> }
> @@ -1517,7 +1543,7 @@ BuildupNetworkInterface (
> IN EFI_HANDLE ControllerHandle
> )
> {
> - UINT32 Id;
> + UINT32 *Id;
> UINT32 Index;
> EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> *NetworkInterface;
> BOOLEAN IsNew;
> @@ -1581,10 +1607,8 @@ BuildupNetworkInterface (
> NetworkInterface->NetworkProtocolType =
> gRequiredProtocol[Index].ProtocolType;
> NetworkInterface->OpenDriverAgentHandle = This-
> >DriverBindingHandle;
> NetworkInterface->OpenDriverControllerHandle = ControllerHandle;
> - NetworkInterface->NetworkInterfaceProtocolInfo.ProtocolGuid = \
> - *gRequiredProtocol[Index].RequiredProtocolGuid;
> - NetworkInterface->NetworkInterfaceProtocolInfo.ProtocolServiceGuid = \
> - *gRequiredProtocol[Index].RequiredServiceBindingProtocolGuid;
> + CopyGuid (&NetworkInterface-
> >NetworkInterfaceProtocolInfo.ProtocolGuid,
> gRequiredProtocol[Index].RequiredProtocolGuid);
> + CopyGuid
> + (&NetworkInterface->NetworkInterfaceProtocolInfo.ProtocolServiceGuid,
> + gRequiredProtocol[Index].RequiredServiceBindingProtocolGuid);
> ProtocolDiscoverIdPtr = &NetworkInterface-
> >NetworkInterfaceProtocolInfo.ProtocolDiscoverId;
> OpenDriverAgentHandle = NetworkInterface->OpenDriverAgentHandle;
> OpenDriverControllerHandle = NetworkInterface-
> >OpenDriverControllerHandle;
> @@ -1678,6 +1702,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++;
Space between Index and "++".
Thanks
Abner
> + if (Index == (sizeof (gRequiredProtocol) / sizeof
> (REDFISH_DISCOVER_REQUIRED_PROTOCOL))) {
> + break;
> + }
> +
> + continue;
> }
> }
>
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
> index 88cec1f416..b25a222a88 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
> @@ -71,7 +71,7 @@ RedfishGetHostInterfaceProtocolData (
> if (*RecordTmp ==
> REDFISH_HOST_INTERFACE_DEVICE_TYPE_PCI_PCIE_V2) {
> ASSERT (SpecificDataLen == sizeof
> (PCI_OR_PCIE_INTERFACE_DEVICE_DESCRIPTOR_V2) + 1);
> } else {
> - ASSERT (SpecificDataLen > sizeof
> (REDFISH_HOST_INTERFACE_DEVICE_TYPE_USB_V2) + 1);
> + ASSERT (SpecificDataLen == sizeof
> + (USB_INTERFACE_DEVICE_DESCRIPTOR_V2) + 1);
> }
>
> *DeviceDescriptor = (REDFISH_INTERFACE_DATA *)RecordTmp;
> --
> 2.39.1.windows.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [edk2-devel] [PATCH 2/2] RedfishPkg: Redfish discover driver improvement
[not found] ` <1740847CF215CCF8.27199@groups.io>
@ 2023-02-04 4:51 ` Chang, Abner
0 siblings, 0 replies; 3+ messages in thread
From: Chang, Abner @ 2023-02-04 4:51 UTC (permalink / raw)
To: devel@edk2.groups.io, Chang, Abner, Nickle Wang
Cc: Igor Kulchytskyy, Nick Ramirez
[AMD Official Use Only - General]
Please ignore my comment for adding space between "Index" and operators.
Thanks
Abner
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang,
> Abner via groups.io
> Sent: Saturday, February 4, 2023 12:25 PM
> To: Nickle Wang <nicklew@nvidia.com>; devel@edk2.groups.io
> Cc: Igor Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@nvidia.com>
> Subject: Re: [edk2-devel] [PATCH 2/2] RedfishPkg: Redfish discover driver
> improvement
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> [AMD Official Use Only - General]
>
> Thanks for catching these bugs.
> Other comments below in line.
>
> > -----Original Message-----
> > From: Nickle Wang <nicklew@nvidia.com>
> > Sent: Wednesday, February 1, 2023 11:05 AM
> > 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/2] 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()
> > 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.
> >
> > 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 | 4 ++
> > .../RedfishDiscoverDxe/RedfishDiscoverDxe.c | 52 +++++++++++++++----
> > .../RedfishSmbiosHostInterface.c | 2 +-
> > 3 files changed, 47 insertions(+), 11 deletions(-)
> >
> > diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> > b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> > index 04fa09e1cc..3e1bfcdc4d 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
> >
> > @@ -182,6 +183,9 @@ typedef struct {
> > //
> > 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;
> >
> > /**
> > diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > index 042d6d5fd5..e72ab71de8 100644
> > --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > @@ -4,6 +4,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
> >
> > @@ -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)); @@ -519,6 +523,14 @@
> > DiscoverRedfishHostInterface (
> > 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);
> > + } 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);
> > } else {
> > @@ -849,6 +861,10 @@ AddAndSignalNewRedfishService (
> > Status = EFI_OUT_OF_RESOURCES;
> > goto EXIT_FREE_CONFIG_DATA;
> > }
> > +
> > + if (Instance->HostAddrFormat == 2) {
> I am thinking if we have to return error if Instance->HostAddrFormat is not equal
> to 2 when LocalAddressIsIPv6 is TRUE?
>
> > + 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 +872,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 (
> > @@ -1467,11 +1489,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 +1514,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) {
> I am not quite understand this change. Could you please elaborate more about
> this change?
> > + DEBUG ((DEBUG_ERROR, "%a: all required protocols are found
> > + on this
> > controller handle: %p.\n", __FUNCTION__, ControllerHandle));
> > + return EFI_SUCCESS;
> > + }
> > }
> > }
> > }
> > @@ -1517,7 +1543,7 @@ BuildupNetworkInterface (
> > IN EFI_HANDLE ControllerHandle
> > )
> > {
> > - UINT32 Id;
> > + UINT32 *Id;
> > UINT32 Index;
> > EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> > *NetworkInterface;
> > BOOLEAN IsNew;
> > @@ -1581,10 +1607,8 @@ BuildupNetworkInterface (
> > NetworkInterface->NetworkProtocolType =
> > gRequiredProtocol[Index].ProtocolType;
> > NetworkInterface->OpenDriverAgentHandle = This-
> > >DriverBindingHandle;
> > NetworkInterface->OpenDriverControllerHandle =
> ControllerHandle;
> > - NetworkInterface->NetworkInterfaceProtocolInfo.ProtocolGuid = \
> > - *gRequiredProtocol[Index].RequiredProtocolGuid;
> > - NetworkInterface->NetworkInterfaceProtocolInfo.ProtocolServiceGuid =
> \
> > - *gRequiredProtocol[Index].RequiredServiceBindingProtocolGuid;
> > + CopyGuid (&NetworkInterface-
> > >NetworkInterfaceProtocolInfo.ProtocolGuid,
> > gRequiredProtocol[Index].RequiredProtocolGuid);
> > + CopyGuid
> > + (&NetworkInterface->NetworkInterfaceProtocolInfo.ProtocolServiceGuid
> > + , gRequiredProtocol[Index].RequiredServiceBindingProtocolGuid);
> > ProtocolDiscoverIdPtr = &NetworkInterface-
> > >NetworkInterfaceProtocolInfo.ProtocolDiscoverId;
> > OpenDriverAgentHandle = NetworkInterface->OpenDriverAgentHandle;
> > OpenDriverControllerHandle = NetworkInterface-
> > >OpenDriverControllerHandle;
> > @@ -1678,6 +1702,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++;
> Space between Index and "++".
>
> Thanks
> Abner
>
> > + if (Index == (sizeof (gRequiredProtocol) / sizeof
> > (REDFISH_DISCOVER_REQUIRED_PROTOCOL))) {
> > + break;
> > + }
> > +
> > + continue;
> > }
> > }
> >
> > diff --git
> > a/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
> > b/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
> > index 88cec1f416..b25a222a88 100644
> > --- a/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
> > +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
> > @@ -71,7 +71,7 @@ RedfishGetHostInterfaceProtocolData (
> > if (*RecordTmp ==
> > REDFISH_HOST_INTERFACE_DEVICE_TYPE_PCI_PCIE_V2) {
> > ASSERT (SpecificDataLen == sizeof
> > (PCI_OR_PCIE_INTERFACE_DEVICE_DESCRIPTOR_V2) + 1);
> > } else {
> > - ASSERT (SpecificDataLen > sizeof
> > (REDFISH_HOST_INTERFACE_DEVICE_TYPE_USB_V2) + 1);
> > + ASSERT (SpecificDataLen == sizeof
> > + (USB_INTERFACE_DEVICE_DESCRIPTOR_V2) + 1);
> > }
> >
> > *DeviceDescriptor = (REDFISH_INTERFACE_DATA *)RecordTmp;
> > --
> > 2.39.1.windows.1
>
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-02-04 4:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-01 3:05 [PATCH 2/2] RedfishPkg: Redfish discover driver improvement Nickle Wang
2023-02-04 4:25 ` Chang, Abner
[not found] ` <1740847CF215CCF8.27199@groups.io>
2023-02-04 4:51 ` [edk2-devel] " Chang, Abner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox