From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web10.10923.1589458657860229005 for ; Thu, 14 May 2020 05:17:38 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=hFxjv2xE; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1589458657; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=aE0+aZb3w+tcJ5Tvnf9qOOxgcDU29jIw5pFly5ZxYk8=; b=hFxjv2xE6zgyVFvFD/oc3ayfbV1y4bcrCEe9ChEe0MbxrVCbSxNB+1D0y6B9BOkMBPYgmV JbS89xN9/GpcElUHtrg8qU6Z7S+jjfqz/kSH4pZhczVZBmOL1qp4jDIjwS7KvKqEHy9ChX RyPR7i+lmSZTQUuZCZXnQ4MWAJEVivE= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-410-2cYspk5LNGOF4fJnLSFlyQ-1; Thu, 14 May 2020 08:17:27 -0400 X-MC-Unique: 2cYspk5LNGOF4fJnLSFlyQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C115B1902EA2; Thu, 14 May 2020 12:17:25 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-179.ams2.redhat.com [10.36.113.179]) by smtp.corp.redhat.com (Postfix) with ESMTP id 78E895C1BE; Thu, 14 May 2020 12:17:23 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v1 08/11] ArmVirtPkg: Add Kvmtool NOR flash lib To: devel@edk2.groups.io, ard.biesheuvel@arm.com, Sami Mujawar Cc: leif@nuviainc.com, Alexandru.Elisei@arm.com, Andre.Przywara@arm.com, Matteo.Carlini@arm.com, Laura.Moretta@arm.com, nd@arm.com References: <20200514084019.71368-1-sami.mujawar@arm.com> <20200514084019.71368-9-sami.mujawar@arm.com> From: "Laszlo Ersek" Message-ID: <2537ff88-0628-a1e2-32b2-0d30151a1a9d@redhat.com> Date: Thu, 14 May 2020 14:17:22 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit On 05/14/20 11:32, Ard Biesheuvel wrote: > 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 >> --- >>   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.
>> + >> + SPDX-License-Identifier: BSD-2-Clause-Patent >> + >> + **/ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +/** Macro defining the maximum number of Flash Banks. >> + */ >> +#define MAX_FLASH_BANKS       4 >> + >> +STATIC NOR_FLASH_DESCRIPTION  mNorFlashDevices[MAX_FLASH_BANKS]; >> +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; >> +  } >> + > > An ASSERT is sufficient here - the DEPEX ensures that this is guaranteed > to succeed. with that: Acked-by: Laszlo Ersek Thanks Laszlo > >> +  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; >> +      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.
>> +#  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 >> + >> > > > >