On 27 October 2017 at 06:33, Daniil Egranov <[email protected]> wrote:
> Corrected memory allocation during startup.
> Added driver stop procedure and exit boot event handler.
> Added driver memory and protocols cleanup procedures.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Daniil Egranov <[email protected]>
Can you split up this patch please?
> ---
> EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c | 281
> ++++++++++++++++++-----
> EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.h | 17 ++
> 2 files changed, 236 insertions(+), 62 deletions(-)
>
> diff --git a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c
> b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c
> index 50253b9160..063086c956 100644
> --- a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c
> +++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c
> @@ -89,20 +89,26 @@ SataSiI3132Constructor (
> {
> SATA_SI3132_INSTANCE *Instance;
> EFI_ATA_PASS_THRU_MODE *AtaPassThruMode;
> + EFI_STATUS Status;
>
> if (!SataSiI3132Instance) {
> return EFI_INVALID_PARAMETER;
> }
>
> - Instance = (SATA_SI3132_INSTANCE*)AllocateZeroPool (sizeof
> (SATA_SI3132_INSTANCE));
> - if (Instance == NULL) {
> - return EFI_OUT_OF_RESOURCES;
> + Status = gBS->AllocatePool (EfiBootServicesData, sizeof
> (SATA_SI3132_INSTANCE), (VOID **)&Instance);
> + if (EFI_ERROR (Status)) {
> + return Status;
> }
> + gBS->SetMem (Instance, sizeof (SATA_SI3132_INSTANCE), 0);
>
> Instance->Signature = SATA_SII3132_SIGNATURE;
> Instance->PciIo = PciIo;
>
> - AtaPassThruMode = (EFI_ATA_PASS_THRU_MODE*)AllocatePool (sizeof
> (EFI_ATA_PASS_THRU_MODE));
> + Status = gBS->AllocatePool (EfiBootServicesData, sizeof
> (EFI_ATA_PASS_THRU_MODE), (VOID **)&AtaPassThruMode);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> AtaPassThruMode->Attributes = EFI_ATA_PASS_THRU_ATTRIBUTES_PHYSICAL |
> EFI_ATA_PASS_THRU_ATTRIBUTES_LOGICAL;
> AtaPassThruMode->IoAlign = 0x1000;
>
> @@ -200,7 +206,10 @@ SataSiI3132PortInitialization (
> }
>
> // Create Device
> - Device = (SATA_SI3132_DEVICE*)AllocatePool (sizeof
> (SATA_SI3132_DEVICE));
> + Status = gBS->AllocatePool (EfiBootServicesData, sizeof
> (SATA_SI3132_DEVICE), (VOID **)&Device);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> Device->Index = Port->Index; //TODO: Could need to be fixed when
> SATA Port Multiplier support
> Device->Port = Port;
> Device->BlockSize = 0;
> @@ -310,6 +319,118 @@ ON_EXIT:
> }
>
> /**
> + Free memory allocated by the driver.
> +
> + @param SataSiI3132Instance pointer to the driver's data structure.
> +
> +**/
> +STATIC
> +VOID
> +SataSiI3132DriverFreeMemory (
> + IN SATA_SI3132_INSTANCE* SataSiI3132Instance
> + )
> +{
> + UINTN PortIndex;
> + SATA_SI3132_PORT *Port;
> + SATA_SI3132_DEVICE *Device;
> + LIST_ENTRY *Node;
> + EFI_STATUS Status;
> + UINTN NumberOfBytes;
> +
> + if (SataSiI3132Instance == NULL) {
> + return;
> + }
> +
> + for (PortIndex = 0; PortIndex < SATA_SII3132_MAXPORT; PortIndex++) {
> + Port = &(SataSiI3132Instance->Ports[PortIndex]);
> + if (Port != NULL) {
> +
> + //Free Device memory on each port
> + Node = Port->Devices.ForwardLink;
> + while (Node != &Port->Devices) {
> + Device = (SATA_SI3132_DEVICE*)Node;
> + Node = Node->ForwardLink;
> + RemoveEntryList (&Device->Link);
> + gBS->FreePool (Device);
> + }
> +
> + //Unmap and deallocate PCI IO memory for each port
> + if (Port->PciAllocMappingPRB != NULL) {
> + Status = SataSiI3132Instance->PciIo->Unmap
> (SataSiI3132Instance->PciIo,
> +
> Port->PciAllocMappingPRB);
> + if (EFI_ERROR (Status)) {
> + SATA_TRACE ("SataSiI3132DriverFreeMemory: unable to unmap memory");
> + }
> + }
> +
> + if (Port->HostPRB != 0) {
> + NumberOfBytes = sizeof (SATA_SI3132_PRB);
> + Status = SataSiI3132Instance->PciIo->FreeBuffer
> (SataSiI3132Instance->PciIo,
> + EFI_SIZE_TO_PAGES
> (NumberOfBytes),
> + Port->HostPRB);
> + if (EFI_ERROR (Status)) {
> + SATA_TRACE ("SataSiI3132DriverFreeMemory: unable to free memory");
> + }
> + }
> + }
> + }
> +
> + if (SataSiI3132Instance->AtaPassThruProtocol.Mode != NULL) {
> + gBS->FreePool (SataSiI3132Instance->AtaPassThruProtocol.Mode);
> + }
> +}
> +
> +/**
> + Uninstall and close protocols used by the driver.
> +
> + @param SataSiI3132Instance pointer to the driver's data structure.
> +
> +**/
> +STATIC
> +VOID
> +SataSiI3132DriverCloseProtocols (
> + IN SATA_SI3132_INSTANCE* SataSiI3132Instance
> + )
> +{
> + EFI_STATUS Status;
> +
> + if (SataSiI3132Instance == NULL) {
> + return;
> + }
> +
> + // Uninstall ATA Pass-Through Protocol
> + Status = gBS->UninstallProtocolInterface (SataSiI3132Instance->Controller,
> + &gEfiAtaPassThruProtocolGuid,
> +
> &SataSiI3132Instance->AtaPassThruProtocol);
> + if (EFI_ERROR (Status)) {
> + SATA_TRACE ("SataSiI3132DriverFreeMemory: unable to uninstall
> AtaPassThruProtocol");
> + }
> +
> + if (SataSiI3132Instance->PciIo != NULL) {
> + if (SataSiI3132Instance->OriginalPciAttributesValid) {
> + // Restore original PCI attributes
> + Status = SataSiI3132Instance->PciIo->Attributes
> (SataSiI3132Instance->PciIo,
> + EfiPciIoAttributeOperationSet,
> +
> SataSiI3132Instance->OriginalPciAttributes,
> + NULL);
> + if (EFI_ERROR (Status)) {
> + SATA_TRACE ("SataSiI3132DriverFreeMemory: unable to restore PCI
> attributes");
> + }
> + }
> +
> + // Close PCI IO Protocol
> + Status = gBS->CloseProtocol (SataSiI3132Instance->Controller,
> + &gEfiPciIoProtocolGuid,
> + SataSiI3132Instance->SataDriver->DriverBindingHandle,
> + SataSiI3132Instance->Controller);
> + if (EFI_ERROR (Status)) {
> + SATA_TRACE ("SataSiI3132DriverFreeMemory: unable to close PCI IO
> protocol");
> + }
> + }
> +
> +}
> +
> +/**
> Starting the Pci SATA Driver.
>
> @param This Protocol instance pointer.
> @@ -333,8 +454,6 @@ SataSiI3132DriverBindingStart (
> EFI_STATUS Status;
> EFI_PCI_IO_PROTOCOL *PciIo;
> UINT64 Supports;
> - UINT64 OriginalPciAttributes;
> - BOOLEAN PciAttributesSaved;
> UINT32 PciID;
> SATA_SI3132_INSTANCE *SataSiI3132Instance = NULL;
>
> @@ -369,7 +488,22 @@ SataSiI3132DriverBindingStart (
> return Status;
> }
>
> - PciAttributesSaved = FALSE;
> + // Create SiI3132 Sata Instance
> + Status = SataSiI3132Constructor (PciIo, &SataSiI3132Instance);
> + if (EFI_ERROR (Status)) {
> + SATA_TRACE ("SataSiI3132DriverBindingStart: failed to allocate driver
> structure");
> + gBS->CloseProtocol (
> + Controller,
> + &gEfiPciIoProtocolGuid,
> + This->DriverBindingHandle,
> + Controller
> + );
> + return Status;
> + }
> +
> + SataSiI3132Instance->Controller = Controller;
> + SataSiI3132Instance->SataDriver = This;
> +
> //
> // Save original PCI attributes
> //
> @@ -377,12 +511,13 @@ SataSiI3132DriverBindingStart (
> PciIo,
> EfiPciIoAttributeOperationGet,
> 0,
> - &OriginalPciAttributes
> + &SataSiI3132Instance->OriginalPciAttributes
> );
> if (EFI_ERROR (Status)) {
> - goto CLOSE_PCIIO;
> + goto QUIT_ERROR;
> }
> - PciAttributesSaved = TRUE;
> +
> + SataSiI3132Instance->OriginalPciAttributesValid = TRUE;
>
> Status = PciIo->Attributes (
> PciIo,
> @@ -401,7 +536,7 @@ SataSiI3132DriverBindingStart (
> }
> if (EFI_ERROR (Status)) {
> DEBUG ((EFI_D_ERROR, "SataSiI3132DriverBindingStart: failed to enable
> controller\n"));
> - goto CLOSE_PCIIO;
> + goto QUIT_ERROR;
> }
>
> //
> @@ -416,7 +551,7 @@ SataSiI3132DriverBindingStart (
> );
> if (EFI_ERROR (Status)) {
> Status = EFI_UNSUPPORTED;
> - goto CLOSE_PCIIO;
> + goto QUIT_ERROR;
> }
>
> //
> @@ -424,21 +559,13 @@ SataSiI3132DriverBindingStart (
> //
> if (PciID != ((SATA_SII3132_DEVICE_ID << 16) | SATA_SII3132_VENDOR_ID)) {
> Status = EFI_UNSUPPORTED;
> - goto CLOSE_PCIIO;
> + goto QUIT_ERROR;
> }
>
> - // Create SiI3132 Sata Instance
> - Status = SataSiI3132Constructor (PciIo, &SataSiI3132Instance);
> - if (EFI_ERROR (Status)) {
> - return Status;
> - }
> -
> - SataSiI3132Instance->OriginalPciAttributes = OriginalPciAttributes;
> -
> // Initialize SiI3132 Sata Controller
> Status = SataSiI3132Initialization (SataSiI3132Instance);
> if (EFI_ERROR (Status)) {
> - return Status;
> + goto QUIT_ERROR;
> }
>
> // Install Ata Pass Thru Protocol
> @@ -449,49 +576,21 @@ SataSiI3132DriverBindingStart (
> &(SataSiI3132Instance->AtaPassThruProtocol)
> );
> if (EFI_ERROR (Status)) {
> - goto FREE_POOL;
> + goto QUIT_ERROR;
> }
>
> -/* //
> - // Create event to stop the HC when exit boot service.
> - //
> - Status = gBS->CreateEventEx (
> - EVT_NOTIFY_SIGNAL,
> - TPL_NOTIFY,
> - EhcExitBootService,
> - Ehc,
> - &gEfiEventExitBootServicesGuid,
> - &Ehc->ExitBootServiceEvent
> - );
> - if (EFI_ERROR (Status)) {
> - goto UNINSTALL_USBHC;
> - }*/
> + Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK,
> + &SataSiI3132DriverExitBoot, SataSiI3132Instance,
> &SataSiI3132Instance->ExitBootEvent);
>
Please check your coding style and line length.
> - SATA_TRACE ("SataSiI3132DriverBindingStart() Success!");
> - return EFI_SUCCESS;
> -
> -FREE_POOL:
> - //TODO: Free SATA Instance
> -
> -CLOSE_PCIIO:
> - if (PciAttributesSaved) {
> - //
> - // Restore original PCI attributes
> - //
> - PciIo->Attributes (
> - PciIo,
> - EfiPciIoAttributeOperationSet,
> - OriginalPciAttributes,
> - NULL
> - );
> + if (!EFI_ERROR (Status)) {
> + SATA_TRACE ("SataSiI3132DriverBindingStart() Success!");
> + return EFI_SUCCESS;
> }
>
> - gBS->CloseProtocol (
> - Controller,
> - &gEfiPciIoProtocolGuid,
> - This->DriverBindingHandle,
> - Controller
> - );
> +QUIT_ERROR:
> + SataSiI3132DriverCloseProtocols (SataSiI3132Instance);
> + SataSiI3132DriverFreeMemory (SataSiI3132Instance);
> + gBS->FreePool (SataSiI3132Instance);
>
> return Status;
> }
> @@ -518,8 +617,66 @@ SataSiI3132DriverBindingStop (
> IN EFI_HANDLE *ChildHandleBuffer
> )
> {
> + SATA_SI3132_INSTANCE *SataSiI3132Instance;
> + EFI_ATA_PASS_THRU_PROTOCOL *AtaPassThruProtocol;
> + EFI_STATUS Status;
> +
> SATA_TRACE ("SataSiI3132DriverBindingStop()");
> - return EFI_UNSUPPORTED;
> +
> + Status = gBS->OpenProtocol (
> + Controller,
> + &gEfiAtaPassThruProtocolGuid,
> + (VOID **)&AtaPassThruProtocol,
> + This->DriverBindingHandle,
> + Controller,
> + EFI_OPEN_PROTOCOL_GET_PROTOCOL
> + );
> +
> + if (EFI_ERROR (Status)) {
> + SATA_TRACE ("SataSiI3132DriverBindingStop: driver is not started");
> + return Status;
> + }
> +
> + SataSiI3132Instance = INSTANCE_FROM_ATAPASSTHRU_THIS (AtaPassThruProtocol);
> +
> + gBS->CloseEvent (SataSiI3132Instance->ExitBootEvent);
> +
> + SataSiI3132DriverCloseProtocols (SataSiI3132Instance);
> + SataSiI3132DriverFreeMemory (SataSiI3132Instance);
> + gBS->FreePool (SataSiI3132Instance);
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + Process exit boot event.
> +
> + @param [in] Event Event id.
> + @param [in] Context Driver context.
> +
> +**/
> +VOID
> +EFIAPI
> +SataSiI3132DriverExitBoot (
> + IN EFI_EVENT Event,
> + IN VOID *Context
> + )
> +{
> + SATA_SI3132_INSTANCE *SataSiI3132Instance;
> + EFI_STATUS Status;
> +
> + if (Context == NULL) {
> + SATA_TRACE ("SataSiI3132DriverExitBoot: invalid Context parameter");
> + } else {
> + SataSiI3132Instance = (SATA_SI3132_INSTANCE*)Context;
> + Status = SataSiI3132Instance->SataDriver->Stop
> (SataSiI3132Instance->SataDriver,
> +
> SataSiI3132Instance->Controller,
> + 0,
> + NULL);
You cannot stop the driver at ExitBootServices, since you are not
allowed to do anything that affects the memory map. All you should do
here is clear the EFI_PCI_COMMAND_BUS_MASTER PciIo attribute, so the
device can no longer access memory.
> + if (EFI_ERROR (Status)) {
> + SATA_TRACE ("SataSiI3132DriverExitBoot: driver stop failed");
> + }
> + }
> }
>
> /**
> diff --git a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.h
> b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.h
> index a7bc956b19..ab4510b97b 100644
> --- a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.h
> +++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.h
> @@ -144,6 +144,16 @@ typedef struct _SATA_SI3132_INSTANCE {
> EFI_ATA_PASS_THRU_PROTOCOL AtaPassThruProtocol;
>
> EFI_PCI_IO_PROTOCOL *PciIo;
> +
> + EFI_DRIVER_BINDING_PROTOCOL *SataDriver;
> +
> + EFI_HANDLE Controller;
> +
> + UINT64 OriginalPciAttributes;
> +
> + BOOLEAN OriginalPciAttributesValid;
> +
> + EFI_EVENT ExitBootEvent;
> } SATA_SI3132_INSTANCE;
>
> #define SATA_SII3132_SIGNATURE SIGNATURE_32('s', 'i', '3', '2')
> @@ -211,6 +221,13 @@ SataSiI3132DriverBindingStop (
> IN EFI_HANDLE *ChildHandleBuffer
> );
>
> +VOID
> +EFIAPI
> +SataSiI3132DriverExitBoot (
> + IN EFI_EVENT Event,
> + IN VOID *Context
> + );
> +
> EFI_STATUS SiI3132AtaPassThruCommand (
> IN SATA_SI3132_INSTANCE *pSataSiI3132Instance,
> IN SATA_SI3132_PORT *pSataPort,
> --
> 2.11.0
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
