[PATCH 0/4] OvmfPkg/RiscVVirt: Separate code and variable storage

[PATCH 0/4] OvmfPkg/RiscVVirt: Separate code and variable storage

[Sunil V L]

Recent updates to RISC-V qemu virt platform merged today (07/14),
have enabled both pflash devices for the S-mode payload like EDK2.
These updates also aligned the design similar to other architectures
where pflash0 is for read-only code and pflash1 for variable store.
Previously only pflash1 was available for S-mode use. 

Current EDK2 will not work with this latest qemu changes since it always
assumed to boot from pflash1. So, separate the code and variable
store and use pflash0 to keep the code.

Add 'readme' about build and test as per these changes.

The changes are available in the branch :
https://github.com/vlsunil/edk2/tree/separate_code_vars

CI tests passed (#4553).

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: Andrei Warkentin <andrei.warkentin@intel.com>
Cc: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Cc: Dann Frazier <dann.frazier@canonical.com>


Sunil V L (4):
  OvmfPkg/RiscVVirt: Fix couple of issues in VarStore
  OvmfPkg/RiscVVirt: Add VirtNorFlashDeviceTreeLib library
  OvmfPkg/RiscVVirt: Add support for separate code and variable store
  OvmfPkg/RiscVVirt: Add a readme for build and test

 OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc           |   2 +-
 OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf           |   9 +-
 .../VirtNorFlashDeviceTreeLib.inf             |  40 +++++
 .../VirtNorFlashDeviceTreeLib.c               | 137 ++++++++++++++++++
 OvmfPkg/RiscVVirt/README.md                   |  41 ++++++
 OvmfPkg/RiscVVirt/RiscVVirt.fdf.inc           |  12 +-
 OvmfPkg/RiscVVirt/VarStore.fdf.inc            |  25 ++--
 7 files changed, 245 insertions(+), 21 deletions(-)
 create mode 100644 OvmfPkg/RiscVVirt/Library/VirtNorFlashPlatformLib/VirtNorFlashDeviceTreeLib.inf
 create mode 100644 OvmfPkg/RiscVVirt/Library/VirtNorFlashPlatformLib/VirtNorFlashDeviceTreeLib.c
 create mode 100644 OvmfPkg/RiscVVirt/README.md

-- 
2.34.1

[PATCH 1/4] OvmfPkg/RiscVVirt: Fix couple of issues in VarStore

[Sunil V L]

The size of the FV and the WriteQueueSize is incorrect which causes
the flash to be re-written during boot. Fix them and update the new
checksum value.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
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: Andrei Warkentin <andrei.warkentin@intel.com>
---
 OvmfPkg/RiscVVirt/VarStore.fdf.inc | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/OvmfPkg/RiscVVirt/VarStore.fdf.inc b/OvmfPkg/RiscVVirt/VarStore.fdf.inc
index 30b170d77997..6bc619e50c1a 100644
--- a/OvmfPkg/RiscVVirt/VarStore.fdf.inc
+++ b/OvmfPkg/RiscVVirt/VarStore.fdf.inc
@@ -25,12 +25,12 @@
   #     { 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50 }}
   0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C,
   0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50,
-  # FvLength: 0x20000
-  0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00,
+  # FvLength: 0xC0000
+  0x00, 0x00, 0x0C, 0x00, 0x00, 0x00, 0x00, 0x00,
   # Signature "_FVH"       # Attributes
   0x5f, 0x46, 0x56, 0x48, 0xff, 0xfe, 0x04, 0x00,
   # HeaderLength # CheckSum # ExtHeaderOffset #Reserved #Revision
-  0x48, 0x00, 0x39, 0xF1, 0x00, 0x00, 0x00, 0x02,
+  0x48, 0x00, 0x2F, 0xF1, 0x00, 0x00, 0x00, 0x02,
   # Blockmap[0]: 0x20 Blocks * 0x1000 Bytes / Block
   0x00, 0x08, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00,
   # Blockmap[1]: End
@@ -60,7 +60,7 @@
 $(VARS_FTW_WORKING_OFFSET)|$(VARS_FTW_WORKING_SIZE)
 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
 #
-#NV_FTW_WROK
+#NV_FTW_WORK
 #
 DATA = {
   # EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER->Signature = gEdkiiWorkingBlockSignatureGuid         =
@@ -68,9 +68,9 @@
   0x2b, 0x29, 0x58, 0x9e, 0x68, 0x7c, 0x7d, 0x49,
   0xa0, 0xce, 0x65,  0x0, 0xfd, 0x9f, 0x1b, 0x95,
   # Crc:UINT32            #WorkingBlockValid:1, WorkingBlockInvalid:1, Reserved
-  0x2c, 0xaf, 0x2c, 0x64, 0xFE, 0xFF, 0xFF, 0xFF,
+  0x5b, 0xe7, 0xc6, 0x86, 0xFE, 0xFF, 0xFF, 0xFF,
   # WriteQueueSize: UINT64
-  0xE0, 0x0F, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+  0xE0, 0xFF, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00
 }
 
 $(VARS_FTW_SPARE_OFFSET)|$(VARS_FTW_SPARE_SIZE)
-- 
2.34.1

[PATCH 2/4] OvmfPkg/RiscVVirt: Add VirtNorFlashDeviceTreeLib library

[Sunil V L]

This library is required to support separate code and
variable store images.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
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: Andrei Warkentin <andrei.warkentin@intel.com>
---
 .../VirtNorFlashDeviceTreeLib.inf             |  40 +++++
 .../VirtNorFlashDeviceTreeLib.c               | 137 ++++++++++++++++++
 2 files changed, 177 insertions(+)
 create mode 100644 OvmfPkg/RiscVVirt/Library/VirtNorFlashPlatformLib/VirtNorFlashDeviceTreeLib.inf
 create mode 100644 OvmfPkg/RiscVVirt/Library/VirtNorFlashPlatformLib/VirtNorFlashDeviceTreeLib.c

diff --git a/OvmfPkg/RiscVVirt/Library/VirtNorFlashPlatformLib/VirtNorFlashDeviceTreeLib.inf b/OvmfPkg/RiscVVirt/Library/VirtNorFlashPlatformLib/VirtNorFlashDeviceTreeLib.inf
new file mode 100644
index 000000000000..90df756e79a9
--- /dev/null
+++ b/OvmfPkg/RiscVVirt/Library/VirtNorFlashPlatformLib/VirtNorFlashDeviceTreeLib.inf
@@ -0,0 +1,40 @@
+#/** @file
+#
+#  Component description file for VirtNorFlashDeviceTreeLib module
+#
+#  Copyright (c) 2023, Ventana Micro Systems Inc. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x0001001B
+  BASE_NAME                      = VirtNorFlashDeviceTreeLib
+  FILE_GUID                      = 1D74E65F-A468-4FA6-ACB6-E4E941EB79D6
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = VirtNorFlashPlatformLib
+
+[Sources.common]
+  VirtNorFlashDeviceTreeLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  UefiBootServicesTableLib
+
+[Protocols]
+  gFdtClientProtocolGuid          ## CONSUMES
+
+[Depex]
+  gFdtClientProtocolGuid
+
+[Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize
diff --git a/OvmfPkg/RiscVVirt/Library/VirtNorFlashPlatformLib/VirtNorFlashDeviceTreeLib.c b/OvmfPkg/RiscVVirt/Library/VirtNorFlashPlatformLib/VirtNorFlashDeviceTreeLib.c
new file mode 100644
index 000000000000..73534a866430
--- /dev/null
+++ b/OvmfPkg/RiscVVirt/Library/VirtNorFlashPlatformLib/VirtNorFlashDeviceTreeLib.c
@@ -0,0 +1,137 @@
+/** @file
+
+ Copyright (c) 2014-2018, Linaro Ltd. All rights reserved.<BR>
+ Copyright (c) 2023, Ventana Micro Systems Inc. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ **/
+
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/VirtNorFlashPlatformLib.h>
+
+#include <Protocol/FdtClient.h>
+
+#define QEMU_NOR_BLOCK_SIZE  SIZE_256KB
+
+#define MAX_FLASH_BANKS  4
+
+EFI_STATUS
+VirtNorFlashPlatformInitialization (
+  VOID
+  )
+{
+  return EFI_SUCCESS;
+}
+
+STATIC VIRT_NOR_FLASH_DESCRIPTION  mNorFlashDevices[MAX_FLASH_BANKS];
+
+EFI_STATUS
+VirtNorFlashPlatformGetDevices (
+  OUT VIRT_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",
+        __func__,
+        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 ((PcdGet32 (PcdOvmfFdBaseAddress) + PcdGet32 (PcdOvmfFirmwareFdSize) > Base) &&
+          ((Base + Size) > PcdGet32 (PcdOvmfFdBaseAddress)))
+      {
+        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;
+}
-- 
2.34.1

[PATCH 3/4] OvmfPkg/RiscVVirt: Add support for separate code and variable store

[Sunil V L]

Currently, RiscVVirtQemu supports unified code and variable store
mainly because only one pflash devices was available in qemu for
EDK2. However, this doesn't allow to map the code part as read-only.

With recent qemu enhancements, it is now possible for EDK2 to make
use of both pflash devices in RISC-V virt machine. So, add support
to create code and vars images separately. This also allows easy
firmware code updates without losing the variable store.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
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: Andrei Warkentin <andrei.warkentin@intel.com>
---
 OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc |  2 +-
 OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf |  9 +++++----
 OvmfPkg/RiscVVirt/RiscVVirt.fdf.inc | 12 +++++-------
 OvmfPkg/RiscVVirt/VarStore.fdf.inc  | 13 ++++++++++---
 4 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc b/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc
index 414d186179fb..04573bc0f32e 100644
--- a/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc
+++ b/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc
@@ -85,7 +85,7 @@ [LibraryClasses.common]
   QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
 
   TimerLib|UefiCpuPkg/Library/BaseRiscV64CpuTimerLib/BaseRiscV64CpuTimerLib.inf
-  VirtNorFlashPlatformLib|OvmfPkg/RiscVVirt/Library/VirtNorFlashPlatformLib/VirtNorFlashStaticLib.inf
+  VirtNorFlashPlatformLib|OvmfPkg/RiscVVirt/Library/VirtNorFlashPlatformLib/VirtNorFlashDeviceTreeLib.inf
 
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
diff --git a/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf b/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
index 354c9271d10c..dbe38a135395 100644
--- a/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
+++ b/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
@@ -12,17 +12,18 @@
 !include RiscVVirt.fdf.inc
 
 ################################################################################
-[FD.RISCV_VIRT]
-BaseAddress   = $(FW_BASE_ADDRESS)|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
-Size          = $(FW_SIZE)|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize
+[FD.RISCV_VIRT_CODE]
+BaseAddress   = $(CODE_BASE_ADDRESS)|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
+Size          = $(CODE_SIZE)|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize
 ErasePolarity = 1
 BlockSize     = $(BLOCK_SIZE)
-NumBlocks     = $(FW_BLOCKS)
+NumBlocks     = $(CODE_BLOCKS)
 
 0x00000000|$(CODE_SIZE)
 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
 FV = FVMAIN_COMPACT
 
+################################################################################
 !include VarStore.fdf.inc
 ################################################################################
 
diff --git a/OvmfPkg/RiscVVirt/RiscVVirt.fdf.inc b/OvmfPkg/RiscVVirt/RiscVVirt.fdf.inc
index b0a1c3293f33..7e8165d85778 100644
--- a/OvmfPkg/RiscVVirt/RiscVVirt.fdf.inc
+++ b/OvmfPkg/RiscVVirt/RiscVVirt.fdf.inc
@@ -10,16 +10,14 @@
 [Defines]
 DEFINE BLOCK_SIZE                  = 0x1000
 
+DEFINE PFLASH0_BASE                = 0x20000000
 DEFINE PFLASH1_BASE                = 0x22000000
 
-DEFINE FW_BASE_ADDRESS             = $(PFLASH1_BASE)
-DEFINE FW_SIZE                     = 0x00800000
-DEFINE FW_BLOCKS                   = 0x800
-
-DEFINE CODE_BASE_ADDRESS           = $(FW_BASE_ADDRESS)
-DEFINE CODE_SIZE                   = 0x00740000
-DEFINE CODE_BLOCKS                 = 0x740
+DEFINE CODE_BASE_ADDRESS           = $(PFLASH0_BASE)
+DEFINE CODE_SIZE                   = 0x00800000
+DEFINE CODE_BLOCKS                 = 0x800
 
+DEFINE VARS_BASE_ADDRESS            = $(PFLASH1_BASE)
 DEFINE VARS_SIZE                    = 0x000C0000
 DEFINE VARS_BLOCK_SIZE              = 0x40000
 DEFINE VARS_BLOCKS                  = 0x3
diff --git a/OvmfPkg/RiscVVirt/VarStore.fdf.inc b/OvmfPkg/RiscVVirt/VarStore.fdf.inc
index 6bc619e50c1a..49293c742b56 100644
--- a/OvmfPkg/RiscVVirt/VarStore.fdf.inc
+++ b/OvmfPkg/RiscVVirt/VarStore.fdf.inc
@@ -10,7 +10,14 @@
 #
 ##
 
-$(VARS_OFFSET)|$(VARS_LIVE_SIZE)
+[FD.RISCV_VIRT_VARS]
+BaseAddress   = $(VARS_BASE_ADDRESS)
+Size          = $(VARS_SIZE)
+ErasePolarity = 1
+BlockSize     = $(VARS_BLOCK_SIZE)
+NumBlocks     = $(VARS_BLOCKS)
+
+0x00000000|$(VARS_LIVE_SIZE)
 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
 #
 # NV_VARIABLE_STORE
@@ -57,7 +64,7 @@
   0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
 }
 
-$(VARS_FTW_WORKING_OFFSET)|$(VARS_FTW_WORKING_SIZE)
+0x00040000|$(VARS_FTW_WORKING_SIZE)
 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
 #
 #NV_FTW_WORK
@@ -73,7 +80,7 @@
   0xE0, 0xFF, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00
 }
 
-$(VARS_FTW_SPARE_OFFSET)|$(VARS_FTW_SPARE_SIZE)
+0x00080000|$(VARS_FTW_SPARE_SIZE)
 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
 #
 #NV_FTW_SPARE
-- 
2.34.1

[PATCH 4/4] OvmfPkg/RiscVVirt: Add a readme for build and test

[Sunil V L]

Add a readme file which provides information regarding how
to build and test EDK2 on RISC-V qemu virt platform.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
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: Andrei Warkentin <andrei.warkentin@intel.com>
---
 OvmfPkg/RiscVVirt/README.md | 41 +++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 OvmfPkg/RiscVVirt/README.md

diff --git a/OvmfPkg/RiscVVirt/README.md b/OvmfPkg/RiscVVirt/README.md
new file mode 100644
index 000000000000..a1738658318d
--- /dev/null
+++ b/OvmfPkg/RiscVVirt/README.md
@@ -0,0 +1,41 @@
+# Support for RISC-V qemu virt platform
+
+## Overview
+RISC-V qemu 'virt' is a generic platform which does not correspond to any real
+hardware.
+
+EDK2 for RISC-V virt platform is a payload (S-mode) for a previous stage M-mode
+firmware like opensbi. It follows PEI less design.
+
+## Build
+    export WORKSPACE=`pwd`
+    export GCC5_RISCV64_PREFIX=riscv64-linux-gnu-
+    export PACKAGES_PATH=$WORKSPACE/edk2
+    export EDK_TOOLS_PATH=$WORKSPACE/edk2/BaseTools
+    source edk2/edksetup.sh
+    make -C edk2/BaseTools
+    source edk2/edksetup.sh BaseTools
+    build -a RISCV64 --buildtarget RELEASE -p OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc -t GCC5
+
+## Test
+1) RISC-V qemu pflash devices should be of of size 32MiB.
+
+    `truncate -s 32M Build/RiscVVirtQemu/RELEASE_GCC5/FV/RISCV_VIRT_CODE.fd`
+
+    `truncate -s 32M Build/RiscVVirtQemu/RELEASE_GCC5/FV/RISCV_VIRT_VARS.fd`
+
+2) Run qemu
+
+        qemu-system-riscv64 \
+        -accel tcg -m 4096 -smp 2 \
+        -serial mon:stdio \
+        -device virtio-gpu-pci -full-screen \
+        -device qemu-xhci \
+        -device usb-kbd \
+        -blockdev node-name=pflash0,driver=file,read-only=on,filename=RISCV_VIRT_CODE.fd \
+        -blockdev node-name=pflash1,driver=file,filename=RISCV_VIRT_VARS.fd \
+        -M virt,pflash0=pflash0,pflash1=pflash1,acpi=off \
+        -kernel linux/arch/riscv/boot/Image \
+        -initrd buildroot/output/images/rootfs.cpio \
+        -netdev user,id=net0 -device virtio-net-pci,netdev=net0 \
+        -append "root=/dev/ram rw console=ttyS0 earlycon=uart8250,mmio,0x10000000"
-- 
2.34.1

Re: [PATCH 0/4] OvmfPkg/RiscVVirt: Separate code and variable storage

[Heinrich Schuchardt]

[-- Attachment #1: Type: text/plain, Size: 2342 bytes --]

Sunil V L <sunilvl@ventanamicro.com> schrieb am Mi., 14. Juni 2023, 19:01:

> Recent updates to RISC-V qemu virt platform merged today (07/14),
> have enabled both pflash devices for the S-mode payload like EDK2.
> These updates also aligned the design similar to other architectures
> where pflash0 is for read-only code and pflash1 for variable store.
> Previously only pflash1 was available for S-mode use.
>
> Current EDK2 will not work with this latest qemu changes since it always
> assumed to boot from pflash1. So, separate the code and variable
> store and use pflash0 to keep the code.
>

Not all users will have the newest QEMU provided by their Linx distro. Will
it be possible to boot the the same EDK II binary on old and new QEMU
releases?

Best regards

Heinrich


> Add 'readme' about build and test as per these changes.
>
> The changes are available in the branch :
> https://github.com/vlsunil/edk2/tree/separate_code_vars
>
> CI tests passed (#4553).
>
> 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: Andrei Warkentin <andrei.warkentin@intel.com>
> Cc: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Cc: Dann Frazier <dann.frazier@canonical.com>
>
>
> Sunil V L (4):
>   OvmfPkg/RiscVVirt: Fix couple of issues in VarStore
>   OvmfPkg/RiscVVirt: Add VirtNorFlashDeviceTreeLib library
>   OvmfPkg/RiscVVirt: Add support for separate code and variable store
>   OvmfPkg/RiscVVirt: Add a readme for build and test
>
>  OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc           |   2 +-
>  OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf           |   9 +-
>  .../VirtNorFlashDeviceTreeLib.inf             |  40 +++++
>  .../VirtNorFlashDeviceTreeLib.c               | 137 ++++++++++++++++++
>  OvmfPkg/RiscVVirt/README.md                   |  41 ++++++
>  OvmfPkg/RiscVVirt/RiscVVirt.fdf.inc           |  12 +-
>  OvmfPkg/RiscVVirt/VarStore.fdf.inc            |  25 ++--
>  7 files changed, 245 insertions(+), 21 deletions(-)
>  create mode 100644
> OvmfPkg/RiscVVirt/Library/VirtNorFlashPlatformLib/VirtNorFlashDeviceTreeLib.inf
>  create mode 100644
> OvmfPkg/RiscVVirt/Library/VirtNorFlashPlatformLib/VirtNorFlashDeviceTreeLib.c
>  create mode 100644 OvmfPkg/RiscVVirt/README.md
>
> --
> 2.34.1
>
>

[-- Attachment #2: Type: text/html, Size: 3897 bytes --]

Re: [PATCH 0/4] OvmfPkg/RiscVVirt: Separate code and variable storage

[Sunil V L]

On Wed, Jun 14, 2023 at 07:11:18PM +0200, Heinrich Schuchardt wrote:
> Sunil V L <sunilvl@ventanamicro.com> schrieb am Mi., 14. Juni 2023, 19:01:
> 
> > Recent updates to RISC-V qemu virt platform merged today (07/14),
> > have enabled both pflash devices for the S-mode payload like EDK2.
> > These updates also aligned the design similar to other architectures
> > where pflash0 is for read-only code and pflash1 for variable store.
> > Previously only pflash1 was available for S-mode use.
> >
> > Current EDK2 will not work with this latest qemu changes since it always
> > assumed to boot from pflash1. So, separate the code and variable
> > store and use pflash0 to keep the code.
> >
> 
> Not all users will have the newest QEMU provided by their Linx distro. Will
> it be possible to boot the the same EDK II binary on old and new QEMU
> releases?
> 
Hi Heinrich,

I was initially thinking to have support for both unified and separate
images something similar to x86. However, it doesn't work for RISC-V
since we need code in the beginning of the image. So, if we try to add
both unified and separate image support, the PCD variables for NVRAM get
different values during build and boot will fail for one of the image. I
don't think it is worth of the effort to have two different
infrastructure files for old and new qemu.

We discussed this issue in qemu community and consensus was it is better
in the long term to move to this design while in short term we will have
this incompatibility. Since RISC-V EDK2 is relatively new, I think it is
OK to request people with old qemu to use older EDK2 or current stable
release. For next stable release, both qemu and edk2 will be compatible
to each other.

Thanks,
Sunil

Re: [PATCH 0/4] OvmfPkg/RiscVVirt: Separate code and variable storage

[Sunil V L]

On Wed, Jun 14, 2023 at 07:11:18PM +0200, Heinrich Schuchardt wrote:
> Sunil V L <sunilvl@ventanamicro.com> schrieb am Mi., 14. Juni 2023, 19:01:
> 
> > Recent updates to RISC-V qemu virt platform merged today (07/14),
> > have enabled both pflash devices for the S-mode payload like EDK2.
> > These updates also aligned the design similar to other architectures
> > where pflash0 is for read-only code and pflash1 for variable store.
> > Previously only pflash1 was available for S-mode use.
> >
> > Current EDK2 will not work with this latest qemu changes since it always
> > assumed to boot from pflash1. So, separate the code and variable
> > store and use pflash0 to keep the code.
> >
> 
> Not all users will have the newest QEMU provided by their Linx distro. Will
> it be possible to boot the the same EDK II binary on old and new QEMU
> releases?
> 
Hi Heinrich,

While both unified and separate images can not be generated at the same
time, a build flag can be used by which the user can build one of
the schemes matching qemu capabilities. Default will be separate
code and vars matching the latest qemu.

I believe this will alleviate the concern. If this looks better, let me
test and send v2.

Thanks!
Sunil

Re: [PATCH 0/4] OvmfPkg/RiscVVirt: Separate code and variable storage

[Heinrich Schuchardt]

On 6/15/23 13:50, Sunil V L wrote:
> On Wed, Jun 14, 2023 at 07:11:18PM +0200, Heinrich Schuchardt wrote:
>> Sunil V L <sunilvl@ventanamicro.com> schrieb am Mi., 14. Juni 2023, 19:01:
>>
>>> Recent updates to RISC-V qemu virt platform merged today (07/14),
>>> have enabled both pflash devices for the S-mode payload like EDK2.
>>> These updates also aligned the design similar to other architectures
>>> where pflash0 is for read-only code and pflash1 for variable store.
>>> Previously only pflash1 was available for S-mode use.
>>>
>>> Current EDK2 will not work with this latest qemu changes since it always
>>> assumed to boot from pflash1. So, separate the code and variable
>>> store and use pflash0 to keep the code.
>>>
>>
>> Not all users will have the newest QEMU provided by their Linx distro. Will
>> it be possible to boot the same EDK II binary on old and new QEMU
>> releases?
>>
> Hi Heinrich,
> 
> While both unified and separate images can not be generated at the same
> time, a build flag can be used by which the user can build one of
> the schemes matching qemu capabilities. Default will be separate
> code and vars matching the latest qemu.
> 
> I believe this will alleviate the concern. If this looks better, let me
> test and send v2.
> 
> Thanks!
> Sunil

At least Debian and Ubuntu packaging relies on the separation of code 
and variables. So anyway we would not be able back-port EDK II as 
package to anything that does not have the upcoming QEMU release.

It would be helpful to document the constraints in the yet to be created 
OvmfPkg/RiscVVirt/ReadMe.rst.

Best regards

Heinrich