* [PATCH] ArmPlatformPkg/NorFlashDxe: Change Flash memory attributes before writes
@ 2017-01-20 11:54 achin.gupta
2017-01-20 11:58 ` Ard Biesheuvel
0 siblings, 1 reply; 2+ messages in thread
From: achin.gupta @ 2017-01-20 11:54 UTC (permalink / raw)
To: edk2-devel; +Cc: leif.lindholm, ard.biesheuvel, nd, Achin Gupta
From: Achin Gupta <achin.gupta@arm.com>
In NorFlashFvbInitialize() if a valid Firmware Volume header is not found at the
start of NOR Flash, the Flash memory is written before it has been remapped with
EFI_MEMORY_UC attributes to allow write commands. Since the flash memory was
previously mapped with Normal and possibly cacheable memory attributes, the
Flash commands might never reach the device.
This patch fixes this issue by remapping the Flash memory region with correct
memory attributes before writing to it.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Achin Gupta <achin.gupta@arm.com>
---
.../Drivers/NorFlashDxe/NorFlashFvbDxe.c | 45 +++++++++++-----------
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
index 42be5c2..12a8612 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
@@ -719,6 +719,29 @@ NorFlashFvbInitialize (
UINTN RuntimeMmioRegionSize;
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);
Instance->Initialized = TRUE;
mFlashNvStorageVariableBase = FixedPcdGet32 (PcdFlashNvStorageVariableBase);
@@ -757,28 +780,6 @@ NorFlashFvbInitialize (
}
//
- // 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);
-
- //
// Register for the virtual address change event
//
Status = gBS->CreateEventEx (
--
1.9.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] ArmPlatformPkg/NorFlashDxe: Change Flash memory attributes before writes
2017-01-20 11:54 [PATCH] ArmPlatformPkg/NorFlashDxe: Change Flash memory attributes before writes achin.gupta
@ 2017-01-20 11:58 ` Ard Biesheuvel
0 siblings, 0 replies; 2+ messages in thread
From: Ard Biesheuvel @ 2017-01-20 11:58 UTC (permalink / raw)
To: Achin Gupta; +Cc: edk2-devel@lists.01.org, Leif Lindholm, nd
On 20 January 2017 at 11:54, <achin.gupta@arm.com> wrote:
> From: Achin Gupta <achin.gupta@arm.com>
>
> In NorFlashFvbInitialize() if a valid Firmware Volume header is not found at the
> start of NOR Flash, the Flash memory is written before it has been remapped with
> EFI_MEMORY_UC attributes to allow write commands. Since the flash memory was
> previously mapped with Normal and possibly cacheable memory attributes, the
> Flash commands might never reach the device.
>
> This patch fixes this issue by remapping the Flash memory region with correct
> memory attributes before writing to it.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Achin Gupta <achin.gupta@arm.com>
Thanks Achin!
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Pushed as 90d1f671cdad
> ---
> .../Drivers/NorFlashDxe/NorFlashFvbDxe.c | 45 +++++++++++-----------
> 1 file changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> index 42be5c2..12a8612 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> @@ -719,6 +719,29 @@ NorFlashFvbInitialize (
> UINTN RuntimeMmioRegionSize;
>
> 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);
>
> Instance->Initialized = TRUE;
> mFlashNvStorageVariableBase = FixedPcdGet32 (PcdFlashNvStorageVariableBase);
> @@ -757,28 +780,6 @@ NorFlashFvbInitialize (
> }
>
> //
> - // 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);
> -
> - //
> // Register for the virtual address change event
> //
> Status = gBS->CreateEventEx (
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-01-20 11:58 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-20 11:54 [PATCH] ArmPlatformPkg/NorFlashDxe: Change Flash memory attributes before writes achin.gupta
2017-01-20 11:58 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox