On 05/20/14 20:08, Jordan Justen wrote:
> On Tue, May 20, 2014 at 2:01 AM, Laszlo Ersek <ler...@redhat.com> wrote:
>> Three questions:
>>
>> On 05/19/14 23:27, Jordan Justen wrote:
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com>
>>> ---
>>> OvmfPkg/OvmfPkgIa32.dsc | 2 ++
>>> OvmfPkg/OvmfPkgIa32.fdf | 6 +++---
>>> OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++
>>> OvmfPkg/OvmfPkgIa32X64.fdf | 6 +++---
>>> OvmfPkg/OvmfPkgX64.dsc | 2 ++
>>> OvmfPkg/OvmfPkgX64.fdf | 6 +++---
>>> 6 files changed, 15 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>>> index f7064b7..1ceb909 100644
>>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>>> @@ -337,6 +337,8 @@
>>> #
>>>
>>> ################################################################################
>>> [Components]
>>> + OvmfPkg/ResetVector/ResetVector.inf
>>> +
>>> #
>>> # SEC Phase modules
>>> #
>>> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
>>> index 76b868d..411e9de 100644
>>> --- a/OvmfPkg/OvmfPkgIa32.fdf
>>> +++ b/OvmfPkg/OvmfPkgIa32.fdf
>>> @@ -1,7 +1,7 @@
>>> ## @file
>>> # Open Virtual Machine Firmware: FDF
>>> #
>>> -# Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
>>> +# Copyright (c) 2006 - 2014, 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
>>> @@ -187,7 +187,7 @@ READ_LOCK_STATUS = TRUE
>>> #
>>> INF OvmfPkg/Sec/SecMain.inf
>>>
>>> -INF RuleOverride=RESET_VECTOR
>>> UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.inf
>>> +INF RuleOverride=RESET_VECTOR OvmfPkg/ResetVector/ResetVector.inf
>>>
>>>
>>> ################################################################################
>>> [FV.PEIFV]
>>> @@ -503,5 +503,5 @@ FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
>>>
>>> [Rule.Common.SEC.RESET_VECTOR]
>>> FILE RAW = $(NAMED_GUID) {
>>> - RAW RAW |.raw
>>> + RAW BIN Align = 16 |.bin
>>> }
>>
>> Ia32, before the patch:
>> (1) "UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.inf" said
>>
>> [Binaries.Ia32]
>> RAW|ResetVector.ia32.raw|*
>>
>> (2) "UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.ia32.raw" was prepared
>> with "UefiCpuPkg/ResetVector/Vtf0/Build.py".
>>
>> (3) Which invoked "nasm" on
>> "UefiCpuPkg/ResetVector/Vtf0/ResetVectorCode.asm", and fixed up the
>> output with "UefiCpuPkg/ResetVector/Vtf0/Tools/FixupForRawSection.py".
>>
>> For Ia32, the fixup means padding with a NOP slide at the front, so that
>> the resultant file size is 4, modulo 8.
>>
>> (4) The "UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.ia32.raw" file was
>> included with the RESET_VECTOR rule:
>> - output file type RAW: binary data,
>> - NAMED_GUID: uses FILE_GUID from INF/[defines], ie.
>> EFI_FFS_VOLUME_TOP_FILE_GUID
>> - we have one leaf section pattern, with:
>> - section type: RAW
>> - input file type: RAW
>> - options: none
>> - extension: .raw -- one leaf section is instantiated for each .raw
>> file listed in the INF file that is connected to the rule.
>> "UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.inf" -->
>> "UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.ia32.raw"
>>
>> After the patch:
>> (1) "OvmfPkg/ResetVector/ResetVector.inf" (from patch 3) says
>>
>> [Sources]
>> ResetVector.nasmbin
>>
>> (2), (3) for which the build rule (from patch 1) designates
>> "OvmfPkg/ResetVector/ResetVector.bin" as output file.
>>
>> The source is "OvmfPkg/ResetVector/ResetVector.nasmbin", which is
>> identical to "UefiCpuPkg/ResetVector/Vtf0/ResetVectorCode.asm". OK.
>>
>> The output is not fixed up with FixupForRawSection.py.
>>
>> (4) "OvmfPkg/ResetVector/ResetVector.bin" is included with the new
>> RESET_VECTOR rule:
>> - output file type RAW: binary data; unchanged, good.
>> - NAMED_GUID: uses FILE_GUID from INF/[defines], ie.
>> EFI_FFS_VOLUME_TOP_FILE_GUID; unchanged, good.
>> - we have one leaf section pattern, with:
>> - section type: RAW
>> - input file type: BIN -- changed.
>> - options: Align = 16; changed
>> - extension: .bin -- one leaf section is instantiated for each .bin
>> file listed in the INF file that is connected to the rule.
>> "OvmfPkg/ResetVector/ResetVector.inf" -->
>> "OvmfPkg/ResetVector/ResetVector.bin"
>>
>> Questions:
>>
>> Q1: Why is the input file type in the leaf section pattern changed from
>> RAW to BIN? The FDF spec doesn't seem to explain the BIN file type. Are
>> we doing this only to match the file suffix ".bin"?
>
> I think I was matching what I saw another platform do when building
> the VTF during the EDK II build.
>
> I think RAW could still work.
>
>> Q2: We seem to replace the NOP slide at the front (which ensures that
>> the file going into the section has size 4 modulo 8), with an Alignment
>> of 16. Why does this work?
>
> Hmm, I think the 4 modulo 8 was that Fv's want sections to start with
> 8 alignment, and there is a 4 byte section header. Therefore we had to
> pad the file so GenFv could be happy putting it (properly aligned) at
> the end of the FV.
>
> But, I think all we really needed was to ask for alignment of the
> binary, and GenFv could handle all that for us. Maybe the old way was
> going through more effort than required.
>
>>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>>> index 26d1132..66a4bc3 100644
>>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>>> @@ -343,6 +343,8 @@
>>> #
>>>
>>> ################################################################################
>>> [Components.IA32]
>>> + OvmfPkg/ResetVector/ResetVector.inf
>>> +
>>> #
>>> # SEC Phase modules
>>> #
>>> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
>>> index e037b72..c2ff257 100644
>>> --- a/OvmfPkg/OvmfPkgIa32X64.fdf
>>> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf
>>> @@ -1,7 +1,7 @@
>>> ## @file
>>> # Open Virtual Machine Firmware: FDF
>>> #
>>> -# Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
>>> +# Copyright (c) 2006 - 2014, 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
>>> @@ -187,7 +187,7 @@ READ_LOCK_STATUS = TRUE
>>> #
>>> INF OvmfPkg/Sec/SecMain.inf
>>>
>>> -INF RuleOverride=RESET_VECTOR USE = IA32
>>> UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.inf
>>> +INF RuleOverride=RESET_VECTOR OvmfPkg/ResetVector/ResetVector.inf
>>>
>>>
>>> ################################################################################
>>> [FV.PEIFV]
>>> @@ -503,5 +503,5 @@ FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
>>>
>>> [Rule.Common.SEC.RESET_VECTOR]
>>> FILE RAW = $(NAMED_GUID) {
>>> - RAW RAW |.raw
>>> + RAW BIN Align = 16 |.bin
>>> }
>>
>> Same questions here, plus another one:
>>
>> Q3: Why is it safe to drop "USE = IA32" in the above?
>>
>> (I don't think it is, but I could be wrong.)
>
> Yikes, I forgot to test this. (I need to make OvmfPkg/build.sh support
> the IA32+X64 combination, so it is easier to test. :)
>
> But, I think since the dsc references the .inf in an IA32 area, then I
> think the FDF knows to use IA32.
If you get a good test result, please post it, and then I'll add my R-b.
I'm OK with answers 1 and 2.
Thanks!
Laszlo
------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
