[edk2-devel] [PATCH] IntelSiliconPkg: Issue reported by ECC in edk2-platforms.

[GuoMinJ]

https://bugzilla.tianocore.org/show_bug.cgi?id=2518

ECC need '.' character at the end of line.

Signed-off-by: GuoMinJ 
---
 .../Intel/IntelSiliconPkg/Include/Library/ConfigBlockLib.h  | 6 +++---
 .../Library/BaseConfigBlockLib/BaseConfigBlockLib.c | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Silicon/Intel/IntelSiliconPkg/Include/Library/ConfigBlockLib.h 
b/Silicon/Intel/IntelSiliconPkg/Include/Library/ConfigBlockLib.h
index 99b8ae4b5a..37a3968168 100644
--- a/Silicon/Intel/IntelSiliconPkg/Include/Library/ConfigBlockLib.h
+++ b/Silicon/Intel/IntelSiliconPkg/Include/Library/ConfigBlockLib.h
@@ -10,7 +10,7 @@
 #define _CONFIG_BLOCK_LIB_H_
 
 /**
-  Create config block table
+  Create config block table.
 
   @param[in] TotalSize- Max size to be allocated for 
the Config Block Table
   @param[out]ConfigBlockTableAddress  - On return, points to a pointer 
to the beginning of Config Block Table Address
@@ -27,7 +27,7 @@ CreateConfigBlockTable (
   );
 
 /**
-  Add config block into config block table structure
+  Add config block into config block table structure.
 
   @param[in] ConfigBlockTableAddress  - A pointer to the beginning of 
Config Block Table Address
   @param[out]ConfigBlockAddress   - On return, points to a pointer 
to the beginning of Config Block Address
@@ -44,7 +44,7 @@ AddConfigBlock (
   );
 
 /**
-  Retrieve a specific Config Block data by GUID
+  Retrieve a specific Config Block data by GUID.
 
   @param[in]  ConfigBlockTableAddress  - A pointer to the beginning of 
Config Block Table Address
   @param[in]  ConfigBlockGuid  - A pointer to the GUID uses to 
search specific Config Block
diff --git 
a/Silicon/Intel/IntelSiliconPkg/Library/BaseConfigBlockLib/BaseConfigBlockLib.c 
b/Silicon/Intel/IntelSiliconPkg/Library/BaseConfigBlockLib/BaseConfigBlockLib.c
index 33e0c81e9d..c89699ea46 100644
--- 
a/Silicon/Intel/IntelSiliconPkg/Library/BaseConfigBlockLib/BaseConfigBlockLib.c
+++ 
b/Silicon/Intel/IntelSiliconPkg/Library/BaseConfigBlockLib/BaseConfigBlockLib.c
@@ -12,7 +12,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 
 /**
-  Create config block table
+  Create config block table.
 
   @param[in] TotalSize- Max size to be allocated for 
the Config Block Table
   @param[out]ConfigBlockTableAddress  - On return, points to a pointer 
to the beginning of Config Block Table Address
@@ -51,7 +51,7 @@ CreateConfigBlockTable (
 }
 
 /**
-  Add config block into config block table structure
+  Add config block into config block table structure.
 
   @param[in] ConfigBlockTableAddress  - A pointer to the beginning of 
Config Block Table Address
   @param[out]ConfigBlockAddress   - On return, points to a pointer 
to the beginning of Config Block Address
@@ -94,7 +94,7 @@ AddConfigBlock (
 }
 
 /**
-  Retrieve a specific Config Block data by GUID
+  Retrieve a specific Config Block data by GUID.
 
   @param[in]  ConfigBlockTableAddress  - A pointer to the beginning of 
Config Block Table Address
   @param[in]  ConfigBlockGuid  - A pointer to the GUID uses to 
search specific Config Block
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54517): https://edk2.groups.io/g/devel/message/54517
Mute This Topic: https://groups.io/mt/71318791/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3] IntelSiliconPkg: FIT based shadow microcode PPI support.

[Siyuan, Fu]

Hi, Mike

The FITPointer points to the FIT table address on flash (within top 16MB 
of 4GB). It's not a memory address and normally it's always large than
the MemoryTop address in PHIT. So we couldn't use PHIT HOB to check
the FIT pointer.



Best Regards
Siyuan 

> -Original Message-
> From: Kinney, Michael D 
> Sent: 2020年2月15日 7:07
> To: Fu, Siyuan ; devel@edk2.groups.io; Kinney,
> Michael D 
> Cc: Ni, Ray ; Chaganty, Rangasai V
> 
> Subject: RE: [edk2-devel] [PATCH v3] IntelSiliconPkg: FIT based shadow
> microcode PPI support.
> 
> Siyuan,
> 
> If the FIT is not valid, then the API should just return
> an error without ASSERT().  Not all system support FIT or
> fill in FIT.  The code is more generic if it just does
> the check and returns an error.

This is an optional PEIM and only these systems which
support FIT should use it. Other platforms should not
include this PEIM so MpInitLib will still use the PCD
based microcode shadow as usual.
But it's ok to me if you think it's necessary to remove
these ASSERT so any platform can include this PEIM
without additional concern. I will update patch for this.

> 
> The check looks incomplete to me.  We know that max physical
> address of the CPU from the PHIT HOB.  If the physical address
> value is greater than the max physical address, then the
> pointer is invalid.  This would cover the 2 point cases of
> all Fs and all Es.

The FITPointer points to the FIT table address on flash (within top
16MB of 4GB). It's not a memory address and normally it's always
larger than the MemoryTop address in PHIT. So we couldn't use
PHIT HOB to check the FIT pointer.

Best Regards,
Siyuan

> 
> Mike
> 
> > -Original Message-
> > From: Fu, Siyuan 
> > Sent: Thursday, February 13, 2020 5:33 PM
> > To: Kinney, Michael D ;
> > devel@edk2.groups.io
> > Cc: Ni, Ray ; Chaganty, Rangasai V
> > 
> > Subject: RE: [edk2-devel] [PATCH v3] IntelSiliconPkg:
> > FIT based shadow microcode PPI support.
> >
> > Hi Mike
> >
> > See my reply for the ASSERT and magic number around FIT
> > table parsing code.
> >
> > > -Original Message-
> > > From: Kinney, Michael D 
> > > Sent: 2020年2月13日 8:58
> > > To: devel@edk2.groups.io; Fu, Siyuan
> > ; Kinney, Michael
> > > D 
> > > Cc: Ni, Ray ; Chaganty, Rangasai V
> > > 
> > > Subject: RE: [edk2-devel] [PATCH v3] IntelSiliconPkg:
> > FIT based shadow
> > > microcode PPI support.
> > >
> > > Siyuan,
> > >
> > > IntelSiliconPkg/Feature/ShadowMicrocode:
> > >
> > > For simple modules that only have a single .c file,
> > there
> > > Is not need to split out a .h file.  Please merge the
> > .h
> > > File content into the .c file and delete the .h file.
> > >
> > > More comments inline below.
> > >
> > > Mike
> > >
> > > > -Original Message-
> > > > From: devel@edk2.groups.io 
> > On
> > > > Behalf Of Siyuan, Fu
> > > > Sent: Tuesday, February 11, 2020 4:48 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Ni, Ray ; Chaganty, Rangasai
> > V
> > > > 
> > > > Subject: [edk2-devel] [PATCH v3] IntelSiliconPkg:
> > FIT
> > > > based shadow microcode PPI support.
> > > >
> > > > V3 Changes:
> > > > Remove the feature PCD PcdCpuShadowMicrocodeByFit
> > > > because the whole FIT microcode shadow code is
> > moved to
> > > > this PEIM so platform could disable this feature by
> > not
> > > > include PEIM now.
> > > >
> > > > V2 Changes:
> > > > Rename EDKII_PEI_CPU_MICROCODE_ID to
> > > > EDKII_PEI_MICROCODE_CPU_ID.
> > > >
> > > > This patch adds a platform PEIM for FIT based
> > shadow
> > > > microcode PPI support. A detailed design doc can be
> > > > found here:
> > > >
> > https://edk2.groups.io/g/devel/files/Designs/2020/0214/
> > > > Support%20
> > > > the%202nd%20Microcode%20FV%20Flash%20Region.pdf
> > Trim long patch content.
> >
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +ShadowMicrocodePatchByFit (
> > > > +  IN  UINTN
> > > > CpuIdCount,
> > > > +  IN  EDKII_PEI_MICROCODE_CPU_ID
> > > > *MicrocodeCpuId,
> > > > +  OUT UINTN
> > > > *BufferSize,
> > > > +  OUT VOID
> > **Buffer
> > > > +  )
> > > > +{
> > > > +  UINT64FitPointer;
> > > > +  FIRMWARE_INTERFACE_TABLE_ENTRY*FitEntry;
> > > > +  UINT32EntryNum;
> > > > +  UINT32Index;
> > > > +  MICROCODE_PATCH_INFO
> > *PatchInfoBuffer;
> > > > +  UINTN
> > MaxPatchNumber;
> > > > +  CPU_MICROCODE_HEADER
> > > > *MicrocodeEntryPoint;
> > > > +  UINTN PatchCount;
> > > > +  UINTN TotalSize;
> > > > +  UINTN TotalLoadSize;
> > > > +
> > > > +  FitPointer = *(UINT64 *) (UINTN)
> > > > FIT_POINTER_ADDRESS;  if
> > > > + ((FitPointer == 0) ||
> > > > +  (FitPointer == 0x) ||
> > > > +  (FitPointer == 0x)) {
> > >
> > > Are these constants defined in the FIT include file?
> > > Would be better if they are #defines from FIT include
> > > file or in this module.
> >
> 

[edk2-devel] Upcoming Event: TianoCore Design Meeting - APAC/NAMO - Fri, 02/21/2020 9:30am-10:30am #cal-reminder

[devel@edk2.groups.io Calendar]

*Reminder:* TianoCore Design Meeting - APAC/NAMO

*When:* Friday, 21 February 2020, 9:30am to 10:30am, (GMT+08:00) Asia/Chongqing

*Where:* BlueJeans Meeting

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=681238 )

*Organizer:* Ray Ni ray...@intel.com ( 
ray...@intel.com?subject=Re:%20Event:%20TianoCore%20Design%20Meeting%20-%20APAC%2FNAMO
 )

*Description:*

For more info, see here: https://www.tianocore.org/design-meeting/

To join the meeting on a computer or mobile phone: 
https://bluejeans.com/889357567?src=calendarLink

Phone Dial-in +1.408.740.7256 (US (San Jose)) +1.408.317.9253 (US (Primary, San 
Jose)) Global Numbers: https://www.bluejeans.com/numbers

Meeting ID: 889 357 567

Room System 199.48.152.152 or bjn.vc

Meeting ID: 889 357 567

Want to test your video connection? https://bluejeans.com/111

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54515): https://edk2.groups.io/g/devel/message/54515
Mute This Topic: https://groups.io/mt/71315831/21656
Mute #cal-reminder: https://groups.io/mk?hashtag=cal-reminder=3846945
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Upcoming Event: TianoCore Bug Triage - APAC / NAMO - Thu, 02/20/2020 5:00pm-5:30pm #cal-reminder

[devel@edk2.groups.io Calendar]

*Reminder:* TianoCore Bug Triage - APAC / NAMO

*When:* Thursday, 20 February 2020, 5:00pm to 5:30pm, (GMT-08:00) America/Los 
Angeles

*Where:* https://bluejeans.com/889357567?src=calendarLink

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=503284 )

*Organizer:* Brian Richardson brian.richard...@intel.com ( 
brian.richard...@intel.com?subject=Re:%20Event:%20TianoCore%20Bug%20Triage%20-%20APAC%20%2F%20NAMO
 )

*Description:*

For more info please see:

https://www.tianocore.org/bug-triage

To join the meeting on a computer or mobile phone:

https://bluejeans.com/889357567?src=calendarLink ( 
https://bluejeans.com/889357567?src=calendarLink )

Phone Dial-in

+1.408.740.7256 (US (San Jose))

+1.408.317.9253 (US (Primary, San Jose))

Global Numbers: https://www.bluejeans.com/numbers ( 
https://www.bluejeans.com/numbers )

Meeting ID: 889 357 567

Room System

199.48.152.152 or bjn.vc

Meeting ID: 889 357 567

Want to test your video connection?

https://bluejeans.com/111 ( https://bluejeans.com/111 )

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54514): https://edk2.groups.io/g/devel/message/54514
Mute This Topic: https://groups.io/mt/71315327/21656
Mute #cal-reminder: https://groups.io/mk?hashtag=cal-reminder=3846945
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v6 0/2] Enhancement and Fixes to BaseHashApiLib

[Michael D Kinney]

Series Reviewed-by: Michael D Kinney 

Mike


> -Original Message-
> From: Sukerkar, Amol N 
> Sent: Saturday, February 15, 2020 11:51 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ;
> Yao, Jiewen ; Wang, Jian J
> ; Agrawal, Sachin
> ; Gao, Liming
> 
> Subject: [PATCH v6 0/2] Enhancement and Fixes to
> BaseHashApiLib
> 
> This patch implements the fixes and enhancement to
> BaseHashApiLib in
> the following manner:
> - Remove reference to MD4 and MD5 hashing algorithms as
> they are
>   deprecated;
> - Align the enumeration for hashing algorithmswith the
> one used in
>   TPM 2.0 implementation defined in
> IndustryStandard/Tpm20.h;
> - Change the type of PcdHashApiLibPolicy to
> PcdsFixedAtBuild to
>   optimize away the unused hashing algorithms for a
> particular
>   instance of HashApiLib.
> 
> More information can be found at Bugzilla ticket,
> https://bugzilla.tianocore.org/show_bug.cgi?id=2511.
> 
> Amol N Sukerkar (2):
>   CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with
> TPM 2.0
> Implementation
>   CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy
> type to
> FixedAtBuild
> 
>  CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c |
> 121 ++--
>  CryptoPkg/CryptoPkg.dec   |
> 17 ++-
>  CryptoPkg/CryptoPkg.uni   |
> 12 +-
>  CryptoPkg/Include/Library/HashApiLib.h|
> 16 +--
>  4 files changed, 51 insertions(+), 115 deletions(-)
> 
> --
> 2.16.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54513): https://edk2.groups.io/g/devel/message/54513
Mute This Topic: https://groups.io/mt/71308983/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v6 2/2] CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy type to FixedAtBuild

[Sukerkar, Amol N]

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2511

This commit changes the PCD PcdHashApiLibPolicy to the type
PcdsFixedAtBuild so as to be able to optimize away the unused hashing
algorithms in HashApiLib instance used by a driver.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Michael D Kinney 
Signed-off-by: Amol N Sukerkar 
---

Notes:
v2
- Fixed closed parantheses in the commit message

v6
- Removed extra PcdsFixedAtBuild from CryptoPkg.dec

 CryptoPkg/CryptoPkg.dec | 1 -
 1 file changed, 1 deletion(-)

diff --git a/CryptoPkg/CryptoPkg.dec b/CryptoPkg/CryptoPkg.dec
index 82437fef6d89..4d1a1368a8d4 100644
--- a/CryptoPkg/CryptoPkg.dec
+++ b/CryptoPkg/CryptoPkg.dec
@@ -69,7 +69,6 @@ [PcdsFixedAtBuild]
   Pcd/PcdCryptoServiceFamilyEnable.h
   }
 
-[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   ## This PCD indicates the HASH algorithm to calculate hash of data
   #  Based on the value set, the required algorithm is chosen to calculate
   #  the hash of data.
-- 
2.16.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54512): https://edk2.groups.io/g/devel/message/54512
Mute This Topic: https://groups.io/mt/71308986/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v6 0/2] Enhancement and Fixes to BaseHashApiLib

[Sukerkar, Amol N]

This patch implements the fixes and enhancement to BaseHashApiLib in
the following manner:
- Remove reference to MD4 and MD5 hashing algorithms as they are
  deprecated;
- Align the enumeration for hashing algorithmswith the one used in
  TPM 2.0 implementation defined in IndustryStandard/Tpm20.h;
- Change the type of PcdHashApiLibPolicy to PcdsFixedAtBuild to
  optimize away the unused hashing algorithms for a particular
  instance of HashApiLib.

More information can be found at Bugzilla ticket,
https://bugzilla.tianocore.org/show_bug.cgi?id=2511.

Amol N Sukerkar (2):
  CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0
Implementation
  CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy type to
FixedAtBuild

 CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | 121 ++--
 CryptoPkg/CryptoPkg.dec   |  17 ++-
 CryptoPkg/CryptoPkg.uni   |  12 +-
 CryptoPkg/Include/Library/HashApiLib.h|  16 +--
 4 files changed, 51 insertions(+), 115 deletions(-)

-- 
2.16.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54510): https://edk2.groups.io/g/devel/message/54510
Mute This Topic: https://groups.io/mt/71308983/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v6 1/2] CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation

[Sukerkar, Amol N]

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2511

This commit aligns the baseHashApiLib with TPM 2.0 Implementation
as follows:
- Remove reference to MD4 and MD5 algorithms as they are deprecated
- Align the enumerations for hashing algoerithms with the one used
  in TPM 2.0 implementation defined in IndustryStandard/Tpm20.h.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Michael D Kinney 
Signed-off-by: Amol N Sukerkar 
---

Notes:
v2
- Fixed closed parentheses in commit message

v3
- Fixed #ifdef for HashApiLib.h
- Changed location of IndustryStandard/Tpm20.h from
  HashApiLib.h to BaseHashApiLib.c
- Changed @ValidRange to @ValidList in CryptoPkg.dec
- Aligned hash algorithm definitions to match Tpm20.h
  in CryptoPkg.dec and CryptoPkg.uni

v4
- Changed PcdHashApiLibPolicy to UINT32
v5
- Changed PcdGet16 to PcdGet32 in BaseHashApiLib.c

 CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | 121 ++--
 CryptoPkg/CryptoPkg.dec   |  16 ++-
 CryptoPkg/CryptoPkg.uni   |  12 +-
 CryptoPkg/Include/Library/HashApiLib.h|  16 +--
 4 files changed, 51 insertions(+), 114 deletions(-)

diff --git a/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c 
b/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c
index 277ef9f0b421..f9796b215865 100644
--- a/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c
+++ b/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c
@@ -12,6 +12,7 @@
 **/
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -31,32 +32,24 @@ HashApiGetContextSize (
   VOID
   )
 {
-  switch (PcdGet8 (PcdHashApiLibPolicy)) {
-case HASH_API_ALGO_MD4:
-  return Md4GetContextSize ();
-  break;
-
-case HASH_API_ALGO_MD5:
-  return Md5GetContextSize ();
-  break;
-
-case HASH_API_ALGO_SHA1:
+  switch (PcdGet32 (PcdHashApiLibPolicy)) {
+case HASH_ALG_SHA1:
   return Sha1GetContextSize ();
   break;
 
-case HASH_API_ALGO_SHA256:
+case HASH_ALG_SHA256:
   return Sha256GetContextSize ();
   break;
 
-case HASH_API_ALGO_SHA384:
+case HASH_ALG_SHA384:
   return Sha384GetContextSize ();
   break;
 
-case HASH_API_ALGO_SHA512:
+case HASH_ALG_SHA512:
   return Sha512GetContextSize ();
   break;
 
-case HASH_API_ALGO_SM3_256:
+case HASH_ALG_SM3_256:
   return Sm3GetContextSize ();
   break;
 
@@ -81,32 +74,24 @@ HashApiInit (
   OUT HASH_API_CONTEXT  HashContext
   )
 {
-  switch (PcdGet8 (PcdHashApiLibPolicy)) {
-case HASH_API_ALGO_MD4:
-  return Md4Init (HashContext);
-  break;
-
-case HASH_API_ALGO_MD5:
-  return Md5Init (HashContext);
-  break;
-
-case HASH_API_ALGO_SHA1:
+  switch (PcdGet32 (PcdHashApiLibPolicy)) {
+case HASH_ALG_SHA1:
   return Sha1Init (HashContext);
   break;
 
-case HASH_API_ALGO_SHA256:
+case HASH_ALG_SHA256:
   return Sha256Init (HashContext);
   break;
 
-case HASH_API_ALGO_SHA384:
+case HASH_ALG_SHA384:
   return Sha384Init (HashContext);
   break;
 
-case HASH_API_ALGO_SHA512:
+case HASH_ALG_SHA512:
   return Sha512Init (HashContext);
   break;
 
-case HASH_API_ALGO_SM3_256:
+case HASH_ALG_SM3_256:
   return Sm3Init (HashContext);
   break;
 
@@ -133,32 +118,24 @@ HashApiDuplicate (
   OUT HASH_API_CONTEXT  NewHashContext
   )
 {
-  switch (PcdGet8 (PcdHashApiLibPolicy)) {
-case HASH_API_ALGO_MD4:
-  return Md4Duplicate (HashContext, NewHashContext);
-  break;
-
-case HASH_API_ALGO_MD5:
-  return Md5Duplicate (HashContext, NewHashContext);
-  break;
-
-case HASH_API_ALGO_SHA1:
+  switch (PcdGet32 (PcdHashApiLibPolicy)) {
+case HASH_ALG_SHA1:
   return Sha1Duplicate (HashContext, NewHashContext);
   break;
 
-case HASH_API_ALGO_SHA256:
+case HASH_ALG_SHA256:
   return Sha256Duplicate (HashContext, NewHashContext);
   break;
 
-case HASH_API_ALGO_SHA384:
+case HASH_ALG_SHA384:
   return Sha384Duplicate (HashContext, NewHashContext);
   break;
 
-case HASH_API_ALGO_SHA512:
+case HASH_ALG_SHA512:
   return Sha512Duplicate (HashContext, NewHashContext);
   break;
 
-case HASH_API_ALGO_SM3_256:
+case HASH_ALG_SM3_256:
   return Sm3Duplicate (HashContext, NewHashContext);
   break;
 
@@ -187,32 +164,24 @@ HashApiUpdate (
   IN UINTN DataToHashLen
   )
 {
-  switch (PcdGet8 (PcdHashApiLibPolicy)) {
-case HASH_API_ALGO_MD4:
-  return Md4Update (HashContext, DataToHash, DataToHashLen);
-  break;
-
-case HASH_API_ALGO_MD5:
-  return Md5Update (HashContext, DataToHash, DataToHashLen);
-  break;
-
-case HASH_API_ALGO_SHA1:
+  switch (PcdGet32 (PcdHashApiLibPolicy)) {
+case HASH_ALG_SHA1:
   return Sha1Update (HashContext, DataToHash, DataToHashLen);
   break;
 
-case HASH_API_ALGO_SHA256:
+case HASH_ALG_SHA256:

Re: [edk2-devel] [PATCH v4 0/2] Enhancement and Fixes to BaseHashApiLib

[Sukerkar, Amol N]

Thanks for the explanation, Mike! Also, I noticed during rebasing my original 
commit (BZ: 2151) that PcdSetxx does not work with PcdsFixedAtBuild anymore, 
which, should be by design.

Please ignore patch v5 as patch v6 contains all the fixes.

~ Amol

-Original Message-
From: Kinney, Michael D  
Sent: Saturday, February 15, 2020 12:44 PM
To: Sukerkar, Amol N ; devel@edk2.groups.io; Kinney, 
Michael D 
Cc: Yao, Jiewen ; Wang, Jian J ; 
Agrawal, Sachin ; Gao, Liming 
Subject: RE: [PATCH v4 0/2] Enhancement and Fixes to BaseHashApiLib

Amol,

FixedPcdGet32() does not apply to this use case.

FixedPcdGet32() is usually only used when a PCD value is used to fill in a 
field of a structure in a global variable where the compiler requires a value 
instead of a variable or a function call.

The general rule is to use PcdGet/Setxx() everywhere possible to maximize 
compatibility with different PCD types and only use FixedPcdGet/Setxx() if the 
compiler cannot build the component when
PcdGet/Setxx() is used.

Thanks,

Mike

> -Original Message-
> From: Sukerkar, Amol N 
> Sent: Saturday, February 15, 2020 11:40 AM
> To: Kinney, Michael D ; 
> devel@edk2.groups.io
> Cc: Yao, Jiewen ; Wang, Jian J 
> ; Agrawal, Sachin ; 
> Gao, Liming ; Sukerkar, Amol N 
> 
> Subject: RE: [PATCH v4 0/2] Enhancement and Fixes to BaseHashApiLib
> 
> Hi Mike,
> 
> Yes, I just noticed and sent the patch with update 1 (build passed and 
> worked with PcdGet16). I didn't notice the second change so I will 
> make it as well in version 6.
> 
> Question: There is a call FixedPcdGet32 as well. Would it be 
> applicable in BaseHashApiLib?
> 
> Thanks,
> Amol
> 
> -Original Message-
> From: Kinney, Michael D 
> Sent: Saturday, February 15, 2020 12:28 PM
> To: Sukerkar, Amol N ; 
> devel@edk2.groups.io; Kinney, Michael D 
> Cc: Yao, Jiewen ; Wang, Jian J 
> ; Agrawal, Sachin ; 
> Gao, Liming 
> Subject: RE: [PATCH v4 0/2] Enhancement and Fixes to BaseHashApiLib
> 
> Hi Amol,
> 
> Thanks for the updates:
> 
> There are a couple items remaining:
> 
> 1) BaseHashApiLib needs to use PcdGet32() instead of
> PcdGet16()
> 2) The extra [PcdsFixedAtBuild] line needs to be removed from 
> CryptoPkg.dec
> 
> Thanks,
> 
> Mike
> 
> > -Original Message-
> > From: Sukerkar, Amol N 
> > Sent: Friday, February 14, 2020 4:19 PM
> > To: devel@edk2.groups.io
> > Cc: Kinney, Michael D ;
> Yao, Jiewen
> > ; Wang, Jian J
> ; Agrawal,
> > Sachin ; Gao, Liming
> 
> > Subject: [PATCH v4 0/2] Enhancement and Fixes to
> BaseHashApiLib
> >
> > This patch implements the fixes and enhancement to
> BaseHashApiLib in
> > the following manner:
> > - Remove reference to MD4 and MD5 hashing algorithms
> as they are
> >   deprecated;
> > - Align the enumeration for hashing algorithmswith
> the one used in
> >   TPM 2.0 implementation defined in
> > IndustryStandard/Tpm20.h;
> > - Change the type of PcdHashApiLibPolicy to
> PcdsFixedAtBuild to
> >   optimize away the unused hashing algorithms for a
> particular
> >   instance of HashApiLib.
> >
> > More information can be found at Bugzilla ticket, 
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2511.
> >
> > Amol N Sukerkar (2):
> >   CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with
> TPM 2.0
> > Implementation
> >   CryptoPkg/BaseHashApiLib: Change
> PcdHashApiLibPolicy type to
> > FixedAtBuild
> >
> >  CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c |
> > 121 ++--
> >  CryptoPkg/CryptoPkg.dec   |
> > 18 ++-
> >  CryptoPkg/CryptoPkg.uni   |
> > 12 +-
> >  CryptoPkg/Include/Library/HashApiLib.h|
> > 16 +--
> >  4 files changed, 52 insertions(+), 115 deletions(-)
> >
> > --
> > 2.16.2.windows.1
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54509): https://edk2.groups.io/g/devel/message/54509
Mute This Topic: https://groups.io/mt/71288763/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] Enhancement and Fixes to BaseHashApiLib

[Michael D Kinney]

Amol,

FixedPcdGet32() does not apply to this use case.

FixedPcdGet32() is usually only used when a PCD value 
is used to fill in a field of a structure in a global
variable where the compiler requires a value instead
of a variable or a function call.

The general rule is to use PcdGet/Setxx() everywhere 
possible to maximize compatibility with different
PCD types and only use FixedPcdGet/Setxx() if the
compiler cannot build the component when
PcdGet/Setxx() is used.

Thanks,

Mike

> -Original Message-
> From: Sukerkar, Amol N 
> Sent: Saturday, February 15, 2020 11:40 AM
> To: Kinney, Michael D ;
> devel@edk2.groups.io
> Cc: Yao, Jiewen ; Wang, Jian J
> ; Agrawal, Sachin
> ; Gao, Liming
> ; Sukerkar, Amol N
> 
> Subject: RE: [PATCH v4 0/2] Enhancement and Fixes to
> BaseHashApiLib
> 
> Hi Mike,
> 
> Yes, I just noticed and sent the patch with update 1
> (build passed and worked with PcdGet16). I didn't
> notice the second change so I will make it as well in
> version 6.
> 
> Question: There is a call FixedPcdGet32 as well. Would
> it be applicable in BaseHashApiLib?
> 
> Thanks,
> Amol
> 
> -Original Message-
> From: Kinney, Michael D 
> Sent: Saturday, February 15, 2020 12:28 PM
> To: Sukerkar, Amol N ;
> devel@edk2.groups.io; Kinney, Michael D
> 
> Cc: Yao, Jiewen ; Wang, Jian J
> ; Agrawal, Sachin
> ; Gao, Liming
> 
> Subject: RE: [PATCH v4 0/2] Enhancement and Fixes to
> BaseHashApiLib
> 
> Hi Amol,
> 
> Thanks for the updates:
> 
> There are a couple items remaining:
> 
> 1) BaseHashApiLib needs to use PcdGet32() instead of
> PcdGet16()
> 2) The extra [PcdsFixedAtBuild] line needs to be
> removed from CryptoPkg.dec
> 
> Thanks,
> 
> Mike
> 
> > -Original Message-
> > From: Sukerkar, Amol N 
> > Sent: Friday, February 14, 2020 4:19 PM
> > To: devel@edk2.groups.io
> > Cc: Kinney, Michael D ;
> Yao, Jiewen
> > ; Wang, Jian J
> ; Agrawal,
> > Sachin ; Gao, Liming
> 
> > Subject: [PATCH v4 0/2] Enhancement and Fixes to
> BaseHashApiLib
> >
> > This patch implements the fixes and enhancement to
> BaseHashApiLib in
> > the following manner:
> > - Remove reference to MD4 and MD5 hashing algorithms
> as they are
> >   deprecated;
> > - Align the enumeration for hashing algorithmswith
> the one used in
> >   TPM 2.0 implementation defined in
> > IndustryStandard/Tpm20.h;
> > - Change the type of PcdHashApiLibPolicy to
> PcdsFixedAtBuild to
> >   optimize away the unused hashing algorithms for a
> particular
> >   instance of HashApiLib.
> >
> > More information can be found at Bugzilla ticket,
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2511.
> >
> > Amol N Sukerkar (2):
> >   CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with
> TPM 2.0
> > Implementation
> >   CryptoPkg/BaseHashApiLib: Change
> PcdHashApiLibPolicy type to
> > FixedAtBuild
> >
> >  CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c |
> > 121 ++--
> >  CryptoPkg/CryptoPkg.dec   |
> > 18 ++-
> >  CryptoPkg/CryptoPkg.uni   |
> > 12 +-
> >  CryptoPkg/Include/Library/HashApiLib.h|
> > 16 +--
> >  4 files changed, 52 insertions(+), 115 deletions(-)
> >
> > --
> > 2.16.2.windows.1
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54508): https://edk2.groups.io/g/devel/message/54508
Mute This Topic: https://groups.io/mt/71288763/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] Enhancement and Fixes to BaseHashApiLib

[Sukerkar, Amol N]

Hi Mike,

Yes, I just noticed and sent the patch with update 1 (build passed and worked 
with PcdGet16). I didn't notice the second change so I will make it as well in 
version 6.

Question: There is a call FixedPcdGet32 as well. Would it be applicable in 
BaseHashApiLib?

Thanks,
Amol

-Original Message-
From: Kinney, Michael D  
Sent: Saturday, February 15, 2020 12:28 PM
To: Sukerkar, Amol N ; devel@edk2.groups.io; Kinney, 
Michael D 
Cc: Yao, Jiewen ; Wang, Jian J ; 
Agrawal, Sachin ; Gao, Liming 
Subject: RE: [PATCH v4 0/2] Enhancement and Fixes to BaseHashApiLib

Hi Amol,

Thanks for the updates:

There are a couple items remaining:

1) BaseHashApiLib needs to use PcdGet32() instead of PcdGet16()
2) The extra [PcdsFixedAtBuild] line needs to be removed from CryptoPkg.dec

Thanks,

Mike

> -Original Message-
> From: Sukerkar, Amol N 
> Sent: Friday, February 14, 2020 4:19 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Yao, Jiewen 
> ; Wang, Jian J ; Agrawal, 
> Sachin ; Gao, Liming 
> Subject: [PATCH v4 0/2] Enhancement and Fixes to BaseHashApiLib
> 
> This patch implements the fixes and enhancement to BaseHashApiLib in 
> the following manner:
> - Remove reference to MD4 and MD5 hashing algorithms as they are
>   deprecated;
> - Align the enumeration for hashing algorithmswith the one used in
>   TPM 2.0 implementation defined in
> IndustryStandard/Tpm20.h;
> - Change the type of PcdHashApiLibPolicy to PcdsFixedAtBuild to
>   optimize away the unused hashing algorithms for a particular
>   instance of HashApiLib.
> 
> More information can be found at Bugzilla ticket, 
> https://bugzilla.tianocore.org/show_bug.cgi?id=2511.
> 
> Amol N Sukerkar (2):
>   CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0
> Implementation
>   CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy type to
> FixedAtBuild
> 
>  CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c |
> 121 ++--
>  CryptoPkg/CryptoPkg.dec   |
> 18 ++-
>  CryptoPkg/CryptoPkg.uni   |
> 12 +-
>  CryptoPkg/Include/Library/HashApiLib.h|
> 16 +--
>  4 files changed, 52 insertions(+), 115 deletions(-)
> 
> --
> 2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54507): https://edk2.groups.io/g/devel/message/54507
Mute This Topic: https://groups.io/mt/71288763/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v5 1/2] CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation

[Sukerkar, Amol N]

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2511

This commit aligns the baseHashApiLib with TPM 2.0 Implementation
as follows:
- Remove reference to MD4 and MD5 algorithms as they are deprecated
- Align the enumerations for hashing algoerithms with the one used
  in TPM 2.0 implementation defined in IndustryStandard/Tpm20.h.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Michael D Kinney 
Signed-off-by: Amol N Sukerkar 
---

Notes:
v2
- Fixed closed parentheses in commit message

v3
- Fixed #ifdef for HashApiLib.h
- Changed location of IndustryStandard/Tpm20.h from
  HashApiLib.h to BaseHashApiLib.c
- Changed @ValidRange to @ValidList in CryptoPkg.dec
- Aligned hash algorithm definitions to match Tpm20.h
  in CryptoPkg.dec and CryptoPkg.uni

v4
- Changed PcdHashApiLibPolicy to UINT32
v5
- Changed PcdGet16 to PcdGet32 in BaseHashApiLib.c

 CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | 121 ++--
 CryptoPkg/CryptoPkg.dec   |  16 ++-
 CryptoPkg/CryptoPkg.uni   |  12 +-
 CryptoPkg/Include/Library/HashApiLib.h|  16 +--
 4 files changed, 51 insertions(+), 114 deletions(-)

diff --git a/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c 
b/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c
index 277ef9f0b421..f9796b215865 100644
--- a/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c
+++ b/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c
@@ -12,6 +12,7 @@
 **/
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -31,32 +32,24 @@ HashApiGetContextSize (
   VOID
   )
 {
-  switch (PcdGet8 (PcdHashApiLibPolicy)) {
-case HASH_API_ALGO_MD4:
-  return Md4GetContextSize ();
-  break;
-
-case HASH_API_ALGO_MD5:
-  return Md5GetContextSize ();
-  break;
-
-case HASH_API_ALGO_SHA1:
+  switch (PcdGet32 (PcdHashApiLibPolicy)) {
+case HASH_ALG_SHA1:
   return Sha1GetContextSize ();
   break;
 
-case HASH_API_ALGO_SHA256:
+case HASH_ALG_SHA256:
   return Sha256GetContextSize ();
   break;
 
-case HASH_API_ALGO_SHA384:
+case HASH_ALG_SHA384:
   return Sha384GetContextSize ();
   break;
 
-case HASH_API_ALGO_SHA512:
+case HASH_ALG_SHA512:
   return Sha512GetContextSize ();
   break;
 
-case HASH_API_ALGO_SM3_256:
+case HASH_ALG_SM3_256:
   return Sm3GetContextSize ();
   break;
 
@@ -81,32 +74,24 @@ HashApiInit (
   OUT HASH_API_CONTEXT  HashContext
   )
 {
-  switch (PcdGet8 (PcdHashApiLibPolicy)) {
-case HASH_API_ALGO_MD4:
-  return Md4Init (HashContext);
-  break;
-
-case HASH_API_ALGO_MD5:
-  return Md5Init (HashContext);
-  break;
-
-case HASH_API_ALGO_SHA1:
+  switch (PcdGet32 (PcdHashApiLibPolicy)) {
+case HASH_ALG_SHA1:
   return Sha1Init (HashContext);
   break;
 
-case HASH_API_ALGO_SHA256:
+case HASH_ALG_SHA256:
   return Sha256Init (HashContext);
   break;
 
-case HASH_API_ALGO_SHA384:
+case HASH_ALG_SHA384:
   return Sha384Init (HashContext);
   break;
 
-case HASH_API_ALGO_SHA512:
+case HASH_ALG_SHA512:
   return Sha512Init (HashContext);
   break;
 
-case HASH_API_ALGO_SM3_256:
+case HASH_ALG_SM3_256:
   return Sm3Init (HashContext);
   break;
 
@@ -133,32 +118,24 @@ HashApiDuplicate (
   OUT HASH_API_CONTEXT  NewHashContext
   )
 {
-  switch (PcdGet8 (PcdHashApiLibPolicy)) {
-case HASH_API_ALGO_MD4:
-  return Md4Duplicate (HashContext, NewHashContext);
-  break;
-
-case HASH_API_ALGO_MD5:
-  return Md5Duplicate (HashContext, NewHashContext);
-  break;
-
-case HASH_API_ALGO_SHA1:
+  switch (PcdGet32 (PcdHashApiLibPolicy)) {
+case HASH_ALG_SHA1:
   return Sha1Duplicate (HashContext, NewHashContext);
   break;
 
-case HASH_API_ALGO_SHA256:
+case HASH_ALG_SHA256:
   return Sha256Duplicate (HashContext, NewHashContext);
   break;
 
-case HASH_API_ALGO_SHA384:
+case HASH_ALG_SHA384:
   return Sha384Duplicate (HashContext, NewHashContext);
   break;
 
-case HASH_API_ALGO_SHA512:
+case HASH_ALG_SHA512:
   return Sha512Duplicate (HashContext, NewHashContext);
   break;
 
-case HASH_API_ALGO_SM3_256:
+case HASH_ALG_SM3_256:
   return Sm3Duplicate (HashContext, NewHashContext);
   break;
 
@@ -187,32 +164,24 @@ HashApiUpdate (
   IN UINTN DataToHashLen
   )
 {
-  switch (PcdGet8 (PcdHashApiLibPolicy)) {
-case HASH_API_ALGO_MD4:
-  return Md4Update (HashContext, DataToHash, DataToHashLen);
-  break;
-
-case HASH_API_ALGO_MD5:
-  return Md5Update (HashContext, DataToHash, DataToHashLen);
-  break;
-
-case HASH_API_ALGO_SHA1:
+  switch (PcdGet32 (PcdHashApiLibPolicy)) {
+case HASH_ALG_SHA1:
   return Sha1Update (HashContext, DataToHash, DataToHashLen);
   break;
 
-case HASH_API_ALGO_SHA256:
+case HASH_ALG_SHA256:

[edk2-devel] [PATCH v5 2/2] CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy type to FixedAtBuild

[Sukerkar, Amol N]

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2511

This commit changes the PCD PcdHashApiLibPolicy to the type
PcdsFixedAtBuild so as to be able to optimize away the unused hashing
algorithms in HashApiLib instance used by a driver.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Michael D Kinney 
Signed-off-by: Amol N Sukerkar 
---

Notes:
v2
- Fixed closed parantheses in the commit message

 CryptoPkg/CryptoPkg.dec | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CryptoPkg/CryptoPkg.dec b/CryptoPkg/CryptoPkg.dec
index 82437fef6d89..510423a61a64 100644
--- a/CryptoPkg/CryptoPkg.dec
+++ b/CryptoPkg/CryptoPkg.dec
@@ -69,7 +69,7 @@ [PcdsFixedAtBuild]
   Pcd/PcdCryptoServiceFamilyEnable.h
   }
 
-[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
+[PcdsFixedAtBuild]
   ## This PCD indicates the HASH algorithm to calculate hash of data
   #  Based on the value set, the required algorithm is chosen to calculate
   #  the hash of data.
-- 
2.16.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54505): https://edk2.groups.io/g/devel/message/54505
Mute This Topic: https://groups.io/mt/71308629/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v5 0/2] Enhancement and Fixes to BaseHashApiLib

[Sukerkar, Amol N]

This patch implements the fixes and enhancement to BaseHashApiLib in
the following manner:
- Remove reference to MD4 and MD5 hashing algorithms as they are
  deprecated;
- Align the enumeration for hashing algorithmswith the one used in
  TPM 2.0 implementation defined in IndustryStandard/Tpm20.h;
- Change the type of PcdHashApiLibPolicy to PcdsFixedAtBuild to
  optimize away the unused hashing algorithms for a particular
  instance of HashApiLib.

More information can be found at Bugzilla ticket,
https://bugzilla.tianocore.org/show_bug.cgi?id=2511.

Amol N Sukerkar (2):
  CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0
Implementation
  CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy type to
FixedAtBuild

 CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | 121 ++--
 CryptoPkg/CryptoPkg.dec   |  18 ++-
 CryptoPkg/CryptoPkg.uni   |  12 +-
 CryptoPkg/Include/Library/HashApiLib.h|  16 +--
 4 files changed, 52 insertions(+), 115 deletions(-)

-- 
2.16.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54503): https://edk2.groups.io/g/devel/message/54503
Mute This Topic: https://groups.io/mt/71308627/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] Enhancement and Fixes to BaseHashApiLib

[Michael D Kinney]

Hi Amol,

Thanks for the updates:

There are a couple items remaining:

1) BaseHashApiLib needs to use PcdGet32() instead of PcdGet16()
2) The extra [PcdsFixedAtBuild] line needs to be removed from CryptoPkg.dec

Thanks,

Mike

> -Original Message-
> From: Sukerkar, Amol N 
> Sent: Friday, February 14, 2020 4:19 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ;
> Yao, Jiewen ; Wang, Jian J
> ; Agrawal, Sachin
> ; Gao, Liming
> 
> Subject: [PATCH v4 0/2] Enhancement and Fixes to
> BaseHashApiLib
> 
> This patch implements the fixes and enhancement to
> BaseHashApiLib in
> the following manner:
> - Remove reference to MD4 and MD5 hashing algorithms as
> they are
>   deprecated;
> - Align the enumeration for hashing algorithmswith the
> one used in
>   TPM 2.0 implementation defined in
> IndustryStandard/Tpm20.h;
> - Change the type of PcdHashApiLibPolicy to
> PcdsFixedAtBuild to
>   optimize away the unused hashing algorithms for a
> particular
>   instance of HashApiLib.
> 
> More information can be found at Bugzilla ticket,
> https://bugzilla.tianocore.org/show_bug.cgi?id=2511.
> 
> Amol N Sukerkar (2):
>   CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with
> TPM 2.0
> Implementation
>   CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy
> type to
> FixedAtBuild
> 
>  CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c |
> 121 ++--
>  CryptoPkg/CryptoPkg.dec   |
> 18 ++-
>  CryptoPkg/CryptoPkg.uni   |
> 12 +-
>  CryptoPkg/Include/Library/HashApiLib.h|
> 16 +--
>  4 files changed, 52 insertions(+), 115 deletions(-)
> 
> --
> 2.16.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54502): https://edk2.groups.io/g/devel/message/54502
Mute This Topic: https://groups.io/mt/71288763/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 0/3] Ovmf: enable TPM 1.2

[Marc-André Lureau]

Hi Yao

On Thu, Feb 13, 2020 at 2:51 PM Yao, Jiewen  wrote:
>
> Hi Lureau
> I don’t think we should expose the TPM Interface type via TpmCommandLib.
>
> That is the TPM device implementation. The TPM device might use TIS/FIFO/CRB, 
> but there might be also other type such as I2C, or fTPM implementation type.
>
> To distinguish TPM2.0 or TPM1.2, the standard way is to send startup command.

Thanks for the feedback, unfortunately I don't know how to achieve
this for both tpm/vtpm (uninitialized) & passthrough (initialized).

If the device is uninitialized, sending Tpm12Startup (TPM_ST_CLEAR) to
detect 1.2 in Tcg2ConfigPeimEntryPoint will work, but then
Tpm12Startup () in TcgPei:PeimEntryMA will later fail.

If the device is initialized/passthrough, sending Tpm12Startup
(TPM_ST_CLEAR) will fail, so it could send Tpm12Startup (TPM_ST_STATE)
instead. But that will fail to detect uninitialized 1.2 device.

I am stuck, any help welcome!

thanks

>
> Thank you
> Yao Jiewen
>
>
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of
> > marcandre.lur...@redhat.com
> > Sent: Thursday, February 13, 2020 9:12 PM
> > To: edk2-de...@lists.01.org; devel@edk2.groups.io
> > Cc: stef...@linux.ibm.com; ler...@redhat.com; simon.ha...@itdev.co.uk;
> > Marc-André Lureau 
> > Subject: [edk2-devel] [PATCH v2 0/3] Ovmf: enable TPM 1.2
> >
> > From: Marc-André Lureau 
> >
> > Hi,
> >
> > The following patches add basic TPM 1.2 support for Ovmf/QEMU.
> >
> > I tested successfully Win10 with TIS/TPM 1.2 & CRB/TPM 2.0
> > passthrough, and emulated CRB/TPM 2.0.
> > (fwiw, I haven't tried to enable TPM_CONFIG_ENABLE)
> >
> > Marc-André Lureau (3):
> >   Ovmf: rename TPM2 config prefix to TPM
> >   SecurityPkg: export Tpm12GetPtpInterfaceType()
> >   Ovmf: enable TPM 1.2 support
> >
> >  OvmfPkg/OvmfPkgIa32.dsc   | 39 +--
> >  OvmfPkg/OvmfPkgIa32.fdf   | 10 +++--
> >  OvmfPkg/OvmfPkgIa32X64.dsc| 39 +--
> >  OvmfPkg/OvmfPkgIa32X64.fdf| 10 +++--
> >  OvmfPkg/OvmfPkgX64.dsc| 39 +--
> >  OvmfPkg/OvmfPkgX64.fdf| 10 +++--
> >  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf  |  3 ++
> >  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c   | 17 +++-
> >  SecurityPkg/Include/Library/Tpm12DeviceLib.h  | 13 +++
> >  .../Library/Tpm12DeviceLibDTpm/Tpm12Tis.c | 17 
> >  10 files changed, 141 insertions(+), 56 deletions(-)
> >
> > --
> > 2.25.0.rc2.1.g09a9a1a997
> >
> >
> >
>
>
> 
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54501): https://edk2.groups.io/g/devel/message/54501
Mute This Topic: https://groups.io/mt/71240877/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 0/3] Ovmf: enable TPM 1.2

[Yao, Jiewen]

For tpm/vtpm (uninitialized), I think you can set PcdTpmInitializationPolicy to 
0 in the TPM detection module. As such, the TCG PEI will skip Startup() command.
Please refer to SecurityPkg\Tcg\Tcg2Config\TpmDetection.c.

I am not clear about the passthrough mode.
Which module initializes the TPM ? Can we let this module pass some information 
?

Thank you
Yao Jiewen

> -Original Message-
> From: Marc-André Lureau 
> Sent: Saturday, February 15, 2020 7:34 PM
> To: edk2-devel-groups-io ; Yao, Jiewen
> 
> Cc: edk2-de...@lists.01.org; stef...@linux.ibm.com; ler...@redhat.com;
> simon.ha...@itdev.co.uk
> Subject: Re: [edk2-devel] [PATCH v2 0/3] Ovmf: enable TPM 1.2
> 
> Hi Yao
> 
> On Thu, Feb 13, 2020 at 2:51 PM Yao, Jiewen  wrote:
> >
> > Hi Lureau
> > I don’t think we should expose the TPM Interface type via TpmCommandLib.
> >
> > That is the TPM device implementation. The TPM device might use
> TIS/FIFO/CRB, but there might be also other type such as I2C, or fTPM
> implementation type.
> >
> > To distinguish TPM2.0 or TPM1.2, the standard way is to send startup
> command.
> 
> Thanks for the feedback, unfortunately I don't know how to achieve
> this for both tpm/vtpm (uninitialized) & passthrough (initialized).
> 
> If the device is uninitialized, sending Tpm12Startup (TPM_ST_CLEAR) to
> detect 1.2 in Tcg2ConfigPeimEntryPoint will work, but then
> Tpm12Startup () in TcgPei:PeimEntryMA will later fail.
> 
> If the device is initialized/passthrough, sending Tpm12Startup
> (TPM_ST_CLEAR) will fail, so it could send Tpm12Startup (TPM_ST_STATE)
> instead. But that will fail to detect uninitialized 1.2 device.
> 
> I am stuck, any help welcome!
> 
> thanks
> 
> >
> > Thank you
> > Yao Jiewen
> >
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of
> > > marcandre.lur...@redhat.com
> > > Sent: Thursday, February 13, 2020 9:12 PM
> > > To: edk2-de...@lists.01.org; devel@edk2.groups.io
> > > Cc: stef...@linux.ibm.com; ler...@redhat.com; simon.ha...@itdev.co.uk;
> > > Marc-André Lureau 
> > > Subject: [edk2-devel] [PATCH v2 0/3] Ovmf: enable TPM 1.2
> > >
> > > From: Marc-André Lureau 
> > >
> > > Hi,
> > >
> > > The following patches add basic TPM 1.2 support for Ovmf/QEMU.
> > >
> > > I tested successfully Win10 with TIS/TPM 1.2 & CRB/TPM 2.0
> > > passthrough, and emulated CRB/TPM 2.0.
> > > (fwiw, I haven't tried to enable TPM_CONFIG_ENABLE)
> > >
> > > Marc-André Lureau (3):
> > >   Ovmf: rename TPM2 config prefix to TPM
> > >   SecurityPkg: export Tpm12GetPtpInterfaceType()
> > >   Ovmf: enable TPM 1.2 support
> > >
> > >  OvmfPkg/OvmfPkgIa32.dsc   | 39 +--
> > >  OvmfPkg/OvmfPkgIa32.fdf   | 10 +++--
> > >  OvmfPkg/OvmfPkgIa32X64.dsc| 39 +--
> > >  OvmfPkg/OvmfPkgIa32X64.fdf| 10 +++--
> > >  OvmfPkg/OvmfPkgX64.dsc| 39 +--
> > >  OvmfPkg/OvmfPkgX64.fdf| 10 +++--
> > >  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf  |  3 ++
> > >  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c   | 17 +++-
> > >  SecurityPkg/Include/Library/Tpm12DeviceLib.h  | 13 +++
> > >  .../Library/Tpm12DeviceLibDTpm/Tpm12Tis.c | 17 
> > >  10 files changed, 141 insertions(+), 56 deletions(-)
> > >
> > > --
> > > 2.25.0.rc2.1.g09a9a1a997
> > >
> > >
> > >
> >
> >
> > 
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54500): https://edk2.groups.io/g/devel/message/54500
Mute This Topic: https://groups.io/mt/71240877/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 0/1] Add PCD to disable safe string constraint assertions

[Vitaly Cheptsov via Groups.Io]

It seems that edk2.groups.io  has hit the limits for 
message size due to quotation levels (?), so I send another copy of my previous 
message to have it visible from the web-interface.

Andrew,

It is ok, as we are all here for mutual benefit, no worries. But you are right 
that we indeed want a solution for BZ 2054[1] to land as early as possible. It 
is great that after more than half a year :) we have some productive 
discussion, so I am all open for it.

* The compliant about a breaking change in DebugLib is honestly fair, and I 
wonder whether we could avoid it. The reason it potentially breaks is the 
introduction of DebugConstraintAssertEnabled function that will need to land in 
every DebugLib implementation. I think we can look differently at this problem. 
Currently functions like DebugAssertEnabled, DebugPrintEnabled, 
DebugClearMemoryEnabled, and so on are copy-pasted from one DebugLib to another 
introducing almost 100 lines of exactly the same code every DebugLib 
implementation should write. It can be argued that one can implement these 
functions differently, but neither the description in DebugLib.h permits this, 
nor I have seen any implementation doing it so far. I believe we should check 
Pcd values directly in DebugLib.h header, which has a lot of pros including the 
backwards compatibility resolution:

— reduction of code copy-pasting.
— making compilers without LTO produce smaller binaries.
— making third-party DebugLib implementations more flexible and easier to 
maintain.
— resolving the problem of third-party DebugLib implementations not working 
with CONSTRAINT_ASSERT.

To make it less intrusive we can provide an iterative approach:
1. Introduce Pcd checking macros like DEBUG_PRINT_ENABLED, DEBUG_ASSERT_ENABLED:
#define DEBUG_PRINT_ENABLED() (BOOLEAN) ((PcdGet8(PcdDebugPropertyMask) & 
DEBUG_PROPERTY_DEBUG_PRINT_ENABLED) != 0)
2. Switch DEBUG, ASSERT, and other related macros to use these new macros by 
replacing calls like DebugPrintEnabled() with macros like DEBUG_PRINT_ENABLED().
3. Introduce DEBUG_CONSTRAINT_ASSERT_ENABLED and CONSTRAINT_ASSERT as discussed 
previously except DebugConstraintAssertEnabled is now a macro.
4. Remove DebugPrintEnabled and other functions from EDK II DebugLib 
implementations and header.

Step 4 can be done through deprecation phase. However, I think these functions 
are not used anywhere but inside DebugLib implementations anyway, and the way 
they are structured should let still legacy third-party DebugLib 
implementations still having these functions to compile fine even without the 
immediate refactoring.

* The point of AssertMacros and changing the world is valid, but to be frank 
with you I have an opinion that we should not incorporate this experience in 
EDK II. I do not know if these macros are used in Mac EFI code, as my only 
experience with them was during XNU kernelspace programming, but I do not have 
a memory of them bringing enough benefit. Basically, macro constructions that 
cannot be easily expressed with normal C functions, especially ones changing 
control flow with something like a goto, are unintuitive and overcomplicated. 
They make the code harder to read, especially to those not familiar with the 
rest of the codebase, for little to no reason. While there certainly may be 
some positive sides in these macros, like improved structural separation by 
introducing different categories of checks, I am afraid we do not need them as 
is. Even if we do implement something close in the future, I believe they 
should rather exist in a separate library and be an opt-in for newly 
contributed code.

* The point of caller/callee control on the constraint violation is slightly 
tricky. We have already spent some time trying to nail it down, and I believe 
there is a good level of logic that can be split in two parts:

1. Assertion type choose. Which to write ASSERT or CONSTRAINT_ASSERT?

You can rely on a simple thought experiment here. Does failing this assertion 
make the function work in the conditions it was not meant to be? I.e. will the 
function break or crash or will some other spec/doc-compliant implementations 
of this potentially do. When the answer is affirmative, it is an ASSERT, 
otherwise it is a CONSTRAINT_ASSERT. Examples in the BZ mentioning SafeString 
give a good grasp of the use cases.

2. Assertion handling choice. When do we want ASSERT or CONSTRAINT_ASSERT to be 
enabled?

This is more likely what your question was about. First is that 
CONSTRAINT_ASSERTs only make sense to enable in DEBUG builds where ASSERTs are 
enabled. So the question comes to whether DEBUG builds should behave the same 
way as RELEASE builds functionality-wise or not. Let’s take an another simple 
thought experiment: should we halt in a build when external untrusted data 
theoretically exists and is not valid? If the answer is yes, then 
CONSTRAINT_ASSERT should be enabled in a DEBUG build, otherwise no. 

Re: [edk2-devel] [PATCH v3 0/1] Add PCD to disable safe string constraint assertions

[Vitaly Cheptsov via Groups.Io]