NorFlashDxe.inf and NorFlashAuthenticatedDxe.inf may still need to be there as NorFlashDxe.inf includes NorFlashVariableDep.c to use gEfiVariableGuid to initialize variable store header and NorFlashAuthenticatedDxe.inf includes NorFlashAuthenticatedVariableDep.c to use EfiAuthenticatedVariableGuid to initialize variable store header.
About git bisect, let me check more to find a way(maybe adjust the patch sequence) to keep the git bisect. If it works, I will resend the serial patches. Thanks, Star -----Original Message----- From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] Sent: Thursday, June 25, 2015 7:28 PM To: edk2-devel@lists.sourceforge.net Subject: Re: [edk2] [PATCH 00/20] Separate auth variable service from Auth Variable driver. 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 ------------------------------------------------------------------------------ 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
