Star,

Thanks for the feedback.  I think the "BuildTime" helps
clarify the scope and usage of this PCD.  The 
LowestSupportedVersion name is used in many places.  
The name of this PCD clarifies that this is the 
LowestSupportedVersion value provided at the time the 
FmpDxe module was built and is only used when the 
LowestSupportedVersion value is not available from 
any other source.

Mike


> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, April 19, 2018 9:53 PM
> To: Yao, Jiewen <jiewen....@intel.com>; Kinney, Michael
> D <michael.d.kin...@intel.com>; edk2-devel@lists.01.org
> Cc: Zeng, Star <star.z...@intel.com>
> Subject: RE: [RFC v2 0/4] Add FmpDevicePkg
> 
> I had a minor comment at
> https://lists.01.org/pipermail/edk2-devel/2018-
> April/023570.html to
> PcdFmpDeviceBuildTimeLowestSupportedVersion before.
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-
> boun...@lists.01.org] On Behalf Of Yao, Jiewen
> Sent: Friday, April 20, 2018 12:18 PM
> To: Kinney, Michael D <michael.d.kin...@intel.com>;
> edk2-devel@lists.01.org
> Subject: Re: [edk2] [RFC v2 0/4] Add FmpDevicePkg
> 
> Thanks Mike.
> 
> I am a little confused about naming:
> CheckLowestSupportedVersion() and
> LockFmpDeviceAtLockEventGuid().
> 
> At the first glace, I think those 2 API will *do the
> check* for LSV, or *do the lock* FMP.
> 
> But actually, it returns a state to indicate *if* the
> caller need check LSV and *if* the caller need lock FMP.
> 
> Can we rename them to be
> NeedCheckLowestSupportedVersion() and
> NeedLockFmpDeviceAtLockEventGuid() ?
> 
> 
> Other update seems good. Reviewed-by:
> jiewen....@intel.com
> 
> 
> Thank you
> Yao Jiewen
> 
> > -----Original Message-----
> > From: Kinney, Michael D
> > Sent: Wednesday, April 18, 2018 5:05 AM
> > To: edk2-devel@lists.01.org
> > Cc: Sean Brogan <sean.bro...@microsoft.com>; Yao,
> Jiewen
> > <jiewen....@intel.com>; Kinney, Michael D
> <michael.d.kin...@intel.com>
> > Subject: [RFC v2 0/4] Add FmpDevicePkg
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=922
> >
> > Changes in V2
> > =============
> > * FmpDevicePkg DEC file
> >   + Change PcdFmpDeviceSystemResetRequired from
> FeatureFlag
> >     to FixedAtBuild
> >   + Add PcdFmpDeviceTestKeySha256Digest for test key
> detection.
> >     Set to {0} to disable test key detection.
> > * FmpDevicePkg DSC file
> >   + Fix ARM and AARCH64 build issues
> >   + Update DisplayUpdateProgressLib mappings to match
> patches in
> >     Bug_801_DisplayUpdateProgressLib_V2
> > * FmpDeviceLib Class and Instance
> >   + Update FmpDeviceGetSize() to return EFI_STATUS
> >   + Update FmpDeviceGetAttributes() to return
> EFI_STATUS
> >   + Update FmpDeviceGetVersionString() to return
> EFI_STATUS
> > * FmpDxe
> >   + Set depex to Variable Arch Write Protocol and
> Variable Lock Protocol
> >   + Break out test key detection in FmpDxe into
> DetectTestKey.c
> >   + Change LockAllVars() to LockAllFmpVariables()
> >   + Update LockAllFmpVariables() to lock each of the
> UEFI variables used.
> >   + Always lock UEFI variables when
> PcdFmpDeviceLockEventGuid is signaled
> >   + Remove ComputeVersionName().  FmpDeviceLib must
> provide version
> > string
> >
> > Based on content from the following branch:
> >
> >
> https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsul
> eSupport/MsCap
> > su
> > leUpdatePkg
> >
> > Branch for review:
> >
> >
> https://github.com/mdkinney/edk2/tree/Bug_922_FmpDeviceP
> kg_V2
> >
> > This package provides an implementation of a Firmware
> Management
> > Protocol instance that supports the update of firmware
> storage devices
> > using UEFI Capsules.  The behavior of the Firmware
> Management Protocol
> > instance is customized using libraries and PCDs.
> >
> > Cc: Sean Brogan <sean.bro...@microsoft.com>
> > Cc: Jiewen Yao <jiewen....@intel.com>
> > Contributed-under: TianoCore Contribution Agreement
> 1.1
> > Signed-off-by: Michael D Kinney
> <michael.d.kin...@intel.com>
> >
> > Kinney, Michael D (4):
> >   FmpDevicePkg: Add package, library classes, and PCDs
> >   FmpDevicePkg: Add library instances
> >   FmpDevicePkg: Add FmpDxe module
> >   FmpDevicePkg: Add DSC file to build all package
> components
> >
> >  FmpDevicePkg/FmpDevicePkg.dec                      |
> 132 ++
> >  FmpDevicePkg/FmpDevicePkg.dsc                      |
> 134 ++
> >  FmpDevicePkg/FmpDevicePkg.uni                      |
> 80 ++
> >  FmpDevicePkg/FmpDevicePkgExtra.uni                 |
> 18 +
> >  FmpDevicePkg/FmpDxe/DetectTestKey.c                |
> 166 +++
> >  FmpDevicePkg/FmpDxe/FmpDxe.c                       |
> 1451
> > ++++++++++++++++++++
> >  FmpDevicePkg/FmpDxe/FmpDxe.inf                     |
> 93 ++
> >  FmpDevicePkg/FmpDxe/FmpDxe.uni                     |
> 20 +
> >  FmpDevicePkg/FmpDxe/FmpDxeExtra.uni                |
> 18 +
> >  FmpDevicePkg/FmpDxe/FmpDxeLib.inf                  |
> 90 ++
> >  FmpDevicePkg/FmpDxe/VariableSupport.c              |
> 461 +++++++
> >  FmpDevicePkg/FmpDxe/VariableSupport.h              |
> 180 +++
> >  .../Include/Library/CapsuleUpdatePolicyLib.h       |
> 120 ++
> >  FmpDevicePkg/Include/Library/FmpDeviceLib.h        |
> 405 ++++++
> >  FmpDevicePkg/Include/Library/FmpPayloadHeaderLib.h |
> 100 ++
> >  .../CapsuleUpdatePolicyLibNull.c                   |
> 136 ++
> >  .../CapsuleUpdatePolicyLibNull.inf                 |
> 45 +
> >  .../CapsuleUpdatePolicyLibNull.uni                 |
> 17 +
> >  .../Library/FmpDeviceLibNull/FmpDeviceLib.c        |
> 427 ++++++
> >  .../Library/FmpDeviceLibNull/FmpDeviceLibNull.inf  |
> 48 +
> >  .../Library/FmpDeviceLibNull/FmpDeviceLibNull.uni  |
> 18 +
> >  .../FmpPayloadHeaderLibV1/FmpPayloadHeaderLib.c    |
> 188 +++
> >  .../FmpPayloadHeaderLibV1.inf                      |
> 48 +
> >  .../FmpPayloadHeaderLibV1.uni                      |
> 21 +
> >  24 files changed, 4416 insertions(+)
> >  create mode 100644 FmpDevicePkg/FmpDevicePkg.dec
> create mode 100644
> > FmpDevicePkg/FmpDevicePkg.dsc  create mode 100644
> > FmpDevicePkg/FmpDevicePkg.uni  create mode 100644
> > FmpDevicePkg/FmpDevicePkgExtra.uni
> >  create mode 100644
> FmpDevicePkg/FmpDxe/DetectTestKey.c
> >  create mode 100644 FmpDevicePkg/FmpDxe/FmpDxe.c
> create mode 100644
> > FmpDevicePkg/FmpDxe/FmpDxe.inf  create mode 100644
> > FmpDevicePkg/FmpDxe/FmpDxe.uni  create mode 100644
> > FmpDevicePkg/FmpDxe/FmpDxeExtra.uni
> >  create mode 100644 FmpDevicePkg/FmpDxe/FmpDxeLib.inf
> create mode
> > 100644 FmpDevicePkg/FmpDxe/VariableSupport.c
> >  create mode 100644
> FmpDevicePkg/FmpDxe/VariableSupport.h
> >  create mode 100644
> > FmpDevicePkg/Include/Library/CapsuleUpdatePolicyLib.h
> >  create mode 100644
> FmpDevicePkg/Include/Library/FmpDeviceLib.h
> >  create mode 100644
> FmpDevicePkg/Include/Library/FmpPayloadHeaderLib.h
> >  create mode 100644
> >
> FmpDevicePkg/Library/CapsuleUpdatePolicyLibNull/CapsuleU
> pdatePolicyLib
> > Null
> > .c
> >  create mode 100644
> >
> FmpDevicePkg/Library/CapsuleUpdatePolicyLibNull/CapsuleU
> pdatePolicyLib
> > Null
> > .inf
> >  create mode 100644
> >
> FmpDevicePkg/Library/CapsuleUpdatePolicyLibNull/CapsuleU
> pdatePolicyLib
> > Null
> > .uni
> >  create mode 100644
> > FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
> >  create mode 100644
> >
> FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLibNull.i
> nf
> >  create mode 100644
> >
> FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLibNull.u
> ni
> >  create mode 100644
> >
> FmpDevicePkg/Library/FmpPayloadHeaderLibV1/FmpPayloadHea
> derLib.c
> >  create mode 100644
> >
> FmpDevicePkg/Library/FmpPayloadHeaderLibV1/FmpPayloadHea
> derLibV1.inf
> >  create mode 100644
> >
> FmpDevicePkg/Library/FmpPayloadHeaderLibV1/FmpPayloadHea
> derLibV1.uni
> >
> > --
> > 2.14.2.windows.3
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to