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

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?


> 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.)

> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 66459c2..73a6460 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -342,6 +342,8 @@
>  #
>  
> ################################################################################
>  [Components]
> +  OvmfPkg/ResetVector/ResetVector.inf
> +
>    #
>    # SEC Phase modules
>    #
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index b9e9c59..5a5276d 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.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 OvmfPkg/ResetVector/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
>    }
> 

OK, so for X64 we don't change from UefiCpuPkg to our own, because we
used to have our own version anyway.

Before the patch:

(1) "OvmfPkg/ResetVector/Bin/ResetVector.inf" said

[Binaries.X64]
  RAW|ResetVector.x64.raw|*

(2) "OvmfPkg/ResetVector/Bin/ResetVector.x64.raw" was prepared with
"OvmfPkg/ResetVector/Build.py".

(3) Which invoked "nasm" on "OvmfPkg/ResetVector/ResetVectorCode.asm",
and fixed up the output with
"OvmfPkg/ResetVector/Tools/FixupForRawSection.py".

In the original UefiCpuPkg fixup script, page tables are built at fixup
time, for X64. However in our own version,
"OvmfPkg/ResetVector/Tools/FixupForRawSection.py", the fixup used to be
the same as for Ia32, because we build our page tables at runtime. This
happens in our version of "PageTables64.asm",
"OvmfPkg/ResetVector/Ia32/PageTables64.asm".

Hence the fixup means padding with a NOP slide at the front, so that the
resultant file size is 4, modulo 8.

(4) The "OvmfPkg/ResetVector/Bin/ResetVector.x64.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.
    "OvmfPkg/ResetVector/Bin/ResetVector.inf" -->
    "OvmfPkg/ResetVector/Bin/ResetVector.x64.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 our OVMF-specific
"OvmfPkg/ResetVector/ResetVectorCode.asm". OK.

The output is not fixed up with FixupForRawSection.py.

(4) (I won't repeat this step, same as for Ia32 and Ia32X64.)

So for X64, no *additional* questions, because the restructuring doesn't
affect "OvmfPkg/ResetVector/Ia32/PageTables64.asm".


So, I have the three questions (repeating them from above):

Q1: Why is the input file type in the leaf section patterns 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"?

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. We do this for all three OVMF builds. Why does this work?

Q3: Why is it safe to drop "USE = IA32" in the Ia32X64 rule override?

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

Reply via email to