Re: [PATCH v2 3/5] OvmfPkg: introduce 4MB flash image (mainly) for Windows HCK

[Jordan Justen <jordan.l.justen@intel.com>]

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

On 2017-05-04 21:07:24, Laszlo Ersek wrote:
> On 05/03/17 23:39, Laszlo Ersek wrote:
> >     - Raise VARS_LIVE_SIZE by 8KB to 256KB, VARS_SPARE_SIZE by 8KB to 264KB,
> 
> Unfortunately, we have a terrible regression here. :(
>

<snip>

> Adapting EmuVariableFvbRuntimeDxe to a non-power-of-two NV spare area
> size is hopeless. (Definitely hopeless for the time frame & resources
> I'm looking at.)
> 
> Worse, even -pflash is broken in the 4MB build, actually. The
> non-power-of-two NV spare area size, when used as Alignment for
> AllocateAlignedRuntimePages() in ReserveEmuVariableNvStore(), triggers
> an assertion. And this path is taken for the -pflash boot as well.

For a short term fix, would something like this work?

1. Force emu fvb buffer alignment to next power-of-two

   Something like the attachment, but I'm guessing you already wrote
   something similar.

2. Revert 4MB by default patch

This should allow you to start using the 4MB layout for your builds,
and we can fix the non-flash path before re-enabling 4MB as the
default.

-Jordan

[-- Attachment #2: 0001-PlatformPei-Force-EmuVariableNvStore-alignment-size-.patch --]
[-- Type: text/x-diff, Size: 1847 bytes --]

>From 58ba393adf60caf806b26dec9aa56d936743f595 Mon Sep 17 00:00:00 2001
From: Jordan Justen <jordan.l.justen@intel.com>
Date: Fri, 5 May 2017 01:43:31 -0700
Subject: [PATCH] PlatformPei: Force EmuVariableNvStore alignment size to power
 of two

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 OvmfPkg/PlatformPei/Platform.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 77a8a16c15..97dce8de92 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -504,6 +504,14 @@ ReserveEmuVariableNvStore (
 {
   EFI_PHYSICAL_ADDRESS VariableStore;
   RETURN_STATUS        PcdStatus;
+  UINT32               EmuFvbSize;
+  INTN                 SizeHighBit;
+
+  EmuFvbSize = 2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize);
+  SizeHighBit = HighBitSet32 (EmuFvbSize);
+  if ((EmuFvbSize & (EmuFvbSize - 1)) != 0) {
+    SizeHighBit++;
+  }
 
   //
   // Allocate storage for NV variables early on so it will be
@@ -514,13 +522,13 @@ ReserveEmuVariableNvStore (
   VariableStore =
     (EFI_PHYSICAL_ADDRESS)(UINTN)
       AllocateAlignedRuntimePages (
-        EFI_SIZE_TO_PAGES (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)),
-        PcdGet32 (PcdFlashNvStorageFtwSpareSize)
+        EFI_SIZE_TO_PAGES (EmuFvbSize),
+        1 << (SizeHighBit - 1)
         );
   DEBUG ((EFI_D_INFO,
           "Reserved variable store memory: 0x%lX; size: %dkb\n",
           VariableStore,
-          (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)) / 1024
+          EmuFvbSize / 1024
         ));
   PcdStatus = PcdSet64S (PcdEmuVariableNvStoreReserved, VariableStore);
   ASSERT_RETURN_ERROR (PcdStatus);
-- 
2.11.0