public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH V2 0/8] Refine BMC USB NIC discovery and Redfish service enablement
@ 2023-11-23  6:47 Chang, Abner via groups.io
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 1/8] RedfishPkg/BmcUsbNicLib: Update BMC USB NIC searching algorithm Chang, Abner via groups.io
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Chang, Abner via groups.io @ 2023-11-23  6:47 UTC (permalink / raw)
  To: devel; +Cc: Nickle Wang, Igor Kulchytskyy, Mike Maslenkin

From: Abner Chang <abner.chang@amd.com>

In V2: Send additional two patches those are missed
       in V1.

This patch set updates the algorithm of BMC USB
NIC discovery and fixes some bugs.

- Add a new protocol to trigger the notification
  of Redfish Host Interface readiness.
  This fixes the issue Redfish config handler driver
  acquires Redfish service before Redfish Host
  Interface is published.
- Add more error debug messages.
- Address some minor issues

Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Mike Maslenkin <mike.maslenkin@gmail.com>

Abner Chang (8):
  RedfishPkg/BmcUsbNicLib: Update BMC USB NIC searching algorithm
  RedfishPkg/RedfishHostInterfaceDxe: Add Redfish HI readiness
    notification
  RedfishPkg/RedfishConfigHandler: Use Redfish HI readiness notification
  RedfishPkg/RedfishConfigHandler: Correct the prototype of callback
    function
  RedfishPkg/RedfishDiscovery: Add more debug message
  RedfishPkg/RedfishDiscovery: Refine SMBIOS 42h code
  RedfishPkg/HostInterfaceBmcUsbNic: Fix incorrect reference of MAC
    address pointer
  RedfishPkg/HostInterfaceBmcUsbNic: Fix incorrect HI protocol record
    size

 RedfishPkg/RedfishPkg.dec                     |   3 +
 .../RedfishConfigHandlerDriver.inf            |   9 +-
 .../RedfishHostInterfaceDxe.inf               |   3 +-
 .../RedfishDiscoverInternal.h                 |   2 +
 .../PlatformHostInterfaceBmcUsbNicLib.c       | 194 ++++++++++++------
 .../RedfishConfigHandlerDriver.c              | 170 +++++++++------
 .../RedfishDiscoverDxe/RedfishDiscoverDxe.c   |  23 +++
 .../RedfishSmbiosHostInterface.c              |  20 +-
 .../RedfishHostInterfaceDxe.c                 |  16 ++
 9 files changed, 312 insertions(+), 128 deletions(-)

-- 
2.37.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111655): https://edk2.groups.io/g/devel/message/111655
Mute This Topic: https://groups.io/mt/102763116/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [edk2-devel] [PATCH V2 1/8] RedfishPkg/BmcUsbNicLib: Update BMC USB NIC searching algorithm
  2023-11-23  6:47 [edk2-devel] [PATCH V2 0/8] Refine BMC USB NIC discovery and Redfish service enablement Chang, Abner via groups.io
@ 2023-11-23  6:47 ` Chang, Abner via groups.io
  2023-11-26 19:38   ` Mike Maslenkin
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 2/8] RedfishPkg/RedfishHostInterfaceDxe: Add Redfish HI readiness notification Chang, Abner via groups.io
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Chang, Abner via groups.io @ 2023-11-23  6:47 UTC (permalink / raw)
  To: devel; +Cc: Nickle Wang, Igor Kulchytskyy, Mike Maslenkin

From: Abner Chang <abner.chang@amd.com>

Update BMC USB NIC searching algorithm for IPv4 only.

Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 .../PlatformHostInterfaceBmcUsbNicLib.c       | 188 ++++++++++++------
 1 file changed, 128 insertions(+), 60 deletions(-)

diff --git a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
index 95900579118..e5bf70cfd58 100644
--- a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
+++ b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
@@ -368,7 +368,9 @@ RetrievedBmcUsbNicInfo (
         ));
       CopyMem ((VOID *)&ThisInstance->RedfishIpAddressIpv4, (VOID *)&DestIpAddress->IpAddress, sizeof (DestIpAddress->IpAddress));
       //
-      // According to UEFI spec, the IP address at BMC USB NIC host end is the IP address at BMC end minus 1.
+      // According to the design spec:
+      // https://github.com/tianocore/edk2/tree/master/RedfishPkg#platform-with-bmc-and-the-bmc-exposed-usb-network-device
+      // The IP address at BMC USB NIC host end is the IP address at BMC end minus 1.
       //
       CopyMem ((VOID *)&ThisInstance->HostIpAddressIpv4, (VOID *)&DestIpAddress->IpAddress, sizeof (DestIpAddress->IpAddress));
       ThisInstance->HostIpAddressIpv4[sizeof (ThisInstance->HostIpAddressIpv4) - 1] -= 1;
@@ -729,8 +731,10 @@ HostInterfaceIpmiCheckMacAddress (
 
       //
       // According to design spec in Readme file under RedfishPkg.
-      // Compare the first five MAC address and
-      // the 6th MAC address.
+      // https://github.com/tianocore/edk2/tree/master/RedfishPkg#platform-with-bmc-and-the-bmc-exposed-usb-network-device
+      // Compare the first five elements of MAC address and the 6th element of MAC address.
+      // The 6th element of MAC address must be the 6th element of
+      // IPMI channel MAC address minus 1.
       //
       if ((IpmiLanMacAddressSize != UsbNicInfo->MacAddressSize) ||
           (CompareMem (
@@ -738,8 +742,8 @@ HostInterfaceIpmiCheckMacAddress (
              (VOID *)&IpmiLanChannelMacAddress.Addr,
              IpmiLanMacAddressSize - 1
              ) != 0) ||
-          (IpmiLanChannelMacAddress.Addr[IpmiLanMacAddressSize - 1] !=
-           *(UsbNicInfo->MacAddress + IpmiLanMacAddressSize - 1) - 1)
+          ((IpmiLanChannelMacAddress.Addr[IpmiLanMacAddressSize - 1] - 1) !=
+           *(UsbNicInfo->MacAddress + IpmiLanMacAddressSize - 1))
           )
       {
         DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    MAC address is not matched.\n"));
@@ -962,6 +966,49 @@ UsbNicSearchUsbIo (
   return EFI_NOT_FOUND;
 }
 
+/**
+  This function identifies if the USB NIC has MAC address and internet
+  protocol device path installed. (Only support IPv4)
+
+  @param[in] UsbDevicePath     USB device path.
+
+  @retval EFI_SUCCESS          Yes, this is IPv4 SNP handle
+  @retval EFI_NOT_FOUND        No, this is not IPv4 SNP handle
+
+**/
+EFI_STATUS
+IdentifyNetworkMessageDevicePath (
+  IN EFI_DEVICE_PATH_PROTOCOL  *UsbDevicePath
+  )
+{
+  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
+
+  DevicePath = UsbDevicePath;
+  while (TRUE) {
+    DevicePath = NextDevicePathNode (DevicePath);
+    if (IsDevicePathEnd (DevicePath)) {
+      DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "MAC address device path is not found on this handle.\n"));
+      break;
+    }
+
+    if ((DevicePath->Type == MESSAGING_DEVICE_PATH) && (DevicePath->SubType == MSG_MAC_ADDR_DP)) {
+      DevicePath = NextDevicePathNode (DevicePath); // Advance to next device path protocol.
+      if (IsDevicePathEnd (DevicePath)) {
+        DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "IPv4 device path is not found on this handle.\n"));
+        break;
+      }
+
+      if ((DevicePath->Type == MESSAGING_DEVICE_PATH) && (DevicePath->SubType == MSG_IPv4_DP)) {
+        return EFI_SUCCESS;
+      }
+
+      break;
+    }
+  }
+
+  return EFI_NOT_FOUND;
+}
+
 /**
   This function identifies if the USB NIC is exposed by BMC as
   the host-BMC channel.
@@ -1025,7 +1072,7 @@ IdentifyUsbNicBmcChannel (
     (VOID *)&Snp->Mode->CurrentAddress,
     BmcUsbNic->MacAddressSize
     );
-  DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    MAC address (in size %d) for this SNP instance:\n      ", BmcUsbNic->MacAddressSize));
+  DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    MAC address (in size %d) for this SNP instance:\n", BmcUsbNic->MacAddressSize));
   for (Index = 0; Index < BmcUsbNic->MacAddressSize; Index++) {
     DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "%02x ", *(BmcUsbNic->MacAddress + Index)));
   }
@@ -1068,7 +1115,8 @@ CheckBmcUsbNicOnHandles (
   UINTN                     Index;
   EFI_STATUS                Status;
   EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
-  BOOLEAN                   GotOneUsbNIc;
+  BOOLEAN                   GotBmcUsbNic;
+  CHAR16                    *DevicePathStr;
 
   if ((HandleNumer == 0) || (HandleBuffer == NULL)) {
     return EFI_INVALID_PARAMETER;
@@ -1076,26 +1124,37 @@ CheckBmcUsbNicOnHandles (
 
   DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry, #%d SNP handle\n", __func__, HandleNumer));
 
-  GotOneUsbNIc = FALSE;
+  GotBmcUsbNic = FALSE;
   for (Index = 0; Index < HandleNumer; Index++) {
+    DEBUG ((DEBUG_MANAGEABILITY, "    Locate device path on handle 0x%08x\n", *(HandleBuffer + Index)));
     Status = gBS->HandleProtocol (
                     *(HandleBuffer + Index),
                     &gEfiDevicePathProtocolGuid,
                     (VOID **)&DevicePath
                     );
     if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "    Failed to locate SNP on %d handle.\n", Index));
+      DEBUG ((DEBUG_ERROR, "    Failed to locate device path on %d handle.\n", __func__, Index));
       continue;
     }
 
+    DevicePathStr = ConvertDevicePathToText (DevicePath, FALSE, FALSE);
+    if (DevicePathStr != NULL) {
+      DEBUG ((DEBUG_MANAGEABILITY, "    Device path: %s\n", DevicePathStr));
+      FreePool (DevicePathStr);
+    }
+
     // Check if this is an BMC exposed USB NIC device.
     while (TRUE) {
       if ((DevicePath->Type == MESSAGING_DEVICE_PATH) && (DevicePath->SubType == MSG_USB_DP)) {
-        Status = IdentifyUsbNicBmcChannel (*(HandleBuffer + Index), DevicePath);
+        Status = IdentifyNetworkMessageDevicePath (DevicePath);
         if (!EFI_ERROR (Status)) {
-          GotOneUsbNIc = TRUE;
-          break;
+          Status = IdentifyUsbNicBmcChannel (*(HandleBuffer + Index), DevicePath);
+          if (!EFI_ERROR (Status)) {
+            GotBmcUsbNic = TRUE;
+          }
         }
+
+        break; // Advance to next SNP handle.
       }
 
       DevicePath = NextDevicePathNode (DevicePath);
@@ -1105,10 +1164,11 @@ CheckBmcUsbNicOnHandles (
     }
   }
 
-  if (GotOneUsbNIc) {
+  if (GotBmcUsbNic) {
     return EFI_SUCCESS;
   }
 
+  DEBUG ((DEBUG_MANAGEABILITY, "No BMC USB NIC found on SNP handles\n"));
   return EFI_NOT_FOUND;
 }
 
@@ -1139,62 +1199,70 @@ CheckBmcUsbNic (
 
   DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry, the registration key - 0x%08x.\n", __func__, Registration));
 
-  Handle     = NULL;
-  Status     = EFI_SUCCESS;
-  BufferSize = 0;
+  Handle = NULL;
+  Status = EFI_SUCCESS;
 
-  Status = gBS->LocateHandle (
-                  Registration == NULL ? ByProtocol : ByRegisterNotify,
-                  &gEfiSimpleNetworkProtocolGuid,
-                  Registration,
-                  &BufferSize,
-                  NULL
-                  );
-  if (Status == EFI_BUFFER_TOO_SMALL) {
-    DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    %d SNP protocol instances.\n", BufferSize/sizeof (EFI_HANDLE)));
-    HandleBuffer = AllocateZeroPool (BufferSize);
-    if (HandleBuffer == NULL) {
-      DEBUG ((DEBUG_ERROR, "    Falied to allocate buffer for the handles.\n"));
-      return EFI_OUT_OF_RESOURCES;
-    }
+  do {
+    BufferSize = 0;
+    Status     = gBS->LocateHandle (
+                        Registration == NULL ? ByProtocol : ByRegisterNotify,
+                        &gEfiSimpleNetworkProtocolGuid,
+                        Registration,
+                        &BufferSize,
+                        NULL
+                        );
+    if (Status == EFI_BUFFER_TOO_SMALL) {
+      DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    %d SNP protocol instance(s).\n", BufferSize/sizeof (EFI_HANDLE)));
+      HandleBuffer = AllocateZeroPool (BufferSize);
+      if (HandleBuffer == NULL) {
+        DEBUG ((DEBUG_ERROR, "    Falied to allocate buffer for the handles.\n"));
+        return EFI_OUT_OF_RESOURCES;
+      }
+
+      Status = gBS->LocateHandle (
+                      Registration == NULL ? ByProtocol : ByRegisterNotify,
+                      &gEfiSimpleNetworkProtocolGuid,
+                      Registration,
+                      &BufferSize,
+                      HandleBuffer
+                      );
+      if (EFI_ERROR (Status)) {
+        DEBUG ((DEBUG_ERROR, "    Falied to locate SNP protocol handles.\n"));
+        FreePool (HandleBuffer);
+        return Status;
+      }
+    } else if (EFI_ERROR (Status)) {
+      if (Registration != NULL) {
+        DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    No more newly installed SNP protocol for this registration - %r.\n", Status));
+        return EFI_SUCCESS;
+      }
 
-    Status = gBS->LocateHandle (
-                    Registration == NULL ? ByProtocol : ByRegisterNotify,
-                    &gEfiSimpleNetworkProtocolGuid,
-                    Registration,
-                    &BufferSize,
-                    HandleBuffer
-                    );
-    if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "    Falied to locate SNP protocol handles.\n"));
-      FreePool (HandleBuffer);
       return Status;
     }
-  } else if (EFI_ERROR (Status)) {
-    return Status;
-  }
 
-  // Check USB NIC on handles.
-  Status = CheckBmcUsbNicOnHandles (BufferSize/sizeof (EFI_HANDLE), HandleBuffer);
-  if (!EFI_ERROR (Status)) {
-    // Retrieve the rest of BMC USB NIC information for Redfish over IP information
-    // and USB Network Interface V2.
-    Status = RetrievedBmcUsbNicInfo ();
+    // Check USB NIC on handles.
+    Status = CheckBmcUsbNicOnHandles (BufferSize/sizeof (EFI_HANDLE), HandleBuffer);
     if (!EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    Install protocol to notify the platform Redfish Host Interface information is ready.\n"));
-      Status = gBS->InstallProtocolInterface (
-                      &Handle,
-                      &mPlatformHostInterfaceBmcUsbNicReadinessGuid,
-                      EFI_NATIVE_INTERFACE,
-                      NULL
-                      );
-      if (EFI_ERROR (Status)) {
-        DEBUG ((DEBUG_ERROR, "    Install protocol fail %r.\n", Status));
+      // Retrieve the rest of BMC USB NIC information for Redfish over IP information
+      // and USB Network Interface V2.
+      Status = RetrievedBmcUsbNicInfo ();
+      if (!EFI_ERROR (Status)) {
+        DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    Install protocol to notify the platform Redfish Host Interface information is ready.\n"));
+        Status = gBS->InstallProtocolInterface (
+                        &Handle,
+                        &mPlatformHostInterfaceBmcUsbNicReadinessGuid,
+                        EFI_NATIVE_INTERFACE,
+                        NULL
+                        );
+        if (EFI_ERROR (Status)) {
+          DEBUG ((DEBUG_ERROR, "    Install protocol fail %r.\n", Status));
+        }
       }
     }
-  }
 
-  FreePool (HandleBuffer);
+    FreePool (HandleBuffer);
+  } while (Registration != NULL);
+
   return Status;
 }
 
-- 
2.37.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111656): https://edk2.groups.io/g/devel/message/111656
Mute This Topic: https://groups.io/mt/102763117/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [edk2-devel] [PATCH V2 2/8] RedfishPkg/RedfishHostInterfaceDxe: Add Redfish HI readiness notification
  2023-11-23  6:47 [edk2-devel] [PATCH V2 0/8] Refine BMC USB NIC discovery and Redfish service enablement Chang, Abner via groups.io
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 1/8] RedfishPkg/BmcUsbNicLib: Update BMC USB NIC searching algorithm Chang, Abner via groups.io
@ 2023-11-23  6:47 ` Chang, Abner via groups.io
  2023-11-26 19:35   ` Mike Maslenkin
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 3/8] RedfishPkg/RedfishConfigHandler: Use " Chang, Abner via groups.io
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Chang, Abner via groups.io @ 2023-11-23  6:47 UTC (permalink / raw)
  To: devel; +Cc: Nickle Wang, Igor Kulchytskyy, Mike Maslenkin

From: Abner Chang <abner.chang@amd.com>

Introduce gEdkIIRedfishHostInterfaceReadyProtocolGuid
and produce it when Redfish Host Interface is installed
on system.

Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 RedfishPkg/RedfishPkg.dec                        |  3 +++
 .../RedfishHostInterfaceDxe.inf                  |  3 ++-
 .../RedfishHostInterfaceDxe.c                    | 16 ++++++++++++++++
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/RedfishPkg/RedfishPkg.dec b/RedfishPkg/RedfishPkg.dec
index 0f18865cea0..e40538247c2 100644
--- a/RedfishPkg/RedfishPkg.dec
+++ b/RedfishPkg/RedfishPkg.dec
@@ -90,6 +90,9 @@
   ## Include/Protocol/EdkIIRedfishPlatformConfig.h
   gEdkIIRedfishPlatformConfigProtocolGuid = { 0X4D94A7C7, 0X4CE4, 0X4A84, { 0X88, 0XC1, 0X33, 0X0C, 0XD4, 0XA3, 0X47, 0X67 } }
 
+  # Redfish Host Interface ready notification protocol
+  gEdkIIRedfishHostInterfaceReadyProtocolGuid = { 0xC3F6D062, 0x3D38, 0x4EA4, { 0x92, 0xB1, 0xE8, 0xF8, 0x02, 0x27, 0x63, 0xDF } }
+
 [Guids]
   gEfiRedfishPkgTokenSpaceGuid      = { 0x4fdbccb7, 0xe829, 0x4b4c, { 0x88, 0x87, 0xb2, 0x3f, 0xd7, 0x25, 0x4b, 0x85 }}
 
diff --git a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.inf b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.inf
index 1cdae149aad..f969e75463f 100644
--- a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.inf
+++ b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.inf
@@ -43,7 +43,8 @@
   UefiLib
 
 [Protocols]
-  gEfiSmbiosProtocolGuid                ## TO_START
+  gEfiSmbiosProtocolGuid                       ## TO_START
+  gEdkIIRedfishHostInterfaceReadyProtocolGuid  ## PRODUCED
 
 [Depex]
   gEfiSmbiosProtocolGuid
diff --git a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
index 55a66decfc8..94c0f9b6a92 100644
--- a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
+++ b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
@@ -53,7 +53,9 @@ RedfishCreateSmbiosTable42 (
   SMBIOS_TABLE_TYPE42                *Type42Record;
   EFI_SMBIOS_PROTOCOL                *Smbios;
   EFI_SMBIOS_HANDLE                  MemArrayMappedAddrSmbiosHandle;
+  EFI_HANDLE                         Handle;
 
+  Handle = NULL;
   //
   // Get platform Redfish host interface device type descriptor data.
   //
@@ -228,6 +230,20 @@ RedfishCreateSmbiosTable42 (
 
   Status = EFI_SUCCESS;
 
+  //
+  // Install Redfish Host Interface ready protocol.
+  //
+  Status = gBS->InstallProtocolInterface (
+                  &Handle,
+                  &gEdkIIRedfishHostInterfaceReadyProtocolGuid,
+                  EFI_NATIVE_INTERFACE,
+                  (VOID *)NULL
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed to install gEdkIIRedfishHostInterfaceReadyProtocolGuid.\n"));
+    DEBUG ((DEBUG_ERROR, "PlatformConfigHandler driver may not be triggered to acquire Redfish service.\n"));
+  }
+
 ON_EXIT:
   if (DeviceDescriptor != NULL) {
     FreePool (DeviceDescriptor);
-- 
2.37.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111657): https://edk2.groups.io/g/devel/message/111657
Mute This Topic: https://groups.io/mt/102763119/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [edk2-devel] [PATCH V2 3/8] RedfishPkg/RedfishConfigHandler: Use Redfish HI readiness notification
  2023-11-23  6:47 [edk2-devel] [PATCH V2 0/8] Refine BMC USB NIC discovery and Redfish service enablement Chang, Abner via groups.io
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 1/8] RedfishPkg/BmcUsbNicLib: Update BMC USB NIC searching algorithm Chang, Abner via groups.io
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 2/8] RedfishPkg/RedfishHostInterfaceDxe: Add Redfish HI readiness notification Chang, Abner via groups.io
@ 2023-11-23  6:47 ` Chang, Abner via groups.io
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 4/8] RedfishPkg/RedfishConfigHandler: Correct the prototype of callback function Chang, Abner via groups.io
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Chang, Abner via groups.io @ 2023-11-23  6:47 UTC (permalink / raw)
  To: devel; +Cc: Nickle Wang, Igor Kulchytskyy, Mike Maslenkin

From: Abner Chang <abner.chang@amd.com>

Wait until Redfish Host Interface is installed on
the system then acquire Redfish service.

Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 .../RedfishConfigHandlerDriver.inf            |   9 +-
 .../RedfishConfigHandlerDriver.c              | 168 ++++++++++++------
 2 files changed, 116 insertions(+), 61 deletions(-)

diff --git a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.inf b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.inf
index b167c6e1ee4..aed93f570cf 100644
--- a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.inf
+++ b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.inf
@@ -46,11 +46,12 @@
   UefiDriverEntryPoint
 
 [Protocols]
-  gEfiRedfishDiscoverProtocolGuid         ## CONSUMES
+  gEfiRedfishDiscoverProtocolGuid              ## CONSUMES
   gEfiRestExServiceBindingProtocolGuid
-  gEfiRestExProtocolGuid                  ## CONSUMES
-  gEdkIIRedfishCredentialProtocolGuid     ## CONSUMES
-  gEdkIIRedfishConfigHandlerProtocolGuid  ## CONSUMES
+  gEfiRestExProtocolGuid                       ## CONSUMES
+  gEdkIIRedfishCredentialProtocolGuid          ## CONSUMES
+  gEdkIIRedfishConfigHandlerProtocolGuid       ## CONSUMES
+  gEdkIIRedfishHostInterfaceReadyProtocolGuid  ## CONSUMES
 
 [Guids]
   gEfiEventExitBootServicesGuid           ## CONSUMES ## Event
diff --git a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
index f987cc67a69..b421f51374d 100644
--- a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
+++ b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
@@ -17,11 +17,15 @@ EFI_EVENT  gEfiRedfishDiscoverProtocolEvent = NULL;
 //
 // Variables for using RFI Redfish Discover Protocol
 //
-VOID                           *gEfiRedfishDiscoverRegistration;
-EFI_HANDLE                     gEfiRedfishDiscoverControllerHandle = NULL;
-EFI_REDFISH_DISCOVER_PROTOCOL  *gEfiRedfishDiscoverProtocol        = NULL;
-BOOLEAN                        gRedfishDiscoverActivated           = FALSE;
-BOOLEAN                        gRedfishServiceDiscovered           = FALSE;
+VOID                                    *gEfiRedfishDiscoverRegistration;
+EFI_HANDLE                              gEfiRedfishDiscoverControllerHandle = NULL;
+EFI_REDFISH_DISCOVER_PROTOCOL           *gEfiRedfishDiscoverProtocol        = NULL;
+BOOLEAN                                 gRedfishDiscoverActivated           = FALSE;
+BOOLEAN                                 gRedfishServiceDiscovered           = FALSE;
+EFI_REDFISH_DISCOVER_NETWORK_INTERFACE  *mNetworkInterfaces                 = NULL;
+UINTN                                   mNumberOfNetworkInterfaces;
+EFI_EVENT                               mEdkIIRedfishHostInterfaceReadyEvent;
+VOID                                    *mEdkIIRedfishHostInterfaceRegistration;
 
 ///
 /// Driver Binding Protocol instance
@@ -339,6 +343,83 @@ RedfishServiceDiscoveredCallback (
   FreePool (RedfishDiscoveredToken);
 }
 
+/**
+  Callback function executed when the gEdkIIRedfishHostInterfaceReadyProtocolGuid
+  protocol interface is installed.
+
+  @param[in]   Event    Event whose notification function is being invoked.
+  @param[in]   Context  Pointer to the Context buffer
+
+**/
+VOID
+EFIAPI
+AcquireRedfishServiceOnNetworkInterfaceCallback (
+  IN  EFI_EVENT  Event,
+  IN  VOID       *Context
+  )
+{
+  EFI_STATUS                              Status;
+  EFI_REDFISH_DISCOVER_NETWORK_INTERFACE  *ThisNetworkInterface;
+  UINTN                                   NetworkInterfaceIndex;
+  EFI_REDFISH_DISCOVERED_TOKEN            *ThisRedfishDiscoveredToken;
+
+  ThisNetworkInterface = mNetworkInterfaces;
+  //
+  // Loop to discover Redfish service on each network interface.
+  //
+  for (NetworkInterfaceIndex = 0; NetworkInterfaceIndex < mNumberOfNetworkInterfaces; NetworkInterfaceIndex++) {
+    ThisRedfishDiscoveredToken = (EFI_REDFISH_DISCOVERED_TOKEN *)AllocateZeroPool (sizeof (EFI_REDFISH_DISCOVERED_TOKEN));
+    if (ThisRedfishDiscoveredToken == NULL) {
+      DEBUG ((DEBUG_ERROR, "%a: Not enough memory for EFI_REDFISH_DISCOVERED_TOKEN.\n", __func__));
+      return;
+    }
+
+    ThisRedfishDiscoveredToken->Signature = REDFISH_DISCOVER_TOKEN_SIGNATURE;
+
+    //
+    // Initial this Redfish Discovered Token
+    //
+    Status = gBS->CreateEvent (
+                    EVT_NOTIFY_SIGNAL,
+                    TPL_CALLBACK,
+                    RedfishServiceDiscoveredCallback,
+                    (VOID *)ThisRedfishDiscoveredToken,
+                    &ThisRedfishDiscoveredToken->Event
+                    );
+    if (EFI_ERROR (Status)) {
+      FreePool (ThisRedfishDiscoveredToken);
+      DEBUG ((DEBUG_ERROR, "%a: Failed to create event for Redfish discovered token.\n", __func__));
+      return;
+    }
+
+    //
+    // Acquire for Redfish service which is reported by
+    // Redfish Host Interface.
+    //
+    Status = gEfiRedfishDiscoverProtocol->AcquireRedfishService (
+                                            gEfiRedfishDiscoverProtocol,
+                                            gRedfishConfigData.Image,
+                                            ThisNetworkInterface,
+                                            EFI_REDFISH_DISCOVER_HOST_INTERFACE,
+                                            ThisRedfishDiscoveredToken
+                                            );
+
+    //
+    // Free Redfish Discovered Token if Discover Instance was not created and
+    // Redfish Service Discovered Callback event was not triggered.
+    //
+    if ((ThisRedfishDiscoveredToken->DiscoverList.NumberOfServiceFound == 0) ||
+        EFI_ERROR (ThisRedfishDiscoveredToken->DiscoverList.RedfishInstances->Status))
+    {
+      gBS->CloseEvent (ThisRedfishDiscoveredToken->Event);
+      DEBUG ((DEBUG_ERROR, "%a: Free Redfish discovered token - %x.\n", __func__, ThisRedfishDiscoveredToken));
+      FreePool (ThisRedfishDiscoveredToken);
+    }
+
+    ThisNetworkInterface++;
+  }
+}
+
 /**
   Callback function executed when the EFI_REDFISH_DISCOVER_PROTOCOL
   protocol interface is installed.
@@ -354,13 +435,10 @@ RedfishDiscoverProtocolInstalled (
   OUT VOID       *Context
   )
 {
-  EFI_STATUS                              Status;
-  UINTN                                   BufferSize;
-  EFI_HANDLE                              HandleBuffer;
-  UINTN                                   NetworkInterfaceIndex;
-  EFI_REDFISH_DISCOVER_NETWORK_INTERFACE  *ThisNetworkInterface;
-  EFI_REDFISH_DISCOVERED_TOKEN            *ThisRedfishDiscoveredToken;
-  UINTN                                   NumberOfNetworkInterfaces;
+  EFI_STATUS  Status;
+  UINTN       BufferSize;
+  EFI_HANDLE  HandleBuffer;
+  VOID        *RedfishHostInterfaceReadyProtocol;
 
   DEBUG ((DEBUG_MANAGEABILITY, "%a: New network interface is installed on system by EFI Redfish discover driver.\n", __func__));
 
@@ -401,67 +479,43 @@ RedfishDiscoverProtocolInstalled (
   Status = gEfiRedfishDiscoverProtocol->GetNetworkInterfaceList (
                                           gEfiRedfishDiscoverProtocol,
                                           gRedfishConfigData.Image,
-                                          &NumberOfNetworkInterfaces,
-                                          &ThisNetworkInterface
+                                          &mNumberOfNetworkInterfaces,
+                                          &mNetworkInterfaces
                                           );
-  if (EFI_ERROR (Status) || (NumberOfNetworkInterfaces == 0)) {
+  if (EFI_ERROR (Status) || (mNumberOfNetworkInterfaces == 0)) {
     DEBUG ((DEBUG_ERROR, "%a: No network interfaces found on the handle.\n", __func__));
     return;
   }
 
   //
-  // Loop to discover Redfish service on each network interface.
+  // Check if Redfish Host Interface is ready or not.
   //
-  for (NetworkInterfaceIndex = 0; NetworkInterfaceIndex < NumberOfNetworkInterfaces; NetworkInterfaceIndex++) {
-    ThisRedfishDiscoveredToken = (EFI_REDFISH_DISCOVERED_TOKEN *)AllocateZeroPool (sizeof (EFI_REDFISH_DISCOVERED_TOKEN));
-    if (ThisRedfishDiscoveredToken == NULL) {
-      DEBUG ((DEBUG_ERROR, "%a: Not enough memory for EFI_REDFISH_DISCOVERED_TOKEN.\n", __func__));
-      return;
-    }
-
-    ThisRedfishDiscoveredToken->Signature = REDFISH_DISCOVER_TOKEN_SIGNATURE;
-
-    //
-    // Initial this Redfish Discovered Token
-    //
+  Status = gBS->LocateProtocol (&gEdkIIRedfishHostInterfaceReadyProtocolGuid, NULL, &RedfishHostInterfaceReadyProtocol);
+  if (!EFI_ERROR (Status)) {
+    // Acquire Redfish service;
+    AcquireRedfishServiceOnNetworkInterfaceCallback ((EFI_EVENT)NULL, (VOID *)NULL);
+  } else {
     Status = gBS->CreateEvent (
                     EVT_NOTIFY_SIGNAL,
                     TPL_CALLBACK,
-                    RedfishServiceDiscoveredCallback,
-                    (VOID *)ThisRedfishDiscoveredToken,
-                    &ThisRedfishDiscoveredToken->Event
+                    AcquireRedfishServiceOnNetworkInterfaceCallback,
+                    NULL,
+                    &mEdkIIRedfishHostInterfaceReadyEvent
                     );
     if (EFI_ERROR (Status)) {
-      FreePool (ThisRedfishDiscoveredToken);
-      DEBUG ((DEBUG_ERROR, "%a: Failed to create event for Redfish discovered token.\n", __func__));
+      DEBUG ((DEBUG_ERROR, "%a: Failed to create event for gEdkIIRedfishHostInterfaceReadyProtocolGuid installation.", __func__));
       return;
     }
 
-    //
-    // Acquire for Redfish service which is reported by
-    // Redfish Host Interface.
-    //
-    Status = gEfiRedfishDiscoverProtocol->AcquireRedfishService (
-                                            gEfiRedfishDiscoverProtocol,
-                                            gRedfishConfigData.Image,
-                                            ThisNetworkInterface,
-                                            EFI_REDFISH_DISCOVER_HOST_INTERFACE,
-                                            ThisRedfishDiscoveredToken
-                                            );
-
-    //
-    // Free Redfish Discovered Token if Discover Instance was not created and
-    // Redfish Service Discovered Callback event was not triggered.
-    //
-    if ((ThisRedfishDiscoveredToken->DiscoverList.NumberOfServiceFound == 0) ||
-        EFI_ERROR (ThisRedfishDiscoveredToken->DiscoverList.RedfishInstances->Status))
-    {
-      gBS->CloseEvent (ThisRedfishDiscoveredToken->Event);
-      DEBUG ((DEBUG_ERROR, "%a: Free Redfish discovered token - %x.\n", __func__, ThisRedfishDiscoveredToken));
-      FreePool (ThisRedfishDiscoveredToken);
+    Status = gBS->RegisterProtocolNotify (
+                    &gEdkIIRedfishHostInterfaceReadyProtocolGuid,
+                    mEdkIIRedfishHostInterfaceReadyEvent,
+                    &mEdkIIRedfishHostInterfaceRegistration
+                    );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "%a: Fail to register event for the installation of gEdkIIRedfishHostInterfaceReadyProtocolGuid.", __func__));
+      return;
     }
-
-    ThisNetworkInterface++;
   }
 
   return;
-- 
2.37.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111663): https://edk2.groups.io/g/devel/message/111663
Mute This Topic: https://groups.io/mt/102763126/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [edk2-devel] [PATCH V2 4/8] RedfishPkg/RedfishConfigHandler: Correct the prototype of callback function
  2023-11-23  6:47 [edk2-devel] [PATCH V2 0/8] Refine BMC USB NIC discovery and Redfish service enablement Chang, Abner via groups.io
                   ` (2 preceding siblings ...)
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 3/8] RedfishPkg/RedfishConfigHandler: Use " Chang, Abner via groups.io
@ 2023-11-23  6:47 ` Chang, Abner via groups.io
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 5/8] RedfishPkg/RedfishDiscovery: Add more debug message Chang, Abner via groups.io
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Chang, Abner via groups.io @ 2023-11-23  6:47 UTC (permalink / raw)
  To: devel; +Cc: Nickle Wang, Igor Kulchytskyy, Mike Maslenkin

From: Abner Chang <abner.chang@amd.com>

Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
index b421f51374d..2d0170d8861 100644
--- a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
+++ b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
@@ -425,14 +425,14 @@ AcquireRedfishServiceOnNetworkInterfaceCallback (
   protocol interface is installed.
 
   @param[in]   Event    Event whose notification function is being invoked.
-  @param[out]  Context  Pointer to the Context buffer
+  @param[in]   Context  Pointer to the Context buffer
 
 **/
 VOID
 EFIAPI
 RedfishDiscoverProtocolInstalled (
   IN  EFI_EVENT  Event,
-  OUT VOID       *Context
+  IN  VOID       *Context
   )
 {
   EFI_STATUS  Status;
-- 
2.37.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111658): https://edk2.groups.io/g/devel/message/111658
Mute This Topic: https://groups.io/mt/102763120/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [edk2-devel] [PATCH V2 5/8] RedfishPkg/RedfishDiscovery: Add more debug message
  2023-11-23  6:47 [edk2-devel] [PATCH V2 0/8] Refine BMC USB NIC discovery and Redfish service enablement Chang, Abner via groups.io
                   ` (3 preceding siblings ...)
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 4/8] RedfishPkg/RedfishConfigHandler: Correct the prototype of callback function Chang, Abner via groups.io
@ 2023-11-23  6:47 ` Chang, Abner via groups.io
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 6/8] RedfishPkg/RedfishDiscovery: Refine SMBIOS 42h code Chang, Abner via groups.io
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Chang, Abner via groups.io @ 2023-11-23  6:47 UTC (permalink / raw)
  To: devel; +Cc: Nickle Wang, Igor Kulchytskyy, Mike Maslenkin

