public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v4 00/15] Kvmtool guest firmware support for Arm
@ 2020-07-07 12:47 Sami Mujawar
  2020-07-07 12:47 ` [PATCH v4 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver Sami Mujawar
                   ` (15 more replies)
  0 siblings, 16 replies; 25+ messages in thread
From: Sami Mujawar @ 2020-07-07 12:47 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ard.biesheuvel, leif, philmd, ray.ni,
	michael.d.kinney, liming.gao, lersek, Alexandru.Elisei,
	Andre.Przywara, Matteo.Carlini, Laura.Moretta, nd

Kvmtool is a virtual machine manager that can be used to launch
guest partitions. Kvmtool additionally supports emulation of
hardware like the RTC, CFI etc. essentially providing an
emulated platform for a Guest OS to run.

To boot a standards-based OS one would need UEFI. In this case
it is UEFI at EL1 or guest/virtual firmware.

Kvmtool has been enhanced to enable launching of KVM guests with
UEFI support e.g. CFI emulation has been added to store UEFI
variables, etc. These changes have been merged in the kvmtool
repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/will/kvmtool.git

This patch series:
  - adds UEFI guest firmware support for the Kvmtool emulated
    Arm platform.
  - addresses the review feedback for the v3 series discussed
    on the mailing list at:
    https://edk2.groups.io/g/devel/message/61662
  - Drops patch for early serial port initialisation due to
    incompatibility with some platforms.

The changes for the v4 series can be seen at:
https://github.com/samimujawar/edk2/tree/299_kvmtool_plat_support_v4

Sami Mujawar (15):
  PcAtChipsetPkg: Add MMIO Support to RTC driver
  ArmVirtPkg: Add Kvmtool RTC Fdt Client Library
  ArmPlatformPkg: Dynamic flash variable base
  ArmVirtPkg: Add kvmtool platform driver
  ArmVirtPkg: kvmtool platform memory map
  ArmVirtPkg: Add Kvmtool NOR flash lib
  MdeModulePkg: Fix constructor invocation ordering
  ArmVirtPkg: GUID Hob for 16550 UART base address
  ArmVirtPkg: 16550 UART Platform hook library
  ArmVirtPkg: Add Kvmtool Platform Pei Lib
  ArmVirtPkg: Support for kvmtool virtual platform
  ArmVirtPkg: Package dependency for MC146818 RTC
  ArmVirtPkg: Add kvmtool to package dictionary
  .python/SpellCheck: Add 'XIPFLAGS' to "words" section
  Maintainer.txt: Add Kvmtool platform reviewer

 .pytool/Plugin/SpellCheck/cspell.base.yaml                                      |   1 +
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c                             |   4 +-
 ArmVirtPkg/ArmVirtKvmTool.dsc                                                   | 321 +++++++++++++++++++
 ArmVirtPkg/ArmVirtKvmTool.fdf                                                   | 234 ++++++++++++++
 ArmVirtPkg/ArmVirtPkg.ci.yaml                                                   |   6 +-
 ArmVirtPkg/ArmVirtPkg.dec                                                       |   2 +
 ArmVirtPkg/Include/Guid/Early16550UartBaseAddress.h                             |  22 ++
 ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c                              |  82 +++++
 ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf                            |  44 +++
 ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.c   | 138 ++++++++
 ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.inf |  36 +++
 ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.c        |  57 ++++
 ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf      |  39 +++
 ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.uni      |  14 +
 ArmVirtPkg/Library/KvmtoolPlatformPeiLib/KvmtoolPlatformPeiLib.c                |  78 +++++
 ArmVirtPkg/Library/KvmtoolPlatformPeiLib/KvmtoolPlatformPeiLib.inf              |  48 +++
 ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c              | 230 ++++++++++++++
 ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf            |  42 +++
 ArmVirtPkg/Library/KvmtoolVirtMemInfoLib/KvmtoolVirtMemInfoLib.c                |  96 ++++++
 ArmVirtPkg/Library/KvmtoolVirtMemInfoLib/KvmtoolVirtMemInfoLib.inf              |  42 +++
 ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c                         | 336 ++++++++++++++++++++
 ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf                    |  49 +++
 Maintainers.txt                                                                 |  10 +
 MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c            |  17 +
 MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf          |   3 +
 PcAtChipsetPkg/PcAtChipsetPkg.dec                                               |  16 +
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c                              | 120 ++++++-
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h                              |   2 +
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c                         |  58 +++-
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf      |   8 +
 30 files changed, 2138 insertions(+), 17 deletions(-)
 create mode 100644 ArmVirtPkg/ArmVirtKvmTool.dsc
 create mode 100644 ArmVirtPkg/ArmVirtKvmTool.fdf
 create mode 100644 ArmVirtPkg/Include/Guid/Early16550UartBaseAddress.h
 create mode 100644 ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c
 create mode 100644 ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
 create mode 100644 ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.c
 create mode 100644 ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.inf
 create mode 100644 ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.c
 create mode 100644 ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf
 create mode 100644 ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.uni
 create mode 100644 ArmVirtPkg/Library/KvmtoolPlatformPeiLib/KvmtoolPlatformPeiLib.c
 create mode 100644 ArmVirtPkg/Library/KvmtoolPlatformPeiLib/KvmtoolPlatformPeiLib.inf
 create mode 100644 ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c
 create mode 100644 ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf
 create mode 100644 ArmVirtPkg/Library/KvmtoolVirtMemInfoLib/KvmtoolVirtMemInfoLib.c
 create mode 100644 ArmVirtPkg/Library/KvmtoolVirtMemInfoLib/KvmtoolVirtMemInfoLib.inf
 create mode 100644 ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c
 create mode 100644 ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf

-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

* [PATCH v4 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver
  2020-07-07 12:47 [PATCH v4 00/15] Kvmtool guest firmware support for Arm Sami Mujawar
@ 2020-07-07 12:47 ` Sami Mujawar
  2020-07-07 13:16   ` Ard Biesheuvel
  2020-07-07 12:47 ` [PATCH v4 02/15] ArmVirtPkg: Add Kvmtool RTC Fdt Client Library Sami Mujawar
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 25+ messages in thread
From: Sami Mujawar @ 2020-07-07 12:47 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ard.biesheuvel, leif, ray.ni, Alexandru.Elisei,
	Andre.Przywara, Matteo.Carlini, Laura.Moretta, nd

Some virtual machine managers like Kvmtool emulate the MC146818
RTC controller in the MMIO space so that architectures that do
not support I/O Mapped I/O can use the RTC. This patch adds MMIO
support to the RTC controller driver.

The PCD PcdRtcUseMmio has been added to select I/O or MMIO support.
  If PcdRtcUseMmio is:
    TRUE  - Indicates the RTC port registers are in MMIO space.
    FALSE - Indicates the RTC port registers are in I/O space.
            Default is I/O space.

Additionally two new PCDs PcdRtcIndexRegister64 and
PcdRtcTargetRegister64 have been introduced to provide the base
address for the RTC registers in the MMIO space.

When MMIO support is selected (PcdRtcUseMmio == TRUE) the driver
converts the pointers to the RTC MMIO registers so that the
RTC registers are accessible post ExitBootServices.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v4:
      - Updated based on review comments.                             [Sami]
      - Use static helper functions instead of function pointers.     [Ard]
        Ref: https://edk2.groups.io/g/devel/topic/75081468
    
    v3:
      - Make PcdRtcUseMmio a feature PCD.                             [Sami]
      - Read the RTC MMIO base address from the DT.                   [Andre]
      - Introduce PCDs for RTC Index and Target register base         [Sami]
        address in the MMIO space.
      - Move RTC MMIO region mapping code to a separate platform      [Sami]
        specific library. This library also reads the base addresses
        for the RTC from DT and configures the RTC Index and Target
        register PCDs.
        Ref: https://edk2.groups.io/g/devel/topic/74200905#60307
    
    v2:
      - Code review comments incorporated.                            [Sami]
    
    v1:
      - Add support to read/write from RTC registers using            [Sami]
        MMIO access
      - Use wrapper functions for RtcRead/Write accessors             [Leif]
        Ref: https://edk2.groups.io/g/devel/topic/30915281#30695

 PcAtChipsetPkg/PcAtChipsetPkg.dec                                          |  16 +++
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c                         | 120 ++++++++++++++++++--
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h                         |   2 +
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c                    |  58 +++++++++-
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf |   8 ++
 5 files changed, 190 insertions(+), 14 deletions(-)

diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dec b/PcAtChipsetPkg/PcAtChipsetPkg.dec
index 88de5cceea593176c3a2425a5963b66b789f2b9e..ed2d95550b8d153995b30cdc290cf3bb905e211b 100644
--- a/PcAtChipsetPkg/PcAtChipsetPkg.dec
+++ b/PcAtChipsetPkg/PcAtChipsetPkg.dec
@@ -6,6 +6,7 @@
 #
 # Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
 # Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+# Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR>
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -41,6 +42,13 @@ [PcdsFeatureFlag]
   # @Prompt Configure HPET to use MSI.
   gPcAtChipsetPkgTokenSpaceGuid.PcdHpetMsiEnable|TRUE|BOOLEAN|0x00001000
 
+  ## Indicates the RTC port registers are in MMIO space, or in I/O space.
+  #  Default is I/O space.<BR><BR>
+  #   TRUE  - RTC port registers are in MMIO space.<BR>
+  #   FALSE - RTC port registers are in I/O space.<BR>
+  # @Prompt RTC port registers use MMIO.
+  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|FALSE|BOOLEAN|0x00000021
+
 [PcdsFixedAtBuild, PcdsDynamic, PcdsDynamicEx, PcdsPatchableInModule]
   ## This PCD specifies the base address of the HPET timer.
   # @Prompt HPET base address.
@@ -68,6 +76,14 @@ [PcdsFixedAtBuild, PcdsDynamic, PcdsDynamicEx, PcdsPatchableInModule]
   # @Expression 0x80000001 | gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear < gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear + 100
   gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2097|UINT16|0x0000000E
 
+  ## Specifies RTC Index Register address in MMIO space.
+  # @Prompt RTC Index Register address
+  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0|UINT64|0x00000022
+
+  ## Specifies RTC Target Register address in MMIO space.
+  # @Prompt RTC Target Register address
+  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64|0x0|UINT64|0x00000023
+
 [PcdsFixedAtBuild, PcdsPatchableInModule]
   ## Defines the ACPI register set base address.
   #  The invalid 0xFFFF is as its default value. It must be configured to the real value.
diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
index 52af17941786ef81c3911512ee64551724e67209..64f36f6fbbd1b03967bd1a1290d108d5b0f294fa 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
@@ -3,6 +3,7 @@
 
 Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
 Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR>
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -10,6 +11,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include "PcRtc.h"
 
+extern UINTN  mRtcIndexRegister;
+extern UINTN  mRtcTargetRegister;
+
 //
 // Days of month.
 //
@@ -54,38 +58,132 @@ IsWithinOneDay (
   );
 
 /**
+  Read RTC content through its registers using IO access.
+
+  @param  Address   Address offset of RTC. It is recommended to use
+                    macros such as RTC_ADDRESS_SECONDS.
+
+  @return The data of UINT8 type read from RTC.
+**/
+STATIC
+UINT8
+IoRtcRead (
+  IN  UINTN Address
+  )
+{
+  IoWrite8 (
+    PcdGet8 (PcdRtcIndexRegister),
+    (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80))
+    );
+  return IoRead8 (PcdGet8 (PcdRtcTargetRegister));
+}
+
+/**
+  Write RTC through its registers  using IO access.
+
+  @param  Address   Address offset of RTC. It is recommended to use
+                    macros such as RTC_ADDRESS_SECONDS.
+  @param  Data      The content you want to write into RTC.
+
+**/
+STATIC
+VOID
+IoRtcWrite (
+  IN  UINTN   Address,
+  IN  UINT8   Data
+  )
+{
+  IoWrite8 (
+    PcdGet8 (PcdRtcIndexRegister),
+    (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80))
+    );
+  IoWrite8 (PcdGet8 (PcdRtcTargetRegister), Data);
+}
+
+/**
+  Read RTC content through its registers using MMIO access.
+
+  @param  Address   Address offset of RTC. It is recommended to use
+                    macros such as RTC_ADDRESS_SECONDS.
+
+  @return The data of UINT8 type read from RTC.
+**/
+STATIC
+UINT8
+MmioRtcRead (
+  IN  UINTN Address
+  )
+{
+  MmioWrite8 (
+    mRtcIndexRegister,
+    (UINT8)(Address | (UINT8)(MmioRead8 (mRtcIndexRegister) & 0x80))
+    );
+  return MmioRead8 (mRtcTargetRegister);
+}
+
+/**
+  Write RTC through its registers using MMIO access.
+
+  @param  Address   Address offset of RTC. It is recommended to use
+                    macros such as RTC_ADDRESS_SECONDS.
+  @param  Data      The content you want to write into RTC.
+
+**/
+STATIC
+VOID
+MmioRtcWrite (
+  IN  UINTN   Address,
+  IN  UINT8   Data
+  )
+{
+  MmioWrite8 (
+    mRtcIndexRegister,
+    (UINT8)(Address | (UINT8)(MmioRead8 (mRtcIndexRegister) & 0x80))
+    );
+  MmioWrite8 (mRtcTargetRegister, Data);
+}
+
+/**
   Read RTC content through its registers.
 
-  @param  Address  Address offset of RTC. It is recommended to use macros such as
-                   RTC_ADDRESS_SECONDS.
+  @param  Address   Address offset of RTC. It is recommended to use
+                    macros such as RTC_ADDRESS_SECONDS.
 
   @return The data of UINT8 type read from RTC.
 **/
+STATIC
 UINT8
 RtcRead (
-  IN  UINT8 Address
+  IN  UINTN Address
   )
 {
-  IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8) (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
-  return IoRead8 (PcdGet8 (PcdRtcTargetRegister));
+  if (FeaturePcdGet (PcdRtcUseMmio)) {
+    return MmioRtcRead (Address);
+  }
+
+  return IoRtcRead (Address);
 }
 
 /**
   Write RTC through its registers.
 
-  @param  Address  Address offset of RTC. It is recommended to use macros such as
-                   RTC_ADDRESS_SECONDS.
-  @param  Data     The content you want to write into RTC.
+  @param  Address   Address offset of RTC. It is recommended to use
+                    macros such as RTC_ADDRESS_SECONDS.
+  @param  Data      The content you want to write into RTC.
 
 **/
+STATIC
 VOID
 RtcWrite (
-  IN  UINT8   Address,
+  IN  UINTN   Address,
   IN  UINT8   Data
   )
 {
-  IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8) (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
-  IoWrite8 (PcdGet8 (PcdRtcTargetRegister), Data);
+  if (FeaturePcdGet (PcdRtcUseMmio)) {
+    MmioRtcWrite (Address, Data);
+  } else {
+    IoRtcWrite (Address, Data);
+  }
 }
 
 /**
diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h
index 47293ce44c5a1f4792892892f7da40d7f0a5a001..c1cb9466b74587233de4e78c0c0d537ca6b66cf3 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h
@@ -3,6 +3,7 @@
 
 Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
 Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.<BR>
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -371,4 +372,5 @@ PcRtcAcpiTableChangeCallback (
   IN EFI_EVENT        Event,
   IN VOID             *Context
   );
+
 #endif
diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
index ccda6331373bfe4069b0a59495b5e5cc731c8fc8..9c5f1ff169c80d1a9580d90336f162968b63bfa2 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
@@ -2,16 +2,23 @@
   Provides Set/Get time operations.
 
 Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
+#include <Library/DxeServicesTableLib.h>
 #include "PcRtc.h"
 
 PC_RTC_MODULE_GLOBALS  mModuleGlobal;
 
 EFI_HANDLE             mHandle = NULL;
 
+STATIC EFI_EVENT       mVirtualAddrChangeEvent;
+
+UINTN                  mRtcIndexRegister;
+UINTN                  mRtcTargetRegister;
+
 /**
   Returns the current time and date information, and the time-keeping capabilities
   of the hardware platform.
@@ -106,6 +113,30 @@ PcRtcEfiSetWakeupTime (
 }
 
 /**
+  Fixup internal data so that EFI can be called in virtual mode.
+  Call the passed in Child Notify event and convert any pointers in
+  lib to virtual mode.
+
+  @param[in]    Event   The Event that is being processed
+  @param[in]    Context Event Context
+**/
+VOID
+EFIAPI
+LibRtcVirtualNotifyEvent (
+  IN EFI_EVENT        Event,
+  IN VOID             *Context
+  )
+{
+  // Only needed if you are going to support the OS calling RTC functions in
+  // virtual mode. You will need to call EfiConvertPointer (). To convert any
+  // stored physical addresses to virtual address. After the OS transitions to
+  // calling in virtual mode, all future runtime calls will be made in virtual
+  // mode.
+  EfiConvertPointer (0x0, (VOID**)&mRtcIndexRegister);
+  EfiConvertPointer (0x0, (VOID**)&mRtcTargetRegister);
+}
+
+/**
   The user Entry Point for PcRTC module.
 
   This is the entry point for PcRTC module. It installs the UEFI runtime service
@@ -125,12 +156,17 @@ InitializePcRtc (
   IN EFI_SYSTEM_TABLE                      *SystemTable
   )
 {
-  EFI_STATUS  Status;
-  EFI_EVENT   Event;
+  EFI_STATUS             Status;
+  EFI_EVENT              Event;
 
   EfiInitializeLock (&mModuleGlobal.RtcLock, TPL_CALLBACK);
   mModuleGlobal.CenturyRtcAddress = GetCenturyRtcAddress ();
 
+  if (FeaturePcdGet (PcdRtcUseMmio)) {
+    mRtcIndexRegister = (UINTN)PcdGet64 (PcdRtcIndexRegister64);
+    mRtcTargetRegister = (UINTN)PcdGet64 (PcdRtcTargetRegister64);
+  }
+
   Status = PcRtcInit (&mModuleGlobal);
   ASSERT_EFI_ERROR (Status);
 
@@ -165,7 +201,23 @@ InitializePcRtc (
                   NULL,
                   NULL
                   );
-  ASSERT_EFI_ERROR (Status);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  if (FeaturePcdGet (PcdRtcUseMmio)) {
+    // Register for the virtual address change event
+    Status = gBS->CreateEventEx (
+                    EVT_NOTIFY_SIGNAL,
+                    TPL_NOTIFY,
+                    LibRtcVirtualNotifyEvent,
+                    NULL,
+                    &gEfiEventVirtualAddressChangeGuid,
+                    &mVirtualAddrChangeEvent
+                    );
+    ASSERT_EFI_ERROR (Status);
+  }
 
   return Status;
 }
diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
index c73ee98105e510f9e4e23c1a6c1e5c505325d2c9..0d8eca28b65954b073a72fc4fe5ad6247320e79d 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
@@ -6,6 +6,7 @@
 #
 # Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 # Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+# Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR>
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -61,6 +62,11 @@ [Guids]
   ## SOMETIMES_CONSUMES ## SystemTable
   gEfiAcpiTableGuid
 
+  gEfiEventVirtualAddressChangeGuid
+
+[FeaturePcd]
+  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio                   ## CONSUMES
+
 [FixedPcd]
   gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterA     ## CONSUMES
   gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterB     ## CONSUMES
@@ -72,6 +78,8 @@ [Pcd]
   gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear             ## CONSUMES
   gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister             ## CONSUMES
   gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister            ## CONSUMES
+  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64           ## CONSUMES
+  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64          ## CONSUMES
 
 [Depex]
   gEfiVariableArchProtocolGuid AND gEfiVariableWriteArchProtocolGuid
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

* [PATCH v4 02/15] ArmVirtPkg: Add Kvmtool RTC Fdt Client Library
  2020-07-07 12:47 [PATCH v4 00/15] Kvmtool guest firmware support for Arm Sami Mujawar
  2020-07-07 12:47 ` [PATCH v4 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver Sami Mujawar
@ 2020-07-07 12:47 ` Sami Mujawar
  2020-07-07 13:19   ` Ard Biesheuvel
  2020-07-07 12:47 ` [PATCH v4 03/15] ArmPlatformPkg: Dynamic flash variable base Sami Mujawar
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 25+ messages in thread
From: Sami Mujawar @ 2020-07-07 12:47 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ard.biesheuvel, leif, lersek, Alexandru.Elisei,
	Andre.Przywara, Matteo.Carlini, Laura.Moretta, nd

Add library that parses the Kvmtool device tree and updates
the dynamic PCDs describing the RTC Memory map.

It also maps the MMIO region used by the RTC as runtime memory
so that the RTC registers are accessible post ExitBootServices.

Since UEFI takes ownership of the RTC hardware disable the RTC
node in the DT to prevent the OS from attaching its device
driver as well.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v4:
      - Updated based on review comments.                             [Sami]
      - Cleanup include file list, make local functions static,       [Ard]
        drop EFIAPI, use EFI_PAGE_MASK and use ReadUnaligned64.
        Ref: https://edk2.groups.io/g/devel/topic/75081472
    
    v3:
      - Introduce library to read and map the MMIO base addresses     [Sami]
        for the RTC registers from the DT and configure the Index
        and Target register PCDs.

 ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c   | 230 ++++++++++++++++++++
 ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf |  42 ++++
 2 files changed, 272 insertions(+)

diff --git a/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c
new file mode 100644
index 0000000000000000000000000000000000000000..93715958952f933007ff143a22cbe81606cc539e
--- /dev/null
+++ b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c
@@ -0,0 +1,230 @@
+/** @file
+  FDT client library for motorola,mc146818 RTC driver
+
+  Copyright (c) 2020, ARM Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DxeServicesTableLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/FdtClient.h>
+
+/** RTC Index register is at offset 0x0
+*/
+#define RTC_INDEX_REG_OFFSET    0x0ULL
+
+/** RTC Target register is at offset 0x1
+*/
+#define RTC_TARGET_REG_OFFSET   0x1ULL
+
+/** Add the RTC controller address range to the memory map.
+
+  @param [in]  ImageHandle  The handle to the image.
+  @param [in]  RtcPageBase  Base address of the RTC controller.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
+  @retval EFI_NOT_FOUND           Flash device not found.
+**/
+STATIC
+EFI_STATUS
+KvmtoolRtcMapMemory (
+  IN EFI_HANDLE               ImageHandle,
+  IN EFI_PHYSICAL_ADDRESS     RtcPageBase
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = gDS->AddMemorySpace (
+                  EfiGcdMemoryTypeMemoryMappedIo,
+                  RtcPageBase,
+                  EFI_PAGE_SIZE,
+                  EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR, "Failed to add memory space. Status = %r\n",
+      Status
+      ));
+    return Status;
+  }
+
+  Status = gDS->AllocateMemorySpace (
+                  EfiGcdAllocateAddress,
+                  EfiGcdMemoryTypeMemoryMappedIo,
+                  0,
+                  EFI_PAGE_SIZE,
+                  &RtcPageBase,
+                  ImageHandle,
+                  NULL
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "Failed to allocate memory space. Status = %r\n",
+      Status
+      ));
+    gDS->RemoveMemorySpace (
+           RtcPageBase,
+           EFI_PAGE_SIZE
+           );
+    return Status;
+  }
+
+  Status = gDS->SetMemorySpaceAttributes (
+                  RtcPageBase,
+                  EFI_PAGE_SIZE,
+                  EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "Failed to set memory attributes. Status = %r\n",
+      Status
+      ));
+    gDS->FreeMemorySpace (
+           RtcPageBase,
+           EFI_PAGE_SIZE
+           );
+    gDS->RemoveMemorySpace (
+           RtcPageBase,
+           EFI_PAGE_SIZE
+           );
+  }
+
+  return Status;
+}
+
+/** Entrypoint for KvmtoolRtcFdtClientLib.
+
+  Locate the RTC node in the DT and update the Index and
+  Target register base addresses in the respective PCDs.
+  Add the RTC memory region to the memory map.
+  Disable the RTC node as the RTC is owned by UEFI.
+
+  @param [in]  ImageHandle  The handle to the image.
+  @param [in]  SystemTable  Pointer to the System Table.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
+  @retval EFI_NOT_FOUND           Flash device not found.
+**/
+EFI_STATUS
+EFIAPI
+KvmtoolRtcFdtClientLibConstructor (
+  IN EFI_HANDLE         ImageHandle,
+  IN EFI_SYSTEM_TABLE   *SystemTable
+  )
+{
+  EFI_STATUS            Status;
+  FDT_CLIENT_PROTOCOL   *FdtClient;
+  INT32                 Node;
+  CONST UINT32          *Reg;
+  UINT32                RegSize;
+  UINT64                RegBase;
+  UINT64                Range;
+  RETURN_STATUS         PcdStatus;
+
+  Status = gBS->LocateProtocol (
+                  &gFdtClientProtocolGuid,
+                  NULL,
+                  (VOID **)&FdtClient
+                  );
+  ASSERT_EFI_ERROR (Status);
+
+  Status = FdtClient->FindCompatibleNode (
+                        FdtClient,
+                        "motorola,mc146818",
+                        &Node
+                        );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: No 'motorola,mc146818' compatible DT node found\n",
+      __FUNCTION__
+      ));
+    return Status;
+  }
+
+  Status = FdtClient->GetNodeProperty (
+                        FdtClient,
+                        Node,
+                        "reg",
+                        (CONST VOID **)&Reg,
+                        &RegSize
+                        );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: No 'reg' property found in 'motorola,mc146818' compatible DT node\n",
+      __FUNCTION__
+      ));
+    return Status;
+  }
+
+  ASSERT (RegSize == 16);
+
+  RegBase = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[0]));
+  Range = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[2]));
+  DEBUG ((
+    DEBUG_INFO,
+    "Found motorola,mc146818 RTC @ 0x%Lx Range = 0x%x\n",
+    RegBase,
+    Range
+    ));
+
+  // The address range must cover the RTC Index and the Target registers.
+  ASSERT (Range >= 0x2);
+
+  // RTC Index register is at offset 0x0
+  PcdStatus = PcdSet64S (
+                PcdRtcIndexRegister64,
+                (RegBase + RTC_INDEX_REG_OFFSET)
+                );
+  ASSERT_RETURN_ERROR (PcdStatus);
+
+  // RTC Target register is at offset 0x1
+  PcdStatus = PcdSet64S (
+                PcdRtcTargetRegister64,
+                (RegBase + RTC_TARGET_REG_OFFSET)
+                );
+  ASSERT_RETURN_ERROR (PcdStatus);
+
+  Status = KvmtoolRtcMapMemory (ImageHandle, (RegBase & ~EFI_PAGE_MASK));
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "Failed to map memory for motorola,mc146818. Status = %r\n",
+      Status
+      ));
+    return Status;
+  }
+
+  //
+  // UEFI takes ownership of the RTC hardware, and exposes its functionality
+  // through the UEFI Runtime Services GetTime, SetTime, etc. This means we
+  // need to disable it in the device tree to prevent the OS from attaching
+  // its device driver as well.
+  //
+  Status = FdtClient->SetNodeProperty (
+                        FdtClient,
+                        Node,
+                        "status",
+                        "disabled",
+                        sizeof ("disabled")
+                        );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_WARN,
+      "Failed to set motorola,mc146818 status to 'disabled', Status = %r\n",
+      Status
+      ));
+  }
+
+  return EFI_SUCCESS;
+}
diff --git a/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf
new file mode 100644
index 0000000000000000000000000000000000000000..0d1b27997ed00d7c1be3578cab4d1d2eba663a90
--- /dev/null
+++ b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf
@@ -0,0 +1,42 @@
+#/** @file
+#  FDT client library for motorola,mc146818 RTC driver
+#
+#  Copyright (c) 2020, ARM Limited. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x0001001B
+  BASE_NAME                      = KvmtoolRtcFdtClientLib
+  FILE_GUID                      = 3254B4F7-30B5-48C6-B06A-D8FF97F3EF95
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = KvmtoolRtcFdtClientLib|DXE_DRIVER DXE_RUNTIME_DRIVER
+  CONSTRUCTOR                    = KvmtoolRtcFdtClientLibConstructor
+
+[Sources]
+  KvmtoolRtcFdtClientLib.c
+
+[Packages]
+  ArmVirtPkg/ArmVirtPkg.dec
+  MdePkg/MdePkg.dec
+  PcAtChipsetPkg/PcAtChipsetPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  PcdLib
+  UefiBootServicesTableLib
+  DxeServicesTableLib
+
+[Protocols]
+  gFdtClientProtocolGuid                                ## CONSUMES
+
+[Pcd]
+  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64
+  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64
+
+[Depex]
+  gFdtClientProtocolGuid
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

* [PATCH v4 03/15] ArmPlatformPkg: Dynamic flash variable base
  2020-07-07 12:47 [PATCH v4 00/15] Kvmtool guest firmware support for Arm Sami Mujawar
  2020-07-07 12:47 ` [PATCH v4 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver Sami Mujawar
  2020-07-07 12:47 ` [PATCH v4 02/15] ArmVirtPkg: Add Kvmtool RTC Fdt Client Library Sami Mujawar
@ 2020-07-07 12:47 ` Sami Mujawar
  2020-07-07 12:47 ` [PATCH v4 04/15] ArmVirtPkg: Add kvmtool platform driver Sami Mujawar
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Sami Mujawar @ 2020-07-07 12:47 UTC (permalink / raw)
  To: devel; +Cc: Sami Mujawar, ard.biesheuvel, leif, philmd

Some virtual machine managers like kvmtool can relocate
the devices in the system memory map. The information
about the devices location in memory is described in the
device tree. Therefore, the CFI memory region and the
associated Non volatile storage variables need to be
adjusted accordingly.

To support such use cases the non-volatile storage
variable base PCD PcdFlashNvStorageVariableBase has
been defined as a dynamic PCD.

The NOR flash driver was using the Flash non-volatile
storage variable base PCD as a fixed PCD, thereby
preventing runtime resolution of the variable base
address.

Therefore update the NOR flash driver to load the
PCD using PcdGet32 instead of FixedPcdGet32.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Ard Biesheuvel <Ard.Biesheuvel@arm.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
---

Notes:
    v4:
      - No code changes, resending patch with v4 series.              [Sami]
        Ref: https://edk2.groups.io/g/devel/message/61666
    
    v3:
      - Resend patch with updated series.                             [Sami]
        Ref: https://edk2.groups.io/g/devel/topic/74200906#60337
    
    v2:
      - Add support to make PCD PcdFlashNvStorageVariableBase         [Sami]
        a dynamic PCD and update NOR flash driver to PcdGet32
        instead of FixedPcdGet32

 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
index e248fdf6db94191648b5d33bf1a9263f446ee141..9cdd85096a463f69b3b864cecdeaf247e65f4f73 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
@@ -1,6 +1,6 @@
 /*++ @file  NorFlashFvbDxe.c
 
- Copyright (c) 2011 - 2014, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2011 - 2020, ARM Ltd. All rights reserved.<BR>
 
  SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -736,7 +736,7 @@ NorFlashFvbInitialize (
       EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
   ASSERT_EFI_ERROR (Status);
 
-  mFlashNvStorageVariableBase = FixedPcdGet32 (PcdFlashNvStorageVariableBase);
+  mFlashNvStorageVariableBase = PcdGet32 (PcdFlashNvStorageVariableBase);
 
   // Set the index of the first LBA for the FVB
   Instance->StartLba = (PcdGet32 (PcdFlashNvStorageVariableBase) - Instance->RegionBaseAddress) / Instance->Media.BlockSize;
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

* [PATCH v4 04/15] ArmVirtPkg: Add kvmtool platform driver
  2020-07-07 12:47 [PATCH v4 00/15] Kvmtool guest firmware support for Arm Sami Mujawar
                   ` (2 preceding siblings ...)
  2020-07-07 12:47 ` [PATCH v4 03/15] ArmPlatformPkg: Dynamic flash variable base Sami Mujawar
@ 2020-07-07 12:47 ` Sami Mujawar
  2020-07-07 13:22   ` Ard Biesheuvel
  2020-07-07 12:47 ` [PATCH v4 05/15] ArmVirtPkg: kvmtool platform memory map Sami Mujawar
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 25+ messages in thread
From: Sami Mujawar @ 2020-07-07 12:47 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ard.biesheuvel, leif, lersek, Alexandru.Elisei,
	Andre.Przywara, Matteo.Carlini, Laura.Moretta, nd

Kvmtool is a virtual machine manager that enables
hosting KVM guests. It essentially provides a
virtual hardware platform for guest operating
systems.

Kvmtool hands of a device tree containing the
current hardware configuration to the firmware.

A standards-based operating system would use
ACPI to consume the platform hardware
information, while some operating systems may
prefer to use Device Tree.

The KvmtoolPlatformDxe performs the platform
actions like determining if the firmware should
expose ACPI or the Device Tree based hardware
description to the operating system.

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

Notes:
    v4:
      - Cleaned up include file and LibraryClasses list.          [Sami]
      - Missing MemoryAllocationLib here - either add it          [Ard]
        here, or remove it from LibraryClasses.
        Ref: https://edk2.groups.io/g/devel/message/61714
    
    v3:
      - Don't use CpuDeadLoop()s in your drivers.                 [Ard]
      - Returned error code instead of dead loop.                 [Sami]
      - Installing a protocol on an image handle should not       [Ard]
        ever fail. So just use ASSERT_EFI_ERROR().
      - Added assert and returned status code.                    [Sami]
        Ref: https://edk2.groups.io/g/devel/topic/74200911#59650
    
    v2:
      - Updated according to review comments.                     [Sami]
    
    v1:
      - Add kvmtool platform driver to support loading platform   [Sami]
        specific information.
      - Keep code to initialise the variable storage PCDs in the  [Laszlo]
        platform-specific FVB driver.
      - Document code derived from                                [Laszlo]
        "ArmVirtPkg/PlatformHasAcpiDtDxe"
        Ref: https://edk2.groups.io/g/devel/topic/30915278#30757

 ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c   | 82 ++++++++++++++++++++
 ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf | 44 +++++++++++
 2 files changed, 126 insertions(+)

diff --git a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c
new file mode 100644
index 0000000000000000000000000000000000000000..a42b64d1061dcdf8163775f66b6d2f550e481315
--- /dev/null
+++ b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c
@@ -0,0 +1,82 @@
+/** @file
+
+  The KvmtoolPlatformDxe performs the platform specific initialization like:
+  - It decides if the firmware should expose ACPI or Device Tree-based
+    hardware description to the operating system.
+
+  Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Guid/VariableFormat.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/FdtClient.h>
+
+/** Decide if the firmware should expose ACPI tables or Device Tree and
+    install the appropriate protocol interface.
+
+  Note: This function is derived from "ArmVirtPkg/PlatformHasAcpiDtDxe",
+        by dropping the word size check, and the fw_cfg check.
+
+  @param [in]  ImageHandle  Handle for this image.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_OUT_OF_RESOURCES    There was not enough memory to install the
+                                  protocols.
+  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
+
+**/
+STATIC
+EFI_STATUS
+PlatformHasAcpiDt (
+  IN EFI_HANDLE           ImageHandle
+  )
+{
+  if (!PcdGetBool (PcdForceNoAcpi)) {
+    // Expose ACPI tables
+    return gBS->InstallProtocolInterface (
+                  &ImageHandle,
+                  &gEdkiiPlatformHasAcpiGuid,
+                  EFI_NATIVE_INTERFACE,
+                  NULL
+                  );
+  }
+
+  // Expose the Device Tree.
+  return gBS->InstallProtocolInterface (
+                &ImageHandle,
+                &gEdkiiPlatformHasDeviceTreeGuid,
+                EFI_NATIVE_INTERFACE,
+                NULL
+                );
+}
+
+/** Entry point for Kvmtool Platform Dxe
+
+  @param [in]  ImageHandle  Handle for this image.
+  @param [in]  SystemTable  Pointer to the EFI system table.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_OUT_OF_RESOURCES    There was not enough memory to install the
+                                  protocols.
+  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
+
+**/
+EFI_STATUS
+EFIAPI
+KvmtoolPlatformDxeEntryPoint (
+  IN EFI_HANDLE           ImageHandle,
+  IN EFI_SYSTEM_TABLE     *SystemTable
+  )
+{
+  EFI_STATUS                     Status;
+
+  Status = PlatformHasAcpiDt (ImageHandle);
+  ASSERT_EFI_ERROR (Status);
+
+  return Status;
+}
diff --git a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
new file mode 100644
index 0000000000000000000000000000000000000000..05087178cc0fa3f7b414336861b39cedec48b68a
--- /dev/null
+++ b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
@@ -0,0 +1,44 @@
+#/** @file
+#
+#  The KvmtoolPlatformDxe performs the platform specific initialization like:
+#  - It decides if the firmware should expose ACPI or Device Tree-based
+#    hardware description to the operating system.
+#
+#  Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x0001001B
+  BASE_NAME                      = KvmtoolPlatformDxe
+  FILE_GUID                      = 7479CCCD-D721-442A-8C73-A72DBB886669
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = KvmtoolPlatformDxeEntryPoint
+
+[Sources]
+  KvmtoolPlatformDxe.c
+
+[Packages]
+  ArmVirtPkg/ArmVirtPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+
+[Guids]
+  gEdkiiPlatformHasAcpiGuid       ## SOMETIMES_PRODUCES ## PROTOCOL
+  gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
+
+[Pcd]
+  gArmVirtTokenSpaceGuid.PcdForceNoAcpi
+
+[Depex]
+  TRUE
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

* [PATCH v4 05/15] ArmVirtPkg: kvmtool platform memory map
  2020-07-07 12:47 [PATCH v4 00/15] Kvmtool guest firmware support for Arm Sami Mujawar
                   ` (3 preceding siblings ...)
  2020-07-07 12:47 ` [PATCH v4 04/15] ArmVirtPkg: Add kvmtool platform driver Sami Mujawar
@ 2020-07-07 12:47 ` Sami Mujawar
  2020-07-07 12:48 ` [PATCH v4 06/15] ArmVirtPkg: Add Kvmtool NOR flash lib Sami Mujawar
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Sami Mujawar @ 2020-07-07 12:47 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ard.biesheuvel, leif, lersek, Alexandru.Elisei,
	Andre.Przywara, Matteo.Carlini, Laura.Moretta, nd

Kvmtool is a virtual machine manager that enables
hosting KVM guests. Kvmtool allows to vary the
hardware configuration of the virtual platform
it provides to the guest partition. It provides
the current hardware configuration to the firmware
by handing off a device tree containing the hardware
information.

This library parses the kvmtool provided device
tree and populates the system memory map for the
kvmtool virtual platform.

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

Notes:
    v4:
      - Remove logging code as this requires support for Early      [Sami]
        serial port initialisation which is dropped in this
        series as it would not work with Xen.
        Ref: https://edk2.groups.io/g/devel/message/61669
             https://edk2.groups.io/g/devel/topic/75081479
    
    v3:
      - Fix coding style, use "Txt" in LOG_VM_MAP(txt).             [Laszlo]
      - Fixed coding style and also formatted log output.           [Sami]
        Ref: https://edk2.groups.io/g/devel/topic/74200913#59548
    v2:
      - Library to populate Kvmtool system memory map.              [Sami]

 ArmVirtPkg/Library/KvmtoolVirtMemInfoLib/KvmtoolVirtMemInfoLib.c   | 96 ++++++++++++++++++++
 ArmVirtPkg/Library/KvmtoolVirtMemInfoLib/KvmtoolVirtMemInfoLib.inf | 42 +++++++++
 2 files changed, 138 insertions(+)

diff --git a/ArmVirtPkg/Library/KvmtoolVirtMemInfoLib/KvmtoolVirtMemInfoLib.c b/ArmVirtPkg/Library/KvmtoolVirtMemInfoLib/KvmtoolVirtMemInfoLib.c
new file mode 100644
index 0000000000000000000000000000000000000000..50e805b252d7564ac4770162eb68eddfc1153d4b
--- /dev/null
+++ b/ArmVirtPkg/Library/KvmtoolVirtMemInfoLib/KvmtoolVirtMemInfoLib.c
@@ -0,0 +1,96 @@
+/** @file
+
+  Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Base.h>
+#include <Library/ArmLib.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+
+// Number of Virtual Memory Map Descriptors
+#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          5
+
+/**
+  Return the Virtual Memory Map of your platform
+
+  This Virtual Memory Map is used by MemoryInitPei Module to initialize the MMU
+  on your platform.
+
+  @param[out]   VirtualMemoryMap    Array of ARM_MEMORY_REGION_DESCRIPTOR
+                                    describing a Physical-to-Virtual Memory
+                                    mapping. This array must be ended by a
+                                    zero-filled entry. The allocated memory
+                                    will not be freed.
+
+**/
+VOID
+ArmVirtGetMemoryMap (
+  OUT ARM_MEMORY_REGION_DESCRIPTOR   **VirtualMemoryMap
+  )
+{
+  ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
+  UINTN                          Idx = 0;
+  EFI_PHYSICAL_ADDRESS           TopOfAddressSpace;
+
+  ASSERT (VirtualMemoryMap != NULL);
+
+  TopOfAddressSpace = LShiftU64 (1ULL, ArmGetPhysicalAddressBits ());
+
+  VirtualMemoryTable = (ARM_MEMORY_REGION_DESCRIPTOR*)
+                        AllocatePages (
+                          EFI_SIZE_TO_PAGES (
+                            sizeof (ARM_MEMORY_REGION_DESCRIPTOR) *
+                            MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS
+                            )
+                          );
+  if (VirtualMemoryTable == NULL) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Error: Failed to Allocate Pages\n",
+      __FUNCTION__
+      ));
+    return;
+  }
+
+  // System DRAM
+  VirtualMemoryTable[Idx].PhysicalBase = PcdGet64 (PcdSystemMemoryBase);
+  VirtualMemoryTable[Idx].VirtualBase  = VirtualMemoryTable[Idx].PhysicalBase;
+  VirtualMemoryTable[Idx].Length       = PcdGet64 (PcdSystemMemorySize);
+  VirtualMemoryTable[Idx].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
+
+  // Peripheral space before DRAM
+  VirtualMemoryTable[++Idx].PhysicalBase = 0x0;
+  VirtualMemoryTable[Idx].VirtualBase    = 0x0;
+  VirtualMemoryTable[Idx].Length         = PcdGet64 (PcdSystemMemoryBase);
+  VirtualMemoryTable[Idx].Attributes     = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+
+  // Peripheral space after DRAM
+  VirtualMemoryTable[++Idx].PhysicalBase = PcdGet64 (PcdSystemMemoryBase) +
+                                           PcdGet64 (PcdSystemMemorySize);
+  VirtualMemoryTable[Idx].VirtualBase    = VirtualMemoryTable[Idx].PhysicalBase;
+  VirtualMemoryTable[Idx].Length         = TopOfAddressSpace -
+                                           VirtualMemoryTable[Idx].PhysicalBase;
+  VirtualMemoryTable[Idx].Attributes     = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+
+  // Map the FV region as normal executable memory
+  VirtualMemoryTable[++Idx].PhysicalBase = PcdGet64 (PcdFvBaseAddress);
+  VirtualMemoryTable[Idx].VirtualBase  = VirtualMemoryTable[Idx].PhysicalBase;
+  VirtualMemoryTable[Idx].Length       = FixedPcdGet32 (PcdFvSize);
+  VirtualMemoryTable[Idx].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
+
+  // End of Table
+  VirtualMemoryTable[++Idx].PhysicalBase  = 0;
+  VirtualMemoryTable[Idx].VirtualBase     = 0;
+  VirtualMemoryTable[Idx].Length          = 0;
+  VirtualMemoryTable[Idx].Attributes      = (ARM_MEMORY_REGION_ATTRIBUTES)0;
+
+  ASSERT((Idx + 1) <= MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS);
+
+  *VirtualMemoryMap = VirtualMemoryTable;
+}
diff --git a/ArmVirtPkg/Library/KvmtoolVirtMemInfoLib/KvmtoolVirtMemInfoLib.inf b/ArmVirtPkg/Library/KvmtoolVirtMemInfoLib/KvmtoolVirtMemInfoLib.inf
new file mode 100644
index 0000000000000000000000000000000000000000..dbf4ceabe3ae0db5e743e1d9a575542dca32ed0a
--- /dev/null
+++ b/ArmVirtPkg/Library/KvmtoolVirtMemInfoLib/KvmtoolVirtMemInfoLib.inf
@@ -0,0 +1,42 @@
+#/* @file
+#
+#  Copyright (c) 2018, ARM Limited. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#*/
+
+[Defines]
+  INF_VERSION                    = 0x0001001B
+  BASE_NAME                      = KvmtoolVirtMemInfoLib
+  FILE_GUID                      = B752E953-394F-462C-811C-F8BE35C8C071
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = ArmVirtMemInfoLib
+
+[Sources]
+  KvmtoolVirtMemInfoLib.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  ArmVirtPkg/ArmVirtPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  ArmLib
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  MemoryAllocationLib
+  PcdLib
+
+[Pcd]
+  gArmTokenSpaceGuid.PcdFvBaseAddress
+  gArmTokenSpaceGuid.PcdSystemMemoryBase
+  gArmTokenSpaceGuid.PcdSystemMemorySize
+
+[FixedPcd]
+  gArmTokenSpaceGuid.PcdFvSize
+
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

* [PATCH v4 06/15] ArmVirtPkg: Add Kvmtool NOR flash lib
  2020-07-07 12:47 [PATCH v4 00/15] Kvmtool guest firmware support for Arm Sami Mujawar
                   ` (4 preceding siblings ...)
  2020-07-07 12:47 ` [PATCH v4 05/15] ArmVirtPkg: kvmtool platform memory map Sami Mujawar
@ 2020-07-07 12:48 ` Sami Mujawar
  2020-07-07 13:26   ` Ard Biesheuvel
  2020-07-07 12:48 ` [PATCH v4 07/15] MdeModulePkg: Fix constructor invocation ordering Sami Mujawar
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 25+ messages in thread
From: Sami Mujawar @ 2020-07-07 12:48 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ard.biesheuvel, leif, philmd, lersek,
	Alexandru.Elisei, Andre.Przywara, Matteo.Carlini, Laura.Moretta,
	nd

Kvmtool places the base address of the CFI flash in
the device tree it passes to UEFI. This library
parses the kvmtool device tree to read the CFI base
address and initialise the PCDs use by the NOR flash
driver and the variable storage.

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

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

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

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

diff --git a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c
new file mode 100644
index 0000000000000000000000000000000000000000..dce585b779a87dabde280cf4bbca5b828ceccda4
--- /dev/null
+++ b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c
@@ -0,0 +1,336 @@
+/** @file
+   An instance of the NorFlashPlatformLib for Kvmtool platform.
+
+ Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ **/
+
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/NorFlashPlatformLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/FdtClient.h>
+
+/** Macro defining the NOR block size configured in Kvmtool.
+*/
+#define KVMTOOL_NOR_BLOCK_SIZE  SIZE_64KB
+
+/** Macro defining the maximum number of Flash devices.
+*/
+#define MAX_FLASH_DEVICES       4
+
+/** Macro defining the cfi-flash label describing the UEFI variable store.
+*/
+#define LABEL_UEFI_VAR_STORE    "System-firmware"
+
+STATIC NOR_FLASH_DESCRIPTION  mNorFlashDevices[MAX_FLASH_DEVICES];
+STATIC UINTN                  mNorFlashDeviceCount = 0;
+STATIC INT32                  mUefiVarStoreNode = MAX_INT32;
+STATIC FDT_CLIENT_PROTOCOL    *mFdtClient;
+
+/** This function performs platform specific actions to initialise
+    the NOR flash, if required.
+
+  @retval EFI_SUCCESS           Success.
+**/
+EFI_STATUS
+NorFlashPlatformInitialization (
+  VOID
+  )
+{
+  EFI_STATUS                  Status;
+
+  DEBUG ((DEBUG_INFO, "NorFlashPlatformInitialization\n"));
+
+  if ((mNorFlashDeviceCount > 0) && (mUefiVarStoreNode != MAX_INT32)) {
+    //
+    // UEFI takes ownership of the cfi-flash hardware, and exposes its
+    // functionality through the UEFI Runtime Variable Service. This means we
+    // need to disable it in the device tree to prevent the OS from attaching
+    // its device driver as well.
+    // Note: This library is loaded twice. First by FaultTolerantWriteDxe to
+    // setup the PcdFlashNvStorageFtw* and later by NorFlashDxe to provide the
+    // NorFlashPlatformLib interfaces. If the node is disabled when the library
+    // is first loaded, then during the subsequent loading of the library the
+    // call to FindNextCompatibleNode() from the library constructor skips the
+    // FDT node used for UEFI storage variable. Due to this we cannot setup the
+    // NOR flash device description i.e. mNorFlashDevices[].
+    // Since NorFlashPlatformInitialization() is called only by NorFlashDxe,
+    // we know it is safe to disable the node here.
+    //
+    Status = mFdtClient->SetNodeProperty (
+                           mFdtClient,
+                           mUefiVarStoreNode,
+                           "status",
+                           "disabled",
+                           sizeof ("disabled")
+                           );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_WARN, "Failed to set cfi-flash status to 'disabled'\n"));
+    }
+  } else {
+    Status = EFI_NOT_FOUND;
+    DEBUG ((DEBUG_ERROR, "Flash device for UEFI variable storage not found\n"));
+  }
+
+  return Status;
+}
+
+/** Initialise Non volatile Flash storage variables.
+
+  @param [in]  FlashDevice Pointer to the NOR Flash device.
+
+  @retval EFI_SUCCESS           Success.
+  @retval EFI_INVALID_PARAMETER A parameter is invalid.
+  @retval EFI_OUT_OF_RESOURCES  Insufficient flash storage space.
+**/
+STATIC
+EFI_STATUS
+SetupVariableStore (
+  IN NOR_FLASH_DESCRIPTION * FlashDevice
+  )
+{
+  UINTN   FlashRegion;
+  UINTN   FlashNvStorageVariableBase;
+  UINTN   FlashNvStorageFtwWorkingBase;
+  UINTN   FlashNvStorageFtwSpareBase;
+  UINTN   FlashNvStorageVariableSize;
+  UINTN   FlashNvStorageFtwWorkingSize;
+  UINTN   FlashNvStorageFtwSpareSize;
+
+  FlashNvStorageVariableSize = PcdGet32 (PcdFlashNvStorageVariableSize);
+  FlashNvStorageFtwWorkingSize = PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
+  FlashNvStorageFtwSpareSize =  PcdGet32 (PcdFlashNvStorageFtwSpareSize);
+
+  if ((FlashNvStorageVariableSize == 0)   ||
+      (FlashNvStorageFtwWorkingSize == 0) ||
+      (FlashNvStorageFtwSpareSize == 0)) {
+    DEBUG ((DEBUG_ERROR, "FlashNvStorage size not defined\n"));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  // Setup the variable store
+  FlashRegion = FlashDevice->DeviceBaseAddress;
+
+  FlashNvStorageVariableBase = FlashRegion;
+  FlashRegion += PcdGet32 (PcdFlashNvStorageVariableSize);
+
+  FlashNvStorageFtwWorkingBase = FlashRegion;
+  FlashRegion += PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
+
+  FlashNvStorageFtwSpareBase = FlashRegion;
+  FlashRegion += PcdGet32 (PcdFlashNvStorageFtwSpareSize);
+
+  if (FlashRegion > (FlashDevice->DeviceBaseAddress + FlashDevice->Size)) {
+    DEBUG ((DEBUG_ERROR, "Insufficient flash storage size\n"));
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  PcdSet32S (
+    PcdFlashNvStorageVariableBase,
+    FlashNvStorageVariableBase
+    );
+
+  PcdSet32S (
+    PcdFlashNvStorageFtwWorkingBase,
+    FlashNvStorageFtwWorkingBase
+    );
+
+  PcdSet32S (
+    PcdFlashNvStorageFtwSpareBase,
+    FlashNvStorageFtwSpareBase
+    );
+
+  DEBUG ((
+    DEBUG_INFO,
+    "PcdFlashNvStorageVariableBase = 0x%x\n",
+    FlashNvStorageVariableBase
+    ));
+  DEBUG ((
+    DEBUG_INFO,
+    "PcdFlashNvStorageVariableSize = 0x%x\n",
+    FlashNvStorageVariableSize
+    ));
+  DEBUG ((
+    DEBUG_INFO,
+    "PcdFlashNvStorageFtwWorkingBase = 0x%x\n",
+    FlashNvStorageFtwWorkingBase
+    ));
+  DEBUG ((
+    DEBUG_INFO,
+    "PcdFlashNvStorageFtwWorkingSize = 0x%x\n",
+    FlashNvStorageFtwWorkingSize
+    ));
+  DEBUG ((
+    DEBUG_INFO,
+    "PcdFlashNvStorageFtwSpareBase = 0x%x\n",
+    FlashNvStorageFtwSpareBase
+    ));
+  DEBUG ((
+    DEBUG_INFO,
+    "PcdFlashNvStorageFtwSpareSize = 0x%x\n",
+    FlashNvStorageFtwSpareSize
+    ));
+
+  return EFI_SUCCESS;
+}
+
+/** Return the Flash devices on the platform.
+
+  @param [out]  NorFlashDescriptions    Pointer to the Flash device description.
+  @param [out]  Count                   Number of Flash devices.
+
+  @retval EFI_SUCCESS           Success.
+  @retval EFI_NOT_FOUND         Flash device not found.
+**/
+EFI_STATUS
+NorFlashPlatformGetDevices (
+  OUT NOR_FLASH_DESCRIPTION   **NorFlashDescriptions,
+  OUT UINT32                  *Count
+  )
+{
+  if (mNorFlashDeviceCount > 0) {
+    *NorFlashDescriptions = mNorFlashDevices;
+    *Count = mNorFlashDeviceCount;
+    return EFI_SUCCESS;
+  }
+  return EFI_NOT_FOUND;
+}
+
+/** Entrypoint for NorFlashPlatformLib.
+
+  @param [in]  ImageHandle  The handle to the image.
+  @param [in]  SystemTable  Pointer to the System Table.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
+  @retval EFI_NOT_FOUND           Flash device not found.
+**/
+EFI_STATUS
+EFIAPI
+NorFlashPlatformLibConstructor (
+  IN  EFI_HANDLE          ImageHandle,
+  IN  EFI_SYSTEM_TABLE  * SystemTable
+  )
+{
+  INT32                       Node;
+  EFI_STATUS                  Status;
+  EFI_STATUS                  FindNodeStatus;
+  CONST UINT32                *Reg;
+  UINT32                      PropSize;
+  UINT64                      Base;
+  UINT64                      Size;
+  UINTN                       UefiVarStoreIndex;
+  CONST CHAR8                 *Label;
+  UINT32                      LabelLen;
+
+  if (mNorFlashDeviceCount != 0) {
+    return EFI_SUCCESS;
+  }
+
+  Status = gBS->LocateProtocol (
+                  &gFdtClientProtocolGuid,
+                  NULL,
+                  (VOID **)&mFdtClient
+                  );
+  ASSERT_EFI_ERROR (Status);
+
+  UefiVarStoreIndex = MAX_UINTN;
+  for (FindNodeStatus = mFdtClient->FindCompatibleNode (
+                                      mFdtClient,
+                                      "cfi-flash",
+                                      &Node
+                                      );
+       !EFI_ERROR (FindNodeStatus) &&
+         (mNorFlashDeviceCount < MAX_FLASH_DEVICES);
+       FindNodeStatus = mFdtClient->FindNextCompatibleNode (
+                                      mFdtClient,
+                                      "cfi-flash",
+                                      Node,
+                                      &Node
+    )) {
+    Status = mFdtClient->GetNodeProperty (
+                           mFdtClient,
+                           Node,
+                           "label",
+                           (CONST VOID **)&Label,
+                           &LabelLen
+                           );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "%a: GetNodeProperty ('label') failed (Status == %r)\n",
+        __FUNCTION__,
+        Status
+        ));
+    } else if (AsciiStrCmp (Label, LABEL_UEFI_VAR_STORE) == 0) {
+      UefiVarStoreIndex = mNorFlashDeviceCount;
+      mUefiVarStoreNode = Node;
+    }
+
+    Status = mFdtClient->GetNodeProperty (
+                           mFdtClient,
+                           Node,
+                           "reg",
+                           (CONST VOID **)&Reg,
+                           &PropSize
+                           );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "%a: GetNodeProperty () failed (Status == %r)\n",
+        __FUNCTION__, Status));
+      continue;
+    }
+
+    ASSERT ((PropSize % (4 * sizeof (UINT32))) == 0);
+
+    while ((PropSize >= (4 * sizeof (UINT32))) &&
+           (mNorFlashDeviceCount < MAX_FLASH_DEVICES)) {
+      Base = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[0]));
+      Size = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[2]));
+      Reg += 4;
+
+      PropSize -= 4 * sizeof (UINT32);
+
+      //
+      // Disregard any flash devices that overlap with the primary FV.
+      // The firmware is not updatable from inside the guest anyway.
+      //
+      if ((PcdGet64 (PcdFvBaseAddress) + PcdGet32 (PcdFvSize) > Base) &&
+          (Base + Size) > PcdGet64 (PcdFvBaseAddress)) {
+        continue;
+      }
+
+      DEBUG ((
+        DEBUG_INFO,
+        "NOR%d : Base = 0x%lx, Size = 0x%lx\n",
+        mNorFlashDeviceCount,
+        Base,
+        Size
+        ));
+
+      mNorFlashDevices[mNorFlashDeviceCount].DeviceBaseAddress = (UINTN)Base;
+      mNorFlashDevices[mNorFlashDeviceCount].RegionBaseAddress = (UINTN)Base;
+      mNorFlashDevices[mNorFlashDeviceCount].Size = (UINTN)Size;
+      mNorFlashDevices[mNorFlashDeviceCount].BlockSize = KVMTOOL_NOR_BLOCK_SIZE;
+      mNorFlashDeviceCount++;
+    }
+  } // for
+
+  // Setup the variable store in the last device
+  if (mNorFlashDeviceCount > 0) {
+    if (UefiVarStoreIndex == MAX_UINTN) {
+      // We did not find a label matching the UEFI Variable store. Default to
+      // using the last cfi-flash device as the variable store.
+      UefiVarStoreIndex = mNorFlashDeviceCount - 1;
+      mUefiVarStoreNode = Node;
+    }
+    if (mNorFlashDevices[UefiVarStoreIndex].DeviceBaseAddress != 0) {
+      return SetupVariableStore (&mNorFlashDevices[UefiVarStoreIndex]);
+    }
+  }
+
+  return EFI_NOT_FOUND;
+}
+
diff --git a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
new file mode 100644
index 0000000000000000000000000000000000000000..a3230e4b2be668904322103825b93e867503984e
--- /dev/null
+++ b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
@@ -0,0 +1,49 @@
+#/** @file
+#
+#  Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x0001001B
+  BASE_NAME                      = NorFlashKvmtoolLib
+  FILE_GUID                      = E75F07A1-B160-4893-BDD4-09E32FF847DC
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = NorFlashPlatformLib
+  CONSTRUCTOR                    = NorFlashPlatformLibConstructor
+
+[Sources.common]
+  NorFlashKvmtool.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  ArmVirtPkg/ArmVirtPkg.dec
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  PcdLib
+  UefiBootServicesTableLib
+
+[Protocols]
+  gFdtClientProtocolGuid          ## CONSUMES
+
+[Pcd]
+  gArmTokenSpaceGuid.PcdFvBaseAddress
+  gArmTokenSpaceGuid.PcdFvSize
+
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
+
+[Depex]
+  gFdtClientProtocolGuid
+
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

* [PATCH v4 07/15] MdeModulePkg: Fix constructor invocation ordering
  2020-07-07 12:47 [PATCH v4 00/15] Kvmtool guest firmware support for Arm Sami Mujawar
                   ` (5 preceding siblings ...)
  2020-07-07 12:48 ` [PATCH v4 06/15] ArmVirtPkg: Add Kvmtool NOR flash lib Sami Mujawar
@ 2020-07-07 12:48 ` Sami Mujawar
  2020-07-07 12:48 ` [PATCH v4 08/15] ArmVirtPkg: GUID Hob for 16550 UART base address Sami Mujawar
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Sami Mujawar @ 2020-07-07 12:48 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ard.biesheuvel, leif, lersek, jian.j.wang, hao.a.wu,
	Alexandru.Elisei, Andre.Przywara, Matteo.Carlini, Laura.Moretta,
	nd

The BaseSerialPortLib16550 library does not implement
a constructor. This prevents the correct constructor
invocation order for dependent libraries.
e.g. A PlatformHookLib (for the Serial Port) may have
a dependency on retrieving data from a Hob. A Hob
library implementation may configure its initial state
in the HobLib constructor. Since BaseSerialPortLib16550
does not implement a constructor, the Basetools do not
resolve the correct order for constructor invocation.

To fix this, add an empty constructor to the serial port
library BaseSerialPortLib16550.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Acked-by: Ard Biesheuvel <Ard.Biesheuvel@arm.com>
---

Notes:
    v4:
      - No code change, resending patch with v4 series.               [Sami]
        Ref: https://edk2.groups.io/g/devel/topic/75081484

 MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c   | 17 +++++++++++++++++
 MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf |  3 +++
 2 files changed, 20 insertions(+)

diff --git a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
index 9cb50dd80d5634ab2aa6d68bf5ca7fb891463eef..0fd1382ee83c9de09d8250830bd9569056fcee2f 100644
--- a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
+++ b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
@@ -4,6 +4,7 @@
   (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
   Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
   Copyright (c) 2018, AMD Incorporated. All rights reserved.<BR>
+  Copyright (c) 2020, ARM Limited. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -1102,3 +1103,19 @@ SerialPortSetAttributes (
   return RETURN_SUCCESS;
 }
 
+/** Base Serial Port 16550 Library Constructor
+
+  @retval RETURN_SUCCESS  Success.
+*/
+EFI_STATUS
+EFIAPI
+BaseSerialPortLib16550 (
+  VOID
+  )
+{
+  // Nothing to do here. This constructor is added to
+  // enable the chain of constructor invocation for
+  // dependent libraries.
+  return RETURN_SUCCESS;
+}
+
diff --git a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
index 8b4ae3f1d4ee1e2e9a8b81eab4c900541ce8cfb6..92b7a8b7896a305d2ce22589f8a9593618d37bb7 100644
--- a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
+++ b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
@@ -2,6 +2,8 @@
 #  SerialPortLib instance for 16550 UART.
 #
 #  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2020, ARM Limited. All rights reserved.
+#
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -14,6 +16,7 @@ [Defines]
   MODULE_TYPE                    = BASE
   VERSION_STRING                 = 1.1
   LIBRARY_CLASS                  = SerialPortLib
+  CONSTRUCTOR                    = BaseSerialPortLib16550
 
 [Packages]
   MdePkg/MdePkg.dec
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

* [PATCH v4 08/15] ArmVirtPkg: GUID Hob for 16550 UART base address
  2020-07-07 12:47 [PATCH v4 00/15] Kvmtool guest firmware support for Arm Sami Mujawar
                   ` (6 preceding siblings ...)
  2020-07-07 12:48 ` [PATCH v4 07/15] MdeModulePkg: Fix constructor invocation ordering Sami Mujawar
@ 2020-07-07 12:48 ` Sami Mujawar
  2020-07-07 12:48 ` [PATCH v4 09/15] ArmVirtPkg: 16550 UART Platform hook library Sami Mujawar
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Sami Mujawar @ 2020-07-07 12:48 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ard.biesheuvel, leif, lersek, Alexandru.Elisei,
	Andre.Przywara, Matteo.Carlini, Laura.Moretta, nd

Introduce a new GUID Hob gEarly16550UartBaseAddressGuid
to cache the base address of the 16550 UART, for when
PCD access is not available.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Ard Biesheuvel <Ard.Biesheuvel@arm.com>
---

Notes:
    v4:
      - No code change, resending patch with v4 series.           [Sami]
        Ref: https://edk2.groups.io/g/devel/message/61720

 ArmVirtPkg/ArmVirtPkg.dec                           |  2 ++
 ArmVirtPkg/Include/Guid/Early16550UartBaseAddress.h | 22 ++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index 08ddd68a863eb82778fa7a92b20ac1bff271f5e5..bf82f7f1f3f2a0ce545d87fa93350f56de78328b 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -1,6 +1,7 @@
 #/** @file
 #
 #  Copyright (c) 2014, Linaro Limited. All rights reserved.
+#  Copyright (c) 2020, ARM Limited. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -30,6 +31,7 @@ [LibraryClasses]
 [Guids.common]
   gArmVirtTokenSpaceGuid = { 0x0B6F5CA7, 0x4F53, 0x445A, { 0xB7, 0x6E, 0x2E, 0x36, 0x5B, 0x80, 0x63, 0x66 } }
   gEarlyPL011BaseAddressGuid       = { 0xB199DEA9, 0xFD5C, 0x4A84, { 0x80, 0x82, 0x2F, 0x41, 0x70, 0x78, 0x03, 0x05 } }
+  gEarly16550UartBaseAddressGuid   = { 0xea67ca3e, 0x1f54, 0x436b, { 0x97, 0x88, 0xd4, 0xeb, 0x29, 0xc3, 0x42, 0x67 } }
 
   gArmVirtVariableGuid   = { 0x50bea1e5, 0xa2c5, 0x46e9, { 0x9b, 0x3a, 0x59, 0x59, 0x65, 0x16, 0xb0, 0x0a } }
 
diff --git a/ArmVirtPkg/Include/Guid/Early16550UartBaseAddress.h b/ArmVirtPkg/Include/Guid/Early16550UartBaseAddress.h
new file mode 100644
index 0000000000000000000000000000000000000000..6b76f37a27dac03d6749f31a03e8026023231056
--- /dev/null
+++ b/ArmVirtPkg/Include/Guid/Early16550UartBaseAddress.h
@@ -0,0 +1,22 @@
+/** @file
+  GUID for the HOB that caches the base address of the 16550 serial port, for
+  when PCD access is not available.
+
+  Copyright (c) 2020, ARM Limited. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef EARLY_16550_UART_BASE_ADDRESS_H__
+#define EARLY_16550_UART_BASE_ADDRESS_H__
+
+#define EARLY_16550_UART_BASE_ADDRESS_GUID {       \
+  0xea67ca3e, 0x1f54, 0x436b, {                    \
+    0x97, 0x88, 0xd4, 0xeb, 0x29, 0xc3, 0x42, 0x67 \
+    }                                              \
+  }
+
+extern EFI_GUID gEarly16550UartBaseAddressGuid;
+
+#endif // EARLY_16550_UART_BASE_ADDRESS_H__
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

* [PATCH v4 09/15] ArmVirtPkg: 16550 UART Platform hook library
  2020-07-07 12:47 [PATCH v4 00/15] Kvmtool guest firmware support for Arm Sami Mujawar
                   ` (7 preceding siblings ...)
  2020-07-07 12:48 ` [PATCH v4 08/15] ArmVirtPkg: GUID Hob for 16550 UART base address Sami Mujawar
@ 2020-07-07 12:48 ` Sami Mujawar
  2020-07-07 12:48 ` [PATCH v4 10/15] ArmVirtPkg: Add Kvmtool Platform Pei Lib Sami Mujawar
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Sami Mujawar @ 2020-07-07 12:48 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ard.biesheuvel, leif, lersek, Alexandru.Elisei,
	Andre.Przywara, Matteo.Carlini, Laura.Moretta, nd

The BaseSerialPort16550 library invokes the
PlatformHookSerialPortInitialize() implemented as
part of the PlatformHook library, to perform platform
specific initialization required to enable use of the
16550 device. The BaseSerialPort16550 library uses
the PcdSerialRegisterBase to obtain the base address
of the UART for MMIO operations.

Some VMMs like Kvmtool provide the base address of
the console serial port in the platform device tree.

This patch introduces two instances of the Platform
Hook library:
1. EarlyFdt16550SerialPortHookLib - parses the
   platform device tree to extract the base
   address of the 16550 UART and update the PCD
   PcdSerialRegisterBase.
2. Fdt16550SerialPortHookLib - reads the GUID
   Hob gEarly16550UartBaseAddressGuid (that caches
   the base address of the 16550 UART discovered
   during early stages) and updates the PCD
   PcdSerialRegisterBase.

Note:
  a. The PCD PcdSerialRegisterBase is configured
     as PatchableInModule.
  b. A separate patch introduces a PlatformPeiLib
     that trampolines the 16550 UART base address
     from the Pcd PcdSerialRegisterBase to the
     GUID Hob gEarly16550UartBaseAddressGuid.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Ard Biesheuvel <Ard.Biesheuvel@arm.com>
---

Notes:
    v4:
      - No code change, resending patch with v4 series.         [Sami]
        Ref: https://edk2.groups.io/g/devel/message/61721

 ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.c   | 138 ++++++++++++++++++++
 ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.inf |  36 +++++
 ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.c        |  57 ++++++++
 ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf      |  39 ++++++
 ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.uni      |  14 ++
 5 files changed, 284 insertions(+)

diff --git a/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.c b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.c
new file mode 100644
index 0000000000000000000000000000000000000000..4e97302d01dc5692e046fb901663373828b532ee
--- /dev/null
+++ b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.c
@@ -0,0 +1,138 @@
+/** @file
+  Early Platform Hook Library instance for 16550 Uart.
+
+  Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Base.h>
+#include <Uefi.h>
+
+#include <Pi/PiBootMode.h>
+#include <Pi/PiHob.h>
+
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/HobLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PlatformHookLib.h>
+#include <libfdt.h>
+
+/** Get the UART base address of the console serial-port from the DT.
+
+  This function fetches the node referenced in the "stdout-path"
+  property of the "chosen" node and returns the base address of
+  the console UART.
+
+  @param [in]   Fdt                   Pointer to a Flattened Device Tree (Fdt).
+  @param [out]  SerialConsoleAddress  If success, contains the base address
+                                      of the console serial-port.
+
+  @retval EFI_SUCCESS             The function completed successfully.
+  @retval EFI_NOT_FOUND           Console serial-port info not found in DT.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+GetSerialConsolePortAddress (
+  IN  CONST VOID    *Fdt,
+  OUT       UINT64  *SerialConsoleAddress
+  )
+{
+  CONST CHAR8   *Prop;
+  INT32         PropSize;
+  CONST CHAR8   *Path;
+  INT32         PathLen;
+  INT32         ChosenNode;
+  INT32         SerialConsoleNode;
+  INT32         Len;
+  CONST CHAR8   *NodeStatus;
+  CONST UINT64  *RegProperty;
+
+  if ((Fdt == NULL) || (fdt_check_header (Fdt) != 0)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  // The "chosen" node resides at the the root of the DT. Fetch it.
+  ChosenNode = fdt_path_offset (Fdt, "/chosen");
+  if (ChosenNode < 0) {
+    return EFI_NOT_FOUND;
+  }
+
+  Prop = fdt_getprop (Fdt, ChosenNode, "stdout-path", &PropSize);
+  if (PropSize < 0) {
+    return EFI_NOT_FOUND;
+  }
+
+  // Determine the actual path length, as a colon terminates the path.
+  Path = ScanMem8 (Prop, ':', PropSize);
+  if (Path == NULL) {
+    PathLen = AsciiStrLen (Prop);
+  } else {
+    PathLen = Path - Prop;
+  }
+
+  // Aliases cannot start with a '/', so it must be the actual path.
+  if (Prop[0] == '/') {
+    SerialConsoleNode = fdt_path_offset_namelen (Fdt, Prop, PathLen);
+  } else {
+    // Lookup the alias, as this contains the actual path.
+    Path = fdt_get_alias_namelen (Fdt, Prop, PathLen);
+    if (Path == NULL) {
+      return EFI_NOT_FOUND;
+    }
+    SerialConsoleNode = fdt_path_offset (Fdt, Path);
+  }
+
+  NodeStatus = fdt_getprop (Fdt, SerialConsoleNode, "status", &Len);
+  if ((NodeStatus != NULL) && (AsciiStrCmp (NodeStatus, "okay") != 0)) {
+    return EFI_NOT_FOUND;
+  }
+
+  RegProperty = fdt_getprop (Fdt, SerialConsoleNode, "reg", &Len);
+  if (Len != 16) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  *SerialConsoleAddress = fdt64_to_cpu (ReadUnaligned64 (RegProperty));
+
+  return EFI_SUCCESS;
+}
+
+/** Platform hook to retrieve the 16550 UART base address from the platform
+    Device tree and store it in PcdSerialRegisterBase.
+
+  @retval RETURN_SUCCESS            Success.
+  @retval RETURN_INVALID_PARAMETER  A parameter was invalid.
+  @retval RETURN_NOT_FOUND          Serial port information not found.
+
+**/
+RETURN_STATUS
+EFIAPI
+PlatformHookSerialPortInitialize (
+  VOID
+  )
+{
+  RETURN_STATUS   Status;
+  VOID            *DeviceTreeBase;
+  UINT64          SerialConsoleAddress;
+
+  if (PcdGet64 (PcdSerialRegisterBase) != 0) {
+    return RETURN_SUCCESS;
+  }
+
+  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
+  if (DeviceTreeBase == NULL) {
+    return RETURN_NOT_FOUND;
+  }
+
+  Status = GetSerialConsolePortAddress (DeviceTreeBase, &SerialConsoleAddress);
+  if (RETURN_ERROR (Status)) {
+    return Status;
+  }
+
+  return (EFI_STATUS)PcdSet64S (PcdSerialRegisterBase, SerialConsoleAddress);
+}
+
diff --git a/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.inf b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.inf
new file mode 100644
index 0000000000000000000000000000000000000000..007a45eca2a622118044b527a5bc6403b706608a
--- /dev/null
+++ b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.inf
@@ -0,0 +1,36 @@
+## @file
+#  Early Platform Hook Library instance for 16550 Uart.
+#
+#  Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001B
+  BASE_NAME                      = EarlyFdt16550SerialPortHookLib
+  MODULE_UNI_FILE                = Fdt16550SerialPortHookLib.uni
+  FILE_GUID                      = FFB19961-79CC-4684-84A8-C31B0A2BBE82
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = PlatformHookLib|SEC PEI_CORE PEIM
+
+[Sources]
+  EarlyFdt16550SerialPortHookLib.c
+
+[LibraryClasses]
+  BaseLib
+  PcdLib
+  FdtLib
+  HobLib
+
+[Packages]
+  ArmVirtPkg/ArmVirtPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+
+[Pcd]
+  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
diff --git a/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.c b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.c
new file mode 100644
index 0000000000000000000000000000000000000000..1100c12aef361e39cf78f0c3e4e14cc2feecfcee
--- /dev/null
+++ b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.c
@@ -0,0 +1,57 @@
+/** @file
+  Platform Hook Library instance for 16550 Uart.
+
+  Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Base.h>
+#include <Uefi.h>
+
+#include <Pi/PiBootMode.h>
+#include <Pi/PiHob.h>
+
+#include <Guid/Early16550UartBaseAddress.h>
+#include <Guid/Fdt.h>
+#include <Guid/FdtHob.h>
+
+#include <Library/BaseLib.h>
+#include <Library/HobLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PlatformHookLib.h>
+
+/** Platform hook to retrieve the 16550 UART base address from the GUID Hob
+    that caches the UART base address from early boot stage and store it in
+    PcdSerialRegisterBase.
+
+  @retval RETURN_SUCCESS    Success.
+  @retval RETURN_NOT_FOUND  Serial Port information not found.
+
+**/
+RETURN_STATUS
+EFIAPI
+PlatformHookSerialPortInitialize (
+  VOID
+  )
+{
+  VOID            *Hob;
+  UINT64          *UartBase;
+
+  if (PcdGet64 (PcdSerialRegisterBase) != 0) {
+    return RETURN_SUCCESS;
+  }
+
+  Hob = GetFirstGuidHob (&gEarly16550UartBaseAddressGuid);
+  if ((Hob == NULL) || (GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (*UartBase))) {
+    return RETURN_NOT_FOUND;
+  }
+
+  UartBase = GET_GUID_HOB_DATA (Hob);
+  if ((UINTN)*UartBase == 0) {
+    return RETURN_NOT_FOUND;
+  }
+
+  return (RETURN_STATUS)PcdSet64S (PcdSerialRegisterBase, (UINTN)*UartBase);
+}
+
diff --git a/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf
new file mode 100644
index 0000000000000000000000000000000000000000..ba789ca69041b647a4a881b03110ff853df89f65
--- /dev/null
+++ b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf
@@ -0,0 +1,39 @@
+## @file
+#  Platform Hook Library instance for 16550 Uart.
+#
+#  Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001B
+  BASE_NAME                      = Fdt16550SerialPortHookLib
+  MODULE_UNI_FILE                = Fdt16550SerialPortHookLib.uni
+  FILE_GUID                      = C6DFD3F0-179D-4376-89A5-F641A2E7EFB5
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = PlatformHookLib|DXE_CORE DXE_DRIVER UEFI_DRIVER DXE_RUNTIME_DRIVER UEFI_APPLICATION
+  CONSTRUCTOR                    = PlatformHookSerialPortInitialize
+
+[Sources]
+  Fdt16550SerialPortHookLib.c
+
+[LibraryClasses]
+  BaseLib
+  PcdLib
+  HobLib
+
+[Packages]
+  ArmVirtPkg/ArmVirtPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
+
+[Guids]
+  gEarly16550UartBaseAddressGuid
+
diff --git a/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.uni b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.uni
new file mode 100644
index 0000000000000000000000000000000000000000..19a61a4a03244321bda7c88cd26128b44bfd46b4
--- /dev/null
+++ b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.uni
@@ -0,0 +1,14 @@
+// /** @file
+// Platform Hook Library instance for 16550 Uart.
+//
+//
+// Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
+//
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// **/
+
+#string STR_MODULE_ABSTRACT     #language en-US "Platform Hook Library instance for 16550 Uart."
+
+#string STR_MODULE_DESCRIPTION  #language en-US "Platform Hook Library instance for 16550 Uart."
+
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

* [PATCH v4 10/15] ArmVirtPkg: Add Kvmtool Platform Pei Lib
  2020-07-07 12:47 [PATCH v4 00/15] Kvmtool guest firmware support for Arm Sami Mujawar
                   ` (8 preceding siblings ...)
  2020-07-07 12:48 ` [PATCH v4 09/15] ArmVirtPkg: 16550 UART Platform hook library Sami Mujawar
@ 2020-07-07 12:48 ` Sami Mujawar
  2020-07-07 12:48 ` [PATCH v4 11/15] ArmVirtPkg: Support for kvmtool virtual platform Sami Mujawar
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Sami Mujawar @ 2020-07-07 12:48 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ard.biesheuvel, leif, lersek, Alexandru.Elisei,
	Andre.Przywara, Matteo.Carlini, Laura.Moretta, nd

The PlatformPeim() in the PlatformPeiLib is invoked
by the PrePiMain() and provides the platform an
opportunity to setup the plaform specific HOBs.

This PlatfromPeiLib initialises the Kvmtool platform
HOBs like the Fdt, 16550BaseAddress, etc.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Ard Biesheuvel <Ard.Biesheuvel@arm.com>
---

Notes:
    v4:
      - No code change, resending patch with v4 series.          [Sami]
        Ref: https://edk2.groups.io/g/devel/message/61722

 ArmVirtPkg/Library/KvmtoolPlatformPeiLib/KvmtoolPlatformPeiLib.c   | 78 ++++++++++++++++++++
 ArmVirtPkg/Library/KvmtoolPlatformPeiLib/KvmtoolPlatformPeiLib.inf | 48 ++++++++++++
 2 files changed, 126 insertions(+)

diff --git a/ArmVirtPkg/Library/KvmtoolPlatformPeiLib/KvmtoolPlatformPeiLib.c b/ArmVirtPkg/Library/KvmtoolPlatformPeiLib/KvmtoolPlatformPeiLib.c
new file mode 100644
index 0000000000000000000000000000000000000000..a97b31537fbc8071eed030f912ade60de3945356
--- /dev/null
+++ b/ArmVirtPkg/Library/KvmtoolPlatformPeiLib/KvmtoolPlatformPeiLib.c
@@ -0,0 +1,78 @@
+/** @file
+*
+*  Copyright (c) 2020, ARM Limited. All rights reserved.
+*
+*  SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+
+#include <PiPei.h>
+
+#include <Guid/Early16550UartBaseAddress.h>
+#include <Guid/FdtHob.h>
+
+#include <Library/MemoryAllocationLib.h>
+#include <Library/DebugLib.h>
+#include <Library/HobLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PeiServicesLib.h>
+#include <libfdt.h>
+
+/** Initialise Platform HOBs
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
+  @retval EFI_OUT_OF_RESOURCES    Out of resources.
+**/
+EFI_STATUS
+EFIAPI
+PlatformPeim (
+  VOID
+  )
+{
+  VOID    *Base;
+  VOID    *NewBase;
+  UINTN   FdtSize;
+  UINTN   FdtPages;
+  UINT64  *FdtHobData;
+  UINT64  *UartHobData;
+
+  Base = (VOID*)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
+  if ((Base == NULL) || (fdt_check_header (Base) != 0)) {
+    ASSERT (0);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  FdtSize = fdt_totalsize (Base) + PcdGet32 (PcdDeviceTreeAllocationPadding);
+  FdtPages = EFI_SIZE_TO_PAGES (FdtSize);
+  NewBase = AllocatePages (FdtPages);
+  if (NewBase == NULL) {
+    ASSERT (0);
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  fdt_open_into (Base, NewBase, EFI_PAGES_TO_SIZE (FdtPages));
+
+  FdtHobData = BuildGuidHob (&gFdtHobGuid, sizeof (*FdtHobData));
+  if (FdtHobData == NULL) {
+    ASSERT (0);
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  *FdtHobData = (UINTN)NewBase;
+
+  UartHobData = BuildGuidHob (
+                  &gEarly16550UartBaseAddressGuid,
+                  sizeof (*UartHobData)
+                  );
+  if (UartHobData == NULL) {
+    ASSERT (0);
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  *UartHobData = PcdGet64 (PcdSerialRegisterBase);
+
+  BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));
+
+  return EFI_SUCCESS;
+}
diff --git a/ArmVirtPkg/Library/KvmtoolPlatformPeiLib/KvmtoolPlatformPeiLib.inf b/ArmVirtPkg/Library/KvmtoolPlatformPeiLib/KvmtoolPlatformPeiLib.inf
new file mode 100644
index 0000000000000000000000000000000000000000..9f44b8885d3c131de1d41ac6947bd9218cfdf3e7
--- /dev/null
+++ b/ArmVirtPkg/Library/KvmtoolPlatformPeiLib/KvmtoolPlatformPeiLib.inf
@@ -0,0 +1,48 @@
+#/** @file
+#
+#  Copyright (c) 2020, ARM Limited. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x0001001B
+  BASE_NAME                      = PlatformPeiLib
+  FILE_GUID                      = 21073FB3-BA6F-43EB-83F0-4A840C648165
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = KvmtoolPlatformPeiLib
+
+[Sources]
+  KvmtoolPlatformPeiLib.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  ArmVirtPkg/ArmVirtPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  DebugLib
+  HobLib
+  FdtLib
+  PcdLib
+  PeiServicesLib
+
+[FixedPcd]
+  gArmTokenSpaceGuid.PcdFvSize
+  gArmVirtTokenSpaceGuid.PcdDeviceTreeAllocationPadding
+
+[Pcd]
+  gArmTokenSpaceGuid.PcdFvBaseAddress
+  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
+
+[Guids]
+  gFdtHobGuid
+  gEarly16550UartBaseAddressGuid
+
+[Depex]
+  gEfiPeiMemoryDiscoveredPpiGuid
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

* [PATCH v4 11/15] ArmVirtPkg: Support for kvmtool virtual platform
  2020-07-07 12:47 [PATCH v4 00/15] Kvmtool guest firmware support for Arm Sami Mujawar
                   ` (9 preceding siblings ...)
  2020-07-07 12:48 ` [PATCH v4 10/15] ArmVirtPkg: Add Kvmtool Platform Pei Lib Sami Mujawar
@ 2020-07-07 12:48 ` Sami Mujawar
  2020-07-07 13:27   ` Ard Biesheuvel
  2020-07-07 12:48 ` [PATCH v4 12/15] ArmVirtPkg: Package dependency for MC146818 RTC Sami Mujawar
                   ` (4 subsequent siblings)
  15 siblings, 1 reply; 25+ messages in thread
From: Sami Mujawar @ 2020-07-07 12:48 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ard.biesheuvel, leif, lersek, Alexandru.Elisei,
	Andre.Przywara, Matteo.Carlini, Laura.Moretta, nd

Kvmtool is a virtual machine manager that enables hosting
KVM guests. Kvmtool emulates certain devices like serial
port, RTC, etc. essentially providing a virtual platform.

This patch adds support for kvmtool virtual platform.

Following is a brief description of the firmware
implementation choices:

- Serial Port: 16550 UART
  On some platforms the 16550 UART is interfaced using
  PCI. Therefore, the 16550 Serial port library is
  dependent on the PCI library. The 16550 UART driver
  checks the Device ID represented using the PCD
  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialPciDeviceInfo
  to determine if the UART is behind PCI.
  If the Device ID is 0xFF then the serial 16550 UART
  is not behind PCI.

  On Kvmtool the Serial 16550 UART is not behind PCI,
  and therefore a combination of BasePciLibPciExpress
  and BasePciExpressLib is used to satisfy the PCI
  library dependency.

  The PcdSerialPciDeviceInfo is also set to 0xFF to
  indicate that the Serial 16550 UART is not behind
  PCI. The PCD PcdSerialUseMmio is also set to TRUE
  to indicate MMIO accesses are required for the
  UART registers.

  Additionally two instances of PlatformHookLibs are
  provided EarlyFdt16550SerialPortHookLib and
  Fdt16550SerialPortHookLib to patch the
  PcdSerialRegisterBase so that BaseSerialPortLib16550
  and retrieve the base address of the 16550 UART.

- Dependency order for Flash
  FaultTolerantWriteDxe makes use of PCDs (e.g.
  PcdFlashNvStorageFtwSpareBase64 etc.), which in
  case of kvmtool will be evaluated based on the CFI
  flash base address read from the DT. These variables
  are populated in the NorFlashPlatformLib loaded by
  ArmVeNorFlashDxe.

  This results in a dependency issue with
  FaultTolerantWriteDxe. To resolve this make the
  NorFlashPlatformLib as a library dependency for
  FaultTolerantWriteDxe.

- RTC Controller
  A separate patch updates the MC146818 RTC controller
  driver to support MMIO accesses.
  A KvmtoolRtcFdtClientLib has been introduced to
  extract the base addresses of the RTC controller
  from the platform device tree and map the RTC
  register space as Runtime Memory.

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

Notes:
    v4:
      - Updated to use XIPFLAGS to set no-jump-tables. Removed     [Sami]
        Networking and EBC support in this patch series. These
        features will be added later as separate patches.
      - Use XIPFLAGS to set no-jump-tables and use .inc files      [Ard]
        for NetworkPkg modules.
        Ref: https://edk2.groups.io/g/devel/topic/75081494
    
    v3:
      - Use correct terminology. kvmtool is *not* an emulator.     [Ard]
      - Updated patch series to reflect correct terminology.       [Sami]
      - Don't define bits and pieces you cannot make use of.       [Ard]
      - Updated workspace to remove unsupported features.          [Sami]
        Ref: https://edk2.groups.io/g/devel/topic/74200917#59552
    
    v2:
      - Updates to reflect review comments and support             [Sami]
        for latest features emulated by kvmtool e.g. CFI.
    
    v1:
      - Add support for Kvmtool emulated platform                  [Sami]
      - Add more justification for platform and                    [Laszlo]
        document platform maintainer.
        Ref: https://edk2.groups.io/g/devel/topic/30915279#30693

 ArmVirtPkg/ArmVirtKvmTool.dsc | 321 ++++++++++++++++++++
 ArmVirtPkg/ArmVirtKvmTool.fdf | 234 ++++++++++++++
 2 files changed, 555 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
new file mode 100644
index 0000000000000000000000000000000000000000..bf008be50fcb96bbe4f559f0b165a7a555b4cc1f
--- /dev/null
+++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
@@ -0,0 +1,321 @@
+#  @file
+#  Workspace file for KVMTool virtual platform.
+#
+#  Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+
+################################################################################
+#
+# Defines Section - statements that will be processed to create a Makefile.
+#
+################################################################################
+[Defines]
+  PLATFORM_NAME                  = ArmVirtKvmTool
+  PLATFORM_GUID                  = 4CB2C61E-FA32-4130-8E37-54ABC71A1A43
+  PLATFORM_VERSION               = 0.1
+  DSC_SPECIFICATION              = 0x0001001B
+!ifdef $(EDK2_OUT_DIR)
+  OUTPUT_DIRECTORY               = $(EDK2_OUT_DIR)
+!else
+  OUTPUT_DIRECTORY               = Build/ArmVirtKvmTool-$(ARCH)
+!endif
+  SUPPORTED_ARCHITECTURES        = AARCH64|ARM
+  BUILD_TARGETS                  = DEBUG|RELEASE
+  SKUID_IDENTIFIER               = DEFAULT
+  FLASH_DEFINITION               = ArmVirtPkg/ArmVirtKvmTool.fdf
+
+!include ArmVirtPkg/ArmVirt.dsc.inc
+
+[LibraryClasses.common]
+  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
+  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
+
+  # Virtio Support
+  VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
+  VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
+
+  ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
+  ArmVirtMemInfoLib|ArmVirtPkg/Library/KvmtoolVirtMemInfoLib/KvmtoolVirtMemInfoLib.inf
+
+  TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
+  NorFlashPlatformLib|ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
+
+  CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
+
+  # BDS Libraries
+  UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
+  PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+  BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
+
+  CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
+  FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
+
+  FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
+
+  PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+  PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
+  PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
+
+  TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
+  AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
+
+  PlatformPeiLib|ArmVirtPkg/Library/KvmtoolPlatformPeiLib/KvmtoolPlatformPeiLib.inf
+
+  PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
+  PlatformHookLib|ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf
+  SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
+
+[LibraryClasses.common.SEC, LibraryClasses.common.PEI_CORE, LibraryClasses.common.PEIM]
+  PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
+  PlatformHookLib|ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.inf
+  SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
+
+[LibraryClasses.common.UEFI_DRIVER]
+  UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
+
+[BuildOptions]
+  *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
+  #
+  # We need to avoid jump tables in SEC and BASE modules, so that the PE/COFF
+  # self-relocation code itself is guaranteed to be position independent.
+  #
+  GCC:*_*_*_CC_XIPFLAGS = -fno-jump-tables
+
+################################################################################
+#
+# Pcd Section - list of all EDK II PCD Entries defined by this Platform
+#
+################################################################################
+
+[PcdsFeatureFlag.common]
+  ## If TRUE, Graphics Output Protocol will be installed on virtual handle created by ConsplitterDxe.
+  #  It could be set FALSE to save size.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
+
+  # Use MMIO for accessing RTC controller registers.
+  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|TRUE
+
+[PcdsFixedAtBuild.common]
+  gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000000F
+
+  gArmPlatformTokenSpaceGuid.PcdCoreCount|1
+
+!if $(ARCH) == AARCH64
+  gArmTokenSpaceGuid.PcdVFPEnabled|1
+!endif
+
+  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
+
+  # Size of the region used by UEFI in permanent memory (Reserved 64MB)
+  gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x04000000
+
+  #
+  # TTY Terminal Type
+  # 0-PCANSI, 1-VT100, 2-VT00+, 3-UTF8, 4-TTYTERM
+  gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4
+
+  #
+  # ARM Virtual Architectural Timer -- fetch frequency from KVM
+  #
+  gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|0
+
+  # Use MMIO for accessing Serial port registers.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialPciDeviceInfo|{0xFF}
+
+  gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
+
+  #
+  # The maximum physical I/O addressability of the processor, set with
+  # BuildCpuHob().
+  #
+  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
+
+[PcdsPatchableInModule.common]
+  #
+  # This will be overridden in the code
+  #
+  gArmTokenSpaceGuid.PcdSystemMemoryBase|0x0
+  gArmTokenSpaceGuid.PcdSystemMemorySize|0x0
+
+  #
+  # The device tree base address is handed off by kvmtool.
+  # We are booting from RAM using the Linux kernel boot protocol,
+  # x0 will point to the DTB image in memory.
+  #
+  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress|0x0
+
+  gArmTokenSpaceGuid.PcdFdBaseAddress|0x0
+  gArmTokenSpaceGuid.PcdFvBaseAddress|0x0
+
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x0
+
+[PcdsDynamicDefault.common]
+  gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
+
+  gArmTokenSpaceGuid.PcdArmArchTimerSecIntrNum|0x0
+  gArmTokenSpaceGuid.PcdArmArchTimerIntrNum|0x0
+  gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum|0x0
+  gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum|0x0
+
+  #
+  # ARM General Interrupt Controller
+  #
+  gArmTokenSpaceGuid.PcdGicDistributorBase|0x0
+  gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x0
+  gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0x0
+
+  #
+  # Set video resolution for boot options and for text setup.
+  # PlatformDxe can set the former at runtime.
+  #
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|800
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|600
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoHorizontalResolution|640
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoVerticalResolution|480
+
+  ## Force DTB
+  gArmVirtTokenSpaceGuid.PcdForceNoAcpi|TRUE
+
+  # Setup Flash storage variables
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x40000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x40000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x40000
+
+  ## RTC Register address in MMIO space.
+  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0
+  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64|0x0
+
+################################################################################
+#
+# Components Section - list of all EDK II Modules needed by this Platform
+#
+################################################################################
+[Components.common]
+  #
+  # PEI Phase modules
+  #
+  ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf {
+    <LibraryClasses>
+      ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf
+      LzmaDecompressLib|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
+      PrePiLib|EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
+      HobLib|EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
+      PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
+      MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
+  }
+
+  #
+  # DXE
+  #
+  MdeModulePkg/Core/Dxe/DxeMain.inf {
+    <LibraryClasses>
+      NULL|MdeModulePkg/Library/DxeCrc32GuidedSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf
+      DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
+  }
+  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
+    <LibraryClasses>
+      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  }
+
+  #
+  # Architectural Protocols
+  #
+  ArmPkg/Drivers/CpuDxe/CpuDxe.inf
+  MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
+  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
+    <LibraryClasses>
+      NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
+      NULL|EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
+      BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
+  }
+
+  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
+  MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
+  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf {
+    <LibraryClasses>
+      NULL|ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
+  }
+
+  MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
+  MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
+  MdeModulePkg/Universal/Metronome/Metronome.inf
+  PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf {
+    <LibraryClasses>
+      NULL|ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf
+  }
+
+  MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
+  MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
+  MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
+  MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
+  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
+
+  MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
+
+  ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
+  ArmPkg/Drivers/TimerDxe/TimerDxe.inf {
+    <LibraryClasses>
+      NULL|ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.inf
+  }
+
+  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
+
+  MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
+
+  #
+  # Platform Driver
+  #
+  ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
+  ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf
+  ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
+  ArmVirtPkg/HighMemDxe/HighMemDxe.inf
+  OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
+  OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
+  OvmfPkg/VirtioNetDxe/VirtioNet.inf
+  OvmfPkg/VirtioRngDxe/VirtioRng.inf
+
+  #
+  # FAT filesystem + GPT/MBR partitioning + UDF filesystem
+  #
+  MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
+  MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
+  MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
+  FatPkg/EnhancedFatDxe/Fat.inf
+  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
+
+  #
+  # Bds
+  #
+  MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf {
+    <LibraryClasses>
+      DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
+      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  }
+  MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
+  MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
+  MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
+  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+  MdeModulePkg/Logo/LogoDxe.inf
+  MdeModulePkg/Application/UiApp/UiApp.inf {
+    <LibraryClasses>
+      NULL|MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf
+      NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf
+      NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
+  }
+
+  #
+  # SCSI Bus and Disk Driver
+  #
+  MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
+  MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
diff --git a/ArmVirtPkg/ArmVirtKvmTool.fdf b/ArmVirtPkg/ArmVirtKvmTool.fdf
new file mode 100644
index 0000000000000000000000000000000000000000..ebc82c5bd53cf98bc829c57dc0946b7a5854ca6c
--- /dev/null
+++ b/ArmVirtPkg/ArmVirtKvmTool.fdf
@@ -0,0 +1,234 @@
+#
+#  Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+
+################################################################################
+#
+# FD Section
+# The [FD] Section is made up of the definition statements and a
+# description of what goes into  the Flash Device Image.  Each FD section
+# defines one flash "device" image.  A flash device image may be one of
+# the following: Removable media bootable image (like a boot floppy
+# image,) an Option ROM image (that would be "flashed" into an add-in
+# card,) a System "Flash"  image (that would be burned into a system's
+# flash) or an Update ("Capsule") image that will be used to update and
+# existing system flash.
+#
+################################################################################
+
+[FD.KVMTOOL_EFI]
+BaseAddress   = 0x00000000|gArmTokenSpaceGuid.PcdFdBaseAddress
+# The size in bytes of the FLASH Device
+Size          = 0x00200000|gArmTokenSpaceGuid.PcdFdSize
+ErasePolarity = 1
+
+# This one is tricky, it must be: BlockSize * NumBlocks = Size
+BlockSize     = 0x00001000
+NumBlocks     = 0x200
+
+################################################################################
+#
+# Following are lists of FD Region layout which correspond to the locations of different
+# images within the flash device.
+#
+# Regions must be defined in ascending order and may not overlap.
+#
+# A Layout Region start with a eight digit hex offset (leading "0x" required) followed by
+# the pipe "|" character, followed by the size of the region, also in hex with the leading
+# "0x" characters. Like:
+# Offset|Size
+# PcdOffsetCName|PcdSizeCName
+# RegionType <FV, DATA, or FILE>
+#
+################################################################################
+
+#
+# Implement the Linux kernel header layout so that the loader will identify
+# it as something bootable, and execute it with a FDT pointer in x0 or r2.
+# This area will be reused to store a copy of the FDT so round it up to 32 KB.
+#
+0x00000000|0x00008000
+DATA = {
+!if $(ARCH) == AARCH64
+  0x01, 0x00, 0x00, 0x10,                         # code0: adr x1, .
+  0xff, 0x1f, 0x00, 0x14,                         # code1: b 0x8000
+  0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, # text_offset: 512 KB
+  0x00, 0x00, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, # image_size: 2 MB
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, # flags
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, # res2
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, # res3
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, # res4
+  0x41, 0x52, 0x4d, 0x64,                         # magic: "ARM\x64"
+  0x00, 0x00, 0x00, 0x00                          # res5
+!else
+  0x08, 0x10, 0x4f, 0xe2, # adr r1, .
+  0x02, 0x00, 0xa0, 0xe1, # mov r0, r2 (DTB)
+  0x00, 0x00, 0xa0, 0xe1, # nop
+  0x00, 0x00, 0xa0, 0xe1, # nop
+  0x00, 0x00, 0xa0, 0xe1, # nop
+  0x00, 0x00, 0xa0, 0xe1, # nop
+  0x00, 0x00, 0xa0, 0xe1, # nop
+  0x00, 0x00, 0xa0, 0xe1, # nop
+
+  0xf6, 0x1f, 0x00, 0xea, # b 0x8000
+  0x18, 0x28, 0x6f, 0x01, # magic
+  0x00, 0x00, 0x00, 0x00, # start
+  0x00, 0x00, 0x20, 0x00, # image size: 2 MB
+  0x01, 0x02, 0x03, 0x04  # endiannness flag
+!endif
+}
+
+0x00008000|0x001f8000
+gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
+FV = FVMAIN_COMPACT
+
+################################################################################
+#
+# FV Section
+#
+# [FV] section is used to define what components or modules are placed within a flash
+# device file.  This section also defines order the components and modules are positioned
+# within the image.  The [FV] section consists of define statements, set statements and
+# module statements.
+#
+################################################################################
+
+[FV.FvMain]
+FvNameGuid         = 8A91C08E-7D9D-4933-84D6-901D26D0766E
+BlockSize          = 0x40
+NumBlocks          = 0         # This FV gets compressed so make it just big enough
+FvAlignment        = 16        # FV alignment and FV attributes setting.
+ERASE_POLARITY     = 1
+MEMORY_MAPPED      = TRUE
+STICKY_WRITE       = TRUE
+LOCK_CAP           = TRUE
+LOCK_STATUS        = TRUE
+WRITE_DISABLED_CAP = TRUE
+WRITE_ENABLED_CAP  = TRUE
+WRITE_STATUS       = TRUE
+WRITE_LOCK_CAP     = TRUE
+WRITE_LOCK_STATUS  = TRUE
+READ_DISABLED_CAP  = TRUE
+READ_ENABLED_CAP   = TRUE
+READ_STATUS        = TRUE
+READ_LOCK_CAP      = TRUE
+READ_LOCK_STATUS   = TRUE
+
+  INF MdeModulePkg/Core/Dxe/DxeMain.inf
+  INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
+  INF ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf
+  INF ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
+  INF ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
+  INF ArmVirtPkg/HighMemDxe/HighMemDxe.inf
+
+  #
+  # PI DXE Drivers producing Architectural Protocols (EFI Services)
+  #
+  INF ArmPkg/Drivers/CpuDxe/CpuDxe.inf
+  INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
+  INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
+  INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
+  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
+  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+
+  INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
+  INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
+
+  INF  MdeModulePkg/Universal/Metronome/Metronome.inf
+  INF  PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
+
+  INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
+
+  #
+  # Multiple Console IO support
+  #
+  INF MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
+  INF MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
+  INF MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
+  INF MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
+  INF MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
+
+  INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
+  INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf
+  INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
+  INF ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
+
+  #
+  # FAT filesystem + GPT/MBR partitioning + UDF filesystem
+  #
+  INF MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
+  INF MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
+  INF FatPkg/EnhancedFatDxe/Fat.inf
+  INF MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
+  INF MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
+
+  #
+  # Platform Driver
+  #
+  INF OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
+  INF OvmfPkg/VirtioNetDxe/VirtioNet.inf
+  INF OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
+  INF OvmfPkg/VirtioRngDxe/VirtioRng.inf
+
+  #
+  # UEFI application (Shell Embedded Boot Loader)
+  #
+  INF ShellPkg/Application/Shell/Shell.inf
+  INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
+
+  #
+  # Bds
+  #
+  INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
+  INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
+  INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
+  INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
+  INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+  INF MdeModulePkg/Application/UiApp/UiApp.inf
+
+  #
+  # SCSI Bus and Disk Driver
+  #
+  INF MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
+  INF MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
+
+  #
+  # TianoCore logo (splash screen)
+  #
+  INF MdeModulePkg/Logo/LogoDxe.inf
+
+  #
+  # Ramdisk support
+  #
+  INF MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
+
+[FV.FVMAIN_COMPACT]
+FvAlignment        = 16
+ERASE_POLARITY     = 1
+MEMORY_MAPPED      = TRUE
+STICKY_WRITE       = TRUE
+LOCK_CAP           = TRUE
+LOCK_STATUS        = TRUE
+WRITE_DISABLED_CAP = TRUE
+WRITE_ENABLED_CAP  = TRUE
+WRITE_STATUS       = TRUE
+WRITE_LOCK_CAP     = TRUE
+WRITE_LOCK_STATUS  = TRUE
+READ_DISABLED_CAP  = TRUE
+READ_ENABLED_CAP   = TRUE
+READ_STATUS        = TRUE
+READ_LOCK_CAP      = TRUE
+READ_LOCK_STATUS   = TRUE
+
+  INF RuleOverride = SELF_RELOC ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
+
+  FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
+    SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
+      SECTION FV_IMAGE = FVMAIN
+    }
+  }
+
+!include ArmVirtRules.fdf.inc
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

* [PATCH v4 12/15] ArmVirtPkg: Package dependency for MC146818 RTC
  2020-07-07 12:47 [PATCH v4 00/15] Kvmtool guest firmware support for Arm Sami Mujawar
                   ` (10 preceding siblings ...)
  2020-07-07 12:48 ` [PATCH v4 11/15] ArmVirtPkg: Support for kvmtool virtual platform Sami Mujawar
@ 2020-07-07 12:48 ` Sami Mujawar
  2020-07-07 12:48 ` [PATCH v4 13/15] ArmVirtPkg: Add kvmtool to package dictionary Sami Mujawar
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Sami Mujawar @ 2020-07-07 12:48 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ard.biesheuvel, leif, lersek, Matteo.Carlini,
	Laura.Moretta, nd

Kvmtool emulates a MC146818 RTC controller in the
MMIO space. To support this the MC146818 RTC driver
PcatRealTimeClockRuntimeDxe has been updated to
support MMIO accesses. PCDs for RTC Index and
Target register base addresses in the MMIO space
have been introduced. The KvmtoolRtcFdtClientLib
reads the MC146818 RTC MMIO base address region
from the Kvmtool device tree and updates the
Index and Target register PCDs.

As these PCDs are defined in PcAtChipsetPkg.dec,
this patch updates the CI script to add this
dependency.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Acked-by: Ard Biesheuvel <Ard.Biesheuvel@arm.com>
---

Notes:
    v4:
      - No code changes, resending patch with v4 series.        [Sami]
        Ref: https://edk2.groups.io/g/devel/message/61724

 ArmVirtPkg/ArmVirtPkg.ci.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtPkg.ci.yaml b/ArmVirtPkg/ArmVirtPkg.ci.yaml
index 4553725ee5285d08a25f5e736f6b1cd25f4ba3e9..554ffb2609b25b46caa1fee8e994e2e6ea51f18f 100644
--- a/ArmVirtPkg/ArmVirtPkg.ci.yaml
+++ b/ArmVirtPkg/ArmVirtPkg.ci.yaml
@@ -5,6 +5,8 @@
 # used for code analysis.
 #
 # Copyright (c) Microsoft Corporation
+# Copyright (c) 2020, ARM Limited. All rights reserved.
+#
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
 {
@@ -34,6 +36,7 @@
             "OvmfPkg/OvmfPkg.dec",
             "EmbeddedPkg/EmbeddedPkg.dec",
             "ArmPlatformPkg/ArmPlatformPkg.dec",
+            "PcAtChipsetPkg/PcAtChipsetPkg.dec",
             "SecurityPkg/SecurityPkg.dec",
             "ShellPkg/ShellPkg.dec"  #Is this ok?
         ],
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

* [PATCH v4 13/15] ArmVirtPkg: Add kvmtool to package dictionary
  2020-07-07 12:47 [PATCH v4 00/15] Kvmtool guest firmware support for Arm Sami Mujawar
                   ` (11 preceding siblings ...)
  2020-07-07 12:48 ` [PATCH v4 12/15] ArmVirtPkg: Package dependency for MC146818 RTC Sami Mujawar
@ 2020-07-07 12:48 ` Sami Mujawar
  2020-07-07 12:48 ` [PATCH v4 14/15] .python/SpellCheck: Add 'XIPFLAGS' to "words" section Sami Mujawar
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Sami Mujawar @ 2020-07-07 12:48 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ard.biesheuvel, leif, lersek, Matteo.Carlini,
	Laura.Moretta, nd

Kvmtool is a virtual machine manager that can be used
to launch guest VMs. Support for Kvmtool virtual
platform has been added to ArmVirtPkg.

Add kvmtool to the ArmVirtPkg dictionary to prevent
the CI Spell check plugin from failing.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Acked-by: Ard Biesheuvel <Ard.Biesheuvel@arm.com>
---

Notes:
    v4:
      - No code change, resending patch with v4 series.         [Sami]
        Ref: https://edk2.groups.io/g/devel/message/61725

 ArmVirtPkg/ArmVirtPkg.ci.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirtPkg.ci.yaml b/ArmVirtPkg/ArmVirtPkg.ci.yaml
index 554ffb2609b25b46caa1fee8e994e2e6ea51f18f..cbe66f82de484dfbec8c81261d98d3ff647193ae 100644
--- a/ArmVirtPkg/ArmVirtPkg.ci.yaml
+++ b/ArmVirtPkg/ArmVirtPkg.ci.yaml
@@ -98,7 +98,8 @@
             "unbootable",
             "virt's",
             "werror",
-            "xenio"
+            "xenio",
+            "kvmtool"
         ],           # words to extend to the dictionary for this package
         "IgnoreStandardPaths": [],   # Standard Plugin defined paths that should be ignore
         "AdditionalIncludePaths": [] # Additional paths to spell check (wildcards supported)
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

* [PATCH v4 14/15] .python/SpellCheck: Add 'XIPFLAGS' to "words" section
  2020-07-07 12:47 [PATCH v4 00/15] Kvmtool guest firmware support for Arm Sami Mujawar
                   ` (12 preceding siblings ...)
  2020-07-07 12:48 ` [PATCH v4 13/15] ArmVirtPkg: Add kvmtool to package dictionary Sami Mujawar
@ 2020-07-07 12:48 ` Sami Mujawar
  2020-07-07 13:28   ` Ard Biesheuvel
  2020-07-07 12:48 ` [PATCH v4 15/15] Maintainer.txt: Add Kvmtool platform reviewer Sami Mujawar
       [not found] ` <161F794363CF42EE.6803@groups.io>
  15 siblings, 1 reply; 25+ messages in thread
From: Sami Mujawar @ 2020-07-07 12:48 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ard.biesheuvel, leif, lersek, Matteo.Carlini,
	Laura.Moretta, nd

The EDKII Core CI reports spelling error for XIPFLAGS. The
XIPFLAGS are typically used to specify XIP options to the
compiler. e.g. GCC:*_*_*_CC_XIPFLAGS = -fno-jump-tables

Add 'XIPFLAGS' to "words" section in cspell.base.yaml file
to avoid spelling check error.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
 .pytool/Plugin/SpellCheck/cspell.base.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.pytool/Plugin/SpellCheck/cspell.base.yaml b/.pytool/Plugin/SpellCheck/cspell.base.yaml
index 1ce5f6991104b83e41e769299b23cb3fa74c46e9..ad6f7ac06ad18efcd4ab9c2346356dfd6885d827 100644
--- a/.pytool/Plugin/SpellCheck/cspell.base.yaml
+++ b/.pytool/Plugin/SpellCheck/cspell.base.yaml
@@ -178,5 +178,6 @@
         "vcruntimed",
         "ucrtd",
         "msvcrtd",
+        "XIPFLAGS"
     ]
 }
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

* [PATCH v4 15/15] Maintainer.txt: Add Kvmtool platform reviewer
  2020-07-07 12:47 [PATCH v4 00/15] Kvmtool guest firmware support for Arm Sami Mujawar
                   ` (13 preceding siblings ...)
  2020-07-07 12:48 ` [PATCH v4 14/15] .python/SpellCheck: Add 'XIPFLAGS' to "words" section Sami Mujawar
@ 2020-07-07 12:48 ` Sami Mujawar
       [not found] ` <161F794363CF42EE.6803@groups.io>
  15 siblings, 0 replies; 25+ messages in thread
From: Sami Mujawar @ 2020-07-07 12:48 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ard.biesheuvel, leif, michael.d.kinney,
	Matteo.Carlini, Laura.Moretta, nd

Kvmtool is a virtual machine manager that can be used
to launch guest partitions. It additionally emulates
some hardware components e.g. RTC, CFI etc. essentially
providing a virtual platform for a guest operating
system (OS) to run.

A standards-based OS would need UEFI firmware support
for the Kvmtool virtual platform, for which additional
modules are added to ArmVirtPkg.
Adding myself as reviewer for these modules as
advised on mailing list discussion at
- https://edk2.groups.io/g/devel/topic/30915279#30693
- https://edk2.groups.io/g/devel/topic/74200911#59650

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

Notes:
    v4:
      - No change, resending patch with v4 series.                     [Sami]
        Ref: https://edk2.groups.io/g/devel/message/61726
    
    v3:
      - Updated Maintainers.txt to add reviewer for Kvmtool platform.  [Sami]
        Ref: https://edk2.groups.io/g/devel/topic/74200911#59650

 Maintainers.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index 32c9003a6209f38da4e422191a4ae327fd166dc0..2d9aefa6ed3181553ce908ddd85d6078fd5f829c 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -152,6 +152,16 @@ F: ArmVirtPkg/XenPlatformHasAcpiDtDxe/
 F: ArmVirtPkg/XenioFdtDxe/
 R: Julien Grall <julien@xen.org>
 
+ArmVirtPkg: Kvmtool emulated platform support
+F: ArmVirtPkg/ArmVirtKvmTool.*
+F: ArmVirtPkg/KvmtoolPlatformDxe/
+F: ArmVirtPkg/Library/Fdt16550SerialPortHookLib/
+F: ArmVirtPkg/Library/KvmtoolPlatformPeiLib/
+F: ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/
+F: ArmVirtPkg/Library/KvmtoolVirtMemInfoLib/
+F: ArmVirtPkg/Library/NorFlashKvmtoolLib/
+R: Sami Mujawar <sami.mujawar@arm.com>
+
 BaseTools
 F: BaseTools/
 W: https://github.com/tianocore/tianocore.github.io/wiki/BaseTools
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

* Re: [PATCH v4 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver
  2020-07-07 12:47 ` [PATCH v4 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver Sami Mujawar
@ 2020-07-07 13:16   ` Ard Biesheuvel
  2020-07-09 11:37     ` Sami Mujawar
  0 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2020-07-07 13:16 UTC (permalink / raw)
  To: Sami Mujawar, devel
  Cc: leif, ray.ni, Alexandru.Elisei, Andre.Przywara, Matteo.Carlini,
	Laura.Moretta, nd

On 7/7/20 3:47 PM, Sami Mujawar wrote:
> Some virtual machine managers like Kvmtool emulate the MC146818
> RTC controller in the MMIO space so that architectures that do
> not support I/O Mapped I/O can use the RTC. This patch adds MMIO
> support to the RTC controller driver.
> 
> The PCD PcdRtcUseMmio has been added to select I/O or MMIO support.
>    If PcdRtcUseMmio is:
>      TRUE  - Indicates the RTC port registers are in MMIO space.
>      FALSE - Indicates the RTC port registers are in I/O space.
>              Default is I/O space.
> 
> Additionally two new PCDs PcdRtcIndexRegister64 and
> PcdRtcTargetRegister64 have been introduced to provide the base
> address for the RTC registers in the MMIO space.
> 
> When MMIO support is selected (PcdRtcUseMmio == TRUE) the driver
> converts the pointers to the RTC MMIO registers so that the
> RTC registers are accessible post ExitBootServices.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>

This code looks good to me now, but please drop the unnecessary 
whitespace and header changes to PcRtc.h, and the whitespace changes to 
PcRtcEntry.c

With that,

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>


> ---
> 
> Notes:
>      v4:
>        - Updated based on review comments.                             [Sami]
>        - Use static helper functions instead of function pointers.     [Ard]
>          Ref: https://edk2.groups.io/g/devel/topic/75081468
>      
>      v3:
>        - Make PcdRtcUseMmio a feature PCD.                             [Sami]
>        - Read the RTC MMIO base address from the DT.                   [Andre]
>        - Introduce PCDs for RTC Index and Target register base         [Sami]
>          address in the MMIO space.
>        - Move RTC MMIO region mapping code to a separate platform      [Sami]
>          specific library. This library also reads the base addresses
>          for the RTC from DT and configures the RTC Index and Target
>          register PCDs.
>          Ref: https://edk2.groups.io/g/devel/topic/74200905#60307
>      
>      v2:
>        - Code review comments incorporated.                            [Sami]
>      
>      v1:
>        - Add support to read/write from RTC registers using            [Sami]
>          MMIO access
>        - Use wrapper functions for RtcRead/Write accessors             [Leif]
>          Ref: https://edk2.groups.io/g/devel/topic/30915281#30695
> 
>   PcAtChipsetPkg/PcAtChipsetPkg.dec                                          |  16 +++
>   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c                         | 120 ++++++++++++++++++--
>   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h                         |   2 +
>   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c                    |  58 +++++++++-
>   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf |   8 ++
>   5 files changed, 190 insertions(+), 14 deletions(-)
> 
> diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dec b/PcAtChipsetPkg/PcAtChipsetPkg.dec
> index 88de5cceea593176c3a2425a5963b66b789f2b9e..ed2d95550b8d153995b30cdc290cf3bb905e211b 100644
> --- a/PcAtChipsetPkg/PcAtChipsetPkg.dec
> +++ b/PcAtChipsetPkg/PcAtChipsetPkg.dec
> @@ -6,6 +6,7 @@
>   #
>   # Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
>   # Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> +# Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR>
>   #
>   # SPDX-License-Identifier: BSD-2-Clause-Patent
>   #
> @@ -41,6 +42,13 @@ [PcdsFeatureFlag]
>     # @Prompt Configure HPET to use MSI.
>     gPcAtChipsetPkgTokenSpaceGuid.PcdHpetMsiEnable|TRUE|BOOLEAN|0x00001000
>   
> +  ## Indicates the RTC port registers are in MMIO space, or in I/O space.
> +  #  Default is I/O space.<BR><BR>
> +  #   TRUE  - RTC port registers are in MMIO space.<BR>
> +  #   FALSE - RTC port registers are in I/O space.<BR>
> +  # @Prompt RTC port registers use MMIO.
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|FALSE|BOOLEAN|0x00000021
> +
>   [PcdsFixedAtBuild, PcdsDynamic, PcdsDynamicEx, PcdsPatchableInModule]
>     ## This PCD specifies the base address of the HPET timer.
>     # @Prompt HPET base address.
> @@ -68,6 +76,14 @@ [PcdsFixedAtBuild, PcdsDynamic, PcdsDynamicEx, PcdsPatchableInModule]
>     # @Expression 0x80000001 | gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear < gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear + 100
>     gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2097|UINT16|0x0000000E
>   
> +  ## Specifies RTC Index Register address in MMIO space.
> +  # @Prompt RTC Index Register address
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0|UINT64|0x00000022
> +
> +  ## Specifies RTC Target Register address in MMIO space.
> +  # @Prompt RTC Target Register address
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64|0x0|UINT64|0x00000023
> +
>   [PcdsFixedAtBuild, PcdsPatchableInModule]
>     ## Defines the ACPI register set base address.
>     #  The invalid 0xFFFF is as its default value. It must be configured to the real value.
> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> index 52af17941786ef81c3911512ee64551724e67209..64f36f6fbbd1b03967bd1a1290d108d5b0f294fa 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> @@ -3,6 +3,7 @@
>   
>   Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
>   Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> +Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR>
>   
>   SPDX-License-Identifier: BSD-2-Clause-Patent
>   
> @@ -10,6 +11,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>   
>   #include "PcRtc.h"
>   
> +extern UINTN  mRtcIndexRegister;
> +extern UINTN  mRtcTargetRegister;
> +
>   //
>   // Days of month.
>   //
> @@ -54,38 +58,132 @@ IsWithinOneDay (
>     );
>   
>   /**
> +  Read RTC content through its registers using IO access.
> +
> +  @param  Address   Address offset of RTC. It is recommended to use
> +                    macros such as RTC_ADDRESS_SECONDS.
> +
> +  @return The data of UINT8 type read from RTC.
> +**/
> +STATIC
> +UINT8
> +IoRtcRead (
> +  IN  UINTN Address
> +  )
> +{
> +  IoWrite8 (
> +    PcdGet8 (PcdRtcIndexRegister),
> +    (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80))
> +    );
> +  return IoRead8 (PcdGet8 (PcdRtcTargetRegister));
> +}
> +
> +/**
> +  Write RTC through its registers  using IO access.
> +
> +  @param  Address   Address offset of RTC. It is recommended to use
> +                    macros such as RTC_ADDRESS_SECONDS.
> +  @param  Data      The content you want to write into RTC.
> +
> +**/
> +STATIC
> +VOID
> +IoRtcWrite (
> +  IN  UINTN   Address,
> +  IN  UINT8   Data
> +  )
> +{
> +  IoWrite8 (
> +    PcdGet8 (PcdRtcIndexRegister),
> +    (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80))
> +    );
> +  IoWrite8 (PcdGet8 (PcdRtcTargetRegister), Data);
> +}
> +
> +/**
> +  Read RTC content through its registers using MMIO access.
> +
> +  @param  Address   Address offset of RTC. It is recommended to use
> +                    macros such as RTC_ADDRESS_SECONDS.
> +
> +  @return The data of UINT8 type read from RTC.
> +**/
> +STATIC
> +UINT8
> +MmioRtcRead (
> +  IN  UINTN Address
> +  )
> +{
> +  MmioWrite8 (
> +    mRtcIndexRegister,
> +    (UINT8)(Address | (UINT8)(MmioRead8 (mRtcIndexRegister) & 0x80))
> +    );
> +  return MmioRead8 (mRtcTargetRegister);
> +}
> +
> +/**
> +  Write RTC through its registers using MMIO access.
> +
> +  @param  Address   Address offset of RTC. It is recommended to use
> +                    macros such as RTC_ADDRESS_SECONDS.
> +  @param  Data      The content you want to write into RTC.
> +
> +**/
> +STATIC
> +VOID
> +MmioRtcWrite (
> +  IN  UINTN   Address,
> +  IN  UINT8   Data
> +  )
> +{
> +  MmioWrite8 (
> +    mRtcIndexRegister,
> +    (UINT8)(Address | (UINT8)(MmioRead8 (mRtcIndexRegister) & 0x80))
> +    );
> +  MmioWrite8 (mRtcTargetRegister, Data);
> +}
> +
> +/**
>     Read RTC content through its registers.
>   
> -  @param  Address  Address offset of RTC. It is recommended to use macros such as
> -                   RTC_ADDRESS_SECONDS.
> +  @param  Address   Address offset of RTC. It is recommended to use
> +                    macros such as RTC_ADDRESS_SECONDS.
>   
>     @return The data of UINT8 type read from RTC.
>   **/
> +STATIC
>   UINT8
>   RtcRead (
> -  IN  UINT8 Address
> +  IN  UINTN Address
>     )
>   {
> -  IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8) (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
> -  return IoRead8 (PcdGet8 (PcdRtcTargetRegister));
> +  if (FeaturePcdGet (PcdRtcUseMmio)) {
> +    return MmioRtcRead (Address);
> +  }
> +
> +  return IoRtcRead (Address);
>   }
>   
>   /**
>     Write RTC through its registers.
>   
> -  @param  Address  Address offset of RTC. It is recommended to use macros such as
> -                   RTC_ADDRESS_SECONDS.
> -  @param  Data     The content you want to write into RTC.
> +  @param  Address   Address offset of RTC. It is recommended to use
> +                    macros such as RTC_ADDRESS_SECONDS.
> +  @param  Data      The content you want to write into RTC.
>   
>   **/
> +STATIC
>   VOID
>   RtcWrite (
> -  IN  UINT8   Address,
> +  IN  UINTN   Address,
>     IN  UINT8   Data
>     )
>   {
> -  IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8) (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
> -  IoWrite8 (PcdGet8 (PcdRtcTargetRegister), Data);
> +  if (FeaturePcdGet (PcdRtcUseMmio)) {
> +    MmioRtcWrite (Address, Data);
> +  } else {
> +    IoRtcWrite (Address, Data);
> +  }
>   }
>   
>   /**
> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h
> index 47293ce44c5a1f4792892892f7da40d7f0a5a001..c1cb9466b74587233de4e78c0c0d537ca6b66cf3 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h
> @@ -3,6 +3,7 @@
>   
>   Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
>   Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> +Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.<BR>
>   
>   SPDX-License-Identifier: BSD-2-Clause-Patent
>   
> @@ -371,4 +372,5 @@ PcRtcAcpiTableChangeCallback (
>     IN EFI_EVENT        Event,
>     IN VOID             *Context
>     );
> +
>   #endif
> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> index ccda6331373bfe4069b0a59495b5e5cc731c8fc8..9c5f1ff169c80d1a9580d90336f162968b63bfa2 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> @@ -2,16 +2,23 @@
>     Provides Set/Get time operations.
>   
>   Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR>
>   SPDX-License-Identifier: BSD-2-Clause-Patent
>   
>   **/
>   
> +#include <Library/DxeServicesTableLib.h>
>   #include "PcRtc.h"
>   
>   PC_RTC_MODULE_GLOBALS  mModuleGlobal;
>   
>   EFI_HANDLE             mHandle = NULL;
>   
> +STATIC EFI_EVENT       mVirtualAddrChangeEvent;
> +
> +UINTN                  mRtcIndexRegister;
> +UINTN                  mRtcTargetRegister;
> +
>   /**
>     Returns the current time and date information, and the time-keeping capabilities
>     of the hardware platform.
> @@ -106,6 +113,30 @@ PcRtcEfiSetWakeupTime (
>   }
>   
>   /**
> +  Fixup internal data so that EFI can be called in virtual mode.
> +  Call the passed in Child Notify event and convert any pointers in
> +  lib to virtual mode.
> +
> +  @param[in]    Event   The Event that is being processed
> +  @param[in]    Context Event Context
> +**/
> +VOID
> +EFIAPI
> +LibRtcVirtualNotifyEvent (
> +  IN EFI_EVENT        Event,
> +  IN VOID             *Context
> +  )
> +{
> +  // Only needed if you are going to support the OS calling RTC functions in
> +  // virtual mode. You will need to call EfiConvertPointer (). To convert any
> +  // stored physical addresses to virtual address. After the OS transitions to
> +  // calling in virtual mode, all future runtime calls will be made in virtual
> +  // mode.
> +  EfiConvertPointer (0x0, (VOID**)&mRtcIndexRegister);
> +  EfiConvertPointer (0x0, (VOID**)&mRtcTargetRegister);
> +}
> +
> +/**
>     The user Entry Point for PcRTC module.
>   
>     This is the entry point for PcRTC module. It installs the UEFI runtime service
> @@ -125,12 +156,17 @@ InitializePcRtc (
>     IN EFI_SYSTEM_TABLE                      *SystemTable
>     )
>   {
> -  EFI_STATUS  Status;
> -  EFI_EVENT   Event;
> +  EFI_STATUS             Status;
> +  EFI_EVENT              Event;
>   
>     EfiInitializeLock (&mModuleGlobal.RtcLock, TPL_CALLBACK);
>     mModuleGlobal.CenturyRtcAddress = GetCenturyRtcAddress ();
>   
> +  if (FeaturePcdGet (PcdRtcUseMmio)) {
> +    mRtcIndexRegister = (UINTN)PcdGet64 (PcdRtcIndexRegister64);
> +    mRtcTargetRegister = (UINTN)PcdGet64 (PcdRtcTargetRegister64);
> +  }
> +
>     Status = PcRtcInit (&mModuleGlobal);
>     ASSERT_EFI_ERROR (Status);
>   
> @@ -165,7 +201,23 @@ InitializePcRtc (
>                     NULL,
>                     NULL
>                     );
> -  ASSERT_EFI_ERROR (Status);
> +  if (EFI_ERROR (Status)) {
> +    ASSERT_EFI_ERROR (Status);
> +    return Status;
> +  }
> +
> +  if (FeaturePcdGet (PcdRtcUseMmio)) {
> +    // Register for the virtual address change event
> +    Status = gBS->CreateEventEx (
> +                    EVT_NOTIFY_SIGNAL,
> +                    TPL_NOTIFY,
> +                    LibRtcVirtualNotifyEvent,
> +                    NULL,
> +                    &gEfiEventVirtualAddressChangeGuid,
> +                    &mVirtualAddrChangeEvent
> +                    );
> +    ASSERT_EFI_ERROR (Status);
> +  }
>   
>     return Status;
>   }
> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
> index c73ee98105e510f9e4e23c1a6c1e5c505325d2c9..0d8eca28b65954b073a72fc4fe5ad6247320e79d 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
> @@ -6,6 +6,7 @@
>   #
>   # Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>   # Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> +# Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR>
>   #
>   # SPDX-License-Identifier: BSD-2-Clause-Patent
>   #
> @@ -61,6 +62,11 @@ [Guids]
>     ## SOMETIMES_CONSUMES ## SystemTable
>     gEfiAcpiTableGuid
>   
> +  gEfiEventVirtualAddressChangeGuid
> +
> +[FeaturePcd]
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio                   ## CONSUMES
> +
>   [FixedPcd]
>     gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterA     ## CONSUMES
>     gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterB     ## CONSUMES
> @@ -72,6 +78,8 @@ [Pcd]
>     gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear             ## CONSUMES
>     gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister             ## CONSUMES
>     gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister            ## CONSUMES
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64           ## CONSUMES
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64          ## CONSUMES
>   
>   [Depex]
>     gEfiVariableArchProtocolGuid AND gEfiVariableWriteArchProtocolGuid
> 


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

* Re: [PATCH v4 02/15] ArmVirtPkg: Add Kvmtool RTC Fdt Client Library
  2020-07-07 12:47 ` [PATCH v4 02/15] ArmVirtPkg: Add Kvmtool RTC Fdt Client Library Sami Mujawar
@ 2020-07-07 13:19   ` Ard Biesheuvel
  2020-07-09 11:34     ` [edk2-devel] " Sami Mujawar
  0 siblings, 1 reply; 25+ messages in thread
From: Ard Biesheuvel @ 2020-07-07 13:19 UTC (permalink / raw)
  To: Sami Mujawar, devel
  Cc: leif, lersek, Alexandru.Elisei, Andre.Przywara, Matteo.Carlini,
	Laura.Moretta, nd

On 7/7/20 3:47 PM, Sami Mujawar wrote:
> Add library that parses the Kvmtool device tree and updates
> the dynamic PCDs describing the RTC Memory map.
> 
> It also maps the MMIO region used by the RTC as runtime memory
> so that the RTC registers are accessible post ExitBootServices.
> 
> Since UEFI takes ownership of the RTC hardware disable the RTC
> node in the DT to prevent the OS from attaching its device
> driver as well.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> 
> Notes:
>      v4:
>        - Updated based on review comments.                             [Sami]
>        - Cleanup include file list, make local functions static,       [Ard]
>          drop EFIAPI, use EFI_PAGE_MASK and use ReadUnaligned64.
>          Ref: https://edk2.groups.io/g/devel/topic/75081472
>      

You haven't answered my question about adding and removing the GCD 
memory space.


>      v3:
>        - Introduce library to read and map the MMIO base addresses     [Sami]
>          for the RTC registers from the DT and configure the Index
>          and Target register PCDs.
> 
>   ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c   | 230 ++++++++++++++++++++
>   ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf |  42 ++++
>   2 files changed, 272 insertions(+)
> 
> diff --git a/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..93715958952f933007ff143a22cbe81606cc539e
> --- /dev/null
> +++ b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c
> @@ -0,0 +1,230 @@
> +/** @file
> +  FDT client library for motorola,mc146818 RTC driver
> +
> +  Copyright (c) 2020, ARM Limited. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DxeServicesTableLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Protocol/FdtClient.h>
> +
> +/** RTC Index register is at offset 0x0
> +*/
> +#define RTC_INDEX_REG_OFFSET    0x0ULL
> +
> +/** RTC Target register is at offset 0x1
> +*/
> +#define RTC_TARGET_REG_OFFSET   0x1ULL
> +
> +/** Add the RTC controller address range to the memory map.
> +
> +  @param [in]  ImageHandle  The handle to the image.
> +  @param [in]  RtcPageBase  Base address of the RTC controller.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
> +  @retval EFI_NOT_FOUND           Flash device not found.
> +**/
> +STATIC
> +EFI_STATUS
> +KvmtoolRtcMapMemory (
> +  IN EFI_HANDLE               ImageHandle,
> +  IN EFI_PHYSICAL_ADDRESS     RtcPageBase
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = gDS->AddMemorySpace (
> +                  EfiGcdMemoryTypeMemoryMappedIo,
> +                  RtcPageBase,
> +                  EFI_PAGE_SIZE,
> +                  EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR, "Failed to add memory space. Status = %r\n",
> +      Status
> +      ));
> +    return Status;
> +  }
> +
> +  Status = gDS->AllocateMemorySpace (
> +                  EfiGcdAllocateAddress,
> +                  EfiGcdMemoryTypeMemoryMappedIo,
> +                  0,
> +                  EFI_PAGE_SIZE,
> +                  &RtcPageBase,
> +                  ImageHandle,
> +                  NULL
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Failed to allocate memory space. Status = %r\n",
> +      Status
> +      ));
> +    gDS->RemoveMemorySpace (
> +           RtcPageBase,
> +           EFI_PAGE_SIZE
> +           );
> +    return Status;
> +  }
> +
> +  Status = gDS->SetMemorySpaceAttributes (
> +                  RtcPageBase,
> +                  EFI_PAGE_SIZE,
> +                  EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Failed to set memory attributes. Status = %r\n",
> +      Status
> +      ));
> +    gDS->FreeMemorySpace (
> +           RtcPageBase,
> +           EFI_PAGE_SIZE
> +           );
> +    gDS->RemoveMemorySpace (
> +           RtcPageBase,
> +           EFI_PAGE_SIZE
> +           );
> +  }
> +
> +  return Status;
> +}
> +
> +/** Entrypoint for KvmtoolRtcFdtClientLib.
> +
> +  Locate the RTC node in the DT and update the Index and
> +  Target register base addresses in the respective PCDs.
> +  Add the RTC memory region to the memory map.
> +  Disable the RTC node as the RTC is owned by UEFI.
> +
> +  @param [in]  ImageHandle  The handle to the image.
> +  @param [in]  SystemTable  Pointer to the System Table.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
> +  @retval EFI_NOT_FOUND           Flash device not found.
> +**/
> +EFI_STATUS
> +EFIAPI
> +KvmtoolRtcFdtClientLibConstructor (
> +  IN EFI_HANDLE         ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  EFI_STATUS            Status;
> +  FDT_CLIENT_PROTOCOL   *FdtClient;
> +  INT32                 Node;
> +  CONST UINT32          *Reg;
> +  UINT32                RegSize;
> +  UINT64                RegBase;
> +  UINT64                Range;
> +  RETURN_STATUS         PcdStatus;
> +
> +  Status = gBS->LocateProtocol (
> +                  &gFdtClientProtocolGuid,
> +                  NULL,
> +                  (VOID **)&FdtClient
> +                  );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  Status = FdtClient->FindCompatibleNode (
> +                        FdtClient,
> +                        "motorola,mc146818",
> +                        &Node
> +                        );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: No 'motorola,mc146818' compatible DT node found\n",
> +      __FUNCTION__
> +      ));
> +    return Status;
> +  }
> +
> +  Status = FdtClient->GetNodeProperty (
> +                        FdtClient,
> +                        Node,
> +                        "reg",
> +                        (CONST VOID **)&Reg,
> +                        &RegSize
> +                        );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: No 'reg' property found in 'motorola,mc146818' compatible DT node\n",
> +      __FUNCTION__
> +      ));
> +    return Status;
> +  }
> +
> +  ASSERT (RegSize == 16);
> +
> +  RegBase = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[0]));
> +  Range = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[2]));
> +  DEBUG ((
> +    DEBUG_INFO,
> +    "Found motorola,mc146818 RTC @ 0x%Lx Range = 0x%x\n",
> +    RegBase,
> +    Range
> +    ));
> +
> +  // The address range must cover the RTC Index and the Target registers.
> +  ASSERT (Range >= 0x2);
> +
> +  // RTC Index register is at offset 0x0
> +  PcdStatus = PcdSet64S (
> +                PcdRtcIndexRegister64,
> +                (RegBase + RTC_INDEX_REG_OFFSET)
> +                );
> +  ASSERT_RETURN_ERROR (PcdStatus);
> +
> +  // RTC Target register is at offset 0x1
> +  PcdStatus = PcdSet64S (
> +                PcdRtcTargetRegister64,
> +                (RegBase + RTC_TARGET_REG_OFFSET)
> +                );
> +  ASSERT_RETURN_ERROR (PcdStatus);
> +
> +  Status = KvmtoolRtcMapMemory (ImageHandle, (RegBase & ~EFI_PAGE_MASK));
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Failed to map memory for motorola,mc146818. Status = %r\n",
> +      Status
> +      ));
> +    return Status;
> +  }
> +
> +  //
> +  // UEFI takes ownership of the RTC hardware, and exposes its functionality
> +  // through the UEFI Runtime Services GetTime, SetTime, etc. This means we
> +  // need to disable it in the device tree to prevent the OS from attaching
> +  // its device driver as well.
> +  //
> +  Status = FdtClient->SetNodeProperty (
> +                        FdtClient,
> +                        Node,
> +                        "status",
> +                        "disabled",
> +                        sizeof ("disabled")
> +                        );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_WARN,
> +      "Failed to set motorola,mc146818 status to 'disabled', Status = %r\n",
> +      Status
> +      ));
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf
> new file mode 100644
> index 0000000000000000000000000000000000000000..0d1b27997ed00d7c1be3578cab4d1d2eba663a90
> --- /dev/null
> +++ b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf
> @@ -0,0 +1,42 @@
> +#/** @file
> +#  FDT client library for motorola,mc146818 RTC driver
> +#
> +#  Copyright (c) 2020, ARM Limited. All rights reserved.<BR>
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001B
> +  BASE_NAME                      = KvmtoolRtcFdtClientLib
> +  FILE_GUID                      = 3254B4F7-30B5-48C6-B06A-D8FF97F3EF95
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = KvmtoolRtcFdtClientLib|DXE_DRIVER DXE_RUNTIME_DRIVER
> +  CONSTRUCTOR                    = KvmtoolRtcFdtClientLibConstructor
> +
> +[Sources]
> +  KvmtoolRtcFdtClientLib.c
> +
> +[Packages]
> +  ArmVirtPkg/ArmVirtPkg.dec
> +  MdePkg/MdePkg.dec
> +  PcAtChipsetPkg/PcAtChipsetPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  PcdLib
> +  UefiBootServicesTableLib
> +  DxeServicesTableLib
> +
> +[Protocols]
> +  gFdtClientProtocolGuid                                ## CONSUMES
> +
> +[Pcd]
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64
> +
> +[Depex]
> +  gFdtClientProtocolGuid
> 


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

* Re: [PATCH v4 04/15] ArmVirtPkg: Add kvmtool platform driver
  2020-07-07 12:47 ` [PATCH v4 04/15] ArmVirtPkg: Add kvmtool platform driver Sami Mujawar
@ 2020-07-07 13:22   ` Ard Biesheuvel
  0 siblings, 0 replies; 25+ messages in thread
From: Ard Biesheuvel @ 2020-07-07 13:22 UTC (permalink / raw)
  To: Sami Mujawar, devel
  Cc: leif, lersek, Alexandru.Elisei, Andre.Przywara, Matteo.Carlini,
	Laura.Moretta, nd

On 7/7/20 3:47 PM, Sami Mujawar wrote:
> Kvmtool is a virtual machine manager that enables
> hosting KVM guests. It essentially provides a
> virtual hardware platform for guest operating
> systems.
> 
> Kvmtool hands of a device tree containing the
> current hardware configuration to the firmware.
> 
> A standards-based operating system would use
> ACPI to consume the platform hardware
> information, while some operating systems may
> prefer to use Device Tree.
> 
> The KvmtoolPlatformDxe performs the platform
> actions like determining if the firmware should
> expose ACPI or the Device Tree based hardware
> description to the operating system.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> Acked-by: Laszlo Ersek <lersek@redhat.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

