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.

-Jordan

------------------------------------------------------------------------------
"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

Reply via email to