From: Abner Chang <abner.chang@amd.com>

Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 .../RedfishDiscoverDxe/RedfishDiscoverDxe.c   | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
index 28ba2d3a9fc..833ae2b969f 100644
--- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
+++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
@@ -603,6 +603,27 @@ DiscoverRedfishHostInterface (
     }
 
     if (MacCompareStatus != 0) {
+      DEBUG ((DEBUG_ERROR, "%a: MAC address is not matched.\n", __func__));
+      DEBUG ((
+        DEBUG_ERROR,
+        "    NetworkInterface: %02x %02x %02x %02x %02x %02x.\n",
+        Instance->NetworkInterface->MacAddress.Addr[0],
+        Instance->NetworkInterface->MacAddress.Addr[1],
+        Instance->NetworkInterface->MacAddress.Addr[2],
+        Instance->NetworkInterface->MacAddress.Addr[3],
+        Instance->NetworkInterface->MacAddress.Addr[4],
+        Instance->NetworkInterface->MacAddress.Addr[5]
+        ));
+      DEBUG ((
+        DEBUG_ERROR,
+        "    Redfish Host interface: %02x %02x %02x %02x %02x %02x.\n",
+        DeviceDescriptor->DeviceDescriptor.UsbDeviceV2.MacAddress[0],
+        DeviceDescriptor->DeviceDescriptor.UsbDeviceV2.MacAddress[1],
+        DeviceDescriptor->DeviceDescriptor.UsbDeviceV2.MacAddress[2],
+        DeviceDescriptor->DeviceDescriptor.UsbDeviceV2.MacAddress[3],
+        DeviceDescriptor->DeviceDescriptor.UsbDeviceV2.MacAddress[4],
+        DeviceDescriptor->DeviceDescriptor.UsbDeviceV2.MacAddress[5]
+        ));
       return EFI_UNSUPPORTED;
     }
 
