public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables)
@ 2017-05-06 19:30 Laszlo Ersek
  2017-05-06 19:30 ` [PATCH 1/5] OvmfPkg/EmuVariableFvbRuntimeDxe: strip trailing whitespace Laszlo Ersek
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Laszlo Ersek @ 2017-05-06 19:30 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Gary Ching-Pang Lin, Jordan Justen

(All hail Saturday!)

Gary, can you please fetch this from my repo (URL & branch name below)
and test it with Xen? Please test both the 4MB and the 2MB build. (I
also tested both, with qemu + "-bios".)

Note: this series depends on:

  [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks
  https://lists.01.org/pipermail/edk2-devel/2017-May/010527.html

and it has been pushed to my github repo as such.

Repo:   https://github.com/lersek/edk2.git
Branch: emu4k

Cc: Gary Ching-Pang Lin <glin@suse.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>

Thanks,
Laszlo

Laszlo Ersek (5):
  OvmfPkg/EmuVariableFvbRuntimeDxe: strip trailing whitespace
  OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB
  OvmfPkg/PlatformPei: align EmuVariableNvStore at any page boundary
  OvmfPkg/README: document 4MB flash layout
  OvmfPkg: make the 4MB flash size the default (again)

 OvmfPkg/OvmfPkgIa32.dsc                |   2 +-
 OvmfPkg/OvmfPkgIa32X64.dsc             |   2 +-
 OvmfPkg/OvmfPkgX64.dsc                 |   2 +-
 OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h |  26 ++-
 OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 191 +++++++++-----------
 OvmfPkg/PlatformPei/Platform.c         |  20 +-
 OvmfPkg/README                         |  39 +++-
 7 files changed, 143 insertions(+), 139 deletions(-)

-- 
2.9.3



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

* [PATCH 1/5] OvmfPkg/EmuVariableFvbRuntimeDxe: strip trailing whitespace
  2017-05-06 19:30 [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables) Laszlo Ersek
@ 2017-05-06 19:30 ` Laszlo Ersek
  2017-05-12  9:07   ` Gary Lin
  2017-05-06 19:30 ` [PATCH 2/5] OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB Laszlo Ersek
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Laszlo Ersek @ 2017-05-06 19:30 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Jordan Justen

Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h | 16 +++----
 OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 44 ++++++++++----------
 2 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
index f34fad2cdd48..4247d21d72f8 100644
--- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
+++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
@@ -1,18 +1,18 @@
 /*++
 
 Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
-This program and the accompanying materials                          
-are licensed and made available under the terms and conditions of the BSD License         
-which accompanies this distribution.  The full text of the license may be found at        
-http://opensource.org/licenses/bsd-license.php                                            
-                                                                                          
-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,                     
-WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.             
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD License
+which accompanies this distribution.  The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 Module Name:
 
   FwBlockService.h
-  
+
 Abstract:
 
   Firmware volume block driver for Intel Firmware Hub (FWH) device
diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
index 7a6d3153ec8c..2f89632e5d75 100644
--- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
+++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
@@ -120,14 +120,14 @@ FvbVirtualAddressChangeEvent (
   only for memory-mapped firmware volumes.
 
   @param This     Indicates the EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL instance.
-  
+
   @param Address  Pointer to a caller-allocated
                   EFI_PHYSICAL_ADDRESS that, on successful
                   return from GetPhysicalAddress(), contains the
                   base address of the firmware volume.
-  
+
   @retval EFI_SUCCESS       The firmware volume base address is returned.
-  
+
   @retval EFI_NOT_SUPPORTED The firmware volume is not memory mapped.
 
 **/
@@ -168,9 +168,9 @@ FvbProtocolGetPhysicalAddress (
                         blocks in this range have a size of
                         BlockSize.
 
-  
+
   @retval EFI_SUCCESS             The firmware volume base address is returned.
-  
+
   @retval EFI_INVALID_PARAMETER   The requested LBA is out of range.
 
 **/
@@ -246,7 +246,7 @@ FvbProtocolGetAttributes (
                       settings of the firmware volume. Type
                       EFI_FVB_ATTRIBUTES_2 is defined in
                       EFI_FIRMWARE_VOLUME_HEADER.
-  
+
   @retval EFI_SUCCESS           The firmware volume attributes were returned.
 
   @retval EFI_INVALID_PARAMETER The attributes requested are in
@@ -302,7 +302,7 @@ FvbProtocolSetAttributes (
 
   @retval EFI_SUCCESS The erase request was successfully
                       completed.
-  
+
   @retval EFI_ACCESS_DENIED   The firmware volume is in the
                               WriteDisabled state.
   @retval EFI_DEVICE_ERROR  The block device is not functioning
@@ -311,7 +311,7 @@ FvbProtocolSetAttributes (
                             partially erased.
   @retval EFI_INVALID_PARAMETER One or more of the LBAs listed
                                 in the variable argument list do
-                                not exist in the firmware volume.  
+                                not exist in the firmware volume.
 
 **/
 EFI_STATUS
@@ -420,29 +420,29 @@ FvbProtocolEraseBlocks (
   returns.
 
   @param This     Indicates the EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL instance.
-  
+
   @param Lba      The starting logical block index to write to.
-  
+
   @param Offset   Offset into the block at which to begin writing.
-  
+
   @param NumBytes Pointer to a UINTN. At entry, *NumBytes
                   contains the total size of the buffer. At
                   exit, *NumBytes contains the total number of
                   bytes actually written.
-  
+
   @param Buffer   Pointer to a caller-allocated buffer that
                   contains the source for the write.
-  
+
   @retval EFI_SUCCESS         The firmware volume was written successfully.
-  
+
   @retval EFI_BAD_BUFFER_SIZE The write was attempted across an
                               LBA boundary. On output, NumBytes
                               contains the total number of bytes
                               actually written.
-  
+
   @retval EFI_ACCESS_DENIED   The firmware volume is in the
                               WriteDisabled state.
-  
+
   @retval EFI_DEVICE_ERROR    The block device is malfunctioning
                               and could not be written.
 
@@ -503,7 +503,7 @@ FvbProtocolWrite (
   aware that a read may be partially completed.
 
   @param This     Indicates the EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL instance.
-  
+
   @param Lba      The starting logical block index
                   from which to read.
 
@@ -519,15 +519,15 @@ FvbProtocolWrite (
 
   @retval EFI_SUCCESS         The firmware volume was read successfully
                               and contents are in Buffer.
-  
+
   @retval EFI_BAD_BUFFER_SIZE Read attempted across an LBA
                               boundary. On output, NumBytes
                               contains the total number of bytes
                               returned in Buffer.
-  
+
   @retval EFI_ACCESS_DENIED   The firmware volume is in the
                               ReadDisabled state.
-  
+
   @retval EFI_DEVICE_ERROR    The block device is not
                               functioning correctly and could
                               not be read.
@@ -715,9 +715,9 @@ InitializeFvAndVariableStoreHeaders (
 /**
   Main entry point.
 
-  @param[in] ImageHandle    The firmware allocated handle for the EFI image.  
+  @param[in] ImageHandle    The firmware allocated handle for the EFI image.
   @param[in] SystemTable    A pointer to the EFI System Table.
-  
+
   @retval EFI_SUCCESS       Successfully initialized.
 
 **/
-- 
2.9.3




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

* [PATCH 2/5] OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB
  2017-05-06 19:30 [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables) Laszlo Ersek
  2017-05-06 19:30 ` [PATCH 1/5] OvmfPkg/EmuVariableFvbRuntimeDxe: strip trailing whitespace Laszlo Ersek
@ 2017-05-06 19:30 ` Laszlo Ersek
  2017-05-12  9:07   ` Gary Lin
  2017-05-15 23:50   ` Jordan Justen
  2017-05-06 19:30 ` [PATCH 3/5] OvmfPkg/PlatformPei: align EmuVariableNvStore at any page boundary Laszlo Ersek
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 19+ messages in thread
From: Laszlo Ersek @ 2017-05-06 19:30 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Jordan Justen

EmuVariableFvbRuntimeDxe currently produces a Firmware Volume Block
protocol that is based on a block map of two blocks, each block having
PcdFlashNvStorageFtwSpareSize for size.

(The total size is 2 * PcdFlashNvStorageFtwSpareSize.)

FaultTolerantWriteDxe in turn expects the block size to be a power of two.

In the 4MB build of OVMF, PcdFlashNvStorageFtwSpareSize is 264KB, which is
not a power of two. In order to equip EmuVariableFvbRuntimeDxe for this
build, shrink the block size to 4KB (EFI_PAGE_SIZE), and grow the block
count from 2 to EFI_SIZE_TO_PAGES(2 * PcdFlashNvStorageFtwSpareSize). The
total size remains

  2 * PcdFlashNvStorageFtwSpareSize
  --------------------------------- * EFI_PAGE_SIZE
            EFI_PAGE_SIZE

Right now EmuVariableFvbRuntimeDxe open-codes the block count of 2 in
various limit checks, so introduce a few new macros:
- EMU_FVB_NUM_TOTAL_BLOCKS, for the LHS of the above product,
- EMU_FVB_NUM_SPARE_BLOCKS for the half of that.

Also rework the FVB protocol members to support an arbitrary count of
blocks.

Keep the invariant intact that the first half of the firmware volume hosts
the variable store and the FTW working block, and that the second half
maps the FTW spare area.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=527
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h |  10 +-
 OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 147 +++++++++-----------
 2 files changed, 75 insertions(+), 82 deletions(-)

diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
index 4247d21d72f8..beb11e3f9a90 100644
--- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
+++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
@@ -58,8 +58,14 @@ typedef struct {
 //
 // Constants
 //
-#define EMU_FVB_BLOCK_SIZE (FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize))
-#define EMU_FVB_SIZE (2 * FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize))
+#define EMU_FVB_BLOCK_SIZE \
+  EFI_PAGE_SIZE
+#define EMU_FVB_NUM_SPARE_BLOCKS \
+  EFI_SIZE_TO_PAGES ((UINTN)FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize))
+#define EMU_FVB_NUM_TOTAL_BLOCKS \
+  (2 * EMU_FVB_NUM_SPARE_BLOCKS)
+#define EMU_FVB_SIZE \
+  (EMU_FVB_NUM_TOTAL_BLOCKS * EMU_FVB_BLOCK_SIZE)
 #define FTW_WRITE_QUEUE_SIZE \
   (FixedPcdGet32 (PcdFlashNvStorageFtwWorkingSize) - \
    sizeof (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER))
diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
index 2f89632e5d75..11c8b1b75cb8 100644
--- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
+++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
@@ -74,8 +74,8 @@ EFI_FW_VOL_BLOCK_DEVICE mEmuVarsFvb = {
     }
   },
   NULL, // BufferPtr
-  FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize), // BlockSize
-  2 * FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize), // Size
+  EMU_FVB_BLOCK_SIZE, // BlockSize
+  EMU_FVB_SIZE, // Size
   {     // FwVolBlockInstance
     FvbProtocolGetAttributes,
     FvbProtocolSetAttributes,
@@ -185,14 +185,14 @@ FvbProtocolGetBlockSize (
 {
   EFI_FW_VOL_BLOCK_DEVICE *FvbDevice;
 
-  if (Lba > 1) {
+  if (Lba >= EMU_FVB_NUM_TOTAL_BLOCKS) {
     return EFI_INVALID_PARAMETER;
   }
 
   FvbDevice = FVB_DEVICE_FROM_THIS (This);
 
   *BlockSize = FvbDevice->BlockSize;
-  *NumberOfBlocks = (UINTN) (2 - (UINTN) Lba);
+  *NumberOfBlocks = (UINTN)(EMU_FVB_NUM_TOTAL_BLOCKS - Lba);
 
   return EFI_SUCCESS;
 }
@@ -322,68 +322,58 @@ FvbProtocolEraseBlocks (
   )
 {
   EFI_FW_VOL_BLOCK_DEVICE *FvbDevice;
-  VA_LIST                 args;
+  VA_LIST                 Args;
   EFI_LBA                 StartingLba;
   UINTN                   NumOfLba;
-  UINT8                   Erase;
-  VOID                    *ErasePtr;
+  UINT8                   *ErasePtr;
   UINTN                   EraseSize;
 
   FvbDevice = FVB_DEVICE_FROM_THIS (This);
-  Erase = 0;
-
-  VA_START (args, This);
 
+  //
+  // Check input parameters
+  //
+  VA_START (Args, This);
   do {
-    StartingLba = VA_ARG (args, EFI_LBA);
+    StartingLba = VA_ARG (Args, EFI_LBA);
     if (StartingLba == EFI_LBA_LIST_TERMINATOR) {
       break;
     }
+    NumOfLba = VA_ARG (Args, UINTN);
 
-    NumOfLba = VA_ARG (args, UINT32);
-
-    //
-    // Check input parameters
-    //
-    if ((NumOfLba == 0) || (StartingLba > 1) || ((StartingLba + NumOfLba) > 2)) {
-      VA_END (args);
+    if (StartingLba > EMU_FVB_NUM_TOTAL_BLOCKS ||
+        NumOfLba > EMU_FVB_NUM_TOTAL_BLOCKS - StartingLba) {
+      VA_END (Args);
       return EFI_INVALID_PARAMETER;
     }
-
-    if (StartingLba == 0) {
-      Erase = (UINT8) (Erase | BIT0);
-    }
-    if ((StartingLba + NumOfLba) == 2) {
-      Erase = (UINT8) (Erase | BIT1);
-    }
-
   } while (1);
+  VA_END (Args);
 
-  VA_END (args);
-
-  ErasePtr = (UINT8*) FvbDevice->BufferPtr;
-  EraseSize = 0;
+  //
+  // Erase blocks
+  //
+  VA_START (Args, This);
+  do {
+    StartingLba = VA_ARG (Args, EFI_LBA);
+    if (StartingLba == EFI_LBA_LIST_TERMINATOR) {
+      break;
+    }
+    NumOfLba = VA_ARG (Args, UINTN);
 
-  if ((Erase & BIT0) != 0) {
-    EraseSize = EraseSize + FvbDevice->BlockSize;
-  } else {
-    ErasePtr = (VOID*) ((UINT8*)ErasePtr + FvbDevice->BlockSize);
-  }
+    ErasePtr = FvbDevice->BufferPtr;
+    ErasePtr += (UINTN)StartingLba * FvbDevice->BlockSize;
+    EraseSize = NumOfLba * FvbDevice->BlockSize;
 
-  if ((Erase & BIT1) != 0) {
-    EraseSize = EraseSize + FvbDevice->BlockSize;
-  }
+    SetMem (ErasePtr, EraseSize, ERASED_UINT8);
+  } while (1);
+  VA_END (Args);
 
-  if (EraseSize != 0) {
-    SetMem (
-      (VOID*) ErasePtr,
-      EraseSize,
-      ERASED_UINT8
-      );
-    VA_START (args, This);
-    PlatformFvbBlocksErased (This, args);
-    VA_END (args);
-  }
+  //
+  // Call platform hook
+  //
+  VA_START (Args, This);
+  PlatformFvbBlocksErased (This, Args);
+  VA_END (Args);
 
   return EFI_SUCCESS;
 }
@@ -458,31 +448,30 @@ FvbProtocolWrite (
   IN        UINT8                               *Buffer
   )
 {
-
   EFI_FW_VOL_BLOCK_DEVICE *FvbDevice;
   UINT8                   *FvbDataPtr;
+  EFI_STATUS              Status;
 
   FvbDevice = FVB_DEVICE_FROM_THIS (This);
 
-  if ((Lba > 1) || (Offset > FvbDevice->BlockSize)) {
+  if (Lba >= EMU_FVB_NUM_TOTAL_BLOCKS ||
+      Offset > FvbDevice->BlockSize) {
     return EFI_INVALID_PARAMETER;
   }
 
-  if ((Offset + *NumBytes) > FvbDevice->BlockSize) {
+  Status = EFI_SUCCESS;
+  if (*NumBytes > FvbDevice->BlockSize - Offset) {
     *NumBytes = FvbDevice->BlockSize - Offset;
+    Status = EFI_BAD_BUFFER_SIZE;
   }
 
-  FvbDataPtr =
-    (UINT8*) FvbDevice->BufferPtr +
-    MultU64x32 (Lba, (UINT32) FvbDevice->BlockSize) +
-    Offset;
+  FvbDataPtr = FvbDevice->BufferPtr;
+  FvbDataPtr += (UINTN)Lba * FvbDevice->BlockSize;
+  FvbDataPtr += Offset;
 
-  if (*NumBytes > 0) {
-    CopyMem (FvbDataPtr, Buffer, *NumBytes);
-    PlatformFvbDataWritten (This, Lba, Offset, *NumBytes, Buffer);
-  }
-
-  return EFI_SUCCESS;
+  CopyMem (FvbDataPtr, Buffer, *NumBytes);
+  PlatformFvbDataWritten (This, Lba, Offset, *NumBytes, Buffer);
+  return Status;
 }
 
 
@@ -545,28 +534,28 @@ FvbProtocolRead (
 {
   EFI_FW_VOL_BLOCK_DEVICE *FvbDevice;
   UINT8                   *FvbDataPtr;
+  EFI_STATUS              Status;
 
   FvbDevice = FVB_DEVICE_FROM_THIS (This);
 
-  if ((Lba > 1) || (Offset > FvbDevice->BlockSize)) {
+  if (Lba >= EMU_FVB_NUM_TOTAL_BLOCKS ||
+      Offset > FvbDevice->BlockSize) {
     return EFI_INVALID_PARAMETER;
   }
 
-  if ((Offset + *NumBytes) > FvbDevice->BlockSize) {
+  Status = EFI_SUCCESS;
+  if (*NumBytes > FvbDevice->BlockSize - Offset) {
     *NumBytes = FvbDevice->BlockSize - Offset;
+    Status = EFI_BAD_BUFFER_SIZE;
   }
 
-  FvbDataPtr =
-    (UINT8*) FvbDevice->BufferPtr +
-    MultU64x32 (Lba, (UINT32) FvbDevice->BlockSize) +
-    Offset;
+  FvbDataPtr = FvbDevice->BufferPtr;
+  FvbDataPtr += (UINTN)Lba * FvbDevice->BlockSize;
+  FvbDataPtr += Offset;
 
-  if (*NumBytes > 0) {
-    CopyMem (Buffer, FvbDataPtr, *NumBytes);
-    PlatformFvbDataRead (This, Lba, Offset, *NumBytes, Buffer);
-  }
-
-  return EFI_SUCCESS;
+  CopyMem (Buffer, FvbDataPtr, *NumBytes);
+  PlatformFvbDataRead (This, Lba, Offset, *NumBytes, Buffer);
+  return Status;
 }
 
 
@@ -663,7 +652,7 @@ InitializeFvAndVariableStoreHeaders (
       // EFI_FV_BLOCK_MAP_ENTRY    BlockMap[1];
       {
         {
-          2, // UINT32 NumBlocks;
+          EMU_FVB_NUM_TOTAL_BLOCKS, // UINT32 NumBlocks;
           EMU_FVB_BLOCK_SIZE  // UINT32 Length;
         }
       }
@@ -745,7 +734,7 @@ FvbInitialize (
        (PcdGet32 (PcdVariableStoreSize) +
         PcdGet32 (PcdFlashNvStorageFtwWorkingSize)
        ) >
-       EMU_FVB_BLOCK_SIZE
+       EMU_FVB_NUM_SPARE_BLOCKS * EMU_FVB_BLOCK_SIZE
      ) {
     DEBUG ((EFI_D_ERROR, "EMU Variable invalid PCD sizes\n"));
     return EFI_INVALID_PARAMETER;
@@ -779,10 +768,7 @@ FvbInitialize (
       Initialize = FALSE;
     }
   } else {
-    Ptr = AllocateAlignedRuntimePages (
-            EFI_SIZE_TO_PAGES (EMU_FVB_SIZE),
-            SIZE_64KB
-            );
+    Ptr = AllocateRuntimePages (EFI_SIZE_TO_PAGES (EMU_FVB_SIZE));
   }
 
   mEmuVarsFvb.BufferPtr = Ptr;
@@ -808,7 +794,8 @@ FvbInitialize (
   //
   // Initialize the Fault Tolerant Write spare block
   //
-  SubPtr = (VOID*) ((UINT8*) Ptr + EMU_FVB_BLOCK_SIZE);
+  SubPtr = (VOID*) ((UINT8*) Ptr +
+                    EMU_FVB_NUM_SPARE_BLOCKS * EMU_FVB_BLOCK_SIZE);
   PcdStatus = PcdSet32S (PcdFlashNvStorageFtwSpareBase,
                 (UINT32)(UINTN) SubPtr);
   ASSERT_RETURN_ERROR (PcdStatus);
-- 
2.9.3




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

* [PATCH 3/5] OvmfPkg/PlatformPei: align EmuVariableNvStore at any page boundary
  2017-05-06 19:30 [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables) Laszlo Ersek
  2017-05-06 19:30 ` [PATCH 1/5] OvmfPkg/EmuVariableFvbRuntimeDxe: strip trailing whitespace Laszlo Ersek
  2017-05-06 19:30 ` [PATCH 2/5] OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB Laszlo Ersek
@ 2017-05-06 19:30 ` Laszlo Ersek
  2017-05-12  9:08   ` Gary Lin
  2017-05-06 19:30 ` [PATCH 4/5] OvmfPkg/README: document 4MB flash layout Laszlo Ersek
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Laszlo Ersek @ 2017-05-06 19:30 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Jordan Justen

EmuVariableFvbRuntimeDxe now uses a 4KB (EFI_PAGE_SIZE) block size.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/PlatformPei/Platform.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 1b4dc00b0180..3e9fda7c7ab0 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -504,7 +504,6 @@ ReserveEmuVariableNvStore (
 {
   EFI_PHYSICAL_ADDRESS VariableStore;
   RETURN_STATUS        PcdStatus;
-  UINT32               Alignment;
 
   //
   // Allocate storage for NV variables early on so it will be
@@ -512,26 +511,15 @@ ReserveEmuVariableNvStore (
   // across reboots, this allows the NV variable storage to survive
   // a VM reboot.
   //
-  Alignment = PcdGet32 (PcdFlashNvStorageFtwSpareSize);
-  if ((Alignment & (Alignment - 1)) != 0) {
-    //
-    // Round up Alignment to the next power of two.
-    //
-    Alignment = GetPowerOfTwo32 (Alignment) << 1;
-  }
-
   VariableStore =
     (EFI_PHYSICAL_ADDRESS)(UINTN)
-      AllocateAlignedRuntimePages (
-        EFI_SIZE_TO_PAGES (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)),
-        Alignment
+      AllocateRuntimePages (
+        EFI_SIZE_TO_PAGES (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize))
         );
   DEBUG ((EFI_D_INFO,
-          "Reserved variable store memory: 0x%lX; size: %dkb, "
-          "alignment: 0x%x\n",
+          "Reserved variable store memory: 0x%lX; size: %dkb\n",
           VariableStore,
-          (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)) / 1024,
-          Alignment
+          (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)) / 1024
         ));
   PcdStatus = PcdSet64S (PcdEmuVariableNvStoreReserved, VariableStore);
   ASSERT_RETURN_ERROR (PcdStatus);
-- 
2.9.3




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

* [PATCH 4/5] OvmfPkg/README: document 4MB flash layout
  2017-05-06 19:30 [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables) Laszlo Ersek
                   ` (2 preceding siblings ...)
  2017-05-06 19:30 ` [PATCH 3/5] OvmfPkg/PlatformPei: align EmuVariableNvStore at any page boundary Laszlo Ersek
@ 2017-05-06 19:30 ` Laszlo Ersek
  2017-05-06 19:30 ` [PATCH 5/5] OvmfPkg: make the 4MB flash size the default (again) Laszlo Ersek
  2017-05-08  4:27 ` [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables) Gary Lin
  5 siblings, 0 replies; 19+ messages in thread
From: Laszlo Ersek @ 2017-05-06 19:30 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Jordan Justen

Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=527
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/README | 39 ++++++++++++++++----
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/OvmfPkg/README b/OvmfPkg/README
index 304e69fbe545..33ff9432bb3e 100644
--- a/OvmfPkg/README
+++ b/OvmfPkg/README
@@ -245,15 +245,20 @@ longer.)
 
 === OVMF Flash Layout ===
 
-Like all current IA32/X64 system designs, OVMF's firmware
-device (rom/flash) appears in QEMU's physical address space
-just below 4GB (0x100000000).
+Like all current IA32/X64 system designs, OVMF's firmware device (rom/flash)
+appears in QEMU's physical address space just below 4GB (0x100000000).
 
-The layout of the firmware device in memory looks like:
+OVMF supports building a 1MB, 2MB or 4MB flash image (see the DSC files for the
+FD_SIZE_1MB, FD_SIZE_2MB, FD_SIZE_4MB build defines). The base address for the
+1MB image in QEMU physical memory is 0xfff00000. The base address for the 2MB
+image is 0xffe00000. The base address for the 4MB image is 0xffc00000.
+
+Using the 1MB or 2MB image, the layout of the firmware device in memory looks
+like:
 
 +--------------------------------------- 4GB (0x100000000)
 | VTF0 (16-bit reset code) and OVMF SEC
-| (SECFV)
+| (SECFV, 208KB/0x34000)
 +--------------------------------------- varies based on flash size
 |
 | Compressed main firmware image
@@ -271,9 +276,27 @@ The layout of the firmware device in memory looks like:
 | area (56KB/0xe000)
 +--------------------------------------- base address
 
-OVMF supports building a 1MB or a 2MB flash image. The base address for
-a 1MB image in QEMU physical memory is 0xfff00000. The base address for
-a 2MB image is 0xffe00000.
+Using the 4MB image, the layout of the firmware device in memory looks like:
+
++--------------------------------------- base + 0x400000 (4GB/0x100000000)
+| VTF0 (16-bit reset code) and OVMF SEC
+| (SECFV, 208KB/0x34000)
++--------------------------------------- base + 0x3cc000
+|
+| Compressed main firmware image
+| (FVMAIN_COMPACT, 3360KB/0x348000)
+|
++--------------------------------------- base + 0x84000
+| Fault-tolerant write (FTW)
+| Spare blocks (264KB/0x42000)
++--------------------------------------- base + 0x42000
+| FTW Work block (4KB/0x1000)
++--------------------------------------- base + 0x41000
+| Event log area (4KB/0x1000)
++--------------------------------------- base + 0x40000
+| Non-volatile variable storage
+| area (256KB/0x40000)
++--------------------------------------- base address (0xffc00000)
 
 The code in SECFV locates FVMAIN_COMPACT, and decompresses the
 main firmware (MAINFV) into RAM memory at address 0x800000. The
-- 
2.9.3




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

* [PATCH 5/5] OvmfPkg: make the 4MB flash size the default (again)
  2017-05-06 19:30 [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables) Laszlo Ersek
                   ` (3 preceding siblings ...)
  2017-05-06 19:30 ` [PATCH 4/5] OvmfPkg/README: document 4MB flash layout Laszlo Ersek
@ 2017-05-06 19:30 ` Laszlo Ersek
  2017-05-08  4:27 ` [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables) Gary Lin
  5 siblings, 0 replies; 19+ messages in thread
From: Laszlo Ersek @ 2017-05-06 19:30 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Jordan Justen

The previously default 2MB can be explicitly selected with

  -D FD_SIZE_2MB

or

  -D FD_SIZE_IN_KB=2048

Cc: Jordan Justen <jordan.l.justen@intel.com>
Suggested-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
(cherry picked from commit bba8dfbec3bbc4fba7fa6398ba3cf76593e0725e)
---
 OvmfPkg/OvmfPkgIa32.dsc    | 2 +-
 OvmfPkg/OvmfPkgIa32X64.dsc | 2 +-
 OvmfPkg/OvmfPkgX64.dsc     | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index bd115c9ced93..8f0ab9f721fe 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -54,7 +54,7 @@ [Defines]
 !ifdef $(FD_SIZE_4MB)
   DEFINE FD_SIZE_IN_KB           = 4096
 !else
-  DEFINE FD_SIZE_IN_KB           = 2048
+  DEFINE FD_SIZE_IN_KB           = 4096
 !endif
 !endif
 !endif
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 9727db842922..4b2136262865 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -54,7 +54,7 @@ [Defines]
 !ifdef $(FD_SIZE_4MB)
   DEFINE FD_SIZE_IN_KB           = 4096
 !else
-  DEFINE FD_SIZE_IN_KB           = 2048
+  DEFINE FD_SIZE_IN_KB           = 4096
 !endif
 !endif
 !endif
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 61aaed761657..2ee30fb00a28 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -54,7 +54,7 @@ [Defines]
 !ifdef $(FD_SIZE_4MB)
   DEFINE FD_SIZE_IN_KB           = 4096
 !else
-  DEFINE FD_SIZE_IN_KB           = 2048
+  DEFINE FD_SIZE_IN_KB           = 4096
 !endif
 !endif
 !endif
-- 
2.9.3



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

* Re: [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables)
  2017-05-06 19:30 [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables) Laszlo Ersek
                   ` (4 preceding siblings ...)
  2017-05-06 19:30 ` [PATCH 5/5] OvmfPkg: make the 4MB flash size the default (again) Laszlo Ersek
@ 2017-05-08  4:27 ` Gary Lin
  2017-05-12  2:02   ` Gary Lin
  5 siblings, 1 reply; 19+ messages in thread
From: Gary Lin @ 2017-05-08  4:27 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: edk2-devel-01, Jordan Justen

On Sat, May 06, 2017 at 09:30:18PM +0200, Laszlo Ersek wrote:
> (All hail Saturday!)
> 
> Gary, can you please fetch this from my repo (URL & branch name below)
> and test it with Xen? Please test both the 4MB and the 2MB build. (I
> also tested both, with qemu + "-bios".)
Hi Laszlo,

I have done some simples test with xen, and the 2MB build seems fine.
It booted into grub2 menu successfully. However, the 4MB build never boots.
The QEMU window showed less than 1 sec and then disappeared.

Here is the snippet from 'xl dmesg'

(d15)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... done.
(d15) Testing HVM environment:
(d15)  - REP INSB across page boundaries ... passed
(d15)  - GS base MSRs and SWAPGS ... passed
(d15) Passed 2 of 2 tests
(d15) Writing SMBIOS tables ...
(d15) Loading OVMF ...
(d15) no BIOS ROM image found
(d15) *** HVMLoader bug at hvmloader.c:381
(d15) *** HVMLoader crashed.

I'm pretty sure that the ovmf path is right, so it seems Xen just rejected
the 4MB build :-\

I'll try to dig more information.

Cheeres,

Gary Lin

> 
> Note: this series depends on:
> 
>   [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks
>   https://lists.01.org/pipermail/edk2-devel/2017-May/010527.html
> 
> and it has been pushed to my github repo as such.
> 
> Repo:   https://github.com/lersek/edk2.git
> Branch: emu4k
> 
> Cc: Gary Ching-Pang Lin <glin@suse.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> 
> Thanks,
> Laszlo
> 
> Laszlo Ersek (5):
>   OvmfPkg/EmuVariableFvbRuntimeDxe: strip trailing whitespace
>   OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB
>   OvmfPkg/PlatformPei: align EmuVariableNvStore at any page boundary
>   OvmfPkg/README: document 4MB flash layout
>   OvmfPkg: make the 4MB flash size the default (again)
> 
>  OvmfPkg/OvmfPkgIa32.dsc                |   2 +-
>  OvmfPkg/OvmfPkgIa32X64.dsc             |   2 +-
>  OvmfPkg/OvmfPkgX64.dsc                 |   2 +-
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h |  26 ++-
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 191 +++++++++-----------
>  OvmfPkg/PlatformPei/Platform.c         |  20 +-
>  OvmfPkg/README                         |  39 +++-
>  7 files changed, 143 insertions(+), 139 deletions(-)
> 
> -- 
> 2.9.3
> 
> 


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

* Re: [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables)
  2017-05-08  4:27 ` [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables) Gary Lin
@ 2017-05-12  2:02   ` Gary Lin
  2017-05-12  8:40     ` Laszlo Ersek
  0 siblings, 1 reply; 19+ messages in thread
From: Gary Lin @ 2017-05-12  2:02 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: Jordan Justen, edk2-devel-01

On Mon, May 08, 2017 at 12:27:59PM +0800, Gary Lin wrote:
> On Sat, May 06, 2017 at 09:30:18PM +0200, Laszlo Ersek wrote:
> > (All hail Saturday!)
> > 
> > Gary, can you please fetch this from my repo (URL & branch name below)
> > and test it with Xen? Please test both the 4MB and the 2MB build. (I
> > also tested both, with qemu + "-bios".)
> Hi Laszlo,
> 
> I have done some simples test with xen, and the 2MB build seems fine.
> It booted into grub2 menu successfully. However, the 4MB build never boots.
> The QEMU window showed less than 1 sec and then disappeared.
> 
> Here is the snippet from 'xl dmesg'
> 
> (d15)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... done.
> (d15) Testing HVM environment:
> (d15)  - REP INSB across page boundaries ... passed
> (d15)  - GS base MSRs and SWAPGS ... passed
> (d15) Passed 2 of 2 tests
> (d15) Writing SMBIOS tables ...
> (d15) Loading OVMF ...
> (d15) no BIOS ROM image found
> (d15) *** HVMLoader bug at hvmloader.c:381
> (d15) *** HVMLoader crashed.
> 
> I'm pretty sure that the ovmf path is right, so it seems Xen just rejected
> the 4MB build :-\
> 
> I'll try to dig more information.
> 
There is a function in the xen hvmloader clearing the memory from
0x400000 to 0x800000. Unfortunately, the hvm_start_info struct of the
4MB OVMF was loaded to 0x588000, so the struct was cleared mistakenly
and hvmloader cannot find the firmware. Xen is not ready for the 4MB
build yet :-\

The discussion in xen-devel:
https://lists.xen.org/archives/html/xen-devel/2017-05/msg01053.html

Cheers,

Gary Lin

> Cheeres,
> 
> Gary Lin
> 
> > 
> > Note: this series depends on:
> > 
> >   [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks
> >   https://lists.01.org/pipermail/edk2-devel/2017-May/010527.html
> > 
> > and it has been pushed to my github repo as such.
> > 
> > Repo:   https://github.com/lersek/edk2.git
> > Branch: emu4k
> > 
> > Cc: Gary Ching-Pang Lin <glin@suse.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > 
> > Thanks,
> > Laszlo
> > 
> > Laszlo Ersek (5):
> >   OvmfPkg/EmuVariableFvbRuntimeDxe: strip trailing whitespace
> >   OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB
> >   OvmfPkg/PlatformPei: align EmuVariableNvStore at any page boundary
> >   OvmfPkg/README: document 4MB flash layout
> >   OvmfPkg: make the 4MB flash size the default (again)
> > 
> >  OvmfPkg/OvmfPkgIa32.dsc                |   2 +-
> >  OvmfPkg/OvmfPkgIa32X64.dsc             |   2 +-
> >  OvmfPkg/OvmfPkgX64.dsc                 |   2 +-
> >  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h |  26 ++-
> >  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 191 +++++++++-----------
> >  OvmfPkg/PlatformPei/Platform.c         |  20 +-
> >  OvmfPkg/README                         |  39 +++-
> >  7 files changed, 143 insertions(+), 139 deletions(-)
> > 
> > -- 
> > 2.9.3
> > 
> > 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 


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

* Re: [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables)
  2017-05-12  2:02   ` Gary Lin
@ 2017-05-12  8:40     ` Laszlo Ersek
  2017-05-16  0:40       ` Jordan Justen
  0 siblings, 1 reply; 19+ messages in thread
From: Laszlo Ersek @ 2017-05-12  8:40 UTC (permalink / raw)
  To: Gary Lin; +Cc: Jordan Justen, edk2-devel-01

On 05/12/17 04:02, Gary Lin wrote:
> On Mon, May 08, 2017 at 12:27:59PM +0800, Gary Lin wrote:
>> On Sat, May 06, 2017 at 09:30:18PM +0200, Laszlo Ersek wrote:
>>> (All hail Saturday!)
>>>
>>> Gary, can you please fetch this from my repo (URL & branch name below)
>>> and test it with Xen? Please test both the 4MB and the 2MB build. (I
>>> also tested both, with qemu + "-bios".)
>> Hi Laszlo,
>>
>> I have done some simples test with xen, and the 2MB build seems fine.
>> It booted into grub2 menu successfully. However, the 4MB build never boots.
>> The QEMU window showed less than 1 sec and then disappeared.
>>
>> Here is the snippet from 'xl dmesg'
>>
>> (d15)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... done.
>> (d15) Testing HVM environment:
>> (d15)  - REP INSB across page boundaries ... passed
>> (d15)  - GS base MSRs and SWAPGS ... passed
>> (d15) Passed 2 of 2 tests
>> (d15) Writing SMBIOS tables ...
>> (d15) Loading OVMF ...
>> (d15) no BIOS ROM image found
>> (d15) *** HVMLoader bug at hvmloader.c:381
>> (d15) *** HVMLoader crashed.
>>
>> I'm pretty sure that the ovmf path is right, so it seems Xen just rejected
>> the 4MB build :-\
>>
>> I'll try to dig more information.
>>
> There is a function in the xen hvmloader clearing the memory from
> 0x400000 to 0x800000. Unfortunately, the hvm_start_info struct of the
> 4MB OVMF was loaded to 0x588000, so the struct was cleared mistakenly
> and hvmloader cannot find the firmware. Xen is not ready for the 4MB
> build yet :-\
> 
> The discussion in xen-devel:
> https://lists.xen.org/archives/html/xen-devel/2017-05/msg01053.html

Thank you for the feedback!

In this case, I think we should drop the last patch from this series.

However, your test results also confirm that the 2MB build continues to
work with Xen, which means that the reworking of the
EmuVariableFvbRuntimeDxe driver in this series, and the underlying
tweaks+cleanups series, cause no regression.

Can you please respond, with your "Regression-tested-by", to:

(1) the full series

  [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks

(2) and patches 1 through 3 in this series? (Patch #4 is just
documentation, for which Tested-by would be strange.)

Thank you!
Laszlo


>>> Note: this series depends on:
>>>
>>>   [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks
>>>   https://lists.01.org/pipermail/edk2-devel/2017-May/010527.html
>>>
>>> and it has been pushed to my github repo as such.
>>>
>>> Repo:   https://github.com/lersek/edk2.git
>>> Branch: emu4k
>>>
>>> Cc: Gary Ching-Pang Lin <glin@suse.com>
>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>>
>>> Thanks,
>>> Laszlo
>>>
>>> Laszlo Ersek (5):
>>>   OvmfPkg/EmuVariableFvbRuntimeDxe: strip trailing whitespace
>>>   OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB
>>>   OvmfPkg/PlatformPei: align EmuVariableNvStore at any page boundary
>>>   OvmfPkg/README: document 4MB flash layout
>>>   OvmfPkg: make the 4MB flash size the default (again)
>>>
>>>  OvmfPkg/OvmfPkgIa32.dsc                |   2 +-
>>>  OvmfPkg/OvmfPkgIa32X64.dsc             |   2 +-
>>>  OvmfPkg/OvmfPkgX64.dsc                 |   2 +-
>>>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h |  26 ++-
>>>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 191 +++++++++-----------
>>>  OvmfPkg/PlatformPei/Platform.c         |  20 +-
>>>  OvmfPkg/README                         |  39 +++-
>>>  7 files changed, 143 insertions(+), 139 deletions(-)
>>>
>>> -- 
>>> 2.9.3
>>>
>>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
>>



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

* Re: [PATCH 1/5] OvmfPkg/EmuVariableFvbRuntimeDxe: strip trailing whitespace
  2017-05-06 19:30 ` [PATCH 1/5] OvmfPkg/EmuVariableFvbRuntimeDxe: strip trailing whitespace Laszlo Ersek
@ 2017-05-12  9:07   ` Gary Lin
  0 siblings, 0 replies; 19+ messages in thread
From: Gary Lin @ 2017-05-12  9:07 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: edk2-devel-01, Jordan Justen

On Sat, May 06, 2017 at 09:30:19PM +0200, Laszlo Ersek wrote:
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Regression-tested-by: Gary Lin <glin@suse.com>

> ---
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h | 16 +++----
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 44 ++++++++++----------
>  2 files changed, 30 insertions(+), 30 deletions(-)
> 
> diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
> index f34fad2cdd48..4247d21d72f8 100644
> --- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
> +++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
> @@ -1,18 +1,18 @@
>  /*++
>  
>  Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> -This program and the accompanying materials                          
> -are licensed and made available under the terms and conditions of the BSD License         
> -which accompanies this distribution.  The full text of the license may be found at        
> -http://opensource.org/licenses/bsd-license.php                                            
> -                                                                                          
> -THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,                     
> -WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.             
> +This program and the accompanying materials
> +are licensed and made available under the terms and conditions of the BSD License
> +which accompanies this distribution.  The full text of the license may be found at
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  
>  Module Name:
>  
>    FwBlockService.h
> -  
> +
>  Abstract:
>  
>    Firmware volume block driver for Intel Firmware Hub (FWH) device
> diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
> index 7a6d3153ec8c..2f89632e5d75 100644
> --- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
> +++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
> @@ -120,14 +120,14 @@ FvbVirtualAddressChangeEvent (
>    only for memory-mapped firmware volumes.
>  
>    @param This     Indicates the EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL instance.
> -  
> +
>    @param Address  Pointer to a caller-allocated
>                    EFI_PHYSICAL_ADDRESS that, on successful
>                    return from GetPhysicalAddress(), contains the
>                    base address of the firmware volume.
> -  
> +
>    @retval EFI_SUCCESS       The firmware volume base address is returned.
> -  
> +
>    @retval EFI_NOT_SUPPORTED The firmware volume is not memory mapped.
>  
>  **/
> @@ -168,9 +168,9 @@ FvbProtocolGetPhysicalAddress (
>                          blocks in this range have a size of
>                          BlockSize.
>  
> -  
> +
>    @retval EFI_SUCCESS             The firmware volume base address is returned.
> -  
> +
>    @retval EFI_INVALID_PARAMETER   The requested LBA is out of range.
>  
>  **/
> @@ -246,7 +246,7 @@ FvbProtocolGetAttributes (
>                        settings of the firmware volume. Type
>                        EFI_FVB_ATTRIBUTES_2 is defined in
>                        EFI_FIRMWARE_VOLUME_HEADER.
> -  
> +
>    @retval EFI_SUCCESS           The firmware volume attributes were returned.
>  
>    @retval EFI_INVALID_PARAMETER The attributes requested are in
> @@ -302,7 +302,7 @@ FvbProtocolSetAttributes (
>  
>    @retval EFI_SUCCESS The erase request was successfully
>                        completed.
> -  
> +
>    @retval EFI_ACCESS_DENIED   The firmware volume is in the
>                                WriteDisabled state.
>    @retval EFI_DEVICE_ERROR  The block device is not functioning
> @@ -311,7 +311,7 @@ FvbProtocolSetAttributes (
>                              partially erased.
>    @retval EFI_INVALID_PARAMETER One or more of the LBAs listed
>                                  in the variable argument list do
> -                                not exist in the firmware volume.  
> +                                not exist in the firmware volume.
>  
>  **/
>  EFI_STATUS
> @@ -420,29 +420,29 @@ FvbProtocolEraseBlocks (
>    returns.
>  
>    @param This     Indicates the EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL instance.
> -  
> +
>    @param Lba      The starting logical block index to write to.
> -  
> +
>    @param Offset   Offset into the block at which to begin writing.
> -  
> +
>    @param NumBytes Pointer to a UINTN. At entry, *NumBytes
>                    contains the total size of the buffer. At
>                    exit, *NumBytes contains the total number of
>                    bytes actually written.
> -  
> +
>    @param Buffer   Pointer to a caller-allocated buffer that
>                    contains the source for the write.
> -  
> +
>    @retval EFI_SUCCESS         The firmware volume was written successfully.
> -  
> +
>    @retval EFI_BAD_BUFFER_SIZE The write was attempted across an
>                                LBA boundary. On output, NumBytes
>                                contains the total number of bytes
>                                actually written.
> -  
> +
>    @retval EFI_ACCESS_DENIED   The firmware volume is in the
>                                WriteDisabled state.
> -  
> +
>    @retval EFI_DEVICE_ERROR    The block device is malfunctioning
>                                and could not be written.
>  
> @@ -503,7 +503,7 @@ FvbProtocolWrite (
>    aware that a read may be partially completed.
>  
>    @param This     Indicates the EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL instance.
> -  
> +
>    @param Lba      The starting logical block index
>                    from which to read.
>  
> @@ -519,15 +519,15 @@ FvbProtocolWrite (
>  
>    @retval EFI_SUCCESS         The firmware volume was read successfully
>                                and contents are in Buffer.
> -  
> +
>    @retval EFI_BAD_BUFFER_SIZE Read attempted across an LBA
>                                boundary. On output, NumBytes
>                                contains the total number of bytes
>                                returned in Buffer.
> -  
> +
>    @retval EFI_ACCESS_DENIED   The firmware volume is in the
>                                ReadDisabled state.
> -  
> +
>    @retval EFI_DEVICE_ERROR    The block device is not
>                                functioning correctly and could
>                                not be read.
> @@ -715,9 +715,9 @@ InitializeFvAndVariableStoreHeaders (
>  /**
>    Main entry point.
>  
> -  @param[in] ImageHandle    The firmware allocated handle for the EFI image.  
> +  @param[in] ImageHandle    The firmware allocated handle for the EFI image.
>    @param[in] SystemTable    A pointer to the EFI System Table.
> -  
> +
>    @retval EFI_SUCCESS       Successfully initialized.
>  
>  **/
> -- 
> 2.9.3
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


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

* Re: [PATCH 2/5] OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB
  2017-05-06 19:30 ` [PATCH 2/5] OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB Laszlo Ersek
@ 2017-05-12  9:07   ` Gary Lin
  2017-05-15 23:50   ` Jordan Justen
  1 sibling, 0 replies; 19+ messages in thread
From: Gary Lin @ 2017-05-12  9:07 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: edk2-devel-01, Jordan Justen

On Sat, May 06, 2017 at 09:30:20PM +0200, Laszlo Ersek wrote:
> EmuVariableFvbRuntimeDxe currently produces a Firmware Volume Block
> protocol that is based on a block map of two blocks, each block having
> PcdFlashNvStorageFtwSpareSize for size.
> 
> (The total size is 2 * PcdFlashNvStorageFtwSpareSize.)
> 
> FaultTolerantWriteDxe in turn expects the block size to be a power of two.
> 
> In the 4MB build of OVMF, PcdFlashNvStorageFtwSpareSize is 264KB, which is
> not a power of two. In order to equip EmuVariableFvbRuntimeDxe for this
> build, shrink the block size to 4KB (EFI_PAGE_SIZE), and grow the block
> count from 2 to EFI_SIZE_TO_PAGES(2 * PcdFlashNvStorageFtwSpareSize). The
> total size remains
> 
>   2 * PcdFlashNvStorageFtwSpareSize
>   --------------------------------- * EFI_PAGE_SIZE
>             EFI_PAGE_SIZE
> 
> Right now EmuVariableFvbRuntimeDxe open-codes the block count of 2 in
> various limit checks, so introduce a few new macros:
> - EMU_FVB_NUM_TOTAL_BLOCKS, for the LHS of the above product,
> - EMU_FVB_NUM_SPARE_BLOCKS for the half of that.
> 
> Also rework the FVB protocol members to support an arbitrary count of
> blocks.
> 
> Keep the invariant intact that the first half of the firmware volume hosts
> the variable store and the FTW working block, and that the second half
> maps the FTW spare area.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=527
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Regression-tested-by: Gary Lin <glin@suse.com>

> ---
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h |  10 +-
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 147 +++++++++-----------
>  2 files changed, 75 insertions(+), 82 deletions(-)
> 
> diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
> index 4247d21d72f8..beb11e3f9a90 100644
> --- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
> +++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
> @@ -58,8 +58,14 @@ typedef struct {
>  //
>  // Constants
>  //
> -#define EMU_FVB_BLOCK_SIZE (FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize))
> -#define EMU_FVB_SIZE (2 * FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize))
> +#define EMU_FVB_BLOCK_SIZE \
> +  EFI_PAGE_SIZE
> +#define EMU_FVB_NUM_SPARE_BLOCKS \
> +  EFI_SIZE_TO_PAGES ((UINTN)FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize))
> +#define EMU_FVB_NUM_TOTAL_BLOCKS \
> +  (2 * EMU_FVB_NUM_SPARE_BLOCKS)
> +#define EMU_FVB_SIZE \
> +  (EMU_FVB_NUM_TOTAL_BLOCKS * EMU_FVB_BLOCK_SIZE)
>  #define FTW_WRITE_QUEUE_SIZE \
>    (FixedPcdGet32 (PcdFlashNvStorageFtwWorkingSize) - \
>     sizeof (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER))
> diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
> index 2f89632e5d75..11c8b1b75cb8 100644
> --- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
> +++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
> @@ -74,8 +74,8 @@ EFI_FW_VOL_BLOCK_DEVICE mEmuVarsFvb = {
>      }
>    },
>    NULL, // BufferPtr
> -  FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize), // BlockSize
> -  2 * FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize), // Size
> +  EMU_FVB_BLOCK_SIZE, // BlockSize
> +  EMU_FVB_SIZE, // Size
>    {     // FwVolBlockInstance
>      FvbProtocolGetAttributes,
>      FvbProtocolSetAttributes,
> @@ -185,14 +185,14 @@ FvbProtocolGetBlockSize (
>  {
>    EFI_FW_VOL_BLOCK_DEVICE *FvbDevice;
>  
> -  if (Lba > 1) {
> +  if (Lba >= EMU_FVB_NUM_TOTAL_BLOCKS) {
>      return EFI_INVALID_PARAMETER;
>    }
>  
>    FvbDevice = FVB_DEVICE_FROM_THIS (This);
>  
>    *BlockSize = FvbDevice->BlockSize;
> -  *NumberOfBlocks = (UINTN) (2 - (UINTN) Lba);
> +  *NumberOfBlocks = (UINTN)(EMU_FVB_NUM_TOTAL_BLOCKS - Lba);
>  
>    return EFI_SUCCESS;
>  }
> @@ -322,68 +322,58 @@ FvbProtocolEraseBlocks (
>    )
>  {
>    EFI_FW_VOL_BLOCK_DEVICE *FvbDevice;
> -  VA_LIST                 args;
> +  VA_LIST                 Args;
>    EFI_LBA                 StartingLba;
>    UINTN                   NumOfLba;
> -  UINT8                   Erase;
> -  VOID                    *ErasePtr;
> +  UINT8                   *ErasePtr;
>    UINTN                   EraseSize;
>  
>    FvbDevice = FVB_DEVICE_FROM_THIS (This);
> -  Erase = 0;
> -
> -  VA_START (args, This);
>  
> +  //
> +  // Check input parameters
> +  //
> +  VA_START (Args, This);
>    do {
> -    StartingLba = VA_ARG (args, EFI_LBA);
> +    StartingLba = VA_ARG (Args, EFI_LBA);
>      if (StartingLba == EFI_LBA_LIST_TERMINATOR) {
>        break;
>      }
> +    NumOfLba = VA_ARG (Args, UINTN);
>  
> -    NumOfLba = VA_ARG (args, UINT32);
> -
> -    //
> -    // Check input parameters
> -    //
> -    if ((NumOfLba == 0) || (StartingLba > 1) || ((StartingLba + NumOfLba) > 2)) {
> -      VA_END (args);
> +    if (StartingLba > EMU_FVB_NUM_TOTAL_BLOCKS ||
> +        NumOfLba > EMU_FVB_NUM_TOTAL_BLOCKS - StartingLba) {
> +      VA_END (Args);
>        return EFI_INVALID_PARAMETER;
>      }
> -
> -    if (StartingLba == 0) {
> -      Erase = (UINT8) (Erase | BIT0);
> -    }
> -    if ((StartingLba + NumOfLba) == 2) {
> -      Erase = (UINT8) (Erase | BIT1);
> -    }
> -
>    } while (1);
> +  VA_END (Args);
>  
> -  VA_END (args);
> -
> -  ErasePtr = (UINT8*) FvbDevice->BufferPtr;
> -  EraseSize = 0;
> +  //
> +  // Erase blocks
> +  //
> +  VA_START (Args, This);
> +  do {
> +    StartingLba = VA_ARG (Args, EFI_LBA);
> +    if (StartingLba == EFI_LBA_LIST_TERMINATOR) {
> +      break;
> +    }
> +    NumOfLba = VA_ARG (Args, UINTN);
>  
> -  if ((Erase & BIT0) != 0) {
> -    EraseSize = EraseSize + FvbDevice->BlockSize;
> -  } else {
> -    ErasePtr = (VOID*) ((UINT8*)ErasePtr + FvbDevice->BlockSize);
> -  }
> +    ErasePtr = FvbDevice->BufferPtr;
> +    ErasePtr += (UINTN)StartingLba * FvbDevice->BlockSize;
> +    EraseSize = NumOfLba * FvbDevice->BlockSize;
>  
> -  if ((Erase & BIT1) != 0) {
> -    EraseSize = EraseSize + FvbDevice->BlockSize;
> -  }
> +    SetMem (ErasePtr, EraseSize, ERASED_UINT8);
> +  } while (1);
> +  VA_END (Args);
>  
> -  if (EraseSize != 0) {
> -    SetMem (
> -      (VOID*) ErasePtr,
> -      EraseSize,
> -      ERASED_UINT8
> -      );
> -    VA_START (args, This);
> -    PlatformFvbBlocksErased (This, args);
> -    VA_END (args);
> -  }
> +  //
> +  // Call platform hook
> +  //
> +  VA_START (Args, This);
> +  PlatformFvbBlocksErased (This, Args);
> +  VA_END (Args);
>  
>    return EFI_SUCCESS;
>  }
> @@ -458,31 +448,30 @@ FvbProtocolWrite (
>    IN        UINT8                               *Buffer
>    )
>  {
> -
>    EFI_FW_VOL_BLOCK_DEVICE *FvbDevice;
>    UINT8                   *FvbDataPtr;
> +  EFI_STATUS              Status;
>  
>    FvbDevice = FVB_DEVICE_FROM_THIS (This);
>  
> -  if ((Lba > 1) || (Offset > FvbDevice->BlockSize)) {
> +  if (Lba >= EMU_FVB_NUM_TOTAL_BLOCKS ||
> +      Offset > FvbDevice->BlockSize) {
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -  if ((Offset + *NumBytes) > FvbDevice->BlockSize) {
> +  Status = EFI_SUCCESS;
> +  if (*NumBytes > FvbDevice->BlockSize - Offset) {
>      *NumBytes = FvbDevice->BlockSize - Offset;
> +    Status = EFI_BAD_BUFFER_SIZE;
>    }
>  
> -  FvbDataPtr =
> -    (UINT8*) FvbDevice->BufferPtr +
> -    MultU64x32 (Lba, (UINT32) FvbDevice->BlockSize) +
> -    Offset;
> +  FvbDataPtr = FvbDevice->BufferPtr;
> +  FvbDataPtr += (UINTN)Lba * FvbDevice->BlockSize;
> +  FvbDataPtr += Offset;
>  
> -  if (*NumBytes > 0) {
> -    CopyMem (FvbDataPtr, Buffer, *NumBytes);
> -    PlatformFvbDataWritten (This, Lba, Offset, *NumBytes, Buffer);
> -  }
> -
> -  return EFI_SUCCESS;
> +  CopyMem (FvbDataPtr, Buffer, *NumBytes);
> +  PlatformFvbDataWritten (This, Lba, Offset, *NumBytes, Buffer);
> +  return Status;
>  }
>  
>  
> @@ -545,28 +534,28 @@ FvbProtocolRead (
>  {
>    EFI_FW_VOL_BLOCK_DEVICE *FvbDevice;
>    UINT8                   *FvbDataPtr;
> +  EFI_STATUS              Status;
>  
>    FvbDevice = FVB_DEVICE_FROM_THIS (This);
>  
> -  if ((Lba > 1) || (Offset > FvbDevice->BlockSize)) {
> +  if (Lba >= EMU_FVB_NUM_TOTAL_BLOCKS ||
> +      Offset > FvbDevice->BlockSize) {
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -  if ((Offset + *NumBytes) > FvbDevice->BlockSize) {
> +  Status = EFI_SUCCESS;
> +  if (*NumBytes > FvbDevice->BlockSize - Offset) {
>      *NumBytes = FvbDevice->BlockSize - Offset;
> +    Status = EFI_BAD_BUFFER_SIZE;
>    }
>  
> -  FvbDataPtr =
> -    (UINT8*) FvbDevice->BufferPtr +
> -    MultU64x32 (Lba, (UINT32) FvbDevice->BlockSize) +
> -    Offset;
> +  FvbDataPtr = FvbDevice->BufferPtr;
> +  FvbDataPtr += (UINTN)Lba * FvbDevice->BlockSize;
> +  FvbDataPtr += Offset;
>  
> -  if (*NumBytes > 0) {
> -    CopyMem (Buffer, FvbDataPtr, *NumBytes);
> -    PlatformFvbDataRead (This, Lba, Offset, *NumBytes, Buffer);
> -  }
> -
> -  return EFI_SUCCESS;
> +  CopyMem (Buffer, FvbDataPtr, *NumBytes);
> +  PlatformFvbDataRead (This, Lba, Offset, *NumBytes, Buffer);
> +  return Status;
>  }
>  
>  
> @@ -663,7 +652,7 @@ InitializeFvAndVariableStoreHeaders (
>        // EFI_FV_BLOCK_MAP_ENTRY    BlockMap[1];
>        {
>          {
> -          2, // UINT32 NumBlocks;
> +          EMU_FVB_NUM_TOTAL_BLOCKS, // UINT32 NumBlocks;
>            EMU_FVB_BLOCK_SIZE  // UINT32 Length;
>          }
>        }
> @@ -745,7 +734,7 @@ FvbInitialize (
>         (PcdGet32 (PcdVariableStoreSize) +
>          PcdGet32 (PcdFlashNvStorageFtwWorkingSize)
>         ) >
> -       EMU_FVB_BLOCK_SIZE
> +       EMU_FVB_NUM_SPARE_BLOCKS * EMU_FVB_BLOCK_SIZE
>       ) {
>      DEBUG ((EFI_D_ERROR, "EMU Variable invalid PCD sizes\n"));
>      return EFI_INVALID_PARAMETER;
> @@ -779,10 +768,7 @@ FvbInitialize (
>        Initialize = FALSE;
>      }
>    } else {
> -    Ptr = AllocateAlignedRuntimePages (
> -            EFI_SIZE_TO_PAGES (EMU_FVB_SIZE),
> -            SIZE_64KB
> -            );
> +    Ptr = AllocateRuntimePages (EFI_SIZE_TO_PAGES (EMU_FVB_SIZE));
>    }
>  
>    mEmuVarsFvb.BufferPtr = Ptr;
> @@ -808,7 +794,8 @@ FvbInitialize (
>    //
>    // Initialize the Fault Tolerant Write spare block
>    //
> -  SubPtr = (VOID*) ((UINT8*) Ptr + EMU_FVB_BLOCK_SIZE);
> +  SubPtr = (VOID*) ((UINT8*) Ptr +
> +                    EMU_FVB_NUM_SPARE_BLOCKS * EMU_FVB_BLOCK_SIZE);
>    PcdStatus = PcdSet32S (PcdFlashNvStorageFtwSpareBase,
>                  (UINT32)(UINTN) SubPtr);
>    ASSERT_RETURN_ERROR (PcdStatus);
> -- 
> 2.9.3
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 


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

* Re: [PATCH 3/5] OvmfPkg/PlatformPei: align EmuVariableNvStore at any page boundary
  2017-05-06 19:30 ` [PATCH 3/5] OvmfPkg/PlatformPei: align EmuVariableNvStore at any page boundary Laszlo Ersek
@ 2017-05-12  9:08   ` Gary Lin
  0 siblings, 0 replies; 19+ messages in thread
From: Gary Lin @ 2017-05-12  9:08 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: edk2-devel-01, Jordan Justen

On Sat, May 06, 2017 at 09:30:21PM +0200, Laszlo Ersek wrote:
> EmuVariableFvbRuntimeDxe now uses a 4KB (EFI_PAGE_SIZE) block size.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Regression-tested-by: Gary Lin <glin@suse.com>

> ---
>  OvmfPkg/PlatformPei/Platform.c | 20 ++++----------------
>  1 file changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
> index 1b4dc00b0180..3e9fda7c7ab0 100644
> --- a/OvmfPkg/PlatformPei/Platform.c
> +++ b/OvmfPkg/PlatformPei/Platform.c
> @@ -504,7 +504,6 @@ ReserveEmuVariableNvStore (
>  {
>    EFI_PHYSICAL_ADDRESS VariableStore;
>    RETURN_STATUS        PcdStatus;
> -  UINT32               Alignment;
>  
>    //
>    // Allocate storage for NV variables early on so it will be
> @@ -512,26 +511,15 @@ ReserveEmuVariableNvStore (
>    // across reboots, this allows the NV variable storage to survive
>    // a VM reboot.
>    //
> -  Alignment = PcdGet32 (PcdFlashNvStorageFtwSpareSize);
> -  if ((Alignment & (Alignment - 1)) != 0) {
> -    //
> -    // Round up Alignment to the next power of two.
> -    //
> -    Alignment = GetPowerOfTwo32 (Alignment) << 1;
> -  }
> -
>    VariableStore =
>      (EFI_PHYSICAL_ADDRESS)(UINTN)
> -      AllocateAlignedRuntimePages (
> -        EFI_SIZE_TO_PAGES (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)),
> -        Alignment
> +      AllocateRuntimePages (
> +        EFI_SIZE_TO_PAGES (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize))
>          );
>    DEBUG ((EFI_D_INFO,
> -          "Reserved variable store memory: 0x%lX; size: %dkb, "
> -          "alignment: 0x%x\n",
> +          "Reserved variable store memory: 0x%lX; size: %dkb\n",
>            VariableStore,
> -          (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)) / 1024,
> -          Alignment
> +          (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)) / 1024
>          ));
>    PcdStatus = PcdSet64S (PcdEmuVariableNvStoreReserved, VariableStore);
>    ASSERT_RETURN_ERROR (PcdStatus);
> -- 
> 2.9.3
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 


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

* Re: [PATCH 2/5] OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB
  2017-05-06 19:30 ` [PATCH 2/5] OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB Laszlo Ersek
  2017-05-12  9:07   ` Gary Lin
@ 2017-05-15 23:50   ` Jordan Justen
  1 sibling, 0 replies; 19+ messages in thread
From: Jordan Justen @ 2017-05-15 23:50 UTC (permalink / raw)
  To: Laszlo Ersek, edk2-devel-01

On 2017-05-06 12:30:20, Laszlo Ersek wrote:
> EmuVariableFvbRuntimeDxe currently produces a Firmware Volume Block
> protocol that is based on a block map of two blocks, each block having
> PcdFlashNvStorageFtwSpareSize for size.
> 
> (The total size is 2 * PcdFlashNvStorageFtwSpareSize.)
> 
> FaultTolerantWriteDxe in turn expects the block size to be a power of two.
> 
> In the 4MB build of OVMF, PcdFlashNvStorageFtwSpareSize is 264KB, which is
> not a power of two. In order to equip EmuVariableFvbRuntimeDxe for this
> build, shrink the block size to 4KB (EFI_PAGE_SIZE), and grow the block
> count from 2 to EFI_SIZE_TO_PAGES(2 * PcdFlashNvStorageFtwSpareSize). The
> total size remains
> 
>   2 * PcdFlashNvStorageFtwSpareSize
>   --------------------------------- * EFI_PAGE_SIZE
>             EFI_PAGE_SIZE
> 
> Right now EmuVariableFvbRuntimeDxe open-codes the block count of 2 in
> various limit checks, so introduce a few new macros:
> - EMU_FVB_NUM_TOTAL_BLOCKS, for the LHS of the above product,
> - EMU_FVB_NUM_SPARE_BLOCKS for the half of that.
> 
> Also rework the FVB protocol members to support an arbitrary count of
> blocks.
> 
> Keep the invariant intact that the first half of the firmware volume hosts
> the variable store and the FTW working block, and that the second half
> maps the FTW spare area.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=527
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h |  10 +-
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 147 +++++++++-----------
>  2 files changed, 75 insertions(+), 82 deletions(-)
> 
> diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
> index 4247d21d72f8..beb11e3f9a90 100644
> --- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
> +++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
> @@ -58,8 +58,14 @@ typedef struct {
>  //
>  // Constants
>  //
> -#define EMU_FVB_BLOCK_SIZE (FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize))
> -#define EMU_FVB_SIZE (2 * FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize))
> +#define EMU_FVB_BLOCK_SIZE \
> +  EFI_PAGE_SIZE
> +#define EMU_FVB_NUM_SPARE_BLOCKS \
> +  EFI_SIZE_TO_PAGES ((UINTN)FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize))
> +#define EMU_FVB_NUM_TOTAL_BLOCKS \
> +  (2 * EMU_FVB_NUM_SPARE_BLOCKS)
> +#define EMU_FVB_SIZE \
> +  (EMU_FVB_NUM_TOTAL_BLOCKS * EMU_FVB_BLOCK_SIZE)
>  #define FTW_WRITE_QUEUE_SIZE \
>    (FixedPcdGet32 (PcdFlashNvStorageFtwWorkingSize) - \
>     sizeof (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER))
> diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
> index 2f89632e5d75..11c8b1b75cb8 100644
> --- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
> +++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
> @@ -74,8 +74,8 @@ EFI_FW_VOL_BLOCK_DEVICE mEmuVarsFvb = {
>      }
>    },
>    NULL, // BufferPtr
> -  FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize), // BlockSize
> -  2 * FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize), // Size
> +  EMU_FVB_BLOCK_SIZE, // BlockSize
> +  EMU_FVB_SIZE, // Size
>    {     // FwVolBlockInstance
>      FvbProtocolGetAttributes,
>      FvbProtocolSetAttributes,
> @@ -185,14 +185,14 @@ FvbProtocolGetBlockSize (
>  {
>    EFI_FW_VOL_BLOCK_DEVICE *FvbDevice;
>  
> -  if (Lba > 1) {
> +  if (Lba >= EMU_FVB_NUM_TOTAL_BLOCKS) {
>      return EFI_INVALID_PARAMETER;
>    }
>  
>    FvbDevice = FVB_DEVICE_FROM_THIS (This);
>  
>    *BlockSize = FvbDevice->BlockSize;
> -  *NumberOfBlocks = (UINTN) (2 - (UINTN) Lba);
> +  *NumberOfBlocks = (UINTN)(EMU_FVB_NUM_TOTAL_BLOCKS - Lba);
>  
>    return EFI_SUCCESS;
>  }
> @@ -322,68 +322,58 @@ FvbProtocolEraseBlocks (
>    )
>  {
>    EFI_FW_VOL_BLOCK_DEVICE *FvbDevice;
> -  VA_LIST                 args;
> +  VA_LIST                 Args;
>    EFI_LBA                 StartingLba;
>    UINTN                   NumOfLba;
> -  UINT8                   Erase;
> -  VOID                    *ErasePtr;
> +  UINT8                   *ErasePtr;
>    UINTN                   EraseSize;
>  
>    FvbDevice = FVB_DEVICE_FROM_THIS (This);
> -  Erase = 0;
> -
> -  VA_START (args, This);
>  
> +  //
> +  // Check input parameters
> +  //
> +  VA_START (Args, This);
>    do {
> -    StartingLba = VA_ARG (args, EFI_LBA);
> +    StartingLba = VA_ARG (Args, EFI_LBA);
>      if (StartingLba == EFI_LBA_LIST_TERMINATOR) {
>        break;
>      }
> +    NumOfLba = VA_ARG (Args, UINTN);
>  
> -    NumOfLba = VA_ARG (args, UINT32);

This bug seems mildly concerning. I guess real users are not going to
specify an lba count > 4G. Still I think you should break this fix
out, and perhaps notify other package owners:

$ git grep -e NumOfLba --and -e VA_ARG --and -e UINT32
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c:    NumOfLba = VA_ARG (Args, UINT32);
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c:    NumOfLba = VA_ARG (Args, UINT32);
DuetPkg/FvbRuntimeService/FWBlockService.c:    NumOfLba = VA_ARG (args, UINT32);
DuetPkg/FvbRuntimeService/FWBlockService.c:    NumOfLba = VA_ARG (args, UINT32);
EmulatorPkg/FvbServicesRuntimeDxe/FWBlockService.c:    NumOfLba = VA_ARG (args, UINT32);
EmulatorPkg/FvbServicesRuntimeDxe/FWBlockService.c:    NumOfLba = VA_ARG (args, UINT32);
Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c:    NumOfLba = VA_ARG (args, UINT32);
Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c:    NumOfLba = VA_ARG (args, UINT32);
OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c:    NumOfLba = VA_ARG (args, UINT32);
OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c:    NumOfLba = VA_ARG (args, UINT32);
OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c:    NumOfLba = VA_ARG (args, UINT32);
QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c:    NumOfLba = VA_ARG (args, UINT32);
QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c:    NumOfLba = VA_ARG (args, UINT32);
Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c:    NumOfLba = VA_ARG (args, UINT32);
Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c:    NumOfLba = VA_ARG (args, UINT32);
Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c:    NumOfLba = VA_ARG (Marker, UINT32);
Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c:    NumOfLba = VA_ARG (Marker, UINT32);

-Jordan

> -
> -    //
> -    // Check input parameters
> -    //
> -    if ((NumOfLba == 0) || (StartingLba > 1) || ((StartingLba + NumOfLba) > 2)) {
> -      VA_END (args);
> +    if (StartingLba > EMU_FVB_NUM_TOTAL_BLOCKS ||
> +        NumOfLba > EMU_FVB_NUM_TOTAL_BLOCKS - StartingLba) {
> +      VA_END (Args);
>        return EFI_INVALID_PARAMETER;
>      }
> -
> -    if (StartingLba == 0) {
> -      Erase = (UINT8) (Erase | BIT0);
> -    }
> -    if ((StartingLba + NumOfLba) == 2) {
> -      Erase = (UINT8) (Erase | BIT1);
> -    }
> -
>    } while (1);
> +  VA_END (Args);
>  
> -  VA_END (args);
> -
> -  ErasePtr = (UINT8*) FvbDevice->BufferPtr;
> -  EraseSize = 0;
> +  //
> +  // Erase blocks
> +  //
> +  VA_START (Args, This);
> +  do {
> +    StartingLba = VA_ARG (Args, EFI_LBA);
> +    if (StartingLba == EFI_LBA_LIST_TERMINATOR) {
> +      break;
> +    }
> +    NumOfLba = VA_ARG (Args, UINTN);
>  
> -  if ((Erase & BIT0) != 0) {
> -    EraseSize = EraseSize + FvbDevice->BlockSize;
> -  } else {
> -    ErasePtr = (VOID*) ((UINT8*)ErasePtr + FvbDevice->BlockSize);
> -  }
> +    ErasePtr = FvbDevice->BufferPtr;
> +    ErasePtr += (UINTN)StartingLba * FvbDevice->BlockSize;
> +    EraseSize = NumOfLba * FvbDevice->BlockSize;
>  
> -  if ((Erase & BIT1) != 0) {
> -    EraseSize = EraseSize + FvbDevice->BlockSize;
> -  }
> +    SetMem (ErasePtr, EraseSize, ERASED_UINT8);
> +  } while (1);
> +  VA_END (Args);
>  
> -  if (EraseSize != 0) {
> -    SetMem (
> -      (VOID*) ErasePtr,
> -      EraseSize,
> -      ERASED_UINT8
> -      );
> -    VA_START (args, This);
> -    PlatformFvbBlocksErased (This, args);
> -    VA_END (args);
> -  }
> +  //
> +  // Call platform hook
> +  //
> +  VA_START (Args, This);
> +  PlatformFvbBlocksErased (This, Args);
> +  VA_END (Args);
>  
>    return EFI_SUCCESS;
>  }
> @@ -458,31 +448,30 @@ FvbProtocolWrite (
>    IN        UINT8                               *Buffer
>    )
>  {
> -
>    EFI_FW_VOL_BLOCK_DEVICE *FvbDevice;
>    UINT8                   *FvbDataPtr;
> +  EFI_STATUS              Status;
>  
>    FvbDevice = FVB_DEVICE_FROM_THIS (This);
>  
> -  if ((Lba > 1) || (Offset > FvbDevice->BlockSize)) {
> +  if (Lba >= EMU_FVB_NUM_TOTAL_BLOCKS ||
> +      Offset > FvbDevice->BlockSize) {
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -  if ((Offset + *NumBytes) > FvbDevice->BlockSize) {
> +  Status = EFI_SUCCESS;
> +  if (*NumBytes > FvbDevice->BlockSize - Offset) {
>      *NumBytes = FvbDevice->BlockSize - Offset;
> +    Status = EFI_BAD_BUFFER_SIZE;
>    }
>  
> -  FvbDataPtr =
> -    (UINT8*) FvbDevice->BufferPtr +
> -    MultU64x32 (Lba, (UINT32) FvbDevice->BlockSize) +
> -    Offset;
> +  FvbDataPtr = FvbDevice->BufferPtr;
> +  FvbDataPtr += (UINTN)Lba * FvbDevice->BlockSize;
> +  FvbDataPtr += Offset;
>  
> -  if (*NumBytes > 0) {
> -    CopyMem (FvbDataPtr, Buffer, *NumBytes);
> -    PlatformFvbDataWritten (This, Lba, Offset, *NumBytes, Buffer);
> -  }
> -
> -  return EFI_SUCCESS;
> +  CopyMem (FvbDataPtr, Buffer, *NumBytes);
> +  PlatformFvbDataWritten (This, Lba, Offset, *NumBytes, Buffer);
> +  return Status;
>  }
>  
>  
> @@ -545,28 +534,28 @@ FvbProtocolRead (
>  {
>    EFI_FW_VOL_BLOCK_DEVICE *FvbDevice;
>    UINT8                   *FvbDataPtr;
> +  EFI_STATUS              Status;
>  
>    FvbDevice = FVB_DEVICE_FROM_THIS (This);
>  
> -  if ((Lba > 1) || (Offset > FvbDevice->BlockSize)) {
> +  if (Lba >= EMU_FVB_NUM_TOTAL_BLOCKS ||
> +      Offset > FvbDevice->BlockSize) {
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -  if ((Offset + *NumBytes) > FvbDevice->BlockSize) {
> +  Status = EFI_SUCCESS;
> +  if (*NumBytes > FvbDevice->BlockSize - Offset) {
>      *NumBytes = FvbDevice->BlockSize - Offset;
> +    Status = EFI_BAD_BUFFER_SIZE;
>    }
>  
> -  FvbDataPtr =
> -    (UINT8*) FvbDevice->BufferPtr +
> -    MultU64x32 (Lba, (UINT32) FvbDevice->BlockSize) +
> -    Offset;
> +  FvbDataPtr = FvbDevice->BufferPtr;
> +  FvbDataPtr += (UINTN)Lba * FvbDevice->BlockSize;
> +  FvbDataPtr += Offset;
>  
> -  if (*NumBytes > 0) {
> -    CopyMem (Buffer, FvbDataPtr, *NumBytes);
> -    PlatformFvbDataRead (This, Lba, Offset, *NumBytes, Buffer);
> -  }
> -
> -  return EFI_SUCCESS;
> +  CopyMem (Buffer, FvbDataPtr, *NumBytes);
> +  PlatformFvbDataRead (This, Lba, Offset, *NumBytes, Buffer);
> +  return Status;
>  }
>  
>  
> @@ -663,7 +652,7 @@ InitializeFvAndVariableStoreHeaders (
>        // EFI_FV_BLOCK_MAP_ENTRY    BlockMap[1];
>        {
>          {
> -          2, // UINT32 NumBlocks;
> +          EMU_FVB_NUM_TOTAL_BLOCKS, // UINT32 NumBlocks;
>            EMU_FVB_BLOCK_SIZE  // UINT32 Length;
>          }
>        }
> @@ -745,7 +734,7 @@ FvbInitialize (
>         (PcdGet32 (PcdVariableStoreSize) +
>          PcdGet32 (PcdFlashNvStorageFtwWorkingSize)
>         ) >
> -       EMU_FVB_BLOCK_SIZE
> +       EMU_FVB_NUM_SPARE_BLOCKS * EMU_FVB_BLOCK_SIZE
>       ) {
>      DEBUG ((EFI_D_ERROR, "EMU Variable invalid PCD sizes\n"));
>      return EFI_INVALID_PARAMETER;
> @@ -779,10 +768,7 @@ FvbInitialize (
>        Initialize = FALSE;
>      }
>    } else {
> -    Ptr = AllocateAlignedRuntimePages (
> -            EFI_SIZE_TO_PAGES (EMU_FVB_SIZE),
> -            SIZE_64KB
> -            );
> +    Ptr = AllocateRuntimePages (EFI_SIZE_TO_PAGES (EMU_FVB_SIZE));
>    }
>  
>    mEmuVarsFvb.BufferPtr = Ptr;
> @@ -808,7 +794,8 @@ FvbInitialize (
>    //
>    // Initialize the Fault Tolerant Write spare block
>    //
> -  SubPtr = (VOID*) ((UINT8*) Ptr + EMU_FVB_BLOCK_SIZE);
> +  SubPtr = (VOID*) ((UINT8*) Ptr +
> +                    EMU_FVB_NUM_SPARE_BLOCKS * EMU_FVB_BLOCK_SIZE);
>    PcdStatus = PcdSet32S (PcdFlashNvStorageFtwSpareBase,
>                  (UINT32)(UINTN) SubPtr);
>    ASSERT_RETURN_ERROR (PcdStatus);
> -- 
> 2.9.3
> 
> 


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

* Re: [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables)
  2017-05-12  8:40     ` Laszlo Ersek
@ 2017-05-16  0:40       ` Jordan Justen
  2017-05-16  4:20         ` Gary Lin
  0 siblings, 1 reply; 19+ messages in thread
From: Jordan Justen @ 2017-05-16  0:40 UTC (permalink / raw)
  To: Gary Lin, Laszlo Ersek; +Cc: edk2-devel-01

On 2017-05-12 01:40:34, Laszlo Ersek wrote:
> On 05/12/17 04:02, Gary Lin wrote:
> > On Mon, May 08, 2017 at 12:27:59PM +0800, Gary Lin wrote:
> >> On Sat, May 06, 2017 at 09:30:18PM +0200, Laszlo Ersek wrote:
> >>> (All hail Saturday!)
> >>>
> >>> Gary, can you please fetch this from my repo (URL & branch name below)
> >>> and test it with Xen? Please test both the 4MB and the 2MB build. (I
> >>> also tested both, with qemu + "-bios".)
> >> Hi Laszlo,
> >>
> >> I have done some simples test with xen, and the 2MB build seems fine.
> >> It booted into grub2 menu successfully. However, the 4MB build never boots.
> >> The QEMU window showed less than 1 sec and then disappeared.
> >>
> >> Here is the snippet from 'xl dmesg'
> >>
> >> (d15)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... done.
> >> (d15) Testing HVM environment:
> >> (d15)  - REP INSB across page boundaries ... passed
> >> (d15)  - GS base MSRs and SWAPGS ... passed
> >> (d15) Passed 2 of 2 tests
> >> (d15) Writing SMBIOS tables ...
> >> (d15) Loading OVMF ...
> >> (d15) no BIOS ROM image found
> >> (d15) *** HVMLoader bug at hvmloader.c:381
> >> (d15) *** HVMLoader crashed.
> >>
> >> I'm pretty sure that the ovmf path is right, so it seems Xen just rejected
> >> the 4MB build :-\
> >>
> >> I'll try to dig more information.
> >>
> > There is a function in the xen hvmloader clearing the memory from
> > 0x400000 to 0x800000. Unfortunately, the hvm_start_info struct of the
> > 4MB OVMF was loaded to 0x588000, so the struct was cleared mistakenly
> > and hvmloader cannot find the firmware. Xen is not ready for the 4MB
> > build yet :-\
> > 
> > The discussion in xen-devel:
> > https://lists.xen.org/archives/html/xen-devel/2017-05/msg01053.html
> 
> Thank you for the feedback!
> 
> In this case, I think we should drop the last patch from this series.

Can we come up with a plan for trying to fix this? Gary, would it be
okay if we opened a bug and assigned it to you? Or, do you have
another suggestion for a possible Xen owner?

Thanks,

-Jordan

> 
> However, your test results also confirm that the 2MB build continues to
> work with Xen, which means that the reworking of the
> EmuVariableFvbRuntimeDxe driver in this series, and the underlying
> tweaks+cleanups series, cause no regression.
> 
> Can you please respond, with your "Regression-tested-by", to:
> 
> (1) the full series
> 
>   [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks
> 
> (2) and patches 1 through 3 in this series? (Patch #4 is just
> documentation, for which Tested-by would be strange.)
> 
> Thank you!
> Laszlo
> 
> 
> >>> Note: this series depends on:
> >>>
> >>>   [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks
> >>>   https://lists.01.org/pipermail/edk2-devel/2017-May/010527.html
> >>>
> >>> and it has been pushed to my github repo as such.
> >>>
> >>> Repo:   https://github.com/lersek/edk2.git
> >>> Branch: emu4k
> >>>
> >>> Cc: Gary Ching-Pang Lin <glin@suse.com>
> >>> Cc: Jordan Justen <jordan.l.justen@intel.com>
> >>>
> >>> Thanks,
> >>> Laszlo
> >>>
> >>> Laszlo Ersek (5):
> >>>   OvmfPkg/EmuVariableFvbRuntimeDxe: strip trailing whitespace
> >>>   OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB
> >>>   OvmfPkg/PlatformPei: align EmuVariableNvStore at any page boundary
> >>>   OvmfPkg/README: document 4MB flash layout
> >>>   OvmfPkg: make the 4MB flash size the default (again)
> >>>
> >>>  OvmfPkg/OvmfPkgIa32.dsc                |   2 +-
> >>>  OvmfPkg/OvmfPkgIa32X64.dsc             |   2 +-
> >>>  OvmfPkg/OvmfPkgX64.dsc                 |   2 +-
> >>>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h |  26 ++-
> >>>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 191 +++++++++-----------
> >>>  OvmfPkg/PlatformPei/Platform.c         |  20 +-
> >>>  OvmfPkg/README                         |  39 +++-
> >>>  7 files changed, 143 insertions(+), 139 deletions(-)
> >>>
> >>> -- 
> >>> 2.9.3
> >>>
> >>>
> >> _______________________________________________
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel
> >>
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


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

* Re: [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables)
  2017-05-16  0:40       ` Jordan Justen
@ 2017-05-16  4:20         ` Gary Lin
  2017-05-18 12:36           ` Laszlo Ersek
  0 siblings, 1 reply; 19+ messages in thread
From: Gary Lin @ 2017-05-16  4:20 UTC (permalink / raw)
  To: Jordan Justen; +Cc: Laszlo Ersek, edk2-devel-01

On Mon, May 15, 2017 at 05:40:59PM -0700, Jordan Justen wrote:
> On 2017-05-12 01:40:34, Laszlo Ersek wrote:
> > On 05/12/17 04:02, Gary Lin wrote:
> > > On Mon, May 08, 2017 at 12:27:59PM +0800, Gary Lin wrote:
> > >> On Sat, May 06, 2017 at 09:30:18PM +0200, Laszlo Ersek wrote:
> > >>> (All hail Saturday!)
> > >>>
> > >>> Gary, can you please fetch this from my repo (URL & branch name below)
> > >>> and test it with Xen? Please test both the 4MB and the 2MB build. (I
> > >>> also tested both, with qemu + "-bios".)
> > >> Hi Laszlo,
> > >>
> > >> I have done some simples test with xen, and the 2MB build seems fine.
> > >> It booted into grub2 menu successfully. However, the 4MB build never boots.
> > >> The QEMU window showed less than 1 sec and then disappeared.
> > >>
> > >> Here is the snippet from 'xl dmesg'
> > >>
> > >> (d15)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... done.
> > >> (d15) Testing HVM environment:
> > >> (d15)  - REP INSB across page boundaries ... passed
> > >> (d15)  - GS base MSRs and SWAPGS ... passed
> > >> (d15) Passed 2 of 2 tests
> > >> (d15) Writing SMBIOS tables ...
> > >> (d15) Loading OVMF ...
> > >> (d15) no BIOS ROM image found
> > >> (d15) *** HVMLoader bug at hvmloader.c:381
> > >> (d15) *** HVMLoader crashed.
> > >>
> > >> I'm pretty sure that the ovmf path is right, so it seems Xen just rejected
> > >> the 4MB build :-\
> > >>
> > >> I'll try to dig more information.
> > >>
> > > There is a function in the xen hvmloader clearing the memory from
> > > 0x400000 to 0x800000. Unfortunately, the hvm_start_info struct of the
> > > 4MB OVMF was loaded to 0x588000, so the struct was cleared mistakenly
> > > and hvmloader cannot find the firmware. Xen is not ready for the 4MB
> > > build yet :-\
> > > 
> > > The discussion in xen-devel:
> > > https://lists.xen.org/archives/html/xen-devel/2017-05/msg01053.html
> > 
> > Thank you for the feedback!
> > 
> > In this case, I think we should drop the last patch from this series.
> 
> Can we come up with a plan for trying to fix this? Gary, would it be
> okay if we opened a bug and assigned it to you? Or, do you have
> another suggestion for a possible Xen owner?
> 
Jan Beulich (also a SUSE employee) is working on the patch(*), and it
works for me.

Cheers,

Gary Lin

(*) https://lists.xen.org/archives/html/xen-devel/2017-05/msg01242.html
> Thanks,
> 
> -Jordan
> 
> > 
> > However, your test results also confirm that the 2MB build continues to
> > work with Xen, which means that the reworking of the
> > EmuVariableFvbRuntimeDxe driver in this series, and the underlying
> > tweaks+cleanups series, cause no regression.
> > 
> > Can you please respond, with your "Regression-tested-by", to:
> > 
> > (1) the full series
> > 
> >   [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks
> > 
> > (2) and patches 1 through 3 in this series? (Patch #4 is just
> > documentation, for which Tested-by would be strange.)
> > 
> > Thank you!
> > Laszlo
> > 
> > 
> > >>> Note: this series depends on:
> > >>>
> > >>>   [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks
> > >>>   https://lists.01.org/pipermail/edk2-devel/2017-May/010527.html
> > >>>
> > >>> and it has been pushed to my github repo as such.
> > >>>
> > >>> Repo:   https://github.com/lersek/edk2.git
> > >>> Branch: emu4k
> > >>>
> > >>> Cc: Gary Ching-Pang Lin <glin@suse.com>
> > >>> Cc: Jordan Justen <jordan.l.justen@intel.com>
> > >>>
> > >>> Thanks,
> > >>> Laszlo
> > >>>
> > >>> Laszlo Ersek (5):
> > >>>   OvmfPkg/EmuVariableFvbRuntimeDxe: strip trailing whitespace
> > >>>   OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB
> > >>>   OvmfPkg/PlatformPei: align EmuVariableNvStore at any page boundary
> > >>>   OvmfPkg/README: document 4MB flash layout
> > >>>   OvmfPkg: make the 4MB flash size the default (again)
> > >>>
> > >>>  OvmfPkg/OvmfPkgIa32.dsc                |   2 +-
> > >>>  OvmfPkg/OvmfPkgIa32X64.dsc             |   2 +-
> > >>>  OvmfPkg/OvmfPkgX64.dsc                 |   2 +-
> > >>>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h |  26 ++-
> > >>>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 191 +++++++++-----------
> > >>>  OvmfPkg/PlatformPei/Platform.c         |  20 +-
> > >>>  OvmfPkg/README                         |  39 +++-
> > >>>  7 files changed, 143 insertions(+), 139 deletions(-)
> > >>>
> > >>> -- 
> > >>> 2.9.3
> > >>>
> > >>>
> > >> _______________________________________________
> > >> edk2-devel mailing list
> > >> edk2-devel@lists.01.org
> > >> https://lists.01.org/mailman/listinfo/edk2-devel
> > >>
> > 
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> 


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

* Re: [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables)
  2017-05-16  4:20         ` Gary Lin
@ 2017-05-18 12:36           ` Laszlo Ersek
  2017-05-23 14:12             ` Is: Fix for 4MB BIOS payload in hvmloader. Was:Re: " Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 19+ messages in thread
From: Laszlo Ersek @ 2017-05-18 12:36 UTC (permalink / raw)
  To: Gary Lin, Jordan Justen; +Cc: edk2-devel-01

On 05/16/17 06:20, Gary Lin wrote:
> On Mon, May 15, 2017 at 05:40:59PM -0700, Jordan Justen wrote:
>> On 2017-05-12 01:40:34, Laszlo Ersek wrote:
>>> On 05/12/17 04:02, Gary Lin wrote:
>>>> On Mon, May 08, 2017 at 12:27:59PM +0800, Gary Lin wrote:
>>>>> On Sat, May 06, 2017 at 09:30:18PM +0200, Laszlo Ersek wrote:
>>>>>> (All hail Saturday!)
>>>>>>
>>>>>> Gary, can you please fetch this from my repo (URL & branch name below)
>>>>>> and test it with Xen? Please test both the 4MB and the 2MB build. (I
>>>>>> also tested both, with qemu + "-bios".)
>>>>> Hi Laszlo,
>>>>>
>>>>> I have done some simples test with xen, and the 2MB build seems fine.
>>>>> It booted into grub2 menu successfully. However, the 4MB build never boots.
>>>>> The QEMU window showed less than 1 sec and then disappeared.
>>>>>
>>>>> Here is the snippet from 'xl dmesg'
>>>>>
>>>>> (d15)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... done.
>>>>> (d15) Testing HVM environment:
>>>>> (d15)  - REP INSB across page boundaries ... passed
>>>>> (d15)  - GS base MSRs and SWAPGS ... passed
>>>>> (d15) Passed 2 of 2 tests
>>>>> (d15) Writing SMBIOS tables ...
>>>>> (d15) Loading OVMF ...
>>>>> (d15) no BIOS ROM image found
>>>>> (d15) *** HVMLoader bug at hvmloader.c:381
>>>>> (d15) *** HVMLoader crashed.
>>>>>
>>>>> I'm pretty sure that the ovmf path is right, so it seems Xen just rejected
>>>>> the 4MB build :-\
>>>>>
>>>>> I'll try to dig more information.
>>>>>
>>>> There is a function in the xen hvmloader clearing the memory from
>>>> 0x400000 to 0x800000. Unfortunately, the hvm_start_info struct of the
>>>> 4MB OVMF was loaded to 0x588000, so the struct was cleared mistakenly
>>>> and hvmloader cannot find the firmware. Xen is not ready for the 4MB
>>>> build yet :-\
>>>>
>>>> The discussion in xen-devel:
>>>> https://lists.xen.org/archives/html/xen-devel/2017-05/msg01053.html
>>>
>>> Thank you for the feedback!
>>>
>>> In this case, I think we should drop the last patch from this series.
>>
>> Can we come up with a plan for trying to fix this? Gary, would it be
>> okay if we opened a bug and assigned it to you? Or, do you have
>> another suggestion for a possible Xen owner?
>>
> Jan Beulich (also a SUSE employee) is working on the patch(*), and it
> works for me.

We should distinguish a TianoCore BZ entry for this, from a Xen bug
report for this. The former would depend on the latter.

The TianoCore BZ assignee's job would be to monitor the upstream Xen
fix, and to submit the last patch of this series -- separated out -- to
edk2-devel once upstream Xen commits the fix.

Upstream Xen does not have a bug tracker that is widely used in their
community:

https://wiki.xen.org/wiki/Reporting_Bugs_against_Xen_Project

> The primary location for reporting bugs against the hypervisor and
> associated bundled tools [...] is by posting to the xen-devel mailing
> list (list info). Please tag your subject line with a '[BUG]' prefix.
> Note that you do not need to be subscribed to the list to post
> (although non-subscribers are moderated this usually happens pretty
> quickly) and that list policy is to CC people so you shouldn't miss
> any replies.
>
> [...]
>
> Although a bugzilla instance does exist it is not well maintained or
> widely used by developers. If you really want to file a bug in
> bugzilla you are strongly recommended to also post to the mailing
> list.

This does not make things easier for us, because rather than recurrently
check a simple bug status field in the Xen tracker, we'd have to be
tapped into Xen development, and follow the email thread & any relevant
commits closely. In reality I'm not even subscribed to xen-devel.

The situation is further hampered by the fact that Xen is (apparently)
right at 4.9.0-rc5, so they likely won't commit Jan's hvmloader patch
until Xen 4.9 is out. This is a problem for a potential TianoCore-side
BZ because the delay will make us forget about the issue.

Thanks,
Laszlo

> 
> Cheers,
> 
> Gary Lin
> 
> (*) https://lists.xen.org/archives/html/xen-devel/2017-05/msg01242.html
>> Thanks,
>>
>> -Jordan
>>
>>>
>>> However, your test results also confirm that the 2MB build continues to
>>> work with Xen, which means that the reworking of the
>>> EmuVariableFvbRuntimeDxe driver in this series, and the underlying
>>> tweaks+cleanups series, cause no regression.
>>>
>>> Can you please respond, with your "Regression-tested-by", to:
>>>
>>> (1) the full series
>>>
>>>   [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks
>>>
>>> (2) and patches 1 through 3 in this series? (Patch #4 is just
>>> documentation, for which Tested-by would be strange.)
>>>
>>> Thank you!
>>> Laszlo
>>>
>>>
>>>>>> Note: this series depends on:
>>>>>>
>>>>>>   [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks
>>>>>>   https://lists.01.org/pipermail/edk2-devel/2017-May/010527.html
>>>>>>
>>>>>> and it has been pushed to my github repo as such.
>>>>>>
>>>>>> Repo:   https://github.com/lersek/edk2.git
>>>>>> Branch: emu4k
>>>>>>
>>>>>> Cc: Gary Ching-Pang Lin <glin@suse.com>
>>>>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>>>>>
>>>>>> Thanks,
>>>>>> Laszlo
>>>>>>
>>>>>> Laszlo Ersek (5):
>>>>>>   OvmfPkg/EmuVariableFvbRuntimeDxe: strip trailing whitespace
>>>>>>   OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB
>>>>>>   OvmfPkg/PlatformPei: align EmuVariableNvStore at any page boundary
>>>>>>   OvmfPkg/README: document 4MB flash layout
>>>>>>   OvmfPkg: make the 4MB flash size the default (again)
>>>>>>
>>>>>>  OvmfPkg/OvmfPkgIa32.dsc                |   2 +-
>>>>>>  OvmfPkg/OvmfPkgIa32X64.dsc             |   2 +-
>>>>>>  OvmfPkg/OvmfPkgX64.dsc                 |   2 +-
>>>>>>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h |  26 ++-
>>>>>>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 191 +++++++++-----------
>>>>>>  OvmfPkg/PlatformPei/Platform.c         |  20 +-
>>>>>>  OvmfPkg/README                         |  39 +++-
>>>>>>  7 files changed, 143 insertions(+), 139 deletions(-)
>>>>>>
>>>>>> -- 
>>>>>> 2.9.3
>>>>>>
>>>>>>
>>>>> _______________________________________________
>>>>> edk2-devel mailing list
>>>>> edk2-devel@lists.01.org
>>>>> https://lists.01.org/mailman/listinfo/edk2-devel
>>>>>
>>>
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>>



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

* Is: Fix for 4MB BIOS payload in hvmloader. Was:Re: [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables)
  2017-05-18 12:36           ` Laszlo Ersek
@ 2017-05-23 14:12             ` Konrad Rzeszutek Wilk
       [not found]               ` <59246AD9020000780015C380@prv-mh.provo.novell.com>
       [not found]               ` <6af13bb5-0bfb-c9e3-e9fe-d1361d851e7d@arm.com>
  0 siblings, 2 replies; 19+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-05-23 14:12 UTC (permalink / raw)
  To: Laszlo Ersek, xen-devel, julien.grall, jbeulich
  Cc: Gary Lin, Jordan Justen, edk2-devel-01

Adding Jan (autor of patch) and Julien (Xen release manager);

Pls see below.

On Thu, May 18, 2017 at 02:36:33PM +0200, Laszlo Ersek wrote:
> On 05/16/17 06:20, Gary Lin wrote:
> > On Mon, May 15, 2017 at 05:40:59PM -0700, Jordan Justen wrote:
> >> On 2017-05-12 01:40:34, Laszlo Ersek wrote:
> >>> On 05/12/17 04:02, Gary Lin wrote:
> >>>> On Mon, May 08, 2017 at 12:27:59PM +0800, Gary Lin wrote:
> >>>>> On Sat, May 06, 2017 at 09:30:18PM +0200, Laszlo Ersek wrote:
> >>>>>> (All hail Saturday!)
> >>>>>>
> >>>>>> Gary, can you please fetch this from my repo (URL & branch name below)
> >>>>>> and test it with Xen? Please test both the 4MB and the 2MB build. (I
> >>>>>> also tested both, with qemu + "-bios".)
> >>>>> Hi Laszlo,
> >>>>>
> >>>>> I have done some simples test with xen, and the 2MB build seems fine.
> >>>>> It booted into grub2 menu successfully. However, the 4MB build never boots.
> >>>>> The QEMU window showed less than 1 sec and then disappeared.
> >>>>>
> >>>>> Here is the snippet from 'xl dmesg'
> >>>>>
> >>>>> (d15)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... done.
> >>>>> (d15) Testing HVM environment:
> >>>>> (d15)  - REP INSB across page boundaries ... passed
> >>>>> (d15)  - GS base MSRs and SWAPGS ... passed
> >>>>> (d15) Passed 2 of 2 tests
> >>>>> (d15) Writing SMBIOS tables ...
> >>>>> (d15) Loading OVMF ...
> >>>>> (d15) no BIOS ROM image found
> >>>>> (d15) *** HVMLoader bug at hvmloader.c:381
> >>>>> (d15) *** HVMLoader crashed.
> >>>>>
> >>>>> I'm pretty sure that the ovmf path is right, so it seems Xen just rejected
> >>>>> the 4MB build :-\
> >>>>>
> >>>>> I'll try to dig more information.
> >>>>>
> >>>> There is a function in the xen hvmloader clearing the memory from
> >>>> 0x400000 to 0x800000. Unfortunately, the hvm_start_info struct of the
> >>>> 4MB OVMF was loaded to 0x588000, so the struct was cleared mistakenly
> >>>> and hvmloader cannot find the firmware. Xen is not ready for the 4MB
> >>>> build yet :-\
> >>>>
> >>>> The discussion in xen-devel:
> >>>> https://lists.xen.org/archives/html/xen-devel/2017-05/msg01053.html
> >>>
> >>> Thank you for the feedback!
> >>>
> >>> In this case, I think we should drop the last patch from this series.
> >>
> >> Can we come up with a plan for trying to fix this? Gary, would it be
> >> okay if we opened a bug and assigned it to you? Or, do you have
> >> another suggestion for a possible Xen owner?
> >>
> > Jan Beulich (also a SUSE employee) is working on the patch(*), and it
> > works for me.
> 
> We should distinguish a TianoCore BZ entry for this, from a Xen bug
> report for this. The former would depend on the latter.
> 
> The TianoCore BZ assignee's job would be to monitor the upstream Xen
> fix, and to submit the last patch of this series -- separated out -- to
> edk2-devel once upstream Xen commits the fix.
> 
> Upstream Xen does not have a bug tracker that is widely used in their
> community:
> 
> https://wiki.xen.org/wiki/Reporting_Bugs_against_Xen_Project
> 
> > The primary location for reporting bugs against the hypervisor and
> > associated bundled tools [...] is by posting to the xen-devel mailing
> > list (list info). Please tag your subject line with a '[BUG]' prefix.
> > Note that you do not need to be subscribed to the list to post
> > (although non-subscribers are moderated this usually happens pretty
> > quickly) and that list policy is to CC people so you shouldn't miss
> > any replies.
> >
> > [...]
> >
> > Although a bugzilla instance does exist it is not well maintained or
> > widely used by developers. If you really want to file a bug in
> > bugzilla you are strongly recommended to also post to the mailing
> > list.
> 
> This does not make things easier for us, because rather than recurrently
> check a simple bug status field in the Xen tracker, we'd have to be
> tapped into Xen development, and follow the email thread & any relevant
> commits closely. In reality I'm not even subscribed to xen-devel.
> 
> The situation is further hampered by the fact that Xen is (apparently)
> right at 4.9.0-rc5, so they likely won't commit Jan's hvmloader patch
> until Xen 4.9 is out. This is a problem for a potential TianoCore-side
> BZ because the delay will make us forget about the issue.
> 
> Thanks,
> Laszlo
> 
> > 
> > Cheers,
> > 
> > Gary Lin
> > 
> > (*) https://lists.xen.org/archives/html/xen-devel/2017-05/msg01242.html
> >> Thanks,
> >>
> >> -Jordan
> >>
> >>>
> >>> However, your test results also confirm that the 2MB build continues to
> >>> work with Xen, which means that the reworking of the
> >>> EmuVariableFvbRuntimeDxe driver in this series, and the underlying
> >>> tweaks+cleanups series, cause no regression.
> >>>
> >>> Can you please respond, with your "Regression-tested-by", to:
> >>>
> >>> (1) the full series
> >>>
> >>>   [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks
> >>>
> >>> (2) and patches 1 through 3 in this series? (Patch #4 is just
> >>> documentation, for which Tested-by would be strange.)
> >>>
> >>> Thank you!
> >>> Laszlo
> >>>
> >>>
> >>>>>> Note: this series depends on:
> >>>>>>
> >>>>>>   [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks
> >>>>>>   https://lists.01.org/pipermail/edk2-devel/2017-May/010527.html
> >>>>>>
> >>>>>> and it has been pushed to my github repo as such.
> >>>>>>
> >>>>>> Repo:   https://github.com/lersek/edk2.git
> >>>>>> Branch: emu4k
> >>>>>>
> >>>>>> Cc: Gary Ching-Pang Lin <glin@suse.com>
> >>>>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
> >>>>>>
> >>>>>> Thanks,
> >>>>>> Laszlo
> >>>>>>
> >>>>>> Laszlo Ersek (5):
> >>>>>>   OvmfPkg/EmuVariableFvbRuntimeDxe: strip trailing whitespace
> >>>>>>   OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB
> >>>>>>   OvmfPkg/PlatformPei: align EmuVariableNvStore at any page boundary
> >>>>>>   OvmfPkg/README: document 4MB flash layout
> >>>>>>   OvmfPkg: make the 4MB flash size the default (again)
> >>>>>>
> >>>>>>  OvmfPkg/OvmfPkgIa32.dsc                |   2 +-
> >>>>>>  OvmfPkg/OvmfPkgIa32X64.dsc             |   2 +-
> >>>>>>  OvmfPkg/OvmfPkgX64.dsc                 |   2 +-
> >>>>>>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h |  26 ++-
> >>>>>>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 191 +++++++++-----------
> >>>>>>  OvmfPkg/PlatformPei/Platform.c         |  20 +-
> >>>>>>  OvmfPkg/README                         |  39 +++-
> >>>>>>  7 files changed, 143 insertions(+), 139 deletions(-)
> >>>>>>
> >>>>>> -- 
> >>>>>> 2.9.3
> >>>>>>
> >>>>>>
> >>>>> _______________________________________________
> >>>>> edk2-devel mailing list
> >>>>> edk2-devel@lists.01.org
> >>>>> https://lists.01.org/mailman/listinfo/edk2-devel
> >>>>>
> >>>
> >>> _______________________________________________
> >>> edk2-devel mailing list
> >>> edk2-devel@lists.01.org
> >>> https://lists.01.org/mailman/listinfo/edk2-devel
> >>
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


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

* Re: Is: Fix for 4MB BIOS payload in hvmloader. Was:Re: [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables)
       [not found]               ` <59246AD9020000780015C380@prv-mh.provo.novell.com>
@ 2017-05-23 16:04                 ` Laszlo Ersek
  0 siblings, 0 replies; 19+ messages in thread
From: Laszlo Ersek @ 2017-05-23 16:04 UTC (permalink / raw)
  To: Jan Beulich, Konrad Rzeszutek Wilk
  Cc: julien.grall, Jordan Justen, edk2-devel-01, xen-devel, Gary Lin

On 05/23/17 17:01, Jan Beulich wrote:
>>>> On 23.05.17 at 16:12, <konrad@kernel.org> wrote:
>> On Thu, May 18, 2017 at 02:36:33PM +0200, Laszlo Ersek wrote:
>>> The situation is further hampered by the fact that Xen is (apparently)
>>> right at 4.9.0-rc5, so they likely won't commit Jan's hvmloader patch
>>> until Xen 4.9 is out. This is a problem for a potential TianoCore-side
>>> BZ because the delay will make us forget about the issue.
> 
> The patch went in in time for rc6.

Thank you Jan for the speedy fix!

While reviewing version 2 of this patch set, Jordan wrote in
<https://lists.01.org/pipermail/edk2-devel/2017-May/010776.html>:

> With the understanding that we're holding off on the final patch for
> now to coordinate with Xen:
>
> Series Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

"the final patch" referenced there was:

  [edk2] [PATCH v2 5/5] OvmfPkg: make the 4MB flash size the default
                        (again)
  https://lists.01.org/pipermail/edk2-devel/2017-May/010763.html

So I have now modified the commit message on that patch, adding the
following paragraph:

> Xen gained support for the 4MB flash image in Xen commit 0d6968635ce5
> ("hvmloader: avoid tests when they would clobber used memory",
> 2017-05-19), which is part of Xen 4.9.0-rc6.

Seeing Gary's Tested-by on the Xen commit, I've also pushed the reworded
edk2 patch, as commit 1c47fcd465a4.

Thanks!
Laszlo


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

* Re: Is: Fix for 4MB BIOS payload in hvmloader. Was:Re: [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables)
       [not found]               ` <6af13bb5-0bfb-c9e3-e9fe-d1361d851e7d@arm.com>
@ 2017-05-23 16:20                 ` Laszlo Ersek
  0 siblings, 0 replies; 19+ messages in thread
From: Laszlo Ersek @ 2017-05-23 16:20 UTC (permalink / raw)
  To: Julien Grall, Konrad Rzeszutek Wilk, xen-devel, jbeulich
  Cc: Gary Lin, Jordan Justen, edk2-devel-01

On 05/23/17 17:02, Julien Grall wrote:
> Hi,
> 
> On 23/05/17 15:12, Konrad Rzeszutek Wilk wrote:
>>>> The primary location for reporting bugs against the hypervisor and
>>>> associated bundled tools [...] is by posting to the xen-devel mailing
>>>> list (list info). Please tag your subject line with a '[BUG]' prefix.
>>>> Note that you do not need to be subscribed to the list to post
>>>> (although non-subscribers are moderated this usually happens pretty
>>>> quickly) and that list policy is to CC people so you shouldn't miss
>>>> any replies.
>>>>
>>>> [...]
>>>>
>>>> Although a bugzilla instance does exist it is not well maintained or
>>>> widely used by developers. If you really want to file a bug in
>>>> bugzilla you are strongly recommended to also post to the mailing
>>>> list.
>>>
>>> This does not make things easier for us, because rather than recurrently
>>> check a simple bug status field in the Xen tracker, we'd have to be
>>> tapped into Xen development, and follow the email thread & any relevant
>>> commits closely. In reality I'm not even subscribed to xen-devel.
> 
> We recently introduced a tracker ([1]) to help us following the state of
> bugs/features. It is managed by maintainers and read-only for the others.
> 
> The usual process is to report the bug on the mailing-list and a
> maintainer will create a bug on the tracker if we have no immediate
> solution.

That's perfect; we'll be able to reference Xen issue reports with URLs
like (just an example) <https://xenproject.atlassian.net/browse/XEN-86>,
from TianoCore BZs. That should enable us to quickly re-check the state
of the Xen report whenever we revisit the dependent TianoCore BZ.

... Can you please add the information about "xenproject.atlassian.net"
to the Xen Wiki article at
<https://wiki.xen.org/wiki/Reporting_Bugs_against_Xen_Project>?

> In this case, the patch made rc6. So I believe the problem is resolved.

Yes, it is. Thanks!
Laszlo

> Cheers,
> 
> [1] https://xenproject.atlassian.net/projects/XEN/issues
> 



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

end of thread, other threads:[~2017-05-23 16:20 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-06 19:30 [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables) Laszlo Ersek
2017-05-06 19:30 ` [PATCH 1/5] OvmfPkg/EmuVariableFvbRuntimeDxe: strip trailing whitespace Laszlo Ersek
2017-05-12  9:07   ` Gary Lin
2017-05-06 19:30 ` [PATCH 2/5] OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB Laszlo Ersek
2017-05-12  9:07   ` Gary Lin
2017-05-15 23:50   ` Jordan Justen
2017-05-06 19:30 ` [PATCH 3/5] OvmfPkg/PlatformPei: align EmuVariableNvStore at any page boundary Laszlo Ersek
2017-05-12  9:08   ` Gary Lin
2017-05-06 19:30 ` [PATCH 4/5] OvmfPkg/README: document 4MB flash layout Laszlo Ersek
2017-05-06 19:30 ` [PATCH 5/5] OvmfPkg: make the 4MB flash size the default (again) Laszlo Ersek
2017-05-08  4:27 ` [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables) Gary Lin
2017-05-12  2:02   ` Gary Lin
2017-05-12  8:40     ` Laszlo Ersek
2017-05-16  0:40       ` Jordan Justen
2017-05-16  4:20         ` Gary Lin
2017-05-18 12:36           ` Laszlo Ersek
2017-05-23 14:12             ` Is: Fix for 4MB BIOS payload in hvmloader. Was:Re: " Konrad Rzeszutek Wilk
     [not found]               ` <59246AD9020000780015C380@prv-mh.provo.novell.com>
2017-05-23 16:04                 ` Laszlo Ersek
     [not found]               ` <6af13bb5-0bfb-c9e3-e9fe-d1361d851e7d@arm.com>
2017-05-23 16:20                 ` Laszlo Ersek

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