Re: [edk2-platforms][PATCH V1 3/5] Platform/ARM/Morello: Add PlatformDxe driver for Morello

["Sami Mujawar" <sami.mujawar@arm.com>]

Hi Chandni,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar

-----Original Message-----
From: Chandni Cherukuri <chandni.cherukuri@arm.com> 
Sent: 24 February 2021 01:12 PM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Leif Lindholm <leif@nuviainc.com>; Sami Mujawar <Sami.Mujawar@arm.com>
Subject: [edk2-platforms][PATCH V1 3/5] Platform/ARM/Morello: Add PlatformDxe driver for Morello

From: Anurag Koul <anurag.koul@arm.com>

This patch adds the PlatformDxe Driver for Morello
platform. It includes the registration of the ramdisk
device.

Co-authored-by: Chandni Cherukuri <chandni.cherukuri@arm.com>
Signed-off-by: Chandni Cherukuri <chandni.cherukuri@arm.com>
---
 Platform/ARM/Morello/Drivers/PlatformDxe/PlatformDxeFvp.inf     | 43 +++++++++
 Platform/ARM/Morello/Include/Guid/MorelloVirtioDevicesFormSet.h | 14 +++
 Platform/ARM/Morello/Drivers/PlatformDxe/PlatformDxeFvp.c       | 58 +++++++++++++
 Platform/ARM/Morello/Drivers/PlatformDxe/VirtioDevices.c        | 91 ++++++++++++++++++++
 4 files changed, 206 insertions(+)

diff --git a/Platform/ARM/Morello/Drivers/PlatformDxe/PlatformDxeFvp.inf b/Platform/ARM/Morello/Drivers/PlatformDxe/PlatformDxeFvp.inf
new file mode 100644
index 000000000000..69df9019cde3
--- /dev/null
+++ b/Platform/ARM/Morello/Drivers/PlatformDxe/PlatformDxeFvp.inf
@@ -0,0 +1,43 @@
+## @file
+#
+#  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001A
+  BASE_NAME                      = PlatformDxe
+  FILE_GUID                      = 11FC8B5A-377D-47A8-AEE9-0093D3D3407F
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = ArmMorelloEntryPoint
+
+[Sources.common]
+  PlatformDxeFvp.c
+  VirtioDevices.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+  Platform/ARM/Morello/MorelloPlatform.dec
+
+[LibraryClasses]
+  UefiDriverEntryPoint
+  VirtioMmioDeviceLib
+
+[Protocols]
+  gEfiRamDiskProtocolGuid
+
+[FeaturePcd]
+  gArmMorelloTokenSpaceGuid.PcdRamDiskSupported
+  gArmMorelloTokenSpaceGuid.PcdVirtioBlkSupported
+
+[FixedPcd]
+  gArmMorelloTokenSpaceGuid.PcdRamDiskBase
+  gArmMorelloTokenSpaceGuid.PcdRamDiskSize
+  gArmMorelloTokenSpaceGuid.PcdVirtioBlkBaseAddress
+
+[Depex]
+  gEfiRamDiskProtocolGuid
diff --git a/Platform/ARM/Morello/Include/Guid/MorelloVirtioDevicesFormSet.h b/Platform/ARM/Morello/Include/Guid/MorelloVirtioDevicesFormSet.h
new file mode 100644
index 000000000000..27ae61bb1cf5
--- /dev/null
+++ b/Platform/ARM/Morello/Include/Guid/MorelloVirtioDevicesFormSet.h
@@ -0,0 +1,14 @@
+/** @file
+
+  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef  MORELLO_VIRTIO_DEVICES_FORMSET_H__
+#define  MORELLO_VIRTIO_DEVICES_FORMSET_H__
+
+#define MORELLO_VIRTIO_BLOCK_GUID  \
+  { 0x2B6E62D0, 0x9346, 0x4E1A, { 0xAA, 0x1E, 0xCB, 0x01, 0xC3, 0x23, 0x4A, 0x00 } }
+
+#endif
diff --git a/Platform/ARM/Morello/Drivers/PlatformDxe/PlatformDxeFvp.c b/Platform/ARM/Morello/Drivers/PlatformDxe/PlatformDxeFvp.c
new file mode 100644
index 000000000000..ff99c16b2c9a
--- /dev/null
+++ b/Platform/ARM/Morello/Drivers/PlatformDxe/PlatformDxeFvp.c
@@ -0,0 +1,58 @@
+/** @file
+
+  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Library/DebugLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/RamDisk.h>
+
+VOID
+InitVirtioDevices (
+  VOID
+  );
+
+EFI_STATUS
+EFIAPI
+ArmMorelloEntryPoint (
+  IN EFI_HANDLE         ImageHandle,
+  IN EFI_SYSTEM_TABLE   *SystemTable
+  )
+{
+  EFI_STATUS                 Status;
+  EFI_RAM_DISK_PROTOCOL      *RamDisk;
+  EFI_DEVICE_PATH_PROTOCOL   *DevicePath;
+
+  Status = EFI_SUCCESS;
+
+  InitVirtioDevices ();
+
+  if (FeaturePcdGet (PcdRamDiskSupported)) {
+    Status = gBS->LocateProtocol (
+                    &gEfiRamDiskProtocolGuid,
+                    NULL,
+                    (VOID**)&RamDisk
+                    );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "Couldn't find the RAM Disk protocol %r\n",
+        __FUNCTION__, Status));
+      return Status;
+    }
+
+    Status = RamDisk->Register (
+                        (UINTN)PcdGet32 (PcdRamDiskBase),
+                        (UINTN)PcdGet32 (PcdRamDiskSize),
+                        &gEfiVirtualCdGuid,
+                        NULL,
+                        &DevicePath
+                        );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "%a: Failed to register RAM Disk - %r\n",
+        __FUNCTION__, Status));
+    }
+  }
+
+  return Status;
+}
diff --git a/Platform/ARM/Morello/Drivers/PlatformDxe/VirtioDevices.c b/Platform/ARM/Morello/Drivers/PlatformDxe/VirtioDevices.c
new file mode 100644
index 000000000000..a83a4e71a47e
--- /dev/null
+++ b/Platform/ARM/Morello/Drivers/PlatformDxe/VirtioDevices.c
@@ -0,0 +1,91 @@
+/** @file
+
+  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Guid/MorelloVirtioDevicesFormSet.h>
+#include <Library/VirtioMmioDeviceLib.h>
+#include <Library/DebugLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+
+#pragma pack (1)
+typedef struct {
+  VENDOR_DEVICE_PATH                  VendorDevicePath;
+  EFI_DEVICE_PATH_PROTOCOL            End;
+} VIRTIO_DEVICE_PATH;
+#pragma pack ()
+
+STATIC VIRTIO_DEVICE_PATH mVirtioBlockDevicePath =
+{
+  {
+    {
+      HARDWARE_DEVICE_PATH,
+      HW_VENDOR_DP,
+      {
+        (UINT8)(sizeof (VENDOR_DEVICE_PATH)),
+        (UINT8)((sizeof (VENDOR_DEVICE_PATH)) >> 8)
+      }
+    },
+    MORELLO_VIRTIO_BLOCK_GUID,
+  },
+  {
+    END_DEVICE_PATH_TYPE,
+    END_ENTIRE_DEVICE_PATH_SUBTYPE,
+    {
+      sizeof (EFI_DEVICE_PATH_PROTOCOL),
+      0
+    }
+  }
+};
+
+//
+//  Initialize platform Virtio devices.
+//
+//  @return None.
+//
+VOID
+InitVirtioDevices (
+  VOID
+  )
[SAMI] Use doxygen style function documentation headers.
[/SAMI]
+{
+  EFI_STATUS Status;
+  STATIC EFI_HANDLE mVirtIoBlkController = NULL;
[SAMI] Why is this variable static?
[/SAMI]
+
+  Status = EFI_SUCCESS;
[SAMI] Is this needed? Should this function return the Status code?
[/SAMI]
+
+  // Install protocol interface for storage device
+  if (FeaturePcdGet (PcdVirtioBlkSupported)) {
+
+    Status = gBS->InstallProtocolInterface (
+                    &mVirtIoBlkController,
+                    &gEfiDevicePathProtocolGuid,
+                    EFI_NATIVE_INTERFACE,
+                    &mVirtioBlockDevicePath
+                    );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "%a: Failed to install EFI_DEVICE_PATH protocol "
+        "for Virtio Block device (Status = %r)\n",
+        __FUNCTION__, Status));
+    } else {
+      // Declare the Virtio BlockIo device
+      Status = VirtioMmioInstallDevice (
+                 FixedPcdGet32 (PcdVirtioBlkBaseAddress),
+                 mVirtIoBlkController
+                 );
+      if (EFI_ERROR (Status)) {
+        DEBUG ((DEBUG_ERROR, "%a: Unable to find Virtio Block MMIO device "
+          "(Status == %r)\n", __FUNCTION__, Status));
+        gBS->UninstallProtocolInterface (
+               mVirtIoBlkController,
+               &gEfiDevicePathProtocolGuid,
+               &mVirtioBlockDevicePath
+               );
+      } else {
+        DEBUG ((DEBUG_INIT, "%a: Installed Virtio Block device\n",
+          __FUNCTION__));
+      }
+    }
+  }
+}
-- 
2.17.1