[PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c

[PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c

[Leo Duran]

We have a situation where an x86 processor does not start fetching code
at the traditional hardware reset vector address 0xFFFFFFF0.
The implication is that the hidden base address part of the CS register
is initialized with a value different than the traditional 0xFFFF0000.

In our case:
CS = 0xF000 (as expected)
EIP = 0xFFF0 (as expected)
CSLIMIT = 0xFFFF (as expected)
CSBASE = 0x####0000 (where #### is not the traditional 0xFFFF)

Thus in our case execution starts at: CSBASE+EIP = 0x####FFF0.

To account for this behavior, we define an FDF address override
to force a rebase of the FV section containing the VTF file.
For example, last FV section defined as:
[FV.FvSecPei]
FvBaseAddress = $(FV_BOOT_BASE)
FvForceRebase = TRUE

However, when the GenFv tool parses that section it is unware
of the possible rebasing, and assumes the section ends at the
traditional 4G-byte boundary. This patch solves this by simply
adding a check for a possible rebase scenario.

Leo Duran (1):
  BaseTools/Source/C/GenFv/GenFvInternalLib.c

 BaseTools/Source/C/GenFv/GenFvInternalLib.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

-- 
1.9.1

[PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c

[Leo Duran]

Account for rebase of FV section containing VTF file on IA32/IA64.
This supports cases where the reset vector may not be set at 0xFFFFFFF0.

For example, FV section defined as:
[FV.FvSecPei]
  FvBaseAddress = $(FV_BOOT_BASE)
  FvForceRebase = TRUE

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
 BaseTools/Source/C/GenFv/GenFvInternalLib.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
index 7c839e2..8c2827b 100644
--- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
+++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
@@ -2770,11 +2770,13 @@ Returns:
       //
       // Update reset vector (SALE_ENTRY for IPF)
       // Now for IA32 and IA64 platform, the fv which has bsf file must have the 
-      // EndAddress of 0xFFFFFFFF. Thus, only this type fv needs to update the   
-      // reset vector. If the PEI Core is found, the VTF file will probably get  
-      // corrupted by updating the entry point.                                  
+      // EndAddress of 0xFFFFFFFF (unless the section was rebased).
+      // Thus, only this type fv needs to update the  reset vector.
+      // If the PEI Core is found, the VTF file will probably get
+      // corrupted by updating the entry point.
       //
-      if ((mFvDataInfo.BaseAddress + mFvDataInfo.Size) == FV_IMAGES_TOP_ADDRESS) {       
+      if ((mFvDataInfo.ForceRebase == 1) ||
+        (mFvDataInfo.BaseAddress + mFvDataInfo.Size) == FV_IMAGES_TOP_ADDRESS) {
         Status = UpdateResetVector (&FvImageMemoryFile, &mFvDataInfo, VtfFileImage);
         if (EFI_ERROR(Status)) {                                               
           Error (NULL, 0, 3000, "Invalid", "Could not update the reset vector.");
-- 
1.9.1

Re: [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c

[Gao, Liming]

Duran:
  Reusing FvForceRebase flag is a good idea to resolve this issue. I agree your change. Reviewed-by: Liming Gao <liming.gao@intel.com>

Thank
Liming
> -----Original Message-----
> From: Leo Duran [mailto:leo.duran@amd.com]
> Sent: Thursday, August 11, 2016 11:23 PM
> To: edk2-devel@lists.01.org
> Cc: Zhu, Yonghong <yonghong.zhu@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Leo Duran <leo.duran@amd.com>
> Subject: [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c
> 
> Account for rebase of FV section containing VTF file on IA32/IA64.
> This supports cases where the reset vector may not be set at 0xFFFFFFF0.
> 
> For example, FV section defined as:
> [FV.FvSecPei]
>   FvBaseAddress = $(FV_BOOT_BASE)
>   FvForceRebase = TRUE
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Leo Duran <leo.duran@amd.com>
> ---
>  BaseTools/Source/C/GenFv/GenFvInternalLib.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> index 7c839e2..8c2827b 100644
> --- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> +++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> @@ -2770,11 +2770,13 @@ Returns:
>        //
>        // Update reset vector (SALE_ENTRY for IPF)
>        // Now for IA32 and IA64 platform, the fv which has bsf file must have the
> -      // EndAddress of 0xFFFFFFFF. Thus, only this type fv needs to update the
> -      // reset vector. If the PEI Core is found, the VTF file will probably get
> -      // corrupted by updating the entry point.
> +      // EndAddress of 0xFFFFFFFF (unless the section was rebased).
> +      // Thus, only this type fv needs to update the  reset vector.
> +      // If the PEI Core is found, the VTF file will probably get
> +      // corrupted by updating the entry point.
>        //
> -      if ((mFvDataInfo.BaseAddress + mFvDataInfo.Size) ==
> FV_IMAGES_TOP_ADDRESS) {
> +      if ((mFvDataInfo.ForceRebase == 1) ||
> +        (mFvDataInfo.BaseAddress + mFvDataInfo.Size) ==
> FV_IMAGES_TOP_ADDRESS) {
>          Status = UpdateResetVector (&FvImageMemoryFile, &mFvDataInfo,
> VtfFileImage);
>          if (EFI_ERROR(Status)) {
>            Error (NULL, 0, 3000, "Invalid", "Could not update the reset vector.");
> --
> 1.9.1

Re: [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c

[Duran, Leo]

> -----Original Message-----
> From: Gao, Liming [mailto:liming.gao@intel.com]
> Sent: Friday, August 12, 2016 3:32 AM
> To: Duran, Leo <leo.duran@amd.com>; edk2-devel@lists.01.org
> Cc: Zhu, Yonghong <yonghong.zhu@intel.com>
> Subject: RE: [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c
> 
> Duran:
>   Reusing FvForceRebase flag is a good idea to resolve this issue. I agree your
> change. Reviewed-by: Liming Gao <liming.gao@intel.com>
> 
[Duran, Leo] 
Excellent, thanks!

Please advise on "next steps":
1) Do we need a reply from Yonghong Zhu?
2) When would it be reasonable to expect integration of this code into mainline?

Thanks again,
Leo.

> Thank
> Liming
> > -----Original Message-----
> > From: Leo Duran [mailto:leo.duran@amd.com]
> > Sent: Thursday, August 11, 2016 11:23 PM
> > To: edk2-devel@lists.01.org
> > Cc: Zhu, Yonghong <yonghong.zhu@intel.com>; Gao, Liming
> > <liming.gao@intel.com>; Leo Duran <leo.duran@amd.com>
> > Subject: [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c
> >
> > Account for rebase of FV section containing VTF file on IA32/IA64.
> > This supports cases where the reset vector may not be set at 0xFFFFFFF0.
> >
> > For example, FV section defined as:
> > [FV.FvSecPei]
> >   FvBaseAddress = $(FV_BOOT_BASE)
> >   FvForceRebase = TRUE
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Leo Duran <leo.duran@amd.com>
> > ---
> >  BaseTools/Source/C/GenFv/GenFvInternalLib.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> > b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> > index 7c839e2..8c2827b 100644
> > --- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> > +++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> > @@ -2770,11 +2770,13 @@ Returns:
> >        //
> >        // Update reset vector (SALE_ENTRY for IPF)
> >        // Now for IA32 and IA64 platform, the fv which has bsf file must have
> the
> > -      // EndAddress of 0xFFFFFFFF. Thus, only this type fv needs to update
> the
> > -      // reset vector. If the PEI Core is found, the VTF file will probably get
> > -      // corrupted by updating the entry point.
> > +      // EndAddress of 0xFFFFFFFF (unless the section was rebased).
> > +      // Thus, only this type fv needs to update the  reset vector.
> > +      // If the PEI Core is found, the VTF file will probably get
> > +      // corrupted by updating the entry point.
> >        //
> > -      if ((mFvDataInfo.BaseAddress + mFvDataInfo.Size) ==
> > FV_IMAGES_TOP_ADDRESS) {
> > +      if ((mFvDataInfo.ForceRebase == 1) ||
> > +        (mFvDataInfo.BaseAddress + mFvDataInfo.Size) ==
> > FV_IMAGES_TOP_ADDRESS) {
> >          Status = UpdateResetVector (&FvImageMemoryFile, &mFvDataInfo,
> > VtfFileImage);
> >          if (EFI_ERROR(Status)) {
> >            Error (NULL, 0, 3000, "Invalid", "Could not update the
> > reset vector.");
> > --
> > 1.9.1

Re: [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c

[Ard Biesheuvel]

On 14 August 2016 at 17:05, Duran, Leo <leo.duran@amd.com> wrote:
>
>> -----Original Message-----
>> From: Gao, Liming [mailto:liming.gao@intel.com]
>> Sent: Friday, August 12, 2016 3:32 AM
>> To: Duran, Leo <leo.duran@amd.com>; edk2-devel@lists.01.org
>> Cc: Zhu, Yonghong <yonghong.zhu@intel.com>
>> Subject: RE: [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c
>>
>> Duran:
>>   Reusing FvForceRebase flag is a good idea to resolve this issue. I agree your
>> change. Reviewed-by: Liming Gao <liming.gao@intel.com>
>>
> [Duran, Leo]
> Excellent, thanks!
>
> Please advise on "next steps":
> 1) Do we need a reply from Yonghong Zhu?
> 2) When would it be reasonable to expect integration of this code into mainline?
>

Hi all,

I pushed this (with Liming's R-b) as

adb6ac256338 BaseTools/GenFv: Account for rebase of FV section
containing VTF file

Thanks,
Ard.

Re: [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c

[Zhu, Yonghong]

Leo,   Thanks for the Contribution. It is good.
Ard,    Thanks for help to push the patch.

Best Regards,
Zhu Yonghong

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] 
Sent: Monday, August 15, 2016 3:25 AM
To: Duran, Leo <leo.duran@amd.com>
Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c

On 14 August 2016 at 17:05, Duran, Leo <leo.duran@amd.com> wrote:
>
>> -----Original Message-----
>> From: Gao, Liming [mailto:liming.gao@intel.com]
>> Sent: Friday, August 12, 2016 3:32 AM
>> To: Duran, Leo <leo.duran@amd.com>; edk2-devel@lists.01.org
>> Cc: Zhu, Yonghong <yonghong.zhu@intel.com>
>> Subject: RE: [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c
>>
>> Duran:
>>   Reusing FvForceRebase flag is a good idea to resolve this issue. I 
>> agree your change. Reviewed-by: Liming Gao <liming.gao@intel.com>
>>
> [Duran, Leo]
> Excellent, thanks!
>
> Please advise on "next steps":
> 1) Do we need a reply from Yonghong Zhu?
> 2) When would it be reasonable to expect integration of this code into mainline?
>

Hi all,

I pushed this (with Liming's R-b) as

adb6ac256338 BaseTools/GenFv: Account for rebase of FV section containing VTF file

Thanks,
Ard.

Re: [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c

[Duran, Leo]

> -----Original Message-----
> From: Zhu, Yonghong [mailto:yonghong.zhu@intel.com]
> Sent: Sunday, August 14, 2016 7:16 PM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Duran, Leo
> <leo.duran@amd.com>
> Cc: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org; Zhu,
> Yonghong <yonghong.zhu@intel.com>
> Subject: RE: [edk2] [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c
> 
> Leo,   Thanks for the Contribution. It is good.
> Ard,    Thanks for help to push the patch.
> 
> Best Regards,
> Zhu Yonghong
[Duran, Leo] 
Thank you Zhu and Ard.

BTW, I assume the executable 'auto-magically' gets built from source?
(I only submitted  a patch for the source... Was that sufficient?)

Thanks,
Leo.

> 
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Monday, August 15, 2016 3:25 AM
> To: Duran, Leo <leo.duran@amd.com>
> Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong
> <yonghong.zhu@intel.com>; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c
> 
> On 14 August 2016 at 17:05, Duran, Leo <leo.duran@amd.com> wrote:
> >
> >> -----Original Message-----
> >> From: Gao, Liming [mailto:liming.gao@intel.com]
> >> Sent: Friday, August 12, 2016 3:32 AM
> >> To: Duran, Leo <leo.duran@amd.com>; edk2-devel@lists.01.org
> >> Cc: Zhu, Yonghong <yonghong.zhu@intel.com>
> >> Subject: RE: [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c
> >>
> >> Duran:
> >>   Reusing FvForceRebase flag is a good idea to resolve this issue. I
> >> agree your change. Reviewed-by: Liming Gao <liming.gao@intel.com>
> >>
> > [Duran, Leo]
> > Excellent, thanks!
> >
> > Please advise on "next steps":
> > 1) Do we need a reply from Yonghong Zhu?
> > 2) When would it be reasonable to expect integration of this code into
> mainline?
> >
> 
> Hi all,
> 
> I pushed this (with Liming's R-b) as
> 
> adb6ac256338 BaseTools/GenFv: Account for rebase of FV section containing
> VTF file
> 
> Thanks,
> Ard.

Re: [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c

[Gao, Liming]

Yes. BaseTools Win32 Binary will auto be updated based on BaseTools source.

Thanks
Liming
From: Duran, Leo [mailto:leo.duran@amd.com]
Sent: Monday, August 15, 2016 9:13 PM
To: Zhu, Yonghong <yonghong.zhu@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c



> -----Original Message-----
> From: Zhu, Yonghong [mailto:yonghong.zhu@intel.com]
> Sent: Sunday, August 14, 2016 7:16 PM
> To: Ard Biesheuvel ; Duran, Leo
>
> Cc: Gao, Liming ; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Zhu,
> Yonghong
> Subject: RE: [edk2] [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c
>
> Leo, Thanks for the Contribution. It is good.
> Ard, Thanks for help to push the patch.
>
> Best Regards,
> Zhu Yonghong
[Duran, Leo]
Thank you Zhu and Ard.

BTW, I assume the executable 'auto-magically' gets built from source?
(I only submitted a patch for the source... Was that sufficient?)

Thanks,
Leo.

>
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Monday, August 15, 2016 3:25 AM
> To: Duran, Leo
> Cc: Gao, Liming ; Zhu, Yonghong
> ; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Subject: Re: [edk2] [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c
>
> On 14 August 2016 at 17:05, Duran, Leo wrote:
> >
> >> -----Original Message-----
> >> From: Gao, Liming [mailto:liming.gao@intel.com]
> >> Sent: Friday, August 12, 2016 3:32 AM
> >> To: Duran, Leo ; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> >> Cc: Zhu, Yonghong
> >> Subject: RE: [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c
> >>
> >> Duran:
> >> Reusing FvForceRebase flag is a good idea to resolve this issue. I
> >> agree your change. Reviewed-by: Liming Gao
> >>
> > [Duran, Leo]
> > Excellent, thanks!
> >
> > Please advise on "next steps":
> > 1) Do we need a reply from Yonghong Zhu?
> > 2) When would it be reasonable to expect integration of this code into
> mainline?
> >
>
> Hi all,
>
> I pushed this (with Liming's R-b) as
>
> adb6ac256338 BaseTools/GenFv: Account for rebase of FV section containing
> VTF file
>
> Thanks,
> Ard.