A dynamic PCD may not work because the pointer to temporary memory will change after temporary ram migration.
About whether it is really worth it to allocate the HOB in this case, it is because we don't want to assume the potential call times of platform. Thanks. Star -----Original Message----- From: Andrew Fish [mailto:af...@apple.com] Sent: Saturday, June 29, 2013 3:56 AM To: Jordan Justen Cc: Zeng, Star; edk2-devel@lists.sourceforge.net Subject: Re: [edk2] [PATCH 1/2] MdeModulePkg and SecurityPkg: PEI variable does not robustly handle crashes during Reclaim(). On Jun 28, 2013, at 12:50 PM, Jordan Justen <jljus...@gmail.com> wrote: > On Fri, Jun 28, 2013 at 11:35 AM, Andrew Fish <af...@apple.com> wrote: >> On Jun 28, 2013, at 11:16 AM, Jordan Justen <jljus...@gmail.com> wrote: >>> On Thu, Jun 27, 2013 at 5:56 PM, Zeng, Star <star.z...@intel.com> wrote: >>>> The purpose is to reduce the burden to merge the header many times >>>> when there are many PeiGetVariable() or >>>> PeiGetNextVariableName() calls. >>> >>> About how many times would it need to be merged? 10? 1000? >>> >>> This boot path should only occur in rare circumstances, right? >>> >>> Is a HOB the best mechanism that we have for storing a global buffer >>> for a PEI driver? I guess I could also see a PCD being used to store >>> an allocated buffer pointer. Or, you could install a PPI to the >>> allocated buffer. But, all of these (including HOBs) seem to be >>> intended for different purposes than your usage model. >>> >> >> Jordan, >> >> The basic problem in early PEI is you are running from FLASH so >> global variables do not work. Most PPIs are registered from a >> template that is in ROM too, so it is hard to play the Protocol game >> where you hide state behind the This pointer with the CR macro. So a >> GUIDed HOB is a way to emulate a global variable in PEI. > > Yes, "a way". I think I pointed out 2 other possibilities. But, maybe > my point was, should we come up with an interface that wraps one of > these ways to do this? (And, thus makes the code more clear.) > > The purpose of HOBs are generally to 'hand off' information to DXE, so > this usage is a bit strange looking. > Back in the olden days there were not good ways to do this, so the HOB was one of the ways. As you point out it does leak up into DXE and maybe that is not a good thing. > Another question, is there a 'cheaper' way to accomplish it? For > example, the PEI core could store a single pointer for each "loaded" > image, and the image could ask the PEI core to set/get that pointer. > That would allow each image to have a global context. Well that is a PI spec change and I don't think it is really required. A dynamic PCD can do the same thing and is probably the most memory efficient solution. > > But, also my question to Star is, whether it is really worth it to > allocate the HOB in this case. I would lean towards no, unless the > performance was shown to be an issue. > > -Jordan > >>>> -----Original Message----- >>>> From: Jordan Justen [mailto:jljus...@gmail.com] >>>> Sent: Friday, June 28, 2013 6:11 AM >>>> To: Zeng, Star >>>> Cc: edk2-devel@lists.sourceforge.net >>>> Subject: Re: [edk2] [PATCH 1/2] MdeModulePkg and SecurityPkg: PEI variable >>>> does not robustly handle crashes during Reclaim(). >>>> >>>> In MdeModulePkg/Universal/Variable/Pei/Variable.c GetVariableHeader, why >>>> do you need to create the HOB? Could you have a VARIABLE_HEADER structure >>>> local variable (on the stack) in FindVariableEx, and merge the header into >>>> that variable instead? >>>> >>>> (Same question for >>>> SecurityPkg/VariableAuthenticated/Pei/Variable.c...) >>>> >>>> -Jordan >>>> >>>> On Mon, Jun 24, 2013 at 4:29 AM, Zeng, Star <star.z...@intel.com> wrote: >>>>> ========== >>>>> >>>>> MdeModulePkg: Variable drivers robustly handle crashes during Reclaim(). >>>>> >>>>> >>>>> >>>>> PEI variable implementation checks only the variable header >>>>> signature for validity. This does not seem robust if system crash >>>>> occurred during previous >>>>> Reclaim() operation. If the crash occurred while FTW was rewriting >>>>> the variable FV, the signature could be valid even though the rest >>>>> of the FV isn't valid. >>>>> >>>>> Solution: Add a FaultTolerantWritePei driver to check and provide >>>>> the FTW last write status, then PEI variable and early >>>>> phase(before FTW protocol >>>>> ready) of DXE variable can check the status and determine if all >>>>> or partial variable data has been backed up in spare block, and >>>>> then use the backed up data. >>>>> >>>>> >>>>> >>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>> >>>>> Signed-off-by: Star Zeng <star.z...@intel.com> >>>>> >>>>> ========== >>>>> >>>>> SecurityPkg: Variable drivers robustly handle crashes during Reclaim(). >>>>> >>>>> >>>>> >>>>> PEI variable implementation checks only the variable header >>>>> signature for validity. This does not seem robust if system crash >>>>> occurred during previous >>>>> Reclaim() operation. If the crash occurred while FTW was rewriting >>>>> the variable FV, the signature could be valid even though the rest >>>>> of the FV isn't valid. >>>>> >>>>> Solution: PEI variable and early phase(before FTW protocol ready) >>>>> of DXE variable can check the FTW last write status provided by >>>>> FaultTolerantWritePei and determine if all or partial variable >>>>> data has been backed up in spare block, and then use the backed up data. >>>>> >>>>> >>>>> >>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>> >>>>> Signed-off-by: Star Zeng <star.z...@intel.com> >>>>> >>>>> ========== >>>>> >>>>> >>>>> >>>>> [Impact] >>>>> >>>>> 1. If Platforms use VariablePei.inf now, they need to add >>>>> FaultTolerantWritePei.inf into the platform *.dsc and *.inf. >>>>> Because PEI variable will be updated to depend on the added >>>>> FaultTolerantWritePei. >>>>> >>>>> 2. The signature of working block header needs to be updated to >>>>> gWorkingBlockSignatureGuid because FTW write header and record >>>>> will be updated and exposed to support crossing archs. Low impact >>>>> to platform because FaultTolerantWrite DXE driver can help correct >>>>> or add the working block header at the first boot if platform >>>>> *.fdf uses the old signature GUID or no working block header init data. >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> Thanks! >>>>> >>>>> Star >>>>> >>>>> >>>>> ------------------------------------------------------------------ >>>>> ---- >>>>> -------- This SF.net email is sponsored by Windows: >>>>> >>>>> Build for Windows Store. >>>>> >>>>> http://p.sf.net/sfu/windows-dev2dev >>>>> _______________________________________________ >>>>> edk2-devel mailing list >>>>> edk2-devel@lists.sourceforge.net >>>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel >>>>> >>> >>> -------------------------------------------------------------------- >>> ---------- This SF.net email is sponsored by Windows: >>> >>> Build for Windows Store. >>> >>> http://p.sf.net/sfu/windows-dev2dev >>> _______________________________________________ >>> edk2-devel mailing list >>> edk2-devel@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/edk2-devel >> ------------------------------------------------------------------------------ This SF.net email is sponsored by Windows: Build for Windows Store. http://p.sf.net/sfu/windows-dev2dev _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
