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/7b4a99be8a39c12d3a7fc4b8db9f0eab4ac688d5/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenu.c#L929-L934
[2]
https://github.com/tianocore/edk2/blob/7b4a99be8a39c12d3a7fc4b8db9f0eab4ac688d5/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/IndustryStandard/UeImage.h
26.08.2021 00:34:12 Michael D Kinney <michael.d.kin...@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_ii_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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Marvin Häuser
Sent: Wednesday, August 25, 2021 2:21 PM
To: devel@edk2.groups.io
Cc: Andrew Fish <af...@apple.com>; l...@nuviainc.com; Kinney, Michael D
<michael.d.kin...@intel.com>; Ni, Ray
<ray...@intel.com>; Gao, Zhichao <zhichao....@intel.com>; Wang, Jian J
<jian.j.w...@intel.com>; Wu, Hao A
<hao.a...@intel.com>; Bi, Dandan <dandan...@intel.com>; Dong, Eric
<eric.d...@intel.com>; Bret Barkelew
<bret.barke...@microsoft.com>; Vitaly Cheptsov <vit9...@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 (#79837): https://edk2.groups.io/g/devel/message/79837
Mute This Topic: https://groups.io/mt/85147044/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
