public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 resend 00/11] ArmVirtPkg/ArmVirtQemu: Performance streamlining
@ 2022-10-19  9:22 Ard Biesheuvel
  2022-10-19  9:22 ` [PATCH v3 resend 01/11] ArmVirtPkg: remove EbcDxe from all platforms Ard Biesheuvel
                   ` (10 more replies)
  0 siblings, 11 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2022-10-19  9:22 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Leif Lindholm, Alexander Graf, Gerd Hoffmann,
	Sami Mujawar

We currently do a substantial amount of processing before enabling the
MMU and caches, which is bad for performance, but also fragile, as it
requires cache coherency to be managed in software.

It also means that when running under virtualization, the hypervisor
must do a non-trivial amount of work to ensure that the host's cached
view of memory is consistent with the guest's uncached view.

Now that the AArch64 version of ArmMmuLib has been updated to allow it
to be invoked with the MMU already enabled, and to no longer disable the
MMU temporarily to make modifications to the page tables that require
break before make (BBM) [e.g., to split block mappings into table
mappings], we can remove all remaining manipulations of main memory that
occur with the MMU and caches disabled, by using a compile time
generated ID map that covers the first bank of NOR flash, the first MMIO
region (for the UART), and the first 128 MiB of DRAM, and switching to
it straight out of reset.

The resulting build no longer performs any non-coherent memory accesses
via the data side, and only relies on instruction fetches before the MMU
is enabled. It also avoids any cache maintenance to the PoC.

Changes since v3:
- drop ArmMmuLib changes that have since been merged

Changes since v2:
- drop shadow page table approach - it only works at EL1, and is a bit
  more intrusive than needed; instead, do a proper break-before-make
  (BBM) unless the break unmaps the page table itself or the code that
  is modifying it;
- add a couple of only tangentially related performance streamlining
  changes, to avoid dispatching and shadowing drivers that we don't need

Changes since v1:
- coding style tweaks to placate our CI overlord
- drop -mstrict-align which is no longer needed now that all C code runs
  with the MMU and caches on

Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Alexander Graf <agraf@csgraf.de>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>

Ard Biesheuvel (11):
  ArmVirtPkg: remove EbcDxe from all platforms
  ArmVirtPkg: do not enable iSCSI driver by default
  ArmVirtPkg: make EFI_LOADER_DATA non-executable
  ArmVirtPkg/ArmVirtQemu: wire up timeout PCD to Timeout variable
  ArmVirtPkg/ArmVirtQemu: implement ArmPlatformLib with static ID map
  ArmVirtPkg/ArmVirtQemu: use first 128 MiB as permanent PEI memory
  ArmVirtPkg/ArmVirtQemu: enable initial ID map at early boot
  ArmVirtPkg/ArmVirtQemu: Drop unused variable PEIM
  ArmVirtPkg/ArmVirtQemu: avoid shadowing PEIMs unless necessary
  ArmVirtPkg/QemuVirtMemInfoLib: use HOB not PCD to record the memory
    size
  ArmVirtPkg/ArmVirtQemu: omit PCD PEIM unless TPM support is enabled

 ArmVirtPkg/ArmVirt.dsc.inc                                                                                   |   7 +-
 ArmVirtPkg/ArmVirtCloudHv.fdf                                                                                |   5 -
 ArmVirtPkg/ArmVirtPkg.dec                                                                                    |   1 +
 ArmVirtPkg/ArmVirtQemu.dsc                                                                                   |  53 ++++++---
 ArmVirtPkg/ArmVirtQemu.fdf                                                                                   |   5 +-
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc                                                                         |   5 -
 ArmVirtPkg/ArmVirtQemuKernel.dsc                                                                             |   1 -
 ArmVirtPkg/ArmVirtXen.fdf                                                                                    |   5 -
 ArmVirtPkg/Library/ArmPlatformLibQemu/AArch64/ArmPlatformHelper.S                                            | 115 ++++++++++++++++++++
 ArmVirtPkg/Library/ArmPlatformLibQemu/ArmPlatformLibQemu.c                                                   |  64 +++++++++++
 ArmVirtPkg/Library/ArmPlatformLibQemu/ArmPlatformLibQemu.inf                                                 |  40 +++++++
 ArmVirtPkg/Library/ArmPlatformLibQemu/IdMap.S                                                                |  57 ++++++++++
 ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c                                         |  14 ++-
 ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf                                       |   1 +
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c                                                   |  35 +++++-
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf                                                 |   5 +-
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf                                              |   8 +-
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLibConstructor.c                                     |  30 +++--
 ArmVirtPkg/MemoryInitPei/MemoryInitPeim.c                                                                    | 104 ++++++++++++++++++
 ArmVirtPkg/{Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf => MemoryInitPei/MemoryInitPeim.inf} |  36 +++---
 20 files changed, 510 insertions(+), 81 deletions(-)
 create mode 100644 ArmVirtPkg/Library/ArmPlatformLibQemu/AArch64/ArmPlatformHelper.S
 create mode 100644 ArmVirtPkg/Library/ArmPlatformLibQemu/ArmPlatformLibQemu.c
 create mode 100644 ArmVirtPkg/Library/ArmPlatformLibQemu/ArmPlatformLibQemu.inf
 create mode 100644 ArmVirtPkg/Library/ArmPlatformLibQemu/IdMap.S
 create mode 100644 ArmVirtPkg/MemoryInitPei/MemoryInitPeim.c
 copy ArmVirtPkg/{Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf => MemoryInitPei/MemoryInitPeim.inf} (64%)

-- 
2.35.1


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

* [PATCH v3 resend 01/11] ArmVirtPkg: remove EbcDxe from all platforms
  2022-10-19  9:22 [PATCH v3 resend 00/11] ArmVirtPkg/ArmVirtQemu: Performance streamlining Ard Biesheuvel
@ 2022-10-19  9:22 ` Ard Biesheuvel
  2022-10-20  8:58   ` [edk2-devel] " Leif Lindholm
  2022-10-19  9:22 ` [PATCH v3 resend 02/11] ArmVirtPkg: do not enable iSCSI driver by default Ard Biesheuvel
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Ard Biesheuvel @ 2022-10-19  9:22 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Leif Lindholm, Alexander Graf, Gerd Hoffmann,
	Sami Mujawar

The EBC interpreter is rarely, if ever, used on ARM, and is especially
pointless on virtual machines. So let's drop it from the builds.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmVirtPkg/ArmVirt.dsc.inc           | 5 -----
 ArmVirtPkg/ArmVirtCloudHv.fdf        | 5 -----
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 5 -----
 ArmVirtPkg/ArmVirtXen.fdf            | 5 -----
 4 files changed, 20 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index c39e2506a3ea..34575585adbb 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -422,8 +422,3 @@ [Components.AARCH64]
     <LibraryClasses>
       NULL|EmbeddedPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
   }
-
-  #
-  # EBC support
-  #
-  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
diff --git a/ArmVirtPkg/ArmVirtCloudHv.fdf b/ArmVirtPkg/ArmVirtCloudHv.fdf
index 81c539590a76..a5f172d79bfc 100644
--- a/ArmVirtPkg/ArmVirtCloudHv.fdf
+++ b/ArmVirtPkg/ArmVirtCloudHv.fdf
@@ -195,11 +195,6 @@ [FV.FvMain]
   INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
   INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
   INF ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
-
-  #
-  # EBC support
-  #
-  INF MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
 !endif
 
   #
diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
index d4df6dede0fe..787286133095 100644
--- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
+++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
@@ -146,11 +146,6 @@ [FV.FvMain]
   INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
   INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
   INF OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
-
-  #
-  # EBC support
-  #
-  INF MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
 !endif
 
   #
diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf
index 132480f03059..770fbf7289be 100644
--- a/ArmVirtPkg/ArmVirtXen.fdf
+++ b/ArmVirtPkg/ArmVirtXen.fdf
@@ -201,11 +201,6 @@ [FV.FvMain]
 !if $(ARCH) == AARCH64
   INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
   INF ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf
-
-  #
-  # EBC support
-  #
-  INF MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
 !endif
 
  #
-- 
2.35.1


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

* [PATCH v3 resend 02/11] ArmVirtPkg: do not enable iSCSI driver by default
  2022-10-19  9:22 [PATCH v3 resend 00/11] ArmVirtPkg/ArmVirtQemu: Performance streamlining Ard Biesheuvel
  2022-10-19  9:22 ` [PATCH v3 resend 01/11] ArmVirtPkg: remove EbcDxe from all platforms Ard Biesheuvel
@ 2022-10-19  9:22 ` Ard Biesheuvel
  2022-10-20  9:03   ` Leif Lindholm
  2022-10-19  9:22 ` [PATCH v3 resend 03/11] ArmVirtPkg: make EFI_LOADER_DATA non-executable Ard Biesheuvel
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Ard Biesheuvel @ 2022-10-19  9:22 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Leif Lindholm, Alexander Graf, Gerd Hoffmann,
	Sami Mujawar

The iSCSI driver slows down the boot on a pristine variable store flash
image, as it creates a couple of large EFI non-volatile variables to
preserve state between boots.

Since iSCSI boot for VMs is kind of niche anyway, let's default to
disabled. If someone needs it in their build, they can use the -D build
command option to re-enable it on the fly.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmVirtPkg/ArmVirtQemu.dsc       | 1 -
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 -
 2 files changed, 2 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 9369a88858fd..45c4a8fc84e0 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -40,7 +40,6 @@ [Defines]
   DEFINE NETWORK_SNP_ENABLE              = FALSE
   DEFINE NETWORK_TLS_ENABLE              = FALSE
   DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS  = TRUE
-  DEFINE NETWORK_ISCSI_ENABLE            = TRUE
 
 !if $(NETWORK_SNP_ENABLE) == TRUE
   !error "NETWORK_SNP_ENABLE is IA32/X64/EBC only"
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 7f7d15d6eee3..66039f07f41b 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -38,7 +38,6 @@ [Defines]
   DEFINE NETWORK_SNP_ENABLE              = FALSE
   DEFINE NETWORK_TLS_ENABLE              = FALSE
   DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS  = TRUE
-  DEFINE NETWORK_ISCSI_ENABLE            = TRUE
 
 !if $(NETWORK_SNP_ENABLE) == TRUE
   !error "NETWORK_SNP_ENABLE is IA32/X64/EBC only"
-- 
2.35.1


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

* [PATCH v3 resend 03/11] ArmVirtPkg: make EFI_LOADER_DATA non-executable
  2022-10-19  9:22 [PATCH v3 resend 00/11] ArmVirtPkg/ArmVirtQemu: Performance streamlining Ard Biesheuvel
  2022-10-19  9:22 ` [PATCH v3 resend 01/11] ArmVirtPkg: remove EbcDxe from all platforms Ard Biesheuvel
  2022-10-19  9:22 ` [PATCH v3 resend 02/11] ArmVirtPkg: do not enable iSCSI driver by default Ard Biesheuvel
@ 2022-10-19  9:22 ` Ard Biesheuvel
  2022-10-19  9:22 ` [PATCH v3 resend 04/11] ArmVirtPkg/ArmVirtQemu: wire up timeout PCD to Timeout variable Ard Biesheuvel
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2022-10-19  9:22 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Leif Lindholm, Alexander Graf, Gerd Hoffmann,
	Sami Mujawar

When the memory protections were implemented and enabled on ArmVirtQemu
5+ years ago, we had to work around the fact that GRUB at the time
expected EFI_LOADER_DATA to be executable, as that is the memory type it
allocates when loading its modules.

This has been fixed in GRUB in August 2017, so by now, we should be able
to tighten this, and remove execute permissions from EFI_LOADER_DATA
allocations.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmVirtPkg/ArmVirt.dsc.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 34575585adbb..462073517a22 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -368,7 +368,7 @@ [PcdsFixedAtBuild.common]
   # reserved ones, with the exception of LoaderData regions, of which OS loaders
   # (i.e., GRUB) may assume that its contents are executable.
   #
-  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
+  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD5
 
 [Components.common]
   #
-- 
2.35.1


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

* [PATCH v3 resend 04/11] ArmVirtPkg/ArmVirtQemu: wire up timeout PCD to Timeout variable
  2022-10-19  9:22 [PATCH v3 resend 00/11] ArmVirtPkg/ArmVirtQemu: Performance streamlining Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2022-10-19  9:22 ` [PATCH v3 resend 03/11] ArmVirtPkg: make EFI_LOADER_DATA non-executable Ard Biesheuvel
@ 2022-10-19  9:22 ` Ard Biesheuvel
  2022-10-19  9:22 ` [PATCH v3 resend 05/11] ArmVirtPkg/ArmVirtQemu: implement ArmPlatformLib with static ID map Ard Biesheuvel
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2022-10-19  9:22 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Leif Lindholm, Alexander Graf, Gerd Hoffmann,
	Sami Mujawar

Use the appropriate PCD definition in the ArmVirtQemu DSC so that the
boot timeout is taken from the Timeout variable automatically, which is
what Linux tools such as efibootmgr expect.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmVirtPkg/ArmVirtQemu.dsc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 45c4a8fc84e0..302c0d2a4e29 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -292,6 +292,8 @@ [PcdsDynamicHii]
   gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS
 !endif
 
+  gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|5
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform
-- 
2.35.1


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

* [PATCH v3 resend 05/11] ArmVirtPkg/ArmVirtQemu: implement ArmPlatformLib with static ID map
  2022-10-19  9:22 [PATCH v3 resend 00/11] ArmVirtPkg/ArmVirtQemu: Performance streamlining Ard Biesheuvel
                   ` (3 preceding siblings ...)
  2022-10-19  9:22 ` [PATCH v3 resend 04/11] ArmVirtPkg/ArmVirtQemu: wire up timeout PCD to Timeout variable Ard Biesheuvel
@ 2022-10-19  9:22 ` Ard Biesheuvel
  2022-10-19  9:22 ` [PATCH v3 resend 06/11] ArmVirtPkg/ArmVirtQemu: use first 128 MiB as permanent PEI memory Ard Biesheuvel
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2022-10-19  9:22 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Leif Lindholm, Alexander Graf, Gerd Hoffmann,
	Sami Mujawar

To substantially reduce the amount of processing that takes place with
the MMU and caches off, implement a version of ArmPlatformLib specific
for QEMU/mach-virt in AArch64 mode that carries a statically allocated
and populated ID map that covers the NOR flash and device region, and
128 MiB of DRAM at the base of memory (0x4000_0000).

Note that 128 MiB has always been the minimum amount of DRAM we support
for this configuration, and the existing code already ASSERT()s in DEBUG
mode when booting with less.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmVirtPkg/Library/ArmPlatformLibQemu/AArch64/ArmPlatformHelper.S | 115 ++++++++++++++++++++
 ArmVirtPkg/Library/ArmPlatformLibQemu/ArmPlatformLibQemu.c        |  64 +++++++++++
 ArmVirtPkg/Library/ArmPlatformLibQemu/ArmPlatformLibQemu.inf      |  40 +++++++
 ArmVirtPkg/Library/ArmPlatformLibQemu/IdMap.S                     |  57 ++++++++++
 4 files changed, 276 insertions(+)

diff --git a/ArmVirtPkg/Library/ArmPlatformLibQemu/AArch64/ArmPlatformHelper.S b/ArmVirtPkg/Library/ArmPlatformLibQemu/AArch64/ArmPlatformHelper.S
new file mode 100644
index 000000000000..05ccc7f9f043
--- /dev/null
+++ b/ArmVirtPkg/Library/ArmPlatformLibQemu/AArch64/ArmPlatformHelper.S
@@ -0,0 +1,115 @@
+//
+//  Copyright (c) 2022, Google LLC. All rights reserved.
+//
+//  SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+//
+
+#include <AsmMacroIoLibV8.h>
+
+  .macro mov_i, reg:req, imm:req
+  movz   \reg, :abs_g3:\imm
+  movk   \reg, :abs_g2_nc:\imm
+  movk   \reg, :abs_g1_nc:\imm
+  movk   \reg, :abs_g0_nc:\imm
+  .endm
+
+ .set    MAIR_DEV_nGnRnE,     0x00
+ .set    MAIR_MEM_NC,         0x44
+ .set    MAIR_MEM_WT,         0xbb
+ .set    MAIR_MEM_WBWA,       0xff
+ .set    mairval, MAIR_DEV_nGnRnE | (MAIR_MEM_NC << 8) | (MAIR_MEM_WT << 16) | (MAIR_MEM_WBWA << 24)
+
+ .set    TCR_TG0_4KB,         0x0 << 14
+ .set    TCR_TG1_4KB,         0x2 << 30
+ .set    TCR_IPS_SHIFT,       32
+ .set    TCR_EPD1,            0x1 << 23
+ .set    TCR_SH_INNER,        0x3 << 12
+ .set    TCR_RGN_OWB,         0x1 << 10
+ .set    TCR_RGN_IWB,         0x1 << 8
+ .set    tcrval, TCR_TG0_4KB | TCR_TG1_4KB | TCR_EPD1 | TCR_RGN_OWB
+ .set    tcrval, tcrval | TCR_RGN_IWB | TCR_SH_INNER
+
+ .set    SCTLR_ELx_I,         0x1 << 12
+ .set    SCTLR_ELx_SA,        0x1 << 3
+ .set    SCTLR_ELx_C,         0x1 << 2
+ .set    SCTLR_ELx_M,         0x1 << 0
+ .set    SCTLR_EL1_SPAN,      0x1 << 23
+ .set    SCTLR_EL1_WXN,       0x1 << 19
+ .set    SCTLR_EL1_SED,       0x1 << 8
+ .set    SCTLR_EL1_ITD,       0x1 << 7
+ .set    SCTLR_EL1_RES1,      (0x1 << 11) | (0x1 << 20) | (0x1 << 22) | (0x1 << 28) | (0x1 << 29)
+ .set    sctlrval, SCTLR_ELx_M | SCTLR_ELx_C | SCTLR_ELx_SA | SCTLR_EL1_ITD | SCTLR_EL1_SED
+ .set    sctlrval, sctlrval | SCTLR_ELx_I | SCTLR_EL1_SPAN | SCTLR_EL1_RES1
+
+
+ASM_FUNC(ArmPlatformPeiBootAction)
+  mrs    x0, CurrentEL           // check current exception level
+  tbz    x0, #3, 0f              // bail if above EL1
+  ret
+
+0:mov_i  x0, mairval
+  mov_i  x1, tcrval
+  adrp   x2, idmap
+  orr    x2, x2, #0xff << 48     // set non-zero ASID
+  mov_i  x3, sctlrval
+
+  mrs    x6, id_aa64mmfr0_el1    // get the supported PA range
+  and    x6, x6, #0xf            // isolate PArange bits
+  cmp    x6, #6                  // 0b0110 == 52 bits
+  sub    x6, x6, #1              // subtract 1
+  cinc   x6, x6, ne              // add back 1 unless PArange == 52 bits
+  bfi    x1, x6, #32, #3         // copy updated PArange into TCR_EL1.IPS
+
+  cmp    x6, #3                  // 0b0011 == 42 bits
+  sub    x6, x6, #1              // subtract 1
+  cinc   x6, x6, lt              // add back 1 unless VA range >= 42
+
+  mov    x7, #32
+  sub    x6, x7, x6, lsl #2      // T0SZ for PArange != 42
+  mov    x7, #64 - 42            // T0SZ for PArange == 42
+  csel   x6, x6, x7, ne
+  orr    x1, x1, x6              // set T0SZ field in TCR
+
+  cmp    x6, #64 - 40            // VA size < 40 bits?
+  add    x4, x2, #0x1000         // advance to level 1 descriptor
+  csel   x2, x4, x2, gt
+
+  msr    mair_el1, x0            // set up the 1:1 mapping
+  msr    tcr_el1, x1
+  msr    ttbr0_el1, x2
+  isb
+
+  tlbi   vmalle1                 // invalidate any cached translations
+  ic     iallu                   // invalidate the I-cache
+  dsb    nsh
+  isb
+
+  msr    sctlr_el1, x3           // enable MMU and caches
+  isb
+  ret
+
+//UINTN
+//ArmPlatformGetCorePosition (
+//  IN UINTN MpId
+//  );
+// With this function: CorePos = (ClusterId * 4) + CoreId
+ASM_FUNC(ArmPlatformGetCorePosition)
+  mov   x0, xzr
+  ret
+
+//UINTN
+//ArmPlatformGetPrimaryCoreMpId (
+//  VOID
+//  );
+ASM_FUNC(ArmPlatformGetPrimaryCoreMpId)
+  MOV32  (w0, FixedPcdGet32 (PcdArmPrimaryCore))
+  ret
+
+//UINTN
+//ArmPlatformIsPrimaryCore (
+//  IN UINTN MpId
+//  );
+ASM_FUNC(ArmPlatformIsPrimaryCore)
+  mov   x0, #1
+  ret
diff --git a/ArmVirtPkg/Library/ArmPlatformLibQemu/ArmPlatformLibQemu.c b/ArmVirtPkg/Library/ArmPlatformLibQemu/ArmPlatformLibQemu.c
new file mode 100644
index 000000000000..1de80422ee4c
--- /dev/null
+++ b/ArmVirtPkg/Library/ArmPlatformLibQemu/ArmPlatformLibQemu.c
@@ -0,0 +1,64 @@
+/** @file
+
+  Copyright (c) 2011-2012, ARM Limited. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/ArmLib.h>
+#include <Library/ArmPlatformLib.h>
+
+/**
+  Return the current Boot Mode.
+
+  This function returns the boot reason on the platform
+
+  @return   Return the current Boot Mode of the platform
+
+**/
+EFI_BOOT_MODE
+ArmPlatformGetBootMode (
+  VOID
+  )
+{
+  return BOOT_WITH_FULL_CONFIGURATION;
+}
+
+/**
+  Initialize controllers that must setup in the normal world.
+
+  This function is called by the ArmPlatformPkg/PrePi or
+  ArmPlatformPkg/PlatformPei in the PEI phase.
+
+  @param[in]     MpId               ID of the calling CPU
+
+  @return        RETURN_SUCCESS unless the operation failed
+**/
+RETURN_STATUS
+ArmPlatformInitialize (
+  IN  UINTN  MpId
+  )
+{
+  return RETURN_SUCCESS;
+}
+
+/**
+  Return the Platform specific PPIs.
+
+  This function exposes the Platform Specific PPIs. They can be used by any
+  PrePi modules or passed to the PeiCore by PrePeiCore.
+
+  @param[out]   PpiListSize         Size in Bytes of the Platform PPI List
+  @param[out]   PpiList             Platform PPI List
+
+**/
+VOID
+ArmPlatformGetPlatformPpiList (
+  OUT UINTN                   *PpiListSize,
+  OUT EFI_PEI_PPI_DESCRIPTOR  **PpiList
+  )
+{
+  *PpiListSize = 0;
+  *PpiList     = NULL;
+}
diff --git a/ArmVirtPkg/Library/ArmPlatformLibQemu/ArmPlatformLibQemu.inf b/ArmVirtPkg/Library/ArmPlatformLibQemu/ArmPlatformLibQemu.inf
new file mode 100644
index 000000000000..b2ecdfa061cb
--- /dev/null
+++ b/ArmVirtPkg/Library/ArmPlatformLibQemu/ArmPlatformLibQemu.inf
@@ -0,0 +1,40 @@
+## @file
+#  ArmPlatformLib implementation for QEMU/mach-virt on AArch64 that contains a
+#  statically allocated 1:1 mapping of the first 128 MiB of DRAM, as well as
+#  the NOR flash and the device region
+#
+#  Copyright (c) 2011-2012, ARM Limited. All rights reserved.
+#  Copyright (c) 2022, Google LLC. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 1.27
+  BASE_NAME                      = ArmPlatformLibQemu
+  FILE_GUID                      = 40af3a25-f02c-4aef-94ef-7ac0282d21d4
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = ArmPlatformLib
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+
+[LibraryClasses]
+  ArmLib
+  DebugLib
+
+[Sources.common]
+  ArmPlatformLibQemu.c
+  IdMap.S
+
+[Sources.AArch64]
+  AArch64/ArmPlatformHelper.S
+
+[FixedPcd]
+  gArmTokenSpaceGuid.PcdArmPrimaryCoreMask
+  gArmTokenSpaceGuid.PcdArmPrimaryCore
diff --git a/ArmVirtPkg/Library/ArmPlatformLibQemu/IdMap.S b/ArmVirtPkg/Library/ArmPlatformLibQemu/IdMap.S
new file mode 100644
index 000000000000..4a4b7b77ed83
--- /dev/null
+++ b/ArmVirtPkg/Library/ArmPlatformLibQemu/IdMap.S
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+// Copyright 2022 Google LLC
+// Author: Ard Biesheuvel <ardb@google.com>
+
+  .set      TT_TYPE_BLOCK, 0x1
+  .set      TT_TYPE_PAGE,  0x3
+  .set      TT_TYPE_TABLE, 0x3
+
+  .set      TT_AF, 0x1 << 10
+  .set      TT_NG, 0x1 << 11
+  .set      TT_RO, 0x2 << 6
+  .set      TT_XN, 0x3 << 53
+
+  .set      TT_MT_DEV, 0x0 << 2                 // MAIR #0
+  .set      TT_MT_MEM, (0x3 << 2) | (0x3 << 8)  // MAIR #3
+
+  .set      PAGE_XIP,  TT_TYPE_PAGE  | TT_MT_MEM | TT_AF | TT_RO | TT_NG
+  .set      BLOCK_DEV, TT_TYPE_BLOCK | TT_MT_DEV | TT_AF | TT_XN | TT_NG
+  .set      BLOCK_MEM, TT_TYPE_BLOCK | TT_MT_MEM | TT_AF | TT_XN | TT_NG
+
+  .globl    idmap
+  .section  ".rodata.idmap", "a", %progbits
+  .align    12
+
+idmap:      /* level 0 */
+  .quad     1f + TT_TYPE_TABLE
+  .fill     511, 8, 0x0
+
+1:          /* level 1 */
+  .quad     20f + TT_TYPE_TABLE           // 1 GB of flash and device mappings
+  .quad     21f + TT_TYPE_TABLE           // up to 1 GB of DRAM
+  .fill     510, 8, 0x0                   // 510 GB of remaining VA space
+
+20:         /* level 2 */
+  .quad     3f + TT_TYPE_TABLE            // up to 2 MB of flash
+  .fill     63, 8, 0x0                    // 126 MB of unused flash
+  .set      idx, 64
+  .rept     448
+  .quad     BLOCK_DEV | (idx << 21)       // 896 MB of RW- device mappings
+  .set      idx, idx + 1
+  .endr
+
+21:         /* level 2 */
+  .set      idx, 0x40000000 >> 21
+  .rept     64
+  .quad     BLOCK_MEM | (idx << 21)       // 128 MB of RW- memory mappings
+  .set      idx, idx + 1
+  .endr
+  .fill     448, 8, 0x0
+
+3:          /* level 3 */
+  .quad     0x0                           // omit first 4k page
+  .set      idx, 1
+  .rept     511
+  .quad     PAGE_XIP | (idx << 12)        // 2044 KiB of R-X flash mappings
+  .set      idx, idx + 1
+  .endr
-- 
2.35.1


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

* [PATCH v3 resend 06/11] ArmVirtPkg/ArmVirtQemu: use first 128 MiB as permanent PEI memory
  2022-10-19  9:22 [PATCH v3 resend 00/11] ArmVirtPkg/ArmVirtQemu: Performance streamlining Ard Biesheuvel
                   ` (4 preceding siblings ...)
  2022-10-19  9:22 ` [PATCH v3 resend 05/11] ArmVirtPkg/ArmVirtQemu: implement ArmPlatformLib with static ID map Ard Biesheuvel
@ 2022-10-19  9:22 ` Ard Biesheuvel
  2022-10-19  9:22 ` [PATCH v3 resend 07/11] ArmVirtPkg/ArmVirtQemu: enable initial ID map at early boot Ard Biesheuvel
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2022-10-19  9:22 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Leif Lindholm, Alexander Graf, Gerd Hoffmann,
	Sami Mujawar

In order to allow booting with the MMU and caches enabled really early,
we need to ensure that the code that populates the page tables can
access those page tables with the statically defined ID map active.

So let's put the permanent PEI RAM in the first 128 MiB of memory, which
we will cover with this initial ID map (as it is the minimum supported
DRAM size for ArmVirtQemu).

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmVirtPkg/MemoryInitPei/MemoryInitPeim.c   | 104 ++++++++++++++++++++
 ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf |  59 +++++++++++
 2 files changed, 163 insertions(+)

diff --git a/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.c b/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.c
new file mode 100644
index 000000000000..ef88a9df1d62
--- /dev/null
+++ b/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.c
@@ -0,0 +1,104 @@
+/** @file
+
+  Copyright (c) 2011, ARM Limited. All rights reserved.
+  Copyright (c) 2022, Google LLC. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <PiPei.h>
+#include <Library/ArmPlatformLib.h>
+#include <Library/DebugLib.h>
+#include <Library/HobLib.h>
+#include <Library/PeimEntryPoint.h>
+#include <Library/PeiServicesLib.h>
+#include <Library/PcdLib.h>
+#include <Guid/MemoryTypeInformation.h>
+
+EFI_STATUS
+EFIAPI
+MemoryPeim (
+  IN EFI_PHYSICAL_ADDRESS  UefiMemoryBase,
+  IN UINT64                UefiMemorySize
+  );
+
+/**
+  Build the memory type information HOB that describes how many pages of each
+  type to preallocate when initializing the GCD memory map.
+**/
+VOID
+EFIAPI
+BuildMemoryTypeInformationHob (
+  VOID
+  )
+{
+  EFI_MEMORY_TYPE_INFORMATION  Info[10];
+
+  Info[0].Type          = EfiACPIReclaimMemory;
+  Info[0].NumberOfPages = FixedPcdGet32 (PcdMemoryTypeEfiACPIReclaimMemory);
+  Info[1].Type          = EfiACPIMemoryNVS;
+  Info[1].NumberOfPages = FixedPcdGet32 (PcdMemoryTypeEfiACPIMemoryNVS);
+  Info[2].Type          = EfiReservedMemoryType;
+  Info[2].NumberOfPages = FixedPcdGet32 (PcdMemoryTypeEfiReservedMemoryType);
+  Info[3].Type          = EfiRuntimeServicesData;
+  Info[3].NumberOfPages = FixedPcdGet32 (PcdMemoryTypeEfiRuntimeServicesData);
+  Info[4].Type          = EfiRuntimeServicesCode;
+  Info[4].NumberOfPages = FixedPcdGet32 (PcdMemoryTypeEfiRuntimeServicesCode);
+  Info[5].Type          = EfiBootServicesCode;
+  Info[5].NumberOfPages = FixedPcdGet32 (PcdMemoryTypeEfiBootServicesCode);
+  Info[6].Type          = EfiBootServicesData;
+  Info[6].NumberOfPages = FixedPcdGet32 (PcdMemoryTypeEfiBootServicesData);
+  Info[7].Type          = EfiLoaderCode;
+  Info[7].NumberOfPages = FixedPcdGet32 (PcdMemoryTypeEfiLoaderCode);
+  Info[8].Type          = EfiLoaderData;
+  Info[8].NumberOfPages = FixedPcdGet32 (PcdMemoryTypeEfiLoaderData);
+
+  // Terminator for the list
+  Info[9].Type          = EfiMaxMemoryType;
+  Info[9].NumberOfPages = 0;
+
+  BuildGuidDataHob (&gEfiMemoryTypeInformationGuid, &Info, sizeof (Info));
+}
+
+/**
+  Module entry point.
+
+  @param[in]    FileHandle    Handle of the file being invoked.
+  @param[in]    PeiServices   Describes the list of possible PEI Services.
+
+  @return       EFI_SUCCESS unless the operation failed.
+**/
+EFI_STATUS
+EFIAPI
+InitializeMemory (
+  IN       EFI_PEI_FILE_HANDLE  FileHandle,
+  IN CONST EFI_PEI_SERVICES     **PeiServices
+  )
+{
+  UINTN       UefiMemoryBase;
+  EFI_STATUS  Status;
+
+  ASSERT (FixedPcdGet64 (PcdSystemMemoryBase) < (UINT64)MAX_ALLOC_ADDRESS);
+
+  //
+  // Put the permanent PEI memory in the first 128 MiB of DRAM so that
+  // it is covered by the statically configured ID map.
+  //
+  UefiMemoryBase = (UINTN)FixedPcdGet64 (PcdSystemMemoryBase) + SIZE_128MB
+                   - FixedPcdGet32 (PcdSystemMemoryUefiRegionSize);
+
+  Status = PeiServicesInstallPeiMemory (
+             UefiMemoryBase,
+             FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)
+             );
+  ASSERT_EFI_ERROR (Status);
+
+  Status = MemoryPeim (
+             UefiMemoryBase,
+             FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)
+             );
+  ASSERT_EFI_ERROR (Status);
+
+  return Status;
+}
diff --git a/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf b/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf
new file mode 100644
index 000000000000..2039f71a0ebe
--- /dev/null
+++ b/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf
@@ -0,0 +1,59 @@
+## @file
+#  Implementation of MemoryInitPeim that uses the first 128 MiB at the base of
+#  DRAM as permanent PEI memory
+#
+#  Copyright (c) 2011-2014, ARM Ltd. All rights reserved.<BR>
+#  Copyright (c) 2022, Google LLC. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 1.27
+  BASE_NAME                      = MemoryInit
+  FILE_GUID                      = 0fbffd44-f98f-4e1c-9922-e9b21f13c3f8
+  MODULE_TYPE                    = PEIM
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = InitializeMemory
+
+[Sources]
+  MemoryInitPeim.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+
+[LibraryClasses]
+  PeimEntryPoint
+  DebugLib
+  HobLib
+  ArmLib
+  ArmPlatformLib
+  MemoryInitPeiLib
+
+[Guids]
+  gEfiMemoryTypeInformationGuid
+
+[FeaturePcd]
+  gEmbeddedTokenSpaceGuid.PcdPrePiProduceMemoryTypeInformationHob
+
+[FixedPcd]
+  gArmTokenSpaceGuid.PcdSystemMemoryBase
+  gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize
+
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesCode
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesData
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderCode
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderData
+
+[Depex]
+  TRUE
-- 
2.35.1


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

* [PATCH v3 resend 07/11] ArmVirtPkg/ArmVirtQemu: enable initial ID map at early boot
  2022-10-19  9:22 [PATCH v3 resend 00/11] ArmVirtPkg/ArmVirtQemu: Performance streamlining Ard Biesheuvel
                   ` (5 preceding siblings ...)
  2022-10-19  9:22 ` [PATCH v3 resend 06/11] ArmVirtPkg/ArmVirtQemu: use first 128 MiB as permanent PEI memory Ard Biesheuvel
@ 2022-10-19  9:22 ` Ard Biesheuvel
  2022-11-08 15:59   ` Gerd Hoffmann
  2022-10-19  9:22 ` [PATCH v3 resend 08/11] ArmVirtPkg/ArmVirtQemu: Drop unused variable PEIM Ard Biesheuvel
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Ard Biesheuvel @ 2022-10-19  9:22 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Leif Lindholm, Alexander Graf, Gerd Hoffmann,
	Sami Mujawar

Now that we have all the pieces in place, switch the AArch64 version of
ArmVirtQemu to a mode where the first thing it does out of reset is
enable a preliminary ID map that covers the NOR flash and sufficient
DRAM to create the UEFI page tables as usual.

The advantage of this is that no manipulation of memory occurs any
longer before the MMU is enabled, which removes the need for explicit
coherency management, which is cumbersome and bad for performance.

It also means we no longer need to build all components that may execute
with the MMU off (including BASE libraries) with strict alignment.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmVirtPkg/ArmVirtQemu.dsc | 17 ++++++++++++++---
 ArmVirtPkg/ArmVirtQemu.fdf |  2 +-
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 302c0d2a4e29..21a321e35794 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -63,8 +63,6 @@ [LibraryClasses.common]
   QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
   QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
 
-  ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
-
   TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
   NorFlashPlatformLib|ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
 
@@ -92,6 +90,12 @@ [LibraryClasses.common]
   TpmPlatformHierarchyLib|SecurityPkg/Library/PeiDxeTpmPlatformHierarchyLibNull/PeiDxeTpmPlatformHierarchyLib.inf
 !endif
 
+[LibraryClasses.AARCH64]
+  ArmPlatformLib|ArmVirtPkg/Library/ArmPlatformLibQemu/ArmPlatformLibQemu.inf
+
+[LibraryClasses.ARM]
+  ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
+
 [LibraryClasses.common.PEIM]
   ArmVirtMemInfoLib|ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
 
@@ -112,6 +116,8 @@ [LibraryClasses.common.UEFI_DRIVER]
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
 
 [BuildOptions]
+  GCC:*_*_AARCH64_CC_XIPFLAGS = -mno-strict-align
+
 !include NetworkPkg/NetworkBuildOptions.dsc.inc
 
 ################################################################################
@@ -310,7 +316,12 @@ [Components.common]
       PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   }
   ArmPlatformPkg/PlatformPei/PlatformPeim.inf
-  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
+  ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf {
+    <LibraryClasses>
+!if $(ARCH) == AARCH64
+      ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
+!endif
+  }
   ArmPkg/Drivers/CpuPei/CpuPei.inf
 
   MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
index b5e2253295fe..7f17aeb3ad0d 100644
--- a/ArmVirtPkg/ArmVirtQemu.fdf
+++ b/ArmVirtPkg/ArmVirtQemu.fdf
@@ -107,7 +107,7 @@ [FV.FVMAIN_COMPACT]
   INF ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
   INF MdeModulePkg/Core/Pei/PeiMain.inf
   INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf
-  INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
+  INF ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf
   INF ArmPkg/Drivers/CpuPei/CpuPei.inf
   INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
   INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
-- 
2.35.1


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

* [PATCH v3 resend 08/11] ArmVirtPkg/ArmVirtQemu: Drop unused variable PEIM
  2022-10-19  9:22 [PATCH v3 resend 00/11] ArmVirtPkg/ArmVirtQemu: Performance streamlining Ard Biesheuvel
                   ` (6 preceding siblings ...)
  2022-10-19  9:22 ` [PATCH v3 resend 07/11] ArmVirtPkg/ArmVirtQemu: enable initial ID map at early boot Ard Biesheuvel
@ 2022-10-19  9:22 ` Ard Biesheuvel
  2022-10-19  9:22 ` [PATCH v3 resend 09/11] ArmVirtPkg/ArmVirtQemu: avoid shadowing PEIMs unless necessary Ard Biesheuvel
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2022-10-19  9:22 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Leif Lindholm, Alexander Graf, Gerd Hoffmann,
	Sami Mujawar

The variable PEIM is included in the build but its runtime prerequisites
are absent so it is never dispatched. Just drop it.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmVirtPkg/ArmVirtQemu.dsc | 2 --
 ArmVirtPkg/ArmVirtQemu.fdf | 1 -
 2 files changed, 3 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 21a321e35794..de34481673ea 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -324,8 +324,6 @@ [Components.common]
   }
   ArmPkg/Drivers/CpuPei/CpuPei.inf
 
-  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
-
 !if $(TPM2_ENABLE) == TRUE
   MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf {
     <LibraryClasses>
diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
index 7f17aeb3ad0d..c85e36b185d3 100644
--- a/ArmVirtPkg/ArmVirtQemu.fdf
+++ b/ArmVirtPkg/ArmVirtQemu.fdf
@@ -110,7 +110,6 @@ [FV.FVMAIN_COMPACT]
   INF ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf
   INF ArmPkg/Drivers/CpuPei/CpuPei.inf
   INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
-  INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
   INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
 
 !if $(TPM2_ENABLE) == TRUE
-- 
2.35.1


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

* [PATCH v3 resend 09/11] ArmVirtPkg/ArmVirtQemu: avoid shadowing PEIMs unless necessary
  2022-10-19  9:22 [PATCH v3 resend 00/11] ArmVirtPkg/ArmVirtQemu: Performance streamlining Ard Biesheuvel
                   ` (7 preceding siblings ...)
  2022-10-19  9:22 ` [PATCH v3 resend 08/11] ArmVirtPkg/ArmVirtQemu: Drop unused variable PEIM Ard Biesheuvel
@ 2022-10-19  9:22 ` Ard Biesheuvel
  2022-10-19  9:22 ` [PATCH v3 resend 10/11] ArmVirtPkg/QemuVirtMemInfoLib: use HOB not PCD to record the memory size Ard Biesheuvel
  2022-10-19  9:22 ` [PATCH v3 resend 11/11] ArmVirtPkg/ArmVirtQemu: omit PCD PEIM unless TPM support is enabled Ard Biesheuvel
  10 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2022-10-19  9:22 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Leif Lindholm, Alexander Graf, Gerd Hoffmann,
	Sami Mujawar

Some PEIMs register for shadow execution explicitly, but others exist
that don't care and can happily execute in place. Since the emulated NOR
flash is just RAM, shadowing has no performance benefits so let's only
do this if needed.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmVirtPkg/ArmVirtQemu.dsc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index de34481673ea..c3d264077bce 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -217,6 +217,9 @@ [PcdsFixedAtBuild.common]
   gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|3
   gEfiShellPkgTokenSpaceGuid.PcdShellFileOperationSize|0x20000
 
+  # Shadowing PEI modules is absolutely pointless when the NOR flash is emulated
+  gEfiMdeModulePkgTokenSpaceGuid.PcdShadowPeimOnBoot|FALSE
+
 [PcdsFixedAtBuild.AARCH64]
   # Clearing BIT0 in this PCD prevents installing a 32-bit SMBIOS entry point,
   # if the entry point version is >= 3.0. AARCH64 OSes cannot assume the
-- 
2.35.1


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

* [PATCH v3 resend 10/11] ArmVirtPkg/QemuVirtMemInfoLib: use HOB not PCD to record the memory size
  2022-10-19  9:22 [PATCH v3 resend 00/11] ArmVirtPkg/ArmVirtQemu: Performance streamlining Ard Biesheuvel
                   ` (8 preceding siblings ...)
  2022-10-19  9:22 ` [PATCH v3 resend 09/11] ArmVirtPkg/ArmVirtQemu: avoid shadowing PEIMs unless necessary Ard Biesheuvel
@ 2022-10-19  9:22 ` Ard Biesheuvel
  2022-10-19  9:22 ` [PATCH v3 resend 11/11] ArmVirtPkg/ArmVirtQemu: omit PCD PEIM unless TPM support is enabled Ard Biesheuvel
  10 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2022-10-19  9:22 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Leif Lindholm, Alexander Graf, Gerd Hoffmann,
	Sami Mujawar

