* [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
* 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
* [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
* 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 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
* [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
* 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
* [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 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
[parent not found: <59246AD9020000780015C380@prv-mh.provo.novell.com>]
* 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
[parent not found: <6af13bb5-0bfb-c9e3-e9fe-d1361d851e7d@arm.com>]
* 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