> ---
> 
> Notes:
>      v4:
>        - Cleaned up include file and LibraryClasses list.          [Sami]
>        - Missing MemoryAllocationLib here - either add it          [Ard]
>          here, or remove it from LibraryClasses.
>          Ref: https://edk2.groups.io/g/devel/message/61714
>      
>      v3:
>        - Don't use CpuDeadLoop()s in your drivers.                 [Ard]
>        - Returned error code instead of dead loop.                 [Sami]
>        - Installing a protocol on an image handle should not       [Ard]
>          ever fail. So just use ASSERT_EFI_ERROR().
>        - Added assert and returned status code.                    [Sami]
>          Ref: https://edk2.groups.io/g/devel/topic/74200911#59650
>      
>      v2:
>        - Updated according to review comments.                     [Sami]
>      
>      v1:
>        - Add kvmtool platform driver to support loading platform   [Sami]
>          specific information.
>        - Keep code to initialise the variable storage PCDs in the  [Laszlo]
>          platform-specific FVB driver.
>        - Document code derived from                                [Laszlo]
>          "ArmVirtPkg/PlatformHasAcpiDtDxe"
>          Ref: https://edk2.groups.io/g/devel/topic/30915278#30757
> 
>   ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c   | 82 ++++++++++++++++++++
>   ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf | 44 +++++++++++
>   2 files changed, 126 insertions(+)
> 
> diff --git a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..a42b64d1061dcdf8163775f66b6d2f550e481315
> --- /dev/null
> +++ b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c
> @@ -0,0 +1,82 @@
> +/** @file
> +
> +  The KvmtoolPlatformDxe performs the platform specific initialization like:
> +  - It decides if the firmware should expose ACPI or Device Tree-based
> +    hardware description to the operating system.
> +
> +  Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Guid/VariableFormat.h>
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Protocol/FdtClient.h>
> +
> +/** Decide if the firmware should expose ACPI tables or Device Tree and
> +    install the appropriate protocol interface.
> +
> +  Note: This function is derived from "ArmVirtPkg/PlatformHasAcpiDtDxe",
> +        by dropping the word size check, and the fw_cfg check.
> +
> +  @param [in]  ImageHandle  Handle for this image.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_OUT_OF_RESOURCES    There was not enough memory to install the
> +                                  protocols.
> +  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +PlatformHasAcpiDt (
> +  IN EFI_HANDLE           ImageHandle
> +  )
> +{
> +  if (!PcdGetBool (PcdForceNoAcpi)) {
> +    // Expose ACPI tables
> +    return gBS->InstallProtocolInterface (
> +                  &ImageHandle,
> +                  &gEdkiiPlatformHasAcpiGuid,
> +                  EFI_NATIVE_INTERFACE,
> +                  NULL
> +                  );
> +  }
> +
> +  // Expose the Device Tree.
> +  return gBS->InstallProtocolInterface (
> +                &ImageHandle,
> +                &gEdkiiPlatformHasDeviceTreeGuid,
> +                EFI_NATIVE_INTERFACE,
> +                NULL
> +                );
> +}
> +
> +/** Entry point for Kvmtool Platform Dxe
> +
> +  @param [in]  ImageHandle  Handle for this image.
> +  @param [in]  SystemTable  Pointer to the EFI system table.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_OUT_OF_RESOURCES    There was not enough memory to install the
> +                                  protocols.
> +  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +KvmtoolPlatformDxeEntryPoint (
> +  IN EFI_HANDLE           ImageHandle,
> +  IN EFI_SYSTEM_TABLE     *SystemTable
> +  )
> +{
> +  EFI_STATUS                     Status;
> +
> +  Status = PlatformHasAcpiDt (ImageHandle);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return Status;
> +}
> diff --git a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
> new file mode 100644
> index 0000000000000000000000000000000000000000..05087178cc0fa3f7b414336861b39cedec48b68a
> --- /dev/null
> +++ b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
> @@ -0,0 +1,44 @@
> +#/** @file
> +#
> +#  The KvmtoolPlatformDxe performs the platform specific initialization like:
> +#  - It decides if the firmware should expose ACPI or Device Tree-based
> +#    hardware description to the operating system.
> +#
> +#  Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001B
> +  BASE_NAME                      = KvmtoolPlatformDxe
> +  FILE_GUID                      = 7479CCCD-D721-442A-8C73-A72DBB886669
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = KvmtoolPlatformDxeEntryPoint
> +
> +[Sources]
> +  KvmtoolPlatformDxe.c
> +
> +[Packages]
> +  ArmVirtPkg/ArmVirtPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +
> +[Guids]
> +  gEdkiiPlatformHasAcpiGuid       ## SOMETIMES_PRODUCES ## PROTOCOL
> +  gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
> +
> +[Pcd]
> +  gArmVirtTokenSpaceGuid.PcdForceNoAcpi
> +
> +[Depex]
> +  TRUE
> 


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

* Re: [PATCH v4 06/15] ArmVirtPkg: Add Kvmtool NOR flash lib
  2020-07-07 12:48 ` [PATCH v4 06/15] ArmVirtPkg: Add Kvmtool NOR flash lib Sami Mujawar
@ 2020-07-07 13:26   ` Ard Biesheuvel
  0 siblings, 0 replies; 25+ messages in thread
From: Ard Biesheuvel @ 2020-07-07 13:26 UTC (permalink / raw)
  To: Sami Mujawar, devel
  Cc: leif, philmd, lersek, Alexandru.Elisei, Andre.Przywara,
	Matteo.Carlini, Laura.Moretta, nd

On 7/7/20 3:48 PM, Sami Mujawar wrote:
> Kvmtool places the base address of the CFI flash in
> the device tree it passes to UEFI. This library
> parses the kvmtool device tree to read the CFI base
> address and initialise the PCDs use by the NOR flash
> driver and the variable storage.
> 
> UEFI takes ownership of the CFI flash hardware, and
> exposes its functionality through the UEFI Runtime
> Variable Service. Therefore, disable the device tree
> node for the CFI flash used for storing the UEFI
> variables, to prevent the OS from attaching its device
> driver as well.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> Acked-by: Laszlo Ersek <lersek@redhat.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

> ---
> 
> Notes:
>      v4:
>        - Added STATIC to local functions and updated comments      [Sami]
>          to explain why DT node deferred to be disabled in
>          NorFlashPlatformInitialization()
>        - Use STATIC for local functions and explain why DT         [Ard]
>          node for flash is disabled in
>          NorFlashPlatformInitialization()?
>          Ref: https://edk2.groups.io/g/devel/topic/75081477
>      
>      v3:
>        - ASSERT is sufficient to test Locating                     [Ard]
>          gFdtClientProtocolGuid as DEPEX ensures that this is
>          guaranteed to succeed.
>        - Removed additional error handling based on review         [Sami]
>          feedback.
>        - Fix confusion caused by use of macro MAX_FLASH_BANKS.     [Philippe]
>        - Renamed MAX_FLASH_BANKS to MAX_FLASH_DEVICES.             [Sami]
>        - Use macro to define block size for flash.                 [Philippe]
>        - Defined macro KVMTOOL_NOR_BLOCK_SIZE and also configured  [Sami]
>          to reflect the correct block size 64KB.
>        - Disable the DT flash node used for UEFI variable storage  [Sami]
>          as UEFI takes ownership of the flash device.
>          Ref: https://edk2.groups.io/g/devel/topic/74200914#60341
>      
>      v2:
>        - Library to read CFI flash base address from DT and initialise [Sami]
>          PCDs used for NOR flash variables.
> 
>   ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c      | 336 ++++++++++++++++++++
>   ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf |  49 +++
>   2 files changed, 385 insertions(+)
> 
> diff --git a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..dce585b779a87dabde280cf4bbca5b828ceccda4
> --- /dev/null
> +++ b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c
> @@ -0,0 +1,336 @@
> +/** @file
> +   An instance of the NorFlashPlatformLib for Kvmtool platform.
> +
> + Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> + **/
> +
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/NorFlashPlatformLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Protocol/FdtClient.h>
> +
> +/** Macro defining the NOR block size configured in Kvmtool.
> +*/
> +#define KVMTOOL_NOR_BLOCK_SIZE  SIZE_64KB
> +
> +/** Macro defining the maximum number of Flash devices.
> +*/
> +#define MAX_FLASH_DEVICES       4
> +
> +/** Macro defining the cfi-flash label describing the UEFI variable store.
> +*/
> +#define LABEL_UEFI_VAR_STORE    "System-firmware"
> +
> +STATIC NOR_FLASH_DESCRIPTION  mNorFlashDevices[MAX_FLASH_DEVICES];
> +STATIC UINTN                  mNorFlashDeviceCount = 0;
> +STATIC INT32                  mUefiVarStoreNode = MAX_INT32;
> +STATIC FDT_CLIENT_PROTOCOL    *mFdtClient;
> +
> +/** This function performs platform specific actions to initialise
> +    the NOR flash, if required.
> +
> +  @retval EFI_SUCCESS           Success.
> +**/
> +EFI_STATUS
> +NorFlashPlatformInitialization (
> +  VOID
> +  )
> +{
> +  EFI_STATUS                  Status;
> +
> +  DEBUG ((DEBUG_INFO, "NorFlashPlatformInitialization\n"));
> +
> +  if ((mNorFlashDeviceCount > 0) && (mUefiVarStoreNode != MAX_INT32)) {
> +    //
> +    // UEFI takes ownership of the cfi-flash hardware, and exposes its
> +    // functionality through the UEFI Runtime Variable Service. This means we
> +    // need to disable it in the device tree to prevent the OS from attaching
> +    // its device driver as well.
> +    // Note: This library is loaded twice. First by FaultTolerantWriteDxe to
> +    // setup the PcdFlashNvStorageFtw* and later by NorFlashDxe to provide the
> +    // NorFlashPlatformLib interfaces. If the node is disabled when the library
> +    // is first loaded, then during the subsequent loading of the library the
> +    // call to FindNextCompatibleNode() from the library constructor skips the
> +    // FDT node used for UEFI storage variable. Due to this we cannot setup the
> +    // NOR flash device description i.e. mNorFlashDevices[].
> +    // Since NorFlashPlatformInitialization() is called only by NorFlashDxe,
> +    // we know it is safe to disable the node here.
> +    //
> +    Status = mFdtClient->SetNodeProperty (
> +                           mFdtClient,
> +                           mUefiVarStoreNode,
> +                           "status",
> +                           "disabled",
> +                           sizeof ("disabled")
> +                           );
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_WARN, "Failed to set cfi-flash status to 'disabled'\n"));
> +    }
> +  } else {
> +    Status = EFI_NOT_FOUND;
> +    DEBUG ((DEBUG_ERROR, "Flash device for UEFI variable storage not found\n"));
> +  }
> +
> +  return Status;
> +}
> +
> +/** Initialise Non volatile Flash storage variables.
> +
> +  @param [in]  FlashDevice Pointer to the NOR Flash device.
> +
> +  @retval EFI_SUCCESS           Success.
> +  @retval EFI_INVALID_PARAMETER A parameter is invalid.
> +  @retval EFI_OUT_OF_RESOURCES  Insufficient flash storage space.
> +**/
> +STATIC
> +EFI_STATUS
> +SetupVariableStore (
> +  IN NOR_FLASH_DESCRIPTION * FlashDevice
> +  )
> +{
> +  UINTN   FlashRegion;
> +  UINTN   FlashNvStorageVariableBase;
> +  UINTN   FlashNvStorageFtwWorkingBase;
> +  UINTN   FlashNvStorageFtwSpareBase;
> +  UINTN   FlashNvStorageVariableSize;
> +  UINTN   FlashNvStorageFtwWorkingSize;
> +  UINTN   FlashNvStorageFtwSpareSize;
> +
> +  FlashNvStorageVariableSize = PcdGet32 (PcdFlashNvStorageVariableSize);
> +  FlashNvStorageFtwWorkingSize = PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
> +  FlashNvStorageFtwSpareSize =  PcdGet32 (PcdFlashNvStorageFtwSpareSize);
> +
> +  if ((FlashNvStorageVariableSize == 0)   ||
> +      (FlashNvStorageFtwWorkingSize == 0) ||
> +      (FlashNvStorageFtwSpareSize == 0)) {
> +    DEBUG ((DEBUG_ERROR, "FlashNvStorage size not defined\n"));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // Setup the variable store
> +  FlashRegion = FlashDevice->DeviceBaseAddress;
> +
> +  FlashNvStorageVariableBase = FlashRegion;
> +  FlashRegion += PcdGet32 (PcdFlashNvStorageVariableSize);
> +
> +  FlashNvStorageFtwWorkingBase = FlashRegion;
> +  FlashRegion += PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
> +
> +  FlashNvStorageFtwSpareBase = FlashRegion;
> +  FlashRegion += PcdGet32 (PcdFlashNvStorageFtwSpareSize);
> +
> +  if (FlashRegion > (FlashDevice->DeviceBaseAddress + FlashDevice->Size)) {
> +    DEBUG ((DEBUG_ERROR, "Insufficient flash storage size\n"));
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  PcdSet32S (
> +    PcdFlashNvStorageVariableBase,
> +    FlashNvStorageVariableBase
> +    );
> +
> +  PcdSet32S (
> +    PcdFlashNvStorageFtwWorkingBase,
> +    FlashNvStorageFtwWorkingBase
> +    );
> +
> +  PcdSet32S (
> +    PcdFlashNvStorageFtwSpareBase,
> +    FlashNvStorageFtwSpareBase
> +    );
> +
> +  DEBUG ((
> +    DEBUG_INFO,
> +    "PcdFlashNvStorageVariableBase = 0x%x\n",
> +    FlashNvStorageVariableBase
> +    ));
> +  DEBUG ((
> +    DEBUG_INFO,
> +    "PcdFlashNvStorageVariableSize = 0x%x\n",
> +    FlashNvStorageVariableSize
> +    ));
> +  DEBUG ((
> +    DEBUG_INFO,
> +    "PcdFlashNvStorageFtwWorkingBase = 0x%x\n",
> +    FlashNvStorageFtwWorkingBase
> +    ));
> +  DEBUG ((
> +    DEBUG_INFO,
> +    "PcdFlashNvStorageFtwWorkingSize = 0x%x\n",
> +    FlashNvStorageFtwWorkingSize
> +    ));
> +  DEBUG ((
> +    DEBUG_INFO,
> +    "PcdFlashNvStorageFtwSpareBase = 0x%x\n",
> +    FlashNvStorageFtwSpareBase
> +    ));
> +  DEBUG ((
> +    DEBUG_INFO,
> +    "PcdFlashNvStorageFtwSpareSize = 0x%x\n",
> +    FlashNvStorageFtwSpareSize
> +    ));
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/** Return the Flash devices on the platform.
> +
> +  @param [out]  NorFlashDescriptions    Pointer to the Flash device description.
> +  @param [out]  Count                   Number of Flash devices.
> +
> +  @retval EFI_SUCCESS           Success.
> +  @retval EFI_NOT_FOUND         Flash device not found.
> +**/
> +EFI_STATUS
> +NorFlashPlatformGetDevices (
> +  OUT NOR_FLASH_DESCRIPTION   **NorFlashDescriptions,
> +  OUT UINT32                  *Count
> +  )
> +{
> +  if (mNorFlashDeviceCount > 0) {
> +    *NorFlashDescriptions = mNorFlashDevices;
> +    *Count = mNorFlashDeviceCount;
> +    return EFI_SUCCESS;
> +  }
> +  return EFI_NOT_FOUND;
> +}
> +
> +/** Entrypoint for NorFlashPlatformLib.
> +
> +  @param [in]  ImageHandle  The handle to the image.
> +  @param [in]  SystemTable  Pointer to the System Table.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
> +  @retval EFI_NOT_FOUND           Flash device not found.
> +**/
> +EFI_STATUS
> +EFIAPI
> +NorFlashPlatformLibConstructor (
> +  IN  EFI_HANDLE          ImageHandle,
> +  IN  EFI_SYSTEM_TABLE  * SystemTable
> +  )
> +{
> +  INT32                       Node;
> +  EFI_STATUS                  Status;
> +  EFI_STATUS                  FindNodeStatus;
> +  CONST UINT32                *Reg;
> +  UINT32                      PropSize;
> +  UINT64                      Base;
> +  UINT64                      Size;
> +  UINTN                       UefiVarStoreIndex;
> +  CONST CHAR8                 *Label;
> +  UINT32                      LabelLen;
> +
> +  if (mNorFlashDeviceCount != 0) {
> +    return EFI_SUCCESS;
> +  }
> +
> +  Status = gBS->LocateProtocol (
> +                  &gFdtClientProtocolGuid,
> +                  NULL,
> +                  (VOID **)&mFdtClient
> +                  );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  UefiVarStoreIndex = MAX_UINTN;
> +  for (FindNodeStatus = mFdtClient->FindCompatibleNode (
> +                                      mFdtClient,
> +                                      "cfi-flash",
> +                                      &Node
> +                                      );
> +       !EFI_ERROR (FindNodeStatus) &&
> +         (mNorFlashDeviceCount < MAX_FLASH_DEVICES);
> +       FindNodeStatus = mFdtClient->FindNextCompatibleNode (
> +                                      mFdtClient,
> +                                      "cfi-flash",
> +                                      Node,
> +                                      &Node
> +    )) {
> +    Status = mFdtClient->GetNodeProperty (
> +                           mFdtClient,
> +                           Node,
> +                           "label",
> +                           (CONST VOID **)&Label,
> +                           &LabelLen
> +                           );
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((
> +        DEBUG_ERROR,
> +        "%a: GetNodeProperty ('label') failed (Status == %r)\n",
> +        __FUNCTION__,
> +        Status
> +        ));
> +    } else if (AsciiStrCmp (Label, LABEL_UEFI_VAR_STORE) == 0) {
> +      UefiVarStoreIndex = mNorFlashDeviceCount;
> +      mUefiVarStoreNode = Node;
> +    }
> +
> +    Status = mFdtClient->GetNodeProperty (
> +                           mFdtClient,
> +                           Node,
> +                           "reg",
> +                           (CONST VOID **)&Reg,
> +                           &PropSize
> +                           );
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_ERROR, "%a: GetNodeProperty () failed (Status == %r)\n",
> +        __FUNCTION__, Status));
> +      continue;
> +    }
> +
> +    ASSERT ((PropSize % (4 * sizeof (UINT32))) == 0);
> +
> +    while ((PropSize >= (4 * sizeof (UINT32))) &&
> +           (mNorFlashDeviceCount < MAX_FLASH_DEVICES)) {
> +      Base = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[0]));
> +      Size = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[2]));
> +      Reg += 4;
> +
> +      PropSize -= 4 * sizeof (UINT32);
> +
> +      //
> +      // Disregard any flash devices that overlap with the primary FV.
> +      // The firmware is not updatable from inside the guest anyway.
> +      //
> +      if ((PcdGet64 (PcdFvBaseAddress) + PcdGet32 (PcdFvSize) > Base) &&
> +          (Base + Size) > PcdGet64 (PcdFvBaseAddress)) {
> +        continue;
> +      }
> +
> +      DEBUG ((
> +        DEBUG_INFO,
> +        "NOR%d : Base = 0x%lx, Size = 0x%lx\n",
> +        mNorFlashDeviceCount,
> +        Base,
> +        Size
> +        ));
> +
> +      mNorFlashDevices[mNorFlashDeviceCount].DeviceBaseAddress = (UINTN)Base;
> +      mNorFlashDevices[mNorFlashDeviceCount].RegionBaseAddress = (UINTN)Base;
> +      mNorFlashDevices[mNorFlashDeviceCount].Size = (UINTN)Size;
> +      mNorFlashDevices[mNorFlashDeviceCount].BlockSize = KVMTOOL_NOR_BLOCK_SIZE;
> +      mNorFlashDeviceCount++;
> +    }
> +  } // for
> +
> +  // Setup the variable store in the last device
> +  if (mNorFlashDeviceCount > 0) {
> +    if (UefiVarStoreIndex == MAX_UINTN) {
> +      // We did not find a label matching the UEFI Variable store. Default to
> +      // using the last cfi-flash device as the variable store.
> +      UefiVarStoreIndex = mNorFlashDeviceCount - 1;
> +      mUefiVarStoreNode = Node;
> +    }
> +    if (mNorFlashDevices[UefiVarStoreIndex].DeviceBaseAddress != 0) {
> +      return SetupVariableStore (&mNorFlashDevices[UefiVarStoreIndex]);
> +    }
> +  }
> +
> +  return EFI_NOT_FOUND;
> +}
> +
> diff --git a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
> new file mode 100644
> index 0000000000000000000000000000000000000000..a3230e4b2be668904322103825b93e867503984e
> --- /dev/null
> +++ b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
> @@ -0,0 +1,49 @@
> +#/** @file
> +#
> +#  Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001B
> +  BASE_NAME                      = NorFlashKvmtoolLib
> +  FILE_GUID                      = E75F07A1-B160-4893-BDD4-09E32FF847DC
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = NorFlashPlatformLib
> +  CONSTRUCTOR                    = NorFlashPlatformLibConstructor
> +
> +[Sources.common]
> +  NorFlashKvmtool.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +  ArmVirtPkg/ArmVirtPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  PcdLib
> +  UefiBootServicesTableLib
> +
> +[Protocols]
> +  gFdtClientProtocolGuid          ## CONSUMES
> +
> +[Pcd]
> +  gArmTokenSpaceGuid.PcdFvBaseAddress
> +  gArmTokenSpaceGuid.PcdFvSize
> +
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
> +
> +[Depex]
> +  gFdtClientProtocolGuid
> +
> 


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

* Re: [PATCH v4 11/15] ArmVirtPkg: Support for kvmtool virtual platform
  2020-07-07 12:48 ` [PATCH v4 11/15] ArmVirtPkg: Support for kvmtool virtual platform Sami Mujawar
@ 2020-07-07 13:27   ` Ard Biesheuvel
  0 siblings, 0 replies; 25+ messages in thread
From: Ard Biesheuvel @ 2020-07-07 13:27 UTC (permalink / raw)
  To: Sami Mujawar, devel
  Cc: leif, lersek, Alexandru.Elisei, Andre.Przywara, Matteo.Carlini,
	Laura.Moretta, nd

On 7/7/20 3:48 PM, Sami Mujawar wrote:
> Kvmtool is a virtual machine manager that enables hosting
> KVM guests. Kvmtool emulates certain devices like serial
> port, RTC, etc. essentially providing a virtual platform.
> 
> This patch adds support for kvmtool virtual platform.
> 
> Following is a brief description of the firmware
> implementation choices:
> 
> - Serial Port: 16550 UART
>    On some platforms the 16550 UART is interfaced using
>    PCI. Therefore, the 16550 Serial port library is
>    dependent on the PCI library. The 16550 UART driver
>    checks the Device ID represented using the PCD
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSerialPciDeviceInfo
>    to determine if the UART is behind PCI.
>    If the Device ID is 0xFF then the serial 16550 UART
>    is not behind PCI.
> 
>    On Kvmtool the Serial 16550 UART is not behind PCI,
>    and therefore a combination of BasePciLibPciExpress
>    and BasePciExpressLib is used to satisfy the PCI
>    library dependency.
> 
>    The PcdSerialPciDeviceInfo is also set to 0xFF to
>    indicate that the Serial 16550 UART is not behind
>    PCI. The PCD PcdSerialUseMmio is also set to TRUE
>    to indicate MMIO accesses are required for the
>    UART registers.
> 
>    Additionally two instances of PlatformHookLibs are
>    provided EarlyFdt16550SerialPortHookLib and
>    Fdt16550SerialPortHookLib to patch the
>    PcdSerialRegisterBase so that BaseSerialPortLib16550
>    and retrieve the base address of the 16550 UART.
> 
> - Dependency order for Flash
>    FaultTolerantWriteDxe makes use of PCDs (e.g.
>    PcdFlashNvStorageFtwSpareBase64 etc.), which in
>    case of kvmtool will be evaluated based on the CFI
>    flash base address read from the DT. These variables
>    are populated in the NorFlashPlatformLib loaded by
>    ArmVeNorFlashDxe.
> 
>    This results in a dependency issue with
>    FaultTolerantWriteDxe. To resolve this make the
>    NorFlashPlatformLib as a library dependency for
>    FaultTolerantWriteDxe.
> 
> - RTC Controller
>    A separate patch updates the MC146818 RTC controller
>    driver to support MMIO accesses.
>    A KvmtoolRtcFdtClientLib has been introduced to
>    extract the base addresses of the RTC controller
>    from the platform device tree and map the RTC
>    register space as Runtime Memory.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> Acked-by: Laszlo Ersek <lersek@redhat.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

> ---
> 
> Notes:
>      v4:
>        - Updated to use XIPFLAGS to set no-jump-tables. Removed     [Sami]
>          Networking and EBC support in this patch series. These
>          features will be added later as separate patches.
>        - Use XIPFLAGS to set no-jump-tables and use .inc files      [Ard]
>          for NetworkPkg modules.
>          Ref: https://edk2.groups.io/g/devel/topic/75081494
>      
>      v3:
>        - Use correct terminology. kvmtool is *not* an emulator.     [Ard]
>        - Updated patch series to reflect correct terminology.       [Sami]
>        - Don't define bits and pieces you cannot make use of.       [Ard]
>        - Updated workspace to remove unsupported features.          [Sami]
>          Ref: https://edk2.groups.io/g/devel/topic/74200917#59552
>      
>      v2:
>        - Updates to reflect review comments and support             [Sami]
>          for latest features emulated by kvmtool e.g. CFI.
>      
>      v1:
>        - Add support for Kvmtool emulated platform                  [Sami]
>        - Add more justification for platform and                    [Laszlo]
>          document platform maintainer.
>          Ref: https://edk2.groups.io/g/devel/topic/30915279#30693
> 
>   ArmVirtPkg/ArmVirtKvmTool.dsc | 321 ++++++++++++++++++++
>   ArmVirtPkg/ArmVirtKvmTool.fdf | 234 ++++++++++++++
>   2 files changed, 555 insertions(+)
> 
> diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
> new file mode 100644
> index 0000000000000000000000000000000000000000..bf008be50fcb96bbe4f559f0b165a7a555b4cc1f
> --- /dev/null
> +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
> @@ -0,0 +1,321 @@
> +#  @file
> +#  Workspace file for KVMTool virtual platform.
> +#
> +#  Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#
> +
> +################################################################################
> +#
> +# Defines Section - statements that will be processed to create a Makefile.
> +#
> +################################################################################
> +[Defines]
> +  PLATFORM_NAME                  = ArmVirtKvmTool
> +  PLATFORM_GUID                  = 4CB2C61E-FA32-4130-8E37-54ABC71A1A43
> +  PLATFORM_VERSION               = 0.1
> +  DSC_SPECIFICATION              = 0x0001001B
> +!ifdef $(EDK2_OUT_DIR)
> +  OUTPUT_DIRECTORY               = $(EDK2_OUT_DIR)
> +!else
> +  OUTPUT_DIRECTORY               = Build/ArmVirtKvmTool-$(ARCH)
> +!endif
> +  SUPPORTED_ARCHITECTURES        = AARCH64|ARM
> +  BUILD_TARGETS                  = DEBUG|RELEASE
> +  SKUID_IDENTIFIER               = DEFAULT
> +  FLASH_DEFINITION               = ArmVirtPkg/ArmVirtKvmTool.fdf
> +
> +!include ArmVirtPkg/ArmVirt.dsc.inc
> +
> +[LibraryClasses.common]
> +  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> +  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> +
> +  # Virtio Support
> +  VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
> +  VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
> +
> +  ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
> +  ArmVirtMemInfoLib|ArmVirtPkg/Library/KvmtoolVirtMemInfoLib/KvmtoolVirtMemInfoLib.inf
> +
> +  TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
> +  NorFlashPlatformLib|ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
> +
> +  CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
> +
> +  # BDS Libraries
> +  UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> +  PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +  BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
> +
> +  CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
> +  FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
> +
> +  FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
> +
> +  PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
> +  PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
> +  PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
> +
> +  TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
> +  AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
> +
> +  PlatformPeiLib|ArmVirtPkg/Library/KvmtoolPlatformPeiLib/KvmtoolPlatformPeiLib.inf
> +
> +  PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
> +  PlatformHookLib|ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf
> +  SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
> +
> +[LibraryClasses.common.SEC, LibraryClasses.common.PEI_CORE, LibraryClasses.common.PEIM]
> +  PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
> +  PlatformHookLib|ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.inf
> +  SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
> +
> +[LibraryClasses.common.UEFI_DRIVER]
> +  UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
> +
> +[BuildOptions]
> +  *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
> +  #
> +  # We need to avoid jump tables in SEC and BASE modules, so that the PE/COFF
> +  # self-relocation code itself is guaranteed to be position independent.
> +  #
> +  GCC:*_*_*_CC_XIPFLAGS = -fno-jump-tables
> +
> +################################################################################
> +#
> +# Pcd Section - list of all EDK II PCD Entries defined by this Platform
> +#
> +################################################################################
> +
> +[PcdsFeatureFlag.common]
> +  ## If TRUE, Graphics Output Protocol will be installed on virtual handle created by ConsplitterDxe.
> +  #  It could be set FALSE to save size.
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
> +
> +  # Use MMIO for accessing RTC controller registers.
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|TRUE
> +
> +[PcdsFixedAtBuild.common]
> +  gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000000F
> +
> +  gArmPlatformTokenSpaceGuid.PcdCoreCount|1
> +
> +!if $(ARCH) == AARCH64
> +  gArmTokenSpaceGuid.PcdVFPEnabled|1
> +!endif
> +
> +  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> +
> +  # Size of the region used by UEFI in permanent memory (Reserved 64MB)
> +  gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x04000000
> +
> +  #
> +  # TTY Terminal Type
> +  # 0-PCANSI, 1-VT100, 2-VT00+, 3-UTF8, 4-TTYTERM
> +  gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4
> +
> +  #
> +  # ARM Virtual Architectural Timer -- fetch frequency from KVM
> +  #
> +  gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|0
> +
> +  # Use MMIO for accessing Serial port registers.
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialPciDeviceInfo|{0xFF}
> +
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
> +
> +  #
> +  # The maximum physical I/O addressability of the processor, set with
> +  # BuildCpuHob().
> +  #
> +  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
> +
> +[PcdsPatchableInModule.common]
> +  #
> +  # This will be overridden in the code
> +  #
> +  gArmTokenSpaceGuid.PcdSystemMemoryBase|0x0
> +  gArmTokenSpaceGuid.PcdSystemMemorySize|0x0
> +
> +  #
> +  # The device tree base address is handed off by kvmtool.
> +  # We are booting from RAM using the Linux kernel boot protocol,
> +  # x0 will point to the DTB image in memory.
> +  #
> +  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress|0x0
> +
> +  gArmTokenSpaceGuid.PcdFdBaseAddress|0x0
> +  gArmTokenSpaceGuid.PcdFvBaseAddress|0x0
> +
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x0
> +
> +[PcdsDynamicDefault.common]
> +  gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
> +
> +  gArmTokenSpaceGuid.PcdArmArchTimerSecIntrNum|0x0
> +  gArmTokenSpaceGuid.PcdArmArchTimerIntrNum|0x0
> +  gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum|0x0
> +  gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum|0x0
> +
> +  #
> +  # ARM General Interrupt Controller
> +  #
> +  gArmTokenSpaceGuid.PcdGicDistributorBase|0x0
> +  gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x0
> +  gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0x0
> +
> +  #
> +  # Set video resolution for boot options and for text setup.
> +  # PlatformDxe can set the former at runtime.
> +  #
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|800
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|600
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoHorizontalResolution|640
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoVerticalResolution|480
> +
> +  ## Force DTB
> +  gArmVirtTokenSpaceGuid.PcdForceNoAcpi|TRUE
> +
> +  # Setup Flash storage variables
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x40000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x40000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x40000
> +
> +  ## RTC Register address in MMIO space.
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64|0x0
> +
> +################################################################################
> +#
> +# Components Section - list of all EDK II Modules needed by this Platform
> +#
> +################################################################################
> +[Components.common]
> +  #
> +  # PEI Phase modules
> +  #
> +  ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf {
> +    <LibraryClasses>
> +      ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf
> +      LzmaDecompressLib|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
> +      PrePiLib|EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
> +      HobLib|EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
> +      PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
> +      MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
> +  }
> +
> +  #
> +  # DXE
> +  #
> +  MdeModulePkg/Core/Dxe/DxeMain.inf {
> +    <LibraryClasses>
> +      NULL|MdeModulePkg/Library/DxeCrc32GuidedSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf
> +      DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
> +  }
> +  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
> +    <LibraryClasses>
> +      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> +  }
> +
> +  #
> +  # Architectural Protocols
> +  #
> +  ArmPkg/Drivers/CpuDxe/CpuDxe.inf
> +  MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
> +  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
> +    <LibraryClasses>
> +      NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
> +      NULL|EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
> +      BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> +  }
> +
> +  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
> +  MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> +  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf {
> +    <LibraryClasses>
> +      NULL|ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
> +  }
> +
> +  MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
> +  MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
> +  MdeModulePkg/Universal/Metronome/Metronome.inf
> +  PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf {
> +    <LibraryClasses>
> +      NULL|ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf
> +  }
> +
> +  MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
> +  MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
> +  MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
> +  MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
> +  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> +
> +  MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
> +
> +  ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
> +  ArmPkg/Drivers/TimerDxe/TimerDxe.inf {
> +    <LibraryClasses>
> +      NULL|ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.inf
> +  }
> +
> +  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> +
> +  MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
> +
> +  #
> +  # Platform Driver
> +  #
> +  ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
> +  ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf
> +  ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> +  ArmVirtPkg/HighMemDxe/HighMemDxe.inf
> +  OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
> +  OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
> +  OvmfPkg/VirtioNetDxe/VirtioNet.inf
> +  OvmfPkg/VirtioRngDxe/VirtioRng.inf
> +
> +  #
> +  # FAT filesystem + GPT/MBR partitioning + UDF filesystem
> +  #
> +  MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
> +  MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
> +  MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
> +  FatPkg/EnhancedFatDxe/Fat.inf
> +  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
> +
> +  #
> +  # Bds
> +  #
> +  MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf {
> +    <LibraryClasses>
> +      DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
> +      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> +  }
> +  MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
> +  MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
> +  MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
> +  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> +  MdeModulePkg/Logo/LogoDxe.inf
> +  MdeModulePkg/Application/UiApp/UiApp.inf {
> +    <LibraryClasses>
> +      NULL|MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf
> +      NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf
> +      NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
> +  }
> +
> +  #
> +  # SCSI Bus and Disk Driver
> +  #
> +  MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
> +  MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
> diff --git a/ArmVirtPkg/ArmVirtKvmTool.fdf b/ArmVirtPkg/ArmVirtKvmTool.fdf
> new file mode 100644
> index 0000000000000000000000000000000000000000..ebc82c5bd53cf98bc829c57dc0946b7a5854ca6c
> --- /dev/null
> +++ b/ArmVirtPkg/ArmVirtKvmTool.fdf
> @@ -0,0 +1,234 @@
> +#
> +#  Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#
> +
> +################################################################################
> +#
> +# FD Section
> +# The [FD] Section is made up of the definition statements and a
> +# description of what goes into  the Flash Device Image.  Each FD section
> +# defines one flash "device" image.  A flash device image may be one of
> +# the following: Removable media bootable image (like a boot floppy
> +# image,) an Option ROM image (that would be "flashed" into an add-in
> +# card,) a System "Flash"  image (that would be burned into a system's
> +# flash) or an Update ("Capsule") image that will be used to update and
> +# existing system flash.
> +#
> +################################################################################
> +
> +[FD.KVMTOOL_EFI]
> +BaseAddress   = 0x00000000|gArmTokenSpaceGuid.PcdFdBaseAddress
> +# The size in bytes of the FLASH Device
> +Size          = 0x00200000|gArmTokenSpaceGuid.PcdFdSize
> +ErasePolarity = 1
> +
> +# This one is tricky, it must be: BlockSize * NumBlocks = Size
> +BlockSize     = 0x00001000
> +NumBlocks     = 0x200
> +
> +################################################################################
> +#
> +# Following are lists of FD Region layout which correspond to the locations of different
> +# images within the flash device.
> +#
> +# Regions must be defined in ascending order and may not overlap.
> +#
> +# A Layout Region start with a eight digit hex offset (leading "0x" required) followed by
> +# the pipe "|" character, followed by the size of the region, also in hex with the leading
> +# "0x" characters. Like:
> +# Offset|Size
> +# PcdOffsetCName|PcdSizeCName
> +# RegionType <FV, DATA, or FILE>
> +#
> +################################################################################
> +
> +#
> +# Implement the Linux kernel header layout so that the loader will identify
> +# it as something bootable, and execute it with a FDT pointer in x0 or r2.
> +# This area will be reused to store a copy of the FDT so round it up to 32 KB.
> +#
> +0x00000000|0x00008000
> +DATA = {
> +!if $(ARCH) == AARCH64
> +  0x01, 0x00, 0x00, 0x10,                         # code0: adr x1, .
> +  0xff, 0x1f, 0x00, 0x14,                         # code1: b 0x8000
> +  0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, # text_offset: 512 KB
> +  0x00, 0x00, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, # image_size: 2 MB
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, # flags
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, # res2
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, # res3
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, # res4
> +  0x41, 0x52, 0x4d, 0x64,                         # magic: "ARM\x64"
> +  0x00, 0x00, 0x00, 0x00                          # res5
> +!else
> +  0x08, 0x10, 0x4f, 0xe2, # adr r1, .
> +  0x02, 0x00, 0xa0, 0xe1, # mov r0, r2 (DTB)
> +  0x00, 0x00, 0xa0, 0xe1, # nop
> +  0x00, 0x00, 0xa0, 0xe1, # nop
> +  0x00, 0x00, 0xa0, 0xe1, # nop
> +  0x00, 0x00, 0xa0, 0xe1, # nop
> +  0x00, 0x00, 0xa0, 0xe1, # nop
> +  0x00, 0x00, 0xa0, 0xe1, # nop
> +
> +  0xf6, 0x1f, 0x00, 0xea, # b 0x8000
> +  0x18, 0x28, 0x6f, 0x01, # magic
> +  0x00, 0x00, 0x00, 0x00, # start
> +  0x00, 0x00, 0x20, 0x00, # image size: 2 MB
> +  0x01, 0x02, 0x03, 0x04  # endiannness flag
> +!endif
> +}
> +
> +0x00008000|0x001f8000
> +gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
> +FV = FVMAIN_COMPACT
> +
> +################################################################################
> +#
> +# FV Section
> +#
> +# [FV] section is used to define what components or modules are placed within a flash
> +# device file.  This section also defines order the components and modules are positioned
> +# within the image.  The [FV] section consists of define statements, set statements and
> +# module statements.
> +#
> +################################################################################
> +
> +[FV.FvMain]
> +FvNameGuid         = 8A91C08E-7D9D-4933-84D6-901D26D0766E
> +BlockSize          = 0x40
> +NumBlocks          = 0         # This FV gets compressed so make it just big enough
> +FvAlignment        = 16        # FV alignment and FV attributes setting.
> +ERASE_POLARITY     = 1
> +MEMORY_MAPPED      = TRUE
> +STICKY_WRITE       = TRUE
> +LOCK_CAP           = TRUE
> +LOCK_STATUS        = TRUE
> +WRITE_DISABLED_CAP = TRUE
> +WRITE_ENABLED_CAP  = TRUE
> +WRITE_STATUS       = TRUE
> +WRITE_LOCK_CAP     = TRUE
> +WRITE_LOCK_STATUS  = TRUE
> +READ_DISABLED_CAP  = TRUE
> +READ_ENABLED_CAP   = TRUE
> +READ_STATUS        = TRUE
> +READ_LOCK_CAP      = TRUE
> +READ_LOCK_STATUS   = TRUE
> +
> +  INF MdeModulePkg/Core/Dxe/DxeMain.inf
> +  INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
> +  INF ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf
> +  INF ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> +  INF ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
> +  INF ArmVirtPkg/HighMemDxe/HighMemDxe.inf
> +
> +  #
> +  # PI DXE Drivers producing Architectural Protocols (EFI Services)
> +  #
> +  INF ArmPkg/Drivers/CpuDxe/CpuDxe.inf
> +  INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
> +  INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
> +  INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> +  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> +  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> +
> +  INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
> +  INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
> +
> +  INF  MdeModulePkg/Universal/Metronome/Metronome.inf
> +  INF  PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
> +
> +  INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
> +
> +  #
> +  # Multiple Console IO support
> +  #
> +  INF MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
> +  INF MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
> +  INF MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
> +  INF MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
> +  INF MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> +
> +  INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
> +  INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf
> +  INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
> +  INF ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> +
> +  #
> +  # FAT filesystem + GPT/MBR partitioning + UDF filesystem
> +  #
> +  INF MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
> +  INF MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
> +  INF FatPkg/EnhancedFatDxe/Fat.inf
> +  INF MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
> +  INF MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
> +
> +  #
> +  # Platform Driver
> +  #
> +  INF OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
> +  INF OvmfPkg/VirtioNetDxe/VirtioNet.inf
> +  INF OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
> +  INF OvmfPkg/VirtioRngDxe/VirtioRng.inf
> +
> +  #
> +  # UEFI application (Shell Embedded Boot Loader)
> +  #
> +  INF ShellPkg/Application/Shell/Shell.inf
> +  INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
> +
> +  #
> +  # Bds
> +  #
> +  INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
> +  INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
> +  INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
> +  INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
> +  INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> +  INF MdeModulePkg/Application/UiApp/UiApp.inf
> +
> +  #
> +  # SCSI Bus and Disk Driver
> +  #
> +  INF MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
> +  INF MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
> +
> +  #
> +  # TianoCore logo (splash screen)
> +  #
> +  INF MdeModulePkg/Logo/LogoDxe.inf
> +
> +  #
> +  # Ramdisk support
> +  #
> +  INF MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
> +
> +[FV.FVMAIN_COMPACT]
> +FvAlignment        = 16
> +ERASE_POLARITY     = 1
> +MEMORY_MAPPED      = TRUE
> +STICKY_WRITE       = TRUE
> +LOCK_CAP           = TRUE
> +LOCK_STATUS        = TRUE
> +WRITE_DISABLED_CAP = TRUE
> +WRITE_ENABLED_CAP  = TRUE
> +WRITE_STATUS       = TRUE
> +WRITE_LOCK_CAP     = TRUE
> +WRITE_LOCK_STATUS  = TRUE
> +READ_DISABLED_CAP  = TRUE
> +READ_ENABLED_CAP   = TRUE
> +READ_STATUS        = TRUE
> +READ_LOCK_CAP      = TRUE
> +READ_LOCK_STATUS   = TRUE
> +
> +  INF RuleOverride = SELF_RELOC ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
> +
> +  FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
> +    SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
> +      SECTION FV_IMAGE = FVMAIN
> +    }
> +  }
> +
> +!include ArmVirtRules.fdf.inc
> 


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

