public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/3] CloudHv:arm: Enable direct kernel boot
@ 2022-09-16  2:46 Jianyong Wu
  2022-09-16  2:46 ` [PATCH 1/3] CloudHv:arm: add kernel load fs driver Jianyong Wu
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Jianyong Wu @ 2022-09-16  2:46 UTC (permalink / raw)
  To: devel, Sami.Mujawar; +Cc: ardb+tianocore, justin.he, jianyong.wu

Direct kernel boot removes the dependency of retrieving kernel image
from block device. For Cloud Hypervisor, we use the following way to
support it.

1. Cloud Hypervisor store kernel image into memory and put kernel info,
   including the memory base and size, into DT;
2. When init memory in edk2, the kernel memory region is retrieved from
   DT and set it as read only memory region;
3. Edk2 fetches kernel from memory and prepare a image handle;
4. Load kernel using LoadImage in the end.

1 is done in Cloud Hypervisor, 2 and 3 is done in this patch set, 4 is
not affected.

github PR link: https://github.com/tianocore/edk2/pull/3339

Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

Jianyong Wu (3):
  CloudHv:arm: add kernel load fs driver
  CloudHv:arm: build hob for kernel image memory as read-only
  CloudHv:arm: add kernel loader lib dsc/fdf

 ArmVirtPkg/ArmVirtCloudHv.dsc                 |   8 +-
 ArmVirtPkg/ArmVirtCloudHv.fdf                 |   1 +
 .../CloudHvKernelLoaderFsDxe.c                | 969 ++++++++++++++++++
 .../CloudHvKernelLoaderFsDxe.inf              |  55 +
 .../CloudHvVirtMemInfoLib.c                   |  66 +-
 5 files changed, 1094 insertions(+), 5 deletions(-)
 create mode 100644 ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c
 create mode 100644 ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

-- 
2.17.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/3] CloudHv:arm: add kernel load fs driver
  2022-09-16  2:46 [PATCH 0/3] CloudHv:arm: Enable direct kernel boot Jianyong Wu
@ 2022-09-16  2:46 ` Jianyong Wu
  2022-11-22 15:44   ` Sami Mujawar
  2022-11-22 15:46   ` Sami Mujawar
  2022-09-16  2:46 ` [PATCH 2/3] CloudHv:arm: build hob for kernel image memory as read-only Jianyong Wu
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 19+ messages in thread
From: Jianyong Wu @ 2022-09-16  2:46 UTC (permalink / raw)
  To: devel, Sami.Mujawar; +Cc: ardb+tianocore, justin.he, jianyong.wu

This is used for supporting direct kernel boot in CloudHv. CloudHv
will store kernel image in system ram and pass kernel info through
DT. It's firmware's responsibility to fetch the kernel data and
create a file device to feed for loadImage

Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
---
 .../CloudHvKernelLoaderFsDxe.c                | 969 ++++++++++++++++++
 .../CloudHvKernelLoaderFsDxe.inf              |  56 +
 2 files changed, 1025 insertions(+)
 create mode 100644 ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c
 create mode 100644 ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

diff --git a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c
new file mode 100644
index 0000000000..e4cfcfab72
--- /dev/null
+++ b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c
@@ -0,0 +1,969 @@
+/** @file
+  DXE driver to expose the 'kernel', 'initrd' and 'cmdline' blobs
+  provided by Cloud Hypervisor as files in an abstract file system
+
+  Copyright (C) 2022, Arm, Limited.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <PiPei.h>
+#include <PiDxe.h>
+
+#include <libfdt.h>
+#include <Guid/FileInfo.h>
+#include <Guid/FileSystemInfo.h>
+#include <Guid/FileSystemVolumeLabelInfo.h>
+#include <Guid/LinuxEfiInitrdMedia.h>
+#include <Guid/QemuKernelLoaderFsMedia.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/BlobVerifierLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DevicePathLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PrePiLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+#include <Protocol/DevicePath.h>
+#include <Protocol/LoadFile2.h>
+#include <Protocol/SimpleFileSystem.h>
+
+//
+// Static data that hosts the data blobs and serves file requests.
+//
+typedef enum {
+  KernelBlobTypeKernel,
+  KernelBlobTypeInitrd,
+  KernelBlobTypeCommandLine,
+  KernelBlobTypeMax
+} KERNEL_BLOB_TYPE;
+
+typedef struct {
+  CONST CHAR16    Name[8];
+  UINT32          Size;
+  UINT8           *Data;
+} KERNEL_BLOB;
+
+STATIC KERNEL_BLOB  mKernelBlob[KernelBlobTypeMax] = {
+  {
+    L"kernel",
+  },{
+    L"initrd",
+  },{
+    L"cmdline",
+  }
+};
+
+//
+// Device path for the handle that incorporates our "EFI stub filesystem".
+//
+#pragma pack (1)
+typedef struct {
+  VENDOR_DEVICE_PATH          VenMediaNode;
+  EFI_DEVICE_PATH_PROTOCOL    EndNode;
+} SINGLE_VENMEDIA_NODE_DEVPATH;
+#pragma pack ()
+
+STATIC CONST SINGLE_VENMEDIA_NODE_DEVPATH  mFileSystemDevicePath = {
+  {
+    {
+      MEDIA_DEVICE_PATH, MEDIA_VENDOR_DP,
+      { sizeof (VENDOR_DEVICE_PATH)       }
+    },
+    QEMU_KERNEL_LOADER_FS_MEDIA_GUID
+  },  {
+    END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE,
+    { sizeof (EFI_DEVICE_PATH_PROTOCOL) }
+  }
+};
+
+STATIC CONST SINGLE_VENMEDIA_NODE_DEVPATH  mInitrdDevicePath = {
+  {
+    {
+      MEDIA_DEVICE_PATH, MEDIA_VENDOR_DP,
+      { sizeof (VENDOR_DEVICE_PATH)       }
+    },
+    LINUX_EFI_INITRD_MEDIA_GUID
+  },  {
+    END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE,
+    { sizeof (EFI_DEVICE_PATH_PROTOCOL) }
+  }
+};
+
+//
+// The "file in the EFI stub filesystem" abstraction.
+//
+STATIC EFI_TIME  mInitTime;
+STATIC UINT64    mTotalBlobBytes;
+
+#define STUB_FILE_SIG  SIGNATURE_64 ('S', 'T', 'U', 'B', 'F', 'I', 'L', 'E')
+
+typedef struct {
+  UINT64               Signature; // Carries STUB_FILE_SIG.
+
+  KERNEL_BLOB_TYPE     BlobType; // Index into mKernelBlob. KernelBlobTypeMax
+                                 // denotes the root directory of the filesystem.
+
+  UINT64               Position; // Byte position for regular files;
+                                 // next directory entry to return for the root
+                                 // directory.
+
+  EFI_FILE_PROTOCOL    File;   // Standard protocol interface.
+} STUB_FILE;
+
+#define STUB_FILE_FROM_FILE(FilePointer) \
+        CR (FilePointer, STUB_FILE, File, STUB_FILE_SIG)
+
+/**
+  Helper function that formats an EFI_FILE_INFO structure into the
+  user-allocated buffer, for any valid KERNEL_BLOB_TYPE value (including
+  KernelBlobTypeMax, which stands for the root directory).
+
+  The interface follows the EFI_FILE_GET_INFO -- and for directories, the
+  EFI_FILE_READ -- interfaces.
+
+  @param[in]     BlobType     The KERNEL_BLOB_TYPE value identifying the fw_cfg
+                              blob backing the STUB_FILE that information is
+                              being requested about. If BlobType equals
+                              KernelBlobTypeMax, then information will be
+                              provided about the root directory of the
+                              filesystem.
+
+  @param[in,out] BufferSize  On input, the size of Buffer. On output, the
+                             amount of data returned in Buffer. In both cases,
+                             the size is measured in bytes.
+
+  @param[out]    Buffer      A pointer to the data buffer to return. The
+                             buffer's type is EFI_FILE_INFO.
+
+  @retval EFI_SUCCESS           The information was returned.
+  @retval EFI_BUFFER_TOO_SMALL  BufferSize is too small to store the
+                                EFI_FILE_INFO structure. BufferSize has been
+                                updated with the size needed to complete the
+                                request.
+**/
+EFI_STATUS
+ConvertKernelBlobTypeToFileInfo (
+  IN KERNEL_BLOB_TYPE  BlobType,
+  IN OUT UINTN         *BufferSize,
+  OUT VOID             *Buffer
+  )
+{
+  CONST CHAR16  *Name;
+  UINT64        FileSize;
+  UINT64        Attribute;
+
+  UINTN          NameSize;
+  UINTN          FileInfoSize;
+  EFI_FILE_INFO  *FileInfo;
+  UINTN          OriginalBufferSize;
+
+  if (BlobType == KernelBlobTypeMax) {
+    //
+    // getting file info about the root directory
+    //
+    Name      = L"\\";
+    FileSize  = KernelBlobTypeMax;
+    Attribute = EFI_FILE_READ_ONLY | EFI_FILE_DIRECTORY;
+  } else {
+    CONST KERNEL_BLOB  *Blob;
+
+    Blob      = &mKernelBlob[BlobType];
+    Name      = Blob->Name;
+    FileSize  = Blob->Size;
+    Attribute = EFI_FILE_READ_ONLY;
+  }
+
+  NameSize     = (StrLen (Name) + 1) * 2;
+  FileInfoSize = OFFSET_OF (EFI_FILE_INFO, FileName) + NameSize;
+  ASSERT (FileInfoSize >= sizeof *FileInfo);
+
+  OriginalBufferSize = *BufferSize;
+  *BufferSize        = FileInfoSize;
+  if (OriginalBufferSize < *BufferSize) {
+    return EFI_BUFFER_TOO_SMALL;
+  }
+
+  FileInfo               = (EFI_FILE_INFO *)Buffer;
+  FileInfo->Size         = FileInfoSize;
+  FileInfo->FileSize     = FileSize;
+  FileInfo->PhysicalSize = FileSize;
+  FileInfo->Attribute    = Attribute;
+
+  CopyMem (&FileInfo->CreateTime, &mInitTime, sizeof mInitTime);
+  CopyMem (&FileInfo->LastAccessTime, &mInitTime, sizeof mInitTime);
+  CopyMem (&FileInfo->ModificationTime, &mInitTime, sizeof mInitTime);
+  CopyMem (FileInfo->FileName, Name, NameSize);
+
+  return EFI_SUCCESS;
+}
+
+/**
+  Reads data from a file, or continues scanning a directory.
+
+  @param[in]     This        A pointer to the EFI_FILE_PROTOCOL instance that
+                             is the file handle to read data from.
+
+  @param[in,out] BufferSize  On input, the size of the Buffer. On output, the
+                             amount of data returned in Buffer. In both cases,
+                             the size is measured in bytes. If the read goes
+                             beyond the end of the file, the read length is
+                             truncated to the end of the file.
+
+                             If This is a directory, the function reads the
+                             directory entry at the current position and
+                             returns the entry (as EFI_FILE_INFO) in Buffer. If
+                             there are no more directory entries, the
+                             BufferSize is set to zero on output.
+
+  @param[out]    Buffer      The buffer into which the data is read.
+
+  @retval EFI_SUCCESS           Data was read.
+  @retval EFI_NO_MEDIA          The device has no medium.
+  @retval EFI_DEVICE_ERROR      The device reported an error.
+  @retval EFI_DEVICE_ERROR      An attempt was made to read from a deleted
+                                file.
+  @retval EFI_DEVICE_ERROR      On entry, the current file position is beyond
+                                the end of the file.
+  @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.
+  @retval EFI_BUFFER_TOO_SMALL  The BufferSize is too small to store the
+                                current directory entry as a EFI_FILE_INFO
+                                structure. BufferSize has been updated with the
+                                size needed to complete the request, and the
+                                directory position has not been advanced.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+StubFileRead (
+  IN EFI_FILE_PROTOCOL  *This,
+  IN OUT UINTN          *BufferSize,
+  OUT VOID              *Buffer
+  )
+{
+  STUB_FILE          *StubFile;
+  CONST KERNEL_BLOB  *Blob;
+  UINT64             Left;
+
+  StubFile = STUB_FILE_FROM_FILE (This);
+
+  //
+  // Scanning the root directory?
+  //
+  if (StubFile->BlobType == KernelBlobTypeMax) {
+    EFI_STATUS  Status;
+
+    if (StubFile->Position == KernelBlobTypeMax) {
+      //
+      // Scanning complete.
+      //
+      *BufferSize = 0;
+      return EFI_SUCCESS;
+    }
+
+    Status = ConvertKernelBlobTypeToFileInfo (
+               (KERNEL_BLOB_TYPE)StubFile->Position,
+               BufferSize,
+               Buffer
+               );
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+
+    ++StubFile->Position;
+    return EFI_SUCCESS;
+  }
+
+  //
+  // Reading a file.
+  //
+  Blob = &mKernelBlob[StubFile->BlobType];
+  if (StubFile->Position > Blob->Size) {
+    return EFI_DEVICE_ERROR;
+  }
+
+  Left = Blob->Size - StubFile->Position;
+  if (*BufferSize > Left) {
+    *BufferSize = (UINTN)Left;
+  }
+
+  if (Blob->Data != NULL) {
+    CopyMem (Buffer, Blob->Data + StubFile->Position, *BufferSize);
+  }
+
+  StubFile->Position += *BufferSize;
+  return EFI_SUCCESS;
+}
+
+/**
+  Closes a specified file handle.
+
+  @param[in] This  A pointer to the EFI_FILE_PROTOCOL instance that is the file
+                   handle to close.
+
+  @retval EFI_SUCCESS  The file was closed.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+StubFileClose (
+  IN EFI_FILE_PROTOCOL  *This
+  )
+{
+  FreePool (STUB_FILE_FROM_FILE (This));
+  return EFI_SUCCESS;
+}
+
+/**
+  Close and delete the file handle.
+
+  @param[in] This  A pointer to the EFI_FILE_PROTOCOL instance that is the
+                   handle to the file to delete.
+
+  @retval EFI_SUCCESS              The file was closed and deleted, and the
+                                   handle was closed.
+  @retval EFI_WARN_DELETE_FAILURE  The handle was closed, but the file was not
+                                   deleted.
+
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+StubFileDelete (
+  IN EFI_FILE_PROTOCOL  *This
+  )
+{
+  FreePool (STUB_FILE_FROM_FILE (This));
+  return EFI_WARN_DELETE_FAILURE;
+}
+
+//
+// Protocol member functions for File.
+//
+
+/**
+  Opens a new file relative to the source file's location.
+
+  (Forward declaration.)
+
+  @param[in]  This        A pointer to the EFI_FILE_PROTOCOL instance that is
+                          the file handle to the source location. This would
+                          typically be an open handle to a directory.
+
+  @param[out] NewHandle   A pointer to the location to return the opened handle
+                          for the new file.
+
+  @param[in]  FileName    The Null-terminated string of the name of the file to
+                          be opened. The file name may contain the following
+                          path modifiers: "\", ".", and "..".
+
+  @param[in]  OpenMode    The mode to open the file. The only valid
+                          combinations that the file may be opened with are:
+                          Read, Read/Write, or Create/Read/Write.
+
+  @param[in]  Attributes  Only valid for EFI_FILE_MODE_CREATE, in which case
+                          these are the attribute bits for the newly created
+                          file.
+
+  @retval EFI_SUCCESS           The file was opened.
+  @retval EFI_NOT_FOUND         The specified file could not be found on the
+                                device.
+  @retval EFI_NO_MEDIA          The device has no medium.
+  @retval EFI_MEDIA_CHANGED     The device has a different medium in it or the
+                                medium is no longer supported.
+  @retval EFI_DEVICE_ERROR      The device reported an error.
+  @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.
+  @retval EFI_WRITE_PROTECTED   An attempt was made to create a file, or open a
+                                file for write when the media is
+                                write-protected.
+  @retval EFI_ACCESS_DENIED     The service denied access to the file.
+  @retval EFI_OUT_OF_RESOURCES  Not enough resources were available to open the
+                                file.
+  @retval EFI_VOLUME_FULL       The volume is full.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+StubFileOpen (
+  IN EFI_FILE_PROTOCOL   *This,
+  OUT EFI_FILE_PROTOCOL  **NewHandle,
+  IN CHAR16              *FileName,
+  IN UINT64              OpenMode,
+  IN UINT64              Attributes
+  );
+
+/**
+  Returns information about a file.
+
+  @param[in]     This             A pointer to the EFI_FILE_PROTOCOL instance
+                                  that is the file handle the requested
+                                  information is for.
+
+  @param[in]     InformationType  The type identifier GUID for the information
+                                  being requested. The following information
+                                  types are supported, storing the
+                                  corresponding structures in Buffer:
+
+                                  - gEfiFileInfoGuid: EFI_FILE_INFO
+
+                                  - gEfiFileSystemInfoGuid:
+                                    EFI_FILE_SYSTEM_INFO
+
+                                  - gEfiFileSystemVolumeLabelInfoIdGuid:
+                                    EFI_FILE_SYSTEM_VOLUME_LABEL
+
+  @param[in,out] BufferSize       On input, the size of Buffer. On output, the
+                                  amount of data returned in Buffer. In both
+                                  cases, the size is measured in bytes.
+
+  @param[out]    Buffer           A pointer to the data buffer to return. The
+                                  buffer's type is indicated by
+                                  InformationType.
+
+  @retval EFI_SUCCESS           The information was returned.
+  @retval EFI_UNSUPPORTED       The InformationType is not known.
+  @retval EFI_NO_MEDIA          The device has no medium.
+  @retval EFI_DEVICE_ERROR      The device reported an error.
+  @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.
+  @retval EFI_BUFFER_TOO_SMALL  The BufferSize is too small to store the
+                                information structure requested by
+                                InformationType. BufferSize has been updated
+                                with the size needed to complete the request.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+StubFileGetInfo (
+  IN EFI_FILE_PROTOCOL  *This,
+  IN EFI_GUID           *InformationType,
+  IN OUT UINTN          *BufferSize,
+  OUT VOID              *Buffer
+  )
+{
+  CONST STUB_FILE  *StubFile;
+  UINTN            OriginalBufferSize;
+
+  StubFile = STUB_FILE_FROM_FILE (This);
+
+  if (CompareGuid (InformationType, &gEfiFileInfoGuid)) {
+    return ConvertKernelBlobTypeToFileInfo (
+             StubFile->BlobType,
+             BufferSize,
+             Buffer
+             );
+  }
+
+  OriginalBufferSize = *BufferSize;
+
+  if (CompareGuid (InformationType, &gEfiFileSystemInfoGuid)) {
+    EFI_FILE_SYSTEM_INFO  *FileSystemInfo;
+
+    *BufferSize = sizeof *FileSystemInfo;
+    if (OriginalBufferSize < *BufferSize) {
+      return EFI_BUFFER_TOO_SMALL;
+    }
+
+    FileSystemInfo                 = (EFI_FILE_SYSTEM_INFO *)Buffer;
+    FileSystemInfo->Size           = sizeof *FileSystemInfo;
+    FileSystemInfo->ReadOnly       = TRUE;
+    FileSystemInfo->VolumeSize     = mTotalBlobBytes;
+    FileSystemInfo->FreeSpace      = 0;
+    FileSystemInfo->BlockSize      = 1;
+    FileSystemInfo->VolumeLabel[0] = L'\0';
+
+    return EFI_SUCCESS;
+  }
+
+  if (CompareGuid (InformationType, &gEfiFileSystemVolumeLabelInfoIdGuid)) {
+    EFI_FILE_SYSTEM_VOLUME_LABEL  *FileSystemVolumeLabel;
+
+    *BufferSize = sizeof *FileSystemVolumeLabel;
+    if (OriginalBufferSize < *BufferSize) {
+      return EFI_BUFFER_TOO_SMALL;
+    }
+
+    FileSystemVolumeLabel                 = (EFI_FILE_SYSTEM_VOLUME_LABEL *)Buffer;
+    FileSystemVolumeLabel->VolumeLabel[0] = L'\0';
+
+    return EFI_SUCCESS;
+  }
+
+  return EFI_UNSUPPORTED;
+}
+
+//
+// External definition of the file protocol template.
+//
+STATIC CONST EFI_FILE_PROTOCOL  mEfiFileProtocolTemplate = {
+  EFI_FILE_PROTOCOL_REVISION, // revision 1
+  StubFileOpen,
+  StubFileClose,
+  StubFileDelete,
+  StubFileRead,
+  NULL,
+  NULL,
+  NULL,
+  StubFileGetInfo,
+  NULL,
+  NULL,
+  NULL,                       // OpenEx, revision 2
+  NULL,                       // ReadEx, revision 2
+  NULL,                       // WriteEx, revision 2
+  NULL                        // FlushEx, revision 2
+};
+
+/**
+  Opens a new file relative to the source file's location.
+
+  (Forward declaration.)
+
+  @param[in]  This        A pointer to the EFI_FILE_PROTOCOL instance that is
+                          the file handle to the source location. This would
+                          typically be an open handle to a directory.
+
+  @param[out] NewHandle   A pointer to the location to return the opened handle
+                          for the new file.
+
+  @param[in]  FileName    The Null-terminated string of the name of the file to
+                          be opened. The file name may contain the following
+                          path modifiers: "\", ".", and "..".
+
+  @param[in]  OpenMode    The mode to open the file. The only valid
+                          combinations that the file may be opened with are:
+                          Read, Read/Write, or Create/Read/Write.
+
+  @param[in]  Attributes  Only valid for EFI_FILE_MODE_CREATE, in which case
+                          these are the attribute bits for the newly created
+                          file.
+
+  @retval EFI_SUCCESS           The file was opened.
+  @retval EFI_NOT_FOUND         The specified file could not be found on the
+                                device.
+  @retval EFI_NO_MEDIA          The device has no medium.
+  @retval EFI_MEDIA_CHANGED     The device has a different medium in it or the
+                                medium is no longer supported.
+  @retval EFI_DEVICE_ERROR      The device reported an error.
+  @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.
+  @retval EFI_WRITE_PROTECTED   An attempt was made to create a file, or open a
+                                file for write when the media is
+                                write-protected.
+  @retval EFI_ACCESS_DENIED     The service denied access to the file.
+  @retval EFI_OUT_OF_RESOURCES  Not enough resources were available to open the
+                                file.
+  @retval EFI_VOLUME_FULL       The volume is full.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+StubFileOpen (
+  IN EFI_FILE_PROTOCOL   *This,
+  OUT EFI_FILE_PROTOCOL  **NewHandle,
+  IN CHAR16              *FileName,
+  IN UINT64              OpenMode,
+  IN UINT64              Attributes
+  )
+{
+  CONST STUB_FILE  *StubFile;
+  UINTN            BlobType;
+  STUB_FILE        *NewStubFile;
+
+  //
+  // We're read-only.
+  //
+  switch (OpenMode) {
+    case EFI_FILE_MODE_READ:
+      break;
+
+    case EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE:
+    case EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE | EFI_FILE_MODE_CREATE:
+      return EFI_WRITE_PROTECTED;
+
+    default:
+      return EFI_INVALID_PARAMETER;
+  }
+
+  //
+  // Only the root directory supports opening files in it.
+  //
+  StubFile = STUB_FILE_FROM_FILE (This);
+  if (StubFile->BlobType != KernelBlobTypeMax) {
+    return EFI_UNSUPPORTED;
+  }
+
+  //
+  // Locate the file.
+  //
+  for (BlobType = 0; BlobType < KernelBlobTypeMax; ++BlobType) {
+    if (StrCmp (FileName, mKernelBlob[BlobType].Name) == 0) {
+      break;
+    }
+  }
+
+  if (BlobType == KernelBlobTypeMax) {
+    return EFI_NOT_FOUND;
+  }
+
+  //
+  // Found it.
+  //
+  NewStubFile = AllocatePool (sizeof *NewStubFile);
+  if (NewStubFile == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  NewStubFile->Signature = STUB_FILE_SIG;
+  NewStubFile->BlobType  = (KERNEL_BLOB_TYPE)BlobType;
+  NewStubFile->Position  = 0;
+  CopyMem (
+    &NewStubFile->File,
+    &mEfiFileProtocolTemplate,
+    sizeof mEfiFileProtocolTemplate
+    );
+  *NewHandle = &NewStubFile->File;
+
+  return EFI_SUCCESS;
+}
+
+//
+// Protocol member functions for SimpleFileSystem.
+//
+
+/**
+  Open the root directory on a volume.
+
+  @param[in]  This  A pointer to the volume to open the root directory on.
+
+  @param[out] Root  A pointer to the location to return the opened file handle
+                    for the root directory in.
+
+  @retval EFI_SUCCESS           The device was opened.
+  @retval EFI_UNSUPPORTED       This volume does not support the requested file
+                                system type.
+  @retval EFI_NO_MEDIA          The device has no medium.
+  @retval EFI_DEVICE_ERROR      The device reported an error.
+  @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.
+  @retval EFI_ACCESS_DENIED     The service denied access to the file.
+  @retval EFI_OUT_OF_RESOURCES  The volume was not opened due to lack of
+                                resources.
+  @retval EFI_MEDIA_CHANGED     The device has a different medium in it or the
+                                medium is no longer supported. Any existing
+                                file handles for this volume are no longer
+                                valid. To access the files on the new medium,
+                                the volume must be reopened with OpenVolume().
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+StubFileSystemOpenVolume (
+  IN EFI_SIMPLE_FILE_SYSTEM_PROTOCOL  *This,
+  OUT EFI_FILE_PROTOCOL               **Root
+  )
+{
+  STUB_FILE  *StubFile;
+
+  StubFile = AllocatePool (sizeof *StubFile);
+  if (StubFile == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  StubFile->Signature = STUB_FILE_SIG;
+  StubFile->BlobType  = KernelBlobTypeMax;
+  StubFile->Position  = 0;
+  CopyMem (
+    &StubFile->File,
+    &mEfiFileProtocolTemplate,
+    sizeof mEfiFileProtocolTemplate
+    );
+  *Root = &StubFile->File;
+
+  return EFI_SUCCESS;
+}
+
+STATIC CONST EFI_SIMPLE_FILE_SYSTEM_PROTOCOL  mFileSystem = {
+  EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_REVISION,
+  StubFileSystemOpenVolume
+};
+
+/**
+  Load initrd data to memory
+
+  @param[in]      This        Not used here.
+  @param[in]      FilePath    Contain some metadata of file device where holds the initrd.
+  @param[in]      BootPolicy  Not support here.
+  @param[in,out]  BufferSize  On input, contains the memory buff size.
+                              On output, contains the initrd blob size.
+  @param[out]     Buffer      A pointer point to memory which contains initrd data.
+
+  @retval EFI_UNSUPPORTED        Once the Boot Policy is true.
+  @retval EFI_INVALID_PARAMETER  Once there is no BuffSize given or no available file device.
+  @retval EFI_NOT_FOUND          File path doesn't match.
+  @retval EFI_BUFFER_TOO_SMALL   Buff size too small.
+  @retval EFI_SUCCESS            Success.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+InitrdLoadFile2 (
+  IN      EFI_LOAD_FILE2_PROTOCOL   *This,
+  IN      EFI_DEVICE_PATH_PROTOCOL  *FilePath,
+  IN      BOOLEAN                   BootPolicy,
+  IN  OUT UINTN                     *BufferSize,
+  OUT     VOID                      *Buffer     OPTIONAL
+  )
+{
+  CONST KERNEL_BLOB  *InitrdBlob = &mKernelBlob[KernelBlobTypeInitrd];
+
+  ASSERT (InitrdBlob->Size > 0);
+
+  if (BootPolicy) {
+    return EFI_UNSUPPORTED;
+  }
+
+  if ((BufferSize == NULL) || !IsDevicePathValid (FilePath, 0)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if ((FilePath->Type != END_DEVICE_PATH_TYPE) ||
+      (FilePath->SubType != END_ENTIRE_DEVICE_PATH_SUBTYPE))
+  {
+    return EFI_NOT_FOUND;
+  }
+
+  if ((Buffer == NULL) || (*BufferSize < InitrdBlob->Size)) {
+    *BufferSize = InitrdBlob->Size;
+    return EFI_BUFFER_TOO_SMALL;
+  }
+
+  CopyMem (Buffer, InitrdBlob->Data, InitrdBlob->Size);
+
+  *BufferSize = InitrdBlob->Size;
+  return EFI_SUCCESS;
+}
+
+STATIC CONST EFI_LOAD_FILE2_PROTOCOL  mInitrdLoadFile2 = {
+  InitrdLoadFile2,
+};
+
+/**
+  Fetch kernel, initrd and commandline into Blob.
+
+  @param Blob    On input, choose which blob to be fetched.
+                 On output, carry the full blob info.
+
+  @retval EFI_NOT_FOUND    Kernel is not fetched or no chosen node found in DT.
+  @retval EFI_SUCCESS      If there is kernel fetched or the Block->Name is not kernel.
+
+**/
+STATIC
+EFI_STATUS
+FetchBlob (
+  IN OUT KERNEL_BLOB  *Blob
+  )
+{
+  VOID          *DeviceTreeBase;
+  UINT64        InitrdStart, InitrdEnd;
+  INT32         Len;
+  CONST UINT64  *Prop;
+  INT32         ChosenNode;
+
+  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
+  ASSERT (DeviceTreeBase != NULL);
+
+  Blob->Size = 0;
+
+  //
+  // Make sure we have a valid device tree blob
+  //
+  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
+
+  //
+  // Find chosen node
+  //
+  ChosenNode = fdt_path_offset (DeviceTreeBase, "/chosen");
+  if (ChosenNode < 0) {
+    return EFI_NOT_FOUND;
+  }
+
+  switch (Blob->Name[0]) {
+    case 'k':
+      Prop = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,kernel-start", &Len);
+      if ((Prop == NULL) || (Len < 0)) {
+        return EFI_NOT_FOUND;
+      }
+
+      Blob->Data = (UINT8 *)fdt64_to_cpu (ReadUnaligned64 (Prop));
+      Prop       = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,kernel-size", &Len);
+      if ((Prop == NULL) || (Len < 0)) {
+        return EFI_SUCCESS;
+      }
+
+      Blob->Size = fdt64_to_cpu (ReadUnaligned64 (Prop));
+      break;
+    case 'i':
+      Prop = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,initrd-start", &Len);
+      if ((Prop == NULL) || (Len < 0)) {
+        return EFI_SUCCESS;
+      }
+
+      InitrdStart = fdt64_to_cpu (ReadUnaligned64 (Prop));
+      Blob->Data  = (UINT8 *)InitrdStart;
+      Prop        = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,initrd-end", &Len);
+      if ((Prop == NULL) || (Len < 0)) {
+        return EFI_SUCCESS;
+      }
+
+      InitrdEnd  = fdt64_to_cpu (ReadUnaligned64 (Prop));
+      Blob->Size = InitrdEnd - InitrdStart;
+      break;
+    case 'c':
+      Prop = fdt_getprop (DeviceTreeBase, ChosenNode, "bootargs", &Len);
+      if ((Prop == NULL) || (Len < 0)) {
+        return EFI_SUCCESS;
+      }
+
+      Blob->Data = (UINT8 *)Prop;
+      Blob->Size = Len;
+      break;
+    default:
+      return EFI_SUCCESS;
+  }
+
+  return EFI_SUCCESS;
+}
+
+//
+// The entry point of the feature.
+//
+
+/**
+  Download the kernel, the initial ramdisk, and the kernel command line from
+  memory or DT. Construct a minimal SimpleFileSystem that contains the two
+  image files.
+
+  @param ImageHandle  Image handle.
+  @param SystemTable  EFI system table.
+
+  @return                       Error codes from any of the underlying
+                                functions. On success, the function doesn't
+                                return.
+**/
+EFI_STATUS
+EFIAPI
+CloudHvKernelLoaderFsDxeEntrypoint (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  UINTN        BlobType;
+  KERNEL_BLOB  *CurrentBlob;
+  KERNEL_BLOB  *KernelBlob;
+  EFI_STATUS   Status;
+  EFI_HANDLE   FileSystemHandle;
+  EFI_HANDLE   InitrdLoadFile2Handle;
+
+  Status = gRT->GetTime (&mInitTime, NULL /* Capabilities */);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a: GetTime(): %r\n", __FUNCTION__, Status));
+    return Status;
+  }
+
+  //
+  // Fetch all blobs.
+  //
+  for (BlobType = 0; BlobType < KernelBlobTypeMax; ++BlobType) {
+    CurrentBlob = &mKernelBlob[BlobType];
+    Status      = FetchBlob (CurrentBlob);
+    if (EFI_ERROR (Status)) {
+      goto FreeBlobs;
+    }
+
+    Status = VerifyBlob (
+               CurrentBlob->Name,
+               CurrentBlob->Data,
+               CurrentBlob->Size
+               );
+    if (EFI_ERROR (Status)) {
+      goto FreeBlobs;
+    }
+
+    mTotalBlobBytes += CurrentBlob->Size;
+  }
+
+  KernelBlob = &mKernelBlob[KernelBlobTypeKernel];
+
+  if (KernelBlob->Data == NULL) {
+    Status = EFI_NOT_FOUND;
+    goto FreeBlobs;
+  }
+
+  //
+  // Create a new handle with a single VenMedia() node device path protocol on
+  // it, plus a custom SimpleFileSystem protocol on it.
+  //
+  FileSystemHandle = NULL;
+  Status           = gBS->InstallMultipleProtocolInterfaces (
+                            &FileSystemHandle,
+                            &gEfiDevicePathProtocolGuid,
+                            &mFileSystemDevicePath,
+                            &gEfiSimpleFileSystemProtocolGuid,
+                            &mFileSystem,
+                            NULL
+                            );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: InstallMultipleProtocolInterfaces(): %r\n",
+      __FUNCTION__,
+      Status
+      ));
+    goto FreeBlobs;
+  }
+
+  if (KernelBlob[KernelBlobTypeInitrd].Size > 0) {
+    InitrdLoadFile2Handle = NULL;
+    Status                = gBS->InstallMultipleProtocolInterfaces (
+                                   &InitrdLoadFile2Handle,
+                                   &gEfiDevicePathProtocolGuid,
+                                   &mInitrdDevicePath,
+                                   &gEfiLoadFile2ProtocolGuid,
+                                   &mInitrdLoadFile2,
+                                   NULL
+                                   );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "%a: InstallMultipleProtocolInterfaces(): %r\n",
+        __FUNCTION__,
+        Status
+        ));
+      goto UninstallFileSystemHandle;
+    }
+  }
+
+  return EFI_SUCCESS;
+
+UninstallFileSystemHandle:
+  Status = gBS->UninstallMultipleProtocolInterfaces (
+                  FileSystemHandle,
+                  &gEfiDevicePathProtocolGuid,
+                  &mFileSystemDevicePath,
+                  &gEfiSimpleFileSystemProtocolGuid,
+                  &mFileSystem,
+                  NULL
+                  );
+  ASSERT_EFI_ERROR (Status);
+
+FreeBlobs:
+  while (BlobType > 0) {
+    CurrentBlob = &mKernelBlob[--BlobType];
+    if (CurrentBlob->Data != NULL) {
+      FreePool (CurrentBlob->Data);
+      CurrentBlob->Size = 0;
+      CurrentBlob->Data = NULL;
+    }
+  }
+
+  return Status;
+}
diff --git a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
new file mode 100644
index 0000000000..b7aa6ebb4e
--- /dev/null
+++ b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
@@ -0,0 +1,56 @@
+##  @file
+#  DXE driver to expose the 'kernel', 'initrd' and 'cmdline' blobs
+#  provided by Cloud Hypervisor as files in an abstract file system
+#
+#  Copyright (C) 2022, Arm, Limited.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001B
+  BASE_NAME                      = CloudHvKernelLoaderFsDxe
+  FILE_GUID                      = 57cdf541-2a29-4ae4-97e5-76a4aa2fe090
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = CloudHvKernelLoaderFsDxeEntrypoint
+
+[Sources]
+  CloudHvKernelLoaderFsDxe.c
+
+[Packages]
+  ArmVirtPkg/ArmVirtPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  DevicePathLib
+  FdtLib
+  MemoryAllocationLib
+  PcdLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+  UefiRuntimeServicesTableLib
+
+[Guids]
+  gEfiFileInfoGuid
+  gEfiFileSystemInfoGuid
+  gEfiFileSystemVolumeLabelInfoIdGuid
+  gQemuKernelLoaderFsMediaGuid
+
+[Protocols]
+  gEfiDevicePathProtocolGuid                ## PRODUCES
+  gEfiLoadFile2ProtocolGuid                 ## PRODUCES
+  gEfiSimpleFileSystemProtocolGuid          ## PRODUCES
+
+[Depex]
+  gEfiRealTimeClockArchProtocolGuid
+
+[FixedPcd]
+  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/3] CloudHv:arm: build hob for kernel image memory as read-only
  2022-09-16  2:46 [PATCH 0/3] CloudHv:arm: Enable direct kernel boot Jianyong Wu
  2022-09-16  2:46 ` [PATCH 1/3] CloudHv:arm: add kernel load fs driver Jianyong Wu
@ 2022-09-16  2:46 ` Jianyong Wu
  2022-11-22 15:47   ` Sami Mujawar
  2022-09-16  2:46 ` [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf Jianyong Wu
  2022-11-15  3:00 ` [PATCH 0/3] CloudHv:arm: Enable direct kernel boot Jianyong Wu
  3 siblings, 1 reply; 19+ messages in thread
From: Jianyong Wu @ 2022-09-16  2:46 UTC (permalink / raw)
  To: devel, Sami.Mujawar; +Cc: ardb+tianocore, justin.he, jianyong.wu

As we use memory to pass kernel image, the memory region where kernel
image locates should be added into hob as read-only.

Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
---
 .../CloudHvVirtMemInfoLib.c                   | 66 +++++++++++++++++--
 1 file changed, 62 insertions(+), 4 deletions(-)

diff --git a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
index 28a0c0b078..d9b7d51a16 100644
--- a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
+++ b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
@@ -37,13 +37,14 @@ CloudHvVirtMemInfoPeiLibConstructor (
   )
 {
   VOID                         *DeviceTreeBase;
-  EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
+  EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes, ReadOnlyResourceAttributes;
   INT32                        Node, Prev;
   UINT64                       FirMemNodeBase, FirMemNodeSize;
-  UINT64                       CurBase, MemBase;
+  UINT64                       CurBase, MemBase, CurSizeOff;
   UINT64                       CurSize;
+  UINT64                       KernelStart, KernelSize;
   CONST CHAR8                  *Type;
-  INT32                        Len;
+  INT32                        Len, ChosenNode;
   CONST UINT64                 *RegProp;
   RETURN_STATUS                PcdStatus;
   UINT8                        Index;
@@ -53,6 +54,8 @@ CloudHvVirtMemInfoPeiLibConstructor (
   FirMemNodeBase     = 0;
   FirMemNodeSize     = 0;
   Index              = 0;
+  CurSizeOff         = 0;
+  KernelSize         = 0;
   MemBase            = FixedPcdGet64 (PcdSystemMemoryBase);
   ResourceAttributes = (
                         EFI_RESOURCE_ATTRIBUTE_PRESENT |
@@ -60,6 +63,12 @@ CloudHvVirtMemInfoPeiLibConstructor (
                         EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
                         EFI_RESOURCE_ATTRIBUTE_TESTED
                         );
+  ReadOnlyResourceAttributes = (
+                                EFI_RESOURCE_ATTRIBUTE_PRESENT |
+                                EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
+                                EFI_RESOURCE_ATTRIBUTE_TESTED |
+                                EFI_RESOURCE_ATTRIBUTE_READ_ONLY_PROTECTED
+                                );
   DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
   if (DeviceTreeBase == NULL) {
     return EFI_NOT_FOUND;
@@ -72,6 +81,21 @@ CloudHvVirtMemInfoPeiLibConstructor (
     return EFI_NOT_FOUND;
   }
 
+  //
+  // Try to get kernel image info from DT
+  //
+  ChosenNode = fdt_path_offset (DeviceTreeBase, "/chosen");
+  if (ChosenNode >= 0) {
+    RegProp = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,kernel-start", &Len);
+    if ((RegProp != NULL) && (Len > 0)) {
+      KernelStart = (UINT64)fdt64_to_cpu (ReadUnaligned64 (RegProp));
+      RegProp     = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,kernel-size", &Len);
+      if ((RegProp != NULL) && (Len > 0)) {
+        KernelSize = (UINT64)fdt64_to_cpu (ReadUnaligned64 (RegProp));
+      }
+    }
+  }
+
   //
   // Look for the lowest memory node
   //
@@ -105,11 +129,26 @@ CloudHvVirtMemInfoPeiLibConstructor (
 
         // We should build Hob seperately for the memory node except the first one
         if (CurBase != MemBase) {
+          // If kernel image resides in current memory node, build hob from CurBase to the beginning of kernel image.
+          if ((KernelSize != 0) && (KernelStart >= CurBase) && (KernelStart + KernelSize <= CurBase + CurSize)) {
+            CurSizeOff =  CurBase + CurSize - KernelStart;
+            // align up with 0x1000
+            CurSizeOff = (CurSizeOff + 0xfff) & ~0xfffUL;
+          }
+
           BuildResourceDescriptorHob (
             EFI_RESOURCE_SYSTEM_MEMORY,
             ResourceAttributes,
             CurBase,
-            CurSize
+            CurSize - CurSizeOff
+            );
+
+          // Add kernel image memory region to hob as read only
+          BuildResourceDescriptorHob (
+            EFI_RESOURCE_SYSTEM_MEMORY,
+            ReadOnlyResourceAttributes,
+            CurBase + CurSize - CurSizeOff,
+            CurSizeOff
             );
         } else {
           FirMemNodeBase = CurBase;
@@ -146,8 +185,27 @@ CloudHvVirtMemInfoPeiLibConstructor (
     return EFI_NOT_FOUND;
   }
 
+  CurSizeOff = 0;
+  // Build hob for the lowest memory node from its base to the beginning of kernel image once the kernel image reside here
+  if ((KernelSize != 0) && (KernelStart >= FirMemNodeBase) && (KernelStart + KernelSize <= FirMemNodeBase + FirMemNodeSize)) {
+    CurSizeOff = FirMemNodeBase + FirMemNodeSize - KernelStart;
+    // Caution the alignment
+    CurSizeOff = (CurSizeOff + 0xfff) & ~0xfffUL;
+
+    // Add kernel image memory region to hob as read only
+    BuildResourceDescriptorHob (
+      EFI_RESOURCE_SYSTEM_MEMORY,
+      ReadOnlyResourceAttributes,
+      FirMemNodeBase + FirMemNodeSize - CurSizeOff,
+      CurSizeOff
+      );
+  }
+
+  FirMemNodeSize -= CurSizeOff;
+
   PcdStatus = PcdSet64S (PcdSystemMemorySize, FirMemNodeSize);
   ASSERT_RETURN_ERROR (PcdStatus);
+
   ASSERT (
     (((UINT64)PcdGet64 (PcdFdBaseAddress) +
       (UINT64)PcdGet32 (PcdFdSize)) <= FirMemNodeBase) ||
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf
  2022-09-16  2:46 [PATCH 0/3] CloudHv:arm: Enable direct kernel boot Jianyong Wu
  2022-09-16  2:46 ` [PATCH 1/3] CloudHv:arm: add kernel load fs driver Jianyong Wu
  2022-09-16  2:46 ` [PATCH 2/3] CloudHv:arm: build hob for kernel image memory as read-only Jianyong Wu
@ 2022-09-16  2:46 ` Jianyong Wu
  2022-11-22 15:48   ` Sami Mujawar
  2022-11-15  3:00 ` [PATCH 0/3] CloudHv:arm: Enable direct kernel boot Jianyong Wu
  3 siblings, 1 reply; 19+ messages in thread
From: Jianyong Wu @ 2022-09-16  2:46 UTC (permalink / raw)
  To: devel, Sami.Mujawar; +Cc: ardb+tianocore, justin.he, jianyong.wu

As CloudHv kernel load fs driver is implemented, add it into dsc/fdf.

Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
---
 ArmVirtPkg/ArmVirtCloudHv.dsc                             | 8 +++++++-
 ArmVirtPkg/ArmVirtCloudHv.fdf                             | 1 +
 .../CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf | 1 -
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc
index 7ca7a391d9..92ccd4ef12 100644
--- a/ArmVirtPkg/ArmVirtCloudHv.dsc
+++ b/ArmVirtPkg/ArmVirtCloudHv.dsc
@@ -37,13 +37,15 @@
   # Virtio Support
   VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
   VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
+  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
+  QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
 
   ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
 
   TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
-  PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+  PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
   PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
   CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
   FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
@@ -330,6 +332,10 @@
       NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf
       NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
   }
+  ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf {
+    <LibraryClasses>
+      NULL|OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierLibNull.inf
+  }
 
   #
   # SCSI Bus and Disk Driver
diff --git a/ArmVirtPkg/ArmVirtCloudHv.fdf b/ArmVirtPkg/ArmVirtCloudHv.fdf
index 81c539590a..15b9c13c59 100644
--- a/ArmVirtPkg/ArmVirtCloudHv.fdf
+++ b/ArmVirtPkg/ArmVirtCloudHv.fdf
@@ -180,6 +180,7 @@ READ_LOCK_STATUS   = TRUE
   INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
   INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
   INF MdeModulePkg/Application/UiApp/UiApp.inf
+  INF ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
 
   #
   # SCSI Bus and Disk Driver
diff --git a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
index b7aa6ebb4e..f7b53d0747 100644
--- a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
+++ b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
@@ -24,7 +24,6 @@
   EmbeddedPkg/EmbeddedPkg.dec
   MdePkg/MdePkg.dec
   OvmfPkg/OvmfPkg.dec
-  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
 
 [LibraryClasses]
   BaseLib
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/3] CloudHv:arm: Enable direct kernel boot
  2022-09-16  2:46 [PATCH 0/3] CloudHv:arm: Enable direct kernel boot Jianyong Wu
                   ` (2 preceding siblings ...)
  2022-09-16  2:46 ` [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf Jianyong Wu
@ 2022-11-15  3:00 ` Jianyong Wu
  2022-11-15  8:51   ` Sami Mujawar
  3 siblings, 1 reply; 19+ messages in thread
From: Jianyong Wu @ 2022-11-15  3:00 UTC (permalink / raw)
  To: Jianyong Wu, devel@edk2.groups.io, Sami Mujawar
  Cc: ardb+tianocore@kernel.org, Justin He

Hello,

Long time silent. Any comments?

Thanks
Jianyong

> -----Original Message-----
> From: Jianyong Wu <jianyong.wu@arm.com>
> Sent: Friday, September 16, 2022 10:46 AM
> To: devel@edk2.groups.io; Sami Mujawar <Sami.Mujawar@arm.com>
> Cc: ardb+tianocore@kernel.org; Justin He <Justin.He@arm.com>; Jianyong
> Wu <Jianyong.Wu@arm.com>
> Subject: [PATCH 0/3] CloudHv:arm: Enable direct kernel boot
>
> Direct kernel boot removes the dependency of retrieving kernel image from
> block device. For Cloud Hypervisor, we use the following way to support it.
>
> 1. Cloud Hypervisor store kernel image into memory and put kernel info,
>    including the memory base and size, into DT; 2. When init memory in edk2,
> the kernel memory region is retrieved from
>    DT and set it as read only memory region; 3. Edk2 fetches kernel from
> memory and prepare a image handle; 4. Load kernel using LoadImage in the
> end.
>
> 1 is done in Cloud Hypervisor, 2 and 3 is done in this patch set, 4 is not
> affected.
>
> github PR link: https://github.com/tianocore/edk2/pull/3339
>
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
>
> Jianyong Wu (3):
>   CloudHv:arm: add kernel load fs driver
>   CloudHv:arm: build hob for kernel image memory as read-only
>   CloudHv:arm: add kernel loader lib dsc/fdf
>
>  ArmVirtPkg/ArmVirtCloudHv.dsc                 |   8 +-
>  ArmVirtPkg/ArmVirtCloudHv.fdf                 |   1 +
>  .../CloudHvKernelLoaderFsDxe.c                | 969 ++++++++++++++++++
>  .../CloudHvKernelLoaderFsDxe.inf              |  55 +
>  .../CloudHvVirtMemInfoLib.c                   |  66 +-
>  5 files changed, 1094 insertions(+), 5 deletions(-)  create mode 100644
> ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c
>  create mode 100644
> ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
>
> --
> 2.17.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/3] CloudHv:arm: Enable direct kernel boot
  2022-11-15  3:00 ` [PATCH 0/3] CloudHv:arm: Enable direct kernel boot Jianyong Wu
@ 2022-11-15  8:51   ` Sami Mujawar
  2022-11-15  9:01     ` Jianyong Wu
  0 siblings, 1 reply; 19+ messages in thread
From: Sami Mujawar @ 2022-11-15  8:51 UTC (permalink / raw)
  To: Jianyong Wu, devel@edk2.groups.io
  Cc: ardb+tianocore@kernel.org, Justin He, nd

Hi Jianyong,

Apologies this slipped off my radar. 
I will start reviewing your patches this week.

Regards,

Sami Mujawar

On 15/11/2022, 03:00, "Jianyong Wu" <Jianyong.Wu@arm.com> wrote:

    Hello,

    Long time silent. Any comments?

    Thanks
    Jianyong

    > -----Original Message-----
    > From: Jianyong Wu <jianyong.wu@arm.com>
    > Sent: Friday, September 16, 2022 10:46 AM
    > To: devel@edk2.groups.io; Sami Mujawar <Sami.Mujawar@arm.com>
    > Cc: ardb+tianocore@kernel.org; Justin He <Justin.He@arm.com>; Jianyong
    > Wu <Jianyong.Wu@arm.com>
    > Subject: [PATCH 0/3] CloudHv:arm: Enable direct kernel boot
    >
    > Direct kernel boot removes the dependency of retrieving kernel image from
    > block device. For Cloud Hypervisor, we use the following way to support it.
    >
    > 1. Cloud Hypervisor store kernel image into memory and put kernel info,
    >    including the memory base and size, into DT; 2. When init memory in edk2,
    > the kernel memory region is retrieved from
    >    DT and set it as read only memory region; 3. Edk2 fetches kernel from
    > memory and prepare a image handle; 4. Load kernel using LoadImage in the
    > end.
    >
    > 1 is done in Cloud Hypervisor, 2 and 3 is done in this patch set, 4 is not
    > affected.
    >
    > github PR link: https://github.com/tianocore/edk2/pull/3339
    >
    > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
    >
    > Jianyong Wu (3):
    >   CloudHv:arm: add kernel load fs driver
    >   CloudHv:arm: build hob for kernel image memory as read-only
    >   CloudHv:arm: add kernel loader lib dsc/fdf
    >
    >  ArmVirtPkg/ArmVirtCloudHv.dsc                 |   8 +-
    >  ArmVirtPkg/ArmVirtCloudHv.fdf                 |   1 +
    >  .../CloudHvKernelLoaderFsDxe.c                | 969 ++++++++++++++++++
    >  .../CloudHvKernelLoaderFsDxe.inf              |  55 +
    >  .../CloudHvVirtMemInfoLib.c                   |  66 +-
    >  5 files changed, 1094 insertions(+), 5 deletions(-)  create mode 100644
    > ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c
    >  create mode 100644
    > ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
    >
    > --
    > 2.17.1



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/3] CloudHv:arm: Enable direct kernel boot
  2022-11-15  8:51   ` Sami Mujawar
@ 2022-11-15  9:01     ` Jianyong Wu
  0 siblings, 0 replies; 19+ messages in thread
From: Jianyong Wu @ 2022-11-15  9:01 UTC (permalink / raw)
  To: Sami Mujawar, devel@edk2.groups.io
  Cc: ardb+tianocore@kernel.org, Justin He, nd

Thanks Sami!

> -----Original Message-----
> From: Sami Mujawar <Sami.Mujawar@arm.com>
> Sent: Tuesday, November 15, 2022 4:52 PM
> To: Jianyong Wu <Jianyong.Wu@arm.com>; devel@edk2.groups.io
> Cc: ardb+tianocore@kernel.org; Justin He <Justin.He@arm.com>; nd
> <nd@arm.com>
> Subject: Re: [PATCH 0/3] CloudHv:arm: Enable direct kernel boot
> 
> Hi Jianyong,
> 
> Apologies this slipped off my radar.
> I will start reviewing your patches this week.
> 
> Regards,
> 
> Sami Mujawar
> 
> On 15/11/2022, 03:00, "Jianyong Wu" <Jianyong.Wu@arm.com> wrote:
> 
>     Hello,
> 
>     Long time silent. Any comments?
> 
>     Thanks
>     Jianyong
> 
>     > -----Original Message-----
>     > From: Jianyong Wu <jianyong.wu@arm.com>
>     > Sent: Friday, September 16, 2022 10:46 AM
>     > To: devel@edk2.groups.io; Sami Mujawar <Sami.Mujawar@arm.com>
>     > Cc: ardb+tianocore@kernel.org; Justin He <Justin.He@arm.com>;
> Jianyong
>     > Wu <Jianyong.Wu@arm.com>
>     > Subject: [PATCH 0/3] CloudHv:arm: Enable direct kernel boot
>     >
>     > Direct kernel boot removes the dependency of retrieving kernel image
> from
>     > block device. For Cloud Hypervisor, we use the following way to support
> it.
>     >
>     > 1. Cloud Hypervisor store kernel image into memory and put kernel info,
>     >    including the memory base and size, into DT; 2. When init memory in
> edk2,
>     > the kernel memory region is retrieved from
>     >    DT and set it as read only memory region; 3. Edk2 fetches kernel from
>     > memory and prepare a image handle; 4. Load kernel using LoadImage in
> the
>     > end.
>     >
>     > 1 is done in Cloud Hypervisor, 2 and 3 is done in this patch set, 4 is not
>     > affected.
>     >
>     > github PR link: https://github.com/tianocore/edk2/pull/3339
>     >
>     > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
>     >
>     > Jianyong Wu (3):
>     >   CloudHv:arm: add kernel load fs driver
>     >   CloudHv:arm: build hob for kernel image memory as read-only
>     >   CloudHv:arm: add kernel loader lib dsc/fdf
>     >
>     >  ArmVirtPkg/ArmVirtCloudHv.dsc                 |   8 +-
>     >  ArmVirtPkg/ArmVirtCloudHv.fdf                 |   1 +
>     >  .../CloudHvKernelLoaderFsDxe.c                | 969 ++++++++++++++++++
>     >  .../CloudHvKernelLoaderFsDxe.inf              |  55 +
>     >  .../CloudHvVirtMemInfoLib.c                   |  66 +-
>     >  5 files changed, 1094 insertions(+), 5 deletions(-)  create mode 100644
>     > ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c
>     >  create mode 100644
>     > ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
>     >
>     > --
>     > 2.17.1
> 
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/3] CloudHv:arm: add kernel load fs driver
  2022-09-16  2:46 ` [PATCH 1/3] CloudHv:arm: add kernel load fs driver Jianyong Wu
@ 2022-11-22 15:44   ` Sami Mujawar
  2022-11-22 15:46   ` Sami Mujawar
  1 sibling, 0 replies; 19+ messages in thread
From: Sami Mujawar @ 2022-11-22 15:44 UTC (permalink / raw)
  To: Jianyong Wu, devel; +Cc: ardb+tianocore, justin.he, nd@arm.com

Hi Jianyoung,

I am trying to understand your patch series. Is it possible to send me 
the steps to test your patches using CloudHv, please?

Also, please see my response for patch 2/3 as I have some queries.

Regards,

Sami Mujawar


On 16/09/2022 03:46 am, Jianyong Wu wrote:
> This is used for supporting direct kernel boot in CloudHv. CloudHv
> will store kernel image in system ram and pass kernel info through
> DT. It's firmware's responsibility to fetch the kernel data and
> create a file device to feed for loadImage
>
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> ---
>   .../CloudHvKernelLoaderFsDxe.c                | 969 ++++++++++++++++++
>   .../CloudHvKernelLoaderFsDxe.inf              |  56 +
>   2 files changed, 1025 insertions(+)
>   create mode 100644 ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c
>   create mode 100644 ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
>
> diff --git a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c
> new file mode 100644
> index 0000000000..e4cfcfab72
> --- /dev/null
> +++ b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c
> @@ -0,0 +1,969 @@
> +/** @file
> +  DXE driver to expose the 'kernel', 'initrd' and 'cmdline' blobs
> +  provided by Cloud Hypervisor as files in an abstract file system
> +
> +  Copyright (C) 2022, Arm, Limited.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <PiPei.h>
> +#include <PiDxe.h>
> +
> +#include <libfdt.h>
> +#include <Guid/FileInfo.h>
> +#include <Guid/FileSystemInfo.h>
> +#include <Guid/FileSystemVolumeLabelInfo.h>
> +#include <Guid/LinuxEfiInitrdMedia.h>
> +#include <Guid/QemuKernelLoaderFsMedia.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/BlobVerifierLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/PrePiLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +#include <Protocol/DevicePath.h>
> +#include <Protocol/LoadFile2.h>
> +#include <Protocol/SimpleFileSystem.h>
> +
> +//
> +// Static data that hosts the data blobs and serves file requests.
> +//
> +typedef enum {
> +  KernelBlobTypeKernel,
> +  KernelBlobTypeInitrd,
> +  KernelBlobTypeCommandLine,
> +  KernelBlobTypeMax
> +} KERNEL_BLOB_TYPE;
> +
> +typedef struct {
> +  CONST CHAR16    Name[8];
> +  UINT32          Size;
> +  UINT8           *Data;
> +} KERNEL_BLOB;
> +
> +STATIC KERNEL_BLOB  mKernelBlob[KernelBlobTypeMax] = {
> +  {
> +    L"kernel",
> +  },{
> +    L"initrd",
> +  },{
> +    L"cmdline",
> +  }
> +};
> +
> +//
> +// Device path for the handle that incorporates our "EFI stub filesystem".
> +//
> +#pragma pack (1)
> +typedef struct {
> +  VENDOR_DEVICE_PATH          VenMediaNode;
> +  EFI_DEVICE_PATH_PROTOCOL    EndNode;
> +} SINGLE_VENMEDIA_NODE_DEVPATH;
> +#pragma pack ()
> +
> +STATIC CONST SINGLE_VENMEDIA_NODE_DEVPATH  mFileSystemDevicePath = {
> +  {
> +    {
> +      MEDIA_DEVICE_PATH, MEDIA_VENDOR_DP,
> +      { sizeof (VENDOR_DEVICE_PATH)       }
> +    },
> +    QEMU_KERNEL_LOADER_FS_MEDIA_GUID
> +  },  {
> +    END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE,
> +    { sizeof (EFI_DEVICE_PATH_PROTOCOL) }
> +  }
> +};
> +
> +STATIC CONST SINGLE_VENMEDIA_NODE_DEVPATH  mInitrdDevicePath = {
> +  {
> +    {
> +      MEDIA_DEVICE_PATH, MEDIA_VENDOR_DP,
> +      { sizeof (VENDOR_DEVICE_PATH)       }
> +    },
> +    LINUX_EFI_INITRD_MEDIA_GUID
> +  },  {
> +    END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE,
> +    { sizeof (EFI_DEVICE_PATH_PROTOCOL) }
> +  }
> +};
> +
> +//
> +// The "file in the EFI stub filesystem" abstraction.
> +//
> +STATIC EFI_TIME  mInitTime;
> +STATIC UINT64    mTotalBlobBytes;
> +
> +#define STUB_FILE_SIG  SIGNATURE_64 ('S', 'T', 'U', 'B', 'F', 'I', 'L', 'E')
> +
> +typedef struct {
> +  UINT64               Signature; // Carries STUB_FILE_SIG.
> +
> +  KERNEL_BLOB_TYPE     BlobType; // Index into mKernelBlob. KernelBlobTypeMax
> +                                 // denotes the root directory of the filesystem.
> +
> +  UINT64               Position; // Byte position for regular files;
> +                                 // next directory entry to return for the root
> +                                 // directory.
> +
> +  EFI_FILE_PROTOCOL    File;   // Standard protocol interface.
> +} STUB_FILE;
> +
> +#define STUB_FILE_FROM_FILE(FilePointer) \
> +        CR (FilePointer, STUB_FILE, File, STUB_FILE_SIG)
> +
> +/**
> +  Helper function that formats an EFI_FILE_INFO structure into the
> +  user-allocated buffer, for any valid KERNEL_BLOB_TYPE value (including
> +  KernelBlobTypeMax, which stands for the root directory).
> +
> +  The interface follows the EFI_FILE_GET_INFO -- and for directories, the
> +  EFI_FILE_READ -- interfaces.
> +
> +  @param[in]     BlobType     The KERNEL_BLOB_TYPE value identifying the fw_cfg
> +                              blob backing the STUB_FILE that information is
> +                              being requested about. If BlobType equals
> +                              KernelBlobTypeMax, then information will be
> +                              provided about the root directory of the
> +                              filesystem.
> +
> +  @param[in,out] BufferSize  On input, the size of Buffer. On output, the
> +                             amount of data returned in Buffer. In both cases,
> +                             the size is measured in bytes.
> +
> +  @param[out]    Buffer      A pointer to the data buffer to return. The
> +                             buffer's type is EFI_FILE_INFO.
> +
> +  @retval EFI_SUCCESS           The information was returned.
> +  @retval EFI_BUFFER_TOO_SMALL  BufferSize is too small to store the
> +                                EFI_FILE_INFO structure. BufferSize has been
> +                                updated with the size needed to complete the
> +                                request.
> +**/
> +EFI_STATUS
> +ConvertKernelBlobTypeToFileInfo (
> +  IN KERNEL_BLOB_TYPE  BlobType,
> +  IN OUT UINTN         *BufferSize,
> +  OUT VOID             *Buffer
> +  )
> +{
> +  CONST CHAR16  *Name;
> +  UINT64        FileSize;
> +  UINT64        Attribute;
> +
> +  UINTN          NameSize;
> +  UINTN          FileInfoSize;
> +  EFI_FILE_INFO  *FileInfo;
> +  UINTN          OriginalBufferSize;
> +
> +  if (BlobType == KernelBlobTypeMax) {
> +    //
> +    // getting file info about the root directory
> +    //
> +    Name      = L"\\";
> +    FileSize  = KernelBlobTypeMax;
> +    Attribute = EFI_FILE_READ_ONLY | EFI_FILE_DIRECTORY;
> +  } else {
> +    CONST KERNEL_BLOB  *Blob;
> +
> +    Blob      = &mKernelBlob[BlobType];
> +    Name      = Blob->Name;
> +    FileSize  = Blob->Size;
> +    Attribute = EFI_FILE_READ_ONLY;
> +  }
> +
> +  NameSize     = (StrLen (Name) + 1) * 2;
> +  FileInfoSize = OFFSET_OF (EFI_FILE_INFO, FileName) + NameSize;
> +  ASSERT (FileInfoSize >= sizeof *FileInfo);
> +
> +  OriginalBufferSize = *BufferSize;
> +  *BufferSize        = FileInfoSize;
> +  if (OriginalBufferSize < *BufferSize) {
> +    return EFI_BUFFER_TOO_SMALL;
> +  }
> +
> +  FileInfo               = (EFI_FILE_INFO *)Buffer;
> +  FileInfo->Size         = FileInfoSize;
> +  FileInfo->FileSize     = FileSize;
> +  FileInfo->PhysicalSize = FileSize;
> +  FileInfo->Attribute    = Attribute;
> +
> +  CopyMem (&FileInfo->CreateTime, &mInitTime, sizeof mInitTime);
> +  CopyMem (&FileInfo->LastAccessTime, &mInitTime, sizeof mInitTime);
> +  CopyMem (&FileInfo->ModificationTime, &mInitTime, sizeof mInitTime);
> +  CopyMem (FileInfo->FileName, Name, NameSize);
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Reads data from a file, or continues scanning a directory.
> +
> +  @param[in]     This        A pointer to the EFI_FILE_PROTOCOL instance that
> +                             is the file handle to read data from.
> +
> +  @param[in,out] BufferSize  On input, the size of the Buffer. On output, the
> +                             amount of data returned in Buffer. In both cases,
> +                             the size is measured in bytes. If the read goes
> +                             beyond the end of the file, the read length is
> +                             truncated to the end of the file.
> +
> +                             If This is a directory, the function reads the
> +                             directory entry at the current position and
> +                             returns the entry (as EFI_FILE_INFO) in Buffer. If
> +                             there are no more directory entries, the
> +                             BufferSize is set to zero on output.
> +
> +  @param[out]    Buffer      The buffer into which the data is read.
> +
> +  @retval EFI_SUCCESS           Data was read.
> +  @retval EFI_NO_MEDIA          The device has no medium.
> +  @retval EFI_DEVICE_ERROR      The device reported an error.
> +  @retval EFI_DEVICE_ERROR      An attempt was made to read from a deleted
> +                                file.
> +  @retval EFI_DEVICE_ERROR      On entry, the current file position is beyond
> +                                the end of the file.
> +  @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.
> +  @retval EFI_BUFFER_TOO_SMALL  The BufferSize is too small to store the
> +                                current directory entry as a EFI_FILE_INFO
> +                                structure. BufferSize has been updated with the
> +                                size needed to complete the request, and the
> +                                directory position has not been advanced.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +StubFileRead (
> +  IN EFI_FILE_PROTOCOL  *This,
> +  IN OUT UINTN          *BufferSize,
> +  OUT VOID              *Buffer
> +  )
> +{
> +  STUB_FILE          *StubFile;
> +  CONST KERNEL_BLOB  *Blob;
> +  UINT64             Left;
> +
> +  StubFile = STUB_FILE_FROM_FILE (This);
> +
> +  //
> +  // Scanning the root directory?
> +  //
> +  if (StubFile->BlobType == KernelBlobTypeMax) {
> +    EFI_STATUS  Status;
> +
> +    if (StubFile->Position == KernelBlobTypeMax) {
> +      //
> +      // Scanning complete.
> +      //
> +      *BufferSize = 0;
> +      return EFI_SUCCESS;
> +    }
> +
> +    Status = ConvertKernelBlobTypeToFileInfo (
> +               (KERNEL_BLOB_TYPE)StubFile->Position,
> +               BufferSize,
> +               Buffer
> +               );
> +    if (EFI_ERROR (Status)) {
> +      return Status;
> +    }
> +
> +    ++StubFile->Position;
> +    return EFI_SUCCESS;
> +  }
> +
> +  //
> +  // Reading a file.
> +  //
> +  Blob = &mKernelBlob[StubFile->BlobType];
> +  if (StubFile->Position > Blob->Size) {
> +    return EFI_DEVICE_ERROR;
> +  }
> +
> +  Left = Blob->Size - StubFile->Position;
> +  if (*BufferSize > Left) {
> +    *BufferSize = (UINTN)Left;
> +  }
> +
> +  if (Blob->Data != NULL) {
> +    CopyMem (Buffer, Blob->Data + StubFile->Position, *BufferSize);
> +  }
> +
> +  StubFile->Position += *BufferSize;
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Closes a specified file handle.
> +
> +  @param[in] This  A pointer to the EFI_FILE_PROTOCOL instance that is the file
> +                   handle to close.
> +
> +  @retval EFI_SUCCESS  The file was closed.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +StubFileClose (
> +  IN EFI_FILE_PROTOCOL  *This
> +  )
> +{
> +  FreePool (STUB_FILE_FROM_FILE (This));
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Close and delete the file handle.
> +
> +  @param[in] This  A pointer to the EFI_FILE_PROTOCOL instance that is the
> +                   handle to the file to delete.
> +
> +  @retval EFI_SUCCESS              The file was closed and deleted, and the
> +                                   handle was closed.
> +  @retval EFI_WARN_DELETE_FAILURE  The handle was closed, but the file was not
> +                                   deleted.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +StubFileDelete (
> +  IN EFI_FILE_PROTOCOL  *This
> +  )
> +{
> +  FreePool (STUB_FILE_FROM_FILE (This));
> +  return EFI_WARN_DELETE_FAILURE;
> +}
> +
> +//
> +// Protocol member functions for File.
> +//
> +
> +/**
> +  Opens a new file relative to the source file's location.
> +
> +  (Forward declaration.)
> +
> +  @param[in]  This        A pointer to the EFI_FILE_PROTOCOL instance that is
> +                          the file handle to the source location. This would
> +                          typically be an open handle to a directory.
> +
> +  @param[out] NewHandle   A pointer to the location to return the opened handle
> +                          for the new file.
> +
> +  @param[in]  FileName    The Null-terminated string of the name of the file to
> +                          be opened. The file name may contain the following
> +                          path modifiers: "\", ".", and "..".
> +
> +  @param[in]  OpenMode    The mode to open the file. The only valid
> +                          combinations that the file may be opened with are:
> +                          Read, Read/Write, or Create/Read/Write.
> +
> +  @param[in]  Attributes  Only valid for EFI_FILE_MODE_CREATE, in which case
> +                          these are the attribute bits for the newly created
> +                          file.
> +
> +  @retval EFI_SUCCESS           The file was opened.
> +  @retval EFI_NOT_FOUND         The specified file could not be found on the
> +                                device.
> +  @retval EFI_NO_MEDIA          The device has no medium.
> +  @retval EFI_MEDIA_CHANGED     The device has a different medium in it or the
> +                                medium is no longer supported.
> +  @retval EFI_DEVICE_ERROR      The device reported an error.
> +  @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.
> +  @retval EFI_WRITE_PROTECTED   An attempt was made to create a file, or open a
> +                                file for write when the media is
> +                                write-protected.
> +  @retval EFI_ACCESS_DENIED     The service denied access to the file.
> +  @retval EFI_OUT_OF_RESOURCES  Not enough resources were available to open the
> +                                file.
> +  @retval EFI_VOLUME_FULL       The volume is full.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +StubFileOpen (
> +  IN EFI_FILE_PROTOCOL   *This,
> +  OUT EFI_FILE_PROTOCOL  **NewHandle,
> +  IN CHAR16              *FileName,
> +  IN UINT64              OpenMode,
> +  IN UINT64              Attributes
> +  );
> +
> +/**
> +  Returns information about a file.
> +
> +  @param[in]     This             A pointer to the EFI_FILE_PROTOCOL instance
> +                                  that is the file handle the requested
> +                                  information is for.
> +
> +  @param[in]     InformationType  The type identifier GUID for the information
> +                                  being requested. The following information
> +                                  types are supported, storing the
> +                                  corresponding structures in Buffer:
> +
> +                                  - gEfiFileInfoGuid: EFI_FILE_INFO
> +
> +                                  - gEfiFileSystemInfoGuid:
> +                                    EFI_FILE_SYSTEM_INFO
> +
> +                                  - gEfiFileSystemVolumeLabelInfoIdGuid:
> +                                    EFI_FILE_SYSTEM_VOLUME_LABEL
> +
> +  @param[in,out] BufferSize       On input, the size of Buffer. On output, the
> +                                  amount of data returned in Buffer. In both
> +                                  cases, the size is measured in bytes.
> +
> +  @param[out]    Buffer           A pointer to the data buffer to return. The
> +                                  buffer's type is indicated by
> +                                  InformationType.
> +
> +  @retval EFI_SUCCESS           The information was returned.
> +  @retval EFI_UNSUPPORTED       The InformationType is not known.
> +  @retval EFI_NO_MEDIA          The device has no medium.
> +  @retval EFI_DEVICE_ERROR      The device reported an error.
> +  @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.
> +  @retval EFI_BUFFER_TOO_SMALL  The BufferSize is too small to store the
> +                                information structure requested by
> +                                InformationType. BufferSize has been updated
> +                                with the size needed to complete the request.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +StubFileGetInfo (
> +  IN EFI_FILE_PROTOCOL  *This,
> +  IN EFI_GUID           *InformationType,
> +  IN OUT UINTN          *BufferSize,
> +  OUT VOID              *Buffer
> +  )
> +{
> +  CONST STUB_FILE  *StubFile;
> +  UINTN            OriginalBufferSize;
> +
> +  StubFile = STUB_FILE_FROM_FILE (This);
> +
> +  if (CompareGuid (InformationType, &gEfiFileInfoGuid)) {
> +    return ConvertKernelBlobTypeToFileInfo (
> +             StubFile->BlobType,
> +             BufferSize,
> +             Buffer
> +             );
> +  }
> +
> +  OriginalBufferSize = *BufferSize;
> +
> +  if (CompareGuid (InformationType, &gEfiFileSystemInfoGuid)) {
> +    EFI_FILE_SYSTEM_INFO  *FileSystemInfo;
> +
> +    *BufferSize = sizeof *FileSystemInfo;
> +    if (OriginalBufferSize < *BufferSize) {
> +      return EFI_BUFFER_TOO_SMALL;
> +    }
> +
> +    FileSystemInfo                 = (EFI_FILE_SYSTEM_INFO *)Buffer;
> +    FileSystemInfo->Size           = sizeof *FileSystemInfo;
> +    FileSystemInfo->ReadOnly       = TRUE;
> +    FileSystemInfo->VolumeSize     = mTotalBlobBytes;
> +    FileSystemInfo->FreeSpace      = 0;
> +    FileSystemInfo->BlockSize      = 1;
> +    FileSystemInfo->VolumeLabel[0] = L'\0';
> +
> +    return EFI_SUCCESS;
> +  }
> +
> +  if (CompareGuid (InformationType, &gEfiFileSystemVolumeLabelInfoIdGuid)) {
> +    EFI_FILE_SYSTEM_VOLUME_LABEL  *FileSystemVolumeLabel;
> +
> +    *BufferSize = sizeof *FileSystemVolumeLabel;
> +    if (OriginalBufferSize < *BufferSize) {
> +      return EFI_BUFFER_TOO_SMALL;
> +    }
> +
> +    FileSystemVolumeLabel                 = (EFI_FILE_SYSTEM_VOLUME_LABEL *)Buffer;
> +    FileSystemVolumeLabel->VolumeLabel[0] = L'\0';
> +
> +    return EFI_SUCCESS;
> +  }
> +
> +  return EFI_UNSUPPORTED;
> +}
> +
> +//
> +// External definition of the file protocol template.
> +//
> +STATIC CONST EFI_FILE_PROTOCOL  mEfiFileProtocolTemplate = {
> +  EFI_FILE_PROTOCOL_REVISION, // revision 1
> +  StubFileOpen,
> +  StubFileClose,
> +  StubFileDelete,
> +  StubFileRead,
> +  NULL,
> +  NULL,
> +  NULL,
> +  StubFileGetInfo,
> +  NULL,
> +  NULL,
> +  NULL,                       // OpenEx, revision 2
> +  NULL,                       // ReadEx, revision 2
> +  NULL,                       // WriteEx, revision 2
> +  NULL                        // FlushEx, revision 2
> +};
> +
> +/**
> +  Opens a new file relative to the source file's location.
> +
> +  (Forward declaration.)
> +
> +  @param[in]  This        A pointer to the EFI_FILE_PROTOCOL instance that is
> +                          the file handle to the source location. This would
> +                          typically be an open handle to a directory.
> +
> +  @param[out] NewHandle   A pointer to the location to return the opened handle
> +                          for the new file.
> +
> +  @param[in]  FileName    The Null-terminated string of the name of the file to
> +                          be opened. The file name may contain the following
> +                          path modifiers: "\", ".", and "..".
> +
> +  @param[in]  OpenMode    The mode to open the file. The only valid
> +                          combinations that the file may be opened with are:
> +                          Read, Read/Write, or Create/Read/Write.
> +
> +  @param[in]  Attributes  Only valid for EFI_FILE_MODE_CREATE, in which case
> +                          these are the attribute bits for the newly created
> +                          file.
> +
> +  @retval EFI_SUCCESS           The file was opened.
> +  @retval EFI_NOT_FOUND         The specified file could not be found on the
> +                                device.
> +  @retval EFI_NO_MEDIA          The device has no medium.
> +  @retval EFI_MEDIA_CHANGED     The device has a different medium in it or the
> +                                medium is no longer supported.
> +  @retval EFI_DEVICE_ERROR      The device reported an error.
> +  @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.
> +  @retval EFI_WRITE_PROTECTED   An attempt was made to create a file, or open a
> +                                file for write when the media is
> +                                write-protected.
> +  @retval EFI_ACCESS_DENIED     The service denied access to the file.
> +  @retval EFI_OUT_OF_RESOURCES  Not enough resources were available to open the
> +                                file.
> +  @retval EFI_VOLUME_FULL       The volume is full.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +StubFileOpen (
> +  IN EFI_FILE_PROTOCOL   *This,
> +  OUT EFI_FILE_PROTOCOL  **NewHandle,
> +  IN CHAR16              *FileName,
> +  IN UINT64              OpenMode,
> +  IN UINT64              Attributes
> +  )
> +{
> +  CONST STUB_FILE  *StubFile;
> +  UINTN            BlobType;
> +  STUB_FILE        *NewStubFile;
> +
> +  //
> +  // We're read-only.
> +  //
> +  switch (OpenMode) {
> +    case EFI_FILE_MODE_READ:
> +      break;
> +
> +    case EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE:
> +    case EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE | EFI_FILE_MODE_CREATE:
> +      return EFI_WRITE_PROTECTED;
> +
> +    default:
> +      return EFI_INVALID_PARAMETER;
> +  }
> +
> +  //
> +  // Only the root directory supports opening files in it.
> +  //
> +  StubFile = STUB_FILE_FROM_FILE (This);
> +  if (StubFile->BlobType != KernelBlobTypeMax) {
> +    return EFI_UNSUPPORTED;
> +  }
> +
> +  //
> +  // Locate the file.
> +  //
> +  for (BlobType = 0; BlobType < KernelBlobTypeMax; ++BlobType) {
> +    if (StrCmp (FileName, mKernelBlob[BlobType].Name) == 0) {
> +      break;
> +    }
> +  }
> +
> +  if (BlobType == KernelBlobTypeMax) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  //
> +  // Found it.
> +  //
> +  NewStubFile = AllocatePool (sizeof *NewStubFile);
> +  if (NewStubFile == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  NewStubFile->Signature = STUB_FILE_SIG;
> +  NewStubFile->BlobType  = (KERNEL_BLOB_TYPE)BlobType;
> +  NewStubFile->Position  = 0;
> +  CopyMem (
> +    &NewStubFile->File,
> +    &mEfiFileProtocolTemplate,
> +    sizeof mEfiFileProtocolTemplate
> +    );
> +  *NewHandle = &NewStubFile->File;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +//
> +// Protocol member functions for SimpleFileSystem.
> +//
> +
> +/**
> +  Open the root directory on a volume.
> +
> +  @param[in]  This  A pointer to the volume to open the root directory on.
> +
> +  @param[out] Root  A pointer to the location to return the opened file handle
> +                    for the root directory in.
> +
> +  @retval EFI_SUCCESS           The device was opened.
> +  @retval EFI_UNSUPPORTED       This volume does not support the requested file
> +                                system type.
> +  @retval EFI_NO_MEDIA          The device has no medium.
> +  @retval EFI_DEVICE_ERROR      The device reported an error.
> +  @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.
> +  @retval EFI_ACCESS_DENIED     The service denied access to the file.
> +  @retval EFI_OUT_OF_RESOURCES  The volume was not opened due to lack of
> +                                resources.
> +  @retval EFI_MEDIA_CHANGED     The device has a different medium in it or the
> +                                medium is no longer supported. Any existing
> +                                file handles for this volume are no longer
> +                                valid. To access the files on the new medium,
> +                                the volume must be reopened with OpenVolume().
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +StubFileSystemOpenVolume (
> +  IN EFI_SIMPLE_FILE_SYSTEM_PROTOCOL  *This,
> +  OUT EFI_FILE_PROTOCOL               **Root
> +  )
> +{
> +  STUB_FILE  *StubFile;
> +
> +  StubFile = AllocatePool (sizeof *StubFile);
> +  if (StubFile == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  StubFile->Signature = STUB_FILE_SIG;
> +  StubFile->BlobType  = KernelBlobTypeMax;
> +  StubFile->Position  = 0;
> +  CopyMem (
> +    &StubFile->File,
> +    &mEfiFileProtocolTemplate,
> +    sizeof mEfiFileProtocolTemplate
> +    );
> +  *Root = &StubFile->File;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC CONST EFI_SIMPLE_FILE_SYSTEM_PROTOCOL  mFileSystem = {
> +  EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_REVISION,
> +  StubFileSystemOpenVolume
> +};
> +
> +/**
> +  Load initrd data to memory
> +
> +  @param[in]      This        Not used here.
> +  @param[in]      FilePath    Contain some metadata of file device where holds the initrd.
> +  @param[in]      BootPolicy  Not support here.
> +  @param[in,out]  BufferSize  On input, contains the memory buff size.
> +                              On output, contains the initrd blob size.
> +  @param[out]     Buffer      A pointer point to memory which contains initrd data.
> +
> +  @retval EFI_UNSUPPORTED        Once the Boot Policy is true.
> +  @retval EFI_INVALID_PARAMETER  Once there is no BuffSize given or no available file device.
> +  @retval EFI_NOT_FOUND          File path doesn't match.
> +  @retval EFI_BUFFER_TOO_SMALL   Buff size too small.
> +  @retval EFI_SUCCESS            Success.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +InitrdLoadFile2 (
> +  IN      EFI_LOAD_FILE2_PROTOCOL   *This,
> +  IN      EFI_DEVICE_PATH_PROTOCOL  *FilePath,
> +  IN      BOOLEAN                   BootPolicy,
> +  IN  OUT UINTN                     *BufferSize,
> +  OUT     VOID                      *Buffer     OPTIONAL
> +  )
> +{
> +  CONST KERNEL_BLOB  *InitrdBlob = &mKernelBlob[KernelBlobTypeInitrd];
> +
> +  ASSERT (InitrdBlob->Size > 0);
> +
> +  if (BootPolicy) {
> +    return EFI_UNSUPPORTED;
> +  }
> +
> +  if ((BufferSize == NULL) || !IsDevicePathValid (FilePath, 0)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if ((FilePath->Type != END_DEVICE_PATH_TYPE) ||
> +      (FilePath->SubType != END_ENTIRE_DEVICE_PATH_SUBTYPE))
> +  {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  if ((Buffer == NULL) || (*BufferSize < InitrdBlob->Size)) {
> +    *BufferSize = InitrdBlob->Size;
> +    return EFI_BUFFER_TOO_SMALL;
> +  }
> +
> +  CopyMem (Buffer, InitrdBlob->Data, InitrdBlob->Size);
> +
> +  *BufferSize = InitrdBlob->Size;
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC CONST EFI_LOAD_FILE2_PROTOCOL  mInitrdLoadFile2 = {
> +  InitrdLoadFile2,
> +};
> +
> +/**
> +  Fetch kernel, initrd and commandline into Blob.
> +
> +  @param Blob    On input, choose which blob to be fetched.
> +                 On output, carry the full blob info.
> +
> +  @retval EFI_NOT_FOUND    Kernel is not fetched or no chosen node found in DT.
> +  @retval EFI_SUCCESS      If there is kernel fetched or the Block->Name is not kernel.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +FetchBlob (
> +  IN OUT KERNEL_BLOB  *Blob
> +  )
> +{
> +  VOID          *DeviceTreeBase;
> +  UINT64        InitrdStart, InitrdEnd;
> +  INT32         Len;
> +  CONST UINT64  *Prop;
> +  INT32         ChosenNode;
> +
> +  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
> +  ASSERT (DeviceTreeBase != NULL);
> +
> +  Blob->Size = 0;
> +
> +  //
> +  // Make sure we have a valid device tree blob
> +  //
> +  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
> +
> +  //
> +  // Find chosen node
> +  //
> +  ChosenNode = fdt_path_offset (DeviceTreeBase, "/chosen");
> +  if (ChosenNode < 0) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  switch (Blob->Name[0]) {
> +    case 'k':
> +      Prop = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,kernel-start", &Len);
> +      if ((Prop == NULL) || (Len < 0)) {
> +        return EFI_NOT_FOUND;
> +      }
> +
> +      Blob->Data = (UINT8 *)fdt64_to_cpu (ReadUnaligned64 (Prop));
> +      Prop       = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,kernel-size", &Len);
> +      if ((Prop == NULL) || (Len < 0)) {
> +        return EFI_SUCCESS;
> +      }
> +
> +      Blob->Size = fdt64_to_cpu (ReadUnaligned64 (Prop));
> +      break;
> +    case 'i':
> +      Prop = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,initrd-start", &Len);
> +      if ((Prop == NULL) || (Len < 0)) {
> +        return EFI_SUCCESS;
> +      }
> +
> +      InitrdStart = fdt64_to_cpu (ReadUnaligned64 (Prop));
> +      Blob->Data  = (UINT8 *)InitrdStart;
> +      Prop        = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,initrd-end", &Len);
> +      if ((Prop == NULL) || (Len < 0)) {
> +        return EFI_SUCCESS;
> +      }
> +
> +      InitrdEnd  = fdt64_to_cpu (ReadUnaligned64 (Prop));
> +      Blob->Size = InitrdEnd - InitrdStart;
> +      break;
> +    case 'c':
> +      Prop = fdt_getprop (DeviceTreeBase, ChosenNode, "bootargs", &Len);
> +      if ((Prop == NULL) || (Len < 0)) {
> +        return EFI_SUCCESS;
> +      }
> +
> +      Blob->Data = (UINT8 *)Prop;
> +      Blob->Size = Len;
> +      break;
> +    default:
> +      return EFI_SUCCESS;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +//
> +// The entry point of the feature.
> +//
> +
> +/**
> +  Download the kernel, the initial ramdisk, and the kernel command line from
> +  memory or DT. Construct a minimal SimpleFileSystem that contains the two
> +  image files.
> +
> +  @param ImageHandle  Image handle.
> +  @param SystemTable  EFI system table.
> +
> +  @return                       Error codes from any of the underlying
> +                                functions. On success, the function doesn't
> +                                return.
> +**/
> +EFI_STATUS
> +EFIAPI
> +CloudHvKernelLoaderFsDxeEntrypoint (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  UINTN        BlobType;
> +  KERNEL_BLOB  *CurrentBlob;
> +  KERNEL_BLOB  *KernelBlob;
> +  EFI_STATUS   Status;
> +  EFI_HANDLE   FileSystemHandle;
> +  EFI_HANDLE   InitrdLoadFile2Handle;
> +
> +  Status = gRT->GetTime (&mInitTime, NULL /* Capabilities */);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "%a: GetTime(): %r\n", __FUNCTION__, Status));
> +    return Status;
> +  }
> +
> +  //
> +  // Fetch all blobs.
> +  //
> +  for (BlobType = 0; BlobType < KernelBlobTypeMax; ++BlobType) {
> +    CurrentBlob = &mKernelBlob[BlobType];
> +    Status      = FetchBlob (CurrentBlob);
> +    if (EFI_ERROR (Status)) {
> +      goto FreeBlobs;
> +    }
> +
> +    Status = VerifyBlob (
> +               CurrentBlob->Name,
> +               CurrentBlob->Data,
> +               CurrentBlob->Size
> +               );
> +    if (EFI_ERROR (Status)) {
> +      goto FreeBlobs;
> +    }
> +
> +    mTotalBlobBytes += CurrentBlob->Size;
> +  }
> +
> +  KernelBlob = &mKernelBlob[KernelBlobTypeKernel];
> +
> +  if (KernelBlob->Data == NULL) {
> +    Status = EFI_NOT_FOUND;
> +    goto FreeBlobs;
> +  }
> +
> +  //
> +  // Create a new handle with a single VenMedia() node device path protocol on
> +  // it, plus a custom SimpleFileSystem protocol on it.
> +  //
> +  FileSystemHandle = NULL;
> +  Status           = gBS->InstallMultipleProtocolInterfaces (
> +                            &FileSystemHandle,
> +                            &gEfiDevicePathProtocolGuid,
> +                            &mFileSystemDevicePath,
> +                            &gEfiSimpleFileSystemProtocolGuid,
> +                            &mFileSystem,
> +                            NULL
> +                            );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: InstallMultipleProtocolInterfaces(): %r\n",
> +      __FUNCTION__,
> +      Status
> +      ));
> +    goto FreeBlobs;
> +  }
> +
> +  if (KernelBlob[KernelBlobTypeInitrd].Size > 0) {
> +    InitrdLoadFile2Handle = NULL;
> +    Status                = gBS->InstallMultipleProtocolInterfaces (
> +                                   &InitrdLoadFile2Handle,
> +                                   &gEfiDevicePathProtocolGuid,
> +                                   &mInitrdDevicePath,
> +                                   &gEfiLoadFile2ProtocolGuid,
> +                                   &mInitrdLoadFile2,
> +                                   NULL
> +                                   );
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((
> +        DEBUG_ERROR,
> +        "%a: InstallMultipleProtocolInterfaces(): %r\n",
> +        __FUNCTION__,
> +        Status
> +        ));
> +      goto UninstallFileSystemHandle;
> +    }
> +  }
> +
> +  return EFI_SUCCESS;
> +
> +UninstallFileSystemHandle:
> +  Status = gBS->UninstallMultipleProtocolInterfaces (
> +                  FileSystemHandle,
> +                  &gEfiDevicePathProtocolGuid,
> +                  &mFileSystemDevicePath,
> +                  &gEfiSimpleFileSystemProtocolGuid,
> +                  &mFileSystem,
> +                  NULL
> +                  );
> +  ASSERT_EFI_ERROR (Status);
> +
> +FreeBlobs:
> +  while (BlobType > 0) {
> +    CurrentBlob = &mKernelBlob[--BlobType];
> +    if (CurrentBlob->Data != NULL) {
> +      FreePool (CurrentBlob->Data);
> +      CurrentBlob->Size = 0;
> +      CurrentBlob->Data = NULL;
> +    }
> +  }
> +
> +  return Status;
> +}
> diff --git a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
> new file mode 100644
> index 0000000000..b7aa6ebb4e
> --- /dev/null
> +++ b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
> @@ -0,0 +1,56 @@
> +##  @file
> +#  DXE driver to expose the 'kernel', 'initrd' and 'cmdline' blobs
> +#  provided by Cloud Hypervisor as files in an abstract file system
> +#
> +#  Copyright (C) 2022, Arm, Limited.
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001B
> +  BASE_NAME                      = CloudHvKernelLoaderFsDxe
> +  FILE_GUID                      = 57cdf541-2a29-4ae4-97e5-76a4aa2fe090
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = CloudHvKernelLoaderFsDxeEntrypoint
> +
> +[Sources]
> +  CloudHvKernelLoaderFsDxe.c
> +
> +[Packages]
> +  ArmVirtPkg/ArmVirtPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
> +  DebugLib
> +  DevicePathLib
> +  FdtLib
> +  MemoryAllocationLib
> +  PcdLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +  UefiRuntimeServicesTableLib
> +
> +[Guids]
> +  gEfiFileInfoGuid
> +  gEfiFileSystemInfoGuid
> +  gEfiFileSystemVolumeLabelInfoIdGuid
> +  gQemuKernelLoaderFsMediaGuid
> +
> +[Protocols]
> +  gEfiDevicePathProtocolGuid                ## PRODUCES
> +  gEfiLoadFile2ProtocolGuid                 ## PRODUCES
> +  gEfiSimpleFileSystemProtocolGuid          ## PRODUCES
> +
> +[Depex]
> +  gEfiRealTimeClockArchProtocolGuid
> +
> +[FixedPcd]
> +  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/3] CloudHv:arm: add kernel load fs driver
  2022-09-16  2:46 ` [PATCH 1/3] CloudHv:arm: add kernel load fs driver Jianyong Wu
  2022-11-22 15:44   ` Sami Mujawar
@ 2022-11-22 15:46   ` Sami Mujawar
  2022-11-23  6:26     ` Jianyong Wu
  1 sibling, 1 reply; 19+ messages in thread
From: Sami Mujawar @ 2022-11-22 15:46 UTC (permalink / raw)
  To: Jianyong Wu, devel; +Cc: ardb+tianocore, justin.he, nd@arm.com

Hi Jianyoung,

I am trying to understand your patch series. Is it possible to send me 
the steps to test your patches using CloudHv, please?

Also, please see my response for patch 2/3 as I have some queries.

Regards,

Sami Mujawar


On 16/09/2022 03:46 am, Jianyong Wu wrote:
> This is used for supporting direct kernel boot in CloudHv. CloudHv
> will store kernel image in system ram and pass kernel info through
> DT. It's firmware's responsibility to fetch the kernel data and
> create a file device to feed for loadImage
>
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> ---
>   .../CloudHvKernelLoaderFsDxe.c                | 969 ++++++++++++++++++
>   .../CloudHvKernelLoaderFsDxe.inf              |  56 +
>   2 files changed, 1025 insertions(+)
>   create mode 100644 ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c
>   create mode 100644 ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
>
> diff --git a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c
> new file mode 100644
> index 0000000000..e4cfcfab72
> --- /dev/null
> +++ b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c
> @@ -0,0 +1,969 @@
> +/** @file
> +  DXE driver to expose the 'kernel', 'initrd' and 'cmdline' blobs
> +  provided by Cloud Hypervisor as files in an abstract file system
> +
> +  Copyright (C) 2022, Arm, Limited.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <PiPei.h>
> +#include <PiDxe.h>
> +
> +#include <libfdt.h>
> +#include <Guid/FileInfo.h>
> +#include <Guid/FileSystemInfo.h>
> +#include <Guid/FileSystemVolumeLabelInfo.h>
> +#include <Guid/LinuxEfiInitrdMedia.h>
> +#include <Guid/QemuKernelLoaderFsMedia.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/BlobVerifierLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/PrePiLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +#include <Protocol/DevicePath.h>
> +#include <Protocol/LoadFile2.h>
> +#include <Protocol/SimpleFileSystem.h>
> +
> +//
> +// Static data that hosts the data blobs and serves file requests.
> +//
> +typedef enum {
> +  KernelBlobTypeKernel,
> +  KernelBlobTypeInitrd,
> +  KernelBlobTypeCommandLine,
> +  KernelBlobTypeMax
> +} KERNEL_BLOB_TYPE;
> +
> +typedef struct {
> +  CONST CHAR16    Name[8];
> +  UINT32          Size;
> +  UINT8           *Data;
> +} KERNEL_BLOB;
> +
> +STATIC KERNEL_BLOB  mKernelBlob[KernelBlobTypeMax] = {
> +  {
> +    L"kernel",
> +  },{
> +    L"initrd",
> +  },{
> +    L"cmdline",
> +  }
> +};
> +
> +//
> +// Device path for the handle that incorporates our "EFI stub filesystem".
> +//
> +#pragma pack (1)
> +typedef struct {
> +  VENDOR_DEVICE_PATH          VenMediaNode;
> +  EFI_DEVICE_PATH_PROTOCOL    EndNode;
> +} SINGLE_VENMEDIA_NODE_DEVPATH;
> +#pragma pack ()
> +
> +STATIC CONST SINGLE_VENMEDIA_NODE_DEVPATH  mFileSystemDevicePath = {
> +  {
> +    {
> +      MEDIA_DEVICE_PATH, MEDIA_VENDOR_DP,
> +      { sizeof (VENDOR_DEVICE_PATH)       }
> +    },
> +    QEMU_KERNEL_LOADER_FS_MEDIA_GUID
> +  },  {
> +    END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE,
> +    { sizeof (EFI_DEVICE_PATH_PROTOCOL) }
> +  }
> +};
> +
> +STATIC CONST SINGLE_VENMEDIA_NODE_DEVPATH  mInitrdDevicePath = {
> +  {
> +    {
> +      MEDIA_DEVICE_PATH, MEDIA_VENDOR_DP,
> +      { sizeof (VENDOR_DEVICE_PATH)       }
> +    },
> +    LINUX_EFI_INITRD_MEDIA_GUID
> +  },  {
> +    END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE,
> +    { sizeof (EFI_DEVICE_PATH_PROTOCOL) }
> +  }
> +};
> +
> +//
> +// The "file in the EFI stub filesystem" abstraction.
> +//
> +STATIC EFI_TIME  mInitTime;
> +STATIC UINT64    mTotalBlobBytes;
> +
> +#define STUB_FILE_SIG  SIGNATURE_64 ('S', 'T', 'U', 'B', 'F', 'I', 'L', 'E')
> +
> +typedef struct {
> +  UINT64               Signature; // Carries STUB_FILE_SIG.
> +
> +  KERNEL_BLOB_TYPE     BlobType; // Index into mKernelBlob. KernelBlobTypeMax
> +                                 // denotes the root directory of the filesystem.
> +
> +  UINT64               Position; // Byte position for regular files;
> +                                 // next directory entry to return for the root
> +                                 // directory.
> +
> +  EFI_FILE_PROTOCOL    File;   // Standard protocol interface.
> +} STUB_FILE;
> +
> +#define STUB_FILE_FROM_FILE(FilePointer) \
> +        CR (FilePointer, STUB_FILE, File, STUB_FILE_SIG)
> +
> +/**
> +  Helper function that formats an EFI_FILE_INFO structure into the
> +  user-allocated buffer, for any valid KERNEL_BLOB_TYPE value (including
> +  KernelBlobTypeMax, which stands for the root directory).
> +
> +  The interface follows the EFI_FILE_GET_INFO -- and for directories, the
> +  EFI_FILE_READ -- interfaces.
> +
> +  @param[in]     BlobType     The KERNEL_BLOB_TYPE value identifying the fw_cfg
> +                              blob backing the STUB_FILE that information is
> +                              being requested about. If BlobType equals
> +                              KernelBlobTypeMax, then information will be
> +                              provided about the root directory of the
> +                              filesystem.
> +
> +  @param[in,out] BufferSize  On input, the size of Buffer. On output, the
> +                             amount of data returned in Buffer. In both cases,
> +                             the size is measured in bytes.
> +
> +  @param[out]    Buffer      A pointer to the data buffer to return. The
> +                             buffer's type is EFI_FILE_INFO.
> +
> +  @retval EFI_SUCCESS           The information was returned.
> +  @retval EFI_BUFFER_TOO_SMALL  BufferSize is too small to store the
> +                                EFI_FILE_INFO structure. BufferSize has been
> +                                updated with the size needed to complete the
> +                                request.
> +**/
> +EFI_STATUS
> +ConvertKernelBlobTypeToFileInfo (
> +  IN KERNEL_BLOB_TYPE  BlobType,
> +  IN OUT UINTN         *BufferSize,
> +  OUT VOID             *Buffer
> +  )
> +{
> +  CONST CHAR16  *Name;
> +  UINT64        FileSize;
> +  UINT64        Attribute;
> +
> +  UINTN          NameSize;
> +  UINTN          FileInfoSize;
> +  EFI_FILE_INFO  *FileInfo;
> +  UINTN          OriginalBufferSize;
> +
> +  if (BlobType == KernelBlobTypeMax) {
> +    //
> +    // getting file info about the root directory
> +    //
> +    Name      = L"\\";
> +    FileSize  = KernelBlobTypeMax;
> +    Attribute = EFI_FILE_READ_ONLY | EFI_FILE_DIRECTORY;
> +  } else {
> +    CONST KERNEL_BLOB  *Blob;
> +
> +    Blob      = &mKernelBlob[BlobType];
> +    Name      = Blob->Name;
> +    FileSize  = Blob->Size;
> +    Attribute = EFI_FILE_READ_ONLY;
> +  }
> +
> +  NameSize     = (StrLen (Name) + 1) * 2;
> +  FileInfoSize = OFFSET_OF (EFI_FILE_INFO, FileName) + NameSize;
> +  ASSERT (FileInfoSize >= sizeof *FileInfo);
> +
> +  OriginalBufferSize = *BufferSize;
> +  *BufferSize        = FileInfoSize;
> +  if (OriginalBufferSize < *BufferSize) {
> +    return EFI_BUFFER_TOO_SMALL;
> +  }
> +
> +  FileInfo               = (EFI_FILE_INFO *)Buffer;
> +  FileInfo->Size         = FileInfoSize;
> +  FileInfo->FileSize     = FileSize;
> +  FileInfo->PhysicalSize = FileSize;
> +  FileInfo->Attribute    = Attribute;
> +
> +  CopyMem (&FileInfo->CreateTime, &mInitTime, sizeof mInitTime);
> +  CopyMem (&FileInfo->LastAccessTime, &mInitTime, sizeof mInitTime);
> +  CopyMem (&FileInfo->ModificationTime, &mInitTime, sizeof mInitTime);
> +  CopyMem (FileInfo->FileName, Name, NameSize);
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Reads data from a file, or continues scanning a directory.
> +
> +  @param[in]     This        A pointer to the EFI_FILE_PROTOCOL instance that
> +                             is the file handle to read data from.
> +
> +  @param[in,out] BufferSize  On input, the size of the Buffer. On output, the
> +                             amount of data returned in Buffer. In both cases,
> +                             the size is measured in bytes. If the read goes
> +                             beyond the end of the file, the read length is
> +                             truncated to the end of the file.
> +
> +                             If This is a directory, the function reads the
> +                             directory entry at the current position and
> +                             returns the entry (as EFI_FILE_INFO) in Buffer. If
> +                             there are no more directory entries, the
> +                             BufferSize is set to zero on output.
> +
> +  @param[out]    Buffer      The buffer into which the data is read.
> +
> +  @retval EFI_SUCCESS           Data was read.
> +  @retval EFI_NO_MEDIA          The device has no medium.
> +  @retval EFI_DEVICE_ERROR      The device reported an error.
> +  @retval EFI_DEVICE_ERROR      An attempt was made to read from a deleted
> +                                file.
> +  @retval EFI_DEVICE_ERROR      On entry, the current file position is beyond
> +                                the end of the file.
> +  @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.
> +  @retval EFI_BUFFER_TOO_SMALL  The BufferSize is too small to store the
> +                                current directory entry as a EFI_FILE_INFO
> +                                structure. BufferSize has been updated with the
> +                                size needed to complete the request, and the
> +                                directory position has not been advanced.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +StubFileRead (
> +  IN EFI_FILE_PROTOCOL  *This,
> +  IN OUT UINTN          *BufferSize,
> +  OUT VOID              *Buffer
> +  )
> +{
> +  STUB_FILE          *StubFile;
> +  CONST KERNEL_BLOB  *Blob;
> +  UINT64             Left;
> +
> +  StubFile = STUB_FILE_FROM_FILE (This);
> +
> +  //
> +  // Scanning the root directory?
> +  //
> +  if (StubFile->BlobType == KernelBlobTypeMax) {
> +    EFI_STATUS  Status;
> +
> +    if (StubFile->Position == KernelBlobTypeMax) {
> +      //
> +      // Scanning complete.
> +      //
> +      *BufferSize = 0;
> +      return EFI_SUCCESS;
> +    }
> +
> +    Status = ConvertKernelBlobTypeToFileInfo (
> +               (KERNEL_BLOB_TYPE)StubFile->Position,
> +               BufferSize,
> +               Buffer
> +               );
> +    if (EFI_ERROR (Status)) {
> +      return Status;
> +    }
> +
> +    ++StubFile->Position;
> +    return EFI_SUCCESS;
> +  }
> +
> +  //
> +  // Reading a file.
> +  //
> +  Blob = &mKernelBlob[StubFile->BlobType];
> +  if (StubFile->Position > Blob->Size) {
> +    return EFI_DEVICE_ERROR;
> +  }
> +
> +  Left = Blob->Size - StubFile->Position;
> +  if (*BufferSize > Left) {
> +    *BufferSize = (UINTN)Left;
> +  }
> +
> +  if (Blob->Data != NULL) {
> +    CopyMem (Buffer, Blob->Data + StubFile->Position, *BufferSize);
> +  }
> +
> +  StubFile->Position += *BufferSize;
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Closes a specified file handle.
> +
> +  @param[in] This  A pointer to the EFI_FILE_PROTOCOL instance that is the file
> +                   handle to close.
> +
> +  @retval EFI_SUCCESS  The file was closed.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +StubFileClose (
> +  IN EFI_FILE_PROTOCOL  *This
> +  )
> +{
> +  FreePool (STUB_FILE_FROM_FILE (This));
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Close and delete the file handle.
> +
> +  @param[in] This  A pointer to the EFI_FILE_PROTOCOL instance that is the
> +                   handle to the file to delete.
> +
> +  @retval EFI_SUCCESS              The file was closed and deleted, and the
> +                                   handle was closed.
> +  @retval EFI_WARN_DELETE_FAILURE  The handle was closed, but the file was not
> +                                   deleted.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +StubFileDelete (
> +  IN EFI_FILE_PROTOCOL  *This
> +  )
> +{
> +  FreePool (STUB_FILE_FROM_FILE (This));
> +  return EFI_WARN_DELETE_FAILURE;
> +}
> +
> +//
> +// Protocol member functions for File.
> +//
> +
> +/**
> +  Opens a new file relative to the source file's location.
> +
> +  (Forward declaration.)
> +
> +  @param[in]  This        A pointer to the EFI_FILE_PROTOCOL instance that is
> +                          the file handle to the source location. This would
> +                          typically be an open handle to a directory.
> +
> +  @param[out] NewHandle   A pointer to the location to return the opened handle
> +                          for the new file.
> +
> +  @param[in]  FileName    The Null-terminated string of the name of the file to
> +                          be opened. The file name may contain the following
> +                          path modifiers: "\", ".", and "..".
> +
> +  @param[in]  OpenMode    The mode to open the file. The only valid
> +                          combinations that the file may be opened with are:
> +                          Read, Read/Write, or Create/Read/Write.
> +
> +  @param[in]  Attributes  Only valid for EFI_FILE_MODE_CREATE, in which case
> +                          these are the attribute bits for the newly created
> +                          file.
> +
> +  @retval EFI_SUCCESS           The file was opened.
> +  @retval EFI_NOT_FOUND         The specified file could not be found on the
> +                                device.
> +  @retval EFI_NO_MEDIA          The device has no medium.
> +  @retval EFI_MEDIA_CHANGED     The device has a different medium in it or the
> +                                medium is no longer supported.
> +  @retval EFI_DEVICE_ERROR      The device reported an error.
> +  @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.
> +  @retval EFI_WRITE_PROTECTED   An attempt was made to create a file, or open a
> +                                file for write when the media is
> +                                write-protected.
> +  @retval EFI_ACCESS_DENIED     The service denied access to the file.
> +  @retval EFI_OUT_OF_RESOURCES  Not enough resources were available to open the
> +                                file.
> +  @retval EFI_VOLUME_FULL       The volume is full.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +StubFileOpen (
> +  IN EFI_FILE_PROTOCOL   *This,
> +  OUT EFI_FILE_PROTOCOL  **NewHandle,
> +  IN CHAR16              *FileName,
> +  IN UINT64              OpenMode,
> +  IN UINT64              Attributes
> +  );
> +
> +/**
> +  Returns information about a file.
> +
> +  @param[in]     This             A pointer to the EFI_FILE_PROTOCOL instance
> +                                  that is the file handle the requested
> +                                  information is for.
> +
> +  @param[in]     InformationType  The type identifier GUID for the information
> +                                  being requested. The following information
> +                                  types are supported, storing the
> +                                  corresponding structures in Buffer:
> +
> +                                  - gEfiFileInfoGuid: EFI_FILE_INFO
> +
> +                                  - gEfiFileSystemInfoGuid:
> +                                    EFI_FILE_SYSTEM_INFO
> +
> +                                  - gEfiFileSystemVolumeLabelInfoIdGuid:
> +                                    EFI_FILE_SYSTEM_VOLUME_LABEL
> +
> +  @param[in,out] BufferSize       On input, the size of Buffer. On output, the
> +                                  amount of data returned in Buffer. In both
> +                                  cases, the size is measured in bytes.
> +
> +  @param[out]    Buffer           A pointer to the data buffer to return. The
> +                                  buffer's type is indicated by
> +                                  InformationType.
> +
> +  @retval EFI_SUCCESS           The information was returned.
> +  @retval EFI_UNSUPPORTED       The InformationType is not known.
> +  @retval EFI_NO_MEDIA          The device has no medium.
> +  @retval EFI_DEVICE_ERROR      The device reported an error.
> +  @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.
> +  @retval EFI_BUFFER_TOO_SMALL  The BufferSize is too small to store the
> +                                information structure requested by
> +                                InformationType. BufferSize has been updated
> +                                with the size needed to complete the request.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +StubFileGetInfo (
> +  IN EFI_FILE_PROTOCOL  *This,
> +  IN EFI_GUID           *InformationType,
> +  IN OUT UINTN          *BufferSize,
> +  OUT VOID              *Buffer
> +  )
> +{
> +  CONST STUB_FILE  *StubFile;
> +  UINTN            OriginalBufferSize;
> +
> +  StubFile = STUB_FILE_FROM_FILE (This);
> +
> +  if (CompareGuid (InformationType, &gEfiFileInfoGuid)) {
> +    return ConvertKernelBlobTypeToFileInfo (
> +             StubFile->BlobType,
> +             BufferSize,
> +             Buffer
> +             );
> +  }
> +
> +  OriginalBufferSize = *BufferSize;
> +
> +  if (CompareGuid (InformationType, &gEfiFileSystemInfoGuid)) {
> +    EFI_FILE_SYSTEM_INFO  *FileSystemInfo;
> +
> +    *BufferSize = sizeof *FileSystemInfo;
> +    if (OriginalBufferSize < *BufferSize) {
> +      return EFI_BUFFER_TOO_SMALL;
> +    }
> +
> +    FileSystemInfo                 = (EFI_FILE_SYSTEM_INFO *)Buffer;
> +    FileSystemInfo->Size           = sizeof *FileSystemInfo;
> +    FileSystemInfo->ReadOnly       = TRUE;
> +    FileSystemInfo->VolumeSize     = mTotalBlobBytes;
> +    FileSystemInfo->FreeSpace      = 0;
> +    FileSystemInfo->BlockSize      = 1;
> +    FileSystemInfo->VolumeLabel[0] = L'\0';
> +
> +    return EFI_SUCCESS;
> +  }
> +
> +  if (CompareGuid (InformationType, &gEfiFileSystemVolumeLabelInfoIdGuid)) {
> +    EFI_FILE_SYSTEM_VOLUME_LABEL  *FileSystemVolumeLabel;
> +
> +    *BufferSize = sizeof *FileSystemVolumeLabel;
> +    if (OriginalBufferSize < *BufferSize) {
> +      return EFI_BUFFER_TOO_SMALL;
> +    }
> +
> +    FileSystemVolumeLabel                 = (EFI_FILE_SYSTEM_VOLUME_LABEL *)Buffer;
> +    FileSystemVolumeLabel->VolumeLabel[0] = L'\0';
> +
> +    return EFI_SUCCESS;
> +  }
> +
> +  return EFI_UNSUPPORTED;
> +}
> +
> +//
> +// External definition of the file protocol template.
> +//
> +STATIC CONST EFI_FILE_PROTOCOL  mEfiFileProtocolTemplate = {
> +  EFI_FILE_PROTOCOL_REVISION, // revision 1
> +  StubFileOpen,
> +  StubFileClose,
> +  StubFileDelete,
> +  StubFileRead,
> +  NULL,
> +  NULL,
> +  NULL,
> +  StubFileGetInfo,
> +  NULL,
> +  NULL,
> +  NULL,                       // OpenEx, revision 2
> +  NULL,                       // ReadEx, revision 2
> +  NULL,                       // WriteEx, revision 2
> +  NULL                        // FlushEx, revision 2
> +};
> +
> +/**
> +  Opens a new file relative to the source file's location.
> +
> +  (Forward declaration.)
> +
> +  @param[in]  This        A pointer to the EFI_FILE_PROTOCOL instance that is
> +                          the file handle to the source location. This would
> +                          typically be an open handle to a directory.
> +
> +  @param[out] NewHandle   A pointer to the location to return the opened handle
> +                          for the new file.
> +
> +  @param[in]  FileName    The Null-terminated string of the name of the file to
> +                          be opened. The file name may contain the following
> +                          path modifiers: "\", ".", and "..".
> +
> +  @param[in]  OpenMode    The mode to open the file. The only valid
> +                          combinations that the file may be opened with are:
> +                          Read, Read/Write, or Create/Read/Write.
> +
> +  @param[in]  Attributes  Only valid for EFI_FILE_MODE_CREATE, in which case
> +                          these are the attribute bits for the newly created
> +                          file.
> +
> +  @retval EFI_SUCCESS           The file was opened.
> +  @retval EFI_NOT_FOUND         The specified file could not be found on the
> +                                device.
> +  @retval EFI_NO_MEDIA          The device has no medium.
> +  @retval EFI_MEDIA_CHANGED     The device has a different medium in it or the
> +                                medium is no longer supported.
> +  @retval EFI_DEVICE_ERROR      The device reported an error.
> +  @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.
> +  @retval EFI_WRITE_PROTECTED   An attempt was made to create a file, or open a
> +                                file for write when the media is
> +                                write-protected.
> +  @retval EFI_ACCESS_DENIED     The service denied access to the file.
> +  @retval EFI_OUT_OF_RESOURCES  Not enough resources were available to open the
> +                                file.
> +  @retval EFI_VOLUME_FULL       The volume is full.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +StubFileOpen (
> +  IN EFI_FILE_PROTOCOL   *This,
> +  OUT EFI_FILE_PROTOCOL  **NewHandle,
> +  IN CHAR16              *FileName,
> +  IN UINT64              OpenMode,
> +  IN UINT64              Attributes
> +  )
> +{
> +  CONST STUB_FILE  *StubFile;
> +  UINTN            BlobType;
> +  STUB_FILE        *NewStubFile;
> +
> +  //
> +  // We're read-only.
> +  //
> +  switch (OpenMode) {
> +    case EFI_FILE_MODE_READ:
> +      break;
> +
> +    case EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE:
> +    case EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE | EFI_FILE_MODE_CREATE:
> +      return EFI_WRITE_PROTECTED;
> +
> +    default:
> +      return EFI_INVALID_PARAMETER;
> +  }
> +
> +  //
> +  // Only the root directory supports opening files in it.
> +  //
> +  StubFile = STUB_FILE_FROM_FILE (This);
> +  if (StubFile->BlobType != KernelBlobTypeMax) {
> +    return EFI_UNSUPPORTED;
> +  }
> +
> +  //
> +  // Locate the file.
> +  //
> +  for (BlobType = 0; BlobType < KernelBlobTypeMax; ++BlobType) {
> +    if (StrCmp (FileName, mKernelBlob[BlobType].Name) == 0) {
> +      break;
> +    }
> +  }
> +
> +  if (BlobType == KernelBlobTypeMax) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  //
> +  // Found it.
> +  //
> +  NewStubFile = AllocatePool (sizeof *NewStubFile);
> +  if (NewStubFile == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  NewStubFile->Signature = STUB_FILE_SIG;
> +  NewStubFile->BlobType  = (KERNEL_BLOB_TYPE)BlobType;
> +  NewStubFile->Position  = 0;
> +  CopyMem (
> +    &NewStubFile->File,
> +    &mEfiFileProtocolTemplate,
> +    sizeof mEfiFileProtocolTemplate
> +    );
> +  *NewHandle = &NewStubFile->File;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +//
> +// Protocol member functions for SimpleFileSystem.
> +//
> +
> +/**
> +  Open the root directory on a volume.
> +
> +  @param[in]  This  A pointer to the volume to open the root directory on.
> +
> +  @param[out] Root  A pointer to the location to return the opened file handle
> +                    for the root directory in.
> +
> +  @retval EFI_SUCCESS           The device was opened.
> +  @retval EFI_UNSUPPORTED       This volume does not support the requested file
> +                                system type.
> +  @retval EFI_NO_MEDIA          The device has no medium.
> +  @retval EFI_DEVICE_ERROR      The device reported an error.
> +  @retval EFI_VOLUME_CORRUPTED  The file system structures are corrupted.
> +  @retval EFI_ACCESS_DENIED     The service denied access to the file.
> +  @retval EFI_OUT_OF_RESOURCES  The volume was not opened due to lack of
> +                                resources.
> +  @retval EFI_MEDIA_CHANGED     The device has a different medium in it or the
> +                                medium is no longer supported. Any existing
> +                                file handles for this volume are no longer
> +                                valid. To access the files on the new medium,
> +                                the volume must be reopened with OpenVolume().
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +StubFileSystemOpenVolume (
> +  IN EFI_SIMPLE_FILE_SYSTEM_PROTOCOL  *This,
> +  OUT EFI_FILE_PROTOCOL               **Root
> +  )
> +{
> +  STUB_FILE  *StubFile;
> +
> +  StubFile = AllocatePool (sizeof *StubFile);
> +  if (StubFile == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  StubFile->Signature = STUB_FILE_SIG;
> +  StubFile->BlobType  = KernelBlobTypeMax;
> +  StubFile->Position  = 0;
> +  CopyMem (
> +    &StubFile->File,
> +    &mEfiFileProtocolTemplate,
> +    sizeof mEfiFileProtocolTemplate
> +    );
> +  *Root = &StubFile->File;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC CONST EFI_SIMPLE_FILE_SYSTEM_PROTOCOL  mFileSystem = {
> +  EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_REVISION,
> +  StubFileSystemOpenVolume
> +};
> +
> +/**
> +  Load initrd data to memory
> +
> +  @param[in]      This        Not used here.
> +  @param[in]      FilePath    Contain some metadata of file device where holds the initrd.
> +  @param[in]      BootPolicy  Not support here.
> +  @param[in,out]  BufferSize  On input, contains the memory buff size.
> +                              On output, contains the initrd blob size.
> +  @param[out]     Buffer      A pointer point to memory which contains initrd data.
> +
> +  @retval EFI_UNSUPPORTED        Once the Boot Policy is true.
> +  @retval EFI_INVALID_PARAMETER  Once there is no BuffSize given or no available file device.
> +  @retval EFI_NOT_FOUND          File path doesn't match.
> +  @retval EFI_BUFFER_TOO_SMALL   Buff size too small.
> +  @retval EFI_SUCCESS            Success.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +InitrdLoadFile2 (
> +  IN      EFI_LOAD_FILE2_PROTOCOL   *This,
> +  IN      EFI_DEVICE_PATH_PROTOCOL  *FilePath,
> +  IN      BOOLEAN                   BootPolicy,
> +  IN  OUT UINTN                     *BufferSize,
> +  OUT     VOID                      *Buffer     OPTIONAL
> +  )
> +{
> +  CONST KERNEL_BLOB  *InitrdBlob = &mKernelBlob[KernelBlobTypeInitrd];
> +
> +  ASSERT (InitrdBlob->Size > 0);
> +
> +  if (BootPolicy) {
> +    return EFI_UNSUPPORTED;
> +  }
> +
> +  if ((BufferSize == NULL) || !IsDevicePathValid (FilePath, 0)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if ((FilePath->Type != END_DEVICE_PATH_TYPE) ||
> +      (FilePath->SubType != END_ENTIRE_DEVICE_PATH_SUBTYPE))
> +  {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  if ((Buffer == NULL) || (*BufferSize < InitrdBlob->Size)) {
> +    *BufferSize = InitrdBlob->Size;
> +    return EFI_BUFFER_TOO_SMALL;
> +  }
> +
> +  CopyMem (Buffer, InitrdBlob->Data, InitrdBlob->Size);
> +
> +  *BufferSize = InitrdBlob->Size;
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC CONST EFI_LOAD_FILE2_PROTOCOL  mInitrdLoadFile2 = {
> +  InitrdLoadFile2,
> +};
> +
> +/**
> +  Fetch kernel, initrd and commandline into Blob.
> +
> +  @param Blob    On input, choose which blob to be fetched.
> +                 On output, carry the full blob info.
> +
> +  @retval EFI_NOT_FOUND    Kernel is not fetched or no chosen node found in DT.
> +  @retval EFI_SUCCESS      If there is kernel fetched or the Block->Name is not kernel.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +FetchBlob (
> +  IN OUT KERNEL_BLOB  *Blob
> +  )
> +{
> +  VOID          *DeviceTreeBase;
> +  UINT64        InitrdStart, InitrdEnd;
> +  INT32         Len;
> +  CONST UINT64  *Prop;
> +  INT32         ChosenNode;
> +
> +  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
> +  ASSERT (DeviceTreeBase != NULL);
> +
> +  Blob->Size = 0;
> +
> +  //
> +  // Make sure we have a valid device tree blob
> +  //
> +  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
> +
> +  //
> +  // Find chosen node
> +  //
> +  ChosenNode = fdt_path_offset (DeviceTreeBase, "/chosen");
> +  if (ChosenNode < 0) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  switch (Blob->Name[0]) {
> +    case 'k':
> +      Prop = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,kernel-start", &Len);
> +      if ((Prop == NULL) || (Len < 0)) {
> +        return EFI_NOT_FOUND;
> +      }
> +
> +      Blob->Data = (UINT8 *)fdt64_to_cpu (ReadUnaligned64 (Prop));
> +      Prop       = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,kernel-size", &Len);
> +      if ((Prop == NULL) || (Len < 0)) {
> +        return EFI_SUCCESS;
> +      }
> +
> +      Blob->Size = fdt64_to_cpu (ReadUnaligned64 (Prop));
> +      break;
> +    case 'i':
> +      Prop = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,initrd-start", &Len);
> +      if ((Prop == NULL) || (Len < 0)) {
> +        return EFI_SUCCESS;
> +      }
> +
> +      InitrdStart = fdt64_to_cpu (ReadUnaligned64 (Prop));
> +      Blob->Data  = (UINT8 *)InitrdStart;
> +      Prop        = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,initrd-end", &Len);
> +      if ((Prop == NULL) || (Len < 0)) {
> +        return EFI_SUCCESS;
> +      }
> +
> +      InitrdEnd  = fdt64_to_cpu (ReadUnaligned64 (Prop));
> +      Blob->Size = InitrdEnd - InitrdStart;
> +      break;
> +    case 'c':
> +      Prop = fdt_getprop (DeviceTreeBase, ChosenNode, "bootargs", &Len);
> +      if ((Prop == NULL) || (Len < 0)) {
> +        return EFI_SUCCESS;
> +      }
> +
> +      Blob->Data = (UINT8 *)Prop;
> +      Blob->Size = Len;
> +      break;
> +    default:
> +      return EFI_SUCCESS;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +//
> +// The entry point of the feature.
> +//
> +
> +/**
> +  Download the kernel, the initial ramdisk, and the kernel command line from
> +  memory or DT. Construct a minimal SimpleFileSystem that contains the two
> +  image files.
> +
> +  @param ImageHandle  Image handle.
> +  @param SystemTable  EFI system table.
> +
> +  @return                       Error codes from any of the underlying
> +                                functions. On success, the function doesn't
> +                                return.
> +**/
> +EFI_STATUS
> +EFIAPI
> +CloudHvKernelLoaderFsDxeEntrypoint (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  UINTN        BlobType;
> +  KERNEL_BLOB  *CurrentBlob;
> +  KERNEL_BLOB  *KernelBlob;
> +  EFI_STATUS   Status;
> +  EFI_HANDLE   FileSystemHandle;
> +  EFI_HANDLE   InitrdLoadFile2Handle;
> +
> +  Status = gRT->GetTime (&mInitTime, NULL /* Capabilities */);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "%a: GetTime(): %r\n", __FUNCTION__, Status));
> +    return Status;
> +  }
> +
> +  //
> +  // Fetch all blobs.
> +  //
> +  for (BlobType = 0; BlobType < KernelBlobTypeMax; ++BlobType) {
> +    CurrentBlob = &mKernelBlob[BlobType];
> +    Status      = FetchBlob (CurrentBlob);
> +    if (EFI_ERROR (Status)) {
> +      goto FreeBlobs;
> +    }
> +
> +    Status = VerifyBlob (
> +               CurrentBlob->Name,
> +               CurrentBlob->Data,
> +               CurrentBlob->Size
> +               );
> +    if (EFI_ERROR (Status)) {
> +      goto FreeBlobs;
> +    }
> +
> +    mTotalBlobBytes += CurrentBlob->Size;
> +  }
> +
> +  KernelBlob = &mKernelBlob[KernelBlobTypeKernel];
> +
> +  if (KernelBlob->Data == NULL) {
> +    Status = EFI_NOT_FOUND;
> +    goto FreeBlobs;
> +  }
> +
> +  //
> +  // Create a new handle with a single VenMedia() node device path protocol on
> +  // it, plus a custom SimpleFileSystem protocol on it.
> +  //
> +  FileSystemHandle = NULL;
> +  Status           = gBS->InstallMultipleProtocolInterfaces (
> +                            &FileSystemHandle,
> +                            &gEfiDevicePathProtocolGuid,
> +                            &mFileSystemDevicePath,
> +                            &gEfiSimpleFileSystemProtocolGuid,
> +                            &mFileSystem,
> +                            NULL
> +                            );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: InstallMultipleProtocolInterfaces(): %r\n",
> +      __FUNCTION__,
> +      Status
> +      ));
> +    goto FreeBlobs;
> +  }
> +
> +  if (KernelBlob[KernelBlobTypeInitrd].Size > 0) {
> +    InitrdLoadFile2Handle = NULL;
> +    Status                = gBS->InstallMultipleProtocolInterfaces (
> +                                   &InitrdLoadFile2Handle,
> +                                   &gEfiDevicePathProtocolGuid,
> +                                   &mInitrdDevicePath,
> +                                   &gEfiLoadFile2ProtocolGuid,
> +                                   &mInitrdLoadFile2,
> +                                   NULL
> +                                   );
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((
> +        DEBUG_ERROR,
> +        "%a: InstallMultipleProtocolInterfaces(): %r\n",
> +        __FUNCTION__,
> +        Status
> +        ));
> +      goto UninstallFileSystemHandle;
> +    }
> +  }
> +
> +  return EFI_SUCCESS;
> +
> +UninstallFileSystemHandle:
> +  Status = gBS->UninstallMultipleProtocolInterfaces (
> +                  FileSystemHandle,
> +                  &gEfiDevicePathProtocolGuid,
> +                  &mFileSystemDevicePath,
> +                  &gEfiSimpleFileSystemProtocolGuid,
> +                  &mFileSystem,
> +                  NULL
> +                  );
> +  ASSERT_EFI_ERROR (Status);
> +
> +FreeBlobs:
> +  while (BlobType > 0) {
> +    CurrentBlob = &mKernelBlob[--BlobType];
> +    if (CurrentBlob->Data != NULL) {
> +      FreePool (CurrentBlob->Data);
> +      CurrentBlob->Size = 0;
> +      CurrentBlob->Data = NULL;
> +    }
> +  }
> +
> +  return Status;
> +}
> diff --git a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
> new file mode 100644
> index 0000000000..b7aa6ebb4e
> --- /dev/null
> +++ b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
> @@ -0,0 +1,56 @@
> +##  @file
> +#  DXE driver to expose the 'kernel', 'initrd' and 'cmdline' blobs
> +#  provided by Cloud Hypervisor as files in an abstract file system
> +#
> +#  Copyright (C) 2022, Arm, Limited.
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001B
> +  BASE_NAME                      = CloudHvKernelLoaderFsDxe
> +  FILE_GUID                      = 57cdf541-2a29-4ae4-97e5-76a4aa2fe090
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = CloudHvKernelLoaderFsDxeEntrypoint
> +
> +[Sources]
> +  CloudHvKernelLoaderFsDxe.c
> +
> +[Packages]
> +  ArmVirtPkg/ArmVirtPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
> +  DebugLib
> +  DevicePathLib
> +  FdtLib
> +  MemoryAllocationLib
> +  PcdLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +  UefiRuntimeServicesTableLib
> +
> +[Guids]
> +  gEfiFileInfoGuid
> +  gEfiFileSystemInfoGuid
> +  gEfiFileSystemVolumeLabelInfoIdGuid
> +  gQemuKernelLoaderFsMediaGuid
> +
> +[Protocols]
> +  gEfiDevicePathProtocolGuid                ## PRODUCES
> +  gEfiLoadFile2ProtocolGuid                 ## PRODUCES
> +  gEfiSimpleFileSystemProtocolGuid          ## PRODUCES
> +
> +[Depex]
> +  gEfiRealTimeClockArchProtocolGuid
> +
> +[FixedPcd]
> +  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] CloudHv:arm: build hob for kernel image memory as read-only
  2022-09-16  2:46 ` [PATCH 2/3] CloudHv:arm: build hob for kernel image memory as read-only Jianyong Wu
@ 2022-11-22 15:47   ` Sami Mujawar
  2022-11-23  2:56     ` Jianyong Wu
  0 siblings, 1 reply; 19+ messages in thread
From: Sami Mujawar @ 2022-11-22 15:47 UTC (permalink / raw)
  To: Jianyong Wu, devel; +Cc: ardb+tianocore, justin.he, nd@arm.com

Hi Jianyong,

Please see my feedback marked inline as [SAMI].

Regards,

Sami Mujawar

On 16/09/2022 03:46 am, Jianyong Wu wrote:
> As we use memory to pass kernel image, the memory region where kernel
> image locates should be added into hob as read-only.
>
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> ---
>   .../CloudHvVirtMemInfoLib.c                   | 66 +++++++++++++++++--
>   1 file changed, 62 insertions(+), 4 deletions(-)
>
> diff --git a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
> index 28a0c0b078..d9b7d51a16 100644
> --- a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
> +++ b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
> @@ -37,13 +37,14 @@ CloudHvVirtMemInfoPeiLibConstructor (
>     )
>   {
>     VOID                         *DeviceTreeBase;
> -  EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> +  EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes, ReadOnlyResourceAttributes;
>     INT32                        Node, Prev;
>     UINT64                       FirMemNodeBase, FirMemNodeSize;
> -  UINT64                       CurBase, MemBase;
> +  UINT64                       CurBase, MemBase, CurSizeOff;
>     UINT64                       CurSize;
> +  UINT64                       KernelStart, KernelSize;
>     CONST CHAR8                  *Type;
> -  INT32                        Len;
> +  INT32                        Len, ChosenNode;
>     CONST UINT64                 *RegProp;
>     RETURN_STATUS                PcdStatus;
>     UINT8                        Index;
> @@ -53,6 +54,8 @@ CloudHvVirtMemInfoPeiLibConstructor (
>     FirMemNodeBase     = 0;
>     FirMemNodeSize     = 0;
>     Index              = 0;
> +  CurSizeOff         = 0;
> +  KernelSize         = 0;
>     MemBase            = FixedPcdGet64 (PcdSystemMemoryBase);
>     ResourceAttributes = (
>                           EFI_RESOURCE_ATTRIBUTE_PRESENT |
> @@ -60,6 +63,12 @@ CloudHvVirtMemInfoPeiLibConstructor (
>                           EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
>                           EFI_RESOURCE_ATTRIBUTE_TESTED
>                           );
> +  ReadOnlyResourceAttributes = (
> +                                EFI_RESOURCE_ATTRIBUTE_PRESENT |
> +                                EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> +                                EFI_RESOURCE_ATTRIBUTE_TESTED |
> +                                EFI_RESOURCE_ATTRIBUTE_READ_ONLY_PROTECTED
> +                                );
>     DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
>     if (DeviceTreeBase == NULL) {
>       return EFI_NOT_FOUND;
> @@ -72,6 +81,21 @@ CloudHvVirtMemInfoPeiLibConstructor (
>       return EFI_NOT_FOUND;
>     }
>   
> +  //
> +  // Try to get kernel image info from DT
> +  //
> +  ChosenNode = fdt_path_offset (DeviceTreeBase, "/chosen");
> +  if (ChosenNode >= 0) {
> +    RegProp = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,kernel-start", &Len);
> +    if ((RegProp != NULL) && (Len > 0)) {
> +      KernelStart = (UINT64)fdt64_to_cpu (ReadUnaligned64 (RegProp));
> +      RegProp     = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,kernel-size", &Len);
> +      if ((RegProp != NULL) && (Len > 0)) {
> +        KernelSize = (UINT64)fdt64_to_cpu (ReadUnaligned64 (RegProp));
> +      }
> +    }
> +  }
> +
>     //
>     // Look for the lowest memory node
>     //
> @@ -105,11 +129,26 @@ CloudHvVirtMemInfoPeiLibConstructor (
>   
>           // We should build Hob seperately for the memory node except the first one
>           if (CurBase != MemBase) {
> +          // If kernel image resides in current memory node, build hob from CurBase to the beginning of kernel image.
> +          if ((KernelSize != 0) && (KernelStart >= CurBase) && (KernelStart + KernelSize <= CurBase + CurSize)) {
> +            CurSizeOff =  CurBase + CurSize - KernelStart;
> +            // align up with 0x1000
> +            CurSizeOff = (CurSizeOff + 0xfff) & ~0xfffUL;
> +          }
> +
>             BuildResourceDescriptorHob (
>               EFI_RESOURCE_SYSTEM_MEMORY,
>               ResourceAttributes,
>               CurBase,
> -            CurSize
> +            CurSize - CurSizeOff
> +            );
> +
> +          // Add kernel image memory region to hob as read only
> +          BuildResourceDescriptorHob (
> +            EFI_RESOURCE_SYSTEM_MEMORY,
> +            ReadOnlyResourceAttributes,
> +            CurBase + CurSize - CurSizeOff,
> +            CurSizeOff
>               );

[SAMI] Can you explain why this is required and what would happen if 
this is not done, please?  It would be good to add this description to 
the commit message.

Also, what about the initrd and the commandline?

[/SAMI]

>           } else {
>             FirMemNodeBase = CurBase;
> @@ -146,8 +185,27 @@ CloudHvVirtMemInfoPeiLibConstructor (
>       return EFI_NOT_FOUND;
>     }
>   
> +  CurSizeOff = 0;
> +  // Build hob for the lowest memory node from its base to the beginning of kernel image once the kernel image reside here
> +  if ((KernelSize != 0) && (KernelStart >= FirMemNodeBase) && (KernelStart + KernelSize <= FirMemNodeBase + FirMemNodeSize)) {
> +    CurSizeOff = FirMemNodeBase + FirMemNodeSize - KernelStart;
> +    // Caution the alignment
> +    CurSizeOff = (CurSizeOff + 0xfff) & ~0xfffUL;
> +
> +    // Add kernel image memory region to hob as read only
> +    BuildResourceDescriptorHob (
> +      EFI_RESOURCE_SYSTEM_MEMORY,
> +      ReadOnlyResourceAttributes,
> +      FirMemNodeBase + FirMemNodeSize - CurSizeOff,
> +      CurSizeOff
> +      );
> +  }
> +
> +  FirMemNodeSize -= CurSizeOff;
> +
>     PcdStatus = PcdSet64S (PcdSystemMemorySize, FirMemNodeSize);
>     ASSERT_RETURN_ERROR (PcdStatus);
> +
>     ASSERT (
>       (((UINT64)PcdGet64 (PcdFdBaseAddress) +
>         (UINT64)PcdGet32 (PcdFdSize)) <= FirMemNodeBase) ||

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf
  2022-09-16  2:46 ` [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf Jianyong Wu
@ 2022-11-22 15:48   ` Sami Mujawar
  2022-11-23  5:44     ` Jianyong Wu
       [not found]     ` <172A2070EC93BBE5.3230@groups.io>
  0 siblings, 2 replies; 19+ messages in thread
From: Sami Mujawar @ 2022-11-22 15:48 UTC (permalink / raw)
  To: Jianyong Wu, devel; +Cc: ardb+tianocore, justin.he, nd@arm.com

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

Hi Jianyong,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar

On 16/09/2022 03:46 am, Jianyong Wu wrote:
> As CloudHv kernel load fs driver is implemented, add it into dsc/fdf.
>
> Signed-off-by: Jianyong Wu<jianyong.wu@arm.com>
> ---
>   ArmVirtPkg/ArmVirtCloudHv.dsc                             | 8 +++++++-
>   ArmVirtPkg/ArmVirtCloudHv.fdf                             | 1 +
>   .../CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf | 1 -
>   3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc
> index 7ca7a391d9..92ccd4ef12 100644
> --- a/ArmVirtPkg/ArmVirtCloudHv.dsc
> +++ b/ArmVirtPkg/ArmVirtCloudHv.dsc
> @@ -37,13 +37,15 @@
>     # Virtio Support
>     VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
>     VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
> +  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
[SAMI] How does this work for CloudHv?
> +  QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
>   
>     ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
>   
>     TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
>     CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
>     BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
> -  PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +  PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf

[SAMI] I believe this brings the dependency on QemuBootOrderLib which 
requires QemuFwCfgLib. Is there a way to to implement 
QemuBootOrderLibNull to remove the dependency on QemuFwCfgLib? The 
QemuBootOrderLib APIs 
ConnectDevicesFromQem(),StoreQemuBootOrder(),SetBootOrderFromQemu () and 
GetFrontPageTimeoutFromQemu ()  could return something like 
EFI_UNSUPPORTED in QemuBootOrderLibNull.

Can you check, please?

[/SAMI]

>     PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
>     CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
>     FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
> @@ -330,6 +332,10 @@
>         NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf
>         NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
>     }
> +  ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf {
> +    <LibraryClasses>
> +      NULL|OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierLibNull.inf
> +  }
>   
>     #
>     # SCSI Bus and Disk Driver
> diff --git a/ArmVirtPkg/ArmVirtCloudHv.fdf b/ArmVirtPkg/ArmVirtCloudHv.fdf
> index 81c539590a..15b9c13c59 100644
> --- a/ArmVirtPkg/ArmVirtCloudHv.fdf
> +++ b/ArmVirtPkg/ArmVirtCloudHv.fdf
> @@ -180,6 +180,7 @@ READ_LOCK_STATUS   = TRUE
>     INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
>     INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
>     INF MdeModulePkg/Application/UiApp/UiApp.inf
> +  INF ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
>   
>     #
>     # SCSI Bus and Disk Driver
> diff --git a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
> index b7aa6ebb4e..f7b53d0747 100644
> --- a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
> +++ b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
> @@ -24,7 +24,6 @@
>     EmbeddedPkg/EmbeddedPkg.dec
>     MdePkg/MdePkg.dec
>     OvmfPkg/OvmfPkg.dec
> -  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
[SAMI] This should be part of patch 1/1.
>   
>   [LibraryClasses]
>     BaseLib

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] CloudHv:arm: build hob for kernel image memory as read-only
  2022-11-22 15:47   ` Sami Mujawar
@ 2022-11-23  2:56     ` Jianyong Wu
  0 siblings, 0 replies; 19+ messages in thread
From: Jianyong Wu @ 2022-11-23  2:56 UTC (permalink / raw)
  To: Sami Mujawar, devel@edk2.groups.io
  Cc: ardb+tianocore@kernel.org, Justin He, nd

Hi Sami, 

Inline reply.

> -----Original Message-----
> From: Sami Mujawar <Sami.Mujawar@arm.com>
> Sent: Tuesday, November 22, 2022 11:48 PM
> To: Jianyong Wu <Jianyong.Wu@arm.com>; devel@edk2.groups.io
> Cc: ardb+tianocore@kernel.org; Justin He <Justin.He@arm.com>; nd
> <nd@arm.com>
> Subject: Re: [PATCH 2/3] CloudHv:arm: build hob for kernel image memory as
> read-only
> 
> Hi Jianyong,
> 
> Please see my feedback marked inline as [SAMI].
> 
> Regards,
> 
> Sami Mujawar
> 
> On 16/09/2022 03:46 am, Jianyong Wu wrote:
> > As we use memory to pass kernel image, the memory region where kernel
> > image locates should be added into hob as read-only.
> >
> > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> > ---
> >   .../CloudHvVirtMemInfoLib.c                   | 66 +++++++++++++++++--
> >   1 file changed, 62 insertions(+), 4 deletions(-)
> >
> > diff --git
> > a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
> > b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
> > index 28a0c0b078..d9b7d51a16 100644
> > ---
> a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
> > +++
> b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
> > @@ -37,13 +37,14 @@ CloudHvVirtMemInfoPeiLibConstructor (
> >     )
> >   {
> >     VOID                         *DeviceTreeBase;
> > -  EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> > +  EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes,
> > + ReadOnlyResourceAttributes;
> >     INT32                        Node, Prev;
> >     UINT64                       FirMemNodeBase, FirMemNodeSize;
> > -  UINT64                       CurBase, MemBase;
> > +  UINT64                       CurBase, MemBase, CurSizeOff;
> >     UINT64                       CurSize;
> > +  UINT64                       KernelStart, KernelSize;
> >     CONST CHAR8                  *Type;
> > -  INT32                        Len;
> > +  INT32                        Len, ChosenNode;
> >     CONST UINT64                 *RegProp;
> >     RETURN_STATUS                PcdStatus;
> >     UINT8                        Index;
> > @@ -53,6 +54,8 @@ CloudHvVirtMemInfoPeiLibConstructor (
> >     FirMemNodeBase     = 0;
> >     FirMemNodeSize     = 0;
> >     Index              = 0;
> > +  CurSizeOff         = 0;
> > +  KernelSize         = 0;
> >     MemBase            = FixedPcdGet64 (PcdSystemMemoryBase);
> >     ResourceAttributes = (
> >                           EFI_RESOURCE_ATTRIBUTE_PRESENT | @@ -60,6
> > +63,12 @@ CloudHvVirtMemInfoPeiLibConstructor (
> >                           EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
> >                           EFI_RESOURCE_ATTRIBUTE_TESTED
> >                           );
> > +  ReadOnlyResourceAttributes = (
> > +                                EFI_RESOURCE_ATTRIBUTE_PRESENT |
> > +                                EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> > +                                EFI_RESOURCE_ATTRIBUTE_TESTED |
> > +                                EFI_RESOURCE_ATTRIBUTE_READ_ONLY_PROTECTED
> > +                                );
> >     DeviceTreeBase = (VOID *)(UINTN)PcdGet64
> (PcdDeviceTreeInitialBaseAddress);
> >     if (DeviceTreeBase == NULL) {
> >       return EFI_NOT_FOUND;
> > @@ -72,6 +81,21 @@ CloudHvVirtMemInfoPeiLibConstructor (
> >       return EFI_NOT_FOUND;
> >     }
> >
> > +  //
> > +  // Try to get kernel image info from DT  //  ChosenNode =
> > + fdt_path_offset (DeviceTreeBase, "/chosen");  if (ChosenNode >= 0) {
> > +    RegProp = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,kernel-
> start", &Len);
> > +    if ((RegProp != NULL) && (Len > 0)) {
> > +      KernelStart = (UINT64)fdt64_to_cpu (ReadUnaligned64 (RegProp));
> > +      RegProp     = fdt_getprop (DeviceTreeBase, ChosenNode,
> "linux,kernel-size", &Len);
> > +      if ((RegProp != NULL) && (Len > 0)) {
> > +        KernelSize = (UINT64)fdt64_to_cpu (ReadUnaligned64 (RegProp));
> > +      }
> > +    }
> > +  }
> > +
> >     //
> >     // Look for the lowest memory node
> >     //
> > @@ -105,11 +129,26 @@ CloudHvVirtMemInfoPeiLibConstructor (
> >
> >           // We should build Hob seperately for the memory node except the
> first one
> >           if (CurBase != MemBase) {
> > +          // If kernel image resides in current memory node, build hob from
> CurBase to the beginning of kernel image.
> > +          if ((KernelSize != 0) && (KernelStart >= CurBase) && (KernelStart +
> KernelSize <= CurBase + CurSize)) {
> > +            CurSizeOff =  CurBase + CurSize - KernelStart;
> > +            // align up with 0x1000
> > +            CurSizeOff = (CurSizeOff + 0xfff) & ~0xfffUL;
> > +          }
> > +
> >             BuildResourceDescriptorHob (
> >               EFI_RESOURCE_SYSTEM_MEMORY,
> >               ResourceAttributes,
> >               CurBase,
> > -            CurSize
> > +            CurSize - CurSizeOff
> > +            );
> > +
> > +          // Add kernel image memory region to hob as read only
> > +          BuildResourceDescriptorHob (
> > +            EFI_RESOURCE_SYSTEM_MEMORY,
> > +            ReadOnlyResourceAttributes,
> > +            CurBase + CurSize - CurSizeOff,
> > +            CurSizeOff
> >               );
> 
> [SAMI] Can you explain why this is required and what would happen if this is
> not done, please?  It would be good to add this description to the commit
> message.
> 	
As we need put kernel image into a range of memory, we can't let uefi modify that region, so we need set it as read-only.
If not, kernel won't start after load.
But I'm not sure how does it impact kernel, for example, the memory used for kernel may decrease, however, I have no better way to do this.

> Also, what about the initrd and the commandline?

I have not considered initrd yet. If there is a requirement for it, we can add later.
Command line is just a string, so it can be conveyed by fdt directly without occupy memory.

Thanks
Jianyong
> 
> [/SAMI]
> 
> >           } else {
> >             FirMemNodeBase = CurBase;
> > @@ -146,8 +185,27 @@ CloudHvVirtMemInfoPeiLibConstructor (
> >       return EFI_NOT_FOUND;
> >     }
> >
> > +  CurSizeOff = 0;
> > +  // Build hob for the lowest memory node from its base to the
> > + beginning of kernel image once the kernel image reside here  if
> ((KernelSize != 0) && (KernelStart >= FirMemNodeBase) && (KernelStart +
> KernelSize <= FirMemNodeBase + FirMemNodeSize)) {
> > +    CurSizeOff = FirMemNodeBase + FirMemNodeSize - KernelStart;
> > +    // Caution the alignment
> > +    CurSizeOff = (CurSizeOff + 0xfff) & ~0xfffUL;
> > +
> > +    // Add kernel image memory region to hob as read only
> > +    BuildResourceDescriptorHob (
> > +      EFI_RESOURCE_SYSTEM_MEMORY,
> > +      ReadOnlyResourceAttributes,
> > +      FirMemNodeBase + FirMemNodeSize - CurSizeOff,
> > +      CurSizeOff
> > +      );
> > +  }
> > +
> > +  FirMemNodeSize -= CurSizeOff;
> > +
> >     PcdStatus = PcdSet64S (PcdSystemMemorySize, FirMemNodeSize);
> >     ASSERT_RETURN_ERROR (PcdStatus);
> > +
> >     ASSERT (
> >       (((UINT64)PcdGet64 (PcdFdBaseAddress) +
> >         (UINT64)PcdGet32 (PcdFdSize)) <= FirMemNodeBase) ||

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf
  2022-11-22 15:48   ` Sami Mujawar
@ 2022-11-23  5:44     ` Jianyong Wu
       [not found]     ` <172A2070EC93BBE5.3230@groups.io>
  1 sibling, 0 replies; 19+ messages in thread
From: Jianyong Wu @ 2022-11-23  5:44 UTC (permalink / raw)
  To: Sami Mujawar, devel@edk2.groups.io
  Cc: ardb+tianocore@kernel.org, Justin He, nd

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

Hi Sami,

Inline reply

From: Sami Mujawar <Sami.Mujawar@arm.com>
Sent: Tuesday, November 22, 2022 11:48 PM
To: Jianyong Wu <Jianyong.Wu@arm.com>; devel@edk2.groups.io
Cc: ardb+tianocore@kernel.org; Justin He <Justin.He@arm.com>; nd <nd@arm.com>
Subject: Re: [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf


Hi Jianyong,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar
On 16/09/2022 03:46 am, Jianyong Wu wrote:

As CloudHv kernel load fs driver is implemented, add it into dsc/fdf.



Signed-off-by: Jianyong Wu <jianyong.wu@arm.com><mailto:jianyong.wu@arm.com>

---

 ArmVirtPkg/ArmVirtCloudHv.dsc                             | 8 +++++++-

 ArmVirtPkg/ArmVirtCloudHv.fdf                             | 1 +

 .../CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf | 1 -

 3 files changed, 8 insertions(+), 2 deletions(-)



diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc

index 7ca7a391d9..92ccd4ef12 100644

--- a/ArmVirtPkg/ArmVirtCloudHv.dsc

+++ b/ArmVirtPkg/ArmVirtCloudHv.dsc

@@ -37,13 +37,15 @@

   # Virtio Support

   VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf

   VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf

+  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
[SAMI] How does this work for CloudHv?

Yes, like you said below, it’s due to the change of BootManagerLib.




+  QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf



   ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf



   TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf

   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf

   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf

-  PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf

+  PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf

[SAMI] I believe this brings the dependency on QemuBootOrderLib which requires QemuFwCfgLib. Is there a way to to implement QemuBootOrderLibNull to remove the dependency on QemuFwCfgLib? The QemuBootOrderLib APIs  ConnectDevicesFromQem(), StoreQemuBootOrder(), SetBootOrderFromQemu () and GetFrontPageTimeoutFromQemu ()  could return something like EFI_UNSUPPORTED in QemuBootOrderLibNull.

Can you check, please?

Sounds a good idea, let me have a try.

[/SAMI]



   PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf

   CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf

   FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf

@@ -330,6 +332,10 @@

       NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf

       NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf

   }

+  ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf {

+    <LibraryClasses>

+      NULL|OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierLibNull.inf

+  }



   #

   # SCSI Bus and Disk Driver

diff --git a/ArmVirtPkg/ArmVirtCloudHv.fdf b/ArmVirtPkg/ArmVirtCloudHv.fdf

index 81c539590a..15b9c13c59 100644

--- a/ArmVirtPkg/ArmVirtCloudHv.fdf

+++ b/ArmVirtPkg/ArmVirtCloudHv.fdf

@@ -180,6 +180,7 @@ READ_LOCK_STATUS   = TRUE

   INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf

   INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf

   INF MdeModulePkg/Application/UiApp/UiApp.inf

+  INF ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf



   #

   # SCSI Bus and Disk Driver

diff --git a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

index b7aa6ebb4e..f7b53d0747 100644

--- a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

+++ b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

@@ -24,7 +24,6 @@

   EmbeddedPkg/EmbeddedPkg.dec

   MdePkg/MdePkg.dec

   OvmfPkg/OvmfPkg.dec

-  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
[SAMI] This should be part of patch 1/1.


Yes, I will fix it next version



Thanks

Jianyong



 [LibraryClasses]

   BaseLib

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/3] CloudHv:arm: add kernel load fs driver
  2022-11-22 15:46   ` Sami Mujawar
@ 2022-11-23  6:26     ` Jianyong Wu
  0 siblings, 0 replies; 19+ messages in thread
From: Jianyong Wu @ 2022-11-23  6:26 UTC (permalink / raw)
  To: Sami Mujawar, devel@edk2.groups.io
  Cc: ardb+tianocore@kernel.org, Justin He, nd

Hi Sami,

You can try it as following steps:

1. build edk2 for CLOUDHv from https://github.com/jongwu/edk2/tree/direct_kernel
2. build cloud hypervisor with my patch set from https://github.com/jongwu/cloud-hypervisor/tree/direct_kernel
	a. install rust environment, see https://www.rust-lang.org/tools/install
	b. clone code and use "cargo build" in the top dir of the code tree
	c find the binary using "find ./ -name cloud-hypervisor"
3. prepare kernel image, I think default config can do.
4 prepare a disk image. I often download ubuntu image from https://cloud-images.ubuntu.com/focal/current/ , then convert it into raw using:
    qemu-img convert -p -f qcow2 -O raw *.img *.raw
5. try to run a cloud hypervisor VM by the following command line:

       ./cloud-hypervisor \
        --cpus boot=1 \
        --memory size=2G \
        --serial tty \
        --console off \
        --disk \
        path=focal-server-cloudimg-arm64.raw \
        --cmdline "root=/dev/vda1 console=ttyAMA0 earlycon rw" \
        --firmware CLOUDHV_EFI.fd \
        --kernel Image                             ## comment out this line to try grub kernel load

You can see that the kernel load role is taken by edk2 and is faster compared to grub load. What's more we can specify kernel image without loading kernel from disk.
Any problem, contact me.

Thanks
Jianyong

> -----Original Message-----
> From: Sami Mujawar <Sami.Mujawar@arm.com>
> Sent: Tuesday, November 22, 2022 11:47 PM
> To: Jianyong Wu <Jianyong.Wu@arm.com>; devel@edk2.groups.io
> Cc: ardb+tianocore@kernel.org; Justin He <Justin.He@arm.com>; nd
> <nd@arm.com>
> Subject: Re: [PATCH 1/3] CloudHv:arm: add kernel load fs driver
> 
> Hi Jianyoung,
> 
> I am trying to understand your patch series. Is it possible to send me the
> steps to test your patches using CloudHv, please?
> 
> Also, please see my response for patch 2/3 as I have some queries.
> 
> Regards,
> 
> Sami Mujawar
> 
> 
> On 16/09/2022 03:46 am, Jianyong Wu wrote:
> > This is used for supporting direct kernel boot in CloudHv. CloudHv
> > will store kernel image in system ram and pass kernel info through DT.
> > It's firmware's responsibility to fetch the kernel data and create a
> > file device to feed for loadImage
> >
> > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> > ---
> >   .../CloudHvKernelLoaderFsDxe.c                | 969 ++++++++++++++++++
> >   .../CloudHvKernelLoaderFsDxe.inf              |  56 +
> >   2 files changed, 1025 insertions(+)
> >   create mode 100644
> ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c
> >   create mode 100644
> > ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
> >
> > diff --git
> > a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c
> > b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c
> > new file mode 100644
> > index 0000000000..e4cfcfab72
> > --- /dev/null
> > +++
> b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.c
> > @@ -0,0 +1,969 @@
> > +/** @file
> > +  DXE driver to expose the 'kernel', 'initrd' and 'cmdline' blobs
> > +  provided by Cloud Hypervisor as files in an abstract file system
> > +
> > +  Copyright (C) 2022, Arm, Limited.
> > +
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent **/
> > +
> > +#include <PiPei.h>
> > +#include <PiDxe.h>
> > +
> > +#include <libfdt.h>
> > +#include <Guid/FileInfo.h>
> > +#include <Guid/FileSystemInfo.h>
> > +#include <Guid/FileSystemVolumeLabelInfo.h>
> > +#include <Guid/LinuxEfiInitrdMedia.h> #include
> > +<Guid/QemuKernelLoaderFsMedia.h> #include <Library/BaseLib.h>
> > +#include <Library/BaseMemoryLib.h> #include
> > +<Library/BlobVerifierLib.h> #include <Library/DebugLib.h> #include
> > +<Library/DevicePathLib.h> #include <Library/MemoryAllocationLib.h>
> > +#include <Library/PcdLib.h> #include <Library/PrePiLib.h> #include
> > +<Library/UefiBootServicesTableLib.h>
> > +#include <Library/UefiRuntimeServicesTableLib.h>
> > +#include <Protocol/DevicePath.h>
> > +#include <Protocol/LoadFile2.h>
> > +#include <Protocol/SimpleFileSystem.h>
> > +
> > +//
> > +// Static data that hosts the data blobs and serves file requests.
> > +//
> > +typedef enum {
> > +  KernelBlobTypeKernel,
> > +  KernelBlobTypeInitrd,
> > +  KernelBlobTypeCommandLine,
> > +  KernelBlobTypeMax
> > +} KERNEL_BLOB_TYPE;
> > +
> > +typedef struct {
> > +  CONST CHAR16    Name[8];
> > +  UINT32          Size;
> > +  UINT8           *Data;
> > +} KERNEL_BLOB;
> > +
> > +STATIC KERNEL_BLOB  mKernelBlob[KernelBlobTypeMax] = {
> > +  {
> > +    L"kernel",
> > +  },{
> > +    L"initrd",
> > +  },{
> > +    L"cmdline",
> > +  }
> > +};
> > +
> > +//
> > +// Device path for the handle that incorporates our "EFI stub filesystem".
> > +//
> > +#pragma pack (1)
> > +typedef struct {
> > +  VENDOR_DEVICE_PATH          VenMediaNode;
> > +  EFI_DEVICE_PATH_PROTOCOL    EndNode;
> > +} SINGLE_VENMEDIA_NODE_DEVPATH;
> > +#pragma pack ()
> > +
> > +STATIC CONST SINGLE_VENMEDIA_NODE_DEVPATH
> mFileSystemDevicePath = {
> > +  {
> > +    {
> > +      MEDIA_DEVICE_PATH, MEDIA_VENDOR_DP,
> > +      { sizeof (VENDOR_DEVICE_PATH)       }
> > +    },
> > +    QEMU_KERNEL_LOADER_FS_MEDIA_GUID
> > +  },  {
> > +    END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE,
> > +    { sizeof (EFI_DEVICE_PATH_PROTOCOL) }
> > +  }
> > +};
> > +
> > +STATIC CONST SINGLE_VENMEDIA_NODE_DEVPATH  mInitrdDevicePath =
> {
> > +  {
> > +    {
> > +      MEDIA_DEVICE_PATH, MEDIA_VENDOR_DP,
> > +      { sizeof (VENDOR_DEVICE_PATH)       }
> > +    },
> > +    LINUX_EFI_INITRD_MEDIA_GUID
> > +  },  {
> > +    END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE,
> > +    { sizeof (EFI_DEVICE_PATH_PROTOCOL) }
> > +  }
> > +};
> > +
> > +//
> > +// The "file in the EFI stub filesystem" abstraction.
> > +//
> > +STATIC EFI_TIME  mInitTime;
> > +STATIC UINT64    mTotalBlobBytes;
> > +
> > +#define STUB_FILE_SIG  SIGNATURE_64 ('S', 'T', 'U', 'B', 'F', 'I', 'L', 'E')
> > +
> > +typedef struct {
> > +  UINT64               Signature; // Carries STUB_FILE_SIG.
> > +
> > +  KERNEL_BLOB_TYPE     BlobType; // Index into mKernelBlob.
> KernelBlobTypeMax
> > +                                 // denotes the root directory of the filesystem.
> > +
> > +  UINT64               Position; // Byte position for regular files;
> > +                                 // next directory entry to return for the root
> > +                                 // directory.
> > +
> > +  EFI_FILE_PROTOCOL    File;   // Standard protocol interface.
> > +} STUB_FILE;
> > +
> > +#define STUB_FILE_FROM_FILE(FilePointer) \
> > +        CR (FilePointer, STUB_FILE, File, STUB_FILE_SIG)
> > +
> > +/**
> > +  Helper function that formats an EFI_FILE_INFO structure into the
> > +  user-allocated buffer, for any valid KERNEL_BLOB_TYPE value (including
> > +  KernelBlobTypeMax, which stands for the root directory).
> > +
> > +  The interface follows the EFI_FILE_GET_INFO -- and for directories, the
> > +  EFI_FILE_READ -- interfaces.
> > +
> > +  @param[in]     BlobType     The KERNEL_BLOB_TYPE value identifying the
> fw_cfg
> > +                              blob backing the STUB_FILE that information is
> > +                              being requested about. If BlobType equals
> > +                              KernelBlobTypeMax, then information will be
> > +                              provided about the root directory of the
> > +                              filesystem.
> > +
> > +  @param[in,out] BufferSize  On input, the size of Buffer. On output, the
> > +                             amount of data returned in Buffer. In both cases,
> > +                             the size is measured in bytes.
> > +
> > +  @param[out]    Buffer      A pointer to the data buffer to return. The
> > +                             buffer's type is EFI_FILE_INFO.
> > +
> > +  @retval EFI_SUCCESS           The information was returned.
> > +  @retval EFI_BUFFER_TOO_SMALL  BufferSize is too small to store the
> > +                                EFI_FILE_INFO structure. BufferSize has been
> > +                                updated with the size needed to complete the
> > +                                request.
> > +**/
> > +EFI_STATUS
> > +ConvertKernelBlobTypeToFileInfo (
> > +  IN KERNEL_BLOB_TYPE  BlobType,
> > +  IN OUT UINTN         *BufferSize,
> > +  OUT VOID             *Buffer
> > +  )
> > +{
> > +  CONST CHAR16  *Name;
> > +  UINT64        FileSize;
> > +  UINT64        Attribute;
> > +
> > +  UINTN          NameSize;
> > +  UINTN          FileInfoSize;
> > +  EFI_FILE_INFO  *FileInfo;
> > +  UINTN          OriginalBufferSize;
> > +
> > +  if (BlobType == KernelBlobTypeMax) {
> > +    //
> > +    // getting file info about the root directory
> > +    //
> > +    Name      = L"\\";
> > +    FileSize  = KernelBlobTypeMax;
> > +    Attribute = EFI_FILE_READ_ONLY | EFI_FILE_DIRECTORY;
> > +  } else {
> > +    CONST KERNEL_BLOB  *Blob;
> > +
> > +    Blob      = &mKernelBlob[BlobType];
> > +    Name      = Blob->Name;
> > +    FileSize  = Blob->Size;
> > +    Attribute = EFI_FILE_READ_ONLY;
> > +  }
> > +
> > +  NameSize     = (StrLen (Name) + 1) * 2;
> > +  FileInfoSize = OFFSET_OF (EFI_FILE_INFO, FileName) + NameSize;
> > +  ASSERT (FileInfoSize >= sizeof *FileInfo);
> > +
> > +  OriginalBufferSize = *BufferSize;
> > +  *BufferSize        = FileInfoSize;
> > +  if (OriginalBufferSize < *BufferSize) {
> > +    return EFI_BUFFER_TOO_SMALL;
> > +  }
> > +
> > +  FileInfo               = (EFI_FILE_INFO *)Buffer;
> > +  FileInfo->Size         = FileInfoSize;
> > +  FileInfo->FileSize     = FileSize;
> > +  FileInfo->PhysicalSize = FileSize;
> > +  FileInfo->Attribute    = Attribute;
> > +
> > +  CopyMem (&FileInfo->CreateTime, &mInitTime, sizeof mInitTime);
> > +  CopyMem (&FileInfo->LastAccessTime, &mInitTime, sizeof mInitTime);
> > +  CopyMem (&FileInfo->ModificationTime, &mInitTime, sizeof mInitTime);
> > +  CopyMem (FileInfo->FileName, Name, NameSize);
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > +  Reads data from a file, or continues scanning a directory.
> > +
> > +  @param[in]     This        A pointer to the EFI_FILE_PROTOCOL instance that
> > +                             is the file handle to read data from.
> > +
> > +  @param[in,out] BufferSize  On input, the size of the Buffer. On output,
> the
> > +                             amount of data returned in Buffer. In both cases,
> > +                             the size is measured in bytes. If the read goes
> > +                             beyond the end of the file, the read length is
> > +                             truncated to the end of the file.
> > +
> > +                             If This is a directory, the function reads the
> > +                             directory entry at the current position and
> > +                             returns the entry (as EFI_FILE_INFO) in Buffer. If
> > +                             there are no more directory entries, the
> > +                             BufferSize is set to zero on output.
> > +
> > +  @param[out]    Buffer      The buffer into which the data is read.
> > +
> > +  @retval EFI_SUCCESS           Data was read.
> > +  @retval EFI_NO_MEDIA          The device has no medium.
> > +  @retval EFI_DEVICE_ERROR      The device reported an error.
> > +  @retval EFI_DEVICE_ERROR      An attempt was made to read from a
> deleted
> > +                                file.
> > +  @retval EFI_DEVICE_ERROR      On entry, the current file position is
> beyond
> > +                                the end of the file.
> > +  @retval EFI_VOLUME_CORRUPTED  The file system structures are
> corrupted.
> > +  @retval EFI_BUFFER_TOO_SMALL  The BufferSize is too small to store
> the
> > +                                current directory entry as a EFI_FILE_INFO
> > +                                structure. BufferSize has been updated with the
> > +                                size needed to complete the request, and the
> > +                                directory position has not been advanced.
> > +**/
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +StubFileRead (
> > +  IN EFI_FILE_PROTOCOL  *This,
> > +  IN OUT UINTN          *BufferSize,
> > +  OUT VOID              *Buffer
> > +  )
> > +{
> > +  STUB_FILE          *StubFile;
> > +  CONST KERNEL_BLOB  *Blob;
> > +  UINT64             Left;
> > +
> > +  StubFile = STUB_FILE_FROM_FILE (This);
> > +
> > +  //
> > +  // Scanning the root directory?
> > +  //
> > +  if (StubFile->BlobType == KernelBlobTypeMax) {
> > +    EFI_STATUS  Status;
> > +
> > +    if (StubFile->Position == KernelBlobTypeMax) {
> > +      //
> > +      // Scanning complete.
> > +      //
> > +      *BufferSize = 0;
> > +      return EFI_SUCCESS;
> > +    }
> > +
> > +    Status = ConvertKernelBlobTypeToFileInfo (
> > +               (KERNEL_BLOB_TYPE)StubFile->Position,
> > +               BufferSize,
> > +               Buffer
> > +               );
> > +    if (EFI_ERROR (Status)) {
> > +      return Status;
> > +    }
> > +
> > +    ++StubFile->Position;
> > +    return EFI_SUCCESS;
> > +  }
> > +
> > +  //
> > +  // Reading a file.
> > +  //
> > +  Blob = &mKernelBlob[StubFile->BlobType];
> > +  if (StubFile->Position > Blob->Size) {
> > +    return EFI_DEVICE_ERROR;
> > +  }
> > +
> > +  Left = Blob->Size - StubFile->Position;
> > +  if (*BufferSize > Left) {
> > +    *BufferSize = (UINTN)Left;
> > +  }
> > +
> > +  if (Blob->Data != NULL) {
> > +    CopyMem (Buffer, Blob->Data + StubFile->Position, *BufferSize);
> > +  }
> > +
> > +  StubFile->Position += *BufferSize;
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > +  Closes a specified file handle.
> > +
> > +  @param[in] This  A pointer to the EFI_FILE_PROTOCOL instance that is
> the file
> > +                   handle to close.
> > +
> > +  @retval EFI_SUCCESS  The file was closed.
> > +**/
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +StubFileClose (
> > +  IN EFI_FILE_PROTOCOL  *This
> > +  )
> > +{
> > +  FreePool (STUB_FILE_FROM_FILE (This));
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > +  Close and delete the file handle.
> > +
> > +  @param[in] This  A pointer to the EFI_FILE_PROTOCOL instance that is
> the
> > +                   handle to the file to delete.
> > +
> > +  @retval EFI_SUCCESS              The file was closed and deleted, and the
> > +                                   handle was closed.
> > +  @retval EFI_WARN_DELETE_FAILURE  The handle was closed, but the file
> was not
> > +                                   deleted.
> > +
> > +**/
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +StubFileDelete (
> > +  IN EFI_FILE_PROTOCOL  *This
> > +  )
> > +{
> > +  FreePool (STUB_FILE_FROM_FILE (This));
> > +  return EFI_WARN_DELETE_FAILURE;
> > +}
> > +
> > +//
> > +// Protocol member functions for File.
> > +//
> > +
> > +/**
> > +  Opens a new file relative to the source file's location.
> > +
> > +  (Forward declaration.)
> > +
> > +  @param[in]  This        A pointer to the EFI_FILE_PROTOCOL instance that
> is
> > +                          the file handle to the source location. This would
> > +                          typically be an open handle to a directory.
> > +
> > +  @param[out] NewHandle   A pointer to the location to return the
> opened handle
> > +                          for the new file.
> > +
> > +  @param[in]  FileName    The Null-terminated string of the name of the
> file to
> > +                          be opened. The file name may contain the following
> > +                          path modifiers: "\", ".", and "..".
> > +
> > +  @param[in]  OpenMode    The mode to open the file. The only valid
> > +                          combinations that the file may be opened with are:
> > +                          Read, Read/Write, or Create/Read/Write.
> > +
> > +  @param[in]  Attributes  Only valid for EFI_FILE_MODE_CREATE, in which
> case
> > +                          these are the attribute bits for the newly created
> > +                          file.
> > +
> > +  @retval EFI_SUCCESS           The file was opened.
> > +  @retval EFI_NOT_FOUND         The specified file could not be found on
> the
> > +                                device.
> > +  @retval EFI_NO_MEDIA          The device has no medium.
> > +  @retval EFI_MEDIA_CHANGED     The device has a different medium in it
> or the
> > +                                medium is no longer supported.
> > +  @retval EFI_DEVICE_ERROR      The device reported an error.
> > +  @retval EFI_VOLUME_CORRUPTED  The file system structures are
> corrupted.
> > +  @retval EFI_WRITE_PROTECTED   An attempt was made to create a file,
> or open a
> > +                                file for write when the media is
> > +                                write-protected.
> > +  @retval EFI_ACCESS_DENIED     The service denied access to the file.
> > +  @retval EFI_OUT_OF_RESOURCES  Not enough resources were available
> to open the
> > +                                file.
> > +  @retval EFI_VOLUME_FULL       The volume is full.
> > +**/
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +StubFileOpen (
> > +  IN EFI_FILE_PROTOCOL   *This,
> > +  OUT EFI_FILE_PROTOCOL  **NewHandle,
> > +  IN CHAR16              *FileName,
> > +  IN UINT64              OpenMode,
> > +  IN UINT64              Attributes
> > +  );
> > +
> > +/**
> > +  Returns information about a file.
> > +
> > +  @param[in]     This             A pointer to the EFI_FILE_PROTOCOL instance
> > +                                  that is the file handle the requested
> > +                                  information is for.
> > +
> > +  @param[in]     InformationType  The type identifier GUID for the
> information
> > +                                  being requested. The following information
> > +                                  types are supported, storing the
> > +                                  corresponding structures in Buffer:
> > +
> > +                                  - gEfiFileInfoGuid: EFI_FILE_INFO
> > +
> > +                                  - gEfiFileSystemInfoGuid:
> > +                                    EFI_FILE_SYSTEM_INFO
> > +
> > +                                  - gEfiFileSystemVolumeLabelInfoIdGuid:
> > +                                    EFI_FILE_SYSTEM_VOLUME_LABEL
> > +
> > +  @param[in,out] BufferSize       On input, the size of Buffer. On output,
> the
> > +                                  amount of data returned in Buffer. In both
> > +                                  cases, the size is measured in bytes.
> > +
> > +  @param[out]    Buffer           A pointer to the data buffer to return. The
> > +                                  buffer's type is indicated by
> > +                                  InformationType.
> > +
> > +  @retval EFI_SUCCESS           The information was returned.
> > +  @retval EFI_UNSUPPORTED       The InformationType is not known.
> > +  @retval EFI_NO_MEDIA          The device has no medium.
> > +  @retval EFI_DEVICE_ERROR      The device reported an error.
> > +  @retval EFI_VOLUME_CORRUPTED  The file system structures are
> corrupted.
> > +  @retval EFI_BUFFER_TOO_SMALL  The BufferSize is too small to store
> the
> > +                                information structure requested by
> > +                                InformationType. BufferSize has been updated
> > +                                with the size needed to complete the request.
> > +**/
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +StubFileGetInfo (
> > +  IN EFI_FILE_PROTOCOL  *This,
> > +  IN EFI_GUID           *InformationType,
> > +  IN OUT UINTN          *BufferSize,
> > +  OUT VOID              *Buffer
> > +  )
> > +{
> > +  CONST STUB_FILE  *StubFile;
> > +  UINTN            OriginalBufferSize;
> > +
> > +  StubFile = STUB_FILE_FROM_FILE (This);
> > +
> > +  if (CompareGuid (InformationType, &gEfiFileInfoGuid)) {
> > +    return ConvertKernelBlobTypeToFileInfo (
> > +             StubFile->BlobType,
> > +             BufferSize,
> > +             Buffer
> > +             );
> > +  }
> > +
> > +  OriginalBufferSize = *BufferSize;
> > +
> > +  if (CompareGuid (InformationType, &gEfiFileSystemInfoGuid)) {
> > +    EFI_FILE_SYSTEM_INFO  *FileSystemInfo;
> > +
> > +    *BufferSize = sizeof *FileSystemInfo;
> > +    if (OriginalBufferSize < *BufferSize) {
> > +      return EFI_BUFFER_TOO_SMALL;
> > +    }
> > +
> > +    FileSystemInfo                 = (EFI_FILE_SYSTEM_INFO *)Buffer;
> > +    FileSystemInfo->Size           = sizeof *FileSystemInfo;
> > +    FileSystemInfo->ReadOnly       = TRUE;
> > +    FileSystemInfo->VolumeSize     = mTotalBlobBytes;
> > +    FileSystemInfo->FreeSpace      = 0;
> > +    FileSystemInfo->BlockSize      = 1;
> > +    FileSystemInfo->VolumeLabel[0] = L'\0';
> > +
> > +    return EFI_SUCCESS;
> > +  }
> > +
> > +  if (CompareGuid (InformationType,
> &gEfiFileSystemVolumeLabelInfoIdGuid)) {
> > +    EFI_FILE_SYSTEM_VOLUME_LABEL  *FileSystemVolumeLabel;
> > +
> > +    *BufferSize = sizeof *FileSystemVolumeLabel;
> > +    if (OriginalBufferSize < *BufferSize) {
> > +      return EFI_BUFFER_TOO_SMALL;
> > +    }
> > +
> > +    FileSystemVolumeLabel                 = (EFI_FILE_SYSTEM_VOLUME_LABEL
> *)Buffer;
> > +    FileSystemVolumeLabel->VolumeLabel[0] = L'\0';
> > +
> > +    return EFI_SUCCESS;
> > +  }
> > +
> > +  return EFI_UNSUPPORTED;
> > +}
> > +
> > +//
> > +// External definition of the file protocol template.
> > +//
> > +STATIC CONST EFI_FILE_PROTOCOL  mEfiFileProtocolTemplate = {
> > +  EFI_FILE_PROTOCOL_REVISION, // revision 1
> > +  StubFileOpen,
> > +  StubFileClose,
> > +  StubFileDelete,
> > +  StubFileRead,
> > +  NULL,
> > +  NULL,
> > +  NULL,
> > +  StubFileGetInfo,
> > +  NULL,
> > +  NULL,
> > +  NULL,                       // OpenEx, revision 2
> > +  NULL,                       // ReadEx, revision 2
> > +  NULL,                       // WriteEx, revision 2
> > +  NULL                        // FlushEx, revision 2
> > +};
> > +
> > +/**
> > +  Opens a new file relative to the source file's location.
> > +
> > +  (Forward declaration.)
> > +
> > +  @param[in]  This        A pointer to the EFI_FILE_PROTOCOL instance that
> is
> > +                          the file handle to the source location. This would
> > +                          typically be an open handle to a directory.
> > +
> > +  @param[out] NewHandle   A pointer to the location to return the
> opened handle
> > +                          for the new file.
> > +
> > +  @param[in]  FileName    The Null-terminated string of the name of the
> file to
> > +                          be opened. The file name may contain the following
> > +                          path modifiers: "\", ".", and "..".
> > +
> > +  @param[in]  OpenMode    The mode to open the file. The only valid
> > +                          combinations that the file may be opened with are:
> > +                          Read, Read/Write, or Create/Read/Write.
> > +
> > +  @param[in]  Attributes  Only valid for EFI_FILE_MODE_CREATE, in which
> case
> > +                          these are the attribute bits for the newly created
> > +                          file.
> > +
> > +  @retval EFI_SUCCESS           The file was opened.
> > +  @retval EFI_NOT_FOUND         The specified file could not be found on
> the
> > +                                device.
> > +  @retval EFI_NO_MEDIA          The device has no medium.
> > +  @retval EFI_MEDIA_CHANGED     The device has a different medium in it
> or the
> > +                                medium is no longer supported.
> > +  @retval EFI_DEVICE_ERROR      The device reported an error.
> > +  @retval EFI_VOLUME_CORRUPTED  The file system structures are
> corrupted.
> > +  @retval EFI_WRITE_PROTECTED   An attempt was made to create a file,
> or open a
> > +                                file for write when the media is
> > +                                write-protected.
> > +  @retval EFI_ACCESS_DENIED     The service denied access to the file.
> > +  @retval EFI_OUT_OF_RESOURCES  Not enough resources were available
> to open the
> > +                                file.
> > +  @retval EFI_VOLUME_FULL       The volume is full.
> > +**/
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +StubFileOpen (
> > +  IN EFI_FILE_PROTOCOL   *This,
> > +  OUT EFI_FILE_PROTOCOL  **NewHandle,
> > +  IN CHAR16              *FileName,
> > +  IN UINT64              OpenMode,
> > +  IN UINT64              Attributes
> > +  )
> > +{
> > +  CONST STUB_FILE  *StubFile;
> > +  UINTN            BlobType;
> > +  STUB_FILE        *NewStubFile;
> > +
> > +  //
> > +  // We're read-only.
> > +  //
> > +  switch (OpenMode) {
> > +    case EFI_FILE_MODE_READ:
> > +      break;
> > +
> > +    case EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE:
> > +    case EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE |
> EFI_FILE_MODE_CREATE:
> > +      return EFI_WRITE_PROTECTED;
> > +
> > +    default:
> > +      return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  //
> > +  // Only the root directory supports opening files in it.
> > +  //
> > +  StubFile = STUB_FILE_FROM_FILE (This);
> > +  if (StubFile->BlobType != KernelBlobTypeMax) {
> > +    return EFI_UNSUPPORTED;
> > +  }
> > +
> > +  //
> > +  // Locate the file.
> > +  //
> > +  for (BlobType = 0; BlobType < KernelBlobTypeMax; ++BlobType) {
> > +    if (StrCmp (FileName, mKernelBlob[BlobType].Name) == 0) {
> > +      break;
> > +    }
> > +  }
> > +
> > +  if (BlobType == KernelBlobTypeMax) {
> > +    return EFI_NOT_FOUND;
> > +  }
> > +
> > +  //
> > +  // Found it.
> > +  //
> > +  NewStubFile = AllocatePool (sizeof *NewStubFile);
> > +  if (NewStubFile == NULL) {
> > +    return EFI_OUT_OF_RESOURCES;
> > +  }
> > +
> > +  NewStubFile->Signature = STUB_FILE_SIG;
> > +  NewStubFile->BlobType  = (KERNEL_BLOB_TYPE)BlobType;
> > +  NewStubFile->Position  = 0;
> > +  CopyMem (
> > +    &NewStubFile->File,
> > +    &mEfiFileProtocolTemplate,
> > +    sizeof mEfiFileProtocolTemplate
> > +    );
> > +  *NewHandle = &NewStubFile->File;
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +//
> > +// Protocol member functions for SimpleFileSystem.
> > +//
> > +
> > +/**
> > +  Open the root directory on a volume.
> > +
> > +  @param[in]  This  A pointer to the volume to open the root directory on.
> > +
> > +  @param[out] Root  A pointer to the location to return the opened file
> handle
> > +                    for the root directory in.
> > +
> > +  @retval EFI_SUCCESS           The device was opened.
> > +  @retval EFI_UNSUPPORTED       This volume does not support the
> requested file
> > +                                system type.
> > +  @retval EFI_NO_MEDIA          The device has no medium.
> > +  @retval EFI_DEVICE_ERROR      The device reported an error.
> > +  @retval EFI_VOLUME_CORRUPTED  The file system structures are
> corrupted.
> > +  @retval EFI_ACCESS_DENIED     The service denied access to the file.
> > +  @retval EFI_OUT_OF_RESOURCES  The volume was not opened due to
> lack of
> > +                                resources.
> > +  @retval EFI_MEDIA_CHANGED     The device has a different medium in it
> or the
> > +                                medium is no longer supported. Any existing
> > +                                file handles for this volume are no longer
> > +                                valid. To access the files on the new medium,
> > +                                the volume must be reopened with OpenVolume().
> > +**/
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +StubFileSystemOpenVolume (
> > +  IN EFI_SIMPLE_FILE_SYSTEM_PROTOCOL  *This,
> > +  OUT EFI_FILE_PROTOCOL               **Root
> > +  )
> > +{
> > +  STUB_FILE  *StubFile;
> > +
> > +  StubFile = AllocatePool (sizeof *StubFile);
> > +  if (StubFile == NULL) {
> > +    return EFI_OUT_OF_RESOURCES;
> > +  }
> > +
> > +  StubFile->Signature = STUB_FILE_SIG;
> > +  StubFile->BlobType  = KernelBlobTypeMax;
> > +  StubFile->Position  = 0;
> > +  CopyMem (
> > +    &StubFile->File,
> > +    &mEfiFileProtocolTemplate,
> > +    sizeof mEfiFileProtocolTemplate
> > +    );
> > +  *Root = &StubFile->File;
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +STATIC CONST EFI_SIMPLE_FILE_SYSTEM_PROTOCOL  mFileSystem = {
> > +  EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_REVISION,
> > +  StubFileSystemOpenVolume
> > +};
> > +
> > +/**
> > +  Load initrd data to memory
> > +
> > +  @param[in]      This        Not used here.
> > +  @param[in]      FilePath    Contain some metadata of file device where
> holds the initrd.
> > +  @param[in]      BootPolicy  Not support here.
> > +  @param[in,out]  BufferSize  On input, contains the memory buff size.
> > +                              On output, contains the initrd blob size.
> > +  @param[out]     Buffer      A pointer point to memory which contains
> initrd data.
> > +
> > +  @retval EFI_UNSUPPORTED        Once the Boot Policy is true.
> > +  @retval EFI_INVALID_PARAMETER  Once there is no BuffSize given or no
> available file device.
> > +  @retval EFI_NOT_FOUND          File path doesn't match.
> > +  @retval EFI_BUFFER_TOO_SMALL   Buff size too small.
> > +  @retval EFI_SUCCESS            Success.
> > +**/
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +InitrdLoadFile2 (
> > +  IN      EFI_LOAD_FILE2_PROTOCOL   *This,
> > +  IN      EFI_DEVICE_PATH_PROTOCOL  *FilePath,
> > +  IN      BOOLEAN                   BootPolicy,
> > +  IN  OUT UINTN                     *BufferSize,
> > +  OUT     VOID                      *Buffer     OPTIONAL
> > +  )
> > +{
> > +  CONST KERNEL_BLOB  *InitrdBlob =
> &mKernelBlob[KernelBlobTypeInitrd];
> > +
> > +  ASSERT (InitrdBlob->Size > 0);
> > +
> > +  if (BootPolicy) {
> > +    return EFI_UNSUPPORTED;
> > +  }
> > +
> > +  if ((BufferSize == NULL) || !IsDevicePathValid (FilePath, 0)) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  if ((FilePath->Type != END_DEVICE_PATH_TYPE) ||
> > +      (FilePath->SubType != END_ENTIRE_DEVICE_PATH_SUBTYPE))
> > +  {
> > +    return EFI_NOT_FOUND;
> > +  }
> > +
> > +  if ((Buffer == NULL) || (*BufferSize < InitrdBlob->Size)) {
> > +    *BufferSize = InitrdBlob->Size;
> > +    return EFI_BUFFER_TOO_SMALL;
> > +  }
> > +
> > +  CopyMem (Buffer, InitrdBlob->Data, InitrdBlob->Size);
> > +
> > +  *BufferSize = InitrdBlob->Size;
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +STATIC CONST EFI_LOAD_FILE2_PROTOCOL  mInitrdLoadFile2 = {
> > +  InitrdLoadFile2,
> > +};
> > +
> > +/**
> > +  Fetch kernel, initrd and commandline into Blob.
> > +
> > +  @param Blob    On input, choose which blob to be fetched.
> > +                 On output, carry the full blob info.
> > +
> > +  @retval EFI_NOT_FOUND    Kernel is not fetched or no chosen node
> found in DT.
> > +  @retval EFI_SUCCESS      If there is kernel fetched or the Block->Name is
> not kernel.
> > +
> > +**/
> > +STATIC
> > +EFI_STATUS
> > +FetchBlob (
> > +  IN OUT KERNEL_BLOB  *Blob
> > +  )
> > +{
> > +  VOID          *DeviceTreeBase;
> > +  UINT64        InitrdStart, InitrdEnd;
> > +  INT32         Len;
> > +  CONST UINT64  *Prop;
> > +  INT32         ChosenNode;
> > +
> > +  DeviceTreeBase = (VOID *)(UINTN)PcdGet64
> (PcdDeviceTreeInitialBaseAddress);
> > +  ASSERT (DeviceTreeBase != NULL);
> > +
> > +  Blob->Size = 0;
> > +
> > +  //
> > +  // Make sure we have a valid device tree blob
> > +  //
> > +  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
> > +
> > +  //
> > +  // Find chosen node
> > +  //
> > +  ChosenNode = fdt_path_offset (DeviceTreeBase, "/chosen");
> > +  if (ChosenNode < 0) {
> > +    return EFI_NOT_FOUND;
> > +  }
> > +
> > +  switch (Blob->Name[0]) {
> > +    case 'k':
> > +      Prop = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,kernel-
> start", &Len);
> > +      if ((Prop == NULL) || (Len < 0)) {
> > +        return EFI_NOT_FOUND;
> > +      }
> > +
> > +      Blob->Data = (UINT8 *)fdt64_to_cpu (ReadUnaligned64 (Prop));
> > +      Prop       = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,kernel-
> size", &Len);
> > +      if ((Prop == NULL) || (Len < 0)) {
> > +        return EFI_SUCCESS;
> > +      }
> > +
> > +      Blob->Size = fdt64_to_cpu (ReadUnaligned64 (Prop));
> > +      break;
> > +    case 'i':
> > +      Prop = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,initrd-start",
> &Len);
> > +      if ((Prop == NULL) || (Len < 0)) {
> > +        return EFI_SUCCESS;
> > +      }
> > +
> > +      InitrdStart = fdt64_to_cpu (ReadUnaligned64 (Prop));
> > +      Blob->Data  = (UINT8 *)InitrdStart;
> > +      Prop        = fdt_getprop (DeviceTreeBase, ChosenNode, "linux,initrd-
> end", &Len);
> > +      if ((Prop == NULL) || (Len < 0)) {
> > +        return EFI_SUCCESS;
> > +      }
> > +
> > +      InitrdEnd  = fdt64_to_cpu (ReadUnaligned64 (Prop));
> > +      Blob->Size = InitrdEnd - InitrdStart;
> > +      break;
> > +    case 'c':
> > +      Prop = fdt_getprop (DeviceTreeBase, ChosenNode, "bootargs", &Len);
> > +      if ((Prop == NULL) || (Len < 0)) {
> > +        return EFI_SUCCESS;
> > +      }
> > +
> > +      Blob->Data = (UINT8 *)Prop;
> > +      Blob->Size = Len;
> > +      break;
> > +    default:
> > +      return EFI_SUCCESS;
> > +  }
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +//
> > +// The entry point of the feature.
> > +//
> > +
> > +/**
> > +  Download the kernel, the initial ramdisk, and the kernel command line
> from
> > +  memory or DT. Construct a minimal SimpleFileSystem that contains the
> two
> > +  image files.
> > +
> > +  @param ImageHandle  Image handle.
> > +  @param SystemTable  EFI system table.
> > +
> > +  @return                       Error codes from any of the underlying
> > +                                functions. On success, the function doesn't
> > +                                return.
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +CloudHvKernelLoaderFsDxeEntrypoint (
> > +  IN EFI_HANDLE        ImageHandle,
> > +  IN EFI_SYSTEM_TABLE  *SystemTable
> > +  )
> > +{
> > +  UINTN        BlobType;
> > +  KERNEL_BLOB  *CurrentBlob;
> > +  KERNEL_BLOB  *KernelBlob;
> > +  EFI_STATUS   Status;
> > +  EFI_HANDLE   FileSystemHandle;
> > +  EFI_HANDLE   InitrdLoadFile2Handle;
> > +
> > +  Status = gRT->GetTime (&mInitTime, NULL /* Capabilities */);
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "%a: GetTime(): %r\n", __FUNCTION__,
> Status));
> > +    return Status;
> > +  }
> > +
> > +  //
> > +  // Fetch all blobs.
> > +  //
> > +  for (BlobType = 0; BlobType < KernelBlobTypeMax; ++BlobType) {
> > +    CurrentBlob = &mKernelBlob[BlobType];
> > +    Status      = FetchBlob (CurrentBlob);
> > +    if (EFI_ERROR (Status)) {
> > +      goto FreeBlobs;
> > +    }
> > +
> > +    Status = VerifyBlob (
> > +               CurrentBlob->Name,
> > +               CurrentBlob->Data,
> > +               CurrentBlob->Size
> > +               );
> > +    if (EFI_ERROR (Status)) {
> > +      goto FreeBlobs;
> > +    }
> > +
> > +    mTotalBlobBytes += CurrentBlob->Size;
> > +  }
> > +
> > +  KernelBlob = &mKernelBlob[KernelBlobTypeKernel];
> > +
> > +  if (KernelBlob->Data == NULL) {
> > +    Status = EFI_NOT_FOUND;
> > +    goto FreeBlobs;
> > +  }
> > +
> > +  //
> > +  // Create a new handle with a single VenMedia() node device path
> protocol on
> > +  // it, plus a custom SimpleFileSystem protocol on it.
> > +  //
> > +  FileSystemHandle = NULL;
> > +  Status           = gBS->InstallMultipleProtocolInterfaces (
> > +                            &FileSystemHandle,
> > +                            &gEfiDevicePathProtocolGuid,
> > +                            &mFileSystemDevicePath,
> > +                            &gEfiSimpleFileSystemProtocolGuid,
> > +                            &mFileSystem,
> > +                            NULL
> > +                            );
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((
> > +      DEBUG_ERROR,
> > +      "%a: InstallMultipleProtocolInterfaces(): %r\n",
> > +      __FUNCTION__,
> > +      Status
> > +      ));
> > +    goto FreeBlobs;
> > +  }
> > +
> > +  if (KernelBlob[KernelBlobTypeInitrd].Size > 0) {
> > +    InitrdLoadFile2Handle = NULL;
> > +    Status                = gBS->InstallMultipleProtocolInterfaces (
> > +                                   &InitrdLoadFile2Handle,
> > +                                   &gEfiDevicePathProtocolGuid,
> > +                                   &mInitrdDevicePath,
> > +                                   &gEfiLoadFile2ProtocolGuid,
> > +                                   &mInitrdLoadFile2,
> > +                                   NULL
> > +                                   );
> > +    if (EFI_ERROR (Status)) {
> > +      DEBUG ((
> > +        DEBUG_ERROR,
> > +        "%a: InstallMultipleProtocolInterfaces(): %r\n",
> > +        __FUNCTION__,
> > +        Status
> > +        ));
> > +      goto UninstallFileSystemHandle;
> > +    }
> > +  }
> > +
> > +  return EFI_SUCCESS;
> > +
> > +UninstallFileSystemHandle:
> > +  Status = gBS->UninstallMultipleProtocolInterfaces (
> > +                  FileSystemHandle,
> > +                  &gEfiDevicePathProtocolGuid,
> > +                  &mFileSystemDevicePath,
> > +                  &gEfiSimpleFileSystemProtocolGuid,
> > +                  &mFileSystem,
> > +                  NULL
> > +                  );
> > +  ASSERT_EFI_ERROR (Status);
> > +
> > +FreeBlobs:
> > +  while (BlobType > 0) {
> > +    CurrentBlob = &mKernelBlob[--BlobType];
> > +    if (CurrentBlob->Data != NULL) {
> > +      FreePool (CurrentBlob->Data);
> > +      CurrentBlob->Size = 0;
> > +      CurrentBlob->Data = NULL;
> > +    }
> > +  }
> > +
> > +  return Status;
> > +}
> > diff --git
> a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
> b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
> > new file mode 100644
> > index 0000000000..b7aa6ebb4e
> > --- /dev/null
> > +++
> b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf
> > @@ -0,0 +1,56 @@
> > +##  @file
> > +#  DXE driver to expose the 'kernel', 'initrd' and 'cmdline' blobs
> > +#  provided by Cloud Hypervisor as files in an abstract file system
> > +#
> > +#  Copyright (C) 2022, Arm, Limited.
> > +#
> > +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +##
> > +
> > +[Defines]
> > +  INF_VERSION                    = 0x0001001B
> > +  BASE_NAME                      = CloudHvKernelLoaderFsDxe
> > +  FILE_GUID                      = 57cdf541-2a29-4ae4-97e5-76a4aa2fe090
> > +  MODULE_TYPE                    = DXE_DRIVER
> > +  VERSION_STRING                 = 1.0
> > +  ENTRY_POINT                    = CloudHvKernelLoaderFsDxeEntrypoint
> > +
> > +[Sources]
> > +  CloudHvKernelLoaderFsDxe.c
> > +
> > +[Packages]
> > +  ArmVirtPkg/ArmVirtPkg.dec
> > +  MdeModulePkg/MdeModulePkg.dec
> > +  EmbeddedPkg/EmbeddedPkg.dec
> > +  MdePkg/MdePkg.dec
> > +  OvmfPkg/OvmfPkg.dec
> > +  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
> > +
> > +[LibraryClasses]
> > +  BaseLib
> > +  BaseMemoryLib
> > +  DebugLib
> > +  DevicePathLib
> > +  FdtLib
> > +  MemoryAllocationLib
> > +  PcdLib
> > +  UefiBootServicesTableLib
> > +  UefiDriverEntryPoint
> > +  UefiRuntimeServicesTableLib
> > +
> > +[Guids]
> > +  gEfiFileInfoGuid
> > +  gEfiFileSystemInfoGuid
> > +  gEfiFileSystemVolumeLabelInfoIdGuid
> > +  gQemuKernelLoaderFsMediaGuid
> > +
> > +[Protocols]
> > +  gEfiDevicePathProtocolGuid                ## PRODUCES
> > +  gEfiLoadFile2ProtocolGuid                 ## PRODUCES
> > +  gEfiSimpleFileSystemProtocolGuid          ## PRODUCES
> > +
> > +[Depex]
> > +  gEfiRealTimeClockArchProtocolGuid
> > +
> > +[FixedPcd]
> > +  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf
       [not found]     ` <172A2070EC93BBE5.3230@groups.io>
@ 2023-01-11  8:12       ` Jianyong Wu
  2023-01-11  8:35         ` Sami Mujawar
  0 siblings, 1 reply; 19+ messages in thread
From: Jianyong Wu @ 2023-01-11  8:12 UTC (permalink / raw)
  To: devel@edk2.groups.io, Jianyong Wu, Sami Mujawar
  Cc: ardb+tianocore@kernel.org, Justin He, nd

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

Hi Sami,

Happy new year and hope you are well.

Following up last discussion. I try to add null lib instead of QemuBootOrderLib and it works. If there are no other comments, I will post the change in the next version.

Regards,
Jianyong

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jianyong Wu via groups.io
Sent: Wednesday, November 23, 2022 1:44 PM
To: Sami Mujawar <Sami.Mujawar@arm.com>; devel@edk2.groups.io
Cc: ardb+tianocore@kernel.org; Justin He <Justin.He@arm.com>; nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf

Hi Sami,

Inline reply

From: Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>
Sent: Tuesday, November 22, 2022 11:48 PM
To: Jianyong Wu <Jianyong.Wu@arm.com<mailto:Jianyong.Wu@arm.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>; Justin He <Justin.He@arm.com<mailto:Justin.He@arm.com>>; nd <nd@arm.com<mailto:nd@arm.com>>
Subject: Re: [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf


Hi Jianyong,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar
On 16/09/2022 03:46 am, Jianyong Wu wrote:

As CloudHv kernel load fs driver is implemented, add it into dsc/fdf.



Signed-off-by: Jianyong Wu <jianyong.wu@arm.com><mailto:jianyong.wu@arm.com>

---

 ArmVirtPkg/ArmVirtCloudHv.dsc                             | 8 +++++++-

 ArmVirtPkg/ArmVirtCloudHv.fdf                             | 1 +

 .../CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf | 1 -

 3 files changed, 8 insertions(+), 2 deletions(-)



diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc

index 7ca7a391d9..92ccd4ef12 100644

--- a/ArmVirtPkg/ArmVirtCloudHv.dsc

+++ b/ArmVirtPkg/ArmVirtCloudHv.dsc

@@ -37,13 +37,15 @@

   # Virtio Support

   VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf

   VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf

+  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
[SAMI] How does this work for CloudHv?

Yes, like you said below, it’s due to the change of BootManagerLib.




+  QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf



   ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf



   TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf

   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf

   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf

-  PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf

+  PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf

[SAMI] I believe this brings the dependency on QemuBootOrderLib which requires QemuFwCfgLib. Is there a way to to implement QemuBootOrderLibNull to remove the dependency on QemuFwCfgLib? The QemuBootOrderLib APIs  ConnectDevicesFromQem(), StoreQemuBootOrder(), SetBootOrderFromQemu () and GetFrontPageTimeoutFromQemu ()  could return something like EFI_UNSUPPORTED in QemuBootOrderLibNull.

Can you check, please?

Sounds a good idea, let me have a try.

[/SAMI]



   PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf

   CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf

   FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf

@@ -330,6 +332,10 @@

       NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf

       NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf

   }

+  ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf {

+    <LibraryClasses>

+      NULL|OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierLibNull.inf

+  }



   #

   # SCSI Bus and Disk Driver

diff --git a/ArmVirtPkg/ArmVirtCloudHv.fdf b/ArmVirtPkg/ArmVirtCloudHv.fdf

index 81c539590a..15b9c13c59 100644

--- a/ArmVirtPkg/ArmVirtCloudHv.fdf

+++ b/ArmVirtPkg/ArmVirtCloudHv.fdf

@@ -180,6 +180,7 @@ READ_LOCK_STATUS   = TRUE

   INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf

   INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf

   INF MdeModulePkg/Application/UiApp/UiApp.inf

+  INF ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf



   #

   # SCSI Bus and Disk Driver

diff --git a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

index b7aa6ebb4e..f7b53d0747 100644

--- a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

+++ b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

@@ -24,7 +24,6 @@

   EmbeddedPkg/EmbeddedPkg.dec

   MdePkg/MdePkg.dec

   OvmfPkg/OvmfPkg.dec

-  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
[SAMI] This should be part of patch 1/1.

Yes, I will fix it next version



Thanks

Jianyong



 [LibraryClasses]

   BaseLib


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf
  2023-01-11  8:12       ` [edk2-devel] " Jianyong Wu
@ 2023-01-11  8:35         ` Sami Mujawar
  2023-01-11  9:27           ` Jianyong Wu
       [not found]           ` <17393714CBDB554C.25137@groups.io>
  0 siblings, 2 replies; 19+ messages in thread
From: Sami Mujawar @ 2023-01-11  8:35 UTC (permalink / raw)
  To: Jianyong Wu, devel@edk2.groups.io
  Cc: ardb+tianocore@kernel.org, Justin He, nd

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

Hi Jianyong,

Thank you for your kind wishes and happy new year to you.

I think linking with a NULL lib implementation of QemuBootOrderLib would be good.

The other comment (probably for patch 2/3 in this series) was with regards to the memory reservation for the Initrd and Kernel arguments. I think these regions should be reserved (in addition to the kernel region) to avoid any accidental overwriting. I believe if you start reducing the System Memory size at some point you may find that the Initrd and Kernel arguments are overwritten. Can you check this, please?
Can you also check if there is a possibility that these regions could overlap the CPU stack, etc. and if so, how do we make sure this never happens?

Regards,

Sami Mujawar

From: Jianyong Wu <Jianyong.Wu@arm.com>
Date: Wednesday, 11 January 2023 at 08:12
To: "devel@edk2.groups.io" <devel@edk2.groups.io>, Jianyong Wu <Jianyong.Wu@arm.com>, Sami Mujawar <Sami.Mujawar@arm.com>
Cc: "ardb+tianocore@kernel.org" <ardb+tianocore@kernel.org>, Justin He <Justin.He@arm.com>, nd <nd@arm.com>
Subject: RE: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf

Hi Sami,

Happy new year and hope you are well.

Following up last discussion. I try to add null lib instead of QemuBootOrderLib and it works. If there are no other comments, I will post the change in the next version.

Regards,
Jianyong

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jianyong Wu via groups.io
Sent: Wednesday, November 23, 2022 1:44 PM
To: Sami Mujawar <Sami.Mujawar@arm.com>; devel@edk2.groups.io
Cc: ardb+tianocore@kernel.org; Justin He <Justin.He@arm.com>; nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf

Hi Sami,

Inline reply

From: Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>
Sent: Tuesday, November 22, 2022 11:48 PM
To: Jianyong Wu <Jianyong.Wu@arm.com<mailto:Jianyong.Wu@arm.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>; Justin He <Justin.He@arm.com<mailto:Justin.He@arm.com>>; nd <nd@arm.com<mailto:nd@arm.com>>
Subject: Re: [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf


Hi Jianyong,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar
On 16/09/2022 03:46 am, Jianyong Wu wrote:

As CloudHv kernel load fs driver is implemented, add it into dsc/fdf.



Signed-off-by: Jianyong Wu <jianyong.wu@arm.com><mailto:jianyong.wu@arm.com>

---

 ArmVirtPkg/ArmVirtCloudHv.dsc                             | 8 +++++++-

 ArmVirtPkg/ArmVirtCloudHv.fdf                             | 1 +

 .../CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf | 1 -

 3 files changed, 8 insertions(+), 2 deletions(-)



diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc

index 7ca7a391d9..92ccd4ef12 100644

--- a/ArmVirtPkg/ArmVirtCloudHv.dsc

+++ b/ArmVirtPkg/ArmVirtCloudHv.dsc

@@ -37,13 +37,15 @@

   # Virtio Support

   VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf

   VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf

+  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
[SAMI] How does this work for CloudHv?

Yes, like you said below, it’s due to the change of BootManagerLib.




+  QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf



   ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf



   TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf

   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf

   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf

-  PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf

+  PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf

[SAMI] I believe this brings the dependency on QemuBootOrderLib which requires QemuFwCfgLib. Is there a way to to implement QemuBootOrderLibNull to remove the dependency on QemuFwCfgLib? The QemuBootOrderLib APIs  ConnectDevicesFromQem(), StoreQemuBootOrder(), SetBootOrderFromQemu () and GetFrontPageTimeoutFromQemu ()  could return something like EFI_UNSUPPORTED in QemuBootOrderLibNull.

Can you check, please?

Sounds a good idea, let me have a try.

[/SAMI]



   PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf

   CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf

   FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf

@@ -330,6 +332,10 @@

       NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf

       NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf

   }

+  ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf {

+    <LibraryClasses>

+      NULL|OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierLibNull.inf

+  }



   #

   # SCSI Bus and Disk Driver

diff --git a/ArmVirtPkg/ArmVirtCloudHv.fdf b/ArmVirtPkg/ArmVirtCloudHv.fdf

index 81c539590a..15b9c13c59 100644

--- a/ArmVirtPkg/ArmVirtCloudHv.fdf

+++ b/ArmVirtPkg/ArmVirtCloudHv.fdf

@@ -180,6 +180,7 @@ READ_LOCK_STATUS   = TRUE

   INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf

   INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf

   INF MdeModulePkg/Application/UiApp/UiApp.inf

+  INF ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf



   #

   # SCSI Bus and Disk Driver

diff --git a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

index b7aa6ebb4e..f7b53d0747 100644

--- a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

+++ b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

@@ -24,7 +24,6 @@

   EmbeddedPkg/EmbeddedPkg.dec

   MdePkg/MdePkg.dec

   OvmfPkg/OvmfPkg.dec

-  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
[SAMI] This should be part of patch 1/1.

Yes, I will fix it next version



Thanks

Jianyong



 [LibraryClasses]

   BaseLib


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf
  2023-01-11  8:35         ` Sami Mujawar
@ 2023-01-11  9:27           ` Jianyong Wu
       [not found]           ` <17393714CBDB554C.25137@groups.io>
  1 sibling, 0 replies; 19+ messages in thread
From: Jianyong Wu @ 2023-01-11  9:27 UTC (permalink / raw)
  To: Sami Mujawar, devel@edk2.groups.io
  Cc: ardb+tianocore@kernel.org, Justin He, nd

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

Hi Sami,

Thanks for quick response.

The other comment (probably for patch 2/3 in this series) was with regards to the memory reservation for the Initrd and Kernel arguments. I think these regions should be reserved (in addition to the kernel region) to avoid any accidental overwriting. I believe if you start reducing the System Memory size at some point you may find that the Initrd and Kernel arguments are overwritten. Can you check this, please?

For the current implementation, initrd has been considered and placed on the top of memory in Cloud Hypervisor. And for kernel image, in the current design, is placed under initrd image. As to kernel parameters, it is just a string in “chosen node” and will not occupy memory. So, I think, in general, it won’t happen of accidental overwriting. But I need to set to memory region for initrd image to read-only like kernel does to avoid the memory being allocated.

Can you also check if there is a possibility that these regions could overlap the CPU stack, etc. and if so, how do we make sure this never happens?

Generally, kernel stack locates at the lowest part of memory. I think it’s safe to place Kernel and Initrd image on the top of memory, unless the memory size is too small.

WDYT?

Thanks
Jianyong

From: Sami Mujawar <Sami.Mujawar@arm.com>
Sent: Wednesday, January 11, 2023 4:35 PM
To: Jianyong Wu <Jianyong.Wu@arm.com>; devel@edk2.groups.io
Cc: ardb+tianocore@kernel.org; Justin He <Justin.He@arm.com>; nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf

Hi Jianyong,

Thank you for your kind wishes and happy new year to you.

I think linking with a NULL lib implementation of QemuBootOrderLib would be good.

The other comment (probably for patch 2/3 in this series) was with regards to the memory reservation for the Initrd and Kernel arguments. I think these regions should be reserved (in addition to the kernel region) to avoid any accidental overwriting. I believe if you start reducing the System Memory size at some point you may find that the Initrd and Kernel arguments are overwritten. Can you check this, please?
Can you also check if there is a possibility that these regions could overlap the CPU stack, etc. and if so, how do we make sure this never happens?

Regards,

Sami Mujawar

From: Jianyong Wu <Jianyong.Wu@arm.com<mailto:Jianyong.Wu@arm.com>>
Date: Wednesday, 11 January 2023 at 08:12
To: "devel@edk2.groups.io<mailto:devel@edk2.groups.io>" <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>, Jianyong Wu <Jianyong.Wu@arm.com<mailto:Jianyong.Wu@arm.com>>, Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>
Cc: "ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>" <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>, Justin He <Justin.He@arm.com<mailto:Justin.He@arm.com>>, nd <nd@arm.com<mailto:nd@arm.com>>
Subject: RE: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf

Hi Sami,

Happy new year and hope you are well.

Following up last discussion. I try to add null lib instead of QemuBootOrderLib and it works. If there are no other comments, I will post the change in the next version.

Regards,
Jianyong

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Jianyong Wu via groups.io
Sent: Wednesday, November 23, 2022 1:44 PM
To: Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>; Justin He <Justin.He@arm.com<mailto:Justin.He@arm.com>>; nd <nd@arm.com<mailto:nd@arm.com>>
Subject: Re: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf

Hi Sami,

Inline reply

From: Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>
Sent: Tuesday, November 22, 2022 11:48 PM
To: Jianyong Wu <Jianyong.Wu@arm.com<mailto:Jianyong.Wu@arm.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>; Justin He <Justin.He@arm.com<mailto:Justin.He@arm.com>>; nd <nd@arm.com<mailto:nd@arm.com>>
Subject: Re: [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf


Hi Jianyong,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar
On 16/09/2022 03:46 am, Jianyong Wu wrote:

As CloudHv kernel load fs driver is implemented, add it into dsc/fdf.



Signed-off-by: Jianyong Wu <jianyong.wu@arm.com><mailto:jianyong.wu@arm.com>

---

 ArmVirtPkg/ArmVirtCloudHv.dsc                             | 8 +++++++-

 ArmVirtPkg/ArmVirtCloudHv.fdf                             | 1 +

 .../CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf | 1 -

 3 files changed, 8 insertions(+), 2 deletions(-)



diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc

index 7ca7a391d9..92ccd4ef12 100644

--- a/ArmVirtPkg/ArmVirtCloudHv.dsc

+++ b/ArmVirtPkg/ArmVirtCloudHv.dsc

@@ -37,13 +37,15 @@

   # Virtio Support

   VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf

   VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf

+  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
[SAMI] How does this work for CloudHv?

Yes, like you said below, it’s due to the change of BootManagerLib.




+  QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf



   ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf



   TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf

   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf

   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf

-  PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf

+  PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf

[SAMI] I believe this brings the dependency on QemuBootOrderLib which requires QemuFwCfgLib. Is there a way to to implement QemuBootOrderLibNull to remove the dependency on QemuFwCfgLib? The QemuBootOrderLib APIs  ConnectDevicesFromQem(), StoreQemuBootOrder(), SetBootOrderFromQemu () and GetFrontPageTimeoutFromQemu ()  could return something like EFI_UNSUPPORTED in QemuBootOrderLibNull.

Can you check, please?

Sounds a good idea, let me have a try.

[/SAMI]



   PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf

   CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf

   FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf

@@ -330,6 +332,10 @@

       NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf

       NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf

   }

+  ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf {

+    <LibraryClasses>

+      NULL|OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierLibNull.inf

+  }



   #

   # SCSI Bus and Disk Driver

diff --git a/ArmVirtPkg/ArmVirtCloudHv.fdf b/ArmVirtPkg/ArmVirtCloudHv.fdf

index 81c539590a..15b9c13c59 100644

--- a/ArmVirtPkg/ArmVirtCloudHv.fdf

+++ b/ArmVirtPkg/ArmVirtCloudHv.fdf

@@ -180,6 +180,7 @@ READ_LOCK_STATUS   = TRUE

   INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf

   INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf

   INF MdeModulePkg/Application/UiApp/UiApp.inf

+  INF ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf



   #

   # SCSI Bus and Disk Driver

diff --git a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

index b7aa6ebb4e..f7b53d0747 100644

--- a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

+++ b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

@@ -24,7 +24,6 @@

   EmbeddedPkg/EmbeddedPkg.dec

   MdePkg/MdePkg.dec

   OvmfPkg/OvmfPkg.dec

-  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
[SAMI] This should be part of patch 1/1.

Yes, I will fix it next version



Thanks

Jianyong



 [LibraryClasses]

   BaseLib


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf
       [not found]           ` <17393714CBDB554C.25137@groups.io>
@ 2023-04-24  2:36             ` Jianyong Wu
  2023-05-23  6:48               ` Jianyong Wu
  0 siblings, 1 reply; 19+ messages in thread
From: Jianyong Wu @ 2023-04-24  2:36 UTC (permalink / raw)
  To: devel@edk2.groups.io, Jianyong Wu, Sami Mujawar
  Cc: ardb+tianocore@kernel.org, Justin He, nd

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

Hi Sami,

Following the last discussion, I check the Cpu stack in edk2 and draw a draft for the memory layout of Cpu stack, Initrd and Kernel image: see it below:

|-------------------------| <-- top of memory   -------|
|                         |                            |
|primary cpu stack (16KB) |                            \
|                         |                            / reserve 2MB is enough
|-------------------------|                            |
|  secondary cpus stack   |                            |
|  (4KB * (cputcount-1))  |                            |
|-------------------------| <-- top of cpu stack ------|
|                         |
|     initrd image        |
|        (64MB)           |
|                         |
|-------------------------|
|                         |
|     kernel image        |
|        (64MB)           |
|                         |
|-------------------------|
|                         |
|                         |
|                         |
|  normal memory region   |
|                         |
|                         |
|                         |
|-------------------------| <-- memory base address
That’s say, the top 2MB of memory region reserved for Cpu stack. It’s enough as it can accommodate for more than 500 Cpus. There are two 64MB regions reserved below Cpu stack, one for Initrd and another for Kernel image.
We can guarantee there is no confliction between each other in cloud hypervisor and double check it in edk2 by checking if there is 2MB region reserved for Cpu stack.

WDYT?

Thanks
Jianyong
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jianyong Wu via groups.io
Sent: 2023年1月11日 17:28
To: Sami Mujawar <Sami.Mujawar@arm.com>; devel@edk2.groups.io
Cc: ardb+tianocore@kernel.org; Justin He <Justin.He@arm.com>; nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf

Hi Sami,

Thanks for quick response.

The other comment (probably for patch 2/3 in this series) was with regards to the memory reservation for the Initrd and Kernel arguments. I think these regions should be reserved (in addition to the kernel region) to avoid any accidental overwriting. I believe if you start reducing the System Memory size at some point you may find that the Initrd and Kernel arguments are overwritten. Can you check this, please?

For the current implementation, initrd has been considered and placed on the top of memory in Cloud Hypervisor. And for kernel image, in the current design, is placed under initrd image. As to kernel parameters, it is just a string in “chosen node” and will not occupy memory. So, I think, in general, it won’t happen of accidental overwriting. But I need to set to memory region for initrd image to read-only like kernel does to avoid the memory being allocated.

Can you also check if there is a possibility that these regions could overlap the CPU stack, etc. and if so, how do we make sure this never happens?

Generally, kernel stack locates at the lowest part of memory. I think it’s safe to place Kernel and Initrd image on the top of memory, unless the memory size is too small.

WDYT?

Thanks
Jianyong

From: Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>
Sent: Wednesday, January 11, 2023 4:35 PM
To: Jianyong Wu <Jianyong.Wu@arm.com<mailto:Jianyong.Wu@arm.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>; Justin He <Justin.He@arm.com<mailto:Justin.He@arm.com>>; nd <nd@arm.com<mailto:nd@arm.com>>
Subject: Re: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf

Hi Jianyong,

Thank you for your kind wishes and happy new year to you.

I think linking with a NULL lib implementation of QemuBootOrderLib would be good.

The other comment (probably for patch 2/3 in this series) was with regards to the memory reservation for the Initrd and Kernel arguments. I think these regions should be reserved (in addition to the kernel region) to avoid any accidental overwriting. I believe if you start reducing the System Memory size at some point you may find that the Initrd and Kernel arguments are overwritten. Can you check this, please?
Can you also check if there is a possibility that these regions could overlap the CPU stack, etc. and if so, how do we make sure this never happens?

Regards,

Sami Mujawar

From: Jianyong Wu <Jianyong.Wu@arm.com<mailto:Jianyong.Wu@arm.com>>
Date: Wednesday, 11 January 2023 at 08:12
To: "devel@edk2.groups.io<mailto:devel@edk2.groups.io>" <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>, Jianyong Wu <Jianyong.Wu@arm.com<mailto:Jianyong.Wu@arm.com>>, Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>
Cc: "ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>" <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>, Justin He <Justin.He@arm.com<mailto:Justin.He@arm.com>>, nd <nd@arm.com<mailto:nd@arm.com>>
Subject: RE: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf

Hi Sami,

Happy new year and hope you are well.

Following up last discussion. I try to add null lib instead of QemuBootOrderLib and it works. If there are no other comments, I will post the change in the next version.

Regards,
Jianyong

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Jianyong Wu via groups.io
Sent: Wednesday, November 23, 2022 1:44 PM
To: Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>; Justin He <Justin.He@arm.com<mailto:Justin.He@arm.com>>; nd <nd@arm.com<mailto:nd@arm.com>>
Subject: Re: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf

Hi Sami,

Inline reply

From: Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>
Sent: Tuesday, November 22, 2022 11:48 PM
To: Jianyong Wu <Jianyong.Wu@arm.com<mailto:Jianyong.Wu@arm.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>; Justin He <Justin.He@arm.com<mailto:Justin.He@arm.com>>; nd <nd@arm.com<mailto:nd@arm.com>>
Subject: Re: [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf


Hi Jianyong,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar
On 16/09/2022 03:46 am, Jianyong Wu wrote:

As CloudHv kernel load fs driver is implemented, add it into dsc/fdf.



Signed-off-by: Jianyong Wu <jianyong.wu@arm.com><mailto:jianyong.wu@arm.com>

---

 ArmVirtPkg/ArmVirtCloudHv.dsc                             | 8 +++++++-

 ArmVirtPkg/ArmVirtCloudHv.fdf                             | 1 +

 .../CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf | 1 -

 3 files changed, 8 insertions(+), 2 deletions(-)



diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc

index 7ca7a391d9..92ccd4ef12 100644

--- a/ArmVirtPkg/ArmVirtCloudHv.dsc

+++ b/ArmVirtPkg/ArmVirtCloudHv.dsc

@@ -37,13 +37,15 @@

   # Virtio Support

   VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf

   VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf

+  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
[SAMI] How does this work for CloudHv?

Yes, like you said below, it’s due to the change of BootManagerLib.




+  QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf



   ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf



   TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf

   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf

   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf

-  PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf

+  PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf

[SAMI] I believe this brings the dependency on QemuBootOrderLib which requires QemuFwCfgLib. Is there a way to to implement QemuBootOrderLibNull to remove the dependency on QemuFwCfgLib? The QemuBootOrderLib APIs  ConnectDevicesFromQem(), StoreQemuBootOrder(), SetBootOrderFromQemu () and GetFrontPageTimeoutFromQemu ()  could return something like EFI_UNSUPPORTED in QemuBootOrderLibNull.

Can you check, please?

Sounds a good idea, let me have a try.

[/SAMI]



   PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf

   CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf

   FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf

@@ -330,6 +332,10 @@

       NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf

       NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf

   }

+  ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf {

+    <LibraryClasses>

+      NULL|OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierLibNull.inf

+  }



   #

   # SCSI Bus and Disk Driver

diff --git a/ArmVirtPkg/ArmVirtCloudHv.fdf b/ArmVirtPkg/ArmVirtCloudHv.fdf

index 81c539590a..15b9c13c59 100644

--- a/ArmVirtPkg/ArmVirtCloudHv.fdf

+++ b/ArmVirtPkg/ArmVirtCloudHv.fdf

@@ -180,6 +180,7 @@ READ_LOCK_STATUS   = TRUE

   INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf

   INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf

   INF MdeModulePkg/Application/UiApp/UiApp.inf

+  INF ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf



   #

   # SCSI Bus and Disk Driver

diff --git a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

index b7aa6ebb4e..f7b53d0747 100644

--- a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

+++ b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

@@ -24,7 +24,6 @@

   EmbeddedPkg/EmbeddedPkg.dec

   MdePkg/MdePkg.dec

   OvmfPkg/OvmfPkg.dec

-  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
[SAMI] This should be part of patch 1/1.

Yes, I will fix it next version



Thanks

Jianyong



 [LibraryClasses]

   BaseLib


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf
  2023-04-24  2:36             ` Jianyong Wu
@ 2023-05-23  6:48               ` Jianyong Wu
  0 siblings, 0 replies; 19+ messages in thread
From: Jianyong Wu @ 2023-05-23  6:48 UTC (permalink / raw)
  To: devel@edk2.groups.io, Sami Mujawar
  Cc: ardb+tianocore@kernel.org, Justin He, nd

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

Hello Sami,

Any comments about the memory layout below?

Thanks
Jianyong

From: Jianyong Wu
Sent: 2023年4月24日 10:37
To: devel@edk2.groups.io; Jianyong Wu <Jianyong.Wu@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com>
Cc: ardb+tianocore@kernel.org; Justin He <Justin.He@arm.com>; nd <nd@arm.com>
Subject: RE: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf

Hi Sami,

Following the last discussion, I check the Cpu stack in edk2 and draw a draft for the memory layout of Cpu stack, Initrd and Kernel image: see it below:

|-------------------------| <-- top of memory   -------|
|                         |                            |
|primary cpu stack (16KB) |                            \
|                         |                            / reserve 2MB is enough
|-------------------------|                            |
|  secondary cpus stack   |                            |
|  (4KB * (cputcount-1))  |                            |
|-------------------------| <-- top of cpu stack ------|
|                         |
|     initrd image        |
|        (64MB)           |
|                         |
|-------------------------|
|                         |
|     kernel image        |
|        (64MB)           |
|                         |
|-------------------------|
|                         |
|                         |
|                         |
|  normal memory region   |
|                         |
|                         |
|                         |
|-------------------------| <-- memory base address

That’s say, the top 2MB of memory region reserved for Cpu stack. It’s enough as it can accommodate for more than 500 Cpus. There are two 64MB regions reserved below Cpu stack, one for Initrd and another for Kernel image.
We can guarantee there is no confliction between each other in cloud hypervisor and double check it in edk2 by checking if there is 2MB region reserved for Cpu stack.

WDYT?

Thanks
Jianyong
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Jianyong Wu via groups.io
Sent: 2023年1月11日 17:28
To: Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>; Justin He <Justin.He@arm.com<mailto:Justin.He@arm.com>>; nd <nd@arm.com<mailto:nd@arm.com>>
Subject: Re: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf

Hi Sami,

Thanks for quick response.

The other comment (probably for patch 2/3 in this series) was with regards to the memory reservation for the Initrd and Kernel arguments. I think these regions should be reserved (in addition to the kernel region) to avoid any accidental overwriting. I believe if you start reducing the System Memory size at some point you may find that the Initrd and Kernel arguments are overwritten. Can you check this, please?

For the current implementation, initrd has been considered and placed on the top of memory in Cloud Hypervisor. And for kernel image, in the current design, is placed under initrd image. As to kernel parameters, it is just a string in “chosen node” and will not occupy memory. So, I think, in general, it won’t happen of accidental overwriting. But I need to set to memory region for initrd image to read-only like kernel does to avoid the memory being allocated.

Can you also check if there is a possibility that these regions could overlap the CPU stack, etc. and if so, how do we make sure this never happens?

Generally, kernel stack locates at the lowest part of memory. I think it’s safe to place Kernel and Initrd image on the top of memory, unless the memory size is too small.

WDYT?

Thanks
Jianyong

From: Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>
Sent: Wednesday, January 11, 2023 4:35 PM
To: Jianyong Wu <Jianyong.Wu@arm.com<mailto:Jianyong.Wu@arm.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>; Justin He <Justin.He@arm.com<mailto:Justin.He@arm.com>>; nd <nd@arm.com<mailto:nd@arm.com>>
Subject: Re: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf

Hi Jianyong,

Thank you for your kind wishes and happy new year to you.

I think linking with a NULL lib implementation of QemuBootOrderLib would be good.

The other comment (probably for patch 2/3 in this series) was with regards to the memory reservation for the Initrd and Kernel arguments. I think these regions should be reserved (in addition to the kernel region) to avoid any accidental overwriting. I believe if you start reducing the System Memory size at some point you may find that the Initrd and Kernel arguments are overwritten. Can you check this, please?
Can you also check if there is a possibility that these regions could overlap the CPU stack, etc. and if so, how do we make sure this never happens?

Regards,

Sami Mujawar

From: Jianyong Wu <Jianyong.Wu@arm.com<mailto:Jianyong.Wu@arm.com>>
Date: Wednesday, 11 January 2023 at 08:12
To: "devel@edk2.groups.io<mailto:devel@edk2.groups.io>" <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>, Jianyong Wu <Jianyong.Wu@arm.com<mailto:Jianyong.Wu@arm.com>>, Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>
Cc: "ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>" <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>, Justin He <Justin.He@arm.com<mailto:Justin.He@arm.com>>, nd <nd@arm.com<mailto:nd@arm.com>>
Subject: RE: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf

Hi Sami,

Happy new year and hope you are well.

Following up last discussion. I try to add null lib instead of QemuBootOrderLib and it works. If there are no other comments, I will post the change in the next version.

Regards,
Jianyong

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Jianyong Wu via groups.io
Sent: Wednesday, November 23, 2022 1:44 PM
To: Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>; Justin He <Justin.He@arm.com<mailto:Justin.He@arm.com>>; nd <nd@arm.com<mailto:nd@arm.com>>
Subject: Re: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf

Hi Sami,

Inline reply

From: Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>
Sent: Tuesday, November 22, 2022 11:48 PM
To: Jianyong Wu <Jianyong.Wu@arm.com<mailto:Jianyong.Wu@arm.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>; Justin He <Justin.He@arm.com<mailto:Justin.He@arm.com>>; nd <nd@arm.com<mailto:nd@arm.com>>
Subject: Re: [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf


Hi Jianyong,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar
On 16/09/2022 03:46 am, Jianyong Wu wrote:

As CloudHv kernel load fs driver is implemented, add it into dsc/fdf.



Signed-off-by: Jianyong Wu <jianyong.wu@arm.com><mailto:jianyong.wu@arm.com>

---

 ArmVirtPkg/ArmVirtCloudHv.dsc                             | 8 +++++++-

 ArmVirtPkg/ArmVirtCloudHv.fdf                             | 1 +

 .../CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf | 1 -

 3 files changed, 8 insertions(+), 2 deletions(-)



diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc

index 7ca7a391d9..92ccd4ef12 100644

--- a/ArmVirtPkg/ArmVirtCloudHv.dsc

+++ b/ArmVirtPkg/ArmVirtCloudHv.dsc

@@ -37,13 +37,15 @@

   # Virtio Support

   VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf

   VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf

+  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
[SAMI] How does this work for CloudHv?

Yes, like you said below, it’s due to the change of BootManagerLib.




+  QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf



   ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf



   TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf

   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf

   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf

-  PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf

+  PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf

[SAMI] I believe this brings the dependency on QemuBootOrderLib which requires QemuFwCfgLib. Is there a way to to implement QemuBootOrderLibNull to remove the dependency on QemuFwCfgLib? The QemuBootOrderLib APIs  ConnectDevicesFromQem(), StoreQemuBootOrder(), SetBootOrderFromQemu () and GetFrontPageTimeoutFromQemu ()  could return something like EFI_UNSUPPORTED in QemuBootOrderLibNull.

Can you check, please?

Sounds a good idea, let me have a try.

[/SAMI]



   PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf

   CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf

   FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf

@@ -330,6 +332,10 @@

       NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf

       NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf

   }

+  ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf {

+    <LibraryClasses>

+      NULL|OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierLibNull.inf

+  }



   #

   # SCSI Bus and Disk Driver

diff --git a/ArmVirtPkg/ArmVirtCloudHv.fdf b/ArmVirtPkg/ArmVirtCloudHv.fdf

index 81c539590a..15b9c13c59 100644

--- a/ArmVirtPkg/ArmVirtCloudHv.fdf

+++ b/ArmVirtPkg/ArmVirtCloudHv.fdf

@@ -180,6 +180,7 @@ READ_LOCK_STATUS   = TRUE

   INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf

   INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf

   INF MdeModulePkg/Application/UiApp/UiApp.inf

+  INF ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf



   #

   # SCSI Bus and Disk Driver

diff --git a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

index b7aa6ebb4e..f7b53d0747 100644

--- a/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

+++ b/ArmVirtPkg/CloudHvKernelLoaderFsDxe/CloudHvKernelLoaderFsDxe.inf

@@ -24,7 +24,6 @@

   EmbeddedPkg/EmbeddedPkg.dec

   MdePkg/MdePkg.dec

   OvmfPkg/OvmfPkg.dec

-  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
[SAMI] This should be part of patch 1/1.

Yes, I will fix it next version



Thanks

Jianyong



 [LibraryClasses]

   BaseLib


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2023-05-23  6:49 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-16  2:46 [PATCH 0/3] CloudHv:arm: Enable direct kernel boot Jianyong Wu
2022-09-16  2:46 ` [PATCH 1/3] CloudHv:arm: add kernel load fs driver Jianyong Wu
2022-11-22 15:44   ` Sami Mujawar
2022-11-22 15:46   ` Sami Mujawar
2022-11-23  6:26     ` Jianyong Wu
2022-09-16  2:46 ` [PATCH 2/3] CloudHv:arm: build hob for kernel image memory as read-only Jianyong Wu
2022-11-22 15:47   ` Sami Mujawar
2022-11-23  2:56     ` Jianyong Wu
2022-09-16  2:46 ` [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf Jianyong Wu
2022-11-22 15:48   ` Sami Mujawar
2022-11-23  5:44     ` Jianyong Wu
     [not found]     ` <172A2070EC93BBE5.3230@groups.io>
2023-01-11  8:12       ` [edk2-devel] " Jianyong Wu
2023-01-11  8:35         ` Sami Mujawar
2023-01-11  9:27           ` Jianyong Wu
     [not found]           ` <17393714CBDB554C.25137@groups.io>
2023-04-24  2:36             ` Jianyong Wu
2023-05-23  6:48               ` Jianyong Wu
2022-11-15  3:00 ` [PATCH 0/3] CloudHv:arm: Enable direct kernel boot Jianyong Wu
2022-11-15  8:51   ` Sami Mujawar
2022-11-15  9:01     ` Jianyong Wu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox