On 05/20/20 10:37, Sandeep Dhanvada wrote: Hi,
I am using edk2(commit id: 0f1946b) with X64 ARCH, GCC49 toolchain and using PCI IO Protocol functions for memory Allocation/Map and Free/Unmap.
As per our NIC driver requirement, there is a need of 1600 bytes(MTU) of allocated memory for 6KB depth of descriptors pre-allocated at initialization and freed at unload. I am observing memory leak while continuously performing load and unload iterations of driver from UEFI Shell.
I simulated this in OVMF with PCI passthrough and observed leaks in this case also. Attached is the SampleDriver which uses PCI IO Protocol which Allocates and Maps memory while initialization and will Free and Unmap in unload path. In OVMF case, 12KB memory is leaked after every load unload iteration of Sample Driver.
Not sure if this expected behavior or indeed a memory leak. The test program you have included suffers from both resource leaks and double-free's. That means undefined behavior. See below.
pci_io_template_driver.patch
diff --git a/MdeModulePkg/Application/SampleDriver/SampleDriver.c b/MdeModulePkg/Application/SampleDriver/SampleDriver.c
new file mode 100755
index 0000000..a5a4db2
--- /dev/null
+++ b/MdeModulePkg/Application/SampleDriver/SampleDriver.c
@@ -0,0 +1,226 @@
+#include <Uefi.h>
+#include <Protocol/DriverBinding.h>
+#include <Protocol/ComponentName2.h>
+#include <Protocol/ComponentName.h>
+#include <Protocol/PciIo.h>
+#include <Library/UefiLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/DebugLib.h>
+#include <Library/BaseLib.h>
+#include <Uefi/UefiBaseType.h>
+
+#define VERSION 0x10
+
+EFI_STATUS
+EFIAPI
+Supported(
+ IN EFI_DRIVER_BINDING_PROTOCOL *This,
+ IN EFI_HANDLE ControllerHandle,
+ IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
+ )
+{
+ EFI_PCI_IO_PROTOCOL *PciIo = NULL;
+ EFI_STATUS Status = gBS->HandleProtocol(ControllerHandle,
+ &gEfiPciIoProtocolGuid,
+ (VOID **) &PciIo);
+
+ if (EFI_ERROR(Status))
+ return EFI_UNSUPPORTED;
+
+ return EFI_SUCCESS;
+}
Your test driver claims to support (i.e., claims the ability to drive) any PCI device whatsoever (any handle with a PciIo protocol interface on it). But: +
+EFI_PCI_IO_PROTOCOL *PciIo;
+VOID *PciBuf[SIZE_2KB * 3];
+VOID *Mapping[SIZE_2KB * 3];
+UINTN pages[SIZE_2KB * 3];
+EFI_PHYSICAL_ADDRESS DmaAddr[SIZE_2KB * 3]; you gave all of these objects static storage duration. Then, +
+EFI_STATUS
+EFIAPI
+Start(
+ IN EFI_DRIVER_BINDING_PROTOCOL *This,
+ IN EFI_HANDLE ControllerHandle,
+ IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
+ )
+{
+ UINTN i = 0;
+ EFI_STATUS Status = gBS->OpenProtocol(ControllerHandle,
+ &gEfiPciIoProtocolGuid,
+ (VOID **) &PciIo,
+ This->DriverBindingHandle,
+ ControllerHandle,
+ EFI_OPEN_PROTOCOL_BY_DRIVER);
+ if (EFI_ERROR(Status))
+ return Status; your driver will bind any handle with a PciIo that is not yet open-by-a-driver. And then, + for (i=0; i < (SIZE_2KB*3) ; i++)
+ {
+ pages[i] = EFI_SIZE_TO_PAGES(1600);
+ Status = PciIo->AllocateBuffer(PciIo, 0,
+ EfiBootServicesData,
+ pages[i],
+ &PciBuf[i], 0);
+ UINTN MappedSize = 1600;
+ Status = PciIo->Map(PciIo, EfiPciIoOperationBusMasterCommonBuffer,
+ PciBuf[i], &MappedSize, &DmaAddr[i], &Mapping[i]);
+ } the 2nd, 3rd, 4th etc time this loop runs (for the 2nd, 3rd, 4th ... handle that Start() binds), you leak previously allocated resources, namely those referenced in PciBuf and Mapping. + return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+Stop(
+ IN EFI_DRIVER_BINDING_PROTOCOL *This,
+ IN EFI_HANDLE ControllerHandle,
+ IN UINTN NumberOfChildren,
+ IN EFI_HANDLE *ChildHandleBuffer OPTIONAL
+ )
+{
+ UINTN i = 0;
+ AsciiPrint("Unload PCI test Driver\n");
+ for (i=0; i < (SIZE_2KB*3) ; i++)
+ {
+ PciIo->Unmap(PciIo, Mapping[i]);
+ PciIo->FreeBuffer(PciIo, pages[i], PciBuf[i]);
+ } And the 2nd, 3rd, 4th etc time this loop runs (for the 2nd, 3rd, 4th... handle that Stop() unbinds), the Unmap() and FreeBuffer() calls attempt to release garbage (already released resources). General hint: whenever in doubt about resource lifecycles, debug-log the address(es) returned on the allocation path, and debug-log the address(es) freed on the release path. Equipped with such a text file, you can use "sort" or other utilities to couple alloc & free actions, and to detect mismatches. Thanks Laszlo +
+ return gBS->CloseProtocol(ControllerHandle,
+ &gEfiPciIoProtocolGuid,
+ This->DriverBindingHandle,
+ ControllerHandle);
+}
+
+EFI_DRIVER_BINDING_PROTOCOL gDriverBinding = {
+ Supported,
+ Start,
+ Stop,
+ VERSION,
+ NULL,
+ NULL
+};
+
+EFI_STATUS
+EFIAPI
+GetDriverName (
+ IN EFI_COMPONENT_NAME2_PROTOCOL *This,
+ IN CHAR8 *Language,
+ OUT CHAR16 **DriverName
+ )
+{
+ StrCpyS(*DriverName, 14, L"Sample Driver");
+ return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+GetControllerName(
+ IN EFI_COMPONENT_NAME2_PROTOCOL *This,
+ IN EFI_HANDLE ControllerHandle,
+ IN EFI_HANDLE ChildHandle OPTIONAL,
+ IN CHAR8 *Language,
+ OUT CHAR16 **ControllerName
+ )
+{
+ return EFI_SUCCESS;
+}
+
+GLOBAL_REMOVE_IF_UNREFERENCED
+EFI_COMPONENT_NAME_PROTOCOL gComponentName = {
+ (EFI_COMPONENT_NAME_GET_DRIVER_NAME) GetDriverName,
+ (EFI_COMPONENT_NAME_GET_CONTROLLER_NAME) GetControllerName,
+ "eng"
+};
+
+GLOBAL_REMOVE_IF_UNREFERENCED
+EFI_COMPONENT_NAME2_PROTOCOL gComponentName2 = {
+ GetDriverName,
+ GetControllerName,
+ "en"
+};
+
+EFI_STATUS EFIAPI
+Unload (
+ IN EFI_HANDLE ImageHandle
+ )
+{
+ EFI_STATUS Status;
+ EFI_HANDLE *HandleBuffer;
+ UINTN HandleCount;
+ UINTN Index;
+
+
+ //
+ // Retrieve array of all handles in the handle database
+ //
+ Status = gBS->LocateHandleBuffer (
+ AllHandles,
+ NULL,
+ NULL,
+ &HandleCount,
+ &HandleBuffer
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ //
+ // Disconnect the current driver from handles in the handle database
+ //
+ for (Index = 0;
+ Index < HandleCount; Index++) {
+ Status = gBS->DisconnectController (
+ HandleBuffer[Index],
+ gImageHandle,
+ NULL
+ );
+ }
+
+ //
+ // Free the array of handles
+ //
+ FreePool (HandleBuffer);
+
+ //
+ // Uninstall protocols installed in the driver entry point
+ //
+ Status = EfiLibUninstallDriverBindingComponentName2 (
+ &gDriverBinding,
+ &gComponentName,
+ &gComponentName2
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ //
+ // Do any additional cleanup that is required for this driver
+ //
+
+ return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+EntryPoint (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ //
+ // Install driver model protocol(s).
+ //
+ Status = EfiLibInstallDriverBindingComponentName2 (
+ ImageHandle, // ImageHandle
+ SystemTable, // SystemTable
+ &gDriverBinding, // DriverBinding
+ ImageHandle, // DriverBindingHandle
+ &gComponentName, // ComponentName
+ &gComponentName2 // ComponentName2
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ return Status;
+}
diff --git a/MdeModulePkg/Application/SampleDriver/SampleDriver.inf b/MdeModulePkg/Application/SampleDriver/SampleDriver.inf
new file mode 100755
index 0000000..cf34e05
--- /dev/null
+++ b/MdeModulePkg/Application/SampleDriver/SampleDriver.inf
@@ -0,0 +1,24 @@
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = TestPciIoDriver
+ FILE_GUID = DA87D340-15C0-4824-9BF3-D52286674BEF
+ MODULE_TYPE = UEFI_DRIVER
+ VERSION_STRING = 1.0
+ ENTRY_POINT = EntryPoint
+ UNLOAD_IMAGE = Unload
+
+[Sources]
+ SampleDriver.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+
+[Protocols]
+ gEfiPciIoProtocolGuid
+
+[LibraryClasses]
+ UefiDriverEntryPoint
+ UefiBootServicesTableLib
+ UefiLib
+ DebugLib
+ MemoryAllocationLib
diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index 25aea3e..7e78335 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -205,6 +205,7 @@
[Components]
MdeModulePkg/Application/HelloWorld/HelloWorld.inf
MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf
+ MdeModulePkg/Application/SampleDriver/SampleDriver.inf
MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.inf
MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
|
