public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Sami Mujawar" <sami.mujawar@arm.com>
To: <devel@edk2.groups.io>
Cc: Sami Mujawar <sami.mujawar@arm.com>, <ard.biesheuvel@arm.com>,
	<leif@nuviainc.com>, <philmd@redhat.com>, <lersek@redhat.com>,
	<Alexandru.Elisei@arm.com>, <Andre.Przywara@arm.com>,
	<Matteo.Carlini@arm.com>, <Ben.Adderson@arm.com>, <nd@arm.com>
Subject: [PATCH v5 06/15] ArmVirtPkg: Add Kvmtool NOR flash lib
Date: Fri, 2 Oct 2020 22:14:00 +0100	[thread overview]
Message-ID: <20201002211409.43888-7-sami.mujawar@arm.com> (raw)
In-Reply-To: <20201002211409.43888-1-sami.mujawar@arm.com>

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.

UEFI takes ownership of the CFI flash hardware, and
exposes its functionality through the UEFI Runtime
Variable Service. Therefore, disable the device tree
node for the CFI flash used for storing the UEFI
variables, to prevent the OS from attaching its device
driver as well.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---

Notes:
    v5:
     - Fixed minor ECC reported issues in file header.            [Sami]
       Ref: https://edk2.groups.io/g/devel/message/62160
    
    v4:
      - Added STATIC to local functions and updated comments      [Sami]
        to explain why DT node deferred to be disabled in
        NorFlashPlatformInitialization()
      - Use STATIC for local functions and explain why DT         [Ard]
        node for flash is disabled in
        NorFlashPlatformInitialization()?
        Ref: https://edk2.groups.io/g/devel/topic/75081477
    
    v3:
      - ASSERT is sufficient to test Locating                     [Ard]
        gFdtClientProtocolGuid as DEPEX ensures that this is
        guaranteed to succeed.
      - Removed additional error handling based on review         [Sami]
        feedback.
      - Fix confusion caused by use of macro MAX_FLASH_BANKS.     [Philippe]
      - Renamed MAX_FLASH_BANKS to MAX_FLASH_DEVICES.             [Sami]
      - Use macro to define block size for flash.                 [Philippe]
      - Defined macro KVMTOOL_NOR_BLOCK_SIZE and also configured  [Sami]
        to reflect the correct block size 64KB.
      - Disable the DT flash node used for UEFI variable storage  [Sami]
        as UEFI takes ownership of the flash device.
        Ref: https://edk2.groups.io/g/devel/topic/74200914#60341
    
    v2:
      - Library to read CFI flash base address from DT and initialise [Sami]
        PCDs used for NOR flash variables.

 ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c      | 336 ++++++++++++++++++++
 ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf |  50 +++
 2 files changed, 386 insertions(+)

diff --git a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c
new file mode 100644
index 0000000000000000000000000000000000000000..dce585b779a87dabde280cf4bbca5b828ceccda4
--- /dev/null
+++ b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c
@@ -0,0 +1,336 @@
+/** @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 NOR block size configured in Kvmtool.
+*/
+#define KVMTOOL_NOR_BLOCK_SIZE  SIZE_64KB
+
+/** Macro defining the maximum number of Flash devices.
+*/
+#define MAX_FLASH_DEVICES       4
+
+/** Macro defining the cfi-flash label describing the UEFI variable store.
+*/
+#define LABEL_UEFI_VAR_STORE    "System-firmware"
+
+STATIC NOR_FLASH_DESCRIPTION  mNorFlashDevices[MAX_FLASH_DEVICES];
+STATIC UINTN                  mNorFlashDeviceCount = 0;
+STATIC INT32                  mUefiVarStoreNode = MAX_INT32;
+STATIC FDT_CLIENT_PROTOCOL    *mFdtClient;
+
+/** This function performs platform specific actions to initialise
+    the NOR flash, if required.
+
+  @retval EFI_SUCCESS           Success.
+**/
+EFI_STATUS
+NorFlashPlatformInitialization (
+  VOID
+  )
+{
+  EFI_STATUS                  Status;
+
+  DEBUG ((DEBUG_INFO, "NorFlashPlatformInitialization\n"));
+
+  if ((mNorFlashDeviceCount > 0) && (mUefiVarStoreNode != MAX_INT32)) {
+    //
+    // UEFI takes ownership of the cfi-flash hardware, and exposes its
+    // functionality through the UEFI Runtime Variable Service. This means we
+    // need to disable it in the device tree to prevent the OS from attaching
+    // its device driver as well.
+    // Note: This library is loaded twice. First by FaultTolerantWriteDxe to
+    // setup the PcdFlashNvStorageFtw* and later by NorFlashDxe to provide the
+    // NorFlashPlatformLib interfaces. If the node is disabled when the library
+    // is first loaded, then during the subsequent loading of the library the
+    // call to FindNextCompatibleNode() from the library constructor skips the
+    // FDT node used for UEFI storage variable. Due to this we cannot setup the
+    // NOR flash device description i.e. mNorFlashDevices[].
+    // Since NorFlashPlatformInitialization() is called only by NorFlashDxe,
+    // we know it is safe to disable the node here.
+    //
+    Status = mFdtClient->SetNodeProperty (
+                           mFdtClient,
+                           mUefiVarStoreNode,
+                           "status",
+                           "disabled",
+                           sizeof ("disabled")
+                           );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_WARN, "Failed to set cfi-flash status to 'disabled'\n"));
+    }
+  } else {
+    Status = EFI_NOT_FOUND;
+    DEBUG ((DEBUG_ERROR, "Flash device for UEFI variable storage not found\n"));
+  }
+
+  return Status;
+}
+
+/** 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.
+**/
+STATIC
+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
+  )
+{
+  INT32                       Node;
+  EFI_STATUS                  Status;
+  EFI_STATUS                  FindNodeStatus;
+  CONST UINT32                *Reg;
+  UINT32                      PropSize;
+  UINT64                      Base;
+  UINT64                      Size;
+  UINTN                       UefiVarStoreIndex;
+  CONST CHAR8                 *Label;
+  UINT32                      LabelLen;
+
+  if (mNorFlashDeviceCount != 0) {
+    return EFI_SUCCESS;
+  }
+
+  Status = gBS->LocateProtocol (
+                  &gFdtClientProtocolGuid,
+                  NULL,
+                  (VOID **)&mFdtClient
+                  );
+  ASSERT_EFI_ERROR (Status);
+
+  UefiVarStoreIndex = MAX_UINTN;
+  for (FindNodeStatus = mFdtClient->FindCompatibleNode (
+                                      mFdtClient,
+                                      "cfi-flash",
+                                      &Node
+                                      );
+       !EFI_ERROR (FindNodeStatus) &&
+         (mNorFlashDeviceCount < MAX_FLASH_DEVICES);
+       FindNodeStatus = mFdtClient->FindNextCompatibleNode (
+                                      mFdtClient,
+                                      "cfi-flash",
+                                      Node,
+                                      &Node
+    )) {
+    Status = mFdtClient->GetNodeProperty (
+                           mFdtClient,
+                           Node,
+                           "label",
+                           (CONST VOID **)&Label,
+                           &LabelLen
+                           );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "%a: GetNodeProperty ('label') failed (Status == %r)\n",
+        __FUNCTION__,
+        Status
+        ));
+    } else if (AsciiStrCmp (Label, LABEL_UEFI_VAR_STORE) == 0) {
+      UefiVarStoreIndex = mNorFlashDeviceCount;
+      mUefiVarStoreNode = Node;
+    }
+
+    Status = mFdtClient->GetNodeProperty (
+                           mFdtClient,
+                           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_DEVICES)) {
+      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 = KVMTOOL_NOR_BLOCK_SIZE;
+      mNorFlashDeviceCount++;
+    }
+  } // for
+
+  // Setup the variable store in the last device
+  if (mNorFlashDeviceCount > 0) {
+    if (UefiVarStoreIndex == MAX_UINTN) {
+      // We did not find a label matching the UEFI Variable store. Default to
+      // using the last cfi-flash device as the variable store.
+      UefiVarStoreIndex = mNorFlashDeviceCount - 1;
+      mUefiVarStoreNode = Node;
+    }
+    if (mNorFlashDevices[UefiVarStoreIndex].DeviceBaseAddress != 0) {
+      return SetupVariableStore (&mNorFlashDevices[UefiVarStoreIndex]);
+    }
+  }
+
+  return EFI_NOT_FOUND;
+}
+
diff --git a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
new file mode 100644
index 0000000000000000000000000000000000000000..b811e99156e2d55a95b4941e927a1006eb18a610
--- /dev/null
+++ b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
@@ -0,0 +1,50 @@
+## @file
+#  Nor Flash library for Kvmtool.
+#
+#  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
+
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


  parent reply	other threads:[~2020-10-02 21:14 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-02 21:13 [PATCH v5 00/15] Kvmtool guest firmware support for Arm Sami Mujawar
2020-10-02 21:13 ` [PATCH v5 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver Sami Mujawar
2020-10-05 10:58   ` Ard Biesheuvel
2020-10-06 10:16     ` Laszlo Ersek
2020-10-15 14:35       ` Ard Biesheuvel
2020-10-02 21:13 ` [PATCH v5 02/15] ArmVirtPkg: Add Kvmtool RTC Fdt Client Library Sami Mujawar
2020-10-02 21:13 ` [PATCH v5 03/15] ArmPlatformPkg: Dynamic flash variable base Sami Mujawar
2020-10-02 21:13 ` [PATCH v5 04/15] ArmVirtPkg: Add kvmtool platform driver Sami Mujawar
2020-10-02 21:13 ` [PATCH v5 05/15] ArmVirtPkg: kvmtool platform memory map Sami Mujawar
2020-10-02 21:14 ` Sami Mujawar [this message]
2020-10-02 21:14 ` [PATCH v5 07/15] MdeModulePkg: Fix constructor invocation ordering Sami Mujawar
2020-10-05  4:35   ` Wu, Hao A
2020-10-02 21:14 ` [PATCH v5 08/15] ArmVirtPkg: GUID Hob for 16550 UART base address Sami Mujawar
2020-10-02 21:14 ` [PATCH v5 09/15] ArmVirtPkg: 16550 UART Platform hook library Sami Mujawar
2020-10-02 21:14 ` [PATCH v5 10/15] ArmVirtPkg: Add Kvmtool Platform Pei Lib Sami Mujawar
2020-10-02 21:14 ` [PATCH v5 11/15] ArmVirtPkg: Support for kvmtool virtual platform Sami Mujawar
2020-10-02 21:14 ` [PATCH v5 12/15] ArmVirtPkg: Package dependency for MC146818 RTC Sami Mujawar
2020-10-02 21:14 ` [PATCH v5 13/15] ArmVirtPkg: Add kvmtool to package dictionary Sami Mujawar
2020-10-02 21:14 ` [PATCH v5 14/15] .python/SpellCheck: Add 'XIPFLAGS' to "words" section Sami Mujawar
2020-10-05 20:51   ` [EXTERNAL] [edk2-devel] " Bret Barkelew
2020-10-06  7:58     ` Sami Mujawar
2020-10-06 18:18       ` Bret Barkelew
2020-10-02 21:14 ` [PATCH v5 15/15] Maintainer.txt: Add Kvmtool platform reviewer Sami Mujawar
2020-10-16 17:27 ` [PATCH v5 00/15] Kvmtool guest firmware support for Arm Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201002211409.43888-7-sami.mujawar@arm.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox