On 07/21/16 04:07, Thomas Palmer wrote:
> This change keeps the .hii sections in GCC built binaries.  Please
> refer to email thread titled "[edk2] HII
> gEfiHiiPackageListProtocolGuid problem with  GCC48 (VS2012x86 works)"
> 
> As this is the first time I've ever touched a GCC linker script,
> please feel free to send feedback
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Thomas Palmer <thomas.pal...@hpe.com>
> ---
>  BaseTools/Scripts/GccBase.lds | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/BaseTools/Scripts/GccBase.lds b/BaseTools/Scripts/GccBase.lds
> index 32310bc..7e4cdde 100644
> --- a/BaseTools/Scripts/GccBase.lds
> +++ b/BaseTools/Scripts/GccBase.lds
> @@ -4,6 +4,7 @@
>  
>    Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR>
>    Copyright (c) 2015, Linaro Ltd. All rights reserved.<BR>
> +  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>  
>    This program and the accompanying materials are licensed and made 
> available under
>    the terms and conditions of the BSD License that accompanies this 
> distribution.
> @@ -57,6 +58,10 @@ SECTIONS {
>      *(.rela .rela.*)
>    }
>  
> +  .hii : ALIGN(CONSTANT(COMMONPAGESIZE)) {
> +    KEEP (*(.hii))
> +  }
> +
>    /DISCARD/ : {
>      *(.note.GNU-stack)
>      *(.gnu_debuglink)
> 

Assuming the patch works out in practice -- I asked Bruce and Heyi to
test it --, I'd like to request the following changes to the commit message:

(1) Please drop the "feel free to send feedback" paragraph from the
commit message. Such comments are valid, but they don't belong to the
patch itself. Alternatives:

(1a) Use "git notes edit" and "git format-patch --notes" (search
<https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers>
for the word "notes").

(1b) send a cover letter (0/1) and add the comment there

(1c) send a followup and ask for the comments there

(2) For the clueless like me, please reference the LoadImage() boot
service from the UEFI specification:

    Once the image is loaded, LoadImage() installs
    EFI_HII_PACKAGE_LIST_PROTOCOL on the handle if the image contains a
    custom PE/COFF resource with the type 'HII'. The protocol's
    interface pointer points to the HII package list which is contained
    in the resource's data.

(3) Again for the clueless like me, please spell out that the edk2 build
tools produce this PE/COFF section when

  UEFI_HII_RESOURCE_SECTION = TRUE

is present in the INF file. (This is documented in the INF spec.)

And that the problem is that the generated .hii section is not being
preserved by the linker script.

(4) Referencing mailing list threads is always useful; using URLs would
improve the commit message:

http://thread.gmane.org/gmane.comp.bios.tianocore.devel/13438
http://thread.gmane.org/gmane.comp.bios.tianocore.devel/14899


I had to hunt down these bits of information from specs and emails; it
would be nice to have them in the commit that connects the dots for the
GCC toolchain family at long last.

Thanks!
Laszlo

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to