On Jun 29, 2013, at 8:05 AM, "Zeng, Star" <star.z...@intel.com> wrote:
> A dynamic PCD may not work because the pointer to temporary memory will > change after temporary ram migration. > I forgot to mention the overhead of the memory migration, so thanks for including that. > 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. > I agree that slower FLASH/ROM speeds have a big impact on boot time. More modern chipsets have improved the NOR FLASH speed, but it is hard to get a general number on something like this. Thanks, Andrew > 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