* Re: [PATCH v4 14/15] .python/SpellCheck: Add 'XIPFLAGS' to "words" section
  2020-07-07 12:48 ` [PATCH v4 14/15] .python/SpellCheck: Add 'XIPFLAGS' to "words" section Sami Mujawar
@ 2020-07-07 13:28   ` Ard Biesheuvel
  0 siblings, 0 replies; 25+ messages in thread
From: Ard Biesheuvel @ 2020-07-07 13:28 UTC (permalink / raw)
  To: Sami Mujawar, devel; +Cc: leif, lersek, Matteo.Carlini, Laura.Moretta, nd

On 7/7/20 3:48 PM, Sami Mujawar wrote:
> The EDKII Core CI reports spelling error for XIPFLAGS. The
> XIPFLAGS are typically used to specify XIP options to the
> compiler. e.g. GCC:*_*_*_CC_XIPFLAGS = -fno-jump-tables
> 
> Add 'XIPFLAGS' to "words" section in cspell.base.yaml file
> to avoid spelling check error.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>

Acked-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

> ---
>   .pytool/Plugin/SpellCheck/cspell.base.yaml | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/.pytool/Plugin/SpellCheck/cspell.base.yaml b/.pytool/Plugin/SpellCheck/cspell.base.yaml
> index 1ce5f6991104b83e41e769299b23cb3fa74c46e9..ad6f7ac06ad18efcd4ab9c2346356dfd6885d827 100644
> --- a/.pytool/Plugin/SpellCheck/cspell.base.yaml
> +++ b/.pytool/Plugin/SpellCheck/cspell.base.yaml
> @@ -178,5 +178,6 @@
>           "vcruntimed",
>           "ucrtd",
>           "msvcrtd",
> +        "XIPFLAGS"
>       ]
>   }
> 


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

* Re: [edk2-devel] [PATCH v4 02/15] ArmVirtPkg: Add Kvmtool RTC Fdt Client Library
  2020-07-07 13:19   ` Ard Biesheuvel
@ 2020-07-09 11:34     ` Sami Mujawar
  0 siblings, 0 replies; 25+ messages in thread
From: Sami Mujawar @ 2020-07-09 11:34 UTC (permalink / raw)
  To: devel@edk2.groups.io, Ard Biesheuvel
  Cc: leif@nuviainc.com, lersek@redhat.com, Alexandru Elisei,
	Andre Przywara, Matteo Carlini, Laura Moretta, nd

Hi Ard,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel via groups.io
Sent: 07 July 2020 02:20 PM
To: Sami Mujawar <Sami.Mujawar@arm.com>; devel@edk2.groups.io
Cc: leif@nuviainc.com; lersek@redhat.com; Alexandru Elisei <Alexandru.Elisei@arm.com>; Andre Przywara <Andre.Przywara@arm.com>; Matteo Carlini <Matteo.Carlini@arm.com>; Laura Moretta <Laura.Moretta@arm.com>; nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH v4 02/15] ArmVirtPkg: Add Kvmtool RTC Fdt Client Library

On 7/7/20 3:47 PM, Sami Mujawar wrote:
> Add library that parses the Kvmtool device tree and updates the 
> dynamic PCDs describing the RTC Memory map.
> 
> It also maps the MMIO region used by the RTC as runtime memory so that 
> the RTC registers are accessible post ExitBootServices.
> 
> Since UEFI takes ownership of the RTC hardware disable the RTC node in 
> the DT to prevent the OS from attaching its device driver as well.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> 
> Notes:
>      v4:
>        - Updated based on review comments.                             [Sami]
>        - Cleanup include file list, make local functions static,       [Ard]
>          drop EFIAPI, use EFI_PAGE_MASK and use ReadUnaligned64.
>          Ref: https://edk2.groups.io/g/devel/topic/75081472
>      

You haven't answered my question about adding and removing the GCD memory space.
[SAMI] The call to gDS->AddMemorySpace () was failing on my experimental branch to enable PCI support for the kvmtool firmware.
With the current kvmtool implementation, when PCI is enabled, some part of the PCI IO region is overlapping with the region used by the RTC controller.
Since this patch series is not adding PCI support for kvmtool firmware, this issue cannot be seen.
[/SAMI]

>      v3:
>        - Introduce library to read and map the MMIO base addresses     [Sami]
>          for the RTC registers from the DT and configure the Index
>          and Target register PCDs.
> 
>   ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c   | 230 ++++++++++++++++++++
>   ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf |  42 ++++
>   2 files changed, 272 insertions(+)
> 
> diff --git a/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..93715958952f933007ff143a22cbe81606cc539e
> --- /dev/null
> +++ b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c
> @@ -0,0 +1,230 @@
> +/** @file
> +  FDT client library for motorola,mc146818 RTC driver
> +
> +  Copyright (c) 2020, ARM Limited. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DxeServicesTableLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Protocol/FdtClient.h>
> +
> +/** RTC Index register is at offset 0x0
> +*/
> +#define RTC_INDEX_REG_OFFSET    0x0ULL
> +
> +/** RTC Target register is at offset 0x1
> +*/
> +#define RTC_TARGET_REG_OFFSET   0x1ULL
> +
> +/** Add the RTC controller address range to the memory map.
> +
> +  @param [in]  ImageHandle  The handle to the image.
> +  @param [in]  RtcPageBase  Base address of the RTC controller.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
> +  @retval EFI_NOT_FOUND           Flash device not found.
> +**/
> +STATIC
> +EFI_STATUS
> +KvmtoolRtcMapMemory (
> +  IN EFI_HANDLE               ImageHandle,
> +  IN EFI_PHYSICAL_ADDRESS     RtcPageBase
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = gDS->AddMemorySpace (
> +                  EfiGcdMemoryTypeMemoryMappedIo,
> +                  RtcPageBase,
> +                  EFI_PAGE_SIZE,
> +                  EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR, "Failed to add memory space. Status = %r\n",
> +      Status
> +      ));
> +    return Status;
> +  }
> +
> +  Status = gDS->AllocateMemorySpace (
> +                  EfiGcdAllocateAddress,
> +                  EfiGcdMemoryTypeMemoryMappedIo,
> +                  0,
> +                  EFI_PAGE_SIZE,
> +                  &RtcPageBase,
> +                  ImageHandle,
> +                  NULL
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Failed to allocate memory space. Status = %r\n",
> +      Status
> +      ));
> +    gDS->RemoveMemorySpace (
> +           RtcPageBase,
> +           EFI_PAGE_SIZE
> +           );
> +    return Status;
> +  }
> +
> +  Status = gDS->SetMemorySpaceAttributes (
> +                  RtcPageBase,
> +                  EFI_PAGE_SIZE,
> +                  EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Failed to set memory attributes. Status = %r\n",
> +      Status
> +      ));
> +    gDS->FreeMemorySpace (
> +           RtcPageBase,
> +           EFI_PAGE_SIZE
> +           );
> +    gDS->RemoveMemorySpace (
> +           RtcPageBase,
> +           EFI_PAGE_SIZE
> +           );
> +  }
> +
> +  return Status;
> +}
> +
> +/** Entrypoint for KvmtoolRtcFdtClientLib.
> +
> +  Locate the RTC node in the DT and update the Index and
> +  Target register base addresses in the respective PCDs.
> +  Add the RTC memory region to the memory map.
> +  Disable the RTC node as the RTC is owned by UEFI.
> +
> +  @param [in]  ImageHandle  The handle to the image.
> +  @param [in]  SystemTable  Pointer to the System Table.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
> +  @retval EFI_NOT_FOUND           Flash device not found.
> +**/
> +EFI_STATUS
> +EFIAPI
> +KvmtoolRtcFdtClientLibConstructor (
> +  IN EFI_HANDLE         ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  EFI_STATUS            Status;
> +  FDT_CLIENT_PROTOCOL   *FdtClient;
> +  INT32                 Node;
> +  CONST UINT32          *Reg;
> +  UINT32                RegSize;
> +  UINT64                RegBase;
> +  UINT64                Range;
> +  RETURN_STATUS         PcdStatus;
> +
> +  Status = gBS->LocateProtocol (
> +                  &gFdtClientProtocolGuid,
> +                  NULL,
> +                  (VOID **)&FdtClient
> +                  );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  Status = FdtClient->FindCompatibleNode (
> +                        FdtClient,
> +                        "motorola,mc146818",
> +                        &Node
> +                        );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: No 'motorola,mc146818' compatible DT node found\n",
> +      __FUNCTION__
> +      ));
> +    return Status;
> +  }
> +
> +  Status = FdtClient->GetNodeProperty (
> +                        FdtClient,
> +                        Node,
> +                        "reg",
> +                        (CONST VOID **)&Reg,
> +                        &RegSize
> +                        );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: No 'reg' property found in 'motorola,mc146818' compatible DT node\n",
> +      __FUNCTION__
> +      ));
> +    return Status;
> +  }
> +
> +  ASSERT (RegSize == 16);
> +
> +  RegBase = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[0]));
> +  Range = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[2]));
> +  DEBUG ((
> +    DEBUG_INFO,
> +    "Found motorola,mc146818 RTC @ 0x%Lx Range = 0x%x\n",
> +    RegBase,
> +    Range
> +    ));
> +
> +  // The address range must cover the RTC Index and the Target registers.
> +  ASSERT (Range >= 0x2);
> +
> +  // RTC Index register is at offset 0x0
> +  PcdStatus = PcdSet64S (
> +                PcdRtcIndexRegister64,
> +                (RegBase + RTC_INDEX_REG_OFFSET)
> +                );
> +  ASSERT_RETURN_ERROR (PcdStatus);
> +
> +  // RTC Target register is at offset 0x1
> +  PcdStatus = PcdSet64S (
> +                PcdRtcTargetRegister64,
> +                (RegBase + RTC_TARGET_REG_OFFSET)
> +                );
> +  ASSERT_RETURN_ERROR (PcdStatus);
> +
> +  Status = KvmtoolRtcMapMemory (ImageHandle, (RegBase & ~EFI_PAGE_MASK));
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Failed to map memory for motorola,mc146818. Status = %r\n",
> +      Status
> +      ));
> +    return Status;
> +  }
> +
> +  //
> +  // UEFI takes ownership of the RTC hardware, and exposes its functionality
> +  // through the UEFI Runtime Services GetTime, SetTime, etc. This means we
> +  // need to disable it in the device tree to prevent the OS from attaching
> +  // its device driver as well.
> +  //
> +  Status = FdtClient->SetNodeProperty (
> +                        FdtClient,
> +                        Node,
> +                        "status",
> +                        "disabled",
> +                        sizeof ("disabled")
> +                        );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_WARN,
> +      "Failed to set motorola,mc146818 status to 'disabled', Status = %r\n",
> +      Status
> +      ));
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf
> new file mode 100644
> index 0000000000000000000000000000000000000000..0d1b27997ed00d7c1be3578cab4d1d2eba663a90
> --- /dev/null
> +++ b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf
> @@ -0,0 +1,42 @@
> +#/** @file
> +#  FDT client library for motorola,mc146818 RTC driver
> +#
> +#  Copyright (c) 2020, ARM Limited. All rights reserved.<BR>
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001B
> +  BASE_NAME                      = KvmtoolRtcFdtClientLib
> +  FILE_GUID                      = 3254B4F7-30B5-48C6-B06A-D8FF97F3EF95
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = KvmtoolRtcFdtClientLib|DXE_DRIVER DXE_RUNTIME_DRIVER
> +  CONSTRUCTOR                    = KvmtoolRtcFdtClientLibConstructor
> +
> +[Sources]
> +  KvmtoolRtcFdtClientLib.c
> +
> +[Packages]
> +  ArmVirtPkg/ArmVirtPkg.dec
> +  MdePkg/MdePkg.dec
> +  PcAtChipsetPkg/PcAtChipsetPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  PcdLib
> +  UefiBootServicesTableLib
> +  DxeServicesTableLib
> +
> +[Protocols]
> +  gFdtClientProtocolGuid                                ## CONSUMES
> +
> +[Pcd]
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64
> +
> +[Depex]
> +  gFdtClientProtocolGuid
> 





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

* Re: [PATCH v4 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver
  2020-07-07 13:16   ` Ard Biesheuvel
@ 2020-07-09 11:37     ` Sami Mujawar
  0 siblings, 0 replies; 25+ messages in thread
From: Sami Mujawar @ 2020-07-09 11:37 UTC (permalink / raw)
  To: Ard Biesheuvel, devel@edk2.groups.io
  Cc: leif@nuviainc.com, ray.ni@intel.com, Alexandru Elisei,
	Andre Przywara, Matteo Carlini, Laura Moretta, nd

Hi Ard,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar

-----Original Message-----
From: Ard Biesheuvel <ard.biesheuvel@arm.com> 
Sent: 07 July 2020 02:17 PM
To: Sami Mujawar <Sami.Mujawar@arm.com>; devel@edk2.groups.io
Cc: leif@nuviainc.com; ray.ni@intel.com; Alexandru Elisei <Alexandru.Elisei@arm.com>; Andre Przywara <Andre.Przywara@arm.com>; Matteo Carlini <Matteo.Carlini@arm.com>; Laura Moretta <Laura.Moretta@arm.com>; nd <nd@arm.com>
Subject: Re: [PATCH v4 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver

On 7/7/20 3:47 PM, Sami Mujawar wrote:
> Some virtual machine managers like Kvmtool emulate the MC146818 RTC 
> controller in the MMIO space so that architectures that do not support 
> I/O Mapped I/O can use the RTC. This patch adds MMIO support to the 
> RTC controller driver.
> 
> The PCD PcdRtcUseMmio has been added to select I/O or MMIO support.
>    If PcdRtcUseMmio is:
>      TRUE  - Indicates the RTC port registers are in MMIO space.
>      FALSE - Indicates the RTC port registers are in I/O space.
>              Default is I/O space.
> 
> Additionally two new PCDs PcdRtcIndexRegister64 and
> PcdRtcTargetRegister64 have been introduced to provide the base 
> address for the RTC registers in the MMIO space.
> 
> When MMIO support is selected (PcdRtcUseMmio == TRUE) the driver 
> converts the pointers to the RTC MMIO registers so that the RTC 
> registers are accessible post ExitBootServices.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>

This code looks good to me now, but please drop the unnecessary whitespace and header changes to PcRtc.h, and the whitespace changes to PcRtcEntry.c

[SAMI] I will fix this and repost the patch.

With that,

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>


> ---
> 
> Notes:
>      v4:
>        - Updated based on review comments.                             [Sami]
>        - Use static helper functions instead of function pointers.     [Ard]
>          Ref: https://edk2.groups.io/g/devel/topic/75081468
>      
>      v3:
>        - Make PcdRtcUseMmio a feature PCD.                             [Sami]
>        - Read the RTC MMIO base address from the DT.                   [Andre]
>        - Introduce PCDs for RTC Index and Target register base         [Sami]
>          address in the MMIO space.
>        - Move RTC MMIO region mapping code to a separate platform      [Sami]
>          specific library. This library also reads the base addresses
>          for the RTC from DT and configures the RTC Index and Target
>          register PCDs.
>          Ref: https://edk2.groups.io/g/devel/topic/74200905#60307
>      
>      v2:
>        - Code review comments incorporated.                            [Sami]
>      
>      v1:
>        - Add support to read/write from RTC registers using            [Sami]
>          MMIO access
>        - Use wrapper functions for RtcRead/Write accessors             [Leif]
>          Ref: https://edk2.groups.io/g/devel/topic/30915281#30695
> 
>   PcAtChipsetPkg/PcAtChipsetPkg.dec                                          |  16 +++
>   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c                         | 120 ++++++++++++++++++--
>   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h                         |   2 +
>   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c                    |  58 +++++++++-
>   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf |   8 ++
>   5 files changed, 190 insertions(+), 14 deletions(-)
> 
> diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dec 
> b/PcAtChipsetPkg/PcAtChipsetPkg.dec
> index 
> 88de5cceea593176c3a2425a5963b66b789f2b9e..ed2d95550b8d153995b30cdc290c
> f3bb905e211b 100644
> --- a/PcAtChipsetPkg/PcAtChipsetPkg.dec
> +++ b/PcAtChipsetPkg/PcAtChipsetPkg.dec
> @@ -6,6 +6,7 @@
>   #
>   # Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
>   # Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> +# Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR>
>   #
>   # SPDX-License-Identifier: BSD-2-Clause-Patent
>   #
> @@ -41,6 +42,13 @@ [PcdsFeatureFlag]
>     # @Prompt Configure HPET to use MSI.
>     
> gPcAtChipsetPkgTokenSpaceGuid.PcdHpetMsiEnable|TRUE|BOOLEAN|0x00001000
>   
> +  ## Indicates the RTC port registers are in MMIO space, or in I/O space.
> +  #  Default is I/O space.<BR><BR>
> +  #   TRUE  - RTC port registers are in MMIO space.<BR>
> +  #   FALSE - RTC port registers are in I/O space.<BR>
> +  # @Prompt RTC port registers use MMIO.
> +  
> + gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|FALSE|BOOLEAN|0x00000021
> +
>   [PcdsFixedAtBuild, PcdsDynamic, PcdsDynamicEx, PcdsPatchableInModule]
>     ## This PCD specifies the base address of the HPET timer.
>     # @Prompt HPET base address.
> @@ -68,6 +76,14 @@ [PcdsFixedAtBuild, PcdsDynamic, PcdsDynamicEx, PcdsPatchableInModule]
>     # @Expression 0x80000001 | gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear < gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear + 100
>     
> gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2097|UINT16|0x000000
> 0E
>   
> +  ## Specifies RTC Index Register address in MMIO space.
> +  # @Prompt RTC Index Register address
> +  
> + gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0|UINT64|0x000
> + 00022
> +
> +  ## Specifies RTC Target Register address in MMIO space.
> +  # @Prompt RTC Target Register address
> +  
> + gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64|0x0|UINT64|0x00
> + 000023
> +
>   [PcdsFixedAtBuild, PcdsPatchableInModule]
>     ## Defines the ACPI register set base address.
>     #  The invalid 0xFFFF is as its default value. It must be configured to the real value.
> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c 
> b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> index 
> 52af17941786ef81c3911512ee64551724e67209..64f36f6fbbd1b03967bd1a1290d1
> 08d5b0f294fa 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> @@ -3,6 +3,7 @@
>   
>   Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
>   Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> +Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR>
>   
>   SPDX-License-Identifier: BSD-2-Clause-Patent
>   
> @@ -10,6 +11,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>   
>   #include "PcRtc.h"
>   
> +extern UINTN  mRtcIndexRegister;
> +extern UINTN  mRtcTargetRegister;
> +
>   //
>   // Days of month.
>   //
> @@ -54,38 +58,132 @@ IsWithinOneDay (
>     );
>   
>   /**
> +  Read RTC content through its registers using IO access.
> +
> +  @param  Address   Address offset of RTC. It is recommended to use
> +                    macros such as RTC_ADDRESS_SECONDS.
> +
> +  @return The data of UINT8 type read from RTC.
> +**/
> +STATIC
> +UINT8
> +IoRtcRead (
> +  IN  UINTN Address
> +  )
> +{
> +  IoWrite8 (
> +    PcdGet8 (PcdRtcIndexRegister),
> +    (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80))
> +    );
> +  return IoRead8 (PcdGet8 (PcdRtcTargetRegister)); }
> +
> +/**
> +  Write RTC through its registers  using IO access.
> +
> +  @param  Address   Address offset of RTC. It is recommended to use
> +                    macros such as RTC_ADDRESS_SECONDS.
> +  @param  Data      The content you want to write into RTC.
> +
> +**/
> +STATIC
> +VOID
> +IoRtcWrite (
> +  IN  UINTN   Address,
> +  IN  UINT8   Data
> +  )
> +{
> +  IoWrite8 (
> +    PcdGet8 (PcdRtcIndexRegister),
> +    (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80))
> +    );
> +  IoWrite8 (PcdGet8 (PcdRtcTargetRegister), Data); }
> +
> +/**
> +  Read RTC content through its registers using MMIO access.
> +
> +  @param  Address   Address offset of RTC. It is recommended to use
> +                    macros such as RTC_ADDRESS_SECONDS.
> +
> +  @return The data of UINT8 type read from RTC.
> +**/
> +STATIC
> +UINT8
> +MmioRtcRead (
> +  IN  UINTN Address
> +  )
> +{
> +  MmioWrite8 (
> +    mRtcIndexRegister,
> +    (UINT8)(Address | (UINT8)(MmioRead8 (mRtcIndexRegister) & 0x80))
> +    );
> +  return MmioRead8 (mRtcTargetRegister); }
> +
> +/**
> +  Write RTC through its registers using MMIO access.
> +
> +  @param  Address   Address offset of RTC. It is recommended to use
> +                    macros such as RTC_ADDRESS_SECONDS.
> +  @param  Data      The content you want to write into RTC.
> +
> +**/
> +STATIC
> +VOID
> +MmioRtcWrite (
> +  IN  UINTN   Address,
> +  IN  UINT8   Data
> +  )
> +{
> +  MmioWrite8 (
> +    mRtcIndexRegister,
> +    (UINT8)(Address | (UINT8)(MmioRead8 (mRtcIndexRegister) & 0x80))
> +    );
> +  MmioWrite8 (mRtcTargetRegister, Data); }
> +
> +/**
>     Read RTC content through its registers.
>   
> -  @param  Address  Address offset of RTC. It is recommended to use macros such as
> -                   RTC_ADDRESS_SECONDS.
> +  @param  Address   Address offset of RTC. It is recommended to use
> +                    macros such as RTC_ADDRESS_SECONDS.
>   
>     @return The data of UINT8 type read from RTC.
>   **/
> +STATIC
>   UINT8
>   RtcRead (
> -  IN  UINT8 Address
> +  IN  UINTN Address
>     )
>   {
> -  IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8) 
> (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
> -  return IoRead8 (PcdGet8 (PcdRtcTargetRegister));
> +  if (FeaturePcdGet (PcdRtcUseMmio)) {
> +    return MmioRtcRead (Address);
> +  }
> +
> +  return IoRtcRead (Address);
>   }
>   
>   /**
>     Write RTC through its registers.
>   
> -  @param  Address  Address offset of RTC. It is recommended to use macros such as
> -                   RTC_ADDRESS_SECONDS.
> -  @param  Data     The content you want to write into RTC.
> +  @param  Address   Address offset of RTC. It is recommended to use
> +                    macros such as RTC_ADDRESS_SECONDS.
> +  @param  Data      The content you want to write into RTC.
>   
>   **/
> +STATIC
>   VOID
>   RtcWrite (
> -  IN  UINT8   Address,
> +  IN  UINTN   Address,
>     IN  UINT8   Data
>     )
>   {
> -  IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8) 
> (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
> -  IoWrite8 (PcdGet8 (PcdRtcTargetRegister), Data);
> +  if (FeaturePcdGet (PcdRtcUseMmio)) {
> +    MmioRtcWrite (Address, Data);
> +  } else {
> +    IoRtcWrite (Address, Data);
> +  }
>   }
>   
>   /**
> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h 
> b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h
> index 
> 47293ce44c5a1f4792892892f7da40d7f0a5a001..c1cb9466b74587233de4e78c0c0d
> 537ca6b66cf3 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h
> @@ -3,6 +3,7 @@
>   
>   Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
>   Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> +Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.<BR>
>   
>   SPDX-License-Identifier: BSD-2-Clause-Patent
>   
> @@ -371,4 +372,5 @@ PcRtcAcpiTableChangeCallback (
>     IN EFI_EVENT        Event,
>     IN VOID             *Context
>     );
> +
>   #endif
> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c 
> b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> index 
> ccda6331373bfe4069b0a59495b5e5cc731c8fc8..9c5f1ff169c80d1a9580d90336f1
> 62968b63bfa2 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> @@ -2,16 +2,23 @@
>     Provides Set/Get time operations.
>   
>   Copyright (c) 2006 - 2018, Intel Corporation. All rights 
> reserved.<BR>
> +Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR>
>   SPDX-License-Identifier: BSD-2-Clause-Patent
>   
>   **/
>   
> +#include <Library/DxeServicesTableLib.h>
>   #include "PcRtc.h"
>   
>   PC_RTC_MODULE_GLOBALS  mModuleGlobal;
>   
>   EFI_HANDLE             mHandle = NULL;
>   
> +STATIC EFI_EVENT       mVirtualAddrChangeEvent;
> +
> +UINTN                  mRtcIndexRegister;
> +UINTN                  mRtcTargetRegister;
> +
>   /**
>     Returns the current time and date information, and the time-keeping capabilities
>     of the hardware platform.
> @@ -106,6 +113,30 @@ PcRtcEfiSetWakeupTime (
>   }
>   
>   /**
> +  Fixup internal data so that EFI can be called in virtual mode.
> +  Call the passed in Child Notify event and convert any pointers in  
> + lib to virtual mode.
> +
> +  @param[in]    Event   The Event that is being processed
> +  @param[in]    Context Event Context
> +**/
> +VOID
> +EFIAPI
> +LibRtcVirtualNotifyEvent (
> +  IN EFI_EVENT        Event,
> +  IN VOID             *Context
> +  )
> +{
> +  // Only needed if you are going to support the OS calling RTC 
> +functions in
> +  // virtual mode. You will need to call EfiConvertPointer (). To 
> +convert any
> +  // stored physical addresses to virtual address. After the OS 
> +transitions to
> +  // calling in virtual mode, all future runtime calls will be made 
> +in virtual
> +  // mode.
> +  EfiConvertPointer (0x0, (VOID**)&mRtcIndexRegister);
> +  EfiConvertPointer (0x0, (VOID**)&mRtcTargetRegister); }
> +
> +/**
>     The user Entry Point for PcRTC module.
>   
>     This is the entry point for PcRTC module. It installs the UEFI 
> runtime service @@ -125,12 +156,17 @@ InitializePcRtc (
>     IN EFI_SYSTEM_TABLE                      *SystemTable
>     )
>   {
> -  EFI_STATUS  Status;
> -  EFI_EVENT   Event;
> +  EFI_STATUS             Status;
> +  EFI_EVENT              Event;
>   
>     EfiInitializeLock (&mModuleGlobal.RtcLock, TPL_CALLBACK);
>     mModuleGlobal.CenturyRtcAddress = GetCenturyRtcAddress ();
>   
> +  if (FeaturePcdGet (PcdRtcUseMmio)) {
> +    mRtcIndexRegister = (UINTN)PcdGet64 (PcdRtcIndexRegister64);
> +    mRtcTargetRegister = (UINTN)PcdGet64 (PcdRtcTargetRegister64);  }
> +
>     Status = PcRtcInit (&mModuleGlobal);
>     ASSERT_EFI_ERROR (Status);
>   
> @@ -165,7 +201,23 @@ InitializePcRtc (
>                     NULL,
>                     NULL
>                     );
> -  ASSERT_EFI_ERROR (Status);
> +  if (EFI_ERROR (Status)) {
> +    ASSERT_EFI_ERROR (Status);
> +    return Status;
> +  }
> +
> +  if (FeaturePcdGet (PcdRtcUseMmio)) {
> +    // Register for the virtual address change event
> +    Status = gBS->CreateEventEx (
> +                    EVT_NOTIFY_SIGNAL,
> +                    TPL_NOTIFY,
> +                    LibRtcVirtualNotifyEvent,
> +                    NULL,
> +                    &gEfiEventVirtualAddressChangeGuid,
> +                    &mVirtualAddrChangeEvent
> +                    );
> +    ASSERT_EFI_ERROR (Status);
> +  }
>   
>     return Status;
>   }
> diff --git 
> a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeD
> xe.inf 
> b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeD
> xe.inf index 
> c73ee98105e510f9e4e23c1a6c1e5c505325d2c9..0d8eca28b65954b073a72fc4fe5a
> d6247320e79d 100644
> --- 
> a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeD
> xe.inf
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRunt
> +++ imeDxe.inf
> @@ -6,6 +6,7 @@
>   #
>   # Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>   # Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> +# Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR>
>   #
>   # SPDX-License-Identifier: BSD-2-Clause-Patent
>   #
> @@ -61,6 +62,11 @@ [Guids]
>     ## SOMETIMES_CONSUMES ## SystemTable
>     gEfiAcpiTableGuid
>   
> +  gEfiEventVirtualAddressChangeGuid
> +
> +[FeaturePcd]
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio                   ## CONSUMES
> +
>   [FixedPcd]
>     gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterA     ## CONSUMES
>     gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterB     ## CONSUMES
> @@ -72,6 +78,8 @@ [Pcd]
>     gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear             ## CONSUMES
>     gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister             ## CONSUMES
>     gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister            ## CONSUMES
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64           ## CONSUMES
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64          ## CONSUMES
>   
>   [Depex]
>     gEfiVariableArchProtocolGuid AND gEfiVariableWriteArchProtocolGuid
> 


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

* Re: [edk2-devel] [PATCH v4 07/15] MdeModulePkg: Fix constructor invocation ordering
       [not found] ` <161F794363CF42EE.6803@groups.io>
@ 2020-07-10 17:34   ` Sami Mujawar
  0 siblings, 0 replies; 25+ messages in thread
From: Sami Mujawar @ 2020-07-10 17:34 UTC (permalink / raw)
  To: devel@edk2.groups.io, Hao A Wu, Ray Ni, jian.j.wang@intel.com,
	hao.a.wu@intel.com
  Cc: Ard Biesheuvel, leif@nuviainc.com, lersek@redhat.com,
	Alexandru Elisei, Andre Przywara, Matteo Carlini, Laura Moretta,
	nd

Hi Hao, Jian,

Can you let me know your feedback for this patch, please?
Previous discussion on this patch can be seen at https://edk2.groups.io/g/devel/topic/75081484

Regards,

Sami Mujawar

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sami Mujawar via groups.io
Sent: 07 July 2020 01:48 PM
To: devel@edk2.groups.io
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; Ard Biesheuvel <Ard.Biesheuvel@arm.com>; leif@nuviainc.com; lersek@redhat.com; jian.j.wang@intel.com; hao.a.wu@intel.com; Alexandru Elisei <Alexandru.Elisei@arm.com>; Andre Przywara <Andre.Przywara@arm.com>; Matteo Carlini <Matteo.Carlini@arm.com>; Laura Moretta <Laura.Moretta@arm.com>; nd <nd@arm.com>
Subject: [edk2-devel] [PATCH v4 07/15] MdeModulePkg: Fix constructor invocation ordering

The BaseSerialPortLib16550 library does not implement a constructor. This prevents the correct constructor invocation order for dependent libraries.
e.g. A PlatformHookLib (for the Serial Port) may have a dependency on retrieving data from a Hob. A Hob library implementation may configure its initial state in the HobLib constructor. Since BaseSerialPortLib16550 does not implement a constructor, the Basetools do not resolve the correct order for constructor invocation.

To fix this, add an empty constructor to the serial port library BaseSerialPortLib16550.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Acked-by: Ard Biesheuvel <Ard.Biesheuvel@arm.com>
---

Notes:
    v4:
      - No code change, resending patch with v4 series.               [Sami]
        Ref: https://edk2.groups.io/g/devel/topic/75081484

 MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c   | 17 +++++++++++++++++
 MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf |  3 +++
 2 files changed, 20 insertions(+)

diff --git a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
index 9cb50dd80d5634ab2aa6d68bf5ca7fb891463eef..0fd1382ee83c9de09d8250830bd9569056fcee2f 100644
--- a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
+++ b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550
+++ .c
@@ -4,6 +4,7 @@
   (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
   Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
   Copyright (c) 2018, AMD Incorporated. All rights reserved.<BR>
+  Copyright (c) 2020, ARM Limited. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -1102,3 +1103,19 @@ SerialPortSetAttributes (
   return RETURN_SUCCESS;
 }
 
+/** Base Serial Port 16550 Library Constructor
+
+  @retval RETURN_SUCCESS  Success.
+*/
+EFI_STATUS
+EFIAPI
+BaseSerialPortLib16550 (
+  VOID
+  )
+{
+  // Nothing to do here. This constructor is added to
+  // enable the chain of constructor invocation for
+  // dependent libraries.
+  return RETURN_SUCCESS;
+}
+
diff --git a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
index 8b4ae3f1d4ee1e2e9a8b81eab4c900541ce8cfb6..92b7a8b7896a305d2ce22589f8a9593618d37bb7 100644
--- a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
+++ b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550
+++ .inf
@@ -2,6 +2,8 @@
 #  SerialPortLib instance for 16550 UART.
 #
 #  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2020, ARM Limited. All rights reserved.
+#
 #  SPDX-License-Identifier: BSD-2-Clause-Patent  #  ## @@ -14,6 +16,7 @@ [Defines]
   MODULE_TYPE                    = BASE
   VERSION_STRING                 = 1.1
   LIBRARY_CLASS                  = SerialPortLib
+  CONSTRUCTOR                    = BaseSerialPortLib16550
 
 [Packages]
   MdePkg/MdePkg.dec
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'





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

end of thread, other threads:[~2020-07-10 17:34 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-07 12:47 [PATCH v4 00/15] Kvmtool guest firmware support for Arm Sami Mujawar
2020-07-07 12:47 ` [PATCH v4 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver Sami Mujawar
2020-07-07 13:16   ` Ard Biesheuvel
2020-07-09 11:37     ` Sami Mujawar
2020-07-07 12:47 ` [PATCH v4 02/15] ArmVirtPkg: Add Kvmtool RTC Fdt Client Library Sami Mujawar
2020-07-07 13:19   ` Ard Biesheuvel
2020-07-09 11:34     ` [edk2-devel] " Sami Mujawar
2020-07-07 12:47 ` [PATCH v4 03/15] ArmPlatformPkg: Dynamic flash variable base Sami Mujawar
2020-07-07 12:47 ` [PATCH v4 04/15] ArmVirtPkg: Add kvmtool platform driver Sami Mujawar
2020-07-07 13:22   ` Ard Biesheuvel
2020-07-07 12:47 ` [PATCH v4 05/15] ArmVirtPkg: kvmtool platform memory map Sami Mujawar
2020-07-07 12:48 ` [PATCH v4 06/15] ArmVirtPkg: Add Kvmtool NOR flash lib Sami Mujawar
2020-07-07 13:26   ` Ard Biesheuvel
2020-07-07 12:48 ` [PATCH v4 07/15] MdeModulePkg: Fix constructor invocation ordering Sami Mujawar
2020-07-07 12:48 ` [PATCH v4 08/15] ArmVirtPkg: GUID Hob for 16550 UART base address Sami Mujawar
2020-07-07 12:48 ` [PATCH v4 09/15] ArmVirtPkg: 16550 UART Platform hook library Sami Mujawar
2020-07-07 12:48 ` [PATCH v4 10/15] ArmVirtPkg: Add Kvmtool Platform Pei Lib Sami Mujawar
2020-07-07 12:48 ` [PATCH v4 11/15] ArmVirtPkg: Support for kvmtool virtual platform Sami Mujawar
2020-07-07 13:27   ` Ard Biesheuvel
2020-07-07 12:48 ` [PATCH v4 12/15] ArmVirtPkg: Package dependency for MC146818 RTC Sami Mujawar
2020-07-07 12:48 ` [PATCH v4 13/15] ArmVirtPkg: Add kvmtool to package dictionary Sami Mujawar
2020-07-07 12:48 ` [PATCH v4 14/15] .python/SpellCheck: Add 'XIPFLAGS' to "words" section Sami Mujawar
2020-07-07 13:28   ` Ard Biesheuvel
2020-07-07 12:48 ` [PATCH v4 15/15] Maintainer.txt: Add Kvmtool platform reviewer Sami Mujawar
     [not found] ` <161F794363CF42EE.6803@groups.io>
2020-07-10 17:34   ` [edk2-devel] [PATCH v4 07/15] MdeModulePkg: Fix constructor invocation ordering Sami Mujawar

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