On Thu, Jun 14, 2018 at 5:08 PM, Sami Mujawar <sami.muja...@arm.com> wrote: > The SATA controller driver crashes while accessing the > PCI memory, as the PCI memory space is not enabled. > > Enable the PCI memory space access to prevent the SATA > Controller driver from crashing. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Sami Mujawar <sami.muja...@arm.com> > --- > The changes can be seen at > https://github.com/samimujawar/edk2/tree/284_sata_controler_pci_mem_fix_v1 > > Notes: > v1: > - Fix SATA Controller driver crash [SAMI] > > MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c | 80 > +++++++++++++++++++- > MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.h | 7 ++ > 2 files changed, 86 insertions(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c > b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c > index > a6d55c15571728eb3fd572003f383ba7c86635ae..21cc101d693f5adfd9d43f0c21a096eb59ba73b1 > 100644 > --- a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c > +++ b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c > @@ -2,6 +2,7 @@ > This driver module produces IDE_CONTROLLER_INIT protocol for Sata > Controllers. > > Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2018, ARM Ltd. All rights reserved.<BR> > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD > License > which accompanies this distribution. The full text of the license may be > found at > @@ -364,6 +365,7 @@ SataControllerStart ( > EFI_SATA_CONTROLLER_PRIVATE_DATA *Private; > UINT32 Data32; > UINTN TotalCount; > + UINT64 PciAttributes; > > DEBUG ((EFI_D_INFO, "SataControllerStart start\n")); > > @@ -406,6 +408,61 @@ SataControllerStart ( > Private->IdeInit.CalculateMode = IdeInitCalculateMode; > Private->IdeInit.SetTiming = IdeInitSetTiming; > Private->IdeInit.EnumAll = SATA_ENUMER_ALL; > + Private->PciAttributesChanged = FALSE; > + > + // Save original PCI attributes > + Status = PciIo->Attributes ( > + PciIo, > + EfiPciIoAttributeOperationGet, > + 0, > + &Private->OriginalPciAttributes > + ); > + if (EFI_ERROR (Status)) { > + goto Done; > + } > + > + DEBUG (( > + EFI_D_INFO, > + "PCI Attributes = 0x%llx\n", > + Private->OriginalPciAttributes > + )); > + > + if ((Private->OriginalPciAttributes & EFI_PCI_IO_ATTRIBUTE_MEMORY) == 0) { > + Status = PciIo->Attributes ( > + PciIo, > + EfiPciIoAttributeOperationSupported, > + 0, > + &PciAttributes > + ); > + if (EFI_ERROR (Status)) { > + goto Done; > + } > + > + DEBUG ((EFI_D_INFO, "Supported PCI Attributes = 0x%llx\n", > PciAttributes)); > + > + if ((PciAttributes & EFI_PCI_IO_ATTRIBUTE_MEMORY) == 0) { > + DEBUG (( > + EFI_D_ERROR, > + "Error: EFI_PCI_IO_ATTRIBUTE_MEMORY not supported\n" > + )); > + Status = EFI_UNSUPPORTED; > + goto Done; > + } > + > + PciAttributes = Private->OriginalPciAttributes | > EFI_PCI_IO_ATTRIBUTE_MEMORY; > + > + DEBUG ((EFI_D_INFO, "Enable PCI Attributes = 0x%llx\n", PciAttributes)); > + Status = PciIo->Attributes ( > + PciIo, > + EfiPciIoAttributeOperationEnable, > + PciAttributes, > + NULL > + ); > + if (EFI_ERROR (Status)) { > + goto Done; > + } > + Private->PciAttributesChanged = TRUE; > + } > > Status = PciIo->Pci.Read ( > PciIo, > @@ -414,7 +471,10 @@ SataControllerStart ( > sizeof (PciData.Hdr.ClassCode), > PciData.Hdr.ClassCode > ); > - ASSERT_EFI_ERROR (Status); > + if (EFI_ERROR (Status)) { > + ASSERT (FALSE); > + goto Done; > + } > > if (IS_PCI_IDE (&PciData)) { > Private->IdeInit.ChannelCount = IDE_MAX_CHANNEL; > @@ -481,6 +541,15 @@ Done: > if (Private->IdentifyValid != NULL) { > FreePool (Private->IdentifyValid); > } > + if (Private->PciAttributesChanged) { > + // Restore original PCI attributes > + PciIo->Attributes ( > + PciIo, > + EfiPciIoAttributeOperationSet, > + Private->OriginalPciAttributes, > + NULL > + ); > + } > FreePool (Private); > } > } > @@ -556,6 +625,15 @@ SataControllerStop ( > if (Private->IdentifyValid != NULL) { > FreePool (Private->IdentifyValid); > } > + if (Private->PciAttributesChanged) { > + // Restore original PCI attributes > + Private->PciIo->Attributes ( > + Private->PciIo, > + EfiPciIoAttributeOperationSet, > + Private->OriginalPciAttributes, > + NULL > + ); > + } > FreePool (Private); > } > > diff --git a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.h > b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.h > index > f7db3b832a14c0c8314518cfdf4198c7a4e8ef25..cb82b55763a077f5994c4a00ea4893bfa2e07a79 > 100644 > --- a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.h > +++ b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.h > @@ -2,6 +2,7 @@ > Header file for Sata Controller driver. > > Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2017, ARM Ltd. All rights reserved.<BR> > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD > License > which accompanies this distribution. The full text of the license may be > found at > @@ -104,6 +105,12 @@ typedef struct _EFI_SATA_CONTROLLER_PRIVATE_DATA { > // > EFI_IDENTIFY_DATA *IdentifyData; > BOOLEAN *IdentifyValid; > + > + /// Track the state so that the PCI attributes that were modified > + /// can be restored to the original value later > + BOOLEAN PciAttributesChanged; > + /// Copy of the original PCI Attributes > + UINT64 OriginalPciAttributes; > } EFI_SATA_CONTROLLER_PRIVATE_DATA; > > #define SATA_CONTROLLER_PRIVATE_DATA_FROM_THIS(a) CR(a, > EFI_SATA_CONTROLLER_PRIVATE_DATA, IdeInit, SATA_CONTROLLER_SIGNATURE) > -- > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
Tested on SGI-575 platform and this helps to avoid a exception when accessing SATA controller over PCI. Tested-by: Thomas Abraham <thomas.abra...@arm.com> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
