On Tue, Nov 27, 2018 at 04:56:19PM +0530, Jagadeesh Ujja wrote: > Adapt the NorFlashDxe driver to be used as a MM_STANDALONE driver to > allow access to NOR flash for code executing in MM_STANDALONE mode. > This allows storing of EFI variables on NOR flash which is accessible > only via the MM STANDALONE mode software. > > Change-Id: Ic55ea0bc4098aefd6edfea03e11116dd5ccf5f2e
Please don't litter commit messages with company-internal tracking data. > Signed-off-by: Jagadeesh Ujja <jagadeesh.u...@arm.com> > Signed-off-by: Thomas Abraham <thomas.abra...@arm.com> > Signed-off-by: Vishwanatha HG <vishwanatha...@arm.com> There can be only one Signed-off-by for a patch. That sign-off is you testifying that this code is submissible under the licenses stated. If you are contributing a patch where you are not the Author, that will be reflected by the From: header added by git. > --- > .../Drivers/NorFlashDxe/NorFlashBlockIoDxe.c | 2 +- > .../Drivers/NorFlashDxe/NorFlashDxe.c | 211 ++++++++++++++---- > .../Drivers/NorFlashDxe/NorFlashDxe.h | 5 +- > .../Drivers/NorFlashDxe/NorFlashDxe.inf | 3 + > .../Drivers/NorFlashDxe/NorFlashFvbDxe.c | 96 ++++---- > .../NorFlashDxe/NorFlashStandaloneMm.inf | 76 +++++++ > 6 files changed, 304 insertions(+), 89 deletions(-) > create mode 100644 > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf Please rework this set and resubmit based on the above comments, and also ensuring to follow the guidelines in https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers Regards, Leif > diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c > b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c > index 279b77c75e..4c002c7d65 100644 > --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c > +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c > @@ -1,6 +1,6 @@ > /** @file NorFlashBlockIoDxe.c > > - Copyright (c) 2011-2013, ARM Ltd. All rights reserved.<BR> > + Copyright (c) 2011-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 > diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c > b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c > index 46e815beb3..706906a974 100644 > --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c > +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c > @@ -1,6 +1,6 @@ > /** @file NorFlashDxe.c > > - Copyright (c) 2011 - 2014, ARM Ltd. All rights reserved.<BR> > + Copyright (c) 2011 - 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 > @@ -134,29 +134,102 @@ NorFlashCreateInstance ( > > if (SupportFvb) { > NorFlashFvbInitialize (Instance); > + if (!InMm ()) { > + Status = gBS->InstallMultipleProtocolInterfaces ( > + &Instance->Handle, > + &gEfiDevicePathProtocolGuid, &Instance->DevicePath, > + &gEfiBlockIoProtocolGuid, > &Instance->BlockIoProtocol, > + &gEfiFirmwareVolumeBlockProtocolGuid, > &Instance->FvbProtocol, > + NULL > + ); > + if (EFI_ERROR(Status)) { > + FreePool (Instance); > + return Status; > + } > + } else { > + //Install DevicePath Protocol > + Status = gMmst->MmInstallProtocolInterface ( > + &Instance->Handle, > + &gEfiDevicePathProtocolGuid, > + EFI_NATIVE_INTERFACE, > + &Instance->DevicePath > + ); > + if (EFI_ERROR(Status)) { > + FreePool (Instance); > + return Status; > + } > + //Install BlockIo Protocol > + Status = gMmst->MmInstallProtocolInterface ( > + &Instance->Handle, > + &gEfiBlockIoProtocolGuid, > + EFI_NATIVE_INTERFACE, > + &Instance->BlockIoProtocol > + ); > + if (EFI_ERROR(Status)) { > + FreePool (Instance); > + return Status; > + } > > - Status = gBS->InstallMultipleProtocolInterfaces ( > - &Instance->Handle, > - &gEfiDevicePathProtocolGuid, &Instance->DevicePath, > - &gEfiBlockIoProtocolGuid, &Instance->BlockIoProtocol, > - &gEfiFirmwareVolumeBlockProtocolGuid, > &Instance->FvbProtocol, > - NULL > - ); > - if (EFI_ERROR(Status)) { > - FreePool (Instance); > - return Status; > + //Install FirmwareVolumeBlock Protocol > + Status = gMmst->MmInstallProtocolInterface ( > + &Instance->Handle, > + &gEfiSmmFirmwareVolumeBlockProtocolGuid, > + EFI_NATIVE_INTERFACE, > + &Instance->FvbProtocol > + ); > + if (EFI_ERROR(Status)) { > + FreePool (Instance); > + return Status; > + } > } > } else { > - Status = gBS->InstallMultipleProtocolInterfaces ( > - &Instance->Handle, > - &gEfiDevicePathProtocolGuid, &Instance->DevicePath, > - &gEfiBlockIoProtocolGuid, &Instance->BlockIoProtocol, > - &gEfiDiskIoProtocolGuid, &Instance->DiskIoProtocol, > - NULL > - ); > - if (EFI_ERROR(Status)) { > - FreePool (Instance); > - return Status; > + if (!InMm ()) { > + Status = gBS->InstallMultipleProtocolInterfaces ( > + &Instance->Handle, > + &gEfiDevicePathProtocolGuid, &Instance->DevicePath, > + &gEfiBlockIoProtocolGuid, &Instance->BlockIoProtocol, > + &gEfiDiskIoProtocolGuid, &Instance->DiskIoProtocol, > + NULL > + ); > + if (EFI_ERROR(Status)) { > + FreePool (Instance); > + return Status; > + } > + } else { > + //Install DevicePath Protocol > + Status = gMmst->MmInstallProtocolInterface ( > + &Instance->Handle, > + &gEfiDevicePathProtocolGuid, > + EFI_NATIVE_INTERFACE, > + &Instance->DevicePath > + ); > + if (EFI_ERROR(Status)) { > + FreePool (Instance); > + return Status; > + } > + //Install BlockIo Protocol > + Status = gMmst->MmInstallProtocolInterface ( > + &Instance->Handle, > + &gEfiBlockIoProtocolGuid, > + EFI_NATIVE_INTERFACE, > + &Instance->BlockIoProtocol > + ); > + if (EFI_ERROR(Status)) { > + FreePool (Instance); > + return Status; > + } > + > + //Install DiskIO Protocol > + Status = gMmst->MmInstallProtocolInterface ( > + &Instance->Handle, > + &gEfiDiskIoProtocolGuid, > + EFI_NATIVE_INTERFACE, > + &Instance->DiskIoProtocol > + ); > + if (EFI_ERROR(Status)) { > + FreePool (Instance); > + return Status; > + } > } > } > > @@ -338,13 +411,15 @@ NorFlashUnlockAndEraseSingleBlock ( > UINTN Index; > EFI_TPL OriginalTPL; > > - if (!EfiAtRuntime ()) { > - // Raise TPL to TPL_HIGH to stop anyone from interrupting us. > - OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL); > - } else { > - // This initialization is only to prevent the compiler to complain about > the > - // use of uninitialized variables > - OriginalTPL = TPL_HIGH_LEVEL; > + if (!InMm ()) { > + if (!EfiAtRuntime ()) { > + // Raise TPL to TPL_HIGH to stop anyone from interrupting us. > + OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL); > + } else { > + // This initialization is only to prevent the compiler to complain > about the > + // use of uninitialized variables > + OriginalTPL = TPL_HIGH_LEVEL; > + } > } > > Index = 0; > @@ -363,9 +438,11 @@ NorFlashUnlockAndEraseSingleBlock ( > DEBUG((EFI_D_ERROR,"EraseSingleBlock(BlockAddress=0x%08x: Block Locked > Error (try to erase %d times)\n", BlockAddress,Index)); > } > > - if (!EfiAtRuntime ()) { > - // Interruptions can resume. > - gBS->RestoreTPL (OriginalTPL); > + if (!InMm ()) { > + if (!EfiAtRuntime ()) { > + // Interruptions can resume. > + gBS->RestoreTPL (OriginalTPL); > + } > } > > return Status; > @@ -591,13 +668,15 @@ NorFlashWriteFullBlock ( > // Start writing from the first address at the start of the block > WordAddress = BlockAddress; > > - if (!EfiAtRuntime ()) { > - // Raise TPL to TPL_HIGH to stop anyone from interrupting us. > - OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL); > - } else { > - // This initialization is only to prevent the compiler to complain about > the > - // use of uninitialized variables > - OriginalTPL = TPL_HIGH_LEVEL; > + if (!InMm ()) { > + if (!EfiAtRuntime ()) { > + // Raise TPL to TPL_HIGH to stop anyone from interrupting us. > + OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL); > + } else { > + // This initialization is only to prevent the compiler to complain > about the > + // use of uninitialized variables > + OriginalTPL = TPL_HIGH_LEVEL; > + } > } > > Status = NorFlashUnlockAndEraseSingleBlock (Instance, BlockAddress); > @@ -657,9 +736,11 @@ NorFlashWriteFullBlock ( > } > > EXIT: > - if (!EfiAtRuntime ()) { > - // Interruptions can resume. > - gBS->RestoreTPL (OriginalTPL); > + if (!InMm ()) { > + if (!EfiAtRuntime ()) { > + // Interruptions can resume. > + gBS->RestoreTPL (OriginalTPL); > + } > } > > if (EFI_ERROR(Status)) { > @@ -1331,6 +1412,54 @@ NorFlashInitialise ( > &mNorFlashVirtualAddrChangeEvent > ); > ASSERT_EFI_ERROR (Status); > + return Status; > +} > > +EFI_STATUS > +EFIAPI > +StandaloneMmNorFlashInitialise ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_MM_SYSTEM_TABLE *SystemTable > + ) > +{ > + EFI_STATUS Status; > + UINT32 Index; > + NOR_FLASH_DESCRIPTION* NorFlashDevices; > + BOOLEAN ContainVariableStorage; > + > + Status = NorFlashPlatformInitialization (); > + if (EFI_ERROR(Status)) { > + DEBUG((EFI_D_ERROR,"NorFlashInitialise: Fail to initialize Nor Flash > devices\n")); > + return Status; > + } > + > + Status = NorFlashPlatformGetDevices (&NorFlashDevices, > &mNorFlashDeviceCount); > + if (EFI_ERROR(Status)) { > + DEBUG((EFI_D_ERROR,"NorFlashInitialise: Fail to get Nor Flash > devices\n")); > + return Status; > + } > + > + mNorFlashInstances = AllocateRuntimePool (sizeof(NOR_FLASH_INSTANCE*) * > mNorFlashDeviceCount); > + > + for (Index = 0; Index < mNorFlashDeviceCount; Index++) { > + // Check if this NOR Flash device contain the variable storage region > + ContainVariableStorage = > + (NorFlashDevices[Index].RegionBaseAddress <= PcdGet32 > (PcdFlashNvStorageVariableBase)) && > + (PcdGet32 (PcdFlashNvStorageVariableBase) + PcdGet32 > (PcdFlashNvStorageVariableSize) <= NorFlashDevices[Index].RegionBaseAddress + > NorFlashDevices[Index].Size); > + > + Status = NorFlashCreateInstance ( > + NorFlashDevices[Index].DeviceBaseAddress, > + NorFlashDevices[Index].RegionBaseAddress, > + NorFlashDevices[Index].Size, > + Index, > + NorFlashDevices[Index].BlockSize, > + ContainVariableStorage, > + &NorFlashDevices[Index].Guid, > + &mNorFlashInstances[Index] > + ); > + if (EFI_ERROR(Status)) { > + DEBUG((EFI_D_ERROR,"NorFlashInitialise: Fail to create instance for > NorFlash[%d]\n",Index)); > + } > + } > return Status; > } > diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h > b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h > index 5c07694fbf..e3932a190b 100644 > --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h > +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h > @@ -1,6 +1,6 @@ > /** @file NorFlashDxe.h > > - Copyright (c) 2011 - 2014, ARM Ltd. All rights reserved.<BR> > + Copyright (c) 2011 - 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 > @@ -19,6 +19,7 @@ > #include <Base.h> > #include <PiDxe.h> > > +#include <PiMm.h> > #include <Guid/EventGroup.h> > > #include <Protocol/BlockIo.h> > @@ -30,6 +31,8 @@ > #include <Library/NorFlashPlatformLib.h> > #include <Library/UefiLib.h> > #include <Library/UefiRuntimeLib.h> > +#include <Library/StandaloneMmServicesTableLib.h> > +#include <Library/StandaloneMmRuntimeDxe.h> > > #define NOR_FLASH_ERASE_RETRY 10 > > diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf > b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf > index a59a21a03e..a704f69ef3 100644 > --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf > +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf > @@ -32,6 +32,7 @@ > MdeModulePkg/MdeModulePkg.dec > ArmPlatformPkg/ArmPlatformPkg.dec > EmbeddedPkg/EmbeddedPkg.dec > + StandaloneMmPkg/StandaloneMmPkg.dec > > [LibraryClasses] > IoLib > @@ -44,6 +45,7 @@ > UefiBootServicesTableLib > UefiRuntimeLib > DxeServicesTableLib > + StandaloneMmRuntimeDxe > > [Guids] > gEfiSystemNvDataFvGuid > @@ -57,6 +59,7 @@ > gEfiDevicePathProtocolGuid > gEfiFirmwareVolumeBlockProtocolGuid > gEfiDiskIoProtocolGuid > + gEfiSmmFirmwareVolumeBlockProtocolGuid > > [Pcd.common] > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase > diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c > b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c > index e62ffbb433..e4d7100ee1 100644 > --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c > +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c > @@ -1,6 +1,6 @@ > /*++ @file NorFlashFvbDxe.c > > - Copyright (c) 2011 - 2014, ARM Ltd. All rights reserved.<BR> > + Copyright (c) 2011 - 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 > @@ -720,27 +720,29 @@ NorFlashFvbInitialize ( > DEBUG((DEBUG_BLKIO,"NorFlashFvbInitialize\n")); > ASSERT((Instance != NULL)); > > - // > - // Declare the Non-Volatile storage as EFI_MEMORY_RUNTIME > - // > - > - // Note: all the NOR Flash region needs to be reserved into the UEFI > Runtime memory; > - // even if we only use the small block region at the top of the NOR > Flash. > - // The reason is when the NOR Flash memory is set into program mode, > the command > - // is written as the base of the flash region (ie: > Instance->DeviceBaseAddress) > - RuntimeMmioRegionSize = (Instance->RegionBaseAddress - > Instance->DeviceBaseAddress) + Instance->Size; > - > - Status = gDS->AddMemorySpace ( > - EfiGcdMemoryTypeMemoryMappedIo, > - Instance->DeviceBaseAddress, RuntimeMmioRegionSize, > - EFI_MEMORY_UC | EFI_MEMORY_RUNTIME > - ); > - ASSERT_EFI_ERROR (Status); > - > - Status = gDS->SetMemorySpaceAttributes ( > - Instance->DeviceBaseAddress, RuntimeMmioRegionSize, > - EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > - ASSERT_EFI_ERROR (Status); > + if (!InMm ()) { > + // > + // Declare the Non-Volatile storage as EFI_MEMORY_RUNTIME > + // > + > + // Note: all the NOR Flash region needs to be reserved into the UEFI > Runtime memory; > + // even if we only use the small block region at the top of the > NOR Flash. > + // The reason is when the NOR Flash memory is set into program > mode, the command > + // is written as the base of the flash region (ie: > Instance->DeviceBaseAddress) > + RuntimeMmioRegionSize = (Instance->RegionBaseAddress - > Instance->DeviceBaseAddress) + Instance->Size; > + > + Status = gDS->AddMemorySpace ( > + EfiGcdMemoryTypeMemoryMappedIo, > + Instance->DeviceBaseAddress, RuntimeMmioRegionSize, > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME > + ); > + ASSERT_EFI_ERROR (Status); > + > + Status = gDS->SetMemorySpaceAttributes ( > + Instance->DeviceBaseAddress, RuntimeMmioRegionSize, > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > + ASSERT_EFI_ERROR (Status); > + } > > mFlashNvStorageVariableBase = FixedPcdGet32 > (PcdFlashNvStorageVariableBase); > > @@ -777,30 +779,32 @@ NorFlashFvbInitialize ( > } > } > > - // > - // The driver implementing the variable read service can now be dispatched; > - // the varstore headers are in place. > - // > - Status = gBS->InstallProtocolInterface ( > - &gImageHandle, > - &gEdkiiNvVarStoreFormattedGuid, > - EFI_NATIVE_INTERFACE, > - NULL > - ); > - ASSERT_EFI_ERROR (Status); > - > - // > - // Register for the virtual address change event > - // > - Status = gBS->CreateEventEx ( > - EVT_NOTIFY_SIGNAL, > - TPL_NOTIFY, > - FvbVirtualNotifyEvent, > - NULL, > - &gEfiEventVirtualAddressChangeGuid, > - &mFvbVirtualAddrChangeEvent > - ); > - ASSERT_EFI_ERROR (Status); > + if (!InMm ()) { > + // > + // The driver implementing the variable read service can now be > dispatched; > + // the varstore headers are in place. > + // > + Status = gBS->InstallProtocolInterface ( > + &gImageHandle, > + &gEdkiiNvVarStoreFormattedGuid, > + EFI_NATIVE_INTERFACE, > + NULL > + ); > + ASSERT_EFI_ERROR (Status); > + > + // > + // Register for the virtual address change event > + // > + Status = gBS->CreateEventEx ( > + EVT_NOTIFY_SIGNAL, > + TPL_NOTIFY, > + FvbVirtualNotifyEvent, > + NULL, > + &gEfiEventVirtualAddressChangeGuid, > + &mFvbVirtualAddrChangeEvent > + ); > + ASSERT_EFI_ERROR (Status); > + } > > return Status; > } > diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf > b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf > new file mode 100644 > index 0000000000..a6d0581b79 > --- /dev/null > +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf > @@ -0,0 +1,76 @@ > +#/** @file > +# > +# Component description file for NorFlashDxe module > +# > +# Copyright (c) 2018, ARM Limited. All rights reserved. > +# > +# 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 > +# http://opensource.org/licenses/bsd-license.php > +# > +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR > IMPLIED. > +# > +#**/ > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = StandaloneMmNorFlash > + FILE_GUID = 166F677B-DAC9-4AE4-AD34-2FF2504B0637 > + MODULE_TYPE = MM_STANDALONE > + VERSION_STRING = 1.0 > + PI_SPECIFICATION_VERSION = 0x00010032 > + ENTRY_POINT = StandaloneMmNorFlashInitialise > + > +[Sources.common] > + NorFlashDxe.c > + NorFlashFvbDxe.c > + NorFlashBlockIoDxe.c > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + ArmPlatformPkg/ArmPlatformPkg.dec > + EmbeddedPkg/EmbeddedPkg.dec > + ArmPkg/ArmPkg.dec > + StandaloneMmPkg/StandaloneMmPkg.dec > + > +[LibraryClasses] > + StandaloneMmDriverEntryPoint > + BaseMemoryLib > + ArmSvcLib > + ArmLib > + IoLib > + BaseLib > + DebugLib > + HobLib > + MemoryAllocationLib > + NorFlashPlatformLib > + MmServicesTableLib > + > +[Guids] > + gEfiSystemNvDataFvGuid > + gEfiVariableGuid > + gEfiAuthenticatedVariableGuid > + gEfiEventVirtualAddressChangeGuid > + gEdkiiNvVarStoreFormattedGuid ## PRODUCES ## PROTOCOL > + > +[Protocols] > + gEfiBlockIoProtocolGuid > + gEfiDevicePathProtocolGuid > + gEfiSmmFirmwareVolumeBlockProtocolGuid > + gEfiDiskIoProtocolGuid > + > +[Pcd.common] > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize > + > + gArmPlatformTokenSpaceGuid.PcdNorFlashCheckBlockLocked > + > +[Depex] > + TRUE > -- > 2.19.1 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel