After internal discussion, we agreed to add a PCD to control this feature. Also we will refine the design, first export data at ReadyToBoot event and from this point to hook all database change event to update the export data. Patch will be send out later. Thanks all for your contribution.
Thanks, Eric > -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of > El-Haj-Mahmoud, Samer > Sent: Wednesday, February 24, 2016 2:06 AM > To: Brian J. Johnson; Dong, Eric; Andrew Fish > Cc: Hale, Robert P; Rothman, Michael A; Gao, Liming; Bi, Dandan; Kinney, > Michael D; [email protected] > Subject: Re: [edk2] [patch] MdeModulePkg: Make HII configuration settings > available to OS runtime > > Even with a single export @ ReadyToBoot, I still think we need a PCD to > allow implementations to disable the feature altogether if they > decide to. > > > > -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of Brian > J. Johnson > Sent: Tuesday, February 23, 2016 11:52 AM > To: Dong, Eric <[email protected]>; Andrew Fish <[email protected]> > Cc: Hale, Robert P <[email protected]>; Rothman, Michael A > <[email protected]>; Gao, Liming > <[email protected]>; Bi, Dandan <[email protected]>; Kinney, Michael D > <[email protected]>; [email protected] > Subject: Re: [edk2] [patch] MdeModulePkg: Make HII configuration settings > available to OS runtime > > On 02/22/2016 09:00 PM, Dong, Eric wrote: > >>>> > >>>> Hi All, > >>>> > >>>> Thanks for your comments. I add my explanation below: > >>>> > >>>> Only hook ReadyToBoot event is not enough. Different drivers may > >>>> hook this event and some may update string package in their > >> callback function. The order to call these callback functions is random, > >> so only hook this event may miss some changes. > >>>> > >> > >> Eric, > >> > >> Could you enable your HII exporting callbacks in a ReadyToBoot event? > >> Then you wouldn't needlessly pay the export cost each time a HII > >> element is added or changed as drivers are loaded. > > > > Brian, > > > > Do you mean we first export HII data at ReadyToBoot event. After it, we > > also hook all HII Database changes action (include add package/ > remove package/ update packages/ HiiSetString)? > > If only export HII data at ReadyToBoot event, it is not enough, because the > > HII data maybe changed later(Through add package/ remove > package ....)? > > > > Thanks, > > Eric > > Yes. Good summary. > > Thanks, > Brian > > >> > >>>> This feature is described in UEFI spec 2.6 chapter 31.2.11.1. It > >>>> required to export all HiiDatabae data and current configuration data. > >>> > >>> I don't see any language in that section that makes it required. It seems > >>> to just describe how to do it. > >>> > >> > >> Agreed. And section 31.2.1.10 says: > >> > >> ===== > >> When it is desired that the forms data be used in the presence of an > >> O/S, this specification describes a means by which to support this > >> capability. By being able to encapsulate the data and export it > >> through standard means such that an O/S agent (e.g. forms > >> browser/processor) can retrieve it, O/S- present usage models can be > >> made available for further value-add implementations. > >> ===== > >> > >> The wording implies that exporting is not always desired, and that > >> O/S-present usage of forms data is a value-add feature, which a > >> platform may choose to support or not. > >> > >>>> We add this feature because we strongly encourage the export of > >>>> config data to support manageability of platform config both for > >>>> pre- > >> OS and OS runtime operations. > >>>> > >>> > >>> Generally the UEFI spec and the edk2 don't try to force platform > >>> policy. For example exposing some configuration information could be > >> considered a security vulnerability on some platforms, so it should not be > >> forced on a platform. > >>> > >>> Thanks, > >>> > >>> Andrew Fish > >>> > >>>> Also we collect the performance data from our reference > >>>> platform(Detail see below). The boot performance is small and we > >>>> can > >> ignore it. This feature cost extra 869KB, but we think the size cost is > >> not care because current we use 16M size of bios flash. > >>>> > >>>> HiiDatabase driver data size cost(This feature add in HiiDatabase > >>>> driver): > >>>> Without this feature: > >>>> Used data size: 151KB > >>>> With this feature > >>>> Used data size: 1020 KB > >>>> PS: This extra size cost mainly depend on how much Hii packages in your > >>>> platform. > >>>> > >>>> Boot Time cost: > >>>> Without this feature: > >>>> Boot Time cost: 1st:3.353s 2nd: 3.035s > >>>> With this feature: > >>>> Boot Time cost: 1st: 3.125s 2nd: 3.126s > >>>> > >>>> Base on the above analysis, we provide this design and prefer to always > >>>> enable this feature instead of use a PCD to control it. > >>>> > >>>> Thanks, > >>>> Eric > >>>> > >> > >> My particular concern involves a simulator (far slower than hardware) > >> for an extremely large, complex server platform with a large amount > >> of HII data. On this platform, we end up disabling the largest but > >> least used setup screens when running on the simulator, simply to > >> avoid the time overhead of processing the HII data. This saves many > >> minutes of execution time. > >> > >>>>> -----Original Message----- > >>>>> From: El-Haj-Mahmoud, Samer [mailto:[email protected]] > >>>>> Sent: Wednesday, February 17, 2016 2:14 AM > >>>>> To: Andrew Fish; Brian J. Johnson > >>>>> Cc: Bi, Dandan; [email protected]; Dong, Eric; Gao, Liming > >>>>> Subject: RE: [edk2] [patch] MdeModulePkg: Make HII configuration > >>>>> settings available to OS runtime > >>>>> > >>>>> +1 > >>>>> > >>>>> I also would add there may be some HII strings that are hidden > >>>>> from user interfaces, and reflect settings for field service or > >>>>> troubleshooting, and that a mass export to the OS may expose these > settings to OS runtime code and possibly applications. > >>>>> > >>>>> > >>>>> > >>>>> -----Original Message----- > >>>>> From: edk2-devel [mailto:[email protected]] On > >>>>> Behalf Of Andrew Fish > >>>>> Sent: Tuesday, February 16, 2016 10:37 AM > >>>>> To: Brian J. Johnson <[email protected]> > >>>>> Cc: Dandan Bi <[email protected]>; [email protected]; Eric > >>>>> Dong <[email protected]>; Liming Gao > >> <[email protected]> > >>>>> Subject: Re: [edk2] [patch] MdeModulePkg: Make HII configuration > >>>>> settings available to OS runtime > >>>>> > >>>>> > >>>>>> On Feb 16, 2016, at 8:33 AM, Brian J. Johnson <[email protected]> wrote: > >>>>>> > >>>>>> On 02/16/2016 12:58 AM, Dandan Bi wrote: > >>>>>>> This feature is aimed to allow OS make use of the HII database > >>>>>>> during runtime. In this case, the contents of the HII Database > >>>>>>> is exported to a buffer. The pointer to the buffer is placed in > >>>>>>> the EFI System Configuration Table, where it can be retrieved by an > >>>>>>> OS application. > >>>>>>> > >>>>>>> Export the configuration data and contents of HiiDatabase when > >>>>>>> driver add package, update package and remove package. > >>>>>>> For string and image may also need to update the contents of > >>>>>>> HiiDatabase when NewString/SetString, NewImage/SetImage. > >>>>>>> > >>>>>>> Cc: Liming Gao <[email protected]> > >>>>>>> Cc: Eric Dong <[email protected]> > >>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0 > >>>>>>> Signed-off-by: Dandan Bi <[email protected]> ... > >>>>>> > >>>>>> Please make this behavior selectable via a PCD. HII operations > >>>>>> are very expensive, especially on simulators. I don't want to > >>>>>> pay the > >>>>> export time every time a package is added or string is changed. > >>>>> Also, platforms should be able to decide if they want to offer > >>>>> this > >> data to > >>>>> the OS. > >>>>>> > >>>>> > >>>>> +1 > >>>>> > >>>>> I would want to opt out to NOT take the memory away from the OS if the > >>>>> platform did not care about the feature. > >>>>> > >>>>> Thanks, > >>>>> > >>>>> Andrew Fish > >>>>> > >>>>>> Why not just export the data once, using a "ready to boot" event hook? > >>>>>> > >>>>>> Thanks, > >>>>>> -- > >>>>>> > >>>>>> Brian J. Johnson > >>>>>> > >>>>>> ----------------------------------------------------------------- > >>>>>> --- > >>>>>> > >>>>>> My statements are my own, are not authorized by SGI, and do not > >>>>>> necessarily represent SGI's positions. > >>>>>> _______________________________________________ > >>>>>> edk2-devel mailing list > >>>>>> [email protected] > >>>>>> https://lists.01.org/mailman/listinfo/edk2-devel > >>>>> > >>>>> _______________________________________________ > >>>>> edk2-devel mailing list > >>>>> [email protected] > >>>>> https://lists.01.org/mailman/listinfo/edk2-devel > >>>> > >>> > >> > >> > >> -- > >> > >> Brian > >> > >> -------------------------------------------------------------------- > >> > >> "If you want your dreams to come true: Wake Up!" > >> -- Jaime Roman's Radio > > _______________________________________________ > > edk2-devel mailing list > > [email protected] > > https://lists.01.org/mailman/listinfo/edk2-devel > > > > > -- > > Brian J. Johnson > > -------------------------------------------------------------------- > > My statements are my own, are not authorized by SGI, and do not > necessarily represent SGI's positions. > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

