On Wed, Oct 30, 2013 at 2:03 PM, Laszlo Ersek <ler...@redhat.com> wrote:
> On 10/28/13 22:27, Jordan Justen wrote:
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com>
>> ---
>> OvmfPkg/OvmfPkgIa32.fdf | 93
>> ++++++++++++++++++++++++++++++++++++++------
>> OvmfPkg/OvmfPkgIa32X64.fdf | 93
>> ++++++++++++++++++++++++++++++++++++++------
>> OvmfPkg/OvmfPkgX64.fdf | 93
>> ++++++++++++++++++++++++++++++++++++++------
>> 3 files changed, 243 insertions(+), 36 deletions(-)
>>
>> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
>> index 40f0651..03cdedc 100644
>> --- a/OvmfPkg/OvmfPkgIa32.fdf
>> +++ b/OvmfPkg/OvmfPkgIa32.fdf
>> @@ -30,26 +30,95 @@ DEFINE FD_SIZE_1MB=
>>
>> !ifdef $(FD_SIZE_1MB)
>> [FD.OVMF]
>> -BaseAddress = 0xFFF00000
>> -Size = 0x00100000
>> +BaseAddress = 0xFFF00000|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
>> +Size = 0x00100000|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize
>
> Ah, much clearer; FDF spec to the rescue. We're capturing these values
> from the FDF tokens in the PCDs (so that we can access them in C too,
> presumably).
>
>> ErasePolarity = 1
>> -BlockSize = 0x1000
>> +BlockSize = 0x1000|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareBlockSize
>> NumBlocks = 0x100
>> +!else
>> +[FD.OVMF]
>> +BaseAddress = 0xFFE00000|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
>> +Size = 0x00200000|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize
>> +ErasePolarity = 1
>> +BlockSize = 0x1000|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareBlockSize
>> +NumBlocks = 0x200
>> +!endif
>
> This duplicates the the !else branch (ie. when the flash size is 2MB),
> and performs the same PCD assignments as above.
>
> At this point we're independent of the FD size.
>
> Five PCDs remain unused (PcdOvmfFlashNvStorage*).
>
>
>> +
>> +0x00000000|0x0000e000
>> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
>
> According to 2.3.4 FD Region Layout, this starts a region as usual (at
> offset 0 from BaseAddress, size 0xe000 == 56K), and the offset and the
> size are captured in the PCDs (shortcut form).
>
> Do you have any estimate how much "useful" data these 56 kilobytes can
> save? (For example, kernel stack dumps are sometimes stored there.) Is
> 56 KB a "usual" amount for real hardware?
I think ~64k is the most common size used for non-volatile data
storage in UEFI systems. (At least it is the most common size I've
seen used by Intel.)
It could be more than what is needed for OVMF. But, I'm not sure how
small is a safe number when secure-boot is enabled.
> Regarding the PCDs that have been introduced previously... We're now
> down to 4 unused PCDs. Interestingly, the size of the region is assigned
> to a PCD in a different "namespace" (GUID).
>
>> +#NV_VARIABLE_STORE
>> +DATA = {
>> + ## This is the EFI_FIRMWARE_VOLUME_HEADER
>> + # ZeroVector []
>> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> + # FileSystemGuid: gEfiSystemNvDataFvGuid =
>> + # { 0xFFF12B8D, 0x7696, 0x4C8B,
>> + # { 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50 }}
>> + 0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C,
>> + 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50,
>> + # FvLength: 0x20000
>
> (1) Isn't FvLength too big? I mean we're in a region that's 56 KB in
> size, and FvLength is 128 KB. This part looks like a copy fron Nt32Pkg
> (and/or EmulatorPkg), shouldn't we update it?
This FV covers the 64KB data, plus the 64KB backup (spare) block. I
don't know why it is always done this way with an FV header.
>> + 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00,
>> + #Signature "_FVH" #Attributes
>> + 0x5f, 0x46, 0x56, 0x48, 0xff, 0xfe, 0x04, 0x00,
>> + #HeaderLength #CheckSum #ExtHeaderOffset #Reserved #Revision
>> + 0x48, 0x00, 0x19, 0xF9, 0x00, 0x00, 0x00, 0x02,
>> + #Blockmap[0]: 2 Blocks * 0x10000 Bytes / Block
>
> OK, so blocksize dictates FvLength... Where does blocksize come from?
This comment is wrong. It should be 0x20 blocks of size 0x1000. QEMU
flash uses 4KB block sizes.
> According to "MdePkg/Include/Pi/PiFirmwareVolume.h",
> EFI_FV_BLOCK_MAP_ENTRY.Length (visible below) determines the size of the
> blocks.
>
> So, I think we could update this if we wanted to -- I don't know if we
> need to.
>
>> + 0x20, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00,
>> + #Blockmap[1]: End
>> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> + ## This is the VARIABLE_STORE_HEADER
>> +!if $(SECURE_BOOT_ENABLE) == TRUE
>> + #Signature: gEfiAuthenticatedVariableGuid =
>> + # { 0xaaf32c78, 0x947b, 0x439a,
>> + # { 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92 }}
>> + 0x78, 0x2c, 0xf3, 0xaa, 0x7b, 0x94, 0x9a, 0x43,
>> + 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92,
>> +!else
>> + #Signature: gEfiVariableGuid =
>> + # { 0xddcf3616, 0x3275, 0x4164,
>> + # { 0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d }}
>> + 0x16, 0x36, 0xcf, 0xdd, 0x75, 0x32, 0x64, 0x41,
>> + 0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d,
>> +!endif
>> + #Size: 0xe000
>> (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) - 0x48 (size
>> of EFI_FIRMWARE_VOLUME_HEADER) = 0xdfb8
>> + # This can speed up the Variable Dispatch a bit.
>> + 0xB8, 0xDF, 0x00, 0x00,
>> + #FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32
>> + 0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
>> +}
>
> OK. I'll trust you on this.
>
>>
>> -0x00000000|0x000EC000
>
> (So normally this introduced FVMAIN_COMPACT...)
>
>> +0x0000e000|0x00001000
>> +#NV_EVENT_LOG
>> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize
>
> I guess we'll use this in a ringbuffer-like fashion... One block (4K).
>
> And we've consumed two further OVMF PCDs; we're down to two
> (PcdOvmfFlashNvStorageFtw*).
>
>> +
>> +0x0000f000|0x00001000
>> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
>> +#NV_FTW_WORKING
>> +DATA = {
>> + # EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER->Signature =
>> gEdkiiWorkingBlockSignatureGuid =
>> + # { 0x9e58292b, 0x7c68, 0x497d, { 0xa0, 0xce, 0x65, 0x0, 0xfd, 0x9f,
>> 0x1b, 0x95 }}
>> + 0x2b, 0x29, 0x58, 0x9e, 0x68, 0x7c, 0x7d, 0x49,
>> + 0xa0, 0xce, 0x65, 0x0, 0xfd, 0x9f, 0x1b, 0x95,
>> + # Crc:UINT32 #WorkingBlockValid:1, WorkingBlockInvalid:1,
>> Reserved
>> + 0x2c, 0xaf, 0x2c, 0x64, 0xFE, 0xFF, 0xFF, 0xFF,
>> + # WriteQueueSize: UINT64
>> + 0xE0, 0x0F, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
>> +}
>
> Guess it's OK... Resembles Nt32Pkg.
>
> No idea about the purpose. (I should probably educate myself about fault
> tolerant writes.)
I wouldn't recommend it if you want to retain your sanity. ;)
> One PCD left.
>
>> +
>> +0x00010000|0x00010000
>> +#NV_FTW_SPARE
>> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwSpareBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
>> +
>
> OK, used up last PCD. 64 KB for the "fault tolerant spare", whatever it is.
When the non-volatile 64KB needs to be erased, it is first programmed
into the spare block.
>> +!ifdef $(FD_SIZE_1MB)
>> +0x00020000|0x000CC000
>> FV = FVMAIN_COMPACT
>
> The size of FVMAIN_COMPACT used to be (in the 1MB build) 0x000EC000 ==
> 944 KB. We're now making sure that FVMAIN_COMPACT ends at the same
> offset (0xEC000), but its region only starts after the nvvar-related
> areas. So the new size is 0xCC000 == 816 KB, 128 KB less than before.
> Hopefully it's enough (for release builds).
I think there is still a reasonable amount remaining with the release
build on GCC47. (~125k)
>> 0x000EC000|0x14000
>> FV = SECFV
>
> Yes, from this on the offsets/sizes are unchanged.
>
> I think the commit message could state explicitly that we're carving out
> the nvvar storage of where FVMAIN_COMPACT used to start.
I guess it would be better to add a README.flash file at this point.
-Jordan
------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
