[edk2-devel] [RFC] Expose HII package list via C variables

Thu Aug 26 19:50:59 UTC 2021

Mike,

That reminds me that my patch to fix this for Xcode is still in limbo. 

Thanks,

Andrew Fish

> On Aug 26, 2021, at 9:44 AM, Kinney, Michael D <michael.d.kinney at intel.com> wrote:
> 
> Marvin,
> 
> The main reason most components in the EDK II repos continue to use the variables is 
> because there are incomplete tools to generate PE/COFF resource sections for all
> the OS/compiler combinations that EDK II supports.
> 
> The preference would be to use PE/COFF resource sections and we would have converted
> all components to that style long ago if the tools existed to be aligned with the
> UEFI Specification instead of an EDK II specific implementation feature.
> 
> Also, it is not a good idea to only look at the open source EDK II repos to
> understand how a feature is used.  There are many downstream consumers of the
> EDK II repos.
> 
> The UEFI Driver Writer's Guide *only* documents the PE/COFF resource section
> method.
> 
> https://tianocore-docs.github.io/edk2-UefiDriverWritersGuide/draft/7_driver_entry_point/74_adding_hii_packages_feature.html#74-adding-hii-packages-feature <https://tianocore-docs.github.io/edk2-UefiDriverWritersGuide/draft/7_driver_entry_point/74_adding_hii_packages_feature.html#74-adding-hii-packages-feature>
> 
> Best regards,
> 
> Mike
> 
>> -----Original Message-----
>> From: Marvin Häuser <mhaeuser at posteo.de <mailto:mhaeuser at posteo.de>>
>> Sent: Thursday, August 26, 2021 9:07 AM
>> To: tim.lewis at insyde.com <mailto:tim.lewis at insyde.com>; devel at edk2.groups.io <mailto:devel at edk2.groups.io>; Kinney, Michael D <michael.d.kinney at intel.com <mailto:michael.d.kinney at intel.com>>
>> Cc: 'Andrew Fish' <afish at apple.com <mailto:afish at apple.com>>; leif at nuviainc.com <mailto:leif at nuviainc.com>; Ni, Ray <ray.ni at intel.com <mailto:ray.ni at intel.com>>; Gao, Zhichao <zhichao.gao at intel.com <mailto:zhichao.gao at intel.com>>;
>> Wang, Jian J <jian.j.wang at intel.com <mailto:jian.j.wang at intel.com>>; Wu, Hao A <hao.a.wu at intel.com <mailto:hao.a.wu at intel.com>>; Bi, Dandan <dandan.bi at intel.com <mailto:dandan.bi at intel.com>>; Dong, Eric
>> <eric.dong at intel.com <mailto:eric.dong at intel.com>>; 'Bret Barkelew' <Bret.Barkelew at microsoft.com <mailto:Bret.Barkelew at microsoft.com>>; 'Vitaly Cheptsov' <vit9696 at protonmail.com <mailto:vit9696 at protonmail.com>>
>> Subject: Re: [edk2-devel] [RFC] Expose HII package list via C variables
>> 
>> Hey Tim,
>> 
>> Interesting, do you happen to have some tool or code that performs such
>> edits at hand to take a look at? Seems like most modules already use
>> variables and thus cannot be modified in such a way even right now?
>> 
>> That's the kind of information I am looking for, thanks a lot!
>> 
>> Best regards,
>> Marvin
>> 
>> On 26/08/2021 18:01, tim.lewis at insyde.com wrote:
>>> Hi Marvin --
>>> 
>>> I would like to add some historical perspective on this. One of the design requirements back when HII was first
>> introduced into the UEFI specification after Intel's initial contribution was that of binary editability. In order to be
>> able to reliably find, extract and then re-insert the HII data into the binary, it needed to be discoverable and not
>> affect the offsets of the code and data in the binary.
>>> 
>>> While it was possible to put some sort of signature in the read-only data sections of the binary and leave padding for
>> possible future edits, it was felt that using a PE/COFF section similar to the resource sections was better. Resource
>> sections are commonly in use for PE/COFF files (in Windows) and the similar idea existed in the then-extant Mac binary
>> format (resource fork?).
>>> 
>>> Thanks,
>>> 
>>> Tim Lewis
>>> CTO, Insyde Software
>>> www.insyde.com
>>> 
>>> -----Original Message-----
>>> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Michael D Kinney
>>> Sent: Thursday, August 26, 2021 7:37 AM
>>> To: devel at edk2.groups.io; mhaeuser at posteo.de; Kinney, Michael D <michael.d.kinney at intel.com>
>>> Cc: Andrew Fish <afish at apple.com>; leif at nuviainc.com; Ni, Ray <ray.ni at intel.com>; Gao, Zhichao <zhichao.gao at intel.com>;
>> Wang, Jian J <jian.j.wang at intel.com>; Wu, Hao A <hao.a.wu at intel.com>; Bi, Dandan <dandan.bi at intel.com>; Dong, Eric
>> <eric.dong at intel.com>; Bret Barkelew <Bret.Barkelew at microsoft.com>; Vitaly Cheptsov <vit9696 at protonmail.com>
>>> Subject: Re: [edk2-devel] [RFC] Expose HII package list via C variables
>>> 
>>> Marvin,
>>> 
>>> One constraint in this discussion is that the HII content in a PE/COFF resource section is defined in the UEFI
>> Specification, Which means UEFI Apps and UEFI Drivers from media and option ROMs that are not part of the system FW image
>> are allowed to use this feature,  This means the system FW PE/COFF loader must support loading HII content from this
>> PE/COFF resource section to be UEFI conformant.  So we cannot remove this feature from the PE/COFF loader without changes
>> to the UEFI Specification.  Even if changes to the UEFI Specification we made, we would have to continue to support this
>> feature for backward compatibility with existing UEFI Apps/Drivers that may be using this feature.
>>> 
>>> Thanks,
>>> 
>>> Mike
>>> 
>>>> -----Original Message-----
>>>> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Marvin
>>>> Häuser
>>>> Sent: Thursday, August 26, 2021 1:51 AM
>>>> To: devel at edk2.groups.io; Kinney, Michael D
>>>> <michael.d.kinney at intel.com>
>>>> Cc: devel at edk2.groups.io; Andrew Fish <afish at apple.com>;
>>>> leif at nuviainc.com; Ni, Ray <ray.ni at intel.com>; Gao, Zhichao
>>>> <zhichao.gao at intel.com>; Wang, Jian J <jian.j.wang at intel.com>; Wu, Hao
>>>> A <hao.a.wu at intel.com>; Bi, Dandan <dandan.bi at intel.com>; Dong, Eric
>>>> <eric.dong at intel.com>; Bret Barkelew <Bret.Barkelew at microsoft.com>;
>>>> Vitaly Cheptsov <vit9696 at protonmail.com>
>>>> Subject: Re: [edk2-devel] [RFC] Expose HII package list via C
>>>> variables
>>>> 
>>>> Hey Mike,
>>>> 
>>>> Thanks for your reply!
>>>> 
>>>> Well, this switch is not well-documented. Looking now at both the
>>>> generation code and the emitted code, it does not generate a package
>>>> list like my code does, but separate data variables (strings and
>>>> images) that cannot easily be passed to HiiDatabase as-is. However
>>>> apparently there are drivers that use this functionality successfully
>>>> by composing the package list at runtime [1].
>>>> 
>>>> Looking with this information now at the pattern of using HII C
>>>> variables (which I did not know existed before) vs the PE/COFF HII
>>>> section, most that use latter are Shell applications, which I guess
>>>> means the section has actually been introduced to resolve D.? There
>>>> are exceptions such as LogoDxe [2], which use the PE/COFF section while D.
>>>> is not a problem, hence I got confused, sorry. I think these modules
>>>> should be updated in any case. Do you agree?
>>>> 
>>>> So, for modules that use C variables already, my patch would save some
>>>> runtime generation code and dynamic memory allocation for the HII
>>>> package list. This was not my goal (as I said, I didn't realise HII C
>>>> variables already were a thing in the first place), but the changes
>>>> are small enough that it might be worth considering anyway, in my opinion.
>>>> If a HII package list is generated for both Shell and non-Shell apps,
>>>> this also means code paths can be unified. For example, there could be
>>>> a library class with constructor and destructor to add/remove packages
>>>> from the HII database for all modules that use such, Shell or not. For
>>>> BaseTools it means that there is no real need for separate Python and
>>>> C paths as ideally they just generate the exact same data.
>>>> 
>>>> Now to D., the only usage for this seems to be that Shell can locate
>>>> the help text in the executable without executing it, yet it is fully
>>>> loaded anyway [3]. To be honest, I find it hard to justify loading an
>>>> executable (PE/COFF loading, memory permission application, the full
>>>> process) to retrieve a help text and then unloading it again,
>>>> especially with the HII code being on a core dispatcher level. 1. to
>>>> 7. still hold true in my opinion. Was there any discussion I could
>>>> read through why Shell apps cannot simply support a "--help" or "-?"
>>>> command and output the string themselves? Pushing the burden to the
>>>> Shell apps does preserve the "drawback" that a full loading is
>>>> required (which honestly does not matter for a debugging application
>>>> like Shell), however it does relieve the burden of PE/COFF HII parsing
>>>> from the core dispatcher (which matters a lot in my opinion to keep
>>>> the core simple). It would simply be a normal Shell app execution as
>>>> any other however. If someone wants to avoid the PE/COFF burden
>>>> altogether, they can still provide a .man file.
>>>> 
>>>> As for my points 6. and 7., maybe I should provide some context. Due
>>>> to many issues with TE files, platforms started abandoning them and
>>>> returned to PE/COFF Images. I think a big reason for this is that TE
>>>> is not really a sound and complete format, but a stripped version of
>>>> PE/COFF with none of the necessary fixups applied. I'm currently
>>>> sketching a possible alternative [4], and I would really like to not
>>>> having to specify a HII section type, while still preserving
>>>> compatibility with all of the UEFI Image types and use-cases [4].
>>>> 
>>>> Thanks again!
>>>> 
>>>> Best regards,
>>>> Marvin
>>>> 
>>>> 
>>>> [1]
>>>> https://github.com/tianocore/edk2/blob/7b4a99be8a39c12d3a7fc4b8db9f0ea
>>>> b4ac688d5/MdeModulePkg/Application/BootManagerMenuAp
>>>> p/BootManagerMenu.c#L929-L934
>>>> 
>>>> [2]
>>>> https://github.com/tianocore/edk2/blob/7b4a99be8a39c12d3a7fc4b8db9f0ea
>>>> b4ac688d5/MdeModulePkg/Logo/LogoDxe.inf#L23
>>>> 
>>>> [3]
>>>> 
>> https://github.com/tianocore/edk2/blob/7b4a99be8a39c12d3a7fc4b8db9f0eab4ac688d5/ShellPkg/Application/Shell/ShellManParser.
>>>> c#L646-L671
>>>> 
>>>> [4]
>>>> https://github.com/mhaeuser/edk2/blob/ue_poc/MdePkg/Include/IndustrySt
>>>> andard/UeImage.h
>>>> 
>>>> 26.08.2021 00:34:12 Michael D Kinney <michael.d.kinney at intel.com>:
>>>> 
>>>>> Hi Marvin,
>>>>> 
>>>>> I think this feature is already there and supported.
>>>>> 
>>>>> HII can either be in a global variable or in a PE/COFF resource section.
>>>>> The default is a global variable because HII was implemented before
>>>>> the PE/COFF resource section feature was added to the UEFI Specification.
>>>>> 
>>>>> There is an INF [Defines] section statement to enable the PE/COFF
>>>>> section. See UefiHiiResource in the following link.
>>>>> 
>>>>> https://tianocore-docs.github.io/edk2-InfSpecification/draft/3_edk_i
>>>>> i_inf_file_format/34_[defines]_section.html#34-
>>>> defines-section
>>>>> How is your proposal different than this existing capability?
>>>>> 
>>>>> Thanks,
>>>>> 
>>>>> Mike
>>>>> 
>>>>>> -----Original Message-----
>>>>>> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
>>>>>> Marvin Häuser
>>>>>> Sent: Wednesday, August 25, 2021 2:21 PM
>>>>>> To: devel at edk2.groups.io
>>>>>> Cc: Andrew Fish <afish at apple.com>; leif at nuviainc.com; Kinney,
>>>>>> Michael D <michael.d.kinney at intel.com>; Ni, Ray <ray.ni at intel.com>;
>>>>>> Gao, Zhichao <zhichao.gao at intel.com>; Wang, Jian J
>>>>>> <jian.j.wang at intel.com>; Wu, Hao A <hao.a.wu at intel.com>; Bi, Dandan
>>>>>> <dandan.bi at intel.com>; Dong, Eric <eric.dong at intel.com>; Bret
>>>>>> Barkelew <Bret.Barkelew at microsoft.com>; Vitaly Cheptsov
>>>>>> <vit9696 at protonmail.com>
>>>>>> Subject: [edk2-devel] [RFC] Expose HII package list via C variables
>>>>>> 
>>>>>> Good day everyone,
>>>>>> 
>>>>>> Currently, the HII package list is stored in a PE/COFF resource
>>>>>> section [1]. I propose to store it in a C variable (byte array with
>>>>>> a pointer to it and its size exposed) instead. DxeCore would have a
>>>>>> guard to toggle the deprecated support for the automatic protocol
>>>>>> installation. This has the following advantages:
>>>>>> 
>>>>>> 1. Fixes BZ (incl. future toolchains):
>>>>>> https://bugzilla.tianocore.org/show_bug.cgi?id=557
>>>>>> 2. Universal method across all toolchains and output file formats
>>>>>> 3. Saves error-prone parsing work 4. Saves protocol install/locate
>>>>>> work, the data is available right away 5. The omission of a
>>>>>> dedicated section can save space 6. Terse file formats can support
>>>>>> this and remain terse :) 7. Removes a dependency on the PE/COFF
>>>>>> format specifically
>>>>>> 
>>>>>> A *very rough* PoC diff can be found here:
>>>>>> https://github.com/mhaeuser/edk2/compare/master...wip_hii_cvar
>>>>>> If the feedback is positive, I will clean it up of course. OVMF
>>>>>> boots with everything working fine.
>>>>>> 
>>>>>> I'd explicitly like feedback on the following:
>>>>>> A. Is this an acceptable solution to BZ 557 (Andrew?)?
>>>>>> B. Is this an acceptable solution for the "HII workflow" (MdeModule
>>>>>> maintainers?)?
>>>>>> C. Is it acceptable to make support UEFI-side support for the old
>>>>>> mechanism optional (Stewards?)?
>>>>>> D. Can an acceptable alternative be found for the removed ShellPkg
>>>>>> code (Shell maintainers?)?
>>>>>> 
>>>>>> As you can see the BaseTools part also is rough, but that is more a
>>>>>> question of "how" rather than "whether", so I'll postpone asking about it.
>>>>>> 
>>>>>> Thanks for your time and feedback!
>>>>>> 
>>>>>> Best regards,
>>>>>> Marvin
>>>>>> 
>>>>>> 
>>>>>> [1] "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'."
>>>>>> - UEFI 2.9, 7.4, "EFI_BOOT_SERVICES.LoadImage()"
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>> 
>>>> 
>>>> 
>>> 
>>> 
>>> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79864): https://edk2.groups.io/g/devel/message/79864
Mute This Topic: https://groups.io/mt/85147044/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20210826/5f5ed69d/attachment.htm>