On 06/25/15 11:46, Star Zeng wrote: > For your easy review, > the forked code is at g...@github.com:lzeng14/edk2.git branch > SeparateAuthVariableService. > > 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. > 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.
Since Ard added secure boot support to ArmVirtPkg, I will let him review patch 19/20. With regard to OVMF, I have a question, and some notes. * Question: In point 4 above, do I understand correctly that the varstore is now supposed to have only one GUID, the authenticated one, regardless of whether secure boot is enabled in the build or not? If that's the case, then it's problematic. First, technically, "OvmfPkg/VarStore.fdf.inc" will be broken for the SECURE_BOOT_ENABLE=FALSE case, because this FDF include file will build a varstore template that will be then rejected by the variable driver. Second, such an update will break all preexistent virtual machines (and, well, physical machines too) that use firmware with SECURE_BOOT_ENABLE=FALSE. Their current varstores contain the non-authenticated GUID, and after a firmware upgrade (despite *keeping* SECURE_BOOT_ENABLE=FALSE), those varstores will be rejected. (If I understand right.) I think that the new, merged, variable driver should be parametrized, with regard to the varstore GUID. The driver should take a fixed pointer PCD, to a 16 byte array, specifying the GUID, and DSC files should be able to specify that PCD, based on SECURE_BOOT_ENABLE. * Notes (for patch 16/20): Please grep OvmfPkg for all instances of SECURE_BOOT_ENABLE, and adapt them all as appropriate. The patch currently adds one new conditional and removes one in each DSC file, and modifies one conditional in each FDF file. That's not good enough. Beyond the VarStore.fdf.inc issue mentioned above, each DSC file contains 8 "SECURE_BOOT_ENABLE" conditionals, and *some* of those overlap with your patch. For example, the TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf library resolution is already present elsewhere. So please investigate all current SECURE_BOOT_ENABLE conditionals under OvmfPkg/, and update them / simplify them all as appropriate. Thanks Laszlo ------------------------------------------------------------------------------ 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
