* [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