From 518c243d42731389502431a0fbf14b932f838888 Mon Sep 17 00:00:00 2001 From: Harry Liebel Date: Thu, 8 May 2014 14:48:55 +0000 Subject: [PATCH] ArmPlatformPkg/NorFlashDxe: Optimise FVB protocol - Only read what needs reading, don't read the whole block. - Don't write back buffers containing no data after an erase. - Reduce number of NOR erases when writing data. Only erase the block when required. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Harry Liebel Reviewed-by: Olivier Martin git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@15500 6f19259b-4bc3-4df7-8a09-765794883524 --- .../Drivers/NorFlashDxe/NorFlashDxe.c | 67 ++++++- .../Drivers/NorFlashDxe/NorFlashDxe.h | 22 +++ .../Drivers/NorFlashDxe/NorFlashFvbDxe.c | 178 +++++++++++++++--- 3 files changed, 237 insertions(+), 30 deletions(-) diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c index a03bf5749e..82b9f3f742 100644 --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c @@ -586,6 +586,7 @@ NorFlashWriteSingleBlock ( UINTN BuffersInBlock; UINTN RemainingWords; EFI_TPL OriginalTPL; + UINTN Cnt; Status = EFI_SUCCESS; @@ -619,13 +620,22 @@ NorFlashWriteSingleBlock ( BuffersInBlock = (UINTN)(BlockSizeInWords * 4) / P30_MAX_BUFFER_SIZE_IN_BYTES; // Then feed each buffer chunk to the NOR Flash + // If a buffer does not contain any data, don't write it. for(BufferIndex=0; BufferIndex < BuffersInBlock; BufferIndex++, WordAddress += P30_MAX_BUFFER_SIZE_IN_BYTES, DataBuffer += P30_MAX_BUFFER_SIZE_IN_WORDS ) { - Status = NorFlashWriteBuffer (Instance, WordAddress, P30_MAX_BUFFER_SIZE_IN_BYTES, DataBuffer); - if (EFI_ERROR(Status)) { - goto EXIT; + // Check the buffer to see if it contains any data (not set all 1s). + for (Cnt = 0; Cnt < P30_MAX_BUFFER_SIZE_IN_WORDS; Cnt++) { + if (~DataBuffer[Cnt] != 0 ) { + // Some data found, write the buffer. + Status = NorFlashWriteBuffer (Instance, WordAddress, P30_MAX_BUFFER_SIZE_IN_BYTES, + DataBuffer); + if (EFI_ERROR(Status)) { + goto EXIT; + } + break; + } } } @@ -784,6 +794,57 @@ NorFlashReadBlocks ( return EFI_SUCCESS; } +EFI_STATUS +NorFlashRead ( + IN NOR_FLASH_INSTANCE *Instance, + IN EFI_LBA Lba, + IN UINTN Offset, + IN UINTN BufferSizeInBytes, + OUT VOID *Buffer + ) +{ + UINT32 NumBlocks; + UINTN StartAddress; + + // The buffer must be valid + if (Buffer == NULL) { + return EFI_INVALID_PARAMETER; + } + + // Return if we have not any byte to read + if (BufferSizeInBytes == 0) { + return EFI_SUCCESS; + } + + // All blocks must be within the device + NumBlocks = ((UINT32)BufferSizeInBytes) / Instance->Media.BlockSize ; + + if ((Lba + NumBlocks) > (Instance->Media.LastBlock + 1)) { + DEBUG ((EFI_D_ERROR, "NorFlashRead: ERROR - Read will exceed last block\n")); + return EFI_INVALID_PARAMETER; + } + + if (Offset + BufferSizeInBytes >= Instance->Size) { + DEBUG ((EFI_D_ERROR, "NorFlashRead: ERROR - Read will exceed device size.\n")); + return EFI_INVALID_PARAMETER; + } + + // Get the address to start reading from + StartAddress = GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress, + Lba, + Instance->Media.BlockSize + ); + + // Put the device into Read Array mode + SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY); + + // Readout the data + CopyMem (Buffer, (UINTN *)(StartAddress + Offset), BufferSizeInBytes); + + return EFI_SUCCESS; +} + + EFI_STATUS NorFlashReset ( IN NOR_FLASH_INSTANCE *Instance diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h index e4d767bf94..6c8cea28ea 100644 --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h @@ -304,6 +304,13 @@ NorFlashWriteBlocks ( IN VOID *Buffer ); +EFI_STATUS +NorFlashWriteSingleWord ( + IN NOR_FLASH_INSTANCE *Instance, + IN UINTN WordAddress, + IN UINT32 WriteData + ); + EFI_STATUS NorFlashReadBlocks ( IN NOR_FLASH_INSTANCE *Instance, @@ -312,9 +319,24 @@ NorFlashReadBlocks ( OUT VOID *Buffer ); +EFI_STATUS +NorFlashRead ( + IN NOR_FLASH_INSTANCE *Instance, + IN EFI_LBA Lba, + IN UINTN Offset, + IN UINTN BufferSizeInBytes, + OUT VOID *Buffer + ); + EFI_STATUS NorFlashReset ( IN NOR_FLASH_INSTANCE *Instance ); +EFI_STATUS +NorFlashUnlockSingleBlockIfNecessary ( + IN NOR_FLASH_INSTANCE *Instance, + IN UINTN BlockAddress + ); + #endif /* __NOR_FLASH_DXE_H__ */ diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c index 84890cbfce..9ad0e14668 100644 --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c @@ -417,7 +417,6 @@ FvbRead ( IN OUT UINT8 *Buffer ) { - EFI_STATUS Status; EFI_STATUS TempStatus; UINTN BlockSize; NOR_FLASH_INSTANCE *Instance; @@ -430,8 +429,7 @@ FvbRead ( Instance->Initialize(Instance); } - Status = EFI_SUCCESS; - TempStatus = Status; + TempStatus = EFI_SUCCESS; // Cache the block size to avoid de-referencing pointers all the time BlockSize = Instance->Media.BlockSize; @@ -452,25 +450,21 @@ FvbRead ( return EFI_BAD_BUFFER_SIZE; } - // Check we did get some memory - if (Instance->FvbBuffer == NULL) { - DEBUG ((EFI_D_ERROR, "FvbRead: ERROR - Buffer not ready\n")); - return EFI_DEVICE_ERROR; - } - - // Read NOR Flash data into shadow buffer - TempStatus = NorFlashReadBlocks (Instance, Instance->StartLba + Lba, BlockSize, Instance->FvbBuffer); - if (EFI_ERROR (TempStatus)) { - // Return one of the pre-approved error statuses - return EFI_DEVICE_ERROR; + // Decide if we are doing full block reads or not. + if (*NumBytes % BlockSize != 0) { + TempStatus = NorFlashRead (Instance, Instance->StartLba + Lba, Offset, *NumBytes, Buffer); + if (EFI_ERROR (TempStatus)) { + return EFI_DEVICE_ERROR; + } + } else { + // Read NOR Flash data into shadow buffer + TempStatus = NorFlashReadBlocks (Instance, Instance->StartLba + Lba, BlockSize, Buffer); + if (EFI_ERROR (TempStatus)) { + // Return one of the pre-approved error statuses + return EFI_DEVICE_ERROR; + } } - - // Put the data at the appropriate location inside the buffer area - DEBUG ((DEBUG_BLKIO, "FvbRead: CopyMem( Dst=0x%08x, Src=0x%08x, Size=0x%x ).\n", Buffer, (UINTN)Instance->FvbBuffer + Offset, *NumBytes)); - - CopyMem (Buffer, (VOID*)((UINTN)Instance->FvbBuffer + Offset), *NumBytes); - - return Status; + return EFI_SUCCESS; } /** @@ -537,10 +531,21 @@ FvbWrite ( IN UINT8 *Buffer ) { - EFI_STATUS Status; EFI_STATUS TempStatus; + UINT32 Tmp; + UINT32 TmpBuf; + UINT32 WordToWrite; + UINT32 Mask; + UINTN DoErase; + UINTN BytesToWrite; + UINTN CurOffset; + UINTN WordAddr; UINTN BlockSize; NOR_FLASH_INSTANCE *Instance; + UINTN BlockAddress; + UINTN PrevBlockAddress; + + PrevBlockAddress = 0; Instance = INSTANCE_FROM_FVB_THIS(This); @@ -550,9 +555,6 @@ FvbWrite ( DEBUG ((DEBUG_BLKIO, "FvbWrite(Parameters: Lba=%ld, Offset=0x%x, *NumBytes=0x%x, Buffer @ 0x%08x)\n", Instance->StartLba + Lba, Offset, *NumBytes, Buffer)); - Status = EFI_SUCCESS; - TempStatus = Status; - // Detect WriteDisabled state if (Instance->Media.ReadOnly == TRUE) { DEBUG ((EFI_D_ERROR, "FvbWrite: ERROR - Can not write: Device is in WriteDisabled state.\n")); @@ -578,7 +580,129 @@ FvbWrite ( return EFI_BAD_BUFFER_SIZE; } - // Check we did get some memory + // Pick 128bytes as a good start for word operations as opposed to erasing the + // block and writing the data regardless if an erase is really needed. + // It looks like most individual NV variable writes are smaller than 128bytes. + if (*NumBytes <= 128) { + // Check to see if we need to erase before programming the data into NOR. + // If the destination bits are only changing from 1s to 0s we can just write. + // After a block is erased all bits in the block is set to 1. + // If any byte requires us to erase we just give up and rewrite all of it. + DoErase = 0; + BytesToWrite = *NumBytes; + CurOffset = Offset; + + while (BytesToWrite > 0) { + // Read full word from NOR, splice as required. A word is the smallest + // unit we can write. + TempStatus = NorFlashRead (Instance, Instance->StartLba + Lba, + CurOffset & ~(0x3), sizeof(Tmp), &Tmp); + if (EFI_ERROR (TempStatus)) { + return EFI_DEVICE_ERROR; + } + + // Physical address of word in NOR to write. + WordAddr = (CurOffset & ~(0x3)) + GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress, + Lba, BlockSize); + // The word of data that is to be written. + TmpBuf = *((UINT32*)(Buffer + (*NumBytes - BytesToWrite))); + + // First do word aligned chunks. + if ((CurOffset & 0x3) == 0) { + if (BytesToWrite >= 4) { + // Is the destination still in 'erased' state? + if (~Tmp != 0) { + // Check to see if we are only changing bits to zero. + if ((Tmp ^ TmpBuf) & TmpBuf) { + DoErase = 1; + break; + } + } + // Write this word to NOR + WordToWrite = TmpBuf; + CurOffset += sizeof(TmpBuf); + BytesToWrite -= sizeof(TmpBuf); + } else { + // BytesToWrite < 4. Do small writes and left-overs + Mask = ~((~0) << (BytesToWrite * 8)); + // Mask out the bytes we want. + TmpBuf &= Mask; + // Is the destination still in 'erased' state? + if ((Tmp & Mask) != Mask) { + // Check to see if we are only changing bits to zero. + if ((Tmp ^ TmpBuf) & TmpBuf) { + DoErase = 1; + break; + } + } + // Merge old and new data. Write merged word to NOR + WordToWrite = (Tmp & ~Mask) | TmpBuf; + CurOffset += BytesToWrite; + BytesToWrite = 0; + } + } else { + // Do multiple words, but starting unaligned. + if (BytesToWrite > (4 - (CurOffset & 0x3))) { + Mask = ~((~0) << ((CurOffset & 0x3) * 8)); + // Mask out the bytes we want. + TmpBuf = (TmpBuf << ((CurOffset & 0x3) * 8)) & Mask; + // Is the destination still in 'erased' state? + if ((Tmp & Mask) != Mask) { + // Check to see if we are only changing bits to zero. + if ((Tmp ^ TmpBuf) & TmpBuf) { + DoErase = 1; + break; + } + } + // Merge old and new data. Write merged word to NOR + WordToWrite = (Tmp & ~Mask) | TmpBuf; + BytesToWrite -= (4 - (CurOffset & 0x3)); + CurOffset += (4 - (CurOffset & 0x3)); + } else { + // Unaligned and fits in one word. + Mask = (~((~0) << (BytesToWrite * 8))) << ((CurOffset & 0x3) * 8); + // Mask out the bytes we want. + TmpBuf = (TmpBuf << ((CurOffset & 0x3) * 8)) & Mask; + // Is the destination still in 'erased' state? + if ((Tmp & Mask) != Mask) { + // Check to see if we are only changing bits to zero. + if ((Tmp ^ TmpBuf) & TmpBuf) { + DoErase = 1; + break; + } + } + // Merge old and new data. Write merged word to NOR + WordToWrite = (Tmp & ~Mask) | TmpBuf; + CurOffset += BytesToWrite; + BytesToWrite = 0; + } + } + + // + // Write the word to NOR. + // + + BlockAddress = GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress, Lba, BlockSize); + if (BlockAddress != PrevBlockAddress) { + TempStatus = NorFlashUnlockSingleBlockIfNecessary (Instance, BlockAddress); + if (EFI_ERROR (TempStatus)) { + return EFI_DEVICE_ERROR; + } + PrevBlockAddress = BlockAddress; + } + TempStatus = NorFlashWriteSingleWord (Instance, WordAddr, WordToWrite); + if (EFI_ERROR (TempStatus)) { + return EFI_DEVICE_ERROR; + } + } + // Exit if we got here and could write all the data. Otherwise do the + // Erase-Write cycle. + if (!DoErase) { + return EFI_SUCCESS; + } + } + + // Check we did get some memory. Buffer is BlockSize. if (Instance->FvbBuffer == NULL) { DEBUG ((EFI_D_ERROR, "FvbWrite: ERROR - Buffer not ready\n")); return EFI_DEVICE_ERROR; @@ -601,7 +725,7 @@ FvbWrite ( return EFI_DEVICE_ERROR; } - return Status; + return EFI_SUCCESS; } /** -- 2.39.2