Hi, Laszlo There is a macro EFI_PCI_DEVICE_ENABLE, which is (EFI_PCI_IO_ATTRIBUTE_IO | EFI_PCI_IO_ATTRIBUTE_MEMORY | EFI_PCI_IO_ATTRIBUTE_BUS_MASTER)
Suggest to use this macro. Others look good to me. Reviewed-by: Feng Tian <[email protected]> Thanks Feng -----Original Message----- From: edk2-devel [mailto:[email protected]] On Behalf Of Laszlo Ersek Sent: Wednesday, September 16, 2015 10:57 To: [email protected] Cc: Tian, Feng; Justen, Jordan L; Gabriel L. Somlo; Alexander Graf; Hannes Reinecke; Reza Jelveh Subject: [edk2] [PATCH v4 05/10] OvmfPkg: SataControllerDxe: enable IO / mem access and DMA when binding When we bind the SATA controller in SataControllerStart(), we read the NP ("Number of Ports") bitfield from the CAP ("HBA Capabilities") register of the controller. (See the AHCI 1.3.1 spec.) This register is memory mapped. If we'd like to access it, we must at least enable memory space access for the device. In addition, Feng Tian recommended enabling Bus Master DMA in <http://thread.gmane.org/gmane.comp.bios.tianocore.devel/10545/focus=10659>. We also enable IO space access for completeness. Further, because we change the PCI attributes of the device with the above when binding it, we must also restore its original PCI attributes when unbinding it. See the Driver Writer's Guide for UEFI 2.3.1 v1.01, section 18.3 "PCI drivers" | 18.3.2 "Start() and Stop()". (OvmfPkg's copy of SataControllerDxe must differ from the same in DuetPkg because Duet inherits a pre-configured SATA controller from the BIOS, as explained by Feng.) Cc: Alexander Graf <[email protected]> Cc: Reza Jelveh <[email protected]> Cc: Jordan Justen <[email protected]> Cc: Hannes Reinecke <[email protected]> Cc: Gabriel L. Somlo <[email protected]> Cc: Feng Tian <[email protected]> Suggested-by: Feng Tian <[email protected]> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <[email protected]> --- OvmfPkg/SataControllerDxe/SataController.h | 5 +++ OvmfPkg/SataControllerDxe/SataController.c | 39 +++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/OvmfPkg/SataControllerDxe/SataController.h b/OvmfPkg/SataControllerDxe/SataController.h index a6c6c16..e5b719e 100644 --- a/OvmfPkg/SataControllerDxe/SataController.h +++ b/OvmfPkg/SataControllerDxe/SataController.h @@ -86,6 +86,11 @@ typedef struct _EFI_SATA_CONTROLLER_PRIVATE_DATA { EFI_PCI_IO_PROTOCOL *PciIo; // + // Original PCI attributes + // + UINT64 OriginalPciAttributes; + + // // The number of devices that are supported by this channel // UINT8 DeviceCount; diff --git a/OvmfPkg/SataControllerDxe/SataController.c b/OvmfPkg/SataControllerDxe/SataController.c index 5e7e23b..275b527 100644 --- a/OvmfPkg/SataControllerDxe/SataController.c +++ b/OvmfPkg/SataControllerDxe/SataController.c @@ -390,6 +390,7 @@ SataControllerStart ( { EFI_STATUS Status; EFI_PCI_IO_PROTOCOL *PciIo; + UINT64 OriginalPciAttributes; PCI_TYPE00 PciData; EFI_SATA_CONTROLLER_PRIVATE_DATA *SataPrivateData; UINT32 Data32; @@ -415,12 +416,33 @@ SataControllerStart ( } // + // Save original PCI attributes, and enable IO space access, memory + space // access, and Bus Master (DMA). + // + Status = PciIo->Attributes (PciIo, EfiPciIoAttributeOperationGet, 0, + &OriginalPciAttributes); if (EFI_ERROR (Status)) { + goto ClosePciIo; + } + Status = PciIo->Attributes ( + PciIo, + EfiPciIoAttributeOperationEnable, + (EFI_PCI_IO_ATTRIBUTE_IO | + EFI_PCI_IO_ATTRIBUTE_MEMORY | + EFI_PCI_IO_ATTRIBUTE_BUS_MASTER), + NULL + ); + if (EFI_ERROR (Status)) { + goto ClosePciIo; + } + + // // Allocate Sata Private Data structure // SataPrivateData = AllocateZeroPool (sizeof (EFI_SATA_CONTROLLER_PRIVATE_DATA)); if (SataPrivateData == NULL) { Status = EFI_OUT_OF_RESOURCES; - goto ClosePciIo; + goto RestorePciAttributes; } // @@ -428,6 +450,7 @@ SataControllerStart ( // SataPrivateData->Signature = SATA_CONTROLLER_SIGNATURE; SataPrivateData->PciIo = PciIo; + SataPrivateData->OriginalPciAttributes = OriginalPciAttributes; SataPrivateData->IdeInit.GetChannelInfo = IdeInitGetChannelInfo; SataPrivateData->IdeInit.NotifyPhase = IdeInitNotifyPhase; SataPrivateData->IdeInit.SubmitData = IdeInitSubmitData; @@ -512,6 +535,10 @@ FreeDisqualifiedModes: FreeSataPrivateData: FreePool (SataPrivateData); +RestorePciAttributes: + PciIo->Attributes (PciIo, EfiPciIoAttributeOperationSet, + OriginalPciAttributes, NULL); + ClosePciIo: gBS->CloseProtocol ( Controller, @@ -595,6 +622,16 @@ SataControllerStop ( } // + // Restore original PCI attributes + // + SataPrivateData->PciIo->Attributes ( + SataPrivateData->PciIo, + EfiPciIoAttributeOperationSet, + SataPrivateData->OriginalPciAttributes, + NULL + ); + + // // Close protocols opened by Sata Controller driver // return gBS->CloseProtocol ( -- 1.8.3.1 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
