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

Reply via email to