On 09/21/15 03:15, Tian, Feng wrote: > 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]>
Good idea. I'll fix it up at commit time. Thanks! Laszlo > > 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
