[edk2-devel] [PATCH v1 08/11] ArmVirtPkg: Add Kvmtool NOR flash lib
Philippe Mathieu-Daudé
philmd at redhat.com
Thu Jun 4 06:30:38 UTC 2020
On 5/27/20 1:59 PM, Philippe Mathieu-Daudé wrote:
> Hi Sami,
>
> On 5/14/20 10:40 AM, Sami Mujawar wrote:
>> Kvmtool places the base address of the CFI flash in
>> the device tree it passes to UEFI. This library
>> parses the kvmtool device tree to read the CFI base
>> address and initialise the PCDs use by the NOR flash
>> driver and the variable storage.
>>
>> Signed-off-by: Sami Mujawar <sami.mujawar at arm.com>
>> ---
>> ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c | 265 ++++++++++++++++++++
>> ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf | 50 ++++
>> 2 files changed, 315 insertions(+)
>>
>> diff --git a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..2e43c2e21bc9ef7dd1dd198eebbd70c3b0b96d1c
>> --- /dev/null
>> +++ b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c
>> @@ -0,0 +1,265 @@
>> +/** @file
>> + An instance of the NorFlashPlatformLib for Kvmtool platform.
>> +
>> + Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
>> +
>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> + **/
>> +
>> +#include <Library/BaseLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/NorFlashPlatformLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Protocol/FdtClient.h>
>> +
>> +/** Macro defining the maximum number of Flash Banks.
>> + */
>> +#define MAX_FLASH_BANKS 4
>> +
>> +STATIC NOR_FLASH_DESCRIPTION mNorFlashDevices[MAX_FLASH_BANKS];
>
> This is confuse, the macro define 4 banks to a flash device, but then
> you declare an array of 4 flash devices.
>
> I'm even more confused because I'm only aware of 2 devices on the Virt
> machine. What am I missing?
Pinging again in case this question has been missed.
>
>> +STATIC UINTN mNorFlashDeviceCount = 0;
>> +
>> +/** This function performs platform specific actions to initialise
>> + the NOR flash, if required.
>> +
>> + @retval EFI_SUCCESS Success.
>> +**/
>> +EFI_STATUS
>> +NorFlashPlatformInitialization (
>> + VOID
>> + )
>> +{
>> + DEBUG ((DEBUG_INFO, "NorFlashPlatformInitialization\n"));
>> + // Nothing to do here
>> + return EFI_SUCCESS;
>> +}
>> +
>> +/** Initialise Non volatile Flash storage variables.
>> +
>> + @param [in] FlashDevice Pointer to the NOR Flash device.
>> +
>> + @retval EFI_SUCCESS Success.
>> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
>> + @retval EFI_OUT_OF_RESOURCES Insufficient flash storage space.
>> +**/
>> +EFI_STATUS
>> +SetupVariableStore (
>> + IN NOR_FLASH_DESCRIPTION * FlashDevice
>> + )
>> +{
>> + UINTN FlashRegion;
>> + UINTN FlashNvStorageVariableBase;
>> + UINTN FlashNvStorageFtwWorkingBase;
>> + UINTN FlashNvStorageFtwSpareBase;
>> + UINTN FlashNvStorageVariableSize;
>> + UINTN FlashNvStorageFtwWorkingSize;
>> + UINTN FlashNvStorageFtwSpareSize;
>> +
>> + FlashNvStorageVariableSize = PcdGet32 (PcdFlashNvStorageVariableSize);
>> + FlashNvStorageFtwWorkingSize = PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
>> + FlashNvStorageFtwSpareSize = PcdGet32 (PcdFlashNvStorageFtwSpareSize);
>> +
>> + if ((FlashNvStorageVariableSize == 0) ||
>> + (FlashNvStorageFtwWorkingSize == 0) ||
>> + (FlashNvStorageFtwSpareSize == 0)) {
>> + DEBUG ((DEBUG_ERROR, "FlashNvStorage size not defined\n"));
>> + return EFI_INVALID_PARAMETER;
>> + }
>> +
>> + // Setup the variable store
>> + FlashRegion = FlashDevice->DeviceBaseAddress;
>> +
>> + FlashNvStorageVariableBase = FlashRegion;
>> + FlashRegion += PcdGet32 (PcdFlashNvStorageVariableSize);
>> +
>> + FlashNvStorageFtwWorkingBase = FlashRegion;
>> + FlashRegion += PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
>> +
>> + FlashNvStorageFtwSpareBase = FlashRegion;
>> + FlashRegion += PcdGet32 (PcdFlashNvStorageFtwSpareSize);
>> +
>> + if (FlashRegion > (FlashDevice->DeviceBaseAddress + FlashDevice->Size)) {
>> + DEBUG ((DEBUG_ERROR, "Insufficient flash storage size\n"));
>> + return EFI_OUT_OF_RESOURCES;
>> + }
>> +
>> + PcdSet32S (
>> + PcdFlashNvStorageVariableBase,
>> + FlashNvStorageVariableBase
>> + );
>> +
>> + PcdSet32S (
>> + PcdFlashNvStorageFtwWorkingBase,
>> + FlashNvStorageFtwWorkingBase
>> + );
>> +
>> + PcdSet32S (
>> + PcdFlashNvStorageFtwSpareBase,
>> + FlashNvStorageFtwSpareBase
>> + );
>> +
>> + DEBUG ((
>> + DEBUG_INFO,
>> + "PcdFlashNvStorageVariableBase = 0x%x\n",
>> + FlashNvStorageVariableBase
>> + ));
>> + DEBUG ((
>> + DEBUG_INFO,
>> + "PcdFlashNvStorageVariableSize = 0x%x\n",
>> + FlashNvStorageVariableSize
>> + ));
>> + DEBUG ((
>> + DEBUG_INFO,
>> + "PcdFlashNvStorageFtwWorkingBase = 0x%x\n",
>> + FlashNvStorageFtwWorkingBase
>> + ));
>> + DEBUG ((
>> + DEBUG_INFO,
>> + "PcdFlashNvStorageFtwWorkingSize = 0x%x\n",
>> + FlashNvStorageFtwWorkingSize
>> + ));
>> + DEBUG ((
>> + DEBUG_INFO,
>> + "PcdFlashNvStorageFtwSpareBase = 0x%x\n",
>> + FlashNvStorageFtwSpareBase
>> + ));
>> + DEBUG ((
>> + DEBUG_INFO,
>> + "PcdFlashNvStorageFtwSpareSize = 0x%x\n",
>> + FlashNvStorageFtwSpareSize
>> + ));
>> +
>> + return EFI_SUCCESS;
>> +}
>> +
>> +/** Return the Flash devices on the platform.
>> +
>> + @param [out] NorFlashDescriptions Pointer to the Flash device description.
>> + @param [out] Count Number of Flash devices.
>> +
>> + @retval EFI_SUCCESS Success.
>> + @retval EFI_NOT_FOUND Flash device not found.
>> +**/
>> +EFI_STATUS
>> +NorFlashPlatformGetDevices (
>> + OUT NOR_FLASH_DESCRIPTION **NorFlashDescriptions,
>> + OUT UINT32 *Count
>> + )
>> +{
>> + if (mNorFlashDeviceCount > 0) {
>> + *NorFlashDescriptions = mNorFlashDevices;
>> + *Count = mNorFlashDeviceCount;
>> + return EFI_SUCCESS;
>> + }
>> + return EFI_NOT_FOUND;
>> +}
>> +
>> +/** Entrypoint for NorFlashPlatformLib.
>> +
>> + @param [in] ImageHandle The handle to the image.
>> + @param [in] SystemTable Pointer to the System Table.
>> +
>> + @retval EFI_SUCCESS Success.
>> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
>> + @retval EFI_NOT_FOUND Flash device not found.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +NorFlashPlatformLibConstructor (
>> + IN EFI_HANDLE ImageHandle,
>> + IN EFI_SYSTEM_TABLE * SystemTable
>> + )
>> +{
>> + FDT_CLIENT_PROTOCOL *FdtClient;
>> + INT32 Node;
>> + EFI_STATUS Status;
>> + EFI_STATUS FindNodeStatus;
>> + CONST UINT32 *Reg;
>> + UINT32 PropSize;
>> + UINT64 Base;
>> + UINT64 Size;
>> +
>> + if (mNorFlashDeviceCount != 0) {
>> + return EFI_SUCCESS;
>> + }
>> +
>> + Status = gBS->LocateProtocol (
>> + &gFdtClientProtocolGuid,
>> + NULL,
>> + (VOID **)&FdtClient
>> + );
>> + if (EFI_ERROR (Status)) {
>> + ASSERT_EFI_ERROR (Status);
>> + return Status;
>> + }
>> +
>> + for (FindNodeStatus = FdtClient->FindCompatibleNode (
>> + FdtClient,
>> + "cfi-flash",
>> + &Node
>> + );
>> + !EFI_ERROR (FindNodeStatus) && (mNorFlashDeviceCount < MAX_FLASH_BANKS);
>> + FindNodeStatus = FdtClient->FindNextCompatibleNode (
>> + FdtClient,
>> + "cfi-flash",
>> + Node,
>> + &Node
>> + )) {
>> + Status = FdtClient->GetNodeProperty (
>> + FdtClient,
>> + Node,
>> + "reg",
>> + (CONST VOID **)&Reg,
>> + &PropSize
>> + );
>> + if (EFI_ERROR (Status)) {
>> + DEBUG ((DEBUG_ERROR, "%a: GetNodeProperty () failed (Status == %r)\n",
>> + __FUNCTION__, Status));
>> + continue;
>> + }
>> +
>> + ASSERT ((PropSize % (4 * sizeof (UINT32))) == 0);
>> +
>> + while ((PropSize >= (4 * sizeof (UINT32))) &&
>> + (mNorFlashDeviceCount < MAX_FLASH_BANKS)) {
>> + Base = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[0]));
>> + Size = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[2]));
>> + Reg += 4;
>> +
>> + PropSize -= 4 * sizeof (UINT32);
>> +
>> + //
>> + // Disregard any flash devices that overlap with the primary FV.
>> + // The firmware is not updatable from inside the guest anyway.
>> + //
>> + if ((PcdGet64 (PcdFvBaseAddress) + PcdGet32 (PcdFvSize) > Base) &&
>> + (Base + Size) > PcdGet64 (PcdFvBaseAddress)) {
>> + continue;
>> + }
>> +
>> + DEBUG ((
>> + DEBUG_INFO,
>> + "NOR%d : Base = 0x%lx, Size = 0x%lx\n",
>> + mNorFlashDeviceCount,
>> + Base,
>> + Size
>> + ));
>> +
>> + mNorFlashDevices[mNorFlashDeviceCount].DeviceBaseAddress = (UINTN)Base;
>> + mNorFlashDevices[mNorFlashDeviceCount].RegionBaseAddress = (UINTN)Base;
>> + mNorFlashDevices[mNorFlashDeviceCount].Size = (UINTN)Size;
>> + mNorFlashDevices[mNorFlashDeviceCount].BlockSize = SIZE_256KB;
>
> Hmm I'm worried that there is no contract enforcing the Virt machine to
> use 256KiB sectors. Can we add a definition elsewhere and use it here,
> instead of burying the fixed sector size here?
>
>> + mNorFlashDeviceCount++;
>> + }
>> + }
>> +
>> + // Setup the variable store in the last bank
>> + if ((mNorFlashDeviceCount > 0) &&
>> + (mNorFlashDevices[mNorFlashDeviceCount - 1].DeviceBaseAddress != 0)) {
>> + return SetupVariableStore (&mNorFlashDevices[mNorFlashDeviceCount - 1]);
>> + }
>> +
>> + return EFI_NOT_FOUND;
>> +}
>> +
>> diff --git a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..8bd6f730dcb52e597b418e59766c1566a9519789
>> --- /dev/null
>> +++ b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
>> @@ -0,0 +1,50 @@
>> +#/** @file
>> +#
>> +# Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
>> +# SPDX-License-Identifier: BSD-2-Clause-Patent
>> +#
>> +#**/
>> +
>> +[Defines]
>> + INF_VERSION = 0x0001001B
>> + BASE_NAME = NorFlashKvmtoolLib
>> + FILE_GUID = E75F07A1-B160-4893-BDD4-09E32FF847DC
>> + MODULE_TYPE = DXE_DRIVER
>> + VERSION_STRING = 1.0
>> + LIBRARY_CLASS = NorFlashPlatformLib
>> + CONSTRUCTOR = NorFlashPlatformLibConstructor
>> +
>> +[Sources.common]
>> + NorFlashKvmtool.c
>> +
>> +[Packages]
>> + ArmPkg/ArmPkg.dec
>> + ArmPlatformPkg/ArmPlatformPkg.dec
>> + ArmVirtPkg/ArmVirtPkg.dec
>> + MdePkg/MdePkg.dec
>> + MdeModulePkg/MdeModulePkg.dec
>> +
>> +[LibraryClasses]
>> + BaseLib
>> + DebugLib
>> + PcdLib
>> + UefiBootServicesTableLib
>> +
>> +[Protocols]
>> + gFdtClientProtocolGuid ## CONSUMES
>> +
>> +[Pcd]
>> + gArmTokenSpaceGuid.PcdFvBaseAddress
>> + gArmTokenSpaceGuid.PcdFvSize
>> +
>> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
>> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
>> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
>> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
>> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
>> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
>> +
>> +
>> +[Depex]
>> + gFdtClientProtocolGuid
>> +
>>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#60713): https://edk2.groups.io/g/devel/message/60713
Mute This Topic: https://groups.io/mt/74200914/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list