On 25 June 2015 at 11:46, Star Zeng <star.z...@intel.com> wrote: > For your easy review, > the forked code is at g...@github.com:lzeng14/edk2.git branch > SeparateAuthVariableService. >
Hello Star, This series looks like an improvement, and thanks for making it available via GitHub, that is very helpful. I do have a concern about the general series, though. You are removing modules from the source tree before removing them from platform .DSCs and .FDFs. This potentially breaks git bisect, which is a vary valuable tool to track down regressions. For instance, if one would inadvertently end up between patches #12 and #19 while bisecting a regressions, the build would be broken for reasons unrelated to the regression. So it would be better to - add/update new modules/libraries - move the platforms to the new modules/libraries - remove the old modules/libraries > What to do: > 1. Move TpmMeasurementLib LibraryClass from SecurityPkg to MdeModulePkg. > 2. Implement a NULL TpmMeasurementLib library instance in MdeModulePkg. > 3. Move authenticated variable definition from AuthenticatedVariableFormat.h > to VariableFormat.h. I do share Laszlo's concern about gEfiVariableGuidg versus EfiAuthenticatedVariableGuid. Could you explain what goes on there? > 4. Merge VariableInfo in SecurityPkg to VariableInfo in MdeModulePkg. > 5. Merge from VariablePei in SecurityPkg to VariablePei in MdeModulePkg. > 6. Add AuthVariableLib LibraryClass definitions in MdeModulePkg. > 7. Implement a NULL AuthVariableLib library instance in MdeModulePkg. > 8. Implement AuthVariableLib library instance in SecurityPkg. > 9. Merge from Auth Variable driver in SecurityPkg to Variable drive in > MdeModulePkg. > 10. Update platform package to use the merged Variable driver. > > Why to do: > 1. Share code. > We are moving forward to separate auth variable service from Auth Variable > driver in SecurityPkg to AuthVariableLib. Then the AuthVariableLib could > benefit > and be used by different implementation of Auth Variable drivers. > 2. Remove code duplication and reduce maintenance effort. > 2.1. After auth variable service separated from Auth Variable driver in > SecurityPkg > to AuthVariableLib. The remaining code logic of Auth Variable driver in > SecurityPkg > will be almost same with Variable driver in MdeModulePkg. We are going to > merge them. > 2.2. The functionality of VariableInfo in SecurityPkg has covered VariableInfo > in MdeModulePkg. > 2.3. The code logic of VariablePei in SecurityPkg is same with VariablePei > in MdeModulePkg. > 3. TpmMeasurementLib is consumed by Auth Variable driver in SecurityPkg now, > as Auth Variable driver in SecurityPkg will be merged to Variable driver in > MdeModulePkg, so the library class also needs to be moved to MdeModulePkg. > 4. gEfiAuthenticatedVariableGuid will be used by both merged Variable driver > and AuthVariableLib, AUTHENTICATED_VARIABLE_HEADER will be used by > merged Variable driver. > > What test done: > Nt32: Boot with SECURE_BOOT_ENABLE = TRUE or FALSE, enable secure boot with > SECURE_BOOT_ENABLE = TRUE. > OVMF: Boot with SECURE_BOOT_ENABLE = TRUE or FALSE, enable secure boot with > SECURE_BOOT_ENABLE = TRUE. > Vlv2TbltDevice: Boot and enable secure boot with SECURE_BOOT_ENABLE = TRUE. > > What is the impact to platform: > 1. Only platform dsc and fdf need to be updated except the change in > ArmPlatformPkg.dec and NorFlashAuthenticatedDxe.inf to remove > gVariableAuthenticatedRuntimeDxeFileGuid and use gVariableRuntimeDxeFileGuid. > This is fine. I will follow up with a patch to merge NorFlashDxe.inf and NorFlashAuthenticatedDxe.inf after this gets merged, since the main difference is the depex on the file GUIDs -- Ard. ------------------------------------------------------------------------------ Monitor 25 network devices or servers for free with OpManager! OpManager is web-based network management software that monitors network devices and physical & virtual servers, alerts via email & sms for fault. Monitor 25 devices for free with no restriction. Download now http://ad.doubleclick.net/ddm/clk/292181274;119417398;o _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel