[edk2-devel] [edk2-staging/RiscV64QemuVirt PATCH 25/29] OvmfPkg: Add NorFlashQemuLib library

Sunil V L posted 29 patches 3 years, 4 months ago
There is a newer version of this series
[edk2-devel] [edk2-staging/RiscV64QemuVirt PATCH 25/29] OvmfPkg: Add NorFlashQemuLib library
Posted by Sunil V L 3 years, 4 months ago
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4076

This is copied from ArmVirtPkg since it is required for
other architectures also.

It also adds the instance for single flash drive which has
both code and variables. This is copied from SbsaQemu.

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel Schaefer <git@danielschaefer.me>
Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
---
 OvmfPkg/OvmfPkg.dec                           |   8 ++
 .../NorFlashQemuLib/NorFlashQemuLib.inf       |  40 ++++++
 .../NorFlashQemuUnifiedLib.inf                |  30 ++++
 OvmfPkg/Include/Library/NorFlashPlatformLib.h |  30 ++++
 .../Library/NorFlashQemuLib/NorFlashQemuLib.c | 136 ++++++++++++++++++
 .../NorFlashQemuLib/NorFlashQemuUnifiedLib.c  |  40 ++++++
 6 files changed, 284 insertions(+)
 create mode 100644 OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
 create mode 100644 OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.inf
 create mode 100644 OvmfPkg/Include/Library/NorFlashPlatformLib.h
 create mode 100644 OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
 create mode 100644 OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.c

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 7d2acc5ea0e0..0697c91c6836 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -129,6 +129,10 @@ [LibraryClasses]
   #
   HardwareInfoLib|Include/Library/HardwareInfoLib.h
 
+  ##  @libraryclass  NorFlashQemuLib
+  #
+  NorFlashPlatformLib|Include/Library/NorFlashPlatformLib.h
+
 [Guids]
   gUefiOvmfPkgTokenSpaceGuid            = {0x93bb96af, 0xb9f2, 0x4eb8, {0x94, 0x62, 0xe0, 0xba, 0x74, 0x56, 0x42, 0x36}}
   gEfiXenInfoGuid                       = {0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d}}
@@ -405,6 +409,10 @@ [PcdsFixedAtBuild]
   #  check to decide whether to abort dispatch of the driver it is linked into.
   gUefiOvmfPkgTokenSpaceGuid.PcdEntryPointOverrideFwCfgVarName|""|VOID*|0x68
 
+  ## The base address and size of the FVMAIN
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFvBaseAddress|0|UINT64|0x71
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFvSize|0|UINT32|0x72
+
 [PcdsDynamic, PcdsDynamicEx]
   gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10
diff --git a/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
new file mode 100644
index 000000000000..ecd8059b3508
--- /dev/null
+++ b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
@@ -0,0 +1,40 @@
+#/** @file
+#
+#  Component description file for NorFlashQemuLib module
+#
+#  Copyright (c) 2014, Linaro Ltd. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = NorFlashQemuLib
+  FILE_GUID                      = 42C30D8E-BFAD-4E77-9041-E7DAAE88DF7A
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = NorFlashPlatformLib
+
+[Sources.common]
+  NorFlashQemuLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  UefiBootServicesTableLib
+
+[Protocols]
+  gFdtClientProtocolGuid          ## CONSUMES
+
+[Depex]
+  gFdtClientProtocolGuid
+
+[Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFvBaseAddress
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFvSize
diff --git a/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.inf b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.inf
new file mode 100644
index 000000000000..91d1406fc3e7
--- /dev/null
+++ b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.inf
@@ -0,0 +1,30 @@
+#/** @file
+#
+#  Component description file for NorFlashQemuLib module
+#
+#  Copyright (c) 2014, Linaro Ltd. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = NorFlashQemuUnifiedLib
+  FILE_GUID                      = 064742F1-E531-4D7D-A154-22315889CC23
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = NorFlashPlatformLib
+
+[Sources.common]
+  NorFlashQemuUnifiedLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
diff --git a/OvmfPkg/Include/Library/NorFlashPlatformLib.h b/OvmfPkg/Include/Library/NorFlashPlatformLib.h
new file mode 100644
index 000000000000..6ef5b70e9948
--- /dev/null
+++ b/OvmfPkg/Include/Library/NorFlashPlatformLib.h
@@ -0,0 +1,30 @@
+/** @file
+
+ Copyright (c) 2011-2012, ARM Ltd. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ **/
+
+#ifndef _NORFLASHPLATFORMLIB_H_
+#define _NORFLASHPLATFORMLIB_H_
+
+typedef struct {
+  UINTN    DeviceBaseAddress;       // Start address of the Device Base Address (DBA)
+  UINTN    RegionBaseAddress;       // Start address of one single region
+  UINTN    Size;
+  UINTN    BlockSize;
+} NOR_FLASH_DESCRIPTION;
+
+EFI_STATUS
+NorFlashPlatformInitialization (
+  VOID
+  );
+
+EFI_STATUS
+NorFlashPlatformGetDevices (
+  OUT NOR_FLASH_DESCRIPTION  **NorFlashDescriptions,
+  OUT UINT32                 *Count
+  );
+
+#endif /* _NORFLASHPLATFORMLIB_H_ */
diff --git a/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
new file mode 100644
index 000000000000..3632fa9e7a98
--- /dev/null
+++ b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
@@ -0,0 +1,136 @@
+/** @file
+
+ Copyright (c) 2014-2018, Linaro 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>
+
+#define QEMU_NOR_BLOCK_SIZE  SIZE_256KB
+
+#define MAX_FLASH_BANKS  4
+
+EFI_STATUS
+NorFlashPlatformInitialization (
+  VOID
+  )
+{
+  return EFI_SUCCESS;
+}
+
+NOR_FLASH_DESCRIPTION  mNorFlashDevices[MAX_FLASH_BANKS];
+
+EFI_STATUS
+NorFlashPlatformGetDevices (
+  OUT NOR_FLASH_DESCRIPTION  **NorFlashDescriptions,
+  OUT UINT32                 *Count
+  )
+{
+  FDT_CLIENT_PROTOCOL  *FdtClient;
+  INT32                Node;
+  EFI_STATUS           Status;
+  EFI_STATUS           FindNodeStatus;
+  CONST UINT32         *Reg;
+  UINT32               PropSize;
+  UINT32               Num;
+  UINT64               Base;
+  UINT64               Size;
+
+  Status = gBS->LocateProtocol (
+                  &gFdtClientProtocolGuid,
+                  NULL,
+                  (VOID **)&FdtClient
+                  );
+  ASSERT_EFI_ERROR (Status);
+
+  Num = 0;
+  for (FindNodeStatus = FdtClient->FindCompatibleNode (
+                                     FdtClient,
+                                     "cfi-flash",
+                                     &Node
+                                     );
+       !EFI_ERROR (FindNodeStatus) && Num < 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)) && Num < 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 (PcdOvmfFvBaseAddress) + PcdGet32 (PcdOvmfFvSize) > Base) &&
+          ((Base + Size) > PcdGet64 (PcdOvmfFvBaseAddress)))
+      {
+        continue;
+      }
+
+      mNorFlashDevices[Num].DeviceBaseAddress = (UINTN)Base;
+      mNorFlashDevices[Num].RegionBaseAddress = (UINTN)Base;
+      mNorFlashDevices[Num].Size              = (UINTN)Size;
+      mNorFlashDevices[Num].BlockSize         = QEMU_NOR_BLOCK_SIZE;
+      Num++;
+    }
+
+    //
+    // UEFI takes ownership of the NOR flash, and exposes its functionality
+    // through the UEFI Runtime Services GetVariable, SetVariable, etc. This
+    // means we need to disable it in the device tree to prevent the OS from
+    // attaching its device driver as well.
+    // Note that this also hides other flash banks, but the only other flash
+    // bank we expect to encounter is the one that carries the UEFI executable
+    // code, which is not intended to be guest updatable, and is usually backed
+    // in a readonly manner by QEMU anyway.
+    //
+    Status = FdtClient->SetNodeProperty (
+                          FdtClient,
+                          Node,
+                          "status",
+                          "disabled",
+                          sizeof ("disabled")
+                          );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_WARN, "Failed to set NOR flash status to 'disabled'\n"));
+    }
+  }
+
+  *NorFlashDescriptions = mNorFlashDevices;
+  *Count                = Num;
+
+  return EFI_SUCCESS;
+}
diff --git a/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.c b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.c
new file mode 100644
index 000000000000..1420fb5b596c
--- /dev/null
+++ b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.c
@@ -0,0 +1,40 @@
+/** @file
+
+ Copyright (c) 2019, Linaro Ltd. All rights reserved
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ **/
+
+#include <Base.h>
+#include <PiDxe.h>
+#include <Library/NorFlashPlatformLib.h>
+
+#define QEMU_NOR_BLOCK_SIZE  SIZE_256KB
+
+EFI_STATUS
+NorFlashPlatformInitialization (
+  VOID
+  )
+{
+  return EFI_SUCCESS;
+}
+
+NOR_FLASH_DESCRIPTION  mNorFlashDevice =
+{
+  FixedPcdGet32 (PcdOvmfFdBaseAddress),
+  FixedPcdGet64 (PcdFlashNvStorageVariableBase),
+  FixedPcdGet32 (PcdOvmfFirmwareFdSize),
+  QEMU_NOR_BLOCK_SIZE
+};
+
+EFI_STATUS
+NorFlashPlatformGetDevices (
+  OUT NOR_FLASH_DESCRIPTION  **NorFlashDescriptions,
+  OUT UINT32                 *Count
+  )
+{
+  *NorFlashDescriptions = &mNorFlashDevice;
+  *Count                = 1;
+  return EFI_SUCCESS;
+}
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94924): https://edk2.groups.io/g/devel/message/94924
Mute This Topic: https://groups.io/mt/94233036/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-staging/RiscV64QemuVirt PATCH 25/29] OvmfPkg: Add NorFlashQemuLib library
Posted by Ard Biesheuvel 3 years, 3 months ago
On Mon, 10 Oct 2022 at 12:13, Sunil V L <sunilvl@ventanamicro.com> wrote:
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4076
>
> This is copied from ArmVirtPkg since it is required for
> other architectures also.
>
> It also adds the instance for single flash drive which has
> both code and variables. This is copied from SbsaQemu.
>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Daniel Schaefer <git@danielschaefer.me>
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>

Let's call these

QemuNorFlashPlatformLib [for the library class]

QemuNorFlashDeviceTreeLib
QemuNorFlashStaticLib

and for the driver

QemuNorFlashDxe


i sent out some patches for edk2-platforms to eliminate the dependency
on ArmPlatformPkg's NorFlashDxe and NorFlashPlatformLib definitions.
Once we move everything in OvmfPkg over to these ones, we can drop the
old one altogether.



> ---
>  OvmfPkg/OvmfPkg.dec                           |   8 ++
>  .../NorFlashQemuLib/NorFlashQemuLib.inf       |  40 ++++++
>  .../NorFlashQemuUnifiedLib.inf                |  30 ++++
>  OvmfPkg/Include/Library/NorFlashPlatformLib.h |  30 ++++
>  .../Library/NorFlashQemuLib/NorFlashQemuLib.c | 136 ++++++++++++++++++
>  .../NorFlashQemuLib/NorFlashQemuUnifiedLib.c  |  40 ++++++
>  6 files changed, 284 insertions(+)
>  create mode 100644 OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
>  create mode 100644 OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.inf
>  create mode 100644 OvmfPkg/Include/Library/NorFlashPlatformLib.h
>  create mode 100644 OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>  create mode 100644 OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.c
>
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index 7d2acc5ea0e0..0697c91c6836 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -129,6 +129,10 @@ [LibraryClasses]
>    #
>    HardwareInfoLib|Include/Library/HardwareInfoLib.h
>
> +  ##  @libraryclass  NorFlashQemuLib
> +  #
> +  NorFlashPlatformLib|Include/Library/NorFlashPlatformLib.h
> +
>  [Guids]
>    gUefiOvmfPkgTokenSpaceGuid            = {0x93bb96af, 0xb9f2, 0x4eb8, {0x94, 0x62, 0xe0, 0xba, 0x74, 0x56, 0x42, 0x36}}
>    gEfiXenInfoGuid                       = {0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d}}
> @@ -405,6 +409,10 @@ [PcdsFixedAtBuild]
>    #  check to decide whether to abort dispatch of the driver it is linked into.
>    gUefiOvmfPkgTokenSpaceGuid.PcdEntryPointOverrideFwCfgVarName|""|VOID*|0x68
>
> +  ## The base address and size of the FVMAIN
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFvBaseAddress|0|UINT64|0x71
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFvSize|0|UINT32|0x72
> +
>  [PcdsDynamic, PcdsDynamicEx]
>    gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10
> diff --git a/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
> new file mode 100644
> index 000000000000..ecd8059b3508
> --- /dev/null
> +++ b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
> @@ -0,0 +1,40 @@
> +#/** @file
> +#
> +#  Component description file for NorFlashQemuLib module
> +#
> +#  Copyright (c) 2014, Linaro Ltd. All rights reserved.<BR>
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = NorFlashQemuLib
> +  FILE_GUID                      = 42C30D8E-BFAD-4E77-9041-E7DAAE88DF7A
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = NorFlashPlatformLib
> +
> +[Sources.common]
> +  NorFlashQemuLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  UefiBootServicesTableLib
> +
> +[Protocols]
> +  gFdtClientProtocolGuid          ## CONSUMES
> +
> +[Depex]
> +  gFdtClientProtocolGuid
> +
> +[Pcd]
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFvBaseAddress
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFvSize
> diff --git a/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.inf b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.inf
> new file mode 100644
> index 000000000000..91d1406fc3e7
> --- /dev/null
> +++ b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.inf
> @@ -0,0 +1,30 @@
> +#/** @file
> +#
> +#  Component description file for NorFlashQemuLib module
> +#
> +#  Copyright (c) 2014, Linaro Ltd. All rights reserved.<BR>
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = NorFlashQemuUnifiedLib
> +  FILE_GUID                      = 064742F1-E531-4D7D-A154-22315889CC23
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = NorFlashPlatformLib
> +
> +[Sources.common]
> +  NorFlashQemuUnifiedLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +
> +[Pcd]
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
> diff --git a/OvmfPkg/Include/Library/NorFlashPlatformLib.h b/OvmfPkg/Include/Library/NorFlashPlatformLib.h
> new file mode 100644
> index 000000000000..6ef5b70e9948
> --- /dev/null
> +++ b/OvmfPkg/Include/Library/NorFlashPlatformLib.h
> @@ -0,0 +1,30 @@
> +/** @file
> +
> + Copyright (c) 2011-2012, ARM Ltd. All rights reserved.<BR>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> + **/
> +
> +#ifndef _NORFLASHPLATFORMLIB_H_
> +#define _NORFLASHPLATFORMLIB_H_
> +
> +typedef struct {
> +  UINTN    DeviceBaseAddress;       // Start address of the Device Base Address (DBA)
> +  UINTN    RegionBaseAddress;       // Start address of one single region
> +  UINTN    Size;
> +  UINTN    BlockSize;
> +} NOR_FLASH_DESCRIPTION;
> +
> +EFI_STATUS
> +NorFlashPlatformInitialization (
> +  VOID
> +  );
> +
> +EFI_STATUS
> +NorFlashPlatformGetDevices (
> +  OUT NOR_FLASH_DESCRIPTION  **NorFlashDescriptions,
> +  OUT UINT32                 *Count
> +  );
> +
> +#endif /* _NORFLASHPLATFORMLIB_H_ */
> diff --git a/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> new file mode 100644
> index 000000000000..3632fa9e7a98
> --- /dev/null
> +++ b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> @@ -0,0 +1,136 @@
> +/** @file
> +
> + Copyright (c) 2014-2018, Linaro 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>
> +
> +#define QEMU_NOR_BLOCK_SIZE  SIZE_256KB
> +
> +#define MAX_FLASH_BANKS  4
> +
> +EFI_STATUS
> +NorFlashPlatformInitialization (
> +  VOID
> +  )
> +{
> +  return EFI_SUCCESS;
> +}
> +
> +NOR_FLASH_DESCRIPTION  mNorFlashDevices[MAX_FLASH_BANKS];
> +
> +EFI_STATUS
> +NorFlashPlatformGetDevices (
> +  OUT NOR_FLASH_DESCRIPTION  **NorFlashDescriptions,
> +  OUT UINT32                 *Count
> +  )
> +{
> +  FDT_CLIENT_PROTOCOL  *FdtClient;
> +  INT32                Node;
> +  EFI_STATUS           Status;
> +  EFI_STATUS           FindNodeStatus;
> +  CONST UINT32         *Reg;
> +  UINT32               PropSize;
> +  UINT32               Num;
> +  UINT64               Base;
> +  UINT64               Size;
> +
> +  Status = gBS->LocateProtocol (
> +                  &gFdtClientProtocolGuid,
> +                  NULL,
> +                  (VOID **)&FdtClient
> +                  );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  Num = 0;
> +  for (FindNodeStatus = FdtClient->FindCompatibleNode (
> +                                     FdtClient,
> +                                     "cfi-flash",
> +                                     &Node
> +                                     );
> +       !EFI_ERROR (FindNodeStatus) && Num < 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)) && Num < 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 (PcdOvmfFvBaseAddress) + PcdGet32 (PcdOvmfFvSize) > Base) &&
> +          ((Base + Size) > PcdGet64 (PcdOvmfFvBaseAddress)))
> +      {
> +        continue;
> +      }
> +
> +      mNorFlashDevices[Num].DeviceBaseAddress = (UINTN)Base;
> +      mNorFlashDevices[Num].RegionBaseAddress = (UINTN)Base;
> +      mNorFlashDevices[Num].Size              = (UINTN)Size;
> +      mNorFlashDevices[Num].BlockSize         = QEMU_NOR_BLOCK_SIZE;
> +      Num++;
> +    }
> +
> +    //
> +    // UEFI takes ownership of the NOR flash, and exposes its functionality
> +    // through the UEFI Runtime Services GetVariable, SetVariable, etc. This
> +    // means we need to disable it in the device tree to prevent the OS from
> +    // attaching its device driver as well.
> +    // Note that this also hides other flash banks, but the only other flash
> +    // bank we expect to encounter is the one that carries the UEFI executable
> +    // code, which is not intended to be guest updatable, and is usually backed
> +    // in a readonly manner by QEMU anyway.
> +    //
> +    Status = FdtClient->SetNodeProperty (
> +                          FdtClient,
> +                          Node,
> +                          "status",
> +                          "disabled",
> +                          sizeof ("disabled")
> +                          );
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_WARN, "Failed to set NOR flash status to 'disabled'\n"));
> +    }
> +  }
> +
> +  *NorFlashDescriptions = mNorFlashDevices;
> +  *Count                = Num;
> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.c b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.c
> new file mode 100644
> index 000000000000..1420fb5b596c
> --- /dev/null
> +++ b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.c
> @@ -0,0 +1,40 @@
> +/** @file
> +
> + Copyright (c) 2019, Linaro Ltd. All rights reserved
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> + **/
> +
> +#include <Base.h>
> +#include <PiDxe.h>
> +#include <Library/NorFlashPlatformLib.h>
> +
> +#define QEMU_NOR_BLOCK_SIZE  SIZE_256KB
> +
> +EFI_STATUS
> +NorFlashPlatformInitialization (
> +  VOID
> +  )
> +{
> +  return EFI_SUCCESS;
> +}
> +
> +NOR_FLASH_DESCRIPTION  mNorFlashDevice =
> +{
> +  FixedPcdGet32 (PcdOvmfFdBaseAddress),
> +  FixedPcdGet64 (PcdFlashNvStorageVariableBase),
> +  FixedPcdGet32 (PcdOvmfFirmwareFdSize),
> +  QEMU_NOR_BLOCK_SIZE
> +};
> +
> +EFI_STATUS
> +NorFlashPlatformGetDevices (
> +  OUT NOR_FLASH_DESCRIPTION  **NorFlashDescriptions,
> +  OUT UINT32                 *Count
> +  )
> +{
> +  *NorFlashDescriptions = &mNorFlashDevice;
> +  *Count                = 1;
> +  return EFI_SUCCESS;
> +}
> --
> 2.25.1
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95389): https://edk2.groups.io/g/devel/message/95389
Mute This Topic: https://groups.io/mt/94233036/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-staging/RiscV64QemuVirt PATCH 25/29] OvmfPkg: Add NorFlashQemuLib library
Posted by Sami Mujawar 3 years, 3 months ago
Hi Ard,

Please see my query inline marked [SAMI].

Regards,

Sami Mujawar

On 19/10/2022 01:19 pm, Ard Biesheuvel via groups.io wrote:
> On Mon, 10 Oct 2022 at 12:13, Sunil V L <sunilvl@ventanamicro.com> wrote:
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4076
>>
>> This is copied from ArmVirtPkg since it is required for
>> other architectures also.
>>
>> It also adds the instance for single flash drive which has
>> both code and variables. This is copied from SbsaQemu.
>>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Daniel Schaefer <git@danielschaefer.me>
>> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> Let's call these
>
> QemuNorFlashPlatformLib [for the library class]
>
> QemuNorFlashDeviceTreeLib
> QemuNorFlashStaticLib
>
> and for the driver
>
> QemuNorFlashDxe

[SAMI] We use the NorFlashDxe for the Kvmtool guest firmware, see 
https://github.com/tianocore/edk2/blob/master/ArmVirtPkg/ArmVirtKvmTool.dsc#L294

Considering this, should QemuNorFlashDxe be called OvmfNorFlashDxe?

[/SAMI]

>
>
> i sent out some patches for edk2-platforms to eliminate the dependency
> on ArmPlatformPkg's NorFlashDxe and NorFlashPlatformLib definitions.
> Once we move everything in OvmfPkg over to these ones, we can drop the
> old one altogether.
>
>
>
>> ---
>>   OvmfPkg/OvmfPkg.dec                           |   8 ++
>>   .../NorFlashQemuLib/NorFlashQemuLib.inf       |  40 ++++++
>>   .../NorFlashQemuUnifiedLib.inf                |  30 ++++
>>   OvmfPkg/Include/Library/NorFlashPlatformLib.h |  30 ++++
>>   .../Library/NorFlashQemuLib/NorFlashQemuLib.c | 136 ++++++++++++++++++
>>   .../NorFlashQemuLib/NorFlashQemuUnifiedLib.c  |  40 ++++++
>>   6 files changed, 284 insertions(+)
>>   create mode 100644 OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
>>   create mode 100644 OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.inf
>>   create mode 100644 OvmfPkg/Include/Library/NorFlashPlatformLib.h
>>   create mode 100644 OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>>   create mode 100644 OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.c
>>
>> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
>> index 7d2acc5ea0e0..0697c91c6836 100644
>> --- a/OvmfPkg/OvmfPkg.dec
>> +++ b/OvmfPkg/OvmfPkg.dec
>> @@ -129,6 +129,10 @@ [LibraryClasses]
>>     #
>>     HardwareInfoLib|Include/Library/HardwareInfoLib.h
>>
>> +  ##  @libraryclass  NorFlashQemuLib
>> +  #
>> +  NorFlashPlatformLib|Include/Library/NorFlashPlatformLib.h
>> +
>>   [Guids]
>>     gUefiOvmfPkgTokenSpaceGuid            = {0x93bb96af, 0xb9f2, 0x4eb8, {0x94, 0x62, 0xe0, 0xba, 0x74, 0x56, 0x42, 0x36}}
>>     gEfiXenInfoGuid                       = {0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d}}
>> @@ -405,6 +409,10 @@ [PcdsFixedAtBuild]
>>     #  check to decide whether to abort dispatch of the driver it is linked into.
>>     gUefiOvmfPkgTokenSpaceGuid.PcdEntryPointOverrideFwCfgVarName|""|VOID*|0x68
>>
>> +  ## The base address and size of the FVMAIN
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFvBaseAddress|0|UINT64|0x71
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFvSize|0|UINT32|0x72
>> +
>>   [PcdsDynamic, PcdsDynamicEx]
>>     gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
>>     gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10
>> diff --git a/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
>> new file mode 100644
>> index 000000000000..ecd8059b3508
>> --- /dev/null
>> +++ b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
>> @@ -0,0 +1,40 @@
>> +#/** @file
>> +#
>> +#  Component description file for NorFlashQemuLib module
>> +#
>> +#  Copyright (c) 2014, Linaro Ltd. All rights reserved.<BR>
>> +#
>> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +#
>> +#**/
>> +
>> +[Defines]
>> +  INF_VERSION                    = 0x00010005
>> +  BASE_NAME                      = NorFlashQemuLib
>> +  FILE_GUID                      = 42C30D8E-BFAD-4E77-9041-E7DAAE88DF7A
>> +  MODULE_TYPE                    = DXE_DRIVER
>> +  VERSION_STRING                 = 1.0
>> +  LIBRARY_CLASS                  = NorFlashPlatformLib
>> +
>> +[Sources.common]
>> +  NorFlashQemuLib.c
>> +
>> +[Packages]
>> +  MdePkg/MdePkg.dec
>> +  OvmfPkg/OvmfPkg.dec
>> +  EmbeddedPkg/EmbeddedPkg.dec
>> +
>> +[LibraryClasses]
>> +  BaseLib
>> +  DebugLib
>> +  UefiBootServicesTableLib
>> +
>> +[Protocols]
>> +  gFdtClientProtocolGuid          ## CONSUMES
>> +
>> +[Depex]
>> +  gFdtClientProtocolGuid
>> +
>> +[Pcd]
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFvBaseAddress
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFvSize
>> diff --git a/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.inf b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.inf
>> new file mode 100644
>> index 000000000000..91d1406fc3e7
>> --- /dev/null
>> +++ b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.inf
>> @@ -0,0 +1,30 @@
>> +#/** @file
>> +#
>> +#  Component description file for NorFlashQemuLib module
>> +#
>> +#  Copyright (c) 2014, Linaro Ltd. All rights reserved.<BR>
>> +#
>> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +#
>> +#**/
>> +
>> +[Defines]
>> +  INF_VERSION                    = 0x00010005
>> +  BASE_NAME                      = NorFlashQemuUnifiedLib
>> +  FILE_GUID                      = 064742F1-E531-4D7D-A154-22315889CC23
>> +  MODULE_TYPE                    = DXE_DRIVER
>> +  VERSION_STRING                 = 1.0
>> +  LIBRARY_CLASS                  = NorFlashPlatformLib
>> +
>> +[Sources.common]
>> +  NorFlashQemuUnifiedLib.c
>> +
>> +[Packages]
>> +  MdePkg/MdePkg.dec
>> +  MdeModulePkg/MdeModulePkg.dec
>> +  OvmfPkg/OvmfPkg.dec
>> +
>> +[Pcd]
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
>> diff --git a/OvmfPkg/Include/Library/NorFlashPlatformLib.h b/OvmfPkg/Include/Library/NorFlashPlatformLib.h
>> new file mode 100644
>> index 000000000000..6ef5b70e9948
>> --- /dev/null
>> +++ b/OvmfPkg/Include/Library/NorFlashPlatformLib.h
>> @@ -0,0 +1,30 @@
>> +/** @file
>> +
>> + Copyright (c) 2011-2012, ARM Ltd. All rights reserved.<BR>
>> +
>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> + **/
>> +
>> +#ifndef _NORFLASHPLATFORMLIB_H_
>> +#define _NORFLASHPLATFORMLIB_H_
>> +
>> +typedef struct {
>> +  UINTN    DeviceBaseAddress;       // Start address of the Device Base Address (DBA)
>> +  UINTN    RegionBaseAddress;       // Start address of one single region
>> +  UINTN    Size;
>> +  UINTN    BlockSize;
>> +} NOR_FLASH_DESCRIPTION;
>> +
>> +EFI_STATUS
>> +NorFlashPlatformInitialization (
>> +  VOID
>> +  );
>> +
>> +EFI_STATUS
>> +NorFlashPlatformGetDevices (
>> +  OUT NOR_FLASH_DESCRIPTION  **NorFlashDescriptions,
>> +  OUT UINT32                 *Count
>> +  );
>> +
>> +#endif /* _NORFLASHPLATFORMLIB_H_ */
>> diff --git a/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>> new file mode 100644
>> index 000000000000..3632fa9e7a98
>> --- /dev/null
>> +++ b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>> @@ -0,0 +1,136 @@
>> +/** @file
>> +
>> + Copyright (c) 2014-2018, Linaro 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>
>> +
>> +#define QEMU_NOR_BLOCK_SIZE  SIZE_256KB
>> +
>> +#define MAX_FLASH_BANKS  4
>> +
>> +EFI_STATUS
>> +NorFlashPlatformInitialization (
>> +  VOID
>> +  )
>> +{
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +NOR_FLASH_DESCRIPTION  mNorFlashDevices[MAX_FLASH_BANKS];
>> +
>> +EFI_STATUS
>> +NorFlashPlatformGetDevices (
>> +  OUT NOR_FLASH_DESCRIPTION  **NorFlashDescriptions,
>> +  OUT UINT32                 *Count
>> +  )
>> +{
>> +  FDT_CLIENT_PROTOCOL  *FdtClient;
>> +  INT32                Node;
>> +  EFI_STATUS           Status;
>> +  EFI_STATUS           FindNodeStatus;
>> +  CONST UINT32         *Reg;
>> +  UINT32               PropSize;
>> +  UINT32               Num;
>> +  UINT64               Base;
>> +  UINT64               Size;
>> +
>> +  Status = gBS->LocateProtocol (
>> +                  &gFdtClientProtocolGuid,
>> +                  NULL,
>> +                  (VOID **)&FdtClient
>> +                  );
>> +  ASSERT_EFI_ERROR (Status);
>> +
>> +  Num = 0;
>> +  for (FindNodeStatus = FdtClient->FindCompatibleNode (
>> +                                     FdtClient,
>> +                                     "cfi-flash",
>> +                                     &Node
>> +                                     );
>> +       !EFI_ERROR (FindNodeStatus) && Num < 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)) && Num < 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 (PcdOvmfFvBaseAddress) + PcdGet32 (PcdOvmfFvSize) > Base) &&
>> +          ((Base + Size) > PcdGet64 (PcdOvmfFvBaseAddress)))
>> +      {
>> +        continue;
>> +      }
>> +
>> +      mNorFlashDevices[Num].DeviceBaseAddress = (UINTN)Base;
>> +      mNorFlashDevices[Num].RegionBaseAddress = (UINTN)Base;
>> +      mNorFlashDevices[Num].Size              = (UINTN)Size;
>> +      mNorFlashDevices[Num].BlockSize         = QEMU_NOR_BLOCK_SIZE;
>> +      Num++;
>> +    }
>> +
>> +    //
>> +    // UEFI takes ownership of the NOR flash, and exposes its functionality
>> +    // through the UEFI Runtime Services GetVariable, SetVariable, etc. This
>> +    // means we need to disable it in the device tree to prevent the OS from
>> +    // attaching its device driver as well.
>> +    // Note that this also hides other flash banks, but the only other flash
>> +    // bank we expect to encounter is the one that carries the UEFI executable
>> +    // code, which is not intended to be guest updatable, and is usually backed
>> +    // in a readonly manner by QEMU anyway.
>> +    //
>> +    Status = FdtClient->SetNodeProperty (
>> +                          FdtClient,
>> +                          Node,
>> +                          "status",
>> +                          "disabled",
>> +                          sizeof ("disabled")
>> +                          );
>> +    if (EFI_ERROR (Status)) {
>> +      DEBUG ((DEBUG_WARN, "Failed to set NOR flash status to 'disabled'\n"));
>> +    }
>> +  }
>> +
>> +  *NorFlashDescriptions = mNorFlashDevices;
>> +  *Count                = Num;
>> +
>> +  return EFI_SUCCESS;
>> +}
>> diff --git a/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.c b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.c
>> new file mode 100644
>> index 000000000000..1420fb5b596c
>> --- /dev/null
>> +++ b/OvmfPkg/Library/NorFlashQemuLib/NorFlashQemuUnifiedLib.c
>> @@ -0,0 +1,40 @@
>> +/** @file
>> +
>> + Copyright (c) 2019, Linaro Ltd. All rights reserved
>> +
>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> + **/
>> +
>> +#include <Base.h>
>> +#include <PiDxe.h>
>> +#include <Library/NorFlashPlatformLib.h>
>> +
>> +#define QEMU_NOR_BLOCK_SIZE  SIZE_256KB
>> +
>> +EFI_STATUS
>> +NorFlashPlatformInitialization (
>> +  VOID
>> +  )
>> +{
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +NOR_FLASH_DESCRIPTION  mNorFlashDevice =
>> +{
>> +  FixedPcdGet32 (PcdOvmfFdBaseAddress),
>> +  FixedPcdGet64 (PcdFlashNvStorageVariableBase),
>> +  FixedPcdGet32 (PcdOvmfFirmwareFdSize),
>> +  QEMU_NOR_BLOCK_SIZE
>> +};
>> +
>> +EFI_STATUS
>> +NorFlashPlatformGetDevices (
>> +  OUT NOR_FLASH_DESCRIPTION  **NorFlashDescriptions,
>> +  OUT UINT32                 *Count
>> +  )
>> +{
>> +  *NorFlashDescriptions = &mNorFlashDevice;
>> +  *Count                = 1;
>> +  return EFI_SUCCESS;
>> +}
>> --
>> 2.25.1
>>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95398): https://edk2.groups.io/g/devel/message/95398
Mute This Topic: https://groups.io/mt/94233036/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-staging/RiscV64QemuVirt PATCH 25/29] OvmfPkg: Add NorFlashQemuLib library
Posted by Ard Biesheuvel 3 years, 3 months ago
On Wed, 19 Oct 2022 at 15:06, Sami Mujawar <sami.mujawar@arm.com> wrote:
>
> Hi Ard,
>
> Please see my query inline marked [SAMI].
>
> Regards,
>
> Sami Mujawar
>
> On 19/10/2022 01:19 pm, Ard Biesheuvel via groups.io wrote:
> > On Mon, 10 Oct 2022 at 12:13, Sunil V L <sunilvl@ventanamicro.com> wrote:
> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4076
> >>
> >> This is copied from ArmVirtPkg since it is required for
> >> other architectures also.
> >>
> >> It also adds the instance for single flash drive which has
> >> both code and variables. This is copied from SbsaQemu.
> >>
> >> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> >> Cc: Jiewen Yao <jiewen.yao@intel.com>
> >> Cc: Jordan Justen <jordan.l.justen@intel.com>
> >> Cc: Gerd Hoffmann <kraxel@redhat.com>
> >> Cc: Daniel Schaefer <git@danielschaefer.me>
> >> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > Let's call these
> >
> > QemuNorFlashPlatformLib [for the library class]
> >
> > QemuNorFlashDeviceTreeLib
> > QemuNorFlashStaticLib
> >
> > and for the driver
> >
> > QemuNorFlashDxe
>
> [SAMI] We use the NorFlashDxe for the Kvmtool guest firmware, see
> https://github.com/tianocore/edk2/blob/master/ArmVirtPkg/ArmVirtKvmTool.dsc#L294
>
> Considering this, should QemuNorFlashDxe be called OvmfNorFlashDxe?
>
> [/SAMI]
>

Ah yes, good point. So using Qemu as a prefix is slightly problematic.

My intent is for this driver to be optimized towards NOR flash
emulation the way QEMU implements it. The main difference between the
platforms is that some of them (notably, ArmVirtQemu.dsc) also execute
from the emulated region, which requires an executable (read-only)
memslot in KVM, as instruction fetches (as opposed to explicit loads
and stores) cannot be emulated by KVM. KvmTool only uses the NOR flash
for variables, so it doesn't really care how the emulation is
implemented, as long as the loads and stores are carried out in the
expected way.

I don't think the Ovmf prefix makes sense here either - OVMF is still
primarily x86, which uses a different flash emulation altogether.
Maybe VirtNorFlashDxe, to emphasize that it does not expect to be
dealing with actual NOR flash?

Suggestions welcome :-)


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95399): https://edk2.groups.io/g/devel/message/95399
Mute This Topic: https://groups.io/mt/94233036/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-staging/RiscV64QemuVirt PATCH 25/29] OvmfPkg: Add NorFlashQemuLib library
Posted by Sami Mujawar 3 years, 3 months ago
Hi Ard,


On 19/10/2022 02:14 pm, Ard Biesheuvel wrote:
> On Wed, 19 Oct 2022 at 15:06, Sami Mujawar <sami.mujawar@arm.com> wrote:
>> Hi Ard,
>>
>> Please see my query inline marked [SAMI].
>>
>> Regards,
>>
>> Sami Mujawar
>>
>> On 19/10/2022 01:19 pm, Ard Biesheuvel via groups.io wrote:
>>> On Mon, 10 Oct 2022 at 12:13, Sunil V L <sunilvl@ventanamicro.com> wrote:
>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4076
>>>>
>>>> This is copied from ArmVirtPkg since it is required for
>>>> other architectures also.
>>>>
>>>> It also adds the instance for single flash drive which has
>>>> both code and variables. This is copied from SbsaQemu.
>>>>
>>>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>>> Cc: Daniel Schaefer <git@danielschaefer.me>
>>>> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
>>> Let's call these
>>>
>>> QemuNorFlashPlatformLib [for the library class]
>>>
>>> QemuNorFlashDeviceTreeLib
>>> QemuNorFlashStaticLib
>>>
>>> and for the driver
>>>
>>> QemuNorFlashDxe
>> [SAMI] We use the NorFlashDxe for the Kvmtool guest firmware, see
>> https://github.com/tianocore/edk2/blob/master/ArmVirtPkg/ArmVirtKvmTool.dsc#L294
>>
>> Considering this, should QemuNorFlashDxe be called OvmfNorFlashDxe?
>>
>> [/SAMI]
>>
> Ah yes, good point. So using Qemu as a prefix is slightly problematic.
>
> My intent is for this driver to be optimized towards NOR flash
> emulation the way QEMU implements it. The main difference between the
> platforms is that some of them (notably, ArmVirtQemu.dsc) also execute
> from the emulated region, which requires an executable (read-only)
> memslot in KVM, as instruction fetches (as opposed to explicit loads
> and stores) cannot be emulated by KVM. KvmTool only uses the NOR flash
> for variables, so it doesn't really care how the emulation is
> implemented, as long as the loads and stores are carried out in the
> expected way.
>
> I don't think the Ovmf prefix makes sense here either - OVMF is still
> primarily x86, which uses a different flash emulation altogether.
> Maybe VirtNorFlashDxe, to emphasize that it does not expect to be
> dealing with actual NOR flash?

VirtNorFlashDxe sounds good to me.

Regards,

Sami Mujawar

>
> Suggestions welcome :-)


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95401): https://edk2.groups.io/g/devel/message/95401
Mute This Topic: https://groups.io/mt/94233036/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-staging/RiscV64QemuVirt PATCH 25/29] OvmfPkg: Add NorFlashQemuLib library
Posted by Leif Lindholm 3 years, 3 months ago
On Wed, Oct 19, 2022 at 14:19:28 +0100, Sami Mujawar wrote:
> > > Considering this, should QemuNorFlashDxe be called OvmfNorFlashDxe?
> > > 
> > > [/SAMI]
> > > 
> > Ah yes, good point. So using Qemu as a prefix is slightly problematic.
> > 
> > My intent is for this driver to be optimized towards NOR flash
> > emulation the way QEMU implements it. The main difference between the
> > platforms is that some of them (notably, ArmVirtQemu.dsc) also execute
> > from the emulated region, which requires an executable (read-only)
> > memslot in KVM, as instruction fetches (as opposed to explicit loads
> > and stores) cannot be emulated by KVM. KvmTool only uses the NOR flash
> > for variables, so it doesn't really care how the emulation is
> > implemented, as long as the loads and stores are carried out in the
> > expected way.
> > 
> > I don't think the Ovmf prefix makes sense here either - OVMF is still
> > primarily x86, which uses a different flash emulation altogether.
> > Maybe VirtNorFlashDxe, to emphasize that it does not expect to be
> > dealing with actual NOR flash?
> 
> VirtNorFlashDxe sounds good to me.

Sounds good to me too.

/
    Leif


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95403): https://edk2.groups.io/g/devel/message/95403
Mute This Topic: https://groups.io/mt/94233036/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-staging/RiscV64QemuVirt PATCH 25/29] OvmfPkg: Add NorFlashQemuLib library
Posted by Sunil V L 3 years, 3 months ago
On Wed, Oct 19, 2022 at 02:19:28PM +0200, Ard Biesheuvel wrote:
> On Mon, 10 Oct 2022 at 12:13, Sunil V L <sunilvl@ventanamicro.com> wrote:
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4076
> >
> > This is copied from ArmVirtPkg since it is required for
> > other architectures also.
> >
> > It also adds the instance for single flash drive which has
> > both code and variables. This is copied from SbsaQemu.
> >
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Daniel Schaefer <git@danielschaefer.me>
> > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> 
> Let's call these
> 
> QemuNorFlashPlatformLib [for the library class]
> 
> QemuNorFlashDeviceTreeLib
> QemuNorFlashStaticLib
> 
> and for the driver
> 
> QemuNorFlashDxe

Sure. I will make these changes when I send next version.
> 
> 
> i sent out some patches for edk2-platforms to eliminate the dependency
> on ArmPlatformPkg's NorFlashDxe and NorFlashPlatformLib definitions.
> Once we move everything in OvmfPkg over to these ones, we can drop the
> old one altogether.

Sure.

Thanks!
Sunil


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95390): https://edk2.groups.io/g/devel/message/95390
Mute This Topic: https://groups.io/mt/94233036/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-