* [PATCH 0/4] SataSiI3132Dxe fixes
@ 2017-10-27 5:33 Daniil Egranov
2017-10-27 5:33 ` [PATCH 1/4] Drivers/SataSiI3132Dxe: Fixed PCI IO read and write operations Daniil Egranov
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: Daniil Egranov @ 2017-10-27 5:33 UTC (permalink / raw)
To: edk2-devel; +Cc: leif.lindholm, ard.biesheuvel, Daniil Egranov
This set of patches fixes an issue with 64-bit DMA and implements
the missing exit boot event and driver stop functionality including
memory/protocols cleanup procedure.
Daniil Egranov (4):
Drivers/SataSiI3132Dxe: Fixed PCI IO read and write operations
Drivers/SataSiI3132Dxe: Allow 64-bit DMA transfer
Drivers/SataSiI3132Dxe: Enable multi-controller support
Drivers/SataSiI3132Dxe: Fixed startup and shutdown procedures
EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c | 301 ++++++++++++++++-----
EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.h | 17 ++
.../Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c | 4 +-
3 files changed, 252 insertions(+), 70 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/4] Drivers/SataSiI3132Dxe: Fixed PCI IO read and write operations 2017-10-27 5:33 [PATCH 0/4] SataSiI3132Dxe fixes Daniil Egranov @ 2017-10-27 5:33 ` Daniil Egranov 2017-10-27 9:22 ` Ard Biesheuvel 2017-10-27 5:33 ` [PATCH 2/4] Drivers/SataSiI3132Dxe: Allow 64-bit DMA transfer Daniil Egranov ` (4 subsequent siblings) 5 siblings, 1 reply; 12+ messages in thread From: Daniil Egranov @ 2017-10-27 5:33 UTC (permalink / raw) To: edk2-devel; +Cc: leif.lindholm, ard.biesheuvel, Daniil Egranov The ATA pass through read should use PCI IO bus master write operation and ATA pass through write should use PCI IO bus master read operation as the read and write operations executed from the bus master's point of view. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Daniil Egranov <daniil.egranov@arm.com> --- EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c index 2fb5fd68db..a938563ebd 100644 --- a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c +++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c @@ -104,7 +104,7 @@ SiI3132AtaPassThruCommand ( } Status = PciIo->Map ( - PciIo, EfiPciIoOperationBusMasterRead, + PciIo, EfiPciIoOperationBusMasterWrite, Packet->InDataBuffer, &InDataBufferLength, &PhysInDataBuffer, &PciAllocMapping ); if (EFI_ERROR (Status)) { @@ -139,7 +139,7 @@ SiI3132AtaPassThruCommand ( OutDataBufferLength = Packet->OutTransferLength * SataDevice->BlockSize; Status = PciIo->Map ( - PciIo, EfiPciIoOperationBusMasterWrite, + PciIo, EfiPciIoOperationBusMasterRead, Packet->OutDataBuffer, &OutDataBufferLength, &PhysOutDataBuffer, &PciAllocMapping ); if (EFI_ERROR (Status)) { -- 2.11.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] Drivers/SataSiI3132Dxe: Fixed PCI IO read and write operations 2017-10-27 5:33 ` [PATCH 1/4] Drivers/SataSiI3132Dxe: Fixed PCI IO read and write operations Daniil Egranov @ 2017-10-27 9:22 ` Ard Biesheuvel 0 siblings, 0 replies; 12+ messages in thread From: Ard Biesheuvel @ 2017-10-27 9:22 UTC (permalink / raw) To: Daniil Egranov; +Cc: edk2-devel@lists.01.org, Leif Lindholm On 27 October 2017 at 06:33, Daniil Egranov <daniil.egranov@arm.com> wrote: > The ATA pass through read should use PCI IO bus master write operation > and ATA pass through write should use PCI IO bus master read operation > as the read and write operations executed from the bus master's point > of view. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Daniil Egranov <daniil.egranov@arm.com> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c > index 2fb5fd68db..a938563ebd 100644 > --- a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c > +++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c > @@ -104,7 +104,7 @@ SiI3132AtaPassThruCommand ( > } > > Status = PciIo->Map ( > - PciIo, EfiPciIoOperationBusMasterRead, > + PciIo, EfiPciIoOperationBusMasterWrite, > Packet->InDataBuffer, &InDataBufferLength, &PhysInDataBuffer, &PciAllocMapping > ); > if (EFI_ERROR (Status)) { > @@ -139,7 +139,7 @@ SiI3132AtaPassThruCommand ( > OutDataBufferLength = Packet->OutTransferLength * SataDevice->BlockSize; > > Status = PciIo->Map ( > - PciIo, EfiPciIoOperationBusMasterWrite, > + PciIo, EfiPciIoOperationBusMasterRead, > Packet->OutDataBuffer, &OutDataBufferLength, &PhysOutDataBuffer, &PciAllocMapping > ); > if (EFI_ERROR (Status)) { > -- > 2.11.0 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/4] Drivers/SataSiI3132Dxe: Allow 64-bit DMA transfer 2017-10-27 5:33 [PATCH 0/4] SataSiI3132Dxe fixes Daniil Egranov 2017-10-27 5:33 ` [PATCH 1/4] Drivers/SataSiI3132Dxe: Fixed PCI IO read and write operations Daniil Egranov @ 2017-10-27 5:33 ` Daniil Egranov 2017-10-27 9:23 ` Ard Biesheuvel 2017-10-27 5:33 ` [PATCH 3/4] Drivers/SataSiI3132Dxe: Enable multi-controller support Daniil Egranov ` (3 subsequent siblings) 5 siblings, 1 reply; 12+ messages in thread From: Daniil Egranov @ 2017-10-27 5:33 UTC (permalink / raw) To: edk2-devel; +Cc: leif.lindholm, ard.biesheuvel, Daniil Egranov Set a PCI IO attribute allowing 64-bit DMA transfer. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Daniil Egranov <daniil.egranov@arm.com> --- EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c index f4946552a0..c1760fdc1b 100644 --- a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c +++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c @@ -384,7 +384,7 @@ SataSiI3132DriverBindingStart ( &Supports ); if (!EFI_ERROR (Status)) { - Supports &= EFI_PCI_DEVICE_ENABLE; + Supports &= EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE; Status = PciIo->Attributes ( PciIo, EfiPciIoAttributeOperationEnable, -- 2.11.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] Drivers/SataSiI3132Dxe: Allow 64-bit DMA transfer 2017-10-27 5:33 ` [PATCH 2/4] Drivers/SataSiI3132Dxe: Allow 64-bit DMA transfer Daniil Egranov @ 2017-10-27 9:23 ` Ard Biesheuvel 0 siblings, 0 replies; 12+ messages in thread From: Ard Biesheuvel @ 2017-10-27 9:23 UTC (permalink / raw) To: Daniil Egranov; +Cc: edk2-devel@lists.01.org, Leif Lindholm On 27 October 2017 at 06:33, Daniil Egranov <daniil.egranov@arm.com> wrote: > Set a PCI IO attribute allowing 64-bit DMA transfer. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Daniil Egranov <daniil.egranov@arm.com> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c > index f4946552a0..c1760fdc1b 100644 > --- a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c > +++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c > @@ -384,7 +384,7 @@ SataSiI3132DriverBindingStart ( > &Supports > ); > if (!EFI_ERROR (Status)) { > - Supports &= EFI_PCI_DEVICE_ENABLE; > + Supports &= EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE; > Status = PciIo->Attributes ( > PciIo, > EfiPciIoAttributeOperationEnable, > -- > 2.11.0 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/4] Drivers/SataSiI3132Dxe: Enable multi-controller support 2017-10-27 5:33 [PATCH 0/4] SataSiI3132Dxe fixes Daniil Egranov 2017-10-27 5:33 ` [PATCH 1/4] Drivers/SataSiI3132Dxe: Fixed PCI IO read and write operations Daniil Egranov 2017-10-27 5:33 ` [PATCH 2/4] Drivers/SataSiI3132Dxe: Allow 64-bit DMA transfer Daniil Egranov @ 2017-10-27 5:33 ` Daniil Egranov 2017-10-27 12:42 ` Ard Biesheuvel 2017-10-27 5:33 ` [PATCH 4/4] Drivers/SataSiI3132Dxe: Fixed startup and shutdown procedures Daniil Egranov ` (2 subsequent siblings) 5 siblings, 1 reply; 12+ messages in thread From: Daniil Egranov @ 2017-10-27 5:33 UTC (permalink / raw) To: edk2-devel; +Cc: leif.lindholm, ard.biesheuvel, Daniil Egranov Saved controller specific data into the driver's information structure. Removed global variable indicating the driver's status and added check for the driver's status based on the available protocol. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Daniil Egranov <daniil.egranov@arm.com> --- EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c index c1760fdc1b..50253b9160 100644 --- a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c +++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c @@ -309,8 +309,6 @@ ON_EXIT: return Status; } -BOOLEAN mbStarted = FALSE; - /** Starting the Pci SATA Driver. @@ -342,9 +340,18 @@ SataSiI3132DriverBindingStart ( SATA_TRACE ("SataSiI3132DriverBindingStart()"); - //TODO: Find a nicer way to do it ! - if (mbStarted) { - return EFI_SUCCESS; // Don't restart me ! + Status = gBS->OpenProtocol ( + Controller, + &gEfiAtaPassThruProtocolGuid, + NULL, + This->DriverBindingHandle, + Controller, + EFI_OPEN_PROTOCOL_TEST_PROTOCOL + ); + + if (!EFI_ERROR (Status)) { + SATA_TRACE ("SataSiI3132DriverBindingStart: driver already started"); + return Status; } // @@ -426,6 +433,8 @@ SataSiI3132DriverBindingStart ( return Status; } + SataSiI3132Instance->OriginalPciAttributes = OriginalPciAttributes; + // Initialize SiI3132 Sata Controller Status = SataSiI3132Initialization (SataSiI3132Instance); if (EFI_ERROR (Status)) { @@ -458,8 +467,6 @@ SataSiI3132DriverBindingStart ( goto UNINSTALL_USBHC; }*/ - mbStarted = TRUE; - SATA_TRACE ("SataSiI3132DriverBindingStart() Success!"); return EFI_SUCCESS; -- 2.11.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] Drivers/SataSiI3132Dxe: Enable multi-controller support 2017-10-27 5:33 ` [PATCH 3/4] Drivers/SataSiI3132Dxe: Enable multi-controller support Daniil Egranov @ 2017-10-27 12:42 ` Ard Biesheuvel 0 siblings, 0 replies; 12+ messages in thread From: Ard Biesheuvel @ 2017-10-27 12:42 UTC (permalink / raw) To: Daniil Egranov; +Cc: edk2-devel@lists.01.org, Leif Lindholm On 27 October 2017 at 06:33, Daniil Egranov <daniil.egranov@arm.com> wrote: > Saved controller specific data into the driver's information structure. > Removed global variable indicating the driver's status and added > check for the driver's status based on the available protocol. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Daniil Egranov <daniil.egranov@arm.com> > --- > EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c | 21 ++++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c > index c1760fdc1b..50253b9160 100644 > --- a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c > +++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c > @@ -309,8 +309,6 @@ ON_EXIT: > return Status; > } > > -BOOLEAN mbStarted = FALSE; > - > /** > Starting the Pci SATA Driver. > > @@ -342,9 +340,18 @@ SataSiI3132DriverBindingStart ( > > SATA_TRACE ("SataSiI3132DriverBindingStart()"); > > - //TODO: Find a nicer way to do it ! > - if (mbStarted) { > - return EFI_SUCCESS; // Don't restart me ! > + Status = gBS->OpenProtocol ( > + Controller, > + &gEfiAtaPassThruProtocolGuid, > + NULL, > + This->DriverBindingHandle, > + Controller, > + EFI_OPEN_PROTOCOL_TEST_PROTOCOL > + ); > + > + if (!EFI_ERROR (Status)) { > + SATA_TRACE ("SataSiI3132DriverBindingStart: driver already started"); > + return Status; Can you explain what this does? Does it check whether the protocol we will install is already installed? Is this how other PCI drivers deal with this? > } > > // > @@ -426,6 +433,8 @@ SataSiI3132DriverBindingStart ( > return Status; > } > > + SataSiI3132Instance->OriginalPciAttributes = OriginalPciAttributes; > + Does this belong in the same patch? > // Initialize SiI3132 Sata Controller > Status = SataSiI3132Initialization (SataSiI3132Instance); > if (EFI_ERROR (Status)) { > @@ -458,8 +467,6 @@ SataSiI3132DriverBindingStart ( > goto UNINSTALL_USBHC; > }*/ > > - mbStarted = TRUE; > - > SATA_TRACE ("SataSiI3132DriverBindingStart() Success!"); > return EFI_SUCCESS; > > -- > 2.11.0 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/4] Drivers/SataSiI3132Dxe: Fixed startup and shutdown procedures 2017-10-27 5:33 [PATCH 0/4] SataSiI3132Dxe fixes Daniil Egranov ` (2 preceding siblings ...) 2017-10-27 5:33 ` [PATCH 3/4] Drivers/SataSiI3132Dxe: Enable multi-controller support Daniil Egranov @ 2017-10-27 5:33 ` Daniil Egranov 2017-10-27 12:47 ` Ard Biesheuvel 2017-10-27 12:48 ` [PATCH 0/4] SataSiI3132Dxe fixes Ard Biesheuvel 2017-10-27 16:57 ` Jeremy Linton 5 siblings, 1 reply; 12+ messages in thread From: Daniil Egranov @ 2017-10-27 5:33 UTC (permalink / raw) To: edk2-devel; +Cc: leif.lindholm, ard.biesheuvel, Daniil Egranov 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 <daniil.egranov@arm.com> --- 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); - 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); + 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] Drivers/SataSiI3132Dxe: Fixed startup and shutdown procedures 2017-10-27 5:33 ` [PATCH 4/4] Drivers/SataSiI3132Dxe: Fixed startup and shutdown procedures Daniil Egranov @ 2017-10-27 12:47 ` Ard Biesheuvel 0 siblings, 0 replies; 12+ messages in thread From: Ard Biesheuvel @ 2017-10-27 12:47 UTC (permalink / raw) To: Daniil Egranov; +Cc: edk2-devel@lists.01.org, Leif Lindholm On 27 October 2017 at 06:33, Daniil Egranov <daniil.egranov@arm.com> 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 <daniil.egranov@arm.com> 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 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] SataSiI3132Dxe fixes 2017-10-27 5:33 [PATCH 0/4] SataSiI3132Dxe fixes Daniil Egranov ` (3 preceding siblings ...) 2017-10-27 5:33 ` [PATCH 4/4] Drivers/SataSiI3132Dxe: Fixed startup and shutdown procedures Daniil Egranov @ 2017-10-27 12:48 ` Ard Biesheuvel 2017-10-27 16:57 ` Jeremy Linton 5 siblings, 0 replies; 12+ messages in thread From: Ard Biesheuvel @ 2017-10-27 12:48 UTC (permalink / raw) To: Daniil Egranov; +Cc: edk2-devel@lists.01.org, Leif Lindholm On 27 October 2017 at 06:33, Daniil Egranov <daniil.egranov@arm.com> wrote: > This set of patches fixes an issue with 64-bit DMA and implements > the missing exit boot event and driver stop functionality including > memory/protocols cleanup procedure. > > Daniil Egranov (4): > Drivers/SataSiI3132Dxe: Fixed PCI IO read and write operations > Drivers/SataSiI3132Dxe: Allow 64-bit DMA transfer > Drivers/SataSiI3132Dxe: Enable multi-controller support > Drivers/SataSiI3132Dxe: Fixed startup and shutdown procedures > > EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c | 301 ++++++++++++++++----- > EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.h | 17 ++ > .../Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c | 4 +- > 3 files changed, 252 insertions(+), 70 deletions(-) > Hi Daniil, Thanks for taking the time to fix this driver. I will go ahead and push the first two patches, given that they are self-contained and obvious bug fixes. The remaining patches, please split them up, and please align more closely with what other upstream PCI drivers do. Regards, Ard. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] SataSiI3132Dxe fixes 2017-10-27 5:33 [PATCH 0/4] SataSiI3132Dxe fixes Daniil Egranov ` (4 preceding siblings ...) 2017-10-27 12:48 ` [PATCH 0/4] SataSiI3132Dxe fixes Ard Biesheuvel @ 2017-10-27 16:57 ` Jeremy Linton 2017-10-27 17:00 ` Ard Biesheuvel 5 siblings, 1 reply; 12+ messages in thread From: Jeremy Linton @ 2017-10-27 16:57 UTC (permalink / raw) To: Daniil Egranov, edk2-devel; +Cc: leif.lindholm, ard.biesheuvel Hi, On 10/27/2017 12:33 AM, Daniil Egranov wrote: > This set of patches fixes an issue with 64-bit DMA and implements > the missing exit boot event and driver stop functionality including > memory/protocols cleanup procedure. > > Daniil Egranov (4): > Drivers/SataSiI3132Dxe: Fixed PCI IO read and write operations > Drivers/SataSiI3132Dxe: Allow 64-bit DMA transfer > Drivers/SataSiI3132Dxe: Enable multi-controller support > Drivers/SataSiI3132Dxe: Fixed startup and shutdown procedures > > EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c | 301 ++++++++++++++++----- > EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.h | 17 ++ > .../Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c | 4 +- > 3 files changed, 252 insertions(+), 70 deletions(-) This is generally good, but there remain quite a number of "errors" in the command submission path as well as the completely unnecessary 4k IO alignment requirement which has been known to break older grubs/etc. A few of those "errors" were fixed in this patch set (1) as well, so might be worthwhile if you are looking at this driver to integrate those fixes as well. (1) https://lists.01.org/pipermail/edk2-devel/2017-March/008277.html Thanks, ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] SataSiI3132Dxe fixes 2017-10-27 16:57 ` Jeremy Linton @ 2017-10-27 17:00 ` Ard Biesheuvel 0 siblings, 0 replies; 12+ messages in thread From: Ard Biesheuvel @ 2017-10-27 17:00 UTC (permalink / raw) To: Jeremy Linton; +Cc: Daniil Egranov, edk2-devel@lists.01.org, Leif Lindholm On 27 October 2017 at 17:57, Jeremy Linton <jeremy.linton@arm.com> wrote: > Hi, > > On 10/27/2017 12:33 AM, Daniil Egranov wrote: >> >> This set of patches fixes an issue with 64-bit DMA and implements >> the missing exit boot event and driver stop functionality including >> memory/protocols cleanup procedure. >> >> Daniil Egranov (4): >> Drivers/SataSiI3132Dxe: Fixed PCI IO read and write operations >> Drivers/SataSiI3132Dxe: Allow 64-bit DMA transfer >> Drivers/SataSiI3132Dxe: Enable multi-controller support >> Drivers/SataSiI3132Dxe: Fixed startup and shutdown procedures >> >> EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c | 301 >> ++++++++++++++++----- >> EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.h | 17 ++ >> .../Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c | 4 +- >> 3 files changed, 252 insertions(+), 70 deletions(-) > > > This is generally good, but there remain quite a number of "errors" in the > command submission path as well as the completely unnecessary 4k IO > alignment requirement which has been known to break older grubs/etc. A few > of those "errors" were fixed in this patch set (1) as well, so might be > worthwhile if you are looking at this driver to integrate those fixes as > well. > Yes, please. And apologies for forgetting about thise patches. > (1) https://lists.01.org/pipermail/edk2-devel/2017-March/008277.html > > Thanks, ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-10-27 16:56 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-27 5:33 [PATCH 0/4] SataSiI3132Dxe fixes Daniil Egranov 2017-10-27 5:33 ` [PATCH 1/4] Drivers/SataSiI3132Dxe: Fixed PCI IO read and write operations Daniil Egranov 2017-10-27 9:22 ` Ard Biesheuvel 2017-10-27 5:33 ` [PATCH 2/4] Drivers/SataSiI3132Dxe: Allow 64-bit DMA transfer Daniil Egranov 2017-10-27 9:23 ` Ard Biesheuvel 2017-10-27 5:33 ` [PATCH 3/4] Drivers/SataSiI3132Dxe: Enable multi-controller support Daniil Egranov 2017-10-27 12:42 ` Ard Biesheuvel 2017-10-27 5:33 ` [PATCH 4/4] Drivers/SataSiI3132Dxe: Fixed startup and shutdown procedures Daniil Egranov 2017-10-27 12:47 ` Ard Biesheuvel 2017-10-27 12:48 ` [PATCH 0/4] SataSiI3132Dxe fixes Ard Biesheuvel 2017-10-27 16:57 ` Jeremy Linton 2017-10-27 17:00 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox