From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout01.posteo.de (mout01.posteo.de [185.67.36.65]) by mx.groups.io with SMTP id smtpd.web11.31637.1629989242684571706 for ; Thu, 26 Aug 2021 07:47:23 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@posteo.de header.s=2017 header.b=HtJCxEkD; spf=pass (domain: posteo.de, ip: 185.67.36.65, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [89.146.220.130]) by mout01.posteo.de (Postfix) with ESMTPS id 506BF240028 for ; Thu, 26 Aug 2021 16:47:19 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1629989240; bh=S/lwmD7cuHLdFSXW+NccVjMbUPGCVqfJsYR282vJavs=; h=Subject:To:Cc:From:Date:From; b=HtJCxEkDlsR7siDldQBXtyvR2ZEi1JdqTvPiD475uwJpqTWa++P65OkhTeu0BPYYx p9ZZdO+h3mEWi3u8BOH2l09qVxglI1HlKrkg9VdQFHNqmtVu+iCWJqyzaQhYDCQFbB 6XQQmnafSQavQ1/sisxgvzQA7B1DfxGDPuIvRy5Bwj082dPj18laI4meJKKSWGsN5d gKsaTeHoF4l9tckBBE+MMLU0ZlxjZ6qXL9jXXDebzAVv5RDXxZDN1Q0RLMlTZziJDU fFpVpkl224mUmvjl8NWjlBX2W+WaVRiYEbFBFTdS/PyffshY27jyBJLHI2b1GaK5GF Upln8FcUibIAg== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4GwQfs1yz5z6tmf; Thu, 26 Aug 2021 16:47:17 +0200 (CEST) Subject: Re: [edk2-devel] [RFC] Expose HII package list via C variables To: devel@edk2.groups.io, michael.d.kinney@intel.com Cc: Andrew Fish , "leif@nuviainc.com" , "Ni, Ray" , "Gao, Zhichao" , "Wang, Jian J" , "Wu, Hao A" , "Bi, Dandan" , "Dong, Eric" , Bret Barkelew , Vitaly Cheptsov References: <56820ad5-5fb8-f93d-60f5-15fa6e9d347d@posteo.de> From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= Message-ID: <478d1f05-d048-4de7-47ec-6cfabe60f762@posteo.de> Date: Thu, 26 Aug 2021 14:47:14 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: quoted-printable Hey Mike, Yes, I am aware. My goal for that would be to declare support as=20 optional to support drivers up until UEFI vX (whatever would be the spec=20 to endorse drivers to move away from the PE/COFF section model), much=20 like EFI 1.10 backwards-compatibility worked with UEFI 2.x, and have=20 some enabled-by-default FixedPcd for HII PE/COFF support. Project=20 Amaranth at ISP RAS for example would disable support for this right=20 away, as there is no urgent dependency on any external software to=20 backwards-support. Production consumer platforms would drop support=20 eventually in the far future, just like with all the EFI 1.10 specifics.=20 Possibly a DEBUG_WARN could be issued whenever such a section is=20 encountered, to help identify modules relying on this model. Considering the nature of the usage right now, i.e. Shell manual=20 queries, I sure hope there aren't actually any dependencies on this. The=20 new PE/COFF loader is yet to undergo extensive real-world platform=20 testing on full-UEFI scale (it is already used to load OS loaders on a=20 wider scale, but not e.g. Option ROMs), and so are the UEFI spec changes=20 that are to be proposed for hardening the parser, so I hope this can=20 just be squeezed into both. If you can give a sort of general vote=20 whether this change can be endorsed or not, ignoring any real-world=20 dependencies for the moment, I will clean up the changes, integrate them=20 into any upcoming PE/COFF loader testing, and provide findings whenever=20 they arrive. Best regards, Marvin On 26/08/2021 16:37, Michael D Kinney wrote: > 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@edk2.groups.io On Behalf Of Marvin H= =C3=A4user >> Sent: Thursday, August 26, 2021 1:51 AM >> To: devel@edk2.groups.io; Kinney, Michael D >> Cc: devel@edk2.groups.io; Andrew Fish ; leif@nuviainc.c= om; Ni, Ray ; Gao, Zhichao >> ; Wang, Jian J ; Wu, Hao A= ; Bi, Dandan >> ; Dong, Eric ; Bret Barkelew <= Bret.Barkelew@microsoft.com>; Vitaly Cheptsov >> >> 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/7b4a99be8a39c12d3a7fc4b8db9f0eab4= ac688d5/MdeModulePkg/Application/BootManagerMenuAp >> p/BootManagerMenu.c#L929-L934 >> >> [2] >> https://github.com/tianocore/edk2/blob/7b4a99be8a39c12d3a7fc4b8db9f0eab4= ac688d5/MdeModulePkg/Logo/LogoDxe.inf#L23 >> >> [3] >> https://github.com/tianocore/edk2/blob/7b4a99be8a39c12d3a7fc4b8db9f0eab4= ac688d5/ShellPkg/Application/Shell/ShellManParser. >> c#L646-L671 >> >> [4] >> https://github.com/mhaeuser/edk2/blob/ue_poc/MdePkg/Include/IndustryStan= dard/UeImage.h >> >> 26.08.2021 00:34:12 Michael D Kinney : >> >>> 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_i= nf_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 On Behalf Of Marvin = H=C3=A4user >>>> Sent: Wednesday, August 25, 2021 2:21 PM >>>> To: devel@edk2.groups.io >>>> Cc: Andrew Fish ; leif@nuviainc.com; Kinney, Michael = D ; Ni, Ray >>>> ; Gao, Zhichao ; Wang, Jian J= ; Wu, Hao A >>>> ; Bi, Dandan ; Dong, Eric ; Bret Barkelew >>>> ; Vitaly Cheptsov >>>> 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 sectio= n >>>> [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 h= as >>>> the following advantages: >>>> >>>> 1. Fixes BZ (incl. future toolchains): >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D557 >>>> 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 awa= y >>>> 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 cod= e >>>> (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()" >>>> >>>> >>>> >>>> >>> >>> >>> >> >> >> > > >=20 > >