@@ -716,6 +737,8 @@ DiscoverRedfishHostInterface (
                  IsHttps
                  );
     }
+  } else {
+    DEBUG ((DEBUG_ERROR, "%a: RedfishGetHostInterfaceProtocolData is failed.\n", __func__));
   }
 
   return Status;
-- 
2.37.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111659): https://edk2.groups.io/g/devel/message/111659
Mute This Topic: https://groups.io/mt/102763121/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [edk2-devel] [PATCH V2 6/8] RedfishPkg/RedfishDiscovery: Refine SMBIOS 42h code
  2023-11-23  6:47 [edk2-devel] [PATCH V2 0/8] Refine BMC USB NIC discovery and Redfish service enablement Chang, Abner via groups.io
                   ` (4 preceding siblings ...)
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 5/8] RedfishPkg/RedfishDiscovery: Add more debug message Chang, Abner via groups.io
@ 2023-11-23  6:47 ` Chang, Abner via groups.io
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 7/8] RedfishPkg/HostInterfaceBmcUsbNic: Fix incorrect reference of MAC address pointer Chang, Abner via groups.io
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Chang, Abner via groups.io @ 2023-11-23  6:47 UTC (permalink / raw)
  To: devel; +Cc: Nickle Wang, Igor Kulchytskyy, Mike Maslenkin

From: Abner Chang <abner.chang@amd.com>

Refine SMBIOS 42h code add mode debug message
for the error conditions.

Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 .../RedfishDiscoverInternal.h                 |  2 ++
 .../RedfishSmbiosHostInterface.c              | 20 +++++++++++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
index e27cfa76e39..de7faa4f975 100644
--- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
+++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
@@ -42,6 +42,8 @@
 #define MAC_COMPARE(This, Target)  (CompareMem ((VOID *)&(This)->MacAddress, &(Target)->MacAddress, (This)->HwAddressSize) == 0)
 #define VALID_TCP6(Target, This)   ((Target)->IsIpv6 && ((This)->NetworkProtocolType == ProtocolTypeTcp6))
 #define VALID_TCP4(Target, This)   (!(Target)->IsIpv6 && ((This)->NetworkProtocolType == ProtocolTypeTcp4))
+#define REDFISH_HI_ITERFACE_SPECIFIC_DATA_LENGTH_OFFSET  ((UINT16)(UINTN)(&((SMBIOS_TABLE_TYPE42 *)0)->InterfaceTypeSpecificDataLength))
+#define REDFISH_HI_PROTOCOL_HOSTNAME_LENGTH_OFFSET       ((UINT16)(UINTN)(&((REDFISH_OVER_IP_PROTOCOL_DATA *)0)->RedfishServiceHostnameLength))
 
 //
 // GUID definitions
diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c b/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
index 0d6edc7dc35..57665f367be 100644
--- a/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
+++ b/RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
@@ -56,7 +56,7 @@ RedfishGetHostInterfaceProtocolData (
       mType42Record = (SMBIOS_TABLE_TYPE42 *)Record;
       if (mType42Record->InterfaceType == MCHostInterfaceTypeNetworkHostInterface) {
         ASSERT (Record->Length >= 9);
-        Offset    = 5;
+        Offset    = REDFISH_HI_ITERFACE_SPECIFIC_DATA_LENGTH_OFFSET;
         RecordTmp = (UINT8 *)Record + Offset;
         //
         // Get interface specific data length.
@@ -70,11 +70,13 @@ 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) {
+            // According to Redfish Host Interface specification, add additional one byte for Device Type field.
             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 {
+            // According to Redfish Host Interface specification, add additional one byte for Device Type field.
             if (SpecificDataLen != sizeof (USB_INTERFACE_DEVICE_DESCRIPTOR_V2) + 1) {
               ASSERT (SpecificDataLen == sizeof (USB_INTERFACE_DEVICE_DESCRIPTOR_V2) + 1);
               return EFI_VOLUME_CORRUPTED;
@@ -105,7 +107,14 @@ RedfishGetHostInterfaceProtocolData (
             // This SMBIOS record is invalid, if the length of protocol specific data for
             // Redfish Over IP protocol is wrong.
             //
-            if ((*(RecordTmp + 90) + sizeof (REDFISH_OVER_IP_PROTOCOL_DATA) - 1) != ProtocolLength) {
+            if ((*(RecordTmp + REDFISH_HI_PROTOCOL_HOSTNAME_LENGTH_OFFSET) + sizeof (REDFISH_OVER_IP_PROTOCOL_DATA) - 1) != ProtocolLength) {
+              DEBUG ((
+                DEBUG_ERROR,
+                "%a: Length of protocol specific data is not match: %d != ProtocolLength(%d).\n",
+                __func__,
+                *(RecordTmp + REDFISH_HI_PROTOCOL_HOSTNAME_LENGTH_OFFSET) + sizeof (REDFISH_OVER_IP_PROTOCOL_DATA) - 1,
+                ProtocolLength
+                ));
               return EFI_SECURITY_VIOLATION;
             }
 
@@ -114,6 +123,13 @@ RedfishGetHostInterfaceProtocolData (
             // This SMBIOS record is invalid, if the length is smaller than the offset.
             //
             if (Offset > mType42Record->Hdr.Length) {
+              DEBUG ((
+                DEBUG_ERROR,
+                "%a: Offset (%d) > mType42Record->Hdr.Length (%d).\n",
+                __func__,
+                Offset,
+                mType42Record->Hdr.Length
+                ));
               return EFI_SECURITY_VIOLATION;
             }
 
-- 
2.37.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111662): https://edk2.groups.io/g/devel/message/111662
Mute This Topic: https://groups.io/mt/102763124/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [edk2-devel] [PATCH V2 7/8] RedfishPkg/HostInterfaceBmcUsbNic: Fix incorrect reference of MAC address pointer
  2023-11-23  6:47 [edk2-devel] [PATCH V2 0/8] Refine BMC USB NIC discovery and Redfish service enablement Chang, Abner via groups.io
                   ` (5 preceding siblings ...)
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 6/8] RedfishPkg/RedfishDiscovery: Refine SMBIOS 42h code Chang, Abner via groups.io
@ 2023-11-23  6:47 ` Chang, Abner via groups.io
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 8/8] RedfishPkg/HostInterfaceBmcUsbNic: Fix incorrect HI protocol record size Chang, Abner via groups.io
  2023-11-26 19:46 ` [edk2-devel] [PATCH V2 0/8] Refine BMC USB NIC discovery and Redfish service enablement Mike Maslenkin
  8 siblings, 0 replies; 16+ messages in thread
From: Chang, Abner via groups.io @ 2023-11-23  6:47 UTC (permalink / raw)
  To: devel; +Cc: Nickle Wang, Igor Kulchytskyy, Mike Maslenkin

From: Abner Chang <abner.chang@amd.com>

MAC address reference is incorrect when it is
copied to Host Interface DeviceDescriptor.

Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 .../PlatformHostInterfaceBmcUsbNicLib.c                         | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
index e5bf70cfd58..c4a71226e63 100644
--- a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
+++ b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
@@ -120,7 +120,7 @@ RedfishPlatformHostInterfaceDeviceDescriptor (
       InterfaceData->DeviceDescriptor.UsbDeviceV2.SerialNumberStr = 0;
       CopyMem (
         (VOID *)&InterfaceData->DeviceDescriptor.UsbDeviceV2.MacAddress,
-        (VOID *)&ThisInstance->MacAddress,
+        (VOID *)ThisInstance->MacAddress,
         sizeof (InterfaceData->DeviceDescriptor.UsbDeviceV2.MacAddress)
         );
       InterfaceData->DeviceDescriptor.UsbDeviceV2.Characteristics              |= (UINT16)ThisInstance->CredentialBootstrapping;
-- 
2.37.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111660): https://edk2.groups.io/g/devel/message/111660
Mute This Topic: https://groups.io/mt/102763122/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [edk2-devel] [PATCH V2 8/8] RedfishPkg/HostInterfaceBmcUsbNic: Fix incorrect HI protocol record size
  2023-11-23  6:47 [edk2-devel] [PATCH V2 0/8] Refine BMC USB NIC discovery and Redfish service enablement Chang, Abner via groups.io
                   ` (6 preceding siblings ...)
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 7/8] RedfishPkg/HostInterfaceBmcUsbNic: Fix incorrect reference of MAC address pointer Chang, Abner via groups.io
@ 2023-11-23  6:47 ` Chang, Abner via groups.io
  2023-11-26 19:30   ` Mike Maslenkin
  2023-11-26 19:46 ` [edk2-devel] [PATCH V2 0/8] Refine BMC USB NIC discovery and Redfish service enablement Mike Maslenkin
  8 siblings, 1 reply; 16+ messages in thread
From: Chang, Abner via groups.io @ 2023-11-23  6:47 UTC (permalink / raw)
  To: devel; +Cc: Nickle Wang, Igor Kulchytskyy, Mike Maslenkin

From: Abner Chang <abner.chang@amd.com>

The size of structure must be minus with byte that is
occupied by the initial array.

Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 .../PlatformHostInterfaceBmcUsbNicLib.c                       | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
index c4a71226e63..a1ce2dd3d93 100644
--- a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
+++ b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
@@ -180,7 +180,7 @@ RedfishPlatformHostInterfaceProtocolData (
       HostNameLength     = (UINT8)AsciiStrSize (HostNameString);
       ThisProtocolRecord = (MC_HOST_INTERFACE_PROTOCOL_RECORD *)AllocateZeroPool (
                                                                   sizeof (MC_HOST_INTERFACE_PROTOCOL_RECORD) - 1 +
-                                                                  sizeof (REDFISH_OVER_IP_PROTOCOL_DATA) +
+                                                                  sizeof (REDFISH_OVER_IP_PROTOCOL_DATA) - 1 +
                                                                   HostNameLength
                                                                   );
       if (ThisProtocolRecord == NULL) {
@@ -189,7 +189,7 @@ RedfishPlatformHostInterfaceProtocolData (
       }
 
       ThisProtocolRecord->ProtocolType        = MCHostInterfaceProtocolTypeRedfishOverIP;
-      ThisProtocolRecord->ProtocolTypeDataLen = sizeof (REDFISH_OVER_IP_PROTOCOL_DATA) + HostNameLength;
+      ThisProtocolRecord->ProtocolTypeDataLen = sizeof (REDFISH_OVER_IP_PROTOCOL_DATA) -1 + HostNameLength;
       RedfishOverIpData                       = (REDFISH_OVER_IP_PROTOCOL_DATA *)&ThisProtocolRecord->ProtocolTypeData[0];
       //
       // Fill up REDFISH_OVER_IP_PROTOCOL_DATA
-- 
2.37.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111661): https://edk2.groups.io/g/devel/message/111661
Mute This Topic: https://groups.io/mt/102763123/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [edk2-devel] [PATCH V2 8/8] RedfishPkg/HostInterfaceBmcUsbNic: Fix incorrect HI protocol record size
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 8/8] RedfishPkg/HostInterfaceBmcUsbNic: Fix incorrect HI protocol record size Chang, Abner via groups.io
@ 2023-11-26 19:30   ` Mike Maslenkin
  2023-11-27  4:38     ` Chang, Abner via groups.io
  0 siblings, 1 reply; 16+ messages in thread
From: Mike Maslenkin @ 2023-11-26 19:30 UTC (permalink / raw)
  To: abner.chang; +Cc: devel, Nickle Wang, Igor Kulchytskyy

On Thu, Nov 23, 2023 at 9:48 AM <abner.chang@amd.com> wrote:
>
> From: Abner Chang <abner.chang@amd.com>
>
> The size of structure must be minus with byte that is
> occupied by the initial array.
>
> Signed-off-by: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  .../PlatformHostInterfaceBmcUsbNicLib.c                       | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
> index c4a71226e63..a1ce2dd3d93 100644
> --- a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
> +++ b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
> @@ -180,7 +180,7 @@ RedfishPlatformHostInterfaceProtocolData (
>        HostNameLength     = (UINT8)AsciiStrSize (HostNameString);
>        ThisProtocolRecord = (MC_HOST_INTERFACE_PROTOCOL_RECORD *)AllocateZeroPool (
>                                                                    sizeof (MC_HOST_INTERFACE_PROTOCOL_RECORD) - 1 +
> -                                                                  sizeof (REDFISH_OVER_IP_PROTOCOL_DATA) +
> +                                                                  sizeof (REDFISH_OVER_IP_PROTOCOL_DATA) - 1 +
>                                                                    HostNameLength
>                                                                    );
>        if (ThisProtocolRecord == NULL) {
> @@ -189,7 +189,7 @@ RedfishPlatformHostInterfaceProtocolData (
>        }
>
>        ThisProtocolRecord->ProtocolType        = MCHostInterfaceProtocolTypeRedfishOverIP;
> -      ThisProtocolRecord->ProtocolTypeDataLen = sizeof (REDFISH_OVER_IP_PROTOCOL_DATA) + HostNameLength;
> +      ThisProtocolRecord->ProtocolTypeDataLen = sizeof (REDFISH_OVER_IP_PROTOCOL_DATA) -1 + HostNameLength;
>        RedfishOverIpData                       = (REDFISH_OVER_IP_PROTOCOL_DATA *)&ThisProtocolRecord->ProtocolTypeData[0];
>        //
>        // Fill up REDFISH_OVER_IP_PROTOCOL_DATA
> --
> 2.37.1.windows.1
>
Excellent!

BTW could we use zero-sized array for
REDFISH_OVER_IP_PROTOCOL_DATA::RedfishServiceHostname?
Alternatively, we could use "HostNameLength = (UINT8) AsciiStrLen
(HostNameString);"  with appropriate comment, that space for \0  is
already reserved,


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111704): https://edk2.groups.io/g/devel/message/111704
Mute This Topic: https://groups.io/mt/102763123/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [edk2-devel] [PATCH V2 2/8] RedfishPkg/RedfishHostInterfaceDxe: Add Redfish HI readiness notification
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 2/8] RedfishPkg/RedfishHostInterfaceDxe: Add Redfish HI readiness notification Chang, Abner via groups.io
@ 2023-11-26 19:35   ` Mike Maslenkin
  2023-11-27  4:33     ` Chang, Abner via groups.io
  0 siblings, 1 reply; 16+ messages in thread
From: Mike Maslenkin @ 2023-11-26 19:35 UTC (permalink / raw)
  To: abner.chang; +Cc: devel, Nickle Wang, Igor Kulchytskyy

On Thu, Nov 23, 2023 at 9:48 AM <abner.chang@amd.com> wrote:
>
> From: Abner Chang <abner.chang@amd.com>
>
> Introduce gEdkIIRedfishHostInterfaceReadyProtocolGuid
> and produce it when Redfish Host Interface is installed
> on system.
>
> Signed-off-by: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  RedfishPkg/RedfishPkg.dec                        |  3 +++
>  .../RedfishHostInterfaceDxe.inf                  |  3 ++-
>  .../RedfishHostInterfaceDxe.c                    | 16 ++++++++++++++++
>  3 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/RedfishPkg/RedfishPkg.dec b/RedfishPkg/RedfishPkg.dec
> index 0f18865cea0..e40538247c2 100644
> --- a/RedfishPkg/RedfishPkg.dec
> +++ b/RedfishPkg/RedfishPkg.dec
> @@ -90,6 +90,9 @@
>    ## Include/Protocol/EdkIIRedfishPlatformConfig.h
>    gEdkIIRedfishPlatformConfigProtocolGuid = { 0X4D94A7C7, 0X4CE4, 0X4A84, { 0X88, 0XC1, 0X33, 0X0C, 0XD4, 0XA3, 0X47, 0X67 } }
>
> +  # Redfish Host Interface ready notification protocol
> +  gEdkIIRedfishHostInterfaceReadyProtocolGuid = { 0xC3F6D062, 0x3D38, 0x4EA4, { 0x92, 0xB1, 0xE8, 0xF8, 0x02, 0x27, 0x63, 0xDF } }
> +
>  [Guids]
>    gEfiRedfishPkgTokenSpaceGuid      = { 0x4fdbccb7, 0xe829, 0x4b4c, { 0x88, 0x87, 0xb2, 0x3f, 0xd7, 0x25, 0x4b, 0x85 }}
>
> diff --git a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.inf b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.inf
> index 1cdae149aad..f969e75463f 100644
> --- a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.inf
> +++ b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.inf
> @@ -43,7 +43,8 @@
>    UefiLib
>
>  [Protocols]
> -  gEfiSmbiosProtocolGuid                ## TO_START
> +  gEfiSmbiosProtocolGuid                       ## TO_START
> +  gEdkIIRedfishHostInterfaceReadyProtocolGuid  ## PRODUCED
>
>  [Depex]
>    gEfiSmbiosProtocolGuid
> diff --git a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
> index 55a66decfc8..94c0f9b6a92 100644
> --- a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
> +++ b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
> @@ -53,7 +53,9 @@ RedfishCreateSmbiosTable42 (
>    SMBIOS_TABLE_TYPE42                *Type42Record;
>    EFI_SMBIOS_PROTOCOL                *Smbios;
>    EFI_SMBIOS_HANDLE                  MemArrayMappedAddrSmbiosHandle;
> +  EFI_HANDLE                         Handle;
>
> +  Handle = NULL;
>    //
>    // Get platform Redfish host interface device type descriptor data.
>    //
> @@ -228,6 +230,20 @@ RedfishCreateSmbiosTable42 (
>
>    Status = EFI_SUCCESS;
>
> +  //
> +  // Install Redfish Host Interface ready protocol.
> +  //
> +  Status = gBS->InstallProtocolInterface (
> +                  &Handle,
> +                  &gEdkIIRedfishHostInterfaceReadyProtocolGuid,
> +                  EFI_NATIVE_INTERFACE,
> +                  (VOID *)NULL
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Failed to install gEdkIIRedfishHostInterfaceReadyProtocolGuid.\n"));
> +    DEBUG ((DEBUG_ERROR, "PlatformConfigHandler driver may not be triggered to acquire Redfish service.\n"));
> +  }
> +
>  ON_EXIT:
>    if (DeviceDescriptor != NULL) {
>      FreePool (DeviceDescriptor);
> --
> 2.37.1.windows.1
>

You have clobbered explicit Status = EFI_SUCCESS;
is it intended? Overall result of this function does not depend from
this InstallProtocolInterface() call, SMBIOS table has been installed
successfully.
Looks like InstallProtocolInterface() should be added before Status
return value set.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111705): https://edk2.groups.io/g/devel/message/111705
Mute This Topic: https://groups.io/mt/102763119/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [edk2-devel] [PATCH V2 1/8] RedfishPkg/BmcUsbNicLib: Update BMC USB NIC searching algorithm
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 1/8] RedfishPkg/BmcUsbNicLib: Update BMC USB NIC searching algorithm Chang, Abner via groups.io
@ 2023-11-26 19:38   ` Mike Maslenkin
  2023-11-27  4:32     ` Chang, Abner via groups.io
  0 siblings, 1 reply; 16+ messages in thread
From: Mike Maslenkin @ 2023-11-26 19:38 UTC (permalink / raw)
  To: abner.chang; +Cc: devel, Nickle Wang, Igor Kulchytskyy

Hi Abner,

please find my comments below.

On Thu, Nov 23, 2023 at 9:47 AM <abner.chang@amd.com> wrote:
>
> From: Abner Chang <abner.chang@amd.com>
>
> Update BMC USB NIC searching algorithm for IPv4 only.
>
> Signed-off-by: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  .../PlatformHostInterfaceBmcUsbNicLib.c       | 188 ++++++++++++------
>  1 file changed, 128 insertions(+), 60 deletions(-)
>
> diff --git a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
> index 95900579118..e5bf70cfd58 100644
> --- a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
> +++ b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
> @@ -368,7 +368,9 @@ RetrievedBmcUsbNicInfo (
>          ));
>        CopyMem ((VOID *)&ThisInstance->RedfishIpAddressIpv4, (VOID *)&DestIpAddress->IpAddress, sizeof (DestIpAddress->IpAddress));
>        //
> -      // According to UEFI spec, the IP address at BMC USB NIC host end is the IP address at BMC end minus 1.
> +      // According to the design spec:
> +      // https://github.com/tianocore/edk2/tree/master/RedfishPkg#platform-with-bmc-and-the-bmc-exposed-usb-network-device
> +      // The IP address at BMC USB NIC host end is the IP address at BMC end minus 1.
>        //
>        CopyMem ((VOID *)&ThisInstance->HostIpAddressIpv4, (VOID *)&DestIpAddress->IpAddress, sizeof (DestIpAddress->IpAddress));
>        ThisInstance->HostIpAddressIpv4[sizeof (ThisInstance->HostIpAddressIpv4) - 1] -= 1;
> @@ -729,8 +731,10 @@ HostInterfaceIpmiCheckMacAddress (
>
>        //
>        // According to design spec in Readme file under RedfishPkg.
> -      // Compare the first five MAC address and
> -      // the 6th MAC address.
> +      // https://github.com/tianocore/edk2/tree/master/RedfishPkg#platform-with-bmc-and-the-bmc-exposed-usb-network-device
> +      // Compare the first five elements of MAC address and the 6th element of MAC address.
> +      // The 6th element of MAC address must be the 6th element of
> +      // IPMI channel MAC address minus 1.
>        //
>        if ((IpmiLanMacAddressSize != UsbNicInfo->MacAddressSize) ||
>            (CompareMem (
> @@ -738,8 +742,8 @@ HostInterfaceIpmiCheckMacAddress (
>               (VOID *)&IpmiLanChannelMacAddress.Addr,
>               IpmiLanMacAddressSize - 1
>               ) != 0) ||
> -          (IpmiLanChannelMacAddress.Addr[IpmiLanMacAddressSize - 1] !=
> -           *(UsbNicInfo->MacAddress + IpmiLanMacAddressSize - 1) - 1)
> +          ((IpmiLanChannelMacAddress.Addr[IpmiLanMacAddressSize - 1] - 1) !=
> +           *(UsbNicInfo->MacAddress + IpmiLanMacAddressSize - 1))
>            )
>        {
>          DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    MAC address is not matched.\n"));
> @@ -962,6 +966,49 @@ UsbNicSearchUsbIo (
>    return EFI_NOT_FOUND;
>  }
>
> +/**
> +  This function identifies if the USB NIC has MAC address and internet
> +  protocol device path installed. (Only support IPv4)
> +
> +  @param[in] UsbDevicePath     USB device path.
> +
> +  @retval EFI_SUCCESS          Yes, this is IPv4 SNP handle
> +  @retval EFI_NOT_FOUND        No, this is not IPv4 SNP handle
> +
> +**/
> +EFI_STATUS
> +IdentifyNetworkMessageDevicePath (
> +  IN EFI_DEVICE_PATH_PROTOCOL  *UsbDevicePath
> +  )
> +{
> +  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
> +
> +  DevicePath = UsbDevicePath;
> +  while (TRUE) {
> +    DevicePath = NextDevicePathNode (DevicePath);
> +    if (IsDevicePathEnd (DevicePath)) {
> +      DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "MAC address device path is not found on this handle.\n"));
> +      break;
> +    }
> +
> +    if ((DevicePath->Type == MESSAGING_DEVICE_PATH) && (DevicePath->SubType == MSG_MAC_ADDR_DP)) {
> +      DevicePath = NextDevicePathNode (DevicePath); // Advance to next device path protocol.
> +      if (IsDevicePathEnd (DevicePath)) {
> +        DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "IPv4 device path is not found on this handle.\n"));
> +        break;
> +      }
> +
> +      if ((DevicePath->Type == MESSAGING_DEVICE_PATH) && (DevicePath->SubType == MSG_IPv4_DP)) {
> +        return EFI_SUCCESS;
> +      }
> +
> +      break;
> +    }
> +  }
> +
> +  return EFI_NOT_FOUND;
> +}
> +
>  /**
>    This function identifies if the USB NIC is exposed by BMC as
>    the host-BMC channel.
> @@ -1025,7 +1072,7 @@ IdentifyUsbNicBmcChannel (
>      (VOID *)&Snp->Mode->CurrentAddress,
>      BmcUsbNic->MacAddressSize
>      );
> -  DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    MAC address (in size %d) for this SNP instance:\n      ", BmcUsbNic->MacAddressSize));
> +  DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    MAC address (in size %d) for this SNP instance:\n", BmcUsbNic->MacAddressSize));
>    for (Index = 0; Index < BmcUsbNic->MacAddressSize; Index++) {
>      DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "%02x ", *(BmcUsbNic->MacAddress + Index)));
>    }
> @@ -1068,7 +1115,8 @@ CheckBmcUsbNicOnHandles (
>    UINTN                     Index;
>    EFI_STATUS                Status;
>    EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
> -  BOOLEAN                   GotOneUsbNIc;
> +  BOOLEAN                   GotBmcUsbNic;
> +  CHAR16                    *DevicePathStr;
>
>    if ((HandleNumer == 0) || (HandleBuffer == NULL)) {
>      return EFI_INVALID_PARAMETER;
> @@ -1076,26 +1124,37 @@ CheckBmcUsbNicOnHandles (
>
>    DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry, #%d SNP handle\n", __func__, HandleNumer));
>
> -  GotOneUsbNIc = FALSE;
> +  GotBmcUsbNic = FALSE;
>    for (Index = 0; Index < HandleNumer; Index++) {
> +    DEBUG ((DEBUG_MANAGEABILITY, "    Locate device path on handle 0x%08x\n", *(HandleBuffer + Index)));
>      Status = gBS->HandleProtocol (
>                      *(HandleBuffer + Index),
>                      &gEfiDevicePathProtocolGuid,
>                      (VOID **)&DevicePath
>                      );
>      if (EFI_ERROR (Status)) {
> -      DEBUG ((DEBUG_ERROR, "    Failed to locate SNP on %d handle.\n", Index));
> +      DEBUG ((DEBUG_ERROR, "    Failed to locate device path on %d handle.\n", __func__, Index));

There is no format for __func__ argument.


>        continue;
>      }
>
> +    DevicePathStr = ConvertDevicePathToText (DevicePath, FALSE, FALSE);
> +    if (DevicePathStr != NULL) {
> +      DEBUG ((DEBUG_MANAGEABILITY, "    Device path: %s\n", DevicePathStr));
> +      FreePool (DevicePathStr);
> +    }
> +
>      // Check if this is an BMC exposed USB NIC device.
>      while (TRUE) {
>        if ((DevicePath->Type == MESSAGING_DEVICE_PATH) && (DevicePath->SubType == MSG_USB_DP)) {
> -        Status = IdentifyUsbNicBmcChannel (*(HandleBuffer + Index), DevicePath);
> +        Status = IdentifyNetworkMessageDevicePath (DevicePath);
>          if (!EFI_ERROR (Status)) {
> -          GotOneUsbNIc = TRUE;
> -          break;
> +          Status = IdentifyUsbNicBmcChannel (*(HandleBuffer + Index), DevicePath);

Not related to this patch, but while you are changing this could you
fix memory allocation issue in IdentifyUsbNicBmcChannel()?
The problem string is:
   BmcUsbNic->MacAddress     = AllocateZeroPool (sizeof
(BmcUsbNic->MacAddressSize));
Obviously sizeof() must be removed.
Also, AllocateZeroPool is not required because of the following
CopyMem (BmcUsbNic->MacAddress, &Snp->Mode->CurrentAddress,
BmcUsbNic->MacAddressSize);


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111706): https://edk2.groups.io/g/devel/message/111706
Mute This Topic: https://groups.io/mt/102763117/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [edk2-devel] [PATCH V2 0/8] Refine BMC USB NIC discovery and Redfish service enablement
  2023-11-23  6:47 [edk2-devel] [PATCH V2 0/8] Refine BMC USB NIC discovery and Redfish service enablement Chang, Abner via groups.io
                   ` (7 preceding siblings ...)
  2023-11-23  6:47 ` [edk2-devel] [PATCH V2 8/8] RedfishPkg/HostInterfaceBmcUsbNic: Fix incorrect HI protocol record size Chang, Abner via groups.io
@ 2023-11-26 19:46 ` Mike Maslenkin
  8 siblings, 0 replies; 16+ messages in thread
From: Mike Maslenkin @ 2023-11-26 19:46 UTC (permalink / raw)
  To: abner.chang; +Cc: devel, Nickle Wang, Igor Kulchytskyy

On Thu, Nov 23, 2023 at 9:47 AM <abner.chang@amd.com> wrote:
>
> From: Abner Chang <abner.chang@amd.com>
>
> In V2: Send additional two patches those are missed
>        in V1.
>
> This patch set updates the algorithm of BMC USB
> NIC discovery and fixes some bugs.
>
> - Add a new protocol to trigger the notification
>   of Redfish Host Interface readiness.
>   This fixes the issue Redfish config handler driver
>   acquires Redfish service before Redfish Host
>   Interface is published.
> - Add more error debug messages.
> - Address some minor issues
>
> Signed-off-by: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
>
> Abner Chang (8):
>   RedfishPkg/BmcUsbNicLib: Update BMC USB NIC searching algorithm
>   RedfishPkg/RedfishHostInterfaceDxe: Add Redfish HI readiness
>     notification
>   RedfishPkg/RedfishConfigHandler: Use Redfish HI readiness notification
>   RedfishPkg/RedfishConfigHandler: Correct the prototype of callback
>     function
>   RedfishPkg/RedfishDiscovery: Add more debug message
>   RedfishPkg/RedfishDiscovery: Refine SMBIOS 42h code
>   RedfishPkg/HostInterfaceBmcUsbNic: Fix incorrect reference of MAC
>     address pointer
>   RedfishPkg/HostInterfaceBmcUsbNic: Fix incorrect HI protocol record
>     size
>
>  RedfishPkg/RedfishPkg.dec                     |   3 +
>  .../RedfishConfigHandlerDriver.inf            |   9 +-
>  .../RedfishHostInterfaceDxe.inf               |   3 +-
>  .../RedfishDiscoverInternal.h                 |   2 +
>  .../PlatformHostInterfaceBmcUsbNicLib.c       | 194 ++++++++++++------
>  .../RedfishConfigHandlerDriver.c              | 170 +++++++++------
>  .../RedfishDiscoverDxe/RedfishDiscoverDxe.c   |  23 +++
>  .../RedfishSmbiosHostInterface.c              |  20 +-
>  .../RedfishHostInterfaceDxe.c                 |  16 ++
>  9 files changed, 312 insertions(+), 128 deletions(-)
>
> --
> 2.37.1.windows.1
>

 Hi Abner!

I've checked and tested this patch set and it works for me.
Thank you for these fixes!
I have some comments for patches 1 and 2. All others look good to me.

Regards,
Mike.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111707): https://edk2.groups.io/g/devel/message/111707
Mute This Topic: https://groups.io/mt/102763116/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [edk2-devel] [PATCH V2 1/8] RedfishPkg/BmcUsbNicLib: Update BMC USB NIC searching algorithm
  2023-11-26 19:38   ` Mike Maslenkin
@ 2023-11-27  4:32     ` Chang, Abner via groups.io
  0 siblings, 0 replies; 16+ messages in thread
From: Chang, Abner via groups.io @ 2023-11-27  4:32 UTC (permalink / raw)
  To: Mike Maslenkin; +Cc: devel@edk2.groups.io, Nickle Wang, Igor Kulchytskyy

[AMD Official Use Only - General]

Mike, your comments will be fixed in V3 9/9.
Thanks for catching this.

Abner
> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Monday, November 27, 2023 3:38 AM
> To: Chang, Abner <Abner.Chang@amd.com>
> Cc: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>; Igor
> Kulchytskyy <igork@ami.com>
> Subject: Re: [PATCH V2 1/8] RedfishPkg/BmcUsbNicLib: Update BMC USB NIC
> searching algorithm
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Hi Abner,
>
> please find my comments below.
>
> On Thu, Nov 23, 2023 at 9:47 AM <abner.chang@amd.com> wrote:
> >
> > From: Abner Chang <abner.chang@amd.com>
> >
> > Update BMC USB NIC searching algorithm for IPv4 only.
> >
> > Signed-off-by: Abner Chang <abner.chang@amd.com>
> > Cc: Nickle Wang <nicklew@nvidia.com>
> > Cc: Igor Kulchytskyy <igork@ami.com>
> > Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
> > ---
> >  .../PlatformHostInterfaceBmcUsbNicLib.c       | 188 ++++++++++++------
> >  1 file changed, 128 insertions(+), 60 deletions(-)
> >
> > diff --git
> a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInter
> faceBmcUsbNicLib.c
> b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInter
> faceBmcUsbNicLib.c
> > index 95900579118..e5bf70cfd58 100644
> > ---
> a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInter
> faceBmcUsbNicLib.c
> > +++
> b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInter
> faceBmcUsbNicLib.c
> > @@ -368,7 +368,9 @@ RetrievedBmcUsbNicInfo (
> >          ));
> >        CopyMem ((VOID *)&ThisInstance->RedfishIpAddressIpv4, (VOID
> *)&DestIpAddress->IpAddress, sizeof (DestIpAddress->IpAddress));
> >        //
> > -      // According to UEFI spec, the IP address at BMC USB NIC host end is the
> IP address at BMC end minus 1.
> > +      // According to the design spec:
> > +      //
> https://github.com/tianocore/edk2/tree/master/RedfishPkg#platform-with-
> bmc-and-the-bmc-exposed-usb-network-device
> > +      // The IP address at BMC USB NIC host end is the IP address at BMC end
> minus 1.
> >        //
> >        CopyMem ((VOID *)&ThisInstance->HostIpAddressIpv4, (VOID
> *)&DestIpAddress->IpAddress, sizeof (DestIpAddress->IpAddress));
> >        ThisInstance->HostIpAddressIpv4[sizeof (ThisInstance-
> >HostIpAddressIpv4) - 1] -= 1;
> > @@ -729,8 +731,10 @@ HostInterfaceIpmiCheckMacAddress (
> >
> >        //
> >        // According to design spec in Readme file under RedfishPkg.
> > -      // Compare the first five MAC address and
> > -      // the 6th MAC address.
> > +      //
> https://github.com/tianocore/edk2/tree/master/RedfishPkg#platform-with-
> bmc-and-the-bmc-exposed-usb-network-device
> > +      // Compare the first five elements of MAC address and the 6th element
> of MAC address.
> > +      // The 6th element of MAC address must be the 6th element of
> > +      // IPMI channel MAC address minus 1.
> >        //
> >        if ((IpmiLanMacAddressSize != UsbNicInfo->MacAddressSize) ||
> >            (CompareMem (
> > @@ -738,8 +742,8 @@ HostInterfaceIpmiCheckMacAddress (
> >               (VOID *)&IpmiLanChannelMacAddress.Addr,
> >               IpmiLanMacAddressSize - 1
> >               ) != 0) ||
> > -          (IpmiLanChannelMacAddress.Addr[IpmiLanMacAddressSize - 1] !=
> > -           *(UsbNicInfo->MacAddress + IpmiLanMacAddressSize - 1) - 1)
> > +          ((IpmiLanChannelMacAddress.Addr[IpmiLanMacAddressSize - 1] -
> 1) !=
> > +           *(UsbNicInfo->MacAddress + IpmiLanMacAddressSize - 1))
> >            )
> >        {
> >          DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    MAC address is not
> matched.\n"));
> > @@ -962,6 +966,49 @@ UsbNicSearchUsbIo (
> >    return EFI_NOT_FOUND;
> >  }
> >
> > +/**
> > +  This function identifies if the USB NIC has MAC address and internet
> > +  protocol device path installed. (Only support IPv4)
> > +
> > +  @param[in] UsbDevicePath     USB device path.
> > +
> > +  @retval EFI_SUCCESS          Yes, this is IPv4 SNP handle
> > +  @retval EFI_NOT_FOUND        No, this is not IPv4 SNP handle
> > +
> > +**/
> > +EFI_STATUS
> > +IdentifyNetworkMessageDevicePath (
> > +  IN EFI_DEVICE_PATH_PROTOCOL  *UsbDevicePath
> > +  )
> > +{
> > +  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
> > +
> > +  DevicePath = UsbDevicePath;
> > +  while (TRUE) {
> > +    DevicePath = NextDevicePathNode (DevicePath);
> > +    if (IsDevicePathEnd (DevicePath)) {
> > +      DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "MAC address device path
> is not found on this handle.\n"));
> > +      break;
> > +    }
> > +
> > +    if ((DevicePath->Type == MESSAGING_DEVICE_PATH) && (DevicePath-
> >SubType == MSG_MAC_ADDR_DP)) {
> > +      DevicePath = NextDevicePathNode (DevicePath); // Advance to next
> device path protocol.
> > +      if (IsDevicePathEnd (DevicePath)) {
> > +        DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "IPv4 device path is not
> found on this handle.\n"));
> > +        break;
> > +      }
> > +
> > +      if ((DevicePath->Type == MESSAGING_DEVICE_PATH) && (DevicePath-
> >SubType == MSG_IPv4_DP)) {
> > +        return EFI_SUCCESS;
> > +      }
> > +
> > +      break;
> > +    }
> > +  }
> > +
> > +  return EFI_NOT_FOUND;
> > +}
> > +
> >  /**
> >    This function identifies if the USB NIC is exposed by BMC as
> >    the host-BMC channel.
> > @@ -1025,7 +1072,7 @@ IdentifyUsbNicBmcChannel (
> >      (VOID *)&Snp->Mode->CurrentAddress,
> >      BmcUsbNic->MacAddressSize
> >      );
> > -  DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    MAC address (in size %d)
> for this SNP instance:\n      ", BmcUsbNic->MacAddressSize));
> > +  DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    MAC address (in size %d)
> for this SNP instance:\n", BmcUsbNic->MacAddressSize));
> >    for (Index = 0; Index < BmcUsbNic->MacAddressSize; Index++) {
> >      DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "%02x ", *(BmcUsbNic-
> >MacAddress + Index)));
> >    }
> > @@ -1068,7 +1115,8 @@ CheckBmcUsbNicOnHandles (
> >    UINTN                     Index;
> >    EFI_STATUS                Status;
> >    EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
> > -  BOOLEAN                   GotOneUsbNIc;
> > +  BOOLEAN                   GotBmcUsbNic;
> > +  CHAR16                    *DevicePathStr;
> >
> >    if ((HandleNumer == 0) || (HandleBuffer == NULL)) {
> >      return EFI_INVALID_PARAMETER;
> > @@ -1076,26 +1124,37 @@ CheckBmcUsbNicOnHandles (
> >
> >    DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry, #%d SNP handle\n",
> __func__, HandleNumer));
> >
> > -  GotOneUsbNIc = FALSE;
> > +  GotBmcUsbNic = FALSE;
> >    for (Index = 0; Index < HandleNumer; Index++) {
> > +    DEBUG ((DEBUG_MANAGEABILITY, "    Locate device path on handle
> 0x%08x\n", *(HandleBuffer + Index)));
> >      Status = gBS->HandleProtocol (
> >                      *(HandleBuffer + Index),
> >                      &gEfiDevicePathProtocolGuid,
> >                      (VOID **)&DevicePath
> >                      );
> >      if (EFI_ERROR (Status)) {
> > -      DEBUG ((DEBUG_ERROR, "    Failed to locate SNP on %d handle.\n",
> Index));
> > +      DEBUG ((DEBUG_ERROR, "    Failed to locate device path on %d
> handle.\n", __func__, Index));
>
> There is no format for __func__ argument.
>
>
> >        continue;
> >      }
> >
> > +    DevicePathStr = ConvertDevicePathToText (DevicePath, FALSE, FALSE);
> > +    if (DevicePathStr != NULL) {
> > +      DEBUG ((DEBUG_MANAGEABILITY, "    Device path: %s\n",
> DevicePathStr));
> > +      FreePool (DevicePathStr);
> > +    }
> > +
> >      // Check if this is an BMC exposed USB NIC device.
> >      while (TRUE) {
> >        if ((DevicePath->Type == MESSAGING_DEVICE_PATH) && (DevicePath-
> >SubType == MSG_USB_DP)) {
> > -        Status = IdentifyUsbNicBmcChannel (*(HandleBuffer + Index),
> DevicePath);
> > +        Status = IdentifyNetworkMessageDevicePath (DevicePath);
> >          if (!EFI_ERROR (Status)) {
> > -          GotOneUsbNIc = TRUE;
> > -          break;
> > +          Status = IdentifyUsbNicBmcChannel (*(HandleBuffer + Index),
> DevicePath);
>
> Not related to this patch, but while you are changing this could you
> fix memory allocation issue in IdentifyUsbNicBmcChannel()?
> The problem string is:
>    BmcUsbNic->MacAddress     = AllocateZeroPool (sizeof
> (BmcUsbNic->MacAddressSize));
> Obviously sizeof() must be removed.
> Also, AllocateZeroPool is not required because of the following
> CopyMem (BmcUsbNic->MacAddress, &Snp->Mode->CurrentAddress,
> BmcUsbNic->MacAddressSize);


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111713): https://edk2.groups.io/g/devel/message/111713
Mute This Topic: https://groups.io/mt/102763117/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [edk2-devel] [PATCH V2 2/8] RedfishPkg/RedfishHostInterfaceDxe: Add Redfish HI readiness notification
  2023-11-26 19:35   ` Mike Maslenkin
@ 2023-11-27  4:33     ` Chang, Abner via groups.io
  0 siblings, 0 replies; 16+ messages in thread
From: Chang, Abner via groups.io @ 2023-11-27  4:33 UTC (permalink / raw)
  To: Mike Maslenkin; +Cc: devel@edk2.groups.io, Nickle Wang, Igor Kulchytskyy

[AMD Official Use Only - General]

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Monday, November 27, 2023 3:36 AM
> To: Chang, Abner <Abner.Chang@amd.com>
> Cc: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>; Igor
> Kulchytskyy <igork@ami.com>
> Subject: Re: [PATCH V2 2/8] RedfishPkg/RedfishHostInterfaceDxe: Add
> Redfish HI readiness notification
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Thu, Nov 23, 2023 at 9:48 AM <abner.chang@amd.com> wrote:
> >
> > From: Abner Chang <abner.chang@amd.com>
> >
> > Introduce gEdkIIRedfishHostInterfaceReadyProtocolGuid
> > and produce it when Redfish Host Interface is installed
> > on system.
> >
> > Signed-off-by: Abner Chang <abner.chang@amd.com>
> > Cc: Nickle Wang <nicklew@nvidia.com>
> > Cc: Igor Kulchytskyy <igork@ami.com>
> > Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
> > ---
> >  RedfishPkg/RedfishPkg.dec                        |  3 +++
> >  .../RedfishHostInterfaceDxe.inf                  |  3 ++-
> >  .../RedfishHostInterfaceDxe.c                    | 16 ++++++++++++++++
> >  3 files changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/RedfishPkg/RedfishPkg.dec b/RedfishPkg/RedfishPkg.dec
> > index 0f18865cea0..e40538247c2 100644
> > --- a/RedfishPkg/RedfishPkg.dec
> > +++ b/RedfishPkg/RedfishPkg.dec
> > @@ -90,6 +90,9 @@
> >    ## Include/Protocol/EdkIIRedfishPlatformConfig.h
> >    gEdkIIRedfishPlatformConfigProtocolGuid = { 0X4D94A7C7, 0X4CE4,
> 0X4A84, { 0X88, 0XC1, 0X33, 0X0C, 0XD4, 0XA3, 0X47, 0X67 } }
> >
> > +  # Redfish Host Interface ready notification protocol
> > +  gEdkIIRedfishHostInterfaceReadyProtocolGuid = { 0xC3F6D062, 0x3D38,
> 0x4EA4, { 0x92, 0xB1, 0xE8, 0xF8, 0x02, 0x27, 0x63, 0xDF } }
> > +
> >  [Guids]
> >    gEfiRedfishPkgTokenSpaceGuid      = { 0x4fdbccb7, 0xe829, 0x4b4c, { 0x88,
> 0x87, 0xb2, 0x3f, 0xd7, 0x25, 0x4b, 0x85 }}
> >
> > diff --git a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.inf
> b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.inf
> > index 1cdae149aad..f969e75463f 100644
> > --- a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.inf
> > +++ b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.inf
> > @@ -43,7 +43,8 @@
> >    UefiLib
> >
> >  [Protocols]
> > -  gEfiSmbiosProtocolGuid                ## TO_START
> > +  gEfiSmbiosProtocolGuid                       ## TO_START
> > +  gEdkIIRedfishHostInterfaceReadyProtocolGuid  ## PRODUCED
> >
> >  [Depex]
> >    gEfiSmbiosProtocolGuid
> > diff --git a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
> b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
> > index 55a66decfc8..94c0f9b6a92 100644
> > --- a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
> > +++ b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
> > @@ -53,7 +53,9 @@ RedfishCreateSmbiosTable42 (
> >    SMBIOS_TABLE_TYPE42                *Type42Record;
> >    EFI_SMBIOS_PROTOCOL                *Smbios;
> >    EFI_SMBIOS_HANDLE                  MemArrayMappedAddrSmbiosHandle;
> > +  EFI_HANDLE                         Handle;
> >
> > +  Handle = NULL;
> >    //
> >    // Get platform Redfish host interface device type descriptor data.
> >    //
> > @@ -228,6 +230,20 @@ RedfishCreateSmbiosTable42 (
> >
> >    Status = EFI_SUCCESS;
> >
> > +  //
> > +  // Install Redfish Host Interface ready protocol.
> > +  //
> > +  Status = gBS->InstallProtocolInterface (
> > +                  &Handle,
> > +                  &gEdkIIRedfishHostInterfaceReadyProtocolGuid,
> > +                  EFI_NATIVE_INTERFACE,
> > +                  (VOID *)NULL
> > +                  );
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "Failed to install
> gEdkIIRedfishHostInterfaceReadyProtocolGuid.\n"));
> > +    DEBUG ((DEBUG_ERROR, "PlatformConfigHandler driver may not be
> triggered to acquire Redfish service.\n"));
> > +  }
> > +
> >  ON_EXIT:
> >    if (DeviceDescriptor != NULL) {
> >      FreePool (DeviceDescriptor);
> > --
> > 2.37.1.windows.1
> >
>
> You have clobbered explicit Status = EFI_SUCCESS;
> is it intended? Overall result of this function does not depend from
> this InstallProtocolInterface() call, SMBIOS table has been installed
> successfully.
> Looks like InstallProtocolInterface() should be added before Status
> return value set.
No, I didn’t aware this 😊.  Move it to below the new change as SMBIOS 42 is installed successfully.

Thanks
Abner




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111714): https://edk2.groups.io/g/devel/message/111714
Mute This Topic: https://groups.io/mt/102763119/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [edk2-devel] [PATCH V2 8/8] RedfishPkg/HostInterfaceBmcUsbNic: Fix incorrect HI protocol record size
  2023-11-26 19:30   ` Mike Maslenkin
@ 2023-11-27  4:38     ` Chang, Abner via groups.io
  0 siblings, 0 replies; 16+ messages in thread
From: Chang, Abner via groups.io @ 2023-11-27  4:38 UTC (permalink / raw)
  To: Mike Maslenkin; +Cc: devel@edk2.groups.io, Nickle Wang, Igor Kulchytskyy

[AMD Official Use Only - General]

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Monday, November 27, 2023 3:30 AM
> To: Chang, Abner <Abner.Chang@amd.com>
> Cc: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>; Igor
> Kulchytskyy <igork@ami.com>
> Subject: Re: [PATCH V2 8/8] RedfishPkg/HostInterfaceBmcUsbNic: Fix
> incorrect HI protocol record size
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Thu, Nov 23, 2023 at 9:48 AM <abner.chang@amd.com> wrote:
> >
> > From: Abner Chang <abner.chang@amd.com>
> >
> > The size of structure must be minus with byte that is
> > occupied by the initial array.
> >
> > Signed-off-by: Abner Chang <abner.chang@amd.com>
> > Cc: Nickle Wang <nicklew@nvidia.com>
> > Cc: Igor Kulchytskyy <igork@ami.com>
> > Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
> > ---
> >  .../PlatformHostInterfaceBmcUsbNicLib.c                       | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git
> a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInter
> faceBmcUsbNicLib.c
> b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInter
> faceBmcUsbNicLib.c
> > index c4a71226e63..a1ce2dd3d93 100644
> > ---
> a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInter
> faceBmcUsbNicLib.c
> > +++
> b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInter
> faceBmcUsbNicLib.c
> > @@ -180,7 +180,7 @@ RedfishPlatformHostInterfaceProtocolData (
> >        HostNameLength     = (UINT8)AsciiStrSize (HostNameString);
> >        ThisProtocolRecord = (MC_HOST_INTERFACE_PROTOCOL_RECORD
> *)AllocateZeroPool (
> >                                                                    sizeof
> (MC_HOST_INTERFACE_PROTOCOL_RECORD) - 1 +
> > -                                                                  sizeof
> (REDFISH_OVER_IP_PROTOCOL_DATA) +
> > +                                                                  sizeof
> (REDFISH_OVER_IP_PROTOCOL_DATA) - 1 +
> >                                                                    HostNameLength
> >                                                                    );
> >        if (ThisProtocolRecord == NULL) {
> > @@ -189,7 +189,7 @@ RedfishPlatformHostInterfaceProtocolData (
> >        }
> >
> >        ThisProtocolRecord->ProtocolType        =
> MCHostInterfaceProtocolTypeRedfishOverIP;
> > -      ThisProtocolRecord->ProtocolTypeDataLen = sizeof
> (REDFISH_OVER_IP_PROTOCOL_DATA) + HostNameLength;
> > +      ThisProtocolRecord->ProtocolTypeDataLen = sizeof
> (REDFISH_OVER_IP_PROTOCOL_DATA) -1 + HostNameLength;
> >        RedfishOverIpData                       = (REDFISH_OVER_IP_PROTOCOL_DATA
> *)&ThisProtocolRecord->ProtocolTypeData[0];
> >        //
> >        // Fill up REDFISH_OVER_IP_PROTOCOL_DATA
> > --
> > 2.37.1.windows.1
> >
> Excellent!
>
> BTW could we use zero-sized array for
> REDFISH_OVER_IP_PROTOCOL_DATA::RedfishServiceHostname?
> Alternatively, we could use "HostNameLength = (UINT8) AsciiStrLen
> (HostNameString);"  with appropriate comment, that space for \0  is
> already reserved,
Let's update this to zeroed size array in next patch to make the code easily read, as we have to update all other structure members those are declared as one byte array and remove "-1" from the code as well. You are also welcome to update these structures after our changes are merged to the master branch.
Thanks
Abner



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111715): https://edk2.groups.io/g/devel/message/111715
Mute This Topic: https://groups.io/mt/102763123/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2023-11-27  4:38 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-23  6:47 [edk2-devel] [PATCH V2 0/8] Refine BMC USB NIC discovery and Redfish service enablement Chang, Abner via groups.io
2023-11-23  6:47 ` [edk2-devel] [PATCH V2 1/8] RedfishPkg/BmcUsbNicLib: Update BMC USB NIC searching algorithm Chang, Abner via groups.io
2023-11-26 19:38   ` Mike Maslenkin
2023-11-27  4:32     ` Chang, Abner via groups.io
2023-11-23  6:47 ` [edk2-devel] [PATCH V2 2/8] RedfishPkg/RedfishHostInterfaceDxe: Add Redfish HI readiness notification Chang, Abner via groups.io
2023-11-26 19:35   ` Mike Maslenkin
2023-11-27  4:33     ` Chang, Abner via groups.io
2023-11-23  6:47 ` [edk2-devel] [PATCH V2 3/8] RedfishPkg/RedfishConfigHandler: Use " Chang, Abner via groups.io
2023-11-23  6:47 ` [edk2-devel] [PATCH V2 4/8] RedfishPkg/RedfishConfigHandler: Correct the prototype of callback function Chang, Abner via groups.io
2023-11-23  6:47 ` [edk2-devel] [PATCH V2 5/8] RedfishPkg/RedfishDiscovery: Add more debug message Chang, Abner via groups.io
2023-11-23  6:47 ` [edk2-devel] [PATCH V2 6/8] RedfishPkg/RedfishDiscovery: Refine SMBIOS 42h code Chang, Abner via groups.io
2023-11-23  6:47 ` [edk2-devel] [PATCH V2 7/8] RedfishPkg/HostInterfaceBmcUsbNic: Fix incorrect reference of MAC address pointer Chang, Abner via groups.io
2023-11-23  6:47 ` [edk2-devel] [PATCH V2 8/8] RedfishPkg/HostInterfaceBmcUsbNic: Fix incorrect HI protocol record size Chang, Abner via groups.io
2023-11-26 19:30   ` Mike Maslenkin
2023-11-27  4:38     ` Chang, Abner via groups.io
2023-11-26 19:46 ` [edk2-devel] [PATCH V2 0/8] Refine BMC USB NIC discovery and Redfish service enablement Mike Maslenkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox