public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch v1 0/2] Support SNP over UsbIo-based UNDI driver
@ 2022-11-04  8:30 Frederik van Hövell
  2022-11-04  8:30 ` [Patch v1 1/2] [NetworkPkg/SnpDxe] More logging to see why SnpDxe fails to start Frederik van Hövell
  2022-11-04  8:30 ` [Patch v1 2/2] [NetworkPkg/SnpDxe] Support SNP over UsbIo-based UNDI driver Frederik van Hövell
  0 siblings, 2 replies; 3+ messages in thread
From: Frederik van Hövell @ 2022-11-04  8:30 UTC (permalink / raw)
  To: devel; +Cc: Maciej Rabeda, Jiaxin Wu, Siyuan Fu, Frederik van Hövell

This patch series will add support for starting and stopping a SnpDxe
instance over a UsbIo based UNDI (nic) driver.

Tested with a (slightly modified) `lan78xx_uefi` UNDI driver for the
Raspberry Pi 3B+. The original lan78xx_uefi driver is available at
https://github.com/microchip-ung/lan78xx_uefi, but requires additional
changes to work on AARCH64.

Frederik van Hövell (2):
  [NetworkPkg/SnpDxe] More logging to see why SnpDxe fails to start
  [NetworkPkg/SnpDxe] Support SNP over UsbIo-based UNDI driver

 NetworkPkg/SnpDxe/SnpDxe.inf |   2 +-
 NetworkPkg/SnpDxe/Snp.h      |   4 +
 NetworkPkg/SnpDxe/Snp.c      | 228 +++++++++++++-------
 3 files changed, 151 insertions(+), 83 deletions(-)

-- 
2.35.1


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

* [Patch v1 1/2] [NetworkPkg/SnpDxe] More logging to see why SnpDxe fails to start
  2022-11-04  8:30 [Patch v1 0/2] Support SNP over UsbIo-based UNDI driver Frederik van Hövell
@ 2022-11-04  8:30 ` Frederik van Hövell
  2022-11-04  8:30 ` [Patch v1 2/2] [NetworkPkg/SnpDxe] Support SNP over UsbIo-based UNDI driver Frederik van Hövell
  1 sibling, 0 replies; 3+ messages in thread
From: Frederik van Hövell @ 2022-11-04  8:30 UTC (permalink / raw)
  To: devel; +Cc: Maciej Rabeda, Jiaxin Wu, Siyuan Fu, Frederik van Hövell

Add debug logging to see where SnpDxe is failing using a UsbIo UNDI
driver.

Signed-off-by: Frederik van Hövell <frederik@fvhovell.nl>
---
 NetworkPkg/SnpDxe/Snp.h | 1 +
 NetworkPkg/SnpDxe/Snp.c | 7 ++++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/NetworkPkg/SnpDxe/Snp.h b/NetworkPkg/SnpDxe/Snp.h
index d57804ca2873..dec238c9eb2c 100644
--- a/NetworkPkg/SnpDxe/Snp.h
+++ b/NetworkPkg/SnpDxe/Snp.h
@@ -27,6 +27,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/MemoryAllocationLib.h>
 #include <Library/PrintLib.h>
 #include <Library/PcdLib.h>
+#include <Library/DevicePathLib.h>
 
 #include <IndustryStandard/Pci.h>
 #include <IndustryStandard/Acpi.h>
diff --git a/NetworkPkg/SnpDxe/Snp.c b/NetworkPkg/SnpDxe/Snp.c
index 95361c3cd343..86bf527f31dd 100644
--- a/NetworkPkg/SnpDxe/Snp.c
+++ b/NetworkPkg/SnpDxe/Snp.c
@@ -160,6 +160,7 @@ SimpleNetworkDriverSupported (
   // check the version, we don't want to connect to the undi16
   //
   if (NiiProtocol->Type != EfiNetworkInterfaceUndi) {
+    DEBUG ((DEBUG_NET, "%a: Unsupported type %a for handle %p\n", __FUNCTION__, NiiProtocol->Type, Controller));
     Status = EFI_UNSUPPORTED;
     goto Done;
   }
@@ -218,7 +219,7 @@ SimpleNetworkDriverSupported (
   }
 
   Status = EFI_SUCCESS;
-  DEBUG ((DEBUG_INFO, "Support(): supported on %p\n", Controller));
+  DEBUG ((DEBUG_INFO, "%a: supported on %p\n", __FUNCTION__, Controller));
 
 Done:
   gBS->CloseProtocol (
@@ -283,6 +284,7 @@ SimpleNetworkDriverStart (
                   );
 
   if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a: Failed to get DevicePath for driver %p and handle %p - status %r\n", __FUNCTION__, This->DriverBindingHandle, Controller, Status));
     return Status;
   }
 
@@ -293,6 +295,7 @@ SimpleNetworkDriverStart (
                   );
 
   if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a: Failed to locate DevicePath using PCI path %s - status %r\n", __FUNCTION__, ConvertDevicePathToText(NiiDevicePath, TRUE, TRUE), Status));
     return Status;
   }
 
@@ -305,6 +308,7 @@ SimpleNetworkDriverStart (
                   EFI_OPEN_PROTOCOL_GET_PROTOCOL
                   );
   if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a: Failed to open PciIo protocol for driver %p and handle %p - status %r\n", __FUNCTION__, This->DriverBindingHandle, Handle, Status));
     return Status;
   }
 
@@ -320,6 +324,7 @@ SimpleNetworkDriverStart (
                   EFI_OPEN_PROTOCOL_BY_DRIVER
                   );
   if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a: Failed to open NII protocol for driver %p and controller %p - status %r\n", __FUNCTION__, This->DriverBindingHandle, Controller, Status));
     gBS->CloseProtocol (
            Controller,
            &gEfiDevicePathProtocolGuid,
-- 
2.35.1


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

* [Patch v1 2/2] [NetworkPkg/SnpDxe] Support SNP over UsbIo-based UNDI driver
  2022-11-04  8:30 [Patch v1 0/2] Support SNP over UsbIo-based UNDI driver Frederik van Hövell
  2022-11-04  8:30 ` [Patch v1 1/2] [NetworkPkg/SnpDxe] More logging to see why SnpDxe fails to start Frederik van Hövell
@ 2022-11-04  8:30 ` Frederik van Hövell
  1 sibling, 0 replies; 3+ messages in thread
From: Frederik van Hövell @ 2022-11-04  8:30 UTC (permalink / raw)
  To: devel; +Cc: Maciej Rabeda, Jiaxin Wu, Siyuan Fu, Frederik van Hövell

Refactor SimpleNetworkDriverStart() and SimpleNetworkDriverStop() to
add support over UsbIo UNDI driver next to PciIo UNDI driver.

Signed-off-by: Frederik van Hövell <frederik@fvhovell.nl>
---
 NetworkPkg/SnpDxe/SnpDxe.inf |   2 +-
 NetworkPkg/SnpDxe/Snp.h      |   3 +
 NetworkPkg/SnpDxe/Snp.c      | 225 ++++++++++++--------
 3 files changed, 146 insertions(+), 84 deletions(-)

diff --git a/NetworkPkg/SnpDxe/SnpDxe.inf b/NetworkPkg/SnpDxe/SnpDxe.inf
index d16f1888b30f..678e5efc5d01 100644
--- a/NetworkPkg/SnpDxe/SnpDxe.inf
+++ b/NetworkPkg/SnpDxe/SnpDxe.inf
@@ -54,7 +54,6 @@ [Packages]
   MdePkg/MdePkg.dec
   NetworkPkg/NetworkPkg.dec
 
-
 [LibraryClasses]
   UefiLib
   BaseLib
@@ -72,6 +71,7 @@ [Protocols]
   gEfiDevicePathProtocolGuid                    ## TO_START
   gEfiNetworkInterfaceIdentifierProtocolGuid_31 ## TO_START
   gEfiPciIoProtocolGuid                         ## TO_START
+  gEfiUsbIoProtocolGuid                         ## TO_START
 
 [Pcd]
   gEfiNetworkPkgTokenSpaceGuid.PcdSnpCreateExitBootServicesEvent   ## CONSUMES
diff --git a/NetworkPkg/SnpDxe/Snp.h b/NetworkPkg/SnpDxe/Snp.h
index dec238c9eb2c..cbc36c7b073d 100644
--- a/NetworkPkg/SnpDxe/Snp.h
+++ b/NetworkPkg/SnpDxe/Snp.h
@@ -13,6 +13,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include <Protocol/SimpleNetwork.h>
 #include <Protocol/PciIo.h>
+#include <Protocol/UsbIo.h>
+#include <Protocol/DeviceIo.h>
 #include <Protocol/NetworkInterfaceIdentifier.h>
 #include <Protocol/DevicePath.h>
 
@@ -94,6 +96,7 @@ typedef struct {
   VOID                           *FillHeaderBufferUnmap;
 
   EFI_PCI_IO_PROTOCOL            *PciIo;
+  EFI_USB_IO_PROTOCOL            *UsbIo;
   UINT8                          IoBarIndex;
   UINT8                          MemoryBarIndex;
 
diff --git a/NetworkPkg/SnpDxe/Snp.c b/NetworkPkg/SnpDxe/Snp.c
index 86bf527f31dd..6ecaa57feda3 100644
--- a/NetworkPkg/SnpDxe/Snp.c
+++ b/NetworkPkg/SnpDxe/Snp.c
@@ -268,6 +268,7 @@ SimpleNetworkDriverStart (
   UINT8                                      BarIndex;
   PXE_STATFLAGS                              InitStatFlags;
   EFI_PCI_IO_PROTOCOL                        *PciIo;
+  EFI_USB_IO_PROTOCOL                        *UsbIo;
   EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR          *BarDesc;
   BOOLEAN                                    FoundIoBar;
   BOOLEAN                                    FoundMemoryBar;
@@ -293,22 +294,44 @@ SimpleNetworkDriverStart (
                   &NiiDevicePath,
                   &Handle
                   );
-
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a: Failed to locate DevicePath using PCI path %s - status %r\n", __FUNCTION__, ConvertDevicePathToText(NiiDevicePath, TRUE, TRUE), Status));
-    return Status;
+    DEBUG ((DEBUG_VERBOSE, "%a: Failed to locate DevicePath using PciIoProtocol path %s - status %r\n", __FUNCTION__, ConvertDevicePathToText(NiiDevicePath, TRUE, TRUE), Status));
+
+    Status = gBS->LocateDevicePath (
+                    &gEfiUsbIoProtocolGuid,
+                    &NiiDevicePath,
+                    &Handle
+                    );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_VERBOSE, "%a: Failed to locate DevicePath using UsbIoProtocol path %s - status %r\n", __FUNCTION__, ConvertDevicePathToText(NiiDevicePath, TRUE, TRUE), Status));
+
+      return Status;
+    } else {
+      // Found NiiDevicePath using UsbIoProtocol
+      Status = gBS->OpenProtocol (
+                      Handle,
+                      &gEfiUsbIoProtocolGuid,
+                      (VOID **) &UsbIo,
+                      This->DriverBindingHandle,
+                      Controller,
+                      EFI_OPEN_PROTOCOL_GET_PROTOCOL
+                      );
+    }
+  } else {
+    // Found NiiDevicePath using PciIoProtocol
+    Status = gBS->OpenProtocol (
+                    Handle,
+                    &gEfiPciIoProtocolGuid,
+                    (VOID **) &PciIo,
+                    This->DriverBindingHandle,
+                    Controller,
+                    EFI_OPEN_PROTOCOL_GET_PROTOCOL
+                    );
   }
 
-  Status = gBS->OpenProtocol (
-                  Handle,
-                  &gEfiPciIoProtocolGuid,
-                  (VOID **)&PciIo,
-                  This->DriverBindingHandle,
-                  Controller,
-                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
-                  );
+
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a: Failed to open PciIo protocol for driver %p and handle %p - status %r\n", __FUNCTION__, This->DriverBindingHandle, Handle, Status));
+    DEBUG ((DEBUG_ERROR, "%a: Failed to open protocol for driver %p and handle %p - status %r\n", __FUNCTION__, This->DriverBindingHandle, Handle, Status));
     return Status;
   }
 
@@ -323,7 +346,7 @@ SimpleNetworkDriverStart (
                   Controller,
                   EFI_OPEN_PROTOCOL_BY_DRIVER
                   );
-  if (EFI_ERROR (Status)) {
+  if (EFI_ERROR (Status) || Nii == NULL) {
     DEBUG ((DEBUG_ERROR, "%a: Failed to open NII protocol for driver %p and controller %p - status %r\n", __FUNCTION__, This->DriverBindingHandle, Controller, Status));
     gBS->CloseProtocol (
            Controller,
@@ -334,7 +357,7 @@ SimpleNetworkDriverStart (
     return Status;
   }
 
-  DEBUG ((DEBUG_INFO, "Start(): UNDI3.1 found\n"));
+  DEBUG ((DEBUG_INFO, "%a: UNDI3.1 found\n", __FUNCTION__));
 
   Pxe = (PXE_UNDI *)(UINTN)(Nii->Id);
 
@@ -362,14 +385,22 @@ SimpleNetworkDriverStart (
   // OK, we like this UNDI, and we know snp is not already there on this handle
   // Allocate and initialize a new simple network protocol structure.
   //
-  Status = PciIo->AllocateBuffer (
-                    PciIo,
-                    AllocateAnyPages,
-                    EfiBootServicesData,
-                    SNP_MEM_PAGES (sizeof (SNP_DRIVER)),
-                    &Address,
-                    0
-                    );
+  if (PciIo != NULL) {
+    Status = PciIo->AllocateBuffer(
+                      PciIo,
+                      AllocateAnyPages,
+                      EfiBootServicesData,
+                      SNP_MEM_PAGES (sizeof (SNP_DRIVER)),
+                      &Address,
+                      0
+                      );
+  } else if (UsbIo != NULL) {
+    DEBUG ((DEBUG_INFO, "%a: AllocatePool for SNP structure with size: %d\n", __FUNCTION__, sizeof (SNP_DRIVER) ));
+    Address = AllocatePool(sizeof (SNP_DRIVER));
+    if (Address == NULL) {
+      Status = EFI_OUT_OF_RESOURCES;
+    }
+  }
 
   if (Status != EFI_SUCCESS) {
     DEBUG ((DEBUG_NET, "\nCould not allocate SNP_DRIVER structure.\n"));
@@ -381,6 +412,7 @@ SimpleNetworkDriverStart (
   ZeroMem (Snp, sizeof (SNP_DRIVER));
 
   Snp->PciIo     = PciIo;
+  Snp->UsbIo     = UsbIo;
   Snp->Signature = SNP_DRIVER_SIGNATURE;
 
   EfiInitializeLock (&Snp->Lock, TPL_NOTIFY);
@@ -450,14 +482,22 @@ SimpleNetworkDriverStart (
   // -it is OK to allocate one global set of CPB, DB pair for each UNDI
   // interface as EFI does not multi-task and so SNP will not be re-entered!
   //
-  Status = PciIo->AllocateBuffer (
-                    PciIo,
-                    AllocateAnyPages,
-                    EfiBootServicesData,
-                    SNP_MEM_PAGES (4096),
-                    &Address,
-                    0
-                    );
+  if (PciIo != NULL) {
+    Status = PciIo->AllocateBuffer (
+                      PciIo,
+                      AllocateAnyPages,
+                      EfiBootServicesData,
+                      SNP_MEM_PAGES (4096),
+                      &Address,
+                      0
+                      );
+  } else if (UsbIo != NULL) {
+    DEBUG ((DEBUG_INFO, "%a: AllocatePool for CPB and DB buffers with size: %d\n", __FUNCTION__, 6144));
+    Address = AllocatePool (6144);
+    if (Address == NULL) {
+      Status = EFI_OUT_OF_RESOURCES;
+    }
+  }
 
   if (Status != EFI_SUCCESS) {
     DEBUG ((DEBUG_NET, "\nCould not allocate CPB and DB structures.\n"));
@@ -467,42 +507,44 @@ SimpleNetworkDriverStart (
   Snp->Cpb = (VOID *)(UINTN)Address;
   Snp->Db  = (VOID *)((UINTN)Address + 2048);
 
-  //
-  // Find the correct BAR to do IO.
-  //
-  // Enumerate through the PCI BARs for the device to determine which one is
-  // the IO BAR.  Save the index of the BAR into the adapter info structure.
-  // for regular 32bit BARs, 0 is memory mapped, 1 is io mapped
-  //
-  Snp->MemoryBarIndex = PCI_MAX_BAR;
-  Snp->IoBarIndex     = PCI_MAX_BAR;
-  FoundMemoryBar      = FALSE;
-  FoundIoBar          = FALSE;
-  for (BarIndex = 0; BarIndex < PCI_MAX_BAR; BarIndex++) {
-    Status = PciIo->GetBarAttributes (
-                      PciIo,
-                      BarIndex,
-                      NULL,
-                      (VOID **)&BarDesc
-                      );
-    if (Status == EFI_UNSUPPORTED) {
-      continue;
-    } else if (EFI_ERROR (Status)) {
-      goto Error_DeleteSNP;
-    }
+  if (PciIo != NULL) {
+    //
+    // Find the correct BAR to do IO.
+    //
+    // Enumerate through the PCI BARs for the device to determine which one is
+    // the IO BAR.  Save the index of the BAR into the adapter info structure.
+    // for regular 32bit BARs, 0 is memory mapped, 1 is io mapped
+    //
+    Snp->MemoryBarIndex = PCI_MAX_BAR;
+    Snp->IoBarIndex     = PCI_MAX_BAR;
+    FoundMemoryBar      = FALSE;
+    FoundIoBar          = FALSE;
+    for (BarIndex = 0; BarIndex < PCI_MAX_BAR; BarIndex++) {
+      Status = PciIo->GetBarAttributes (
+                        PciIo,
+                        BarIndex,
+                        NULL,
+                        (VOID **)&BarDesc
+                        );
+      if (Status == EFI_UNSUPPORTED) {
+        continue;
+      } else if (EFI_ERROR (Status)) {
+        goto Error_DeleteSNP;
+      }
 
-    if ((!FoundMemoryBar) && (BarDesc->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM)) {
-      Snp->MemoryBarIndex = BarIndex;
-      FoundMemoryBar      = TRUE;
-    } else if ((!FoundIoBar) && (BarDesc->ResType == ACPI_ADDRESS_SPACE_TYPE_IO)) {
-      Snp->IoBarIndex = BarIndex;
-      FoundIoBar      = TRUE;
-    }
+      if ((!FoundMemoryBar) && (BarDesc->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM)) {
+        Snp->MemoryBarIndex = BarIndex;
+        FoundMemoryBar      = TRUE;
+      } else if ((!FoundIoBar) && (BarDesc->ResType == ACPI_ADDRESS_SPACE_TYPE_IO)) {
+        Snp->IoBarIndex = BarIndex;
+        FoundIoBar      = TRUE;
+      }
 
-    FreePool (BarDesc);
+      FreePool (BarDesc);
 
-    if (FoundMemoryBar && FoundIoBar) {
-      break;
+      if (FoundMemoryBar && FoundIoBar) {
+        break;
+      }
     }
   }
 
@@ -682,11 +724,15 @@ SimpleNetworkDriverStart (
     return Status;
   }
 
-  PciIo->FreeBuffer (
-           PciIo,
-           SNP_MEM_PAGES (4096),
-           Snp->Cpb
-           );
+  if (PciIo != NULL) {
+    PciIo->FreeBuffer (
+             PciIo,
+             SNP_MEM_PAGES (4096),
+             Snp->Cpb
+             );
+  } else if (UsbIo != NULL) {
+    FreePool (Snp->Cpb);
+  }
 
 Error_DeleteSNP:
 
@@ -694,11 +740,15 @@ Error_DeleteSNP:
     FreePool (Snp->RecycledTxBuf);
   }
 
-  PciIo->FreeBuffer (
-           PciIo,
-           SNP_MEM_PAGES (sizeof (SNP_DRIVER)),
-           Snp
-           );
+  if (PciIo != NULL) {
+    PciIo->FreeBuffer (
+             PciIo,
+             SNP_MEM_PAGES (sizeof (SNP_DRIVER)),
+             Snp
+             );
+  } else if (UsbIo != NULL) {
+    FreePool (Snp);
+  }
 NiiError:
   gBS->CloseProtocol (
          Controller,
@@ -755,6 +805,7 @@ SimpleNetworkDriverStop (
   EFI_SIMPLE_NETWORK_PROTOCOL  *SnpProtocol;
   SNP_DRIVER                   *Snp;
   EFI_PCI_IO_PROTOCOL          *PciIo;
+  EFI_USB_IO_PROTOCOL          *UsbIo;
 
   //
   // Get our context back.
@@ -811,17 +862,25 @@ SimpleNetworkDriverStop (
   FreePool (Snp->RecycledTxBuf);
 
   PciIo = Snp->PciIo;
-  PciIo->FreeBuffer (
-           PciIo,
-           SNP_MEM_PAGES (4096),
-           Snp->Cpb
-           );
+  if (PciIo != NULL) {
+    PciIo->FreeBuffer (
+             PciIo,
+             SNP_MEM_PAGES (4096),
+             Snp->Cpb
+             );
 
-  PciIo->FreeBuffer (
-           PciIo,
-           SNP_MEM_PAGES (sizeof (SNP_DRIVER)),
-           Snp
-           );
+    PciIo->FreeBuffer (
+             PciIo,
+             SNP_MEM_PAGES (sizeof (SNP_DRIVER)),
+             Snp
+             );
+  } else {
+    UsbIo = Snp->UsbIo;
+    FreePool (Snp->Cpb);
+
+    FreePool (Snp);
+
+  }
 
   return Status;
 }
-- 
2.35.1


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

end of thread, other threads:[~2022-11-04  8:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-04  8:30 [Patch v1 0/2] Support SNP over UsbIo-based UNDI driver Frederik van Hövell
2022-11-04  8:30 ` [Patch v1 1/2] [NetworkPkg/SnpDxe] More logging to see why SnpDxe fails to start Frederik van Hövell
2022-11-04  8:30 ` [Patch v1 2/2] [NetworkPkg/SnpDxe] Support SNP over UsbIo-based UNDI driver Frederik van Hövell

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