Due to the way we inherited the formerly fixed PCDs to describe the
system memory base and size from ArmPlatformPkg, we ended up with a
MemoryInit PEIM that relies on dynamic PCDs to communicate the size of
system memory between the constructor of one of its library dependencies
and the core module. This is unnecessary, and forces us to incorporate
the PCD PEIM as well, for no good reason. So instead, let's use a HOB.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmVirtPkg/ArmVirtPkg.dec                                                |  1 +
 ArmVirtPkg/ArmVirtQemu.dsc                                               |  6 ++--
 ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c     | 14 ++++++--
 ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf   |  1 +
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c               | 35 ++++++++++++++++++--
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf             |  5 ++-
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf          |  8 ++---
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLibConstructor.c | 30 ++++++++++-------
 8 files changed, 75 insertions(+), 25 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index 4e165f6cd845..89d21ec3a364 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -32,6 +32,7 @@ [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 } }
+  gArmVirtSystemMemorySizeGuid     = { 0x504eccb9, 0x1bf0, 0x4420, { 0x86, 0x5d, 0xdc, 0x66, 0x06, 0xd4, 0x13, 0xbf } }
 
   gArmVirtVariableGuid   = { 0x50bea1e5, 0xa2c5, 0x46e9, { 0x9b, 0x3a, 0x59, 0x59, 0x65, 0x16, 0xb0, 0x0a } }
 
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index c3d264077bce..43e19f605084 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -220,6 +220,9 @@ [PcdsFixedAtBuild.common]
   # Shadowing PEI modules is absolutely pointless when the NOR flash is emulated
   gEfiMdeModulePkgTokenSpaceGuid.PcdShadowPeimOnBoot|FALSE
 
+  # System Memory Size -- 128 MB initially, actual size will be fetched from DT
+  gArmTokenSpaceGuid.PcdSystemMemorySize|0x8000000
+
 [PcdsFixedAtBuild.AARCH64]
   # Clearing BIT0 in this PCD prevents installing a 32-bit SMBIOS entry point,
   # if the entry point version is >= 3.0. AARCH64 OSes cannot assume the
@@ -236,9 +239,6 @@ [PcdsDynamicDefault.common]
   #  enumeration to complete before installing ACPI tables.
   gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|TRUE
 
-  # System Memory Size -- 1 MB initially, actual size will be fetched from DT
-  gArmTokenSpaceGuid.PcdSystemMemorySize|0x00100000
-
   gArmTokenSpaceGuid.PcdArmArchTimerSecIntrNum|0x0
   gArmTokenSpaceGuid.PcdArmArchTimerIntrNum|0x0
   gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum|0x0
diff --git a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
index 98d90ad4200d..72e5c65af79e 100644
--- a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
+++ b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
@@ -52,10 +52,19 @@ MemoryPeim (
 {
   EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
   UINT64                       SystemMemoryTop;
+  UINT64                       SystemMemorySize;
+  VOID                         *Hob;
 
   // Ensure PcdSystemMemorySize has been set
   ASSERT (PcdGet64 (PcdSystemMemorySize) != 0);
 
+  SystemMemorySize = PcdGet64 (PcdSystemMemorySize);
+
+  Hob = GetFirstGuidHob (&gArmVirtSystemMemorySizeGuid);
+  if (Hob != NULL) {
+    SystemMemorySize = *(UINT64 *)GET_GUID_HOB_DATA (Hob);
+  }
+
   //
   // Now, the permanent memory has been installed, we can call AllocatePages()
   //
@@ -66,8 +75,7 @@ MemoryPeim (
                         EFI_RESOURCE_ATTRIBUTE_TESTED
                         );
 
-  SystemMemoryTop = PcdGet64 (PcdSystemMemoryBase) +
-                    PcdGet64 (PcdSystemMemorySize);
+  SystemMemoryTop = PcdGet64 (PcdSystemMemoryBase) + SystemMemorySize;
 
   if (SystemMemoryTop - 1 > MAX_ALLOC_ADDRESS) {
     BuildResourceDescriptorHob (
@@ -87,7 +95,7 @@ MemoryPeim (
       EFI_RESOURCE_SYSTEM_MEMORY,
       ResourceAttributes,
       PcdGet64 (PcdSystemMemoryBase),
-      PcdGet64 (PcdSystemMemorySize)
+      SystemMemorySize
       );
   }
 
diff --git a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf
index 21327f79f4b9..48d9c66b22db 100644
--- a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf
+++ b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf
@@ -34,6 +34,7 @@ [LibraryClasses]
   CacheMaintenanceLib
 
 [Guids]
+  gArmVirtSystemMemorySizeGuid
   gEfiMemoryTypeInformationGuid
 
 [FeaturePcd]
diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
index cf569bed99c4..9cf43f06c073 100644
--- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
+++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
@@ -6,10 +6,12 @@
 
 **/
 
-#include <Base.h>
+#include <Uefi.h>
+#include <Pi/PiMultiPhase.h>
 #include <Library/ArmLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
+#include <Library/HobLib.h>
 #include <Library/MemoryAllocationLib.h>
 
 // Number of Virtual Memory Map Descriptors
@@ -24,6 +26,28 @@
 #define MACH_VIRT_PERIPH_BASE  0x08000000
 #define MACH_VIRT_PERIPH_SIZE  SIZE_128MB
 
+/**
+  Default library constructur that obtains the memory size from a PCD.
+
+  @return  Always returns RETURN_SUCCESS
+
+**/
+RETURN_STATUS
+EFIAPI
+QemuVirtMemInfoLibConstructor (
+  VOID
+  )
+{
+  UINT64  Size;
+  VOID    *Hob;
+
+  Size = PcdGet64 (PcdSystemMemorySize);
+  Hob  = BuildGuidDataHob (&gArmVirtSystemMemorySizeGuid, &Size, sizeof Size);
+  ASSERT (Hob != NULL);
+
+  return RETURN_SUCCESS;
+}
+
 /**
   Return the Virtual Memory Map of your platform
 
@@ -43,9 +67,16 @@ ArmVirtGetMemoryMap (
   )
 {
   ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
+  VOID                          *MemorySizeHob;
 
   ASSERT (VirtualMemoryMap != NULL);
 
+  MemorySizeHob = GetFirstGuidHob (&gArmVirtSystemMemorySizeGuid);
+  ASSERT (MemorySizeHob != NULL);
+  if (MemorySizeHob == NULL) {
+    return;
+  }
+
   VirtualMemoryTable = AllocatePool (
                          sizeof (ARM_MEMORY_REGION_DESCRIPTOR) *
                          MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS
@@ -59,7 +90,7 @@ ArmVirtGetMemoryMap (
   // System DRAM
   VirtualMemoryTable[0].PhysicalBase = PcdGet64 (PcdSystemMemoryBase);
   VirtualMemoryTable[0].VirtualBase  = VirtualMemoryTable[0].PhysicalBase;
-  VirtualMemoryTable[0].Length       = PcdGet64 (PcdSystemMemorySize);
+  VirtualMemoryTable[0].Length       = *(UINT64 *)GET_GUID_HOB_DATA (MemorySizeHob);
   VirtualMemoryTable[0].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
 
   DEBUG ((
diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
index 7150de6c10d0..6acad8bbd7f3 100644
--- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
+++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
@@ -14,6 +14,7 @@ [Defines]
   MODULE_TYPE                    = BASE
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = ArmVirtMemInfoLib
+  CONSTRUCTOR                    = QemuVirtMemInfoLibConstructor
 
 [Sources]
   QemuVirtMemInfoLib.c
@@ -30,7 +31,9 @@ [LibraryClasses]
   BaseMemoryLib
   DebugLib
   MemoryAllocationLib
-  PcdLib
+
+[Guids]
+  gArmVirtSystemMemorySizeGuid
 
 [Pcd]
   gArmTokenSpaceGuid.PcdFvBaseAddress
diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
index 7ecf6dfbb786..f045e39a4131 100644
--- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
+++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
@@ -32,16 +32,16 @@ [LibraryClasses]
   BaseMemoryLib
   DebugLib
   FdtLib
-  PcdLib
   MemoryAllocationLib
 
-[Pcd]
+[Guids]
+  gArmVirtSystemMemorySizeGuid
+
+[FixedPcd]
   gArmTokenSpaceGuid.PcdFdBaseAddress
   gArmTokenSpaceGuid.PcdFvBaseAddress
   gArmTokenSpaceGuid.PcdSystemMemoryBase
   gArmTokenSpaceGuid.PcdSystemMemorySize
-
-[FixedPcd]
   gArmTokenSpaceGuid.PcdFdSize
   gArmTokenSpaceGuid.PcdFvSize
   gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLibConstructor.c b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLibConstructor.c
index 33d3597d57ab..c47ab8296622 100644
--- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLibConstructor.c
+++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLibConstructor.c
@@ -6,9 +6,10 @@
 
 **/
 
-#include <Base.h>
+#include <Uefi.h>
+#include <Pi/PiMultiPhase.h>
 #include <Library/DebugLib.h>
-#include <Library/PcdLib.h>
+#include <Library/HobLib.h>
 #include <libfdt.h>
 
 RETURN_STATUS
@@ -17,14 +18,14 @@ QemuVirtMemInfoPeiLibConstructor (
   VOID
   )
 {
-  VOID           *DeviceTreeBase;
-  INT32          Node, Prev;
-  UINT64         NewBase, CurBase;
-  UINT64         NewSize, CurSize;
-  CONST CHAR8    *Type;
-  INT32          Len;
-  CONST UINT64   *RegProp;
-  RETURN_STATUS  PcdStatus;
+  VOID          *DeviceTreeBase;
+  INT32         Node, Prev;
+  UINT64        NewBase, CurBase;
+  UINT64        NewSize, CurSize;
+  CONST CHAR8   *Type;
+  INT32         Len;
+  CONST UINT64  *RegProp;
+  VOID          *Hob;
 
   NewBase = 0;
   NewSize = 0;
@@ -86,8 +87,13 @@ QemuVirtMemInfoPeiLibConstructor (
   // Make sure the start of DRAM matches our expectation
   //
   ASSERT (FixedPcdGet64 (PcdSystemMemoryBase) == NewBase);
-  PcdStatus = PcdSet64S (PcdSystemMemorySize, NewSize);
-  ASSERT_RETURN_ERROR (PcdStatus);
+
+  Hob = BuildGuidDataHob (
+          &gArmVirtSystemMemorySizeGuid,
+          &NewSize,
+          sizeof NewSize
+          );
+  ASSERT (Hob != NULL);
 
   //
   // We need to make sure that the machine we are running on has at least
-- 
2.35.1


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

* [PATCH v3 resend 11/11] ArmVirtPkg/ArmVirtQemu: omit PCD PEIM unless TPM support is enabled
  2022-10-19  9:22 [PATCH v3 resend 00/11] ArmVirtPkg/ArmVirtQemu: Performance streamlining Ard Biesheuvel
                   ` (9 preceding siblings ...)
  2022-10-19  9:22 ` [PATCH v3 resend 10/11] ArmVirtPkg/QemuVirtMemInfoLib: use HOB not PCD to record the memory size Ard Biesheuvel
@ 2022-10-19  9:22 ` Ard Biesheuvel
  10 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2022-10-19  9:22 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Leif Lindholm, Alexander Graf, Gerd Hoffmann,
	Sami Mujawar

The TPM discovery code relies on a dynamic PCD to communicate the TPM
base address to other components. But no other code relies on dynamic
PCDs in the PEI phase so let's drop the PCD PEIM when TPM support is not
enabled.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmVirtPkg/ArmVirtQemu.dsc | 22 +++++++++++++++-----
 ArmVirtPkg/ArmVirtQemu.fdf |  2 +-
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 43e19f605084..842a298e0435 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -287,10 +287,15 @@ [PcdsDynamicDefault.common]
   #
   # TPM2 support
   #
-  gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress|0x0
 !if $(TPM2_ENABLE) == TRUE
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress|0x0
   gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
   gEfiSecurityPkgTokenSpaceGuid.PcdTpm2HashMask|0
+!else
+[PcdsPatchableInModule]
+  # make this PCD patchable instead of dynamic when TPM support is not enabled
+  # this permits setting the PCD in unreachable code without pulling in dynamic PCD support
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress|0x0
 !endif
 
 [PcdsDynamicHii]
@@ -303,6 +308,13 @@ [PcdsDynamicHii]
 
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|5
 
+[LibraryClasses.common.PEI_CORE, LibraryClasses.common.PEIM]
+!if $(TPM2_ENABLE) == TRUE
+  PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
+!else
+  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+!endif
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform
@@ -314,10 +326,6 @@ [Components.common]
   #
   ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
   MdeModulePkg/Core/Pei/PeiMain.inf
-  MdeModulePkg/Universal/PCD/Pei/Pcd.inf {
-    <LibraryClasses>
-      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
-  }
   ArmPlatformPkg/PlatformPei/PlatformPeim.inf
   ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf {
     <LibraryClasses>
@@ -328,6 +336,10 @@ [Components.common]
   ArmPkg/Drivers/CpuPei/CpuPei.inf
 
 !if $(TPM2_ENABLE) == TRUE
+  MdeModulePkg/Universal/PCD/Pei/Pcd.inf {
+    <LibraryClasses>
+      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  }
   MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf {
     <LibraryClasses>
       ResetSystemLib|ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.inf
diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
index c85e36b185d3..764f652afd0e 100644
--- a/ArmVirtPkg/ArmVirtQemu.fdf
+++ b/ArmVirtPkg/ArmVirtQemu.fdf
@@ -109,10 +109,10 @@ [FV.FVMAIN_COMPACT]
   INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf
   INF ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf
   INF ArmPkg/Drivers/CpuPei/CpuPei.inf
-  INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
   INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
 
 !if $(TPM2_ENABLE) == TRUE
+  INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
   INF MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf
   INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
   INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
-- 
2.35.1


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

* Re: [edk2-devel] [PATCH v3 resend 01/11] ArmVirtPkg: remove EbcDxe from all platforms
  2022-10-19  9:22 ` [PATCH v3 resend 01/11] ArmVirtPkg: remove EbcDxe from all platforms Ard Biesheuvel
@ 2022-10-20  8:58   ` Leif Lindholm
  0 siblings, 0 replies; 16+ messages in thread
From: Leif Lindholm @ 2022-10-20  8:58 UTC (permalink / raw)
  To: devel, ardb; +Cc: Alexander Graf, Gerd Hoffmann, Sami Mujawar

On Wed, Oct 19, 2022 at 11:22:01 +0200, Ard Biesheuvel wrote:
> The EBC interpreter is rarely, if ever, used on ARM, and is especially
> pointless on virtual machines. So let's drop it from the builds.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>

> ---
>  ArmVirtPkg/ArmVirt.dsc.inc           | 5 -----
>  ArmVirtPkg/ArmVirtCloudHv.fdf        | 5 -----
>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 5 -----
>  ArmVirtPkg/ArmVirtXen.fdf            | 5 -----
>  4 files changed, 20 deletions(-)
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index c39e2506a3ea..34575585adbb 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -422,8 +422,3 @@ [Components.AARCH64]
>      <LibraryClasses>
>        NULL|EmbeddedPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
>    }
> -
> -  #
> -  # EBC support
> -  #
> -  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
> diff --git a/ArmVirtPkg/ArmVirtCloudHv.fdf b/ArmVirtPkg/ArmVirtCloudHv.fdf
> index 81c539590a76..a5f172d79bfc 100644
> --- a/ArmVirtPkg/ArmVirtCloudHv.fdf
> +++ b/ArmVirtPkg/ArmVirtCloudHv.fdf
> @@ -195,11 +195,6 @@ [FV.FvMain]
>    INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
>    INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
>    INF ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
> -
> -  #
> -  # EBC support
> -  #
> -  INF MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
>  !endif
>  
>    #
> diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> index d4df6dede0fe..787286133095 100644
> --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> @@ -146,11 +146,6 @@ [FV.FvMain]
>    INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
>    INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
>    INF OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
> -
> -  #
> -  # EBC support
> -  #
> -  INF MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
>  !endif
>  
>    #
> diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf
> index 132480f03059..770fbf7289be 100644
> --- a/ArmVirtPkg/ArmVirtXen.fdf
> +++ b/ArmVirtPkg/ArmVirtXen.fdf
> @@ -201,11 +201,6 @@ [FV.FvMain]
>  !if $(ARCH) == AARCH64
>    INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
>    INF ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf
> -
> -  #
> -  # EBC support
> -  #
> -  INF MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
>  !endif
>  
>   #
> -- 
> 2.35.1
> 
> 
> 
> 
> 
> 

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

* Re: [PATCH v3 resend 02/11] ArmVirtPkg: do not enable iSCSI driver by default
  2022-10-19  9:22 ` [PATCH v3 resend 02/11] ArmVirtPkg: do not enable iSCSI driver by default Ard Biesheuvel
@ 2022-10-20  9:03   ` Leif Lindholm
  2022-10-20 12:34     ` Ard Biesheuvel
  0 siblings, 1 reply; 16+ messages in thread
From: Leif Lindholm @ 2022-10-20  9:03 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: devel, Alexander Graf, Gerd Hoffmann, Sami Mujawar

On Wed, Oct 19, 2022 at 11:22:02 +0200, Ard Biesheuvel wrote:
> The iSCSI driver slows down the boot on a pristine variable store flash
> image, as it creates a couple of large EFI non-volatile variables to
> preserve state between boots.
> 
> Since iSCSI boot for VMs is kind of niche anyway, let's default to
> disabled. If someone needs it in their build, they can use the -D build
> command option to re-enable it on the fly.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Apologies for bikeshedding, but ... I find the listing of tweakable
options in a block at the start of a .dsc very useful.
Could we explicitly set to FALSE instead?

Either way:
Acked-by: Leif Lindholm <quic_llindhol@quicinc.com>

> ---
>  ArmVirtPkg/ArmVirtQemu.dsc       | 1 -
>  ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 9369a88858fd..45c4a8fc84e0 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -40,7 +40,6 @@ [Defines]
>    DEFINE NETWORK_SNP_ENABLE              = FALSE
>    DEFINE NETWORK_TLS_ENABLE              = FALSE
>    DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS  = TRUE
> -  DEFINE NETWORK_ISCSI_ENABLE            = TRUE
>  
>  !if $(NETWORK_SNP_ENABLE) == TRUE
>    !error "NETWORK_SNP_ENABLE is IA32/X64/EBC only"
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index 7f7d15d6eee3..66039f07f41b 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -38,7 +38,6 @@ [Defines]
>    DEFINE NETWORK_SNP_ENABLE              = FALSE
>    DEFINE NETWORK_TLS_ENABLE              = FALSE
>    DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS  = TRUE
> -  DEFINE NETWORK_ISCSI_ENABLE            = TRUE
>  
>  !if $(NETWORK_SNP_ENABLE) == TRUE
>    !error "NETWORK_SNP_ENABLE is IA32/X64/EBC only"
> -- 
> 2.35.1
> 

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

* Re: [PATCH v3 resend 02/11] ArmVirtPkg: do not enable iSCSI driver by default
  2022-10-20  9:03   ` Leif Lindholm
@ 2022-10-20 12:34     ` Ard Biesheuvel
  0 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2022-10-20 12:34 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: devel, Alexander Graf, Gerd Hoffmann, Sami Mujawar

On Thu, 20 Oct 2022 at 11:04, Leif Lindholm <quic_llindhol@quicinc.com> wrote:
>
> On Wed, Oct 19, 2022 at 11:22:02 +0200, Ard Biesheuvel wrote:
> > The iSCSI driver slows down the boot on a pristine variable store flash
> > image, as it creates a couple of large EFI non-volatile variables to
> > preserve state between boots.
> >
> > Since iSCSI boot for VMs is kind of niche anyway, let's default to
> > disabled. If someone needs it in their build, they can use the -D build
> > command option to re-enable it on the fly.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> Apologies for bikeshedding, but ... I find the listing of tweakable
> options in a block at the start of a .dsc very useful.
> Could we explicitly set to FALSE instead?
>

Sure

> Either way:
> Acked-by: Leif Lindholm <quic_llindhol@quicinc.com>
>

Thanks.

> > ---
> >  ArmVirtPkg/ArmVirtQemu.dsc       | 1 -
> >  ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 -
> >  2 files changed, 2 deletions(-)
> >
> > diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> > index 9369a88858fd..45c4a8fc84e0 100644
> > --- a/ArmVirtPkg/ArmVirtQemu.dsc
> > +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> > @@ -40,7 +40,6 @@ [Defines]
> >    DEFINE NETWORK_SNP_ENABLE              = FALSE
> >    DEFINE NETWORK_TLS_ENABLE              = FALSE
> >    DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS  = TRUE
> > -  DEFINE NETWORK_ISCSI_ENABLE            = TRUE
> >
> >  !if $(NETWORK_SNP_ENABLE) == TRUE
> >    !error "NETWORK_SNP_ENABLE is IA32/X64/EBC only"
> > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> > index 7f7d15d6eee3..66039f07f41b 100644
> > --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> > +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> > @@ -38,7 +38,6 @@ [Defines]
> >    DEFINE NETWORK_SNP_ENABLE              = FALSE
> >    DEFINE NETWORK_TLS_ENABLE              = FALSE
> >    DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS  = TRUE
> > -  DEFINE NETWORK_ISCSI_ENABLE            = TRUE
> >
> >  !if $(NETWORK_SNP_ENABLE) == TRUE
> >    !error "NETWORK_SNP_ENABLE is IA32/X64/EBC only"
> > --
> > 2.35.1
> >

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

* Re: [PATCH v3 resend 07/11] ArmVirtPkg/ArmVirtQemu: enable initial ID map at early boot
  2022-10-19  9:22 ` [PATCH v3 resend 07/11] ArmVirtPkg/ArmVirtQemu: enable initial ID map at early boot Ard Biesheuvel
@ 2022-11-08 15:59   ` Gerd Hoffmann
  0 siblings, 0 replies; 16+ messages in thread
From: Gerd Hoffmann @ 2022-11-08 15:59 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: devel, Leif Lindholm, Alexander Graf, Sami Mujawar

On Wed, Oct 19, 2022 at 11:22:07AM +0200, Ard Biesheuvel wrote:
> Now that we have all the pieces in place, switch the AArch64 version of
> ArmVirtQemu to a mode where the first thing it does out of reset is
> enable a preliminary ID map that covers the NOR flash and sufficient
> DRAM to create the UEFI page tables as usual.
> 
> The advantage of this is that no manipulation of memory occurs any
> longer before the MMU is enabled, which removes the need for explicit
> coherency management, which is cumbersome and bad for performance.
> 
> It also means we no longer need to build all components that may execute
> with the MMU off (including BASE libraries) with strict alignment.

raspberry pi 3 host stopped working with this.  No log messages,
probably fails before enabling the serial console (not surprising
given the nature of the change).

raspberry pi 4 host continues to work fine.


Possibly related: raspberry pi 3 host (before this commit) waits forever
for a key press instead of continuing after 5 seconds timeout.  That one
is reproducible on raspberry pi 4 host using ...

	qemu-system-aarch64 -machine virt,kernel-irqchip=off

... which is default on rpi3 due to lack of a gic.  Which hints some
problem with timer interrupt (possibly a qemu bug not a edk2 armvirt
issue).

(not that much of a problem in practice given that the rpi3 isn't a
great platform with only 1G of ram, but due to supply chain problems
it is hard to get your hands on a rpi4 right now so I'm trying to use
the rpi3 I have lying around ...).

take care,
  Gerd


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

end of thread, other threads:[~2022-11-08 16:00 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-19  9:22 [PATCH v3 resend 00/11] ArmVirtPkg/ArmVirtQemu: Performance streamlining Ard Biesheuvel
2022-10-19  9:22 ` [PATCH v3 resend 01/11] ArmVirtPkg: remove EbcDxe from all platforms Ard Biesheuvel
2022-10-20  8:58   ` [edk2-devel] " Leif Lindholm
2022-10-19  9:22 ` [PATCH v3 resend 02/11] ArmVirtPkg: do not enable iSCSI driver by default Ard Biesheuvel
2022-10-20  9:03   ` Leif Lindholm
2022-10-20 12:34     ` Ard Biesheuvel
2022-10-19  9:22 ` [PATCH v3 resend 03/11] ArmVirtPkg: make EFI_LOADER_DATA non-executable Ard Biesheuvel
2022-10-19  9:22 ` [PATCH v3 resend 04/11] ArmVirtPkg/ArmVirtQemu: wire up timeout PCD to Timeout variable Ard Biesheuvel
2022-10-19  9:22 ` [PATCH v3 resend 05/11] ArmVirtPkg/ArmVirtQemu: implement ArmPlatformLib with static ID map Ard Biesheuvel
2022-10-19  9:22 ` [PATCH v3 resend 06/11] ArmVirtPkg/ArmVirtQemu: use first 128 MiB as permanent PEI memory Ard Biesheuvel
2022-10-19  9:22 ` [PATCH v3 resend 07/11] ArmVirtPkg/ArmVirtQemu: enable initial ID map at early boot Ard Biesheuvel
2022-11-08 15:59   ` Gerd Hoffmann
2022-10-19  9:22 ` [PATCH v3 resend 08/11] ArmVirtPkg/ArmVirtQemu: Drop unused variable PEIM Ard Biesheuvel
2022-10-19  9:22 ` [PATCH v3 resend 09/11] ArmVirtPkg/ArmVirtQemu: avoid shadowing PEIMs unless necessary Ard Biesheuvel
2022-10-19  9:22 ` [PATCH v3 resend 10/11] ArmVirtPkg/QemuVirtMemInfoLib: use HOB not PCD to record the memory size Ard Biesheuvel
2022-10-19  9:22 ` [PATCH v3 resend 11/11] ArmVirtPkg/ArmVirtQemu: omit PCD PEIM unless TPM support is enabled Ard Biesheuvel

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